[RESEND,v9,1/2] x86/tdx: Retry TDVMCALL_MAP_GPA() when needed
Commit Message
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>
Reviewed-by: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com>
Signed-off-by: Dexuan Cui <decui@microsoft.com>
---
arch/x86/coco/tdx/tdx.c | 64 +++++++++++++++++++++++++------
arch/x86/include/asm/shared/tdx.h | 2 +
2 files changed, 54 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.
Changes in v9:
Added a comment before 'max_retries_per_page'.
Moved 'args', 'map_fail_paddr' and 'ret' into the loop.
Added Kuppuswamy Sathyanarayanan's Reviewed-by.
Comments
On 8/10/23 19:12, 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.
This changelog is not great. It gives zero background and wastes bytes
on telling me which register the error code is in (I don't care in a
changelog) and then using marketing fluff words like "fully enlightened".
Let's stick to the facts, give some background, and also avoid
regurgitating the GHCI, eh?
How's this?
x86/tdx: Retry partially-completed page conversion hypercalls
TDX guest memory is private by default and the VMM may not access it.
However, in cases where the guest needs to share data with the VMM,
the guest and the VMM can coordinate to make memory shared between
them.
The guest side of this protocol includes the "MapGPA" hypercall. This
call takes a guest physical address range. The hypercall spec (aka.
the GHCI) says that the MapGPA call is allowed to return partial
progress in mapping this range and indicate that fact with a special
error code. A guest that sees such partial progress is expected to
retry the operation for the portion of the address range that was not
completed.
Hyper-V does this partial completion dance when set_memory_decrypted()
is called to "decrypt" swiotlb bounce buffers that can be up to 1GB
in size. It is evidently the only VMM that does this, which is why
nobody noticed this until now.
> From: Dave Hansen <dave.hansen@intel.com>
> Sent: Friday, August 11, 2023 7:27 AM
> [...]
> On 8/10/23 19:12, 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.
>
> This changelog is not great. It gives zero background and wastes bytes
> on telling me which register the error code is in (I don't care in a
> changelog) and then using marketing fluff words like "fully enlightened".
>
> Let's stick to the facts, give some background, and also avoid
> regurgitating the GHCI, eh?
>
> How's this?
I appreciate the great changelog! Will use this in v10.
> x86/tdx: Retry partially-completed page conversion hypercalls
>
> TDX guest memory is private by default and the VMM may not access it.
> However, in cases where the guest needs to share data with the VMM,
> the guest and the VMM can coordinate to make memory shared between
> them.
>
> The guest side of this protocol includes the "MapGPA" hypercall. This
> call takes a guest physical address range. The hypercall spec (aka.
> the GHCI) says that the MapGPA call is allowed to return partial
> progress in mapping this range and indicate that fact with a special
> error code. A guest that sees such partial progress is expected to
> retry the operation for the portion of the address range that was not
> completed.
>
> Hyper-V does this partial completion dance when set_memory_decrypted()
> is called to "decrypt" swiotlb bounce buffers that can be up to 1GB
> in size. It is evidently the only VMM that does this, which is why
> nobody noticed this until now.
This clarifies the obscure "when needed" in my version by emphasizing
the "partial progress". This also gives necessary background info. Thanks!
@@ -703,14 +703,15 @@ 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);
+ /* Retrying the hypercall a second time should succeed; use 3 just in case */
+ const int max_retries_per_page = 3;
+ int retry_count = 0;
if (!enc) {
/* Set the shared (decrypted) bits: */
@@ -718,12 +719,51 @@ 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) {
+ struct tdx_hypercall_args args = {
+ .r10 = TDX_HYPERCALL_STANDARD,
+ .r11 = TDVMCALL_MAP_GPA,
+ .r12 = start,
+ .r13 = end - start };
+
+ u64 map_fail_paddr;
+ u64 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;
+ 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 */
@@ -24,6 +24,8 @@
#define TDVMCALL_MAP_GPA 0x10001
#define TDVMCALL_REPORT_FATAL_ERROR 0x10003
+#define TDVMCALL_STATUS_RETRY 1
+
#ifndef __ASSEMBLY__
/*