xen/arm: track the state of guest IRQs

Introduce a status field in struct pending_irq. Valid states are
GUEST_PENDING, GUEST_VISIBLE and GUEST_ENABLED and they are not mutually
exclusive.  See the in-code comment for an explanation of the states and
how they are used.
Use atomic operations to set and clear the status bits. Note that
setting GIC_IRQ_GUEST_VISIBLE and clearing GIC_IRQ_GUEST_PENDING can be
done in two separate operations as the underlying pending status is
actually only cleared on the LR after the guest ACKs the interrupts.
Until that happens it's not possible to receive another interrupt.

The main effect of this patch is that an IRQ can be set to GUEST_PENDING
while it is being serviced by the guest. In maintenance_interrupt we
check whether GUEST_PENDING is set and if it is we add the irq back into
the lr_pending queue so that it's going to be reinjected one more time,
if the interrupt is still enabled at the vgicd level.
If it is not, it is going to be injected as soon as the guest renables
the interrupt.

One exception is evtchn_irq: in that case we don't want to
set the GIC_IRQ_GUEST_PENDING bit if it is already GUEST_VISIBLE,
because as part of the event handling loop, the guest would realize that
new events are present even without a new notification.
Also we already have a way to figure out exactly when we do need to
inject a second notification if vgic_vcpu_inject_irq is called after the
end of the guest event handling loop and before the guest EOIs the
interrupt (see db453468d92369e7182663fb13e14d83ec4ce456 "arm: vgic: fix
race between evtchn upcall and evtchnop_send").

Don't call gic_inject_irq_stop from maintenance_interrupt because
gic_inject (called by leave_hypervisor_tail) is going to call
gic_inject_irq_start/stop appropriately later anyway.

Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
Acked-by: Ian Campbell <ian.campbell@citrix.com>

diff --git a/xen/arch/arm/gic.c b/xen/arch/arm/gic.c
index 5f7a5481a13e1816cc351b9c73d7dac894d6ba50..02f19845117b0a50f4f20c9dbe3747620d13051f 100644 (file)
--- a/xen/arch/arm/gic.c
+++ b/xen/arch/arm/gic.c
@@ -615,6 +615,7 @@ static inline void gic_set_lr(int lr, unsigned int virtual_irq,
         unsigned int state, unsigned int priority)
 {
     int maintenance_int = GICH_LR_MAINTENANCE_IRQ;
+    struct pending_irq *p = irq_to_pending(current, virtual_irq);
 
     BUG_ON(lr >= nr_lrs);
     BUG_ON(lr < 0);
@@ -624,13 +625,34 @@ static inline void gic_set_lr(int lr, unsigned int virtual_irq,
         maintenance_int |
         ((priority >> 3) << GICH_LR_PRIORITY_SHIFT) |
         ((virtual_irq & GICH_LR_VIRTUAL_MASK) << GICH_LR_VIRTUAL_SHIFT);
+
+    set_bit(GIC_IRQ_GUEST_VISIBLE, &p->status);
+    clear_bit(GIC_IRQ_GUEST_PENDING, &p->status);
+}
+
+static inline void gic_add_to_lr_pending(struct vcpu *v, unsigned int irq,
+        unsigned int priority)
+{
+    struct pending_irq *iter, *n = irq_to_pending(v, irq);
+
+    if ( !list_empty(&n->lr_queue) )
+        return;
+
+    list_for_each_entry ( iter, &v->arch.vgic.lr_pending, lr_queue )
+    {
+        if ( iter->priority > priority )
+        {
+            list_add_tail(&n->lr_queue, &iter->lr_queue);
+            return;
+        }
+    }
+    list_add_tail(&n->lr_queue, &v->arch.vgic.lr_pending);
 }
 
 void gic_set_guest_irq(struct vcpu *v, unsigned int virtual_irq,
         unsigned int state, unsigned int priority)
 {
     int i;
-    struct pending_irq *iter, *n;
     unsigned long flags;
 
     spin_lock_irqsave(&gic.lock, flags);
@@ -645,19 +667,7 @@ void gic_set_guest_irq(struct vcpu *v, unsigned int virtual_irq,
         }
     }
 
-    n = irq_to_pending(v, virtual_irq);
-    if ( !list_empty(&n->lr_queue) )
-        goto out;
-
-    list_for_each_entry ( iter, &v->arch.vgic.lr_pending, lr_queue )
-    {
-        if ( iter->priority > priority )
-        {
-            list_add_tail(&n->lr_queue, &iter->lr_queue);
-            goto out;
-        }
-    }
-    list_add_tail(&n->lr_queue, &v->arch.vgic.lr_pending);
+    gic_add_to_lr_pending(v, virtual_irq, priority);
 
 out:
     spin_unlock_irqrestore(&gic.lock, flags);
@@ -887,10 +897,12 @@ static void maintenance_interrupt(int irq, void *dev_id, struct cpu_user_regs *r
 
     while ((i = find_next_bit((const long unsigned int *) &eisr,
                               64, i)) < 64) {
-        struct pending_irq *p;
+        struct pending_irq *p, *p2;
         int cpu;
+        bool_t inflight;
 
         cpu = -1;
+        inflight = 0;
 
         spin_lock_irq(&gic.lock);
         lr = GICH[GICH_LR + i];
@@ -898,17 +910,6 @@ static void maintenance_interrupt(int irq, void *dev_id, struct cpu_user_regs *r
         GICH[GICH_LR + i] = 0;
         clear_bit(i, &this_cpu(lr_mask));
 
-        if ( !list_empty(&v->arch.vgic.lr_pending) ) {
-            p = list_entry(v->arch.vgic.lr_pending.next, typeof(*p), lr_queue);
-            gic_set_lr(i, p->irq, GICH_LR_PENDING, p->priority);
-            list_del_init(&p->lr_queue);
-            set_bit(i, &this_cpu(lr_mask));
-        } else {
-            gic_inject_irq_stop();
-        }
-        spin_unlock_irq(&gic.lock);
-
-        spin_lock_irq(&v->arch.vgic.lock);
         p = irq_to_pending(v, virq);
         if ( p->desc != NULL ) {
             p->desc->status &= ~IRQ_INPROGRESS;
@@ -916,8 +917,29 @@ static void maintenance_interrupt(int irq, void *dev_id, struct cpu_user_regs *r
             cpu = p->desc->arch.eoi_cpu;
             pirq = p->desc->irq;
         }
-        list_del_init(&p->inflight);
-        spin_unlock_irq(&v->arch.vgic.lock);
+        if ( test_bit(GIC_IRQ_GUEST_PENDING, &p->status) &&
+             test_bit(GIC_IRQ_GUEST_ENABLED, &p->status))
+        {
+            inflight = 1;
+            gic_add_to_lr_pending(v, virq, p->priority);
+        }
+
+        clear_bit(GIC_IRQ_GUEST_VISIBLE, &p->status);
+
+        if ( !list_empty(&v->arch.vgic.lr_pending) ) {
+            p2 = list_entry(v->arch.vgic.lr_pending.next, typeof(*p2), lr_queue);
+            gic_set_lr(i, p2->irq, GICH_LR_PENDING, p2->priority);
+            list_del_init(&p2->lr_queue);
+            set_bit(i, &this_cpu(lr_mask));
+        }
+        spin_unlock_irq(&gic.lock);
+
+        if ( !inflight )
+        {
+            spin_lock_irq(&v->arch.vgic.lock);
+            list_del_init(&p->inflight);
+            spin_unlock_irq(&v->arch.vgic.lock);
+        }
 
         if ( p->desc != NULL ) {
             /* this is not racy because we can't receive another irq of the

diff --git a/xen/arch/arm/vgic.c b/xen/arch/arm/vgic.c
index 2e4b11f46f39fbf0221abe7ca58081dec7027ef0..b449f450a08e0a2470fb91194f966876eb68f194 100644 (file)
--- a/xen/arch/arm/vgic.c
+++ b/xen/arch/arm/vgic.c
@@ -369,6 +369,7 @@ static void vgic_enable_irqs(struct vcpu *v, uint32_t r, int n)
     while ( (i = find_next_bit((const long unsigned int *) &r, 32, i)) < 32 ) {
         irq = i + (32 * n);
         p = irq_to_pending(v, irq);
+        set_bit(GIC_IRQ_GUEST_ENABLED, &p->status);
         if ( !list_empty(&p->inflight) )
             gic_set_guest_irq(v, irq, GICH_LR_PENDING, p->priority);
         i++;
@@ -672,8 +673,17 @@ void vgic_vcpu_inject_irq(struct vcpu *v, unsigned int irq, int virtual)
 
     spin_lock_irqsave(&v->arch.vgic.lock, flags);
 
-    /* vcpu offline or irq already pending */
-    if (test_bit(_VPF_down, &v->pause_flags) || !list_empty(&n->inflight))
+    if ( !list_empty(&n->inflight) )
+    {
+        if ( (irq != current->domain->arch.evtchn_irq) ||
+             (!test_bit(GIC_IRQ_GUEST_VISIBLE, &n->status)) )
+            set_bit(GIC_IRQ_GUEST_PENDING, &n->status);
+        spin_unlock_irqrestore(&v->arch.vgic.lock, flags);
+        return;
+    }
+
+    /* vcpu offline */
+    if ( test_bit(_VPF_down, &v->pause_flags) )
     {
         spin_unlock_irqrestore(&v->arch.vgic.lock, flags);
         return;
@@ -682,6 +692,7 @@ void vgic_vcpu_inject_irq(struct vcpu *v, unsigned int irq, int virtual)
     priority = byte_read(rank->ipriority[REG_RANK_INDEX(8, idx)], 0, byte);
 
     n->irq = irq;
+    set_bit(GIC_IRQ_GUEST_PENDING, &n->status);
     n->priority = priority;
     if (!virtual)
         n->desc = irq_to_desc(irq);
@@ -689,7 +700,7 @@ void vgic_vcpu_inject_irq(struct vcpu *v, unsigned int irq, int virtual)
         n->desc = NULL;
 
     /* the irq is enabled */
-    if ( rank->ienable & (1 << (irq % 32)) )
+    if ( test_bit(GIC_IRQ_GUEST_ENABLED, &n->status) )
         gic_set_guest_irq(v, irq, GICH_LR_PENDING, priority);
 
     list_for_each_entry ( iter, &v->arch.vgic.inflight_irqs, inflight )

diff --git a/xen/include/asm-arm/domain.h b/xen/include/asm-arm/domain.h
index 8ebee3e05f6f2026ffcc1cdf2fb3977e70810f27..0733c5e37565d772910bbac0efcec8294fccb164 100644 (file)
--- a/xen/include/asm-arm/domain.h
+++ b/xen/include/asm-arm/domain.h
@@ -22,6 +22,43 @@ struct vgic_irq_rank {
 struct pending_irq
 {
     int irq;
+    /*
+     * The following two states track the lifecycle of the guest irq.
+     * However because we are not sure and we don't want to track
+     * whether an irq added to an LR register is PENDING or ACTIVE, the
+     * following states are just an approximation.
+     *
+     * GIC_IRQ_GUEST_PENDING: the irq is asserted
+     *
+     * GIC_IRQ_GUEST_VISIBLE: the irq has been added to an LR register,
+     * therefore the guest is aware of it. From the guest point of view
+     * the irq can be pending (if the guest has not acked the irq yet)
+     * or active (after acking the irq).
+     *
+     * In order for the state machine to be fully accurate, for level
+     * interrupts, we should keep the GIC_IRQ_GUEST_PENDING state until
+     * the guest deactivates the irq. However because we are not sure
+     * when that happens, we simply remove the GIC_IRQ_GUEST_PENDING
+     * state when we add the irq to an LR register. We add it back when
+     * we receive another interrupt notification.
+     * Therefore it is possible to set GIC_IRQ_GUEST_PENDING while the
+     * irq is GIC_IRQ_GUEST_VISIBLE. We could also change the state of
+     * the guest irq in the LR register from active to active and
+     * pending, but for simplicity we simply inject a second irq after
+     * the guest EOIs the first one.
+     *
+     *
+     * An additional state is used to keep track of whether the guest
+     * irq is enabled at the vgicd level:
+     *
+     * GIC_IRQ_GUEST_ENABLED: the guest IRQ is enabled at the VGICD
+     * level (GICD_ICENABLER/GICD_ISENABLER).
+     *
+     */
+#define GIC_IRQ_GUEST_PENDING  0
+#define GIC_IRQ_GUEST_VISIBLE  1
+#define GIC_IRQ_GUEST_ENABLED  2
+    unsigned long status;
     struct irq_desc *desc; /* only set it the irq corresponds to a physical irq */
     uint8_t priority;
     /* inflight is used to append instances of pending_irq to