[PATCH v2] vfio/ccw: Flush notoper_work before returning from reset

[PATCH v2] vfio/ccw: Flush notoper_work before returning from reset

[Yuho Choi]

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.  Since commit
0c11f61a876e ("s390/vfio_ccw: Move cp cleanup out of not operational"),
fsm_notoper() only queues notoper_work, which is where cp_free() then
runs.  That commit flushes the work in vfio_ccw_mdev_close_device() and
vfio_ccw_mdev_release_dev(), but nothing waits for it on the reset
path, 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.  The flush returns immediately when the work was never
queued, so the normal path is unaffected.

Fixes: 0c11f61a876e ("s390/vfio_ccw: Move cp cleanup out of not operational")
Signed-off-by: Yuho Choi <oss.patchbox@gmail.com>
---
v2:
- Point the Fixes tag at 0c11f61a876e, which moved cp_free() into
  notoper_work. v1 named ce4b4657ff18 by mistake.
- Mention that commit in the changelog.
- Rebased on v7.4-rc1.

v1: https://lore.kernel.org/r/20260922164928.477669-1-oss.patchbox@gmail.com/

Found by code review. Cross-compiled for s390 with gcc 14.2.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(+)

diff --git a/drivers/s390/cio/vfio_ccw_ops.c b/drivers/s390/cio/vfio_ccw_ops.c
index 5ce91285c7d5..8e0a2edbd34a 100644
--- a/drivers/s390/cio/vfio_ccw_ops.c
+++ b/drivers/s390/cio/vfio_ccw_ops.c
@@ -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: 6628e7d4c6b25c8d18f1345e3a20d21829aae7e4
-- 
2.43.0

Re: [PATCH v2] vfio/ccw: Flush notoper_work before returning from reset

[Christian Borntraeger]

Am 06.11.26 um 20:10 schrieb Yuho Choi:
> 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.  Since commit
> 0c11f61a876e ("s390/vfio_ccw: Move cp cleanup out of not operational"),
> fsm_notoper() only queues notoper_work, which is where cp_free() then
> runs.  That commit flushes the work in vfio_ccw_mdev_close_device() and
> vfio_ccw_mdev_release_dev(), but nothing waits for it on the reset
> path, 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.

Does the the iommufd backend have the same problem?
iopt_unmap_iova_range() keeps calling the unmap callback and gives up with
WARN_ON(tries > 100) and -EDEADLOCK. So that path is also fixed, I guess.

[..]

> Fixes: 0c11f61a876e ("s390/vfio_ccw: Move cp cleanup out of not operational")
cc stable ?

> Signed-off-by: Yuho Choi <oss.patchbox@gmail.com>


Eric, Matt,

can you have a look?

[..]

Re: [PATCH v2] vfio/ccw: Flush notoper_work before returning from reset

[Jason Gunthorpe]

On Mon, Nov 09, 2026 at 01:38:44PM +0100, Christian Borntraeger wrote:
> Am 06.11.26 um 20:10 schrieb Yuho Choi:
> > 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.  Since commit
> > 0c11f61a876e ("s390/vfio_ccw: Move cp cleanup out of not operational"),
> > fsm_notoper() only queues notoper_work, which is where cp_free() then
> > runs.  That commit flushes the work in vfio_ccw_mdev_close_device() and
> > vfio_ccw_mdev_release_dev(), but nothing waits for it on the reset
> > path, 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.
> 
> Does the the iommufd backend have the same problem?
> iopt_unmap_iova_range() keeps calling the unmap callback and gives up with
> WARN_ON(tries > 100) and -EDEADLOCK. So that path is also fixed, I
> guess.

Drivers are not allowed to refuse the shootdown, it is broken if
someone has done this.

Jason