Re: [PATCH] drm/xe/xe3p: Bump offset for PPHWSP software scratch area

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential
issue to consider:
- [High] drm/xe: LRC_PPHWSP_FLUSH_INVAL_SCRATCH_ADDR collides with job
  timestamp offset
--

commit 7647f6d3f27709eeef4ffa64a14662ab16a3295d
Author: Gustavo Sousa <gustavo.sousa@intel.com>

drm/xe/xe3p: Bump offset for PPHWSP software scratch area

This commit defines a base offset of dword 128 for the driver's PPHWSP
software scratch area and expresses the flush and PXP invalidation
scratch addresses relative to it.

> diff --git a/drivers/gpu/drm/xe/xe_lrc.h b/drivers/gpu/drm/xe/xe_lrc.h
> index f2e829e621684..77949593562c7 100644
> --- a/drivers/gpu/drm/xe/xe_lrc.h
> +++ b/drivers/gpu/drm/xe/xe_lrc.h
> @@ -51,8 +51,23 @@ struct xe_lrc_snapshot {
>  	u32 ctx_job_timestamp;
>  };
>  
> -#define LRC_PPHWSP_FLUSH_INVAL_SCRATCH_ADDR (0x34 * 4)
> -#define LRC_PPHWSP_PXP_INVAL_SCRATCH_ADDR (0x40 * 4)
> +/*
> + * The PPHWSP software scratch area starts at different locations
> + * depending on the IP:
> + *
> + *  - Before graphics version 35.10 B0-step: dword #34.
> + *  - Starting with graphics version 35.10 B0: dword #36.
> + *
> + * For our driver usage, we use a custom base that fits all supported
> + * IPs and adds some extra buffer in case future hardware updates end
> + * up reserving extra dwords and further pushing the base of the
> + * scratch area.
> + */
> +#define LRC_PPHWSP_SCRATCH_BASE			(128 * 4)
> +#define LRC_PPHWSP_SCRATCH_DW(idx)		(LRC_PPHWSP_SCRATCH_BASE + (idx) * 4)
> +
> +#define LRC_PPHWSP_FLUSH_INVAL_SCRATCH_ADDR	LRC_PPHWSP_SCRATCH_DW(0)
> +#define LRC_PPHWSP_PXP_INVAL_SCRATCH_ADDR	LRC_PPHWSP_SCRATCH_DW(12)

[Severity: High]
Does setting LRC_PPHWSP_SCRATCH_BASE to (128 * 4) make
LRC_PPHWSP_FLUSH_INVAL_SCRATCH_ADDR collide with
LRC_CTX_JOB_TIMESTAMP_OFFSET in drivers/gpu/drm/xe/xe_lrc.c?

drivers/gpu/drm/xe/xe_lrc.c:
#define LRC_CTX_JOB_TIMESTAMP_OFFSET 512
...
static u32 __xe_lrc_ctx_job_timestamp_offset(struct xe_lrc *lrc)
{
	/* This is stored in the driver-defined portion of PPHWSP */
	return xe_lrc_pphwsp_offset(lrc) + LRC_CTX_JOB_TIMESTAMP_OFFSET;
}

In __emit_job_gen12_render_compute(), emit_copy_timestamp() stores the
job start timestamp at PPHWSP + 512, and emit_pipe_invalidate()
immediately follows with a PIPE_CONTROL_STORE_DATA_INDEX write of 0 to
PPHWSP + LRC_PPHWSP_FLUSH_INVAL_SCRATCH_ADDR (now also 512).  That
zeroes ctx_job_timestamp before the batch runs, causing check_timeout()
in drivers/gpu/drm/xe/xe_guc_submit.c to treat the total cumulative
context runtime as the current job's runtime and trigger false-positive
job timeouts and engine resets.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261106-xe3p-bump-pphwsp-scratch-v1-1-d1b3e64a4e0a@intel.com?part=1