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

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

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


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