[iommufd PATCH v2 0/2] Make mdev driver dma_unmap callback tolerant to unmaps come before device open

[iommufd PATCH v2 0/2] Make mdev driver dma_unmap callback tolerant to unmaps come before device open

[Yi Liu]

Jason's "Connect VFIO to IOMMUFD" introduces vfio iommufd compat mode. Under
this mode, vfio_iommufd_bind() creates an access which has an unmap callback,
which can be called immediately. This means mdev drivers may receive unmap
requests before the mdev is opened. For now, there are only three drivers
(gvt, vfio-ap and vfio-ccw) providing dma_unmap(). vfio-ccw is fine with
such requests. While gvt-g and vfio-ap may have potential problem with such
requests due to internal implementation. This series tries to enhance the two
drivers.

This series is based on Jason's below branch.

https://github.com/jgunthorpe/linux/tree/iommufd

(commit: 41973418f6c8c241ed5647d1408d5b917f24dfd8)

Change:
v2:
 - Refine the cover letter and commit message of patch 0001 (Kevin)
 - Rename patch 0001 to better fit the commit message
 - Add r-b from Zhi for patch 0001
 - tweak iova range test to assume page-aligned for patch 0002 (Jason)
 - Remove break so all queues within range are removed for patch 0002 (Kevin)

v1: https://lore.kernel.org/kvm/20221123134832.429589-1-yi.l.liu@intel.com/

Cc: Tony Krowiak <akrowiak@linux.ibm.com>
Cc: Halil Pasic <pasic@linux.ibm.com>
Cc: Jason Herne <jjherne@linux.ibm.com>
Cc: linux-s390@vger.kernel.org
Cc: Zhenyu Wang <zhenyuw@linux.intel.com>
Cc: Zhi Wang <zhi.a.wang@intel.com>
Cc: Kevin Tian <kevin.tian@intel.com>
Cc: intel-gvt-dev@lists.freedesktop.org

Regards,
	Yi Liu

Matthew Rosato (1):
  vfio/ap: validate iova during dma_unmap and trigger irq disable

Yi Liu (1):
  i915/gvt: Move gvt mapping cache initialization to vGPU creation

 drivers/gpu/drm/i915/gvt/gvt.h    |  2 ++
 drivers/gpu/drm/i915/gvt/kvmgt.c  |  7 ++-----
 drivers/gpu/drm/i915/gvt/vgpu.c   |  2 ++
 drivers/s390/crypto/vfio_ap_ops.c | 18 +++++++++++++++++-
 4 files changed, 23 insertions(+), 6 deletions(-)

-- 
2.34.1

[iommufd PATCH v2 1/2] i915/gvt: Move gvt mapping cache initialization to vGPU creation

[Yi Liu]

vfio container registers .dma_unmap() callback after the device is opened.
So it's fine for mdev drivers to initialize internal mapping cache in
.open_device(). See vfio_device_container_register().

Now with iommufd an access ops with an unmap callback is registered
when the device is bound to iommufd which is before .open_device()
is called. This implies gvt's .dma_unmap() could be called before its
internal mapping cache is initialized.

The fix is moving gvt mapping cache initialization to vGPU creation.
While at it also move ptable initialization together.

Reviewed-by: Zhi Wang <zhi.a.wang@intel.com>
Signed-off-by: Yi Liu <yi.l.liu@intel.com>
---
 drivers/gpu/drm/i915/gvt/gvt.h   | 2 ++
 drivers/gpu/drm/i915/gvt/kvmgt.c | 7 ++-----
 drivers/gpu/drm/i915/gvt/vgpu.c  | 2 ++
 3 files changed, 6 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/i915/gvt/gvt.h b/drivers/gpu/drm/i915/gvt/gvt.h
index dbf8d7470b2c..a3a7e16078ba 100644
--- a/drivers/gpu/drm/i915/gvt/gvt.h
+++ b/drivers/gpu/drm/i915/gvt/gvt.h
@@ -754,6 +754,8 @@ void intel_gvt_debugfs_remove_vgpu(struct intel_vgpu *vgpu);
 void intel_gvt_debugfs_init(struct intel_gvt *gvt);
 void intel_gvt_debugfs_clean(struct intel_gvt *gvt);
 
+void gvt_cache_init(struct intel_vgpu *vgpu);
+void kvmgt_protect_table_init(struct intel_vgpu *info);
 int intel_gvt_page_track_add(struct intel_vgpu *info, u64 gfn);
 int intel_gvt_page_track_remove(struct intel_vgpu *info, u64 gfn);
 int intel_gvt_dma_pin_guest_page(struct intel_vgpu *vgpu, dma_addr_t dma_addr);
diff --git a/drivers/gpu/drm/i915/gvt/kvmgt.c b/drivers/gpu/drm/i915/gvt/kvmgt.c
index 7a45e5360caf..a9e4eda94057 100644
--- a/drivers/gpu/drm/i915/gvt/kvmgt.c
+++ b/drivers/gpu/drm/i915/gvt/kvmgt.c
@@ -322,7 +322,7 @@ static void gvt_cache_destroy(struct intel_vgpu *vgpu)
 	}
 }
 
-static void gvt_cache_init(struct intel_vgpu *vgpu)
+void gvt_cache_init(struct intel_vgpu *vgpu)
 {
 	vgpu->gfn_cache = RB_ROOT;
 	vgpu->dma_addr_cache = RB_ROOT;
@@ -330,7 +330,7 @@ static void gvt_cache_init(struct intel_vgpu *vgpu)
 	mutex_init(&vgpu->cache_lock);
 }
 
-static void kvmgt_protect_table_init(struct intel_vgpu *info)
+void kvmgt_protect_table_init(struct intel_vgpu *info)
 {
 	hash_init(info->ptable);
 }
@@ -671,9 +671,6 @@ static int intel_vgpu_open_device(struct vfio_device *vfio_dev)
 
 	vgpu->attached = true;
 
