[Qemu-devel] [PATCH V2 0/3] hw/pcie: Introduce Generic PCI Express Root Port

[Qemu-devel] [PATCH V2 0/3] hw/pcie: Introduce Generic PCI Express Root Port

[Marcel Apfelbaum]

v1 -> v2:
 - Rebased on master.

The Generic Root Port behaves the same as the
Intel's IOH device with id 3420, without having
Intel specific attributes.

The device has two purposes:
 (1) Can be used on both X86 and ARM machines.
 (2) It will allow us to tweak the behaviour
    (e.g add vendor-specific PCI capabilities)
     - something that obviously cannot be done
       on a known device.

Patch 1/3: Introduce a base class for Root Ports - most of the code
           is migrated from IOH3420 implementation.
Patch 2/3: Derives the IOH3420 from the new base class
Patch 3/3: Introduces the generic Root Port.

Tested with Linux and Windows guests only on x86 hosts.

Marcel Apfelbaum (3):
  hw/pcie: Introduce a base class for PCI Express Root Ports
  hw/ioh3420: derive from PCI Express Root Port base class
  hw/pcie: Introduce Generic PCI Express Root Port

 default-configs/arm-softmmu.mak    |   1 +
 default-configs/i386-softmmu.mak   |   1 +
 default-configs/x86_64-softmmu.mak |   1 +
 hw/pci-bridge/Makefile.objs        |   1 +
 hw/pci-bridge/ioh3420.c            | 152 ++-----------------------
 hw/pci-bridge/pcie_root_port.c     | 228 +++++++++++++++++++++++++++++++++++++
 include/hw/pci/pci.h               |   1 +
 include/hw/pci/pcie_port.h         |  18 +++
 8 files changed, 258 insertions(+), 145 deletions(-)
 create mode 100644 hw/pci-bridge/pcie_root_port.c

-- 
2.5.5

[Qemu-devel] [PATCH V2 1/3] hw/pcie: Introduce a base class for PCI Express Root Ports

[Marcel Apfelbaum]

The 'base' PCI Express Root Port includes
the common code to be re-used for all
Root Ports implementations. Most of the code
was taken from the current implementation
of Intel's IOH 3420 Root Port.

Signed-off-by: Marcel Apfelbaum <marcel@redhat.com>
---
 default-configs/arm-softmmu.mak    |   1 +
 default-configs/i386-softmmu.mak   |   1 +
 default-configs/x86_64-softmmu.mak |   1 +
 hw/pci-bridge/Makefile.objs        |   1 +
 hw/pci-bridge/pcie_root_port.c     | 194 +++++++++++++++++++++++++++++++++++++
 include/hw/pci/pcie_port.h         |  18 ++++
 6 files changed, 216 insertions(+)
 create mode 100644 hw/pci-bridge/pcie_root_port.c

diff --git a/default-configs/arm-softmmu.mak b/default-configs/arm-softmmu.mak
index 6de3e16..6f2a180 100644
--- a/default-configs/arm-softmmu.mak
+++ b/default-configs/arm-softmmu.mak
@@ -108,6 +108,7 @@ CONFIG_FSL_IMX25=y
 
 CONFIG_IMX_I2C=y
 
+CONFIG_PCIE_PORT=y
 CONFIG_XIO3130=y
 CONFIG_IOH3420=y
 CONFIG_I82801B11=y
diff --git a/default-configs/i386-softmmu.mak b/default-configs/i386-softmmu.mak
index 0b51360..9288838 100644
--- a/default-configs/i386-softmmu.mak
+++ b/default-configs/i386-softmmu.mak
@@ -51,6 +51,7 @@ CONFIG_PVPANIC=y
 CONFIG_MEM_HOTPLUG=y
 CONFIG_NVDIMM=y
 CONFIG_ACPI_NVDIMM=y
+CONFIG_PCIE_PORT=y
 CONFIG_XIO3130=y
 CONFIG_IOH3420=y
 CONFIG_I82801B11=y
diff --git a/default-configs/x86_64-softmmu.mak b/default-configs/x86_64-softmmu.mak
index 7f89503..7d2c2d4 100644
--- a/default-configs/x86_64-softmmu.mak
+++ b/default-configs/x86_64-softmmu.mak
@@ -51,6 +51,7 @@ CONFIG_PVPANIC=y
 CONFIG_MEM_HOTPLUG=y
 CONFIG_NVDIMM=y
 CONFIG_ACPI_NVDIMM=y
+CONFIG_PCIE_PORT=y
 CONFIG_XIO3130=y
 CONFIG_IOH3420=y
 CONFIG_I82801B11=y
diff --git a/hw/pci-bridge/Makefile.objs b/hw/pci-bridge/Makefile.objs
index f2adfe3..4f2039f 100644
--- a/hw/pci-bridge/Makefile.objs
+++ b/hw/pci-bridge/Makefile.objs
@@ -1,5 +1,6 @@
 common-obj-y += pci_bridge_dev.o
 common-obj-y += pci_expander_bridge.o
+common-obj-$(CONFIG_PCIE_PORT) += pcie_root_port.o
 common-obj-$(CONFIG_XIO3130) += xio3130_upstream.o xio3130_downstream.o
 common-obj-$(CONFIG_IOH3420) += ioh3420.o
 common-obj-$(CONFIG_I82801B11) += i82801b11.o
diff --git a/hw/pci-bridge/pcie_root_port.c b/hw/pci-bridge/pcie_root_port.c
new file mode 100644
index 0000000..e84ae14
--- /dev/null
+++ b/hw/pci-bridge/pcie_root_port.c
@@ -0,0 +1,194 @@
+/*
+ * Generic PCI Express Root Port emulation
+ *
+ * Copyright (C) 2016 Red Hat Inc
+ *
+ * Authors:
+ *   Marcel Apfelbaum <marcel@redhat.com>
+ *
+ * Most of the code was migrated from hw/pci-bridge/ioh3420.
+ *
+ * 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 "qemu/osdep.h"
+#include "qapi/error.h"
+#include "hw/pci/pcie_port.h"
+#include "hw/pci/msi.h"
+
+#define PCIE_ROOT_PORT_MSI_VECTORS              2
+#define PCIE_ROOT_PORT_MSI_SUPPORTED_FLAGS      PCI_MSI_FLAGS_MASKBIT
+#define PCIE_ROOT_PORT_AER_OFFSET               0x100
+
+/*
+ * If two MSI vector are allocated, Advanced Error Interrupt Message Number
+ * is 1. otherwise 0.
+ * 17.12.5.10 RPERRSTS,  32:27 bit Advanced Error Interrupt Message Number.
+ */
+static uint8_t rp_aer_vector(const PCIDevice *d)
+{
+    switch (msi_nr_vectors_allocated(d)) {
+    case 1:
+        return 0;
+    case 2:
+        return 1;
+    case 4:
+    case 8:
+    case 16:
+    case 32:
+    default:
+        break;
+    }
+    abort();
+    return 0;
+}
+
+static void rp_aer_vector_update(PCIDevice *d)
+{
+    pcie_aer_root_set_vector(d, rp_aer_vector(d));
+}
+
+static void rp_write_config(PCIDevice *d, uint32_t address,
+                            uint32_t val, int len)
+{
+    uint32_t root_cmd =
+        pci_get_long(d->config + d->exp.aer_cap + PCI_ERR_ROOT_COMMAND);
+
+    pci_bridge_write_config(d, address, val, len);
+    rp_aer_vector_update(d);
+    pcie_cap_slot_write_config(d, address, val, len);
+    pcie_aer_write_config(d, address, val, len);
+    pcie_aer_root_write_config(d, address, val, len, root_cmd);
+}
+
+static void rp_reset(DeviceState *qdev)
+{
+    PCIDevice *d = PCI_DEVICE(qdev);
+
+    rp_aer_vector_update(d);
+    pcie_cap_root_reset(d);
+    pcie_cap_deverr_reset(d);
+    pcie_cap_slot_reset(d);
+    pcie_cap_arifwd_reset(d);
+    pcie_aer_root_reset(d);
+    pci_bridge_reset(qdev);
+    pci_bridge_disable_base_limit(d);
+}
+
+static void rp_realize(PCIDevice *d, Error **errp)
+{
+    PCIEPort *p = PCIE_PORT(d);
+    PCIESlot *s = PCIE_SLOT(d);
+    PCIDeviceClass *dc = PCI_DEVICE_GET_CLASS(d);
+    PCIERootPortClass *rpc = PCIE_ROOT_PORT_GET_CLASS(d);
+    int rc;
+    Error *local_err = NULL;
+
+    pci_config_set_interrupt_pin(d->config, 1);
+    pci_bridge_initfn(d, TYPE_PCIE_BUS);
+    pcie_port_init_reg(d);
+
+    rc = pci_bridge_ssvid_init(d, rpc->ssvid_offset, dc->vendor_id, rpc->ssid);
+    if (rc < 0) {
+        error_setg(errp, "Can't init SSV ID, error %d", rc);
+        goto err_bridge;
+    }
+
+    rc = msi_init(d, rpc->msi_offset, PCIE_ROOT_PORT_MSI_VECTORS,
+                  PCIE_ROOT_PORT_MSI_SUPPORTED_FLAGS & PCI_MSI_FLAGS_64BIT,
+                  PCIE_ROOT_PORT_MSI_SUPPORTED_FLAGS & PCI_MSI_FLAGS_MASKBIT,
+                  &local_err);
+    if (rc < 0) {
+        assert(rc == -ENOTSUP);
+        error_propagate(errp, local_err);
+        goto err_bridge;
+    }
+
+    rc = pcie_cap_init(d, rpc->exp_offset, PCI_EXP_TYPE_ROOT_PORT, p->port);
+    if (rc < 0) {
+        error_setg(errp, "Can't add Root Port capability, error %d", rc);
+        goto err_msi;
+    }
+
+    pcie_cap_arifwd_init(d);
+    pcie_cap_deverr_init(d);
+    pcie_cap_slot_init(d, s->slot);
+    pcie_cap_root_init(d);
+
+    pcie_chassis_create(s->chassis);
+    rc = pcie_chassis_add_slot(s);
+    if (rc < 0) {
+        error_setg(errp, "Can't add chassis slot, error %d", rc);
+        goto err_pcie_cap;
+    }
+
+    rc = pcie_aer_init(d, PCI_ERR_VER, rpc->aer_offset,
+                       PCI_ERR_SIZEOF, &local_err);
+    if (rc < 0) {
+        error_propagate(errp, local_err);
+        goto err;
+    }
+    pcie_aer_root_init(d);
+    rp_aer_vector_update(d);
+
+    return;
+
+err:
+    pcie_chassis_del_slot(s);
+err_pcie_cap:
+    pcie_cap_exit(d);
+err_msi:
+    msi_uninit(d);
+err_bridge:
+    pci_bridge_exitfn(d);
+}
+
+static void rp_exit(PCIDevice *d)
+{
+    PCIESlot *s = PCIE_SLOT(d);
+
+    pcie_aer_exit(d);
+    pcie_chassis_del_slot(s);
+    pcie_cap_exit(d);
+    msi_uninit(d);
+    pci_bridge_exitfn(d);
+}
+
+static Property rp_props[] = {
+    DEFINE_PROP_BIT(COMPAT_PROP_PCP, PCIDevice, cap_present,
+                    QEMU_PCIE_SLTCAP_PCP_BITNR, true),
+    DEFINE_PROP_END_OF_LIST()
+};
+
+static void rp_class_init(ObjectClass *klass, void *data)
+{
+    DeviceClass *dc = DEVICE_CLASS(klass);
+    PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
+    PCIERootPortClass *rpc = PCIE_ROOT_PORT_CLASS(klass);
+
+    k->is_express = 1;
+    k->is_bridge = 1;
+    k->config_write = rp_write_config;
+    k->realize = rp_realize;
+    k->exit = rp_exit;
+    set_bit(DEVICE_CATEGORY_BRIDGE, dc->categories);
+    dc->reset = rp_reset;
+    dc->props = rp_props;
+    rpc->aer_offset = PCIE_ROOT_PORT_AER_OFFSET;
+}
+
+static const TypeInfo rp_info = {
+    .name          = TYPE_PCIE_ROOT_PORT,
+    .parent        = TYPE_PCIE_SLOT,
+    .class_init    = rp_class_init,
+    .abstract      = true,
+    .class_size = sizeof(PCIERootPortClass),
+};
+
+static void rp_register_types(void)
+{
+    type_register_static(&rp_info);
+}
+
+type_init(rp_register_types)
diff --git a/include/hw/pci/pcie_port.h b/include/hw/pci/pcie_port.h
index f7b64db..a8eee56 100644
--- a/include/hw/pci/pcie_port.h
+++ b/include/hw/pci/pcie_port.h
@@ -57,4 +57,22 @@ PCIESlot *pcie_chassis_find_slot(uint8_t chassis, uint16_t slot);
 int pcie_chassis_add_slot(struct PCIESlot *slot);
 void pcie_chassis_del_slot(PCIESlot *s);
 
