vfio/ccw: Flush notoper_work before returning from reset
Subsystems (4)
PATCH 1/1 vfio/ccw: Flush notoper_work before returning from reset
vfio_ccw_dma_unmap() is the DMA invalidation callback, which must unpin the pages covering the range before it returns. It calls vfio_ccw_mdev_reset(), which raises a CLOSE event.
On the normal path fsm_close() calls cp_free() itself, so the pages are unpinned synchronously. But if the subchannel cannot be disabled or quiesced, fsm_close() falls through to a NOT_OPER event, and fsm_notoper() only queues notoper_work, which is where cp_free() then runs. Nothing waits for that work, so the callback returns with the pages still pinned.
Because notoper_work runs asynchronously, vfio_dma_do_unmap() can exhaust its 10 retries before cp_free() completes, hitting BUG_ON(++retries > 10) and taking the host down. A further CLOSE cannot help either, as both CLOSE and OPEN are fsm_nop in the NOT_OPER state.
Flush notoper_work after the CLOSE event, as vfio_ccw_mdev_close_device() already does for the same reason. The flush returns immediately when the work was never queued, so the normal path is unaffected.
---
Found by code review. Cross-compiled for s390 with gcc 14.3.0, W=1 clean, but not tested on s390 hardware and I have no reproducer.
VFIO_DEVICE_RESET also reaches vfio_ccw_mdev_reset() and benefits from this flush, ensuring channel program memory is cleanly released before the reset ioctl returns.
On locking: the flush is not called with io_mutex held. cp_iova_pinned() takes and drops it before vfio_ccw_mdev_reset() runs, vfio_ccw_fsm_event() takes no lock, and the VFIO_DEVICE_RESET path holds nothing. This matches vfio_ccw_mdev_close_device(), which already flushes the same work.
drivers/s390/cio/vfio_ccw_ops.c | 8 ++++++++ 1 file changed, 8 insertions(+)
drivers/s390/cio/vfio_ccw_ops.c +8 -0
@@ -25,6 +25,14 @@ static int vfio_ccw_mdev_reset(struct vfio_ccw_private *private)* and re-opening the mdev, return an error. */ vfio_ccw_fsm_event (private,VFIO_CCW_EVENT_CLOSE ); + +/* +* A failed CLOSE leaves the FSM Not Operational and defers cp_free() +* to notoper_work. Wait for it, so the channel program pages are +* unpinned before this returns. +*/ +flush_work (&private->notoper_work); +vfio_ccw_fsm_event (private,VFIO_CCW_EVENT_OPEN );if (private->state ==VFIO_CCW_STATE_NOT_OPER )return -EINVAL ;
base-commit: f0100363d8c374bd8e9ea7c9ba02744f0b802ca4
Signature · 1 line
2.43.0
[email protected]
AI reviewSashiko has reviewed this patch and found no issues. It looks great!
Quoted 15 lines
vfio_ccw_dma_unmap()is the DMA invalidation callback, which must unpin the pages covering the range before it returns. It callsvfio_ccw_mdev_reset(), which raises a CLOSE event.On the normal path
fsm_close()callscp_free()itself, so the pages are unpinned synchronously. But if the subchannel cannot be disabled or quiesced,fsm_close()falls through to aNOT_OPERevent, andfsm_notoper()only queuesnotoper_work, which is wherecp_free()then runs. Nothing waits for that work, so the callback returns with the pages still pinned.Because
notoper_workruns asynchronously,vfio_dma_do_unmap()can exhaust its 10 retries beforecp_free()completes, hittingBUG_ON(++retries > 10)and taking the host down. A further CLOSE cannot help either, as both CLOSE and OPEN arefsm_nopin theNOT_OPERstate.
Quoted 6 lines
Flush
notoper_workafter the CLOSE event, asvfio_ccw_mdev_close_device()already does for the same reason. The flush returns immediately when the work was never queued, so the normal path is unaffected.Fixes ce4b4657ff18 ("vfio: Replace the DMA unmapping notifier with a callback")Signed-off-by Yuho Choi <[email protected]>
Sashiko has reviewed this patch and found no issues. It looks great!
Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=1