Re: [PATCH v2 1/4] x86/tdx: Use PFN directly for mapping guest private memory

Re: [PATCH v2 1/4] x86/tdx: Use PFN directly for mapping guest private memory

[Xiaoyao Li]

On 4/30/2026 9:49 AM, Yan Zhao wrote:
> From: Sean Christopherson <seanjc@google.com>
> 
> Remove struct page assumptions/constraints in the SEAMCALL wrapper APIs for
> mapping guest private memory and have them take PFN directly.
> 
> Having core TDX make assumptions that guest private memory must be backed
> by struct page (and/or folio) will create subtle dependencies on how
> KVM/guest_memfd allocates/manages memory (e.g., whether it uses memory
> allocated from core MM, if the memory is refcounted, or if the folio is
> split) that are easily avoided. [1].
> 
> KVM's MMUs work with PFNs. This is very much an intentional design choice.
> It ensures that the KVM MMUs remain flexible and are not too tied to the
> regular CPU MMUs and the kernel code around them. Using 'struct page' for
> TDX guest memory is not a good fit anywhere near the KVM MMU code [2].
> 
> Use "kvm_pfn_t pfn" for type safety. Using this KVM type is appropriate
> since APIs tdh_mem_page_add() and tdh_mem_page_aug() are exported to KVM
> only.
> 
> [ Yan: Replace "u64 pfn" with "kvm_pfn_t pfn" ]
> 
> Signed-off-by: Sean Christopherson <seanjc@google.com>
> Signed-off-by: Yan Zhao <yan.y.zhao@intel.com>
> Link: https://lore.kernel.org/all/aWgyhmTJphGQqO0Y@google.com [1]
> Link: https://lore.kernel.org/all/ac7V0g2q2hN3dU5u@google.com [2]

Reviewed-by: Xiaoyao Li <xiaoyao.li@intel.com>

...
> +static void tdx_clflush_pfn(kvm_pfn_t pfn)
> +{
> +	clflush_cache_range(__va(PFN_PHYS(pfn)), PAGE_SIZE);

If the pfn is not in the kernel direct map, we will get #PF, right?

There is on-going attempt to remove the direct map for guest_memfd. The 
good news is TDX is excluded. [1]

[1] https://lore.kernel.org/all/20260410151746.61150-9-kalyazin@amazon.com/

Re: [PATCH v2 2/4] x86/tdx: Use PFN directly for unmapping guest private memory

[Xiaoyao Li]

On 4/30/2026 9:49 AM, Yan Zhao wrote:
> From: Sean Christopherson<seanjc@google.com>
> 
> Remove struct page assumptions/constraints in APIs for unmapping guest
> private memory and have them take physical address directly.
> 
> Having core TDX make assumptions that guest private memory must be backed
> by struct page (and/or folio) will create subtle dependencies on how
> KVM/guest_memfd allocates/manages memory (e.g., whether it uses memory
> allocated from core MM, if the memory is refcounted, or if the folio is
> split) that are easily avoided. [1].
> 
> KVM's MMUs work with PFNs. This is very much an intentional design choice.
> It ensures that the KVM MMUs remain flexible and are not too tightly tied
> to the regular CPU MMUs and the kernel code around them. Using
> "struct page" for TDX guest memory is not a good fit anywhere near the KVM
> MMU code [2].
> 
> Therefore, for unmapping guest private memory: export
> tdx_quirk_reset_paddr() for direct KVM invocation, and convert the SEAMCALL
> wrapper API tdh_phymem_page_wbinvd_hkid() to take PFN as input (thus
> updating mk_keyed_paddr() and tdh_phymem_page_wbinvd_tdr()).
> 
> Intentionally have KVM pass PAGE_SIZE (rather than KVM_HPAGE_SIZE(level))
> to tdx_quirk_reset_paddr() in tdx_sept_remove_private_spte() to avoid
> mixing in huge page changes. The KVM_BUG_ON() check for !PG_LEVEL_4K in
> tdx_sept_remove_private_spte() justifies using PAGE_SIZE.
> 
> Do not convert tdx_reclaim_page() to use PFN as input since it currently
> does not remove guest private memory.
> 
> Use "kvm_pfn_t pfn" for type safety. Using this KVM type is appropriate
> since APIs tdh_phymem_page_wbinvd_hkid() and tdx_quirk_reset_paddr() are
> exported to KVM only.
> 
> [Yan: Use kvm_pfn_t,exclude tdx_reclaim_page(),use tdx_quirk_reset_paddr()]
> 
> Signed-off-by: Sean Christopherson<seanjc@google.com>
> Signed-off-by: Yan Zhao<yan.y.zhao@intel.com>
> Link:https://lore.kernel.org/all/aWgyhmTJphGQqO0Y@google.com [1]
> Link:https://lore.kernel.org/all/ac7V0g2q2hN3dU5u@google.com [2]

Reviewed-by: Xiaoyao Li <xiaoyao.li@intel.com>

Re: [PATCH v2 3/4] x86/tdx: Drop exported function tdx_quirk_reset_page()

[Xiaoyao Li]

On 4/30/2026 9:50 AM, Yan Zhao wrote:
> KVM invokes tdx_quirk_reset_page() to reset TDX control pages (including
> S-EPT pages, TDR page, etc.), as all those pages are allocated by KVM TDX
> and thus always have struct page.
> 
> However, it's also reasonable for KVM to reset those TDX control pages via
> tdx_quirk_reset_paddr() directly, eliminating the need to export two
> parallel APIs. Keeping tdx_quirk_reset_page() as a one-line helper in the
> header file is also unnecessary.
> 
> No functional change intended.
> 
> Suggested-by: Paolo Bonzini <pbonzini@redhat.com>
> Suggested-by: Xiaoyao Li <xiaoyao.li@intel.com>
> Signed-off-by: Yan Zhao <yan.y.zhao@intel.com>

Reviewed-by: Xiaoyao Li <xiaoyao.li@intel.com>

> ---
>   arch/x86/include/asm/tdx.h  | 1 -
>   arch/x86/kvm/vmx/tdx.c      | 4 ++--
>   arch/x86/virt/vmx/tdx/tdx.c | 6 ------
>   3 files changed, 2 insertions(+), 9 deletions(-)
> 
> diff --git a/arch/x86/include/asm/tdx.h b/arch/x86/include/asm/tdx.h
> index 65f7d874fb5a..9c63deaa0e8f 100644
> --- a/arch/x86/include/asm/tdx.h
> +++ b/arch/x86/include/asm/tdx.h
> @@ -153,7 +153,6 @@ int tdx_guest_keyid_alloc(void);
>   u32 tdx_get_nr_guest_keyids(void);
>   void tdx_guest_keyid_free(unsigned int keyid);
>   
> -void tdx_quirk_reset_page(struct page *page);
>   void tdx_quirk_reset_paddr(unsigned long base, unsigned long size);
>   
>   struct tdx_td {
> diff --git a/arch/x86/kvm/vmx/tdx.c b/arch/x86/kvm/vmx/tdx.c
> index a2aadc6d0174..9bd4fd748e2a 100644
> --- a/arch/x86/kvm/vmx/tdx.c
> +++ b/arch/x86/kvm/vmx/tdx.c
> @@ -343,7 +343,7 @@ static int tdx_reclaim_page(struct page *page)
>   
>   	r = __tdx_reclaim_page(page);
>   	if (!r)
> -		tdx_quirk_reset_page(page);
> +		tdx_quirk_reset_paddr(page_to_phys(page), PAGE_SIZE);
>   	return r;
>   }
>   
> @@ -597,7 +597,7 @@ static void tdx_reclaim_td_control_pages(struct kvm *kvm)
>   	if (TDX_BUG_ON(err, TDH_PHYMEM_PAGE_WBINVD, kvm))
>   		return;
>   
> -	tdx_quirk_reset_page(kvm_tdx->td.tdr_page);
> +	tdx_quirk_reset_paddr(page_to_phys(kvm_tdx->td.tdr_page), PAGE_SIZE);
>   
>   	__free_page(kvm_tdx->td.tdr_page);
>   	kvm_tdx->td.tdr_page = NULL;
> diff --git a/arch/x86/virt/vmx/tdx/tdx.c b/arch/x86/virt/vmx/tdx/tdx.c
> index e5a37ea2d4a0..deb67e68f85f 100644
> --- a/arch/x86/virt/vmx/tdx/tdx.c
> +++ b/arch/x86/virt/vmx/tdx/tdx.c
> @@ -731,12 +731,6 @@ void tdx_quirk_reset_paddr(unsigned long base, unsigned long size)
>   }
>   EXPORT_SYMBOL_FOR_KVM(tdx_quirk_reset_paddr);
>   
> -void tdx_quirk_reset_page(struct page *page)
> -{
> -	tdx_quirk_reset_paddr(page_to_phys(page), PAGE_SIZE);
> -}
> -EXPORT_SYMBOL_FOR_KVM(tdx_quirk_reset_page);
> -
>   static __init void tdmr_quirk_reset_pamt(struct tdmr_info *tdmr)
>   
>   {

Re: [PATCH v2 4/4] x86/virt/tdx: Move mk_keyed_paddr() to tdx.c due to no external users

[Xiaoyao Li]

On 4/30/2026 9:50 AM, Yan Zhao wrote:
> Move mk_keyed_paddr() from tdx.h to tdx.c to avoid unnecessary header
> inclusion and improve encapsulation since there are no users outside of
> tdx.c.
> 
> No functional change intended.

Missing a new blank line.

> Signed-off-by: Yan Zhao <yan.y.zhao@intel.com>

Reviewed-by: Xiaoyao Li <xiaoyao.li@intel.com>

> ---
>   arch/x86/include/asm/tdx.h  | 6 ------
>   arch/x86/virt/vmx/tdx/tdx.c | 6 ++++++
>   2 files changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/arch/x86/include/asm/tdx.h b/arch/x86/include/asm/tdx.h
> index 9c63deaa0e8f..503f9a3f46d6 100644
> --- a/arch/x86/include/asm/tdx.h
> +++ b/arch/x86/include/asm/tdx.h
> @@ -177,12 +177,6 @@ struct tdx_vp {
>   	struct page **tdcx_pages;
>   };
>   
> -static inline u64 mk_keyed_paddr(u16 hkid, kvm_pfn_t pfn)
> -{
> -	/* KeyID bits are just above the physical address bits. */
> -	return PFN_PHYS(pfn) | ((u64)hkid << boot_cpu_data.x86_phys_bits);
> -}
> -
>   u64 tdh_vp_enter(struct tdx_vp *vp, struct tdx_module_args *args);
>   u64 tdh_mng_addcx(struct tdx_td *td, struct page *tdcs_page);
>   u64 tdh_mem_page_add(struct tdx_td *td, u64 gpa, kvm_pfn_t pfn, struct page *source,
> diff --git a/arch/x86/virt/vmx/tdx/tdx.c b/arch/x86/virt/vmx/tdx/tdx.c
> index deb67e68f85f..967482ae3c80 100644
> --- a/arch/x86/virt/vmx/tdx/tdx.c
> +++ b/arch/x86/virt/vmx/tdx/tdx.c
> @@ -1911,6 +1911,12 @@ u64 tdh_phymem_cache_wb(bool resume)
>   }
>   EXPORT_SYMBOL_FOR_KVM(tdh_phymem_cache_wb);
>   
> +static inline u64 mk_keyed_paddr(u16 hkid, kvm_pfn_t pfn)
> +{
> +	/* KeyID bits are just above the physical address bits. */
> +	return PFN_PHYS(pfn) | ((u64)hkid << boot_cpu_data.x86_phys_bits);
> +}
> +
>   u64 tdh_phymem_page_wbinvd_tdr(struct tdx_td *td)
>   {
>   	struct tdx_module_args args = {};

Re: [PATCH v2 1/4] x86/tdx: Use PFN directly for mapping guest private memory

[Yan Zhao]

On Thu, May 07, 2026 at 03:49:09PM +0800, Xiaoyao Li wrote:
> On 4/30/2026 9:49 AM, Yan Zhao wrote:
> > From: Sean Christopherson <seanjc@google.com>
> > 
> > Remove struct page assumptions/constraints in the SEAMCALL wrapper APIs for
> > mapping guest private memory and have them take PFN directly.
> > 
> > Having core TDX make assumptions that guest private memory must be backed
> > by struct page (and/or folio) will create subtle dependencies on how
> > KVM/guest_memfd allocates/manages memory (e.g., whether it uses memory
> > allocated from core MM, if the memory is refcounted, or if the folio is
> > split) that are easily avoided. [1].
> > 
> > KVM's MMUs work with PFNs. This is very much an intentional design choice.
> > It ensures that the KVM MMUs remain flexible and are not too tied to the
> > regular CPU MMUs and the kernel code around them. Using 'struct page' for
> > TDX guest memory is not a good fit anywhere near the KVM MMU code [2].
> > 
> > Use "kvm_pfn_t pfn" for type safety. Using this KVM type is appropriate
> > since APIs tdh_mem_page_add() and tdh_mem_page_aug() are exported to KVM
> > only.
> > 
> > [ Yan: Replace "u64 pfn" with "kvm_pfn_t pfn" ]
> > 
> > Signed-off-by: Sean Christopherson <seanjc@google.com>
> > Signed-off-by: Yan Zhao <yan.y.zhao@intel.com>
> > Link: https://lore.kernel.org/all/aWgyhmTJphGQqO0Y@google.com [1]
> > Link: https://lore.kernel.org/all/ac7V0g2q2hN3dU5u@google.com [2]
> 
> Reviewed-by: Xiaoyao Li <xiaoyao.li@intel.com>
Thanks!

> > +static void tdx_clflush_pfn(kvm_pfn_t pfn)
> > +{
> > +	clflush_cache_range(__va(PFN_PHYS(pfn)), PAGE_SIZE);
> 
> If the pfn is not in the kernel direct map, we will get #PF, right?
Right.

There's no simple interface like pfn_range_is_mapped() that tells whether a PFN
has direct map or not if removing direct map is supported.

So, as PFNs not in the kernel direct map are unexpected for TDX, this series
leaves #PF, which is obvious enough for debugging.

> There is on-going attempt to remove the direct map for guest_memfd. The good
> news is TDX is excluded. [1]
We can see if any code refinement is necessary if TDX is included in the future.

 
> [1] https://lore.kernel.org/all/20260410151746.61150-9-kalyazin@amazon.com/

Re: [PATCH v2 1/4] x86/tdx: Use PFN directly for mapping guest private memory

[Sean Christopherson]

On Thu, May 07, 2026, Yan Zhao wrote:
> On Thu, May 07, 2026 at 03:49:09PM +0800, Xiaoyao Li wrote:
> > On 4/30/2026 9:49 AM, Yan Zhao wrote:
> > There is on-going attempt to remove the direct map for guest_memfd. The good
> > news is TDX is excluded. [1]
> We can see if any code refinement is necessary if TDX is included in the future.

Yeah, I wouldn't worry too much about that effort.  The onus will firmly be on
that series to do the right thing for TDX (and any other unique code).