+#define TYPE_PCIE_ROOT_PORT         "pcie-root-port-base"
+#define PCIE_ROOT_PORT_CLASS(klass) \
+     OBJECT_CLASS_CHECK(PCIERootPortClass, (klass), TYPE_PCIE_ROOT_PORT)
+#define PCIE_ROOT_PORT_CLASS(klass) \
+     OBJECT_CLASS_CHECK(PCIERootPortClass, (klass), TYPE_PCIE_ROOT_PORT)
+#define PCIE_ROOT_PORT_GET_CLASS(obj) \
+     OBJECT_GET_CLASS(PCIERootPortClass, (obj), TYPE_PCIE_ROOT_PORT)
+
+typedef struct PCIERootPortClass {
+    PCIDeviceClass parent_class;
+
+    int msi_offset;
+    int exp_offset;
+    int aer_offset;
+    int ssvid_offset;
+    int ssid;
+} PCIERootPortClass;
+
 #endif /* QEMU_PCIE_PORT_H */
-- 
2.5.5

[Qemu-devel] [PATCH V2 2/3] hw/ioh3420: derive from PCI Express Root Port base class

[Marcel Apfelbaum]

Preserve only Intel specific details.

Signed-off-by: Marcel Apfelbaum <marcel@redhat.com>
---
 hw/pci-bridge/ioh3420.c | 152 +++---------------------------------------------
 1 file changed, 7 insertions(+), 145 deletions(-)

diff --git a/hw/pci-bridge/ioh3420.c b/hw/pci-bridge/ioh3420.c
index 84b7946..431266c 100644
--- a/hw/pci-bridge/ioh3420.c
+++ b/hw/pci-bridge/ioh3420.c
@@ -38,142 +38,6 @@
 #define IOH_EP_EXP_OFFSET               0x90
 #define IOH_EP_AER_OFFSET               0x100
 
-/*
- * If two MSI vector are allocated, Advanced Error Interrupt Message Number
- * is 1. otherwise 0.
- * 17.12.5.10 RPERRSTS,  32:27 bit Advanced Error Interrupt Message Number.
- */
-static uint8_t ioh3420_aer_vector(const PCIDevice *d)
-{
-    switch (msi_nr_vectors_allocated(d)) {
-    case 1:
-        return 0;
-    case 2:
-        return 1;
-    case 4:
-    case 8:
-    case 16:
-    case 32:
-    default:
-        break;
-    }
-    abort();
-    return 0;
-}
-
-static void ioh3420_aer_vector_update(PCIDevice *d)
-{
-    pcie_aer_root_set_vector(d, ioh3420_aer_vector(d));
-}
-
-static void ioh3420_write_config(PCIDevice *d,
-                                   uint32_t address, uint32_t val, int len)
-{
-    uint32_t root_cmd =
-        pci_get_long(d->config + d->exp.aer_cap + PCI_ERR_ROOT_COMMAND);
-
-    pci_bridge_write_config(d, address, val, len);
-    ioh3420_aer_vector_update(d);
-    pcie_cap_slot_write_config(d, address, val, len);
-    pcie_aer_write_config(d, address, val, len);
-    pcie_aer_root_write_config(d, address, val, len, root_cmd);
-}
-
-static void ioh3420_reset(DeviceState *qdev)
-{
-    PCIDevice *d = PCI_DEVICE(qdev);
-
-    ioh3420_aer_vector_update(d);
-    pcie_cap_root_reset(d);
-    pcie_cap_deverr_reset(d);
-    pcie_cap_slot_reset(d);
-    pcie_cap_arifwd_reset(d);
-    pcie_aer_root_reset(d);
-    pci_bridge_reset(qdev);
-    pci_bridge_disable_base_limit(d);
-}
-
-static int ioh3420_initfn(PCIDevice *d)
-{
-    PCIEPort *p = PCIE_PORT(d);
-    PCIESlot *s = PCIE_SLOT(d);
-    int rc;
-    Error *err = NULL;
-
-    pci_config_set_interrupt_pin(d->config, 1);
-    pci_bridge_initfn(d, TYPE_PCIE_BUS);
-    pcie_port_init_reg(d);
-
-    rc = pci_bridge_ssvid_init(d, IOH_EP_SSVID_OFFSET,
-                               IOH_EP_SSVID_SVID, IOH_EP_SSVID_SSID);
-    if (rc < 0) {
-        goto err_bridge;
-    }
-
-    rc = msi_init(d, IOH_EP_MSI_OFFSET, IOH_EP_MSI_NR_VECTOR,
-                  IOH_EP_MSI_SUPPORTED_FLAGS & PCI_MSI_FLAGS_64BIT,
-                  IOH_EP_MSI_SUPPORTED_FLAGS & PCI_MSI_FLAGS_MASKBIT, &err);
-    if (rc < 0) {
-        assert(rc == -ENOTSUP);
-        error_report_err(err);
-        goto err_bridge;
-    }
-
-    rc = pcie_cap_init(d, IOH_EP_EXP_OFFSET, PCI_EXP_TYPE_ROOT_PORT, p->port);
-    if (rc < 0) {
-        goto err_msi;
-    }
-
-    pcie_cap_arifwd_init(d);
-    pcie_cap_deverr_init(d);
-    pcie_cap_slot_init(d, s->slot);
-    pcie_cap_root_init(d);
-
-    pcie_chassis_create(s->chassis);
-    rc = pcie_chassis_add_slot(s);
-    if (rc < 0) {
-        goto err_pcie_cap;
-    }
-
-    rc = pcie_aer_init(d, PCI_ERR_VER, IOH_EP_AER_OFFSET,
-                       PCI_ERR_SIZEOF, &err);
-    if (rc < 0) {
-        error_report_err(err);
-        goto err;
-    }
-    pcie_aer_root_init(d);
-    ioh3420_aer_vector_update(d);
-
-    return 0;
-
-err:
-    pcie_chassis_del_slot(s);
-err_pcie_cap:
-    pcie_cap_exit(d);
-err_msi:
-    msi_uninit(d);
-err_bridge:
-    pci_bridge_exitfn(d);
-    return rc;
-}
-
-static void ioh3420_exitfn(PCIDevice *d)
-{
-    PCIESlot *s = PCIE_SLOT(d);
-
-    pcie_aer_exit(d);
-    pcie_chassis_del_slot(s);
-    pcie_cap_exit(d);
-    msi_uninit(d);
-    pci_bridge_exitfn(d);
-}
-
-static Property ioh3420_props[] = {
-    DEFINE_PROP_BIT(COMPAT_PROP_PCP, PCIDevice, cap_present,
-                    QEMU_PCIE_SLTCAP_PCP_BITNR, true),
-    DEFINE_PROP_END_OF_LIST()
-};
-
 static const VMStateDescription vmstate_ioh3420 = {
     .name = "ioh-3240-express-root-port",
     .version_id = 1,
@@ -191,25 +55,23 @@ static void ioh3420_class_init(ObjectClass *klass, void *data)
 {
     DeviceClass *dc = DEVICE_CLASS(klass);
     PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
+    PCIERootPortClass *rpc = PCIE_ROOT_PORT_CLASS(klass);
 
-    k->is_express = 1;
-    k->is_bridge = 1;
-    k->config_write = ioh3420_write_config;
-    k->init = ioh3420_initfn;
-    k->exit = ioh3420_exitfn;
     k->vendor_id = PCI_VENDOR_ID_INTEL;
     k->device_id = PCI_DEVICE_ID_IOH_EPORT;
     k->revision = PCI_DEVICE_ID_IOH_REV;
-    set_bit(DEVICE_CATEGORY_BRIDGE, dc->categories);
     dc->desc = "Intel IOH device id 3420 PCIE Root Port";
-    dc->reset = ioh3420_reset;
     dc->vmsd = &vmstate_ioh3420;
-    dc->props = ioh3420_props;
+    rpc->msi_offset = IOH_EP_MSI_OFFSET;
+    rpc->exp_offset = IOH_EP_EXP_OFFSET;
+    rpc->aer_offset = IOH_EP_AER_OFFSET;
+    rpc->ssvid_offset = IOH_EP_SSVID_OFFSET;
+    rpc->ssid = IOH_EP_SSVID_SSID;
 }
 
 static const TypeInfo ioh3420_info = {
     .name          = "ioh3420",
-    .parent        = TYPE_PCIE_SLOT,
+    .parent        = TYPE_PCIE_ROOT_PORT,
     .class_init    = ioh3420_class_init,
 };
 
-- 
2.5.5

[Qemu-devel] [PATCH V2 3/3] hw/pcie: Introduce Generic PCI Express Root Port

[Marcel Apfelbaum]

The Generic Root Port behaves the same as the
Intel's IOH device with id 3420, without having
Intel specific attributes.

The device has two purposes:
 (1) Can be used on both X86 and ARM machines.
 (2) It will allow us to tweak the behaviour
    (e.g add vendor-specific PCI capabilities)
     - something that obviously cannot be done
       on a known device.

Signed-off-by: Marcel Apfelbaum <marcel@redhat.com>
---
 hw/pci-bridge/pcie_root_port.c | 34 ++++++++++++++++++++++++++++++++++
 include/hw/pci/pci.h           |  1 +
 2 files changed, 35 insertions(+)

diff --git a/hw/pci-bridge/pcie_root_port.c b/hw/pci-bridge/pcie_root_port.c
index e84ae14..32c6201 100644
--- a/hw/pci-bridge/pcie_root_port.c
+++ b/hw/pci-bridge/pcie_root_port.c
@@ -21,6 +21,9 @@
 #define PCIE_ROOT_PORT_MSI_SUPPORTED_FLAGS      PCI_MSI_FLAGS_MASKBIT
 #define PCIE_ROOT_PORT_AER_OFFSET               0x100
 
+#define TYPE_PCIE_ROOT_PORT_DEV                 "pcie-root-port"
+
+
 /*
  * If two MSI vector are allocated, Advanced Error Interrupt Message Number
  * is 1. otherwise 0.
@@ -186,9 +189,40 @@ static const TypeInfo rp_info = {
     .class_size = sizeof(PCIERootPortClass),
 };
 
+static const VMStateDescription vmstate_rp_dev = {
+    .name = "pcie-root-port",
+    .version_id = 1,
+    .minimum_version_id = 1,
+    .post_load = pcie_cap_slot_post_load,
+    .fields = (VMStateField[]) {
+        VMSTATE_PCIE_DEVICE(parent_obj.parent_obj.parent_obj, PCIESlot),
+        VMSTATE_STRUCT(parent_obj.parent_obj.parent_obj.exp.aer_log,
+                       PCIESlot, 0, vmstate_pcie_aer_log, PCIEAERLog),
+        VMSTATE_END_OF_LIST()
+    }
+};
+
+static void rp_dev_class_init(ObjectClass *klass, void *data)
+{
+    DeviceClass *dc = DEVICE_CLASS(klass);
+    PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
+
+    k->vendor_id = PCI_VENDOR_ID_REDHAT;
+    k->device_id = PCI_DEVICE_ID_REDHAT_PCIE_RP;
+    dc->desc = "PCI Express Root Port";
+    dc->vmsd = &vmstate_rp_dev;
+}
+
+static const TypeInfo rp_dev_info = {
+    .name          = TYPE_PCIE_ROOT_PORT_DEV,
+    .parent        = TYPE_PCIE_ROOT_PORT,
+    .class_init    = rp_dev_class_init,
+};
+
 static void rp_register_types(void)
 {
     type_register_static(&rp_info);
+    type_register_static(&rp_dev_info);
 }
 
 type_init(rp_register_types)
diff --git a/include/hw/pci/pci.h b/include/hw/pci/pci.h
index 772692f..cbc1fdf 100644
--- a/include/hw/pci/pci.h
+++ b/include/hw/pci/pci.h
@@ -96,6 +96,7 @@
 #define PCI_DEVICE_ID_REDHAT_PXB         0x0009
 #define PCI_DEVICE_ID_REDHAT_BRIDGE_SEAT 0x000a
 #define PCI_DEVICE_ID_REDHAT_PXB_PCIE    0x000b
+#define PCI_DEVICE_ID_REDHAT_PCIE_RP     0x000c
 #define PCI_DEVICE_ID_REDHAT_QXL         0x0100
 
 #define FMT_PCIBUS                      PRIx64
-- 
2.5.5

Re: [Qemu-devel] [PATCH V2 1/3] hw/pcie: Introduce a base class for PCI Express Root Ports

[Michael S. Tsirkin]

On Wed, Jan 11, 2017 at 02:18:54PM +0200, Marcel Apfelbaum wrote:
> The 'base' PCI Express Root Port includes
> the common code to be re-used for all
> Root Ports implementations. Most of the code
> was taken from the current implementation
> of Intel's IOH 3420 Root Port.
> 
> Signed-off-by: Marcel Apfelbaum <marcel@redhat.com>
> ---
>  default-configs/arm-softmmu.mak    |   1 +
>  default-configs/i386-softmmu.mak   |   1 +
>  default-configs/x86_64-softmmu.mak |   1 +
>  hw/pci-bridge/Makefile.objs        |   1 +
>  hw/pci-bridge/pcie_root_port.c     | 194 +++++++++++++++++++++++++++++++++++++
>  include/hw/pci/pcie_port.h         |  18 ++++
>  6 files changed, 216 insertions(+)
>  create mode 100644 hw/pci-bridge/pcie_root_port.c
> 
> diff --git a/default-configs/arm-softmmu.mak b/default-configs/arm-softmmu.mak
> index 6de3e16..6f2a180 100644
> --- a/default-configs/arm-softmmu.mak
> +++ b/default-configs/arm-softmmu.mak
> @@ -108,6 +108,7 @@ CONFIG_FSL_IMX25=y
>  
>  CONFIG_IMX_I2C=y
>  
> +CONFIG_PCIE_PORT=y
>  CONFIG_XIO3130=y
>  CONFIG_IOH3420=y
>  CONFIG_I82801B11=y
> diff --git a/default-configs/i386-softmmu.mak b/default-configs/i386-softmmu.mak
> index 0b51360..9288838 100644
> --- a/default-configs/i386-softmmu.mak
> +++ b/default-configs/i386-softmmu.mak
> @@ -51,6 +51,7 @@ CONFIG_PVPANIC=y
>  CONFIG_MEM_HOTPLUG=y
>  CONFIG_NVDIMM=y
>  CONFIG_ACPI_NVDIMM=y
> +CONFIG_PCIE_PORT=y
>  CONFIG_XIO3130=y
>  CONFIG_IOH3420=y
>  CONFIG_I82801B11=y
> diff --git a/default-configs/x86_64-softmmu.mak b/default-configs/x86_64-softmmu.mak
> index 7f89503..7d2c2d4 100644
> --- a/default-configs/x86_64-softmmu.mak
> +++ b/default-configs/x86_64-softmmu.mak
> @@ -51,6 +51,7 @@ CONFIG_PVPANIC=y
>  CONFIG_MEM_HOTPLUG=y
>  CONFIG_NVDIMM=y
>  CONFIG_ACPI_NVDIMM=y
> +CONFIG_PCIE_PORT=y
>  CONFIG_XIO3130=y
>  CONFIG_IOH3420=y
>  CONFIG_I82801B11=y
> diff --git a/hw/pci-bridge/Makefile.objs b/hw/pci-bridge/Makefile.objs
> index f2adfe3..4f2039f 100644
> --- a/hw/pci-bridge/Makefile.objs
> +++ b/hw/pci-bridge/Makefile.objs
> @@ -1,5 +1,6 @@
>  common-obj-y += pci_bridge_dev.o
>  common-obj-y += pci_expander_bridge.o
> +common-obj-$(CONFIG_PCIE_PORT) += pcie_root_port.o
>  common-obj-$(CONFIG_XIO3130) += xio3130_upstream.o xio3130_downstream.o
>  common-obj-$(CONFIG_IOH3420) += ioh3420.o
>  common-obj-$(CONFIG_I82801B11) += i82801b11.o
> diff --git a/hw/pci-bridge/pcie_root_port.c b/hw/pci-bridge/pcie_root_port.c
> new file mode 100644
> index 0000000..e84ae14
> --- /dev/null
> +++ b/hw/pci-bridge/pcie_root_port.c
> @@ -0,0 +1,194 @@
> +/*
> + * Generic PCI Express Root Port emulation
> + *
> + * Copyright (C) 2016 Red Hat Inc
> + *
> + * Authors:
> + *   Marcel Apfelbaum <marcel@redhat.com>
> + *
> + * Most of the code was migrated from hw/pci-bridge/ioh3420.
> + *
> + * 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 "qemu/osdep.h"
> +#include "qapi/error.h"
> +#include "hw/pci/pcie_port.h"
> +#include "hw/pci/msi.h"
> +
> +#define PCIE_ROOT_PORT_MSI_VECTORS              2
> +#define PCIE_ROOT_PORT_MSI_SUPPORTED_FLAGS      PCI_MSI_FLAGS_MASKBIT
> +#define PCIE_ROOT_PORT_AER_OFFSET               0x100

This isn't generic either.

> +
> +/*
> + * If two MSI vector


MSI vectors

> are allocated, Advanced Error Interrupt Message Number
> + * is 1. otherwise 0.
> + * 17.12.5.10 RPERRSTS,  32:27 bit Advanced Error Interrupt Message Number.

Hmm I see it in
7.10.10. Root Error Status Register (Offset 30h)

which spec is this number from?


> + */
> +static uint8_t rp_aer_vector(const PCIDevice *d)
> +{
> +    switch (msi_nr_vectors_allocated(d)) {
> +    case 1:
> +        return 0;
> +    case 2:
> +        return 1;
> +    case 4:
> +    case 8:
> +    case 16:
> +    case 32:

what are these doing here?

> +    default:
> +        break;
> +    }
> +    abort();
> +    return 0;
> +}

I don't believe it's a generic thing. This is just how
intel did it, isn't it?


> +
> +static void rp_aer_vector_update(PCIDevice *d)
> +{
> +    pcie_aer_root_set_vector(d, rp_aer_vector(d));
> +}
> +
> +static void rp_write_config(PCIDevice *d, uint32_t address,
> +                            uint32_t val, int len)
> +{
> +    uint32_t root_cmd =
> +        pci_get_long(d->config + d->exp.aer_cap + PCI_ERR_ROOT_COMMAND);
> +
> +    pci_bridge_write_config(d, address, val, len);
> +    rp_aer_vector_update(d);
> +    pcie_cap_slot_write_config(d, address, val, len);
> +    pcie_aer_write_config(d, address, val, len);
> +    pcie_aer_root_write_config(d, address, val, len, root_cmd);
> +}
> +
> +static void rp_reset(DeviceState *qdev)
> +{
> +    PCIDevice *d = PCI_DEVICE(qdev);
> +
> +    rp_aer_vector_update(d);
> +    pcie_cap_root_reset(d);
> +    pcie_cap_deverr_reset(d);
> +    pcie_cap_slot_reset(d);
> +    pcie_cap_arifwd_reset(d);
> +    pcie_aer_root_reset(d);
> +    pci_bridge_reset(qdev);
> +    pci_bridge_disable_base_limit(d);
> +}
> +
> +static void rp_realize(PCIDevice *d, Error **errp)
> +{
> +    PCIEPort *p = PCIE_PORT(d);
> +    PCIESlot *s = PCIE_SLOT(d);
> +    PCIDeviceClass *dc = PCI_DEVICE_GET_CLASS(d);
> +    PCIERootPortClass *rpc = PCIE_ROOT_PORT_GET_CLASS(d);
> +    int rc;
> +    Error *local_err = NULL;
> +
> +    pci_config_set_interrupt_pin(d->config, 1);
> +    pci_bridge_initfn(d, TYPE_PCIE_BUS);
> +    pcie_port_init_reg(d);
> +
> +    rc = pci_bridge_ssvid_init(d, rpc->ssvid_offset, dc->vendor_id, rpc->ssid);
> +    if (rc < 0) {
> +        error_setg(errp, "Can't init SSV ID, error %d", rc);
> +        goto err_bridge;
> +    }
> +
> +    rc = msi_init(d, rpc->msi_offset, PCIE_ROOT_PORT_MSI_VECTORS,
> +                  PCIE_ROOT_PORT_MSI_SUPPORTED_FLAGS & PCI_MSI_FLAGS_64BIT,
> +                  PCIE_ROOT_PORT_MSI_SUPPORTED_FLAGS & PCI_MSI_FLAGS_MASKBIT,
> +                  &local_err);
> +    if (rc < 0) {
> +        assert(rc == -ENOTSUP);
> +        error_propagate(errp, local_err);
> +        goto err_bridge;
> +    }
> +
> +    rc = pcie_cap_init(d, rpc->exp_offset, PCI_EXP_TYPE_ROOT_PORT, p->port);
> +    if (rc < 0) {
> +        error_setg(errp, "Can't add Root Port capability, error %d", rc);
> +        goto err_msi;
> +    }
> +
> +    pcie_cap_arifwd_init(d);
> +    pcie_cap_deverr_init(d);
> +    pcie_cap_slot_init(d, s->slot);
> +    pcie_cap_root_init(d);
> +
> +    pcie_chassis_create(s->chassis);
> +    rc = pcie_chassis_add_slot(s);
> +    if (rc < 0) {
> +        error_setg(errp, "Can't add chassis slot, error %d", rc);
> +        goto err_pcie_cap;
> +    }
> +
> +    rc = pcie_aer_init(d, PCI_ERR_VER, rpc->aer_offset,
> +                       PCI_ERR_SIZEOF, &local_err);
> +    if (rc < 0) {
> +        error_propagate(errp, local_err);
> +        goto err;
> +    }
> +    pcie_aer_root_init(d);
> +    rp_aer_vector_update(d);
> +
> +    return;
> +
> +err:
> +    pcie_chassis_del_slot(s);
> +err_pcie_cap:
> +    pcie_cap_exit(d);
> +err_msi:
> +    msi_uninit(d);
> +err_bridge:
> +    pci_bridge_exitfn(d);
> +}
> +
> +static void rp_exit(PCIDevice *d)
> +{
> +    PCIESlot *s = PCIE_SLOT(d);
> +
> +    pcie_aer_exit(d);
> +    pcie_chassis_del_slot(s);
> +    pcie_cap_exit(d);
> +    msi_uninit(d);
> +    pci_bridge_exitfn(d);
> +}
> +
> +static Property rp_props[] = {
> +    DEFINE_PROP_BIT(COMPAT_PROP_PCP, PCIDevice, cap_present,
> +                    QEMU_PCIE_SLTCAP_PCP_BITNR, true),
> +    DEFINE_PROP_END_OF_LIST()
> +};
> +
> +static void rp_class_init(ObjectClass *klass, void *data)
> +{
> +    DeviceClass *dc = DEVICE_CLASS(klass);
> +    PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
> +    PCIERootPortClass *rpc = PCIE_ROOT_PORT_CLASS(klass);
> +
> +    k->is_express = 1;
> +    k->is_bridge = 1;
> +    k->config_write = rp_write_config;
> +    k->realize = rp_realize;
> +    k->exit = rp_exit;
> +    set_bit(DEVICE_CATEGORY_BRIDGE, dc->categories);
> +    dc->reset = rp_reset;
> +    dc->props = rp_props;
> +    rpc->aer_offset = PCIE_ROOT_PORT_AER_OFFSET;
> +}
> +
> +static const TypeInfo rp_info = {
> +    .name          = TYPE_PCIE_ROOT_PORT,
> +    .parent        = TYPE_PCIE_SLOT,
> +    .class_init    = rp_class_init,
> +    .abstract      = true,
> +    .class_size = sizeof(PCIERootPortClass),
> +};
> +
> +static void rp_register_types(void)
> +{
> +    type_register_static(&rp_info);
> +}
> +
> +type_init(rp_register_types)
> diff --git a/include/hw/pci/pcie_port.h b/include/hw/pci/pcie_port.h
> index f7b64db..a8eee56 100644
> --- a/include/hw/pci/pcie_port.h
> +++ b/include/hw/pci/pcie_port.h
> @@ -57,4 +57,22 @@ PCIESlot *pcie_chassis_find_slot(uint8_t chassis, uint16_t slot);
>  int pcie_chassis_add_slot(struct PCIESlot *slot);
>  void pcie_chassis_del_slot(PCIESlot *s);
>  
> +#define TYPE_PCIE_ROOT_PORT         "pcie-root-port-base"
> +#define PCIE_ROOT_PORT_CLASS(klass) \
> +     OBJECT_CLASS_CHECK(PCIERootPortClass, (klass), TYPE_PCIE_ROOT_PORT)
> +#define PCIE_ROOT_PORT_CLASS(klass) \
> +     OBJECT_CLASS_CHECK(PCIERootPortClass, (klass), TYPE_PCIE_ROOT_PORT)
> +#define PCIE_ROOT_PORT_GET_CLASS(obj) \
> +     OBJECT_GET_CLASS(PCIERootPortClass, (obj), TYPE_PCIE_ROOT_PORT)
> +
> +typedef struct PCIERootPortClass {
> +    PCIDeviceClass parent_class;
> +
> +    int msi_offset;
> +    int exp_offset;
> +    int aer_offset;
> +    int ssvid_offset;
> +    int ssid;
> +} PCIERootPortClass;
> +
>  #endif /* QEMU_PCIE_PORT_H */
> -- 
> 2.5.5

Re: [Qemu-devel] [PATCH V2 1/3] hw/pcie: Introduce a base class for PCI Express Root Ports

[Marcel Apfelbaum]

On 01/12/2017 05:36 PM, Michael S. Tsirkin wrote:
> On Wed, Jan 11, 2017 at 02:18:54PM +0200, Marcel Apfelbaum wrote:
>> The 'base' PCI Express Root Port includes
>> the common code to be re-used for all
>> Root Ports implementations. Most of the code
>> was taken from the current implementation
>> of Intel's IOH 3420 Root Port.
>>
>> Signed-off-by: Marcel Apfelbaum <marcel@redhat.com>
>> ---
>>  default-configs/arm-softmmu.mak    |   1 +
>>  default-configs/i386-softmmu.mak   |   1 +
>>  default-configs/x86_64-softmmu.mak |   1 +
>>  hw/pci-bridge/Makefile.objs        |   1 +
>>  hw/pci-bridge/pcie_root_port.c     | 194 +++++++++++++++++++++++++++++++++++++
>>  include/hw/pci/pcie_port.h         |  18 ++++
>>  6 files changed, 216 insertions(+)
>>  create mode 100644 hw/pci-bridge/pcie_root_port.c
>>
>> diff --git a/default-configs/arm-softmmu.mak b/default-configs/arm-softmmu.mak
>> index 6de3e16..6f2a180 100644
>> --- a/default-configs/arm-softmmu.mak
>> +++ b/default-configs/arm-softmmu.mak
>> @@ -108,6 +108,7 @@ CONFIG_FSL_IMX25=y
>>
>>  CONFIG_IMX_I2C=y
>>
>> +CONFIG_PCIE_PORT=y
>>  CONFIG_XIO3130=y
>>  CONFIG_IOH3420=y
>>  CONFIG_I82801B11=y
>> diff --git a/default-configs/i386-softmmu.mak b/default-configs/i386-softmmu.mak
>> index 0b51360..9288838 100644
>> --- a/default-configs/i386-softmmu.mak
>> +++ b/default-configs/i386-softmmu.mak
>> @@ -51,6 +51,7 @@ CONFIG_PVPANIC=y
>>  CONFIG_MEM_HOTPLUG=y
>>  CONFIG_NVDIMM=y
>>  CONFIG_ACPI_NVDIMM=y
>> +CONFIG_PCIE_PORT=y
>>  CONFIG_XIO3130=y
>>  CONFIG_IOH3420=y
>>  CONFIG_I82801B11=y
>> diff --git a/default-configs/x86_64-softmmu.mak b/default-configs/x86_64-softmmu.mak
>> index 7f89503..7d2c2d4 100644
>> --- a/default-configs/x86_64-softmmu.mak
>> +++ b/default-configs/x86_64-softmmu.mak
>> @@ -51,6 +51,7 @@ CONFIG_PVPANIC=y
>>  CONFIG_MEM_HOTPLUG=y
>>  CONFIG_NVDIMM=y
>>  CONFIG_ACPI_NVDIMM=y
>> +CONFIG_PCIE_PORT=y
>>  CONFIG_XIO3130=y
>>  CONFIG_IOH3420=y
>>  CONFIG_I82801B11=y
>> diff --git a/hw/pci-bridge/Makefile.objs b/hw/pci-bridge/Makefile.objs
>> index f2adfe3..4f2039f 100644
>> --- a/hw/pci-bridge/Makefile.objs
>> +++ b/hw/pci-bridge/Makefile.objs
>> @@ -1,5 +1,6 @@
>>  common-obj-y += pci_bridge_dev.o
>>  common-obj-y += pci_expander_bridge.o
>> +common-obj-$(CONFIG_PCIE_PORT) += pcie_root_port.o
>>  common-obj-$(CONFIG_XIO3130) += xio3130_upstream.o xio3130_downstream.o
>>  common-obj-$(CONFIG_IOH3420) += ioh3420.o
>>  common-obj-$(CONFIG_I82801B11) += i82801b11.o
>> diff --git a/hw/pci-bridge/pcie_root_port.c b/hw/pci-bridge/pcie_root_port.c
>> new file mode 100644
>> index 0000000..e84ae14
>> --- /dev/null
>> +++ b/hw/pci-bridge/pcie_root_port.c
>> @@ -0,0 +1,194 @@
>> +/*
>> + * Generic PCI Express Root Port emulation
>> + *
>> + * Copyright (C) 2016 Red Hat Inc
>> + *
>> + * Authors:
>> + *   Marcel Apfelbaum <marcel@redhat.com>
>> + *
>> + * Most of the code was migrated from hw/pci-bridge/ioh3420.
>> + *
>> + * 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 "qemu/osdep.h"
>> +#include "qapi/error.h"
>> +#include "hw/pci/pcie_port.h"
>> +#include "hw/pci/msi.h"
>> +
>> +#define PCIE_ROOT_PORT_MSI_VECTORS              2
>> +#define PCIE_ROOT_PORT_MSI_SUPPORTED_FLAGS      PCI_MSI_FLAGS_MASKBIT
>> +#define PCIE_ROOT_PORT_AER_OFFSET               0x100
>

Hi Michael

> This isn't generic either.
>

It is used by the generic port which happens to be in the same file (patch 3).
I'll move the define to patch 3.

>> +
>> +/*
>> + * If two MSI vector
>
>
> MSI vectors
>

OK

>> are allocated, Advanced Error Interrupt Message Number
>> + * is 1. otherwise 0.
>> + * 17.12.5.10 RPERRSTS,  32:27 bit Advanced Error Interrupt Message Number.
>
> Hmm I see it in
> 7.10.10. Root Error Status Register (Offset 30h)
>
> which spec is this number from?
>

No idea, the description is from the Intel Port code, I'll update.

>
>> + */
>> +static uint8_t rp_aer_vector(const PCIDevice *d)
>> +{
>> +    switch (msi_nr_vectors_allocated(d)) {
>> +    case 1:
>> +        return 0;
>> +    case 2:
>> +        return 1;
>> +    case 4:
>> +    case 8:
>> +    case 16:
>> +    case 32:
>
> what are these doing here?
>

Will remove.

>> +    default:
>> +        break;
>> +    }
>> +    abort();
>> +    return 0;
>> +}
>
> I don't believe it's a generic thing. This is just how
> intel did it, isn't it?
>

Yes, but since we have only 2 implementations
and both are working the same I think they can share it
until the functionality will split.

An alternative is to add a function to the base class and
an implementation for each derived class, but since the code
ultimately will look the same I preferred to keep it simple.

Thanks for the review,
Marcel

>
>> +
>> +static void rp_aer_vector_update(PCIDevice *d)
>> +{
>> +    pcie_aer_root_set_vector(d, rp_aer_vector(d));
>> +}
>> +
>> +static void rp_write_config(PCIDevice *d, uint32_t address,
>> +                            uint32_t val, int len)
>> +{
>> +    uint32_t root_cmd =
>> +        pci_get_long(d->config + d->exp.aer_cap + PCI_ERR_ROOT_COMMAND);
>> +
>> +    pci_bridge_write_config(d, address, val, len);
>> +    rp_aer_vector_update(d);
>> +    pcie_cap_slot_write_config(d, address, val, len);
>> +    pcie_aer_write_config(d, address, val, len);
>> +    pcie_aer_root_write_config(d, address, val, len, root_cmd);
>> +}
>> +
>> +static void rp_reset(DeviceState *qdev)
>> +{
>> +    PCIDevice *d = PCI_DEVICE(qdev);
>> +
>> +    rp_aer_vector_update(d);
>> +    pcie_cap_root_reset(d);
>> +    pcie_cap_deverr_reset(d);
>> +    pcie_cap_slot_reset(d);
>> +    pcie_cap_arifwd_reset(d);
>> +    pcie_aer_root_reset(d);
>> +    pci_bridge_reset(qdev);
>> +    pci_bridge_disable_base_limit(d);
>> +}
>> +
>> +static void rp_realize(PCIDevice *d, Error **errp)
>> +{
>> +    PCIEPort *p = PCIE_PORT(d);
>> +    PCIESlot *s = PCIE_SLOT(d);
>> +    PCIDeviceClass *dc = PCI_DEVICE_GET_CLASS(d);
>> +    PCIERootPortClass *rpc = PCIE_ROOT_PORT_GET_CLASS(d);
>> +    int rc;
>> +    Error *local_err = NULL;
>> +
>> +    pci_config_set_interrupt_pin(d->config, 1);
>> +    pci_bridge_initfn(d, TYPE_PCIE_BUS);
>> +    pcie_port_init_reg(d);
>> +
>> +    rc = pci_bridge_ssvid_init(d, rpc->ssvid_offset, dc->vendor_id, rpc->ssid);
>> +    if (rc < 0) {
>> +        error_setg(errp, "Can't init SSV ID, error %d", rc);
>> +        goto err_bridge;
>> +    }
>> +
>> +    rc = msi_init(d, rpc->msi_offset, PCIE_ROOT_PORT_MSI_VECTORS,
>> +                  PCIE_ROOT_PORT_MSI_SUPPORTED_FLAGS & PCI_MSI_FLAGS_64BIT,
>> +                  PCIE_ROOT_PORT_MSI_SUPPORTED_FLAGS & PCI_MSI_FLAGS_MASKBIT,
>> +                  &local_err);
>> +    if (rc < 0) {
>> +        assert(rc == -ENOTSUP);
>> +        error_propagate(errp, local_err);
>> +        goto err_bridge;
>> +    }
>> +
>> +    rc = pcie_cap_init(d, rpc->exp_offset, PCI_EXP_TYPE_ROOT_PORT, p->port);
>> +    if (rc < 0) {
>> +        error_setg(errp, "Can't add Root Port capability, error %d", rc);
>> +        goto err_msi;
>> +    }
>> +
>> +    pcie_cap_arifwd_init(d);
>> +    pcie_cap_deverr_init(d);
>> +    pcie_cap_slot_init(d, s->slot);
>> +    pcie_cap_root_init(d);
>> +
>> +    pcie_chassis_create(s->chassis);
>> +    rc = pcie_chassis_add_slot(s);
>> +    if (rc < 0) {
>> +        error_setg(errp, "Can't add chassis slot, error %d", rc);
>> +        goto err_pcie_cap;
>> +    }
>> +
>> +    rc = pcie_aer_init(d, PCI_ERR_VER, rpc->aer_offset,
>> +                       PCI_ERR_SIZEOF, &local_err);
>> +    if (rc < 0) {
>> +        error_propagate(errp, local_err);
>> +        goto err;
>> +    }
>> +    pcie_aer_root_init(d);
>> +    rp_aer_vector_update(d);
>> +
>> +    return;
>> +
>> +err:
>> +    pcie_chassis_del_slot(s);
>> +err_pcie_cap:
>> +    pcie_cap_exit(d);
>> +err_msi:
>> +    msi_uninit(d);
>> +err_bridge:
>> +    pci_bridge_exitfn(d);
>> +}
>> +
>> +static void rp_exit(PCIDevice *d)
>> +{
>> +    PCIESlot *s = PCIE_SLOT(d);
>> +
>> +    pcie_aer_exit(d);
>> +    pcie_chassis_del_slot(s);
>> +    pcie_cap_exit(d);
>> +    msi_uninit(d);
>> +    pci_bridge_exitfn(d);
>> +}
>> +
>> +static Property rp_props[] = {
>> +    DEFINE_PROP_BIT(COMPAT_PROP_PCP, PCIDevice, cap_present,
>> +                    QEMU_PCIE_SLTCAP_PCP_BITNR, true),
>> +    DEFINE_PROP_END_OF_LIST()
>> +};
>> +
>> +static void rp_class_init(ObjectClass *klass, void *data)
>> +{
>> +    DeviceClass *dc = DEVICE_CLASS(klass);
>> +    PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
>> +    PCIERootPortClass *rpc = PCIE_ROOT_PORT_CLASS(klass);
>> +
>> +    k->is_express = 1;
>> +    k->is_bridge = 1;
>> +    k->config_write = rp_write_config;
>> +    k->realize = rp_realize;
>> +    k->exit = rp_exit;
>> +    set_bit(DEVICE_CATEGORY_BRIDGE, dc->categories);
>> +    dc->reset = rp_reset;
>> +    dc->props = rp_props;
>> +    rpc->aer_offset = PCIE_ROOT_PORT_AER_OFFSET;
>> +}
>> +
>> +static const TypeInfo rp_info = {
>> +    .name          = TYPE_PCIE_ROOT_PORT,
>> +    .parent        = TYPE_PCIE_SLOT,
>> +    .class_init    = rp_class_init,
>> +    .abstract      = true,
>> +    .class_size = sizeof(PCIERootPortClass),
>> +};
>> +
>> +static void rp_register_types(void)
>> +{
>> +    type_register_static(&rp_info);
>> +}
>> +
>> +type_init(rp_register_types)
>> diff --git a/include/hw/pci/pcie_port.h b/include/hw/pci/pcie_port.h
>> index f7b64db..a8eee56 100644
>> --- a/include/hw/pci/pcie_port.h
>> +++ b/include/hw/pci/pcie_port.h
>> @@ -57,4 +57,22 @@ PCIESlot *pcie_chassis_find_slot(uint8_t chassis, uint16_t slot);
>>  int pcie_chassis_add_slot(struct PCIESlot *slot);
>>  void pcie_chassis_del_slot(PCIESlot *s);
>>
>> +#define TYPE_PCIE_ROOT_PORT         "pcie-root-port-base"
>> +#define PCIE_ROOT_PORT_CLASS(klass) \
>> +     OBJECT_CLASS_CHECK(PCIERootPortClass, (klass), TYPE_PCIE_ROOT_PORT)
>> +#define PCIE_ROOT_PORT_CLASS(klass) \
>> +     OBJECT_CLASS_CHECK(PCIERootPortClass, (klass), TYPE_PCIE_ROOT_PORT)
>> +#define PCIE_ROOT_PORT_GET_CLASS(obj) \
>> +     OBJECT_GET_CLASS(PCIERootPortClass, (obj), TYPE_PCIE_ROOT_PORT)
>> +
>> +typedef struct PCIERootPortClass {
>> +    PCIDeviceClass parent_class;
>> +
>> +    int msi_offset;
>> +    int exp_offset;
>> +    int aer_offset;
>> +    int ssvid_offset;
>> +    int ssid;
>> +} PCIERootPortClass;
>> +
>>  #endif /* QEMU_PCIE_PORT_H */
>> --
>> 2.5.5

Re: [Qemu-devel] [PATCH V2 0/3] hw/pcie: Introduce Generic PCI Express Root Port

[Michael S. Tsirkin]

On Wed, Jan 11, 2017 at 02:18:53PM +0200, Marcel Apfelbaum wrote:
> v1 -> v2:
>  - Rebased on master.
> 
> The Generic Root Port behaves the same as the
> Intel's IOH device with id 3420, without having
> Intel specific attributes.
> 
> The device has two purposes:
>  (1) Can be used on both X86 and ARM machines.
>  (2) It will allow us to tweak the behaviour
>     (e.g add vendor-specific PCI capabilities)
>      - something that obviously cannot be done
>        on a known device.
> 
> Patch 1/3: Introduce a base class for Root Ports - most of the code
>            is migrated from IOH3420 implementation.
> Patch 2/3: Derives the IOH3420 from the new base class
> Patch 3/3: Introduces the generic Root Port.
> 
> Tested with Linux and Windows guests only on x86 hosts.

So I understand how it's helpful to share code
with an existing bridge, but I worries me that
you picked up some arbitrary values from the intel chip
and promote them as a standard pcie thing.

You want to have e.g. base-pcie-root-port and
inherit that. Only put really common stuff in there.
Similar to how we do it in hw/pci/pci_bridge.c


Now we do not want to over-engineer, so I understand
how you might want to say hey I use this option
and intel does the same so we do not need a flag
for that now. And that's fine but please add a comment
to that end.

And when you do you will discover there's some stuff you
might be able to simplify, e.g. I don't yet see why
you would want AER support there.

> Marcel Apfelbaum (3):
>   hw/pcie: Introduce a base class for PCI Express Root Ports
>   hw/ioh3420: derive from PCI Express Root Port base class
>   hw/pcie: Introduce Generic PCI Express Root Port
> 
>  default-configs/arm-softmmu.mak    |   1 +
>  default-configs/i386-softmmu.mak   |   1 +
>  default-configs/x86_64-softmmu.mak |   1 +
>  hw/pci-bridge/Makefile.objs        |   1 +
>  hw/pci-bridge/ioh3420.c            | 152 ++-----------------------
>  hw/pci-bridge/pcie_root_port.c     | 228 +++++++++++++++++++++++++++++++++++++
>  include/hw/pci/pci.h               |   1 +
>  include/hw/pci/pcie_port.h         |  18 +++
>  8 files changed, 258 insertions(+), 145 deletions(-)
>  create mode 100644 hw/pci-bridge/pcie_root_port.c
> 
> -- 
> 2.5.5

Re: [Qemu-devel] [PATCH V2 0/3] hw/pcie: Introduce Generic PCI Express Root Port

[Marcel Apfelbaum]

On 01/12/2017 05:56 PM, Michael S. Tsirkin wrote:
> On Wed, Jan 11, 2017 at 02:18:53PM +0200, Marcel Apfelbaum wrote:
>> v1 -> v2:
>>  - Rebased on master.
>>
>> The Generic Root Port behaves the same as the
>> Intel's IOH device with id 3420, without having
>> Intel specific attributes.
>>
>> The device has two purposes:
>>  (1) Can be used on both X86 and ARM machines.
>>  (2) It will allow us to tweak the behaviour
>>     (e.g add vendor-specific PCI capabilities)
>>      - something that obviously cannot be done
>>        on a known device.
>>
>> Patch 1/3: Introduce a base class for Root Ports - most of the code
>>            is migrated from IOH3420 implementation.
>> Patch 2/3: Derives the IOH3420 from the new base class
>> Patch 3/3: Introduces the generic Root Port.
>>
>> Tested with Linux and Windows guests only on x86 hosts.
>

Hi Michael,

> So I understand how it's helpful to share code
> with an existing bridge, but I worries me that
> you picked up some arbitrary values from the intel chip
> and promote them as a standard pcie thing.
>

I suppose you are referring to the macros, I can duplicate
them, of course.

> You want to have e.g. base-pcie-root-port and
> inherit that. Only put really common stuff in there.
> Similar to how we do it in hw/pci/pci_bridge.c
>

So you prefer a different code file for the generic device.
I currently put them both in the same file, I'll split.

>
> Now we do not want to over-engineer, so I understand
> how you might want to say hey I use this option
> and intel does the same so we do not need a flag
> for that now. And that's fine but please add a comment
> to that end.
>

Indeed, I wanted to keep things simple. I'll add a comment
on the functions that are not generic but are the same on
both implementations 'by chance'.

> And when you do you will discover there's some stuff you
> might be able to simplify, e.g. I don't yet see why
> you would want AER support there.
>

I am not sure *why not*. I am 'afraid' people will not use the new port
because of missing features and they can see the AER as one of them.

Do you see a special issue in imitating Intel's behavior on AER?

Thanks,
Marcel

>> Marcel Apfelbaum (3):
>>   hw/pcie: Introduce a base class for PCI Express Root Ports
>>   hw/ioh3420: derive from PCI Express Root Port base class
>>   hw/pcie: Introduce Generic PCI Express Root Port
>>
>>  default-configs/arm-softmmu.mak    |   1 +
>>  default-configs/i386-softmmu.mak   |   1 +
>>  default-configs/x86_64-softmmu.mak |   1 +
>>  hw/pci-bridge/Makefile.objs        |   1 +
>>  hw/pci-bridge/ioh3420.c            | 152 ++-----------------------
>>  hw/pci-bridge/pcie_root_port.c     | 228 +++++++++++++++++++++++++++++++++++++
>>  include/hw/pci/pci.h               |   1 +
>>  include/hw/pci/pcie_port.h         |  18 +++
>>  8 files changed, 258 insertions(+), 145 deletions(-)
>>  create mode 100644 hw/pci-bridge/pcie_root_port.c
>>
>> --
>> 2.5.5

Re: [Qemu-devel] [PATCH V2 0/3] hw/pcie: Introduce Generic PCI Express Root Port

[Michael S. Tsirkin]

On Thu, Jan 12, 2017 at 06:05:20PM +0200, Marcel Apfelbaum wrote:
> On 01/12/2017 05:56 PM, Michael S. Tsirkin wrote:
> > On Wed, Jan 11, 2017 at 02:18:53PM +0200, Marcel Apfelbaum wrote:
> > > v1 -> v2:
> > >  - Rebased on master.
> > > 
> > > The Generic Root Port behaves the same as the
> > > Intel's IOH device with id 3420, without having
> > > Intel specific attributes.
> > > 
> > > The device has two purposes:
> > >  (1) Can be used on both X86 and ARM machines.
> > >  (2) It will allow us to tweak the behaviour
> > >     (e.g add vendor-specific PCI capabilities)
> > >      - something that obviously cannot be done
> > >        on a known device.
> > > 
> > > Patch 1/3: Introduce a base class for Root Ports - most of the code
> > >            is migrated from IOH3420 implementation.
> > > Patch 2/3: Derives the IOH3420 from the new base class
> > > Patch 3/3: Introduces the generic Root Port.
> > > 
> > > Tested with Linux and Windows guests only on x86 hosts.
> > 
> 
> Hi Michael,
> 
> > So I understand how it's helpful to share code
> > with an existing bridge, but I worries me that
> > you picked up some arbitrary values from the intel chip
> > and promote them as a standard pcie thing.
> > 
> 
> I suppose you are referring to the macros, I can duplicate
> them, of course.
> 
> > You want to have e.g. base-pcie-root-port and
> > inherit that. Only put really common stuff in there.
> > Similar to how we do it in hw/pci/pci_bridge.c
> > 
> 
> So you prefer a different code file for the generic device.
> I currently put them both in the same file, I'll split.
> 
> > 
> > Now we do not want to over-engineer, so I understand
> > how you might want to say hey I use this option
> > and intel does the same so we do not need a flag
> > for that now. And that's fine but please add a comment
> > to that end.
> > 
> 
> Indeed, I wanted to keep things simple. I'll add a comment
> on the functions that are not generic but are the same on
> both implementations 'by chance'.
> 
> > And when you do you will discover there's some stuff you
> > might be able to simplify, e.g. I don't yet see why
> > you would want AER support there.
> > 
> 
> I am not sure *why not*.
> I am 'afraid' people will not use the new port
> because of missing features and they can see the AER as one of them.
> Do you see a special issue in imitating Intel's behavior on AER?
> 
> Thanks,
> Marcel

Not really, fair enough. Just keep an eye out for caps
you don't need. E.g. it's worth considering whether
msi-x would be better since then you can just use
a fixed vector number without playing tricks like
you have to with msi.

> > > Marcel Apfelbaum (3):
> > >   hw/pcie: Introduce a base class for PCI Express Root Ports
> > >   hw/ioh3420: derive from PCI Express Root Port base class
> > >   hw/pcie: Introduce Generic PCI Express Root Port
> > > 
> > >  default-configs/arm-softmmu.mak    |   1 +
> > >  default-configs/i386-softmmu.mak   |   1 +
> > >  default-configs/x86_64-softmmu.mak |   1 +
> > >  hw/pci-bridge/Makefile.objs        |   1 +
> > >  hw/pci-bridge/ioh3420.c            | 152 ++-----------------------
> > >  hw/pci-bridge/pcie_root_port.c     | 228 +++++++++++++++++++++++++++++++++++++
> > >  include/hw/pci/pci.h               |   1 +
> > >  include/hw/pci/pcie_port.h         |  18 +++
> > >  8 files changed, 258 insertions(+), 145 deletions(-)
> > >  create mode 100644 hw/pci-bridge/pcie_root_port.c
> > > 
> > > --
> > > 2.5.5

Re: [Qemu-devel] [PATCH V2 0/3] hw/pcie: Introduce Generic PCI Express Root Port

[Marcel Apfelbaum]

On 01/12/2017 06:20 PM, Michael S. Tsirkin wrote:
> On Thu, Jan 12, 2017 at 06:05:20PM +0200, Marcel Apfelbaum wrote:
>> On 01/12/2017 05:56 PM, Michael S. Tsirkin wrote:
>>> On Wed, Jan 11, 2017 at 02:18:53PM +0200, Marcel Apfelbaum wrote:
>>>> v1 -> v2:
>>>>  - Rebased on master.
>>>>
>>>> The Generic Root Port behaves the same as the
>>>> Intel's IOH device with id 3420, without having
>>>> Intel specific attributes.
>>>>
>>>> The device has two purposes:
>>>>  (1) Can be used on both X86 and ARM machines.
>>>>  (2) It will allow us to tweak the behaviour
>>>>     (e.g add vendor-specific PCI capabilities)
>>>>      - something that obviously cannot be done
>>>>        on a known device.
>>>>
>>>> Patch 1/3: Introduce a base class for Root Ports - most of the code
>>>>            is migrated from IOH3420 implementation.
>>>> Patch 2/3: Derives the IOH3420 from the new base class
>>>> Patch 3/3: Introduces the generic Root Port.
>>>>
>>>> Tested with Linux and Windows guests only on x86 hosts.
>>>
>>
>> Hi Michael,
>>
>>> So I understand how it's helpful to share code
>>> with an existing bridge, but I worries me that
>>> you picked up some arbitrary values from the intel chip
>>> and promote them as a standard pcie thing.
>>>
>>
>> I suppose you are referring to the macros, I can duplicate
>> them, of course.
>>
>>> You want to have e.g. base-pcie-root-port and
>>> inherit that. Only put really common stuff in there.
>>> Similar to how we do it in hw/pci/pci_bridge.c
>>>
>>
>> So you prefer a different code file for the generic device.
>> I currently put them both in the same file, I'll split.
>>
>>>
>>> Now we do not want to over-engineer, so I understand
>>> how you might want to say hey I use this option
>>> and intel does the same so we do not need a flag
>>> for that now. And that's fine but please add a comment
>>> to that end.
>>>
>>
>> Indeed, I wanted to keep things simple. I'll add a comment
>> on the functions that are not generic but are the same on
>> both implementations 'by chance'.
>>
>>> And when you do you will discover there's some stuff you
>>> might be able to simplify, e.g. I don't yet see why
>>> you would want AER support there.
>>>
>>
>> I am not sure *why not*.
>> I am 'afraid' people will not use the new port
>> because of missing features and they can see the AER as one of them.
>> Do you see a special issue in imitating Intel's behavior on AER?
>>
>> Thanks,
>> Marcel
>
> Not really, fair enough. Just keep an eye out for caps
> you don't need. E.g. it's worth considering whether
> msi-x would be better since then you can just use
> a fixed vector number without playing tricks like
> you have to with msi.
>

Sure, thanks for the pointers!
I suppose I can change later to msi-x in a follow up patch
to avoid introducing new bugs from the beginning :)

