Re: [PATCH v5 18/25] arm64/mm: Split __flush_tlb_range() to elide trailing DSB

David Hildenbrand
2024-02-12T12:44:34Z

On 02.02.24 09:07, Ryan Roberts wrote:
> Split __flush_tlb_range() into __flush_tlb_range_nosync() +
> __flush_tlb_range(), in the same way as the existing flush_tlb_page()
> arrangement. This allows calling __flush_tlb_range_nosync() to elide the
> trailing DSB. Forthcoming "contpte" code will take advantage of this
> when clearing the young bit from a contiguous range of ptes.
>
> Tested-by: John Hubbard <jhubbard@nvidia.com>
> Signed-off-by: Ryan Roberts <ryan.roberts@arm.com>
> ---
> arch/arm64/include/asm/tlbflush.h | 13 +++++++++++--
> 1 file changed, 11 insertions(+), 2 deletions(-)
>
> diff --git a/arch/arm64/include/asm/tlbflush.h b/arch/arm64/include/asm/tlbflush.h
> index 79e932a1bdf8..50a765917327 100644
> --- a/arch/arm64/include/asm/tlbflush.h
> +++ b/arch/arm64/include/asm/tlbflush.h
> @@ -422,7 +422,7 @@ do { \
> #define __flush_s2_tlb_range_op(op, start, pages, stride, tlb_level) \
> __flush_tlb_range_op(op, start, pages, stride, 0, tlb_level, false, kvm_lpa2_is_enabled());
>
> -static inline void __flush_tlb_range(struct vm_area_struct *vma,
> +static inline void __flush_tlb_range_nosync(struct vm_area_struct *vma,
> unsigned long start, unsigned long end,
> unsigned long stride, bool last_level,
> int tlb_level)
> @@ -456,10 +456,19 @@ static inline void __flush_tlb_range(struct vm_area_struct *vma,
> __flush_tlb_range_op(vae1is, start, pages, stride, asid,
> tlb_level, true, lpa2_is_enabled());
>
> - dsb(ish);
> mmu_notifier_arch_invalidate_secondary_tlbs(vma->vm_mm, start, end);
> }
>
> +static inline void __flush_tlb_range(struct vm_area_struct *vma,
> + unsigned long start, unsigned long end,
> + unsigned long stride, bool last_level,
> + int tlb_level)
> +{
> + __flush_tlb_range_nosync(vma, start, end, stride,
> + last_level, tlb_level);
> + dsb(ish);
> +}
> +
> static inline void flush_tlb_range(struct vm_area_struct *vma,
> unsigned long start, unsigned long end)
> {

You're now calling dsb() after
mmu_notifier_arch_invalidate_secondary_tlbs().

In flush_tlb_mm(), we have the order

dsb(ish);
mmu_notifier_arch_invalidate_secondary_tlbs()

In flush_tlb_page(), we have the effective order:

mmu_notifier_arch_invalidate_secondary_tlbs()
dsb(ish);

In flush_tlb_range(), we used to have the order:

dsb(ish);
mmu_notifier_arch_invalidate_secondary_tlbs();

So I *suspect* having that DSB before
mmu_notifier_arch_invalidate_secondary_tlbs() is fine. Hopefully,
nothing in there relies on that placement.

Maybe wort spelling out in the patch description

Reviewed-by: David Hildenbrand <david@redhat.com>

--
Cheers,

David / dhildenb