Skip to contentSkip to conversation

vhost: support for cross endian guests

PATCH Superseded Greg Kurz 3 participants 32 messages Apr 23, 2015 – Apr 24, 2015

Subsystems (6)
Network drivers KVM virtio core ABI/API +2 more subsystems

v5 of 6 Superseded Latest is v6 →

lore
Share & tools
lore

Greg Kurz

author
#0 lore raw

v5 cover vhost: support for cross endian guests

Hi,

This patchset allows vhost to be used with legacy virtio when guest and host have a different endianness. It is compatible with modern virtio and can be fully compiled out through kernel config.

FWIW, I could flawlessly kexec/reboot guests from ppc64 to ppc64le and back. I could also migrate from a ppc64 to a ppc64le host and back. No regressions on x86 as expected. My experimental QEMU tree is here:

https://github.com/gkurz/qemu.git vhost/cross-endian

I'd be glad if this series could make it to 4.1.

Cheers.

---

Greg Kurz (8):
virtio: introduce virtio_is_little_endian() helper
tun: add tun_is_little_endian() helper
macvtap: introduce macvtap_is_little_endian() helper
vringh: introduce vringh_is_little_endian() helper
vhost: introduce vhost_is_little_endian() helper
virtio: add explicit big-endian support to memory accessors
vhost: cross-endian support for legacy devices macvtap/tun: cross-endian support for little-endian hosts

 drivers/net/Kconfig              |   14 ++++++
 drivers/net/macvtap.c            |   68 +++++++++++++++++++++++++++++-
 drivers/net/tun.c                |   70 ++++++++++++++++++++++++++++++-
 drivers/vhost/Kconfig            |   15 +++++++
 drivers/vhost/vhost.c            |   86 ++++++++++++++++++++++++++++++++++++++
 drivers/vhost/vhost.h            |   25 ++++++++---
 include/linux/virtio_byteorder.h |   24 ++++++-----
 include/linux/virtio_config.h    |   20 ++++++---
 include/linux/vringh.h           |   17 +++++---
 include/uapi/linux/if_tun.h      |    6 +++
 include/uapi/linux/vhost.h       |   12 +++++
 11 files changed, 324 insertions(+), 33 deletions(-)

--
Greg

author
#1 lore raw

PATCH v5 1/8 virtio: introduce virtio_is_little_endian() helper

Signed-off-by Greg Kurz <[email protected]>
---
 include/linux/virtio_config.h |   17 +++++++++++------
 1 file changed, 11 insertions(+), 6 deletions(-)
include/linux/virtio_config.h +11 -6
index ca3ed78..bd1a582 100644
@@ -205,35 +205,40 @@ int virtqueue_set_affinity(struct virtqueue *vq, int cpu)
 	return 0;
 }
 
+static inline bool virtio_is_little_endian(struct virtio_device *vdev)
+{
+	return virtio_has_feature(vdev, VIRTIO_F_VERSION_1);
+}
+
 /* Memory accessors */
 static inline u16 virtio16_to_cpu(struct virtio_device *vdev, __virtio16 val)
 {
-	return __virtio16_to_cpu(virtio_has_feature(vdev, VIRTIO_F_VERSION_1), val);
+	return __virtio16_to_cpu(virtio_is_little_endian(vdev), val);
 }
 
 static inline __virtio16 cpu_to_virtio16(struct virtio_device *vdev, u16 val)
 {
-	return __cpu_to_virtio16(virtio_has_feature(vdev, VIRTIO_F_VERSION_1), val);
+	return __cpu_to_virtio16(virtio_is_little_endian(vdev), val);
 }
 
 static inline u32 virtio32_to_cpu(struct virtio_device *vdev, __virtio32 val)
 {
-	return __virtio32_to_cpu(virtio_has_feature(vdev, VIRTIO_F_VERSION_1), val);
+	return __virtio32_to_cpu(virtio_is_little_endian(vdev), val);
 }
 
 static inline __virtio32 cpu_to_virtio32(struct virtio_device *vdev, u32 val)
 {
-	return __cpu_to_virtio32(virtio_has_feature(vdev, VIRTIO_F_VERSION_1), val);
+	return __cpu_to_virtio32(virtio_is_little_endian(vdev), val);
 }
 
 static inline u64 virtio64_to_cpu(struct virtio_device *vdev, __virtio64 val)
 {
-	return __virtio64_to_cpu(virtio_has_feature(vdev, VIRTIO_F_VERSION_1), val);
+	return __virtio64_to_cpu(virtio_is_little_endian(vdev), val);
 }
 
 static inline __virtio64 cpu_to_virtio64(struct virtio_device *vdev, u64 val)
 {
-	return __cpu_to_virtio64(virtio_has_feature(vdev, VIRTIO_F_VERSION_1), val);
+	return __cpu_to_virtio64(virtio_is_little_endian(vdev), val);
 }
 
 /* Config space accessors. */
#2 lore raw

Re: [PATCH v5 1/8] virtio: introduce virtio_is_little_endian() helper

Am Thu, 23 Apr 2015 17:26:20 +0200
schrieb Greg Kurz <[email protected]>:

Quoted 10 lines
Signed-off-by Greg Kurz <[email protected]>
---
 include/linux/virtio_config.h |   17 +++++++++++------
 1 file changed, 11 insertions(+), 6 deletions(-)
include/linux/virtio_config.h +0 -0
index ca3ed78..bd1a582 100644

include/linux/virtio_config.h int virtqueue_set_affinity(struct virtqueue *vq, int cpu) · line 211

Quoted 7 lines
include/linux/virtio_config.h +4 -0
@@ -205,35 +205,40 @@ int virtqueue_set_affinity(struct virtqueue *vq, int cpu)
 	return 0;
 }
 
+static inline bool virtio_is_little_endian(struct virtio_device *vdev)
+{
+	return virtio_has_feature(vdev, VIRTIO_F_VERSION_1);
+}

So this function returns false when _not_ using version 1, but running on a little endian host + guest? Sounds confusing. Maybe you could name it "virtio_is_v1()" or so instead?

Thomas

#3 lore raw

Re: [PATCH v5 1/8] virtio: introduce virtio_is_little_endian() helper

Am Thu, 23 Apr 2015 19:22:15 +0200
schrieb Thomas Huth <[email protected]>:

Quoted 16 lines

Am Thu, 23 Apr 2015 17:26:20 +0200
schrieb Greg Kurz <[email protected]>:

Quoted 13 lines
Signed-off-by Greg Kurz <[email protected]>
---
 include/linux/virtio_config.h |   17 +++++++++++------
 1 file changed, 11 insertions(+), 6 deletions(-)
include/linux/virtio_config.h +0 -0
index ca3ed78..bd1a582 100644
@@ -205,35 +205,40 @@ int virtqueue_set_affinity(struct virtqueue *vq, int cpu)
 	return 0;
 }
Quoted 8 lines

include/linux/virtio_config.h int virtqueue_set_affinity(struct virtqueue *vq, int cpu) · line 211

Quoted 4 lines
include/linux/virtio_config.h +4 -0
+static inline bool virtio_is_little_endian(struct virtio_device *vdev)
+{
+	return virtio_has_feature(vdev, VIRTIO_F_VERSION_1);
+}

So this function returns false when _not_ using version 1, but running on a little endian host + guest? Sounds confusing. Maybe you could name it "virtio_is_v1()" or so instead?

Ah, never mind, I should have looked at patch 6 first, then it makes sense. (maybe you could put a note to the later patch in this patch description?)

Thomas

#4 lore raw

Re: [PATCH v5 1/8] virtio: introduce virtio_is_little_endian() helper

In reply to Thomas Huth

Am Thu, 23 Apr 2015 19:22:15 +0200
schrieb Thomas Huth <[email protected]>:

Quoted 16 lines

Am Thu, 23 Apr 2015 17:26:20 +0200
schrieb Greg Kurz <[email protected]>:

Quoted 13 lines
Signed-off-by Greg Kurz <[email protected]>
---
 include/linux/virtio_config.h |   17 +++++++++++------
 1 file changed, 11 insertions(+), 6 deletions(-)
include/linux/virtio_config.h +0 -0
index ca3ed78..bd1a582 100644
@@ -205,35 +205,40 @@ int virtqueue_set_affinity(struct virtqueue *vq, int cpu)
 	return 0;
 }
Quoted 8 lines

include/linux/virtio_config.h int virtqueue_set_affinity(struct virtqueue *vq, int cpu) · line 211

Quoted 4 lines
include/linux/virtio_config.h +4 -0
+static inline bool virtio_is_little_endian(struct virtio_device *vdev)
+{
+	return virtio_has_feature(vdev, VIRTIO_F_VERSION_1);
+}

So this function returns false when _not_ using version 1, but running on a little endian host + guest? Sounds confusing. Maybe you could name it "virtio_is_v1()" or so instead?

Ah, never mind, I should have looked at patch 6 first, then it makes sense. (maybe you could put a note to the later patch in this patch description?)

Thomas

#5 lore raw

Re: [PATCH v5 1/8] virtio: introduce virtio_is_little_endian() helper

In reply to Greg Kurz

Am Thu, 23 Apr 2015 17:26:20 +0200
schrieb Greg Kurz <[email protected]>:

Quoted 10 lines
Signed-off-by Greg Kurz <[email protected]>
---
 include/linux/virtio_config.h |   17 +++++++++++------
 1 file changed, 11 insertions(+), 6 deletions(-)
include/linux/virtio_config.h +0 -0
index ca3ed78..bd1a582 100644

include/linux/virtio_config.h int virtqueue_set_affinity(struct virtqueue *vq, int cpu) · line 211

Quoted 7 lines
include/linux/virtio_config.h +4 -0
@@ -205,35 +205,40 @@ int virtqueue_set_affinity(struct virtqueue *vq, int cpu)
 	return 0;
 }
 
+static inline bool virtio_is_little_endian(struct virtio_device *vdev)
+{
+	return virtio_has_feature(vdev, VIRTIO_F_VERSION_1);
+}

So this function returns false when _not_ using version 1, but running on a little endian host + guest? Sounds confusing. Maybe you could name it "virtio_is_v1()" or so instead?

Thomas

Reviewed-by on v5 1/8
#6 lore raw

Re: [PATCH v5 1/8] virtio: introduce virtio_is_little_endian() helper

In reply to Greg Kurz
Quoted 44 lines · On Thu, 23 Apr 2015 17:26:20 +0200 Greg Kurz <[email protected]> wrote:
Signed-off-by Greg Kurz <[email protected]>
---
 include/linux/virtio_config.h |   17 +++++++++++------
 1 file changed, 11 insertions(+), 6 deletions(-)
include/linux/virtio_config.h +9 -4
index ca3ed78..bd1a582 100644
@@ -205,35 +205,40 @@ int virtqueue_set_affinity(struct virtqueue *vq, int cpu)
 	return 0;
 }
 
+static inline bool virtio_is_little_endian(struct virtio_device *vdev)
+{
+	return virtio_has_feature(vdev, VIRTIO_F_VERSION_1);
+}
+
 /* Memory accessors */
 static inline u16 virtio16_to_cpu(struct virtio_device *vdev, __virtio16 val)
 {
-	return __virtio16_to_cpu(virtio_has_feature(vdev, VIRTIO_F_VERSION_1), val);
+	return __virtio16_to_cpu(virtio_is_little_endian(vdev), val);
 }
 
 static inline __virtio16 cpu_to_virtio16(struct virtio_device *vdev, u16 val)
 {
-	return __cpu_to_virtio16(virtio_has_feature(vdev, VIRTIO_F_VERSION_1), val);
+	return __cpu_to_virtio16(virtio_is_little_endian(vdev), val);
 }
 
 static inline u32 virtio32_to_cpu(struct virtio_device *vdev, __virtio32 val)
 {
-	return __virtio32_to_cpu(virtio_has_feature(vdev, VIRTIO_F_VERSION_1), val);
+	return __virtio32_to_cpu(virtio_is_little_endian(vdev), val);
 }
 
 static inline __virtio32 cpu_to_virtio32(struct virtio_device *vdev, u32 val)
 {
-	return __cpu_to_virtio32(virtio_has_feature(vdev, VIRTIO_F_VERSION_1), val);
+	return __cpu_to_virtio32(virtio_is_little_endian(vdev), val);
 }
 
 static inline u64 virtio64_to_cpu(struct virtio_device *vdev, __virtio64 val)

