[edk2-devel] [RFC PATCH v3 30/43] OvmfPkg/Sec: Add #VC exception handling for Sec phase
Laszlo Ersek via Groups.Io
lersek=redhat.com at groups.io
Fri Nov 22 21:10:44 UTC 2019
On 11/22/19 17:30, Tom Lendacky wrote:
> On 11/22/19 6:52 AM, Laszlo Ersek wrote:
>> On 11/21/19 21:46, Tom Lendacky wrote:
>>> On 11/21/19 6:06 AM, Laszlo Ersek wrote:
>>>> On 11/20/19 21:06, Lendacky, Thomas wrote:
>>>>> @@ -737,6 +738,21 @@ SecCoreStartupWithStack (
>>>>> Table[Index] = 0;
>>>>> }
>>>>>
>>>>> + //
>>>>> + // Initialize IDT
>>>>> + //
>>>>> + IdtTableInStack.PeiService = NULL;
>>>>> + for (Index = 0; Index < SEC_IDT_ENTRY_COUNT; Index ++) {
>>>>> + CopyMem (&IdtTableInStack.IdtTable[Index], &mIdtEntryTemplate,
>>>>> sizeof (mIdtEntryTemplate));
>>>>> + }
>>>>> +
>>>>> + IdtDescriptor.Base = (UINTN)&IdtTableInStack.IdtTable;
>>>>> + IdtDescriptor.Limit = (UINT16)(sizeof (IdtTableInStack.IdtTable)
>>>>> - 1);
>>>>> +
>>>>> + AsmWriteIdtr (&IdtDescriptor);
>>>>> +
>>>>> + InitializeCpuExceptionHandlers (NULL);
>>>>> +
>>>>> ProcessLibraryConstructorList (NULL, NULL);
>>>>>
>>>>> DEBUG ((EFI_D_INFO,
>>>>
>>>> (1) The problem here is that we call multiple library APIs before
>>>> calling ProcessLibraryConstructorList() -- namely CopyMem(),
>>>> AsmWriteIdtr(), and InitializeCpuExceptionHandlers().
>
> We can reduce this exposure a bit and replace the CopyMem() call with
> something similar to the loop above it.
That would be nice, if you're not too annoyed by the extra busywork.
> I could also use assembler code directly in here to load the IDTR.
I think it would be enough to copy
MdePkg/Library/BaseLib/Ia32/WriteIdtr.nasm
MdePkg/Library/BaseLib/X64/WriteIdtr.nasm
under OvmfPkg/Sec, and use a new function name.
> That would leave just InitializeCpuExceptionHandlers(). Is there
> something that can added so as to warn when a library has a
> CONSTRUCTOR added to/part of the definition?
Nothing comes to my mind :(
[...]
>> (1) Append a UINT8 ("BOOLEAN") field to the SEV_ES_AP_JMP_FAR structure
>> (and possibly rename the structure),
>>
>> (2) in the OvmfPkgX64 reset vector, where you determine SEV-ES anyway,
>> explicitly set this field to zero if SEV-ES is disabled, and set it to
>> one, if SEV-ES is enabled,
>>
>> (3) In OvmfPkg/Sec, introduce a new (local) header file declaring the
>> function SevEsIsEnabled(),
>>
>> (4) Provide two C-language implementations (under the Ia32 and X64
>> directories): in the 32-bit version, return constant FALSE; in the
>> 64-bit version, return the value of the new field. Something like:
>>
>> return ((SEV_ES_AP_JMP_FAR *)FixedPcdGet32
>> (PcdSevEsResetRipBase))->SevEsEnabled;
>>
>> FixedPcdGet32() is explicitly safe to use without library constructors
>> having run.
>>
>> Does this look viable? (It might require you to reshuffle patch 37 vs.
>> patch 30.)
>
> I think this does. Since this is SEC and the reset vector page isn't
> needed until PEI and later we could even just use the first byte (make a
> union with an SEC usage field) and make this even simpler. Then we don't
> have to worry about positioning it.
Ah, nice.
> Let me work on that and see where I
> get. Anything after the #VC is established would use the current method
> of determine SEV-ES status.
Thanks!
Laszlo
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#51199): https://edk2.groups.io/g/devel/message/51199
Mute This Topic: https://groups.io/mt/60973137/1813853
Mute #vc: https://groups.io/mk?hashtag=vc&subid=3943202
Group Owner: devel+owner at edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [edk2-devel-archive at redhat.com]
-=-=-=-=-=-=-=-=-=-=-=-
More information about the edk2-devel-archive
mailing list