Re: [PATCH v3 6/8] ARM: Change MPIDR_AFFINITY_LEVEL to ignore Aff3

[Vladimir Murzin <vladimir.murzin@arm.com>]

On 09/09/16 17:59, Marc Zyngier wrote:
> On 08/09/16 17:06, Vladimir Murzin wrote:
>> vgic-v3 driver queries CPU affinity level up to Aff3, which is valid
>> for arm64.  However, for arm up to Aff2 levels are supported, so
>> querying for third level ends with upper bits of MPIDR are treated as
>> valid affinity level which is not true. Make sure we report zero for
>> any affinity level above two.
>>
>> Signed-off-by: Vladimir Murzin <vladimir.murzin@arm.com>
>> ---
>>  arch/arm/include/asm/cputype.h |    3 ++-
>>  1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/arch/arm/include/asm/cputype.h b/arch/arm/include/asm/cputype.h
>> index 1ee94c7..f08fac4 100644
>> --- a/arch/arm/include/asm/cputype.h
>> +++ b/arch/arm/include/asm/cputype.h
>> @@ -55,9 +55,10 @@
>>  
>>  #define MPIDR_LEVEL_BITS 8
>>  #define MPIDR_LEVEL_MASK ((1 << MPIDR_LEVEL_BITS) - 1)
>> +#define MPIDR_LEVEL_SHIFT(level) (MPIDR_LEVEL_BITS * level)
>>  
>>  #define MPIDR_AFFINITY_LEVEL(mpidr, level) \
>> -	((mpidr >> (MPIDR_LEVEL_BITS * level)) & MPIDR_LEVEL_MASK)
>> +	(((mpidr & MPIDR_HWID_BITMASK) >> MPIDR_LEVEL_SHIFT(level)) & MPIDR_LEVEL_MASK)
>>  
>>  #define ARM_CPU_IMP_ARM			0x41
>>  #define ARM_CPU_IMP_INTEL		0x69
>>
> 
> There is something I don't quite get. Is this patch really necessary?
> Are there cases where we construct a MPIDR that can have Aff3 set on 32bit?

I've just checked and it seems that I've been keeping the change for
MPIDR_AFFINITY_LEVEL() since vgic-old era where it was used with

static u32 compress_mpidr(unsigned long mpidr)
{
        u32 ret;

        ret = MPIDR_AFFINITY_LEVEL(mpidr, 0);
        ret |= MPIDR_AFFINITY_LEVEL(mpidr, 1) << 8;
        ret |= MPIDR_AFFINITY_LEVEL(mpidr, 2) << 16;
        ret |= MPIDR_AFFINITY_LEVEL(mpidr, 3) << 24;

        return ret;
}

and now that code gone and nobody passes level 3 into this macro, so I
can drop this change.

However, I do need MPIDR_LEVEL_SHIFT() macro since it is used in vgic-v3.

Should I leave patch as is or you have something in mind?

Cheers
Vladimir

> 
> Thanks,
> 
> 	M.
>