On 25/09/25 08:38AM, Cédric Le Goater wrote:
> On 9/24/25 14:14, Ganesh G R wrote:
> >
> > On 8/29/25 3:19 AM, Cédric Le Goater wrote:
> > > On 8/28/25 14:04, Aditya Gupta wrote:
> > > > + Ganesh
> > > >
> > > > On 25/08/10 02:46PM, 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").
> > > > >
> > > >
> > > > Hi Cedric,
> > > >
> > > > While I am working with XIVE engineers to get the pnv_xive2_get_remote()
> > > > reworked as suggested (since it's a bit more work due to multiple cases
> > > > of indirect/direct vst, nvg/nvc types in case of XIVE2), I would like
> > > > to propose below patch to have as an interim solution to unblock
> > > > the PowerNV11 support patches.
> > >
> > > pHyp is an unknown FW implementation for opensource. Until an image
> > > is released somewhere (please think about it), QEMU has nothing to
> > > worry about other than supporting OPAL.
> > >
> > > For now, let's forget about the grouping aspect of XIVE2, simply
> > > rework pnv_xive2_get_remote() as it was done in b68147b7a5bf for
> > > XIVE. This shouldn't take long. And, for the nvg/nvc types, report
> > > an error of some sort and add a TODO in the code.
> > >
> > A similar change cannot be done to XIVE2, because Fredric’s commit
> > (96a2132ce95) has introduced modifications to the NVPG and NVC MMIO
> > callbacks in order to support backlog counter operations.
>
> Thanks for looking.
>
> Indeed. So I guess Aditya's proposal adding a new PnvChipClass handler
> is then the best alternative :
>
> @@ -170,6 +170,7 @@ struct PnvChipClass {
> void (*intc_reset)(PnvChip *chip, PowerPCCPU *cpu);
> void (*intc_destroy)(PnvChip *chip, PowerPCCPU *cpu);
> void (*intc_print_info)(PnvChip *chip, PowerPCCPU *cpu, GString *buf);
> + void* (*intc_get)(PnvChip *chip);
> ISABus *(*isa_create)(PnvChip *chip, Error **errp);
> void (*dt_populate)(PnvChip *chip, void *fdt);
> void (*pic_print_info)(PnvChip *chip, GString *buf);
>
> Aditya,
>
> Could you please resend the whole powernv11 series including this
> new patch for xive2 ?
Sure, will post v10 today.
Thanks Cedric !
>
> Thanks,
>
> C.
>
>
>
