Re: [PATCH v4 01/18] nitro_enclaves: Add ioctl interface definition

[Paraschiv, Andra-Irina]

On 02/07/2020 18:24, Alexander Graf wrote:



On 22.06.20 22:03, 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.

Signed-off-by: Alexandru Vasile 
Signed-off-by: Andra Paraschiv 


Reviewed-by: Alexander Graf 


Added. Thanks for reviewing the group of patches so far.

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.

Re: [PATCH v4 01/18] nitro_enclaves: Add ioctl interface definition

[Alexander Graf]

On 22.06.20 22:03, 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.

Signed-off-by: Alexandru Vasile 
Signed-off-by: Andra Paraschiv 


Reviewed-by: Alexander Graf 


Alex



Amazon Development Center Germany GmbH
Krausenstr. 38
10117 Berlin
Geschaeftsfuehrung: Christian Schlaeger, Jonathan Weiss
Eingetragen am Amtsgericht Charlottenburg unter HRB 149173 B
Sitz: Berlin
Ust-ID: DE 289 237 879

Re: [PATCH v4 01/18] nitro_enclaves: Add ioctl interface definition

[Paraschiv, Andra-Irina]

On 25/06/2020 16:29, Stefan Hajnoczi wrote:

On Wed, Jun 24, 2020 at 05:02:54PM +0300, Paraschiv, Andra-Irina wrote:

On 23/06/2020 11:56, Stefan Hajnoczi wrote:

On Mon, Jun 22, 2020 at 11:03:12PM +0300, Andra Paraschiv wrote:

+/* User memory region flags */
+
+/* Memory region for enclave general usage. */
+#define NE_DEFAULT_MEMORY_REGION (0x00)
+
+/* Memory region to be set for an enclave (write). */
+struct ne_user_memory_region {
+   /**
+* Flags to determine the usage for the memory region (write).
+*/
+   __u64 flags;

Where is the write flag defined?

I guess it's supposed to be:

#define NE_USER_MEMORY_REGION_FLAG_WRITE (0x01)

For now, the flags field is included in the NE ioctl interface for
extensions, it is not part of the NE PCI device interface yet.

...

Ah, and just as a note, that "read" / "write" in parentheses means that a
certain data structure / field is read / written by user space. I updated to
use "in" / "out" instead of "read" / "write" in v5.

Oops, I got confused. I thought "(write)" was an example of a flag that
can be set on the memory region. Now I realize "write" means this field
is an input to the ioctl. :)

Thanks for updating the docs.


I was thinking this may be the case. :) Should be less confusing now, 
with the "in / out" updates.


Thanks also for feedback.

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.

Re: [PATCH v4 01/18] nitro_enclaves: Add ioctl interface definition

[Stefan Hajnoczi]

On Wed, Jun 24, 2020 at 05:02:54PM +0300, Paraschiv, Andra-Irina wrote:
> On 23/06/2020 11:56, Stefan Hajnoczi wrote:
> > On Mon, Jun 22, 2020 at 11:03:12PM +0300, Andra Paraschiv wrote:
> > > +/* User memory region flags */
> > > +
> > > +/* Memory region for enclave general usage. */
> > > +#define NE_DEFAULT_MEMORY_REGION (0x00)
> > > +
> > > +/* Memory region to be set for an enclave (write). */
> > > +struct ne_user_memory_region {
> > > + /**
> > > +  * Flags to determine the usage for the memory region (write).
> > > +  */
> > > + __u64 flags;
> > Where is the write flag defined?
> > 
> > I guess it's supposed to be:
> > 
> >#define NE_USER_MEMORY_REGION_FLAG_WRITE (0x01)
> 
> For now, the flags field is included in the NE ioctl interface for
> extensions, it is not part of the NE PCI device interface yet.
...
> Ah, and just as a note, that "read" / "write" in parentheses means that a
> certain data structure / field is read / written by user space. I updated to
> use "in" / "out" instead of "read" / "write" in v5.

Oops, I got confused. I thought "(write)" was an example of a flag that
can be set on the memory region. Now I realize "write" means this field
is an input to the ioctl. :)

Thanks for updating the docs.

