Re: [PATCH v2 09/45] ARM: GIC: Allow tweaking the active and pending state of an IRQ

[Julien Grall <julien.grall@arm.com>]

On 03/16/2018 04:05 PM, Andre Przywara wrote:
> Hi,

Hi Andre,

> On 15/03/18 20:30, Andre Przywara wrote:
>> +    }
>> +}
>> +
>> +static void gicv2_set_pending_state(struct irq_desc *irqd, bool pending)
>> +{
>> +    ASSERT(spin_is_locked(&irqd->lock));
>> +
>> +    if ( pending )
>> +    {
>> +        /* The INPROGRESS bit will be set when the interrupt fires. */
>> +        gicv2_poke_irq(irqd, GICD_ISPENDR);
>> +    }
>> +    else
>> +    {
>> +        gicv2_poke_irq(irqd, GICD_ICPENDR);
>> +        if ( !gicv2_peek_irq(irqd, GICD_ISACTIVER) &&
>> +             test_bit(_IRQ_GUEST, &irqd->status) )
>> +            clear_bit(_IRQ_INPROGRESS, &irqd->status);
> 
> We should not need to touch the _IRQ_INPROGRESS bit here. That bit
> really shadows the *active* bit, so changing the pending state should
> not matter here:
> - If the h/w IRQ is active, the bit is set already and should remain so,
> as Xen and the guest are still dealing with it. Clearing the h/w pending
> state does not change that.
> - If the h/w IRQ is not active, the _IRQ_INPROGRESS bit is not set, so
> clearing it would be a NOP.
> So we can remove  the _IRQ_INPROGRESS handling here completely.
> 
> I will amend the code accordingly, including the respective GICv3 parts.

Thank you for summarizing our discussion. Is it still making sense to 
document how those helpers should be called?

Cheers,

-- 
Julien Grall

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel