On 11/23/22 13:48, Liu, Yi L wrote:
> 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.
After reading code on Jason's tree, I think it would be nice to document
the requirements for a vfio driver somewhere, like what they must do and
must not do in the callbacks of vfio_device_ops. maybe in include/linux/vfio.h?
> 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.
>
> 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)
> {
> 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);
It would be nice that you can still keep the initialization in the kvmgt.c as they are
kvmgt related.
With the changes above: Reviewed-by: Zhi Wang <zhi.a.wang@xxxxxxxxx>
> return 0;
>
> out_clean_sched_policy:
