On Tue, 15 Sep 2020 08:30:42 +0800 Yan Zhao <yan.y.zhao@xxxxxxxxx> wrote: > vfio_pin_pages() and vfio_group_pin_pages() are used purely to mark dirty > pages to devices with IOMMU backend as they already have all VM pages > pinned at VM startup. This is wrong. The entire initial basis of mdev devices is for non-IOMMU backed devices which provide mediation outside of the scope of the IOMMU. That mediation includes interpreting device DMA programming and making use of the vfio_pin_pages() interface to translate and pin IOVA address to HPA. Marking pages dirty is a secondary feature. > when there're multiple devices in the vfio group, the dirty pages > marked through pin_pages interface by one device is not useful as the > other devices may access and dirty any VM pages. I don't know of any cases where there are multiple devices in a group that would make use of this interface, however, all devices within a group necessarily share an IOMMU context and any one device dirtying a page will dirty that page for all devices, so I don't see that this is a valid statement either. > So added a check such that only singleton IOMMU groups can pin pages > in vfio_group_pin_pages. for mdevs, there's always only one dev in a > vfio group. > This is a fix to the commit below that added a singleton IOMMU group > check in vfio_pin_pages. None of the justification above is accurate, please try again. Thanks, Alex > Fixes: 95fc87b44104 (vfio: Selective dirty page tracking if IOMMU backed > device pins pages) > > Signed-off-by: Yan Zhao <yan.y.zhao@xxxxxxxxx> > --- > drivers/vfio/vfio.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/drivers/vfio/vfio.c b/drivers/vfio/vfio.c > index 5e6e0511b5aa..2f0fa272ebf2 100644 > --- a/drivers/vfio/vfio.c > +++ b/drivers/vfio/vfio.c > @@ -2053,6 +2053,9 @@ int vfio_group_pin_pages(struct vfio_group *group, > if (!group || !user_iova_pfn || !phys_pfn || !npage) > return -EINVAL; > > + if (group->dev_counter > 1) > + return -EINVAL; > + > if (npage > VFIO_PIN_PAGES_MAX_ENTRIES) > return -E2BIG; >
