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")
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(-)
--
2.31.1
Realize the VFIO_DEVICE_SET_IRQS ioctl to set an eventfd file descriptor
to be used by the vfio_ap device driver to signal a device request to
userspace.
Signed-off-by: Tony Krowiak <[email protected]>
---
drivers/s390/crypto/vfio_ap_ops.c | 83 +++++++++++++++++++++++++++
drivers/s390/crypto/vfio_ap_private.h | 3 +
2 files changed, 86 insertions(+)
diff --git a/drivers/s390/crypto/vfio_ap_ops.c b/drivers/s390/crypto/vfio_ap_ops.c
index 35cd90eee937..44f159136891 100644
--- a/drivers/s390/crypto/vfio_ap_ops.c
+++ b/drivers/s390/crypto/vfio_ap_ops.c
@@ -716,6 +716,7 @@ static int vfio_ap_mdev_probe(struct mdev_device *mdev)
ret = vfio_register_emulated_iommu_dev(&matrix_mdev->vdev);
if (ret)
goto err_put_vdev;
+ matrix_mdev->req_trigger = NULL;
dev_set_drvdata(&mdev->dev, matrix_mdev);
mutex_lock(&matrix_dev->mdevs_lock);
list_add(&matrix_mdev->node, &matrix_dev->mdev_list);
@@ -1780,6 +1781,85 @@ static ssize_t vfio_ap_get_irq_info(unsigned long arg)
return copy_to_user((void __user *)arg, &info, minsz) ? -EFAULT : 0;
}
+static int vfio_ap_irq_set_init(struct vfio_irq_set *irq_set, unsigned long arg)
+{
+ int ret;
+ size_t data_size;
+ unsigned long minsz;
+
+ minsz = offsetofend(struct vfio_irq_set, count);
+
+ if (copy_from_user(irq_set, (void __user *)arg, minsz))
+ return -EFAULT;
+
+ ret = vfio_set_irqs_validate_and_prepare(irq_set, 1, VFIO_AP_NUM_IRQS,
+ &data_size);
+ if (ret)
+ return ret;
+
+ if (!(irq_set->flags & VFIO_IRQ_SET_ACTION_TRIGGER))
+ return -EINVAL;
+
+ return 0;
+}
+
+static int vfio_ap_set_request_irq(struct ap_matrix_mdev *matrix_mdev,
+ unsigned long arg)
+{
+ s32 fd;
+ void __user *data;
+ unsigned long minsz;
+ struct eventfd_ctx *req_trigger;
+
+ minsz = offsetofend(struct vfio_irq_set, count);
+ data = (void __user *)(arg + minsz);
+
+ if (get_user(fd, (s32 __user *)data))
+ return -EFAULT;
+
+ if (fd == -1) {
+ if (matrix_mdev->req_trigger)
+ eventfd_ctx_put(matrix_mdev->req_trigger);
+ matrix_mdev->req_trigger = NULL;
+ } else if (fd >= 0) {
+ req_trigger = eventfd_ctx_fdget(fd);
+ if (IS_ERR(req_trigger))
+ return PTR_ERR(req_trigger);
+
+ if (matrix_mdev->req_trigger)
+ eventfd_ctx_put(matrix_mdev->req_trigger);
+
+ matrix_mdev->req_trigger = req_trigger;
+ } else {
+ return -EINVAL;
+ }
+
+ return 0;
+}
+
+static int vfio_ap_set_irqs(struct ap_matrix_mdev *matrix_mdev,
+ unsigned long arg)
+{
+ int ret;
+ struct vfio_irq_set irq_set;
+
+ ret = vfio_ap_irq_set_init(&irq_set, arg);
+ if (ret)
+ return ret;
+
+ switch (irq_set.flags & VFIO_IRQ_SET_DATA_TYPE_MASK) {
+ case VFIO_IRQ_SET_DATA_EVENTFD:
+ switch (irq_set.index) {
+ case VFIO_AP_REQ_IRQ_INDEX:
+ return vfio_ap_set_request_irq(matrix_mdev, arg);
+ default:
+ return -EINVAL;
+ }
+ default:
+ return -EINVAL;
+ }
+}
+
static ssize_t vfio_ap_mdev_ioctl(struct vfio_device *vdev,
unsigned int cmd, unsigned long arg)
{
@@ -1798,6 +1878,9 @@ static ssize_t vfio_ap_mdev_ioctl(struct vfio_device *vdev,
case VFIO_DEVICE_GET_IRQ_INFO:
ret = vfio_ap_get_irq_info(arg);
break;
+ case VFIO_DEVICE_SET_IRQS:
+ ret = vfio_ap_set_irqs(matrix_mdev, arg);
+ break;
default:
ret = -EOPNOTSUPP;
break;
diff --git a/drivers/s390/crypto/vfio_ap_private.h b/drivers/s390/crypto/vfio_ap_private.h
index 976a65f32e7d..4642bbdbd1b2 100644
--- a/drivers/s390/crypto/vfio_ap_private.h
+++ b/drivers/s390/crypto/vfio_ap_private.h
@@ -15,6 +15,7 @@
#include <linux/types.h>
#include <linux/mdev.h>
#include <linux/delay.h>
+#include <linux/eventfd.h>
#include <linux/mutex.h>
#include <linux/kvm_host.h>
#include <linux/vfio.h>
@@ -103,6 +104,7 @@ struct ap_queue_table {
* PQAP(AQIC) instruction.
* @mdev: the mediated device
* @qtable: table of queues (struct vfio_ap_queue) assigned to the mdev
+ * @req_trigger eventfd ctx for signaling userspace to return a device
* @apm_add: bitmap of APIDs added to the host's AP configuration
* @aqm_add: bitmap of APQIs added to the host's AP configuration
* @adm_add: bitmap of control domain numbers added to the host's AP
@@ -117,6 +119,7 @@ struct ap_matrix_mdev {
crypto_hook pqap_hook;
struct mdev_device *mdev;
struct ap_queue_table qtable;
+ struct eventfd_ctx *req_trigger;
DECLARE_BITMAP(apm_add, AP_DEVICES);
DECLARE_BITMAP(aqm_add, AP_DOMAINS);
DECLARE_BITMAP(adm_add, AP_DOMAINS);
--
2.31.1
The mdev device is being removed, so pass the request to userspace to
ask for a graceful cleanup. This should free up the thread that
would otherwise loop waiting for the device to be fully released.
Signed-off-by: Tony Krowiak <[email protected]>
---
drivers/s390/crypto/vfio_ap_ops.c | 21 +++++++++++++++++++++
1 file changed, 21 insertions(+)
diff --git a/drivers/s390/crypto/vfio_ap_ops.c b/drivers/s390/crypto/vfio_ap_ops.c
index 44f159136891..a8f58e133e6e 100644
--- a/drivers/s390/crypto/vfio_ap_ops.c
+++ b/drivers/s390/crypto/vfio_ap_ops.c
@@ -1736,6 +1736,26 @@ static void vfio_ap_mdev_close_device(struct vfio_device *vdev)
vfio_ap_mdev_unset_kvm(matrix_mdev);
}
+static void vfio_ap_mdev_request(struct vfio_device *vdev, unsigned int count)
+{
+ struct device *dev = vdev->dev;
+ struct ap_matrix_mdev *matrix_mdev;
+
+ matrix_mdev = container_of(vdev, struct ap_matrix_mdev, vdev);
+
+ if (matrix_mdev->req_trigger) {
+ if (!(count % 10))
+ dev_notice_ratelimited(dev,
+ "Relaying device request to user (#%u)\n",
+ count);
+
+ eventfd_signal(matrix_mdev->req_trigger, 1);
+ } else if (count == 0) {
+ dev_notice(dev,
+ "No device request registered, blocked until released by user\n");
+ }
+}
+
static int vfio_ap_mdev_get_device_info(unsigned long arg)
{
unsigned long minsz;
@@ -1955,6 +1975,7 @@ static const struct vfio_device_ops vfio_ap_matrix_dev_ops = {
.bind_iommufd = vfio_iommufd_emulated_bind,
.unbind_iommufd = vfio_iommufd_emulated_unbind,
.attach_ioas = vfio_iommufd_emulated_attach_ioas,
+ .request = vfio_ap_mdev_request
};
static struct mdev_driver vfio_ap_matrix_driver = {
--
2.31.1
Realize the VFIO_DEVICE_GET_IRQ_INFO ioctl to retrieve the information for
the VFIO device request IRQ.
Signed-off-by: Tony Krowiak <[email protected]>
---
drivers/s390/crypto/vfio_ap_ops.c | 30 +++++++++++++++++++++++++++++-
include/uapi/linux/vfio.h | 9 +++++++++
2 files changed, 38 insertions(+), 1 deletion(-)
diff --git a/drivers/s390/crypto/vfio_ap_ops.c b/drivers/s390/crypto/vfio_ap_ops.c
index cfbcb864ab63..35cd90eee937 100644
--- a/drivers/s390/crypto/vfio_ap_ops.c
+++ b/drivers/s390/crypto/vfio_ap_ops.c
@@ -1750,7 +1750,32 @@ static int vfio_ap_mdev_get_device_info(unsigned long arg)
info.flags = VFIO_DEVICE_FLAGS_AP | VFIO_DEVICE_FLAGS_RESET;
info.num_regions = 0;
- info.num_irqs = 0;
+ info.num_irqs = VFIO_AP_NUM_IRQS;
+
+ return copy_to_user((void __user *)arg, &info, minsz) ? -EFAULT : 0;
+}
+
+static ssize_t vfio_ap_get_irq_info(unsigned long arg)
+{
+ unsigned long minsz;
+ struct vfio_irq_info info;
+
+ minsz = offsetofend(struct vfio_irq_info, count);
+
+ if (copy_from_user(&info, (void __user *)arg, minsz))
+ return -EFAULT;
+
+ if (info.argsz < minsz || info.index >= VFIO_AP_NUM_IRQS)
+ return -EINVAL;
+
+ switch (info.index) {
+ case VFIO_AP_REQ_IRQ_INDEX:
+ info.count = 1;
+ info.flags = VFIO_IRQ_INFO_EVENTFD;
+ break;
+ default:
+ return -EINVAL;
+ }
return copy_to_user((void __user *)arg, &info, minsz) ? -EFAULT : 0;
}
@@ -1770,6 +1795,9 @@ static ssize_t vfio_ap_mdev_ioctl(struct vfio_device *vdev,
case VFIO_DEVICE_RESET:
ret = vfio_ap_mdev_reset_queues(&matrix_mdev->qtable);
break;
+ case VFIO_DEVICE_GET_IRQ_INFO:
+ ret = vfio_ap_get_irq_info(arg);
+ break;
default:
ret = -EOPNOTSUPP;
break;
diff --git a/include/uapi/linux/vfio.h b/include/uapi/linux/vfio.h
index 0552e8dcf0cb..b71276bd7f91 100644
--- a/include/uapi/linux/vfio.h
+++ b/include/uapi/linux/vfio.h
@@ -646,6 +646,15 @@ enum {
VFIO_CCW_NUM_IRQS
};
+/*
+ * The vfio-ap bus driver makes use of the following IRQ index mapping.
+ * Unimplemented IRQ types return a count of zero.
+ */
+enum {
+ VFIO_AP_REQ_IRQ_INDEX,
+ VFIO_AP_NUM_IRQS
+};
+
/**
* VFIO_DEVICE_GET_PCI_HOT_RESET_INFO - _IOWR(VFIO_TYPE, VFIO_BASE + 12,
* struct vfio_pci_hot_reset_info)
--
2.31.1
On 5/31/23 00:35, Tony Krowiak wrote:
> The mdev device is being removed, so pass the request to userspace to
> ask for a graceful cleanup. This should free up the thread that
> would otherwise loop waiting for the device to be fully released.
>
> Signed-off-by: Tony Krowiak <[email protected]>
Reviewed-by: Cédric Le Goater <[email protected]>
Thanks,
C.
> ---
> drivers/s390/crypto/vfio_ap_ops.c | 21 +++++++++++++++++++++
> 1 file changed, 21 insertions(+)
>
> diff --git a/drivers/s390/crypto/vfio_ap_ops.c b/drivers/s390/crypto/vfio_ap_ops.c
> index 44f159136891..a8f58e133e6e 100644
> --- a/drivers/s390/crypto/vfio_ap_ops.c
> +++ b/drivers/s390/crypto/vfio_ap_ops.c
> @@ -1736,6 +1736,26 @@ static void vfio_ap_mdev_close_device(struct vfio_device *vdev)
> vfio_ap_mdev_unset_kvm(matrix_mdev);
> }
>
> +static void vfio_ap_mdev_request(struct vfio_device *vdev, unsigned int count)
> +{
> + struct device *dev = vdev->dev;
> + struct ap_matrix_mdev *matrix_mdev;
> +
> + matrix_mdev = container_of(vdev, struct ap_matrix_mdev, vdev);
> +
> + if (matrix_mdev->req_trigger) {
> + if (!(count % 10))
> + dev_notice_ratelimited(dev,
> + "Relaying device request to user (#%u)\n",
> + count);
> +
> + eventfd_signal(matrix_mdev->req_trigger, 1);
> + } else if (count == 0) {
> + dev_notice(dev,
> + "No device request registered, blocked until released by user\n");
> + }
> +}
> +
> static int vfio_ap_mdev_get_device_info(unsigned long arg)
> {
> unsigned long minsz;
> @@ -1955,6 +1975,7 @@ static const struct vfio_device_ops vfio_ap_matrix_dev_ops = {
> .bind_iommufd = vfio_iommufd_emulated_bind,
> .unbind_iommufd = vfio_iommufd_emulated_unbind,
> .attach_ioas = vfio_iommufd_emulated_attach_ioas,
> + .request = vfio_ap_mdev_request
> };
>
> static struct mdev_driver vfio_ap_matrix_driver = {
On 5/31/23 00:35, Tony Krowiak wrote:
> Realize the VFIO_DEVICE_GET_IRQ_INFO ioctl to retrieve the information for
> the VFIO device request IRQ.
>
> Signed-off-by: Tony Krowiak <[email protected]>
Reviewed-by: Cédric Le Goater <[email protected]>
Thanks,
C.
> ---
> drivers/s390/crypto/vfio_ap_ops.c | 30 +++++++++++++++++++++++++++++-
> include/uapi/linux/vfio.h | 9 +++++++++
> 2 files changed, 38 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/s390/crypto/vfio_ap_ops.c b/drivers/s390/crypto/vfio_ap_ops.c
> index cfbcb864ab63..35cd90eee937 100644
> --- a/drivers/s390/crypto/vfio_ap_ops.c
> +++ b/drivers/s390/crypto/vfio_ap_ops.c
> @@ -1750,7 +1750,32 @@ static int vfio_ap_mdev_get_device_info(unsigned long arg)
>
> info.flags = VFIO_DEVICE_FLAGS_AP | VFIO_DEVICE_FLAGS_RESET;
> info.num_regions = 0;
> - info.num_irqs = 0;
> + info.num_irqs = VFIO_AP_NUM_IRQS;
> +
> + return copy_to_user((void __user *)arg, &info, minsz) ? -EFAULT : 0;
> +}
> +
> +static ssize_t vfio_ap_get_irq_info(unsigned long arg)
> +{
> + unsigned long minsz;
> + struct vfio_irq_info info;
> +
> + minsz = offsetofend(struct vfio_irq_info, count);
> +
> + if (copy_from_user(&info, (void __user *)arg, minsz))
> + return -EFAULT;
> +
> + if (info.argsz < minsz || info.index >= VFIO_AP_NUM_IRQS)
> + return -EINVAL;
> +
> + switch (info.index) {
> + case VFIO_AP_REQ_IRQ_INDEX:
> + info.count = 1;
> + info.flags = VFIO_IRQ_INFO_EVENTFD;
> + break;
> + default:
> + return -EINVAL;
> + }
>
> return copy_to_user((void __user *)arg, &info, minsz) ? -EFAULT : 0;
> }
> @@ -1770,6 +1795,9 @@ static ssize_t vfio_ap_mdev_ioctl(struct vfio_device *vdev,
> case VFIO_DEVICE_RESET:
> ret = vfio_ap_mdev_reset_queues(&matrix_mdev->qtable);
> break;
> + case VFIO_DEVICE_GET_IRQ_INFO:
> + ret = vfio_ap_get_irq_info(arg);
> + break;
> default:
> ret = -EOPNOTSUPP;
> break;
> diff --git a/include/uapi/linux/vfio.h b/include/uapi/linux/vfio.h
> index 0552e8dcf0cb..b71276bd7f91 100644
> --- a/include/uapi/linux/vfio.h
> +++ b/include/uapi/linux/vfio.h
> @@ -646,6 +646,15 @@ enum {
> VFIO_CCW_NUM_IRQS
> };
>
> +/*
> + * The vfio-ap bus driver makes use of the following IRQ index mapping.
> + * Unimplemented IRQ types return a count of zero.
> + */
> +enum {
> + VFIO_AP_REQ_IRQ_INDEX,
> + VFIO_AP_NUM_IRQS
> +};
> +
> /**
> * VFIO_DEVICE_GET_PCI_HOT_RESET_INFO - _IOWR(VFIO_TYPE, VFIO_BASE + 12,
> * struct vfio_pci_hot_reset_info)
On 5/31/23 00:35, Tony Krowiak wrote:
> Realize the VFIO_DEVICE_SET_IRQS ioctl to set an eventfd file descriptor
> to be used by the vfio_ap device driver to signal a device request to
> userspace.
>
> Signed-off-by: Tony Krowiak <[email protected]>
Reviewed-by: Cédric Le Goater <[email protected]>
Thanks,
C.
> ---
> drivers/s390/crypto/vfio_ap_ops.c | 83 +++++++++++++++++++++++++++
> drivers/s390/crypto/vfio_ap_private.h | 3 +
> 2 files changed, 86 insertions(+)
>
> diff --git a/drivers/s390/crypto/vfio_ap_ops.c b/drivers/s390/crypto/vfio_ap_ops.c
> index 35cd90eee937..44f159136891 100644
> --- a/drivers/s390/crypto/vfio_ap_ops.c
> +++ b/drivers/s390/crypto/vfio_ap_ops.c
> @@ -716,6 +716,7 @@ static int vfio_ap_mdev_probe(struct mdev_device *mdev)
> ret = vfio_register_emulated_iommu_dev(&matrix_mdev->vdev);
> if (ret)
> goto err_put_vdev;
> + matrix_mdev->req_trigger = NULL;
> dev_set_drvdata(&mdev->dev, matrix_mdev);
> mutex_lock(&matrix_dev->mdevs_lock);
> list_add(&matrix_mdev->node, &matrix_dev->mdev_list);
> @@ -1780,6 +1781,85 @@ static ssize_t vfio_ap_get_irq_info(unsigned long arg)
> return copy_to_user((void __user *)arg, &info, minsz) ? -EFAULT : 0;
> }
>
> +static int vfio_ap_irq_set_init(struct vfio_irq_set *irq_set, unsigned long arg)
> +{
> + int ret;
> + size_t data_size;
> + unsigned long minsz;
> +
> + minsz = offsetofend(struct vfio_irq_set, count);
> +
> + if (copy_from_user(irq_set, (void __user *)arg, minsz))
> + return -EFAULT;
> +
> + ret = vfio_set_irqs_validate_and_prepare(irq_set, 1, VFIO_AP_NUM_IRQS,
> + &data_size);
> + if (ret)
> + return ret;
> +
> + if (!(irq_set->flags & VFIO_IRQ_SET_ACTION_TRIGGER))
> + return -EINVAL;
> +
> + return 0;
> +}
> +
> +static int vfio_ap_set_request_irq(struct ap_matrix_mdev *matrix_mdev,
> + unsigned long arg)
> +{
> + s32 fd;
> + void __user *data;
> + unsigned long minsz;
> + struct eventfd_ctx *req_trigger;
> +
> + minsz = offsetofend(struct vfio_irq_set, count);
> + data = (void __user *)(arg + minsz);
> +
> + if (get_user(fd, (s32 __user *)data))
> + return -EFAULT;
> +
> + if (fd == -1) {
> + if (matrix_mdev->req_trigger)
> + eventfd_ctx_put(matrix_mdev->req_trigger);
> + matrix_mdev->req_trigger = NULL;
> + } else if (fd >= 0) {
> + req_trigger = eventfd_ctx_fdget(fd);
> + if (IS_ERR(req_trigger))
> + return PTR_ERR(req_trigger);
> +
> + if (matrix_mdev->req_trigger)
> + eventfd_ctx_put(matrix_mdev->req_trigger);
> +
> + matrix_mdev->req_trigger = req_trigger;
> + } else {
> + return -EINVAL;
> + }
> +
> + return 0;
> +}
> +
> +static int vfio_ap_set_irqs(struct ap_matrix_mdev *matrix_mdev,
> + unsigned long arg)
> +{
> + int ret;
> + struct vfio_irq_set irq_set;
> +
> + ret = vfio_ap_irq_set_init(&irq_set, arg);
> + if (ret)
> + return ret;
> +
> + switch (irq_set.flags & VFIO_IRQ_SET_DATA_TYPE_MASK) {
> + case VFIO_IRQ_SET_DATA_EVENTFD:
> + switch (irq_set.index) {
> + case VFIO_AP_REQ_IRQ_INDEX:
> + return vfio_ap_set_request_irq(matrix_mdev, arg);
> + default:
> + return -EINVAL;
> + }
> + default:
> + return -EINVAL;
> + }
> +}
> +
> static ssize_t vfio_ap_mdev_ioctl(struct vfio_device *vdev,
> unsigned int cmd, unsigned long arg)
> {
> @@ -1798,6 +1878,9 @@ static ssize_t vfio_ap_mdev_ioctl(struct vfio_device *vdev,
> case VFIO_DEVICE_GET_IRQ_INFO:
> ret = vfio_ap_get_irq_info(arg);
> break;
> + case VFIO_DEVICE_SET_IRQS:
> + ret = vfio_ap_set_irqs(matrix_mdev, arg);
> + break;
> default:
> ret = -EOPNOTSUPP;
> break;
> diff --git a/drivers/s390/crypto/vfio_ap_private.h b/drivers/s390/crypto/vfio_ap_private.h
> index 976a65f32e7d..4642bbdbd1b2 100644
> --- a/drivers/s390/crypto/vfio_ap_private.h
> +++ b/drivers/s390/crypto/vfio_ap_private.h
> @@ -15,6 +15,7 @@
> #include <linux/types.h>
> #include <linux/mdev.h>
> #include <linux/delay.h>
> +#include <linux/eventfd.h>
> #include <linux/mutex.h>
> #include <linux/kvm_host.h>
> #include <linux/vfio.h>
> @@ -103,6 +104,7 @@ struct ap_queue_table {
> * PQAP(AQIC) instruction.
> * @mdev: the mediated device
> * @qtable: table of queues (struct vfio_ap_queue) assigned to the mdev
> + * @req_trigger eventfd ctx for signaling userspace to return a device
> * @apm_add: bitmap of APIDs added to the host's AP configuration
> * @aqm_add: bitmap of APQIs added to the host's AP configuration
> * @adm_add: bitmap of control domain numbers added to the host's AP
> @@ -117,6 +119,7 @@ struct ap_matrix_mdev {
> crypto_hook pqap_hook;
> struct mdev_device *mdev;
> struct ap_queue_table qtable;
> + struct eventfd_ctx *req_trigger;
> DECLARE_BITMAP(apm_add, AP_DEVICES);
> DECLARE_BITMAP(aqm_add, AP_DOMAINS);
> DECLARE_BITMAP(adm_add, AP_DOMAINS);
On 5/31/23 8:54 AM, Cédric Le Goater wrote:
> Reviewed-by: Cédric Le Goater <[email protected]>
Thank you for the review.
On 5/31/23 15:05, Anthony Krowiak wrote:
>
>
> On 5/31/23 8:54 AM, Cédric Le Goater wrote:
>> Reviewed-by: Cédric Le Goater <[email protected]>
>
> Thank you for the review.
>
I also ran a few tests with the QEMU part on guests with passthrough
crypto devices. This is probably a v6.5 candidate.
C.
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 <[email protected]>
I also did some testing using the companion qemu series at
https://lore.kernel.org/qemu-devel/[email protected]
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 <[email protected]>
Thanks for the review.
>
>
> I also did some testing using the companion qemu series at
> https://lore.kernel.org/qemu-devel/[email protected]
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/[email protected]
>
> Shall I credit you with Tested-by also?
>
Sure.
Thanks,
Matt
On Wed, 31 May 2023 10:51:54 -0400
Matthew Rosato <[email protected]> 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 Tue, 30 May 2023 18:35:36 -0400
Tony Krowiak <[email protected]> wrote:
> Realize the VFIO_DEVICE_GET_IRQ_INFO ioctl to retrieve the information for
> the VFIO device request IRQ.
>
> Signed-off-by: Tony Krowiak <[email protected]>
> ---
> drivers/s390/crypto/vfio_ap_ops.c | 30 +++++++++++++++++++++++++++++-
> include/uapi/linux/vfio.h | 9 +++++++++
> 2 files changed, 38 insertions(+), 1 deletion(-)
Acked-by: Alex Williamson <[email protected]>
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