[PATCH v8 6/6] s390/vfio-ap: replace guest-reachable WARNs with ratelimited warnings

[Anthony Krowiak <akrowiak@linux.ibm.com>]

WARN and WARN_ONCE macros in code paths reachable by a guest
can be triggered repeatedly by a malicious or misbehaving guest,
flooding the kernel log and potentially impacting system
stability. Replace all WARN and WARN_ONCE calls reachable from
the guest AP interrupt enable/disable and queue reset paths with
ratelimited warning functions. When the queue is assigned to
an mdev, dev_warn_ratelimited() is used so the mdev device name
(which includes the UUID) appears in the message. Otherwise,
pr_warn_ratelimited() is used.

Five reporting functions are introduced:

report_tapq_rc() - reports an invalid or unexpected response
code from PQAP(TAPQ). Used in vfio_ap_wait_for_irqclear() and
apq_status_check(). The signatures of both functions are changed
to accept a struct vfio_ap_queue pointer instead of an apqn so
the queue's mdev context is available for reporting.

report_irqclear_timeout() - reports a timeout waiting for the
IR bit to clear after a PQAP(AQIC) disable in
vfio_ap_wait_for_irqclear().

report_aqic_disable_error() - reports a failed PQAP(AQIC)
disable operation in vfio_ap_irq_disable(). Replaces three
WARN_ONCE calls covering the non-operational queue, rejected
disable, and retry exhaustion cases.

report_zapq_rc() - reports an invalid response code from
PQAP(ZAPQ) in vfio_ap_mdev_reset_queue().

report_gisc_unregister_failure() - reports a failure to
unregister the guest ISC due to the fact that
q->matrix_mdev or q->matrix_mdev->kvm is NULL.

The VFIO_AP_DBF_WARN() calls in vfio_ap_irq_enable() and
handle_pqap() are retained but augmented with companion
dev_warn_ratelimited() or pr_warn_ratelimited() calls.
VFIO_AP_DBF_WARN() writes only to the s390 debug feature
ring buffer, which requires a sysadmin to know to look in
/sys/kernel/debug/s390dbf/ to find the messages. The companion
dmesg log entries ensure that warning conditions are immediately
visible in the kernel log without requiring familiarity with
the s390 debug feature infrastructure.

Signed-off-by: Anthony Krowiak <akrowiak@linux.ibm.com>
---
 drivers/s390/crypto/vfio_ap_ops.c | 186 +++++++++++++++++++++++-------
 1 file changed, 142 insertions(+), 44 deletions(-)

diff --git a/drivers/s390/crypto/vfio_ap_ops.c b/drivers/s390/crypto/vfio_ap_ops.c
index cd4a436c4319..4b6e64daed25 100644
--- a/drivers/s390/crypto/vfio_ap_ops.c
+++ b/drivers/s390/crypto/vfio_ap_ops.c
@@ -226,6 +226,58 @@ static struct vfio_ap_queue *vfio_ap_mdev_get_queue(
 	return NULL;
 }
 
+static void report_tapq_rc(struct vfio_ap_queue *q, u8 rc)
+{
+	if (q->matrix_mdev)
+		dev_warn_ratelimited(mdev_dev(q->matrix_mdev->mdev),
+				     "PQAP(TAPQ) for %02x.%04x failed with invalid rc=%#02x\n",
+				     AP_QID_CARD(q->apqn),
+				     AP_QID_QUEUE(q->apqn), rc);
+	else
+		pr_warn_ratelimited("PQAP(TAPQ) for %02x.%04x failed with invalid rc=%#02x\n",
+				    AP_QID_CARD(q->apqn),
+				    AP_QID_QUEUE(q->apqn), rc);
+}
+
+static void report_irqclear_timeout(struct vfio_ap_queue *q, u8 rc)
+{
+	if (q->matrix_mdev)
+		dev_warn_ratelimited(mdev_dev(q->matrix_mdev->mdev),
+				     "PQAP(TAPQ) timed out waiting for IRQ clear on %02x.%04x: rc=%#02x\n",
+				     AP_QID_CARD(q->apqn),
+				     AP_QID_QUEUE(q->apqn), rc);
+	else
+		pr_warn_ratelimited("PQAP(TAPQ) timed out waiting for IRQ clear on %02x.%04x: rc=%#02x\n",
+				    AP_QID_CARD(q->apqn),
+				    AP_QID_QUEUE(q->apqn), rc);
+}
+
+static void report_aqic_disable_error(struct vfio_ap_queue *q, u8 rc)
+{
+	if (q->matrix_mdev)
+		dev_warn_ratelimited(mdev_dev(q->matrix_mdev->mdev),
+				     "PQAP(AQIC) disable for %02x.%04x failed with rc=%#02x\n",
+				     AP_QID_CARD(q->apqn),
+				     AP_QID_QUEUE(q->apqn), rc);
+	else
+		pr_warn_ratelimited("PQAP(AQIC) disable for %02x.%04x failed with rc=%#02x\n",
+				    AP_QID_CARD(q->apqn),
+				    AP_QID_QUEUE(q->apqn), rc);
+}
+
+static void report_zapq_rc(struct vfio_ap_queue *q, u8 rc)
+{
+	if (q->matrix_mdev)
+		dev_warn_ratelimited(mdev_dev(q->matrix_mdev->mdev),
+				     "PQAP(ZAPQ) for %02x.%04x failed with invalid rc=%#02x\n",
+				     AP_QID_CARD(q->apqn),
+				     AP_QID_QUEUE(q->apqn), rc);
+	else
+		pr_warn_ratelimited("PQAP(ZAPQ) for %02x.%04x failed with invalid rc=%#02x\n",
+				    AP_QID_CARD(q->apqn),
+				    AP_QID_QUEUE(q->apqn), rc);
+}
+
 /**
  * vfio_ap_wait_for_irqclear - wait for the IR bit to clear after a disable
  *
@@ -256,14 +308,16 @@ static struct vfio_ap_queue *vfio_ap_mdev_get_queue(
  *
  * -EIO		PQAP-TAPQ returned an invalid response code
  */
-static int vfio_ap_wait_for_irqclear(int apqn, struct ap_queue_status *tapq_status)
+static int vfio_ap_wait_for_irqclear(struct vfio_ap_queue *q,
+				     struct ap_queue_status *tapq_status)
 {
 	struct ap_queue_status status;
 	int retry = 5;
 
 	do {
-		status = ap_tapq(apqn, NULL);
+		status = ap_tapq(q->apqn, NULL);
 		memcpy(tapq_status, &status, sizeof(status));
+
 		switch (status.response_code) {
 		case AP_RESPONSE_NORMAL:
 		case AP_RESPONSE_RESET_IN_PROGRESS:
@@ -276,8 +330,6 @@ static int vfio_ap_wait_for_irqclear(int apqn, struct ap_queue_status *tapq_stat
 		case AP_RESPONSE_Q_NOT_AVAIL:
 		case AP_RESPONSE_DECONFIGURED:
 		case AP_RESPONSE_CHECKSTOPPED:
-			WARN_ONCE(1, "%s: tapq rc %02x: %04x\n", __func__,
-				  status.response_code, apqn);
 			return -ENODEV;
 		case AP_RESPONSE_ASSOC_SECRET_NOT_UNIQUE:
 		case AP_RESPONSE_ASSOC_FAILED:
@@ -289,23 +341,53 @@ static int vfio_ap_wait_for_irqclear(int apqn, struct ap_queue_status *tapq_stat
 			 * since that would happen anyway if we continued to
 			 * execute the TAPQ.
 			 */
-			WARN_ONCE(1, "%s: tapq rc %02x: %04x\n", __func__,
-				  status.response_code, apqn);
+			report_tapq_rc(q, status.response_code);
 			return -ETIMEDOUT;
 		default:
-			WARN_ONCE(1, "%s: tapq rc %02x: %04x\n", __func__,
-				  status.response_code, apqn);
+			report_tapq_rc(q, status.response_code);
 			return -EIO;
 		}
 	} while (--retry);
 
-	WARN_ONCE(1, "%s: tapq rc %02x: timed out waiting for interrupts disabled for %02x.%04x\n",
-		  __func__, status.response_code,
-		  AP_QID_CARD(apqn), AP_QID_QUEUE(apqn));
+	report_irqclear_timeout(q, status.response_code);
 
 	return -ETIMEDOUT;
 }
 
+static void report_gisc_unregister_failure(struct vfio_ap_queue *q)
+{
+	if (q->matrix_mdev) {
+		dev_warn_ratelimited(mdev_dev(q->matrix_mdev->mdev),
+				     "APQN %02x.%04x: Failed to unregister guest ISC %c\n",
+				     AP_QID_CARD(q->apqn), AP_QID_QUEUE(q->apqn),
+				     q->saved_isc);
+	} else {
+		pr_warn_ratelimited("APQN %02x.%04x: Failed to unregister guest ISC %c\n",
+				    AP_QID_CARD(q->apqn), AP_QID_QUEUE(q->apqn),
+				    q->saved_isc);
+	}
+}
+
+static bool verify_free_aqic_resourcers(struct vfio_ap_queue *q)
+{
+	bool verified = true;
+
+	if (q->saved_isc != VFIO_AP_ISC_INVALID &&
+	    !(q->matrix_mdev && q->matrix_mdev->kvm)) {
+		report_gisc_unregister_failure(q);
+		verified = false;
+	}
+
+	if (q->saved_iova && !q->matrix_mdev) {
+		pr_warn_ratelimited("APQN %02x.%04x: Failed to unpin NIB page %08x\n",
+				    AP_QID_CARD(q->apqn), AP_QID_QUEUE(q->apqn),
+				    q->saved_iova);
+		verified = false;
+	}
+
+	return verified;
+}
+
 /**
  * vfio_ap_free_aqic_resources - free vfio_ap_queue resources
  * @q: The vfio_ap_queue
@@ -316,17 +398,13 @@ static int vfio_ap_wait_for_irqclear(int apqn, struct ap_queue_status *tapq_stat
  */
 static void vfio_ap_free_aqic_resources(struct vfio_ap_queue *q)
 {
-	if (!q)
+	if (!q || !verify_free_aqic_resourcers(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);
-		q->saved_isc = VFIO_AP_ISC_INVALID;
-	}
-	if (q->saved_iova && !WARN_ON(!q->matrix_mdev)) {
-		vfio_unpin_pages(&q->matrix_mdev->vdev, q->saved_iova, 1);
-		q->saved_iova = 0;
-	}
+
+	kvm_s390_gisc_unregister(q->matrix_mdev->kvm, q->saved_isc);
+	q->saved_isc = VFIO_AP_ISC_INVALID;
+	vfio_unpin_pages(&q->matrix_mdev->vdev, q->saved_iova, 1);
+	q->saved_iova = 0;
 }
 
 /**
@@ -373,7 +451,8 @@ static struct ap_queue_status vfio_ap_irq_disable(struct vfio_ap_queue *q)
 			 * wait until interrupt processing has been disabled
 			 * before proceeding.
 			 */
-			ret = vfio_ap_wait_for_irqclear(q->apqn, &tapq_status);
+			ret = vfio_ap_wait_for_irqclear(q, &tapq_status);
+
 			if (ret == 0 || ret == -ENODEV)
 				goto end_free;
 
@@ -420,8 +499,7 @@ static struct ap_queue_status vfio_ap_irq_disable(struct vfio_ap_queue *q)
 		case AP_RESPONSE_DECONFIGURED:
 		case AP_RESPONSE_CHECKSTOPPED:
 			/* AP not operational; no further interrupts possible */
-			WARN_ONCE(1, "%s: ap_aqic status %d\n", __func__,
-				  status.response_code);
+			report_aqic_disable_error(q, status.response_code);
 			goto end_free;
 		case AP_RESPONSE_INVALID_ADDRESS:
 		case AP_RESPONSE_INVALID_GISA:
@@ -433,14 +511,12 @@ static struct ap_queue_status vfio_ap_irq_disable(struct vfio_ap_queue *q)
 			 * and the hardware still holds the NIB address. Do not
 			 * free resources.
 			 */
-			WARN_ONCE(1, "%s: ap_aqic status %d\n", __func__,
-				  status.response_code);
+			report_aqic_disable_error(q, status.response_code);
 			goto end_fail;
 		}
 	} while (retries--);
 
-	WARN_ONCE(1, "%s: ap_aqic status %d\n", __func__,
-		  status.response_code);
+	report_aqic_disable_error(q, status.response_code);
 
 end_fail:
 	/*
@@ -569,7 +645,10 @@ static struct ap_queue_status vfio_ap_irq_enable(struct vfio_ap_queue *q,
 	if (vfio_ap_validate_nib(vcpu, &nib)) {
 		VFIO_AP_DBF_WARN("%s: invalid NIB address: nib=%pad, apqn=%#04x\n",
 				 __func__, &nib, q->apqn);
-
+		dev_warn_ratelimited(mdev_dev(q->matrix_mdev->mdev),
+				     "PQAP(AQIC) enable for %02x.%04x: invalid NIB address %pad\n",
+				     AP_QID_CARD(q->apqn),
+				     AP_QID_QUEUE(q->apqn), &nib);
 		status.response_code = AP_RESPONSE_INVALID_ADDRESS;
 		return status;
 	}
@@ -584,7 +663,10 @@ static struct ap_queue_status vfio_ap_irq_enable(struct vfio_ap_queue *q,
 		VFIO_AP_DBF_WARN("%s: vfio_pin_pages failed: rc=%d,"
 				 "nib=%pad, apqn=%#04x\n",
 				 __func__, ret, &nib, q->apqn);
-
+		dev_warn_ratelimited(mdev_dev(q->matrix_mdev->mdev),
+				     "PQAP(AQIC) enable for %02x.%04x: vfio_pin_pages failed rc=%d\n",
+				     AP_QID_CARD(q->apqn),
+				     AP_QID_QUEUE(q->apqn), ret);
 		status.response_code = AP_RESPONSE_INVALID_ADDRESS;
 		return status;
 	}
@@ -607,7 +689,10 @@ static struct ap_queue_status vfio_ap_irq_enable(struct vfio_ap_queue *q,
 	if (nisc < 0) {
 		VFIO_AP_DBF_WARN("%s: gisc registration failed: nisc=%d, isc=%d, apqn=%#04x\n",
 				 __func__, nisc, isc, q->apqn);
-
+		dev_warn_ratelimited(mdev_dev(q->matrix_mdev->mdev),
+				     "PQAP(AQIC) enable for %02x.%04x: GISC registration failed rc=%d isc=%d\n",
+				     AP_QID_CARD(q->apqn),
+				     AP_QID_QUEUE(q->apqn), nisc, isc);
 		vfio_unpin_pages(&q->matrix_mdev->vdev, nib, 1);
 		status.response_code = AP_RESPONSE_INVALID_ADDRESS;
 		return status;
@@ -651,9 +736,14 @@ static struct ap_queue_status vfio_ap_irq_enable(struct vfio_ap_queue *q,
 		 * ISC that were prepared for this (rejected) request.
 		 */
 		ret = kvm_s390_gisc_unregister(kvm, isc);
-		if (ret)
+		if (ret) {
 			VFIO_AP_DBF_WARN("%s: kvm_s390_gisc_unregister: rc=%d isc=%d, apqn=%#04x\n",
 					 __func__, ret, isc, q->apqn);
+			dev_warn_ratelimited(mdev_dev(q->matrix_mdev->mdev),
+					     "PQAP(AQIC) enable for %02x.%04x: GISC unregister failed rc=%d isc=%d\n",
+					     AP_QID_CARD(q->apqn),
+					     AP_QID_QUEUE(q->apqn), ret, isc);
+		}
 		vfio_unpin_pages(&q->matrix_mdev->vdev, nib, 1);
 		break;
 	}
@@ -667,6 +757,11 @@ static struct ap_queue_status vfio_ap_irq_enable(struct vfio_ap_queue *q,
 				 aqic_gisa.zone, aqic_gisa.ir, aqic_gisa.gisc,
 				 aqic_gisa.gf, aqic_gisa.gisa, aqic_gisa.isc,
 				 q->apqn);
+		dev_warn_ratelimited(mdev_dev(q->matrix_mdev->mdev),
+				     "PQAP(AQIC) enable for %02x.%04x failed with rc=%#02x\n",
+				     AP_QID_CARD(q->apqn),
+				     AP_QID_QUEUE(q->apqn),
+				     status.response_code);
 	}
 
 	return status;
@@ -751,7 +846,8 @@ static int handle_pqap(struct kvm_vcpu *vcpu)
 	if (!(vcpu->arch.sie_block->eca & ECA_AIV)) {
 		VFIO_AP_DBF_WARN("%s: AIV facility not installed: apqn=0x%04x, eca=0x%04x\n",
 				 __func__, apqn, vcpu->arch.sie_block->eca);
-
+		pr_warn_ratelimited("PQAP(AQIC) for %02x.%04x: AIV facility not installed\n",
+				    AP_QID_CARD(apqn), AP_QID_QUEUE(apqn));
 		return -EOPNOTSUPP;
 	}
 
@@ -760,7 +856,8 @@ static int handle_pqap(struct kvm_vcpu *vcpu)
 	if (!vcpu->kvm->arch.crypto.pqap_hook) {
 		VFIO_AP_DBF_WARN("%s: PQAP(AQIC) hook not registered with the vfio_ap driver: apqn=0x%04x\n",
 				 __func__, apqn);
-
+		pr_warn_ratelimited("PQAP(AQIC) for %02x.%04x: hook not registered with the vfio_ap driver\n",
+				    AP_QID_CARD(apqn), AP_QID_QUEUE(apqn));
 		goto out_unlock;
 	}
 
@@ -773,6 +870,9 @@ static int handle_pqap(struct kvm_vcpu *vcpu)
 		VFIO_AP_DBF_WARN("%s: mdev %08lx-%04lx-%04lx-%04lx-%04lx%08lx not in use: apqn=0x%04x\n",
 				 __func__, uuid[0],  uuid[1], uuid[2],
 				 uuid[3], uuid[4], uuid[5], apqn);
+		dev_warn_ratelimited(mdev_dev(matrix_mdev->mdev),
+				     "PQAP(AQIC) for %02x.%04x: mdev not in use\n",
+				     AP_QID_CARD(apqn), AP_QID_QUEUE(apqn));
 		goto out_unlock;
 	}
 
@@ -781,6 +881,9 @@ static int handle_pqap(struct kvm_vcpu *vcpu)
 		VFIO_AP_DBF_WARN("%s: Queue %02x.%04x not bound to the vfio_ap driver\n",
 				 __func__, AP_QID_CARD(apqn),
 				 AP_QID_QUEUE(apqn));
+		dev_warn_ratelimited(mdev_dev(matrix_mdev->mdev),
+				     "PQAP(AQIC) for %02x.%04x: queue not bound to the vfio_ap driver\n",
+				     AP_QID_CARD(apqn), AP_QID_QUEUE(apqn));
 		goto out_unlock;
 	}
 
@@ -2084,7 +2187,8 @@ static struct vfio_ap_queue *vfio_ap_find_queue(int apqn)
 	return q;
 }
 
-static int apq_status_check(int apqn, struct ap_queue_status *status)
+static int apq_status_check(struct vfio_ap_queue *q,
+			    struct ap_queue_status *status)
 {
 	switch (status->response_code) {
 	case AP_RESPONSE_NORMAL:
@@ -2146,10 +2250,7 @@ static int apq_status_check(int apqn, struct ap_queue_status *status)
 		return -EAGAIN;
 
 	default:
-		WARN(true,
-		     "failed to verify reset of queue %02x.%04x: TAPQ rc=%u\n",
-		     AP_QID_CARD(apqn), AP_QID_QUEUE(apqn),
-		     status->response_code);
+		report_tapq_rc(q, status->response_code);
 		return -EIO;
 	}
 }
@@ -2219,7 +2320,7 @@ static void apq_reset_check(struct work_struct *reset_work)
 		msleep(AP_RESET_INTERVAL);
 		elapsed += AP_RESET_INTERVAL;
 		status = ap_tapq(q->apqn, NULL);
-		ret = apq_status_check(q->apqn, &status);
+		ret = apq_status_check(q, &status);
 		if (ret == -EIO) {
 			/*
 			 * TAPQ returned an invalid response code indicating a
@@ -2343,10 +2444,7 @@ static void vfio_ap_mdev_reset_queue(struct vfio_ap_queue *q)
 		 * would corrupt the new owner's memory which could crash or
 		 * compromise the host kernel.
 		 */
-		WARN(true,
-		     "PQAP/ZAPQ for %02x.%04x failed with invalid rc=%u\n",
-		     AP_QID_CARD(q->apqn), AP_QID_QUEUE(q->apqn),
-		     status.response_code);
+		report_zapq_rc(q, status.response_code);
 	}
 }
 
-- 
2.53.0