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

[Ard Biesheuvel]

On Thu, 11 Mar 2021 at 19:44, Will Deacon <will at kernel.org> wrote:
>
> 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.
>

Can't we just stop using VA_BITS altogether, and only keep the _MIN
and _MAX variants?