Re: [PATCH v8 17/22] s390: vfio-ap: zeroize the AP queues.

[Tony Krowiak]

On 08/10/2018 07:16 AM, Cornelia Huck wrote:
> On Fri, 10 Aug 2018 12:49:08 +0200
> Pierre Morel <pmorel@xxxxxxxxxxxxx> wrote:
>
> > On 10/08/2018 11:14, Cornelia Huck wrote:
> > > On Wed,  8 Aug 2018 10:44:27 -0400
> > > Tony Krowiak <akrowiak@xxxxxxxxxxxxxxxxxx> wrote:
> > >   
> > > > From: Tony Krowiak <akrowiak@xxxxxxxxxxxxx>
> > > >
> > > > Let's call PAPQ(ZAPQ) to zeroize a queue:
> > > >
> > > > * For each queue configured for a mediated matrix device
> > > >     when it is released.
> > > >
> > > > Zeroizing a queue resets the queue, clears all pending
> > > > messages for the queue entries and disables adapter interruptions
> > > > associated with the queue.
> > > >
> > > > Signed-off-by: Tony Krowiak <akrowiak@xxxxxxxxxxxxx>
> > > > Reviewed-by: Halil Pasic <pasic@xxxxxxxxxxxxx>
> > > > Tested-by: Michael Mueller <mimu@xxxxxxxxxxxxx>
> > > > Tested-by: Farhan Ali <alifm@xxxxxxxxxxxxx>
> > > > Signed-off-by: Christian Borntraeger <borntraeger@xxxxxxxxxx>
> > > > ---
> > > >    drivers/s390/crypto/vfio_ap_ops.c     |   29 ++++++++++++++++++++++++++++-
> > > >    drivers/s390/crypto/vfio_ap_private.h |   25 +++++++++++++++++++++++++
> > > >    2 files changed, 53 insertions(+), 1 deletions(-)
> > > >
> > > > @@ -788,7 +812,10 @@ static void vfio_ap_mdev_release(struct mdev_device *mdev)
> > > >    {
> > > >    	struct ap_matrix_mdev *matrix_mdev = mdev_get_drvdata(mdev);
> > > >    
> > > > -	kvm_arch_crypto_clear_masks(matrix_mdev->kvm);
> > > > +	if (matrix_mdev->kvm)
> > > > +		kvm_arch_crypto_clear_masks(matrix_mdev->kvm);
> > > Confused. Why is the check for matrix_mdev->kvm added here?
> > When using the KVM notifier we can get two notifications:
> > -> KVM is here / is comming
> > -> KVM is not here / disappearing
> >
> > In the first case we initialize matrix_mdev->kvm with a pointer to KVM
> > In the second case we nullify the pointer.
> >
> > During the open of the mediated device, the guest should have been started
> > or we refuse to start.
> >
> > During the close of the mediated device, the guest should be there, but
> > we have no certitude that the guest did not disappear before the VFIO
> > file being closed.
> > Since we do not allow multiple guests using the same mediated device
> > this case should not happen with QEMU. But I am not sure that
> > a rogue user program could not stop KVM before closing the VFIO
> > mediated device.
> I'm not sure why the check is introduced in this patch, though. But
> maybe I just need weekend :)

Good catch, it belongs in patch 15 where the function is introduced.
Is that the only reason for your objection?

> > Maybe Alex can confirm this point, if not we can remove the test.