[PATCH-4.5 v3 0/12] remove maintenance interrupts

[PATCH-4.5 v3 0/12] 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.
It also improves priority handling, keeping the highest priority
outstanding interrupts in the GICH_LR registers.


Changes in v3:
- add "no need to set HCR_VI when using the vgic to inject irqs";
- add "s/gic_set_guest_irq/gic_raise_guest_irq";
- add "xen/arm: call gic_clear_lrs on entry to the hypervisor";
- do not use the PENDING and ACTIVE state for HW interrupts;
- unify the inflight and non-inflight code paths in
vgic_vcpu_inject_irq;
- remove "avoid taking unconditionally the vgic.lock in gic_clear_lrs";
- add "xen/arm: gic_events_need_delivery and irq priorities";
- use spin_lock_irqsave and spin_unlock_irqrestore in gic_dump_info.

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 (12):
      xen/arm: no need to set HCR_VI when using the vgic to inject irqs
      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_UIE 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: s/gic_set_guest_irq/gic_raise_guest_irq
      xen/arm: call gic_clear_lrs on entry to the hypervisor
      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: gic_events_need_delivery and irq priorities
      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           |  281 ++++++++++++++++++++++++------------------
 xen/arch/arm/irq.c           |    2 +-
 xen/arch/arm/time.c          |    2 +-
 xen/arch/arm/traps.c         |   10 ++
 xen/arch/arm/vgic.c          |   50 ++++----
 xen/arch/arm/vtimer.c        |    4 +-
 xen/include/asm-arm/domain.h |    6 +-
 xen/include/asm-arm/gic.h    |   10 +-
 9 files changed, 214 insertions(+), 153 deletions(-)


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

[PATCH-4.5 v3 01/12] xen/arm: no need to set HCR_VI when using the vgic to inject irqs

[Stefano Stabellini]

It can actually cause spurious interrupt injections into the guest.

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

diff --git a/xen/arch/arm/gic.c b/xen/arch/arm/gic.c
index 1484e1f..f860194 100644
--- a/xen/arch/arm/gic.c
+++ b/xen/arch/arm/gic.c
@@ -725,22 +725,6 @@ void gic_clear_pending_irqs(struct vcpu *v)
     spin_unlock_irqrestore(&gic.lock, flags);
 }
 
-static void gic_inject_irq_start(void)
-{
-    register_t hcr = READ_SYSREG(HCR_EL2);
-    WRITE_SYSREG(hcr | HCR_VI, HCR_EL2);
-    isb();
-}
-
-static void gic_inject_irq_stop(void)
-{
-    register_t hcr = READ_SYSREG(HCR_EL2);
-    if (hcr & HCR_VI) {
-        WRITE_SYSREG(hcr & ~HCR_VI, HCR_EL2);
-        isb();
-    }
-}
-
 int gic_events_need_delivery(void)
 {
     return (!list_empty(&current->arch.vgic.lr_pending) ||
@@ -753,10 +737,6 @@ void gic_inject(void)
         vgic_vcpu_inject_irq(current, current->domain->arch.evtchn_irq, 1);
 
     gic_restore_pending_irqs(current);
-    if (!gic_events_need_delivery())
-        gic_inject_irq_stop();
-    else
-        gic_inject_irq_start();
 }
 
 int gic_route_irq_to_guest(struct domain *d, const struct dt_irq *irq,
-- 
1.7.10.4

[PATCH-4.5 v3 02/12] 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 f860194..6f27630 100644
--- a/xen/arch/arm/gic.c
+++ b/xen/arch/arm/gic.c
@@ -734,7 +734,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);
 }
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 v3 03/12] 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.

This patch needs the following patch to work correctly. It has been sent
separately to make it easier to review.

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 6f27630..fd42922 100644
--- a/xen/arch/arm/gic.c
+++ b/xen/arch/arm/gic.c
@@ -620,20 +620,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);
@@ -668,7 +672,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;
@@ -681,12 +685,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);
@@ -705,7 +709,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);
@@ -886,15 +890,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);
@@ -902,10 +900,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);
@@ -915,12 +911,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))
         {
@@ -932,7 +924,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));
         }
@@ -945,16 +937,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 v3 04/12] 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).

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 fd42922..15e5f91 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;
@@ -128,6 +130,7 @@ void gic_restore_state(struct vcpu *v)
     GICH[GICH_HCR] = GICH_HCR_EN;
     isb();
 
+    gic_clear_lrs(v);
     gic_restore_pending_irqs(v);
 }
 
@@ -630,8 +633,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 |
@@ -697,6 +699,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_set_guest_irq(v, irq, GICH_LR_PENDING, 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;
@@ -737,6 +783,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);
 
@@ -892,53 +940,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 v3 05/12] 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 |    6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/xen/arch/arm/gic.c b/xen/arch/arm/gic.c
index 15e5f91..371ebd8 100644
--- a/xen/arch/arm/gic.c
+++ b/xen/arch/arm/gic.c
@@ -789,6 +789,12 @@ void gic_inject(void)
         vgic_vcpu_inject_irq(current, current->domain->arch.evtchn_irq);
 
     gic_restore_pending_irqs(current);
+
+    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 v3 06/12] 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 371ebd8..9293c16 100644
--- a/xen/arch/arm/gic.c
+++ b/xen/arch/arm/gic.c
@@ -643,6 +643,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,
@@ -723,6 +724,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))
             {
@@ -965,12 +967,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 v3 07/12] xen/arm: s/gic_set_guest_irq/gic_raise_guest_irq

[Stefano Stabellini]

Rename gic_set_guest_irq to gic_raise_guest_irq and remove the state
parameter.

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

diff --git a/xen/arch/arm/gic.c b/xen/arch/arm/gic.c
index 9293c16..d1e7ed3 100644
--- a/xen/arch/arm/gic.c
+++ b/xen/arch/arm/gic.c
@@ -675,8 +675,8 @@ 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 irq,
-        unsigned int state, unsigned int priority)
+void gic_raise_guest_irq(struct vcpu *v, unsigned int irq,
+                         unsigned int priority)
 {
     int i;
     unsigned long flags;
@@ -688,7 +688,7 @@ void gic_set_guest_irq(struct vcpu *v, unsigned int 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(v, i, irq, state, priority);
+            gic_set_lr(v, i, irq, GICH_LR_PENDING, priority);
             goto out;
         }
     }
@@ -729,7 +729,7 @@ static void gic_clear_lrs(struct vcpu *v)
                     test_bit(GIC_IRQ_GUEST_ENABLED, &p->status))
             {
                 inflight = 1;
-                gic_set_guest_irq(v, irq, GICH_LR_PENDING, p->priority);
+                gic_raise_guest_irq(v, irq, p->priority);
             }
             spin_unlock(&gic.lock);
             if ( !inflight )
diff --git a/xen/arch/arm/vgic.c b/xen/arch/arm/vgic.c
index da15f4d..b003f29 100644
--- a/xen/arch/arm/vgic.c
+++ b/xen/arch/arm/vgic.c
@@ -388,7 +388,7 @@ static void vgic_enable_irqs(struct vcpu *v, uint32_t r, int 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) )
-            gic_set_guest_irq(v, irq, GICH_LR_PENDING, p->priority);
+            gic_raise_guest_irq(v, irq, p->priority);
         if ( p->desc != NULL )
             p->desc->handler->enable(p->desc);
         i++;
@@ -717,7 +717,7 @@ void vgic_vcpu_inject_irq(struct vcpu *v, unsigned int irq)
 
     /* the irq is enabled */
     if ( test_bit(GIC_IRQ_GUEST_ENABLED, &n->status) )
-        gic_set_guest_irq(v, irq, GICH_LR_PENDING, priority);
+        gic_raise_guest_irq(v, irq, priority);
 
     list_for_each_entry ( iter, &v->arch.vgic.inflight_irqs, inflight )
     {
diff --git a/xen/include/asm-arm/gic.h b/xen/include/asm-arm/gic.h
index 6fce5c2..4834cd6 100644
--- a/xen/include/asm-arm/gic.h
+++ b/xen/include/asm-arm/gic.h
@@ -178,8 +178,8 @@ extern void gic_clear_pending_irqs(struct vcpu *v);
 extern int gic_events_need_delivery(void);
 
 extern void __cpuinit init_maintenance_interrupt(void);
-extern void gic_set_guest_irq(struct vcpu *v, unsigned int irq,
-        unsigned int state, unsigned int priority);
+extern void gic_raise_guest_irq(struct vcpu *v, unsigned int irq,
+        unsigned int priority);
 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,
-- 
1.7.10.4

[PATCH-4.5 v3 08/12] xen/arm: call gic_clear_lrs on entry to the hypervisor

[Stefano Stabellini]

This change is needed by other patches later on. It is going to make
sure that the calculation in Xen of the highest priority interrupt
currently inflight is correct and accurate and not based on stale data.

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

diff --git a/xen/arch/arm/gic.c b/xen/arch/arm/gic.c
index d1e7ed3..0e429c8 100644
--- a/xen/arch/arm/gic.c
+++ b/xen/arch/arm/gic.c
@@ -67,8 +67,6 @@ 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;
@@ -130,7 +128,6 @@ void gic_restore_state(struct vcpu *v)
     GICH[GICH_HCR] = GICH_HCR_EN;
     isb();
 
-    gic_clear_lrs(v);
     gic_restore_pending_irqs(v);
 }
 
@@ -700,14 +697,15 @@ out:
     return;
 }
 
-static void gic_clear_lrs(struct vcpu *v)
+void gic_clear_lrs(struct vcpu *v)
 {
     struct pending_irq *p;
     int i = 0, irq;
     uint32_t lr;
     bool_t inflight;
+    unsigned long flags;
 
-    ASSERT(!local_irq_is_enabled());
+    spin_lock_irqsave(&v->arch.vgic.lock, flags);
 
     while ((i = find_next_bit((const long unsigned int *) &this_cpu(lr_mask),
                               nr_lrs, i)) < nr_lrs) {
@@ -743,6 +741,8 @@ static void gic_clear_lrs(struct vcpu *v)
 
         i++;
     }
+
+    spin_unlock_irqrestore(&v->arch.vgic.lock, flags);
 }
 
 static void gic_restore_pending_irqs(struct vcpu *v)
@@ -785,8 +785,6 @@ 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);
 
diff --git a/xen/arch/arm/traps.c b/xen/arch/arm/traps.c
index ea77cb8..7663114 100644
--- a/xen/arch/arm/traps.c
+++ b/xen/arch/arm/traps.c
@@ -70,6 +70,7 @@ static int debug_stack_lines = 40;
 
 integer_param("debug_stack_lines", debug_stack_lines);
 
+static void enter_hypervisor_head(void);
 
 void __cpuinit init_traps(void)
 {
@@ -1701,6 +1702,8 @@ asmlinkage void do_trap_hypervisor(struct cpu_user_regs *regs)
 {
     union hsr hsr = { .bits = READ_SYSREG32(ESR_EL2) };
 
+    enter_hypervisor_head();
+
     switch (hsr.ec) {
     case HSR_EC_WFI_WFE:
         if ( !check_conditional_instr(regs, hsr) )
@@ -1778,11 +1781,13 @@ asmlinkage void do_trap_hypervisor(struct cpu_user_regs *regs)
 
 asmlinkage void do_trap_irq(struct cpu_user_regs *regs)
 {
+    enter_hypervisor_head();
     gic_interrupt(regs, 0);
 }
 
 asmlinkage void do_trap_fiq(struct cpu_user_regs *regs)
 {
+    enter_hypervisor_head();
     gic_interrupt(regs, 1);
 }
 
@@ -1800,6 +1805,11 @@ asmlinkage void leave_hypervisor_tail(void)
     }
 }
 
+static void enter_hypervisor_head(void)
+{
+    gic_clear_lrs(current);
+}
+
 /*
  * Local variables:
  * mode: C
diff --git a/xen/include/asm-arm/gic.h b/xen/include/asm-arm/gic.h
index 4834cd6..5a9dc77 100644
--- a/xen/include/asm-arm/gic.h
+++ b/xen/include/asm-arm/gic.h
@@ -220,6 +220,7 @@ extern unsigned int gic_number_lines(void);
 /* IRQ translation function for the device tree */
 int gic_irq_xlate(const u32 *intspec, unsigned int intsize,
                   unsigned int *out_hwirq, unsigned int *out_type);
+void gic_clear_lrs(struct vcpu *v);
 
 #endif /* __ASSEMBLY__ */
 #endif
-- 
1.7.10.4

[PATCH-4.5 v3 09/12] 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.

Unify the inflight and non-inflight code paths in vgic_vcpu_inject_irq.

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>

---
Changes in v3:
- do not use the PENDING and ACTIVE state for HW interrupts,
- check that p->lr is valid in gic_set_clear_lr.
---
 xen/arch/arm/gic.c  |   89 +++++++++++++++++++++++++++++----------------------
 xen/arch/arm/vgic.c |   33 ++++++++++---------
 2 files changed, 68 insertions(+), 54 deletions(-)

diff --git a/xen/arch/arm/gic.c b/xen/arch/arm/gic.c
index 0e429c8..2dc6386 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_lr(struct vcpu *v, int i);
+
 static unsigned int gic_cpu_mask(const cpumask_t *cpumask)
 {
     unsigned int cpu;
@@ -677,6 +679,14 @@ void gic_raise_guest_irq(struct vcpu *v, unsigned int irq,
 {
     int i;
     unsigned long flags;
+    struct pending_irq *n = irq_to_pending(v, irq);
+
+    if ( test_bit(GIC_IRQ_GUEST_VISIBLE, &n->status))
+    {
+        if ( v == current )
+            _gic_clear_lr(v, n->lr);
+        return;
+    }
 
     spin_lock_irqsave(&gic.lock, flags);
 
@@ -697,51 +707,57 @@ out:
     return;
 }
 
-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;
+
+    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 )
+    {
+        /* HW interrupts cannot be ACTIVE and PENDING */
+        if ( p->desc == NULL &&
+             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))
+        {
+            gic_raise_guest_irq(v, irq, p->priority);
+        } else
+            list_del_init(&p->inflight);
+
+        spin_unlock(&gic.lock);
+    }
+}
+
+void gic_clear_lrs(struct vcpu *v)
+{
+    int i = 0;
     unsigned long flags;
 
     spin_lock_irqsave(&v->arch.vgic.lock, flags);
-
     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);
-            p->lr = nr_lrs;
-            if ( test_bit(GIC_IRQ_GUEST_PENDING, &p->status) &&
-                    test_bit(GIC_IRQ_GUEST_ENABLED, &p->status))
-            {
-                inflight = 1;
-                gic_raise_guest_irq(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_clear_lr(v, i);
         i++;
     }
-
     spin_unlock_irqrestore(&v->arch.vgic.lock, flags);
 }
 
@@ -785,9 +801,6 @@ 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);
-
     gic_restore_pending_irqs(current);
 
     if ( !list_empty(&current->arch.vgic.lr_pending) &&
diff --git a/xen/arch/arm/vgic.c b/xen/arch/arm/vgic.c
index b003f29..981db6c 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_raise_guest_irq(v, irq, p->priority);
         if ( p->desc != NULL )
             p->desc->handler->enable(p->desc);
@@ -694,14 +698,6 @@ void vgic_vcpu_inject_irq(struct vcpu *v, unsigned int irq)
 
     spin_lock_irqsave(&v->arch.vgic.lock, flags);
 
-    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);
-        goto out;
-    }
-
     /* vcpu offline */
     if ( test_bit(_VPF_down, &v->pause_flags) )
     {
@@ -713,21 +709,26 @@ void vgic_vcpu_inject_irq(struct vcpu *v, unsigned int irq)
 
     n->irq = irq;
     set_bit(GIC_IRQ_GUEST_PENDING, &n->status);
-    n->priority = priority;
 
     /* the irq is enabled */
     if ( test_bit(GIC_IRQ_GUEST_ENABLED, &n->status) )
         gic_raise_guest_irq(v, irq, priority);
 
-    list_for_each_entry ( iter, &v->arch.vgic.inflight_irqs, inflight )
+    if ( list_empty(&n->inflight) )
     {
-        if ( iter->priority > priority )
+        n->priority = priority;
+        list_for_each_entry ( iter, &v->arch.vgic.inflight_irqs, inflight )
         {
-            list_add_tail(&n->inflight, &iter->inflight);
-            goto out;
+            if ( iter->priority > priority )
+            {
+                list_add_tail(&n->inflight, &iter->inflight);
+                goto out;
+            }
         }
-    }
-    list_add_tail(&n->inflight, &v->arch.vgic.inflight_irqs);
+        list_add_tail(&n->inflight, &v->arch.vgic.inflight_irqs);
+    } else if ( n->priority != priority )
+        gdprintk(XENLOG_WARNING, "Changing priority of an inflight interrupt is not supported");
+
 out:
     spin_unlock_irqrestore(&v->arch.vgic.lock, flags);
     /* we have a new higher priority irq, inject it into the guest */
-- 
1.7.10.4

[PATCH-4.5 v3 10/12] 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 2dc6386..296d9a7 100644
--- a/xen/arch/arm/gic.c
+++ b/xen/arch/arm/gic.c
@@ -668,17 +668,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_raise_guest_irq(struct vcpu *v, unsigned int irq,
                          unsigned int priority)
 {
     int i;
-    unsigned long flags;
     struct pending_irq *n = irq_to_pending(v, irq);
 
     if ( test_bit(GIC_IRQ_GUEST_VISIBLE, &n->status))
@@ -688,23 +685,17 @@ void gic_raise_guest_irq(struct vcpu *v, unsigned int irq,
         return;
     }
 
-    spin_lock_irqsave(&gic.lock, flags);
-
     if ( v == current && list_empty(&v->arch.vgic.lr_pending) )
     {
         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(v, i, irq, GICH_LR_PENDING, 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)
@@ -726,8 +717,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));
 
@@ -741,8 +730,6 @@ static void _gic_clear_lr(struct vcpu *v, int i)
             gic_raise_guest_irq(v, irq, p->priority);
         } else
             list_del_init(&p->inflight);
-
-        spin_unlock(&gic.lock);
     }
 }
 
@@ -772,11 +759,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);
     }
 
 }
@@ -784,13 +771,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);
 }
 
 int gic_events_need_delivery(void)
