Re: [Xen-devel] [PATCH v5 08/10] xen/arm: second irq injection while the first irq is still inflight

[Ian Campbell <Ian.Campbell@xxxxxxxxxx>]

On Wed, 2014-04-02 at 16:31 +0100, Stefano Stabellini wrote:
> On Tue, 1 Apr 2014, Ian Campbell wrote:
> > On Mon, 2014-03-24 at 18:49 +0000, Stefano Stabellini wrote:
> > > Set GICH_LR_PENDING in the corresponding GICH_LR to inject a second irq
> > > while the first one is still active.
> > > If the first irq is already pending (not active), just clear
> > > GIC_IRQ_GUEST_PENDING because the irq has already been injected and is
> > > already visible by the guest.
> > 
> > I'm confused by this.
> > 
> > If the interrupt is pending but not active then the guest has yet to
> > read GICC_IAR, so while it might be "visible" to the guest it has not
> > been observed by the guest.
>  
> Yes, it is visible to the guest VM but not yet been observed by the
> guest operating system.
> 
> 
> > The comment says:
> > 
> >      * GIC_IRQ_GUEST_PENDING: the irq is asserted
> > 
> > I'm not sure how a second irq arriving would correspond to deasserting
> > the IRQ.
> 
> I see where the confusion is coming from.
> This comment is not quite correct unfortunately.
> 
> GIC_IRQ_GUEST_PENDING is set when the irq needs to be asserted (by
> adding it to the LRs). Once the irq has become guest visible,
> GIC_IRQ_GUEST_PENDING is cleared.
> 
> Going back to the top of the commit message:
> 
> "If the first irq is already pending (not active), just clear
> GIC_IRQ_GUEST_PENDING because the irq has already been injected and is
> already visible by the guest"
> 
> means that if the first irq has already been added to the LRs but it is
> still in pending state, we cannot add it a second time, so just go ahead
> and clear GIC_IRQ_GUEST_PENDING as if we did add it to the LRs, because
> the guest is still going to receive a notification.

OK, so when you use lower case pending/active you are talking about the
LR state. But when you use upper case GIC_IRQ_GUEST_PENDING this is a
completely separate state related to the queueing of interrupts within
Xen itself?

Can we s/GIC_IRQ_GUEST_PENDING/GIC_IRQ_GUEST_QUEUED/ perhaps? The
comment would be something like:
     * GIC_IRQ_GUEST_QUEUED: the irq is asserted and queued for
       injection into the guests LRs.

> 
> 
> > Also if you are clearing GIC_IRQ_GUEST_PENDING without clearing the
> > pending bit in the LR that's rather confusing -- I thought the state was
> > supposed to correspond to (the most recently observed) LR state.
> 
> Unfortunately not quite: GIC_IRQ_GUEST_PENDING is set by
> vgic_vcpu_inject_irq and cleared by gic_set_lr and gic_clear_one_lr.
> Later in this series it can also be set by gic_restore_pending_irqs in
> case we want to evict an irq from an LR to leave room for an higher
> priority irq.
> 
> 
> > (having been told that there should be a v6 I'm going to stop here while
> > I figure out where it went...)
> 
> This patch is unchanged in v6.

Thanks. I'll look into v6 ASAP, probably tomorrow AM.

Ian.


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