On Fri, Aug 23, 2019 at 9:05 AM Robin Murphy <robin.murphy@xxxxxxx> wrote: > > On 23/08/2019 14:18, Rob Herring wrote: > > On Fri, Aug 23, 2019 at 7:56 AM Robin Murphy <robin.murphy@xxxxxxx> wrote: > >> > >> On 23/08/2019 03:12, Rob Herring wrote: > >>> tlb_inv_context() hook is only called when freeing the page tables. There's > >>> no point in flushing only to free the page tables immediately following. > >> > >> FWIW, in general the point of flushing is *because* we're about to free > >> the pagetables - if there's any possibility that the hardware could > >> continue to issue translation table walks (speculative or otherwise) > >> after those pages have been reused by someone else, TLB badness may ensue. > >> > >> For panfrost in particular I suspect we can probably get away without > >> it, at least for the moment, but it might be worth moving the flush to > >> mmu_disable() for complete peace of mind (which kind of preempts the > >> sort of thing that per-process AS switching will want anyway). > > > > There's bigger problem that mmu_disable() is still only called for AS0 > > and only for driver unload. > > Sure, but AS0 is the only one we ever enable, and conceptually we do > that once at probe time (AFAICS it stays nominally live through resets > and resumes), so while it may be the least-clever AS usage possible it's > at least self-consistent. And at the moment the only time we'll call > .tlb_inv_context is from panfrost_mmu_fini() when we're unconditionally > poking registers anyway, so either way I don't think there's any actual > problem today - I'm viewing this patch as getting things straight in > preparation for the future. It was only AS0, but we now have per FD AS. > > I guess we should fix that and then figure > > out where a flush is needed if at all. I would think changing the TTBR > > would be enough to quiesce the h/w and TLBs. That seems to be the case > > in my testing of switching address spaces. > > From a quick scan through kbase, kbase_mmu_disable() appears to perform > an full FLUSH_MEM before rewriting TRANSTAB, and it looks like that does > get called when scheduling out a context. That's in line with what I was > imagining, so unless we know better for sure, we may as well play it > safe and follow the same pattern. Agreed. Rob _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/dri-devel
