Looks good to me. Thanks. Reviewed-by: Yuan, Hang <hang.yuan@xxxxxxxxx> > -----Original Message----- > From: Zhenyu Wang [mailto:zhenyuw@xxxxxxxxxxxxxxx] > Sent: Thursday, December 6, 2018 4:03 PM > To: intel-gfx@xxxxxxxxxxxxxxxxxxxxx > Cc: intel-gvt-dev@xxxxxxxxxxxxxxxxxxxxx; Yuan, Hang <hang.yuan@xxxxxxxxx>; > Alex Williamson <alex.williamson@xxxxxxxxxx> > 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@xxxxxxxxx> > Cc: Alex Williamson <alex.williamson@xxxxxxxxxx> > Signed-off-by: Zhenyu Wang <zhenyuw@xxxxxxxxxxxxxxx> > --- > 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@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx
