[Xen-devel] [PATCH v2] xen/gic: EOI irqs on the right pcpu

[Stefano Stabellini <stefano.stabellini@xxxxxxxxxxxxx>]

We need to write the irq number to GICC_DIR on the physical cpu that
previously received the interrupt, but currently we are doing it on the
pcpu that received the maintenance interrupt. As a consequence if a
vcpu is migrated to a different pcpu, the irq is going to be EOI'ed on
the wrong pcpu.

This covers the case where dom0 vcpu0 is running on pcpu1 for example
(you can test this scenario by using xl vcpu-pin).


Changes in v2:
- pass virq by value to gic_irq_eoi;
- EOI the interrupt without any spin locks held and with interrupt
enabled.

Signed-off-by: Stefano Stabellini <stefano.stabellini@xxxxxxxxxxxxx>
CC: keir@xxxxxxx
---
 xen/arch/arm/gic.c           |   24 +++++++++++++++++++++++-
 xen/arch/arm/vgic.c          |    7 ++++++-
 xen/include/asm-arm/domain.h |    2 ++
 3 files changed, 31 insertions(+), 2 deletions(-)

diff --git a/xen/arch/arm/gic.c b/xen/arch/arm/gic.c
index 61de230..2674f84 100644
--- a/xen/arch/arm/gic.c
+++ b/xen/arch/arm/gic.c
@@ -723,6 +723,12 @@ int gicv_setup(struct domain *d)
                         gic.vbase);
 }
 
+static void gic_irq_eoi(void *info)
+{
+    int virq = (int) info;
+    GICC[GICC_DIR] = virq;
+}
+
 static void maintenance_interrupt(int irq, void *dev_id, struct cpu_user_regs 
*regs)
 {
     int i = 0, virq;
@@ -733,6 +739,10 @@ 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;
+        int cpu, eoi;
+
+        cpu = -1;
+        eoi = 0;
 
         spin_lock_irq(&gic.lock);
         lr = GICH[GICH_LR + i];
@@ -754,11 +764,23 @@ static void maintenance_interrupt(int irq, void *dev_id, 
struct cpu_user_regs *r
         p = irq_to_pending(v, virq);
         if ( p->desc != NULL ) {
             p->desc->status &= ~IRQ_INPROGRESS;
-            GICC[GICC_DIR] = virq;
+            /* Assume only one pcpu needs to EOI the irq */
+            cpu = cpumask_first(&p->eoimask);
+            cpumask_clear(&p->eoimask);
+            eoi = 1;
         }
         list_del_init(&p->inflight);
         spin_unlock_irq(&v->arch.vgic.lock);
 
+        if ( eoi ) {
+            /* this is not racy because we can't receive another irq of the
+             * same type until we EOI it.  */
+            if ( cpu == smp_processor_id() )
+                gic_irq_eoi((void*)virq);
+            else
+                on_selected_cpus(cpumask_of(cpu), gic_irq_eoi, (void*)virq, 0);
+        }
+
         i++;
     }
 }
diff --git a/xen/arch/arm/vgic.c b/xen/arch/arm/vgic.c
index f9c1a6b..c5370d5 100644
--- a/xen/arch/arm/vgic.c
+++ b/xen/arch/arm/vgic.c
@@ -676,9 +676,14 @@ void vgic_vcpu_inject_irq(struct vcpu *v, unsigned int 
irq, int virtual)
     n->irq = irq;
     n->priority = priority;
     if (!virtual)
+    {
         n->desc = irq_to_desc(irq);
-    else
+        cpumask_clear(&n->eoimask);
+        /* Assume we received the IRQ on the current pcpu */
+        cpumask_set_cpu(smp_processor_id(), &n->eoimask);
+    } else {
         n->desc = NULL;
+    }
 
     /* the irq is enabled */
     if ( rank->ienable & (1 << (irq % 32)) )
diff --git a/xen/include/asm-arm/domain.h b/xen/include/asm-arm/domain.h
index cca7416..5561531 100644
--- a/xen/include/asm-arm/domain.h
+++ b/xen/include/asm-arm/domain.h
@@ -3,6 +3,7 @@
 
 #include <xen/config.h>
 #include <xen/cache.h>
+#include <xen/cpumask.h>
 #include <xen/sched.h>
 #include <asm/page.h>
 #include <asm/p2m.h>
@@ -21,6 +22,7 @@ struct pending_irq
 {
     int irq;
     struct irq_desc *desc; /* only set it the irq corresponds to a physical 
irq */
+    cpumask_t eoimask;
     uint8_t priority;
     /* inflight is used to append instances of pending_irq to
      * vgic.inflight_irqs */
-- 
1.7.2.5


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