On 10/6/26 00:13, Eva Crystal wrote:
On Mon, Oct 05, 2026 at 09:22:26PM -0700, David Zhang wrote:
When the device is runtime-suspended, pm_runtime_forbid() synchronously
resumes the device via rpm_resume(), which invokes
amdxdna_pm_runtime_resume(). Because amdxdna_pm_runtime_resume()
acquires dev_lock, calling amdxdna_pm_fini() inside ops->fini() while
holding dev_lock in amdxdna_remove() causes a deadlock.
- amdxdna_pm_fini(xdna);
aie2_hw_stop(xdna);
aie2_hwctx_sched_fini(xdna->dev_handle);
This is worth more than its position in the series suggests: the deadlock is
already live on shipping AIE2 parts, not only on the new AIE4 path.
On current drm-misc-next, amdxdna_remove() holds dev_lock across ops->fini(xdna)
(drivers/accel/amdxdna/amdxdna_pci_drv.c:457 and
drivers/accel/amdxdna/amdxdna_pci_drv.c:463 at 34e9ab018249), and aie2_fini() opens with
amdxdna_pm_fini() (drivers/accel/amdxdna/aie2_pci.c:640 at the same commit).
pm_runtime_forbid() then calls rpm_resume(dev, 0) synchronously
(drivers/base/power/runtime.c:1672), which lands in amdxdna_pm_resume() and its
guard(mutex)(&xdna->dev_lock) on the same task. The base wires
RUNTIME_PM_OPS(amdxdna_pm_suspend, amdxdna_pm_resume, NULL) and aie2_ops supplies
.suspend and .resume, so runtime PM is active on aie2 before this series adds
.runtime_suspend. With amdxdna_pm_init() setting a 5000 ms autosuspend delay then
pm_runtime_allow(), an unbind or rmmod more than five seconds after the last NPU access
hangs holding dev_lock.
On the remove path, pm_runtime_forbid() does not call
amdxdna_pm_resume(). The PCI core has already resumed the device.
pci_device_remove() calls pm_runtime_get_sync() and pm_runtime_barrier()
before amdxdna_remove(). That get resumes a runtime-suspended device,
and the usage count stays elevated through aie2_fini().
pm_runtime_forbid() then enters rpm_resume() with status RPM_ACTIVE,
which returns immediately and does not run ->runtime_resume()
Lizhi
Three things that would help it travel:
* Fixes: 1aa82181a3c2 ("accel/amdxdna: Fix dead lock for suspend and resume")
looks right. amdxdna_pm.c had no dev_lock when 063db451832b created it, and 1aa82181a3c2
adds exactly the two guards the base still carries.
* Cc: [email protected] is warranted, since 1aa82181a3c2 is in v7.0 and
later.
* Could this be split out to drm-misc-fixes on its own? At position 16 of a 20
patch AIE4 series it is unlikely to be picked up as a fix, and splitting it
stops the fixes cadence holding up the feature work.
One question: amdxdna_pm_fini() now runs after drm_dev_unplug(), so
pm_runtime_forbid() resumes hardware on a device already unregistered with its
user mappings torn down. Intended?
Eva Crystal (0xiviel)
XSource Security
https://xsourcesec.com
