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

[Stefano Stabellini <stefano.stabellini@xxxxxxxxxxxxx>]

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.


> 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.

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