Change KVMGT into self loadable 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 <[email protected]>

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

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

drivers/gpu/drm/i915/Makefile +1 -0

index 0ff878c994e2..ae0d975a6f34 100644
@@ -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

drivers/gpu/drm/i915/gvt/Makefile +0 -1

index b016dc753db9..271fb46d4dd0 100644
@@ -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

drivers/gpu/drm/i915/gvt/gvt.c +43 -64

index a5b760b7bc10..e1c9c20918af 100644
@@ -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;
+	}
+	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);
+
+void
+intel_gvt_unregister_hypervisor(void)
+{
+	intel_gvt_hypervisor_host_exit(intel_gvt_host.dev);
+	module_put(THIS_MODULE);
+}
+EXPORT_SYMBOL_GPL(intel_gvt_unregister_hypervisor);

drivers/gpu/drm/i915/gvt/gvt.h +1 -5

index b4ab1dad0143..8a4cf995d755 100644
@@ -52,12 +52,8 @@
 
 #define GVT_MAX_VGPU 8
 
-enum {
-	INTEL_GVT_HYPERVISOR_XEN = 0,
-	INTEL_GVT_HYPERVISOR_KVM,
-};
-
 struct intel_gvt_host {
+	struct device *dev;
 	bool initialized;
 	int hypervisor_type;
 	struct intel_gvt_mpt *mpt;

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

index e49a9247ed78..50798868ab15 100644
@@ -33,11 +33,17 @@
 #ifndef _GVT_HYPERCALL_H_
 #define _GVT_HYPERCALL_H_
 
+enum hypervisor_type {
+	INTEL_GVT_HYPERVISOR_XEN = 0,
+	INTEL_GVT_HYPERVISOR_KVM,
+};
+
 /*
  * Specific GVT-g MPT modules function collections. Currently GVT-g supports
  * both Xen and KVM by providing dedicated hypervisor-related MPT modules.
  */
 struct intel_gvt_mpt {
+	enum hypervisor_type type;
 	int (*host_init)(struct device *dev, void *gvt, const void *ops);
 	void (*host_exit)(struct device *dev);
 	int (*attach_vgpu)(void *vgpu, unsigned long *handle);

@@ -67,6 +73,5 @@ struct intel_gvt_mpt {
 };
 
 extern struct intel_gvt_mpt xengt_mpt;
-extern struct intel_gvt_mpt kvmgt_mpt;
 
 #endif /* _GVT_HYPERCALL_H_ */

drivers/gpu/drm/i915/gvt/kvmgt.c +18 -3

index 1bbd04d30c42..ef248d577e49 100644
@@ -50,6 +50,7 @@
 #include "gvt.h"
 
 static const struct intel_gvt_ops *intel_gvt_ops;
+static struct intel_gvt *intel_gvt;
 
 /* helper macros copied from vfio-pci */
 #define VFIO_PCI_OFFSET_SHIFT   40

@@ -627,6 +628,12 @@ static int intel_vgpu_open(struct mdev_device *mdev)
 		goto undo_iommu;
 	}
 
+	/* Take a module reference as mdev core doesn't take
+	 * a reference for vendor driver.
+	 */
+	if (!try_module_get(THIS_MODULE))
+		goto undo_group;
+
 	ret = kvmgt_guest_init(mdev);
 	if (ret)
 		goto undo_group;

@@ -679,6 +686,9 @@ static void __intel_vgpu_release(struct intel_vgpu *vgpu)
 					&vgpu->vdev.group_notifier);
 	WARN(ret, "vfio_unregister_notifier for group failed: %d\n", ret);
 
+	/* dereference module reference taken at open */
+	module_put(THIS_MODULE);
+
 	info = (struct kvmgt_guest_info *)vgpu->handle;
 	kvmgt_guest_exit(info);
 

@@ -1459,8 +1469,10 @@ static int kvmgt_host_init(struct device *dev, void *gvt, const void *ops)
 	struct attribute_group **kvm_vgpu_type_groups;
 
 	intel_gvt_ops = ops;