Thanks,
Marcel

>>>> Marcel Apfelbaum (3):
>>>>   hw/pcie: Introduce a base class for PCI Express Root Ports
>>>>   hw/ioh3420: derive from PCI Express Root Port base class
>>>>   hw/pcie: Introduce Generic PCI Express Root Port
>>>>
>>>>  default-configs/arm-softmmu.mak    |   1 +
>>>>  default-configs/i386-softmmu.mak   |   1 +
>>>>  default-configs/x86_64-softmmu.mak |   1 +
>>>>  hw/pci-bridge/Makefile.objs        |   1 +
>>>>  hw/pci-bridge/ioh3420.c            | 152 ++-----------------------
>>>>  hw/pci-bridge/pcie_root_port.c     | 228 +++++++++++++++++++++++++++++++++++++
>>>>  include/hw/pci/pci.h               |   1 +
>>>>  include/hw/pci/pcie_port.h         |  18 +++
>>>>  8 files changed, 258 insertions(+), 145 deletions(-)
>>>>  create mode 100644 hw/pci-bridge/pcie_root_port.c
>>>>
>>>> --
>>>> 2.5.5

Re: [Qemu-devel] [PATCH V2 0/3] hw/pcie: Introduce Generic PCI Express Root Port

[Michael S. Tsirkin]

