thr3ads.net - Xen devel - [PATCH v2 0/5] xen/arm: multiple guests support in the GIC and VGIC [Jul 2012]

If this information is useful, please help other people find it:
Share via:

Stefano Stabellini

2012-Jul-04 10:45 UTC

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

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

Stefano Stabellini

2012-Jul-04 10:45 UTC

head link

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

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

Stefano Stabellini

2012-Jul-04 10:45 UTC

head link

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

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

Stefano Stabellini

2012-Jul-04 10:45 UTC

head link

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

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

Stefano Stabellini

2012-Jul-04 10:45 UTC

head link

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

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

Stefano Stabellini

2012-Jul-04 10:45 UTC

head link

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

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

Ian Campbell

2012-Jul-18 08:59 UTC

head link

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

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.

Stefano Stabellini

2012-Jul-18 13:49 UTC

head link

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

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

Xen devel - Jul 2012 - [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

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

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

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

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

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

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

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