Re: [PATCH 1/4] arm64: tlb: Use __tlbi_sync_s1ish_kernel() for kernel TLB maintenance

[Catalin Marinas <catalin.marinas@arm.com>]

On Tue, Mar 03, 2026 at 01:12:50PM +0000, Mark Rutland wrote:
> On Mon, Mar 02, 2026 at 04:57:54PM +0000, Catalin Marinas wrote:
> > Add __tlbi_sync_s1ish_kernel() similar to __tlbi_sync_s1ish() and use it
> > for kernel TLB maintenance. Also use this function in flush_tlb_all()
> > which is only used in relation to kernel mappings. Subsequent patches
> > can differentiate between workarounds that apply to user only or both
> > user and kernel.
> > 
> > Signed-off-by: Catalin Marinas <catalin.marinas@arm.com>
> > Cc: Will Deacon <will@kernel.org>
> > Cc: Mark Rutland <mark.rutland@arm.com>
> 
> This looks fine to me. I have one minor comment/naming nit below, but
> this looks functionally correct, and I'm happy to spin a follow-up for
> that.
> 
> With or without the changes below:
> 
> Acked-by: Mark Rutland <mark.rutland@arm.com>

Thanks Mark.

> > diff --git a/arch/arm64/include/asm/tlbflush.h b/arch/arm64/include/asm/tlbflush.h
> > index 1416e652612b..19be0f7bfca5 100644
> > --- a/arch/arm64/include/asm/tlbflush.h
> > +++ b/arch/arm64/include/asm/tlbflush.h
> > @@ -191,6 +191,12 @@ static inline void __tlbi_sync_s1ish(void)
> >  	__repeat_tlbi_sync(vale1is, 0);
> >  }
> >  
> > +static inline void __tlbi_sync_s1ish_kernel(void)
> > +{
> > +	dsb(ish);
> > +	__repeat_tlbi_sync(vale1is, 0);
> > +}
> > +
> >  /*
> >   * Complete broadcast TLB maintenance issued by hyp code which invalidates
> >   * stage 1 translation information in any translation regime.
> > @@ -299,7 +305,7 @@ static inline void flush_tlb_all(void)
> >  {
> >  	dsb(ishst);
> >  	__tlbi(vmalle1is);
> > -	__tlbi_sync_s1ish();
> > +	__tlbi_sync_s1ish_kernel();
> >  	isb();
> >  }
> 
> The commit message is correct that flush_tlb_all() is only used for
> kernel mappings today, via flush_tlb_kernel_range(), so this is safe.

Unfortunately, it's also used by the core code -
hugetlb_vmemmap_restore_folios() (and another function in this file).

> However, the big comment block around line 200 says:
> 
> 	flush_tlb_all()
> 		Invalidate the entire TLB (kernel + user) on all CPUs
> 
> ... and:
> 
> 	local_flush_tlb_all()
> 		Same as flush_tlb_all(), but only applies to the calling CPU.
> 
> ... where the latter is used for user mappings (upon ASID overflow), so
> I think there's some risk of future confusion.

Ignoring this erratum, the statements are still correct for arm64 as it
flushes both kernel and user, though I see what you mean w.r.t. its
intended use.

> To minimize the risk that flush_tlb_all() gets used for user mappings in
> future, how about we rename flush_tlb_all() => flush_tlb_kernel_all(), and
> update those comments:
> 
> 	flush_tlb_kernel_all()
> 		Invalidate all kernel mappings on all CPUs.
> 		Should not be used to invalidate user mappings.
> 
> 	local_flush_tlb_all()
> 		Invalidate all (kernel + user) mappings on the calling CPU.
> 
> Note: I chose flush_tlb_kernel_all() rather than flush_tlb_all_kernel()
> __flush_tlb_kernel_{pgtable,range}, with 'kernel' before the operation/scope.

I'm fine to update the comments but, for backporting, I'd not change the
function name as it will have to touch core code. Ideally we should go
around and change the other architectures to follow the same semantics
(I briefly looked at x86 and powerpc and they also seem to use
flush_tlb_all() only for kernel mappings).

So, I think it's better to do this cleanup separately ;).

-- 
Catalin