[PATCH v2 0/5] xen/arm: multiple guests support in the GIC and VGIC

[PATCH v2 0/5] xen/arm: multiple guests support in the GIC and VGIC

[Stefano Stabellini]

Hi all,
this patch series fixes the GIC, VGIC and vtimer issues that caused dom1
to hang, as described by Ian in
http://marc.info/?l=xen-devel&m=133856539418794.

With this patch series applied on top of Ian's, dom1 boots up to:

VFS: Cannot open root device "(null)" or unknown-block(2,0)



Changes in v2:

- prettify gic_restore_pending_irqs;

- remove warning from gic_set_guest_irq.

- rebased on "xen: event channel remapping for emulated MSIs".



Stefano Stabellini (5):
      arm/vtimer: do not let the guest interact with the physical timer
      arm/gic: fix gic context switch
      xen/gic: support injecting IRQs even to VCPUs not currently running
      xen/vgic: vgic: support irq enable/disable
      xen/vgic: initialize pending_irqs.lr_queue

 xen/arch/arm/gic.c           |   65 +++++++++++++++++++++++++++++-------------
 xen/arch/arm/gic.h           |    2 +-
 xen/arch/arm/time.c          |    4 +-
 xen/arch/arm/vgic.c          |   27 +++++++++++++++++-
 xen/include/asm-arm/domain.h |    9 ++++++
 5 files changed, 83 insertions(+), 24 deletions(-)

Cheers,

Stefano

[PATCH v2 1/5] arm/vtimer: do not let the guest interact with the physical timer

[Stefano Stabellini]

The guest can read the physical counter but it shouldn't be able to
cause interrupts of the physical timer to go to the hypervisor.
Trap physical timer reads/writes in vtimer.c instead.

Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
Acked-by: Tim Deegan <tim@xen.org>
---
 xen/arch/arm/time.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/xen/arch/arm/time.c b/xen/arch/arm/time.c
index 1587fa2..b6d7015 100644
--- a/xen/arch/arm/time.c
+++ b/xen/arch/arm/time.c
@@ -160,8 +160,8 @@ void __cpuinit init_timer_interrupt(void)
     WRITE_CP64(0, CNTVOFF);     /* No VM-specific offset */
     WRITE_CP32(0, CNTKCTL);     /* No user-mode access */
 #if USE_HYP_TIMER
-    /* Let the VMs read the physical counter and timer so they can tell time */
-    WRITE_CP32(CNTHCTL_PA|CNTHCTL_TA, CNTHCTL);
+    /* Do not let the VMs program the physical timer, only read the physical counter */
+    WRITE_CP32(CNTHCTL_PA, CNTHCTL);
 #else
     /* Cannot let VMs access physical counter if we are using it */
     WRITE_CP32(0, CNTHCTL);
-- 
1.7.2.5

[PATCH v2 2/5] arm/gic: fix gic context switch

[Stefano Stabellini]

gic_save/restore_state should also save and restore lr_mask and
event_mask too.

Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
Acked-by: Tim Deegan <tim@xen.org>
Acked-by: Ian Campbell <ian.campbell@citrix.com>
---
 xen/arch/arm/gic.c           |    4 ++++
 xen/include/asm-arm/domain.h |    2 ++
 2 files changed, 6 insertions(+), 0 deletions(-)

diff --git a/xen/arch/arm/gic.c b/xen/arch/arm/gic.c
index ee83825..94b7ad7 100644
--- a/xen/arch/arm/gic.c
+++ b/xen/arch/arm/gic.c
@@ -67,6 +67,8 @@ void gic_save_state(struct vcpu *v)
 
     for ( i=0; i<nr_lrs; i++)
         v->arch.gic_lr[i] = GICH[GICH_LR + i];
+    v->arch.lr_mask = gic.lr_mask;
+    v->arch.event_mask = gic.event_mask;
     /* Disable until next VCPU scheduled */
     GICH[GICH_HCR] = 0;
     isb();
@@ -79,6 +81,8 @@ void gic_restore_state(struct vcpu *v)
     if ( is_idle_vcpu(v) )
         return;
 
+    gic.lr_mask = v->arch.lr_mask;
+    gic.event_mask = v->arch.event_mask;
     for ( i=0; i<nr_lrs; i++)
         GICH[GICH_LR + i] = v->arch.gic_lr[i];
     GICH[GICH_HCR] = GICH_HCR_EN;
diff --git a/xen/include/asm-arm/domain.h b/xen/include/asm-arm/domain.h
index 230ea8c..3576d50 100644
--- a/xen/include/asm-arm/domain.h
+++ b/xen/include/asm-arm/domain.h
@@ -121,6 +121,8 @@ struct arch_vcpu
 
     uint32_t gic_hcr, gic_vmcr, gic_apr;
     uint32_t gic_lr[64];
+    uint64_t event_mask;
+    uint64_t lr_mask;
 
     struct {
         /*
-- 
1.7.2.5

[PATCH v2 3/5] xen/gic: support injecting IRQs even to VCPUs not currently running

[Stefano Stabellini]

The lr_pending list belongs to the vgic rather than the gic, so move it
there.

gic_set_guest_irq should take into account whether the vcpu is currently
running and if it is not it should add the irq to the right lr_pending
list.

When restoring the gic state we need to go through the lr_pending list
because it is possible that some irqs have been "injected" while the
vcpu wasn't running.

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

diff --git a/xen/arch/arm/gic.c b/xen/arch/arm/gic.c
index 94b7ad7..37de696 100644
--- a/xen/arch/arm/gic.c
+++ b/xen/arch/arm/gic.c
@@ -38,6 +38,7 @@
 #define GICH ((volatile uint32_t *) (FIXMAP_ADDR(FIXMAP_GICH)  \
                                      + (GIC_HR_OFFSET & 0xfff)))
 static void events_maintenance(struct vcpu *v);
+static void gic_restore_pending_irqs(struct vcpu *v);
 
 /* Global state */
 static struct {
@@ -49,13 +50,6 @@ static struct {
     spinlock_t lock;
     uint64_t event_mask;
     uint64_t lr_mask;
-    /* lr_pending is used to queue IRQs (struct pending_irq) that the
-     * vgic tried to inject in the guest (calling gic_set_guest_irq) but
-     * no LRs were available at the time.
-     * As soon as an LR is freed we remove the first IRQ from this
-     * list and write it to the LR register.
-     * lr_pending is a subset of vgic.inflight_irqs. */
-    struct list_head lr_pending;
 } gic;
 
 irq_desc_t irq_desc[NR_IRQS];
@@ -87,6 +81,8 @@ void gic_restore_state(struct vcpu *v)
         GICH[GICH_LR + i] = v->arch.gic_lr[i];
     GICH[GICH_HCR] = GICH_HCR_EN;
     isb();
+
+    gic_restore_pending_irqs(v);
 }
 
 static unsigned int gic_irq_startup(struct irq_desc *desc)
@@ -323,7 +319,6 @@ int __init gic_init(void)
 
     gic.lr_mask = 0ULL;
     gic.event_mask = 0ULL;
-    INIT_LIST_HEAD(&gic.lr_pending);
 
     spin_unlock(&gic.lock);
 
@@ -435,17 +430,20 @@ static inline void gic_set_lr(int lr, unsigned int virtual_irq,
         ((virtual_irq & GICH_LR_VIRTUAL_MASK) << GICH_LR_VIRTUAL_SHIFT);
 }
 
-void gic_set_guest_irq(unsigned int virtual_irq,
+void gic_set_guest_irq(struct vcpu *v, unsigned int virtual_irq,
         unsigned int state, unsigned int priority)
 {
     int i;
     struct pending_irq *iter, *n;
 
-    events_maintenance(current);
+    if ( v->is_running )
+    {
+        events_maintenance(v);
+    }
 
     spin_lock_irq(&gic.lock);
 
-    if ( list_empty(&gic.lr_pending) )
+    if ( v->is_running && list_empty(&v->arch.vgic.lr_pending) )
     {
         i = find_first_zero_bit(&gic.lr_mask, nr_lrs);
         if (i < nr_lrs) {
@@ -457,8 +455,8 @@ void gic_set_guest_irq(unsigned int virtual_irq,
         }
     }
 
-    n = irq_to_pending(current, virtual_irq);
-    list_for_each_entry ( iter, &gic.lr_pending, lr_queue )
+    n = irq_to_pending(v, virtual_irq);
+    list_for_each_entry ( iter, &v->arch.vgic.lr_pending, lr_queue )
     {
         if ( iter->priority > priority )
         {
@@ -466,13 +464,32 @@ void gic_set_guest_irq(unsigned int virtual_irq,
             goto out;
         }
     }
-    list_add_tail(&n->lr_queue, &gic.lr_pending);
+    list_add_tail(&n->lr_queue, &v->arch.vgic.lr_pending);
 
 out:
     spin_unlock_irq(&gic.lock);
     return;
 }
 
+static void gic_restore_pending_irqs(struct vcpu *v)
+{
+    int i;
+    struct pending_irq *p;
+
+    list_for_each_entry ( p, &v->arch.vgic.lr_pending, lr_queue )
+    {
+        i = find_first_zero_bit(&gic.lr_mask, nr_lrs);
+        if ( i >= nr_lrs ) return;
+
+        gic_set_lr(i, p->irq, GICH_LR_PENDING, p->priority);
+        list_del_init(&p->lr_queue);
+        set_bit(i, &gic.lr_mask);
+        if ( p->irq == VGIC_IRQ_EVTCHN_CALLBACK )
+            set_bit(i, &gic.event_mask);
+    }
+
+}
+
 static void gic_inject_irq_start(void)
 {
     uint32_t hcr;
@@ -579,9 +596,10 @@ static void maintenance_interrupt(int irq, void *dev_id, struct cpu_user_regs *r
 {
     int i = 0, virq;
     uint32_t lr;
+    struct vcpu *v = current;
     uint64_t eisr = GICH[GICH_EISR0] | (((uint64_t) GICH[GICH_EISR1]) << 32);
 
-    events_maintenance(current);
+    events_maintenance(v);
 
     while ((i = find_next_bit((const long unsigned int *) &eisr,
                               sizeof(eisr), i)) < sizeof(eisr)) {
@@ -593,8 +611,8 @@ static void maintenance_interrupt(int irq, void *dev_id, struct cpu_user_regs *r
         GICH[GICH_LR + i] = 0;
         clear_bit(i, &gic.lr_mask);
 
-        if ( !list_empty(&gic.lr_pending) ) {
-            p = list_entry(gic.lr_pending.next, typeof(*p), lr_queue);
+        if ( !list_empty(&v->arch.vgic.lr_pending) ) {
+            p = list_entry(v->arch.vgic.lr_pending.next, typeof(*p), lr_queue);
             gic_set_lr(i, p->irq, GICH_LR_PENDING, p->priority);
             list_del_init(&p->lr_queue);
             set_bit(i, &gic.lr_mask);
@@ -605,14 +623,14 @@ static void maintenance_interrupt(int irq, void *dev_id, struct cpu_user_regs *r
         }
         spin_unlock_irq(&gic.lock);
 
-        spin_lock_irq(&current->arch.vgic.lock);
-        p = irq_to_pending(current, virq);
+        spin_lock_irq(&v->arch.vgic.lock);
+        p = irq_to_pending(v, virq);
         if ( p->desc != NULL ) {
             p->desc->status &= ~IRQ_INPROGRESS;
             GICC[GICC_DIR] = virq;
         }
         list_del_init(&p->inflight);
-        spin_unlock_irq(&current->arch.vgic.lock);
+        spin_unlock_irq(&v->arch.vgic.lock);
 
         i++;
     }
diff --git a/xen/arch/arm/gic.h b/xen/arch/arm/gic.h
index e36d6ad..fa2cf06 100644
--- a/xen/arch/arm/gic.h
+++ b/xen/arch/arm/gic.h
@@ -137,7 +137,7 @@ extern void gic_route_irqs(void);
 extern void gic_inject(void);
 
 extern void __cpuinit init_maintenance_interrupt(void);
-extern void gic_set_guest_irq(unsigned int irq,
+extern void gic_set_guest_irq(struct vcpu *v, unsigned int irq,
         unsigned int state, unsigned int priority);
 extern int gic_route_irq_to_guest(struct domain *d, unsigned int irq,
                                   const char * devname);
diff --git a/xen/arch/arm/vgic.c b/xen/arch/arm/vgic.c
index f2fa927..9af91e2 100644
--- a/xen/arch/arm/vgic.c
+++ b/xen/arch/arm/vgic.c
@@ -115,6 +115,7 @@ int vcpu_vgic_init(struct vcpu *v)
             | (1<<(v->vcpu_id+16))
             | (1<<(v->vcpu_id+24));
     INIT_LIST_HEAD(&v->arch.vgic.inflight_irqs);
+    INIT_LIST_HEAD(&v->arch.vgic.lr_pending);
     spin_lock_init(&v->arch.vgic.lock);
 
     return 0;
@@ -571,7 +572,7 @@ void vgic_vcpu_inject_irq(struct vcpu *v, unsigned int irq, int virtual)
     else
         n->desc = NULL;
 
-    gic_set_guest_irq(irq, GICH_LR_PENDING, priority);
+    gic_set_guest_irq(v, irq, GICH_LR_PENDING, priority);
 
     spin_lock_irqsave(&v->arch.vgic.lock, flags);
     list_for_each_entry ( iter, &v->arch.vgic.inflight_irqs, inflight )
diff --git a/xen/include/asm-arm/domain.h b/xen/include/asm-arm/domain.h
index 3576d50..2b14545 100644
--- a/xen/include/asm-arm/domain.h
+++ b/xen/include/asm-arm/domain.h
@@ -140,6 +140,13 @@ struct arch_vcpu
          * As soon as an IRQ is EOI'd by the guest and removed from the
          * corresponding LR it is also removed from this list. */
         struct list_head inflight_irqs;
+        /* lr_pending is used to queue IRQs (struct pending_irq) that the
+         * vgic tried to inject in the guest (calling gic_set_guest_irq) but
+         * no LRs were available at the time.
+         * As soon as an LR is freed we remove the first IRQ from this
+         * list and write it to the LR register.
+         * lr_pending is a subset of vgic.inflight_irqs. */
+        struct list_head lr_pending;
         spinlock_t lock;
     } vgic;
 
-- 
1.7.2.5

[PATCH v2 4/5] xen/vgic: vgic: support irq enable/disable

[Stefano Stabellini]

If vgic_vcpu_inject_irq is called (for example by a device emulator like
vtimer.c) but the corresponding irq is not enabled in the virtual gicd
just queue it in the inflight_irqs list.

When the irq is enabled make sure to call gic_set_guest_irq.

Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
Acked-by: Tim Deegan <tim@xen.org>
Acked-by: Ian Campbell <ian.campbell@citrix.com>
---
 xen/arch/arm/vgic.c |   23 ++++++++++++++++++++++-
 1 files changed, 22 insertions(+), 1 deletions(-)

diff --git a/xen/arch/arm/vgic.c b/xen/arch/arm/vgic.c
index 9af91e2..f11e7bf 100644
--- a/xen/arch/arm/vgic.c
+++ b/xen/arch/arm/vgic.c
@@ -346,6 +346,22 @@ read_as_zero:
     return 1;
 }
 
+static void vgic_enable_irqs(struct vcpu *v, uint32_t r, int n)
+{
+    struct pending_irq *p;
+    unsigned int irq;
+    int i = 0;
+
+    while ( (i = find_next_bit((const long unsigned int *) &r,
+                        sizeof(uint32_t), i)) < sizeof(uint32_t) ) {
+        irq = i + (32 * n);
+        p = irq_to_pending(v, irq);
+        if ( !list_empty(&p->inflight) )
+            gic_set_guest_irq(v, irq, GICH_LR_PENDING, p->priority);
+        i++;
+    }
+}
+
 static int vgic_distr_mmio_write(struct vcpu *v, mmio_info_t *info)
 {
     struct hsr_dabt dabt = info->dabt;
@@ -354,6 +370,7 @@ static int vgic_distr_mmio_write(struct vcpu *v, mmio_info_t *info)
     struct vgic_irq_rank *rank;
     int offset = (int)(info->gpa - VGIC_DISTR_BASE_ADDRESS);
     int gicd_reg = REG(offset);
+    uint32_t tr;
 
     switch ( gicd_reg )
     {
@@ -381,8 +398,10 @@ static int vgic_distr_mmio_write(struct vcpu *v, mmio_info_t *info)
         rank = vgic_irq_rank(v, 8, gicd_reg - GICD_ISENABLER);
         if ( rank == NULL) goto write_ignore;
         vgic_lock_rank(v, rank);
+        tr = rank->ienable;
         rank->ienable |= *r;
         vgic_unlock_rank(v, rank);
+        vgic_enable_irqs(v, (*r) & (~tr), gicd_reg - GICD_ISENABLER);
         return 1;
 
     case GICD_ICENABLER ... GICD_ICENABLERN:
@@ -572,7 +591,9 @@ void vgic_vcpu_inject_irq(struct vcpu *v, unsigned int irq, int virtual)
     else
         n->desc = NULL;
 
-    gic_set_guest_irq(v, irq, GICH_LR_PENDING, priority);
+    /* the irq is enabled */
+    if ( rank->ienable & (1 << (irq % 32)) )
+        gic_set_guest_irq(v, irq, GICH_LR_PENDING, priority);
 
     spin_lock_irqsave(&v->arch.vgic.lock, flags);
     list_for_each_entry ( iter, &v->arch.vgic.inflight_irqs, inflight )
-- 
1.7.2.5

[PATCH v2 5/5] xen/vgic: initialize pending_irqs.lr_queue

[Stefano Stabellini]

Properly initialize all the pending_irqs.lr_queue like we do for
inflight.
Check whether we already have the irq in lr_queue before adding it.

Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
Acked-by: Tim Deegan <tim@xen.org>
---
 xen/arch/arm/gic.c  |    3 +++
 xen/arch/arm/vgic.c |    3 +++
 2 files changed, 6 insertions(+), 0 deletions(-)

diff --git a/xen/arch/arm/gic.c b/xen/arch/arm/gic.c
index 37de696..c8d7caa 100644
--- a/xen/arch/arm/gic.c
+++ b/xen/arch/arm/gic.c
@@ -456,6 +456,9 @@ void gic_set_guest_irq(struct vcpu *v, unsigned int virtual_irq,
     }
 
     n = irq_to_pending(v, virtual_irq);
+    if ( !list_empty(&n->lr_queue) )
+        goto out;
+
     list_for_each_entry ( iter, &v->arch.vgic.lr_pending, lr_queue )
     {
         if ( iter->priority > priority )
diff --git a/xen/arch/arm/vgic.c b/xen/arch/arm/vgic.c
index f11e7bf..005f478 100644
--- a/xen/arch/arm/vgic.c
+++ b/xen/arch/arm/vgic.c
@@ -84,7 +84,10 @@ int domain_vgic_init(struct domain *d)
     d->arch.vgic.pending_irqs =
         xzalloc_array(struct pending_irq, d->arch.vgic.nr_lines);
     for (i=0; i<d->arch.vgic.nr_lines; i++)
+    {
         INIT_LIST_HEAD(&d->arch.vgic.pending_irqs[i].inflight);
+        INIT_LIST_HEAD(&d->arch.vgic.pending_irqs[i].lr_queue);
+    }
     for (i=0; i<DOMAIN_NR_RANKS(d); i++)
         spin_lock_init(&d->arch.vgic.shared_irqs[i].lock);
     return 0;
-- 
1.7.2.5

Re: [PATCH v2 0/5] xen/arm: multiple guests support in the GIC and VGIC

[Ian Campbell]

On Wed, 2012-07-04 at 11:45 +0100, Stefano Stabellini wrote:
> Hi all,
> this patch series fixes the GIC, VGIC and vtimer issues that caused dom1
> to hang, as described by Ian in
> http://marc.info/?l=xen-devel&m=133856539418794.
> 
> With this patch series applied on top of Ian's, dom1 boots up to:
> 
> VFS: Cannot open root device "(null)" or unknown-block(2,0)

All applied, thanks, (actually yesterday, I seem to have forgotten to hit send here)

> Stefano Stabellini (5):
>       arm/vtimer: do not let the guest interact with the physical timer
>       arm/gic: fix gic context switch
>       xen/gic: support injecting IRQs even to VCPUs not currently running
>       xen/vgic: vgic: support irq enable/disable
>       xen/vgic: initialize pending_irqs.lr_queue

I rewrote these last three to "arm/foo" which I think is what you meant?

Ian.

Re: [PATCH v2 0/5] xen/arm: multiple guests support in the GIC and VGIC

[Stefano Stabellini]

On Wed, 18 Jul 2012, Ian Campbell wrote:
> On Wed, 2012-07-04 at 11:45 +0100, Stefano Stabellini wrote:
> > Hi all,
> > this patch series fixes the GIC, VGIC and vtimer issues that caused dom1
> > to hang, as described by Ian in
> > http://marc.info/?l=xen-devel&m=133856539418794.
> > 
> > With this patch series applied on top of Ian's, dom1 boots up to:
> > 
> > VFS: Cannot open root device "(null)" or unknown-block(2,0)
> 
> All applied, thanks, (actually yesterday, I seem to have forgotten to hit send here)
> 
> > Stefano Stabellini (5):
> >       arm/vtimer: do not let the guest interact with the physical timer
> >       arm/gic: fix gic context switch
> >       xen/gic: support injecting IRQs even to VCPUs not currently running
> >       xen/vgic: vgic: support irq enable/disable
> >       xen/vgic: initialize pending_irqs.lr_queue
> 
> I rewrote these last three to "arm/foo" which I think is what you meant?

Yes it is, thanks