[PATCH v3 2/5] machine/nitro-enclave: Add vhost-user-vsock device

Dorjoy Chowdhury posted 5 patches 3 months, 2 weeks ago
There is a newer version of this series
[PATCH v3 2/5] machine/nitro-enclave: Add vhost-user-vsock device
Posted by Dorjoy Chowdhury 3 months, 2 weeks ago
AWS Nitro Enclaves have built-in vhost-vsock device support which
enables applications in enclave VMs to communicate with the parent
EC2 VM over vsock. The enclave VMs have dynamic CID while the parent
always has CID 3. In QEMU, the vsock emulation for nitro enclave is
added using vhost-user-vsock as opposed to vhost-vsock. vhost-vsock
doesn't support sibling VM communication which is needed for nitro
enclaves.

In QEMU's nitro-enclave emulation, for the vsock communication to CID
3 to work, another process that does the vsock emulation in  userspace
must be run, for example, vhost-device-vsock[1] from rust-vmm, with
necessary vsock communication support in another guest VM with CID 3.
A new mandatory nitro-enclave machine option 'vsock' has been added.
The value for this option should be the chardev id from the '-chardev'
option for the vhost-user-vsock device to work.

Using vhost-user-vsock also enables the possibility to implement some
proxying support in the vhost-user-vsock daemon that will forward all
the packets to the host machine instead of CID 3 so that users of
nitro-enclave can run the necessary applications in their host machine
instead of running another whole VM with CID 3.

[1] https://github.com/rust-vmm/vhost-device/tree/main/vhost-device-vsock

Signed-off-by: Dorjoy Chowdhury <dorjoychy111@gmail.com>
---
 backends/hostmem-memfd.c        |   2 -
 hw/core/machine.c               |  71 +++++++++---------
 hw/i386/Kconfig                 |   1 +
 hw/i386/nitro_enclave.c         | 123 ++++++++++++++++++++++++++++++++
 include/hw/boards.h             |   2 +
 include/hw/i386/nitro_enclave.h |   8 +++
 include/sysemu/hostmem.h        |   2 +
 7 files changed, 174 insertions(+), 35 deletions(-)

diff --git a/backends/hostmem-memfd.c b/backends/hostmem-memfd.c
index 6a3c89a12b..9f890a813e 100644
--- a/backends/hostmem-memfd.c
+++ b/backends/hostmem-memfd.c
@@ -18,8 +18,6 @@
 #include "qapi/error.h"
 #include "qom/object.h"
 
-#define TYPE_MEMORY_BACKEND_MEMFD "memory-backend-memfd"
-
 OBJECT_DECLARE_SIMPLE_TYPE(HostMemoryBackendMemfd, MEMORY_BACKEND_MEMFD)
 
 
diff --git a/hw/core/machine.c b/hw/core/machine.c
index 27dcda0248..b4662b2795 100644
--- a/hw/core/machine.c
+++ b/hw/core/machine.c
@@ -998,6 +998,39 @@ void machine_add_audiodev_property(MachineClass *mc)
                                           "Audiodev to use for default machine devices");
 }
 
