Re: [PATCH] x86/vdso: Flatten and clean up struct sgx_enclave_run

[Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>]

On Fri, Oct 02, 2020 at 05:27:56PM +0300, Jarkko Sakkinen wrote:
> On Fri, Oct 02, 2020 at 07:25:05AM +0300, Jarkko Sakkinen wrote:
> > On Fri, Oct 02, 2020 at 12:46:11AM +0300, Jarkko Sakkinen wrote:
> > > > > The documentation, selftest and the kernel source code use the terms "user
> > > > > handler" and "exit handler" in somewhat inconsistent manner. Start using
> > > > > "AEX handler" consistently as this is the term that Intel SDM volume D uses
> > > > > for these events.
> > > > 
> > > > No, AEX is specifically for "asynchronus" exits.  In this context, that is
> > > > limited to the exception path.  The vDSO invokes the user handler for EEXIT,
> > > > i.e. the synchrous path, which is not an AEX.
> > > 
> > > OK, I but this one.
> > > 
> > > I got gray hairs because:
> > > 
> > > - API called it user handler.
> > > - Documentation called it exit handler.
> > > - Selftest called it exit handler.
> > > 
> > > I think user_handler is too generic and abstract but I'm happy to hear
> > > other proposals than 'exit_' to which I will change given your feedback.
> > 
> > OK, so here I use my "too generic" argument pick user handler anyhow
> > because we don't know what the heck the handler is doing.
> > 
> > 'exit_handler' implies that not only it would be called for enclave
> > exit resulting from an exception, but also for EEXIT path.
> 
> Wile editing the commit message I read what I had written about flags.
> It was quite terrible, to say the least.
> 
> I'll send update soon.

When I added validation, I noticed that the offsets were wrong. Fixing
this but I think this is also one good reason for always validate the
full struct and not just some flags-field. It helps to catch bugs early
on [*].

Not going to do selftest improvements before sending v39 but the
selftest needs even more exception test than provision. I think I work
on both of these after the patch set has been sent.

For exception, wouldn't be easiest to add a 2nd TCS and code path that
trigger's #PF / EPCM conflict? I could add a write operation on a
read-only page.

[*] Proudly admitting that in v1 of this patch everything was wrong
    as far as flags-field is considered: no flags, no padding
    validation and no legit reasoning.

/Jarkko