[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 5/8] pci: Replace pci_find_domain() with more ge
From: |
Michael S. Tsirkin |
Subject: |
Re: [Qemu-devel] [PATCH 5/8] pci: Replace pci_find_domain() with more general pci_root_bus_path() |
Date: |
Thu, 23 May 2013 14:04:08 +0300 |
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 <address@hidden>
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
- Re: [Qemu-devel] [PATCH 6/8] pci: Simpler implementation of primary PCI bus, (continued)
Re: [Qemu-devel] [PATCH 6/8] pci: Simpler implementation of primary PCI bus, Michael S. Tsirkin, 2013/05/23
[Qemu-devel] [PATCH 4/8] pci: Use helper o find device's root bus in pci_find_domain(), David Gibson, 2013/05/08
[Qemu-devel] [PATCH 3/8] pci: Abolish pci_find_root_bus(), David Gibson, 2013/05/08
[Qemu-devel] [PATCH 7/8] pci: Remove domain from PCIHostBus, David Gibson, 2013/05/08
[Qemu-devel] [PATCH 8/8] pci: Fold host_buses list into PCIHostState functionality, David Gibson, 2013/05/08
[Qemu-devel] [PATCH 5/8] pci: Replace pci_find_domain() with more general pci_root_bus_path(), David Gibson, 2013/05/08
- Re: [Qemu-devel] [PATCH 5/8] pci: Replace pci_find_domain() with more general pci_root_bus_path(),
Michael S. Tsirkin <=
- Re: [Qemu-devel] [PATCH 5/8] pci: Replace pci_find_domain() with more general pci_root_bus_path(), David Gibson, 2013/05/23
- Re: [Qemu-devel] [PATCH 5/8] pci: Replace pci_find_domain() with more general pci_root_bus_path(), Paolo Bonzini, 2013/05/23
- Re: [Qemu-devel] [PATCH 5/8] pci: Replace pci_find_domain() with more general pci_root_bus_path(), Michael S. Tsirkin, 2013/05/23
- Re: [Qemu-devel] [PATCH 5/8] pci: Replace pci_find_domain() with more general pci_root_bus_path(), Paolo Bonzini, 2013/05/23
- Re: [Qemu-devel] [PATCH 5/8] pci: Replace pci_find_domain() with more general pci_root_bus_path(), Michael S. Tsirkin, 2013/05/23
- Re: [Qemu-devel] [PATCH 5/8] pci: Replace pci_find_domain() with more general pci_root_bus_path(), David Gibson, 2013/05/24
[Qemu-devel] [PATCH 1/8] pci: Cleanup configuration for pci-hotplug.c, David Gibson, 2013/05/08