[PATCH 1/1] s390/vfio-ap: No need to disable IRQ after queue reset

[PATCH 1/1] s390/vfio-ap: No need to disable IRQ after queue reset

[Halil Pasic]

From: Tony Krowiak <akrowiak@linux.ibm.com>

The queues assigned to a matrix mediated device are currently reset when:

* The VFIO_DEVICE_RESET ioctl is invoked
* The mdev fd is closed by userspace (QEMU)
* The mdev is removed from sysfs.

Immediately after the reset of a queue, a call is made to disable
interrupts for the queue. This is entirely unnecessary because the reset of
a queue disables interrupts, so this will be removed.

Furthermore, vfio_ap_irq_disable() does an unconditional PQAP/AQIC which
can result in a specification exception (when the corresponding facility
is not available), so this is actually a bugfix.

Signed-off-by: Tony Krowiak <akrowiak@linux.ibm.com>
[pasic@linux.ibm.com: minor rework before merging]
Reviewed-by: Halil Pasic <pasic@linux.ibm.com>
Signed-off-by: Halil Pasic <pasic@linux.ibm.com>
Fixes: ec89b55e3bce ("s390: ap: implement PAPQ AQIC interception in kernel")
Cc: <stable@vger.kernel.org>

---

Since it turned out disabling the interrupts via PQAP/AQIC is not only
unnecesary but also buggy, we decided to put this patch, which
used to be apart of the series https://lkml.org/lkml/2020/12/22/757 on the fast
lane.

If the backports turn out to be a bother, which I hope won't be the case
not, I am happy to help with those.

---
 drivers/s390/crypto/vfio_ap_drv.c     |   6 +-
 drivers/s390/crypto/vfio_ap_ops.c     | 100 ++++++++++++++++----------
 drivers/s390/crypto/vfio_ap_private.h |  12 ++--
 3 files changed, 69 insertions(+), 49 deletions(-)

diff --git a/drivers/s390/crypto/vfio_ap_drv.c b/drivers/s390/crypto/vfio_ap_drv.c
index be2520cc010b..7dc72cb718b0 100644
--- a/drivers/s390/crypto/vfio_ap_drv.c
+++ b/drivers/s390/crypto/vfio_ap_drv.c
@@ -71,15 +71,11 @@ static int vfio_ap_queue_dev_probe(struct ap_device *apdev)
 static void vfio_ap_queue_dev_remove(struct ap_device *apdev)
 {
 	struct vfio_ap_queue *q;
-	int apid, apqi;
 
 	mutex_lock(&matrix_dev->lock);
 	q = dev_get_drvdata(&apdev->device);
+	vfio_ap_mdev_reset_queue(q, 1);
 	dev_set_drvdata(&apdev->device, NULL);
-	apid = AP_QID_CARD(q->apqn);
-	apqi = AP_QID_QUEUE(q->apqn);
-	vfio_ap_mdev_reset_queue(apid, apqi, 1);
-	vfio_ap_irq_disable(q);
 	kfree(q);
 	mutex_unlock(&matrix_dev->lock);
 }
diff --git a/drivers/s390/crypto/vfio_ap_ops.c b/drivers/s390/crypto/vfio_ap_ops.c
index e0bde8518745..7ceb6c433b3b 100644
--- a/drivers/s390/crypto/vfio_ap_ops.c
+++ b/drivers/s390/crypto/vfio_ap_ops.c
@@ -25,6 +25,7 @@
 #define VFIO_AP_MDEV_NAME_HWVIRT "VFIO AP Passthrough Device"
 
 static int vfio_ap_mdev_reset_queues(struct mdev_device *mdev);
+static struct vfio_ap_queue *vfio_ap_find_queue(int apqn);
 
 static int match_apqn(struct device *dev, const void *data)
 {
@@ -49,20 +50,15 @@ static struct vfio_ap_queue *vfio_ap_get_queue(
 					int apqn)
 {
 	struct vfio_ap_queue *q;
-	struct device *dev;
 
 	if (!test_bit_inv(AP_QID_CARD(apqn), matrix_mdev->matrix.apm))
 		return NULL;
 	if (!test_bit_inv(AP_QID_QUEUE(apqn), matrix_mdev->matrix.aqm))
 		return NULL;
 
-	dev = driver_find_device(&matrix_dev->vfio_ap_drv->driver, NULL,
-				 &apqn, match_apqn);
-	if (!dev)
-		return NULL;
-	q = dev_get_drvdata(dev);
-	q->matrix_mdev = matrix_mdev;
-	put_device(dev);
+	q = vfio_ap_find_queue(apqn);
+	if (q)
+		q->matrix_mdev = matrix_mdev;
 
 	return q;
 }
@@ -119,13 +115,18 @@ static void vfio_ap_wait_for_irqclear(int apqn)
  */
 static void vfio_ap_free_aqic_resources(struct vfio_ap_queue *q)
 {
-	if (q->saved_isc != VFIO_AP_ISC_INVALID && q->matrix_mdev)
+	if (!q)
+		return;
+	if (q->saved_isc != VFIO_AP_ISC_INVALID &&
+	    !WARN_ON(!(q->matrix_mdev && q->matrix_mdev->kvm))) {
 		kvm_s390_gisc_unregister(q->matrix_mdev->kvm, q->saved_isc);
-	if (q->saved_pfn && q->matrix_mdev)
+		q->saved_isc = VFIO_AP_ISC_INVALID;
+	}
+	if (q->saved_pfn && !WARN_ON(!q->matrix_mdev)) {
 		vfio_unpin_pages(mdev_dev(q->matrix_mdev->mdev),
 				 &q->saved_pfn, 1);
-	q->saved_pfn = 0;
-	q->saved_isc = VFIO_AP_ISC_INVALID;
+		q->saved_pfn = 0;
+	}
 }
 
 /**
@@ -144,7 +145,7 @@ static void vfio_ap_free_aqic_resources(struct vfio_ap_queue *q)
  * Returns if ap_aqic function failed with invalid, deconfigured or
  * checkstopped AP.
  */
-struct ap_queue_status vfio_ap_irq_disable(struct vfio_ap_queue *q)
+static struct ap_queue_status vfio_ap_irq_disable(struct vfio_ap_queue *q)
 {
 	struct ap_qirq_ctrl aqic_gisa = {};
 	struct ap_queue_status status;
@@ -1114,48 +1115,70 @@ 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_queue(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,
+int vfio_ap_mdev_reset_queue(struct vfio_ap_queue *q,
 			     unsigned int retry)
 {
 	struct ap_queue_status status;
+	int ret;
 	int retry2 = 2;
-	int apqn = AP_MKQID(apid, apqi);
 
-	do {
-		status = ap_zapq(apqn);
-		switch (status.response_code) {
-		case AP_RESPONSE_NORMAL:
-			while (!status.queue_empty && retry2--) {
-				msleep(20);
-				status = ap_tapq(apqn, NULL);
-			}
-			WARN_ON_ONCE(retry2 <= 0);
-			return 0;
-		case AP_RESPONSE_RESET_IN_PROGRESS:
-		case AP_RESPONSE_BUSY:
+	if (!q)
+		return 0;
+
+retry_zapq:
+	status = ap_zapq(q->apqn);
+	switch (status.response_code) {
+	case AP_RESPONSE_NORMAL:
+		ret = 0;
+		break;
+	case AP_RESPONSE_RESET_IN_PROGRESS:
+		if (retry--) {
 			msleep(20);
-			break;
-		default:
-			/* things are really broken, give up */
-			return -EIO;
+			goto retry_zapq;
 		}
-	} while (retry--);
+		ret = -EBUSY;
+		break;
+	case AP_RESPONSE_Q_NOT_AVAIL:
+	case AP_RESPONSE_DECONFIGURED:
+	case AP_RESPONSE_CHECKSTOPPED:
+		WARN_ON_ONCE(status.irq_enabled);
+		ret = -EBUSY;
+		goto free_resources;
+	default:
+		/* things are really broken, give up */
+		WARN(true, "PQAP/ZAPQ completed with invalid rc (%x)\n",
+		     status.response_code);
+		return -EIO;
+	}
+
+	/* wait for the reset to take effect */
+	while (retry2--) {
+		if (status.queue_empty && !status.irq_enabled)
+			break;
+		msleep(20);
+		status = ap_tapq(q->apqn, NULL);
+	}
+	WARN_ON_ONCE(retry2 <= 0);
 
-	return -EBUSY;
+free_resources:
+	vfio_ap_free_aqic_resources(q);
+
+	return ret;
 }
 
 static int vfio_ap_mdev_reset_queues(struct mdev_device *mdev)
@@ -1163,13 +1186,15 @@ static int vfio_ap_mdev_reset_queues(struct mdev_device *mdev)
 	int ret;
 	int rc = 0;
 	unsigned long apid, apqi;
+	struct vfio_ap_queue *q;
 	struct ap_matrix_mdev *matrix_mdev = mdev_get_drvdata(mdev);
 
 	for_each_set_bit_inv(apid, matrix_mdev->matrix.apm,
 			     matrix_mdev->matrix.apm_max + 1) {
 		for_each_set_bit_inv(apqi, matrix_mdev->matrix.aqm,
 				     matrix_mdev->matrix.aqm_max + 1) {
-			ret = vfio_ap_mdev_reset_queue(apid, apqi, 1);
+			q = vfio_ap_find_queue(AP_MKQID(apid, apqi));
+			ret = vfio_ap_mdev_reset_queue(q, 1);
 			/*
 			 * Regardless whether a queue turns out to be busy, or
 			 * is not operational, we need to continue resetting
@@ -1177,7 +1202,6 @@ static int vfio_ap_mdev_reset_queues(struct mdev_device *mdev)
 			 */
 			if (ret)
 				rc = ret;
-			vfio_ap_irq_disable_apqn(AP_MKQID(apid, apqi));
 		}
 	}
 
diff --git a/drivers/s390/crypto/vfio_ap_private.h b/drivers/s390/crypto/vfio_ap_private.h
index f46dde56b464..28e9d9989768 100644
--- a/drivers/s390/crypto/vfio_ap_private.h
+++ b/drivers/s390/crypto/vfio_ap_private.h
@@ -88,11 +88,6 @@ struct ap_matrix_mdev {
 	struct mdev_device *mdev;
 };
 
-extern int vfio_ap_mdev_register(void);
-extern void vfio_ap_mdev_unregister(void);
-int vfio_ap_mdev_reset_queue(unsigned int apid, unsigned int apqi,
-			     unsigned int retry);
-
 struct vfio_ap_queue {
 	struct ap_matrix_mdev *matrix_mdev;
 	unsigned long saved_pfn;
@@ -100,5 +95,10 @@ struct vfio_ap_queue {
 #define VFIO_AP_ISC_INVALID 0xff
 	unsigned char saved_isc;
 };
-struct ap_queue_status vfio_ap_irq_disable(struct vfio_ap_queue *q);
+
+int vfio_ap_mdev_register(void);
+void vfio_ap_mdev_unregister(void);
+int vfio_ap_mdev_reset_queue(struct vfio_ap_queue *q,
+			     unsigned int retry);
+
 #endif /* _VFIO_AP_PRIVATE_H_ */

base-commit: 9791581c049c10929e97098374dd1716a81fefcc
-- 
2.17.1

Re: [PATCH 1/1] s390/vfio-ap: No need to disable IRQ after queue reset

[Cornelia Huck]

On Thu, 21 Jan 2021 08:20:08 +0100
Halil Pasic <pasic@linux.ibm.com> wrote:

> From: Tony Krowiak <akrowiak@linux.ibm.com>
> 
> The queues assigned to a matrix mediated device are currently reset when:
> 
> * The VFIO_DEVICE_RESET ioctl is invoked
> * The mdev fd is closed by userspace (QEMU)
> * The mdev is removed from sysfs.
> 
> Immediately after the reset of a queue, a call is made to disable
> interrupts for the queue. This is entirely unnecessary because the reset of
> a queue disables interrupts, so this will be removed.
> 
> Furthermore, vfio_ap_irq_disable() does an unconditional PQAP/AQIC which
> can result in a specification exception (when the corresponding facility
> is not available), so this is actually a bugfix.
> 
> Signed-off-by: Tony Krowiak <akrowiak@linux.ibm.com>
> [pasic@linux.ibm.com: minor rework before merging]
> Reviewed-by: Halil Pasic <pasic@linux.ibm.com>
> Signed-off-by: Halil Pasic <pasic@linux.ibm.com>
> Fixes: ec89b55e3bce ("s390: ap: implement PAPQ AQIC interception in kernel")
> Cc: <stable@vger.kernel.org>
> 
> ---
> 
> Since it turned out disabling the interrupts via PQAP/AQIC is not only
> unnecesary but also buggy, we decided to put this patch, which
> used to be apart of the series https://lkml.org/lkml/2020/12/22/757 on the fast
> lane.
> 
> If the backports turn out to be a bother, which I hope won't be the case
> not, I am happy to help with those.
> 
> ---
>  drivers/s390/crypto/vfio_ap_drv.c     |   6 +-
>  drivers/s390/crypto/vfio_ap_ops.c     | 100 ++++++++++++++++----------
>  drivers/s390/crypto/vfio_ap_private.h |  12 ++--
>  3 files changed, 69 insertions(+), 49 deletions(-)
> 

(...)

> diff --git a/drivers/s390/crypto/vfio_ap_private.h b/drivers/s390/crypto/vfio_ap_private.h
> index f46dde56b464..28e9d9989768 100644
> --- a/drivers/s390/crypto/vfio_ap_private.h
> +++ b/drivers/s390/crypto/vfio_ap_private.h
> @@ -88,11 +88,6 @@ struct ap_matrix_mdev {
>  	struct mdev_device *mdev;
>  };
>  
> -extern int vfio_ap_mdev_register(void);
> -extern void vfio_ap_mdev_unregister(void);
> -int vfio_ap_mdev_reset_queue(unsigned int apid, unsigned int apqi,
> -			     unsigned int retry);
> -
>  struct vfio_ap_queue {
>  	struct ap_matrix_mdev *matrix_mdev;
>  	unsigned long saved_pfn;
> @@ -100,5 +95,10 @@ struct vfio_ap_queue {
>  #define VFIO_AP_ISC_INVALID 0xff
>  	unsigned char saved_isc;
>  };
> -struct ap_queue_status vfio_ap_irq_disable(struct vfio_ap_queue *q);
> +
> +int vfio_ap_mdev_register(void);
> +void vfio_ap_mdev_unregister(void);

Nit: was moving these two necessary?

> +int vfio_ap_mdev_reset_queue(struct vfio_ap_queue *q,
> +			     unsigned int retry);
> +
>  #endif /* _VFIO_AP_PRIVATE_H_ */
> 
> base-commit: 9791581c049c10929e97098374dd1716a81fefcc

Anyway, if I didn't entangle myself in the various branches, this seems
sane.

Reviewed-by: Cornelia Huck <cohuck@redhat.com>

Re: [PATCH 1/1] s390/vfio-ap: No need to disable IRQ after queue reset

[Halil Pasic]

On Thu, 21 Jan 2021 09:20:44 +0100
Cornelia Huck <cohuck@redhat.com> wrote:

> On Thu, 21 Jan 2021 08:20:08 +0100
> Halil Pasic <pasic@linux.ibm.com> wrote:
[..]
> > --- a/drivers/s390/crypto/vfio_ap_private.h
> > +++ b/drivers/s390/crypto/vfio_ap_private.h
> > @@ -88,11 +88,6 @@ struct ap_matrix_mdev {
> >  	struct mdev_device *mdev;
> >  };
> >  
> > -extern int vfio_ap_mdev_register(void);
> > -extern void vfio_ap_mdev_unregister(void);
> > -int vfio_ap_mdev_reset_queue(unsigned int apid, unsigned int apqi,
> > -			     unsigned int retry);
> > -
> >  struct vfio_ap_queue {
> >  	struct ap_matrix_mdev *matrix_mdev;
> >  	unsigned long saved_pfn;
> > @@ -100,5 +95,10 @@ struct vfio_ap_queue {
> >  #define VFIO_AP_ISC_INVALID 0xff
> >  	unsigned char saved_isc;
> >  };
> > -struct ap_queue_status vfio_ap_irq_disable(struct vfio_ap_queue *q);
> > +
> > +int vfio_ap_mdev_register(void);
> > +void vfio_ap_mdev_unregister(void);  
> 
> Nit: was moving these two necessary?
> 

No not strictly necessary. I decided that having the data types
first and the function prototypes in one place after the former
is nicer.

> > +int vfio_ap_mdev_reset_queue(struct vfio_ap_queue *q,
> > +			     unsigned int retry);
> > +
> >  #endif /* _VFIO_AP_PRIVATE_H_ */
> > 
> > base-commit: 9791581c049c10929e97098374dd1716a81fefcc  
> 
> Anyway, if I didn't entangle myself in the various branches, this seems
> sane.
> 
> Reviewed-by: Cornelia Huck <cohuck@redhat.com>
> 

Thank you very much!

Regards,
Halil

Re: [PATCH 1/1] s390/vfio-ap: No need to disable IRQ after queue reset

[Christian Borntraeger]

On 21.01.21 08:20, Halil Pasic wrote:
> From: Tony Krowiak <akrowiak@linux.ibm.com>
> 
> The queues assigned to a matrix mediated device are currently reset when:
> 
> * The VFIO_DEVICE_RESET ioctl is invoked
> * The mdev fd is closed by userspace (QEMU)
> * The mdev is removed from sysfs.
> 
> Immediately after the reset of a queue, a call is made to disable
> interrupts for the queue. This is entirely unnecessary because the reset of
> a queue disables interrupts, so this will be removed.
> 
> Furthermore, vfio_ap_irq_disable() does an unconditional PQAP/AQIC which
> can result in a specification exception (when the corresponding facility
> is not available), so this is actually a bugfix.
> 
> Signed-off-by: Tony Krowiak <akrowiak@linux.ibm.com>
> [pasic@linux.ibm.com: minor rework before merging]
> Reviewed-by: Halil Pasic <pasic@linux.ibm.com>
> Signed-off-by: Halil Pasic <pasic@linux.ibm.com>
> Fixes: ec89b55e3bce ("s390: ap: implement PAPQ AQIC interception in kernel")
> Cc: <stable@vger.kernel.org>
Acked-by: Christian Borntraeger <borntraeger@de.ibm.com>

Heiko, Vasily, lets carry this via the s390 tree.



> 
> ---
> 
> Since it turned out disabling the interrupts via PQAP/AQIC is not only
> unnecesary but also buggy, we decided to put this patch, which
> used to be apart of the series https://lkml.org/lkml/2020/12/22/757 on the fast
> lane.
> 
> If the backports turn out to be a bother, which I hope won't be the case
> not, I am happy to help with those.
> 
> ---
>  drivers/s390/crypto/vfio_ap_drv.c     |   6 +-
>  drivers/s390/crypto/vfio_ap_ops.c     | 100 ++++++++++++++++----------
>  drivers/s390/crypto/vfio_ap_private.h |  12 ++--
>  3 files changed, 69 insertions(+), 49 deletions(-)
> 
> diff --git a/drivers/s390/crypto/vfio_ap_drv.c b/drivers/s390/crypto/vfio_ap_drv.c
> index be2520cc010b..7dc72cb718b0 100644
> --- a/drivers/s390/crypto/vfio_ap_drv.c
> +++ b/drivers/s390/crypto/vfio_ap_drv.c
> @@ -71,15 +71,11 @@ static int vfio_ap_queue_dev_probe(struct ap_device *apdev)
>  static void vfio_ap_queue_dev_remove(struct ap_device *apdev)
>  {
>  	struct vfio_ap_queue *q;
> -	int apid, apqi;
>  
>  	mutex_lock(&matrix_dev->lock);
>  	q = dev_get_drvdata(&apdev->device);
> +	vfio_ap_mdev_reset_queue(q, 1);
>  	dev_set_drvdata(&apdev->device, NULL);
> -	apid = AP_QID_CARD(q->apqn);
> -	apqi = AP_QID_QUEUE(q->apqn);
> -	vfio_ap_mdev_reset_queue(apid, apqi, 1);
> -	vfio_ap_irq_disable(q);
>  	kfree(q);
>  	mutex_unlock(&matrix_dev->lock);
>  }
> diff --git a/drivers/s390/crypto/vfio_ap_ops.c b/drivers/s390/crypto/vfio_ap_ops.c
> index e0bde8518745..7ceb6c433b3b 100644
> --- a/drivers/s390/crypto/vfio_ap_ops.c
> +++ b/drivers/s390/crypto/vfio_ap_ops.c
> @@ -25,6 +25,7 @@
>  #define VFIO_AP_MDEV_NAME_HWVIRT "VFIO AP Passthrough Device"
>  
>  static int vfio_ap_mdev_reset_queues(struct mdev_device *mdev);
> +static struct vfio_ap_queue *vfio_ap_find_queue(int apqn);
>  
>  static int match_apqn(struct device *dev, const void *data)
>  {
> @@ -49,20 +50,15 @@ static struct vfio_ap_queue *vfio_ap_get_queue(
>  					int apqn)
>  {
>  	struct vfio_ap_queue *q;
> -	struct device *dev;
>  
>  	if (!test_bit_inv(AP_QID_CARD(apqn), matrix_mdev->matrix.apm))
>  		return NULL;
>  	if (!test_bit_inv(AP_QID_QUEUE(apqn), matrix_mdev->matrix.aqm))
>  		return NULL;
>  
> -	dev = driver_find_device(&matrix_dev->vfio_ap_drv->driver, NULL,
> -				 &apqn, match_apqn);
> -	if (!dev)
> -		return NULL;
> -	q = dev_get_drvdata(dev);
> -	q->matrix_mdev = matrix_mdev;
> -	put_device(dev);
> +	q = vfio_ap_find_queue(apqn);
> +	if (q)
> +		q->matrix_mdev = matrix_mdev;
>  
>  	return q;
>  }
> @@ -119,13 +115,18 @@ static void vfio_ap_wait_for_irqclear(int apqn)
>   */
>  static void vfio_ap_free_aqic_resources(struct vfio_ap_queue *q)
>  {
> -	if (q->saved_isc != VFIO_AP_ISC_INVALID && q->matrix_mdev)
> +	if (!q)
> +		return;
> +	if (q->saved_isc != VFIO_AP_ISC_INVALID &&
> +	    !WARN_ON(!(q->matrix_mdev && q->matrix_mdev->kvm))) {
>  		kvm_s390_gisc_unregister(q->matrix_mdev->kvm, q->saved_isc);
> -	if (q->saved_pfn && q->matrix_mdev)
> +		q->saved_isc = VFIO_AP_ISC_INVALID;
> +	}
> +	if (q->saved_pfn && !WARN_ON(!q->matrix_mdev)) {
>  		vfio_unpin_pages(mdev_dev(q->matrix_mdev->mdev),
>  				 &q->saved_pfn, 1);
> -	q->saved_pfn = 0;
> -	q->saved_isc = VFIO_AP_ISC_INVALID;
> +		q->saved_pfn = 0;
> +	}
>  }
>  
>  /**
> @@ -144,7 +145,7 @@ static void vfio_ap_free_aqic_resources(struct vfio_ap_queue *q)
>   * Returns if ap_aqic function failed with invalid, deconfigured or
>   * checkstopped AP.
>   */
> -struct ap_queue_status vfio_ap_irq_disable(struct vfio_ap_queue *q)
> +static struct ap_queue_status vfio_ap_irq_disable(struct vfio_ap_queue *q)
>  {
>  	struct ap_qirq_ctrl aqic_gisa = {};
>  	struct ap_queue_status status;
> @@ -1114,48 +1115,70 @@ 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_queue(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,
> +int vfio_ap_mdev_reset_queue(struct vfio_ap_queue *q,
>  			     unsigned int retry)
>  {
>  	struct ap_queue_status status;
> +	int ret;
>  	int retry2 = 2;
> -	int apqn = AP_MKQID(apid, apqi);
>  
> -	do {
> -		status = ap_zapq(apqn);
> -		switch (status.response_code) {
> -		case AP_RESPONSE_NORMAL:
> -			while (!status.queue_empty && retry2--) {
> -				msleep(20);
> -				status = ap_tapq(apqn, NULL);
> -			}
> -			WARN_ON_ONCE(retry2 <= 0);
> -			return 0;
> -		case AP_RESPONSE_RESET_IN_PROGRESS:
> -		case AP_RESPONSE_BUSY:
> +	if (!q)
> +		return 0;
> +
> +retry_zapq:
> +	status = ap_zapq(q->apqn);
> +	switch (status.response_code) {
> +	case AP_RESPONSE_NORMAL:
> +		ret = 0;
> +		break;
> +	case AP_RESPONSE_RESET_IN_PROGRESS:
> +		if (retry--) {
>  			msleep(20);
> -			break;
> -		default:
> -			/* things are really broken, give up */
> -			return -EIO;
> +			goto retry_zapq;
>  		}
> -	} while (retry--);
> +		ret = -EBUSY;
> +		break;
> +	case AP_RESPONSE_Q_NOT_AVAIL:
> +	case AP_RESPONSE_DECONFIGURED:
> +	case AP_RESPONSE_CHECKSTOPPED:
> +		WARN_ON_ONCE(status.irq_enabled);
> +		ret = -EBUSY;
> +		goto free_resources;
> +	default:
> +		/* things are really broken, give up */
> +		WARN(true, "PQAP/ZAPQ completed with invalid rc (%x)\n",
> +		     status.response_code);
> +		return -EIO;
> +	}
> +
> +	/* wait for the reset to take effect */
> +	while (retry2--) {
> +		if (status.queue_empty && !status.irq_enabled)
> +			break;
> +		msleep(20);
> +		status = ap_tapq(q->apqn, NULL);
> +	}
> +	WARN_ON_ONCE(retry2 <= 0);
>  
> -	return -EBUSY;
> +free_resources:
> +	vfio_ap_free_aqic_resources(q);
> +
> +	return ret;
>  }
>  
>  static int vfio_ap_mdev_reset_queues(struct mdev_device *mdev)
> @@ -1163,13 +1186,15 @@ static int vfio_ap_mdev_reset_queues(struct mdev_device *mdev)
>  	int ret;
>  	int rc = 0;
>  	unsigned long apid, apqi;
> +	struct vfio_ap_queue *q;
>  	struct ap_matrix_mdev *matrix_mdev = mdev_get_drvdata(mdev);
>  
>  	for_each_set_bit_inv(apid, matrix_mdev->matrix.apm,
>  			     matrix_mdev->matrix.apm_max + 1) {
>  		for_each_set_bit_inv(apqi, matrix_mdev->matrix.aqm,
>  				     matrix_mdev->matrix.aqm_max + 1) {
> -			ret = vfio_ap_mdev_reset_queue(apid, apqi, 1);
> +			q = vfio_ap_find_queue(AP_MKQID(apid, apqi));
> +			ret = vfio_ap_mdev_reset_queue(q, 1);
>  			/*
>  			 * Regardless whether a queue turns out to be busy, or
>  			 * is not operational, we need to continue resetting
> @@ -1177,7 +1202,6 @@ static int vfio_ap_mdev_reset_queues(struct mdev_device *mdev)
>  			 */
>  			if (ret)
>  				rc = ret;
> -			vfio_ap_irq_disable_apqn(AP_MKQID(apid, apqi));
>  		}
>  	}
>  
> diff --git a/drivers/s390/crypto/vfio_ap_private.h b/drivers/s390/crypto/vfio_ap_private.h
> index f46dde56b464..28e9d9989768 100644
> --- a/drivers/s390/crypto/vfio_ap_private.h
> +++ b/drivers/s390/crypto/vfio_ap_private.h
> @@ -88,11 +88,6 @@ struct ap_matrix_mdev {
>  	struct mdev_device *mdev;
>  };
>  
> -extern int vfio_ap_mdev_register(void);
> -extern void vfio_ap_mdev_unregister(void);
> -int vfio_ap_mdev_reset_queue(unsigned int apid, unsigned int apqi,
> -			     unsigned int retry);
> -
>  struct vfio_ap_queue {
>  	struct ap_matrix_mdev *matrix_mdev;
>  	unsigned long saved_pfn;
> @@ -100,5 +95,10 @@ struct vfio_ap_queue {
>  #define VFIO_AP_ISC_INVALID 0xff
>  	unsigned char saved_isc;
>  };
> -struct ap_queue_status vfio_ap_irq_disable(struct vfio_ap_queue *q);
> +
> +int vfio_ap_mdev_register(void);
> +void vfio_ap_mdev_unregister(void);
> +int vfio_ap_mdev_reset_queue(struct vfio_ap_queue *q,
> +			     unsigned int retry);
> +
>  #endif /* _VFIO_AP_PRIVATE_H_ */
> 
> base-commit: 9791581c049c10929e97098374dd1716a81fefcc
>