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

On Thu, 11 Mar 2021 at 19:44, Will Deacon [off-list ref] wrote:

On Thu, Mar 11, 2021 at 06:23:33PM +0100, Ard Biesheuvel wrote:

quoted

On Thu, 11 Mar 2021 at 10:49, Will Deacon [off-list ref] wrote:

quoted

On Wed, Mar 10, 2021 at 06:15:13PM +0100, Ard Biesheuvel wrote:

quoted

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

quoted

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?

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel