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.lendacky@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.justen@intel.com>
>> Cc: Laszlo Ersek <lersek@redhat.com>
>> Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>> Signed-off-by: Tom Lendacky <thomas.lendacky@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 <lersek@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/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-