Re: [PATCH 0/2] iommu: Remove iommu_sva_ops::mm_exit()

[Jacob Pan <jacob.jun.pan@linux.intel.com>]

On Thu, 9 Apr 2020 16:50:58 +0200
Jean-Philippe Brucker <jean-philippe@linaro.org> 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.

Also, since you are not freeing ioasid in mmu_notifier release anymore,
does it mean the IOASID notifier chain can be non-atomic?

Thanks,

Jacob
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu