Re: [PATCH v3 03/19] xen/arm: p2m: Replace level_{orders, masks} arrays with XEN_PT_LEVEL_{ORDER, MASK}

[Julien Grall <julien@xxxxxxx>]

On 25/02/2022 08:27, Bertrand Marquis wrote:
> Hi Julien,
Hi Bertrand,

> > On 24 Feb 2022, at 22:41, Julien Grall <julien@xxxxxxx> wrote:
> >
> >
> >
> > On 22/02/2022 15:55, Bertrand Marquis wrote:
> > > Hi Julien,
> > Hi Bertrand,
> >
> > > > On 21 Feb 2022, at 10:22, Julien Grall <julien@xxxxxxx> wrote:
> > > >
> > > > From: Julien Grall <jgrall@xxxxxxxxxx>
> > > >
> > > > The array level_orders and level_masks can be replaced with the
> > > > recently introduced macros LEVEL_ORDER and LEVEL_MASK.
> > > >
> > > > Signed-off-by: Julien Grall <jgrall@xxxxxxxxxx>
> > > Reviewed-by: Bertrand Marquis <bertrand.marquis@xxxxxxx>
> > > One open question: At this stage the convenience aliases that you
> > > kept in include/asm/lpae.h are used in a very limited number of places.
> > I am not sure I would call it very limited:
> >
> > 42sh> ack "(FIRST|SECOND|THIRD)_(ORDER|SHIFT|MASK)" | wc -l
> > 65
> >
> > That's including the 9 definitions.
> My bad I looked with your full serie in my tree.
>
> > > Could we remove those and use only XEN_PT_LEVEL_* to make the
> > > code a bit more coherent.
> > I made an attempt in the past and it resulted to longer line in assembly. So I 
> > am on the fence to whether the aliases should be completely removed.
> >
> > At the same time, XEN_PT_LEVEL(...) is handy for places where we don't know at 
> > compile time the level.
> One other big argument for making the switch is that XEN_PT_LEVEL is far more 
> specific then FIRST_ORDER and others which are very unspecific names.
How about renaming them to XEN_PT_L0_ORDER? Or maybe XPT_L0_ORDER?

This would allows us to keep the assembly line relatively short.

Cheers,

--
Julien Grall