Fix issues caused by gvt init timing

PATCH In review Zhenyu Wang 4 participants 13 messages Jan 10, 2017 – Jan 11, 2017

Subsystems (3)

DRM drivers Intel i915 Intel GVT-g drivers (Intel GPU Virtualization)

lore

Share & tools

lore

Zhenyu Wang

author

2017-01-10 06:52 UTC #0 lore raw

cover Fix issues caused by gvt init timing

This trys to change origin gvt init time which was too early that caused several workarounds be applied in GVT-g device model driver. To move gvt init a bit later, we can fix up those without workarounds.

First one in series touches i915 driver init path that need to be acked.

thanks

Zhenyu Wang (5):
drm/i915: make intel_gvt_init() later instead of too early drm/i915/gvt: move intel iommu detection to intel_gvt_init() drm/i915/gvt: remove detect_host() MPT hook drm/i915/gvt: use normal mmio read function for firmware exposure drm/i915/gvt: fix vgpu type size init

 drivers/gpu/drm/i915/gvt/firmware.c  | 47 +++---------------------------------
 drivers/gpu/drm/i915/gvt/gvt.c       | 30 ++++++++++-------------
 drivers/gpu/drm/i915/gvt/hypercall.h |  1 -
 drivers/gpu/drm/i915/gvt/kvmgt.c     | 38 -----------------------------
 drivers/gpu/drm/i915/gvt/mpt.h       | 12 ---------
 drivers/gpu/drm/i915/gvt/vgpu.c      | 13 +++++-----
 drivers/gpu/drm/i915/i915_drv.c      | 14 +++++------
 drivers/gpu/drm/i915/intel_gvt.c     |  5 ++++
 8 files changed, 36 insertions(+), 124 deletions(-)

Signature · 6 lines

2.11.0

_______________________________________________
Intel-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Zhenyu Wang

author

2017-01-10 06:53 UTC #1 lore raw

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

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 <[email protected]>

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

drivers/gpu/drm/i915/gvt/gvt.c +8 ~~-12~~

index e6bf5c533fbe..35264a991776 100644
@@ -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
 	}
 
 	/* Fail to load MPT modules - bail out */

drivers/gpu/drm/i915/i915_drv.c +7 -7

index 4d22b4b479b8..9270dda354db 100644
@@ -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);

drivers/gpu/drm/i915/intel_gvt.c +5 -0

index 290384e86c63..d23c0fcff751 100644
@@ -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;

Signature · 6 lines

2.11.0

_______________________________________________
Intel-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Jike Song

2017-01-11 03:14 UTC #2 lore raw

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

Quoted 44 lines · Zhenyu Wang

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 <[email protected]>
---
 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(-)
drivers/gpu/drm/i915/gvt/gvt.c +8 -5
index e6bf5c533fbe..35264a991776 100644
@@ -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");

drivers/gpu/drm/i915/gvt/gvt.c int intel_gvt_init_host(void) · line 90

Quoted 10 lines

drivers/gpu/drm/i915/gvt/gvt.c +0 -7

 		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

Quoted 61 lines

drivers/gpu/drm/i915/gvt/gvt.c +0 -0

 	/* Fail to load MPT modules - bail out */

drivers/gpu/drm/i915/i915_drv.c +7 -7

index 4d22b4b479b8..9270dda354db 100644
@@ -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);

drivers/gpu/drm/i915/intel_gvt.c +0 -0

index 290384e86c63..d23c0fcff751 100644
@@ -67,6 +67,11 @@ int intel_gvt_init(struct drm_i915_private *dev_priv)
 		return 0;

drivers/gpu/drm/i915/intel_gvt.c int intel_gvt_init(struct drm_i915_private *dev_priv) · line 77

Quoted 10 lines

drivers/gpu/drm/i915/intel_gvt.c +5 -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
[email protected]
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Zhenyu Wang

author

