[Xen-changelog] [xen master] xen/arm: vgic-v3: Correctly retrieve the vCPU associated to a re-distributor

[patchbot@xxxxxxx]

commit 9b9d51e98edb8c5c731e2d06dfad3633053d88a4
Author:     Julien Grall <julien.grall@xxxxxxxxxx>
AuthorDate: Tue Sep 29 15:44:40 2015 +0100
Commit:     Ian Campbell <ian.campbell@xxxxxxxxxx>
CommitDate: Thu Oct 1 13:48:51 2015 +0100

    xen/arm: vgic-v3: Correctly retrieve the vCPU associated to a re-distributor
    
    When the guest is accessing the re-distributor, Xen retrieves the base
    of the re-distributor using a mask based on the stride.
    
    When the stride contains multiple bits set, the corresponding mask will be
    computed incorrectly [1] and therefore giving invalid vCPU and offset:
    
    (XEN) d0v0: vGICR: unknown gpa read address 000000008d130008
    (XEN) traps.c:2447:d0v1 HSR=0x93c08006 pc=0xffffffc00032362c
    gva=0xffffff80000b0008 gpa=0x0000008d130008
    
    For instance if the region of re-distributor is starting at 0x8d100000
    and the stride is 0x30000, an access to the address 0x8d130008 should
    be valid and use the re-distributor of vCPU1 with an offset of 0x8.
    Although, Xen is returning the vCPU0 and an offset of 0x20008.
    
    I didn't find a way to replace the current computation of the mask with
    a valid one. The only solution I have found is to pass the region in
    private data of the handler. So we can directly get the offset from the
    beginning of the region and find the corresponding vCPU/offset in the
    re-distributor.
    
    This is also make the code simpler and avoid fast/slow path.
    
    [1] http://lists.xen.org/archives/html/xen-devel/2015-09/msg03372.html
    
    Signed-off-by: Julien Grall <julien.grall@xxxxxxxxxx>
    Acked-by: Ian Campbell <ian.campbell@xxxxxxxxxx>
---
 xen/arch/arm/vgic-v3.c |   58 ++++++++++++++++-------------------------------
 1 files changed, 20 insertions(+), 38 deletions(-)

diff --git a/xen/arch/arm/vgic-v3.c b/xen/arch/arm/vgic-v3.c
index 6a4feb2..0a14184 100644
--- a/xen/arch/arm/vgic-v3.c
+++ b/xen/arch/arm/vgic-v3.c
@@ -593,55 +593,34 @@ write_ignore_32:
     return 1;
 }
 
-static inline struct vcpu *get_vcpu_from_rdist(paddr_t gpa,
-                                               struct vcpu *v,
-                                               uint32_t *offset)
+static struct vcpu *get_vcpu_from_rdist(struct domain *d,
+    const struct vgic_rdist_region *region,
+    paddr_t gpa, uint32_t *offset)
 {
-    struct domain *d = v->domain;
+    struct vcpu *v;
     uint32_t stride = d->arch.vgic.rdist_stride;
-    paddr_t base;
-    int i, vcpu_id;
-    struct vgic_rdist_region *region;
-
-    *offset = gpa & (stride - 1);
-    base = gpa & ~((paddr_t)stride - 1);
-
-    /* Fast path: the VCPU is trying to access its re-distributor */
-    if ( likely(v->arch.vgic.rdist_base == base) )
-        return v;
-
-    /* Slow path: the VCPU is trying to access another re-distributor */
-
-    /*
-     * Find the region where the re-distributor lives. For this purpose,
-     * we look one region ahead as only MMIO range for redistributors
-     * traps here.
-     * Note: The region has been ordered during the GIC initialization
-     */
-    for ( i = 1; i < d->arch.vgic.nr_regions; i++ )
-    {
-        if ( base < d->arch.vgic.rdist_regions[i].base )
-            break;
-    }
-
-    region = &d->arch.vgic.rdist_regions[i - 1];
-
-    vcpu_id = region->first_cpu + ((base - region->base) / stride);
+    unsigned int vcpu_id;
 
+    vcpu_id = region->first_cpu + ((gpa - region->base) / stride);
     if ( unlikely(vcpu_id >= d->max_vcpus) )
         return NULL;
 
-    return d->vcpu[vcpu_id];
+    v = d->vcpu[vcpu_id];
+
+    *offset = gpa - v->arch.vgic.rdist_base;
+
+    return v;
 }
 
 static int vgic_v3_rdistr_mmio_read(struct vcpu *v, mmio_info_t *info,
                                     void *priv)
 {
     uint32_t offset;
+    const struct vgic_rdist_region *region = priv;
 
     perfc_incr(vgicr_reads);
 
-    v = get_vcpu_from_rdist(info->gpa, v, &offset);
+    v = get_vcpu_from_rdist(v->domain, region, info->gpa, &offset);
     if ( unlikely(!v) )
         return 0;
 
@@ -661,10 +640,11 @@ static int vgic_v3_rdistr_mmio_write(struct vcpu *v, 
mmio_info_t *info,
                                      void *priv)
 {
     uint32_t offset;
+    const struct vgic_rdist_region *region = priv;
 
     perfc_incr(vgicr_writes);
 
-    v = get_vcpu_from_rdist(info->gpa, v, &offset);
+    v = get_vcpu_from_rdist(v->domain, region, info->gpa, &offset);
     if ( unlikely(!v) )
         return 0;
 
@@ -1212,10 +1192,12 @@ static int vgic_v3_domain_init(struct domain *d)
      * redistributor is targeted.
      */
     for ( i = 0; i < d->arch.vgic.nr_regions; i++ )
+    {
+        struct vgic_rdist_region *region = &d->arch.vgic.rdist_regions[i];
+
         register_mmio_handler(d, &vgic_rdistr_mmio_handler,
-            d->arch.vgic.rdist_regions[i].base,
-            d->arch.vgic.rdist_regions[i].size,
-            NULL);
+                              region->base, region->size, region);
+    }
 
     d->arch.vgic.ctlr = VGICD_CTLR_DEFAULT;
 
--
generated by git-patchbot for /home/xen/git/xen.git#master

_______________________________________________
Xen-changelog mailing list
Xen-changelog@xxxxxxxxxxxxx
http://lists.xensource.com/xen-changelog