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/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]
-=-=-=-=-=-=-=-=-=-=-=-

Reply via email to