On 7/14/26 07:04, Kevin Brodsky wrote:
...
> @@ -188,7 +188,7 @@ DECLARE_PER_CPU_SHARED_ALIGNED(struct tlb_state_shared,
> cpu_tlbstate_shared);
> #define enter_lazy_tlb enter_lazy_tlb
> static __always_inline void enter_lazy_tlb(struct mm_struct *mm, struct
> task_struct *tsk)
> {
> - if (this_cpu_read(cpu_tlbstate.loaded_mm) == &init_mm)
> + if (mm_is_kernel(this_cpu_read(cpu_tlbstate.loaded_mm)))
> return;
>
> this_cpu_write(cpu_tlbstate_shared.is_lazy, true);
This one makes me nervous, especially combined with the efi_mm changes
later in the series. init_mm really is special. Before this patch,
running on init_mm qualifies as being in lazy tlb mode. After this
patch, efi_mm qualifies too.
Is that OK?
> diff --git a/arch/x86/mm/pgtable.c b/arch/x86/mm/pgtable.c
> index f32facdb3035..91d2a6f2ac5a 100644
> --- a/arch/x86/mm/pgtable.c
> +++ b/arch/x86/mm/pgtable.c
> @@ -168,7 +168,7 @@ static int preallocate_pmds(struct mm_struct *mm, pmd_t
> *pmds[], int count)
> bool failed = false;
> gfp_t gfp = GFP_PGTABLE_USER;
>
> - if (mm == &init_mm)
> + if (mm_is_kernel(mm))
> gfp &= ~__GFP_ACCOUNT;
> gfp &= ~__GFP_HIGHMEM;
This hunk is probably fine.
> diff --git a/arch/x86/mm/tlb.c b/arch/x86/mm/tlb.c
> index 1023acadd8f8..5517cab8ad00 100644
> --- a/arch/x86/mm/tlb.c
> +++ b/arch/x86/mm/tlb.c
> @@ -594,7 +594,7 @@ void leave_mm(void)
> * This needs to happen before any other sanity checks due to
> * intel_idle's shenanigans.
> */
> - if (loaded_mm == &init_mm)
> + if (mm_is_kernel(loaded_mm))
> return;
This one is almost *certainly* buggy.
The efi_mm stuff uses leave_mm(). In fact, I think it's what's
responsible for switching the hardware back over to use the init_mm page
tables. If this check trips for init_mm, then the kernel will stay
running on efi_mm when it's not intended to.
Remember when I asked about whether there was going to be *a* kernel mm
or lots? This is where it starts to matter. It's outside the diff
context but the code here is basically:
if (loaded_mm == &init_mm)
return;
switch_mm(NULL, &init_mm, NULL);
So either all kernel mm's are equivalent or this code doesn't quite work.
> /* Warn if we're not lazy. */
> @@ -855,7 +855,7 @@ void switch_mm_irqs_off(struct mm_struct *unused, struct
> mm_struct *next,
> * cpu_tlbstate_shared.is_lazy whether or not to send an IPI.
> */
> if (IS_ENABLED(CONFIG_DEBUG_VM) &&
> - WARN_ON_ONCE(prev != &init_mm && !is_notrack_mm(prev) &&
> + WARN_ON_ONCE(!mm_is_kernel(prev) && !is_notrack_mm(prev) &&
> !cpumask_test_cpu(cpu, mm_cpumask(next))))
> cpumask_set_cpu(cpu, mm_cpumask(next));
>
> @@ -932,7 +932,8 @@ void switch_mm_irqs_off(struct mm_struct *unused, struct
> mm_struct *next,
> * This way switch_mm() must see the new tlb_gen or
> * flush_tlb_mm_range() must see the new loaded_mm, or both.
> */
> - if (next != &init_mm && !cpumask_test_cpu(cpu,
> mm_cpumask(next)))
> + if (!mm_is_kernel(next) &&
> + !cpumask_test_cpu(cpu, mm_cpumask(next)))
> cpumask_set_cpu(cpu, mm_cpumask(next));
> else
> smp_mb();
This one is interesting. It's basically saying that init_mm doesn't get
mm_cpumask() updates. That makes sense since it has special TLB flushing
rules. But what does this mean for efi_mm? Does it needs mm_cpumask()
updates?
