Re: [PATCH v2 05/31] x86/virt/tdx: Extend tdx_page_array to support IOMMU_MT

Xu Yilun
2026-04-19T08:55:11Z

On Fri, Apr 17, 2026 at 04:58:43PM -0700, Dan Williams wrote:
> Xu Yilun wrote:
> [..]
> > >
> > > I'm drafting some changes and make the tdx_page_array look like:
> > >
> > > struct tdx_page_array {
> > > /* public: */
> > > unsigned int nr_pages;
> > > struct page **pages;
> > >
> > > /* private: */
> > > u64 *root;
> > > bool flush_on_free;
>
> How about "need_phymem_page_wbinvd"?

Yes.

>
> That makes it a bit more greppable and not to be confused with other
> flushing.
>
> [..]
> > Hi, I end up made the following changes on top of this series:
> >
> > -------8<--------
> >
> > arch/x86/include/asm/tdx.h | 32 +-
> > arch/x86/virt/vmx/tdx/tdx.c | 561 ++++++++------------------
> > drivers/virt/coco/tdx-host/tdx-host.c | 179 ++++++--
> > 3 files changed, 316 insertions(+), 456 deletions(-)
> >
> > + ret = tdx_ext_mem_setup(nr_pages, &ext_mem);
> > if (ret)
> > + return ret;
> > }
> >
> > + ret = tdx_ext_init();
> > + if (ret)
> > + goto out_remove_ext_mem;
> > +
> > /*
> > + * Extensions memory is never reclaimed once assigned, stop tracking it
> > + * and free the tracking structures.
> > */
> > + tdx_page_array_free(ext_mem.chunk);
>
> Wait, these pages belong to the module now, they can't be freed, or I am
> missing something?

With this new solution, tdx_page_array is downgraded to a descriptor,
doesn't manage the actual data pages/memory any more. So
tdx_page_array_free() will not free data pages, only frees the
tdx_page_array descriptor.

>
> > + kfree(ext_mem.pages);
>
> Releasing this makes sense.
>
> >
> > pr_info("%lu KB allocated for TDX Module Extensions\n",
> > nr_pages * PAGE_SIZE / 1024);
> >
> > return 0;
> >
> > -out_flush:
> > - if (ext_mem)
> > +out_remove_ext_mem:
> > + if (nr_pages) {
> > + /*
> > + * TDH.EXT.MEM.ADD only collects required memory. TDX.EXT.INIT
> > + * does the actual initialization so if it fails some pages may
> > + * have been touched by the TDX module, flush cache before
> > + * returning these pages to kernel.
> > + */
> > wbinvd_on_all_cpus();
> > + tdx_ext_mem_remove(&ext_mem);
>
> This only releases the last populated chunk, not all previous chunks,
> right?

Not true. ext_mem stores all the data pages and the reusable descriptor
'chunk' for SEAMCALL. tdx_ext_mem_remove() removes all the data pages
and the 'chunk'.