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

[Julien Grall <julien.grall@xxxxxxx>]

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@xxxxxxxxxx>
Acked-by: Julien Grall <julien.grall@xxxxxxx>

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)
--
Julien Grall

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel