Re: [Xen-devel] [PATCH 2/7] ARM: vGICv3: drop GUEST_GICV3_RDIST_REGIONS symbol

[Julien Grall <julien.grall@xxxxxxxxxx>]

Hi Andre,

On 24/01/18 14:35, Andre Przywara wrote:
> Architecturally there is only one GICv3 redistributor region.
> Drop the symbol which suggested that was a delibarate choice for Xen
> guests, instead hard code the "1" in the appropriate places, along with
> a comment to explain the reasons.
>
> Signed-off-by: Andre Przywara <andre.przywara@xxxxxxxxxx>
> ---
>   xen/arch/arm/vgic-v3.c        | 17 ++++++++++++-----
>   xen/include/public/arch-arm.h |  1 -
>   2 files changed, 12 insertions(+), 6 deletions(-)
>
> diff --git a/xen/arch/arm/vgic-v3.c b/xen/arch/arm/vgic-v3.c
> index af16dfd005..7d3ea171b4 100644
> --- a/xen/arch/arm/vgic-v3.c
> +++ b/xen/arch/arm/vgic-v3.c
> @@ -1640,8 +1640,18 @@ static int vgic_v3_vcpu_init(struct vcpu *v)
>   
>   static inline unsigned int vgic_v3_rdist_count(struct domain *d)
>   {
> -    return is_hardware_domain(d) ? vgic_v3_hw.nr_rdist_regions :
> -               GUEST_GICV3_RDIST_REGIONS;
> +    /*
> +     * Architecturally there is only one GICv3 redistributor region.
> +     * The GICv3 DT binding provisions for multiple regions, since there are
> +     * platforms out there which break this architectural assumption.
> +     * ACPI does not support this workaround at all.
This is not true. The ACPI spec supports multiple regions of 
redistributors. What ACPI does not support is a different stride.
> +     * For Dom0 we have to live with the MMIO layout the hardware provides,
> +     * so we have to copy the multiple regions - as the first region may not
> +     * provide enough space to hold all redistributors we need.
> +     * However DomU get a constructed memory map, so we can go with
> +     * the architected single redistributor region.
> +     */
> +    return is_hardware_domain(d) ? vgic_v3_hw.nr_rdist_regions : 1; >   }
>   
>   static int vgic_v3_domain_init(struct domain *d)
> @@ -1700,9 +1710,6 @@ static int vgic_v3_domain_init(struct domain *d)
>       {
>           d->arch.vgic.dbase = GUEST_GICV3_GICD_BASE;
>   
> -        /* XXX: Only one Re-distributor region mapped for the guest */
> -        BUILD_BUG_ON(GUEST_GICV3_RDIST_REGIONS != 1);
> -
>           d->arch.vgic.rdist_stride = GUEST_GICV3_RDIST_STRIDE;
>   
>           /* The first redistributor should contain enough space for all CPUs */
> diff --git a/xen/include/public/arch-arm.h b/xen/include/public/arch-arm.h
> index 05fd11ca38..ca79ab6284 100644
> --- a/xen/include/public/arch-arm.h
> +++ b/xen/include/public/arch-arm.h
> @@ -402,7 +402,6 @@ typedef uint64_t xen_callback_t;
>   #define GUEST_GICV3_GICD_SIZE      xen_mk_ullong(0x00010000)
>   
>   #define GUEST_GICV3_RDIST_STRIDE   xen_mk_ullong(0x00020000)
> -#define GUEST_GICV3_RDIST_REGIONS  1
>   
>   #define GUEST_GICV3_GICR0_BASE     xen_mk_ullong(0x03020000) /* vCPU0..127 */
>   #define GUEST_GICV3_GICR0_SIZE     xen_mk_ullong(0x01000000)
Cheers,

--
Julien Grall

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