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

["Edgecombe, Rick P" <rick.p.edgecombe@intel.com>]

On Thu, 2026-04-30 at 11:17 -0700, Ackerley Tng wrote:
> Yan Zhao <yan.y.zhao@intel.com> writes:
> 
> > 
> > [...snip...]
> > 
> > diff --git a/arch/x86/virt/vmx/tdx/tdx.c b/arch/x86/virt/vmx/tdx/tdx.c
> > index b24b81cea5ea..e5a37ea2d4a0 100644
> > --- a/arch/x86/virt/vmx/tdx/tdx.c
> > +++ b/arch/x86/virt/vmx/tdx/tdx.c
> > @@ -710,7 +710,7 @@ static __init int tdmrs_set_up_pamt_all(struct tdmr_info_list *tdmr_list,
> >    * to normal kernel memory. Systems with the X86_BUG_TDX_PW_MCE erratum need to
> >    * do the conversion explicitly via MOVDIR64B.
> >    */
> > -static void tdx_quirk_reset_paddr(unsigned long base, unsigned long size)
> > +void tdx_quirk_reset_paddr(unsigned long base, unsigned long size)
> 
> Could this be updated to use phys_addr_t base and size_t size instead of
> generic unsigned long?

A type is a really good idea. It could look like a virtual address, despite the
paddr in the name.

I added it to the cleanup list. But I would prefer to keep this series focused
on resolving the critical controversy around the struct page/pfn type, rather
than adding in more things to debate. If you don't mind.

> 
> >   {
> >   	const void *zero_page = (const void *)page_address(ZERO_PAGE(0));
> >   	unsigned long phys, end;
> > @@ -729,6 +729,7 @@ static void tdx_quirk_reset_paddr(unsigned long base, unsigned long size)
> >   	 */
> >   	mb();
> >   }
> > +EXPORT_SYMBOL_FOR_KVM(tdx_quirk_reset_paddr);
> > 
> >   void tdx_quirk_reset_page(struct page *page)
> >   {
> > @@ -1920,17 +1921,17 @@ u64 tdh_phymem_page_wbinvd_tdr(struct tdx_td *td)
> >   {
> >   	struct tdx_module_args args = {};
> > 
> > -	args.rcx = mk_keyed_paddr(tdx_global_keyid, td->tdr_page);
> > +	args.rcx = mk_keyed_paddr(tdx_global_keyid, page_to_pfn(td->tdr_page));
> 
> Should mk_keyed_paddr() be updated to have a return type of phys_addr_t?
> I guess in this case since mk_keyed_paddr() is pretty much an internal
> function, returning u64 also makes sense to indicate that it should only
> be used to set 64 bit registers.

Yea, this is used to construct u64 inputs for seamcall args. So I think it
should keep returning u64s. Maybe instead it would be better to have a function
name pattern that denotes that purpose. We have some more helpers like this
coming.

But similarly, I'd like to not grow a snowballing cleanup series for this one.