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

[Yan Zhao <yan.y.zhao@intel.com>]

On Thu, Mar 19, 2026 at 04:56:10PM +0800, Xiaoyao Li wrote:
> On 3/19/2026 2:45 PM, Yan Zhao wrote:
> > On Thu, Mar 19, 2026 at 11:20:48AM +0800, Xiaoyao Li wrote:
> > > On 3/19/2026 8:58 AM, Yan Zhao wrote:
> > > > From: Sean Christopherson <seanjc@google.com>
> [...]
> > > > diff --git a/arch/x86/virt/vmx/tdx/tdx.c b/arch/x86/virt/vmx/tdx/tdx.c
> > > > index a9dd75190c67..2f9d07ad1a9a 100644
> > > > --- a/arch/x86/virt/vmx/tdx/tdx.c
> > > > +++ b/arch/x86/virt/vmx/tdx/tdx.c
> > > > @@ -730,9 +730,9 @@ static void tdx_quirk_reset_paddr(unsigned long base, unsigned long size)
> > > >    	mb();
> > > >    }
> > > > -void tdx_quirk_reset_page(struct page *page)
> > > > +void tdx_quirk_reset_page(kvm_pfn_t pfn)
> > > 
> > > So why keep the function tdx_quirk_reset_page() but expect passing in the
> > > kvm_pfn_t? It looks werid that the name indicates to reset a page but what
> > > gets passed in is a pfn.
> > I thought about introducing tdx_quirk_reset_pfn(). But considering
> > tdx_quirk_reset_pfn() has to be an exported API, I'm reluctant to do that.
> > 
> > Given that even with tdx_quirk_reset_pfn(), it still expects TDX convertible
> > RAM, I think having tdx_quirk_reset_page() to take pfn is still acceptable.
> > 
> > We just don't want KVM to do pfn --> struct page --> pfn conversions.
> 
> Only tdx_sept_remove_private_spte() is doing such conversions. While
> tdx_reclaim_page() and tdx_reclaim_td_control_pages() already have the
> struct page natively.
Unlike requiring KVM to call pfn_to_page() before invoking guest private memory
related APIs, Having tdx_reclaim_page() and tdx_reclaim_td_control_pages() to
call page_to_pfn() does not impose unnecessary assumptions of how KVM allocates
memory. So, I think it's fine for them to invoke tdx_quirk_reset_page() which
takes PFN as input.

> So why not considering option 2?
> 
>   2. keep tdx_quirk_reset_page() as-is for the cases of
>      tdx_reclaim_page() and tdx_reclaim_td_control_pages() that have the
>      struct page. But only change tdx_sept_remove_private_spte() to use
>      tdx_quirk_reset_paddr() directly.
> 
> It will need export tdx_quirk_reset_paddr() for KVM. I think it will be OK?
I don't think it's necessary. But if we have to export an extra API, IMHO,
tdx_quirk_reset_pfn() is better than tdx_quirk_reset_paddr(). Otherwise,
why not only expose tdx_quirk_reset_paddr()?