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 result of MapGPA
is "TDG.VP.VMCALL_RETRY", TDVF must retry mapping for pages in that region,
starting with the GPA specified in R11.
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.inf | 1 +
.../BaseMemEncryptTdxLib/MemoryEncryption.c | 36 ++++++++++++++++++-
2 files changed, 36 insertions(+), 1 deletion(-)
diff --git a/OvmfPkg/Library/BaseMemEncryptTdxLib/BaseMemEncryptTdxLib.inf b/OvmfPkg/Library/BaseMemEncryptTdxLib/BaseMemEncryptTdxLib.inf
index 11768825f8ca..742b65a289ce 100644
--- a/OvmfPkg/Library/BaseMemEncryptTdxLib/BaseMemEncryptTdxLib.inf
+++ b/OvmfPkg/Library/BaseMemEncryptTdxLib/BaseMemEncryptTdxLib.inf
@@ -30,6 +30,7 @@
[Sources]
VirtualMemory.h
MemoryEncryption.c
+ X64/TdVmCallMapGPA.nasm
[LibraryClasses]
BaseLib
diff --git a/OvmfPkg/Library/BaseMemEncryptTdxLib/MemoryEncryption.c b/OvmfPkg/Library/BaseMemEncryptTdxLib/MemoryEncryption.c
index b47f56b391a5..1f29f9194c30 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
+
/**
This function is used to help request the host VMM to map a GPA range as
private or shared-memory mappings.
@@ -546,6 +550,13 @@ SetOrClearSharedBit (
EFI_STATUS Status;
EDKII_MEMORY_ACCEPT_PROTOCOL *MemoryAcceptProtocol;
+ UINT64 MapGpaRetryaddr;
+ UINT32 RetryCount;
+ UINT64 EndAddress;
+
+ MapGpaRetryaddr = 0;
+ RetryCount = 0;
+
AddressEncMask = GetMemEncryptionAddressMask ();
//
@@ -559,7 +570,30 @@ SetOrClearSharedBit (
PhysicalAddress &= ~AddressEncMask;
}
- TdStatus = TdVmCall (TDVMCALL_MAPGPA, PhysicalAddress, Length, 0, 0, NULL);
+ while (RetryCount < MAX_RETRIES_PER_PAGE) {
+ TdStatus = TdVmCallMapGPA (PhysicalAddress, Length, &MapGpaRetryaddr);
+ if (TdStatus != TDVMCALL_STATUS_RETRY) {
+ break;
+ }
+
+ DEBUG ((DEBUG_VERBOSE, "%a: TdVmcall(MAPGPA) Retry PhysicalAddress is %llx, MapGpaRetryaddr is %llx\n", __func__, PhysicalAddress, MapGpaRetryaddr));
+
+ EndAddress = PhysicalAddress + Length;
+ if ((MapGpaRetryaddr < PhysicalAddress) || (MapGpaRetryaddr > EndAddress)) {
+ DEBUG ((DEBUG_ERROR, "%a: TdVmcall(MAPGPA) failed Retry PhysicalAddress is %llx, MapGpaRetryaddr is %llx\n", __func__, PhysicalAddress, MapGpaRetryaddr));
+ 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 (#110159): https://edk2.groups.io/g/devel/message/110159
Mute This Topic: https://groups.io/mt/102212640/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-Hi,
This should be the [PATCH V1 2/2] I assume?
On Thu, Oct 26, 2023 at 5:58 PM sunceping <cepingx.sun@intel.com> wrote:
> 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 result of MapGPA
> is "TDG.VP.VMCALL_RETRY", TDVF must retry mapping for pages in that region,
> starting with the GPA specified in R11.
>
> 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.inf | 1 +
> .../BaseMemEncryptTdxLib/MemoryEncryption.c | 36 ++++++++++++++++++-
> 2 files changed, 36 insertions(+), 1 deletion(-)
>
> diff --git a/OvmfPkg/Library/BaseMemEncryptTdxLib/BaseMemEncryptTdxLib.inf
> b/OvmfPkg/Library/BaseMemEncryptTdxLib/BaseMemEncryptTdxLib.inf
> index 11768825f8ca..742b65a289ce 100644
> --- a/OvmfPkg/Library/BaseMemEncryptTdxLib/BaseMemEncryptTdxLib.inf
> +++ b/OvmfPkg/Library/BaseMemEncryptTdxLib/BaseMemEncryptTdxLib.inf
> @@ -30,6 +30,7 @@
> [Sources]
> VirtualMemory.h
> MemoryEncryption.c
> + X64/TdVmCallMapGPA.nasm
>
I do not think we need another TdVmCallMapGPA definition, do we?
>
> [LibraryClasses]
> BaseLib
> diff --git a/OvmfPkg/Library/BaseMemEncryptTdxLib/MemoryEncryption.c
> b/OvmfPkg/Library/BaseMemEncryptTdxLib/MemoryEncryption.c
> index b47f56b391a5..1f29f9194c30 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
> +
> /**
> This function is used to help request the host VMM to map a GPA range as
> private or shared-memory mappings.
> @@ -546,6 +550,13 @@ SetOrClearSharedBit (
> EFI_STATUS Status;
> EDKII_MEMORY_ACCEPT_PROTOCOL *MemoryAcceptProtocol;
>
> + UINT64 MapGpaRetryaddr;
>
Should be replaced with MapGpaRetryAddr for consistency in variable
name casing style ?
> + UINT32 RetryCount;
> + UINT64 EndAddress;
> +
> + MapGpaRetryaddr = 0;
> + RetryCount = 0;
> +
> AddressEncMask = GetMemEncryptionAddressMask ();
>
> //
> @@ -559,7 +570,30 @@ SetOrClearSharedBit (
> PhysicalAddress &= ~AddressEncMask;
> }
>
> - TdStatus = TdVmCall (TDVMCALL_MAPGPA, PhysicalAddress, Length, 0, 0,
> NULL);
> + while (RetryCount < MAX_RETRIES_PER_PAGE) {
> + TdStatus = TdVmCallMapGPA (PhysicalAddress, Length, &MapGpaRetryaddr);
>
Why not this?
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));
> +
> + EndAddress = PhysicalAddress + Length;
> + if ((MapGpaRetryaddr < PhysicalAddress) || (MapGpaRetryaddr >
> EndAddress)) {
>
should be?
if ((MapGpaRetryaddr < PhysicalAddress) || (MapGpaRetryaddr >=
EndAddress))
> + DEBUG ((DEBUG_ERROR, "%a: TdVmcall(MAPGPA) failed Retry
> PhysicalAddress is %llx, MapGpaRetryaddr is %llx\n", __func__,
> PhysicalAddress, MapGpaRetryaddr));
> + 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 (#110223): https://edk2.groups.io/g/devel/message/110223
Mute This Topic: https://groups.io/mt/102212640/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
On Saturday, October 28, 2023 12:45 AM, Erdem Aktas wrote:
This should be the [PATCH V1 2/2] I assume?
Yes, the name is same with [PATCH v1 0/2] , may be confusion, I would update in next version to avoid the same title name.
On Thu, Oct 26, 2023 at 5:58 PM sunceping <cepingx.sun@intel.com<mailto:cepingx.sun@intel.com>> wrote:
[Sources]
VirtualMemory.h
MemoryEncryption.c
+ X64/TdVmCallMapGPA.nasm
I do not think we need another TdVmCallMapGPA definition, do we?
Currently, the TdVmCall always clears the R11 if the return code is not successful, which means we need to change TdVmCall if we don't add TdVmCallMapGPA.
Refer the GHCI Spec , if the returns code is not successful, the R11 value is not valid for the sub-functions except MapGPA, which means TdVmCall should clear the value on
unsuccessful returns and only save the value if MapGPA returns unsuccessfully. If an update is required, the logic in TdVmCall could be complex.
[LibraryClasses]
BaseLib
diff --git a/OvmfPkg/Library/BaseMemEncryptTdxLib/MemoryEncryption.c b/OvmfPkg/Library/BaseMemEncryptTdxLib/MemoryEncryption.c
index b47f56b391a5..1f29f9194c30 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
+
/**
This function is used to help request the host VMM to map a GPA range as
private or shared-memory mappings.
@@ -546,6 +550,13 @@ SetOrClearSharedBit (
EFI_STATUS Status;
EDKII_MEMORY_ACCEPT_PROTOCOL *MemoryAcceptProtocol;
+ UINT64 MapGpaRetryaddr;
Should be replaced with MapGpaRetryAddr for consistency in variable name casing style ?
Yes, it would be updated in next version.
+ UINT32 RetryCount;
+ UINT64 EndAddress;
+
+ MapGpaRetryaddr = 0;
+ RetryCount = 0;
+
AddressEncMask = GetMemEncryptionAddressMask ();
//
@@ -559,7 +570,30 @@ SetOrClearSharedBit (
PhysicalAddress &= ~AddressEncMask;
}
- TdStatus = TdVmCall (TDVMCALL_MAPGPA, PhysicalAddress, Length, 0, 0, NULL);
+ while (RetryCount < MAX_RETRIES_PER_PAGE) {
+ TdStatus = TdVmCallMapGPA (PhysicalAddress, Length, &MapGpaRetryaddr);
Why not this?
TdStatus = TdVmCall (TDVMCALL_MAPGPA, PhysicalAddress, Length, 0, 0, &MapGpaRetryaddr);
The TdVmCall always clears the R11 value when unsuccessful returns as above comments, therefor add the TdVmCallMapGPA to handle it.
+ if (TdStatus != TDVMCALL_STATUS_RETRY) {
+ break;
+ }
+
+ DEBUG ((DEBUG_VERBOSE, "%a: TdVmcall(MAPGPA) Retry PhysicalAddress is %llx, MapGpaRetryaddr is %llx\n", __func__, PhysicalAddress, MapGpaRetryaddr));
+
+ EndAddress = PhysicalAddress + Length;
+ if ((MapGpaRetryaddr < PhysicalAddress) || (MapGpaRetryaddr > EndAddress)) {
should be?
if ((MapGpaRetryaddr < PhysicalAddress) || (MapGpaRetryaddr >= EndAddress))
Yes, that’s right, it would be updated in next version.
Thanks
Ceping
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#110291): https://edk2.groups.io/g/devel/message/110291
Mute This Topic: https://groups.io/mt/102212640/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
On Sun, Oct 29, 2023 at 11:42 PM Sun, CepingX <cepingx.sun@intel.com> wrote:
>
> On Saturday, October 28, 2023 12:45 AM, Erdem Aktas wrote:
> This should be the [PATCH V1 2/2] I assume?
> Yes, the name is same with [PATCH v1 0/2] , may be confusion, I would update in next version to avoid the same title name.
>
>
> On Thu, Oct 26, 2023 at 5:58 PM sunceping <cepingx.sun@intel.com> wrote:
> [Sources]
> VirtualMemory.h
> MemoryEncryption.c
> + X64/TdVmCallMapGPA.nasm
> I do not think we need another TdVmCallMapGPA definition, do we?
> Currently, the TdVmCall always clears the R11 if the return code is not successful, which means we need to change TdVmCall if we don't add TdVmCallMapGPA.
> Refer the GHCI Spec , if the returns code is not successful, the R11 value is not valid for the sub-functions except MapGPA, which means TdVmCall should clear the value on
> unsuccessful returns and only save the value if MapGPA returns unsuccessfully. If an update is required, the logic in TdVmCall could be complex.
It seems like TdVmCallMapGPA function is actually duplicating most of
the code that the TdVmCall function is already doing.
According to the spec, R11 has meaningful data when mapgpa has RETRY,
GPA_IN_USE or ALIGN_ERROR.
I do not think the TdVmCall change logic would be complex (or not more
than what TdVmCallMapGPA is already doing).
I would like to see what others are saying on this.
>
> [LibraryClasses]
> BaseLib
> diff --git a/OvmfPkg/Library/BaseMemEncryptTdxLib/MemoryEncryption.c b/OvmfPkg/Library/BaseMemEncryptTdxLib/MemoryEncryption.c
> index b47f56b391a5..1f29f9194c30 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
> +
> /**
> This function is used to help request the host VMM to map a GPA range as
> private or shared-memory mappings.
> @@ -546,6 +550,13 @@ SetOrClearSharedBit (
> EFI_STATUS Status;
> EDKII_MEMORY_ACCEPT_PROTOCOL *MemoryAcceptProtocol;
>
> + UINT64 MapGpaRetryaddr;
> Should be replaced with MapGpaRetryAddr for consistency in variable name casing style ?
> Yes, it would be updated in next version.
>
> + UINT32 RetryCount;
> + UINT64 EndAddress;
> +
> + MapGpaRetryaddr = 0;
> + RetryCount = 0;
> +
> AddressEncMask = GetMemEncryptionAddressMask ();
>
> //
> @@ -559,7 +570,30 @@ SetOrClearSharedBit (
> PhysicalAddress &= ~AddressEncMask;
> }
>
> - TdStatus = TdVmCall (TDVMCALL_MAPGPA, PhysicalAddress, Length, 0, 0, NULL);
> + while (RetryCount < MAX_RETRIES_PER_PAGE) {
> + TdStatus = TdVmCallMapGPA (PhysicalAddress, Length, &MapGpaRetryaddr);
> Why not this?
> TdStatus = TdVmCall (TDVMCALL_MAPGPA, PhysicalAddress, Length, 0, 0, &MapGpaRetryaddr);
> The TdVmCall always clears the R11 value when unsuccessful returns as above comments, therefor add the TdVmCallMapGPA to handle it.
right, the tdvmcall does not handle the R11 correctly for mapGPA. I
think it should be an easy fix in that function instead of creating a
whole copy of that function.
Is there a reason why we think it is complicated?
>
> + if (TdStatus != TDVMCALL_STATUS_RETRY) {
> + break;
> + }
> +
> + DEBUG ((DEBUG_VERBOSE, "%a: TdVmcall(MAPGPA) Retry PhysicalAddress is %llx, MapGpaRetryaddr is %llx\n", __func__, PhysicalAddress, MapGpaRetryaddr));
> +
> + EndAddress = PhysicalAddress + Length;
> + if ((MapGpaRetryaddr < PhysicalAddress) || (MapGpaRetryaddr > EndAddress)) {
> should be?
> if ((MapGpaRetryaddr < PhysicalAddress) || (MapGpaRetryaddr >= EndAddress))
> Yes, that’s right, it would be updated in next version.
>
> Thanks
> Ceping
>
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#110350): https://edk2.groups.io/g/devel/message/110350
Mute This Topic: https://groups.io/mt/102212640/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
On Tuesday, October 31, 2023 8:45 AM, Erdem Aktas wrote:
> On Sun, Oct 29, 2023 at 11:42 PM Sun, CepingX <cepingx.sun@intel.com<mailto:cepingx.sun@intel.com>>
> wrote:
> >
> > On Saturday, October 28, 2023 12:45 AM, Erdem Aktas wrote:
> > This should be the [PATCH V1 2/2] I assume?
> > Yes, the name is same with [PATCH v1 0/2] , may be confusion, I would
> update in next version to avoid the same title name.
> >
> >
> > On Thu, Oct 26, 2023 at 5:58 PM sunceping <cepingx.sun@intel.com<mailto:cepingx.sun@intel.com>> wrote:
> > [Sources]
> > VirtualMemory.h
> > MemoryEncryption.c
> > + X64/TdVmCallMapGPA.nasm
> > I do not think we need another TdVmCallMapGPA definition, do we?
> > Currently, the TdVmCall always clears the R11 if the return code is not
> successful, which means we need to change TdVmCall if we don't add
> TdVmCallMapGPA.
> > Refer the GHCI Spec , if the returns code is not successful, the R11
> > value is not valid for the sub-functions except MapGPA, which means
> TdVmCall should clear the value on unsuccessful returns and only save the
> value if MapGPA returns unsuccessfully. If an update is required, the logic in
> TdVmCall could be complex.
>
> It seems like TdVmCallMapGPA function is actually duplicating most of the
> code that the TdVmCall function is already doing.
> According to the spec, R11 has meaningful data when mapgpa has RETRY,
> GPA_IN_USE or ALIGN_ERROR.
> I do not think the TdVmCall change logic would be complex (or not more
> than what TdVmCallMapGPA is already doing).
> I would like to see what others are saying on this.
>
Yes, we can wait for other's comments. @Gerd Hoffmann<mailto:kraxel@redhat.com> , Could you help add a comment?
> > - TdStatus = TdVmCall (TDVMCALL_MAPGPA, PhysicalAddress, Length, 0,
> > 0, NULL);
> > + while (RetryCount < MAX_RETRIES_PER_PAGE) {
> > + TdStatus = TdVmCallMapGPA (PhysicalAddress, Length,
> > + &MapGpaRetryaddr);
> > Why not this?
> > TdStatus = TdVmCall (TDVMCALL_MAPGPA, PhysicalAddress, Length, 0, 0,
> > &MapGpaRetryaddr); The TdVmCall always clears the R11 value when
> unsuccessful returns as above comments, therefor add the
> TdVmCallMapGPA to handle it.
> right, the tdvmcall does not handle the R11 correctly for mapGPA. I think it
> should be an easy fix in that function instead of creating a whole copy of that
> function.
> Is there a reason why we think it is complicated?
If an update is required, the TdVmCall has to add a flag or value to indicate the
MapGPA and always compares the return code before clearing. This indicates
that additional code must be developed specifically to handle the results and solely
for MapGPA at this time, it would make the code seems complex and not easy extend
in the future.
Based on the above considerations, we extract the code for MapGPA as an individual function,
that' s easy to understand and extend for other results in the future.
Thanks
Ceping
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#110383): https://edk2.groups.io/g/devel/message/110383
Mute This Topic: https://groups.io/mt/102212640/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
Hi,
> + while (RetryCount < MAX_RETRIES_PER_PAGE) {
> + TdStatus = TdVmCallMapGPA (PhysicalAddress, Length, &MapGpaRetryaddr);
> + if (TdStatus != TDVMCALL_STATUS_RETRY) {
> + break;
> + }
> +
> + DEBUG ((DEBUG_VERBOSE, "%a: TdVmcall(MAPGPA) Retry PhysicalAddress is %llx, MapGpaRetryaddr is %llx\n", __func__, PhysicalAddress, MapGpaRetryaddr));
> +
> + EndAddress = PhysicalAddress + Length;
The end address doesn't change, you can move that out of the loop.
> + if ((MapGpaRetryaddr < PhysicalAddress) || (MapGpaRetryaddr > EndAddress)) {
> + DEBUG ((DEBUG_ERROR, "%a: TdVmcall(MAPGPA) failed Retry PhysicalAddress is %llx, MapGpaRetryaddr is %llx\n", __func__, PhysicalAddress, MapGpaRetryaddr));
> + break;
> + }
The error message is misleading. The actual problem is that the
'PhysicalAddress <= MapGpaRetryaddr <= EndAddress' sanity check failed.
take care,
Gerd
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#110202): https://edk2.groups.io/g/devel/message/110202
Mute This Topic: https://groups.io/mt/102212640/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
On Friday, October 27, 2023 7:05 PM, Gerd Hoffmann wrote:
> > + while (RetryCount < MAX_RETRIES_PER_PAGE) {
> > + TdStatus = TdVmCallMapGPA (PhysicalAddress, Length,
> &MapGpaRetryaddr);
> > + if (TdStatus != TDVMCALL_STATUS_RETRY) {
> > + break;
> > + }
> > +
> > + DEBUG ((DEBUG_VERBOSE, "%a: TdVmcall(MAPGPA) Retry
> > + PhysicalAddress is %llx, MapGpaRetryaddr is %llx\n", __func__,
> > + PhysicalAddress, MapGpaRetryaddr));
> > +
> > + EndAddress = PhysicalAddress + Length;
>
> The end address doesn't change, you can move that out of the loop.
It would be updated in next version.
>
> > + if ((MapGpaRetryaddr < PhysicalAddress) || (MapGpaRetryaddr >
> EndAddress)) {
> > + DEBUG ((DEBUG_ERROR, "%a: TdVmcall(MAPGPA) failed Retry
> PhysicalAddress is %llx, MapGpaRetryaddr is %llx\n", __func__,
> PhysicalAddress, MapGpaRetryaddr));
> > + break;
> > + }
>
> The error message is misleading. The actual problem is that the
> 'PhysicalAddress <= MapGpaRetryaddr <= EndAddress' sanity check failed.
The error message would be updated in next version.
>
Thanks
Ceping
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#110283): https://edk2.groups.io/g/devel/message/110283
Mute This Topic: https://groups.io/mt/102212640/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
© 2016 - 2026 Red Hat, Inc.