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

[Ayan Kumar Halder <ayankuma@xxxxxxx>]

On 01/11/2022 08:58, Michal Orzel wrote:
> On 31/10/2022 18:53, Michal Orzel wrote:
> > Hi Ayan,
Hi Michal,
> > 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.
I was illustrating how the bits are represented in software

Refer 
https://elixir.bootlin.com/linux/latest/source/drivers/irqchip/irq-gic-v3.c#L625
I was trying to depict how "u64 
<https://elixir.bootlin.com/linux/latest/C/ident/u64> aff 
<https://elixir.bootlin.com/linux/latest/C/ident/aff>" bit 
representation differs between arm32 and arm64.
- Ayan

> > > 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