Re: [PATCH v14 Kernel 7/7] vfio: Selective dirty page tracking if IOMMU backed device pins pages

Alex Williamson
2020-03-20T19:41:54Z

On Thu, 19 Mar 2020 02:24:33 -0400
Yan Zhao <yan.y.zhao@intel.com> wrote:
> On Thu, Mar 19, 2020 at 03:41:14AM +0800, Kirti Wankhede wrote:
> > diff --git a/drivers/vfio/vfio_iommu_type1.c b/drivers/vfio/vfio_iommu_type1.c
> > index 912629320719..deec09f4b0f6 100644
> > --- a/drivers/vfio/vfio_iommu_type1.c
> > +++ b/drivers/vfio/vfio_iommu_type1.c
> > @@ -72,6 +72,7 @@ struct vfio_iommu {
> > bool v2;
> > bool nesting;
> > bool dirty_page_tracking;
> > + bool pinned_page_dirty_scope;
> > };
> >
> > struct vfio_domain {
> > @@ -99,6 +100,7 @@ struct vfio_group {
> > struct iommu_group *iommu_group;
> > struct list_head next;
> > bool mdev_group; /* An mdev group */
> > + bool pinned_page_dirty_scope;
> > };
> >
> > struct vfio_iova {
> > @@ -132,6 +134,10 @@ struct vfio_regions {
> > static int put_pfn(unsigned long pfn, int prot);
> > static unsigned long vfio_pgsize_bitmap(struct vfio_iommu *iommu);
> >
> > +static struct vfio_group *vfio_iommu_find_iommu_group(struct vfio_iommu *iommu,
> > + struct iommu_group *iommu_group);
> > +
> > +static void update_pinned_page_dirty_scope(struct vfio_iommu *iommu);
> > /*
> > * This code handles mapping and unmapping of user data buffers
> > * into DMA'ble space using the IOMMU
> > @@ -556,11 +562,13 @@ static int vfio_unpin_page_external(struct vfio_dma *dma, dma_addr_t iova,
> > }
> >
> > static int vfio_iommu_type1_pin_pages(void *iommu_data,
> > + struct iommu_group *iommu_group,
> > unsigned long *user_pfn,
> > int npage, int prot,
> > unsigned long *phys_pfn)
> > {
> > struct vfio_iommu *iommu = iommu_data;
> > + struct vfio_group *group;
> > int i, j, ret;
> > unsigned long remote_vaddr;
> > struct vfio_dma *dma;
> > @@ -630,8 +638,14 @@ static int vfio_iommu_type1_pin_pages(void *iommu_data,
> > (vpfn->iova - dma->iova) >> pgshift, 1);
> > }
> > }
>
> Could you provide an interface lightweight than vfio_pin_pages for pass-through
> devices? e.g. vfio_mark_iova_dirty()
>
> Or at least allowing phys_pfn to be empty for pass-through devices.
>
> This is really inefficient:
> bitmap_set(dma->bitmap, (vpfn->iova - dma->iova) / pgsize, 1));
> i.e.
> in order to mark an iova dirty, it has to go through iova ---> pfn --> iova
> while acquiring pfn is not necessary for pass-through devices.

I think this would be possible, but I don't think it should be gating
to this series. We don't have such consumers yet. Thanks,

Alex