On Mon, Aug 17, 2020 at 12:27:46PM +0200, Cornelia Huck wrote:
> On Thu, 13 Aug 2020 14:04:15 +0200
> Stefano Garzarella <sgarz...@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 <sgarz...@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
