Subject: Re: [PATCH v9 01/23] x86/virt/tdx: Consolidate TDX global initialization states
From: Dave Hansen <[email-masked]>
To: [email-masked]

[Draft review generated by x86-maintainer-agent applying Dave_Hansen.txt
 rules to the v9 series.  Strip this banner before sending.]

> +struct tdx_module_state {
> +	bool initialized;
> +	bool sysinit_done;
> +	int sysinit_ret;
> +};
> +
> +static struct tdx_module_state tdx_module_state;

Two small things on this one.

1. The name (Dave_Hansen.txt RULE 10).  These three fields are
   bookkeeping for the *initialization* flow, not "the state of the
   module".  Calling it `tdx_module_state` invites a future reader (or
   developer) to dump the module's runtime status into it, which is
   not what you want.  Suggest `tdx_init_state` or
   `tdx_module_init_state`.

2. The struct and the variable share an identifier.  Kernel-legal, but
   when you grep tdx_module_state you can no longer tell type uses
   from instance uses.  Renaming the variable
   (e.g. tdx_module_init_state.{initialized,sysinit_done,sysinit_ret})
   reads better and pairs with point (1).

The changelog is clear about *why* this consolidation is happening
(RULE 1) -- specifically the "easy to miss when a new state is added"
sentence is the whole justification and it's exactly the right place.

Drop of __ro_after_init: also called out in the changelog.  Good.

With the naming addressed:

Reviewed-by: Dave Hansen <[email-masked]>  [conditional]