Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit f25a49ab authored by Colin Xu's avatar Colin Xu Committed by Zhenyu Wang
Browse files

drm/i915/gvt: Use vgpu_lock to protect per vgpu access 



The patch set splits out 2 small locks from the original big gvt lock:
  - vgpu_lock protects per-vGPU data and logic, especially the vGPU
    trap emulation path.
  - sched_lock protects gvt scheudler structure, context schedule logic
    and vGPU's schedule data.

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.

v9:
  - Change commit author since the patches are improved a lot compared
    with original version.
    Original author: Pei Zhang <pei.zhang@intel.com>
  - Rebase to latest gvt-staging.
v8:
  - Correct coding and comment style.
  - Rebase to latest gvt-staging.
v7:
  - Remove gtt_lock since already proteced by gvt_lock and vgpu_lock.
  - Fix a typo in intel_gvt_deactivate_vgpu, unlock the wrong 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

Performance comparison on Kabylake platform.
  - Configuration:
    Host: Ubuntu 16.04.
    Guest 1 & 2: Ubuntu 16.04.

glmark2 score comparison:
  - Configuration:
    Host: glxgears.
    Guests: glmark2.
+--------------------------------+-----------------+
| Setup                          | glmark2 score   |
+--------------------------------+-----------------+
| unified lock, iommu=on         | 58~62 (avg. 60) |
+--------------------------------+-----------------+
| unified lock, iommu=igfx_off   | 57~61 (avg. 59) |
+--------------------------------+-----------------+
| per-logic lock, iommu=on       | 60~68 (avg. 64) |
+--------------------------------+-----------------+
| per-logic lock, iommu=igfx_off | 61~67 (avg. 64) |
+--------------------------------+-----------------+

lock_stat comparison:
  - Configuration:
    Stop lock stat immediately after boot up.
    Boot 2 VM Guests.
    Run glmark2 in guests.
    Start perf lock_stat for 20 seconds and stop again.
  - Legend: c - contentions; w - waittime-avg
+------------+-----------------+-----------+---------------+------------+
|            | gvt_lock        |sched_lock | vgpu_lock     | gtt_lock   |
+ lock type; +-----------------+-----------+---------------+------------+
| iommu set  | c     | w       | c  | w    | c    | w      | c   | w    |
+------------+-------+---------+----+------+------+--------+-----+------+
| unified;   | 20697 | 839     |N/A | N/A  | N/A  | N/A    | N/A | N/A  |
| on         |       |         |    |      |      |        |     |      |
+------------+-------+---------+----+------+------+--------+-----+------+
| unified;   | 21838 | 658.15  |N/A | N/A  | N/A  | N/A    | N/A | N/A  |
| igfx_off   |       |         |    |      |      |        |     |      |
+------------+-------+---------+----+------+------+--------+-----+------+
| per-logic; | 1553  | 1599.96 |9458|429.97| 5846 | 274.33 | 0   | 0.00 |
| on         |       |         |    |      |      |        |     |      |
+------------+-------+---------+----+------+------+--------+-----+------+
| per-logic; | 1911  | 1678.32 |8335|445.16| 5451 | 244.80 | 0   | 0.00 |
| igfx_off   |       |         |    |      |      |        |     |      |
+------------+-------+---------+----+------+------+--------+-----+------+

Signed-off-by: default avatarPei Zhang <pei.zhang@intel.com>
Signed-off-by: default avatarColin Xu <colin.xu@intel.com>
Signed-off-by: default avatarZhenyu Wang <zhenyuw@linux.intel.com>
parent 579e2f6d

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

+19 −16
Original line number Diff line number Diff line
@@ -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 */

	if (!found)

		hrtimer_cancel(&irq->vblank_timer.timer);

	return;



out:

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

}



/**

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

+1 −4
Original line number Diff line number Diff line
@@ -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) ||

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

+8 −0
Original line number Diff line number Diff line
@@ -170,6 +170,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;
@@ -294,6 +295,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 drm_i915_private *dev_priv;

	struct idr vgpu_idr;	/* vGPU IDR pool */
@@ -314,6 +318,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 {

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

+4 −0
Original line number Diff line number Diff line
@@ -316,6 +316,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 */
@@ -420,7 +421,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;

}

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

+6 −6
Original line number Diff line number Diff line
@@ -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;

}