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

[Xu Yilun]

On Thu, Apr 23, 2026 at 12:59:55AM +0000, Huang, Kai wrote:
> On Sat, 2026-03-28 at 00:01 +0800, Xu Yilun wrote:
> > +static int tdx_ext_mem_add(struct tdx_page_array *ext_mem)
> > +{
> > +	struct tdx_module_args args = {
> > +		.rcx = hpa_list_info_assign_raw(ext_mem),
> > +	};
> > +	u64 r;
> > +
> > +	tdx_clflush_page_array(ext_mem);
> > +
> > +	do {
> > +		r = seamcall_ret(TDH_EXT_MEM_ADD, &args);
> > +		cond_resched();
> > +	} while (r == TDX_INTERRUPTED_RESUMABLE);
> 
> 
> Ditto here.  I don't think we should introduce any more cond_resched().

Good to me.

> 
> 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.

No we can't. TDH_EXT_MEM_ADD is designed to use output parameter RCX
to override/update input parameter RCX, so the caller doesn't have to
do manual parameter update on retry call. Using seamcall_ir_resched()
makes each retry use the original RCX, not the updated one.

> 
> 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'd like to know why some SEAMCALLs needs resume flag but others don't.
If there is chance we don't introduce too much variants for the same thing,
that's most friendly to OS. And "no resume flag" is my best preference.

For now, I can see only one SEAMCALL with resume flag in mainline,
tdh_phymem_cache_wb(). I'd rather we treat it as an exception and no
resume flag any more if possible.

Then we don't have to make all following efforts, they are complex...

> 
> Unless you have good idea to unify them all?
> 
> E.g., we have something like below in our internal KVM code, using macros to do
> 'resume=1' and retry as the caller wishes.  But my understanding is Dave
> probably won't like macros.  :-)
> 
> (you may see broken indent/text due to text wrapper and sorry for that.) 
> 
> /*
>  * ...
>  *
>  * The retry_func and update_args allow the SEAMCALL to be retried in a loop if
>  * it can still return other error code when there's no race from both KVM and 
>  * vCPUs and can be "retried" until it succeeds.                               
>  */
> #define tdh_do_no_vcpus_retry(tdh_func, kvm, retry_func, update_args, args...)\
> ({                                                                            \
>         struct kvm_tdx *__kvm_tdx = to_kvm_tdx(kvm);                          \
>         u64 __err;                                                            \
>                                                                               \
>         lockdep_assert_held_write(&kvm->mmu_lock);                            \
>                                                                               \
>         __err = retry_func(tdh_func, update_args, args);                      \
>         if (unlikely(tdx_operand_busy(__err))) {                              \
>                 WRITE_ONCE(__kvm_tdx->wait_for_sept_zap, true);               \
>                 kvm_make_all_cpus_request(kvm, KVM_REQ_OUTSIDE_GUEST_MODE);   \
>                                                                               \
>                 __err = retry_func(tdh_func, update_args, args);              \
>                                                                               \
>                 WRITE_ONCE(__kvm_tdx->wait_for_sept_zap, false);              \
>         }                                                                     \
>         __err;                                                                \
> })                                                                             
> 
> #define tdh_intr_retry(tdh_func, update_args, args...)                        \
> ({                                                                            \
>         u64 ____err;                                                          \
>                                                                               \
>         do {                                                                  \
>                 ____err = tdh_func(args);                                     \
>                                                                               \
>                 if ((____err & TDX_SEAMCALL_STATUS_MASK) !=		      \
> 				TDX_INTERRUPTED_RESUMABLE)                   
> \	                       
>                         break;                                                \
>                                                                               \
>                 update_args;                                                  \
>         } while (1);                                                          \
>         ____err;                                                              \
> })
> 
> #define tdh_no_retry(tdh_func, update_args, args...)    tdh_func(args)
> 
> #define tdh_do_no_vcpus(tdh_func, kvm, args...) \
>         tdh_do_no_vcpus_retry(tdh_func, kvm, tdh_no_retry, ;, args)
> 
> #define tdh_do_no_vcpus_intr_retry(tdh_func, kvm, update_args, args...) \
>         tdh_do_no_vcpus_retry(tdh_func, kvm, tdh_intr_retry, update_args, args)