Re: [PATCH V6 02/10] xen/arm: Add functions of mapping between vCPUID and virtual affinity

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

On Mon, 2015-06-01 at 20:56 +0800, Chen Baozi wrote:
> From: Chen Baozi <baozich@gmail.com>
> 
> GICv3 restricts that the maximum number of CPUs in affinity 0 (one
> cluster) is 16.

Please add the reference to why this is.


>  That is to say the upper 4 bits of affinity 0 is unused.
> Current implementation considers that AFF0 is equal to vCPUID, which
> makes all vCPUs in one cluster, limiting its number to 16. If we would
> like to support more than 16 number of vCPU in one guest, we need to
> make use of AFF1. Considering the unused upper 4 bits, we need to create
> a pair of functions mapping the vCPUID and virtual affinity.
> 
> Signed-off-by: Chen Baozi <baozich@gmail.com>
> Reviewed-by: Julien Grall <julien.grall@citrix.com>
> ---
>  xen/include/asm-arm/domain.h | 41 +++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 41 insertions(+)
> 
> diff --git a/xen/include/asm-arm/domain.h b/xen/include/asm-arm/domain.h
> index 75b17af..b7b5cd2 100644
> --- a/xen/include/asm-arm/domain.h
> +++ b/xen/include/asm-arm/domain.h
> @@ -266,6 +266,47 @@ static inline unsigned int domain_max_vcpus(const struct domain *d)
>      return MAX_VIRT_CPUS;
>  }
>  
> +/*
> + * Due to the restriction of GICv3, the number of vCPUs in AFF0 is
> + * limited to 16,

Actually, maybe this is a better place to say why this is than the
commit message (or do both).

>                    thus only the first 4 bits of AFF0 are legal. We will
> + * use the first 2 affinity levels here, expanding the number of vCPU up
> + * to 4096 (16*256), which is more than 128 PEs that GIC-500 supports.
                                           ^the

(although I'm not sure a reference to GIC-500 here is all that useful)

> + * Since we don't save information of vCPU's topology (affinity) in
> + * vMPIDR at the moment, we map the vcpuid to the vMPIDR linearly.
> + *
> + * XXX: We may have multi-threading or virtual cluster information in
> + * the future.

We may, but I don't think that's worth mentioning here and now.

The code itself looks good, thanks.

> + */
> +static inline unsigned int vaffinity_to_vcpuid(register_t vaff)
> +{
> +    unsigned int vcpuid;
> +
> +    vaff &= MPIDR_HWID_MASK;
> +
> +    vcpuid = MPIDR_AFFINITY_LEVEL(vaff, 0);
> +    vcpuid |= MPIDR_AFFINITY_LEVEL(vaff, 1) << 4;
> +
> +    return vcpuid;
> +}
> +
> +static inline register_t vcpuid_to_vaffinity(unsigned int vcpuid)
> +{
> +    register_t vaff;
> +
> +    /*
> +     * Right now only AFF0 and AFF1 are supported in virtual affinity.
> +     * Since only the first 4 bits in AFF0 are used in GICv3, the
> +     * available bits are 12 (4+8).
> +     */
> +    BUILD_BUG_ON(!(MAX_VIRT_CPUS < ((1 << 12))));
> +
> +    vaff = (vcpuid & 0x0f) << MPIDR_LEVEL_SHIFT(0);
> +    vaff |= ((vcpuid >> 4) & MPIDR_LEVEL_MASK) << MPIDR_LEVEL_SHIFT(1);
> +
> +    return vaff;
> +}
> +
>  #endif /* __ASM_DOMAIN_H__ */
>  
>  /*