a couple of corrections to the IRQ enablement function

From: Anthony Krowiak <[email protected]>

In the vfio_ap_irq_enable function, after the page containing the notification indicator byte (NIB) is pinned, the function attempts to register the guest ISC. If registration fails, the function sets the status response code and returns without unpinning the page containing the NIB. In order to avoid a memory leak, the NIB should be unpinned before returning from the vfio_ap_irq_enable function.

Co-developed-by Janosch Frank <[email protected]>

Signed-off-by Janosch Frank <[email protected]>

Signed-off-by Anthony Krowiak <[email protected]>

Reviewed-by Matthew Rosato <[email protected]>

Fixes 783f0a3ccd79 ("s390/vfio-ap: add s390dbf logging to the vfio_ap_irq_enable function")

Cc <[email protected]>

---
  drivers/s390/crypto/vfio_ap_ops.c | 1 +
  1 file changed, 1 insertion(+)

drivers/s390/crypto/vfio_ap_ops.c +1 -0

index 4db538a55192..9cb28978c186 100644
@@ -457,6 +457,7 @@ static struct ap_queue_status vfio_ap_irq_enable(struct vfio_ap_queue *q,
  		VFIO_AP_DBF_WARN("%s: gisc registration failed: nisc=%d, isc=%d, apqn=%#04x\n",
  				 __func__, nisc, isc, q->apqn);
 
+		vfio_unpin_pages(&q->matrix_mdev->vdev, nib, 1);
  		status.response_code = AP_RESPONSE_INVALID_GISA;
  		return status;
  	}

From: Anthony Krowiak <[email protected]>

The interception handler for the PQAP(AQIC) command calls the kvm_s390_gisc_register function to register the guest ISC with the channel subsystem. If that call fails, the status response code 08 - indicating Invalid ZONE/GISA designation - is returned to the guest. This response code is not valid because setting the ZONE/GISA values is the responsibility of the hypervisor controlling the guest and there is nothing that can be done from the guest perspective to correct that problem.

The likelihood of GISC registration failure is nil and there is no status response code to indicate an invalid ISC value, so let's set the response code to 06 indicating 'Invalid address of AP-queue notification byte'. While this is not entirely accurate, it is better than setting a response code which makes no sense for the guest.

Signed-off-by Anthony Krowiak <[email protected]>

Suggested-by Halil Pasic <[email protected]>

Reviewed-by Halil Pasic <[email protected]>

Reviewed-by Harald Freudenberger <[email protected]>

---
  drivers/s390/crypto/vfio_ap_ops.c | 6 +++---
  1 file changed, 3 insertions(+), 3 deletions(-)

drivers/s390/crypto/vfio_ap_ops.c +3 -3

index 9cb28978c186..25d7ce2094f8 100644
@@ -393,8 +393,8 @@ static int ensure_nib_shared(unsigned long addr, struct gmap *gmap)
   * Register the guest ISC to GIB interface and retrieve the
   * host ISC to issue the host side PQAP/AQIC
   *
- * Response.status may be set to AP_RESPONSE_INVALID_ADDRESS in case the
- * vfio_pin_pages failed.
+ * status.response_code may be set to AP_RESPONSE_INVALID_ADDRESS in case the
+ * vfio_pin_pages or kvm_s390_gisc_register failed.
   *
   * Otherwise return the ap_queue_status returned by the ap_aqic(),
   * all retry handling will be done by the guest.

@@ -458,7 +458,7 @@ static struct ap_queue_status vfio_ap_irq_enable(struct vfio_ap_queue *q,
  				 __func__, nisc, isc, q->apqn);
 
  		vfio_unpin_pages(&q->matrix_mdev->vdev, nib, 1);
-		status.response_code = AP_RESPONSE_INVALID_GISA;
+		status.response_code = AP_RESPONSE_INVALID_ADDRESS;
  		return status;
  	}
 

Let's improve the vfio_ap driver's reaction to reception of response code 07 from the PQAP(AQIC) command when enabling interrupts on behalf of a guest:

* Unregister the guest's ISC before the pages containing the notification indicator bytes are unpinned.

* Capture the return code from the kvm_s390_gisc_unregister function and log a DBF warning if it fails.

Suggested-by Matthew Rosato <[email protected]>

Signed-off-by Tony Krowiak <[email protected]>

Reviewed-by Matthew Rosato <[email protected]>

---
  drivers/s390/crypto/vfio_ap_ops.c | 5 ++++-
  1 file changed, 4 insertions(+), 1 deletion(-)

drivers/s390/crypto/vfio_ap_ops.c +4 -1

index 25d7ce2094f8..4e80c211ba47 100644
@@ -476,8 +476,11 @@ static struct ap_queue_status vfio_ap_irq_enable(struct vfio_ap_queue *q,
  		break;
  	case AP_RESPONSE_OTHERWISE_CHANGED:
  		/* We could not modify IRQ settings: clear new configuration */
+		ret = kvm_s390_gisc_unregister(kvm, isc);
+		if (ret)
+			VFIO_AP_DBF_WARN("%s: kvm_s390_gisc_unregister: rc=%d isc=%d, apqn=%#04x\n",
+					 __func__, ret, isc, q->apqn);
  		vfio_unpin_pages(&q->matrix_mdev->vdev, nib, 1);
-		kvm_s390_gisc_unregister(kvm, isc);
  		break;
  	default:
  		pr_warn("%s: apqn %04x: response: %02x\n", __func__, q->apqn,