[RESEND,v6,13/36] multi-process: setup PCI host bridge for remote device

Commit Message

Elena Ufimtseva April 23, 2020, 4:13 a.m. UTC

From: Jagannathan Raman <[e-mail]>

PCI host bridge is setup for the remote device process. It is
implemented using remote-pcihost object. It is an extension of the PCI
host bridge setup by QEMU.
Remote-pcihost configures a PCI bus which could be used by the remote
 PCI device to latch on to.

Signed-off-by: Jagannathan Raman <[e-mail]>
Signed-off-by: John G Johnson <[e-mail]>
Signed-off-by: Elena Ufimtseva <[e-mail]>
---
 MAINTAINERS              |  2 ++
 hw/pci/Makefile.objs     |  2 +-
 include/remote/pcihost.h | 45 ++++++++++++++++++++++++++++
 remote/Makefile.objs     |  1 +
 remote/pcihost.c         | 64 ++++++++++++++++++++++++++++++++++++++++
 5 files changed, 113 insertions(+), 1 deletion(-)
 create mode 100644 include/remote/pcihost.h
 create mode 100644 remote/pcihost.c

Comments

Stefan Hajnoczi May 12, 2020, 10:31 a.m. UTC | #1

On Wed, Apr 22, 2020 at 09:13:48PM -0700, [e-mail] wrote:
> diff --git a/include/remote/pcihost.h b/include/remote/pcihost.h
> new file mode 100644
> index 0000000000..7aca9ccaf1
> --- /dev/null
> +++ b/include/remote/pcihost.h
> @@ -0,0 +1,45 @@
> +/*
> + * PCI Host for remote device
> + *
> + * Copyright © 2018, 2020 Oracle and/or its affiliates.
> + *
> + * This work is licensed under the terms of the GNU GPL, version 2 or later.
> + * See the COPYING file in the top-level directory.
> + *
> + */
> +
> +#ifndef REMOTE_PCIHOST_H
> +#define REMOTE_PCIHOST_H
> +
> +#include <stddef.h>
> +#include <stdint.h>
> +
> +#include "exec/memory.h"
> +#include "hw/pci/pcie_host.h"
> +
> +#define TYPE_REMOTE_HOST_DEVICE "remote-pcihost"
> +#define REMOTE_HOST_DEVICE(obj) \
> +    OBJECT_CHECK(RemPCIHost, (obj), TYPE_REMOTE_HOST_DEVICE)
> +
> +typedef struct RemPCIHost {

Hmm...this object has no state or behavior specific to remote device
emulation. Could you use an existing PCIe host instead? It's not clear
to me that a new object is needed.

> +    /*< private >*/
> +    PCIExpressHost parent_obj;
> +    /*< public >*/
> +
> +    /*
> +     * Memory Controller Hub (MCH) may not be necessary for the emulation
> +     * program. The two important reasons for implementing a PCI host in the
> +     * emulation program are:
> +     * - Provide a PCI bus for IO devices
> +     * - Enable translation of guest PA to the PCI bar regions
> +     *
> +     * For both the above mentioned purposes, it doesn't look like we would
> +     * need the MCH
> +     */
> +
> +    MemoryRegion *mr_pci_mem;
> +    MemoryRegion *mr_sys_mem;

Unused?

> +    MemoryRegion *mr_sys_io;
> +} RemPCIHost;

The name "RemotePCIHost" would be consistent with the QOM type and the
filename. It might seem trivial but when reading code that others have
written every time the naming changes you need to figure out why (just
an inconsistency or is this a different concept/abstraction?).

> +
> +#endif
> diff --git a/remote/Makefile.objs b/remote/Makefile.objs
> index a9b2256b2a..2757f5a265 100644
> --- a/remote/Makefile.objs
> +++ b/remote/Makefile.objs
> @@ -1 +1,2 @@
>  remote-pci-obj-$(CONFIG_MPQEMU) += remote-main.o
> +remote-pci-obj-$(CONFIG_MPQEMU) += pcihost.o
> diff --git a/remote/pcihost.c b/remote/pcihost.c
> new file mode 100644
> index 0000000000..dbe081903e
> --- /dev/null
> +++ b/remote/pcihost.c
> @@ -0,0 +1,64 @@
> +/*
> + * Remote PCI host device
> + *
> + * Copyright © 2018, 2020 Oracle and/or its affiliates.
> + *
> + * This work is licensed under the terms of the GNU GPL, version 2 or later.
> + * See the COPYING file in the top-level directory.
> + *
> + */
> +
> +#include <sys/types.h>
> +#include <unistd.h>

