[Qemu-devel] [PATCH RFC 0/1] Extend s390 pci representation in qemu

[Qemu-devel] [PATCH RFC 0/1] Extend s390 pci representation in qemu

[Frank Blaschka]

For a better understanding of the following patch here is
some general information about how PCI is implemented on the s390
platform. The physical structure of the pci system (bridge, bus, slot)
is not shown to the OS. Instead a firmware and I/O configuration
layer abstracts each PCI card to a bare PCI function.

In essence, the fw layer provides a simple enumeration of the
individual devices, which the s390 pci implementation in the Linux
kernel translates into individual pci domains.

HW layer      |       FW layer       |       Linux kernel
              |                      |
(opaque)      |       function 1     |       0001:00:00.0
              |       function 2     |       0002:00:00.0
              |       function 3     |       0003:00:00.0

In qemu we have following problems:

(1) We have to represent this s390 specific topology information,
    respectively the lack thereof
(2) We have/want to use common qemu PCI infrastructure

The initial implementation did not honor (1) much and tried to derive s390
specific configuration attributes from attributes of qemu pci devices. It
turns out that this is not flexible enough and is not sufficient to support
s390 specific configurations.

The following patch introduces a new zPCI device that kind of represents the
fw layer on real hardware and provides the s390 specific information needed
by guest operating systems. We keep the pci devices the same as in the
general case and just hook them up with the corresponding zPCI device.

Frank Blaschka (1):
  s390x/pci: Extend pci representation by new zpci device

 hw/s390x/s390-pci-bus.c    | 253 ++++++++++++++++++++++++++++++++-------------
 hw/s390x/s390-pci-bus.h    |  38 ++++++-
 hw/s390x/s390-pci-inst.c   |   2 +-
 hw/s390x/s390-virtio-ccw.c |   8 +-
 4 files changed, 227 insertions(+), 74 deletions(-)

-- 
2.1.4

[Qemu-devel] [PATCH RFC 1/1] s390x/pci: Extend pci representation by new zpci device

[Frank Blaschka]

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.

On s390 the physical structure of the pci system (bridge, bus, slot)
in not shown to the OS. For this the pci bridge and bus created
in qemu can also not be shown to the guest. The new zpci device class
represents this abstract view on the bare pci function and allows to
provide s390 specific configuration attributes for it.

Sample qemu configuration:
-device e1000,id=zpci1
-device ne2k_pci,id=zpci2
-device zpci,fid=2,uid=1248,pci_id=zpci1
-device zpci,fid=17,uid=2244,pci_id=zpci2

A zpci device references the corresponding PCI device via device id.
The new design allows to define multiple host bridges and support more
pci devices.

Signed-off-by: Frank Blaschka <blaschka@linux.vnet.ibm.com>
---
 hw/s390x/s390-pci-bus.c    | 253 ++++++++++++++++++++++++++++++++-------------
 hw/s390x/s390-pci-bus.h    |  38 ++++++-
 hw/s390x/s390-pci-inst.c   |   2 +-
 hw/s390x/s390-virtio-ccw.c |   8 +-
 4 files changed, 227 insertions(+), 74 deletions(-)

diff --git a/hw/s390x/s390-pci-bus.c b/hw/s390x/s390-pci-bus.c
index dc455a2..0bb448c 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;
+    ZPci *zpci;
+    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) {
+        zpci = (ZPci *)kid->child;
+        if (zpci->pbdev->fid == fid) {
+            return zpci->pbdev;
         }
     }
 
@@ -137,25 +125,16 @@ static uint32_t s390_pci_get_pfh(PCIDevice *pdev)
 
 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));
-
-    if (!s) {
-        return NULL;
-    }
-
-    for (i = 0; i < PCI_SLOT_MAX; i++) {
-        pbdev = &s->pbdev[i];
-
-        if (pbdev->fh == 0) {
-            continue;
-        }
+    BusChild *kid;
+    ZPci *zpci;
+    PCIFacility *s = PCI_FACILITY(
+        object_resolve_path(TYPE_PCI_FACILITY, NULL));
 
+    QTAILQ_FOREACH(kid, &s->sbus.qbus.children, sibling) {
+        zpci = (ZPci *)kid->child;
         if (j == idx) {
-            return pbdev;
+            return zpci->pbdev;
         }
         j++;
     }
@@ -165,19 +144,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;
+    ZPci *zpci;
+    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) {
+        zpci = (ZPci *)kid->child;
+        if (zpci->pbdev->fh == fh) {
+            return zpci->pbdev;
         }
     }
 
@@ -188,12 +167,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;
@@ -480,7 +455,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;
 }
 
@@ -529,12 +503,6 @@ static void s390_pcihost_hot_plug(HotplugHandler *hotplug_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;
 }
 
@@ -546,14 +514,11 @@ static void s390_pcihost_hot_unplug(HotplugHandler *hotplug_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);
+    if (pbdev->in_use) {
+        error_setg(errp, "device in use by zpci device");
+        return;
     }
 
-    s390_pci_generate_plug_event(HP_EVENT_STANDBY_TO_RESERVED,
-                                 pbdev->fh, pbdev->fid);
     pbdev->fh = 0;
     pbdev->fid = 0;
     pbdev->pdev = NULL;
@@ -563,10 +528,8 @@ static void s390_pcihost_hot_unplug(HotplugHandler *hotplug_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;
@@ -584,9 +547,159 @@ static const TypeInfo s390_pcihost_info = {
     }
 };
 
+static const TypeInfo pci_facility_bus_info = {
+    .name = TYPE_PCI_FACILITY_BUS,
+    .parent = TYPE_BUS,
+};
+
+static int init_pci_facility(PCIFacility *facility)
+{
+    DeviceState *sdev = DEVICE(facility);
+
+    qbus_create_inplace(&facility->sbus, sizeof(facility->sbus),
+                        TYPE_PCI_FACILITY_BUS, sdev, NULL);
+
+    qbus_set_hotplug_handler(&facility->sbus.qbus, sdev, NULL);
+    QTAILQ_INIT(&facility->pending_sei);
+    return 0;
+}
+
+static void pci_facility_plug(HotplugHandler *hotplug_dev,
+                              DeviceState *dev, Error **errp)
+{
+    ZPci *zpci = ZPCI(dev);
+
+    if (dev->hotplugged) {
+        s390_pci_generate_plug_event(HP_EVENT_RESERVED_TO_STANDBY,
+                                     zpci->pbdev->fh, zpci->pbdev->fid);
+        s390_pci_generate_plug_event(HP_EVENT_TO_CONFIGURED,
+                                     zpci->pbdev->fh, zpci->pbdev->fid);
+    }
+}
+
+static void pci_facility_unplug(HotplugHandler *hotplug_dev,
+                                DeviceState *dev, Error **errp)
+{
+    ZPci *zpci = ZPCI(dev);
+
+    if (zpci->pbdev->configured) {
+        zpci->pbdev->configured = false;
+        s390_pci_generate_plug_event(HP_EVENT_CONFIGURED_TO_STBRES,
+                                     zpci->pbdev->fh, zpci->pbdev->fid);
+    }
+
+    s390_pci_generate_plug_event(HP_EVENT_STANDBY_TO_RESERVED,
+                                 zpci->pbdev->fh, zpci->pbdev->fid);
+
+    object_unparent(OBJECT(dev));
+}
+
+static void init_pci_facility_class(ObjectClass *klass, void *data)
+{
+    SysBusDeviceClass *sbdc = SYS_BUS_DEVICE_CLASS(klass);
+    DeviceClass *dc = DEVICE_CLASS(sbdc);
+    PCIFacilityClass *k = PCI_FACILITY_CLASS(dc);
+    HotplugHandlerClass *hc = HOTPLUG_HANDLER_CLASS(klass);
+
+    k->init = init_pci_facility;
+    hc->plug = pci_facility_plug;
+    hc->unplug = pci_facility_unplug;
+}
+
+static const TypeInfo pci_facility_info = {
+    .name          = TYPE_PCI_FACILITY,
+    .parent        = TYPE_SYS_BUS_DEVICE,
+    .instance_size = sizeof(PCIFacility),
+    .class_init    = init_pci_facility_class,
+    .class_size    = sizeof(PCIFacilityClass),
+    .interfaces = (InterfaceInfo[]) {
+        { TYPE_HOTPLUG_HANDLER },
+        { }
+    }
+};
+
+static void zpci_realize(DeviceState *qdev, Error **errp)
+{
+    ZPci *zpci = ZPCI(qdev);
+    ZPci *tmp;
+    PCIDevice *dev;
+    int ret;
+    S390pciState *s;
+    BusChild *kid;
+    PCIFacility *f = PCI_FACILITY(
+        object_resolve_path(TYPE_PCI_FACILITY, NULL));
+
+    if (!zpci->pci_id || !zpci->fid || !zpci->uid) {
+        error_setg(errp, "zpci needs valid fid, uid and pci_id");
+        return;
+    }
+
+    ret = pci_qdev_find_device(zpci->pci_id, &dev);
+    if (ret < 0) {
+        error_setg(errp, "zpci device not found");
+        return;
+    }
+
+    QTAILQ_FOREACH(kid, &f->sbus.qbus.children, sibling) {
+        tmp = (ZPci *)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(dev)
+                             ->qbus.parent);
+
+    zpci->pbdev = &s->pbdev[PCI_SLOT(dev->devfn)];
+    zpci->pbdev->fid = zpci->fid;
+    zpci->pbdev->uid = zpci->uid;
+    zpci->pbdev->fh = zpci->fid | FH_VIRT;
+    zpci->pbdev->in_use = true;
+}
+
+static void zpci_unrealize(DeviceState *qdev, Error **errp)
+{
+    ZPci *zpci = ZPCI(qdev);
+
+    zpci->pbdev->in_use = false;
+}
+
+static Property zpci_properties[] = {
+    DEFINE_PROP_UINT32("fid", ZPci, fid, 0),
+    DEFINE_PROP_UINT32("uid", ZPci, uid, 0),
+    DEFINE_PROP_STRING("pci_id", ZPci, pci_id),
+    DEFINE_PROP_END_OF_LIST(),
+};
+
+static void zpci_class_init(ObjectClass *klass, void *data)
+{
+    DeviceClass *dc = DEVICE_CLASS(klass);
+
+    dc->bus_type = TYPE_PCI_FACILITY_BUS;
+    dc->realize = zpci_realize;
+    dc->unrealize = zpci_unrealize;
+    dc->props = zpci_properties;
+}
+
+static const TypeInfo zpci_type_info = {
+    .name = TYPE_ZPCI,
+    .parent = TYPE_DEVICE,
+    .instance_size = sizeof(ZPci),
+    .class_init = zpci_class_init,
+};
+
 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_register_static(&zpci_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..f7f506c 100644
--- a/hw/s390x/s390-pci-bus.h
+++ b/hw/s390x/s390-pci-bus.h
@@ -215,11 +215,13 @@ typedef struct S390MsixInfo {
 
 typedef struct S390PCIBusDevice {
     PCIDevice *pdev;
+    bool in_use;
     bool configured;
     bool error_state;
     bool lgstg_blocked;
     uint32_t fh;
     uint32_t fid;
+    uint32_t uid;
     uint64_t g_iota;
     uint64_t pba;
     uint64_t pal;
@@ -238,7 +240,6 @@ typedef struct S390pciState {
     S390PCIBusDevice pbdev[PCI_SLOT_MAX];
     AddressSpace msix_notify_as;
     MemoryRegion msix_notify_mr;
-    QTAILQ_HEAD(, SeiContainer) pending_sei;
 } S390pciState;
 
 int chsc_sei_nt2_get_event(void *res);
@@ -248,4 +249,39 @@ 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 TYPE_ZPCI "zpci"
+
+#define PCI_FACILITY(obj) \
+    OBJECT_CHECK(PCIFacility, (obj), TYPE_PCI_FACILITY)
+
+#define PCI_FACILITY_CLASS(klass) \
+    OBJECT_CLASS_CHECK(PCIFacilityClass, (klass), TYPE_PCI_FACILITY)
+
+#define ZPCI(obj) \
+    OBJECT_CHECK(ZPci, (obj), TYPE_ZPCI)
+
+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;
+
+typedef struct ZPci {
+    DeviceState qdev;
+    uint32_t fid;
+    uint32_t uid;
+    char *pci_id;
+    S390PCIBusDevice *pbdev;
+} ZPci;
 #endif
diff --git a/hw/s390x/s390-pci-inst.c b/hw/s390x/s390-pci-inst.c
index 9e5bc5b..401a156 100644
--- a/hw/s390x/s390-pci-inst.c
+++ b/hw/s390x/s390-pci-inst.c
@@ -246,7 +246,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 71bafe0..12d4900 100644
--- a/hw/s390x/s390-virtio-ccw.c
+++ b/hw/s390x/s390-virtio-ccw.c
@@ -129,11 +129,14 @@ static void ccw_init(MachineState *machine)
                       machine->initrd_filename, "s390-ccw.img");
     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);
 
+    dev = qdev_create(NULL, TYPE_S390_PCI_HOST_BRIDGE);
+    qdev_init_nofail(dev);
+
     /* register hypercalls */
     virtio_ccw_register_hcalls();
 
@@ -190,6 +193,7 @@ static void ccw_machine_class_init(ObjectClass *oc, void *data)
     mc->no_sdcard = 1;
     mc->use_sclp = 1;
     mc->max_cpus = 255;
+    mc->has_dynamic_sysbus = 1;
     nc->nmi_monitor_handler = s390_nmi;
 }
 
