[PATCH 3/5] arm64: mm: use a compile time constant for vabits_actual when possible

[Will Deacon]

On Thu, Mar 11, 2021 at 06:23:33PM +0100, Ard Biesheuvel wrote:
> On Thu, 11 Mar 2021 at 10:49, Will Deacon <will at kernel.org> wrote:
> >
> > On Wed, Mar 10, 2021 at 06:15:13PM +0100, Ard Biesheuvel wrote:
> > > The size of the kernel VA space is a compile time constant unless the
> > > kernel is built to support 52-bit virtual addressing, which today is
> > > only supported on 64k page size kernels (although this has recently
> > > changed in the architecture).
> > >
> > > This means that in many configurations, vabits_actual can never deviate
> > > from its build time default, making it rather pointless to carry this
> > > value in a variable. So use a compile time constant for vabits_actual
> > > unless it can really assume different values.
> > >
> > > Signed-off-by: Ard Biesheuvel <ardb at kernel.org>
> > > ---
> > >  arch/arm64/include/asm/memory.h |  4 ++++
> > >  arch/arm64/kernel/head.S        | 12 ++++++------
> > >  arch/arm64/mm/mmu.c             |  2 ++
> > >  3 files changed, 12 insertions(+), 6 deletions(-)
> > >
> > > diff --git a/arch/arm64/include/asm/memory.h b/arch/arm64/include/asm/memory.h
> > > index c759faf7a1ff..501c5c87ec0a 100644
> > > --- a/arch/arm64/include/asm/memory.h
> > > +++ b/arch/arm64/include/asm/memory.h
> > > @@ -179,7 +179,11 @@
> > >  #include <linux/types.h>
> > >  #include <asm/bug.h>
> > >
> > > +#ifdef CONFIG_ARM64_VA_BITS_52
> > >  extern u64                   vabits_actual;
> > > +#else
> > > +#define vabits_actual                ((u64)VA_BITS)
> > > +#endif
> >
> > Maybe we should have VA_BITS_MIN, VA_BITS and VA_BITS_MAX instead of
> > the current VA_BITS_MIN, VA_BITS and vabits_actual? The current naming is
> > definitely a source of confusion for me.
> >
> 
> 
> 
> We will still need build time and runtime versions, and at build time,
> we sometimes need the minimum value (e.g, for sizing the vmalloc
> region) and sometimes the maximum value (e.g., in the definition of
> PTRS_PER_PGD). We could rename VA_BITS to VA_BITS_MAX, as you suggest,
> and find another name for vabits_actual, but we definitely need all
> these three quantities in one way or another.

Right, I was thinking that for !52-bit VA we've have:

#define VA_BITS		CONFIG_ARM64_VA_BITS
#define VA_BITS_MIN	VA_BITS_MIN
#define	VA_BITS_MAX	VA_BITS_MIN

and then for 52-bit VA we'd have:

#define VA_BITS		(vabits_actual)
#define VA_BITS_MIN	48
#define VA_BITS_MAX	52

> However, renaming vabits_actual to VA_BITS is likely to cause
> confusion as well, due to the change in meaning, as well as the fact
> that all-caps identifiers are usually build time constants.

Yeah, fair enough, but it reads a tonne better imo.

Will