[PATCH v2 8/8] drm/panfrost: Remove unnecessary flushing from tlb_inv_context

Robin Murphy robin.murphy at arm.com
Fri Aug 23 14:05:44 UTC 2019


On 23/08/2019 14:18, Rob Herring wrote:
> On Fri, Aug 23, 2019 at 7:56 AM Robin Murphy <robin.murphy at arm.com> 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.

> 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.

Robin.


More information about the dri-devel mailing list