[PATCH v3 1/2] drm/i915/gvt: optimization of context shadowing

[Zhao, Yan Y]

> -----Original Message-----
> From: Tian, Kevin <kevin.tian at intel.com>
> Sent: Friday, March 13, 2020 5:30 PM
> To: Zhao, Yan Y <yan.y.zhao at intel.com>; intel-gvt-dev at lists.freedesktop.org
> Cc: Zhao, Yan Y <yan.y.zhao at intel.com>; zhenyuw at linux.intel.com
> Subject: RE: [PATCH v3 1/2] drm/i915/gvt: optimization of context shadowing
> 
> > From: Yan Zhao
> > Sent: Friday, March 13, 2020 5:11 PM
> >
> > Software is not expected to populate engine context except when using
> > restore inhibit bit or golden state to initialize it for the first time.
> >
> > Therefore, if a newly submitted guest context is the same as the last
> > shadowed one, no need to populate its engine context from guest again.
> >
> > Currently using lrca + ring_context_gpa to identify whether two guest
> > contexts are the same.
> >
> > The reason of why context id is not included as an identifier is that
> > i915 recently changed the code and context id is only unique for a
> > context when OA is enabled. And when OA is on, context id is generated
> > based on lrca. Therefore, in that case, if two contexts are of the
> > same lrca, they have identical context ids as well.
> >
> > This patch also works with old guest kernel like 4.20.
> 
> given that this patch only works correctly together with 1/2, please enable
> the optimization only at the last step, i.e. with 1/3 to simply create/update
> the the cached information, 2/3 to invalidate the cached info due to GGTT
> change, and then 3/3 to finally enable the optimization by skipping matched
> context. otherwise 1/2 alone may lead to undesired behavior when doing
> bisect later.
> 
Got it!
Thanks!

> >
> > v3: updated commit message to describe engine context and context id
> > clearly (Kevin Tian)
> > v2: rebased to 5.6.0-rc4+
> > Signed-off-by: Yan Zhao <yan.y.zhao at intel.com>
> > ---
> >  drivers/gpu/drm/i915/gvt/gvt.h       |  4 ++++
> >  drivers/gpu/drm/i915/gvt/scheduler.c | 25 ++++++++++++++++++++-----
> >  2 files changed, 24 insertions(+), 5 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/gvt/gvt.h
> > b/drivers/gpu/drm/i915/gvt/gvt.h index 58c2c7932e3f..e2d7ffd84457
> > 100644
> > --- a/drivers/gpu/drm/i915/gvt/gvt.h
> > +++ b/drivers/gpu/drm/i915/gvt/gvt.h
> > @@ -163,6 +163,10 @@ struct intel_vgpu_submission {
> >  	const struct intel_vgpu_submission_ops *ops;
> >  	int virtual_submission_interface;
> >  	bool active;
> > +	struct {
> > +		u32 lrca;
> > +		u64 ring_context_gpa;
> > +	} last_ctx[I915_NUM_ENGINES];
> >  };
> >
> >  struct intel_vgpu {
> > diff --git a/drivers/gpu/drm/i915/gvt/scheduler.c
> > b/drivers/gpu/drm/i915/gvt/scheduler.c
> > index 1c95bf8cbed0..a66050a3d65a 100644
> > --- a/drivers/gpu/drm/i915/gvt/scheduler.c
> > +++ b/drivers/gpu/drm/i915/gvt/scheduler.c
> > @@ -134,7 +134,10 @@ static int populate_shadow_context(struct
> > intel_vgpu_workload *workload)
> >  	struct page *page;
> >  	void *dst;
> >  	unsigned long context_gpa, context_page_num;
> > +	struct intel_vgpu_submission *s = &vgpu->submission;
> >  	int i;
> > +	bool skip = false;
> > +	int ring_id = workload->engine->id;
> >
> >  	page = i915_gem_object_get_page(ctx_obj, LRC_STATE_PN);
> >  	shadow_ring_context = kmap(page);
> > @@ -171,12 +174,22 @@ static int populate_shadow_context(struct
> > intel_vgpu_workload *workload)
> >  	sr_oa_regs(workload, (u32 *)shadow_ring_context, false);
> >  	kunmap(page);
> >
> > -	if (IS_RESTORE_INHIBIT(shadow_ring_context->ctx_ctrl.val))
> > -		return 0;
> > +	gvt_dbg_sched("ring %s workload lrca %x, ctx_id %x, ctx gpa %llx",
> > +			workload->engine->name, workload->ctx_desc.lrca,
> > +			workload->ctx_desc.context_id,
> > +			workload->ring_context_gpa);
> >
> > -	gvt_dbg_sched("ring %s workload lrca %x",
> > -		      workload->engine->name,
> > -		      workload->ctx_desc.lrca);
> > +	if ((s->last_ctx[ring_id].lrca ==
> > +				workload->ctx_desc.lrca) &&
> > +			(s->last_ctx[ring_id].ring_context_gpa ==
> > +				workload->ring_context_gpa))
> > +		skip = true;
> > +
> > +	s->last_ctx[ring_id].lrca = workload->ctx_desc.lrca;
> > +	s->last_ctx[ring_id].ring_context_gpa = workload->ring_context_gpa;
> > +
> > +	if (IS_RESTORE_INHIBIT(shadow_ring_context->ctx_ctrl.val) || skip)
> > +		return 0;
> >
> >  	context_page_num = workload->engine->context_size;
> >  	context_page_num = context_page_num >> PAGE_SHIFT; @@ -
> 1260,6
> > +1273,8 @@ int intel_vgpu_setup_submission(struct intel_vgpu
> > *vgpu)
> >  	atomic_set(&s->running_workload_num, 0);
> >  	bitmap_zero(s->tlb_handle_pending, I915_NUM_ENGINES);
> >
> > +	memset(s->last_ctx, 0, sizeof(s->last_ctx));
> > +
> >  	i915_vm_put(&ppgtt->vm);
> >  	return 0;
> >
> > --
> > 2.17.1
> >
> > _______________________________________________
> > intel-gvt-dev mailing list
> > intel-gvt-dev at lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/intel-gvt-dev