On 2022/11/24 15:07, Tian, Kevin wrote:
From: Liu, Yi L <yi.l.liu@xxxxxxxxx>
Sent: Wednesday, November 23, 2022 9:49 PM
vfio_iommufd_bind() creates an access which has an unmap callback, which
can be called immediately. So dma_unmap() callback should tolerate the
unmaps that come before the emulated device is opened.
this should first talk about how it works today and then why iommufd changes
it.
To achieve above, move the protect_table_init and gvt_cache_init into the
init op which is supposed to be triggered prior to the open_device() op.
what about below?
--
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.
much clearer :-)
Cc: Zhenyu Wang <zhenyuw@xxxxxxxxxxxxxxx>
Cc: Zhi Wang <zhi.a.wang@xxxxxxxxx>
Cc: Kevin Tian <kevin.tian@xxxxxxxxx>
Signed-off-by: Yi Liu <yi.l.liu@xxxxxxxxx>
---
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 579b230a0f58..cb21b1ba4162 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)
those are local functions. Just move to vgpu.c.
or you can remove the function wrap and directly put the internal lines
in intel_gvt_create_vgpu()
yes. maybe see Zhenyu and Zhi's input. which way is preferred by them.
{
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
--
Regards,
Yi Liu