On Thu, Jan 12, 2017 at 06:23:08PM +0200, Marcel Apfelbaum wrote:
> On 01/12/2017 06:20 PM, Michael S. Tsirkin wrote:
> > On Thu, Jan 12, 2017 at 06:05:20PM +0200, Marcel Apfelbaum wrote:
> > > On 01/12/2017 05:56 PM, Michael S. Tsirkin wrote:
> > > > On Wed, Jan 11, 2017 at 02:18:53PM +0200, Marcel Apfelbaum wrote:
> > > > > v1 -> v2:
> > > > >  - Rebased on master.
> > > > > 
> > > > > The Generic Root Port behaves the same as the
> > > > > Intel's IOH device with id 3420, without having
> > > > > Intel specific attributes.
> > > > > 
> > > > > The device has two purposes:
> > > > >  (1) Can be used on both X86 and ARM machines.
> > > > >  (2) It will allow us to tweak the behaviour
> > > > >     (e.g add vendor-specific PCI capabilities)
> > > > >      - something that obviously cannot be done
> > > > >        on a known device.
> > > > > 
> > > > > Patch 1/3: Introduce a base class for Root Ports - most of the code
> > > > >            is migrated from IOH3420 implementation.
> > > > > Patch 2/3: Derives the IOH3420 from the new base class
> > > > > Patch 3/3: Introduces the generic Root Port.
> > > > > 
> > > > > Tested with Linux and Windows guests only on x86 hosts.
> > > > 
> > > 
> > > Hi Michael,
> > > 
> > > > So I understand how it's helpful to share code
> > > > with an existing bridge, but I worries me that
> > > > you picked up some arbitrary values from the intel chip
> > > > and promote them as a standard pcie thing.
> > > > 
> > > 
> > > I suppose you are referring to the macros, I can duplicate
> > > them, of course.
> > > 
> > > > You want to have e.g. base-pcie-root-port and
> > > > inherit that. Only put really common stuff in there.
> > > > Similar to how we do it in hw/pci/pci_bridge.c
> > > > 
> > > 
> > > So you prefer a different code file for the generic device.
> > > I currently put them both in the same file, I'll split.
> > > 
> > > > 
> > > > Now we do not want to over-engineer, so I understand
> > > > how you might want to say hey I use this option
> > > > and intel does the same so we do not need a flag
> > > > for that now. And that's fine but please add a comment
> > > > to that end.
> > > > 
> > > 
> > > Indeed, I wanted to keep things simple. I'll add a comment
> > > on the functions that are not generic but are the same on
> > > both implementations 'by chance'.
> > > 
> > > > And when you do you will discover there's some stuff you
> > > > might be able to simplify, e.g. I don't yet see why
> > > > you would want AER support there.
> > > > 
> > > 
> > > I am not sure *why not*.
> > > I am 'afraid' people will not use the new port
> > > because of missing features and they can see the AER as one of them.
> > > Do you see a special issue in imitating Intel's behavior on AER?
> > > 
> > > Thanks,
> > > Marcel
> > 
> > Not really, fair enough. Just keep an eye out for caps
> > you don't need. E.g. it's worth considering whether
> > msi-x would be better since then you can just use
> > a fixed vector number without playing tricks like
> > you have to with msi.
> > 
> 
> Sure, thanks for the pointers!
> I suppose I can change later to msi-x in a follow up patch
> to avoid introducing new bugs from the beginning :)
> 
> Thanks,
> Marcel

