[PATCH 0/7] kvmtool: handle guests of a different endianness

[PATCH 0/7] kvmtool: handle guests of a different endianness

[Marc Zyngier]

This patch series adds some infrastructure to kvmtool to allow a BE
guest to use virtio-mmio on a LE host, provided that the architecture
actually supports such madness.

Not all the backend have been converted, only those I actually cared
about. Converting them is pretty easy though, and will be done if the
method is deemed acceptable.

This has been tested on arm64, with some fixes to KVM and a set of
changes to virtio, both which I am posting separately.

A branch containing all the relevant changes is at:
git://git.kernel.org/pub/scm/linux/kernel/git/maz/arm-platforms.git kvm-arm64/be-on-le-3.12-rc4

Cc: Pekka Enberg <penberg@kernel.org>
Cc: Will Deacon <will.deacon@arm.com>

Marc Zyngier (7):
  kvmtool: virt_queue configuration based on endianness
  kvmtool: virt_queue: handle guest endianness
  kvmtool: convert console backend to support bi-endianness
  kvmtool: convert 9p backend to support bi-endianness
  kvmtool: convert blk backend to support bi-endianness
  kvmtool: convert net backend to support bi-endianness
  kvmtool: virtio: enable arm/arm64 support for bi-endianness

 tools/kvm/arm/include/arm-common/kvm-arch.h |   2 +
 tools/kvm/include/kvm/virtio.h              | 198 +++++++++++++++++++++++++++-
 tools/kvm/virtio/9p.c                       |   5 +-
 tools/kvm/virtio/blk.c                      |  34 ++++-
 tools/kvm/virtio/console.c                  |  12 +-
 tools/kvm/virtio/core.c                     |  59 +++++----
 tools/kvm/virtio/net.c                      |  17 ++-
 7 files changed, 286 insertions(+), 41 deletions(-)

-- 
1.8.2.3

[PATCH 1/7] kvmtool: virt_queue configuration based on endianness

[Marc Zyngier]

Define a simple infrastructure to configure a virt_queue
depending on the guest endianness, as reported by the feature
flags. At this stage, the endianness is always the host's.

Cc: Pekka Enberg <penberg@kernel.org>
Cc: Will Deacon <will.deacon@arm.com>
Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
---
 tools/kvm/include/kvm/virtio.h | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/tools/kvm/include/kvm/virtio.h b/tools/kvm/include/kvm/virtio.h
index 820b94a..d6b0f47 100644
--- a/tools/kvm/include/kvm/virtio.h
+++ b/tools/kvm/include/kvm/virtio.h
@@ -15,6 +15,10 @@
 #define VIRTIO_PCI_O_CONFIG	0
 #define VIRTIO_PCI_O_MSIX	1
 
