Re: [PATCH v3 01/18] nitro_enclaves: Add ioctl interface definition
From: Paraschiv, Andra-Irina
Date: Thu May 28 2020 - 14:05:37 EST
On 27/05/2020 11:49, Stefan Hajnoczi wrote:
On Tue, May 26, 2020 at 01:13:17AM +0300, Andra Paraschiv wrote:
The Nitro Enclaves driver handles the enclave lifetime management. This
includes enclave creation, termination and setting up its resources such
as memory and CPU.
An enclave runs alongside the VM that spawned it. It is abstracted as a
process running in the VM that launched it. The process interacts with
the NE driver, that exposes an ioctl interface for creating an enclave
and setting up its resources.
Include part of the KVM ioctls in the provided ioctl interface, with
additional NE ioctl commands that e.g. triggers the enclave run.
Signed-off-by: Alexandru Vasile <lexnv@xxxxxxxxxx>
Signed-off-by: Andra Paraschiv <andraprs@xxxxxxxxxx>
---
Changelog
v2 -> v3
* Remove the GPL additional wording as SPDX-License-Identifier is already in
place.
v1 -> v2
* Add ioctl for getting enclave image load metadata.
* Update NE_ENCLAVE_START ioctl name to NE_START_ENCLAVE.
* Add entry in Documentation/userspace-api/ioctl/ioctl-number.rst for NE ioctls.
* Update NE ioctls definition based on the updated ioctl range for major and
minor.
---
.../userspace-api/ioctl/ioctl-number.rst | 5 +-
include/linux/nitro_enclaves.h | 11 ++++
include/uapi/linux/nitro_enclaves.h | 65 +++++++++++++++++++
3 files changed, 80 insertions(+), 1 deletion(-)
create mode 100644 include/linux/nitro_enclaves.h
create mode 100644 include/uapi/linux/nitro_enclaves.h
diff --git a/Documentation/userspace-api/ioctl/ioctl-number.rst b/Documentation/userspace-api/ioctl/ioctl-number.rst
index f759edafd938..8a19b5e871d3 100644
--- a/Documentation/userspace-api/ioctl/ioctl-number.rst
+++ b/Documentation/userspace-api/ioctl/ioctl-number.rst
@@ -325,8 +325,11 @@ Code Seq# Include File Comments
0xAC 00-1F linux/raw.h
0xAD 00 Netfilter device in development:
<mailto:rusty@xxxxxxxxxxxxxxx>
-0xAE all linux/kvm.h Kernel-based Virtual Machine
+0xAE 00-1F linux/kvm.h Kernel-based Virtual Machine
<mailto:kvm@xxxxxxxxxxxxxxx>
+0xAE 40-FF linux/kvm.h Kernel-based Virtual Machine
+ <mailto:kvm@xxxxxxxxxxxxxxx>
+0xAE 20-3F linux/nitro_enclaves.h Nitro Enclaves
0xAF 00-1F linux/fsl_hypervisor.h Freescale hypervisor
0xB0 all RATIO devices in development:
<mailto:vgo@xxxxxxxx>
Reusing KVM ioctls seems a little hacky. Even the ioctls that are used
by this driver don't use all the fields or behave in the same way as
kvm.ko.
For example, the memory regions slot number is not used by Nitro
Enclaves.
It would be cleaner to define NE-specific ioctls instead.
Indeed, a couple of fields / parameters are not used during the KVM
ioctl calls for now.
Will adapt the logic to follow a similar model of creating VMs and
adding resources, with NE ioctls.
diff --git a/include/linux/nitro_enclaves.h b/include/linux/nitro_enclaves.h
new file mode 100644
index 000000000000..d91ef2bfdf47
--- /dev/null
+++ b/include/linux/nitro_enclaves.h
@@ -0,0 +1,11 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Copyright 2020 Amazon.com, Inc. or its affiliates. All Rights Reserved.
+ */
+
+#ifndef _LINUX_NITRO_ENCLAVES_H_
+#define _LINUX_NITRO_ENCLAVES_H_
+
+#include <uapi/linux/nitro_enclaves.h>
+
+#endif /* _LINUX_NITRO_ENCLAVES_H_ */
diff --git a/include/uapi/linux/nitro_enclaves.h b/include/uapi/linux/nitro_enclaves.h
new file mode 100644
index 000000000000..3413352baf32
--- /dev/null
+++ b/include/uapi/linux/nitro_enclaves.h
@@ -0,0 +1,65 @@
+/* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */
+/*
+ * Copyright 2020 Amazon.com, Inc. or its affiliates. All Rights Reserved.
+ */
+
+#ifndef _UAPI_LINUX_NITRO_ENCLAVES_H_
+#define _UAPI_LINUX_NITRO_ENCLAVES_H_
+
+#include <linux/kvm.h>
+#include <linux/types.h>
+
+/* Nitro Enclaves (NE) Kernel Driver Interface */
+
+/**
+ * The command is used to get information needed for in-memory enclave image
+ * loading e.g. offset in enclave memory to start placing the enclave image.
+ *
+ * The image load metadata is an in / out data structure. It includes info
+ * provided by the caller - flags - and returns the offset in enclave memory
+ * where to start placing the enclave image.
+ */
+#define NE_GET_IMAGE_LOAD_METADATA _IOWR(0xAE, 0x20, struct image_load_metadata)
+
+/**
+ * The command is used to trigger enclave start after the enclave resources,
+ * such as memory and CPU, have been set.
+ *
+ * The enclave start metadata is an in / out data structure. It includes info
+ * provided by the caller - enclave cid and flags - and returns the slot uid
+ * and the cid (if input cid is 0).
+ */
+#define NE_START_ENCLAVE _IOWR(0xAE, 0x21, struct enclave_start_metadata)
+
image_load_metadata->flags and enclave_start_metadata->flags constants
are missing.
I added them now in this file. Thank you.
+/* Metadata necessary for in-memory enclave image loading. */
+struct image_load_metadata {
+ /**
+ * Flags to determine the enclave image type e.g. Enclave Image Format
+ * (EIF) (in).
+ */
+ __u64 flags;
+
+ /**
+ * Offset in enclave memory where to start placing the enclave image
+ * (out).
+ */
+ __u64 memory_offset;
+};
What about feature bits or a API version number field? If you add
features to the NE driver, how will userspace detect them?
Even if you intend to always compile userspace against the exact kernel
headers that the program will run on, it can still be useful to have an
API version for informational purposes and to easily prevent user
errors (running a new userspace binary on an old kernel where the API is
different).
Good point. I'll add a get API version ioctl for this purpose.
Finally, reserved struct fields may come in handy in the future. That
way userspace and the kernel don't need to explicitly handle multiple
struct sizes.
True, I've seen this pattern of including reserved fields e.g. in a
couple of KVM data structures.
Thanks for feedback, Stefan.
Andra
Amazon Development Center (Romania) S.R.L. registered office: 27A Sf. Lazar Street, UBC5, floor 2, Iasi, Iasi County, 700045, Romania. Registered in Romania. Registration number J22/2621/2005.