Re: [PATCH v4] drm/i915/gvt: Change KVMGT as self load module

["Yuan, Hang" <hang.yuan@intel.com>]

> -----Original Message-----
> From: intel-gvt-dev [mailto:intel-gvt-dev-bounces@lists.freedesktop.org] On
> Behalf Of Zhenyu Wang
> Sent: Tuesday, December 4, 2018 10:40 AM
> To: intel-gfx@lists.freedesktop.org
> Cc: Alex Williamson <alex.williamson@redhat.com>; intel-gvt-
> dev@lists.freedesktop.org
> Subject: [PATCH v4] drm/i915/gvt: Change KVMGT as self load module
> 
> This trys to make 'kvmgt' module as self loadable instead of loading by
> i915/gvt device model. So hypervisor specific module could be stand-alone,
> e.g only after loading hypervisor specific module, GVT feature could be
> enabled via specific hypervisor interface, e.g VFIO/mdev.
> 
> So this trys to use hypervisor module register/unregister interface for that.
> Hypervisor module needs to take care of module reference itself when
> working for hypervisor interface, e.g for VFIO/mdev, hypervisor module
> would reference counting mdev when open and release.
> 
> This makes 'kvmgt' module really split from GVT device model. User needs to
> load 'kvmgt' to enable VFIO/mdev interface.
> 
> v4:
> - fix checkpatch warning
> 
> v3:
> - Fix module reference handling for device open and release. Unused
>   mdev devices would be cleaned up in device unregister when module
> unload.
> 
> v2:
> - Fix kvmgt order after i915 for built-in case
> 
> Cc: Alex Williamson <alex.williamson@redhat.com>
> Signed-off-by: Zhenyu Wang <zhenyuw@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/Makefile        |   1 +
>  drivers/gpu/drm/i915/gvt/Makefile    |   1 -
>  drivers/gpu/drm/i915/gvt/gvt.c       | 107 +++++++++++----------------
>  drivers/gpu/drm/i915/gvt/gvt.h       |   6 +-
>  drivers/gpu/drm/i915/gvt/hypercall.h |   7 +-
>  drivers/gpu/drm/i915/gvt/kvmgt.c     |  21 +++++-
>  drivers/gpu/drm/i915/gvt/mpt.h       |   3 +
>  drivers/gpu/drm/i915/intel_gvt.c     |   9 ---
>  8 files changed, 72 insertions(+), 83 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
> index 0ff878c994e2..ae0d975a6f34 100644
> --- a/drivers/gpu/drm/i915/Makefile
> +++ b/drivers/gpu/drm/i915/Makefile
> @@ -195,3 +195,4 @@ endif
>  i915-y += intel_lpe_audio.o
> 
>  obj-$(CONFIG_DRM_I915) += i915.o
> +obj-$(CONFIG_DRM_I915_GVT_KVMGT) += gvt/kvmgt.o
> diff --git a/drivers/gpu/drm/i915/gvt/Makefile
> b/drivers/gpu/drm/i915/gvt/Makefile
> index b016dc753db9..271fb46d4dd0 100644
> --- a/drivers/gpu/drm/i915/gvt/Makefile
> +++ b/drivers/gpu/drm/i915/gvt/Makefile
> @@ -7,4 +7,3 @@ GVT_SOURCE := gvt.o aperture_gm.o handlers.o vgpu.o
> trace_points.o firmware.o \
> 
>  ccflags-y				+= -I$(src) -I$(src)/$(GVT_DIR)
>  i915-y					+= $(addprefix $(GVT_DIR)/,
> $(GVT_SOURCE))
> -obj-$(CONFIG_DRM_I915_GVT_KVMGT)	+= $(GVT_DIR)/kvmgt.o
> diff --git a/drivers/gpu/drm/i915/gvt/gvt.c b/drivers/gpu/drm/i915/gvt/gvt.c
> index a5b760b7bc10..e1c9c20918af 100644
> --- a/drivers/gpu/drm/i915/gvt/gvt.c
> +++ b/drivers/gpu/drm/i915/gvt/gvt.c
> @@ -187,52 +187,6 @@ static const struct intel_gvt_ops intel_gvt_ops = {
>  	.write_protect_handler = intel_vgpu_page_track_handler,  };
> 
> -/**
> - * intel_gvt_init_host - Load MPT modules and detect if we're running in
> host
> - *
> - * This function is called at the driver loading stage. If failed to find a
> - * loadable MPT module or detect currently we're running in a VM, then
> GVT-g
> - * will be disabled
> - *
> - * Returns:
> - * Zero on success, negative error code if failed.
> - *
> - */
> -int intel_gvt_init_host(void)
> -{
> -	if (intel_gvt_host.initialized)
> -		return 0;
> -
> -	/* Xen DOM U */
> -	if (xen_domain() && !xen_initial_domain())
> -		return -ENODEV;
> -
> -	/* Try to load MPT modules for hypervisors */
> -	if (xen_initial_domain()) {
> -		/* In Xen dom0 */
> -		intel_gvt_host.mpt = try_then_request_module(
> -				symbol_get(xengt_mpt), "xengt");
> -		intel_gvt_host.hypervisor_type =
> INTEL_GVT_HYPERVISOR_XEN;
> -	} else {
> -#if IS_ENABLED(CONFIG_DRM_I915_GVT_KVMGT)
> -		/* not in Xen. Try KVMGT */
> -		intel_gvt_host.mpt = try_then_request_module(
> -				symbol_get(kvmgt_mpt), "kvmgt");
> -		intel_gvt_host.hypervisor_type =
> INTEL_GVT_HYPERVISOR_KVM;
> -#endif
> -	}
> -
> -	/* Fail to load MPT modules - bail out */
> -	if (!intel_gvt_host.mpt)
> -		return -EINVAL;
> -
> -	gvt_dbg_core("Running with hypervisor %s in host mode\n",
> -
> 	supported_hypervisors[intel_gvt_host.hypervisor_type]);
> -
> -	intel_gvt_host.initialized = true;
> -	return 0;
> -}
> -
>  static void init_device_info(struct intel_gvt *gvt)  {
>  	struct intel_gvt_device_info *info = &gvt->device_info; @@ -316,7
> +270,6 @@ void intel_gvt_clean_device(struct drm_i915_private *dev_priv)
>  		return;
> 
>  	intel_gvt_destroy_idle_vgpu(gvt->idle_vgpu);
> -	intel_gvt_hypervisor_host_exit(&dev_priv->drm.pdev->dev);
>  	intel_gvt_cleanup_vgpu_type_groups(gvt);
>  	intel_gvt_clean_vgpu_types(gvt);
> 
> @@ -352,13 +305,6 @@ int intel_gvt_init_device(struct drm_i915_private
> *dev_priv)
>  	struct intel_vgpu *vgpu;
>  	int ret;
> 
> -	/*
> -	 * Cannot initialize GVT device without intel_gvt_host gets
> -	 * initialized first.
> -	 */
> -	if (WARN_ON(!intel_gvt_host.initialized))
> -		return -EINVAL;
> -
>  	if (WARN_ON(dev_priv->gvt))
>  		return -EEXIST;
> 
> @@ -420,13 +366,6 @@ int intel_gvt_init_device(struct drm_i915_private
> *dev_priv)
>  		goto out_clean_types;
>  	}
> 
> -	ret = intel_gvt_hypervisor_host_init(&dev_priv->drm.pdev->dev, gvt,
> -				&intel_gvt_ops);
> -	if (ret) {
> -		gvt_err("failed to register gvt-g host device: %d\n", ret);
> -		goto out_clean_types;
> -	}
> -
>  	vgpu = intel_gvt_create_idle_vgpu(gvt);
>  	if (IS_ERR(vgpu)) {
>  		ret = PTR_ERR(vgpu);
> @@ -441,6 +380,8 @@ int intel_gvt_init_device(struct drm_i915_private
> *dev_priv)
> 
>  	gvt_dbg_core("gvt device initialization is done\n");
>  	dev_priv->gvt = gvt;
> +	intel_gvt_host.dev = &dev_priv->drm.pdev->dev;
> +	intel_gvt_host.initialized = true;
>  	return 0;
> 
>  out_clean_types:
> @@ -467,6 +408,44 @@ int intel_gvt_init_device(struct drm_i915_private
> *dev_priv)
>  	return ret;
>  }
> 
> -#if IS_ENABLED(CONFIG_DRM_I915_GVT_KVMGT)
> -MODULE_SOFTDEP("pre: kvmgt");
> -#endif
> +int
> +intel_gvt_register_hypervisor(struct intel_gvt_mpt *m) {
> +	int ret;
> +	void *gvt;
> +
> +	if (!intel_gvt_host.initialized)
> +		return -ENODEV;
> +
> +	if (m->type != INTEL_GVT_HYPERVISOR_KVM &&
> +	    m->type != INTEL_GVT_HYPERVISOR_XEN)
> +		return -EINVAL;
> +
> +	/* Get a reference for device model module */
> +	if (!try_module_get(THIS_MODULE))
> +		return -ENODEV;
> +
> +	intel_gvt_host.mpt = m;
> +	intel_gvt_host.hypervisor_type = m->type;
> +	gvt = (void *)kdev_to_i915(intel_gvt_host.dev)->gvt;
> +
> +	ret = intel_gvt_hypervisor_host_init(intel_gvt_host.dev, gvt,
> +					     &intel_gvt_ops);
> +	if (ret < 0) {
> +		gvt_err("Failed to init %s hypervisor module\n",
> +
> 	supported_hypervisors[intel_gvt_host.hypervisor_type]);
> +		return -ENODEV;
[Yuan, Hang] put_module in failing case?

> +	}
> +	gvt_dbg_core("Running with hypervisor %s in host mode\n",
> +		     supported_hypervisors[intel_gvt_host.hypervisor_type]);
> +	return 0;
> +}
> +EXPORT_SYMBOL_GPL(intel_gvt_register_hypervisor);
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx