Re: [PATCH v10 18/27] drm/xe: Implement SR-IOV and eudebug exclusivity

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Race condition in eudebug disable path allows SR-IOV VFs to be enabled while eudebug hardware polling is still running, causing hardware interference.
- [Medium] Unconditional drm_info() log spam during SR-IOV VF initialization.
--

commit 639131d56a88cb02a02b54b4b9a2563ae44da661
Author: Christoph Manszewski <c.manszewski@gmail.com>

drm/xe: Implement SR-IOV and eudebug exclusivity

EU debug functionality relies on access to specific mmio registers.
Since VFs don't have access to those registers and in order to avoid
interference with VFs, make SR-IOV and eudebug functionality exclusive.

> diff --git a/drivers/gpu/drm/xe/xe_eudebug.c b/drivers/gpu/drm/xe/xe_eudebug.c
> index 470246af9810a..2c8d8fc82898e 100644
> --- a/drivers/gpu/drm/xe/xe_eudebug.c
> +++ b/drivers/gpu/drm/xe/xe_eudebug.c
> @@ -2276,10 +2286,14 @@ int xe_eudebug_enable(struct xe_device *xe, bool enable)
>  	WRITE_ONCE(xe->eudebug.cap_state, enable ?
>  		   XE_EUDEBUG_CAP_ENABLED : XE_EUDEBUG_CAP_DISABLED);
>  
> -	if (enable)
> +	if (enable) {
>  		xe_eudebug_attention_poll_start(xe);
> -	else
> +	} else {
> +		if (IS_SRIOV_PF(xe))
> +			xe_sriov_pf_end_lockdown(xe);
> +
>  		xe_eudebug_attention_poll_stop(xe);
> +	}

[Severity: High]
Does releasing the SR-IOV lockdown before stopping the eudebug worker allow
VFs to be enabled while the worker is still actively reading GT MMIO 
registers?

Since xe_eudebug_attention_poll_stop() uses cancel_delayed_work() (which is
asynchronous), the polling worker may still be executing and accessing MMIO
registers after xe_sriov_pf_end_lockdown() releases the exclusivity guard.
Reversing the order of these two calls might still be susceptible to a race 
since the cancel is async.

>  
>  	return 0;
>  }
> [ ... ]
> @@ -2359,6 +2373,11 @@ void xe_eudebug_init(struct xe_device *xe)
>  	if (xe->eudebug.cap_state == XE_EUDEBUG_CAP_NOT_SUPPORTED)
>  		return;
>  
> +	if (IS_SRIOV_VF(xe)) {
> +		drm_info(&xe->drm, "eudebug not available in SR-IOV VF mode\n");
> +		goto out_not_supported;
> +	}
> +

[Severity: Medium]
Will this unconditionally spam the kernel log with drm_info messages during
SR-IOV VF initialization?

When SR-IOV is enabled, many Virtual Functions are initialized simultaneously.
Calling drm_info unconditionally during the initialization of every VF will
flood the dmesg buffer with identical "eudebug not available in SR-IOV VF 
mode" messages.

Would drm_info_once() or drm_dbg() be more appropriate here?

>  	wq = drmm_alloc_ordered_workqueue(dev, "xe-eudebug-discovery", 0);
>  	if (IS_ERR(wq)) {
>  		err = PTR_ERR(wq);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260903145952.848051-1-mika.kuoppala@linux.intel.com?part=18