+	intel_gvt = (struct intel_gvt *)gvt;
+
 	if (!intel_gvt_ops->get_gvt_attrs(&kvm_type_attrs,
-			&kvm_vgpu_type_groups))
+					  &kvm_vgpu_type_groups))
 		return -EFAULT;
 	intel_vgpu_ops.supported_type_groups = kvm_vgpu_type_groups;
 

@@ -1849,7 +1861,8 @@ static bool kvmgt_is_valid_gfn(unsigned long handle, unsigned long gfn)
 	return ret;
 }
 
-struct intel_gvt_mpt kvmgt_mpt = {
+static struct intel_gvt_mpt kvmgt_mpt = {
+	.type = INTEL_GVT_HYPERVISOR_KVM,
 	.host_init = kvmgt_host_init,
 	.host_exit = kvmgt_host_exit,
 	.attach_vgpu = kvmgt_attach_vgpu,

@@ -1868,15 +1881,17 @@ struct intel_gvt_mpt kvmgt_mpt = {
 	.put_vfio_device = kvmgt_put_vfio_device,
 	.is_valid_gfn = kvmgt_is_valid_gfn,
 };
-EXPORT_SYMBOL_GPL(kvmgt_mpt);
 
 static int __init kvmgt_init(void)
 {
+	if (intel_gvt_register_hypervisor(&kvmgt_mpt) < 0)
+		return -ENODEV;
 	return 0;
 }
 
 static void __exit kvmgt_exit(void)
 {
+	intel_gvt_unregister_hypervisor();
 }
 
 module_init(kvmgt_init);

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

index c95ef77da62c..9b4225d44243 100644
@@ -360,4 +360,7 @@ static inline bool intel_gvt_hypervisor_is_valid_gfn(
 	return intel_gvt_host.mpt->is_valid_gfn(vgpu->handle, gfn);
 }
 
+int intel_gvt_register_hypervisor(struct intel_gvt_mpt *);
+void intel_gvt_unregister_hypervisor(void);
+
 #endif /* _GVT_MPT_H_ */

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

index c22b3e18a0f5..d74e59e22c9d 100644
@@ -105,15 +105,6 @@ int intel_gvt_init(struct drm_i915_private *dev_priv)
 		return -EIO;
 	}
 
-	/*
-	 * We're not in host or fail to find a MPT module, disable GVT-g
-	 */
-	ret = intel_gvt_init_host();
-	if (ret) {
-		DRM_DEBUG_DRIVER("Not in host or MPT modules not found\n");
-		goto bail;
-	}
-
 	ret = intel_gvt_init_device(dev_priv);
 	if (ret) {
 		DRM_DEBUG_DRIVER("Fail to init GVT device\n");

Signature · 6 lines

2.19.1

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

-----Original Message-----
From: Zhenyu Wang [mailto:[email protected]]
Sent: Thursday, December 6, 2018 12:28 PM
To: Yuan, Hang <[email protected]>; Wang, Zhi A <[email protected]>; Zhang, Xiong Y <[email protected]>; He, Min <[email protected]>
Cc: [email protected]; Alex Williamson <[email protected]>; [email protected]
Subject: Re: [PATCH v4] drm/i915/gvt: Change KVMGT as self load module

Ping for any more comments? Or ack?

Adding Min, for head-up notify, I've moved 'kvmgt' into self load module instead of loaded by i915/gvt, in order to clean up the dependence, so need to load 'kvmgt.ko' to enable GVT through VFIO/mdev. Hypervisor module needs to call new register/unregister function to initialize hypervisor specific interface for GVT. If AcrnGT rebase on this, it may need to change initial setup too, so could you double check?

thanks

Quoted 13 lines · Zhenyu Wang

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.

Quoted 6 lines

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.

Quoted 17 lines

v2:
- Fix kvmgt order after i915 for built-in case

Cc Alex Williamson <[email protected]>

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

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

Review

Quoted 1 line

(quoted diff) +0 -0

b/drivers/gpu/drm/i915/Makefile

Review

Quoted 9 lines

index 0ff878c994e2..ae0d975a6f34 100644

drivers/gpu/drm/i915/Makefile +1 -0

@@ -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

(quoted diff) +0 -0

b/drivers/gpu/drm/i915/gvt/Makefile

drivers/gpu/drm/i915/gvt/Makefile GVT_SOURCE := gvt.o aperture_gm.o handlers.o vgpu.o

Quoted 4 lines

index b016dc753db9..271fb46d4dd0 100644

drivers/gpu/drm/i915/gvt/Makefile +0 -0

@@ -7,4 +7,3 @@ GVT_SOURCE := gvt.o aperture_gm.o handlers.o vgpu.o

trace_points.o firmware.o \

drivers/gpu/drm/i915/gvt/Makefile GVT_SOURCE := gvt.o aperture_gm.o handlers.o vgpu.o · line 8

Quoted 3 lines

drivers/gpu/drm/i915/gvt/Makefile +0 -0

 ccflags-y				+= -I$(src) -I$(src)/$(GVT_DIR)
 i915-y					+= $(addprefix $(GVT_DIR)/,

$(GVT_SOURCE))

Review

Quoted 2 lines

drivers/gpu/drm/i915/gvt/Makefile +0 -1

-obj-$(CONFIG_DRM_I915_GVT_KVMGT)	+= $(GVT_DIR)/kvmgt.o

(quoted diff) +0 -0

b/drivers/gpu/drm/i915/gvt/gvt.c

drivers/gpu/drm/i915/gvt/gvt.c static const struct intel_gvt_ops intel_gvt_ops = { · line 191

Quoted 9 lines

index a5b760b7bc10..e1c9c20918af 100644

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

@@ -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

drivers/gpu/drm/i915/gvt/gvt.c static const struct intel_gvt_ops intel_gvt_ops = {

Quoted 3 lines

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

- *
- * 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

Quoted 11 lines

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

- * 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;
-

drivers/gpu/drm/i915/gvt/gvt.c static const struct intel_gvt_ops intel_gvt_ops = {

Quoted 10 lines

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

-	/* 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;

drivers/gpu/drm/i915/gvt/gvt.c static const struct intel_gvt_ops intel_gvt_ops = {

Quoted 6 lines

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

-	} 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;

drivers/gpu/drm/i915/gvt/gvt.c static const struct intel_gvt_ops intel_gvt_ops = { · line 231

Quoted 9 lines

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

-#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]);

drivers/gpu/drm/i915/gvt/gvt.c void intel_gvt_clean_device(struct

Quoted 9 lines

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

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

drivers/gpu/drm/i915/gvt/gvt.c int intel_gvt_init_device(struct drm_i915_private

Quoted 8 lines

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

 		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)

drivers/gpu/drm/i915/gvt/gvt.c int intel_gvt_init_device(struct drm_i915_private

Quoted 14 lines

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

 	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)

drivers/gpu/drm/i915/gvt/gvt.c int intel_gvt_init_device(struct drm_i915_private

Quoted 14 lines

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

 		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)

drivers/gpu/drm/i915/gvt/gvt.c int intel_gvt_init_device(struct drm_i915_private

Quoted 9 lines

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

 	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)

Quoted 21 lines

drivers/gpu/drm/i915/gvt/gvt.c +15 -3

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

drivers/gpu/drm/i915/gvt/gvt.c int intel_gvt_init_device(struct drm_i915_private *dev_priv) · line 433

Quoted 8 lines

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

+		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);

drivers/gpu/drm/i915/gvt/gvt.c int intel_gvt_init_device(struct drm_i915_private *dev_priv) · line 444

Quoted 3 lines

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

+	if (ret < 0) {
+		gvt_err("Failed to init %s hypervisor module\n",
+

	supported_hypervisors[intel_gvt_host.hypervisor_type]);

Review

Quoted 16 lines

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

+		return -ENODEV;
+	}
+	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);
+
+void
+intel_gvt_unregister_hypervisor(void)
+{
+	intel_gvt_hypervisor_host_exit(intel_gvt_host.dev);
+	module_put(THIS_MODULE);
+}
+EXPORT_SYMBOL_GPL(intel_gvt_unregister_hypervisor);

(quoted diff) +0 -0

b/drivers/gpu/drm/i915/gvt/gvt.h

Review

Quoted 18 lines

index b4ab1dad0143..8a4cf995d755 100644

drivers/gpu/drm/i915/gvt/gvt.h +1 -5

@@ -52,12 +52,8 @@
 
 #define GVT_MAX_VGPU 8
 
-enum {
-	INTEL_GVT_HYPERVISOR_XEN = 0,
-	INTEL_GVT_HYPERVISOR_KVM,
-};
-
 struct intel_gvt_host {
+	struct device *dev;
 	bool initialized;
 	int hypervisor_type;
 	struct intel_gvt_mpt *mpt;

(quoted diff) +0 -0

b/drivers/gpu/drm/i915/gvt/hypercall.h

Quoted 4 lines

index e49a9247ed78..50798868ab15 100644

drivers/gpu/drm/i915/gvt/hypercall.h line 42

Quoted 10 lines

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

@@ -33,11 +33,17 @@
 #ifndef _GVT_HYPERCALL_H_
 #define _GVT_HYPERCALL_H_
 
+enum hypervisor_type {
+	INTEL_GVT_HYPERVISOR_XEN = 0,
+	INTEL_GVT_HYPERVISOR_KVM,
+};
+
 /*
  * Specific GVT-g MPT modules function collections. Currently GVT-g

supports

drivers/gpu/drm/i915/gvt/hypercall.h line 43

Quoted 1 line

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

  * both Xen and KVM by providing dedicated hypervisor-related MPT

modules.

Quoted 9 lines

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

  */
 struct intel_gvt_mpt {
+	enum hypervisor_type type;
 	int (*host_init)(struct device *dev, void *gvt, const void *ops);
 	void (*host_exit)(struct device *dev);
 	int (*attach_vgpu)(void *vgpu, unsigned long *handle);

@@ -67,6 +73,5 @@ struct intel_gvt_mpt {
 };
 

drivers/gpu/drm/i915/gvt/hypercall.h struct intel_gvt_mpt { · line 75

Quoted 1 line

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

 extern struct intel_gvt_mpt xengt_mpt;

Review

Quoted 4 lines

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

-extern struct intel_gvt_mpt kvmgt_mpt;
 
 #endif /* _GVT_HYPERCALL_H_ */

(quoted diff) +0 -0

b/drivers/gpu/drm/i915/gvt/kvmgt.c

drivers/gpu/drm/i915/gvt/kvmgt.c line 53

Quoted 8 lines

index 1bbd04d30c42..ef248d577e49 100644

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

@@ -50,6 +50,7 @@
 #include "gvt.h"
 
 static const struct intel_gvt_ops *intel_gvt_ops;
+static struct intel_gvt *intel_gvt;

drivers/gpu/drm/i915/gvt/kvmgt.c static int intel_vgpu_open(struct mdev_device

Quoted 4 lines

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

 /* helper macros copied from vfio-pci */
 #define VFIO_PCI_OFFSET_SHIFT   40

@@ -627,6 +628,12 @@ static int intel_vgpu_open(struct mdev_device

*mdev)

drivers/gpu/drm/i915/gvt/kvmgt.c static void __intel_vgpu_release(struct intel_vgpu

Quoted 13 lines

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

 		goto undo_iommu;
 	}
 
+	/* Take a module reference as mdev core doesn't take
+	 * a reference for vendor driver.
+	 */
+	if (!try_module_get(THIS_MODULE))
+		goto undo_group;
+
 	ret = kvmgt_guest_init(mdev);
 	if (ret)
 		goto undo_group;

@@ -679,6 +686,9 @@ static void __intel_vgpu_release(struct intel_vgpu

*vgpu)

drivers/gpu/drm/i915/gvt/kvmgt.c static int kvmgt_host_init(struct device *dev,

Quoted 10 lines

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

 					&vgpu->vdev.group_notifier);
 	WARN(ret, "vfio_unregister_notifier for group failed: %d\n", ret);
 
+	/* dereference module reference taken at open */
+	module_put(THIS_MODULE);
+
 	info = (struct kvmgt_guest_info *)vgpu->handle;
 	kvmgt_guest_exit(info);
 

@@ -1459,8 +1469,10 @@ static int kvmgt_host_init(struct device *dev,

void *gvt, const void *ops)

drivers/gpu/drm/i915/gvt/kvmgt.c static int kvmgt_host_init(struct device *dev, void *gvt, const void *ops) · line 1475

Quoted 8 lines

drivers/gpu/drm/i915/gvt/kvmgt.c +3 -1

 	struct attribute_group **kvm_vgpu_type_groups;
 
 	intel_gvt_ops = ops;
+	intel_gvt = (struct intel_gvt *)gvt;
+
 	if (!intel_gvt_ops->get_gvt_attrs(&kvm_type_attrs,
-			&kvm_vgpu_type_groups))
+					  &kvm_vgpu_type_groups))

drivers/gpu/drm/i915/gvt/kvmgt.c static bool kvmgt_is_valid_gfn(unsigned long

Quoted 4 lines

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

 		return -EFAULT;
 	intel_vgpu_ops.supported_type_groups = kvm_vgpu_type_groups;
 

@@ -1849,7 +1861,8 @@ static bool kvmgt_is_valid_gfn(unsigned long

handle, unsigned long gfn)

Review

Quoted 29 lines

drivers/gpu/drm/i915/gvt/kvmgt.c +5 -2

 	return ret;
 }
 
-struct intel_gvt_mpt kvmgt_mpt = {
+static struct intel_gvt_mpt kvmgt_mpt = {
+	.type = INTEL_GVT_HYPERVISOR_KVM,
 	.host_init = kvmgt_host_init,
 	.host_exit = kvmgt_host_exit,
 	.attach_vgpu = kvmgt_attach_vgpu,

@@ -1868,15 +1881,17 @@ struct intel_gvt_mpt kvmgt_mpt = {
 	.put_vfio_device = kvmgt_put_vfio_device,
 	.is_valid_gfn = kvmgt_is_valid_gfn,
 };
-EXPORT_SYMBOL_GPL(kvmgt_mpt);
 
 static int __init kvmgt_init(void)
 {
+	if (intel_gvt_register_hypervisor(&kvmgt_mpt) < 0)
+		return -ENODEV;
 	return 0;
 }
 
 static void __exit kvmgt_exit(void)
 {
+	intel_gvt_unregister_hypervisor();
 }
 
 module_init(kvmgt_init);

(quoted diff) +0 -0

b/drivers/gpu/drm/i915/gvt/mpt.h

drivers/gpu/drm/i915/gvt/mpt.h static inline bool

Quoted 4 lines

index c95ef77da62c..9b4225d44243 100644

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

@@ -360,4 +360,7 @@ static inline bool

intel_gvt_hypervisor_is_valid_gfn(

Review

Quoted 8 lines

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

 	return intel_gvt_host.mpt->is_valid_gfn(vgpu->handle, gfn);
 }
 
+int intel_gvt_register_hypervisor(struct intel_gvt_mpt *);
+void intel_gvt_unregister_hypervisor(void);
+
 #endif /* _GVT_MPT_H_ */

(quoted diff) +0 -0

b/drivers/gpu/drm/i915/intel_gvt.c

drivers/gpu/drm/i915/intel_gvt.c int intel_gvt_init(struct drm_i915_private

Quoted 4 lines

index c22b3e18a0f5..d74e59e22c9d 100644

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

@@ -105,15 +105,6 @@ int intel_gvt_init(struct drm_i915_private

*dev_priv)

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

Quoted 9 lines

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

 		return -EIO;
 	}
 
-	/*
-	 * We're not in host or fail to find a MPT module, disable GVT-g
-	 */
-	ret = intel_gvt_init_host();
-	if (ret) {
-		DRM_DEBUG_DRIVER("Not in host or MPT modules not

found\n");

Quoted 9 lines

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

-		goto bail;
-	}
-
 	ret = intel_gvt_init_device(dev_priv);
 	if (ret) {
 		DRM_DEBUG_DRIVER("Fail to init GVT device\n");
--

2.19.1

Quoted 4 lines

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

--
Open Source Technology Center, Intel ltd.

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

Zhenyu,
Overall I think the impact to AcrnGT is not big, we can modify our code to adapt to the new interfaces.
But I have some comments embedded.

Quoted 23 lines

Review int intel_gvt_init_device(struct drm_i915_private

Quoted 1 line

(quoted diff) +0 -0

@@ -467,6 +408,44 @@ int intel_gvt_init_device(struct drm_i915_private

*dev_priv)

Quoted 21 lines

drivers/gpu/drm/i915/gvt/gvt.c +15 -3

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

Quoted 8 lines

drivers/gpu/drm/i915/gvt/gvt.c int intel_gvt_init_device(struct drm_i915_private *dev_priv) · line 433

Quoted 8 lines

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

+		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);

I think we can remove the host_init and host_exit interfaces, since now it's mpt modules who proactively call the GVT-g to initialize and un-initialize, Thus we can return the intel_gvt_ops in intel_gvt_register_hypervisor() and moduels initialize its rest part.
Current kvmgt_init-> intel_gvt_register_hypervisor->host_init seems a little bit redundant.
But it's up to you to make the call to remove it in this patch or other furture optimization patches.

Quoted 4 lines

drivers/gpu/drm/i915/gvt/hypercall.h struct intel_gvt_mpt { · line 75

Quoted 4 lines

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

@@ -67,6 +73,5 @@ struct intel_gvt_mpt {
 };
 
 extern struct intel_gvt_mpt xengt_mpt;

We can remove this definition, too.

Quoted 8 lines

Review

Quoted 1 line

(quoted diff) +0 -0

b/drivers/gpu/drm/i915/gvt/kvmgt.c

Quoted 6 lines

index 1bbd04d30c42..ef248d577e49 100644

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

@@ -50,6 +50,7 @@
 #include "gvt.h"
 

Quoted 2 lines

drivers/gpu/drm/i915/gvt/kvmgt.c line 53

Quoted 2 lines

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

 static const struct intel_gvt_ops *intel_gvt_ops;
+static struct intel_gvt *intel_gvt;

This variable intel_gvt seems useless.

-----Original Message-----
From: intel-gvt-dev [mailto:[email protected]] On Behalf Of Zhenyu Wang
Sent: Tuesday, December 4, 2018 10:40 AM
To: [email protected]
Cc: Alex Williamson <[email protected]>; intel-gvt- [email protected]
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 <[email protected]>

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

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

drivers/gpu/drm/i915/Makefile +1 -0

index 0ff878c994e2..ae0d975a6f34 100644
@@ -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

drivers/gpu/drm/i915/gvt/Makefile +0 -0

index b016dc753db9..271fb46d4dd0 100644
@@ -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))

drivers/gpu/drm/i915/gvt/Makefile +0 -1

-obj-$(CONFIG_DRM_I915_GVT_KVMGT)	+= $(GVT_DIR)/kvmgt.o

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

index a5b760b7bc10..e1c9c20918af 100644
@@ -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.
- *
- */

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

-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;

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

-	} 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;

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

-#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]);

drivers/gpu/drm/i915/gvt/gvt.c +1 -20

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

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

 		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:

drivers/gpu/drm/i915/gvt/gvt.c +25 -3

@@ -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]);

-----Original Message-----
From: Zhenyu Wang [mailto:[email protected]]
Sent: Thursday, December 6, 2018 4:03 PM
To: [email protected]
Cc: [email protected]; Yuan, Hang <[email protected]>; Alex Williamson <[email protected]>
Subject: [PATCH v5] 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.

v5:
- put module reference in register error path

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 "Yuan, Hang" <[email protected]>

Cc Alex Williamson <[email protected]>

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

---
 drivers/gpu/drm/i915/Makefile        |   1 +
 drivers/gpu/drm/i915/gvt/Makefile    |   1 -
 drivers/gpu/drm/i915/gvt/gvt.c       | 108 +++++++++++----------------
 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, 73 insertions(+), 83 deletions(-)

drivers/gpu/drm/i915/Makefile +1 -0

index 0ff878c994e2..ae0d975a6f34 100644
@@ -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

drivers/gpu/drm/i915/gvt/Makefile +0 -0

index b016dc753db9..271fb46d4dd0 100644
@@ -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))

drivers/gpu/drm/i915/gvt/Makefile +0 -1

-obj-$(CONFIG_DRM_I915_GVT_KVMGT)	+= $(GVT_DIR)/kvmgt.o

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

index a5b760b7bc10..4e8947f33bd0 100644
@@ -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.
- *
- */

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

-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;

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

-	} 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;

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

-#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]);

drivers/gpu/drm/i915/gvt/gvt.c +1 -20

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

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

 		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:

drivers/gpu/drm/i915/gvt/gvt.c +25 -3

@@ -467,6 +408,45 @@ 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]);

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

+		module_put(THIS_MODULE);
+		return -ENODEV;
+	}
+	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);
+
+void
+intel_gvt_unregister_hypervisor(void)
+{
+	intel_gvt_hypervisor_host_exit(intel_gvt_host.dev);
+	module_put(THIS_MODULE);
+}
+EXPORT_SYMBOL_GPL(intel_gvt_unregister_hypervisor);

