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

[Xenia Ragiadakou <burzalodowa@xxxxxxxxx>]

On 11/2/22 10:46, Julien Grall wrote:
> Hi,
>
> Title: The macros you are moving are not GICv3 specific.
>
> On 31/10/2022 15:13, Ayan Kumar Halder wrote:
> > Refer https://elixir.bootlin.com/linux/v6.1-rc1/source/arch/arm64/ \
> > include/asm/cputype.h#L14 , for the macros specific for arm64.
> >
> > Refer 
> > https://elixir.bootlin.com/linux/v6.1-rc1/source/arch/arm/include/ \
> > 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
> As discussed with Michal, AFF3 doesn't exist for 32-bit. So it is not 
> clear to me what we are gaining by moving the macros.
I cannot understand what do you mean by "what we are gaining by moving 
the macros".
IIUC, when identifying the cpu topology, a mask is applied to the value 
of MPIDR_EL1
#ifdef CONFIG_ARM_64
#define MPIDR_HWID_MASK     _AC(0xff00ffffff,UL)
#else
#define MPIDR_HWID_MASK     _AC(0xffffff,U)
#endif
So, for arm32, the affinity at level 3 is considered to be 0.

Do you mean, what we are gaining by defining the MPIDR_LEVEL_SHIFT in a 
different way for arm32 and for arm64?
IMO, we need to do so, because the shift, used to retrieve the affinity 
at each level, cannot be calculated using the same logic i.e
(((1 << (level)) >> 1) << MPIDR_LEVEL_BITS_SHIFT)

For arm32 the affinity at each level is calculated as follows
((level) << MPIDR_LEVEL_BITS_SHIFT)

Also, IMO, since MPIDR_HWID_MASK is defined in the common header, maybe 
the same needs to be done for MPIDR_LEVEL_SHIFT, to make easier for 
someone reading the code to understand the difference.
I hope I'm not out of context.

> Cheers,
--
Xenia