[Qemu-devel] [PATCH v1 0/1] s390 pci infrastucture modeling

[Qemu-devel] [PATCH v1 0/1] s390 pci infrastucture modeling

[Hong Bo Li]

This patch extends the current s390 pci implementation to
provide more flexibility in configuration of s390 specific
device handling. For this we had to introduce a new facility
(and bus) to hold devices representing information actually
provided by s390 firmware and I/O configuration.

For each vfio pci device, I create a zpci device to store s390
specific informations. And attach all of these special zpci devices
to the s390 facility bus. A zpci device references the corresponding
PCI device via device id. 

Compare to the old implementation, I moved the actual hotplug/unplug 
codes to s390 pci device hot plug function. Then in the pcihost 
hotplug function, we don't need to do anything special. In the pcihost
unplug function, we need to unplug the corresponding zpci device.

The new design allows to define multiple host bridges, each host bridge
could hold 32 zpci devices at most.

The topology for this implementation could be:

  dev: s390-pcihost, id ""
    bus: pci.0
      type PCI
      dev: vfio-pci, id "vpci1"
        host = "0000:00:00.0"
	......
      dev: vfio-pci, id "vpci2"
        host = "0001:00:00.0"
	......
  dev: s390-pci-facility, id ""
    bus: s390-pci-fac-bus.0
      type s390-pci-fac-bus
      dev: zpci, id "zpci1"
        fid = 1 (0x1)
        uid = 2 (0x2)
        pci_id = "vpci1"
      dev: zpci, id "zpci2"
        fid = 6 (0x6)
        uid = 7 (0x7)
        pci_id = "vpci2"

To make the review easier, I keep all of the old names, such as 
S390PCIBusDevice to name a zpci device. I will make a cleanup 
patch later to change these names to a more suitable name.

Hong Bo Li (1):
  s390 pci infrastructure modeling

 hw/s390x/s390-pci-bus.c    | 317 +++++++++++++++++++++++++++++++++------------
 hw/s390x/s390-pci-bus.h    |  48 ++++++-
 hw/s390x/s390-pci-inst.c   |   4 +-
 hw/s390x/s390-virtio-ccw.c |   4 +-
 4 files changed, 285 insertions(+), 88 deletions(-)

-- 
1.9.3

[Qemu-devel] [PATCH v1 1/1] s390 pci infrastructure modeling

[Hong Bo Li]

This patch contains the actual interesting changes.
usage example:
-device s390-pcihost
-device vfio-pci,host=0000:00:00.0,id=vpci1
-device zpci,fid=2,uid=5,pci_id=vpci1,id=zpci1

The first line will create a s390 pci host bridge
and init the root bus. The second line will create
a standard vfio pci device, and attach it to the
root bus. These are similiar to the standard process
to define a pci device on other platform.

The third line will create a s390 pci device to
store s390 specific information, and references
the corresponding vfio pci device via device id.
We create a s390 pci facility bus to hold all the
zpci devices.

Signed-off-by: Hong Bo Li <lihbbj@linux.vnet.ibm.com>
---
 hw/s390x/s390-pci-bus.c    | 317 +++++++++++++++++++++++++++++++++------------
 hw/s390x/s390-pci-bus.h    |  48 ++++++-
 hw/s390x/s390-pci-inst.c   |   4 +-
 hw/s390x/s390-virtio-ccw.c |   4 +-
 4 files changed, 285 insertions(+), 88 deletions(-)

diff --git a/hw/s390x/s390-pci-bus.c b/hw/s390x/s390-pci-bus.c
index 3c086f6..c81093e 100644
--- a/hw/s390x/s390-pci-bus.c
+++ b/hw/s390x/s390-pci-bus.c
@@ -32,8 +32,8 @@ int chsc_sei_nt2_get_event(void *res)
     PciCcdfErr *eccdf;
     int rc = 1;
     SeiContainer *sei_cont;
