Re: [Qemu-devel] [PATCH 2/2 RFC] s390x/pci: rework pci infrastructure modeling

[Frank Blaschka]

On Thu, Mar 12, 2015 at 11:50:02AM +0100, Frank Blaschka wrote:
> On Thu, Mar 12, 2015 at 11:03:50AM +0100, Michael S. Tsirkin wrote:
> > On Thu, Mar 12, 2015 at 10:54:24AM +0100, Frank Blaschka wrote:
> > > On Wed, Mar 11, 2015 at 06:42:34PM +0100, Michael S. Tsirkin wrote:
> > > > On Wed, Mar 11, 2015 at 03:38:44PM +0100, Frank Blaschka wrote:
> > > > > On Tue, Mar 10, 2015 at 03:26:23PM +0100, Michael S. Tsirkin wrote:
> > > > > > On Tue, Mar 10, 2015 at 02:03:34PM +0100, Frank Blaschka wrote:
> > > > > > > This patch changes the modeling of the s390 qemu pci 
> > > > > > > infrastructure to
> > > > > > > better match the actual pci architecture defined by the real 
> > > > > > > hardware.
> > > > > > > 
> > > > > > > A pci host bridge like device (s390-pcihost) models the abstract 
> > > > > > > view
> > > > > > > of the bare pci function. It provides s390 specific configuration
> > > > > > > attributes (fid and uid) for the attached pci device. The host 
> > > > > > > bridge
> > > > > > > restrict the pci bus to just hold one single pci device. Also we 
> > > > > > > have
> > > > > > > to make the s390 pci host bridge hot plugable.
> > > > > > 
> > > > > > This requirement is really because of the 1 device per bus
> > > > > > limitation, isn't it?
> > > > > > If you supported many devices per bus, you could use
> > > > > > hotplug there and there won't be need to support hotplug
> > > > > > of the host bridge.
> > > > > >
> > > > > Absolutely yes. Have you seen my first proposal?
> > > > > It basically exploits the normal pci bridge/bus/slot mechanism but 
> > > > > need
> > > > > a place to store s390 specific configuration attributes.
> > > > >
> > > > > The idea of a host bridge having this attributes and limit the bus
> > > > > to one slot was an alternate design approach suggested by Alex.
> > > > > 
> > > > > I like Alex's idea because:
> > > > > 1) It reflects pretty well the actual nature of the pci system in 
> > > > > real s390 hw
> > > > > 2) It does not create an somehow "artifical" pci topology
> > > > > 
> > > > 
> > > > I'll have to re-read but here's a thought: use your patch but
> > > > remove host bridge hotplug support code.
> > > > Stick a standard bridge with shpc support in the single slot
> > > > behind your host bridge (existing pci-bridge-dev should do the trick,
> > > > though not many people use it, so you might
> > > > run into bugs, but fixing them is a good idea anyway).
> > > > You can instanciate it automatically like Marcel's patches do
> > > > for PXB.
> > > Still don't undertsand so I try to summarize in my words please corrent me
> > > if I got something wrong
> > > 
> > > - create a standard host bridge
> > > - change the s390-pcihost to be a pci 2 pci bridge
> > 
> > Actually I suggested simply adding a pci 2 pci bridge behind
> > s390-pcihost.
> > 
> > > - now we can hotplug the s390-pcihost + hotplug a pci device to this
> > >   s390-pcihost using standard pci hotplug mechanism
> > 
> > My idea was to just hotplug a pci device behind the standard pci 2 pci
> > bridge. don't support hotplugging bridge itself or s390-pcihost itself.
> > 
> > > - we keep the 1 slot limit on the s390-pcihost. We need a place to
> > >   store fid and uid information (see mail thread from my 1 proposal)
> > 
> > Yes.
> > 
> > > - If we need more than 32 pci functions we have to extend the primary pci 
> > > bus
> > >   via standart pci 2 pci bridges or add another standart host bridge
> > > 
> > > Is this your suggestion?
> > 
> > Almost, clarifications above.
> >
> OK, got your idea. Have to think about it and may do some prototyping. THX!
>

hm, after thinking more about this I realized this is not working for us.
Remember we need a place to store the fid and uid attributes. This place
must be:
1) uid/fid per pci device
2) uid/fid in a hotplugable device

