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
