diff mbox series

[v5,47/50] multi-process: Enable support for multiple devices in remote

Message ID cf33de60b827e0cdd3d5fce48eb038cb056dc304.1582576372.git.jag.raman@oracle.com (mailing list archive)
State New, archived
Headers show
Series Initial support for multi-process qemu | expand

Commit Message

Jag Raman Feb. 24, 2020, 8:55 p.m. UTC
From: Elena Ufimtseva <elena.ufimtseva@oracle.com>

Add support to allow multiple devices to be configured in the
remote process

Signed-off-by: Elena Ufimtseva <elena.ufimtseva@oracle.com>
Signed-off-by: John G Johnson <john.g.johnson@oracle.com>
Signed-off-by: Jagannathan Raman <jag.raman@oracle.com>
---
 hw/proxy/qemu-proxy.c         |  4 ++-
 include/hw/proxy/qemu-proxy.h |  3 +++
 include/io/mpqemu-link.h      |  1 +
 qdev-monitor.c                |  2 ++
 remote/remote-main.c          | 62 +++++++++++++++++++++++++++++++++++--------
 5 files changed, 60 insertions(+), 12 deletions(-)

Comments

Stefan Hajnoczi Feb. 28, 2020, 4:44 p.m. UTC | #1
On Mon, Feb 24, 2020 at 03:55:38PM -0500, Jagannathan Raman wrote:
> From: Elena Ufimtseva <elena.ufimtseva@oracle.com>
> 
> Add support to allow multiple devices to be configured in the
> remote process

This patch allows multiple devices to be addressed over a single UNIX
domain socket.  This could be a scalability/performance bottleneck
because an SMP guest can only talk to 1 device at a time.

This approach doesn't address the qemu-storage-daemon use case where one
device emulation process provides devices to multiple guests.  Multiple
UNIX domain sockets are needed for that.

Is multiplexing multiple devices over a single connection is a desirable
feature?  The alternative of one UNIX domain socket per device instance
seems more practical to me because it should perform better and solves
the qemu-storage-daemon use case.
Jag Raman March 2, 2020, 7:28 p.m. UTC | #2
On 2/28/2020 11:44 AM, Stefan Hajnoczi wrote:
> On Mon, Feb 24, 2020 at 03:55:38PM -0500, Jagannathan Raman wrote:
>> From: Elena Ufimtseva <elena.ufimtseva@oracle.com>
>>
>> Add support to allow multiple devices to be configured in the
>> remote process
> 
> This patch allows multiple devices to be addressed over a single UNIX
> domain socket.  This could be a scalability/performance bottleneck
> because an SMP guest can only talk to 1 device at a time.
> 
> This approach doesn't address the qemu-storage-daemon use case where one
> device emulation process provides devices to multiple guests.  Multiple
> UNIX domain sockets are needed for that.
> 
> Is multiplexing multiple devices over a single connection is a desirable
> feature?  The alternative of one UNIX domain socket per device instance
> seems more practical to me because it should perform better and solves
> the qemu-storage-daemon use case.

Hi Stefan,

We will implement a separate communication channel for each device in
the remote process, in the next series.

Thanks!
--
Jag

>
diff mbox series

Patch

diff --git a/hw/proxy/qemu-proxy.c b/hw/proxy/qemu-proxy.c
index 5876957..4d4eff4 100644
--- a/hw/proxy/qemu-proxy.c
+++ b/hw/proxy/qemu-proxy.c
@@ -242,7 +242,7 @@  static int set_remote_opts(PCIDevice *dev, QDict *qdict, unsigned int cmd)
     msg.cmd = cmd;
     msg.bytestream = 1;
     msg.size = qstring_get_length(qstr) + 1;
-
+    msg.id = pdev->id;
 
     wait = eventfd(0, EFD_NONBLOCK);
     msg.num_fds = 1;
