Re: [PATCH v3 04/12] ppc/pnv: add PHB4 bus init helper

From: Frederic Barrat <fbarrat@linux.ibm.com>
To: Daniel Henrique Barboza <danielhb413@gmail.com>, qemu-devel@nongnu.org
Cc: qemu-ppc@nongnu.org, clg@kaod.org
Subject: Re: [PATCH v3 04/12] ppc/pnv: add PHB4 bus init helper
Date: Wed, 27 Jul 2022 19:32:05 +0200	[thread overview]
Message-ID: <2d0b9a63-825d-8562-bf90-977b99d42883@linux.ibm.com> (raw)
In-Reply-To: <20220624084921.399219-5-danielhb413@gmail.com>


On 24/06/2022 10:49, Daniel Henrique Barboza wrote:
> Similar to what we already did for the PnvPHB3 device, let's add a
> helper to init the bus when using a PnvPHB4. This helper will be used by
> PnvPHb when PnvPHB4 turns into a backend.
> 
> Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
> ---


Reviewed-by: Frederic Barrat <fbarrat@linux.ibm.com>

   Fred


>   hw/pci-host/pnv_phb.c          |  2 ++
>   hw/pci-host/pnv_phb4.c         | 39 ++++++++++++++++++++--------------
>   include/hw/pci-host/pnv_phb4.h |  1 +
>   3 files changed, 26 insertions(+), 16 deletions(-)
> 
> diff --git a/hw/pci-host/pnv_phb.c b/hw/pci-host/pnv_phb.c
> index 6fefff7d44..abcbcca445 100644
> --- a/hw/pci-host/pnv_phb.c
> +++ b/hw/pci-host/pnv_phb.c
> @@ -69,6 +69,8 @@ static void pnv_phb_realize(DeviceState *dev, Error **errp)
>   
>       if (phb->version == 3) {
>           pnv_phb3_bus_init(dev, PNV_PHB3(phb->backend));
> +    } else {
> +        pnv_phb4_bus_init(dev, PNV_PHB4(phb->backend));
>       }
>   
>       pnv_phb_attach_root_port(pci, phb_rootport_typename,
> diff --git a/hw/pci-host/pnv_phb4.c b/hw/pci-host/pnv_phb4.c
> index d225ab5b0f..a7a4519f30 100644
> --- a/hw/pci-host/pnv_phb4.c
> +++ b/hw/pci-host/pnv_phb4.c
> @@ -1544,30 +1544,16 @@ static void pnv_phb4_instance_init(Object *obj)
>       object_initialize_child(obj, "source", &phb->xsrc, TYPE_XIVE_SOURCE);
>   }
>   
> -static void pnv_phb4_realize(DeviceState *dev, Error **errp)
> +void pnv_phb4_bus_init(DeviceState *dev, PnvPHB4 *phb)
>   {
> -    PnvPHB4 *phb = PNV_PHB4(dev);
> -    PnvPhb4PecClass *pecc = PNV_PHB4_PEC_GET_CLASS(phb->pec);
>       PCIHostState *pci = PCI_HOST_BRIDGE(dev);
> -    XiveSource *xsrc = &phb->xsrc;
> -    int nr_irqs;
>       char name[32];
>   
> -    /* Set the "big_phb" flag */
> -    phb->big_phb = phb->phb_id == 0 || phb->phb_id == 3;
> -
> -    /* Controller Registers */
> -    snprintf(name, sizeof(name), "phb4-%d.%d-regs", phb->chip_id,
> -             phb->phb_id);
> -    memory_region_init_io(&phb->mr_regs, OBJECT(phb), &pnv_phb4_reg_ops, phb,
> -                          name, 0x2000);
> -
>       /*
>        * PHB4 doesn't support IO space. However, qemu gets very upset if
>        * we don't have an IO region to anchor IO BARs onto so we just
>        * initialize one which we never hook up to anything
>        */
> -
>       snprintf(name, sizeof(name), "phb4-%d.%d-pci-io", phb->chip_id,
>                phb->phb_id);
>       memory_region_init(&phb->pci_io, OBJECT(phb), name, 0x10000);
> @@ -1577,12 +1563,33 @@ static void pnv_phb4_realize(DeviceState *dev, Error **errp)
>       memory_region_init(&phb->pci_mmio, OBJECT(phb), name,
>                          PCI_MMIO_TOTAL_SIZE);
>   
> -    pci->bus = pci_register_root_bus(dev, dev->id,
> +    pci->bus = pci_register_root_bus(dev, dev->id ? dev->id : NULL,
>                                        pnv_phb4_set_irq, pnv_phb4_map_irq, phb,
>                                        &phb->pci_mmio, &phb->pci_io,
>                                        0, 4, TYPE_PNV_PHB4_ROOT_BUS);
>       pci_setup_iommu(pci->bus, pnv_phb4_dma_iommu, phb);
>       pci->bus->flags |= PCI_BUS_EXTENDED_CONFIG_SPACE;
> +}
> +
> +static void pnv_phb4_realize(DeviceState *dev, Error **errp)
> +{
> +    PnvPHB4 *phb = PNV_PHB4(dev);
> +    PnvPhb4PecClass *pecc = PNV_PHB4_PEC_GET_CLASS(phb->pec);
> +    PCIHostState *pci = PCI_HOST_BRIDGE(dev);
> +    XiveSource *xsrc = &phb->xsrc;
> +    int nr_irqs;
> +    char name[32];
> +
> +    /* Set the "big_phb" flag */
> +    phb->big_phb = phb->phb_id == 0 || phb->phb_id == 3;
> +
> +    /* Controller Registers */
> +    snprintf(name, sizeof(name), "phb4-%d.%d-regs", phb->chip_id,
> +             phb->phb_id);
> +    memory_region_init_io(&phb->mr_regs, OBJECT(phb), &pnv_phb4_reg_ops, phb,
> +                          name, 0x2000);
> +
> +    pnv_phb4_bus_init(dev, phb);
>   
>       /* Add a single Root port if running with defaults */
>       pnv_phb_attach_root_port(pci, pecc->rp_model,
> diff --git a/include/hw/pci-host/pnv_phb4.h b/include/hw/pci-host/pnv_phb4.h
> index 19dcbd6f87..90843ac3a9 100644
> --- a/include/hw/pci-host/pnv_phb4.h
> +++ b/include/hw/pci-host/pnv_phb4.h
> @@ -157,6 +157,7 @@ struct PnvPHB4 {
>   
>   void pnv_phb4_pic_print_info(PnvPHB4 *phb, Monitor *mon);
>   int pnv_phb4_pec_get_phb_id(PnvPhb4PecState *pec, int stack_index);
> +void pnv_phb4_bus_init(DeviceState *dev, PnvPHB4 *phb);
>   extern const MemoryRegionOps pnv_phb4_xscom_ops;
>   
>   /*