Stefan


signature.asc
Description: PGP signature

Re: [PATCH v4 01/18] nitro_enclaves: Add ioctl interface definition

[Paraschiv, Andra-Irina]

On 23/06/2020 11:56, Stefan Hajnoczi wrote:

On Mon, Jun 22, 2020 at 11:03:12PM +0300, Andra Paraschiv wrote:

diff --git a/include/uapi/linux/nitro_enclaves.h 
b/include/uapi/linux/nitro_enclaves.h
new file mode 100644
index ..3270eb939a97
--- /dev/null
+++ b/include/uapi/linux/nitro_enclaves.h
@@ -0,0 +1,137 @@
+/* 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 
+
+/* Nitro Enclaves (NE) Kernel Driver Interface */
+
+#define NE_API_VERSION (1)
+
+/**
+ * The command is used to get the version of the NE API. This way the user 
space
+ * processes can be aware of the feature sets provided by the NE kernel driver.
+ *
+ * The NE API version is returned as result of this ioctl call.
+ */
+#define NE_GET_API_VERSION _IO(0xAE, 0x20)
+
+/**
+ * The command is used to create a slot that is associated with an enclave VM.
+ *
+ * The generated unique slot id is a read parameter of this command. An enclave
+ * file descriptor is returned as result of this ioctl call. The enclave fd can
+ * be further used with ioctl calls to set vCPUs and memory regions, then start
+ * the enclave.
+ */
+#define NE_CREATE_VM _IOR(0xAE, 0x21, __u64)

Information that would be useful for the ioctls:

1. Which fd the ioctl must be invoked on (/dev/nitro-enclaves, enclave fd, vCPU 
fd)

2. Errnos and their meanings

3. Which state(s) the ioctls may be invoked in (e.g. enclave 
created/started/etc)


I'll include this info in v5. Indeed, that's useful for the user space 
tooling that interacts with the kernel driver, in addition to the code 
review itself and future refs, to understand how it works.





