Re: [edk2-devel] [PATCH V3 2/2] OvmfPkg/BaseMemEncryptTdxLib: Handle retry result of MapGPA

["Yao, Jiewen" <jiewen.yao@intel.com>]

Hey Ceping
Please don't change two packages in one patch, because it is hard to let the corresponding maintainer to review and give R-B, if he/she only reviews part of them.

The patch should be split to MdePkg update and OvmfPkg update.

Thank you
Yao, Jiewen


> -----Original Message-----
> From: Sun, CepingX <cepingx.sun@intel.com>
> Sent: Wednesday, November 8, 2023 4:32 PM
> To: devel@edk2.groups.io
> Cc: Sun, CepingX <cepingx.sun@intel.com>; Aktas, Erdem
> <erdemaktas@google.com>; James Bottomley <jejb@linux.ibm.com>; Yao,
> Jiewen <jiewen.yao@intel.com>; Xu, Min M <min.m.xu@intel.com>; Tom
> Lendacky <thomas.lendacky@amd.com>; Michael Roth
> <michael.roth@amd.com>; Gerd Hoffmann <kraxel@redhat.com>
> Subject: [PATCH V3 2/2] OvmfPkg/BaseMemEncryptTdxLib: Handle retry result of
> MapGPA
> 
> From: Ceping Sun <cepingx.sun@intel.com>
> 
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=4572
> 
> According to section 3.2 of the [GHCI] document, if the return status
> of MapGPA is "TDG.VP.VMCALL_RETRY", TD must retry this operation for the
> pages in the region starting at the GPA specified in R11.
> 
> In this patch, when a retry state is detected, TDVF needs to retry the
> mapping with the specified address from the output results of TdVmCall.
> 
> Reference:
> [GHCI]: TDX Guest-Host-Communication Interface v1.0
> https://cdrdv2.intel.com/v1/dl/getContent/726790
> 
> Cc: Erdem Aktas <erdemaktas@google.com>
> Cc: James Bottomley <jejb@linux.ibm.com>
> Cc: Jiewen Yao <jiewen.yao@intel.com>
> Cc: Min Xu <min.m.xu@intel.com>
> Cc: Tom Lendacky <thomas.lendacky@amd.com>
> Cc: Michael Roth <michael.roth@amd.com>
> Cc: Gerd Hoffmann <kraxel@redhat.com>
> Signed-off-by: Ceping Sun <cepingx.sun@intel.com>
> ---
>  MdePkg/Include/IndustryStandard/Tdx.h         |  2 +
>  .../BaseMemEncryptTdxLib/MemoryEncryption.c   | 41 ++++++++++++++++++-
>  2 files changed, 42 insertions(+), 1 deletion(-)
> 
> diff --git a/MdePkg/Include/IndustryStandard/Tdx.h
> b/MdePkg/Include/IndustryStandard/Tdx.h
> index 81df1361842b..2662761883e5 100644
> --- a/MdePkg/Include/IndustryStandard/Tdx.h
> +++ b/MdePkg/Include/IndustryStandard/Tdx.h
> @@ -103,6 +103,8 @@
>  #define TDVMCALL_REPORT_FATAL_ERR    0x10003
>  #define TDVMCALL_SETUP_EVENT_NOTIFY  0x10004
> 
> +#define TDVMCALL_STATUS_RETRY  0x1
> +
>  #pragma pack(1)
>  typedef struct {
>    UINT64    Data[6];
> diff --git a/OvmfPkg/Library/BaseMemEncryptTdxLib/MemoryEncryption.c
> b/OvmfPkg/Library/BaseMemEncryptTdxLib/MemoryEncryption.c
> index a01dc98852b8..a71b1efbca7a 100644
> --- a/OvmfPkg/Library/BaseMemEncryptTdxLib/MemoryEncryption.c
> +++ b/OvmfPkg/Library/BaseMemEncryptTdxLib/MemoryEncryption.c
> @@ -38,6 +38,8 @@ typedef enum {
> 
>  STATIC PAGE_TABLE_POOL  *mPageTablePool = NULL;
> 
> +#define MAX_RETRIES_PER_PAGE  3
> +
>  /**
>    Returns boolean to indicate whether to indicate which, if any, memory
> encryption is enabled
> 
> @@ -527,6 +529,13 @@ SetOrClearSharedBit (
>    EFI_STATUS                    Status;
>    EDKII_MEMORY_ACCEPT_PROTOCOL  *MemoryAcceptProtocol;
> 
> +  UINT64  MapGpaRetryAddr;
> +  UINT32  RetryCount;
> +  UINT64  EndAddress;
> +
> +  MapGpaRetryAddr = 0;
> +  RetryCount      = 0;
> +
>    AddressEncMask = GetMemEncryptionAddressMask ();
> 
>    //
> @@ -540,7 +549,37 @@ SetOrClearSharedBit (
>      PhysicalAddress   &= ~AddressEncMask;
>    }
> 
> -  TdStatus = TdVmCall (TDVMCALL_MAPGPA, PhysicalAddress, Length, 0, 0,
> NULL);
> +  EndAddress = PhysicalAddress + Length;
> +  while (RetryCount < MAX_RETRIES_PER_PAGE) {
> +    TdStatus = TdVmCall (TDVMCALL_MAPGPA, PhysicalAddress, Length, 0, 0,
> &MapGpaRetryAddr);
> +    if (TdStatus != TDVMCALL_STATUS_RETRY) {
> +      break;
> +    }
> +
> +    DEBUG ((DEBUG_VERBOSE, "%a: TdVmcall(MAPGPA) Retry PhysicalAddress
> is %llx, MapGpaRetryAddr is %llx\n", __func__, PhysicalAddress,
> MapGpaRetryAddr));
> +
> +    if ((MapGpaRetryAddr < PhysicalAddress) || (MapGpaRetryAddr >=
> EndAddress)) {
> +      DEBUG ((
> +        DEBUG_ERROR,
> +        "%a: TdVmcall(MAPGPA) failed with MapGpaRetryAddr(%llx) less than
> PhysicalAddress(%llx) or more than or equal to EndAddress(%llx) \n",
> +        __func__,
> +        MapGpaRetryAddr,
> +        PhysicalAddress,
> +        EndAddress
> +        ));
> +      break;
> +    }
> +
> +    if (MapGpaRetryAddr == PhysicalAddress) {
> +      RetryCount++;
> +      continue;
> +    }
> +
> +    PhysicalAddress = MapGpaRetryAddr;
> +    Length          = EndAddress - PhysicalAddress;
> +    RetryCount      = 0;
> +  }
> +
>    if (TdStatus != 0) {
>      DEBUG ((DEBUG_ERROR, "%a: TdVmcall(MAPGPA) failed with %llx\n",
> __func__, TdStatus));
>      ASSERT (FALSE);
> --
> 2.34.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#110907): https://edk2.groups.io/g/devel/message/110907
Mute This Topic: https://groups.io/mt/102460273/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-