Re: [Xen-devel] [PATCH 3/5] xen/arm: Don't reinject the IRQ if it's already in LRs

[Ian Campbell <Ian.Campbell@xxxxxxxxxx>]

On Tue, 2013-06-25 at 18:05 +0100, Stefano Stabellini wrote:
> On Tue, 25 Jun 2013, Ian Campbell wrote:
> > On Tue, 2013-06-25 at 17:36 +0100, Stefano Stabellini wrote:
> > > I think it's time we introduce a "status" member in struct irq_desc, so
> > > that we are not dependent on the information in the GICH_LR registers or
> > > the queue a pending_irq has been added to.
> > 
> > Yes please, I find this one of the hardest things to keep straight in my
> > head (not helped by my inability to remember which of pending and
> > inflight is which...)
> > 
> > > I would implement it as a bitfield:
> > > 
> > > int status;
> > > #define GIC_IRQ_ENABLED  (1<<0)
> > > #define GIC_IRQ_INFLIGHT (1<<1)
> > > #define GIC_IRQ_INLR     (1<<2)
> > > 
> > > This way you should just go through the inflight queue and check whether
> > > status & GIC_IRQ_INLR.
> > 
> > Since some of this stuff happens in interrupt context you probably want
> > test_bit/set_bit et al rather than regular boolean logic, don't you?
> > 
> > > At the moment we just want to represent this basic state machine:
> > > 
> > > irq guest asserted -> inflight -> injected (inlr) -> unasserted 
> > > (maintenance interrupt)
> > 
> > Can we model the states after the active/pending states which the gic
> > has? It might make a bunch of stuff clearer?
> 
> It might be worth storing those too.
> So maybe:
> 
> #define GIC_IRQ_ENABLED           (1<<0)
> #define GIC_IRQ_PENDING           (1<<1)
> #define GIC_IRQ_ACTIVE            (1<<2)
> #define GIC_IRQ_GUEST_INFLIGHT    (1<<3)
> #define GIC_IRQ_GUEST_INLR        (1<<4)
> 
> however if we do store the physical gic states (GIC_IRQ_PENDING and
> GIC_IRQ_ACTIVE) then maybe the right place for them is actually irq_desc
> rather than irq_pending that is used just for guest irqs.

I was thinking of these as states of the emulated interrupts, rather
than any underlying physical interrupts, so I think irq_pending is
correct?

It occurs to me that at least some of these bits are also fields in the
LR. I think it is good to save them separately (reducing the
intertwining of our interrupt handling from GIC internals is a good
thing) but it means you will need to take care of syncing the state
between the LR and our internal state at various points, since the LR
can change based on the guest EOI etc.

Ian.


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel