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/21656 Mute #vc: https://groups.io/mk?hashtag=vc&subid=3846945 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-