From 680b7ddd7e2ab7638d431722432f6d02d75dade1 Mon Sep 17 00:00:00 2001 From: Tony Krowiak Date: Tue, 15 Aug 2023 14:43:22 -0400 Subject: s390/vfio-ap: no need to check the 'E' and 'I' bits in APQSW after TAPQ After a ZAPQ is executed to reset a queue, if the queue is not empty or interrupts are still enabled, the vfio_ap driver will wait for the reset operation to complete by repeatedly executing the TAPQ instruction and checking the 'E' and 'I' bits in the APQSW to verify that the queue is empty and interrupts are disabled. This is unnecessary because it is sufficient to check only the response code in the APQSW. If the reset is still in progress, the response code will be 02; however, if the reset has completed successfully, the response code will be 00. Signed-off-by: Tony Krowiak Acked-by: Janosch Frank Tested-by: Viktor Mihajlovski Link: https://lore.kernel.org/r/20230815184333.6554-2-akrowiak@linux.ibm.com Signed-off-by: Heiko Carstens --- drivers/s390/crypto/vfio_ap_ops.c | 13 ++----------- 1 file changed, 2 insertions(+), 11 deletions(-) diff --git a/drivers/s390/crypto/vfio_ap_ops.c b/drivers/s390/crypto/vfio_ap_ops.c index b441745b0418..3fd80533194b 100644 --- a/drivers/s390/crypto/vfio_ap_ops.c +++ b/drivers/s390/crypto/vfio_ap_ops.c @@ -1608,19 +1608,10 @@ static int apq_status_check(int apqn, struct ap_queue_status *status) { switch (status->response_code) { case AP_RESPONSE_NORMAL: - case AP_RESPONSE_RESET_IN_PROGRESS: - if (status->queue_empty && !status->irq_enabled) - return 0; - return -EBUSY; case AP_RESPONSE_DECONFIGURED: - /* - * If the AP queue is deconfigured, any subsequent AP command - * targeting the queue will fail with the same response code. On the - * other hand, when an AP adapter is deconfigured, the associated - * queues are reset, so let's return a value indicating the reset - * for which we're waiting completed successfully. - */ return 0; + case AP_RESPONSE_RESET_IN_PROGRESS: + return -EBUSY; default: WARN(true, "failed to verify reset of queue %02x.%04x: TAPQ rc=%u\n", -- cgit v1.2.3-58-ga151 From 7aa7b2a80cb70d528785f06a54d6c8148826006d Mon Sep 17 00:00:00 2001 From: Tony Krowiak Date: Tue, 15 Aug 2023 14:43:23 -0400 Subject: s390/vfio-ap: clean up irq resources if possible The architecture does not specify whether interrupts are disabled as part of the asynchronous reset or upon return from the PQAP/ZAPQ instruction. If, however, PQAP/ZAPQ completes with APQSW response code 0 and the interrupt bit in the status word is also 0, we know the interrupts are disabled and we can go ahead and clean up the corresponding resources; otherwise, we must wait until the asynchronous reset has completed. Signed-off-by: Tony Krowiak Suggested-by: Halil Pasic Reviewed-by: Jason J. Herne Acked-by: Halil Pasic Acked-by: Janosch Frank Tested-by: Viktor Mihajlovski Link: https://lore.kernel.org/r/20230815184333.6554-3-akrowiak@linux.ibm.com Signed-off-by: Heiko Carstens --- drivers/s390/crypto/vfio_ap_ops.c | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/drivers/s390/crypto/vfio_ap_ops.c b/drivers/s390/crypto/vfio_ap_ops.c index 3fd80533194b..be92ba45226d 100644 --- a/drivers/s390/crypto/vfio_ap_ops.c +++ b/drivers/s390/crypto/vfio_ap_ops.c @@ -1654,9 +1654,13 @@ retry_zapq: switch (status.response_code) { case AP_RESPONSE_NORMAL: ret = 0; - /* if the reset has not completed, wait for it to take effect */ - if (!status.queue_empty || status.irq_enabled) + if (!status.irq_enabled) + vfio_ap_free_aqic_resources(q); + if (!status.queue_empty || status.irq_enabled) { ret = apq_reset_check(q); + if (status.irq_enabled && ret == 0) + vfio_ap_free_aqic_resources(q); + } break; case AP_RESPONSE_RESET_IN_PROGRESS: /* @@ -1675,6 +1679,7 @@ retry_zapq: * completed successfully. */ ret = 0; + vfio_ap_free_aqic_resources(q); break; default: WARN(true, @@ -1684,8 +1689,6 @@ retry_zapq: return -EIO; } - vfio_ap_free_aqic_resources(q); - return ret; } -- cgit v1.2.3-58-ga151 From 411b0109daa52d1cc5be39635631e22a5590c5d8 Mon Sep 17 00:00:00 2001 From: Tony Krowiak Date: Tue, 15 Aug 2023 14:43:24 -0400 Subject: s390/vfio-ap: wait for response code 05 to clear on queue reset Response code 05, AP busy, is a valid response code for a ZAPQ or TAPQ. Instead of returning error -EIO when a ZAPQ fails with response code 05, let's wait until the queue is no longer busy and try the ZAPQ again. Signed-off-by: Tony Krowiak Acked-by: Janosch Frank Tested-by: Viktor Mihajlovski Link: https://lore.kernel.org/r/20230815184333.6554-4-akrowiak@linux.ibm.com Signed-off-by: Heiko Carstens --- drivers/s390/crypto/vfio_ap_ops.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/s390/crypto/vfio_ap_ops.c b/drivers/s390/crypto/vfio_ap_ops.c index be92ba45226d..3f67cfb53d0c 100644 --- a/drivers/s390/crypto/vfio_ap_ops.c +++ b/drivers/s390/crypto/vfio_ap_ops.c @@ -1611,6 +1611,7 @@ static int apq_status_check(int apqn, struct ap_queue_status *status) case AP_RESPONSE_DECONFIGURED: return 0; case AP_RESPONSE_RESET_IN_PROGRESS: + case AP_RESPONSE_BUSY: return -EBUSY; default: WARN(true, @@ -1663,6 +1664,7 @@ retry_zapq: } break; case AP_RESPONSE_RESET_IN_PROGRESS: + case AP_RESPONSE_BUSY: /* * There is a reset issued by another process in progress. Let's wait * for that to complete. Since we have no idea whether it was a RAPQ or -- cgit v1.2.3-58-ga151 From c51f8c6bb5c8a4878310d55e3a0b91747954b43d Mon Sep 17 00:00:00 2001 From: Tony Krowiak Date: Tue, 15 Aug 2023 14:43:25 -0400 Subject: s390/vfio-ap: allow deconfigured queue to be passed through to a guest When a queue is reset, the status response code returned from the reset operation is stored in the reset_rc field of the vfio_ap_queue structure representing the queue being reset. This field is later used to decide whether the queue should be passed through to a guest. If the reset_rc field is a non-zero value, the queue will be filtered from the list of queues passed through. When an adapter is deconfigured, all queues associated with that adapter are reset. That being the case, it is not necessary to filter those queues; so, if the status response code returned from a reset operation indicates the queue is deconfigured, the reset_rc field of the vfio_ap_queue structure will be set to zero so it will be passed through (i.e., not filtered). Signed-off-by: Tony Krowiak Reviewed-by: Jason J. Herne Acked-by: Halil Pasic Tested-by: Viktor Mihajlovski Link: https://lore.kernel.org/r/20230815184333.6554-5-akrowiak@linux.ibm.com Signed-off-by: Heiko Carstens --- drivers/s390/crypto/vfio_ap_ops.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/drivers/s390/crypto/vfio_ap_ops.c b/drivers/s390/crypto/vfio_ap_ops.c index 3f67cfb53d0c..a489536c508a 100644 --- a/drivers/s390/crypto/vfio_ap_ops.c +++ b/drivers/s390/crypto/vfio_ap_ops.c @@ -1677,9 +1677,11 @@ retry_zapq: case AP_RESPONSE_DECONFIGURED: /* * When an AP adapter is deconfigured, the associated - * queues are reset, so let's return a value indicating the reset - * completed successfully. + * queues are reset, so let's set the status response code to 0 + * so the queue may be passed through (i.e., not filtered) and + * return a value indicating the reset completed successfully. */ + q->reset_rc = 0; ret = 0; vfio_ap_free_aqic_resources(q); break; -- cgit v1.2.3-58-ga151 From dd174833e44e7717f88f0925b1f78e9ba1d2626e Mon Sep 17 00:00:00 2001 From: Tony Krowiak Date: Tue, 15 Aug 2023 14:43:26 -0400 Subject: s390/vfio-ap: remove upper limit on wait for queue reset to complete The architecture does not define an upper limit on how long a queue reset (RAPQ/ZAPQ) can take to complete. In order to ensure both the security requirements and prevent resource leakage and corruption in the hypervisor, it is necessary to remove the upper limit (200ms) the vfio_ap driver currently waits for a reset to complete. This, of course, may result in a hang which is a less than desirable user experience, but until a firmware solution is provided, this is a necessary evil. Signed-off-by: Tony Krowiak Reviewed-by: Jason J. Herne Acked-by: Halil Pasic Tested-by: Viktor Mihajlovski Link: https://lore.kernel.org/r/20230815184333.6554-6-akrowiak@linux.ibm.com Signed-off-by: Heiko Carstens --- drivers/s390/crypto/vfio_ap_ops.c | 64 +++++++++++++++++++++------------------ 1 file changed, 35 insertions(+), 29 deletions(-) diff --git a/drivers/s390/crypto/vfio_ap_ops.c b/drivers/s390/crypto/vfio_ap_ops.c index a489536c508a..2517868aad56 100644 --- a/drivers/s390/crypto/vfio_ap_ops.c +++ b/drivers/s390/crypto/vfio_ap_ops.c @@ -30,7 +30,6 @@ #define AP_QUEUE_UNASSIGNED "unassigned" #define AP_QUEUE_IN_USE "in use" -#define MAX_RESET_CHECK_WAIT 200 /* Sleep max 200ms for reset check */ #define AP_RESET_INTERVAL 20 /* Reset sleep interval (20ms) */ static int vfio_ap_mdev_reset_queues(struct ap_queue_table *qtable); @@ -1622,58 +1621,66 @@ static int apq_status_check(int apqn, struct ap_queue_status *status) } } +#define WAIT_MSG "Waited %dms for reset of queue %02x.%04x (%u, %u, %u)" + static int apq_reset_check(struct vfio_ap_queue *q) { - int ret; - int iters = MAX_RESET_CHECK_WAIT / AP_RESET_INTERVAL; + int ret = -EBUSY, elapsed = 0; struct ap_queue_status status; - for (; iters > 0; iters--) { + while (true) { msleep(AP_RESET_INTERVAL); + elapsed += AP_RESET_INTERVAL; status = ap_tapq(q->apqn, NULL); ret = apq_status_check(q->apqn, &status); - if (ret != -EBUSY) + if (ret == -EIO) return ret; + if (ret == -EBUSY) { + pr_notice_ratelimited(WAIT_MSG, elapsed, + AP_QID_CARD(q->apqn), + AP_QID_QUEUE(q->apqn), + status.response_code, + status.queue_empty, + status.irq_enabled); + } else { + if (q->reset_rc == AP_RESPONSE_RESET_IN_PROGRESS || + q->reset_rc == AP_RESPONSE_BUSY) { + status = ap_zapq(q->apqn, 0); + q->reset_rc = status.response_code; + continue; + } + /* + * When an AP adapter is deconfigured, the associated + * queues are reset, so let's set the status response + * code to 0 so the queue may be passed through (i.e., + * not filtered). + */ + if (q->reset_rc == AP_RESPONSE_DECONFIGURED) + q->reset_rc = 0; + if (q->saved_isc != VFIO_AP_ISC_INVALID) + vfio_ap_free_aqic_resources(q); + break; + } } - WARN_ONCE(iters <= 0, - "timeout verifying reset of queue %02x.%04x (%u, %u, %u)", - AP_QID_CARD(q->apqn), AP_QID_QUEUE(q->apqn), - status.queue_empty, status.irq_enabled, status.response_code); return ret; } static int vfio_ap_mdev_reset_queue(struct vfio_ap_queue *q) { struct ap_queue_status status; - int ret; + int ret = 0; if (!q) return 0; -retry_zapq: status = ap_zapq(q->apqn, 0); q->reset_rc = status.response_code; switch (status.response_code) { case AP_RESPONSE_NORMAL: - ret = 0; - if (!status.irq_enabled) - vfio_ap_free_aqic_resources(q); - if (!status.queue_empty || status.irq_enabled) { - ret = apq_reset_check(q); - if (status.irq_enabled && ret == 0) - vfio_ap_free_aqic_resources(q); - } - break; case AP_RESPONSE_RESET_IN_PROGRESS: case AP_RESPONSE_BUSY: - /* - * There is a reset issued by another process in progress. Let's wait - * for that to complete. Since we have no idea whether it was a RAPQ or - * ZAPQ, then if it completes successfully, let's issue the ZAPQ. - */ + /* Let's verify whether the ZAPQ completed successfully */ ret = apq_reset_check(q); - if (ret) - break; - goto retry_zapq; + break; case AP_RESPONSE_DECONFIGURED: /* * When an AP adapter is deconfigured, the associated @@ -1682,7 +1689,6 @@ retry_zapq: * return a value indicating the reset completed successfully. */ q->reset_rc = 0; - ret = 0; vfio_ap_free_aqic_resources(q); break; default: -- cgit v1.2.3-58-ga151 From 62aab082e9993163731656ce270cd3c1d29079af Mon Sep 17 00:00:00 2001 From: Tony Krowiak Date: Tue, 15 Aug 2023 14:43:27 -0400 Subject: s390/vfio-ap: store entire AP queue status word with the queue object Store the entire AP queue status word returned from the ZAPQ command with the struct vfio_ap_queue object instead of just the response code field. The other information contained in the status word is need by the apq_reset_check function to display a proper message to indicate that the vfio_ap driver is waiting for the ZAPQ to complete because the queue is not empty or IRQs are still enabled. Signed-off-by: Tony Krowiak Tested-by: Viktor Mihajlovski Link: https://lore.kernel.org/r/20230815184333.6554-7-akrowiak@linux.ibm.com Signed-off-by: Heiko Carstens --- drivers/s390/crypto/vfio_ap_ops.c | 27 +++++++++++++++------------ drivers/s390/crypto/vfio_ap_private.h | 4 ++-- 2 files changed, 17 insertions(+), 14 deletions(-) diff --git a/drivers/s390/crypto/vfio_ap_ops.c b/drivers/s390/crypto/vfio_ap_ops.c index 2517868aad56..43224f7a40ea 100644 --- a/drivers/s390/crypto/vfio_ap_ops.c +++ b/drivers/s390/crypto/vfio_ap_ops.c @@ -674,7 +674,7 @@ static bool vfio_ap_mdev_filter_matrix(unsigned long *apm, unsigned long *aqm, */ apqn = AP_MKQID(apid, apqi); q = vfio_ap_mdev_get_queue(matrix_mdev, apqn); - if (!q || q->reset_rc) { + if (!q || q->reset_status.response_code) { clear_bit_inv(apid, matrix_mdev->shadow_apcb.apm); break; @@ -1628,6 +1628,7 @@ static int apq_reset_check(struct vfio_ap_queue *q) int ret = -EBUSY, elapsed = 0; struct ap_queue_status status; + memcpy(&status, &q->reset_status, sizeof(status)); while (true) { msleep(AP_RESET_INTERVAL); elapsed += AP_RESET_INTERVAL; @@ -1643,20 +1644,20 @@ static int apq_reset_check(struct vfio_ap_queue *q) status.queue_empty, status.irq_enabled); } else { - if (q->reset_rc == AP_RESPONSE_RESET_IN_PROGRESS || - q->reset_rc == AP_RESPONSE_BUSY) { + if (q->reset_status.response_code == AP_RESPONSE_RESET_IN_PROGRESS || + q->reset_status.response_code == AP_RESPONSE_BUSY) { status = ap_zapq(q->apqn, 0); - q->reset_rc = status.response_code; + memcpy(&q->reset_status, &status, sizeof(status)); continue; } /* - * When an AP adapter is deconfigured, the associated - * queues are reset, so let's set the status response - * code to 0 so the queue may be passed through (i.e., - * not filtered). + * When an AP adapter is deconfigured, the + * associated queues are reset, so let's set the + * status response code to 0 so the queue may be + * passed through (i.e., not filtered) */ - if (q->reset_rc == AP_RESPONSE_DECONFIGURED) - q->reset_rc = 0; + if (status.response_code == AP_RESPONSE_DECONFIGURED) + q->reset_status.response_code = 0; if (q->saved_isc != VFIO_AP_ISC_INVALID) vfio_ap_free_aqic_resources(q); break; @@ -1673,7 +1674,7 @@ static int vfio_ap_mdev_reset_queue(struct vfio_ap_queue *q) if (!q) return 0; status = ap_zapq(q->apqn, 0); - q->reset_rc = status.response_code; + memcpy(&q->reset_status, &status, sizeof(status)); switch (status.response_code) { case AP_RESPONSE_NORMAL: case AP_RESPONSE_RESET_IN_PROGRESS: @@ -1688,7 +1689,8 @@ static int vfio_ap_mdev_reset_queue(struct vfio_ap_queue *q) * so the queue may be passed through (i.e., not filtered) and * return a value indicating the reset completed successfully. */ - q->reset_rc = 0; + q->reset_status.response_code = 0; + ret = 0; vfio_ap_free_aqic_resources(q); break; default: @@ -2042,6 +2044,7 @@ int vfio_ap_mdev_probe_queue(struct ap_device *apdev) q->apqn = to_ap_queue(&apdev->device)->qid; q->saved_isc = VFIO_AP_ISC_INVALID; + memset(&q->reset_status, 0, sizeof(q->reset_status)); matrix_mdev = get_update_locks_by_apqn(q->apqn); if (matrix_mdev) { diff --git a/drivers/s390/crypto/vfio_ap_private.h b/drivers/s390/crypto/vfio_ap_private.h index 4642bbdbd1b2..d6eb3527e056 100644 --- a/drivers/s390/crypto/vfio_ap_private.h +++ b/drivers/s390/crypto/vfio_ap_private.h @@ -133,7 +133,7 @@ struct ap_matrix_mdev { * @apqn: the APQN of the AP queue device * @saved_isc: the guest ISC registered with the GIB interface * @mdev_qnode: allows the vfio_ap_queue struct to be added to a hashtable - * @reset_rc: the status response code from the last reset of the queue + * @reset_status: the status from the last reset of the queue */ struct vfio_ap_queue { struct ap_matrix_mdev *matrix_mdev; @@ -142,7 +142,7 @@ struct vfio_ap_queue { #define VFIO_AP_ISC_INVALID 0xff unsigned char saved_isc; struct hlist_node mdev_qnode; - unsigned int reset_rc; + struct ap_queue_status reset_status; }; int vfio_ap_mdev_register(void); -- cgit v1.2.3-58-ga151 From 9261f0438835a97254590046e1be83733cca440f Mon Sep 17 00:00:00 2001 From: Tony Krowiak Date: Tue, 15 Aug 2023 14:43:28 -0400 Subject: s390/vfio-ap: use work struct to verify queue reset Instead of waiting to verify that a queue is reset in the vfio_ap_mdev_reset_queue function, let's use a wait queue to check the the state of the reset. This way, when resetting all of the queues assigned to a matrix mdev, we don't have to wait for each queue to be reset before initiating a reset on the next queue to be reset. Signed-off-by: Tony Krowiak Reviewed-by: Jason J. Herne Suggested-by: Halil Pasic Acked-by: Janosch Frank Tested-by: Viktor Mihajlovski Link: https://lore.kernel.org/r/20230815184333.6554-8-akrowiak@linux.ibm.com Signed-off-by: Heiko Carstens --- drivers/s390/crypto/vfio_ap_ops.c | 48 +++++++++++++++++------------------ drivers/s390/crypto/vfio_ap_private.h | 2 ++ 2 files changed, 25 insertions(+), 25 deletions(-) diff --git a/drivers/s390/crypto/vfio_ap_ops.c b/drivers/s390/crypto/vfio_ap_ops.c index 43224f7a40ea..3a59f1c5390f 100644 --- a/drivers/s390/crypto/vfio_ap_ops.c +++ b/drivers/s390/crypto/vfio_ap_ops.c @@ -35,7 +35,7 @@ static int vfio_ap_mdev_reset_queues(struct ap_queue_table *qtable); static struct vfio_ap_queue *vfio_ap_find_queue(int apqn); static const struct vfio_device_ops vfio_ap_matrix_dev_ops; -static int vfio_ap_mdev_reset_queue(struct vfio_ap_queue *q); +static void vfio_ap_mdev_reset_queue(struct vfio_ap_queue *q); /** * get_update_locks_for_kvm: Acquire the locks required to dynamically update a @@ -1623,11 +1623,13 @@ static int apq_status_check(int apqn, struct ap_queue_status *status) #define WAIT_MSG "Waited %dms for reset of queue %02x.%04x (%u, %u, %u)" -static int apq_reset_check(struct vfio_ap_queue *q) +static void apq_reset_check(struct work_struct *reset_work) { int ret = -EBUSY, elapsed = 0; struct ap_queue_status status; + struct vfio_ap_queue *q; + q = container_of(reset_work, struct vfio_ap_queue, reset_work); memcpy(&status, &q->reset_status, sizeof(status)); while (true) { msleep(AP_RESET_INTERVAL); @@ -1635,7 +1637,7 @@ static int apq_reset_check(struct vfio_ap_queue *q) status = ap_tapq(q->apqn, NULL); ret = apq_status_check(q->apqn, &status); if (ret == -EIO) - return ret; + return; if (ret == -EBUSY) { pr_notice_ratelimited(WAIT_MSG, elapsed, AP_QID_CARD(q->apqn), @@ -1663,34 +1665,32 @@ static int apq_reset_check(struct vfio_ap_queue *q) break; } } - return ret; } -static int vfio_ap_mdev_reset_queue(struct vfio_ap_queue *q) +static void vfio_ap_mdev_reset_queue(struct vfio_ap_queue *q) { struct ap_queue_status status; - int ret = 0; if (!q) - return 0; + return; status = ap_zapq(q->apqn, 0); memcpy(&q->reset_status, &status, sizeof(status)); switch (status.response_code) { case AP_RESPONSE_NORMAL: case AP_RESPONSE_RESET_IN_PROGRESS: case AP_RESPONSE_BUSY: - /* Let's verify whether the ZAPQ completed successfully */ - ret = apq_reset_check(q); + /* + * Let's verify whether the ZAPQ completed successfully on a work queue. + */ + queue_work(system_long_wq, &q->reset_work); break; case AP_RESPONSE_DECONFIGURED: /* * When an AP adapter is deconfigured, the associated * queues are reset, so let's set the status response code to 0 - * so the queue may be passed through (i.e., not filtered) and - * return a value indicating the reset completed successfully. + * so the queue may be passed through (i.e., not filtered). */ q->reset_status.response_code = 0; - ret = 0; vfio_ap_free_aqic_resources(q); break; default: @@ -1698,29 +1698,25 @@ static int vfio_ap_mdev_reset_queue(struct vfio_ap_queue *q) "PQAP/ZAPQ for %02x.%04x failed with invalid rc=%u\n", AP_QID_CARD(q->apqn), AP_QID_QUEUE(q->apqn), status.response_code); - return -EIO; } - - return ret; } static int vfio_ap_mdev_reset_queues(struct ap_queue_table *qtable) { - int ret, loop_cursor, rc = 0; + int ret = 0, loop_cursor; struct vfio_ap_queue *q; + hash_for_each(qtable->queues, loop_cursor, q, mdev_qnode) + vfio_ap_mdev_reset_queue(q); + hash_for_each(qtable->queues, loop_cursor, q, mdev_qnode) { - ret = vfio_ap_mdev_reset_queue(q); - /* - * Regardless whether a queue turns out to be busy, or - * is not operational, we need to continue resetting - * the remaining queues. - */ - if (ret) - rc = ret; + flush_work(&q->reset_work); + + if (q->reset_status.response_code) + ret = -EIO; } - return rc; + return ret; } static int vfio_ap_mdev_open_device(struct vfio_device *vdev) @@ -2045,6 +2041,7 @@ int vfio_ap_mdev_probe_queue(struct ap_device *apdev) q->apqn = to_ap_queue(&apdev->device)->qid; q->saved_isc = VFIO_AP_ISC_INVALID; memset(&q->reset_status, 0, sizeof(q->reset_status)); + INIT_WORK(&q->reset_work, apq_reset_check); matrix_mdev = get_update_locks_by_apqn(q->apqn); if (matrix_mdev) { @@ -2094,6 +2091,7 @@ void vfio_ap_mdev_remove_queue(struct ap_device *apdev) } vfio_ap_mdev_reset_queue(q); + flush_work(&q->reset_work); dev_set_drvdata(&apdev->device, NULL); kfree(q); release_update_locks_for_mdev(matrix_mdev); diff --git a/drivers/s390/crypto/vfio_ap_private.h b/drivers/s390/crypto/vfio_ap_private.h index d6eb3527e056..88aff8b81f2f 100644 --- a/drivers/s390/crypto/vfio_ap_private.h +++ b/drivers/s390/crypto/vfio_ap_private.h @@ -134,6 +134,7 @@ struct ap_matrix_mdev { * @saved_isc: the guest ISC registered with the GIB interface * @mdev_qnode: allows the vfio_ap_queue struct to be added to a hashtable * @reset_status: the status from the last reset of the queue + * @reset_work: work to wait for queue reset to complete */ struct vfio_ap_queue { struct ap_matrix_mdev *matrix_mdev; @@ -143,6 +144,7 @@ struct vfio_ap_queue { unsigned char saved_isc; struct hlist_node mdev_qnode; struct ap_queue_status reset_status; + struct work_struct reset_work; }; int vfio_ap_mdev_register(void); -- cgit v1.2.3-58-ga151 From e1f17f8ea93d8fc9d6d0562d38bb0a5fb3e8355e Mon Sep 17 00:00:00 2001 From: Tony Krowiak Date: Tue, 15 Aug 2023 14:43:29 -0400 Subject: s390/vfio-ap: handle queue state change in progress on reset A new APQSW response code (0xA) indicating the designated queue is in the process of being bound or associated to a configuration may be returned from the PQAP(ZAPQ) command. This patch introduces code that will verify when the PQAP(ZAPQ) command can be re-issued after receiving response code 0xA. Signed-off-by: Tony Krowiak Reviewed-by: Jason J. Herne Acked-by: Halil Pasic Tested-by: Viktor Mihajlovski Link: https://lore.kernel.org/r/20230815184333.6554-9-akrowiak@linux.ibm.com Signed-off-by: Heiko Carstens --- drivers/s390/crypto/vfio_ap_ops.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/drivers/s390/crypto/vfio_ap_ops.c b/drivers/s390/crypto/vfio_ap_ops.c index 3a59f1c5390f..43dea259fe23 100644 --- a/drivers/s390/crypto/vfio_ap_ops.c +++ b/drivers/s390/crypto/vfio_ap_ops.c @@ -1647,7 +1647,8 @@ static void apq_reset_check(struct work_struct *reset_work) status.irq_enabled); } else { if (q->reset_status.response_code == AP_RESPONSE_RESET_IN_PROGRESS || - q->reset_status.response_code == AP_RESPONSE_BUSY) { + q->reset_status.response_code == AP_RESPONSE_BUSY || + q->reset_status.response_code == AP_RESPONSE_STATE_CHANGE_IN_PROGRESS) { status = ap_zapq(q->apqn, 0); memcpy(&q->reset_status, &status, sizeof(status)); continue; @@ -1679,6 +1680,7 @@ static void vfio_ap_mdev_reset_queue(struct vfio_ap_queue *q) case AP_RESPONSE_NORMAL: case AP_RESPONSE_RESET_IN_PROGRESS: case AP_RESPONSE_BUSY: + case AP_RESPONSE_STATE_CHANGE_IN_PROGRESS: /* * Let's verify whether the ZAPQ completed successfully on a work queue. */ -- cgit v1.2.3-58-ga151 From 7847a19b5b6265f11e71c8499a3b608edac7f398 Mon Sep 17 00:00:00 2001 From: Tony Krowiak Date: Tue, 15 Aug 2023 14:43:30 -0400 Subject: s390/vfio-ap: check for TAPQ response codes 0x35 and 0x36 Check for response codes 0x35 and 0x36 which are asynchronous return codes indicating a failure of the guest to associate a secret with a queue. Since there can be no interaction with this queue from the guest (i.e., the vcpus are out of SIE for hot unplug, the guest is being shut down or an emulated subsystem reset of the guest is taking place), let's go ahead and re-issue the ZAPQ to reset and zeroize the queue. Signed-off-by: Tony Krowiak Reviewed-by: Jason J. Herne Reviewed-by: Halil Pasic Tested-by: Viktor Mihajlovski Link: https://lore.kernel.org/r/20230815184333.6554-10-akrowiak@linux.ibm.com Signed-off-by: Heiko Carstens --- drivers/s390/crypto/vfio_ap_ops.c | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/drivers/s390/crypto/vfio_ap_ops.c b/drivers/s390/crypto/vfio_ap_ops.c index 43dea259fe23..8bda52c46df0 100644 --- a/drivers/s390/crypto/vfio_ap_ops.c +++ b/drivers/s390/crypto/vfio_ap_ops.c @@ -1612,6 +1612,16 @@ static int apq_status_check(int apqn, struct ap_queue_status *status) case AP_RESPONSE_RESET_IN_PROGRESS: case AP_RESPONSE_BUSY: return -EBUSY; + case AP_RESPONSE_ASSOC_SECRET_NOT_UNIQUE: + case AP_RESPONSE_ASSOC_FAILED: + /* + * These asynchronous response codes indicate a PQAP(AAPQ) + * instruction to associate a secret with the guest failed. All + * subsequent AP instructions will end with the asynchronous + * response code until the AP queue is reset; so, let's return + * a value indicating a reset needs to be performed again. + */ + return -EAGAIN; default: WARN(true, "failed to verify reset of queue %02x.%04x: TAPQ rc=%u\n", @@ -1648,7 +1658,8 @@ static void apq_reset_check(struct work_struct *reset_work) } else { if (q->reset_status.response_code == AP_RESPONSE_RESET_IN_PROGRESS || q->reset_status.response_code == AP_RESPONSE_BUSY || - q->reset_status.response_code == AP_RESPONSE_STATE_CHANGE_IN_PROGRESS) { + q->reset_status.response_code == AP_RESPONSE_STATE_CHANGE_IN_PROGRESS || + ret == -EAGAIN) { status = ap_zapq(q->apqn, 0); memcpy(&q->reset_status, &status, sizeof(status)); continue; -- cgit v1.2.3-58-ga151 From cf3fa16a6fd49216ae83502e61bea0d8322b51eb Mon Sep 17 00:00:00 2001 From: Janosch Frank Date: Tue, 15 Aug 2023 14:43:31 -0400 Subject: s390/uv: export uv_pin_shared for direct usage Export the uv_pin_shared function so that it can be called from other modules that carry a GPL-compatible license. Signed-off-by: Janosch Frank Signed-off-by: Tony Krowiak Tested-by: Viktor Mihajlovski Link: https://lore.kernel.org/r/20230815184333.6554-11-akrowiak@linux.ibm.com Signed-off-by: Heiko Carstens --- arch/s390/include/asm/uv.h | 6 ++++++ arch/s390/kernel/uv.c | 3 ++- 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/arch/s390/include/asm/uv.h b/arch/s390/include/asm/uv.h index d6bb2f4f78d1..d2cd42bb2c26 100644 --- a/arch/s390/include/asm/uv.h +++ b/arch/s390/include/asm/uv.h @@ -463,6 +463,7 @@ static inline int is_prot_virt_host(void) return prot_virt_host; } +int uv_pin_shared(unsigned long paddr); int gmap_make_secure(struct gmap *gmap, unsigned long gaddr, void *uvcb); int gmap_destroy_page(struct gmap *gmap, unsigned long gaddr); int uv_destroy_owned_page(unsigned long paddr); @@ -475,6 +476,11 @@ void setup_uv(void); #define is_prot_virt_host() 0 static inline void setup_uv(void) {} +static inline int uv_pin_shared(unsigned long paddr) +{ + return 0; +} + static inline int uv_destroy_owned_page(unsigned long paddr) { return 0; diff --git a/arch/s390/kernel/uv.c b/arch/s390/kernel/uv.c index 66f0eb1c872b..b771f1b4cdd1 100644 --- a/arch/s390/kernel/uv.c +++ b/arch/s390/kernel/uv.c @@ -88,7 +88,7 @@ fail: * Requests the Ultravisor to pin the page in the shared state. This will * cause an intercept when the guest attempts to unshare the pinned page. */ -static int uv_pin_shared(unsigned long paddr) +int uv_pin_shared(unsigned long paddr) { struct uv_cb_cfs uvcb = { .header.cmd = UVC_CMD_PIN_PAGE_SHARED, @@ -100,6 +100,7 @@ static int uv_pin_shared(unsigned long paddr) return -EINVAL; return 0; } +EXPORT_SYMBOL_GPL(uv_pin_shared); /* * Requests the Ultravisor to destroy a guest page and make it -- cgit v1.2.3-58-ga151 From fb5040ef7f707525d0681cf6bfe424ccd1aadab7 Mon Sep 17 00:00:00 2001 From: Tony Krowiak Date: Tue, 15 Aug 2023 14:43:32 -0400 Subject: KVM: s390: export kvm_s390_pv*_is_protected functions Export the kvm_s390_pv_is_protected and kvm_s390_pv_cpu_is_protected functions so that they can be called from other modules that carry a GPL-compatible license. Signed-off-by: Janosch Frank Signed-off-by: Tony Krowiak Tested-by: Viktor Mihajlovski Link: https://lore.kernel.org/r/20230815184333.6554-12-akrowiak@linux.ibm.com Signed-off-by: Heiko Carstens --- arch/s390/include/asm/kvm_host.h | 3 +++ arch/s390/kvm/kvm-s390.h | 12 ------------ arch/s390/kvm/pv.c | 14 ++++++++++++++ 3 files changed, 17 insertions(+), 12 deletions(-) diff --git a/arch/s390/include/asm/kvm_host.h b/arch/s390/include/asm/kvm_host.h index 2bbc3d54959d..91bfecb91321 100644 --- a/arch/s390/include/asm/kvm_host.h +++ b/arch/s390/include/asm/kvm_host.h @@ -1028,6 +1028,9 @@ static inline int sie64a(struct kvm_s390_sie_block *sie_block, u64 *rsa) extern char sie_exit; +bool kvm_s390_pv_is_protected(struct kvm *kvm); +bool kvm_s390_pv_cpu_is_protected(struct kvm_vcpu *vcpu); + extern int kvm_s390_gisc_register(struct kvm *kvm, u32 gisc); extern int kvm_s390_gisc_unregister(struct kvm *kvm, u32 gisc); diff --git a/arch/s390/kvm/kvm-s390.h b/arch/s390/kvm/kvm-s390.h index 0261d42c7d01..a7ea80cfa445 100644 --- a/arch/s390/kvm/kvm-s390.h +++ b/arch/s390/kvm/kvm-s390.h @@ -270,18 +270,6 @@ static inline u64 kvm_s390_pv_cpu_get_handle(struct kvm_vcpu *vcpu) return vcpu->arch.pv.handle; } -static inline bool kvm_s390_pv_is_protected(struct kvm *kvm) -{ - lockdep_assert_held(&kvm->lock); - return !!kvm_s390_pv_get_handle(kvm); -} - -static inline bool kvm_s390_pv_cpu_is_protected(struct kvm_vcpu *vcpu) -{ - lockdep_assert_held(&vcpu->mutex); - return !!kvm_s390_pv_cpu_get_handle(vcpu); -} - /* implemented in interrupt.c */ int kvm_s390_handle_wait(struct kvm_vcpu *vcpu); void kvm_s390_vcpu_wakeup(struct kvm_vcpu *vcpu); diff --git a/arch/s390/kvm/pv.c b/arch/s390/kvm/pv.c index 2f34c7c3c5ab..856140e9942e 100644 --- a/arch/s390/kvm/pv.c +++ b/arch/s390/kvm/pv.c @@ -18,6 +18,20 @@ #include #include "kvm-s390.h" +bool kvm_s390_pv_is_protected(struct kvm *kvm) +{ + lockdep_assert_held(&kvm->lock); + return !!kvm_s390_pv_get_handle(kvm); +} +EXPORT_SYMBOL_GPL(kvm_s390_pv_is_protected); + +bool kvm_s390_pv_cpu_is_protected(struct kvm_vcpu *vcpu) +{ + lockdep_assert_held(&vcpu->mutex); + return !!kvm_s390_pv_cpu_get_handle(vcpu); +} +EXPORT_SYMBOL_GPL(kvm_s390_pv_cpu_is_protected); + /** * struct pv_vm_to_be_destroyed - Represents a protected VM that needs to * be destroyed -- cgit v1.2.3-58-ga151 From f88fb1335733029b4630fb93cfaad349a81e57b2 Mon Sep 17 00:00:00 2001 From: Tony Krowiak Date: Tue, 15 Aug 2023 14:43:33 -0400 Subject: s390/vfio-ap: make sure nib is shared Since the NIB is visible by HW, KVM and the (PV) guest it needs to be in non-secure or secure but shared storage. Return code 6 is used to indicate to a PV guest that its NIB would be on secure, unshared storage and therefore the NIB address is invalid. Unfortunately we have no easy way to check if a page is unshared after vfio_pin_pages() since it will automatically export an unshared page if the UV pin shared call did not succeed due to a page being in unshared state. Therefore we use the fact that UV pinning it a second time is a nop but trying to pin an exported page is an error (0x102). If we encounter this error, we do a vfio unpin and import the page again, since vfio_pin_pages() exported it. Signed-off-by: Janosch Frank Signed-off-by: Tony Krowiak Acked-by: Halil Pasic Tested-by: Viktor Mihajlovski Link: https://lore.kernel.org/r/20230815184333.6554-13-akrowiak@linux.ibm.com Signed-off-by: Heiko Carstens --- drivers/s390/crypto/vfio_ap_ops.c | 30 ++++++++++++++++++++++++++++++ 1 file changed, 30 insertions(+) diff --git a/drivers/s390/crypto/vfio_ap_ops.c b/drivers/s390/crypto/vfio_ap_ops.c index 8bda52c46df0..0509f80622cd 100644 --- a/drivers/s390/crypto/vfio_ap_ops.c +++ b/drivers/s390/crypto/vfio_ap_ops.c @@ -359,6 +359,28 @@ static int vfio_ap_validate_nib(struct kvm_vcpu *vcpu, dma_addr_t *nib) return 0; } +static int ensure_nib_shared(unsigned long addr, struct gmap *gmap) +{ + int ret; + + /* + * The nib has to be located in shared storage since guest and + * host access it. vfio_pin_pages() will do a pin shared and + * if that fails (possibly because it's not a shared page) it + * calls export. We try to do a second pin shared here so that + * the UV gives us an error code if we try to pin a non-shared + * page. + * + * If the page is already pinned shared the UV will return a success. + */ + ret = uv_pin_shared(addr); + if (ret) { + /* vfio_pin_pages() likely exported the page so let's re-import */ + gmap_convert_to_secure(gmap, addr); + } + return ret; +} + /** * vfio_ap_irq_enable - Enable Interruption for a APQN * @@ -422,6 +444,14 @@ static struct ap_queue_status vfio_ap_irq_enable(struct vfio_ap_queue *q, h_nib = page_to_phys(h_page) | (nib & ~PAGE_MASK); aqic_gisa.gisc = isc; + /* NIB in non-shared storage is a rc 6 for PV guests */ + if (kvm_s390_pv_cpu_is_protected(vcpu) && + ensure_nib_shared(h_nib & PAGE_MASK, kvm->arch.gmap)) { + vfio_unpin_pages(&q->matrix_mdev->vdev, nib, 1); + status.response_code = AP_RESPONSE_INVALID_ADDRESS; + return status; + } + nisc = kvm_s390_gisc_register(kvm, isc); if (nisc < 0) { VFIO_AP_DBF_WARN("%s: gisc registration failed: nisc=%d, isc=%d, apqn=%#04x\n", -- cgit v1.2.3-58-ga151