Re: virtio-vsock requires 'disable-legacy=on' in QEMU 5.1

[Stefano Garzarella]

On Mon, Aug 17, 2020 at 12:27:46PM +0200, Cornelia Huck wrote:
> On Thu, 13 Aug 2020 14:04:15 +0200
> Stefano Garzarella <sgarzare@redhat.com> wrote:
> 
> > On Thu, Aug 13, 2020 at 12:37:37PM +0200, Cornelia Huck wrote:
> > > On Thu, 13 Aug 2020 12:24:30 +0200
> > > Stefano Garzarella <sgarzare@redhat.com> wrote:
> > >   
> > > > On Thu, Aug 13, 2020 at 11:28:20AM +0200, Cornelia Huck wrote:  
> > > > > We basically have three possible ways to deal with this:
> > > > > 
> > > > > - Force it to modern (i.e., what you have been doing; would need the
> > > > >   equivalent changes in ccw as well.)    
> > > > 
> > > > Oo, thanks for pointing out ccw!
> > > > I don't know ccw well, in this case should we set dev->max_rev to 1 or 2
> > > > to force to modern?  
> > > 
> > > No, ->max_rev is the wrong side of the limit :) You want  
> > 
> > Well :-) Thanks!
> > 
> > > 
> > >     ccw_dev->force_revision_1 = true;
> > > 
> > > in _instance_init() (see e.g. virtio-ccw-gpu.c).
> > >   
> > > >   
> > > > >   Pro: looks like the cleanest approach.
> > > > >   Con: not sure if we would need backwards compatibility support,
> > > > >   which looks hairy.    
> > > > 
> > > > Not sure too.  
> > > 
> > > Yes, I'm not sure at all how to handle user-specified values for
> > > legacy/modern.
> 
> Thinking a bit more about it, I'm not sure whether we even *can*
> provide backwards compatibility: we have different autoconfigurations
> for PCI based upon where it is plugged, and ccw does not have a way to
> turn legacy on/off, except from within the code.

Yes, I discovered today for example that the PCIe bus set auto-legacy
mode to off.

> 
> > >   
> > > >   
> > > > > - Add vsock to the list of devices with legacy support.
> > > > >   Pro: Existing setups continue to work.
> > > > >   Con: If vsock is really virtio-1-only, we still carry around
> > > > >   possibly broken legacy support.    
> > > > 
> > > > I'm not sure it is virtio-1-only, but virtio-vsock was introduced in
> > > > 2016, so I supposed it is modern-only.  
> > > 
> > > Yes, I would guess so as well.
> > >   
> > > > 
> > > > How can I verify that? Maybe forcing legacy mode and run some tests.  
> > > 
> > > Probably yes. The likeliest area with issues is probably endianness, so
> > > maybe with something big endian in the mix?
> > >   
> > 
> > Yeah, I'll try this setup!
> > 
> > > >   
> > > > > - Do nothing, have users force legacy off. Bad idea, as ccw has no way
> > > > >   to do that on the command line.
> > > > > 
> > > > > The first option is probably best.
> 
> The first option is now "force modern, but with no backwards
> compatibility", which is not that great; but "allow legacy, even though
> it should not exist" is not particularly appealing, either... what a
> mess :(

Yeah, it's a mess :-( anyway I still prefer option 1, it seems a little
bit more correct to me.

> 
> > > > >    
> > > > 
> > > > Yeah, I agree with you!  
> > > 
> > > Yes, it's really a pity we only noticed this after the release; this
> > > was supposed to stop new devices with legacy support creeping in, not
> > > to break existing command lines :(
> > >   
> > 
> > Yes, I forgot to test vsock stuff before the release :-(
> > Maybe we should add some tests...
> 
> Speaking of tests: do you have a quick way to test vhost-vsock at hand?
> Maybe I should add it to my manual repertoire...
> 

Sure, maybe the quickest way is to use ncat. Starting from version 7.80,
it supports AF_VSOCK sockets:

    host$ ./qemu-system-x86_64 ... -device vhost-vsock-pci,guest-cid=5

    host$ ncat --vsock -l 1234

    # vsock address is <cid, port>, cid=2 is used always to reach the host
    guest$ ncat --vsock 2 1234

Other tests that I usually run are:
- iperf-vsock: https://github.com/stefano-garzarella/iperf-vsock
- vsock test suite in the Linux kernel (tools/testing/vsock)

Let me know if you want more details on these :-)

Thanks,
Stefano