All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v3 0/3] Change KVMGT into self loadable module
@ 2018-12-03  4:05 Zhenyu Wang
  2018-12-03  4:05 ` [PATCH v3 1/3] drm/i915/gvt: mandatory require hypervisor's host_init Zhenyu Wang
                   ` (9 more replies)
  0 siblings, 10 replies; 18+ messages in thread
From: Zhenyu Wang @ 2018-12-03  4:05 UTC (permalink / raw)
  To: intel-gfx; +Cc: intel-gvt-dev

This series try to change 'kvmgt' as self loadable module to enable
GVT feature for VFIO/mdev interface when user loads 'kvmgt' module,
instead of current method to load hypervisor module when i915/gvt
device model initializes.

v3:
- Fix module reference handling for device open and release. Unused
  mdev devices would be cleaned up in device unregister when module unload.

Zhenyu Wang (3):
  drm/i915/gvt: mandatory require hypervisor's host_init
  drm/i915/gvt: remove unused parameter for hypervisor's host_exit call
  drm/i915/gvt: Change KVMGT as self load module

 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 |   9 ++-
 drivers/gpu/drm/i915/gvt/kvmgt.c     |  22 +++++-
 drivers/gpu/drm/i915/gvt/mpt.h       |  13 ++--
 drivers/gpu/drm/i915/intel_gvt.c     |   9 ---
 8 files changed, 77 insertions(+), 91 deletions(-)

-- 
2.19.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 18+ messages in thread
* [PATCH v3 1/3] drm/i915/gvt: mandatory require hypervisor's host_init
  2018-12-03  4:05 [PATCH v3 0/3] Change KVMGT into self loadable module Zhenyu Wang
@ 2018-12-03  4:05 ` Zhenyu Wang
  2018-12-03  4:05 ` [PATCH v3 2/3] drm/i915/gvt: remove unused parameter for hypervisor's host_exit call Zhenyu Wang
                   ` (8 subsequent siblings)
  9 siblings, 0 replies; 18+ messages in thread
From: Zhenyu Wang @ 2018-12-03  4:05 UTC (permalink / raw)
  To: intel-gfx; +Cc: intel-gvt-dev

Don't mark hypervisor module's host_init as optional,
but mandatory required.

Signed-off-by: Zhenyu Wang <zhenyuw@linux.intel.com>
---
 drivers/gpu/drm/i915/gvt/mpt.h | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/gvt/mpt.h b/drivers/gpu/drm/i915/gvt/mpt.h
index 67f19992b226..ce721099a020 100644
--- a/drivers/gpu/drm/i915/gvt/mpt.h
+++ b/drivers/gpu/drm/i915/gvt/mpt.h
@@ -50,11 +50,10 @@
  * Zero on success, negative error code if failed
  */
 static inline int intel_gvt_hypervisor_host_init(struct device *dev,
-			void *gvt, const void *ops)
+						 void *gvt, const void *ops)
 {
-	/* optional to provide */
 	if (!intel_gvt_host.mpt->host_init)
-		return 0;
+		return -ENODEV;
 
 	return intel_gvt_host.mpt->host_init(dev, gvt, ops);
 }
-- 
2.19.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related	[flat|nested] 18+ messages in thread
* [PATCH v3 2/3] drm/i915/gvt: remove unused parameter for hypervisor's host_exit call
  2018-12-03  4:05 [PATCH v3 0/3] Change KVMGT into self loadable module Zhenyu Wang
  2018-12-03  4:05 ` [PATCH v3 1/3] drm/i915/gvt: mandatory require hypervisor's host_init Zhenyu Wang
@ 2018-12-03  4:05 ` Zhenyu Wang
  2018-12-03  4:05 ` [PATCH v3 3/3] drm/i915/gvt: Change KVMGT as self load module Zhenyu Wang
                   ` (7 subsequent siblings)
  9 siblings, 0 replies; 18+ messages in thread
From: Zhenyu Wang @ 2018-12-03  4:05 UTC (permalink / raw)
  To: intel-gfx; +Cc: intel-gvt-dev

The parameter 'void *gvt' is not used and required for hypervisor's
exit call. Even for non-merged Xen hypervisor support. So just remove it.

Signed-off-by: Zhenyu Wang <zhenyuw@linux.intel.com>
---
 drivers/gpu/drm/i915/gvt/gvt.c       | 2 +-
 drivers/gpu/drm/i915/gvt/hypercall.h | 2 +-
 drivers/gpu/drm/i915/gvt/kvmgt.c     | 2 +-
 drivers/gpu/drm/i915/gvt/mpt.h       | 5 ++---
 4 files changed, 5 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/i915/gvt/gvt.c b/drivers/gpu/drm/i915/gvt/gvt.c
index 733a2a0d0c30..a5b760b7bc10 100644
--- a/drivers/gpu/drm/i915/gvt/gvt.c
+++ b/drivers/gpu/drm/i915/gvt/gvt.c
@@ -316,7 +316,7 @@ 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, gvt);
+	intel_gvt_hypervisor_host_exit(&dev_priv->drm.pdev->dev);
 	intel_gvt_cleanup_vgpu_type_groups(gvt);
 	intel_gvt_clean_vgpu_types(gvt);
 
diff --git a/drivers/gpu/drm/i915/gvt/hypercall.h b/drivers/gpu/drm/i915/gvt/hypercall.h
index 5af11cf1b482..e49a9247ed78 100644
--- a/drivers/gpu/drm/i915/gvt/hypercall.h
+++ b/drivers/gpu/drm/i915/gvt/hypercall.h
@@ -39,7 +39,7 @@
  */
 struct intel_gvt_mpt {
 	int (*host_init)(struct device *dev, void *gvt, const void *ops);
-	void (*host_exit)(struct device *dev, void *gvt);
+	void (*host_exit)(struct device *dev);
 	int (*attach_vgpu)(void *vgpu, unsigned long *handle);
 	void (*detach_vgpu)(unsigned long handle);
 	int (*inject_msi)(unsigned long handle, u32 addr, u16 data);
diff --git a/drivers/gpu/drm/i915/gvt/kvmgt.c b/drivers/gpu/drm/i915/gvt/kvmgt.c
index c1072143da1d..1bbd04d30c42 100644
--- a/drivers/gpu/drm/i915/gvt/kvmgt.c
+++ b/drivers/gpu/drm/i915/gvt/kvmgt.c
@@ -1467,7 +1467,7 @@ static int kvmgt_host_init(struct device *dev, void *gvt, const void *ops)
 	return mdev_register_device(dev, &intel_vgpu_ops);
 }
 
-static void kvmgt_host_exit(struct device *dev, void *gvt)
+static void kvmgt_host_exit(struct device *dev)
 {
 	mdev_unregister_device(dev);
 }
diff --git a/drivers/gpu/drm/i915/gvt/mpt.h b/drivers/gpu/drm/i915/gvt/mpt.h
index ce721099a020..c95ef77da62c 100644
--- a/drivers/gpu/drm/i915/gvt/mpt.h
+++ b/drivers/gpu/drm/i915/gvt/mpt.h
@@ -61,14 +61,13 @@ static inline int intel_gvt_hypervisor_host_init(struct device *dev,
 /**
  * intel_gvt_hypervisor_host_exit - exit GVT-g host side
  */
-static inline void intel_gvt_hypervisor_host_exit(struct device *dev,
-			void *gvt)
+static inline void intel_gvt_hypervisor_host_exit(struct device *dev)
 {
 	/* optional to provide */
 	if (!intel_gvt_host.mpt->host_exit)
 		return;
 
-	intel_gvt_host.mpt->host_exit(dev, gvt);
+	intel_gvt_host.mpt->host_exit(dev);
 }
 
 /**
-- 
2.19.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related	[flat|nested] 18+ messages in thread
* [PATCH v3 3/3] drm/i915/gvt: Change KVMGT as self load module
  2018-12-03  4:05 [PATCH v3 0/3] Change KVMGT into self loadable module Zhenyu Wang
  2018-12-03  4:05 ` [PATCH v3 1/3] drm/i915/gvt: mandatory require hypervisor's host_init Zhenyu Wang
  2018-12-03  4:05 ` [PATCH v3 2/3] drm/i915/gvt: remove unused parameter for hypervisor's host_exit call Zhenyu Wang
@ 2018-12-03  4:05 ` Zhenyu Wang
  2018-12-04  2:40   ` [PATCH v4] " Zhenyu Wang
  2018-12-03  4:14 ` ✗ Fi.CI.CHECKPATCH: warning for Change KVMGT into self loadable module Patchwork
                   ` (6 subsequent siblings)
  9 siblings, 1 reply; 18+ messages in thread
From: Zhenyu Wang @ 2018-12-03  4:05 UTC (permalink / raw)
  To: intel-gfx; +Cc: intel-gvt-dev

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 reference counting
for any VFIO/mdev creation.

This makes 'kvmgt' module really split from GVT device model. User
needs to load 'kvmgt' to enable VFIO/mdev interface.

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     |  20 ++++-
 drivers/gpu/drm/i915/gvt/mpt.h       |   3 +
 drivers/gpu/drm/i915/intel_gvt.c     |   9 ---
 8 files changed, 71 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;
+	}
+	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);
diff --git a/drivers/gpu/drm/i915/gvt/gvt.h b/drivers/gpu/drm/i915/gvt/gvt.h
index b4ab1dad0143..8a4cf995d755 100644
--- a/drivers/gpu/drm/i915/gvt/gvt.h
+++ b/drivers/gpu/drm/i915/gvt/gvt.h
@@ -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;
diff --git a/drivers/gpu/drm/i915/gvt/hypercall.h b/drivers/gpu/drm/i915/gvt/hypercall.h
index e49a9247ed78..50798868ab15 100644
--- a/drivers/gpu/drm/i915/gvt/hypercall.h
+++ b/drivers/gpu/drm/i915/gvt/hypercall.h
@@ -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_ */
diff --git a/drivers/gpu/drm/i915/gvt/kvmgt.c b/drivers/gpu/drm/i915/gvt/kvmgt.c
index 1bbd04d30c42..f867b4408d7f 100644
--- a/drivers/gpu/drm/i915/gvt/kvmgt.c
+++ b/drivers/gpu/drm/i915/gvt/kvmgt.c
@@ -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,11 @@ 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 +685,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 +1468,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 +1860,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 +1880,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);
diff --git a/drivers/gpu/drm/i915/gvt/mpt.h b/drivers/gpu/drm/i915/gvt/mpt.h
index c95ef77da62c..181194000fbd 100644
--- a/drivers/gpu/drm/i915/gvt/mpt.h
+++ b/drivers/gpu/drm/i915/gvt/mpt.h
@@ -360,4 +360,7 @@ static inline bool intel_gvt_hypervisor_is_valid_gfn(
 	return intel_gvt_host.mpt->is_valid_gfn(vgpu->handle, gfn);
 }
 
+extern int intel_gvt_register_hypervisor(struct intel_gvt_mpt *);
+extern void intel_gvt_unregister_hypervisor(void);
+
 #endif /* _GVT_MPT_H_ */
diff --git a/drivers/gpu/drm/i915/intel_gvt.c b/drivers/gpu/drm/i915/intel_gvt.c
index c22b3e18a0f5..d74e59e22c9d 100644
--- a/drivers/gpu/drm/i915/intel_gvt.c
+++ b/drivers/gpu/drm/i915/intel_gvt.c
@@ -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

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related	[flat|nested] 18+ messages in thread
* ✗ Fi.CI.CHECKPATCH: warning for Change KVMGT into self loadable module
  2018-12-03  4:05 [PATCH v3 0/3] Change KVMGT into self loadable module Zhenyu Wang
                   ` (2 preceding siblings ...)
  2018-12-03  4:05 ` [PATCH v3 3/3] drm/i915/gvt: Change KVMGT as self load module Zhenyu Wang
@ 2018-12-03  4:14 ` Patchwork
  2018-12-03  4:32 ` ✓ Fi.CI.BAT: success " Patchwork
                   ` (5 subsequent siblings)
  9 siblings, 0 replies; 18+ messages in thread
