[Intel-gfx] [PATCH 2/2] drm/i915/pxp: Trigger the global teardown for before suspending

[Teres Alexis, Alan Previn]

On Mon, 2022-11-21 at 10:39 -0800, Juston Li wrote:
> On Thu, 2022-11-17 at 16:36 -0800, Alan Previn wrote:
> > A driver bug was recently discovered where the security firmware was
> > receiving internal HW signals indicating that session key expirations
> > had occurred. Architecturally, the firmware was expecting a response
> > from the GuC to acknowledge the event with the firmware side.
> > However the OS was in a suspended state and GuC had been reset.
> > Internal specifications actually required the driver to ensure
> > that all active sessions be properly cleaned up in such cases where
> > the system is suspended and the GuC potentially unable to respond.
> > 
> > This patch adds the global teardown code in i915's suspend_prepare
> > code path.
> > 
Alan:[snip]

> > +void intel_pxp_end(struct intel_pxp *pxp)
> > +{
> > +       struct drm_i915_private *i915 = pxp_to_gt(pxp)->i915;
> > +       intel_wakeref_t wakeref;
> > +
> > +       if (!intel_pxp_is_enabled(pxp))
> > +               return;
> > +
> > +       wakeref = intel_runtime_pm_get(&i915->runtime_pm);
> > +
> > +       mutex_lock(&pxp->arb_mutex);
> > +
> > +       if (__pxp_global_teardown_locked(pxp, true))
> > +               drm_dbg(&(pxp_to_gt(pxp))->i915->drm, "PXP end timed
> > out\n");
> > +
> > +       mutex_unlock(&pxp->arb_mutex);
> > +
> > +       intel_pxp_fini_hw(pxp);
> 
> Is intel_pxp_suspend() still needed then if we already fini_hw() here
> and mark invalidation in intel_pxp_terminate()?
> 

Good catch - looks like we might not need intel_pxp_suspend. But I'll verify that for you.
Also, looks like i forgot to include a non-CONFIG_DRM_I915_PXP version of intel_pxp_end which was causing the build
failure. Will resend.

Btw, thanks for reviewing this Juston, i had cc'd you because of the impact to suspend-resume flows and I believe you
have had prior experience debugging issues with that and runtime-suspend/resume. Do you any other issues with this
change?

...alan