Re: [PATCH V6 05/10] xen/arm64: gicv3: Use AFF1 when translating ICC_SGI1R_EL1 to cpumask

[Ian Campbell <ian.campbell@citrix.com>]

On Mon, 2015-06-01 at 20:56 +0800, Chen Baozi wrote:
> From: Chen Baozi <baozich@gmail.com>
> 
> To support more than 16 vCPUs, we have to calculate cpumask with AFF1
> field value in ICC_SGI1R_EL1.
> 
> Signed-off-by: Chen Baozi <baozich@gmail.com>
> ---
>  xen/arch/arm/vgic-v3.c            | 30 ++++++++++++++++++++++++++----
>  xen/include/asm-arm/gic_v3_defs.h |  2 ++
>  2 files changed, 28 insertions(+), 4 deletions(-)
> 
> diff --git a/xen/arch/arm/vgic-v3.c b/xen/arch/arm/vgic-v3.c
> index 2bf5294..f2b78a4 100644
> --- a/xen/arch/arm/vgic-v3.c
> +++ b/xen/arch/arm/vgic-v3.c
> @@ -972,13 +972,28 @@ write_ignore:
>      return 1;
>  }
>  
> -static inline void gicv3_sgir_to_cpumask(cpumask_t *cpumask,
> +static inline int gicv3_sgir_to_cpumask(cpumask_t *cpumask,
>                                           const register_t sgir)
>  {
>      unsigned long target_list;
> +    uint16_t *target_bitmap;
> +    unsigned int aff1;
>  
>      target_list = sgir & ICH_SGI_TARGETLIST_MASK;
> -    bitmap_copy(cpumask_bits(cpumask), &target_list, ICH_SGI_TARGET_BITS);
> +    /* We assume that only AFF1 is used in ICC_SGI1R_EL1. */
> +    aff1 = (sgir >> ICH_SGI_AFFINITY_LEVEL(1)) & ICH_SGI_AFFx_MASK;
> +
> +    /* There might be up to 4096 vCPUs with all bits in affinity 1
> +     * are used, so we have to check whether it will overflow the
> +     * bitmap array of cpumask_t.
> +     */
> +    if ( ((aff1 + 1) * ICH_SGI_TARGET_BITS) > NR_CPUS )
> +        return 1;
> +
> +    target_bitmap = (uint16_t *)cpumask_bits(cpumask);
> +    target_bitmap[aff1] = target_list;

I think this is another argument for passing the cluster and target list
separately at the affinity level.

> +
> +    return 0;
>  }
>  
>  static int vgic_v3_to_sgi(struct vcpu *v, register_t sgir)
> @@ -996,8 +1011,15 @@ static int vgic_v3_to_sgi(struct vcpu *v, register_t sgir)
>      switch ( irqmode )
>      {
>      case ICH_SGI_TARGET_LIST:
> -        /* SGI's are injected at Rdist level 0. ignoring affinity 1, 2, 3 */
> -        gicv3_sgir_to_cpumask(&vcpu_mask, sgir);
> +        /*
> +         * Currenty we assume only affinity level-1 is used in SGI's

"Currently"

> +         * injection, ignoring level 2 & 3.
> +         */
> +        if ( gicv3_sgir_to_cpumask(&vcpu_mask, sgir) )
> +        {
> +            gprintk(XENLOG_WARNING, "Wrong affinity in SGI1R_EL register\n");

I don't think we need to log this. The guest has asked to send an SGI to
a VCPU which we know can't possibly exist. I'm not sure what real h/w
would do, but if it is e.g. UNPREDICTABLE then we should consider
killing the guest here. I suspect it's actually just ignored, in which
case we can silently do the same.

Ian.