diff mbox

[2/5] pseries: Remove "busname" property for PCI host bridge

Message ID 1363226008-26639-3-git-send-email-david@gibson.dropbear.id.au
State New
Headers show

Commit Message

David Gibson March 14, 2013, 1:53 a.m. UTC
Currently the "spapr-pci-host-bridge" device has a "busname" property which
can be used to override the default assignment of qbus names for the bus
subordinate to the PHB.  We use that for the default primary PCI bus, to
make libvirt happy, which expects there to be a bus named simply "pci".
The default qdev core logic would name the bus "pci.0", and the pseries
code would otherwise name it "pci@800000020000000" which is the name it
is given in the device tree based on its BUID.

The "busname" property is rather clunky though, so this patch simplifies
things by just using a special case hack for the default PHB, setting
busname to "pci" when index=0.

Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
 hw/ppc/spapr.c |    2 +-
 hw/spapr_pci.c |   30 ++++++++++++++++++++++--------
 hw/spapr_pci.h |    4 +---
 3 files changed, 24 insertions(+), 12 deletions(-)

Comments

Alexander Graf March 15, 2013, 12:39 p.m. UTC | #1
On 14.03.2013, at 02:53, David Gibson wrote:

> Currently the "spapr-pci-host-bridge" device has a "busname" property which
> can be used to override the default assignment of qbus names for the bus
> subordinate to the PHB.  We use that for the default primary PCI bus, to
> make libvirt happy, which expects there to be a bus named simply "pci".
> The default qdev core logic would name the bus "pci.0", and the pseries
> code would otherwise name it "pci@800000020000000" which is the name it
> is given in the device tree based on its BUID.
> 
> The "busname" property is rather clunky though, so this patch simplifies
> things by just using a special case hack for the default PHB, setting
> busname to "pci" when index=0.
> 
> Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
> Signed-off-by: David Gibson <david@gibson.dropbear.id.au>

Thanks, applied to ppc-next.

Alex