2017-01-11 03:34 UTC #3 lore raw

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

Quoted 75 lines · Jike Song

Quoted 44 lines · Zhenyu Wang
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 <[email protected]>
---
 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(-)
drivers/gpu/drm/i915/gvt/gvt.c +8 -5
index e6bf5c533fbe..35264a991776 100644
@@ -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");
drivers/gpu/drm/i915/gvt/gvt.c int intel_gvt_init_host(void) · line 90
Quoted 10 lines
drivers/gpu/drm/i915/gvt/gvt.c +0 -7
 		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

Quoted 5 lines

With logic above, we can avoid calling xen_initial_domain(), and remove "#include <xen/xen.h>" from gvt.c.
What'd you say? :)

yep, sounds good to me.

Signature · 3 lines

Open Source Technology Center, Intel ltd.

$gpg --keyserver wwwkeys.pgp.net --recv-keys 4D781827

_______________________________________________
Intel-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Zhenyu Wang

author

2017-01-10 06:53 UTC #4 lore raw

PATCH 2/5 drm/i915/gvt: move intel iommu detection to intel_gvt_init()

In reply to Zhenyu Wang

Prepare to remove detect_host() hook. Move intel iommu detection early in intel_gvt_init().

Signed-off-by Zhenyu Wang <[email protected]>

---
 drivers/gpu/drm/i915/gvt/gvt.c   | 7 +++++++
 drivers/gpu/drm/i915/gvt/kvmgt.c | 6 ------
 2 files changed, 7 insertions(+), 6 deletions(-)

drivers/gpu/drm/i915/gvt/gvt.c +7 -0

index 35264a991776..7a7886644acf 100644
@@ -73,6 +73,13 @@ int intel_gvt_init_host(void)
 	if (intel_gvt_host.initialized)
 		return 0;
 
+#ifdef CONFIG_INTEL_IOMMU
+	if (intel_iommu_gfx_mapped) {
+		gvt_err("Hardware IOMMU compatibility not yet supported, try to boot with intel_iommu=igfx_off\n");
+		return -ENODEV;
+	}
+#endif
+
 	/* Try to load MPT modules for hypervisors */
 #if IS_ENABLED(CONFIG_DRM_I915_GVT_KVMGT)
 	/* Try KVMGT */

drivers/gpu/drm/i915/gvt/kvmgt.c +0 -6

index 0c9234a87a20..f29d2a27ccb1 100644
@@ -1276,12 +1276,6 @@ static bool kvmgt_check_guest(void)
  */
 static int kvmgt_detect_host(void)
 {
-#ifdef CONFIG_INTEL_IOMMU
-	if (intel_iommu_gfx_mapped) {
-		gvt_err("Hardware IOMMU compatibility not yet supported, try to boot with intel_iommu=igfx_off\n");
-		return -ENODEV;
-	}
-#endif
 	return kvmgt_check_guest() ? -ENODEV : 0;
 }

Signature · 6 lines

2.11.0

_______________________________________________
Intel-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Jike Song

2017-01-11 02:20 UTC #5 lore raw

Re: [PATCH 2/5] drm/i915/gvt: move intel iommu detection to intel_gvt_init()

Quoted 14 lines · Zhenyu Wang

Prepare to remove detect_host() hook. Move intel iommu detection early in intel_gvt_init().
Signed-off-by Zhenyu Wang <[email protected]>
---
 drivers/gpu/drm/i915/gvt/gvt.c   | 7 +++++++
 drivers/gpu/drm/i915/gvt/kvmgt.c | 6 ------
 2 files changed, 7 insertions(+), 6 deletions(-)
drivers/gpu/drm/i915/gvt/gvt.c +0 -0
index 35264a991776..7a7886644acf 100644

drivers/gpu/drm/i915/gvt/gvt.c int intel_gvt_init_host(void) · line 82

Quoted 10 lines

