[PATCH] drm/i915/gvt: refine vm engine reset
Zhenyu Wang
zhenyuw at linux.intel.com
Wed Jan 24 09:00:00 UTC 2018
On 2018.01.19 10:13:15 +0800, Weinan Li wrote:
> Use per engine vgpu execlist reset/init instead of per vgpu, ensure to only
> reset vgpu execlist state of the exact engine which gets reset request from
> vm. Otherwise if there is engine reset from vm, GVT-g will reset all
Let's write VM in capital for virtual machine.
> engines execlist state. If virtual HWSP also be enabled at this time, vm
> will keep the CSB read pointer but does not read from vGPU, then vm will
> get wrong CSB state.
>
> Signed-off-by: Weinan Li <weinan.z.li at intel.com>
> Cc: Fred Gao <fred.gao at intel.com>
> Cc: Zhi Wang <zhi.a.wang at intel.com>
> ---
> drivers/gpu/drm/i915/gvt/execlist.c | 24 ++++++++++++-----------
> drivers/gpu/drm/i915/gvt/gvt.h | 5 +++--
> drivers/gpu/drm/i915/gvt/handlers.c | 10 ++++------
> drivers/gpu/drm/i915/gvt/sched_policy.c | 3 ++-
> drivers/gpu/drm/i915/gvt/scheduler.c | 34 ++++++++++++++++-----------------
> drivers/gpu/drm/i915/gvt/scheduler.h | 1 +
> drivers/gpu/drm/i915/gvt/vgpu.c | 3 +--
> 7 files changed, 41 insertions(+), 39 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/gvt/execlist.c b/drivers/gpu/drm/i915/gvt/execlist.c
> index 769c1c2..615bc6b 100644
> --- a/drivers/gpu/drm/i915/gvt/execlist.c
> +++ b/drivers/gpu/drm/i915/gvt/execlist.c
> @@ -521,25 +521,26 @@ static void init_vgpu_execlist(struct intel_vgpu *vgpu, int ring_id)
>
> ctx_status_ptr_reg = execlist_ring_mmio(vgpu->gvt, ring_id,
> _EL_OFFSET_STATUS_PTR);
> -
> ctx_status_ptr.dw = vgpu_vreg(vgpu, ctx_status_ptr_reg);
> ctx_status_ptr.read_ptr = 0;
> ctx_status_ptr.write_ptr = 0x7;
> vgpu_vreg(vgpu, ctx_status_ptr_reg) = ctx_status_ptr.dw;
> + gvt_dbg_el("init vgpu execlist\n");
This doesn't bring helpful debug info without e.g target init ring info?
> }
>
> -static void clean_execlist(struct intel_vgpu *vgpu)
> +static void clean_execlist(struct intel_vgpu *vgpu, unsigned long engine_mask)
> {
> - enum intel_engine_id i;
> + unsigned int tmp;
> + struct drm_i915_private *dev_priv = vgpu->gvt->dev_priv;
> struct intel_engine_cs *engine;
> + struct intel_vgpu_submission *s = &vgpu->submission;
>
> - for_each_engine(engine, vgpu->gvt->dev_priv, i) {
> - struct intel_vgpu_submission *s = &vgpu->submission;
> -
> - kfree(s->ring_scan_buffer[i]);
> - s->ring_scan_buffer[i] = NULL;
> - s->ring_scan_buffer_size[i] = 0;
> + for_each_engine_masked(engine, dev_priv, engine_mask, tmp) {
> + kfree(s->ring_scan_buffer[engine->id]);
> + s->ring_scan_buffer[engine->id] = NULL;
> + s->ring_scan_buffer_size[engine->id] = 0;
> }
> + gvt_dbg_el("clean execlist\n");
ditto
> }
>
> static void reset_execlist(struct intel_vgpu *vgpu,
> @@ -553,9 +554,10 @@ static void reset_execlist(struct intel_vgpu *vgpu,
> init_vgpu_execlist(vgpu, engine->id);
> }
>
> -static int init_execlist(struct intel_vgpu *vgpu)
> +static int init_execlist(struct intel_vgpu *vgpu,
> + unsigned long engine_mask)
> {
> - reset_execlist(vgpu, ALL_ENGINES);
> + reset_execlist(vgpu, engine_mask);
> return 0;
> }
>
> diff --git a/drivers/gpu/drm/i915/gvt/gvt.h b/drivers/gpu/drm/i915/gvt/gvt.h
> index 7dc7a80..a6a4253 100644
> --- a/drivers/gpu/drm/i915/gvt/gvt.h
> +++ b/drivers/gpu/drm/i915/gvt/gvt.h
> @@ -152,8 +152,8 @@ enum {
>
> struct intel_vgpu_submission_ops {
> const char *name;
> - int (*init)(struct intel_vgpu *vgpu);
> - void (*clean)(struct intel_vgpu *vgpu);
> + int (*init)(struct intel_vgpu *vgpu, unsigned long engine_mask);
> + void (*clean)(struct intel_vgpu *vgpu, unsigned long engine_mask);
> void (*reset)(struct intel_vgpu *vgpu, unsigned long engine_mask);
> };
>
> @@ -182,6 +182,7 @@ struct intel_vgpu {
> unsigned int resetting_eng;
> void *sched_data;
> struct vgpu_sched_ctl sched_ctl;
> + bool vgpu_sched_active;
>
> struct intel_vgpu_fence fence;
> struct intel_vgpu_gm gm;
> diff --git a/drivers/gpu/drm/i915/gvt/handlers.c b/drivers/gpu/drm/i915/gvt/handlers.c
> index 38f3b00..284efd2 100644
> --- a/drivers/gpu/drm/i915/gvt/handlers.c
> +++ b/drivers/gpu/drm/i915/gvt/handlers.c
> @@ -1494,7 +1494,6 @@ static int elsp_mmio_write(struct intel_vgpu *vgpu, unsigned int offset,
> static int ring_mode_mmio_write(struct intel_vgpu *vgpu, unsigned int offset,
> void *p_data, unsigned int bytes)
> {
> - struct intel_vgpu_submission *s = &vgpu->submission;
> u32 data = *(u32 *)p_data;
> int ring_id = intel_gvt_render_mmio_to_ring_id(vgpu->gvt, offset);
> bool enable_execlist;
> @@ -1523,15 +1522,14 @@ static int ring_mode_mmio_write(struct intel_vgpu *vgpu, unsigned int offset,
> if (!enable_execlist)
> return 0;
>
> - if (s->active)
> - return 0;
> -
> ret = intel_vgpu_select_submission_ops(vgpu,
> - INTEL_VGPU_EXECLIST_SUBMISSION);
> + ENGINE_MASK(ring_id),
> + INTEL_VGPU_EXECLIST_SUBMISSION);
> if (ret)
> return ret;
>
> - intel_vgpu_start_schedule(vgpu);
> + if (!vgpu->vgpu_sched_active)
> + intel_vgpu_start_schedule(vgpu);
I think it's better to wrap the state track inside of scheduler itself
instead of tracking out of sched data. And looks this change is not related
to this fix, that can be split.
> }
> return 0;
> }
> diff --git a/drivers/gpu/drm/i915/gvt/sched_policy.c b/drivers/gpu/drm/i915/gvt/sched_policy.c
> index eea1a2f..b1bc5f7 100644
> --- a/drivers/gpu/drm/i915/gvt/sched_policy.c
> +++ b/drivers/gpu/drm/i915/gvt/sched_policy.c
> @@ -325,6 +325,7 @@ static void tbs_sched_start_schedule(struct intel_vgpu *vgpu)
> if (!hrtimer_active(&sched_data->timer))
> hrtimer_start(&sched_data->timer, ktime_add_ns(ktime_get(),
> sched_data->period), HRTIMER_MODE_ABS);
> + vgpu->vgpu_sched_active = true;
> }
>
> static void tbs_sched_stop_schedule(struct intel_vgpu *vgpu)
> @@ -332,6 +333,7 @@ static void tbs_sched_stop_schedule(struct intel_vgpu *vgpu)
> struct vgpu_sched_data *vgpu_data = vgpu->sched_data;
>
> list_del_init(&vgpu_data->lru_list);
> + vgpu->vgpu_sched_active = false;
> }
>
> static struct intel_gvt_sched_policy_ops tbs_schedule_ops = {
> @@ -368,7 +370,6 @@ void intel_vgpu_clean_sched_policy(struct intel_vgpu *vgpu)
> void intel_vgpu_start_schedule(struct intel_vgpu *vgpu)
> {
> gvt_dbg_core("vgpu%d: start schedule\n", vgpu->id);
> -
> vgpu->gvt->scheduler.sched_ops->start_schedule(vgpu);
> }
>
> diff --git a/drivers/gpu/drm/i915/gvt/scheduler.c b/drivers/gpu/drm/i915/gvt/scheduler.c
> index 0056638..d3b7adc 100644
> --- a/drivers/gpu/drm/i915/gvt/scheduler.c
> +++ b/drivers/gpu/drm/i915/gvt/scheduler.c
> @@ -991,7 +991,7 @@ void intel_vgpu_clean_submission(struct intel_vgpu *vgpu)
> {
> struct intel_vgpu_submission *s = &vgpu->submission;
>
> - intel_vgpu_select_submission_ops(vgpu, 0);
> + intel_vgpu_select_submission_ops(vgpu, ALL_ENGINES, 0);
> i915_gem_context_put(s->shadow_ctx);
> kmem_cache_destroy(s->workloads);
> }
> @@ -1009,7 +1009,6 @@ void intel_vgpu_reset_submission(struct intel_vgpu *vgpu,
> unsigned long engine_mask)
> {
> struct intel_vgpu_submission *s = &vgpu->submission;
> -
> if (!s->active)
> return;
>
> @@ -1079,6 +1078,7 @@ int intel_vgpu_setup_submission(struct intel_vgpu *vgpu)
> *
> */
> int intel_vgpu_select_submission_ops(struct intel_vgpu *vgpu,
> + unsigned long engine_mask,
> unsigned int interface)
> {
> struct intel_vgpu_submission *s = &vgpu->submission;
> @@ -1091,31 +1091,31 @@ int intel_vgpu_select_submission_ops(struct intel_vgpu *vgpu,
> if (WARN_ON(interface >= ARRAY_SIZE(ops)))
> return -EINVAL;
>
> - if (s->active) {
> - s->ops->clean(vgpu);
> - s->active = false;
> - gvt_dbg_core("vgpu%d: de-select ops [ %s ] \n",
> - vgpu->id, s->ops->name);
> - }
> + if (WARN_ON(interface == 0 && engine_mask != ALL_ENGINES))
> + return -EINVAL;
> +
> + if (s->active)
> + s->ops->clean(vgpu, engine_mask);
>
> if (interface == 0) {
> s->ops = NULL;
> s->virtual_submission_interface = 0;
> - gvt_dbg_core("vgpu%d: no submission ops\n", vgpu->id);
> + s->active = false;
> + gvt_dbg_core("vgpu%d: remove submission ops\n", vgpu->id);
> return 0;
> }
>
> - ret = ops[interface]->init(vgpu);
> + ret = ops[interface]->init(vgpu, engine_mask);
> if (ret)
> return ret;
>
> - s->ops = ops[interface];
> - s->virtual_submission_interface = interface;
> - s->active = true;
> -
> - gvt_dbg_core("vgpu%d: activate ops [ %s ]\n",
> - vgpu->id, s->ops->name);
> -
> + if (!s->active) {
> + s->ops = ops[interface];
> + s->virtual_submission_interface = interface;
> + s->active = true;
> + gvt_dbg_core("vgpu%d: activate ops [ %s ]\n",
> + vgpu->id, s->ops->name);
> + }
> return 0;
> }
>
> diff --git a/drivers/gpu/drm/i915/gvt/scheduler.h b/drivers/gpu/drm/i915/gvt/scheduler.h
> index 3de77df..ff175a9 100644
> --- a/drivers/gpu/drm/i915/gvt/scheduler.h
> +++ b/drivers/gpu/drm/i915/gvt/scheduler.h
> @@ -141,6 +141,7 @@ void intel_vgpu_reset_submission(struct intel_vgpu *vgpu,
> void intel_vgpu_clean_submission(struct intel_vgpu *vgpu);
>
> int intel_vgpu_select_submission_ops(struct intel_vgpu *vgpu,
> + unsigned long engine_mask,
> unsigned int interface);
>
> extern const struct intel_vgpu_submission_ops
> diff --git a/drivers/gpu/drm/i915/gvt/vgpu.c b/drivers/gpu/drm/i915/gvt/vgpu.c
> index 4688619..6f09a1d 100644
> --- a/drivers/gpu/drm/i915/gvt/vgpu.c
> +++ b/drivers/gpu/drm/i915/gvt/vgpu.c
> @@ -518,8 +518,7 @@ void intel_gvt_reset_vgpu_locked(struct intel_vgpu *vgpu, bool dmlr,
> intel_vgpu_reset_submission(vgpu, resetting_eng);
> /* full GPU reset or device model level reset */
> if (engine_mask == ALL_ENGINES || dmlr) {
> - intel_vgpu_select_submission_ops(vgpu, 0);
> -
> + intel_vgpu_select_submission_ops(vgpu, resetting_eng, 0);
Can be just ALL_ENGINES
> /*fence will not be reset during virtual reset */
> if (dmlr) {
> intel_vgpu_reset_gtt(vgpu);
> --
> 1.9.1
>
> _______________________________________________
> intel-gvt-dev mailing list
> intel-gvt-dev at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gvt-dev
--
Open Source Technology Center, Intel ltd.
$gpg --keyserver wwwkeys.pgp.net --recv-keys 4D781827
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 195 bytes
Desc: not available
URL: <https://lists.freedesktop.org/archives/intel-gvt-dev/attachments/20180124/2fe441cd/attachment.sig>
More information about the intel-gvt-dev
mailing list