> ---
> hw/ppc/spapr.c |    2 +-
> hw/spapr_pci.c |   30 ++++++++++++++++++++++--------
> hw/spapr_pci.h |    4 +---
> 3 files changed, 24 insertions(+), 12 deletions(-)
> 
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index fd24113..9a13697 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -852,7 +852,7 @@ static void ppc_spapr_init(QEMUMachineInitArgs *args)
>     /* Set up PCI */
>     spapr_pci_rtas_init();
> 
> -    phb = spapr_create_phb(spapr, 0, "pci");
> +    phb = spapr_create_phb(spapr, 0);
> 
>     for (i = 0; i < nb_nics; i++) {
>         NICInfo *nd = &nd_table[i];
> diff --git a/hw/spapr_pci.c b/hw/spapr_pci.c
> index 36adbc5..42c8b61 100644
> --- a/hw/spapr_pci.c
> +++ b/hw/spapr_pci.c
> @@ -518,6 +518,7 @@ static int spapr_phb_init(SysBusDevice *s)
> {
>     sPAPRPHBState *sphb = SPAPR_PCI_HOST_BRIDGE(s);
>     PCIHostState *phb = PCI_HOST_BRIDGE(s);
> +    const char *busname;
>     char *namebuf;
>     int i;
>     PCIBus *bus;
> @@ -575,9 +576,6 @@ static int spapr_phb_init(SysBusDevice *s)
>     }
> 
>     sphb->dtbusname = g_strdup_printf("pci@%" PRIx64, sphb->buid);
> -    if (!sphb->busname) {
> -        sphb->busname = sphb->dtbusname;
> -    }
> 
>     namebuf = alloca(strlen(sphb->dtbusname) + 32);
> 
> @@ -621,7 +619,26 @@ static int spapr_phb_init(SysBusDevice *s)
>                                     &sphb->msiwindow);
>     }
> 
> -    bus = pci_register_bus(DEVICE(s), sphb->busname,
> +    /*
> +     * Selecting a busname is more complex than you'd think, due to
> +     * interacting constraints.  If the user has specified an id
> +     * explicitly for the phb , then we want to use the qdev default
> +     * of naming the bus based on the bridge device (so the user can
> +     * then assign devices to it in the way they expect).  For the
> +     * first / default PCI bus (index=0) we want to use just "pci"
> +     * because libvirt expects there to be a bus called, simply,
> +     * "pci".  Otherwise, we use the same name as in the device tree,
> +     * since it's unique by construction, and makes the guest visible
> +     * BUID clear.
> +     */
> +    if (s->qdev.id) {
> +        busname = NULL;
> +    } else if (sphb->index == 0) {
> +        busname = "pci";
> +    } else {
> +        busname = sphb->dtbusname;
> +    }
> +    bus = pci_register_bus(DEVICE(s), busname,
>                            pci_spapr_set_irq, pci_spapr_map_irq, sphb,
>                            &sphb->memspace, &sphb->iospace,
>                            PCI_DEVFN(0, 0), PCI_NUM_PINS);
> @@ -663,7 +680,6 @@ static void spapr_phb_reset(DeviceState *qdev)
> }
> 
> static Property spapr_phb_properties[] = {
> -    DEFINE_PROP_STRING("busname", sPAPRPHBState, busname),
>     DEFINE_PROP_INT32("index", sPAPRPHBState, index, -1),
>     DEFINE_PROP_HEX64("buid", sPAPRPHBState, buid, -1),
>     DEFINE_PROP_HEX32("liobn", sPAPRPHBState, dma_liobn, -1),
> @@ -694,14 +710,12 @@ static const TypeInfo spapr_phb_info = {
>     .class_init    = spapr_phb_class_init,
> };
> 
> -PCIHostState *spapr_create_phb(sPAPREnvironment *spapr, int index,
> -                               const char *busname)
> +PCIHostState *spapr_create_phb(sPAPREnvironment *spapr, int index)
> {
>     DeviceState *dev;
> 
>     dev = qdev_create(NULL, TYPE_SPAPR_PCI_HOST_BRIDGE);
>     qdev_prop_set_uint32(dev, "index", index);
> -    qdev_prop_set_string(dev, "busname", busname);
>     qdev_init_nofail(dev);
> 
>     return PCI_HOST_BRIDGE(dev);
> diff --git a/hw/spapr_pci.h b/hw/spapr_pci.h
> index 8bb3c62..8bd8a66 100644
> --- a/hw/spapr_pci.h
> +++ b/hw/spapr_pci.h
> @@ -39,7 +39,6 @@ typedef struct sPAPRPHBState {
> 
>     int32_t index;
>     uint64_t buid;
> -    char *busname;
>     char *dtbusname;
> 
>     MemoryRegion memspace, iospace;
> @@ -82,8 +81,7 @@ static inline qemu_irq spapr_phb_lsi_qirq(struct sPAPRPHBState *phb, int pin)
>     return xics_get_qirq(spapr->icp, phb->lsi_table[pin].irq);
> }
> 
> -PCIHostState *spapr_create_phb(sPAPREnvironment *spapr, int index,
> -                               const char *busname);
> +PCIHostState *spapr_create_phb(sPAPREnvironment *spapr, int index);
> 
> int spapr_populate_pci_dt(sPAPRPHBState *phb,
>                           uint32_t xics_phandle,
> -- 
> 1.7.10.4
>
diff mbox

Patch

diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index fd24113..9a13697 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -852,7 +852,7 @@  static void ppc_spapr_init(QEMUMachineInitArgs *args)
     /* Set up PCI */
     spapr_pci_rtas_init();
 
-    phb = spapr_create_phb(spapr, 0, "pci");
+    phb = spapr_create_phb(spapr, 0);
 
     for (i = 0; i < nb_nics; i++) {
         NICInfo *nd = &nd_table[i];
diff --git a/hw/spapr_pci.c b/hw/spapr_pci.c
index 36adbc5..42c8b61 100644
--- a/hw/spapr_pci.c
+++ b/hw/spapr_pci.c
@@ -518,6 +518,7 @@  static int spapr_phb_init(SysBusDevice *s)
 {
     sPAPRPHBState *sphb = SPAPR_PCI_HOST_BRIDGE(s);
     PCIHostState *phb = PCI_HOST_BRIDGE(s);
+    const char *busname;
     char *namebuf;
     int i;
     PCIBus *bus;
@@ -575,9 +576,6 @@  static int spapr_phb_init(SysBusDevice *s)
     }
 
     sphb->dtbusname = g_strdup_printf("pci@%" PRIx64, sphb->buid);
-    if (!sphb->busname) {
-        sphb->busname = sphb->dtbusname;
-    }
 
     namebuf = alloca(strlen(sphb->dtbusname) + 32);
 
@@ -621,7 +619,26 @@  static int spapr_phb_init(SysBusDevice *s)
                                     &sphb->msiwindow);
     }
 
-    bus = pci_register_bus(DEVICE(s), sphb->busname,
+    /*
+     * Selecting a busname is more complex than you'd think, due to
+     * interacting constraints.  If the user has specified an id
+     * explicitly for the phb , then we want to use the qdev default
+     * of naming the bus based on the bridge device (so the user can
+     * then assign devices to it in the way they expect).  For the
+     * first / default PCI bus (index=0) we want to use just "pci"
+     * because libvirt expects there to be a bus called, simply,
+     * "pci".  Otherwise, we use the same name as in the device tree,
+     * since it's unique by construction, and makes the guest visible
+     * BUID clear.
+     */
+    if (s->qdev.id) {
+        busname = NULL;
+    } else if (sphb->index == 0) {
+        busname = "pci";
+    } else {
+        busname = sphb->dtbusname;
+    }
+    bus = pci_register_bus(DEVICE(s), busname,
                            pci_spapr_set_irq, pci_spapr_map_irq, sphb,
                            &sphb->memspace, &sphb->iospace,
                            PCI_DEVFN(0, 0), PCI_NUM_PINS);
@@ -663,7 +680,6 @@  static void spapr_phb_reset(DeviceState *qdev)
 }
 
 static Property spapr_phb_properties[] = {
-    DEFINE_PROP_STRING("busname", sPAPRPHBState, busname),
     DEFINE_PROP_INT32("index", sPAPRPHBState, index, -1),
     DEFINE_PROP_HEX64("buid", sPAPRPHBState, buid, -1),
     DEFINE_PROP_HEX32("liobn", sPAPRPHBState, dma_liobn, -1),
@@ -694,14 +710,12 @@  static const TypeInfo spapr_phb_info = {
     .class_init    = spapr_phb_class_init,
 };
 
-PCIHostState *spapr_create_phb(sPAPREnvironment *spapr, int index,
-                               const char *busname)
+PCIHostState *spapr_create_phb(sPAPREnvironment *spapr, int index)
 {
     DeviceState *dev;
 
     dev = qdev_create(NULL, TYPE_SPAPR_PCI_HOST_BRIDGE);
     qdev_prop_set_uint32(dev, "index", index);
-    qdev_prop_set_string(dev, "busname", busname);
     qdev_init_nofail(dev);
 
     return PCI_HOST_BRIDGE(dev);
diff --git a/hw/spapr_pci.h b/hw/spapr_pci.h
index 8bb3c62..8bd8a66 100644
--- a/hw/spapr_pci.h
+++ b/hw/spapr_pci.h
@@ -39,7 +39,6 @@  typedef struct sPAPRPHBState {
 
     int32_t index;
     uint64_t buid;
-    char *busname;
     char *dtbusname;
 
     MemoryRegion memspace, iospace;
@@ -82,8 +81,7 @@  static inline qemu_irq spapr_phb_lsi_qirq(struct sPAPRPHBState *phb, int pin)
     return xics_get_qirq(spapr->icp, phb->lsi_table[pin].irq);
 }
 
-PCIHostState *spapr_create_phb(sPAPREnvironment *spapr, int index,
-                               const char *busname);
+PCIHostState *spapr_create_phb(sPAPREnvironment *spapr, int index);
 
 int spapr_populate_pci_dt(sPAPRPHBState *phb,
                           uint32_t xics_phandle,