Re: [RFC PATCH v1 02/12] Arm: GICv3: Move the macros to compute the affnity level to arm64/arm32

[Xenia Ragiadakou <burzalodowa@xxxxxxxxx>]

On 10/21/22 18:31, Ayan Kumar Halder wrote:
Hi Ayan

> Refer https://elixir.bootlin.com/linux/v6.1-rc1/source/arch/arm64/ \
> include/asm/cputype.h#L14 , these macros are specific for arm64.
>
> When one computes MPIDR_LEVEL_SHIFT(3), it crosses the width of a 32
> bit register.
>
> Refer https://elixir.bootlin.com/linux/v6.1-rc1/source/arch/arm/include/ \
> asm/cputype.h#L54  , these macros are specific for arm32.
>
> Signed-off-by: Ayan Kumar Halder <ayankuma@xxxxxxx>
> ---
>   xen/arch/arm/include/asm/arm32/processor.h | 10 ++++++++++
>   xen/arch/arm/include/asm/arm64/processor.h | 13 +++++++++++++
>   xen/arch/arm/include/asm/processor.h       | 14 --------------
>   3 files changed, 23 insertions(+), 14 deletions(-)
>
> diff --git a/xen/arch/arm/include/asm/arm32/processor.h 
> b/xen/arch/arm/include/asm/arm32/processor.h
> index 4e679f3273..3e03ce78dc 100644
> --- a/xen/arch/arm/include/asm/arm32/processor.h
> +++ b/xen/arch/arm/include/asm/arm32/processor.h
> @@ -56,6 +56,16 @@ struct cpu_user_regs
>       uint32_t pad1; /* Doubleword-align the user half of the frame */
>   };
>   
> +/*
> + * Macros to extract affinity level. Picked from kernel
> + */
> +
> +#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)
> +
>   #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..c026334eec 100644
> --- a/xen/arch/arm/include/asm/arm64/processor.h
> +++ b/xen/arch/arm/include/asm/arm64/processor.h
> @@ -84,6 +84,19 @@ 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_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)
> +
>   #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..7d90c3b5f2 100644
> --- a/xen/arch/arm/include/asm/processor.h
> +++ b/xen/arch/arm/include/asm/processor.h
> @@ -118,20 +118,6 @@
>   #define MPIDR_INVALID       (~MPIDR_HWID_MASK)
>   #define MPIDR_LEVEL_BITS    (8)
>   
> -
> -/*
> - * 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)
> -
>   #define AFFINITY_MASK(level)    ~((_AC(0x1,UL) << MPIDR_LEVEL_SHIFT(level)) - 
> 1)
>   
>   /* TTBCR Translation Table Base Control Register */
Since only the definition of the MPIDR_AFFINITY_LEVEL() differs, maybe 
you could add only this one to the arch specific headers e.g
for arm64:
#define MPIDR_LEVEL_SHIFT(level) \
    (((1 << (level)) >> 1) << MPIDR_LEVEL_BITS_SHIFT)
for arm32:
#define MPIDR_LEVEL_SHIFT(level) \
    ((level) << MPIDR_LEVEL_BITS_SHIFT)

But in any case don't forget to add parentheses around the macro 
parameters when an operator acts on them to avoid trouble with operator 
precedence (MISRA-C Rule 20.7 :))
--
Xenia