| Message ID | 1295119664-25920-1-git-send-email-aurelien@aurel32.net |
|---|---|
| State | New |
| Headers | show |
On Sat, Jan 15, 2011 at 08:27:43PM +0100, Aurelien Jarno wrote: > virtio-net used to work on cross-endianness configurations, but doesn't > anymore with recent guest kernels, as the new features don't handle > endianness correctly. > > This patch fixes wrong conversion, and add missing ones to make > virtio-net working. Tested on the following configurations: > - i386 guest on x86_64 host > - ppc guest on x86_64 host > - i386 guest on mips host > - ppc guest on mips host > > Cc: Anthony Liguori <aliguori@us.ibm.com> > Signed-off-by: Aurelien Jarno <aurelien@aurel32.net> > --- > hw/virtio-net.c | 13 +++++++------ > 1 files changed, 7 insertions(+), 6 deletions(-) > > diff --git a/hw/virtio-net.c b/hw/virtio-net.c > index ec1bf8d..515fb19 100644 > --- a/hw/virtio-net.c > +++ b/hw/virtio-net.c > @@ -81,7 +81,7 @@ static void virtio_net_get_config(VirtIODevice *vdev, uint8_t *config) > VirtIONet *n = to_virtio_net(vdev); > struct virtio_net_config netcfg; > > - netcfg.status = n->status; > + netcfg.status = lduw_p(&n->status); > memcpy(netcfg.mac, n->mac, ETH_ALEN); > memcpy(config, &netcfg, sizeof(netcfg)); > } > @@ -340,7 +340,7 @@ static int virtio_net_handle_mac(VirtIONet *n, uint8_t cmd, > n->mac_table.multi_overflow = 0; > memset(n->mac_table.macs, 0, MAC_TABLE_ENTRIES * ETH_ALEN); > > - mac_data.entries = ldl_le_p(elem->out_sg[1].iov_base); > + mac_data.entries = ldl_p(elem->out_sg[1].iov_base); > > if (sizeof(mac_data.entries) + > (mac_data.entries * ETH_ALEN) > elem->out_sg[1].iov_len) > @@ -356,7 +356,7 @@ static int virtio_net_handle_mac(VirtIONet *n, uint8_t cmd, > > n->mac_table.first_multi = n->mac_table.in_use; > > - mac_data.entries = ldl_le_p(elem->out_sg[2].iov_base); > + mac_data.entries = ldl_p(elem->out_sg[2].iov_base); > > if (sizeof(mac_data.entries) + > (mac_data.entries * ETH_ALEN) > elem->out_sg[2].iov_len) > @@ -386,7 +386,7 @@ static int virtio_net_handle_vlan_table(VirtIONet *n, uint8_t cmd, > return VIRTIO_NET_ERR; > } > > - vid = lduw_le_p(elem->out_sg[1].iov_base); > + vid = lduw_p(elem->out_sg[1].iov_base); > > if (vid >= MAX_VLAN) > return VIRTIO_NET_ERR; > @@ -675,8 +675,9 @@ static ssize_t virtio_net_receive(VLANClientState *nc, const uint8_t *buf, size_ > virtqueue_fill(n->rx_vq, &elem, total, i++); > } > > - if (mhdr) > - mhdr->num_buffers = i; > + if (mhdr) { > + mhdr->num_buffers = lduw_p(&i); > + } > > virtqueue_flush(n->rx_vq, i); > virtio_notify(&n->vdev, n->rx_vq); > -- > 1.7.2.3 > Ping?
> -----Original Message----- > From: qemu-devel-bounces+yu.liu=freescale.com@nongnu.org > [mailto:qemu-devel-bounces+yu.liu=freescale.com@nongnu.org] > On Behalf Of Aurelien Jarno > Sent: Sunday, January 16, 2011 3:28 AM > To: qemu-devel@nongnu.org > Cc: Anthony Liguori; Aurelien Jarno > Subject: [Qemu-devel] [PATCH 1/2] virtio-net: fix > cross-endianness support > > virtio-net used to work on cross-endianness configurations, > but doesn't > anymore with recent guest kernels, as the new features don't handle > endianness correctly. > > This patch fixes wrong conversion, and add missing ones to make > virtio-net working. Tested on the following configurations: > - i386 guest on x86_64 host > - ppc guest on x86_64 host > - i386 guest on mips host > - ppc guest on mips host > > Cc: Anthony Liguori <aliguori@us.ibm.com> > Signed-off-by: Aurelien Jarno <aurelien@aurel32.net> > --- > hw/virtio-net.c | 13 +++++++------ > 1 files changed, 7 insertions(+), 6 deletions(-) > > diff --git a/hw/virtio-net.c b/hw/virtio-net.c > index ec1bf8d..515fb19 100644 > --- a/hw/virtio-net.c > +++ b/hw/virtio-net.c > @@ -81,7 +81,7 @@ static void > virtio_net_get_config(VirtIODevice *vdev, uint8_t *config) > VirtIONet *n = to_virtio_net(vdev); > struct virtio_net_config netcfg; > > - netcfg.status = n->status; > + netcfg.status = lduw_p(&n->status); > memcpy(netcfg.mac, n->mac, ETH_ALEN); > memcpy(config, &netcfg, sizeof(netcfg)); > } > @@ -340,7 +340,7 @@ static int > virtio_net_handle_mac(VirtIONet *n, uint8_t cmd, > n->mac_table.multi_overflow = 0; > memset(n->mac_table.macs, 0, MAC_TABLE_ENTRIES * ETH_ALEN); > > - mac_data.entries = ldl_le_p(elem->out_sg[1].iov_base); > + mac_data.entries = ldl_p(elem->out_sg[1].iov_base); > > if (sizeof(mac_data.entries) + > (mac_data.entries * ETH_ALEN) > elem->out_sg[1].iov_len) > @@ -356,7 +356,7 @@ static int > virtio_net_handle_mac(VirtIONet *n, uint8_t cmd, > > n->mac_table.first_multi = n->mac_table.in_use; > > - mac_data.entries = ldl_le_p(elem->out_sg[2].iov_base); > + mac_data.entries = ldl_p(elem->out_sg[2].iov_base); > Hello Aurelien, Not clear what is happen, but this commit break e500(PowerPC big endian) kvm. Looks like e500 is happy to use ldl_le_p(), but this patch change it to use ldl_be_p(). Any thoughts about that? Thanks, Yu
On Mon, Feb 28, 2011 at 08:43:41AM +0000, Liu Yu-B13201 wrote: > > > > -----Original Message----- > > From: qemu-devel-bounces+yu.liu=freescale.com@nongnu.org > > [mailto:qemu-devel-bounces+yu.liu=freescale.com@nongnu.org] > > On Behalf Of Aurelien Jarno > > Sent: Sunday, January 16, 2011 3:28 AM > > To: qemu-devel@nongnu.org > > Cc: Anthony Liguori; Aurelien Jarno > > Subject: [Qemu-devel] [PATCH 1/2] virtio-net: fix > > cross-endianness support > > > > virtio-net used to work on cross-endianness configurations, > > but doesn't > > anymore with recent guest kernels, as the new features don't handle > > endianness correctly. > > > > This patch fixes wrong conversion, and add missing ones to make > > virtio-net working. Tested on the following configurations: > > - i386 guest on x86_64 host > > - ppc guest on x86_64 host > > - i386 guest on mips host > > - ppc guest on mips host > > > > Cc: Anthony Liguori <aliguori@us.ibm.com> > > Signed-off-by: Aurelien Jarno <aurelien@aurel32.net> > > --- > > hw/virtio-net.c | 13 +++++++------ > > 1 files changed, 7 insertions(+), 6 deletions(-) > > > > diff --git a/hw/virtio-net.c b/hw/virtio-net.c > > index ec1bf8d..515fb19 100644 > > --- a/hw/virtio-net.c > > +++ b/hw/virtio-net.c > > @@ -81,7 +81,7 @@ static void > > virtio_net_get_config(VirtIODevice *vdev, uint8_t *config) > > VirtIONet *n = to_virtio_net(vdev); > > struct virtio_net_config netcfg; > > > > - netcfg.status = n->status; > > + netcfg.status = lduw_p(&n->status); > > memcpy(netcfg.mac, n->mac, ETH_ALEN); > > memcpy(config, &netcfg, sizeof(netcfg)); > > } > > @@ -340,7 +340,7 @@ static int > > virtio_net_handle_mac(VirtIONet *n, uint8_t cmd, > > n->mac_table.multi_overflow = 0; > > memset(n->mac_table.macs, 0, MAC_TABLE_ENTRIES * ETH_ALEN); > > > > - mac_data.entries = ldl_le_p(elem->out_sg[1].iov_base); > > + mac_data.entries = ldl_p(elem->out_sg[1].iov_base); > > > > if (sizeof(mac_data.entries) + > > (mac_data.entries * ETH_ALEN) > elem->out_sg[1].iov_len) > > @@ -356,7 +356,7 @@ static int > > virtio_net_handle_mac(VirtIONet *n, uint8_t cmd, > > > > n->mac_table.first_multi = n->mac_table.in_use; > > > > - mac_data.entries = ldl_le_p(elem->out_sg[2].iov_base); > > + mac_data.entries = ldl_p(elem->out_sg[2].iov_base); > > > > Hello Aurelien, Hi, > Not clear what is happen, but this commit break e500(PowerPC big endian) kvm. > Looks like e500 is happy to use ldl_le_p(), but this patch change it to use ldl_be_p(). I am sorry about that. I have just found I have done my tests on the big endian host using and old kernel, so which doesn't use this part of the code. > Ainy thoughts about that? > My patch basically fixed the big endian guest on little endian host and broke the big endian guest on big endian host support. The endianness handling in virtio-net is different than in virtio-blk. In virtio-blk we basically have to byteswap the data when the endianness of the guest is different than the one of the guest. If I am correct, in virtio-net it seems we have to byteswap the data solely depending on the guest endianness and independently on the host endianness. This is something quite weird that we don't have in cpu-all.h I am currently trying to write a patch, will come back to you when it's ready. Aurelien
On Thu, Mar 3, 2011 at 7:16 PM, Aurelien Jarno <aurelien@aurel32.net> wrote: > The endianness handling in virtio-net is different than in virtio-blk. > In virtio-blk we basically have to byteswap the data when the endianness > of the guest is different than the one of the guest. If I am correct, in > virtio-net it seems we have to byteswap the data solely depending on the > guest endianness and independently on the host endianness. This is > something quite weird that we don't have in cpu-all.h That sounds odd. Virtio is guest endian so the host must do guest<->host endian conversion. Which bits of code make you think otherwise? Stefan
On Thu, Mar 03, 2011 at 07:50:03PM +0000, Stefan Hajnoczi wrote: > On Thu, Mar 3, 2011 at 7:16 PM, Aurelien Jarno <aurelien@aurel32.net> wrote: > > The endianness handling in virtio-net is different than in virtio-blk. > > In virtio-blk we basically have to byteswap the data when the endianness > > of the guest is different than the one of the guest. If I am correct, in > > virtio-net it seems we have to byteswap the data solely depending on the > > guest endianness and independently on the host endianness. This is > > something quite weird that we don't have in cpu-all.h > > That sounds odd. Virtio is guest endian so the host must do > guest<->host endian conversion. Which bits of code make you think > otherwise? > That's exactly what my patch was doing, changing the ldx_le_p() into ldx_p(). In pratice it doesn't work. Basically, with ldlx_le_p(), it only works when the endianness of the guess and of the host are the same. With ldx_p() it only works when the host is little endian. That's how I conclude that byteswapping should only be done on big endian guests. I agree it's weird and should be validated by some real tests, I am planning to do that soon.
diff --git a/hw/virtio-net.c b/hw/virtio-net.c index ec1bf8d..515fb19 100644 --- a/hw/virtio-net.c +++ b/hw/virtio-net.c @@ -81,7 +81,7 @@ static void virtio_net_get_config(VirtIODevice *vdev, uint8_t *config) VirtIONet *n = to_virtio_net(vdev); struct virtio_net_config netcfg; - netcfg.status = n->status; + netcfg.status = lduw_p(&n->status); memcpy(netcfg.mac, n->mac, ETH_ALEN); memcpy(config, &netcfg, sizeof(netcfg)); } @@ -340,7 +340,7 @@ static int virtio_net_handle_mac(VirtIONet *n, uint8_t cmd, n->mac_table.multi_overflow = 0; memset(n->mac_table.macs, 0, MAC_TABLE_ENTRIES * ETH_ALEN); - mac_data.entries = ldl_le_p(elem->out_sg[1].iov_base); + mac_data.entries = ldl_p(elem->out_sg[1].iov_base); if (sizeof(mac_data.entries) + (mac_data.entries * ETH_ALEN) > elem->out_sg[1].iov_len) @@ -356,7 +356,7 @@ static int virtio_net_handle_mac(VirtIONet *n, uint8_t cmd, n->mac_table.first_multi = n->mac_table.in_use; - mac_data.entries = ldl_le_p(elem->out_sg[2].iov_base); + mac_data.entries = ldl_p(elem->out_sg[2].iov_base); if (sizeof(mac_data.entries) + (mac_data.entries * ETH_ALEN) > elem->out_sg[2].iov_len) @@ -386,7 +386,7 @@ static int virtio_net_handle_vlan_table(VirtIONet *n, uint8_t cmd, return VIRTIO_NET_ERR; } - vid = lduw_le_p(elem->out_sg[1].iov_base); + vid = lduw_p(elem->out_sg[1].iov_base); if (vid >= MAX_VLAN) return VIRTIO_NET_ERR; @@ -675,8 +675,9 @@ static ssize_t virtio_net_receive(VLANClientState *nc, const uint8_t *buf, size_ virtqueue_fill(n->rx_vq, &elem, total, i++); } - if (mhdr) - mhdr->num_buffers = i; + if (mhdr) { + mhdr->num_buffers = lduw_p(&i); + } virtqueue_flush(n->rx_vq, i); virtio_notify(&n->vdev, n->rx_vq);
virtio-net used to work on cross-endianness configurations, but doesn't anymore with recent guest kernels, as the new features don't handle endianness correctly. This patch fixes wrong conversion, and add missing ones to make virtio-net working. Tested on the following configurations: - i386 guest on x86_64 host - ppc guest on x86_64 host - i386 guest on mips host - ppc guest on mips host Cc: Anthony Liguori <aliguori@us.ibm.com> Signed-off-by: Aurelien Jarno <aurelien@aurel32.net> --- hw/virtio-net.c | 13 +++++++------ 1 files changed, 7 insertions(+), 6 deletions(-)