[Intel-gfx] [PATCH v11 08/17] drm/i915/pxp: Implement arb session teardown

[Teres Alexis, Alan Previn]

On Wed, 2021-09-22 at 15:56 -0700, Harish Chegondi wrote:
> On Tue, Sep 21, 2021 at 05:15:22PM -0700, Alan Previn wrote:
> > From: "Huang, Sean Z" <sean.z.huang at intel.com>
> > 
> > Teardown is triggered when the display topology changes and no
> > long meets the secure playback requirement, and hardware trashes
> > all the encryption keys for display. Additionally, we want to emit a
> > teardown operation to make sure we're clean on boot and resume
> > 
> > v2: emit in the ring, use high prio request (Chris)
> > v3: better defines, stalling flush, cleaned up and renamed submission
> >     funcs (Chris)
> > 
> > Signed-off-by: Huang, Sean Z <sean.z.huang at intel.com>
> > Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio at intel.com>
> > Cc: Chris Wilson <chris at chris-wilson.co.uk>
> > Reviewed-by: Rodrigo Vivi <rodrigo.vivi at intel.com>
> > ---
> >  drivers/gpu/drm/i915/Makefile                |   1 +
> >  drivers/gpu/drm/i915/gt/intel_gpu_commands.h |  22 ++-
> >  drivers/gpu/drm/i915/pxp/intel_pxp.c         |   7 +-
> >  drivers/gpu/drm/i915/pxp/intel_pxp_cmd.c     | 141 +++++++++++++++++++
> >  drivers/gpu/drm/i915/pxp/intel_pxp_cmd.h     |  15 ++
> >  drivers/gpu/drm/i915/pxp/intel_pxp_session.c |  29 ++++
> >  drivers/gpu/drm/i915/pxp/intel_pxp_session.h |   1 +
> >  7 files changed, 212 insertions(+), 4 deletions(-)
> >  create mode 100644 drivers/gpu/drm/i915/pxp/intel_pxp_cmd.c
> >  create mode 100644 drivers/gpu/drm/i915/pxp/intel_pxp_cmd.h
> > 

> >  
> >  void intel_pxp_fini_hw(struct intel_pxp *pxp)
> > diff --git a/drivers/gpu/drm/i915/pxp/intel_pxp_cmd.c b/drivers/gpu/drm/i915/pxp/intel_pxp_cmd.c
> > new file mode 100644
> > index 000000000000..80678dafde15
> > --- /dev/null
> > +++ b/drivers/gpu/drm/i915/pxp/intel_pxp_cmd.c
> > 

> > +int intel_pxp_terminate_session(struct intel_pxp *pxp, u32 id)
> > +{
> > +	struct i915_request *rq;
> > +	struct intel_context *ce = pxp->ce;
> > +	u32 *cs;
> > +	int err;
> err needs to be initialized to 0. If not initialized, it may be used
> uninitialized in the line I mentioned below.
> > +
> > +	if (!intel_pxp_is_enabled(pxp))
> > +		return 0;
> > +
> > +	rq = i915_request_create(ce);
> > +	if (IS_ERR(rq))
> > +		return PTR_ERR(rq);
> > +
> > +	if (ce->engine->emit_init_breadcrumb) {
> > +		err = ce->engine->emit_init_breadcrumb(rq);
> > +		if (err)
> > +			goto out_rq;
> > +	}
> > +
> > +	cs = intel_ring_begin(rq, SESSION_TERMINATION_LEN(1) + WAIT_LEN);
> > +	if (IS_ERR(cs)) {
> > +		err = PTR_ERR(cs);
> > +		goto out_rq;
> > +	}
> > +
> > +	cs = pxp_emit_session_termination(cs, id);
> > +	cs = pxp_emit_wait(cs);
> > +
> > +	intel_ring_advance(rq, cs);
> > +
> > +out_rq:
> > +	i915_request_get(rq);
> > +
> > +	if (unlikely(err))
> uninitialized 'err' may be used in the above line.
thanks for catching this Harish - its a valid bug.
i should have used a stronger make option when building / testing.
since the series needs a rebase anyway (drm-tip from just now
causing issues), I will push a new rev out.
..alan
> > +		i915_request_set_error_once(rq, err);
> > 
> > +
> > +	pxp_request_commit(rq);