Re: [PATCH 1/5] drm/i915: make intel_gvt_init() later instead of too early

[Jike Song <jike.song@intel.com>]

On 01/10/2017 02:52 PM, Zhenyu Wang wrote:
> Previously intel_gvt_init() was called very early even before
> MMIO initialization which had several drawbacks:
> - Have to handle MMIO access for initial MMIO state dump if golden
>   state firmware is not available
> - Hypervisor detection should depend on pvinfo only instead of detecting
>   hypervisor status.
> - Don't know hw resource size e.g aperture, ggtt size to determine
>   for vGPU type, etc.
> 
> This trys to move intel_gvt_init() call late after required info
> has already been initialized for GVT host.
> 
> Signed-off-by: Zhenyu Wang <zhenyuw@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/gvt/gvt.c   | 20 ++++++++------------
>  drivers/gpu/drm/i915/i915_drv.c  | 14 +++++++-------
>  drivers/gpu/drm/i915/intel_gvt.c |  5 +++++
>  3 files changed, 20 insertions(+), 19 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/gvt/gvt.c b/drivers/gpu/drm/i915/gvt/gvt.c
> index e6bf5c533fbe..35264a991776 100644
> --- a/drivers/gpu/drm/i915/gvt/gvt.c
> +++ b/drivers/gpu/drm/i915/gvt/gvt.c
> @@ -73,23 +73,19 @@ 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()) {
> +#if IS_ENABLED(CONFIG_DRM_I915_GVT_KVMGT)
> +	/* 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
> +
> +	if (!intel_gvt_host.mpt && 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
>  	}
>  

Hi Zhenyu,

It is always easy for xengt to detect dom0 case, but difficult for kvmgt
to detect host, so I guess xen_initial_domain() should be removed from here,
and placed in xengt.ko.

[xengt.ko]

	static int __init xengt_init(void)
	{
	       if (!xen_initial_domain())
		       return -EINVAL;
		...

[gvt]
	request xengt_mpt
	if (failed)
		request kvmgt_mpt

With logic above, we can avoid calling xen_initial_domain(), and remove
"#include <xen/xen.h>" from gvt.c.

What'd you say? :)

--
Thanks,
Jike

>  	/* Fail to load MPT modules - bail out */
> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> index 4d22b4b479b8..9270dda354db 100644
> --- a/drivers/gpu/drm/i915/i915_drv.c
> +++ b/drivers/gpu/drm/i915/i915_drv.c
> @@ -815,10 +815,6 @@ static int i915_driver_init_early(struct drm_i915_private *dev_priv,
>  	if (ret < 0)
>  		return ret;
>  
> -	ret = intel_gvt_init(dev_priv);
> -	if (ret < 0)
> -		goto err_workqueues;
> -
>  	/* This must be called before any calls to HAS_PCH_* */
>  	intel_detect_pch(dev_priv);
>  
> @@ -832,7 +828,7 @@ static int i915_driver_init_early(struct drm_i915_private *dev_priv,
>  	intel_init_audio_hooks(dev_priv);
>  	ret = i915_gem_load_init(dev_priv);
>  	if (ret < 0)
> -		goto err_gvt;
> +		goto err_workqueues;
>  
>  	intel_display_crc_init(dev_priv);
>  
> @@ -844,8 +840,6 @@ static int i915_driver_init_early(struct drm_i915_private *dev_priv,
>  
>  	return 0;
>  
> -err_gvt:
> -	intel_gvt_cleanup(dev_priv);
>  err_workqueues:
>  	i915_workqueues_cleanup(dev_priv);
>  	return ret;
> @@ -1068,6 +1062,10 @@ static int i915_driver_init_hw(struct drm_i915_private *dev_priv)
>  			DRM_DEBUG_DRIVER("can't enable MSI");
>  	}
>  
> +	ret = intel_gvt_init(dev_priv);
> +	if (ret)
> +		goto out_ggtt;
> +
>  	return 0;
>  
>  out_ggtt:
> @@ -1281,6 +1279,8 @@ void i915_driver_unload(struct drm_device *dev)
>  
>  	intel_display_power_get(dev_priv, POWER_DOMAIN_INIT);
>  
> +	intel_gvt_cleanup(dev_priv);
> +
>  	i915_driver_unregister(dev_priv);
>  
>  	drm_vblank_cleanup(dev);
> diff --git a/drivers/gpu/drm/i915/intel_gvt.c b/drivers/gpu/drm/i915/intel_gvt.c
> index 290384e86c63..d23c0fcff751 100644
> --- a/drivers/gpu/drm/i915/intel_gvt.c
> +++ b/drivers/gpu/drm/i915/intel_gvt.c
> @@ -67,6 +67,11 @@ int intel_gvt_init(struct drm_i915_private *dev_priv)
>  		return 0;
>  	}
>  
> +	if (intel_vgpu_active(dev_priv)) {
> +		DRM_DEBUG_DRIVER("GVT-g is disabled for guest\n");
> +		goto bail;
> +	}
> +
>  	if (!is_supported_device(dev_priv)) {
>  		DRM_DEBUG_DRIVER("Unsupported device. GVT-g is disabled\n");
>  		goto bail;
> 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx