[PATCH-4.5 v2 0/10] remove maintenance interrupts

[PATCH-4.5 v2 0/10] remove maintenance interrupts

[Stefano Stabellini]

Hi all,
this patch series removes any needs for maintenance interrupts for both
hardware and software interrupts in Xen.
It achieves the goal by using the GICH_LR_HW bit for hardware interrupts
and by checking the status of the GICH_LR registers on return to guest,
clearing the registers that are invalid and handling the lifecycle of
the corresponding interrupts in Xen data structures.


Changes in v2:
- do not assume physical IRQ == virtual IRQ;
- refactor gic_set_lr;
- simplify gic_clear_lrs;
- disable/enable the GICH_HCR_UIE bit in GICH_HCR;
- only enable GICH_HCR_UIE if this_cpu(lr_mask) == ((1 << nr_lrs) - 1);
- add a patch to keep track of the LR number in pending_irq;
- add a patch to set GICH_LR_PENDING to inject a second irq while the
first one is still active;
- add a patch to simplify and reduce the usage of gic.lock;
- add a patch to reduce the usage of vgic.lock;
- add a patch to use GICH_ELSR[01] to avoid reading all the GICH_LRs in
gic_clear_lrs;
- add a debug patch to print more info in gic_dump_info.


Stefano Stabellini (10):
      xen/arm: remove unused virtual parameter from vgic_vcpu_inject_irq
      xen/arm: support HW interrupts in gic_set_lr
      xen/arm: do not request maintenance_interrupts
      xen/arm: set GICH_HCR_NPIE if all the LRs are in use
      xen/arm: keep track of the GICH_LR used for the irq in struct pending_irq
      xen/arm: second irq injection while the first irq is still inflight
      xen/arm: don't protect GICH and lr_queue accesses with gic.lock
      xen/arm: avoid taking unconditionally the vgic.lock in gic_clear_lrs
      xen/arm: use GICH_ELSR[01] to avoid reading all the GICH_LRs in gic_clear_lrs
      xen/arm: print more info in gic_dump_info, keep gic_lr sync'ed

 xen/arch/arm/domain.c        |    2 +-
 xen/arch/arm/gic.c           |  216 +++++++++++++++++++++++++++++--------------------------
 xen/arch/arm/irq.c           |    2 +-
 xen/arch/arm/time.c          |    2 +-
 xen/arch/arm/vgic.c          |   34 ++++++---
 xen/arch/arm/vtimer.c        |    4 +-
 xen/include/asm-arm/domain.h |    1 +
 xen/include/asm-arm/gic.h    |    4 +-
 8 files changed, 151 insertions(+), 114 deletions(-)


git://xenbits.xen.org/people/sstabellini/xen-unstable.git no_maintenance_interrupts-v2

[PATCH-4.5 v2 01/10] xen/arm: remove unused virtual parameter from vgic_vcpu_inject_irq

[Stefano Stabellini]

Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
Acked-by: Julien Grall <julien.grall@linaro.org>
---
 xen/arch/arm/domain.c     |    2 +-
 xen/arch/arm/gic.c        |    2 +-
 xen/arch/arm/irq.c        |    2 +-
 xen/arch/arm/time.c       |    2 +-
 xen/arch/arm/vgic.c       |    4 ++--
 xen/arch/arm/vtimer.c     |    4 ++--
 xen/include/asm-arm/gic.h |    2 +-
 7 files changed, 9 insertions(+), 9 deletions(-)

diff --git a/xen/arch/arm/domain.c b/xen/arch/arm/domain.c
index 635a9a4..244738d 100644
--- a/xen/arch/arm/domain.c
+++ b/xen/arch/arm/domain.c
@@ -791,7 +791,7 @@ void vcpu_mark_events_pending(struct vcpu *v)
     if ( already_pending )
         return;
 
-    vgic_vcpu_inject_irq(v, v->domain->arch.evtchn_irq, 1);
+    vgic_vcpu_inject_irq(v, v->domain->arch.evtchn_irq);
 }
 
 /*
diff --git a/xen/arch/arm/gic.c b/xen/arch/arm/gic.c
index 50b3a38..acf7195 100644
--- a/xen/arch/arm/gic.c
+++ b/xen/arch/arm/gic.c
@@ -748,7 +748,7 @@ int gic_events_need_delivery(void)
 void gic_inject(void)
 {
     if ( vcpu_info(current, evtchn_upcall_pending) )
-        vgic_vcpu_inject_irq(current, current->domain->arch.evtchn_irq, 1);
+        vgic_vcpu_inject_irq(current, current->domain->arch.evtchn_irq);
 
     gic_restore_pending_irqs(current);
     if (!gic_events_need_delivery())
diff --git a/xen/arch/arm/irq.c b/xen/arch/arm/irq.c
index 3e326b0..5daa269 100644
--- a/xen/arch/arm/irq.c
+++ b/xen/arch/arm/irq.c
@@ -159,7 +159,7 @@ void do_IRQ(struct cpu_user_regs *regs, unsigned int irq, int is_fiq)
         desc->arch.eoi_cpu = smp_processor_id();
 
         /* XXX: inject irq into all guest vcpus */
-        vgic_vcpu_inject_irq(d->vcpu[0], irq, 0);
+        vgic_vcpu_inject_irq(d->vcpu[0], irq);
         goto out_no_end;
     }
 
diff --git a/xen/arch/arm/time.c b/xen/arch/arm/time.c
index 68b939d..0548201 100644
--- a/xen/arch/arm/time.c
+++ b/xen/arch/arm/time.c
@@ -215,7 +215,7 @@ static void vtimer_interrupt(int irq, void *dev_id, struct cpu_user_regs *regs)
 {
     current->arch.virt_timer.ctl = READ_SYSREG32(CNTV_CTL_EL0);
     WRITE_SYSREG32(current->arch.virt_timer.ctl | CNTx_CTL_MASK, CNTV_CTL_EL0);
-    vgic_vcpu_inject_irq(current, current->arch.virt_timer.irq, 1);
+    vgic_vcpu_inject_irq(current, current->arch.virt_timer.irq);
 }
 
 /* Route timer's IRQ on this CPU */
diff --git a/xen/arch/arm/vgic.c b/xen/arch/arm/vgic.c
index 90e9707..7d10227 100644
--- a/xen/arch/arm/vgic.c
+++ b/xen/arch/arm/vgic.c
@@ -455,7 +455,7 @@ static int vgic_to_sgi(struct vcpu *v, register_t sgir)
                      sgir, vcpu_mask);
             continue;
         }
-        vgic_vcpu_inject_irq(d->vcpu[vcpuid], virtual_irq, 1);
+        vgic_vcpu_inject_irq(d->vcpu[vcpuid], virtual_irq);
     }
     return 1;
 }
@@ -683,7 +683,7 @@ void vgic_clear_pending_irqs(struct vcpu *v)
     spin_unlock_irqrestore(&v->arch.vgic.lock, flags);
 }
 
-void vgic_vcpu_inject_irq(struct vcpu *v, unsigned int irq, int virtual)
+void vgic_vcpu_inject_irq(struct vcpu *v, unsigned int irq)
 {
     int idx = irq >> 2, byte = irq & 0x3;
     uint8_t priority;
diff --git a/xen/arch/arm/vtimer.c b/xen/arch/arm/vtimer.c
index e325f78..87be11e 100644
--- a/xen/arch/arm/vtimer.c
+++ b/xen/arch/arm/vtimer.c
@@ -34,14 +34,14 @@ static void phys_timer_expired(void *data)
     struct vtimer *t = data;
     t->ctl |= CNTx_CTL_PENDING;
     if ( !(t->ctl & CNTx_CTL_MASK) )
-        vgic_vcpu_inject_irq(t->v, t->irq, 1);
+        vgic_vcpu_inject_irq(t->v, t->irq);
 }
 
 static void virt_timer_expired(void *data)
 {
     struct vtimer *t = data;
     t->ctl |= CNTx_CTL_MASK;
-    vgic_vcpu_inject_irq(t->v, t->irq, 1);
+    vgic_vcpu_inject_irq(t->v, t->irq);
 }
 
 int vcpu_domain_init(struct domain *d)
diff --git a/xen/include/asm-arm/gic.h b/xen/include/asm-arm/gic.h
index 071280b..6fce5c2 100644
--- a/xen/include/asm-arm/gic.h
+++ b/xen/include/asm-arm/gic.h
@@ -162,7 +162,7 @@ extern void domain_vgic_free(struct domain *d);
 
 extern int vcpu_vgic_init(struct vcpu *v);
 
-extern void vgic_vcpu_inject_irq(struct vcpu *v, unsigned int irq,int virtual);
+extern void vgic_vcpu_inject_irq(struct vcpu *v, unsigned int irq);
 extern void vgic_clear_pending_irqs(struct vcpu *v);
 extern struct pending_irq *irq_to_pending(struct vcpu *v, unsigned int irq);
 
-- 
1.7.10.4

[PATCH-4.5 v2 02/10] xen/arm: support HW interrupts in gic_set_lr

[Stefano Stabellini]

If the irq to be injected is an hardware irq (p->desc != NULL), set
GICH_LR_HW.

Remove the code to EOI a physical interrupt on behalf of the guest
because it has become unnecessary.

Also add a struct vcpu* parameter to gic_set_lr.

Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>

---

Changes in v2:
- remove the EOI code, now unnecessary;
- do not assume physical IRQ == virtual IRQ;
- refactor gic_set_lr.
---
 xen/arch/arm/gic.c |   52 +++++++++++++++++-----------------------------------
 1 file changed, 17 insertions(+), 35 deletions(-)

diff --git a/xen/arch/arm/gic.c b/xen/arch/arm/gic.c
index acf7195..64c8aa7 100644
--- a/xen/arch/arm/gic.c
+++ b/xen/arch/arm/gic.c
@@ -618,20 +618,24 @@ int __init setup_dt_irq(const struct dt_irq *irq, struct irqaction *new)
     return rc;
 }
 
-static inline void gic_set_lr(int lr, unsigned int virtual_irq,
+static inline void gic_set_lr(struct vcpu *v, int lr, unsigned int irq,
         unsigned int state, unsigned int priority)
 {
-    int maintenance_int = GICH_LR_MAINTENANCE_IRQ;
-    struct pending_irq *p = irq_to_pending(current, virtual_irq);
+    struct pending_irq *p = irq_to_pending(v, irq);
+    uint32_t lr_reg;
 
     BUG_ON(lr >= nr_lrs);
     BUG_ON(lr < 0);
     BUG_ON(state & ~(GICH_LR_STATE_MASK<<GICH_LR_STATE_SHIFT));
 
-    GICH[GICH_LR + lr] = state |
-        maintenance_int |
+    lr_reg = state | GICH_LR_MAINTENANCE_IRQ |
         ((priority >> 3) << GICH_LR_PRIORITY_SHIFT) |
-        ((virtual_irq & GICH_LR_VIRTUAL_MASK) << GICH_LR_VIRTUAL_SHIFT);
+        ((irq & GICH_LR_VIRTUAL_MASK) << GICH_LR_VIRTUAL_SHIFT);
+    if ( p->desc != NULL )
+        lr_reg |= GICH_LR_HW |
+            ((p->desc->irq & GICH_LR_PHYSICAL_MASK) << GICH_LR_PHYSICAL_SHIFT);
+
+    GICH[GICH_LR + lr] = lr_reg;
 
     set_bit(GIC_IRQ_GUEST_VISIBLE, &p->status);
     clear_bit(GIC_IRQ_GUEST_PENDING, &p->status);
@@ -666,7 +670,7 @@ void gic_remove_from_queues(struct vcpu *v, unsigned int virtual_irq)
     spin_unlock(&gic.lock);
 }
 
-void gic_set_guest_irq(struct vcpu *v, unsigned int virtual_irq,
+void gic_set_guest_irq(struct vcpu *v, unsigned int irq,
         unsigned int state, unsigned int priority)
 {
     int i;
@@ -679,12 +683,12 @@ void gic_set_guest_irq(struct vcpu *v, unsigned int virtual_irq,
         i = find_first_zero_bit(&this_cpu(lr_mask), nr_lrs);
         if (i < nr_lrs) {
             set_bit(i, &this_cpu(lr_mask));
-            gic_set_lr(i, virtual_irq, state, priority);
+            gic_set_lr(v, i, irq, state, priority);
             goto out;
         }
     }
 
-    gic_add_to_lr_pending(v, virtual_irq, priority);
+    gic_add_to_lr_pending(v, irq, priority);
 
 out:
     spin_unlock_irqrestore(&gic.lock, flags);
@@ -703,7 +707,7 @@ static void gic_restore_pending_irqs(struct vcpu *v)
         if ( i >= nr_lrs ) return;
 
         spin_lock_irqsave(&gic.lock, flags);
-        gic_set_lr(i, p->irq, GICH_LR_PENDING, p->priority);
+        gic_set_lr(v, i, p->irq, GICH_LR_PENDING, p->priority);
         list_del_init(&p->lr_queue);
         set_bit(i, &this_cpu(lr_mask));
         spin_unlock_irqrestore(&gic.lock, flags);
@@ -904,15 +908,9 @@ int gicv_setup(struct domain *d)
 
 }
 
-static void gic_irq_eoi(void *info)
-{
-    int virq = (uintptr_t) info;
-    GICC[GICC_DIR] = virq;
-}
-
 static void maintenance_interrupt(int irq, void *dev_id, struct cpu_user_regs *regs)
 {
-    int i = 0, virq, pirq = -1;
+    int i = 0, virq;
     uint32_t lr;
     struct vcpu *v = current;
     uint64_t eisr = GICH[GICH_EISR0] | (((uint64_t) GICH[GICH_EISR1]) << 32);
@@ -920,10 +918,8 @@ 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, *p2;
-        int cpu;
         bool_t inflight;
 
-        cpu = -1;
         inflight = 0;
 
         spin_lock_irq(&gic.lock);
@@ -933,12 +929,8 @@ static void maintenance_interrupt(int irq, void *dev_id, struct cpu_user_regs *r
         clear_bit(i, &this_cpu(lr_mask));
 
         p = irq_to_pending(v, virq);
-        if ( p->desc != NULL ) {
+        if ( p->desc != NULL )
             p->desc->status &= ~IRQ_INPROGRESS;
-            /* Assume only one pcpu needs to EOI the irq */
-            cpu = p->desc->arch.eoi_cpu;
-            pirq = p->desc->irq;
-        }
         if ( test_bit(GIC_IRQ_GUEST_PENDING, &p->status) &&
              test_bit(GIC_IRQ_GUEST_ENABLED, &p->status))
         {
@@ -950,7 +942,7 @@ static void maintenance_interrupt(int irq, void *dev_id, struct cpu_user_regs *r
 
         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);
+            gic_set_lr(v, i, p2->irq, GICH_LR_PENDING, p2->priority);
             list_del_init(&p2->lr_queue);
             set_bit(i, &this_cpu(lr_mask));
         }
@@ -963,16 +955,6 @@ static void maintenance_interrupt(int irq, void *dev_id, struct cpu_user_regs *r
             spin_unlock_irq(&v->arch.vgic.lock);
         }
 
-        if ( p->desc != NULL ) {
-            /* 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*)(uintptr_t)pirq);
-            else
-                on_selected_cpus(cpumask_of(cpu),
-                                 gic_irq_eoi, (void*)(uintptr_t)pirq, 0);
-        }
-
         i++;
     }
 }
-- 
1.7.10.4

[PATCH-4.5 v2 03/10] xen/arm: do not request maintenance_interrupts

[Stefano Stabellini]

Do not set GICH_LR_MAINTENANCE_IRQ for every interrupt with set in the
GICH_LR registers.

Introduce a new function, gic_clear_lrs, that goes over the GICH_LR
registers, clear the invalid ones and free the corresponding interrupts
from the inflight queue if appropriate. Add the interrupt to lr_pending
if the GIC_IRQ_GUEST_PENDING is still set.

Call gic_clear_lrs from gic_restore_state and on return to guest
(gic_inject).

Remove the now unused code in maintenance_interrupts and gic_irq_eoi.

In vgic_vcpu_inject_irq, if the target is a vcpu running on another cpu,
send and SGI to it to interrupt it and force it to clear the old LRs.

Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>

---

Changes in v2:
- simplify gic_clear_lrs.
---
 xen/arch/arm/gic.c  |   99 ++++++++++++++++++++++++++-------------------------
 xen/arch/arm/vgic.c |    3 +-
 2 files changed, 51 insertions(+), 51 deletions(-)

diff --git a/xen/arch/arm/gic.c b/xen/arch/arm/gic.c
index 64c8aa7..ee383bc 100644
--- a/xen/arch/arm/gic.c
+++ b/xen/arch/arm/gic.c
@@ -67,6 +67,8 @@ static DEFINE_PER_CPU(u8, gic_cpu_id);
 /* Maximum cpu interface per GIC */
 #define NR_GIC_CPU_IF 8
 
+static void gic_clear_lrs(struct vcpu *v);
+
 static unsigned int gic_cpu_mask(const cpumask_t *cpumask)
 {
     unsigned int cpu;
@@ -126,6 +128,7 @@ void gic_restore_state(struct vcpu *v)
     GICH[GICH_HCR] = GICH_HCR_EN;
     isb();
 
+    gic_clear_lrs(v);
     gic_restore_pending_irqs(v);
 }
 
@@ -628,8 +631,7 @@ static inline void gic_set_lr(struct vcpu *v, int lr, unsigned int irq,
     BUG_ON(lr < 0);
     BUG_ON(state & ~(GICH_LR_STATE_MASK<<GICH_LR_STATE_SHIFT));
 
-    lr_reg = state | GICH_LR_MAINTENANCE_IRQ |
-        ((priority >> 3) << GICH_LR_PRIORITY_SHIFT) |
+    lr_reg = state | ((priority >> 3) << GICH_LR_PRIORITY_SHIFT) |
         ((irq & GICH_LR_VIRTUAL_MASK) << GICH_LR_VIRTUAL_SHIFT);
     if ( p->desc != NULL )
         lr_reg |= GICH_LR_HW |
@@ -695,6 +697,50 @@ out:
     return;
 }
 
+static void gic_clear_lrs(struct vcpu *v)
+{
+    struct pending_irq *p;
+    int i = 0, irq;
+    uint32_t lr;
+    bool_t inflight;
+
+    ASSERT(!local_irq_is_enabled());
+
+    while ((i = find_next_bit((const long unsigned int *) &this_cpu(lr_mask),
+                              nr_lrs, i)) < nr_lrs) {
+        lr = GICH[GICH_LR + i];
+        if ( !(lr & (GICH_LR_PENDING|GICH_LR_ACTIVE)) )
+        {
+            inflight = 0;
+            GICH[GICH_LR + i] = 0;
+            clear_bit(i, &this_cpu(lr_mask));
+
+            irq = (lr >> GICH_LR_VIRTUAL_SHIFT) & GICH_LR_VIRTUAL_MASK;
+            spin_lock(&gic.lock);
+            p = irq_to_pending(v, irq);
+            if ( p->desc != NULL )
+                p->desc->status &= ~IRQ_INPROGRESS;
+            clear_bit(GIC_IRQ_GUEST_VISIBLE, &p->status);
+            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, irq, p->priority);
+            }
+            spin_unlock(&gic.lock);
+            if ( !inflight )
+            {
+                spin_lock(&v->arch.vgic.lock);
+                list_del_init(&p->inflight);
+                spin_unlock(&v->arch.vgic.lock);
+            }
+
+        }
+
+        i++;
+    }
+}
+
 static void gic_restore_pending_irqs(struct vcpu *v)
 {
     int i;
@@ -751,6 +797,8 @@ int gic_events_need_delivery(void)
 
 void gic_inject(void)
 {
+    gic_clear_lrs(current);
+
     if ( vcpu_info(current, evtchn_upcall_pending) )
         vgic_vcpu_inject_irq(current, current->domain->arch.evtchn_irq);
 
@@ -910,53 +958,6 @@ int gicv_setup(struct domain *d)
 
 static void maintenance_interrupt(int irq, void *dev_id, struct cpu_user_regs *regs)
 {
-    int i = 0, virq;
-    uint32_t lr;
-    struct vcpu *v = current;
-    uint64_t eisr = GICH[GICH_EISR0] | (((uint64_t) GICH[GICH_EISR1]) << 32);
-
-    while ((i = find_next_bit((const long unsigned int *) &eisr,
-                              64, i)) < 64) {
-        struct pending_irq *p, *p2;
-        bool_t inflight;
-
-        inflight = 0;
-
-        spin_lock_irq(&gic.lock);
-        lr = GICH[GICH_LR + i];
-        virq = lr & GICH_LR_VIRTUAL_MASK;
-        GICH[GICH_LR + i] = 0;
-        clear_bit(i, &this_cpu(lr_mask));
-
-        p = irq_to_pending(v, virq);
-        if ( p->desc != NULL )
-            p->desc->status &= ~IRQ_INPROGRESS;
-        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(v, 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);
-        }
-
-        i++;
-    }
 }
 
 void gic_dump_info(struct vcpu *v)
diff --git a/xen/arch/arm/vgic.c b/xen/arch/arm/vgic.c
index 7d10227..da15f4d 100644
--- a/xen/arch/arm/vgic.c
+++ b/xen/arch/arm/vgic.c
@@ -699,8 +699,7 @@ void vgic_vcpu_inject_irq(struct vcpu *v, unsigned int irq)
         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;
+        goto out;
     }
 
     /* vcpu offline */
-- 
1.7.10.4

[PATCH-4.5 v2 04/10] xen/arm: set GICH_HCR_UIE if all the LRs are in use

[Stefano Stabellini]

On return to guest, if there are no free LRs and we still have more
interrupt to inject, set GICH_HCR_UIE so that we are going to receive a
maintenance interrupt when no pending interrupts are present in the LR
registers.
The maintenance interrupt handler won't do anything anymore, but
receiving the interrupt is going to cause gic_inject to be called on
return to guest that is going to clear the old LRs and inject new
interrupts.

Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>

---

Changes in v2:
- disable/enable the GICH_HCR_UIE bit in GICH_HCR;
- only enable GICH_HCR_UIE if this_cpu(lr_mask) == ((1 << nr_lrs) - 1).

---
 xen/arch/arm/gic.c |    9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/xen/arch/arm/gic.c b/xen/arch/arm/gic.c
index ee383bc..0928aca 100644
--- a/xen/arch/arm/gic.c
+++ b/xen/arch/arm/gic.c
@@ -805,8 +805,15 @@ void gic_inject(void)
     gic_restore_pending_irqs(current);
     if (!gic_events_need_delivery())
         gic_inject_irq_stop();
-    else
+    else {
         gic_inject_irq_start();
+    }
+
+    if ( !list_empty(&current->arch.vgic.lr_pending) &&
+         this_cpu(lr_mask) == ((1 << nr_lrs) - 1) )
+        GICH[GICH_HCR] |= GICH_HCR_UIE;
+    else
+        GICH[GICH_HCR] &= ~GICH_HCR_UIE;
 }
 
 int gic_route_irq_to_guest(struct domain *d, const struct dt_irq *irq,
-- 
1.7.10.4

[PATCH-4.5 v2 05/10] xen/arm: keep track of the GICH_LR used for the irq in struct pending_irq

[Stefano Stabellini]

Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
---
 xen/arch/arm/gic.c           |    6 ++++--
 xen/include/asm-arm/domain.h |    1 +
 2 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/xen/arch/arm/gic.c b/xen/arch/arm/gic.c
index 0928aca..5fca5be 100644
--- a/xen/arch/arm/gic.c
+++ b/xen/arch/arm/gic.c
@@ -641,6 +641,7 @@ static inline void gic_set_lr(struct vcpu *v, int lr, unsigned int irq,
 
     set_bit(GIC_IRQ_GUEST_VISIBLE, &p->status);
     clear_bit(GIC_IRQ_GUEST_PENDING, &p->status);
+    p->lr = lr;
 }
 
 static inline void gic_add_to_lr_pending(struct vcpu *v, unsigned int irq,
@@ -721,6 +722,7 @@ static void gic_clear_lrs(struct vcpu *v)
             if ( p->desc != NULL )
                 p->desc->status &= ~IRQ_INPROGRESS;
             clear_bit(GIC_IRQ_GUEST_VISIBLE, &p->status);
+            p->lr = nr_lrs;
             if ( test_bit(GIC_IRQ_GUEST_PENDING, &p->status) &&
                     test_bit(GIC_IRQ_GUEST_ENABLED, &p->status))
             {
@@ -984,12 +986,12 @@ void gic_dump_info(struct vcpu *v)
 
     list_for_each_entry ( p, &v->arch.vgic.inflight_irqs, inflight )
     {
-        printk("Inflight irq=%d\n", p->irq);
+        printk("Inflight irq=%d lr=%u\n", p->irq, p->lr);
     }
 
     list_for_each_entry( p, &v->arch.vgic.lr_pending, lr_queue )
     {
-        printk("Pending irq=%d\n", p->irq);
+        printk("Pending irq=%d lr=%u\n", p->irq, p->lr);
     }
 
 }
diff --git a/xen/include/asm-arm/domain.h b/xen/include/asm-arm/domain.h
index af8c64b..7b636c8 100644
--- a/xen/include/asm-arm/domain.h
+++ b/xen/include/asm-arm/domain.h
@@ -59,6 +59,7 @@ struct pending_irq
 #define GIC_IRQ_GUEST_VISIBLE  1
 #define GIC_IRQ_GUEST_ENABLED  2
     unsigned long status;
+    uint8_t lr;
     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
-- 
1.7.10.4

[PATCH-4.5 v2 06/10] xen/arm: second irq injection while the first irq is still inflight

[Stefano Stabellini]

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.
If the irq has already been EOI'ed then just clear the GICH_LR right
away and move the interrupt to lr_pending so that it is going to be
reinjected by gic_restore_pending_irqs on return to guest.

If the target cpu is not the current cpu, then set GIC_IRQ_GUEST_PENDING
and send an SGI. The target cpu is going to be interrupted and call
gic_clear_lrs, that is going to take the same actions.

Do not call vgic_vcpu_inject_irq from gic_inject if
evtchn_upcall_pending is set. If we remove that call, we don't need to
special case evtchn_irq in vgic_vcpu_inject_irq anymore.
We also need to force the first injection of evtchn_irq (call
gic_vcpu_inject_irq) from vgic_enable_irqs because evtchn_upcall_pending
is already set by common code on vcpu creation.

Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
---
 xen/arch/arm/gic.c        |   82 +++++++++++++++++++++++++--------------------
 xen/arch/arm/vgic.c       |   18 +++++++---
 xen/include/asm-arm/gic.h |    1 +
 3 files changed, 61 insertions(+), 40 deletions(-)

diff --git a/xen/arch/arm/gic.c b/xen/arch/arm/gic.c
index 5fca5be..0955d48 100644
--- a/xen/arch/arm/gic.c
+++ b/xen/arch/arm/gic.c
@@ -698,51 +698,64 @@ out:
     return;
 }
 
-static void gic_clear_lrs(struct vcpu *v)
+static void _gic_clear_lr(struct vcpu *v, int i)
 {
-    struct pending_irq *p;
-    int i = 0, irq;
+    int irq;
     uint32_t lr;
-    bool_t inflight;
+    struct pending_irq *p;
 
     ASSERT(!local_irq_is_enabled());
 
-    while ((i = find_next_bit((const long unsigned int *) &this_cpu(lr_mask),
-                              nr_lrs, i)) < nr_lrs) {
-        lr = GICH[GICH_LR + i];
-        if ( !(lr & (GICH_LR_PENDING|GICH_LR_ACTIVE)) )
+    lr = GICH[GICH_LR + i];
+    irq = (lr >> GICH_LR_VIRTUAL_SHIFT) & GICH_LR_VIRTUAL_MASK;
+    p = irq_to_pending(v, irq);
+    if ( lr & GICH_LR_ACTIVE )
+    {
+        if ( test_bit(GIC_IRQ_GUEST_ENABLED, &p->status) &&
+             test_and_clear_bit(GIC_IRQ_GUEST_PENDING, &p->status) )
+            GICH[GICH_LR + i] = lr | GICH_LR_PENDING;
+    } else if ( lr & GICH_LR_PENDING ) {
+        clear_bit(GIC_IRQ_GUEST_PENDING, &p->status);
+    } else {
+        spin_lock(&gic.lock);
+
+        GICH[GICH_LR + i] = 0;
+        clear_bit(i, &this_cpu(lr_mask));
+
+        if ( p->desc != NULL )
+            p->desc->status &= ~IRQ_INPROGRESS;
+        clear_bit(GIC_IRQ_GUEST_VISIBLE, &p->status);
+        p->lr = nr_lrs;
+        if ( test_bit(GIC_IRQ_GUEST_PENDING, &p->status) &&
+                test_bit(GIC_IRQ_GUEST_ENABLED, &p->status))
         {
-            inflight = 0;
-            GICH[GICH_LR + i] = 0;
-            clear_bit(i, &this_cpu(lr_mask));
-
-            irq = (lr >> GICH_LR_VIRTUAL_SHIFT) & GICH_LR_VIRTUAL_MASK;
-            spin_lock(&gic.lock);
-            p = irq_to_pending(v, irq);
-            if ( p->desc != NULL )
-                p->desc->status &= ~IRQ_INPROGRESS;
-            clear_bit(GIC_IRQ_GUEST_VISIBLE, &p->status);
-            p->lr = nr_lrs;
-            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, irq, p->priority);
-            }
-            spin_unlock(&gic.lock);
-            if ( !inflight )
-            {
-                spin_lock(&v->arch.vgic.lock);
-                list_del_init(&p->inflight);
-                spin_unlock(&v->arch.vgic.lock);
-            }
+            gic_add_to_lr_pending(v, irq, p->priority);
+        } else
+            list_del_init(&p->inflight);
 
-        }
+        spin_unlock(&gic.lock);
+    }
+}
+
+static void gic_clear_lrs(struct vcpu *v)
+{
+    int i = 0;
 
+    while ((i = find_next_bit((const long unsigned int *) &this_cpu(lr_mask),
+                              nr_lrs, i)) < nr_lrs) {
+
+        spin_lock(&v->arch.vgic.lock);
+        _gic_clear_lr(v, i);
+        spin_unlock(&v->arch.vgic.lock);
         i++;
     }
 }
 
+void gic_set_clear_lr(struct vcpu *v, struct pending_irq *p)
+{
+    _gic_clear_lr(v, p->lr);
+}
+
 static void gic_restore_pending_irqs(struct vcpu *v)
 {
     int i;
@@ -801,9 +814,6 @@ void gic_inject(void)
 {
     gic_clear_lrs(current);
 
-    if ( vcpu_info(current, evtchn_upcall_pending) )
-        vgic_vcpu_inject_irq(current, current->domain->arch.evtchn_irq);
-
     gic_restore_pending_irqs(current);
     if (!gic_events_need_delivery())
         gic_inject_irq_stop();
diff --git a/xen/arch/arm/vgic.c b/xen/arch/arm/vgic.c
index da15f4d..210ac39 100644
--- a/xen/arch/arm/vgic.c
+++ b/xen/arch/arm/vgic.c
@@ -387,7 +387,11 @@ static void vgic_enable_irqs(struct vcpu *v, uint32_t r, int n)
         irq = i + (32 * n);
         p = irq_to_pending(v, irq);
         set_bit(GIC_IRQ_GUEST_ENABLED, &p->status);
-        if ( !list_empty(&p->inflight) && !test_bit(GIC_IRQ_GUEST_VISIBLE, &p->status) )
+        if ( irq == v->domain->arch.evtchn_irq &&
+             vcpu_info(current, evtchn_upcall_pending) &&
+             list_empty(&p->inflight) )
+            vgic_vcpu_inject_irq(v, irq);
+        else if ( !list_empty(&p->inflight) && !test_bit(GIC_IRQ_GUEST_VISIBLE, &p->status) )
             gic_set_guest_irq(v, irq, GICH_LR_PENDING, p->priority);
         if ( p->desc != NULL )
             p->desc->handler->enable(p->desc);
@@ -696,10 +700,16 @@ void vgic_vcpu_inject_irq(struct vcpu *v, unsigned int irq)
 
     if ( !list_empty(&n->inflight) )
     {
-        if ( (irq != current->domain->arch.evtchn_irq) ||
-             (!test_bit(GIC_IRQ_GUEST_VISIBLE, &n->status)) )
+        if ( v == current )
+        {
+            set_bit(GIC_IRQ_GUEST_PENDING, &n->status);
+            gic_set_clear_lr(v, n);
+            spin_unlock_irqrestore(&v->arch.vgic.lock, flags);
+            return;
+        } else {
             set_bit(GIC_IRQ_GUEST_PENDING, &n->status);
-        goto out;
+            goto out;
+        }
     }
 
     /* vcpu offline */
diff --git a/xen/include/asm-arm/gic.h b/xen/include/asm-arm/gic.h
index 6fce5c2..6de0d9b 100644
--- a/xen/include/asm-arm/gic.h
+++ b/xen/include/asm-arm/gic.h
@@ -184,6 +184,7 @@ extern void gic_remove_from_queues(struct vcpu *v, unsigned int virtual_irq);
 extern int gic_route_irq_to_guest(struct domain *d,
                                   const struct dt_irq *irq,
                                   const char * devname);
+extern void gic_set_clear_lr(struct vcpu *v, struct pending_irq *p);
 
 /* Accept an interrupt from the GIC and dispatch its handler */
 extern void gic_interrupt(struct cpu_user_regs *regs, int is_fiq);
-- 
1.7.10.4

[PATCH-4.5 v2 07/10] xen/arm: don't protect GICH and lr_queue accesses with gic.lock

[Stefano Stabellini]

GICH is banked, protect accesses by disabling interrupts.
Protect lr_queue accesses with the vgic.lock only.
gic.lock only protects accesses to GICD now.

Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
---
 xen/arch/arm/gic.c  |   22 +++-------------------
 xen/arch/arm/vgic.c |   12 ++++++++++--
 2 files changed, 13 insertions(+), 21 deletions(-)

diff --git a/xen/arch/arm/gic.c b/xen/arch/arm/gic.c
index 0955d48..6386ccb 100644
--- a/xen/arch/arm/gic.c
+++ b/xen/arch/arm/gic.c
@@ -667,19 +667,14 @@ void gic_remove_from_queues(struct vcpu *v, unsigned int virtual_irq)
 {
     struct pending_irq *p = irq_to_pending(v, virtual_irq);
 
-    spin_lock(&gic.lock);
     if ( !list_empty(&p->lr_queue) )
         list_del_init(&p->lr_queue);
-    spin_unlock(&gic.lock);
 }
 
 void gic_set_guest_irq(struct vcpu *v, unsigned int irq,
         unsigned int state, unsigned int priority)
 {
     int i;
-    unsigned long flags;
-
-    spin_lock_irqsave(&gic.lock, flags);
 
     if ( v == current && list_empty(&v->arch.vgic.lr_pending) )
     {
@@ -687,15 +682,11 @@ void gic_set_guest_irq(struct vcpu *v, unsigned int irq,
         if (i < nr_lrs) {
             set_bit(i, &this_cpu(lr_mask));
             gic_set_lr(v, i, irq, state, priority);
-            goto out;
+            return;
         }
     }
 
     gic_add_to_lr_pending(v, irq, priority);
-
-out:
-    spin_unlock_irqrestore(&gic.lock, flags);
-    return;
 }
 
 static void _gic_clear_lr(struct vcpu *v, int i)
@@ -717,8 +708,6 @@ static void _gic_clear_lr(struct vcpu *v, int i)
     } else if ( lr & GICH_LR_PENDING ) {
         clear_bit(GIC_IRQ_GUEST_PENDING, &p->status);
     } else {
-        spin_lock(&gic.lock);
-
         GICH[GICH_LR + i] = 0;
         clear_bit(i, &this_cpu(lr_mask));
 
@@ -732,8 +721,6 @@ static void _gic_clear_lr(struct vcpu *v, int i)
             gic_add_to_lr_pending(v, irq, p->priority);
         } else
             list_del_init(&p->inflight);
-
-        spin_unlock(&gic.lock);
     }
 }
 
@@ -767,11 +754,11 @@ static void gic_restore_pending_irqs(struct vcpu *v)
         i = find_first_zero_bit(&this_cpu(lr_mask), nr_lrs);
         if ( i >= nr_lrs ) return;
 
-        spin_lock_irqsave(&gic.lock, flags);
+        spin_lock_irqsave(&v->arch.vgic.lock, flags);
         gic_set_lr(v, i, p->irq, GICH_LR_PENDING, p->priority);
         list_del_init(&p->lr_queue);
         set_bit(i, &this_cpu(lr_mask));
-        spin_unlock_irqrestore(&gic.lock, flags);
+        spin_unlock_irqrestore(&v->arch.vgic.lock, flags);
     }
 
 }
@@ -779,13 +766,10 @@ static void gic_restore_pending_irqs(struct vcpu *v)
 void gic_clear_pending_irqs(struct vcpu *v)
 {
     struct pending_irq *p, *t;
-    unsigned long flags;
 
-    spin_lock_irqsave(&gic.lock, flags);
     v->arch.lr_mask = 0;
     list_for_each_entry_safe ( p, t, &v->arch.vgic.lr_pending, lr_queue )
         list_del_init(&p->lr_queue);
-    spin_unlock_irqrestore(&gic.lock, flags);
 }
 
 static void gic_inject_irq_start(void)
diff --git a/xen/arch/arm/vgic.c b/xen/arch/arm/vgic.c
index 210ac39..4bfab26 100644
--- a/xen/arch/arm/vgic.c
+++ b/xen/arch/arm/vgic.c
@@ -365,12 +365,15 @@ static void vgic_disable_irqs(struct vcpu *v, uint32_t r, int n)
     struct pending_irq *p;
     unsigned int irq;
     int i = 0;
+    unsigned long flags;
 
     while ( (i = find_next_bit((const long unsigned int *) &r, 32, i)) < 32 ) {
         irq = i + (32 * n);
         p = irq_to_pending(v, irq);
+        spin_lock_irqsave(&v->arch.vgic.lock, flags);
         clear_bit(GIC_IRQ_GUEST_ENABLED, &p->status);
         gic_remove_from_queues(v, irq);
+        spin_unlock_irqrestore(&v->arch.vgic.lock, flags);
         if ( p->desc != NULL )
             p->desc->handler->disable(p->desc);
         i++;
@@ -391,8 +394,13 @@ static void vgic_enable_irqs(struct vcpu *v, uint32_t r, int n)
              vcpu_info(current, evtchn_upcall_pending) &&
              list_empty(&p->inflight) )
             vgic_vcpu_inject_irq(v, irq);
-        else if ( !list_empty(&p->inflight) && !test_bit(GIC_IRQ_GUEST_VISIBLE, &p->status) )
-            gic_set_guest_irq(v, irq, GICH_LR_PENDING, p->priority);
+        else {
+            unsigned long flags;
+            spin_lock_irqsave(&v->arch.vgic.lock, flags);
+            if ( !list_empty(&p->inflight) && !test_bit(GIC_IRQ_GUEST_VISIBLE, &p->status) )
+                gic_set_guest_irq(v, irq, GICH_LR_PENDING, p->priority);
+            spin_unlock_irqrestore(&v->arch.vgic.lock, flags);
+        }
         if ( p->desc != NULL )
             p->desc->handler->enable(p->desc);
         i++;
-- 
1.7.10.4

[PATCH-4.5 v2 08/10] xen/arm: avoid taking unconditionally the vgic.lock in gic_clear_lrs

[Stefano Stabellini]

We don't actually need to call _gic_clear_lr with the vgic.lock taken,
only in case the GICH_LR has to be free'ed we need the lock.
Add a boolean vgic_locked parameter to _gic_clear_lr, so that we can
avoid taking the lock if it has been already taken, or take the lock
only in the necessary case if it hasn't.

Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
---
 xen/arch/arm/gic.c |   10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/xen/arch/arm/gic.c b/xen/arch/arm/gic.c