I have the feeling we are at the beginning again. Although I liked Alex's
idea (host bridge containing uid/fid and having only 1 slot on the bus), it
looks like we end up at my first proposal. This does not require any
modification in base pci/bus code.

Thx to all of you for the discussion and suggestions.

> > > > 
> > > > You get hotplug + multiple devices for free.
> > > > 
> > > > And the resulting topology is clearly something that *can* exist
> > > > on bare-metal, since no one can prevent users sticking
> > > > a pci bridge device in a pci slot.
> > > 
> > > Again on bar s390 metal we do not have bridge bus slot at all.
> > > OS does not have any knowledge about it. So anything we try to model
> > > is somehow artificial.
> > 
> > So I wonder: what does guest see if you do it like the above in QEMU?
> > Do s390 guests even have ability to enumerate pci to pci bridges?
> I think the answer is no. All a s390 guest can do is call a s390 
> instruction to get a list of pci functions. There is no further
> information about the function. The fact there might be a bridge/bus/slot
> is completely abstracted in hw and not exposed to the OS.
> 
> I think the fact, the hw guaranties a pci function is completetly
> independent, made the kernel folks to decide to enumarate the domain.
> Looking at a pci address on the guest (or in general on s390 kernel)
> you will never see a bus or slot != 0 (xxxx:00:00.0)
> 
> Anyhow what we are doing do model pci in qemu, the guest will only see
> a list of pci functions and enumerate the domain to create a pci
> address and that's it 
> 
> > 
> > > > 
> > > > Why do I prefer pci hotplug to s390 hotplug?
> > > > s390 hotplug is really surprise removal. There's no way
> > > 
> > > Don't understand this neither. s390 linux kernel is well prepared
> > > to receive s390 specific events to signal a pci function is
> > > going away. Anyhow we end up I can generate this events
> > > 
> > > > to get ack from guest. Linux isn't very well equipped to
> > > > handle that, we might be able to fix it with time but current
> > > > virtio drivers aren't very happy.
> > > > 
> > > > >  
> > > > > > > This is done by
> > > > > > > implementing a s390 specific bus to attach this new host bridge 
> > > > > > > like
> > > > > > > devices.
> > > > > > > 
> > > > > > > Sample qemu configuration:
> > > > > > > -device s390-pcihost,fid=16,uid=2216
> > > > > > > -device e1000,bus=pci.0
> > > > > > > -device s390-pcihost,fid=17,uid=2217,id=mydev
> > > > > > > -device ne2k_pci,bus=mydev.0,addr=0
> > > > > > > 
> > > > > > > A pci device references the corresponding host bridge via pci bus 
> > > > > > > name
> > > > > > > (as usual). The pci device must be attached to slot 0 of the bus.
> > > > > > >
> > > > > > > The fid and uid must be unique for the qemu instance. The design
> > > > > > > allows to define (static and hotplug) multiple host bridges and 
> > > > > > > support
> > > > > > > a large number of pci devices.
> > > > > > 
> > > > > > How about sticking a pci to pci bridge behind the host bridge?
> > > > > > You could also support hotplug using shpc, all
> > > > > > this without writing code.
> > > > > > 
> > > > > Hm, I don't understand this in detail, can you elaborate a little bit 
> > > > > more
> > > > > on this?
> > > > > 
> > > > > For me it looks like this has the same issues like my first proposal. 
> > > > > We
> > > > > build a complete artifical pci topology in qemu, which has nothing to 
> > > > > do with
> > > > > the real hw. If we include pci 2 pci bridges this makes configuration 
> > > > > even
> > > > > more a nightmare for users.
> > > > > 
> > > > > Do you think detangle pci bus from bridge breaks some fundamental 
> > > > > design?
> > > > > If so, I would rather go with my first proposal than adding even more
> > > > > complexity to implementation and configuration.
> > > > > 
> > > > > Thx, Frank
> > > > > > 
> > > > > > > Signed-off-by: Frank Blaschka <address@hidden>
> > > > > > > ---
> > > > > > >  hw/s390x/s390-pci-bus.c    | 174 
> > > > > > > ++++++++++++++++++++++++++-------------------
> > > > > > >  hw/s390x/s390-pci-bus.h    |  24 ++++++-
> > > > > > >  hw/s390x/s390-pci-inst.c   |   2 +-
> > > > > > >  hw/s390x/s390-virtio-ccw.c |   4 +-
> > > > > > >  4 files changed, 128 insertions(+), 76 deletions(-)
> > > > > > > 
> > > > > > > diff --git a/hw/s390x/s390-pci-bus.c b/hw/s390x/s390-pci-bus.c
> > > > > > > index dc455a2..6ad80d9 100644
> > > > > > > --- a/hw/s390x/s390-pci-bus.c
> > > > > > > +++ b/hw/s390x/s390-pci-bus.c
> > > > > > > @@ -32,12 +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));
> > > > > > > -
> > > > > > > -    if (!s) {
> > > > > > > -        return rc;
> > > > > > > -    }
> > > > > > > +    PCIFacility *s = PCI_FACILITY(
> > > > > > > +        object_resolve_path(TYPE_PCI_FACILITY, NULL));
> > > > > > >  
> > > > > > >      sei_cont = QTAILQ_FIRST(&s->pending_sei);
> > > > > > >      if (sei_cont) {
> > > > > > > @@ -71,31 +67,23 @@ 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));
> > > > > > > -
> > > > > > > -    if (!s) {
> > > > > > > -        return 0;
> > > > > > > -    }
> > > > > > > +    PCIFacility *s = PCI_FACILITY(
> > > > > > > +        object_resolve_path(TYPE_PCI_FACILITY, NULL));
> > > > > > >  
> > > > > > >      return !QTAILQ_EMPTY(&s->pending_sei);
> > > > > > >  }
> > > > > > >  
> > > > > > >  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;
> > > > > > > +    S390pciState *state;
> > > > > > > +    PCIFacility *s = PCI_FACILITY(
> > > > > > > +        object_resolve_path(TYPE_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)) {
> > > > > > > -            return pbdev;
> > > > > > > +    QTAILQ_FOREACH(kid, &s->sbus.qbus.children, sibling) {
> > > > > > > +        state = (S390pciState *)kid->child;
> > > > > > > +        if ((state->pbdev[0].fh != 0) && (state->pbdev[0].fid == 
> > > > > > > fid)) {
> > > > > > > +            return &state->pbdev[0];
> > > > > > >          }
> > > > > > >      }
> > > > > > >  
> > > > > > > @@ -125,37 +113,23 @@ 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;
> > > > > > > +    BusChild *kid;
> > > > > > >      int j = 0;
> > > > > > > -    S390pciState *s = S390_PCI_HOST_BRIDGE(
> > > > > > > -        object_resolve_path(TYPE_S390_PCI_HOST_BRIDGE, NULL));
> > > > > > > -
> > > > > > > -    if (!s) {
> > > > > > > -        return NULL;
> > > > > > > -    }
> > > > > > > +    S390pciState *state;
> > > > > > > +    PCIFacility *s = PCI_FACILITY(
> > > > > > > +        object_resolve_path(TYPE_PCI_FACILITY, NULL));
> > > > > > >  
> > > > > > > -    for (i = 0; i < PCI_SLOT_MAX; i++) {
> > > > > > > -        pbdev = &s->pbdev[i];
> > > > > > > +    QTAILQ_FOREACH(kid, &s->sbus.qbus.children, sibling) {
> > > > > > > +        state = (S390pciState *)kid->child;
> > > > > > >  
> > > > > > > -        if (pbdev->fh == 0) {
> > > > > > > +        if (state->pbdev[0].fh == 0) {
> > > > > > >              continue;
> > > > > > >          }
> > > > > > >  
> > > > > > >          if (j == idx) {
> > > > > > > -            return pbdev;
> > > > > > > +            return &state->pbdev[0];
> > > > > > >          }
> > > > > > >          j++;
> > > > > > >      }
> > > > > > > @@ -165,19 +139,19 @@ 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;
> > > > > > > +    S390pciState *state;
> > > > > > > +    PCIFacility *s = PCI_FACILITY(
> > > > > > > +        object_resolve_path(TYPE_PCI_FACILITY, NULL));
> > > > > > >  
> > > > > > > -    if (!s || !fh) {
> > > > > > > +    if (!fh) {
> > > > > > >          return NULL;
> > > > > > >      }
> > > > > > >  
> > > > > > > -    for (i = 0; i < PCI_SLOT_MAX; i++) {
> > > > > > > -        pbdev = &s->pbdev[i];
> > > > > > > -        if (pbdev->fh == fh) {
> > > > > > > -            return pbdev;
> > > > > > > +    QTAILQ_FOREACH(kid, &s->sbus.qbus.children, sibling) {
> > > > > > > +        state = (S390pciState *)kid->child;
> > > > > > > +        if (state->pbdev[0].fh == fh) {
> > > > > > > +            return &state->pbdev[0];
> > > > > > >          }
> > > > > > >      }
> > > > > > >  
> > > > > > > @@ -188,12 +162,8 @@ 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));
> > > > > > > -
> > > > > > > -    if (!s) {
> > > > > > > -        return;
> > > > > > > -    }
> > > > > > > +    PCIFacility *s = PCI_FACILITY(
> > > > > > > +        object_resolve_path(TYPE_PCI_FACILITY, NULL));
> > > > > > >  
> > > > > > >      sei_cont = g_malloc0(sizeof(SeiContainer));
> > > > > > >      sei_cont->fh = fh;
> > > > > > > @@ -461,15 +431,29 @@ static void 
> > > > > > > s390_pcihost_init_as(S390pciState *s)
> > > > > > >      address_space_init(&s->msix_notify_as, &s->msix_notify_mr, 
> > > > > > > "msix-pci");
> > > > > > >  }
> > > > > > >  
> > > > > > > -static int s390_pcihost_init(SysBusDevice *dev)
> > > > > > > +static void s390_pcihost_realize(DeviceState *dev, Error **errp)
> > > > > > >  {
> > > > > > >      PCIBus *b;
> > > > > > >      BusState *bus;
> > > > > > > -    PCIHostState *phb = PCI_HOST_BRIDGE(dev);
> > > > > > >      S390pciState *s = S390_PCI_HOST_BRIDGE(dev);
> > > > > > > +    S390pciState *state;
> > > > > > > +    BusChild *kid;
> > > > > > > +    PCIFacility *fac = PCI_FACILITY(
> > > > > > > +        object_resolve_path(TYPE_PCI_FACILITY, NULL));
> > > > > > >  
> > > > > > >      DPRINTF("host_init\n");
> > > > > > >  
> > > > > > > +    QTAILQ_FOREACH(kid, &fac->sbus.qbus.children, sibling) {
> > > > > > > +        state = (S390pciState *)kid->child;
> > > > > > > +        if (state == s) {
> > > > > > > +            continue;
> > > > > > > +        }
> > > > > > > +        if (state->fid == s->fid || state->uid == s->uid) {
> > > > > > > +            error_setg(errp, "s390-pcihost needs unique fid and 
> > > > > > > uid");
> > > > > > > +            return;
> > > > > > > +        }
> > > > > > > +    }
> > > > > > > +
> > > > > > >      b = pci_register_bus(DEVICE(dev), NULL,
> > > > > > >                           s390_pci_set_irq, s390_pci_map_irq, 
> > > > > > > NULL,
> > > > > > >                           get_system_memory(), get_system_io(), 
> > > > > > > 0, 64,
> > > > > > > @@ -479,9 +463,6 @@ 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;
> > > > > > >  }
> > > > > > >  
> > > > > > >  static int s390_pcihost_setup_msix(S390PCIBusDevice *pbdev)
> > > > > > > @@ -520,12 +501,18 @@ static void 
> > > > > > > s390_pcihost_hot_plug(HotplugHandler *hotplug_dev,
> > > > > > >      S390pciState *s = 
> > > > > > > S390_PCI_HOST_BRIDGE(pci_device_root_bus(pci_dev)
> > > > > > >                                             ->qbus.parent);
> > > > > > >  
> > > > > > > +    if (PCI_SLOT(pci_dev->devfn) != 0) {
> > > > > > > +        error_setg(errp, "s390-pcihost only slot 0 allowed.");
> > > > > > > +        return;
> > > > > > > +    }
> > > > > > > +
> > > > > > >      pbdev = &s->pbdev[PCI_SLOT(pci_dev->devfn)];
> > > > > > >  
> > > > > > > -    pbdev->fid = s390_pci_get_pfid(pci_dev);
> > > > > > > +    pbdev->fid = s->fid;
> > > > > > > +    pbdev->uid = s->uid;
> > > > > > >      pbdev->pdev = pci_dev;
> > > > > > >      pbdev->configured = true;
> > > > > > > -    pbdev->fh = s390_pci_get_pfh(pci_dev);
> > > > > > > +    pbdev->fh = s->fid | FH_VIRT;
> > > > > > >  
> > > > > > >      s390_pcihost_setup_msix(pbdev);
> > > > > > >  
> > > > > > > @@ -560,14 +547,20 @@ static void 
> > > > > > > s390_pcihost_hot_unplug(HotplugHandler *hotplug_dev,
> > > > > > >      object_unparent(OBJECT(pci_dev));
> > > > > > >  }
> > > > > > >  
> > > > > > > +static Property s390_pcihost_properties[] = {
> > > > > > > +    DEFINE_PROP_UINT32("fid", S390pciState, fid, 0),
> > > > > > > +    DEFINE_PROP_UINT32("uid", S390pciState, uid, 0),
> > > > > > > +    DEFINE_PROP_END_OF_LIST(),
> > > > > > > +};
> > > > > > > +
> > > > > > >  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;
> > > > > > > +    dc->props = s390_pcihost_properties;
> > > > > > > +    dc->bus_type = TYPE_PCI_FACILITY_BUS;
> > > > > > > +    dc->realize = s390_pcihost_realize;
> > > > > > >      hc->plug = s390_pcihost_hot_plug;
> > > > > > >      hc->unplug = s390_pcihost_hot_unplug;
> > > > > > >      msi_supported = true;
> > > > > > > @@ -575,7 +568,7 @@ static void 
> > > > > > > s390_pcihost_class_init(ObjectClass *klass, void *data)
> > > > > > >  
> > > > > > >  static const TypeInfo s390_pcihost_info = {
> > > > > > >      .name          = TYPE_S390_PCI_HOST_BRIDGE,
> > > > > > > -    .parent        = TYPE_PCI_HOST_BRIDGE,
> > > > > > > +    .parent        = TYPE_DEVICE,
> > > > > > >      .instance_size = sizeof(S390pciState),
> > > > > > >      .class_init    = s390_pcihost_class_init,
> > > > > > >      .interfaces = (InterfaceInfo[]) {
> > > > > > > @@ -584,9 +577,46 @@ static const TypeInfo s390_pcihost_info = {
> > > > > > >      }
> > > > > > >  };
> > > > > > >  
> > > > > > > +static const TypeInfo pci_facility_bus_info = {
> > > > > > > +    .name = TYPE_PCI_FACILITY_BUS,
> > > > > > > +    .parent = TYPE_BUS,
> > > > > > > +};
> > > > > > > +
> > > > > > > +static void pci_facility_realize(DeviceState *qdev, Error **errp)
> > > > > > > +{
> > > > > > > +    PCIFacility *facility = PCI_FACILITY(qdev);
> > > > > > > +
> > > > > > > +    qbus_create_inplace(&facility->sbus, sizeof(facility->sbus),
> > > > > > > +                        TYPE_PCI_FACILITY_BUS, qdev, NULL);
> > > > > > > +
> > > > > > > +    qbus_set_hotplug_handler(&facility->sbus.qbus, qdev, NULL);
> > > > > > > +    QTAILQ_INIT(&facility->pending_sei);
> > > > > > > +}
> > > > > > > +
> > > > > > > +static void pci_facility_class_init(ObjectClass *klass, void 
> > > > > > > *data)
> > > > > > > +{
> > > > > > > +    DeviceClass *dc = DEVICE_CLASS(klass);
> > > > > > > +
> > > > > > > +    dc->realize = pci_facility_realize;
> > > > > > > +}
> > > > > > > +
> > > > > > > +static const TypeInfo pci_facility_info = {
> > > > > > > +    .name          = TYPE_PCI_FACILITY,
> > > > > > > +    .parent        = TYPE_SYS_BUS_DEVICE,
> > > > > > > +    .instance_size = sizeof(PCIFacility),
> > > > > > > +    .class_init    = pci_facility_class_init,
> > > > > > > +    .class_size    = sizeof(PCIFacilityClass),
> > > > > > > +    .interfaces = (InterfaceInfo[]) {
> > > > > > > +        { TYPE_HOTPLUG_HANDLER },
> > > > > > > +        { }
> > > > > > > +    }
> > > > > > > +};
> > > > > > > +
> > > > > > >  static void s390_pci_register_types(void)
> > > > > > >  {
> > > > > > >      type_register_static(&s390_pcihost_info);
> > > > > > > +    type_register_static(&pci_facility_info);
> > > > > > > +    type_register_static(&pci_facility_bus_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..78a5c83 100644
> > > > > > > --- a/hw/s390x/s390-pci-bus.h
> > > > > > > +++ b/hw/s390x/s390-pci-bus.h
> > > > > > > @@ -220,6 +220,7 @@ typedef struct S390PCIBusDevice {
> > > > > > >      bool lgstg_blocked;
> > > > > > >      uint32_t fh;
> > > > > > >      uint32_t fid;
> > > > > > > +    uint32_t uid;
> > > > > > >      uint64_t g_iota;
> > > > > > >      uint64_t pba;
> > > > > > >      uint64_t pal;
> > > > > > > @@ -238,7 +239,8 @@ typedef struct S390pciState {
> > > > > > >      S390PCIBusDevice pbdev[PCI_SLOT_MAX];
> > > > > > >      AddressSpace msix_notify_as;
> > > > > > >      MemoryRegion msix_notify_mr;
> > > > > > > -    QTAILQ_HEAD(, SeiContainer) pending_sei;
> > > > > > > +    uint32_t fid;
> > > > > > > +    uint32_t uid;
> > > > > > >  } S390pciState;
> > > > > > >  
> > > > > > >  int chsc_sei_nt2_get_event(void *res);
> > > > > > > @@ -248,4 +250,24 @@ 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);
> > > > > > >  
> > > > > > > +#define TYPE_PCI_FACILITY "pci-facility"
> > > > > > > +#define TYPE_PCI_FACILITY_BUS "pci-facility-bus"
> > > > > > > +
> > > > > > > +#define PCI_FACILITY(obj) \
> > > > > > > +    OBJECT_CHECK(PCIFacility, (obj), TYPE_PCI_FACILITY)
> > > > > > > +
> > > > > > > +typedef struct PCIFacilityBus {
> > > > > > > +    BusState qbus;
> > > > > > > +} PCIFacilityBus;
> > > > > > > +
> > > > > > > +typedef struct PCIFacility {
> > > > > > > +    SysBusDevice parent_obj;
> > > > > > > +    PCIFacilityBus sbus;
> > > > > > > +    QTAILQ_HEAD(, SeiContainer) pending_sei;
> > > > > > > +} PCIFacility;
> > > > > > > +
> > > > > > > +typedef struct PCIFacilityClass {
> > > > > > > +    DeviceClass parent_class;
> > > > > > > +    int (*init)(PCIFacility *pf);
> > > > > > > +} PCIFacilityClass;
> > > > > > >  #endif
> > > > > > > diff --git a/hw/s390x/s390-pci-inst.c b/hw/s390x/s390-pci-inst.c
> > > > > > > index 08d8aa6..d138b28 100644
> > > > > > > --- a/hw/s390x/s390-pci-inst.c
> > > > > > > +++ b/hw/s390x/s390-pci-inst.c
> > > > > > > @@ -254,7 +254,7 @@ int clp_service_call(S390CPU *cpu, uint8_t r2)
> > > > > > >          stq_p(&resquery->edma, ZPCI_EDMA_ADDR);
> > > > > > >          stw_p(&resquery->pchid, 0);
> > > > > > >          stw_p(&resquery->ug, 1);
> > > > > > > -        stl_p(&resquery->uid, pbdev->fid);
> > > > > > > +        stl_p(&resquery->uid, pbdev->uid);
> > > > > > >          stw_p(&resquery->hdr.rsp, CLP_RC_OK);
> > > > > > >          break;
> > > > > > >      }
> > > > > > > diff --git a/hw/s390x/s390-virtio-ccw.c 
> > > > > > > b/hw/s390x/s390-virtio-ccw.c
> > > > > > > index 8f0ae59..358b192 100644
> > > > > > > --- a/hw/s390x/s390-virtio-ccw.c
> > > > > > > +++ b/hw/s390x/s390-virtio-ccw.c
> > > > > > > @@ -129,8 +129,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_PCI_FACILITY);
> > > > > > > +    object_property_add_child(qdev_get_machine(), 
> > > > > > > TYPE_PCI_FACILITY,
> > > > > > >                                OBJECT(dev), NULL);
> > > > > > >      qdev_init_nofail(dev);
> > > > > > >  
> > > > > > > -- 
> > > > > > > 2.1.4
> > > > > > 
> > > > 
> > 
> 
>