Re: [PATCH for 8.1 v2 5/6] vdpa: move CVQ isolation check to net_init_vhost_vdpa

[Eugenio Perez Martin]

On Thu, Mar 30, 2023 at 8:23 AM Jason Wang <jasowang@redhat.com> wrote:
>
> On Thu, Mar 30, 2023 at 2:20 PM Jason Wang <jasowang@redhat.com> wrote:
> >
> > On Fri, Mar 24, 2023 at 3:54 AM Eugenio Pérez <eperezma@redhat.com> wrote:
> > >
> > > Evaluating it at start time instead of initialization time may make the
> > > guest capable of dynamically adding or removing migration blockers.
> > >
> > > Also, moving to initialization reduces the number of ioctls in the
> > > migration, reducing failure possibilities.
> > >
> > > As a drawback we need to check for CVQ isolation twice: one time with no
> > > MQ negotiated and another one acking it, as long as the device supports
> > > it.  This is because Vring ASID / group management is based on vq
> > > indexes, but we don't know the index of CVQ before negotiating MQ.
> >
> > We need to fail if we see a device that can isolate cvq without MQ but
> > not with MQ.
> >
> > >
> > > Signed-off-by: Eugenio Pérez <eperezma@redhat.com>
> > > ---
> > > v2: Take out the reset of the device from vhost_vdpa_cvq_is_isolated
> > > ---
> > >  net/vhost-vdpa.c | 194 ++++++++++++++++++++++++++++++++++++-----------
> > >  1 file changed, 151 insertions(+), 43 deletions(-)
> > >
> > > diff --git a/net/vhost-vdpa.c b/net/vhost-vdpa.c
> > > index 4397c0d4b3..db2c9afcb3 100644
> > > --- a/net/vhost-vdpa.c
> > > +++ b/net/vhost-vdpa.c
> > > @@ -43,6 +43,13 @@ typedef struct VhostVDPAState {
> > >
> > >      /* The device always have SVQ enabled */
> > >      bool always_svq;
> > > +
> > > +    /* The device can isolate CVQ in its own ASID if MQ is negotiated */
> > > +    bool cvq_isolated_mq;
> > > +
> > > +    /* The device can isolate CVQ in its own ASID if MQ is not 
> > > negotiated */
> > > +    bool cvq_isolated;
> >
> > As stated above, if we need a device that cvq_isolated_mq^cvq_isolated
> > == true, we need to fail. This may reduce the complexity of the code?
> >
> > Thanks
>
> Since we are the mediation layer, Qemu can alway choose to negotiate
> MQ regardless whether or not it is supported by the guest. In this
> way, we can have a stable virtqueue index for cvq.
>

I think it is a great idea and it simplifies this patch somehow.
However, we need something like the queue mapping [1] to do so :).

To double confirm:
* If the device supports MQ, only probe MQ. If not, only probe !MQ.
* Only store cvq_isolated in VhostVDPAState.

Now, if the device does not negotiate MQ but the device supports MQ:
* All the requests to queue 3 must be redirected to the last queue in
the device. That includes set_vq_address, notifiers regions, etc.

I'm totally ok to go this route but it's not immediate.

Thanks!

[1] https://lists.gnu.org/archive/html/qemu-devel/2023-01/msg07157.html