[RFC 0/4] Enable virtio-fs on s390x

Marc Hartmayer posted 4 patches 3 years, 10 months ago
Test FreeBSD passed
Test docker-quick@centos7 failed
Test checkpatch failed
Test docker-mingw@fedora passed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20200625100430.22407-1-mhartmay@linux.ibm.com
Maintainers: David Hildenbrand <david@redhat.com>, "Michael S. Tsirkin" <mst@redhat.com>, Christian Borntraeger <borntraeger@de.ibm.com>, Richard Henderson <rth@twiddle.net>, Thomas Huth <thuth@redhat.com>, Cornelia Huck <cohuck@redhat.com>, Halil Pasic <pasic@linux.ibm.com>
There is a newer version of this series
Makefile.objs                           |   1 +
contrib/libvhost-user/libvhost-access.h |  87 +++++++++++++++++
contrib/libvhost-user/libvhost-user.c   | 124 ++++++++++++------------
hw/s390x/Makefile.objs                  |   1 +
hw/s390x/vhost-user-fs-ccw.c            |  74 ++++++++++++++
5 files changed, 227 insertions(+), 60 deletions(-)
create mode 100644 contrib/libvhost-user/libvhost-access.h
create mode 100644 hw/s390x/vhost-user-fs-ccw.c
[RFC 0/4] Enable virtio-fs on s390x
Posted by Marc Hartmayer 3 years, 10 months ago
This RFC is about enabling virtio-fs on s390x. For that we need
 + some shim code (first patch), and we need
 + libvhost-user to deal with virtio endiannes as mandated by the spec.
 
The second part is trickier, because unlike QEMU we are not certain
about the guest's native endianness, which is needed to handle the
legacy-interface appropriately. In fact, this is the reason why just
RFC.

One of the open questions is whether to build separate versions, one
for guest little endian and one for guest big endian, or do we want
something like a command line option? (Digression on the libvirt
modeling)

A third option would be to refuse legacy altogether.

libvhost-access.h is based on hw/virtio/virtio-access.h.


How to use?

For general instructions how to use virtio-fs (on x86) please have a
look at https://virtio-fs.gitlab.io/howto-qemu.html. Most of the
instructions can also be applied on s390x.

In short:

1. Install self-compiled QEMU with this patch series applied
2. Prepare host and guest kernel so they support virtio-fs

Start virtiofsd on the host

 $ virtiofsd -f --socket-path=/tmp/vhostqemu -o source=/tmp/shared

Now you can start QEMU in a separate shell on the host:

 $ qemu-system-s390x -machine type=s390-ccw-virtio,accel=kvm,memory-backend=mem \
   -object memory-backend-file,id=mem,size=2G,mem-path=/dev/shm/virtiofs,share=on,prealloc=on,prealloc-threads=1 \
   -chardev socket,id=char0,path=/tmp/vhostqemu -device vhost-user-fs-ccw,queue-size=1024,chardev=char0,tag=myfs \
   -drive if=virtio,file=disk.qcow2 \
   -m 2G -smp 2 -nographic

Log into the guest and mount it

 $ mount -t virtiofs myfs /mnt


Halil Pasic (1):
  virtio: add vhost-user-fs-ccw device

Marc Hartmayer (3):
  libvhost-user: print invalid address on vu_panic
  libvhost-user: handle endianness as mandated by the spec
  HACK: Hard-code the libvhost-user.o-cflags for s390x

 Makefile.objs                           |   1 +
 contrib/libvhost-user/libvhost-access.h |  87 +++++++++++++++++
 contrib/libvhost-user/libvhost-user.c   | 124 ++++++++++++------------
 hw/s390x/Makefile.objs                  |   1 +
 hw/s390x/vhost-user-fs-ccw.c            |  74 ++++++++++++++
 5 files changed, 227 insertions(+), 60 deletions(-)
 create mode 100644 contrib/libvhost-user/libvhost-access.h
 create mode 100644 hw/s390x/vhost-user-fs-ccw.c

-- 
2.25.4


Re: [RFC 0/4] Enable virtio-fs on s390x
Posted by Stefan Hajnoczi 3 years, 10 months ago
On Thu, Jun 25, 2020 at 12:04:26PM +0200, Marc Hartmayer wrote:
> This RFC is about enabling virtio-fs on s390x. For that we need
>  + some shim code (first patch), and we need
>  + libvhost-user to deal with virtio endiannes as mandated by the spec.
>  
> The second part is trickier, because unlike QEMU we are not certain
> about the guest's native endianness, which is needed to handle the
> legacy-interface appropriately. In fact, this is the reason why just
> RFC.
> 
> One of the open questions is whether to build separate versions, one
> for guest little endian and one for guest big endian, or do we want
> something like a command line option? (Digression on the libvirt
> modeling)
> 
> A third option would be to refuse legacy altogether.

