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

Cornelia Huck
2019-08-13T11:30:14Z

On Mon, 12 Aug 2019 16:47:09 -0400
Tony Krowiak <akrowiak@linux.ibm.com> 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@linux.ibm.com>
> ---
> 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.

> +
> + 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@redhat.com>