Re: [RFC PATCH v5 08/26] x86/sgx: Expose SGX architectural definitions to the kernel

Borislav Petkov
2021-02-16T18:48:19Z

On Tue, Feb 16, 2021 at 07:18:27AM -0800, Dave Hansen wrote:
> On 2/16/21 3:48 AM, Borislav Petkov wrote:
> > What I'm trying to point you at is, to not give some artificial reasons
> > why the headers should be separate - artificial as the SDM says it
> > is architectural and so on - but give a reason from software design
> > perspective why the separation is needed: better build times, less
> > symbols exposed to modules, blabla and so on.
>
> I think I actually suggested this sgx_arch.h split for SGX in the first
> place.
>
> I was reading the patches and I had a really hard time separating the
> hardware and software structures. There would be a 'struct sgx_foo {}'
> and some chit chat about what it did... and I still had no idea if it
> was an architectural structure or not.
>
> This way, it's 100% crystal clear what Linux is defining and what the
> hardware defines from the diff context.

Yap, that's a valid reason in my book. And arch.h has at the top:

* Contains data structures defined by the SGX architecture. Data structures
* defined by the Linux software stack should not be placed here.

and by now we already have:

$ git ls-files | grep \/sgx.h
arch/x86/include/uapi/asm/sgx.h
arch/x86/kernel/cpu/sgx/sgx.h

two sgx.h headers.

So how about we have a single

arch/x86/include/asm/sgx.h

header which contains the architectural definitions at the *top* and at
their end, there'll be a:

/* Do not put any hardware-defined SGX structure representations below this line! */

and after that line begin the other, software definitions?

Then that arch/x86/kernel/cpu/sgx/sgx.h can be renamed to private.h
because it is included only there so you'll have:

arch/x86/include/uapi/asm/sgx.h - user API crap
arch/x86/include/asm/sgx.h - shared with other kernel facilities
arch/x86/kernel/cpu/sgx/internal.h - SGX-internal definitions

How does that look?

And we do have similar header structure with other features...

Thx.

--
Regards/Gruss,
Boris.