include/linux/virtio_config.h int virtqueue_set_affinity(struct virtqueue *vq, int cpu) · line 242

Quoted 10 lines
include/linux/virtio_config.h +2 -2
 {
-	return __virtio64_to_cpu(virtio_has_feature(vdev, VIRTIO_F_VERSION_1), val);
+	return __virtio64_to_cpu(virtio_is_little_endian(vdev), val);
 }
 
 static inline __virtio64 cpu_to_virtio64(struct virtio_device *vdev, u64 val)
 {
-	return __cpu_to_virtio64(virtio_has_feature(vdev, VIRTIO_F_VERSION_1), val);
+	return __cpu_to_virtio64(virtio_is_little_endian(vdev), val);
 }
Reviewed-by Thomas Huth <[email protected]>
Reviewed-by on v5 1/8
#7 lore raw

Re: [PATCH v5 1/8] virtio: introduce virtio_is_little_endian() helper

In reply to Greg Kurz
Quoted 44 lines · On Thu, 23 Apr 2015 17:26:20 +0200 Greg Kurz <[email protected]> wrote:
Signed-off-by Greg Kurz <[email protected]>
---
 include/linux/virtio_config.h |   17 +++++++++++------
 1 file changed, 11 insertions(+), 6 deletions(-)
include/linux/virtio_config.h +9 -4
index ca3ed78..bd1a582 100644
@@ -205,35 +205,40 @@ int virtqueue_set_affinity(struct virtqueue *vq, int cpu)
 	return 0;
 }
 
+static inline bool virtio_is_little_endian(struct virtio_device *vdev)
+{
+	return virtio_has_feature(vdev, VIRTIO_F_VERSION_1);
+}
+
 /* Memory accessors */
 static inline u16 virtio16_to_cpu(struct virtio_device *vdev, __virtio16 val)
 {
-	return __virtio16_to_cpu(virtio_has_feature(vdev, VIRTIO_F_VERSION_1), val);
+	return __virtio16_to_cpu(virtio_is_little_endian(vdev), val);
 }
 
 static inline __virtio16 cpu_to_virtio16(struct virtio_device *vdev, u16 val)
 {
-	return __cpu_to_virtio16(virtio_has_feature(vdev, VIRTIO_F_VERSION_1), val);
+	return __cpu_to_virtio16(virtio_is_little_endian(vdev), val);
 }
 
 static inline u32 virtio32_to_cpu(struct virtio_device *vdev, __virtio32 val)
 {
-	return __virtio32_to_cpu(virtio_has_feature(vdev, VIRTIO_F_VERSION_1), val);
+	return __virtio32_to_cpu(virtio_is_little_endian(vdev), val);
 }
 
 static inline __virtio32 cpu_to_virtio32(struct virtio_device *vdev, u32 val)
 {
-	return __cpu_to_virtio32(virtio_has_feature(vdev, VIRTIO_F_VERSION_1), val);
+	return __cpu_to_virtio32(virtio_is_little_endian(vdev), val);
 }
 
 static inline u64 virtio64_to_cpu(struct virtio_device *vdev, __virtio64 val)

include/linux/virtio_config.h int virtqueue_set_affinity(struct virtqueue *vq, int cpu) · line 242

Quoted 10 lines
include/linux/virtio_config.h +2 -2
 {
-	return __virtio64_to_cpu(virtio_has_feature(vdev, VIRTIO_F_VERSION_1), val);
+	return __virtio64_to_cpu(virtio_is_little_endian(vdev), val);
 }
 
 static inline __virtio64 cpu_to_virtio64(struct virtio_device *vdev, u64 val)
 {
-	return __cpu_to_virtio64(virtio_has_feature(vdev, VIRTIO_F_VERSION_1), val);
+	return __cpu_to_virtio64(virtio_is_little_endian(vdev), val);
 }
Reviewed-by Thomas Huth <[email protected]>
author
#8 lore raw

PATCH v5 2/8 tun: add tun_is_little_endian() helper

In reply to Greg Kurz
Signed-off-by Greg Kurz <[email protected]>
---
 drivers/net/tun.c |    9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)
drivers/net/tun.c +7 -2
index 857dca4..3c3d6c0 100644
@@ -206,14 +206,19 @@ struct tun_struct {
 	u32 flow_count;
 };
 
+static inline bool tun_is_little_endian(struct tun_struct *tun)
+{
+	return tun->flags & TUN_VNET_LE;
+}
+
 static inline u16 tun16_to_cpu(struct tun_struct *tun, __virtio16 val)
 {
-	return __virtio16_to_cpu(tun->flags & TUN_VNET_LE, val);
+	return __virtio16_to_cpu(tun_is_little_endian(tun), val);
 }
 
 static inline __virtio16 cpu_to_tun16(struct tun_struct *tun, u16 val)
 {
-	return __cpu_to_virtio16(tun->flags & TUN_VNET_LE, val);
+	return __cpu_to_virtio16(tun_is_little_endian(tun), val);
 }
 
 static inline u32 tun_hashfn(u32 rxhash)
Reviewed-by on v5 2/8
#9 lore raw

Re: [PATCH v5 2/8] tun: add tun_is_little_endian() helper

Quoted 19 lines · On Thu, 23 Apr 2015 17:26:30 +0200 Greg Kurz <[email protected]> wrote:
Signed-off-by Greg Kurz <[email protected]>
---
 drivers/net/tun.c |    9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)
drivers/net/tun.c +5 -0
index 857dca4..3c3d6c0 100644
@@ -206,14 +206,19 @@ struct tun_struct {
 	u32 flow_count;
 };
 
+static inline bool tun_is_little_endian(struct tun_struct *tun)
+{
+	return tun->flags & TUN_VNET_LE;
+}
+
 static inline u16 tun16_to_cpu(struct tun_struct *tun, __virtio16 val)

drivers/net/tun.c struct tun_struct { · line 222

Quoted 10 lines
drivers/net/tun.c +2 -2
 {
-	return __virtio16_to_cpu(tun->flags & TUN_VNET_LE, val);
+	return __virtio16_to_cpu(tun_is_little_endian(tun), val);
 }
 
 static inline __virtio16 cpu_to_tun16(struct tun_struct *tun, u16 val)
 {
-	return __cpu_to_virtio16(tun->flags & TUN_VNET_LE, val);
+	return __cpu_to_virtio16(tun_is_little_endian(tun), val);
 }
Reviewed-by Thomas Huth <[email protected]>
Reviewed-by on v5 2/8
#10 lore raw

Re: [PATCH v5 2/8] tun: add tun_is_little_endian() helper

In reply to Greg Kurz
Quoted 19 lines · On Thu, 23 Apr 2015 17:26:30 +0200 Greg Kurz <[email protected]> wrote:
Signed-off-by Greg Kurz <[email protected]>
---
 drivers/net/tun.c |    9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)
drivers/net/tun.c +5 -0
index 857dca4..3c3d6c0 100644
@@ -206,14 +206,19 @@ struct tun_struct {
 	u32 flow_count;
 };
 
+static inline bool tun_is_little_endian(struct tun_struct *tun)
+{
+	return tun->flags & TUN_VNET_LE;
+}
+
 static inline u16 tun16_to_cpu(struct tun_struct *tun, __virtio16 val)

drivers/net/tun.c struct tun_struct { · line 222

Quoted 10 lines
drivers/net/tun.c +2 -2
 {
-	return __virtio16_to_cpu(tun->flags & TUN_VNET_LE, val);
+	return __virtio16_to_cpu(tun_is_little_endian(tun), val);
 }
 
 static inline __virtio16 cpu_to_tun16(struct tun_struct *tun, u16 val)
 {
-	return __cpu_to_virtio16(tun->flags & TUN_VNET_LE, val);
+	return __cpu_to_virtio16(tun_is_little_endian(tun), val);
 }
Reviewed-by Thomas Huth <[email protected]>
author
#11 lore raw

PATCH v5 3/8 macvtap: introduce macvtap_is_little_endian() helper

In reply to Greg Kurz
Signed-off-by Greg Kurz <[email protected]>
---
 drivers/net/macvtap.c |    9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)
drivers/net/macvtap.c +7 -2
index 27ecc5c..a2f2958 100644
@@ -49,14 +49,19 @@ struct macvtap_queue {
 
 #define MACVTAP_VNET_LE 0x80000000
 
+static inline bool macvtap_is_little_endian(struct macvtap_queue *q)
+{
+	return q->flags & MACVTAP_VNET_LE;
+}
+
 static inline u16 macvtap16_to_cpu(struct macvtap_queue *q, __virtio16 val)
 {
-	return __virtio16_to_cpu(q->flags & MACVTAP_VNET_LE, val);
+	return __virtio16_to_cpu(macvtap_is_little_endian(q), val);
 }
 
 static inline __virtio16 cpu_to_macvtap16(struct macvtap_queue *q, u16 val)
 {
-	return __cpu_to_virtio16(q->flags & MACVTAP_VNET_LE, val);
+	return __cpu_to_virtio16(macvtap_is_little_endian(q), val);
 }
 
 static struct proto macvtap_proto = {
Reviewed-by on v5 3/8
#12 lore raw

Re: [PATCH v5 3/8] macvtap: introduce macvtap_is_little_endian() helper

Quoted 19 lines · On Thu, 23 Apr 2015 17:26:41 +0200 Greg Kurz <[email protected]> wrote:
Signed-off-by Greg Kurz <[email protected]>
---
 drivers/net/macvtap.c |    9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)
drivers/net/macvtap.c +5 -0
index 27ecc5c..a2f2958 100644
@@ -49,14 +49,19 @@ struct macvtap_queue {
 
 #define MACVTAP_VNET_LE 0x80000000
 
+static inline bool macvtap_is_little_endian(struct macvtap_queue *q)
+{
+	return q->flags & MACVTAP_VNET_LE;
+}
+
 static inline u16 macvtap16_to_cpu(struct macvtap_queue *q, __virtio16 val)

drivers/net/macvtap.c struct macvtap_queue { · line 65

Quoted 10 lines
drivers/net/macvtap.c +2 -2
 {
-	return __virtio16_to_cpu(q->flags & MACVTAP_VNET_LE, val);
+	return __virtio16_to_cpu(macvtap_is_little_endian(q), val);
 }
 
 static inline __virtio16 cpu_to_macvtap16(struct macvtap_queue *q, u16 val)
 {
-	return __cpu_to_virtio16(q->flags & MACVTAP_VNET_LE, val);
+	return __cpu_to_virtio16(macvtap_is_little_endian(q), val);
 }
Reviewed-by Thomas Huth <[email protected]>
Reviewed-by on v5 3/8
#13 lore raw

Re: [PATCH v5 3/8] macvtap: introduce macvtap_is_little_endian() helper

In reply to Greg Kurz
Quoted 19 lines · On Thu, 23 Apr 2015 17:26:41 +0200 Greg Kurz <[email protected]> wrote:
Signed-off-by Greg Kurz <[email protected]>
---
 drivers/net/macvtap.c |    9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)
drivers/net/macvtap.c +5 -0
index 27ecc5c..a2f2958 100644
@@ -49,14 +49,19 @@ struct macvtap_queue {
 
 #define MACVTAP_VNET_LE 0x80000000
 
+static inline bool macvtap_is_little_endian(struct macvtap_queue *q)
+{
+	return q->flags & MACVTAP_VNET_LE;
+}
+
 static inline u16 macvtap16_to_cpu(struct macvtap_queue *q, __virtio16 val)

drivers/net/macvtap.c struct macvtap_queue { · line 65

Quoted 10 lines
drivers/net/macvtap.c +2 -2
 {
-	return __virtio16_to_cpu(q->flags & MACVTAP_VNET_LE, val);
+	return __virtio16_to_cpu(macvtap_is_little_endian(q), val);
 }
 
 static inline __virtio16 cpu_to_macvtap16(struct macvtap_queue *q, u16 val)
 {
-	return __cpu_to_virtio16(q->flags & MACVTAP_VNET_LE, val);
+	return __cpu_to_virtio16(macvtap_is_little_endian(q), val);
 }
Reviewed-by Thomas Huth <[email protected]>
author
#14 lore raw

PATCH v5 4/8 vringh: introduce vringh_is_little_endian() helper

In reply to Greg Kurz
Signed-off-by Greg Kurz <[email protected]>
---
 include/linux/vringh.h |   17 +++++++++++------
 1 file changed, 11 insertions(+), 6 deletions(-)
include/linux/vringh.h +11 -6
index a3fa537..3ed62ef 100644
@@ -226,33 +226,38 @@ static inline void vringh_notify(struct vringh *vrh)
 		vrh->notify(vrh);
 }
 
+static inline bool vringh_is_little_endian(const struct vringh *vrh)
+{
+	return vrh->little_endian;
+}
+
 static inline u16 vringh16_to_cpu(const struct vringh *vrh, __virtio16 val)
 {
-	return __virtio16_to_cpu(vrh->little_endian, val);
+	return __virtio16_to_cpu(vringh_is_little_endian(vrh), val);
 }
 
 static inline __virtio16 cpu_to_vringh16(const struct vringh *vrh, u16 val)
 {
-	return __cpu_to_virtio16(vrh->little_endian, val);
+	return __cpu_to_virtio16(vringh_is_little_endian(vrh), val);
 }
 
 static inline u32 vringh32_to_cpu(const struct vringh *vrh, __virtio32 val)
 {
-	return __virtio32_to_cpu(vrh->little_endian, val);
+	return __virtio32_to_cpu(vringh_is_little_endian(vrh), val);
 }
 
 static inline __virtio32 cpu_to_vringh32(const struct vringh *vrh, u32 val)
 {
-	return __cpu_to_virtio32(vrh->little_endian, val);
+	return __cpu_to_virtio32(vringh_is_little_endian(vrh), val);
 }
 
 static inline u64 vringh64_to_cpu(const struct vringh *vrh, __virtio64 val)
 {
-	return __virtio64_to_cpu(vrh->little_endian, val);
+	return __virtio64_to_cpu(vringh_is_little_endian(vrh), val);
 }
 
 static inline __virtio64 cpu_to_vringh64(const struct vringh *vrh, u64 val)
 {
-	return __cpu_to_virtio64(vrh->little_endian, val);
+	return __cpu_to_virtio64(vringh_is_little_endian(vrh), val);
 }
 #endif /* _LINUX_VRINGH_H */
Reviewed-by on v5 4/8
#15 lore raw

Re: [PATCH v5 4/8] vringh: introduce vringh_is_little_endian() helper

Quoted 44 lines · On Thu, 23 Apr 2015 17:26:52 +0200 Greg Kurz <[email protected]> wrote:
Signed-off-by Greg Kurz <[email protected]>
---
 include/linux/vringh.h |   17 +++++++++++------
 1 file changed, 11 insertions(+), 6 deletions(-)
include/linux/vringh.h +9 -4
index a3fa537..3ed62ef 100644
@@ -226,33 +226,38 @@ static inline void vringh_notify(struct vringh *vrh)
 		vrh->notify(vrh);
 }
 
+static inline bool vringh_is_little_endian(const struct vringh *vrh)
+{
+	return vrh->little_endian;
+}
+
 static inline u16 vringh16_to_cpu(const struct vringh *vrh, __virtio16 val)
 {
-	return __virtio16_to_cpu(vrh->little_endian, val);
+	return __virtio16_to_cpu(vringh_is_little_endian(vrh), val);
 }
 
 static inline __virtio16 cpu_to_vringh16(const struct vringh *vrh, u16 val)
 {
-	return __cpu_to_virtio16(vrh->little_endian, val);
+	return __cpu_to_virtio16(vringh_is_little_endian(vrh), val);
 }
 
 static inline u32 vringh32_to_cpu(const struct vringh *vrh, __virtio32 val)
 {
-	return __virtio32_to_cpu(vrh->little_endian, val);
+	return __virtio32_to_cpu(vringh_is_little_endian(vrh), val);
 }
 
 static inline __virtio32 cpu_to_vringh32(const struct vringh *vrh, u32 val)
 {
-	return __cpu_to_virtio32(vrh->little_endian, val);
+	return __cpu_to_virtio32(vringh_is_little_endian(vrh), val);
 }
 
 static inline u64 vringh64_to_cpu(const struct vringh *vrh, __virtio64 val)
 {

include/linux/vringh.h static inline void vringh_notify(struct vringh *vrh) · line 263

Quoted 10 lines
include/linux/vringh.h +2 -2
-	return __virtio64_to_cpu(vrh->little_endian, val);
+	return __virtio64_to_cpu(vringh_is_little_endian(vrh), val);
 }
 
 static inline __virtio64 cpu_to_vringh64(const struct vringh *vrh, u64 val)
 {
-	return __cpu_to_virtio64(vrh->little_endian, val);
+	return __cpu_to_virtio64(vringh_is_little_endian(vrh), val);
 }
 #endif /* _LINUX_VRINGH_H */
Reviewed-by Thomas Huth <[email protected]>
Reviewed-by on v5 4/8
#16 lore raw

Re: [PATCH v5 4/8] vringh: introduce vringh_is_little_endian() helper

In reply to Greg Kurz
Quoted 44 lines · On Thu, 23 Apr 2015 17:26:52 +0200 Greg Kurz <[email protected]> wrote:
Signed-off-by Greg Kurz <[email protected]>
---
 include/linux/vringh.h |   17 +++++++++++------
 1 file changed, 11 insertions(+), 6 deletions(-)
include/linux/vringh.h +9 -4
index a3fa537..3ed62ef 100644
@@ -226,33 +226,38 @@ static inline void vringh_notify(struct vringh *vrh)
 		vrh->notify(vrh);
 }
 
+static inline bool vringh_is_little_endian(const struct vringh *vrh)
+{
+	return vrh->little_endian;
+}
+
 static inline u16 vringh16_to_cpu(const struct vringh *vrh, __virtio16 val)
 {
-	return __virtio16_to_cpu(vrh->little_endian, val);
+	return __virtio16_to_cpu(vringh_is_little_endian(vrh), val);
 }
 
 static inline __virtio16 cpu_to_vringh16(const struct vringh *vrh, u16 val)
 {
-	return __cpu_to_virtio16(vrh->little_endian, val);
+	return __cpu_to_virtio16(vringh_is_little_endian(vrh), val);
 }
 
 static inline u32 vringh32_to_cpu(const struct vringh *vrh, __virtio32 val)
 {
-	return __virtio32_to_cpu(vrh->little_endian, val);
+	return __virtio32_to_cpu(vringh_is_little_endian(vrh), val);
 }
 
 static inline __virtio32 cpu_to_vringh32(const struct vringh *vrh, u32 val)
 {
-	return __cpu_to_virtio32(vrh->little_endian, val);
+	return __cpu_to_virtio32(vringh_is_little_endian(vrh), val);
 }
 
 static inline u64 vringh64_to_cpu(const struct vringh *vrh, __virtio64 val)
 {

include/linux/vringh.h static inline void vringh_notify(struct vringh *vrh) · line 263

Quoted 10 lines
include/linux/vringh.h +2 -2
-	return __virtio64_to_cpu(vrh->little_endian, val);
+	return __virtio64_to_cpu(vringh_is_little_endian(vrh), val);
 }
 
 static inline __virtio64 cpu_to_vringh64(const struct vringh *vrh, u64 val)
 {
-	return __cpu_to_virtio64(vrh->little_endian, val);
+	return __cpu_to_virtio64(vringh_is_little_endian(vrh), val);
 }
 #endif /* _LINUX_VRINGH_H */
Reviewed-by Thomas Huth <[email protected]>
author
#17 lore raw

PATCH v5 5/8 vhost: introduce vhost_is_little_endian() helper

In reply to Greg Kurz
Signed-off-by Greg Kurz <[email protected]>
---
 drivers/vhost/vhost.h |   17 +++++++++++------
 1 file changed, 11 insertions(+), 6 deletions(-)
drivers/vhost/vhost.h +11 -6
index 8c1c792..6a49960 100644
@@ -173,34 +173,39 @@ static inline bool vhost_has_feature(struct vhost_virtqueue *vq, int bit)
 	return vq->acked_features & (1ULL << bit);
 }
 
+static inline bool vhost_is_little_endian(struct vhost_virtqueue *vq)
+{
+	return vhost_has_feature(vq, VIRTIO_F_VERSION_1);
+}
+
 /* Memory accessors */
 static inline u16 vhost16_to_cpu(struct vhost_virtqueue *vq, __virtio16 val)
 {
-	return __virtio16_to_cpu(vhost_has_feature(vq, VIRTIO_F_VERSION_1), val);
+	return __virtio16_to_cpu(vhost_is_little_endian(vq), val);
 }
 
 static inline __virtio16 cpu_to_vhost16(struct vhost_virtqueue *vq, u16 val)
 {
-	return __cpu_to_virtio16(vhost_has_feature(vq, VIRTIO_F_VERSION_1), val);
+	return __cpu_to_virtio16(vhost_is_little_endian(vq), val);
 }
 
 static inline u32 vhost32_to_cpu(struct vhost_virtqueue *vq, __virtio32 val)
 {
-	return __virtio32_to_cpu(vhost_has_feature(vq, VIRTIO_F_VERSION_1), val);
+	return __virtio32_to_cpu(vhost_is_little_endian(vq), val);
 }
 
 static inline __virtio32 cpu_to_vhost32(struct vhost_virtqueue *vq, u32 val)
 {
-	return __cpu_to_virtio32(vhost_has_feature(vq, VIRTIO_F_VERSION_1), val);
+	return __cpu_to_virtio32(vhost_is_little_endian(vq), val);
 }
 
 static inline u64 vhost64_to_cpu(struct vhost_virtqueue *vq, __virtio64 val)
 {
-	return __virtio64_to_cpu(vhost_has_feature(vq, VIRTIO_F_VERSION_1), val);
+	return __virtio64_to_cpu(vhost_is_little_endian(vq), val);
 }
 
 static inline __virtio64 cpu_to_vhost64(struct vhost_virtqueue *vq, u64 val)
 {
-	return __cpu_to_virtio64(vhost_has_feature(vq, VIRTIO_F_VERSION_1), val);
+	return __cpu_to_virtio64(vhost_is_little_endian(vq), val);
 }
 #endif
Reviewed-by on v5 5/8
#18 lore raw

Re: [PATCH v5 5/8] vhost: introduce vhost_is_little_endian() helper

Quoted 45 lines · On Thu, 23 Apr 2015 17:27:05 +0200 Greg Kurz…
Signed-off-by Greg Kurz <[email protected]>
---
 drivers/vhost/vhost.h |   17 +++++++++++------
 1 file changed, 11 insertions(+), 6 deletions(-)
drivers/vhost/vhost.h +9 -4
index 8c1c792..6a49960 100644
@@ -173,34 +173,39 @@ static inline bool vhost_has_feature(struct vhost_virtqueue *vq, int bit)
 	return vq->acked_features & (1ULL << bit);
 }
 
