Re: [Qemu-ppc] [PATCH 2/5] pseries: Remove "busname" property for PCI host bridge

[Alexander Graf]

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 "address@hidden" 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 <address@hidden>
> Signed-off-by: David Gibson <address@hidden>

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("address@hidden" 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
>