drivers/gpu/drm/i915/gvt/gvt.c +7 -0

@@ -73,6 +73,13 @@ int intel_gvt_init_host(void)
 	if (intel_gvt_host.initialized)
 		return 0;
 
+#ifdef CONFIG_INTEL_IOMMU
+	if (intel_iommu_gfx_mapped) {
+		gvt_err("Hardware IOMMU compatibility not yet supported, try to boot with intel_iommu=igfx_off\n");
+		return -ENODEV;
+	}
+#endif
+

Hi Zhenyu,

Per my understanding, the "intel_iommu=" parameter acts only on native (think about XenGT), so it's better to keep it somewhere in kvmgt.c, maybe kvmgt_init()?

--
Thanks,
Jike

Quoted 11 lines

drivers/gpu/drm/i915/gvt/gvt.c +0 -0

 	/* Try to load MPT modules for hypervisors */
 #if IS_ENABLED(CONFIG_DRM_I915_GVT_KVMGT)
 	/* Try KVMGT */

drivers/gpu/drm/i915/gvt/kvmgt.c +0 -0

index 0c9234a87a20..f29d2a27ccb1 100644
@@ -1276,12 +1276,6 @@ static bool kvmgt_check_guest(void)
  */
 static int kvmgt_detect_host(void)

drivers/gpu/drm/i915/gvt/kvmgt.c static bool kvmgt_check_guest(void) · line 1281

Quoted 10 lines

drivers/gpu/drm/i915/gvt/kvmgt.c +0 -6

 {
-#ifdef CONFIG_INTEL_IOMMU
-	if (intel_iommu_gfx_mapped) {
-		gvt_err("Hardware IOMMU compatibility not yet supported, try to boot with intel_iommu=igfx_off\n");
-		return -ENODEV;
-	}
-#endif
 	return kvmgt_check_guest() ? -ENODEV : 0;
 }

_______________________________________________
Intel-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Zhenyu Wang

author

2017-01-11 02:44 UTC #6 lore raw

Re: [PATCH 2/5] drm/i915/gvt: move intel iommu detection to intel_gvt_init()

Quoted 28 lines · Jike Song

Quoted 14 lines · Zhenyu Wang
Prepare to remove detect_host() hook. Move intel iommu detection early in intel_gvt_init().
Signed-off-by Zhenyu Wang <[email protected]>
---
 drivers/gpu/drm/i915/gvt/gvt.c   | 7 +++++++
 drivers/gpu/drm/i915/gvt/kvmgt.c | 6 ------
 2 files changed, 7 insertions(+), 6 deletions(-)
drivers/gpu/drm/i915/gvt/gvt.c +0 -0
index 35264a991776..7a7886644acf 100644
drivers/gpu/drm/i915/gvt/gvt.c int intel_gvt_init_host(void) · line 82
Quoted 10 lines
drivers/gpu/drm/i915/gvt/gvt.c +7 -0
@@ -73,6 +73,13 @@ int intel_gvt_init_host(void)
 	if (intel_gvt_host.initialized)
 		return 0;
 
+#ifdef CONFIG_INTEL_IOMMU
+	if (intel_iommu_gfx_mapped) {
+		gvt_err("Hardware IOMMU compatibility not yet supported, try to boot with intel_iommu=igfx_off\n");
+		return -ENODEV;
+	}
+#endif
+
Hi Zhenyu,

Quoted 3 lines

Per my understanding, the "intel_iommu=" parameter acts only on native (think about XenGT), so it's better to keep it somewhere in kvmgt.c, maybe kvmgt_init()?

hmm, I think it's just a limit for current gvt device model but not related to hypervisor, and it would bail out to disable gvt only. Anyway we'll fix it soon so not worry much for that.

Signature · 3 lines

Open Source Technology Center, Intel ltd.

$gpg --keyserver wwwkeys.pgp.net --recv-keys 4D781827

_______________________________________________
Intel-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Jike Song

2017-01-11 03:03 UTC #7 lore raw

Re: [PATCH 2/5] drm/i915/gvt: move intel iommu detection to intel_gvt_init()

Quoted 29 lines · Zhenyu Wang

Quoted 28 lines · Jike Song
Quoted 14 lines · Zhenyu Wang
Prepare to remove detect_host() hook. Move intel iommu detection early in intel_gvt_init().
Signed-off-by Zhenyu Wang <[email protected]>
---
 drivers/gpu/drm/i915/gvt/gvt.c   | 7 +++++++
 drivers/gpu/drm/i915/gvt/kvmgt.c | 6 ------
 2 files changed, 7 insertions(+), 6 deletions(-)
drivers/gpu/drm/i915/gvt/gvt.c +0 -0
index 35264a991776..7a7886644acf 100644
drivers/gpu/drm/i915/gvt/gvt.c int intel_gvt_init_host(void) · line 82
Quoted 10 lines
drivers/gpu/drm/i915/gvt/gvt.c +7 -0
@@ -73,6 +73,13 @@ int intel_gvt_init_host(void)
 	if (intel_gvt_host.initialized)
 		return 0;
 
+#ifdef CONFIG_INTEL_IOMMU
+	if (intel_iommu_gfx_mapped) {
+		gvt_err("Hardware IOMMU compatibility not yet supported, try to boot with intel_iommu=igfx_off\n");
+		return -ENODEV;
+	}
+#endif
+
Hi Zhenyu,

Quoted 7 lines

Quoted 3 lines
Per my understanding, the "intel_iommu=" parameter acts only on native (think about XenGT), so it's better to keep it somewhere in kvmgt.c, maybe kvmgt_init()?
hmm, I think it's just a limit for current gvt device model but not related to hypervisor, and it would bail out to disable gvt only. Anyway we'll fix it soon so not worry much for that.

I agree that it's only a temporary workaround, only want to point out that hypervisor does
matter: if we take XenGT considered, it has IOMMU hardware managed by Hypervisor, so its the host (dom0) kernel doesn't need any special configuration for iommu :)

--
Thanks,
Jike
_______________________________________________
Intel-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Zhenyu Wang

author

2017-01-10 06:53 UTC #8 lore raw

PATCH 3/5 drm/i915/gvt: remove detect_host() MPT hook

In reply to Zhenyu Wang

We only depend on pvinfo register for GVT-g state detection, not require hypervisor host detect any more.

Signed-off-by Zhenyu Wang <[email protected]>

---
 drivers/gpu/drm/i915/gvt/gvt.c       |  7 -------
 drivers/gpu/drm/i915/gvt/hypercall.h |  1 -
 drivers/gpu/drm/i915/gvt/kvmgt.c     | 32 --------------------------------
 drivers/gpu/drm/i915/gvt/mpt.h       | 12 ------------
 4 files changed, 52 deletions(-)

drivers/gpu/drm/i915/gvt/gvt.c +0 -7

index 7a7886644acf..c8002bf200ad 100644
@@ -68,8 +68,6 @@ static const struct intel_gvt_ops intel_gvt_ops = {
  */
 int intel_gvt_init_host(void)
 {
-	int ret;
-
 	if (intel_gvt_host.initialized)
 		return 0;

@@ -99,11 +97,6 @@ int intel_gvt_init_host(void)
 	if (!intel_gvt_host.mpt)
 		return -EINVAL;
 
-	/* Try to detect if we're running in host instead of VM. */
-	ret = intel_gvt_hypervisor_detect_host();
-	if (ret)
-		return -ENODEV;
-
 	gvt_dbg_core("Running with hypervisor %s in host mode\n",
 			supported_hypervisors[intel_gvt_host.hypervisor_type]);

drivers/gpu/drm/i915/gvt/hypercall.h +0 -1

index 30e543f5a703..df7f33abd393 100644
@@ -38,7 +38,6 @@
  * both Xen and KVM by providing dedicated hypervisor-related MPT modules.
  */
 struct intel_gvt_mpt {
-	int (*detect_host)(void);
 	int (*host_init)(struct device *dev, void *gvt, const void *ops);
 	void (*host_exit)(struct device *dev, void *gvt);
 	int (*attach_vgpu)(void *vgpu, unsigned long *handle);

drivers/gpu/drm/i915/gvt/kvmgt.c +0 ~~-32~~

index f29d2a27ccb1..080ca77abd22 100644
@@ -1248,37 +1248,6 @@ static void kvmgt_page_track_flush_slot(struct kvm *kvm,
 	spin_unlock(&kvm->mmu_lock);
 }
 
-static bool kvmgt_check_guest(void)
-{
-	unsigned int eax, ebx, ecx, edx;
-	char s[12];
-	unsigned int *i;
-
-	eax = KVM_CPUID_SIGNATURE;
-	ebx = ecx = edx = 0;
-
-	asm volatile ("cpuid"
-		      : "+a"(eax), "=b"(ebx), "=c"(ecx), "=d"(edx)
-		      :
-		      : "cc", "memory");
-	i = (unsigned int *)s;
-	i[0] = ebx;
-	i[1] = ecx;
-	i[2] = edx;
-
-	return !strncmp(s, "KVMKVMKVM", strlen("KVMKVMKVM"));
-}
-
-/**
- * NOTE:
- * It's actually impossible to check if we are running in KVM host,
- * since the "KVM host" is simply native. So we only dectect guest here.
- */
-static int kvmgt_detect_host(void)
-{
-	return kvmgt_check_guest() ? -ENODEV : 0;
-}
-
 static bool __kvmgt_vgpu_exist(struct intel_vgpu *vgpu, struct kvm *kvm)
 {
 	struct intel_vgpu *itr;

@@ -1453,7 +1422,6 @@ static unsigned long kvmgt_virt_to_pfn(void *addr)
 }
 
 struct intel_gvt_mpt kvmgt_mpt = {
-	.detect_host = kvmgt_detect_host,
 	.host_init = kvmgt_host_init,
 	.host_exit = kvmgt_host_exit,
 	.attach_vgpu = kvmgt_attach_vgpu,

drivers/gpu/drm/i915/gvt/mpt.h +0 ~~-12~~

index 1af5830c0a56..419353624c5a 100644
@@ -44,18 +44,6 @@
  */
 
 /**
- * intel_gvt_hypervisor_detect_host - check if GVT-g is running within
- * hypervisor host/privilged domain
- *
- * Returns:
- * Zero on success, -ENODEV if current kernel is running inside a VM
- */
-static inline int intel_gvt_hypervisor_detect_host(void)
-{
-	return intel_gvt_host.mpt->detect_host();
-}
-
-/**
  * intel_gvt_hypervisor_host_init - init GVT-g host side
  *
  * Returns:

Signature · 6 lines

2.11.0

_______________________________________________
Intel-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Zhenyu Wang

author

2017-01-10 06:53 UTC #9 lore raw

PATCH 4/5 drm/i915/gvt: use normal mmio read function for firmware exposure

In reply to Zhenyu Wang

As now gvt init is late after MMIO initialization, use normal MMIO read function for initial firmware exposure if no available firmware loaded.

Signed-off-by Zhenyu Wang <[email protected]>

---
 drivers/gpu/drm/i915/gvt/firmware.c | 47 ++++---------------------------------
 1 file changed, 4 insertions(+), 43 deletions(-)

drivers/gpu/drm/i915/gvt/firmware.c +4 ~~-43~~

index 2fae2a2ca96f..1cb29b2d7dc6 100644
@@ -48,31 +48,6 @@ struct gvt_firmware_header {
 	unsigned char data[1];
 };
 
-#define RD(offset) (readl(mmio + offset.reg))
-#define WR(v, offset) (writel(v, mmio + offset.reg))
-
-static void bdw_forcewake_get(void __iomem *mmio)
-{
-	WR(_MASKED_BIT_DISABLE(0xffff), FORCEWAKE_MT);
-
-	RD(ECOBUS);
-
-	if (wait_for((RD(FORCEWAKE_ACK_HSW) & FORCEWAKE_KERNEL) == 0, 50))
-		gvt_err("fail to wait forcewake idle\n");
-
-	WR(_MASKED_BIT_ENABLE(FORCEWAKE_KERNEL), FORCEWAKE_MT);
-
-	if (wait_for((RD(FORCEWAKE_ACK_HSW) & FORCEWAKE_KERNEL), 50))
-		gvt_err("fail to wait forcewake ack\n");
-
-	if (wait_for((RD(GEN6_GT_THREAD_STATUS_REG) &
-		      GEN6_GT_THREAD_STATUS_CORE_MASK) == 0, 50))
-		gvt_err("fail to wait c0 wake up\n");
-}
-
-#undef RD
-#undef WR
-
 #define dev_to_drm_minor(d) dev_get_drvdata((d))
 
 static ssize_t

@@ -91,9 +66,9 @@ static struct bin_attribute firmware_attr = {
 	.mmap = NULL,
 };
 
-static int expose_firmware_sysfs(struct intel_gvt *gvt,
-					void __iomem *mmio)
+static int expose_firmware_sysfs(struct intel_gvt *gvt)
 {
+	struct drm_i915_private *dev_priv = gvt->dev_priv;
 	struct intel_gvt_device_info *info = &gvt->device_info;
 	struct pci_dev *pdev = gvt->dev_priv->drm.pdev;
 	struct intel_gvt_mmio_info *e;

@@ -132,7 +107,7 @@ static int expose_firmware_sysfs(struct intel_gvt *gvt,
 
 		for (j = 0; j < e->length; j += 4)
 			*(u32 *)(p + e->offset + j) =
-				readl(mmio + e->offset + j);
+				I915_READ_NOTRACE(_MMIO(e->offset + j));
 	}
 
 	memcpy(gvt->firmware.mmio, p, info->mmio_size);

@@ -235,7 +210,6 @@ int intel_gvt_load_firmware(struct intel_gvt *gvt)
 	struct gvt_firmware_header *h;
 	const struct firmware *fw;
 	char *path;
-	void __iomem *mmio;
 	void *mem;
 	int ret;

@@ -260,17 +234,6 @@ int intel_gvt_load_firmware(struct intel_gvt *gvt)
 
 	firmware->mmio = mem;
 
-	mmio = pci_iomap(pdev, info->mmio_bar, info->mmio_size);
-	if (!mmio) {
-		kfree(path);
-		kfree(firmware->cfg_space);
-		kfree(firmware->mmio);
-		return -EINVAL;
-	}
-
-	if (IS_BROADWELL(gvt->dev_priv) || IS_SKYLAKE(gvt->dev_priv))
-		bdw_forcewake_get(mmio);
-
 	sprintf(path, "%s/vid_0x%04x_did_0x%04x_rid_0x%04x.golden_hw_state",
 		 GVT_FIRMWARE_PATH, pdev->vendor, pdev->device,
 		 pdev->revision);

@@ -300,13 +263,11 @@ int intel_gvt_load_firmware(struct intel_gvt *gvt)
 
 	release_firmware(fw);
 	firmware->firmware_loaded = true;
-	pci_iounmap(pdev, mmio);
 	return 0;
 
 out_free_fw:
 	release_firmware(fw);
 expose_firmware:
-	expose_firmware_sysfs(gvt, mmio);
-	pci_iounmap(pdev, mmio);
+	expose_firmware_sysfs(gvt);
 	return 0;
 }

Signature · 6 lines

2.11.0

_______________________________________________
Intel-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Zhenyu Wang

author

2017-01-10 06:53 UTC #10 lore raw

PATCH 5/5 drm/i915/gvt: fix vgpu type size init

In reply to Zhenyu Wang

As now gvt init after knowing hw resource info, we can determine vGPU type from machine size instead of pre-defined value.

Signed-off-by Zhenyu Wang <[email protected]>

---
 drivers/gpu/drm/i915/gvt/vgpu.c | 13 +++++++------
 1 file changed, 7 insertions(+), 6 deletions(-)

drivers/gpu/drm/i915/gvt/vgpu.c +7 -6

index f0e86123e45b..cfeba443f7e2 100644
@@ -147,7 +147,7 @@ void populate_pvinfo_page(struct intel_vgpu *vgpu)
 int intel_gvt_init_vgpu_types(struct intel_gvt *gvt)
 {
 	unsigned int num_types;
-	unsigned int i, low_avail;
+	unsigned int i, low_avail, high_avail;
 	unsigned int min_low;
 
 	/* vGPU type name is defined as GVTg_Vx_y which contains

@@ -162,9 +162,9 @@ int intel_gvt_init_vgpu_types(struct intel_gvt *gvt)
 	 * to indicate how many vGPU instance can be created for this
 	 * type.
 	 *
-	 * Currently use static size here as we init type earlier..
 	 */
-	low_avail = MB_TO_BYTES(256) - HOST_LOW_GM_SIZE;
+	low_avail = gvt_aperture_sz(gvt) - HOST_LOW_GM_SIZE;
+	high_avail = gvt_hidden_sz(gvt) - HOST_HIGH_GM_SIZE;
 	num_types = 4;
 
 	gvt->types = kzalloc(num_types * sizeof(struct intel_vgpu_type),

@@ -179,7 +179,8 @@ int intel_gvt_init_vgpu_types(struct intel_gvt *gvt)
 		gvt->types[i].low_gm_size = min_low;
 		gvt->types[i].high_gm_size = max((min_low<<3), MB_TO_BYTES(384U));
 		gvt->types[i].fence = 4;
-		gvt->types[i].max_instance = low_avail / min_low;
+		gvt->types[i].max_instance = min(low_avail / min_low,
+						 high_avail / gvt->types[i].high_gm_size);
 		gvt->types[i].avail_instance = gvt->types[i].max_instance;
 
 		if (IS_GEN8(gvt->dev_priv))

@@ -215,9 +216,9 @@ static void intel_gvt_update_vgpu_types(struct intel_gvt *gvt)
 	/* Need to depend on maxium hw resource size but keep on
 	 * static config for now.
 	 */
-	low_gm_avail = MB_TO_BYTES(256) - HOST_LOW_GM_SIZE -
+	low_gm_avail = gvt_aperture_sz(gvt) - HOST_LOW_GM_SIZE -
 		gvt->gm.vgpu_allocated_low_gm_size;
-	high_gm_avail = MB_TO_BYTES(256) * 8UL - HOST_HIGH_GM_SIZE -
+	high_gm_avail = gvt_hidden_sz(gvt) - HOST_HIGH_GM_SIZE -
 		gvt->gm.vgpu_allocated_high_gm_size;
 	fence_avail = gvt_fence_sz(gvt) - HOST_FENCE -
 		gvt->fence.vgpu_allocated_fence_num;

Signature · 6 lines

2.11.0

_______________________________________________
Intel-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Patchwork

2017-01-10 07:23 UTC #11 lore raw

✓ Fi.CI.BAT: success for Fix issues caused by gvt init timing

In reply to Zhenyu Wang

== Series Details ==

Series: Fix issues caused by gvt init timing
URL : https://patchwork.freedesktop.org/series/17736/
State : success

== Summary ==

Series 17736v1 Fix issues caused by gvt init timing
https://patchwork.freedesktop.org/api/1.0/series/17736/revisions/1/mbox/

Test kms_force_connector_basic:

        Subgroup force-connector-state:
                dmesg-warn -> PASS       (fi-snb-2520m)
        Subgroup prune-stale-modes:
                skip       -> PASS       (fi-ivb-3520m)

fi-bdw-5557u     total:246  pass:232  dwarn:0   dfail:0   fail:0   skip:14 
fi-bsw-n3050     total:246  pass:207  dwarn:0   dfail:0   fail:0   skip:39 
fi-bxt-j4205     total:246  pass:224  dwarn:0   dfail:0   fail:0   skip:22 
fi-bxt-t5700     total:82   pass:69   dwarn:0   dfail:0   fail:0   skip:12 
fi-byt-j1900     total:246  pass:219  dwarn:0   dfail:0   fail:0   skip:27 
fi-byt-n2820     total:246  pass:215  dwarn:0   dfail:0   fail:0   skip:31 
fi-hsw-4770      total:246  pass:227  dwarn:0   dfail:0   fail:0   skip:19 
fi-hsw-4770r     total:246  pass:227  dwarn:0   dfail:0   fail:0   skip:19 
fi-ivb-3520m     total:246  pass:225  dwarn:0   dfail:0   fail:0   skip:21 
fi-ivb-3770      total:246  pass:225  dwarn:0   dfail:0   fail:0   skip:21 
fi-kbl-7500u     total:246  pass:225  dwarn:0   dfail:0   fail:0   skip:21 
fi-skl-6260u     total:246  pass:233  dwarn:0   dfail:0   fail:0   skip:13 
fi-skl-6700hq    total:246  pass:226  dwarn:0   dfail:0   fail:0   skip:20 
fi-skl-6700k     total:246  pass:222  dwarn:3   dfail:0   fail:0   skip:21 
fi-skl-6770hq    total:246  pass:233  dwarn:0   dfail:0   fail:0   skip:13 
fi-snb-2520m     total:246  pass:215  dwarn:0   dfail:0   fail:0   skip:31 
fi-snb-2600      total:246  pass:214  dwarn:0   dfail:0   fail:0   skip:32

9ea6a075c23ea914695d57944c0e74cff0c6bff4 drm-tip: 2017y-01m-09d-16h-11m-21s UTC integration manifest 6faf939 drm/i915/gvt: use normal mmio read function for firmware exposure
24d0010 drm/i915/gvt: remove detect_host() MPT hook
23811b5 drm/i915/gvt: move intel iommu detection to intel_gvt_init()
9bd7d16 drm/i915: make intel_gvt_init() later instead of too early

== Logs ==

For more details see: https://intel-gfx-ci.01.org/CI/Patchwork_3459/
_______________________________________________
Intel-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Chris Wilson

Acked-by on the v1 series

2017-01-10 15:41 UTC #12 lore raw

In reply to Zhenyu Wang

Quoted 6 lines · Zhenyu Wang

This trys to change origin gvt init time which was too early that caused several workarounds be applied in GVT-g device model driver. To move gvt init a bit later, we can fix up those without workarounds.
First one in series touches i915 driver init path that need to be acked.

Quoted 8 lines

thanks
Zhenyu Wang (5):
drm/i915: make intel_gvt_init() later instead of too early drm/i915/gvt: move intel iommu detection to intel_gvt_init() drm/i915/gvt: remove detect_host() MPT hook drm/i915/gvt: use normal mmio read function for firmware exposure drm/i915/gvt: fix vgpu type size init

Couldn't see anything horribly wrong, if it works for you, that's great. One thing to bare in mind for future (I don't think it would make any difference to this patchset) is that there is nothing wrong with being hooked into multiple init phases.

Acked-by Chris Wilson <[email protected]>

-Chris

Signature · 5 lines

Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/intel-gfx