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

[Xu Yilun]

On Wed, Apr 01, 2026 at 12:17:45AM +0000, Edgecombe, Rick P wrote:
> On Tue, 2026-03-31 at 22:19 +0800, Xu Yilun wrote:
> > > Consider the amount of tricks that are needed to coax the tdx_page_array to
> > > populate the handoff page as needed. It adds 2 pages here, then subtracts
> > > them
> > > later in the callback. Then tweaks the pa in tdx_page_array_populate() to
> > > add
> > > the length...
> > 
> > mm.. The tricky part is the specific memory requirement/allocation, the
> > common part is the pa list contained in a root page. Maybe we only model
> > the later, let the specific user does the memory allocation. Is that
> > closer to your "break concepts apart" idea?
> 
> I haven't wrapped my head around this enough to suggest anything is definitely
> the right approach.
> 
> But yes, the idea would be that the allocation of the list of pages to give to
> the TDX module would be a separate allocation and set of management functions.
> And the the allocation of the pages that are used to communicate the list of
> pages (and in this case other args) with the module would be another set. So
> each type of TDX module arg page format (IOMMU_MT, etc) would be separable, but
> share the page list allocation part only. It looks like Nikolay was probing
> along the same path. Not sure if he had the same solution in mind.
> 
> So for this:
> 1. Allocate a list or array of pages using a generic method.
> 2. Allocate these two IOMMU special pages.
> 3. Allocate memory needed for the seamcall (root pages)
> 
> Hand all three to the wrapper and have it shove them all through in the special
> way it prefers.

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;
  };

  - I removed the page allocations for tdx_page_array kAPIs. Now the
    caller needs to allocate the struct page **pages and the page list,
    then create the tdx_page_array by providing these pages.

    struct tdx_page_array *tdx_page_array_create(struct page **pages,
						 unsigned int nr_pages)

    This also means tdx_page_array doesn't have to hold more than 512
    pages anymore, it now an exact descriptor for the TDX Module's
    definitions rather than a manager. It's a chunk of the required
    memory when we need more than 512 pages. This eliminates the need
    for 'offset' field and the slide window operations so make the
    helpers simpler.

  - I still keep the generic struct tdx_page_array to represent all
    kinds of object types (HPA_ARRAY_T, HPA_LIST_INFO, IOMMU_MT), and
    provide the tdx_page_array to SEAMCALL helpers as parameters. I
    think this structure is generally good enough to represent a list of
    pages, keeps type safety compared to a list of HPAs.

  - I still record both the page list (struct page **pages) and the HPA
    list (in u64 *root). struct page **pages works with kernel memory
    management (e.g. vmap) well while the populated root works with
    SEAMCALLs.

  - I'm not introducing more structures each for an object type, like 
    struct hpa_array, struct hpa_list_info, struct iommu_metadata. They
    are conceptually the same thing. The iommu_mt supports multi-order
    pages, hpa_array_t & hpa_list_info don't support. But their bit
    definitions don't conflict. I can use the same piece of code to
    populate their root page content.

  - Add a flush_on_free field to mark if a cache write back is needed on
    tdx_page_array_free(), then we don't need 2 free APIs.

I want to clean up my code, then post an incremental patch for preview.

Thanks.