Re: [PATCH v2 1/1] drm/xe/guc: Exclude indirect ring state page from ADS engine state size

[Matthew Brost <matthew.brost@intel.com>]

On Mon, May 04, 2026 at 09:49:26AM +0000, Satyanarayana K V P wrote:
> The engine state size reported to GuC via ADS should only include the
> engine state portion and should not include the indirect ring state page
> that comes after it in the context image. The GuC uses this size to
> overwrite the engine state in the LRC on watchdog resets and we don't
> want it to overwrite the indirect ring state as well.
> 

I actually noticed this well over a year ago when looking at a indirect
ring state problem.

> Fixes: d6219e1cd5e3 ("drm/xe: Add Indirect Ring State support")
> Suggested-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
> Signed-off-by: Satyanarayana K V P <satyanarayana.k.v.p@intel.com>
> Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
> Cc: Matthew Brost <matthew.brost@intel.com>

This looks right to me, so:

Reviewed-by: Matthew Brost <matthew.brost@intel.com>

But also since Daniele reviewed v1, ensure he doesn't have any other
comments before merging this.

Matt

> ---
> V1 -> V2:
> - Replaced xe_lrc_skip_size() with xe_lrc_engine_state_size().
> - Updated commit message (Daniel).
> ---
>  drivers/gpu/drm/xe/xe_guc_ads.c |  5 +----
>  drivers/gpu/drm/xe/xe_lrc.c     | 11 +++++++++--
>  drivers/gpu/drm/xe/xe_lrc.h     |  2 +-
>  3 files changed, 11 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/gpu/drm/xe/xe_guc_ads.c b/drivers/gpu/drm/xe/xe_guc_ads.c
> index ce651da6f318..b9bca6084a4f 100644
> --- a/drivers/gpu/drm/xe/xe_guc_ads.c
> +++ b/drivers/gpu/drm/xe/xe_guc_ads.c
> @@ -515,12 +515,9 @@ static void guc_golden_lrc_init(struct xe_guc_ads *ads)
>  		 * that starts after the execlists LRC registers. This is
>  		 * required to allow the GuC to restore just the engine state
>  		 * when a watchdog reset occurs.
> -		 * We calculate the engine state size by removing the size of
> -		 * what comes before it in the context image (which is identical
> -		 * on all engines).
>  		 */
>  		ads_blob_write(ads, ads.eng_state_size[guc_class],
> -			       real_size - xe_lrc_skip_size(xe));
> +			       xe_lrc_engine_state_size(gt, class));
>  		ads_blob_write(ads, ads.golden_context_lrca[guc_class],
>  			       addr_ggtt);
>  
> diff --git a/drivers/gpu/drm/xe/xe_lrc.c b/drivers/gpu/drm/xe/xe_lrc.c
> index 9db914584347..fdfe2ed5f683 100644
> --- a/drivers/gpu/drm/xe/xe_lrc.c
> +++ b/drivers/gpu/drm/xe/xe_lrc.c
> @@ -727,9 +727,16 @@ size_t xe_lrc_reg_size(struct xe_device *xe)
>  		return 80 * sizeof(u32);
>  }
>  
> -size_t xe_lrc_skip_size(struct xe_device *xe)
> +/**
> + * xe_lrc_engine_state_size() - Get size of the engine state within LRC
> + * @gt: the &xe_gt struct instance
> + * @class: Hardware engine class
> + *
> + * Returns: Size of the engine state
> + */
> +size_t xe_lrc_engine_state_size(struct xe_gt *gt, enum xe_engine_class class)
>  {
> -	return LRC_PPHWSP_SIZE + xe_lrc_reg_size(xe);
> +	return xe_gt_lrc_hang_replay_size(gt, class) - xe_lrc_reg_size(gt_to_xe(gt));
>  }
>  
>  static inline u32 __xe_lrc_seqno_offset(struct xe_lrc *lrc)
> diff --git a/drivers/gpu/drm/xe/xe_lrc.h b/drivers/gpu/drm/xe/xe_lrc.h
> index e7c975f9e2d9..5440663183f6 100644
> --- a/drivers/gpu/drm/xe/xe_lrc.h
> +++ b/drivers/gpu/drm/xe/xe_lrc.h
> @@ -130,7 +130,7 @@ u32 xe_lrc_parallel_ggtt_addr(struct xe_lrc *lrc);
>  struct iosys_map xe_lrc_parallel_map(struct xe_lrc *lrc);
>  
>  size_t xe_lrc_reg_size(struct xe_device *xe);
> -size_t xe_lrc_skip_size(struct xe_device *xe);
> +size_t xe_lrc_engine_state_size(struct xe_gt *gt, enum xe_engine_class class);
>  
>  void xe_lrc_dump_default(struct drm_printer *p,
>  			 struct xe_gt *gt,
> -- 
> 2.43.0
>