If I'm right and msix will be less code, that would be a
strage approach imho. But you decide.

> > > > > Marcel Apfelbaum (3):
> > > > >   hw/pcie: Introduce a base class for PCI Express Root Ports
> > > > >   hw/ioh3420: derive from PCI Express Root Port base class
> > > > >   hw/pcie: Introduce Generic PCI Express Root Port
> > > > > 
> > > > >  default-configs/arm-softmmu.mak    |   1 +
> > > > >  default-configs/i386-softmmu.mak   |   1 +
> > > > >  default-configs/x86_64-softmmu.mak |   1 +
> > > > >  hw/pci-bridge/Makefile.objs        |   1 +
> > > > >  hw/pci-bridge/ioh3420.c            | 152 ++-----------------------
> > > > >  hw/pci-bridge/pcie_root_port.c     | 228 +++++++++++++++++++++++++++++++++++++
> > > > >  include/hw/pci/pci.h               |   1 +
> > > > >  include/hw/pci/pcie_port.h         |  18 +++
> > > > >  8 files changed, 258 insertions(+), 145 deletions(-)
> > > > >  create mode 100644 hw/pci-bridge/pcie_root_port.c
> > > > > 
> > > > > --
> > > > > 2.5.5

Re: [Qemu-devel] [PATCH V2 0/3] hw/pcie: Introduce Generic PCI Express Root Port