I suggest the following:

1. Combinations that worked with libvhost-user in the past must not break.

2. New combinations should only support VIRTIO 1.0 and later.

This means continue to allow Legacy mode devices where they already run
today but don't add new code for the cases that didn't work.

Stefan
Re: [RFC 0/4] Enable virtio-fs on s390x
Posted by Daniel P. Berrangé 3 years, 10 months ago
On Mon, Jun 29, 2020 at 01:53:05PM +0100, Stefan Hajnoczi wrote:
> On Thu, Jun 25, 2020 at 12:04:26PM +0200, Marc Hartmayer wrote:
> > This RFC is about enabling virtio-fs on s390x. For that we need
> >  + some shim code (first patch), and we need
> >  + libvhost-user to deal with virtio endiannes as mandated by the spec.
> >  
> > The second part is trickier, because unlike QEMU we are not certain
> > about the guest's native endianness, which is needed to handle the
> > legacy-interface appropriately. In fact, this is the reason why just
> > RFC.
> > 
> > One of the open questions is whether to build separate versions, one
> > for guest little endian and one for guest big endian, or do we want
> > something like a command line option? (Digression on the libvirt
> > modeling)
> > 
> > A third option would be to refuse legacy altogether.
> 
> I suggest the following:
> 
> 1. Combinations that worked with libvhost-user in the past must not break.
> 
> 2. New combinations should only support VIRTIO 1.0 and later.
> 
> This means continue to allow Legacy mode devices where they already run
> today but don't add new code for the cases that didn't work.

What I'm missing here is what PCI product ID was being used when the
current impl is in legacy/transitional mode ?

Normally legacy and transitional mode devices need an explicit PCI ID
reserved, where as modern-only devices have a PCI ID derived from their
VirtIO ID + a fixed offset.

Was this mistakenly using a VirtIO ID + fixed offset for the legacy
mode too ?

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|


Re: [RFC 0/4] Enable virtio-fs on s390x
Posted by Stefan Hajnoczi 3 years, 10 months ago
On Mon, Jun 29, 2020 at 02:07:16PM +0100, Daniel P. Berrangé wrote:
> On Mon, Jun 29, 2020 at 01:53:05PM +0100, Stefan Hajnoczi wrote:
> > On Thu, Jun 25, 2020 at 12:04:26PM +0200, Marc Hartmayer wrote:
> > > This RFC is about enabling virtio-fs on s390x. For that we need
> > >  + some shim code (first patch), and we need
> > >  + libvhost-user to deal with virtio endiannes as mandated by the spec.
> > >  
> > > The second part is trickier, because unlike QEMU we are not certain
> > > about the guest's native endianness, which is needed to handle the
> > > legacy-interface appropriately. In fact, this is the reason why just
> > > RFC.
> > > 
> > > One of the open questions is whether to build separate versions, one
> > > for guest little endian and one for guest big endian, or do we want
> > > something like a command line option? (Digression on the libvirt
> > > modeling)
> > > 
> > > A third option would be to refuse legacy altogether.
> > 
> > I suggest the following:
> > 
> > 1. Combinations that worked with libvhost-user in the past must not break.
> > 
> > 2. New combinations should only support VIRTIO 1.0 and later.
> > 
> > This means continue to allow Legacy mode devices where they already run
> > today but don't add new code for the cases that didn't work.
> 
> What I'm missing here is what PCI product ID was being used when the
> current impl is in legacy/transitional mode ?
> 
> Normally legacy and transitional mode devices need an explicit PCI ID
> reserved, where as modern-only devices have a PCI ID derived from their
> VirtIO ID + a fixed offset.
> 
> Was this mistakenly using a VirtIO ID + fixed offset for the legacy
> mode too ?

vhost-user-fs-pci does not support Legacy or Transitional mode. See
hw/virtio/vhost-user-fs-pci.c:

  static const VirtioPCIDeviceTypeInfo vhost_user_fs_pci_info = {
      .base_name             = TYPE_VHOST_USER_FS_PCI,
      .non_transitional_name = "vhost-user-fs-pci",
      ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
      .instance_size = sizeof(VHostUserFSPCI),
      .instance_init = vhost_user_fs_pci_instance_init,
      .class_init    = vhost_user_fs_pci_class_init,
  };