@@ -404,6 +404,7 @@  static int config_op_send(PCIProxyDev *dev, uint32_t addr, uint32_t *val, int l,
     msg.size = sizeof(conf_data);
     msg.cmd = op;
     msg.bytestream = 1;
+    msg.id = dev->id;
 
     if (op == PCI_CONFIG_WRITE) {
         msg.num_fds = 0;
@@ -710,6 +711,7 @@  static void setup_irqfd(PCIProxyDev *dev)
 
     memset(&msg, 0, sizeof(MPQemuMsg));
     msg.cmd = SET_IRQFD;
+    msg.id = dev->id;
     msg.num_fds = 2;
     msg.fds[0] = event_notifier_get_fd(&dev->intr);
     msg.fds[1] = event_notifier_get_fd(&dev->resample);
diff --git a/include/hw/proxy/qemu-proxy.h b/include/hw/proxy/qemu-proxy.h
index 6784298..5ff3aec 100644
--- a/include/hw/proxy/qemu-proxy.h
+++ b/include/hw/proxy/qemu-proxy.h
@@ -43,6 +43,9 @@  extern const MemoryRegionOps proxy_default_ops;
 struct PCIProxyDev {
     PCIDevice parent_dev;
 
+    uint64_t id;
+    uint64_t nr_devices;
+
     int n_mr_sections;
     MemoryRegionSection *mr_sections;
 
diff --git a/include/io/mpqemu-link.h b/include/io/mpqemu-link.h
index beb8293..722f08c 100644
--- a/include/io/mpqemu-link.h
+++ b/include/io/mpqemu-link.h
@@ -124,6 +124,7 @@  typedef struct {
 typedef struct {
     mpqemu_cmd_t cmd;
     int bytestream;
+    uint64_t id;
     size_t size;
 
     union {
diff --git a/qdev-monitor.c b/qdev-monitor.c
index 8efa1a8..cda2545 100644
--- a/qdev-monitor.c
+++ b/qdev-monitor.c
@@ -735,8 +735,10 @@  DeviceState *qdev_proxy_add(const char *rid, const char *id, char *bus,
         pdev->mmio_sock = old_pdev->mmio_sock;
         pdev->remote_pid = old_pdev->remote_pid;
         pdev->mem_init = true;
+        pdev->id = old_pdev->nr_devices++;
     } else {
         pdev->socket = managed ? socket : -1;
+        pdev->id =  pdev->nr_devices++;
     }
     pdev->managed = managed;
 
diff --git a/remote/remote-main.c b/remote/remote-main.c
index ab4ad0b..20d160e 100644
--- a/remote/remote-main.c
+++ b/remote/remote-main.c
@@ -72,15 +72,23 @@ 
 
 static MPQemuLinkState *mpqemu_link;
 
-PCIDevice *remote_pci_dev;
+PCIDevice **remote_pci_devs;
+uint64_t nr_devices;
+#define MAX_REMOTE_DEVICES 256
+
 bool create_done;
 
 static void process_config_write(MPQemuMsg *msg)
 {
     struct conf_data_msg *conf = (struct conf_data_msg *)msg->data2;
 
+    if (msg->id > nr_devices) {
+        return;
+    }
+
     qemu_mutex_lock_iothread();
-    pci_default_write_config(remote_pci_dev, conf->addr, conf->val, conf->l);
+    pci_default_write_config(remote_pci_devs[msg->id], conf->addr, conf->val,
+                             conf->l);
     qemu_mutex_unlock_iothread();
 }
 
@@ -92,8 +100,13 @@  static void process_config_read(MPQemuMsg *msg)
 
     wait = msg->fds[0];
 
+    if (msg->id > nr_devices) {
+        return;
+    }
+
     qemu_mutex_lock_iothread();
-    val = pci_default_read_config(remote_pci_dev, conf->addr, conf->l);
+    val = pci_default_read_config(remote_pci_devs[msg->id], conf->addr,
+                                  conf->l);
     qemu_mutex_unlock_iothread();
 
     notify_proxy(wait, val);
@@ -282,6 +295,13 @@  static int setup_device(MPQemuMsg *msg, Error **errp)
     }
 
     if (!msg->data2) {
+        error_setg(errp, "Message data is empty");
+        return rc;
+    }
+
+    if (msg->id > MAX_REMOTE_DEVICES) {
+        error_setg(errp, "id of the device is larger than max number of "\
+                         "devices per remote process.");
         return rc;
     }
 
@@ -319,8 +339,15 @@  static int setup_device(MPQemuMsg *msg, Error **errp)
                    qstring_get_str(qobject_to_json(QOBJECT(qdict))));
         goto device_failed;
     }
+
     if (object_dynamic_cast(OBJECT(dev), TYPE_PCI_DEVICE)) {
-        remote_pci_dev = PCI_DEVICE(dev);
+        if (nr_devices <= msg->id) {
+            nr_devices = msg->id + 1;
+            remote_pci_devs = g_realloc(remote_pci_devs,
+                                        nr_devices * sizeof(PCIDevice *));
+        }
+
+        remote_pci_devs[msg->id] = PCI_DEVICE(dev);
     }
 
     notify_proxy(wait, (uint32_t)REMOTE_OK);
@@ -405,11 +432,17 @@  static void process_msg(GIOCondition cond, MPQemuChannel *chan)
         goto finalize_loop;
     }
 
+    if (msg->id > MAX_REMOTE_DEVICES) {
+        error_setg(&err, "id of the device is larger than max number of "\
+                         "devices per remote process.");
+        goto finalize_loop;
+    }
+
     switch (msg->cmd) {
     case INIT:
         break;
     case GET_PCI_INFO:
-        process_get_pci_info_msg(remote_pci_dev, msg);
+        process_get_pci_info_msg(remote_pci_devs[msg->id], msg);
         break;
     case PCI_CONFIG_WRITE:
         if (create_done) {
@@ -449,12 +482,19 @@  static void process_msg(GIOCondition cond, MPQemuChannel *chan)
         }
         break;
     case SET_IRQFD:
-        process_set_irqfd_msg(remote_pci_dev, msg);
-        qdev_machine_creation_done();
-        qemu_mutex_lock_iothread();
-        qemu_run_machine_init_done_notifiers();
-        qemu_mutex_unlock_iothread();
-        create_done = true;
+        if (msg->id > nr_devices) {
+            error_setg(&err, "incorrect device id in the message");
+            goto finalize_loop;
+        }
+        process_set_irqfd_msg(remote_pci_devs[msg->id], msg);
+
+        if (!create_done) {
+            qdev_machine_creation_done();
+            qemu_mutex_lock_iothread();
+            qemu_run_machine_init_done_notifiers();
+            qemu_mutex_unlock_iothread();
+            create_done = true;
+        }
         break;
     case DEV_OPTS:
         if (setup_device(msg, &err)) {