Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [Medium] amdxdna: infinite userspace loop on AMDXDNA_WAIT_CMD during device disconnect --
commit 59707c18aa0b6cb9051b6124aa8c05886c58491f Author: David Zhang <[email protected]> accel/amdxdna: Prepare for AIE4 command submission This commit prepares the data structures and completion wait helpers for AIE4 command submission. It extends data structures for ERT_START_DPU payload and queue list linkage. It also updates wait helpers to detect asynchronous device disconnects and handle non-atomic read index updates. > diff --git a/drivers/accel/amdxdna/aie4_ctx.c > b/drivers/accel/amdxdna/aie4_ctx.c > index 226570367f71f..09c92b1b5134e 100644 > --- a/drivers/accel/amdxdna/aie4_ctx.c > +++ b/drivers/accel/amdxdna/aie4_ctx.c [ ... ] > @@ -437,11 +449,21 @@ int aie4_cmd_wait(struct amdxdna_hwctx *hwctx, u64 seq, > u32 timeout) > wait_jifs = msecs_to_jiffies(timeout); > > ret = wait_event_interruptible_timeout(cert_comp->waitq, > - (check_cmd_done(hwctx, seq)), > + check_cmd_done(hwctx, seq, > cert_comp), > wait_jifs); > > - if (!ret) > + if (!ret) { > ret = -ETIME; > + } else if (ret > 0 && !check_cert_comp_linked(hwctx, cert_comp) && > + get_read_index(hwctx) <= seq) { > + /* > + * Disconnected, or read_index invalid (0), before completion > was > + * seen. -EAGAIN is by design: the caller retries the wait > until a > + * valid read_index is observed. Command state and fence are not > + * affected. > + */ > + ret = -EAGAIN; [Severity: Medium] When the device is disconnected, does returning -EAGAIN here cause an infinite loop in userspace? If the device disconnects or resets, the cert_comp is unlinked, but the context is not removed from client->hwctx_xa during teardown. Consequently, amdxdna_drm_wait_cmd_ioctl() will successfully look up the context and invoke aie4_cmd_wait() again. Because amdxdna_drm_wait_cmd_ioctl() lacks a drm_dev_enter() check to abort early on unplug, userspace will receive -EAGAIN and instantly retry. On the next attempt, it hits the !cert_comp check at the start of aie4_cmd_wait() which also instantly returns -EAGAIN: drivers/accel/amdxdna/aie4_ctx.c:aie4_cmd_wait() { ... struct cert_comp *cert_comp = aie4_get_cert_comp(hwctx); long ret; if (!cert_comp) return -EAGAIN; ... } If the disconnect is permanent or recovery fails, will userspace poll forever without observing any delays? > + } > > aie4_put_cert_comp(cert_comp); > -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=13
