Re: [PATCH i-g-t 1/3] lib/xe/xe_spin: fix premature batch exit on BO resubmission

On Fri, Jul 31, 2026 at 04:18:04PM +0200, Marcin Bernatowicz wrote:
> COND_BBE reads ticks_delta from memory. When the same BO is resubmitted,
> COND_BBE can read the value left by the previous batch(elapsed >= ctx_ticks
> at that batch's exit), satisfying the exit condition on the very first loop
> iteration before any new elapsed time is stored.
> 
> Write sentinel 0 to ticks_delta at batch start and use
> MI_SEMAPHORE_WAIT to poll until it is visible in memory before entering
> the loop. This guarantees COND_BBE cannot read the stale value from the
> previous batch.
> 
> Also switch from inverted ticks_delta logic (STOREINV + ~ctx_ticks) to
> direct elapsed ticks (STORE + MAD_LT_IDD + ctx_ticks) for readability.
> The pad loop between SRM and COND_BBE is removed as it does not help.
> 
> Suggested-by: Zbigniew Kempczyński <[email protected]>
> Signed-off-by: Marcin Bernatowicz <[email protected]>
> Cc: Adam Miszczak <[email protected]>
> Cc: Kamil Konieczny <[email protected]>
> Cc: Lukasz Laguna <[email protected]>
> Cc: Zbigniew Kempczyński <[email protected]>
> ---
>  lib/xe/xe_spin.c | 31 +++++++++++++++++--------------
>  1 file changed, 17 insertions(+), 14 deletions(-)
> 
> diff --git a/lib/xe/xe_spin.c b/lib/xe/xe_spin.c
> index 874310789..2863d2163 100644
> --- a/lib/xe/xe_spin.c
> +++ b/lib/xe/xe_spin.c
> @@ -52,7 +52,6 @@ void xe_spin_init(struct xe_spin *spin, struct xe_spin_opts *opts)
>  	uint64_t start_addr = opts->addr + offsetof(struct xe_spin, start);
>  	uint64_t end_addr = opts->addr + offsetof(struct xe_spin, end);
>  	uint64_t ticks_delta_addr = opts->addr + offsetof(struct xe_spin, ticks_delta);
> -	uint64_t pad_addr = opts->addr + offsetof(struct xe_spin, pad);
>  	uint64_t timestamp_addr = opts->addr + offsetof(struct xe_spin, timestamp);
>  	int b = 0;
>  	uint32_t devid;
> @@ -77,6 +76,18 @@ void xe_spin_init(struct xe_spin *spin, struct xe_spin_opts *opts)
>  	spin->batch[b++] = start_addr >> 32;
>  	spin->batch[b++] = 0xc0ffee;
>  
> +	if (opts->ctx_ticks) {
> +		/* Write sentinel and wait until it is visible in memory before loop. */
> +		spin->batch[b++] = MI_STORE_DWORD_IMM_GEN4;
> +		spin->batch[b++] = ticks_delta_addr;
> +		spin->batch[b++] = ticks_delta_addr >> 32;
> +		spin->batch[b++] = 0;
> +		spin->batch[b++] = MI_SEMAPHORE_WAIT | MI_SEMAPHORE_POLL | MI_SEMAPHORE_SAD_EQ_SDD;
> +		spin->batch[b++] = 0;
> +		spin->batch[b++] = ticks_delta_addr;
> +		spin->batch[b++] = ticks_delta_addr >> 32;
> +	}
> +

Great, it is more reliable than resetting ticks_delta outside spinner code.

>  	loop_addr = opts->addr + b * sizeof(uint32_t);
>  
>  	if (opts->preempt)
> @@ -101,12 +112,12 @@ void xe_spin_init(struct xe_spin *spin, struct xe_spin_opts *opts)
>  		spin->batch[b++] = opts->use_queue_timestamp ? QUEUE_TIMESTAMP : CTX_TIMESTAMP;
>  		spin->batch[b++] = CS_GPR(NOW_TS);
>  
> -		/* delta = now - start; inverted to match COND_BBE */
> +		/* delta = now - start */
>  		spin->batch[b++] = MI_MATH(4);
>  		spin->batch[b++] = MI_MATH_LOAD(MI_MATH_REG_SRCA, MI_MATH_REG(NOW_TS));
>  		spin->batch[b++] = MI_MATH_LOAD(MI_MATH_REG_SRCB, MI_MATH_REG(START_TS));
>  		spin->batch[b++] = MI_MATH_SUB;
> -		spin->batch[b++] = MI_MATH_STOREINV(MI_MATH_REG(NOW_TS), MI_MATH_REG_ACCU);
> +		spin->batch[b++] = MI_MATH_STORE(MI_MATH_REG(NOW_TS), MI_MATH_REG_ACCU);
>  
>  		/* Save delta for reading by COND_BBE */
>  		spin->batch[b++] = MI_STORE_REGISTER_MEM_GEN8 | MI_SRM_CS_MMIO;
> @@ -114,17 +125,9 @@ void xe_spin_init(struct xe_spin *spin, struct xe_spin_opts *opts)
>  		spin->batch[b++] = ticks_delta_addr;
>  		spin->batch[b++] = ticks_delta_addr >> 32;
>  
> -		/* Delay between SRM and COND_BBE to post the writes */
> -		for (int n = 0; n < 8; n++) {
> -			spin->batch[b++] = MI_STORE_DWORD_IMM_GEN4;
> -			spin->batch[b++] = pad_addr;
> -			spin->batch[b++] = pad_addr >> 32;
> -			spin->batch[b++] = 0xc0ffee;
> -		}
> -
> -		/* Break if delta [time elapsed] > ns */
> -		spin->batch[b++] = MI_COND_BATCH_BUFFER_END | MI_DO_COMPARE | 2;
> -		spin->batch[b++] = ~(opts->ctx_ticks);
> +		/* Break if delta [time elapsed] >= ns */
> +		spin->batch[b++] = MI_COND_BATCH_BUFFER_END | MI_DO_COMPARE | MAD_LT_IDD | 2;
> +		spin->batch[b++] = opts->ctx_ticks;

And this is easier to read.

Reviewed-by: Zbigniew Kempczyński <[email protected]>

--
Zbigniew

>  		spin->batch[b++] = ticks_delta_addr;
>  		spin->batch[b++] = ticks_delta_addr >> 32;
>  	}
> -- 
> 2.43.0
>