Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider: - [High] amdxdna: Missing runtime PM reference during hardware context destruction - [Low] amdxdna: Dead code due to unconditional assignment of PM callbacks in aie2_ops --
commit e84dc9f8e09a4669bbdb0e93c4368ea773cd8d73 Author: David Zhang <[email protected]> accel/amdxdna: Implement runtime suspend and resume support Add runtime suspend/resume for the AIE4 classic device by using its system suspend/resume handlers. > diff --git a/drivers/accel/amdxdna/aie2_pci.c > b/drivers/accel/amdxdna/aie2_pci.c > index 690ad035463c5..a3866e8933e61 100644 > --- a/drivers/accel/amdxdna/aie2_pci.c > +++ b/drivers/accel/amdxdna/aie2_pci.c > @@ -1221,6 +1221,8 @@ const struct amdxdna_dev_ops aie2_ops = { > .fini = aie2_fini, > .resume = aie2_hw_resume, > .suspend = aie2_hw_suspend, > + .runtime_resume = aie2_hw_resume, > + .runtime_suspend = aie2_hw_suspend, [Severity: Low] Are pm_ptr() wrappers missing here? Unlike aie4_classic_ops, which correctly wraps these assignments in pm_ptr(), aie2_ops omits the wrappers. When the power management configuration is disabled, the compiler cannot automatically discard these functions, resulting in dead code being linked into the kernel. > .get_aie_info = aie2_get_info, > .set_aie_state = aie2_set_state, > .hwctx_init = aie2_hwctx_init, [ ... ] > diff --git a/drivers/accel/amdxdna/aie4_pci.c > b/drivers/accel/amdxdna/aie4_pci.c > index 70a052ab5f775..60140c3927a2e 100644 > --- a/drivers/accel/amdxdna/aie4_pci.c > +++ b/drivers/accel/amdxdna/aie4_pci.c > @@ -1041,6 +1041,7 @@ static int aie4_classic_init(struct amdxdna_dev *xdna) > if (ret) > goto free_work_buf; > > + amdxdna_pm_init(xdna); [Severity: High] Could this introduce a bug when destroying hardware contexts while the device is suspended? The driver enables runtime autosuspend here, allowing the device to enter a suspended state when idle. When a user destroys a hardware context, the destruction path runs without ensuring the device is active: amdxdna_drm_destroy_hwctx_ioctl() amdxdna_hwctx_destroy_rcu() aie4_hwctx_stop() aie4_hwctx_destroy() aie4_msg_destroy_context() None of these destruction paths call amdxdna_pm_resume_get() to wake up the device. If the device has autosuspended, the firmware is stopped and the mailbox is finalized (via aie4_mailbox_fini()). Accessing the mailbox registers in this state will cause a bus error, system hang, or kernel crash. > return 0; > > free_work_buf: -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=19
