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

[Julien Grall <julien.grall@xxxxxxx>]

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@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel