On 10/2/19 7:05 AM, Laszlo Ersek via Groups.Io wrote: > On 09/19/19 21:52, Lendacky, Thomas wrote: >> From: Tom Lendacky <thomas.lenda...@amd.com> >> >> BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=2198 >> >> The SEV support will clear the C-bit from non-RAM areas. The early GDT >> lives in a non-RAM area, so when an exception occurs (like a #VC) the GDT >> will be read as un-encrypted even though it is encrypted. This will result >> in a failure to be able to handle the exception. >> >> Move the GDT into RAM so it can be accessed without error when running as >> an SEV-ES guest. >> >> Cc: Jordan Justen <jordan.l.jus...@intel.com> >> Cc: Laszlo Ersek <ler...@redhat.com> >> Cc: Ard Biesheuvel <ard.biesheu...@linaro.org> >> Signed-off-by: Tom Lendacky <thomas.lenda...@amd.com> >> --- >> OvmfPkg/PlatformPei/AmdSev.c | 16 ++++++++++++++++ >> 1 file changed, 16 insertions(+) >> >> diff --git a/OvmfPkg/PlatformPei/AmdSev.c b/OvmfPkg/PlatformPei/AmdSev.c >> index 699bb8b11557..d6733447bdf2 100644 >> --- a/OvmfPkg/PlatformPei/AmdSev.c >> +++ b/OvmfPkg/PlatformPei/AmdSev.c >> @@ -37,6 +37,8 @@ AmdSevEsInitialize ( >> PHYSICAL_ADDRESS GhcbBasePa; >> UINTN GhcbPageCount; >> RETURN_STATUS PcdStatus, DecryptStatus; >> + IA32_DESCRIPTOR Gdtr; >> + VOID *Gdt; >> >> if (!MemEncryptSevEsIsEnabled ()) { >> return; >> @@ -72,6 +74,20 @@ AmdSevEsInitialize ( >> DEBUG ((DEBUG_INFO, "SEV-ES is enabled, %u GHCB pages allocated starting >> at 0x%lx\n", GhcbPageCount, GhcbBase)); >> >> AsmWriteMsr64 (MSR_SEV_ES_GHCB, (UINT64)GhcbBasePa); >> + >> + // >> + // The SEV support will clear the C-bit from the non-RAM areas. Since >> + // the GDT initially lives in that area and it will be read when a #VC >> + // exception happens, it needs to be moved to RAM for an SEV-ES guest. >> + // >> + AsmReadGdtr (&Gdtr); >> + >> + Gdt = AllocatePages (EFI_SIZE_TO_PAGES (Gdtr.Limit + 1)); > > EFI_SIZE_TO_PAGES() expects an UINTN argument. "Gdtr.Limit" is UINT16 > ("unsigned short"), which is promoted (as part of the integer > promotions) to INT32 ("int"). The addition is then performed in INT32. > > (1) Please cast either Gdtr.Limit, or the sum, to UINTN explicitly.
Will do. > >> + ASSERT (Gdt); > > (2) Please write (Gdt != NULL). Yup. Thanks, Tom > >> + >> + CopyMem (Gdt, (VOID *) Gdtr.Base, Gdtr.Limit + 1); >> + Gdtr.Base = (UINTN) Gdt; >> + AsmWriteGdtr (&Gdtr); >> } >> >> /** >> > > With those changes: > > Reviewed-by: Laszlo Ersek <ler...@redhat.com> > > Thanks > Laszlo > > > -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#48380): https://edk2.groups.io/g/devel/message/48380 Mute This Topic: https://groups.io/mt/34203548/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
