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@xxxxxxxxxxxxxxx> > Reviewed-by: Michael Kelley <mikelley@xxxxxxxxxxxxx> > Signed-off-by: Dexuan Cui <decui@xxxxxxxxxxxxx> > --- > > 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@xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx > > 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
