Re: [PATCH v10 10/20] drm/xe/vsec: Support possible hotplug exit

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] cleanup: mixing goto-based cleanup and scoped_guard violates guidelines
--

commit c7e9d9c776cd4cd0e4508280573de2b886711d95
Author: Michael J. Ruhl <michael.j.ruhl@intel.com>
Subject: drm/xe/vsec: Support possible hotplug exit

This commit introduces a check in the VSEC callback API to verify that the
device is valid in a hotplug context. It uses drm_dev_enter() and
drm_dev_exit() to safeguard accesses, introducing a goto-based dev_exit path
to handle early returns.

> diff --git a/drivers/gpu/drm/xe/xe_vsec.c b/drivers/gpu/drm/xe/xe_vsec.c
> --- a/drivers/gpu/drm/xe/xe_vsec.c
> +++ b/drivers/gpu/drm/xe/xe_vsec.c
[ ... ]
> @@ -188,11 +199,14 @@ int xe_pmt_telem_read(struct device *dev, u32 guid, u64 *data, loff_t user_offse
>  		xe_pm_runtime_get(xe);
>  		break;
>  	case TELEMETRY:
> -		if (!xe_pm_runtime_get_if_active(xe))
> -			return -ENODATA;
> +		if (!xe_pm_runtime_get_if_active(xe)) {
> +			ret = -ENODATA;
> +			goto dev_exit;
> +		}
>  		break;
>  	case WATCHER:
> -		return -EINVAL;
> +		ret = -EINVAL;
> +		goto dev_exit;
>  	}
>  
>  	scoped_guard(mutex, &xe->pmt.lock) {

[Severity: Low]
Does introducing goto-based cleanup (goto dev_exit) in xe_pmt_telem_read()
conflict with the pre-existing use of scoped_guard() in the same function?

The kernel cleanup guidelines in include/linux/cleanup.h state that mixing
goto-based cleanup and __free()/guard() declarations within the same
function should be avoided. Mixing these patterns can create confusing
ownership semantics and increases the risk of double-free or resource leak
bugs during future modifications.

Could the drm_dev_enter()/drm_dev_exit() pairing be handled by a cleanup
guard instead, or should the scoped_guard() be converted to standard
lock/unlock calls to maintain a single cleanup style in this function?

>  		/* set SoC re-mapper index register based on GUID memory region */
>  		xe->soc_remapper.set_telem_region(xe, mem_region);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260918131751.2408883-22-michael.j.ruhl@intel.com?part=10