+static inline bool vhost_is_little_endian(struct vhost_virtqueue *vq)
+{
+	return vhost_has_feature(vq, VIRTIO_F_VERSION_1);
+}
+
 /* Memory accessors */
 static inline u16 vhost16_to_cpu(struct vhost_virtqueue *vq, __virtio16 val)
 {
-	return __virtio16_to_cpu(vhost_has_feature(vq, VIRTIO_F_VERSION_1), val);
+	return __virtio16_to_cpu(vhost_is_little_endian(vq), val);
 }
 
 static inline __virtio16 cpu_to_vhost16(struct vhost_virtqueue *vq, u16 val)
 {
-	return __cpu_to_virtio16(vhost_has_feature(vq, VIRTIO_F_VERSION_1), val);
+	return __cpu_to_virtio16(vhost_is_little_endian(vq), val);
 }
 
 static inline u32 vhost32_to_cpu(struct vhost_virtqueue *vq, __virtio32 val)
 {
-	return __virtio32_to_cpu(vhost_has_feature(vq, VIRTIO_F_VERSION_1), val);
+	return __virtio32_to_cpu(vhost_is_little_endian(vq), val);
 }
 
 static inline __virtio32 cpu_to_vhost32(struct vhost_virtqueue *vq, u32 val)
 {
-	return __cpu_to_virtio32(vhost_has_feature(vq, VIRTIO_F_VERSION_1), val);
+	return __cpu_to_virtio32(vhost_is_little_endian(vq), val);
 }
 
 static inline u64 vhost64_to_cpu(struct vhost_virtqueue *vq, __virtio64 val)
 {

drivers/vhost/vhost.h static inline bool vhost_has_feature(struct vhost_virtqueue *vq, int bit) · line 211

Quoted 10 lines
drivers/vhost/vhost.h +2 -2
-	return __virtio64_to_cpu(vhost_has_feature(vq, VIRTIO_F_VERSION_1), val);
+	return __virtio64_to_cpu(vhost_is_little_endian(vq), val);
 }
 
 static inline __virtio64 cpu_to_vhost64(struct vhost_virtqueue *vq, u64 val)
 {
-	return __cpu_to_virtio64(vhost_has_feature(vq, VIRTIO_F_VERSION_1), val);
+	return __cpu_to_virtio64(vhost_is_little_endian(vq), val);
 }
 #endif
Reviewed-by Thomas Huth <[email protected]>
Reviewed-by on v5 5/8
#19 lore raw

Re: [PATCH v5 5/8] vhost: introduce vhost_is_little_endian() helper

In reply to Greg Kurz
Quoted 45 lines · On Thu, 23 Apr 2015 17:27:05 +0200 Greg Kurz <[email protected]> wrote:
Signed-off-by Greg Kurz <[email protected]>
---
 drivers/vhost/vhost.h |   17 +++++++++++------
 1 file changed, 11 insertions(+), 6 deletions(-)
drivers/vhost/vhost.h +9 -4
index 8c1c792..6a49960 100644
@@ -173,34 +173,39 @@ static inline bool vhost_has_feature(struct vhost_virtqueue *vq, int bit)
 	return vq->acked_features & (1ULL << bit);
 }
 
+static inline bool vhost_is_little_endian(struct vhost_virtqueue *vq)
+{
+	return vhost_has_feature(vq, VIRTIO_F_VERSION_1);
+}
+
 /* Memory accessors */
 static inline u16 vhost16_to_cpu(struct vhost_virtqueue *vq, __virtio16 val)
 {
-	return __virtio16_to_cpu(vhost_has_feature(vq, VIRTIO_F_VERSION_1), val);
+	return __virtio16_to_cpu(vhost_is_little_endian(vq), val);
 }
 
 static inline __virtio16 cpu_to_vhost16(struct vhost_virtqueue *vq, u16 val)
 {
-	return __cpu_to_virtio16(vhost_has_feature(vq, VIRTIO_F_VERSION_1), val);
+	return __cpu_to_virtio16(vhost_is_little_endian(vq), val);
 }
 
 static inline u32 vhost32_to_cpu(struct vhost_virtqueue *vq, __virtio32 val)
 {
-	return __virtio32_to_cpu(vhost_has_feature(vq, VIRTIO_F_VERSION_1), val);
+	return __virtio32_to_cpu(vhost_is_little_endian(vq), val);
 }
 
 static inline __virtio32 cpu_to_vhost32(struct vhost_virtqueue *vq, u32 val)
 {
-	return __cpu_to_virtio32(vhost_has_feature(vq, VIRTIO_F_VERSION_1), val);
+	return __cpu_to_virtio32(vhost_is_little_endian(vq), val);
 }
 
 static inline u64 vhost64_to_cpu(struct vhost_virtqueue *vq, __virtio64 val)
 {

drivers/vhost/vhost.h static inline bool vhost_has_feature(struct vhost_virtqueue *vq, int bit) · line 211

Quoted 10 lines
drivers/vhost/vhost.h +2 -2
-	return __virtio64_to_cpu(vhost_has_feature(vq, VIRTIO_F_VERSION_1), val);
+	return __virtio64_to_cpu(vhost_is_little_endian(vq), val);
 }
 
 static inline __virtio64 cpu_to_vhost64(struct vhost_virtqueue *vq, u64 val)
 {
-	return __cpu_to_virtio64(vhost_has_feature(vq, VIRTIO_F_VERSION_1), val);
+	return __cpu_to_virtio64(vhost_is_little_endian(vq), val);
 }
 #endif
Reviewed-by Thomas Huth <[email protected]>
author
#20 lore raw

PATCH v5 6/8 virtio: add explicit big-endian support to memory accessors

In reply to Greg Kurz

The current memory accessors logic is:
- little endian if little_endian
- native endian (i.e. no byteswap) if !little_endian

If we want to fully support cross-endian vhost, we also need to be able to convert to big endian.

Instead of changing the little_endian argument to some 3-value enum, this patch changes the logic to:
- little endian if little_endian
- big endian if !little_endian

The native endian case is handled by all users with a trivial helper. This patch doesn't change any functionality, nor it does add overhead.

Signed-off-by Greg Kurz <[email protected]>

---

Changes since v4:
- style fixes (I have chosen if ... else in most places to stay below 80 columns, with the notable exception of the vhost helper which gets shorten in a later patch)

 drivers/net/macvtap.c            |    5 ++++-
 drivers/net/tun.c                |    5 ++++-
 drivers/vhost/vhost.h            |    2 +-
 include/linux/virtio_byteorder.h |   24 ++++++++++++++----------
 include/linux/virtio_config.h    |    5 ++++-
 include/linux/vringh.h           |    2 +-
 6 files changed, 28 insertions(+), 15 deletions(-)
drivers/net/macvtap.c +4 -1
index a2f2958..6cf6b3e 100644
@@ -51,7 +51,10 @@ struct macvtap_queue {
 
 static inline bool macvtap_is_little_endian(struct macvtap_queue *q)
 {
-	return q->flags & MACVTAP_VNET_LE;
+	if (q->flags & MACVTAP_VNET_LE)
+		return true;
+	else
+		return virtio_legacy_is_little_endian();
 }
 
 static inline u16 macvtap16_to_cpu(struct macvtap_queue *q, __virtio16 val)
drivers/net/tun.c +4 -1
index 3c3d6c0..5b044d4 100644
@@ -208,7 +208,10 @@ struct tun_struct {
 
 static inline bool tun_is_little_endian(struct tun_struct *tun)
 {
-	return tun->flags & TUN_VNET_LE;
+	if (tun->flags & TUN_VNET_LE)
+		return true;
+	else
+		return virtio_legacy_is_little_endian();
 }
 
 static inline u16 tun16_to_cpu(struct tun_struct *tun, __virtio16 val)
drivers/vhost/vhost.h +1 -1
index 6a49960..954c657 100644
@@ -175,7 +175,7 @@ static inline bool vhost_has_feature(struct vhost_virtqueue *vq, int bit)
 
 static inline bool vhost_is_little_endian(struct vhost_virtqueue *vq)
 {
-	return vhost_has_feature(vq, VIRTIO_F_VERSION_1);
+	return vhost_has_feature(vq, VIRTIO_F_VERSION_1) ? true : virtio_legacy_is_little_endian();
 }
 
 /* Memory accessors */
include/linux/virtio_byteorder.h +14 -10
index 51865d0..ce63a2c 100644
@@ -3,17 +3,21 @@
 #include <linux/types.h>
 #include <uapi/linux/virtio_types.h>
 
-/*
- * Low-level memory accessors for handling virtio in modern little endian and in
- * compatibility native endian format.
- */
+static inline bool virtio_legacy_is_little_endian(void)
+{
+#ifdef __LITTLE_ENDIAN
+	return true;
+#else
+	return false;
+#endif
+}
 
 static inline u16 __virtio16_to_cpu(bool little_endian, __virtio16 val)
 {
 	if (little_endian)
 		return le16_to_cpu((__force __le16)val);
 	else
-		return (__force u16)val;
+		return be16_to_cpu((__force __be16)val);
 }
 
 static inline __virtio16 __cpu_to_virtio16(bool little_endian, u16 val)

@@ -21,7 +25,7 @@ static inline __virtio16 __cpu_to_virtio16(bool little_endian, u16 val)
 	if (little_endian)
 		return (__force __virtio16)cpu_to_le16(val);
 	else
-		return (__force __virtio16)val;
+		return (__force __virtio16)cpu_to_be16(val);
 }
 
 static inline u32 __virtio32_to_cpu(bool little_endian, __virtio32 val)

@@ -29,7 +33,7 @@ static inline u32 __virtio32_to_cpu(bool little_endian, __virtio32 val)
 	if (little_endian)
 		return le32_to_cpu((__force __le32)val);
 	else
-		return (__force u32)val;
+		return be32_to_cpu((__force __be32)val);
 }
 
 static inline __virtio32 __cpu_to_virtio32(bool little_endian, u32 val)

@@ -37,7 +41,7 @@ static inline __virtio32 __cpu_to_virtio32(bool little_endian, u32 val)
 	if (little_endian)
 		return (__force __virtio32)cpu_to_le32(val);
 	else
-		return (__force __virtio32)val;
+		return (__force __virtio32)cpu_to_be32(val);
 }
 
 static inline u64 __virtio64_to_cpu(bool little_endian, __virtio64 val)

@@ -45,7 +49,7 @@ static inline u64 __virtio64_to_cpu(bool little_endian, __virtio64 val)
 	if (little_endian)
 		return le64_to_cpu((__force __le64)val);
 	else
-		return (__force u64)val;
+		return be64_to_cpu((__force __be64)val);
 }
 
 static inline __virtio64 __cpu_to_virtio64(bool little_endian, u64 val)

@@ -53,7 +57,7 @@ static inline __virtio64 __cpu_to_virtio64(bool little_endian, u64 val)
 	if (little_endian)
 		return (__force __virtio64)cpu_to_le64(val);
 	else
-		return (__force __virtio64)val;
+		return (__force __virtio64)cpu_to_be64(val);
 }
 
 #endif /* _LINUX_VIRTIO_BYTEORDER */
include/linux/virtio_config.h +4 -1
index bd1a582..743e24c 100644
@@ -207,7 +207,10 @@ int virtqueue_set_affinity(struct virtqueue *vq, int cpu)
 
 static inline bool virtio_is_little_endian(struct virtio_device *vdev)
 {
-	return virtio_has_feature(vdev, VIRTIO_F_VERSION_1);
+	if (virtio_has_feature(vdev, VIRTIO_F_VERSION_1))
+		return true;
+	else
+		return virtio_legacy_is_little_endian();
 }
 
 /* Memory accessors */