Stefan
Re: [RFC 0/4] Enable virtio-fs on s390x
Posted by Cornelia Huck 3 years, 10 months ago
On Tue, 30 Jun 2020 10:04:51 +0100
Stefan Hajnoczi <stefanha@redhat.com> wrote:

> On Mon, Jun 29, 2020 at 02:07:16PM +0100, Daniel P. Berrangé wrote:
> > On Mon, Jun 29, 2020 at 01:53:05PM +0100, Stefan Hajnoczi wrote:  
> > > On Thu, Jun 25, 2020 at 12:04:26PM +0200, Marc Hartmayer wrote:  
> > > > This RFC is about enabling virtio-fs on s390x. For that we need
> > > >  + some shim code (first patch), and we need
> > > >  + libvhost-user to deal with virtio endiannes as mandated by the spec.
> > > >  
> > > > The second part is trickier, because unlike QEMU we are not certain
> > > > about the guest's native endianness, which is needed to handle the
> > > > legacy-interface appropriately. In fact, this is the reason why just
> > > > RFC.
> > > > 
> > > > One of the open questions is whether to build separate versions, one
> > > > for guest little endian and one for guest big endian, or do we want
> > > > something like a command line option? (Digression on the libvirt
> > > > modeling)
> > > > 
> > > > A third option would be to refuse legacy altogether.  
> > > 
> > > I suggest the following:
> > > 
> > > 1. Combinations that worked with libvhost-user in the past must not break.
> > > 
> > > 2. New combinations should only support VIRTIO 1.0 and later.
> > > 
> > > This means continue to allow Legacy mode devices where they already run
> > > today but don't add new code for the cases that didn't work.  
> > 
> > What I'm missing here is what PCI product ID was being used when the
> > current impl is in legacy/transitional mode ?
> > 
> > Normally legacy and transitional mode devices need an explicit PCI ID
> > reserved, where as modern-only devices have a PCI ID derived from their
> > VirtIO ID + a fixed offset.
> > 
> > Was this mistakenly using a VirtIO ID + fixed offset for the legacy
> > mode too ?  
> 
> vhost-user-fs-pci does not support Legacy or Transitional mode. See
> hw/virtio/vhost-user-fs-pci.c:
> 
>   static const VirtioPCIDeviceTypeInfo vhost_user_fs_pci_info = {
>       .base_name             = TYPE_VHOST_USER_FS_PCI,
>       .non_transitional_name = "vhost-user-fs-pci",
>       ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
>       .instance_size = sizeof(VHostUserFSPCI),
>       .instance_init = vhost_user_fs_pci_instance_init,
>       .class_init    = vhost_user_fs_pci_class_init,
>   };

This makes it very unlikely that someone accidentally configures
non-modern, but does not prevent it AFAICS. See
<20200630113527.7b27f34f.cohuck@redhat.com>, which I just sent.

(I may be off, because that is all very confusing...)
Re: [RFC 0/4] Enable virtio-fs on s390x
Posted by Michael S. Tsirkin 3 years, 9 months ago
On Tue, Jun 30, 2020 at 11:39:32AM +0200, Cornelia Huck wrote:
> On Tue, 30 Jun 2020 10:04:51 +0100
> Stefan Hajnoczi <stefanha@redhat.com> wrote:
> 
> > On Mon, Jun 29, 2020 at 02:07:16PM +0100, Daniel P. Berrangé wrote:
> > > On Mon, Jun 29, 2020 at 01:53:05PM +0100, Stefan Hajnoczi wrote:  
> > > > On Thu, Jun 25, 2020 at 12:04:26PM +0200, Marc Hartmayer wrote:  
> > > > > This RFC is about enabling virtio-fs on s390x. For that we need
> > > > >  + some shim code (first patch), and we need
> > > > >  + libvhost-user to deal with virtio endiannes as mandated by the spec.
> > > > >  
> > > > > The second part is trickier, because unlike QEMU we are not certain
> > > > > about the guest's native endianness, which is needed to handle the
> > > > > legacy-interface appropriately. In fact, this is the reason why just
> > > > > RFC.
> > > > > 
> > > > > One of the open questions is whether to build separate versions, one
> > > > > for guest little endian and one for guest big endian, or do we want
> > > > > something like a command line option? (Digression on the libvirt
> > > > > modeling)
> > > > > 
> > > > > A third option would be to refuse legacy altogether.  
> > > > 
> > > > I suggest the following:
> > > > 
> > > > 1. Combinations that worked with libvhost-user in the past must not break.
> > > > 
> > > > 2. New combinations should only support VIRTIO 1.0 and later.
> > > > 
> > > > This means continue to allow Legacy mode devices where they already run
> > > > today but don't add new code for the cases that didn't work.  
> > > 
> > > What I'm missing here is what PCI product ID was being used when the
> > > current impl is in legacy/transitional mode ?
> > > 
> > > Normally legacy and transitional mode devices need an explicit PCI ID
> > > reserved, where as modern-only devices have a PCI ID derived from their
> > > VirtIO ID + a fixed offset.
> > > 
> > > Was this mistakenly using a VirtIO ID + fixed offset for the legacy
> > > mode too ?  
> > 
> > vhost-user-fs-pci does not support Legacy or Transitional mode. See
> > hw/virtio/vhost-user-fs-pci.c:
> > 
> >   static const VirtioPCIDeviceTypeInfo vhost_user_fs_pci_info = {
> >       .base_name             = TYPE_VHOST_USER_FS_PCI,
> >       .non_transitional_name = "vhost-user-fs-pci",
> >       ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> >       .instance_size = sizeof(VHostUserFSPCI),
> >       .instance_init = vhost_user_fs_pci_instance_init,
> >       .class_init    = vhost_user_fs_pci_class_init,
> >   };
> 
> This makes it very unlikely that someone accidentally configures
> non-modern, but does not prevent it AFAICS. See
> <20200630113527.7b27f34f.cohuck@redhat.com>, which I just sent.
> 
> (I may be off, because that is all very confusing...)

Right. We'll block legacy for modern only devices going forward.
Going back to the patchset in question, virtio-fs is modern
only, legacy will not be supported.

-- 
MST


Re: [RFC 0/4] Enable virtio-fs on s390x
Posted by no-reply@patchew.org 3 years, 10 months ago
Patchew URL: https://patchew.org/QEMU/20200625100430.22407-1-mhartmay@linux.ibm.com/



Hi,

This series seems to have some coding style problems. See output below for
more information:

Subject: [RFC 0/4] Enable virtio-fs on s390x
Type: series
Message-id: 20200625100430.22407-1-mhartmay@linux.ibm.com

=== TEST SCRIPT BEGIN ===
#!/bin/bash
git rev-parse base > /dev/null || exit 0
git config --local diff.renamelimit 0
git config --local diff.renames True
git config --local diff.algorithm histogram
./scripts/checkpatch.pl --mailback base..
=== TEST SCRIPT END ===

Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
From https://github.com/patchew-project/qemu
 - [tag update]      patchew/20200622153318.751107-1-berrange@redhat.com -> patchew/20200622153318.751107-1-berrange@redhat.com
 * [new tag]         patchew/20200625100430.22407-1-mhartmay@linux.ibm.com -> patchew/20200625100430.22407-1-mhartmay@linux.ibm.com
 * [new tag]         patchew/20200625100811.12690-1-peter.maydell@linaro.org -> patchew/20200625100811.12690-1-peter.maydell@linaro.org
Switched to a new branch 'test'
b2cd719 HACK: Hard-code the libvhost-user.o-cflags for s390x
f0ba6d5 libvhost-user: handle endianness as mandated by the spec
5f2feba libvhost-user: print invalid address on vu_panic
19994e0 virtio: add vhost-user-fs-ccw device

=== OUTPUT BEGIN ===
1/4 Checking commit 19994e01a67c (virtio: add vhost-user-fs-ccw device)
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#25: 
new file mode 100644

ERROR: space required after that ',' (ctx:VxV)
#86: FILE: hw/s390x/vhost-user-fs-ccw.c:57:
+    device_class_set_props(dc,vhost_user_fs_ccw_properties);
                              ^

total: 1 errors, 1 warnings, 81 lines checked

Patch 1/4 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

2/4 Checking commit 5f2feba1e3c5 (libvhost-user: print invalid address on vu_panic)
3/4 Checking commit f0ba6d5325c1 (libvhost-user: handle endianness as mandated by the spec)
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#27: 
new file mode 100644

ERROR: braces {} are necessary for all arms of this statement
#100: FILE: contrib/libvhost-user/libvhost-access.h:69:
+    if (vu_access_is_big_endian(vdev))
[...]

ERROR: braces {} are necessary for all arms of this statement
#107: FILE: contrib/libvhost-user/libvhost-access.h:76:
+    if (vu_access_is_big_endian(vdev))
[...]

ERROR: braces {} are necessary for all arms of this statement
#114: FILE: contrib/libvhost-user/libvhost-access.h:83:
+    if (vu_access_is_big_endian(vdev))
[...]

WARNING: line over 80 characters
#375: FILE: contrib/libvhost-user/libvhost-user.c:2515:
+                               vu_ldq_p(dev, &desc[i].addr), vu_ldl_p(dev, &desc[i].len));

WARNING: line over 80 characters
#384: FILE: contrib/libvhost-user/libvhost-user.c:2523:
+                               vu_ldq_p(dev, &desc[i].addr), vu_ldl_p(dev, &desc[i].len));

total: 3 errors, 3 warnings, 392 lines checked

Patch 3/4 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

4/4 Checking commit b2cd71997828 (HACK: Hard-code the libvhost-user.o-cflags for s390x)
=== OUTPUT END ===

Test command exited with code: 1


The full log is available at
http://patchew.org/logs/20200625100430.22407-1-mhartmay@linux.ibm.com/testing.checkpatch/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-devel@redhat.com
Re: [RFC 0/4] Enable virtio-fs on s390x
Posted by no-reply@patchew.org 3 years, 10 months ago
Patchew URL: https://patchew.org/QEMU/20200625100430.22407-1-mhartmay@linux.ibm.com/



Hi,

This series failed the docker-quick@centos7 build test. Please find the testing commands and
their output below. If you have Docker installed, you can probably reproduce it
locally.

=== TEST SCRIPT BEGIN ===
#!/bin/bash
make docker-image-centos7 V=1 NETWORK=1
time make docker-test-quick@centos7 SHOW_ENV=1 J=14 NETWORK=1
=== TEST SCRIPT END ===

  CC      qga/qapi-generated/qga-qapi-commands.o
  CC      qga/qapi-generated/qga-qapi-init-commands.o
In file included from /tmp/qemu-test/src/contrib/libvhost-user/libvhost-user.c:44:0:
/tmp/qemu-test/src/include/qemu/osdep.h:32:27: fatal error: config-target.h: No such file or directory
 #include "config-target.h"
                           ^
compilation terminated.
---
  BUILD   pc-bios/optionrom/linuxboot_dma.raw
  SIGN    pc-bios/optionrom/pvh.bin
  SIGN    pc-bios/optionrom/linuxboot_dma.bin
make: *** [contrib/libvhost-user/libvhost-user.o] Error 1
make: *** Waiting for unfinished jobs....
Traceback (most recent call last):
  File "./tests/docker/docker.py", line 669, in <module>
---
    raise CalledProcessError(retcode, cmd)
subprocess.CalledProcessError: Command '['sudo', '-n', 'docker', 'run', '--label', 'com.qemu.instance.uuid=7638277b753647899fcb0c31ef086305', '-u', '1001', '--security-opt', 'seccomp=unconfined', '--rm', '-e', 'TARGET_LIST=', '-e', 'EXTRA_CONFIGURE_OPTS=', '-e', 'V=', '-e', 'J=14', '-e', 'DEBUG=', '-e', 'SHOW_ENV=1', '-e', 'CCACHE_DIR=/var/tmp/ccache', '-v', '/home/patchew/.cache/qemu-docker-ccache:/var/tmp/ccache:z', '-v', '/var/tmp/patchew-tester-tmp-uuens814/src/docker-src.2020-06-25-06.13.41.17712:/var/tmp/qemu:z,ro', 'qemu:centos7', '/var/tmp/qemu/run', 'test-quick']' returned non-zero exit status 2.
filter=--filter=label=com.qemu.instance.uuid=7638277b753647899fcb0c31ef086305
make[1]: *** [docker-run] Error 1
make[1]: Leaving directory `/var/tmp/patchew-tester-tmp-uuens814/src'
make: *** [docker-run-test-quick@centos7] Error 2

real    2m49.544s
user    0m8.892s


The full log is available at
http://patchew.org/logs/20200625100430.22407-1-mhartmay@linux.ibm.com/testing.docker-quick@centos7/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-devel@redhat.com
Re: [RFC 0/4] Enable virtio-fs on s390x
Posted by Cornelia Huck 3 years, 10 months ago
On Thu, 25 Jun 2020 12:04:26 +0200
Marc Hartmayer <mhartmay@linux.ibm.com> wrote:

> This RFC is about enabling virtio-fs on s390x. For that we need
>  + some shim code (first patch), and we need
>  + libvhost-user to deal with virtio endiannes as mandated by the spec.
>  
> The second part is trickier, because unlike QEMU we are not certain
> about the guest's native endianness, which is needed to handle the
> legacy-interface appropriately. In fact, this is the reason why just
> RFC.
> 
> One of the open questions is whether to build separate versions, one
> for guest little endian and one for guest big endian, or do we want
> something like a command line option? (Digression on the libvirt
> modeling)
> 
> A third option would be to refuse legacy altogether.

The third option looks the most tempting to me. It is a new device, so
I don't think there's much of a case to be made for pre-virtio-1
support?


Re: [RFC 0/4] Enable virtio-fs on s390x
Posted by Halil Pasic 3 years, 10 months ago
On Thu, 25 Jun 2020 12:17:55 +0200
Cornelia Huck <cohuck@redhat.com> wrote:

> On Thu, 25 Jun 2020 12:04:26 +0200
> Marc Hartmayer <mhartmay@linux.ibm.com> wrote:
> 
> > This RFC is about enabling virtio-fs on s390x. For that we need
> >  + some shim code (first patch), and we need
> >  + libvhost-user to deal with virtio endiannes as mandated by the spec.
> >  
> > The second part is trickier, because unlike QEMU we are not certain
> > about the guest's native endianness, which is needed to handle the
> > legacy-interface appropriately. In fact, this is the reason why just
> > RFC.
> > 
> > One of the open questions is whether to build separate versions, one
> > for guest little endian and one for guest big endian, or do we want
> > something like a command line option? (Digression on the libvirt
> > modeling)
> > 
> > A third option would be to refuse legacy altogether.
> 
> The third option looks the most tempting to me. It is a new device, so
> I don't think there's much of a case to be made for pre-virtio-1
> support?
> 
> 

Yes, virtio-fs is a new device, but libvhost-user is not specific to
virtio-fs.

Regards,
Halil

Re: [RFC 0/4] Enable virtio-fs on s390x
Posted by Daniel P. Berrangé 3 years, 10 months ago
On Thu, Jun 25, 2020 at 12:04:26PM +0200, Marc Hartmayer wrote:
> This RFC is about enabling virtio-fs on s390x. For that we need
>  + some shim code (first patch), and we need
>  + libvhost-user to deal with virtio endiannes as mandated by the spec.
>  
> The second part is trickier, because unlike QEMU we are not certain
> about the guest's native endianness, which is needed to handle the
> legacy-interface appropriately. In fact, this is the reason why just
> RFC.
> 
> One of the open questions is whether to build separate versions, one
> for guest little endian and one for guest big endian, or do we want
> something like a command line option? (Digression on the libvirt
> modeling)

When you talk about  big vs little endian, are you referring to TCG
scenarios with mixed host/guest arch, or arches which can support
either endianess, or both ? i guess it doesn't matter actually, as
I think the latter forces a specific answer.

Considering that some architectures allow the guest OS to flip between
big & little endian as they boot, libvirt cannot know what endianess
the guest is using when it launches virtiofsd. It thus cannot pick
between two different endianness builds of virtiofsd automatically.
This would force the user to tell libvirt what arch the guest is using
at the time they define the guest. This is an undesirable restriction
for use cases where the admin of the guest OS has no direct control
over the host config.

IOW, I think the only practical answer is to have a single binary that
automagically does the right thing at runtime according to guest
endianess that currently is in use.

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|


Re: [RFC 0/4] Enable virtio-fs on s390x
Posted by Dr. David Alan Gilbert 3 years, 10 months ago
* Daniel P. Berrangé (berrange@redhat.com) wrote:
> On Thu, Jun 25, 2020 at 12:04:26PM +0200, Marc Hartmayer wrote:
> > This RFC is about enabling virtio-fs on s390x. For that we need
> >  + some shim code (first patch), and we need
> >  + libvhost-user to deal with virtio endiannes as mandated by the spec.
> >  
> > The second part is trickier, because unlike QEMU we are not certain
> > about the guest's native endianness, which is needed to handle the
> > legacy-interface appropriately. In fact, this is the reason why just
> > RFC.
> > 
> > One of the open questions is whether to build separate versions, one
> > for guest little endian and one for guest big endian, or do we want
> > something like a command line option? (Digression on the libvirt
> > modeling)
> 
> When you talk about  big vs little endian, are you referring to TCG
> scenarios with mixed host/guest arch, or arches which can support
> either endianess, or both ? i guess it doesn't matter actually, as
> I think the latter forces a specific answer.
> 
> Considering that some architectures allow the guest OS to flip between
> big & little endian as they boot, libvirt cannot know what endianess
> the guest is using when it launches virtiofsd. It thus cannot pick
> between two different endianness builds of virtiofsd automatically.
> This would force the user to tell libvirt what arch the guest is using
> at the time they define the guest. This is an undesirable restriction
> for use cases where the admin of the guest OS has no direct control
> over the host config.
> 
> IOW, I think the only practical answer is to have a single binary that
> automagically does the right thing at runtime according to guest
> endianess that currently is in use.

Modifying the fuse code in virtiofsd to handle cross-endian would be a
big job;  IMHO unless you really have a cross-endian need I'd just make
sure it deals ok with big<->big and little<->little.

Dave

> Regards,
> Daniel
> -- 
> |: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
> |: https://libvirt.org         -o-            https://fstop138.berrange.com :|
> |: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|
> 
> 
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK


Re: [RFC 0/4] Enable virtio-fs on s390x
Posted by Halil Pasic 3 years, 10 months ago
On Thu, 25 Jun 2020 11:19:35 +0100
Daniel P. Berrangé <berrange@redhat.com> wrote:

> On Thu, Jun 25, 2020 at 12:04:26PM +0200, Marc Hartmayer wrote:
> > This RFC is about enabling virtio-fs on s390x. For that we need
> >  + some shim code (first patch), and we need
> >  + libvhost-user to deal with virtio endiannes as mandated by the spec.
> >  
> > The second part is trickier, because unlike QEMU we are not certain
> > about the guest's native endianness, which is needed to handle the
> > legacy-interface appropriately. In fact, this is the reason why just
> > RFC.
> > 
> > One of the open questions is whether to build separate versions, one
> > for guest little endian and one for guest big endian, or do we want
> > something like a command line option? (Digression on the libvirt
> > modeling)
> 
> When you talk about  big vs little endian, are you referring to TCG
> scenarios with mixed host/guest arch, or arches which can support
> either endianess, or both ? i guess it doesn't matter actually, as
> I think the latter forces a specific answer.
> 

Our primary concern is big endian cpu and little endian virtio because
virto 1.0 or better. But since we talk about a lib here, everything is
possible.

> Considering that some architectures allow the guest OS to flip between
> big & little endian as they boot, libvirt cannot know what endianess
> the guest is using when it launches virtiofsd. It thus cannot pick
> between two different endianness builds of virtiofsd automatically.
> This would force the user to tell libvirt what arch the guest is using
> at the time they define the guest. This is an undesirable restriction
> for use cases where the admin of the guest OS has no direct control
> over the host config.
> 
> IOW, I think the only practical answer is to have a single binary that
> automagically does the right thing at runtime according to guest
> endianess that currently is in use.
> 

The problem is that AFAIK there is no way to figure out what is the right
thing at the moment. I guess QEMU should now, but not virtiofsd or
whatever userspace virtio device. But I will double check if there is
something in the protocol addressing this.

Regards,
Halil

Re: [RFC 0/4] Enable virtio-fs on s390x
Posted by Cornelia Huck 3 years, 10 months ago
On Thu, 25 Jun 2020 11:19:35 +0100
Daniel P. Berrangé <berrange@redhat.com> wrote:

> On Thu, Jun 25, 2020 at 12:04:26PM +0200, Marc Hartmayer wrote:
> > This RFC is about enabling virtio-fs on s390x. For that we need
> >  + some shim code (first patch), and we need
> >  + libvhost-user to deal with virtio endiannes as mandated by the spec.
> >  
> > The second part is trickier, because unlike QEMU we are not certain
> > about the guest's native endianness, which is needed to handle the
> > legacy-interface appropriately. In fact, this is the reason why just
> > RFC.
> > 
> > One of the open questions is whether to build separate versions, one
> > for guest little endian and one for guest big endian, or do we want
> > something like a command line option? (Digression on the libvirt
> > modeling)  
> 
> When you talk about  big vs little endian, are you referring to TCG
> scenarios with mixed host/guest arch, or arches which can support
> either endianess, or both ? i guess it doesn't matter actually, as
> I think the latter forces a specific answer.
> 
> Considering that some architectures allow the guest OS to flip between
> big & little endian as they boot, libvirt cannot know what endianess
> the guest is using when it launches virtiofsd. It thus cannot pick
> between two different endianness builds of virtiofsd automatically.
> This would force the user to tell libvirt what arch the guest is using
> at the time they define the guest. This is an undesirable restriction
> for use cases where the admin of the guest OS has no direct control
> over the host config.

Right, but that is in practice only a problem for legacy devices, isn't
it? The standard says that non-legacy devices use little-endian
everywhere; it's the legacy 'device endian' that is causing us
headaches.

Which leads to the question: Do we really need to support legacy
virtio-fs devices, or can we just force virtio-1, as many (most?) newer
virtio devices do?

> 
> IOW, I think the only practical answer is to have a single binary that
> automagically does the right thing at runtime according to guest
> endianess that currently is in use.
> 
> Regards,
> Daniel


Re: [RFC 0/4] Enable virtio-fs on s390x
Posted by Daniel P. Berrangé 3 years, 10 months ago
On Thu, Jun 25, 2020 at 12:31:36PM +0200, Cornelia Huck wrote:
> On Thu, 25 Jun 2020 11:19:35 +0100
> Daniel P. Berrangé <berrange@redhat.com> wrote:
> 
> > On Thu, Jun 25, 2020 at 12:04:26PM +0200, Marc Hartmayer wrote:
> > > This RFC is about enabling virtio-fs on s390x. For that we need
> > >  + some shim code (first patch), and we need
> > >  + libvhost-user to deal with virtio endiannes as mandated by the spec.
> > >  
> > > The second part is trickier, because unlike QEMU we are not certain
> > > about the guest's native endianness, which is needed to handle the
> > > legacy-interface appropriately. In fact, this is the reason why just
> > > RFC.
> > > 
> > > One of the open questions is whether to build separate versions, one
> > > for guest little endian and one for guest big endian, or do we want
> > > something like a command line option? (Digression on the libvirt
> > > modeling)  
> > 
> > When you talk about  big vs little endian, are you referring to TCG
> > scenarios with mixed host/guest arch, or arches which can support
> > either endianess, or both ? i guess it doesn't matter actually, as
> > I think the latter forces a specific answer.
> > 
> > Considering that some architectures allow the guest OS to flip between
> > big & little endian as they boot, libvirt cannot know what endianess
> > the guest is using when it launches virtiofsd. It thus cannot pick
> > between two different endianness builds of virtiofsd automatically.
> > This would force the user to tell libvirt what arch the guest is using
> > at the time they define the guest. This is an undesirable restriction
> > for use cases where the admin of the guest OS has no direct control
> > over the host config.
> 
> Right, but that is in practice only a problem for legacy devices, isn't
> it? The standard says that non-legacy devices use little-endian
> everywhere; it's the legacy 'device endian' that is causing us
> headaches.
> 
> Which leads to the question: Do we really need to support legacy
> virtio-fs devices, or can we just force virtio-1, as many (most?) newer
> virtio devices do?

I'd hope virtio-fs is already forced to modern only, as there's no legacy
PCI ID assigned to it in the spec.

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|


Re: [RFC 0/4] Enable virtio-fs on s390x
Posted by Cornelia Huck 3 years, 10 months ago
On Thu, 25 Jun 2020 11:39:24 +0100
Daniel P. Berrangé <berrange@redhat.com> wrote:

> On Thu, Jun 25, 2020 at 12:31:36PM +0200, Cornelia Huck wrote:
> > On Thu, 25 Jun 2020 11:19:35 +0100
> > Daniel P. Berrangé <berrange@redhat.com> wrote:
> >   
> > > On Thu, Jun 25, 2020 at 12:04:26PM +0200, Marc Hartmayer wrote:  
> > > > This RFC is about enabling virtio-fs on s390x. For that we need
> > > >  + some shim code (first patch), and we need
> > > >  + libvhost-user to deal with virtio endiannes as mandated by the spec.
> > > >  
> > > > The second part is trickier, because unlike QEMU we are not certain
> > > > about the guest's native endianness, which is needed to handle the
> > > > legacy-interface appropriately. In fact, this is the reason why just
> > > > RFC.
> > > > 
> > > > One of the open questions is whether to build separate versions, one
> > > > for guest little endian and one for guest big endian, or do we want
> > > > something like a command line option? (Digression on the libvirt
> > > > modeling)    
> > > 
> > > When you talk about  big vs little endian, are you referring to TCG
> > > scenarios with mixed host/guest arch, or arches which can support
> > > either endianess, or both ? i guess it doesn't matter actually, as
> > > I think the latter forces a specific answer.
> > > 
> > > Considering that some architectures allow the guest OS to flip between
> > > big & little endian as they boot, libvirt cannot know what endianess
> > > the guest is using when it launches virtiofsd. It thus cannot pick
> > > between two different endianness builds of virtiofsd automatically.
> > > This would force the user to tell libvirt what arch the guest is using
> > > at the time they define the guest. This is an undesirable restriction
> > > for use cases where the admin of the guest OS has no direct control
> > > over the host config.  
> > 
> > Right, but that is in practice only a problem for legacy devices, isn't
> > it? The standard says that non-legacy devices use little-endian
> > everywhere; it's the legacy 'device endian' that is causing us
> > headaches.
> > 
> > Which leads to the question: Do we really need to support legacy
> > virtio-fs devices, or can we just force virtio-1, as many (most?) newer
> > virtio devices do?  
> 
> I'd hope virtio-fs is already forced to modern only, as there's no legacy
> PCI ID assigned to it in the spec.

I did not find a call to virtio_pci_force_virtio_1(), so apparently not?