Re: [PATCH v3 2/2] cxl: Enable global TLBIs for cxl contexts

[Alistair Popple <alistair@popple.id.au>]

On Fri, 8 Sep 2017 04:56:24 PM Nicholas Piggin wrote:
> On Sun,  3 Sep 2017 20:15:13 +0200
> Frederic Barrat <fbarrat@linux.vnet.ibm.com> wrote:
> 
> > The PSL and nMMU need to see all TLB invalidations for the memory
> > contexts used on the adapter. For the hash memory model, it is done by
> > making all TLBIs global as soon as the cxl driver is in use. For
> > radix, we need something similar, but we can refine and only convert
> > to global the invalidations for contexts actually used by the device.
> > 
> > The new mm_context_add_copro() API increments the 'active_cpus' count
> > for the contexts attached to the cxl adapter. As soon as there's more
> > than 1 active cpu, the TLBIs for the context become global. Active cpu
> > count must be decremented when detaching to restore locality if
> > possible and to avoid overflowing the counter.
> > 
> > The hash memory model support is somewhat limited, as we can't
> > decrement the active cpus count when mm_context_remove_copro() is
> > called, because we can't flush the TLB for a mm on hash. So TLBIs
> > remain global on hash.
> 
> Sorry I didn't look at this earlier and just wading in here a bit, but
> what do you think of using mmu notifiers for invalidating nMMU and
> coprocessor caches, rather than put the details into the host MMU
> management? npu-dma.c already looks to have almost everything covered
> with its notifiers (in that it wouldn't have to rely on tlbie coming
> from host MMU code).

Sorry, just finding time to catch up on this. From subsequent emails it looks
like you may have figured this out. The TLB flush in npu-dma.c is a workaround
for a HW issue rather than there to explicitly manage the NMMU caches. The
intent for NPU was always to have the NMMU snoop normal core tlbies rather than
do it via notifiers. A subsequent patch series
(https://patchwork.ozlabs.org/project/linuxppc-dev/list/?series=1681) removes
this flush now that the HW issue has been worked around via a FW fix.

I agree this is something we could look into optimising in the medium term, but
for the moment it would be good if we could get this series merged.

- Alistair