Re: [PATCH v2 1/6] x86/tdx: Retry TDVMCALL_MAP_GPA() when needed

From: "Kirill A. Shutemov" <kirill@shutemov.name>
To: Dexuan Cui <decui@microsoft.com>
Cc: ak@linux.intel.com, arnd@arndb.de, bp@alien8.de,
	brijesh.singh@amd.com, dan.j.williams@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, sathyanarayanan.kuppuswamy@linux.intel.com,
	seanjc@google.com, tglx@linutronix.de, tony.luck@intel.com,
	wei.liu@kernel.org, x86@kernel.org, mikelley@microsoft.com,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH v2 1/6] x86/tdx: Retry TDVMCALL_MAP_GPA() when needed
Date: Thu, 8 Dec 2022 22:48:00 +0300	[thread overview]
Message-ID: <20221208194800.n27ak4xj6pmyny46@box.shutemov.name> (raw)
In-Reply-To: <20221207003325.21503-2-decui@microsoft.com>

On Tue, Dec 06, 2022 at 04:33:20PM -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@microsoft.com>
> ---
> 
> 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.
> 
>  arch/x86/coco/tdx/tdx.c | 64 +++++++++++++++++++++++++++++++++--------
>  1 file changed, 52 insertions(+), 12 deletions(-)
> 
> diff --git a/arch/x86/coco/tdx/tdx.c b/arch/x86/coco/tdx/tdx.c
> index 3fee96931ff5..cdeda698d308 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
> @@ -692,14 +694,15 @@ static bool try_accept_one(phys_addr_t *start, unsigned long len,
>  }
>  
>  /*
> - * 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);
> +	int max_retry_cnt = 1000, retry_cnt = 0;

Hm. max_retry_cnt looks too high to me. I expected to see 3 or something.

Any justification for it to be *that* high?

-- 
  Kiryl Shutsemau / Kirill A. Shutemov