From: Sathyanarayanan Kuppuswamy <sathyanarayanan.kuppuswamy@linux.intel.com>
To: Dexuan Cui <decui@microsoft.com>,
ak@linux.intel.com, arnd@arndb.de, bp@alien8.de,
brijesh.singh@amd.com, dan.j.williams@intel.com,
dave.hansen@intel.com, dave.hansen@linux.intel.com,
haiyangz@microsoft.com, hpa@zytor.com, jane.chu@oracle.com,
kirill.shutemov@linux.intel.com, kys@microsoft.com,
linux-arch@vger.kernel.org, linux-hyperv@vger.kernel.org,
luto@kernel.org, mingo@redhat.com, peterz@infradead.org,
rostedt@goodmis.org, seanjc@google.com, tglx@linutronix.de,
tony.luck@intel.com, wei.liu@kernel.org, x86@kernel.org,
mikelley@microsoft.com
Cc: linux-kernel@vger.kernel.org, Tianyu.Lan@microsoft.com,
rick.p.edgecombe@intel.com
Subject: Re: [PATCH v8 1/2] x86/tdx: Retry TDVMCALL_MAP_GPA() when needed
Date: Tue, 20 Jun 2023 11:30:51 -0700 [thread overview]
Message-ID: <49cb0f01-f1c2-8812-7f2f-9a70ff576085@linux.intel.com> (raw)
In-Reply-To: <20230620154830.25442-2-decui@microsoft.com>
Hi,
On 6/20/23 8:48 AM, Dexuan Cui wrote:
> GHCI spec for TDX 1.0 says that the MapGPA call may fail with the R10
> error code = TDG.VP.VMCALL_RETRY (1), and the guest must retry this
> operation for the pages in the region starting at the GPA specified
> in R11.
>
> When a fully enlightened TDX guest runs on Hyper-V, Hyper-V can return
> the retry error when set_memory_decrypted() is called to decrypt up to
> 1GB of swiotlb bounce buffers.
>
> Acked-by: Kirill A. Shutemov <kirill.shutemov@linux.intel.com>
> Reviewed-by: Michael Kelley <mikelley@microsoft.com>
> Signed-off-by: Dexuan Cui <decui@microsoft.com>
> ---
>
> arch/x86/coco/tdx/tdx.c | 63 +++++++++++++++++++++++++------
> arch/x86/include/asm/shared/tdx.h | 2 +
> 2 files changed, 53 insertions(+), 12 deletions(-)
>
> Changes in v2:
> Used __tdx_hypercall() directly in tdx_map_gpa().
> Added a max_retry_cnt of 1000.
> Renamed a few variables, e.g., r11 -> map_fail_paddr.
>
> Changes in v3:
> Changed max_retry_cnt from 1000 to 3.
>
> Changes in v4:
> __tdx_hypercall(&args, TDX_HCALL_HAS_OUTPUT) -> __tdx_hypercall_ret()
> Added Kirill's Acked-by.
>
> Changes in v5:
> Added Michael's Reviewed-by.
>
> Changes in v6: None.
>
> Changes in v7:
> Addressed Dave's comments:
> see https://lwn.net/ml/linux-kernel/SA1PR21MB1335736123C2BCBBFD7460C3BF46A@SA1PR21MB1335.namprd21.prod.outlook.com
>
> Changes in v8:
> Rebased to tip.git's master branch.
>
> diff --git a/arch/x86/coco/tdx/tdx.c b/arch/x86/coco/tdx/tdx.c
> index 1d6b863c42b0..0c198ab73aa7 100644
> --- a/arch/x86/coco/tdx/tdx.c
> +++ b/arch/x86/coco/tdx/tdx.c
> @@ -703,14 +703,16 @@ static bool tdx_cache_flush_required(void)
> }
>
> /*
> - * Inform the VMM of the guest's intent for this physical page: shared with
> - * the VMM or private to the guest. The VMM is expected to change its mapping
> - * of the page in response.
> + * Notify the VMM about page mapping conversion. More info about ABI
> + * can be found in TDX Guest-Host-Communication Interface (GHCI),
> + * section "TDG.VP.VMCALL<MapGPA>".
> */
> -static bool tdx_enc_status_changed(unsigned long vaddr, int numpages, bool enc)
> +static bool tdx_map_gpa(phys_addr_t start, phys_addr_t end, bool enc)
> {
> - phys_addr_t start = __pa(vaddr);
> - phys_addr_t end = __pa(vaddr + numpages * PAGE_SIZE);
> + const int max_retries_per_page = 3;
Add some details about why you chose 3? Maybe you can also use macro for it.
> + struct tdx_hypercall_args args;
> + u64 map_fail_paddr, ret;
> + int retry_count = 0;
>
> if (!enc) {
> /* Set the shared (decrypted) bits: */
> @@ -718,12 +720,49 @@ static bool tdx_enc_status_changed(unsigned long vaddr, int numpages, bool enc)
> end |= cc_mkdec(0);
> }
>
> - /*
> - * Notify the VMM about page mapping conversion. More info about ABI
> - * can be found in TDX Guest-Host-Communication Interface (GHCI),
> - * section "TDG.VP.VMCALL<MapGPA>"
> - */
> - if (_tdx_hypercall(TDVMCALL_MAP_GPA, start, end - start, 0, 0))
> + while (retry_count < max_retries_per_page) {
> + memset(&args, 0, sizeof(args));
> + args.r10 = TDX_HYPERCALL_STANDARD;
> + args.r11 = TDVMCALL_MAP_GPA;
> + args.r12 = start;
> + args.r13 = end - start;
> +
> + ret = __tdx_hypercall_ret(&args);
> + if (ret != TDVMCALL_STATUS_RETRY)
> + return !ret;
> + /*
> + * The guest must retry the operation for the pages in the
> + * region starting at the GPA specified in R11. R11 comes
> + * from the untrusted VMM. Sanity check it.
> + */
> + map_fail_paddr = args.r11;
Do you really need map_fail_paddr? Why not directly use args.r11?
> + if (map_fail_paddr < start || map_fail_paddr >= end)
> + return false;
> +
> + /* "Consume" a retry without forward progress */
> + if (map_fail_paddr == start) {
> + retry_count++;
> + continue;
> + }
> +
> + start = map_fail_paddr;
> + retry_count = 0;
> + }
> +
> + return false;
> +}
> +
> +/*
> + * Inform the VMM of the guest's intent for this physical page: shared with
> + * the VMM or private to the guest. The VMM is expected to change its mapping
> + * of the page in response.
> + */
> +static bool tdx_enc_status_changed(unsigned long vaddr, int numpages, bool enc)
> +{
> + phys_addr_t start = __pa(vaddr);
> + phys_addr_t end = __pa(vaddr + numpages * PAGE_SIZE);
> +
> + if (!tdx_map_gpa(start, end, enc))
> return false;
>
> /* shared->private conversion requires memory to be accepted before use */
> diff --git a/arch/x86/include/asm/shared/tdx.h b/arch/x86/include/asm/shared/tdx.h
> index 90ea813c4b99..9db89a99ae5b 100644
> --- a/arch/x86/include/asm/shared/tdx.h
> +++ b/arch/x86/include/asm/shared/tdx.h
> @@ -24,6 +24,8 @@
> #define TDVMCALL_MAP_GPA 0x10001
> #define TDVMCALL_REPORT_FATAL_ERROR 0x10003
>
> +#define TDVMCALL_STATUS_RETRY 1
> +
> #ifndef __ASSEMBLY__
>
> /*
--
Sathyanarayanan Kuppuswamy
Linux Kernel Developer
next prev parent reply other threads:[~2023-06-20 18:31 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-06-20 15:48 [PATCH v8 0/2] Support TDX guests on Hyper-V (the x86/tdx part) Dexuan Cui
2023-06-20 15:48 ` [PATCH v8 1/2] x86/tdx: Retry TDVMCALL_MAP_GPA() when needed Dexuan Cui
2023-06-20 18:30 ` Sathyanarayanan Kuppuswamy [this message]
2023-06-20 19:23 ` Dexuan Cui
2023-06-20 19:44 ` Sathyanarayanan Kuppuswamy
2023-06-20 23:34 ` Dave Hansen
2023-06-21 0:28 ` Dexuan Cui
2023-06-20 15:48 ` [PATCH v8 2/2] x86/tdx: Support vmalloc() for tdx_enc_status_changed() Dexuan Cui
2023-06-20 18:34 ` Sathyanarayanan Kuppuswamy
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-all 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=49cb0f01-f1c2-8812-7f2f-9a70ff576085@linux.intel.com \
--to=sathyanarayanan.kuppuswamy@linux.intel.com \
--cc=Tianyu.Lan@microsoft.com \
--cc=ak@linux.intel.com \
--cc=arnd@arndb.de \
--cc=bp@alien8.de \
--cc=brijesh.singh@amd.com \
--cc=dan.j.williams@intel.com \
--cc=dave.hansen@intel.com \
--cc=dave.hansen@linux.intel.com \
--cc=decui@microsoft.com \
--cc=haiyangz@microsoft.com \
--cc=hpa@zytor.com \
--cc=jane.chu@oracle.com \
--cc=kirill.shutemov@linux.intel.com \
--cc=kys@microsoft.com \
--cc=linux-arch@vger.kernel.org \
--cc=linux-hyperv@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=luto@kernel.org \
--cc=mikelley@microsoft.com \
--cc=mingo@redhat.com \
--cc=peterz@infradead.org \
--cc=rick.p.edgecombe@intel.com \
--cc=rostedt@goodmis.org \
--cc=seanjc@google.com \
--cc=tglx@linutronix.de \
--cc=tony.luck@intel.com \
--cc=wei.liu@kernel.org \
--cc=x86@kernel.org \
/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;
as well as URLs for NNTP newsgroup(s).