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.

Please let me know if it looks good to you.

It's a hack. So please try the above first.


Thanks,

C.



Reply via email to