Re: [PATCH v3 07/26] x86/mm: introduce mm-local region

[Brendan Jackman]

On Mon Aug 3, 2026 at 11:29 PM BST, Yosry Ahmed wrote:
...
>> +#ifdef CONFIG_MM_LOCAL_REGION
>> +static inline void mm_local_region_free(struct mm_struct *mm)
>> +{
>> +	if (!mm_local_region_used(mm))
>> +		return;
>> +
>> +	struct mmu_gather tlb;
>> +	unsigned long start = MM_LOCAL_BASE_ADDR;
>> +	unsigned long end = MM_LOCAL_END_ADDR;
>
> These declarations should probably go at the beginning of the function.

Oops, ack.

>> +
>> +	/*
>> +	 * Although free_pgd_range() is intended for freeing user
>> +	 * page-tables, it also works out for kernel mappings on x86.
>> +	 * Use tlb_gather_mmu_fullmm() to avoid confusing the
>> +	 * range-tracking logic in __tlb_adjust_range().
>> +	 */
>> +	tlb_gather_mmu_fullmm(&tlb, mm);
>> +	free_pgd_range(&tlb, start, end, start, end);
>> +	tlb_finish_mmu(&tlb);
>> +
>> +	mm_flags_clear(MMF_LOCAL_REGION_USED, mm);
>> +}
>> +
>> +#if defined(CONFIG_MITIGATION_PAGE_TABLE_ISOLATION) && defined(CONFIG_X86_PAE)
>
> Would it be clearer to have nested #ifdefs instead?
>
> #ifdef CONFIG_MITIGATION_PAGE_TABLE_ISOLATION
>
> #ifdef CONFIG_X86_PAE
> ...
> #else /* CONFIG_X86_PAE */
> ...
> #endif /* CONFIG_X86_PAE */
>
> #else /* CONFIG_MITIGATION_PAGE_TABLE_ISOLATION */
>
> #endif /* CONFIG_MITIGATION_PAGE_TABLE_ISOLATION */
>
> Maybe not, just thinking out loud.

Hm, I wrote it out in the editor and no I don't think it's clearer. I
think as the reader it just means you basically have to reconstruct the
&&/elif in your head since you need to see this as a "three-headed if"
for it to make any sense.

...
>> +#elif defined(CONFIG_MITIGATION_PAGE_TABLE_ISOLATION)
>> +static inline int mm_local_map_to_user(struct mm_struct *mm)
>> +{
>> +	pgd_t *pgd;
>> +	int err;
>> +
>> +	err = preallocate_sub_pgd(mm, MM_LOCAL_BASE_ADDR);
>> +	if (err)
>> +		return err;
>> +
>> +	pgd = pgd_offset(mm, MM_LOCAL_BASE_ADDR);
>> +	set_pgd(kernel_to_user_pgdp(pgd), *pgd);
>> +	return 0;
>> +}
>
> The code above bears a lot of similarity to the LDT code removed in
> patch 8, and reviewing them separately is annoying. I realize that they
> were a single patch in the previous version and Dave complained that it
> was too large.
>
> What if we go a different way:
> 1. Move the LDT functions that will be repurposed to mmu_context.h.
> 2. Rename the functions to the mm_local_* domain where needed.
> 3. Actually perform the switch for LDT to use mm local region.
>
> Maybe (2) and (3) should be combined, depending on what the git diff
> looks like.
>
> I think this will make the diffs much clearer, for example
> mm_local_map_to_user() mainly differ from map_ldt_struct_to_user() in
> preallocation.

Sounds fine to me, let's try it out and I'll come back here if it turns
out to be messy.

>> +#else
>> +static inline int mm_local_map_to_user(struct mm_struct *mm)
>> +{
>> +	WARN_ONCE(1, "mm_local_map_to_user() not implemented");
>> +	return -EINVAL;
>> +}
>> +#endif
> [..]
>> diff --git a/arch/x86/include/asm/pgtable_32_areas.h b/arch/x86/include/asm/pgtable_32_areas.h
>> index 921148b429676..7fccb887f8b33 100644
>> --- a/arch/x86/include/asm/pgtable_32_areas.h
>> +++ b/arch/x86/include/asm/pgtable_32_areas.h
>> @@ -30,9 +30,14 @@ extern bool __vmalloc_start_set; /* set once high_memory is set */
>>  #define CPU_ENTRY_AREA_BASE	\
>>  	((FIXADDR_TOT_START - PAGE_SIZE*(CPU_ENTRY_AREA_PAGES+1)) & PMD_MASK)
>>  
>> -#define LDT_BASE_ADDR		\
>> -	((CPU_ENTRY_AREA_BASE - PAGE_SIZE) & PMD_MASK)
>> +/*
>> + * On 32-bit the mm-local region is currently completely consumed by the LDT
>> + * remap.
>> + */
>> +#define MM_LOCAL_BASE_ADDR	((CPU_ENTRY_AREA_BASE - PAGE_SIZE) & PMD_MASK)
>> +#define MM_LOCAL_END_ADDR	(MM_LOCAL_BASE_ADDR + PMD_SIZE)
>>  
>> +#define LDT_BASE_ADDR		MM_LOCAL_BASE_ADDR
>>  #define LDT_END_ADDR		(LDT_BASE_ADDR + PMD_SIZE)
>>  
>>  #define PKMAP_BASE		\
>> diff --git a/arch/x86/include/asm/pgtable_64_types.h b/arch/x86/include/asm/pgtable_64_types.h
>> index 7eb61ef6a185f..1181565966405 100644
>> --- a/arch/x86/include/asm/pgtable_64_types.h
>> +++ b/arch/x86/include/asm/pgtable_64_types.h
>> @@ -5,8 +5,11 @@
>>  #include <asm/sparsemem.h>
>>  
>>  #ifndef __ASSEMBLER__
>> +#include <linux/build_bug.h>
>>  #include <linux/types.h>
>>  #include <asm/kaslr.h>
>> +#include <asm/page_types.h>
>> +#include <uapi/asm/ldt.h>
>>  
>>  /*
>>   * These are used to make use of C type-checking..
>> @@ -100,9 +103,12 @@ extern unsigned int ptrs_per_p4d;
>>  #define GUARD_HOLE_BASE_ADDR	(GUARD_HOLE_PGD_ENTRY << PGDIR_SHIFT)
>>  #define GUARD_HOLE_END_ADDR	(GUARD_HOLE_BASE_ADDR + GUARD_HOLE_SIZE)
>>  
>> -#define LDT_PGD_ENTRY		-240UL
>> -#define LDT_BASE_ADDR		(LDT_PGD_ENTRY << PGDIR_SHIFT)
>> -#define LDT_END_ADDR		(LDT_BASE_ADDR + PGDIR_SIZE)
>> +#define MM_LOCAL_PGD_ENTRY	-240UL
>> +#define MM_LOCAL_BASE_ADDR	(MM_LOCAL_PGD_ENTRY << PGDIR_SHIFT)
>> +#define MM_LOCAL_END_ADDR	((MM_LOCAL_PGD_ENTRY + 1) << PGDIR_SHIFT)
>
> Any reason not keep the current formula (i.e. MM_LOCAL_BASE_ADDR +
> PGDIR_SIZE)?

Er no I don't see any good reason I changed this.

>> +
>> +#define LDT_BASE_ADDR		MM_LOCAL_BASE_ADDR
>> +#define LDT_END_ADDR		(LDT_BASE_ADDR + PMD_SIZE)
>
> Looks like the LDT area was silently changed to PMD_SIZE here. I assume
> this is to give the rest of the pgd-mapped address space to the mermap,
> but maybe we should call this out explicitly, or do it when the mermap
> is introduced (or separately)?

Right. This might be a bug  that causes us to leak pagetables on some
platforms. Haven't checked as it gets fixed in the next commit
regardless, but let's just do what you suggested.