-	kvmgt_protect_table_init(vgpu);
-	gvt_cache_init(vgpu);
-
 	vgpu->track_node.track_write = kvmgt_page_track_write;
 	vgpu->track_node.track_flush_slot = kvmgt_page_track_flush_slot;
 	kvm_page_track_register_notifier(vgpu->vfio_device.kvm,
diff --git a/drivers/gpu/drm/i915/gvt/vgpu.c b/drivers/gpu/drm/i915/gvt/vgpu.c
index 56c71474008a..036e1a72a26b 100644
--- a/drivers/gpu/drm/i915/gvt/vgpu.c
+++ b/drivers/gpu/drm/i915/gvt/vgpu.c
@@ -382,6 +382,8 @@ int intel_gvt_create_vgpu(struct intel_vgpu *vgpu,
 
 	intel_gvt_update_reg_whitelist(vgpu);
 	mutex_unlock(&gvt->lock);
+	kvmgt_protect_table_init(vgpu);
+	gvt_cache_init(vgpu);
 	return 0;
 
 out_clean_sched_policy:
-- 
2.34.1

[iommufd PATCH v2 2/2] vfio/ap: validate iova during dma_unmap and trigger irq disable

[Yi Liu]

From: Matthew Rosato <mjrosato@linux.ibm.com>

Currently, each mapped iova is stashed in its associated vfio_ap_queue;
when we get an unmap request, validate that it matches with one or more
of these stashed values before attempting unpins.

Each stashed iova represents IRQ that was enabled for a queue.  Therefore,
if a match is found, trigger IRQ disable for this queue to ensure that
underlying firmware will no longer try to use the associated pfn after
the page is unpinned. IRQ disable will also handle the associated unpin.

Signed-off-by: Matthew Rosato <mjrosato@linux.ibm.com>
Signed-off-by: Yi Liu <yi.l.liu@intel.com>
---
 drivers/s390/crypto/vfio_ap_ops.c | 18 +++++++++++++++++-
 1 file changed, 17 insertions(+), 1 deletion(-)

diff --git a/drivers/s390/crypto/vfio_ap_ops.c b/drivers/s390/crypto/vfio_ap_ops.c
index 0b4cc8c597ae..8bf353d46820 100644
--- a/drivers/s390/crypto/vfio_ap_ops.c
+++ b/drivers/s390/crypto/vfio_ap_ops.c
@@ -1535,13 +1535,29 @@ static int vfio_ap_mdev_set_kvm(struct ap_matrix_mdev *matrix_mdev,
 	return 0;
 }
 
+static void unmap_iova(struct ap_matrix_mdev *matrix_mdev, u64 iova, u64 length)
+{
+	struct ap_queue_table *qtable = &matrix_mdev->qtable;
+	struct vfio_ap_queue *q;
+	int loop_cursor;
+
+	hash_for_each(qtable->queues, loop_cursor, q, mdev_qnode) {
+		if (q->saved_iova >= iova && q->saved_iova < iova + length)
+			vfio_ap_irq_disable(q);
+	}
+}
+
 static void vfio_ap_mdev_dma_unmap(struct vfio_device *vdev, u64 iova,
 				   u64 length)
 {
 	struct ap_matrix_mdev *matrix_mdev =
 		container_of(vdev, struct ap_matrix_mdev, vdev);
 
-	vfio_unpin_pages(&matrix_mdev->vdev, iova, 1);
+	mutex_lock(&matrix_dev->mdevs_lock);
+
+	unmap_iova(matrix_mdev, iova, length);
+
+	mutex_unlock(&matrix_dev->mdevs_lock);
 }
 
 /**
-- 
2.34.1

Re: [iommufd PATCH v2 0/2] Make mdev driver dma_unmap callback tolerant to unmaps come before device open

[Yi Liu]

On 2022/11/29 17:35, Yi Liu wrote:
> Jason's "Connect VFIO to IOMMUFD" introduces vfio iommufd compat mode. Under
> this mode, vfio_iommufd_bind() creates an access which has an unmap callback,
> which can be called immediately. This means mdev drivers may receive unmap
> requests before the mdev is opened. For now, there are only three drivers
> (gvt, vfio-ap and vfio-ccw) providing dma_unmap(). vfio-ccw is fine with
> such requests. While gvt-g and vfio-ap may have potential problem with such
> requests due to internal implementation. This series tries to enhance the two
> drivers.
> 
> This series is based on Jason's below branch.
> 
> https://github.com/jgunthorpe/linux/tree/iommufd
> 
> (commit: 41973418f6c8c241ed5647d1408d5b917f24dfd8)

it's resent as below link. Please ignore this series.

https://lore.kernel.org/kvm/20221129105831.466954-1-yi.l.liu@intel.com/

> Change:
> v2:
>   - Refine the cover letter and commit message of patch 0001 (Kevin)
>   - Rename patch 0001 to better fit the commit message
>   - Add r-b from Zhi for patch 0001
>   - tweak iova range test to assume page-aligned for patch 0002 (Jason)
>   - Remove break so all queues within range are removed for patch 0002 (Kevin)
> 
> v1: https://lore.kernel.org/kvm/20221123134832.429589-1-yi.l.liu@intel.com/
> 
> Cc: Tony Krowiak <akrowiak@linux.ibm.com>
> Cc: Halil Pasic <pasic@linux.ibm.com>
> Cc: Jason Herne <jjherne@linux.ibm.com>
> Cc: linux-s390@vger.kernel.org
> Cc: Zhenyu Wang <zhenyuw@linux.intel.com>
> Cc: Zhi Wang <zhi.a.wang@intel.com>
> Cc: Kevin Tian <kevin.tian@intel.com>
> Cc: intel-gvt-dev@lists.freedesktop.org
> 
> Regards,
> 	Yi Liu
> 
> Matthew Rosato (1):
>    vfio/ap: validate iova during dma_unmap and trigger irq disable
> 
> Yi Liu (1):
>    i915/gvt: Move gvt mapping cache initialization to vGPU creation
> 
>   drivers/gpu/drm/i915/gvt/gvt.h    |  2 ++
>   drivers/gpu/drm/i915/gvt/kvmgt.c  |  7 ++-----
>   drivers/gpu/drm/i915/gvt/vgpu.c   |  2 ++
>   drivers/s390/crypto/vfio_ap_ops.c | 18 +++++++++++++++++-
>   4 files changed, 23 insertions(+), 6 deletions(-)
> 

-- 
Regards,
Yi Liu