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 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 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 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 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 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 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 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 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 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 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 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 From 16631c42e6ff4f5acf30998fda6c2ea09d1adbfe Mon Sep 17 00:00:00 2001 From: Ilya Leoshkevich Date: Tue, 25 Jul 2023 16:37:16 +0200 Subject: KVM: s390: interrupt: Fix single-stepping into interrupt handlers After single-stepping an instruction that generates an interrupt, GDB ends up on the second instruction of the respective interrupt handler. The reason is that vcpu_pre_run() manually delivers the interrupt, and then __vcpu_run() runs the first handler instruction using the CPUSTAT_P flag. This causes a KVM_SINGLESTEP exit on the second handler instruction. Fix by delaying the KVM_SINGLESTEP exit until after the manual interrupt delivery. Acked-by: David Hildenbrand Reviewed-by: Claudio Imbrenda Signed-off-by: Ilya Leoshkevich Message-ID: <20230725143857.228626-2-iii@linux.ibm.com> Signed-off-by: Claudio Imbrenda Signed-off-by: Janosch Frank --- arch/s390/kvm/interrupt.c | 14 ++++++++++++++ arch/s390/kvm/kvm-s390.c | 4 ++-- 2 files changed, 16 insertions(+), 2 deletions(-) diff --git a/arch/s390/kvm/interrupt.c b/arch/s390/kvm/interrupt.c index 9bd0a873f3b1..85e39f472bb4 100644 --- a/arch/s390/kvm/interrupt.c +++ b/arch/s390/kvm/interrupt.c @@ -1392,6 +1392,7 @@ int __must_check kvm_s390_deliver_pending_interrupts(struct kvm_vcpu *vcpu) { struct kvm_s390_local_interrupt *li = &vcpu->arch.local_int; int rc = 0; + bool delivered = false; unsigned long irq_type; unsigned long irqs; @@ -1465,6 +1466,19 @@ int __must_check kvm_s390_deliver_pending_interrupts(struct kvm_vcpu *vcpu) WARN_ONCE(1, "Unknown pending irq type %ld", irq_type); clear_bit(irq_type, &li->pending_irqs); } + delivered |= !rc; + } + + /* + * We delivered at least one interrupt and modified the PC. Force a + * singlestep event now. + */ + if (delivered && guestdbg_sstep_enabled(vcpu)) { + struct kvm_debug_exit_arch *debug_exit = &vcpu->run->debug.arch; + + debug_exit->addr = vcpu->arch.sie_block->gpsw.addr; + debug_exit->type = KVM_SINGLESTEP; + vcpu->guest_debug |= KVM_GUESTDBG_EXIT_PENDING; } set_intercept_indicators(vcpu); diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c index d1e768bcfe1d..0c6333b108ba 100644 --- a/arch/s390/kvm/kvm-s390.c +++ b/arch/s390/kvm/kvm-s390.c @@ -4611,7 +4611,7 @@ static int vcpu_pre_run(struct kvm_vcpu *vcpu) if (!kvm_is_ucontrol(vcpu->kvm)) { rc = kvm_s390_deliver_pending_interrupts(vcpu); - if (rc) + if (rc || guestdbg_exit_pending(vcpu)) return rc; } @@ -4738,7 +4738,7 @@ static int __vcpu_run(struct kvm_vcpu *vcpu) do { rc = vcpu_pre_run(vcpu); - if (rc) + if (rc || guestdbg_exit_pending(vcpu)) break; kvm_vcpu_srcu_read_unlock(vcpu); -- cgit v1.2.3 From 74a439ef7b67d89d29ec7485c3aeca20a64449c5 Mon Sep 17 00:00:00 2001 From: Ilya Leoshkevich Date: Tue, 25 Jul 2023 16:37:17 +0200 Subject: KVM: s390: interrupt: Fix single-stepping into program interrupt handlers Currently, after single-stepping an instruction that generates a specification exception, GDB ends up on the instruction immediately following it. The reason is that vcpu_post_run() injects the interrupt and sets KVM_GUESTDBG_EXIT_PENDING, causing a KVM_SINGLESTEP exit. The interrupt is not delivered, however, therefore userspace sees the address of the next instruction. Fix by letting the __vcpu_run() loop go into the next iteration, where vcpu_pre_run() delivers the interrupt and sets KVM_GUESTDBG_EXIT_PENDING. Reviewed-by: David Hildenbrand Signed-off-by: Ilya Leoshkevich Reviewed-by: Claudio Imbrenda Message-ID: <20230725143857.228626-3-iii@linux.ibm.com> Signed-off-by: Claudio Imbrenda Signed-off-by: Janosch Frank --- arch/s390/kvm/intercept.c | 17 ++++++++++++++++- 1 file changed, 16 insertions(+), 1 deletion(-) diff --git a/arch/s390/kvm/intercept.c b/arch/s390/kvm/intercept.c index 954d39adf85c..e54496740859 100644 --- a/arch/s390/kvm/intercept.c +++ b/arch/s390/kvm/intercept.c @@ -228,6 +228,21 @@ static int handle_itdb(struct kvm_vcpu *vcpu) #define per_event(vcpu) (vcpu->arch.sie_block->iprcc & PGM_PER) +static bool should_handle_per_event(const struct kvm_vcpu *vcpu) +{ + if (!guestdbg_enabled(vcpu) || !per_event(vcpu)) + return false; + if (guestdbg_sstep_enabled(vcpu) && + vcpu->arch.sie_block->iprcc != PGM_PER) { + /* + * __vcpu_run() will exit after delivering the concurrently + * indicated condition. + */ + return false; + } + return true; +} + static int handle_prog(struct kvm_vcpu *vcpu) { psw_t psw; @@ -242,7 +257,7 @@ static int handle_prog(struct kvm_vcpu *vcpu) if (kvm_s390_pv_cpu_is_protected(vcpu)) return -EOPNOTSUPP; - if (guestdbg_enabled(vcpu) && per_event(vcpu)) { + if (should_handle_per_event(vcpu)) { rc = kvm_s390_handle_per_event(vcpu); if (rc) return rc; -- cgit v1.2.3 From ba853a4e1c7addc631df55535bf0b04c62dc79d8 Mon Sep 17 00:00:00 2001 From: Ilya Leoshkevich Date: Tue, 25 Jul 2023 16:37:18 +0200 Subject: KVM: s390: interrupt: Fix single-stepping kernel-emulated instructions Single-stepping a kernel-emulated instruction that generates an interrupt causes GDB to land on the instruction following it instead of the respective interrupt handler. The reason is that kvm_handle_sie_intercept(), after injecting the interrupt, also processes the PER event and arranges a KVM_SINGLESTEP exit. The interrupt is not yet delivered, however, so the userspace sees the next instruction. Fix by avoiding the KVM_SINGLESTEP exit when there is a pending interrupt. The next __vcpu_run() loop iteration will arrange a KVM_SINGLESTEP exit after delivering the interrupt. Reviewed-by: David Hildenbrand Reviewed-by: Claudio Imbrenda Signed-off-by: Ilya Leoshkevich Message-ID: <20230725143857.228626-4-iii@linux.ibm.com> Signed-off-by: Claudio Imbrenda Signed-off-by: Janosch Frank --- arch/s390/kvm/intercept.c | 17 ++++++++++++++--- 1 file changed, 14 insertions(+), 3 deletions(-) diff --git a/arch/s390/kvm/intercept.c b/arch/s390/kvm/intercept.c index e54496740859..db222c749e5e 100644 --- a/arch/s390/kvm/intercept.c +++ b/arch/s390/kvm/intercept.c @@ -583,6 +583,19 @@ static int handle_pv_notification(struct kvm_vcpu *vcpu) return handle_instruction(vcpu); } +static bool should_handle_per_ifetch(const struct kvm_vcpu *vcpu, int rc) +{ + /* Process PER, also if the instruction is processed in user space. */ + if (!(vcpu->arch.sie_block->icptstatus & 0x02)) + return false; + if (rc != 0 && rc != -EOPNOTSUPP) + return false; + if (guestdbg_sstep_enabled(vcpu) && vcpu->arch.local_int.pending_irqs) + /* __vcpu_run() will exit after delivering the interrupt. */ + return false; + return true; +} + int kvm_handle_sie_intercept(struct kvm_vcpu *vcpu) { int rc, per_rc = 0; @@ -645,9 +658,7 @@ int kvm_handle_sie_intercept(struct kvm_vcpu *vcpu) return -EOPNOTSUPP; } - /* process PER, also if the instruction is processed in user space */ - if (vcpu->arch.sie_block->icptstatus & 0x02 && - (!rc || rc == -EOPNOTSUPP)) + if (should_handle_per_ifetch(vcpu, rc)) per_rc = kvm_s390_handle_per_ifetch_icpt(vcpu); return per_rc ? per_rc : rc; } -- cgit v1.2.3 From 1ad1fa820e6424ae75d3d9f59774e40c9c7ec1e5 Mon Sep 17 00:00:00 2001 From: Ilya Leoshkevich Date: Tue, 25 Jul 2023 16:37:19 +0200 Subject: KVM: s390: interrupt: Fix single-stepping userspace-emulated instructions Single-stepping a userspace-emulated instruction that generates an interrupt causes GDB to land on the instruction following it instead of the respective interrupt handler. The reason is that after arranging a KVM_EXIT_S390_SIEIC exit, kvm_handle_sie_intercept() calls kvm_s390_handle_per_ifetch_icpt(), which sets KVM_GUESTDBG_EXIT_PENDING. This bit, however, is not processed immediately, but rather persists until the next ioctl(), causing a spurious single-step exit. Fix by clearing this bit in ioctl(). Reviewed-by: David Hildenbrand Reviewed-by: Claudio Imbrenda Signed-off-by: Ilya Leoshkevich Message-ID: <20230725143857.228626-5-iii@linux.ibm.com> Signed-off-by: Claudio Imbrenda Signed-off-by: Janosch Frank --- arch/s390/kvm/kvm-s390.c | 23 ++++++++++++++++++++--- 1 file changed, 20 insertions(+), 3 deletions(-) diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c index 0c6333b108ba..e6511608280c 100644 --- a/arch/s390/kvm/kvm-s390.c +++ b/arch/s390/kvm/kvm-s390.c @@ -5383,6 +5383,7 @@ long kvm_arch_vcpu_async_ioctl(struct file *filp, { struct kvm_vcpu *vcpu = filp->private_data; void __user *argp = (void __user *)arg; + int rc; switch (ioctl) { case KVM_S390_IRQ: { @@ -5390,7 +5391,8 @@ long kvm_arch_vcpu_async_ioctl(struct file *filp, if (copy_from_user(&s390irq, argp, sizeof(s390irq))) return -EFAULT; - return kvm_s390_inject_vcpu(vcpu, &s390irq); + rc = kvm_s390_inject_vcpu(vcpu, &s390irq); + break; } case KVM_S390_INTERRUPT: { struct kvm_s390_interrupt s390int; @@ -5400,10 +5402,25 @@ long kvm_arch_vcpu_async_ioctl(struct file *filp, return -EFAULT; if (s390int_to_s390irq(&s390int, &s390irq)) return -EINVAL; - return kvm_s390_inject_vcpu(vcpu, &s390irq); + rc = kvm_s390_inject_vcpu(vcpu, &s390irq); + break; } + default: + rc = -ENOIOCTLCMD; + break; } - return -ENOIOCTLCMD; + + /* + * To simplify single stepping of userspace-emulated instructions, + * KVM_EXIT_S390_SIEIC exit sets KVM_GUESTDBG_EXIT_PENDING (see + * should_handle_per_ifetch()). However, if userspace emulation injects + * an interrupt, it needs to be cleared, so that KVM_EXIT_DEBUG happens + * after (and not before) the interrupt delivery. + */ + if (!rc) + vcpu->guest_debug &= ~KVM_GUESTDBG_EXIT_PENDING; + + return rc; } static int kvm_s390_handle_pv_vcpu_dump(struct kvm_vcpu *vcpu, -- cgit v1.2.3 From fdbeb55ebdf1d83e610303f7e5c50a4d6904b7fa Mon Sep 17 00:00:00 2001 From: Ilya Leoshkevich Date: Tue, 25 Jul 2023 16:37:20 +0200 Subject: KVM: s390: interrupt: Fix single-stepping keyless mode exits kvm_s390_skey_check_enable() does not emulate any instructions, rather, it clears CPUSTAT_KSS and arranges the instruction that caused the exit (e.g., ISKE, SSKE, RRBE or LPSWE with a keyed PSW) to run again. Therefore, skip the PER check and let the instruction execution happen. Otherwise, a debugger will see two single-step events on the same instruction. Reviewed-by: Christian Borntraeger Reviewed-by: David Hildenbrand Signed-off-by: Ilya Leoshkevich Message-ID: <20230725143857.228626-6-iii@linux.ibm.com> Signed-off-by: Claudio Imbrenda Signed-off-by: Janosch Frank --- arch/s390/kvm/intercept.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/arch/s390/kvm/intercept.c b/arch/s390/kvm/intercept.c index db222c749e5e..9f64f27f086e 100644 --- a/arch/s390/kvm/intercept.c +++ b/arch/s390/kvm/intercept.c @@ -630,8 +630,8 @@ int kvm_handle_sie_intercept(struct kvm_vcpu *vcpu) rc = handle_partial_execution(vcpu); break; case ICPT_KSS: - rc = kvm_s390_skey_check_enable(vcpu); - break; + /* Instruction will be redriven, skip the PER check. */ + return kvm_s390_skey_check_enable(vcpu); case ICPT_MCHKREQ: case ICPT_INT_ENABLE: /* -- cgit v1.2.3 From 642dbc0312d67781dabf97a70b43810165f21527 Mon Sep 17 00:00:00 2001 From: Ilya Leoshkevich Date: Tue, 25 Jul 2023 16:37:21 +0200 Subject: KVM: s390: selftests: Add selftest for single-stepping Test different variations of single-stepping into interrupts: - SVC and PGM interrupts; - Interrupts generated by ISKE; - Interrupts generated by instructions emulated by KVM; - Interrupts generated by instructions emulated by userspace. Reviewed-by: Claudio Imbrenda Signed-off-by: Ilya Leoshkevich Message-ID: <20230725143857.228626-7-iii@linux.ibm.com> Signed-off-by: Claudio Imbrenda [frankja@de.igm.com: s/ASSERT_EQ/TEST_ASSERT_EQ/ because function was renamed in the selftest printf series] Signed-off-by: Janosch Frank --- tools/testing/selftests/kvm/Makefile | 1 + tools/testing/selftests/kvm/s390x/debug_test.c | 160 +++++++++++++++++++++++++ 2 files changed, 161 insertions(+) create mode 100644 tools/testing/selftests/kvm/s390x/debug_test.c diff --git a/tools/testing/selftests/kvm/Makefile b/tools/testing/selftests/kvm/Makefile index 77026907968f..6092ccfc49ac 100644 --- a/tools/testing/selftests/kvm/Makefile +++ b/tools/testing/selftests/kvm/Makefile @@ -169,6 +169,7 @@ TEST_GEN_PROGS_s390x += s390x/resets TEST_GEN_PROGS_s390x += s390x/sync_regs_test TEST_GEN_PROGS_s390x += s390x/tprot TEST_GEN_PROGS_s390x += s390x/cmma_test +TEST_GEN_PROGS_s390x += s390x/debug_test TEST_GEN_PROGS_s390x += demand_paging_test TEST_GEN_PROGS_s390x += dirty_log_test TEST_GEN_PROGS_s390x += guest_print_test diff --git a/tools/testing/selftests/kvm/s390x/debug_test.c b/tools/testing/selftests/kvm/s390x/debug_test.c new file mode 100644 index 000000000000..84313fb27529 --- /dev/null +++ b/tools/testing/selftests/kvm/s390x/debug_test.c @@ -0,0 +1,160 @@ +// SPDX-License-Identifier: GPL-2.0-only +/* Test KVM debugging features. */ +#include "kvm_util.h" +#include "test_util.h" + +#include + +#define __LC_SVC_NEW_PSW 0x1c0 +#define __LC_PGM_NEW_PSW 0x1d0 +#define ICPT_INSTRUCTION 0x04 +#define IPA0_DIAG 0x8300 +#define PGM_SPECIFICATION 0x06 + +/* Common code for testing single-stepping interruptions. */ +extern char int_handler[]; +asm("int_handler:\n" + "j .\n"); + +static struct kvm_vm *test_step_int_1(struct kvm_vcpu **vcpu, void *guest_code, + size_t new_psw_off, uint64_t *new_psw) +{ + struct kvm_guest_debug debug = {}; + struct kvm_regs regs; + struct kvm_vm *vm; + char *lowcore; + + vm = vm_create_with_one_vcpu(vcpu, guest_code); + lowcore = addr_gpa2hva(vm, 0); + new_psw[0] = (*vcpu)->run->psw_mask; + new_psw[1] = (uint64_t)int_handler; + memcpy(lowcore + new_psw_off, new_psw, 16); + vcpu_regs_get(*vcpu, ®s); + regs.gprs[2] = -1; + vcpu_regs_set(*vcpu, ®s); + debug.control = KVM_GUESTDBG_ENABLE | KVM_GUESTDBG_SINGLESTEP; + vcpu_guest_debug_set(*vcpu, &debug); + vcpu_run(*vcpu); + + return vm; +} + +static void test_step_int(void *guest_code, size_t new_psw_off) +{ + struct kvm_vcpu *vcpu; + uint64_t new_psw[2]; + struct kvm_vm *vm; + + vm = test_step_int_1(&vcpu, guest_code, new_psw_off, new_psw); + TEST_ASSERT_KVM_EXIT_REASON(vcpu, KVM_EXIT_DEBUG); + TEST_ASSERT_EQ(vcpu->run->psw_mask, new_psw[0]); + TEST_ASSERT_EQ(vcpu->run->psw_addr, new_psw[1]); + kvm_vm_free(vm); +} + +/* Test single-stepping "boring" program interruptions. */ +extern char test_step_pgm_guest_code[]; +asm("test_step_pgm_guest_code:\n" + ".insn rr,0x1d00,%r1,%r0 /* dr %r1,%r0 */\n" + "j .\n"); + +static void test_step_pgm(void) +{ + test_step_int(test_step_pgm_guest_code, __LC_PGM_NEW_PSW); +} + +/* + * Test single-stepping program interruptions caused by DIAG. + * Userspace emulation must not interfere with single-stepping. + */ +extern char test_step_pgm_diag_guest_code[]; +asm("test_step_pgm_diag_guest_code:\n" + "diag %r0,%r0,0\n" + "j .\n"); + +static void test_step_pgm_diag(void) +{ + struct kvm_s390_irq irq = { + .type = KVM_S390_PROGRAM_INT, + .u.pgm.code = PGM_SPECIFICATION, + }; + struct kvm_vcpu *vcpu; + uint64_t new_psw[2]; + struct kvm_vm *vm; + + vm = test_step_int_1(&vcpu, test_step_pgm_diag_guest_code, + __LC_PGM_NEW_PSW, new_psw); + TEST_ASSERT_KVM_EXIT_REASON(vcpu, KVM_EXIT_S390_SIEIC); + TEST_ASSERT_EQ(vcpu->run->s390_sieic.icptcode, ICPT_INSTRUCTION); + TEST_ASSERT_EQ(vcpu->run->s390_sieic.ipa & 0xff00, IPA0_DIAG); + vcpu_ioctl(vcpu, KVM_S390_IRQ, &irq); + vcpu_run(vcpu); + TEST_ASSERT_KVM_EXIT_REASON(vcpu, KVM_EXIT_DEBUG); + TEST_ASSERT_EQ(vcpu->run->psw_mask, new_psw[0]); + TEST_ASSERT_EQ(vcpu->run->psw_addr, new_psw[1]); + kvm_vm_free(vm); +} + +/* + * Test single-stepping program interruptions caused by ISKE. + * CPUSTAT_KSS handling must not interfere with single-stepping. + */ +extern char test_step_pgm_iske_guest_code[]; +asm("test_step_pgm_iske_guest_code:\n" + "iske %r2,%r2\n" + "j .\n"); + +static void test_step_pgm_iske(void) +{ + test_step_int(test_step_pgm_iske_guest_code, __LC_PGM_NEW_PSW); +} + +/* + * Test single-stepping program interruptions caused by LCTL. + * KVM emulation must not interfere with single-stepping. + */ +extern char test_step_pgm_lctl_guest_code[]; +asm("test_step_pgm_lctl_guest_code:\n" + "lctl %c0,%c0,1\n" + "j .\n"); + +static void test_step_pgm_lctl(void) +{ + test_step_int(test_step_pgm_lctl_guest_code, __LC_PGM_NEW_PSW); +} + +/* Test single-stepping supervisor-call interruptions. */ +extern char test_step_svc_guest_code[]; +asm("test_step_svc_guest_code:\n" + "svc 0\n" + "j .\n"); + +static void test_step_svc(void) +{ + test_step_int(test_step_svc_guest_code, __LC_SVC_NEW_PSW); +} + +/* Run all tests above. */ +static struct testdef { + const char *name; + void (*test)(void); +} testlist[] = { + { "single-step pgm", test_step_pgm }, + { "single-step pgm caused by diag", test_step_pgm_diag }, + { "single-step pgm caused by iske", test_step_pgm_iske }, + { "single-step pgm caused by lctl", test_step_pgm_lctl }, + { "single-step svc", test_step_svc }, +}; + +int main(int argc, char *argv[]) +{ + int idx; + + ksft_print_header(); + ksft_set_plan(ARRAY_SIZE(testlist)); + for (idx = 0; idx < ARRAY_SIZE(testlist); idx++) { + testlist[idx].test(); + ksft_test_result_pass("%s\n", testlist[idx].name); + } + ksft_finished(); +} -- cgit v1.2.3 From b1e428615f154c87a94267b67e819644b50948f8 Mon Sep 17 00:00:00 2001 From: Viktor Mihajlovski Date: Tue, 15 Aug 2023 17:14:12 +0200 Subject: KVM: s390: pv: relax WARN_ONCE condition for destroy fast Destroy configuration fast may return with RC 0x104 if there are still bound APQNs in the configuration. The final cleanup will occur with the standard destroy configuration UVC as at this point in time all APQNs have been reset and thus unbound. Therefore, don't warn if RC 0x104 is reported. Signed-off-by: Viktor Mihajlovski Reviewed-by: Claudio Imbrenda Reviewed-by: Janosch Frank Reviewed-by: Steffen Eiden Reviewed-by: Michael Mueller Link: https://lore.kernel.org/r/20230815151415.379760-2-seiden@linux.ibm.com Signed-off-by: Janosch Frank Message-ID: <20230815151415.379760-2-seiden@linux.ibm.com> --- arch/s390/kvm/pv.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/arch/s390/kvm/pv.c b/arch/s390/kvm/pv.c index 856140e9942e..beeebc11b1a1 100644 --- a/arch/s390/kvm/pv.c +++ b/arch/s390/kvm/pv.c @@ -285,7 +285,8 @@ static int kvm_s390_pv_deinit_vm_fast(struct kvm *kvm, u16 *rc, u16 *rrc) WRITE_ONCE(kvm->arch.gmap->guest_handle, 0); KVM_UV_EVENT(kvm, 3, "PROTVIRT DESTROY VM FAST: rc %x rrc %x", uvcb.header.rc, uvcb.header.rrc); - WARN_ONCE(cc, "protvirt destroy vm fast failed handle %llx rc %x rrc %x", + WARN_ONCE(cc && uvcb.header.rc != 0x104, + "protvirt destroy vm fast failed handle %llx rc %x rrc %x", kvm_s390_pv_get_handle(kvm), uvcb.header.rc, uvcb.header.rrc); /* Intended memory leak on "impossible" error */ if (!cc) -- cgit v1.2.3 From 59a881402cc89788652fa2c2ce7421d14f131c34 Mon Sep 17 00:00:00 2001 From: Steffen Eiden Date: Tue, 15 Aug 2023 17:14:13 +0200 Subject: s390/uv: UV feature check utility Introduces a function to check the existence of an UV feature. Refactor feature bit checks to use the new function. Signed-off-by: Steffen Eiden Reviewed-by: Claudio Imbrenda Reviewed-by: Janosch Frank Signed-off-by: Janosch Frank Reviewed-by: Michael Mueller Link: https://lore.kernel.org/r/20230815151415.379760-3-seiden@linux.ibm.com Message-Id: <20230815151415.379760-3-seiden@linux.ibm.com> --- arch/s390/include/asm/uv.h | 7 +++++++ arch/s390/kernel/uv.c | 2 +- arch/s390/kvm/kvm-s390.c | 2 +- arch/s390/mm/fault.c | 2 +- 4 files changed, 10 insertions(+), 3 deletions(-) diff --git a/arch/s390/include/asm/uv.h b/arch/s390/include/asm/uv.h index d2cd42bb2c26..823adfff7315 100644 --- a/arch/s390/include/asm/uv.h +++ b/arch/s390/include/asm/uv.h @@ -397,6 +397,13 @@ struct uv_info { extern struct uv_info uv_info; +static inline bool uv_has_feature(u8 feature_bit) +{ + if (feature_bit >= sizeof(uv_info.uv_feature_indications) * 8) + return false; + return test_bit_inv(feature_bit, &uv_info.uv_feature_indications); +} + #ifdef CONFIG_PROTECTED_VIRTUALIZATION_GUEST extern int prot_virt_guest; diff --git a/arch/s390/kernel/uv.c b/arch/s390/kernel/uv.c index b771f1b4cdd1..fc07bc39e698 100644 --- a/arch/s390/kernel/uv.c +++ b/arch/s390/kernel/uv.c @@ -258,7 +258,7 @@ static bool should_export_before_import(struct uv_cb_header *uvcb, struct mm_str * shared page from a different protected VM will automatically also * transfer its ownership. */ - if (test_bit_inv(BIT_UV_FEAT_MISC, &uv_info.uv_feature_indications)) + if (uv_has_feature(BIT_UV_FEAT_MISC)) return false; if (uvcb->cmd == UVC_CMD_UNPIN_PAGE_SHARED) return false; diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c index e6511608280c..813cc3d59c90 100644 --- a/arch/s390/kvm/kvm-s390.c +++ b/arch/s390/kvm/kvm-s390.c @@ -2406,7 +2406,7 @@ static int kvm_s390_cpus_to_pv(struct kvm *kvm, u16 *rc, u16 *rrc) struct kvm_vcpu *vcpu; /* Disable the GISA if the ultravisor does not support AIV. */ - if (!test_bit_inv(BIT_UV_FEAT_AIV, &uv_info.uv_feature_indications)) + if (!uv_has_feature(BIT_UV_FEAT_AIV)) kvm_s390_gisa_disable(kvm); kvm_for_each_vcpu(i, vcpu, kvm) { diff --git a/arch/s390/mm/fault.c b/arch/s390/mm/fault.c index dbe8394234e2..390819d03215 100644 --- a/arch/s390/mm/fault.c +++ b/arch/s390/mm/fault.c @@ -825,7 +825,7 @@ void do_secure_storage_access(struct pt_regs *regs) * reliable without the misc UV feature so we need to check * for that as well. */ - if (test_bit_inv(BIT_UV_FEAT_MISC, &uv_info.uv_feature_indications) && + if (uv_has_feature(BIT_UV_FEAT_MISC) && !test_bit_inv(61, ®s->int_parm_long)) { /* * When this happens, userspace did something that it -- cgit v1.2.3 From 19c654bf05ae33ad0a2a9c039d5fe7d411a8bb06 Mon Sep 17 00:00:00 2001 From: Steffen Eiden Date: Tue, 15 Aug 2023 17:14:14 +0200 Subject: KVM: s390: Add UV feature negotiation Add a uv_feature list for pv-guests to the KVM cpu-model. The feature bits 'AP-interpretation for secure guests' and 'AP-interrupt for secure guests' are available. Signed-off-by: Steffen Eiden Reviewed-by: Janosch Frank Reviewed-by: Michael Mueller Signed-off-by: Janosch Frank Link: https://lore.kernel.org/r/20230815151415.379760-4-seiden@linux.ibm.com Message-Id: <20230815151415.379760-4-seiden@linux.ibm.com> --- arch/s390/include/asm/kvm_host.h | 2 ++ arch/s390/include/uapi/asm/kvm.h | 16 +++++++++ arch/s390/kvm/kvm-s390.c | 73 ++++++++++++++++++++++++++++++++++++++++ 3 files changed, 91 insertions(+) diff --git a/arch/s390/include/asm/kvm_host.h b/arch/s390/include/asm/kvm_host.h index 91bfecb91321..427f9528a7b6 100644 --- a/arch/s390/include/asm/kvm_host.h +++ b/arch/s390/include/asm/kvm_host.h @@ -817,6 +817,8 @@ struct kvm_s390_cpu_model { __u64 *fac_list; u64 cpuid; unsigned short ibc; + /* subset of available UV-features for pv-guests enabled by user space */ + struct kvm_s390_vm_cpu_uv_feat uv_feat_guest; }; typedef int (*crypto_hook)(struct kvm_vcpu *vcpu); diff --git a/arch/s390/include/uapi/asm/kvm.h b/arch/s390/include/uapi/asm/kvm.h index a73cf01a1606..abe926d43cbe 100644 --- a/arch/s390/include/uapi/asm/kvm.h +++ b/arch/s390/include/uapi/asm/kvm.h @@ -159,6 +159,22 @@ struct kvm_s390_vm_cpu_subfunc { __u8 reserved[1728]; }; +#define KVM_S390_VM_CPU_PROCESSOR_UV_FEAT_GUEST 6 +#define KVM_S390_VM_CPU_MACHINE_UV_FEAT_GUEST 7 + +#define KVM_S390_VM_CPU_UV_FEAT_NR_BITS 64 +struct kvm_s390_vm_cpu_uv_feat { + union { + struct { + __u64 : 4; + __u64 ap : 1; /* bit 4 */ + __u64 ap_intr : 1; /* bit 5 */ + __u64 : 58; + }; + __u64 feat; + }; +}; + /* kvm attributes for crypto */ #define KVM_S390_VM_CRYPTO_ENABLE_AES_KW 0 #define KVM_S390_VM_CRYPTO_ENABLE_DEA_KW 1 diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c index 813cc3d59c90..b3f17e014cab 100644 --- a/arch/s390/kvm/kvm-s390.c +++ b/arch/s390/kvm/kvm-s390.c @@ -1531,6 +1531,39 @@ static int kvm_s390_set_processor_subfunc(struct kvm *kvm, return 0; } +#define KVM_S390_VM_CPU_UV_FEAT_GUEST_MASK \ +( \ + ((struct kvm_s390_vm_cpu_uv_feat){ \ + .ap = 1, \ + .ap_intr = 1, \ + }) \ + .feat \ +) + +static int kvm_s390_set_uv_feat(struct kvm *kvm, struct kvm_device_attr *attr) +{ + struct kvm_s390_vm_cpu_uv_feat __user *ptr = (void __user *)attr->addr; + unsigned long data, filter; + + filter = uv_info.uv_feature_indications & KVM_S390_VM_CPU_UV_FEAT_GUEST_MASK; + if (get_user(data, &ptr->feat)) + return -EFAULT; + if (!bitmap_subset(&data, &filter, KVM_S390_VM_CPU_UV_FEAT_NR_BITS)) + return -EINVAL; + + mutex_lock(&kvm->lock); + if (kvm->created_vcpus) { + mutex_unlock(&kvm->lock); + return -EBUSY; + } + kvm->arch.model.uv_feat_guest.feat = data; + mutex_unlock(&kvm->lock); + + VM_EVENT(kvm, 3, "SET: guest UV-feat: 0x%16.16lx", data); + + return 0; +} + static int kvm_s390_set_cpu_model(struct kvm *kvm, struct kvm_device_attr *attr) { int ret = -ENXIO; @@ -1545,6 +1578,9 @@ static int kvm_s390_set_cpu_model(struct kvm *kvm, struct kvm_device_attr *attr) case KVM_S390_VM_CPU_PROCESSOR_SUBFUNC: ret = kvm_s390_set_processor_subfunc(kvm, attr); break; + case KVM_S390_VM_CPU_PROCESSOR_UV_FEAT_GUEST: + ret = kvm_s390_set_uv_feat(kvm, attr); + break; } return ret; } @@ -1777,6 +1813,33 @@ static int kvm_s390_get_machine_subfunc(struct kvm *kvm, return 0; } +static int kvm_s390_get_processor_uv_feat(struct kvm *kvm, struct kvm_device_attr *attr) +{ + struct kvm_s390_vm_cpu_uv_feat __user *dst = (void __user *)attr->addr; + unsigned long feat = kvm->arch.model.uv_feat_guest.feat; + + if (put_user(feat, &dst->feat)) + return -EFAULT; + VM_EVENT(kvm, 3, "GET: guest UV-feat: 0x%16.16lx", feat); + + return 0; +} + +static int kvm_s390_get_machine_uv_feat(struct kvm *kvm, struct kvm_device_attr *attr) +{ + struct kvm_s390_vm_cpu_uv_feat __user *dst = (void __user *)attr->addr; + unsigned long feat; + + BUILD_BUG_ON(sizeof(*dst) != sizeof(uv_info.uv_feature_indications)); + + feat = uv_info.uv_feature_indications & KVM_S390_VM_CPU_UV_FEAT_GUEST_MASK; + if (put_user(feat, &dst->feat)) + return -EFAULT; + VM_EVENT(kvm, 3, "GET: guest UV-feat: 0x%16.16lx", feat); + + return 0; +} + static int kvm_s390_get_cpu_model(struct kvm *kvm, struct kvm_device_attr *attr) { int ret = -ENXIO; @@ -1800,6 +1863,12 @@ static int kvm_s390_get_cpu_model(struct kvm *kvm, struct kvm_device_attr *attr) case KVM_S390_VM_CPU_MACHINE_SUBFUNC: ret = kvm_s390_get_machine_subfunc(kvm, attr); break; + case KVM_S390_VM_CPU_PROCESSOR_UV_FEAT_GUEST: + ret = kvm_s390_get_processor_uv_feat(kvm, attr); + break; + case KVM_S390_VM_CPU_MACHINE_UV_FEAT_GUEST: + ret = kvm_s390_get_machine_uv_feat(kvm, attr); + break; } return ret; } @@ -1952,6 +2021,8 @@ static int kvm_s390_vm_has_attr(struct kvm *kvm, struct kvm_device_attr *attr) case KVM_S390_VM_CPU_MACHINE_FEAT: case KVM_S390_VM_CPU_MACHINE_SUBFUNC: case KVM_S390_VM_CPU_PROCESSOR_SUBFUNC: + case KVM_S390_VM_CPU_MACHINE_UV_FEAT_GUEST: + case KVM_S390_VM_CPU_PROCESSOR_UV_FEAT_GUEST: ret = 0; break; default: @@ -3296,6 +3367,8 @@ int kvm_arch_init_vm(struct kvm *kvm, unsigned long type) kvm->arch.model.cpuid = kvm_s390_get_initial_cpuid(); kvm->arch.model.ibc = sclp.ibc & 0x0fff; + kvm->arch.model.uv_feat_guest.feat = 0; + kvm_s390_crypto_init(kvm); if (IS_ENABLED(CONFIG_VFIO_PCI_ZDEV_KVM)) { -- cgit v1.2.3 From 899e2206f46aece42d8194c350bc1de71344dbc7 Mon Sep 17 00:00:00 2001 From: Steffen Eiden Date: Tue, 15 Aug 2023 17:14:15 +0200 Subject: KVM: s390: pv: Allow AP-instructions for pv-guests Introduces new feature bits and enablement flags for AP and AP IRQ support. Signed-off-by: Steffen Eiden Reviewed-by: Janosch Frank Reviewed-by: Michael Mueller Signed-off-by: Janosch Frank Link: https://lore.kernel.org/r/20230815151415.379760-5-seiden@linux.ibm.com Message-Id: <20230815151415.379760-5-seiden@linux.ibm.com> --- arch/s390/include/asm/uv.h | 12 +++++++++++- arch/s390/kvm/pv.c | 6 ++++-- 2 files changed, 15 insertions(+), 3 deletions(-) diff --git a/arch/s390/include/asm/uv.h b/arch/s390/include/asm/uv.h index 823adfff7315..0e7bd3873907 100644 --- a/arch/s390/include/asm/uv.h +++ b/arch/s390/include/asm/uv.h @@ -99,6 +99,8 @@ enum uv_cmds_inst { enum uv_feat_ind { BIT_UV_FEAT_MISC = 0, BIT_UV_FEAT_AIV = 1, + BIT_UV_FEAT_AP = 4, + BIT_UV_FEAT_AP_INTR = 5, }; struct uv_cb_header { @@ -159,7 +161,15 @@ struct uv_cb_cgc { u64 guest_handle; u64 conf_base_stor_origin; u64 conf_virt_stor_origin; - u64 reserved30; + u8 reserved30[6]; + union { + struct { + u16 : 14; + u16 ap_instr_intr : 1; + u16 ap_allow_instr : 1; + }; + u16 raw; + } flags; u64 guest_stor_origin; u64 guest_stor_len; u64 guest_sca; diff --git a/arch/s390/kvm/pv.c b/arch/s390/kvm/pv.c index beeebc11b1a1..22fb482f042d 100644 --- a/arch/s390/kvm/pv.c +++ b/arch/s390/kvm/pv.c @@ -572,12 +572,14 @@ int kvm_s390_pv_init_vm(struct kvm *kvm, u16 *rc, u16 *rrc) uvcb.conf_base_stor_origin = virt_to_phys((void *)kvm->arch.pv.stor_base); uvcb.conf_virt_stor_origin = (u64)kvm->arch.pv.stor_var; + uvcb.flags.ap_allow_instr = kvm->arch.model.uv_feat_guest.ap; + uvcb.flags.ap_instr_intr = kvm->arch.model.uv_feat_guest.ap_intr; cc = uv_call_sched(0, (u64)&uvcb); *rc = uvcb.header.rc; *rrc = uvcb.header.rrc; - KVM_UV_EVENT(kvm, 3, "PROTVIRT CREATE VM: handle %llx len %llx rc %x rrc %x", - uvcb.guest_handle, uvcb.guest_stor_len, *rc, *rrc); + KVM_UV_EVENT(kvm, 3, "PROTVIRT CREATE VM: handle %llx len %llx rc %x rrc %x flags %04x", + uvcb.guest_handle, uvcb.guest_stor_len, *rc, *rrc, uvcb.flags.raw); /* Outputs */ kvm->arch.pv.handle = uvcb.guest_handle; -- cgit v1.2.3