-- 
2.1.4

Re: [Qemu-devel] [PATCH RFC 1/1] s390x/pci: Extend pci representation by new zpci device

[Alexander Graf]

On 26.02.15 12:59, Frank Blaschka wrote:
> 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.
> 
> On s390 the physical structure of the pci system (bridge, bus, slot)
> in not shown to the OS. For this the pci bridge and bus created
> in qemu can also not be shown to the guest. The new zpci device class
> represents this abstract view on the bare pci function and allows to
> provide s390 specific configuration attributes for it.
> 
> Sample qemu configuration:
> -device e1000,id=zpci1
> -device ne2k_pci,id=zpci2
> -device zpci,fid=2,uid=1248,pci_id=zpci1
> -device zpci,fid=17,uid=2244,pci_id=zpci2
> 
> A zpci device references the corresponding PCI device via device id.
> The new design allows to define multiple host bridges and support more
> pci devices.

Isn't this reverse? Shouldn't it rather be

  -device zpci,...,id=zpci1
  -device e1000,bus=zpci1.0

with a limit on each virtual zpci bus to only support one device?


Alex

Re: [Qemu-devel] [PATCH RFC 1/1] s390x/pci: Extend pci representation by new zpci device

[Frank Blaschka]

On Thu, Feb 26, 2015 at 03:39:15PM +0100, Alexander Graf wrote:
> 
> 
> On 26.02.15 12:59, Frank Blaschka wrote:
> > 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.
> > 
> > On s390 the physical structure of the pci system (bridge, bus, slot)
> > in not shown to the OS. For this the pci bridge and bus created
> > in qemu can also not be shown to the guest. The new zpci device class
> > represents this abstract view on the bare pci function and allows to
> > provide s390 specific configuration attributes for it.
> > 
> > Sample qemu configuration:
> > -device e1000,id=zpci1
> > -device ne2k_pci,id=zpci2
> > -device zpci,fid=2,uid=1248,pci_id=zpci1
> > -device zpci,fid=17,uid=2244,pci_id=zpci2
> > 
> > A zpci device references the corresponding PCI device via device id.
> > The new design allows to define multiple host bridges and support more
> > pci devices.
> 
> Isn't this reverse? Shouldn't it rather be
> 
>   -device zpci,...,id=zpci1
>   -device e1000,bus=zpci1.0
> 
> with a limit on each virtual zpci bus to only support one device?

Do you mean something like having multiple host bridges (providing a pci bus
each) and limit the bus to just one device?

-device s390-pcihost,fid=16,uid=1234
-device s390-pcihost,fid=17,uid=5678
-device e1000,bus=pci.0
-device ne2k_pci,bus=pci.1

We also discussed this option but we don't like the idea to put attributes
belong to the pci device to the host bridge.

> 
> 
> Alex
> 

Re: [Qemu-devel] [PATCH RFC 1/1] s390x/pci: Extend pci representation by new zpci device

[Alexander Graf]

On 26.02.15 16:27, Frank Blaschka wrote:
> On Thu, Feb 26, 2015 at 03:39:15PM +0100, Alexander Graf wrote:
>>
>>
>> On 26.02.15 12:59, Frank Blaschka wrote:
>>> 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.
>>>
>>> On s390 the physical structure of the pci system (bridge, bus, slot)
>>> in not shown to the OS. For this the pci bridge and bus created
>>> in qemu can also not be shown to the guest. The new zpci device class
>>> represents this abstract view on the bare pci function and allows to
>>> provide s390 specific configuration attributes for it.
>>>
>>> Sample qemu configuration:
>>> -device e1000,id=zpci1
>>> -device ne2k_pci,id=zpci2
>>> -device zpci,fid=2,uid=1248,pci_id=zpci1
>>> -device zpci,fid=17,uid=2244,pci_id=zpci2
>>>
>>> A zpci device references the corresponding PCI device via device id.
>>> The new design allows to define multiple host bridges and support more
>>> pci devices.
>>
>> Isn't this reverse? Shouldn't it rather be
>>
>>   -device zpci,...,id=zpci1
>>   -device e1000,bus=zpci1.0
>>
>> with a limit on each virtual zpci bus to only support one device?
> 
> Do you mean something like having multiple host bridges (providing a pci bus
> each) and limit the bus to just one device?
> 
> -device s390-pcihost,fid=16,uid=1234
> -device s390-pcihost,fid=17,uid=5678
> -device e1000,bus=pci.0
> -device ne2k_pci,bus=pci.1
> 
> We also discussed this option but we don't like the idea to put attributes
> belong to the pci device to the host bridge.

I guess I'm not grasping something obvious here :). What exactly are the
attributes again?


Alex

Re: [Qemu-devel] [PATCH RFC 1/1] s390x/pci: Extend pci representation by new zpci device

[Frank Blaschka]

On Thu, Feb 26, 2015 at 04:34:06PM +0100, Alexander Graf wrote:
> 
> 
> On 26.02.15 16:27, Frank Blaschka wrote:
> > On Thu, Feb 26, 2015 at 03:39:15PM +0100, Alexander Graf wrote:
> >>
> >>
> >> On 26.02.15 12:59, Frank Blaschka wrote:
> >>> 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.
> >>>
> >>> On s390 the physical structure of the pci system (bridge, bus, slot)
> >>> in not shown to the OS. For this the pci bridge and bus created
> >>> in qemu can also not be shown to the guest. The new zpci device class
> >>> represents this abstract view on the bare pci function and allows to
> >>> provide s390 specific configuration attributes for it.
> >>>
> >>> Sample qemu configuration:
> >>> -device e1000,id=zpci1
> >>> -device ne2k_pci,id=zpci2
> >>> -device zpci,fid=2,uid=1248,pci_id=zpci1
> >>> -device zpci,fid=17,uid=2244,pci_id=zpci2
> >>>
> >>> A zpci device references the corresponding PCI device via device id.
> >>> The new design allows to define multiple host bridges and support more
> >>> pci devices.
> >>
> >> Isn't this reverse? Shouldn't it rather be
> >>
> >>   -device zpci,...,id=zpci1
> >>   -device e1000,bus=zpci1.0
> >>
> >> with a limit on each virtual zpci bus to only support one device?
> > 
> > Do you mean something like having multiple host bridges (providing a pci bus
> > each) and limit the bus to just one device?
> > 
> > -device s390-pcihost,fid=16,uid=1234
> > -device s390-pcihost,fid=17,uid=5678
> > -device e1000,bus=pci.0
> > -device ne2k_pci,bus=pci.1
> > 
> > We also discussed this option but we don't like the idea to put attributes
> > belong to the pci device to the host bridge.
> 
> I guess I'm not grasping something obvious here :). What exactly are the
> attributes again?
>
Sorry for the late response, I was on vacation the last couple days.

The fid and uid values are provided by microcode/io layer on the real hardware.
You can read them out via s390 specific device attributes e.g.

# cat /sys/bus/pci/devices/0000\:00\:00.0/function_id
0x00000016
# cat /sys/bus/pci/devices/0000\:00\:00.0/uid
0x25

Since there is no regular pci address (as explained earlier) I think this is a
mechanism to unique identify a pci function.

We discussed both options how to model this in qemu, but maybe you have another
even better idea how to bring this additional attributes to a qemu pci device.

Thx for any help and new ideas ...

Frank 
> 
> Alex
> 

Re: [Qemu-devel] [PATCH RFC 1/1] s390x/pci: Extend pci representation by new zpci device

[Alexander Graf]

> Am 03.03.2015 um 09:06 schrieb Frank Blaschka <blaschka@linux.vnet.ibm.com>:
> 
>> On Thu, Feb 26, 2015 at 04:34:06PM +0100, Alexander Graf wrote:
>> 
>> 
>>> On 26.02.15 16:27, Frank Blaschka wrote:
>>>> On Thu, Feb 26, 2015 at 03:39:15PM +0100, Alexander Graf wrote:
>>>> 
>>>> 
>>>>> On 26.02.15 12:59, Frank Blaschka wrote:
>>>>> 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.
>>>>> 
>>>>> On s390 the physical structure of the pci system (bridge, bus, slot)
>>>>> in not shown to the OS. For this the pci bridge and bus created
>>>>> in qemu can also not be shown to the guest. The new zpci device class
>>>>> represents this abstract view on the bare pci function and allows to
>>>>> provide s390 specific configuration attributes for it.
>>>>> 
>>>>> Sample qemu configuration:
>>>>> -device e1000,id=zpci1
>>>>> -device ne2k_pci,id=zpci2
>>>>> -device zpci,fid=2,uid=1248,pci_id=zpci1
>>>>> -device zpci,fid=17,uid=2244,pci_id=zpci2
>>>>> 
>>>>> A zpci device references the corresponding PCI device via device id.
>>>>> The new design allows to define multiple host bridges and support more
>>>>> pci devices.
>>>> 
>>>> Isn't this reverse? Shouldn't it rather be
>>>> 
>>>>  -device zpci,...,id=zpci1
>>>>  -device e1000,bus=zpci1.0
>>>> 
>>>> with a limit on each virtual zpci bus to only support one device?
>>> 
>>> Do you mean something like having multiple host bridges (providing a pci bus
>>> each) and limit the bus to just one device?
>>> 
>>> -device s390-pcihost,fid=16,uid=1234
>>> -device s390-pcihost,fid=17,uid=5678
>>> -device e1000,bus=pci.0
>>> -device ne2k_pci,bus=pci.1
>>> 
>>> We also discussed this option but we don't like the idea to put attributes
>>> belong to the pci device to the host bridge.
>> 
>> I guess I'm not grasping something obvious here :). What exactly are the
>> attributes again?
> Sorry for the late response, I was on vacation the last couple days.
> 
> The fid and uid values are provided by microcode/io layer on the real hardware.

So they are arbitrary numbers? What uniqueness constraints do we have on them?

IIUC you can only have a single pcie device behind a virtual "bus" anyway, so what if we just calculate uid and fid from the bus id?

Alex

> You can read them out via s390 specific device attributes e.g.
> 
> # cat /sys/bus/pci/devices/0000\:00\:00.0/function_id
> 0x00000016
> # cat /sys/bus/pci/devices/0000\:00\:00.0/uid
> 0x25
> 
> Since there is no regular pci address (as explained earlier) I think this is a
> mechanism to unique identify a pci function.
> 
> We discussed both options how to model this in qemu, but maybe you have another
> even better idea how to bring this additional attributes to a qemu pci device.
> 
> Thx for any help and new ideas ...
> 
> Frank 
>> 
>> Alex
> 

Re: [Qemu-devel] [PATCH RFC 1/1] s390x/pci: Extend pci representation by new zpci device

[Frank Blaschka]

On Tue, Mar 03, 2015 at 10:33:05AM +0100, Alexander Graf wrote:
> 
> 
> 
> > Am 03.03.2015 um 09:06 schrieb Frank Blaschka <blaschka@linux.vnet.ibm.com>:
> > 
> >> On Thu, Feb 26, 2015 at 04:34:06PM +0100, Alexander Graf wrote:
> >> 
> >> 
> >>> On 26.02.15 16:27, Frank Blaschka wrote:
> >>>> On Thu, Feb 26, 2015 at 03:39:15PM +0100, Alexander Graf wrote:
> >>>> 
> >>>> 
> >>>>> On 26.02.15 12:59, Frank Blaschka wrote:
> >>>>> 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.
> >>>>> 
> >>>>> On s390 the physical structure of the pci system (bridge, bus, slot)
> >>>>> in not shown to the OS. For this the pci bridge and bus created
> >>>>> in qemu can also not be shown to the guest. The new zpci device class
> >>>>> represents this abstract view on the bare pci function and allows to
> >>>>> provide s390 specific configuration attributes for it.
> >>>>> 
> >>>>> Sample qemu configuration:
> >>>>> -device e1000,id=zpci1
> >>>>> -device ne2k_pci,id=zpci2
> >>>>> -device zpci,fid=2,uid=1248,pci_id=zpci1
> >>>>> -device zpci,fid=17,uid=2244,pci_id=zpci2
> >>>>> 
> >>>>> A zpci device references the corresponding PCI device via device id.
> >>>>> The new design allows to define multiple host bridges and support more
> >>>>> pci devices.
> >>>> 
> >>>> Isn't this reverse? Shouldn't it rather be
> >>>> 
> >>>>  -device zpci,...,id=zpci1
> >>>>  -device e1000,bus=zpci1.0
> >>>> 
> >>>> with a limit on each virtual zpci bus to only support one device?
> >>> 
> >>> Do you mean something like having multiple host bridges (providing a pci bus
> >>> each) and limit the bus to just one device?
> >>> 
> >>> -device s390-pcihost,fid=16,uid=1234
> >>> -device s390-pcihost,fid=17,uid=5678
> >>> -device e1000,bus=pci.0
> >>> -device ne2k_pci,bus=pci.1
> >>> 
> >>> We also discussed this option but we don't like the idea to put attributes
> >>> belong to the pci device to the host bridge.
> >> 
> >> I guess I'm not grasping something obvious here :). What exactly are the
> >> attributes again?
> > Sorry for the late response, I was on vacation the last couple days.
> > 
> > The fid and uid values are provided by microcode/io layer on the real hardware.
> 
> So they are arbitrary numbers? What uniqueness constraints do we have on them?
fid and uid must be unique within the same qemu. At a first look the numbers are
arbitrary but our configuration folks want explicitly define a particular fid and uid
to better support migration and pass-through scenarios.
 
> IIUC you can only have a single pcie device behind a virtual "bus" anyway, so what if we just calculate uid and fid from the bus id?
I think this similar to the current implementation. We use the slot (idea for the future was
bus + slot) to generate uid and fid. But this is not flexible enough. As I said, our
configuration folks want to be able to specify fid and uid for the device.

> 
> Alex
> 
> > You can read them out via s390 specific device attributes e.g.
> > 
> > # cat /sys/bus/pci/devices/0000\:00\:00.0/function_id
> > 0x00000016
> > # cat /sys/bus/pci/devices/0000\:00\:00.0/uid
> > 0x25
> > 
> > Since there is no regular pci address (as explained earlier) I think this is a
> > mechanism to unique identify a pci function.
> > 
> > We discussed both options how to model this in qemu, but maybe you have another
> > even better idea how to bring this additional attributes to a qemu pci device.
> > 
> > Thx for any help and new ideas ...
> > 
> > Frank 
> >> 
> >> Alex
> > 
> 

Re: [Qemu-devel] [PATCH RFC 1/1] s390x/pci: Extend pci representation by new zpci device

[Alexander Graf]

On 03.03.15 14:25, Frank Blaschka wrote:
> On Tue, Mar 03, 2015 at 10:33:05AM +0100, Alexander Graf wrote:
>>
>>
>>
>>> Am 03.03.2015 um 09:06 schrieb Frank Blaschka <blaschka@linux.vnet.ibm.com>:
>>>
>>>> On Thu, Feb 26, 2015 at 04:34:06PM +0100, Alexander Graf wrote:
>>>>
>>>>
>>>>> On 26.02.15 16:27, Frank Blaschka wrote:
>>>>>> On Thu, Feb 26, 2015 at 03:39:15PM +0100, Alexander Graf wrote:
>>>>>>
>>>>>>
>>>>>>> On 26.02.15 12:59, Frank Blaschka wrote:
>>>>>>> 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.
>>>>>>>
>>>>>>> On s390 the physical structure of the pci system (bridge, bus, slot)
>>>>>>> in not shown to the OS. For this the pci bridge and bus created
>>>>>>> in qemu can also not be shown to the guest. The new zpci device class
>>>>>>> represents this abstract view on the bare pci function and allows to
>>>>>>> provide s390 specific configuration attributes for it.
>>>>>>>
>>>>>>> Sample qemu configuration:
>>>>>>> -device e1000,id=zpci1
>>>>>>> -device ne2k_pci,id=zpci2
>>>>>>> -device zpci,fid=2,uid=1248,pci_id=zpci1
>>>>>>> -device zpci,fid=17,uid=2244,pci_id=zpci2
>>>>>>>
>>>>>>> A zpci device references the corresponding PCI device via device id.
>>>>>>> The new design allows to define multiple host bridges and support more
>>>>>>> pci devices.
>>>>>>
>>>>>> Isn't this reverse? Shouldn't it rather be
>>>>>>
>>>>>>  -device zpci,...,id=zpci1
>>>>>>  -device e1000,bus=zpci1.0
>>>>>>
>>>>>> with a limit on each virtual zpci bus to only support one device?
>>>>>
>>>>> Do you mean something like having multiple host bridges (providing a pci bus
>>>>> each) and limit the bus to just one device?
>>>>>
>>>>> -device s390-pcihost,fid=16,uid=1234
>>>>> -device s390-pcihost,fid=17,uid=5678
>>>>> -device e1000,bus=pci.0
>>>>> -device ne2k_pci,bus=pci.1
>>>>>
>>>>> We also discussed this option but we don't like the idea to put attributes
>>>>> belong to the pci device to the host bridge.
>>>>
>>>> I guess I'm not grasping something obvious here :). What exactly are the
>>>> attributes again?
>>> Sorry for the late response, I was on vacation the last couple days.
>>>
>>> The fid and uid values are provided by microcode/io layer on the real hardware.
>>
>> So they are arbitrary numbers? What uniqueness constraints do we have on them?
> fid and uid must be unique within the same qemu. At a first look the numbers are
> arbitrary but our configuration folks want explicitly define a particular fid and uid
> to better support migration and pass-through scenarios.

