On Fri, 20 Nov 2020 19:07:38 +0100 Eric Farman <farman@xxxxxxxxxxxxx> wrote: > There is a situation where removing all the paths from a device > connected via mdev and vfio-ccw can cause some difficulty. > Using the "chchp -c 0 xx" command to all paths will cause the > device to be removed from the configuration, and any guest > filesystem that is relying on that device will encounter errors. > Interestingly, the last chchp command will actually fail to > return to a shell prompt, and subsequent commands (another > chchp to bring the paths back online, chzdev, etc.) will also > hang because of the outstanding chchp. > > The last chchp command drives to vfio_ccw_sch_remove() for every > affected mediated device, and ultimately enters an infinite loop > in vfio_del_group_dev(). This loop is broken when the guest goes > away, which in this case doesn't occur until the guest is shutdown. > This drives vfio_ccw_mdev_release() and thus vfio_device_release() > to wake up the vfio_del_group_dev() thread. > > There is also a callback mechanism called "request" to ask a > driver (and perhaps user) to release the device, but this is not > implemented for mdev. So this adds one to that point, and then > wire it to vfio-ccw to pass it along to userspace. This will > gracefully drive the unplug code, and everything behaves nicely. > > Despite the testing that was occurring, this doesn't appear related > to the vfio-ccw channel path handling code. I can reproduce this with > an older kernel/QEMU, which makes sense because the above behavior is > driven from the subchannel event codepaths and not the chpid events. > Because of that, I didn't flag anything with a Fixes tag, since it's > seemingly been this way forever. Both patches look good to me. Which would be the best way to merge this? Via vfio or via vfio-ccw? > > RFC->V2: > - Patch 1 > - Added a message when registering a device without a request callback > - Changed the "if(!callback) return" to "if(callback) do" layout > - Removed "unlikely" from "if(callback)" logic > - Clarified some wording in the device ops struct commentary > - Patch 2 > - Added Conny's r-b > > Eric Farman (2): > vfio-mdev: Wire in a request handler for mdev parent > vfio-ccw: Wire in the request callback > > drivers/s390/cio/vfio_ccw_ops.c | 26 ++++++++++++++++++++++++++ > drivers/s390/cio/vfio_ccw_private.h | 4 ++++ > drivers/vfio/mdev/mdev_core.c | 4 ++++ > drivers/vfio/mdev/vfio_mdev.c | 10 ++++++++++ > include/linux/mdev.h | 4 ++++ > include/uapi/linux/vfio.h | 1 + > 6 files changed, 49 insertions(+) >
