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

["Cédric Le Goater" <clg@redhat.com>]

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 ?

Thanks,

C.