On 5/4/21 11:30 AM, Cornelia Huck wrote:
On Mon, 3 May 2021 16:14:43 -0400
Tony Krowiak <akrowiak@xxxxxxxxxxxxx> wrote:
On 4/26/21 1:48 PM, Cornelia Huck wrote:
On Fri, 23 Apr 2021 20:03:03 -0300
Jason Gunthorpe <jgg@xxxxxxxxxx> wrote:
This is straightforward conversion, the ap_matrix_mdev is actually serving
as the vfio_device and we can replace all the mdev_get_drvdata()'s with a
simple container_of().
Signed-off-by: Jason Gunthorpe <jgg@xxxxxxxxxx>
---
drivers/s390/crypto/vfio_ap_ops.c | 137 ++++++++++++++++----------
drivers/s390/crypto/vfio_ap_private.h | 2 +
2 files changed, 89 insertions(+), 50 deletions(-)
(...)
-static int vfio_ap_mdev_remove(struct mdev_device *mdev)
+static void vfio_ap_mdev_remove(struct mdev_device *mdev)
{
- struct ap_matrix_mdev *matrix_mdev = mdev_get_drvdata(mdev);
+ struct ap_matrix_mdev *matrix_mdev = dev_get_drvdata(&mdev->dev);
- if (matrix_mdev->kvm)
- return -EBUSY;
+ /* FIXME: Remove isn't allowed to fail */
+ if (WARN_ON(matrix_mdev->kvm))
+ return;
This is a pre-existing problem, but the rework now makes it more
obvious.
I agree, I was not aware that returning a non-zero return code
from this callback did not return the -EBUSY to userspace
when the mdev is removed.
Previously, the mdev code would only print a warning and then continue
with device removal, even if a ->remove() callback returned an error.
Now, it's quite clear that we'll end up in a weird half-dead state.
With the latest kernel from our tree, the remove hangs until the
guest is shutdown and the mdev fd is closed. During the hang, the
dmesg log has the following:
"No mdev vendor driver request callback support, blocked until released
by user"
So, it looks like nothing is done with the mdev until the fd for the
mdev is closed when the guest is shut down, at which time the
mdev is removed.
You probably want to wire up the request callback and notify userspace.
Not sure what you mean by this, but I also don't think it matters.
After coding up the fix for this and testing it, I've learned that
if a user attempts to remove an mdev via the sysfs 'remove'
attribute while the mdev fd is still open (i.e., in use by the guest),
the mdev remove callback is not invoked until the fd is closed
(i.e., the guest is shut down). During that time, the mdev is
physically removed from sysfs so no further actions can be taken
on it; but, since the remove callback has yet to be called, the
guest will have access to the AP resources provided by the
mdev during that time. I also tested detaching the mdev device from the
guest
(i.e., virsh detach-device) while the mdev was in the process of
being removed and this resulted in allowing the remove to
progress due to the mdev fd getting closed when it is detached.
What happens today if the device in QEMU is removed via device_del?
Does that already clean up things properly?
Hot plug/unplug is already supported, so yes, things get cleaned up
properly when the mdev fd is closed. This is handled by the mdev
release callback.
