Re: [PATCH v5 3/4] drm/xe/forcewake: flush delayed release at power boundaries

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] PMU events hold long-lived forcewake references that outlive device teardown and suspend boundaries, defeating the patch's attempt to quiesce forcewake accesses before power-off.
--

--- Patch [3]: [PATCH v5 3/4] drm/xe/forcewake: flush delayed release at power boundaries ---
commit 137d6a969747b4828385eae977e1c1ba6eb0b98d
Author: Zongyao Bai <zongyao.bai@intel.com>

drm/xe/forcewake: flush delayed release at power boundaries
    
Add a synchronous forcewake flush that cancels per-domain timers and
settles pending sleep acknowledgments. Provide a scoped cleanup class
so the final put is always followed by the flush before the caller
continues toward power-off.

> diff --git a/drivers/gpu/drm/xe/xe_force_wake.c b/drivers/gpu/drm/xe/xe_force_wake.c
> index 3f5013c7424c8..ef43899e1f7d3 100644
> --- a/drivers/gpu/drm/xe/xe_force_wake.c
> +++ b/drivers/gpu/drm/xe/xe_force_wake.c
> @@ -391,6 +391,55 @@ void xe_force_wake_put_delay(struct xe_force_wake *fw, unsigned int fw_ref)
>  	__xe_force_wake_put(fw, fw_ref, true);
>  }
>  
> +/**
> + * xe_force_wake_flush - Flush delayed forcewake releases
> + * @fw: Pointer to the force wake structure
> + *
> + * Synchronously cancels delayed-release timers and completes pending sleep
> + * requests and ACK waits. This must be called after the final forcewake put
> + * and before power-off because the timer callback accesses MMIO.
> + *
> + * The caller must ensure no forcewake reference outlives this call and
> + * serialize against new delayed releases or final puts that consume a
> + * recorded delayed-release request. Timers are canceled before taking
> + * fw->lock because their callbacks acquire the same lock.
> + */
> +void xe_force_wake_flush(struct xe_force_wake *fw)
> +{
> +	struct xe_gt *gt = fw->gt;
> +	struct xe_force_wake_domain *domain;
> +	unsigned int tmp, sleep, ack_fail = 0;
> +	unsigned long flags;
> +
> +	for_each_fw_domain(domain, fw, tmp)
> +		hrtimer_cancel(&domain->sleep_timer);
> +
> +	spin_lock_irqsave(&fw->lock, flags);
> +	sleep = fw->sleep_ack_pending_domains;
> +	for_each_fw_domain(domain, fw, tmp) {
> +		assert_domain_state(fw, domain);
> +		domain->delayed_release_requested = false;
> +		if ((fw->delayed_release_domains & BIT(domain->id)) &&
> +		    !domain->ref) {

[Severity: High]
This is a pre-existing issue, but does this flush mechanism fail to quiesce
active PMU references before power-off?

If a user opens a PMU event (e.g., engine-active-ticks) and acquires a
forcewake reference, it can outlive device teardown or suspend. When
xe_force_wake_flush() runs, it skips the domain because domain->ref > 0.

If the device is unplugged or suspended while the PMU event is open, the domain
is not put to sleep due to the active reference, and the hardware loses power.
On resume, software may incorrectly believe the domain is awake, failing to send
wake requests and causing hardware timeouts or hangs on subsequent accesses.

Furthermore, on unplug, mmio_fini sets regs = NULL. When the user finally
closes the PMU event fd, xe_pmu_event_destroy() releases forcewake:

drivers/gpu/drm/xe/xe_pmu.c:xe_pmu_event_destroy() {
    ...
    xe_force_wake_put(gt_to_fw(gt), *fw_ref);
    ...
}

Will this cause a use-after-unmap logic hitting a WARN_ON_ONCE in
mmio_available when accessing the unmapped MMIO?

> +			fw->delayed_release_domains &= ~BIT(domain->id);
> +			sleep |= BIT(domain->id);
> +			domain_sleep(gt, domain);
> +		}
> +		assert_domain_state(fw, domain);
> +	}
> +	for_each_fw_domain_masked(domain, sleep, fw, tmp) {
> +		fw->sleep_ack_pending_domains &= ~BIT(domain->id);
> +		if (domain_sleep_wait(gt, domain) == 0)
> +			fw->awake_domains &= ~BIT(domain->id);
> +		else
> +			ack_fail |= BIT(domain->id);
> +	}
> +	spin_unlock_irqrestore(&fw->lock, flags);
> +
> +	xe_gt_WARN(gt, ack_fail, "Forcewake domain%s %#x failed to acknowledge sleep request\n",
> +		   str_plural(hweight_long(ack_fail)), ack_fail);
> +}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260813000654.2712317-1-zongyao.bai@intel.com?part=3