[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH RFC 4/7] vhost: set vring endianness for legacy
Michael S. Tsirkin
Re: [Qemu-devel] [PATCH RFC 4/7] vhost: set vring endianness for legacy virtio
Tue, 12 May 2015 18:40:35 +0200
On Tue, May 12, 2015 at 06:25:32PM +0200, Cornelia Huck wrote:
> On Tue, 12 May 2015 17:15:53 +0200
> "Michael S. Tsirkin" <address@hidden> wrote:
> > On Tue, May 12, 2015 at 03:25:30PM +0200, Cornelia Huck wrote:
> > > On Wed, 06 May 2015 14:08:02 +0200
> > > Greg Kurz <address@hidden> wrote:
> > >
> > > > Legacy virtio is native endian: if the guest and host endianness differ,
> > > > we have to tell vhost so it can swap bytes where appropriate. This is
> > > > done through a vhost ring ioctl.
> > > >
> > > > Signed-off-by: Greg Kurz <address@hidden>
> > > > ---
> > > > hw/virtio/vhost.c | 50
> > > > +++++++++++++++++++++++++++++++++++++++++++++++++-
> > > > 1 file changed, 49 insertions(+), 1 deletion(-)
> > > >
> > > > diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c
> > > > index 54851b7..1d7b939 100644
> > > > --- a/hw/virtio/vhost.c
> > > > +++ b/hw/virtio/vhost.c
> > > (...)
> > > > @@ -677,6 +700,16 @@ static int vhost_virtqueue_start(struct vhost_dev
> > > > *dev,
> > > > return -errno;
> > > > }
> > > >
> > > > + if (!virtio_has_feature(vdev, VIRTIO_F_VERSION_1) &&
> > >
> > > I think this should either go in after the virtio-1 base support (more
> > > feature bits etc.) or get a big fat comment and be touched up later.
> > > I'd prefer the first solution so it does not get forgotten, but I'm not
> > > sure when Michael plans to proceed with the virtio-1 patches (I think
> > > they're mostly fine already).
> > There are three main issues with virtio 1 patches that I am aware of.
> > One issue with virtio 1 patches as they are is with how features are
> > handled ATM. There are 3 types of features
> > a. virtio 1 only features
> > b. virtio 0 only features
> > c. shared features
> > and 3 types of devices
> > a. legacy device: has b+c features
> > b. modern device: has a+c features
> > c. transitional device: has a+c features but exposes
> > only c through the legacy interface
> Wouldn't a transitional device be able to expose b as well?
No because the virtio 1 spec says it shouldn't.
> > So I think a callback that gets features depending on guest
> > version isn't a good way to model it because fundamentally device
> > has one set of features.
> > A better way to model this is really just a single
> > host_features bitmask, and for transitional devices, a mask
> > hiding a features - which are so far all bits > 31, so maybe
> > for now we can just have a global mask.
> How would this work for transitional presenting a modern device - would
> you have a superset of bits and masks for legacy and modern?
Basically we expose through modern interface a superset
of bits exposed through legacy.
F_BAD for pci is probably the only exception.
> > We need to validate features at initialization time and make
> > sure they make sense, fail if not (sometimes we need to mask
> > features if they don't make sense - this is unfortunate
> > but might be needed for compatibility).
> > Moving host_features to virtio core would make all of the above
> > easier.
> I have started hacking up code that moves host_features, but I'm quite
> lost with all the different virtio versions floating around. Currently
> trying against master, but that of course ignores the virtio-1 issues.
Yes, I think we should focus on infrastructure cleanups in master first.
> > Second issue is migration, some of it is with migrating the new
> > features, so that's tied to the first one.
> There's also the used and avail addresses, but that kind of follows
> from virtio-1 support.
> > Third issue is fixing devices so they don't try to
> > access guest memory until DRIVER_OK is set.
> > This is surprisingly hard to do generally given need to support old
> > drivers which don't set DRIVER_OK or set it very late, and the fact that
> > we tied work-arounds for even older drivers which dont' set pci bus
> > master to the DRIVER_OK bit. I tried, and I'm close to giving up and
> > just checking guest ack for virtio 1, and ignoring DRIVER_OK requirement
> > if not there.
> If legacy survived like it is until now, it might be best to focus on
> modern devices for this.
I'm kind of unhappy that it's up to guest though as that controls
whether we run in modern mode. But yea.
[Qemu-devel] [PATCH RFC 3/7] virtio: introduce virtio_legacy_is_cross_endian(), Greg Kurz, 2015/05/06
[Qemu-devel] [PATCH RFC 5/7] tap: add VNET_LE/VNET_BE operations, Greg Kurz, 2015/05/06
[Qemu-devel] [PATCH RFC 4/7] vhost: set vring endianness for legacy virtio, Greg Kurz, 2015/05/06
[Qemu-devel] [PATCH RFC 6/7] vhost-net: tell tap backend about the vnet endianness, Greg Kurz, 2015/05/06
[Qemu-devel] [PATCH RFC 7/7] vhost_net: re-enable when cross endian, Greg Kurz, 2015/05/06