Re: [PATCH 2/2] libvhost-user: handle endianness as mandated by the spec

[Cornelia Huck]

On Thu, 30 Jul 2020 16:07:31 +0200
Marc Hartmayer <mhartmay@linux.ibm.com> wrote:

> Since virtio existed even before it got standardized, the virtio
> standard defines the following types of virtio devices:
> 
>  + legacy device (pre-virtio 1.0)
>  + non-legacy or VIRTIO 1.0 device
>  + transitional device (which can act both as legacy and non-legacy)
> 
> Virtio 1.0 defines the fields of the virtqueues as little endian,
> while legacy uses guest's native endian [1]. Currently libvhost-user
> does not handle virtio endianness at all, i.e. it works only if the
> native endianness matches with whatever is actually needed. That means
> things break spectacularly on big-endian targets. Let us handle virtio
> endianness for non-legacy as required by the virtio specification
> [1]. 

Maybe add

"and fence legacy virtio, as there is no safe way to figure out the
needed endianness conversions for all cases."

> The fencing of legacy virtio devices is done in
> `vu_set_features_exec`.

Not that I disagree with fencing legacy virtio, but looking at some
vhost-user* drivers, I'm not sure everything will work as desired for
those (I might be missing something, though.)

- vhost-user-blk lists VERSION_1 in the supported features, but
  vhost-user-scsi doesn't... is there some inheritance going on that
  I'm missing?
- vhost-user-gpu-pci inherits from virtio-gpu-pci, so I guess it's fine
- vhost-user-input should also always have been virtio-1

So, has anybody been using vhost-user-scsi and can confirm that it
still works, or at least can be made to work?

> 
> [1] 
> https://docs.oasis-open.org/virtio/virtio/v1.1/cs01/virtio-v1.1-cs01.html#x1-210003
> 
> Signed-off-by: Marc Hartmayer <mhartmay@linux.ibm.com>
> ---
>  contrib/libvhost-user/libvhost-user.c | 77 +++++++++++++++------------
>  1 file changed, 43 insertions(+), 34 deletions(-)

The code change per se LGTM.