Re: [PATCH] drm/xe/guc: Let xe_guc_ct_send_locked() callers reserve G2H credit

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

On Tue, Sep 29, 2026 at 10:33:42AM +0530, Sujaritha Sundaresan wrote:
> Commit d4a7e1611716 ("drm/xe: batch CT pagefault acks with periodic
> flush") added a defer_flush argument to xe_guc_ct_send_locked(). At the
> same time it dropped the g2h_len and num_g2h arguments, which had no
> in-tree users, and hardcoded a zero G2H reservation.
> 
> That leaves no way to send an H2G that expects a G2H reply while
> already holding ct->lock. The G2H receive path (parse_g2h_event() and
> g2h_fast_path()) releases credit unconditionally for replies such as
> DEREGISTER_CONTEXT_DONE, SCHED_CONTEXT_MODE_DONE and
> TLB_INVALIDATION_DONE. If the credit was never reserved,
> __g2h_release_space() reports an "Invalid G2H release" and marks the CT
> dead with CT_DEAD_G2H_RELEASE.
> 
> Replacing two u32 arguments with a bool also means a caller written
> against the old signature can silently change meaning when the two
> reservation arguments get merged into one: the call still compiles,
> still sends the H2G, and loses only the reservation.
> 
> Add g2h_len and num_g2h back, ahead of defer_flush, so that the
> function matches xe_guc_ct_send() and forwards both values to
> guc_ct_send_locked(). The kernel-doc now says when a reservation is
> required and that a non-zero reservation must not be made from a G2H
> handler, because waiting for credit can dequeue G2H inline.
> 
> The only existing caller, the pagefault ack path, does not expect a
> reply. It passes 0, 0 and behaves exactly as before.
> 
> Cc: Matthew Brost <matthew.brost@intel.com>

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

> Signed-off-by: Sujaritha Sundaresan <sujaritha.sundaresan@intel.com>
> ---
>  drivers/gpu/drm/xe/xe_guc_ct.c        | 17 +++++++++++++++--
>  drivers/gpu/drm/xe/xe_guc_ct.h        |  2 +-
>  drivers/gpu/drm/xe/xe_guc_pagefault.c |  3 ++-
>  3 files changed, 18 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/xe/xe_guc_ct.c b/drivers/gpu/drm/xe/xe_guc_ct.c
> index 5c393aa29de3..da5ce97484b8 100644
> --- a/drivers/gpu/drm/xe/xe_guc_ct.c
> +++ b/drivers/gpu/drm/xe/xe_guc_ct.c
> @@ -1290,11 +1290,23 @@ int xe_guc_ct_send(struct xe_guc_ct *ct, const u32 *action, u32 len,
>   * @ct: GuC CT object
>   * @action: payload dwords (HxG header dword is expected at @action[-1])
>   * @len: number of payload dwords in @action
> + * @g2h_len: G2H response space to reserve in dwords, or 0
> + * @num_g2h: number of G2H messages expected, or 0
>   * @defer_flush: defer publishing/doorbell for batching
>   *
>   * Sends a single H2G message to the GuC CT buffer while the caller already
>   * holds @ct->lock.
>   *
> + * Callers that expect the GuC to reply with a G2H message must reserve the
> + * matching credit via @g2h_len / @num_g2h. The G2H receive path releases that
> + * credit unconditionally when the reply arrives, so failing to reserve it here
> + * corrupts the G2H accounting and kills the CT channel.
> + *
> + * Reserving G2H credit may wait for, and process, outstanding G2H messages, so
> + * this must not be called with a non-zero @g2h_len from a G2H handler. Handlers
> + * should use xe_guc_ct_send_g2h_handler(), with any reply credit reserved up
> + * front by the H2G that triggered the handler.
> + *
>   * If @defer_flush is false, the function completes the submission immediately:
>   * it makes the payload visible to the device, updates the H2G descriptor and
>   * rings the GuC doorbell.
> @@ -1312,11 +1324,12 @@ int xe_guc_ct_send(struct xe_guc_ct *ct, const u32 *action, u32 len,
>   *   Must be called with @ct->lock held.
>   */
>  int xe_guc_ct_send_locked(struct xe_guc_ct *ct, const u32 *action, u32 len,
> -			  bool defer_flush)
> +			  u32 g2h_len, u32 num_g2h, bool defer_flush)
>  {
>  	int ret;
>  
> -	ret = guc_ct_send_locked(ct, action, len, 0, 0, NULL, defer_flush);
> +	ret = guc_ct_send_locked(ct, action, len, g2h_len, num_g2h, NULL,
> +				 defer_flush);
>  	if (ret == -EDEADLK)
>  		kick_reset(ct);
>  
> diff --git a/drivers/gpu/drm/xe/xe_guc_ct.h b/drivers/gpu/drm/xe/xe_guc_ct.h
> index 3ddc665ab84a..e2a33249d65d 100644
> --- a/drivers/gpu/drm/xe/xe_guc_ct.h
> +++ b/drivers/gpu/drm/xe/xe_guc_ct.h
> @@ -56,7 +56,7 @@ static inline void xe_guc_ct_irq_handler(struct xe_guc_ct *ct)
>  int xe_guc_ct_send(struct xe_guc_ct *ct, const u32 *action, u32 len,
>  		   u32 g2h_len, u32 num_g2h);
>  int xe_guc_ct_send_locked(struct xe_guc_ct *ct, const u32 *action, u32 len,
> -			  bool defer_flush);
> +			  u32 g2h_len, u32 num_g2h, bool defer_flush);
>  int xe_guc_ct_send_recv(struct xe_guc_ct *ct, const u32 *action, u32 len,
>  			u32 *response_buffer);
>  static inline int
> diff --git a/drivers/gpu/drm/xe/xe_guc_pagefault.c b/drivers/gpu/drm/xe/xe_guc_pagefault.c
> index df237fd40551..6b48550e7579 100644
> --- a/drivers/gpu/drm/xe/xe_guc_pagefault.c
> +++ b/drivers/gpu/drm/xe/xe_guc_pagefault.c
> @@ -55,7 +55,8 @@ static void guc_ack_fault(struct xe_pagefault *pf, int err)
>  	bool write_only = guc->pagefault_ack_counter++ &
>  		(XE_GUC_PAGEFAULT_FLUSH_PERIOD - 1);
>  
> -	xe_guc_ct_send_locked(&guc->ct, action, ARRAY_SIZE(action),
> +	/* Pagefault acks are fire-and-forget, no G2H reply expected. */
> +	xe_guc_ct_send_locked(&guc->ct, action, ARRAY_SIZE(action), 0, 0,
>  			      write_only);
>  }
>  
> -- 
> 2.34.1
>