Re: [PATCH v2 4/4] OvmfPkg/Tcg2ConfigPei: Mark TPM MMIO range as unencrypted for SEV-ES


Lendacky, Thomas
 

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.lendacky@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&amp;sdata=G1GwQc6sZqRuNHWC5vbdb78gCOl4YkAq%2BHi0F0ceucg%3D&amp;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





Join devel@edk2.groups.io to automatically receive all group messages.