Re: [PATCH v3 04/10] pnv_phb4_pec.c: remove stack 'phb-id' alias

[Cédric Le Goater]

On 1/10/22 15:33, Daniel Henrique Barboza wrote:
> This alias is a indirect way of setting stack->phb->index. Since we have
> access to a valid stack->phb (for default_enabled() at least - next
> patch will deal with it accordingly) we can directly set the phb 'index'
> attribute.
>
> Let's also take the opportunity to explain why we're having to deal with
> stack->phb attributes during pec_realize().
>
> Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
> ---
>   hw/pci-host/pnv_phb4_pec.c | 12 ++++++++++--
>   1 file changed, 10 insertions(+), 2 deletions(-)
>
> diff --git a/hw/pci-host/pnv_phb4_pec.c b/hw/pci-host/pnv_phb4_pec.c
> index 1f264d0a9c..417fac4cef 100644
> --- a/hw/pci-host/pnv_phb4_pec.c
> +++ b/hw/pci-host/pnv_phb4_pec.c
> @@ -395,8 +395,17 @@ static void pnv_pec_realize(DeviceState *dev, Error **errp)
>           int phb_id = pnv_phb4_pec_get_phb_id(pec, i);
>   
>           object_property_set_int(stk_obj, "stack-no", i, &error_abort);
> -        object_property_set_int(stk_obj, "phb-id", phb_id, &error_abort);
>           object_property_set_link(stk_obj, "pec", OBJECT(pec), &error_abort);
> +
> +        /*
> +         * stack->phb->index is dependent on the position the
> +         * stack occupies in pec->stacks[]. We have this information
> +         * available here via the 'i' iterator so it's convenient to
> +         * do it now.
> +         */
> +        object_property_set_int(OBJECT(&stack->phb), "index", phb_id,
> +                                &error_abort);

I don't like the fact that we are exposing ->phb under the PEC model.
It looks like this is going to be a problem afterwards when defaults
are disabled.

We should move the setting of the PHB ID under pnv_pec_stk_realize()
before the PHB is realized and compute the id with :

       int phb_id = pnv_phb4_pec_get_phb_id(pec, stack->stack_no);

Thanks,

C.

> +
>           if (!qdev_realize(DEVICE(stk_obj), NULL, errp)) {
>               return;
>           }
> @@ -534,7 +543,6 @@ static void pnv_pec_stk_instance_init(Object *obj)
>       PnvPhb4PecStack *stack = PNV_PHB4_PEC_STACK(obj);
>   
>       object_initialize_child(obj, "phb", &stack->phb, TYPE_PNV_PHB4);
> -    object_property_add_alias(obj, "phb-id", OBJECT(&stack->phb), "index");
>   }
>   
>   static void pnv_pec_stk_realize(DeviceState *dev, Error **errp)