Message ID | 20230530223538.279198-1-akrowiak@linux.ibm.com (mailing list archive) |
---|---|
Headers | show |
Series | s390/vfio-ap: fix hang when mdev attached to guest is removed | expand |
On 5/30/23 6:35 PM, Tony Krowiak wrote: > When a user attempts to remove a vfio-ap mediated device attached to a > guest, the operation hangs until the mdev's fd is closed by the guest > (i.e., the hostdev is detached or the guest is shut down). This patch > series provides kernel-side code that allows userspace to set up a > communication channel that will allow the vfio_ap device driver to notify > userspace when a request to release the mdev is received, so that userspace > can close the mdev fd and avoid the hang. The patch series provides the > following: > > 1. Introduces code to handle the VFIO_DEVICE_GET_IRQ_INFO and > VFIO_DEVICE_SET_IRQS ioctl calls to set the eventfd_ctx for signaling a > device request to userspace. > > 2. Wires up the VFIO bus driver callback to request a release of the mdev. > When invoked, the vfio_ap device driver will use the eventfd_ctx set up > in #1 to signal a request to userspace to release the mdev. > > > Note: > ---- > If a user subsequently attempts to restart the guest or re-attach the mdev, > the operation will fail with a message indicating the domain is already > active. This is a libvirt problem resolved with the following commit: > > commit ebd004a03dbd ("security: do not remember/recall labels for VFIO > MDEVs") For the series: Reviewed-by: Matthew Rosato <mjrosato@linux.ibm.com> I also did some testing using the companion qemu series at https://lore.kernel.org/qemu-devel/20230530225544.280031-1-akrowiak@linux.ibm.com Before kernel+qemu changes: 1. mdevctl start -u <uuid>, where <uuid> references a vfio-ap mdev 2. start a qemu guest with <uuid> attached 3. mdvectl stop -u <uuid> 4. -mdevctl will now hang indefinitely; the mdev remains in-use by the guest- Note: detaching the device or powering off the guest will allow the mdevctl command to complete. After kernel+qemu changes: 1. mdevctl start -u <uuid>, where <uuid> references a vfio-ap mdev 2. start a qemu guest with <uuid> attached 3. mdvectl stop -u <uuid> 4. -device is detached from the guest and stopped- 5. Using a libvirt that includes ebd004a03dbd I also verified that the mdev can be started again and re-attached to the running guest without error. > > Tony Krowiak (3): > vfio: ap: realize the VFIO_DEVICE_GET_IRQ_INFO ioctl > vfio: ap: realize the VFIO_DEVICE_SET_IRQS ioctl > s390/vfio-ap: Wire in the vfio_device_ops request callback > > drivers/s390/crypto/vfio_ap_ops.c | 134 +++++++++++++++++++++++++- > drivers/s390/crypto/vfio_ap_private.h | 3 + > include/uapi/linux/vfio.h | 9 ++ > 3 files changed, 145 insertions(+), 1 deletion(-) >
On 5/30/23 6:35 PM, Tony Krowiak wrote: > When a user attempts to remove a vfio-ap mediated device attached to a > guest, the operation hangs until the mdev's fd is closed by the guest > (i.e., the hostdev is detached or the guest is shut down). This patch > series provides kernel-side code that allows userspace to set up a > communication channel that will allow the vfio_ap device driver to notify > userspace when a request to release the mdev is received, so that userspace > can close the mdev fd and avoid the hang. The patch series provides the > following: > > 1. Introduces code to handle the VFIO_DEVICE_GET_IRQ_INFO and > VFIO_DEVICE_SET_IRQS ioctl calls to set the eventfd_ctx for signaling a > device request to userspace. > > 2. Wires up the VFIO bus driver callback to request a release of the mdev. > When invoked, the vfio_ap device driver will use the eventfd_ctx set up > in #1 to signal a request to userspace to release the mdev. > As to how this series eventually reaches master... It touches both s390 and vfio. @Alex/@s390 maintainers -- I suggest it go through s390 given the diffstat, it's almost completely in s390 drivers code. However there is a uapi hit to vfio.h (in patch 1) that should get at least an ACK from Alex beforehand.
On 5/31/23 10:48 AM, Matthew Rosato wrote: > On 5/30/23 6:35 PM, Tony Krowiak wrote: >> When a user attempts to remove a vfio-ap mediated device attached to a >> guest, the operation hangs until the mdev's fd is closed by the guest >> (i.e., the hostdev is detached or the guest is shut down). This patch >> series provides kernel-side code that allows userspace to set up a >> communication channel that will allow the vfio_ap device driver to notify >> userspace when a request to release the mdev is received, so that userspace >> can close the mdev fd and avoid the hang. The patch series provides the >> following: >> >> 1. Introduces code to handle the VFIO_DEVICE_GET_IRQ_INFO and >> VFIO_DEVICE_SET_IRQS ioctl calls to set the eventfd_ctx for signaling a >> device request to userspace. >> >> 2. Wires up the VFIO bus driver callback to request a release of the mdev. >> When invoked, the vfio_ap device driver will use the eventfd_ctx set up >> in #1 to signal a request to userspace to release the mdev. >> >> >> Note: >> ---- >> If a user subsequently attempts to restart the guest or re-attach the mdev, >> the operation will fail with a message indicating the domain is already >> active. This is a libvirt problem resolved with the following commit: >> >> commit ebd004a03dbd ("security: do not remember/recall labels for VFIO >> MDEVs") > > For the series: > > Reviewed-by: Matthew Rosato <mjrosato@linux.ibm.com> Thanks for the review. > > > I also did some testing using the companion qemu series at > https://lore.kernel.org/qemu-devel/20230530225544.280031-1-akrowiak@linux.ibm.com Shall I credit you with Tested-by also? > > Before kernel+qemu changes: > 1. mdevctl start -u <uuid>, where <uuid> references a vfio-ap mdev > 2. start a qemu guest with <uuid> attached > 3. mdvectl stop -u <uuid> > 4. -mdevctl will now hang indefinitely; the mdev remains in-use by the guest- > Note: detaching the device or powering off the guest will allow the mdevctl command to complete. > > After kernel+qemu changes: > 1. mdevctl start -u <uuid>, where <uuid> references a vfio-ap mdev > 2. start a qemu guest with <uuid> attached > 3. mdvectl stop -u <uuid> > 4. -device is detached from the guest and stopped- > 5. Using a libvirt that includes ebd004a03dbd I also verified that the mdev can be started again and re-attached to the running guest without error. > > >> >> Tony Krowiak (3): >> vfio: ap: realize the VFIO_DEVICE_GET_IRQ_INFO ioctl >> vfio: ap: realize the VFIO_DEVICE_SET_IRQS ioctl >> s390/vfio-ap: Wire in the vfio_device_ops request callback >> >> drivers/s390/crypto/vfio_ap_ops.c | 134 +++++++++++++++++++++++++- >> drivers/s390/crypto/vfio_ap_private.h | 3 + >> include/uapi/linux/vfio.h | 9 ++ >> 3 files changed, 145 insertions(+), 1 deletion(-) >> >
On 6/1/23 8:15 AM, Anthony Krowiak wrote: > > > On 5/31/23 10:48 AM, Matthew Rosato wrote: >> I also did some testing using the companion qemu series at >> https://lore.kernel.org/qemu-devel/20230530225544.280031-1-akrowiak@linux.ibm.com > > Shall I credit you with Tested-by also? > Sure. Thanks, Matt
On Wed, 31 May 2023 10:51:54 -0400 Matthew Rosato <mjrosato@linux.ibm.com> wrote: > On 5/30/23 6:35 PM, Tony Krowiak wrote: > > When a user attempts to remove a vfio-ap mediated device attached to a > > guest, the operation hangs until the mdev's fd is closed by the guest > > (i.e., the hostdev is detached or the guest is shut down). This patch > > series provides kernel-side code that allows userspace to set up a > > communication channel that will allow the vfio_ap device driver to notify > > userspace when a request to release the mdev is received, so that userspace > > can close the mdev fd and avoid the hang. The patch series provides the > > following: > > > > 1. Introduces code to handle the VFIO_DEVICE_GET_IRQ_INFO and > > VFIO_DEVICE_SET_IRQS ioctl calls to set the eventfd_ctx for signaling a > > device request to userspace. > > > > 2. Wires up the VFIO bus driver callback to request a release of the mdev. > > When invoked, the vfio_ap device driver will use the eventfd_ctx set up > > in #1 to signal a request to userspace to release the mdev. > > > > As to how this series eventually reaches master... It touches both s390 and vfio. > > @Alex/@s390 maintainers -- I suggest it go through s390 given the > diffstat, it's almost completely in s390 drivers code. However there > is a uapi hit to vfio.h (in patch 1) that should get at least an ACK > from Alex beforehand. Ack'd, I'll expect this to go through the s390 tree. Thanks, Alex
On Thu, Jun 01, 2023 at 02:47:22PM -0600, Alex Williamson wrote: ... > > As to how this series eventually reaches master... It touches both s390 and vfio. > > > > @Alex/@s390 maintainers -- I suggest it go through s390 given the > > diffstat, it's almost completely in s390 drivers code. However there > > is a uapi hit to vfio.h (in patch 1) that should get at least an ACK > > from Alex beforehand. > > Ack'd, I'll expect this to go through the s390 tree. Thanks, Applied, thanks! > Alex