drm/i915/pxp: Trigger the global teardown for before suspending
authorAlan Previn <alan.previn.teres.alexis@intel.com>
Wed, 25 Jan 2023 08:26:36 +0000 (00:26 -0800)
committerRodrigo Vivi <rodrigo.vivi@intel.com>
Fri, 27 Jan 2023 20:23:17 +0000 (15:23 -0500)
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.

v2 : Split __pxp_global_teardown_locked helper into two variants
     for teardown-with-restart vs teardown-for-suspend/shutdown.

Signed-off-by: Alan Previn <alan.previn.teres.alexis@intel.com>
Reviewed-by: Juston Li <justonli@chromium.org>
Acked-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20230125082637.118970-6-alan.previn.teres.alexis@intel.com
drivers/gpu/drm/i915/pxp/intel_pxp.c
drivers/gpu/drm/i915/pxp/intel_pxp.h
drivers/gpu/drm/i915/pxp/intel_pxp_pm.c
drivers/gpu/drm/i915/pxp/intel_pxp_session.c
drivers/gpu/drm/i915/pxp/intel_pxp_session.h

index cfc9af8b3d213c8cacd5d78b8a619b954d74fe74..9d4c7724e98edf335ef66a1eede01caca646fbed 100644 (file)
@@ -270,6 +270,60 @@ static bool pxp_component_bound(struct intel_pxp *pxp)
        return bound;
 }
 
+static int __pxp_global_teardown_final(struct intel_pxp *pxp)
+{
+       if (!pxp->arb_is_valid)
+               return 0;
+       /*
+        * To ensure synchronous and coherent session teardown completion
+        * in response to suspend or shutdown triggers, don't use a worker.
+        */
+       intel_pxp_mark_termination_in_progress(pxp);
+       intel_pxp_terminate(pxp, false);
+
+       if (!wait_for_completion_timeout(&pxp->termination, msecs_to_jiffies(250)))
+               return -ETIMEDOUT;
+
+       return 0;
+}
+
+static int __pxp_global_teardown_restart(struct intel_pxp *pxp)
+{
+       if (pxp->arb_is_valid)
+               return 0;
+       /*
+        * The arb-session is currently inactive and we are doing a reset and restart
+        * due to a runtime event. Use the worker that was designed for this.
+        */
+       pxp_queue_termination(pxp);
+
+       if (!wait_for_completion_timeout(&pxp->termination, msecs_to_jiffies(250)))
+               return -ETIMEDOUT;
+
+       return 0;
+}
+
+void intel_pxp_end(struct intel_pxp *pxp)
+{
+       struct drm_i915_private *i915 = pxp->ctrl_gt->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_final(pxp))
+               drm_dbg(&i915->drm, "PXP end timed out\n");
+
+       mutex_unlock(&pxp->arb_mutex);
+
+       intel_pxp_fini_hw(pxp);
+       intel_runtime_pm_put(&i915->runtime_pm, wakeref);
+}
+
 /*
  * the arb session is restarted from the irq work when we receive the
  * termination completion interrupt
@@ -286,16 +340,9 @@ int intel_pxp_start(struct intel_pxp *pxp)
 
        mutex_lock(&pxp->arb_mutex);
 
-       if (pxp->arb_is_valid)
-               goto unlock;
-
-       pxp_queue_termination(pxp);
-
-       if (!wait_for_completion_timeout(&pxp->termination,
-                                       msecs_to_jiffies(250))) {
-               ret = -ETIMEDOUT;
+       ret = __pxp_global_teardown_restart(pxp);
+       if (ret)
                goto unlock;
-       }
 
        /* make sure the compiler doesn't optimize the double access */
        barrier();
index 9658d30052224a9d531dffcfcd59f9b585824dd2..3ded0890cd277b38b4f57d714939283a3ca85084 100644 (file)
@@ -27,6 +27,7 @@ void intel_pxp_mark_termination_in_progress(struct intel_pxp *pxp);
 void intel_pxp_tee_end_arb_fw_session(struct intel_pxp *pxp, u32 arb_session_id);
 
 int intel_pxp_start(struct intel_pxp *pxp);
+void intel_pxp_end(struct intel_pxp *pxp);
 
 int intel_pxp_key_check(struct intel_pxp *pxp,
                        struct drm_i915_gem_object *obj,
index 892d39cc61c155e06732f3c802fdb4c264a7c922..e427464aa1319d343f404c093ccb9876998c48b3 100644 (file)
@@ -16,7 +16,7 @@ void intel_pxp_suspend_prepare(struct intel_pxp *pxp)
        if (!intel_pxp_is_enabled(pxp))
                return;
 
-       pxp->arb_is_valid = false;
+       intel_pxp_end(pxp);
 
        intel_pxp_invalidate(pxp);
 }
index 74ed7e16e481125f2a4c15261ca9c602db8d7603..448cacb0465d9a0dd439b909cf0afbd3454ddc8a 100644 (file)
@@ -115,11 +115,11 @@ static int pxp_terminate_arb_session_and_global(struct intel_pxp *pxp)
        return ret;
 }
 
-static void pxp_terminate(struct intel_pxp *pxp)
+void intel_pxp_terminate(struct intel_pxp *pxp, bool post_invalidation_needs_restart)
 {
        int ret;
 
-       pxp->hw_state_invalidated = true;
+       pxp->hw_state_invalidated = post_invalidation_needs_restart;
 
        /*
         * if we fail to submit the termination there is no point in waiting for
@@ -167,7 +167,7 @@ static void pxp_session_work(struct work_struct *work)
 
        if (events & PXP_TERMINATION_REQUEST) {
                events &= ~PXP_TERMINATION_COMPLETE;
-               pxp_terminate(pxp);
+               intel_pxp_terminate(pxp, true);
        }
 
        if (events & PXP_TERMINATION_COMPLETE)
index 903ac52cffa1ae78cbf353dd9e4e087f16800c3d..ba5788127109f2a6e38e2bf111d2e3c179f2ef9b 100644 (file)
@@ -12,9 +12,14 @@ struct intel_pxp;
 
 #ifdef CONFIG_DRM_I915_PXP
 void intel_pxp_session_management_init(struct intel_pxp *pxp);
+void intel_pxp_terminate(struct intel_pxp *pxp, bool post_invalidation_needs_restart);
 #else
 static inline void intel_pxp_session_management_init(struct intel_pxp *pxp)
 {
 }
+
+static inline void intel_pxp_terminate(struct intel_pxp *pxp, bool post_invalidation_needs_restart)
+{
+}
 #endif
 #endif /* __INTEL_PXP_SESSION_H__ */