Re: [[iommufd] PATCH v3 1/2] i915/gvt: Move gvt mapping cache initialization to intel_vgpu_init_dev()

[Zhenyu Wang <zhenyuw@linux.intel.com>]

[-- Attachment #1: Type: text/plain, Size: 2694 bytes --]

On 2022.12.02 05:54:01 -0800, Yi Liu wrote:
> 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 init. While
> at it also move ptable initialization together.
> 
> 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
> Reviewed-by: Zhi Wang <zhi.a.wang@intel.com>
> Signed-off-by: Yi Liu <yi.l.liu@intel.com>
> ---
>  drivers/gpu/drm/i915/gvt/kvmgt.c | 18 ++++++++++++++----
>  1 file changed, 14 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/gvt/kvmgt.c b/drivers/gpu/drm/i915/gvt/kvmgt.c
> index 7a45e5360caf..aaf0d9e8da95 100644
> --- a/drivers/gpu/drm/i915/gvt/kvmgt.c
> +++ b/drivers/gpu/drm/i915/gvt/kvmgt.c
> @@ -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,
> @@ -718,6 +715,11 @@ static void intel_vgpu_close_device(struct vfio_device *vfio_dev)
>  	kvmgt_protect_table_destroy(vgpu);
>  	gvt_cache_destroy(vgpu);
>  
> +	WARN_ON(vgpu->nr_cache_entries);
> +
> +	vgpu->gfn_cache = RB_ROOT;
> +	vgpu->dma_addr_cache = RB_ROOT;
> +
>  	intel_vgpu_release_msi_eventfd_ctx(vgpu);
>  
>  	vgpu->attached = false;
> @@ -1451,9 +1453,17 @@ static int intel_vgpu_init_dev(struct vfio_device *vfio_dev)
>  	struct intel_vgpu *vgpu = vfio_dev_to_vgpu(vfio_dev);
>  	struct intel_vgpu_type *type =
>  		container_of(mdev->type, struct intel_vgpu_type, type);
> +	int ret;
>  
>  	vgpu->gvt = kdev_to_i915(mdev->type->parent->dev)->gvt;
> -	return intel_gvt_create_vgpu(vgpu, type->conf);
> +	ret = intel_gvt_create_vgpu(vgpu, type->conf);
> +	if (ret)
> +		return ret;
> +
> +	kvmgt_protect_table_init(vgpu);
> +	gvt_cache_init(vgpu);
> +
> +	return 0;
>  }
>  
>  static void intel_vgpu_release_dev(struct vfio_device *vfio_dev)
> -- 

Reviewed-by: Zhenyu Wang <zhenyuw@linux.intel.com>

thanks!

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]