> -----Original Message-----
> From: Huang, Kai <kai.hu...@intel.com>
> Sent: Wednesday, August 13, 2025 2:08 PM
> To: Reshetova, Elena <elena.reshet...@intel.com>; Hansen, Dave
> <dave.han...@intel.com>
> Cc: sea...@google.com; mi...@kernel.org; Scarlata, Vincent R
> <vincent.r.scarl...@intel.com>; x...@kernel.org; jar...@kernel.org;
> Annapurve, Vishal <vannapu...@google.com>; linux-kernel@vger.kernel.org;
> Mallick, Asit K <asit.k.mall...@intel.com>; Aktas, Erdem
> <erdemak...@google.com>; Cai, Chong <cho...@google.com>; Bondarevska,
> Nataliia <bond...@google.com>; linux-...@vger.kernel.org; Raynor, Scott
> <scott.ray...@intel.com>
> Subject: Re: [PATCH v13 4/5] x86/sgx: Implement ENCLS[EUPDATESVN]
>
>
> >
> > +/* Counter to count the active SGX users */
> > +static int sgx_usage_count;
> >
>
> [...]
>
> > + * Return:
> > + * %0: - Success or not supported
> > + * %-EAGAIN: - Can be safely retried, failure is due to lack of
> > + * entropy in RNG
> > + * %-EIO: - Unexpected error, retries are not advisable
> > + */
>
> This time I actually downloaded those patches and applied to my local, and
> I found the descriptions of the error codes are not vertically aligned.
>
> Please fix (and it's sad we still need to fix this type of thing in v13).
Sorry about this, it seems like I need to change the editor, which keeps
reverting
this on rebases ((
>
> Nit: as said before, the k-doc comment doc says:
>
> .. in order to produce the desired line breaks, you need to use a ReST
> list, e. g.:
>
> * Return:
> * * %0 - OK to runtime suspend the device
> * * %-EBUSY - Device should not be runtime suspended
>
> (hint: there's an additional '*' before the '%'.)
>
> But I guess it's just a nit but not a blocker.
Yes, I did go and check the kdoc style, but missed the additional '*'.
Will fix.
>
> > +static int __maybe_unused sgx_update_svn(void)
> > +{
> > + int ret;
> > +
> > + /*
> > + * If EUPDATESVN is not available, it is ok to
> > + * silently skip it to comply with legacy behavior.
> > + */
> > + if (!cpu_feature_enabled(X86_FEATURE_SGX_EUPDATESVN))
> > + return 0;
> > +
> > + /*
> > + * EPC is guaranteed to be empty when there are no users.
> > + * Ensure we are on our first user before proceeding further.
> > + */
> > + WARN(sgx_usage_count != 1, "Elevated usage count when calling
> EUPDATESVN\n");
>
> It seems you are obsessed to use "!= 1", rather than "!= 0".
>
> IIUC, Dave suggested the latter [*]:
>
> /* EPC is guaranteed to be empty when there are no users: */
> WARN(count, "Elevated usage count...");
>
> .. which is my natural response too.
>
> And the odd is I actually need to look at the next patch to see why "!= 1"
> is used.
I can change it to this version given that I change the code in the previous
patch.
Thank you very much for your prompt review!
Best Regards,
Elena.
