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>
---
 .../BaseMemEncryptTdxLib/MemoryEncryption.c   | 43 ++++++++++++++++++-
 1 file changed, 42 insertions(+), 1 deletion(-)

diff --git a/OvmfPkg/Library/BaseMemEncryptTdxLib/MemoryEncryption.c b/OvmfPkg/Library/BaseMemEncryptTdxLib/MemoryEncryption.c
index a01dc98852b8..b9de699a6489 100644
--- a/OvmfPkg/Library/BaseMemEncryptTdxLib/MemoryEncryption.c
+++ b/OvmfPkg/Library/BaseMemEncryptTdxLib/MemoryEncryption.c
@@ -38,6 +38,10 @@ typedef enum {
 
 STATIC PAGE_TABLE_POOL  *mPageTablePool = NULL;
 
+#define TDVMCALL_STATUS_RETRY  0x1
+
+#define MAX_RETRIES_PER_PAGE  3
+
 /**
   Returns boolean to indicate whether to indicate which, if any, memory encryption is enabled
 
@@ -527,6 +531,13 @@ SetOrClearSharedBit (
   EFI_STATUS                    Status;
   EDKII_MEMORY_ACCEPT_PROTOCOL  *MemoryAcceptProtocol;
 
+  UINT64  MapGpaRetryAddr;
+  UINT32  RetryCount;
+  UINT64  EndAddress;
+
+  MapGpaRetryAddr = 0;
+  RetryCount      = 0;
+
   AddressEncMask = GetMemEncryptionAddressMask ();
 
   //
@@ -540,7 +551,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 (#110507): https://edk2.groups.io/g/devel/message/110507
Mute This Topic: https://groups.io/mt/102337977/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-

I think the macro definition (#define TDVMCALL_STATUS_RETRY  0x1) should be in https://github.com/tianocore/edk2/blob/master/MdePkg/Include/IndustryStandard/Tdx.h, together with other TDX definition.

Thank you
Yao, Jiewen

> -----Original Message-----
> From: Sun, CepingX <cepingx.sun@intel.com>
> Sent: Thursday, November 2, 2023 5:10 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 V2 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>
> ---
>  .../BaseMemEncryptTdxLib/MemoryEncryption.c   | 43 ++++++++++++++++++-
>  1 file changed, 42 insertions(+), 1 deletion(-)
> 
> diff --git a/OvmfPkg/Library/BaseMemEncryptTdxLib/MemoryEncryption.c
> b/OvmfPkg/Library/BaseMemEncryptTdxLib/MemoryEncryption.c
> index a01dc98852b8..b9de699a6489 100644
> --- a/OvmfPkg/Library/BaseMemEncryptTdxLib/MemoryEncryption.c
> +++ b/OvmfPkg/Library/BaseMemEncryptTdxLib/MemoryEncryption.c
> @@ -38,6 +38,10 @@ typedef enum {
> 
>  STATIC PAGE_TABLE_POOL  *mPageTablePool = NULL;
> 
> +#define TDVMCALL_STATUS_RETRY  0x1
> +
> +#define MAX_RETRIES_PER_PAGE  3
> +
>  /**
>    Returns boolean to indicate whether to indicate which, if any, memory
> encryption is enabled
> 
> @@ -527,6 +531,13 @@ SetOrClearSharedBit (
>    EFI_STATUS                    Status;
>    EDKII_MEMORY_ACCEPT_PROTOCOL  *MemoryAcceptProtocol;
> 
> +  UINT64  MapGpaRetryAddr;
> +  UINT32  RetryCount;
> +  UINT64  EndAddress;
> +
> +  MapGpaRetryAddr = 0;
> +  RetryCount      = 0;
> +
>    AddressEncMask = GetMemEncryptionAddressMask ();
> 
>    //
> @@ -540,7 +551,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 (#110895): https://edk2.groups.io/g/devel/message/110895
Mute This Topic: https://groups.io/mt/102337977/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-