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

[Hong Bo Li <lihbbj@linux.vnet.ibm.com>]

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);
>>>>
>>>
>>
>>