Messages in this thread |  | | Date | Thu, 15 Dec 2022 09:24:15 +0100 | From | Harald Freudenberger <> | Subject | Re: [PATCH 1/7] s390/vfio-ap: verify reset complete in separate function |
| |
On 2022-12-13 16:44, Tony Krowiak wrote: > The vfio_ap_mdev_reset_queue() function contains a loop to verify that > the > reset successfully completes within 40ms. This patch moves that loop > into > a separate function. > > Signed-off-by: Tony Krowiak <akrowiak@linux.ibm.com> > --- > drivers/s390/crypto/vfio_ap_ops.c | 30 +++++++++++++++++++++--------- > 1 file changed, 21 insertions(+), 9 deletions(-) > > diff --git a/drivers/s390/crypto/vfio_ap_ops.c > b/drivers/s390/crypto/vfio_ap_ops.c > index 0b4cc8c597ae..83ff94a38102 100644 > --- a/drivers/s390/crypto/vfio_ap_ops.c > +++ b/drivers/s390/crypto/vfio_ap_ops.c > @@ -1587,12 +1587,30 @@ static struct vfio_ap_queue > *vfio_ap_find_queue(int apqn) > return q; > } > > +static int apq_reset_check(struct vfio_ap_queue *q) > +{ > + int iters = 2; > + struct ap_queue_status status; > + > + while (iters--) { > + msleep(20); > + status = ap_tapq(q->apqn, NULL); > + if (status.queue_empty && !status.irq_enabled) > + return 0; > + } > + 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; > +} > + > static int vfio_ap_mdev_reset_queue(struct vfio_ap_queue *q, > unsigned int retry) > { > struct ap_queue_status status; > int ret; > - int retry2 = 2; > > if (!q) > return 0; > @@ -1629,14 +1647,8 @@ static int vfio_ap_mdev_reset_queue(struct > vfio_ap_queue *q, > } > > /* wait for the reset to take effect */ > - while (retry2--) { > - if (status.queue_empty && !status.irq_enabled) > - break; > - msleep(20); > - status = ap_tapq(q->apqn, NULL); > - } > - WARN_ONCE(retry2 <= 0, "unable to verify reset of queue %02x.%04x", > - AP_QID_CARD(q->apqn), AP_QID_QUEUE(q->apqn)); > + if (!status.queue_empty && status.irq_enabled)
Hm, you check for a) status.queue_empty and b) !status.irq_enabled which is (status.queue_empty && !status.irq_enabled) and now let's negate this to: !(status.queue_empty && !status.irq_enabled) with de-morgan this gives: (!status.queue_empty || status.irq_enabled)
> + ret = apq_reset_check(q); > > free_resources: > vfio_ap_free_aqic_resources(q);
Reviewed-by: Harald Freudenberger <freude@linux.ibm.com>
|  |