include/linux/vringh.h +1 -1
index 3ed62ef..d070bbcd 100644
@@ -228,7 +228,7 @@ static inline void vringh_notify(struct vringh *vrh)
 
 static inline bool vringh_is_little_endian(const struct vringh *vrh)
 {
-	return vrh->little_endian;
+	return vrh->little_endian ? true : virtio_legacy_is_little_endian();
 }
 
 static inline u16 vringh16_to_cpu(const struct vringh *vrh, __virtio16 val)
#21 lore raw

Re: [PATCH v5 6/8] virtio: add explicit big-endian support to memory accessors

Quoted 36 lines · On Thu, 23 Apr 2015 17:29:06 +0200 Greg Kurz…

The current memory accessors logic is:
- little endian if little_endian
- native endian (i.e. no byteswap) if !little_endian

If we want to fully support cross-endian vhost, we also need to be able to convert to big endian.

Instead of changing the little_endian argument to some 3-value enum, this patch changes the logic to:
- little endian if little_endian
- big endian if !little_endian

The native endian case is handled by all users with a trivial helper. This patch doesn't change any functionality, nor it does add overhead.

Signed-off-by Greg Kurz <[email protected]>

---

Changes since v4:
- style fixes (I have chosen if ... else in most places to stay below 80 columns, with the notable exception of the vhost helper which gets shorten in a later patch)

 drivers/net/macvtap.c            |    5 ++++-
 drivers/net/tun.c                |    5 ++++-
 drivers/vhost/vhost.h            |    2 +-
 include/linux/virtio_byteorder.h |   24 ++++++++++++++----------
 include/linux/virtio_config.h    |    5 ++++-
 include/linux/vringh.h           |    2 +-
 6 files changed, 28 insertions(+), 15 deletions(-)
drivers/net/macvtap.c +0 -0
index a2f2958..6cf6b3e 100644