+/* User memory region flags */
+
+/* Memory region for enclave general usage. */
+#define NE_DEFAULT_MEMORY_REGION (0x00)
+
+/* Memory region to be set for an enclave (write). */
+struct ne_user_memory_region {
+   /**
+* Flags to determine the usage for the memory region (write).
+*/
+   __u64 flags;

Where is the write flag defined?

I guess it's supposed to be:

   #define NE_USER_MEMORY_REGION_FLAG_WRITE (0x01)


For now, the flags field is included in the NE ioctl interface for 
extensions, it is not part of the NE PCI device interface yet.


The enclave image is copied into enclave memory before the enclave 
memory is carved out of the primary / parent VM. After carving it out 
(when the command request to add memory is sent to the PCI device and it 
is successfully completed), there will be faults if the enclave memory 
is written from the primary / parent VM.


Ah, and just as a note, that "read" / "write" in parentheses means that 
a certain data structure / field is read / written by user space. I 
updated to use "in" / "out" instead of "read" / "write" in v5.


Thank you.

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.

Re: [PATCH v4 01/18] nitro_enclaves: Add ioctl interface definition

[Stefan Hajnoczi]

On Mon, Jun 22, 2020 at 11:03:12PM +0300, Andra Paraschiv wrote:
> diff --git a/include/uapi/linux/nitro_enclaves.h 
> b/include/uapi/linux/nitro_enclaves.h
> new file mode 100644
> index ..3270eb939a97
> --- /dev/null
> +++ b/include/uapi/linux/nitro_enclaves.h
> @@ -0,0 +1,137 @@
> +/* 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 
> +
> +/* Nitro Enclaves (NE) Kernel Driver Interface */
> +
> +#define NE_API_VERSION (1)
> +
> +/**
> + * The command is used to get the version of the NE API. This way the user 
> space
> + * processes can be aware of the feature sets provided by the NE kernel 
> driver.
> + *
> + * The NE API version is returned as result of this ioctl call.
> + */
> +#define NE_GET_API_VERSION _IO(0xAE, 0x20)
> +
> +/**
> + * The command is used to create a slot that is associated with an enclave 
> VM.
> + *
> + * The generated unique slot id is a read parameter of this command. An 
> enclave
> + * file descriptor is returned as result of this ioctl call. The enclave fd 
> can
> + * be further used with ioctl calls to set vCPUs and memory regions, then 
> start
> + * the enclave.
> + */
> +#define NE_CREATE_VM _IOR(0xAE, 0x21, __u64)

Information that would be useful for the ioctls:

1. Which fd the ioctl must be invoked on (/dev/nitro-enclaves, enclave fd, vCPU 
fd)

2. Errnos and their meanings

3. Which state(s) the ioctls may be invoked in (e.g. enclave 
created/started/etc)

> +/* User memory region flags */
> +
> +/* Memory region for enclave general usage. */
> +#define NE_DEFAULT_MEMORY_REGION (0x00)
> +
> +/* Memory region to be set for an enclave (write). */
> +struct ne_user_memory_region {
> + /**
> +  * Flags to determine the usage for the memory region (write).
> +  */
> + __u64 flags;

Where is the write flag defined?

I guess it's supposed to be:

  #define NE_USER_MEMORY_REGION_FLAG_WRITE (0x01)


signature.asc
Description: PGP signature

[PATCH v4 01/18] nitro_enclaves: Add ioctl interface definition

[Andra Paraschiv]

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.

Signed-off-by: Alexandru Vasile 
Signed-off-by: Andra Paraschiv 
---
Changelog

v3 -> v4

* Decouple NE ioctl interface from KVM API.
* Add NE API version and the corresponding ioctl call.
* Add enclave / image load flags options.

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   | 137 ++
 3 files changed, 152 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 59472cd6a11d..783440c6719b 100644
--- a/Documentation/userspace-api/ioctl/ioctl-number.rst
+++ b/Documentation/userspace-api/ioctl/ioctl-number.rst
@@ -328,8 +328,11 @@ Code  Seq#Include File 
  Comments
 0xAC  00-1F  linux/raw.h
 0xAD  00 Netfilter 
device in development:
  

-0xAE  alllinux/kvm.h 
Kernel-based Virtual Machine
+0xAE  00-1F  linux/kvm.h 
Kernel-based Virtual Machine
  

+0xAE  40-FF  linux/kvm.h 
Kernel-based Virtual Machine
+ 

+0xAE  20-3F  linux/nitro_enclaves.h  Nitro 
Enclaves
 0xAF  00-1F  linux/fsl_hypervisor.h  Freescale 
hypervisor
 0xB0  allRATIO 
devices in development:
  

diff --git a/include/linux/nitro_enclaves.h b/include/linux/nitro_enclaves.h
new file mode 100644
index ..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 
+
+#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 ..3270eb939a97
--- /dev/null
+++ b/include/uapi/linux/nitro_enclaves.h
@@ -0,0 +1,137 @@
+/* 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 
+
+/* Nitro Enclaves (NE) Kernel Driver Interface */
+
+#define NE_API_VERSION (1)
+
+/**
+ * The command is used to get the version of the NE API. This way the user 
space
+ * processes can be aware of the feature sets provided by the NE kernel driver.
+ *
+ * The NE API version is returned as result of this ioctl call.
+ */
+#define NE_GET_API_VERSION _IO(0xAE, 0x20)
+
+/**
+ * The command is used to create a slot that is associated with an enclave VM.
+ *
+ * The generated unique slot id is a read parameter of this command. An enclave
+ * file descriptor is returned as result of this ioctl call. The enclave fd can
+ * be further used with ioctl calls to set vCPUs and memory regions, then start
+ * the enclave.
+ */
+#define NE_CREATE_VM _IOR(0xAE, 0x21, __u64)
+
+/**
+ * The command is used to set a vCPU for an enclave. A CPU pool needs to be set
+ * for enclave usage, before calling this function. CPU 0 and its siblings need
+ * to remain available for the primary / parent VM, so they cannot be set for
+ * an enclave.
+ *
+ * The vCPU id is a write / read parameter. If its value is 0, then a CPU is
+ * chosen from the enclave CPU pool and returned via this parameter. A vCPU 
file
+ * descriptor is returned as