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

[Jason Wang]

在 2023/3/30 18:42, 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:


I'm not sure I understand here, if device supports MQ it should accepts 
MQ or we can fail the initialization here.


> * All the requests to queue 3 must be redirected to the last queue in
> the device. That includes set_vq_address, notifiers regions, etc.


This also means we will only mediate the case:

1) Qemu emulated virtio-net has 1 queue but device support multiple queue

but not

2) Qemu emulated virtio-net has M queue but device support N queue (N>M)


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


Yes but I mean, we can start from failing the device if 
cvq_isolated_mq^cvq_isolated == true (or I wonder if we can meet this 
condition for any existing parents).

Thanks


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