[Intel-xe] [PATCH 08/12] drm/xe/gsc: Trigger a driver flr to cleanup the GSC on unload

[John Harrison]

On 10/27/2023 15:29, Daniele Ceraolo Spurio wrote:
> GSC is only killed by an FLR, so we need to trigger one on unload to
> make sure we stop it. This is because we assign a chunk of memory to
> the GSC as part of the FW load, so we need to make sure it stops
> using it when we release it to the system on driver unload. Note that
> this is not a problem of the unload per-se, because the GSC will not
> touch that memory unless there are requests for it coming from the
> driver; therefore, no accesses will happen while Xe is not loaded,
> but if we re-load the driver then the GSC might wake up and try to
> access that old memory location again.
>
> Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio at intel.com>
> Cc: Alan Previn <alan.previn.teres.alexis at intel.com>
Reviewed-by: John Harrison <John.C.Harrison at Intel.com>

> ---
>   drivers/gpu/drm/xe/xe_gsc.c | 18 ++++++++++++++++++
>   1 file changed, 18 insertions(+)
>
> diff --git a/drivers/gpu/drm/xe/xe_gsc.c b/drivers/gpu/drm/xe/xe_gsc.c
> index 7e5ba9039163..edf74780322a 100644
> --- a/drivers/gpu/drm/xe/xe_gsc.c
> +++ b/drivers/gpu/drm/xe/xe_gsc.c
> @@ -106,6 +106,7 @@ static int gsc_fw_wait(struct xe_gt *gt)
>   static int gsc_upload(struct xe_gsc *gsc)
>   {
>   	struct xe_gt *gt = gsc_to_gt(gsc);
> +	struct xe_device *xe = gt_to_xe(gt);
>   	int err;
>   
>   	if (XE_WARN_ON(!xe_uc_fw_is_loadable(&gsc->fw) || !gsc->q || gsc_fw_is_loaded(gt)))
> @@ -117,6 +118,23 @@ static int gsc_upload(struct xe_gsc *gsc)
>   		return err;
>   	}
>   
> +	/*
> +	 * GSC is only killed by an FLR, so we need to trigger one on unload to
> +	 * make sure we stop it. This is because we assign a chunk of memory to
> +	 * the GSC as part of the FW load, so we need to make sure it stops
> +	 * using it when we release it to the system on driver unload. Note that
> +	 * this is not a problem of the unload per-se, because the GSC will not
> +	 * touch that memory unless there are requests for it coming from the
> +	 * driver; therefore, no accesses will happen while Xe is not loaded,
> +	 * but if we re-load the driver then the GSC might wake up and try to
> +	 * access that old memory location again.
> +	 * Given that an FLR is a very disruptive action (see the FLR function
> +	 * for details), we want to do it as the last action before releasing
> +	 * the access to the MMIO bar, which means we need to do it as part of
> +	 * mmio cleanup.
> +	 */
> +	xe->needs_flr_on_fini = true;
> +
>   	err = emit_gsc_upload(gsc);
>   	if (err) {
>   		xe_gt_err(gt, "Failed to emit GSC FW upload (%pe)\n", ERR_PTR(err));