On Mon, Nov 21, 2022 at 11:51:47AM -0800, 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 TDX guest runs on Hyper-V, Hyper-V returns the retry error
> when hyperv_init() -> swiotlb_update_mem_attributes() ->
> set_memory_decrypted() decrypts up to 1GB of swiotlb bounce buffers.
>
> Signed-off-by: Dexuan Cui <decui@xxxxxxxxxxxxx>
> ---
> arch/x86/coco/tdx/tdx.c | 65 +++++++++++++++++++++++++++++++++++++----
> 1 file changed, 59 insertions(+), 6 deletions(-)
>
> diff --git a/arch/x86/coco/tdx/tdx.c b/arch/x86/coco/tdx/tdx.c
> index 3fee96931ff5..46971cc7d006 100644
> --- a/arch/x86/coco/tdx/tdx.c
> +++ b/arch/x86/coco/tdx/tdx.c
> @@ -20,6 +20,8 @@
> /* TDX hypercall Leaf IDs */
> #define TDVMCALL_MAP_GPA 0x10001
>
> +#define TDVMCALL_STATUS_RETRY 1
> +
> /* MMIO direction */
> #define EPT_READ 0
> #define EPT_WRITE 1
> @@ -52,6 +54,25 @@ static inline u64 _tdx_hypercall(u64 fn, u64 r12, u64 r13, u64 r14, u64 r15)
> return __tdx_hypercall(&args, 0);
> }
>
> +static inline u64 _tdx_hypercall_output_r11(u64 fn, u64 r12, u64 r13, u64 r14,
> + u64 r15, u64 *r11)
> +{
> + struct tdx_hypercall_args args = {
> + .r10 = TDX_HYPERCALL_STANDARD,
> + .r11 = fn,
> + .r12 = r12,
> + .r13 = r13,
> + .r14 = r14,
> + .r15 = r15,
> + };
> +
> + u64 ret;
> +
> + ret = __tdx_hypercall(&args, TDX_HCALL_HAS_OUTPUT);
> + *r11 = args.r11;
> + return ret;
> +}
> +
I'm not convinced it deserves a separate helper for one user.
Does it look that ugly if tdx_map_gpa() uses __tdx_hypercall() directly?
> /* Called from __tdx_hypercall() for unrecoverable failure */
> void __tdx_hypercall_failed(void)
> {
> @@ -691,6 +712,43 @@ static bool try_accept_one(phys_addr_t *start, unsigned long len,
> return true;
> }
>
> +/*
> + * 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_map_gpa(phys_addr_t start, phys_addr_t end, bool enc)
> +{
> + u64 ret, r11;
> +
> + while (1) {
Endless? Maybe an upper limit if no progress?
> + ret = _tdx_hypercall_output_r11(TDVMCALL_MAP_GPA, start,
> + end - start, 0, 0, &r11);
> + if (!ret)
> + break;
> +
> + if (ret != TDVMCALL_STATUS_RETRY)
> + break;
> +
> + /*
> + * The guest must retry the operation for the pages in the
> + * region starting at the GPA specified in R11. Make sure R11
> + * contains a sane value.
> + */
> + if ((r11 & ~cc_mkdec(0)) < (start & ~cc_mkdec(0)) ||
> + (r11 & ~cc_mkdec(0)) >= (end & ~cc_mkdec(0)))
> + return false;
Emm. All of them suppose to have shared bit set, why not compare directly
without cc_mkdec() dance?
> +
> + start = r11;
> +
> + /* Set the shared (decrypted) bit. */
> + if (!enc)
> + start |= cc_mkdec(0);
> + }
> +
> + return !ret;
> +}
> +
> /*
> * 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
> @@ -707,12 +765,7 @@ 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))
> + if (!tdx_map_gpa(start, end, enc))
> return false;
>
> /* private->shared conversion requires only MapGPA call */
> --
> 2.25.1
>
--
Kiryl Shutsemau / Kirill A. Shutemov
