On 2026-04-22 at 10:24:19 -0700, Sohil Mehta wrote:
>On 4/10/2026 2:55 AM, Maciej Wieczor-Retman wrote:
>> diff --git a/arch/x86/kernel/process_64.c b/arch/x86/kernel/process_64.c
>> index 08e72f429870..d6f8e71156cd 100644
>> --- a/arch/x86/kernel/process_64.c
>> +++ b/arch/x86/kernel/process_64.c
>> @@ -797,7 +797,10 @@ static long prctl_map_vdso(const struct vdso_image
>> *image, unsigned long addr)
>>
>> #ifdef CONFIG_ADDRESS_MASKING
>>
>> -#define LAM_U57_BITS 6
>> +#define LAM_TAG_BITS 4
>
>This makes sense as the tag width would be fixed at 4.
>
>> +#define LAM_LS_BIT 57
>> +#define LAM_MS_BIT (LAM_LS_BIT + LAM_TAG_BITS - 1) /* 60 */
>> +#define LAM_UNTAG_MASK ~GENMASK(LAM_MS_BIT, LAM_LS_BIT)
>>
>
>This seems overkill. What we essentially need is 4 bits starting from
>bit 57. GENMASK is the right macro for that, but it doesn't need these
>extra argument macros and tail comments.
>
>I think using direct numbers with a simple comment on top would be
>better. How about something like?
>
>/* Must use the 4 lower bits from the LAM_U57 tag bits 62:57 */
>#define LAM_UNTAG_MASK ~GENMASK(60, 57)
Perhaps this is for the best. I was trying to keep some aspect of variability
here but there doesn't seem to be a point in doing that anymore. Thanks, I'll
get rid of these bit definitions.
>You can probably add some explanation in the commit log about why only
>the lower bits can be used.
>
>> static void enable_lam_func(void *__mm)
>> {
>> @@ -814,7 +817,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(62, 57);
>> + mm->context.untag_mask = LAM_UNTAG_MASK;
>>
>
>
>Other than that,
>
>Reviewed-by: Sohil Mehta <[email protected]>
--
Kind regards
Maciej Wieczór-Retman
