Skip to contentSkip to conversation

standard pci bridge device

PATCH In review Michael S. Tsirkin 3 participants 7 messages Feb 20, 2012 – Feb 27, 2012

Subsystems (2)
lore
Share & tools
lore
#0 lore raw

v3 cover standard pci bridge device

Here's a new version of the patch.

TODOs:
- windows guest testing

Changes from v2:
- added slot id capability
- migration support
- misc fixes
- fix checkpatch errors

I also include a patch to make all bridges have a user-friendly id. You don't absolutely have to have it for testing,
but this makes use easier. Without it, you must replace bus=<id> in instructions below with bus=<id>.0
(add ".0" literally after the id).

Changes from v1:
- hotplug support

Status: tested with a linux guest on x86.
Deep nesting of bridges is supported.
You need a small BIOS patch to support the OSHP method if you want hotplug to work. Posted separately. We'd need a full ACPI driver to make hotplug work for guests without an SHPC driver (e.g. windows XP). Management support will also be needed.

One small wrinkle is that the pci_addr property
used for pci_add command wants data in a format
bus:device.function which is
broken as guests can change bus numbers.
I don't intend to fix it short term.
So, don't use the pci_add command.
Only device_add works with a bridge.

(Side note: the 'addr' property
encodes slot*8+function#. It is not user-friendly. Let's add optional slot= func= ?)

The SHPC controller supports up to 31 devices (out of 32 slots) so slot 0 doesn't support hotplug. Non hot-pluggable devices behind the bridge don't work currectly (we'll try to unplug them) so don't do this.
For now I just blocked adding devices in slot 0, in the future it might be possible to add
a non-hotpluggable device there.

Each bridge must have a unique chassis #. You *must* set it to a value > 0.
I don't check that it is unique, just that it is >0. If you make it non unique, guest will be confused.

Example:

qemu-system-x86_64 -enable-kvm -m 1G
-drive file=/home/mst/rhel6.qcow2
-netdev tap,id=foo,ifname=msttap0,script=/home/mst/ifup,downscript=no,vhost=on -device pci-bridge,id=bog,chassis_nr=1
-device virtio-net-pci,netdev=foo,bus=bog,addr=8

Hot-unplug currently causes qemu to crash, this happens without this patch too, so I'm not worried :)

Michael S. Tsirkin (4):
pci_bridge: user-friendly default bus name
shpc: standard hot plug controller
slotid: add slot id capability
pci: add standard bridge device

 Makefile.objs       |    4 +-
 hw/pci.h            |    8 +
 hw/pci_bridge.c     |   10 +
 hw/pci_bridge_dev.c |  151 ++++++++++++
 hw/shpc.c           |  673 +++++++++++++++++++++++++++++++++++++++++++++++++++
 hw/shpc.h           |   45 ++++
 hw/slotid_cap.c     |   44 ++++
 hw/slotid_cap.h     |   11 +
 qemu-common.h       |    1 +
 9 files changed, 946 insertions(+), 1 deletions(-)
 create mode 100644 hw/pci_bridge_dev.c
 create mode 100644 hw/shpc.c
 create mode 100644 hw/shpc.h
 create mode 100644 hw/slotid_cap.c
 create mode 100644 hw/slotid_cap.h
Signature · 1 line
1.7.9.111.gf3fb0
#1 lore raw

PATCH v3 1/4 pci_bridge: user-friendly default bus name

For a pci bridge device, if we don't override the name with custom code, the bus will be addressed as <id>.0, where id is the id specified by the user.

Since PCI Bridge devices have a single bus each, we don't need the index: address the bus using the parent device name. This is better since this way users don't care about our internal bus/device distinctions.

As far as I could see, we only have built-in
bridges at this point which always override the
name. So this change will only affect ioh3420.c.

---
 hw/pci_bridge.c |   10 ++++++++++
 1 files changed, 10 insertions(+), 0 deletions(-)
hw/pci_bridge.c +10 -0
index fea3873..eeee8a6 100644
@@ -314,6 +314,16 @@ int pci_bridge_initfn(PCIDevice *dev)
     pci_set_word(dev->config + PCI_SEC_STATUS,
                  PCI_STATUS_66MHZ | PCI_STATUS_FAST_BACK);
 
+    /*
+     * If we don't specify the name, the bus will be addressed as <id>.0, where
+     * id is the device id.
+     * Since PCI Bridge devices have a single bus each, we don't need the index:
+     * let users address the bus using the device name.
+     */
+    if (!br->bus_name && dev->qdev.id && *dev->qdev.id) {
+	    br->bus_name = dev->qdev.id;
+    }
+
     qbus_create_inplace(&sec_bus->qbus, &pci_bus_info, &dev->qdev,
                         br->bus_name);
     sec_bus->parent_dev = dev;
Signature · 1 line
1.7.9.111.gf3fb0
Reviewed-by on v3 1/4
#2 lore raw

Re: [PATCHv3 1/4] pci_bridge: user-friendly default bus name

Quoted 9 lines · Michael S. Tsirkin

For a pci bridge device, if we don't override the name with custom code, the bus will be addressed as <id>.0, where id is the id specified by the user.

Since PCI Bridge devices have a single bus each, we don't need the index: address the bus using the parent device name. This is better since this way users don't care about our internal bus/device distinctions.

Quoted 5 lines

As far as I could see, we only have built-in
bridges at this point which always override the
name. So this change will only affect ioh3420.c.

I think we're safe here from a compatibility perspective.

Regards,

Anthony Liguori

Quoted 25 lines
---
  hw/pci_bridge.c |   10 ++++++++++
  1 files changed, 10 insertions(+), 0 deletions(-)
hw/pci_bridge.c +10 -0
index fea3873..eeee8a6 100644
@@ -314,6 +314,16 @@ int pci_bridge_initfn(PCIDevice *dev)
      pci_set_word(dev->config + PCI_SEC_STATUS,
                   PCI_STATUS_66MHZ | PCI_STATUS_FAST_BACK);
 
+    /*
+     * If we don't specify the name, the bus will be addressed as<id>.0, where
+     * id is the device id.
+     * Since PCI Bridge devices have a single bus each, we don't need the index:
+     * let users address the bus using the device name.
+     */
+    if (!br->bus_name&&  dev->qdev.id&&  *dev->qdev.id) {
+	    br->bus_name = dev->qdev.id;
+    }
+
      qbus_create_inplace(&sec_bus->qbus,&pci_bus_info,&dev->qdev,
                          br->bus_name);
      sec_bus->parent_dev = dev;
#3 lore raw

PATCH v3 2/4 shpc: standard hot plug controller

In reply to Michael S. Tsirkin

This adds support for SHPC interface, as defined by PCI Standard Hot-Plug Controller and Subsystem Specification, Rev 1.0
http://www.pcisig.com/specifications/conventional/pci_hot_plug/SHPC_10

Only SHPC intergrated with a PCI-to-PCI bridge is supported, SHPC integrated with a host bridge would need more work.

All main SHPC features are supported:
- MRL sensor
- Attention button
- Attention indicator
- Power indicator

Wake on hotplug and serr generation are stubbed out but unused as we don't have interfaces to generate these events ATM.

One issue that isn't completely resolved is that qemu currently expects an "eject" interface, which SHPC does not provide: it merely removes the power to device and it's up to the user to remove the device from slot. This patch works around that by ejecting the device when power is removed and power LED goes off.

---
 Makefile.objs |    1 +
 hw/pci.h      |    6 +
 hw/shpc.c     |  673 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 hw/shpc.h     |   45 ++++
 qemu-common.h |    1 +
 5 files changed, 726 insertions(+), 0 deletions(-)
 create mode 100644 hw/shpc.c
 create mode 100644 hw/shpc.h
Makefile.objs +1 -0
index 391e524..4546477 100644
@@ -195,6 +195,7 @@ hw-obj-$(CONFIG_VIRTIO_PCI) += virtio-pci.o
 hw-obj-y += fw_cfg.o
 hw-obj-$(CONFIG_PCI) += pci.o pci_bridge.o
 hw-obj-$(CONFIG_PCI) += msix.o msi.o
+hw-obj-$(CONFIG_PCI) += shpc.o
 hw-obj-$(CONFIG_PCI) += pci_host.o pcie_host.o
 hw-obj-$(CONFIG_PCI) += ioh3420.o xio3130_upstream.o xio3130_downstream.o
 hw-obj-y += watchdog.o
hw/pci.h +6 -0
index 2a29654..05a7ebb 100644
@@ -125,6 +125,9 @@ enum {
     /* command register SERR bit enabled */
 #define QEMU_PCI_CAP_SERR_BITNR 4
     QEMU_PCI_CAP_SERR = (1 << QEMU_PCI_CAP_SERR_BITNR),
+    /* Standard hot plug controller. */
+#define QEMU_PCI_SHPC_BITNR 5
+    QEMU_PCI_CAP_SHPC = (1 << QEMU_PCI_SHPC_BITNR),
 };
 
 #define TYPE_PCI_DEVICE "pci-device"
@@ -229,6 +232,9 @@ struct PCIDevice {
     /* PCI Express */
     PCIExpressDevice exp;
 
+    /* SHPC */
+    SHPCDevice *shpc;
+
     /* Location of option rom */
     char *romfile;
     bool has_rom;
hw/shpc.c new file +673 -0

673 lines not on this page · raw message

hw/shpc.h new file +45 -0

45 lines not on this page · raw message

qemu-common.h +1 -0
index c5e9cad..dee4db3 100644
@@ -247,6 +247,7 @@ typedef struct SSIBus SSIBus;
 typedef struct EventNotifier EventNotifier;
 typedef struct VirtIODevice VirtIODevice;
 typedef struct QEMUSGList QEMUSGList;
+typedef struct SHPCDevice SHPCDevice;
 
 typedef uint64_t pcibus_t;
 
Signature · 1 line
1.7.9.111.gf3fb0
#4 lore raw

PATCH v3 3/4 slotid: add slot id capability

In reply to Michael S. Tsirkin

This capability makes it possible for the guest to report a unique chassis identifier to the user.

The spec also recommends making chassis indentifier persist in eeprom.
This isn't implemented.

---
 Makefile.objs   |    1 +
 hw/pci.h        |    2 ++
 hw/slotid_cap.c |   44 ++++++++++++++++++++++++++++++++++++++++++++
 hw/slotid_cap.h |   11 +++++++++++
 4 files changed, 58 insertions(+), 0 deletions(-)
 create mode 100644 hw/slotid_cap.c
 create mode 100644 hw/slotid_cap.h
Makefile.objs +1 -0
index 4546477..bd74eef 100644
@@ -196,6 +196,7 @@ hw-obj-y += fw_cfg.o
 hw-obj-$(CONFIG_PCI) += pci.o pci_bridge.o
 hw-obj-$(CONFIG_PCI) += msix.o msi.o
 hw-obj-$(CONFIG_PCI) += shpc.o
+hw-obj-$(CONFIG_PCI) += slotid_cap.o
 hw-obj-$(CONFIG_PCI) += pci_host.o pcie_host.o
 hw-obj-$(CONFIG_PCI) += ioh3420.o xio3130_upstream.o xio3130_downstream.o
 hw-obj-y += watchdog.o
hw/pci.h +2 -0
index 05a7ebb..448c44e 100644
@@ -128,6 +128,8 @@ enum {
     /* Standard hot plug controller. */
 #define QEMU_PCI_SHPC_BITNR 5
     QEMU_PCI_CAP_SHPC = (1 << QEMU_PCI_SHPC_BITNR),
+#define QEMU_PCI_SLOTID_BITNR 6
+    QEMU_PCI_CAP_SLOTID = (1 << QEMU_PCI_SLOTID_BITNR),
 };
 
 #define TYPE_PCI_DEVICE "pci-device"
hw/slotid_cap.c new file +44 -0

44 lines not on this page · raw message

hw/slotid_cap.h new file +11 -0
new file mode 100644
index 0000000..70db047
@@ -0,0 +1,11 @@
+#ifndef PCI_SLOTID_CAP_H
+#define PCI_SLOTID_CAP_H
+
+#include "qemu-common.h"
+
+int slotid_cap_init(PCIDevice *dev, int nslots,
+                    uint8_t chassis,
+                    unsigned offset);
+void slotid_cap_cleanup(PCIDevice *dev);
+
+#endif
Signature · 1 line
1.7.9.111.gf3fb0
#5 lore raw

PATCH v3 4/4 pci: add standard bridge device

In reply to Michael S. Tsirkin

This adds support for a standard pci to pci bridge, enabling support for more than 32 PCI devices in the system. Device hotplug is supported by means of SHPC controller. For guests with an SHPC driver, this allows robust hotplug and even hotplug of nested bridges, up to 31 devices per bridge.

---
 Makefile.objs       |    2 +-
 hw/pci_bridge_dev.c |  151 +++++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 152 insertions(+), 1 deletions(-)
 create mode 100644 hw/pci_bridge_dev.c
Makefile.objs +1 -1
index bd74eef..d6b43f3 100644
@@ -193,7 +193,7 @@ hw-obj-$(CONFIG_VIRTIO) += virtio-console.o
 hw-obj-y += usb-libhw.o
 hw-obj-$(CONFIG_VIRTIO_PCI) += virtio-pci.o
 hw-obj-y += fw_cfg.o
-hw-obj-$(CONFIG_PCI) += pci.o pci_bridge.o
+hw-obj-$(CONFIG_PCI) += pci.o pci_bridge.o pci_bridge_dev.o
 hw-obj-$(CONFIG_PCI) += msix.o msi.o
 hw-obj-$(CONFIG_PCI) += shpc.o
 hw-obj-$(CONFIG_PCI) += slotid_cap.o
hw/pci_bridge_dev.c new file +151 -0

151 lines not on this page · raw message

Signature · 1 line
1.7.9.111.gf3fb0
#6 lore raw
In reply to Michael S. Tsirkin
Quoted 10 lines · Michael S. Tsirkin

Here's a new version of the patch.

TODOs:
- windows guest testing

Changes from v2:
- added slot id capability
- migration support
- misc fixes
- fix checkpatch errors

64bit prefetch memory window works now:

00:10.0 PCI bridge: Red Hat, Inc. Device 0001 (prog-if 00 [Normal decode])
Physical Slot: 16
Flags: bus master, 66MHz, fast devsel, latency 0
Memory at f5126000 (32-bit, non-prefetchable) [size=256]
Bus: primary=00, secondary=01, subordinate=01, sec-latency=0 Memory behind bridge: f5000000-f50fffff
Prefetchable memory behind bridge: 00000000f8000000-00000000fbffffff
Capabilities: [48] Slot ID: 0 slots, First+, chassis 01
Capabilities: [40] Hot-plug capable
Kernel modules: shpchp

Looks good to me, did only light testing though.

cheers,
Gerd