Well, at the end of the day you want to make sure they're identical on
both sides, yes.

>> IIUC you can only have a single pcie device behind a virtual "bus" anyway, so what if we just calculate uid and fid from the bus id?
> I think this similar to the current implementation. We use the slot (idea for the future was
> bus + slot) to generate uid and fid. But this is not flexible enough. As I said, our
> configuration folks want to be able to specify fid and uid for the device.

I don't see how this is different from what PPC does with its LIOBN
which is a property of the PHB.


Alex

Re: [Qemu-devel] [PATCH RFC 1/1] s390x/pci: Extend pci representation by new zpci device

[Frank Blaschka]

On Tue, Mar 03, 2015 at 09:38:37PM +0100, Alexander Graf wrote:
> 
> 
> On 03.03.15 14:25, Frank Blaschka wrote:
> > On Tue, Mar 03, 2015 at 10:33:05AM +0100, Alexander Graf wrote:
> >>
> >>
> >>
> >>> Am 03.03.2015 um 09:06 schrieb Frank Blaschka <blaschka@linux.vnet.ibm.com>:
> >>>
> >>>> On Thu, Feb 26, 2015 at 04:34:06PM +0100, Alexander Graf wrote:
> >>>>
> >>>>
> >>>>> On 26.02.15 16:27, Frank Blaschka wrote:
> >>>>>> On Thu, Feb 26, 2015 at 03:39:15PM +0100, Alexander Graf wrote:
> >>>>>>
> >>>>>>
> >>>>>>> On 26.02.15 12:59, Frank Blaschka wrote:
> >>>>>>> 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.
> >>>>>>>
> >>>>>>> On s390 the physical structure of the pci system (bridge, bus, slot)
> >>>>>>> in not shown to the OS. For this the pci bridge and bus created
> >>>>>>> in qemu can also not be shown to the guest. The new zpci device class
> >>>>>>> represents this abstract view on the bare pci function and allows to
> >>>>>>> provide s390 specific configuration attributes for it.
> >>>>>>>
> >>>>>>> Sample qemu configuration:
> >>>>>>> -device e1000,id=zpci1
> >>>>>>> -device ne2k_pci,id=zpci2
> >>>>>>> -device zpci,fid=2,uid=1248,pci_id=zpci1
> >>>>>>> -device zpci,fid=17,uid=2244,pci_id=zpci2
> >>>>>>>
> >>>>>>> A zpci device references the corresponding PCI device via device id.
> >>>>>>> The new design allows to define multiple host bridges and support more
> >>>>>>> pci devices.
> >>>>>>
> >>>>>> Isn't this reverse? Shouldn't it rather be
> >>>>>>
> >>>>>>  -device zpci,...,id=zpci1
> >>>>>>  -device e1000,bus=zpci1.0
> >>>>>>
> >>>>>> with a limit on each virtual zpci bus to only support one device?
> >>>>>
> >>>>> Do you mean something like having multiple host bridges (providing a pci bus
> >>>>> each) and limit the bus to just one device?
> >>>>>
> >>>>> -device s390-pcihost,fid=16,uid=1234
> >>>>> -device s390-pcihost,fid=17,uid=5678
> >>>>> -device e1000,bus=pci.0
> >>>>> -device ne2k_pci,bus=pci.1
> >>>>>
> >>>>> We also discussed this option but we don't like the idea to put attributes
> >>>>> belong to the pci device to the host bridge.
> >>>>
> >>>> I guess I'm not grasping something obvious here :). What exactly are the
> >>>> attributes again?
> >>> Sorry for the late response, I was on vacation the last couple days.
> >>>
> >>> The fid and uid values are provided by microcode/io layer on the real hardware.
> >>
> >> So they are arbitrary numbers? What uniqueness constraints do we have on them?
> > fid and uid must be unique within the same qemu. At a first look the numbers are
> > arbitrary but our configuration folks want explicitly define a particular fid and uid
> > to better support migration and pass-through scenarios.
> 
> Well, at the end of the day you want to make sure they're identical on
> both sides, yes.
> 
> >> IIUC you can only have a single pcie device behind a virtual "bus" anyway, so what if we just calculate uid and fid from the bus id?
> > I think this similar to the current implementation. We use the slot (idea for the future was
> > bus + slot) to generate uid and fid. But this is not flexible enough. As I said, our
> > configuration folks want to be able to specify fid and uid for the device.
> 
> I don't see how this is different from what PPC does with its LIOBN
> which is a property of the PHB.
> 
> 
> Alex
> 

I played arround with the idea of having multiple host bridges and this worked well
at least for static (non hotplug) configuration. In case I want to hotplug a host
bridge I got following error:

(qemu) device_add s390-pcihost,fid=8,uid=9
Bus 'main-system-bus' does not support hotplugging

Is there anything I have to enable to support this?

I have: has_dynamic_sysbus = 1 and cannot_instantiate_with_device_add_yet = false
but this seems not to help for the hotplug case.

Frank

Re: [Qemu-devel] [PATCH RFC 1/1] s390x/pci: Extend pci representation by new zpci device

[Alexander Graf]

On 04.03.15 14:44, Frank Blaschka wrote:
> On Tue, Mar 03, 2015 at 09:38:37PM +0100, Alexander Graf wrote:
>>
>>
>> On 03.03.15 14:25, Frank Blaschka wrote:
>>> On Tue, Mar 03, 2015 at 10:33:05AM +0100, Alexander Graf wrote:
>>>>
>>>>
>>>>
>>>>> Am 03.03.2015 um 09:06 schrieb Frank Blaschka <blaschka@linux.vnet.ibm.com>:
>>>>>
>>>>>> On Thu, Feb 26, 2015 at 04:34:06PM +0100, Alexander Graf wrote:
>>>>>>
>>>>>>
>>>>>>> On 26.02.15 16:27, Frank Blaschka wrote:
>>>>>>>> On Thu, Feb 26, 2015 at 03:39:15PM +0100, Alexander Graf wrote:
>>>>>>>>
>>>>>>>>
>>>>>>>>> On 26.02.15 12:59, Frank Blaschka wrote:
>>>>>>>>> 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.
>>>>>>>>>
>>>>>>>>> On s390 the physical structure of the pci system (bridge, bus, slot)
>>>>>>>>> in not shown to the OS. For this the pci bridge and bus created
>>>>>>>>> in qemu can also not be shown to the guest. The new zpci device class
>>>>>>>>> represents this abstract view on the bare pci function and allows to
>>>>>>>>> provide s390 specific configuration attributes for it.
>>>>>>>>>
>>>>>>>>> Sample qemu configuration:
>>>>>>>>> -device e1000,id=zpci1
>>>>>>>>> -device ne2k_pci,id=zpci2
>>>>>>>>> -device zpci,fid=2,uid=1248,pci_id=zpci1
>>>>>>>>> -device zpci,fid=17,uid=2244,pci_id=zpci2
>>>>>>>>>
>>>>>>>>> A zpci device references the corresponding PCI device via device id.
>>>>>>>>> The new design allows to define multiple host bridges and support more
>>>>>>>>> pci devices.
>>>>>>>>
>>>>>>>> Isn't this reverse? Shouldn't it rather be
>>>>>>>>
>>>>>>>>  -device zpci,...,id=zpci1
>>>>>>>>  -device e1000,bus=zpci1.0
>>>>>>>>
>>>>>>>> with a limit on each virtual zpci bus to only support one device?
>>>>>>>
>>>>>>> Do you mean something like having multiple host bridges (providing a pci bus
>>>>>>> each) and limit the bus to just one device?
>>>>>>>
>>>>>>> -device s390-pcihost,fid=16,uid=1234
>>>>>>> -device s390-pcihost,fid=17,uid=5678
>>>>>>> -device e1000,bus=pci.0
>>>>>>> -device ne2k_pci,bus=pci.1
>>>>>>>
>>>>>>> We also discussed this option but we don't like the idea to put attributes
>>>>>>> belong to the pci device to the host bridge.
>>>>>>
>>>>>> I guess I'm not grasping something obvious here :). What exactly are the
>>>>>> attributes again?
>>>>> Sorry for the late response, I was on vacation the last couple days.
>>>>>
>>>>> The fid and uid values are provided by microcode/io layer on the real hardware.
>>>>
>>>> So they are arbitrary numbers? What uniqueness constraints do we have on them?
>>> fid and uid must be unique within the same qemu. At a first look the numbers are
>>> arbitrary but our configuration folks want explicitly define a particular fid and uid
>>> to better support migration and pass-through scenarios.
>>
>> Well, at the end of the day you want to make sure they're identical on
>> both sides, yes.
>>
>>>> IIUC you can only have a single pcie device behind a virtual "bus" anyway, so what if we just calculate uid and fid from the bus id?
>>> I think this similar to the current implementation. We use the slot (idea for the future was
>>> bus + slot) to generate uid and fid. But this is not flexible enough. As I said, our
>>> configuration folks want to be able to specify fid and uid for the device.
>>
>> I don't see how this is different from what PPC does with its LIOBN
>> which is a property of the PHB.
>>
>>
>> Alex
>>
> 
> I played arround with the idea of having multiple host bridges and this worked well
> at least for static (non hotplug) configuration. In case I want to hotplug a host
> bridge I got following error:
> 
> (qemu) device_add s390-pcihost,fid=8,uid=9
> Bus 'main-system-bus' does not support hotplugging
> 
> Is there anything I have to enable to support this?
> 
> I have: has_dynamic_sysbus = 1 and cannot_instantiate_with_device_add_yet = false
> but this seems not to help for the hotplug case.

Having s390 devices reside on sysbus is probably a bad idea. Instead,
they should be on an s390 specific bus which then can implement hotplug
easily.


Alex

Re: [Qemu-devel] [PATCH RFC 1/1] s390x/pci: Extend pci representation by new zpci device

[Frank Blaschka]

On Wed, Mar 04, 2015 at 03:49:15PM +0100, Alexander Graf wrote:
> 
> 
> On 04.03.15 14:44, Frank Blaschka wrote:
> > On Tue, Mar 03, 2015 at 09:38:37PM +0100, Alexander Graf wrote:
> >>
> >>
> >> On 03.03.15 14:25, Frank Blaschka wrote:
> >>> On Tue, Mar 03, 2015 at 10:33:05AM +0100, Alexander Graf wrote:
> >>>>
> >>>>
> >>>>
> >>>>> Am 03.03.2015 um 09:06 schrieb Frank Blaschka <blaschka@linux.vnet.ibm.com>:
> >>>>>
> >>>>>> On Thu, Feb 26, 2015 at 04:34:06PM +0100, Alexander Graf wrote:
> >>>>>>
> >>>>>>
> >>>>>>> On 26.02.15 16:27, Frank Blaschka wrote:
> >>>>>>>> On Thu, Feb 26, 2015 at 03:39:15PM +0100, Alexander Graf wrote:
> >>>>>>>>
> >>>>>>>>
> >>>>>>>>> On 26.02.15 12:59, Frank Blaschka wrote:
> >>>>>>>>> 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.
> >>>>>>>>>
> >>>>>>>>> On s390 the physical structure of the pci system (bridge, bus, slot)
> >>>>>>>>> in not shown to the OS. For this the pci bridge and bus created
> >>>>>>>>> in qemu can also not be shown to the guest. The new zpci device class
> >>>>>>>>> represents this abstract view on the bare pci function and allows to
> >>>>>>>>> provide s390 specific configuration attributes for it.
> >>>>>>>>>
> >>>>>>>>> Sample qemu configuration:
> >>>>>>>>> -device e1000,id=zpci1
> >>>>>>>>> -device ne2k_pci,id=zpci2
> >>>>>>>>> -device zpci,fid=2,uid=1248,pci_id=zpci1
> >>>>>>>>> -device zpci,fid=17,uid=2244,pci_id=zpci2
> >>>>>>>>>
> >>>>>>>>> A zpci device references the corresponding PCI device via device id.
> >>>>>>>>> The new design allows to define multiple host bridges and support more
> >>>>>>>>> pci devices.
> >>>>>>>>
> >>>>>>>> Isn't this reverse? Shouldn't it rather be
> >>>>>>>>
> >>>>>>>>  -device zpci,...,id=zpci1
> >>>>>>>>  -device e1000,bus=zpci1.0
> >>>>>>>>
> >>>>>>>> with a limit on each virtual zpci bus to only support one device?
> >>>>>>>
> >>>>>>> Do you mean something like having multiple host bridges (providing a pci bus
> >>>>>>> each) and limit the bus to just one device?
> >>>>>>>
> >>>>>>> -device s390-pcihost,fid=16,uid=1234
> >>>>>>> -device s390-pcihost,fid=17,uid=5678
> >>>>>>> -device e1000,bus=pci.0
> >>>>>>> -device ne2k_pci,bus=pci.1
> >>>>>>>
> >>>>>>> We also discussed this option but we don't like the idea to put attributes
> >>>>>>> belong to the pci device to the host bridge.
> >>>>>>
> >>>>>> I guess I'm not grasping something obvious here :). What exactly are the
> >>>>>> attributes again?
> >>>>> Sorry for the late response, I was on vacation the last couple days.
> >>>>>
> >>>>> The fid and uid values are provided by microcode/io layer on the real hardware.
> >>>>
> >>>> So they are arbitrary numbers? What uniqueness constraints do we have on them?
> >>> fid and uid must be unique within the same qemu. At a first look the numbers are
> >>> arbitrary but our configuration folks want explicitly define a particular fid and uid
> >>> to better support migration and pass-through scenarios.
> >>
> >> Well, at the end of the day you want to make sure they're identical on
> >> both sides, yes.
> >>
> >>>> IIUC you can only have a single pcie device behind a virtual "bus" anyway, so what if we just calculate uid and fid from the bus id?
> >>> I think this similar to the current implementation. We use the slot (idea for the future was
> >>> bus + slot) to generate uid and fid. But this is not flexible enough. As I said, our
> >>> configuration folks want to be able to specify fid and uid for the device.
> >>
> >> I don't see how this is different from what PPC does with its LIOBN
> >> which is a property of the PHB.
> >>
> >>
> >> Alex
> >>
> > 
> > I played arround with the idea of having multiple host bridges and this worked well
> > at least for static (non hotplug) configuration. In case I want to hotplug a host
> > bridge I got following error:
> > 
> > (qemu) device_add s390-pcihost,fid=8,uid=9
> > Bus 'main-system-bus' does not support hotplugging
> > 
> > Is there anything I have to enable to support this?
> > 
> > I have: has_dynamic_sysbus = 1 and cannot_instantiate_with_device_add_yet = false
> > but this seems not to help for the hotplug case.
> 
> Having s390 devices reside on sysbus is probably a bad idea. Instead,
> they should be on an s390 specific bus which then can implement hotplug
> easily.
> 
> 
> Alex
>

Hm now I get lost ...

Do you suggest we should implement a s390 specific device (which is not derived from
TYPE_PCI_HOST_BRIDGE) but implements a pci bus so we can attach a pci device to this
device?  

Re: [Qemu-devel] [PATCH RFC 1/1] s390x/pci: Extend pci representation by new zpci device

[Alexander Graf]

On 04.03.15 16:07, Frank Blaschka wrote:
> On Wed, Mar 04, 2015 at 03:49:15PM +0100, Alexander Graf wrote:
>>
>>
>> On 04.03.15 14:44, Frank Blaschka wrote:
>>> On Tue, Mar 03, 2015 at 09:38:37PM +0100, Alexander Graf wrote:
>>>>
>>>>
>>>> On 03.03.15 14:25, Frank Blaschka wrote:
>>>>> On Tue, Mar 03, 2015 at 10:33:05AM +0100, Alexander Graf wrote:
>>>>>>
>>>>>>
>>>>>>
>>>>>>> Am 03.03.2015 um 09:06 schrieb Frank Blaschka <blaschka@linux.vnet.ibm.com>:
>>>>>>>
>>>>>>>> On Thu, Feb 26, 2015 at 04:34:06PM +0100, Alexander Graf wrote:
>>>>>>>>
>>>>>>>>
>>>>>>>>> On 26.02.15 16:27, Frank Blaschka wrote:
>>>>>>>>>> On Thu, Feb 26, 2015 at 03:39:15PM +0100, Alexander Graf wrote:
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>> On 26.02.15 12:59, Frank Blaschka wrote:
>>>>>>>>>>> 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.
>>>>>>>>>>>
>>>>>>>>>>> On s390 the physical structure of the pci system (bridge, bus, slot)
>>>>>>>>>>> in not shown to the OS. For this the pci bridge and bus created
>>>>>>>>>>> in qemu can also not be shown to the guest. The new zpci device class
>>>>>>>>>>> represents this abstract view on the bare pci function and allows to
>>>>>>>>>>> provide s390 specific configuration attributes for it.
>>>>>>>>>>>
>>>>>>>>>>> Sample qemu configuration:
>>>>>>>>>>> -device e1000,id=zpci1
>>>>>>>>>>> -device ne2k_pci,id=zpci2
>>>>>>>>>>> -device zpci,fid=2,uid=1248,pci_id=zpci1
>>>>>>>>>>> -device zpci,fid=17,uid=2244,pci_id=zpci2
>>>>>>>>>>>
>>>>>>>>>>> A zpci device references the corresponding PCI device via device id.
>>>>>>>>>>> The new design allows to define multiple host bridges and support more
>>>>>>>>>>> pci devices.
>>>>>>>>>>
>>>>>>>>>> Isn't this reverse? Shouldn't it rather be
>>>>>>>>>>
>>>>>>>>>>  -device zpci,...,id=zpci1
>>>>>>>>>>  -device e1000,bus=zpci1.0
>>>>>>>>>>
>>>>>>>>>> with a limit on each virtual zpci bus to only support one device?
>>>>>>>>>
>>>>>>>>> Do you mean something like having multiple host bridges (providing a pci bus
>>>>>>>>> each) and limit the bus to just one device?
>>>>>>>>>
>>>>>>>>> -device s390-pcihost,fid=16,uid=1234
>>>>>>>>> -device s390-pcihost,fid=17,uid=5678
>>>>>>>>> -device e1000,bus=pci.0
>>>>>>>>> -device ne2k_pci,bus=pci.1
>>>>>>>>>
>>>>>>>>> We also discussed this option but we don't like the idea to put attributes
>>>>>>>>> belong to the pci device to the host bridge.
>>>>>>>>
>>>>>>>> I guess I'm not grasping something obvious here :). What exactly are the
>>>>>>>> attributes again?
>>>>>>> Sorry for the late response, I was on vacation the last couple days.
>>>>>>>
>>>>>>> The fid and uid values are provided by microcode/io layer on the real hardware.
>>>>>>
>>>>>> So they are arbitrary numbers? What uniqueness constraints do we have on them?
>>>>> fid and uid must be unique within the same qemu. At a first look the numbers are
>>>>> arbitrary but our configuration folks want explicitly define a particular fid and uid
>>>>> to better support migration and pass-through scenarios.
>>>>
>>>> Well, at the end of the day you want to make sure they're identical on
>>>> both sides, yes.
>>>>
>>>>>> IIUC you can only have a single pcie device behind a virtual "bus" anyway, so what if we just calculate uid and fid from the bus id?
>>>>> I think this similar to the current implementation. We use the slot (idea for the future was
>>>>> bus + slot) to generate uid and fid. But this is not flexible enough. As I said, our
>>>>> configuration folks want to be able to specify fid and uid for the device.
>>>>
>>>> I don't see how this is different from what PPC does with its LIOBN
>>>> which is a property of the PHB.
>>>>
>>>>
>>>> Alex
>>>>
>>>
>>> I played arround with the idea of having multiple host bridges and this worked well
>>> at least for static (non hotplug) configuration. In case I want to hotplug a host
>>> bridge I got following error:
>>>
>>> (qemu) device_add s390-pcihost,fid=8,uid=9
>>> Bus 'main-system-bus' does not support hotplugging
>>>
>>> Is there anything I have to enable to support this?
>>>
>>> I have: has_dynamic_sysbus = 1 and cannot_instantiate_with_device_add_yet = false
>>> but this seems not to help for the hotplug case.
>>
>> Having s390 devices reside on sysbus is probably a bad idea. Instead,
>> they should be on an s390 specific bus which then can implement hotplug
>> easily.
>>
>>
>> Alex
>>
> 
> Hm now I get lost ...
> 
> Do you suggest we should implement a s390 specific device (which is not derived from
> TYPE_PCI_HOST_BRIDGE) but implements a pci bus so we can attach a pci device to this
> device?  

