* [Qemu-devel] [RFC 0/7] pci: Type-safety and phb->bus initialization cleanup
@ 2017-04-17 21:59 Eduardo Habkost
2017-04-17 21:59 ` [Qemu-devel] [RFC 1/7] pci: Change pci_host_bus_register() parameter to PCIHostState Eduardo Habkost
` (6 more replies)
0 siblings, 7 replies; 29+ messages in thread
From: Eduardo Habkost @ 2017-04-17 21:59 UTC (permalink / raw)
To: qemu-devel
Cc: aik, David Gibson, Michael S. Tsirkin, Laszlo Ersek, Marcel Apfelbaum
I've noticed that pci_bus_new*() and pci_register_bus() require
'parent' to be a PCI_HOST_BRIDGE object, but this is not clear
from the function signatures.
This series implements two changes in the PCI code:
1) Replace DeviceState with PCIHostState on functions that
already require a PCI_HOST_BRIDGE argument. Makes the
functions harder to misuse.
2) Move PCIHostState::bus initialization inside pci_bus_new*(),
to avoid code duplication and make sure the field will be
always initialized consistently.
Eduardo Habkost (7):
pci: Change pci_host_bus_register() parameter to PCIHostState
pci: Change pci_bus_init() 'parent' parameter to PCIHostState
pci: Change pci_bus_new*() parameter to PCIHostState
pci: Change pci_register_bus() 'parent' parameter to PCIHostState
pci: Set phb->bus inside pci_register_bus()
pci: Set phb->bus inside pci_bus_new()
pci: Set phb->bus inside pci_bus_new_inplace()
include/hw/pci/pci.h | 17 ++++++++--------
hw/alpha/typhoon.c | 10 +++++-----
hw/mips/gt64xxx_pci.c | 9 +++------
hw/pci-bridge/pci_expander_bridge.c | 15 +++++++-------
hw/pci-host/apb.c | 7 ++-----
hw/pci-host/bonito.c | 7 +++----
hw/pci-host/gpex.c | 5 ++---
hw/pci-host/grackle.c | 9 ++-------
hw/pci-host/piix.c | 3 +--
hw/pci-host/ppce500.c | 8 ++++----
hw/pci-host/prep.c | 4 +---
hw/pci-host/q35.c | 6 +++---
hw/pci-host/uninorth.c | 18 ++++++-----------
hw/pci-host/versatile.c | 3 +--
hw/pci-host/xilinx-pcie.c | 6 +++---
hw/pci/pci.c | 40 ++++++++++++++++++-------------------
hw/ppc/ppc4xx_pci.c | 8 ++++----
hw/ppc/spapr_pci.c | 10 +++++-----
hw/s390x/s390-pci-bus.c | 10 +++++-----
hw/sh4/sh_pci.c | 9 +++------
20 files changed, 89 insertions(+), 115 deletions(-)
--
2.11.0.259.g40922b1
^ permalink raw reply [flat|nested] 29+ messages in thread
* [Qemu-devel] [RFC 1/7] pci: Change pci_host_bus_register() parameter to PCIHostState
2017-04-17 21:59 [Qemu-devel] [RFC 0/7] pci: Type-safety and phb->bus initialization cleanup Eduardo Habkost
@ 2017-04-17 21:59 ` Eduardo Habkost
2017-04-18 3:46 ` David Gibson
2017-04-18 13:01 ` Marcel Apfelbaum
2017-04-17 21:59 ` [Qemu-devel] [RFC 2/7] pci: Change pci_bus_init() 'parent' " Eduardo Habkost
` (5 subsequent siblings)
6 siblings, 2 replies; 29+ messages in thread
From: Eduardo Habkost @ 2017-04-17 21:59 UTC (permalink / raw)
To: qemu-devel
Cc: aik, David Gibson, Michael S. Tsirkin, Laszlo Ersek, Marcel Apfelbaum
The function requires a PCI_HOST_BRIDGE object, so change the parameter
type to reflect that.
Cc: "Michael S. Tsirkin" <mst@redhat.com>
Cc: Marcel Apfelbaum <marcel@redhat.com>
Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
hw/pci/pci.c | 8 +++-----
1 file changed, 3 insertions(+), 5 deletions(-)
diff --git a/hw/pci/pci.c b/hw/pci/pci.c
index 259483b1c0..25118fb91d 100644
--- a/hw/pci/pci.c
+++ b/hw/pci/pci.c
@@ -312,11 +312,9 @@ static void pcibus_reset(BusState *qbus)
}
}
-static void pci_host_bus_register(DeviceState *host)
+static void pci_host_bus_register(PCIHostState *phb)
{
- PCIHostState *host_bridge = PCI_HOST_BRIDGE(host);
-
- QLIST_INSERT_HEAD(&pci_host_bridges, host_bridge, next);
+ QLIST_INSERT_HEAD(&pci_host_bridges, phb, next);
}
PCIBus *pci_find_primary_bus(void)
@@ -377,7 +375,7 @@ static void pci_bus_init(PCIBus *bus, DeviceState *parent,
/* host bridge */
QLIST_INIT(&bus->child);
- pci_host_bus_register(parent);
+ pci_host_bus_register(PCI_HOST_BRIDGE(host));
}
bool pci_bus_is_express(PCIBus *bus)
--
2.11.0.259.g40922b1
^ permalink raw reply related [flat|nested] 29+ messages in thread
* [Qemu-devel] [RFC 2/7] pci: Change pci_bus_init() 'parent' parameter to PCIHostState
2017-04-17 21:59 [Qemu-devel] [RFC 0/7] pci: Type-safety and phb->bus initialization cleanup Eduardo Habkost
2017-04-17 21:59 ` [Qemu-devel] [RFC 1/7] pci: Change pci_host_bus_register() parameter to PCIHostState Eduardo Habkost
@ 2017-04-17 21:59 ` Eduardo Habkost
2017-04-18 3:51 ` David Gibson
2017-04-17 21:59 ` [Qemu-devel] [RFC 3/7] pci: Change pci_bus_new*() " Eduardo Habkost
` (4 subsequent siblings)
6 siblings, 1 reply; 29+ messages in thread
From: Eduardo Habkost @ 2017-04-17 21:59 UTC (permalink / raw)
To: qemu-devel
Cc: aik, David Gibson, Michael S. Tsirkin, Laszlo Ersek, Marcel Apfelbaum
pci_bus_init() already requires 'parent' to be a PCI_HOST_BRIDGE object,
so change the parameter type to reflect that.
Cc: "Michael S. Tsirkin" <mst@redhat.com>
Cc: Marcel Apfelbaum <marcel@redhat.com>
Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
hw/pci/pci.c | 10 ++++++----
1 file changed, 6 insertions(+), 4 deletions(-)
diff --git a/hw/pci/pci.c b/hw/pci/pci.c
index 25118fb91d..d9535c0bdc 100644
--- a/hw/pci/pci.c
+++ b/hw/pci/pci.c
@@ -362,7 +362,7 @@ const char *pci_root_bus_path(PCIDevice *dev)
return rootbus->qbus.name;
}
-static void pci_bus_init(PCIBus *bus, DeviceState *parent,
+static void pci_bus_init(PCIBus *bus, PCIHostState *phb,
MemoryRegion *address_space_mem,
MemoryRegion *address_space_io,
uint8_t devfn_min)
@@ -375,7 +375,7 @@ static void pci_bus_init(PCIBus *bus, DeviceState *parent,
/* host bridge */
QLIST_INIT(&bus->child);
- pci_host_bus_register(PCI_HOST_BRIDGE(host));
+ pci_host_bus_register(phb);
}
bool pci_bus_is_express(PCIBus *bus)
@@ -394,8 +394,9 @@ void pci_bus_new_inplace(PCIBus *bus, size_t bus_size, DeviceState *parent,
MemoryRegion *address_space_io,
uint8_t devfn_min, const char *typename)
{
+ PCIHostState *phb = PCI_HOST_BRIDGE(parent);
qbus_create_inplace(bus, bus_size, typename, parent, name);
- pci_bus_init(bus, parent, address_space_mem, address_space_io, devfn_min);
+ pci_bus_init(bus, phb, address_space_mem, address_space_io, devfn_min);
}
PCIBus *pci_bus_new(DeviceState *parent, const char *name,
@@ -403,10 +404,11 @@ PCIBus *pci_bus_new(DeviceState *parent, const char *name,
MemoryRegion *address_space_io,
uint8_t devfn_min, const char *typename)
{
+ PCIHostState *phb = PCI_HOST_BRIDGE(parent);
PCIBus *bus;
bus = PCI_BUS(qbus_create(typename, parent, name));
- pci_bus_init(bus, parent, address_space_mem, address_space_io, devfn_min);
+ pci_bus_init(bus, phb, address_space_mem, address_space_io, devfn_min);
return bus;
}
--
2.11.0.259.g40922b1
^ permalink raw reply related [flat|nested] 29+ messages in thread
* [Qemu-devel] [RFC 3/7] pci: Change pci_bus_new*() parameter to PCIHostState
2017-04-17 21:59 [Qemu-devel] [RFC 0/7] pci: Type-safety and phb->bus initialization cleanup Eduardo Habkost
2017-04-17 21:59 ` [Qemu-devel] [RFC 1/7] pci: Change pci_host_bus_register() parameter to PCIHostState Eduardo Habkost
2017-04-17 21:59 ` [Qemu-devel] [RFC 2/7] pci: Change pci_bus_init() 'parent' " Eduardo Habkost
@ 2017-04-17 21:59 ` Eduardo Habkost
2017-04-18 3:52 ` David Gibson
` (2 more replies)
2017-04-17 21:59 ` [Qemu-devel] [RFC 4/7] pci: Change pci_register_bus() 'parent' " Eduardo Habkost
` (3 subsequent siblings)
6 siblings, 3 replies; 29+ messages in thread
From: Eduardo Habkost @ 2017-04-17 21:59 UTC (permalink / raw)
To: qemu-devel
Cc: aik, David Gibson, Michael S. Tsirkin, Laszlo Ersek,
Marcel Apfelbaum, Hervé Poussineau, Peter Maydell, qemu-ppc,
qemu-arm
The pci_bus_new*() functions already require the 'parent' argument to be
a PCI_HOST_BRIDGE object. Change the parameter type to reflect that.
Cc: "Michael S. Tsirkin" <mst@redhat.com>
Cc: Marcel Apfelbaum <marcel@redhat.com>
Cc: "Hervé Poussineau" <hpoussin@reactos.org>
Cc: Peter Maydell <peter.maydell@linaro.org>
Cc: qemu-ppc@nongnu.org
Cc: qemu-arm@nongnu.org
Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
include/hw/pci/pci.h | 5 +++--
hw/pci-bridge/pci_expander_bridge.c | 15 ++++++++-------
hw/pci-host/piix.c | 2 +-
hw/pci-host/prep.c | 2 +-
hw/pci-host/q35.c | 2 +-
hw/pci-host/versatile.c | 2 +-
hw/pci/pci.c | 13 ++++++-------
7 files changed, 21 insertions(+), 20 deletions(-)
diff --git a/include/hw/pci/pci.h b/include/hw/pci/pci.h
index a37a2d5cb6..2242aa25eb 100644
--- a/include/hw/pci/pci.h
+++ b/include/hw/pci/pci.h
@@ -393,12 +393,13 @@ 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,
+void pci_bus_new_inplace(PCIBus *bus, size_t bus_size,
+ PCIHostState *phb,
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,
+PCIBus *pci_bus_new(PCIHostState *phb, const char *name,
MemoryRegion *address_space_mem,
MemoryRegion *address_space_io,
uint8_t devfn_min, const char *typename);
diff --git a/hw/pci-bridge/pci_expander_bridge.c b/hw/pci-bridge/pci_expander_bridge.c
index 6ac187fa32..39d29d2230 100644
--- a/hw/pci-bridge/pci_expander_bridge.c
+++ b/hw/pci-bridge/pci_expander_bridge.c
@@ -213,7 +213,8 @@ static gint pxb_compare(gconstpointer a, gconstpointer b)
static void pxb_dev_realize_common(PCIDevice *dev, bool pcie, Error **errp)
{
PXBDev *pxb = convert_to_pxb(dev);
- DeviceState *ds, *bds = NULL;
+ DeviceState *bds = NULL;
+ PCIHostState *phb;
PCIBus *bus;
const char *dev_name = NULL;
Error *local_err = NULL;
@@ -228,11 +229,11 @@ static void pxb_dev_realize_common(PCIDevice *dev, bool pcie, Error **errp)
dev_name = dev->qdev.id;
}
- ds = qdev_create(NULL, TYPE_PXB_HOST);
+ phb = PCI_HOST_BRIDGE(qdev_create(NULL, TYPE_PXB_HOST));
if (pcie) {
- bus = pci_bus_new(ds, dev_name, NULL, NULL, 0, TYPE_PXB_PCIE_BUS);
+ bus = pci_bus_new(phb, 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_bus_new(phb, "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);
@@ -244,7 +245,7 @@ static void pxb_dev_realize_common(PCIDevice *dev, bool pcie, Error **errp)
bus->address_space_io = dev->bus->address_space_io;
bus->map_irq = pxb_map_irq_fn;
- PCI_HOST_BRIDGE(ds)->bus = bus;
+ phb->bus = bus;
pxb_register_bus(dev, bus, &local_err);
if (local_err) {
@@ -252,7 +253,7 @@ static void pxb_dev_realize_common(PCIDevice *dev, bool pcie, Error **errp)
goto err_register_bus;
}
- qdev_init_nofail(ds);
+ qdev_init_nofail(DEVICE(phb));
if (bds) {
qdev_init_nofail(bds);
}
@@ -267,7 +268,7 @@ static void pxb_dev_realize_common(PCIDevice *dev, bool pcie, Error **errp)
err_register_bus:
object_unref(OBJECT(bds));
object_unparent(OBJECT(bus));
- object_unref(OBJECT(ds));
+ object_unref(OBJECT(phb));
}
static void pxb_dev_realize(PCIDevice *dev, Error **errp)
diff --git a/hw/pci-host/piix.c b/hw/pci-host/piix.c
index f9218aa952..91fec05b38 100644
--- a/hw/pci-host/piix.c
+++ b/hw/pci-host/piix.c
@@ -340,7 +340,7 @@ 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,
+ b = pci_bus_new(s, 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);
diff --git a/hw/pci-host/prep.c b/hw/pci-host/prep.c
index 260a119a9e..2e2cd267f4 100644
--- a/hw/pci-host/prep.c
+++ b/hw/pci-host/prep.c
@@ -269,7 +269,7 @@ 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,
+ pci_bus_new_inplace(&s->pci_bus, sizeof(s->pci_bus), h, NULL,
&s->pci_memory, &s->pci_io, 0, TYPE_PCI_BUS);
/* Bus master address space */
diff --git a/hw/pci-host/q35.c b/hw/pci-host/q35.c
index 344f77b10c..860b47a1ba 100644
--- a/hw/pci-host/q35.c
+++ b/hw/pci-host/q35.c
@@ -49,7 +49,7 @@ 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",
+ pci->bus = pci_bus_new(pci, "pcie.0",
s->mch.pci_address_space, s->mch.address_space_io,
0, TYPE_PCIE_BUS);
PC_MACHINE(qdev_get_machine())->bus = pci->bus;
diff --git a/hw/pci-host/versatile.c b/hw/pci-host/versatile.c
index 467cbb9cb8..24ef87610b 100644
--- a/hw/pci-host/versatile.c
+++ b/hw/pci-host/versatile.c
@@ -386,7 +386,7 @@ static void pci_vpb_init(Object *obj)
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), DEVICE(obj), "pci",
+ pci_bus_new_inplace(&s->pci_bus, sizeof(s->pci_bus), h, "pci",
&s->pci_mem_space, &s->pci_io_space,
PCI_DEVFN(11, 0), TYPE_PCI_BUS);
h->bus = &s->pci_bus;
diff --git a/hw/pci/pci.c b/hw/pci/pci.c
index d9535c0bdc..f4488b46fc 100644
--- a/hw/pci/pci.c
+++ b/hw/pci/pci.c
@@ -388,26 +388,25 @@ 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,
+void pci_bus_new_inplace(PCIBus *bus, size_t bus_size,
+ PCIHostState *phb,
const char *name,
MemoryRegion *address_space_mem,
MemoryRegion *address_space_io,
uint8_t devfn_min, const char *typename)
{
- PCIHostState *phb = PCI_HOST_BRIDGE(parent);
- qbus_create_inplace(bus, bus_size, typename, parent, name);
+ qbus_create_inplace(bus, bus_size, typename, DEVICE(phb), name);
pci_bus_init(bus, phb, address_space_mem, address_space_io, devfn_min);
}
-PCIBus *pci_bus_new(DeviceState *parent, const char *name,
+PCIBus *pci_bus_new(PCIHostState *phb, const char *name,
MemoryRegion *address_space_mem,
MemoryRegion *address_space_io,
uint8_t devfn_min, const char *typename)
{
- PCIHostState *phb = PCI_HOST_BRIDGE(parent);
PCIBus *bus;
- bus = PCI_BUS(qbus_create(typename, parent, name));
+ bus = PCI_BUS(qbus_create(typename, DEVICE(phb), name));
pci_bus_init(bus, phb, address_space_mem, address_space_io, devfn_min);
return bus;
}
@@ -431,7 +430,7 @@ PCIBus *pci_register_bus(DeviceState *parent, const char *name,
{
PCIBus *bus;
- bus = pci_bus_new(parent, name, address_space_mem,
+ bus = pci_bus_new(PCI_HOST_BRIDGE(parent), name, address_space_mem,
address_space_io, devfn_min, typename);
pci_bus_irqs(bus, set_irq, map_irq, irq_opaque, nirq);
return bus;
--
2.11.0.259.g40922b1
^ permalink raw reply related [flat|nested] 29+ messages in thread
* [Qemu-devel] [RFC 4/7] pci: Change pci_register_bus() 'parent' parameter to PCIHostState
2017-04-17 21:59 [Qemu-devel] [RFC 0/7] pci: Type-safety and phb->bus initialization cleanup Eduardo Habkost
` (2 preceding siblings ...)
2017-04-17 21:59 ` [Qemu-devel] [RFC 3/7] pci: Change pci_bus_new*() " Eduardo Habkost
@ 2017-04-17 21:59 ` Eduardo Habkost
2017-04-18 3:53 ` David Gibson
` (3 more replies)
2017-04-17 21:59 ` [Qemu-devel] [RFC 5/7] pci: Set phb->bus inside pci_register_bus() Eduardo Habkost
` (2 subsequent siblings)
6 siblings, 4 replies; 29+ messages in thread
From: Eduardo Habkost @ 2017-04-17 21:59 UTC (permalink / raw)
To: qemu-devel
Cc: aik, David Gibson, Michael S. Tsirkin, Laszlo Ersek,
Marcel Apfelbaum, Richard Henderson, Aurelien Jarno, Yongbok Kim,
Alexander Graf, Scott Wood, Paul Burton, David Gibson,
Cornelia Huck, Christian Borntraeger, qemu-ppc
pci_register_bus() already requires the 'parent' argument to be a
PCI_HOST_BRIDGE object. Change the parameter type to reflect that.
Cc: Richard Henderson <rth@twiddle.net>
Cc: Aurelien Jarno <aurelien@aurel32.net>
Cc: Yongbok Kim <yongbok.kim@imgtec.com>
Cc: Alexander Graf <agraf@suse.de>
Cc: Scott Wood <scottwood@freescale.com>
Cc: Paul Burton <paul.burton@imgtec.com>
Cc: "Michael S. Tsirkin" <mst@redhat.com>
Cc: Marcel Apfelbaum <marcel@redhat.com>
Cc: David Gibson <david@gibson.dropbear.id.au>
Cc: Cornelia Huck <cornelia.huck@de.ibm.com>
Cc: Christian Borntraeger <borntraeger@de.ibm.com>
Cc: qemu-ppc@nongnu.org
Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
include/hw/pci/pci.h | 2 +-
hw/alpha/typhoon.c | 2 +-
hw/mips/gt64xxx_pci.c | 2 +-
hw/pci-host/apb.c | 2 +-
hw/pci-host/bonito.c | 2 +-
hw/pci-host/gpex.c | 2 +-
hw/pci-host/grackle.c | 2 +-
hw/pci-host/ppce500.c | 2 +-
hw/pci-host/uninorth.c | 4 ++--
hw/pci-host/xilinx-pcie.c | 2 +-
hw/pci/pci.c | 4 ++--
hw/ppc/ppc4xx_pci.c | 2 +-
hw/ppc/spapr_pci.c | 2 +-
hw/s390x/s390-pci-bus.c | 2 +-
hw/sh4/sh_pci.c | 2 +-
15 files changed, 17 insertions(+), 17 deletions(-)
diff --git a/include/hw/pci/pci.h b/include/hw/pci/pci.h
index 2242aa25eb..56387ccb0c 100644
--- a/include/hw/pci/pci.h
+++ b/include/hw/pci/pci.h
@@ -408,7 +408,7 @@ void pci_bus_irqs(PCIBus *bus, pci_set_irq_fn set_irq, pci_map_irq_fn map_irq,
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,
+PCIBus *pci_register_bus(PCIHostState *phb, const char *name,
pci_set_irq_fn set_irq, pci_map_irq_fn map_irq,
void *irq_opaque,
MemoryRegion *address_space_mem,
diff --git a/hw/alpha/typhoon.c b/hw/alpha/typhoon.c
index f50f5cf186..ac0633a55e 100644
--- a/hw/alpha/typhoon.c
+++ b/hw/alpha/typhoon.c
@@ -883,7 +883,7 @@ 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",
+ b = pci_register_bus(phb, "pci",
typhoon_set_irq, sys_map_irq, s,
&s->pchip.reg_mem, &s->pchip.reg_io,
0, 64, TYPE_PCI_BUS);
diff --git a/hw/mips/gt64xxx_pci.c b/hw/mips/gt64xxx_pci.c
index 4811843ab6..bd131bcdc6 100644
--- a/hw/mips/gt64xxx_pci.c
+++ b/hw/mips/gt64xxx_pci.c
@@ -1171,7 +1171,7 @@ 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",
+ phb->bus = pci_register_bus(phb, "pci",
gt64120_pci_set_irq, gt64120_pci_map_irq,
pic,
&d->pci0_mem,
diff --git a/hw/pci-host/apb.c b/hw/pci-host/apb.c
index 653e711121..1156a54224 100644
--- a/hw/pci-host/apb.c
+++ b/hw/pci-host/apb.c
@@ -671,7 +671,7 @@ 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",
+ phb->bus = pci_register_bus(phb, "pci",
pci_apb_set_irq, pci_pbm_map_irq, d,
&d->pci_mmio,
get_system_io(),
diff --git a/hw/pci-host/bonito.c b/hw/pci-host/bonito.c
index 1999ece590..27842edc04 100644
--- a/hw/pci-host/bonito.c
+++ b/hw/pci-host/bonito.c
@@ -714,7 +714,7 @@ static int bonito_pcihost_initfn(SysBusDevice *dev)
{
PCIHostState *phb = PCI_HOST_BRIDGE(dev);
- phb->bus = pci_register_bus(DEVICE(dev), "pci",
+ phb->bus = pci_register_bus(phb, "pci",
pci_bonito_set_irq, pci_bonito_map_irq, dev,
get_system_memory(), get_system_io(),
0x28, 32, TYPE_PCI_BUS);
diff --git a/hw/pci-host/gpex.c b/hw/pci-host/gpex.c
index 66055ee5cc..042d127271 100644
--- a/hw/pci-host/gpex.c
+++ b/hw/pci-host/gpex.c
@@ -62,7 +62,7 @@ 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->bus = pci_register_bus(pci, "pcie.0", gpex_set_irq,
pci_swizzle_map_irq_fn, s, &s->io_mmio,
&s->io_ioport, 0, 4, TYPE_PCIE_BUS);
diff --git a/hw/pci-host/grackle.c b/hw/pci-host/grackle.c
index 2c8acdaaca..a56c063be9 100644
--- a/hw/pci-host/grackle.c
+++ b/hw/pci-host/grackle.c
@@ -82,7 +82,7 @@ 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,
+ phb->bus = pci_register_bus(phb, NULL,
pci_grackle_set_irq,
pci_grackle_map_irq,
pic,
diff --git a/hw/pci-host/ppce500.c b/hw/pci-host/ppce500.c
index e502bc0505..4a1e99f426 100644
--- a/hw/pci-host/ppce500.c
+++ b/hw/pci-host/ppce500.c
@@ -465,7 +465,7 @@ 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,
+ b = pci_register_bus(h, 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;
diff --git a/hw/pci-host/uninorth.c b/hw/pci-host/uninorth.c
index df342ac3cb..b1b89183fc 100644
--- a/hw/pci-host/uninorth.c
+++ b/hw/pci-host/uninorth.c
@@ -233,7 +233,7 @@ 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,
+ h->bus = pci_register_bus(h, NULL,
pci_unin_set_irq, pci_unin_map_irq,
pic,
&d->pci_mmio,
@@ -299,7 +299,7 @@ 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,
+ h->bus = pci_register_bus(h, NULL,
pci_unin_set_irq, pci_unin_map_irq,
pic,
&d->pci_mmio,
diff --git a/hw/pci-host/xilinx-pcie.c b/hw/pci-host/xilinx-pcie.c
index 8b71e2d950..c951684148 100644
--- a/hw/pci-host/xilinx-pcie.c
+++ b/hw/pci-host/xilinx-pcie.c
@@ -129,7 +129,7 @@ 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->bus = pci_register_bus(pci, s->name, xilinx_pcie_set_irq,
pci_swizzle_map_irq_fn, s, &s->mmio,
&s->io, 0, 4, TYPE_PCIE_BUS);
diff --git a/hw/pci/pci.c b/hw/pci/pci.c
index f4488b46fc..f60d0497ef 100644
--- a/hw/pci/pci.c
+++ b/hw/pci/pci.c
@@ -421,7 +421,7 @@ 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,
+PCIBus *pci_register_bus(PCIHostState *phb, const char *name,
pci_set_irq_fn set_irq, pci_map_irq_fn map_irq,
void *irq_opaque,
MemoryRegion *address_space_mem,
@@ -430,7 +430,7 @@ PCIBus *pci_register_bus(DeviceState *parent, const char *name,
{
PCIBus *bus;
- bus = pci_bus_new(PCI_HOST_BRIDGE(parent), name, address_space_mem,
+ bus = pci_bus_new(phb, 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 dc19682970..f755c6faae 100644
--- a/hw/ppc/ppc4xx_pci.c
+++ b/hw/ppc/ppc4xx_pci.c
@@ -314,7 +314,7 @@ 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,
+ b = pci_register_bus(h, 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;
diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c
index 98c52e411f..0f293f9e75 100644
--- a/hw/ppc/spapr_pci.c
+++ b/hw/ppc/spapr_pci.c
@@ -1697,7 +1697,7 @@ 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,
+ bus = pci_register_bus(phb, NULL,
pci_spapr_set_irq, pci_spapr_map_irq, sphb,
&sphb->memspace, &sphb->iospace,
PCI_DEVFN(0, 0), PCI_NUM_PINS, TYPE_PCI_BUS);
diff --git a/hw/s390x/s390-pci-bus.c b/hw/s390x/s390-pci-bus.c
index 69b0291e8a..99aae965bb 100644
--- a/hw/s390x/s390-pci-bus.c
+++ b/hw/s390x/s390-pci-bus.c
@@ -560,7 +560,7 @@ static int s390_pcihost_init(SysBusDevice *dev)
DPRINTF("host_init\n");
- b = pci_register_bus(DEVICE(dev), NULL,
+ b = pci_register_bus(phb, NULL,
s390_pci_set_irq, s390_pci_map_irq, NULL,
get_system_memory(), get_system_io(), 0, 64,
TYPE_PCI_BUS);
diff --git a/hw/sh4/sh_pci.c b/hw/sh4/sh_pci.c
index 1747628f3d..bca849c10f 100644
--- a/hw/sh4/sh_pci.c
+++ b/hw/sh4/sh_pci.c
@@ -131,7 +131,7 @@ 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",
+ phb->bus = pci_register_bus(phb, "pci",
sh_pci_set_irq, sh_pci_map_irq,
s->irq,
get_system_memory(),
--
2.11.0.259.g40922b1
^ permalink raw reply related [flat|nested] 29+ messages in thread
* [Qemu-devel] [RFC 5/7] pci: Set phb->bus inside pci_register_bus()
2017-04-17 21:59 [Qemu-devel] [RFC 0/7] pci: Type-safety and phb->bus initialization cleanup Eduardo Habkost
` (3 preceding siblings ...)
2017-04-17 21:59 ` [Qemu-devel] [RFC 4/7] pci: Change pci_register_bus() 'parent' " Eduardo Habkost
@ 2017-04-17 21:59 ` Eduardo Habkost
2017-04-18 3:55 ` David Gibson
` (2 more replies)
2017-04-17 21:59 ` [Qemu-devel] [RFC 6/7] pci: Set phb->bus inside pci_bus_new() Eduardo Habkost
2017-04-17 21:59 ` [Qemu-devel] [RFC 7/7] pci: Set phb->bus inside pci_bus_new_inplace() Eduardo Habkost
6 siblings, 3 replies; 29+ messages in thread
From: Eduardo Habkost @ 2017-04-17 21:59 UTC (permalink / raw)
To: qemu-devel
Cc: aik, David Gibson, Michael S. Tsirkin, Laszlo Ersek,
Marcel Apfelbaum, Richard Henderson, Aurelien Jarno, Yongbok Kim,
Alexander Graf, Scott Wood, Paul Burton, David Gibson,
Cornelia Huck, Christian Borntraeger, qemu-ppc
Every single caller of of pci_register_bus() saves the return value in
phb->bus. Do that inside pci_register_bus() to avoid code duplication
and make it harder to break.
Most (but not all) conversions done using the following Coccinelle script:
@@
identifier b;
expression phb;
@@
-b = pci_register_bus(phb, ARGS);
+phb->bus = pci_register_bus(phb, ARGS);
...
-phb->bus = b;
@@
expression phb;
expression list ARGS;
@@
-phb->bus = pci_register_bus(phb, ARGS);
+pci_register_bus(phb, ARGS);
Cc: Richard Henderson <rth@twiddle.net>
Cc: Aurelien Jarno <aurelien@aurel32.net>
Cc: Yongbok Kim <yongbok.kim@imgtec.com>
Cc: Alexander Graf <agraf@suse.de>
Cc: Scott Wood <scottwood@freescale.com>
Cc: Paul Burton <paul.burton@imgtec.com>
Cc: "Michael S. Tsirkin" <mst@redhat.com>
Cc: Marcel Apfelbaum <marcel@redhat.com>
Cc: David Gibson <david@gibson.dropbear.id.au>
Cc: Cornelia Huck <cornelia.huck@de.ibm.com>
Cc: Christian Borntraeger <borntraeger@de.ibm.com>
Cc: qemu-ppc@nongnu.org
Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
include/hw/pci/pci.h | 12 ++++++------
hw/alpha/typhoon.c | 10 +++++-----
hw/mips/gt64xxx_pci.c | 9 +++------
hw/pci-host/apb.c | 7 ++-----
hw/pci-host/bonito.c | 7 +++----
hw/pci-host/gpex.c | 5 ++---
hw/pci-host/grackle.c | 9 ++-------
hw/pci-host/ppce500.c | 8 ++++----
hw/pci-host/uninorth.c | 18 ++++++------------
hw/pci-host/xilinx-pcie.c | 6 +++---
hw/pci/pci.c | 14 +++++++-------
hw/ppc/ppc4xx_pci.c | 8 ++++----
hw/ppc/spapr_pci.c | 10 +++++-----
hw/s390x/s390-pci-bus.c | 10 +++++-----
hw/sh4/sh_pci.c | 9 +++------
15 files changed, 60 insertions(+), 82 deletions(-)
diff --git a/include/hw/pci/pci.h b/include/hw/pci/pci.h
index 56387ccb0c..3b1e2c408a 100644
--- a/include/hw/pci/pci.h
+++ b/include/hw/pci/pci.h
@@ -408,12 +408,12 @@ void pci_bus_irqs(PCIBus *bus, pci_set_irq_fn set_irq, pci_map_irq_fn map_irq,
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(PCIHostState *phb, 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_register_bus(PCIHostState *phb, 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);
diff --git a/hw/alpha/typhoon.c b/hw/alpha/typhoon.c
index ac0633a55e..5926686d79 100644
--- a/hw/alpha/typhoon.c
+++ b/hw/alpha/typhoon.c
@@ -883,11 +883,11 @@ 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(phb, "pci",
- typhoon_set_irq, sys_map_irq, s,
- &s->pchip.reg_mem, &s->pchip.reg_io,
- 0, 64, TYPE_PCI_BUS);
- phb->bus = b;
+ pci_register_bus(phb, "pci",
+ typhoon_set_irq, sys_map_irq, s,
+ &s->pchip.reg_mem, &s->pchip.reg_io,
+ 0, 64, TYPE_PCI_BUS);
+ b = phb->bus;
qdev_init_nofail(dev);
/* Host memory as seen from the PCI side, via the IOMMU. */
diff --git a/hw/mips/gt64xxx_pci.c b/hw/mips/gt64xxx_pci.c
index bd131bcdc6..69963453f0 100644
--- a/hw/mips/gt64xxx_pci.c
+++ b/hw/mips/gt64xxx_pci.c
@@ -1171,12 +1171,9 @@ 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(phb, "pci",
- gt64120_pci_set_irq, gt64120_pci_map_irq,
- pic,
- &d->pci0_mem,
- get_system_io(),
- PCI_DEVFN(18, 0), 4, TYPE_PCI_BUS);
+ pci_register_bus(phb, "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-host/apb.c b/hw/pci-host/apb.c
index 1156a54224..ea86260b04 100644
--- a/hw/pci-host/apb.c
+++ b/hw/pci-host/apb.c
@@ -671,11 +671,8 @@ 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(phb, "pci",
- pci_apb_set_irq, pci_pbm_map_irq, d,
- &d->pci_mmio,
- get_system_io(),
- 0, 32, TYPE_PCI_BUS);
+ pci_register_bus(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 27842edc04..bbe595605d 100644
--- a/hw/pci-host/bonito.c
+++ b/hw/pci-host/bonito.c
@@ -714,10 +714,9 @@ static int bonito_pcihost_initfn(SysBusDevice *dev)
{
PCIHostState *phb = PCI_HOST_BRIDGE(dev);
- phb->bus = pci_register_bus(phb, "pci",
- pci_bonito_set_irq, pci_bonito_map_irq, dev,
- get_system_memory(), get_system_io(),
- 0x28, 32, TYPE_PCI_BUS);
+ pci_register_bus(phb, "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 042d127271..66e2f3d29c 100644
--- a/hw/pci-host/gpex.c
+++ b/hw/pci-host/gpex.c
@@ -62,9 +62,8 @@ static void gpex_host_realize(DeviceState *dev, Error **errp)
sysbus_init_irq(sbd, &s->irq[i]);
}
- pci->bus = pci_register_bus(pci, "pcie.0", gpex_set_irq,
- pci_swizzle_map_irq_fn, s, &s->io_mmio,
- &s->io_ioport, 0, 4, TYPE_PCIE_BUS);
+ pci_register_bus(pci, "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));
qdev_init_nofail(DEVICE(&s->gpex_root));
diff --git a/hw/pci-host/grackle.c b/hw/pci-host/grackle.c
index a56c063be9..691af0a29e 100644
--- a/hw/pci-host/grackle.c
+++ b/hw/pci-host/grackle.c
@@ -82,13 +82,8 @@ 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(phb, NULL,
- pci_grackle_set_irq,
- pci_grackle_map_irq,
- pic,
- &d->pci_mmio,
- address_space_io,
- 0, 4, TYPE_PCI_BUS);
+ pci_register_bus(phb, 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/ppce500.c b/hw/pci-host/ppce500.c
index 4a1e99f426..43a06961d0 100644
--- a/hw/pci-host/ppce500.c
+++ b/hw/pci-host/ppce500.c
@@ -465,10 +465,10 @@ 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(h, 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;
+ pci_register_bus(h, 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 = h->bus;
/* Set up PCI view of memory */
memory_region_init(&s->bm, OBJECT(s), "bm-e500", UINT64_MAX);
diff --git a/hw/pci-host/uninorth.c b/hw/pci-host/uninorth.c
index b1b89183fc..4d709e5468 100644
--- a/hw/pci-host/uninorth.c
+++ b/hw/pci-host/uninorth.c
@@ -233,12 +233,9 @@ PCIBus *pci_pmac_init(qemu_irq *pic,
memory_region_add_subregion(address_space_mem, 0x80000000ULL,
&d->pci_hole);
- h->bus = pci_register_bus(h, NULL,
- pci_unin_set_irq, pci_unin_map_irq,
- pic,
- &d->pci_mmio,
- address_space_io,
- PCI_DEVFN(11, 0), 4, TYPE_PCI_BUS);
+ pci_register_bus(h, 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 +296,9 @@ PCIBus *pci_pmac_u3_init(qemu_irq *pic,
memory_region_add_subregion(address_space_mem, 0x80000000ULL,
&d->pci_hole);
- h->bus = pci_register_bus(h, NULL,
- pci_unin_set_irq, pci_unin_map_irq,
- pic,
- &d->pci_mmio,
- address_space_io,
- PCI_DEVFN(11, 0), 4, TYPE_PCI_BUS);
+ pci_register_bus(h, 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/xilinx-pcie.c b/hw/pci-host/xilinx-pcie.c
index c951684148..6d53677f57 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(pci, s->name, xilinx_pcie_set_irq,
- pci_swizzle_map_irq_fn, s, &s->mmio,
- &s->io, 0, 4, TYPE_PCIE_BUS);
+ pci_register_bus(pci, 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 f60d0497ef..d3adf806e5 100644
--- a/hw/pci/pci.c
+++ b/hw/pci/pci.c
@@ -421,19 +421,19 @@ 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(PCIHostState *phb, 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_register_bus(PCIHostState *phb, 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(phb, name, address_space_mem,
address_space_io, devfn_min, typename);
pci_bus_irqs(bus, set_irq, map_irq, irq_opaque, nirq);
- return bus;
+ phb->bus = bus;
}
int pci_bus_num(PCIBus *s)
diff --git a/hw/ppc/ppc4xx_pci.c b/hw/ppc/ppc4xx_pci.c
index f755c6faae..ab158b3ef1 100644
--- a/hw/ppc/ppc4xx_pci.c
+++ b/hw/ppc/ppc4xx_pci.c
@@ -314,10 +314,10 @@ static int ppc4xx_pcihost_initfn(SysBusDevice *dev)
sysbus_init_irq(dev, &s->irq[i]);
}
- b = pci_register_bus(h, 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_register_bus(h, NULL, ppc4xx_pci_set_irq,
+ ppc4xx_pci_map_irq, s->irq, get_system_memory(),
+ get_system_io(), 0, 4, TYPE_PCI_BUS);
+ b = h->bus;
pci_create_simple(b, 0, "ppc4xx-host-bridge");
diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c
index 0f293f9e75..5749f14d9f 100644
--- a/hw/ppc/spapr_pci.c
+++ b/hw/ppc/spapr_pci.c
@@ -1697,11 +1697,11 @@ 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(phb, 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;
+ pci_register_bus(phb, 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 = phb->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 99aae965bb..466067a380 100644
--- a/hw/s390x/s390-pci-bus.c
+++ b/hw/s390x/s390-pci-bus.c
@@ -560,15 +560,15 @@ static int s390_pcihost_init(SysBusDevice *dev)
DPRINTF("host_init\n");
- b = pci_register_bus(phb, NULL,
- s390_pci_set_irq, s390_pci_map_irq, NULL,
- get_system_memory(), get_system_io(), 0, 64,
- TYPE_PCI_BUS);
+ pci_register_bus(phb, NULL,
+ s390_pci_set_irq, s390_pci_map_irq, NULL,
+ get_system_memory(), get_system_io(), 0, 64,
+ TYPE_PCI_BUS);
+ b = phb->bus;
pci_setup_iommu(b, s390_pci_dma_iommu, s);
bus = BUS(b);
qbus_set_hotplug_handler(bus, DEVICE(dev), NULL);
- phb->bus = b;
s->bus = S390_PCI_BUS(qbus_create(TYPE_S390_PCI_BUS, DEVICE(s), NULL));
qbus_set_hotplug_handler(BUS(s->bus), DEVICE(s), NULL);
diff --git a/hw/sh4/sh_pci.c b/hw/sh4/sh_pci.c
index bca849c10f..bb43bad77d 100644
--- a/hw/sh4/sh_pci.c
+++ b/hw/sh4/sh_pci.c
@@ -131,12 +131,9 @@ 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(phb, "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);
+ pci_register_bus(phb, "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",
--
2.11.0.259.g40922b1
^ permalink raw reply related [flat|nested] 29+ messages in thread
* [Qemu-devel] [RFC 6/7] pci: Set phb->bus inside pci_bus_new()
2017-04-17 21:59 [Qemu-devel] [RFC 0/7] pci: Type-safety and phb->bus initialization cleanup Eduardo Habkost
` (4 preceding siblings ...)
2017-04-17 21:59 ` [Qemu-devel] [RFC 5/7] pci: Set phb->bus inside pci_register_bus() Eduardo Habkost
@ 2017-04-17 21:59 ` Eduardo Habkost
2017-04-18 3:56 ` David Gibson
2017-04-17 21:59 ` [Qemu-devel] [RFC 7/7] pci: Set phb->bus inside pci_bus_new_inplace() Eduardo Habkost
6 siblings, 1 reply; 29+ messages in thread
From: Eduardo Habkost @ 2017-04-17 21:59 UTC (permalink / raw)
To: qemu-devel
Cc: aik, David Gibson, Michael S. Tsirkin, Laszlo Ersek, Marcel Apfelbaum
Every single caller of pci_bus_new() saves the return value inside
phb->bus. Do that inside pci_bus_new() to avoid code duplication and
make it harder to break.
Cc: "Michael S. Tsirkin" <mst@redhat.com>
Cc: Marcel Apfelbaum <marcel@redhat.com>
Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
hw/pci-bridge/pci_expander_bridge.c | 2 --
hw/pci-host/piix.c | 1 -
hw/pci-host/q35.c | 6 +++---
hw/pci/pci.c | 2 +-
4 files changed, 4 insertions(+), 7 deletions(-)
diff --git a/hw/pci-bridge/pci_expander_bridge.c b/hw/pci-bridge/pci_expander_bridge.c
index 39d29d2230..8344ca1cc8 100644
--- a/hw/pci-bridge/pci_expander_bridge.c
+++ b/hw/pci-bridge/pci_expander_bridge.c
@@ -245,8 +245,6 @@ static void pxb_dev_realize_common(PCIDevice *dev, bool pcie, Error **errp)
bus->address_space_io = dev->bus->address_space_io;
bus->map_irq = pxb_map_irq_fn;
- phb->bus = bus;
-
pxb_register_bus(dev, bus, &local_err);
if (local_err) {
error_propagate(errp, local_err);
diff --git a/hw/pci-host/piix.c b/hw/pci-host/piix.c
index 91fec05b38..818e4979d8 100644
--- a/hw/pci-host/piix.c
+++ b/hw/pci-host/piix.c
@@ -342,7 +342,6 @@ PCIBus *i440fx_init(const char *host_type, const char *pci_type,
s = PCI_HOST_BRIDGE(dev);
b = pci_bus_new(s, 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/q35.c b/hw/pci-host/q35.c
index 860b47a1ba..5b41412075 100644
--- a/hw/pci-host/q35.c
+++ b/hw/pci-host/q35.c
@@ -49,9 +49,9 @@ 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(pci, "pcie.0",
- s->mch.pci_address_space, s->mch.address_space_io,
- 0, TYPE_PCIE_BUS);
+ pci_bus_new(pci, "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/pci.c b/hw/pci/pci.c
index d3adf806e5..486aeb7514 100644
--- a/hw/pci/pci.c
+++ b/hw/pci/pci.c
@@ -408,6 +408,7 @@ PCIBus *pci_bus_new(PCIHostState *phb, const char *name,
bus = PCI_BUS(qbus_create(typename, DEVICE(phb), name));
pci_bus_init(bus, phb, address_space_mem, address_space_io, devfn_min);
+ phb->bus = bus;
return bus;
}
@@ -433,7 +434,6 @@ void pci_register_bus(PCIHostState *phb, const char *name,
bus = pci_bus_new(phb, name, address_space_mem,
address_space_io, devfn_min, typename);
pci_bus_irqs(bus, set_irq, map_irq, irq_opaque, nirq);
- phb->bus = bus;
}
int pci_bus_num(PCIBus *s)
--
2.11.0.259.g40922b1
^ permalink raw reply related [flat|nested] 29+ messages in thread
* [Qemu-devel] [RFC 7/7] pci: Set phb->bus inside pci_bus_new_inplace()
2017-04-17 21:59 [Qemu-devel] [RFC 0/7] pci: Type-safety and phb->bus initialization cleanup Eduardo Habkost
` (5 preceding siblings ...)
2017-04-17 21:59 ` [Qemu-devel] [RFC 6/7] pci: Set phb->bus inside pci_bus_new() Eduardo Habkost
@ 2017-04-17 21:59 ` Eduardo Habkost
2017-04-18 3:56 ` David Gibson
6 siblings, 1 reply; 29+ messages in thread
From: Eduardo Habkost @ 2017-04-17 21:59 UTC (permalink / raw)
To: qemu-devel
Cc: aik, David Gibson, Michael S. Tsirkin, Laszlo Ersek,
Marcel Apfelbaum, Hervé Poussineau, Peter Maydell, qemu-arm,
qemu-ppc
Every single caller of pci_bus_new_inplace() sets phb->bus to point to
'bus'. Do that inside pci_bus_new_inplace() to avoid code duplication
and make it harder to break.
Cc: "Hervé Poussineau" <hpoussin@reactos.org>
Cc: Marcel Apfelbaum <marcel@redhat.com>
Cc: "Michael S. Tsirkin" <mst@redhat.com>
Cc: Peter Maydell <peter.maydell@linaro.org>
Cc: qemu-arm@nongnu.org
Cc: qemu-ppc@nongnu.org
Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
hw/pci-host/prep.c | 2 --
hw/pci-host/versatile.c | 1 -
hw/pci/pci.c | 1 +
3 files changed, 1 insertion(+), 3 deletions(-)
diff --git a/hw/pci-host/prep.c b/hw/pci-host/prep.c
index 2e2cd267f4..6efa5bc5ef 100644
--- a/hw/pci-host/prep.c
+++ b/hw/pci-host/prep.c
@@ -284,8 +284,6 @@ static void raven_pcihost_initfn(Object *obj)
address_space_init(&s->bm_as, &s->bm, "raven-bm");
pci_setup_iommu(&s->pci_bus, raven_pcihost_set_iommu, s);
- h->bus = &s->pci_bus;
-
object_initialize(&s->pci_dev, sizeof(s->pci_dev), TYPE_RAVEN_PCI_DEVICE);
pci_dev = DEVICE(&s->pci_dev);
qdev_set_parent_bus(pci_dev, BUS(&s->pci_bus));
diff --git a/hw/pci-host/versatile.c b/hw/pci-host/versatile.c
index 24ef87610b..630f1ac1c5 100644
--- a/hw/pci-host/versatile.c
+++ b/hw/pci-host/versatile.c
@@ -389,7 +389,6 @@ static void pci_vpb_init(Object *obj)
pci_bus_new_inplace(&s->pci_bus, sizeof(s->pci_bus), h, "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);
qdev_set_parent_bus(DEVICE(&s->pci_dev), BUS(&s->pci_bus));
diff --git a/hw/pci/pci.c b/hw/pci/pci.c
index 486aeb7514..ef226f8b41 100644
--- a/hw/pci/pci.c
+++ b/hw/pci/pci.c
@@ -397,6 +397,7 @@ void pci_bus_new_inplace(PCIBus *bus, size_t bus_size,
{
qbus_create_inplace(bus, bus_size, typename, DEVICE(phb), name);
pci_bus_init(bus, phb, address_space_mem, address_space_io, devfn_min);
+ phb->bus = bus;
}
PCIBus *pci_bus_new(PCIHostState *phb, const char *name,
--
2.11.0.259.g40922b1
^ permalink raw reply related [flat|nested] 29+ messages in thread
* Re: [Qemu-devel] [RFC 1/7] pci: Change pci_host_bus_register() parameter to PCIHostState
2017-04-17 21:59 ` [Qemu-devel] [RFC 1/7] pci: Change pci_host_bus_register() parameter to PCIHostState Eduardo Habkost
@ 2017-04-18 3:46 ` David Gibson
2017-04-18 13:01 ` Marcel Apfelbaum
1 sibling, 0 replies; 29+ messages in thread
From: David Gibson @ 2017-04-18 3:46 UTC (permalink / raw)
To: Eduardo Habkost
Cc: qemu-devel, aik, David Gibson, Michael S. Tsirkin, Laszlo Ersek,
Marcel Apfelbaum
[-- Attachment #1: Type: text/plain, Size: 1525 bytes --]
On Mon, Apr 17, 2017 at 06:59:10PM -0300, Eduardo Habkost wrote:
> The function requires a PCI_HOST_BRIDGE object, so change the parameter
> type to reflect that.
>
> Cc: "Michael S. Tsirkin" <mst@redhat.com>
> Cc: Marcel Apfelbaum <marcel@redhat.com>
> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
Reviewed-by: David Gibson <david@gibson.dropbear.id.au>
> ---
> hw/pci/pci.c | 8 +++-----
> 1 file changed, 3 insertions(+), 5 deletions(-)
>
> diff --git a/hw/pci/pci.c b/hw/pci/pci.c
> index 259483b1c0..25118fb91d 100644
> --- a/hw/pci/pci.c
> +++ b/hw/pci/pci.c
> @@ -312,11 +312,9 @@ static void pcibus_reset(BusState *qbus)
> }
> }
>
> -static void pci_host_bus_register(DeviceState *host)
> +static void pci_host_bus_register(PCIHostState *phb)
> {
> - PCIHostState *host_bridge = PCI_HOST_BRIDGE(host);
> -
> - QLIST_INSERT_HEAD(&pci_host_bridges, host_bridge, next);
> + QLIST_INSERT_HEAD(&pci_host_bridges, phb, next);
> }
>
> PCIBus *pci_find_primary_bus(void)
> @@ -377,7 +375,7 @@ static void pci_bus_init(PCIBus *bus, DeviceState *parent,
> /* host bridge */
> QLIST_INIT(&bus->child);
>
> - pci_host_bus_register(parent);
> + pci_host_bus_register(PCI_HOST_BRIDGE(host));
> }
>
> bool pci_bus_is_express(PCIBus *bus)
--
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: 819 bytes --]
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [Qemu-devel] [RFC 2/7] pci: Change pci_bus_init() 'parent' parameter to PCIHostState
2017-04-17 21:59 ` [Qemu-devel] [RFC 2/7] pci: Change pci_bus_init() 'parent' " Eduardo Habkost
@ 2017-04-18 3:51 ` David Gibson
2017-04-18 11:48 ` Eduardo Habkost
0 siblings, 1 reply; 29+ messages in thread
From: David Gibson @ 2017-04-18 3:51 UTC (permalink / raw)
To: Eduardo Habkost
Cc: qemu-devel, aik, David Gibson, Michael S. Tsirkin, Laszlo Ersek,
Marcel Apfelbaum
[-- Attachment #1: Type: text/plain, Size: 2922 bytes --]
On Mon, Apr 17, 2017 at 06:59:11PM -0300, Eduardo Habkost wrote:
> pci_bus_init() already requires 'parent' to be a PCI_HOST_BRIDGE object,
> so change the parameter type to reflect that.
>
> Cc: "Michael S. Tsirkin" <mst@redhat.com>
> Cc: Marcel Apfelbaum <marcel@redhat.com>
> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
Reviewed-by: David Gibson <david@gibson.dropbear.id.au>
I had to do some looking to convince myself this wouldn't break P2P
bridges.
I wonder if we should rename pci_bus_init() / pci_bus_new() to make it
clearer that they're about creating a whole new PCI domain, and not
for a new bus within an existing domain.
> ---
> hw/pci/pci.c | 10 ++++++----
> 1 file changed, 6 insertions(+), 4 deletions(-)
>
> diff --git a/hw/pci/pci.c b/hw/pci/pci.c
> index 25118fb91d..d9535c0bdc 100644
> --- a/hw/pci/pci.c
> +++ b/hw/pci/pci.c
> @@ -362,7 +362,7 @@ const char *pci_root_bus_path(PCIDevice *dev)
> return rootbus->qbus.name;
> }
>
> -static void pci_bus_init(PCIBus *bus, DeviceState *parent,
> +static void pci_bus_init(PCIBus *bus, PCIHostState *phb,
> MemoryRegion *address_space_mem,
> MemoryRegion *address_space_io,
> uint8_t devfn_min)
> @@ -375,7 +375,7 @@ static void pci_bus_init(PCIBus *bus, DeviceState *parent,
> /* host bridge */
> QLIST_INIT(&bus->child);
>
> - pci_host_bus_register(PCI_HOST_BRIDGE(host));
> + pci_host_bus_register(phb);
> }
>
> bool pci_bus_is_express(PCIBus *bus)
> @@ -394,8 +394,9 @@ void pci_bus_new_inplace(PCIBus *bus, size_t bus_size, DeviceState *parent,
> MemoryRegion *address_space_io,
> uint8_t devfn_min, const char *typename)
> {
> + PCIHostState *phb = PCI_HOST_BRIDGE(parent);
> qbus_create_inplace(bus, bus_size, typename, parent, name);
> - pci_bus_init(bus, parent, address_space_mem, address_space_io, devfn_min);
> + pci_bus_init(bus, phb, address_space_mem, address_space_io, devfn_min);
> }
>
> PCIBus *pci_bus_new(DeviceState *parent, const char *name,
> @@ -403,10 +404,11 @@ PCIBus *pci_bus_new(DeviceState *parent, const char *name,
> MemoryRegion *address_space_io,
> uint8_t devfn_min, const char *typename)
> {
> + PCIHostState *phb = PCI_HOST_BRIDGE(parent);
> PCIBus *bus;
>
> bus = PCI_BUS(qbus_create(typename, parent, name));
> - pci_bus_init(bus, parent, address_space_mem, address_space_io, devfn_min);
> + pci_bus_init(bus, phb, address_space_mem, address_space_io, devfn_min);
> return bus;
> }
>
--
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: 819 bytes --]
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [Qemu-devel] [RFC 3/7] pci: Change pci_bus_new*() parameter to PCIHostState
2017-04-17 21:59 ` [Qemu-devel] [RFC 3/7] pci: Change pci_bus_new*() " Eduardo Habkost
@ 2017-04-18 3:52 ` David Gibson
2017-04-18 13:27 ` Marcel Apfelbaum
2017-04-26 16:16 ` Michael S. Tsirkin
2 siblings, 0 replies; 29+ messages in thread
From: David Gibson @ 2017-04-18 3:52 UTC (permalink / raw)
To: Eduardo Habkost
Cc: qemu-devel, aik, David Gibson, Michael S. Tsirkin, Laszlo Ersek,
Marcel Apfelbaum, Hervé Poussineau, Peter Maydell, qemu-ppc,
qemu-arm
[-- Attachment #1: Type: text/plain, Size: 9306 bytes --]
On Mon, Apr 17, 2017 at 06:59:12PM -0300, Eduardo Habkost wrote:
> The pci_bus_new*() functions already require the 'parent' argument to be
> a PCI_HOST_BRIDGE object. Change the parameter type to reflect that.
>
> Cc: "Michael S. Tsirkin" <mst@redhat.com>
> Cc: Marcel Apfelbaum <marcel@redhat.com>
> Cc: "Hervé Poussineau" <hpoussin@reactos.org>
> Cc: Peter Maydell <peter.maydell@linaro.org>
> Cc: qemu-ppc@nongnu.org
> Cc: qemu-arm@nongnu.org
> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
Reviewed-by: David Gibson <david@gibson.dropbear.id.au>
> ---
> include/hw/pci/pci.h | 5 +++--
> hw/pci-bridge/pci_expander_bridge.c | 15 ++++++++-------
> hw/pci-host/piix.c | 2 +-
> hw/pci-host/prep.c | 2 +-
> hw/pci-host/q35.c | 2 +-
> hw/pci-host/versatile.c | 2 +-
> hw/pci/pci.c | 13 ++++++-------
> 7 files changed, 21 insertions(+), 20 deletions(-)
>
> diff --git a/include/hw/pci/pci.h b/include/hw/pci/pci.h
> index a37a2d5cb6..2242aa25eb 100644
> --- a/include/hw/pci/pci.h
> +++ b/include/hw/pci/pci.h
> @@ -393,12 +393,13 @@ 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,
> +void pci_bus_new_inplace(PCIBus *bus, size_t bus_size,
> + PCIHostState *phb,
> 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,
> +PCIBus *pci_bus_new(PCIHostState *phb, const char *name,
> MemoryRegion *address_space_mem,
> MemoryRegion *address_space_io,
> uint8_t devfn_min, const char *typename);
> diff --git a/hw/pci-bridge/pci_expander_bridge.c b/hw/pci-bridge/pci_expander_bridge.c
> index 6ac187fa32..39d29d2230 100644
> --- a/hw/pci-bridge/pci_expander_bridge.c
> +++ b/hw/pci-bridge/pci_expander_bridge.c
> @@ -213,7 +213,8 @@ static gint pxb_compare(gconstpointer a, gconstpointer b)
> static void pxb_dev_realize_common(PCIDevice *dev, bool pcie, Error **errp)
> {
> PXBDev *pxb = convert_to_pxb(dev);
> - DeviceState *ds, *bds = NULL;
> + DeviceState *bds = NULL;
> + PCIHostState *phb;
> PCIBus *bus;
> const char *dev_name = NULL;
> Error *local_err = NULL;
> @@ -228,11 +229,11 @@ static void pxb_dev_realize_common(PCIDevice *dev, bool pcie, Error **errp)
> dev_name = dev->qdev.id;
> }
>
> - ds = qdev_create(NULL, TYPE_PXB_HOST);
> + phb = PCI_HOST_BRIDGE(qdev_create(NULL, TYPE_PXB_HOST));
> if (pcie) {
> - bus = pci_bus_new(ds, dev_name, NULL, NULL, 0, TYPE_PXB_PCIE_BUS);
> + bus = pci_bus_new(phb, 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_bus_new(phb, "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);
> @@ -244,7 +245,7 @@ static void pxb_dev_realize_common(PCIDevice *dev, bool pcie, Error **errp)
> bus->address_space_io = dev->bus->address_space_io;
> bus->map_irq = pxb_map_irq_fn;
>
> - PCI_HOST_BRIDGE(ds)->bus = bus;
> + phb->bus = bus;
>
> pxb_register_bus(dev, bus, &local_err);
> if (local_err) {
> @@ -252,7 +253,7 @@ static void pxb_dev_realize_common(PCIDevice *dev, bool pcie, Error **errp)
> goto err_register_bus;
> }
>
> - qdev_init_nofail(ds);
> + qdev_init_nofail(DEVICE(phb));
> if (bds) {
> qdev_init_nofail(bds);
> }
> @@ -267,7 +268,7 @@ static void pxb_dev_realize_common(PCIDevice *dev, bool pcie, Error **errp)
> err_register_bus:
> object_unref(OBJECT(bds));
> object_unparent(OBJECT(bus));
> - object_unref(OBJECT(ds));
> + object_unref(OBJECT(phb));
> }
>
> static void pxb_dev_realize(PCIDevice *dev, Error **errp)
> diff --git a/hw/pci-host/piix.c b/hw/pci-host/piix.c
> index f9218aa952..91fec05b38 100644
> --- a/hw/pci-host/piix.c
> +++ b/hw/pci-host/piix.c
> @@ -340,7 +340,7 @@ 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,
> + b = pci_bus_new(s, 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);
> diff --git a/hw/pci-host/prep.c b/hw/pci-host/prep.c
> index 260a119a9e..2e2cd267f4 100644
> --- a/hw/pci-host/prep.c
> +++ b/hw/pci-host/prep.c
> @@ -269,7 +269,7 @@ 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,
> + pci_bus_new_inplace(&s->pci_bus, sizeof(s->pci_bus), h, NULL,
> &s->pci_memory, &s->pci_io, 0, TYPE_PCI_BUS);
>
> /* Bus master address space */
> diff --git a/hw/pci-host/q35.c b/hw/pci-host/q35.c
> index 344f77b10c..860b47a1ba 100644
> --- a/hw/pci-host/q35.c
> +++ b/hw/pci-host/q35.c
> @@ -49,7 +49,7 @@ 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",
> + pci->bus = pci_bus_new(pci, "pcie.0",
> s->mch.pci_address_space, s->mch.address_space_io,
> 0, TYPE_PCIE_BUS);
> PC_MACHINE(qdev_get_machine())->bus = pci->bus;
> diff --git a/hw/pci-host/versatile.c b/hw/pci-host/versatile.c
> index 467cbb9cb8..24ef87610b 100644
> --- a/hw/pci-host/versatile.c
> +++ b/hw/pci-host/versatile.c
> @@ -386,7 +386,7 @@ static void pci_vpb_init(Object *obj)
> 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), DEVICE(obj), "pci",
> + pci_bus_new_inplace(&s->pci_bus, sizeof(s->pci_bus), h, "pci",
> &s->pci_mem_space, &s->pci_io_space,
> PCI_DEVFN(11, 0), TYPE_PCI_BUS);
> h->bus = &s->pci_bus;
> diff --git a/hw/pci/pci.c b/hw/pci/pci.c
> index d9535c0bdc..f4488b46fc 100644
> --- a/hw/pci/pci.c
> +++ b/hw/pci/pci.c
> @@ -388,26 +388,25 @@ 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,
> +void pci_bus_new_inplace(PCIBus *bus, size_t bus_size,
> + PCIHostState *phb,
> const char *name,
> MemoryRegion *address_space_mem,
> MemoryRegion *address_space_io,
> uint8_t devfn_min, const char *typename)
> {
> - PCIHostState *phb = PCI_HOST_BRIDGE(parent);
> - qbus_create_inplace(bus, bus_size, typename, parent, name);
> + qbus_create_inplace(bus, bus_size, typename, DEVICE(phb), name);
> pci_bus_init(bus, phb, address_space_mem, address_space_io, devfn_min);
> }
>
> -PCIBus *pci_bus_new(DeviceState *parent, const char *name,
> +PCIBus *pci_bus_new(PCIHostState *phb, const char *name,
> MemoryRegion *address_space_mem,
> MemoryRegion *address_space_io,
> uint8_t devfn_min, const char *typename)
> {
> - PCIHostState *phb = PCI_HOST_BRIDGE(parent);
> PCIBus *bus;
>
> - bus = PCI_BUS(qbus_create(typename, parent, name));
> + bus = PCI_BUS(qbus_create(typename, DEVICE(phb), name));
> pci_bus_init(bus, phb, address_space_mem, address_space_io, devfn_min);
> return bus;
> }
> @@ -431,7 +430,7 @@ PCIBus *pci_register_bus(DeviceState *parent, const char *name,
> {
> PCIBus *bus;
>
> - bus = pci_bus_new(parent, name, address_space_mem,
> + bus = pci_bus_new(PCI_HOST_BRIDGE(parent), name, address_space_mem,
> address_space_io, devfn_min, typename);
> pci_bus_irqs(bus, set_irq, map_irq, irq_opaque, nirq);
> return bus;
--
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: 819 bytes --]
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [Qemu-devel] [RFC 4/7] pci: Change pci_register_bus() 'parent' parameter to PCIHostState
2017-04-17 21:59 ` [Qemu-devel] [RFC 4/7] pci: Change pci_register_bus() 'parent' " Eduardo Habkost
@ 2017-04-18 3:53 ` David Gibson
2017-04-18 10:41 ` Cornelia Huck
` (2 subsequent siblings)
3 siblings, 0 replies; 29+ messages in thread
From: David Gibson @ 2017-04-18 3:53 UTC (permalink / raw)
To: Eduardo Habkost
Cc: qemu-devel, aik, David Gibson, Michael S. Tsirkin, Laszlo Ersek,
Marcel Apfelbaum, Richard Henderson, Aurelien Jarno, Yongbok Kim,
Alexander Graf, Scott Wood, Paul Burton, Cornelia Huck,
Christian Borntraeger, qemu-ppc
[-- Attachment #1: Type: text/plain, Size: 11909 bytes --]
On Mon, Apr 17, 2017 at 06:59:13PM -0300, Eduardo Habkost wrote:
> pci_register_bus() already requires the 'parent' argument to be a
> PCI_HOST_BRIDGE object. Change the parameter type to reflect that.
>
> Cc: Richard Henderson <rth@twiddle.net>
> Cc: Aurelien Jarno <aurelien@aurel32.net>
> Cc: Yongbok Kim <yongbok.kim@imgtec.com>
> Cc: Alexander Graf <agraf@suse.de>
> Cc: Scott Wood <scottwood@freescale.com>
> Cc: Paul Burton <paul.burton@imgtec.com>
> Cc: "Michael S. Tsirkin" <mst@redhat.com>
> Cc: Marcel Apfelbaum <marcel@redhat.com>
> Cc: David Gibson <david@gibson.dropbear.id.au>
> Cc: Cornelia Huck <cornelia.huck@de.ibm.com>
> Cc: Christian Borntraeger <borntraeger@de.ibm.com>
> Cc: qemu-ppc@nongnu.org
> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
Reviewed-by: David Gibson <david@gibson.dropbear.id.au>
> ---
> include/hw/pci/pci.h | 2 +-
> hw/alpha/typhoon.c | 2 +-
> hw/mips/gt64xxx_pci.c | 2 +-
> hw/pci-host/apb.c | 2 +-
> hw/pci-host/bonito.c | 2 +-
> hw/pci-host/gpex.c | 2 +-
> hw/pci-host/grackle.c | 2 +-
> hw/pci-host/ppce500.c | 2 +-
> hw/pci-host/uninorth.c | 4 ++--
> hw/pci-host/xilinx-pcie.c | 2 +-
> hw/pci/pci.c | 4 ++--
> hw/ppc/ppc4xx_pci.c | 2 +-
> hw/ppc/spapr_pci.c | 2 +-
> hw/s390x/s390-pci-bus.c | 2 +-
> hw/sh4/sh_pci.c | 2 +-
> 15 files changed, 17 insertions(+), 17 deletions(-)
>
> diff --git a/include/hw/pci/pci.h b/include/hw/pci/pci.h
> index 2242aa25eb..56387ccb0c 100644
> --- a/include/hw/pci/pci.h
> +++ b/include/hw/pci/pci.h
> @@ -408,7 +408,7 @@ void pci_bus_irqs(PCIBus *bus, pci_set_irq_fn set_irq, pci_map_irq_fn map_irq,
> 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,
> +PCIBus *pci_register_bus(PCIHostState *phb, const char *name,
> pci_set_irq_fn set_irq, pci_map_irq_fn map_irq,
> void *irq_opaque,
> MemoryRegion *address_space_mem,
> diff --git a/hw/alpha/typhoon.c b/hw/alpha/typhoon.c
> index f50f5cf186..ac0633a55e 100644
> --- a/hw/alpha/typhoon.c
> +++ b/hw/alpha/typhoon.c
> @@ -883,7 +883,7 @@ 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",
> + b = pci_register_bus(phb, "pci",
> typhoon_set_irq, sys_map_irq, s,
> &s->pchip.reg_mem, &s->pchip.reg_io,
> 0, 64, TYPE_PCI_BUS);
> diff --git a/hw/mips/gt64xxx_pci.c b/hw/mips/gt64xxx_pci.c
> index 4811843ab6..bd131bcdc6 100644
> --- a/hw/mips/gt64xxx_pci.c
> +++ b/hw/mips/gt64xxx_pci.c
> @@ -1171,7 +1171,7 @@ 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",
> + phb->bus = pci_register_bus(phb, "pci",
> gt64120_pci_set_irq, gt64120_pci_map_irq,
> pic,
> &d->pci0_mem,
> diff --git a/hw/pci-host/apb.c b/hw/pci-host/apb.c
> index 653e711121..1156a54224 100644
> --- a/hw/pci-host/apb.c
> +++ b/hw/pci-host/apb.c
> @@ -671,7 +671,7 @@ 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",
> + phb->bus = pci_register_bus(phb, "pci",
> pci_apb_set_irq, pci_pbm_map_irq, d,
> &d->pci_mmio,
> get_system_io(),
> diff --git a/hw/pci-host/bonito.c b/hw/pci-host/bonito.c
> index 1999ece590..27842edc04 100644
> --- a/hw/pci-host/bonito.c
> +++ b/hw/pci-host/bonito.c
> @@ -714,7 +714,7 @@ static int bonito_pcihost_initfn(SysBusDevice *dev)
> {
> PCIHostState *phb = PCI_HOST_BRIDGE(dev);
>
> - phb->bus = pci_register_bus(DEVICE(dev), "pci",
> + phb->bus = pci_register_bus(phb, "pci",
> pci_bonito_set_irq, pci_bonito_map_irq, dev,
> get_system_memory(), get_system_io(),
> 0x28, 32, TYPE_PCI_BUS);
> diff --git a/hw/pci-host/gpex.c b/hw/pci-host/gpex.c
> index 66055ee5cc..042d127271 100644
> --- a/hw/pci-host/gpex.c
> +++ b/hw/pci-host/gpex.c
> @@ -62,7 +62,7 @@ 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->bus = pci_register_bus(pci, "pcie.0", gpex_set_irq,
> pci_swizzle_map_irq_fn, s, &s->io_mmio,
> &s->io_ioport, 0, 4, TYPE_PCIE_BUS);
>
> diff --git a/hw/pci-host/grackle.c b/hw/pci-host/grackle.c
> index 2c8acdaaca..a56c063be9 100644
> --- a/hw/pci-host/grackle.c
> +++ b/hw/pci-host/grackle.c
> @@ -82,7 +82,7 @@ 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,
> + phb->bus = pci_register_bus(phb, NULL,
> pci_grackle_set_irq,
> pci_grackle_map_irq,
> pic,
> diff --git a/hw/pci-host/ppce500.c b/hw/pci-host/ppce500.c
> index e502bc0505..4a1e99f426 100644
> --- a/hw/pci-host/ppce500.c
> +++ b/hw/pci-host/ppce500.c
> @@ -465,7 +465,7 @@ 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,
> + b = pci_register_bus(h, 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;
> diff --git a/hw/pci-host/uninorth.c b/hw/pci-host/uninorth.c
> index df342ac3cb..b1b89183fc 100644
> --- a/hw/pci-host/uninorth.c
> +++ b/hw/pci-host/uninorth.c
> @@ -233,7 +233,7 @@ 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,
> + h->bus = pci_register_bus(h, NULL,
> pci_unin_set_irq, pci_unin_map_irq,
> pic,
> &d->pci_mmio,
> @@ -299,7 +299,7 @@ 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,
> + h->bus = pci_register_bus(h, NULL,
> pci_unin_set_irq, pci_unin_map_irq,
> pic,
> &d->pci_mmio,
> diff --git a/hw/pci-host/xilinx-pcie.c b/hw/pci-host/xilinx-pcie.c
> index 8b71e2d950..c951684148 100644
> --- a/hw/pci-host/xilinx-pcie.c
> +++ b/hw/pci-host/xilinx-pcie.c
> @@ -129,7 +129,7 @@ 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->bus = pci_register_bus(pci, s->name, xilinx_pcie_set_irq,
> pci_swizzle_map_irq_fn, s, &s->mmio,
> &s->io, 0, 4, TYPE_PCIE_BUS);
>
> diff --git a/hw/pci/pci.c b/hw/pci/pci.c
> index f4488b46fc..f60d0497ef 100644
> --- a/hw/pci/pci.c
> +++ b/hw/pci/pci.c
> @@ -421,7 +421,7 @@ 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,
> +PCIBus *pci_register_bus(PCIHostState *phb, const char *name,
> pci_set_irq_fn set_irq, pci_map_irq_fn map_irq,
> void *irq_opaque,
> MemoryRegion *address_space_mem,
> @@ -430,7 +430,7 @@ PCIBus *pci_register_bus(DeviceState *parent, const char *name,
> {
> PCIBus *bus;
>
> - bus = pci_bus_new(PCI_HOST_BRIDGE(parent), name, address_space_mem,
> + bus = pci_bus_new(phb, 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 dc19682970..f755c6faae 100644
> --- a/hw/ppc/ppc4xx_pci.c
> +++ b/hw/ppc/ppc4xx_pci.c
> @@ -314,7 +314,7 @@ 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,
> + b = pci_register_bus(h, 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;
> diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c
> index 98c52e411f..0f293f9e75 100644
> --- a/hw/ppc/spapr_pci.c
> +++ b/hw/ppc/spapr_pci.c
> @@ -1697,7 +1697,7 @@ 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,
> + bus = pci_register_bus(phb, NULL,
> pci_spapr_set_irq, pci_spapr_map_irq, sphb,
> &sphb->memspace, &sphb->iospace,
> PCI_DEVFN(0, 0), PCI_NUM_PINS, TYPE_PCI_BUS);
> diff --git a/hw/s390x/s390-pci-bus.c b/hw/s390x/s390-pci-bus.c
> index 69b0291e8a..99aae965bb 100644
> --- a/hw/s390x/s390-pci-bus.c
> +++ b/hw/s390x/s390-pci-bus.c
> @@ -560,7 +560,7 @@ static int s390_pcihost_init(SysBusDevice *dev)
>
> DPRINTF("host_init\n");
>
> - b = pci_register_bus(DEVICE(dev), NULL,
> + b = pci_register_bus(phb, NULL,
> s390_pci_set_irq, s390_pci_map_irq, NULL,
> get_system_memory(), get_system_io(), 0, 64,
> TYPE_PCI_BUS);
> diff --git a/hw/sh4/sh_pci.c b/hw/sh4/sh_pci.c
> index 1747628f3d..bca849c10f 100644
> --- a/hw/sh4/sh_pci.c
> +++ b/hw/sh4/sh_pci.c
> @@ -131,7 +131,7 @@ 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",
> + phb->bus = pci_register_bus(phb, "pci",
> sh_pci_set_irq, sh_pci_map_irq,
> s->irq,
> get_system_memory(),
--
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: 819 bytes --]
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [Qemu-devel] [RFC 5/7] pci: Set phb->bus inside pci_register_bus()
2017-04-17 21:59 ` [Qemu-devel] [RFC 5/7] pci: Set phb->bus inside pci_register_bus() Eduardo Habkost
@ 2017-04-18 3:55 ` David Gibson
2017-04-18 10:42 ` Cornelia Huck
2017-04-18 13:43 ` Marcel Apfelbaum
2 siblings, 0 replies; 29+ messages in thread
From: David Gibson @ 2017-04-18 3:55 UTC (permalink / raw)
To: Eduardo Habkost
Cc: qemu-devel, aik, David Gibson, Michael S. Tsirkin, Laszlo Ersek,
Marcel Apfelbaum, Richard Henderson, Aurelien Jarno, Yongbok Kim,
Alexander Graf, Scott Wood, Paul Burton, Cornelia Huck,
Christian Borntraeger, qemu-ppc
[-- Attachment #1: Type: text/plain, Size: 17890 bytes --]
On Mon, Apr 17, 2017 at 06:59:14PM -0300, Eduardo Habkost wrote:
> Every single caller of of pci_register_bus() saves the return value in
> phb->bus. Do that inside pci_register_bus() to avoid code duplication
> and make it harder to break.
>
> Most (but not all) conversions done using the following Coccinelle script:
>
> @@
> identifier b;
> expression phb;
> @@
> -b = pci_register_bus(phb, ARGS);
> +phb->bus = pci_register_bus(phb, ARGS);
> ...
> -phb->bus = b;
>
> @@
> expression phb;
> expression list ARGS;
> @@
> -phb->bus = pci_register_bus(phb, ARGS);
> +pci_register_bus(phb, ARGS);
>
> Cc: Richard Henderson <rth@twiddle.net>
> Cc: Aurelien Jarno <aurelien@aurel32.net>
> Cc: Yongbok Kim <yongbok.kim@imgtec.com>
> Cc: Alexander Graf <agraf@suse.de>
> Cc: Scott Wood <scottwood@freescale.com>
> Cc: Paul Burton <paul.burton@imgtec.com>
> Cc: "Michael S. Tsirkin" <mst@redhat.com>
> Cc: Marcel Apfelbaum <marcel@redhat.com>
> Cc: David Gibson <david@gibson.dropbear.id.au>
> Cc: Cornelia Huck <cornelia.huck@de.ibm.com>
> Cc: Christian Borntraeger <borntraeger@de.ibm.com>
> Cc: qemu-ppc@nongnu.org
> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
Reviewed-by: David Gibson <david@gibson.dropbear.id.au>
> ---
> include/hw/pci/pci.h | 12 ++++++------
> hw/alpha/typhoon.c | 10 +++++-----
> hw/mips/gt64xxx_pci.c | 9 +++------
> hw/pci-host/apb.c | 7 ++-----
> hw/pci-host/bonito.c | 7 +++----
> hw/pci-host/gpex.c | 5 ++---
> hw/pci-host/grackle.c | 9 ++-------
> hw/pci-host/ppce500.c | 8 ++++----
> hw/pci-host/uninorth.c | 18 ++++++------------
> hw/pci-host/xilinx-pcie.c | 6 +++---
> hw/pci/pci.c | 14 +++++++-------
> hw/ppc/ppc4xx_pci.c | 8 ++++----
> hw/ppc/spapr_pci.c | 10 +++++-----
> hw/s390x/s390-pci-bus.c | 10 +++++-----
> hw/sh4/sh_pci.c | 9 +++------
> 15 files changed, 60 insertions(+), 82 deletions(-)
>
> diff --git a/include/hw/pci/pci.h b/include/hw/pci/pci.h
> index 56387ccb0c..3b1e2c408a 100644
> --- a/include/hw/pci/pci.h
> +++ b/include/hw/pci/pci.h
> @@ -408,12 +408,12 @@ void pci_bus_irqs(PCIBus *bus, pci_set_irq_fn set_irq, pci_map_irq_fn map_irq,
> 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(PCIHostState *phb, 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_register_bus(PCIHostState *phb, 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);
> diff --git a/hw/alpha/typhoon.c b/hw/alpha/typhoon.c
> index ac0633a55e..5926686d79 100644
> --- a/hw/alpha/typhoon.c
> +++ b/hw/alpha/typhoon.c
> @@ -883,11 +883,11 @@ 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(phb, "pci",
> - typhoon_set_irq, sys_map_irq, s,
> - &s->pchip.reg_mem, &s->pchip.reg_io,
> - 0, 64, TYPE_PCI_BUS);
> - phb->bus = b;
> + pci_register_bus(phb, "pci",
> + typhoon_set_irq, sys_map_irq, s,
> + &s->pchip.reg_mem, &s->pchip.reg_io,
> + 0, 64, TYPE_PCI_BUS);
> + b = phb->bus;
> qdev_init_nofail(dev);
>
> /* Host memory as seen from the PCI side, via the IOMMU. */
> diff --git a/hw/mips/gt64xxx_pci.c b/hw/mips/gt64xxx_pci.c
> index bd131bcdc6..69963453f0 100644
> --- a/hw/mips/gt64xxx_pci.c
> +++ b/hw/mips/gt64xxx_pci.c
> @@ -1171,12 +1171,9 @@ 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(phb, "pci",
> - gt64120_pci_set_irq, gt64120_pci_map_irq,
> - pic,
> - &d->pci0_mem,
> - get_system_io(),
> - PCI_DEVFN(18, 0), 4, TYPE_PCI_BUS);
> + pci_register_bus(phb, "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-host/apb.c b/hw/pci-host/apb.c
> index 1156a54224..ea86260b04 100644
> --- a/hw/pci-host/apb.c
> +++ b/hw/pci-host/apb.c
> @@ -671,11 +671,8 @@ 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(phb, "pci",
> - pci_apb_set_irq, pci_pbm_map_irq, d,
> - &d->pci_mmio,
> - get_system_io(),
> - 0, 32, TYPE_PCI_BUS);
> + pci_register_bus(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 27842edc04..bbe595605d 100644
> --- a/hw/pci-host/bonito.c
> +++ b/hw/pci-host/bonito.c
> @@ -714,10 +714,9 @@ static int bonito_pcihost_initfn(SysBusDevice *dev)
> {
> PCIHostState *phb = PCI_HOST_BRIDGE(dev);
>
> - phb->bus = pci_register_bus(phb, "pci",
> - pci_bonito_set_irq, pci_bonito_map_irq, dev,
> - get_system_memory(), get_system_io(),
> - 0x28, 32, TYPE_PCI_BUS);
> + pci_register_bus(phb, "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 042d127271..66e2f3d29c 100644
> --- a/hw/pci-host/gpex.c
> +++ b/hw/pci-host/gpex.c
> @@ -62,9 +62,8 @@ static void gpex_host_realize(DeviceState *dev, Error **errp)
> sysbus_init_irq(sbd, &s->irq[i]);
> }
>
> - pci->bus = pci_register_bus(pci, "pcie.0", gpex_set_irq,
> - pci_swizzle_map_irq_fn, s, &s->io_mmio,
> - &s->io_ioport, 0, 4, TYPE_PCIE_BUS);
> + pci_register_bus(pci, "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));
> qdev_init_nofail(DEVICE(&s->gpex_root));
> diff --git a/hw/pci-host/grackle.c b/hw/pci-host/grackle.c
> index a56c063be9..691af0a29e 100644
> --- a/hw/pci-host/grackle.c
> +++ b/hw/pci-host/grackle.c
> @@ -82,13 +82,8 @@ 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(phb, NULL,
> - pci_grackle_set_irq,
> - pci_grackle_map_irq,
> - pic,
> - &d->pci_mmio,
> - address_space_io,
> - 0, 4, TYPE_PCI_BUS);
> + pci_register_bus(phb, 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/ppce500.c b/hw/pci-host/ppce500.c
> index 4a1e99f426..43a06961d0 100644
> --- a/hw/pci-host/ppce500.c
> +++ b/hw/pci-host/ppce500.c
> @@ -465,10 +465,10 @@ 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(h, 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;
> + pci_register_bus(h, 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 = h->bus;
>
> /* Set up PCI view of memory */
> memory_region_init(&s->bm, OBJECT(s), "bm-e500", UINT64_MAX);
> diff --git a/hw/pci-host/uninorth.c b/hw/pci-host/uninorth.c
> index b1b89183fc..4d709e5468 100644
> --- a/hw/pci-host/uninorth.c
> +++ b/hw/pci-host/uninorth.c
> @@ -233,12 +233,9 @@ PCIBus *pci_pmac_init(qemu_irq *pic,
> memory_region_add_subregion(address_space_mem, 0x80000000ULL,
> &d->pci_hole);
>
> - h->bus = pci_register_bus(h, NULL,
> - pci_unin_set_irq, pci_unin_map_irq,
> - pic,
> - &d->pci_mmio,
> - address_space_io,
> - PCI_DEVFN(11, 0), 4, TYPE_PCI_BUS);
> + pci_register_bus(h, 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 +296,9 @@ PCIBus *pci_pmac_u3_init(qemu_irq *pic,
> memory_region_add_subregion(address_space_mem, 0x80000000ULL,
> &d->pci_hole);
>
> - h->bus = pci_register_bus(h, NULL,
> - pci_unin_set_irq, pci_unin_map_irq,
> - pic,
> - &d->pci_mmio,
> - address_space_io,
> - PCI_DEVFN(11, 0), 4, TYPE_PCI_BUS);
> + pci_register_bus(h, 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/xilinx-pcie.c b/hw/pci-host/xilinx-pcie.c
> index c951684148..6d53677f57 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(pci, s->name, xilinx_pcie_set_irq,
> - pci_swizzle_map_irq_fn, s, &s->mmio,
> - &s->io, 0, 4, TYPE_PCIE_BUS);
> + pci_register_bus(pci, 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 f60d0497ef..d3adf806e5 100644
> --- a/hw/pci/pci.c
> +++ b/hw/pci/pci.c
> @@ -421,19 +421,19 @@ 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(PCIHostState *phb, 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_register_bus(PCIHostState *phb, 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(phb, name, address_space_mem,
> address_space_io, devfn_min, typename);
> pci_bus_irqs(bus, set_irq, map_irq, irq_opaque, nirq);
> - return bus;
> + phb->bus = bus;
> }
>
> int pci_bus_num(PCIBus *s)
> diff --git a/hw/ppc/ppc4xx_pci.c b/hw/ppc/ppc4xx_pci.c
> index f755c6faae..ab158b3ef1 100644
> --- a/hw/ppc/ppc4xx_pci.c
> +++ b/hw/ppc/ppc4xx_pci.c
> @@ -314,10 +314,10 @@ static int ppc4xx_pcihost_initfn(SysBusDevice *dev)
> sysbus_init_irq(dev, &s->irq[i]);
> }
>
> - b = pci_register_bus(h, 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_register_bus(h, NULL, ppc4xx_pci_set_irq,
> + ppc4xx_pci_map_irq, s->irq, get_system_memory(),
> + get_system_io(), 0, 4, TYPE_PCI_BUS);
> + b = h->bus;
>
> pci_create_simple(b, 0, "ppc4xx-host-bridge");
>
> diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c
> index 0f293f9e75..5749f14d9f 100644
> --- a/hw/ppc/spapr_pci.c
> +++ b/hw/ppc/spapr_pci.c
> @@ -1697,11 +1697,11 @@ 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(phb, 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;
> + pci_register_bus(phb, 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 = phb->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 99aae965bb..466067a380 100644
> --- a/hw/s390x/s390-pci-bus.c
> +++ b/hw/s390x/s390-pci-bus.c
> @@ -560,15 +560,15 @@ static int s390_pcihost_init(SysBusDevice *dev)
>
> DPRINTF("host_init\n");
>
> - b = pci_register_bus(phb, NULL,
> - s390_pci_set_irq, s390_pci_map_irq, NULL,
> - get_system_memory(), get_system_io(), 0, 64,
> - TYPE_PCI_BUS);
> + pci_register_bus(phb, NULL,
> + s390_pci_set_irq, s390_pci_map_irq, NULL,
> + get_system_memory(), get_system_io(), 0, 64,
> + TYPE_PCI_BUS);
> + b = phb->bus;
> pci_setup_iommu(b, s390_pci_dma_iommu, s);
>
> bus = BUS(b);
> qbus_set_hotplug_handler(bus, DEVICE(dev), NULL);
> - phb->bus = b;
>
> s->bus = S390_PCI_BUS(qbus_create(TYPE_S390_PCI_BUS, DEVICE(s), NULL));
> qbus_set_hotplug_handler(BUS(s->bus), DEVICE(s), NULL);
> diff --git a/hw/sh4/sh_pci.c b/hw/sh4/sh_pci.c
> index bca849c10f..bb43bad77d 100644
> --- a/hw/sh4/sh_pci.c
> +++ b/hw/sh4/sh_pci.c
> @@ -131,12 +131,9 @@ 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(phb, "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);
> + pci_register_bus(phb, "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",
--
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: 819 bytes --]
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [Qemu-devel] [RFC 6/7] pci: Set phb->bus inside pci_bus_new()
2017-04-17 21:59 ` [Qemu-devel] [RFC 6/7] pci: Set phb->bus inside pci_bus_new() Eduardo Habkost
@ 2017-04-18 3:56 ` David Gibson
0 siblings, 0 replies; 29+ messages in thread
From: David Gibson @ 2017-04-18 3:56 UTC (permalink / raw)
To: Eduardo Habkost
Cc: qemu-devel, aik, David Gibson, Michael S. Tsirkin, Laszlo Ersek,
Marcel Apfelbaum
[-- Attachment #1: Type: text/plain, Size: 3593 bytes --]
On Mon, Apr 17, 2017 at 06:59:15PM -0300, Eduardo Habkost wrote:
> Every single caller of pci_bus_new() saves the return value inside
> phb->bus. Do that inside pci_bus_new() to avoid code duplication and
> make it harder to break.
>
> Cc: "Michael S. Tsirkin" <mst@redhat.com>
> Cc: Marcel Apfelbaum <marcel@redhat.com>
> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
Reviewed-by: David Gibson <david@gibson.dropbear.id.au>
> ---
> hw/pci-bridge/pci_expander_bridge.c | 2 --
> hw/pci-host/piix.c | 1 -
> hw/pci-host/q35.c | 6 +++---
> hw/pci/pci.c | 2 +-
> 4 files changed, 4 insertions(+), 7 deletions(-)
>
> diff --git a/hw/pci-bridge/pci_expander_bridge.c b/hw/pci-bridge/pci_expander_bridge.c
> index 39d29d2230..8344ca1cc8 100644
> --- a/hw/pci-bridge/pci_expander_bridge.c
> +++ b/hw/pci-bridge/pci_expander_bridge.c
> @@ -245,8 +245,6 @@ static void pxb_dev_realize_common(PCIDevice *dev, bool pcie, Error **errp)
> bus->address_space_io = dev->bus->address_space_io;
> bus->map_irq = pxb_map_irq_fn;
>
> - phb->bus = bus;
> -
> pxb_register_bus(dev, bus, &local_err);
> if (local_err) {
> error_propagate(errp, local_err);
> diff --git a/hw/pci-host/piix.c b/hw/pci-host/piix.c
> index 91fec05b38..818e4979d8 100644
> --- a/hw/pci-host/piix.c
> +++ b/hw/pci-host/piix.c
> @@ -342,7 +342,6 @@ PCIBus *i440fx_init(const char *host_type, const char *pci_type,
> s = PCI_HOST_BRIDGE(dev);
> b = pci_bus_new(s, 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/q35.c b/hw/pci-host/q35.c
> index 860b47a1ba..5b41412075 100644
> --- a/hw/pci-host/q35.c
> +++ b/hw/pci-host/q35.c
> @@ -49,9 +49,9 @@ 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(pci, "pcie.0",
> - s->mch.pci_address_space, s->mch.address_space_io,
> - 0, TYPE_PCIE_BUS);
> + pci_bus_new(pci, "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/pci.c b/hw/pci/pci.c
> index d3adf806e5..486aeb7514 100644
> --- a/hw/pci/pci.c
> +++ b/hw/pci/pci.c
> @@ -408,6 +408,7 @@ PCIBus *pci_bus_new(PCIHostState *phb, const char *name,
>
> bus = PCI_BUS(qbus_create(typename, DEVICE(phb), name));
> pci_bus_init(bus, phb, address_space_mem, address_space_io, devfn_min);
> + phb->bus = bus;
> return bus;
> }
>
> @@ -433,7 +434,6 @@ void pci_register_bus(PCIHostState *phb, const char *name,
> bus = pci_bus_new(phb, name, address_space_mem,
> address_space_io, devfn_min, typename);
> pci_bus_irqs(bus, set_irq, map_irq, irq_opaque, nirq);
> - phb->bus = bus;
> }
>
> int pci_bus_num(PCIBus *s)
--
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: 819 bytes --]
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [Qemu-devel] [RFC 7/7] pci: Set phb->bus inside pci_bus_new_inplace()
2017-04-17 21:59 ` [Qemu-devel] [RFC 7/7] pci: Set phb->bus inside pci_bus_new_inplace() Eduardo Habkost
@ 2017-04-18 3:56 ` David Gibson
0 siblings, 0 replies; 29+ messages in thread
From: David Gibson @ 2017-04-18 3:56 UTC (permalink / raw)
To: Eduardo Habkost
Cc: qemu-devel, aik, David Gibson, Michael S. Tsirkin, Laszlo Ersek,
Marcel Apfelbaum, Hervé Poussineau, Peter Maydell, qemu-arm,
qemu-ppc
[-- Attachment #1: Type: text/plain, Size: 2608 bytes --]
On Mon, Apr 17, 2017 at 06:59:16PM -0300, Eduardo Habkost wrote:
> Every single caller of pci_bus_new_inplace() sets phb->bus to point to
> 'bus'. Do that inside pci_bus_new_inplace() to avoid code duplication
> and make it harder to break.
>
> Cc: "Hervé Poussineau" <hpoussin@reactos.org>
> Cc: Marcel Apfelbaum <marcel@redhat.com>
> Cc: "Michael S. Tsirkin" <mst@redhat.com>
> Cc: Peter Maydell <peter.maydell@linaro.org>
> Cc: qemu-arm@nongnu.org
> Cc: qemu-ppc@nongnu.org
> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
Reviewed-by: David Gibson <david@gibson.dropbear.id.au>
> ---
> hw/pci-host/prep.c | 2 --
> hw/pci-host/versatile.c | 1 -
> hw/pci/pci.c | 1 +
> 3 files changed, 1 insertion(+), 3 deletions(-)
>
> diff --git a/hw/pci-host/prep.c b/hw/pci-host/prep.c
> index 2e2cd267f4..6efa5bc5ef 100644
> --- a/hw/pci-host/prep.c
> +++ b/hw/pci-host/prep.c
> @@ -284,8 +284,6 @@ static void raven_pcihost_initfn(Object *obj)
> address_space_init(&s->bm_as, &s->bm, "raven-bm");
> pci_setup_iommu(&s->pci_bus, raven_pcihost_set_iommu, s);
>
> - h->bus = &s->pci_bus;
> -
> object_initialize(&s->pci_dev, sizeof(s->pci_dev), TYPE_RAVEN_PCI_DEVICE);
> pci_dev = DEVICE(&s->pci_dev);
> qdev_set_parent_bus(pci_dev, BUS(&s->pci_bus));
> diff --git a/hw/pci-host/versatile.c b/hw/pci-host/versatile.c
> index 24ef87610b..630f1ac1c5 100644
> --- a/hw/pci-host/versatile.c
> +++ b/hw/pci-host/versatile.c
> @@ -389,7 +389,6 @@ static void pci_vpb_init(Object *obj)
> pci_bus_new_inplace(&s->pci_bus, sizeof(s->pci_bus), h, "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);
> qdev_set_parent_bus(DEVICE(&s->pci_dev), BUS(&s->pci_bus));
> diff --git a/hw/pci/pci.c b/hw/pci/pci.c
> index 486aeb7514..ef226f8b41 100644
> --- a/hw/pci/pci.c
> +++ b/hw/pci/pci.c
> @@ -397,6 +397,7 @@ void pci_bus_new_inplace(PCIBus *bus, size_t bus_size,
> {
> qbus_create_inplace(bus, bus_size, typename, DEVICE(phb), name);
> pci_bus_init(bus, phb, address_space_mem, address_space_io, devfn_min);
> + phb->bus = bus;
> }
>
> PCIBus *pci_bus_new(PCIHostState *phb, const char *name,
--
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: 819 bytes --]
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [Qemu-devel] [RFC 4/7] pci: Change pci_register_bus() 'parent' parameter to PCIHostState
2017-04-17 21:59 ` [Qemu-devel] [RFC 4/7] pci: Change pci_register_bus() 'parent' " Eduardo Habkost
2017-04-18 3:53 ` David Gibson
@ 2017-04-18 10:41 ` Cornelia Huck
2017-04-18 13:37 ` Marcel Apfelbaum
2017-04-26 16:17 ` Michael S. Tsirkin
3 siblings, 0 replies; 29+ messages in thread
From: Cornelia Huck @ 2017-04-18 10:41 UTC (permalink / raw)
To: Eduardo Habkost
Cc: qemu-devel, aik, David Gibson, Michael S. Tsirkin, Laszlo Ersek,
Marcel Apfelbaum, Richard Henderson, Aurelien Jarno, Yongbok Kim,
Alexander Graf, Scott Wood, Paul Burton, David Gibson,
Christian Borntraeger, qemu-ppc
On Mon, 17 Apr 2017 18:59:13 -0300
Eduardo Habkost <ehabkost@redhat.com> wrote:
> pci_register_bus() already requires the 'parent' argument to be a
> PCI_HOST_BRIDGE object. Change the parameter type to reflect that.
>
> Cc: Richard Henderson <rth@twiddle.net>
> Cc: Aurelien Jarno <aurelien@aurel32.net>
> Cc: Yongbok Kim <yongbok.kim@imgtec.com>
> Cc: Alexander Graf <agraf@suse.de>
> Cc: Scott Wood <scottwood@freescale.com>
> Cc: Paul Burton <paul.burton@imgtec.com>
> Cc: "Michael S. Tsirkin" <mst@redhat.com>
> Cc: Marcel Apfelbaum <marcel@redhat.com>
> Cc: David Gibson <david@gibson.dropbear.id.au>
> Cc: Cornelia Huck <cornelia.huck@de.ibm.com>
> Cc: Christian Borntraeger <borntraeger@de.ibm.com>
> Cc: qemu-ppc@nongnu.org
> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
> ---
> include/hw/pci/pci.h | 2 +-
> hw/alpha/typhoon.c | 2 +-
> hw/mips/gt64xxx_pci.c | 2 +-
> hw/pci-host/apb.c | 2 +-
> hw/pci-host/bonito.c | 2 +-
> hw/pci-host/gpex.c | 2 +-
> hw/pci-host/grackle.c | 2 +-
> hw/pci-host/ppce500.c | 2 +-
> hw/pci-host/uninorth.c | 4 ++--
> hw/pci-host/xilinx-pcie.c | 2 +-
> hw/pci/pci.c | 4 ++--
> hw/ppc/ppc4xx_pci.c | 2 +-
> hw/ppc/spapr_pci.c | 2 +-
> hw/s390x/s390-pci-bus.c | 2 +-
> hw/sh4/sh_pci.c | 2 +-
> 15 files changed, 17 insertions(+), 17 deletions(-)
s390 parts:
Acked-by: Cornelia Huck <cornelia.huck@de.ibm.com>
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [Qemu-devel] [RFC 5/7] pci: Set phb->bus inside pci_register_bus()
2017-04-17 21:59 ` [Qemu-devel] [RFC 5/7] pci: Set phb->bus inside pci_register_bus() Eduardo Habkost
2017-04-18 3:55 ` David Gibson
@ 2017-04-18 10:42 ` Cornelia Huck
2017-04-18 13:43 ` Marcel Apfelbaum
2 siblings, 0 replies; 29+ messages in thread
From: Cornelia Huck @ 2017-04-18 10:42 UTC (permalink / raw)
To: Eduardo Habkost
Cc: qemu-devel, aik, David Gibson, Michael S. Tsirkin, Laszlo Ersek,
Marcel Apfelbaum, Richard Henderson, Aurelien Jarno, Yongbok Kim,
Alexander Graf, Scott Wood, Paul Burton, David Gibson,
Christian Borntraeger, qemu-ppc
On Mon, 17 Apr 2017 18:59:14 -0300
Eduardo Habkost <ehabkost@redhat.com> wrote:
> Every single caller of of pci_register_bus() saves the return value in
> phb->bus. Do that inside pci_register_bus() to avoid code duplication
> and make it harder to break.
>
> Most (but not all) conversions done using the following Coccinelle script:
>
> @@
> identifier b;
> expression phb;
> @@
> -b = pci_register_bus(phb, ARGS);
> +phb->bus = pci_register_bus(phb, ARGS);
> ...
> -phb->bus = b;
>
> @@
> expression phb;
> expression list ARGS;
> @@
> -phb->bus = pci_register_bus(phb, ARGS);
> +pci_register_bus(phb, ARGS);
>
> Cc: Richard Henderson <rth@twiddle.net>
> Cc: Aurelien Jarno <aurelien@aurel32.net>
> Cc: Yongbok Kim <yongbok.kim@imgtec.com>
> Cc: Alexander Graf <agraf@suse.de>
> Cc: Scott Wood <scottwood@freescale.com>
> Cc: Paul Burton <paul.burton@imgtec.com>
> Cc: "Michael S. Tsirkin" <mst@redhat.com>
> Cc: Marcel Apfelbaum <marcel@redhat.com>
> Cc: David Gibson <david@gibson.dropbear.id.au>
> Cc: Cornelia Huck <cornelia.huck@de.ibm.com>
> Cc: Christian Borntraeger <borntraeger@de.ibm.com>
> Cc: qemu-ppc@nongnu.org
> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
> ---
> include/hw/pci/pci.h | 12 ++++++------
> hw/alpha/typhoon.c | 10 +++++-----
> hw/mips/gt64xxx_pci.c | 9 +++------
> hw/pci-host/apb.c | 7 ++-----
> hw/pci-host/bonito.c | 7 +++----
> hw/pci-host/gpex.c | 5 ++---
> hw/pci-host/grackle.c | 9 ++-------
> hw/pci-host/ppce500.c | 8 ++++----
> hw/pci-host/uninorth.c | 18 ++++++------------
> hw/pci-host/xilinx-pcie.c | 6 +++---
> hw/pci/pci.c | 14 +++++++-------
> hw/ppc/ppc4xx_pci.c | 8 ++++----
> hw/ppc/spapr_pci.c | 10 +++++-----
> hw/s390x/s390-pci-bus.c | 10 +++++-----
> hw/sh4/sh_pci.c | 9 +++------
> 15 files changed, 60 insertions(+), 82 deletions(-)
s390 parts:
Acked-by: Cornelia Huck <cornelia.huck@de.ibm.com>
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [Qemu-devel] [RFC 2/7] pci: Change pci_bus_init() 'parent' parameter to PCIHostState
2017-04-18 3:51 ` David Gibson
@ 2017-04-18 11:48 ` Eduardo Habkost
2017-04-18 13:22 ` Marcel Apfelbaum
0 siblings, 1 reply; 29+ messages in thread
From: Eduardo Habkost @ 2017-04-18 11:48 UTC (permalink / raw)
To: David Gibson
Cc: qemu-devel, aik, David Gibson, Michael S. Tsirkin, Laszlo Ersek,
Marcel Apfelbaum
On Tue, Apr 18, 2017 at 01:51:02PM +1000, David Gibson wrote:
> On Mon, Apr 17, 2017 at 06:59:11PM -0300, Eduardo Habkost wrote:
> > pci_bus_init() already requires 'parent' to be a PCI_HOST_BRIDGE object,
> > so change the parameter type to reflect that.
> >
> > Cc: "Michael S. Tsirkin" <mst@redhat.com>
> > Cc: Marcel Apfelbaum <marcel@redhat.com>
> > Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
>
> Reviewed-by: David Gibson <david@gibson.dropbear.id.au>
>
> I had to do some looking to convince myself this wouldn't break P2P
> bridges.
>
> I wonder if we should rename pci_bus_init() / pci_bus_new() to make it
> clearer that they're about creating a whole new PCI domain, and not
> for a new bus within an existing domain.
Good point. The current naming is confusing. I only knew the
change was safe because pci_bus_new() would crash if 'parent'
wasn't a PCI_HOST_BRIDGE. But I took some time to find the other
places where PCI buses could be created, and I'm not sure yet if
I found all of them.
Is there any other code that creates TYPE_PCI_BUS objects, except
for the ones touched by this series and pci_bridge_initfn()?
>
> > ---
> > hw/pci/pci.c | 10 ++++++----
> > 1 file changed, 6 insertions(+), 4 deletions(-)
> >
> > diff --git a/hw/pci/pci.c b/hw/pci/pci.c
> > index 25118fb91d..d9535c0bdc 100644
> > --- a/hw/pci/pci.c
> > +++ b/hw/pci/pci.c
> > @@ -362,7 +362,7 @@ const char *pci_root_bus_path(PCIDevice *dev)
> > return rootbus->qbus.name;
> > }
> >
> > -static void pci_bus_init(PCIBus *bus, DeviceState *parent,
> > +static void pci_bus_init(PCIBus *bus, PCIHostState *phb,
> > MemoryRegion *address_space_mem,
> > MemoryRegion *address_space_io,
> > uint8_t devfn_min)
> > @@ -375,7 +375,7 @@ static void pci_bus_init(PCIBus *bus, DeviceState *parent,
> > /* host bridge */
> > QLIST_INIT(&bus->child);
> >
> > - pci_host_bus_register(PCI_HOST_BRIDGE(host));
> > + pci_host_bus_register(phb);
> > }
> >
> > bool pci_bus_is_express(PCIBus *bus)
> > @@ -394,8 +394,9 @@ void pci_bus_new_inplace(PCIBus *bus, size_t bus_size, DeviceState *parent,
> > MemoryRegion *address_space_io,
> > uint8_t devfn_min, const char *typename)
> > {
> > + PCIHostState *phb = PCI_HOST_BRIDGE(parent);
> > qbus_create_inplace(bus, bus_size, typename, parent, name);
> > - pci_bus_init(bus, parent, address_space_mem, address_space_io, devfn_min);
> > + pci_bus_init(bus, phb, address_space_mem, address_space_io, devfn_min);
> > }
> >
> > PCIBus *pci_bus_new(DeviceState *parent, const char *name,
> > @@ -403,10 +404,11 @@ PCIBus *pci_bus_new(DeviceState *parent, const char *name,
> > MemoryRegion *address_space_io,
> > uint8_t devfn_min, const char *typename)
> > {
> > + PCIHostState *phb = PCI_HOST_BRIDGE(parent);
> > PCIBus *bus;
> >
> > bus = PCI_BUS(qbus_create(typename, parent, name));
> > - pci_bus_init(bus, parent, address_space_mem, address_space_io, devfn_min);
> > + pci_bus_init(bus, phb, address_space_mem, address_space_io, devfn_min);
> > return bus;
> > }
> >
>
> --
> 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
--
Eduardo
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [Qemu-devel] [RFC 1/7] pci: Change pci_host_bus_register() parameter to PCIHostState
2017-04-17 21:59 ` [Qemu-devel] [RFC 1/7] pci: Change pci_host_bus_register() parameter to PCIHostState Eduardo Habkost
2017-04-18 3:46 ` David Gibson
@ 2017-04-18 13:01 ` Marcel Apfelbaum
1 sibling, 0 replies; 29+ messages in thread
From: Marcel Apfelbaum @ 2017-04-18 13:01 UTC (permalink / raw)
To: Eduardo Habkost, qemu-devel
Cc: aik, David Gibson, Michael S. Tsirkin, Laszlo Ersek
On 04/18/2017 12:59 AM, Eduardo Habkost wrote:
> The function requires a PCI_HOST_BRIDGE object, so change the parameter
> type to reflect that.
>
> Cc: "Michael S. Tsirkin" <mst@redhat.com>
> Cc: Marcel Apfelbaum <marcel@redhat.com>
> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
> ---
> hw/pci/pci.c | 8 +++-----
> 1 file changed, 3 insertions(+), 5 deletions(-)
>
> diff --git a/hw/pci/pci.c b/hw/pci/pci.c
> index 259483b1c0..25118fb91d 100644
> --- a/hw/pci/pci.c
> +++ b/hw/pci/pci.c
> @@ -312,11 +312,9 @@ static void pcibus_reset(BusState *qbus)
> }
> }
>
> -static void pci_host_bus_register(DeviceState *host)
> +static void pci_host_bus_register(PCIHostState *phb)
> {
> - PCIHostState *host_bridge = PCI_HOST_BRIDGE(host);
> -
> - QLIST_INSERT_HEAD(&pci_host_bridges, host_bridge, next);
> + QLIST_INSERT_HEAD(&pci_host_bridges, phb, next);
> }
>
> PCIBus *pci_find_primary_bus(void)
> @@ -377,7 +375,7 @@ static void pci_bus_init(PCIBus *bus, DeviceState *parent,
> /* host bridge */
> QLIST_INIT(&bus->child);
>
> - pci_host_bus_register(parent);
> + pci_host_bus_register(PCI_HOST_BRIDGE(host));
> }
>
> bool pci_bus_is_express(PCIBus *bus)
>
Reviewed-by: Marcel Apfelbaum <marcel@redhat.com>
Thanks,
Marcel
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [Qemu-devel] [RFC 2/7] pci: Change pci_bus_init() 'parent' parameter to PCIHostState
2017-04-18 11:48 ` Eduardo Habkost
@ 2017-04-18 13:22 ` Marcel Apfelbaum
0 siblings, 0 replies; 29+ messages in thread
From: Marcel Apfelbaum @ 2017-04-18 13:22 UTC (permalink / raw)
To: Eduardo Habkost, David Gibson
Cc: qemu-devel, aik, David Gibson, Michael S. Tsirkin, Laszlo Ersek
On 04/18/2017 02:48 PM, Eduardo Habkost wrote:
> On Tue, Apr 18, 2017 at 01:51:02PM +1000, David Gibson wrote:
>> On Mon, Apr 17, 2017 at 06:59:11PM -0300, Eduardo Habkost wrote:
>>> pci_bus_init() already requires 'parent' to be a PCI_HOST_BRIDGE object,
>>> so change the parameter type to reflect that.
>>>
>>> Cc: "Michael S. Tsirkin" <mst@redhat.com>
>>> Cc: Marcel Apfelbaum <marcel@redhat.com>
>>> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
>>
>> Reviewed-by: David Gibson <david@gibson.dropbear.id.au>
>>
>> I had to do some looking to convince myself this wouldn't break P2P
>> bridges.
>>
>> I wonder if we should rename pci_bus_init() / pci_bus_new() to make it
>> clearer that they're about creating a whole new PCI domain, and not
>> for a new bus within an existing domain.
>
> Good point. The current naming is confusing. I only knew the
> change was safe because pci_bus_new() would crash if 'parent'
> wasn't a PCI_HOST_BRIDGE. But I took some time to find the other
> places where PCI buses could be created, and I'm not sure yet if
> I found all of them.
>
> Is there any other code that creates TYPE_PCI_BUS objects, except
> for the ones touched by this series and pci_bridge_initfn()?
>
pxb and pxb-pcie devices have a built-in PCI/PCIe bus,
but I saw them handled by another patch.
Reviewed-by: Marcel Apfelbaum <marcel@redhat.com>
Thanks,
Marcel
>>
>>> ---
>>> hw/pci/pci.c | 10 ++++++----
>>> 1 file changed, 6 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/hw/pci/pci.c b/hw/pci/pci.c
>>> index 25118fb91d..d9535c0bdc 100644
>>> --- a/hw/pci/pci.c
>>> +++ b/hw/pci/pci.c
>>> @@ -362,7 +362,7 @@ const char *pci_root_bus_path(PCIDevice *dev)
>>> return rootbus->qbus.name;
>>> }
>>>
>>> -static void pci_bus_init(PCIBus *bus, DeviceState *parent,
>>> +static void pci_bus_init(PCIBus *bus, PCIHostState *phb,
>>> MemoryRegion *address_space_mem,
>>> MemoryRegion *address_space_io,
>>> uint8_t devfn_min)
>>> @@ -375,7 +375,7 @@ static void pci_bus_init(PCIBus *bus, DeviceState *parent,
>>> /* host bridge */
>>> QLIST_INIT(&bus->child);
>>>
>>> - pci_host_bus_register(PCI_HOST_BRIDGE(host));
>>> + pci_host_bus_register(phb);
>>> }
>>>
>>> bool pci_bus_is_express(PCIBus *bus)
>>> @@ -394,8 +394,9 @@ void pci_bus_new_inplace(PCIBus *bus, size_t bus_size, DeviceState *parent,
>>> MemoryRegion *address_space_io,
>>> uint8_t devfn_min, const char *typename)
>>> {
>>> + PCIHostState *phb = PCI_HOST_BRIDGE(parent);
>>> qbus_create_inplace(bus, bus_size, typename, parent, name);
>>> - pci_bus_init(bus, parent, address_space_mem, address_space_io, devfn_min);
>>> + pci_bus_init(bus, phb, address_space_mem, address_space_io, devfn_min);
>>> }
>>>
>>> PCIBus *pci_bus_new(DeviceState *parent, const char *name,
>>> @@ -403,10 +404,11 @@ PCIBus *pci_bus_new(DeviceState *parent, const char *name,
>>> MemoryRegion *address_space_io,
>>> uint8_t devfn_min, const char *typename)
>>> {
>>> + PCIHostState *phb = PCI_HOST_BRIDGE(parent);
>>> PCIBus *bus;
>>>
>>> bus = PCI_BUS(qbus_create(typename, parent, name));
>>> - pci_bus_init(bus, parent, address_space_mem, address_space_io, devfn_min);
>>> + pci_bus_init(bus, phb, address_space_mem, address_space_io, devfn_min);
>>> return bus;
>>> }
>>>
>>
>> --
>> 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
>
>
>
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [Qemu-devel] [RFC 3/7] pci: Change pci_bus_new*() parameter to PCIHostState
2017-04-17 21:59 ` [Qemu-devel] [RFC 3/7] pci: Change pci_bus_new*() " Eduardo Habkost
2017-04-18 3:52 ` David Gibson
@ 2017-04-18 13:27 ` Marcel Apfelbaum
2017-04-18 13:30 ` Eduardo Habkost
2017-04-26 16:16 ` Michael S. Tsirkin
2 siblings, 1 reply; 29+ messages in thread
From: Marcel Apfelbaum @ 2017-04-18 13:27 UTC (permalink / raw)
To: Eduardo Habkost, qemu-devel
Cc: aik, David Gibson, Michael S. Tsirkin, Laszlo Ersek,
Hervé Poussineau, Peter Maydell, qemu-ppc, qemu-arm
On 04/18/2017 12:59 AM, Eduardo Habkost wrote:
> The pci_bus_new*() functions already require the 'parent' argument to be
> a PCI_HOST_BRIDGE object. Change the parameter type to reflect that.
>
> Cc: "Michael S. Tsirkin" <mst@redhat.com>
> Cc: Marcel Apfelbaum <marcel@redhat.com>
> Cc: "Hervé Poussineau" <hpoussin@reactos.org>
> Cc: Peter Maydell <peter.maydell@linaro.org>
> Cc: qemu-ppc@nongnu.org
> Cc: qemu-arm@nongnu.org
> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
> ---
> include/hw/pci/pci.h | 5 +++--
> hw/pci-bridge/pci_expander_bridge.c | 15 ++++++++-------
> hw/pci-host/piix.c | 2 +-
> hw/pci-host/prep.c | 2 +-
> hw/pci-host/q35.c | 2 +-
> hw/pci-host/versatile.c | 2 +-
> hw/pci/pci.c | 13 ++++++-------
> 7 files changed, 21 insertions(+), 20 deletions(-)
>
> diff --git a/include/hw/pci/pci.h b/include/hw/pci/pci.h
> index a37a2d5cb6..2242aa25eb 100644
> --- a/include/hw/pci/pci.h
> +++ b/include/hw/pci/pci.h
> @@ -393,12 +393,13 @@ 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,
> +void pci_bus_new_inplace(PCIBus *bus, size_t bus_size,
> + PCIHostState *phb,
> 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,
> +PCIBus *pci_bus_new(PCIHostState *phb, const char *name,
> MemoryRegion *address_space_mem,
> MemoryRegion *address_space_io,
> uint8_t devfn_min, const char *typename);
> diff --git a/hw/pci-bridge/pci_expander_bridge.c b/hw/pci-bridge/pci_expander_bridge.c
> index 6ac187fa32..39d29d2230 100644
> --- a/hw/pci-bridge/pci_expander_bridge.c
> +++ b/hw/pci-bridge/pci_expander_bridge.c
> @@ -213,7 +213,8 @@ static gint pxb_compare(gconstpointer a, gconstpointer b)
> static void pxb_dev_realize_common(PCIDevice *dev, bool pcie, Error **errp)
> {
> PXBDev *pxb = convert_to_pxb(dev);
> - DeviceState *ds, *bds = NULL;
> + DeviceState *bds = NULL;
> + PCIHostState *phb;
> PCIBus *bus;
> const char *dev_name = NULL;
> Error *local_err = NULL;
> @@ -228,11 +229,11 @@ static void pxb_dev_realize_common(PCIDevice *dev, bool pcie, Error **errp)
> dev_name = dev->qdev.id;
> }
>
> - ds = qdev_create(NULL, TYPE_PXB_HOST);
> + phb = PCI_HOST_BRIDGE(qdev_create(NULL, TYPE_PXB_HOST));
> if (pcie) {
> - bus = pci_bus_new(ds, dev_name, NULL, NULL, 0, TYPE_PXB_PCIE_BUS);
> + bus = pci_bus_new(phb, 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_bus_new(phb, "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);
> @@ -244,7 +245,7 @@ static void pxb_dev_realize_common(PCIDevice *dev, bool pcie, Error **errp)
> bus->address_space_io = dev->bus->address_space_io;
> bus->map_irq = pxb_map_irq_fn;
>
> - PCI_HOST_BRIDGE(ds)->bus = bus;
> + phb->bus = bus;
>
> pxb_register_bus(dev, bus, &local_err);
> if (local_err) {
> @@ -252,7 +253,7 @@ static void pxb_dev_realize_common(PCIDevice *dev, bool pcie, Error **errp)
> goto err_register_bus;
> }
>
> - qdev_init_nofail(ds);
> + qdev_init_nofail(DEVICE(phb));
> if (bds) {
> qdev_init_nofail(bds);
> }
> @@ -267,7 +268,7 @@ static void pxb_dev_realize_common(PCIDevice *dev, bool pcie, Error **errp)
> err_register_bus:
> object_unref(OBJECT(bds));
> object_unparent(OBJECT(bus));
> - object_unref(OBJECT(ds));
> + object_unref(OBJECT(phb));
> }
>
> static void pxb_dev_realize(PCIDevice *dev, Error **errp)
> diff --git a/hw/pci-host/piix.c b/hw/pci-host/piix.c
> index f9218aa952..91fec05b38 100644
> --- a/hw/pci-host/piix.c
> +++ b/hw/pci-host/piix.c
> @@ -340,7 +340,7 @@ 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,
> + b = pci_bus_new(s, 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);
> diff --git a/hw/pci-host/prep.c b/hw/pci-host/prep.c
> index 260a119a9e..2e2cd267f4 100644
> --- a/hw/pci-host/prep.c
> +++ b/hw/pci-host/prep.c
> @@ -269,7 +269,7 @@ 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,
> + pci_bus_new_inplace(&s->pci_bus, sizeof(s->pci_bus), h, NULL,
> &s->pci_memory, &s->pci_io, 0, TYPE_PCI_BUS);
>
> /* Bus master address space */
> diff --git a/hw/pci-host/q35.c b/hw/pci-host/q35.c
> index 344f77b10c..860b47a1ba 100644
> --- a/hw/pci-host/q35.c
> +++ b/hw/pci-host/q35.c
> @@ -49,7 +49,7 @@ 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",
> + pci->bus = pci_bus_new(pci, "pcie.0",
> s->mch.pci_address_space, s->mch.address_space_io,
> 0, TYPE_PCIE_BUS);
> PC_MACHINE(qdev_get_machine())->bus = pci->bus;
> diff --git a/hw/pci-host/versatile.c b/hw/pci-host/versatile.c
> index 467cbb9cb8..24ef87610b 100644
> --- a/hw/pci-host/versatile.c
> +++ b/hw/pci-host/versatile.c
> @@ -386,7 +386,7 @@ static void pci_vpb_init(Object *obj)
> 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), DEVICE(obj), "pci",
> + pci_bus_new_inplace(&s->pci_bus, sizeof(s->pci_bus), h, "pci",
> &s->pci_mem_space, &s->pci_io_space,
> PCI_DEVFN(11, 0), TYPE_PCI_BUS);
> h->bus = &s->pci_bus;
> diff --git a/hw/pci/pci.c b/hw/pci/pci.c
> index d9535c0bdc..f4488b46fc 100644
> --- a/hw/pci/pci.c
> +++ b/hw/pci/pci.c
> @@ -388,26 +388,25 @@ 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,
> +void pci_bus_new_inplace(PCIBus *bus, size_t bus_size,
> + PCIHostState *phb,
> const char *name,
> MemoryRegion *address_space_mem,
> MemoryRegion *address_space_io,
> uint8_t devfn_min, const char *typename)
> {
> - PCIHostState *phb = PCI_HOST_BRIDGE(parent);
> - qbus_create_inplace(bus, bus_size, typename, parent, name);
> + qbus_create_inplace(bus, bus_size, typename, DEVICE(phb), name);
> pci_bus_init(bus, phb, address_space_mem, address_space_io, devfn_min);
> }
>
> -PCIBus *pci_bus_new(DeviceState *parent, const char *name,
> +PCIBus *pci_bus_new(PCIHostState *phb, const char *name,
> MemoryRegion *address_space_mem,
> MemoryRegion *address_space_io,
> uint8_t devfn_min, const char *typename)
> {
> - PCIHostState *phb = PCI_HOST_BRIDGE(parent);
> PCIBus *bus;
>
> - bus = PCI_BUS(qbus_create(typename, parent, name));
> + bus = PCI_BUS(qbus_create(typename, DEVICE(phb), name));
> pci_bus_init(bus, phb, address_space_mem, address_space_io, devfn_min);
> return bus;
> }
> @@ -431,7 +430,7 @@ PCIBus *pci_register_bus(DeviceState *parent, const char *name,
> {
> PCIBus *bus;
>
> - bus = pci_bus_new(parent, name, address_space_mem,
> + bus = pci_bus_new(PCI_HOST_BRIDGE(parent), name, address_space_mem,
If you request the parent to be PCI_HOST_BRIDGE, why don't you change also the signature of
pci_register_bus ?
Thanks,
Marcel
> address_space_io, devfn_min, typename);
> pci_bus_irqs(bus, set_irq, map_irq, irq_opaque, nirq);
> return bus;
>
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [Qemu-devel] [RFC 3/7] pci: Change pci_bus_new*() parameter to PCIHostState
2017-04-18 13:27 ` Marcel Apfelbaum
@ 2017-04-18 13:30 ` Eduardo Habkost
2017-04-18 13:36 ` Marcel Apfelbaum
0 siblings, 1 reply; 29+ messages in thread
From: Eduardo Habkost @ 2017-04-18 13:30 UTC (permalink / raw)
To: Marcel Apfelbaum
Cc: qemu-devel, aik, David Gibson, Michael S. Tsirkin, Laszlo Ersek,
Hervé Poussineau, Peter Maydell, qemu-ppc, qemu-arm
On Tue, Apr 18, 2017 at 04:27:23PM +0300, Marcel Apfelbaum wrote:
[...]
> > @@ -431,7 +430,7 @@ PCIBus *pci_register_bus(DeviceState *parent, const char *name,
> > {
> > PCIBus *bus;
> >
> > - bus = pci_bus_new(parent, name, address_space_mem,
> > + bus = pci_bus_new(PCI_HOST_BRIDGE(parent), name, address_space_mem,
>
> If you request the parent to be PCI_HOST_BRIDGE, why don't you change also the signature of
> pci_register_bus ?
I do it on patch 4/7.
--
Eduardo
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [Qemu-devel] [RFC 3/7] pci: Change pci_bus_new*() parameter to PCIHostState
2017-04-18 13:30 ` Eduardo Habkost
@ 2017-04-18 13:36 ` Marcel Apfelbaum
0 siblings, 0 replies; 29+ messages in thread
From: Marcel Apfelbaum @ 2017-04-18 13:36 UTC (permalink / raw)
To: Eduardo Habkost
Cc: qemu-devel, aik, David Gibson, Michael S. Tsirkin, Laszlo Ersek,
Hervé Poussineau, Peter Maydell, qemu-ppc, qemu-arm
On 04/18/2017 04:30 PM, Eduardo Habkost wrote:
> On Tue, Apr 18, 2017 at 04:27:23PM +0300, Marcel Apfelbaum wrote:
> [...]
>>> @@ -431,7 +430,7 @@ PCIBus *pci_register_bus(DeviceState *parent, const char *name,
>>> {
>>> PCIBus *bus;
>>>
>>> - bus = pci_bus_new(parent, name, address_space_mem,
>>> + bus = pci_bus_new(PCI_HOST_BRIDGE(parent), name, address_space_mem,
>>
>> If you request the parent to be PCI_HOST_BRIDGE, why don't you change also the signature of
>> pci_register_bus ?
>
> I do it on patch 4/7.
>
Yes, I saw it :). at least it means I am following the patches...
Thanks,
Marcel
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [Qemu-devel] [RFC 4/7] pci: Change pci_register_bus() 'parent' parameter to PCIHostState
2017-04-17 21:59 ` [Qemu-devel] [RFC 4/7] pci: Change pci_register_bus() 'parent' " Eduardo Habkost
2017-04-18 3:53 ` David Gibson
2017-04-18 10:41 ` Cornelia Huck
@ 2017-04-18 13:37 ` Marcel Apfelbaum
2017-04-26 16:17 ` Michael S. Tsirkin
3 siblings, 0 replies; 29+ messages in thread
From: Marcel Apfelbaum @ 2017-04-18 13:37 UTC (permalink / raw)
To: Eduardo Habkost, qemu-devel
Cc: aik, David Gibson, Michael S. Tsirkin, Laszlo Ersek,
Richard Henderson, Aurelien Jarno, Yongbok Kim, Alexander Graf,
Scott Wood, Paul Burton, David Gibson, Cornelia Huck,
Christian Borntraeger, qemu-ppc
On 04/18/2017 12:59 AM, Eduardo Habkost wrote:
> pci_register_bus() already requires the 'parent' argument to be a
> PCI_HOST_BRIDGE object. Change the parameter type to reflect that.
>
> Cc: Richard Henderson <rth@twiddle.net>
> Cc: Aurelien Jarno <aurelien@aurel32.net>
> Cc: Yongbok Kim <yongbok.kim@imgtec.com>
> Cc: Alexander Graf <agraf@suse.de>
> Cc: Scott Wood <scottwood@freescale.com>
> Cc: Paul Burton <paul.burton@imgtec.com>
> Cc: "Michael S. Tsirkin" <mst@redhat.com>
> Cc: Marcel Apfelbaum <marcel@redhat.com>
> Cc: David Gibson <david@gibson.dropbear.id.au>
> Cc: Cornelia Huck <cornelia.huck@de.ibm.com>
> Cc: Christian Borntraeger <borntraeger@de.ibm.com>
> Cc: qemu-ppc@nongnu.org
> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
> ---
> include/hw/pci/pci.h | 2 +-
> hw/alpha/typhoon.c | 2 +-
> hw/mips/gt64xxx_pci.c | 2 +-
> hw/pci-host/apb.c | 2 +-
> hw/pci-host/bonito.c | 2 +-
> hw/pci-host/gpex.c | 2 +-
> hw/pci-host/grackle.c | 2 +-
> hw/pci-host/ppce500.c | 2 +-
> hw/pci-host/uninorth.c | 4 ++--
> hw/pci-host/xilinx-pcie.c | 2 +-
> hw/pci/pci.c | 4 ++--
> hw/ppc/ppc4xx_pci.c | 2 +-
> hw/ppc/spapr_pci.c | 2 +-
> hw/s390x/s390-pci-bus.c | 2 +-
> hw/sh4/sh_pci.c | 2 +-
> 15 files changed, 17 insertions(+), 17 deletions(-)
>
> diff --git a/include/hw/pci/pci.h b/include/hw/pci/pci.h
> index 2242aa25eb..56387ccb0c 100644
> --- a/include/hw/pci/pci.h
> +++ b/include/hw/pci/pci.h
> @@ -408,7 +408,7 @@ void pci_bus_irqs(PCIBus *bus, pci_set_irq_fn set_irq, pci_map_irq_fn map_irq,
> 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,
> +PCIBus *pci_register_bus(PCIHostState *phb, const char *name,
> pci_set_irq_fn set_irq, pci_map_irq_fn map_irq,
> void *irq_opaque,
> MemoryRegion *address_space_mem,
> diff --git a/hw/alpha/typhoon.c b/hw/alpha/typhoon.c
> index f50f5cf186..ac0633a55e 100644
> --- a/hw/alpha/typhoon.c
> +++ b/hw/alpha/typhoon.c
> @@ -883,7 +883,7 @@ 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",
> + b = pci_register_bus(phb, "pci",
> typhoon_set_irq, sys_map_irq, s,
> &s->pchip.reg_mem, &s->pchip.reg_io,
> 0, 64, TYPE_PCI_BUS);
> diff --git a/hw/mips/gt64xxx_pci.c b/hw/mips/gt64xxx_pci.c
> index 4811843ab6..bd131bcdc6 100644
> --- a/hw/mips/gt64xxx_pci.c
> +++ b/hw/mips/gt64xxx_pci.c
> @@ -1171,7 +1171,7 @@ 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",
> + phb->bus = pci_register_bus(phb, "pci",
> gt64120_pci_set_irq, gt64120_pci_map_irq,
> pic,
> &d->pci0_mem,
> diff --git a/hw/pci-host/apb.c b/hw/pci-host/apb.c
> index 653e711121..1156a54224 100644
> --- a/hw/pci-host/apb.c
> +++ b/hw/pci-host/apb.c
> @@ -671,7 +671,7 @@ 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",
> + phb->bus = pci_register_bus(phb, "pci",
> pci_apb_set_irq, pci_pbm_map_irq, d,
> &d->pci_mmio,
> get_system_io(),
> diff --git a/hw/pci-host/bonito.c b/hw/pci-host/bonito.c
> index 1999ece590..27842edc04 100644
> --- a/hw/pci-host/bonito.c
> +++ b/hw/pci-host/bonito.c
> @@ -714,7 +714,7 @@ static int bonito_pcihost_initfn(SysBusDevice *dev)
> {
> PCIHostState *phb = PCI_HOST_BRIDGE(dev);
>
> - phb->bus = pci_register_bus(DEVICE(dev), "pci",
> + phb->bus = pci_register_bus(phb, "pci",
> pci_bonito_set_irq, pci_bonito_map_irq, dev,
> get_system_memory(), get_system_io(),
> 0x28, 32, TYPE_PCI_BUS);
> diff --git a/hw/pci-host/gpex.c b/hw/pci-host/gpex.c
> index 66055ee5cc..042d127271 100644
> --- a/hw/pci-host/gpex.c
> +++ b/hw/pci-host/gpex.c
> @@ -62,7 +62,7 @@ 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->bus = pci_register_bus(pci, "pcie.0", gpex_set_irq,
> pci_swizzle_map_irq_fn, s, &s->io_mmio,
> &s->io_ioport, 0, 4, TYPE_PCIE_BUS);
>
> diff --git a/hw/pci-host/grackle.c b/hw/pci-host/grackle.c
> index 2c8acdaaca..a56c063be9 100644
> --- a/hw/pci-host/grackle.c
> +++ b/hw/pci-host/grackle.c
> @@ -82,7 +82,7 @@ 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,
> + phb->bus = pci_register_bus(phb, NULL,
> pci_grackle_set_irq,
> pci_grackle_map_irq,
> pic,
> diff --git a/hw/pci-host/ppce500.c b/hw/pci-host/ppce500.c
> index e502bc0505..4a1e99f426 100644
> --- a/hw/pci-host/ppce500.c
> +++ b/hw/pci-host/ppce500.c
> @@ -465,7 +465,7 @@ 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,
> + b = pci_register_bus(h, 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;
> diff --git a/hw/pci-host/uninorth.c b/hw/pci-host/uninorth.c
> index df342ac3cb..b1b89183fc 100644
> --- a/hw/pci-host/uninorth.c
> +++ b/hw/pci-host/uninorth.c
> @@ -233,7 +233,7 @@ 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,
> + h->bus = pci_register_bus(h, NULL,
> pci_unin_set_irq, pci_unin_map_irq,
> pic,
> &d->pci_mmio,
> @@ -299,7 +299,7 @@ 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,
> + h->bus = pci_register_bus(h, NULL,
> pci_unin_set_irq, pci_unin_map_irq,
> pic,
> &d->pci_mmio,
> diff --git a/hw/pci-host/xilinx-pcie.c b/hw/pci-host/xilinx-pcie.c
> index 8b71e2d950..c951684148 100644
> --- a/hw/pci-host/xilinx-pcie.c
> +++ b/hw/pci-host/xilinx-pcie.c
> @@ -129,7 +129,7 @@ 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->bus = pci_register_bus(pci, s->name, xilinx_pcie_set_irq,
> pci_swizzle_map_irq_fn, s, &s->mmio,
> &s->io, 0, 4, TYPE_PCIE_BUS);
>
> diff --git a/hw/pci/pci.c b/hw/pci/pci.c
> index f4488b46fc..f60d0497ef 100644
> --- a/hw/pci/pci.c
> +++ b/hw/pci/pci.c
> @@ -421,7 +421,7 @@ 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,
> +PCIBus *pci_register_bus(PCIHostState *phb, const char *name,
> pci_set_irq_fn set_irq, pci_map_irq_fn map_irq,
> void *irq_opaque,
> MemoryRegion *address_space_mem,
> @@ -430,7 +430,7 @@ PCIBus *pci_register_bus(DeviceState *parent, const char *name,
> {
> PCIBus *bus;
>
> - bus = pci_bus_new(PCI_HOST_BRIDGE(parent), name, address_space_mem,
> + bus = pci_bus_new(phb, 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 dc19682970..f755c6faae 100644
> --- a/hw/ppc/ppc4xx_pci.c
> +++ b/hw/ppc/ppc4xx_pci.c
> @@ -314,7 +314,7 @@ 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,
> + b = pci_register_bus(h, 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;
> diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c
> index 98c52e411f..0f293f9e75 100644
> --- a/hw/ppc/spapr_pci.c
> +++ b/hw/ppc/spapr_pci.c
> @@ -1697,7 +1697,7 @@ 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,
> + bus = pci_register_bus(phb, NULL,
> pci_spapr_set_irq, pci_spapr_map_irq, sphb,
> &sphb->memspace, &sphb->iospace,
> PCI_DEVFN(0, 0), PCI_NUM_PINS, TYPE_PCI_BUS);
> diff --git a/hw/s390x/s390-pci-bus.c b/hw/s390x/s390-pci-bus.c
> index 69b0291e8a..99aae965bb 100644
> --- a/hw/s390x/s390-pci-bus.c
> +++ b/hw/s390x/s390-pci-bus.c
> @@ -560,7 +560,7 @@ static int s390_pcihost_init(SysBusDevice *dev)
>
> DPRINTF("host_init\n");
>
> - b = pci_register_bus(DEVICE(dev), NULL,
> + b = pci_register_bus(phb, NULL,
> s390_pci_set_irq, s390_pci_map_irq, NULL,
> get_system_memory(), get_system_io(), 0, 64,
> TYPE_PCI_BUS);
> diff --git a/hw/sh4/sh_pci.c b/hw/sh4/sh_pci.c
> index 1747628f3d..bca849c10f 100644
> --- a/hw/sh4/sh_pci.c
> +++ b/hw/sh4/sh_pci.c
> @@ -131,7 +131,7 @@ 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",
> + phb->bus = pci_register_bus(phb, "pci",
> sh_pci_set_irq, sh_pci_map_irq,
> s->irq,
> get_system_memory(),
>
Reviewed-by: Marcel Apfelbaum <marcel@redhat.com>
Thanks,
Marcel
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [Qemu-devel] [RFC 5/7] pci: Set phb->bus inside pci_register_bus()
2017-04-17 21:59 ` [Qemu-devel] [RFC 5/7] pci: Set phb->bus inside pci_register_bus() Eduardo Habkost
2017-04-18 3:55 ` David Gibson
2017-04-18 10:42 ` Cornelia Huck
@ 2017-04-18 13:43 ` Marcel Apfelbaum
2017-04-18 13:53 ` Eduardo Habkost
2 siblings, 1 reply; 29+ messages in thread
From: Marcel Apfelbaum @ 2017-04-18 13:43 UTC (permalink / raw)
To: Eduardo Habkost, qemu-devel
Cc: aik, David Gibson, Michael S. Tsirkin, Laszlo Ersek,
Richard Henderson, Aurelien Jarno, Yongbok Kim, Alexander Graf,
Scott Wood, Paul Burton, David Gibson, Cornelia Huck,
Christian Borntraeger, qemu-ppc
On 04/18/2017 12:59 AM, Eduardo Habkost wrote:
> Every single caller of of pci_register_bus() saves the return value in
> phb->bus. Do that inside pci_register_bus() to avoid code duplication
> and make it harder to break.
>
I personally find that more difficult to follow, maybe the function
name is misleading.
Maybe we can find a better function name or explain the side effect
in a comment?
Thanks,
Marcel
> Most (but not all) conversions done using the following Coccinelle script:
>
> @@
> identifier b;
> expression phb;
> @@
> -b = pci_register_bus(phb, ARGS);
> +phb->bus = pci_register_bus(phb, ARGS);
> ...
> -phb->bus = b;
>
> @@
> expression phb;
> expression list ARGS;
> @@
> -phb->bus = pci_register_bus(phb, ARGS);
> +pci_register_bus(phb, ARGS);
>
> Cc: Richard Henderson <rth@twiddle.net>
> Cc: Aurelien Jarno <aurelien@aurel32.net>
> Cc: Yongbok Kim <yongbok.kim@imgtec.com>
> Cc: Alexander Graf <agraf@suse.de>
> Cc: Scott Wood <scottwood@freescale.com>
> Cc: Paul Burton <paul.burton@imgtec.com>
> Cc: "Michael S. Tsirkin" <mst@redhat.com>
> Cc: Marcel Apfelbaum <marcel@redhat.com>
> Cc: David Gibson <david@gibson.dropbear.id.au>
> Cc: Cornelia Huck <cornelia.huck@de.ibm.com>
> Cc: Christian Borntraeger <borntraeger@de.ibm.com>
> Cc: qemu-ppc@nongnu.org
> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
> ---
> include/hw/pci/pci.h | 12 ++++++------
> hw/alpha/typhoon.c | 10 +++++-----
> hw/mips/gt64xxx_pci.c | 9 +++------
> hw/pci-host/apb.c | 7 ++-----
> hw/pci-host/bonito.c | 7 +++----
> hw/pci-host/gpex.c | 5 ++---
> hw/pci-host/grackle.c | 9 ++-------
> hw/pci-host/ppce500.c | 8 ++++----
> hw/pci-host/uninorth.c | 18 ++++++------------
> hw/pci-host/xilinx-pcie.c | 6 +++---
> hw/pci/pci.c | 14 +++++++-------
> hw/ppc/ppc4xx_pci.c | 8 ++++----
> hw/ppc/spapr_pci.c | 10 +++++-----
> hw/s390x/s390-pci-bus.c | 10 +++++-----
> hw/sh4/sh_pci.c | 9 +++------
> 15 files changed, 60 insertions(+), 82 deletions(-)
>
> diff --git a/include/hw/pci/pci.h b/include/hw/pci/pci.h
> index 56387ccb0c..3b1e2c408a 100644
> --- a/include/hw/pci/pci.h
> +++ b/include/hw/pci/pci.h
> @@ -408,12 +408,12 @@ void pci_bus_irqs(PCIBus *bus, pci_set_irq_fn set_irq, pci_map_irq_fn map_irq,
> 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(PCIHostState *phb, 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_register_bus(PCIHostState *phb, 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);
> diff --git a/hw/alpha/typhoon.c b/hw/alpha/typhoon.c
> index ac0633a55e..5926686d79 100644
> --- a/hw/alpha/typhoon.c
> +++ b/hw/alpha/typhoon.c
> @@ -883,11 +883,11 @@ 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(phb, "pci",
> - typhoon_set_irq, sys_map_irq, s,
> - &s->pchip.reg_mem, &s->pchip.reg_io,
> - 0, 64, TYPE_PCI_BUS);
> - phb->bus = b;
> + pci_register_bus(phb, "pci",
> + typhoon_set_irq, sys_map_irq, s,
> + &s->pchip.reg_mem, &s->pchip.reg_io,
> + 0, 64, TYPE_PCI_BUS);
> + b = phb->bus;
> qdev_init_nofail(dev);
>
> /* Host memory as seen from the PCI side, via the IOMMU. */
> diff --git a/hw/mips/gt64xxx_pci.c b/hw/mips/gt64xxx_pci.c
> index bd131bcdc6..69963453f0 100644
> --- a/hw/mips/gt64xxx_pci.c
> +++ b/hw/mips/gt64xxx_pci.c
> @@ -1171,12 +1171,9 @@ 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(phb, "pci",
> - gt64120_pci_set_irq, gt64120_pci_map_irq,
> - pic,
> - &d->pci0_mem,
> - get_system_io(),
> - PCI_DEVFN(18, 0), 4, TYPE_PCI_BUS);
> + pci_register_bus(phb, "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-host/apb.c b/hw/pci-host/apb.c
> index 1156a54224..ea86260b04 100644
> --- a/hw/pci-host/apb.c
> +++ b/hw/pci-host/apb.c
> @@ -671,11 +671,8 @@ 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(phb, "pci",
> - pci_apb_set_irq, pci_pbm_map_irq, d,
> - &d->pci_mmio,
> - get_system_io(),
> - 0, 32, TYPE_PCI_BUS);
> + pci_register_bus(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 27842edc04..bbe595605d 100644
> --- a/hw/pci-host/bonito.c
> +++ b/hw/pci-host/bonito.c
> @@ -714,10 +714,9 @@ static int bonito_pcihost_initfn(SysBusDevice *dev)
> {
> PCIHostState *phb = PCI_HOST_BRIDGE(dev);
>
> - phb->bus = pci_register_bus(phb, "pci",
> - pci_bonito_set_irq, pci_bonito_map_irq, dev,
> - get_system_memory(), get_system_io(),
> - 0x28, 32, TYPE_PCI_BUS);
> + pci_register_bus(phb, "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 042d127271..66e2f3d29c 100644
> --- a/hw/pci-host/gpex.c
> +++ b/hw/pci-host/gpex.c
> @@ -62,9 +62,8 @@ static void gpex_host_realize(DeviceState *dev, Error **errp)
> sysbus_init_irq(sbd, &s->irq[i]);
> }
>
> - pci->bus = pci_register_bus(pci, "pcie.0", gpex_set_irq,
> - pci_swizzle_map_irq_fn, s, &s->io_mmio,
> - &s->io_ioport, 0, 4, TYPE_PCIE_BUS);
> + pci_register_bus(pci, "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));
> qdev_init_nofail(DEVICE(&s->gpex_root));
> diff --git a/hw/pci-host/grackle.c b/hw/pci-host/grackle.c
> index a56c063be9..691af0a29e 100644
> --- a/hw/pci-host/grackle.c
> +++ b/hw/pci-host/grackle.c
> @@ -82,13 +82,8 @@ 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(phb, NULL,
> - pci_grackle_set_irq,
> - pci_grackle_map_irq,
> - pic,
> - &d->pci_mmio,
> - address_space_io,
> - 0, 4, TYPE_PCI_BUS);
> + pci_register_bus(phb, 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/ppce500.c b/hw/pci-host/ppce500.c
> index 4a1e99f426..43a06961d0 100644
> --- a/hw/pci-host/ppce500.c
> +++ b/hw/pci-host/ppce500.c
> @@ -465,10 +465,10 @@ 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(h, 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;
> + pci_register_bus(h, 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 = h->bus;
>
> /* Set up PCI view of memory */
> memory_region_init(&s->bm, OBJECT(s), "bm-e500", UINT64_MAX);
> diff --git a/hw/pci-host/uninorth.c b/hw/pci-host/uninorth.c
> index b1b89183fc..4d709e5468 100644
> --- a/hw/pci-host/uninorth.c
> +++ b/hw/pci-host/uninorth.c
> @@ -233,12 +233,9 @@ PCIBus *pci_pmac_init(qemu_irq *pic,
> memory_region_add_subregion(address_space_mem, 0x80000000ULL,
> &d->pci_hole);
>
> - h->bus = pci_register_bus(h, NULL,
> - pci_unin_set_irq, pci_unin_map_irq,
> - pic,
> - &d->pci_mmio,
> - address_space_io,
> - PCI_DEVFN(11, 0), 4, TYPE_PCI_BUS);
> + pci_register_bus(h, 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 +296,9 @@ PCIBus *pci_pmac_u3_init(qemu_irq *pic,
> memory_region_add_subregion(address_space_mem, 0x80000000ULL,
> &d->pci_hole);
>
> - h->bus = pci_register_bus(h, NULL,
> - pci_unin_set_irq, pci_unin_map_irq,
> - pic,
> - &d->pci_mmio,
> - address_space_io,
> - PCI_DEVFN(11, 0), 4, TYPE_PCI_BUS);
> + pci_register_bus(h, 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/xilinx-pcie.c b/hw/pci-host/xilinx-pcie.c
> index c951684148..6d53677f57 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(pci, s->name, xilinx_pcie_set_irq,
> - pci_swizzle_map_irq_fn, s, &s->mmio,
> - &s->io, 0, 4, TYPE_PCIE_BUS);
> + pci_register_bus(pci, 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 f60d0497ef..d3adf806e5 100644
> --- a/hw/pci/pci.c
> +++ b/hw/pci/pci.c
> @@ -421,19 +421,19 @@ 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(PCIHostState *phb, 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_register_bus(PCIHostState *phb, 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(phb, name, address_space_mem,
> address_space_io, devfn_min, typename);
> pci_bus_irqs(bus, set_irq, map_irq, irq_opaque, nirq);
> - return bus;
> + phb->bus = bus;
> }
>
> int pci_bus_num(PCIBus *s)
> diff --git a/hw/ppc/ppc4xx_pci.c b/hw/ppc/ppc4xx_pci.c
> index f755c6faae..ab158b3ef1 100644
> --- a/hw/ppc/ppc4xx_pci.c
> +++ b/hw/ppc/ppc4xx_pci.c
> @@ -314,10 +314,10 @@ static int ppc4xx_pcihost_initfn(SysBusDevice *dev)
> sysbus_init_irq(dev, &s->irq[i]);
> }
>
> - b = pci_register_bus(h, 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_register_bus(h, NULL, ppc4xx_pci_set_irq,
> + ppc4xx_pci_map_irq, s->irq, get_system_memory(),
> + get_system_io(), 0, 4, TYPE_PCI_BUS);
> + b = h->bus;
>
> pci_create_simple(b, 0, "ppc4xx-host-bridge");
>
> diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c
> index 0f293f9e75..5749f14d9f 100644
> --- a/hw/ppc/spapr_pci.c
> +++ b/hw/ppc/spapr_pci.c
> @@ -1697,11 +1697,11 @@ 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(phb, 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;
> + pci_register_bus(phb, 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 = phb->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 99aae965bb..466067a380 100644
> --- a/hw/s390x/s390-pci-bus.c
> +++ b/hw/s390x/s390-pci-bus.c
> @@ -560,15 +560,15 @@ static int s390_pcihost_init(SysBusDevice *dev)
>
> DPRINTF("host_init\n");
>
> - b = pci_register_bus(phb, NULL,
> - s390_pci_set_irq, s390_pci_map_irq, NULL,
> - get_system_memory(), get_system_io(), 0, 64,
> - TYPE_PCI_BUS);
> + pci_register_bus(phb, NULL,
> + s390_pci_set_irq, s390_pci_map_irq, NULL,
> + get_system_memory(), get_system_io(), 0, 64,
> + TYPE_PCI_BUS);
> + b = phb->bus;
> pci_setup_iommu(b, s390_pci_dma_iommu, s);
>
> bus = BUS(b);
> qbus_set_hotplug_handler(bus, DEVICE(dev), NULL);
> - phb->bus = b;
>
> s->bus = S390_PCI_BUS(qbus_create(TYPE_S390_PCI_BUS, DEVICE(s), NULL));
> qbus_set_hotplug_handler(BUS(s->bus), DEVICE(s), NULL);
> diff --git a/hw/sh4/sh_pci.c b/hw/sh4/sh_pci.c
> index bca849c10f..bb43bad77d 100644
> --- a/hw/sh4/sh_pci.c
> +++ b/hw/sh4/sh_pci.c
> @@ -131,12 +131,9 @@ 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(phb, "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);
> + pci_register_bus(phb, "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",
>
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [Qemu-devel] [RFC 5/7] pci: Set phb->bus inside pci_register_bus()
2017-04-18 13:43 ` Marcel Apfelbaum
@ 2017-04-18 13:53 ` Eduardo Habkost
0 siblings, 0 replies; 29+ messages in thread
From: Eduardo Habkost @ 2017-04-18 13:53 UTC (permalink / raw)
To: Marcel Apfelbaum
Cc: qemu-devel, aik, David Gibson, Michael S. Tsirkin, Laszlo Ersek,
Richard Henderson, Aurelien Jarno, Yongbok Kim, Alexander Graf,
Scott Wood, Paul Burton, David Gibson, Cornelia Huck,
Christian Borntraeger, qemu-ppc
On Tue, Apr 18, 2017 at 04:43:48PM +0300, Marcel Apfelbaum wrote:
> On 04/18/2017 12:59 AM, Eduardo Habkost wrote:
> > Every single caller of of pci_register_bus() saves the return value in
> > phb->bus. Do that inside pci_register_bus() to avoid code duplication
> > and make it harder to break.
> >
>
> I personally find that more difficult to follow, maybe the function
> name is misleading.
> Maybe we can find a better function name or explain the side effect
> in a comment?
I agree we need better function names (for both
pci_register_bus() and pci_bus_new*()). But I am not experienced
enough with the PCI code to find a good name. Any suggestions?
--
Eduardo
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [Qemu-devel] [RFC 3/7] pci: Change pci_bus_new*() parameter to PCIHostState
2017-04-17 21:59 ` [Qemu-devel] [RFC 3/7] pci: Change pci_bus_new*() " Eduardo Habkost
2017-04-18 3:52 ` David Gibson
2017-04-18 13:27 ` Marcel Apfelbaum
@ 2017-04-26 16:16 ` Michael S. Tsirkin
2017-04-26 17:49 ` Eduardo Habkost
2 siblings, 1 reply; 29+ messages in thread
From: Michael S. Tsirkin @ 2017-04-26 16:16 UTC (permalink / raw)
To: Eduardo Habkost
Cc: qemu-devel, aik, David Gibson, Laszlo Ersek, Marcel Apfelbaum,
Hervé Poussineau, Peter Maydell, qemu-ppc, qemu-arm
On Mon, Apr 17, 2017 at 06:59:12PM -0300, Eduardo Habkost wrote:
> The pci_bus_new*() functions already require the 'parent' argument to be
> a PCI_HOST_BRIDGE object. Change the parameter type to reflect that.
>
> Cc: "Michael S. Tsirkin" <mst@redhat.com>
> Cc: Marcel Apfelbaum <marcel@redhat.com>
> Cc: "Hervé Poussineau" <hpoussin@reactos.org>
> Cc: Peter Maydell <peter.maydell@linaro.org>
> Cc: qemu-ppc@nongnu.org
> Cc: qemu-arm@nongnu.org
> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
This function doesn't create a generic pci bus, it's only good
for creating root buses.
As we are changing all callers anyway, let's rename this to
pci_host_bus_new or pci_root_bus_new?
> ---
> include/hw/pci/pci.h | 5 +++--
> hw/pci-bridge/pci_expander_bridge.c | 15 ++++++++-------
> hw/pci-host/piix.c | 2 +-
> hw/pci-host/prep.c | 2 +-
> hw/pci-host/q35.c | 2 +-
> hw/pci-host/versatile.c | 2 +-
> hw/pci/pci.c | 13 ++++++-------
> 7 files changed, 21 insertions(+), 20 deletions(-)
>
> diff --git a/include/hw/pci/pci.h b/include/hw/pci/pci.h
> index a37a2d5cb6..2242aa25eb 100644
> --- a/include/hw/pci/pci.h
> +++ b/include/hw/pci/pci.h
> @@ -393,12 +393,13 @@ 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,
> +void pci_bus_new_inplace(PCIBus *bus, size_t bus_size,
> + PCIHostState *phb,
> 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,
> +PCIBus *pci_bus_new(PCIHostState *phb, const char *name,
> MemoryRegion *address_space_mem,
> MemoryRegion *address_space_io,
> uint8_t devfn_min, const char *typename);
> diff --git a/hw/pci-bridge/pci_expander_bridge.c b/hw/pci-bridge/pci_expander_bridge.c
> index 6ac187fa32..39d29d2230 100644
> --- a/hw/pci-bridge/pci_expander_bridge.c
> +++ b/hw/pci-bridge/pci_expander_bridge.c
> @@ -213,7 +213,8 @@ static gint pxb_compare(gconstpointer a, gconstpointer b)
> static void pxb_dev_realize_common(PCIDevice *dev, bool pcie, Error **errp)
> {
> PXBDev *pxb = convert_to_pxb(dev);
> - DeviceState *ds, *bds = NULL;
> + DeviceState *bds = NULL;
> + PCIHostState *phb;
> PCIBus *bus;
> const char *dev_name = NULL;
> Error *local_err = NULL;
> @@ -228,11 +229,11 @@ static void pxb_dev_realize_common(PCIDevice *dev, bool pcie, Error **errp)
> dev_name = dev->qdev.id;
> }
>
> - ds = qdev_create(NULL, TYPE_PXB_HOST);
> + phb = PCI_HOST_BRIDGE(qdev_create(NULL, TYPE_PXB_HOST));
> if (pcie) {
> - bus = pci_bus_new(ds, dev_name, NULL, NULL, 0, TYPE_PXB_PCIE_BUS);
> + bus = pci_bus_new(phb, 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_bus_new(phb, "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);
> @@ -244,7 +245,7 @@ static void pxb_dev_realize_common(PCIDevice *dev, bool pcie, Error **errp)
> bus->address_space_io = dev->bus->address_space_io;
> bus->map_irq = pxb_map_irq_fn;
>
> - PCI_HOST_BRIDGE(ds)->bus = bus;
> + phb->bus = bus;
>
> pxb_register_bus(dev, bus, &local_err);
> if (local_err) {
> @@ -252,7 +253,7 @@ static void pxb_dev_realize_common(PCIDevice *dev, bool pcie, Error **errp)
> goto err_register_bus;
> }
>
> - qdev_init_nofail(ds);
> + qdev_init_nofail(DEVICE(phb));
> if (bds) {
> qdev_init_nofail(bds);
> }
> @@ -267,7 +268,7 @@ static void pxb_dev_realize_common(PCIDevice *dev, bool pcie, Error **errp)
> err_register_bus:
> object_unref(OBJECT(bds));
> object_unparent(OBJECT(bus));
> - object_unref(OBJECT(ds));
> + object_unref(OBJECT(phb));
> }
>
> static void pxb_dev_realize(PCIDevice *dev, Error **errp)
> diff --git a/hw/pci-host/piix.c b/hw/pci-host/piix.c
> index f9218aa952..91fec05b38 100644
> --- a/hw/pci-host/piix.c
> +++ b/hw/pci-host/piix.c
> @@ -340,7 +340,7 @@ 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,
> + b = pci_bus_new(s, 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);
> diff --git a/hw/pci-host/prep.c b/hw/pci-host/prep.c
> index 260a119a9e..2e2cd267f4 100644
> --- a/hw/pci-host/prep.c
> +++ b/hw/pci-host/prep.c
> @@ -269,7 +269,7 @@ 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,
> + pci_bus_new_inplace(&s->pci_bus, sizeof(s->pci_bus), h, NULL,
> &s->pci_memory, &s->pci_io, 0, TYPE_PCI_BUS);
>
> /* Bus master address space */
> diff --git a/hw/pci-host/q35.c b/hw/pci-host/q35.c
> index 344f77b10c..860b47a1ba 100644
> --- a/hw/pci-host/q35.c
> +++ b/hw/pci-host/q35.c
> @@ -49,7 +49,7 @@ 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",
> + pci->bus = pci_bus_new(pci, "pcie.0",
> s->mch.pci_address_space, s->mch.address_space_io,
> 0, TYPE_PCIE_BUS);
> PC_MACHINE(qdev_get_machine())->bus = pci->bus;
> diff --git a/hw/pci-host/versatile.c b/hw/pci-host/versatile.c
> index 467cbb9cb8..24ef87610b 100644
> --- a/hw/pci-host/versatile.c
> +++ b/hw/pci-host/versatile.c
> @@ -386,7 +386,7 @@ static void pci_vpb_init(Object *obj)
> 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), DEVICE(obj), "pci",
> + pci_bus_new_inplace(&s->pci_bus, sizeof(s->pci_bus), h, "pci",
> &s->pci_mem_space, &s->pci_io_space,
> PCI_DEVFN(11, 0), TYPE_PCI_BUS);
> h->bus = &s->pci_bus;
> diff --git a/hw/pci/pci.c b/hw/pci/pci.c
> index d9535c0bdc..f4488b46fc 100644
> --- a/hw/pci/pci.c
> +++ b/hw/pci/pci.c
> @@ -388,26 +388,25 @@ 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,
> +void pci_bus_new_inplace(PCIBus *bus, size_t bus_size,
> + PCIHostState *phb,
> const char *name,
> MemoryRegion *address_space_mem,
> MemoryRegion *address_space_io,
> uint8_t devfn_min, const char *typename)
> {
> - PCIHostState *phb = PCI_HOST_BRIDGE(parent);
> - qbus_create_inplace(bus, bus_size, typename, parent, name);
> + qbus_create_inplace(bus, bus_size, typename, DEVICE(phb), name);
> pci_bus_init(bus, phb, address_space_mem, address_space_io, devfn_min);
> }
>
> -PCIBus *pci_bus_new(DeviceState *parent, const char *name,
> +PCIBus *pci_bus_new(PCIHostState *phb, const char *name,
> MemoryRegion *address_space_mem,
> MemoryRegion *address_space_io,
> uint8_t devfn_min, const char *typename)
> {
> - PCIHostState *phb = PCI_HOST_BRIDGE(parent);
> PCIBus *bus;
>
> - bus = PCI_BUS(qbus_create(typename, parent, name));
> + bus = PCI_BUS(qbus_create(typename, DEVICE(phb), name));
> pci_bus_init(bus, phb, address_space_mem, address_space_io, devfn_min);
> return bus;
> }
> @@ -431,7 +430,7 @@ PCIBus *pci_register_bus(DeviceState *parent, const char *name,
> {
> PCIBus *bus;
>
> - bus = pci_bus_new(parent, name, address_space_mem,
> + bus = pci_bus_new(PCI_HOST_BRIDGE(parent), name, address_space_mem,
> address_space_io, devfn_min, typename);
> pci_bus_irqs(bus, set_irq, map_irq, irq_opaque, nirq);
> return bus;
> --
> 2.11.0.259.g40922b1
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [Qemu-devel] [RFC 4/7] pci: Change pci_register_bus() 'parent' parameter to PCIHostState
2017-04-17 21:59 ` [Qemu-devel] [RFC 4/7] pci: Change pci_register_bus() 'parent' " Eduardo Habkost
` (2 preceding siblings ...)
2017-04-18 13:37 ` Marcel Apfelbaum
@ 2017-04-26 16:17 ` Michael S. Tsirkin
3 siblings, 0 replies; 29+ messages in thread
From: Michael S. Tsirkin @ 2017-04-26 16:17 UTC (permalink / raw)
To: Eduardo Habkost
Cc: qemu-devel, aik, David Gibson, Laszlo Ersek, Marcel Apfelbaum,
Richard Henderson, Aurelien Jarno, Yongbok Kim, Alexander Graf,
Scott Wood, Paul Burton, David Gibson, Cornelia Huck,
Christian Borntraeger, qemu-ppc
On Mon, Apr 17, 2017 at 06:59:13PM -0300, Eduardo Habkost wrote:
> pci_register_bus() already requires the 'parent' argument to be a
> PCI_HOST_BRIDGE object. Change the parameter type to reflect that.
>
> Cc: Richard Henderson <rth@twiddle.net>
> Cc: Aurelien Jarno <aurelien@aurel32.net>
> Cc: Yongbok Kim <yongbok.kim@imgtec.com>
> Cc: Alexander Graf <agraf@suse.de>
> Cc: Scott Wood <scottwood@freescale.com>
> Cc: Paul Burton <paul.burton@imgtec.com>
> Cc: "Michael S. Tsirkin" <mst@redhat.com>
> Cc: Marcel Apfelbaum <marcel@redhat.com>
> Cc: David Gibson <david@gibson.dropbear.id.au>
> Cc: Cornelia Huck <cornelia.huck@de.ibm.com>
> Cc: Christian Borntraeger <borntraeger@de.ibm.com>
> Cc: qemu-ppc@nongnu.org
> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
> ---
Same here. Let's rename so it implies what it does?
> include/hw/pci/pci.h | 2 +-
> hw/alpha/typhoon.c | 2 +-
> hw/mips/gt64xxx_pci.c | 2 +-
> hw/pci-host/apb.c | 2 +-
> hw/pci-host/bonito.c | 2 +-
> hw/pci-host/gpex.c | 2 +-
> hw/pci-host/grackle.c | 2 +-
> hw/pci-host/ppce500.c | 2 +-
> hw/pci-host/uninorth.c | 4 ++--
> hw/pci-host/xilinx-pcie.c | 2 +-
> hw/pci/pci.c | 4 ++--
> hw/ppc/ppc4xx_pci.c | 2 +-
> hw/ppc/spapr_pci.c | 2 +-
> hw/s390x/s390-pci-bus.c | 2 +-
> hw/sh4/sh_pci.c | 2 +-
> 15 files changed, 17 insertions(+), 17 deletions(-)
>
> diff --git a/include/hw/pci/pci.h b/include/hw/pci/pci.h
> index 2242aa25eb..56387ccb0c 100644
> --- a/include/hw/pci/pci.h
> +++ b/include/hw/pci/pci.h
> @@ -408,7 +408,7 @@ void pci_bus_irqs(PCIBus *bus, pci_set_irq_fn set_irq, pci_map_irq_fn map_irq,
> 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,
> +PCIBus *pci_register_bus(PCIHostState *phb, const char *name,
> pci_set_irq_fn set_irq, pci_map_irq_fn map_irq,
> void *irq_opaque,
> MemoryRegion *address_space_mem,
> diff --git a/hw/alpha/typhoon.c b/hw/alpha/typhoon.c
> index f50f5cf186..ac0633a55e 100644
> --- a/hw/alpha/typhoon.c
> +++ b/hw/alpha/typhoon.c
> @@ -883,7 +883,7 @@ 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",
> + b = pci_register_bus(phb, "pci",
> typhoon_set_irq, sys_map_irq, s,
> &s->pchip.reg_mem, &s->pchip.reg_io,
> 0, 64, TYPE_PCI_BUS);
> diff --git a/hw/mips/gt64xxx_pci.c b/hw/mips/gt64xxx_pci.c
> index 4811843ab6..bd131bcdc6 100644
> --- a/hw/mips/gt64xxx_pci.c
> +++ b/hw/mips/gt64xxx_pci.c
> @@ -1171,7 +1171,7 @@ 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",
> + phb->bus = pci_register_bus(phb, "pci",
> gt64120_pci_set_irq, gt64120_pci_map_irq,
> pic,
> &d->pci0_mem,
> diff --git a/hw/pci-host/apb.c b/hw/pci-host/apb.c
> index 653e711121..1156a54224 100644
> --- a/hw/pci-host/apb.c
> +++ b/hw/pci-host/apb.c
> @@ -671,7 +671,7 @@ 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",
> + phb->bus = pci_register_bus(phb, "pci",
> pci_apb_set_irq, pci_pbm_map_irq, d,
> &d->pci_mmio,
> get_system_io(),
> diff --git a/hw/pci-host/bonito.c b/hw/pci-host/bonito.c
> index 1999ece590..27842edc04 100644
> --- a/hw/pci-host/bonito.c
> +++ b/hw/pci-host/bonito.c
> @@ -714,7 +714,7 @@ static int bonito_pcihost_initfn(SysBusDevice *dev)
> {
> PCIHostState *phb = PCI_HOST_BRIDGE(dev);
>
> - phb->bus = pci_register_bus(DEVICE(dev), "pci",
> + phb->bus = pci_register_bus(phb, "pci",
> pci_bonito_set_irq, pci_bonito_map_irq, dev,
> get_system_memory(), get_system_io(),
> 0x28, 32, TYPE_PCI_BUS);
> diff --git a/hw/pci-host/gpex.c b/hw/pci-host/gpex.c
> index 66055ee5cc..042d127271 100644
> --- a/hw/pci-host/gpex.c
> +++ b/hw/pci-host/gpex.c
> @@ -62,7 +62,7 @@ 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->bus = pci_register_bus(pci, "pcie.0", gpex_set_irq,
> pci_swizzle_map_irq_fn, s, &s->io_mmio,
> &s->io_ioport, 0, 4, TYPE_PCIE_BUS);
>
> diff --git a/hw/pci-host/grackle.c b/hw/pci-host/grackle.c
> index 2c8acdaaca..a56c063be9 100644
> --- a/hw/pci-host/grackle.c
> +++ b/hw/pci-host/grackle.c
> @@ -82,7 +82,7 @@ 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,
> + phb->bus = pci_register_bus(phb, NULL,
> pci_grackle_set_irq,
> pci_grackle_map_irq,
> pic,
> diff --git a/hw/pci-host/ppce500.c b/hw/pci-host/ppce500.c
> index e502bc0505..4a1e99f426 100644
> --- a/hw/pci-host/ppce500.c
> +++ b/hw/pci-host/ppce500.c
> @@ -465,7 +465,7 @@ 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,
> + b = pci_register_bus(h, 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;
> diff --git a/hw/pci-host/uninorth.c b/hw/pci-host/uninorth.c
> index df342ac3cb..b1b89183fc 100644
> --- a/hw/pci-host/uninorth.c
> +++ b/hw/pci-host/uninorth.c
> @@ -233,7 +233,7 @@ 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,
> + h->bus = pci_register_bus(h, NULL,
> pci_unin_set_irq, pci_unin_map_irq,
> pic,
> &d->pci_mmio,
> @@ -299,7 +299,7 @@ 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,
> + h->bus = pci_register_bus(h, NULL,
> pci_unin_set_irq, pci_unin_map_irq,
> pic,
> &d->pci_mmio,
> diff --git a/hw/pci-host/xilinx-pcie.c b/hw/pci-host/xilinx-pcie.c
> index 8b71e2d950..c951684148 100644
> --- a/hw/pci-host/xilinx-pcie.c
> +++ b/hw/pci-host/xilinx-pcie.c
> @@ -129,7 +129,7 @@ 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->bus = pci_register_bus(pci, s->name, xilinx_pcie_set_irq,
> pci_swizzle_map_irq_fn, s, &s->mmio,
> &s->io, 0, 4, TYPE_PCIE_BUS);
>
> diff --git a/hw/pci/pci.c b/hw/pci/pci.c
> index f4488b46fc..f60d0497ef 100644
> --- a/hw/pci/pci.c
> +++ b/hw/pci/pci.c
> @@ -421,7 +421,7 @@ 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,
> +PCIBus *pci_register_bus(PCIHostState *phb, const char *name,
> pci_set_irq_fn set_irq, pci_map_irq_fn map_irq,
> void *irq_opaque,
> MemoryRegion *address_space_mem,
> @@ -430,7 +430,7 @@ PCIBus *pci_register_bus(DeviceState *parent, const char *name,
> {
> PCIBus *bus;
>
> - bus = pci_bus_new(PCI_HOST_BRIDGE(parent), name, address_space_mem,
> + bus = pci_bus_new(phb, 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 dc19682970..f755c6faae 100644
> --- a/hw/ppc/ppc4xx_pci.c
> +++ b/hw/ppc/ppc4xx_pci.c
> @@ -314,7 +314,7 @@ 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,
> + b = pci_register_bus(h, 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;
> diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c
> index 98c52e411f..0f293f9e75 100644
> --- a/hw/ppc/spapr_pci.c
> +++ b/hw/ppc/spapr_pci.c
> @@ -1697,7 +1697,7 @@ 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,
> + bus = pci_register_bus(phb, NULL,
> pci_spapr_set_irq, pci_spapr_map_irq, sphb,
> &sphb->memspace, &sphb->iospace,
> PCI_DEVFN(0, 0), PCI_NUM_PINS, TYPE_PCI_BUS);
> diff --git a/hw/s390x/s390-pci-bus.c b/hw/s390x/s390-pci-bus.c
> index 69b0291e8a..99aae965bb 100644
> --- a/hw/s390x/s390-pci-bus.c
> +++ b/hw/s390x/s390-pci-bus.c
> @@ -560,7 +560,7 @@ static int s390_pcihost_init(SysBusDevice *dev)
>
> DPRINTF("host_init\n");
>
> - b = pci_register_bus(DEVICE(dev), NULL,
> + b = pci_register_bus(phb, NULL,
> s390_pci_set_irq, s390_pci_map_irq, NULL,
> get_system_memory(), get_system_io(), 0, 64,
> TYPE_PCI_BUS);
> diff --git a/hw/sh4/sh_pci.c b/hw/sh4/sh_pci.c
> index 1747628f3d..bca849c10f 100644
> --- a/hw/sh4/sh_pci.c
> +++ b/hw/sh4/sh_pci.c
> @@ -131,7 +131,7 @@ 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",
> + phb->bus = pci_register_bus(phb, "pci",
> sh_pci_set_irq, sh_pci_map_irq,
> s->irq,
> get_system_memory(),
> --
> 2.11.0.259.g40922b1
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [Qemu-devel] [RFC 3/7] pci: Change pci_bus_new*() parameter to PCIHostState
2017-04-26 16:16 ` Michael S. Tsirkin
@ 2017-04-26 17:49 ` Eduardo Habkost
0 siblings, 0 replies; 29+ messages in thread
From: Eduardo Habkost @ 2017-04-26 17:49 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: qemu-devel, aik, David Gibson, Laszlo Ersek, Marcel Apfelbaum,
Hervé Poussineau, Peter Maydell, qemu-ppc, qemu-arm
On Wed, Apr 26, 2017 at 07:16:26PM +0300, Michael S. Tsirkin wrote:
> On Mon, Apr 17, 2017 at 06:59:12PM -0300, Eduardo Habkost wrote:
> > The pci_bus_new*() functions already require the 'parent' argument to be
> > a PCI_HOST_BRIDGE object. Change the parameter type to reflect that.
> >
> > Cc: "Michael S. Tsirkin" <mst@redhat.com>
> > Cc: Marcel Apfelbaum <marcel@redhat.com>
> > Cc: "Hervé Poussineau" <hpoussin@reactos.org>
> > Cc: Peter Maydell <peter.maydell@linaro.org>
> > Cc: qemu-ppc@nongnu.org
> > Cc: qemu-arm@nongnu.org
> > Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
>
> This function doesn't create a generic pci bus, it's only good
> for creating root buses.
>
> As we are changing all callers anyway, let's rename this to
> pci_host_bus_new or pci_root_bus_new?
Something similar was already done on v2[1]. But based on
additional feedback on v2, I plan to make pci_bus_new() more
generic, moving the PCIHostState-specific initialization to
pci_host_bus_init*() wrappers.
[1] The new function names were:
* pci_host_bus_init() (replacing pci_bus_new())
* pci_host_bus_init_inplace() (replacing pci_bus_new_inplace())
* pci_host_bus_init_irqs() (replacing pci_register_bus())
>
> > ---
> > include/hw/pci/pci.h | 5 +++--
> > hw/pci-bridge/pci_expander_bridge.c | 15 ++++++++-------
> > hw/pci-host/piix.c | 2 +-
> > hw/pci-host/prep.c | 2 +-
> > hw/pci-host/q35.c | 2 +-
> > hw/pci-host/versatile.c | 2 +-
> > hw/pci/pci.c | 13 ++++++-------
> > 7 files changed, 21 insertions(+), 20 deletions(-)
> >
> > diff --git a/include/hw/pci/pci.h b/include/hw/pci/pci.h
> > index a37a2d5cb6..2242aa25eb 100644
> > --- a/include/hw/pci/pci.h
> > +++ b/include/hw/pci/pci.h
> > @@ -393,12 +393,13 @@ 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,
> > +void pci_bus_new_inplace(PCIBus *bus, size_t bus_size,
> > + PCIHostState *phb,
> > 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,
> > +PCIBus *pci_bus_new(PCIHostState *phb, const char *name,
> > MemoryRegion *address_space_mem,
> > MemoryRegion *address_space_io,
> > uint8_t devfn_min, const char *typename);
> > diff --git a/hw/pci-bridge/pci_expander_bridge.c b/hw/pci-bridge/pci_expander_bridge.c
> > index 6ac187fa32..39d29d2230 100644
> > --- a/hw/pci-bridge/pci_expander_bridge.c
> > +++ b/hw/pci-bridge/pci_expander_bridge.c
> > @@ -213,7 +213,8 @@ static gint pxb_compare(gconstpointer a, gconstpointer b)
> > static void pxb_dev_realize_common(PCIDevice *dev, bool pcie, Error **errp)
> > {
> > PXBDev *pxb = convert_to_pxb(dev);
> > - DeviceState *ds, *bds = NULL;
> > + DeviceState *bds = NULL;
> > + PCIHostState *phb;
> > PCIBus *bus;
> > const char *dev_name = NULL;
> > Error *local_err = NULL;
> > @@ -228,11 +229,11 @@ static void pxb_dev_realize_common(PCIDevice *dev, bool pcie, Error **errp)
> > dev_name = dev->qdev.id;
> > }
> >
> > - ds = qdev_create(NULL, TYPE_PXB_HOST);
> > + phb = PCI_HOST_BRIDGE(qdev_create(NULL, TYPE_PXB_HOST));
> > if (pcie) {
> > - bus = pci_bus_new(ds, dev_name, NULL, NULL, 0, TYPE_PXB_PCIE_BUS);
> > + bus = pci_bus_new(phb, 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_bus_new(phb, "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);
> > @@ -244,7 +245,7 @@ static void pxb_dev_realize_common(PCIDevice *dev, bool pcie, Error **errp)
> > bus->address_space_io = dev->bus->address_space_io;
> > bus->map_irq = pxb_map_irq_fn;
> >
> > - PCI_HOST_BRIDGE(ds)->bus = bus;
> > + phb->bus = bus;
> >
> > pxb_register_bus(dev, bus, &local_err);
> > if (local_err) {
> > @@ -252,7 +253,7 @@ static void pxb_dev_realize_common(PCIDevice *dev, bool pcie, Error **errp)
> > goto err_register_bus;
> > }
> >
> > - qdev_init_nofail(ds);
> > + qdev_init_nofail(DEVICE(phb));
> > if (bds) {
> > qdev_init_nofail(bds);
> > }
> > @@ -267,7 +268,7 @@ static void pxb_dev_realize_common(PCIDevice *dev, bool pcie, Error **errp)
> > err_register_bus:
> > object_unref(OBJECT(bds));
> > object_unparent(OBJECT(bus));
> > - object_unref(OBJECT(ds));
> > + object_unref(OBJECT(phb));
> > }
> >
> > static void pxb_dev_realize(PCIDevice *dev, Error **errp)
> > diff --git a/hw/pci-host/piix.c b/hw/pci-host/piix.c
> > index f9218aa952..91fec05b38 100644
> > --- a/hw/pci-host/piix.c
> > +++ b/hw/pci-host/piix.c
> > @@ -340,7 +340,7 @@ 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,
> > + b = pci_bus_new(s, 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);
> > diff --git a/hw/pci-host/prep.c b/hw/pci-host/prep.c
> > index 260a119a9e..2e2cd267f4 100644
> > --- a/hw/pci-host/prep.c
> > +++ b/hw/pci-host/prep.c
> > @@ -269,7 +269,7 @@ 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,
> > + pci_bus_new_inplace(&s->pci_bus, sizeof(s->pci_bus), h, NULL,
> > &s->pci_memory, &s->pci_io, 0, TYPE_PCI_BUS);
> >
> > /* Bus master address space */
> > diff --git a/hw/pci-host/q35.c b/hw/pci-host/q35.c
> > index 344f77b10c..860b47a1ba 100644
> > --- a/hw/pci-host/q35.c
> > +++ b/hw/pci-host/q35.c
> > @@ -49,7 +49,7 @@ 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",
> > + pci->bus = pci_bus_new(pci, "pcie.0",
> > s->mch.pci_address_space, s->mch.address_space_io,
> > 0, TYPE_PCIE_BUS);
> > PC_MACHINE(qdev_get_machine())->bus = pci->bus;
> > diff --git a/hw/pci-host/versatile.c b/hw/pci-host/versatile.c
> > index 467cbb9cb8..24ef87610b 100644
> > --- a/hw/pci-host/versatile.c
> > +++ b/hw/pci-host/versatile.c
> > @@ -386,7 +386,7 @@ static void pci_vpb_init(Object *obj)
> > 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), DEVICE(obj), "pci",
> > + pci_bus_new_inplace(&s->pci_bus, sizeof(s->pci_bus), h, "pci",
> > &s->pci_mem_space, &s->pci_io_space,
> > PCI_DEVFN(11, 0), TYPE_PCI_BUS);
> > h->bus = &s->pci_bus;
> > diff --git a/hw/pci/pci.c b/hw/pci/pci.c
> > index d9535c0bdc..f4488b46fc 100644
> > --- a/hw/pci/pci.c
> > +++ b/hw/pci/pci.c
> > @@ -388,26 +388,25 @@ 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,
> > +void pci_bus_new_inplace(PCIBus *bus, size_t bus_size,
> > + PCIHostState *phb,
> > const char *name,
> > MemoryRegion *address_space_mem,
> > MemoryRegion *address_space_io,
> > uint8_t devfn_min, const char *typename)
> > {
> > - PCIHostState *phb = PCI_HOST_BRIDGE(parent);
> > - qbus_create_inplace(bus, bus_size, typename, parent, name);
> > + qbus_create_inplace(bus, bus_size, typename, DEVICE(phb), name);
> > pci_bus_init(bus, phb, address_space_mem, address_space_io, devfn_min);
> > }
> >
> > -PCIBus *pci_bus_new(DeviceState *parent, const char *name,
> > +PCIBus *pci_bus_new(PCIHostState *phb, const char *name,
> > MemoryRegion *address_space_mem,
> > MemoryRegion *address_space_io,
> > uint8_t devfn_min, const char *typename)
> > {
> > - PCIHostState *phb = PCI_HOST_BRIDGE(parent);
> > PCIBus *bus;
> >
> > - bus = PCI_BUS(qbus_create(typename, parent, name));
> > + bus = PCI_BUS(qbus_create(typename, DEVICE(phb), name));
> > pci_bus_init(bus, phb, address_space_mem, address_space_io, devfn_min);
> > return bus;
> > }
> > @@ -431,7 +430,7 @@ PCIBus *pci_register_bus(DeviceState *parent, const char *name,
> > {
> > PCIBus *bus;
> >
> > - bus = pci_bus_new(parent, name, address_space_mem,
> > + bus = pci_bus_new(PCI_HOST_BRIDGE(parent), name, address_space_mem,
> > address_space_io, devfn_min, typename);
> > pci_bus_irqs(bus, set_irq, map_irq, irq_opaque, nirq);
> > return bus;
> > --
> > 2.11.0.259.g40922b1
--
Eduardo
^ permalink raw reply [flat|nested] 29+ messages in thread
end of thread, other threads:[~2017-04-26 17:49 UTC | newest]
Thread overview: 29+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-04-17 21:59 [Qemu-devel] [RFC 0/7] pci: Type-safety and phb->bus initialization cleanup Eduardo Habkost
2017-04-17 21:59 ` [Qemu-devel] [RFC 1/7] pci: Change pci_host_bus_register() parameter to PCIHostState Eduardo Habkost
2017-04-18 3:46 ` David Gibson
2017-04-18 13:01 ` Marcel Apfelbaum
2017-04-17 21:59 ` [Qemu-devel] [RFC 2/7] pci: Change pci_bus_init() 'parent' " Eduardo Habkost
2017-04-18 3:51 ` David Gibson
2017-04-18 11:48 ` Eduardo Habkost
2017-04-18 13:22 ` Marcel Apfelbaum
2017-04-17 21:59 ` [Qemu-devel] [RFC 3/7] pci: Change pci_bus_new*() " Eduardo Habkost
2017-04-18 3:52 ` David Gibson
2017-04-18 13:27 ` Marcel Apfelbaum
2017-04-18 13:30 ` Eduardo Habkost
2017-04-18 13:36 ` Marcel Apfelbaum
2017-04-26 16:16 ` Michael S. Tsirkin
2017-04-26 17:49 ` Eduardo Habkost
2017-04-17 21:59 ` [Qemu-devel] [RFC 4/7] pci: Change pci_register_bus() 'parent' " Eduardo Habkost
2017-04-18 3:53 ` David Gibson
2017-04-18 10:41 ` Cornelia Huck
2017-04-18 13:37 ` Marcel Apfelbaum
2017-04-26 16:17 ` Michael S. Tsirkin
2017-04-17 21:59 ` [Qemu-devel] [RFC 5/7] pci: Set phb->bus inside pci_register_bus() Eduardo Habkost
2017-04-18 3:55 ` David Gibson
2017-04-18 10:42 ` Cornelia Huck
2017-04-18 13:43 ` Marcel Apfelbaum
2017-04-18 13:53 ` Eduardo Habkost
2017-04-17 21:59 ` [Qemu-devel] [RFC 6/7] pci: Set phb->bus inside pci_bus_new() Eduardo Habkost
2017-04-18 3:56 ` David Gibson
2017-04-17 21:59 ` [Qemu-devel] [RFC 7/7] pci: Set phb->bus inside pci_bus_new_inplace() Eduardo Habkost
2017-04-18 3:56 ` David Gibson
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.