Re: [PATCH v2 16/18] pnv_phb4.c: introduce pnv_pec_init_stack_xscom()

[Cédric Le Goater]

On 1/5/22 22:23, Daniel Henrique Barboza wrote:
> The XSCOM address space of the stack must be populated after the
> initialization of its associated PHB4 is completed. At this moment this
> is always true because stk_realize() will always succeeds the realize of
> stack->phb, but that will not be the case with user creatable pnv-phb4
> devices.
>
> Create a helper that can be used later on during pnv-phb4 realize() to
> initialize the xscom address space of its stack.

I think this patch prepares ground for user-created devices and should come
earlier in the series.

> Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>

Reviewed-by: Cédric Le Goater <clg@kaod.org>

Thanks,

C.


> ---
>   hw/pci-host/pnv_phb4.c         | 26 ++++++++++++++++++++++++++
>   hw/pci-host/pnv_phb4_pec.c     | 19 +------------------
>   include/hw/pci-host/pnv_phb4.h |  1 +
>   3 files changed, 28 insertions(+), 18 deletions(-)
>
> diff --git a/hw/pci-host/pnv_phb4.c b/hw/pci-host/pnv_phb4.c
> index 9e670e41d2..430a5c10f4 100644
> --- a/hw/pci-host/pnv_phb4.c
> +++ b/hw/pci-host/pnv_phb4.c
> @@ -1158,6 +1158,32 @@ static AddressSpace *pnv_phb4_dma_iommu(PCIBus *bus, 
> void *opaque, int devfn)
>       return &ds->dma_as;
>   }
>   
> +/*
> + * Init the xscom address space of the stack. This must be
> + * called after the associated stack->phb is defined.
> + */
> +void pnv_pec_init_stack_xscom(PnvPhb4PecStack *stack)
> +{
> +    PnvPhb4PecState *pec = stack->pec;
> +    PnvPhb4PecClass *pecc = PNV_PHB4_PEC_GET_CLASS(pec);
> +    PnvChip *chip = pec->chip;
> +    uint32_t pec_nest_base = pecc->xscom_nest_base(pec);
> +    uint32_t pec_pci_base = pecc->xscom_pci_base(pec);
> +
> +
> +    /* Populate the XSCOM address space. */
> +    pnv_xscom_add_subregion(chip,
> +                            pec_nest_base + 0x40 * (stack->stack_no + 1),
> +                            &stack->nest_regs_mr);
> +    pnv_xscom_add_subregion(chip,
> +                            pec_pci_base + 0x40 * (stack->stack_no + 1),
> +                            &stack->pci_regs_mr);
> +    pnv_xscom_add_subregion(chip,
> +                            pec_pci_base + PNV9_XSCOM_PEC_PCI_STK0 +
> +                            0x40 * stack->stack_no,
> +                            &stack->phb_regs_mr);
> +}
> +
>   /*
>    * Return the index/phb-id of a PHB4 that belongs to a
>    * pec->stacks[stack_index] stack.
> diff --git a/hw/pci-host/pnv_phb4_pec.c b/hw/pci-host/pnv_phb4_pec.c
> index 638691783b..41c79d24c4 100644
> --- a/hw/pci-host/pnv_phb4_pec.c
> +++ b/hw/pci-host/pnv_phb4_pec.c
> @@ -556,10 +556,6 @@ static void pnv_pec_stk_realize(DeviceState *dev, Error 
> **errp)
>   {
>       PnvPhb4PecStack *stack = PNV_PHB4_PEC_STACK(dev);
>       PnvPhb4PecState *pec = stack->pec;
> -    PnvPhb4PecClass *pecc = PNV_PHB4_PEC_GET_CLASS(pec);
> -    PnvChip *chip = pec->chip;
> -    uint32_t pec_nest_base;
> -    uint32_t pec_pci_base;
>       char name[64];
>   
>       assert(pec);
> @@ -583,20 +579,7 @@ static void pnv_pec_stk_realize(DeviceState *dev, Error 
> **errp)
>           return;
>       }
>   
> -    pec_nest_base = pecc->xscom_nest_base(pec);
> -    pec_pci_base = pecc->xscom_pci_base(pec);
> -
> -    /* Populate the XSCOM address space. */
> -    pnv_xscom_add_subregion(chip,
> -                            pec_nest_base + 0x40 * (stack->stack_no + 1),
> -                            &stack->nest_regs_mr);
> -    pnv_xscom_add_subregion(chip,
> -                            pec_pci_base + 0x40 * (stack->stack_no + 1),
> -                            &stack->pci_regs_mr);
> -    pnv_xscom_add_subregion(chip,
> -                            pec_pci_base + PNV9_XSCOM_PEC_PCI_STK0 +
> -                            0x40 * stack->stack_no,
> -                            &stack->phb_regs_mr);
> +    pnv_pec_init_stack_xscom(stack);
>   }
>   
>   static Property pnv_pec_stk_properties[] = {
> diff --git a/include/hw/pci-host/pnv_phb4.h b/include/hw/pci-host/pnv_phb4.h
> index 2fb5e119c4..610580a88f 100644
> --- a/include/hw/pci-host/pnv_phb4.h
> +++ b/include/hw/pci-host/pnv_phb4.h
> @@ -132,6 +132,7 @@ struct PnvPHB4 {
>   
>   void pnv_phb4_pic_print_info(PnvPHB4 *phb, Monitor *mon);
>   void pnv_phb4_update_regions(PnvPhb4PecStack *stack);
> +void pnv_pec_init_stack_xscom(PnvPhb4PecStack *stack);
>   void pnv_phb4_set_stack_phb_props(PnvPhb4PecStack *stack, PnvPHB4 *phb);
>   int pnv_phb4_pec_get_phb_id(PnvPhb4PecState *pec, int stack_index);
>   extern const MemoryRegionOps pnv_phb4_xscom_ops;