Re: [PATCH v20 15/28] x86/sgx: Add the Linux SGX Enclave Driver

[Sean Christopherson]

On Tue, Apr 23, 2019 at 11:29:24PM +0000, Jethro Beekman wrote:
> On 2019-04-22 14:58, Sean Christopherson wrote:
> >+Cc Jethro
> >
> >On Wed, Apr 17, 2019 at 01:39:25PM +0300, Jarkko Sakkinen wrote:
> >>Intel Software Guard eXtensions (SGX) is a set of CPU instructions that
> >>can be used by applications to set aside private regions of code and
> >>data. The code outside the enclave is disallowed to access the memory
> >>inside the enclave by the CPU access control.
> >>
> >>This commit adds the Linux SGX Enclave Driver that provides an ioctl API
> >>to manage enclaves. The address range for an enclave, commonly referred
> >>as ELRANGE in the documentation (e.g. Intel SDM), is reserved with
> >>mmap() against /dev/sgx/enclave. After that a set ioctls is used to
> >>build the enclave to the ELRANGE.
> >>
> >>Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@xxxxxxxxxxxxxxx>
> >>Co-developed-by: Sean Christopherson <sean.j.christopherson@xxxxxxxxx>
> >>Signed-off-by: Sean Christopherson <sean.j.christopherson@xxxxxxxxx>
> >>Co-developed-by: Serge Ayoun <serge.ayoun@xxxxxxxxx>
> >>Signed-off-by: Serge Ayoun <serge.ayoun@xxxxxxxxx>
> >>Co-developed-by: Shay Katz-zamir <shay.katz-zamir@xxxxxxxxx>
> >>Signed-off-by: Shay Katz-zamir <shay.katz-zamir@xxxxxxxxx>
> >>Co-developed-by: Suresh Siddha <suresh.b.siddha@xxxxxxxxx>
> >>Signed-off-by: Suresh Siddha <suresh.b.siddha@xxxxxxxxx>
> >>---
> >
> >...
> >
> >>+#ifdef CONFIG_ACPI
> >>+static struct acpi_device_id sgx_device_ids[] = {
> >>+	{"INT0E0C", 0},
> >>+	{"", 0},
> >>+};
> >>+MODULE_DEVICE_TABLE(acpi, sgx_device_ids);
> >>+#endif
> >>+
> >>+static struct platform_driver sgx_drv = {
> >>+	.probe = sgx_drv_probe,
> >>+	.remove = sgx_drv_remove,
> >>+	.driver = {
> >>+		.name			= "sgx",
> >>+		.acpi_match_table	= ACPI_PTR(sgx_device_ids),
> >>+	},
> >>+};
> >
> >Where do we stand on removing the ACPI and platform_driver dependencies?
> >Can we get rid of them sooner rather than later?
> 
> You know my position on this...
> https://www.spinics.net/lists/linux-sgx/msg00624.html . I don't really have
> any new arguments.
> 
> Considering the amount of planned changes for the driver post-merge, I think
> it's crucial that the driver part can be swapped out with alternative
> implementations.

This gets far outside of my area of expertise as I think this is more of
a policy question as opposed to a technical question, e.g. do we export
function simply to allow out-of-tree alternatives.

> >Now that the core SGX code is approaching stability, I'd like to start
> >sending RFCs for the EPC virtualization and KVM bits to hash out that side
> >of things.  The ACPI crud is the last chunk of code that would require
> >non-trivial changes to the core SGX code for the proposed virtualization
> >implementation.  I'd strongly prefer to get it out of the way before
> >sending the KVM RFCs.
> 
> What kind of changes? Wouldn't KVM just be another consumer of the same API
> used by the driver?

Nope, userspace "only" needs to be able to mmap() arbitrary chunks of EPC.
Except for EPC management, which is already in built into the kernel, the
EPC virtualization code has effectively zero overlap with the driver.  Of
course this is all technically speculative since none of this is upstream...