Re: [PATCH v5 1/3] x86/process: Shorten the default LAM tag width

[Maciej Wieczor-Retman]

On 2026-04-07 at 14:30:32 -0700, Sohil Mehta wrote:
>On 4/7/2026 1:31 PM, Maciej Wieczor-Retman wrote:
>> On 2026-04-07 at 12:52:07 -0700, Sohil Mehta wrote:
>
>>>> +	mm->context.untag_mask =  ~GENMASK(57 + LAM_DEFAULT_BITS - 1, 57);
>>>>
>>>
>>> Also, would it be useful to calculate the LAM mask as a #define because
>>> it might need to be reused later or copied over to the selftest (as in
>>> patch 3)?
>> 
>> I think as it's only used during this initialization here it's not very useful
>> to give it a separate #define. And if it's only for selftest purposes it's
>> probably not worth it to export it to userspace.
>> 
>
>Oh, I didn't mean export to userspace. I just meant having these two
>defined together in arch/x86/kernel/process_64.c mainly for readability.
>It makes copying the define over to the selftest easier. The names also
>match ARCH_GET_MAX_TAG_BITS and ARCH_GET_UNTAG_MASK.
>
>#define LAM_TAG_BITS	4
>#define LAM_UNTAG_MASK	(~GENMASK(57 + LAM_TAG_BITS - 1, 57))
>
>To me, it makes the resulting code significantly more readable. I am
>suggesting it because you are already touching these lines in patch 1
>and 3. I'll leave it up to your/maintainer's preference, but personally
>I like the below result:
>
>diff --git a/arch/x86/kernel/process_64.c b/arch/x86/kernel/process_64.c
>index 1a0e96835bbc..745e16bde227 100644
>--- a/arch/x86/kernel/process_64.c
>+++ b/arch/x86/kernel/process_64.c
>@@ -797,7 +797,8 @@ static long prctl_map_vdso(const struct vdso_image
>*image, unsigned long addr)
>
> #ifdef CONFIG_ADDRESS_MASKING
>
>-#define LAM_DEFAULT_BITS	4
>+#define LAM_TAG_BITS		4
>+#define LAM_UNTAG_MASK		(~GENMASK(57 + LAM_TAG_BITS - 1, 57))
>
> static void enable_lam_func(void *__mm)
> {
>@@ -814,7 +815,7 @@ static void enable_lam_func(void *__mm)
> static void mm_enable_lam(struct mm_struct *mm)
> {
> 	mm->context.lam_cr3_mask = X86_CR3_LAM_U57;
>-	mm->context.untag_mask =  ~GENMASK(57 + LAM_DEFAULT_BITS - 1, 57);
>+	mm->context.untag_mask = LAM_UNTAG_MASK;
>
> 	/*
> 	 * Even though the process must still be single-threaded at this
>@@ -850,7 +851,7 @@ static int prctl_enable_tagged_addr(struct mm_struct
>*mm, unsigned long nr_bits)
> 		return -EBUSY;
> 	}
>
>-	if (!nr_bits || nr_bits > LAM_DEFAULT_BITS) {
>+	if (!nr_bits || nr_bits > LAM_TAG_BITS) {
> 		mmap_write_unlock(mm);
> 		return -EINVAL;
> 	}
>@@ -965,7 +966,7 @@ long do_arch_prctl_64(struct task_struct *task, int
>option, unsigned long arg2)
> 		if (!cpu_feature_enabled(X86_FEATURE_LAM))
> 			return put_user(0, (unsigned long __user *)arg2);
> 		else
>-			return put_user(LAM_DEFAULT_BITS, (unsigned long __user *)arg2);
>+			return put_user(LAM_TAG_BITS, (unsigned long __user *)arg2);
> #endif
> 	case ARCH_SHSTK_ENABLE:
> 	case ARCH_SHSTK_DISABLE:
>diff --git a/tools/testing/selftests/x86/lam.c
>b/tools/testing/selftests/x86/lam.c
>index d27f947ea694..4e514cae27f2 100644
>--- a/tools/testing/selftests/x86/lam.c
>+++ b/tools/testing/selftests/x86/lam.c
>@@ -17,6 +17,7 @@
> #include <sched.h>
>
> #include <sys/uio.h>
>+#include <linux/bits.h>
> #include <linux/io_uring.h>
> #include "kselftest.h"
>
>@@ -26,9 +27,9 @@
>
> /* LAM modes, these definitions were copied from kernel code */
> #define LAM_NONE                0
>-#define LAM_BITS                4
>+#define LAM_TAG_BITS		4
>+#define LAM_UNTAG_MASK		(~GENMASK(57 + LAM_TAG_BITS - 1, 57))
>
>-#define LAM_MASK                (0xfULL << 57)
> /* arch prctl for LAM */
> #define ARCH_GET_UNTAG_MASK     0x4001
> #define ARCH_ENABLE_TAGGED_ADDR 0x4002
...

Oh, okay, yeah, that does look better. Thanks, if there are no objections I'll
apply that!

-- 
Kind regards
Maciej Wieczór-Retman