| Message ID | 20221213154437.15480-3-akrowiak@linux.ibm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | improve AP queue reset processing | expand |
On 2022-12-13 16:44, Tony Krowiak wrote: > The vfio_ap_mdev_reset_queue() function does not check the status > response code returned form the PQAP(TAPQ) function when verifying the > queue's status; consequently, there is no way of knowing whether > verification failed because the wait time was exceeded, or because the > PQAP(TAPQ) failed. > > This patch adds a function to check the status response code from the > PQAP(TAPQ) instruction and logs an appropriate message if it fails. > > Signed-off-by: Tony Krowiak <akrowiak@linux.ibm.com> > --- > drivers/s390/crypto/vfio_ap_ops.c | 36 ++++++++++++++++++++++++++----- > 1 file changed, 31 insertions(+), 5 deletions(-) > > diff --git a/drivers/s390/crypto/vfio_ap_ops.c > b/drivers/s390/crypto/vfio_ap_ops.c > index 83ff94a38102..a5530a46cf31 100644 > --- a/drivers/s390/crypto/vfio_ap_ops.c > +++ b/drivers/s390/crypto/vfio_ap_ops.c > @@ -1587,23 +1587,49 @@ static struct vfio_ap_queue > *vfio_ap_find_queue(int apqn) > return q; > } > > +static int apq_status_check(int apqn, struct ap_queue_status *status) > +{ > + 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; > + default: > + WARN(true, > + "failed to verify reset of queue %02x.%04x: TAPQ rc=%u\n", > + AP_QID_CARD(apqn), AP_QID_QUEUE(apqn), > + status->response_code); > + return -EIO; > + } > +} > + > static int apq_reset_check(struct vfio_ap_queue *q) > { > - int iters = 2; > + int iters = 2, ret; > struct ap_queue_status status; > > while (iters--) { > msleep(20); > status = ap_tapq(q->apqn, NULL); > - if (status.queue_empty && !status.irq_enabled) > - return 0; > + ret = apq_status_check(q->apqn, &status); > + if (ret != -EBUSY) > + return ret; > } > 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 -EBUSY; > + return ret; > } > > static int vfio_ap_mdev_reset_queue(struct vfio_ap_queue *q, Reviewed-by: Harald Freudenberger <freude@linux.ibm.com> Just one word here: this function is only called once and it is very very special to just check the status after RAPQ/ZAPQ. I would merge this function into the calling code or rename the function to reflect the special condition under which it is called. However - this is not my code and I don't need to maintain it, so maybe simple ignore my words.
diff --git a/drivers/s390/crypto/vfio_ap_ops.c b/drivers/s390/crypto/vfio_ap_ops.c index 83ff94a38102..a5530a46cf31 100644 --- a/drivers/s390/crypto/vfio_ap_ops.c +++ b/drivers/s390/crypto/vfio_ap_ops.c @@ -1587,23 +1587,49 @@ static struct vfio_ap_queue *vfio_ap_find_queue(int apqn) return q; } +static int apq_status_check(int apqn, struct ap_queue_status *status) +{ + 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; + default: + WARN(true, + "failed to verify reset of queue %02x.%04x: TAPQ rc=%u\n", + AP_QID_CARD(apqn), AP_QID_QUEUE(apqn), + status->response_code); + return -EIO; + } +} + static int apq_reset_check(struct vfio_ap_queue *q) { - int iters = 2; + int iters = 2, ret; struct ap_queue_status status; while (iters--) { msleep(20); status = ap_tapq(q->apqn, NULL); - if (status.queue_empty && !status.irq_enabled) - return 0; + ret = apq_status_check(q->apqn, &status); + if (ret != -EBUSY) + return ret; } 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 -EBUSY; + return ret; } static int vfio_ap_mdev_reset_queue(struct vfio_ap_queue *q,
The vfio_ap_mdev_reset_queue() function does not check the status response code returned form the PQAP(TAPQ) function when verifying the queue's status; consequently, there is no way of knowing whether verification failed because the wait time was exceeded, or because the PQAP(TAPQ) failed. This patch adds a function to check the status response code from the PQAP(TAPQ) instruction and logs an appropriate message if it fails. Signed-off-by: Tony Krowiak <akrowiak@linux.ibm.com> --- drivers/s390/crypto/vfio_ap_ops.c | 36 ++++++++++++++++++++++++++----- 1 file changed, 31 insertions(+), 5 deletions(-)