> 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.
>
> 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()
> {
> 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
