On Wed, May 29, 2024 at 8:18 PM Halil Pasic <pa...@linux.ibm.com> wrote:
>
> On Tue, 28 May 2024 11:25:51 +0800
> Jason Wang <jasow...@redhat.com> wrote:
>
> > > 5) Based on the following, I would very much prefer a per device list of
> > > features with the semantic "hey QEMU can do that feature without any
> > > specialized vhost-device support (e.g. like VIRTIO_SCSI_F_CHANGE)"
> >
> > Indeed the current code is kind of tricky and may need better
> > documentation. But the problem is some features were datapath related
> > and they are supported since the birth of a specific vhost device. For
> > example, some GSO related features (actually, it's not a feature of
> > vhost-net but TUN/TAP).
> >
> > And I've found another interesting thing, for RING_REST, actually we
> > don't need to do anything but we have the following commits:
> >
> > 313389be06ff6 ("vhost-net: support VIRTIO_F_RING_RESET")
> > 2a3552baafb ("vhost: vhost-kernel: enable vq reset feature")
> >
> > Technically, they are not necessary as RING_RESET for vhost-kernel
> > doesn't require any additional new ioctls. But it's too late to change
> > as the kernel commit has been merged.
> >
> > > over
> > > the current list with the semantics "these are the features that
> > > need vhost backend support, and anything else will just work out". That
> > > way if an omission is made, we would end up with the usually safer
> > > under-indication instead of the current over-indication.
> > >
> > >
> > > @Michael, Jason: Could you guys chime in?
> >
> > Another issue is that it seems to require a change of the semantic of
> > VHOST_GET_FEATURES. If my understanding is true, it seems a
> > non-trivial change which I'm not sure it's worth to bother.
>
> I don't quite understand. Would you mind to elaborate on this?
>
> For starters, what is the current semantic of VHOST_GET_FEATURES, and
> where is it documented?
Unfortunately, no documentation. The semantic is kind of complicated
which requires the userspace to know how a specific vhost device
works.
For example, userspace knows vhost-net works with tuntap. So it checks
part of the features with vhost-net and the rest with tuntap.
> You mean the ioctl, right?
Yes.
>
> Then why do you think the semantic of VHOST_GET_FEATURES should change?
For example vhost-net can return GSO related features.
If we don't do that, I don't know how we could achieve
under-indication as you mentioned.
>
> IMHO changing the semantic of the VHOST_GET_FEATURES ioctl is not viable,
> but also not necessary. What I am proposing is changing the (in QEMU)
> logic of processing the features returned by VHOST_GET_FEATURES, while
> preserving the outcomes (essentially realize the same function in a
> mathematical sense, but with code that is less fragile), modulo bugs like
> the one addressed with this patch of course.
Ok, I think I misunderstood you here. Maybe an RFC to see?
Thanks
>
> Regards,
> Halil
>