"qemu/osdep.h" already includes these headers.

> +#include "qemu/osdep.h"
> +#include "hw/pci/pci.h"
> +#include "hw/pci/pci_host.h"
> +#include "hw/pci/pcie_host.h"
> +#include "hw/qdev-properties.h"
> +#include "remote/pcihost.h"
> +#include "exec/memory.h"
> +
> +static const char *remote_host_root_bus_path(PCIHostState *host_bridge,
> +                                             PCIBus *rootbus)
> +{
> +    return "0000:00";
> +}
> +
> +static void remote_host_realize(DeviceState *dev, Error **errp)
> +{
> +    char *busname = g_strdup_printf("remote-pci-%ld", (unsigned long)getpid());
> +    PCIHostState *pci = PCI_HOST_BRIDGE(dev);
> +    RemPCIHost *s = REMOTE_HOST_DEVICE(dev);
> +
> +    pci->bus = pci_root_bus_new(DEVICE(s), busname,
> +                                s->mr_pci_mem, s->mr_sys_io,
> +                                0, TYPE_PCIE_BUS);
> +}
> +
> +static void remote_host_class_init(ObjectClass *klass, void *data)
> +{
> +    DeviceClass *dc = DEVICE_CLASS(klass);
> +    PCIHostBridgeClass *hc = PCI_HOST_BRIDGE_CLASS(klass);
> +
> +    hc->root_bus_path = remote_host_root_bus_path;
> +    dc->realize = remote_host_realize;
> +
> +    dc->user_creatable = false;
> +    set_bit(DEVICE_CATEGORY_BRIDGE, dc->categories);
> +    dc->fw_name = "pci";
> +}
> +
> +static const TypeInfo remote_host_info = {
> +    .name = TYPE_REMOTE_HOST_DEVICE,
> +    .parent = TYPE_PCIE_HOST_BRIDGE,
> +    .instance_size = sizeof(RemPCIHost),
> +    .class_init = remote_host_class_init,
> +};
> +
> +static void remote_machine_register(void)
> +{
> +    type_register_static(&remote_host_info);
> +}
> +
> +type_init(remote_machine_register)

The naming in this file is inconsistent:

remote_host_root_bus_path -> remote_pcihost_root_bus_path
remote_machine_register -> remote_pcihost_register

I haven't listed all instances.

Patch

diff --git a/MAINTAINERS b/MAINTAINERS
index 93ad693da4..0cda5ee06a 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -2859,6 +2859,8 @@  F: remote/Makefile.objs
 F: remote/remote-main.c
 F: include/io/mpqemu-link.h
 F: io/mpqemu-link.c
+F: include/remote/pcihost.h
+F: remote/pcihost.c
 
 Build and test automation
 -------------------------
diff --git a/hw/pci/Makefile.objs b/hw/pci/Makefile.objs
index 955be54472..90693a7695 100644
--- a/hw/pci/Makefile.objs
+++ b/hw/pci/Makefile.objs
@@ -13,6 +13,6 @@  common-obj-$(CONFIG_PCI_EXPRESS) += pcie_port.o pcie_host.o
 common-obj-$(call lnot,$(CONFIG_PCI)) += pci-stub.o
 common-obj-$(CONFIG_ALL) += pci-stub.o
 
-remote-pci-obj-$(CONFIG_MPQEMU) += pci.o pci_bridge.o
+remote-pci-obj-$(CONFIG_MPQEMU) += pci.o pci_bridge.o pci_host.o pcie_host.o
 remote-pci-obj-$(CONFIG_MPQEMU) += msi.o msix.o
 remote-pci-obj-$(CONFIG_MPQEMU) += pcie.o
