Re: [PATCH 04/14] ppc/pnv: Introduce support for user created PHB3 devices

[Cédric Le Goater]

On 12/7/21 10:47, Frederic Barrat wrote:
> On 02/12/2021 15:42, Cédric Le Goater wrote:
> > PHB3 devices and PCI devices can now be added to the powernv8 machine
> > using :
> >
> >    -device pnv-phb3,chip-id=0,index=1 \
> >    -device nec-usb-xhci,bus=pci.1,addr=0x0
> >
> > The 'index' property identifies the PHB3 in the chip. In case of user
> > created devices, a lookup on 'chip-id' is required to assign the
> > owning chip.
> >
> > Signed-off-by: Cédric Le Goater <clg@kaod.org>
> > ---
>
> > diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c
> > index de277c457838..d7fe92cb082d 100644
> > --- a/hw/ppc/pnv.c
> > +++ b/hw/ppc/pnv.c
> > @@ -1097,14 +1097,14 @@ static void pnv_chip_power8_instance_init(Object *obj)
> >       object_initialize_child(obj, "homer", &chip8->homer, TYPE_PNV8_HOMER);
> > -    for (i = 0; i < pcc->num_phbs; i++) {
> > +    if (defaults_enabled()) {
> > +        chip->num_phbs = pcc->num_phbs;
> > +    }
> > +
> > +    for (i = 0; i < chip->num_phbs; i++) {
> >           object_initialize_child(obj, "phb[*]", &chip8->phbs[i], 
> > TYPE_PNV_PHB3);
> >       }
> > -    /*
> > -     * Number of PHBs is the chip default
> > -     */
> > -    chip->num_phbs = pcc->num_phbs;
> >   }
>
>
> So if "-nodefaults" is mentioned on the command line, then chip->num_phbs is not 
> set. It seems the intention is to have only the PHBs defined on the CLI, which is fine. 
> However, I don't see where chip->num_phbs is incremented in that case.

Good catch :) That's why we need another patch fixing all this because
it is breaking the XICS fabric handlers, ics_get and ics_resend.
'info pic' is impacted also.

Here is the proposed fix for v2 :

 
https://github.com/legoater/qemu/commit/b47bce3109f316a65aa2fa2a46651b2960e93fca

I chose to loop on the children of the chip to find the user
created devices and leave the PnvChip model with empty defaults.


'info pic' is impacted the same on P9

  
https://github.com/legoater/qemu/commit/d4733edca94c95f717f4ee35bbea6dc085365286

Thanks,

C.