On Sat, Mar 10, 2012 at 11:40:41PM +0100, Alexander Graf wrote:
>
> On 08.03.2012, at 02:12, David Gibson wrote:
>
> > Currently, the function spapr_create_phb() uses its parameters to
> > initialize the correct memory windows for the new PCI Host Bridge
> > (PHB). This is not the way things are supposed to be done with qdevs,
> > and means you can't create extra PHBs easily using -device.
> >
> > Since pSeries machines can and do have many PHBs with various
> > configurations, this is a real limitation, not just a theoretical.
> > This patch, therefore, alters the PHB initialization code to use qdev
> > properties to set these parameters of the new bridge, moving most of
> > the code from spapr_create_phb() to spapr_phb_init().
> >
> > While we're at it, we change the naming of each PCI bus and its
> > associated memory regions to be less arbitrary and make it easier to
> > relate the guest and qemu views of memory to each other.
> >
> > Signed-off-by: Alexey Kardashevskiy <a...@ozlabs.ru>
> > Signed-off-by: David Gibson <da...@gibson.dropbear.id.au>
>
> Andreas, could you please (n)ack this version?
Actually, can you review and merge this updated version instead.
Turns out that libvirt and some other tools assume that there will be
a PCI bus named simply "pci", which is no longer true with the patch
above (we call each bus "pci@XXXXXX" to match the guest's device
tree). The revised version calls the default bus simply "pci" to work
around libvirt's PC centrism, and allows manually added extra busses
to be named explicitly with a property, defaulting to "pci@XXXXX".
>From c2ce4ad7c4e36ea79d34cfa3305f4b027492474e Mon Sep 17 00:00:00 2001
From: David Gibson <da...@gibson.dropbear.id.au>
Date: Tue, 13 Mar 2012 14:13:26 +1100
Subject: [PATCH] pseries: Configure PCI bridge using properties
Currently, the function spapr_create_phb() uses its parameters to
initialize the correct memory windows for the new PCI Host Bridge
(PHB). This is not the way things are supposed to be done with qdevs,
and means you can't create extra PHBs easily using -device.
Since pSeries machines can and do have many PHBs with various
configurations, this is a real limitation, not just a theoretical.
This patch, therefore, alters the PHB initialization code to use qdev
properties to set these parameters of the new bridge, moving most of
the code from spapr_create_phb() to spapr_phb_init().
While we're at it, we change the naming of each PCI bus and its
associated memory regions to be less arbitrary and make it easier to
relate the guest and qemu views of memory to each other.
Signed-off-by: Alexey Kardashevskiy <a...@ozlabs.ru>
Signed-off-by: David Gibson <da...@gibson.dropbear.id.au>
---
hw/spapr_pci.c | 166 +++++++++++++++++++++++++++++++-------------------------
hw/spapr_pci.h | 4 +-
2 files changed, 94 insertions(+), 76 deletions(-)
diff --git a/hw/spapr_pci.c b/hw/spapr_pci.c
index 38cd7b3..77f3e52 100644
--- a/hw/spapr_pci.c
+++ b/hw/spapr_pci.c
@@ -161,49 +161,6 @@ static void pci_spapr_set_irq(void *opaque, int irq_num,
int level)
qemu_set_irq(phb->lsi_table[irq_num].qirq, level);
}
-static int spapr_phb_init(SysBusDevice *s)
-{
- sPAPRPHBState *phb = FROM_SYSBUS(sPAPRPHBState, s);
- int i;
-
- /* Initialize the LSI table */
- for (i = 0; i < SPAPR_PCI_NUM_LSI; i++) {
- qemu_irq qirq;
- uint32_t num;
-
- qirq = spapr_allocate_lsi(0, &num);
- if (!qirq) {
- return -1;
- }
-
- phb->lsi_table[i].dt_irq = num;
- phb->lsi_table[i].qirq = qirq;
- }
-
- return 0;
-}
-
-static void spapr_phb_class_init(ObjectClass *klass, void *data)
-{
- SysBusDeviceClass *sdc = SYS_BUS_DEVICE_CLASS(klass);
-
- sdc->init = spapr_phb_init;
-}
-
-static TypeInfo spapr_phb_info = {
- .name = "spapr-pci-host-bridge",
- .parent = TYPE_SYS_BUS_DEVICE,
- .instance_size = sizeof(sPAPRPHBState),
- .class_init = spapr_phb_class_init,
-};
-
-static void spapr_register_types(void)
-{
- type_register_static(&spapr_phb_info);
-}
-
-type_init(spapr_register_types)
-
static uint64_t spapr_io_read(void *opaque, target_phys_addr_t addr,
unsigned size)
{
@@ -241,35 +198,29 @@ static const MemoryRegionOps spapr_io_ops = {
.write = spapr_io_write
};
-void spapr_create_phb(sPAPREnvironment *spapr,
- const char *busname, uint64_t buid,
- uint64_t mem_win_addr, uint64_t mem_win_size,
- uint64_t io_win_addr)
+/*
+ * PHB PCI device
+ */
+static int spapr_phb_init(SysBusDevice *s)
{
- DeviceState *dev;
- SysBusDevice *s;
- sPAPRPHBState *phb;
+ sPAPRPHBState *phb = FROM_SYSBUS(sPAPRPHBState, s);
+ char *namebuf;
+ int i;
PCIBus *bus;
- char namebuf[strlen(busname)+11];
- dev = qdev_create(NULL, "spapr-pci-host-bridge");
- qdev_init_nofail(dev);
- s = sysbus_from_qdev(dev);
- phb = FROM_SYSBUS(sPAPRPHBState, s);
+ phb->dtbusname = g_strdup_printf("pci@%" PRIx64, phb->buid);
+ namebuf = alloca(strlen(phb->dtbusname) + 32);
- phb->mem_win_addr = mem_win_addr;
-
- sprintf(namebuf, "%s-mem", busname);
+ /* Initialize memory regions */
+ sprintf(namebuf, "%s.mmio", phb->dtbusname);
memory_region_init(&phb->memspace, namebuf, INT64_MAX);
- sprintf(namebuf, "%s-memwindow", busname);
+ sprintf(namebuf, "%s.mmio-alias", phb->dtbusname);
memory_region_init_alias(&phb->memwindow, namebuf, &phb->memspace,
- SPAPR_PCI_MEM_WIN_BUS_OFFSET, mem_win_size);
- memory_region_add_subregion(get_system_memory(), mem_win_addr,
+ SPAPR_PCI_MEM_WIN_BUS_OFFSET, phb->mem_win_size);
+ memory_region_add_subregion(get_system_memory(), phb->mem_win_addr,
&phb->memwindow);
- phb->io_win_addr = io_win_addr;
-
/* On ppc, we only have MMIO no specific IO space from the CPU
* perspective. In theory we ought to be able to embed the PCI IO
* memory region direction in the system memory space. However,
@@ -278,33 +229,92 @@ void spapr_create_phb(sPAPREnvironment *spapr,
* system io address space. This hack to bounce things via
* system_io works around the problem until all the users of
* old_portion are updated */
- sprintf(namebuf, "%s-io", busname);
+ sprintf(namebuf, "%s.io", phb->dtbusname);
memory_region_init(&phb->iospace, namebuf, SPAPR_PCI_IO_WIN_SIZE);
/* FIXME: fix to support multiple PHBs */
memory_region_add_subregion(get_system_io(), 0, &phb->iospace);
- sprintf(namebuf, "%s-iowindow", busname);
+ sprintf(namebuf, "%s.io-alias", phb->dtbusname);
memory_region_init_io(&phb->iowindow, &spapr_io_ops, phb,
namebuf, SPAPR_PCI_IO_WIN_SIZE);
- memory_region_add_subregion(get_system_memory(), io_win_addr,
+ memory_region_add_subregion(get_system_memory(), phb->io_win_addr,
&phb->iowindow);
- phb->host_state.bus = bus = pci_register_bus(&phb->busdev.qdev, busname,
- pci_spapr_set_irq,
- pci_spapr_map_irq,
- phb,
- &phb->memspace, &phb->iospace,
- PCI_DEVFN(0, 0),
- SPAPR_PCI_NUM_LSI);
+ bus = pci_register_bus(&phb->busdev.qdev,
+ phb->busname ? phb->busname : phb->dtbusname,
+ pci_spapr_set_irq, pci_spapr_map_irq, phb,
+ &phb->memspace, &phb->iospace,
+ PCI_DEVFN(0, 0), SPAPR_PCI_NUM_LSI);
+ phb->host_state.bus = bus;
+
+ QLIST_INSERT_HEAD(&spapr->phbs, phb, list);
+
+ /* Initialize the LSI table */
+ for (i = 0; i < SPAPR_PCI_NUM_LSI; i++) {
+ qemu_irq qirq;
+ uint32_t num;
+
+ qirq = spapr_allocate_lsi(0, &num);
+ if (!qirq) {
+ return -1;
+ }
+
+ phb->lsi_table[i].dt_irq = num;
+ phb->lsi_table[i].qirq = qirq;
+ }
+
+ return 0;
+}
+
+static Property spapr_phb_properties[] = {
+ DEFINE_PROP_HEX64("buid", sPAPRPHBState, buid, 0),
+ DEFINE_PROP_STRING("busname", sPAPRPHBState, busname),
+ DEFINE_PROP_HEX64("mem_win_addr", sPAPRPHBState, mem_win_addr, 0),
+ DEFINE_PROP_HEX64("mem_win_size", sPAPRPHBState, mem_win_size, 0x20000000),
+ DEFINE_PROP_HEX64("io_win_addr", sPAPRPHBState, io_win_addr, 0),
+ DEFINE_PROP_HEX64("io_win_size", sPAPRPHBState, io_win_size, 0x10000),
+ DEFINE_PROP_END_OF_LIST(),
+};
+
+static void spapr_phb_class_init(ObjectClass *klass, void *data)
+{
+ SysBusDeviceClass *sdc = SYS_BUS_DEVICE_CLASS(klass);
+ DeviceClass *dc = DEVICE_CLASS(klass);
+
+ sdc->init = spapr_phb_init;
+ dc->props = spapr_phb_properties;
spapr_rtas_register("read-pci-config", rtas_read_pci_config);
spapr_rtas_register("write-pci-config", rtas_write_pci_config);
spapr_rtas_register("ibm,read-pci-config", rtas_ibm_read_pci_config);
spapr_rtas_register("ibm,write-pci-config", rtas_ibm_write_pci_config);
+}
- QLIST_INSERT_HEAD(&spapr->phbs, phb, list);
+static TypeInfo spapr_phb_info = {
+ .name = "spapr-pci-host-bridge",
+ .parent = TYPE_SYS_BUS_DEVICE,
+ .instance_size = sizeof(sPAPRPHBState),
+ .class_init = spapr_phb_class_init,
+};
+
+void spapr_create_phb(sPAPREnvironment *spapr,
+ const char *busname, uint64_t buid,
+ uint64_t mem_win_addr, uint64_t mem_win_size,
+ uint64_t io_win_addr)
+{
+ DeviceState *dev;
+
+ dev = qdev_create(NULL, spapr_phb_info.name);
- /* pci_bus_set_mem_base(bus, mem_va_start - SPAPR_PCI_MEM_BAR_START); */
+ if (busname) {
+ qdev_prop_set_string(dev, "busname", g_strdup(busname));
+ }
+ qdev_prop_set_uint64(dev, "buid", buid);
+ qdev_prop_set_uint64(dev, "mem_win_addr", mem_win_addr);
+ qdev_prop_set_uint64(dev, "mem_win_size", mem_win_size);
+ qdev_prop_set_uint64(dev, "io_win_addr", io_win_addr);
+
+ qdev_init_nofail(dev);
}
/* Macros to operate with address in OF binding to PCI */
@@ -396,3 +406,9 @@ int spapr_populate_pci_devices(sPAPRPHBState *phb,
return 0;
}
+
+static void register_types(void)
+{
+ type_register_static(&spapr_phb_info);
+}
+type_init(register_types)
diff --git a/hw/spapr_pci.h b/hw/spapr_pci.h
index 213340c..039f85b 100644
--- a/hw/spapr_pci.h
+++ b/hw/spapr_pci.h
@@ -33,9 +33,11 @@ typedef struct sPAPRPHBState {
PCIHostState host_state;
uint64_t buid;
+ char *busname;
+ char *dtbusname;
MemoryRegion memspace, iospace;
- target_phys_addr_t mem_win_addr, io_win_addr;
+ target_phys_addr_t mem_win_addr, mem_win_size, io_win_addr, io_win_size;
MemoryRegion memwindow, iowindow;
struct {
--
1.7.9.1
--
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
