[PATCH v3 0/2] vgic emulation and GICD_ITARGETSR

[PATCH v3 0/2] vgic emulation and GICD_ITARGETSR

[Stefano Stabellini]

Hi all,
this small patch series improves vgic emulation in relation to
GICD_ITARGETSR and implements irq delivery to vcpus other than vcpu0.

vgic_enable_irqs and vgic_disable_irqs currently ignore the itarget
settings and just enable/disable irqs on the current vcpu. Fix their
behaviour to enable/disable irqs on the vcpu set by itarget, that is
always vcpu0 for irq >= 32.

Introduce a new vgic function called vgic_get_target_vcpu to retrieve
the right target vcpu (looking at itargets) and use it from do_IRQ.


Stefano Stabellini (2):
      xen/arm: observe itarget setting in vgic_enable_irqs and vgic_disable_irqs
      xen/arm: support irq delivery to vcpu > 0

 xen/arch/arm/gic.c        |    3 +--
 xen/arch/arm/irq.c        |    7 +++----
 xen/arch/arm/vgic.c       |   44 ++++++++++++++++++++++++++++++++++++--------
 xen/include/asm-arm/gic.h |    2 ++
 4 files changed, 42 insertions(+), 14 deletions(-)

[PATCH v3 1/2] xen/arm: observe itarget setting in vgic_enable_irqs and vgic_disable_irqs

[Stefano Stabellini]

vgic_enable_irqs should enable irq delivery to the vcpu specified by
GICD_ITARGETSR, rather than the vcpu that wrote to GICD_ISENABLER.
Similarly vgic_disable_irqs should use the target vcpu specified by
itarget to disable irqs.

Correctly initialize itargets for SPIs.

Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
Acked-by: Julien Grall <julien.grall@linaro.org>

---

Changes in v3:
- add assert in get_target_vcpu;
- rename get_target_vcpu to vgic_get_target_vcpu.

Changes in v2:
- refactor the common code in get_target_vcpu;
- unify PPI and SPI paths;
- correctly initialize itargets for SPI;
- use byte_read.
---
 xen/arch/arm/vgic.c |   44 ++++++++++++++++++++++++++++++++++++--------
 1 file changed, 36 insertions(+), 8 deletions(-)

diff --git a/xen/arch/arm/vgic.c b/xen/arch/arm/vgic.c
index cb8df3a..7614c2f 100644
--- a/xen/arch/arm/vgic.c
+++ b/xen/arch/arm/vgic.c
@@ -106,7 +106,15 @@ int domain_vgic_init(struct domain *d)
         INIT_LIST_HEAD(&d->arch.vgic.pending_irqs[i].lr_queue);
     }
     for (i=0; i<DOMAIN_NR_RANKS(d); i++)
+    {
+        int j;
+
         spin_lock_init(&d->arch.vgic.shared_irqs[i].lock);
+        /* Only delivery to CPU0 */
+        for ( j = 0 ; j < 8 ; j++ )
+            d->arch.vgic.shared_irqs[i].itargets[j] =
+                (1<<0) | (1<<8) | (1<<16) | (1<<24);
+    }
     return 0;
 }
 
@@ -369,6 +377,22 @@ read_as_zero:
     return 1;
 }
 