[Gerd Hoffmann]

  Hi,

> Sure, thanks for the pointers!
> I suppose I can change later to msi-x in a follow up patch
> to avoid introducing new bugs from the beginning :)

I'd use msi-x right from start to avoid any backward compatibility
issues.

cheers,
  Gerd

Re: [Qemu-devel] [PATCH V2 0/3] hw/pcie: Introduce Generic PCI Express Root Port

[Marcel Apfelbaum]

On 01/13/2017 10:35 AM, Gerd Hoffmann wrote:
>   Hi,
>
>> Sure, thanks for the pointers!
>> I suppose I can change later to msi-x in a follow up patch
>> to avoid introducing new bugs from the beginning :)
>
> I'd use msi-x right from start to avoid any backward compatibility
> issues.
>

Hi Gerd, Michael

I'll post a version with msi-x.

Thanks,
Marcel

> cheers,
>   Gerd
>

Re: [Qemu-devel] [PATCH V2 0/3] hw/pcie: Introduce Generic PCI Express Root Port

[Andrea Bolognani]

On Wed, 2017-01-11 at 14:18 +0200, Marcel Apfelbaum wrote:
> v1 -> v2:
>  - Rebased on master.
> 
> The Generic Root Port behaves the same as the
> Intel's IOH device with id 3420, without having
> Intel specific attributes.
> 
> The device has two purposes:
>  (1) Can be used on both X86 and ARM machines.
>  (2) It will allow us to tweak the behaviour
>     (e.g add vendor-specific PCI capabilities)
>      - something that obviously cannot be done
>        on a known device.
> 
> Patch 1/3: Introduce a base class for Root Ports - most of the code
>            is migrated from IOH3420 implementation.
> Patch 2/3: Derives the IOH3420 from the new base class
> Patch 3/3: Introduces the generic Root Port.
> 
> Tested with Linux and Windows guests only on x86 hosts.