Ugh, PCI_HOST_BRIDGE is a sysbus device. Awesome.

Conceptually your PCI bridge is not a sysbus device, since it doesn't
live on a flat MMIO + legacy IRQ routing bus. Instead, it lives on its
own thing that handles MMIO and IRQs via special backdoor interfaces.

How much of the PCI_HOST_BRIDGE device are you actually using? Would it
be a lot of effort to have another s390 specific device that exposes a
PCIBus, but is not of type PCI_HOST_BRIDGE (and thus sysbus)?


Alex

Re: [Qemu-devel] [PATCH RFC 1/1] s390x/pci: Extend pci representation by new zpci device

[Frank Blaschka]

On Wed, Mar 04, 2015 at 04:25:07PM +0100, Alexander Graf wrote:
> 
> 
> On 04.03.15 16:07, Frank Blaschka wrote:
> > On Wed, Mar 04, 2015 at 03:49:15PM +0100, Alexander Graf wrote:
> >>
> >>
> >> On 04.03.15 14:44, Frank Blaschka wrote:
> >>> On Tue, Mar 03, 2015 at 09:38:37PM +0100, Alexander Graf wrote:
> >>>>
> >>>>
> >>>> On 03.03.15 14:25, Frank Blaschka wrote:
> >>>>> On Tue, Mar 03, 2015 at 10:33:05AM +0100, Alexander Graf wrote:
> >>>>>>
> >>>>>>
> >>>>>>
> >>>>>>> Am 03.03.2015 um 09:06 schrieb Frank Blaschka <blaschka@linux.vnet.ibm.com>:
> >>>>>>>
> >>>>>>>> On Thu, Feb 26, 2015 at 04:34:06PM +0100, Alexander Graf wrote:
> >>>>>>>>
> >>>>>>>>
> >>>>>>>>> On 26.02.15 16:27, Frank Blaschka wrote:
> >>>>>>>>>> On Thu, Feb 26, 2015 at 03:39:15PM +0100, Alexander Graf wrote:
> >>>>>>>>>>
> >>>>>>>>>>
> >>>>>>>>>>> On 26.02.15 12:59, Frank Blaschka wrote:
> >>>>>>>>>>> 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.
> >>>>>>>>>>>
> >>>>>>>>>>> On s390 the physical structure of the pci system (bridge, bus, slot)
> >>>>>>>>>>> in not shown to the OS. For this the pci bridge and bus created
> >>>>>>>>>>> in qemu can also not be shown to the guest. The new zpci device class
> >>>>>>>>>>> represents this abstract view on the bare pci function and allows to
> >>>>>>>>>>> provide s390 specific configuration attributes for it.
> >>>>>>>>>>>
> >>>>>>>>>>> Sample qemu configuration:
> >>>>>>>>>>> -device e1000,id=zpci1
> >>>>>>>>>>> -device ne2k_pci,id=zpci2
> >>>>>>>>>>> -device zpci,fid=2,uid=1248,pci_id=zpci1
> >>>>>>>>>>> -device zpci,fid=17,uid=2244,pci_id=zpci2
> >>>>>>>>>>>
> >>>>>>>>>>> A zpci device references the corresponding PCI device via device id.
> >>>>>>>>>>> The new design allows to define multiple host bridges and support more
> >>>>>>>>>>> pci devices.
> >>>>>>>>>>
> >>>>>>>>>> Isn't this reverse? Shouldn't it rather be
> >>>>>>>>>>
> >>>>>>>>>>  -device zpci,...,id=zpci1
> >>>>>>>>>>  -device e1000,bus=zpci1.0
> >>>>>>>>>>
> >>>>>>>>>> with a limit on each virtual zpci bus to only support one device?
> >>>>>>>>>
> >>>>>>>>> Do you mean something like having multiple host bridges (providing a pci bus
> >>>>>>>>> each) and limit the bus to just one device?
> >>>>>>>>>
> >>>>>>>>> -device s390-pcihost,fid=16,uid=1234
> >>>>>>>>> -device s390-pcihost,fid=17,uid=5678
> >>>>>>>>> -device e1000,bus=pci.0
> >>>>>>>>> -device ne2k_pci,bus=pci.1
> >>>>>>>>>
> >>>>>>>>> We also discussed this option but we don't like the idea to put attributes
> >>>>>>>>> belong to the pci device to the host bridge.
> >>>>>>>>
> >>>>>>>> I guess I'm not grasping something obvious here :). What exactly are the
> >>>>>>>> attributes again?
> >>>>>>> Sorry for the late response, I was on vacation the last couple days.
> >>>>>>>
> >>>>>>> The fid and uid values are provided by microcode/io layer on the real hardware.
> >>>>>>
> >>>>>> So they are arbitrary numbers? What uniqueness constraints do we have on them?
> >>>>> fid and uid must be unique within the same qemu. At a first look the numbers are
> >>>>> arbitrary but our configuration folks want explicitly define a particular fid and uid
> >>>>> to better support migration and pass-through scenarios.
> >>>>
> >>>> Well, at the end of the day you want to make sure they're identical on
> >>>> both sides, yes.
> >>>>
> >>>>>> IIUC you can only have a single pcie device behind a virtual "bus" anyway, so what if we just calculate uid and fid from the bus id?
> >>>>> I think this similar to the current implementation. We use the slot (idea for the future was
> >>>>> bus + slot) to generate uid and fid. But this is not flexible enough. As I said, our
> >>>>> configuration folks want to be able to specify fid and uid for the device.
> >>>>
> >>>> I don't see how this is different from what PPC does with its LIOBN
> >>>> which is a property of the PHB.
> >>>>
> >>>>
> >>>> Alex
> >>>>
> >>>
> >>> I played arround with the idea of having multiple host bridges and this worked well
> >>> at least for static (non hotplug) configuration. In case I want to hotplug a host
> >>> bridge I got following error:
> >>>
> >>> (qemu) device_add s390-pcihost,fid=8,uid=9
> >>> Bus 'main-system-bus' does not support hotplugging
> >>>
> >>> Is there anything I have to enable to support this?
> >>>
> >>> I have: has_dynamic_sysbus = 1 and cannot_instantiate_with_device_add_yet = false
> >>> but this seems not to help for the hotplug case.
> >>
> >> Having s390 devices reside on sysbus is probably a bad idea. Instead,
> >> they should be on an s390 specific bus which then can implement hotplug
> >> easily.
> >>
> >>
> >> Alex
> >>
> > 
> > Hm now I get lost ...
> > 
> > Do you suggest we should implement a s390 specific device (which is not derived from
> > TYPE_PCI_HOST_BRIDGE) but implements a pci bus so we can attach a pci device to this
> > device?  
> 
> Ugh, PCI_HOST_BRIDGE is a sysbus device. Awesome.
> 
> Conceptually your PCI bridge is not a sysbus device, since it doesn't
> live on a flat MMIO + legacy IRQ routing bus. Instead, it lives on its
> own thing that handles MMIO and IRQs via special backdoor interfaces.
>
well spoken :-)

> How much of the PCI_HOST_BRIDGE device are you actually using? Would it
> be a lot of effort to have another s390 specific device that exposes a
> PCIBus, but is not of type PCI_HOST_BRIDGE (and thus sysbus)?
> 
I do not use much functionality of the PCI_HOST_BRIDGE but I was not able
to put a pci bus on a device != PCI_HOST_BRIDGE. If I recall it correctly
the pci bus code wants to collect all bridges in a list.
I have to do some more research to find out if it is possible to change
this ...

> 
> Alex
> 

Re: [Qemu-devel] [PATCH RFC 1/1] s390x/pci: Extend pci representation by new zpci device

[Frank Blaschka]

On Wed, Mar 04, 2015 at 04:58:25PM +0100, Frank Blaschka wrote:
> On Wed, Mar 04, 2015 at 04:25:07PM +0100, Alexander Graf wrote:
> > 
> > 
> > On 04.03.15 16:07, Frank Blaschka wrote:
> > > On Wed, Mar 04, 2015 at 03:49:15PM +0100, Alexander Graf wrote:
> > >>
> > >>
> > >> On 04.03.15 14:44, Frank Blaschka wrote:
> > >>> On Tue, Mar 03, 2015 at 09:38:37PM +0100, Alexander Graf wrote:
> > >>>>
> > >>>>
> > >>>> On 03.03.15 14:25, Frank Blaschka wrote:
> > >>>>> On Tue, Mar 03, 2015 at 10:33:05AM +0100, Alexander Graf wrote:
> > >>>>>>
> > >>>>>>
> > >>>>>>
> > >>>>>>> Am 03.03.2015 um 09:06 schrieb Frank Blaschka <blaschka@linux.vnet.ibm.com>:
> > >>>>>>>
> > >>>>>>>> On Thu, Feb 26, 2015 at 04:34:06PM +0100, Alexander Graf wrote:
> > >>>>>>>>
> > >>>>>>>>
> > >>>>>>>>> On 26.02.15 16:27, Frank Blaschka wrote:
> > >>>>>>>>>> On Thu, Feb 26, 2015 at 03:39:15PM +0100, Alexander Graf wrote:
> > >>>>>>>>>>
> > >>>>>>>>>>
> > >>>>>>>>>>> On 26.02.15 12:59, Frank Blaschka wrote:
> > >>>>>>>>>>> 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.
> > >>>>>>>>>>>
> > >>>>>>>>>>> On s390 the physical structure of the pci system (bridge, bus, slot)
> > >>>>>>>>>>> in not shown to the OS. For this the pci bridge and bus created
> > >>>>>>>>>>> in qemu can also not be shown to the guest. The new zpci device class
> > >>>>>>>>>>> represents this abstract view on the bare pci function and allows to
> > >>>>>>>>>>> provide s390 specific configuration attributes for it.
> > >>>>>>>>>>>
> > >>>>>>>>>>> Sample qemu configuration:
> > >>>>>>>>>>> -device e1000,id=zpci1
> > >>>>>>>>>>> -device ne2k_pci,id=zpci2
> > >>>>>>>>>>> -device zpci,fid=2,uid=1248,pci_id=zpci1
> > >>>>>>>>>>> -device zpci,fid=17,uid=2244,pci_id=zpci2
> > >>>>>>>>>>>
> > >>>>>>>>>>> A zpci device references the corresponding PCI device via device id.
> > >>>>>>>>>>> The new design allows to define multiple host bridges and support more
> > >>>>>>>>>>> pci devices.
> > >>>>>>>>>>
> > >>>>>>>>>> Isn't this reverse? Shouldn't it rather be
> > >>>>>>>>>>
> > >>>>>>>>>>  -device zpci,...,id=zpci1
> > >>>>>>>>>>  -device e1000,bus=zpci1.0
> > >>>>>>>>>>
> > >>>>>>>>>> with a limit on each virtual zpci bus to only support one device?
> > >>>>>>>>>
> > >>>>>>>>> Do you mean something like having multiple host bridges (providing a pci bus
> > >>>>>>>>> each) and limit the bus to just one device?
> > >>>>>>>>>
> > >>>>>>>>> -device s390-pcihost,fid=16,uid=1234
> > >>>>>>>>> -device s390-pcihost,fid=17,uid=5678
> > >>>>>>>>> -device e1000,bus=pci.0
> > >>>>>>>>> -device ne2k_pci,bus=pci.1
> > >>>>>>>>>
> > >>>>>>>>> We also discussed this option but we don't like the idea to put attributes
> > >>>>>>>>> belong to the pci device to the host bridge.
> > >>>>>>>>
> > >>>>>>>> I guess I'm not grasping something obvious here :). What exactly are the
> > >>>>>>>> attributes again?
> > >>>>>>> Sorry for the late response, I was on vacation the last couple days.
> > >>>>>>>
> > >>>>>>> The fid and uid values are provided by microcode/io layer on the real hardware.
> > >>>>>>
> > >>>>>> So they are arbitrary numbers? What uniqueness constraints do we have on them?
> > >>>>> fid and uid must be unique within the same qemu. At a first look the numbers are
> > >>>>> arbitrary but our configuration folks want explicitly define a particular fid and uid
> > >>>>> to better support migration and pass-through scenarios.
> > >>>>
> > >>>> Well, at the end of the day you want to make sure they're identical on
> > >>>> both sides, yes.
> > >>>>
> > >>>>>> IIUC you can only have a single pcie device behind a virtual "bus" anyway, so what if we just calculate uid and fid from the bus id?
> > >>>>> I think this similar to the current implementation. We use the slot (idea for the future was
> > >>>>> bus + slot) to generate uid and fid. But this is not flexible enough. As I said, our
> > >>>>> configuration folks want to be able to specify fid and uid for the device.
> > >>>>
> > >>>> I don't see how this is different from what PPC does with its LIOBN
> > >>>> which is a property of the PHB.
> > >>>>
> > >>>>
> > >>>> Alex
> > >>>>
> > >>>
> > >>> I played arround with the idea of having multiple host bridges and this worked well
> > >>> at least for static (non hotplug) configuration. In case I want to hotplug a host
> > >>> bridge I got following error:
> > >>>
> > >>> (qemu) device_add s390-pcihost,fid=8,uid=9
> > >>> Bus 'main-system-bus' does not support hotplugging
> > >>>
> > >>> Is there anything I have to enable to support this?
> > >>>
> > >>> I have: has_dynamic_sysbus = 1 and cannot_instantiate_with_device_add_yet = false
> > >>> but this seems not to help for the hotplug case.
> > >>
> > >> Having s390 devices reside on sysbus is probably a bad idea. Instead,
> > >> they should be on an s390 specific bus which then can implement hotplug
> > >> easily.
> > >>
> > >>
> > >> Alex
> > >>
> > > 
> > > Hm now I get lost ...
> > > 
> > > Do you suggest we should implement a s390 specific device (which is not derived from
> > > TYPE_PCI_HOST_BRIDGE) but implements a pci bus so we can attach a pci device to this
> > > device?  
> > 
> > Ugh, PCI_HOST_BRIDGE is a sysbus device. Awesome.
> > 
> > Conceptually your PCI bridge is not a sysbus device, since it doesn't
> > live on a flat MMIO + legacy IRQ routing bus. Instead, it lives on its
> > own thing that handles MMIO and IRQs via special backdoor interfaces.
> >
> well spoken :-)
> 
> > How much of the PCI_HOST_BRIDGE device are you actually using? Would it
> > be a lot of effort to have another s390 specific device that exposes a
> > PCIBus, but is not of type PCI_HOST_BRIDGE (and thus sysbus)?
> > 
> I do not use much functionality of the PCI_HOST_BRIDGE but I was not able
> to put a pci bus on a device != PCI_HOST_BRIDGE. If I recall it correctly
> the pci bus code wants to collect all bridges in a list.
> I have to do some more research to find out if it is possible to change
> this ...
>

I have implemented your idea (I can provide the patch later if you want) but had
to change pci code to allow to have a pci bus without a host bridge. I don't
know if this makes sense at all or if this breaks some general concept. Since
my code does not use any functionality of the host bridge following patch seems
to be sufficient for me.

Can anybody with more experience in qemu pci and host bridge code comment on this?

Thx!

---
 hw/pci/pci.c |   24 ++++++++++++++++--------
 1 file changed, 16 insertions(+), 8 deletions(-)

--- a/hw/pci/pci.c
+++ b/hw/pci/pci.c
@@ -253,9 +253,11 @@ static void pcibus_reset(BusState *qbus)

 static void pci_host_bus_register(PCIBus *bus, DeviceState *parent)
 {
-    PCIHostState *host_bridge = PCI_HOST_BRIDGE(parent);
-
-    QLIST_INSERT_HEAD(&pci_host_bridges, host_bridge, next);
+    PCIHostState *host_bridge = (PCIHostState *)object_dynamic_cast(
+                                OBJECT(parent), TYPE_PCI_HOST_BRIDGE);
+    if (host_bridge) {
+        QLIST_INSERT_HEAD(&pci_host_bridges, host_bridge, next);
+    }
 }

 PCIBus *pci_find_primary_bus(void)
@@ -288,14 +290,20 @@ PCIBus *pci_device_root_bus(const PCIDev
 const char *pci_root_bus_path(PCIDevice *dev)
 {
     PCIBus *rootbus = pci_device_root_bus(dev);
-    PCIHostState *host_bridge = PCI_HOST_BRIDGE(rootbus->qbus.parent);
-    PCIHostBridgeClass *hc = PCI_HOST_BRIDGE_GET_CLASS(host_bridge);
+    PCIHostState *host_bridge;
+    PCIHostBridgeClass *hc;
+
+    host_bridge = (PCIHostState *)object_dynamic_cast(
+                  OBJECT(rootbus->qbus.parent), TYPE_PCI_HOST_BRIDGE);

     assert(!rootbus->parent_dev);
-    assert(host_bridge->bus == rootbus);

-    if (hc->root_bus_path) {
-        return (*hc->root_bus_path)(host_bridge, rootbus);
+    if (host_bridge) {
+        assert(host_bridge->bus == rootbus);
+        hc = PCI_HOST_BRIDGE_GET_CLASS(host_bridge);
+        if (hc->root_bus_path) {
+            return (*hc->root_bus_path)(host_bridge, rootbus);
+        }
     }

     return rootbus->qbus.name;
> 
> 

Re: [Qemu-devel] [PATCH RFC 1/1] s390x/pci: Extend pci representation by new zpci device

[Alexander Graf]

On 06.03.15 11:34, Frank Blaschka wrote:
> On Wed, Mar 04, 2015 at 04:58:25PM +0100, Frank Blaschka wrote:
>> On Wed, Mar 04, 2015 at 04:25:07PM +0100, Alexander Graf wrote:
>>>
>>>
>>> On 04.03.15 16:07, Frank Blaschka wrote:
>>>> On Wed, Mar 04, 2015 at 03:49:15PM +0100, Alexander Graf wrote:
>>>>>
>>>>>
>>>>> On 04.03.15 14:44, Frank Blaschka wrote:
>>>>>> On Tue, Mar 03, 2015 at 09:38:37PM +0100, Alexander Graf wrote:
>>>>>>>
>>>>>>>
>>>>>>> On 03.03.15 14:25, Frank Blaschka wrote:
>>>>>>>> On Tue, Mar 03, 2015 at 10:33:05AM +0100, Alexander Graf wrote:
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>> Am 03.03.2015 um 09:06 schrieb Frank Blaschka <blaschka@linux.vnet.ibm.com>:
>>>>>>>>>>
>>>>>>>>>>> On Thu, Feb 26, 2015 at 04:34:06PM +0100, Alexander Graf wrote:
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>>> On 26.02.15 16:27, Frank Blaschka wrote:
>>>>>>>>>>>>> On Thu, Feb 26, 2015 at 03:39:15PM +0100, Alexander Graf wrote:
>>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>>>> On 26.02.15 12:59, Frank Blaschka wrote:
>>>>>>>>>>>>>> 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.
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> On s390 the physical structure of the pci system (bridge, bus, slot)
>>>>>>>>>>>>>> in not shown to the OS. For this the pci bridge and bus created
>>>>>>>>>>>>>> in qemu can also not be shown to the guest. The new zpci device class
>>>>>>>>>>>>>> represents this abstract view on the bare pci function and allows to
>>>>>>>>>>>>>> provide s390 specific configuration attributes for it.
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> Sample qemu configuration:
>>>>>>>>>>>>>> -device e1000,id=zpci1
>>>>>>>>>>>>>> -device ne2k_pci,id=zpci2
>>>>>>>>>>>>>> -device zpci,fid=2,uid=1248,pci_id=zpci1
>>>>>>>>>>>>>> -device zpci,fid=17,uid=2244,pci_id=zpci2
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> A zpci device references the corresponding PCI device via device id.
>>>>>>>>>>>>>> The new design allows to define multiple host bridges and support more
>>>>>>>>>>>>>> pci devices.
>>>>>>>>>>>>>
>>>>>>>>>>>>> Isn't this reverse? Shouldn't it rather be
>>>>>>>>>>>>>
>>>>>>>>>>>>>  -device zpci,...,id=zpci1
>>>>>>>>>>>>>  -device e1000,bus=zpci1.0
>>>>>>>>>>>>>
>>>>>>>>>>>>> with a limit on each virtual zpci bus to only support one device?
>>>>>>>>>>>>
>>>>>>>>>>>> Do you mean something like having multiple host bridges (providing a pci bus
>>>>>>>>>>>> each) and limit the bus to just one device?
>>>>>>>>>>>>
>>>>>>>>>>>> -device s390-pcihost,fid=16,uid=1234
>>>>>>>>>>>> -device s390-pcihost,fid=17,uid=5678
>>>>>>>>>>>> -device e1000,bus=pci.0
>>>>>>>>>>>> -device ne2k_pci,bus=pci.1
>>>>>>>>>>>>
>>>>>>>>>>>> We also discussed this option but we don't like the idea to put attributes
>>>>>>>>>>>> belong to the pci device to the host bridge.
>>>>>>>>>>>
>>>>>>>>>>> I guess I'm not grasping something obvious here :). What exactly are the
>>>>>>>>>>> attributes again?
>>>>>>>>>> Sorry for the late response, I was on vacation the last couple days.
>>>>>>>>>>
>>>>>>>>>> The fid and uid values are provided by microcode/io layer on the real hardware.
>>>>>>>>>
>>>>>>>>> So they are arbitrary numbers? What uniqueness constraints do we have on them?
>>>>>>>> fid and uid must be unique within the same qemu. At a first look the numbers are
>>>>>>>> arbitrary but our configuration folks want explicitly define a particular fid and uid
>>>>>>>> to better support migration and pass-through scenarios.
>>>>>>>
>>>>>>> Well, at the end of the day you want to make sure they're identical on
>>>>>>> both sides, yes.
>>>>>>>
>>>>>>>>> IIUC you can only have a single pcie device behind a virtual "bus" anyway, so what if we just calculate uid and fid from the bus id?
>>>>>>>> I think this similar to the current implementation. We use the slot (idea for the future was
>>>>>>>> bus + slot) to generate uid and fid. But this is not flexible enough. As I said, our
>>>>>>>> configuration folks want to be able to specify fid and uid for the device.
>>>>>>>
>>>>>>> I don't see how this is different from what PPC does with its LIOBN
>>>>>>> which is a property of the PHB.
>>>>>>>
>>>>>>>
>>>>>>> Alex
>>>>>>>
>>>>>>
>>>>>> I played arround with the idea of having multiple host bridges and this worked well
>>>>>> at least for static (non hotplug) configuration. In case I want to hotplug a host
>>>>>> bridge I got following error:
>>>>>>
>>>>>> (qemu) device_add s390-pcihost,fid=8,uid=9
>>>>>> Bus 'main-system-bus' does not support hotplugging
>>>>>>
>>>>>> Is there anything I have to enable to support this?
>>>>>>
>>>>>> I have: has_dynamic_sysbus = 1 and cannot_instantiate_with_device_add_yet = false
>>>>>> but this seems not to help for the hotplug case.
>>>>>
>>>>> Having s390 devices reside on sysbus is probably a bad idea. Instead,
>>>>> they should be on an s390 specific bus which then can implement hotplug
>>>>> easily.
>>>>>
>>>>>
>>>>> Alex
>>>>>
>>>>
>>>> Hm now I get lost ...
>>>>
>>>> Do you suggest we should implement a s390 specific device (which is not derived from
>>>> TYPE_PCI_HOST_BRIDGE) but implements a pci bus so we can attach a pci device to this
>>>> device?  
>>>
>>> Ugh, PCI_HOST_BRIDGE is a sysbus device. Awesome.
>>>
>>> Conceptually your PCI bridge is not a sysbus device, since it doesn't
>>> live on a flat MMIO + legacy IRQ routing bus. Instead, it lives on its
>>> own thing that handles MMIO and IRQs via special backdoor interfaces.
>>>
>> well spoken :-)
>>
>>> How much of the PCI_HOST_BRIDGE device are you actually using? Would it
>>> be a lot of effort to have another s390 specific device that exposes a
>>> PCIBus, but is not of type PCI_HOST_BRIDGE (and thus sysbus)?
>>>
>> I do not use much functionality of the PCI_HOST_BRIDGE but I was not able
>> to put a pci bus on a device != PCI_HOST_BRIDGE. If I recall it correctly
>> the pci bus code wants to collect all bridges in a list.
>> I have to do some more research to find out if it is possible to change
>> this ...
>>
> 
> I have implemented your idea (I can provide the patch later if you want) but had
> to change pci code to allow to have a pci bus without a host bridge. I don't
> know if this makes sense at all or if this breaks some general concept. Since
> my code does not use any functionality of the host bridge following patch seems
> to be sufficient for me.
> 
> Can anybody with more experience in qemu pci and host bridge code comment on this?

I think it's reasonable to detangle the Sysbus PCI bridges from PCIBuses.

Michael, what do you think?


Alex

> 
> Thx!
> 
> ---
>  hw/pci/pci.c |   24 ++++++++++++++++--------
>  1 file changed, 16 insertions(+), 8 deletions(-)
> 
> --- a/hw/pci/pci.c
> +++ b/hw/pci/pci.c
> @@ -253,9 +253,11 @@ static void pcibus_reset(BusState *qbus)
> 
>  static void pci_host_bus_register(PCIBus *bus, DeviceState *parent)
>  {
> -    PCIHostState *host_bridge = PCI_HOST_BRIDGE(parent);
> -
> -    QLIST_INSERT_HEAD(&pci_host_bridges, host_bridge, next);
> +    PCIHostState *host_bridge = (PCIHostState *)object_dynamic_cast(
> +                                OBJECT(parent), TYPE_PCI_HOST_BRIDGE);
> +    if (host_bridge) {
> +        QLIST_INSERT_HEAD(&pci_host_bridges, host_bridge, next);
> +    }
>  }
> 
>  PCIBus *pci_find_primary_bus(void)
> @@ -288,14 +290,20 @@ PCIBus *pci_device_root_bus(const PCIDev
>  const char *pci_root_bus_path(PCIDevice *dev)
>  {
>      PCIBus *rootbus = pci_device_root_bus(dev);
> -    PCIHostState *host_bridge = PCI_HOST_BRIDGE(rootbus->qbus.parent);
> -    PCIHostBridgeClass *hc = PCI_HOST_BRIDGE_GET_CLASS(host_bridge);
> +    PCIHostState *host_bridge;
> +    PCIHostBridgeClass *hc;
> +
> +    host_bridge = (PCIHostState *)object_dynamic_cast(
> +                  OBJECT(rootbus->qbus.parent), TYPE_PCI_HOST_BRIDGE);
> 
>      assert(!rootbus->parent_dev);
> -    assert(host_bridge->bus == rootbus);
> 
> -    if (hc->root_bus_path) {
> -        return (*hc->root_bus_path)(host_bridge, rootbus);
> +    if (host_bridge) {
> +        assert(host_bridge->bus == rootbus);
> +        hc = PCI_HOST_BRIDGE_GET_CLASS(host_bridge);
> +        if (hc->root_bus_path) {
> +            return (*hc->root_bus_path)(host_bridge, rootbus);
> +        }
>      }
> 
>      return rootbus->qbus.name;
>>
>>
>