diff --git a/xen/arch/arm/vgic.c b/xen/arch/arm/vgic.c
index 981db6c..e9464b2 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_raise_guest_irq(v, irq, 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_raise_guest_irq(v, irq, 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 v3 11/12] xen/arm: gic_events_need_delivery and irq priorities

[Stefano Stabellini]

gic_events_need_delivery should only return positive if an outstanding
pending irq has an higher priority than the currently active irq and the
priority mask.
Rewrite the function by going through the priority ordered inflight and
lr_queue lists.

In gic_restore_pending_irqs replace lower priority pending (and not
active) irqs in GICH_LRs with higher priority irqs if no more GICH_LRs
are available.

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

diff --git a/xen/arch/arm/gic.c b/xen/arch/arm/gic.c
index 296d9a7..aaba8b0 100644
--- a/xen/arch/arm/gic.c
+++ b/xen/arch/arm/gic.c
@@ -709,6 +709,7 @@ static void _gic_clear_lr(struct vcpu *v, int i)
     p = irq_to_pending(v, irq);
     if ( lr & GICH_LR_ACTIVE )
     {
+        set_bit(GIC_IRQ_GUEST_ACTIVE, &p->status);
         /* HW interrupts cannot be ACTIVE and PENDING */
         if ( p->desc == NULL &&
              test_bit(GIC_IRQ_GUEST_ENABLED, &p->status) &&
@@ -723,6 +724,7 @@ static void _gic_clear_lr(struct vcpu *v, int i)
         if ( p->desc != NULL )
             p->desc->status &= ~IRQ_INPROGRESS;
         clear_bit(GIC_IRQ_GUEST_VISIBLE, &p->status);
+        clear_bit(GIC_IRQ_GUEST_ACTIVE, &p->status);
         p->lr = nr_lrs;
         if ( test_bit(GIC_IRQ_GUEST_PENDING, &p->status) &&
                 test_bit(GIC_IRQ_GUEST_ENABLED, &p->status))
@@ -750,22 +752,47 @@ void gic_clear_lrs(struct vcpu *v)
 
 static void gic_restore_pending_irqs(struct vcpu *v)
 {
-    int i;
-    struct pending_irq *p, *t;
+    int i = 0, lrs = nr_lrs;
+    struct pending_irq *p, *t, *p_r;
     unsigned long flags;
 
+    if ( list_empty(&v->arch.vgic.lr_pending) )
+        return;
+
+    spin_lock_irqsave(&v->arch.vgic.lock, flags);
+
+    p_r = list_entry(v->arch.vgic.inflight_irqs.prev,
+                         typeof(*p_r), inflight);
     list_for_each_entry_safe ( p, t, &v->arch.vgic.lr_pending, lr_queue )
     {
         i = find_first_zero_bit(&this_cpu(lr_mask), nr_lrs);
-        if ( i >= nr_lrs ) return;
+        if ( i >= nr_lrs )
+        {
+            while ( !test_bit(GIC_IRQ_GUEST_VISIBLE, &p_r->status) ||
+                    test_bit(GIC_IRQ_GUEST_ACTIVE, &p_r->status) )
+            {
+                p_r = list_entry(p_r->inflight.prev, typeof(*p_r), inflight);
+                if ( &p_r->inflight == p->inflight.next )
+                    goto out;
+            }
+            i = p_r->lr;
+            p_r->lr = nr_lrs;
+            set_bit(GIC_IRQ_GUEST_PENDING, &p_r->status);
+            clear_bit(GIC_IRQ_GUEST_VISIBLE, &p_r->status);
+            gic_add_to_lr_pending(v, p_r->irq, p_r->priority);
+        }
 
-        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(&v->arch.vgic.lock, flags);
+
+        lrs--;
+        if ( lrs == 0 )
+            break;
     }
 
+out:
+    spin_unlock_irqrestore(&v->arch.vgic.lock, flags);
 }
 
 void gic_clear_pending_irqs(struct vcpu *v)
@@ -779,8 +806,38 @@ void gic_clear_pending_irqs(struct vcpu *v)
 
 int gic_events_need_delivery(void)
 {
-    return (!list_empty(&current->arch.vgic.lr_pending) ||
-            this_cpu(lr_mask));
+    int mask_priority, lrs = nr_lrs;
+    int max_priority = 0xff, active_priority = 0xff;
+    struct vcpu *v = current;
+    struct pending_irq *p;
+    unsigned long flags;
+
+    mask_priority = (GICH[GICH_VMCR] >> GICH_VMCR_PRIORITY_SHIFT) & GICH_VMCR_PRIORITY_MASK;
+
+    spin_lock_irqsave(&v->arch.vgic.lock, flags);
+
+    list_for_each_entry( p, &v->arch.vgic.lr_pending, lr_queue )
+    {
+        if ( test_bit(GIC_IRQ_GUEST_ACTIVE, &p->status) )
+        {
+            if ( p->priority < active_priority )
+                active_priority = p->priority;
+        } else {
+            if ( p->priority < max_priority )
+                max_priority = p->priority;
+        }
+        lrs--;
+        if ( lrs == 0 )
+            break;
+    }
+
+    spin_unlock_irqrestore(&v->arch.vgic.lock, flags);
+
+    if ( max_priority < active_priority &&
+         (max_priority >> 3) < mask_priority )
+        return 1;
+    else
+        return 0;
 }
 
 void gic_inject(void)
diff --git a/xen/include/asm-arm/domain.h b/xen/include/asm-arm/domain.h
index 7b636c8..86cb361 100644
--- a/xen/include/asm-arm/domain.h
+++ b/xen/include/asm-arm/domain.h
@@ -56,8 +56,9 @@ struct pending_irq
      *
      */
 #define GIC_IRQ_GUEST_PENDING  0
-#define GIC_IRQ_GUEST_VISIBLE  1
-#define GIC_IRQ_GUEST_ENABLED  2
+#define GIC_IRQ_GUEST_ACTIVE   1
+#define GIC_IRQ_GUEST_VISIBLE  2
+#define GIC_IRQ_GUEST_ENABLED  3
     unsigned long status;
     uint8_t lr;
     struct irq_desc *desc; /* only set it the irq corresponds to a physical irq */
diff --git a/xen/include/asm-arm/gic.h b/xen/include/asm-arm/gic.h
index 5a9dc77..5d8f7f1 100644
--- a/xen/include/asm-arm/gic.h
+++ b/xen/include/asm-arm/gic.h
@@ -129,6 +129,9 @@
 #define GICH_LR_CPUID_SHIFT     9
 #define GICH_VTR_NRLRGS         0x3f
 
+#define GICH_VMCR_PRIORITY_MASK   0x1f
+#define GICH_VMCR_PRIORITY_SHIFT  27
+
 /*
  * The minimum GICC_BPR is required to be in the range 0-3. We set
  * GICC_BPR to 0 but we must expect that it might be 3. This means we
-- 
1.7.10.4

[PATCH-4.5 v3 12/12] 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 and disable interrupts 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>

---
Changes in v3:
- use spin_lock_irqsave and spin_unlock_irqrestore in gic_dump_info.
---
 xen/arch/arm/gic.c |   20 +++++++++++++++++---
 1 file changed, 17 insertions(+), 3 deletions(-)

diff --git a/xen/arch/arm/gic.c b/xen/arch/arm/gic.c
index aaba8b0..7de4443 100644
--- a/xen/arch/arm/gic.c
+++ b/xen/arch/arm/gic.c
@@ -639,6 +639,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);
@@ -714,11 +715,15 @@ static void _gic_clear_lr(struct vcpu *v, int i)
         if ( p->desc == NULL &&
              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 ( p->desc != NULL )
@@ -1006,8 +1011,11 @@ void gic_dump_info(struct vcpu *v)
 {
     int i;
     struct pending_irq *p;
+    unsigned long flags;
 
     printk("GICH_LRs (vcpu %d) mask=%"PRIx64"\n", v->vcpu_id, v->arch.lr_mask);
+
+    spin_lock_irqsave(&v->arch.vgic.lock, flags);
     if ( v == current )
     {
         for ( i = 0; i < nr_lrs; i++ )
@@ -1019,14 +1027,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_irqrestore(&v->arch.vgic.lock, flags);
 }
 
 void __cpuinit init_maintenance_interrupt(void)
-- 
1.7.10.4

Re: [PATCH-4.5 v3 11/12] xen/arm: gic_events_need_delivery and irq priorities

[Stefano Stabellini]

On Wed, 26 Feb 2014, Stefano Stabellini wrote:
> gic_events_need_delivery should only return positive if an outstanding
> pending irq has an higher priority than the currently active irq and the
> priority mask.
> Rewrite the function by going through the priority ordered inflight and
> lr_queue lists.
> 
> In gic_restore_pending_irqs replace lower priority pending (and not
> active) irqs in GICH_LRs with higher priority irqs if no more GICH_LRs
> are available.
> 
> Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>

[snip]

> @@ -779,8 +806,38 @@ void gic_clear_pending_irqs(struct vcpu *v)
>  
>  int gic_events_need_delivery(void)
>  {
> -    return (!list_empty(&current->arch.vgic.lr_pending) ||
> -            this_cpu(lr_mask));
> +    int mask_priority, lrs = nr_lrs;
> +    int max_priority = 0xff, active_priority = 0xff;
> +    struct vcpu *v = current;
> +    struct pending_irq *p;
> +    unsigned long flags;
> +
> +    mask_priority = (GICH[GICH_VMCR] >> GICH_VMCR_PRIORITY_SHIFT) & GICH_VMCR_PRIORITY_MASK;
> +
> +    spin_lock_irqsave(&v->arch.vgic.lock, flags);
> +
> +    list_for_each_entry( p, &v->arch.vgic.lr_pending, lr_queue )
> +    {

Unfortunately I sent the wrong version of this patch. We should be going
through the inflight list here, not the lr_pending list:

list_for_each_entry( p, &v->arch.vgic.inflight_irqs, inflight )

I also added a check for GIC_IRQ_GUEST_ENABLED.
I pushed the full series with this small fix to:

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

I am also appending the newer version here.


commit cfddc7eb3a7d4649ceacdf89609a86d826773e9e
Author: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
Date:   Wed Feb 26 19:21:35 2014 +0000

    xen/arm: gic_events_need_delivery and irq priorities
    
    gic_events_need_delivery should only return positive if an outstanding
    pending irq has an higher priority than the currently active irq and the
    priority mask.
    Rewrite the function by going through the priority ordered inflight and
    lr_queue lists.
    
    In gic_restore_pending_irqs replace lower priority pending (and not
    active) irqs in GICH_LRs with higher priority irqs if no more GICH_LRs
    are available.
    
    Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
    
    ---
    Changes in v4:
    - in gic_events_need_delivery go through inflight_irqs and only consider
    enabled irqs.

diff --git a/xen/arch/arm/gic.c b/xen/arch/arm/gic.c
index 296d9a7..d2e23a9 100644
--- a/xen/arch/arm/gic.c
+++ b/xen/arch/arm/gic.c
@@ -709,6 +709,7 @@ static void _gic_clear_lr(struct vcpu *v, int i)
     p = irq_to_pending(v, irq);
     if ( lr & GICH_LR_ACTIVE )
     {
+        set_bit(GIC_IRQ_GUEST_ACTIVE, &p->status);
         /* HW interrupts cannot be ACTIVE and PENDING */
         if ( p->desc == NULL &&
              test_bit(GIC_IRQ_GUEST_ENABLED, &p->status) &&
@@ -723,6 +724,7 @@ static void _gic_clear_lr(struct vcpu *v, int i)
         if ( p->desc != NULL )
             p->desc->status &= ~IRQ_INPROGRESS;
         clear_bit(GIC_IRQ_GUEST_VISIBLE, &p->status);
+        clear_bit(GIC_IRQ_GUEST_ACTIVE, &p->status);
         p->lr = nr_lrs;
         if ( test_bit(GIC_IRQ_GUEST_PENDING, &p->status) &&
                 test_bit(GIC_IRQ_GUEST_ENABLED, &p->status))
@@ -750,22 +752,47 @@ void gic_clear_lrs(struct vcpu *v)
 
 static void gic_restore_pending_irqs(struct vcpu *v)
 {
-    int i;
-    struct pending_irq *p, *t;
+    int i = 0, lrs = nr_lrs;
+    struct pending_irq *p, *t, *p_r;
     unsigned long flags;
 
+    if ( list_empty(&v->arch.vgic.lr_pending) )
+        return;
+
+    spin_lock_irqsave(&v->arch.vgic.lock, flags);
+
+    p_r = list_entry(v->arch.vgic.inflight_irqs.prev,
+                         typeof(*p_r), inflight);
     list_for_each_entry_safe ( p, t, &v->arch.vgic.lr_pending, lr_queue )
     {
         i = find_first_zero_bit(&this_cpu(lr_mask), nr_lrs);
-        if ( i >= nr_lrs ) return;
+        if ( i >= nr_lrs )
+        {
+            while ( !test_bit(GIC_IRQ_GUEST_VISIBLE, &p_r->status) ||
+                    test_bit(GIC_IRQ_GUEST_ACTIVE, &p_r->status) )
+            {
+                p_r = list_entry(p_r->inflight.prev, typeof(*p_r), inflight);
+                if ( &p_r->inflight == p->inflight.next )
+                    goto out;
+            }
+            i = p_r->lr;
+            p_r->lr = nr_lrs;
+            set_bit(GIC_IRQ_GUEST_PENDING, &p_r->status);
+            clear_bit(GIC_IRQ_GUEST_VISIBLE, &p_r->status);
+            gic_add_to_lr_pending(v, p_r->irq, p_r->priority);
+        }
 
-        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(&v->arch.vgic.lock, flags);
+
+        lrs--;
+        if ( lrs == 0 )
+            break;
     }
 
+out:
+    spin_unlock_irqrestore(&v->arch.vgic.lock, flags);
 }
 
 void gic_clear_pending_irqs(struct vcpu *v)
@@ -779,8 +806,38 @@ void gic_clear_pending_irqs(struct vcpu *v)
 
 int gic_events_need_delivery(void)
 {
-    return (!list_empty(&current->arch.vgic.lr_pending) ||
-            this_cpu(lr_mask));
+    int mask_priority, lrs = nr_lrs;
+    int max_priority = 0xff, active_priority = 0xff;
+    struct vcpu *v = current;
+    struct pending_irq *p;
+    unsigned long flags;
+
+    mask_priority = (GICH[GICH_VMCR] >> GICH_VMCR_PRIORITY_SHIFT) & GICH_VMCR_PRIORITY_MASK;
+
+    spin_lock_irqsave(&v->arch.vgic.lock, flags);
+
+    list_for_each_entry( p, &v->arch.vgic.inflight_irqs, inflight )
+    {
+        if ( test_bit(GIC_IRQ_GUEST_ACTIVE, &p->status) )
+        {
+            if ( p->priority < active_priority )
+                active_priority = p->priority;
+        } else if ( test_bit(GIC_IRQ_GUEST_ENABLED, &p->status) ) {
+            if ( p->priority < max_priority )
+                max_priority = p->priority;
+        }
+        lrs--;
+        if ( lrs == 0 )
+            break;
+    }
+
+    spin_unlock_irqrestore(&v->arch.vgic.lock, flags);
+
+    if ( max_priority < active_priority &&
+         (max_priority >> 3) < mask_priority )
+        return 1;
+    else
+        return 0;
 }
 
 void gic_inject(void)
diff --git a/xen/include/asm-arm/domain.h b/xen/include/asm-arm/domain.h
index 7b636c8..86cb361 100644
--- a/xen/include/asm-arm/domain.h
+++ b/xen/include/asm-arm/domain.h
@@ -56,8 +56,9 @@ struct pending_irq
      *
      */
 #define GIC_IRQ_GUEST_PENDING  0
-#define GIC_IRQ_GUEST_VISIBLE  1
-#define GIC_IRQ_GUEST_ENABLED  2
+#define GIC_IRQ_GUEST_ACTIVE   1
+#define GIC_IRQ_GUEST_VISIBLE  2
+#define GIC_IRQ_GUEST_ENABLED  3
     unsigned long status;
     uint8_t lr;
     struct irq_desc *desc; /* only set it the irq corresponds to a physical irq */
diff --git a/xen/include/asm-arm/gic.h b/xen/include/asm-arm/gic.h
index 5a9dc77..5d8f7f1 100644
--- a/xen/include/asm-arm/gic.h
+++ b/xen/include/asm-arm/gic.h
@@ -129,6 +129,9 @@
 #define GICH_LR_CPUID_SHIFT     9
 #define GICH_VTR_NRLRGS         0x3f
 
+#define GICH_VMCR_PRIORITY_MASK   0x1f
+#define GICH_VMCR_PRIORITY_SHIFT  27
+
 /*
  * The minimum GICC_BPR is required to be in the range 0-3. We set
  * GICC_BPR to 0 but we must expect that it might be 3. This means we

Re: [PATCH-4.5 v3 01/12] xen/arm: no need to set HCR_VI when using the vgic to inject irqs

[Julien Grall]

On 26/02/14 18:39, Stefano Stabellini wrote:
> It can actually cause spurious interrupt injections into the guest.
>

I would explain what is the current behaviour with HCR_VI, e.g. the
guest is entering in interrupt mode...

By only reading the documentation and your commit message, this patch 
seems wrong (I know it's not the case ;)).

-- 
Julien Grall

Re: [PATCH-4.5 v3 03/12] xen/arm: support HW interrupts in gic_set_lr

[Julien Grall]

On 26/02/14 18:39, 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.
>
> This patch needs the following patch to work correctly. It has been sent
> separately to make it easier to review.
>
> Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
Acked-by: Julien Grall <julien.grall@linaro.org>

-- 
Julien Grall

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

[Julien Grall]

Hi Stefano,

On 26/02/14 18:39, Stefano Stabellini wrote:
> GICH is banked, protect accesses by disabling interrupts.
> Protect lr_queue accesses with the vgic.lock only.

When the interrupt is an SPI, the lr_queue is shared between every VCPU. 
Using only vgic.lock seems wrong to me.

-- 
Julien Grall

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

[Stefano Stabellini]

On Wed, 26 Feb 2014, Julien Grall wrote:
> Hi Stefano,
> 
> On 26/02/14 18:39, Stefano Stabellini wrote:
> > GICH is banked, protect accesses by disabling interrupts.
> > Protect lr_queue accesses with the vgic.lock only.
> 
> When the interrupt is an SPI, the lr_queue is shared between every VCPU. Using
> only vgic.lock seems wrong to me.

Even though lr_queue is a field in a struct that can be per domain for
irq > 32, the lr_pending queue is always per vcpu, in fact it keeps
track of the irqs waiting to go into lr registers, that are banked.

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

[Julien Grall]

Hi Stefano,

On 03/18/2014 02:59 PM, Stefano Stabellini wrote:
> On Wed, 26 Feb 2014, Julien Grall wrote:
>> On 26/02/14 18:39, Stefano Stabellini wrote:
>>> GICH is banked, protect accesses by disabling interrupts.
>>> Protect lr_queue accesses with the vgic.lock only.
>>
>> When the interrupt is an SPI, the lr_queue is shared between every VCPU. Using
>> only vgic.lock seems wrong to me.
> 
> Even though lr_queue is a field in a struct that can be per domain for
> irq > 32, the lr_pending queue is always per vcpu, in fact it keeps
> track of the irqs waiting to go into lr registers, that are banked.
> 

It might have a race between adding and removing lr_queue. For now it's
safe because the SPIs are always routed to VCPU0 (even if the guest ask
to route on VCPU1).

Just by reading the code, nothing protect correctly SPIs between VCPUs.
We are only take vgic.lock, which is wrong.

Regards,

-- 
Julien Grall

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

[Stefano Stabellini]

On Tue, 18 Mar 2014, Julien Grall wrote:
> Hi Stefano,
> 
> On 03/18/2014 02:59 PM, Stefano Stabellini wrote:
> > On Wed, 26 Feb 2014, Julien Grall wrote:
> >> On 26/02/14 18:39, Stefano Stabellini wrote:
> >>> GICH is banked, protect accesses by disabling interrupts.
> >>> Protect lr_queue accesses with the vgic.lock only.
> >>
> >> When the interrupt is an SPI, the lr_queue is shared between every VCPU. Using
> >> only vgic.lock seems wrong to me.
> > 
> > Even though lr_queue is a field in a struct that can be per domain for
> > irq > 32, the lr_pending queue is always per vcpu, in fact it keeps
> > track of the irqs waiting to go into lr registers, that are banked.
> > 
> 
> It might have a race between adding and removing lr_queue. For now it's
> safe because the SPIs are always routed to VCPU0 (even if the guest ask
> to route on VCPU1).
> 
> Just by reading the code, nothing protect correctly SPIs between VCPUs.
> We are only take vgic.lock, which is wrong.

Like you said, today SPIs are always routed to the same cpu.
When we'll implement IRQ migration we'll have to make sure that we take
the appropriate locks during the operation but I don't expect that this
patch is going to make things more difficult.

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

[Julien Grall]

Hi Stefano,

On 03/18/2014 06:52 PM, Stefano Stabellini wrote:
> On Tue, 18 Mar 2014, Julien Grall wrote:
>> Hi Stefano,
>>
>> On 03/18/2014 02:59 PM, Stefano Stabellini wrote:
>>> On Wed, 26 Feb 2014, Julien Grall wrote:
>>>> On 26/02/14 18:39, Stefano Stabellini wrote:
>>>>> GICH is banked, protect accesses by disabling interrupts.
>>>>> Protect lr_queue accesses with the vgic.lock only.
>>>>
>>>> When the interrupt is an SPI, the lr_queue is shared between every VCPU. Using
>>>> only vgic.lock seems wrong to me.
>>>
>>> Even though lr_queue is a field in a struct that can be per domain for
>>> irq > 32, the lr_pending queue is always per vcpu, in fact it keeps
>>> track of the irqs waiting to go into lr registers, that are banked.
>>>
>>
>> It might have a race between adding and removing lr_queue. For now it's
>> safe because the SPIs are always routed to VCPU0 (even if the guest ask
>> to route on VCPU1).
>>
>> Just by reading the code, nothing protect correctly SPIs between VCPUs.
>> We are only take vgic.lock, which is wrong.
> 
> Like you said, today SPIs are always routed to the same cpu.
> When we'll implement IRQ migration we'll have to make sure that we take
> the appropriate locks during the operation but I don't expect that this
> patch is going to make things more difficult.

I'm fine with that. Can you create a TODO somewhere to not forget this
possible locking issue later?

Regards,

-- 
Julien Grall

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

[Stefano Stabellini]

On Tue, 18 Mar 2014, Julien Grall wrote:
> Hi Stefano,
> 
> On 03/18/2014 06:52 PM, Stefano Stabellini wrote:
> > On Tue, 18 Mar 2014, Julien Grall wrote:
> >> Hi Stefano,
> >>
> >> On 03/18/2014 02:59 PM, Stefano Stabellini wrote:
> >>> On Wed, 26 Feb 2014, Julien Grall wrote:
> >>>> On 26/02/14 18:39, Stefano Stabellini wrote:
> >>>>> GICH is banked, protect accesses by disabling interrupts.
> >>>>> Protect lr_queue accesses with the vgic.lock only.
> >>>>
> >>>> When the interrupt is an SPI, the lr_queue is shared between every VCPU. Using
> >>>> only vgic.lock seems wrong to me.
> >>>
> >>> Even though lr_queue is a field in a struct that can be per domain for
> >>> irq > 32, the lr_pending queue is always per vcpu, in fact it keeps
> >>> track of the irqs waiting to go into lr registers, that are banked.
> >>>
> >>
> >> It might have a race between adding and removing lr_queue. For now it's
> >> safe because the SPIs are always routed to VCPU0 (even if the guest ask
> >> to route on VCPU1).
> >>
> >> Just by reading the code, nothing protect correctly SPIs between VCPUs.
> >> We are only take vgic.lock, which is wrong.
> > 
> > Like you said, today SPIs are always routed to the same cpu.
> > When we'll implement IRQ migration we'll have to make sure that we take
> > the appropriate locks during the operation but I don't expect that this
> > patch is going to make things more difficult.
> 
> I'm fine with that. Can you create a TODO somewhere to not forget this
> possible locking issue later?

Sure