Re: [PATCH] s390: vfio-ap: remove unnecessary calls to disable queue interrupts

[Tony Krowiak]

On 8/13/19 7:29 AM, Cornelia Huck wrote:
> On Mon, 12 Aug 2019 16:47:09 -0400
> Tony Krowiak <akrowiak@xxxxxxxxxxxxx> wrote:
>
> > When an AP queue is reset (zeroized), interrupts are disabled. The queue
> > reset function currently tries to disable interrupts unnecessarily. This patch
> > removes the unnecessary calls to disable interrupts after queue reset.
> >
> > Signed-off-by: Tony Krowiak <akrowiak@xxxxxxxxxxxxx>
> > ---
> >   drivers/s390/crypto/vfio_ap_ops.c | 13 +++++++++----
> >   1 file changed, 9 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/s390/crypto/vfio_ap_ops.c b/drivers/s390/crypto/vfio_ap_ops.c
> > index 0604b49a4d32..407c2f0f25f9 100644
> > --- a/drivers/s390/crypto/vfio_ap_ops.c
> > +++ b/drivers/s390/crypto/vfio_ap_ops.c
> > @@ -1114,18 +1114,19 @@ static int vfio_ap_mdev_group_notifier(struct notifier_block *nb,
> >   	return NOTIFY_OK;
> >   }
> >   
> > -static void vfio_ap_irq_disable_apqn(int apqn)
> > +static struct vfio_ap_queue *vfio_ap_find_qdev(int apqn)
> >   {
> >   	struct device *dev;
> > -	struct vfio_ap_queue *q;
> > +	struct vfio_ap_queue *q = NULL;
> >   
> >   	dev = driver_find_device(&matrix_dev->vfio_ap_drv->driver, NULL,
> >   				 &apqn, match_apqn);
> >   	if (dev) {
> >   		q = dev_get_drvdata(dev);
> > -		vfio_ap_irq_disable(q);
> >   		put_device(dev);
> >   	}
> > +
> > +	return q;
> >   }
> >   
> >   int vfio_ap_mdev_reset_queue(unsigned int apid, unsigned int apqi,
> > @@ -1164,6 +1165,7 @@ static int vfio_ap_mdev_reset_queues(struct mdev_device *mdev)
> >   	int rc = 0;
> >   	unsigned long apid, apqi;
> >   	struct ap_matrix_mdev *matrix_mdev = mdev_get_drvdata(mdev);
> > +	struct vfio_ap_queue *q;
> >   
> >   	for_each_set_bit_inv(apid, matrix_mdev->matrix.apm,
> >   			     matrix_mdev->matrix.apm_max + 1) {
> > @@ -1177,7 +1179,10 @@ static int vfio_ap_mdev_reset_queues(struct mdev_device *mdev)
> >   			 */
> >   			if (ret)
> >   				rc = ret;
> > -			vfio_ap_irq_disable_apqn(AP_MKQID(apid, apqi));
>
> Might be useful to stick a comment in this function that resetting the
> queue has also disabled the interrupts, as the architecture
> documentation for that is not publicly available.

Will do.

> > +
> > +			q = vfio_ap_find_qdev(AP_MKQID(apid, apqi));
> > +			if (q)
> > +				vfio_ap_free_aqic_resources(q);
> >   		}
> >   	}
> >   
>
> Trusting your reading of the architecture,
> Reviewed-by: Cornelia Huck <cohuck@xxxxxxxxxx>