Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
MIME-Version: 1.0
References: <20220519143145.767845-1-eperezma@redhat.com> <20220519144801.m7ioxoa5beo5jzv7@sgarzare-redhat>
In-Reply-To: <20220519144801.m7ioxoa5beo5jzv7@sgarzare-redhat>
From:   Eugenio Perez Martin <eperezma@redhat.com>
Date:   Thu, 19 May 2022 16:57:18 +0200
Message-ID: <CAJaqyWfywBEe21P1a1LG2v=Ab2d-L5x19MrtWLvDqccOxFLMqQ@mail.gmail.com>
Subject: Re: [PATCH] vdpasim: allow to enable a vq repeatedly
To:     Stefano Garzarella <sgarzare@redhat.com>
Cc:     Michael Tsirkin <mst@redhat.com>, Jason Wang <jasowang@redhat.com>,
        linux-kernel@vger.kernel.org, Gautam Dawar <gdawar@xilinx.com>,
        Zhu Lingshan <lingshan.zhu@intel.com>,
        kvm list <kvm@vger.kernel.org>, Cindy Lu <lulu@redhat.com>,
        netdev@vger.kernel.org, Laurent Vivier <lvivier@redhat.com>,
        Eli Cohen <eli@mellanox.com>,
        virtualization <virtualization@lists.linux-foundation.org>,
        Parav Pandit <parav@nvidia.com>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Precedence: bulk

On Thu, May 19, 2022 at 4:48 PM Stefano Garzarella <sgarzare@redhat.com> wr=
ote:
>
> On Thu, May 19, 2022 at 04:31:45PM +0200, Eugenio P=C3=A9rez wrote:
> >Code must be resilient to enable a queue many times.
> >
> >At the moment the queue is resetting so it's definitely not the expected
> >behavior.
> >
> >Fixes: 2c53d0f64c06 ("vdpasim: vDPA device simulator")
> >Cc: stable@vger.kernel.org
> >Signed-off-by: Eugenio P=C3=A9rez <eperezma@redhat.com>
> >---
> > drivers/vdpa/vdpa_sim/vdpa_sim.c | 5 +++--
> > 1 file changed, 3 insertions(+), 2 deletions(-)
> >
> >diff --git a/drivers/vdpa/vdpa_sim/vdpa_sim.c b/drivers/vdpa/vdpa_sim/vd=
pa_sim.c
> >index ddbe142af09a..b53cd00ad161 100644
> >--- a/drivers/vdpa/vdpa_sim/vdpa_sim.c
> >+++ b/drivers/vdpa/vdpa_sim/vdpa_sim.c
> >@@ -355,9 +355,10 @@ static void vdpasim_set_vq_ready(struct vdpa_device=
 *vdpa, u16 idx, bool ready)
> >       struct vdpasim_virtqueue *vq =3D &vdpasim->vqs[idx];
> >
> >       spin_lock(&vdpasim->lock);
> >-      vq->ready =3D ready;
> >-      if (vq->ready)
> >+      if (!vq->ready) {
> >+              vq->ready =3D ready;
> >               vdpasim_queue_ready(vdpasim, idx);
> >+      }
>
> But this way the first time vq->ready is set to true, then it will never
> be set back to false.
>

You're right, I had in mind to reset the flow before enabling as the
only possibility.

Sending v2 with that part fixed, thanks!

> Should we leave the assignment out of the block?
> Maybe after the if block to avoid the problem we are fixing.
>
> Thanks,
> Stefano
>