Re: [PATCH 06/12] vfio/ap_ops: Convert to use vfio_register_group_dev()

[Cornelia Huck <cohuck@redhat.com>]

On Mon, 3 May 2021 16:14:43 -0400
Tony Krowiak <akrowiak@linux.ibm.com> wrote:

> On 4/26/21 1:48 PM, Cornelia Huck wrote:
> > On Fri, 23 Apr 2021 20:03:03 -0300
> > Jason Gunthorpe <jgg@nvidia.com> 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@nvidia.com>
> >> ---
> >>   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.

What happens today if the device in QEMU is removed via device_del?
Does that already clean up things properly?