Re: [PATCH v2 03/31] x86/virt/tdx: Add tdx_page_array helpers for new TDX Module objects

[Edgecombe, Rick P]

On Mon, 2026-03-30 at 18:25 +0800, Xu Yilun wrote:
> > On Sat, 2026-03-28 at 00:01 +0800, Xu Yilun wrote:
> > > Add struct tdx_page_array definition for new TDX Module object
> > > types - HPA_ARRAY_T and HPA_LIST_INFO. 
> > 
> > This is unfortunate. I see you agree in the comments.
> 
> Yes, basically they are defining the same concept, behave mostly the same
> but some differences...
> 
> > 
> > > 
> > > They are used as input/output
> > > parameters in newly defined SEAMCALLs. Also define some helpers to
> > > allocate, setup and free tdx_page_array.
> > > 
> > > HPA_ARRAY_T and HPA_LIST_INFO are similar in most aspects. They both
> > > represent a list of pages for TDX Module accessing. There are several
> > > use cases for these 2 structures:
> > > 
> > >  - As SEAMCALL inputs. They are claimed by TDX Module as control pages.
> > >    Control pages are private pages for TDX Module to hold its internal
> > >    control structures or private data. TDR, TDCS, TDVPR... are existing
> > >    control pages, just not added via tdx_page_array.
> > >  - As SEAMCALL outputs. They were TDX Module control pages and now are
> > >    released.
> > >  - As SEAMCALL inputs. They are just temporary buffers for exchanging
> > >    data blobs in one SEAMCALL. TDX Module will not hold them for long
> > >    time.
> > 
> > This is kind of verbose for what it seems to be trying to say. It's just
> > that
> 
> I assume if you feel the explanation of "what is control page" is off
> track. I added it cause the term firstly appears in x86 (only in KVM
> TDX previously), and people ask the definition:
> 
> https://lore.kernel.org/all/cfcfb160-fcd2-4a75-9639-5f7f0894d14b@xxxxxxxxx/

I meant it more generally.

> 
> > these types can be input or output params. The TDX module could hold on to
> > the
> > pages for a long time, or just transiently.
> 
> Mm.. I'm trying to ramp up on the kernel API level flow:
> 
> For control pages, it would be hold by TDX Module long time, so host
> inputs the page array, later TDX Module outputs the page array back.
> Host need to verify the outputs.
> 
> For shared pages, TDX Module's accessing is transient in one SEAMCALL,
> so only as input, TDX Module never needs to output the array.
> 
> I think the verboseness makes the following pseudo code easier to
> understand.
> 
> > For that latter part I think you are
> > trying to say sometimes they need flushing and sometimes they don't?
> 
> Yeah.
> control pages => long term => host verifies and releases => flush on release
> shared pages => transient => no verify and releases => no flush
> 
> Maybe I should mention the flushing is already covered by releasing
> kAPI.

I hear:
 1. Long time vs short time
 2. Accessed as private vs "shared"

I think (2) is the important point, right? Why does (1) matter?

> > 
> > > 
> > > The 2 structures both need a 'root page' which contains a list of HPAs.
> > > They collapse the HPA of the root page and the number of valid HPAs
> > > into a 64 bit raw value for SEAMCALL parameters. The root page is
> > > always a medium for passing data pages, TDX Module never keeps the
> > > root page.
> > > 
> > > A main difference is HPA_ARRAY_T requires singleton mode when
> > > containing just 1 functional page (page0). In this mode the root page is
> > > not needed and the HPA field of the raw value directly points to the
> > > page0. But in this patch, root page is always allocated for user
> > > friendly kAPIs.
> > 
> > "singleton mode"? What is it? If it's the case of not needing populate loop,
> > it
> 
> It is the SEAMCALL level detail for HPA_ARRAY_T. It is literally as
> explained above - the HPA field should be filled by page0, not root page.
> 
> > probably deserves more explanation. I'm not sure, but the populate loop
> > seems to
> > drive a lot of the struct design?
> 
> The caller is not aware of singleton mode. Actually, I'm trying to make
> the tdx_page_array independent of HPA_ARRAY_T or HPA_LIST_INFO details
> when allocating/populating, root page is still populated even not needed
> for singleton mode. The differences only happen when collaping the struct
> into u64 SEAMCALL parameters.

It seems tdx_page_array combines two concepts. An array of pages, and the method
that the pages get handed to the TDX module. What if we broke apart these
concepts?

> 
> > 
> > > 
> > > Another small difference is HPA_LIST_INFO contains a 'first entry' field
> > > which could be filled by TDX Module. This simplifies host by providing
> > > the same structure when re-invoke the interrupted SEAMCALL. No need for
> > > host to touch this field.
> > 
> > Not clear what this is talking about. But I'm starting to wonder if we
> > should be
> > so bold to claim that the differences between the types really simplify the
> > host. 
> 
> I'm talking about another SEAMCALL level detail. Sometimes TDX Module
> got interrupted in the middle of page array processing, it needs an
> anchor to resuming from where it stops, TDX Module record the anchor
> in the 'first entry'.
> 
> By illustrating these SEAMCALL level differences, I want to explain
> they don't impact the general SW flow and kAPI cares about them
> internally.
> 
> Yes in POC code we do write dedicated code for each type, but it ends up
> with plenty of similar logics on caller side about root page
> manipulation. By now, the differences are not much, but I think we
> should not write copies for every type, we should stop new types.

Hmm, doesn't it seem like this is quickly becoming complicated though, to
combine all the different types together? And it seems there are more coming
that people want to add to this.

> 
> Please allow me to stop here, will continue later...
> 
> Thanks.