On 16/02/2019 00:29, Tony Krowiak wrote:
On 2/14/19 8:51 AM, Pierre Morel wrote:
When the device is remove, we must make sure to
clear the interruption and reset the AP device.
We also need to clear the CRYCB of the guest.
Signed-off-by: Pierre Morel <pmorel@xxxxxxxxxxxxx>
---
drivers/s390/crypto/vfio_ap_drv.c | 92
+++++++++++++++++++++++++++++++++++
drivers/s390/crypto/vfio_ap_ops.c | 2 +-
drivers/s390/crypto/vfio_ap_private.h | 2 +
3 files changed, 95 insertions(+), 1 deletion(-)
diff --git a/drivers/s390/crypto/vfio_ap_drv.c
b/drivers/s390/crypto/vfio_ap_drv.c
index 03153e6..50428be 100644
--- a/drivers/s390/crypto/vfio_ap_drv.c
+++ b/drivers/s390/crypto/vfio_ap_drv.c
@@ -5,6 +5,7 @@
* Copyright IBM Corp. 2018
*
* Author(s): Tony Krowiak <akrowiak@xxxxxxxxxxxxx>
+ * Pierre Morel <pmorel@xxxxxxxxxxxxx>
*/
#include <linux/module.h>
@@ -12,6 +13,8 @@
#include <linux/slab.h>
#include <linux/string.h>
#include <asm/facility.h>
+#include <linux/bitops.h>
+#include <linux/kvm_host.h>
#include "vfio_ap_private.h"
#define VFIO_AP_ROOT_NAME "vfio_ap"
@@ -64,6 +67,88 @@ static int vfio_ap_queue_dev_probe(struct ap_device
*apdev)
return 0;
}
+/*
+ * vfio_ap_drain_queue
+ * @q: the queue to drain
+ *
+ * This function waits until the queue is empty.
+ */
+static void vfio_ap_drain_queue(struct vfio_ap_queue *q)
+{
+ struct ap_queue_status status;
+ int retry = 20;
+
+ status = ap_tapq(q->apqn, NULL);
+ while (!status.queue_empty && retry--) {
+ msleep(200);
+ status = ap_tapq(q->apqn, NULL);
+ }
+ if (retry <= 0) {
+ pr_warn("%s: queue not empty after zapq on apqn 0x%04x\n",
+ __func__, q->apqn);
+ }
+}
+
+/*
+ * vfio_ap_zapq
+ * @q: The queue to zerro
+ *
+ * It is best effort, no return value is done, however on success
+ * we will drain the queue before getting the queue back to the
+ * AP bus.
+ */
+static void vfio_ap_zapq(struct vfio_ap_queue *q)
+{
+ struct ap_queue_status status;
+ int retry = 20;
+
+ do {
+ status = ap_zapq(q->apqn);
+ switch (status.response_code) {
+ case AP_RESPONSE_RESET_IN_PROGRESS:
+ case AP_RESPONSE_BUSY:
+ msleep(20);
+ break;
+ default:
+ pr_warn("%s: zapq error %02x on apqn 0x%04x\n",
+ __func__, status.response_code, q->apqn);
+ return;
+ case AP_RESPONSE_NORMAL:
+ vfio_ap_drain_queue(q);
I don't think this is necessary. The zeroize is performed on
each AP-queue entry in an AP queue. My understanding is that when a
reset or zeroize is pending, any AP instructions subsequently issued
are rejected with condition-code 3 indicating an AP queue reset is in
progress. It is also my understanding that once the AP commands
currently executing in a given AP queue entry complete, the queue
entry will be zeroized. So it seems to me that there is no need to
"drain" the queue, it will have already been done by the zeroize.
My understanding from the specifications is that after a RAPQ or a ZAPQ
we should wait for the queue to be really empty.
If you agree we don't need to "drain" the queue, then I'd rather just
make the zapq function in the vfio_ap_ops.c non-static and make it
available to the driver. There is no sense in duplicating this code. In
fact, even if you keep the draining function, you still don't need to
duplicate a zaapq instruction here, you can just call the vfio_ap_ops.c
version and then drain the queue on AP_RESPONSE_NORMAL.
OK I will see how to optimize this.
+ return;
+
+ vfio_ap_update_crycb(q);
+ vfio_ap_zapq(q);
+
+ vfio_ap_free_irq(q);
If you make the zapq function in vfio_ap_ops.c available to the driver
rather than duplicating it in this file, you won't need this call
to vfio_ap_free_irq because it is done as part of the zapq in
vfio_ap_ops.c.
Another solution.
OK I will look at this.
Thanks,
Pierre
--
Pierre Morel
Linux/KVM/QEMU in Böblingen - Germany
