Re: [PATCH 12/22] x86/mm: use mm_is_kernel() for kernel mm checks

[Dave Hansen <dave.hansen@intel.com>]

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?