Re: [RFC PATCH v2 4/4] x86/vdso: Add __vdso_sgx_enter_enclave() to wrap SGX enclave transitions

From: Andy Lutomirski <luto@kernel.org>
To: "Christopherson, Sean J" <sean.j.christopherson@intel.com>
Cc: Andrew Lutomirski <luto@kernel.org>,
	Thomas Gleixner <tglx@linutronix.de>,
	Ingo Molnar <mingo@redhat.com>, Borislav Petkov <bp@alien8.de>,
	X86 ML <x86@kernel.org>,
	Dave Hansen <dave.hansen@linux.intel.com>,
	Peter Zijlstra <peterz@infradead.org>,
	"H. Peter Anvin" <hpa@zytor.com>,
	LKML <linux-kernel@vger.kernel.org>,
	Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>,
	Josh Triplett <josh@joshtriplett.org>
Subject: Re: [RFC PATCH v2 4/4] x86/vdso: Add __vdso_sgx_enter_enclave() to wrap SGX enclave transitions
Date: Thu, 6 Dec 2018 14:50:01 -0800	[thread overview]
Message-ID: <CALCETrXRJ645=08fyeoMQ949fLB1TvhsgERFVx5mAHdViEjq8Q@mail.gmail.com> (raw)
In-Reply-To: <20181206221922.31012-5-sean.j.christopherson@intel.com>

On Thu, Dec 6, 2018 at 2:19 PM Sean Christopherson
<sean.j.christopherson@intel.com> wrote:
>
 +
> +       /*
> +        * Invoke the caller's exit handler if one was provided.  The return
> +        * value tells us whether to re-enter the enclave (EENTER or ERESUME)
> +        * or to return (EEXIT).
> +        */
> +       if (exit_handler) {
> +               leaf = exit_handler(exit_info, tcs, priv);
> +               if (leaf == SGX_EENTER || leaf == SGX_ERESUME)
> +                       goto enter_enclave;
> +               if (leaf == SGX_EEXIT)
> +                       return 0;
> +               return -EINVAL;
> +       } else if (leaf != SGX_EEXIT) {
> +               return -EFAULT;
> +       }

This still seems overcomplicated to me.  How about letting the
requested leaf (EENTER or ERESUME) be a parameter to the function and
then just returning here?  As it stands, you're requiring any ERESUME
that gets issued (other than the implicit ones) to be issued in the
same call stack, which is very awkward if you're doing something like
forwarding the fault to a different task over a socket and then
waiting in epoll_wait() or similar before resuming the enclave.

--Andy