[Xen-devel,03/17] ARM: vGICv3: always use architected redist stride

Message ID 20180309151133.31371-4-andre.przywara@linaro.org
State New
Headers show
Series
  • ARM: vGIC: prepare for splitting the vGIC code
Related show

Commit Message

Andre Przywara March 9, 2018, 3:11 p.m.
The redistributor-stride property in a GICv3 DT node is only there to
cover broken platforms where this value deviates from the architected one.
Since we emulate the GICv3 distributor even for Dom0, we don't need to
copy the broken behaviour. All the special handling for Dom0s using
GICv3 is just for using the hardware's memory map, which is unaffected
by the redistributor stride - it can never be smaller than the
architected two pages.
Remove the redistributor-stride property from Dom0's DT node and also
remove the code that tried to reuse the hardware value for Dom0's GICv3
emulation.

Signed-off-by: Andre Przywara <andre.przywara@linaro.org>
---
Changelog:
- merge in GICV3_GICR_SIZE definition

 xen/arch/arm/gic-v3.c             |  4 ----
 xen/arch/arm/vgic-v3.c            | 14 ++++++--------
 xen/include/asm-arm/gic_v3_defs.h |  5 +++++
 3 files changed, 11 insertions(+), 12 deletions(-)

Comments

Julien Grall March 12, 2018, 11:08 a.m. | #1
Hi Andre,

On 09/03/18 15:11, Andre Przywara wrote:
> The redistributor-stride property in a GICv3 DT node is only there to
> cover broken platforms where this value deviates from the architected one.
> Since we emulate the GICv3 distributor even for Dom0, we don't need to
> copy the broken behaviour. All the special handling for Dom0s using
> GICv3 is just for using the hardware's memory map, which is unaffected
> by the redistributor stride - it can never be smaller than the
> architected two pages.
> Remove the redistributor-stride property from Dom0's DT node and also
> remove the code that tried to reuse the hardware value for Dom0's GICv3
> emulation.
> 
> Signed-off-by: Andre Przywara <andre.przywara@linaro.org>

Acked-by: Julien Grall <julien.grall@arm.com>

Cheers,

> ---
> Changelog:
> - merge in GICV3_GICR_SIZE definition
> 
>   xen/arch/arm/gic-v3.c             |  4 ----
>   xen/arch/arm/vgic-v3.c            | 14 ++++++--------
>   xen/include/asm-arm/gic_v3_defs.h |  5 +++++
>   3 files changed, 11 insertions(+), 12 deletions(-)
> 
> diff --git a/xen/arch/arm/gic-v3.c b/xen/arch/arm/gic-v3.c
> index b1f8a86409..047af691b1 100644
> --- a/xen/arch/arm/gic-v3.c
> +++ b/xen/arch/arm/gic-v3.c
> @@ -1162,10 +1162,6 @@ static int gicv3_make_hwdom_dt_node(const struct domain *d,
>       if ( res )
>           return res;
>   
> -    res = fdt_property_cell(fdt, "redistributor-stride", gicv3.rdist_stride);
> -    if ( res )
> -        return res;
> -
>       res = fdt_property_cell(fdt, "#redistributor-regions", gicv3.rdist_count);
>       if ( res )
>           return res;
> diff --git a/xen/arch/arm/vgic-v3.c b/xen/arch/arm/vgic-v3.c
> index d5b34a7d0f..56cc38ffcc 100644
> --- a/xen/arch/arm/vgic-v3.c
> +++ b/xen/arch/arm/vgic-v3.c
> @@ -1024,10 +1024,9 @@ static struct vcpu *get_vcpu_from_rdist(struct domain *d,
>       paddr_t gpa, uint32_t *offset)
>   {
>       struct vcpu *v;
> -    uint32_t stride = d->arch.vgic.rdist_stride;
>       unsigned int vcpu_id;
>   
> -    vcpu_id = region->first_cpu + ((gpa - region->base) / stride);
> +    vcpu_id = region->first_cpu + ((gpa - region->base) / GICV3_GICR_SIZE);
>       if ( unlikely(vcpu_id >= d->max_vcpus) )
>           return NULL;
>   
> @@ -1586,7 +1585,6 @@ static int vgic_v3_vcpu_init(struct vcpu *v)
>   
>       /* Convenient alias */
>       struct domain *d = v->domain;
> -    uint32_t rdist_stride = d->arch.vgic.rdist_stride;
>   
>       /*
>        * Find the region where the re-distributor lives. For this purpose,
> @@ -1602,11 +1600,11 @@ static int vgic_v3_vcpu_init(struct vcpu *v)
>   
>       /* Get the base address of the redistributor */
>       rdist_base = region->base;
> -    rdist_base += (v->vcpu_id - region->first_cpu) * rdist_stride;
> +    rdist_base += (v->vcpu_id - region->first_cpu) * GICV3_GICR_SIZE;
>   
>       /* Check if a valid region was found for the re-distributor */
>       if ( (rdist_base < region->base) ||
> -         ((rdist_base + rdist_stride) > (region->base + region->size)) )
> +         ((rdist_base + GICV3_GICR_SIZE) > (region->base + region->size)) )
>       {
>           dprintk(XENLOG_ERR,
>                   "d%u: Unable to find a re-distributor for VCPU %u\n",
> @@ -1622,7 +1620,7 @@ static int vgic_v3_vcpu_init(struct vcpu *v)
>        * VGIC_V3_RDIST_LAST flags.
>        * Note that we are assuming max_vcpus will never change.
>        */
> -    last_cpu = (region->size / rdist_stride) + region->first_cpu - 1;
> +    last_cpu = (region->size / GICV3_GICR_SIZE) + region->first_cpu - 1;
>   
>       if ( v->vcpu_id == last_cpu || (v->vcpu_id == (d->max_vcpus - 1)) )
>           v->arch.vgic.flags |= VGIC_V3_RDIST_LAST;
> @@ -1693,7 +1691,7 @@ static int vgic_v3_domain_init(struct domain *d)
>               /* Set the first CPU handled by this region */
>               d->arch.vgic.rdist_regions[i].first_cpu = first_cpu;
>   
> -            first_cpu += size / d->arch.vgic.rdist_stride;
> +            first_cpu += size / GICV3_GICR_SIZE;
>           }
>   
>           d->arch.vgic.intid_bits = vgic_v3_hw.intid_bits;
> @@ -1708,7 +1706,7 @@ static int vgic_v3_domain_init(struct domain *d)
>           d->arch.vgic.rdist_stride = GUEST_GICV3_RDIST_STRIDE;
>   
>           /* The first redistributor should contain enough space for all CPUs */
> -        BUILD_BUG_ON((GUEST_GICV3_GICR0_SIZE / GUEST_GICV3_RDIST_STRIDE) < MAX_VIRT_CPUS);
> +        BUILD_BUG_ON((GUEST_GICV3_GICR0_SIZE / GICV3_GICR_SIZE) < MAX_VIRT_CPUS);
>           d->arch.vgic.rdist_regions[0].base = GUEST_GICV3_GICR0_BASE;
>           d->arch.vgic.rdist_regions[0].size = GUEST_GICV3_GICR0_SIZE;
>           d->arch.vgic.rdist_regions[0].first_cpu = 0;
> diff --git a/xen/include/asm-arm/gic_v3_defs.h b/xen/include/asm-arm/gic_v3_defs.h
> index 65c9dc47cf..bb34d17eca 100644
> --- a/xen/include/asm-arm/gic_v3_defs.h
> +++ b/xen/include/asm-arm/gic_v3_defs.h
> @@ -18,6 +18,8 @@
>   #ifndef __ASM_ARM_GIC_V3_DEFS_H__
>   #define __ASM_ARM_GIC_V3_DEFS_H__
>   
> +#include <xen/sizes.h>
> +
>   /*
>    * Additional registers defined in GIC v3.
>    * Common GICD registers are defined in gic.h
> @@ -68,6 +70,9 @@
>   #define GICV3_GICD_IIDR_VAL          0x34c
>   #define GICV3_GICR_IIDR_VAL          GICV3_GICD_IIDR_VAL
>   
> +/* Two pages for the RD_base and SGI_base register frame. */
> +#define GICV3_GICR_SIZE              (2 * SZ_64K)
> +
>   #define GICR_CTLR                    (0x0000)
>   #define GICR_IIDR                    (0x0004)
>   #define GICR_TYPER                   (0x0008)
>

Patch

diff --git a/xen/arch/arm/gic-v3.c b/xen/arch/arm/gic-v3.c
index b1f8a86409..047af691b1 100644
--- a/xen/arch/arm/gic-v3.c
+++ b/xen/arch/arm/gic-v3.c
@@ -1162,10 +1162,6 @@  static int gicv3_make_hwdom_dt_node(const struct domain *d,
     if ( res )
         return res;
 
-    res = fdt_property_cell(fdt, "redistributor-stride", gicv3.rdist_stride);
-    if ( res )
-        return res;
-
     res = fdt_property_cell(fdt, "#redistributor-regions", gicv3.rdist_count);
     if ( res )
         return res;
diff --git a/xen/arch/arm/vgic-v3.c b/xen/arch/arm/vgic-v3.c
index d5b34a7d0f..56cc38ffcc 100644
--- a/xen/arch/arm/vgic-v3.c
+++ b/xen/arch/arm/vgic-v3.c
@@ -1024,10 +1024,9 @@  static struct vcpu *get_vcpu_from_rdist(struct domain *d,
     paddr_t gpa, uint32_t *offset)
 {
     struct vcpu *v;
-    uint32_t stride = d->arch.vgic.rdist_stride;
     unsigned int vcpu_id;
 
-    vcpu_id = region->first_cpu + ((gpa - region->base) / stride);
+    vcpu_id = region->first_cpu + ((gpa - region->base) / GICV3_GICR_SIZE);
     if ( unlikely(vcpu_id >= d->max_vcpus) )
         return NULL;
 
@@ -1586,7 +1585,6 @@  static int vgic_v3_vcpu_init(struct vcpu *v)
 
     /* Convenient alias */
     struct domain *d = v->domain;
-    uint32_t rdist_stride = d->arch.vgic.rdist_stride;
 
     /*
      * Find the region where the re-distributor lives. For this purpose,
@@ -1602,11 +1600,11 @@  static int vgic_v3_vcpu_init(struct vcpu *v)
 
     /* Get the base address of the redistributor */
     rdist_base = region->base;
-    rdist_base += (v->vcpu_id - region->first_cpu) * rdist_stride;
+    rdist_base += (v->vcpu_id - region->first_cpu) * GICV3_GICR_SIZE;
 
     /* Check if a valid region was found for the re-distributor */
     if ( (rdist_base < region->base) ||
-         ((rdist_base + rdist_stride) > (region->base + region->size)) )
+         ((rdist_base + GICV3_GICR_SIZE) > (region->base + region->size)) )
     {
         dprintk(XENLOG_ERR,
                 "d%u: Unable to find a re-distributor for VCPU %u\n",
@@ -1622,7 +1620,7 @@  static int vgic_v3_vcpu_init(struct vcpu *v)
      * VGIC_V3_RDIST_LAST flags.
      * Note that we are assuming max_vcpus will never change.
      */
-    last_cpu = (region->size / rdist_stride) + region->first_cpu - 1;
+    last_cpu = (region->size / GICV3_GICR_SIZE) + region->first_cpu - 1;
 
     if ( v->vcpu_id == last_cpu || (v->vcpu_id == (d->max_vcpus - 1)) )
         v->arch.vgic.flags |= VGIC_V3_RDIST_LAST;
@@ -1693,7 +1691,7 @@  static int vgic_v3_domain_init(struct domain *d)
             /* Set the first CPU handled by this region */
             d->arch.vgic.rdist_regions[i].first_cpu = first_cpu;
 
-            first_cpu += size / d->arch.vgic.rdist_stride;
+            first_cpu += size / GICV3_GICR_SIZE;
         }
 
         d->arch.vgic.intid_bits = vgic_v3_hw.intid_bits;
@@ -1708,7 +1706,7 @@  static int vgic_v3_domain_init(struct domain *d)
         d->arch.vgic.rdist_stride = GUEST_GICV3_RDIST_STRIDE;
 
         /* The first redistributor should contain enough space for all CPUs */
-        BUILD_BUG_ON((GUEST_GICV3_GICR0_SIZE / GUEST_GICV3_RDIST_STRIDE) < MAX_VIRT_CPUS);
+        BUILD_BUG_ON((GUEST_GICV3_GICR0_SIZE / GICV3_GICR_SIZE) < MAX_VIRT_CPUS);
         d->arch.vgic.rdist_regions[0].base = GUEST_GICV3_GICR0_BASE;
         d->arch.vgic.rdist_regions[0].size = GUEST_GICV3_GICR0_SIZE;
         d->arch.vgic.rdist_regions[0].first_cpu = 0;
diff --git a/xen/include/asm-arm/gic_v3_defs.h b/xen/include/asm-arm/gic_v3_defs.h
index 65c9dc47cf..bb34d17eca 100644
--- a/xen/include/asm-arm/gic_v3_defs.h
+++ b/xen/include/asm-arm/gic_v3_defs.h
@@ -18,6 +18,8 @@ 
 #ifndef __ASM_ARM_GIC_V3_DEFS_H__
 #define __ASM_ARM_GIC_V3_DEFS_H__
 
+#include <xen/sizes.h>
+
 /*
  * Additional registers defined in GIC v3.
  * Common GICD registers are defined in gic.h
@@ -68,6 +70,9 @@ 
 #define GICV3_GICD_IIDR_VAL          0x34c
 #define GICV3_GICR_IIDR_VAL          GICV3_GICD_IIDR_VAL
 
+/* Two pages for the RD_base and SGI_base register frame. */
+#define GICV3_GICR_SIZE              (2 * SZ_64K)
+
 #define GICR_CTLR                    (0x0000)
 #define GICR_IIDR                    (0x0004)
 #define GICR_TYPER                   (0x0008)