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

Mark Rutland
2026-03-03T13:12:58Z

Hi Catalin,

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>

> ---
> arch/arm64/include/asm/tlbflush.h | 12 +++++++++---
> 1 file changed, 9 insertions(+), 3 deletions(-)
>
> 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.

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.

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.

Thanks,
Mark.

> @@ -568,7 +574,7 @@ static inline void flush_tlb_kernel_range(unsigned long start, unsigned long end
> dsb(ishst);
> __flush_tlb_range_op(vaale1is, start, pages, stride, 0,
> TLBI_TTL_UNKNOWN, false, lpa2_is_enabled());
> - __tlbi_sync_s1ish();
> + __tlbi_sync_s1ish_kernel();
> isb();
> }
>
> @@ -582,7 +588,7 @@ static inline void __flush_tlb_kernel_pgtable(unsigned long kaddr)
>
> dsb(ishst);
> __tlbi(vaae1is, addr);
> - __tlbi_sync_s1ish();
> + __tlbi_sync_s1ish_kernel();
> isb();
> }
>