Re: [PATCH v3 1/1] drm/xe/gsc: Fix GSC proxy cleanup on early initialization failure

[Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>]

On 1/29/2026 3:11 PM, Zhanjun Dong wrote:
> xe_gsc_proxy_remove undoes what is done in both xe_gsc_proxy_init and
> xe_gsc_proxy_start; however, if we fail between those 2 calls, it is
> possible that the HW forcewake access hasn't been initialized yet and so
> we hit errors when the cleanup code tries to write GSC register. To
> avoid that, split the cleanup in 2 functions so that the HW cleanup is
> only called if the HW setup was completed successfully.
>
> Since the HW cleanup (interrupt disabling) is now removed from
> xe_gsc_proxy_remove, the cleanup on error paths in xe_gsc_proxy_start
> must be updated to disable interrupts before returning.
>
> Fixes: ff6cd29b690b ("drm/xe: Cleanup unwind of gt initialization")
> Signed-off-by: Zhanjun Dong <zhanjun.dong@intel.com>
> Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>

Reviewed-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>

Daniele

> ---
> v3:
> - Move xe_gsc_wait_for_worker_completion() to xe_gsc_proxy_stop() after
>    disabling interrupts, since the worker shouldn't be queued anymore
>    after interrupts are disabled.
> - Update commit message to clarify that the error handling changes in
>    xe_gsc_proxy_start() are necessary due to the cleanup refactoring,
>    not a separate fix.
>
> v2:
> - Split cleanup into two functions: xe_gsc_proxy_remove() for SW cleanup
>    and xe_gsc_proxy_stop() for HW cleanup that requires forcewake access.
> - Add error handling in xe_gsc_proxy_start to disable interrupts on
>    early error exits.
> ---
>   drivers/gpu/drm/xe/xe_gsc_proxy.c | 46 ++++++++++++++++++++-----------
>   1 file changed, 30 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/gpu/drm/xe/xe_gsc_proxy.c b/drivers/gpu/drm/xe/xe_gsc_proxy.c
> index 42438b21f235..591baf98e445 100644
> --- a/drivers/gpu/drm/xe/xe_gsc_proxy.c
> +++ b/drivers/gpu/drm/xe/xe_gsc_proxy.c
> @@ -444,18 +444,6 @@ static void xe_gsc_proxy_remove(void *arg)
>   	if (!gsc->proxy.component_added)
>   		return;
>   
> -	/* disable HECI2 IRQs */
> -	scoped_guard(xe_pm_runtime, xe) {
> -		CLASS(xe_force_wake, fw_ref)(gt_to_fw(gt), XE_FW_GSC);
> -		if (!fw_ref.domains)
> -			xe_gt_err(gt, "failed to get forcewake to disable GSC interrupts\n");
> -
> -		/* try do disable irq even if forcewake failed */
> -		gsc_proxy_irq_toggle(gsc, false);
> -	}
> -
> -	xe_gsc_wait_for_worker_completion(gsc);
> -
>   	component_del(xe->drm.dev, &xe_gsc_proxy_component_ops);
>   	gsc->proxy.component_added = false;
>   }
> @@ -502,6 +490,25 @@ int xe_gsc_proxy_init(struct xe_gsc *gsc)
>   	return devm_add_action_or_reset(xe->drm.dev, xe_gsc_proxy_remove, gsc);
>   }
>   
> +static void xe_gsc_proxy_stop(void *arg)
> +{
> +	struct xe_gsc *gsc = arg;
> +	struct xe_gt *gt = gsc_to_gt(gsc);
> +	struct xe_device *xe = gt_to_xe(gt);
> +
> +	/* disable HECI2 IRQs */
> +	scoped_guard(xe_pm_runtime, xe) {
> +		CLASS(xe_force_wake, fw_ref)(gt_to_fw(gt), XE_FW_GSC);
> +		if (!fw_ref.domains)
> +			xe_gt_err(gt, "failed to get forcewake to disable GSC interrupts\n");
> +
> +		/* try do disable irq even if forcewake failed */
> +		gsc_proxy_irq_toggle(gsc, false);
> +	}
> +
> +	xe_gsc_wait_for_worker_completion(gsc);
> +}
> +
>   /**
>    * xe_gsc_proxy_start() - start the proxy by submitting the first request
>    * @gsc: the GSC uC
> @@ -510,6 +517,8 @@ int xe_gsc_proxy_init(struct xe_gsc *gsc)
>    */
>   int xe_gsc_proxy_start(struct xe_gsc *gsc)
>   {
> +	struct xe_gt *gt = gsc_to_gt(gsc);
> +	struct xe_device *xe = gt_to_xe(gt);
>   	int err;
>   
>   	/* enable the proxy interrupt in the GSC shim layer */
> @@ -521,12 +530,17 @@ int xe_gsc_proxy_start(struct xe_gsc *gsc)
>   	 */
>   	err = xe_gsc_proxy_request_handler(gsc);
>   	if (err)
> -		return err;
> +		goto err_irq_disable;
>   
>   	if (!xe_gsc_proxy_init_done(gsc)) {
> -		xe_gt_err(gsc_to_gt(gsc), "GSC FW reports proxy init not completed\n");
> -		return -EIO;
> +		xe_gt_err(gt, "GSC FW reports proxy init not completed\n");
> +		err = -EIO;
> +		goto err_irq_disable;
>   	}
>   
> -	return 0;
> +	return devm_add_action_or_reset(xe->drm.dev, xe_gsc_proxy_stop, gsc);
> +
> +err_irq_disable:
> +	gsc_proxy_irq_toggle(gsc, false);
> +	return err;
>   }