index 6386ccb..54be9ca 100644
--- a/xen/arch/arm/gic.c
+++ b/xen/arch/arm/gic.c
@@ -689,7 +689,7 @@ void gic_set_guest_irq(struct vcpu *v, unsigned int irq,
     gic_add_to_lr_pending(v, irq, priority);
 }
 
-static void _gic_clear_lr(struct vcpu *v, int i)
+static void _gic_clear_lr(struct vcpu *v, int i, int vgic_locked)
 {
     int irq;
     uint32_t lr;
@@ -711,6 +711,7 @@ static void _gic_clear_lr(struct vcpu *v, int i)
         GICH[GICH_LR + i] = 0;
         clear_bit(i, &this_cpu(lr_mask));
 
+        if ( !vgic_locked ) spin_lock(&v->arch.vgic.lock);
         if ( p->desc != NULL )
             p->desc->status &= ~IRQ_INPROGRESS;
         clear_bit(GIC_IRQ_GUEST_VISIBLE, &p->status);
@@ -721,6 +722,7 @@ static void _gic_clear_lr(struct vcpu *v, int i)
             gic_add_to_lr_pending(v, irq, p->priority);
         } else
             list_del_init(&p->inflight);
+        if ( !vgic_locked ) spin_unlock(&v->arch.vgic.lock);
     }
 }
 
@@ -731,16 +733,14 @@ static void gic_clear_lrs(struct vcpu *v)
     while ((i = find_next_bit((const long unsigned int *) &this_cpu(lr_mask),
                               nr_lrs, i)) < nr_lrs) {
 
-        spin_lock(&v->arch.vgic.lock);
-        _gic_clear_lr(v, i);
-        spin_unlock(&v->arch.vgic.lock);
+        _gic_clear_lr(v, i, 0);
         i++;
     }
 }
 
 void gic_set_clear_lr(struct vcpu *v, struct pending_irq *p)
 {
-    _gic_clear_lr(v, p->lr);
+    _gic_clear_lr(v, p->lr, 1);
 }
 
 static void gic_restore_pending_irqs(struct vcpu *v)
-- 
1.7.10.4

[PATCH-4.5 v2 09/10] xen/arm: use GICH_ELSR[01] to avoid reading all the GICH_LRs in gic_clear_lrs

[Stefano Stabellini]

Read GICH_ELSR0 and GICH_ELSR1 to figure out which GICH_LR registers
do not contain valid interrupts. Only call _gic_clear_lr on those.

If a cpu is trying to inject an interrupt that is already inflight into
another cpu, it sets GIC_IRQ_GUEST_PENDING and sends an SGI to it.
The target cpu is going to be interrupted and _gic_clear_lr, called by
gic_clear_lrs, will take care of setting GICH_LR_PENDING if the irq is
active.
In order to make sure that _gic_clear_lr is called for this irq, avoid
filtering lr_mask with GICH_ELSR[01] in this case (so that in this
situation we call _gic_clear_lr on all the GICH_LRs). Use a simple
percpu bit, lr_clear_all, set by the sender cpu and reset by the
receiver cpu, to understand whether we need to evaluate all GICH_LRs or
we can filter them.

Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
---
 xen/arch/arm/gic.c        |   27 ++++++++++++++++++++++-----
 xen/arch/arm/vgic.c       |    1 +
 xen/include/asm-arm/gic.h |    1 +
 3 files changed, 24 insertions(+), 5 deletions(-)

diff --git a/xen/arch/arm/gic.c b/xen/arch/arm/gic.c
index 54be9ca..b00f77c 100644
--- a/xen/arch/arm/gic.c
+++ b/xen/arch/arm/gic.c
@@ -55,6 +55,7 @@ static struct {
 static irq_desc_t irq_desc[NR_IRQS];
 static DEFINE_PER_CPU(irq_desc_t[NR_LOCAL_IRQS], local_irq_desc);
 static DEFINE_PER_CPU(uint64_t, lr_mask);
+static DEFINE_PER_CPU(uint8_t, lr_clear_all);
 
 static unsigned nr_lrs;
 
@@ -67,7 +68,7 @@ static DEFINE_PER_CPU(u8, gic_cpu_id);
 /* Maximum cpu interface per GIC */
 #define NR_GIC_CPU_IF 8
 
-static void gic_clear_lrs(struct vcpu *v);
+static void gic_clear_lrs(struct vcpu *v, bool_t all);
 
 static unsigned int gic_cpu_mask(const cpumask_t *cpumask)
 {
@@ -109,6 +110,7 @@ void gic_save_state(struct vcpu *v)
         v->arch.gic_lr[i] = GICH[GICH_LR + i];
     v->arch.lr_mask = this_cpu(lr_mask);
     v->arch.gic_apr = GICH[GICH_APR];
+    this_cpu(lr_clear_all) = 0ULL;
     /* Disable until next VCPU scheduled */
     GICH[GICH_HCR] = 0;
     isb();
@@ -122,13 +124,14 @@ void gic_restore_state(struct vcpu *v)
         return;
 
     this_cpu(lr_mask) = v->arch.lr_mask;
+    this_cpu(lr_clear_all) = 0ULL;
     for ( i=0; i<nr_lrs; i++)
         GICH[GICH_LR + i] = v->arch.gic_lr[i];
     GICH[GICH_APR] = v->arch.gic_apr;
     GICH[GICH_HCR] = GICH_HCR_EN;
     isb();
 
-    gic_clear_lrs(v);
+    gic_clear_lrs(v, 1);
     gic_restore_pending_irqs(v);
 }
 
@@ -372,6 +375,7 @@ static void __cpuinit gic_hyp_init(void)
 
     GICH[GICH_MISR] = GICH_MISR_EOI;
     this_cpu(lr_mask) = 0ULL;
+    this_cpu(lr_clear_all) = 0ULL;
 }
 
 static void __cpuinit gic_hyp_disable(void)
@@ -726,11 +730,19 @@ static void _gic_clear_lr(struct vcpu *v, int i, int vgic_locked)
     }
 }
 
