Re: [RFC PATCH v2 11/26] x86/sgx: Add encls_faulted() helper

Jarkko Sakkinen
2021-01-21T01:11:19Z

On Thu, Jan 21, 2021 at 12:43:59PM +1300, Kai Huang wrote:
> On Wed, 20 Jan 2021 14:03:08 +0200 Jarkko Sakkinen wrote:
> > On Mon, Jan 18, 2021 at 04:28:04PM +1300, Kai Huang wrote:
> > > From: Sean Christopherson <sean.j.christopherson@intel.com>
> > >
> > > Add a helper to extract the fault indicator from an encoded ENCLS return
> > > value. SGX virtualization will also need to detect ENCLS faults.
> > >
> > > Signed-off-by: Sean Christopherson <sean.j.christopherson@intel.com>
> > > Acked-by: Dave Hansen <dave.hansen@intel.com>
> > > Signed-off-by: Kai Huang <kai.huang@intel.com>
> > > ---
> > > arch/x86/kernel/cpu/sgx/encls.h | 14 +++++++++++++-
> > > arch/x86/kernel/cpu/sgx/ioctl.c | 2 +-
> > > 2 files changed, 14 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/arch/x86/kernel/cpu/sgx/encls.h b/arch/x86/kernel/cpu/sgx/encls.h
> > > index be5c49689980..55919a2b01b0 100644
> > > --- a/arch/x86/kernel/cpu/sgx/encls.h
> > > +++ b/arch/x86/kernel/cpu/sgx/encls.h
> > > @@ -40,6 +40,18 @@
> > > } while (0); \
> > > }
> > >
> > > +/*
> > > + * encls_faulted() - Check if an ENCLS leaf faulted given an error code
> > > + * @ret the return value of an ENCLS leaf function call
> > > + *
> > > + * Return:
> > > + * %true if @ret indicates a fault, %false otherwise
> >
> > Follow here the style of commenting as in ioctl.c, for the return value.
> > It has optimal readability both as text, and also when converted to HTML.
> > See sgx_ioc_enclave_add_pages() for an example.
>
> You mean something like below?
>
> Return:
> - %true: @ret indicates a fault.
> - %false: @ret indicates no fault.

Yeah, with '\t' indentation. I'd remove also '%'. Also '@ret' is redudant.

To put this all together:

* Return:
* - true: ENCLS leaf faulted.
* - false: Otherwise.

I tried various ways and this was the best way to document return values
that i've found. It's easy to read as plain text, and also has the benefit
that return values get nicely lined up in htmldocs.

I've been even considering a patch for

https://www.kernel.org/doc/Documentation/kernel-doc-nano-HOWTO.txt

The only advice given ATM is: "Take a look around the source tree for
examples."

/Jarkko