[v7,13/21] multi-process: Forward PCI config space acceses to the remote process

Commit Message

Elena Ufimtseva June 27, 2020, 5:09 p.m. UTC

From: Elena Ufimtseva <[e-mail]>

The Proxy Object sends the PCI config space accesses as messages
to the remote process over the communication channel

TODO:
Investigate if the local PCI config writes can be dropped.
Without the proxy local PCI config space writes for the device,
the driver in the guest times out on the probing.
We have tried to only refer to the remote for the PCI config writes,
but the driver timeout in the guest forced as to left this
as it is (removing local PCI config only).

Signed-off-by: Elena Ufimtseva <[e-mail]>
Signed-off-by: Jagannathan Raman <[e-mail]>
Signed-off-by: John G Johnson <[e-mail]>
---
 hw/i386/remote-msg.c     | 54 ++++++++++++++++++++++++++++++++++++++++
 hw/pci/proxy.c           | 54 ++++++++++++++++++++++++++++++++++++++++
 include/io/mpqemu-link.h |  8 ++++++
 io/mpqemu-link.c         | 14 +++++++++++
 4 files changed, 130 insertions(+)

Comments

Stefan Hajnoczi July 1, 2020, 9:40 a.m. UTC | #1

On Sat, Jun 27, 2020 at 10:09:35AM -0700, [e-mail] wrote:
> @@ -42,6 +48,12 @@ gboolean mpqemu_process_msg(QIOChannel *ioc, GIOCondition cond,
>      case CONNECT_DEV:
>          process_connect_dev_msg(&msg, ioc, &local_err);
>          break;
> +    case PCI_CONFIG_WRITE:
> +        process_config_write(ioc, pci_dev, &msg);
> +        break;
> +    case PCI_CONFIG_READ:
> +        process_config_read(ioc, pci_dev, &msg);
> +        break;

pci_dev is NULL when mpqemu_process_msg() is called on the main socket.
This is an example of how the N:1 model complicates things.  Now
process_config_read/write() need to check that pci_dev is non-NULL to
avoid crashing.

>      default:
>          error_setg(&local_err, "Unknown command (%d) received from proxy \
>                     in remote process pid=%d", msg.cmd, getpid());
> @@ -89,3 +101,45 @@ exit:
>  
>      mpqemu_msg_send(&ret, com);
>  }
> +
> +static void process_config_write(QIOChannel *ioc, PCIDevice *dev,
> +                                 MPQemuMsg *msg)
> +{
> +    struct conf_data_msg *conf = (struct conf_data_msg *)msg->data2;
> +    MPQemuMsg ret = { 0 };
> +
> +    if (conf->addr >= PCI_CFG_SPACE_EXP_SIZE) {

This check treats all devices as PCIe devices. Traditional PCI devices
have a smaller config space and pci_default_write_config() has an
assertion that fails on out-of-bounds writes:

  assert(addr + l <= pci_config_size(d));

Are you sure all devices are PCIe? If yes, please enforce that in the
code. If no, then please fix the size check.

> +struct conf_data_msg {
> +    uint32_t addr;
> +    uint32_t val;
> +    int l;
> +};

QEMU coding style uses typedefs:

  typedef struct {
      uint32_t addr;
      uint32_t val;
      int l;
  } ConfDataMsg;

Patch

diff --git a/hw/i386/remote-msg.c b/hw/i386/remote-msg.c
index 68f50866bb..aa5780d521 100644
--- a/hw/i386/remote-msg.c
+++ b/hw/i386/remote-msg.c
@@ -11,10 +11,16 @@ 
 
 static void process_connect_dev_msg(MPQemuMsg *msg, QIOChannel *com,
                                     Error **errp);
+static void process_config_write(QIOChannel *ioc, PCIDevice *dev,
+                                 MPQemuMsg *msg);
+static void process_config_read(QIOChannel *ioc, PCIDevice *dev,
+                                MPQemuMsg *msg);
 
 gboolean mpqemu_process_msg(QIOChannel *ioc, GIOCondition cond,
                             gpointer opaque)
 {
+    DeviceState *dev = (DeviceState *)opaque;
+    PCIDevice *pci_dev = PCI_DEVICE(dev);
     Error *local_err = NULL;
     MPQemuMsg msg = { 0 };
 
@@ -42,6 +48,12 @@  gboolean mpqemu_process_msg(QIOChannel *ioc, GIOCondition cond,
     case CONNECT_DEV:
         process_connect_dev_msg(&msg, ioc, &local_err);
         break;
+    case PCI_CONFIG_WRITE:
+        process_config_write(ioc, pci_dev, &msg);
+        break;
+    case PCI_CONFIG_READ:
+        process_config_read(ioc, pci_dev, &msg);
+        break;
     default:
         error_setg(&local_err, "Unknown command (%d) received from proxy \
                    in remote process pid=%d", msg.cmd, getpid());
@@ -89,3 +101,45 @@  exit:
 
     mpqemu_msg_send(&ret, com);
 }
+
+static void process_config_write(QIOChannel *ioc, PCIDevice *dev,
+                                 MPQemuMsg *msg)
+{
+    struct conf_data_msg *conf = (struct conf_data_msg *)msg->data2;
+    MPQemuMsg ret = { 0 };
+
+    if (conf->addr >= PCI_CFG_SPACE_EXP_SIZE) {
+        error_report("Bad address received when writing PCI config, pid %d",
+                     getpid());
+        ret.data1.u64 = UINT64_MAX;
+    } else {
+        pci_default_write_config(dev, conf->addr, conf->val, conf->l);
+    }
+
+    ret.cmd = RET_MSG;
+    ret.bytestream = 0;
+    ret.size = sizeof(ret.data1);
+
+    mpqemu_msg_send(&ret, ioc);
+}
+
+static void process_config_read(QIOChannel *ioc, PCIDevice *dev,
+                                MPQemuMsg *msg)
+{
+    struct conf_data_msg *conf = (struct conf_data_msg *)msg->data2;
+    MPQemuMsg ret = { 0 };
+
+    if (conf->addr >= PCI_CFG_SPACE_EXP_SIZE) {
+        error_report("Bad address received when reading PCI config, pid %d",
+                     getpid());
+        ret.data1.u64 = UINT64_MAX;
+    } else {
+        ret.data1.u64 = pci_default_read_config(dev, conf->addr, conf->l);
+    }
+
+    ret.cmd = RET_MSG;
+    ret.bytestream = 0;
+    ret.size = sizeof(ret.data1);
+
+    mpqemu_msg_send(&ret, ioc);
+}
diff --git a/hw/pci/proxy.c b/hw/pci/proxy.c
index 16649ed0ec..8934070a20 100644
--- a/hw/pci/proxy.c
+++ b/hw/pci/proxy.c
@@ -16,6 +16,7 @@ 
 #include "hw/qdev-properties.h"
 #include "monitor/monitor.h"
 #include "io/mpqemu-link.h"
+#include "qemu/error-report.h"
 
 static void proxy_set_socket(PCIProxyDev *pdev, int fd, Error **errp)
 {
@@ -69,12 +70,65 @@  static void pci_proxy_dev_realize(PCIDevice *device, Error **errp)
     }
 }
 
+static int config_op_send(PCIProxyDev *pdev, uint32_t addr, uint32_t *val,
+                          int l, unsigned int op)
+{
+    struct conf_data_msg conf_data;
+    MPQemuMsg msg = { 0 };
+    long ret = -EINVAL;
+    Error *local_err = NULL;
+
+    conf_data.addr = addr;
+    conf_data.val = (op == PCI_CONFIG_WRITE) ? *val : 0;
+    conf_data.l = l;
+
+    msg.data2 = (uint8_t *)&conf_data;
+
+    msg.size = sizeof(conf_data);
+    msg.cmd = op;
+    msg.bytestream = 1;
+
+    ret = mpqemu_msg_send_reply_co(&msg, pdev->dev, &local_err);
+    if (local_err) {
+        error_report("Failed to exchange PCI_CONFIG message with remote");
+    }
+    if (op == PCI_CONFIG_READ) {
+        *val = (uint32_t)ret;
+    }
+
+    return ret;
+}
+
+static uint32_t pci_proxy_read_config(PCIDevice *d, uint32_t addr, int len)
+{
+    uint32_t val;
+
+    (void)config_op_send(PCI_PROXY_DEV(d), addr, &val, len, PCI_CONFIG_READ);
+
+    return val;
+}
+
+static void pci_proxy_write_config(PCIDevice *d, uint32_t addr, uint32_t val,
+                                   int l)
+{
+    /*
+     * Some of the functions access the copy of the remote device
+     * PCI config space, therefore maintain it updated.
+     */
+    pci_default_write_config(d, addr, val, l);
+
+    (void)config_op_send(PCI_PROXY_DEV(d), addr, &val, l, PCI_CONFIG_WRITE);
+}
+
 static void pci_proxy_dev_class_init(ObjectClass *klass, void *data)
 {
     DeviceClass *dc = DEVICE_CLASS(klass);
     PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
 
     k->realize = pci_proxy_dev_realize;
+    k->config_read = pci_proxy_read_config;
+    k->config_write = pci_proxy_write_config;
+
     device_class_set_props(dc, proxy_properties);
 }
 
diff --git a/include/io/mpqemu-link.h b/include/io/mpqemu-link.h
index d620806c17..bbd6f3db44 100644
--- a/include/io/mpqemu-link.h
+++ b/include/io/mpqemu-link.h
@@ -38,6 +38,8 @@  typedef enum {
     SYNC_SYSMEM,
     CONNECT_DEV,
     RET_MSG,
+    PCI_CONFIG_WRITE,
+    PCI_CONFIG_READ,
     MAX = INT_MAX,
 } MPQemuCmd;
 
@@ -47,6 +49,12 @@  typedef struct {
     off_t offsets[REMOTE_MAX_FDS];
 } SyncSysmemMsg;
 
+struct conf_data_msg {
+    uint32_t addr;
+    uint32_t val;
+    int l;
+};
+
 /**
  * Maximum size of data2 field in the message to be transmitted.
  */
diff --git a/io/mpqemu-link.c b/io/mpqemu-link.c
index 54df3b254e..19887611d0 100644
--- a/io/mpqemu-link.c
+++ b/io/mpqemu-link.c
@@ -242,6 +242,20 @@  bool mpqemu_msg_valid(MPQemuMsg *msg)
             return false;
         }
         break;
+    case PCI_CONFIG_WRITE:
+    case PCI_CONFIG_READ:
+        if (msg->size != sizeof(struct conf_data_msg)) {
+            return false;
+        }
+        if (!msg->bytestream) {
+            return false;
+        }
+        struct conf_data_msg *conf = (struct conf_data_msg *)msg->data2;
+
+        if (conf->l != 1 && conf->l != 2 && conf->l != 4) {
+            return false;
+        }
+        break;
     default:
         break;
     }