Re: [PATCH 11/13] pnv/xive: Update PIPR when updating CPPR

[Mike Kowal <kowal@linux.ibm.com>]

On 8/30/2024 3:25 AM, Cédric Le Goater wrote:
> On 8/29/24 22:35, Mike Kowal wrote:
>>
>> On 8/29/2024 7:29 AM, Cédric Le Goater wrote:
>>> On 8/1/24 22:30, Michael Kowal wrote:
>>>> From: Glenn Miles <milesg@linux.vnet.ibm.com>
>>>>
>>>> Current code was updating the PIPR inside the xive_tctx_accept() 
>>>> function
>>>> instead of the xive_tctx_set_cppr function, which is where the HW 
>>>> would
>>>> have it updated.
>>>
>>> Did you confirm with the HW designer ?
>>>
>>> AFAIR, the PIPR is constructed from the IPB and the later is it updated
>>> the better. However, if now, both PIPR (HW and Pool) are required to
>>> identify the ctx to notify, I agree set_cppr() needs a change but what
>>> about xive_tctx_ipb_update() which is called when an interrupt
>>> needs a resend ?
>>
>>
>> This was fix to a bug and matches what  is specified in the XIVE2 
>> architecture document CPPR flows (MMIO CPPR xxx processing).
>
> ok. I was also wondering if this was fixing a bug. Do you think this
> is the correct commit id ?
>
>  cdd4de68edb6 ("ppc/xive: notify the CPU when the interrupt priority 
> is more privileged")
>
> If so, could you please add a Fixes tags ?
>
> Thanks,
>
> C.
>

Many of these parts have been changed multiple time for different 
things.   I am not sure which commit this fixes.  I am upstreaming other 
peoples work  that was done over the last couple of years so it hard to 
tell.  Also,  the original xive support was only complete enough to 
support Linux.   Much of this I would consider 'new development' 
expanding XIVE support for Power VM.   If you think it should still have 
a fixes-tag, I will add it.

MAK


>
>
>>
>>
>>>
>>>
>>> Thanks,
>>>
>>> C.
>>>
>>>
>>>
>>>> Moved the update to the xive_tctx_set_cppr function which required
>>>> additional support for pool interrupts.
>>>>
>>>> Signed-off-by: Glenn Miles <milesg@linux.vnet.ibm.com>
>>>> Signed-off-by: Michael Kowal <kowal@linux.ibm.com>
>>>> ---
>>>>   hw/intc/xive.c | 34 ++++++++++++++++++++++++++++++++--
>>>>   1 file changed, 32 insertions(+), 2 deletions(-)
>>>>
>>>> diff --git a/hw/intc/xive.c b/hw/intc/xive.c
>>>> index 5c4ca7f6e0..d951aac3a0 100644
>>>> --- a/hw/intc/xive.c
>>>> +++ b/hw/intc/xive.c
>>>> @@ -89,7 +89,6 @@ static uint64_t xive_tctx_accept(XiveTCTX *tctx, 
>>>> uint8_t ring)
>>>>             /* Reset the pending buffer bit */
>>>>           aregs[TM_IPB] &= ~xive_priority_to_ipb(cppr);
>>>> -        regs[TM_PIPR] = ipb_to_pipr(aregs[TM_IPB]);
>>>>             /* Drop Exception bit */
>>>>           regs[TM_NSR] &= ~mask;
>>>> @@ -143,6 +142,8 @@ void xive_tctx_reset_signal(XiveTCTX *tctx, 
>>>> uint8_t ring)
>>>>   static void xive_tctx_set_cppr(XiveTCTX *tctx, uint8_t ring, 
>>>> uint8_t cppr)
>>>>   {
>>>>       uint8_t *regs = &tctx->regs[ring];
>>>> +    uint8_t pipr_min;
>>>> +    uint8_t ring_min;
>>>>         trace_xive_tctx_set_cppr(tctx->cs->cpu_index, ring,
>>>>                                regs[TM_IPB], regs[TM_PIPR],
>>>> @@ -154,8 +155,37 @@ static void xive_tctx_set_cppr(XiveTCTX *tctx, 
>>>> uint8_t ring, uint8_t cppr)
>>>>         tctx->regs[ring + TM_CPPR] = cppr;
>>>>   +    /*
>>>> +     * Recompute the PIPR based on local pending interrupts. The PHYS
>>>> +     * ring must take the minimum of both the PHYS and POOL PIPR 
>>>> values.
>>>> +     */
>>>> +    pipr_min = ipb_to_pipr(regs[TM_IPB]);
>>>> +    ring_min = ring;
>>>> +
>>>> +    /* PHYS updates also depend on POOL values */
>>>> +    if (ring == TM_QW3_HV_PHYS) {
>>>> +        uint8_t *pregs = &tctx->regs[TM_QW2_HV_POOL];
>>>> +
>>>> +        /* POOL values only matter if POOL ctx is valid */
>>>> +        if (pregs[TM_WORD2] & 0x80) {
>>>> +
>>>> +            uint8_t pool_pipr = ipb_to_pipr(pregs[TM_IPB]);
>>>> +
>>>> +            /*
>>>> +             * Determine highest priority interrupt and
>>>> +             * remember which ring has it.
>>>> +             */
>>>> +            if (pool_pipr < pipr_min) {
>>>> +                pipr_min = pool_pipr;
>>>> +                ring_min = TM_QW2_HV_POOL;
>>>> +            }
>>>> +        }
>>>> +    }
>>>> +
>>>> +    regs[TM_PIPR] = pipr_min;
>>>> +
>>>>       /* CPPR has changed, check if we need to raise a pending 
>>>> exception */
>>>> -    xive_tctx_notify(tctx, ring);
>>>> +    xive_tctx_notify(tctx, ring_min);
>>>>   }
>>>>     void xive_tctx_ipb_update(XiveTCTX *tctx, uint8_t ring, uint8_t 
>>>> ipb)
>>>
>