drivers/net/macvtap.c struct macvtap_queue { · line 57

Quoted 8 lines
drivers/net/macvtap.c +4 -1
@@ -51,7 +51,10 @@ struct macvtap_queue {
 
 static inline bool macvtap_is_little_endian(struct macvtap_queue *q)
 {
-	return q->flags & MACVTAP_VNET_LE;
+	if (q->flags & MACVTAP_VNET_LE)
+		return true;
+	else
+		return virtio_legacy_is_little_endian();

simply:

	return (q->flags & MACVTAP_VNET_LE) ||
	       virtio_legacy_is_little_endian();

?

Quoted 8 lines
drivers/net/macvtap.c +0 -0
 }
 
 static inline u16 macvtap16_to_cpu(struct macvtap_queue *q, __virtio16 val)
drivers/net/tun.c +0 -0
index 3c3d6c0..5b044d4 100644

drivers/net/tun.c struct tun_struct { · line 214

Quoted 8 lines
drivers/net/tun.c +4 -1
@@ -208,7 +208,10 @@ struct tun_struct {
 
 static inline bool tun_is_little_endian(struct tun_struct *tun)
 {
-	return tun->flags & TUN_VNET_LE;
+	if (tun->flags & TUN_VNET_LE)
+		return true;
+	else
+		return virtio_legacy_is_little_endian();

dito?

Quoted 8 lines
drivers/net/tun.c +0 -0
 }
 
 static inline u16 tun16_to_cpu(struct tun_struct *tun, __virtio16 val)
drivers/vhost/vhost.h +0 -0
index 6a49960..954c657 100644

drivers/vhost/vhost.h static inline bool vhost_has_feature(struct vhost_virtqueue *vq, int bit) · line 179

Quoted 6 lines
drivers/vhost/vhost.h +1 -1
@@ -175,7 +175,7 @@ static inline bool vhost_has_feature(struct vhost_virtqueue *vq, int bit)
 
 static inline bool vhost_is_little_endian(struct vhost_virtqueue *vq)
 {
-	return vhost_has_feature(vq, VIRTIO_F_VERSION_1);
+	return vhost_has_feature(vq, VIRTIO_F_VERSION_1) ? true : virtio_legacy_is_little_endian();
 }

That line is way longer than 80 characters ... may I suggest to switch at least here to:

	return vhost_has_feature(vq, VIRTIO_F_VERSION_1) ||
	       virtio_legacy_is_little_endian();

?

Apart from the cosmetics, the patch looks good to me.

Thomas

#22 lore raw

Re: [PATCH v5 6/8] virtio: add explicit big-endian support to memory accessors

Quoted 50 lines · On Thu, 23 Apr 2015 21:27:19 +0200 Thomas Huth <[email protected]> wrote:
Quoted 36 lines · On Thu, 23 Apr 2015 17:29:06 +0200 Greg Kurz <[email protected]> wrote:

The current memory accessors logic is:
- little endian if little_endian
- native endian (i.e. no byteswap) if !little_endian

If we want to fully support cross-endian vhost, we also need to be able to convert to big endian.

Instead of changing the little_endian argument to some 3-value enum, this patch changes the logic to:
- little endian if little_endian
- big endian if !little_endian

The native endian case is handled by all users with a trivial helper. This patch doesn't change any functionality, nor it does add overhead.

Signed-off-by Greg Kurz <[email protected]>

---

Changes since v4:
- style fixes (I have chosen if ... else in most places to stay below 80 columns, with the notable exception of the vhost helper which gets shorten in a later patch)

 drivers/net/macvtap.c            |    5 ++++-
 drivers/net/tun.c                |    5 ++++-
 drivers/vhost/vhost.h            |    2 +-
 include/linux/virtio_byteorder.h |   24 ++++++++++++++----------
 include/linux/virtio_config.h    |    5 ++++-
 include/linux/vringh.h           |    2 +-
 6 files changed, 28 insertions(+), 15 deletions(-)
drivers/net/macvtap.c +0 -0
index a2f2958..6cf6b3e 100644

drivers/net/macvtap.c struct macvtap_queue { · line 57

Quoted 8 lines
drivers/net/macvtap.c +4 -1
@@ -51,7 +51,10 @@ struct macvtap_queue {
 
 static inline bool macvtap_is_little_endian(struct macvtap_queue *q)
 {
-	return q->flags & MACVTAP_VNET_LE;
+	if (q->flags & MACVTAP_VNET_LE)
+		return true;
+	else
+		return virtio_legacy_is_little_endian();

simply:

Quoted 4 lines
	return (q->flags & MACVTAP_VNET_LE) ||
	       virtio_legacy_is_little_endian();

?

ISTR that MST preferred the current notation, but I like either your way or ?: (with linebreak) even better.

Quoted 3 lines
Quoted 3 lines
drivers/net/macvtap.c +0 -0
 }

(...)

Quoted 15 lines

drivers/vhost/vhost.h static inline bool vhost_has_feature(struct vhost_virtqueue *vq, int bit) · line 179

Quoted 11 lines
drivers/vhost/vhost.h +1 -1
index 6a49960..954c657 100644
@@ -175,7 +175,7 @@ static inline bool vhost_has_feature(struct vhost_virtqueue *vq, int bit)
 
 static inline bool vhost_is_little_endian(struct vhost_virtqueue *vq)
 {
-	return vhost_has_feature(vq, VIRTIO_F_VERSION_1);
+	return vhost_has_feature(vq, VIRTIO_F_VERSION_1) ? true : virtio_legacy_is_little_endian();
 }

That line is way longer than 80 characters ... may I suggest to switch at least here to:

Quoted 2 lines
	return vhost_has_feature(vq, VIRTIO_F_VERSION_1) ||
	       virtio_legacy_is_little_endian();

I think the line will collapse in a further patch anyway.

author
#23 lore raw

Re: [PATCH v5 6/8] virtio: add explicit big-endian support to memory accessors

Quoted 3 lines · On Fri, 24 Apr 2015 09:04:21 +0200 Cornelia Huck…

On Thu, 23 Apr 2015 21:27:19 +0200
Thomas Huth <[email protected]> wrote:

Thomas's e-mail did not make it to my mailbox... weird. :-\

Quoted 50 lines
Quoted 50 lines
Quoted 36 lines · On Thu, 23 Apr 2015 17:29:06 +0200 Greg Kurz…

The current memory accessors logic is:
- little endian if little_endian
- native endian (i.e. no byteswap) if !little_endian

If we want to fully support cross-endian vhost, we also need to be able to convert to big endian.

Instead of changing the little_endian argument to some 3-value enum, this patch changes the logic to:
- little endian if little_endian
- big endian if !little_endian

The native endian case is handled by all users with a trivial helper. This patch doesn't change any functionality, nor it does add overhead.

Signed-off-by Greg Kurz <[email protected]>

---

Changes since v4:
- style fixes (I have chosen if ... else in most places to stay below 80 columns, with the notable exception of the vhost helper which gets shorten in a later patch)

 drivers/net/macvtap.c            |    5 ++++-
 drivers/net/tun.c                |    5 ++++-
 drivers/vhost/vhost.h            |    2 +-
 include/linux/virtio_byteorder.h |   24 ++++++++++++++----------
 include/linux/virtio_config.h    |    5 ++++-
 include/linux/vringh.h           |    2 +-
 6 files changed, 28 insertions(+), 15 deletions(-)
drivers/net/macvtap.c +0 -0
index a2f2958..6cf6b3e 100644

drivers/net/macvtap.c struct macvtap_queue { · line 57

Quoted 8 lines
drivers/net/macvtap.c +4 -1
@@ -51,7 +51,10 @@ struct macvtap_queue {
 
 static inline bool macvtap_is_little_endian(struct macvtap_queue *q)
 {
-	return q->flags & MACVTAP_VNET_LE;
+	if (q->flags & MACVTAP_VNET_LE)
+		return true;
+	else
+		return virtio_legacy_is_little_endian();

simply:

Quoted 8 lines
Quoted 4 lines
	return (q->flags & MACVTAP_VNET_LE) ||
	       virtio_legacy_is_little_endian();

?

ISTR that MST preferred the current notation, but I like either your way or ?: (with linebreak) even better.

MST did not like the initial notation I had used actually. FWIW I like the simplicity of Thomas's suggestion... even better than ?:
which is:
some_LE_check() ? true : some_default_endianness()

Quoted 19 lines
Quoted 3 lines
Quoted 3 lines
drivers/net/macvtap.c +0 -0
 }

(...)

Quoted 15 lines

drivers/vhost/vhost.h static inline bool vhost_has_feature(struct vhost_virtqueue *vq, int bit) · line 179

Quoted 11 lines
drivers/vhost/vhost.h +1 -1
index 6a49960..954c657 100644
@@ -175,7 +175,7 @@ static inline bool vhost_has_feature(struct vhost_virtqueue *vq, int bit)
 
 static inline bool vhost_is_little_endian(struct vhost_virtqueue *vq)
 {
-	return vhost_has_feature(vq, VIRTIO_F_VERSION_1);
+	return vhost_has_feature(vq, VIRTIO_F_VERSION_1) ? true : virtio_legacy_is_little_endian();
 }

That line is way longer than 80 characters ... may I suggest to switch at least here to:

Quoted 5 lines
Quoted 2 lines
	return vhost_has_feature(vq, VIRTIO_F_VERSION_1) ||
	       virtio_legacy_is_little_endian();

I think the line will collapse in a further patch anyway.

Yes, as mentionned in the changelog :)

Quoted 5 lines

--
To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html

author
#24 lore raw

Re: [PATCH v5 6/8] virtio: add explicit big-endian support to memory accessors

In reply to Cornelia Huck
Quoted 3 lines · On Fri, 24 Apr 2015 09:04:21 +0200 Cornelia Huck <[email protected]>…

On Thu, 23 Apr 2015 21:27:19 +0200
Thomas Huth <[email protected]> wrote:

Thomas's e-mail did not make it to my mailbox... weird. :-\

Quoted 50 lines
Quoted 50 lines
Quoted 36 lines · On Thu, 23 Apr 2015 17:29:06 +0200 Greg Kurz <[email protected]> wrote:

The current memory accessors logic is:
- little endian if little_endian
- native endian (i.e. no byteswap) if !little_endian

If we want to fully support cross-endian vhost, we also need to be able to convert to big endian.

Instead of changing the little_endian argument to some 3-value enum, this patch changes the logic to:
- little endian if little_endian
- big endian if !little_endian

The native endian case is handled by all users with a trivial helper. This patch doesn't change any functionality, nor it does add overhead.

Signed-off-by Greg Kurz <[email protected]>

---

Changes since v4:
- style fixes (I have chosen if ... else in most places to stay below 80 columns, with the notable exception of the vhost helper which gets shorten in a later patch)

 drivers/net/macvtap.c            |    5 ++++-
 drivers/net/tun.c                |    5 ++++-
 drivers/vhost/vhost.h            |    2 +-
 include/linux/virtio_byteorder.h |   24 ++++++++++++++----------
 include/linux/virtio_config.h    |    5 ++++-
 include/linux/vringh.h           |    2 +-
 6 files changed, 28 insertions(+), 15 deletions(-)
drivers/net/macvtap.c +0 -0
index a2f2958..6cf6b3e 100644

drivers/net/macvtap.c struct macvtap_queue { · line 57

Quoted 8 lines
drivers/net/macvtap.c +4 -1
@@ -51,7 +51,10 @@ struct macvtap_queue {
 
 static inline bool macvtap_is_little_endian(struct macvtap_queue *q)
 {
-	return q->flags & MACVTAP_VNET_LE;
+	if (q->flags & MACVTAP_VNET_LE)
+		return true;
+	else
+		return virtio_legacy_is_little_endian();

simply:

Quoted 8 lines
Quoted 4 lines
	return (q->flags & MACVTAP_VNET_LE) ||
	       virtio_legacy_is_little_endian();

?

ISTR that MST preferred the current notation, but I like either your way or ?: (with linebreak) even better.

MST did not like the initial notation I had used actually. FWIW I like the simplicity of Thomas's suggestion... even better than ?:
which is:
some_LE_check() ? true : some_default_endianness()

Quoted 19 lines
Quoted 3 lines
Quoted 3 lines
drivers/net/macvtap.c +0 -0
 }

(...)

Quoted 15 lines

drivers/vhost/vhost.h static inline bool vhost_has_feature(struct vhost_virtqueue *vq, int bit) · line 179

Quoted 11 lines
drivers/vhost/vhost.h +1 -1
index 6a49960..954c657 100644
@@ -175,7 +175,7 @@ static inline bool vhost_has_feature(struct vhost_virtqueue *vq, int bit)
 
 static inline bool vhost_is_little_endian(struct vhost_virtqueue *vq)
 {
-	return vhost_has_feature(vq, VIRTIO_F_VERSION_1);
+	return vhost_has_feature(vq, VIRTIO_F_VERSION_1) ? true : virtio_legacy_is_little_endian();
 }

That line is way longer than 80 characters ... may I suggest to switch at least here to:

Quoted 5 lines
Quoted 2 lines
	return vhost_has_feature(vq, VIRTIO_F_VERSION_1) ||
	       virtio_legacy_is_little_endian();

I think the line will collapse in a further patch anyway.

Yes, as mentionned in the changelog :)

Quoted 5 lines

--
To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

#25 lore raw

Re: [PATCH v5 6/8] virtio: add explicit big-endian support to memory accessors

In reply to Greg Kurz
Quoted 36 lines · On Thu, 23 Apr 2015 17:29:06 +0200 Greg Kurz <[email protected]> wrote:

The current memory accessors logic is:
- little endian if little_endian
- native endian (i.e. no byteswap) if !little_endian

If we want to fully support cross-endian vhost, we also need to be able to convert to big endian.

Instead of changing the little_endian argument to some 3-value enum, this patch changes the logic to:
- little endian if little_endian
- big endian if !little_endian

The native endian case is handled by all users with a trivial helper. This patch doesn't change any functionality, nor it does add overhead.

Signed-off-by Greg Kurz <[email protected]>

---

Changes since v4:
- style fixes (I have chosen if ... else in most places to stay below 80 columns, with the notable exception of the vhost helper which gets shorten in a later patch)

 drivers/net/macvtap.c            |    5 ++++-
 drivers/net/tun.c                |    5 ++++-
 drivers/vhost/vhost.h            |    2 +-
 include/linux/virtio_byteorder.h |   24 ++++++++++++++----------
 include/linux/virtio_config.h    |    5 ++++-
 include/linux/vringh.h           |    2 +-
 6 files changed, 28 insertions(+), 15 deletions(-)
drivers/net/macvtap.c +0 -0
index a2f2958..6cf6b3e 100644

drivers/net/macvtap.c struct macvtap_queue { · line 57

Quoted 8 lines
drivers/net/macvtap.c +4 -1
@@ -51,7 +51,10 @@ struct macvtap_queue {
 
 static inline bool macvtap_is_little_endian(struct macvtap_queue *q)
 {
-	return q->flags & MACVTAP_VNET_LE;
+	if (q->flags & MACVTAP_VNET_LE)
+		return true;
+	else
+		return virtio_legacy_is_little_endian();

simply:

	return (q->flags & MACVTAP_VNET_LE) ||
	       virtio_legacy_is_little_endian();

?

Quoted 8 lines
drivers/net/macvtap.c +0 -0
 }
 
 static inline u16 macvtap16_to_cpu(struct macvtap_queue *q, __virtio16 val)
drivers/net/tun.c +0 -0
index 3c3d6c0..5b044d4 100644

drivers/net/tun.c struct tun_struct { · line 214

Quoted 8 lines
drivers/net/tun.c +4 -1
@@ -208,7 +208,10 @@ struct tun_struct {
 
 static inline bool tun_is_little_endian(struct tun_struct *tun)
 {
-	return tun->flags & TUN_VNET_LE;
+	if (tun->flags & TUN_VNET_LE)
+		return true;
+	else
+		return virtio_legacy_is_little_endian();

dito?

Quoted 8 lines
drivers/net/tun.c +0 -0
 }
 
 static inline u16 tun16_to_cpu(struct tun_struct *tun, __virtio16 val)
drivers/vhost/vhost.h +0 -0
index 6a49960..954c657 100644

drivers/vhost/vhost.h static inline bool vhost_has_feature(struct vhost_virtqueue *vq, int bit) · line 179

Quoted 6 lines
drivers/vhost/vhost.h +1 -1
@@ -175,7 +175,7 @@ static inline bool vhost_has_feature(struct vhost_virtqueue *vq, int bit)
 
 static inline bool vhost_is_little_endian(struct vhost_virtqueue *vq)
 {
-	return vhost_has_feature(vq, VIRTIO_F_VERSION_1);
+	return vhost_has_feature(vq, VIRTIO_F_VERSION_1) ? true : virtio_legacy_is_little_endian();
 }

That line is way longer than 80 characters ... may I suggest to switch at least here to:

	return vhost_has_feature(vq, VIRTIO_F_VERSION_1) ||
	       virtio_legacy_is_little_endian();

?

Apart from the cosmetics, the patch looks good to me.

Thomas

author
#26 lore raw

PATCH v5 7/8 vhost: cross-endian support for legacy devices

In reply to Greg Kurz

This patch brings cross-endian support to vhost when used to implement legacy virtio devices. Since it is a relatively rare situation, the feature availability is controlled by a kernel config option (not set by default).

The vq->is_le boolean field is added to cache the endianness to be used for ring accesses. It defaults to native endian, as expected by legacy virtio devices. When the ring gets active, we force little endian if the device is modern. When the ring is deactivated, we revert to the native endian default.

If cross-endian was compiled in, a vq->user_be boolean field is added so that userspace may request a specific endianness. This field is used to override the default when activating the ring of a legacy device. It has no effect on modern devices.

Signed-off-by Greg Kurz <[email protected]>

---

Changes since v4:
- rewrote patch title to mention cross-endian
- renamed config to VHOST_CROSS_ENDIAN_LEGACY
- rewrote config description and help
- moved ifdefery to top of vhost.c
- added a detailed comment about the lifecycle of vq->user_be in vhost_init_is_le()
- renamed ioctls to VHOST_[GS]ET_VRING_ENDIAN
- added LE/BE defines to the ioctl API
- rewrote ioctl sanity check with the LE/BE defines
- updated comment in <uapi/linux/vhost.h> to mention that the availibility of both SET and GET ioctls depends on the kernel config

 drivers/vhost/Kconfig      |   15 ++++++++
 drivers/vhost/vhost.c      |   86 +++++++++++++++++++++++++++++++++++++++++++-
 drivers/vhost/vhost.h      |   10 +++++
 include/uapi/linux/vhost.h |   12 ++++++
 4 files changed, 121 insertions(+), 2 deletions(-)
drivers/vhost/Kconfig +15 -0
index 017a1e8..74d7380 100644
@@ -32,3 +32,18 @@ config VHOST
 	---help---
 	  This option is selected by any driver which needs to access
 	  the core of vhost.
+
+config VHOST_CROSS_ENDIAN_LEGACY
+	bool "Cross-endian support for vhost"
+	default n
+	---help---
+	  This option allows vhost to support guests with a different byte
+	  ordering from host.
+
+	  Userspace programs can control the feature using the
+	  VHOST_SET_VRING_ENDIAN and VHOST_GET_VRING_ENDIAN ioctls.
+
+	  This is only useful on a few platforms (ppc64 and arm64). Since it
+	  adds some overhead, it is disabled default.
+
+	  If unsure, say "N".
drivers/vhost/vhost.c +85 -1

111 lines not on this page · raw message

drivers/vhost/vhost.h +9 -1
index 954c657..ce6f6da 100644
@@ -106,6 +106,14 @@ struct vhost_virtqueue {
 	/* Log write descriptors */
 	void __user *log_base;
 	struct vhost_log *log;
+
+	/* Ring endianness. Defaults to legacy native endianness.
+	 * Set to true when starting a modern virtio device. */
+	bool is_le;
+#ifdef CONFIG_VHOST_CROSS_ENDIAN_LEGACY
+	/* Ring endianness requested by userspace for cross-endian support. */
+	bool user_be;
+#endif
 };
 
 struct vhost_dev {

@@ -175,7 +183,7 @@ static inline bool vhost_has_feature(struct vhost_virtqueue *vq, int bit)
 
 static inline bool vhost_is_little_endian(struct vhost_virtqueue *vq)
 {
-	return vhost_has_feature(vq, VIRTIO_F_VERSION_1) ? true : virtio_legacy_is_little_endian();
+	return vq->is_le;
 }
 
 /* Memory accessors */
include/uapi/linux/vhost.h +12 -0
index bb6a5b4..b980b53 100644
@@ -103,6 +103,18 @@ struct vhost_memory {
 /* Get accessor: reads index, writes value in num */
 #define VHOST_GET_VRING_BASE _IOWR(VHOST_VIRTIO, 0x12, struct vhost_vring_state)
 
+/* Set the vring byte order in num. Valid values are VHOST_VRING_LITTLE_ENDIAN
+ * or VHOST_VRING_BIG_ENDIAN (other values return EINVAL).
+ * This is a legacy only API that is simply ignored when VIRTIO_F_VERSION_1 is
+ * set.
+ * Not all kernel configurations support this ioctl, but all configurations that
+ * support SET also support GET.
+ */
+#define VHOST_VRING_LITTLE_ENDIAN 0
+#define VHOST_VRING_BIG_ENDIAN 1
+#define VHOST_SET_VRING_ENDIAN _IOW(VHOST_VIRTIO, 0x13, struct vhost_vring_state)
+#define VHOST_GET_VRING_ENDIAN _IOW(VHOST_VIRTIO, 0x14, struct vhost_vring_state)
+
 /* The following ioctls use eventfd file descriptors to signal and poll
  * for events. */
#27 lore raw

Re: [PATCH v5 7/8] vhost: cross-endian support for legacy devices

Quoted 43 lines · On Thu, 23 Apr 2015 17:29:42 +0200 Greg Kurz <[email protected]> wrote:

This patch brings cross-endian support to vhost when used to implement legacy virtio devices. Since it is a relatively rare situation, the feature availability is controlled by a kernel config option (not set by default).

The vq->is_le boolean field is added to cache the endianness to be used for ring accesses. It defaults to native endian, as expected by legacy virtio devices. When the ring gets active, we force little endian if the device is modern. When the ring is deactivated, we revert to the native endian default.

If cross-endian was compiled in, a vq->user_be boolean field is added so that userspace may request a specific endianness. This field is used to override the default when activating the ring of a legacy device. It has no effect on modern devices.

Signed-off-by Greg Kurz <[email protected]>

---

Changes since v4:
- rewrote patch title to mention cross-endian
- renamed config to VHOST_CROSS_ENDIAN_LEGACY
- rewrote config description and help
- moved ifdefery to top of vhost.c
- added a detailed comment about the lifecycle of vq->user_be in vhost_init_is_le()
- renamed ioctls to VHOST_[GS]ET_VRING_ENDIAN
- added LE/BE defines to the ioctl API
- rewrote ioctl sanity check with the LE/BE defines
- updated comment in <uapi/linux/vhost.h> to mention that the availibility of both SET and GET ioctls depends on the kernel config

 drivers/vhost/Kconfig      |   15 ++++++++
 drivers/vhost/vhost.c      |   86 +++++++++++++++++++++++++++++++++++++++++++-
 drivers/vhost/vhost.h      |   10 +++++
 include/uapi/linux/vhost.h |   12 ++++++
 4 files changed, 121 insertions(+), 2 deletions(-)
drivers/vhost/Kconfig +0 -0
index 017a1e8..74d7380 100644

drivers/vhost/Kconfig config VHOST · line 41

Quoted 10 lines
drivers/vhost/Kconfig +7 -0
@@ -32,3 +32,18 @@ config VHOST
 	---help---
 	  This option is selected by any driver which needs to access
 	  the core of vhost.
+
+config VHOST_CROSS_ENDIAN_LEGACY
+	bool "Cross-endian support for vhost"
+	default n
+	---help---
+	  This option allows vhost to support guests with a different byte
+	  ordering from host.

"...while using legacy virtio."

Might help to explain the "LEGACY" in the config option ;)

drivers/vhost/Kconfig config VHOST · line 47

Quoted 6 lines
drivers/vhost/Kconfig +6 -0
+
+	  Userspace programs can control the feature using the
+	  VHOST_SET_VRING_ENDIAN and VHOST_GET_VRING_ENDIAN ioctls.
+
+	  This is only useful on a few platforms (ppc64 and arm64). Since it
+	  adds some overhead, it is disabled default.

s/default/by default/

Quoted 52 lines
drivers/vhost/Kconfig +2 -0
+
+	  If unsure, say "N".
drivers/vhost/vhost.c +42 -0
index 2ee2826..8c4390d 100644
@@ -36,6 +36,78 @@ enum {
 #define vhost_used_event(vq) ((__virtio16 __user *)&vq->avail->ring[vq->num])
 #define vhost_avail_event(vq) ((__virtio16 __user *)&vq->used->ring[vq->num])
 
+#ifdef CONFIG_VHOST_CROSS_ENDIAN_LEGACY
+static void vhost_vq_reset_user_be(struct vhost_virtqueue *vq)
+{
+	vq->user_be = !virtio_legacy_is_little_endian();
+}
+
+static long vhost_set_vring_endian(struct vhost_virtqueue *vq, int __user *argp)
+{
+	struct vhost_vring_state s;
+
+	if (vq->private_data)
+		return -EBUSY;
+
+	if (copy_from_user(&s, argp, sizeof(s)))
+		return -EFAULT;
+
+	if (s.num != VHOST_VRING_LITTLE_ENDIAN &&
+	    s.num != VHOST_VRING_BIG_ENDIAN)
+		return -EINVAL;
+
+	vq->user_be = s.num;
+
+	return 0;
+}
+
+static long vhost_get_vring_endian(struct vhost_virtqueue *vq, u32 idx,
+				   int __user *argp)
+{
+	struct vhost_vring_state s = {
+		.index = idx,
+		.num = vq->user_be
+	};
+
+	if (copy_to_user(argp, &s, sizeof(s)))
+		return -EFAULT;
+
+	return 0;
+}
+
+static void vhost_init_is_le(struct vhost_virtqueue *vq)
+{
+	/* Note for legacy virtio: user_be is initialized at reset time

drivers/vhost/vhost.c enum { · line 90

Quoted 10 lines
drivers/vhost/vhost.c +10 -0
+	 * according to the host endianness. If userspace does not set an
+	 * explicit endianness, the default behavior is native endian, as
+	 * expected by legacy virtio.
+	 */
+	vq->is_le = vhost_has_feature(vq, VIRTIO_F_VERSION_1) || !vq->user_be;
+}
+#else
+static void vhost_vq_reset_user_be(struct vhost_virtqueue *vq)
+{
+	;

Just leave the function body empty?

Quoted 23 lines
drivers/vhost/vhost.c +20 -0
+}
+
+static long vhost_set_vring_endian(struct vhost_virtqueue *vq, int __user *argp)
+{
+	return -ENOIOCTLCMD;
+}
+
+static long vhost_get_vring_endian(struct vhost_virtqueue *vq, u32 idx,
+				   int __user *argp)
+{
+	return -ENOIOCTLCMD;
+}
+
+static void vhost_init_is_le(struct vhost_virtqueue *vq)
+{
+	if (vhost_has_feature(vq, VIRTIO_F_VERSION_1))
+		vq->is_le = true;
+}
+#endif /* CONFIG_VHOST_CROSS_ENDIAN_LEGACY */
+
 static void vhost_poll_func(struct file *file, wait_queue_head_t *wqh,
 			    poll_table *pt)
 {

(...)

include/uapi/linux/vhost.h struct vhost_memory { · line 107

Quoted 10 lines
include/uapi/linux/vhost.h +2 -0
index bb6a5b4..b980b53 100644
@@ -103,6 +103,18 @@ struct vhost_memory {
 /* Get accessor: reads index, writes value in num */
 #define VHOST_GET_VRING_BASE _IOWR(VHOST_VIRTIO, 0x12, struct vhost_vring_state)
 
+/* Set the vring byte order in num. Valid values are VHOST_VRING_LITTLE_ENDIAN
+ * or VHOST_VRING_BIG_ENDIAN (other values return EINVAL).

-EINVAL?

Should you also mention when you return -EBUSY?

include/uapi/linux/vhost.h struct vhost_memory { · line 119

Quoted 12 lines
include/uapi/linux/vhost.h +10 -0
+ * This is a legacy only API that is simply ignored when VIRTIO_F_VERSION_1 is
+ * set.
+ * Not all kernel configurations support this ioctl, but all configurations that
+ * support SET also support GET.
+ */
+#define VHOST_VRING_LITTLE_ENDIAN 0
+#define VHOST_VRING_BIG_ENDIAN 1
+#define VHOST_SET_VRING_ENDIAN _IOW(VHOST_VIRTIO, 0x13, struct vhost_vring_state)
+#define VHOST_GET_VRING_ENDIAN _IOW(VHOST_VIRTIO, 0x14, struct vhost_vring_state)
+
 /* The following ioctls use eventfd file descriptors to signal and poll
  * for events. */

Apart from style nitpicks, looks good.

author
#28 lore raw

Re: [PATCH v5 7/8] vhost: cross-endian support for legacy devices

Quoted 59 lines · On Fri, 24 Apr 2015 09:19:26 +0200 Cornelia Huck <[email protected]>…
Quoted 43 lines · On Thu, 23 Apr 2015 17:29:42 +0200 Greg Kurz <[email protected]> wrote:

This patch brings cross-endian support to vhost when used to implement legacy virtio devices. Since it is a relatively rare situation, the feature availability is controlled by a kernel config option (not set by default).

The vq->is_le boolean field is added to cache the endianness to be used for ring accesses. It defaults to native endian, as expected by legacy virtio devices. When the ring gets active, we force little endian if the device is modern. When the ring is deactivated, we revert to the native endian default.

If cross-endian was compiled in, a vq->user_be boolean field is added so that userspace may request a specific endianness. This field is used to override the default when activating the ring of a legacy device. It has no effect on modern devices.

Signed-off-by Greg Kurz <[email protected]>

---

Changes since v4:
- rewrote patch title to mention cross-endian
- renamed config to VHOST_CROSS_ENDIAN_LEGACY
- rewrote config description and help
- moved ifdefery to top of vhost.c
- added a detailed comment about the lifecycle of vq->user_be in vhost_init_is_le()
- renamed ioctls to VHOST_[GS]ET_VRING_ENDIAN
- added LE/BE defines to the ioctl API
- rewrote ioctl sanity check with the LE/BE defines
- updated comment in <uapi/linux/vhost.h> to mention that the availibility of both SET and GET ioctls depends on the kernel config

 drivers/vhost/Kconfig      |   15 ++++++++
 drivers/vhost/vhost.c      |   86 +++++++++++++++++++++++++++++++++++++++++++-
 drivers/vhost/vhost.h      |   10 +++++
 include/uapi/linux/vhost.h |   12 ++++++
 4 files changed, 121 insertions(+), 2 deletions(-)
drivers/vhost/Kconfig +0 -0
index 017a1e8..74d7380 100644

drivers/vhost/Kconfig config VHOST · line 41

Quoted 10 lines
drivers/vhost/Kconfig +7 -0
@@ -32,3 +32,18 @@ config VHOST
 	---help---
 	  This option is selected by any driver which needs to access
 	  the core of vhost.
+
+config VHOST_CROSS_ENDIAN_LEGACY
+	bool "Cross-endian support for vhost"
+	default n
+	---help---
+	  This option allows vhost to support guests with a different byte
+	  ordering from host.

"...while using legacy virtio."

Quoted 2 lines

Might help to explain the "LEGACY" in the config option ;)

Makes sense indeed !

Quoted 9 lines

drivers/vhost/Kconfig config VHOST · line 47

Quoted 6 lines
drivers/vhost/Kconfig +6 -0
+
+	  Userspace programs can control the feature using the
+	  VHOST_SET_VRING_ENDIAN and VHOST_GET_VRING_ENDIAN ioctls.
+
+	  This is only useful on a few platforms (ppc64 and arm64). Since it
+	  adds some overhead, it is disabled default.

s/default/by default/

Ok.

Quoted 56 lines
Quoted 56 lines
drivers/vhost/Kconfig +2 -0
+
+	  If unsure, say "N".
drivers/vhost/vhost.c +46 -0
index 2ee2826..8c4390d 100644
@@ -36,6 +36,78 @@ enum {
 #define vhost_used_event(vq) ((__virtio16 __user *)&vq->avail->ring[vq->num])
 #define vhost_avail_event(vq) ((__virtio16 __user *)&vq->used->ring[vq->num])
 
+#ifdef CONFIG_VHOST_CROSS_ENDIAN_LEGACY
+static void vhost_vq_reset_user_be(struct vhost_virtqueue *vq)
+{
+	vq->user_be = !virtio_legacy_is_little_endian();
+}
+
+static long vhost_set_vring_endian(struct vhost_virtqueue *vq, int __user *argp)
+{
+	struct vhost_vring_state s;
+
+	if (vq->private_data)
+		return -EBUSY;
+
+	if (copy_from_user(&s, argp, sizeof(s)))
+		return -EFAULT;
+
+	if (s.num != VHOST_VRING_LITTLE_ENDIAN &&
+	    s.num != VHOST_VRING_BIG_ENDIAN)
+		return -EINVAL;
+
+	vq->user_be = s.num;
+
+	return 0;
+}
+
+static long vhost_get_vring_endian(struct vhost_virtqueue *vq, u32 idx,
+				   int __user *argp)
+{
+	struct vhost_vring_state s = {
+		.index = idx,
+		.num = vq->user_be
+	};
+
+	if (copy_to_user(argp, &s, sizeof(s)))
+		return -EFAULT;
+
+	return 0;
+}
+
+static void vhost_init_is_le(struct vhost_virtqueue *vq)
+{
+	/* Note for legacy virtio: user_be is initialized at reset time
+	 * according to the host endianness. If userspace does not set an
+	 * explicit endianness, the default behavior is native endian, as
+	 * expected by legacy virtio.
+	 */
Quoted 9 lines

drivers/vhost/vhost.c enum { · line 90

Quoted 6 lines
drivers/vhost/vhost.c +6 -0
+	vq->is_le = vhost_has_feature(vq, VIRTIO_F_VERSION_1) || !vq->user_be;
+}
+#else
+static void vhost_vq_reset_user_be(struct vhost_virtqueue *vq)
+{
+	;

Just leave the function body empty?

Ok.

Quoted 32 lines
Quoted 23 lines
drivers/vhost/vhost.c +20 -0
+}
+
+static long vhost_set_vring_endian(struct vhost_virtqueue *vq, int __user *argp)
+{
+	return -ENOIOCTLCMD;
+}
+
+static long vhost_get_vring_endian(struct vhost_virtqueue *vq, u32 idx,
+				   int __user *argp)
+{
+	return -ENOIOCTLCMD;
+}
+
+static void vhost_init_is_le(struct vhost_virtqueue *vq)
+{
+	if (vhost_has_feature(vq, VIRTIO_F_VERSION_1))
+		vq->is_le = true;
+}
+#endif /* CONFIG_VHOST_CROSS_ENDIAN_LEGACY */
+
 static void vhost_poll_func(struct file *file, wait_queue_head_t *wqh,
 			    poll_table *pt)
 {

(...)

Quoted 8 lines
include/uapi/linux/vhost.h +0 -0
index bb6a5b4..b980b53 100644
@@ -103,6 +103,18 @@ struct vhost_memory {
 /* Get accessor: reads index, writes value in num */
 #define VHOST_GET_VRING_BASE _IOWR(VHOST_VIRTIO, 0x12, struct vhost_vring_state)
Quoted 5 lines

include/uapi/linux/vhost.h struct vhost_memory { · line 107

Quoted 2 lines
include/uapi/linux/vhost.h +2 -0
+/* Set the vring byte order in num. Valid values are VHOST_VRING_LITTLE_ENDIAN
+ * or VHOST_VRING_BIG_ENDIAN (other values return EINVAL).

-EINVAL?

Oops, yes. :)

Quoted 2 lines

Should you also mention when you return -EBUSY?

Indeed... what about:

"The byte order cannot be changed when the device is active: trying to do so returns -EBUSY."

Quoted 6 lines
Quoted 6 lines
include/uapi/linux/vhost.h +6 -0
+ * This is a legacy only API that is simply ignored when VIRTIO_F_VERSION_1 is
+ * set.
+ * Not all kernel configurations support this ioctl, but all configurations that
+ * support SET also support GET.
+ */
+#define VHOST_VRING_LITTLE_ENDIAN 0
Quoted 9 lines

include/uapi/linux/vhost.h struct vhost_memory { · line 119

Quoted 6 lines
include/uapi/linux/vhost.h +4 -0
+#define VHOST_VRING_BIG_ENDIAN 1
+#define VHOST_SET_VRING_ENDIAN _IOW(VHOST_VIRTIO, 0x13, struct vhost_vring_state)
+#define VHOST_GET_VRING_ENDIAN _IOW(VHOST_VIRTIO, 0x14, struct vhost_vring_state)
+
 /* The following ioctls use eventfd file descriptors to signal and poll
  * for events. */

Apart from style nitpicks, looks good.

Cool ! :)

Quoted 5 lines

--
To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

#29 lore raw

Re: [PATCH v5 7/8] vhost: cross-endian support for legacy devices

Quoted 22 lines · On Fri, 24 Apr 2015 10:06:19 +0200 Greg Kurz <[email protected]> wrote:
Quoted 11 lines · On Fri, 24 Apr 2015 09:19:26 +0200 Cornelia Huck <[email protected]>…
Quoted 8 lines · On Thu, 23 Apr 2015 17:29:42 +0200 Greg Kurz <[email protected]> wrote:
include/uapi/linux/vhost.h +0 -0
index bb6a5b4..b980b53 100644
@@ -103,6 +103,18 @@ struct vhost_memory {
 /* Get accessor: reads index, writes value in num */
 #define VHOST_GET_VRING_BASE _IOWR(VHOST_VIRTIO, 0x12, struct vhost_vring_state)
Quoted 5 lines

include/uapi/linux/vhost.h struct vhost_memory { · line 107

Quoted 2 lines
include/uapi/linux/vhost.h +2 -0
+/* Set the vring byte order in num. Valid values are VHOST_VRING_LITTLE_ENDIAN
+ * or VHOST_VRING_BIG_ENDIAN (other values return EINVAL).

-EINVAL?

Oops, yes. :)

Quoted 7 lines
Quoted 2 lines

Should you also mention when you return -EBUSY?

Indeed... what about:

"The byte order cannot be changed when the device is active: trying to do so returns -EBUSY."

s/when/while/ ?

But sounds good.

author
#30 lore raw

PATCH v5 8/8 macvtap/tun: cross-endian support for little-endian hosts

In reply to Greg Kurz

The VNET_LE flag was introduced to fix accesses to virtio 1.0 headers that are always little-endian. It can also be used to handle the special case of a legacy little-endian device implemented by a big-endian host.

Let's add a flag and ioctls for big-endian devices as well. If both flags are set, little-endian wins.

Since this is isn't a common usecase, the feature is controlled by a kernel config option (not set by default).

Both macvtap and tun are covered by this patch since they share the same API with userland.

Signed-off-by Greg Kurz <[email protected]>

---

Changes since v4:
- rewrote patch title to mention cross-endian
- renamed config to TUN_VNET_CROSS_LE
- rewrote config description and help
- moved ifdefery to top of tun.c and macvtap.c
- updated comment in <uapi/linux/if_tun.h> to mention that the availibility of both SET and GET ioctls depends on the kernel config

 drivers/net/Kconfig         |   14 ++++++++++
 drivers/net/macvtap.c       |   58 +++++++++++++++++++++++++++++++++++++++++-
 drivers/net/tun.c           |   60 ++++++++++++++++++++++++++++++++++++++++++-
 include/uapi/linux/if_tun.h |    6 ++++
 4 files changed, 136 insertions(+), 2 deletions(-)
drivers/net/Kconfig +14 -0
index df51d60..71ac0ec 100644
@@ -244,6 +244,20 @@ config TUN
 
 	  If you don't know what to use this for, you don't need it.
 
+config TUN_VNET_CROSS_LE
+	bool "Support for cross-endian vnet headers on little-endian kernels"
+	default n
+	---help---
+	  This option allows TUN/TAP and MACVTAP device drivers in a
+	  little-endian kernel to parse vnet headers that come from a
+	  big-endian legacy virtio device.
+
+	  Userspace programs can control the feature using the TUNSETVNETBE
+	  and TUNGETVNETBE ioctls.
+
+	  Unless you have a little-endian system hosting a big-endian virtual
+	  machine with a legacy virtio NIC, you should say N.
+
 config VETH
 	tristate "Virtual ethernet pair device"
 	---help---
drivers/net/macvtap.c +57 -1

76 lines not on this page · raw message

drivers/net/tun.c +59 -1

83 lines not on this page · raw message

include/uapi/linux/if_tun.h +6 -0
index 50ae243..3cb5e1d 100644
@@ -50,6 +50,12 @@
 #define TUNGETFILTER _IOR('T', 219, struct sock_fprog)
 #define TUNSETVNETLE _IOW('T', 220, int)
 #define TUNGETVNETLE _IOR('T', 221, int)
+/* The TUNSETVNETBE and TUNGETVNETBE ioctls are for cross-endian support on
+ * little-endian hosts. Not all kernel configurations support them, but all
+ * configurations that support SET also support GET.
+ */
+#define TUNSETVNETBE _IOW('T', 222, int)
+#define TUNGETVNETBE _IOR('T', 223, int)
 
 /* TUNSETIFF ifr flags */
 #define IFF_TUN		0x0001
author
#31 lore raw

PATCH v5 8/8 macvtap/tun: cross-endian support for little-endian hosts

In reply to Greg Kurz

The VNET_LE flag was introduced to fix accesses to virtio 1.0 headers that are always little-endian. It can also be used to handle the special case of a legacy little-endian device implemented by a big-endian host.

Let's add a flag and ioctls for big-endian devices as well. If both flags are set, little-endian wins.

Since this is isn't a common usecase, the feature is controlled by a kernel config option (not set by default).

Both macvtap and tun are covered by this patch since they share the same API with userland.

Signed-off-by Greg Kurz <[email protected]>

---

Changes since v4:
- rewrote patch title to mention cross-endian
- renamed config to TUN_VNET_CROSS_LE
- rewrote config description and help
- moved ifdefery to top of tun.c and macvtap.c
- updated comment in <uapi/linux/if_tun.h> to mention that the availibility of both SET and GET ioctls depends on the kernel config

 drivers/net/Kconfig         |   14 ++++++++++
 drivers/net/macvtap.c       |   58 +++++++++++++++++++++++++++++++++++++++++-
 drivers/net/tun.c           |   60 ++++++++++++++++++++++++++++++++++++++++++-
 include/uapi/linux/if_tun.h |    6 ++++
 4 files changed, 136 insertions(+), 2 deletions(-)
drivers/net/Kconfig +14 -0
index df51d60..71ac0ec 100644
@@ -244,6 +244,20 @@ config TUN
 
 	  If you don't know what to use this for, you don't need it.
 
+config TUN_VNET_CROSS_LE
+	bool "Support for cross-endian vnet headers on little-endian kernels"
+	default n
+	---help---
+	  This option allows TUN/TAP and MACVTAP device drivers in a
+	  little-endian kernel to parse vnet headers that come from a
+	  big-endian legacy virtio device.
+
+	  Userspace programs can control the feature using the TUNSETVNETBE
+	  and TUNGETVNETBE ioctls.
+
+	  Unless you have a little-endian system hosting a big-endian virtual
+	  machine with a legacy virtio NIC, you should say N.
+
 config VETH
 	tristate "Virtual ethernet pair device"
 	---help---
drivers/net/macvtap.c +57 -1

76 lines not on this page · raw message

drivers/net/tun.c +59 -1

83 lines not on this page · raw message

include/uapi/linux/if_tun.h +6 -0
index 50ae243..3cb5e1d 100644
@@ -50,6 +50,12 @@
 #define TUNGETFILTER _IOR('T', 219, struct sock_fprog)
 #define TUNSETVNETLE _IOW('T', 220, int)
 #define TUNGETVNETLE _IOR('T', 221, int)
+/* The TUNSETVNETBE and TUNGETVNETBE ioctls are for cross-endian support on
+ * little-endian hosts. Not all kernel configurations support them, but all
+ * configurations that support SET also support GET.
+ */
+#define TUNSETVNETBE _IOW('T', 222, int)
+#define TUNGETVNETBE _IOR('T', 223, int)
 
 /* TUNSETIFF ifr flags */
 #define IFF_TUN		0x0001