-static void gic_clear_lrs(struct vcpu *v)
+static void gic_clear_lrs(struct vcpu *v, bool_t all)
 {
     int i = 0;
+    uint64_t elsr;
+    
+    if ( !all )
+    {
+        elsr = GICH[GICH_ELSR0] | (((uint64_t) GICH[GICH_ELSR1]) << 32);
+        elsr &= this_cpu(lr_mask);
+    } else
+        elsr = this_cpu(lr_mask);
 
-    while ((i = find_next_bit((const long unsigned int *) &this_cpu(lr_mask),
+    while ((i = find_next_bit((const long unsigned int *) &elsr,
                               nr_lrs, i)) < nr_lrs) {
 
         _gic_clear_lr(v, i, 0);
@@ -743,6 +755,11 @@ void gic_set_clear_lr(struct vcpu *v, struct pending_irq *p)
     _gic_clear_lr(v, p->lr, 1);
 }
 
+void gic_set_clear_lrs_other(struct vcpu *v)
+{
+    set_bit(0, &per_cpu(lr_clear_all, v->processor));
+}
+
 static void gic_restore_pending_irqs(struct vcpu *v)
 {
     int i;
@@ -796,7 +813,7 @@ int gic_events_need_delivery(void)
 
 void gic_inject(void)
 {
-    gic_clear_lrs(current);
+    gic_clear_lrs(current, test_and_clear_bit(0, &this_cpu(lr_clear_all)));
 
     gic_restore_pending_irqs(current);
     if (!gic_events_need_delivery())
diff --git a/xen/arch/arm/vgic.c b/xen/arch/arm/vgic.c
index 4bfab26..02ad3cd 100644
--- a/xen/arch/arm/vgic.c
+++ b/xen/arch/arm/vgic.c
@@ -716,6 +716,7 @@ void vgic_vcpu_inject_irq(struct vcpu *v, unsigned int irq)
             return;
         } else {
             set_bit(GIC_IRQ_GUEST_PENDING, &n->status);
+            gic_set_clear_lrs_other(v);
             goto out;
         }
     }
diff --git a/xen/include/asm-arm/gic.h b/xen/include/asm-arm/gic.h
index 6de0d9b..8d36f7c 100644
--- a/xen/include/asm-arm/gic.h
+++ b/xen/include/asm-arm/gic.h
@@ -185,6 +185,7 @@ extern int gic_route_irq_to_guest(struct domain *d,
                                   const struct dt_irq *irq,
                                   const char * devname);
 extern void gic_set_clear_lr(struct vcpu *v, struct pending_irq *p);
+extern void gic_set_clear_lrs_other(struct vcpu *v);
 
 /* Accept an interrupt from the GIC and dispatch its handler */
 extern void gic_interrupt(struct cpu_user_regs *regs, int is_fiq);
-- 
1.7.10.4

[PATCH-4.5 v2 10/10] xen/arm: print more info in gic_dump_info, keep gic_lr sync'ed

[Stefano Stabellini]

For each inflight and pending irqs print GIC_IRQ_GUEST_ENABLED,
GIC_IRQ_GUEST_PENDING and GIC_IRQ_GUEST_VISIBLE.

In order to get consistent information from gic_dump_info, we need to
get the vgic.lock before walking the inflight and lr_pending lists.

We also need to keep v->arch.gic_lr in sync with GICH_LR registers.

Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
---
 xen/arch/arm/gic.c |   19 ++++++++++++++++---
 1 file changed, 16 insertions(+), 3 deletions(-)

diff --git a/xen/arch/arm/gic.c b/xen/arch/arm/gic.c
index b00f77c..af0994a 100644
--- a/xen/arch/arm/gic.c
+++ b/xen/arch/arm/gic.c
@@ -642,6 +642,7 @@ static inline void gic_set_lr(struct vcpu *v, int lr, unsigned int irq,
             ((p->desc->irq & GICH_LR_PHYSICAL_MASK) << GICH_LR_PHYSICAL_SHIFT);
 
     GICH[GICH_LR + lr] = lr_reg;
+    v->arch.gic_lr[lr] = lr_reg;
 
     set_bit(GIC_IRQ_GUEST_VISIBLE, &p->status);
     clear_bit(GIC_IRQ_GUEST_PENDING, &p->status);
@@ -708,11 +709,15 @@ static void _gic_clear_lr(struct vcpu *v, int i, int vgic_locked)
     {
         if ( test_bit(GIC_IRQ_GUEST_ENABLED, &p->status) &&
              test_and_clear_bit(GIC_IRQ_GUEST_PENDING, &p->status) )
+        {
             GICH[GICH_LR + i] = lr | GICH_LR_PENDING;
+            v->arch.gic_lr[i] = lr | GICH_LR_PENDING;
+        }
     } else if ( lr & GICH_LR_PENDING ) {
         clear_bit(GIC_IRQ_GUEST_PENDING, &p->status);
     } else {
         GICH[GICH_LR + i] = 0;
+        v->arch.gic_lr[i] = 0;
         clear_bit(i, &this_cpu(lr_mask));
 
         if ( !vgic_locked ) spin_lock(&v->arch.vgic.lock);
@@ -986,6 +991,8 @@ void gic_dump_info(struct vcpu *v)
     struct pending_irq *p;
 
     printk("GICH_LRs (vcpu %d) mask=%"PRIx64"\n", v->vcpu_id, v->arch.lr_mask);
+
+    spin_lock(&v->arch.vgic.lock);
     if ( v == current )
     {
         for ( i = 0; i < nr_lrs; i++ )
@@ -997,14 +1004,20 @@ void gic_dump_info(struct vcpu *v)
 
     list_for_each_entry ( p, &v->arch.vgic.inflight_irqs, inflight )
     {
-        printk("Inflight irq=%d lr=%u\n", p->irq, p->lr);
+        printk("Inflight irq=%d lr=%u enable=%d pending=%d visible=%d\n",
+                p->irq, p->lr, test_bit(GIC_IRQ_GUEST_ENABLED, &p->status),
+                test_bit(GIC_IRQ_GUEST_PENDING, &p->status),
+                test_bit(GIC_IRQ_GUEST_VISIBLE, &p->status));
     }
 
     list_for_each_entry( p, &v->arch.vgic.lr_pending, lr_queue )
     {
-        printk("Pending irq=%d lr=%u\n", p->irq, p->lr);
+        printk("Pending irq=%d lr=%u enable=%d pending=%d visible=%d\n",
+                p->irq, p->lr, test_bit(GIC_IRQ_GUEST_ENABLED, &p->status),
+                test_bit(GIC_IRQ_GUEST_PENDING, &p->status),
+                test_bit(GIC_IRQ_GUEST_VISIBLE, &p->status));
     }
-
+    spin_unlock(&v->arch.vgic.lock);
 }
 
 void __cpuinit init_maintenance_interrupt(void)
-- 
1.7.10.4

Re: [PATCH-4.5 v2 07/10] xen/arm: don't protect GICH and lr_queue accesses with gic.lock

[Julien Grall]

Hi Stefano,

On 02/14/2014 03:51 PM, Stefano Stabellini wrote:
> GICH is banked, protect accesses by disabling interrupts.
> Protect lr_queue accesses with the vgic.lock only.
> gic.lock only protects accesses to GICD now.
> 
> Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
> ---
>  xen/arch/arm/gic.c  |   22 +++-------------------
>  xen/arch/arm/vgic.c |   12 ++++++++++--
>  2 files changed, 13 insertions(+), 21 deletions(-)
> 
> diff --git a/xen/arch/arm/gic.c b/xen/arch/arm/gic.c
> index 0955d48..6386ccb 100644
> --- a/xen/arch/arm/gic.c
> +++ b/xen/arch/arm/gic.c
> @@ -667,19 +667,14 @@ void gic_remove_from_queues(struct vcpu *v, unsigned int virtual_irq)
>  {
>      struct pending_irq *p = irq_to_pending(v, virtual_irq);
>  
> -    spin_lock(&gic.lock);
>      if ( !list_empty(&p->lr_queue) )
>          list_del_init(&p->lr_queue);
> -    spin_unlock(&gic.lock);
>  }

This patch doesn't apply correctly on the latest master. The commit
0ddaeff has replace spin_lock by spin_lock_irqsave.

You need to rebase your patches series at least on top of this patch.

Regards,

-- 
Julien Grall

Re: [PATCH-4.5 v2 02/10] xen/arm: support HW interrupts in gic_set_lr

[Julien Grall]

On 02/14/2014 03:51 PM, Stefano Stabellini wrote:
> If the irq to be injected is an hardware irq (p->desc != NULL), set
> GICH_LR_HW.
> 
> Remove the code to EOI a physical interrupt on behalf of the guest
> because it has become unnecessary.
> 
> Also add a struct vcpu* parameter to gic_set_lr.
> 
> Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>

IRL you told me that this patch as dependency on another. It would be
nice to mention this dependency in the commit message for bisection.

> ---
> 
> Changes in v2:
> - remove the EOI code, now unnecessary;
> - do not assume physical IRQ == virtual IRQ;
> - refactor gic_set_lr.
> ---
>  xen/arch/arm/gic.c |   52 +++++++++++++++++-----------------------------------
>  1 file changed, 17 insertions(+), 35 deletions(-)
> 
> diff --git a/xen/arch/arm/gic.c b/xen/arch/arm/gic.c
> index acf7195..64c8aa7 100644
> --- a/xen/arch/arm/gic.c
> +++ b/xen/arch/arm/gic.c
> @@ -618,20 +618,24 @@ int __init setup_dt_irq(const struct dt_irq *irq, struct irqaction *new)
>      return rc;
>  }
>  
> -static inline void gic_set_lr(int lr, unsigned int virtual_irq,
> +static inline void gic_set_lr(struct vcpu *v, int lr, unsigned int irq,
>          unsigned int state, unsigned int priority)
>  {
> -    int maintenance_int = GICH_LR_MAINTENANCE_IRQ;
> -    struct pending_irq *p = irq_to_pending(current, virtual_irq);
> +    struct pending_irq *p = irq_to_pending(v, irq);
> +    uint32_t lr_reg;
>  
>      BUG_ON(lr >= nr_lrs);
>      BUG_ON(lr < 0);
>      BUG_ON(state & ~(GICH_LR_STATE_MASK<<GICH_LR_STATE_SHIFT));
>  
> -    GICH[GICH_LR + lr] = state |
> -        maintenance_int |
> +    lr_reg = state | GICH_LR_MAINTENANCE_IRQ |
>          ((priority >> 3) << GICH_LR_PRIORITY_SHIFT) |
> -        ((virtual_irq & GICH_LR_VIRTUAL_MASK) << GICH_LR_VIRTUAL_SHIFT);
> +        ((irq & GICH_LR_VIRTUAL_MASK) << GICH_LR_VIRTUAL_SHIFT);
> +    if ( p->desc != NULL )
> +        lr_reg |= GICH_LR_HW |
> +            ((p->desc->irq & GICH_LR_PHYSICAL_MASK) << GICH_LR_PHYSICAL_SHIFT);
> +
> +    GICH[GICH_LR + lr] = lr_reg;
>  
>      set_bit(GIC_IRQ_GUEST_VISIBLE, &p->status);
>      clear_bit(GIC_IRQ_GUEST_PENDING, &p->status);
> @@ -666,7 +670,7 @@ void gic_remove_from_queues(struct vcpu *v, unsigned int virtual_irq)
>      spin_unlock(&gic.lock);
>  }
>  
> -void gic_set_guest_irq(struct vcpu *v, unsigned int virtual_irq,
> +void gic_set_guest_irq(struct vcpu *v, unsigned int irq,
>          unsigned int state, unsigned int priority)
>  {
>      int i;
> @@ -679,12 +683,12 @@ void gic_set_guest_irq(struct vcpu *v, unsigned int virtual_irq,
>          i = find_first_zero_bit(&this_cpu(lr_mask), nr_lrs);
>          if (i < nr_lrs) {
>              set_bit(i, &this_cpu(lr_mask));
> -            gic_set_lr(i, virtual_irq, state, priority);
> +            gic_set_lr(v, i, irq, state, priority);
>              goto out;
>          }
>      }
>  
> -    gic_add_to_lr_pending(v, virtual_irq, priority);
> +    gic_add_to_lr_pending(v, irq, priority);
>  
>  out:
>      spin_unlock_irqrestore(&gic.lock, flags);
> @@ -703,7 +707,7 @@ static void gic_restore_pending_irqs(struct vcpu *v)
>          if ( i >= nr_lrs ) return;
>  
>          spin_lock_irqsave(&gic.lock, flags);
> -        gic_set_lr(i, p->irq, GICH_LR_PENDING, p->priority);
> +        gic_set_lr(v, i, p->irq, GICH_LR_PENDING, p->priority);
>          list_del_init(&p->lr_queue);
>          set_bit(i, &this_cpu(lr_mask));
>          spin_unlock_irqrestore(&gic.lock, flags);
> @@ -904,15 +908,9 @@ int gicv_setup(struct domain *d)
>  
>  }
>  
> -static void gic_irq_eoi(void *info)
> -{
> -    int virq = (uintptr_t) info;
> -    GICC[GICC_DIR] = virq;
> -}
> -
>  static void maintenance_interrupt(int irq, void *dev_id, struct cpu_user_regs *regs)
>  {
> -    int i = 0, virq, pirq = -1;
> +    int i = 0, virq;
>      uint32_t lr;
>      struct vcpu *v = current;
>      uint64_t eisr = GICH[GICH_EISR0] | (((uint64_t) GICH[GICH_EISR1]) << 32);
> @@ -920,10 +918,8 @@ 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, *p2;
> -        int cpu;
>          bool_t inflight;
>  
> -        cpu = -1;
>          inflight = 0;
>  
>          spin_lock_irq(&gic.lock);
> @@ -933,12 +929,8 @@ static void maintenance_interrupt(int irq, void *dev_id, struct cpu_user_regs *r
>          clear_bit(i, &this_cpu(lr_mask));
>  
>          p = irq_to_pending(v, virq);
> -        if ( p->desc != NULL ) {
> +        if ( p->desc != NULL )
>              p->desc->status &= ~IRQ_INPROGRESS;
> -            /* Assume only one pcpu needs to EOI the irq */
> -            cpu = p->desc->arch.eoi_cpu;
> -            pirq = p->desc->irq;
> -        }
>          if ( test_bit(GIC_IRQ_GUEST_PENDING, &p->status) &&
>               test_bit(GIC_IRQ_GUEST_ENABLED, &p->status))
>          {
> @@ -950,7 +942,7 @@ static void maintenance_interrupt(int irq, void *dev_id, struct cpu_user_regs *r
>  
>          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);
> +            gic_set_lr(v, i, p2->irq, GICH_LR_PENDING, p2->priority);
>              list_del_init(&p2->lr_queue);
>              set_bit(i, &this_cpu(lr_mask));
>          }
> @@ -963,16 +955,6 @@ static void maintenance_interrupt(int irq, void *dev_id, struct cpu_user_regs *r
>              spin_unlock_irq(&v->arch.vgic.lock);
>          }
>  
> -        if ( p->desc != NULL ) {
> -            /* 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*)(uintptr_t)pirq);
> -            else
> -                on_selected_cpus(cpumask_of(cpu),
> -                                 gic_irq_eoi, (void*)(uintptr_t)pirq, 0);
> -        }
> -
>          i++;
>      }
>  }
> 


-- 
Julien Grall

Re: [PATCH-4.5 v2 0/10] remove maintenance interrupts

[Julien Grall]

Hi Stefano,

On 14/02/14 15:50, Stefano Stabellini wrote:
> Hi all,
> this patch series removes any needs for maintenance interrupts for both
> hardware and software interrupts in Xen.
> It achieves the goal by using the GICH_LR_HW bit for hardware interrupts
> and by checking the status of the GICH_LR registers on return to guest,
> clearing the registers that are invalid and handling the lifecycle of
> the corresponding interrupts in Xen data structures.

To keep track here, I have tried the patch series on top of the latest 
Xen. Booting Xen and Dom0 is slower with this patch series.

-- 
Julien Grall

Re: [PATCH-4.5 v2 10/10] xen/arm: print more info in gic_dump_info, keep gic_lr sync'ed

[Julien Grall]

Hi Stefano,

On 14/02/14 15:51, Stefano Stabellini wrote:
> @@ -986,6 +991,8 @@ void gic_dump_info(struct vcpu *v)
>       struct pending_irq *p;
>
>       printk("GICH_LRs (vcpu %d) mask=%"PRIx64"\n", v->vcpu_id, v->arch.lr_mask);
> +
> +    spin_lock(&v->arch.vgic.lock);

interrupts need to be disabled here. If not, it's possible to receive an 
interrupt while the lock is taken. Xen might deadlock if the interrupt 
is inject to this vcpu "v" (see vgic_vcpu_inject_irq).

Regards,

-- 
Julien Grall

Re: [PATCH-4.5 v2 0/10] remove maintenance interrupts

[Ian Campbell]

On Fri, 2014-02-14 at 15:50 +0000, Stefano Stabellini wrote:
> Hi all,
> this patch series removes any needs for maintenance interrupts for both
> hardware and software interrupts in Xen.

I tried this on Xgene and it fails to boot, apparently it's not seeing
any SATA interrupts (the logs are uninteresting I think, all looks like
SATA failures).

bisecting fingers:

8916c25c9f4fc0d71dc3ac5dcb28b68bf4effb4e is the first bad commit
commit 8916c25c9f4fc0d71dc3ac5dcb28b68bf4effb4e
Author: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
Date:   Wed Feb 12 17:48:10 2014 +0000

    xen/arm: support HW interrupts in gic_set_lr
    
    If the irq to be injected is an hardware irq (p->desc != NULL), set
    GICH_LR_HW.
    
    Remove the code to EOI a physical interrupt on behalf of the guest
    because it has become unnecessary.
    
    Also add a struct vcpu* parameter to gic_set_lr.
    
    Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
    
    ---
    
    Changes in v2:
    - remove the EOI code, now unnecessary;
    - do not assume physical IRQ == virtual IRQ;
    - refactor gic_set_lr.

:040000 040000 9647c168e6e7d81fd57f70c8519d1c4bbee7d33c 93344e9ddb4fa2da3c0e578bc3ebfbcf153ad5a1 M	xen


$ git bisect log
git bisect start
# good: [feee1ace547cf6247a358d082dd64fa762be2488] Merge branch 'master' into staging
git bisect good feee1ace547cf6247a358d082dd64fa762be2488
# bad: [739a2ff8910dac953c3adebddebfe621b537fab4] Merge branch 'no_maintenance_interrupts-v2' of git://xenbits.xen.org/people/sstabellini/xen-unstable into no-maint-irq
git bisect bad 739a2ff8910dac953c3adebddebfe621b537fab4
# bad: [c94edf5af6962aba8840fe03529717d359781ae7] xen/arm: keep track of the GICH_LR used for the irq in struct pending_irq
git bisect bad c94edf5af6962aba8840fe03529717d359781ae7
# bad: [8916c25c9f4fc0d71dc3ac5dcb28b68bf4effb4e] xen/arm: support HW interrupts in gic_set_lr
git bisect bad 8916c25c9f4fc0d71dc3ac5dcb28b68bf4effb4e
# good: [38b0d97d407d33d9eeb26e310daf25119867a943] xen/arm: remove unused virtual parameter from vgic_vcpu_inject_irq
git bisect good 38b0d97d407d33d9eeb26e310daf25119867a943

Re: [PATCH-4.5 v2 01/10] xen/arm: remove unused virtual parameter from vgic_vcpu_inject_irq

[Ian Campbell]

On Fri, 2014-02-14 at 15:51 +0000, Stefano Stabellini wrote:
> Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
> Acked-by: Julien Grall <julien.grall@linaro.org>

Acked-by: Ian Campbell <ian.campbell@citrix.com>

Re: [PATCH-4.5 v2 02/10] xen/arm: support HW interrupts in gic_set_lr

[Ian Campbell]

On Fri, 2014-02-14 at 17:49 +0000, Julien Grall wrote:
> On 02/14/2014 03:51 PM, Stefano Stabellini wrote:
> > If the irq to be injected is an hardware irq (p->desc != NULL), set
> > GICH_LR_HW.
> > 
> > Remove the code to EOI a physical interrupt on behalf of the guest
> > because it has become unnecessary.
> > 
> > Also add a struct vcpu* parameter to gic_set_lr.
> > 
> > Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
> 
> IRL you told me that this patch as dependency on another. It would be
> nice to mention this dependency in the commit message for bisection.

As in bisection is broken and needs manual intervention? Please don't do
that, reorder it or resplit things to make the issue go away.

The patch itself looks ok to me, but I won't ack until this is explained
or resolved.

Ian.

Re: [PATCH-4.5 v2 0/10] remove maintenance interrupts

[Ian Campbell]

On Sun, 2014-02-16 at 20:23 +0000, Julien Grall wrote:
> Hi Stefano,
> 
> On 14/02/14 15:50, Stefano Stabellini wrote:
> > Hi all,
> > this patch series removes any needs for maintenance interrupts for both
> > hardware and software interrupts in Xen.
> > It achieves the goal by using the GICH_LR_HW bit for hardware interrupts
> > and by checking the status of the GICH_LR registers on return to guest,
> > clearing the registers that are invalid and handling the lifecycle of
> > the corresponding interrupts in Xen data structures.
> 
> To keep track here, I have tried the patch series on top of the latest 
> Xen. Booting Xen and Dom0 is slower with this patch series.

Presumably lots slower or you wouldn't have noticed.

Stefano, I started reviewing before I saw this -- should I continue or
is it worth waiting for a v3?

Ian.

Re: [PATCH-4.5 v2 0/10] remove maintenance interrupts

[Ian Campbell]

On Tue, 2014-03-18 at 16:02 +0000, Ian Campbell wrote:
> On Sun, 2014-02-16 at 20:23 +0000, Julien Grall wrote:
> > Hi Stefano,
> > 
> > On 14/02/14 15:50, Stefano Stabellini wrote:
> > > Hi all,
> > > this patch series removes any needs for maintenance interrupts for both
> > > hardware and software interrupts in Xen.
> > > It achieves the goal by using the GICH_LR_HW bit for hardware interrupts
> > > and by checking the status of the GICH_LR registers on return to guest,
> > > clearing the registers that are invalid and handling the lifecycle of
> > > the corresponding interrupts in Xen data structures.
> > 
> > To keep track here, I have tried the patch series on top of the latest 
> > Xen. Booting Xen and Dom0 is slower with this patch series.
> 
> Presumably lots slower or you wouldn't have noticed.
> 
> Stefano, I started reviewing before I saw this -- should I continue or
> is it worth waiting for a v3?

Julien tells me there was a v3, but if I received it then I have
misfiled it because I can't find it anywhere. Would you mind resending?
Sorry for the hassle.

Ian.

Re: [PATCH-4.5 v2 0/10] remove maintenance interrupts

[Stefano Stabellini]

On Tue, 18 Mar 2014, Ian Campbell wrote:
> On Sun, 2014-02-16 at 20:23 +0000, Julien Grall wrote:
> > Hi Stefano,
> > 
> > On 14/02/14 15:50, Stefano Stabellini wrote:
> > > Hi all,
> > > this patch series removes any needs for maintenance interrupts for both
> > > hardware and software interrupts in Xen.
> > > It achieves the goal by using the GICH_LR_HW bit for hardware interrupts
> > > and by checking the status of the GICH_LR registers on return to guest,
> > > clearing the registers that are invalid and handling the lifecycle of
> > > the corresponding interrupts in Xen data structures.
> > 
> > To keep track here, I have tried the patch series on top of the latest 
> > Xen. Booting Xen and Dom0 is slower with this patch series.
> 
> Presumably lots slower or you wouldn't have noticed.
> 
> Stefano, I started reviewing before I saw this -- should I continue or
> is it worth waiting for a v3?

I have already sent a v3 and the issue is fixed there:

alpine.DEB.2.02.1402261517510.31489@kaball.uk.xensource.com

Re: [PATCH-4.5 v2 0/10] remove maintenance interrupts

[Stefano Stabellini]

On Tue, 18 Mar 2014, Ian Campbell wrote:
> On Tue, 2014-03-18 at 16:02 +0000, Ian Campbell wrote:
> > On Sun, 2014-02-16 at 20:23 +0000, Julien Grall wrote:
> > > Hi Stefano,
> > > 
> > > On 14/02/14 15:50, Stefano Stabellini wrote:
> > > > Hi all,
> > > > this patch series removes any needs for maintenance interrupts for both
> > > > hardware and software interrupts in Xen.
> > > > It achieves the goal by using the GICH_LR_HW bit for hardware interrupts
> > > > and by checking the status of the GICH_LR registers on return to guest,
> > > > clearing the registers that are invalid and handling the lifecycle of
> > > > the corresponding interrupts in Xen data structures.
> > > 
> > > To keep track here, I have tried the patch series on top of the latest 
> > > Xen. Booting Xen and Dom0 is slower with this patch series.
> > 
> > Presumably lots slower or you wouldn't have noticed.
> > 
> > Stefano, I started reviewing before I saw this -- should I continue or
> > is it worth waiting for a v3?
> 
> Julien tells me there was a v3, but if I received it then I have
> misfiled it because I can't find it anywhere. Would you mind resending?
> Sorry for the hassle.

That's OK, I plan to send a v4 soon rebased on Xen 4.4

Re: [PATCH-4.5 v2 02/10] xen/arm: support HW interrupts in gic_set_lr

[Stefano Stabellini]

On Tue, 18 Mar 2014, Ian Campbell wrote:
> On Fri, 2014-02-14 at 17:49 +0000, Julien Grall wrote:
> > On 02/14/2014 03:51 PM, Stefano Stabellini wrote:
> > > If the irq to be injected is an hardware irq (p->desc != NULL), set
> > > GICH_LR_HW.
> > > 
> > > Remove the code to EOI a physical interrupt on behalf of the guest
> > > because it has become unnecessary.
> > > 
> > > Also add a struct vcpu* parameter to gic_set_lr.
> > > 
> > > Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
> > 
> > IRL you told me that this patch as dependency on another. It would be
> > nice to mention this dependency in the commit message for bisection.
> 
> As in bisection is broken and needs manual intervention? Please don't do
> that, reorder it or resplit things to make the issue go away.
> 
> The patch itself looks ok to me, but I won't ack until this is explained
> or resolved.

Patch #2 and #3 should really be applied together, I separated them out
just for the sake of readibility. The reason is that you can't receive
maintenance interrupts for hw interrupts, so in order for this to work
properly we need the following patch "do not request
maintenance_interrupts".

I can send them as a single patch next time, but it would be harder to
review.

Re: [PATCH-4.5 v2 02/10] xen/arm: support HW interrupts in gic_set_lr

[Ian Campbell]

On Tue, 2014-03-18 at 17:38 +0000, Stefano Stabellini wrote:
> On Tue, 18 Mar 2014, Ian Campbell wrote:
> > On Fri, 2014-02-14 at 17:49 +0000, Julien Grall wrote:
> > > On 02/14/2014 03:51 PM, Stefano Stabellini wrote:
> > > > If the irq to be injected is an hardware irq (p->desc != NULL), set
> > > > GICH_LR_HW.
> > > > 
> > > > Remove the code to EOI a physical interrupt on behalf of the guest
> > > > because it has become unnecessary.
> > > > 
> > > > Also add a struct vcpu* parameter to gic_set_lr.
> > > > 
> > > > Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
> > > 
> > > IRL you told me that this patch as dependency on another. It would be
> > > nice to mention this dependency in the commit message for bisection.
> > 
> > As in bisection is broken and needs manual intervention? Please don't do
> > that, reorder it or resplit things to make the issue go away.
> > 
> > The patch itself looks ok to me, but I won't ack until this is explained
> > or resolved.
> 
> Patch #2 and #3 should really be applied together, I separated them out
> just for the sake of readibility. The reason is that you can't receive
> maintenance interrupts for hw interrupts, so in order for this to work
> properly we need the following patch "do not request
> maintenance_interrupts".
> 
> I can send them as a single patch next time, but it would be harder to
> review.

I think it'll be ok and/or we'll have to live with it.


Ian.