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

["Sundaresan, Sujaritha" <sujaritha.sundaresan@intel.com>]

On 9/29/2026 11:43 PM, Matthew Brost wrote:
> 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>

Thanks for the review. The CI failures are unrelated to the changes here.

- Sujaritha Sundaresan

>
>> 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
>>