On Fri, 8 Sep 2017 04:56:24 PM Nicholas Piggin wrote: > On Sun, 3 Sep 2017 20:15:13 +0200 > Frederic Barrat <fbar...@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
