On 4/28/21 2:43 PM, Tom Lendacky wrote:
> On 4/28/21 12:51 PM, Laszlo Ersek via groups.io wrote:
>> I'm going to ask for v3 after all:
>>
>> On 04/27/21 18:21, Lendacky, Thomas wrote:
>>> From: Tom Lendacky <thomas.lenda...@amd.com>
>>>
>>> BZ:
>>> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fbugzilla.tianocore.org%2Fshow_bug.cgi%3Fid%3D3345&data=04%7C01%7Cthomas.lendacky%40amd.com%7C3c65ebfe044e4f3eb5b808d90a6e5455%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637552291252644310%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=G1GwQc6sZqRuNHWC5vbdb78gCOl4YkAq%2BHi0F0ceucg%3D&reserved=0
>>>
>>> During PEI, the MMIO range for the TPM is marked as encrypted when running
>>> as an SEV guest. While this isn't an issue for an SEV guest because of
>>> the way the nested page fault is handled, it does result in an SEV-ES
>>> guest terminating because of a mitigation check in the #VC handler to
>>> prevent MMIO to an encrypted address. For an SEV-ES guest, this range
>>> must be marked as unencrypted.
>>>
>>> Create a new x86 PEIM for TPM support that will map the TPM MMIO range as
>>> unencrypted when SEV-ES is active. The gOvmfTpmMmioAccessiblePpiGuid PPI
>>> will be unconditionally installed before exiting. The PEIM will exit with
>>> the EFI_ABORTED status so that the PEIM does not stay resident.
>>
...
>>> +
>>> + //
>>> + // If SEV or SEV-ES is active, MMIO succeeds against an encrypted
>>> physical
>>> + // address because the nested page fault (NPF) that occurs on access
>>> does not
>>> + // include the encryption bit in the guest physical address provided to
>>> the
>>> + // hypervisor.
>>> + //
>>> + // However, if SEV-ES is active, before performing the actual MMIO, an
>>> + // additional MMIO mitigation check is performed in the #VC handler to
>>> ensure
>>> + // that MMIO is being done to an unencrypted address. To prevent guest
>>> + // termination in this scenario, mark the range unencrypted ahead of
>>> access.
>>> + //
>>
>> Lovely comment, thanks!
I'm going to expand on this a bit more to really show the distinction
between SEV and SEV-ES when it comes to MMIO. Look for a bit more info in v3.
Thanks,
Tom
>>
>>> + if (MemEncryptSevEsIsEnabled ()) {
>>> + DEBUG ((DEBUG_INFO, "%a: mapping TPM MMIO address range
>>> unencrypted\n", __FUNCTION__));
>>> +
>>> + DecryptStatus = MemEncryptSevClearPageEncMask (
>>> + 0,
>>> + PcdGet64 (PcdTpmBaseAddress),
>>
>> (11) The INF file says [FixedPcd], so it would be cleanest to say
>> FixedPcdGet64() here.
>
> Will do.
>
>>
>>
>> (12) PcdLib is missing from both the [LibraryClasses] section and the
>> #include directives.
>
> Right, I'll update that.
>
>>
>>
>>> + EFI_SIZE_TO_PAGES ((UINTN) 0x5000),
>>> + FALSE
>>> + );
>>> +
>>> + if (RETURN_ERROR (DecryptStatus)) {
>>> + DEBUG ((DEBUG_INFO, "%a: failed to map TPM MMIO address range
>>> unencrypted\n", __FUNCTION__));
>>
>> (13) Overlong line.
>
> Ok, I'll change that. I though that was ok now since PatchCheck.py didn't
> complain.
>
>>
>>
>> (14) Please report errors with DEBUG_ERROR.
>
> Yup, will change.
>
> Thanks,
> Tom
>
>>
>>
>>> + ASSERT_RETURN_ERROR (DecryptStatus);
>>> + }
>>> + }
>>> +
>>> + //
>>> + // MMIO range available
>>> + //
>>> + Status = PeiServicesInstallPpi (&mTpmMmioRangeAccessible);
>>> + ASSERT_EFI_ERROR (Status);
>>> +
>>> + return EFI_ABORTED;
>>> +}
>>>
>>
>> Thanks!
>> Laszlo
>>
>>
>>
>>
>>
>>
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#74592): https://edk2.groups.io/g/devel/message/74592
Mute This Topic: https://groups.io/mt/82407869/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
