[edk2-devel] [PATCH v3 4/5] OvmfPkg/Tcg2ConfigPei: Mark TPM MMIO range as unencrypted for SEV-ES
Laszlo Ersek
lersek at redhat.com
Fri Apr 30 17:01:41 UTC 2021
On 04/29/21 19:12, Lendacky, Thomas wrote:
> BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=3345
>
> 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. This new
> PEIM will depend on the installation of the permanent PEI RAM, by
> PlatformPei, so that in case page table splitting is required during the
> clearing of the encryption bit, the new page table(s) will be allocated
> from permanent PEI RAM.
>
> Update all OVMF Ia32 and X64 build packages to include this new PEIM.
>
> Cc: Laszlo Ersek <lersek at redhat.com>
> Cc: Ard Biesheuvel <ardb+tianocore at kernel.org>
> Cc: Jordan Justen <jordan.l.justen at intel.com>
> Cc: Brijesh Singh <brijesh.singh at amd.com>
> Cc: Erdem Aktas <erdemaktas at google.com>
> Cc: James Bottomley <jejb at linux.ibm.com>
> Cc: Jiewen Yao <jiewen.yao at intel.com>
> Cc: Min Xu <min.m.xu at intel.com>
> Cc: Marc-André Lureau <marcandre.lureau at redhat.com>
> Cc: Stefan Berger <stefanb at linux.ibm.com>
> Signed-off-by: Tom Lendacky <thomas.lendacky at amd.com>
> ---
> OvmfPkg/AmdSev/AmdSevX64.dsc | 1 +
> OvmfPkg/OvmfPkgIa32.dsc | 1 +
> OvmfPkg/OvmfPkgIa32X64.dsc | 1 +
> OvmfPkg/OvmfPkgX64.dsc | 1 +
> OvmfPkg/AmdSev/AmdSevX64.fdf | 1 +
> OvmfPkg/OvmfPkgIa32.fdf | 1 +
> OvmfPkg/OvmfPkgIa32X64.fdf | 1 +
> OvmfPkg/OvmfPkgX64.fdf | 1 +
> OvmfPkg/Tcg/TpmMmioSevDecryptPei/TpmMmioSevDecryptPei.inf | 40 +++++++++
> OvmfPkg/Tcg/TpmMmioSevDecryptPei/TpmMmioSevDecryptPeim.c | 87 ++++++++++++++++++++
> 10 files changed, 135 insertions(+)
>
> diff --git a/OvmfPkg/AmdSev/AmdSevX64.dsc b/OvmfPkg/AmdSev/AmdSevX64.dsc
> index cdb29d53142d..66bbbc80cd18 100644
> --- a/OvmfPkg/AmdSev/AmdSevX64.dsc
> +++ b/OvmfPkg/AmdSev/AmdSevX64.dsc
> @@ -626,6 +626,7 @@ [Components]
> OvmfPkg/AmdSev/SecretPei/SecretPei.inf
>
> !if $(TPM_ENABLE) == TRUE
> + OvmfPkg/Tcg/TpmMmioSevDecryptPei/TpmMmioSevDecryptPei.inf
> OvmfPkg/Tcg/Tcg2Config/Tcg2ConfigPei.inf
> SecurityPkg/Tcg/TcgPei/TcgPei.inf
> SecurityPkg/Tcg/Tcg2Pei/Tcg2Pei.inf {
> diff --git a/OvmfPkg/OvmfPkgIa32.dsc b/OvmfPkg/OvmfPkgIa32.dsc
> index 1730b6558b5c..33fbd767903e 100644
> --- a/OvmfPkg/OvmfPkgIa32.dsc
> +++ b/OvmfPkg/OvmfPkgIa32.dsc
> @@ -706,6 +706,7 @@ [Components]
> UefiCpuPkg/CpuMpPei/CpuMpPei.inf
>
> !if $(TPM_ENABLE) == TRUE
> + OvmfPkg/Tcg/TpmMmioSevDecryptPei/TpmMmioSevDecryptPei.inf
> OvmfPkg/Tcg/Tcg2Config/Tcg2ConfigPei.inf
> SecurityPkg/Tcg/TcgPei/TcgPei.inf
> SecurityPkg/Tcg/Tcg2Pei/Tcg2Pei.inf {
> diff --git a/OvmfPkg/OvmfPkgIa32X64.dsc b/OvmfPkg/OvmfPkgIa32X64.dsc
> index 78a559da0d0b..b13e5cfd9047 100644
> --- a/OvmfPkg/OvmfPkgIa32X64.dsc
> +++ b/OvmfPkg/OvmfPkgIa32X64.dsc
> @@ -719,6 +719,7 @@ [Components.IA32]
> UefiCpuPkg/CpuMpPei/CpuMpPei.inf
>
> !if $(TPM_ENABLE) == TRUE
> + OvmfPkg/Tcg/TpmMmioSevDecryptPei/TpmMmioSevDecryptPei.inf
> OvmfPkg/Tcg/Tcg2Config/Tcg2ConfigPei.inf
> SecurityPkg/Tcg/TcgPei/TcgPei.inf
> SecurityPkg/Tcg/Tcg2Pei/Tcg2Pei.inf {
> diff --git a/OvmfPkg/OvmfPkgX64.dsc b/OvmfPkg/OvmfPkgX64.dsc
> index a7d747f6b4ab..999738dc39cd 100644
> --- a/OvmfPkg/OvmfPkgX64.dsc
> +++ b/OvmfPkg/OvmfPkgX64.dsc
> @@ -718,6 +718,7 @@ [Components]
> UefiCpuPkg/CpuMpPei/CpuMpPei.inf
>
> !if $(TPM_ENABLE) == TRUE
> + OvmfPkg/Tcg/TpmMmioSevDecryptPei/TpmMmioSevDecryptPei.inf
> OvmfPkg/Tcg/Tcg2Config/Tcg2ConfigPei.inf
> SecurityPkg/Tcg/TcgPei/TcgPei.inf
> SecurityPkg/Tcg/Tcg2Pei/Tcg2Pei.inf {
> diff --git a/OvmfPkg/AmdSev/AmdSevX64.fdf b/OvmfPkg/AmdSev/AmdSevX64.fdf
> index c0098502aa90..dd0030dbf189 100644
> --- a/OvmfPkg/AmdSev/AmdSevX64.fdf
> +++ b/OvmfPkg/AmdSev/AmdSevX64.fdf
> @@ -147,6 +147,7 @@ [FV.PEIFV]
> INF OvmfPkg/AmdSev/SecretPei/SecretPei.inf
>
> !if $(TPM_ENABLE) == TRUE
> +INF OvmfPkg/Tcg/TpmMmioSevDecryptPei/TpmMmioSevDecryptPei.inf
> INF OvmfPkg/Tcg/Tcg2Config/Tcg2ConfigPei.inf
> INF SecurityPkg/Tcg/TcgPei/TcgPei.inf
> INF SecurityPkg/Tcg/Tcg2Pei/Tcg2Pei.inf
> diff --git a/OvmfPkg/OvmfPkgIa32.fdf b/OvmfPkg/OvmfPkgIa32.fdf
> index f400c845b9c9..b3c8b56f3b60 100644
> --- a/OvmfPkg/OvmfPkgIa32.fdf
> +++ b/OvmfPkg/OvmfPkgIa32.fdf
> @@ -162,6 +162,7 @@ [FV.PEIFV]
> INF UefiCpuPkg/CpuMpPei/CpuMpPei.inf
>
> !if $(TPM_ENABLE) == TRUE
> +INF OvmfPkg/Tcg/TpmMmioSevDecryptPei/TpmMmioSevDecryptPei.inf
> INF OvmfPkg/Tcg/Tcg2Config/Tcg2ConfigPei.inf
> INF SecurityPkg/Tcg/TcgPei/TcgPei.inf
> INF SecurityPkg/Tcg/Tcg2Pei/Tcg2Pei.inf
> diff --git a/OvmfPkg/OvmfPkgIa32X64.fdf b/OvmfPkg/OvmfPkgIa32X64.fdf
> index d055552fd09f..86592c2364a3 100644
> --- a/OvmfPkg/OvmfPkgIa32X64.fdf
> +++ b/OvmfPkg/OvmfPkgIa32X64.fdf
> @@ -162,6 +162,7 @@ [FV.PEIFV]
> INF UefiCpuPkg/CpuMpPei/CpuMpPei.inf
>
> !if $(TPM_ENABLE) == TRUE
> +INF OvmfPkg/Tcg/TpmMmioSevDecryptPei/TpmMmioSevDecryptPei.inf
> INF OvmfPkg/Tcg/Tcg2Config/Tcg2ConfigPei.inf
> INF SecurityPkg/Tcg/TcgPei/TcgPei.inf
> INF SecurityPkg/Tcg/Tcg2Pei/Tcg2Pei.inf
> diff --git a/OvmfPkg/OvmfPkgX64.fdf b/OvmfPkg/OvmfPkgX64.fdf
> index d519f8532822..d6be798fcadd 100644
> --- a/OvmfPkg/OvmfPkgX64.fdf
> +++ b/OvmfPkg/OvmfPkgX64.fdf
> @@ -174,6 +174,7 @@ [FV.PEIFV]
> INF UefiCpuPkg/CpuMpPei/CpuMpPei.inf
>
> !if $(TPM_ENABLE) == TRUE
> +INF OvmfPkg/Tcg/TpmMmioSevDecryptPei/TpmMmioSevDecryptPei.inf
> INF OvmfPkg/Tcg/Tcg2Config/Tcg2ConfigPei.inf
> INF SecurityPkg/Tcg/TcgPei/TcgPei.inf
> INF SecurityPkg/Tcg/Tcg2Pei/Tcg2Pei.inf
> diff --git a/OvmfPkg/Tcg/TpmMmioSevDecryptPei/TpmMmioSevDecryptPei.inf b/OvmfPkg/Tcg/TpmMmioSevDecryptPei/TpmMmioSevDecryptPei.inf
> new file mode 100644
> index 000000000000..51ad6d0d055d
> --- /dev/null
> +++ b/OvmfPkg/Tcg/TpmMmioSevDecryptPei/TpmMmioSevDecryptPei.inf
> @@ -0,0 +1,40 @@
> +## @file
> +# Map TPM MMIO range unencrypted when SEV-ES is active.
> +# Install gOvmfTpmMmioAccessiblePpiGuid unconditionally.
> +#
> +# Copyright (C) 2021, Advanced Micro Devices, Inc.
> +#
> +# SPDX-License-Identifier: BSD-2-Clause-Patent
> +##
> +
> +[Defines]
> + INF_VERSION = 1.29
> + BASE_NAME = TpmMmioSevDecryptPei
> + FILE_GUID = F12F698A-E506-4A1B-B32E-6920E55DA1C4
> + MODULE_TYPE = PEIM
> + VERSION_STRING = 1.0
> + ENTRY_POINT = TpmMmioSevDecryptPeimEntryPoint
> +
> +[Sources]
> + TpmMmioSevDecryptPeim.c
> +
> +[Packages]
> + MdePkg/MdePkg.dec
> + OvmfPkg/OvmfPkg.dec
> + SecurityPkg/SecurityPkg.dec
> +
> +[LibraryClasses]
> + DebugLib
> + MemEncryptSevLib
> + PcdLib
> + PeimEntryPoint
> + PeiServicesLib
> +
> +[Ppis]
> + gOvmfTpmMmioAccessiblePpiGuid ## PRODUCES
> +
> +[FixedPcd]
> + gEfiSecurityPkgTokenSpaceGuid.PcdTpmBaseAddress ## CONSUMES
> +
> +[Depex]
> + gEfiPeiMemoryDiscoveredPpiGuid
> diff --git a/OvmfPkg/Tcg/TpmMmioSevDecryptPei/TpmMmioSevDecryptPeim.c b/OvmfPkg/Tcg/TpmMmioSevDecryptPei/TpmMmioSevDecryptPeim.c
> new file mode 100644
> index 000000000000..df2ad623308d
> --- /dev/null
> +++ b/OvmfPkg/Tcg/TpmMmioSevDecryptPei/TpmMmioSevDecryptPeim.c
> @@ -0,0 +1,87 @@
> +/** @file
> + Map TPM MMIO range unencrypted when SEV-ES is active.
> + Install gOvmfTpmMmioAccessiblePpiGuid unconditionally.
> +
> + Copyright (C) 2021, Advanced Micro Devices, Inc.
> +
> + SPDX-License-Identifier: BSD-2-Clause-Patent
> +**/
> +
> +
> +#include <PiPei.h>
> +
> +#include <Library/DebugLib.h>
> +#include <Library/MemEncryptSevLib.h>
> +#include <Library/PcdLib.h>
> +#include <Library/PeiServicesLib.h>
> +
> +STATIC CONST EFI_PEI_PPI_DESCRIPTOR mTpmMmioRangeAccessible = {
> + EFI_PEI_PPI_DESCRIPTOR_PPI | EFI_PEI_PPI_DESCRIPTOR_TERMINATE_LIST,
> + &gOvmfTpmMmioAccessiblePpiGuid,
> + NULL
> +};
> +
> +/**
> + The entry point for TPM MMIO range mapping driver.
> +
> + @param[in] FileHandle Handle of the file being invoked.
> + @param[in] PeiServices Describes the list of possible PEI Services.
> +
> + @retval EFI_ABORTED No need to keep this PEIM resident
> +**/
> +EFI_STATUS
> +EFIAPI
> +TpmMmioSevDecryptPeimEntryPoint (
> + IN EFI_PEI_FILE_HANDLE FileHandle,
> + IN CONST EFI_PEI_SERVICES **PeiServices
> + )
> +{
> + RETURN_STATUS DecryptStatus;
> + EFI_STATUS Status;
> +
> + DEBUG ((DEBUG_INFO, "%a\n", __FUNCTION__));
> +
> + //
> + // If SEV 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.
> + //
> + // If SEV-ES is active, MMIO would succeed against an encrypted physical
> + // address because the #VC handler uses the virtual address (which is an
> + // identity mapped physical address without the encryption bit) as the guest
> + // physical address of the MMIO target in the VMGEXIT.
> + //
> + // 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/from an unencrypted address. To prevent guest
> + // termination in this scenario, mark the range unencrypted ahead of access.
> + //
> + if (MemEncryptSevEsIsEnabled ()) {
> + DEBUG ((DEBUG_INFO,
> + "%a: mapping TPM MMIO address range unencrypted\n",
> + __FUNCTION__));
> +
> + DecryptStatus = MemEncryptSevClearPageEncMask (
> + 0,
> + FixedPcdGet64 (PcdTpmBaseAddress),
> + EFI_SIZE_TO_PAGES ((UINTN) 0x5000),
> + FALSE
> + );
> +
> + if (RETURN_ERROR (DecryptStatus)) {
> + DEBUG ((DEBUG_ERROR,
> + "%a: failed to map TPM MMIO address range unencrypted\n",
> + __FUNCTION__));
> + ASSERT_RETURN_ERROR (DecryptStatus);
> + }
> + }
> +
> + //
> + // MMIO range available
> + //
> + Status = PeiServicesInstallPpi (&mTpmMmioRangeAccessible);
> + ASSERT_EFI_ERROR (Status);
> +
> + return EFI_ABORTED;
> +}
>
Reviewed-by: Laszlo Ersek <lersek at redhat.com>
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#74664): https://edk2.groups.io/g/devel/message/74664
Mute This Topic: https://groups.io/mt/82461201/1813853
Group Owner: devel+owner at edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [edk2-devel-archive at redhat.com]
-=-=-=-=-=-=-=-=-=-=-=-
More information about the edk2-devel-archive
mailing list