-    S390pciState *s = S390_PCI_HOST_BRIDGE(
-        object_resolve_path(TYPE_S390_PCI_HOST_BRIDGE, NULL));
+    S390PCIFacility *s = S390_PCI_FACILITY(
+        object_resolve_path(TYPE_S390_PCI_FACILITY, NULL));
 
     if (!s) {
         return rc;
@@ -72,8 +72,8 @@ int chsc_sei_nt2_get_event(void *res)
 
 int chsc_sei_nt2_have_event(void)
 {
-    S390pciState *s = S390_PCI_HOST_BRIDGE(
-        object_resolve_path(TYPE_S390_PCI_HOST_BRIDGE, NULL));
+    S390PCIFacility *s = S390_PCI_FACILITY(
+        object_resolve_path(TYPE_S390_PCI_FACILITY, NULL));
 
     if (!s) {
         return 0;
@@ -82,20 +82,32 @@ int chsc_sei_nt2_have_event(void)
     return !QTAILQ_EMPTY(&s->pending_sei);
 }
 
+void s390_pci_device_enable(S390PCIBusDevice *zpci)
+{
+    zpci->fh = zpci->fh | 1 << ENABLE_BIT_OFFSET;
+}
+
+void s390_pci_device_disable(S390PCIBusDevice *zpci)
+{
+    zpci->fh = zpci->fh & ~(1 << ENABLE_BIT_OFFSET);
+    if (zpci->is_unplugged)
+        object_unparent(OBJECT(zpci));
+}
+
 S390PCIBusDevice *s390_pci_find_dev_by_fid(uint32_t fid)
 {
     S390PCIBusDevice *pbdev;
-    int i;
-    S390pciState *s = S390_PCI_HOST_BRIDGE(
-        object_resolve_path(TYPE_S390_PCI_HOST_BRIDGE, NULL));
+    BusChild *kid;
+    S390PCIFacility *s = S390_PCI_FACILITY(
+        object_resolve_path(TYPE_S390_PCI_FACILITY, NULL));
 
     if (!s) {
         return NULL;
     }
 
-    for (i = 0; i < PCI_SLOT_MAX; i++) {
-        pbdev = &s->pbdev[i];
-        if ((pbdev->fh != 0) && (pbdev->fid == fid)) {
+    QTAILQ_FOREACH(kid, &s->fbus->qbus.children, sibling) {
+        pbdev = (S390PCIBusDevice *)kid->child;
+        if (pbdev->fid == fid) {
             return pbdev;
         }
     }
@@ -126,39 +138,24 @@ void s390_pci_sclp_configure(int configure, SCCB *sccb)
     return;
 }
 
-static uint32_t s390_pci_get_pfid(PCIDevice *pdev)
-{
-    return PCI_SLOT(pdev->devfn);
-}
-
-static uint32_t s390_pci_get_pfh(PCIDevice *pdev)
-{
-    return PCI_SLOT(pdev->devfn) | FH_VIRT;
-}
-
 S390PCIBusDevice *s390_pci_find_dev_by_idx(uint32_t idx)
 {
     S390PCIBusDevice *pbdev;
-    int i;
-    int j = 0;
-    S390pciState *s = S390_PCI_HOST_BRIDGE(
-        object_resolve_path(TYPE_S390_PCI_HOST_BRIDGE, NULL));
+    BusChild *kid;
+    int i = 0;
+    S390PCIFacility *s = S390_PCI_FACILITY(
+        object_resolve_path(TYPE_S390_PCI_FACILITY, NULL));
 
     if (!s) {
         return NULL;
     }
 
-    for (i = 0; i < PCI_SLOT_MAX; i++) {
-        pbdev = &s->pbdev[i];
-
-        if (pbdev->fh == 0) {
-            continue;
-        }
-
-        if (j == idx) {
+    QTAILQ_FOREACH(kid, &s->fbus->qbus.children, sibling) {
+        pbdev = (S390PCIBusDevice *)kid->child;
+        if (i == idx) {
             return pbdev;
         }
-        j++;
+        i++;
     }
 
     return NULL;
@@ -167,16 +164,17 @@ S390PCIBusDevice *s390_pci_find_dev_by_idx(uint32_t idx)
 S390PCIBusDevice *s390_pci_find_dev_by_fh(uint32_t fh)
 {
     S390PCIBusDevice *pbdev;
-    int i;
-    S390pciState *s = S390_PCI_HOST_BRIDGE(
-        object_resolve_path(TYPE_S390_PCI_HOST_BRIDGE, NULL));
+    BusChild *kid;
+    S390PCIFacility *s = S390_PCI_FACILITY(
+        object_resolve_path(TYPE_S390_PCI_FACILITY, NULL));
+
 
     if (!s || !fh) {
         return NULL;
     }
 
-    for (i = 0; i < PCI_SLOT_MAX; i++) {
-        pbdev = &s->pbdev[i];
+    QTAILQ_FOREACH(kid, &s->fbus->qbus.children, sibling) {
+        pbdev = (S390PCIBusDevice *)kid->child;
         if (pbdev->fh == fh) {
             return pbdev;
         }
@@ -185,12 +183,33 @@ S390PCIBusDevice *s390_pci_find_dev_by_fh(uint32_t fh)
     return NULL;
 }
 
+static S390PCIBusDevice *s390_pci_find_dev_by_pdev(PCIDevice *pdev)
+{
+    S390PCIBusDevice *pbdev;
+    BusChild *kid;
+    S390PCIFacility *s = S390_PCI_FACILITY(
+        object_resolve_path(TYPE_S390_PCI_FACILITY, NULL));
+
+    if (!s || !pdev) {
+         return NULL;
+     }
+
+    QTAILQ_FOREACH(kid, &s->fbus->qbus.children, sibling) {
+        pbdev = (S390PCIBusDevice *)kid->child;
+        if (pbdev->pdev == pdev) {
+            return pbdev;
+        }
+    }
+
+    return NULL;
+}
+
 static void s390_pci_generate_event(uint8_t cc, uint16_t pec, uint32_t fh,
                                     uint32_t fid, uint64_t faddr, uint32_t e)
 {
     SeiContainer *sei_cont;
-    S390pciState *s = S390_PCI_HOST_BRIDGE(
-        object_resolve_path(TYPE_S390_PCI_HOST_BRIDGE, NULL));
+    S390PCIFacility *s = S390_PCI_FACILITY(
+        object_resolve_path(TYPE_S390_PCI_FACILITY, NULL));
 
     if (!s) {
         return;
@@ -305,7 +324,9 @@ static IOMMUTLBEntry s390_translate_iommu(MemoryRegion *iommu, hwaddr addr,
 {
     uint64_t pte;
     uint32_t flags;
-    S390PCIBusDevice *pbdev = container_of(iommu, S390PCIBusDevice, mr);
+    S390PCIDeviceConn *conn = container_of(iommu, S390PCIDeviceConn,
+                                           iommu_mr);
+    S390PCIBusDevice *pbdev = conn->zpci;
     S390pciState *s = S390_PCI_HOST_BRIDGE(pci_device_root_bus(pbdev->pdev)
                                            ->qbus.parent);
     IOMMUTLBEntry ret = {
@@ -316,8 +337,14 @@ static IOMMUTLBEntry s390_translate_iommu(MemoryRegion *iommu, hwaddr addr,
         .perm = IOMMU_NONE,
     };
 
+    if (!pbdev) {
+        return ret;
+    }
+
     DPRINTF("iommu trans addr 0x%" PRIx64 "\n", addr);
 
+    s = S390_PCI_HOST_BRIDGE(pci_device_root_bus(pbdev->pdev)->qbus.parent);
+
     /* s390 does not have an APIC mapped to main storage so we use
      * a separate AddressSpace only for msix notifications
      */
@@ -379,7 +406,7 @@ static AddressSpace *s390_pci_dma_iommu(PCIBus *bus, void *opaque, int devfn)
 {
     S390pciState *s = opaque;
 
-    return &s->pbdev[PCI_SLOT(devfn)].as;
+    return &s->conn[PCI_SLOT(devfn)].iommu_as;
 }
 
 static uint8_t set_ind_atomic(uint64_t ind_loc, uint8_t to_be_set)
@@ -452,9 +479,10 @@ static void s390_pcihost_init_as(S390pciState *s)
     int i;
 
     for (i = 0; i < PCI_SLOT_MAX; i++) {
-        memory_region_init_iommu(&s->pbdev[i].mr, OBJECT(s),
+        memory_region_init_iommu(&s->conn[i].iommu_mr, OBJECT(s),
                                  &s390_iommu_ops, "iommu-s390", UINT64_MAX);
-        address_space_init(&s->pbdev[i].as, &s->pbdev[i].mr, "iommu-pci");
+        address_space_init(&s->conn[i].iommu_as, &s->conn[i].iommu_mr,
+                           "iommu-pci");
     }
 
     memory_region_init_io(&s->msix_notify_mr, OBJECT(s),
@@ -481,7 +509,7 @@ static int s390_pcihost_init(SysBusDevice *dev)
     bus = BUS(b);
     qbus_set_hotplug_handler(bus, DEVICE(dev), NULL);
     phb->bus = b;
-    QTAILQ_INIT(&s->pending_sei);
+
     return 0;
 }
 
@@ -513,29 +541,12 @@ static int s390_pcihost_setup_msix(S390PCIBusDevice *pbdev)
     return 0;
 }
 
+/* this function is useless, just keep it to make the patch more
+   readable. Without this function, the diff will mix these hotplug
+   functions together. will be removed in future */
 static void s390_pcihost_hot_plug(HotplugHandler *hotplug_dev,
                                   DeviceState *dev, Error **errp)
 {
-    PCIDevice *pci_dev = PCI_DEVICE(dev);
-    S390PCIBusDevice *pbdev;
-    S390pciState *s = S390_PCI_HOST_BRIDGE(pci_device_root_bus(pci_dev)
-                                           ->qbus.parent);
-
-    pbdev = &s->pbdev[PCI_SLOT(pci_dev->devfn)];
-
-    pbdev->fid = s390_pci_get_pfid(pci_dev);
-    pbdev->pdev = pci_dev;
-    pbdev->configured = true;
-    pbdev->fh = s390_pci_get_pfh(pci_dev);
-
-    s390_pcihost_setup_msix(pbdev);
-
-    if (dev->hotplugged) {
-        s390_pci_generate_plug_event(HP_EVENT_RESERVED_TO_STANDBY,
-                                     pbdev->fh, pbdev->fid);
-        s390_pci_generate_plug_event(HP_EVENT_TO_CONFIGURED,
-                                     pbdev->fh, pbdev->fid);
-    }
     return;
 }
 
@@ -543,31 +554,30 @@ static void s390_pcihost_hot_unplug(HotplugHandler *hotplug_dev,
                                     DeviceState *dev, Error **errp)
 {
     PCIDevice *pci_dev = PCI_DEVICE(dev);
-    S390pciState *s = S390_PCI_HOST_BRIDGE(pci_device_root_bus(pci_dev)
-                                           ->qbus.parent);
-    S390PCIBusDevice *pbdev = &s->pbdev[PCI_SLOT(pci_dev->devfn)];
-
-    if (pbdev->configured) {
-        pbdev->configured = false;
-        s390_pci_generate_plug_event(HP_EVENT_CONFIGURED_TO_STBRES,
-                                     pbdev->fh, pbdev->fid);
+    S390PCIBusDevice *pbdev;
+    HotplugHandler *hotplug_ctrl;
+    S390PCIFacility *f = S390_PCI_FACILITY(
+        object_resolve_path(TYPE_S390_PCI_FACILITY, NULL));
+    S390PCIFacilityClass *k = S390_PCI_FACILITY_GET_CLASS(f);
+    HotplugHandlerClass *hdc = HOTPLUG_HANDLER_CLASS(k);
+
+    /* unplug corresponding zpci device */
+    pbdev = s390_pci_find_dev_by_pdev(pci_dev);
+    if (pbdev) {
+        hotplug_ctrl = pbdev->qdev.parent_bus->hotplug_handler;
+        if (hdc->unplug_request) {
+            hdc->unplug_request(hotplug_ctrl, &pbdev->qdev, errp);
+        }
     }
 
-    s390_pci_generate_plug_event(HP_EVENT_STANDBY_TO_RESERVED,
-                                 pbdev->fh, pbdev->fid);
-    pbdev->fh = 0;
-    pbdev->fid = 0;
-    pbdev->pdev = NULL;
     object_unparent(OBJECT(pci_dev));
 }
 
 static void s390_pcihost_class_init(ObjectClass *klass, void *data)
 {
     SysBusDeviceClass *k = SYS_BUS_DEVICE_CLASS(klass);
-    DeviceClass *dc = DEVICE_CLASS(klass);
     HotplugHandlerClass *hc = HOTPLUG_HANDLER_CLASS(klass);
 
-    dc->cannot_instantiate_with_device_add_yet = true;
     k->init = s390_pcihost_init;
     hc->plug = s390_pcihost_hot_plug;
     hc->unplug = s390_pcihost_hot_unplug;
@@ -585,9 +595,156 @@ static const TypeInfo s390_pcihost_info = {
     }
 };
 
+static void s390_pci_device_hot_plug(HotplugHandler *hotplug_dev,
+                                     DeviceState *dev, Error **errp)
+{
+    S390PCIBusDevice *zpci = S390_PCI_DEVICE(dev);
+
+    zpci->configured = true;
+
+    if (dev->hotplugged) {
+        s390_pci_generate_plug_event(HP_EVENT_RESERVED_TO_STANDBY,
+                                     zpci->fh, zpci->fid);
+        s390_pci_generate_plug_event(HP_EVENT_TO_CONFIGURED,
+                                     zpci->fh, zpci->fid);
+    }
+}
+
+static void s390_pci_device_hot_unplug_request(HotplugHandler *hotplug_dev,
+                                       DeviceState *dev, Error **errp)
+{
+    S390PCIBusDevice *zpci = S390_PCI_DEVICE(dev);
+
+    if (zpci->configured) {
+        zpci->configured = false;
+        s390_pci_generate_plug_event(HP_EVENT_CONFIGURED_TO_STBRES,
+                                     zpci->fh, zpci->fid);
+    }
+
+    s390_pci_generate_plug_event(HP_EVENT_STANDBY_TO_RESERVED,
+                                 zpci->fh, zpci->fid);
+
+    zpci->is_unplugged = true;
+}
+
+static const TypeInfo s390_pci_fac_bus_info = {
+    .name = TYPE_S390_PCI_FAC_BUS,
+    .parent = TYPE_BUS,
+    .instance_size = sizeof(S390PCIFacBus),
+};
+
+static int s390_pci_facility_init(S390PCIFacility *f)
+{
+    DeviceState *dev = DEVICE(f);
+
+    QTAILQ_INIT(&f->pending_sei);
+    msi_supported = true;
+    f->fbus = S390_PCI_FAC_BUS(qbus_create(TYPE_S390_PCI_FAC_BUS, dev, NULL));
+    qbus_set_hotplug_handler(BUS(&f->fbus->qbus), DEVICE(dev), NULL);
+
+    return 0;
+}
+
+static void s390_pci_facility_class_init(ObjectClass *klass, void *data)
+{
+    S390PCIFacilityClass *k = S390_PCI_FACILITY_CLASS(klass);
+    HotplugHandlerClass *hc = HOTPLUG_HANDLER_CLASS(k);
+
+    k->init = s390_pci_facility_init;
+    hc->plug = s390_pci_device_hot_plug;
+    hc->unplug_request = s390_pci_device_hot_unplug_request;
+}
+
+static const TypeInfo s390_pci_facility_info = {
+    .name          = TYPE_S390_PCI_FACILITY,
+    .parent        = TYPE_SYS_BUS_DEVICE,
+    .instance_size = sizeof(S390PCIFacility),
+    .class_init    = s390_pci_facility_class_init,
+    .class_size    = sizeof(S390PCIFacilityClass),
+    .interfaces = (InterfaceInfo[]) {
+        { TYPE_HOTPLUG_HANDLER },
+        { }
+    }
+};
+
+static void s390_pci_device_realize(DeviceState *dev, Error **errp)
+{
+    S390PCIBusDevice *zpci = S390_PCI_DEVICE(dev);
+    S390PCIBusDevice *tmp;
+    S390pciState *s;
+    BusChild *kid;
+    PCIDevice *pdev;
+    int ret;
+    S390PCIFacility *f = S390_PCI_FACILITY(
+        object_resolve_path(TYPE_S390_PCI_FACILITY, NULL));
+
+    ret = pci_qdev_find_device(zpci->pci_id, &pdev);
+    if (ret < 0) {
+        error_setg(errp, "vfio pci device %s not found", zpci->pci_id);
+        return;
+    }
+
+    QTAILQ_FOREACH(kid, &f->fbus->qbus.children, sibling) {
+        tmp = (S390PCIBusDevice *)kid->child;
+        if (tmp == zpci) {
+            continue;
+        }
+
+        if (tmp->fid == zpci->fid || tmp->uid == zpci->uid ||
+            !strcmp(tmp->pci_id, zpci->pci_id)) {
+            error_setg(errp, "zpci needs unique fid, uid and pci_id");
+            return;
+        }
+    }
+
+    s = S390_PCI_HOST_BRIDGE(pci_device_root_bus(pdev)->qbus.parent);
+    s->conn[PCI_SLOT(pdev->devfn)].zpci = zpci;
+
+    zpci->pdev = pdev;
+    zpci->fh = zpci->fid | FH_VIRT;
+    s390_pcihost_setup_msix(zpci);
+}
+
+static void s390_pci_device_unrealize(DeviceState *dev, Error **errp)
+{
+    S390PCIBusDevice *zpci = S390_PCI_DEVICE(dev);
+
+    zpci->fh = 0;
+    zpci->fid = 0;
+    zpci->pdev = NULL;
+}
+
+static Property s390_pci_device_properties[] = {
+    DEFINE_PROP_UINT32("fid", S390PCIBusDevice, fid, 0),
+    DEFINE_PROP_UINT32("uid", S390PCIBusDevice, uid, 0),
+    DEFINE_PROP_STRING("pci_id", S390PCIBusDevice, pci_id),
+    DEFINE_PROP_END_OF_LIST(),
+};
+
+static void s390_pci_device_class_init(ObjectClass *klass, void *data)
+{
+    DeviceClass *dc = DEVICE_CLASS(klass);
+
+    dc->desc = "s390 pci device";
+    dc->bus_type = TYPE_S390_PCI_FAC_BUS;
+    dc->realize = s390_pci_device_realize;
+    dc->unrealize = s390_pci_device_unrealize;
+    dc->props = s390_pci_device_properties;
+}
+
+static const TypeInfo s390_pci_device_type_info = {
+    .name = TYPE_S390_PCI_DEVICE,
+    .parent = TYPE_DEVICE,
+    .instance_size = sizeof(S390PCIBusDevice),
+    .class_init = s390_pci_device_class_init,
+};
+
 static void s390_pci_register_types(void)
 {
     type_register_static(&s390_pcihost_info);
+    type_register_static(&s390_pci_facility_info);
+    type_register_static(&s390_pci_fac_bus_info);
+    type_register_static(&s390_pci_device_type_info);
 }
 
 type_init(s390_pci_register_types)
diff --git a/hw/s390x/s390-pci-bus.h b/hw/s390x/s390-pci-bus.h
index 464a92e..5bf3913 100644
--- a/hw/s390x/s390-pci-bus.h
+++ b/hw/s390x/s390-pci-bus.h
@@ -149,6 +149,21 @@ enum ZpciIoatDtype {
 #define ZPCI_TABLE_VALID_MASK           0x20
 #define ZPCI_TABLE_PROT_MASK            0x200
 
+#define TYPE_S390_PCI_FACILITY "s390-pci-facility"
+#define TYPE_S390_PCI_FAC_BUS "s390-pci-fac-bus"
+#define TYPE_S390_PCI_DEVICE "zpci"
+
+#define S390_PCI_FACILITY(obj) \
+    OBJECT_CHECK(S390PCIFacility, (obj), TYPE_S390_PCI_FACILITY)
+#define S390_PCI_FAC_BUS(obj) \
+    OBJECT_CHECK(S390PCIFacBus, (obj), TYPE_S390_PCI_FAC_BUS)
+#define S390_PCI_FACILITY_CLASS(klass) \
+    OBJECT_CLASS_CHECK(S390PCIFacilityClass, (klass), TYPE_S390_PCI_FACILITY)
+#define S390_PCI_DEVICE(obj) \
+    OBJECT_CHECK(S390PCIBusDevice, (obj), TYPE_S390_PCI_DEVICE)
+#define S390_PCI_FACILITY_GET_CLASS(obj) \
+    OBJECT_GET_CLASS(S390PCIFacilityClass, (obj), TYPE_S390_PCI_FACILITY)
+
 typedef struct SeiContainer {
     QTAILQ_ENTRY(SeiContainer) link;
     uint32_t fid;
@@ -214,12 +229,16 @@ typedef struct S390MsixInfo {
 } S390MsixInfo;
 
 typedef struct S390PCIBusDevice {
+    DeviceState qdev;
     PCIDevice *pdev;
     bool configured;
+    bool is_unplugged;
     bool error_state;
     bool lgstg_blocked;
     uint32_t fh;
     uint32_t fid;
+    uint32_t uid;
+    char *pci_id;
     uint64_t g_iota;
     uint64_t pba;
     uint64_t pal;
@@ -229,21 +248,42 @@ typedef struct S390PCIBusDevice {
     uint8_t sum;
     S390MsixInfo msix;
     AdapterRoutes routes;
-    AddressSpace as;
-    MemoryRegion mr;
+    QLIST_ENTRY(S390PCIDevice) entry;
 } S390PCIBusDevice;
 
+typedef struct S390PCIDeviceConn {
+    S390PCIBusDevice *zpci;
+    AddressSpace iommu_as;
+    MemoryRegion iommu_mr;
+} S390PCIDeviceConn;
+
 typedef struct S390pciState {
     PCIHostState parent_obj;
-    S390PCIBusDevice pbdev[PCI_SLOT_MAX];
+    S390PCIDeviceConn conn[PCI_SLOT_MAX];
     AddressSpace msix_notify_as;
     MemoryRegion msix_notify_mr;
-    QTAILQ_HEAD(, SeiContainer) pending_sei;
 } S390pciState;
 
+typedef struct S390PCIFacBus {
+    BusState qbus;
+} S390PCIFacBus;
+
+typedef struct S390PCIFacility {
+    SysBusDevice parent_obj;
+    S390PCIFacBus *fbus;
+    QTAILQ_HEAD(, SeiContainer) pending_sei;
+} S390PCIFacility;
+
+typedef struct S390PCIFacilityClass {
+    DeviceClass parent_class;
+    int (*init)(S390PCIFacility *f);
+} S390PCIFacilityClass;
+
 int chsc_sei_nt2_get_event(void *res);
 int chsc_sei_nt2_have_event(void);
 void s390_pci_sclp_configure(int configure, SCCB *sccb);
+void s390_pci_device_enable(S390PCIBusDevice *zpci);
+void s390_pci_device_disable(S390PCIBusDevice *zpci);
 S390PCIBusDevice *s390_pci_find_dev_by_idx(uint32_t idx);
 S390PCIBusDevice *s390_pci_find_dev_by_fh(uint32_t fh);
 S390PCIBusDevice *s390_pci_find_dev_by_fid(uint32_t fid);
diff --git a/hw/s390x/s390-pci-inst.c b/hw/s390x/s390-pci-inst.c
index cac3d83..fc8e8e1 100644
--- a/hw/s390x/s390-pci-inst.c
+++ b/hw/s390x/s390-pci-inst.c
@@ -208,12 +208,12 @@ int clp_service_call(S390CPU *cpu, uint8_t r2)
 
         switch (reqsetpci->oc) {
         case CLP_SET_ENABLE_PCI_FN:
-            pbdev->fh = pbdev->fh | 1 << ENABLE_BIT_OFFSET;
+            s390_pci_device_enable(pbdev);
             stl_p(&ressetpci->fh, pbdev->fh);
             stw_p(&ressetpci->hdr.rsp, CLP_RC_OK);
             break;
         case CLP_SET_DISABLE_PCI_FN:
-            pbdev->fh = pbdev->fh & ~(1 << ENABLE_BIT_OFFSET);
+            s390_pci_device_disable(pbdev);
             pbdev->error_state = false;
             pbdev->lgstg_blocked = false;
             stl_p(&ressetpci->fh, pbdev->fh);
diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c
index 312b8ac..7ee5202 100644
--- a/hw/s390x/s390-virtio-ccw.c
+++ b/hw/s390x/s390-virtio-ccw.c
@@ -177,8 +177,8 @@ static void ccw_init(MachineState *machine)
                       machine->initrd_filename, "s390-ccw.img", true);
     s390_flic_init();
 
-    dev = qdev_create(NULL, TYPE_S390_PCI_HOST_BRIDGE);
-    object_property_add_child(qdev_get_machine(), TYPE_S390_PCI_HOST_BRIDGE,
+    dev = qdev_create(NULL, TYPE_S390_PCI_FACILITY);
+    object_property_add_child(qdev_get_machine(), TYPE_S390_PCI_FACILITY,
                               OBJECT(dev), NULL);
     qdev_init_nofail(dev);
 
-- 
1.9.3

[Qemu-devel] [PATCH v1 0/1] s390 pci infrastucture modeling

[Hong Bo Li]

This patch extends the current s390 pci implementation to
provide more flexibility in configuration of s390 specific
device handling. For this we had to introduce a new facility
(and bus) to hold devices representing information actually
provided by s390 firmware and I/O configuration.

For each vfio pci device, I create a zpci device to store s390
specific informations. And attach all of these special zpci devices
to the s390 facility bus. A zpci device references the corresponding
PCI device via device id. 

Compare to the old implementation, I moved the actual hotplug/unplug 
codes to s390 pci device hot plug function. Then in the pcihost 
hotplug function, we don't need to do anything special. In the pcihost
unplug function, we need to unplug the corresponding zpci device.

The new design allows to define multiple host bridges, each host bridge
could hold 32 zpci devices at most.

The topology for this implementation could be:

  dev: s390-pcihost, id ""
    bus: pci.0
      type PCI
      dev: vfio-pci, id "vpci1"
        host = "0000:00:00.0"
	......
      dev: vfio-pci, id "vpci2"
        host = "0001:00:00.0"
	......
  dev: s390-pci-facility, id ""
    bus: s390-pci-fac-bus.0
      type s390-pci-fac-bus
      dev: zpci, id "zpci1"
        fid = 1 (0x1)
        uid = 2 (0x2)
        pci_id = "vpci1"
      dev: zpci, id "zpci2"
        fid = 6 (0x6)
        uid = 7 (0x7)
        pci_id = "vpci2"

To make the review easier, I keep all of the old names, such as 
S390PCIBusDevice to name a zpci device. I will make a cleanup 
patch later to change these names to a more suitable name.

Hong Bo Li (1):
  S390 pci infrastructure modeling

 hw/s390x/s390-pci-bus.c    | 317 +++++++++++++++++++++++++++++++++------------
 hw/s390x/s390-pci-bus.h    |  48 ++++++-
 hw/s390x/s390-pci-inst.c   |   4 +-
 hw/s390x/s390-virtio-ccw.c |   4 +-
 4 files changed, 285 insertions(+), 88 deletions(-)

-- 
1.9.3

[Qemu-devel] [PATCH v1 1/1] S390 pci infrastructure modeling

[Hong Bo Li]

This patch contains the actual interesting changes.
usage example:
-device s390-pcihost
-device vfio-pci,host=0000:00:00.0,id=vpci1
-device zpci,fid=2,uid=5,pci_id=vpci1,id=zpci1

The first line will create a s390 pci host bridge
and init the root bus. The second line will create
a standard vfio pci device, and attach it to the
root bus. These are similiar to the standard process
to define a pci device on other platform.

The third line will create a s390 pci device to
store s390 specific information, and references
the corresponding vfio pci device via device id.
We create a s390 pci facility bus to hold all the
zpci devices.

Signed-off-by: Hong Bo Li <lihbbj@linux.vnet.ibm.com>
---
 hw/s390x/s390-pci-bus.c    | 317 +++++++++++++++++++++++++++++++++------------
 hw/s390x/s390-pci-bus.h    |  48 ++++++-
 hw/s390x/s390-pci-inst.c   |   4 +-
 hw/s390x/s390-virtio-ccw.c |   4 +-
 4 files changed, 285 insertions(+), 88 deletions(-)

diff --git a/hw/s390x/s390-pci-bus.c b/hw/s390x/s390-pci-bus.c
index 3c086f6..c81093e 100644
--- a/hw/s390x/s390-pci-bus.c
+++ b/hw/s390x/s390-pci-bus.c
@@ -32,8 +32,8 @@ int chsc_sei_nt2_get_event(void *res)
     PciCcdfErr *eccdf;
     int rc = 1;
     SeiContainer *sei_cont;
-    S390pciState *s = S390_PCI_HOST_BRIDGE(
-        object_resolve_path(TYPE_S390_PCI_HOST_BRIDGE, NULL));
+    S390PCIFacility *s = S390_PCI_FACILITY(
+        object_resolve_path(TYPE_S390_PCI_FACILITY, NULL));
 
     if (!s) {
         return rc;
@@ -72,8 +72,8 @@ int chsc_sei_nt2_get_event(void *res)
 
 int chsc_sei_nt2_have_event(void)
 {
-    S390pciState *s = S390_PCI_HOST_BRIDGE(
-        object_resolve_path(TYPE_S390_PCI_HOST_BRIDGE, NULL));
+    S390PCIFacility *s = S390_PCI_FACILITY(
+        object_resolve_path(TYPE_S390_PCI_FACILITY, NULL));
 
     if (!s) {
         return 0;
@@ -82,20 +82,32 @@ int chsc_sei_nt2_have_event(void)
     return !QTAILQ_EMPTY(&s->pending_sei);
 }
 
+void s390_pci_device_enable(S390PCIBusDevice *zpci)
+{
+    zpci->fh = zpci->fh | 1 << ENABLE_BIT_OFFSET;
+}
+
+void s390_pci_device_disable(S390PCIBusDevice *zpci)
+{
+    zpci->fh = zpci->fh & ~(1 << ENABLE_BIT_OFFSET);
+    if (zpci->is_unplugged)
+        object_unparent(OBJECT(zpci));
+}
+
 S390PCIBusDevice *s390_pci_find_dev_by_fid(uint32_t fid)
 {
     S390PCIBusDevice *pbdev;
-    int i;
-    S390pciState *s = S390_PCI_HOST_BRIDGE(
-        object_resolve_path(TYPE_S390_PCI_HOST_BRIDGE, NULL));
+    BusChild *kid;
+    S390PCIFacility *s = S390_PCI_FACILITY(
+        object_resolve_path(TYPE_S390_PCI_FACILITY, NULL));
 
     if (!s) {
         return NULL;
     }
 
-    for (i = 0; i < PCI_SLOT_MAX; i++) {
-        pbdev = &s->pbdev[i];
-        if ((pbdev->fh != 0) && (pbdev->fid == fid)) {
+    QTAILQ_FOREACH(kid, &s->fbus->qbus.children, sibling) {
+        pbdev = (S390PCIBusDevice *)kid->child;
+        if (pbdev->fid == fid) {
             return pbdev;
         }
     }
@@ -126,39 +138,24 @@ void s390_pci_sclp_configure(int configure, SCCB *sccb)
     return;
 }
 
-static uint32_t s390_pci_get_pfid(PCIDevice *pdev)
-{
-    return PCI_SLOT(pdev->devfn);
-}
-
-static uint32_t s390_pci_get_pfh(PCIDevice *pdev)
-{
-    return PCI_SLOT(pdev->devfn) | FH_VIRT;
-}
-
 S390PCIBusDevice *s390_pci_find_dev_by_idx(uint32_t idx)
 {
     S390PCIBusDevice *pbdev;
-    int i;
-    int j = 0;
-    S390pciState *s = S390_PCI_HOST_BRIDGE(
-        object_resolve_path(TYPE_S390_PCI_HOST_BRIDGE, NULL));
+    BusChild *kid;
+    int i = 0;
+    S390PCIFacility *s = S390_PCI_FACILITY(
+        object_resolve_path(TYPE_S390_PCI_FACILITY, NULL));
 
     if (!s) {
         return NULL;
     }
 
-    for (i = 0; i < PCI_SLOT_MAX; i++) {
-        pbdev = &s->pbdev[i];
-
-        if (pbdev->fh == 0) {
-            continue;
-        }
-
-        if (j == idx) {
+    QTAILQ_FOREACH(kid, &s->fbus->qbus.children, sibling) {
+        pbdev = (S390PCIBusDevice *)kid->child;
+        if (i == idx) {
             return pbdev;
         }
-        j++;
+        i++;
     }
 
     return NULL;
@@ -167,16 +164,17 @@ S390PCIBusDevice *s390_pci_find_dev_by_idx(uint32_t idx)
 S390PCIBusDevice *s390_pci_find_dev_by_fh(uint32_t fh)
 {
     S390PCIBusDevice *pbdev;
-    int i;
-    S390pciState *s = S390_PCI_HOST_BRIDGE(
-        object_resolve_path(TYPE_S390_PCI_HOST_BRIDGE, NULL));
+    BusChild *kid;
+    S390PCIFacility *s = S390_PCI_FACILITY(
+        object_resolve_path(TYPE_S390_PCI_FACILITY, NULL));
+
 
     if (!s || !fh) {
         return NULL;
     }
 
-    for (i = 0; i < PCI_SLOT_MAX; i++) {
-        pbdev = &s->pbdev[i];
+    QTAILQ_FOREACH(kid, &s->fbus->qbus.children, sibling) {
+        pbdev = (S390PCIBusDevice *)kid->child;
         if (pbdev->fh == fh) {
             return pbdev;
         }
@@ -185,12 +183,33 @@ S390PCIBusDevice *s390_pci_find_dev_by_fh(uint32_t fh)
     return NULL;
 }
 
+static S390PCIBusDevice *s390_pci_find_dev_by_pdev(PCIDevice *pdev)
+{
+    S390PCIBusDevice *pbdev;
+    BusChild *kid;
+    S390PCIFacility *s = S390_PCI_FACILITY(
+        object_resolve_path(TYPE_S390_PCI_FACILITY, NULL));
+
+    if (!s || !pdev) {
+         return NULL;
+     }
+
+    QTAILQ_FOREACH(kid, &s->fbus->qbus.children, sibling) {
+        pbdev = (S390PCIBusDevice *)kid->child;
+        if (pbdev->pdev == pdev) {
+            return pbdev;
+        }
+    }
+
+    return NULL;
+}
+
 static void s390_pci_generate_event(uint8_t cc, uint16_t pec, uint32_t fh,
                                     uint32_t fid, uint64_t faddr, uint32_t e)
 {
     SeiContainer *sei_cont;
-    S390pciState *s = S390_PCI_HOST_BRIDGE(
-        object_resolve_path(TYPE_S390_PCI_HOST_BRIDGE, NULL));
+    S390PCIFacility *s = S390_PCI_FACILITY(
+        object_resolve_path(TYPE_S390_PCI_FACILITY, NULL));
 
     if (!s) {
         return;
@@ -305,7 +324,9 @@ static IOMMUTLBEntry s390_translate_iommu(MemoryRegion *iommu, hwaddr addr,
 {
     uint64_t pte;
     uint32_t flags;
-    S390PCIBusDevice *pbdev = container_of(iommu, S390PCIBusDevice, mr);
+    S390PCIDeviceConn *conn = container_of(iommu, S390PCIDeviceConn,
+                                           iommu_mr);
+    S390PCIBusDevice *pbdev = conn->zpci;
     S390pciState *s = S390_PCI_HOST_BRIDGE(pci_device_root_bus(pbdev->pdev)
                                            ->qbus.parent);
     IOMMUTLBEntry ret = {
@@ -316,8 +337,14 @@ static IOMMUTLBEntry s390_translate_iommu(MemoryRegion *iommu, hwaddr addr,
         .perm = IOMMU_NONE,
     };
 
+    if (!pbdev) {
+        return ret;
+    }
+
     DPRINTF("iommu trans addr 0x%" PRIx64 "\n", addr);
 
+    s = S390_PCI_HOST_BRIDGE(pci_device_root_bus(pbdev->pdev)->qbus.parent);
+
     /* s390 does not have an APIC mapped to main storage so we use
      * a separate AddressSpace only for msix notifications
      */
@@ -379,7 +406,7 @@ static AddressSpace *s390_pci_dma_iommu(PCIBus *bus, void *opaque, int devfn)
 {
     S390pciState *s = opaque;
 
-    return &s->pbdev[PCI_SLOT(devfn)].as;
+    return &s->conn[PCI_SLOT(devfn)].iommu_as;
 }
 
 static uint8_t set_ind_atomic(uint64_t ind_loc, uint8_t to_be_set)
@@ -452,9 +479,10 @@ static void s390_pcihost_init_as(S390pciState *s)
     int i;
 
     for (i = 0; i < PCI_SLOT_MAX; i++) {
-        memory_region_init_iommu(&s->pbdev[i].mr, OBJECT(s),
+        memory_region_init_iommu(&s->conn[i].iommu_mr, OBJECT(s),
                                  &s390_iommu_ops, "iommu-s390", UINT64_MAX);
-        address_space_init(&s->pbdev[i].as, &s->pbdev[i].mr, "iommu-pci");
+        address_space_init(&s->conn[i].iommu_as, &s->conn[i].iommu_mr,
+                           "iommu-pci");
     }
 
     memory_region_init_io(&s->msix_notify_mr, OBJECT(s),
@@ -481,7 +509,7 @@ static int s390_pcihost_init(SysBusDevice *dev)
     bus = BUS(b);
     qbus_set_hotplug_handler(bus, DEVICE(dev), NULL);
     phb->bus = b;
-    QTAILQ_INIT(&s->pending_sei);
+
     return 0;
 }
 
@@ -513,29 +541,12 @@ static int s390_pcihost_setup_msix(S390PCIBusDevice *pbdev)
     return 0;
 }
 
+/* this function is useless, just keep it to make the patch more
+   readable. Without this function, the diff will mix these hotplug
+   functions together. will be removed in future */
 static void s390_pcihost_hot_plug(HotplugHandler *hotplug_dev,
                                   DeviceState *dev, Error **errp)
 {
-    PCIDevice *pci_dev = PCI_DEVICE(dev);
-    S390PCIBusDevice *pbdev;
-    S390pciState *s = S390_PCI_HOST_BRIDGE(pci_device_root_bus(pci_dev)
-                                           ->qbus.parent);
-
-    pbdev = &s->pbdev[PCI_SLOT(pci_dev->devfn)];
-
-    pbdev->fid = s390_pci_get_pfid(pci_dev);
-    pbdev->pdev = pci_dev;
-    pbdev->configured = true;
-    pbdev->fh = s390_pci_get_pfh(pci_dev);
-
-    s390_pcihost_setup_msix(pbdev);
-
-    if (dev->hotplugged) {
-        s390_pci_generate_plug_event(HP_EVENT_RESERVED_TO_STANDBY,
-                                     pbdev->fh, pbdev->fid);
-        s390_pci_generate_plug_event(HP_EVENT_TO_CONFIGURED,
-                                     pbdev->fh, pbdev->fid);
-    }
     return;
 }
 
@@ -543,31 +554,30 @@ static void s390_pcihost_hot_unplug(HotplugHandler *hotplug_dev,
                                     DeviceState *dev, Error **errp)
 {
     PCIDevice *pci_dev = PCI_DEVICE(dev);
-    S390pciState *s = S390_PCI_HOST_BRIDGE(pci_device_root_bus(pci_dev)
-                                           ->qbus.parent);
-    S390PCIBusDevice *pbdev = &s->pbdev[PCI_SLOT(pci_dev->devfn)];
-
-    if (pbdev->configured) {
-        pbdev->configured = false;
-        s390_pci_generate_plug_event(HP_EVENT_CONFIGURED_TO_STBRES,
-                                     pbdev->fh, pbdev->fid);
+    S390PCIBusDevice *pbdev;
+    HotplugHandler *hotplug_ctrl;
+    S390PCIFacility *f = S390_PCI_FACILITY(
+        object_resolve_path(TYPE_S390_PCI_FACILITY, NULL));
+    S390PCIFacilityClass *k = S390_PCI_FACILITY_GET_CLASS(f);
+    HotplugHandlerClass *hdc = HOTPLUG_HANDLER_CLASS(k);
+
+    /* unplug corresponding zpci device */
+    pbdev = s390_pci_find_dev_by_pdev(pci_dev);
+    if (pbdev) {
+        hotplug_ctrl = pbdev->qdev.parent_bus->hotplug_handler;
+        if (hdc->unplug_request) {
+            hdc->unplug_request(hotplug_ctrl, &pbdev->qdev, errp);
+        }
     }
 
-    s390_pci_generate_plug_event(HP_EVENT_STANDBY_TO_RESERVED,
-                                 pbdev->fh, pbdev->fid);
-    pbdev->fh = 0;
-    pbdev->fid = 0;
-    pbdev->pdev = NULL;
     object_unparent(OBJECT(pci_dev));
 }
 
 static void s390_pcihost_class_init(ObjectClass *klass, void *data)
 {
     SysBusDeviceClass *k = SYS_BUS_DEVICE_CLASS(klass);
-    DeviceClass *dc = DEVICE_CLASS(klass);
     HotplugHandlerClass *hc = HOTPLUG_HANDLER_CLASS(klass);
 
-    dc->cannot_instantiate_with_device_add_yet = true;
     k->init = s390_pcihost_init;
     hc->plug = s390_pcihost_hot_plug;
     hc->unplug = s390_pcihost_hot_unplug;
@@ -585,9 +595,156 @@ static const TypeInfo s390_pcihost_info = {
     }
 };
 
+static void s390_pci_device_hot_plug(HotplugHandler *hotplug_dev,
+                                     DeviceState *dev, Error **errp)
+{
+    S390PCIBusDevice *zpci = S390_PCI_DEVICE(dev);
+
+    zpci->configured = true;
+
+    if (dev->hotplugged) {
+        s390_pci_generate_plug_event(HP_EVENT_RESERVED_TO_STANDBY,
+                                     zpci->fh, zpci->fid);
+        s390_pci_generate_plug_event(HP_EVENT_TO_CONFIGURED,
+                                     zpci->fh, zpci->fid);
+    }
+}
+
+static void s390_pci_device_hot_unplug_request(HotplugHandler *hotplug_dev,
+                                       DeviceState *dev, Error **errp)
+{
+    S390PCIBusDevice *zpci = S390_PCI_DEVICE(dev);
+
+    if (zpci->configured) {
+        zpci->configured = false;
+        s390_pci_generate_plug_event(HP_EVENT_CONFIGURED_TO_STBRES,
+                                     zpci->fh, zpci->fid);
+    }
+
+    s390_pci_generate_plug_event(HP_EVENT_STANDBY_TO_RESERVED,
+                                 zpci->fh, zpci->fid);
+
+    zpci->is_unplugged = true;
+}
+
+static const TypeInfo s390_pci_fac_bus_info = {
+    .name = TYPE_S390_PCI_FAC_BUS,
+    .parent = TYPE_BUS,
+    .instance_size = sizeof(S390PCIFacBus),
+};
+
+static int s390_pci_facility_init(S390PCIFacility *f)
+{
+    DeviceState *dev = DEVICE(f);
+
+    QTAILQ_INIT(&f->pending_sei);
+    msi_supported = true;
+    f->fbus = S390_PCI_FAC_BUS(qbus_create(TYPE_S390_PCI_FAC_BUS, dev, NULL));
+    qbus_set_hotplug_handler(BUS(&f->fbus->qbus), DEVICE(dev), NULL);
+
+    return 0;
+}
+
+static void s390_pci_facility_class_init(ObjectClass *klass, void *data)
+{
+    S390PCIFacilityClass *k = S390_PCI_FACILITY_CLASS(klass);
+    HotplugHandlerClass *hc = HOTPLUG_HANDLER_CLASS(k);
+
+    k->init = s390_pci_facility_init;
+    hc->plug = s390_pci_device_hot_plug;
+    hc->unplug_request = s390_pci_device_hot_unplug_request;
+}
+
+static const TypeInfo s390_pci_facility_info = {
+    .name          = TYPE_S390_PCI_FACILITY,
+    .parent        = TYPE_SYS_BUS_DEVICE,
+    .instance_size = sizeof(S390PCIFacility),
+    .class_init    = s390_pci_facility_class_init,
+    .class_size    = sizeof(S390PCIFacilityClass),
+    .interfaces = (InterfaceInfo[]) {
+        { TYPE_HOTPLUG_HANDLER },
+        { }
+    }
+};
+
+static void s390_pci_device_realize(DeviceState *dev, Error **errp)
+{
+    S390PCIBusDevice *zpci = S390_PCI_DEVICE(dev);
+    S390PCIBusDevice *tmp;
+    S390pciState *s;
+    BusChild *kid;
+    PCIDevice *pdev;
+    int ret;
+    S390PCIFacility *f = S390_PCI_FACILITY(
+        object_resolve_path(TYPE_S390_PCI_FACILITY, NULL));
+
+    ret = pci_qdev_find_device(zpci->pci_id, &pdev);
+    if (ret < 0) {
+        error_setg(errp, "vfio pci device %s not found", zpci->pci_id);
+        return;
+    }
+
+    QTAILQ_FOREACH(kid, &f->fbus->qbus.children, sibling) {
+        tmp = (S390PCIBusDevice *)kid->child;
+        if (tmp == zpci) {
+            continue;
+        }
+
+        if (tmp->fid == zpci->fid || tmp->uid == zpci->uid ||
+            !strcmp(tmp->pci_id, zpci->pci_id)) {
+            error_setg(errp, "zpci needs unique fid, uid and pci_id");
+            return;
+        }
+    }
+
+    s = S390_PCI_HOST_BRIDGE(pci_device_root_bus(pdev)->qbus.parent);
+    s->conn[PCI_SLOT(pdev->devfn)].zpci = zpci;
+
+    zpci->pdev = pdev;
+    zpci->fh = zpci->fid | FH_VIRT;
+    s390_pcihost_setup_msix(zpci);
+}
+
+static void s390_pci_device_unrealize(DeviceState *dev, Error **errp)
+{
+    S390PCIBusDevice *zpci = S390_PCI_DEVICE(dev);
+
+    zpci->fh = 0;
+    zpci->fid = 0;
+    zpci->pdev = NULL;
+}
+
+static Property s390_pci_device_properties[] = {
+    DEFINE_PROP_UINT32("fid", S390PCIBusDevice, fid, 0),
+    DEFINE_PROP_UINT32("uid", S390PCIBusDevice, uid, 0),
+    DEFINE_PROP_STRING("pci_id", S390PCIBusDevice, pci_id),
+    DEFINE_PROP_END_OF_LIST(),
+};
+
+static void s390_pci_device_class_init(ObjectClass *klass, void *data)
+{
+    DeviceClass *dc = DEVICE_CLASS(klass);
+
+    dc->desc = "s390 pci device";
+    dc->bus_type = TYPE_S390_PCI_FAC_BUS;
+    dc->realize = s390_pci_device_realize;
+    dc->unrealize = s390_pci_device_unrealize;
+    dc->props = s390_pci_device_properties;
+}
+
+static const TypeInfo s390_pci_device_type_info = {
+    .name = TYPE_S390_PCI_DEVICE,
+    .parent = TYPE_DEVICE,
+    .instance_size = sizeof(S390PCIBusDevice),
+    .class_init = s390_pci_device_class_init,
+};
+
 static void s390_pci_register_types(void)
 {
     type_register_static(&s390_pcihost_info);
+    type_register_static(&s390_pci_facility_info);
+    type_register_static(&s390_pci_fac_bus_info);
+    type_register_static(&s390_pci_device_type_info);
 }
 
 type_init(s390_pci_register_types)
diff --git a/hw/s390x/s390-pci-bus.h b/hw/s390x/s390-pci-bus.h
index 464a92e..5bf3913 100644
--- a/hw/s390x/s390-pci-bus.h
+++ b/hw/s390x/s390-pci-bus.h
@@ -149,6 +149,21 @@ enum ZpciIoatDtype {
 #define ZPCI_TABLE_VALID_MASK           0x20
 #define ZPCI_TABLE_PROT_MASK            0x200
 
+#define TYPE_S390_PCI_FACILITY "s390-pci-facility"
+#define TYPE_S390_PCI_FAC_BUS "s390-pci-fac-bus"
+#define TYPE_S390_PCI_DEVICE "zpci"
+
+#define S390_PCI_FACILITY(obj) \
+    OBJECT_CHECK(S390PCIFacility, (obj), TYPE_S390_PCI_FACILITY)
+#define S390_PCI_FAC_BUS(obj) \
+    OBJECT_CHECK(S390PCIFacBus, (obj), TYPE_S390_PCI_FAC_BUS)
+#define S390_PCI_FACILITY_CLASS(klass) \
+    OBJECT_CLASS_CHECK(S390PCIFacilityClass, (klass), TYPE_S390_PCI_FACILITY)
+#define S390_PCI_DEVICE(obj) \
+    OBJECT_CHECK(S390PCIBusDevice, (obj), TYPE_S390_PCI_DEVICE)
+#define S390_PCI_FACILITY_GET_CLASS(obj) \
+    OBJECT_GET_CLASS(S390PCIFacilityClass, (obj), TYPE_S390_PCI_FACILITY)
+
 typedef struct SeiContainer {
     QTAILQ_ENTRY(SeiContainer) link;
     uint32_t fid;
@@ -214,12 +229,16 @@ typedef struct S390MsixInfo {
 } S390MsixInfo;
 
 typedef struct S390PCIBusDevice {
+    DeviceState qdev;
     PCIDevice *pdev;
     bool configured;
+    bool is_unplugged;
     bool error_state;
     bool lgstg_blocked;
     uint32_t fh;
     uint32_t fid;
+    uint32_t uid;
+    char *pci_id;
     uint64_t g_iota;
     uint64_t pba;
     uint64_t pal;
@@ -229,21 +248,42 @@ typedef struct S390PCIBusDevice {
     uint8_t sum;
     S390MsixInfo msix;
     AdapterRoutes routes;
-    AddressSpace as;
-    MemoryRegion mr;
+    QLIST_ENTRY(S390PCIDevice) entry;
 } S390PCIBusDevice;
 
+typedef struct S390PCIDeviceConn {
+    S390PCIBusDevice *zpci;
+    AddressSpace iommu_as;
+    MemoryRegion iommu_mr;
+} S390PCIDeviceConn;
+
 typedef struct S390pciState {
     PCIHostState parent_obj;
-    S390PCIBusDevice pbdev[PCI_SLOT_MAX];
+    S390PCIDeviceConn conn[PCI_SLOT_MAX];
     AddressSpace msix_notify_as;
     MemoryRegion msix_notify_mr;
-    QTAILQ_HEAD(, SeiContainer) pending_sei;
 } S390pciState;
 
+typedef struct S390PCIFacBus {
+    BusState qbus;
+} S390PCIFacBus;
+
+typedef struct S390PCIFacility {
+    SysBusDevice parent_obj;
+    S390PCIFacBus *fbus;
+    QTAILQ_HEAD(, SeiContainer) pending_sei;
+} S390PCIFacility;
+
+typedef struct S390PCIFacilityClass {
+    DeviceClass parent_class;
+    int (*init)(S390PCIFacility *f);
+} S390PCIFacilityClass;
+
 int chsc_sei_nt2_get_event(void *res);
 int chsc_sei_nt2_have_event(void);
 void s390_pci_sclp_configure(int configure, SCCB *sccb);
+void s390_pci_device_enable(S390PCIBusDevice *zpci);
+void s390_pci_device_disable(S390PCIBusDevice *zpci);
 S390PCIBusDevice *s390_pci_find_dev_by_idx(uint32_t idx);
 S390PCIBusDevice *s390_pci_find_dev_by_fh(uint32_t fh);
 S390PCIBusDevice *s390_pci_find_dev_by_fid(uint32_t fid);
diff --git a/hw/s390x/s390-pci-inst.c b/hw/s390x/s390-pci-inst.c
index cac3d83..fc8e8e1 100644
--- a/hw/s390x/s390-pci-inst.c
+++ b/hw/s390x/s390-pci-inst.c
@@ -208,12 +208,12 @@ int clp_service_call(S390CPU *cpu, uint8_t r2)
 
         switch (reqsetpci->oc) {
         case CLP_SET_ENABLE_PCI_FN:
-            pbdev->fh = pbdev->fh | 1 << ENABLE_BIT_OFFSET;
+            s390_pci_device_enable(pbdev);
             stl_p(&ressetpci->fh, pbdev->fh);
             stw_p(&ressetpci->hdr.rsp, CLP_RC_OK);
             break;
         case CLP_SET_DISABLE_PCI_FN:
-            pbdev->fh = pbdev->fh & ~(1 << ENABLE_BIT_OFFSET);
+            s390_pci_device_disable(pbdev);
             pbdev->error_state = false;
             pbdev->lgstg_blocked = false;
             stl_p(&ressetpci->fh, pbdev->fh);
diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c
index 312b8ac..7ee5202 100644
--- a/hw/s390x/s390-virtio-ccw.c
+++ b/hw/s390x/s390-virtio-ccw.c
@@ -177,8 +177,8 @@ static void ccw_init(MachineState *machine)
                       machine->initrd_filename, "s390-ccw.img", true);
     s390_flic_init();
 
-    dev = qdev_create(NULL, TYPE_S390_PCI_HOST_BRIDGE);
-    object_property_add_child(qdev_get_machine(), TYPE_S390_PCI_HOST_BRIDGE,
+    dev = qdev_create(NULL, TYPE_S390_PCI_FACILITY);
+    object_property_add_child(qdev_get_machine(), TYPE_S390_PCI_FACILITY,
                               OBJECT(dev), NULL);
     qdev_init_nofail(dev);
 
-- 
1.9.3

Re: [Qemu-devel] [PATCH v1 1/1] S390 pci infrastructure modeling

[Hong Bo Li]

Sorry, I sent the patch three times by accident.  Please take this one
as the formal patch.

This is a followup to the discussion started by Frank Blaschka:

Option 1: s390x/pci: Extend pci representation by new zpci device
Option 2: s390x/pci: rework pci infrastructure modeling

This patch provides an option 3, it is similar to option 1, but did
some changes to hot-plug & hot-unplug.

The process of hot-plug looks like:
    a. hot-plug a vfio-pci device, we need to do nothing for this step.
    b. hot-plug a zpci device, we do the actual zpci device hot-plug
       action in this step, and store specific s390 information.

We have two ways for hot-unplug:

1. hot-unplug zpci device first, then unplug vfio-pci device.
2. hot-unplug vfio-pci device directly, the unplug handler will unplug
    the corresponding zpci device.

This option 3 tries to address the hot-plug/unplug issue which is the major
concerned point in previous discussion. The idea of option 3 is: for each
vfio-pci device, we define a zpci device, and do all s390 specific action
on that device.

On 4/15/2015 16:04, Hong Bo Li wrote:
> This patch contains the actual interesting changes.
> usage example:
> -device s390-pcihost
> -device vfio-pci,host=0000:00:00.0,id=vpci1
> -device zpci,fid=2,uid=5,pci_id=vpci1,id=zpci1
>
> The first line will create a s390 pci host bridge
> and init the root bus. The second line will create
> a standard vfio pci device, and attach it to the
> root bus. These are similiar to the standard process
> to define a pci device on other platform.
>
> The third line will create a s390 pci device to
> store s390 specific information, and references
> the corresponding vfio pci device via device id.
> We create a s390 pci facility bus to hold all the
> zpci devices.
>
> Signed-off-by: Hong Bo Li <lihbbj@linux.vnet.ibm.com>
> ---
>   hw/s390x/s390-pci-bus.c    | 317 +++++++++++++++++++++++++++++++++------------
>   hw/s390x/s390-pci-bus.h    |  48 ++++++-
>   hw/s390x/s390-pci-inst.c   |   4 +-
>   hw/s390x/s390-virtio-ccw.c |   4 +-
>   4 files changed, 285 insertions(+), 88 deletions(-)
>
> diff --git a/hw/s390x/s390-pci-bus.c b/hw/s390x/s390-pci-bus.c
> index 3c086f6..c81093e 100644
> --- a/hw/s390x/s390-pci-bus.c
> +++ b/hw/s390x/s390-pci-bus.c
> @@ -32,8 +32,8 @@ int chsc_sei_nt2_get_event(void *res)
>       PciCcdfErr *eccdf;
>       int rc = 1;
>       SeiContainer *sei_cont;
> -    S390pciState *s = S390_PCI_HOST_BRIDGE(
> -        object_resolve_path(TYPE_S390_PCI_HOST_BRIDGE, NULL));
> +    S390PCIFacility *s = S390_PCI_FACILITY(
> +        object_resolve_path(TYPE_S390_PCI_FACILITY, NULL));
>
>       if (!s) {
>           return rc;
> @@ -72,8 +72,8 @@ int chsc_sei_nt2_get_event(void *res)
>
>   int chsc_sei_nt2_have_event(void)
>   {
> -    S390pciState *s = S390_PCI_HOST_BRIDGE(
> -        object_resolve_path(TYPE_S390_PCI_HOST_BRIDGE, NULL));
> +    S390PCIFacility *s = S390_PCI_FACILITY(
> +        object_resolve_path(TYPE_S390_PCI_FACILITY, NULL));
>
>       if (!s) {
>           return 0;
> @@ -82,20 +82,32 @@ int chsc_sei_nt2_have_event(void)
>       return !QTAILQ_EMPTY(&s->pending_sei);
>   }
>
> +void s390_pci_device_enable(S390PCIBusDevice *zpci)
> +{
> +    zpci->fh = zpci->fh | 1 << ENABLE_BIT_OFFSET;
> +}
> +
> +void s390_pci_device_disable(S390PCIBusDevice *zpci)
> +{
> +    zpci->fh = zpci->fh & ~(1 << ENABLE_BIT_OFFSET);
> +    if (zpci->is_unplugged)
> +        object_unparent(OBJECT(zpci));
> +}
> +
>   S390PCIBusDevice *s390_pci_find_dev_by_fid(uint32_t fid)
>   {
>       S390PCIBusDevice *pbdev;
> -    int i;
> -    S390pciState *s = S390_PCI_HOST_BRIDGE(
> -        object_resolve_path(TYPE_S390_PCI_HOST_BRIDGE, NULL));
> +    BusChild *kid;
> +    S390PCIFacility *s = S390_PCI_FACILITY(
> +        object_resolve_path(TYPE_S390_PCI_FACILITY, NULL));
>
>       if (!s) {
>           return NULL;
>       }
>
> -    for (i = 0; i < PCI_SLOT_MAX; i++) {
> -        pbdev = &s->pbdev[i];
> -        if ((pbdev->fh != 0) && (pbdev->fid == fid)) {
> +    QTAILQ_FOREACH(kid, &s->fbus->qbus.children, sibling) {
> +        pbdev = (S390PCIBusDevice *)kid->child;
> +        if (pbdev->fid == fid) {
>               return pbdev;
>           }
>       }
> @@ -126,39 +138,24 @@ void s390_pci_sclp_configure(int configure, SCCB *sccb)
>       return;
>   }
>
> -static uint32_t s390_pci_get_pfid(PCIDevice *pdev)
> -{
> -    return PCI_SLOT(pdev->devfn);
> -}
> -
> -static uint32_t s390_pci_get_pfh(PCIDevice *pdev)
> -{
> -    return PCI_SLOT(pdev->devfn) | FH_VIRT;
> -}
> -
>   S390PCIBusDevice *s390_pci_find_dev_by_idx(uint32_t idx)
>   {
>       S390PCIBusDevice *pbdev;
> -    int i;
> -    int j = 0;
> -    S390pciState *s = S390_PCI_HOST_BRIDGE(
> -        object_resolve_path(TYPE_S390_PCI_HOST_BRIDGE, NULL));
> +    BusChild *kid;
> +    int i = 0;
> +    S390PCIFacility *s = S390_PCI_FACILITY(
> +        object_resolve_path(TYPE_S390_PCI_FACILITY, NULL));
>
>       if (!s) {
>           return NULL;
>       }
>
> -    for (i = 0; i < PCI_SLOT_MAX; i++) {
> -        pbdev = &s->pbdev[i];
> -
> -        if (pbdev->fh == 0) {
> -            continue;
> -        }
> -
> -        if (j == idx) {
> +    QTAILQ_FOREACH(kid, &s->fbus->qbus.children, sibling) {
> +        pbdev = (S390PCIBusDevice *)kid->child;
> +        if (i == idx) {
>               return pbdev;
>           }
> -        j++;
> +        i++;
>       }
>
>       return NULL;
> @@ -167,16 +164,17 @@ S390PCIBusDevice *s390_pci_find_dev_by_idx(uint32_t idx)
>   S390PCIBusDevice *s390_pci_find_dev_by_fh(uint32_t fh)
>   {
>       S390PCIBusDevice *pbdev;
> -    int i;
> -    S390pciState *s = S390_PCI_HOST_BRIDGE(
> -        object_resolve_path(TYPE_S390_PCI_HOST_BRIDGE, NULL));
> +    BusChild *kid;
> +    S390PCIFacility *s = S390_PCI_FACILITY(
> +        object_resolve_path(TYPE_S390_PCI_FACILITY, NULL));
> +
>
>       if (!s || !fh) {
>           return NULL;
>       }
>
> -    for (i = 0; i < PCI_SLOT_MAX; i++) {
> -        pbdev = &s->pbdev[i];
> +    QTAILQ_FOREACH(kid, &s->fbus->qbus.children, sibling) {
> +        pbdev = (S390PCIBusDevice *)kid->child;
>           if (pbdev->fh == fh) {
>               return pbdev;
>           }
> @@ -185,12 +183,33 @@ S390PCIBusDevice *s390_pci_find_dev_by_fh(uint32_t fh)
>       return NULL;
>   }
>
> +static S390PCIBusDevice *s390_pci_find_dev_by_pdev(PCIDevice *pdev)
> +{
> +    S390PCIBusDevice *pbdev;
> +    BusChild *kid;
> +    S390PCIFacility *s = S390_PCI_FACILITY(
> +        object_resolve_path(TYPE_S390_PCI_FACILITY, NULL));
> +
> +    if (!s || !pdev) {
> +         return NULL;
> +     }
> +
> +    QTAILQ_FOREACH(kid, &s->fbus->qbus.children, sibling) {
> +        pbdev = (S390PCIBusDevice *)kid->child;
> +        if (pbdev->pdev == pdev) {
> +            return pbdev;
> +        }
> +    }
> +
> +    return NULL;
> +}
> +
>   static void s390_pci_generate_event(uint8_t cc, uint16_t pec, uint32_t fh,
>                                       uint32_t fid, uint64_t faddr, uint32_t e)
>   {
>       SeiContainer *sei_cont;
> -    S390pciState *s = S390_PCI_HOST_BRIDGE(
> -        object_resolve_path(TYPE_S390_PCI_HOST_BRIDGE, NULL));
> +    S390PCIFacility *s = S390_PCI_FACILITY(
> +        object_resolve_path(TYPE_S390_PCI_FACILITY, NULL));
>
>       if (!s) {
>           return;
> @@ -305,7 +324,9 @@ static IOMMUTLBEntry s390_translate_iommu(MemoryRegion *iommu, hwaddr addr,
>   {
>       uint64_t pte;
>       uint32_t flags;
> -    S390PCIBusDevice *pbdev = container_of(iommu, S390PCIBusDevice, mr);
> +    S390PCIDeviceConn *conn = container_of(iommu, S390PCIDeviceConn,
> +                                           iommu_mr);
> +    S390PCIBusDevice *pbdev = conn->zpci;
>       S390pciState *s = S390_PCI_HOST_BRIDGE(pci_device_root_bus(pbdev->pdev)
>                                              ->qbus.parent);
>       IOMMUTLBEntry ret = {
> @@ -316,8 +337,14 @@ static IOMMUTLBEntry s390_translate_iommu(MemoryRegion *iommu, hwaddr addr,
>           .perm = IOMMU_NONE,
>       };
>
> +    if (!pbdev) {
> +        return ret;
> +    }
> +
>       DPRINTF("iommu trans addr 0x%" PRIx64 "\n", addr);
>
> +    s = S390_PCI_HOST_BRIDGE(pci_device_root_bus(pbdev->pdev)->qbus.parent);
> +
>       /* s390 does not have an APIC mapped to main storage so we use
>        * a separate AddressSpace only for msix notifications
>        */
> @@ -379,7 +406,7 @@ static AddressSpace *s390_pci_dma_iommu(PCIBus *bus, void *opaque, int devfn)
>   {
>       S390pciState *s = opaque;
>
> -    return &s->pbdev[PCI_SLOT(devfn)].as;
> +    return &s->conn[PCI_SLOT(devfn)].iommu_as;
>   }
>
>   static uint8_t set_ind_atomic(uint64_t ind_loc, uint8_t to_be_set)
> @@ -452,9 +479,10 @@ static void s390_pcihost_init_as(S390pciState *s)
>       int i;
>
>       for (i = 0; i < PCI_SLOT_MAX; i++) {
> -        memory_region_init_iommu(&s->pbdev[i].mr, OBJECT(s),
> +        memory_region_init_iommu(&s->conn[i].iommu_mr, OBJECT(s),
>                                    &s390_iommu_ops, "iommu-s390", UINT64_MAX);
> -        address_space_init(&s->pbdev[i].as, &s->pbdev[i].mr, "iommu-pci");
> +        address_space_init(&s->conn[i].iommu_as, &s->conn[i].iommu_mr,
> +                           "iommu-pci");
>       }
>
>       memory_region_init_io(&s->msix_notify_mr, OBJECT(s),
> @@ -481,7 +509,7 @@ static int s390_pcihost_init(SysBusDevice *dev)
>       bus = BUS(b);
>       qbus_set_hotplug_handler(bus, DEVICE(dev), NULL);
>       phb->bus = b;
> -    QTAILQ_INIT(&s->pending_sei);
> +
>       return 0;
>   }
>
> @@ -513,29 +541,12 @@ static int s390_pcihost_setup_msix(S390PCIBusDevice *pbdev)
>       return 0;
>   }
>
> +/* this function is useless, just keep it to make the patch more
> +   readable. Without this function, the diff will mix these hotplug
> +   functions together. will be removed in future */
>   static void s390_pcihost_hot_plug(HotplugHandler *hotplug_dev,
>                                     DeviceState *dev, Error **errp)
>   {
> -    PCIDevice *pci_dev = PCI_DEVICE(dev);
> -    S390PCIBusDevice *pbdev;
> -    S390pciState *s = S390_PCI_HOST_BRIDGE(pci_device_root_bus(pci_dev)
> -                                           ->qbus.parent);
> -
> -    pbdev = &s->pbdev[PCI_SLOT(pci_dev->devfn)];
> -
> -    pbdev->fid = s390_pci_get_pfid(pci_dev);
> -    pbdev->pdev = pci_dev;
> -    pbdev->configured = true;
> -    pbdev->fh = s390_pci_get_pfh(pci_dev);
> -
> -    s390_pcihost_setup_msix(pbdev);
> -
> -    if (dev->hotplugged) {
> -        s390_pci_generate_plug_event(HP_EVENT_RESERVED_TO_STANDBY,
> -                                     pbdev->fh, pbdev->fid);
> -        s390_pci_generate_plug_event(HP_EVENT_TO_CONFIGURED,
> -                                     pbdev->fh, pbdev->fid);
> -    }
>       return;
>   }
>
> @@ -543,31 +554,30 @@ static void s390_pcihost_hot_unplug(HotplugHandler *hotplug_dev,
>                                       DeviceState *dev, Error **errp)
>   {
>       PCIDevice *pci_dev = PCI_DEVICE(dev);
> -    S390pciState *s = S390_PCI_HOST_BRIDGE(pci_device_root_bus(pci_dev)
> -                                           ->qbus.parent);
> -    S390PCIBusDevice *pbdev = &s->pbdev[PCI_SLOT(pci_dev->devfn)];
> -
> -    if (pbdev->configured) {
> -        pbdev->configured = false;
> -        s390_pci_generate_plug_event(HP_EVENT_CONFIGURED_TO_STBRES,
> -                                     pbdev->fh, pbdev->fid);
> +    S390PCIBusDevice *pbdev;
> +    HotplugHandler *hotplug_ctrl;
> +    S390PCIFacility *f = S390_PCI_FACILITY(
> +        object_resolve_path(TYPE_S390_PCI_FACILITY, NULL));
> +    S390PCIFacilityClass *k = S390_PCI_FACILITY_GET_CLASS(f);
> +    HotplugHandlerClass *hdc = HOTPLUG_HANDLER_CLASS(k);
> +
> +    /* unplug corresponding zpci device */
> +    pbdev = s390_pci_find_dev_by_pdev(pci_dev);
> +    if (pbdev) {
> +        hotplug_ctrl = pbdev->qdev.parent_bus->hotplug_handler;
> +        if (hdc->unplug_request) {
> +            hdc->unplug_request(hotplug_ctrl, &pbdev->qdev, errp);
> +        }
>       }
>
> -    s390_pci_generate_plug_event(HP_EVENT_STANDBY_TO_RESERVED,
> -                                 pbdev->fh, pbdev->fid);
> -    pbdev->fh = 0;
> -    pbdev->fid = 0;
> -    pbdev->pdev = NULL;
>       object_unparent(OBJECT(pci_dev));
>   }
>
>   static void s390_pcihost_class_init(ObjectClass *klass, void *data)
>   {
>       SysBusDeviceClass *k = SYS_BUS_DEVICE_CLASS(klass);
> -    DeviceClass *dc = DEVICE_CLASS(klass);
>       HotplugHandlerClass *hc = HOTPLUG_HANDLER_CLASS(klass);
>
> -    dc->cannot_instantiate_with_device_add_yet = true;
>       k->init = s390_pcihost_init;
>       hc->plug = s390_pcihost_hot_plug;
>       hc->unplug = s390_pcihost_hot_unplug;
> @@ -585,9 +595,156 @@ static const TypeInfo s390_pcihost_info = {
>       }
>   };
>
> +static void s390_pci_device_hot_plug(HotplugHandler *hotplug_dev,
> +                                     DeviceState *dev, Error **errp)
> +{
> +    S390PCIBusDevice *zpci = S390_PCI_DEVICE(dev);
> +
> +    zpci->configured = true;
> +
> +    if (dev->hotplugged) {
> +        s390_pci_generate_plug_event(HP_EVENT_RESERVED_TO_STANDBY,
> +                                     zpci->fh, zpci->fid);
> +        s390_pci_generate_plug_event(HP_EVENT_TO_CONFIGURED,
> +                                     zpci->fh, zpci->fid);
> +    }
> +}
> +
> +static void s390_pci_device_hot_unplug_request(HotplugHandler *hotplug_dev,
> +                                       DeviceState *dev, Error **errp)
> +{
> +    S390PCIBusDevice *zpci = S390_PCI_DEVICE(dev);
> +
> +    if (zpci->configured) {
> +        zpci->configured = false;
> +        s390_pci_generate_plug_event(HP_EVENT_CONFIGURED_TO_STBRES,
> +                                     zpci->fh, zpci->fid);
> +    }
> +
> +    s390_pci_generate_plug_event(HP_EVENT_STANDBY_TO_RESERVED,
> +                                 zpci->fh, zpci->fid);
> +
> +    zpci->is_unplugged = true;
> +}
> +
> +static const TypeInfo s390_pci_fac_bus_info = {
> +    .name = TYPE_S390_PCI_FAC_BUS,
> +    .parent = TYPE_BUS,
> +    .instance_size = sizeof(S390PCIFacBus),
> +};
> +
> +static int s390_pci_facility_init(S390PCIFacility *f)
> +{
> +    DeviceState *dev = DEVICE(f);
> +
> +    QTAILQ_INIT(&f->pending_sei);
> +    msi_supported = true;
> +    f->fbus = S390_PCI_FAC_BUS(qbus_create(TYPE_S390_PCI_FAC_BUS, dev, NULL));
> +    qbus_set_hotplug_handler(BUS(&f->fbus->qbus), DEVICE(dev), NULL);
> +
> +    return 0;
> +}
> +
> +static void s390_pci_facility_class_init(ObjectClass *klass, void *data)
> +{
> +    S390PCIFacilityClass *k = S390_PCI_FACILITY_CLASS(klass);
> +    HotplugHandlerClass *hc = HOTPLUG_HANDLER_CLASS(k);
> +
> +    k->init = s390_pci_facility_init;
> +    hc->plug = s390_pci_device_hot_plug;
> +    hc->unplug_request = s390_pci_device_hot_unplug_request;
> +}
> +
> +static const TypeInfo s390_pci_facility_info = {
> +    .name          = TYPE_S390_PCI_FACILITY,
> +    .parent        = TYPE_SYS_BUS_DEVICE,
> +    .instance_size = sizeof(S390PCIFacility),
> +    .class_init    = s390_pci_facility_class_init,
> +    .class_size    = sizeof(S390PCIFacilityClass),
> +    .interfaces = (InterfaceInfo[]) {
> +        { TYPE_HOTPLUG_HANDLER },
> +        { }
> +    }
> +};
> +
> +static void s390_pci_device_realize(DeviceState *dev, Error **errp)
> +{
> +    S390PCIBusDevice *zpci = S390_PCI_DEVICE(dev);
> +    S390PCIBusDevice *tmp;
> +    S390pciState *s;
> +    BusChild *kid;
> +    PCIDevice *pdev;
> +    int ret;
> +    S390PCIFacility *f = S390_PCI_FACILITY(
> +        object_resolve_path(TYPE_S390_PCI_FACILITY, NULL));
> +
> +    ret = pci_qdev_find_device(zpci->pci_id, &pdev);
> +    if (ret < 0) {
> +        error_setg(errp, "vfio pci device %s not found", zpci->pci_id);
> +        return;
> +    }
> +
> +    QTAILQ_FOREACH(kid, &f->fbus->qbus.children, sibling) {
> +        tmp = (S390PCIBusDevice *)kid->child;
> +        if (tmp == zpci) {
> +            continue;
> +        }
> +
> +        if (tmp->fid == zpci->fid || tmp->uid == zpci->uid ||
> +            !strcmp(tmp->pci_id, zpci->pci_id)) {
> +            error_setg(errp, "zpci needs unique fid, uid and pci_id");
> +            return;
> +        }
> +    }
> +
> +    s = S390_PCI_HOST_BRIDGE(pci_device_root_bus(pdev)->qbus.parent);
> +    s->conn[PCI_SLOT(pdev->devfn)].zpci = zpci;
> +
> +    zpci->pdev = pdev;
> +    zpci->fh = zpci->fid | FH_VIRT;
> +    s390_pcihost_setup_msix(zpci);
> +}
> +
> +static void s390_pci_device_unrealize(DeviceState *dev, Error **errp)
> +{
> +    S390PCIBusDevice *zpci = S390_PCI_DEVICE(dev);
> +
> +    zpci->fh = 0;
> +    zpci->fid = 0;
> +    zpci->pdev = NULL;
> +}
> +
> +static Property s390_pci_device_properties[] = {
> +    DEFINE_PROP_UINT32("fid", S390PCIBusDevice, fid, 0),
> +    DEFINE_PROP_UINT32("uid", S390PCIBusDevice, uid, 0),
> +    DEFINE_PROP_STRING("pci_id", S390PCIBusDevice, pci_id),
> +    DEFINE_PROP_END_OF_LIST(),
> +};
> +
> +static void s390_pci_device_class_init(ObjectClass *klass, void *data)
> +{
> +    DeviceClass *dc = DEVICE_CLASS(klass);
> +
> +    dc->desc = "s390 pci device";
> +    dc->bus_type = TYPE_S390_PCI_FAC_BUS;
> +    dc->realize = s390_pci_device_realize;
> +    dc->unrealize = s390_pci_device_unrealize;
> +    dc->props = s390_pci_device_properties;
> +}
> +
> +static const TypeInfo s390_pci_device_type_info = {
> +    .name = TYPE_S390_PCI_DEVICE,
> +    .parent = TYPE_DEVICE,
> +    .instance_size = sizeof(S390PCIBusDevice),
> +    .class_init = s390_pci_device_class_init,
> +};
> +
>   static void s390_pci_register_types(void)
>   {
>       type_register_static(&s390_pcihost_info);
> +    type_register_static(&s390_pci_facility_info);
> +    type_register_static(&s390_pci_fac_bus_info);
> +    type_register_static(&s390_pci_device_type_info);
>   }
>
>   type_init(s390_pci_register_types)
> diff --git a/hw/s390x/s390-pci-bus.h b/hw/s390x/s390-pci-bus.h
> index 464a92e..5bf3913 100644
> --- a/hw/s390x/s390-pci-bus.h
> +++ b/hw/s390x/s390-pci-bus.h
> @@ -149,6 +149,21 @@ enum ZpciIoatDtype {
>   #define ZPCI_TABLE_VALID_MASK           0x20
>   #define ZPCI_TABLE_PROT_MASK            0x200
>
> +#define TYPE_S390_PCI_FACILITY "s390-pci-facility"
> +#define TYPE_S390_PCI_FAC_BUS "s390-pci-fac-bus"
> +#define TYPE_S390_PCI_DEVICE "zpci"
> +
> +#define S390_PCI_FACILITY(obj) \
> +    OBJECT_CHECK(S390PCIFacility, (obj), TYPE_S390_PCI_FACILITY)
> +#define S390_PCI_FAC_BUS(obj) \
> +    OBJECT_CHECK(S390PCIFacBus, (obj), TYPE_S390_PCI_FAC_BUS)
> +#define S390_PCI_FACILITY_CLASS(klass) \
> +    OBJECT_CLASS_CHECK(S390PCIFacilityClass, (klass), TYPE_S390_PCI_FACILITY)
> +#define S390_PCI_DEVICE(obj) \
> +    OBJECT_CHECK(S390PCIBusDevice, (obj), TYPE_S390_PCI_DEVICE)
> +#define S390_PCI_FACILITY_GET_CLASS(obj) \
> +    OBJECT_GET_CLASS(S390PCIFacilityClass, (obj), TYPE_S390_PCI_FACILITY)
> +
>   typedef struct SeiContainer {
>       QTAILQ_ENTRY(SeiContainer) link;
>       uint32_t fid;
> @@ -214,12 +229,16 @@ typedef struct S390MsixInfo {
>   } S390MsixInfo;
>
>   typedef struct S390PCIBusDevice {
> +    DeviceState qdev;
>       PCIDevice *pdev;
>       bool configured;
> +    bool is_unplugged;
>       bool error_state;
>       bool lgstg_blocked;
>       uint32_t fh;
>       uint32_t fid;
> +    uint32_t uid;
> +    char *pci_id;
>       uint64_t g_iota;
>       uint64_t pba;
>       uint64_t pal;
> @@ -229,21 +248,42 @@ typedef struct S390PCIBusDevice {
>       uint8_t sum;
>       S390MsixInfo msix;
>       AdapterRoutes routes;
> -    AddressSpace as;
> -    MemoryRegion mr;
> +    QLIST_ENTRY(S390PCIDevice) entry;
>   } S390PCIBusDevice;
>
> +typedef struct S390PCIDeviceConn {
> +    S390PCIBusDevice *zpci;
> +    AddressSpace iommu_as;
> +    MemoryRegion iommu_mr;
> +} S390PCIDeviceConn;
> +
>   typedef struct S390pciState {
>       PCIHostState parent_obj;
> -    S390PCIBusDevice pbdev[PCI_SLOT_MAX];
> +    S390PCIDeviceConn conn[PCI_SLOT_MAX];
>       AddressSpace msix_notify_as;
>       MemoryRegion msix_notify_mr;
> -    QTAILQ_HEAD(, SeiContainer) pending_sei;
>   } S390pciState;
>
> +typedef struct S390PCIFacBus {
> +    BusState qbus;
> +} S390PCIFacBus;
> +
> +typedef struct S390PCIFacility {
> +    SysBusDevice parent_obj;
> +    S390PCIFacBus *fbus;
> +    QTAILQ_HEAD(, SeiContainer) pending_sei;
> +} S390PCIFacility;
> +
> +typedef struct S390PCIFacilityClass {
> +    DeviceClass parent_class;
> +    int (*init)(S390PCIFacility *f);
> +} S390PCIFacilityClass;
> +
>   int chsc_sei_nt2_get_event(void *res);
>   int chsc_sei_nt2_have_event(void);
>   void s390_pci_sclp_configure(int configure, SCCB *sccb);
> +void s390_pci_device_enable(S390PCIBusDevice *zpci);
> +void s390_pci_device_disable(S390PCIBusDevice *zpci);
>   S390PCIBusDevice *s390_pci_find_dev_by_idx(uint32_t idx);
>   S390PCIBusDevice *s390_pci_find_dev_by_fh(uint32_t fh);
>   S390PCIBusDevice *s390_pci_find_dev_by_fid(uint32_t fid);
> diff --git a/hw/s390x/s390-pci-inst.c b/hw/s390x/s390-pci-inst.c
> index cac3d83..fc8e8e1 100644
> --- a/hw/s390x/s390-pci-inst.c
> +++ b/hw/s390x/s390-pci-inst.c
> @@ -208,12 +208,12 @@ int clp_service_call(S390CPU *cpu, uint8_t r2)
>
>           switch (reqsetpci->oc) {
>           case CLP_SET_ENABLE_PCI_FN:
> -            pbdev->fh = pbdev->fh | 1 << ENABLE_BIT_OFFSET;
> +            s390_pci_device_enable(pbdev);
>               stl_p(&ressetpci->fh, pbdev->fh);
>               stw_p(&ressetpci->hdr.rsp, CLP_RC_OK);
>               break;
>           case CLP_SET_DISABLE_PCI_FN:
> -            pbdev->fh = pbdev->fh & ~(1 << ENABLE_BIT_OFFSET);
> +            s390_pci_device_disable(pbdev);
>               pbdev->error_state = false;
>               pbdev->lgstg_blocked = false;
>               stl_p(&ressetpci->fh, pbdev->fh);
> diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c
> index 312b8ac..7ee5202 100644
> --- a/hw/s390x/s390-virtio-ccw.c
> +++ b/hw/s390x/s390-virtio-ccw.c
> @@ -177,8 +177,8 @@ static void ccw_init(MachineState *machine)
>                         machine->initrd_filename, "s390-ccw.img", true);
>       s390_flic_init();
>
> -    dev = qdev_create(NULL, TYPE_S390_PCI_HOST_BRIDGE);
> -    object_property_add_child(qdev_get_machine(), TYPE_S390_PCI_HOST_BRIDGE,
> +    dev = qdev_create(NULL, TYPE_S390_PCI_FACILITY);
> +    object_property_add_child(qdev_get_machine(), TYPE_S390_PCI_FACILITY,
>                                 OBJECT(dev), NULL);
>       qdev_init_nofail(dev);
>

Re: [Qemu-devel] [PATCH v1 1/1] S390 pci infrastructure modeling

[Cornelia Huck]

On Tue, 21 Apr 2015 13:29:01 +0800
Hong Bo Li <lihbbj@linux.vnet.ibm.com> wrote:

> Sorry, I sent the patch three times by accident.  Please take this one
> as the formal patch.
> 
> This is a followup to the discussion started by Frank Blaschka:
> 
> Option 1: s390x/pci: Extend pci representation by new zpci device
> Option 2: s390x/pci: rework pci infrastructure modeling
> 
> This patch provides an option 3, it is similar to option 1, but did
> some changes to hot-plug & hot-unplug.
> 
> The process of hot-plug looks like:
>     a. hot-plug a vfio-pci device, we need to do nothing for this step.
>     b. hot-plug a zpci device, we do the actual zpci device hot-plug
>        action in this step, and store specific s390 information.
> 
> We have two ways for hot-unplug:
> 
> 1. hot-unplug zpci device first, then unplug vfio-pci device.
> 2. hot-unplug vfio-pci device directly, the unplug handler will unplug
>     the corresponding zpci device.
> 
> This option 3 tries to address the hot-plug/unplug issue which is the major
> concerned point in previous discussion. The idea of option 3 is: for each
> vfio-pci device, we define a zpci device, and do all s390 specific action
> on that device.
> 
> On 4/15/2015 16:04, Hong Bo Li wrote:
> > This patch contains the actual interesting changes.
> > usage example:
> > -device s390-pcihost
> > -device vfio-pci,host=0000:00:00.0,id=vpci1
> > -device zpci,fid=2,uid=5,pci_id=vpci1,id=zpci1
> >
> > The first line will create a s390 pci host bridge
> > and init the root bus. The second line will create
> > a standard vfio pci device, and attach it to the
> > root bus. These are similiar to the standard process
> > to define a pci device on other platform.
> >
> > The third line will create a s390 pci device to
> > store s390 specific information, and references
> > the corresponding vfio pci device via device id.
> > We create a s390 pci facility bus to hold all the
> > zpci devices.
> >
> > Signed-off-by: Hong Bo Li <lihbbj@linux.vnet.ibm.com>
> > ---
> >   hw/s390x/s390-pci-bus.c    | 317 +++++++++++++++++++++++++++++++++------------
> >   hw/s390x/s390-pci-bus.h    |  48 ++++++-
> >   hw/s390x/s390-pci-inst.c   |   4 +-
> >   hw/s390x/s390-virtio-ccw.c |   4 +-
> >   4 files changed, 285 insertions(+), 88 deletions(-)

This looks fine from my s390 perspective, but I'd love to see some
feedback from PCI experts on this. Does modelling our s390 oddities in
this way sit fine with the general PCI infrastructure?

> >
> > diff --git a/hw/s390x/s390-pci-bus.c b/hw/s390x/s390-pci-bus.c
> > index 3c086f6..c81093e 100644
> > --- a/hw/s390x/s390-pci-bus.c
> > +++ b/hw/s390x/s390-pci-bus.c
> > @@ -32,8 +32,8 @@ int chsc_sei_nt2_get_event(void *res)
> >       PciCcdfErr *eccdf;
> >       int rc = 1;
> >       SeiContainer *sei_cont;
> > -    S390pciState *s = S390_PCI_HOST_BRIDGE(
> > -        object_resolve_path(TYPE_S390_PCI_HOST_BRIDGE, NULL));
> > +    S390PCIFacility *s = S390_PCI_FACILITY(
> > +        object_resolve_path(TYPE_S390_PCI_FACILITY, NULL));
> >
> >       if (!s) {
> >           return rc;
> > @@ -72,8 +72,8 @@ int chsc_sei_nt2_get_event(void *res)
> >
> >   int chsc_sei_nt2_have_event(void)
> >   {
> > -    S390pciState *s = S390_PCI_HOST_BRIDGE(
> > -        object_resolve_path(TYPE_S390_PCI_HOST_BRIDGE, NULL));
> > +    S390PCIFacility *s = S390_PCI_FACILITY(
> > +        object_resolve_path(TYPE_S390_PCI_FACILITY, NULL));
> >
> >       if (!s) {
> >           return 0;
> > @@ -82,20 +82,32 @@ int chsc_sei_nt2_have_event(void)
> >       return !QTAILQ_EMPTY(&s->pending_sei);
> >   }
> >
> > +void s390_pci_device_enable(S390PCIBusDevice *zpci)
> > +{
> > +    zpci->fh = zpci->fh | 1 << ENABLE_BIT_OFFSET;
> > +}
> > +
> > +void s390_pci_device_disable(S390PCIBusDevice *zpci)
> > +{
> > +    zpci->fh = zpci->fh & ~(1 << ENABLE_BIT_OFFSET);
> > +    if (zpci->is_unplugged)
> > +        object_unparent(OBJECT(zpci));
> > +}
> > +
> >   S390PCIBusDevice *s390_pci_find_dev_by_fid(uint32_t fid)
> >   {
> >       S390PCIBusDevice *pbdev;
> > -    int i;
> > -    S390pciState *s = S390_PCI_HOST_BRIDGE(
> > -        object_resolve_path(TYPE_S390_PCI_HOST_BRIDGE, NULL));
> > +    BusChild *kid;
> > +    S390PCIFacility *s = S390_PCI_FACILITY(
> > +        object_resolve_path(TYPE_S390_PCI_FACILITY, NULL));
> >
> >       if (!s) {
> >           return NULL;
> >       }
> >
> > -    for (i = 0; i < PCI_SLOT_MAX; i++) {
> > -        pbdev = &s->pbdev[i];
> > -        if ((pbdev->fh != 0) && (pbdev->fid == fid)) {
> > +    QTAILQ_FOREACH(kid, &s->fbus->qbus.children, sibling) {
> > +        pbdev = (S390PCIBusDevice *)kid->child;
> > +        if (pbdev->fid == fid) {
> >               return pbdev;
> >           }
> >       }
> > @@ -126,39 +138,24 @@ void s390_pci_sclp_configure(int configure, SCCB *sccb)
> >       return;
> >   }
> >
> > -static uint32_t s390_pci_get_pfid(PCIDevice *pdev)
> > -{
> > -    return PCI_SLOT(pdev->devfn);
> > -}
> > -
> > -static uint32_t s390_pci_get_pfh(PCIDevice *pdev)
> > -{
> > -    return PCI_SLOT(pdev->devfn) | FH_VIRT;
> > -}
> > -
> >   S390PCIBusDevice *s390_pci_find_dev_by_idx(uint32_t idx)
> >   {
> >       S390PCIBusDevice *pbdev;
> > -    int i;
> > -    int j = 0;
> > -    S390pciState *s = S390_PCI_HOST_BRIDGE(
> > -        object_resolve_path(TYPE_S390_PCI_HOST_BRIDGE, NULL));
> > +    BusChild *kid;
> > +    int i = 0;
> > +    S390PCIFacility *s = S390_PCI_FACILITY(
> > +        object_resolve_path(TYPE_S390_PCI_FACILITY, NULL));
> >
> >       if (!s) {
> >           return NULL;
> >       }
> >
> > -    for (i = 0; i < PCI_SLOT_MAX; i++) {
> > -        pbdev = &s->pbdev[i];
> > -
> > -        if (pbdev->fh == 0) {
> > -            continue;
> > -        }
> > -
> > -        if (j == idx) {
> > +    QTAILQ_FOREACH(kid, &s->fbus->qbus.children, sibling) {
> > +        pbdev = (S390PCIBusDevice *)kid->child;
> > +        if (i == idx) {
> >               return pbdev;
> >           }
> > -        j++;
> > +        i++;
> >       }
> >
> >       return NULL;
> > @@ -167,16 +164,17 @@ S390PCIBusDevice *s390_pci_find_dev_by_idx(uint32_t idx)
> >   S390PCIBusDevice *s390_pci_find_dev_by_fh(uint32_t fh)
> >   {
> >       S390PCIBusDevice *pbdev;
> > -    int i;
> > -    S390pciState *s = S390_PCI_HOST_BRIDGE(
> > -        object_resolve_path(TYPE_S390_PCI_HOST_BRIDGE, NULL));
> > +    BusChild *kid;
> > +    S390PCIFacility *s = S390_PCI_FACILITY(
> > +        object_resolve_path(TYPE_S390_PCI_FACILITY, NULL));
> > +
> >
> >       if (!s || !fh) {
> >           return NULL;
> >       }
> >
> > -    for (i = 0; i < PCI_SLOT_MAX; i++) {
> > -        pbdev = &s->pbdev[i];
> > +    QTAILQ_FOREACH(kid, &s->fbus->qbus.children, sibling) {
> > +        pbdev = (S390PCIBusDevice *)kid->child;
> >           if (pbdev->fh == fh) {
> >               return pbdev;
> >           }
> > @@ -185,12 +183,33 @@ S390PCIBusDevice *s390_pci_find_dev_by_fh(uint32_t fh)
> >       return NULL;
> >   }
> >
> > +static S390PCIBusDevice *s390_pci_find_dev_by_pdev(PCIDevice *pdev)
> > +{
> > +    S390PCIBusDevice *pbdev;
> > +    BusChild *kid;
> > +    S390PCIFacility *s = S390_PCI_FACILITY(
> > +        object_resolve_path(TYPE_S390_PCI_FACILITY, NULL));
> > +
> > +    if (!s || !pdev) {
> > +         return NULL;
> > +     }
> > +
> > +    QTAILQ_FOREACH(kid, &s->fbus->qbus.children, sibling) {
> > +        pbdev = (S390PCIBusDevice *)kid->child;
> > +        if (pbdev->pdev == pdev) {
> > +            return pbdev;
> > +        }
> > +    }
> > +
> > +    return NULL;
> > +}
> > +
> >   static void s390_pci_generate_event(uint8_t cc, uint16_t pec, uint32_t fh,
> >                                       uint32_t fid, uint64_t faddr, uint32_t e)
> >   {
> >       SeiContainer *sei_cont;
> > -    S390pciState *s = S390_PCI_HOST_BRIDGE(
> > -        object_resolve_path(TYPE_S390_PCI_HOST_BRIDGE, NULL));
> > +    S390PCIFacility *s = S390_PCI_FACILITY(
> > +        object_resolve_path(TYPE_S390_PCI_FACILITY, NULL));
> >
> >       if (!s) {
> >           return;
> > @@ -305,7 +324,9 @@ static IOMMUTLBEntry s390_translate_iommu(MemoryRegion *iommu, hwaddr addr,
> >   {
> >       uint64_t pte;
> >       uint32_t flags;
> > -    S390PCIBusDevice *pbdev = container_of(iommu, S390PCIBusDevice, mr);
> > +    S390PCIDeviceConn *conn = container_of(iommu, S390PCIDeviceConn,
> > +                                           iommu_mr);
> > +    S390PCIBusDevice *pbdev = conn->zpci;
> >       S390pciState *s = S390_PCI_HOST_BRIDGE(pci_device_root_bus(pbdev->pdev)
> >                                              ->qbus.parent);
> >       IOMMUTLBEntry ret = {
> > @@ -316,8 +337,14 @@ static IOMMUTLBEntry s390_translate_iommu(MemoryRegion *iommu, hwaddr addr,
> >           .perm = IOMMU_NONE,
> >       };
> >
> > +    if (!pbdev) {
> > +        return ret;
> > +    }
> > +
> >       DPRINTF("iommu trans addr 0x%" PRIx64 "\n", addr);
> >
> > +    s = S390_PCI_HOST_BRIDGE(pci_device_root_bus(pbdev->pdev)->qbus.parent);
> > +
> >       /* s390 does not have an APIC mapped to main storage so we use
> >        * a separate AddressSpace only for msix notifications
> >        */
> > @@ -379,7 +406,7 @@ static AddressSpace *s390_pci_dma_iommu(PCIBus *bus, void *opaque, int devfn)
> >   {
> >       S390pciState *s = opaque;
> >
> > -    return &s->pbdev[PCI_SLOT(devfn)].as;
> > +    return &s->conn[PCI_SLOT(devfn)].iommu_as;
> >   }
> >
> >   static uint8_t set_ind_atomic(uint64_t ind_loc, uint8_t to_be_set)
> > @@ -452,9 +479,10 @@ static void s390_pcihost_init_as(S390pciState *s)
> >       int i;
> >
> >       for (i = 0; i < PCI_SLOT_MAX; i++) {
> > -        memory_region_init_iommu(&s->pbdev[i].mr, OBJECT(s),
> > +        memory_region_init_iommu(&s->conn[i].iommu_mr, OBJECT(s),
> >                                    &s390_iommu_ops, "iommu-s390", UINT64_MAX);
> > -        address_space_init(&s->pbdev[i].as, &s->pbdev[i].mr, "iommu-pci");
> > +        address_space_init(&s->conn[i].iommu_as, &s->conn[i].iommu_mr,
> > +                           "iommu-pci");
> >       }
> >
> >       memory_region_init_io(&s->msix_notify_mr, OBJECT(s),
> > @@ -481,7 +509,7 @@ static int s390_pcihost_init(SysBusDevice *dev)
> >       bus = BUS(b);
> >       qbus_set_hotplug_handler(bus, DEVICE(dev), NULL);
> >       phb->bus = b;
> > -    QTAILQ_INIT(&s->pending_sei);
> > +
> >       return 0;
> >   }
> >
> > @@ -513,29 +541,12 @@ static int s390_pcihost_setup_msix(S390PCIBusDevice *pbdev)
> >       return 0;
> >   }
> >
> > +/* this function is useless, just keep it to make the patch more
> > +   readable. Without this function, the diff will mix these hotplug
> > +   functions together. will be removed in future */
> >   static void s390_pcihost_hot_plug(HotplugHandler *hotplug_dev,
> >                                     DeviceState *dev, Error **errp)
> >   {
> > -    PCIDevice *pci_dev = PCI_DEVICE(dev);
> > -    S390PCIBusDevice *pbdev;
> > -    S390pciState *s = S390_PCI_HOST_BRIDGE(pci_device_root_bus(pci_dev)
> > -                                           ->qbus.parent);
> > -
> > -    pbdev = &s->pbdev[PCI_SLOT(pci_dev->devfn)];
> > -
> > -    pbdev->fid = s390_pci_get_pfid(pci_dev);
> > -    pbdev->pdev = pci_dev;
> > -    pbdev->configured = true;
> > -    pbdev->fh = s390_pci_get_pfh(pci_dev);
> > -
> > -    s390_pcihost_setup_msix(pbdev);
> > -
> > -    if (dev->hotplugged) {
> > -        s390_pci_generate_plug_event(HP_EVENT_RESERVED_TO_STANDBY,
> > -                                     pbdev->fh, pbdev->fid);
> > -        s390_pci_generate_plug_event(HP_EVENT_TO_CONFIGURED,
> > -                                     pbdev->fh, pbdev->fid);
> > -    }
> >       return;
> >   }
> >
> > @@ -543,31 +554,30 @@ static void s390_pcihost_hot_unplug(HotplugHandler *hotplug_dev,
> >                                       DeviceState *dev, Error **errp)
> >   {
> >       PCIDevice *pci_dev = PCI_DEVICE(dev);
> > -    S390pciState *s = S390_PCI_HOST_BRIDGE(pci_device_root_bus(pci_dev)
> > -                                           ->qbus.parent);
> > -    S390PCIBusDevice *pbdev = &s->pbdev[PCI_SLOT(pci_dev->devfn)];
> > -
> > -    if (pbdev->configured) {
> > -        pbdev->configured = false;
> > -        s390_pci_generate_plug_event(HP_EVENT_CONFIGURED_TO_STBRES,
> > -                                     pbdev->fh, pbdev->fid);
> > +    S390PCIBusDevice *pbdev;
> > +    HotplugHandler *hotplug_ctrl;
> > +    S390PCIFacility *f = S390_PCI_FACILITY(
> > +        object_resolve_path(TYPE_S390_PCI_FACILITY, NULL));
> > +    S390PCIFacilityClass *k = S390_PCI_FACILITY_GET_CLASS(f);
> > +    HotplugHandlerClass *hdc = HOTPLUG_HANDLER_CLASS(k);
> > +
> > +    /* unplug corresponding zpci device */
> > +    pbdev = s390_pci_find_dev_by_pdev(pci_dev);
> > +    if (pbdev) {
> > +        hotplug_ctrl = pbdev->qdev.parent_bus->hotplug_handler;
> > +        if (hdc->unplug_request) {
> > +            hdc->unplug_request(hotplug_ctrl, &pbdev->qdev, errp);
> > +        }
> >       }
> >
> > -    s390_pci_generate_plug_event(HP_EVENT_STANDBY_TO_RESERVED,
> > -                                 pbdev->fh, pbdev->fid);
> > -    pbdev->fh = 0;
> > -    pbdev->fid = 0;
> > -    pbdev->pdev = NULL;
> >       object_unparent(OBJECT(pci_dev));
> >   }
> >
> >   static void s390_pcihost_class_init(ObjectClass *klass, void *data)
> >   {
> >       SysBusDeviceClass *k = SYS_BUS_DEVICE_CLASS(klass);
> > -    DeviceClass *dc = DEVICE_CLASS(klass);
> >       HotplugHandlerClass *hc = HOTPLUG_HANDLER_CLASS(klass);
> >
> > -    dc->cannot_instantiate_with_device_add_yet = true;
> >       k->init = s390_pcihost_init;
> >       hc->plug = s390_pcihost_hot_plug;
> >       hc->unplug = s390_pcihost_hot_unplug;
> > @@ -585,9 +595,156 @@ static const TypeInfo s390_pcihost_info = {
> >       }
> >   };
> >
> > +static void s390_pci_device_hot_plug(HotplugHandler *hotplug_dev,
> > +                                     DeviceState *dev, Error **errp)
> > +{
> > +    S390PCIBusDevice *zpci = S390_PCI_DEVICE(dev);
> > +
> > +    zpci->configured = true;
> > +
> > +    if (dev->hotplugged) {
> > +        s390_pci_generate_plug_event(HP_EVENT_RESERVED_TO_STANDBY,
> > +                                     zpci->fh, zpci->fid);
> > +        s390_pci_generate_plug_event(HP_EVENT_TO_CONFIGURED,
> > +                                     zpci->fh, zpci->fid);
> > +    }
> > +}
> > +
> > +static void s390_pci_device_hot_unplug_request(HotplugHandler *hotplug_dev,
> > +                                       DeviceState *dev, Error **errp)
> > +{
> > +    S390PCIBusDevice *zpci = S390_PCI_DEVICE(dev);
> > +
> > +    if (zpci->configured) {
> > +        zpci->configured = false;
> > +        s390_pci_generate_plug_event(HP_EVENT_CONFIGURED_TO_STBRES,
> > +                                     zpci->fh, zpci->fid);
> > +    }
> > +
> > +    s390_pci_generate_plug_event(HP_EVENT_STANDBY_TO_RESERVED,
> > +                                 zpci->fh, zpci->fid);
> > +
> > +    zpci->is_unplugged = true;
> > +}
> > +
> > +static const TypeInfo s390_pci_fac_bus_info = {
> > +    .name = TYPE_S390_PCI_FAC_BUS,
> > +    .parent = TYPE_BUS,
> > +    .instance_size = sizeof(S390PCIFacBus),
> > +};
> > +
> > +static int s390_pci_facility_init(S390PCIFacility *f)
> > +{
> > +    DeviceState *dev = DEVICE(f);
> > +
> > +    QTAILQ_INIT(&f->pending_sei);
> > +    msi_supported = true;
> > +    f->fbus = S390_PCI_FAC_BUS(qbus_create(TYPE_S390_PCI_FAC_BUS, dev, NULL));
> > +    qbus_set_hotplug_handler(BUS(&f->fbus->qbus), DEVICE(dev), NULL);
> > +
> > +    return 0;
> > +}
> > +
> > +static void s390_pci_facility_class_init(ObjectClass *klass, void *data)
> > +{
> > +    S390PCIFacilityClass *k = S390_PCI_FACILITY_CLASS(klass);
> > +    HotplugHandlerClass *hc = HOTPLUG_HANDLER_CLASS(k);
> > +
> > +    k->init = s390_pci_facility_init;
> > +    hc->plug = s390_pci_device_hot_plug;
> > +    hc->unplug_request = s390_pci_device_hot_unplug_request;
> > +}
> > +
> > +static const TypeInfo s390_pci_facility_info = {
> > +    .name          = TYPE_S390_PCI_FACILITY,
> > +    .parent        = TYPE_SYS_BUS_DEVICE,
> > +    .instance_size = sizeof(S390PCIFacility),
> > +    .class_init    = s390_pci_facility_class_init,
> > +    .class_size    = sizeof(S390PCIFacilityClass),
> > +    .interfaces = (InterfaceInfo[]) {
> > +        { TYPE_HOTPLUG_HANDLER },
> > +        { }
> > +    }
> > +};
> > +
> > +static void s390_pci_device_realize(DeviceState *dev, Error **errp)
> > +{
> > +    S390PCIBusDevice *zpci = S390_PCI_DEVICE(dev);
> > +    S390PCIBusDevice *tmp;
> > +    S390pciState *s;
> > +    BusChild *kid;
> > +    PCIDevice *pdev;
> > +    int ret;
> > +    S390PCIFacility *f = S390_PCI_FACILITY(
> > +        object_resolve_path(TYPE_S390_PCI_FACILITY, NULL));
> > +
> > +    ret = pci_qdev_find_device(zpci->pci_id, &pdev);
> > +    if (ret < 0) {
> > +        error_setg(errp, "vfio pci device %s not found", zpci->pci_id);
> > +        return;
> > +    }
> > +
> > +    QTAILQ_FOREACH(kid, &f->fbus->qbus.children, sibling) {
> > +        tmp = (S390PCIBusDevice *)kid->child;
> > +        if (tmp == zpci) {
> > +            continue;
> > +        }
> > +
> > +        if (tmp->fid == zpci->fid || tmp->uid == zpci->uid ||
> > +            !strcmp(tmp->pci_id, zpci->pci_id)) {
> > +            error_setg(errp, "zpci needs unique fid, uid and pci_id");
> > +            return;
> > +        }
> > +    }
> > +
> > +    s = S390_PCI_HOST_BRIDGE(pci_device_root_bus(pdev)->qbus.parent);
> > +    s->conn[PCI_SLOT(pdev->devfn)].zpci = zpci;
> > +
> > +    zpci->pdev = pdev;
> > +    zpci->fh = zpci->fid | FH_VIRT;
> > +    s390_pcihost_setup_msix(zpci);
> > +}
> > +
> > +static void s390_pci_device_unrealize(DeviceState *dev, Error **errp)
> > +{
> > +    S390PCIBusDevice *zpci = S390_PCI_DEVICE(dev);
> > +
> > +    zpci->fh = 0;
> > +    zpci->fid = 0;
> > +    zpci->pdev = NULL;
> > +}
> > +
> > +static Property s390_pci_device_properties[] = {
> > +    DEFINE_PROP_UINT32("fid", S390PCIBusDevice, fid, 0),
> > +    DEFINE_PROP_UINT32("uid", S390PCIBusDevice, uid, 0),
> > +    DEFINE_PROP_STRING("pci_id", S390PCIBusDevice, pci_id),
> > +    DEFINE_PROP_END_OF_LIST(),
> > +};
> > +
> > +static void s390_pci_device_class_init(ObjectClass *klass, void *data)
> > +{
> > +    DeviceClass *dc = DEVICE_CLASS(klass);
> > +
> > +    dc->desc = "s390 pci device";
> > +    dc->bus_type = TYPE_S390_PCI_FAC_BUS;
> > +    dc->realize = s390_pci_device_realize;
> > +    dc->unrealize = s390_pci_device_unrealize;
> > +    dc->props = s390_pci_device_properties;
> > +}
> > +
> > +static const TypeInfo s390_pci_device_type_info = {
> > +    .name = TYPE_S390_PCI_DEVICE,
> > +    .parent = TYPE_DEVICE,
> > +    .instance_size = sizeof(S390PCIBusDevice),
> > +    .class_init = s390_pci_device_class_init,
> > +};
> > +
> >   static void s390_pci_register_types(void)
> >   {
> >       type_register_static(&s390_pcihost_info);
> > +    type_register_static(&s390_pci_facility_info);
> > +    type_register_static(&s390_pci_fac_bus_info);
> > +    type_register_static(&s390_pci_device_type_info);
> >   }
> >
> >   type_init(s390_pci_register_types)
> > diff --git a/hw/s390x/s390-pci-bus.h b/hw/s390x/s390-pci-bus.h
> > index 464a92e..5bf3913 100644
> > --- a/hw/s390x/s390-pci-bus.h
> > +++ b/hw/s390x/s390-pci-bus.h
> > @@ -149,6 +149,21 @@ enum ZpciIoatDtype {
> >   #define ZPCI_TABLE_VALID_MASK           0x20
> >   #define ZPCI_TABLE_PROT_MASK            0x200
> >
> > +#define TYPE_S390_PCI_FACILITY "s390-pci-facility"
> > +#define TYPE_S390_PCI_FAC_BUS "s390-pci-fac-bus"
> > +#define TYPE_S390_PCI_DEVICE "zpci"
> > +
> > +#define S390_PCI_FACILITY(obj) \
> > +    OBJECT_CHECK(S390PCIFacility, (obj), TYPE_S390_PCI_FACILITY)
> > +#define S390_PCI_FAC_BUS(obj) \
> > +    OBJECT_CHECK(S390PCIFacBus, (obj), TYPE_S390_PCI_FAC_BUS)
> > +#define S390_PCI_FACILITY_CLASS(klass) \
> > +    OBJECT_CLASS_CHECK(S390PCIFacilityClass, (klass), TYPE_S390_PCI_FACILITY)
> > +#define S390_PCI_DEVICE(obj) \
> > +    OBJECT_CHECK(S390PCIBusDevice, (obj), TYPE_S390_PCI_DEVICE)
> > +#define S390_PCI_FACILITY_GET_CLASS(obj) \
> > +    OBJECT_GET_CLASS(S390PCIFacilityClass, (obj), TYPE_S390_PCI_FACILITY)
> > +
> >   typedef struct SeiContainer {
> >       QTAILQ_ENTRY(SeiContainer) link;
> >       uint32_t fid;
> > @@ -214,12 +229,16 @@ typedef struct S390MsixInfo {
> >   } S390MsixInfo;
> >
> >   typedef struct S390PCIBusDevice {
> > +    DeviceState qdev;
> >       PCIDevice *pdev;
> >       bool configured;
> > +    bool is_unplugged;
> >       bool error_state;
> >       bool lgstg_blocked;
> >       uint32_t fh;
> >       uint32_t fid;
> > +    uint32_t uid;
> > +    char *pci_id;
> >       uint64_t g_iota;
> >       uint64_t pba;
> >       uint64_t pal;
> > @@ -229,21 +248,42 @@ typedef struct S390PCIBusDevice {
> >       uint8_t sum;
> >       S390MsixInfo msix;
> >       AdapterRoutes routes;
> > -    AddressSpace as;
> > -    MemoryRegion mr;
> > +    QLIST_ENTRY(S390PCIDevice) entry;
> >   } S390PCIBusDevice;
> >
> > +typedef struct S390PCIDeviceConn {
> > +    S390PCIBusDevice *zpci;
> > +    AddressSpace iommu_as;
> > +    MemoryRegion iommu_mr;
> > +} S390PCIDeviceConn;
> > +
> >   typedef struct S390pciState {
> >       PCIHostState parent_obj;
> > -    S390PCIBusDevice pbdev[PCI_SLOT_MAX];
> > +    S390PCIDeviceConn conn[PCI_SLOT_MAX];
> >       AddressSpace msix_notify_as;
> >       MemoryRegion msix_notify_mr;
> > -    QTAILQ_HEAD(, SeiContainer) pending_sei;
> >   } S390pciState;
> >
> > +typedef struct S390PCIFacBus {
> > +    BusState qbus;
> > +} S390PCIFacBus;
> > +
> > +typedef struct S390PCIFacility {
> > +    SysBusDevice parent_obj;
> > +    S390PCIFacBus *fbus;
> > +    QTAILQ_HEAD(, SeiContainer) pending_sei;
> > +} S390PCIFacility;
> > +
> > +typedef struct S390PCIFacilityClass {
> > +    DeviceClass parent_class;
> > +    int (*init)(S390PCIFacility *f);
> > +} S390PCIFacilityClass;
> > +
> >   int chsc_sei_nt2_get_event(void *res);
> >   int chsc_sei_nt2_have_event(void);
> >   void s390_pci_sclp_configure(int configure, SCCB *sccb);
> > +void s390_pci_device_enable(S390PCIBusDevice *zpci);
> > +void s390_pci_device_disable(S390PCIBusDevice *zpci);
> >   S390PCIBusDevice *s390_pci_find_dev_by_idx(uint32_t idx);
> >   S390PCIBusDevice *s390_pci_find_dev_by_fh(uint32_t fh);
> >   S390PCIBusDevice *s390_pci_find_dev_by_fid(uint32_t fid);
> > diff --git a/hw/s390x/s390-pci-inst.c b/hw/s390x/s390-pci-inst.c
> > index cac3d83..fc8e8e1 100644
> > --- a/hw/s390x/s390-pci-inst.c
> > +++ b/hw/s390x/s390-pci-inst.c
> > @@ -208,12 +208,12 @@ int clp_service_call(S390CPU *cpu, uint8_t r2)
> >
> >           switch (reqsetpci->oc) {
> >           case CLP_SET_ENABLE_PCI_FN:
> > -            pbdev->fh = pbdev->fh | 1 << ENABLE_BIT_OFFSET;
> > +            s390_pci_device_enable(pbdev);
> >               stl_p(&ressetpci->fh, pbdev->fh);
> >               stw_p(&ressetpci->hdr.rsp, CLP_RC_OK);
> >               break;
> >           case CLP_SET_DISABLE_PCI_FN:
> > -            pbdev->fh = pbdev->fh & ~(1 << ENABLE_BIT_OFFSET);
> > +            s390_pci_device_disable(pbdev);
> >               pbdev->error_state = false;
> >               pbdev->lgstg_blocked = false;
> >               stl_p(&ressetpci->fh, pbdev->fh);
> > diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c
> > index 312b8ac..7ee5202 100644
> > --- a/hw/s390x/s390-virtio-ccw.c
> > +++ b/hw/s390x/s390-virtio-ccw.c
> > @@ -177,8 +177,8 @@ static void ccw_init(MachineState *machine)
> >                         machine->initrd_filename, "s390-ccw.img", true);
> >       s390_flic_init();
> >
> > -    dev = qdev_create(NULL, TYPE_S390_PCI_HOST_BRIDGE);
> > -    object_property_add_child(qdev_get_machine(), TYPE_S390_PCI_HOST_BRIDGE,
> > +    dev = qdev_create(NULL, TYPE_S390_PCI_FACILITY);
> > +    object_property_add_child(qdev_get_machine(), TYPE_S390_PCI_FACILITY,
> >                                 OBJECT(dev), NULL);
> >       qdev_init_nofail(dev);
> >
> 
> 

Re: [Qemu-devel] [PATCH v1 1/1] S390 pci infrastructure modeling

[Hong Bo Li]

On 5/12/2015 17:07, Cornelia Huck wrote:
> On Tue, 21 Apr 2015 13:29:01 +0800
> Hong Bo Li <lihbbj@linux.vnet.ibm.com> wrote:
>
>> Sorry, I sent the patch three times by accident.  Please take this one
>> as the formal patch.
>>
>> This is a followup to the discussion started by Frank Blaschka:
>>
>> Option 1: s390x/pci: Extend pci representation by new zpci device
>> Option 2: s390x/pci: rework pci infrastructure modeling
>>
>> This patch provides an option 3, it is similar to option 1, but did
>> some changes to hot-plug & hot-unplug.
>>
>> The process of hot-plug looks like:
>>      a. hot-plug a vfio-pci device, we need to do nothing for this step.
>>      b. hot-plug a zpci device, we do the actual zpci device hot-plug
>>         action in this step, and store specific s390 information.
>>
>> We have two ways for hot-unplug:
>>
>> 1. hot-unplug zpci device first, then unplug vfio-pci device.
>> 2. hot-unplug vfio-pci device directly, the unplug handler will unplug
>>      the corresponding zpci device.
>>
>> This option 3 tries to address the hot-plug/unplug issue which is the major
>> concerned point in previous discussion. The idea of option 3 is: for each
>> vfio-pci device, we define a zpci device, and do all s390 specific action
>> on that device.
>>
>> On 4/15/2015 16:04, Hong Bo Li wrote:
>>> This patch contains the actual interesting changes.
>>> usage example:
>>> -device s390-pcihost
>>> -device vfio-pci,host=0000:00:00.0,id=vpci1
>>> -device zpci,fid=2,uid=5,pci_id=vpci1,id=zpci1
>>>
>>> The first line will create a s390 pci host bridge
>>> and init the root bus. The second line will create
>>> a standard vfio pci device, and attach it to the
>>> root bus. These are similiar to the standard process
>>> to define a pci device on other platform.
>>>
>>> The third line will create a s390 pci device to
>>> store s390 specific information, and references
>>> the corresponding vfio pci device via device id.
>>> We create a s390 pci facility bus to hold all the
>>> zpci devices.
>>>
>>> Signed-off-by: Hong Bo Li <lihbbj@linux.vnet.ibm.com>
>>> ---
>>>    hw/s390x/s390-pci-bus.c    | 317 +++++++++++++++++++++++++++++++++------------
>>>    hw/s390x/s390-pci-bus.h    |  48 ++++++-
>>>    hw/s390x/s390-pci-inst.c   |   4 +-
>>>    hw/s390x/s390-virtio-ccw.c |   4 +-
>>>    4 files changed, 285 insertions(+), 88 deletions(-)
> This looks fine from my s390 perspective, but I'd love to see some
> feedback from PCI experts on this. Does modelling our s390 oddities in
> this way sit fine with the general PCI infrastructure?

Hello Alex & Michael,

Any comment from your sides? Appreciate to get your opinion.


>>> diff --git a/hw/s390x/s390-pci-bus.c b/hw/s390x/s390-pci-bus.c
>>> index 3c086f6..c81093e 100644
>>> --- a/hw/s390x/s390-pci-bus.c
>>> +++ b/hw/s390x/s390-pci-bus.c
>>> @@ -32,8 +32,8 @@ int chsc_sei_nt2_get_event(void *res)
>>>        PciCcdfErr *eccdf;
>>>        int rc = 1;
>>>        SeiContainer *sei_cont;
>>> -    S390pciState *s = S390_PCI_HOST_BRIDGE(
>>> -        object_resolve_path(TYPE_S390_PCI_HOST_BRIDGE, NULL));
>>> +    S390PCIFacility *s = S390_PCI_FACILITY(
>>> +        object_resolve_path(TYPE_S390_PCI_FACILITY, NULL));
>>>
>>>        if (!s) {
>>>            return rc;
>>> @@ -72,8 +72,8 @@ int chsc_sei_nt2_get_event(void *res)
>>>
>>>    int chsc_sei_nt2_have_event(void)
>>>    {
>>> -    S390pciState *s = S390_PCI_HOST_BRIDGE(
>>> -        object_resolve_path(TYPE_S390_PCI_HOST_BRIDGE, NULL));
>>> +    S390PCIFacility *s = S390_PCI_FACILITY(
>>> +        object_resolve_path(TYPE_S390_PCI_FACILITY, NULL));
>>>
>>>        if (!s) {
>>>            return 0;
>>> @@ -82,20 +82,32 @@ int chsc_sei_nt2_have_event(void)
>>>        return !QTAILQ_EMPTY(&s->pending_sei);
>>>    }
>>>
>>> +void s390_pci_device_enable(S390PCIBusDevice *zpci)
>>> +{
>>> +    zpci->fh = zpci->fh | 1 << ENABLE_BIT_OFFSET;
>>> +}
>>> +
>>> +void s390_pci_device_disable(S390PCIBusDevice *zpci)
>>> +{
>>> +    zpci->fh = zpci->fh & ~(1 << ENABLE_BIT_OFFSET);
>>> +    if (zpci->is_unplugged)
>>> +        object_unparent(OBJECT(zpci));
>>> +}
>>> +
>>>    S390PCIBusDevice *s390_pci_find_dev_by_fid(uint32_t fid)
>>>    {
>>>        S390PCIBusDevice *pbdev;
>>> -    int i;
>>> -    S390pciState *s = S390_PCI_HOST_BRIDGE(
>>> -        object_resolve_path(TYPE_S390_PCI_HOST_BRIDGE, NULL));
>>> +    BusChild *kid;
>>> +    S390PCIFacility *s = S390_PCI_FACILITY(
>>> +        object_resolve_path(TYPE_S390_PCI_FACILITY, NULL));
>>>
>>>        if (!s) {
>>>            return NULL;
>>>        }
>>>
>>> -    for (i = 0; i < PCI_SLOT_MAX; i++) {
>>> -        pbdev = &s->pbdev[i];
>>> -        if ((pbdev->fh != 0) && (pbdev->fid == fid)) {
>>> +    QTAILQ_FOREACH(kid, &s->fbus->qbus.children, sibling) {
>>> +        pbdev = (S390PCIBusDevice *)kid->child;
>>> +        if (pbdev->fid == fid) {
>>>                return pbdev;
>>>            }
>>>        }
>>> @@ -126,39 +138,24 @@ void s390_pci_sclp_configure(int configure, SCCB *sccb)
>>>        return;
>>>    }
>>>
>>> -static uint32_t s390_pci_get_pfid(PCIDevice *pdev)
>>> -{
>>> -    return PCI_SLOT(pdev->devfn);
>>> -}
>>> -
>>> -static uint32_t s390_pci_get_pfh(PCIDevice *pdev)
>>> -{
>>> -    return PCI_SLOT(pdev->devfn) | FH_VIRT;
>>> -}
>>> -
>>>    S390PCIBusDevice *s390_pci_find_dev_by_idx(uint32_t idx)
>>>    {
>>>        S390PCIBusDevice *pbdev;
>>> -    int i;
>>> -    int j = 0;
>>> -    S390pciState *s = S390_PCI_HOST_BRIDGE(
>>> -        object_resolve_path(TYPE_S390_PCI_HOST_BRIDGE, NULL));
>>> +    BusChild *kid;
>>> +    int i = 0;
>>> +    S390PCIFacility *s = S390_PCI_FACILITY(
>>> +        object_resolve_path(TYPE_S390_PCI_FACILITY, NULL));
>>>
>>>        if (!s) {
>>>            return NULL;
>>>        }
>>>
>>> -    for (i = 0; i < PCI_SLOT_MAX; i++) {
>>> -        pbdev = &s->pbdev[i];
>>> -
>>> -        if (pbdev->fh == 0) {
>>> -            continue;
>>> -        }
>>> -
>>> -        if (j == idx) {
>>> +    QTAILQ_FOREACH(kid, &s->fbus->qbus.children, sibling) {
>>> +        pbdev = (S390PCIBusDevice *)kid->child;
>>> +        if (i == idx) {
>>>                return pbdev;
>>>            }
>>> -        j++;
>>> +        i++;
>>>        }
>>>
>>>        return NULL;
>>> @@ -167,16 +164,17 @@ S390PCIBusDevice *s390_pci_find_dev_by_idx(uint32_t idx)
>>>    S390PCIBusDevice *s390_pci_find_dev_by_fh(uint32_t fh)
>>>    {
>>>        S390PCIBusDevice *pbdev;
>>> -    int i;
>>> -    S390pciState *s = S390_PCI_HOST_BRIDGE(
>>> -        object_resolve_path(TYPE_S390_PCI_HOST_BRIDGE, NULL));
>>> +    BusChild *kid;
>>> +    S390PCIFacility *s = S390_PCI_FACILITY(
>>> +        object_resolve_path(TYPE_S390_PCI_FACILITY, NULL));
>>> +
>>>
>>>        if (!s || !fh) {
>>>            return NULL;
>>>        }
>>>
>>> -    for (i = 0; i < PCI_SLOT_MAX; i++) {
>>> -        pbdev = &s->pbdev[i];
>>> +    QTAILQ_FOREACH(kid, &s->fbus->qbus.children, sibling) {
>>> +        pbdev = (S390PCIBusDevice *)kid->child;
>>>            if (pbdev->fh == fh) {
>>>                return pbdev;
>>>            }
>>> @@ -185,12 +183,33 @@ S390PCIBusDevice *s390_pci_find_dev_by_fh(uint32_t fh)
>>>        return NULL;
>>>    }
>>>
>>> +static S390PCIBusDevice *s390_pci_find_dev_by_pdev(PCIDevice *pdev)
>>> +{
>>> +    S390PCIBusDevice *pbdev;
>>> +    BusChild *kid;
>>> +    S390PCIFacility *s = S390_PCI_FACILITY(
>>> +        object_resolve_path(TYPE_S390_PCI_FACILITY, NULL));
>>> +
>>> +    if (!s || !pdev) {
>>> +         return NULL;
>>> +     }
>>> +
>>> +    QTAILQ_FOREACH(kid, &s->fbus->qbus.children, sibling) {
>>> +        pbdev = (S390PCIBusDevice *)kid->child;
>>> +        if (pbdev->pdev == pdev) {
>>> +            return pbdev;
>>> +        }
>>> +    }
>>> +
>>> +    return NULL;
>>> +}
>>> +
>>>    static void s390_pci_generate_event(uint8_t cc, uint16_t pec, uint32_t fh,
>>>                                        uint32_t fid, uint64_t faddr, uint32_t e)
>>>    {
>>>        SeiContainer *sei_cont;
>>> -    S390pciState *s = S390_PCI_HOST_BRIDGE(
>>> -        object_resolve_path(TYPE_S390_PCI_HOST_BRIDGE, NULL));
>>> +    S390PCIFacility *s = S390_PCI_FACILITY(
>>> +        object_resolve_path(TYPE_S390_PCI_FACILITY, NULL));
>>>
>>>        if (!s) {
>>>            return;
>>> @@ -305,7 +324,9 @@ static IOMMUTLBEntry s390_translate_iommu(MemoryRegion *iommu, hwaddr addr,
>>>    {
>>>        uint64_t pte;
>>>        uint32_t flags;
>>> -    S390PCIBusDevice *pbdev = container_of(iommu, S390PCIBusDevice, mr);
>>> +    S390PCIDeviceConn *conn = container_of(iommu, S390PCIDeviceConn,
>>> +                                           iommu_mr);
>>> +    S390PCIBusDevice *pbdev = conn->zpci;
>>>        S390pciState *s = S390_PCI_HOST_BRIDGE(pci_device_root_bus(pbdev->pdev)
>>>                                               ->qbus.parent);
>>>        IOMMUTLBEntry ret = {
>>> @@ -316,8 +337,14 @@ static IOMMUTLBEntry s390_translate_iommu(MemoryRegion *iommu, hwaddr addr,
>>>            .perm = IOMMU_NONE,
>>>        };
>>>
>>> +    if (!pbdev) {
>>> +        return ret;
>>> +    }
>>> +
>>>        DPRINTF("iommu trans addr 0x%" PRIx64 "\n", addr);
>>>
>>> +    s = S390_PCI_HOST_BRIDGE(pci_device_root_bus(pbdev->pdev)->qbus.parent);
>>> +
>>>        /* s390 does not have an APIC mapped to main storage so we use
>>>         * a separate AddressSpace only for msix notifications
>>>         */
>>> @@ -379,7 +406,7 @@ static AddressSpace *s390_pci_dma_iommu(PCIBus *bus, void *opaque, int devfn)
>>>    {
>>>        S390pciState *s = opaque;
>>>
>>> -    return &s->pbdev[PCI_SLOT(devfn)].as;
>>> +    return &s->conn[PCI_SLOT(devfn)].iommu_as;
>>>    }
>>>
>>>    static uint8_t set_ind_atomic(uint64_t ind_loc, uint8_t to_be_set)
>>> @@ -452,9 +479,10 @@ static void s390_pcihost_init_as(S390pciState *s)
>>>        int i;
>>>
>>>        for (i = 0; i < PCI_SLOT_MAX; i++) {
>>> -        memory_region_init_iommu(&s->pbdev[i].mr, OBJECT(s),
>>> +        memory_region_init_iommu(&s->conn[i].iommu_mr, OBJECT(s),
>>>                                     &s390_iommu_ops, "iommu-s390", UINT64_MAX);
>>> -        address_space_init(&s->pbdev[i].as, &s->pbdev[i].mr, "iommu-pci");
>>> +        address_space_init(&s->conn[i].iommu_as, &s->conn[i].iommu_mr,
>>> +                           "iommu-pci");
>>>        }
>>>
>>>        memory_region_init_io(&s->msix_notify_mr, OBJECT(s),
>>> @@ -481,7 +509,7 @@ static int s390_pcihost_init(SysBusDevice *dev)
>>>        bus = BUS(b);
>>>        qbus_set_hotplug_handler(bus, DEVICE(dev), NULL);
>>>        phb->bus = b;
>>> -    QTAILQ_INIT(&s->pending_sei);
>>> +
>>>        return 0;
>>>    }
>>>
>>> @@ -513,29 +541,12 @@ static int s390_pcihost_setup_msix(S390PCIBusDevice *pbdev)
>>>        return 0;
>>>    }
>>>
>>> +/* this function is useless, just keep it to make the patch more
>>> +   readable. Without this function, the diff will mix these hotplug
>>> +   functions together. will be removed in future */
>>>    static void s390_pcihost_hot_plug(HotplugHandler *hotplug_dev,
>>>                                      DeviceState *dev, Error **errp)
>>>    {
>>> -    PCIDevice *pci_dev = PCI_DEVICE(dev);
>>> -    S390PCIBusDevice *pbdev;
>>> -    S390pciState *s = S390_PCI_HOST_BRIDGE(pci_device_root_bus(pci_dev)
>>> -                                           ->qbus.parent);
>>> -
>>> -    pbdev = &s->pbdev[PCI_SLOT(pci_dev->devfn)];
>>> -
>>> -    pbdev->fid = s390_pci_get_pfid(pci_dev);
>>> -    pbdev->pdev = pci_dev;
>>> -    pbdev->configured = true;
>>> -    pbdev->fh = s390_pci_get_pfh(pci_dev);
>>> -
>>> -    s390_pcihost_setup_msix(pbdev);
>>> -
>>> -    if (dev->hotplugged) {
>>> -        s390_pci_generate_plug_event(HP_EVENT_RESERVED_TO_STANDBY,
>>> -                                     pbdev->fh, pbdev->fid);
>>> -        s390_pci_generate_plug_event(HP_EVENT_TO_CONFIGURED,
>>> -                                     pbdev->fh, pbdev->fid);
>>> -    }
>>>        return;
>>>    }
>>>
>>> @@ -543,31 +554,30 @@ static void s390_pcihost_hot_unplug(HotplugHandler *hotplug_dev,
>>>                                        DeviceState *dev, Error **errp)
>>>    {
>>>        PCIDevice *pci_dev = PCI_DEVICE(dev);
>>> -    S390pciState *s = S390_PCI_HOST_BRIDGE(pci_device_root_bus(pci_dev)
>>> -                                           ->qbus.parent);
>>> -    S390PCIBusDevice *pbdev = &s->pbdev[PCI_SLOT(pci_dev->devfn)];
>>> -
>>> -    if (pbdev->configured) {
>>> -        pbdev->configured = false;
>>> -        s390_pci_generate_plug_event(HP_EVENT_CONFIGURED_TO_STBRES,
>>> -                                     pbdev->fh, pbdev->fid);
>>> +    S390PCIBusDevice *pbdev;
>>> +    HotplugHandler *hotplug_ctrl;
>>> +    S390PCIFacility *f = S390_PCI_FACILITY(
>>> +        object_resolve_path(TYPE_S390_PCI_FACILITY, NULL));
>>> +    S390PCIFacilityClass *k = S390_PCI_FACILITY_GET_CLASS(f);
>>> +    HotplugHandlerClass *hdc = HOTPLUG_HANDLER_CLASS(k);
>>> +
>>> +    /* unplug corresponding zpci device */
>>> +    pbdev = s390_pci_find_dev_by_pdev(pci_dev);
>>> +    if (pbdev) {
>>> +        hotplug_ctrl = pbdev->qdev.parent_bus->hotplug_handler;
>>> +        if (hdc->unplug_request) {
>>> +            hdc->unplug_request(hotplug_ctrl, &pbdev->qdev, errp);
>>> +        }
>>>        }
>>>
>>> -    s390_pci_generate_plug_event(HP_EVENT_STANDBY_TO_RESERVED,
>>> -                                 pbdev->fh, pbdev->fid);
>>> -    pbdev->fh = 0;
>>> -    pbdev->fid = 0;
>>> -    pbdev->pdev = NULL;
>>>        object_unparent(OBJECT(pci_dev));
>>>    }
>>>
>>>    static void s390_pcihost_class_init(ObjectClass *klass, void *data)
>>>    {
>>>        SysBusDeviceClass *k = SYS_BUS_DEVICE_CLASS(klass);
>>> -    DeviceClass *dc = DEVICE_CLASS(klass);
>>>        HotplugHandlerClass *hc = HOTPLUG_HANDLER_CLASS(klass);
>>>
>>> -    dc->cannot_instantiate_with_device_add_yet = true;
>>>        k->init = s390_pcihost_init;
>>>        hc->plug = s390_pcihost_hot_plug;
>>>        hc->unplug = s390_pcihost_hot_unplug;
>>> @@ -585,9 +595,156 @@ static const TypeInfo s390_pcihost_info = {
>>>        }
>>>    };
>>>
>>> +static void s390_pci_device_hot_plug(HotplugHandler *hotplug_dev,
>>> +                                     DeviceState *dev, Error **errp)
>>> +{
>>> +    S390PCIBusDevice *zpci = S390_PCI_DEVICE(dev);
>>> +
>>> +    zpci->configured = true;
>>> +
>>> +    if (dev->hotplugged) {
>>> +        s390_pci_generate_plug_event(HP_EVENT_RESERVED_TO_STANDBY,
>>> +                                     zpci->fh, zpci->fid);
>>> +        s390_pci_generate_plug_event(HP_EVENT_TO_CONFIGURED,
>>> +                                     zpci->fh, zpci->fid);
>>> +    }
>>> +}
>>> +
>>> +static void s390_pci_device_hot_unplug_request(HotplugHandler *hotplug_dev,
>>> +                                       DeviceState *dev, Error **errp)
>>> +{
>>> +    S390PCIBusDevice *zpci = S390_PCI_DEVICE(dev);
>>> +
>>> +    if (zpci->configured) {
>>> +        zpci->configured = false;
>>> +        s390_pci_generate_plug_event(HP_EVENT_CONFIGURED_TO_STBRES,
>>> +                                     zpci->fh, zpci->fid);
>>> +    }
>>> +
>>> +    s390_pci_generate_plug_event(HP_EVENT_STANDBY_TO_RESERVED,
>>> +                                 zpci->fh, zpci->fid);
>>> +
>>> +    zpci->is_unplugged = true;
>>> +}
>>> +
>>> +static const TypeInfo s390_pci_fac_bus_info = {
>>> +    .name = TYPE_S390_PCI_FAC_BUS,
>>> +    .parent = TYPE_BUS,
>>> +    .instance_size = sizeof(S390PCIFacBus),
>>> +};
>>> +
>>> +static int s390_pci_facility_init(S390PCIFacility *f)
>>> +{
>>> +    DeviceState *dev = DEVICE(f);
>>> +
>>> +    QTAILQ_INIT(&f->pending_sei);
>>> +    msi_supported = true;
>>> +    f->fbus = S390_PCI_FAC_BUS(qbus_create(TYPE_S390_PCI_FAC_BUS, dev, NULL));
>>> +    qbus_set_hotplug_handler(BUS(&f->fbus->qbus), DEVICE(dev), NULL);
>>> +
>>> +    return 0;
>>> +}
>>> +
>>> +static void s390_pci_facility_class_init(ObjectClass *klass, void *data)
>>> +{
>>> +    S390PCIFacilityClass *k = S390_PCI_FACILITY_CLASS(klass);
>>> +    HotplugHandlerClass *hc = HOTPLUG_HANDLER_CLASS(k);
>>> +
>>> +    k->init = s390_pci_facility_init;
>>> +    hc->plug = s390_pci_device_hot_plug;
>>> +    hc->unplug_request = s390_pci_device_hot_unplug_request;
>>> +}
>>> +
>>> +static const TypeInfo s390_pci_facility_info = {
>>> +    .name          = TYPE_S390_PCI_FACILITY,
>>> +    .parent        = TYPE_SYS_BUS_DEVICE,
>>> +    .instance_size = sizeof(S390PCIFacility),
>>> +    .class_init    = s390_pci_facility_class_init,
>>> +    .class_size    = sizeof(S390PCIFacilityClass),
>>> +    .interfaces = (InterfaceInfo[]) {
>>> +        { TYPE_HOTPLUG_HANDLER },
>>> +        { }
>>> +    }
>>> +};
>>> +
>>> +static void s390_pci_device_realize(DeviceState *dev, Error **errp)
>>> +{
>>> +    S390PCIBusDevice *zpci = S390_PCI_DEVICE(dev);
>>> +    S390PCIBusDevice *tmp;
>>> +    S390pciState *s;
>>> +    BusChild *kid;
>>> +    PCIDevice *pdev;
>>> +    int ret;
>>> +    S390PCIFacility *f = S390_PCI_FACILITY(
>>> +        object_resolve_path(TYPE_S390_PCI_FACILITY, NULL));
>>> +
>>> +    ret = pci_qdev_find_device(zpci->pci_id, &pdev);
>>> +    if (ret < 0) {
>>> +        error_setg(errp, "vfio pci device %s not found", zpci->pci_id);
>>> +        return;
>>> +    }
>>> +
>>> +    QTAILQ_FOREACH(kid, &f->fbus->qbus.children, sibling) {
>>> +        tmp = (S390PCIBusDevice *)kid->child;
>>> +        if (tmp == zpci) {
>>> +            continue;
>>> +        }
>>> +
>>> +        if (tmp->fid == zpci->fid || tmp->uid == zpci->uid ||
>>> +            !strcmp(tmp->pci_id, zpci->pci_id)) {
>>> +            error_setg(errp, "zpci needs unique fid, uid and pci_id");
>>> +            return;
>>> +        }
>>> +    }
>>> +
>>> +    s = S390_PCI_HOST_BRIDGE(pci_device_root_bus(pdev)->qbus.parent);
>>> +    s->conn[PCI_SLOT(pdev->devfn)].zpci = zpci;
>>> +
>>> +    zpci->pdev = pdev;
>>> +    zpci->fh = zpci->fid | FH_VIRT;
>>> +    s390_pcihost_setup_msix(zpci);
>>> +}
>>> +
>>> +static void s390_pci_device_unrealize(DeviceState *dev, Error **errp)
>>> +{
>>> +    S390PCIBusDevice *zpci = S390_PCI_DEVICE(dev);
>>> +
>>> +    zpci->fh = 0;
>>> +    zpci->fid = 0;
>>> +    zpci->pdev = NULL;
>>> +}
>>> +
>>> +static Property s390_pci_device_properties[] = {
>>> +    DEFINE_PROP_UINT32("fid", S390PCIBusDevice, fid, 0),
>>> +    DEFINE_PROP_UINT32("uid", S390PCIBusDevice, uid, 0),
>>> +    DEFINE_PROP_STRING("pci_id", S390PCIBusDevice, pci_id),
>>> +    DEFINE_PROP_END_OF_LIST(),
>>> +};
>>> +
>>> +static void s390_pci_device_class_init(ObjectClass *klass, void *data)
>>> +{
>>> +    DeviceClass *dc = DEVICE_CLASS(klass);
>>> +
>>> +    dc->desc = "s390 pci device";
>>> +    dc->bus_type = TYPE_S390_PCI_FAC_BUS;
>>> +    dc->realize = s390_pci_device_realize;
>>> +    dc->unrealize = s390_pci_device_unrealize;
>>> +    dc->props = s390_pci_device_properties;
>>> +}
>>> +
>>> +static const TypeInfo s390_pci_device_type_info = {
>>> +    .name = TYPE_S390_PCI_DEVICE,
>>> +    .parent = TYPE_DEVICE,
>>> +    .instance_size = sizeof(S390PCIBusDevice),
>>> +    .class_init = s390_pci_device_class_init,
>>> +};
>>> +
>>>    static void s390_pci_register_types(void)
>>>    {
>>>        type_register_static(&s390_pcihost_info);
>>> +    type_register_static(&s390_pci_facility_info);
>>> +    type_register_static(&s390_pci_fac_bus_info);
>>> +    type_register_static(&s390_pci_device_type_info);
>>>    }
>>>
>>>    type_init(s390_pci_register_types)
>>> diff --git a/hw/s390x/s390-pci-bus.h b/hw/s390x/s390-pci-bus.h
>>> index 464a92e..5bf3913 100644
>>> --- a/hw/s390x/s390-pci-bus.h
>>> +++ b/hw/s390x/s390-pci-bus.h
>>> @@ -149,6 +149,21 @@ enum ZpciIoatDtype {
>>>    #define ZPCI_TABLE_VALID_MASK           0x20
>>>    #define ZPCI_TABLE_PROT_MASK            0x200
>>>
>>> +#define TYPE_S390_PCI_FACILITY "s390-pci-facility"
>>> +#define TYPE_S390_PCI_FAC_BUS "s390-pci-fac-bus"
>>> +#define TYPE_S390_PCI_DEVICE "zpci"
>>> +
>>> +#define S390_PCI_FACILITY(obj) \
>>> +    OBJECT_CHECK(S390PCIFacility, (obj), TYPE_S390_PCI_FACILITY)
>>> +#define S390_PCI_FAC_BUS(obj) \
>>> +    OBJECT_CHECK(S390PCIFacBus, (obj), TYPE_S390_PCI_FAC_BUS)
>>> +#define S390_PCI_FACILITY_CLASS(klass) \
>>> +    OBJECT_CLASS_CHECK(S390PCIFacilityClass, (klass), TYPE_S390_PCI_FACILITY)
>>> +#define S390_PCI_DEVICE(obj) \
>>> +    OBJECT_CHECK(S390PCIBusDevice, (obj), TYPE_S390_PCI_DEVICE)
>>> +#define S390_PCI_FACILITY_GET_CLASS(obj) \
>>> +    OBJECT_GET_CLASS(S390PCIFacilityClass, (obj), TYPE_S390_PCI_FACILITY)
>>> +
>>>    typedef struct SeiContainer {
>>>        QTAILQ_ENTRY(SeiContainer) link;
>>>        uint32_t fid;
>>> @@ -214,12 +229,16 @@ typedef struct S390MsixInfo {
>>>    } S390MsixInfo;
>>>
>>>    typedef struct S390PCIBusDevice {
>>> +    DeviceState qdev;
>>>        PCIDevice *pdev;
>>>        bool configured;
>>> +    bool is_unplugged;
>>>        bool error_state;
>>>        bool lgstg_blocked;
>>>        uint32_t fh;
>>>        uint32_t fid;
>>> +    uint32_t uid;
>>> +    char *pci_id;
>>>        uint64_t g_iota;
>>>        uint64_t pba;
>>>        uint64_t pal;
>>> @@ -229,21 +248,42 @@ typedef struct S390PCIBusDevice {
>>>        uint8_t sum;
>>>        S390MsixInfo msix;
>>>        AdapterRoutes routes;
>>> -    AddressSpace as;
>>> -    MemoryRegion mr;
>>> +    QLIST_ENTRY(S390PCIDevice) entry;
>>>    } S390PCIBusDevice;
>>>
>>> +typedef struct S390PCIDeviceConn {
>>> +    S390PCIBusDevice *zpci;
>>> +    AddressSpace iommu_as;
>>> +    MemoryRegion iommu_mr;
>>> +} S390PCIDeviceConn;
>>> +
>>>    typedef struct S390pciState {
>>>        PCIHostState parent_obj;
>>> -    S390PCIBusDevice pbdev[PCI_SLOT_MAX];
>>> +    S390PCIDeviceConn conn[PCI_SLOT_MAX];
>>>        AddressSpace msix_notify_as;
>>>        MemoryRegion msix_notify_mr;
>>> -    QTAILQ_HEAD(, SeiContainer) pending_sei;
>>>    } S390pciState;
>>>
>>> +typedef struct S390PCIFacBus {
>>> +    BusState qbus;
>>> +} S390PCIFacBus;
>>> +
>>> +typedef struct S390PCIFacility {
>>> +    SysBusDevice parent_obj;
>>> +    S390PCIFacBus *fbus;
>>> +    QTAILQ_HEAD(, SeiContainer) pending_sei;
>>> +} S390PCIFacility;
>>> +
>>> +typedef struct S390PCIFacilityClass {
>>> +    DeviceClass parent_class;
>>> +    int (*init)(S390PCIFacility *f);
>>> +} S390PCIFacilityClass;
>>> +
>>>    int chsc_sei_nt2_get_event(void *res);
>>>    int chsc_sei_nt2_have_event(void);
>>>    void s390_pci_sclp_configure(int configure, SCCB *sccb);
>>> +void s390_pci_device_enable(S390PCIBusDevice *zpci);
>>> +void s390_pci_device_disable(S390PCIBusDevice *zpci);
>>>    S390PCIBusDevice *s390_pci_find_dev_by_idx(uint32_t idx);
>>>    S390PCIBusDevice *s390_pci_find_dev_by_fh(uint32_t fh);
>>>    S390PCIBusDevice *s390_pci_find_dev_by_fid(uint32_t fid);
>>> diff --git a/hw/s390x/s390-pci-inst.c b/hw/s390x/s390-pci-inst.c
>>> index cac3d83..fc8e8e1 100644
>>> --- a/hw/s390x/s390-pci-inst.c
>>> +++ b/hw/s390x/s390-pci-inst.c
>>> @@ -208,12 +208,12 @@ int clp_service_call(S390CPU *cpu, uint8_t r2)
>>>
>>>            switch (reqsetpci->oc) {
>>>            case CLP_SET_ENABLE_PCI_FN:
>>> -            pbdev->fh = pbdev->fh | 1 << ENABLE_BIT_OFFSET;
>>> +            s390_pci_device_enable(pbdev);
>>>                stl_p(&ressetpci->fh, pbdev->fh);
>>>                stw_p(&ressetpci->hdr.rsp, CLP_RC_OK);
>>>                break;
>>>            case CLP_SET_DISABLE_PCI_FN:
>>> -            pbdev->fh = pbdev->fh & ~(1 << ENABLE_BIT_OFFSET);
>>> +            s390_pci_device_disable(pbdev);
>>>                pbdev->error_state = false;
>>>                pbdev->lgstg_blocked = false;
>>>                stl_p(&ressetpci->fh, pbdev->fh);
>>> diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c
>>> index 312b8ac..7ee5202 100644
>>> --- a/hw/s390x/s390-virtio-ccw.c
>>> +++ b/hw/s390x/s390-virtio-ccw.c
>>> @@ -177,8 +177,8 @@ static void ccw_init(MachineState *machine)
>>>                          machine->initrd_filename, "s390-ccw.img", true);
>>>        s390_flic_init();
>>>
>>> -    dev = qdev_create(NULL, TYPE_S390_PCI_HOST_BRIDGE);
>>> -    object_property_add_child(qdev_get_machine(), TYPE_S390_PCI_HOST_BRIDGE,
>>> +    dev = qdev_create(NULL, TYPE_S390_PCI_FACILITY);
>>> +    object_property_add_child(qdev_get_machine(), TYPE_S390_PCI_FACILITY,
>>>                                  OBJECT(dev), NULL);
>>>        qdev_init_nofail(dev);
>>>
>>
>

Re: [Qemu-devel] [PATCH v1 1/1] S390 pci infrastructure modeling

[Paolo Bonzini]

On 12/05/2015 11:07, Cornelia Huck wrote:
>>> This patch contains the actual interesting changes.
>>> usage example:
>>> -device s390-pcihost
>>> -device vfio-pci,host=0000:00:00.0,id=vpci1
>>> -device zpci,fid=2,uid=5,pci_id=vpci1,id=zpci1

One possible alternative could be making each zpci device expose a PCI
bus, as if it were a PCI bridge.  The actual PCI device is then hung
below the zpci device:

  -device s390-pcihost,id=s390pci
  -device zpci,fid=2,uid=5,id=zpci1,bus=s390pci.0
  -device vfio-pci,host=00:00.0,id=vpci1,bus=zpci1.0

Would this make sense from an s390 perspective?

Paolo

>>> The first line will create a s390 pci host bridge
>>> and init the root bus. The second line will create
>>> a standard vfio pci device, and attach it to the
>>> root bus. These are similiar to the standard process
>>> to define a pci device on other platform.
>>>
>>> The third line will create a s390 pci device to
>>> store s390 specific information, and references
>>> the corresponding vfio pci device via device id.
>>> We create a s390 pci facility bus to hold all the
>>> zpci devices.
>>>
>>> Signed-off-by: Hong Bo Li <lihbbj@linux.vnet.ibm.com>
>>> ---
>>>   hw/s390x/s390-pci-bus.c    | 317 +++++++++++++++++++++++++++++++++------------
>>>   hw/s390x/s390-pci-bus.h    |  48 ++++++-
>>>   hw/s390x/s390-pci-inst.c   |   4 +-
>>>   hw/s390x/s390-virtio-ccw.c |   4 +-
>>>   4 files changed, 285 insertions(+), 88 deletions(-)
> 
> This looks fine from my s390 perspective, but I'd love to see some
> feedback from PCI experts on this. Does modelling our s390 oddities in
> this way sit fine with the general PCI infrastructure?
> 
>>>
>>> diff --git a/hw/s390x/s390-pci-bus.c b/hw/s390x/s390-pci-bus.c
>>> index 3c086f6..c81093e 100644
>>> --- a/hw/s390x/s390-pci-bus.c
>>> +++ b/hw/s390x/s390-pci-bus.c
>>> @@ -32,8 +32,8 @@ int chsc_sei_nt2_get_event(void *res)
>>>       PciCcdfErr *eccdf;
>>>       int rc = 1;
>>>       SeiContainer *sei_cont;
>>> -    S390pciState *s = S390_PCI_HOST_BRIDGE(
>>> -        object_resolve_path(TYPE_S390_PCI_HOST_BRIDGE, NULL));
>>> +    S390PCIFacility *s = S390_PCI_FACILITY(
>>> +        object_resolve_path(TYPE_S390_PCI_FACILITY, NULL));
>>>
>>>       if (!s) {
>>>           return rc;
>>> @@ -72,8 +72,8 @@ int chsc_sei_nt2_get_event(void *res)
>>>
>>>   int chsc_sei_nt2_have_event(void)
>>>   {
>>> -    S390pciState *s = S390_PCI_HOST_BRIDGE(
>>> -        object_resolve_path(TYPE_S390_PCI_HOST_BRIDGE, NULL));
>>> +    S390PCIFacility *s = S390_PCI_FACILITY(
>>> +        object_resolve_path(TYPE_S390_PCI_FACILITY, NULL));
>>>
>>>       if (!s) {
>>>           return 0;
>>> @@ -82,20 +82,32 @@ int chsc_sei_nt2_have_event(void)
>>>       return !QTAILQ_EMPTY(&s->pending_sei);
>>>   }
>>>
>>> +void s390_pci_device_enable(S390PCIBusDevice *zpci)
>>> +{
>>> +    zpci->fh = zpci->fh | 1 << ENABLE_BIT_OFFSET;
>>> +}
>>> +
>>> +void s390_pci_device_disable(S390PCIBusDevice *zpci)
>>> +{
>>> +    zpci->fh = zpci->fh & ~(1 << ENABLE_BIT_OFFSET);
>>> +    if (zpci->is_unplugged)
>>> +        object_unparent(OBJECT(zpci));
>>> +}
>>> +
>>>   S390PCIBusDevice *s390_pci_find_dev_by_fid(uint32_t fid)
>>>   {
>>>       S390PCIBusDevice *pbdev;
>>> -    int i;
>>> -    S390pciState *s = S390_PCI_HOST_BRIDGE(
>>> -        object_resolve_path(TYPE_S390_PCI_HOST_BRIDGE, NULL));
>>> +    BusChild *kid;
>>> +    S390PCIFacility *s = S390_PCI_FACILITY(
>>> +        object_resolve_path(TYPE_S390_PCI_FACILITY, NULL));
>>>
>>>       if (!s) {
>>>           return NULL;
>>>       }
>>>
>>> -    for (i = 0; i < PCI_SLOT_MAX; i++) {
>>> -        pbdev = &s->pbdev[i];
>>> -        if ((pbdev->fh != 0) && (pbdev->fid == fid)) {
>>> +    QTAILQ_FOREACH(kid, &s->fbus->qbus.children, sibling) {
>>> +        pbdev = (S390PCIBusDevice *)kid->child;
>>> +        if (pbdev->fid == fid) {
>>>               return pbdev;
>>>           }
>>>       }
>>> @@ -126,39 +138,24 @@ void s390_pci_sclp_configure(int configure, SCCB *sccb)
>>>       return;
>>>   }
>>>
>>> -static uint32_t s390_pci_get_pfid(PCIDevice *pdev)
>>> -{
>>> -    return PCI_SLOT(pdev->devfn);
>>> -}
>>> -
>>> -static uint32_t s390_pci_get_pfh(PCIDevice *pdev)
>>> -{
>>> -    return PCI_SLOT(pdev->devfn) | FH_VIRT;
>>> -}
>>> -
>>>   S390PCIBusDevice *s390_pci_find_dev_by_idx(uint32_t idx)
>>>   {
>>>       S390PCIBusDevice *pbdev;
>>> -    int i;
>>> -    int j = 0;
>>> -    S390pciState *s = S390_PCI_HOST_BRIDGE(
>>> -        object_resolve_path(TYPE_S390_PCI_HOST_BRIDGE, NULL));
>>> +    BusChild *kid;
>>> +    int i = 0;
>>> +    S390PCIFacility *s = S390_PCI_FACILITY(
>>> +        object_resolve_path(TYPE_S390_PCI_FACILITY, NULL));
>>>
>>>       if (!s) {
>>>           return NULL;
>>>       }
>>>
>>> -    for (i = 0; i < PCI_SLOT_MAX; i++) {
>>> -        pbdev = &s->pbdev[i];
>>> -
>>> -        if (pbdev->fh == 0) {
>>> -            continue;
>>> -        }
>>> -
>>> -        if (j == idx) {
>>> +    QTAILQ_FOREACH(kid, &s->fbus->qbus.children, sibling) {
>>> +        pbdev = (S390PCIBusDevice *)kid->child;
>>> +        if (i == idx) {
>>>               return pbdev;
>>>           }
>>> -        j++;
>>> +        i++;
>>>       }
>>>
>>>       return NULL;
>>> @@ -167,16 +164,17 @@ S390PCIBusDevice *s390_pci_find_dev_by_idx(uint32_t idx)
>>>   S390PCIBusDevice *s390_pci_find_dev_by_fh(uint32_t fh)
>>>   {
>>>       S390PCIBusDevice *pbdev;
>>> -    int i;
>>> -    S390pciState *s = S390_PCI_HOST_BRIDGE(
>>> -        object_resolve_path(TYPE_S390_PCI_HOST_BRIDGE, NULL));
>>> +    BusChild *kid;
>>> +    S390PCIFacility *s = S390_PCI_FACILITY(
>>> +        object_resolve_path(TYPE_S390_PCI_FACILITY, NULL));
>>> +
>>>
>>>       if (!s || !fh) {
>>>           return NULL;
>>>       }
>>>
>>> -    for (i = 0; i < PCI_SLOT_MAX; i++) {
>>> -        pbdev = &s->pbdev[i];
>>> +    QTAILQ_FOREACH(kid, &s->fbus->qbus.children, sibling) {
>>> +        pbdev = (S390PCIBusDevice *)kid->child;
>>>           if (pbdev->fh == fh) {
>>>               return pbdev;
>>>           }
>>> @@ -185,12 +183,33 @@ S390PCIBusDevice *s390_pci_find_dev_by_fh(uint32_t fh)
>>>       return NULL;
>>>   }
>>>
>>> +static S390PCIBusDevice *s390_pci_find_dev_by_pdev(PCIDevice *pdev)
>>> +{
>>> +    S390PCIBusDevice *pbdev;
>>> +    BusChild *kid;
>>> +    S390PCIFacility *s = S390_PCI_FACILITY(
>>> +        object_resolve_path(TYPE_S390_PCI_FACILITY, NULL));
>>> +
>>> +    if (!s || !pdev) {
>>> +         return NULL;
>>> +     }
>>> +
>>> +    QTAILQ_FOREACH(kid, &s->fbus->qbus.children, sibling) {
>>> +        pbdev = (S390PCIBusDevice *)kid->child;
>>> +        if (pbdev->pdev == pdev) {
>>> +            return pbdev;
>>> +        }
>>> +    }
>>> +
>>> +    return NULL;
>>> +}
>>> +
>>>   static void s390_pci_generate_event(uint8_t cc, uint16_t pec, uint32_t fh,
>>>                                       uint32_t fid, uint64_t faddr, uint32_t e)
>>>   {
>>>       SeiContainer *sei_cont;
>>> -    S390pciState *s = S390_PCI_HOST_BRIDGE(
>>> -        object_resolve_path(TYPE_S390_PCI_HOST_BRIDGE, NULL));
>>> +    S390PCIFacility *s = S390_PCI_FACILITY(
>>> +        object_resolve_path(TYPE_S390_PCI_FACILITY, NULL));
>>>
>>>       if (!s) {
>>>           return;
>>> @@ -305,7 +324,9 @@ static IOMMUTLBEntry s390_translate_iommu(MemoryRegion *iommu, hwaddr addr,
>>>   {
>>>       uint64_t pte;
>>>       uint32_t flags;
>>> -    S390PCIBusDevice *pbdev = container_of(iommu, S390PCIBusDevice, mr);
>>> +    S390PCIDeviceConn *conn = container_of(iommu, S390PCIDeviceConn,
>>> +                                           iommu_mr);
>>> +    S390PCIBusDevice *pbdev = conn->zpci;
>>>       S390pciState *s = S390_PCI_HOST_BRIDGE(pci_device_root_bus(pbdev->pdev)
>>>                                              ->qbus.parent);
>>>       IOMMUTLBEntry ret = {
>>> @@ -316,8 +337,14 @@ static IOMMUTLBEntry s390_translate_iommu(MemoryRegion *iommu, hwaddr addr,
>>>           .perm = IOMMU_NONE,
>>>       };
>>>
>>> +    if (!pbdev) {
>>> +        return ret;
>>> +    }
>>> +
>>>       DPRINTF("iommu trans addr 0x%" PRIx64 "\n", addr);
>>>
>>> +    s = S390_PCI_HOST_BRIDGE(pci_device_root_bus(pbdev->pdev)->qbus.parent);
>>> +
>>>       /* s390 does not have an APIC mapped to main storage so we use
>>>        * a separate AddressSpace only for msix notifications
>>>        */
>>> @@ -379,7 +406,7 @@ static AddressSpace *s390_pci_dma_iommu(PCIBus *bus, void *opaque, int devfn)
>>>   {
>>>       S390pciState *s = opaque;
>>>
>>> -    return &s->pbdev[PCI_SLOT(devfn)].as;
>>> +    return &s->conn[PCI_SLOT(devfn)].iommu_as;
>>>   }
>>>
>>>   static uint8_t set_ind_atomic(uint64_t ind_loc, uint8_t to_be_set)
>>> @@ -452,9 +479,10 @@ static void s390_pcihost_init_as(S390pciState *s)
>>>       int i;
>>>
>>>       for (i = 0; i < PCI_SLOT_MAX; i++) {
>>> -        memory_region_init_iommu(&s->pbdev[i].mr, OBJECT(s),
>>> +        memory_region_init_iommu(&s->conn[i].iommu_mr, OBJECT(s),
>>>                                    &s390_iommu_ops, "iommu-s390", UINT64_MAX);
>>> -        address_space_init(&s->pbdev[i].as, &s->pbdev[i].mr, "iommu-pci");
>>> +        address_space_init(&s->conn[i].iommu_as, &s->conn[i].iommu_mr,
>>> +                           "iommu-pci");
>>>       }
>>>
>>>       memory_region_init_io(&s->msix_notify_mr, OBJECT(s),
>>> @@ -481,7 +509,7 @@ static int s390_pcihost_init(SysBusDevice *dev)
>>>       bus = BUS(b);
>>>       qbus_set_hotplug_handler(bus, DEVICE(dev), NULL);
>>>       phb->bus = b;
>>> -    QTAILQ_INIT(&s->pending_sei);
>>> +
>>>       return 0;
>>>   }
>>>
>>> @@ -513,29 +541,12 @@ static int s390_pcihost_setup_msix(S390PCIBusDevice *pbdev)
>>>       return 0;
>>>   }
>>>
>>> +/* this function is useless, just keep it to make the patch more
>>> +   readable. Without this function, the diff will mix these hotplug
>>> +   functions together. will be removed in future */
>>>   static void s390_pcihost_hot_plug(HotplugHandler *hotplug_dev,
>>>                                     DeviceState *dev, Error **errp)
>>>   {
>>> -    PCIDevice *pci_dev = PCI_DEVICE(dev);
>>> -    S390PCIBusDevice *pbdev;
>>> -    S390pciState *s = S390_PCI_HOST_BRIDGE(pci_device_root_bus(pci_dev)
>>> -                                           ->qbus.parent);
>>> -
>>> -    pbdev = &s->pbdev[PCI_SLOT(pci_dev->devfn)];
>>> -
>>> -    pbdev->fid = s390_pci_get_pfid(pci_dev);
>>> -    pbdev->pdev = pci_dev;
>>> -    pbdev->configured = true;
>>> -    pbdev->fh = s390_pci_get_pfh(pci_dev);
>>> -
>>> -    s390_pcihost_setup_msix(pbdev);
>>> -
>>> -    if (dev->hotplugged) {
>>> -        s390_pci_generate_plug_event(HP_EVENT_RESERVED_TO_STANDBY,
>>> -                                     pbdev->fh, pbdev->fid);
>>> -        s390_pci_generate_plug_event(HP_EVENT_TO_CONFIGURED,
>>> -                                     pbdev->fh, pbdev->fid);
>>> -    }
>>>       return;
>>>   }
>>>
>>> @@ -543,31 +554,30 @@ static void s390_pcihost_hot_unplug(HotplugHandler *hotplug_dev,
>>>                                       DeviceState *dev, Error **errp)
>>>   {
>>>       PCIDevice *pci_dev = PCI_DEVICE(dev);
>>> -    S390pciState *s = S390_PCI_HOST_BRIDGE(pci_device_root_bus(pci_dev)
>>> -                                           ->qbus.parent);
>>> -    S390PCIBusDevice *pbdev = &s->pbdev[PCI_SLOT(pci_dev->devfn)];
>>> -
>>> -    if (pbdev->configured) {
>>> -        pbdev->configured = false;
>>> -        s390_pci_generate_plug_event(HP_EVENT_CONFIGURED_TO_STBRES,
>>> -                                     pbdev->fh, pbdev->fid);
>>> +    S390PCIBusDevice *pbdev;
>>> +    HotplugHandler *hotplug_ctrl;
>>> +    S390PCIFacility *f = S390_PCI_FACILITY(
>>> +        object_resolve_path(TYPE_S390_PCI_FACILITY, NULL));
>>> +    S390PCIFacilityClass *k = S390_PCI_FACILITY_GET_CLASS(f);
>>> +    HotplugHandlerClass *hdc = HOTPLUG_HANDLER_CLASS(k);
>>> +
>>> +    /* unplug corresponding zpci device */
>>> +    pbdev = s390_pci_find_dev_by_pdev(pci_dev);
>>> +    if (pbdev) {
>>> +        hotplug_ctrl = pbdev->qdev.parent_bus->hotplug_handler;
>>> +        if (hdc->unplug_request) {
>>> +            hdc->unplug_request(hotplug_ctrl, &pbdev->qdev, errp);
>>> +        }
>>>       }
>>>
>>> -    s390_pci_generate_plug_event(HP_EVENT_STANDBY_TO_RESERVED,
>>> -                                 pbdev->fh, pbdev->fid);
>>> -    pbdev->fh = 0;
>>> -    pbdev->fid = 0;
>>> -    pbdev->pdev = NULL;
>>>       object_unparent(OBJECT(pci_dev));
>>>   }
>>>
>>>   static void s390_pcihost_class_init(ObjectClass *klass, void *data)
>>>   {
>>>       SysBusDeviceClass *k = SYS_BUS_DEVICE_CLASS(klass);
>>> -    DeviceClass *dc = DEVICE_CLASS(klass);
>>>       HotplugHandlerClass *hc = HOTPLUG_HANDLER_CLASS(klass);
>>>
>>> -    dc->cannot_instantiate_with_device_add_yet = true;
>>>       k->init = s390_pcihost_init;
>>>       hc->plug = s390_pcihost_hot_plug;
>>>       hc->unplug = s390_pcihost_hot_unplug;
>>> @@ -585,9 +595,156 @@ static const TypeInfo s390_pcihost_info = {
>>>       }
>>>   };
>>>
>>> +static void s390_pci_device_hot_plug(HotplugHandler *hotplug_dev,
>>> +                                     DeviceState *dev, Error **errp)
>>> +{
>>> +    S390PCIBusDevice *zpci = S390_PCI_DEVICE(dev);
>>> +
>>> +    zpci->configured = true;
>>> +
>>> +    if (dev->hotplugged) {
>>> +        s390_pci_generate_plug_event(HP_EVENT_RESERVED_TO_STANDBY,
>>> +                                     zpci->fh, zpci->fid);
>>> +        s390_pci_generate_plug_event(HP_EVENT_TO_CONFIGURED,
>>> +                                     zpci->fh, zpci->fid);
>>> +    }
>>> +}
>>> +
>>> +static void s390_pci_device_hot_unplug_request(HotplugHandler *hotplug_dev,
>>> +                                       DeviceState *dev, Error **errp)
>>> +{
>>> +    S390PCIBusDevice *zpci = S390_PCI_DEVICE(dev);
>>> +
>>> +    if (zpci->configured) {
>>> +        zpci->configured = false;
>>> +        s390_pci_generate_plug_event(HP_EVENT_CONFIGURED_TO_STBRES,
>>> +                                     zpci->fh, zpci->fid);
>>> +    }
>>> +
>>> +    s390_pci_generate_plug_event(HP_EVENT_STANDBY_TO_RESERVED,
>>> +                                 zpci->fh, zpci->fid);
>>> +
>>> +    zpci->is_unplugged = true;
>>> +}
>>> +
>>> +static const TypeInfo s390_pci_fac_bus_info = {
>>> +    .name = TYPE_S390_PCI_FAC_BUS,
>>> +    .parent = TYPE_BUS,
>>> +    .instance_size = sizeof(S390PCIFacBus),
>>> +};
>>> +
>>> +static int s390_pci_facility_init(S390PCIFacility *f)
>>> +{
>>> +    DeviceState *dev = DEVICE(f);
>>> +
>>> +    QTAILQ_INIT(&f->pending_sei);
>>> +    msi_supported = true;
>>> +    f->fbus = S390_PCI_FAC_BUS(qbus_create(TYPE_S390_PCI_FAC_BUS, dev, NULL));
>>> +    qbus_set_hotplug_handler(BUS(&f->fbus->qbus), DEVICE(dev), NULL);
>>> +
>>> +    return 0;
>>> +}
>>> +
>>> +static void s390_pci_facility_class_init(ObjectClass *klass, void *data)
>>> +{
>>> +    S390PCIFacilityClass *k = S390_PCI_FACILITY_CLASS(klass);
>>> +    HotplugHandlerClass *hc = HOTPLUG_HANDLER_CLASS(k);
>>> +
>>> +    k->init = s390_pci_facility_init;
>>> +    hc->plug = s390_pci_device_hot_plug;
>>> +    hc->unplug_request = s390_pci_device_hot_unplug_request;
>>> +}
>>> +
>>> +static const TypeInfo s390_pci_facility_info = {
>>> +    .name          = TYPE_S390_PCI_FACILITY,
>>> +    .parent        = TYPE_SYS_BUS_DEVICE,
>>> +    .instance_size = sizeof(S390PCIFacility),
>>> +    .class_init    = s390_pci_facility_class_init,
>>> +    .class_size    = sizeof(S390PCIFacilityClass),
>>> +    .interfaces = (InterfaceInfo[]) {
>>> +        { TYPE_HOTPLUG_HANDLER },
>>> +        { }
>>> +    }
>>> +};
>>> +
>>> +static void s390_pci_device_realize(DeviceState *dev, Error **errp)
>>> +{
>>> +    S390PCIBusDevice *zpci = S390_PCI_DEVICE(dev);
>>> +    S390PCIBusDevice *tmp;
>>> +    S390pciState *s;
>>> +    BusChild *kid;
>>> +    PCIDevice *pdev;
>>> +    int ret;
>>> +    S390PCIFacility *f = S390_PCI_FACILITY(
>>> +        object_resolve_path(TYPE_S390_PCI_FACILITY, NULL));
>>> +
>>> +    ret = pci_qdev_find_device(zpci->pci_id, &pdev);
>>> +    if (ret < 0) {
>>> +        error_setg(errp, "vfio pci device %s not found", zpci->pci_id);
>>> +        return;
>>> +    }
>>> +
>>> +    QTAILQ_FOREACH(kid, &f->fbus->qbus.children, sibling) {
>>> +        tmp = (S390PCIBusDevice *)kid->child;
>>> +        if (tmp == zpci) {
>>> +            continue;
>>> +        }
>>> +
>>> +        if (tmp->fid == zpci->fid || tmp->uid == zpci->uid ||
>>> +            !strcmp(tmp->pci_id, zpci->pci_id)) {
>>> +            error_setg(errp, "zpci needs unique fid, uid and pci_id");
>>> +            return;
>>> +        }
>>> +    }
>>> +
>>> +    s = S390_PCI_HOST_BRIDGE(pci_device_root_bus(pdev)->qbus.parent);
>>> +    s->conn[PCI_SLOT(pdev->devfn)].zpci = zpci;
>>> +
>>> +    zpci->pdev = pdev;
>>> +    zpci->fh = zpci->fid | FH_VIRT;
>>> +    s390_pcihost_setup_msix(zpci);
>>> +}
>>> +
>>> +static void s390_pci_device_unrealize(DeviceState *dev, Error **errp)
>>> +{
>>> +    S390PCIBusDevice *zpci = S390_PCI_DEVICE(dev);
>>> +
>>> +    zpci->fh = 0;
>>> +    zpci->fid = 0;
>>> +    zpci->pdev = NULL;
>>> +}
>>> +
>>> +static Property s390_pci_device_properties[] = {
>>> +    DEFINE_PROP_UINT32("fid", S390PCIBusDevice, fid, 0),
>>> +    DEFINE_PROP_UINT32("uid", S390PCIBusDevice, uid, 0),
>>> +    DEFINE_PROP_STRING("pci_id", S390PCIBusDevice, pci_id),
>>> +    DEFINE_PROP_END_OF_LIST(),
>>> +};
>>> +
>>> +static void s390_pci_device_class_init(ObjectClass *klass, void *data)
>>> +{
>>> +    DeviceClass *dc = DEVICE_CLASS(klass);
>>> +
>>> +    dc->desc = "s390 pci device";
>>> +    dc->bus_type = TYPE_S390_PCI_FAC_BUS;
>>> +    dc->realize = s390_pci_device_realize;
>>> +    dc->unrealize = s390_pci_device_unrealize;
>>> +    dc->props = s390_pci_device_properties;
>>> +}
>>> +
>>> +static const TypeInfo s390_pci_device_type_info = {
>>> +    .name = TYPE_S390_PCI_DEVICE,
>>> +    .parent = TYPE_DEVICE,
>>> +    .instance_size = sizeof(S390PCIBusDevice),
>>> +    .class_init = s390_pci_device_class_init,
>>> +};
>>> +
>>>   static void s390_pci_register_types(void)
>>>   {
>>>       type_register_static(&s390_pcihost_info);
>>> +    type_register_static(&s390_pci_facility_info);
>>> +    type_register_static(&s390_pci_fac_bus_info);
>>> +    type_register_static(&s390_pci_device_type_info);
>>>   }
>>>
>>>   type_init(s390_pci_register_types)
>>> diff --git a/hw/s390x/s390-pci-bus.h b/hw/s390x/s390-pci-bus.h
>>> index 464a92e..5bf3913 100644
>>> --- a/hw/s390x/s390-pci-bus.h
>>> +++ b/hw/s390x/s390-pci-bus.h
>>> @@ -149,6 +149,21 @@ enum ZpciIoatDtype {
>>>   #define ZPCI_TABLE_VALID_MASK           0x20
>>>   #define ZPCI_TABLE_PROT_MASK            0x200
>>>
>>> +#define TYPE_S390_PCI_FACILITY "s390-pci-facility"
>>> +#define TYPE_S390_PCI_FAC_BUS "s390-pci-fac-bus"
>>> +#define TYPE_S390_PCI_DEVICE "zpci"
>>> +
>>> +#define S390_PCI_FACILITY(obj) \
>>> +    OBJECT_CHECK(S390PCIFacility, (obj), TYPE_S390_PCI_FACILITY)
>>> +#define S390_PCI_FAC_BUS(obj) \
>>> +    OBJECT_CHECK(S390PCIFacBus, (obj), TYPE_S390_PCI_FAC_BUS)
>>> +#define S390_PCI_FACILITY_CLASS(klass) \
>>> +    OBJECT_CLASS_CHECK(S390PCIFacilityClass, (klass), TYPE_S390_PCI_FACILITY)
>>> +#define S390_PCI_DEVICE(obj) \
>>> +    OBJECT_CHECK(S390PCIBusDevice, (obj), TYPE_S390_PCI_DEVICE)
>>> +#define S390_PCI_FACILITY_GET_CLASS(obj) \
>>> +    OBJECT_GET_CLASS(S390PCIFacilityClass, (obj), TYPE_S390_PCI_FACILITY)
>>> +
>>>   typedef struct SeiContainer {
>>>       QTAILQ_ENTRY(SeiContainer) link;
>>>       uint32_t fid;
>>> @@ -214,12 +229,16 @@ typedef struct S390MsixInfo {
>>>   } S390MsixInfo;
>>>
>>>   typedef struct S390PCIBusDevice {
>>> +    DeviceState qdev;
>>>       PCIDevice *pdev;
>>>       bool configured;
>>> +    bool is_unplugged;
>>>       bool error_state;
>>>       bool lgstg_blocked;
>>>       uint32_t fh;
>>>       uint32_t fid;
>>> +    uint32_t uid;
>>> +    char *pci_id;
>>>       uint64_t g_iota;
>>>       uint64_t pba;
>>>       uint64_t pal;
>>> @@ -229,21 +248,42 @@ typedef struct S390PCIBusDevice {
>>>       uint8_t sum;
>>>       S390MsixInfo msix;
>>>       AdapterRoutes routes;
>>> -    AddressSpace as;
>>> -    MemoryRegion mr;
>>> +    QLIST_ENTRY(S390PCIDevice) entry;
>>>   } S390PCIBusDevice;
>>>
>>> +typedef struct S390PCIDeviceConn {
>>> +    S390PCIBusDevice *zpci;
>>> +    AddressSpace iommu_as;
>>> +    MemoryRegion iommu_mr;
>>> +} S390PCIDeviceConn;
>>> +
>>>   typedef struct S390pciState {
>>>       PCIHostState parent_obj;
>>> -    S390PCIBusDevice pbdev[PCI_SLOT_MAX];
>>> +    S390PCIDeviceConn conn[PCI_SLOT_MAX];
>>>       AddressSpace msix_notify_as;
>>>       MemoryRegion msix_notify_mr;
>>> -    QTAILQ_HEAD(, SeiContainer) pending_sei;
>>>   } S390pciState;
>>>
>>> +typedef struct S390PCIFacBus {
>>> +    BusState qbus;
>>> +} S390PCIFacBus;
>>> +
>>> +typedef struct S390PCIFacility {
>>> +    SysBusDevice parent_obj;
>>> +    S390PCIFacBus *fbus;
>>> +    QTAILQ_HEAD(, SeiContainer) pending_sei;
>>> +} S390PCIFacility;
>>> +
>>> +typedef struct S390PCIFacilityClass {
>>> +    DeviceClass parent_class;
>>> +    int (*init)(S390PCIFacility *f);
>>> +} S390PCIFacilityClass;
>>> +
>>>   int chsc_sei_nt2_get_event(void *res);
>>>   int chsc_sei_nt2_have_event(void);
>>>   void s390_pci_sclp_configure(int configure, SCCB *sccb);
>>> +void s390_pci_device_enable(S390PCIBusDevice *zpci);
>>> +void s390_pci_device_disable(S390PCIBusDevice *zpci);
>>>   S390PCIBusDevice *s390_pci_find_dev_by_idx(uint32_t idx);
>>>   S390PCIBusDevice *s390_pci_find_dev_by_fh(uint32_t fh);
>>>   S390PCIBusDevice *s390_pci_find_dev_by_fid(uint32_t fid);
>>> diff --git a/hw/s390x/s390-pci-inst.c b/hw/s390x/s390-pci-inst.c
>>> index cac3d83..fc8e8e1 100644
>>> --- a/hw/s390x/s390-pci-inst.c
>>> +++ b/hw/s390x/s390-pci-inst.c
>>> @@ -208,12 +208,12 @@ int clp_service_call(S390CPU *cpu, uint8_t r2)
>>>
>>>           switch (reqsetpci->oc) {
>>>           case CLP_SET_ENABLE_PCI_FN:
>>> -            pbdev->fh = pbdev->fh | 1 << ENABLE_BIT_OFFSET;
>>> +            s390_pci_device_enable(pbdev);
>>>               stl_p(&ressetpci->fh, pbdev->fh);
>>>               stw_p(&ressetpci->hdr.rsp, CLP_RC_OK);
>>>               break;
>>>           case CLP_SET_DISABLE_PCI_FN:
>>> -            pbdev->fh = pbdev->fh & ~(1 << ENABLE_BIT_OFFSET);
>>> +            s390_pci_device_disable(pbdev);
>>>               pbdev->error_state = false;
>>>               pbdev->lgstg_blocked = false;
>>>               stl_p(&ressetpci->fh, pbdev->fh);
>>> diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c
>>> index 312b8ac..7ee5202 100644
>>> --- a/hw/s390x/s390-virtio-ccw.c
>>> +++ b/hw/s390x/s390-virtio-ccw.c
>>> @@ -177,8 +177,8 @@ static void ccw_init(MachineState *machine)
>>>                         machine->initrd_filename, "s390-ccw.img", true);
>>>       s390_flic_init();
>>>
>>> -    dev = qdev_create(NULL, TYPE_S390_PCI_HOST_BRIDGE);
>>> -    object_property_add_child(qdev_get_machine(), TYPE_S390_PCI_HOST_BRIDGE,
>>> +    dev = qdev_create(NULL, TYPE_S390_PCI_FACILITY);
>>> +    object_property_add_child(qdev_get_machine(), TYPE_S390_PCI_FACILITY,
>>>                                 OBJECT(dev), NULL);
>>>       qdev_init_nofail(dev);
>>>
>>
>>
> 
> 
> 

Re: [Qemu-devel] [PATCH v1 1/1] S390 pci infrastructure modeling

[Hong Bo Li]

On 5/20/2015 15:47, Paolo Bonzini wrote:
> On 12/05/2015 11:07, Cornelia Huck wrote:
>>>> This patch contains the actual interesting changes.
>>>> usage example:
>>>> -device s390-pcihost
>>>> -device vfio-pci,host=0000:00:00.0,id=vpci1
>>>> -device zpci,fid=2,uid=5,pci_id=vpci1,id=zpci1
> One possible alternative could be making each zpci device expose a PCI
> bus, as if it were a PCI bridge.  The actual PCI device is then hung
> below the zpci device:
>
>    -device s390-pcihost,id=s390pci
>    -device zpci,fid=2,uid=5,id=zpci1,bus=s390pci.0
>    -device vfio-pci,host=00:00.0,id=vpci1,bus=zpci1.0
>
> Would this make sense from an s390 perspective?
>
> Paolo

This will require the hotplug support for s3900-pcihost & zpci.
It is similar as Frank's second proposal:
"s390x/pci: rework pci infrastructure modeling" on March 10.

-device s390-pcihost,fid=16,uid=2216,id=s390pci1
-device vfio-pci,id=vpci1,bus=s390pci1.0
-device s390-pcihost,fid=17,uid=2217,id=s390pci2
-device vfio-pci,id=vpci2,bus=s390pci2.0

That proposal stores s390 specific information in s390-pcihost.
Michael doesn't like the host bridge hotplug support code.

  

>>>> The first line will create a s390 pci host bridge
>>>> and init the root bus. The second line will create
>>>> a standard vfio pci device, and attach it to the
>>>> root bus. These are similiar to the standard process
>>>> to define a pci device on other platform.
>>>>
>>>> The third line will create a s390 pci device to
>>>> store s390 specific information, and references
>>>> the corresponding vfio pci device via device id.
>>>> We create a s390 pci facility bus to hold all the
>>>> zpci devices.
>>>>
>>>> Signed-off-by: Hong Bo Li <lihbbj@linux.vnet.ibm.com>
>>>> ---
>>>>    hw/s390x/s390-pci-bus.c    | 317 +++++++++++++++++++++++++++++++++------------
>>>>    hw/s390x/s390-pci-bus.h    |  48 ++++++-
>>>>    hw/s390x/s390-pci-inst.c   |   4 +-
>>>>    hw/s390x/s390-virtio-ccw.c |   4 +-
>>>>    4 files changed, 285 insertions(+), 88 deletions(-)
>> This looks fine from my s390 perspective, but I'd love to see some
>> feedback from PCI experts on this. Does modelling our s390 oddities in
>> this way sit fine with the general PCI infrastructure?
>>
>>>> diff --git a/hw/s390x/s390-pci-bus.c b/hw/s390x/s390-pci-bus.c
>>>> index 3c086f6..c81093e 100644
>>>> --- a/hw/s390x/s390-pci-bus.c
>>>> +++ b/hw/s390x/s390-pci-bus.c
>>>> @@ -32,8 +32,8 @@ int chsc_sei_nt2_get_event(void *res)
>>>>        PciCcdfErr *eccdf;
>>>>        int rc = 1;
>>>>        SeiContainer *sei_cont;
>>>> -    S390pciState *s = S390_PCI_HOST_BRIDGE(
>>>> -        object_resolve_path(TYPE_S390_PCI_HOST_BRIDGE, NULL));
>>>> +    S390PCIFacility *s = S390_PCI_FACILITY(
>>>> +        object_resolve_path(TYPE_S390_PCI_FACILITY, NULL));
>>>>
>>>>        if (!s) {
>>>>            return rc;
>>>> @@ -72,8 +72,8 @@ int chsc_sei_nt2_get_event(void *res)
>>>>
>>>>    int chsc_sei_nt2_have_event(void)
>>>>    {
>>>> -    S390pciState *s = S390_PCI_HOST_BRIDGE(
>>>> -        object_resolve_path(TYPE_S390_PCI_HOST_BRIDGE, NULL));
>>>> +    S390PCIFacility *s = S390_PCI_FACILITY(
>>>> +        object_resolve_path(TYPE_S390_PCI_FACILITY, NULL));
>>>>
>>>>        if (!s) {
>>>>            return 0;
>>>> @@ -82,20 +82,32 @@ int chsc_sei_nt2_have_event(void)
>>>>        return !QTAILQ_EMPTY(&s->pending_sei);
>>>>    }
>>>>
>>>> +void s390_pci_device_enable(S390PCIBusDevice *zpci)
>>>> +{
>>>> +    zpci->fh = zpci->fh | 1 << ENABLE_BIT_OFFSET;
>>>> +}
>>>> +
>>>> +void s390_pci_device_disable(S390PCIBusDevice *zpci)
>>>> +{
>>>> +    zpci->fh = zpci->fh & ~(1 << ENABLE_BIT_OFFSET);
>>>> +    if (zpci->is_unplugged)
>>>> +        object_unparent(OBJECT(zpci));
>>>> +}
>>>> +
>>>>    S390PCIBusDevice *s390_pci_find_dev_by_fid(uint32_t fid)
>>>>    {
>>>>        S390PCIBusDevice *pbdev;
>>>> -    int i;
>>>> -    S390pciState *s = S390_PCI_HOST_BRIDGE(
>>>> -        object_resolve_path(TYPE_S390_PCI_HOST_BRIDGE, NULL));
>>>> +    BusChild *kid;
>>>> +    S390PCIFacility *s = S390_PCI_FACILITY(
>>>> +        object_resolve_path(TYPE_S390_PCI_FACILITY, NULL));
>>>>
>>>>        if (!s) {
>>>>            return NULL;
>>>>        }
>>>>
>>>> -    for (i = 0; i < PCI_SLOT_MAX; i++) {
>>>> -        pbdev = &s->pbdev[i];
>>>> -        if ((pbdev->fh != 0) && (pbdev->fid == fid)) {
>>>> +    QTAILQ_FOREACH(kid, &s->fbus->qbus.children, sibling) {
>>>> +        pbdev = (S390PCIBusDevice *)kid->child;
>>>> +        if (pbdev->fid == fid) {
>>>>                return pbdev;
>>>>            }
>>>>        }
>>>> @@ -126,39 +138,24 @@ void s390_pci_sclp_configure(int configure, SCCB *sccb)
>>>>        return;
>>>>    }
>>>>
>>>> -static uint32_t s390_pci_get_pfid(PCIDevice *pdev)
>>>> -{
>>>> -    return PCI_SLOT(pdev->devfn);
>>>> -}
>>>> -
>>>> -static uint32_t s390_pci_get_pfh(PCIDevice *pdev)
>>>> -{
>>>> -    return PCI_SLOT(pdev->devfn) | FH_VIRT;
>>>> -}
>>>> -
>>>>    S390PCIBusDevice *s390_pci_find_dev_by_idx(uint32_t idx)
>>>>    {
>>>>        S390PCIBusDevice *pbdev;
>>>> -    int i;
>>>> -    int j = 0;
>>>> -    S390pciState *s = S390_PCI_HOST_BRIDGE(
>>>> -        object_resolve_path(TYPE_S390_PCI_HOST_BRIDGE, NULL));
>>>> +    BusChild *kid;
>>>> +    int i = 0;
>>>> +    S390PCIFacility *s = S390_PCI_FACILITY(
>>>> +        object_resolve_path(TYPE_S390_PCI_FACILITY, NULL));
>>>>
>>>>        if (!s) {
>>>>            return NULL;
>>>>        }
>>>>
>>>> -    for (i = 0; i < PCI_SLOT_MAX; i++) {
>>>> -        pbdev = &s->pbdev[i];
>>>> -
>>>> -        if (pbdev->fh == 0) {
>>>> -            continue;
>>>> -        }
>>>> -
>>>> -        if (j == idx) {
>>>> +    QTAILQ_FOREACH(kid, &s->fbus->qbus.children, sibling) {
>>>> +        pbdev = (S390PCIBusDevice *)kid->child;
>>>> +        if (i == idx) {
>>>>                return pbdev;
>>>>            }
>>>> -        j++;
>>>> +        i++;
>>>>        }
>>>>
>>>>        return NULL;
>>>> @@ -167,16 +164,17 @@ S390PCIBusDevice *s390_pci_find_dev_by_idx(uint32_t idx)
>>>>    S390PCIBusDevice *s390_pci_find_dev_by_fh(uint32_t fh)
>>>>    {
>>>>        S390PCIBusDevice *pbdev;
>>>> -    int i;
>>>> -    S390pciState *s = S390_PCI_HOST_BRIDGE(
>>>> -        object_resolve_path(TYPE_S390_PCI_HOST_BRIDGE, NULL));
>>>> +    BusChild *kid;
>>>> +    S390PCIFacility *s = S390_PCI_FACILITY(
>>>> +        object_resolve_path(TYPE_S390_PCI_FACILITY, NULL));
>>>> +
>>>>
>>>>        if (!s || !fh) {
>>>>            return NULL;
>>>>        }
>>>>
>>>> -    for (i = 0; i < PCI_SLOT_MAX; i++) {
>>>> -        pbdev = &s->pbdev[i];
>>>> +    QTAILQ_FOREACH(kid, &s->fbus->qbus.children, sibling) {
>>>> +        pbdev = (S390PCIBusDevice *)kid->child;
>>>>            if (pbdev->fh == fh) {
>>>>                return pbdev;
>>>>            }
>>>> @@ -185,12 +183,33 @@ S390PCIBusDevice *s390_pci_find_dev_by_fh(uint32_t fh)
>>>>        return NULL;
>>>>    }
>>>>
>>>> +static S390PCIBusDevice *s390_pci_find_dev_by_pdev(PCIDevice *pdev)
>>>> +{
>>>> +    S390PCIBusDevice *pbdev;
>>>> +    BusChild *kid;
>>>> +    S390PCIFacility *s = S390_PCI_FACILITY(
>>>> +        object_resolve_path(TYPE_S390_PCI_FACILITY, NULL));
>>>> +
>>>> +    if (!s || !pdev) {
>>>> +         return NULL;
>>>> +     }
>>>> +
>>>> +    QTAILQ_FOREACH(kid, &s->fbus->qbus.children, sibling) {
>>>> +        pbdev = (S390PCIBusDevice *)kid->child;
>>>> +        if (pbdev->pdev == pdev) {
>>>> +            return pbdev;
>>>> +        }
>>>> +    }
>>>> +
>>>> +    return NULL;
>>>> +}
>>>> +
>>>>    static void s390_pci_generate_event(uint8_t cc, uint16_t pec, uint32_t fh,
>>>>                                        uint32_t fid, uint64_t faddr, uint32_t e)
>>>>    {
>>>>        SeiContainer *sei_cont;
>>>> -    S390pciState *s = S390_PCI_HOST_BRIDGE(
>>>> -        object_resolve_path(TYPE_S390_PCI_HOST_BRIDGE, NULL));
>>>> +    S390PCIFacility *s = S390_PCI_FACILITY(
>>>> +        object_resolve_path(TYPE_S390_PCI_FACILITY, NULL));
>>>>
>>>>        if (!s) {
>>>>            return;
>>>> @@ -305,7 +324,9 @@ static IOMMUTLBEntry s390_translate_iommu(MemoryRegion *iommu, hwaddr addr,
>>>>    {
>>>>        uint64_t pte;
>>>>        uint32_t flags;
>>>> -    S390PCIBusDevice *pbdev = container_of(iommu, S390PCIBusDevice, mr);
>>>> +    S390PCIDeviceConn *conn = container_of(iommu, S390PCIDeviceConn,
>>>> +                                           iommu_mr);
>>>> +    S390PCIBusDevice *pbdev = conn->zpci;
>>>>        S390pciState *s = S390_PCI_HOST_BRIDGE(pci_device_root_bus(pbdev->pdev)
>>>>                                               ->qbus.parent);
>>>>        IOMMUTLBEntry ret = {
>>>> @@ -316,8 +337,14 @@ static IOMMUTLBEntry s390_translate_iommu(MemoryRegion *iommu, hwaddr addr,
>>>>            .perm = IOMMU_NONE,
>>>>        };
>>>>
>>>> +    if (!pbdev) {
>>>> +        return ret;
>>>> +    }
>>>> +
>>>>        DPRINTF("iommu trans addr 0x%" PRIx64 "\n", addr);
>>>>
>>>> +    s = S390_PCI_HOST_BRIDGE(pci_device_root_bus(pbdev->pdev)->qbus.parent);
>>>> +
>>>>        /* s390 does not have an APIC mapped to main storage so we use
>>>>         * a separate AddressSpace only for msix notifications
>>>>         */
>>>> @@ -379,7 +406,7 @@ static AddressSpace *s390_pci_dma_iommu(PCIBus *bus, void *opaque, int devfn)
>>>>    {
>>>>        S390pciState *s = opaque;
>>>>
>>>> -    return &s->pbdev[PCI_SLOT(devfn)].as;
>>>> +    return &s->conn[PCI_SLOT(devfn)].iommu_as;
>>>>    }
>>>>
>>>>    static uint8_t set_ind_atomic(uint64_t ind_loc, uint8_t to_be_set)
>>>> @@ -452,9 +479,10 @@ static void s390_pcihost_init_as(S390pciState *s)
>>>>        int i;
>>>>
>>>>        for (i = 0; i < PCI_SLOT_MAX; i++) {
>>>> -        memory_region_init_iommu(&s->pbdev[i].mr, OBJECT(s),
>>>> +        memory_region_init_iommu(&s->conn[i].iommu_mr, OBJECT(s),
>>>>                                     &s390_iommu_ops, "iommu-s390", UINT64_MAX);
>>>> -        address_space_init(&s->pbdev[i].as, &s->pbdev[i].mr, "iommu-pci");
>>>> +        address_space_init(&s->conn[i].iommu_as, &s->conn[i].iommu_mr,
>>>> +                           "iommu-pci");
>>>>        }
>>>>
>>>>        memory_region_init_io(&s->msix_notify_mr, OBJECT(s),
>>>> @@ -481,7 +509,7 @@ static int s390_pcihost_init(SysBusDevice *dev)
>>>>        bus = BUS(b);
>>>>        qbus_set_hotplug_handler(bus, DEVICE(dev), NULL);
>>>>        phb->bus = b;
>>>> -    QTAILQ_INIT(&s->pending_sei);
>>>> +
>>>>        return 0;
>>>>    }
>>>>
>>>> @@ -513,29 +541,12 @@ static int s390_pcihost_setup_msix(S390PCIBusDevice *pbdev)
>>>>        return 0;
>>>>    }
>>>>
>>>> +/* this function is useless, just keep it to make the patch more
>>>> +   readable. Without this function, the diff will mix these hotplug
>>>> +   functions together. will be removed in future */
>>>>    static void s390_pcihost_hot_plug(HotplugHandler *hotplug_dev,
>>>>                                      DeviceState *dev, Error **errp)
>>>>    {
>>>> -    PCIDevice *pci_dev = PCI_DEVICE(dev);
>>>> -    S390PCIBusDevice *pbdev;
>>>> -    S390pciState *s = S390_PCI_HOST_BRIDGE(pci_device_root_bus(pci_dev)
>>>> -                                           ->qbus.parent);
>>>> -
>>>> -    pbdev = &s->pbdev[PCI_SLOT(pci_dev->devfn)];
>>>> -
>>>> -    pbdev->fid = s390_pci_get_pfid(pci_dev);
>>>> -    pbdev->pdev = pci_dev;
>>>> -    pbdev->configured = true;
>>>> -    pbdev->fh = s390_pci_get_pfh(pci_dev);
>>>> -
>>>> -    s390_pcihost_setup_msix(pbdev);
>>>> -
>>>> -    if (dev->hotplugged) {
>>>> -        s390_pci_generate_plug_event(HP_EVENT_RESERVED_TO_STANDBY,
>>>> -                                     pbdev->fh, pbdev->fid);
>>>> -        s390_pci_generate_plug_event(HP_EVENT_TO_CONFIGURED,
>>>> -                                     pbdev->fh, pbdev->fid);
>>>> -    }
>>>>        return;
>>>>    }
>>>>
>>>> @@ -543,31 +554,30 @@ static void s390_pcihost_hot_unplug(HotplugHandler *hotplug_dev,
>>>>                                        DeviceState *dev, Error **errp)
>>>>    {
>>>>        PCIDevice *pci_dev = PCI_DEVICE(dev);
>>>> -    S390pciState *s = S390_PCI_HOST_BRIDGE(pci_device_root_bus(pci_dev)
>>>> -                                           ->qbus.parent);
>>>> -    S390PCIBusDevice *pbdev = &s->pbdev[PCI_SLOT(pci_dev->devfn)];
>>>> -
>>>> -    if (pbdev->configured) {
>>>> -        pbdev->configured = false;
>>>> -        s390_pci_generate_plug_event(HP_EVENT_CONFIGURED_TO_STBRES,
>>>> -                                     pbdev->fh, pbdev->fid);
>>>> +    S390PCIBusDevice *pbdev;
>>>> +    HotplugHandler *hotplug_ctrl;
>>>> +    S390PCIFacility *f = S390_PCI_FACILITY(
>>>> +        object_resolve_path(TYPE_S390_PCI_FACILITY, NULL));
>>>> +    S390PCIFacilityClass *k = S390_PCI_FACILITY_GET_CLASS(f);
>>>> +    HotplugHandlerClass *hdc = HOTPLUG_HANDLER_CLASS(k);
>>>> +
>>>> +    /* unplug corresponding zpci device */
>>>> +    pbdev = s390_pci_find_dev_by_pdev(pci_dev);
>>>> +    if (pbdev) {
>>>> +        hotplug_ctrl = pbdev->qdev.parent_bus->hotplug_handler;
>>>> +        if (hdc->unplug_request) {
>>>> +            hdc->unplug_request(hotplug_ctrl, &pbdev->qdev, errp);
>>>> +        }
>>>>        }
>>>>
>>>> -    s390_pci_generate_plug_event(HP_EVENT_STANDBY_TO_RESERVED,
>>>> -                                 pbdev->fh, pbdev->fid);
>>>> -    pbdev->fh = 0;
>>>> -    pbdev->fid = 0;
>>>> -    pbdev->pdev = NULL;
>>>>        object_unparent(OBJECT(pci_dev));
>>>>    }
>>>>
>>>>    static void s390_pcihost_class_init(ObjectClass *klass, void *data)
>>>>    {
>>>>        SysBusDeviceClass *k = SYS_BUS_DEVICE_CLASS(klass);
>>>> -    DeviceClass *dc = DEVICE_CLASS(klass);
>>>>        HotplugHandlerClass *hc = HOTPLUG_HANDLER_CLASS(klass);
>>>>
>>>> -    dc->cannot_instantiate_with_device_add_yet = true;
>>>>        k->init = s390_pcihost_init;
>>>>        hc->plug = s390_pcihost_hot_plug;
>>>>        hc->unplug = s390_pcihost_hot_unplug;
>>>> @@ -585,9 +595,156 @@ static const TypeInfo s390_pcihost_info = {
>>>>        }
>>>>    };
>>>>
>>>> +static void s390_pci_device_hot_plug(HotplugHandler *hotplug_dev,
>>>> +                                     DeviceState *dev, Error **errp)
>>>> +{
>>>> +    S390PCIBusDevice *zpci = S390_PCI_DEVICE(dev);
>>>> +
>>>> +    zpci->configured = true;
>>>> +
>>>> +    if (dev->hotplugged) {
>>>> +        s390_pci_generate_plug_event(HP_EVENT_RESERVED_TO_STANDBY,
>>>> +                                     zpci->fh, zpci->fid);
>>>> +        s390_pci_generate_plug_event(HP_EVENT_TO_CONFIGURED,
>>>> +                                     zpci->fh, zpci->fid);
>>>> +    }
>>>> +}
>>>> +
>>>> +static void s390_pci_device_hot_unplug_request(HotplugHandler *hotplug_dev,
>>>> +                                       DeviceState *dev, Error **errp)
>>>> +{
>>>> +    S390PCIBusDevice *zpci = S390_PCI_DEVICE(dev);
>>>> +
>>>> +    if (zpci->configured) {
>>>> +        zpci->configured = false;
>>>> +        s390_pci_generate_plug_event(HP_EVENT_CONFIGURED_TO_STBRES,
>>>> +                                     zpci->fh, zpci->fid);
>>>> +    }
>>>> +
>>>> +    s390_pci_generate_plug_event(HP_EVENT_STANDBY_TO_RESERVED,
>>>> +                                 zpci->fh, zpci->fid);
>>>> +
>>>> +    zpci->is_unplugged = true;
>>>> +}
>>>> +
>>>> +static const TypeInfo s390_pci_fac_bus_info = {
>>>> +    .name = TYPE_S390_PCI_FAC_BUS,
>>>> +    .parent = TYPE_BUS,
>>>> +    .instance_size = sizeof(S390PCIFacBus),
>>>> +};
>>>> +
>>>> +static int s390_pci_facility_init(S390PCIFacility *f)
>>>> +{
>>>> +    DeviceState *dev = DEVICE(f);
>>>> +
>>>> +    QTAILQ_INIT(&f->pending_sei);
>>>> +    msi_supported = true;
>>>> +    f->fbus = S390_PCI_FAC_BUS(qbus_create(TYPE_S390_PCI_FAC_BUS, dev, NULL));
>>>> +    qbus_set_hotplug_handler(BUS(&f->fbus->qbus), DEVICE(dev), NULL);
>>>> +
>>>> +    return 0;
>>>> +}
>>>> +
>>>> +static void s390_pci_facility_class_init(ObjectClass *klass, void *data)
>>>> +{
>>>> +    S390PCIFacilityClass *k = S390_PCI_FACILITY_CLASS(klass);
>>>> +    HotplugHandlerClass *hc = HOTPLUG_HANDLER_CLASS(k);
>>>> +
>>>> +    k->init = s390_pci_facility_init;
>>>> +    hc->plug = s390_pci_device_hot_plug;
>>>> +    hc->unplug_request = s390_pci_device_hot_unplug_request;
>>>> +}
>>>> +
>>>> +static const TypeInfo s390_pci_facility_info = {
>>>> +    .name          = TYPE_S390_PCI_FACILITY,
>>>> +    .parent        = TYPE_SYS_BUS_DEVICE,
>>>> +    .instance_size = sizeof(S390PCIFacility),
>>>> +    .class_init    = s390_pci_facility_class_init,
>>>> +    .class_size    = sizeof(S390PCIFacilityClass),
>>>> +    .interfaces = (InterfaceInfo[]) {
>>>> +        { TYPE_HOTPLUG_HANDLER },
>>>> +        { }
>>>> +    }
>>>> +};
>>>> +
>>>> +static void s390_pci_device_realize(DeviceState *dev, Error **errp)
>>>> +{
>>>> +    S390PCIBusDevice *zpci = S390_PCI_DEVICE(dev);
>>>> +    S390PCIBusDevice *tmp;
>>>> +    S390pciState *s;
>>>> +    BusChild *kid;
>>>> +    PCIDevice *pdev;
>>>> +    int ret;
>>>> +    S390PCIFacility *f = S390_PCI_FACILITY(
>>>> +        object_resolve_path(TYPE_S390_PCI_FACILITY, NULL));
>>>> +
>>>> +    ret = pci_qdev_find_device(zpci->pci_id, &pdev);
>>>> +    if (ret < 0) {
>>>> +        error_setg(errp, "vfio pci device %s not found", zpci->pci_id);
>>>> +        return;
>>>> +    }
>>>> +
>>>> +    QTAILQ_FOREACH(kid, &f->fbus->qbus.children, sibling) {
>>>> +        tmp = (S390PCIBusDevice *)kid->child;
>>>> +        if (tmp == zpci) {
>>>> +            continue;
>>>> +        }
>>>> +
>>>> +        if (tmp->fid == zpci->fid || tmp->uid == zpci->uid ||
>>>> +            !strcmp(tmp->pci_id, zpci->pci_id)) {
>>>> +            error_setg(errp, "zpci needs unique fid, uid and pci_id");
>>>> +            return;
>>>> +        }
>>>> +    }
>>>> +
>>>> +    s = S390_PCI_HOST_BRIDGE(pci_device_root_bus(pdev)->qbus.parent);
>>>> +    s->conn[PCI_SLOT(pdev->devfn)].zpci = zpci;
>>>> +
>>>> +    zpci->pdev = pdev;
>>>> +    zpci->fh = zpci->fid | FH_VIRT;
>>>> +    s390_pcihost_setup_msix(zpci);
>>>> +}
>>>> +
>>>> +static void s390_pci_device_unrealize(DeviceState *dev, Error **errp)
>>>> +{
>>>> +    S390PCIBusDevice *zpci = S390_PCI_DEVICE(dev);
>>>> +
>>>> +    zpci->fh = 0;
>>>> +    zpci->fid = 0;
>>>> +    zpci->pdev = NULL;
>>>> +}
>>>> +
>>>> +static Property s390_pci_device_properties[] = {
>>>> +    DEFINE_PROP_UINT32("fid", S390PCIBusDevice, fid, 0),
>>>> +    DEFINE_PROP_UINT32("uid", S390PCIBusDevice, uid, 0),
>>>> +    DEFINE_PROP_STRING("pci_id", S390PCIBusDevice, pci_id),
>>>> +    DEFINE_PROP_END_OF_LIST(),
>>>> +};
>>>> +
>>>> +static void s390_pci_device_class_init(ObjectClass *klass, void *data)
>>>> +{
>>>> +    DeviceClass *dc = DEVICE_CLASS(klass);
>>>> +
>>>> +    dc->desc = "s390 pci device";
>>>> +    dc->bus_type = TYPE_S390_PCI_FAC_BUS;
>>>> +    dc->realize = s390_pci_device_realize;
>>>> +    dc->unrealize = s390_pci_device_unrealize;
>>>> +    dc->props = s390_pci_device_properties;
>>>> +}
>>>> +
>>>> +static const TypeInfo s390_pci_device_type_info = {
>>>> +    .name = TYPE_S390_PCI_DEVICE,
>>>> +    .parent = TYPE_DEVICE,
>>>> +    .instance_size = sizeof(S390PCIBusDevice),
>>>> +    .class_init = s390_pci_device_class_init,
>>>> +};
>>>> +
>>>>    static void s390_pci_register_types(void)
>>>>    {
>>>>        type_register_static(&s390_pcihost_info);
>>>> +    type_register_static(&s390_pci_facility_info);
>>>> +    type_register_static(&s390_pci_fac_bus_info);
>>>> +    type_register_static(&s390_pci_device_type_info);
>>>>    }
>>>>
>>>>    type_init(s390_pci_register_types)
>>>> diff --git a/hw/s390x/s390-pci-bus.h b/hw/s390x/s390-pci-bus.h
>>>> index 464a92e..5bf3913 100644
>>>> --- a/hw/s390x/s390-pci-bus.h
>>>> +++ b/hw/s390x/s390-pci-bus.h
>>>> @@ -149,6 +149,21 @@ enum ZpciIoatDtype {
>>>>    #define ZPCI_TABLE_VALID_MASK           0x20
>>>>    #define ZPCI_TABLE_PROT_MASK            0x200
>>>>
>>>> +#define TYPE_S390_PCI_FACILITY "s390-pci-facility"
>>>> +#define TYPE_S390_PCI_FAC_BUS "s390-pci-fac-bus"
>>>> +#define TYPE_S390_PCI_DEVICE "zpci"
>>>> +
>>>> +#define S390_PCI_FACILITY(obj) \
>>>> +    OBJECT_CHECK(S390PCIFacility, (obj), TYPE_S390_PCI_FACILITY)
>>>> +#define S390_PCI_FAC_BUS(obj) \
>>>> +    OBJECT_CHECK(S390PCIFacBus, (obj), TYPE_S390_PCI_FAC_BUS)
>>>> +#define S390_PCI_FACILITY_CLASS(klass) \
>>>> +    OBJECT_CLASS_CHECK(S390PCIFacilityClass, (klass), TYPE_S390_PCI_FACILITY)
>>>> +#define S390_PCI_DEVICE(obj) \
>>>> +    OBJECT_CHECK(S390PCIBusDevice, (obj), TYPE_S390_PCI_DEVICE)
>>>> +#define S390_PCI_FACILITY_GET_CLASS(obj) \
>>>> +    OBJECT_GET_CLASS(S390PCIFacilityClass, (obj), TYPE_S390_PCI_FACILITY)
>>>> +
>>>>    typedef struct SeiContainer {
>>>>        QTAILQ_ENTRY(SeiContainer) link;
>>>>        uint32_t fid;
>>>> @@ -214,12 +229,16 @@ typedef struct S390MsixInfo {
>>>>    } S390MsixInfo;
>>>>
>>>>    typedef struct S390PCIBusDevice {
>>>> +    DeviceState qdev;
>>>>        PCIDevice *pdev;
>>>>        bool configured;
>>>> +    bool is_unplugged;
>>>>        bool error_state;
>>>>        bool lgstg_blocked;
>>>>        uint32_t fh;
>>>>        uint32_t fid;
>>>> +    uint32_t uid;
>>>> +    char *pci_id;
>>>>        uint64_t g_iota;
>>>>        uint64_t pba;
>>>>        uint64_t pal;
>>>> @@ -229,21 +248,42 @@ typedef struct S390PCIBusDevice {
>>>>        uint8_t sum;
>>>>        S390MsixInfo msix;
>>>>        AdapterRoutes routes;
>>>> -    AddressSpace as;
>>>> -    MemoryRegion mr;
>>>> +    QLIST_ENTRY(S390PCIDevice) entry;
>>>>    } S390PCIBusDevice;
>>>>
>>>> +typedef struct S390PCIDeviceConn {
>>>> +    S390PCIBusDevice *zpci;
>>>> +    AddressSpace iommu_as;
>>>> +    MemoryRegion iommu_mr;
>>>> +} S390PCIDeviceConn;
>>>> +
>>>>    typedef struct S390pciState {
>>>>        PCIHostState parent_obj;
>>>> -    S390PCIBusDevice pbdev[PCI_SLOT_MAX];
>>>> +    S390PCIDeviceConn conn[PCI_SLOT_MAX];
>>>>        AddressSpace msix_notify_as;
>>>>        MemoryRegion msix_notify_mr;
>>>> -    QTAILQ_HEAD(, SeiContainer) pending_sei;
>>>>    } S390pciState;
>>>>
>>>> +typedef struct S390PCIFacBus {
>>>> +    BusState qbus;
>>>> +} S390PCIFacBus;
>>>> +
>>>> +typedef struct S390PCIFacility {
>>>> +    SysBusDevice parent_obj;
>>>> +    S390PCIFacBus *fbus;
>>>> +    QTAILQ_HEAD(, SeiContainer) pending_sei;
>>>> +} S390PCIFacility;
>>>> +
>>>> +typedef struct S390PCIFacilityClass {
>>>> +    DeviceClass parent_class;
>>>> +    int (*init)(S390PCIFacility *f);
>>>> +} S390PCIFacilityClass;
>>>> +
>>>>    int chsc_sei_nt2_get_event(void *res);
>>>>    int chsc_sei_nt2_have_event(void);
>>>>    void s390_pci_sclp_configure(int configure, SCCB *sccb);
>>>> +void s390_pci_device_enable(S390PCIBusDevice *zpci);
>>>> +void s390_pci_device_disable(S390PCIBusDevice *zpci);
>>>>    S390PCIBusDevice *s390_pci_find_dev_by_idx(uint32_t idx);
>>>>    S390PCIBusDevice *s390_pci_find_dev_by_fh(uint32_t fh);
>>>>    S390PCIBusDevice *s390_pci_find_dev_by_fid(uint32_t fid);
>>>> diff --git a/hw/s390x/s390-pci-inst.c b/hw/s390x/s390-pci-inst.c
>>>> index cac3d83..fc8e8e1 100644
>>>> --- a/hw/s390x/s390-pci-inst.c
>>>> +++ b/hw/s390x/s390-pci-inst.c
>>>> @@ -208,12 +208,12 @@ int clp_service_call(S390CPU *cpu, uint8_t r2)
>>>>
>>>>            switch (reqsetpci->oc) {
>>>>            case CLP_SET_ENABLE_PCI_FN:
>>>> -            pbdev->fh = pbdev->fh | 1 << ENABLE_BIT_OFFSET;
>>>> +            s390_pci_device_enable(pbdev);
>>>>                stl_p(&ressetpci->fh, pbdev->fh);
>>>>                stw_p(&ressetpci->hdr.rsp, CLP_RC_OK);
>>>>                break;
>>>>            case CLP_SET_DISABLE_PCI_FN:
>>>> -            pbdev->fh = pbdev->fh & ~(1 << ENABLE_BIT_OFFSET);
>>>> +            s390_pci_device_disable(pbdev);
>>>>                pbdev->error_state = false;
>>>>                pbdev->lgstg_blocked = false;
>>>>                stl_p(&ressetpci->fh, pbdev->fh);
>>>> diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c
>>>> index 312b8ac..7ee5202 100644
>>>> --- a/hw/s390x/s390-virtio-ccw.c
>>>> +++ b/hw/s390x/s390-virtio-ccw.c
>>>> @@ -177,8 +177,8 @@ static void ccw_init(MachineState *machine)
>>>>                          machine->initrd_filename, "s390-ccw.img", true);
>>>>        s390_flic_init();
>>>>
>>>> -    dev = qdev_create(NULL, TYPE_S390_PCI_HOST_BRIDGE);
>>>> -    object_property_add_child(qdev_get_machine(), TYPE_S390_PCI_HOST_BRIDGE,
>>>> +    dev = qdev_create(NULL, TYPE_S390_PCI_FACILITY);
>>>> +    object_property_add_child(qdev_get_machine(), TYPE_S390_PCI_FACILITY,
>>>>                                  OBJECT(dev), NULL);
>>>>        qdev_init_nofail(dev);
>>>>
>>>
>>
>>

Re: [Qemu-devel] [PATCH v1 1/1] S390 pci infrastructure modeling

[Paolo Bonzini]

On 15/04/2015 10:04, Hong Bo Li wrote:
> @@ -513,29 +541,12 @@ static int s390_pcihost_setup_msix(S390PCIBusDevice *pbdev)
>      return 0;
>  }
>  
> +/* this function is useless, just keep it to make the patch more
> +   readable. Without this function, the diff will mix these hotplug
> +   functions together. will be removed in future */

Try adding this to your .gitconfig:

	[diff]
	algorithm = patience

Paolo

[Qemu-devel] [PATCH v1 1/1] S390 pci infrastructure modeling

[Hong Bo Li]

This patch contains the actual interesting changes.
usage example:
-device s390-pcihost
-device vfio-pci,host=0000:00:00.0,id=vpci1
-device zpci,fid=2,uid=5,pci_id=vpci1,id=zpci1

The first line will create a s390 pci host bridge
and init the root bus. The second line will create
a standard vfio pci device, and attach it to the
root bus. These are similiar to the standard process
to define a pci device on other platform.

The third line will create a s390 pci device to
store s390 specific information, and references
the corresponding vfio pci device via device id.
We create a s390 pci facility bus to hold all the
zpci devices.

Signed-off-by: Hong Bo Li<lihbbj@linux.vnet.ibm.com>
---
  hw/s390x/s390-pci-bus.c    | 317 +++++++++++++++++++++++++++++++++------------
  hw/s390x/s390-pci-bus.h    |  48 ++++++-
  hw/s390x/s390-pci-inst.c   |   4 +-
  hw/s390x/s390-virtio-ccw.c |   4 +-
  4 files changed, 285 insertions(+), 88 deletions(-)

diff --git a/hw/s390x/s390-pci-bus.c b/hw/s390x/s390-pci-bus.c
index 3c086f6..c81093e 100644
--- a/hw/s390x/s390-pci-bus.c
+++ b/hw/s390x/s390-pci-bus.c
@@ -32,8 +32,8 @@ int chsc_sei_nt2_get_event(void *res)
      PciCcdfErr *eccdf;
      int rc = 1;
      SeiContainer *sei_cont;
-    S390pciState *s = S390_PCI_HOST_BRIDGE(
-        object_resolve_path(TYPE_S390_PCI_HOST_BRIDGE, NULL));
+    S390PCIFacility *s = S390_PCI_FACILITY(
+        object_resolve_path(TYPE_S390_PCI_FACILITY, NULL));

      if (!s) {
          return rc;
@@ -72,8 +72,8 @@ int chsc_sei_nt2_get_event(void *res)

  int chsc_sei_nt2_have_event(void)
  {
-    S390pciState *s = S390_PCI_HOST_BRIDGE(
-        object_resolve_path(TYPE_S390_PCI_HOST_BRIDGE, NULL));
+    S390PCIFacility *s = S390_PCI_FACILITY(
+        object_resolve_path(TYPE_S390_PCI_FACILITY, NULL));

      if (!s) {
          return 0;
@@ -82,20 +82,32 @@ int chsc_sei_nt2_have_event(void)
      return !QTAILQ_EMPTY(&s->pending_sei);
  }

+void s390_pci_device_enable(S390PCIBusDevice *zpci)
+{
+    zpci->fh = zpci->fh | 1 << ENABLE_BIT_OFFSET;
+}
+
+void s390_pci_device_disable(S390PCIBusDevice *zpci)
+{
+    zpci->fh = zpci->fh & ~(1 << ENABLE_BIT_OFFSET);
+    if (zpci->is_unplugged)
+        object_unparent(OBJECT(zpci));
+}
+
  S390PCIBusDevice *s390_pci_find_dev_by_fid(uint32_t fid)
  {
      S390PCIBusDevice *pbdev;
-    int i;
-    S390pciState *s = S390_PCI_HOST_BRIDGE(
-        object_resolve_path(TYPE_S390_PCI_HOST_BRIDGE, NULL));
+    BusChild *kid;
+    S390PCIFacility *s = S390_PCI_FACILITY(
+        object_resolve_path(TYPE_S390_PCI_FACILITY, NULL));

      if (!s) {
          return NULL;
      }

-    for (i = 0; i < PCI_SLOT_MAX; i++) {
-        pbdev = &s->pbdev[i];
-        if ((pbdev->fh != 0) && (pbdev->fid == fid)) {
+    QTAILQ_FOREACH(kid, &s->fbus->qbus.children, sibling) {
+        pbdev = (S390PCIBusDevice *)kid->child;
+        if (pbdev->fid == fid) {
              return pbdev;
          }
      }
@@ -126,39 +138,24 @@ void s390_pci_sclp_configure(int configure, SCCB *sccb)
      return;
  }

-static uint32_t s390_pci_get_pfid(PCIDevice *pdev)
-{
-    return PCI_SLOT(pdev->devfn);
-}
-
-static uint32_t s390_pci_get_pfh(PCIDevice *pdev)
-{
-    return PCI_SLOT(pdev->devfn) | FH_VIRT;
-}
-
  S390PCIBusDevice *s390_pci_find_dev_by_idx(uint32_t idx)
  {
      S390PCIBusDevice *pbdev;
-    int i;
-    int j = 0;
-    S390pciState *s = S390_PCI_HOST_BRIDGE(
-        object_resolve_path(TYPE_S390_PCI_HOST_BRIDGE, NULL));
+    BusChild *kid;
+    int i = 0;
+    S390PCIFacility *s = S390_PCI_FACILITY(
+        object_resolve_path(TYPE_S390_PCI_FACILITY, NULL));

      if (!s) {
          return NULL;
      }

-    for (i = 0; i < PCI_SLOT_MAX; i++) {
-        pbdev = &s->pbdev[i];
-
-        if (pbdev->fh == 0) {
-            continue;
-        }
-
-        if (j == idx) {
+    QTAILQ_FOREACH(kid, &s->fbus->qbus.children, sibling) {
+        pbdev = (S390PCIBusDevice *)kid->child;
+        if (i == idx) {
              return pbdev;
          }
-        j++;
+        i++;
      }

      return NULL;
@@ -167,16 +164,17 @@ S390PCIBusDevice *s390_pci_find_dev_by_idx(uint32_t idx)
  S390PCIBusDevice *s390_pci_find_dev_by_fh(uint32_t fh)
  {
      S390PCIBusDevice *pbdev;
-    int i;
-    S390pciState *s = S390_PCI_HOST_BRIDGE(
-        object_resolve_path(TYPE_S390_PCI_HOST_BRIDGE, NULL));
+    BusChild *kid;
+    S390PCIFacility *s = S390_PCI_FACILITY(
+        object_resolve_path(TYPE_S390_PCI_FACILITY, NULL));
+

      if (!s || !fh) {
          return NULL;
      }

-    for (i = 0; i < PCI_SLOT_MAX; i++) {
-        pbdev = &s->pbdev[i];
+    QTAILQ_FOREACH(kid, &s->fbus->qbus.children, sibling) {
+        pbdev = (S390PCIBusDevice *)kid->child;
          if (pbdev->fh == fh) {
              return pbdev;
          }
@@ -185,12 +183,33 @@ S390PCIBusDevice *s390_pci_find_dev_by_fh(uint32_t fh)
      return NULL;
  }

+static S390PCIBusDevice *s390_pci_find_dev_by_pdev(PCIDevice *pdev)
+{
+    S390PCIBusDevice *pbdev;
+    BusChild *kid;
+    S390PCIFacility *s = S390_PCI_FACILITY(
+        object_resolve_path(TYPE_S390_PCI_FACILITY, NULL));
+
+    if (!s || !pdev) {
+         return NULL;
+     }
+
+    QTAILQ_FOREACH(kid, &s->fbus->qbus.children, sibling) {
+        pbdev = (S390PCIBusDevice *)kid->child;
+        if (pbdev->pdev == pdev) {
+            return pbdev;
+        }
+    }
+
+    return NULL;
+}
+
  static void s390_pci_generate_event(uint8_t cc, uint16_t pec, uint32_t fh,
                                      uint32_t fid, uint64_t faddr, uint32_t e)
  {
      SeiContainer *sei_cont;
-    S390pciState *s = S390_PCI_HOST_BRIDGE(
-        object_resolve_path(TYPE_S390_PCI_HOST_BRIDGE, NULL));
+    S390PCIFacility *s = S390_PCI_FACILITY(
+        object_resolve_path(TYPE_S390_PCI_FACILITY, NULL));

      if (!s) {
          return;
@@ -305,7 +324,9 @@ static IOMMUTLBEntry s390_translate_iommu(MemoryRegion *iommu, hwaddr addr,
  {
      uint64_t pte;
      uint32_t flags;
-    S390PCIBusDevice *pbdev = container_of(iommu, S390PCIBusDevice, mr);
+    S390PCIDeviceConn *conn = container_of(iommu, S390PCIDeviceConn,
+                                           iommu_mr);
+    S390PCIBusDevice *pbdev = conn->zpci;
      S390pciState *s = S390_PCI_HOST_BRIDGE(pci_device_root_bus(pbdev->pdev)
                                             ->qbus.parent);
      IOMMUTLBEntry ret = {
@@ -316,8 +337,14 @@ static IOMMUTLBEntry s390_translate_iommu(MemoryRegion *iommu, hwaddr addr,
          .perm = IOMMU_NONE,
      };

+    if (!pbdev) {
+        return ret;
+    }
+
      DPRINTF("iommu trans addr 0x%" PRIx64 "\n", addr);

+    s = S390_PCI_HOST_BRIDGE(pci_device_root_bus(pbdev->pdev)->qbus.parent);
+
      /* s390 does not have an APIC mapped to main storage so we use
       * a separate AddressSpace only for msix notifications
       */
@@ -379,7 +406,7 @@ static AddressSpace *s390_pci_dma_iommu(PCIBus *bus, void *opaque, int devfn)
  {
      S390pciState *s = opaque;

-    return &s->pbdev[PCI_SLOT(devfn)].as;
+    return &s->conn[PCI_SLOT(devfn)].iommu_as;
  }

  static uint8_t set_ind_atomic(uint64_t ind_loc, uint8_t to_be_set)
@@ -452,9 +479,10 @@ static void s390_pcihost_init_as(S390pciState *s)
      int i;

      for (i = 0; i < PCI_SLOT_MAX; i++) {
-        memory_region_init_iommu(&s->pbdev[i].mr, OBJECT(s),
+        memory_region_init_iommu(&s->conn[i].iommu_mr, OBJECT(s),
                                   &s390_iommu_ops, "iommu-s390", UINT64_MAX);
-        address_space_init(&s->pbdev[i].as, &s->pbdev[i].mr, "iommu-pci");
+        address_space_init(&s->conn[i].iommu_as, &s->conn[i].iommu_mr,
+                           "iommu-pci");
      }

      memory_region_init_io(&s->msix_notify_mr, OBJECT(s),
@@ -481,7 +509,7 @@ static int s390_pcihost_init(SysBusDevice *dev)
      bus = BUS(b);
      qbus_set_hotplug_handler(bus, DEVICE(dev), NULL);
      phb->bus = b;
-    QTAILQ_INIT(&s->pending_sei);
+
      return 0;
  }

@@ -513,29 +541,12 @@ static int s390_pcihost_setup_msix(S390PCIBusDevice *pbdev)
      return 0;
  }

+/* this function is useless, just keep it to make the patch more
+   readable. Without this function, the diff will mix these hotplug
+   functions together. will be removed in future */
  static void s390_pcihost_hot_plug(HotplugHandler *hotplug_dev,
                                    DeviceState *dev, Error **errp)
  {
-    PCIDevice *pci_dev = PCI_DEVICE(dev);
-    S390PCIBusDevice *pbdev;
-    S390pciState *s = S390_PCI_HOST_BRIDGE(pci_device_root_bus(pci_dev)
-                                           ->qbus.parent);
-
-    pbdev = &s->pbdev[PCI_SLOT(pci_dev->devfn)];
-
-    pbdev->fid = s390_pci_get_pfid(pci_dev);
-    pbdev->pdev = pci_dev;
-    pbdev->configured = true;
-    pbdev->fh = s390_pci_get_pfh(pci_dev);
-
-    s390_pcihost_setup_msix(pbdev);
-
-    if (dev->hotplugged) {
-        s390_pci_generate_plug_event(HP_EVENT_RESERVED_TO_STANDBY,
-                                     pbdev->fh, pbdev->fid);
-        s390_pci_generate_plug_event(HP_EVENT_TO_CONFIGURED,
-                                     pbdev->fh, pbdev->fid);
-    }
      return;
  }

@@ -543,31 +554,30 @@ static void s390_pcihost_hot_unplug(HotplugHandler *hotplug_dev,
                                      DeviceState *dev, Error **errp)
  {
      PCIDevice *pci_dev = PCI_DEVICE(dev);
-    S390pciState *s = S390_PCI_HOST_BRIDGE(pci_device_root_bus(pci_dev)
-                                           ->qbus.parent);
-    S390PCIBusDevice *pbdev = &s->pbdev[PCI_SLOT(pci_dev->devfn)];
-
-    if (pbdev->configured) {
-        pbdev->configured = false;
-        s390_pci_generate_plug_event(HP_EVENT_CONFIGURED_TO_STBRES,
-                                     pbdev->fh, pbdev->fid);
+    S390PCIBusDevice *pbdev;
+    HotplugHandler *hotplug_ctrl;
+    S390PCIFacility *f = S390_PCI_FACILITY(
+        object_resolve_path(TYPE_S390_PCI_FACILITY, NULL));
+    S390PCIFacilityClass *k = S390_PCI_FACILITY_GET_CLASS(f);
+    HotplugHandlerClass *hdc = HOTPLUG_HANDLER_CLASS(k);
+
+    /* unplug corresponding zpci device */
+    pbdev = s390_pci_find_dev_by_pdev(pci_dev);
+    if (pbdev) {
+        hotplug_ctrl = pbdev->qdev.parent_bus->hotplug_handler;
+        if (hdc->unplug_request) {
+            hdc->unplug_request(hotplug_ctrl, &pbdev->qdev, errp);
+        }
      }

-    s390_pci_generate_plug_event(HP_EVENT_STANDBY_TO_RESERVED,
-                                 pbdev->fh, pbdev->fid);
-    pbdev->fh = 0;
-    pbdev->fid = 0;
-    pbdev->pdev = NULL;
      object_unparent(OBJECT(pci_dev));
  }

  static void s390_pcihost_class_init(ObjectClass *klass, void *data)
  {
      SysBusDeviceClass *k = SYS_BUS_DEVICE_CLASS(klass);
-    DeviceClass *dc = DEVICE_CLASS(klass);
      HotplugHandlerClass *hc = HOTPLUG_HANDLER_CLASS(klass);

-    dc->cannot_instantiate_with_device_add_yet = true;
      k->init = s390_pcihost_init;
      hc->plug = s390_pcihost_hot_plug;
      hc->unplug = s390_pcihost_hot_unplug;
@@ -585,9 +595,156 @@ static const TypeInfo s390_pcihost_info = {
      }
  };

+static void s390_pci_device_hot_plug(HotplugHandler *hotplug_dev,
+                                     DeviceState *dev, Error **errp)
+{
+    S390PCIBusDevice *zpci = S390_PCI_DEVICE(dev);
+
+    zpci->configured = true;
+
+    if (dev->hotplugged) {
+        s390_pci_generate_plug_event(HP_EVENT_RESERVED_TO_STANDBY,
+                                     zpci->fh, zpci->fid);
+        s390_pci_generate_plug_event(HP_EVENT_TO_CONFIGURED,
+                                     zpci->fh, zpci->fid);
+    }
+}
+
+static void s390_pci_device_hot_unplug_request(HotplugHandler *hotplug_dev,
+                                       DeviceState *dev, Error **errp)
+{
+    S390PCIBusDevice *zpci = S390_PCI_DEVICE(dev);
+
+    if (zpci->configured) {
+        zpci->configured = false;
+        s390_pci_generate_plug_event(HP_EVENT_CONFIGURED_TO_STBRES,
+                                     zpci->fh, zpci->fid);
+    }
+
+    s390_pci_generate_plug_event(HP_EVENT_STANDBY_TO_RESERVED,
+                                 zpci->fh, zpci->fid);
+
+    zpci->is_unplugged = true;
+}
+
+static const TypeInfo s390_pci_fac_bus_info = {
+    .name = TYPE_S390_PCI_FAC_BUS,
+    .parent = TYPE_BUS,
+    .instance_size = sizeof(S390PCIFacBus),
+};
+
+static int s390_pci_facility_init(S390PCIFacility *f)
+{
+    DeviceState *dev = DEVICE(f);
+
+    QTAILQ_INIT(&f->pending_sei);
+    msi_supported = true;
+    f->fbus = S390_PCI_FAC_BUS(qbus_create(TYPE_S390_PCI_FAC_BUS, dev, NULL));
+    qbus_set_hotplug_handler(BUS(&f->fbus->qbus), DEVICE(dev), NULL);
+
+    return 0;
+}
+
+static void s390_pci_facility_class_init(ObjectClass *klass, void *data)
+{
+    S390PCIFacilityClass *k = S390_PCI_FACILITY_CLASS(klass);
+    HotplugHandlerClass *hc = HOTPLUG_HANDLER_CLASS(k);
+
+    k->init = s390_pci_facility_init;
+    hc->plug = s390_pci_device_hot_plug;
+    hc->unplug_request = s390_pci_device_hot_unplug_request;
+}
+
+static const TypeInfo s390_pci_facility_info = {
+    .name          = TYPE_S390_PCI_FACILITY,
+    .parent        = TYPE_SYS_BUS_DEVICE,
+    .instance_size = sizeof(S390PCIFacility),
+    .class_init    = s390_pci_facility_class_init,
+    .class_size    = sizeof(S390PCIFacilityClass),
+    .interfaces = (InterfaceInfo[]) {
+        { TYPE_HOTPLUG_HANDLER },
+        { }
+    }
+};
+
+static void s390_pci_device_realize(DeviceState *dev, Error **errp)
+{
+    S390PCIBusDevice *zpci = S390_PCI_DEVICE(dev);
+    S390PCIBusDevice *tmp;
+    S390pciState *s;
+    BusChild *kid;
+    PCIDevice *pdev;
+    int ret;
+    S390PCIFacility *f = S390_PCI_FACILITY(
+        object_resolve_path(TYPE_S390_PCI_FACILITY, NULL));
+
+    ret = pci_qdev_find_device(zpci->pci_id, &pdev);
+    if (ret < 0) {
+        error_setg(errp, "vfio pci device %s not found", zpci->pci_id);
+        return;
+    }
+
+    QTAILQ_FOREACH(kid, &f->fbus->qbus.children, sibling) {
+        tmp = (S390PCIBusDevice *)kid->child;
+        if (tmp == zpci) {
+            continue;
+        }
+
+        if (tmp->fid == zpci->fid || tmp->uid == zpci->uid ||
+            !strcmp(tmp->pci_id, zpci->pci_id)) {
+            error_setg(errp, "zpci needs unique fid, uid and pci_id");
+            return;
+        }
+    }
+
+    s = S390_PCI_HOST_BRIDGE(pci_device_root_bus(pdev)->qbus.parent);
+    s->conn[PCI_SLOT(pdev->devfn)].zpci = zpci;
+
+    zpci->pdev = pdev;
+    zpci->fh = zpci->fid | FH_VIRT;
+    s390_pcihost_setup_msix(zpci);
+}
+
+static void s390_pci_device_unrealize(DeviceState *dev, Error **errp)
+{
+    S390PCIBusDevice *zpci = S390_PCI_DEVICE(dev);
+
+    zpci->fh = 0;
+    zpci->fid = 0;
+    zpci->pdev = NULL;
+}
+
+static Property s390_pci_device_properties[] = {
+    DEFINE_PROP_UINT32("fid", S390PCIBusDevice, fid, 0),
+    DEFINE_PROP_UINT32("uid", S390PCIBusDevice, uid, 0),
+    DEFINE_PROP_STRING("pci_id", S390PCIBusDevice, pci_id),
+    DEFINE_PROP_END_OF_LIST(),
+};
+
+static void s390_pci_device_class_init(ObjectClass *klass, void *data)
+{
+    DeviceClass *dc = DEVICE_CLASS(klass);
+
+    dc->desc = "s390 pci device";
+    dc->bus_type = TYPE_S390_PCI_FAC_BUS;
+    dc->realize = s390_pci_device_realize;
+    dc->unrealize = s390_pci_device_unrealize;
+    dc->props = s390_pci_device_properties;
+}
+
+static const TypeInfo s390_pci_device_type_info = {
+    .name = TYPE_S390_PCI_DEVICE,
+    .parent = TYPE_DEVICE,
+    .instance_size = sizeof(S390PCIBusDevice),
+    .class_init = s390_pci_device_class_init,
+};
+
  static void s390_pci_register_types(void)
  {
      type_register_static(&s390_pcihost_info);
+    type_register_static(&s390_pci_facility_info);
+    type_register_static(&s390_pci_fac_bus_info);
+    type_register_static(&s390_pci_device_type_info);
  }

  type_init(s390_pci_register_types)
diff --git a/hw/s390x/s390-pci-bus.h b/hw/s390x/s390-pci-bus.h
index 464a92e..5bf3913 100644
--- a/hw/s390x/s390-pci-bus.h
+++ b/hw/s390x/s390-pci-bus.h
@@ -149,6 +149,21 @@ enum ZpciIoatDtype {
  #define ZPCI_TABLE_VALID_MASK           0x20
  #define ZPCI_TABLE_PROT_MASK            0x200

+#define TYPE_S390_PCI_FACILITY "s390-pci-facility"
+#define TYPE_S390_PCI_FAC_BUS "s390-pci-fac-bus"
+#define TYPE_S390_PCI_DEVICE "zpci"
+
+#define S390_PCI_FACILITY(obj) \
+    OBJECT_CHECK(S390PCIFacility, (obj), TYPE_S390_PCI_FACILITY)
+#define S390_PCI_FAC_BUS(obj) \
+    OBJECT_CHECK(S390PCIFacBus, (obj), TYPE_S390_PCI_FAC_BUS)
+#define S390_PCI_FACILITY_CLASS(klass) \
+    OBJECT_CLASS_CHECK(S390PCIFacilityClass, (klass), TYPE_S390_PCI_FACILITY)
+#define S390_PCI_DEVICE(obj) \
+    OBJECT_CHECK(S390PCIBusDevice, (obj), TYPE_S390_PCI_DEVICE)
+#define S390_PCI_FACILITY_GET_CLASS(obj) \
+    OBJECT_GET_CLASS(S390PCIFacilityClass, (obj), TYPE_S390_PCI_FACILITY)
+
  typedef struct SeiContainer {
      QTAILQ_ENTRY(SeiContainer) link;
      uint32_t fid;
@@ -214,12 +229,16 @@ typedef struct S390MsixInfo {
  } S390MsixInfo;

  typedef struct S390PCIBusDevice {
+    DeviceState qdev;
      PCIDevice *pdev;
      bool configured;
+    bool is_unplugged;
      bool error_state;
      bool lgstg_blocked;
      uint32_t fh;
      uint32_t fid;
+    uint32_t uid;
+    char *pci_id;
      uint64_t g_iota;
      uint64_t pba;
      uint64_t pal;
@@ -229,21 +248,42 @@ typedef struct S390PCIBusDevice {
      uint8_t sum;
      S390MsixInfo msix;
      AdapterRoutes routes;
-    AddressSpace as;
-    MemoryRegion mr;
+    QLIST_ENTRY(S390PCIDevice) entry;
  } S390PCIBusDevice;

+typedef struct S390PCIDeviceConn {
+    S390PCIBusDevice *zpci;
+    AddressSpace iommu_as;
+    MemoryRegion iommu_mr;
+} S390PCIDeviceConn;
+
  typedef struct S390pciState {
      PCIHostState parent_obj;
-    S390PCIBusDevice pbdev[PCI_SLOT_MAX];
+    S390PCIDeviceConn conn[PCI_SLOT_MAX];
      AddressSpace msix_notify_as;
      MemoryRegion msix_notify_mr;
-    QTAILQ_HEAD(, SeiContainer) pending_sei;
  } S390pciState;

+typedef struct S390PCIFacBus {
+    BusState qbus;
+} S390PCIFacBus;
+
+typedef struct S390PCIFacility {
+    SysBusDevice parent_obj;
+    S390PCIFacBus *fbus;
+    QTAILQ_HEAD(, SeiContainer) pending_sei;
+} S390PCIFacility;
+
+typedef struct S390PCIFacilityClass {
+    DeviceClass parent_class;
+    int (*init)(S390PCIFacility *f);
+} S390PCIFacilityClass;
+
  int chsc_sei_nt2_get_event(void *res);
  int chsc_sei_nt2_have_event(void);
  void s390_pci_sclp_configure(int configure, SCCB *sccb);
+void s390_pci_device_enable(S390PCIBusDevice *zpci);
+void s390_pci_device_disable(S390PCIBusDevice *zpci);
  S390PCIBusDevice *s390_pci_find_dev_by_idx(uint32_t idx);
  S390PCIBusDevice *s390_pci_find_dev_by_fh(uint32_t fh);
  S390PCIBusDevice *s390_pci_find_dev_by_fid(uint32_t fid);
diff --git a/hw/s390x/s390-pci-inst.c b/hw/s390x/s390-pci-inst.c
index cac3d83..fc8e8e1 100644
--- a/hw/s390x/s390-pci-inst.c
+++ b/hw/s390x/s390-pci-inst.c
@@ -208,12 +208,12 @@ int clp_service_call(S390CPU *cpu, uint8_t r2)

          switch (reqsetpci->oc) {
          case CLP_SET_ENABLE_PCI_FN:
-            pbdev->fh = pbdev->fh | 1 << ENABLE_BIT_OFFSET;
+            s390_pci_device_enable(pbdev);
              stl_p(&ressetpci->fh, pbdev->fh);
              stw_p(&ressetpci->hdr.rsp, CLP_RC_OK);
              break;
          case CLP_SET_DISABLE_PCI_FN:
-            pbdev->fh = pbdev->fh & ~(1 << ENABLE_BIT_OFFSET);
+            s390_pci_device_disable(pbdev);
              pbdev->error_state = false;
              pbdev->lgstg_blocked = false;
              stl_p(&ressetpci->fh, pbdev->fh);
diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c
index 312b8ac..7ee5202 100644
--- a/hw/s390x/s390-virtio-ccw.c
+++ b/hw/s390x/s390-virtio-ccw.c
@@ -177,8 +177,8 @@ static void ccw_init(MachineState *machine)
                        machine->initrd_filename, "s390-ccw.img", true);
      s390_flic_init();

-    dev = qdev_create(NULL, TYPE_S390_PCI_HOST_BRIDGE);
-    object_property_add_child(qdev_get_machine(), TYPE_S390_PCI_HOST_BRIDGE,
+    dev = qdev_create(NULL, TYPE_S390_PCI_FACILITY);
+    object_property_add_child(qdev_get_machine(), TYPE_S390_PCI_FACILITY,
                                OBJECT(dev), NULL);
      qdev_init_nofail(dev);

-- 1.9.3