Re: [PATCH v2 1/5] x86/virt/tdx: Move TDH.SYS.CONFIG operations into a wrapper

[Xu Yilun]

On Tue, Sep 15, 2026 at 08:45:04PM +0000, Edgecombe, Rick P wrote:
> On Tue, 2026-09-15 at 18:26 +0800, Xu Yilun wrote:
> > In Linux, SEAMCALL wrappers are introduced to avoid broad SEAMCALL
> > access by exposing only a selection of SEAMCALL leafs, but also to
> > abstract the SEAMCALL register ABIs.
> > 
> 
> 
> > The latter improves readability and
> > reuse for SEAMCALL leafs that are called multiple times.
> 
> I'm trying to adjust to not using former/latter. The feedback I've seen is that
> it is too much to remember as you read along. How about:
> The abstraction improves...

Yes.

> 
> > 
> > Some SEAMCALL leafs are not explicitly wrapped because the level of TDX
> > ABI details needed to perform the call is low enough to flow well with
>                                          ^are

the level ... is low enough, seems "is" is the right one? AI also
prefers "is".

> > the calling code.
> > 
> > For some of the currently unwrapped SEAMCALL leafs, TDX architecture
> > adjusts the ABI and adds SEAMCALL version selection for backward
> > compatibility.
> > 
> 
> Reads a little weird to me. Like I'm not sure when this adjusting is happening.
> How about:
> 
> ..., the TDX architecture has evolved the ABI to introduce new versions of
> existing SEAMCALLs. VMM code can select the version to call based on what is
> supported by the loaded TDX module.

good to me.

> 
> >  Future kernel will need to support the changes. 
> > 
> 
> ?? I guess you mean selecting between SEAMCALL versions?

Yes. I refactored the whole paragraph:

    For some of the currently unwrapped SEAMCALL leafs, the TDX architecture
    has evolved the ABI to introduce new leaf versions. The host can select
    the version to call based on what is supported by the loaded TDX module.
    When future kernel implements this version selection, more ABI details
    will leak into the surrounding caller code, which decreases the
    readability of the caller logic. To keep the ABI details contained, move
    the SEAMCALL leaf that will need version selection into a wrapper.

> 
> > This will
> > leak more ABI details into the surrounding caller code and decrease
> > readability of the other logic. To keep the ABI details contained, move
> > the SEAMCALL leafs that will need version selection into wrappers.
> > 
> > The cleanest separation would be to have kernel data types for the
> > SEAMCALL wrapper arguments,
> > 
> 
> This is now talking about general seamcall wrapper design. It could read like
> it's instead talking about clean separation of SEAMCALL versions?

OK, the transition is not good here.

    When defining the wrapper, the cleanest separation would be...

[...]

> Outside the diff it has:
> array_sz = tdmr_list->nr_consumed_tdmrs * sizeof(u64);
> 
> Could be now changed to:
> array_sz = tdmr_list->nr_consumed_tdmrs * sizeof(*tdmr_pa_array->phys);

Good to me. I tweaked it a bit on my preference:

	array_sz = tdmr_list->nr_consumed_tdmrs *
		sizeof(tdmr_pa_array->phys[0]);

> 
> A bit of existing cleanup, but the u64 is especially tucked away compared to
> before, so I'd argue its maintaining readability of the existing code.