Re: [PATCH 07/33] x86/virt/tdx: Add SEAMCALL wrappers for TDX page cache management

[Chenyi Qiang]

On 2/27/2025 2:14 AM, Paolo Bonzini wrote:
> From: Rick Edgecombe <rick.p.edgecombe@xxxxxxxxx>
> 
> Intel TDX protects guest VMs from malicious host and certain physical
> attacks. The TDX module uses pages provided by the host for both control
> structures and for TD guest pages. These pages are encrypted using the
> MK-TME encryption engine, with its special requirements around cache
> invalidation. For its own security, the TDX module ensures pages are
> flushed properly and track which usage they are currently assigned. For
> creating and tearing down TD VMs and vCPUs KVM will need to use the
> TDH.PHYMEM.PAGE.RECLAIM, TDH.PHYMEM.CACHE.WB, and TDH.PHYMEM.PAGE.WBINVD
> SEAMCALLs.
> 
> Add tdh_phymem_page_reclaim() to enable KVM to call
> TDH.PHYMEM.PAGE.RECLAIM to reclaim the page for use by the host kernel.
> This effectively resets its state in the TDX module's page tracking
> (PAMT), if the page is available to be reclaimed. This will be used by KVM
> to reclaim the various types of pages owned by the TDX module. It will
> have a small wrapper in KVM that retries in the case of a relevant error
> code. Don't implement this wrapper in arch/x86 because KVM's solution
> around retrying SEAMCALLs will be better located in a single place.
> 
> Add tdh_phymem_cache_wb() to enable KVM to call TDH.PHYMEM.CACHE.WB to do
> a cache write back in a way that the TDX module can verify, before it
> allows a KeyID to be freed. The KVM code will use this to have a small
> wrapper that handles retries. Since the TDH.PHYMEM.CACHE.WB operation is
> interruptible, have tdh_phymem_cache_wb() take a resume argument to pass
> this info to the TDX module for restarts. It is worth noting that this
> SEAMCALL uses a SEAM specific MSR to do the write back in sections. In
> this way it does export some new functionality that affects CPU state.
> 
> Add tdh_phymem_page_wbinvd_tdr() to enable KVM to call
> TDH.PHYMEM.PAGE.WBINVD to do a cache write back and invalidate of a TDR,
> using the global KeyID. The underlying TDH.PHYMEM.PAGE.WBINVD SEAMCALL
> requires the related KeyID to be encoded into the SEAMCALL args. Since the
> global KeyID is not exposed to KVM, a dedicated wrapper is needed for TDR
> focused TDH.PHYMEM.PAGE.WBINVD operations.
> 
> Co-developed-by: Sean Christopherson <sean.j.christopherson@xxxxxxxxx>
> Signed-off-by: Sean Christopherson <sean.j.christopherson@xxxxxxxxx>
> Signed-off-by: Isaku Yamahata <isaku.yamahata@xxxxxxxxx>
> Signed-off-by: Kai Huang <kai.huang@xxxxxxxxx>
> Signed-off-by: Rick Edgecombe <rick.p.edgecombe@xxxxxxxxx>
> Reviewed-by: Binbin Wu <binbin.wu@xxxxxxxxxxxxxxx>
> Reviewed-by: Yuan Yao <yuan.yao@xxxxxxxxx>
> Message-ID: <20241203010317.827803-5-rick.p.edgecombe@xxxxxxxxx>
> Acked-by: Dave Hansen <dave.hansen@xxxxxxxxxxxxxxx>
> Signed-off-by: Paolo Bonzini <pbonzini@xxxxxxxxxx>
> ---
>  arch/x86/include/asm/tdx.h  | 17 +++++++++++++++
>  arch/x86/virt/vmx/tdx/tdx.c | 42 +++++++++++++++++++++++++++++++++++++
>  arch/x86/virt/vmx/tdx/tdx.h |  3 +++
>  3 files changed, 62 insertions(+)
> 
> diff --git a/arch/x86/include/asm/tdx.h b/arch/x86/include/asm/tdx.h
> index f783d5f1a0e1..ea26d79ec9d9 100644
> --- a/arch/x86/include/asm/tdx.h
> +++ b/arch/x86/include/asm/tdx.h
> @@ -33,6 +33,7 @@
>  #ifndef __ASSEMBLY__
>  
>  #include <uapi/asm/mce.h>
> +#include <linux/pgtable.h>
>  
>  /*
>   * Used by the #VE exception handler to gather the #VE exception
> @@ -140,6 +141,19 @@ struct tdx_vp {
>  	struct page **tdcx_pages;
>  };
>  
> +
> +static inline u64 mk_keyed_paddr(u16 hkid, struct page *page)
> +{
> +	u64 ret;
> +
> +	ret = page_to_phys(page);

We did "make allyesconfig" build test, and see the build error:

./arch/x86/include/asm/tdx.h: In function ‘mk_keyed_paddr’:
    ./include/asm-generic/memory_model.h:72:23: error: implicit
declaration of function ‘pfn_valid’ [-Werror=implicit-function-declaration]
       72 |         WARN_ON_ONCE(!pfn_valid(__pfn));
           \
          |                       ^~~~~~~~~
    ./include/asm-generic/bug.h:111:32: note: in definition of macro
‘WARN_ON_ONCE’
      111 |         int __ret_warn_on = !!(condition);
   \
          |                                ^~~~~~~~~
    ./arch/x86/include/asm/tdx.h:155:15: note: in expansion of macro
‘page_to_phys’
      155 |         ret = page_to_phys(page);
          |               ^~~~~~~~~~~~

Maybe add the fix:

diff --git a/arch/x86/include/asm/tdx.h b/arch/x86/include/asm/tdx.h
index 847252f520df..428243384696 100644
--- a/arch/x86/include/asm/tdx.h
+++ b/arch/x86/include/asm/tdx.h
@@ -5,6 +5,7 @@

 #include <linux/init.h>
 #include <linux/bits.h>
+#include <linux/mmzone.h>

 #include <asm/errno.h>
 #include <asm/ptrace.h>


> +	/* KeyID bits are just above the physical address bits: */
> +	ret |= (u64)hkid << boot_cpu_data.x86_phys_bits;
> +
> +	return ret;
> +
> +}