On Mon, 18 Jan 2021 21:16:08 +0800
Keqian Zhu <zhukeqian1@xxxxxxxxxx> wrote:
> On 2021/1/16 3:14, Alex Williamson wrote:
> > On Fri, 15 Jan 2021 17:26:43 +0800
> > Keqian Zhu <zhukeqian1@xxxxxxxxxx> wrote:
> >
> >> 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