drivers/gpu/drm/i915/gvt/gvt.h +1 -5

index b4ab1dad0143..8a4cf995d755 100644
@@ -52,12 +52,8 @@
 
 #define GVT_MAX_VGPU 8
 
-enum {
-	INTEL_GVT_HYPERVISOR_XEN = 0,
-	INTEL_GVT_HYPERVISOR_KVM,
-};
-
 struct intel_gvt_host {
+	struct device *dev;
 	bool initialized;
 	int hypervisor_type;
 	struct intel_gvt_mpt *mpt;

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

index e49a9247ed78..50798868ab15 100644
@@ -33,11 +33,17 @@
 #ifndef _GVT_HYPERCALL_H_
 #define _GVT_HYPERCALL_H_
 
+enum hypervisor_type {
+	INTEL_GVT_HYPERVISOR_XEN = 0,
+	INTEL_GVT_HYPERVISOR_KVM,
+};
+
 /*
  * Specific GVT-g MPT modules function collections. Currently GVT-g

supports
* both Xen and KVM by providing dedicated hypervisor-related MPT
modules.
*/
struct intel_gvt_mpt {

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

+	enum hypervisor_type type;
 	int (*host_init)(struct device *dev, void *gvt, const void *ops);
 	void (*host_exit)(struct device *dev);
 	int (*attach_vgpu)(void *vgpu, unsigned long *handle); @@ -67,6
+73,5 @@ struct intel_gvt_mpt {  };
 
 extern struct intel_gvt_mpt xengt_mpt;
-extern struct intel_gvt_mpt kvmgt_mpt;
 
 #endif /* _GVT_HYPERCALL_H_ */

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

index 1bbd04d30c42..ef248d577e49 100644
@@ -50,6 +50,7 @@
 #include "gvt.h"
 
 static const struct intel_gvt_ops *intel_gvt_ops;
+static struct intel_gvt *intel_gvt;
 
 /* helper macros copied from vfio-pci */
 #define VFIO_PCI_OFFSET_SHIFT   40

@@ -627,6 +628,12 @@ static int intel_vgpu_open(struct mdev_device
 *mdev)
 		goto undo_iommu;
 	}
 
+	/* Take a module reference as mdev core doesn't take
+	 * a reference for vendor driver.
+	 */
+	if (!try_module_get(THIS_MODULE))
+		goto undo_group;
+
 	ret = kvmgt_guest_init(mdev);
 	if (ret)

drivers/gpu/drm/i915/gvt/kvmgt.c +8 -2

 		goto undo_group;

@@ -679,6 +686,9 @@ static void __intel_vgpu_release(struct intel_vgpu
 *vgpu)
 					&vgpu->vdev.group_notifier);
 	WARN(ret, "vfio_unregister_notifier for group failed: %d\n", ret);
 
+	/* dereference module reference taken at open */
+	module_put(THIS_MODULE);
+
 	info = (struct kvmgt_guest_info *)vgpu->handle;
 	kvmgt_guest_exit(info);

@@ -1459,8 +1469,10 @@ static int kvmgt_host_init(struct device *dev, void
 *gvt, const void *ops)
 	struct attribute_group **kvm_vgpu_type_groups;
 
 	intel_gvt_ops = ops;
+	intel_gvt = (struct intel_gvt *)gvt;
+
 	if (!intel_gvt_ops->get_gvt_attrs(&kvm_type_attrs,
-			&kvm_vgpu_type_groups))
+					  &kvm_vgpu_type_groups))
 		return -EFAULT;
 	intel_vgpu_ops.supported_type_groups = kvm_vgpu_type_groups;

@@ -1849,7 +1861,8 @@ static bool kvmgt_is_valid_gfn(unsigned long
 handle, unsigned long gfn)
 	return ret;
 }
 
-struct intel_gvt_mpt kvmgt_mpt = {
+static struct intel_gvt_mpt kvmgt_mpt = {
+	.type = INTEL_GVT_HYPERVISOR_KVM,
 	.host_init = kvmgt_host_init,
 	.host_exit = kvmgt_host_exit,

drivers/gpu/drm/i915/gvt/kvmgt.c +3 -1

 	.attach_vgpu = kvmgt_attach_vgpu,

@@ -1868,15 +1881,17 @@ struct intel_gvt_mpt kvmgt_mpt = {
 	.put_vfio_device = kvmgt_put_vfio_device,
 	.is_valid_gfn = kvmgt_is_valid_gfn,
 };
-EXPORT_SYMBOL_GPL(kvmgt_mpt);
 
 static int __init kvmgt_init(void)
 {
+	if (intel_gvt_register_hypervisor(&kvmgt_mpt) < 0)
+		return -ENODEV;
 	return 0;
 }
 
 static void __exit kvmgt_exit(void)
 {
+	intel_gvt_unregister_hypervisor();
 }
 
 module_init(kvmgt_init);

(quoted diff) +0 -0

b/drivers/gpu/drm/i915/gvt/mpt.h index c95ef77da62c..9b4225d44243 100644

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

@@ -360,4 +360,7 @@ static inline bool intel_gvt_hypervisor_is_valid_gfn(
 	return intel_gvt_host.mpt->is_valid_gfn(vgpu->handle, gfn);  }
 
+int intel_gvt_register_hypervisor(struct intel_gvt_mpt *); void
+intel_gvt_unregister_hypervisor(void);
+
 #endif /* _GVT_MPT_H_ */

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

index c22b3e18a0f5..d74e59e22c9d 100644
@@ -105,15 +105,6 @@ int intel_gvt_init(struct drm_i915_private *dev_priv)
 		return -EIO;
 	}
 
-	/*
-	 * We're not in host or fail to find a MPT module, disable GVT-g
-	 */
-	ret = intel_gvt_init_host();
-	if (ret) {
-		DRM_DEBUG_DRIVER("Not in host or MPT modules not
 found\n");
-		goto bail;
-	}
-
 	ret = intel_gvt_init_device(dev_priv);
 	if (ret) {

DRM_DEBUG_DRIVER("Fail to init GVT device\n");
--
2.19.1