Re: [PATCH v2 08/16] ppc/pnv: user created pnv-phb for powernv9

[Frederic Barrat <fbarrat@linux.ibm.com>]

On 03/06/2022 23:00, Daniel Henrique Barboza wrote:
>>>   static void pnv_phb4_realize(DeviceState *dev, Error **errp)
>>>   {
>>>       PnvPHB4 *phb = PNV_PHB4(dev);
>>> +    PnvMachineState *pnv = PNV_MACHINE(qdev_get_machine());
>>> +    PnvChip *chip = pnv_get_chip(pnv, phb->chip_id);
>>>       XiveSource *xsrc = &phb->xsrc;
>>> +    BusState *s;
>>> +    Error *local_err = NULL;
>>>       int nr_irqs;
>>>       char name[32];
>>> +    if (!chip) {
>>> +        error_setg(errp, "invalid chip id: %d", phb->chip_id);
>>> +        return;
>>> +    }
>>> +
>>> +    /* User created PHBs need to be assigned to a PEC */
>>> +    if (!phb->pec) {
>>> +        phb->pec = pnv_phb4_get_pec(chip, phb, &local_err);
>>> +        if (local_err) {
>>> +            error_propagate(errp, local_err);
>>> +            return;
>>> +        }
>>> +    }
>>> +
>>> +    /* Reparent the PHB to the chip to build the device tree */
>>> +    pnv_chip_parent_fixup(chip, OBJECT(phb->phb_base), phb->phb_id);
>>
>>
>> Didn't you mean to do that only for the user-created case? 
> 
> It's done for both because the default generated PHBs are being created 
> loosely
> from the chip starting on 3f4c369ea63e ("ppc/pnv: make PECs create and 
> realize
> PHB4s"). We had to amend the code after that commit to fix the QOM 
> hierarchy
> of these devices. 6e7b96750359 ("ppc/pnv: fix default PHB4 QOM 
> hierarchy") has
> more details.
> 
> 
>> And why not attaching the PHB to the PEC instead of the chip, like in 
>> pnv_pec_default_phb_realize() ? I think it makes more sense to keep 
>> the chip->PEC->PHB parenting in the qom tree (and incidentally, that's 
>> where I'd prefer to have the phb3 model move to).
> 
> I can only speculate since I didn't participate in the initial design. My
> educated guess is that we didn't want to show PECs in qtree, hence we
> made the PHB a child of the chip instead. I'll also guess that this can be
> due to how PHB3 worked and we just copied the existing design.


I don't believe that's correct though. As Cedric replied, the PECs show 
up in the qom tree on P9/P10, with chip->PEC->PHB, in that order. And I 
think that's fine, that's the model we should tend to (and which would 
require a fix on P8, since there we have chip->phb->pbcq, which is 
backwards. The pbcq object is the equivalent of the PEC).

So I think we should keep the qom relationship as it currently is on 
P9/P10, since it looks good. On P8, we can keep the current status for 
now and, as discussed privately, have another series later to clean 
things up.

   Fred