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

[Xu Yilun]

On Tue, Sep 22, 2026 at 02:48:58PM +0000, Edgecombe, Rick P wrote:
> On Mon, 2026-09-21 at 19:42 +0800, Xu Yilun wrote:
> > > 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.
> 
> Ok.
> 
> > 
> > And the cleaning up of its caller - config_tdx_module() - could be in a
> > separate cleanup patch out of this series, make sense?
> 
> Hmm, I'm not sure about that. I'd think you would need one patch to remove
> global_keyid arg and use the global reference. That stands on its own.
> 
> Another patch to extract the helper. BTW, since the goal of this series is to
> get acked, I'd think the cleanup patches could still go ahead.

Per the discussion [1], I'll leave the global_keyid unchanged. It's good
to pass the value of a global var around, to show we are not modifying it.

Thanks for connecting the 2 discussions.

[1]: https://lore.kernel.org/lkml/199d392a-02da-430b-95ee-b176ed0f43de@xxxxxxxxx/

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