Re: Access to remote XIVE2 units (was Re: [PATCH v9 0/7] Power11 support for QEMU [PowerNV])

[Miles Glenn <milesg@linux.ibm.com>]

+ Mike Kowal

On Sun, 2025-08-10 at 14:46 +0200, Cédric Le Goater wrote:
> + Glenn
> + Gautam
> 
> On 8/10/25 12:45, Aditya Gupta wrote:
> > On 25/08/10 12:26PM, Aditya Gupta wrote:
> > > > <...snip...>
> > > 
> > > About the error, seems xive2 always expecting powernv10 chip, I will
> > > have to rethink how should I use the same xive2 for powernv11.
> > > 
> > 
> > There's a type cast to Pnv10Chip in 'pnv_xive2_get_remote'.
> > The cast is only temporarily used to get the 'PnvXive2' object in the
> > Pnv10Chip.
> > It's the only place in hw/intc/pnv_xive2.c assuming Pnv10Chip object.
> > 
> > Thinking to have a helper function to just return the 'PnvXive2' object
> > inside the chip, given a 'PnvChip'.
> > 
> > Or the below change where we are adding another pointer in PnvChipClass:
> > 
> >      diff --git a/hw/intc/pnv_xive2.c b/hw/intc/pnv_xive2.c
> >      index e019cad5c14c..9832be5fd297 100644
> >      --- a/hw/intc/pnv_xive2.c
> >      +++ b/hw/intc/pnv_xive2.c
> >      @@ -110,8 +110,8 @@ static PnvXive2 *pnv_xive2_get_remote(uint32_t vsd_type, hwaddr fwd_addr)
> >           int i;
> >       
> >           for (i = 0; i < pnv->num_chips; i++) {
> >      -        Pnv10Chip *chip10 = PNV10_CHIP(pnv->chips[i]);
> >      -        PnvXive2 *xive = &chip10->xive;
> >      +        PnvChipClass *k = PNV_CHIP_GET_CLASS(pnv->chips[i]);
> >      +        PnvXive2 *xive = k->intc_get(pnv->chips[i]);
> >       
> >               /*
> >                * Is this the XIVE matching the forwarded VSD address is for this
> > 
> > Which one do you suggest ? Or should I look for another way ?
> > 
> > I am preferring the first way to have a helper, but both ways look hacky.
> 
> Any call to qdev_get_machine() in device model is at best
> a modeling shortcut, most likely it is a hack :
> 
>    /*
>     * Remote access to INT controllers. HW uses MMIOs(?). For now, a simple
>     * scan of all the chips INT controller is good enough.
>     */
> 
> So all these calls are suspicious :
> 
>    $ git grep qdev_get_machine hw/*/*pnv*
>    hw/intc/pnv_xive2.c:    PnvMachineState *pnv = PNV_MACHINE(qdev_get_machine());
>    hw/pci-host/pnv_phb.c:    PnvMachineState *pnv = PNV_MACHINE(qdev_get_machine());
>    hw/pci-host/pnv_phb3.c:    PnvMachineState *pnv = PNV_MACHINE(qdev_get_machine());
>    hw/ppc/pnv.c:    PnvMachineState *pnv = PNV_MACHINE(qdev_get_machine());
>    hw/ppc/pnv.c:    PnvMachineState *pnv = PNV_MACHINE(qdev_get_machine());
>    hw/ppc/pnv_chiptod.c:    PnvMachineState *pnv = PNV_MACHINE(qdev_get_machine());
>    hw/ppc/pnv_chiptod.c:    PnvMachineState *pnv = PNV_MACHINE(qdev_get_machine());
>    hw/ppc/pnv_lpc.c:    PnvMachineState *pnv = PNV_MACHINE(qdev_get_machine());
> 
> In the particular case of XIVE2, the solution is to rework
> pnv_xive2_get_remote() like it was for P9. See b68147b7a5bf
> ("ppc/xive: Add support for the PC MMIOs").
> 
> 
> Thanks,
> 
> C.
> 
> 
> 
> 
> 
> 
>