I tested this both on x86/q35 (Debian guest) and on
aarch64/virt (Fedora guest) very briefly, eg. started
the guest, performed some network I/O and shut it down.

It seems to be working fine :)

-- 
Andrea Bolognani / Red Hat / Virtualization

Re: [Qemu-devel] [PATCH V2 0/3] hw/pcie: Introduce Generic PCI Express Root Port

[Marcel Apfelbaum]

On 01/16/2017 06:46 PM, Andrea Bolognani wrote:
> On Wed, 2017-01-11 at 14:18 +0200, Marcel Apfelbaum wrote:
>> v1 -> v2:
>>   - Rebased on master.
>>
>> The Generic Root Port behaves the same as the
>> Intel's IOH device with id 3420, without having
>> Intel specific attributes.
>>
>> The device has two purposes:
>>   (1) Can be used on both X86 and ARM machines.
>>   (2) It will allow us to tweak the behaviour
>>      (e.g add vendor-specific PCI capabilities)
>>       - something that obviously cannot be done
>>         on a known device.
>>
>> Patch 1/3: Introduce a base class for Root Ports - most of the code
>>             is migrated from IOH3420 implementation.
>> Patch 2/3: Derives the IOH3420 from the new base class
>> Patch 3/3: Introduces the generic Root Port.
>>
>> Tested with Linux and Windows guests only on x86 hosts.
>
> I tested this both on x86/q35 (Debian guest) and on
> aarch64/virt (Fedora guest) very briefly, eg. started
> the guest, performed some network I/O and shut it down.
>
> It seems to be working fine :)

Thanks for testing it! A tested-by tag would be appreciated for the next version :)
Marcel

>
> --
> Andrea Bolognani / Red Hat / Virtualization
>