From feddf6e866c9cdbdec45b09f0a9566ea538a0da3 Mon Sep 17 00:00:00 2001 From: Zhenyu Wang Date: Thu, 20 Oct 2016 17:15:03 +0800 Subject: [PATCH 01/19] drm/i915/gvt: clean up intel_gvt.h as interface for i915 core i915 core should only call functions and structures exposed through intel_gvt.h. Remove internal gvt.h and i915_pvinfo.h. Change for internal intel_gvt structure as private handler which not requires to expose gvt internal structure for i915 core. v2: Fix per Chris's comment - carefully handle dev_priv->gvt assignment - add necessary bracket for macro helper - forward declartion struct intel_gvt - keep free operation within same file handling alloc v3: fix use after free and remove intel_gvt.initialized v4: change to_gvt() to an inline Reviewed-by: Chris Wilson Signed-off-by: Zhenyu Wang --- drivers/gpu/drm/i915/gvt/aperture_gm.c | 1 + drivers/gpu/drm/i915/gvt/cfg_space.c | 1 + drivers/gpu/drm/i915/gvt/cmd_parser.c | 2 ++ drivers/gpu/drm/i915/gvt/display.c | 1 + drivers/gpu/drm/i915/gvt/edid.c | 1 + drivers/gpu/drm/i915/gvt/execlist.c | 1 + drivers/gpu/drm/i915/gvt/firmware.c | 2 ++ drivers/gpu/drm/i915/gvt/gtt.c | 2 ++ drivers/gpu/drm/i915/gvt/gvt.c | 19 +++++++++++++------ drivers/gpu/drm/i915/gvt/gvt.h | 7 +++++-- drivers/gpu/drm/i915/gvt/handlers.c | 2 ++ drivers/gpu/drm/i915/gvt/interrupt.c | 1 + drivers/gpu/drm/i915/gvt/mmio.c | 1 + drivers/gpu/drm/i915/gvt/opregion.c | 1 + drivers/gpu/drm/i915/gvt/render.c | 1 + drivers/gpu/drm/i915/gvt/sched_policy.c | 1 + drivers/gpu/drm/i915/gvt/scheduler.c | 5 +++-- drivers/gpu/drm/i915/gvt/vgpu.c | 2 ++ drivers/gpu/drm/i915/i915_drv.h | 4 ++-- drivers/gpu/drm/i915/intel_gvt.h | 3 +-- 20 files changed, 44 insertions(+), 14 deletions(-) diff --git a/drivers/gpu/drm/i915/gvt/aperture_gm.c b/drivers/gpu/drm/i915/gvt/aperture_gm.c index e0211f83bd93b..db503c164b2fe 100644 --- a/drivers/gpu/drm/i915/gvt/aperture_gm.c +++ b/drivers/gpu/drm/i915/gvt/aperture_gm.c @@ -35,6 +35,7 @@ */ #include "i915_drv.h" +#include "gvt.h" #define MB_TO_BYTES(mb) ((mb) << 20ULL) #define BYTES_TO_MB(b) ((b) >> 20ULL) diff --git a/drivers/gpu/drm/i915/gvt/cfg_space.c b/drivers/gpu/drm/i915/gvt/cfg_space.c index 16360e449ed08..4c687740f5f11 100644 --- a/drivers/gpu/drm/i915/gvt/cfg_space.c +++ b/drivers/gpu/drm/i915/gvt/cfg_space.c @@ -32,6 +32,7 @@ */ #include "i915_drv.h" +#include "gvt.h" enum { INTEL_GVT_PCI_BAR_GTTMMIO = 0, diff --git a/drivers/gpu/drm/i915/gvt/cmd_parser.c b/drivers/gpu/drm/i915/gvt/cmd_parser.c index 5808ee7c19356..5b4658ff4d8ac 100644 --- a/drivers/gpu/drm/i915/gvt/cmd_parser.c +++ b/drivers/gpu/drm/i915/gvt/cmd_parser.c @@ -36,6 +36,8 @@ #include #include "i915_drv.h" +#include "gvt.h" +#include "i915_pvinfo.h" #include "trace.h" #define INVALID_OP (~0U) diff --git a/drivers/gpu/drm/i915/gvt/display.c b/drivers/gpu/drm/i915/gvt/display.c index 534000b916815..d8908d4cd09a4 100644 --- a/drivers/gpu/drm/i915/gvt/display.c +++ b/drivers/gpu/drm/i915/gvt/display.c @@ -33,6 +33,7 @@ */ #include "i915_drv.h" +#include "gvt.h" static int get_edp_pipe(struct intel_vgpu *vgpu) { diff --git a/drivers/gpu/drm/i915/gvt/edid.c b/drivers/gpu/drm/i915/gvt/edid.c index a07e4276126cc..7e1da1c563ca8 100644 --- a/drivers/gpu/drm/i915/gvt/edid.c +++ b/drivers/gpu/drm/i915/gvt/edid.c @@ -33,6 +33,7 @@ */ #include "i915_drv.h" +#include "gvt.h" #define GMBUS1_TOTAL_BYTES_SHIFT 16 #define GMBUS1_TOTAL_BYTES_MASK 0x1ff diff --git a/drivers/gpu/drm/i915/gvt/execlist.c b/drivers/gpu/drm/i915/gvt/execlist.c index c50a3d1a51314..b87b4f5e4c8f4 100644 --- a/drivers/gpu/drm/i915/gvt/execlist.c +++ b/drivers/gpu/drm/i915/gvt/execlist.c @@ -33,6 +33,7 @@ */ #include "i915_drv.h" +#include "gvt.h" #define _EL_OFFSET_STATUS 0x234 #define _EL_OFFSET_STATUS_BUF 0x370 diff --git a/drivers/gpu/drm/i915/gvt/firmware.c b/drivers/gpu/drm/i915/gvt/firmware.c index 4578a4d69a091..d068a524a6992 100644 --- a/drivers/gpu/drm/i915/gvt/firmware.c +++ b/drivers/gpu/drm/i915/gvt/firmware.c @@ -32,6 +32,8 @@ #include #include "i915_drv.h" +#include "gvt.h" +#include "i915_pvinfo.h" #define FIRMWARE_VERSION (0x0) diff --git a/drivers/gpu/drm/i915/gvt/gtt.c b/drivers/gpu/drm/i915/gvt/gtt.c index 29de179920e8b..0722d1e61fcea 100644 --- a/drivers/gpu/drm/i915/gvt/gtt.c +++ b/drivers/gpu/drm/i915/gvt/gtt.c @@ -34,6 +34,8 @@ */ #include "i915_drv.h" +#include "gvt.h" +#include "i915_pvinfo.h" #include "trace.h" static bool enable_out_of_sync = false; diff --git a/drivers/gpu/drm/i915/gvt/gvt.c b/drivers/gpu/drm/i915/gvt/gvt.c index e72e26c61a156..31b59d40f3fb4 100644 --- a/drivers/gpu/drm/i915/gvt/gvt.c +++ b/drivers/gpu/drm/i915/gvt/gvt.c @@ -35,6 +35,7 @@ #include #include "i915_drv.h" +#include "gvt.h" struct intel_gvt_host intel_gvt_host; @@ -173,9 +174,9 @@ static int init_service_thread(struct intel_gvt *gvt) */ void intel_gvt_clean_device(struct drm_i915_private *dev_priv) { - struct intel_gvt *gvt = &dev_priv->gvt; + struct intel_gvt *gvt = to_gvt(dev_priv); - if (WARN_ON(!gvt->initialized)) + if (WARN_ON(!gvt)) return; clean_service_thread(gvt); @@ -188,7 +189,8 @@ void intel_gvt_clean_device(struct drm_i915_private *dev_priv) intel_gvt_clean_mmio_info(gvt); intel_gvt_free_firmware(gvt); - gvt->initialized = false; + kfree(dev_priv->gvt); + dev_priv->gvt = NULL; } /** @@ -204,7 +206,7 @@ void intel_gvt_clean_device(struct drm_i915_private *dev_priv) */ int intel_gvt_init_device(struct drm_i915_private *dev_priv) { - struct intel_gvt *gvt = &dev_priv->gvt; + struct intel_gvt *gvt; int ret; /* @@ -214,9 +216,13 @@ int intel_gvt_init_device(struct drm_i915_private *dev_priv) if (WARN_ON(!intel_gvt_host.initialized)) return -EINVAL; - if (WARN_ON(gvt->initialized)) + if (WARN_ON(dev_priv->gvt)) return -EEXIST; + gvt = kzalloc(sizeof(struct intel_gvt), GFP_KERNEL); + if (!gvt) + return -ENOMEM; + gvt_dbg_core("init gvt device\n"); mutex_init(&gvt->lock); @@ -261,7 +267,7 @@ int intel_gvt_init_device(struct drm_i915_private *dev_priv) goto out_clean_cmd_parser; gvt_dbg_core("gvt device creation is done\n"); - gvt->initialized = true; + dev_priv->gvt = gvt; return 0; out_clean_cmd_parser: @@ -280,5 +286,6 @@ int intel_gvt_init_device(struct drm_i915_private *dev_priv) intel_gvt_free_firmware(gvt); out_clean_mmio_info: intel_gvt_clean_mmio_info(gvt); + kfree(gvt); return ret; } diff --git a/drivers/gpu/drm/i915/gvt/gvt.h b/drivers/gpu/drm/i915/gvt/gvt.h index 1564554b74597..15c595e0a63b6 100644 --- a/drivers/gpu/drm/i915/gvt/gvt.h +++ b/drivers/gpu/drm/i915/gvt/gvt.h @@ -192,8 +192,6 @@ struct intel_gvt_opregion { struct intel_gvt { struct mutex lock; - bool initialized; - struct drm_i915_private *dev_priv; struct idr vgpu_idr; /* vGPU IDR pool */ @@ -213,6 +211,11 @@ struct intel_gvt { unsigned long service_request; }; +static inline struct intel_gvt *to_gvt(struct drm_i915_private *i915) +{ + return i915->gvt; +} + enum { INTEL_GVT_REQUEST_EMULATE_VBLANK = 0, }; diff --git a/drivers/gpu/drm/i915/gvt/handlers.c b/drivers/gpu/drm/i915/gvt/handlers.c index e8ec403b75a18..b21115fecf860 100644 --- a/drivers/gpu/drm/i915/gvt/handlers.c +++ b/drivers/gpu/drm/i915/gvt/handlers.c @@ -37,6 +37,8 @@ */ #include "i915_drv.h" +#include "gvt.h" +#include "i915_pvinfo.h" /* XXX FIXME i915 has changed PP_XXX definition */ #define PCH_PP_STATUS _MMIO(0xc7200) diff --git a/drivers/gpu/drm/i915/gvt/interrupt.c b/drivers/gpu/drm/i915/gvt/interrupt.c index 84d7174d0081d..e43ef72285574 100644 --- a/drivers/gpu/drm/i915/gvt/interrupt.c +++ b/drivers/gpu/drm/i915/gvt/interrupt.c @@ -30,6 +30,7 @@ */ #include "i915_drv.h" +#include "gvt.h" /* common offset among interrupt control registers */ #define regbase_to_isr(base) (base) diff --git a/drivers/gpu/drm/i915/gvt/mmio.c b/drivers/gpu/drm/i915/gvt/mmio.c index ce3af95d049f1..585b01f632544 100644 --- a/drivers/gpu/drm/i915/gvt/mmio.c +++ b/drivers/gpu/drm/i915/gvt/mmio.c @@ -34,6 +34,7 @@ */ #include "i915_drv.h" +#include "gvt.h" /** * intel_vgpu_gpa_to_mmio_offset - translate a GPA to MMIO offset diff --git a/drivers/gpu/drm/i915/gvt/opregion.c b/drivers/gpu/drm/i915/gvt/opregion.c index 46cc2407a0a22..53ac81f63c645 100644 --- a/drivers/gpu/drm/i915/gvt/opregion.c +++ b/drivers/gpu/drm/i915/gvt/opregion.c @@ -23,6 +23,7 @@ #include #include "i915_drv.h" +#include "gvt.h" static int init_vgpu_opregion(struct intel_vgpu *vgpu, u32 gpa) { diff --git a/drivers/gpu/drm/i915/gvt/render.c b/drivers/gpu/drm/i915/gvt/render.c index f54ab8540b12a..feebb65ba6417 100644 --- a/drivers/gpu/drm/i915/gvt/render.c +++ b/drivers/gpu/drm/i915/gvt/render.c @@ -34,6 +34,7 @@ */ #include "i915_drv.h" +#include "gvt.h" struct render_mmio { int ring_id; diff --git a/drivers/gpu/drm/i915/gvt/sched_policy.c b/drivers/gpu/drm/i915/gvt/sched_policy.c index c607354c37980..278db0c180fc9 100644 --- a/drivers/gpu/drm/i915/gvt/sched_policy.c +++ b/drivers/gpu/drm/i915/gvt/sched_policy.c @@ -32,6 +32,7 @@ */ #include "i915_drv.h" +#include "gvt.h" static bool vgpu_has_pending_workload(struct intel_vgpu *vgpu) { diff --git a/drivers/gpu/drm/i915/gvt/scheduler.c b/drivers/gpu/drm/i915/gvt/scheduler.c index b15cdf5978a9e..01d23ad03637f 100644 --- a/drivers/gpu/drm/i915/gvt/scheduler.c +++ b/drivers/gpu/drm/i915/gvt/scheduler.c @@ -33,10 +33,11 @@ * */ -#include "i915_drv.h" - #include +#include "i915_drv.h" +#include "gvt.h" + #define RING_CTX_OFF(x) \ offsetof(struct execlist_ring_context, x) diff --git a/drivers/gpu/drm/i915/gvt/vgpu.c b/drivers/gpu/drm/i915/gvt/vgpu.c index e5e0a72336c83..9401436d721fe 100644 --- a/drivers/gpu/drm/i915/gvt/vgpu.c +++ b/drivers/gpu/drm/i915/gvt/vgpu.c @@ -32,6 +32,8 @@ */ #include "i915_drv.h" +#include "gvt.h" +#include "i915_pvinfo.h" static void clean_vgpu_mmio(struct intel_vgpu *vgpu) { diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index 4d1133ffe0938..5024ad91d1327 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -1778,7 +1778,7 @@ struct drm_i915_private { struct i915_virtual_gpu vgpu; - struct intel_gvt gvt; + struct intel_gvt *gvt; struct intel_guc guc; @@ -2992,7 +2992,7 @@ int intel_wait_for_register_fw(struct drm_i915_private *dev_priv, static inline bool intel_gvt_active(struct drm_i915_private *dev_priv) { - return dev_priv->gvt.initialized; + return dev_priv->gvt; } static inline bool intel_vgpu_active(struct drm_i915_private *dev_priv) diff --git a/drivers/gpu/drm/i915/intel_gvt.h b/drivers/gpu/drm/i915/intel_gvt.h index 0f00105f4c5dd..25df2d65b985c 100644 --- a/drivers/gpu/drm/i915/intel_gvt.h +++ b/drivers/gpu/drm/i915/intel_gvt.h @@ -24,8 +24,7 @@ #ifndef _INTEL_GVT_H_ #define _INTEL_GVT_H_ -#include "i915_pvinfo.h" -#include "gvt/gvt.h" +struct intel_gvt; #ifdef CONFIG_DRM_I915_GVT int intel_gvt_init(struct drm_i915_private *dev_priv); From 66a46e9df043cc316cc26efcb0972435db63654b Mon Sep 17 00:00:00 2001 From: Zhenyu Wang Date: Wed, 19 Oct 2016 11:38:33 +0800 Subject: [PATCH 02/19] MAINTAINERS: Add new Intel GVT-g driver maintainer This adds new item for Intel GVT-g driver maintainer info. Signed-off-by: Zhenyu Wang --- MAINTAINERS | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/MAINTAINERS b/MAINTAINERS index 703fcb51b7826..8af083e4e193b 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -4009,6 +4009,16 @@ F: include/drm/i915* F: include/uapi/drm/i915_drm.h F: Documentation/gpu/i915.rst +INTEL GVT-g DRIVERS (Intel GPU Virtualization) +M: Zhenyu Wang +M: Zhi Wang +L: igvt-g-dev@lists.01.org +L: intel-gfx@lists.freedesktop.org +W: https://01.org/igvt-g +T: git https://github.com/01org/gvt-linux.git +S: Supported +F: drivers/gpu/drm/i915/gvt/ + DRM DRIVERS FOR ATMEL HLCDC M: Boris Brezillon L: dri-devel@lists.freedesktop.org From bbc3693351fcf4ab74b0913e15189362588cd34f Mon Sep 17 00:00:00 2001 From: Zhenyu Wang Date: Wed, 19 Oct 2016 12:36:56 +0800 Subject: [PATCH 03/19] drm/i915/gvt: Fix warning on obsolete function usage Don't use obsolete drm_gem_object_unreference() but switch to i915_gem_object_put(). Signed-off-by: Zhenyu Wang --- drivers/gpu/drm/i915/gvt/execlist.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/i915/gvt/execlist.c b/drivers/gpu/drm/i915/gvt/execlist.c index b87b4f5e4c8f4..983bf863bc1f0 100644 --- a/drivers/gpu/drm/i915/gvt/execlist.c +++ b/drivers/gpu/drm/i915/gvt/execlist.c @@ -499,7 +499,7 @@ static void release_shadow_batch_buffer(struct intel_vgpu_workload *workload) list_for_each_entry_safe(entry_obj, temp, &workload->shadow_bb, list) { - drm_gem_object_unreference(&(entry_obj->obj->base)); + i915_gem_object_put(entry_obj->obj); kvfree(entry_obj->va); list_del(&entry_obj->list); kfree(entry_obj); @@ -512,7 +512,7 @@ static void release_shadow_wa_ctx(struct intel_shadow_wa_ctx *wa_ctx) if (wa_ctx->indirect_ctx.size == 0) return; - drm_gem_object_unreference(&(wa_ctx->indirect_ctx.obj->base)); + i915_gem_object_put(wa_ctx->indirect_ctx.obj); kvfree(wa_ctx->indirect_ctx.shadow_va); } From 22681c7bc79aefbb8e1c459474ac668a9493b577 Mon Sep 17 00:00:00 2001 From: Zhenyu Wang Date: Wed, 19 Oct 2016 14:40:59 +0800 Subject: [PATCH 04/19] Documentation/gpu: Add section for Intel GVT-g host support Update with brief overview and reference for more detailed arch design documents. Add new section for Intel GVT-g host support. Signed-off-by: Zhenyu Wang --- Documentation/gpu/i915.rst | 9 +++++++++ drivers/gpu/drm/i915/intel_gvt.c | 8 ++++++-- 2 files changed, 15 insertions(+), 2 deletions(-) diff --git a/Documentation/gpu/i915.rst b/Documentation/gpu/i915.rst index 87aaffc229202..95ce77ff43423 100644 --- a/Documentation/gpu/i915.rst +++ b/Documentation/gpu/i915.rst @@ -49,6 +49,15 @@ Intel GVT-g Guest Support(vGPU) .. kernel-doc:: drivers/gpu/drm/i915/i915_vgpu.c :internal: +Intel GVT-g Host Support(vGPU device model) +------------------------------------------- + +.. kernel-doc:: drivers/gpu/drm/i915/intel_gvt.c + :doc: Intel GVT-g host support + +.. kernel-doc:: drivers/gpu/drm/i915/intel_gvt.c + :internal: + Display Hardware Handling ========================= diff --git a/drivers/gpu/drm/i915/intel_gvt.c b/drivers/gpu/drm/i915/intel_gvt.c index 8e8596da89b15..290384e86c635 100644 --- a/drivers/gpu/drm/i915/intel_gvt.c +++ b/drivers/gpu/drm/i915/intel_gvt.c @@ -31,8 +31,12 @@ * GPU among multiple virtual machines on a time-sharing basis. Each * virtual machine is presented a virtual GPU (vGPU), which has equivalent * features as the underlying physical GPU (pGPU), so i915 driver can run - * seamlessly in a virtual machine. This file provides the englightments - * of GVT and the necessary components used by GVT in i915 driver. + * seamlessly in a virtual machine. + * + * To virtualize GPU resources GVT-g driver depends on hypervisor technology + * e.g KVM/VFIO/mdev, Xen, etc. to provide resource access trapping capability + * and be virtualized within GVT-g device module. More architectural design + * doc is available on https://01.org/group/2230/documentation-list. */ static bool is_supported_device(struct drm_i915_private *dev_priv) From 75ea10da063f96d81828316cc25a896ae523c826 Mon Sep 17 00:00:00 2001 From: Chris Wilson Date: Wed, 19 Oct 2016 11:11:37 +0100 Subject: [PATCH 05/19] drm/i915/gvt: Add runtime pm around fences Manipulating the fence_list requires the runtime wakelock, as does writing to the fence registers. Acquire a wakelock for the former, and assert that the device is awake for the latter. Signed-off-by: Chris Wilson Signed-off-by: Zhenyu Wang --- drivers/gpu/drm/i915/gvt/aperture_gm.c | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/drivers/gpu/drm/i915/gvt/aperture_gm.c b/drivers/gpu/drm/i915/gvt/aperture_gm.c index db503c164b2fe..0d41ebc4aea63 100644 --- a/drivers/gpu/drm/i915/gvt/aperture_gm.c +++ b/drivers/gpu/drm/i915/gvt/aperture_gm.c @@ -145,6 +145,8 @@ void intel_vgpu_write_fence(struct intel_vgpu *vgpu, struct drm_i915_fence_reg *reg; i915_reg_t fence_reg_lo, fence_reg_hi; + assert_rpm_wakelock_held(dev_priv); + if (WARN_ON(fence > vgpu_fence_sz(vgpu))) return; @@ -173,6 +175,8 @@ static void free_vgpu_fence(struct intel_vgpu *vgpu) if (WARN_ON(!vgpu_fence_sz(vgpu))) return; + intel_runtime_pm_get(dev_priv); + mutex_lock(&dev_priv->drm.struct_mutex); for (i = 0; i < vgpu_fence_sz(vgpu); i++) { reg = vgpu->fence.regs[i]; @@ -181,6 +185,8 @@ static void free_vgpu_fence(struct intel_vgpu *vgpu) &dev_priv->mm.fence_list); } mutex_unlock(&dev_priv->drm.struct_mutex); + + intel_runtime_pm_put(dev_priv); } static int alloc_vgpu_fence(struct intel_vgpu *vgpu) @@ -191,6 +197,8 @@ static int alloc_vgpu_fence(struct intel_vgpu *vgpu) int i; struct list_head *pos, *q; + intel_runtime_pm_get(dev_priv); + /* Request fences from host */ mutex_lock(&dev_priv->drm.struct_mutex); i = 0; @@ -208,6 +216,7 @@ static int alloc_vgpu_fence(struct intel_vgpu *vgpu) goto out_free_fence; mutex_unlock(&dev_priv->drm.struct_mutex); + intel_runtime_pm_put(dev_priv); return 0; out_free_fence: /* Return fences to host, if fail */ @@ -219,6 +228,7 @@ static int alloc_vgpu_fence(struct intel_vgpu *vgpu) &dev_priv->mm.fence_list); } mutex_unlock(&dev_priv->drm.struct_mutex); + intel_runtime_pm_put(dev_priv); return -ENOSPC; } From 894cf7d156346986946cc573dc8c251804dc8321 Mon Sep 17 00:00:00 2001 From: Chris Wilson Date: Wed, 19 Oct 2016 11:11:38 +0100 Subject: [PATCH 06/19] drm/i915/gvt: i915_gem_object_create() returns an error pointer On failure from i915_gem_object_create(), we need to check for an error pointer not NULL. Signed-off-by: Chris Wilson Reviewed-by: Zhenyu Wang Signed-off-by: Zhenyu Wang --- drivers/gpu/drm/i915/gvt/cmd_parser.c | 48 ++++++++++++++++----------- 1 file changed, 28 insertions(+), 20 deletions(-) diff --git a/drivers/gpu/drm/i915/gvt/cmd_parser.c b/drivers/gpu/drm/i915/gvt/cmd_parser.c index 5b4658ff4d8ac..d942da9a0c8a7 100644 --- a/drivers/gpu/drm/i915/gvt/cmd_parser.c +++ b/drivers/gpu/drm/i915/gvt/cmd_parser.c @@ -1640,16 +1640,19 @@ static int perform_bb_shadow(struct parser_exec_state *s) if (entry_obj == NULL) return -ENOMEM; - entry_obj->obj = i915_gem_object_create(&(s->vgpu->gvt->dev_priv->drm), - round_up(bb_size, PAGE_SIZE)); - if (entry_obj->obj == NULL) - return -ENOMEM; + entry_obj->obj = + i915_gem_object_create(&(s->vgpu->gvt->dev_priv->drm), + roundup(bb_size, PAGE_SIZE)); + if (IS_ERR(entry_obj->obj)) { + ret = PTR_ERR(entry_obj->obj); + goto free_entry; + } entry_obj->len = bb_size; INIT_LIST_HEAD(&entry_obj->list); ret = i915_gem_object_get_pages(entry_obj->obj); if (ret) - return ret; + goto put_obj; i915_gem_object_pin_pages(entry_obj->obj); @@ -1675,7 +1678,7 @@ static int perform_bb_shadow(struct parser_exec_state *s) gma, gma + bb_size, dst); if (ret) { gvt_err("fail to copy guest ring buffer\n"); - return ret; + goto unmap_src; } list_add(&entry_obj->list, &s->workload->shadow_bb); @@ -1696,7 +1699,10 @@ static int perform_bb_shadow(struct parser_exec_state *s) vunmap(dst); unpin_src: i915_gem_object_unpin_pages(entry_obj->obj); - +put_obj: + i915_gem_object_put(entry_obj->obj); +free_entry: + kfree(entry_obj); return ret; } @@ -2709,31 +2715,31 @@ static int shadow_indirect_ctx(struct intel_shadow_wa_ctx *wa_ctx) struct drm_device *dev = &wa_ctx->workload->vgpu->gvt->dev_priv->drm; int ctx_size = wa_ctx->indirect_ctx.size; unsigned long guest_gma = wa_ctx->indirect_ctx.guest_gma; + struct drm_i915_gem_object *obj; int ret = 0; void *dest = NULL; - wa_ctx->indirect_ctx.obj = i915_gem_object_create(dev, - round_up(ctx_size + CACHELINE_BYTES, PAGE_SIZE)); - if (wa_ctx->indirect_ctx.obj == NULL) - return -ENOMEM; + obj = i915_gem_object_create(dev, + roundup(ctx_size + CACHELINE_BYTES, + PAGE_SIZE)); + if (IS_ERR(obj)) + return PTR_ERR(obj); - ret = i915_gem_object_get_pages(wa_ctx->indirect_ctx.obj); + ret = i915_gem_object_get_pages(obj); if (ret) - return ret; + goto put_obj; - i915_gem_object_pin_pages(wa_ctx->indirect_ctx.obj); + i915_gem_object_pin_pages(obj); /* get the va of the shadow batch buffer */ - dest = (void *)vmap_batch(wa_ctx->indirect_ctx.obj, 0, - ctx_size + CACHELINE_BYTES); + dest = (void *)vmap_batch(obj, 0, ctx_size + CACHELINE_BYTES); if (!dest) { gvt_err("failed to vmap shadow indirect ctx\n"); ret = -ENOMEM; goto unpin_src; } - ret = i915_gem_object_set_to_cpu_domain(wa_ctx->indirect_ctx.obj, - false); + ret = i915_gem_object_set_to_cpu_domain(obj, false); if (ret) { gvt_err("failed to set shadow indirect ctx to CPU\n"); goto unmap_src; @@ -2748,16 +2754,18 @@ static int shadow_indirect_ctx(struct intel_shadow_wa_ctx *wa_ctx) guest_gma, guest_gma + ctx_size, dest); if (ret) { gvt_err("fail to copy guest indirect ctx\n"); - return ret; + goto unmap_src; } + wa_ctx->indirect_ctx.obj = obj; return 0; unmap_src: vunmap(dest); unpin_src: i915_gem_object_unpin_pages(wa_ctx->indirect_ctx.obj); - +put_obj: + i915_gem_object_put(wa_ctx->indirect_ctx.obj); return ret; } From b6d891429d29f4ff7cacbaa5c4bb1511797a4bce Mon Sep 17 00:00:00 2001 From: Chris Wilson Date: Wed, 19 Oct 2016 11:11:40 +0100 Subject: [PATCH 07/19] drm/i915/gvt: Use the returned VMA to provide the virtual address The purpose of returning the just-pinned VMA is so that we can use the information within, like its address. Also it should be tracked and used as the cookie to unpin... Signed-off-by: Chris Wilson Reviewed-by: Zhenyu Wang Signed-off-by: Zhenyu Wang --- drivers/gpu/drm/i915/gvt/execlist.c | 20 +++++++++----------- 1 file changed, 9 insertions(+), 11 deletions(-) diff --git a/drivers/gpu/drm/i915/gvt/execlist.c b/drivers/gpu/drm/i915/gvt/execlist.c index 983bf863bc1f0..f865ce0c77272 100644 --- a/drivers/gpu/drm/i915/gvt/execlist.c +++ b/drivers/gpu/drm/i915/gvt/execlist.c @@ -386,8 +386,6 @@ static int set_gma_to_bb_cmd(struct intel_shadow_bb_entry *entry_obj, static void prepare_shadow_batch_buffer(struct intel_vgpu_workload *workload) { int gmadr_bytes = workload->vgpu->gvt->device_info.gmadr_bytes_in_cmd; - struct i915_vma *vma; - unsigned long gma; /* pin the gem object to ggtt */ if (!list_empty(&workload->shadow_bb)) { @@ -399,8 +397,10 @@ static void prepare_shadow_batch_buffer(struct intel_vgpu_workload *workload) list_for_each_entry_safe(entry_obj, temp, &workload->shadow_bb, list) { + struct i915_vma *vma; + vma = i915_gem_object_ggtt_pin(entry_obj->obj, NULL, 0, - 0, 0); + 4, 0); if (IS_ERR(vma)) { gvt_err("Cannot pin\n"); return; @@ -408,9 +408,9 @@ static void prepare_shadow_batch_buffer(struct intel_vgpu_workload *workload) i915_gem_object_unpin_pages(entry_obj->obj); /* update the relocate gma with shadow batch buffer*/ - gma = i915_gem_object_ggtt_offset(entry_obj->obj, NULL); - WARN_ON(!IS_ALIGNED(gma, 4)); - set_gma_to_bb_cmd(entry_obj, gma, gmadr_bytes); + set_gma_to_bb_cmd(entry_obj, + i915_ggtt_offset(vma), + gmadr_bytes); } } } @@ -442,7 +442,6 @@ static int update_wa_ctx_2_shadow_ctx(struct intel_shadow_wa_ctx *wa_ctx) static void prepare_shadow_wa_ctx(struct intel_shadow_wa_ctx *wa_ctx) { struct i915_vma *vma; - unsigned long gma; unsigned char *per_ctx_va = (unsigned char *)wa_ctx->indirect_ctx.shadow_va + wa_ctx->indirect_ctx.size; @@ -450,16 +449,15 @@ static void prepare_shadow_wa_ctx(struct intel_shadow_wa_ctx *wa_ctx) if (wa_ctx->indirect_ctx.size == 0) return; - vma = i915_gem_object_ggtt_pin(wa_ctx->indirect_ctx.obj, NULL, 0, 0, 0); + vma = i915_gem_object_ggtt_pin(wa_ctx->indirect_ctx.obj, NULL, + 0, CACHELINE_BYTES, 0); if (IS_ERR(vma)) { gvt_err("Cannot pin indirect ctx obj\n"); return; } i915_gem_object_unpin_pages(wa_ctx->indirect_ctx.obj); - gma = i915_gem_object_ggtt_offset(wa_ctx->indirect_ctx.obj, NULL); - WARN_ON(!IS_ALIGNED(gma, CACHELINE_BYTES)); - wa_ctx->indirect_ctx.shadow_gma = gma; + wa_ctx->indirect_ctx.shadow_gma = i915_ggtt_offset(vma); wa_ctx->per_ctx.shadow_gma = *((unsigned int *)per_ctx_va + 1); memset(per_ctx_va, 0, CACHELINE_BYTES); From eeacd86efa53e6328c63b79d1999a7d214972278 Mon Sep 17 00:00:00 2001 From: Chris Wilson Date: Wed, 19 Oct 2016 11:11:41 +0100 Subject: [PATCH 08/19] drm/i915/gvt: Remove dangerous unpin of backing storage of bound GPU object Unpinning the pages prior to the object being release from the GPU may allow the GPU to read and write into system pages (i.e. use after free by the hw). Signed-off-by: Chris Wilson Signed-off-by: Zhenyu Wang --- drivers/gpu/drm/i915/gvt/execlist.c | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/i915/gvt/execlist.c b/drivers/gpu/drm/i915/gvt/execlist.c index f865ce0c77272..5534336814f00 100644 --- a/drivers/gpu/drm/i915/gvt/execlist.c +++ b/drivers/gpu/drm/i915/gvt/execlist.c @@ -405,7 +405,11 @@ static void prepare_shadow_batch_buffer(struct intel_vgpu_workload *workload) gvt_err("Cannot pin\n"); return; } - i915_gem_object_unpin_pages(entry_obj->obj); + + /* FIXME: we are not tracking our pinned VMA leaving it + * up to the core to fix up the stray pin_count upon + * free. + */ /* update the relocate gma with shadow batch buffer*/ set_gma_to_bb_cmd(entry_obj, @@ -455,7 +459,11 @@ static void prepare_shadow_wa_ctx(struct intel_shadow_wa_ctx *wa_ctx) gvt_err("Cannot pin indirect ctx obj\n"); return; } - i915_gem_object_unpin_pages(wa_ctx->indirect_ctx.obj); + + /* FIXME: we are not tracking our pinned VMA leaving it + * up to the core to fix up the stray pin_count upon + * free. + */ wa_ctx->indirect_ctx.shadow_gma = i915_ggtt_offset(vma); From 0eb742d7af224481ab7abb7b38bd7166e47661e2 Mon Sep 17 00:00:00 2001 From: Chris Wilson Date: Thu, 20 Oct 2016 17:29:36 +0800 Subject: [PATCH 09/19] drm/i915/gvt: Hold a reference on the request The workload took a pointer to the request, and even waited upon, without holding a reference on the request. Take that reference explicitly and fix up the error path following request allocation that missed flushing the request. v2: [zhenyuw] - drop request put in error path for dispatch, as main thread caller will handle it identically to a real request. Signed-off-by: Chris Wilson Signed-off-by: Zhenyu Wang --- drivers/gpu/drm/i915/gvt/scheduler.c | 23 ++++++++++++----------- 1 file changed, 12 insertions(+), 11 deletions(-) diff --git a/drivers/gpu/drm/i915/gvt/scheduler.c b/drivers/gpu/drm/i915/gvt/scheduler.c index 01d23ad03637f..3c2d8e9b2d3fa 100644 --- a/drivers/gpu/drm/i915/gvt/scheduler.c +++ b/drivers/gpu/drm/i915/gvt/scheduler.c @@ -164,6 +164,7 @@ static int dispatch_workload(struct intel_vgpu_workload *workload) int ring_id = workload->ring_id; struct i915_gem_context *shadow_ctx = workload->vgpu->shadow_ctx; struct drm_i915_private *dev_priv = workload->vgpu->gvt->dev_priv; + struct drm_i915_gem_request *rq; int ret; gvt_dbg_sched("ring id %d prepare to dispatch workload %p\n", @@ -172,17 +173,16 @@ static int dispatch_workload(struct intel_vgpu_workload *workload) shadow_ctx->desc_template = workload->ctx_desc.addressing_mode << GEN8_CTX_ADDRESSING_MODE_SHIFT; - workload->req = i915_gem_request_alloc(dev_priv->engine[ring_id], - shadow_ctx); - if (IS_ERR_OR_NULL(workload->req)) { + rq = i915_gem_request_alloc(dev_priv->engine[ring_id], shadow_ctx); + if (IS_ERR(rq)) { gvt_err("fail to allocate gem request\n"); - workload->status = PTR_ERR(workload->req); - workload->req = NULL; + workload->status = PTR_ERR(rq); return workload->status; } - gvt_dbg_sched("ring id %d get i915 gem request %p\n", - ring_id, workload->req); + gvt_dbg_sched("ring id %d get i915 gem request %p\n", ring_id, rq); + + workload->req = i915_gem_request_get(rq); mutex_lock(&gvt->lock); @@ -209,16 +209,15 @@ static int dispatch_workload(struct intel_vgpu_workload *workload) gvt_dbg_sched("ring id %d submit workload to i915 %p\n", ring_id, workload->req); - i915_add_request_no_flush(workload->req); - + i915_add_request_no_flush(rq); workload->dispatched = true; return 0; err: workload->status = ret; - if (workload->req) - workload->req = NULL; mutex_unlock(&gvt->lock); + + i915_add_request_no_flush(rq); return ret; } @@ -459,6 +458,8 @@ static int workload_thread(void *priv) complete_current_workload(gvt, ring_id); + i915_gem_request_put(fetch_and_zero(&workload->req)); + if (need_force_wake) intel_uncore_forcewake_put(gvt->dev_priv, FORCEWAKE_ALL); From f460c251ea37836c57584a18981820fbde809d1d Mon Sep 17 00:00:00 2001 From: Chris Wilson Date: Wed, 19 Oct 2016 11:11:43 +0100 Subject: [PATCH 10/19] drm/i915/gvt: Stop checking for impossible interrupts from a kthread The kthread will not be interrupted, don't even bother checking. Signed-off-by: Chris Wilson Reviewed-by: Zhenyu Wang Signed-off-by: Zhenyu Wang --- drivers/gpu/drm/i915/gvt/scheduler.c | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/drivers/gpu/drm/i915/gvt/scheduler.c b/drivers/gpu/drm/i915/gvt/scheduler.c index 3c2d8e9b2d3fa..9c508c307ff0b 100644 --- a/drivers/gpu/drm/i915/gvt/scheduler.c +++ b/drivers/gpu/drm/i915/gvt/scheduler.c @@ -423,12 +423,7 @@ static int workload_thread(void *priv) /* * Always take i915 big lock first */ - ret = i915_mutex_lock_interruptible(&gvt->dev_priv->drm); - if (ret < 0) { - gvt_err("i915 submission is not available, retry\n"); - schedule_timeout(1); - continue; - } + mutex_lock(&gvt->dev_priv->drm.struct_mutex); gvt_dbg_sched("ring id %d will dispatch workload %p\n", workload->ring_id, workload); @@ -447,7 +442,7 @@ static int workload_thread(void *priv) workload->ring_id, workload); workload->status = i915_wait_request(workload->req, - I915_WAIT_INTERRUPTIBLE | I915_WAIT_LOCKED, + I915_WAIT_LOCKED, NULL, NULL); if (workload->status != 0) gvt_err("fail to wait workload, skip\n"); From 66bbc3b2b16b4d15de0bd737147538bcf4d355b6 Mon Sep 17 00:00:00 2001 From: Chris Wilson Date: Wed, 19 Oct 2016 11:11:44 +0100 Subject: [PATCH 11/19] drm/i915/gvt: Stop waiting whilst holding struct_mutex For whatever reason, the gvt scheduler runs synchronously. At the very least, lets run synchronously without holding the struct_mutex. v2: cut'n'paste mutex_lock instead of unlock. Replace long hold of struct_mutex with a mutex to serialise the worker threads. Signed-off-by: Chris Wilson Signed-off-by: Zhenyu Wang --- drivers/gpu/drm/i915/gvt/scheduler.c | 22 +++++++++++++--------- 1 file changed, 13 insertions(+), 9 deletions(-) diff --git a/drivers/gpu/drm/i915/gvt/scheduler.c b/drivers/gpu/drm/i915/gvt/scheduler.c index 9c508c307ff0b..12f825512e9be 100644 --- a/drivers/gpu/drm/i915/gvt/scheduler.c +++ b/drivers/gpu/drm/i915/gvt/scheduler.c @@ -390,6 +390,8 @@ struct workload_thread_param { int ring_id; }; +static DEFINE_MUTEX(scheduler_mutex); + static int workload_thread(void *priv) { struct workload_thread_param *p = (struct workload_thread_param *)priv; @@ -414,17 +416,14 @@ static int workload_thread(void *priv) if (kthread_should_stop()) break; + mutex_lock(&scheduler_mutex); + gvt_dbg_sched("ring id %d next workload %p vgpu %d\n", workload->ring_id, workload, workload->vgpu->id); intel_runtime_pm_get(gvt->dev_priv); - /* - * Always take i915 big lock first - */ - mutex_lock(&gvt->dev_priv->drm.struct_mutex); - gvt_dbg_sched("ring id %d will dispatch workload %p\n", workload->ring_id, workload); @@ -432,7 +431,10 @@ static int workload_thread(void *priv) intel_uncore_forcewake_get(gvt->dev_priv, FORCEWAKE_ALL); + mutex_lock(&gvt->dev_priv->drm.struct_mutex); ret = dispatch_workload(workload); + mutex_unlock(&gvt->dev_priv->drm.struct_mutex); + if (ret) { gvt_err("fail to dispatch workload, skip\n"); goto complete; @@ -442,8 +444,7 @@ static int workload_thread(void *priv) workload->ring_id, workload); workload->status = i915_wait_request(workload->req, - I915_WAIT_LOCKED, - NULL, NULL); + 0, NULL, NULL); if (workload->status != 0) gvt_err("fail to wait workload, skip\n"); @@ -451,7 +452,9 @@ static int workload_thread(void *priv) gvt_dbg_sched("will complete workload %p\n, status: %d\n", workload, workload->status); + mutex_lock(&gvt->dev_priv->drm.struct_mutex); complete_current_workload(gvt, ring_id); + mutex_unlock(&gvt->dev_priv->drm.struct_mutex); i915_gem_request_put(fetch_and_zero(&workload->req)); @@ -459,9 +462,10 @@ static int workload_thread(void *priv) intel_uncore_forcewake_put(gvt->dev_priv, FORCEWAKE_ALL); - mutex_unlock(&gvt->dev_priv->drm.struct_mutex); - intel_runtime_pm_put(gvt->dev_priv); + + mutex_unlock(&scheduler_mutex); + } return 0; } From bcd0aeded478f1ed6dfbbeafc3e2581c4021a99c Mon Sep 17 00:00:00 2001 From: Chris Wilson Date: Wed, 19 Oct 2016 11:11:45 +0100 Subject: [PATCH 12/19] drm/i915/gvt: Use common mapping routines for indirect_ctx object We have the ability to map an object, so use it rather than opencode it badly. Signed-off-by: Chris Wilson Signed-off-by: Zhenyu Wang --- drivers/gpu/drm/i915/gvt/cmd_parser.c | 28 +++++++++------------------ drivers/gpu/drm/i915/gvt/execlist.c | 2 +- 2 files changed, 10 insertions(+), 20 deletions(-) diff --git a/drivers/gpu/drm/i915/gvt/cmd_parser.c b/drivers/gpu/drm/i915/gvt/cmd_parser.c index d942da9a0c8a7..153943a9411e4 100644 --- a/drivers/gpu/drm/i915/gvt/cmd_parser.c +++ b/drivers/gpu/drm/i915/gvt/cmd_parser.c @@ -2717,7 +2717,7 @@ static int shadow_indirect_ctx(struct intel_shadow_wa_ctx *wa_ctx) unsigned long guest_gma = wa_ctx->indirect_ctx.guest_gma; struct drm_i915_gem_object *obj; int ret = 0; - void *dest = NULL; + void *map; obj = i915_gem_object_create(dev, roundup(ctx_size + CACHELINE_BYTES, @@ -2725,18 +2725,12 @@ static int shadow_indirect_ctx(struct intel_shadow_wa_ctx *wa_ctx) if (IS_ERR(obj)) return PTR_ERR(obj); - ret = i915_gem_object_get_pages(obj); - if (ret) - goto put_obj; - - i915_gem_object_pin_pages(obj); - /* get the va of the shadow batch buffer */ - dest = (void *)vmap_batch(obj, 0, ctx_size + CACHELINE_BYTES); - if (!dest) { + map = i915_gem_object_pin_map(obj, I915_MAP_WB); + if (IS_ERR(map)) { gvt_err("failed to vmap shadow indirect ctx\n"); - ret = -ENOMEM; - goto unpin_src; + ret = PTR_ERR(map); + goto put_obj; } ret = i915_gem_object_set_to_cpu_domain(obj, false); @@ -2745,25 +2739,21 @@ static int shadow_indirect_ctx(struct intel_shadow_wa_ctx *wa_ctx) goto unmap_src; } - wa_ctx->indirect_ctx.shadow_va = dest; - - memset(dest, 0, round_up(ctx_size + CACHELINE_BYTES, PAGE_SIZE)); - ret = copy_gma_to_hva(wa_ctx->workload->vgpu, wa_ctx->workload->vgpu->gtt.ggtt_mm, - guest_gma, guest_gma + ctx_size, dest); + guest_gma, guest_gma + ctx_size, + map); if (ret) { gvt_err("fail to copy guest indirect ctx\n"); goto unmap_src; } wa_ctx->indirect_ctx.obj = obj; + wa_ctx->indirect_ctx.shadow_va = map; return 0; unmap_src: - vunmap(dest); -unpin_src: - i915_gem_object_unpin_pages(wa_ctx->indirect_ctx.obj); + i915_gem_object_unpin_map(obj); put_obj: i915_gem_object_put(wa_ctx->indirect_ctx.obj); return ret; diff --git a/drivers/gpu/drm/i915/gvt/execlist.c b/drivers/gpu/drm/i915/gvt/execlist.c index 5534336814f00..88430ca23504b 100644 --- a/drivers/gpu/drm/i915/gvt/execlist.c +++ b/drivers/gpu/drm/i915/gvt/execlist.c @@ -518,8 +518,8 @@ static void release_shadow_wa_ctx(struct intel_shadow_wa_ctx *wa_ctx) if (wa_ctx->indirect_ctx.size == 0) return; + i915_gem_object_unpin_map(wa_ctx->indirect_ctx.obj); i915_gem_object_put(wa_ctx->indirect_ctx.obj); - kvfree(wa_ctx->indirect_ctx.shadow_va); } static int complete_execlist_workload(struct intel_vgpu_workload *workload) From a28615041ea2d1645143b868cd5ea65e9cf28381 Mon Sep 17 00:00:00 2001 From: Chris Wilson Date: Wed, 19 Oct 2016 11:11:46 +0100 Subject: [PATCH 13/19] drm/i915/gvt: Use common mapping routines for shadow_bb object We have the ability to map an object, so use it rather than opencode it badly. Note that the object remains permanently pinned, this is poor practise. Signed-off-by: Chris Wilson Signed-off-by: Zhenyu Wang --- drivers/gpu/drm/i915/gvt/cmd_parser.c | 21 ++++++--------------- drivers/gpu/drm/i915/gvt/execlist.c | 2 +- 2 files changed, 7 insertions(+), 16 deletions(-) diff --git a/drivers/gpu/drm/i915/gvt/cmd_parser.c b/drivers/gpu/drm/i915/gvt/cmd_parser.c index 153943a9411e4..ff719e77a594f 100644 --- a/drivers/gpu/drm/i915/gvt/cmd_parser.c +++ b/drivers/gpu/drm/i915/gvt/cmd_parser.c @@ -1650,18 +1650,10 @@ static int perform_bb_shadow(struct parser_exec_state *s) entry_obj->len = bb_size; INIT_LIST_HEAD(&entry_obj->list); - ret = i915_gem_object_get_pages(entry_obj->obj); - if (ret) + dst = i915_gem_object_pin_map(entry_obj->obj, I915_MAP_WB); + if (IS_ERR(dst)) { + ret = PTR_ERR(dst); goto put_obj; - - i915_gem_object_pin_pages(entry_obj->obj); - - /* get the va of the shadow batch buffer */ - dst = (void *)vmap_batch(entry_obj->obj, 0, bb_size); - if (!dst) { - gvt_err("failed to vmap shadow batch\n"); - ret = -ENOMEM; - goto unpin_src; } ret = i915_gem_object_set_to_cpu_domain(entry_obj->obj, false); @@ -1675,7 +1667,8 @@ static int perform_bb_shadow(struct parser_exec_state *s) /* copy batch buffer to shadow batch buffer*/ ret = copy_gma_to_hva(s->vgpu, s->vgpu->gtt.ggtt_mm, - gma, gma + bb_size, dst); + gma, gma + bb_size, + dst); if (ret) { gvt_err("fail to copy guest ring buffer\n"); goto unmap_src; @@ -1696,9 +1689,7 @@ static int perform_bb_shadow(struct parser_exec_state *s) return 0; unmap_src: - vunmap(dst); -unpin_src: - i915_gem_object_unpin_pages(entry_obj->obj); + i915_gem_object_unpin_map(entry_obj->obj); put_obj: i915_gem_object_put(entry_obj->obj); free_entry: diff --git a/drivers/gpu/drm/i915/gvt/execlist.c b/drivers/gpu/drm/i915/gvt/execlist.c index 88430ca23504b..d4bd29306d84c 100644 --- a/drivers/gpu/drm/i915/gvt/execlist.c +++ b/drivers/gpu/drm/i915/gvt/execlist.c @@ -505,8 +505,8 @@ static void release_shadow_batch_buffer(struct intel_vgpu_workload *workload) list_for_each_entry_safe(entry_obj, temp, &workload->shadow_bb, list) { + i915_gem_object_unpin_map(entry_obj->obj); i915_gem_object_put(entry_obj->obj); - kvfree(entry_obj->va); list_del(&entry_obj->list); kfree(entry_obj); } From 3eec872207cc8230bc98cffa8895173e4effacb2 Mon Sep 17 00:00:00 2001 From: Chris Wilson Date: Wed, 19 Oct 2016 11:11:47 +0100 Subject: [PATCH 14/19] drm/i915/gvt: Remove defunct vmap_batch() This code was removed from i915_cmd_parser.c but still an obsolete version wound up being duplicated into gvt/cmd_parser.c. Good riddance. Signed-off-by: Chris Wilson Signed-off-by: Zhenyu Wang --- drivers/gpu/drm/i915/gvt/cmd_parser.c | 38 --------------------------- 1 file changed, 38 deletions(-) diff --git a/drivers/gpu/drm/i915/gvt/cmd_parser.c b/drivers/gpu/drm/i915/gvt/cmd_parser.c index ff719e77a594f..0d322e035d9f8 100644 --- a/drivers/gpu/drm/i915/gvt/cmd_parser.c +++ b/drivers/gpu/drm/i915/gvt/cmd_parser.c @@ -1583,44 +1583,6 @@ static uint32_t find_bb_size(struct parser_exec_state *s) return bb_size; } -static u32 *vmap_batch(struct drm_i915_gem_object *obj, - unsigned int start, unsigned int len) -{ - int i; - void *addr = NULL; - struct sg_page_iter sg_iter; - int first_page = start >> PAGE_SHIFT; - int last_page = (len + start + 4095) >> PAGE_SHIFT; - int npages = last_page - first_page; - struct page **pages; - - pages = drm_malloc_ab(npages, sizeof(*pages)); - if (pages == NULL) { - DRM_DEBUG_DRIVER("Failed to get space for pages\n"); - goto finish; - } - - i = 0; - for_each_sg_page(obj->pages->sgl, &sg_iter, obj->pages->nents, - first_page) { - pages[i++] = sg_page_iter_page(&sg_iter); - if (i == npages) - break; - } - - addr = vmap(pages, i, 0, PAGE_KERNEL); - if (addr == NULL) { - DRM_DEBUG_DRIVER("Failed to vmap pages\n"); - goto finish; - } - -finish: - if (pages) - drm_free_large(pages); - return (u32 *)addr; -} - - static int perform_bb_shadow(struct parser_exec_state *s) { struct intel_shadow_bb_entry *entry_obj; From 0fac21e7e978f8556d3f9bb1b2fadfc722bfe992 Mon Sep 17 00:00:00 2001 From: Zhenyu Wang Date: Thu, 20 Oct 2016 13:30:33 +0800 Subject: [PATCH 15/19] drm/i915/gvt: properly access enabled intel_engine_cs Switch to use new for_each_engine() helper to properly access enabled intel_engine_cs as i915 core has changed that to be dynamic managed. At GVT-g init time would still depend on ring mask to determine engine list as it's earlier. Signed-off-by: Zhenyu Wang --- drivers/gpu/drm/i915/gvt/execlist.c | 5 +++-- drivers/gpu/drm/i915/gvt/handlers.c | 11 ++++++----- drivers/gpu/drm/i915/gvt/sched_policy.c | 12 +++++++----- drivers/gpu/drm/i915/gvt/scheduler.c | 4 ++++ 4 files changed, 20 insertions(+), 12 deletions(-) diff --git a/drivers/gpu/drm/i915/gvt/execlist.c b/drivers/gpu/drm/i915/gvt/execlist.c index d4bd29306d84c..0e9b340897e3b 100644 --- a/drivers/gpu/drm/i915/gvt/execlist.c +++ b/drivers/gpu/drm/i915/gvt/execlist.c @@ -817,10 +817,11 @@ void intel_vgpu_clean_execlist(struct intel_vgpu *vgpu) int intel_vgpu_init_execlist(struct intel_vgpu *vgpu) { - int i; + enum intel_engine_id i; + struct intel_engine_cs *engine; /* each ring has a virtual execlist engine */ - for (i = 0; i < I915_NUM_ENGINES; i++) { + for_each_engine(engine, vgpu->gvt->dev_priv, i) { init_vgpu_execlist(vgpu, i); INIT_LIST_HEAD(&vgpu->workload_q_head[i]); } diff --git a/drivers/gpu/drm/i915/gvt/handlers.c b/drivers/gpu/drm/i915/gvt/handlers.c index b21115fecf860..3e74fb3d4aa93 100644 --- a/drivers/gpu/drm/i915/gvt/handlers.c +++ b/drivers/gpu/drm/i915/gvt/handlers.c @@ -132,12 +132,13 @@ static int new_mmio_info(struct intel_gvt *gvt, static int render_mmio_to_ring_id(struct intel_gvt *gvt, unsigned int reg) { - int i; + enum intel_engine_id id; + struct intel_engine_cs *engine; reg &= ~GENMASK(11, 0); - for (i = 0; i < I915_NUM_ENGINES; i++) { - if (gvt->dev_priv->engine[i]->mmio_base == reg) - return i; + for_each_engine(engine, gvt->dev_priv, id) { + if (engine->mmio_base == reg) + return id; } return -1; } @@ -1306,7 +1307,7 @@ static int elsp_mmio_write(struct intel_vgpu *vgpu, unsigned int offset, u32 data = *(u32 *)p_data; int ret; - if (WARN_ON(ring_id < 0)) + if (WARN_ON(ring_id < 0 || ring_id > I915_NUM_ENGINES - 1)) return -EINVAL; execlist = &vgpu->execlist[ring_id]; diff --git a/drivers/gpu/drm/i915/gvt/sched_policy.c b/drivers/gpu/drm/i915/gvt/sched_policy.c index 278db0c180fc9..b605ac6137ebf 100644 --- a/drivers/gpu/drm/i915/gvt/sched_policy.c +++ b/drivers/gpu/drm/i915/gvt/sched_policy.c @@ -37,9 +37,10 @@ static bool vgpu_has_pending_workload(struct intel_vgpu *vgpu) { struct intel_vgpu_execlist *execlist; - int i; + enum intel_engine_id i; + struct intel_engine_cs *engine; - for (i = 0; i < I915_NUM_ENGINES; i++) { + for_each_engine(engine, vgpu->gvt->dev_priv, i) { execlist = &vgpu->execlist[i]; if (!list_empty(workload_q_head(vgpu, i))) return true; @@ -51,7 +52,8 @@ static bool vgpu_has_pending_workload(struct intel_vgpu *vgpu) static void try_to_schedule_next_vgpu(struct intel_gvt *gvt) { struct intel_gvt_workload_scheduler *scheduler = &gvt->scheduler; - int i; + enum intel_engine_id i; + struct intel_engine_cs *engine; /* no target to schedule */ if (!scheduler->next_vgpu) @@ -67,7 +69,7 @@ static void try_to_schedule_next_vgpu(struct intel_gvt *gvt) scheduler->need_reschedule = true; /* still have uncompleted workload? */ - for (i = 0; i < I915_NUM_ENGINES; i++) { + for_each_engine(engine, gvt->dev_priv, i) { if (scheduler->current_workload[i]) { gvt_dbg_sched("still have running workload\n"); return; @@ -84,7 +86,7 @@ static void try_to_schedule_next_vgpu(struct intel_gvt *gvt) scheduler->need_reschedule = false; /* wake up workload dispatch thread */ - for (i = 0; i < I915_NUM_ENGINES; i++) + for_each_engine(engine, gvt->dev_priv, i) wake_up(&scheduler->waitq[i]); } diff --git a/drivers/gpu/drm/i915/gvt/scheduler.c b/drivers/gpu/drm/i915/gvt/scheduler.c index 12f825512e9be..a6ba60141ff45 100644 --- a/drivers/gpu/drm/i915/gvt/scheduler.c +++ b/drivers/gpu/drm/i915/gvt/scheduler.c @@ -510,6 +510,10 @@ int intel_gvt_init_workload_scheduler(struct intel_gvt *gvt) init_waitqueue_head(&scheduler->workload_complete_wq); for (i = 0; i < I915_NUM_ENGINES; i++) { + /* check ring mask at init time */ + if (!HAS_ENGINE(gvt->dev_priv, i)) + continue; + init_waitqueue_head(&scheduler->waitq[i]); param = kzalloc(sizeof(*param), GFP_KERNEL); From 321927db98320f0121adc50a8325f23e08735c34 Mon Sep 17 00:00:00 2001 From: "Du, Changbin" Date: Thu, 20 Oct 2016 14:08:46 +0800 Subject: [PATCH 16/19] drm/i915/gvt: fix sparse warnings on different address spaces Add proper __iomem annotation for pointers obtained via ioremap(). Signed-off-by: Du, Changbin Signed-off-by: Zhenyu Wang --- drivers/gpu/drm/i915/gvt/firmware.c | 8 +++++--- drivers/gpu/drm/i915/gvt/gtt.c | 4 ++-- drivers/gpu/drm/i915/gvt/gvt.h | 2 +- drivers/gpu/drm/i915/gvt/opregion.c | 2 +- 4 files changed, 9 insertions(+), 7 deletions(-) diff --git a/drivers/gpu/drm/i915/gvt/firmware.c b/drivers/gpu/drm/i915/gvt/firmware.c index d068a524a6992..2fae2a2ca96f6 100644 --- a/drivers/gpu/drm/i915/gvt/firmware.c +++ b/drivers/gpu/drm/i915/gvt/firmware.c @@ -51,7 +51,7 @@ struct gvt_firmware_header { #define RD(offset) (readl(mmio + offset.reg)) #define WR(v, offset) (writel(v, mmio + offset.reg)) -static void bdw_forcewake_get(void *mmio) +static void bdw_forcewake_get(void __iomem *mmio) { WR(_MASKED_BIT_DISABLE(0xffff), FORCEWAKE_MT); @@ -91,7 +91,8 @@ static struct bin_attribute firmware_attr = { .mmap = NULL, }; -static int expose_firmware_sysfs(struct intel_gvt *gvt, void *mmio) +static int expose_firmware_sysfs(struct intel_gvt *gvt, + void __iomem *mmio) { struct intel_gvt_device_info *info = &gvt->device_info; struct pci_dev *pdev = gvt->dev_priv->drm.pdev; @@ -234,7 +235,8 @@ int intel_gvt_load_firmware(struct intel_gvt *gvt) struct gvt_firmware_header *h; const struct firmware *fw; char *path; - void *mmio, *mem; + void __iomem *mmio; + void *mem; int ret; path = kmalloc(PATH_MAX, GFP_KERNEL); diff --git a/drivers/gpu/drm/i915/gvt/gtt.c b/drivers/gpu/drm/i915/gvt/gtt.c index 0722d1e61fcea..a8c2405f6f3e2 100644 --- a/drivers/gpu/drm/i915/gvt/gtt.c +++ b/drivers/gpu/drm/i915/gvt/gtt.c @@ -269,7 +269,7 @@ static inline int get_pse_type(int type) static u64 read_pte64(struct drm_i915_private *dev_priv, unsigned long index) { - void *addr = (u64 *)dev_priv->ggtt.gsm + index; + void __iomem *addr = (gen8_pte_t __iomem *)dev_priv->ggtt.gsm + index; u64 pte; #ifdef readq @@ -284,7 +284,7 @@ static u64 read_pte64(struct drm_i915_private *dev_priv, unsigned long index) static void write_pte64(struct drm_i915_private *dev_priv, unsigned long index, u64 pte) { - void *addr = (u64 *)dev_priv->ggtt.gsm + index; + void __iomem *addr = (gen8_pte_t __iomem *)dev_priv->ggtt.gsm + index; #ifdef writeq writeq(pte, addr); diff --git a/drivers/gpu/drm/i915/gvt/gvt.h b/drivers/gpu/drm/i915/gvt/gvt.h index 15c595e0a63b6..11df62b542b19 100644 --- a/drivers/gpu/drm/i915/gvt/gvt.h +++ b/drivers/gpu/drm/i915/gvt/gvt.h @@ -186,7 +186,7 @@ struct intel_gvt_firmware { }; struct intel_gvt_opregion { - void *opregion_va; + void __iomem *opregion_va; u32 opregion_pa; }; diff --git a/drivers/gpu/drm/i915/gvt/opregion.c b/drivers/gpu/drm/i915/gvt/opregion.c index 53ac81f63c645..973c8a9d0b153 100644 --- a/drivers/gpu/drm/i915/gvt/opregion.c +++ b/drivers/gpu/drm/i915/gvt/opregion.c @@ -27,7 +27,7 @@ static int init_vgpu_opregion(struct intel_vgpu *vgpu, u32 gpa) { - void *host_va = vgpu->gvt->opregion.opregion_va; + void __iomem *host_va = vgpu->gvt->opregion.opregion_va; u8 *buf; int i; From 999ccb4017c2c818afae18a90060385ec1db903b Mon Sep 17 00:00:00 2001 From: "Du, Changbin" Date: Thu, 20 Oct 2016 14:08:47 +0800 Subject: [PATCH 17/19] drm/i915/gvt: mark symbols static where possible Mark all local functions & variables as static. Signed-off-by: Du, Changbin Signed-off-by: Zhenyu Wang --- drivers/gpu/drm/i915/gvt/cmd_parser.c | 6 +++--- drivers/gpu/drm/i915/gvt/display.c | 2 +- drivers/gpu/drm/i915/gvt/execlist.c | 2 +- drivers/gpu/drm/i915/gvt/gtt.c | 8 ++++---- drivers/gpu/drm/i915/gvt/interrupt.c | 2 +- drivers/gpu/drm/i915/gvt/sched_policy.c | 2 +- drivers/gpu/drm/i915/gvt/scheduler.c | 3 ++- 7 files changed, 13 insertions(+), 12 deletions(-) diff --git a/drivers/gpu/drm/i915/gvt/cmd_parser.c b/drivers/gpu/drm/i915/gvt/cmd_parser.c index 0d322e035d9f8..aafb57e26288d 100644 --- a/drivers/gpu/drm/i915/gvt/cmd_parser.c +++ b/drivers/gpu/drm/i915/gvt/cmd_parser.c @@ -480,8 +480,8 @@ struct parser_exec_state { #define gmadr_dw_number(s) \ (s->vgpu->gvt->device_info.gmadr_bytes_in_cmd >> 2) -unsigned long bypass_scan_mask = 0; -bool bypass_batch_buffer_scan = true; +static unsigned long bypass_scan_mask = 0; +static bool bypass_batch_buffer_scan = true; /* ring ALL, type = 0 */ static struct sub_op_bits sub_op_mi[] = { @@ -960,7 +960,7 @@ struct cmd_interrupt_event { int mi_user_interrupt; }; -struct cmd_interrupt_event cmd_interrupt_events[] = { +static struct cmd_interrupt_event cmd_interrupt_events[] = { [RCS] = { .pipe_control_notify = RCS_PIPE_CONTROL, .mi_flush_dw = INTEL_GVT_EVENT_RESERVED, diff --git a/drivers/gpu/drm/i915/gvt/display.c b/drivers/gpu/drm/i915/gvt/display.c index d8908d4cd09a4..c0c884aeb30ea 100644 --- a/drivers/gpu/drm/i915/gvt/display.c +++ b/drivers/gpu/drm/i915/gvt/display.c @@ -120,7 +120,7 @@ static unsigned char virtual_dp_monitor_edid[] = { #define DPCD_HEADER_SIZE 0xb -u8 dpcd_fix_data[DPCD_HEADER_SIZE] = { +static u8 dpcd_fix_data[DPCD_HEADER_SIZE] = { 0x11, 0x0a, 0x04, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00 }; diff --git a/drivers/gpu/drm/i915/gvt/execlist.c b/drivers/gpu/drm/i915/gvt/execlist.c index 0e9b340897e3b..d251ca5d173c3 100644 --- a/drivers/gpu/drm/i915/gvt/execlist.c +++ b/drivers/gpu/drm/i915/gvt/execlist.c @@ -623,7 +623,7 @@ static int prepare_mm(struct intel_vgpu_workload *workload) (list_empty(q) ? NULL : container_of(q->prev, \ struct intel_vgpu_workload, list)) -bool submit_context(struct intel_vgpu *vgpu, int ring_id, +static bool submit_context(struct intel_vgpu *vgpu, int ring_id, struct execlist_ctx_descriptor_format *desc, bool emulate_schedule_in) { diff --git a/drivers/gpu/drm/i915/gvt/gtt.c b/drivers/gpu/drm/i915/gvt/gtt.c index a8c2405f6f3e2..d3230bea7e41b 100644 --- a/drivers/gpu/drm/i915/gvt/gtt.c +++ b/drivers/gpu/drm/i915/gvt/gtt.c @@ -1921,7 +1921,7 @@ int intel_vgpu_emulate_gtt_mmio_write(struct intel_vgpu *vgpu, unsigned int off, return ret; } -bool intel_gvt_create_scratch_page(struct intel_vgpu *vgpu) +static bool create_scratch_page(struct intel_vgpu *vgpu) { struct intel_vgpu_gtt *gtt = &vgpu->gtt; void *p; @@ -1955,7 +1955,7 @@ bool intel_gvt_create_scratch_page(struct intel_vgpu *vgpu) return 0; } -void intel_gvt_release_scratch_page(struct intel_vgpu *vgpu) +static void release_scratch_page(struct intel_vgpu *vgpu) { if (vgpu->gtt.scratch_page != NULL) { __free_page(vgpu->gtt.scratch_page); @@ -1995,7 +1995,7 @@ int intel_vgpu_init_gtt(struct intel_vgpu *vgpu) gtt->ggtt_mm = ggtt_mm; - intel_gvt_create_scratch_page(vgpu); + create_scratch_page(vgpu); return 0; } @@ -2015,7 +2015,7 @@ void intel_vgpu_clean_gtt(struct intel_vgpu *vgpu) struct intel_vgpu_mm *mm; ppgtt_free_all_shadow_page(vgpu); - intel_gvt_release_scratch_page(vgpu); + release_scratch_page(vgpu); list_for_each_safe(pos, n, &vgpu->gtt.mm_list_head) { mm = container_of(pos, struct intel_vgpu_mm, list); diff --git a/drivers/gpu/drm/i915/gvt/interrupt.c b/drivers/gpu/drm/i915/gvt/interrupt.c index e43ef72285574..f7be02ac4be19 100644 --- a/drivers/gpu/drm/i915/gvt/interrupt.c +++ b/drivers/gpu/drm/i915/gvt/interrupt.c @@ -50,7 +50,7 @@ static void update_upstream_irq(struct intel_vgpu *vgpu, struct intel_gvt_irq_info *info); -const char * const irq_name[INTEL_GVT_EVENT_MAX] = { +static const char * const irq_name[INTEL_GVT_EVENT_MAX] = { [RCS_MI_USER_INTERRUPT] = "Render CS MI USER INTERRUPT", [RCS_DEBUG] = "Render EU debug from SVG", [RCS_MMIO_SYNC_FLUSH] = "Render MMIO sync flush status", diff --git a/drivers/gpu/drm/i915/gvt/sched_policy.c b/drivers/gpu/drm/i915/gvt/sched_policy.c index b605ac6137ebf..1df6a5460f3e1 100644 --- a/drivers/gpu/drm/i915/gvt/sched_policy.c +++ b/drivers/gpu/drm/i915/gvt/sched_policy.c @@ -236,7 +236,7 @@ static void tbs_sched_stop_schedule(struct intel_vgpu *vgpu) list_del_init(&vgpu_data->list); } -struct intel_gvt_sched_policy_ops tbs_schedule_ops = { +static struct intel_gvt_sched_policy_ops tbs_schedule_ops = { .init = tbs_sched_init, .clean = tbs_sched_clean, .init_vgpu = tbs_sched_init_vgpu, diff --git a/drivers/gpu/drm/i915/gvt/scheduler.c b/drivers/gpu/drm/i915/gvt/scheduler.c index a6ba60141ff45..e96eaeebeb0a4 100644 --- a/drivers/gpu/drm/i915/gvt/scheduler.c +++ b/drivers/gpu/drm/i915/gvt/scheduler.c @@ -41,7 +41,8 @@ #define RING_CTX_OFF(x) \ offsetof(struct execlist_ring_context, x) -void set_context_pdp_root_pointer(struct execlist_ring_context *ring_context, +static void set_context_pdp_root_pointer( + struct execlist_ring_context *ring_context, u32 pdp[8]) { struct execlist_mmio_pair *pdp_pair = &ring_context->pdp3_UDW; From 76a79d59ada00fa22e5f8cd94b36296f395c3406 Mon Sep 17 00:00:00 2001 From: "Du, Changbin" Date: Thu, 20 Oct 2016 14:08:48 +0800 Subject: [PATCH 18/19] drm/i915/gvt: fix spare warnings on odd constant _Bool cast The function return values should has type int if it return a integer value. Signed-off-by: Du, Changbin Signed-off-by: Zhenyu Wang --- drivers/gpu/drm/i915/gvt/execlist.c | 2 +- drivers/gpu/drm/i915/gvt/gtt.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/i915/gvt/execlist.c b/drivers/gpu/drm/i915/gvt/execlist.c index d251ca5d173c3..c1f6019d88957 100644 --- a/drivers/gpu/drm/i915/gvt/execlist.c +++ b/drivers/gpu/drm/i915/gvt/execlist.c @@ -623,7 +623,7 @@ static int prepare_mm(struct intel_vgpu_workload *workload) (list_empty(q) ? NULL : container_of(q->prev, \ struct intel_vgpu_workload, list)) -static bool submit_context(struct intel_vgpu *vgpu, int ring_id, +static int submit_context(struct intel_vgpu *vgpu, int ring_id, struct execlist_ctx_descriptor_format *desc, bool emulate_schedule_in) { diff --git a/drivers/gpu/drm/i915/gvt/gtt.c b/drivers/gpu/drm/i915/gvt/gtt.c index d3230bea7e41b..ca15720707925 100644 --- a/drivers/gpu/drm/i915/gvt/gtt.c +++ b/drivers/gpu/drm/i915/gvt/gtt.c @@ -1921,7 +1921,7 @@ int intel_vgpu_emulate_gtt_mmio_write(struct intel_vgpu *vgpu, unsigned int off, return ret; } -static bool create_scratch_page(struct intel_vgpu *vgpu) +static int create_scratch_page(struct intel_vgpu *vgpu) { struct intel_vgpu_gtt *gtt = &vgpu->gtt; void *p; From 19e6393fb5366a89705a62b3276ce42e990d12ce Mon Sep 17 00:00:00 2001 From: "Du, Changbin" Date: Thu, 20 Oct 2016 14:08:49 +0800 Subject: [PATCH 19/19] drm/i915/gvt: do not ignore return value of create_scratch_page Function create_scratch_page() may fail in some cases. Signed-off-by: Du, Changbin Signed-off-by: Zhenyu Wang --- drivers/gpu/drm/i915/gvt/gtt.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/drivers/gpu/drm/i915/gvt/gtt.c b/drivers/gpu/drm/i915/gvt/gtt.c index ca15720707925..2cc761328569c 100644 --- a/drivers/gpu/drm/i915/gvt/gtt.c +++ b/drivers/gpu/drm/i915/gvt/gtt.c @@ -1995,8 +1995,7 @@ int intel_vgpu_init_gtt(struct intel_vgpu *vgpu) gtt->ggtt_mm = ggtt_mm; - create_scratch_page(vgpu); - return 0; + return create_scratch_page(vgpu); } /**