lkml.org 
[lkml]   [2020]   [Sep]   [15]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH] vfio: add a singleton check for vfio_group_pin_pages
On Tue, Sep 15, 2020 at 01:30:11PM -0600, Alex Williamson wrote:
> On Tue, 15 Sep 2020 13:03:01 -0600
> Alex Williamson <alex.williamson@redhat.com> wrote:
>
> > On Tue, 15 Sep 2020 08:30:42 +0800
> > Yan Zhao <yan.y.zhao@intel.com> 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,
>
> FWIW, I think this should read something like "Page pinning is used
> both to translate and pin device mappings for DMA purpose, as well as
> to indicate to the IOMMU backend to limit the dirty page scope to those
> pages that have been pinned, in the case of an IOMMU backed device. To
> support this, the vfio_pin_pages() interface limits itself to only
> singleton groups such that the IOMMU backend can consider dirty page
> scope only at the group level. Implement the same requirement for the
> vfio_group_pin_pages() interface." Thanks,
>
yes, I'm sorry that I didn't express the meaning clearly.
will resend it using this version.

Thanks
Yan


>
>
> > > Fixes: 95fc87b44104 (vfio: Selective dirty page tracking if IOMMU backed
> > > device pins pages)
> > >
> > > Signed-off-by: Yan Zhao <yan.y.zhao@intel.com>
> > > ---
> > > 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;
> > >
> >
>

\
 
 \ /
  Last update: 2020-09-16 03:31    [W:0.040 / U:14.436 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site