Re: [RFC PATCH 0/2] Invalidate secondary IOMMU TLB on permission upgrade

[Jason Gunthorpe]

On Tue, Jun 20, 2023 at 09:18:24PM +1000, Alistair Popple wrote:

> 1. Add a call to mmu_notifier_invalidate_secondary_tlbs() to the arm64
>    version of ptep_set_access_flags().

I prefer we modify the whole thing to only call
mmu_notifier_arch_invalidate_secondary_tlbs() (note the arch in the
name) directly beside the arch's existing tlb invalidation, and we
only need to define this for x86 and ARM arches that have secondary
TLB using drivers - eg it is very much not a generic general purpose
mmu notifier that has any meaning outside arch code.

> This is what this RFC series does as it is the simplest
> solution. Arguably this call should be made by generic kernel code
> though to catch other platforms that need it.

But that is the whole point, the generic kernel cannot and does not
know the rules for TLB invalidation the platform model requires.

> It is unclear if mmu_notifier_invalidate_secondary_tlbs() should be
> called from mmu_notifier_range_end(). Currently it is, as an analysis
> of existing code shows most code doesn't explicitly invalidate
> secondary TLBs and relies on it being called as part of the end()
> call.

If you do the above we don't need to answer this question. Calling it
unconditionally at the arches tlbi points is always correct.

> To solve that we could add secondary TLB invalidation calls to the TLB
> batching code, but that adds complexity so I'm not sure it's worth it
> but would appreciate feedback.

It sounds like the right direction to me.. Batching is going to be
important, we don't need to different verions of batching logic. We
already call the notifier from the batch infrastructure anyhow.

This still fits with the above as batching goes down to the arch's
flush_tlb_range()/etc which can carry the batch to the notifier. We
can decide if we leave the notifier call in the core code and let the
arch elide it or push it to the arch so it the call is more
consistently placed.

eg we have arch_tlbbatch_flush() on x86 too that looks kind of
suspicious that is already really funky to figure out where the
notifier is actually called from. Calling from arch code closer to the
actual TLB flush would be alot easier to reason about.

Jason