+#define VIRTIO_ENDIAN_HOST	0
+#define VIRTIO_ENDIAN_LE	1
+#define VIRTIO_ENDIAN_BE	2
+
 struct virt_queue {
 	struct vring	vring;
 	u32		pfn;
@@ -22,8 +26,15 @@ struct virt_queue {
 	   It's where we assume the next request index is at.  */
 	u16		last_avail_idx;
 	u16		last_used_signalled;
+	u16		endian;
 };
 
+
+static inline void virt_queue__init(struct virt_queue *vq, u32 features)
+{
+	vq->endian = VIRTIO_ENDIAN_HOST;
+}
+
 static inline u16 virt_queue__pop(struct virt_queue *queue)
 {
 	return queue->vring.avail->ring[queue->last_avail_idx++ % queue->vring.num];
-- 
1.8.2.3

[PATCH 2/7] kvmtool: virt_queue: handle guest endianness

[Marc Zyngier]

Wrap all accesses to virt_queue data structures shared between
host and guest with byte swapping helpers.

Should the architecture only support one endianness, these helpers
are reduced to the identity function.

Cc: Pekka Enberg <penberg@kernel.org>
Cc: Will Deacon <will.deacon@arm.com>
Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
---
 tools/kvm/include/kvm/virtio.h | 189 ++++++++++++++++++++++++++++++++++++++++-
 tools/kvm/virtio/core.c        |  59 +++++++------
 2 files changed, 219 insertions(+), 29 deletions(-)

diff --git a/tools/kvm/include/kvm/virtio.h b/tools/kvm/include/kvm/virtio.h
index d6b0f47..04ec137 100644
--- a/tools/kvm/include/kvm/virtio.h
+++ b/tools/kvm/include/kvm/virtio.h
@@ -1,6 +1,8 @@
 #ifndef KVM__VIRTIO_H
 #define KVM__VIRTIO_H
 
+#include <endian.h>
+
 #include <linux/virtio_ring.h>
 #include <linux/virtio_pci.h>
 
@@ -29,15 +31,194 @@ struct virt_queue {
 	u16		endian;
 };
 
+/*
+ * The default policy is not to cope with the guest endianness.
+ * It also helps not breaking archs that do not care about supporting
+ * such a configuration.
+ */
+#ifndef VIRTIO_RING_ENDIAN
+#define VIRTIO_RING_ENDIAN 0
+#endif
+
+#if (VIRTIO_RING_ENDIAN & ((1UL << VIRTIO_RING_F_GUEST_LE) | (1UL << VIRTIO_RING_F_GUEST_BE)))
+
+#ifndef __BYTE_ORDER
+#error "No byteorder? Giving up..."
+#endif
+
+
+static inline __u16 __virtio_guest_to_host_u16(u16 endian, __u16 val)
+{
+#if __BYTE_ORDER == __BIG_ENDIAN
+	if (endian == VIRTIO_ENDIAN_LE)
+		return le16toh(val);
+#else
+	if (endian == VIRTIO_ENDIAN_BE)
+		return be16toh(val);
+#endif
+
+	return val;
+}
+
+static inline __u16 virtio_guest_to_host_u16(struct virt_queue *vq, __u16 val)
+{
+	return __virtio_guest_to_host_u16(vq->endian, val);
+}
+
+static inline __u16 __virtio_host_to_guest_u16(u16 endian, __u16 val)
+{
+#if __BYTE_ORDER == __BIG_ENDIAN
+	if (endian == VIRTIO_ENDIAN_LE)
+		return htole16(val);
+#else
+	if (endian == VIRTIO_ENDIAN_BE)
+		return htobe16(val);
+#endif
+
+	return val;
+}
+
+static inline __u16 virtio_host_to_guest_u16(struct virt_queue *vq, __u16 val)
+{
+	return __virtio_host_to_guest_u16(vq->endian, val);
+}
+
+static inline __u32 __virtio_guest_to_host_u32(u16 endian, __u32 val)
+{
+#if __BYTE_ORDER == __BIG_ENDIAN
+	if (endian == VIRTIO_ENDIAN_LE)
+		return le32toh(val);
+#else
+	if (endian == VIRTIO_ENDIAN_BE)
+		return be32toh(val);
+#endif
+
+	return val;
+}
+
+static inline __u32 virtio_guest_to_host_u32(struct virt_queue *vq, __u32 val)
+{
+	return __virtio_guest_to_host_u32(vq->endian, val);
+}
+
+static inline __u32 __virtio_host_to_guest_u32(u16 endian, __u32 val)
+{
+#if __BYTE_ORDER == __BIG_ENDIAN
+	if (endian == VIRTIO_ENDIAN_LE)
+		return htole32(val);
+#else
+	if (endian == VIRTIO_ENDIAN_BE)
+		return htobe32(val);
+#endif
+
+	return val;
+}
+
+static inline __u32 virtio_host_to_guest_u32(struct virt_queue *vq, __u32 val)
+{
+	return __virtio_host_to_guest_u32(vq->endian, val);
+}
+
+static inline __u64 __virtio_guest_to_host_u64(u16 endian, __u64 val)
+{
+#if __BYTE_ORDER == __BIG_ENDIAN
+	if (endian == VIRTIO_ENDIAN_LE)
+		return le64toh(val);
+#else
+	if (endian == VIRTIO_ENDIAN_BE)
+		return be64toh(val);
+#endif
+
+	return val;
+}
+
+static inline __u64 virtio_guest_to_host_u64(struct virt_queue *vq, __u64 val)
+{
+	return __virtio_guest_to_host_u64(vq->endian, val);
+}
+
+static inline __u64 __virtio_host_to_guest_u64(u16 endian, __u64 val)
+{
+#if __BYTE_ORDER == __BIG_ENDIAN
+	if (endian == VIRTIO_ENDIAN_LE)
+		return htole64(val);
+#else
+	if (endian == VIRTIO_ENDIAN_BE)
+		return htobe64(val);
+#endif
+
+	return val;
+}
+
+static inline __u64 virtio_host_to_guest_u64(struct virt_queue *vq, __u64 val)
+{
+	return __virtio_host_to_guest_u64(vq->endian, val);
+}
+
+static inline u16 virtio_features_to_endian(u32 features)
+{
+	u16 endian = VIRTIO_ENDIAN_HOST;
+
+#if __BYTE_ORDER == __BIG_ENDIAN
+	if (features & (1 << VIRTIO_RING_F_GUEST_LE))
+		endian = VIRTIO_ENDIAN_LE;
+#else
+	if (features & (1 << VIRTIO_RING_F_GUEST_BE))
+		endian = VIRTIO_ENDIAN_BE;
+#endif
+
+	return endian;
+}
+
+#else
+
+static inline __u16 virtio_guest_to_host_u16(struct virt_queue *vq, __u16 val)
+{
+	return val;
+}
+
+static inline __u16 virtio_host_to_guest_u16(struct virt_queue *vq, __u16 val)
+{
+	return val;
+}
+
+static inline __u32 virtio_guest_to_host_u32(struct virt_queue *vq, __u32 val)
+{
+	return val;
+}
+
+static inline __u32 virtio_host_to_guest_u32(struct virt_queue *vq, __u32 val)
+{
+	return val;
+}
+
+static inline __u64 virtio_guest_to_host_u64(struct virt_queue *vq, __u64 val)
+{
+	return val;
+}
+
+static inline __u64 virtio_host_to_guest_u64(struct virt_queue *vq, __u64 val)
+{
+	return val;
+}
+
+static inline u16 virtio_features_to_endian(u32 features)
+{
+	return VIRTIO_ENDIAN_HOST;
+}
+#endif
 
 static inline void virt_queue__init(struct virt_queue *vq, u32 features)
 {
-	vq->endian = VIRTIO_ENDIAN_HOST;
+	vq->endian = virtio_features_to_endian(features);
 }
 
 static inline u16 virt_queue__pop(struct virt_queue *queue)
 {
-	return queue->vring.avail->ring[queue->last_avail_idx++ % queue->vring.num];
+	__u16 guest_idx;
+
+	guest_idx = queue->vring.avail->ring[queue->last_avail_idx++ % queue->vring.num];
+	return virtio_guest_to_host_u16(queue, guest_idx);
 }
 
 static inline struct vring_desc *virt_queue__get_desc(struct virt_queue *queue, u16 desc_ndx)
@@ -50,8 +231,8 @@ static inline bool virt_queue__available(struct virt_queue *vq)
 	if (!vq->vring.avail)
 		return 0;
 
-	vring_avail_event(&vq->vring) = vq->last_avail_idx;
-	return vq->vring.avail->idx !=  vq->last_avail_idx;
+	vring_avail_event(&vq->vring) = virtio_host_to_guest_u16(vq, vq->last_avail_idx);
+	return virtio_guest_to_host_u16(vq, vq->vring.avail->idx) != vq->last_avail_idx;
 }
 
 struct vring_used_elem *virt_queue__set_used_elem(struct virt_queue *queue, u32 head, u32 len);
diff --git a/tools/kvm/virtio/core.c b/tools/kvm/virtio/core.c
index 2dfb828..9ae7887 100644
--- a/tools/kvm/virtio/core.c
+++ b/tools/kvm/virtio/core.c
@@ -15,10 +15,11 @@
 struct vring_used_elem *virt_queue__set_used_elem(struct virt_queue *queue, u32 head, u32 len)
 {
 	struct vring_used_elem *used_elem;
+	u16 idx = virtio_guest_to_host_u16(queue, queue->vring.used->idx);
 
-	used_elem	= &queue->vring.used->ring[queue->vring.used->idx % queue->vring.num];
-	used_elem->id	= head;
-	used_elem->len	= len;
+	used_elem	= &queue->vring.used->ring[idx % queue->vring.num];
+	used_elem->id	= virtio_host_to_guest_u32(queue, head);
+	used_elem->len	= virtio_host_to_guest_u32(queue, len);
 
 	/*
 	 * Use wmb to assure that used elem was updated with head and len.
@@ -26,7 +27,8 @@ struct vring_used_elem *virt_queue__set_used_elem(struct virt_queue *queue, u32
 	 * to pass the used element to the guest.
 	 */
 	wmb();
-	queue->vring.used->idx++;
+	idx++;
+	queue->vring.used->idx = virtio_host_to_guest_u16(queue, idx);
 
 	/*
 	 * Use wmb to assure used idx has been increased before we signal the guest.
@@ -38,22 +40,28 @@ struct vring_used_elem *virt_queue__set_used_elem(struct virt_queue *queue, u32
 	return used_elem;
 }
 
+static inline bool virt_desc__test_flag(struct virt_queue *vq,
+					struct vring_desc *desc, u16 flag)
+{
+	return !!(virtio_guest_to_host_u16(vq, desc->flags) & flag);
+}
+
 /*
  * Each buffer in the virtqueues is actually a chain of descriptors.  This
  * function returns the next descriptor in the chain, or vq->vring.num if we're
  * at the end.
  */
-static unsigned next_desc(struct vring_desc *desc,
+static unsigned next_desc(struct virt_queue *vq, struct vring_desc *desc,
 			  unsigned int i, unsigned int max)
 {
 	unsigned int next;
 
 	/* If this descriptor says it doesn't chain, we're done. */
-	if (!(desc[i].flags & VRING_DESC_F_NEXT))
+	if (!virt_desc__test_flag(vq, &desc[i], VRING_DESC_F_NEXT))
 		return max;
 
 	/* Check they're not leading us off end of descriptors. */
-	next = desc[i].next;
+	next = virtio_guest_to_host_u16(vq, desc[i].next);
 	/* Make sure compiler knows to grab that: we don't want it changing! */
 	wmb();
 
@@ -71,22 +79,23 @@ u16 virt_queue__get_head_iov(struct virt_queue *vq, struct iovec iov[], u16 *out
 	max = vq->vring.num;
 	desc = vq->vring.desc;
 
-	if (desc[idx].flags & VRING_DESC_F_INDIRECT) {
-		max = desc[idx].len / sizeof(struct vring_desc);
-		desc = guest_flat_to_host(kvm, desc[idx].addr);
+	if (virt_desc__test_flag(vq, &desc[idx], VRING_DESC_F_INDIRECT)) {
+		max = virtio_guest_to_host_u32(vq, desc[idx].len) / sizeof(struct vring_desc);
+		desc = guest_flat_to_host(kvm, virtio_guest_to_host_u64(vq, desc[idx].addr));
 		idx = 0;
 	}
 
 	do {
 		/* Grab the first descriptor, and check it's OK. */
-		iov[*out + *in].iov_len = desc[idx].len;
-		iov[*out + *in].iov_base = guest_flat_to_host(kvm, desc[idx].addr);
+		iov[*out + *in].iov_len = virtio_guest_to_host_u32(vq, desc[idx].len);
+		iov[*out + *in].iov_base = guest_flat_to_host(kvm,
+							      virtio_guest_to_host_u64(vq, desc[idx].addr));
 		/* If this is an input descriptor, increment that count. */
-		if (desc[idx].flags & VRING_DESC_F_WRITE)
+		if (virt_desc__test_flag(vq, &desc[idx], VRING_DESC_F_WRITE))
 			(*in)++;
 		else
 			(*out)++;
-	} while ((idx = next_desc(desc, idx, max)) != max);
+	} while ((idx = next_desc(vq, desc, idx, max)) != max);
 
 	return head;
 }
@@ -111,20 +120,20 @@ u16 virt_queue__get_inout_iov(struct kvm *kvm, struct virt_queue *queue,
 	idx = head = virt_queue__pop(queue);
 	*out = *in = 0;
 	do {
+		u64 addr;
 		desc = virt_queue__get_desc(queue, idx);
-		if (desc->flags & VRING_DESC_F_WRITE) {
-			in_iov[*in].iov_base = guest_flat_to_host(kvm,
-								  desc->addr);
-			in_iov[*in].iov_len = desc->len;
+		addr = virtio_guest_to_host_u64(queue, desc->addr);
+		if (virt_desc__test_flag(queue, desc, VRING_DESC_F_WRITE)) {
+			in_iov[*in].iov_base = guest_flat_to_host(kvm, addr);
+			in_iov[*in].iov_len = virtio_guest_to_host_u32(queue, desc->len);
 			(*in)++;
 		} else {
-			out_iov[*out].iov_base = guest_flat_to_host(kvm,
-								    desc->addr);
-			out_iov[*out].iov_len = desc->len;
+			out_iov[*out].iov_base = guest_flat_to_host(kvm, addr);
+			out_iov[*out].iov_len = virtio_guest_to_host_u32(queue, desc->len);
 			(*out)++;
 		}
-		if (desc->flags & VRING_DESC_F_NEXT)
-			idx = desc->next;
+		if (virt_desc__test_flag(queue, desc, VRING_DESC_F_NEXT))
+			idx = virtio_guest_to_host_u16(queue, desc->next);
 		else
 			break;
 	} while (1);
@@ -151,8 +160,8 @@ bool virtio_queue__should_signal(struct virt_queue *vq)
 	u16 old_idx, new_idx, event_idx;
 
 	old_idx		= vq->last_used_signalled;
-	new_idx		= vq->vring.used->idx;
-	event_idx	= vring_used_event(&vq->vring);
+	new_idx		= virtio_guest_to_host_u16(vq, vq->vring.used->idx);
+	event_idx	= virtio_guest_to_host_u16(vq, vring_used_event(&vq->vring));
 
 	if (vring_need_event(event_idx, new_idx, old_idx)) {
 		vq->last_used_signalled = new_idx;
-- 
1.8.2.3

[PATCH 2/7] kvmtool: virt_queue: handle guest endianness

[Will Deacon]

On Fri, Oct 11, 2013 at 03:36:30PM +0100, Marc Zyngier wrote:
> Wrap all accesses to virt_queue data structures shared between
> host and guest with byte swapping helpers.
> 
> Should the architecture only support one endianness, these helpers
> are reduced to the identity function.
> 
> Cc: Pekka Enberg <penberg@kernel.org>
> Cc: Will Deacon <will.deacon@arm.com>
> Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
> ---
>  tools/kvm/include/kvm/virtio.h | 189 ++++++++++++++++++++++++++++++++++++++++-
>  tools/kvm/virtio/core.c        |  59 +++++++------
>  2 files changed, 219 insertions(+), 29 deletions(-)
> 
> diff --git a/tools/kvm/include/kvm/virtio.h b/tools/kvm/include/kvm/virtio.h
> index d6b0f47..04ec137 100644
> --- a/tools/kvm/include/kvm/virtio.h
> +++ b/tools/kvm/include/kvm/virtio.h
> @@ -1,6 +1,8 @@
>  #ifndef KVM__VIRTIO_H
>  #define KVM__VIRTIO_H
>  
> +#include <endian.h>
> +
>  #include <linux/virtio_ring.h>
>  #include <linux/virtio_pci.h>
>  
> @@ -29,15 +31,194 @@ struct virt_queue {
>  	u16		endian;
>  };
>  
> +/*
> + * The default policy is not to cope with the guest endianness.
> + * It also helps not breaking archs that do not care about supporting
> + * such a configuration.
> + */

Jesus Marc, are you *trying* to crash my preprocessor? Seriously though,
maybe this is better done as a block:

#if __BYTE_ORDER == __BIG_ENDIAN
#define	virtio_le16toh(x)	le16toh(x)
#define virtio_be16toh(x)
[...]

> +#ifndef VIRTIO_RING_ENDIAN
> +#define VIRTIO_RING_ENDIAN 0
> +#endif
> +
> +#if (VIRTIO_RING_ENDIAN & ((1UL << VIRTIO_RING_F_GUEST_LE) | (1UL << VIRTIO_RING_F_GUEST_BE)))
> +
> +#ifndef __BYTE_ORDER
> +#error "No byteorder? Giving up..."
> +#endif
> +
> +
> +static inline __u16 __virtio_guest_to_host_u16(u16 endian, __u16 val)
> +{
> +#if __BYTE_ORDER == __BIG_ENDIAN
> +	if (endian == VIRTIO_ENDIAN_LE)
> +		return le16toh(val);
> +#else
> +	if (endian == VIRTIO_ENDIAN_BE)
> +		return be16toh(val);
> +#endif

Then you can just use the endian parameter to do the right thing.

Will

[PATCH 2/7] kvmtool: virt_queue: handle guest endianness

[Robin Murphy]

On 11/10/13 15:50, Will Deacon wrote:
> On Fri, Oct 11, 2013 at 03:36:30PM +0100, Marc Zyngier wrote:
>> Wrap all accesses to virt_queue data structures shared between
>> host and guest with byte swapping helpers.
>>
>> Should the architecture only support one endianness, these helpers
>> are reduced to the identity function.
>>
>> Cc: Pekka Enberg <penberg@kernel.org>
>> Cc: Will Deacon <will.deacon@arm.com>
>> Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
>> ---
>>   tools/kvm/include/kvm/virtio.h | 189 ++++++++++++++++++++++++++++++++++++++++-
>>   tools/kvm/virtio/core.c        |  59 +++++++------
>>   2 files changed, 219 insertions(+), 29 deletions(-)
>>
>> diff --git a/tools/kvm/include/kvm/virtio.h b/tools/kvm/include/kvm/virtio.h
>> index d6b0f47..04ec137 100644
>> --- a/tools/kvm/include/kvm/virtio.h
>> +++ b/tools/kvm/include/kvm/virtio.h
>> @@ -1,6 +1,8 @@
>>   #ifndef KVM__VIRTIO_H
>>   #define KVM__VIRTIO_H
>>
>> +#include <endian.h>
>> +
>>   #include <linux/virtio_ring.h>
>>   #include <linux/virtio_pci.h>
>>
>> @@ -29,15 +31,194 @@ struct virt_queue {
>>   	u16		endian;
>>   };
>>
>> +/*
>> + * The default policy is not to cope with the guest endianness.
>> + * It also helps not breaking archs that do not care about supporting
>> + * such a configuration.
>> + */
>
> Jesus Marc, are you *trying* to crash my preprocessor? Seriously though,
> maybe this is better done as a block:
>
> #if __BYTE_ORDER == __BIG_ENDIAN
> #define	virtio_le16toh(x)	le16toh(x)
> #define virtio_be16toh(x)
> [...]
>

The preprocessor magic to turn the functions into one-liners is pretty 
gruesome in itself, though...

>> +#ifndef VIRTIO_RING_ENDIAN
>> +#define VIRTIO_RING_ENDIAN 0
>> +#endif
>> +
>> +#if (VIRTIO_RING_ENDIAN & ((1UL << VIRTIO_RING_F_GUEST_LE) | (1UL << VIRTIO_RING_F_GUEST_BE)))
>> +
>> +#ifndef __BYTE_ORDER
>> +#error "No byteorder? Giving up..."
>> +#endif

#ifdef __BYTE_ORDER
#if __BYTE_ORDER == __BIG_ENDIAN
#define BYTEORDER_TOKEN	be
#define ENDIAN_OPPOSITE	VIRTIO_ENDIAN_LE
#else
#define BYTEORDER_TOKEN	le
#define ENDIAN_OPPOSITE	VIRTIO_ENDIAN_BE
#endif
#define _CAT3(a,b,c)	a##b##c
#define CAT3(a,b,c)	_CAT3(a,b,c)
#define vio_gtoh(size, val) (endian==ENDIAN_OPPOSITE) ?\
	CAT3(BYTEORDER_TOKEN,size,toh(val)) : val
#define vio_htog(size, val) (endian==ENDIAN_OPPOSITE) ?\
	CAT3(hto,BYTEORDER_TOKEN,size(val)) : val
#else
#error "No byteorder? Giving up..."
#endif

>> +
>> +
>> +static inline __u16 __virtio_guest_to_host_u16(u16 endian, __u16 val)
>> +{
>> +#if __BYTE_ORDER == __BIG_ENDIAN
>> +	if (endian == VIRTIO_ENDIAN_LE)
>> +		return le16toh(val);
>> +#else
>> +	if (endian == VIRTIO_ENDIAN_BE)
>> +		return be16toh(val);
>> +#endif

{
	return vio_gtoh(16, val);
}

On the upside however, it does remove all the duplication and keep all 
the mess in one place.

Robin.

>
> Then you can just use the endian parameter to do the right thing.
>
> Will
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
>

[PATCH 3/7] kvmtool: convert console backend to support bi-endianness

[Marc Zyngier]

Advertise bi-endianness support in the feature flags, and provide
byte-swapping of the config structure depending on the guest selection.

Cc: Pekka Enberg <penberg@kernel.org>
Cc: Will Deacon <will.deacon@arm.com>
Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
---
 tools/kvm/virtio/console.c | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/tools/kvm/virtio/console.c b/tools/kvm/virtio/console.c
index f982dab7..ac041df 100644
--- a/tools/kvm/virtio/console.c
+++ b/tools/kvm/virtio/console.c
@@ -126,12 +126,19 @@ static u8 *get_config(struct kvm *kvm, void *dev)
 
 static u32 get_host_features(struct kvm *kvm, void *dev)
 {
-	return 0;
+	return VIRTIO_RING_ENDIAN;
 }
 
 static void set_guest_features(struct kvm *kvm, void *dev, u32 features)
 {
-	/* Unused */
+	struct con_dev *cdev = dev;
+	struct virtio_console_config *conf = &cdev->config;
+
+	cdev->features = features;
+
+	conf->cols = htole16(conf->cols);
+	conf->rows = htole16(conf->rows);
+	conf->max_nr_ports = htole32(conf->max_nr_ports);
 }
 
 static int init_vq(struct kvm *kvm, void *dev, u32 vq, u32 page_size, u32 align,
@@ -149,6 +156,7 @@ static int init_vq(struct kvm *kvm, void *dev, u32 vq, u32 page_size, u32 align,
 	p		= virtio_get_vq(kvm, queue->pfn, page_size);
 
 	vring_init(&queue->vring, VIRTIO_CONSOLE_QUEUE_SIZE, p, align);
+	virt_queue__init(queue, cdev.features);
 
 	if (vq == VIRTIO_CONSOLE_TX_QUEUE) {
 		thread_pool__init_job(&cdev.jobs[vq], kvm, virtio_console_handle_callback, queue);
-- 
1.8.2.3

[PATCH 4/7] kvmtool: convert 9p backend to support bi-endianness

[Marc Zyngier]

Advertise bi-endianness support in the feature flags, and provide
byte-swapping of the config structure depending on the guest selection.

Cc: Pekka Enberg <penberg@kernel.org>
Cc: Will Deacon <will.deacon@arm.com>
Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
---
 tools/kvm/virtio/9p.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/tools/kvm/virtio/9p.c b/tools/kvm/virtio/9p.c
index 127b13a..b823306 100644
--- a/tools/kvm/virtio/9p.c
+++ b/tools/kvm/virtio/9p.c
@@ -1245,14 +1245,16 @@ static u8 *get_config(struct kvm *kvm, void *dev)
 
 static u32 get_host_features(struct kvm *kvm, void *dev)
 {
-	return 1 << VIRTIO_9P_MOUNT_TAG;
+	return (1 << VIRTIO_9P_MOUNT_TAG) | VIRTIO_RING_ENDIAN;
 }
 
 static void set_guest_features(struct kvm *kvm, void *dev, u32 features)
 {
 	struct p9_dev *p9dev = dev;
+	struct virtio_9p_config *conf = p9dev->config;
 
 	p9dev->features = features;
+	conf->tag_len = htole16(conf->tag_len);
 }
 
 static int init_vq(struct kvm *kvm, void *dev, u32 vq, u32 page_size, u32 align,
@@ -1271,6 +1273,7 @@ static int init_vq(struct kvm *kvm, void *dev, u32 vq, u32 page_size, u32 align,
 	job		= &p9dev->jobs[vq];
 
 	vring_init(&queue->vring, VIRTQUEUE_NUM, p, align);
+	virt_queue__init(queue, p9dev->features);
 
 	*job		= (struct p9_dev_job) {
 		.vq		= queue,
-- 
1.8.2.3

[PATCH 5/7] kvmtool: convert blk backend to support bi-endianness

[Marc Zyngier]

Advertise bi-endianness support in the feature flags, and provide
byte-swapping of the config structure depending on the guest selection.

Cc: Pekka Enberg <penberg@kernel.org>
Cc: Will Deacon <will.deacon@arm.com>
Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
---
 tools/kvm/virtio/blk.c | 34 +++++++++++++++++++++++++++-------
 1 file changed, 27 insertions(+), 7 deletions(-)

diff --git a/tools/kvm/virtio/blk.c b/tools/kvm/virtio/blk.c
index ab18716..237bf56 100644
--- a/tools/kvm/virtio/blk.c
+++ b/tools/kvm/virtio/blk.c
@@ -77,13 +77,15 @@ void virtio_blk_complete(void *param, long len)
 		bdev->vdev.ops->signal_vq(req->kvm, &bdev->vdev, queueid);
 }
 
-static void virtio_blk_do_io_request(struct kvm *kvm, struct blk_dev_req *req)
+static void virtio_blk_do_io_request(struct kvm *kvm, struct virt_queue *vq, struct blk_dev_req *req)
 {
 	struct virtio_blk_outhdr *req_hdr;
 	ssize_t block_cnt;
 	struct blk_dev *bdev;
 	struct iovec *iov;
 	u16 out, in;
+	u32 type;
+	u64 sector;
 
 	block_cnt	= -1;
 	bdev		= req->bdev;
@@ -92,13 +94,16 @@ static void virtio_blk_do_io_request(struct kvm *kvm, struct blk_dev_req *req)
 	in		= req->in;
 	req_hdr		= iov[0].iov_base;
 
-	switch (req_hdr->type) {
+	type = virtio_guest_to_host_u32(vq, req_hdr->type);
+	sector = virtio_guest_to_host_u64(vq, req_hdr->sector);
+
+	switch (type) {
 	case VIRTIO_BLK_T_IN:
-		block_cnt = disk_image__read(bdev->disk, req_hdr->sector,
+		block_cnt = disk_image__read(bdev->disk, sector,
 				iov + 1, in + out - 2, req);
 		break;
 	case VIRTIO_BLK_T_OUT:
-		block_cnt = disk_image__write(bdev->disk, req_hdr->sector,
+		block_cnt = disk_image__write(bdev->disk, sector,
 				iov + 1, in + out - 2, req);
 		break;
 	case VIRTIO_BLK_T_FLUSH:
@@ -112,7 +117,7 @@ static void virtio_blk_do_io_request(struct kvm *kvm, struct blk_dev_req *req)
 		virtio_blk_complete(req, block_cnt);
 		break;
 	default:
-		pr_warning("request type %d", req_hdr->type);
+		pr_warning("request type %d", type);
 		block_cnt	= -1;
 		break;
 	}
@@ -130,7 +135,7 @@ static void virtio_blk_do_io(struct kvm *kvm, struct virt_queue *vq, struct blk_
 					&req->in, head, kvm);
 		req->vq		= vq;
 
-		virtio_blk_do_io_request(kvm, req);
+		virtio_blk_do_io_request(kvm, vq, req);
 	}
 }
 
@@ -146,14 +151,28 @@ static u32 get_host_features(struct kvm *kvm, void *dev)
 	return	1UL << VIRTIO_BLK_F_SEG_MAX
 		| 1UL << VIRTIO_BLK_F_FLUSH
 		| 1UL << VIRTIO_RING_F_EVENT_IDX
-		| 1UL << VIRTIO_RING_F_INDIRECT_DESC;
+		| 1UL << VIRTIO_RING_F_INDIRECT_DESC
+		| VIRTIO_RING_ENDIAN;
 }
 
 static void set_guest_features(struct kvm *kvm, void *dev, u32 features)
 {
 	struct blk_dev *bdev = dev;
+	struct virtio_blk_config *conf = &bdev->blk_config;
+	struct virtio_blk_geometry *geo = &conf->geometry;
 
 	bdev->features = features;
+
+	conf->capacity = htole64(conf->capacity);
+	conf->size_max = htole32(conf->size_max);
+	conf->seg_max = htole32(conf->seg_max);
+
+	/* Geometry */
+	geo->cylinders = htole16(geo->cylinders);
+
+	conf->blk_size = htole32(conf->blk_size);
+	conf->min_io_size = htole16(conf->min_io_size);
+	conf->opt_io_size = htole32(conf->opt_io_size);
 }
 
 static int init_vq(struct kvm *kvm, void *dev, u32 vq, u32 page_size, u32 align,
@@ -170,6 +189,7 @@ static int init_vq(struct kvm *kvm, void *dev, u32 vq, u32 page_size, u32 align,
 	p		= virtio_get_vq(kvm, queue->pfn, page_size);
 
 	vring_init(&queue->vring, VIRTIO_BLK_QUEUE_SIZE, p, align);
+	virt_queue__init(queue, bdev->features);
 
 	return 0;
 }
-- 
1.8.2.3

[PATCH 6/7] kvmtool: convert net backend to support bi-endianness

[Marc Zyngier]

Advertise bi-endianness support in the feature flags, and provide
byte-swapping of the config structure depending on the guest selection.

Cc: Pekka Enberg <penberg@kernel.org>
Cc: Will Deacon <will.deacon@arm.com>
Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
---
 tools/kvm/virtio/net.c | 17 ++++++++++++++---
 1 file changed, 14 insertions(+), 3 deletions(-)

diff --git a/tools/kvm/virtio/net.c b/tools/kvm/virtio/net.c
index 2c34996..1c07a98 100644
--- a/tools/kvm/virtio/net.c
+++ b/tools/kvm/virtio/net.c
@@ -116,8 +116,10 @@ static void *virtio_net_rx_thread(void *p)
 
 				memcpy_toiovecend(iov, buffer, copied, iovsize);
 				copied += iovsize;
-				if (has_virtio_feature(ndev, VIRTIO_NET_F_MRG_RXBUF))
-					hdr->num_buffers++;
+				if (has_virtio_feature(ndev, VIRTIO_NET_F_MRG_RXBUF)) {
+					u16 num_buffers = virtio_guest_to_host_u16(vq, hdr->num_buffers);
+					hdr->num_buffers = virtio_host_to_guest_u16(vq, num_buffers + 1);
+				}
 				virt_queue__set_used_elem(vq, head, iovsize);
 				if (copied == len)
 					break;
@@ -381,14 +383,19 @@ static u32 get_host_features(struct kvm *kvm, void *dev)
 		| 1UL << VIRTIO_RING_F_INDIRECT_DESC
 		| 1UL << VIRTIO_NET_F_CTRL_VQ
 		| 1UL << VIRTIO_NET_F_MRG_RXBUF
-		| 1UL << (ndev->queue_pairs > 1 ? VIRTIO_NET_F_MQ : 0);
+		| 1UL << (ndev->queue_pairs > 1 ? VIRTIO_NET_F_MQ : 0)
+		| VIRTIO_RING_ENDIAN;
 }
 
 static void set_guest_features(struct kvm *kvm, void *dev, u32 features)
 {
 	struct net_dev *ndev = dev;
+	struct virtio_net_config *conf = &ndev->config;
 
 	ndev->features = features;
+
+	conf->status = htole16(conf->status);
+	conf->max_virtqueue_pairs = htole16(conf->max_virtqueue_pairs);
 }
 
 static bool is_ctrl_vq(struct net_dev *ndev, u32 vq)
@@ -413,6 +420,7 @@ static int init_vq(struct kvm *kvm, void *dev, u32 vq, u32 page_size, u32 align,
 	p		= virtio_get_vq(kvm, queue->pfn, page_size);
 
 	vring_init(&queue->vring, VIRTIO_NET_QUEUE_SIZE, p, align);
+	virt_queue__init(queue, ndev->features);
 
 	mutex_init(&ndev->io_lock[vq]);
 	pthread_cond_init(&ndev->io_cond[vq], NULL);
@@ -429,6 +437,9 @@ static int init_vq(struct kvm *kvm, void *dev, u32 vq, u32 page_size, u32 align,
 		return 0;
 	}
 
+	if (queue->endian != VIRTIO_ENDIAN_HOST)
+		die_perror("VHOST requires VIRTIO_ENDIAN_HOST");
+
 	state.num = queue->vring.num;
 	r = ioctl(ndev->vhost_fd, VHOST_SET_VRING_NUM, &state);
 	if (r < 0)
-- 
1.8.2.3

[PATCH 7/7] kvmtool: virtio: enable arm/arm64 support for bi-endianness

[Marc Zyngier]

Cc: Pekka Enberg <penberg@kernel.org>
Cc: Will Deacon <will.deacon@arm.com>
Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
---
 tools/kvm/arm/include/arm-common/kvm-arch.h | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/tools/kvm/arm/include/arm-common/kvm-arch.h b/tools/kvm/arm/include/arm-common/kvm-arch.h
index 72b204f..91836cb 100644
--- a/tools/kvm/arm/include/arm-common/kvm-arch.h
+++ b/tools/kvm/arm/include/arm-common/kvm-arch.h
@@ -27,6 +27,8 @@
 
 #define VIRTIO_DEFAULT_TRANS	VIRTIO_MMIO
 
+#define VIRTIO_RING_ENDIAN	((1UL << VIRTIO_RING_F_GUEST_LE) | (1UL << VIRTIO_RING_F_GUEST_BE))
+
 static inline bool arm_addr_in_ioport_region(u64 phys_addr)
 {
 	u64 limit = KVM_IOPORT_AREA + ARM_IOPORT_SIZE;
-- 
1.8.2.3

[PATCH 7/7] kvmtool: virtio: enable arm/arm64 support for bi-endianness

[Paolo Bonzini]

Il 11/10/2013 16:36, Marc Zyngier ha scritto:
> Cc: Pekka Enberg <penberg@kernel.org>
> Cc: Will Deacon <will.deacon@arm.com>
> Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
> ---
>  tools/kvm/arm/include/arm-common/kvm-arch.h | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/tools/kvm/arm/include/arm-common/kvm-arch.h b/tools/kvm/arm/include/arm-common/kvm-arch.h
> index 72b204f..91836cb 100644
> --- a/tools/kvm/arm/include/arm-common/kvm-arch.h
> +++ b/tools/kvm/arm/include/arm-common/kvm-arch.h
> @@ -27,6 +27,8 @@
>  
>  #define VIRTIO_DEFAULT_TRANS	VIRTIO_MMIO
>  
> +#define VIRTIO_RING_ENDIAN	((1UL << VIRTIO_RING_F_GUEST_LE) | (1UL << VIRTIO_RING_F_GUEST_BE))
> +
>  static inline bool arm_addr_in_ioport_region(u64 phys_addr)
>  {
>  	u64 limit = KVM_IOPORT_AREA + ARM_IOPORT_SIZE;
> 

Why only arm/arm64?

Paolo

[PATCH 7/7] kvmtool: virtio: enable arm/arm64 support for bi-endianness

[Marc Zyngier]

On 14/10/13 14:05, Paolo Bonzini wrote:
> Il 11/10/2013 16:36, Marc Zyngier ha scritto:
>> Cc: Pekka Enberg <penberg@kernel.org>
>> Cc: Will Deacon <will.deacon@arm.com>
>> Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
>> ---
>>  tools/kvm/arm/include/arm-common/kvm-arch.h | 2 ++
>>  1 file changed, 2 insertions(+)
>>
>> diff --git a/tools/kvm/arm/include/arm-common/kvm-arch.h b/tools/kvm/arm/include/arm-common/kvm-arch.h
>> index 72b204f..91836cb 100644
>> --- a/tools/kvm/arm/include/arm-common/kvm-arch.h
>> +++ b/tools/kvm/arm/include/arm-common/kvm-arch.h
>> @@ -27,6 +27,8 @@
>>  
>>  #define VIRTIO_DEFAULT_TRANS	VIRTIO_MMIO
>>  
>> +#define VIRTIO_RING_ENDIAN	((1UL << VIRTIO_RING_F_GUEST_LE) | (1UL << VIRTIO_RING_F_GUEST_BE))
>> +
>>  static inline bool arm_addr_in_ioport_region(u64 phys_addr)
>>  {
>>  	u64 limit = KVM_IOPORT_AREA + ARM_IOPORT_SIZE;
>>
> 
> Why only arm/arm64?

Because it has a runtime impact (testing the queue endianness on each
access is not free). Also, I have no idea how much of an interest other
arches have when it comes to mixed endianness.

	M.
-- 
Jazz is not dead. It just smells funny...

[PATCH 0/7] kvmtool: handle guests of a different endianness

[Will Deacon]

On Fri, Oct 11, 2013 at 03:36:28PM +0100, Marc Zyngier wrote:
> This patch series adds some infrastructure to kvmtool to allow a BE
> guest to use virtio-mmio on a LE host, provided that the architecture
> actually supports such madness.
> 
> Not all the backend have been converted, only those I actually cared
> about. Converting them is pretty easy though, and will be done if the
> method is deemed acceptable.
> 
> This has been tested on arm64, with some fixes to KVM and a set of
> changes to virtio, both which I am posting separately.
> 
> A branch containing all the relevant changes is at:
> git://git.kernel.org/pub/scm/linux/kernel/git/maz/arm-platforms.git kvm-arm64/be-on-le-3.12-rc4

Whilst this looks like the right sort of idea, I think the virtio spec needs
to be updated before we can implement this stuff in kvmtool. Have you warned
Rusty?

Will