Re: [RFC PATCH v5 21/26] vhost: Add vhost_svq_valid_guest_features to shadow vq

[Eugenio Perez Martin <eperezma@redhat.com>]

On Tue, Nov 2, 2021 at 6:26 AM Jason Wang <jasowang@redhat.com> wrote:
>
> On Sat, Oct 30, 2021 at 2:44 AM Eugenio Pérez <eperezma@redhat.com> wrote:
> >
> > This allows it to test if the guest has aknowledge an invalid transport
> > feature for SVQ. This will include packed vq layout or event_idx,
> > where VirtIO device needs help from SVQ.
> >
> > There is not needed at this moment, but since SVQ will not re-negotiate
> > features again with the guest, a failure in acknowledge them is fatal
> > for SVQ.
> >
>
> It's not clear to me why we need this. Maybe you can give me an
> example. E.g isn't it sufficient to filter out the device with
> event_idx?
>

If the guest did negotiate _F_EVENT_IDX, it expects to be notified
only when device marks as used a specific number of descriptors.

If we use VirtQueue notification, the VirtQueue code handles it
transparently. But if we want to be able to change the guest VQ's
call_fd, we cannot use VirtQueue's, so this needs to be handled by SVQ
code. And that is still not implemented.

Of course in the event_idx case we could just ignore it and notify in
all used descriptors, but it seems not polite to me :). I will develop
event_idx on top of this, either exposing the needed pieces in
VirtQueue (I prefer this) or rolling our own in SVQ.

Same reasoning can be applied to unknown transport features.

Thanks!

> Thanks
>
> > Signed-off-by: Eugenio Pérez <eperezma@redhat.com>
> > ---
> >  hw/virtio/vhost-shadow-virtqueue.h | 1 +
> >  hw/virtio/vhost-shadow-virtqueue.c | 6 ++++++
> >  2 files changed, 7 insertions(+)
> >
> > diff --git a/hw/virtio/vhost-shadow-virtqueue.h b/hw/virtio/vhost-shadow-virtqueue.h
> > index 946b2c6295..ac55588009 100644
> > --- a/hw/virtio/vhost-shadow-virtqueue.h
> > +++ b/hw/virtio/vhost-shadow-virtqueue.h
> > @@ -16,6 +16,7 @@
> >  typedef struct VhostShadowVirtqueue VhostShadowVirtqueue;
> >
> >  bool vhost_svq_valid_device_features(uint64_t *features);
> > +bool vhost_svq_valid_guest_features(uint64_t *features);
> >
> >  void vhost_svq_set_svq_kick_fd(VhostShadowVirtqueue *svq, int svq_kick_fd);
> >  void vhost_svq_set_guest_call_notifier(VhostShadowVirtqueue *svq, int call_fd);
> > diff --git a/hw/virtio/vhost-shadow-virtqueue.c b/hw/virtio/vhost-shadow-virtqueue.c
> > index 6e0508a231..cb9ffcb015 100644
> > --- a/hw/virtio/vhost-shadow-virtqueue.c
> > +++ b/hw/virtio/vhost-shadow-virtqueue.c
> > @@ -62,6 +62,12 @@ bool vhost_svq_valid_device_features(uint64_t *dev_features)
> >      return true;
> >  }
> >
> > +/* If the guest is using some of these, SVQ cannot communicate */
> > +bool vhost_svq_valid_guest_features(uint64_t *guest_features)
> > +{
> > +    return true;
> > +}
> > +
> >  /* Forward guest notifications */
> >  static void vhost_handle_guest_kick(EventNotifier *n)
> >  {
> > --
> > 2.27.0
> >
>