From: Patchwork @ 2018-12-03  4:14 UTC (permalink / raw)
  To: Zhenyu Wang; +Cc: intel-gfx

== Series Details ==

Series: Change KVMGT into self loadable module
URL   : https://patchwork.freedesktop.org/series/53379/
State : warning

== Summary ==

$ dim checkpatch origin/drm-tip
c0de8b6d6a23 drm/i915/gvt: mandatory require hypervisor's host_init
98c49ae62049 drm/i915/gvt: remove unused parameter for hypervisor's host_exit call
0cf5b56dab69 drm/i915/gvt: Change KVMGT as self load module
-:261: WARNING:BLOCK_COMMENT_STYLE: Block comments use * on subsequent lines
#261: FILE: drivers/gpu/drm/i915/gvt/kvmgt.c:632:
+	/* Take a module reference as mdev core doesn't take
+	   a reference for vendor driver */

-:261: WARNING:BLOCK_COMMENT_STYLE: Block comments use a trailing */ on a separate line
#261: FILE: drivers/gpu/drm/i915/gvt/kvmgt.c:632:
+	   a reference for vendor driver */

-:327: CHECK:AVOID_EXTERNS: extern prototypes should be avoided in .h files
#327: FILE: drivers/gpu/drm/i915/gvt/mpt.h:363:
+extern int intel_gvt_register_hypervisor(struct intel_gvt_mpt *);

-:328: CHECK:AVOID_EXTERNS: extern prototypes should be avoided in .h files
#328: FILE: drivers/gpu/drm/i915/gvt/mpt.h:364:
+extern void intel_gvt_unregister_hypervisor(void);

total: 0 errors, 2 warnings, 2 checks, 270 lines checked

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 18+ messages in thread
* ✓ Fi.CI.BAT: success for Change KVMGT into self loadable module
  2018-12-03  4:05 [PATCH v3 0/3] Change KVMGT into self loadable module Zhenyu Wang
                   ` (3 preceding siblings ...)
  2018-12-03  4:14 ` ✗ Fi.CI.CHECKPATCH: warning for Change KVMGT into self loadable module Patchwork
@ 2018-12-03  4:32 ` Patchwork
  2018-12-03  5:34 ` ✓ Fi.CI.IGT: " Patchwork
                   ` (4 subsequent siblings)
  9 siblings, 0 replies; 18+ messages in thread
From: Patchwork @ 2018-12-03  4:32 UTC (permalink / raw)
  To: Zhenyu Wang; +Cc: intel-gfx

== Series Details ==

Series: Change KVMGT into self loadable module
URL   : https://patchwork.freedesktop.org/series/53379/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_5237 -> Patchwork_10993
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

  External URL: https://patchwork.freedesktop.org/api/1.0/series/53379/revisions/1/mbox/

Known issues
------------

  Here are the changes found in Patchwork_10993 that come from known issues:

### IGT changes ###

#### Issues hit ####

  * igt@gem_exec_suspend@basic-s4-devices:
    - fi-ivb-3520m:       PASS -> FAIL [fdo#108880]

  * igt@i915_selftest@live_coherency:
    - fi-gdg-551:         PASS -> DMESG-FAIL [fdo#107164]

  * igt@i915_selftest@live_contexts:
    - {fi-icl-u3}:        NOTRUN -> DMESG-FAIL [fdo#108569]

  * igt@kms_flip@basic-flip-vs-modeset:
    - fi-skl-6700hq:      PASS -> DMESG-WARN [fdo#105998] +2

  * igt@kms_psr@sprite_plane_onoff:
    - fi-skl-6700hq:      PASS -> FAIL [fdo#107383] +3

  * igt@prime_vgem@basic-fence-flip:
    - fi-gdg-551:         PASS -> DMESG-FAIL [fdo#103182]

  * {igt@runner@aborted}:
    - {fi-icl-y}:         NOTRUN -> FAIL [fdo#108070]

  
#### Possible fixes ####

  * igt@kms_flip@basic-flip-vs-dpms:
    - {fi-icl-u3}:        DMESG-WARN -> PASS

  
  {name}: This element is suppressed. This means it is ignored when computing
          the status of the difference (SUCCESS, WARNING, or FAILURE).

  [fdo#103182]: https://bugs.freedesktop.org/show_bug.cgi?id=103182
  [fdo#105998]: https://bugs.freedesktop.org/show_bug.cgi?id=105998
  [fdo#107164]: https://bugs.freedesktop.org/show_bug.cgi?id=107164
  [fdo#107383]: https://bugs.freedesktop.org/show_bug.cgi?id=107383
  [fdo#108070]: https://bugs.freedesktop.org/show_bug.cgi?id=108070
  [fdo#108569]: https://bugs.freedesktop.org/show_bug.cgi?id=108569
  [fdo#108880]: https://bugs.freedesktop.org/show_bug.cgi?id=108880


Participating hosts (49 -> 41)
------------------------------

  Additional (1): fi-icl-y 
  Missing    (9): fi-kbl-soraka fi-ilk-m540 fi-hsw-4200u fi-hsw-peppy fi-byt-squawks fi-bsw-cyan fi-ctg-p8600 fi-kbl-7560u fi-byt-clapper 


Build changes
-------------

    * Linux: CI_DRM_5237 -> Patchwork_10993

  CI_DRM_5237: 2f99c4889e4124f9cf50b745d037f432318c4bb4 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4736: 285ebfb3b7adc56586031afa5150c4e5ad40c229 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_10993: 0cf5b56dab690d4c0c8b3c61718852c9943a736a @ git://anongit.freedesktop.org/gfx-ci/linux


== Linux commits ==

0cf5b56dab69 drm/i915/gvt: Change KVMGT as self load module
98c49ae62049 drm/i915/gvt: remove unused parameter for hypervisor's host_exit call
c0de8b6d6a23 drm/i915/gvt: mandatory require hypervisor's host_init

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_10993/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 18+ messages in thread
* ✓ Fi.CI.IGT: success for Change KVMGT into self loadable module
  2018-12-03  4:05 [PATCH v3 0/3] Change KVMGT into self loadable module Zhenyu Wang
                   ` (4 preceding siblings ...)
  2018-12-03  4:32 ` ✓ Fi.CI.BAT: success " Patchwork
@ 2018-12-03  5:34 ` Patchwork
  2018-12-04  3:15 ` ✓ Fi.CI.BAT: success for Change KVMGT into self loadable module (rev2) Patchwork
                   ` (3 subsequent siblings)
  9 siblings, 0 replies; 18+ messages in thread
From: Patchwork @ 2018-12-03  5:34 UTC (permalink / raw)
  To: Zhenyu Wang; +Cc: intel-gfx

== Series Details ==

Series: Change KVMGT into self loadable module
URL   : https://patchwork.freedesktop.org/series/53379/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_5237_full -> Patchwork_10993_full
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

  

Known issues
------------

  Here are the changes found in Patchwork_10993_full that come from known issues:

### IGT changes ###

#### Issues hit ####

  * igt@gem_exec_schedule@pi-ringfull-bsd:
    - shard-skl:          NOTRUN -> FAIL [fdo#103158]

  * igt@gem_ppgtt@blt-vs-render-ctx0:
    - shard-skl:          PASS -> TIMEOUT [fdo#108039]

  * igt@i915_suspend@shrink:
    - shard-hsw:          NOTRUN -> DMESG-WARN [fdo#108784]
    - {shard-iclb}:       NOTRUN -> DMESG-WARN [fdo#108784]

  * igt@kms_busy@extended-modeset-hang-newfb-render-a:
    - shard-snb:          PASS -> DMESG-WARN [fdo#107956]

  * igt@kms_busy@extended-modeset-hang-newfb-render-b:
    - shard-skl:          NOTRUN -> DMESG-WARN [fdo#107956]

  * igt@kms_ccs@pipe-b-crc-sprite-planes-basic:
    - shard-glk:          PASS -> FAIL [fdo#108145]

  * igt@kms_cursor_crc@cursor-256x256-dpms:
    - shard-glk:          PASS -> FAIL [fdo#103232] +3

  * igt@kms_cursor_crc@cursor-64x21-sliding:
    - {shard-iclb}:       NOTRUN -> FAIL [fdo#103232] +2
    - shard-apl:          PASS -> FAIL [fdo#103232] +2

  * igt@kms_cursor_crc@cursor-64x64-random:
    - shard-skl:          PASS -> FAIL [fdo#103232]

  * igt@kms_cursor_legacy@2x-long-flip-vs-cursor-legacy:
    - shard-glk:          PASS -> FAIL [fdo#104873]

  * igt@kms_flip@2x-flip-vs-expired-vblank:
    - shard-glk:          PASS -> FAIL [fdo#105363]

  * igt@kms_flip@flip-vs-expired-vblank:
    - shard-skl:          PASS -> FAIL [fdo#105363]

  * igt@kms_frontbuffer_tracking@fbc-1p-primscrn-spr-indfb-draw-mmap-cpu:
    - shard-apl:          PASS -> FAIL [fdo#103167] +1

  * igt@kms_frontbuffer_tracking@fbc-stridechange:
    - shard-skl:          NOTRUN -> FAIL [fdo#105683]

  * igt@kms_frontbuffer_tracking@psr-1p-primscrn-spr-indfb-draw-blt:
    - {shard-iclb}:       PASS -> FAIL [fdo#103167] +3

  * igt@kms_plane@pixel-format-pipe-a-planes:
    - shard-skl:          NOTRUN -> DMESG-WARN [fdo#106885]

  * igt@kms_plane@plane-panning-bottom-right-suspend-pipe-c-planes:
    - shard-skl:          PASS -> INCOMPLETE [fdo#104108] / [fdo#107773]

  * igt@kms_plane_alpha_blend@pipe-b-coverage-7efc:
    - shard-skl:          PASS -> FAIL [fdo#107815]

  * igt@kms_plane_multiple@atomic-pipe-b-tiling-x:
    - {shard-iclb}:       PASS -> FAIL [fdo#103166]

  * igt@kms_plane_multiple@atomic-pipe-c-tiling-x:
    - shard-glk:          PASS -> FAIL [fdo#103166] +1
    - shard-apl:          PASS -> FAIL [fdo#103166]

  * igt@kms_plane_multiple@atomic-pipe-c-tiling-y:
    - {shard-iclb}:       NOTRUN -> FAIL [fdo#103166]

  * igt@kms_universal_plane@cursor-fb-leak-pipe-b:
    - shard-kbl:          PASS -> DMESG-WARN [fdo#103558] / [fdo#105602] +20

  
#### Possible fixes ####

  * igt@gem_ctx_isolation@rcs0-s3:
    - shard-snb:          DMESG-WARN [fdo#102365] -> PASS

  * igt@kms_cursor_crc@cursor-64x64-onscreen:
    - shard-apl:          FAIL [fdo#103232] -> PASS +2

  * igt@kms_frontbuffer_tracking@fbc-1p-primscrn-spr-indfb-draw-mmap-gtt:
    - shard-apl:          FAIL [fdo#103167] -> PASS

  * igt@kms_frontbuffer_tracking@fbc-2p-primscrn-spr-indfb-draw-mmap-cpu:
    - shard-glk:          FAIL [fdo#103167] -> PASS

  * igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-spr-indfb-onoff:
    - {shard-iclb}:       FAIL [fdo#103167] -> PASS +2

  * igt@kms_plane@plane-position-covered-pipe-b-planes:
    - shard-glk:          FAIL [fdo#103166] -> PASS +4
    - {shard-iclb}:       FAIL [fdo#103166] -> PASS +2

  * igt@kms_plane_alpha_blend@pipe-a-coverage-7efc:
    - shard-skl:          FAIL [fdo#107815] / [fdo#108145] -> PASS

  * igt@kms_plane_alpha_blend@pipe-b-alpha-opaque-fb:
    - shard-glk:          FAIL [fdo#108145] -> PASS

  * igt@kms_plane_multiple@atomic-pipe-a-tiling-x:
    - shard-apl:          FAIL [fdo#103166] -> PASS +5

  * igt@kms_plane_scaling@pipe-b-scaler-with-rotation:
    - {shard-iclb}:       DMESG-WARN [fdo#107724] -> PASS +1

  * igt@kms_vblank@pipe-b-ts-continuation-suspend:
    - {shard-iclb}:       INCOMPLETE [fdo#107713] -> PASS

  * igt@pm_rpm@legacy-planes:
    - {shard-iclb}:       DMESG-WARN [fdo#108654] -> PASS

  
#### Warnings ####

  * igt@i915_suspend@shrink:
    - shard-snb:          INCOMPLETE [fdo#105411] / [fdo#106886] -> DMESG-WARN [fdo#108784]

  * igt@kms_plane_alpha_blend@pipe-c-alpha-7efc:
    - shard-kbl:          FAIL [fdo#108145] / [fdo#108590] -> DMESG-FAIL [fdo#103558] / [fdo#105602] / [fdo#108145]

  
  {name}: This element is suppressed. This means it is ignored when computing
          the status of the difference (SUCCESS, WARNING, or FAILURE).

  [fdo#102365]: https://bugs.freedesktop.org/show_bug.cgi?id=102365
  [fdo#103158]: https://bugs.freedesktop.org/show_bug.cgi?id=103158
  [fdo#103166]: https://bugs.freedesktop.org/show_bug.cgi?id=103166
  [fdo#103167]: https://bugs.freedesktop.org/show_bug.cgi?id=103167
  [fdo#103232]: https://bugs.freedesktop.org/show_bug.cgi?id=103232
  [fdo#103558]: https://bugs.freedesktop.org/show_bug.cgi?id=103558
  [fdo#104108]: https://bugs.freedesktop.org/show_bug.cgi?id=104108
  [fdo#104873]: https://bugs.freedesktop.org/show_bug.cgi?id=104873
  [fdo#105363]: https://bugs.freedesktop.org/show_bug.cgi?id=105363
  [fdo#105411]: https://bugs.freedesktop.org/show_bug.cgi?id=105411
  [fdo#105602]: https://bugs.freedesktop.org/show_bug.cgi?id=105602
  [fdo#105683]: https://bugs.freedesktop.org/show_bug.cgi?id=105683
  [fdo#106885]: https://bugs.freedesktop.org/show_bug.cgi?id=106885
  [fdo#106886]: https://bugs.freedesktop.org/show_bug.cgi?id=106886
  [fdo#107713]: https://bugs.freedesktop.org/show_bug.cgi?id=107713
  [fdo#107724]: https://bugs.freedesktop.org/show_bug.cgi?id=107724
  [fdo#107773]: https://bugs.freedesktop.org/show_bug.cgi?id=107773
  [fdo#107815]: https://bugs.freedesktop.org/show_bug.cgi?id=107815
  [fdo#107956]: https://bugs.freedesktop.org/show_bug.cgi?id=107956
  [fdo#108039]: https://bugs.freedesktop.org/show_bug.cgi?id=108039
  [fdo#108145]: https://bugs.freedesktop.org/show_bug.cgi?id=108145
  [fdo#108590]: https://bugs.freedesktop.org/show_bug.cgi?id=108590
  [fdo#108654]: https://bugs.freedesktop.org/show_bug.cgi?id=108654
  [fdo#108784]: https://bugs.freedesktop.org/show_bug.cgi?id=108784


Participating hosts (7 -> 7)
------------------------------

  No changes in participating hosts


Build changes
-------------

    * Linux: CI_DRM_5237 -> Patchwork_10993

  CI_DRM_5237: 2f99c4889e4124f9cf50b745d037f432318c4bb4 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4736: 285ebfb3b7adc56586031afa5150c4e5ad40c229 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_10993: 0cf5b56dab690d4c0c8b3c61718852c9943a736a @ git://anongit.freedesktop.org/gfx-ci/linux
  piglit_4509: fdc5a4ca11124ab8413c7988896eec4c97336694 @ git://anongit.freedesktop.org/piglit

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_10993/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 18+ messages in thread
* [PATCH v4] drm/i915/gvt: Change KVMGT as self load module
  2018-12-03  4:05 ` [PATCH v3 3/3] drm/i915/gvt: Change KVMGT as self load module Zhenyu Wang
@ 2018-12-04  2:40   ` Zhenyu Wang
  2018-12-06  4:27     ` Zhenyu Wang
                       ` (2 more replies)
  0 siblings, 3 replies; 18+ messages in thread
From: Zhenyu Wang @ 2018-12-04  2:40 UTC (permalink / raw)
  To: intel-gfx; +Cc: intel-gvt-dev

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;
+	}
+	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);
diff --git a/drivers/gpu/drm/i915/gvt/gvt.h b/drivers/gpu/drm/i915/gvt/gvt.h
index b4ab1dad0143..8a4cf995d755 100644
--- a/drivers/gpu/drm/i915/gvt/gvt.h
+++ b/drivers/gpu/drm/i915/gvt/gvt.h
@@ -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;
diff --git a/drivers/gpu/drm/i915/gvt/hypercall.h b/drivers/gpu/drm/i915/gvt/hypercall.h
index e49a9247ed78..50798868ab15 100644
--- a/drivers/gpu/drm/i915/gvt/hypercall.h
+++ b/drivers/gpu/drm/i915/gvt/hypercall.h
@@ -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_ */
diff --git a/drivers/gpu/drm/i915/gvt/kvmgt.c b/drivers/gpu/drm/i915/gvt/kvmgt.c
index 1bbd04d30c42..ef248d577e49 100644
--- a/drivers/gpu/drm/i915/gvt/kvmgt.c
+++ b/drivers/gpu/drm/i915/gvt/kvmgt.c
@@ -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);
diff --git a/drivers/gpu/drm/i915/gvt/mpt.h b/drivers/gpu/drm/i915/gvt/mpt.h
index c95ef77da62c..9b4225d44243 100644
--- a/drivers/gpu/drm/i915/gvt/mpt.h
+++ b/drivers/gpu/drm/i915/gvt/mpt.h
@@ -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_ */
diff --git a/drivers/gpu/drm/i915/intel_gvt.c b/drivers/gpu/drm/i915/intel_gvt.c
index c22b3e18a0f5..d74e59e22c9d 100644
--- a/drivers/gpu/drm/i915/intel_gvt.c
+++ b/drivers/gpu/drm/i915/intel_gvt.c
@@ -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

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related	[flat|nested] 18+ messages in thread
* ✓ Fi.CI.BAT: success for Change KVMGT into self loadable module (rev2)
  2018-12-03  4:05 [PATCH v3 0/3] Change KVMGT into self loadable module Zhenyu Wang
                   ` (5 preceding siblings ...)
  2018-12-03  5:34 ` ✓ Fi.CI.IGT: " Patchwork
@ 2018-12-04  3:15 ` Patchwork
  2018-12-04  4:47 ` ✓ Fi.CI.IGT: " Patchwork
                   ` (2 subsequent siblings)
  9 siblings, 0 replies; 18+ messages in thread
From: Patchwork @ 2018-12-04  3:15 UTC (permalink / raw)
  To: Zhenyu Wang; +Cc: intel-gfx

== Series Details ==

Series: Change KVMGT into self loadable module (rev2)
URL   : https://patchwork.freedesktop.org/series/53379/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_5250 -> Patchwork_11005
====================================================

Summary
-------

  **WARNING**

  Minor unknown changes coming with Patchwork_11005 need to be verified
  manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in Patchwork_11005, please notify your bug team to allow them
  to document this new failure mode, which will reduce false positives in CI.

  External URL: https://patchwork.freedesktop.org/api/1.0/series/53379/revisions/2/mbox/

Possible new issues
-------------------

  Here are the unknown changes that may have been introduced in Patchwork_11005:

### IGT changes ###

#### Warnings ####

  * igt@kms_flip@basic-flip-vs-dpms:
    - fi-skl-6770hq:      PASS -> SKIP +36

  
Known issues
------------

  Here are the changes found in Patchwork_11005 that come from known issues:

### IGT changes ###

#### Issues hit ####

  * igt@gem_ctx_create@basic-files:
    - fi-bsw-n3050:       PASS -> FAIL [fdo#108656]

  
#### Possible fixes ####

  * igt@gem_mmap_gtt@basic-small-copy-xy:
    - fi-glk-dsi:         INCOMPLETE [fdo#103359] / [k.org#198133] -> PASS

  
  [fdo#103359]: https://bugs.freedesktop.org/show_bug.cgi?id=103359
  [fdo#108656]: https://bugs.freedesktop.org/show_bug.cgi?id=108656
  [k.org#198133]: https://bugzilla.kernel.org/show_bug.cgi?id=198133


Participating hosts (47 -> 41)
------------------------------

  Missing    (6): fi-ilk-m540 fi-hsw-4200u fi-byt-squawks fi-bsw-cyan fi-ctg-p8600 fi-pnv-d510 


Build changes
-------------

    * Linux: CI_DRM_5250 -> Patchwork_11005

  CI_DRM_5250: 1e4e49c57969d1b53dea913c92e1d23ec23aee31 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4736: 285ebfb3b7adc56586031afa5150c4e5ad40c229 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_11005: 04ae594846618d6f387f50d5a0b00c4c445098fd @ git://anongit.freedesktop.org/gfx-ci/linux


== Linux commits ==

04ae59484661 drm/i915/gvt: Change KVMGT as self load module
837fca394cd3 drm/i915/gvt: remove unused parameter for hypervisor's host_exit call
8d9e260bf1d5 drm/i915/gvt: mandatory require hypervisor's host_init

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_11005/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 18+ messages in thread
* ✓ Fi.CI.IGT: success for Change KVMGT into self loadable module (rev2)
  2018-12-03  4:05 [PATCH v3 0/3] Change KVMGT into self loadable module Zhenyu Wang
                   ` (6 preceding siblings ...)
  2018-12-04  3:15 ` ✓ Fi.CI.BAT: success for Change KVMGT into self loadable module (rev2) Patchwork
@ 2018-12-04  4:47 ` Patchwork
  2018-12-06  8:55 ` ✓ Fi.CI.BAT: success for Change KVMGT into self loadable module (rev3) Patchwork
  2018-12-06 23:33 ` ✓ Fi.CI.IGT: " Patchwork
  9 siblings, 0 replies; 18+ messages in thread
From: Patchwork @ 2018-12-04  4:47 UTC (permalink / raw)
  To: Zhenyu Wang; +Cc: intel-gfx

== Series Details ==

Series: Change KVMGT into self loadable module (rev2)
URL   : https://patchwork.freedesktop.org/series/53379/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_5250_full -> Patchwork_11005_full
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

  

Known issues
------------

  Here are the changes found in Patchwork_11005_full that come from known issues:

### IGT changes ###

#### Issues hit ####

  * igt@gem_exec_schedule@pi-ringfull-vebox:
    - shard-apl:          NOTRUN -> FAIL [fdo#103158]

  * igt@i915_suspend@fence-restore-untiled:
    - shard-skl:          PASS -> INCOMPLETE [fdo#104108] / [fdo#107773]

  * igt@kms_busy@extended-modeset-hang-newfb-with-reset-render-c:
    - shard-hsw:          PASS -> DMESG-WARN [fdo#107956]

  * igt@kms_busy@extended-pageflip-modeset-hang-oldfb-render-a:
    - shard-glk:          PASS -> DMESG-WARN [fdo#107956]

  * igt@kms_busy@extended-pageflip-modeset-hang-oldfb-render-c:
    - shard-skl:          NOTRUN -> DMESG-WARN [fdo#107956]

  * igt@kms_cursor_crc@cursor-256x256-suspend:
    - shard-skl:          PASS -> INCOMPLETE [fdo#104108]

  * igt@kms_cursor_crc@cursor-size-change:
    - shard-glk:          PASS -> FAIL [fdo#103232] +2

  * igt@kms_flip@flip-vs-expired-vblank-interruptible:
    - shard-skl:          PASS -> FAIL [fdo#105363]

  * igt@kms_frontbuffer_tracking@fbc-1p-primscrn-cur-indfb-onoff:
    - shard-apl:          PASS -> FAIL [fdo#103167]

  * igt@kms_frontbuffer_tracking@fbc-2p-primscrn-cur-indfb-move:
    - shard-glk:          PASS -> FAIL [fdo#103167] +3

  * igt@kms_frontbuffer_tracking@fbcpsr-stridechange:
    - shard-skl:          NOTRUN -> FAIL [fdo#105683]

  * igt@kms_frontbuffer_tracking@fbcpsr-suspend:
    - shard-skl:          PASS -> INCOMPLETE [fdo#104108] / [fdo#106978] / [fdo#107773]

  * igt@kms_plane@pixel-format-pipe-a-planes:
    - shard-skl:          NOTRUN -> DMESG-WARN [fdo#106885]

  * igt@kms_plane@plane-position-covered-pipe-b-planes:
    - shard-glk:          PASS -> FAIL [fdo#103166]

  * igt@kms_plane_alpha_blend@pipe-a-alpha-transparant-fb:
    - shard-skl:          NOTRUN -> FAIL [fdo#108145] +1

  * igt@kms_plane_alpha_blend@pipe-a-constant-alpha-max:
    - shard-apl:          NOTRUN -> FAIL [fdo#108145]

  * igt@kms_properties@connector-properties-atomic:
    - shard-skl:          NOTRUN -> FAIL [fdo#108642]

  * igt@kms_vblank@pipe-c-wait-busy:
    - shard-apl:          PASS -> DMESG-WARN [fdo#103558] / [fdo#105602] +1

  
#### Possible fixes ####

  * igt@gem_eio@reset-stress:
    - shard-glk:          FAIL [fdo#107799] -> PASS

  * igt@gem_exec_schedule@preempt-queue-chain-render:
    - shard-apl:          INCOMPLETE [fdo#103927] -> PASS

  * igt@kms_cursor_crc@cursor-128x128-suspend:
    - shard-skl:          INCOMPLETE [fdo#104108] -> PASS

  * igt@kms_cursor_crc@cursor-64x21-sliding:
    - shard-apl:          FAIL [fdo#103232] -> PASS

  * igt@kms_cursor_legacy@2x-long-flip-vs-cursor-legacy:
    - shard-glk:          FAIL [fdo#104873] -> PASS

  * igt@kms_flip@flip-vs-expired-vblank:
    - shard-skl:          FAIL [fdo#105363] -> PASS

  * igt@kms_frontbuffer_tracking@fbc-1p-primscrn-cur-indfb-draw-blt:
    - shard-glk:          FAIL [fdo#103167] -> PASS

  * igt@kms_frontbuffer_tracking@fbc-1p-primscrn-spr-indfb-draw-mmap-gtt:
    - shard-apl:          FAIL [fdo#103167] -> PASS

  * igt@kms_plane@plane-position-covered-pipe-b-planes:
    - shard-apl:          FAIL [fdo#103166] -> PASS

  * igt@kms_setmode@basic:
    - shard-kbl:          FAIL [fdo#99912] -> PASS

  * igt@pm_rpm@cursor-dpms:
    - shard-skl:          INCOMPLETE [fdo#107807] -> PASS

  * igt@pm_rpm@gem-execbuf-stress-extra-wait:
    - shard-skl:          INCOMPLETE [fdo#107803] / [fdo#107807] -> PASS

  * igt@pm_rpm@reg-read-ioctl:
    - {shard-iclb}:       DMESG-WARN [fdo#107724] -> PASS

  
#### Warnings ####

  * igt@kms_color@pipe-c-legacy-gamma:
    - shard-apl:          FAIL [fdo#104782] -> DMESG-FAIL [fdo#103558] / [fdo#105602]

  * igt@kms_plane_alpha_blend@pipe-c-alpha-7efc:
    - shard-apl:          FAIL [fdo#108145] -> DMESG-FAIL [fdo#103558] / [fdo#105602] / [fdo#108145] +1

  
  {name}: This element is suppressed. This means it is ignored when computing
          the status of the difference (SUCCESS, WARNING, or FAILURE).

  [fdo#103158]: https://bugs.freedesktop.org/show_bug.cgi?id=103158
  [fdo#103166]: https://bugs.freedesktop.org/show_bug.cgi?id=103166
  [fdo#103167]: https://bugs.freedesktop.org/show_bug.cgi?id=103167
  [fdo#103232]: https://bugs.freedesktop.org/show_bug.cgi?id=103232
  [fdo#103558]: https://bugs.freedesktop.org/show_bug.cgi?id=103558
  [fdo#103927]: https://bugs.freedesktop.org/show_bug.cgi?id=103927
  [fdo#104108]: https://bugs.freedesktop.org/show_bug.cgi?id=104108
  [fdo#104782]: https://bugs.freedesktop.org/show_bug.cgi?id=104782
  [fdo#104873]: https://bugs.freedesktop.org/show_bug.cgi?id=104873
  [fdo#105363]: https://bugs.freedesktop.org/show_bug.cgi?id=105363
  [fdo#105602]: https://bugs.freedesktop.org/show_bug.cgi?id=105602
  [fdo#105683]: https://bugs.freedesktop.org/show_bug.cgi?id=105683
  [fdo#106885]: https://bugs.freedesktop.org/show_bug.cgi?id=106885
  [fdo#106978]: https://bugs.freedesktop.org/show_bug.cgi?id=106978
  [fdo#107724]: https://bugs.freedesktop.org/show_bug.cgi?id=107724
  [fdo#107773]: https://bugs.freedesktop.org/show_bug.cgi?id=107773
  [fdo#107799]: https://bugs.freedesktop.org/show_bug.cgi?id=107799
  [fdo#107803]: https://bugs.freedesktop.org/show_bug.cgi?id=107803
  [fdo#107807]: https://bugs.freedesktop.org/show_bug.cgi?id=107807
  [fdo#107956]: https://bugs.freedesktop.org/show_bug.cgi?id=107956
  [fdo#108145]: https://bugs.freedesktop.org/show_bug.cgi?id=108145
  [fdo#108642]: https://bugs.freedesktop.org/show_bug.cgi?id=108642
  [fdo#99912]: https://bugs.freedesktop.org/show_bug.cgi?id=99912


Participating hosts (7 -> 7)
------------------------------

  No changes in participating hosts


Build changes
-------------

    * Linux: CI_DRM_5250 -> Patchwork_11005

  CI_DRM_5250: 1e4e49c57969d1b53dea913c92e1d23ec23aee31 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4736: 285ebfb3b7adc56586031afa5150c4e5ad40c229 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_11005: 04ae594846618d6f387f50d5a0b00c4c445098fd @ git://anongit.freedesktop.org/gfx-ci/linux
  piglit_4509: fdc5a4ca11124ab8413c7988896eec4c97336694 @ git://anongit.freedesktop.org/piglit

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_11005/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 18+ messages in thread
* Re: [PATCH v4] drm/i915/gvt: Change KVMGT as self load module
  2018-12-04  2:40   ` [PATCH v4] " Zhenyu Wang
@ 2018-12-06  4:27     ` Zhenyu Wang
  2018-12-07  7:47       ` He, Min
  2018-12-06  7:17     ` Yuan, Hang
  2018-12-06  8:02     ` [PATCH v5] " Zhenyu Wang
  2 siblings, 1 reply; 18+ messages in thread
From: Zhenyu Wang @ 2018-12-06  4:27 UTC (permalink / raw)
  To: Yuan, Hang, Zhi Wang, Xiong Zhang, He, Min; +Cc: intel-gfx, intel-gvt-dev


[-- Attachment #1.1: Type: text/plain, Size: 13106 bytes --]


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

On 2018.12.04 10:40:28 +0800, Zhenyu Wang wrote:
> 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;
> +	}
> +	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);
> diff --git a/drivers/gpu/drm/i915/gvt/gvt.h b/drivers/gpu/drm/i915/gvt/gvt.h
> index b4ab1dad0143..8a4cf995d755 100644
> --- a/drivers/gpu/drm/i915/gvt/gvt.h
> +++ b/drivers/gpu/drm/i915/gvt/gvt.h
> @@ -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;
> diff --git a/drivers/gpu/drm/i915/gvt/hypercall.h b/drivers/gpu/drm/i915/gvt/hypercall.h
> index e49a9247ed78..50798868ab15 100644
> --- a/drivers/gpu/drm/i915/gvt/hypercall.h
> +++ b/drivers/gpu/drm/i915/gvt/hypercall.h
> @@ -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_ */
> diff --git a/drivers/gpu/drm/i915/gvt/kvmgt.c b/drivers/gpu/drm/i915/gvt/kvmgt.c
> index 1bbd04d30c42..ef248d577e49 100644
> --- a/drivers/gpu/drm/i915/gvt/kvmgt.c
> +++ b/drivers/gpu/drm/i915/gvt/kvmgt.c
> @@ -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);
> diff --git a/drivers/gpu/drm/i915/gvt/mpt.h b/drivers/gpu/drm/i915/gvt/mpt.h
> index c95ef77da62c..9b4225d44243 100644
> --- a/drivers/gpu/drm/i915/gvt/mpt.h
> +++ b/drivers/gpu/drm/i915/gvt/mpt.h
> @@ -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_ */
> diff --git a/drivers/gpu/drm/i915/intel_gvt.c b/drivers/gpu/drm/i915/intel_gvt.c
> index c22b3e18a0f5..d74e59e22c9d 100644
> --- a/drivers/gpu/drm/i915/intel_gvt.c
> +++ b/drivers/gpu/drm/i915/intel_gvt.c
> @@ -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
> 
> _______________________________________________
> intel-gvt-dev mailing list
> intel-gvt-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gvt-dev

-- 
Open Source Technology Center, Intel ltd.

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

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

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 18+ messages in thread
* Re: [PATCH v4] drm/i915/gvt: Change KVMGT as self load module
  2018-12-04  2:40   ` [PATCH v4] " Zhenyu Wang
  2018-12-06  4:27     ` Zhenyu Wang
@ 2018-12-06  7:17     ` Yuan, Hang
  2018-12-06  8:02     ` [PATCH v5] " Zhenyu Wang
  2 siblings, 0 replies; 18+ messages in thread
From: Yuan, Hang @ 2018-12-06  7:17 UTC (permalink / raw)
  To: Zhenyu Wang, intel-gfx; +Cc: intel-gvt-dev

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

^ permalink raw reply	[flat|nested] 18+ messages in thread
* [PATCH v5] drm/i915/gvt: Change KVMGT as self load module
  2018-12-04  2:40   ` [PATCH v4] " Zhenyu Wang
  2018-12-06  4:27     ` Zhenyu Wang
  2018-12-06  7:17     ` Yuan, Hang
@ 2018-12-06  8:02     ` Zhenyu Wang
  2018-12-06  9:31       ` Yuan, Hang
  2 siblings, 1 reply; 18+ messages in thread
From: Zhenyu Wang @ 2018-12-06  8:02 UTC (permalink / raw)
  To: intel-gfx; +Cc: intel-gvt-dev, Yuan, Hang

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" <hang.yuan@intel.com>
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       | 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(-)

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..4e8947f33bd0 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,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]);
+		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);
diff --git a/drivers/gpu/drm/i915/gvt/gvt.h b/drivers/gpu/drm/i915/gvt/gvt.h
index b4ab1dad0143..8a4cf995d755 100644
--- a/drivers/gpu/drm/i915/gvt/gvt.h
+++ b/drivers/gpu/drm/i915/gvt/gvt.h
@@ -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;
diff --git a/drivers/gpu/drm/i915/gvt/hypercall.h b/drivers/gpu/drm/i915/gvt/hypercall.h
index e49a9247ed78..50798868ab15 100644
--- a/drivers/gpu/drm/i915/gvt/hypercall.h
+++ b/drivers/gpu/drm/i915/gvt/hypercall.h
@@ -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_ */
diff --git a/drivers/gpu/drm/i915/gvt/kvmgt.c b/drivers/gpu/drm/i915/gvt/kvmgt.c
index 1bbd04d30c42..ef248d577e49 100644
--- a/drivers/gpu/drm/i915/gvt/kvmgt.c
+++ b/drivers/gpu/drm/i915/gvt/kvmgt.c
@@ -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);
diff --git a/drivers/gpu/drm/i915/gvt/mpt.h b/drivers/gpu/drm/i915/gvt/mpt.h
index c95ef77da62c..9b4225d44243 100644
--- a/drivers/gpu/drm/i915/gvt/mpt.h
+++ b/drivers/gpu/drm/i915/gvt/mpt.h
@@ -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_ */
diff --git a/drivers/gpu/drm/i915/intel_gvt.c b/drivers/gpu/drm/i915/intel_gvt.c
index c22b3e18a0f5..d74e59e22c9d 100644
--- a/drivers/gpu/drm/i915/intel_gvt.c
+++ b/drivers/gpu/drm/i915/intel_gvt.c
@@ -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

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related	[flat|nested] 18+ messages in thread
* ✓ Fi.CI.BAT: success for Change KVMGT into self loadable module (rev3)
  2018-12-03  4:05 [PATCH v3 0/3] Change KVMGT into self loadable module Zhenyu Wang
                   ` (7 preceding siblings ...)
  2018-12-04  4:47 ` ✓ Fi.CI.IGT: " Patchwork
@ 2018-12-06  8:55 ` Patchwork
  2018-12-06 23:33 ` ✓ Fi.CI.IGT: " Patchwork
  9 siblings, 0 replies; 18+ messages in thread
From: Patchwork @ 2018-12-06  8:55 UTC (permalink / raw)
  To: Zhenyu Wang; +Cc: intel-gfx

== Series Details ==

Series: Change KVMGT into self loadable module (rev3)
URL   : https://patchwork.freedesktop.org/series/53379/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_5272 -> Patchwork_11030
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

  External URL: https://patchwork.freedesktop.org/api/1.0/series/53379/revisions/3/mbox/

Known issues
------------

  Here are the changes found in Patchwork_11030 that come from known issues:

### IGT changes ###

#### Issues hit ####

  * {igt@runner@aborted}:
    - {fi-icl-u3}:        NOTRUN -> FAIL [fdo#108315]

  
#### Possible fixes ####

  * igt@gem_ctx_create@basic-files:
    - fi-bsw-kefka:       FAIL [fdo#108656] -> PASS

  * igt@gem_exec_suspend@basic-s4-devices:
    - fi-ivb-3520m:       FAIL [fdo#108880] -> PASS

  
#### Warnings ####

  * igt@i915_selftest@live_contexts:
    - {fi-icl-u3}:        DMESG-FAIL [fdo#108569] -> INCOMPLETE [fdo#108315]

  
  {name}: This element is suppressed. This means it is ignored when computing
          the status of the difference (SUCCESS, WARNING, or FAILURE).

  [fdo#108315]: https://bugs.freedesktop.org/show_bug.cgi?id=108315
  [fdo#108569]: https://bugs.freedesktop.org/show_bug.cgi?id=108569
  [fdo#108656]: https://bugs.freedesktop.org/show_bug.cgi?id=108656
  [fdo#108880]: https://bugs.freedesktop.org/show_bug.cgi?id=108880


Participating hosts (48 -> 45)
------------------------------

  Additional (1): fi-skl-guc 
  Missing    (4): fi-ilk-m540 fi-byt-squawks fi-bsw-cyan fi-hsw-4200u 


Build changes
-------------

    * Linux: CI_DRM_5272 -> Patchwork_11030

  CI_DRM_5272: 4bb8baa3a7b836ce18e1b27ba12bae2130ee38cc @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4743: edb2db2cf2b6665d7ba3fa9117263302f6307a4f @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_11030: 66ee14434e7534c0aa38e16f08a89d910461716d @ git://anongit.freedesktop.org/gfx-ci/linux


== Linux commits ==

66ee14434e75 drm/i915/gvt: Change KVMGT as self load module
d5d9d337bd09 drm/i915/gvt: remove unused parameter for hypervisor's host_exit call
e4458706d879 drm/i915/gvt: mandatory require hypervisor's host_init

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_11030/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 18+ messages in thread
* Re: [PATCH v5] drm/i915/gvt: Change KVMGT as self load module
  2018-12-06  8:02     ` [PATCH v5] " Zhenyu Wang
@ 2018-12-06  9:31       ` Yuan, Hang
  0 siblings, 0 replies; 18+ messages in thread
From: Yuan, Hang @ 2018-12-06  9:31 UTC (permalink / raw)
  To: Zhenyu Wang, intel-gfx; +Cc: intel-gvt-dev

Looks good to me. Thanks.

Reviewed-by: Yuan, Hang <hang.yuan@intel.com>

> -----Original Message-----
> From: Zhenyu Wang [mailto:zhenyuw@linux.intel.com]
> Sent: Thursday, December 6, 2018 4:03 PM
> To: intel-gfx@lists.freedesktop.org
> Cc: intel-gvt-dev@lists.freedesktop.org; Yuan, Hang <hang.yuan@intel.com>;
> Alex Williamson <alex.williamson@redhat.com>
> 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" <hang.yuan@intel.com>
> 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       | 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(-)
> 
> 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..4e8947f33bd0 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,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]);
> +		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);
> diff --git a/drivers/gpu/drm/i915/gvt/gvt.h b/drivers/gpu/drm/i915/gvt/gvt.h
> index b4ab1dad0143..8a4cf995d755 100644
> --- a/drivers/gpu/drm/i915/gvt/gvt.h
> +++ b/drivers/gpu/drm/i915/gvt/gvt.h
> @@ -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;
> diff --git a/drivers/gpu/drm/i915/gvt/hypercall.h
> b/drivers/gpu/drm/i915/gvt/hypercall.h
> index e49a9247ed78..50798868ab15 100644
> --- a/drivers/gpu/drm/i915/gvt/hypercall.h
> +++ b/drivers/gpu/drm/i915/gvt/hypercall.h
> @@ -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_ */
> diff --git a/drivers/gpu/drm/i915/gvt/kvmgt.c
> b/drivers/gpu/drm/i915/gvt/kvmgt.c
> index 1bbd04d30c42..ef248d577e49 100644
> --- a/drivers/gpu/drm/i915/gvt/kvmgt.c
> +++ b/drivers/gpu/drm/i915/gvt/kvmgt.c
> @@ -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);
> diff --git a/drivers/gpu/drm/i915/gvt/mpt.h
> b/drivers/gpu/drm/i915/gvt/mpt.h index c95ef77da62c..9b4225d44243
> 100644
> --- a/drivers/gpu/drm/i915/gvt/mpt.h
> +++ b/drivers/gpu/drm/i915/gvt/mpt.h
> @@ -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_ */
> diff --git a/drivers/gpu/drm/i915/intel_gvt.c
> b/drivers/gpu/drm/i915/intel_gvt.c
> index c22b3e18a0f5..d74e59e22c9d 100644
> --- a/drivers/gpu/drm/i915/intel_gvt.c
> +++ b/drivers/gpu/drm/i915/intel_gvt.c
> @@ -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

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 18+ messages in thread
* ✓ Fi.CI.IGT: success for Change KVMGT into self loadable module (rev3)
  2018-12-03  4:05 [PATCH v3 0/3] Change KVMGT into self loadable module Zhenyu Wang
                   ` (8 preceding siblings ...)
  2018-12-06  8:55 ` ✓ Fi.CI.BAT: success for Change KVMGT into self loadable module (rev3) Patchwork
@ 2018-12-06 23:33 ` Patchwork
  9 siblings, 0 replies; 18+ messages in thread
From: Patchwork @ 2018-12-06 23:33 UTC (permalink / raw)
  To: Zhenyu Wang; +Cc: intel-gfx

== Series Details ==

Series: Change KVMGT into self loadable module (rev3)
URL   : https://patchwork.freedesktop.org/series/53379/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_5272_full -> Patchwork_11030_full
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

  

Known issues
------------

  Here are the changes found in Patchwork_11030_full that come from known issues:

### IGT changes ###

#### Issues hit ####

  * igt@drm_read@short-buffer-nonblock:
    - shard-hsw:          PASS -> DMESG-WARN [fdo#102614] +1

  * igt@gem_ppgtt@blt-vs-render-ctx0:
    - shard-skl:          NOTRUN -> TIMEOUT [fdo#108039]

  * igt@i915_suspend@fence-restore-tiled2untiled:
    - shard-skl:          PASS -> INCOMPLETE [fdo#104108] / [fdo#107773]

  * igt@kms_busy@extended-modeset-hang-newfb-render-c:
    - shard-skl:          NOTRUN -> DMESG-WARN [fdo#107956] +1

  * igt@kms_busy@extended-pageflip-modeset-hang-oldfb-render-a:
    - shard-apl:          NOTRUN -> DMESG-WARN [fdo#107956]

  * igt@kms_chv_cursor_fail@pipe-c-128x128-left-edge:
    - shard-skl:          NOTRUN -> FAIL [fdo#104671]

  * igt@kms_cursor_crc@cursor-128x128-onscreen:
    - shard-skl:          NOTRUN -> FAIL [fdo#103232]

  * igt@kms_cursor_crc@cursor-128x128-sliding:
    - shard-apl:          PASS -> FAIL [fdo#103232] +2

  * igt@kms_cursor_crc@cursor-64x64-suspend:
    - shard-apl:          PASS -> FAIL [fdo#103191] / [fdo#103232]

  * igt@kms_draw_crc@draw-method-xrgb8888-mmap-gtt-xtiled:
    - {shard-iclb}:       PASS -> WARN [fdo#108336]

  * igt@kms_frontbuffer_tracking@fbc-1p-primscrn-pri-indfb-draw-mmap-wc:
    - {shard-iclb}:       PASS -> DMESG-FAIL [fdo#107724]

  * igt@kms_panel_fitting@legacy:
    - shard-skl:          NOTRUN -> FAIL [fdo#105456]

  * {igt@kms_plane@pixel-format-pipe-c-planes-source-clamping}:
    - shard-apl:          PASS -> FAIL [fdo#108948]
    - shard-skl:          NOTRUN -> DMESG-WARN [fdo#106885] +1

  * igt@kms_plane_alpha_blend@pipe-a-constant-alpha-max:
    - shard-skl:          NOTRUN -> FAIL [fdo#108145] +3

  * igt@kms_plane_multiple@atomic-pipe-b-tiling-y:
    - {shard-iclb}:       PASS -> FAIL [fdo#103166] +1

  * igt@kms_vblank@pipe-b-ts-continuation-idle-hang:
    - {shard-iclb}:       PASS -> DMESG-WARN [fdo#107724] +5

  * igt@pm_rpm@basic-pci-d3-state:
    - shard-skl:          PASS -> INCOMPLETE [fdo#107807]

  * igt@pm_rpm@legacy-planes-dpms:
    - {shard-iclb}:       PASS -> INCOMPLETE [fdo#108840]

  
#### Possible fixes ####

  * igt@kms_atomic_transition@1x-modeset-transitions-nonblocking-fencing:
    - shard-skl:          FAIL [fdo#107815] / [fdo#108470] -> PASS

  * igt@kms_atomic_transition@plane-all-modeset-transition-internal-panels:
    - {shard-iclb}:       DMESG-WARN [fdo#107724] -> PASS +5

  * igt@kms_color@pipe-c-legacy-gamma:
    - shard-apl:          FAIL [fdo#104782] -> PASS

  * igt@kms_cursor_crc@cursor-256x85-offscreen:
    - shard-skl:          FAIL [fdo#103232] -> PASS

  * igt@kms_cursor_legacy@all-pipes-single-move:
    - shard-hsw:          DMESG-WARN [fdo#102614] -> PASS

  * igt@kms_flip@2x-plain-flip-fb-recreate-interruptible:
    - shard-hsw:          DMESG-FAIL [fdo#102614] -> PASS

  * igt@kms_flip@flip-vs-expired-vblank:
    - shard-apl:          FAIL [fdo#102887] / [fdo#105363] -> PASS

  * igt@kms_frontbuffer_tracking@fbc-stridechange:
    - {shard-iclb}:       FAIL [fdo#105683] / [fdo#108040] -> PASS

  * igt@kms_frontbuffer_tracking@fbcpsr-1p-offscren-pri-indfb-draw-pwrite:
    - {shard-iclb}:       DMESG-FAIL [fdo#107724] -> PASS +3

  * igt@kms_frontbuffer_tracking@psr-1p-primscrn-cur-indfb-draw-mmap-wc:
    - {shard-iclb}:       DMESG-WARN [fdo#107724] / [fdo#108336] -> PASS +5

  * igt@kms_frontbuffer_tracking@psr-1p-primscrn-spr-indfb-move:
    - {shard-iclb}:       FAIL [fdo#103167] -> PASS +4

  * {igt@kms_plane@pixel-format-pipe-c-planes-source-clamping}:
    - shard-glk:          FAIL [fdo#108948] -> PASS

  * igt@kms_plane_alpha_blend@pipe-a-alpha-opaque-fb:
    - shard-apl:          FAIL [fdo#108145] -> PASS

  * igt@kms_plane_alpha_blend@pipe-b-constant-alpha-max:
    - shard-glk:          FAIL [fdo#108145] -> PASS

  * igt@kms_plane_multiple@atomic-pipe-a-tiling-y:
    - {shard-iclb}:       FAIL [fdo#103166] -> PASS +2

  * igt@kms_plane_multiple@atomic-pipe-c-tiling-y:
    - shard-apl:          FAIL [fdo#103166] -> PASS

  * igt@kms_plane_multiple@atomic-pipe-c-tiling-yf:
    - shard-glk:          FAIL [fdo#103166] -> PASS

  * igt@kms_vblank@pipe-b-ts-continuation-dpms-suspend:
    - {shard-iclb}:       FAIL [fdo#103375] -> PASS +1

  * igt@kms_vblank@pipe-c-ts-continuation-suspend:
    - shard-kbl:          INCOMPLETE [fdo#103665] -> PASS

  * igt@pm_rpm@gem-pread:
    - shard-skl:          INCOMPLETE [fdo#107807] -> PASS +1

  * igt@pm_rpm@legacy-planes:
    - {shard-iclb}:       DMESG-WARN -> PASS

  * igt@pm_rpm@modeset-non-lpsp-stress-no-wait:
    - shard-skl:          INCOMPLETE [fdo#107807] -> SKIP

  
#### Warnings ####

  * igt@i915_selftest@live_contexts:
    - {shard-iclb}:       DMESG-FAIL [fdo#108569] -> INCOMPLETE [fdo#108315]

  * igt@kms_content_protection@legacy:
    - shard-apl:          INCOMPLETE [fdo#103927] -> FAIL [fdo#108597]

  * igt@kms_cursor_crc@cursor-64x64-suspend:
    - {shard-iclb}:       FAIL [fdo#103232] -> DMESG-FAIL [fdo#103232] / [fdo#107724]

  * {igt@kms_plane@pixel-format-pipe-b-planes-source-clamping}:
    - {shard-iclb}:       DMESG-WARN [fdo#107724] / [fdo#108336] -> FAIL [fdo#108948]

  * igt@kms_plane_multiple@atomic-pipe-b-tiling-x:
    - {shard-iclb}:       FAIL [fdo#103166] -> DMESG-WARN [fdo#107724] / [fdo#108336]

  
  {name}: This element is suppressed. This means it is ignored when computing
          the status of the difference (SUCCESS, WARNING, or FAILURE).

  [fdo#102614]: https://bugs.freedesktop.org/show_bug.cgi?id=102614
  [fdo#102887]: https://bugs.freedesktop.org/show_bug.cgi?id=102887
  [fdo#103166]: https://bugs.freedesktop.org/show_bug.cgi?id=103166
  [fdo#103167]: https://bugs.freedesktop.org/show_bug.cgi?id=103167
  [fdo#103191]: https://bugs.freedesktop.org/show_bug.cgi?id=103191
  [fdo#103232]: https://bugs.freedesktop.org/show_bug.cgi?id=103232
  [fdo#103375]: https://bugs.freedesktop.org/show_bug.cgi?id=103375
  [fdo#103665]: https://bugs.freedesktop.org/show_bug.cgi?id=103665
  [fdo#103927]: https://bugs.freedesktop.org/show_bug.cgi?id=103927
  [fdo#104108]: https://bugs.freedesktop.org/show_bug.cgi?id=104108
  [fdo#104671]: https://bugs.freedesktop.org/show_bug.cgi?id=104671
  [fdo#104782]: https://bugs.freedesktop.org/show_bug.cgi?id=104782
  [fdo#105363]: https://bugs.freedesktop.org/show_bug.cgi?id=105363
  [fdo#105456]: https://bugs.freedesktop.org/show_bug.cgi?id=105456
  [fdo#105683]: https://bugs.freedesktop.org/show_bug.cgi?id=105683
  [fdo#106885]: https://bugs.freedesktop.org/show_bug.cgi?id=106885
  [fdo#107724]: https://bugs.freedesktop.org/show_bug.cgi?id=107724
  [fdo#107773]: https://bugs.freedesktop.org/show_bug.cgi?id=107773
  [fdo#107807]: https://bugs.freedesktop.org/show_bug.cgi?id=107807
  [fdo#107815]: https://bugs.freedesktop.org/show_bug.cgi?id=107815
  [fdo#107956]: https://bugs.freedesktop.org/show_bug.cgi?id=107956
  [fdo#108039]: https://bugs.freedesktop.org/show_bug.cgi?id=108039
  [fdo#108040]: https://bugs.freedesktop.org/show_bug.cgi?id=108040
  [fdo#108145]: https://bugs.freedesktop.org/show_bug.cgi?id=108145
  [fdo#108315]: https://bugs.freedesktop.org/show_bug.cgi?id=108315
  [fdo#108336]: https://bugs.freedesktop.org/show_bug.cgi?id=108336
  [fdo#108470]: https://bugs.freedesktop.org/show_bug.cgi?id=108470
  [fdo#108569]: https://bugs.freedesktop.org/show_bug.cgi?id=108569
  [fdo#108597]: https://bugs.freedesktop.org/show_bug.cgi?id=108597
  [fdo#108840]: https://bugs.freedesktop.org/show_bug.cgi?id=108840
  [fdo#108948]: https://bugs.freedesktop.org/show_bug.cgi?id=108948


Participating hosts (7 -> 7)
------------------------------

  No changes in participating hosts


Build changes
-------------

    * Linux: CI_DRM_5272 -> Patchwork_11030

  CI_DRM_5272: 4bb8baa3a7b836ce18e1b27ba12bae2130ee38cc @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4743: edb2db2cf2b6665d7ba3fa9117263302f6307a4f @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_11030: 66ee14434e7534c0aa38e16f08a89d910461716d @ git://anongit.freedesktop.org/gfx-ci/linux
  piglit_4509: fdc5a4ca11124ab8413c7988896eec4c97336694 @ git://anongit.freedesktop.org/piglit

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_11030/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 18+ messages in thread
* Re: [PATCH v4] drm/i915/gvt: Change KVMGT as self load module
  2018-12-06  4:27     ` Zhenyu Wang
@ 2018-12-07  7:47       ` He, Min
  2018-12-07  7:50         ` Zhenyu Wang
  0 siblings, 1 reply; 18+ messages in thread
From: He, Min @ 2018-12-07  7:47 UTC (permalink / raw)
  To: Zhenyu Wang, Yuan, Hang, Wang, Zhi A, Zhang, Xiong Y
  Cc: intel-gfx, intel-gvt-dev

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. 

> -----Original Message-----
> From: Zhenyu Wang [mailto:zhenyuw@linux.intel.com]
> Sent: Thursday, December 6, 2018 12:28 PM
> To: Yuan, Hang <hang.yuan@intel.com>; Wang, Zhi A
> <zhi.a.wang@intel.com>; Zhang, Xiong Y <xiong.y.zhang@intel.com>; He,
> Min <min.he@intel.com>
> Cc: intel-gfx@lists.freedesktop.org; Alex Williamson
> <alex.williamson@redhat.com>; intel-gvt-dev@lists.freedesktop.org
> 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
> 
> On 2018.12.04 10:40:28 +0800, Zhenyu Wang wrote:
> > 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);
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.

> > +	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);
> > diff --git a/drivers/gpu/drm/i915/gvt/gvt.h
> b/drivers/gpu/drm/i915/gvt/gvt.h
> > index b4ab1dad0143..8a4cf995d755 100644
> > --- a/drivers/gpu/drm/i915/gvt/gvt.h
> > +++ b/drivers/gpu/drm/i915/gvt/gvt.h
> > @@ -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;
> > diff --git a/drivers/gpu/drm/i915/gvt/hypercall.h
> b/drivers/gpu/drm/i915/gvt/hypercall.h
> > index e49a9247ed78..50798868ab15 100644
> > --- a/drivers/gpu/drm/i915/gvt/hypercall.h
> > +++ b/drivers/gpu/drm/i915/gvt/hypercall.h
> > @@ -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;
We can remove this definition, too. 

> > -extern struct intel_gvt_mpt kvmgt_mpt;
> >
> >  #endif /* _GVT_HYPERCALL_H_ */
> > diff --git a/drivers/gpu/drm/i915/gvt/kvmgt.c
> b/drivers/gpu/drm/i915/gvt/kvmgt.c
> > index 1bbd04d30c42..ef248d577e49 100644
> > --- a/drivers/gpu/drm/i915/gvt/kvmgt.c
> > +++ b/drivers/gpu/drm/i915/gvt/kvmgt.c
> > @@ -50,6 +50,7 @@
> >  #include "gvt.h"
> >
> >  static const struct intel_gvt_ops *intel_gvt_ops;
> > +static struct intel_gvt *intel_gvt;
This variable intel_gvt seems useless.

> >
> >  /* 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))
Unrelated to this patch.

> >  		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);
> > diff --git a/drivers/gpu/drm/i915/gvt/mpt.h
> b/drivers/gpu/drm/i915/gvt/mpt.h
> > index c95ef77da62c..9b4225d44243 100644
> > --- a/drivers/gpu/drm/i915/gvt/mpt.h
> > +++ b/drivers/gpu/drm/i915/gvt/mpt.h
> > @@ -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_ */
> > diff --git a/drivers/gpu/drm/i915/intel_gvt.c
> b/drivers/gpu/drm/i915/intel_gvt.c
> > index c22b3e18a0f5..d74e59e22c9d 100644
> > --- a/drivers/gpu/drm/i915/intel_gvt.c
> > +++ b/drivers/gpu/drm/i915/intel_gvt.c
> > @@ -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
> >
> > _______________________________________________
> > intel-gvt-dev mailing list
> > intel-gvt-dev@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/intel-gvt-dev
> 
> --
> Open Source Technology Center, Intel ltd.
> 
> $gpg --keyserver wwwkeys.pgp.net --recv-keys 4D781827
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 18+ messages in thread
* Re: [PATCH v4] drm/i915/gvt: Change KVMGT as self load module
  2018-12-07  7:47       ` He, Min
@ 2018-12-07  7:50         ` Zhenyu Wang
  0 siblings, 0 replies; 18+ messages in thread
From: Zhenyu Wang @ 2018-12-07  7:50 UTC (permalink / raw)
  To: He, Min; +Cc: intel-gfx, Yuan, Hang, intel-gvt-dev


[-- Attachment #1.1: Type: text/plain, Size: 2761 bytes --]

On 2018.12.07 07:47:59 +0000, He, Min wrote:
> 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. 
> 

Good!

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

I'd like to keep as in current approach, as it can keep hypervisor init
function in one place instead of passing gvt host info to hypervisor module.
And calling submodule's init function is fine process I think.

> > > @@ -67,6 +73,5 @@ struct intel_gvt_mpt {
> > >  };
> > >
> > >  extern struct intel_gvt_mpt xengt_mpt;
> We can remove this definition, too. 
>

yeah, but maybe in another xengt cleanup patch.

> > > diff --git a/drivers/gpu/drm/i915/gvt/kvmgt.c
> > b/drivers/gpu/drm/i915/gvt/kvmgt.c
> > > index 1bbd04d30c42..ef248d577e49 100644
> > > --- a/drivers/gpu/drm/i915/gvt/kvmgt.c
> > > +++ b/drivers/gpu/drm/i915/gvt/kvmgt.c
> > > @@ -50,6 +50,7 @@
> > >  #include "gvt.h"
> > >
> > >  static const struct intel_gvt_ops *intel_gvt_ops;
> > > +static struct intel_gvt *intel_gvt;
> This variable intel_gvt seems useless.
>

Thanks for pointing this out, was trying to use for exit path,
but looks I missed to remove it finally.

-- 
Open Source Technology Center, Intel ltd.

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

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

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 18+ messages in thread
end of thread, other threads:[~2018-12-07  7:50 UTC | newest]

Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-12-03  4:05 [PATCH v3 0/3] Change KVMGT into self loadable module Zhenyu Wang
2018-12-03  4:05 ` [PATCH v3 1/3] drm/i915/gvt: mandatory require hypervisor's host_init Zhenyu Wang
2018-12-03  4:05 ` [PATCH v3 2/3] drm/i915/gvt: remove unused parameter for hypervisor's host_exit call Zhenyu Wang
2018-12-03  4:05 ` [PATCH v3 3/3] drm/i915/gvt: Change KVMGT as self load module Zhenyu Wang
2018-12-04  2:40   ` [PATCH v4] " Zhenyu Wang
2018-12-06  4:27     ` Zhenyu Wang
2018-12-07  7:47       ` He, Min
2018-12-07  7:50         ` Zhenyu Wang
2018-12-06  7:17     ` Yuan, Hang
2018-12-06  8:02     ` [PATCH v5] " Zhenyu Wang
2018-12-06  9:31       ` Yuan, Hang
2018-12-03  4:14 ` ✗ Fi.CI.CHECKPATCH: warning for Change KVMGT into self loadable module Patchwork
2018-12-03  4:32 ` ✓ Fi.CI.BAT: success " Patchwork
2018-12-03  5:34 ` ✓ Fi.CI.IGT: " Patchwork
2018-12-04  3:15 ` ✓ Fi.CI.BAT: success for Change KVMGT into self loadable module (rev2) Patchwork
2018-12-04  4:47 ` ✓ Fi.CI.IGT: " Patchwork
2018-12-06  8:55 ` ✓ Fi.CI.BAT: success for Change KVMGT into self loadable module (rev3) Patchwork
2018-12-06 23:33 ` ✓ Fi.CI.IGT: " Patchwork

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.