On Fri, Apr 10, 2020 at 08:52:49AM -0700, Jacob Pan wrote: > On Thu, 9 Apr 2020 16:50:58 +0200 > Jean-Philippe Brucker <jean-philippe@xxxxxxxxxx> wrote: > > > > So unbind is coming anyway, the difference in handling in mmu > > > release notifier is whether we silently drop DMA fault vs. > > > reporting fault? > > > > What I meant is, between mmu release notifier and unbind(), we can't > > print any error from DMA fault on dmesg, because an mm exit is easily > > triggered by userspace. Look at the lifetime of the bond: > > > > bind() > > | > > : Here any DMA fault is handled by mm, and on error we don't print > > : anything to dmesg. Userspace can easily trigger faults by issuing > > DMA : on unmapped buffers. > > | > > mm exit -> clear pgd, invalidate IOTLBs > > | > > : Here the PASID descriptor doesn't have the pgd anymore, but we > > don't : print out any error to dmesg either. DMA is likely still > > running but : any fault has to be ignored. > > : > > : We also can't free the PASID yet, since transactions are still > > coming : in with this PASID. > > | > > unbind() -> clear context descriptor, release PASID and mmu notifier > > | > > : Here the PASID descriptor is clear. If DMA is still running the > > device : driver really messed up and we have to print out any fault. > > > > For that middle state I had to introduce a new pasid descriptor state > > in the SMMU driver, to avoid reporting errors between mm exit and > > unbind(). > I must have missed something, but why bother with a state when you can > always check if the mm is dead by mmget_not_zero()? You would not > handle IOPF if the mm is dead anyway, similarly for other DMA errors. In the SMMU a cleared PASID descriptor results in unrecoverable faults, which do not go through the I/O page fault handler. I've been thinking about injecting everything to the IOPF handler, recoverable or not, but filtering down the stream is complicated. Most of the time outside this small window, we really need to print out those messages because they would indicate serious bugs. > Also, since you are not freeing ioasid in mmu_notifier release anymore, > does it mean the IOASID notifier chain can be non-atomic? Unfortunately not, ioasid_free() is called from mmu_notifier_ops::free_notifier() in the RCU callback that results from mmu_notifier_put(). Thanks, Jean
