Re: [PATCH v9 1/3] [PATCH] drm/xe/lrc: document sentinel and make CTX_TIMESTAMP read TOCTOU-safe

On Fri, Jul 31, 2026 at 04:00:06AM +0000, Gajendra Uttamchand wrote:
>Problem: CTX_TIMESTAMP MMIO reads could be stale if a context
>switched out between check and read; LRC stores a sentinel while
>a context starts that must not be treated as a real timestamp.
>
>Fix: Check the LRC-stored sentinel before and after the MMIO read;
>return the LRC value if the context switched out to avoid TOCTOU.
>
>Note: Keep XE_LRC_CTX_TIMESTAMP_ACTIVE in xe_lrc.h as the
>canonical sentinel.

This one needs a fixes tag.

Fixes: d243ef6a39c6 ("drm/xe/lrc: Refactor xe_lrc_timestamp to simplify logic")

Thanks,
Umesh

>
>Signed-off-by: Gajendra Uttamchand <[email protected]>
>Reviewed-by: Umesh Nerlige Ramappa <[email protected]>
>Acked-by: Matthew Brost <[email protected]>
>---
> drivers/gpu/drm/xe/xe_lrc.c | 29 +++++++++++++++++++++--------
> drivers/gpu/drm/xe/xe_lrc.h |  7 +++++++
> 2 files changed, 28 insertions(+), 8 deletions(-)
>
>diff --git a/drivers/gpu/drm/xe/xe_lrc.c b/drivers/gpu/drm/xe/xe_lrc.c
>index 3e7c995085d0..78d0456cf3c9 100644
>--- a/drivers/gpu/drm/xe/xe_lrc.c
>+++ b/drivers/gpu/drm/xe/xe_lrc.c
>@@ -1096,7 +1096,7 @@ static void xe_lrc_finish(struct xe_lrc *lrc)
>  * on until it is scheduled, we also read the ENGINE_ID MMIO in the WA BB and
>  * store it in the PPHSWP.
>  */
>-#define CONTEXT_ACTIVE 1ULL
>+#define CONTEXT_ACTIVE XE_LRC_CTX_TIMESTAMP_ACTIVE
> static ssize_t setup_utilization_wa(struct xe_lrc *lrc,
> 				    struct xe_hw_engine *hwe,
> 				    u32 *batch,
>@@ -2726,21 +2726,34 @@ static u64 xe_lrc_update_multi_queue_timestamp(struct xe_lrc *lrc, u64 *old_ts)
> static u64 xe_lrc_context_timestamp(struct xe_lrc *lrc)
> {
> 	u64 reg_ts, new_ts = lrc->ctx_timestamp;
>+	u64 stored;
>
> 	/* CTX_TIMESTAMP mmio read is invalid on VF, so return the LRC value */
> 	if (IS_SRIOV_VF(lrc_to_xe(lrc)))
> 		return xe_lrc_ctx_timestamp(lrc);
>
>-	if (context_active(lrc) &&
>-	    !get_ctx_timestamp(lrc, xe_lrc_engine_id(lrc), &reg_ts))
>+	/*
>+	 * Safely read CTX_TIMESTAMP: check the LRC-stored value before and
>+	 * after the MMIO read to avoid a TOCTOU where a context switch makes the
>+	 * MMIO value stale. If the LRC value is not `CONTEXT_ACTIVE` return it;
>+	 * otherwise accept the MMIO value only if the context remained active.
>+	 */
>+
>+	stored = xe_lrc_ctx_timestamp(lrc);
>+	if (stored != CONTEXT_ACTIVE)
>+		return stored;
>+
>+	/* Context is active: read the live timestamp from the engine's MMIO register. */
>+	if (!get_ctx_timestamp(lrc, xe_lrc_engine_id(lrc), &reg_ts))
> 		new_ts = reg_ts;
>
>-	/*
>-	 * If context swicthed out while we were here, just return the latest
>-	 * LRC CTX TIMESTAMP value.
>+	/* Re-check the LRC-stored timestamp: if the context switched out while
>+	 * reading MMIO the hardware saved the canonical timestamp into the LRC
>+	 * during context-save, so return that value instead of the MMIO read.
> 	 */
>-	if (!context_active(lrc))
>-		return xe_lrc_ctx_timestamp(lrc);
>+	stored = xe_lrc_ctx_timestamp(lrc);
>+	if (stored != CONTEXT_ACTIVE)
>+		return stored;
>
> 	return new_ts;
> }
>diff --git a/drivers/gpu/drm/xe/xe_lrc.h b/drivers/gpu/drm/xe/xe_lrc.h
>index 0a3a611391ee..7be5e3da8bc8 100644
>--- a/drivers/gpu/drm/xe/xe_lrc.h
>+++ b/drivers/gpu/drm/xe/xe_lrc.h
>@@ -9,6 +9,13 @@
>
> #include "xe_lrc_types.h"
>
>+/*
>+ * Sentinel value stored in lrc->ctx_timestamp while a context is starting.
>+ * The hardware hasn't yet written the real CTX_TIMESTAMP, so this is not a
>+ * valid elapsed-time sample and must not be used as one.
>+ */
>+#define XE_LRC_CTX_TIMESTAMP_ACTIVE 1ULL
>+
> struct drm_printer;
> struct xe_bb;
> struct xe_device;
>--
>2.43.0
>