The mdev remove callback for the vfio_ap device driver bails out with
-EBUSY if the mdev is in use by a KVM guest (i.e., the KVM pointer in the
struct ap_matrix_mdev is not NULL). The intended purpose was
to prevent the mdev from being removed while in use. There are two
problems with this scenario:
1. Returning a non-zero return code from the remove callback does not
prevent the removal of the mdev.
2. The KVM pointer in the struct ap_matrix_mdev will always be NULL because
the remove callback will not get invoked until the mdev fd is closed.
When the mdev fd is closed, the mdev release callback is invoked and
clears the KVM pointer from the struct ap_matrix_mdev.
Let's go ahead and remove the check for KVM in the remove callback and
allow the cleanup of mdev resources to proceed.
Signed-off-by: Tony Krowiak <[email protected]>
---
drivers/s390/crypto/vfio_ap_ops.c | 10 ----------
1 file changed, 10 deletions(-)
diff --git a/drivers/s390/crypto/vfio_ap_ops.c b/drivers/s390/crypto/vfio_ap_ops.c
index b2c7e10dfdcd..122c85c22469 100644
--- a/drivers/s390/crypto/vfio_ap_ops.c
+++ b/drivers/s390/crypto/vfio_ap_ops.c
@@ -366,16 +366,6 @@ static int vfio_ap_mdev_remove(struct mdev_device *mdev)
struct ap_matrix_mdev *matrix_mdev = mdev_get_drvdata(mdev);
mutex_lock(&matrix_dev->lock);
-
- /*
- * If the KVM pointer is in flux or the guest is running, disallow
- * un-assignment of control domain.
- */
- if (matrix_mdev->kvm_busy || matrix_mdev->kvm) {
- mutex_unlock(&matrix_dev->lock);
- return -EBUSY;
- }
-
vfio_ap_mdev_reset_queues(mdev);
list_del(&matrix_mdev->node);
kfree(matrix_mdev);
--
2.30.2
On Wed, Jun 09, 2021 at 06:46:32PM -0400, Tony Krowiak wrote:
> The mdev remove callback for the vfio_ap device driver bails out with
> -EBUSY if the mdev is in use by a KVM guest (i.e., the KVM pointer in the
> struct ap_matrix_mdev is not NULL). The intended purpose was
> to prevent the mdev from being removed while in use. There are two
> problems with this scenario:
>
> 1. Returning a non-zero return code from the remove callback does not
> prevent the removal of the mdev.
>
> 2. The KVM pointer in the struct ap_matrix_mdev will always be NULL because
> the remove callback will not get invoked until the mdev fd is closed.
> When the mdev fd is closed, the mdev release callback is invoked and
> clears the KVM pointer from the struct ap_matrix_mdev.
>
> Let's go ahead and remove the check for KVM in the remove callback and
> allow the cleanup of mdev resources to proceed.
>
> Signed-off-by: Tony Krowiak <[email protected]>
> ---
> drivers/s390/crypto/vfio_ap_ops.c | 10 ----------
> 1 file changed, 10 deletions(-)
Reviewed-by: Jason Gunthorpe <[email protected]>
Jason
On 6/11/21 12:48 PM, Jason Gunthorpe wrote:
> Reviewed-by: Jason Gunthorpe<[email protected]>
Thanks for the review.
On 11.06.21 18:48, Jason Gunthorpe wrote:
> On Wed, Jun 09, 2021 at 06:46:32PM -0400, Tony Krowiak wrote:
>> The mdev remove callback for the vfio_ap device driver bails out with
>> -EBUSY if the mdev is in use by a KVM guest (i.e., the KVM pointer in the
>> struct ap_matrix_mdev is not NULL). The intended purpose was
>> to prevent the mdev from being removed while in use. There are two
>> problems with this scenario:
>>
>> 1. Returning a non-zero return code from the remove callback does not
>> prevent the removal of the mdev.
>>
>> 2. The KVM pointer in the struct ap_matrix_mdev will always be NULL because
>> the remove callback will not get invoked until the mdev fd is closed.
>> When the mdev fd is closed, the mdev release callback is invoked and
>> clears the KVM pointer from the struct ap_matrix_mdev.
>>
>> Let's go ahead and remove the check for KVM in the remove callback and
>> allow the cleanup of mdev resources to proceed.
>>
>> Signed-off-by: Tony Krowiak <[email protected]>
>> ---
>> drivers/s390/crypto/vfio_ap_ops.c | 10 ----------
>> 1 file changed, 10 deletions(-)
>
> Reviewed-by: Jason Gunthorpe <[email protected]>
Jason, I guess you want this patch still in 5.13, the other 2 can be 5.14?