On Wed, Feb 25, 2015 at 02:50:22PM +1030, Rusty Russell wrote: > OK, I am trying to experiment with virtio 1.0 support using the > latest kernel and MST's qemu tree: > > https://git.kernel.org/cgit/virt/kvm/mst/qemu.git/?h=virtio-1.0 > > The first issue is that the device config endian was wrong (see > attached patch). > > I'm now setting up a BE guest on my x86 laptop, and a BE and LE guest > on a BE powerpc machine, to check that all combinations work correctly. > If others test too, that would be appreciated! > > Cheers, > Rusty. Thanks a lot for finding this! The issue is certainly there, though I think looking at guest features is not the right thing to do: drivers can access config before acking features. At least for PCI, it's very simple: we have a separate memory region for modern devices, we should just use a different accessor, not virtio_config_readw and friends. Untested patch sent (sorry about the untested part, a bit busy right now). > >From 95ac91554ed602f856a2a5fcc25eaffcad1b1c8d Mon Sep 17 00:00:00 2001 > From: Rusty Russell <rusty@xxxxxxxxxxxxxxx> > Date: Tue, 24 Feb 2015 14:47:44 +1030 > Subject: [PATCH] virtio_config_write*/virtio_config_read*: Don't endian swap > for virtio 1.0. > > Signed-off-by: Rusty Russell <rusty@xxxxxxxxxxxxxxx> > > diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c > index 079944c..882a31b 100644 > --- a/hw/virtio/virtio.c > +++ b/hw/virtio/virtio.c > @@ -662,7 +662,12 @@ uint32_t virtio_config_readw(VirtIODevice *vdev, uint32_t addr) > > k->get_config(vdev, vdev->config); > > - val = lduw_p(vdev->config + addr); > + /* Virtio 1.0 is always LE */ > + if (virtio_has_feature(vdev, VIRTIO_F_VERSION_1)) { > + val = lduw_le_p(vdev->config + addr); > + } else { > + val = lduw_p(vdev->config + addr); > + } > return val; > } > > @@ -677,7 +682,12 @@ uint32_t virtio_config_readl(VirtIODevice *vdev, uint32_t addr) > > k->get_config(vdev, vdev->config); > > - val = ldl_p(vdev->config + addr); > + /* Virtio 1.0 is always LE */ > + if (virtio_has_feature(vdev, VIRTIO_F_VERSION_1)) { > + val = ldl_le_p(vdev->config + addr); > + } else { > + val = ldl_p(vdev->config + addr); > + } > return val; > } > > @@ -706,7 +716,12 @@ void virtio_config_writew(VirtIODevice *vdev, uint32_t addr, uint32_t data) > return; > } > > - stw_p(vdev->config + addr, val); > + /* Virtio 1.0 is always LE */ > + if (virtio_has_feature(vdev, VIRTIO_F_VERSION_1)) { > + stw_le_p(vdev->config + addr, val); > + } else { > + stw_p(vdev->config + addr, val); > + } > > if (k->set_config) { > k->set_config(vdev, vdev->config); > @@ -722,7 +737,12 @@ void virtio_config_writel(VirtIODevice *vdev, uint32_t addr, uint32_t data) > return; > } > > - stl_p(vdev->config + addr, val); > + /* Virtio 1.0 is always LE */ > + if (virtio_has_feature(vdev, VIRTIO_F_VERSION_1)) { > + stl_le_p(vdev->config + addr, val); > + } else { > + stl_p(vdev->config + addr, val); > + } > > if (k->set_config) { > k->set_config(vdev, vdev->config); -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
