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

[Ganesh G R <ganeshgr@linux.ibm.com>]

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.
>