Re: [XEN v2 02/12] xen/Arm: GICv3: Move the macros to compute the affnity level to arm64/arm32

[Michal Orzel <michal.orzel@xxxxxxx>]

On 31/10/2022 18:53, Michal Orzel wrote:
> 
> 
> Hi Ayan,
> 
> On 31/10/2022 16:13, Ayan Kumar Halder wrote:
>>
>>
>> Refer 
>> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Felixir.bootlin.com%2Flinux%2Fv6.1-rc1%2Fsource%2Farch%2Farm64%2F&data=05%7C01%7Cmichal.orzel%40amd.com%7C0b2a0d1537104c2391d008dabb68eabb%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C638028356554609284%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=RhhL0XWxLJsO7vsP0DoP1QMvUMwGV%2F4FPJwAyvStj4k%3D&reserved=0
>>  \
> You should not split the link as it is becoming unusable in that form.
> 
>> include/asm/cputype.h#L14 , for the macros specific for arm64.
>>
>> Refer 
>> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Felixir.bootlin.com%2Flinux%2Fv6.1-rc1%2Fsource%2Farch%2Farm%2Finclude%2F&data=05%7C01%7Cmichal.orzel%40amd.com%7C0b2a0d1537104c2391d008dabb68eabb%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C638028356554609284%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=gLsNWm5%2BSyy51rn%2BA6H8PrWg8Yv%2BERicyyDjshOd3hc%3D&reserved=0
>>  \
> Same here.
> 
>> asm/cputype.h#L54  , for the macros specific for arm32.
>>
>> MPIDR_LEVEL_SHIFT() differs between 64 and 32 bit.
>> For 64 bit :-
>>
>>  aff_lev3          aff_lev2 aff_lev1 aff_lev0
>> |________|________|________|________|________|
>> 40       32       24       16       8        0
>>
>> For 32 bit :-
>>
>>  aff_lev3 aff_lev2 aff_lev1 aff_lev0
>> |________|________|________|________|
>> 32       24       16       8        0
>>
> 
> Where did you get this info from?
> FWICS by looking at ARM ARM DDI 0487I.a D17-6118,
> "Aff3 is not supported in AArch32 state."
We're talking about arm32 and not AArch32. My bad.
Nevertheless, looking at ARM ARM DDI 0406C.d B4-1644,
MPIDR for Armv7A/R also does not have aff3.

> 
> 
>> Signed-off-by: Ayan Kumar Halder <ayankuma@xxxxxxx>
>> ---
>>
>> Changes from :-
>> v1 - 1. Rearranged the macro defines so that the common code (between arm32
>> and arm64) is placed in "arm/include/asm/processor.h".
>>
>>  xen/arch/arm/include/asm/arm32/processor.h | 5 +++++
>>  xen/arch/arm/include/asm/arm64/processor.h | 8 ++++++++
>>  xen/arch/arm/include/asm/processor.h       | 6 ------
>>  3 files changed, 13 insertions(+), 6 deletions(-)
>>
>> diff --git a/xen/arch/arm/include/asm/arm32/processor.h 
>> b/xen/arch/arm/include/asm/arm32/processor.h
>> index 4e679f3273..82aa7f8d9d 100644
>> --- a/xen/arch/arm/include/asm/arm32/processor.h
>> +++ b/xen/arch/arm/include/asm/arm32/processor.h
>> @@ -56,6 +56,11 @@ struct cpu_user_regs
>>      uint32_t pad1; /* Doubleword-align the user half of the frame */
>>  };
>>
>> +/*
>> + * Macros to extract affinity level. Picked from kernel
>> + */
> No need for a multiline comment here and everywhere else.
> 
>> +#define MPIDR_LEVEL_SHIFT(level) (MPIDR_LEVEL_BITS * (level))
>> +
>>  #endif
>>
>>  #endif /* __ASM_ARM_ARM32_PROCESSOR_H */
>> diff --git a/xen/arch/arm/include/asm/arm64/processor.h 
>> b/xen/arch/arm/include/asm/arm64/processor.h
>> index c749f80ad9..295483a9dd 100644
>> --- a/xen/arch/arm/include/asm/arm64/processor.h
>> +++ b/xen/arch/arm/include/asm/arm64/processor.h
>> @@ -84,6 +84,14 @@ struct cpu_user_regs
>>      uint64_t sp_el1, elr_el1;
>>  };
>>
>> +/*
>> + * Macros to extract affinity level. picked from kernel
>> + */
>> +#define MPIDR_LEVEL_BITS_SHIFT  3
>> +
>> +#define MPIDR_LEVEL_SHIFT(level) \
>> +         (((1 << (level)) >> 1) << MPIDR_LEVEL_BITS_SHIFT)
>> +
> You should move these macros below __DECL_REG as they do not require having 
> it defined.
> 
>>  #undef __DECL_REG
>>
>>  #endif /* __ASSEMBLY__ */
>> diff --git a/xen/arch/arm/include/asm/processor.h 
>> b/xen/arch/arm/include/asm/processor.h
>> index 1dd81d7d52..ecfb62bbbe 100644
>> --- a/xen/arch/arm/include/asm/processor.h
>> +++ b/xen/arch/arm/include/asm/processor.h
>> @@ -122,13 +122,7 @@
>>  /*
>>   * Macros to extract affinity level. picked from kernel
>>   */
>> -
>> -#define MPIDR_LEVEL_BITS_SHIFT  3
>>  #define MPIDR_LEVEL_MASK        ((1 << MPIDR_LEVEL_BITS) - 1)
>> -
>> -#define MPIDR_LEVEL_SHIFT(level) \
>> -         (((1 << (level)) >> 1) << MPIDR_LEVEL_BITS_SHIFT)
>> -
>>  #define MPIDR_AFFINITY_LEVEL(mpidr, level) \
>>           (((mpidr) >> MPIDR_LEVEL_SHIFT(level)) & MPIDR_LEVEL_MASK)
>>
>> --
>> 2.17.1
>>
>>
> 
> ~Michal
>