On Mon, 18 Jan 2021 21:16:08 +0800
Keqian Zhu [off-list ref] wrote:

On 2021/1/16 3:14, Alex Williamson wrote:

quoted

On Fri, 15 Jan 2021 17:26:43 +0800
Keqian Zhu [off-list ref] wrote:
  

quoted

vfio_sanity_check_pfn_list() is used to check whether pfn_list of
vfio_dma is empty when remove the external domain, so it makes a
wrong assumption that only external domain will add pfn to dma pfn_list.

Now we apply this check when remove a specific vfio_dma and extract
the notifier check just for external domain.  

The page pinning interface is gated by having a notifier registered for
unmaps, therefore non-external domains would also need to register a
notifier.  There's currently no other way to add entries to the
pfn_list.  So if we allow pinning for such domains, then it's wrong to
WARN_ON() when the notifier list is not-empty when removing an external
domain.  Long term we should probably extend page {un}pinning for the
caller to pass their notifier to be validated against the notifier list
rather than just allowing page pinning if *any* notifier is registered.
Thanks,  

I was misled by the code comments. So when the commit a54eb55045ae is
added, the only user of pin interface is mdev vendor driver, but now
we also allow iommu backed group to use this interface to constraint
dirty scope. Is vfio_iommu_unmap_unpin_all() a proper place to put
this WARN()?

vfio_iommu_unmap_unpin_all() deals with removing vfio_dmas, it's
logically unrelated to whether any driver is registered to receive
unmap notifications.  Thanks,

Alex