Re: [PATCH v2 2/5] x86/virt/tdx: Configure add-on features on TDX module init

[Xu Yilun]

On Tue, Sep 15, 2026 at 08:54:01PM +0000, Edgecombe, Rick P wrote:
> On Tue, 2026-09-15 at 18:26 +0800, Xu Yilun wrote:
> > @@ -1012,12 +1021,22 @@ struct tdmr_info_pa_array {
> >  static __init int tdx_sys_config(struct tdmr_info_pa_array *tdmr_pa_array,
> >  				 unsigned int nr_tdmr_pa, u64 global_keyid)
> >  {
> > +	u64 addon_features0 = get_tdx_addon_features0();
> >  	struct tdx_module_args args = {
> >  		.rcx = __pa(tdmr_pa_array),
> >  		.rdx = nr_tdmr_pa,
> >  		.r8 = global_keyid,
> >  	};
> 
> It seems inconsistent that global_keyid is passed in even though it is basically
> global state, while addon_features0 is retrieved from the global state. Passing
> the result of get_tdx_addon_features0() in from the caller doesn't make a ton of
> sense. And we already reference tdx_global_keyid globally from another seamcall
> wrapper. So I'd think dropping the arg when you create the wrapper would be best
> in the end.
> 
> The smallest change to stay consistent would be go add an addon_features0 arg to
> tdx_sys_config(), but it's hard to justify as good code. I'd maybe go with the
> global_keyid change and it probably is a separate patch, but we are brushing up
> against mixing cleanup and feature enabling... thoughts?

I'm not sure. My thought would be removing "u64 global_keyid" argument
for the new added tdx_sys_config(), to make this patch reasonable.

And the cleaning up of its caller - config_tdx_module() - could be in a
separate cleanup patch out of this series, make sense?

make W=1 doesn't complain, but maybe shashiko will.

> 
> 
> >  
> > +	/*
> > +	 * Use SEAMCALL version 1 that supports add-on features if any are
> > +	 * requested. Otherwise use version 0 for backward compatibility.
> > +	 */
> > +	if (addon_features0) {
> > +		args.r9 = addon_features0;
> > +		args.version = 1;
> > +	}
> > +
> >  	return seamcall_prerr(TDH_SYS_CONFIG, &args);
> >  }
>