* [Qemu-devel] [RFC 0/5] Assorted PCI/PCIe cleanups cleanups
@ 2017-10-03 9:14 David Gibson
2017-10-03 9:14 ` [Qemu-devel] [RFC 1/5] pci: Rename root bus initialization functions for clarity David Gibson
` (4 more replies)
0 siblings, 5 replies; 8+ messages in thread
From: David Gibson @ 2017-10-03 9:14 UTC (permalink / raw)
To: mst, ehabkost, marcel; +Cc: qemu-devel, alex.williamson, David Gibson
I've been reworking my patches to consolidate the handling of PCI/PCIe
"hybrid" devices based on feedback from my earlier series. I'm still
wrestling with some problems here, but along the way I've made some
cleanups which I think stand on their own. So, here they are.
I built these on top of Eduardo's series to advertise PCI and PCIe
capability of devices via interface names. I don't think I
fundamentally rely on anything in there, but if there are conflicts
against master, that'd be why.
David Gibson (5):
pci: Rename root bus initialization functions for clarity
pci: Move bridge data structures from pci_bus.h to pci_bridge.h
pci: Fold pci_bus.h into pci.h
pci: Simplify pci_bus_is_root()
pcie: Don't allow extended config space access via conventional PCI
bridges
hw/acpi/pcihp.c | 1 -
hw/alpha/typhoon.c | 8 ++--
hw/i386/acpi-build.c | 1 -
hw/i386/amd_iommu.h | 1 -
hw/i386/intel_iommu.c | 1 -
hw/i386/pc.c | 1 -
hw/isa/lpc_ich9.c | 1 -
hw/mips/gt64xxx_pci.c | 12 ++---
hw/pci-bridge/dec.c | 1 -
hw/pci-bridge/pci_bridge_dev.c | 1 -
hw/pci-bridge/pci_expander_bridge.c | 11 +----
hw/pci-bridge/pcie_pci_bridge.c | 1 -
hw/pci-host/apb.c | 11 ++---
hw/pci-host/bonito.c | 8 ++--
hw/pci-host/gpex.c | 6 +--
hw/pci-host/grackle.c | 14 +++---
hw/pci-host/piix.c | 4 +-
hw/pci-host/ppce500.c | 6 +--
hw/pci-host/prep.c | 5 +-
hw/pci-host/q35.c | 7 +--
hw/pci-host/uninorth.c | 24 +++++-----
hw/pci-host/versatile.c | 7 ++-
hw/pci-host/xilinx-pcie.c | 6 +--
hw/pci/pci.c | 71 ++++++++++++++--------------
hw/pci/pci_bridge.c | 1 -
hw/pci/pci_host.c | 1 -
hw/pci/pcie.c | 1 -
hw/pci/pcie_aer.c | 1 -
hw/pci/shpc.c | 1 -
hw/ppc/ppc4xx_pci.c | 6 +--
hw/ppc/spapr_pci.c | 9 ++--
hw/s390x/s390-pci-bus.c | 9 ++--
hw/sh4/sh_pci.c | 12 ++---
include/hw/i386/ich9.h | 1 -
include/hw/pci-host/xilinx-pcie.h | 2 +-
include/hw/pci/pci.h | 91 ++++++++++++++++++++++++++++-------
include/hw/pci/pci_bridge.h | 47 +++++++++++++++++++
include/hw/pci/pci_bus.h | 94 -------------------------------------
include/hw/pci/pcie_port.h | 1 -
39 files changed, 237 insertions(+), 249 deletions(-)
delete mode 100644 include/hw/pci/pci_bus.h
--
2.13.6
^ permalink raw reply [flat|nested] 8+ messages in thread
* [Qemu-devel] [RFC 1/5] pci: Rename root bus initialization functions for clarity
2017-10-03 9:14 [Qemu-devel] [RFC 0/5] Assorted PCI/PCIe cleanups cleanups David Gibson
@ 2017-10-03 9:14 ` David Gibson
2017-10-03 9:14 ` [Qemu-devel] [RFC 2/5] pci: Move bridge data structures from pci_bus.h to pci_bridge.h David Gibson
` (3 subsequent siblings)
4 siblings, 0 replies; 8+ messages in thread
From: David Gibson @ 2017-10-03 9:14 UTC (permalink / raw)
To: mst, ehabkost, marcel; +Cc: qemu-devel, alex.williamson, David Gibson
pci_bus_init(), pci_bus_new_inplace(), pci_bus_new() and pci_register_bus()
are misleadingly named. They're not used for initializing *any* PCI bus,
but only for a root PCI bus.
Non-root buses - i.e. ones under a logical PCI to PCI bridge - are instead
created with a direct qbus_create_inplace() (see pci_bridge_initfn()).
This patch renames the functions to make it clear they're only used for
a root bus.
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
hw/alpha/typhoon.c | 8 +++----
hw/mips/gt64xxx_pci.c | 12 +++++-----
hw/pci-bridge/pci_expander_bridge.c | 4 ++--
hw/pci-host/apb.c | 10 ++++----
hw/pci-host/bonito.c | 8 +++----
hw/pci-host/gpex.c | 6 ++---
hw/pci-host/grackle.c | 14 +++++------
hw/pci-host/piix.c | 4 ++--
hw/pci-host/ppce500.c | 6 ++---
hw/pci-host/prep.c | 4 ++--
hw/pci-host/q35.c | 7 +++---
hw/pci-host/uninorth.c | 24 +++++++++----------
hw/pci-host/versatile.c | 6 ++---
hw/pci-host/xilinx-pcie.c | 6 ++---
hw/pci/pci.c | 48 +++++++++++++++++++------------------
hw/ppc/ppc4xx_pci.c | 6 ++---
hw/ppc/spapr_pci.c | 8 +++----
hw/s390x/s390-pci-bus.c | 8 +++----
hw/sh4/sh_pci.c | 12 +++++-----
include/hw/pci/pci.h | 25 +++++++++----------
20 files changed, 115 insertions(+), 111 deletions(-)
diff --git a/hw/alpha/typhoon.c b/hw/alpha/typhoon.c
index ae11e012c7..6a40869488 100644
--- a/hw/alpha/typhoon.c
+++ b/hw/alpha/typhoon.c
@@ -881,10 +881,10 @@ PCIBus *typhoon_init(ram_addr_t ram_size, ISABus **isa_bus,
memory_region_add_subregion(addr_space, 0x801fc000000ULL,
&s->pchip.reg_io);
- b = pci_register_bus(dev, "pci",
- typhoon_set_irq, sys_map_irq, s,
- &s->pchip.reg_mem, &s->pchip.reg_io,
- 0, 64, TYPE_PCI_BUS);
+ b = pci_register_root_bus(dev, "pci",
+ typhoon_set_irq, sys_map_irq, s,
+ &s->pchip.reg_mem, &s->pchip.reg_io,
+ 0, 64, TYPE_PCI_BUS);
phb->bus = b;
qdev_init_nofail(dev);
diff --git a/hw/mips/gt64xxx_pci.c b/hw/mips/gt64xxx_pci.c
index 5a9dad9aae..a9c222a967 100644
--- a/hw/mips/gt64xxx_pci.c
+++ b/hw/mips/gt64xxx_pci.c
@@ -1171,12 +1171,12 @@ PCIBus *gt64120_register(qemu_irq *pic)
phb = PCI_HOST_BRIDGE(dev);
memory_region_init(&d->pci0_mem, OBJECT(dev), "pci0-mem", UINT32_MAX);
address_space_init(&d->pci0_mem_as, &d->pci0_mem, "pci0-mem");
- phb->bus = pci_register_bus(dev, "pci",
- gt64120_pci_set_irq, gt64120_pci_map_irq,
- pic,
- &d->pci0_mem,
- get_system_io(),
- PCI_DEVFN(18, 0), 4, TYPE_PCI_BUS);
+ phb->bus = pci_register_root_bus(dev, "pci",
+ gt64120_pci_set_irq, gt64120_pci_map_irq,
+ pic,
+ &d->pci0_mem,
+ get_system_io(),
+ PCI_DEVFN(18, 0), 4, TYPE_PCI_BUS);
qdev_init_nofail(dev);
memory_region_init_io(&d->ISD_mem, OBJECT(dev), &isd_mem_ops, d, "isd-mem", 0x1000);
diff --git a/hw/pci-bridge/pci_expander_bridge.c b/hw/pci-bridge/pci_expander_bridge.c
index 8c8ac737ad..b2fa829e29 100644
--- a/hw/pci-bridge/pci_expander_bridge.c
+++ b/hw/pci-bridge/pci_expander_bridge.c
@@ -230,9 +230,9 @@ static void pxb_dev_realize_common(PCIDevice *dev, bool pcie, Error **errp)
ds = qdev_create(NULL, TYPE_PXB_HOST);
if (pcie) {
- bus = pci_bus_new(ds, dev_name, NULL, NULL, 0, TYPE_PXB_PCIE_BUS);
+ bus = pci_root_bus_new(ds, dev_name, NULL, NULL, 0, TYPE_PXB_PCIE_BUS);
} else {
- bus = pci_bus_new(ds, "pxb-internal", NULL, NULL, 0, TYPE_PXB_BUS);
+ bus = pci_root_bus_new(ds, "pxb-internal", NULL, NULL, 0, TYPE_PXB_BUS);
bds = qdev_create(BUS(bus), "pci-bridge");
bds->id = dev_name;
qdev_prop_set_uint8(bds, PCI_BRIDGE_DEV_PROP_CHASSIS_NR, pxb->bus_nr);
diff --git a/hw/pci-host/apb.c b/hw/pci-host/apb.c
index 1edf57f600..20c4be36ff 100644
--- a/hw/pci-host/apb.c
+++ b/hw/pci-host/apb.c
@@ -689,11 +689,11 @@ PCIBus *pci_apb_init(hwaddr special_base,
dev = qdev_create(NULL, TYPE_APB);
d = APB_DEVICE(dev);
phb = PCI_HOST_BRIDGE(dev);
- phb->bus = pci_register_bus(DEVICE(phb), "pci",
- pci_apb_set_irq, pci_pbm_map_irq, d,
- &d->pci_mmio,
- get_system_io(),
- 0, 32, TYPE_PCI_BUS);
+ phb->bus = pci_register_root_bus(DEVICE(phb), "pci",
+ pci_apb_set_irq, pci_pbm_map_irq, d,
+ &d->pci_mmio,
+ get_system_io(),
+ 0, 32, TYPE_PCI_BUS);
qdev_init_nofail(dev);
s = SYS_BUS_DEVICE(dev);
/* apb_config */
diff --git a/hw/pci-host/bonito.c b/hw/pci-host/bonito.c
index 9f61e27edc..f08593feab 100644
--- a/hw/pci-host/bonito.c
+++ b/hw/pci-host/bonito.c
@@ -714,10 +714,10 @@ static int bonito_pcihost_initfn(SysBusDevice *dev)
{
PCIHostState *phb = PCI_HOST_BRIDGE(dev);
- phb->bus = pci_register_bus(DEVICE(dev), "pci",
- pci_bonito_set_irq, pci_bonito_map_irq, dev,
- get_system_memory(), get_system_io(),
- 0x28, 32, TYPE_PCI_BUS);
+ phb->bus = pci_register_root_bus(DEVICE(dev), "pci",
+ pci_bonito_set_irq, pci_bonito_map_irq,
+ dev, get_system_memory(), get_system_io(),
+ 0x28, 32, TYPE_PCI_BUS);
return 0;
}
diff --git a/hw/pci-host/gpex.c b/hw/pci-host/gpex.c
index 4090793cf0..05a13fa9a1 100644
--- a/hw/pci-host/gpex.c
+++ b/hw/pci-host/gpex.c
@@ -83,9 +83,9 @@ static void gpex_host_realize(DeviceState *dev, Error **errp)
sysbus_init_irq(sbd, &s->irq[i]);
}
- pci->bus = pci_register_bus(dev, "pcie.0", gpex_set_irq,
- pci_swizzle_map_irq_fn, s, &s->io_mmio,
- &s->io_ioport, 0, 4, TYPE_PCIE_BUS);
+ pci->bus = pci_register_root_bus(dev, "pcie.0", gpex_set_irq,
+ pci_swizzle_map_irq_fn, s, &s->io_mmio,
+ &s->io_ioport, 0, 4, TYPE_PCIE_BUS);
qdev_set_parent_bus(DEVICE(&s->gpex_root), BUS(pci->bus));
pci_bus_set_route_irq_fn(pci->bus, gpex_route_intx_pin_to_irq);
diff --git a/hw/pci-host/grackle.c b/hw/pci-host/grackle.c
index 38cd279b6b..3caf1ccb37 100644
--- a/hw/pci-host/grackle.c
+++ b/hw/pci-host/grackle.c
@@ -82,13 +82,13 @@ PCIBus *pci_grackle_init(uint32_t base, qemu_irq *pic,
memory_region_add_subregion(address_space_mem, 0x80000000ULL,
&d->pci_hole);
- phb->bus = pci_register_bus(dev, NULL,
- pci_grackle_set_irq,
- pci_grackle_map_irq,
- pic,
- &d->pci_mmio,
- address_space_io,
- 0, 4, TYPE_PCI_BUS);
+ phb->bus = pci_register_root_bus(dev, NULL,
+ pci_grackle_set_irq,
+ pci_grackle_map_irq,
+ pic,
+ &d->pci_mmio,
+ address_space_io,
+ 0, 4, TYPE_PCI_BUS);
pci_create_simple(phb->bus, 0, "grackle");
qdev_init_nofail(dev);
diff --git a/hw/pci-host/piix.c b/hw/pci-host/piix.c
index a7e2256870..c434e10ffd 100644
--- a/hw/pci-host/piix.c
+++ b/hw/pci-host/piix.c
@@ -334,8 +334,8 @@ PCIBus *i440fx_init(const char *host_type, const char *pci_type,
dev = qdev_create(NULL, host_type);
s = PCI_HOST_BRIDGE(dev);
- b = pci_bus_new(dev, NULL, pci_address_space,
- address_space_io, 0, TYPE_PCI_BUS);
+ b = pci_root_bus_new(dev, NULL, pci_address_space,
+ address_space_io, 0, TYPE_PCI_BUS);
s->bus = b;
object_property_add_child(qdev_get_machine(), "i440fx", OBJECT(dev), NULL);
qdev_init_nofail(dev);
diff --git a/hw/pci-host/ppce500.c b/hw/pci-host/ppce500.c
index 39cd24464d..67edbf744c 100644
--- a/hw/pci-host/ppce500.c
+++ b/hw/pci-host/ppce500.c
@@ -465,9 +465,9 @@ static int e500_pcihost_initfn(SysBusDevice *dev)
/* PIO lives at the bottom of our bus space */
memory_region_add_subregion_overlap(&s->busmem, 0, &s->pio, -2);
- b = pci_register_bus(DEVICE(dev), NULL, mpc85xx_pci_set_irq,
- mpc85xx_pci_map_irq, s, &s->busmem, &s->pio,
- PCI_DEVFN(s->first_slot, 0), 4, TYPE_PCI_BUS);
+ b = pci_register_root_bus(DEVICE(dev), NULL, mpc85xx_pci_set_irq,
+ mpc85xx_pci_map_irq, s, &s->busmem, &s->pio,
+ PCI_DEVFN(s->first_slot, 0), 4, TYPE_PCI_BUS);
h->bus = b;
/* Set up PCI view of memory */
diff --git a/hw/pci-host/prep.c b/hw/pci-host/prep.c
index 92eed0f3e1..01f67f9db1 100644
--- a/hw/pci-host/prep.c
+++ b/hw/pci-host/prep.c
@@ -269,8 +269,8 @@ static void raven_pcihost_initfn(Object *obj)
memory_region_add_subregion_overlap(address_space_mem, 0x80000000,
&s->pci_io_non_contiguous, 1);
memory_region_add_subregion(address_space_mem, 0xc0000000, &s->pci_memory);
- pci_bus_new_inplace(&s->pci_bus, sizeof(s->pci_bus), DEVICE(obj), NULL,
- &s->pci_memory, &s->pci_io, 0, TYPE_PCI_BUS);
+ pci_root_bus_new_inplace(&s->pci_bus, sizeof(s->pci_bus), DEVICE(obj), NULL,
+ &s->pci_memory, &s->pci_io, 0, TYPE_PCI_BUS);
/* Bus master address space */
memory_region_init(&s->bm, obj, "bm-raven", UINT32_MAX);
diff --git a/hw/pci-host/q35.c b/hw/pci-host/q35.c
index ddaa7d1b44..93e3bbaf8b 100644
--- a/hw/pci-host/q35.c
+++ b/hw/pci-host/q35.c
@@ -49,9 +49,10 @@ static void q35_host_realize(DeviceState *dev, Error **errp)
sysbus_add_io(sbd, MCH_HOST_BRIDGE_CONFIG_DATA, &pci->data_mem);
sysbus_init_ioports(sbd, MCH_HOST_BRIDGE_CONFIG_DATA, 4);
- pci->bus = pci_bus_new(DEVICE(s), "pcie.0",
- s->mch.pci_address_space, s->mch.address_space_io,
- 0, TYPE_PCIE_BUS);
+ pci->bus = pci_root_bus_new(DEVICE(s), "pcie.0",
+ s->mch.pci_address_space,
+ s->mch.address_space_io,
+ 0, TYPE_PCIE_BUS);
PC_MACHINE(qdev_get_machine())->bus = pci->bus;
qdev_set_parent_bus(DEVICE(&s->mch), BUS(pci->bus));
qdev_init_nofail(DEVICE(&s->mch));
diff --git a/hw/pci-host/uninorth.c b/hw/pci-host/uninorth.c
index ea5c265718..5d8ccaa711 100644
--- a/hw/pci-host/uninorth.c
+++ b/hw/pci-host/uninorth.c
@@ -233,12 +233,12 @@ PCIBus *pci_pmac_init(qemu_irq *pic,
memory_region_add_subregion(address_space_mem, 0x80000000ULL,
&d->pci_hole);
- h->bus = pci_register_bus(dev, NULL,
- pci_unin_set_irq, pci_unin_map_irq,
- pic,
- &d->pci_mmio,
- address_space_io,
- PCI_DEVFN(11, 0), 4, TYPE_PCI_BUS);
+ h->bus = pci_register_root_bus(dev, NULL,
+ pci_unin_set_irq, pci_unin_map_irq,
+ pic,
+ &d->pci_mmio,
+ address_space_io,
+ PCI_DEVFN(11, 0), 4, TYPE_PCI_BUS);
#if 0
pci_create_simple(h->bus, PCI_DEVFN(11, 0), "uni-north");
@@ -299,12 +299,12 @@ PCIBus *pci_pmac_u3_init(qemu_irq *pic,
memory_region_add_subregion(address_space_mem, 0x80000000ULL,
&d->pci_hole);
- h->bus = pci_register_bus(dev, NULL,
- pci_unin_set_irq, pci_unin_map_irq,
- pic,
- &d->pci_mmio,
- address_space_io,
- PCI_DEVFN(11, 0), 4, TYPE_PCI_BUS);
+ h->bus = pci_register_root_bus(dev, NULL,
+ pci_unin_set_irq, pci_unin_map_irq,
+ pic,
+ &d->pci_mmio,
+ address_space_io,
+ PCI_DEVFN(11, 0), 4, TYPE_PCI_BUS);
sysbus_mmio_map(s, 0, 0xf0800000);
sysbus_mmio_map(s, 1, 0xf0c00000);
diff --git a/hw/pci-host/versatile.c b/hw/pci-host/versatile.c
index 6394a520fc..8803ada925 100644
--- a/hw/pci-host/versatile.c
+++ b/hw/pci-host/versatile.c
@@ -399,9 +399,9 @@ static void pci_vpb_realize(DeviceState *dev, Error **errp)
memory_region_init(&s->pci_io_space, OBJECT(s), "pci_io", 1ULL << 32);
memory_region_init(&s->pci_mem_space, OBJECT(s), "pci_mem", 1ULL << 32);
- pci_bus_new_inplace(&s->pci_bus, sizeof(s->pci_bus), dev, "pci",
- &s->pci_mem_space, &s->pci_io_space,
- PCI_DEVFN(11, 0), TYPE_PCI_BUS);
+ pci_root_bus_new_inplace(&s->pci_bus, sizeof(s->pci_bus), dev, "pci",
+ &s->pci_mem_space, &s->pci_io_space,
+ PCI_DEVFN(11, 0), TYPE_PCI_BUS);
h->bus = &s->pci_bus;
object_initialize(&s->pci_dev, sizeof(s->pci_dev), TYPE_VERSATILE_PCI_HOST);
diff --git a/hw/pci-host/xilinx-pcie.c b/hw/pci-host/xilinx-pcie.c
index 7659253090..d2f88d11dd 100644
--- a/hw/pci-host/xilinx-pcie.c
+++ b/hw/pci-host/xilinx-pcie.c
@@ -129,9 +129,9 @@ static void xilinx_pcie_host_realize(DeviceState *dev, Error **errp)
sysbus_init_mmio(sbd, &pex->mmio);
sysbus_init_mmio(sbd, &s->mmio);
- pci->bus = pci_register_bus(dev, s->name, xilinx_pcie_set_irq,
- pci_swizzle_map_irq_fn, s, &s->mmio,
- &s->io, 0, 4, TYPE_PCIE_BUS);
+ pci->bus = pci_register_root_bus(dev, s->name, xilinx_pcie_set_irq,
+ pci_swizzle_map_irq_fn, s, &s->mmio,
+ &s->io, 0, 4, TYPE_PCIE_BUS);
qdev_set_parent_bus(DEVICE(&s->root), BUS(pci->bus));
qdev_init_nofail(DEVICE(&s->root));
diff --git a/hw/pci/pci.c b/hw/pci/pci.c
index 5ed3c8dca4..62601a5596 100644
--- a/hw/pci/pci.c
+++ b/hw/pci/pci.c
@@ -376,10 +376,10 @@ const char *pci_root_bus_path(PCIDevice *dev)
return rootbus->qbus.name;
}
-static void pci_bus_init(PCIBus *bus, DeviceState *parent,
- MemoryRegion *address_space_mem,
- MemoryRegion *address_space_io,
- uint8_t devfn_min)
+static void pci_root_bus_init(PCIBus *bus, DeviceState *parent,
+ MemoryRegion *address_space_mem,
+ MemoryRegion *address_space_io,
+ uint8_t devfn_min)
{
assert(PCI_FUNC(devfn_min) == 0);
bus->devfn_min = devfn_min;
@@ -403,25 +403,26 @@ bool pci_bus_is_root(PCIBus *bus)
return PCI_BUS_GET_CLASS(bus)->is_root(bus);
}
-void pci_bus_new_inplace(PCIBus *bus, size_t bus_size, DeviceState *parent,
- const char *name,
- MemoryRegion *address_space_mem,
- MemoryRegion *address_space_io,
- uint8_t devfn_min, const char *typename)
+void pci_root_bus_new_inplace(PCIBus *bus, size_t bus_size, DeviceState *parent,
+ const char *name,
+ MemoryRegion *address_space_mem,
+ MemoryRegion *address_space_io,
+ uint8_t devfn_min, const char *typename)
{
qbus_create_inplace(bus, bus_size, typename, parent, name);
- pci_bus_init(bus, parent, address_space_mem, address_space_io, devfn_min);
+ pci_root_bus_init(bus, parent, address_space_mem, address_space_io,
+ devfn_min);
}
-PCIBus *pci_bus_new(DeviceState *parent, const char *name,
- MemoryRegion *address_space_mem,
- MemoryRegion *address_space_io,
- uint8_t devfn_min, const char *typename)
+PCIBus *pci_root_bus_new(DeviceState *parent, const char *name,
+ MemoryRegion *address_space_mem,
+ MemoryRegion *address_space_io,
+ uint8_t devfn_min, const char *typename)
{
PCIBus *bus;
bus = PCI_BUS(qbus_create(typename, parent, name));
- pci_bus_init(bus, parent, address_space_mem, address_space_io, devfn_min);
+ pci_root_bus_init(bus, parent, address_space_mem, address_space_io, devfn_min);
return bus;
}
@@ -435,17 +436,18 @@ void pci_bus_irqs(PCIBus *bus, pci_set_irq_fn set_irq, pci_map_irq_fn map_irq,
bus->irq_count = g_malloc0(nirq * sizeof(bus->irq_count[0]));
}
-PCIBus *pci_register_bus(DeviceState *parent, const char *name,
- pci_set_irq_fn set_irq, pci_map_irq_fn map_irq,
- void *irq_opaque,
- MemoryRegion *address_space_mem,
- MemoryRegion *address_space_io,
- uint8_t devfn_min, int nirq, const char *typename)
+PCIBus *pci_register_root_bus(DeviceState *parent, const char *name,
+ pci_set_irq_fn set_irq, pci_map_irq_fn map_irq,
+ void *irq_opaque,
+ MemoryRegion *address_space_mem,
+ MemoryRegion *address_space_io,
+ uint8_t devfn_min, int nirq,
+ const char *typename)
{
PCIBus *bus;
- bus = pci_bus_new(parent, name, address_space_mem,
- address_space_io, devfn_min, typename);
+ bus = pci_root_bus_new(parent, name, address_space_mem,
+ address_space_io, devfn_min, typename);
pci_bus_irqs(bus, set_irq, map_irq, irq_opaque, nirq);
return bus;
}
diff --git a/hw/ppc/ppc4xx_pci.c b/hw/ppc/ppc4xx_pci.c
index 4765dcecca..b7642bac01 100644
--- a/hw/ppc/ppc4xx_pci.c
+++ b/hw/ppc/ppc4xx_pci.c
@@ -314,9 +314,9 @@ static int ppc4xx_pcihost_initfn(SysBusDevice *dev)
sysbus_init_irq(dev, &s->irq[i]);
}
- b = pci_register_bus(DEVICE(dev), NULL, ppc4xx_pci_set_irq,
- ppc4xx_pci_map_irq, s->irq, get_system_memory(),
- get_system_io(), 0, 4, TYPE_PCI_BUS);
+ b = pci_register_root_bus(DEVICE(dev), NULL, ppc4xx_pci_set_irq,
+ ppc4xx_pci_map_irq, s->irq, get_system_memory(),
+ get_system_io(), 0, 4, TYPE_PCI_BUS);
h->bus = b;
pci_create_simple(b, 0, "ppc4xx-host-bridge");
diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c
index 5049ced4e8..58b9cfbafd 100644
--- a/hw/ppc/spapr_pci.c
+++ b/hw/ppc/spapr_pci.c
@@ -1611,10 +1611,10 @@ static void spapr_phb_realize(DeviceState *dev, Error **errp)
memory_region_add_subregion(get_system_memory(), sphb->io_win_addr,
&sphb->iowindow);
- bus = pci_register_bus(dev, NULL,
- pci_spapr_set_irq, pci_spapr_map_irq, sphb,
- &sphb->memspace, &sphb->iospace,
- PCI_DEVFN(0, 0), PCI_NUM_PINS, TYPE_PCI_BUS);
+ bus = pci_register_root_bus(dev, NULL,
+ pci_spapr_set_irq, pci_spapr_map_irq, sphb,
+ &sphb->memspace, &sphb->iospace,
+ PCI_DEVFN(0, 0), PCI_NUM_PINS, TYPE_PCI_BUS);
phb->bus = bus;
qbus_set_hotplug_handler(BUS(phb->bus), DEVICE(sphb), NULL);
diff --git a/hw/s390x/s390-pci-bus.c b/hw/s390x/s390-pci-bus.c
index 3b9965fde0..e24ba781bd 100644
--- a/hw/s390x/s390-pci-bus.c
+++ b/hw/s390x/s390-pci-bus.c
@@ -554,10 +554,10 @@ static int s390_pcihost_init(SysBusDevice *dev)
DPRINTF("host_init\n");
- b = pci_register_bus(DEVICE(dev), NULL,
- s390_pci_set_irq, s390_pci_map_irq, NULL,
- get_system_memory(), get_system_io(), 0, 64,
- TYPE_PCI_BUS);
+ b = pci_register_root_bus(DEVICE(dev), NULL,
+ s390_pci_set_irq, s390_pci_map_irq, NULL,
+ get_system_memory(), get_system_io(), 0, 64,
+ TYPE_PCI_BUS);
pci_setup_iommu(b, s390_pci_dma_iommu, s);
bus = BUS(b);
diff --git a/hw/sh4/sh_pci.c b/hw/sh4/sh_pci.c
index cbb01af57f..4ec2e35500 100644
--- a/hw/sh4/sh_pci.c
+++ b/hw/sh4/sh_pci.c
@@ -131,12 +131,12 @@ static int sh_pci_device_init(SysBusDevice *dev)
for (i = 0; i < 4; i++) {
sysbus_init_irq(dev, &s->irq[i]);
}
- phb->bus = pci_register_bus(DEVICE(dev), "pci",
- sh_pci_set_irq, sh_pci_map_irq,
- s->irq,
- get_system_memory(),
- get_system_io(),
- PCI_DEVFN(0, 0), 4, TYPE_PCI_BUS);
+ phb->bus = pci_register_root_bus(DEVICE(dev), "pci",
+ sh_pci_set_irq, sh_pci_map_irq,
+ s->irq,
+ get_system_memory(),
+ get_system_io(),
+ PCI_DEVFN(0, 0), 4, TYPE_PCI_BUS);
memory_region_init_io(&s->memconfig_p4, OBJECT(s), &sh_pci_reg_ops, s,
"sh_pci", 0x224);
memory_region_init_alias(&s->memconfig_a7, OBJECT(s), "sh_pci.2",
diff --git a/include/hw/pci/pci.h b/include/hw/pci/pci.h
index 8d02a0a383..870ebcfd4b 100644
--- a/include/hw/pci/pci.h
+++ b/include/hw/pci/pci.h
@@ -400,26 +400,27 @@ typedef PCIINTxRoute (*pci_route_irq_fn)(void *opaque, int pin);
bool pci_bus_is_express(PCIBus *bus);
bool pci_bus_is_root(PCIBus *bus);
-void pci_bus_new_inplace(PCIBus *bus, size_t bus_size, DeviceState *parent,
- const char *name,
+void pci_root_bus_new_inplace(PCIBus *bus, size_t bus_size, DeviceState *parent,
+ const char *name,
+ MemoryRegion *address_space_mem,
+ MemoryRegion *address_space_io,
+ uint8_t devfn_min, const char *typename);
+PCIBus *pci_root_bus_new(DeviceState *parent, const char *name,
MemoryRegion *address_space_mem,
MemoryRegion *address_space_io,
uint8_t devfn_min, const char *typename);
-PCIBus *pci_bus_new(DeviceState *parent, const char *name,
- MemoryRegion *address_space_mem,
- MemoryRegion *address_space_io,
- uint8_t devfn_min, const char *typename);
void pci_bus_irqs(PCIBus *bus, pci_set_irq_fn set_irq, pci_map_irq_fn map_irq,
void *irq_opaque, int nirq);
int pci_bus_get_irq_level(PCIBus *bus, int irq_num);
/* 0 <= pin <= 3 0 = INTA, 1 = INTB, 2 = INTC, 3 = INTD */
int pci_swizzle_map_irq_fn(PCIDevice *pci_dev, int pin);
-PCIBus *pci_register_bus(DeviceState *parent, const char *name,
- pci_set_irq_fn set_irq, pci_map_irq_fn map_irq,
- void *irq_opaque,
- MemoryRegion *address_space_mem,
- MemoryRegion *address_space_io,
- uint8_t devfn_min, int nirq, const char *typename);
+PCIBus *pci_register_root_bus(DeviceState *parent, const char *name,
+ pci_set_irq_fn set_irq, pci_map_irq_fn map_irq,
+ void *irq_opaque,
+ MemoryRegion *address_space_mem,
+ MemoryRegion *address_space_io,
+ uint8_t devfn_min, int nirq,
+ const char *typename);
void pci_bus_set_route_irq_fn(PCIBus *, pci_route_irq_fn);
PCIINTxRoute pci_device_route_intx_to_irq(PCIDevice *dev, int pin);
bool pci_intx_route_changed(PCIINTxRoute *old, PCIINTxRoute *new);
--
2.13.6
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [Qemu-devel] [RFC 2/5] pci: Move bridge data structures from pci_bus.h to pci_bridge.h
2017-10-03 9:14 [Qemu-devel] [RFC 0/5] Assorted PCI/PCIe cleanups cleanups David Gibson
2017-10-03 9:14 ` [Qemu-devel] [RFC 1/5] pci: Rename root bus initialization functions for clarity David Gibson
@ 2017-10-03 9:14 ` David Gibson
2017-10-03 9:14 ` [Qemu-devel] [RFC 3/5] pci: Fold pci_bus.h into pci.h David Gibson
` (2 subsequent siblings)
4 siblings, 0 replies; 8+ messages in thread
From: David Gibson @ 2017-10-03 9:14 UTC (permalink / raw)
To: mst, ehabkost, marcel; +Cc: qemu-devel, alex.williamson, David Gibson
include/hw/pci/pci_bus.h contains several data structures related to PCI
bridges that aren't needed by most users of pci_bus.h. We already have
a pci_bridge.h, so move them there.
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
include/hw/pci-host/xilinx-pcie.h | 2 +-
include/hw/pci/pci_bridge.h | 48 ++++++++++++++++++++++++++++++++++++
include/hw/pci/pci_bus.h | 51 ++-------------------------------------
3 files changed, 51 insertions(+), 50 deletions(-)
diff --git a/include/hw/pci-host/xilinx-pcie.h b/include/hw/pci-host/xilinx-pcie.h
index bec66b27c5..74c04dc9bb 100644
--- a/include/hw/pci-host/xilinx-pcie.h
+++ b/include/hw/pci-host/xilinx-pcie.h
@@ -23,7 +23,7 @@
#include "hw/hw.h"
#include "hw/sysbus.h"
#include "hw/pci/pci.h"
-#include "hw/pci/pci_bus.h"
+#include "hw/pci/pci_bridge.h"
#include "hw/pci/pcie_host.h"
#define TYPE_XILINX_PCIE_HOST "xilinx-pcie-host"
diff --git a/include/hw/pci/pci_bridge.h b/include/hw/pci/pci_bridge.h
index 1acadc2c15..9b44ffd22a 100644
--- a/include/hw/pci/pci_bridge.h
+++ b/include/hw/pci/pci_bridge.h
@@ -27,6 +27,54 @@
#define QEMU_PCI_BRIDGE_H
#include "hw/pci/pci.h"
+#include "hw/pci/pci_bus.h"
+
+typedef struct PCIBridgeWindows PCIBridgeWindows;
+
+/*
+ * Aliases for each of the address space windows that the bridge
+ * can forward. Mapped into the bridge's parent's address space,
+ * as subregions.
+ */
+struct PCIBridgeWindows {
+ MemoryRegion alias_pref_mem;
+ MemoryRegion alias_mem;
+ MemoryRegion alias_io;
+ /*
+ * When bridge control VGA forwarding is enabled, bridges will
+ * provide positive decode on the PCI VGA defined I/O port and
+ * MMIO ranges. When enabled forwarding is only qualified on the
+ * I/O and memory enable bits in the bridge command register.
+ */
+ MemoryRegion alias_vga[QEMU_PCI_VGA_NUM_REGIONS];
+};
+
+#define TYPE_PCI_BRIDGE "base-pci-bridge"
+#define PCI_BRIDGE(obj) OBJECT_CHECK(PCIBridge, (obj), TYPE_PCI_BRIDGE)
+
+struct PCIBridge {
+ /*< private >*/
+ PCIDevice parent_obj;
+ /*< public >*/
+
+ /* private member */
+ PCIBus sec_bus;
+ /*
+ * Memory regions for the bridge's address spaces. These regions are not
+ * directly added to system_memory/system_io or its descendants.
+ * Bridge's secondary bus points to these, so that devices
+ * under the bridge see these regions as its address spaces.
+ * The regions are as large as the entire address space -
+ * they don't take into account any windows.
+ */
+ MemoryRegion address_space_mem;
+ MemoryRegion address_space_io;
+
+ PCIBridgeWindows *windows;
+
+ pci_map_irq_fn map_irq;
+ const char *bus_name;
+};
#define PCI_BRIDGE_DEV_PROP_CHASSIS_NR "chassis_nr"
#define PCI_BRIDGE_DEV_PROP_MSI "msi"
diff --git a/include/hw/pci/pci_bus.h b/include/hw/pci/pci_bus.h
index bc34fd0017..b7da8f555b 100644
--- a/include/hw/pci/pci_bus.h
+++ b/include/hw/pci/pci_bus.h
@@ -2,10 +2,10 @@
#define QEMU_PCI_BUS_H
/*
- * PCI Bus and Bridge datastructures.
+ * PCI Bus datastructures.
*
* Do not access the following members directly;
- * use accessor functions in pci.h, pci_bridge.h
+ * use accessor functions in pci.h
*/
typedef struct PCIBusClass {
@@ -44,51 +44,4 @@ struct PCIBus {
Notifier machine_done;
};
-typedef struct PCIBridgeWindows PCIBridgeWindows;
-
-/*
- * Aliases for each of the address space windows that the bridge
- * can forward. Mapped into the bridge's parent's address space,
- * as subregions.
- */
-struct PCIBridgeWindows {
- MemoryRegion alias_pref_mem;
- MemoryRegion alias_mem;
- MemoryRegion alias_io;
- /*
- * When bridge control VGA forwarding is enabled, bridges will
- * provide positive decode on the PCI VGA defined I/O port and
- * MMIO ranges. When enabled forwarding is only qualified on the
- * I/O and memory enable bits in the bridge command register.
- */
- MemoryRegion alias_vga[QEMU_PCI_VGA_NUM_REGIONS];
-};
-
-#define TYPE_PCI_BRIDGE "base-pci-bridge"
-#define PCI_BRIDGE(obj) OBJECT_CHECK(PCIBridge, (obj), TYPE_PCI_BRIDGE)
-
-struct PCIBridge {
- /*< private >*/
- PCIDevice parent_obj;
- /*< public >*/
-
- /* private member */
- PCIBus sec_bus;
- /*
- * Memory regions for the bridge's address spaces. These regions are not
- * directly added to system_memory/system_io or its descendants.
- * Bridge's secondary bus points to these, so that devices
- * under the bridge see these regions as its address spaces.
- * The regions are as large as the entire address space -
- * they don't take into account any windows.
- */
- MemoryRegion address_space_mem;
- MemoryRegion address_space_io;
-
- PCIBridgeWindows *windows;
-
- pci_map_irq_fn map_irq;
- const char *bus_name;
-};
-
#endif /* QEMU_PCI_BUS_H */
--
2.13.6
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [Qemu-devel] [RFC 3/5] pci: Fold pci_bus.h into pci.h
2017-10-03 9:14 [Qemu-devel] [RFC 0/5] Assorted PCI/PCIe cleanups cleanups David Gibson
2017-10-03 9:14 ` [Qemu-devel] [RFC 1/5] pci: Rename root bus initialization functions for clarity David Gibson
2017-10-03 9:14 ` [Qemu-devel] [RFC 2/5] pci: Move bridge data structures from pci_bus.h to pci_bridge.h David Gibson
@ 2017-10-03 9:14 ` David Gibson
2017-10-03 9:14 ` [Qemu-devel] [RFC 4/5] pci: Simplify pci_bus_is_root() David Gibson
2017-10-03 9:14 ` [Qemu-devel] [RFC 5/5] pcie: Don't allow extended config space access via conventional PCI bridges David Gibson
4 siblings, 0 replies; 8+ messages in thread
From: David Gibson @ 2017-10-03 9:14 UTC (permalink / raw)
To: mst, ehabkost, marcel; +Cc: qemu-devel, alex.williamson, David Gibson
include/hw/pci/pci_bus.h is now very small and can only safely be included
after hw/pci/pci.h. So, just fold it into pci.h.
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
hw/acpi/pcihp.c | 1 -
hw/i386/acpi-build.c | 1 -
hw/i386/amd_iommu.h | 1 -
hw/i386/intel_iommu.c | 1 -
hw/i386/pc.c | 1 -
hw/isa/lpc_ich9.c | 1 -
hw/pci-bridge/dec.c | 1 -
hw/pci-bridge/pci_bridge_dev.c | 1 -
hw/pci-bridge/pci_expander_bridge.c | 1 -
hw/pci-bridge/pcie_pci_bridge.c | 1 -
hw/pci-host/apb.c | 1 -
hw/pci-host/prep.c | 1 -
hw/pci-host/versatile.c | 1 -
hw/pci/pci.c | 1 -
hw/pci/pci_bridge.c | 1 -
hw/pci/pci_host.c | 1 -
hw/pci/pcie.c | 1 -
hw/pci/pcie_aer.c | 1 -
hw/pci/shpc.c | 1 -
hw/ppc/spapr_pci.c | 1 -
hw/s390x/s390-pci-bus.c | 1 -
include/hw/i386/ich9.h | 1 -
include/hw/pci/pci.h | 44 ++++++++++++++++++++++++++++++++--
include/hw/pci/pci_bridge.h | 1 -
include/hw/pci/pci_bus.h | 47 -------------------------------------
include/hw/pci/pcie_port.h | 1 -
26 files changed, 42 insertions(+), 73 deletions(-)
delete mode 100644 include/hw/pci/pci_bus.h
diff --git a/hw/acpi/pcihp.c b/hw/acpi/pcihp.c
index 7da51c0569..0da905ab3a 100644
--- a/hw/acpi/pcihp.c
+++ b/hw/acpi/pcihp.c
@@ -34,7 +34,6 @@
#include "sysemu/sysemu.h"
#include "exec/ioport.h"
#include "exec/address-spaces.h"
-#include "hw/pci/pci_bus.h"
#include "qapi/error.h"
#include "qom/qom-qobject.h"
diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
index 2af37a9129..6ad54e1e5b 100644
--- a/hw/i386/acpi-build.c
+++ b/hw/i386/acpi-build.c
@@ -51,7 +51,6 @@
#include "hw/acpi/piix4.h"
#include "hw/acpi/pcihp.h"
#include "hw/i386/ich9.h"
-#include "hw/pci/pci_bus.h"
#include "hw/pci-host/q35.h"
#include "hw/i386/x86-iommu.h"
diff --git a/hw/i386/amd_iommu.h b/hw/i386/amd_iommu.h
index d370ae3549..b587f6b49f 100644
--- a/hw/i386/amd_iommu.h
+++ b/hw/i386/amd_iommu.h
@@ -27,7 +27,6 @@
#include "hw/sysbus.h"
#include "sysemu/dma.h"
#include "hw/i386/pc.h"
-#include "hw/pci/pci_bus.h"
#include "hw/i386/x86-iommu.h"
/* Capability registers */
diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c
index 3a5bb0bc2e..3ef4bfbe05 100644
--- a/hw/i386/intel_iommu.c
+++ b/hw/i386/intel_iommu.c
@@ -26,7 +26,6 @@
#include "exec/address-spaces.h"
#include "intel_iommu_internal.h"
#include "hw/pci/pci.h"
-#include "hw/pci/pci_bus.h"
#include "hw/i386/pc.h"
#include "hw/i386/apic-msidef.h"
#include "hw/boards.h"
diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index 05985d4927..5fd778a627 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -31,7 +31,6 @@
#include "hw/block/fdc.h"
#include "hw/ide.h"
#include "hw/pci/pci.h"
-#include "hw/pci/pci_bus.h"
#include "hw/nvram/fw_cfg.h"
#include "hw/timer/hpet.h"
#include "hw/smbios/smbios.h"
diff --git a/hw/isa/lpc_ich9.c b/hw/isa/lpc_ich9.c
index 39f56ba44e..7b4ebbc0c7 100644
--- a/hw/isa/lpc_ich9.c
+++ b/hw/isa/lpc_ich9.c
@@ -44,7 +44,6 @@
#include "hw/i386/ich9.h"
#include "hw/acpi/acpi.h"
#include "hw/acpi/ich9.h"
-#include "hw/pci/pci_bus.h"
#include "exec/address-spaces.h"
#include "sysemu/sysemu.h"
#include "qom/cpu.h"
diff --git a/hw/pci-bridge/dec.c b/hw/pci-bridge/dec.c
index 84492d5e5f..ae4b9697ed 100644
--- a/hw/pci-bridge/dec.c
+++ b/hw/pci-bridge/dec.c
@@ -29,7 +29,6 @@
#include "hw/pci/pci.h"
#include "hw/pci/pci_host.h"
#include "hw/pci/pci_bridge.h"
-#include "hw/pci/pci_bus.h"
/* debug DEC */
//#define DEBUG_DEC
diff --git a/hw/pci-bridge/pci_bridge_dev.c b/hw/pci-bridge/pci_bridge_dev.c
index d56f6638c2..bc3c8d9f57 100644
--- a/hw/pci-bridge/pci_bridge_dev.c
+++ b/hw/pci-bridge/pci_bridge_dev.c
@@ -27,7 +27,6 @@
#include "hw/pci/shpc.h"
#include "hw/pci/slotid_cap.h"
#include "exec/memory.h"
-#include "hw/pci/pci_bus.h"
#include "hw/hotplug.h"
#define TYPE_PCI_BRIDGE_DEV "pci-bridge"
diff --git a/hw/pci-bridge/pci_expander_bridge.c b/hw/pci-bridge/pci_expander_bridge.c
index b2fa829e29..5652cf06e9 100644
--- a/hw/pci-bridge/pci_expander_bridge.c
+++ b/hw/pci-bridge/pci_expander_bridge.c
@@ -13,7 +13,6 @@
#include "qemu/osdep.h"
#include "qapi/error.h"
#include "hw/pci/pci.h"
-#include "hw/pci/pci_bus.h"
#include "hw/pci/pci_host.h"
#include "hw/pci/pci_bridge.h"
#include "hw/i386/pc.h"
diff --git a/hw/pci-bridge/pcie_pci_bridge.c b/hw/pci-bridge/pcie_pci_bridge.c
index 88db143633..85fd053fbd 100644
--- a/hw/pci-bridge/pcie_pci_bridge.c
+++ b/hw/pci-bridge/pcie_pci_bridge.c
@@ -10,7 +10,6 @@
#include "qemu/osdep.h"
#include "qapi/error.h"
#include "hw/pci/pci.h"
-#include "hw/pci/pci_bus.h"
#include "hw/pci/pci_bridge.h"
#include "hw/pci/msi.h"
#include "hw/pci/shpc.h"
diff --git a/hw/pci-host/apb.c b/hw/pci-host/apb.c
index 20c4be36ff..0e0c1ce0bc 100644
--- a/hw/pci-host/apb.c
+++ b/hw/pci-host/apb.c
@@ -32,7 +32,6 @@
#include "hw/pci/pci.h"
#include "hw/pci/pci_host.h"
#include "hw/pci/pci_bridge.h"
-#include "hw/pci/pci_bus.h"
#include "hw/pci-host/apb.h"
#include "sysemu/sysemu.h"
#include "exec/address-spaces.h"
diff --git a/hw/pci-host/prep.c b/hw/pci-host/prep.c
index 01f67f9db1..56920914c6 100644
--- a/hw/pci-host/prep.c
+++ b/hw/pci-host/prep.c
@@ -27,7 +27,6 @@
#include "qapi/error.h"
#include "hw/hw.h"
#include "hw/pci/pci.h"
-#include "hw/pci/pci_bus.h"
#include "hw/pci/pci_host.h"
#include "hw/i386/pc.h"
#include "hw/loader.h"
diff --git a/hw/pci-host/versatile.c b/hw/pci-host/versatile.c
index 8803ada925..b5bf4dce55 100644
--- a/hw/pci-host/versatile.c
+++ b/hw/pci-host/versatile.c
@@ -10,7 +10,6 @@
#include "qemu/osdep.h"
#include "hw/sysbus.h"
#include "hw/pci/pci.h"
-#include "hw/pci/pci_bus.h"
#include "hw/pci/pci_host.h"
#include "exec/address-spaces.h"
#include "qemu/log.h"
diff --git a/hw/pci/pci.c b/hw/pci/pci.c
index 62601a5596..ffaa449ec1 100644
--- a/hw/pci/pci.c
+++ b/hw/pci/pci.c
@@ -25,7 +25,6 @@
#include "hw/hw.h"
#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"
diff --git a/hw/pci/pci_bridge.c b/hw/pci/pci_bridge.c
index 17feae5ed8..63b0adac15 100644
--- a/hw/pci/pci_bridge.c
+++ b/hw/pci/pci_bridge.c
@@ -31,7 +31,6 @@
#include "qemu/osdep.h"
#include "hw/pci/pci_bridge.h"
-#include "hw/pci/pci_bus.h"
#include "qemu/range.h"
#include "qapi/error.h"
diff --git a/hw/pci/pci_host.c b/hw/pci/pci_host.c
index 5eaa935cb5..3a26880f18 100644
--- a/hw/pci/pci_host.c
+++ b/hw/pci/pci_host.c
@@ -21,7 +21,6 @@
#include "qemu/osdep.h"
#include "hw/pci/pci.h"
#include "hw/pci/pci_host.h"
-#include "hw/pci/pci_bus.h"
#include "trace.h"
/* debug PCI */
diff --git a/hw/pci/pcie.c b/hw/pci/pcie.c
index 32191f2a55..28ba4a0a72 100644
--- a/hw/pci/pcie.c
+++ b/hw/pci/pcie.c
@@ -25,7 +25,6 @@
#include "hw/pci/pcie.h"
#include "hw/pci/msix.h"
#include "hw/pci/msi.h"
-#include "hw/pci/pci_bus.h"
#include "hw/pci/pcie_regs.h"
#include "qemu/range.h"
diff --git a/hw/pci/pcie_aer.c b/hw/pci/pcie_aer.c
index 97200742b4..171955195f 100644
--- a/hw/pci/pcie_aer.c
+++ b/hw/pci/pcie_aer.c
@@ -27,7 +27,6 @@
#include "hw/pci/pcie.h"
#include "hw/pci/msix.h"
#include "hw/pci/msi.h"
-#include "hw/pci/pci_bus.h"
#include "hw/pci/pcie_regs.h"
#include "qapi/error.h"
diff --git a/hw/pci/shpc.c b/hw/pci/shpc.c
index 69fc14b218..7d25e5dc78 100644
--- a/hw/pci/shpc.c
+++ b/hw/pci/shpc.c
@@ -5,7 +5,6 @@
#include "qemu/error-report.h"
#include "hw/pci/shpc.h"
#include "hw/pci/pci.h"
-#include "hw/pci/pci_bus.h"
#include "hw/pci/msi.h"
/* TODO: model power only and disabled slot states. */
diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c
index 58b9cfbafd..70d0ad56f4 100644
--- a/hw/ppc/spapr_pci.c
+++ b/hw/ppc/spapr_pci.c
@@ -42,7 +42,6 @@
#include "qapi/qmp/qerror.h"
#include "hw/ppc/fdt.h"
#include "hw/pci/pci_bridge.h"
-#include "hw/pci/pci_bus.h"
#include "hw/pci/pci_ids.h"
#include "hw/ppc/spapr_drc.h"
#include "sysemu/device_tree.h"
diff --git a/hw/s390x/s390-pci-bus.c b/hw/s390x/s390-pci-bus.c
index e24ba781bd..69ac2d812d 100644
--- a/hw/s390x/s390-pci-bus.c
+++ b/hw/s390x/s390-pci-bus.c
@@ -18,7 +18,6 @@
#include "cpu.h"
#include "s390-pci-bus.h"
#include "s390-pci-inst.h"
-#include "hw/pci/pci_bus.h"
#include "hw/pci/pci_bridge.h"
#include "hw/pci/msi.h"
#include "qemu/error-report.h"
diff --git a/include/hw/i386/ich9.h b/include/hw/i386/ich9.h
index 673d13d28f..c613f85b11 100644
--- a/include/hw/i386/ich9.h
+++ b/include/hw/i386/ich9.h
@@ -12,7 +12,6 @@
#include "hw/pci/pci_bridge.h"
#include "hw/acpi/acpi.h"
#include "hw/acpi/ich9.h"
-#include "hw/pci/pci_bus.h"
void ich9_lpc_set_irq(void *opaque, int irq_num, int level);
int ich9_lpc_map_irq(PCIDevice *pci_dev, int intx);
diff --git a/include/hw/pci/pci.h b/include/hw/pci/pci.h
index 870ebcfd4b..77d92a3dc4 100644
--- a/include/hw/pci/pci.h
+++ b/include/hw/pci/pci.h
@@ -392,12 +392,54 @@ typedef void (*pci_set_irq_fn)(void *opaque, int irq_num, int level);
typedef int (*pci_map_irq_fn)(PCIDevice *pci_dev, int irq_num);
typedef PCIINTxRoute (*pci_route_irq_fn)(void *opaque, int pin);
+typedef AddressSpace *(*PCIIOMMUFunc)(PCIBus *, void *, int);
+
+/*
+ * PCI Bus datastructures.
+ */
+
#define TYPE_PCI_BUS "PCI"
#define PCI_BUS(obj) OBJECT_CHECK(PCIBus, (obj), TYPE_PCI_BUS)
#define PCI_BUS_CLASS(klass) OBJECT_CLASS_CHECK(PCIBusClass, (klass), TYPE_PCI_BUS)
#define PCI_BUS_GET_CLASS(obj) OBJECT_GET_CLASS(PCIBusClass, (obj), TYPE_PCI_BUS)
#define TYPE_PCIE_BUS "PCIE"
+typedef struct PCIBusClass {
+ /*< private >*/
+ BusClass parent_class;
+ /*< public >*/
+
+ bool (*is_root)(PCIBus *bus);
+ int (*bus_num)(PCIBus *bus);
+ uint16_t (*numa_node)(PCIBus *bus);
+} PCIBusClass;
+
+struct PCIBus {
+ BusState qbus;
+ PCIIOMMUFunc iommu_fn;
+ void *iommu_opaque;
+ uint8_t devfn_min;
+ uint32_t slot_reserved_mask;
+ pci_set_irq_fn set_irq;
+ pci_map_irq_fn map_irq;
+ pci_route_irq_fn route_intx_to_irq;
+ void *irq_opaque;
+ PCIDevice *devices[PCI_SLOT_MAX * PCI_FUNC_MAX];
+ PCIDevice *parent_dev;
+ MemoryRegion *address_space_mem;
+ MemoryRegion *address_space_io;
+
+ QLIST_HEAD(, PCIBus) child; /* this will be replaced by qdev later */
+ QLIST_ENTRY(PCIBus) sibling;/* this will be replaced by qdev later */
+
+ /* The bus IRQ state is the logical OR of the connected devices.
+ Keep a count of the number of devices with raised IRQs. */
+ int nirq;
+ int *irq_count;
+
+ Notifier machine_done;
+};
+
bool pci_bus_is_express(PCIBus *bus);
bool pci_bus_is_root(PCIBus *bus);
void pci_root_bus_new_inplace(PCIBus *bus, size_t bus_size, DeviceState *parent,
@@ -468,8 +510,6 @@ void pci_bus_get_w64_range(PCIBus *bus, Range *range);
void pci_device_deassert_intx(PCIDevice *dev);
-typedef AddressSpace *(*PCIIOMMUFunc)(PCIBus *, void *, int);
-
AddressSpace *pci_device_iommu_address_space(PCIDevice *dev);
void pci_setup_iommu(PCIBus *bus, PCIIOMMUFunc fn, void *opaque);
diff --git a/include/hw/pci/pci_bridge.h b/include/hw/pci/pci_bridge.h
index 9b44ffd22a..454bb09951 100644
--- a/include/hw/pci/pci_bridge.h
+++ b/include/hw/pci/pci_bridge.h
@@ -27,7 +27,6 @@
#define QEMU_PCI_BRIDGE_H
#include "hw/pci/pci.h"
-#include "hw/pci/pci_bus.h"
typedef struct PCIBridgeWindows PCIBridgeWindows;
diff --git a/include/hw/pci/pci_bus.h b/include/hw/pci/pci_bus.h
deleted file mode 100644
index b7da8f555b..0000000000
--- a/include/hw/pci/pci_bus.h
+++ /dev/null
@@ -1,47 +0,0 @@
-#ifndef QEMU_PCI_BUS_H
-#define QEMU_PCI_BUS_H
-
-/*
- * PCI Bus datastructures.
- *
- * Do not access the following members directly;
- * use accessor functions in pci.h
- */
-
-typedef struct PCIBusClass {
- /*< private >*/
- BusClass parent_class;
- /*< public >*/
-
- bool (*is_root)(PCIBus *bus);
- int (*bus_num)(PCIBus *bus);
- uint16_t (*numa_node)(PCIBus *bus);
-} PCIBusClass;
-
-struct PCIBus {
- BusState qbus;
- PCIIOMMUFunc iommu_fn;
- void *iommu_opaque;
- uint8_t devfn_min;
- uint32_t slot_reserved_mask;
- pci_set_irq_fn set_irq;
- pci_map_irq_fn map_irq;
- pci_route_irq_fn route_intx_to_irq;
- void *irq_opaque;
- PCIDevice *devices[PCI_SLOT_MAX * PCI_FUNC_MAX];
- PCIDevice *parent_dev;
- MemoryRegion *address_space_mem;
- MemoryRegion *address_space_io;
-
- QLIST_HEAD(, PCIBus) child; /* this will be replaced by qdev later */
- QLIST_ENTRY(PCIBus) sibling;/* this will be replaced by qdev later */
-
- /* The bus IRQ state is the logical OR of the connected devices.
- Keep a count of the number of devices with raised IRQs. */
- int nirq;
- int *irq_count;
-
- Notifier machine_done;
-};
-
-#endif /* QEMU_PCI_BUS_H */
diff --git a/include/hw/pci/pcie_port.h b/include/hw/pci/pcie_port.h
index 0736014bfd..bda76d79e0 100644
--- a/include/hw/pci/pcie_port.h
+++ b/include/hw/pci/pcie_port.h
@@ -22,7 +22,6 @@
#define QEMU_PCIE_PORT_H
#include "hw/pci/pci_bridge.h"
-#include "hw/pci/pci_bus.h"
#define TYPE_PCIE_PORT "pcie-port"
#define PCIE_PORT(obj) OBJECT_CHECK(PCIEPort, (obj), TYPE_PCIE_PORT)
--
2.13.6
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [Qemu-devel] [RFC 4/5] pci: Simplify pci_bus_is_root()
2017-10-03 9:14 [Qemu-devel] [RFC 0/5] Assorted PCI/PCIe cleanups cleanups David Gibson
` (2 preceding siblings ...)
2017-10-03 9:14 ` [Qemu-devel] [RFC 3/5] pci: Fold pci_bus.h into pci.h David Gibson
@ 2017-10-03 9:14 ` David Gibson
2017-10-03 14:42 ` Eduardo Habkost
2017-10-03 9:14 ` [Qemu-devel] [RFC 5/5] pcie: Don't allow extended config space access via conventional PCI bridges David Gibson
4 siblings, 1 reply; 8+ messages in thread
From: David Gibson @ 2017-10-03 9:14 UTC (permalink / raw)
To: mst, ehabkost, marcel; +Cc: qemu-devel, alex.williamson, David Gibson
pci_bus_is_root() currently relies on a method in the PCIBusClass.
But it's always known if a PCI bus is a root bus when we create it, so
using a dynamic method is overkill.
This replaces it with an IS_ROOT bit in a new flags field, which is set on
root buses and otherwise clear. As a bonus this removes the special
is_root logic from pci_expander_bridge, since it already creates its bus
as a root bus.
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
# Conflicts:
# include/hw/pci/pci_bus.h
---
hw/pci-bridge/pci_expander_bridge.c | 6 ------
hw/pci/pci.c | 14 ++------------
include/hw/pci/pci.h | 12 +++++++++++-
3 files changed, 13 insertions(+), 19 deletions(-)
diff --git a/hw/pci-bridge/pci_expander_bridge.c b/hw/pci-bridge/pci_expander_bridge.c
index 5652cf06e9..11dfa9258e 100644
--- a/hw/pci-bridge/pci_expander_bridge.c
+++ b/hw/pci-bridge/pci_expander_bridge.c
@@ -65,11 +65,6 @@ static int pxb_bus_num(PCIBus *bus)
return pxb->bus_nr;
}
-static bool pxb_is_root(PCIBus *bus)
-{
- return true; /* by definition */
-}
-
static uint16_t pxb_bus_numa_node(PCIBus *bus)
{
PXBDev *pxb = convert_to_pxb(bus->parent_dev);
@@ -82,7 +77,6 @@ static void pxb_bus_class_init(ObjectClass *class, void *data)
PCIBusClass *pbc = PCI_BUS_CLASS(class);
pbc->bus_num = pxb_bus_num;
- pbc->is_root = pxb_is_root;
pbc->numa_node = pxb_bus_numa_node;
}
diff --git a/hw/pci/pci.c b/hw/pci/pci.c
index ffaa449ec1..4baf591562 100644
--- a/hw/pci/pci.c
+++ b/hw/pci/pci.c
@@ -126,14 +126,9 @@ static void pci_bus_unrealize(BusState *qbus, Error **errp)
vmstate_unregister(NULL, &vmstate_pcibus, bus);
}
-static bool pcibus_is_root(PCIBus *bus)
-{
- return !bus->parent_dev;
-}
-
static int pcibus_num(PCIBus *bus)
{
- if (pcibus_is_root(bus)) {
+ if (pci_bus_is_root(bus)) {
return 0; /* pci host bridge */
}
return bus->parent_dev->config[PCI_SECONDARY_BUS];
@@ -156,7 +151,6 @@ static void pci_bus_class_init(ObjectClass *klass, void *data)
k->unrealize = pci_bus_unrealize;
k->reset = pcibus_reset;
- pbc->is_root = pcibus_is_root;
pbc->bus_num = pcibus_num;
pbc->numa_node = pcibus_numa_node;
}
@@ -385,6 +379,7 @@ static void pci_root_bus_init(PCIBus *bus, DeviceState *parent,
bus->slot_reserved_mask = 0x0;
bus->address_space_mem = address_space_mem;
bus->address_space_io = address_space_io;
+ bus->flags |= PCI_BUS_IS_ROOT;
/* host bridge */
QLIST_INIT(&bus->child);
@@ -397,11 +392,6 @@ bool pci_bus_is_express(PCIBus *bus)
return object_dynamic_cast(OBJECT(bus), TYPE_PCIE_BUS);
}
-bool pci_bus_is_root(PCIBus *bus)
-{
- return PCI_BUS_GET_CLASS(bus)->is_root(bus);
-}
-
void pci_root_bus_new_inplace(PCIBus *bus, size_t bus_size, DeviceState *parent,
const char *name,
MemoryRegion *address_space_mem,
diff --git a/include/hw/pci/pci.h b/include/hw/pci/pci.h
index 77d92a3dc4..cbb3386207 100644
--- a/include/hw/pci/pci.h
+++ b/include/hw/pci/pci.h
@@ -404,6 +404,11 @@ typedef AddressSpace *(*PCIIOMMUFunc)(PCIBus *, void *, int);
#define PCI_BUS_GET_CLASS(obj) OBJECT_GET_CLASS(PCIBusClass, (obj), TYPE_PCI_BUS)
#define TYPE_PCIE_BUS "PCIE"
+enum PCIBusFlags {
+ /* This bus is the root of a PCI domain */
+ PCI_BUS_IS_ROOT = 0x0001,
+};
+
typedef struct PCIBusClass {
/*< private >*/
BusClass parent_class;
@@ -416,6 +421,7 @@ typedef struct PCIBusClass {
struct PCIBus {
BusState qbus;
+ enum PCIBusFlags flags;
PCIIOMMUFunc iommu_fn;
void *iommu_opaque;
uint8_t devfn_min;
@@ -440,8 +446,12 @@ struct PCIBus {
Notifier machine_done;
};
+static inline bool pci_bus_is_root(PCIBus *bus)
+{
+ return !!(bus->flags & PCI_BUS_IS_ROOT);
+}
+
bool pci_bus_is_express(PCIBus *bus);
-bool pci_bus_is_root(PCIBus *bus);
void pci_root_bus_new_inplace(PCIBus *bus, size_t bus_size, DeviceState *parent,
const char *name,
MemoryRegion *address_space_mem,
--
2.13.6
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [Qemu-devel] [RFC 5/5] pcie: Don't allow extended config space access via conventional PCI bridges
2017-10-03 9:14 [Qemu-devel] [RFC 0/5] Assorted PCI/PCIe cleanups cleanups David Gibson
` (3 preceding siblings ...)
2017-10-03 9:14 ` [Qemu-devel] [RFC 4/5] pci: Simplify pci_bus_is_root() David Gibson
@ 2017-10-03 9:14 ` David Gibson
4 siblings, 0 replies; 8+ messages in thread
From: David Gibson @ 2017-10-03 9:14 UTC (permalink / raw)
To: mst, ehabkost, marcel; +Cc: qemu-devel, alex.williamson, David Gibson
In hardware it's possible, if odd, to have a configuration like:
PCIe host bridge
\- PCIe to PCI bridge
\- PCI to PCIe bridge
\- PCIe device
The PCIe extended configuration space on the device won't be accessible to
the host, because the cycles can't traverse the conventional PCI bus on the
way there.
However, if we attempt to model that configuration under qemu, extended
config access on the device *will* work, because pci_config_size() depends
only on whether the device itself is PCIe capable.
This patch fixes that modelling error by adding a flag to each PCI/PCIe bus
instance indicating whether extended config space accesses are possible on
it. It will always be false for conventional PCI buses, for PCIe buses it
will be true if and only if the parent bus also has the flag set.
AIUI earlier attempts to correct this have been rejected, because they
involved expensively traversing the whole bus heirarchy on each config
access. This approach avoids that by computing the value as the bus
heirarchy is contructed, meaning we only need a single bit check when we
actually attempt the config access.
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
hw/pci/pci.c | 12 ++++++++++++
include/hw/pci/pci.h | 10 +++++++++-
2 files changed, 21 insertions(+), 1 deletion(-)
diff --git a/hw/pci/pci.c b/hw/pci/pci.c
index 4baf591562..b0a757ffd0 100644
--- a/hw/pci/pci.c
+++ b/hw/pci/pci.c
@@ -115,6 +115,18 @@ static void pci_bus_realize(BusState *qbus, Error **errp)
qemu_add_machine_init_done_notifier(&bus->machine_done);
vmstate_register(NULL, -1, &vmstate_pcibus, bus);
+
+ /* a PCI-E bus can supported extended config space if it's the
+ * root bus, or if the bus/bridge above it does as well */
+ if (pci_bus_is_root(bus)) {
+ bus->flags |= PCI_BUS_EXTENDED_CONFIG_SPACE;
+ } else {
+ PCIBus *parent_bus = bus->parent_dev->bus;
+
+ if (pci_bus_extended_config_space(parent_bus)) {
+ bus->flags |= PCI_BUS_EXTENDED_CONFIG_SPACE;
+ }
+ }
}
static void pci_bus_unrealize(BusState *qbus, Error **errp)
diff --git a/include/hw/pci/pci.h b/include/hw/pci/pci.h
index cbb3386207..471c13b50a 100644
--- a/include/hw/pci/pci.h
+++ b/include/hw/pci/pci.h
@@ -407,6 +407,8 @@ typedef AddressSpace *(*PCIIOMMUFunc)(PCIBus *, void *, int);
enum PCIBusFlags {
/* This bus is the root of a PCI domain */
PCI_BUS_IS_ROOT = 0x0001,
+ /* PCIe extended configuration space is accessible on this bus */
+ PCI_BUS_EXTENDED_CONFIG_SPACE = 0x0002,
};
typedef struct PCIBusClass {
@@ -451,6 +453,11 @@ static inline bool pci_bus_is_root(PCIBus *bus)
return !!(bus->flags & PCI_BUS_IS_ROOT);
}
+static inline bool pci_bus_extended_config_space(PCIBus *bus)
+{
+ return !!(bus->flags & PCI_BUS_EXTENDED_CONFIG_SPACE);
+}
+
bool pci_bus_is_express(PCIBus *bus);
void pci_root_bus_new_inplace(PCIBus *bus, size_t bus_size, DeviceState *parent,
const char *name,
@@ -785,7 +792,8 @@ static inline int pci_is_express(const PCIDevice *d)
static inline uint32_t pci_config_size(const PCIDevice *d)
{
- return pci_is_express(d) ? PCIE_CONFIG_SPACE_SIZE : PCI_CONFIG_SPACE_SIZE;
+ return (pci_is_express(d) && pci_bus_extended_config_space(d->bus))
+ ? PCIE_CONFIG_SPACE_SIZE : PCI_CONFIG_SPACE_SIZE;
}
static inline uint16_t pci_get_bdf(PCIDevice *dev)
--
2.13.6
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [Qemu-devel] [RFC 4/5] pci: Simplify pci_bus_is_root()
2017-10-03 9:14 ` [Qemu-devel] [RFC 4/5] pci: Simplify pci_bus_is_root() David Gibson
@ 2017-10-03 14:42 ` Eduardo Habkost
2017-10-04 6:03 ` David Gibson
0 siblings, 1 reply; 8+ messages in thread
From: Eduardo Habkost @ 2017-10-03 14:42 UTC (permalink / raw)
To: David Gibson; +Cc: mst, marcel, qemu-devel, alex.williamson
On Tue, Oct 03, 2017 at 08:14:22PM +1100, David Gibson wrote:
> pci_bus_is_root() currently relies on a method in the PCIBusClass.
> But it's always known if a PCI bus is a root bus when we create it, so
> using a dynamic method is overkill.
>
> This replaces it with an IS_ROOT bit in a new flags field, which is set on
> root buses and otherwise clear. As a bonus this removes the special
> is_root logic from pci_expander_bridge, since it already creates its bus
> as a root bus.
>
> Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
>
> # Conflicts:
> # include/hw/pci/pci_bus.h
Should this be part of the commit message?
> ---
[...]
> diff --git a/include/hw/pci/pci.h b/include/hw/pci/pci.h
> index 77d92a3dc4..cbb3386207 100644
> --- a/include/hw/pci/pci.h
> +++ b/include/hw/pci/pci.h
> @@ -404,6 +404,11 @@ typedef AddressSpace *(*PCIIOMMUFunc)(PCIBus *, void *, int);
> #define PCI_BUS_GET_CLASS(obj) OBJECT_GET_CLASS(PCIBusClass, (obj), TYPE_PCI_BUS)
> #define TYPE_PCIE_BUS "PCIE"
>
> +enum PCIBusFlags {
> + /* This bus is the root of a PCI domain */
> + PCI_BUS_IS_ROOT = 0x0001,
> +};
> +
> typedef struct PCIBusClass {
> /*< private >*/
> BusClass parent_class;
> @@ -416,6 +421,7 @@ typedef struct PCIBusClass {
>
> struct PCIBus {
> BusState qbus;
> + enum PCIBusFlags flags;
Why not a simple boolean field? If we want to keep the struct
size smaller when adding more flags, we can use bit-fields.
> PCIIOMMUFunc iommu_fn;
> void *iommu_opaque;
> uint8_t devfn_min;
> @@ -440,8 +446,12 @@ struct PCIBus {
> Notifier machine_done;
> };
>
> +static inline bool pci_bus_is_root(PCIBus *bus)
> +{
> + return !!(bus->flags & PCI_BUS_IS_ROOT);
> +}
> +
> bool pci_bus_is_express(PCIBus *bus);
> -bool pci_bus_is_root(PCIBus *bus);
> void pci_root_bus_new_inplace(PCIBus *bus, size_t bus_size, DeviceState *parent,
> const char *name,
> MemoryRegion *address_space_mem,
> --
> 2.13.6
>
--
Eduardo
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Qemu-devel] [RFC 4/5] pci: Simplify pci_bus_is_root()
2017-10-03 14:42 ` Eduardo Habkost
@ 2017-10-04 6:03 ` David Gibson
0 siblings, 0 replies; 8+ messages in thread
From: David Gibson @ 2017-10-04 6:03 UTC (permalink / raw)
To: Eduardo Habkost; +Cc: mst, marcel, qemu-devel, alex.williamson
[-- Attachment #1: Type: text/plain, Size: 2823 bytes --]
On Tue, Oct 03, 2017 at 11:42:59AM -0300, Eduardo Habkost wrote:
> On Tue, Oct 03, 2017 at 08:14:22PM +1100, David Gibson wrote:
> > pci_bus_is_root() currently relies on a method in the PCIBusClass.
> > But it's always known if a PCI bus is a root bus when we create it, so
> > using a dynamic method is overkill.
> >
> > This replaces it with an IS_ROOT bit in a new flags field, which is set on
> > root buses and otherwise clear. As a bonus this removes the special
> > is_root logic from pci_expander_bridge, since it already creates its bus
> > as a root bus.
> >
> > Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
> >
> > # Conflicts:
> > # include/hw/pci/pci_bus.h
>
> Should this be part of the commit message?
Oops. I'll fix that for the next spin.
> > ---
> [...]
> > diff --git a/include/hw/pci/pci.h b/include/hw/pci/pci.h
> > index 77d92a3dc4..cbb3386207 100644
> > --- a/include/hw/pci/pci.h
> > +++ b/include/hw/pci/pci.h
> > @@ -404,6 +404,11 @@ typedef AddressSpace *(*PCIIOMMUFunc)(PCIBus *, void *, int);
> > #define PCI_BUS_GET_CLASS(obj) OBJECT_GET_CLASS(PCIBusClass, (obj), TYPE_PCI_BUS)
> > #define TYPE_PCIE_BUS "PCIE"
> >
> > +enum PCIBusFlags {
> > + /* This bus is the root of a PCI domain */
> > + PCI_BUS_IS_ROOT = 0x0001,
> > +};
> > +
> > typedef struct PCIBusClass {
> > /*< private >*/
> > BusClass parent_class;
> > @@ -416,6 +421,7 @@ typedef struct PCIBusClass {
> >
> > struct PCIBus {
> > BusState qbus;
> > + enum PCIBusFlags flags;
>
> Why not a simple boolean field? If we want to keep the struct
> size smaller when adding more flags, we can use bit-fields.
Well, mst suggested "flags", so this is what came to mind. I guess
being a past kernel dev has given be a tendency to avoid bitfields,
though I don't think any of the problems they can cause would apply
here.
Note that I do add a second flags bit in patch 5/5.
>
> > PCIIOMMUFunc iommu_fn;
> > void *iommu_opaque;
> > uint8_t devfn_min;
> > @@ -440,8 +446,12 @@ struct PCIBus {
> > Notifier machine_done;
> > };
> >
> > +static inline bool pci_bus_is_root(PCIBus *bus)
> > +{
> > + return !!(bus->flags & PCI_BUS_IS_ROOT);
> > +}
> > +
> > bool pci_bus_is_express(PCIBus *bus);
> > -bool pci_bus_is_root(PCIBus *bus);
> > void pci_root_bus_new_inplace(PCIBus *bus, size_t bus_size, DeviceState *parent,
> > const char *name,
> > MemoryRegion *address_space_mem,
>
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2017-10-04 6:17 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-10-03 9:14 [Qemu-devel] [RFC 0/5] Assorted PCI/PCIe cleanups cleanups David Gibson
2017-10-03 9:14 ` [Qemu-devel] [RFC 1/5] pci: Rename root bus initialization functions for clarity David Gibson
2017-10-03 9:14 ` [Qemu-devel] [RFC 2/5] pci: Move bridge data structures from pci_bus.h to pci_bridge.h David Gibson
2017-10-03 9:14 ` [Qemu-devel] [RFC 3/5] pci: Fold pci_bus.h into pci.h David Gibson
2017-10-03 9:14 ` [Qemu-devel] [RFC 4/5] pci: Simplify pci_bus_is_root() David Gibson
2017-10-03 14:42 ` Eduardo Habkost
2017-10-04 6:03 ` David Gibson
2017-10-03 9:14 ` [Qemu-devel] [RFC 5/5] pcie: Don't allow extended config space access via conventional PCI bridges David Gibson
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).