[edk2-devel] [RFC PATCH v3 30/43] OvmfPkg/Sec: Add #VC exception handling for Sec phase
Lendacky, Thomas via Groups.Io
thomas.lendacky=amd.com at groups.io
Fri Nov 22 22:48:00 UTC 2019
On 11/22/19 3:10 PM, Laszlo Ersek wrote:
> 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.
I haven't done the WriteIdtr() stuff, yet, but the passing of the SEV-ES
status from the ResetVector code to the SEC code via the SEV-ES page at
0x0080_B000 is working nicely.
Thanks,
Tom
>
>> 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 (#51200): https://edk2.groups.io/g/devel/message/51200
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