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

[Michal Orzel <michal.orzel@xxxxxxx>]

Hi Ayan,

On 31/10/2022 16:13, Ayan Kumar Halder wrote:
> 
> 
> Refer https://elixir.bootlin.com/linux/v6.1-rc1/source/arch/arm64/ \
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://elixir.bootlin.com/linux/v6.1-rc1/source/arch/arm/include/ \
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."


> 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