From: "Erdem Aktas via groups.io" <erdemaktas=google.com@groups.io>
To: sunceping <cepingx.sun@intel.com>
Cc: devel@edk2.groups.io, James Bottomley <jejb@linux.ibm.com>,
Jiewen Yao <jiewen.yao@intel.com>, Min Xu <min.m.xu@intel.com>,
Tom Lendacky <thomas.lendacky@amd.com>,
Michael Roth <michael.roth@amd.com>,
Gerd Hoffmann <kraxel@redhat.com>
Subject: Re: [edk2-devel] [PATCH V1 2/2] OvmfPkg/BaseMemEncryptTdxLib: Handle retry result of MapGPA
Date: Fri, 27 Oct 2023 09:44:36 -0700 [thread overview]
Message-ID: <CAAYXXYypULbiPyTO=qwa-4nhAYrx+a6Z+R_48fm2XLpunN68hw@mail.gmail.com> (raw)
In-Reply-To: <20231027005738.371-3-cepingx.sun@intel.com>
[-- Attachment #1: Type: text/plain, Size: 4633 bytes --]
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/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-
[-- Attachment #2: Type: text/html, Size: 7099 bytes --]
next prev parent reply other threads:[~2023-10-27 16:44 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-10-27 0:57 [edk2-devel] [PATCH V1 0/2] OvmfPkg/BaseMemEncryptTdxLib: Handle retry result of MapGPA sunceping
2023-10-27 0:57 ` [edk2-devel] [PATCH V1 1/2] OvmfPkg/BaseMemEncryptTdxLib: Add TdVmCallMapGPA sunceping
2023-10-27 0:57 ` [edk2-devel] [PATCH V1 2/2] OvmfPkg/BaseMemEncryptTdxLib: Handle retry result of MapGPA sunceping
2023-10-27 11:04 ` Gerd Hoffmann
2023-10-30 2:26 ` sunceping
2023-10-27 16:44 ` Erdem Aktas via groups.io [this message]
2023-10-30 6:41 ` sunceping
2023-10-31 0:45 ` Erdem Aktas via groups.io
2023-10-31 7:46 ` sunceping
2023-11-02 9:25 ` sunceping
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-list from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to='CAAYXXYypULbiPyTO=qwa-4nhAYrx+a6Z+R_48fm2XLpunN68hw@mail.gmail.com' \
--to=devel@edk2.groups.io \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox