[v6,2/3] drm/i915/gvt: Use vgpu_lock to protect per vgpu access

Submitted by Colin Xu on April 20, 2018, 7:26 a.m.

Details

Message ID 20180420072627.23731-3-colin.xu@intel.com
State New
Headers show
Series "Split gvt lock into per-logic locks" ( rev: 3 ) in Intel GVT devel

Not browsing as part of any series.

Commit Message

Colin Xu April 20, 2018, 7:26 a.m.
From: Pei Zhang <pei.zhang@intel.com>

Use vgpu_lock to replace the gvt big lock. By doing this, the
mmio read/write trap path, vgpu virtual event emulation and other
vgpu related process, would be protected under per vgpu_lock.

v6:
  - Rebase to latest gvt-staging.
v5:
  - Rebase to latest gvt-staging.
  - intel_vgpu_page_track_handler should use vgpu_lock.
v4:
  - Rebase to latest gvt-staging.
  - Protect vgpu->active access with vgpu_lock.
  - Do not wait gpu idle in vgpu_lock.
v3: update to latest code base
v2: add gvt->lock in function gvt_check_vblank_emulation

Signed-off-by: Pei Zhang <pei.zhang@intel.com>
Signed-off-by: Colin Xu <colin.xu@intel.com>
---
 drivers/gpu/drm/i915/gvt/display.c    | 35 ++++++++++--------
 drivers/gpu/drm/i915/gvt/gvt.c        |  5 +--
 drivers/gpu/drm/i915/gvt/gvt.h        |  9 +++++
 drivers/gpu/drm/i915/gvt/handlers.c   |  4 ++
 drivers/gpu/drm/i915/gvt/mmio.c       | 12 +++---
 drivers/gpu/drm/i915/gvt/page_track.c |  5 +--
 drivers/gpu/drm/i915/gvt/scheduler.c  |  9 +++--
 drivers/gpu/drm/i915/gvt/vgpu.c       | 53 ++++++++++++++-------------
 8 files changed, 73 insertions(+), 59 deletions(-)

Patch hide | download patch | download mbox

diff --git a/drivers/gpu/drm/i915/gvt/display.c b/drivers/gpu/drm/i915/gvt/display.c
index 6d8180e8d1e2..8e4a63c5b7d1 100644
--- a/drivers/gpu/drm/i915/gvt/display.c
+++ b/drivers/gpu/drm/i915/gvt/display.c
@@ -337,26 +337,28 @@  void intel_gvt_check_vblank_emulation(struct intel_gvt *gvt)
 	struct intel_gvt_irq *irq = &gvt->irq;
 	struct intel_vgpu *vgpu;
 	int pipe, id;
+	int found = false;
 
-	if (WARN_ON(!mutex_is_locked(&gvt->lock)))
-		return;
-
+	mutex_lock(&gvt->lock);
 	for_each_active_vgpu(gvt, vgpu, id) {
 		for (pipe = 0; pipe < I915_MAX_PIPES; pipe++) {
-			if (pipe_is_enabled(vgpu, pipe))
-				goto out;
+			if (pipe_is_enabled(vgpu, pipe)) {
+				found = true;
+				break;
+			}
 		}
+		if (found)
+			break;
 	}
 
 	/* all the pipes are disabled */
-	hrtimer_cancel(&irq->vblank_timer.timer);
-	return;
-
-out:
-	hrtimer_start(&irq->vblank_timer.timer,
-		ktime_add_ns(ktime_get(), irq->vblank_timer.period),
-		HRTIMER_MODE_ABS);
-
+	if (!found)
+		hrtimer_cancel(&irq->vblank_timer.timer);
+	else
+		hrtimer_start(&irq->vblank_timer.timer,
+			ktime_add_ns(ktime_get(), irq->vblank_timer.period),
+			HRTIMER_MODE_ABS);
+	mutex_unlock(&gvt->lock);
 }
 
 static void emulate_vblank_on_pipe(struct intel_vgpu *vgpu, int pipe)
@@ -393,8 +395,10 @@  static void emulate_vblank(struct intel_vgpu *vgpu)
 {
 	int pipe;
 
+	mutex_lock(&vgpu->vgpu_lock);
 	for_each_pipe(vgpu->gvt->dev_priv, pipe)
 		emulate_vblank_on_pipe(vgpu, pipe);
+	mutex_unlock(&vgpu->vgpu_lock);
 }
 
 /**
@@ -409,11 +413,10 @@  void intel_gvt_emulate_vblank(struct intel_gvt *gvt)
 	struct intel_vgpu *vgpu;
 	int id;
 
-	if (WARN_ON(!mutex_is_locked(&gvt->lock)))
-		return;
-
+	mutex_lock(&gvt->lock);
 	for_each_active_vgpu(gvt, vgpu, id)
 		emulate_vblank(vgpu);
+	mutex_unlock(&gvt->lock);
 }
 
 /**
diff --git a/drivers/gpu/drm/i915/gvt/gvt.c b/drivers/gpu/drm/i915/gvt/gvt.c
index 91c37f115780..129a7795cb1f 100644
--- a/drivers/gpu/drm/i915/gvt/gvt.c
+++ b/drivers/gpu/drm/i915/gvt/gvt.c
@@ -271,11 +271,8 @@  static int gvt_service_thread(void *data)
 			continue;
 
 		if (test_and_clear_bit(INTEL_GVT_REQUEST_EMULATE_VBLANK,
-					(void *)&gvt->service_request)) {
-			mutex_lock(&gvt->lock);
+					(void *)&gvt->service_request))
 			intel_gvt_emulate_vblank(gvt);
-			mutex_unlock(&gvt->lock);
-		}
 
 		if (test_bit(INTEL_GVT_REQUEST_SCHED,
 				(void *)&gvt->service_request) ||
diff --git a/drivers/gpu/drm/i915/gvt/gvt.h b/drivers/gpu/drm/i915/gvt/gvt.h
index 8860343e850f..16a877c97e64 100644
--- a/drivers/gpu/drm/i915/gvt/gvt.h
+++ b/drivers/gpu/drm/i915/gvt/gvt.h
@@ -171,6 +171,7 @@  struct intel_vgpu_submission {
 
 struct intel_vgpu {
 	struct intel_gvt *gvt;
+	struct mutex vgpu_lock;
 	int id;
 	unsigned long handle; /* vGPU handle used by hypervisor MPT modules */
 	bool active;
@@ -178,6 +179,7 @@  struct intel_vgpu {
 	bool failsafe;
 	unsigned int resetting_eng;
 	void *sched_data;
+
 	struct vgpu_sched_ctl sched_ctl;
 
 	struct intel_vgpu_fence fence;
@@ -295,6 +297,9 @@  struct intel_vgpu_type {
 };
 
 struct intel_gvt {
+	/* GVT scope lock, protect GVT itself, and all resource currently
+	 * not yet protected by special locks(vgpu and scheduler lock).
+	 */
 	struct mutex lock;
 	struct mutex gtt_lock;
 
@@ -317,6 +322,10 @@  struct intel_gvt {
 
 	struct task_struct *service_thread;
 	wait_queue_head_t service_thread_wq;
+
+	/* service_request is always used in bit operation, we should always
+	 * use it with atomic bit ops so that no need to use gvt big lock.
+	 */
 	unsigned long service_request;
 
 	struct {
diff --git a/drivers/gpu/drm/i915/gvt/handlers.c b/drivers/gpu/drm/i915/gvt/handlers.c
index b77adcca0d4a..16f6f92b0ca9 100644
--- a/drivers/gpu/drm/i915/gvt/handlers.c
+++ b/drivers/gpu/drm/i915/gvt/handlers.c
@@ -319,6 +319,7 @@  static int gdrst_mmio_write(struct intel_vgpu *vgpu, unsigned int offset,
 		}
 	}
 
+	// vgpu_lock already hold by emulate mmio r/w
 	intel_gvt_reset_vgpu_locked(vgpu, false, engine_mask);
 
 	/* sw will wait for the device to ack the reset request */
@@ -423,7 +424,10 @@  static int pipeconf_mmio_write(struct intel_vgpu *vgpu, unsigned int offset,
 		vgpu_vreg(vgpu, offset) |= I965_PIPECONF_ACTIVE;
 	else
 		vgpu_vreg(vgpu, offset) &= ~I965_PIPECONF_ACTIVE;
+	// vgpu_lock already hold by emulate mmio r/w
+	mutex_unlock(&vgpu->vgpu_lock);
 	intel_gvt_check_vblank_emulation(vgpu->gvt);
+	mutex_lock(&vgpu->vgpu_lock);
 	return 0;
 }
 
diff --git a/drivers/gpu/drm/i915/gvt/mmio.c b/drivers/gpu/drm/i915/gvt/mmio.c
index 11b71b33f1c0..4104d71f0764 100644
--- a/drivers/gpu/drm/i915/gvt/mmio.c
+++ b/drivers/gpu/drm/i915/gvt/mmio.c
@@ -67,7 +67,7 @@  static void failsafe_emulate_mmio_rw(struct intel_vgpu *vgpu, uint64_t pa,
 		return;
 
 	gvt = vgpu->gvt;
-	mutex_lock(&gvt->lock);
+	mutex_lock(&vgpu->vgpu_lock);
 	offset = intel_vgpu_gpa_to_mmio_offset(vgpu, pa);
 	if (reg_is_mmio(gvt, offset)) {
 		if (read)
@@ -85,7 +85,7 @@  static void failsafe_emulate_mmio_rw(struct intel_vgpu *vgpu, uint64_t pa,
 			memcpy(pt, p_data, bytes);
 
 	}
-	mutex_unlock(&gvt->lock);
+	mutex_unlock(&vgpu->vgpu_lock);
 }
 
 /**
@@ -109,7 +109,7 @@  int intel_vgpu_emulate_mmio_read(struct intel_vgpu *vgpu, uint64_t pa,
 		failsafe_emulate_mmio_rw(vgpu, pa, p_data, bytes, true);
 		return 0;
 	}
-	mutex_lock(&gvt->lock);
+	mutex_lock(&vgpu->vgpu_lock);
 
 	offset = intel_vgpu_gpa_to_mmio_offset(vgpu, pa);
 
@@ -156,7 +156,7 @@  int intel_vgpu_emulate_mmio_read(struct intel_vgpu *vgpu, uint64_t pa,
 	gvt_vgpu_err("fail to emulate MMIO read %08x len %d\n",
 			offset, bytes);
 out:
-	mutex_unlock(&gvt->lock);
+	mutex_unlock(&vgpu->vgpu_lock);
 	return ret;
 }
 
@@ -182,7 +182,7 @@  int intel_vgpu_emulate_mmio_write(struct intel_vgpu *vgpu, uint64_t pa,
 		return 0;
 	}
 
-	mutex_lock(&gvt->lock);
+	mutex_lock(&vgpu->vgpu_lock);
 
 	offset = intel_vgpu_gpa_to_mmio_offset(vgpu, pa);
 
@@ -220,7 +220,7 @@  int intel_vgpu_emulate_mmio_write(struct intel_vgpu *vgpu, uint64_t pa,
 	gvt_vgpu_err("fail to emulate MMIO write %08x len %d\n", offset,
 		     bytes);
 out:
-	mutex_unlock(&gvt->lock);
+	mutex_unlock(&vgpu->vgpu_lock);
 	return ret;
 }
 
diff --git a/drivers/gpu/drm/i915/gvt/page_track.c b/drivers/gpu/drm/i915/gvt/page_track.c
index 53e2bd79c97d..256d0db8bbb1 100644
--- a/drivers/gpu/drm/i915/gvt/page_track.c
+++ b/drivers/gpu/drm/i915/gvt/page_track.c
@@ -157,11 +157,10 @@  int intel_vgpu_disable_page_track(struct intel_vgpu *vgpu, unsigned long gfn)
 int intel_vgpu_page_track_handler(struct intel_vgpu *vgpu, u64 gpa,
 		void *data, unsigned int bytes)
 {
-	struct intel_gvt *gvt = vgpu->gvt;
 	struct intel_vgpu_page_track *page_track;
 	int ret = 0;
 
-	mutex_lock(&gvt->lock);
+	mutex_lock(&vgpu->vgpu_lock);
 
 	page_track = intel_vgpu_find_page_track(vgpu, gpa >> PAGE_SHIFT);
 	if (!page_track) {
@@ -179,6 +178,6 @@  int intel_vgpu_page_track_handler(struct intel_vgpu *vgpu, u64 gpa,
 	}
 
 out:
-	mutex_unlock(&gvt->lock);
+	mutex_unlock(&vgpu->vgpu_lock);
 	return ret;
 }
diff --git a/drivers/gpu/drm/i915/gvt/scheduler.c b/drivers/gpu/drm/i915/gvt/scheduler.c
index 080fb5027d9c..95664d2f0bfb 100644
--- a/drivers/gpu/drm/i915/gvt/scheduler.c
+++ b/drivers/gpu/drm/i915/gvt/scheduler.c
@@ -680,6 +680,7 @@  static int dispatch_workload(struct intel_vgpu_workload *workload)
 	gvt_dbg_sched("ring id %d prepare to dispatch workload %p\n",
 		ring_id, workload);
 
+	mutex_lock(&vgpu->vgpu_lock);
 	mutex_lock(&dev_priv->drm.struct_mutex);
 
 	ret = intel_gvt_scan_and_shadow_workload(workload);
@@ -704,6 +705,7 @@  static int dispatch_workload(struct intel_vgpu_workload *workload)
 	}
 
 	mutex_unlock(&dev_priv->drm.struct_mutex);
+	mutex_unlock(&vgpu->vgpu_lock);
 	return ret;
 }
 
@@ -861,14 +863,14 @@  static void complete_current_workload(struct intel_gvt *gvt, int ring_id)
 	int event;
 
 	mutex_lock(&gvt->lock);
+	mutex_lock(&vgpu->vgpu_lock);
 
 	/* For the workload w/ request, needs to wait for the context
 	 * switch to make sure request is completed.
 	 * For the workload w/o request, directly complete the workload.
 	 */
 	if (workload->req) {
-		struct drm_i915_private *dev_priv =
-			workload->vgpu->gvt->dev_priv;
+		struct drm_i915_private *dev_priv = vgpu->gvt->dev_priv;
 		struct intel_engine_cs *engine =
 			dev_priv->engine[workload->ring_id];
 		wait_event(workload->shadow_ctx_status_wq,
@@ -939,6 +941,7 @@  static void complete_current_workload(struct intel_gvt *gvt, int ring_id)
 	if (gvt->scheduler.need_reschedule)
 		intel_gvt_request_service(gvt, INTEL_GVT_REQUEST_EVENT_SCHED);
 
+	mutex_unlock(&vgpu->vgpu_lock);
 	mutex_unlock(&gvt->lock);
 }
 
@@ -991,9 +994,7 @@  static int workload_thread(void *priv)
 			intel_uncore_forcewake_get(gvt->dev_priv,
 					FORCEWAKE_ALL);
 
-		mutex_lock(&gvt->lock);
 		ret = dispatch_workload(workload);
-		mutex_unlock(&gvt->lock);
 
 		if (ret) {
 			vgpu = workload->vgpu;
diff --git a/drivers/gpu/drm/i915/gvt/vgpu.c b/drivers/gpu/drm/i915/gvt/vgpu.c
index 2e0a02a80fe4..948b8a94c814 100644
--- a/drivers/gpu/drm/i915/gvt/vgpu.c
+++ b/drivers/gpu/drm/i915/gvt/vgpu.c
@@ -223,22 +223,20 @@  void intel_gvt_activate_vgpu(struct intel_vgpu *vgpu)
  */
 void intel_gvt_deactivate_vgpu(struct intel_vgpu *vgpu)
 {
-	struct intel_gvt *gvt = vgpu->gvt;
-
-	mutex_lock(&gvt->lock);
+	mutex_lock(&vgpu->vgpu_lock);
 
 	vgpu->active = false;
 
 	if (atomic_read(&vgpu->submission.running_workload_num)) {
-		mutex_unlock(&gvt->lock);
+		mutex_unlock(&vgpu->gvt->lock);
 		intel_gvt_wait_vgpu_idle(vgpu);
-		mutex_lock(&gvt->lock);
+		mutex_lock(&vgpu->gvt->lock);
 	}
 
 	intel_vgpu_stop_schedule(vgpu);
 	intel_vgpu_dmabuf_cleanup(vgpu);
 
-	mutex_unlock(&gvt->lock);
+	mutex_unlock(&vgpu->vgpu_lock);
 }
 
 /**
@@ -252,14 +250,11 @@  void intel_gvt_destroy_vgpu(struct intel_vgpu *vgpu)
 {
 	struct intel_gvt *gvt = vgpu->gvt;
 
-	mutex_lock(&gvt->lock);
+	mutex_lock(&vgpu->vgpu_lock);
 
 	WARN(vgpu->active, "vGPU is still active!\n");
 
 	intel_gvt_debugfs_remove_vgpu(vgpu);
-	idr_remove(&gvt->vgpu_idr, vgpu->id);
-	if (idr_is_empty(&gvt->vgpu_idr))
-		intel_gvt_clean_irq(gvt);
 	intel_vgpu_clean_sched_policy(vgpu);
 	intel_vgpu_clean_submission(vgpu);
 	intel_vgpu_clean_display(vgpu);
@@ -269,10 +264,16 @@  void intel_gvt_destroy_vgpu(struct intel_vgpu *vgpu)
 	intel_vgpu_free_resource(vgpu);
 	intel_vgpu_clean_mmio(vgpu);
 	intel_vgpu_dmabuf_cleanup(vgpu);
-	vfree(vgpu);
+	mutex_unlock(&vgpu->vgpu_lock);
 
+	mutex_lock(&gvt->lock);
+	idr_remove(&gvt->vgpu_idr, vgpu->id);
+	if (idr_is_empty(&gvt->vgpu_idr))
+		intel_gvt_clean_irq(gvt);
 	intel_gvt_update_vgpu_types(gvt);
 	mutex_unlock(&gvt->lock);
+
+	vfree(vgpu);
 }
 
 #define IDLE_VGPU_IDR 0
@@ -298,6 +299,7 @@  struct intel_vgpu *intel_gvt_create_idle_vgpu(struct intel_gvt *gvt)
 
 	vgpu->id = IDLE_VGPU_IDR;
 	vgpu->gvt = gvt;
+	mutex_init(&vgpu->vgpu_lock);
 
 	for (i = 0; i < I915_NUM_ENGINES; i++)
 		INIT_LIST_HEAD(&vgpu->submission.workload_q_head[i]);
@@ -324,7 +326,10 @@  struct intel_vgpu *intel_gvt_create_idle_vgpu(struct intel_gvt *gvt)
  */
 void intel_gvt_destroy_idle_vgpu(struct intel_vgpu *vgpu)
 {
+	mutex_lock(&vgpu->vgpu_lock);
 	intel_vgpu_clean_sched_policy(vgpu);
+	mutex_unlock(&vgpu->vgpu_lock);
+
 	vfree(vgpu);
 }
 
@@ -342,8 +347,6 @@  static struct intel_vgpu *__intel_gvt_create_vgpu(struct intel_gvt *gvt,
 	if (!vgpu)
 		return ERR_PTR(-ENOMEM);
 
-	mutex_lock(&gvt->lock);
-
 	ret = idr_alloc(&gvt->vgpu_idr, vgpu, IDLE_VGPU_IDR + 1, GVT_MAX_VGPU,
 		GFP_KERNEL);
 	if (ret < 0)
@@ -353,6 +356,7 @@  static struct intel_vgpu *__intel_gvt_create_vgpu(struct intel_gvt *gvt,
 	vgpu->handle = param->handle;
 	vgpu->gvt = gvt;
 	vgpu->sched_ctl.weight = param->weight;
+	mutex_init(&vgpu->vgpu_lock);
 	INIT_LIST_HEAD(&vgpu->dmabuf_obj_list_head);
 	INIT_RADIX_TREE(&vgpu->page_track_tree, GFP_KERNEL);
 	idr_init(&vgpu->object_idr);
@@ -400,8 +404,6 @@  static struct intel_vgpu *__intel_gvt_create_vgpu(struct intel_gvt *gvt,
 	if (ret)
 		goto out_clean_sched_policy;
 
-	mutex_unlock(&gvt->lock);
-
 	return vgpu;
 
 out_clean_sched_policy:
@@ -424,7 +426,6 @@  static struct intel_vgpu *__intel_gvt_create_vgpu(struct intel_gvt *gvt,
 	idr_remove(&gvt->vgpu_idr, vgpu->id);
 out_free_vgpu:
 	vfree(vgpu);
-	mutex_unlock(&gvt->lock);
 	return ERR_PTR(ret);
 }
 
@@ -456,12 +457,12 @@  struct intel_vgpu *intel_gvt_create_vgpu(struct intel_gvt *gvt,
 	param.low_gm_sz = BYTES_TO_MB(param.low_gm_sz);
 	param.high_gm_sz = BYTES_TO_MB(param.high_gm_sz);
 
+	mutex_lock(&gvt->lock);
 	vgpu = __intel_gvt_create_vgpu(gvt, &param);
-	if (IS_ERR(vgpu))
-		return vgpu;
-
-	/* calculate left instance change for types */
-	intel_gvt_update_vgpu_types(gvt);
+	if (!IS_ERR(vgpu))
+		/* calculate left instance change for types */
+		intel_gvt_update_vgpu_types(gvt);
+	mutex_unlock(&gvt->lock);
 
 	return vgpu;
 }
@@ -473,7 +474,7 @@  struct intel_vgpu *intel_gvt_create_vgpu(struct intel_gvt *gvt,
  * @engine_mask: engines to reset for GT reset
  *
  * This function is called when user wants to reset a virtual GPU through
- * device model reset or GT reset. The caller should hold the gvt lock.
+ * device model reset or GT reset. The caller should hold the vgpu lock.
  *
  * vGPU Device Model Level Reset (DMLR) simulates the PCI level reset to reset
  * the whole vGPU to default state as when it is created. This vGPU function
@@ -513,9 +514,9 @@  void intel_gvt_reset_vgpu_locked(struct intel_vgpu *vgpu, bool dmlr,
 	 * scheduler when the reset is triggered by current vgpu.
 	 */
 	if (scheduler->current_vgpu == NULL) {
-		mutex_unlock(&gvt->lock);
+		mutex_unlock(&vgpu->vgpu_lock);
 		intel_gvt_wait_vgpu_idle(vgpu);
-		mutex_lock(&gvt->lock);
+		mutex_lock(&vgpu->vgpu_lock);
 	}
 
 	intel_vgpu_reset_submission(vgpu, resetting_eng);
@@ -555,7 +556,7 @@  void intel_gvt_reset_vgpu_locked(struct intel_vgpu *vgpu, bool dmlr,
  */
 void intel_gvt_reset_vgpu(struct intel_vgpu *vgpu)
 {
-	mutex_lock(&vgpu->gvt->lock);
+	mutex_lock(&vgpu->vgpu_lock);
 	intel_gvt_reset_vgpu_locked(vgpu, true, 0);
-	mutex_unlock(&vgpu->gvt->lock);
+	mutex_unlock(&vgpu->vgpu_lock);
 }

Comments

Reviewed-by: Changbin Du <changbin.du@intel.com>

On Fri, Apr 20, 2018 at 03:26:26PM +0800, colin.xu@intel.com wrote:
> From: Pei Zhang <pei.zhang@intel.com>
> 
> Use vgpu_lock to replace the gvt big lock. By doing this, the
> mmio read/write trap path, vgpu virtual event emulation and other
> vgpu related process, would be protected under per vgpu_lock.
> 
> v6:
>   - Rebase to latest gvt-staging.
> v5:
>   - Rebase to latest gvt-staging.
>   - intel_vgpu_page_track_handler should use vgpu_lock.
> v4:
>   - Rebase to latest gvt-staging.
>   - Protect vgpu->active access with vgpu_lock.
>   - Do not wait gpu idle in vgpu_lock.
> v3: update to latest code base
> v2: add gvt->lock in function gvt_check_vblank_emulation
> 
> Signed-off-by: Pei Zhang <pei.zhang@intel.com>
> Signed-off-by: Colin Xu <colin.xu@intel.com>
> ---
>  drivers/gpu/drm/i915/gvt/display.c    | 35 ++++++++++--------
>  drivers/gpu/drm/i915/gvt/gvt.c        |  5 +--
>  drivers/gpu/drm/i915/gvt/gvt.h        |  9 +++++
>  drivers/gpu/drm/i915/gvt/handlers.c   |  4 ++
>  drivers/gpu/drm/i915/gvt/mmio.c       | 12 +++---
>  drivers/gpu/drm/i915/gvt/page_track.c |  5 +--
>  drivers/gpu/drm/i915/gvt/scheduler.c  |  9 +++--
>  drivers/gpu/drm/i915/gvt/vgpu.c       | 53 ++++++++++++++-------------
>  8 files changed, 73 insertions(+), 59 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/gvt/display.c b/drivers/gpu/drm/i915/gvt/display.c
> index 6d8180e8d1e2..8e4a63c5b7d1 100644
> --- a/drivers/gpu/drm/i915/gvt/display.c
> +++ b/drivers/gpu/drm/i915/gvt/display.c
> @@ -337,26 +337,28 @@ void intel_gvt_check_vblank_emulation(struct intel_gvt *gvt)
>  	struct intel_gvt_irq *irq = &gvt->irq;
>  	struct intel_vgpu *vgpu;
>  	int pipe, id;
> +	int found = false;
>  
> -	if (WARN_ON(!mutex_is_locked(&gvt->lock)))
> -		return;
> -
> +	mutex_lock(&gvt->lock);
>  	for_each_active_vgpu(gvt, vgpu, id) {
>  		for (pipe = 0; pipe < I915_MAX_PIPES; pipe++) {
> -			if (pipe_is_enabled(vgpu, pipe))
> -				goto out;
> +			if (pipe_is_enabled(vgpu, pipe)) {
> +				found = true;
> +				break;
> +			}
>  		}
> +		if (found)
> +			break;
>  	}
>  
>  	/* all the pipes are disabled */
> -	hrtimer_cancel(&irq->vblank_timer.timer);
> -	return;
> -
> -out:
> -	hrtimer_start(&irq->vblank_timer.timer,
> -		ktime_add_ns(ktime_get(), irq->vblank_timer.period),
> -		HRTIMER_MODE_ABS);
> -
> +	if (!found)
> +		hrtimer_cancel(&irq->vblank_timer.timer);
> +	else
> +		hrtimer_start(&irq->vblank_timer.timer,
> +			ktime_add_ns(ktime_get(), irq->vblank_timer.period),
> +			HRTIMER_MODE_ABS);
> +	mutex_unlock(&gvt->lock);
>  }
>  
>  static void emulate_vblank_on_pipe(struct intel_vgpu *vgpu, int pipe)
> @@ -393,8 +395,10 @@ static void emulate_vblank(struct intel_vgpu *vgpu)
>  {
>  	int pipe;
>  
> +	mutex_lock(&vgpu->vgpu_lock);
>  	for_each_pipe(vgpu->gvt->dev_priv, pipe)
>  		emulate_vblank_on_pipe(vgpu, pipe);
> +	mutex_unlock(&vgpu->vgpu_lock);
>  }
>  
>  /**
> @@ -409,11 +413,10 @@ void intel_gvt_emulate_vblank(struct intel_gvt *gvt)
>  	struct intel_vgpu *vgpu;
>  	int id;
>  
> -	if (WARN_ON(!mutex_is_locked(&gvt->lock)))
> -		return;
> -
> +	mutex_lock(&gvt->lock);
>  	for_each_active_vgpu(gvt, vgpu, id)
>  		emulate_vblank(vgpu);
> +	mutex_unlock(&gvt->lock);
>  }
>  
>  /**
> diff --git a/drivers/gpu/drm/i915/gvt/gvt.c b/drivers/gpu/drm/i915/gvt/gvt.c
> index 91c37f115780..129a7795cb1f 100644
> --- a/drivers/gpu/drm/i915/gvt/gvt.c
> +++ b/drivers/gpu/drm/i915/gvt/gvt.c
> @@ -271,11 +271,8 @@ static int gvt_service_thread(void *data)
>  			continue;
>  
>  		if (test_and_clear_bit(INTEL_GVT_REQUEST_EMULATE_VBLANK,
> -					(void *)&gvt->service_request)) {
> -			mutex_lock(&gvt->lock);
> +					(void *)&gvt->service_request))
>  			intel_gvt_emulate_vblank(gvt);
> -			mutex_unlock(&gvt->lock);
> -		}
>  
>  		if (test_bit(INTEL_GVT_REQUEST_SCHED,
>  				(void *)&gvt->service_request) ||
> diff --git a/drivers/gpu/drm/i915/gvt/gvt.h b/drivers/gpu/drm/i915/gvt/gvt.h
> index 8860343e850f..16a877c97e64 100644
> --- a/drivers/gpu/drm/i915/gvt/gvt.h
> +++ b/drivers/gpu/drm/i915/gvt/gvt.h
> @@ -171,6 +171,7 @@ struct intel_vgpu_submission {
>  
>  struct intel_vgpu {
>  	struct intel_gvt *gvt;
> +	struct mutex vgpu_lock;
>  	int id;
>  	unsigned long handle; /* vGPU handle used by hypervisor MPT modules */
>  	bool active;
> @@ -178,6 +179,7 @@ struct intel_vgpu {
>  	bool failsafe;
>  	unsigned int resetting_eng;
>  	void *sched_data;
> +
>  	struct vgpu_sched_ctl sched_ctl;
>  
>  	struct intel_vgpu_fence fence;
> @@ -295,6 +297,9 @@ struct intel_vgpu_type {
>  };
>  
>  struct intel_gvt {
> +	/* GVT scope lock, protect GVT itself, and all resource currently
> +	 * not yet protected by special locks(vgpu and scheduler lock).
> +	 */
>  	struct mutex lock;
>  	struct mutex gtt_lock;
>  
> @@ -317,6 +322,10 @@ struct intel_gvt {
>  
>  	struct task_struct *service_thread;
>  	wait_queue_head_t service_thread_wq;
> +
> +	/* service_request is always used in bit operation, we should always
> +	 * use it with atomic bit ops so that no need to use gvt big lock.
> +	 */
>  	unsigned long service_request;
>  
>  	struct {
> diff --git a/drivers/gpu/drm/i915/gvt/handlers.c b/drivers/gpu/drm/i915/gvt/handlers.c
> index b77adcca0d4a..16f6f92b0ca9 100644
> --- a/drivers/gpu/drm/i915/gvt/handlers.c
> +++ b/drivers/gpu/drm/i915/gvt/handlers.c
> @@ -319,6 +319,7 @@ static int gdrst_mmio_write(struct intel_vgpu *vgpu, unsigned int offset,
>  		}
>  	}
>  
> +	// vgpu_lock already hold by emulate mmio r/w
>  	intel_gvt_reset_vgpu_locked(vgpu, false, engine_mask);
>  
>  	/* sw will wait for the device to ack the reset request */
> @@ -423,7 +424,10 @@ static int pipeconf_mmio_write(struct intel_vgpu *vgpu, unsigned int offset,
>  		vgpu_vreg(vgpu, offset) |= I965_PIPECONF_ACTIVE;
>  	else
>  		vgpu_vreg(vgpu, offset) &= ~I965_PIPECONF_ACTIVE;
> +	// vgpu_lock already hold by emulate mmio r/w
> +	mutex_unlock(&vgpu->vgpu_lock);
>  	intel_gvt_check_vblank_emulation(vgpu->gvt);
> +	mutex_lock(&vgpu->vgpu_lock);
>  	return 0;
>  }
>  
> diff --git a/drivers/gpu/drm/i915/gvt/mmio.c b/drivers/gpu/drm/i915/gvt/mmio.c
> index 11b71b33f1c0..4104d71f0764 100644
> --- a/drivers/gpu/drm/i915/gvt/mmio.c
> +++ b/drivers/gpu/drm/i915/gvt/mmio.c
> @@ -67,7 +67,7 @@ static void failsafe_emulate_mmio_rw(struct intel_vgpu *vgpu, uint64_t pa,
>  		return;
>  
>  	gvt = vgpu->gvt;
> -	mutex_lock(&gvt->lock);
> +	mutex_lock(&vgpu->vgpu_lock);
>  	offset = intel_vgpu_gpa_to_mmio_offset(vgpu, pa);
>  	if (reg_is_mmio(gvt, offset)) {
>  		if (read)
> @@ -85,7 +85,7 @@ static void failsafe_emulate_mmio_rw(struct intel_vgpu *vgpu, uint64_t pa,
>  			memcpy(pt, p_data, bytes);
>  
>  	}
> -	mutex_unlock(&gvt->lock);
> +	mutex_unlock(&vgpu->vgpu_lock);
>  }
>  
>  /**
> @@ -109,7 +109,7 @@ int intel_vgpu_emulate_mmio_read(struct intel_vgpu *vgpu, uint64_t pa,
>  		failsafe_emulate_mmio_rw(vgpu, pa, p_data, bytes, true);
>  		return 0;
>  	}
> -	mutex_lock(&gvt->lock);
> +	mutex_lock(&vgpu->vgpu_lock);
>  
>  	offset = intel_vgpu_gpa_to_mmio_offset(vgpu, pa);
>  
> @@ -156,7 +156,7 @@ int intel_vgpu_emulate_mmio_read(struct intel_vgpu *vgpu, uint64_t pa,
>  	gvt_vgpu_err("fail to emulate MMIO read %08x len %d\n",
>  			offset, bytes);
>  out:
> -	mutex_unlock(&gvt->lock);
> +	mutex_unlock(&vgpu->vgpu_lock);
>  	return ret;
>  }
>  
> @@ -182,7 +182,7 @@ int intel_vgpu_emulate_mmio_write(struct intel_vgpu *vgpu, uint64_t pa,
>  		return 0;
>  	}
>  
> -	mutex_lock(&gvt->lock);
> +	mutex_lock(&vgpu->vgpu_lock);
>  
>  	offset = intel_vgpu_gpa_to_mmio_offset(vgpu, pa);
>  
> @@ -220,7 +220,7 @@ int intel_vgpu_emulate_mmio_write(struct intel_vgpu *vgpu, uint64_t pa,
>  	gvt_vgpu_err("fail to emulate MMIO write %08x len %d\n", offset,
>  		     bytes);
>  out:
> -	mutex_unlock(&gvt->lock);
> +	mutex_unlock(&vgpu->vgpu_lock);
>  	return ret;
>  }
>  
> diff --git a/drivers/gpu/drm/i915/gvt/page_track.c b/drivers/gpu/drm/i915/gvt/page_track.c
> index 53e2bd79c97d..256d0db8bbb1 100644
> --- a/drivers/gpu/drm/i915/gvt/page_track.c
> +++ b/drivers/gpu/drm/i915/gvt/page_track.c
> @@ -157,11 +157,10 @@ int intel_vgpu_disable_page_track(struct intel_vgpu *vgpu, unsigned long gfn)
>  int intel_vgpu_page_track_handler(struct intel_vgpu *vgpu, u64 gpa,
>  		void *data, unsigned int bytes)
>  {
> -	struct intel_gvt *gvt = vgpu->gvt;
>  	struct intel_vgpu_page_track *page_track;
>  	int ret = 0;
>  
> -	mutex_lock(&gvt->lock);
> +	mutex_lock(&vgpu->vgpu_lock);
>  
>  	page_track = intel_vgpu_find_page_track(vgpu, gpa >> PAGE_SHIFT);
>  	if (!page_track) {
> @@ -179,6 +178,6 @@ int intel_vgpu_page_track_handler(struct intel_vgpu *vgpu, u64 gpa,
>  	}
>  
>  out:
> -	mutex_unlock(&gvt->lock);
> +	mutex_unlock(&vgpu->vgpu_lock);
>  	return ret;
>  }
> diff --git a/drivers/gpu/drm/i915/gvt/scheduler.c b/drivers/gpu/drm/i915/gvt/scheduler.c
> index 080fb5027d9c..95664d2f0bfb 100644
> --- a/drivers/gpu/drm/i915/gvt/scheduler.c
> +++ b/drivers/gpu/drm/i915/gvt/scheduler.c
> @@ -680,6 +680,7 @@ static int dispatch_workload(struct intel_vgpu_workload *workload)
>  	gvt_dbg_sched("ring id %d prepare to dispatch workload %p\n",
>  		ring_id, workload);
>  
> +	mutex_lock(&vgpu->vgpu_lock);
>  	mutex_lock(&dev_priv->drm.struct_mutex);
>  
>  	ret = intel_gvt_scan_and_shadow_workload(workload);
> @@ -704,6 +705,7 @@ static int dispatch_workload(struct intel_vgpu_workload *workload)
>  	}
>  
>  	mutex_unlock(&dev_priv->drm.struct_mutex);
> +	mutex_unlock(&vgpu->vgpu_lock);
>  	return ret;
>  }
>  
> @@ -861,14 +863,14 @@ static void complete_current_workload(struct intel_gvt *gvt, int ring_id)
>  	int event;
>  
>  	mutex_lock(&gvt->lock);
> +	mutex_lock(&vgpu->vgpu_lock);
>  
>  	/* For the workload w/ request, needs to wait for the context
>  	 * switch to make sure request is completed.
>  	 * For the workload w/o request, directly complete the workload.
>  	 */
>  	if (workload->req) {
> -		struct drm_i915_private *dev_priv =
> -			workload->vgpu->gvt->dev_priv;
> +		struct drm_i915_private *dev_priv = vgpu->gvt->dev_priv;
>  		struct intel_engine_cs *engine =
>  			dev_priv->engine[workload->ring_id];
>  		wait_event(workload->shadow_ctx_status_wq,
> @@ -939,6 +941,7 @@ static void complete_current_workload(struct intel_gvt *gvt, int ring_id)
>  	if (gvt->scheduler.need_reschedule)
>  		intel_gvt_request_service(gvt, INTEL_GVT_REQUEST_EVENT_SCHED);
>  
> +	mutex_unlock(&vgpu->vgpu_lock);
>  	mutex_unlock(&gvt->lock);
>  }
>  
> @@ -991,9 +994,7 @@ static int workload_thread(void *priv)
>  			intel_uncore_forcewake_get(gvt->dev_priv,
>  					FORCEWAKE_ALL);
>  
> -		mutex_lock(&gvt->lock);
>  		ret = dispatch_workload(workload);
> -		mutex_unlock(&gvt->lock);
>  
>  		if (ret) {
>  			vgpu = workload->vgpu;
> diff --git a/drivers/gpu/drm/i915/gvt/vgpu.c b/drivers/gpu/drm/i915/gvt/vgpu.c
> index 2e0a02a80fe4..948b8a94c814 100644
> --- a/drivers/gpu/drm/i915/gvt/vgpu.c
> +++ b/drivers/gpu/drm/i915/gvt/vgpu.c
> @@ -223,22 +223,20 @@ void intel_gvt_activate_vgpu(struct intel_vgpu *vgpu)
>   */
>  void intel_gvt_deactivate_vgpu(struct intel_vgpu *vgpu)
>  {
> -	struct intel_gvt *gvt = vgpu->gvt;
> -
> -	mutex_lock(&gvt->lock);
> +	mutex_lock(&vgpu->vgpu_lock);
>  
>  	vgpu->active = false;
>  
>  	if (atomic_read(&vgpu->submission.running_workload_num)) {
> -		mutex_unlock(&gvt->lock);
> +		mutex_unlock(&vgpu->gvt->lock);
>  		intel_gvt_wait_vgpu_idle(vgpu);
> -		mutex_lock(&gvt->lock);
> +		mutex_lock(&vgpu->gvt->lock);
>  	}
>  
>  	intel_vgpu_stop_schedule(vgpu);
>  	intel_vgpu_dmabuf_cleanup(vgpu);
>  
> -	mutex_unlock(&gvt->lock);
> +	mutex_unlock(&vgpu->vgpu_lock);
>  }
>  
>  /**
> @@ -252,14 +250,11 @@ void intel_gvt_destroy_vgpu(struct intel_vgpu *vgpu)
>  {
>  	struct intel_gvt *gvt = vgpu->gvt;
>  
> -	mutex_lock(&gvt->lock);
> +	mutex_lock(&vgpu->vgpu_lock);
>  
>  	WARN(vgpu->active, "vGPU is still active!\n");
>  
>  	intel_gvt_debugfs_remove_vgpu(vgpu);
> -	idr_remove(&gvt->vgpu_idr, vgpu->id);
> -	if (idr_is_empty(&gvt->vgpu_idr))
> -		intel_gvt_clean_irq(gvt);
>  	intel_vgpu_clean_sched_policy(vgpu);
>  	intel_vgpu_clean_submission(vgpu);
>  	intel_vgpu_clean_display(vgpu);
> @@ -269,10 +264,16 @@ void intel_gvt_destroy_vgpu(struct intel_vgpu *vgpu)
>  	intel_vgpu_free_resource(vgpu);
>  	intel_vgpu_clean_mmio(vgpu);
>  	intel_vgpu_dmabuf_cleanup(vgpu);
> -	vfree(vgpu);
> +	mutex_unlock(&vgpu->vgpu_lock);
>  
> +	mutex_lock(&gvt->lock);
> +	idr_remove(&gvt->vgpu_idr, vgpu->id);
> +	if (idr_is_empty(&gvt->vgpu_idr))
> +		intel_gvt_clean_irq(gvt);
>  	intel_gvt_update_vgpu_types(gvt);
>  	mutex_unlock(&gvt->lock);
> +
> +	vfree(vgpu);
>  }
>  
>  #define IDLE_VGPU_IDR 0
> @@ -298,6 +299,7 @@ struct intel_vgpu *intel_gvt_create_idle_vgpu(struct intel_gvt *gvt)
>  
>  	vgpu->id = IDLE_VGPU_IDR;
>  	vgpu->gvt = gvt;
> +	mutex_init(&vgpu->vgpu_lock);
>  
>  	for (i = 0; i < I915_NUM_ENGINES; i++)
>  		INIT_LIST_HEAD(&vgpu->submission.workload_q_head[i]);
> @@ -324,7 +326,10 @@ struct intel_vgpu *intel_gvt_create_idle_vgpu(struct intel_gvt *gvt)
>   */
>  void intel_gvt_destroy_idle_vgpu(struct intel_vgpu *vgpu)
>  {
> +	mutex_lock(&vgpu->vgpu_lock);
>  	intel_vgpu_clean_sched_policy(vgpu);
> +	mutex_unlock(&vgpu->vgpu_lock);
> +
>  	vfree(vgpu);
>  }
>  
> @@ -342,8 +347,6 @@ static struct intel_vgpu *__intel_gvt_create_vgpu(struct intel_gvt *gvt,
>  	if (!vgpu)
>  		return ERR_PTR(-ENOMEM);
>  
> -	mutex_lock(&gvt->lock);
> -
>  	ret = idr_alloc(&gvt->vgpu_idr, vgpu, IDLE_VGPU_IDR + 1, GVT_MAX_VGPU,
>  		GFP_KERNEL);
>  	if (ret < 0)
> @@ -353,6 +356,7 @@ static struct intel_vgpu *__intel_gvt_create_vgpu(struct intel_gvt *gvt,
>  	vgpu->handle = param->handle;
>  	vgpu->gvt = gvt;
>  	vgpu->sched_ctl.weight = param->weight;
> +	mutex_init(&vgpu->vgpu_lock);
>  	INIT_LIST_HEAD(&vgpu->dmabuf_obj_list_head);
>  	INIT_RADIX_TREE(&vgpu->page_track_tree, GFP_KERNEL);
>  	idr_init(&vgpu->object_idr);
> @@ -400,8 +404,6 @@ static struct intel_vgpu *__intel_gvt_create_vgpu(struct intel_gvt *gvt,
>  	if (ret)
>  		goto out_clean_sched_policy;
>  
> -	mutex_unlock(&gvt->lock);
> -
>  	return vgpu;
>  
>  out_clean_sched_policy:
> @@ -424,7 +426,6 @@ static struct intel_vgpu *__intel_gvt_create_vgpu(struct intel_gvt *gvt,
>  	idr_remove(&gvt->vgpu_idr, vgpu->id);
>  out_free_vgpu:
>  	vfree(vgpu);
> -	mutex_unlock(&gvt->lock);
>  	return ERR_PTR(ret);
>  }
>  
> @@ -456,12 +457,12 @@ struct intel_vgpu *intel_gvt_create_vgpu(struct intel_gvt *gvt,
>  	param.low_gm_sz = BYTES_TO_MB(param.low_gm_sz);
>  	param.high_gm_sz = BYTES_TO_MB(param.high_gm_sz);
>  
> +	mutex_lock(&gvt->lock);
>  	vgpu = __intel_gvt_create_vgpu(gvt, &param);
> -	if (IS_ERR(vgpu))
> -		return vgpu;
> -
> -	/* calculate left instance change for types */
> -	intel_gvt_update_vgpu_types(gvt);
> +	if (!IS_ERR(vgpu))
> +		/* calculate left instance change for types */
> +		intel_gvt_update_vgpu_types(gvt);
> +	mutex_unlock(&gvt->lock);
>  
>  	return vgpu;
>  }
> @@ -473,7 +474,7 @@ struct intel_vgpu *intel_gvt_create_vgpu(struct intel_gvt *gvt,
>   * @engine_mask: engines to reset for GT reset
>   *
>   * This function is called when user wants to reset a virtual GPU through
> - * device model reset or GT reset. The caller should hold the gvt lock.
> + * device model reset or GT reset. The caller should hold the vgpu lock.
>   *
>   * vGPU Device Model Level Reset (DMLR) simulates the PCI level reset to reset
>   * the whole vGPU to default state as when it is created. This vGPU function
> @@ -513,9 +514,9 @@ void intel_gvt_reset_vgpu_locked(struct intel_vgpu *vgpu, bool dmlr,
>  	 * scheduler when the reset is triggered by current vgpu.
>  	 */
>  	if (scheduler->current_vgpu == NULL) {
> -		mutex_unlock(&gvt->lock);
> +		mutex_unlock(&vgpu->vgpu_lock);
>  		intel_gvt_wait_vgpu_idle(vgpu);
> -		mutex_lock(&gvt->lock);
> +		mutex_lock(&vgpu->vgpu_lock);
>  	}
>  
>  	intel_vgpu_reset_submission(vgpu, resetting_eng);
> @@ -555,7 +556,7 @@ void intel_gvt_reset_vgpu_locked(struct intel_vgpu *vgpu, bool dmlr,
>   */
>  void intel_gvt_reset_vgpu(struct intel_vgpu *vgpu)
>  {
> -	mutex_lock(&vgpu->gvt->lock);
> +	mutex_lock(&vgpu->vgpu_lock);
>  	intel_gvt_reset_vgpu_locked(vgpu, true, 0);
> -	mutex_unlock(&vgpu->gvt->lock);
> +	mutex_unlock(&vgpu->vgpu_lock);
>  }
> -- 
> 2.17.0
> 
> _______________________________________________
> intel-gvt-dev mailing list
> intel-gvt-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gvt-dev