On Thu, May 09, 2013 at 10:31:09AM +1000, David Gibson wrote: > pci_find_domain() is used in a number of places where we want an id for a > whole PCI domain (i.e. the subtree under a PCI root bus). The trouble is > that many platforms may support multiple independent host bridges with no > hardware supplied notion of domain number. > > This patch, therefore, replaces calls to pci_find_domain() with calls to > a new pci_root_bus_path() returning a string. The new call is implemented > in terms of a new callback in the host bridge class, so it can be defined > in some way that's well defined for the platform. When no callback is > available we fall back on the qbus name. > > Most current uses of pci_find_domain() are for error or informational > messages, so the change in identifiers should be harmless. The exception > is pci_get_dev_path(), whose results form part of migration streams. To > maintain compatibility with old migration streams, the PIIX PCI host is > altered to always supply "0000" for this path, which matches the old domain > number (since the code didn't actually support domains other than 0). > > For the pseries (spapr) PCI bridge we use a different platform-unique > identifier (pseries machines can routinely have dozens of PCI host > bridges). Theoretically that breaks migration streams, but given that we > don't yet have migration support for pseries, it doesn't matter. > > Any other machines that have working migration support including PCI > devices will need to be updated to maintain migration stream compatibility. > > Signed-off-by: David Gibson <da...@gibson.dropbear.id.au>
AFAIK PC is the only one with working migration, yes, but we have Q35 as well which can be migrated. > --- > hw/pci-host/piix.c | 9 +++++++++ > hw/pci/pci-hotplug-old.c | 4 ++-- > hw/pci/pci.c | 38 ++++++++++++++++++++------------------ > hw/pci/pci_host.c | 1 + > hw/pci/pcie_aer.c | 8 ++++---- > hw/ppc/spapr_pci.c | 10 ++++++++++ > include/hw/pci/pci.h | 2 +- > include/hw/pci/pci_host.h | 10 ++++++++++ > 8 files changed, 57 insertions(+), 25 deletions(-) > > diff --git a/hw/pci-host/piix.c b/hw/pci-host/piix.c > index f9e68c3..c36e725 100644 > --- a/hw/pci-host/piix.c > +++ b/hw/pci-host/piix.c > @@ -629,11 +629,20 @@ static const TypeInfo i440fx_info = { > .class_init = i440fx_class_init, > }; > > +static const char *i440fx_pcihost_root_bus_path(PCIHostState *host_bridge, > + PCIBus *rootbus) > +{ > + /* For backwards compat with old device paths */ > + return "0000"; > +} > + > static void i440fx_pcihost_class_init(ObjectClass *klass, void *data) > { > DeviceClass *dc = DEVICE_CLASS(klass); > SysBusDeviceClass *k = SYS_BUS_DEVICE_CLASS(klass); > + PCIHostBridgeClass *hc = PCI_HOST_BRIDGE_CLASS(klass); > > + hc->root_bus_path = i440fx_pcihost_root_bus_path; > k->init = i440fx_pcihost_initfn; > dc->fw_name = "pci"; > dc->no_user = 1; > diff --git a/hw/pci/pci-hotplug-old.c b/hw/pci/pci-hotplug-old.c > index 98b4c18..d26674d 100644 > --- a/hw/pci/pci-hotplug-old.c > +++ b/hw/pci/pci-hotplug-old.c > @@ -273,8 +273,8 @@ void pci_device_hot_add(Monitor *mon, const QDict *qdict) > } > > if (dev) { > - monitor_printf(mon, "OK domain %d, bus %d, slot %d, function %d\n", > - pci_find_domain(dev), > + monitor_printf(mon, "OK root bus %s, bus %d, slot %d, function %d\n", > + pci_root_bus_path(dev), > pci_bus_num(dev->bus), PCI_SLOT(dev->devfn), > PCI_FUNC(dev->devfn)); > } else > diff --git a/hw/pci/pci.c b/hw/pci/pci.c > index f1cee73..a3c192c 100644 > --- a/hw/pci/pci.c > +++ b/hw/pci/pci.c > @@ -25,6 +25,7 @@ > #include "hw/pci/pci.h" > #include "hw/pci/pci_bridge.h" > #include "hw/pci/pci_bus.h" > +#include "hw/pci/pci_host.h" > #include "monitor/monitor.h" > #include "net/net.h" > #include "sysemu/sysemu.h" > @@ -270,19 +271,20 @@ PCIBus *pci_device_root_bus(const PCIDevice *d) > return bus; > } > > -int pci_find_domain(const PCIDevice *dev) > +const char *pci_root_bus_path(PCIDevice *dev) > { > - const PCIBus *rootbus = pci_device_root_bus(dev); > - struct PCIHostBus *host; > + 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); > > - QLIST_FOREACH(host, &host_buses, next) { > - if (host->bus == rootbus) { > - return host->domain; > - } > + assert(!rootbus->parent_dev); > + assert(host_bridge->bus == rootbus); > + > + if (hc->root_bus_path) { > + return (*hc->root_bus_path)(host_bridge, rootbus); > } > > - abort(); /* should not be reached */ > - return -1; > + return rootbus->qbus.name; > } > > static void pci_bus_init(PCIBus *bus, DeviceState *parent, > @@ -2005,10 +2007,10 @@ int pci_add_capability(PCIDevice *pdev, uint8_t > cap_id, > for (i = offset; i < offset + size; i++) { > overlapping_cap = pci_find_capability_at_offset(pdev, i); > if (overlapping_cap) { > - fprintf(stderr, "ERROR: %04x:%02x:%02x.%x " > + fprintf(stderr, "ERROR: %s:%02x:%02x.%x " > "Attempt to add PCI capability %x at offset " > "%x overlaps existing capability %x at offset %x\n", > - pci_find_domain(pdev), pci_bus_num(pdev->bus), > + pci_root_bus_path(pdev), pci_bus_num(pdev->bus), > PCI_SLOT(pdev->devfn), PCI_FUNC(pdev->devfn), > cap_id, offset, overlapping_cap, i); > return -EINVAL; > @@ -2142,30 +2144,30 @@ static char *pcibus_get_dev_path(DeviceState *dev) > * domain:Bus:Slot.Func for systems without nested PCI bridges. > * Slot.Function list specifies the slot and function numbers for all > * devices on the path from root to the specific device. */ > - char domain[] = "DDDD:00"; > + const char *root_bus_path; > + int root_bus_len; > char slot[] = ":SS.F"; > - int domain_len = sizeof domain - 1 /* For '\0' */; > int slot_len = sizeof slot - 1 /* For '\0' */; > int path_len; > char *path, *p; > int s; > > + root_bus_path = pci_root_bus_path(d); > + root_bus_len = strlen(root_bus_path); > + > /* Calculate # of slots on path between device and root. */; > slot_depth = 0; > for (t = d; t; t = t->bus->parent_dev) { > ++slot_depth; > } > > - path_len = domain_len + slot_len * slot_depth; > + path_len = root_bus_len + slot_len * slot_depth; > > /* Allocate memory, fill in the terminating null byte. */ > path = g_malloc(path_len + 1 /* For '\0' */); > path[path_len] = '\0'; > > - /* First field is the domain. */ > - s = snprintf(domain, sizeof domain, "%04x:00", pci_find_domain(d)); > - assert(s == domain_len); > - memcpy(path, domain, domain_len); > + memcpy(path, root_bus_path, root_bus_len); > > /* Fill in slot numbers. We walk up from device to root, so need to print > * them in the reverse order, last to first. */ > diff --git a/hw/pci/pci_host.c b/hw/pci/pci_host.c > index 12254b1..7dd9b25 100644 > --- a/hw/pci/pci_host.c > +++ b/hw/pci/pci_host.c > @@ -169,6 +169,7 @@ static const TypeInfo pci_host_type_info = { > .name = TYPE_PCI_HOST_BRIDGE, > .parent = TYPE_SYS_BUS_DEVICE, > .abstract = true, > + .class_size = sizeof(PCIHostBridgeClass), > .instance_size = sizeof(PCIHostState), > }; > > diff --git a/hw/pci/pcie_aer.c b/hw/pci/pcie_aer.c > index 06f77ac..ca762ab 100644 > --- a/hw/pci/pcie_aer.c > +++ b/hw/pci/pcie_aer.c > @@ -817,9 +817,9 @@ void pcie_aer_inject_error_print(Monitor *mon, const > QObject *data) > qdict = qobject_to_qdict(data); > > devfn = (int)qdict_get_int(qdict, "devfn"); > - monitor_printf(mon, "OK id: %s domain: %x, bus: %x devfn: %x.%x\n", > + monitor_printf(mon, "OK id: %s root bus: %s, bus: %x devfn: %x.%x\n", > qdict_get_str(qdict, "id"), > - (int) qdict_get_int(qdict, "domain"), > + qdict_get_str(qdict, "root_bus"), > (int) qdict_get_int(qdict, "bus"), > PCI_SLOT(devfn), PCI_FUNC(devfn)); > } > @@ -1020,9 +1020,9 @@ int do_pcie_aer_inject_error(Monitor *mon, > > ret = pcie_aer_inject_error(dev, &err); > *ret_data = qobject_from_jsonf("{'id': %s, " > - "'domain': %d, 'bus': %d, 'devfn': %d, " > + "'root_bus': %s, 'bus': %d, 'devfn': %d, " > "'ret': %d}", > - id, pci_find_domain(dev), > + id, pci_root_bus_path(dev), > pci_bus_num(dev->bus), dev->devfn, > ret); > assert(*ret_data); > diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c > index 62ff323..2d102f5 100644 > --- a/hw/ppc/spapr_pci.c > +++ b/hw/ppc/spapr_pci.c > @@ -693,11 +693,21 @@ static Property spapr_phb_properties[] = { > DEFINE_PROP_END_OF_LIST(), > }; > > +static const char *spapr_phb_root_bus_path(PCIHostState *host_bridge, > + PCIBus *rootbus) > +{ > + sPAPRPHBState *sphb = SPAPR_PCI_HOST_BRIDGE(host_bridge); > + > + return sphb->dtbusname; > +} > + > static void spapr_phb_class_init(ObjectClass *klass, void *data) > { > + PCIHostBridgeClass *hc = PCI_HOST_BRIDGE_CLASS(klass); > SysBusDeviceClass *sdc = SYS_BUS_DEVICE_CLASS(klass); > DeviceClass *dc = DEVICE_CLASS(klass); > > + hc->root_bus_path = spapr_phb_root_bus_path; > sdc->init = spapr_phb_init; > dc->props = spapr_phb_properties; > dc->reset = spapr_phb_reset; > diff --git a/include/hw/pci/pci.h b/include/hw/pci/pci.h > index 1383cfe..1b50dc0 100644 > --- a/include/hw/pci/pci.h > +++ b/include/hw/pci/pci.h > @@ -392,7 +392,7 @@ void pci_for_each_device(PCIBus *bus, int bus_num, > void *opaque); > PCIBus *pci_get_primary_bus(void); > PCIBus *pci_device_root_bus(const PCIDevice *d); > -int pci_find_domain(const PCIDevice *dev); > +const char *pci_root_bus_path(PCIDevice *dev); > PCIDevice *pci_find_device(PCIBus *bus, int bus_num, uint8_t devfn); > int pci_qdev_find_device(const char *id, PCIDevice **pdev); > PCIBus *pci_get_bus_devfn(int *devfnp, const char *devaddr); > diff --git a/include/hw/pci/pci_host.h b/include/hw/pci/pci_host.h > index 236cd0f..44052f2 100644 > --- a/include/hw/pci/pci_host.h > +++ b/include/hw/pci/pci_host.h > @@ -33,6 +33,10 @@ > #define TYPE_PCI_HOST_BRIDGE "pci-host-bridge" > #define PCI_HOST_BRIDGE(obj) \ > OBJECT_CHECK(PCIHostState, (obj), TYPE_PCI_HOST_BRIDGE) > +#define PCI_HOST_BRIDGE_CLASS(klass) \ > + OBJECT_CLASS_CHECK(PCIHostBridgeClass, (klass), TYPE_PCI_HOST_BRIDGE) > +#define PCI_HOST_BRIDGE_GET_CLASS(obj) \ > + OBJECT_GET_CLASS(PCIHostBridgeClass, (obj), TYPE_PCI_HOST_BRIDGE) > > struct PCIHostState { > SysBusDevice busdev; > @@ -44,6 +48,12 @@ struct PCIHostState { > PCIBus *bus; > }; > > +typedef struct PCIHostBridgeClass { > + SysBusDeviceClass parent_class; > + > + const char *(*root_bus_path)(PCIHostState *, PCIBus *); > +} PCIHostBridgeClass; > + > /* common internal helpers for PCI/PCIe hosts, cut off overflows */ > void pci_host_config_write_common(PCIDevice *pci_dev, uint32_t addr, > uint32_t limit, uint32_t val, uint32_t > len); > -- > 1.7.10.4