+static bool create_default_memdev(MachineState *ms, const char *path,
+                                  Error **errp)
+{
+    Object *obj;
+    MachineClass *mc = MACHINE_GET_CLASS(ms);
+    bool r = false;
+
+    obj = object_new(path ? TYPE_MEMORY_BACKEND_FILE : TYPE_MEMORY_BACKEND_RAM);
+    if (path) {
+        if (!object_property_set_str(obj, "mem-path", path, errp)) {
+            goto out;
+        }
+    }
+    if (!object_property_set_int(obj, "size", ms->ram_size, errp)) {
+        goto out;
+    }
+    object_property_add_child(object_get_objects_root(), mc->default_ram_id,
+                              obj);
+    /* Ensure backend's memory region name is equal to mc->default_ram_id */
+    if (!object_property_set_bool(obj, "x-use-canonical-path-for-ramblock-id",
+                             false, errp)) {
+        goto out;
+    }
+    if (!user_creatable_complete(USER_CREATABLE(obj), errp)) {
+        goto out;
+    }
+    r = object_property_set_link(OBJECT(ms), "memory-backend", obj, errp);
+
+out:
+    object_unref(obj);
+    return r;
+}
+
 static void machine_class_init(ObjectClass *oc, void *data)
 {
     MachineClass *mc = MACHINE_CLASS(oc);
@@ -1017,6 +1050,8 @@ static void machine_class_init(ObjectClass *oc, void *data)
      */
     mc->numa_mem_align_shift = 23;
 
+    mc->create_default_memdev = create_default_memdev;
+
     object_class_property_add_str(oc, "kernel",
         machine_get_kernel, machine_set_kernel);
     object_class_property_set_description(oc, "kernel",
@@ -1410,38 +1445,6 @@ MemoryRegion *machine_consume_memdev(MachineState *machine,
     return ret;
 }
 
-static bool create_default_memdev(MachineState *ms, const char *path, Error **errp)
-{
-    Object *obj;
-    MachineClass *mc = MACHINE_GET_CLASS(ms);
-    bool r = false;
-
-    obj = object_new(path ? TYPE_MEMORY_BACKEND_FILE : TYPE_MEMORY_BACKEND_RAM);
-    if (path) {
-        if (!object_property_set_str(obj, "mem-path", path, errp)) {
-            goto out;
-        }
-    }
-    if (!object_property_set_int(obj, "size", ms->ram_size, errp)) {
-        goto out;
-    }
-    object_property_add_child(object_get_objects_root(), mc->default_ram_id,
-                              obj);
-    /* Ensure backend's memory region name is equal to mc->default_ram_id */
-    if (!object_property_set_bool(obj, "x-use-canonical-path-for-ramblock-id",
-                             false, errp)) {
-        goto out;
-    }
-    if (!user_creatable_complete(USER_CREATABLE(obj), errp)) {
-        goto out;
-    }
-    r = object_property_set_link(OBJECT(ms), "memory-backend", obj, errp);
-
-out:
-    object_unref(obj);
-    return r;
-}
-
 const char *machine_class_default_cpu_type(MachineClass *mc)
 {
     if (mc->valid_cpu_types && !mc->valid_cpu_types[1]) {
@@ -1545,7 +1548,9 @@ void machine_run_board_init(MachineState *machine, const char *mem_path, Error *
                 machine_class->default_ram_id);
             return;
         }
-        if (!create_default_memdev(current_machine, mem_path, errp)) {
+
+        if (!machine_class->create_default_memdev(current_machine, mem_path,
+                                                  errp)) {
             return;
         }
     }
diff --git a/hw/i386/Kconfig b/hw/i386/Kconfig
index eba8eaa960..821532c4c8 100644
--- a/hw/i386/Kconfig
+++ b/hw/i386/Kconfig
@@ -132,6 +132,7 @@ config MICROVM
 config NITRO_ENCLAVE
     default y
     depends on MICROVM
+    select VHOST_USER_VSOCK
 
 config X86_IOMMU
     bool
diff --git a/hw/i386/nitro_enclave.c b/hw/i386/nitro_enclave.c
index 98690c6373..280ab4cc9b 100644
--- a/hw/i386/nitro_enclave.c
+++ b/hw/i386/nitro_enclave.c
@@ -11,11 +11,81 @@
 #include "qemu/osdep.h"
 #include "qemu/error-report.h"
 #include "qapi/error.h"
+#include "qom/object_interfaces.h"
 
+#include "chardev/char.h"
+#include "hw/sysbus.h"
 #include "hw/core/eif.h"
 #include "hw/i386/x86.h"
 #include "hw/i386/microvm.h"
 #include "hw/i386/nitro_enclave.h"
+#include "hw/virtio/virtio-mmio.h"
+#include "hw/virtio/vhost-user-vsock.h"
+#include "sysemu/hostmem.h"
+
+static BusState *find_free_virtio_mmio_bus(void)
+{
+    BusChild *kid;
+    BusState *bus = sysbus_get_default();
+
+    QTAILQ_FOREACH(kid, &bus->children, sibling) {
+        DeviceState *dev = kid->child;
+        if (object_dynamic_cast(OBJECT(dev), TYPE_VIRTIO_MMIO)) {
+            VirtIOMMIOProxy *mmio = VIRTIO_MMIO(OBJECT(dev));
+            VirtioBusState *mmio_virtio_bus = &mmio->bus;
+            BusState *mmio_bus = &mmio_virtio_bus->parent_obj;
+            if (QTAILQ_EMPTY(&mmio_bus->children)) {
+                return mmio_bus;
+            }
+        }
+    }
+
+    return NULL;
+}
+
+static void vhost_user_vsock_init(NitroEnclaveMachineState *nems)
+{
+    DeviceState *dev = qdev_new(TYPE_VHOST_USER_VSOCK);
+    VHostUserVSock *vsock = VHOST_USER_VSOCK(dev);
+    BusState *bus;
+
+    if (!nems->vsock) {
+        error_report("A valid chardev id for vhost-user-vsock device must be "
+                     "provided using the 'vsock' machine option");
+        exit(1);
+    }
+
+    bus = find_free_virtio_mmio_bus();
+    if (!bus) {
+        error_report("Failed to find bus for vhost-user-vsock device");
+        exit(1);
+    }
+
+    Chardev *chardev = qemu_chr_find(nems->vsock);
+    if (!chardev) {
+        error_report("Failed to find chardev with id %s", nems->vsock);
+        exit(1);
+    }
+
+    vsock->conf.chardev.chr = chardev;
+
+    qdev_realize_and_unref(dev, bus, &error_fatal);
+}
+
+static void nitro_enclave_devices_init(NitroEnclaveMachineState *nems)
+{
+    vhost_user_vsock_init(nems);
+}
+
+static void nitro_enclave_machine_state_init(MachineState *machine)
+{
+    NitroEnclaveMachineClass *ne_class =
+        NITRO_ENCLAVE_MACHINE_GET_CLASS(machine);
+    NitroEnclaveMachineState *ne_state = NITRO_ENCLAVE_MACHINE(machine);
+
+    ne_class->parent_init(machine);
+    nitro_enclave_devices_init(ne_state);
+}
 
 static void nitro_enclave_machine_initfn(Object *obj)
 {
@@ -66,15 +136,68 @@ static void x86_load_eif(X86MachineState *x86ms, FWCfgState *fw_cfg,
     return;
 }
 
+static bool create_memfd_backend(MachineState *ms, const char *path,
+                                 Error **errp)
+{
+    Object *obj;
+    MachineClass *mc = MACHINE_GET_CLASS(ms);
+    bool r = false;
+
+    obj = object_new(TYPE_MEMORY_BACKEND_MEMFD);
+    if (!object_property_set_int(obj, "size", ms->ram_size, errp)) {
+        goto out;
+    }
+    object_property_add_child(object_get_objects_root(), mc->default_ram_id,
+                              obj);
+
+    if (!user_creatable_complete(USER_CREATABLE(obj), errp)) {
+        goto out;
+    }
+    r = object_property_set_link(OBJECT(ms), "memory-backend", obj, errp);
+
+out:
+    object_unref(obj);
+    return r;
+}
+
+static char *nitro_enclave_get_vsock_chardev_id(Object *obj, Error **errp)
+{
+    NitroEnclaveMachineState *nems = NITRO_ENCLAVE_MACHINE(obj);
+
+    return g_strdup(nems->vsock);
+}
+
+static void nitro_enclave_set_vsock_chardev_id(Object *obj, const char *value,
+                                               Error **errp)
+{
+    NitroEnclaveMachineState *nems = NITRO_ENCLAVE_MACHINE(obj);
+
+    g_free(nems->vsock);
+    nems->vsock = g_strdup(value);
+}
+
 static void nitro_enclave_class_init(ObjectClass *oc, void *data)
 {
     MachineClass *mc = MACHINE_CLASS(oc);
     MicrovmMachineClass *mmc = MICROVM_MACHINE_CLASS(oc);
+    NitroEnclaveMachineClass *nemc = NITRO_ENCLAVE_MACHINE_CLASS(oc);
 
     mmc->x86_load_linux = x86_load_eif;
 
     mc->family = "nitro_enclave_i386";
     mc->desc = "AWS Nitro Enclave";
+
+    nemc->parent_init = mc->init;
+    mc->init = nitro_enclave_machine_state_init;
+
+    mc->create_default_memdev = create_memfd_backend;
+
+    object_class_property_add_str(oc, NITRO_ENCLAVE_VSOCK_CHARDEV_ID,
+                                  nitro_enclave_get_vsock_chardev_id,
+                                  nitro_enclave_set_vsock_chardev_id);
+    object_class_property_set_description(oc, NITRO_ENCLAVE_VSOCK_CHARDEV_ID,
+                                          "Set chardev id for vhost-user-vsock "
+                                          "device");
 }
 
 static const TypeInfo nitro_enclave_machine_info = {
diff --git a/include/hw/boards.h b/include/hw/boards.h
index 48ff6d8b93..c268e7f005 100644
--- a/include/hw/boards.h
+++ b/include/hw/boards.h
@@ -308,6 +308,8 @@ struct MachineClass {
     int64_t (*get_default_cpu_node_id)(const MachineState *ms, int idx);
     ram_addr_t (*fixup_ram_size)(ram_addr_t size);
     uint64_t smbios_memory_device_size;
+    bool (*create_default_memdev)(MachineState *ms, const char *path,
+                                  Error **errp);
 };
 
 /**
diff --git a/include/hw/i386/nitro_enclave.h b/include/hw/i386/nitro_enclave.h
index a1dada9371..3e302de851 100644
--- a/include/hw/i386/nitro_enclave.h
+++ b/include/hw/i386/nitro_enclave.h
@@ -14,12 +14,20 @@
 #include "hw/i386/microvm.h"
 #include "qom/object.h"
 
+/* Machine type options */
+#define NITRO_ENCLAVE_VSOCK_CHARDEV_ID "vsock"
+
 struct NitroEnclaveMachineClass {
     MicrovmMachineClass parent;
+
+    void (*parent_init)(MachineState *state);
 };
 
 struct NitroEnclaveMachineState {
     MicrovmMachineState parent;
+
+    /* Machine type options */
+    char *vsock;
 };
 
 #define TYPE_NITRO_ENCLAVE_MACHINE MACHINE_TYPE_NAME("nitro-enclave")
diff --git a/include/sysemu/hostmem.h b/include/sysemu/hostmem.h
index de47ae59e4..67f45abe39 100644
--- a/include/sysemu/hostmem.h
+++ b/include/sysemu/hostmem.h
@@ -39,6 +39,8 @@ OBJECT_DECLARE_TYPE(HostMemoryBackend, HostMemoryBackendClass,
  */
 #define TYPE_MEMORY_BACKEND_FILE "memory-backend-file"
 
+#define TYPE_MEMORY_BACKEND_MEMFD "memory-backend-memfd"
+
 
 /**
  * HostMemoryBackendClass:
-- 
2.39.2
Re: [PATCH v3 2/5] machine/nitro-enclave: Add vhost-user-vsock device
Posted by Daniel P. Berrangé 3 months, 1 week ago
On Sat, Aug 10, 2024 at 10:44:59PM +0600, Dorjoy Chowdhury wrote:
> AWS Nitro Enclaves have built-in vhost-vsock device support which
> enables applications in enclave VMs to communicate with the parent
> EC2 VM over vsock. The enclave VMs have dynamic CID while the parent
> always has CID 3. In QEMU, the vsock emulation for nitro enclave is
> added using vhost-user-vsock as opposed to vhost-vsock. vhost-vsock
> doesn't support sibling VM communication which is needed for nitro
> enclaves.
> 
> In QEMU's nitro-enclave emulation, for the vsock communication to CID
> 3 to work, another process that does the vsock emulation in  userspace
> must be run, for example, vhost-device-vsock[1] from rust-vmm, with
> necessary vsock communication support in another guest VM with CID 3.
> A new mandatory nitro-enclave machine option 'vsock' has been added.
> The value for this option should be the chardev id from the '-chardev'
> option for the vhost-user-vsock device to work.
> 
> Using vhost-user-vsock also enables the possibility to implement some
> proxying support in the vhost-user-vsock daemon that will forward all
> the packets to the host machine instead of CID 3 so that users of
> nitro-enclave can run the necessary applications in their host machine
> instead of running another whole VM with CID 3.
> 
> [1] https://github.com/rust-vmm/vhost-device/tree/main/vhost-device-vsock
> 
> Signed-off-by: Dorjoy Chowdhury <dorjoychy111@gmail.com>
> ---
>  backends/hostmem-memfd.c        |   2 -
>  hw/core/machine.c               |  71 +++++++++---------
>  hw/i386/Kconfig                 |   1 +
>  hw/i386/nitro_enclave.c         | 123 ++++++++++++++++++++++++++++++++
>  include/hw/boards.h             |   2 +
>  include/hw/i386/nitro_enclave.h |   8 +++
>  include/sysemu/hostmem.h        |   2 +
>  7 files changed, 174 insertions(+), 35 deletions(-)
> 

> diff --git a/hw/i386/nitro_enclave.c b/hw/i386/nitro_enclave.c
> index 98690c6373..280ab4cc9b 100644
> --- a/hw/i386/nitro_enclave.c
> +++ b/hw/i386/nitro_enclave.c
> @@ -11,11 +11,81 @@
>  #include "qemu/osdep.h"
>  #include "qemu/error-report.h"
>  #include "qapi/error.h"
> +#include "qom/object_interfaces.h"
>  
> +#include "chardev/char.h"
> +#include "hw/sysbus.h"
>  #include "hw/core/eif.h"
>  #include "hw/i386/x86.h"
>  #include "hw/i386/microvm.h"
>  #include "hw/i386/nitro_enclave.h"
> +#include "hw/virtio/virtio-mmio.h"
> +#include "hw/virtio/vhost-user-vsock.h"
> +#include "sysemu/hostmem.h"
> +
> +static BusState *find_free_virtio_mmio_bus(void)
> +{
> +    BusChild *kid;
> +    BusState *bus = sysbus_get_default();
> +
> +    QTAILQ_FOREACH(kid, &bus->children, sibling) {
> +        DeviceState *dev = kid->child;
> +        if (object_dynamic_cast(OBJECT(dev), TYPE_VIRTIO_MMIO)) {
> +            VirtIOMMIOProxy *mmio = VIRTIO_MMIO(OBJECT(dev));
> +            VirtioBusState *mmio_virtio_bus = &mmio->bus;
> +            BusState *mmio_bus = &mmio_virtio_bus->parent_obj;
> +            if (QTAILQ_EMPTY(&mmio_bus->children)) {
> +                return mmio_bus;
> +            }
> +        }
> +    }
> +
> +    return NULL;
> +}
> +
> +static void vhost_user_vsock_init(NitroEnclaveMachineState *nems)
> +{
> +    DeviceState *dev = qdev_new(TYPE_VHOST_USER_VSOCK);
> +    VHostUserVSock *vsock = VHOST_USER_VSOCK(dev);
> +    BusState *bus;
> +
> +    if (!nems->vsock) {
> +        error_report("A valid chardev id for vhost-user-vsock device must be "
> +                     "provided using the 'vsock' machine option");
> +        exit(1);
> +    }
> +
> +    bus = find_free_virtio_mmio_bus();
> +    if (!bus) {
> +        error_report("Failed to find bus for vhost-user-vsock device");
> +        exit(1);
> +    }
> +
> +    Chardev *chardev = qemu_chr_find(nems->vsock);
> +    if (!chardev) {
> +        error_report("Failed to find chardev with id %s", nems->vsock);
> +        exit(1);
> +    }
> +
> +    vsock->conf.chardev.chr = chardev;
> +
> +    qdev_realize_and_unref(dev, bus, &error_fatal);
> +}

Why does this machine need to create the vhost-user-vsock device itself ?
Doing it this way prevents the mgmt app from changing any of the other
vsock device settings beyond 'chardev'. The entity creating QEMU can use
-device to create the vsock device.


> +
> +static void nitro_enclave_devices_init(NitroEnclaveMachineState *nems)
> +{
> +    vhost_user_vsock_init(nems);
> +}
> +
> +static void nitro_enclave_machine_state_init(MachineState *machine)
> +{
> +    NitroEnclaveMachineClass *ne_class =
> +        NITRO_ENCLAVE_MACHINE_GET_CLASS(machine);
> +    NitroEnclaveMachineState *ne_state = NITRO_ENCLAVE_MACHINE(machine);
> +
> +    ne_class->parent_init(machine);
> +    nitro_enclave_devices_init(ne_state);
> +}
>  
>  static void nitro_enclave_machine_initfn(Object *obj)
>  {
> @@ -66,15 +136,68 @@ static void x86_load_eif(X86MachineState *x86ms, FWCfgState *fw_cfg,
>      return;
>  }
>  
> +static bool create_memfd_backend(MachineState *ms, const char *path,
> +                                 Error **errp)
> +{
> +    Object *obj;
> +    MachineClass *mc = MACHINE_GET_CLASS(ms);
> +    bool r = false;
> +
> +    obj = object_new(TYPE_MEMORY_BACKEND_MEMFD);
> +    if (!object_property_set_int(obj, "size", ms->ram_size, errp)) {
> +        goto out;
> +    }
> +    object_property_add_child(object_get_objects_root(), mc->default_ram_id,
> +                              obj);
> +
> +    if (!user_creatable_complete(USER_CREATABLE(obj), errp)) {
> +        goto out;
> +    }
> +    r = object_property_set_link(OBJECT(ms), "memory-backend", obj, errp);
> +
> +out:
> +    object_unref(obj);
> +    return r;
> +}
> +
> +static char *nitro_enclave_get_vsock_chardev_id(Object *obj, Error **errp)
> +{
> +    NitroEnclaveMachineState *nems = NITRO_ENCLAVE_MACHINE(obj);
> +
> +    return g_strdup(nems->vsock);
> +}
> +
> +static void nitro_enclave_set_vsock_chardev_id(Object *obj, const char *value,
> +                                               Error **errp)
> +{
> +    NitroEnclaveMachineState *nems = NITRO_ENCLAVE_MACHINE(obj);
> +
> +    g_free(nems->vsock);
> +    nems->vsock = g_strdup(value);
> +}
> +
>  static void nitro_enclave_class_init(ObjectClass *oc, void *data)
>  {
>      MachineClass *mc = MACHINE_CLASS(oc);
>      MicrovmMachineClass *mmc = MICROVM_MACHINE_CLASS(oc);
> +    NitroEnclaveMachineClass *nemc = NITRO_ENCLAVE_MACHINE_CLASS(oc);
>  
>      mmc->x86_load_linux = x86_load_eif;
>  
>      mc->family = "nitro_enclave_i386";
>      mc->desc = "AWS Nitro Enclave";
> +
> +    nemc->parent_init = mc->init;
> +    mc->init = nitro_enclave_machine_state_init;
> +
> +    mc->create_default_memdev = create_memfd_backend;
> +
> +    object_class_property_add_str(oc, NITRO_ENCLAVE_VSOCK_CHARDEV_ID,
> +                                  nitro_enclave_get_vsock_chardev_id,
> +                                  nitro_enclave_set_vsock_chardev_id);
> +    object_class_property_set_description(oc, NITRO_ENCLAVE_VSOCK_CHARDEV_ID,
> +                                          "Set chardev id for vhost-user-vsock "
> +                                          "device");
>  }
>  
>  static const TypeInfo nitro_enclave_machine_info = {
> diff --git a/include/hw/boards.h b/include/hw/boards.h
> index 48ff6d8b93..c268e7f005 100644
> --- a/include/hw/boards.h
> +++ b/include/hw/boards.h
> @@ -308,6 +308,8 @@ struct MachineClass {
>      int64_t (*get_default_cpu_node_id)(const MachineState *ms, int idx);
>      ram_addr_t (*fixup_ram_size)(ram_addr_t size);
>      uint64_t smbios_memory_device_size;
> +    bool (*create_default_memdev)(MachineState *ms, const char *path,
> +                                  Error **errp);
>  };
>  
>  /**
> diff --git a/include/hw/i386/nitro_enclave.h b/include/hw/i386/nitro_enclave.h
> index a1dada9371..3e302de851 100644
> --- a/include/hw/i386/nitro_enclave.h
> +++ b/include/hw/i386/nitro_enclave.h
> @@ -14,12 +14,20 @@
>  #include "hw/i386/microvm.h"
>  #include "qom/object.h"
>  
> +/* Machine type options */
> +#define NITRO_ENCLAVE_VSOCK_CHARDEV_ID "vsock"
> +
>  struct NitroEnclaveMachineClass {
>      MicrovmMachineClass parent;
> +
> +    void (*parent_init)(MachineState *state);
>  };
>  
>  struct NitroEnclaveMachineState {
>      MicrovmMachineState parent;
> +
> +    /* Machine type options */
> +    char *vsock;
>  };
>  
>  #define TYPE_NITRO_ENCLAVE_MACHINE MACHINE_TYPE_NAME("nitro-enclave")
> diff --git a/include/sysemu/hostmem.h b/include/sysemu/hostmem.h
> index de47ae59e4..67f45abe39 100644
> --- a/include/sysemu/hostmem.h
> +++ b/include/sysemu/hostmem.h
> @@ -39,6 +39,8 @@ OBJECT_DECLARE_TYPE(HostMemoryBackend, HostMemoryBackendClass,
>   */
>  #define TYPE_MEMORY_BACKEND_FILE "memory-backend-file"
>  
> +#define TYPE_MEMORY_BACKEND_MEMFD "memory-backend-memfd"
> +
>  
>  /**
>   * HostMemoryBackendClass:
> -- 
> 2.39.2
> 

With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|
Re: [PATCH v3 2/5] machine/nitro-enclave: Add vhost-user-vsock device
Posted by Dorjoy Chowdhury 3 months, 1 week ago
On Mon, Aug 12, 2024 at 8:24 PM Daniel P. Berrangé <berrange@redhat.com> wrote:
>
> On Sat, Aug 10, 2024 at 10:44:59PM +0600, Dorjoy Chowdhury wrote:
> > AWS Nitro Enclaves have built-in vhost-vsock device support which
> > enables applications in enclave VMs to communicate with the parent
> > EC2 VM over vsock. The enclave VMs have dynamic CID while the parent
> > always has CID 3. In QEMU, the vsock emulation for nitro enclave is
> > added using vhost-user-vsock as opposed to vhost-vsock. vhost-vsock
> > doesn't support sibling VM communication which is needed for nitro
> > enclaves.
> >
> > In QEMU's nitro-enclave emulation, for the vsock communication to CID
> > 3 to work, another process that does the vsock emulation in  userspace
> > must be run, for example, vhost-device-vsock[1] from rust-vmm, with
> > necessary vsock communication support in another guest VM with CID 3.
> > A new mandatory nitro-enclave machine option 'vsock' has been added.
> > The value for this option should be the chardev id from the '-chardev'
> > option for the vhost-user-vsock device to work.
> >
> > Using vhost-user-vsock also enables the possibility to implement some
> > proxying support in the vhost-user-vsock daemon that will forward all
> > the packets to the host machine instead of CID 3 so that users of
> > nitro-enclave can run the necessary applications in their host machine
> > instead of running another whole VM with CID 3.
> >
> > [1] https://github.com/rust-vmm/vhost-device/tree/main/vhost-device-vsock
> >
> > Signed-off-by: Dorjoy Chowdhury <dorjoychy111@gmail.com>
> > ---
> >  backends/hostmem-memfd.c        |   2 -
> >  hw/core/machine.c               |  71 +++++++++---------
> >  hw/i386/Kconfig                 |   1 +
> >  hw/i386/nitro_enclave.c         | 123 ++++++++++++++++++++++++++++++++
> >  include/hw/boards.h             |   2 +
> >  include/hw/i386/nitro_enclave.h |   8 +++
> >  include/sysemu/hostmem.h        |   2 +
> >  7 files changed, 174 insertions(+), 35 deletions(-)
> >
>
> > diff --git a/hw/i386/nitro_enclave.c b/hw/i386/nitro_enclave.c
> > index 98690c6373..280ab4cc9b 100644
> > --- a/hw/i386/nitro_enclave.c
> > +++ b/hw/i386/nitro_enclave.c
> > @@ -11,11 +11,81 @@
> >  #include "qemu/osdep.h"
> >  #include "qemu/error-report.h"
> >  #include "qapi/error.h"
> > +#include "qom/object_interfaces.h"
> >
> > +#include "chardev/char.h"
> > +#include "hw/sysbus.h"
> >  #include "hw/core/eif.h"
> >  #include "hw/i386/x86.h"
> >  #include "hw/i386/microvm.h"
> >  #include "hw/i386/nitro_enclave.h"
> > +#include "hw/virtio/virtio-mmio.h"
> > +#include "hw/virtio/vhost-user-vsock.h"
> > +#include "sysemu/hostmem.h"
> > +
> > +static BusState *find_free_virtio_mmio_bus(void)
> > +{
> > +    BusChild *kid;
> > +    BusState *bus = sysbus_get_default();
> > +
> > +    QTAILQ_FOREACH(kid, &bus->children, sibling) {
> > +        DeviceState *dev = kid->child;
> > +        if (object_dynamic_cast(OBJECT(dev), TYPE_VIRTIO_MMIO)) {
> > +            VirtIOMMIOProxy *mmio = VIRTIO_MMIO(OBJECT(dev));
> > +            VirtioBusState *mmio_virtio_bus = &mmio->bus;
> > +            BusState *mmio_bus = &mmio_virtio_bus->parent_obj;
> > +            if (QTAILQ_EMPTY(&mmio_bus->children)) {
> > +                return mmio_bus;
> > +            }
> > +        }
> > +    }
> > +
> > +    return NULL;
> > +}
> > +
> > +static void vhost_user_vsock_init(NitroEnclaveMachineState *nems)
> > +{
> > +    DeviceState *dev = qdev_new(TYPE_VHOST_USER_VSOCK);
> > +    VHostUserVSock *vsock = VHOST_USER_VSOCK(dev);
> > +    BusState *bus;
> > +
> > +    if (!nems->vsock) {
> > +        error_report("A valid chardev id for vhost-user-vsock device must be "
> > +                     "provided using the 'vsock' machine option");
> > +        exit(1);
> > +    }
> > +
> > +    bus = find_free_virtio_mmio_bus();
> > +    if (!bus) {
> > +        error_report("Failed to find bus for vhost-user-vsock device");
> > +        exit(1);
> > +    }
> > +
> > +    Chardev *chardev = qemu_chr_find(nems->vsock);
> > +    if (!chardev) {
> > +        error_report("Failed to find chardev with id %s", nems->vsock);
> > +        exit(1);
> > +    }
> > +
> > +    vsock->conf.chardev.chr = chardev;
> > +
> > +    qdev_realize_and_unref(dev, bus, &error_fatal);
> > +}
>
> Why does this machine need to create the vhost-user-vsock device itself ?
> Doing it this way prevents the mgmt app from changing any of the other
> vsock device settings beyond 'chardev'. The entity creating QEMU can use
> -device to create the vsock device.
>

Hi Daniel. Good point. The reason to make the vhost-user-vsock device
built-in is because nitro VMs will always need it anyway (like the
virtio-nsm device which is built-in too). When an EIF image is built
using nitro-cli the "init" process in the EIF image tries to connect
to (AF_VSOCK, CID 3, port 9000) to send a heartbeat and expects a
heartbeat reply. So my understanding is that if we don't create it
inside the machine code itself, users would always need to provide the
explicit options for the device anyway. But as you point out this also
makes the device settings non-configurable.

Hey Alex, do you have any suggestions on this?

Regards,
Dorjoy
Re: [PATCH v3 2/5] machine/nitro-enclave: Add vhost-user-vsock device
Posted by Alexander Graf 3 months, 1 week ago
On 13.08.24 20:02, Dorjoy Chowdhury wrote:
> On Mon, Aug 12, 2024 at 8:24 PM Daniel P. Berrangé <berrange@redhat.com> wrote:
>> On Sat, Aug 10, 2024 at 10:44:59PM +0600, Dorjoy Chowdhury wrote:
>>> AWS Nitro Enclaves have built-in vhost-vsock device support which
>>> enables applications in enclave VMs to communicate with the parent
>>> EC2 VM over vsock. The enclave VMs have dynamic CID while the parent
>>> always has CID 3. In QEMU, the vsock emulation for nitro enclave is
>>> added using vhost-user-vsock as opposed to vhost-vsock. vhost-vsock
>>> doesn't support sibling VM communication which is needed for nitro
>>> enclaves.
>>>
>>> In QEMU's nitro-enclave emulation, for the vsock communication to CID
>>> 3 to work, another process that does the vsock emulation in  userspace
>>> must be run, for example, vhost-device-vsock[1] from rust-vmm, with
>>> necessary vsock communication support in another guest VM with CID 3.
>>> A new mandatory nitro-enclave machine option 'vsock' has been added.
>>> The value for this option should be the chardev id from the '-chardev'
>>> option for the vhost-user-vsock device to work.
>>>
>>> Using vhost-user-vsock also enables the possibility to implement some
>>> proxying support in the vhost-user-vsock daemon that will forward all
>>> the packets to the host machine instead of CID 3 so that users of
>>> nitro-enclave can run the necessary applications in their host machine
>>> instead of running another whole VM with CID 3.
>>>
>>> [1] https://github.com/rust-vmm/vhost-device/tree/main/vhost-device-vsock
>>>
>>> Signed-off-by: Dorjoy Chowdhury <dorjoychy111@gmail.com>
>>> ---
>>>   backends/hostmem-memfd.c        |   2 -
>>>   hw/core/machine.c               |  71 +++++++++---------
>>>   hw/i386/Kconfig                 |   1 +
>>>   hw/i386/nitro_enclave.c         | 123 ++++++++++++++++++++++++++++++++
>>>   include/hw/boards.h             |   2 +
>>>   include/hw/i386/nitro_enclave.h |   8 +++
>>>   include/sysemu/hostmem.h        |   2 +
>>>   7 files changed, 174 insertions(+), 35 deletions(-)
>>>
>>> diff --git a/hw/i386/nitro_enclave.c b/hw/i386/nitro_enclave.c
>>> index 98690c6373..280ab4cc9b 100644
>>> --- a/hw/i386/nitro_enclave.c
>>> +++ b/hw/i386/nitro_enclave.c
>>> @@ -11,11 +11,81 @@
>>>   #include "qemu/osdep.h"
>>>   #include "qemu/error-report.h"
>>>   #include "qapi/error.h"
>>> +#include "qom/object_interfaces.h"
>>>
>>> +#include "chardev/char.h"
>>> +#include "hw/sysbus.h"
>>>   #include "hw/core/eif.h"
>>>   #include "hw/i386/x86.h"
>>>   #include "hw/i386/microvm.h"
>>>   #include "hw/i386/nitro_enclave.h"
>>> +#include "hw/virtio/virtio-mmio.h"
>>> +#include "hw/virtio/vhost-user-vsock.h"
>>> +#include "sysemu/hostmem.h"
>>> +
>>> +static BusState *find_free_virtio_mmio_bus(void)
>>> +{
>>> +    BusChild *kid;
>>> +    BusState *bus = sysbus_get_default();
>>> +
>>> +    QTAILQ_FOREACH(kid, &bus->children, sibling) {
>>> +        DeviceState *dev = kid->child;
>>> +        if (object_dynamic_cast(OBJECT(dev), TYPE_VIRTIO_MMIO)) {
>>> +            VirtIOMMIOProxy *mmio = VIRTIO_MMIO(OBJECT(dev));
>>> +            VirtioBusState *mmio_virtio_bus = &mmio->bus;
>>> +            BusState *mmio_bus = &mmio_virtio_bus->parent_obj;
>>> +            if (QTAILQ_EMPTY(&mmio_bus->children)) {
>>> +                return mmio_bus;
>>> +            }
>>> +        }
>>> +    }
>>> +
>>> +    return NULL;
>>> +}
>>> +
>>> +static void vhost_user_vsock_init(NitroEnclaveMachineState *nems)
>>> +{
>>> +    DeviceState *dev = qdev_new(TYPE_VHOST_USER_VSOCK);
>>> +    VHostUserVSock *vsock = VHOST_USER_VSOCK(dev);
>>> +    BusState *bus;
>>> +
>>> +    if (!nems->vsock) {
>>> +        error_report("A valid chardev id for vhost-user-vsock device must be "
>>> +                     "provided using the 'vsock' machine option");
>>> +        exit(1);
>>> +    }
>>> +
>>> +    bus = find_free_virtio_mmio_bus();
>>> +    if (!bus) {
>>> +        error_report("Failed to find bus for vhost-user-vsock device");
>>> +        exit(1);
>>> +    }
>>> +
>>> +    Chardev *chardev = qemu_chr_find(nems->vsock);
>>> +    if (!chardev) {
>>> +        error_report("Failed to find chardev with id %s", nems->vsock);
>>> +        exit(1);
>>> +    }
>>> +
>>> +    vsock->conf.chardev.chr = chardev;
>>> +
>>> +    qdev_realize_and_unref(dev, bus, &error_fatal);
>>> +}
>> Why does this machine need to create the vhost-user-vsock device itself ?
>> Doing it this way prevents the mgmt app from changing any of the other
>> vsock device settings beyond 'chardev'. The entity creating QEMU can use
>> -device to create the vsock device.
>>
> Hi Daniel. Good point. The reason to make the vhost-user-vsock device
> built-in is because nitro VMs will always need it anyway (like the
> virtio-nsm device which is built-in too). When an EIF image is built
> using nitro-cli the "init" process in the EIF image tries to connect
> to (AF_VSOCK, CID 3, port 9000) to send a heartbeat and expects a
> heartbeat reply. So my understanding is that if we don't create it
> inside the machine code itself, users would always need to provide the
> explicit options for the device anyway. But as you point out this also
> makes the device settings non-configurable.
>
> Hey Alex, do you have any suggestions on this?


IMHO devices that are required for the machine to function should be 
part of the machine. Since vsock is a core part of the Nitro Enclave, it 
should be part of the machine definition. It's as much a board 
dependency as fw_cfg.

Alex




Amazon Web Services Development Center Germany GmbH
Krausenstr. 38
10117 Berlin
Geschaeftsfuehrung: Christian Schlaeger, Jonathan Weiss
Eingetragen am Amtsgericht Charlottenburg unter HRB 257764 B
Sitz: Berlin
Ust-ID: DE 365 538 597