On 13/06/2022 17:44, Daniel Henrique Barboza wrote:
> There's nothing special that is being done in
> pnv_chip_power8_instance_init() that can't be done during
> pnv_chip_power8_realize(). Move the PHB creating and phbs[] assignment
> to power8_realize().
>
> We also need to assign a proper phb->chip parent and bus. This is done
> by the PHB itself, in pnv_phb3_realize(), in a similar fashion that user
> created PHB3s are going to do.
>
> After all this we're left with logic that, aside from phb chip
> assignment that are still being done in power8_realize(), behaves the
> same for default and user created PHB3s.
>
> Signed-off-by: Daniel Henrique Barboza <danielhb@linux.ibm.com>
> ---
> hw/pci-host/pnv_phb3.c | 14 ++++++++++++++
> hw/ppc/pnv.c | 24 +++++-------------------
> 2 files changed, 19 insertions(+), 19 deletions(-)
>
> diff --git a/hw/pci-host/pnv_phb3.c b/hw/pci-host/pnv_phb3.c
> index bda23fd20b..c1c73fb88d 100644
> --- a/hw/pci-host/pnv_phb3.c
> +++ b/hw/pci-host/pnv_phb3.c
> @@ -998,6 +998,20 @@ static void pnv_phb3_realize(DeviceState *dev, Error **errp)
> return;
> }
>
> + /*
> + * We need the chip to parent the PHB to allow the DT
> + * to build correctly (via pnv_xscom_dt()).
> + *
> + * TODO: the PHB should be parented by a PHB3 PEC device.
> + */
> + pnv_parent_qom_fixup(OBJECT(phb->chip), OBJECT(phb), phb->phb_id);
> +
Wouldn't we get the same result in a cleaner way by adding the phb as a
child of the chip in pnv_chip_power8_realize() ? Right next to when the
PnvPHB3 object pointer is added to the chip8->phbs array
Fred
> + /*
> + * pnv-phb3 buses are child of the main-system-bus, same as
> + * the chip.
> + */
> + pnv_parent_bus_fixup(DEVICE(phb->chip), dev);
> +
> /* LSI sources */
> object_property_set_link(OBJECT(&phb->lsis), "xics", OBJECT(pnv),
> &error_abort);
> diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c
> index d77c90d64a..e4080a98e1 100644
> --- a/hw/ppc/pnv.c
> +++ b/hw/ppc/pnv.c
> @@ -1130,8 +1130,6 @@ static void pnv_chip_power10_intc_print_info(PnvChip *chip, PowerPCCPU *cpu,
> static void pnv_chip_power8_instance_init(Object *obj)
> {
> Pnv8Chip *chip8 = PNV8_CHIP(obj);
> - PnvChipClass *pcc = PNV_CHIP_GET_CLASS(obj);
> - int i;
>
> object_property_add_link(obj, "xics", TYPE_XICS_FABRIC,
> (Object **)&chip8->xics,
> @@ -1145,22 +1143,6 @@ static void pnv_chip_power8_instance_init(Object *obj)
> object_initialize_child(obj, "occ", &chip8->occ, TYPE_PNV8_OCC);
>
> object_initialize_child(obj, "homer", &chip8->homer, TYPE_PNV8_HOMER);
> -
> - chip8->num_phbs = pcc->num_phbs;
> -
> - for (i = 0; i < chip8->num_phbs; i++) {
> - PnvPHB3 *phb3 = PNV_PHB3(object_new(TYPE_PNV_PHB3));
> -
> - /*
> - * We need the chip to parent the PHB to allow the DT
> - * to build correctly (via pnv_xscom_dt()).
> - *
> - * TODO: the PHB should be parented by a PEC device.
> - */
> - object_property_add_child(obj, "phb[*]", OBJECT(phb3));
> - chip8->phbs[i] = phb3;
> - }
> -
> }
>
> static void pnv_chip_icp_realize(Pnv8Chip *chip8, Error **errp)
> @@ -1286,8 +1268,12 @@ static void pnv_chip_power8_realize(DeviceState *dev, Error **errp)
> &chip8->homer.regs);
>
> /* PHB3 controllers */
> + chip8->num_phbs = pcc->num_phbs;
> +
> for (i = 0; i < chip8->num_phbs; i++) {
> - PnvPHB3 *phb = chip8->phbs[i];
> + PnvPHB3 *phb = PNV_PHB3(object_new(TYPE_PNV_PHB3));
> +
> + chip8->phbs[i] = phb;
>
> object_property_set_int(OBJECT(phb), "index", i, &error_fatal);
> object_property_set_int(OBJECT(phb), "chip-id", chip->chip_id,