Re: [PATCH v2 10/31] x86/virt/tdx: Add extra memory to TDX Module for Extensions

[Huang, Kai]

On Fri, 2026-04-24 at 11:07 +0800, Xu Yilun wrote:
> On Thu, Apr 23, 2026 at 10:29:31PM +0000, Huang, Kai wrote:
> > On Thu, 2026-04-23 at 17:05 +0000, Edgecombe, Rick P wrote:
> > > On Thu, 2026-04-23 at 00:59 +0000, Huang, Kai wrote:
> > > > Ditto here.  I don't think we should introduce any more cond_resched().
> > > > 
> > > > Btw, I think technically we can reuse the seamcall_ir_resched() you introduced
> > > > later, albeit in which a local '_args' is used as a copy of the original 'args',
> > > > but that has no harm for the case where we can just use the 'args' to loop.
> > > > 
> > > > I am wondering whether we can just use that here, or we just get rid of that
> > > > helper (then open retry by the callers of these SEAMCALL wrappers), since there
> > > > will be more cases where we need to manually set 'resume=1' in the SEAMCALL
> > > > input 'args' when retrying TDX_INTERRUPTED_RESUMABLE.
> > > 
> > > I kind of like the latter option to open code more of this stuff. The stacks of
> > > seamcall wrapper macros is already too much. 
> > 
> > Agreed.
> > 
> > And SEAMCALL *users* can actually come up with their own version of wrapper(s)
> > to do the retry.  E.g., currently seamcall_ir_resched() is only used for IOMMU
> > SEAMCALLs, and we can put this wrapper in the IOMMU code or coco/tdx-host.
> 
> After we have introduced TDX Module Extension, irq preemptable
> EXT-SEAMCALLs become a common concept. 
> 

It has been a concept since before the EXT-SEAMCALLs actually.  For instance,
TDX live migration using blocking export doesn't need any opt-in via module
extension (only the non-blocking way needs), but the SEAMCALLs to export/import
TD/vCPU/memory are all interruptible.

In fact, they had the "latency requirement" behind the INTERRUPT_RESUMABLE in
mind at the very beginning.  It's just at that point all SEAMCALLs were not that
heavy.

> It is irq preemptable so that the
> secure world remembers and resumes the context, no need for host to
> remind via resume lag.

The fact is the aforementioned live migration related export/import SEAMCALLs 
(there are 8 at least, but maybe more) all requires the explicit setting of
'resume=1' (plus using the SEAMCALL output as input for retry).  I don't know
the story behind this, though.  There might be some tricky thing here for the
module to remember and manage (e.g., migration has a concept of "migration
stream", and the resume is per-stream).

> 
> Today there are 3 EXT-SEAMCALLs, TDH_SPDM_CONNECT/DISCONNECT/MNG,
> irq preemption handling is a general requirement for them, and I think
> it is still true for any further EXT-SEAMCALLs.
> 
> So I think a general helper for EXT-SEAMCALLs makes sense.

Yes conceptually I agree, but not need to distinguish EXT-SEAMCALLs or not IMHO.

The problem is there isn't a common rule to follow.

E.g., let's say "the module can remember thus no resume flag is needed", how
about the SEAMCALL inputs?  Can the "output" args be directly used as input for
retry, or the original input should always be used?

Not to mention there's existing SEAMCALLs which require explicitly setting
'resume=1'.

I believe we can use some smart hack to implement a common one to cover all
cases above, but I am not sure whether it's worth to do (maybe we can have a try
to see how does it look like, though, I think).

Given the SEAMCALLs for TDX Connect seem to follow one rule to retry, and live
migration SEAMCALLs follow another rule, it seems for now the simplest way is to
introduce the needed retry helper in the layer of SEAMCALL *user* (TDX Connect
and migration).

> TDH.IOMMU.SETUP, however, is another case. It is not a EXT-SEAMCALL but
> happened to follow the same irq-retry handling process. To avoid code
> duplication we have:
> 
>  /*
>   * seamcall_ret_ir_exec() aliases seamcall_ret_ir_resched() for
>   * documentation purposes. It documents the TDX Module extension
>   * seamcalls that are long running / hard-irq preemptible flows that
>   * generate events. The calls using seamcall_ret_ir_resched() are long
>   * running flows, that periodically yield.
>   */
>  #define seamcall_ret_ir_exec seamcall_ret_ir_resched
> 
> TDH.IOMMU.SETUP uses seamcall_ret_ir_resched(), and EXT-SEAMCALLs use
> seamcall_ret_ir_exec().
> 
> How do you think?

Sorry I don't quite get.  What does "exec" postfix mean?

From patch 25, they are all in TDX core, so I don't quite get why we need to
distinguish EXT-SEAMCALLs vs normal ones.  IMHO it's an additional layer which
doesn't actually help address any problem.

Btw, we should really get rid of the "resched()" postfix from the function name
since cond_resched() is no longer needed and possibility of rescheduling is
implied pretty much all places in the kernel code now (except some special code
such as code in IRQ context).