diff --git a/include/remote/pcihost.h b/include/remote/pcihost.h
new file mode 100644
index 0000000000..7aca9ccaf1
--- /dev/null
+++ b/include/remote/pcihost.h
@@ -0,0 +1,45 @@ 
+/*
+ * PCI Host for remote device
+ *
+ * Copyright © 2018, 2020 Oracle and/or its affiliates.
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ *
+ */
+
+#ifndef REMOTE_PCIHOST_H
+#define REMOTE_PCIHOST_H
+
+#include <stddef.h>
+#include <stdint.h>
+
+#include "exec/memory.h"
+#include "hw/pci/pcie_host.h"
+
+#define TYPE_REMOTE_HOST_DEVICE "remote-pcihost"
+#define REMOTE_HOST_DEVICE(obj) \
+    OBJECT_CHECK(RemPCIHost, (obj), TYPE_REMOTE_HOST_DEVICE)
+
+typedef struct RemPCIHost {
+    /*< private >*/
+    PCIExpressHost parent_obj;
+    /*< public >*/
+
+    /*
+     * Memory Controller Hub (MCH) may not be necessary for the emulation
+     * program. The two important reasons for implementing a PCI host in the
+     * emulation program are:
+     * - Provide a PCI bus for IO devices
+     * - Enable translation of guest PA to the PCI bar regions
+     *
+     * For both the above mentioned purposes, it doesn't look like we would
+     * need the MCH
+     */
+
+    MemoryRegion *mr_pci_mem;
+    MemoryRegion *mr_sys_mem;
+    MemoryRegion *mr_sys_io;
+} RemPCIHost;
+
+#endif
diff --git a/remote/Makefile.objs b/remote/Makefile.objs
index a9b2256b2a..2757f5a265 100644
--- a/remote/Makefile.objs
+++ b/remote/Makefile.objs
@@ -1 +1,2 @@ 
 remote-pci-obj-$(CONFIG_MPQEMU) += remote-main.o
+remote-pci-obj-$(CONFIG_MPQEMU) += pcihost.o
diff --git a/remote/pcihost.c b/remote/pcihost.c
new file mode 100644
index 0000000000..dbe081903e
--- /dev/null
+++ b/remote/pcihost.c
@@ -0,0 +1,64 @@ 
+/*
+ * Remote PCI host device
+ *
+ * Copyright © 2018, 2020 Oracle and/or its affiliates.
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ *
+ */
+
+#include <sys/types.h>
+#include <unistd.h>
+
+#include "qemu/osdep.h"
+#include "hw/pci/pci.h"
+#include "hw/pci/pci_host.h"
+#include "hw/pci/pcie_host.h"
+#include "hw/qdev-properties.h"
+#include "remote/pcihost.h"
+#include "exec/memory.h"
+
+static const char *remote_host_root_bus_path(PCIHostState *host_bridge,
+                                             PCIBus *rootbus)
+{
+    return "0000:00";
+}
+
+static void remote_host_realize(DeviceState *dev, Error **errp)
+{
+    char *busname = g_strdup_printf("remote-pci-%ld", (unsigned long)getpid());
+    PCIHostState *pci = PCI_HOST_BRIDGE(dev);
+    RemPCIHost *s = REMOTE_HOST_DEVICE(dev);
+
+    pci->bus = pci_root_bus_new(DEVICE(s), busname,
+                                s->mr_pci_mem, s->mr_sys_io,
+                                0, TYPE_PCIE_BUS);
+}
+
+static void remote_host_class_init(ObjectClass *klass, void *data)
+{
+    DeviceClass *dc = DEVICE_CLASS(klass);
+    PCIHostBridgeClass *hc = PCI_HOST_BRIDGE_CLASS(klass);
+
+    hc->root_bus_path = remote_host_root_bus_path;
+    dc->realize = remote_host_realize;
+
+    dc->user_creatable = false;
+    set_bit(DEVICE_CATEGORY_BRIDGE, dc->categories);
+    dc->fw_name = "pci";
+}
+
+static const TypeInfo remote_host_info = {
+    .name = TYPE_REMOTE_HOST_DEVICE,
+    .parent = TYPE_PCIE_HOST_BRIDGE,
+    .instance_size = sizeof(RemPCIHost),
+    .class_init = remote_host_class_init,
+};
+
+static void remote_machine_register(void)
+{
+    type_register_static(&remote_host_info);
+}
+
+type_init(remote_machine_register)