On 8/26/2020 1:15 PM, Sean Christopherson wrote:
On Wed, Aug 26, 2020 at 12:27:54PM -0700, Xing, Cedric wrote:
On 8/17/2020 9:24 PM, Sean Christopherson wrote:
Rework __vdso_sgx_enter_enclave() to use a struct to hold the input and
output params. In the new struct, add an opaque "user_data" that can be
used to pass context across the vDSO, and an explicit "exit_reason" to
avoid overloading the return value.
In order to pass additional parameters to the exit handler, the exinfo
structure could be embedded in a user-defined structure while the handler
could pick it up using "container_of" macro. IMO the original interface was
neat and suffcient, and we are over-engineering it.
container_of/offsetof shenanigans were my original suggestion as well.
Nathaniel's argument is that using such tricks is less than pleasent in a
Rust environment.
https://lkml.kernel.org/r/CAOASepOFh-vOrNZEVDFrDSuHs+9GEzzpXUTG-fZMuyjWAkpRWw@xxxxxxxxxxxxxx
I don't know much about rust but there seem to be offsetof/container_of
solutions in rust. Here's one:
https://gist.github.com/resilar/baefbfbf0d733c69d70970d829938875.
After countless discussions along this upstream journey, I thought we
had agreed that simplicity was the priority. And for simplicity we were
even willing to give up compliance to x86_64 ABI. Then how do you
justify treating rust better than other programming languages? This
looks a hard sell to me.
Moving the params into a struct will also make it less painful to use
dedicated exit reasons, and to support exiting on interrupts in future
patches.
My apology for not following discussion lately. What did you mean by
"dedicated exit reasons" and why was it "painful"? According to another
thread, I guess we wouldn't have "exiting on interrupts".
Currently the vDSO returns -EFAULT if the enclave encounters an exception,
which is kludgy for two reasons:
1. EFAULT traditionally means "bad address", whereas an exception could be
a #UD in the enclave.
2. Returning -EFAULT provides weird semantics as it implies the vDSO call
failed, which is not the case. The vDSO itself was successful, i.e. it
did the requested EENTER/ERESUME operation, and should really return '0'.
EFAULT is descriptive enough for me. And more importantly
detailed/specific info is always available in exinfo.
IMO, the purpose of return code is for the caller/handler to branch on.
If 2 cases share the same execution path, then there's no need to
distinguish them. In the case of SGX, most (if not all) exceptions are
handled in the same way - nested EENTER, then what's point of
distinguishing them? Please note that detailed info is always available
in exinfo to cover corner cases.
Moreover, even the vDSO API itself cannot tell between faults at
EENTER/ERESUME (the vDSO itself failes) vs. faults inside the enclave
(vDSO succeeded but the enclave fails). I don't think you can be
accurate without complicating the code significantly.
The proposed solution for #1 is to define arbitrary return values to
differentiate between synchronous (EEXIT) exits and asynchronous exits due
to exceptions. But that doesn't address #2, and gets especially awkward when
dealing with the call back return, which is also overloaded to use positive
return values for ENCLU leafs.
Passing a "run" struct makes it trivially easy to different the return value
of the vDSO function from the enclave exit reason, and to avoid collisions
with the return value from the userspace callback.
When a callback is configured, the vDSO NEVER returns to caller directly
- i.e. all return codes must be from the callback but NOT the vDSO. I
don't see any collisions here.
