Re: [edk2-devel] [PATCH 3/3] OvmfPkg/PlatformPei: Mark TPM MMIO range as unencrypted for SEV

["Lendacky, Thomas" <thomas.lendacky@amd.com>]

On 4/20/21 5:54 PM, Lendacky, Thomas via groups.io wrote:
> From: Tom Lendacky <thomas.lendacky@amd.com>
> 
> BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=3345
> 
> The TPM support in OVMF performs MMIO accesses during the PEI phase. At
> this point, MMIO ranges have not been marked un-encyrpted, so an SEV-ES
> guest will fail attempting to perform MMIO to an encrypted address.
> 
> Read the PcdTpmBaseAddress and mark the specification defined range
> (0x5000 in length) as un-encrypted, to allow an SEV-ES guest to process
> the MMIO requests.
> 
> Cc: Laszlo Ersek <lersek@redhat.com>
> Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>
> Cc: Jordan Justen <jordan.l.justen@intel.com>
> Cc: Brijesh Singh <brijesh.singh@amd.com>
> Cc: James Bottomley <jejb@linux.ibm.com>
> Cc: Jiewen Yao <jiewen.yao@intel.com>
> Cc: Min Xu <min.m.xu@intel.com>
> Signed-off-by: Tom Lendacky <thomas.lendacky@amd.com>
> ---
>  OvmfPkg/PlatformPei/PlatformPei.inf |  1 +
>  OvmfPkg/PlatformPei/AmdSev.c        | 19 +++++++++++++++++++
>  2 files changed, 20 insertions(+)
> 
> diff --git a/OvmfPkg/PlatformPei/PlatformPei.inf b/OvmfPkg/PlatformPei/PlatformPei.inf
> index 6ef77ba7bb21..de60332e9390 100644
> --- a/OvmfPkg/PlatformPei/PlatformPei.inf
> +++ b/OvmfPkg/PlatformPei/PlatformPei.inf
> @@ -113,6 +113,7 @@ [Pcd]
>  
>  [FixedPcd]
>    gEfiMdePkgTokenSpaceGuid.PcdPciExpressBaseAddress
> +  gEfiSecurityPkgTokenSpaceGuid.PcdTpmBaseAddress
>    gEmbeddedTokenSpaceGuid.PcdMemoryTypeEfiACPIMemoryNVS
>    gEmbeddedTokenSpaceGuid.PcdMemoryTypeEfiACPIReclaimMemory
>    gEmbeddedTokenSpaceGuid.PcdMemoryTypeEfiReservedMemoryType
> diff --git a/OvmfPkg/PlatformPei/AmdSev.c b/OvmfPkg/PlatformPei/AmdSev.c
> index dddffdebda4b..d524929f9e10 100644
> --- a/OvmfPkg/PlatformPei/AmdSev.c
> +++ b/OvmfPkg/PlatformPei/AmdSev.c
> @@ -141,6 +141,7 @@ AmdSevInitialize (
>    )
>  {
>    UINT64                            EncryptionMask;
> +  UINT64                            TpmBaseAddress;
>    RETURN_STATUS                     PcdStatus;
>  
>    //
> @@ -206,6 +207,24 @@ AmdSevInitialize (
>      }
>    }
>  
> +  //
> +  // PEI TPM support will perform MMIO accesses, be sure this range is not
> +  // marked encrypted.
> +  //
> +  TpmBaseAddress = PcdGet64 (PcdTpmBaseAddress);
> +  if (TpmBaseAddress != 0) {
> +    RETURN_STATUS  DecryptStatus;
> +
> +    DecryptStatus = MemEncryptSevClearPageEncMask (
> +                      0,
> +                      TpmBaseAddress,
> +                      EFI_SIZE_TO_PAGES (0x5000),
> +                      FALSE
> +                      );
> +
> +    ASSERT_RETURN_ERROR (DecryptStatus);
> +  }
> +

Laszlo, I'm not sure if this is the best way to approach this. It is
simple and straight forward and the TCG/TPM support is one of the few
(only?) pieces of code that does actual MMIO during PEI that is bitten by
not having the address marked as shared/unencrypted.

I was also thinking of just marking everything above the highest system
memory address below 4GB and up to the PcdOvmfFdBaseAddress as
unencrypted. That also works.

Once DXE starts it takes care of all this and there isn't an issue in DXE.
This is just for PEI.

Thoughts? Other suggestions?

Thanks,
Tom

>    //
>    // Check and perform SEV-ES initialization if required.
>    //
>