Re: [PATCH 1/4] pnv_phb4_pec: Keep track of instantiated PHBs

[Philippe Mathieu-Daudé]

Hi Frederic,

On 2/3/23 17:37, Frederic Barrat wrote:
> Add an array on the PEC object to keep track of the PHBs which are
> instantiated. The array can be sparsely populated when using
> user-created PHBs. It will be useful for the next patch to only export
> instantiated PHBs in the device tree.
>
> Signed-off-by: Frederic Barrat <fbarrat@linux.ibm.com>
> ---
>   hw/pci-host/pnv_phb4_pec.c     | 12 +++++++-----
>   hw/ppc/pnv.c                   |  1 +
>   include/hw/pci-host/pnv_phb4.h |  2 ++
>   3 files changed, 10 insertions(+), 5 deletions(-)
>
> diff --git a/hw/pci-host/pnv_phb4_pec.c b/hw/pci-host/pnv_phb4_pec.c
> index 43267a428f..97c06bb0a0 100644
> --- a/hw/pci-host/pnv_phb4_pec.c
> +++ b/hw/pci-host/pnv_phb4_pec.c

> +static PnvPHB *pnv_pec_default_phb_realize(PnvPhb4PecState *pec,
> +                                           int stack_no,
> +                                           Error **errp)
>   {
>       PnvPHB *phb = PNV_PHB(qdev_new(TYPE_PNV_PHB));
>       int phb_id = pnv_phb4_pec_get_phb_id(pec, stack_no);
> @@ -128,8 +128,9 @@ static void pnv_pec_default_phb_realize(PnvPhb4PecState 
> *pec,
>                               &error_fatal);
>   
>       if (!sysbus_realize(SYS_BUS_DEVICE(phb), errp)) {
> -        return;
> +        return NULL;
>       }
> +    return phb;
>   }


> diff --git a/include/hw/pci-host/pnv_phb4.h b/include/hw/pci-host/pnv_phb4.h
> index 28d61b96c7..0b72ef1471 100644
> --- a/include/hw/pci-host/pnv_phb4.h
> +++ b/include/hw/pci-host/pnv_phb4.h
> @@ -185,6 +185,8 @@ struct PnvPhb4PecState {
>   
>       /* PHBs */
>       uint32_t num_phbs;
> +#define MAX_PHBS_PER_PEC        3
> +    PnvPHB *phbs[MAX_PHBS_PER_PEC];
>   
>       PnvChip *chip;
>   };

From QOM PoV, better would be to 'embed' the PnvPHB structure (not only
a pointer to it), and initialize the PnvPHB instance calling
object_initialize_child() instead of qdev_new().

See for example the recent conversion of OHCISysBusState in commit
01c400ae43 ("hw/display/sm501: Embed OHCI QOM child in chipset").

Regards,

Phil.