Re: [Qemu-devel] [RFC PATCH v2 13/21] ppc/xive: handle interrupt acknowledgment by the O/S

[Cédric Le Goater]

On 09/19/2017 09:53 AM, David Gibson wrote:
> On Mon, Sep 11, 2017 at 07:12:27PM +0200, Cédric Le Goater wrote:
>> When an O/S Exception is raised, the O/S acknowledges the interrupt
>> with a special read in the TIMA. If the EO bit of the Notification
>> Source Register (NSR) is set (and it should), the Current Processor
>> Priority Register (CPPR) takes the value of the Pending Interrupt
>> Priority Register (PIPR), which contains the priority of the most
>> favored pending notification. The bit number corresponding to the
>> priority of the pending interrupt is reseted in the Interrupt Pending
>> Buffer (IPB) and so is the EO bit of the NSR.
>>
>> Signed-off-by: Cédric Le Goater <address@hidden>
>> ---
>>  hw/intc/spapr_xive.c | 19 ++++++++++++++++++-
>>  1 file changed, 18 insertions(+), 1 deletion(-)
>>
>> diff --git a/hw/intc/spapr_xive.c b/hw/intc/spapr_xive.c
>> index e5d4b723b7e0..ad3ff91b13ea 100644
>> --- a/hw/intc/spapr_xive.c
>> +++ b/hw/intc/spapr_xive.c
>> @@ -50,7 +50,24 @@ static uint8_t ipb_to_pipr(uint8_t ibp)
>>  
>>  static uint64_t spapr_xive_icp_accept(ICPState *icp)
>>  {
>> -    return 0;
>> +    uint8_t nsr = icp->tima_os[TM_NSR];
>> +
>> +    qemu_irq_lower(icp->output);
> 
> Ah, here's the lower.  This should not be in a different patch from
> the matching raise.  

ok I can merge these.

> Plus, this doesn't seem right.  Shouldn't this
> recheck the CPPR against the PIPR, in case a higher priority irq has
> been delivered since the one the cpu is acking.

If a higher priority is delivered, it means that the CPPR was more 
privileged and that we have now two bits set in the IPB by the time 
the interrupt is acked. The high priority PIPR will become the new 
CPPR and the IBP will be modified keeping only the lower priority. 

if the CPPR is modified to the lower priority level, then the 
first interrupt will be delivered again. 

I think this is fine.

C.


> 
>> +    if (icp->tima_os[TM_NSR] & TM_QW1_NSR_EO) {
>> +        uint8_t cppr = icp->tima_os[TM_PIPR];
>> +
>> +        icp->tima_os[TM_CPPR] = cppr;
>> +
>> +        /* Reset the pending buffer bit */
>> +        icp->tima_os[TM_IPB] &= ~priority_to_ipb(cppr);
>> +        icp->tima_os[TM_PIPR] = ipb_to_pipr(icp->tima_os[TM_IPB]);
>> +
>> +        /* Drop Exception bit for OS */
>> +        icp->tima_os[TM_NSR] &= ~TM_QW1_NSR_EO;
>> +    }
>> +
>> +    return (nsr << 8) | icp->tima_os[TM_CPPR];
>>  }
>>  
>>  static void spapr_xive_icp_notify(ICPState *icp)
>