+static struct vcpu *vgic_get_target_vcpu(struct vcpu *v, unsigned int irq)
+{
+    int target;
+    struct vgic_irq_rank *rank;
+    struct vcpu *v_target;
+
+    rank = vgic_irq_rank(v, 1, irq/32);
+    vgic_lock_rank(v, rank);
+    target = byte_read(rank->itargets[(irq%32)/4], 0, irq % 4);
+    target = find_next_bit((const unsigned long *) &target, 8, 0);
+    ASSERT(target < v->domain->max_vcpus);
+    v_target = v->domain->vcpu[target];
+    vgic_unlock_rank(v, rank);
+    return v_target;
+}
+
 static void vgic_disable_irqs(struct vcpu *v, uint32_t r, int n)
 {
     const unsigned long mask = r;
@@ -376,12 +400,14 @@ static void vgic_disable_irqs(struct vcpu *v, uint32_t r, int n)
     unsigned int irq;
     unsigned long flags;
     int i = 0;
+    struct vcpu *v_target;
 
     while ( (i = find_next_bit(&mask, 32, i)) < 32 ) {
         irq = i + (32 * n);
-        p = irq_to_pending(v, irq);
+        v_target = vgic_get_target_vcpu(v, irq);
+        p = irq_to_pending(v_target, irq);
         clear_bit(GIC_IRQ_GUEST_ENABLED, &p->status);
-        gic_remove_from_queues(v, irq);
+        gic_remove_from_queues(v_target, irq);
         if ( p->desc != NULL )
         {
             spin_lock_irqsave(&p->desc->lock, flags);
@@ -399,24 +425,26 @@ static void vgic_enable_irqs(struct vcpu *v, uint32_t r, int n)
     unsigned int irq;
     unsigned long flags;
     int i = 0;
+    struct vcpu *v_target;
 
     while ( (i = find_next_bit(&mask, 32, i)) < 32 ) {
         irq = i + (32 * n);
-        p = irq_to_pending(v, irq);
+        v_target = vgic_get_target_vcpu(v, irq);
+        p = irq_to_pending(v_target, irq);
         set_bit(GIC_IRQ_GUEST_ENABLED, &p->status);
         /* We need to force the first injection of evtchn_irq because
          * evtchn_upcall_pending is already set by common code on vcpu
          * creation. */
-        if ( irq == v->domain->arch.evtchn_irq &&
+        if ( irq == v_target->domain->arch.evtchn_irq &&
              vcpu_info(current, evtchn_upcall_pending) &&
              list_empty(&p->inflight) )
-            vgic_vcpu_inject_irq(v, irq);
+            vgic_vcpu_inject_irq(v_target, irq);
         else {
             unsigned long flags;
-            spin_lock_irqsave(&v->arch.vgic.lock, flags);
+            spin_lock_irqsave(&v_target->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);
+                gic_raise_guest_irq(v_target, irq, p->priority);
+            spin_unlock_irqrestore(&v_target->arch.vgic.lock, flags);
         }
         if ( p->desc != NULL )
         {
-- 
1.7.10.4

[PATCH v3 2/2] xen/arm: support irq delivery to vcpu > 0

[Stefano Stabellini]

Export vgic_get_target_vcpu.
Use vgic_get_target_vcpu to retrieve the target vcpu from do_IRQ.
Route guest irqs to vcpu0 initially.
Remove in-code comments about missing implementation of SGI delivery to
vcpus other than 0.

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

diff --git a/xen/arch/arm/gic.c b/xen/arch/arm/gic.c
index 08ae23b..125ff36 100644
--- a/xen/arch/arm/gic.c
+++ b/xen/arch/arm/gic.c
@@ -287,8 +287,7 @@ void gic_route_irq_to_guest(struct domain *d, struct irq_desc *desc,
     gic_set_irq_properties(desc->irq, level, cpumask_of(smp_processor_id()),
                            GIC_PRI_IRQ);
 
-    /* TODO: do not assume delivery to vcpu0 */
-    p = irq_to_pending(d->vcpu[0], desc->irq);
+    p = irq_to_pending(d->vcpu[cpumask_first(cpu_mask)], desc->irq);
     p->desc = desc;
 }
 
diff --git a/xen/arch/arm/irq.c b/xen/arch/arm/irq.c
index a33c797..f886155 100644
--- a/xen/arch/arm/irq.c
+++ b/xen/arch/arm/irq.c
@@ -175,8 +175,7 @@ void do_IRQ(struct cpu_user_regs *regs, unsigned int irq, int is_fiq)
         desc->status |= IRQ_INPROGRESS;
         desc->arch.eoi_cpu = smp_processor_id();
 
-        /* XXX: inject irq into all guest vcpus */
-        vgic_vcpu_inject_irq(d->vcpu[0], irq);
+        vgic_vcpu_inject_irq(vgic_get_target_vcpu(d->vcpu[0], irq), irq);
         goto out_no_end;
     }
 
@@ -342,8 +341,8 @@ int route_dt_irq_to_guest(struct domain *d, const struct dt_irq *irq,
         goto out;
 
     level = dt_irq_is_level_triggered(irq);
-    gic_route_irq_to_guest(d, desc, level, cpumask_of(smp_processor_id()),
-                           GIC_PRI_IRQ);
+    /* route to vcpu0 initially */
+    gic_route_irq_to_guest(d, desc, level, cpumask_of(0), GIC_PRI_IRQ);
     spin_unlock_irqrestore(&desc->lock, flags);
     return 0;
 
diff --git a/xen/arch/arm/vgic.c b/xen/arch/arm/vgic.c
index 7614c2f..6a9c7f0 100644
--- a/xen/arch/arm/vgic.c
+++ b/xen/arch/arm/vgic.c
@@ -377,7 +377,7 @@ read_as_zero:
     return 1;
 }
 
-static struct vcpu *vgic_get_target_vcpu(struct vcpu *v, unsigned int irq)
+struct vcpu *vgic_get_target_vcpu(struct vcpu *v, unsigned int irq)
 {
     int target;
     struct vgic_irq_rank *rank;
diff --git a/xen/include/asm-arm/gic.h b/xen/include/asm-arm/gic.h
index bf6fb1e..bd40628 100644
--- a/xen/include/asm-arm/gic.h
+++ b/xen/include/asm-arm/gic.h
@@ -227,6 +227,8 @@ 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);
 
+struct vcpu *vgic_get_target_vcpu(struct vcpu *v, unsigned int irq);
+
 #endif /* __ASSEMBLY__ */
 #endif
 
-- 
1.7.10.4

Re: [PATCH v3 2/2] xen/arm: support irq delivery to vcpu > 0

[Julien Grall]

Hi Stefano,

You removed the bits to make ITARGET read-only. Why? With this patch
series, Xen doesn't validate ITARGET and a malicious guest could crash
Xen...

On 06/04/2014 03:35 PM, Stefano Stabellini wrote:
> Export vgic_get_target_vcpu.
> Use vgic_get_target_vcpu to retrieve the target vcpu from do_IRQ.
> Route guest irqs to vcpu0 initially.
> Remove in-code comments about missing implementation of SGI delivery to
> vcpus other than 0.
> 
> Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
> ---
>  xen/arch/arm/gic.c        |    3 +--
>  xen/arch/arm/irq.c        |    7 +++----
>  xen/arch/arm/vgic.c       |    2 +-
>  xen/include/asm-arm/gic.h |    2 ++
>  4 files changed, 7 insertions(+), 7 deletions(-)
> 
> diff --git a/xen/arch/arm/gic.c b/xen/arch/arm/gic.c
> index 08ae23b..125ff36 100644
> --- a/xen/arch/arm/gic.c
> +++ b/xen/arch/arm/gic.c
> @@ -287,8 +287,7 @@ void gic_route_irq_to_guest(struct domain *d, struct irq_desc *desc,
>      gic_set_irq_properties(desc->irq, level, cpumask_of(smp_processor_id()),
>                             GIC_PRI_IRQ);
>  
> -    /* TODO: do not assume delivery to vcpu0 */
> -    p = irq_to_pending(d->vcpu[0], desc->irq);
> +    p = irq_to_pending(d->vcpu[cpumask_first(cpu_mask)], desc->irq);

Hrmmm... you misused the mask here. cpumask contains a list a physical
CPU not Virtual CPU...

[..]

> @@ -342,8 +341,8 @@ int route_dt_irq_to_guest(struct domain *d, const struct dt_irq *irq,
>          goto out;
>  
>      level = dt_irq_is_level_triggered(irq);
> -    gic_route_irq_to_guest(d, desc, level, cpumask_of(smp_processor_id()),
> -                           GIC_PRI_IRQ);
> +    /* route to vcpu0 initially */
> +    gic_route_irq_to_guest(d, desc, level, cpumask_of(0), GIC_PRI_IRQ);

That makes the comment here wrong.

Regards,

-- 
Julien Grall