Re: [PATCH 10/13] pnv/xive: Add special handling for pool targets

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

[-- Attachment #1: Type: text/plain, Size: 6422 bytes --]


On 8/29/2024 7:14 AM, Cédric Le Goater wrote:
> On 8/1/24 22:30, Michael Kowal wrote:
>> From: Glenn Miles <milesg@linux.vnet.ibm.com>
>>
>> Hypervisor "pool" targets do not get their own interrupt line and 
>> instead
>> must share an interrupt line with the hypervisor "physical" targets.
>> This also means that the pool ring must use some of the registers 
>> from the
>> physical ring in the TIMA.  Specifically, the NSR, PIPR and CPPR 
>> registers:
>>
>>    NSR = Notification Source Register
>>    PIPR = Post Interrupt Priority Register
>>    CPPR = Current Processor Priority Register
>>
>> The NSR specifies that there is an active interrupt.  The CPPR
>> specifies the priority of the context and the PIPR specifies the
>> priority of the interrupt.  For an interrupt to be presented to
>> a context, the priority of the interrupt must be higher than the
>> priority of the context it is interrupting (value must be lower).
>>
>> The existing code was not aware of the sharing of these registers.
>> This commit adds that support.
>>
>> Signed-off-by: Glenn Miles <milesg@linux.vnet.ibm.com>
>> Signed-off-by: Michael Kowal <kowal@linux.ibm.com>
>> ---
>>   hw/intc/xive.c | 36 ++++++++++++++++++++++++++----------
>>   1 file changed, 26 insertions(+), 10 deletions(-)
>>
>> diff --git a/hw/intc/xive.c b/hw/intc/xive.c
>> index 9d85da0999..5c4ca7f6e0 100644
>> --- a/hw/intc/xive.c
>> +++ b/hw/intc/xive.c
>> @@ -67,25 +67,35 @@ static qemu_irq xive_tctx_output(XiveTCTX *tctx, 
>> uint8_t ring)
>>   static uint64_t xive_tctx_accept(XiveTCTX *tctx, uint8_t ring)
>>   {
>>       uint8_t *regs = &tctx->regs[ring];
>> -    uint8_t nsr = regs[TM_NSR];
>> +    uint64_t nsr = regs[TM_NSR];
>
> why ?


I asked Glenn the same question.  I think was worried about overflow on 
the return statement below.  I can find an alternative.

return (nsr << 8) | regs[TM_CPPR];

>
>>       uint8_t mask = exception_mask(ring);
>>         qemu_irq_lower(xive_tctx_output(tctx, ring));
>>         if (regs[TM_NSR] & mask) {
>>           uint8_t cppr = regs[TM_PIPR];
>> +        uint8_t alt_ring;
>> +        uint8_t *aregs;
>
> I would prefer keeping the same prefix :
>
>         uint8_t *alt_regs;
>
>
> Thanks,
>
> C.
>
>
>> +
>> +        /* POOL interrupt uses IPB in QW2, POOL ring */
>> +        if ((ring == TM_QW3_HV_PHYS) && (nsr & (TM_QW3_NSR_HE_POOL 
>> << 6))) {
>> +            alt_ring = TM_QW2_HV_POOL;
>> +        } else {
>> +            alt_ring = ring;
>> +        }
>> +        aregs = &tctx->regs[alt_ring];
>>             regs[TM_CPPR] = cppr;
>>             /* Reset the pending buffer bit */
>> -        regs[TM_IPB] &= ~xive_priority_to_ipb(cppr);
>> -        regs[TM_PIPR] = ipb_to_pipr(regs[TM_IPB]);
>> +        aregs[TM_IPB] &= ~xive_priority_to_ipb(cppr);
>> +        regs[TM_PIPR] = ipb_to_pipr(aregs[TM_IPB]);
>>             /* Drop Exception bit */
>>           regs[TM_NSR] &= ~mask;
>>   -        trace_xive_tctx_accept(tctx->cs->cpu_index, ring,
>> -                               regs[TM_IPB], regs[TM_PIPR],
>> +        trace_xive_tctx_accept(tctx->cs->cpu_index, alt_ring,
>> +                               aregs[TM_IPB], regs[TM_PIPR],
>>                                  regs[TM_CPPR], regs[TM_NSR]);
>>       }
>>   @@ -94,13 +104,19 @@ static uint64_t xive_tctx_accept(XiveTCTX 
>> *tctx, uint8_t ring)
>>     static void xive_tctx_notify(XiveTCTX *tctx, uint8_t ring)
>>   {
>> +    /* HV_POOL ring uses HV_PHYS NSR, CPPR and PIPR registers */
>> +    uint8_t alt_ring = (ring == TM_QW2_HV_POOL) ? TM_QW3_HV_PHYS : 
>> ring;
>> +    uint8_t *aregs = &tctx->regs[alt_ring];
>>       uint8_t *regs = &tctx->regs[ring];
>>   -    if (regs[TM_PIPR] < regs[TM_CPPR]) {
>> +    if (aregs[TM_PIPR] < aregs[TM_CPPR]) {
>>           switch (ring) {
>>           case TM_QW1_OS:
>>               regs[TM_NSR] |= TM_QW1_NSR_EO;
>>               break;
>> +        case TM_QW2_HV_POOL:
>> +            aregs[TM_NSR] = (TM_QW3_NSR_HE_POOL << 6);
>> +            break;
>>           case TM_QW3_HV_PHYS:
>>               regs[TM_NSR] |= (TM_QW3_NSR_HE_PHYS << 6);
>>               break;
>> @@ -108,8 +124,8 @@ static void xive_tctx_notify(XiveTCTX *tctx, 
>> uint8_t ring)
>>               g_assert_not_reached();
>>           }
>>           trace_xive_tctx_notify(tctx->cs->cpu_index, ring,
>> -                               regs[TM_IPB], regs[TM_PIPR],
>> -                               regs[TM_CPPR], regs[TM_NSR]);
>> +                               regs[TM_IPB], aregs[TM_PIPR],
>> +                               aregs[TM_CPPR], aregs[TM_NSR]);
>>           qemu_irq_raise(xive_tctx_output(tctx, ring));
>>       }
>>   }
>> @@ -217,14 +233,14 @@ static uint64_t xive_tm_vt_poll(XivePresenter 
>> *xptr, XiveTCTX *tctx,
>>   static const uint8_t xive_tm_hw_view[] = {
>>       3, 0, 0, 0,   0, 0, 0, 0,   3, 3, 3, 3,   0, 0, 0, 0, /* QW-0 
>> User */
>>       3, 3, 3, 3,   3, 3, 0, 2,   3, 3, 3, 3,   0, 0, 0, 0, /* QW-1 
>> OS   */
>> -    0, 0, 3, 3,   0, 0, 0, 0,   3, 3, 3, 3,   0, 0, 0, 0, /* QW-2 
>> POOL */
>> +    0, 0, 3, 3,   0, 3, 3, 0,   3, 3, 3, 3,   0, 0, 0, 0, /* QW-2 
>> POOL */
>>       3, 3, 3, 3,   0, 3, 0, 2,   3, 0, 0, 3,   3, 3, 3, 0, /* QW-3 
>> PHYS */
>>   };
>>     static const uint8_t xive_tm_hv_view[] = {
>>       3, 0, 0, 0,   0, 0, 0, 0,   3, 3, 3, 3,   0, 0, 0, 0, /* QW-0 
>> User */
>>       3, 3, 3, 3,   3, 3, 0, 2,   3, 3, 3, 3,   0, 0, 0, 0, /* QW-1 
>> OS   */
>> -    0, 0, 3, 3,   0, 0, 0, 0,   0, 3, 3, 3,   0, 0, 0, 0, /* QW-2 
>> POOL */
>> +    0, 0, 3, 3,   0, 3, 3, 0,   0, 3, 3, 3,   0, 0, 0, 0, /* QW-2 
>> POOL */
>>       3, 3, 3, 3,   0, 3, 0, 2,   3, 0, 0, 3,   0, 0, 0, 0, /* QW-3 
>> PHYS */
>>   };
>

[-- Attachment #2: Type: text/html, Size: 9947 bytes --]