drm/xe/pxp: add termination on resume

Suspend/resume causes the PXP keys to become invalid, but doesn't
actually kill the session. The driver also doesn't explicitly kill and
re-start the session until a new PXP request comes in, which means that
the "zombie" session can potentially stick around if there are no new
requests from userspace. While this is not an issue for PXP, HDCP has a
new behavior starting on PTL where a communication is sent to GSC if a
session is active at suspend time (even if it doesn't have a valid key),
which can lead to delays in the suspend flow if we suspend while the
zombie session is still active.
To avoid this, we can trigger a termination on resume and kill the
zombie session immediately, instead of delaying the termination to the
next PXP request. Due to restrictions in the rpm suspend/resume flow, we
can't call the termination flow from within the resume call itself, so
the pxp irq worker is expanded to cover this scenario.
The existing logic in the worker doesn't work as-is for the new flow,
because the pm_get_if_active will fail if the worker runs before the
pci_resume call has completed (which is possible, since we queue it
from within that call) or after we're started to suspend again.
Given that we always want to run the worker after a resume (differently
from the irq case, where we want to skip if we're suspended), we can
solve this by just taking the PM reference before queueing the worker.
As part of this rework, the pxp->events variable has been moved to atomic,
to avoid having to take xe->irq.lock from non-irq related paths.

Fixes: b1dcec9bd8a1 ("drm/xe/ptl: Enable PXP for PTL")
Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
Cc: Julia Filipchuk <julia.filipchuk@intel.com>
Cc: Alan Previn <alan.previn.teres.alexis@intel.com>
Reviewed-by: Alan Previn <alan.previn.teres.alexis@intel.com>
Link: https://patch.msgid.link/20260720222757.3876338-2-daniele.ceraolospurio@intel.com
(cherry picked from commit 757bda2b8b93fa36ad9b2c7993081d5f9d0d6e3b)
Signed-off-by: Thomas Hellström <thomas.hellstrom@linux.intel.com>

diff --git a/drivers/gpu/drm/xe/xe_pxp.c b/drivers/gpu/drm/xe/xe_pxp.c
index 968b7e70b3f9729070572174770278d01c8d397c..d17ab6e59df5310a118a7989569d8b287ac2f85a 100644 (file)
--- a/drivers/gpu/drm/xe/xe_pxp.c
+++ b/drivers/gpu/drm/xe/xe_pxp.c
@@ -8,6 +8,8 @@
 #include <drm/drm_managed.h>
 #include <uapi/drm/xe_drm.h>
 
+#include <linux/device.h>
+
 #include "xe_bo.h"
 #include "xe_bo_types.h"
 #include "xe_device_types.h"
@@ -164,16 +166,9 @@ static void mark_termination_in_progress(struct xe_pxp *pxp)
        pxp->status = XE_PXP_TERMINATION_IN_PROGRESS;
 }
 
-static void pxp_terminate(struct xe_pxp *pxp)
+static bool pxp_prep_for_termination(struct xe_pxp *pxp)
 {
-       int ret = 0;
-       struct xe_device *xe = pxp->xe;
-
-       if (!wait_for_completion_timeout(&pxp->activation,
-                                        msecs_to_jiffies(PXP_ACTIVATION_TIMEOUT_MS)))
-               drm_err(&xe->drm, "failed to wait for PXP start before termination\n");
-
-       mutex_lock(&pxp->mutex);
+       lockdep_assert_held(&pxp->mutex);
 
        if (pxp->status == XE_PXP_ACTIVE)
                pxp->key_instance++;
@@ -182,10 +177,8 @@ static void pxp_terminate(struct xe_pxp *pxp)
         * we'll mark the status as needing termination on resume, so no need to
         * emit a termination now.
         */
-       if (pxp->status == XE_PXP_SUSPENDED) {
-               mutex_unlock(&pxp->mutex);
-               return;
-       }
+       if (pxp->status == XE_PXP_SUSPENDED)
+               return false;
 
        /*
         * If we have a termination already in progress, we need to wait for
@@ -195,15 +188,44 @@ static void pxp_terminate(struct xe_pxp *pxp)
         */
        if (pxp->status == XE_PXP_TERMINATION_IN_PROGRESS) {
                pxp->status = XE_PXP_NEEDS_ADDITIONAL_TERMINATION;
-               mutex_unlock(&pxp->mutex);
-               return;
+               return false;
        }
 
        mark_termination_in_progress(pxp);
 
-       mutex_unlock(&pxp->mutex);
+       return true;
+}
+
+static void pxp_terminate(struct xe_pxp *pxp, bool hw_only)
+{
+       struct xe_device *xe = pxp->xe;
+       int ret = 0;
 
-       pxp_invalidate_queues(pxp);
+       if (!wait_for_completion_timeout(&pxp->activation,
+                                        msecs_to_jiffies(PXP_ACTIVATION_TIMEOUT_MS)))
+               drm_err(&xe->drm, "failed to wait for PXP start before termination\n");
+
+       if (!hw_only) {
+               bool prep_ok;
+
+               mutex_lock(&pxp->mutex);
+
+               prep_ok = pxp_prep_for_termination(pxp);
+
+               mutex_unlock(&pxp->mutex);
+
+               if (!prep_ok)
+                       return;
+
+               pxp_invalidate_queues(pxp);
+       } else {
+               /*
+                * The caller of the HW-only termination should have already
+                * called pxp_prep_for_termination and marked the termination as
+                * in progress.
+                */
+               xe_assert(xe, !completion_done(&pxp->termination));
+       }
 
        ret = pxp_terminate_hw(pxp);
        if (ret) {
@@ -249,33 +271,46 @@ static void pxp_terminate_complete(struct xe_pxp *pxp)
        mutex_unlock(&pxp->mutex);
 }
 
-static void pxp_irq_work(struct work_struct *work)
+static void pxp_events_work(struct work_struct *work)
 {
-       struct xe_pxp *pxp = container_of(work, typeof(*pxp), irq.work);
+       struct xe_pxp *pxp = container_of(work, typeof(*pxp), events.work);
        struct xe_device *xe = pxp->xe;
+       bool hw_only = false;
        u32 events = 0;
 
-       spin_lock_irq(&xe->irq.lock);
-       events = pxp->irq.events;
-       pxp->irq.events = 0;
-       spin_unlock_irq(&xe->irq.lock);
+       events = atomic_xchg(&pxp->events.pending, 0);
 
        if (!events)
                return;
 
        /*
-        * If we're processing a termination irq while suspending then don't
-        * bother, we're going to re-init everything on resume anyway.
+        * If the termination request comes from an irq while we're suspending,
+        * then we can defer it to the resume path instead of waking the device
+        * up.
+        * In the case of the termination on resume the pm reference is taken
+        * in xe_pxp_pm_resume() and released here.
+        * Note that we do not expect both events to be set at the same time,
+        * but if it does happen due to a spurious interrupt we want to behave
+        * as if the only request we got was the one from the resume path; this
+        * is because the termination prep has already been done in
+        * xe_pxp_pm_resume() and it is impossible for any PXP operations to
+        * occur between the prep and the termination completion, so there is no
+        * need for a new SW prep.
         */
-       if ((events & PXP_TERMINATION_REQUEST) && !xe_pm_runtime_get_if_active(xe))
+       if (events & PXP_TERMINATION_REQUEST_ON_RESUME) {
+               events &= ~PXP_TERMINATION_REQUEST_IRQ;
+               hw_only = true;
+       }
+
+       if ((events & PXP_TERMINATION_REQUEST_IRQ) && !xe_pm_runtime_get_if_active(xe))
                return;
 
        if (events & PXP_TERMINATION_REQUEST) {
-               events &= ~PXP_TERMINATION_COMPLETE;
-               pxp_terminate(pxp);
+               events &= ~PXP_TERMINATION_COMPLETE_IRQ;
+               pxp_terminate(pxp, hw_only);
        }
 
-       if (events & PXP_TERMINATION_COMPLETE)
+       if (events & PXP_TERMINATION_COMPLETE_IRQ)
                pxp_terminate_complete(pxp);
 
        if (events & PXP_TERMINATION_REQUEST)
@@ -296,20 +331,18 @@ void xe_pxp_irq_handler(struct xe_device *xe, u16 iir)
                return;
        }
 
-       lockdep_assert_held(&xe->irq.lock);
-
        if (unlikely(!iir))
                return;
 
        if (iir & (KCR_PXP_STATE_TERMINATED_INTERRUPT |
                   KCR_APP_TERMINATED_PER_FW_REQ_INTERRUPT))
-               pxp->irq.events |= PXP_TERMINATION_REQUEST;
+               atomic_or(PXP_TERMINATION_REQUEST_IRQ, &pxp->events.pending);
 
        if (iir & KCR_PXP_STATE_RESET_COMPLETE_INTERRUPT)
-               pxp->irq.events |= PXP_TERMINATION_COMPLETE;
+               atomic_or(PXP_TERMINATION_COMPLETE_IRQ, &pxp->events.pending);
 
-       if (pxp->irq.events)
-               queue_work(pxp->irq.wq, &pxp->irq.work);
+       if (atomic_read(&pxp->events.pending))
+               queue_work(pxp->events.wq, &pxp->events.work);
 }
 
 static int kcr_pxp_set_status(const struct xe_pxp *pxp, bool enable)
@@ -340,7 +373,7 @@ static void pxp_fini(void *arg)
 {
        struct xe_pxp *pxp = arg;
 
-       destroy_workqueue(pxp->irq.wq);
+       destroy_workqueue(pxp->events.wq);
        xe_pxp_destroy_execution_resources(pxp);
 
        /* no need to explicitly disable KCR since we're going to do an FLR */
@@ -402,7 +435,7 @@ int xe_pxp_init(struct xe_device *xe)
 
        INIT_LIST_HEAD(&pxp->queues.list);
        spin_lock_init(&pxp->queues.lock);
-       INIT_WORK(&pxp->irq.work, pxp_irq_work);
+       INIT_WORK(&pxp->events.work, pxp_events_work);
        pxp->xe = xe;
        pxp->gt = gt;
 
@@ -421,8 +454,8 @@ int xe_pxp_init(struct xe_device *xe)
 
        mutex_init(&pxp->mutex);
 
-       pxp->irq.wq = alloc_ordered_workqueue("pxp-wq", 0);
-       if (!pxp->irq.wq) {
+       pxp->events.wq = alloc_ordered_workqueue("pxp-wq", 0);
+       if (!pxp->events.wq) {
                err = -ENOMEM;
                goto out_free;
        }
@@ -442,7 +475,7 @@ int xe_pxp_init(struct xe_device *xe)
 out_kcr_disable:
        kcr_pxp_disable(pxp);
 out_wq:
-       destroy_workqueue(pxp->irq.wq);
+       destroy_workqueue(pxp->events.wq);
 out_free:
        drmm_kfree(&xe->drm, pxp);
 out:
@@ -889,6 +922,7 @@ wait_for_activation:
                fallthrough;
        case XE_PXP_ACTIVE:
                pxp->key_instance++;
+               pxp->needs_termination_on_resume = true;
                needs_queue_inval = true;
                break;
        }
@@ -924,6 +958,7 @@ wait_for_activation:
  */
 void xe_pxp_pm_resume(struct xe_pxp *pxp)
 {
+       bool has_pm = false;
        int err;
 
        if (!xe_pxp_is_enabled(pxp))
@@ -931,14 +966,57 @@ void xe_pxp_pm_resume(struct xe_pxp *pxp)
 
        err = kcr_pxp_enable(pxp);
 
+       /*
+        * We want to avoid the device runtime suspending before we're done with
+        * the termination queued below, so we need a runtime PM reference; we
+        * can't call the rpm functions from within the PXP lock, so we take the
+        * ref here. Note that we don't want the rpm resume code to actually run
+        * here as that would call back into this function, but as long as we
+        * don't enable DPM_FLAG_SMART_SUSPEND (which we currently do not) we're
+        * guaranteed to not be runtime suspended at this point, so we can
+        * safely use the get_noresume variant.
+        */
+       if (pxp->needs_termination_on_resume) {
+               has_pm = true;
+
+               xe_assert(pxp->xe, !dev_pm_smart_suspend(pxp->xe->drm.dev));
+               xe_pm_runtime_get_noresume(pxp->xe);
+       }
+
        mutex_lock(&pxp->mutex);
 
        xe_assert(pxp->xe, pxp->status == XE_PXP_SUSPENDED);
 
-       if (err)
+       if (err) {
                pxp->status = XE_PXP_ERROR;
-       else
+       } else {
                pxp->status = XE_PXP_NEEDS_TERMINATION;
 
+               if (pxp->needs_termination_on_resume) {
+                       pxp->needs_termination_on_resume = false;
+
+                       /*
+                        * We can't call pxp_terminate_hw directly from here
+                        * because we're not allowed to do allocations within
+                        * the rpm resume call, so we defer the termination to
+                        * the worker that we use for the termination irqs.
+                        * However, we do not want any PXP ops to go through
+                        * between the suspend completing and the worker
+                        * starting, so we need to do the termination prep
+                        * immediately, which will mark the termination as in
+                        * progress and stall PXP ops.
+                        */
+                       if (pxp_prep_for_termination(pxp)) {
+                               has_pm = false; /* move PM ref ownership to worker */
+
+                               atomic_or(PXP_TERMINATION_REQUEST_ON_RESUME, &pxp->events.pending);
+                               queue_work(pxp->events.wq, &pxp->events.work);
+                       }
+               }
+       }
+
        mutex_unlock(&pxp->mutex);
+
+       if (has_pm)
+               xe_pm_runtime_put(pxp->xe);
 }

diff --git a/drivers/gpu/drm/xe/xe_pxp_types.h b/drivers/gpu/drm/xe/xe_pxp_types.h
index ec86306e16f49ddc1cd18e70af61123609ebed99..8132a9750b6e336fb831700d531f4ac6e8e65c47 100644 (file)
--- a/drivers/gpu/drm/xe/xe_pxp_types.h
+++ b/drivers/gpu/drm/xe/xe_pxp_types.h
@@ -85,17 +85,20 @@ struct xe_pxp {
        /** @gsc_res: kernel-owned objects for PXP submissions to the GSCCS */
        struct xe_pxp_gsc_client_resources gsc_res;
 
-       /** @irq: wrapper for the worker and queue used for PXP irq support */
+       /** @events: wrapper for the worker and queue used for PXP event handling */
        struct {
-               /** @irq.work: worker that manages irq events. */
+               /** @events.work: worker that manages termination events. */
                struct work_struct work;
-               /** @irq.wq: workqueue on which to queue the irq work. */
+               /** @events.wq: workqueue on which to queue the work. */
                struct workqueue_struct *wq;
-               /** @irq.events: pending events, protected with xe->irq.lock. */
-               u32 events;
-#define PXP_TERMINATION_REQUEST  BIT(0)
-#define PXP_TERMINATION_COMPLETE BIT(1)
-       } irq;
+               /** @events.pending: pending events */
+               atomic_t pending;
+#define PXP_TERMINATION_REQUEST_IRQ            BIT(0)
+#define PXP_TERMINATION_REQUEST_ON_RESUME      BIT(1)
+#define PXP_TERMINATION_REQUEST        (PXP_TERMINATION_REQUEST_IRQ | \
+                                PXP_TERMINATION_REQUEST_ON_RESUME)
+#define PXP_TERMINATION_COMPLETE_IRQ           BIT(2)
+       } events;
 
        /** @mutex: protects the pxp status and the queue list */
        struct mutex mutex;
@@ -130,6 +133,14 @@ struct xe_pxp {
         * suspend cycles.
         */
        u32 last_suspend_key_instance;
+       /**
+        * @needs_termination_on_resume: indicates if PXP termination is needed
+        * on resume. This is set if PXP was active when we suspend and it is
+        * cleared when we queue the termination on resume. Since the suspend
+        * and resume calls cannot execute at the same time, this variable does
+        * not need to be protected by the PXP lock.
+        */
+       bool needs_termination_on_resume;
 };
 
 #endif /* _XE_PXP_TYPES_H_ */