All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/3] virtio-mmio: handle BE guests on LE hosts
@ 2013-10-11 14:36 ` Marc Zyngier
  0 siblings, 0 replies; 86+ messages in thread
From: Marc Zyngier @ 2013-10-11 14:36 UTC (permalink / raw)
  To: linux-arm-kernel, kvmarm, kvm
  Cc: Rusty Russell, Michael S. Tsirkin, Pawel Moll

This small patch series adds just enough kernel infrastructure and
fixes to allow a BE guest to use virtio-mmio on a LE host, provided
that the host actually supports such madness.

This has been tested on arm64, with some fixes to KVM and a set of
changes to kvmtool, 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: Rusty Russell <rusty@rustcorp.com.au>
Cc: Michael S. Tsirkin <mst@redhat.com>
Cc: Pawel Moll <pawel.moll@arm.com>

Marc Zyngier (3):
  virtio: let the guest report its endianess if advertized by the host
  virtio: mmio: fix signature checking for BE guests
  virtio: mmio: access configuration space as little-endian

 drivers/block/virtio_blk.c             |  6 +--
 drivers/char/virtio_console.c          |  4 +-
 drivers/lguest/lguest_device.c         |  8 +++-
 drivers/net/caif/caif_virtio.c         |  2 +-
 drivers/net/virtio_net.c               |  2 +-
 drivers/remoteproc/remoteproc_virtio.c |  8 +++-
 drivers/s390/kvm/kvm_virtio.c          |  8 +++-
 drivers/s390/kvm/virtio_ccw.c          |  9 +++-
 drivers/scsi/virtio_scsi.c             |  4 +-
 drivers/virtio/virtio_balloon.c        |  8 ++--
 drivers/virtio/virtio_mmio.c           | 88 ++++++++++++++++++++++++++++++----
 drivers/virtio/virtio_pci.c            |  8 +++-
 drivers/virtio/virtio_ring.c           |  8 ++++
 include/linux/virtio_config.h          | 19 ++++----
 include/uapi/linux/virtio_ring.h       |  8 ++++
 net/9p/trans_virtio.c                  |  4 +-
 16 files changed, 150 insertions(+), 44 deletions(-)

-- 
1.8.2.3



^ permalink raw reply	[flat|nested] 86+ messages in thread
* [PATCH 0/3] virtio-mmio: handle BE guests on LE hosts
@ 2013-10-11 14:36 ` Marc Zyngier
  0 siblings, 0 replies; 86+ messages in thread
From: Marc Zyngier @ 2013-10-11 14:36 UTC (permalink / raw)
  To: linux-arm-kernel

This small patch series adds just enough kernel infrastructure and
fixes to allow a BE guest to use virtio-mmio on a LE host, provided
that the host actually supports such madness.

This has been tested on arm64, with some fixes to KVM and a set of
changes to kvmtool, 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: Rusty Russell <rusty@rustcorp.com.au>
Cc: Michael S. Tsirkin <mst@redhat.com>
Cc: Pawel Moll <pawel.moll@arm.com>

Marc Zyngier (3):
  virtio: let the guest report its endianess if advertized by the host
  virtio: mmio: fix signature checking for BE guests
  virtio: mmio: access configuration space as little-endian

 drivers/block/virtio_blk.c             |  6 +--
 drivers/char/virtio_console.c          |  4 +-
 drivers/lguest/lguest_device.c         |  8 +++-
 drivers/net/caif/caif_virtio.c         |  2 +-
 drivers/net/virtio_net.c               |  2 +-
 drivers/remoteproc/remoteproc_virtio.c |  8 +++-
 drivers/s390/kvm/kvm_virtio.c          |  8 +++-
 drivers/s390/kvm/virtio_ccw.c          |  9 +++-
 drivers/scsi/virtio_scsi.c             |  4 +-
 drivers/virtio/virtio_balloon.c        |  8 ++--
 drivers/virtio/virtio_mmio.c           | 88 ++++++++++++++++++++++++++++++----
 drivers/virtio/virtio_pci.c            |  8 +++-
 drivers/virtio/virtio_ring.c           |  8 ++++
 include/linux/virtio_config.h          | 19 ++++----
 include/uapi/linux/virtio_ring.h       |  8 ++++
 net/9p/trans_virtio.c                  |  4 +-
 16 files changed, 150 insertions(+), 44 deletions(-)

-- 
1.8.2.3

^ permalink raw reply	[flat|nested] 86+ messages in thread
* [PATCH 1/3] virtio: let the guest report its endianess if advertized by the host
  2013-10-11 14:36 ` Marc Zyngier
@ 2013-10-11 14:36   ` Marc Zyngier
  -1 siblings, 0 replies; 86+ messages in thread
From: Marc Zyngier @ 2013-10-11 14:36 UTC (permalink / raw)
  To: linux-arm-kernel, kvmarm, kvm
  Cc: Rusty Russell, Michael S. Tsirkin, Pawel Moll

In order for a guest to report its endianess, we introduce a pair of
per-ring flags: VIRTIO_RING_F_GUEST_{LE,BE}.

A host is allowed to advertise support for either or both endianess
for this ring. If it advertises none, it is assumed to be able to
handle the guest (best effort).

A guest is allowed to select one of the endiannesses advertised by
the host (but obviously not both). If none is selected, it is assumed
to be of an endianness compatible with the host (best effort).

This mechanism allows the host to deal with guests of different
endiannesses.

Cc: Rusty Russell <rusty@rustcorp.com.au>
Cc: Michael S. Tsirkin <mst@redhat.com>
Cc: Pawel Moll <pawel.moll@arm.com>
Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
---
 drivers/virtio/virtio_ring.c     | 8 ++++++++
 include/uapi/linux/virtio_ring.h | 8 ++++++++
 2 files changed, 16 insertions(+)

diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c
index 6b4a4db..efff20a 100644
--- a/drivers/virtio/virtio_ring.c
+++ b/drivers/virtio/virtio_ring.c
@@ -813,6 +813,14 @@ void vring_transport_features(struct virtio_device *vdev)
 			break;
 		case VIRTIO_RING_F_EVENT_IDX:
 			break;
+#ifdef __LITTLE_ENDIAN
+		case VIRTIO_RING_F_GUEST_LE:
+			break;
+#endif
+#ifdef __BIG_ENDIAN
+		case VIRTIO_RING_F_GUEST_BE:
+			break;
+#endif
 		default:
 			/* We don't understand this bit. */
 			clear_bit(i, vdev->features);
diff --git a/include/uapi/linux/virtio_ring.h b/include/uapi/linux/virtio_ring.h
index a99f9b7..496b46a 100644
--- a/include/uapi/linux/virtio_ring.h
+++ b/include/uapi/linux/virtio_ring.h
@@ -58,6 +58,14 @@
  * at the end of the used ring. Guest should ignore the used->flags field. */
 #define VIRTIO_RING_F_EVENT_IDX		29
 
+/* The Host can advertise to support either or both endianness for the vring.
+ * If it advertise none, it is assumed to be able to handle the Guest
+ * endianness.
+ * The Guest can select either (but not both) endianness. If it selects none,
+ * it is assumed to be of an endianness compatible with the Host. */
+#define VIRTIO_RING_F_GUEST_LE		30
+#define VIRTIO_RING_F_GUEST_BE		31
+
 /* Virtio ring descriptors: 16 bytes.  These can chain together via "next". */
 struct vring_desc {
 	/* Address (guest-physical). */
-- 
1.8.2.3



^ permalink raw reply related	[flat|nested] 86+ messages in thread
* [PATCH 1/3] virtio: let the guest report its endianess if advertized by the host
@ 2013-10-11 14:36   ` Marc Zyngier
  0 siblings, 0 replies; 86+ messages in thread
From: Marc Zyngier @ 2013-10-11 14:36 UTC (permalink / raw)
  To: linux-arm-kernel

In order for a guest to report its endianess, we introduce a pair of
per-ring flags: VIRTIO_RING_F_GUEST_{LE,BE}.

A host is allowed to advertise support for either or both endianess
for this ring. If it advertises none, it is assumed to be able to
handle the guest (best effort).

A guest is allowed to select one of the endiannesses advertised by
the host (but obviously not both). If none is selected, it is assumed
to be of an endianness compatible with the host (best effort).

This mechanism allows the host to deal with guests of different
endiannesses.

Cc: Rusty Russell <rusty@rustcorp.com.au>
Cc: Michael S. Tsirkin <mst@redhat.com>
Cc: Pawel Moll <pawel.moll@arm.com>
Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
---
 drivers/virtio/virtio_ring.c     | 8 ++++++++
 include/uapi/linux/virtio_ring.h | 8 ++++++++
 2 files changed, 16 insertions(+)

diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c
index 6b4a4db..efff20a 100644
--- a/drivers/virtio/virtio_ring.c
+++ b/drivers/virtio/virtio_ring.c
@@ -813,6 +813,14 @@ void vring_transport_features(struct virtio_device *vdev)
 			break;
 		case VIRTIO_RING_F_EVENT_IDX:
 			break;
+#ifdef __LITTLE_ENDIAN
+		case VIRTIO_RING_F_GUEST_LE:
+			break;
+#endif
+#ifdef __BIG_ENDIAN
+		case VIRTIO_RING_F_GUEST_BE:
+			break;
+#endif
 		default:
 			/* We don't understand this bit. */
 			clear_bit(i, vdev->features);
diff --git a/include/uapi/linux/virtio_ring.h b/include/uapi/linux/virtio_ring.h
index a99f9b7..496b46a 100644
--- a/include/uapi/linux/virtio_ring.h
+++ b/include/uapi/linux/virtio_ring.h
@@ -58,6 +58,14 @@
  * at the end of the used ring. Guest should ignore the used->flags field. */
 #define VIRTIO_RING_F_EVENT_IDX		29
 
+/* The Host can advertise to support either or both endianness for the vring.
+ * If it advertise none, it is assumed to be able to handle the Guest
+ * endianness.
+ * The Guest can select either (but not both) endianness. If it selects none,
+ * it is assumed to be of an endianness compatible with the Host. */
+#define VIRTIO_RING_F_GUEST_LE		30
+#define VIRTIO_RING_F_GUEST_BE		31
+
 /* Virtio ring descriptors: 16 bytes.  These can chain together via "next". */
 struct vring_desc {
 	/* Address (guest-physical). */
-- 
1.8.2.3

^ permalink raw reply related	[flat|nested] 86+ messages in thread
* [PATCH 2/3] virtio: mmio: fix signature checking for BE guests
  2013-10-11 14:36 ` Marc Zyngier
@ 2013-10-11 14:36   ` Marc Zyngier
  -1 siblings, 0 replies; 86+ messages in thread
From: Marc Zyngier @ 2013-10-11 14:36 UTC (permalink / raw)
  To: linux-arm-kernel, kvmarm, kvm
  Cc: Rusty Russell, Michael S. Tsirkin, Pawel Moll

As virtio-mmio config registers are specified to be little-endian,
using readl() to read the magic value and then memcmp() to check it
fails on BE (as readl() has an implicit swab).

Fix it by encoding the magic value as an integer instead of a string.

Cc: Rusty Russell <rusty@rustcorp.com.au>
Cc: Michael S. Tsirkin <mst@redhat.com>
Cc: Pawel Moll <pawel.moll@arm.com>
Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
---
 drivers/virtio/virtio_mmio.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/virtio/virtio_mmio.c b/drivers/virtio/virtio_mmio.c
index 1ba0d68..57f24fd 100644
--- a/drivers/virtio/virtio_mmio.c
+++ b/drivers/virtio/virtio_mmio.c
@@ -470,7 +470,7 @@ static int virtio_mmio_probe(struct platform_device *pdev)
 
 	/* Check magic value */
 	magic = readl(vm_dev->base + VIRTIO_MMIO_MAGIC_VALUE);
-	if (memcmp(&magic, "virt", 4) != 0) {
+	if (magic != ('v' | 'i' << 8 | 'r' << 16 | 't' << 24)) {
 		dev_warn(&pdev->dev, "Wrong magic value 0x%08lx!\n", magic);
 		return -ENODEV;
 	}
-- 
1.8.2.3



^ permalink raw reply related	[flat|nested] 86+ messages in thread
* [PATCH 2/3] virtio: mmio: fix signature checking for BE guests
@ 2013-10-11 14:36   ` Marc Zyngier
  0 siblings, 0 replies; 86+ messages in thread
From: Marc Zyngier @ 2013-10-11 14:36 UTC (permalink / raw)
  To: linux-arm-kernel

As virtio-mmio config registers are specified to be little-endian,
using readl() to read the magic value and then memcmp() to check it
fails on BE (as readl() has an implicit swab).

Fix it by encoding the magic value as an integer instead of a string.

Cc: Rusty Russell <rusty@rustcorp.com.au>
Cc: Michael S. Tsirkin <mst@redhat.com>
Cc: Pawel Moll <pawel.moll@arm.com>
Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
---
 drivers/virtio/virtio_mmio.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/virtio/virtio_mmio.c b/drivers/virtio/virtio_mmio.c
index 1ba0d68..57f24fd 100644
--- a/drivers/virtio/virtio_mmio.c
+++ b/drivers/virtio/virtio_mmio.c
@@ -470,7 +470,7 @@ static int virtio_mmio_probe(struct platform_device *pdev)
 
 	/* Check magic value */
 	magic = readl(vm_dev->base + VIRTIO_MMIO_MAGIC_VALUE);
-	if (memcmp(&magic, "virt", 4) != 0) {
+	if (magic != ('v' | 'i' << 8 | 'r' << 16 | 't' << 24)) {
 		dev_warn(&pdev->dev, "Wrong magic value 0x%08lx!\n", magic);
 		return -ENODEV;
 	}
-- 
1.8.2.3

^ permalink raw reply related	[flat|nested] 86+ messages in thread
* [PATCH 3/3] virtio: mmio: access configuration space as little-endian
  2013-10-11 14:36 ` Marc Zyngier
@ 2013-10-11 14:36   ` Marc Zyngier
  -1 siblings, 0 replies; 86+ messages in thread
From: Marc Zyngier @ 2013-10-11 14:36 UTC (permalink / raw)
  To: linux-arm-kernel, kvmarm, kvm
  Cc: Rusty Russell, Michael S. Tsirkin, Pawel Moll

virtio_mmio defines the config space to be little-endian. This means
that a big-endian guest has to perform the access with byte-swapping
accessors.

The config space accessors are changed to take an "access_size"
parameter, allowing the low-level code to use the correct primitives.
Drivers and transports are updated to use the modified API. Only
virtio_mmio is actually changed to do something different.

Cc: Rusty Russell <rusty@rustcorp.com.au>
Cc: Michael S. Tsirkin <mst@redhat.com>
Cc: Pawel Moll <pawel.moll@arm.com>
Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
---
 drivers/block/virtio_blk.c             |  6 +--
 drivers/char/virtio_console.c          |  4 +-
 drivers/lguest/lguest_device.c         |  8 +++-
 drivers/net/caif/caif_virtio.c         |  2 +-
 drivers/net/virtio_net.c               |  2 +-
 drivers/remoteproc/remoteproc_virtio.c |  8 +++-
 drivers/s390/kvm/kvm_virtio.c          |  8 +++-
 drivers/s390/kvm/virtio_ccw.c          |  9 +++-
 drivers/scsi/virtio_scsi.c             |  4 +-
 drivers/virtio/virtio_balloon.c        |  8 ++--
 drivers/virtio/virtio_mmio.c           | 86 ++++++++++++++++++++++++++++++----
 drivers/virtio/virtio_pci.c            |  8 +++-
 include/linux/virtio_config.h          | 19 ++++----
 net/9p/trans_virtio.c                  |  4 +-
 14 files changed, 133 insertions(+), 43 deletions(-)

diff --git a/drivers/block/virtio_blk.c b/drivers/block/virtio_blk.c
index 5cdf88b..9eadf52 100644
--- a/drivers/block/virtio_blk.c
+++ b/drivers/block/virtio_blk.c
@@ -530,7 +530,7 @@ static void virtblk_config_changed_work(struct work_struct *work)
 
 	/* Host must always specify the capacity. */
 	vdev->config->get(vdev, offsetof(struct virtio_blk_config, capacity),
-			  &capacity, sizeof(capacity));
+			  &capacity, 1, sizeof(capacity));
 
 	/* If capacity is too big, truncate with warning. */
 	if ((sector_t)capacity != capacity) {
@@ -655,7 +655,7 @@ virtblk_cache_type_store(struct device *dev, struct device_attribute *attr,
 	writeback = i;
 	vdev->config->set(vdev,
 			  offsetof(struct virtio_blk_config, wce),
-			  &writeback, sizeof(writeback));
+			  &writeback, 1, sizeof(writeback));
 
 	virtblk_update_cache_mode(vdev);
 	return count;
@@ -773,7 +773,7 @@ static int virtblk_probe(struct virtio_device *vdev)
 
 	/* Host must always specify the capacity. */
 	vdev->config->get(vdev, offsetof(struct virtio_blk_config, capacity),
-			  &cap, sizeof(cap));
+			  &cap, 1, sizeof(cap));
 
 	/* If capacity is too big, truncate with warning. */
 	if ((sector_t)cap != cap) {
diff --git a/drivers/char/virtio_console.c b/drivers/char/virtio_console.c
index b79cf3e..5ca3eb1 100644
--- a/drivers/char/virtio_console.c
+++ b/drivers/char/virtio_console.c
@@ -1839,10 +1839,10 @@ static void config_intr(struct virtio_device *vdev)
 
 		vdev->config->get(vdev,
 				  offsetof(struct virtio_console_config, cols),
-				  &cols, sizeof(u16));
+				  &cols, 1, sizeof(u16));
 		vdev->config->get(vdev,
 				  offsetof(struct virtio_console_config, rows),
-				  &rows, sizeof(u16));
+				  &rows, 1, sizeof(u16));
 
 		port = find_port_by_id(portdev, 0);
 		set_console_size(port, rows, cols);
diff --git a/drivers/lguest/lguest_device.c b/drivers/lguest/lguest_device.c
index b3256ff..5858a9b 100644
--- a/drivers/lguest/lguest_device.c
+++ b/drivers/lguest/lguest_device.c
@@ -154,10 +154,12 @@ static void lg_finalize_features(struct virtio_device *vdev)
 
 /* Once they've found a field, getting a copy of it is easy. */
 static void lg_get(struct virtio_device *vdev, unsigned int offset,
-		   void *buf, unsigned len)
+		   void *buf, unsigned len, unsigned access_size)
 {
 	struct lguest_device_desc *desc = to_lgdev(vdev)->desc;
 
+	len *= access_size;
+
 	/* Check they didn't ask for more than the length of the config! */
 	BUG_ON(offset + len > desc->config_len);
 	memcpy(buf, lg_config(desc) + offset, len);
@@ -165,10 +167,12 @@ static void lg_get(struct virtio_device *vdev, unsigned int offset,
 
 /* Setting the contents is also trivial. */
 static void lg_set(struct virtio_device *vdev, unsigned int offset,
-		   const void *buf, unsigned len)
+		   const void *buf, unsigned len, unsigned access_size)
 {
 	struct lguest_device_desc *desc = to_lgdev(vdev)->desc;
 
+	len *= access_size;
+
 	/* Check they didn't ask for more than the length of the config! */
 	BUG_ON(offset + len > desc->config_len);
 	memcpy(lg_config(desc) + offset, buf, len);
diff --git a/drivers/net/caif/caif_virtio.c b/drivers/net/caif/caif_virtio.c
index b9ed128..5ace13d 100644
--- a/drivers/net/caif/caif_virtio.c
+++ b/drivers/net/caif/caif_virtio.c
@@ -689,7 +689,7 @@ static int cfv_probe(struct virtio_device *vdev)
 #define GET_VIRTIO_CONFIG_OPS(_v, _var, _f) \
 	((_v)->config->get(_v, offsetof(struct virtio_caif_transf_config, _f), \
 			   &_var, \
-			   FIELD_SIZEOF(struct virtio_caif_transf_config, _f)))
+			   1, FIELD_SIZEOF(struct virtio_caif_transf_config, _f)))
 
 	if (vdev->config->get) {
 		GET_VIRTIO_CONFIG_OPS(vdev, cfv->tx_hr, headroom);
diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index defec2b..54e9a22 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -853,7 +853,7 @@ static int virtnet_set_mac_address(struct net_device *dev, void *p)
 		}
 	} else if (virtio_has_feature(vdev, VIRTIO_NET_F_MAC)) {
 		vdev->config->set(vdev, offsetof(struct virtio_net_config, mac),
-				  addr->sa_data, dev->addr_len);
+				  addr->sa_data, dev->addr_len, 1);
 	}
 
 	eth_commit_mac_addr_change(dev, p);
diff --git a/drivers/remoteproc/remoteproc_virtio.c b/drivers/remoteproc/remoteproc_virtio.c
index b09c75c..40af32f 100644
--- a/drivers/remoteproc/remoteproc_virtio.c
+++ b/drivers/remoteproc/remoteproc_virtio.c
@@ -234,12 +234,14 @@ static void rproc_virtio_finalize_features(struct virtio_device *vdev)
 }
 
 static void rproc_virtio_get(struct virtio_device *vdev, unsigned offset,
-							void *buf, unsigned len)
+			     void *buf, unsigned len, unsigned access_size)
 {
 	struct rproc_vdev *rvdev = vdev_to_rvdev(vdev);
 	struct fw_rsc_vdev *rsc;
 	void *cfg;
 
+	len *= access_size;
+
 	rsc = (void *)rvdev->rproc->table_ptr + rvdev->rsc_offset;
 	cfg = &rsc->vring[rsc->num_of_vrings];
 
@@ -252,12 +254,14 @@ static void rproc_virtio_get(struct virtio_device *vdev, unsigned offset,
 }
 
 static void rproc_virtio_set(struct virtio_device *vdev, unsigned offset,
-		      const void *buf, unsigned len)
+			     const void *buf, unsigned len, unsigned access_size)
 {
 	struct rproc_vdev *rvdev = vdev_to_rvdev(vdev);
 	struct fw_rsc_vdev *rsc;
 	void *cfg;
 
+	len *= access_size;
+
 	rsc = (void *)rvdev->rproc->table_ptr + rvdev->rsc_offset;
 	cfg = &rsc->vring[rsc->num_of_vrings];
 
diff --git a/drivers/s390/kvm/kvm_virtio.c b/drivers/s390/kvm/kvm_virtio.c
index af2166f..119b106 100644
--- a/drivers/s390/kvm/kvm_virtio.c
+++ b/drivers/s390/kvm/kvm_virtio.c
@@ -115,19 +115,23 @@ static void kvm_finalize_features(struct virtio_device *vdev)
  * Reading and writing elements in config space
  */
 static void kvm_get(struct virtio_device *vdev, unsigned int offset,
-		   void *buf, unsigned len)
+		    void *buf, unsigned len, unsigned access_size)
 {
 	struct kvm_device_desc *desc = to_kvmdev(vdev)->desc;
 
+	len *= access_size;
+
 	BUG_ON(offset + len > desc->config_len);
 	memcpy(buf, kvm_vq_configspace(desc) + offset, len);
 }
 
 static void kvm_set(struct virtio_device *vdev, unsigned int offset,
-		   const void *buf, unsigned len)
+		    const void *buf, unsigned len, unsigned access_size)
 {
 	struct kvm_device_desc *desc = to_kvmdev(vdev)->desc;
 
+	len *= access_size;
+
 	BUG_ON(offset + len > desc->config_len);
 	memcpy(kvm_vq_configspace(desc) + offset, buf, len);
 }
diff --git a/drivers/s390/kvm/virtio_ccw.c b/drivers/s390/kvm/virtio_ccw.c
index 779dc51..2c36788 100644
--- a/drivers/s390/kvm/virtio_ccw.c
+++ b/drivers/s390/kvm/virtio_ccw.c
@@ -477,13 +477,16 @@ out_free:
 }
 
 static void virtio_ccw_get_config(struct virtio_device *vdev,
-				  unsigned int offset, void *buf, unsigned len)
+				  unsigned int offset, void *buf,
+				  unsigned len, unsigned access_size)
 {
 	struct virtio_ccw_device *vcdev = to_vc_device(vdev);
 	int ret;
 	struct ccw1 *ccw;
 	void *config_area;
 
+	len *= access_size;
+
 	ccw = kzalloc(sizeof(*ccw), GFP_DMA | GFP_KERNEL);
 	if (!ccw)
 		return;
@@ -511,12 +514,14 @@ out_free:
 
 static void virtio_ccw_set_config(struct virtio_device *vdev,
 				  unsigned int offset, const void *buf,
-				  unsigned len)
+				  unsigned len, unsigned access_size)
 {
 	struct virtio_ccw_device *vcdev = to_vc_device(vdev);
 	struct ccw1 *ccw;
 	void *config_area;
 
+	len *= access_size;
+
 	ccw = kzalloc(sizeof(*ccw), GFP_DMA | GFP_KERNEL);
 	if (!ccw)
 		return;
diff --git a/drivers/scsi/virtio_scsi.c b/drivers/scsi/virtio_scsi.c
index 74b88ef..652d64e 100644
--- a/drivers/scsi/virtio_scsi.c
+++ b/drivers/scsi/virtio_scsi.c
@@ -712,7 +712,7 @@ static struct scsi_host_template virtscsi_host_template_multi = {
 		typeof(((struct virtio_scsi_config *)0)->fld) __val; \
 		vdev->config->get(vdev, \
 				  offsetof(struct virtio_scsi_config, fld), \
-				  &__val, sizeof(__val)); \
+				  &__val, 1, sizeof(__val));		\
 		__val; \
 	})
 
@@ -721,7 +721,7 @@ static struct scsi_host_template virtscsi_host_template_multi = {
 		typeof(((struct virtio_scsi_config *)0)->fld) __val = (val); \
 		vdev->config->set(vdev, \
 				  offsetof(struct virtio_scsi_config, fld), \
-				  &__val, sizeof(__val)); \
+				  &__val, 1, sizeof(__val));		\
 	})
 
 static void __virtscsi_set_affinity(struct virtio_scsi *vscsi, bool affinity)
diff --git a/drivers/virtio/virtio_balloon.c b/drivers/virtio/virtio_balloon.c
index 1f572c0..ccac818 100644
--- a/drivers/virtio/virtio_balloon.c
+++ b/drivers/virtio/virtio_balloon.c
@@ -272,23 +272,21 @@ static void virtballoon_changed(struct virtio_device *vdev)
 
 static inline s64 towards_target(struct virtio_balloon *vb)
 {
-	__le32 v;
 	s64 target;
 
 	vb->vdev->config->get(vb->vdev,
 			      offsetof(struct virtio_balloon_config, num_pages),
-			      &v, sizeof(v));
-	target = le32_to_cpu(v);
+			      &target, 1, sizeof(target));
 	return target - vb->num_pages;
 }
 
 static void update_balloon_size(struct virtio_balloon *vb)
 {
-	__le32 actual = cpu_to_le32(vb->num_pages);
+	u32 actual = vb->num_pages;
 
 	vb->vdev->config->set(vb->vdev,
 			      offsetof(struct virtio_balloon_config, actual),
-			      &actual, sizeof(actual));
+			      &actual, 1, sizeof(actual));
 }
 
 static int balloon(void *_vballoon)
diff --git a/drivers/virtio/virtio_mmio.c b/drivers/virtio/virtio_mmio.c
index 57f24fd..bbc4410 100644
--- a/drivers/virtio/virtio_mmio.c
+++ b/drivers/virtio/virtio_mmio.c
@@ -101,7 +101,7 @@
 #include <linux/virtio_mmio.h>
 #include <linux/virtio_ring.h>
 
-
+#include <asm-generic/io-64-nonatomic-lo-hi.h>
 
 /* The alignment to use between consumer and producer parts of vring.
  * Currently hardcoded to the page size. */
@@ -168,25 +168,93 @@ static void vm_finalize_features(struct virtio_device *vdev)
 }
 
 static void vm_get(struct virtio_device *vdev, unsigned offset,
-		   void *buf, unsigned len)
+		   void *buf, unsigned len, unsigned access_size)
 {
 	struct virtio_mmio_device *vm_dev = to_virtio_mmio_device(vdev);
-	u8 *ptr = buf;
 	int i;
 
-	for (i = 0; i < len; i++)
-		ptr[i] = readb(vm_dev->base + VIRTIO_MMIO_CONFIG + offset + i);
+	switch (access_size) {
+	case 1: {
+		u8 *ptr = buf;
+
+		for (i = 0; i < len; i++)
+			ptr[i] = readb(vm_dev->base + VIRTIO_MMIO_CONFIG + offset + i);
+
+		break;
+	}
+	case 2: {
+		u16 *ptr = buf;
+
+		for (i = 0; i < len; i++)
+			ptr[i] = readw(vm_dev->base + VIRTIO_MMIO_CONFIG + offset + i);
+
+		break;
+	}
+	case 4: {
+		u32 *ptr = buf;
+
+		for (i = 0; i < len; i++)
+			ptr[i] = readl(vm_dev->base + VIRTIO_MMIO_CONFIG + offset + i);
+
+		break;
+	}
+	case 8: {
+		u64 *ptr = buf;
+
+		for (i = 0; i < len; i++)
+			ptr[i] = readq(vm_dev->base + VIRTIO_MMIO_CONFIG + offset + i);
+
+		break;
+	}
+	default:
+		pr_err("virtio: illegal access size %d\n", access_size);
+		BUG();
+	}
 }
 
 static void vm_set(struct virtio_device *vdev, unsigned offset,
-		   const void *buf, unsigned len)
+		   const void *buf, unsigned len, unsigned access_size)
 {
 	struct virtio_mmio_device *vm_dev = to_virtio_mmio_device(vdev);
-	const u8 *ptr = buf;
 	int i;
 
-	for (i = 0; i < len; i++)
-		writeb(ptr[i], vm_dev->base + VIRTIO_MMIO_CONFIG + offset + i);
+	switch (access_size) {
+	case 1: {
+		const u8 *ptr = buf;
+
+		for (i = 0; i < len; i++)
+			writeb(ptr[i], vm_dev->base + VIRTIO_MMIO_CONFIG + offset + i);
+
+		break;
+	}
+	case 2: {
+		const u16 *ptr = buf;
+
+		for (i = 0; i < len; i++)
+			writew(ptr[i], vm_dev->base + VIRTIO_MMIO_CONFIG + offset + i);
+
+		break;
+	}
+	case 4: {
+		const u32 *ptr = buf;
+
+		for (i = 0; i < len; i++)
+			writel(ptr[i], vm_dev->base + VIRTIO_MMIO_CONFIG + offset + i);
+
+		break;
+	}
+	case 8: {
+		const u64 *ptr = buf;
+
+		for (i = 0; i < len; i++)
+			writeq(ptr[i], vm_dev->base + VIRTIO_MMIO_CONFIG + offset + i);
+
+		break;
+	}
+	default:
+		pr_err("virtio: illegal access size %d\n", access_size);
+		BUG();
+	}
 }
 
 static u8 vm_get_status(struct virtio_device *vdev)
diff --git a/drivers/virtio/virtio_pci.c b/drivers/virtio/virtio_pci.c
index 98917fc..f06671c 100644
--- a/drivers/virtio/virtio_pci.c
+++ b/drivers/virtio/virtio_pci.c
@@ -129,7 +129,7 @@ static void vp_finalize_features(struct virtio_device *vdev)
 
 /* virtio config->get() implementation */
 static void vp_get(struct virtio_device *vdev, unsigned offset,
-		   void *buf, unsigned len)
+		   void *buf, unsigned len, unsigned access_size)
 {
 	struct virtio_pci_device *vp_dev = to_vp_device(vdev);
 	void __iomem *ioaddr = vp_dev->ioaddr +
@@ -137,6 +137,8 @@ static void vp_get(struct virtio_device *vdev, unsigned offset,
 	u8 *ptr = buf;
 	int i;
 
+	len *= access_size;
+
 	for (i = 0; i < len; i++)
 		ptr[i] = ioread8(ioaddr + i);
 }
@@ -144,7 +146,7 @@ static void vp_get(struct virtio_device *vdev, unsigned offset,
 /* the config->set() implementation.  it's symmetric to the config->get()
  * implementation */
 static void vp_set(struct virtio_device *vdev, unsigned offset,
-		   const void *buf, unsigned len)
+		   const void *buf, unsigned len, unsigned access_size)
 {
 	struct virtio_pci_device *vp_dev = to_vp_device(vdev);
 	void __iomem *ioaddr = vp_dev->ioaddr +
@@ -152,6 +154,8 @@ static void vp_set(struct virtio_device *vdev, unsigned offset,
 	const u8 *ptr = buf;
 	int i;
 
+	len *= access_size;
+
 	for (i = 0; i < len; i++)
 		iowrite8(ptr[i], ioaddr + i);
 }
diff --git a/include/linux/virtio_config.h b/include/linux/virtio_config.h
index 29b9104..69d1884 100644
--- a/include/linux/virtio_config.h
+++ b/include/linux/virtio_config.h
@@ -12,12 +12,14 @@
  *	vdev: the virtio_device
  *	offset: the offset of the configuration field
  *	buf: the buffer to write the field value into.
- *	len: the length of the buffer
+ *	len: the length of the buffer in access_size unit
+ *	access_size: access length
  * @set: write the value of a configuration field
  *	vdev: the virtio_device
  *	offset: the offset of the configuration field
  *	buf: the buffer to read the field value from.
- *	len: the length of the buffer
+ *	len: the length of the buffer in access_size unit
+ *	access_size: access length
  * @get_status: read the status byte
  *	vdev: the virtio_device
  *	Returns the status byte
@@ -55,9 +57,9 @@
 typedef void vq_callback_t(struct virtqueue *);
 struct virtio_config_ops {
 	void (*get)(struct virtio_device *vdev, unsigned offset,
-		    void *buf, unsigned len);
+		    void *buf, unsigned len, unsigned access_size);
 	void (*set)(struct virtio_device *vdev, unsigned offset,
-		    const void *buf, unsigned len);
+		    const void *buf, unsigned len, unsigned access_size);
 	u8 (*get_status)(struct virtio_device *vdev);
 	void (*set_status)(struct virtio_device *vdev, u8 status);
 	void (*reset)(struct virtio_device *vdev);
@@ -106,20 +108,21 @@ static inline bool virtio_has_feature(const struct virtio_device *vdev,
  * The return value is -ENOENT if the feature doesn't exist.  Otherwise
  * the config value is copied into whatever is pointed to by v. */
 #define virtio_config_val(vdev, fbit, offset, v) \
-	virtio_config_buf((vdev), (fbit), (offset), (v), sizeof(*v))
+	virtio_config_buf((vdev), (fbit), (offset), (v), 1, sizeof(*v))
 
 #define virtio_config_val_len(vdev, fbit, offset, v, len) \
-	virtio_config_buf((vdev), (fbit), (offset), (v), (len))
+	virtio_config_buf((vdev), (fbit), (offset), (v), (len), 1)
 
 static inline int virtio_config_buf(struct virtio_device *vdev,
 				    unsigned int fbit,
 				    unsigned int offset,
-				    void *buf, unsigned len)
+				    void *buf, unsigned len,
+				    unsigned access_size)
 {
 	if (!virtio_has_feature(vdev, fbit))
 		return -ENOENT;
 
-	vdev->config->get(vdev, offset, buf, len);
+	vdev->config->get(vdev, offset, buf, len, access_size);
 	return 0;
 }
 
diff --git a/net/9p/trans_virtio.c b/net/9p/trans_virtio.c
index 990afab..d12a2aa 100644
--- a/net/9p/trans_virtio.c
+++ b/net/9p/trans_virtio.c
@@ -546,7 +546,7 @@ static int p9_virtio_probe(struct virtio_device *vdev)
 	if (virtio_has_feature(vdev, VIRTIO_9P_MOUNT_TAG)) {
 		vdev->config->get(vdev,
 				offsetof(struct virtio_9p_config, tag_len),
-				&tag_len, sizeof(tag_len));
+				  &tag_len, 1, sizeof(tag_len));
 	} else {
 		err = -EINVAL;
 		goto out_free_vq;
@@ -557,7 +557,7 @@ static int p9_virtio_probe(struct virtio_device *vdev)
 		goto out_free_vq;
 	}
 	vdev->config->get(vdev, offsetof(struct virtio_9p_config, tag),
-			tag, tag_len);
+			  tag, tag_len, 1);
 	chan->tag = tag;
 	chan->tag_len = tag_len;
 	err = sysfs_create_file(&(vdev->dev.kobj), &dev_attr_mount_tag.attr);
-- 
1.8.2.3



^ permalink raw reply related	[flat|nested] 86+ messages in thread
* [PATCH 3/3] virtio: mmio: access configuration space as little-endian
@ 2013-10-11 14:36   ` Marc Zyngier
  0 siblings, 0 replies; 86+ messages in thread
From: Marc Zyngier @ 2013-10-11 14:36 UTC (permalink / raw)
  To: linux-arm-kernel

virtio_mmio defines the config space to be little-endian. This means
that a big-endian guest has to perform the access with byte-swapping
accessors.

The config space accessors are changed to take an "access_size"
parameter, allowing the low-level code to use the correct primitives.
Drivers and transports are updated to use the modified API. Only
virtio_mmio is actually changed to do something different.

Cc: Rusty Russell <rusty@rustcorp.com.au>
Cc: Michael S. Tsirkin <mst@redhat.com>
Cc: Pawel Moll <pawel.moll@arm.com>
Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
---
 drivers/block/virtio_blk.c             |  6 +--
 drivers/char/virtio_console.c          |  4 +-
 drivers/lguest/lguest_device.c         |  8 +++-
 drivers/net/caif/caif_virtio.c         |  2 +-
 drivers/net/virtio_net.c               |  2 +-
 drivers/remoteproc/remoteproc_virtio.c |  8 +++-
 drivers/s390/kvm/kvm_virtio.c          |  8 +++-
 drivers/s390/kvm/virtio_ccw.c          |  9 +++-
 drivers/scsi/virtio_scsi.c             |  4 +-
 drivers/virtio/virtio_balloon.c        |  8 ++--
 drivers/virtio/virtio_mmio.c           | 86 ++++++++++++++++++++++++++++++----
 drivers/virtio/virtio_pci.c            |  8 +++-
 include/linux/virtio_config.h          | 19 ++++----
 net/9p/trans_virtio.c                  |  4 +-
 14 files changed, 133 insertions(+), 43 deletions(-)

diff --git a/drivers/block/virtio_blk.c b/drivers/block/virtio_blk.c
index 5cdf88b..9eadf52 100644
--- a/drivers/block/virtio_blk.c
+++ b/drivers/block/virtio_blk.c
@@ -530,7 +530,7 @@ static void virtblk_config_changed_work(struct work_struct *work)
 
 	/* Host must always specify the capacity. */
 	vdev->config->get(vdev, offsetof(struct virtio_blk_config, capacity),
-			  &capacity, sizeof(capacity));
+			  &capacity, 1, sizeof(capacity));
 
 	/* If capacity is too big, truncate with warning. */
 	if ((sector_t)capacity != capacity) {
@@ -655,7 +655,7 @@ virtblk_cache_type_store(struct device *dev, struct device_attribute *attr,
 	writeback = i;
 	vdev->config->set(vdev,
 			  offsetof(struct virtio_blk_config, wce),
-			  &writeback, sizeof(writeback));
+			  &writeback, 1, sizeof(writeback));
 
 	virtblk_update_cache_mode(vdev);
 	return count;
@@ -773,7 +773,7 @@ static int virtblk_probe(struct virtio_device *vdev)
 
 	/* Host must always specify the capacity. */
 	vdev->config->get(vdev, offsetof(struct virtio_blk_config, capacity),
-			  &cap, sizeof(cap));
+			  &cap, 1, sizeof(cap));
 
 	/* If capacity is too big, truncate with warning. */
 	if ((sector_t)cap != cap) {
diff --git a/drivers/char/virtio_console.c b/drivers/char/virtio_console.c
index b79cf3e..5ca3eb1 100644
--- a/drivers/char/virtio_console.c
+++ b/drivers/char/virtio_console.c
@@ -1839,10 +1839,10 @@ static void config_intr(struct virtio_device *vdev)
 
 		vdev->config->get(vdev,
 				  offsetof(struct virtio_console_config, cols),
-				  &cols, sizeof(u16));
+				  &cols, 1, sizeof(u16));
 		vdev->config->get(vdev,
 				  offsetof(struct virtio_console_config, rows),
-				  &rows, sizeof(u16));
+				  &rows, 1, sizeof(u16));
 
 		port = find_port_by_id(portdev, 0);
 		set_console_size(port, rows, cols);
diff --git a/drivers/lguest/lguest_device.c b/drivers/lguest/lguest_device.c
index b3256ff..5858a9b 100644
--- a/drivers/lguest/lguest_device.c
+++ b/drivers/lguest/lguest_device.c
@@ -154,10 +154,12 @@ static void lg_finalize_features(struct virtio_device *vdev)
 
 /* Once they've found a field, getting a copy of it is easy. */
 static void lg_get(struct virtio_device *vdev, unsigned int offset,
-		   void *buf, unsigned len)
+		   void *buf, unsigned len, unsigned access_size)
 {
 	struct lguest_device_desc *desc = to_lgdev(vdev)->desc;
 
+	len *= access_size;
+
 	/* Check they didn't ask for more than the length of the config! */
 	BUG_ON(offset + len > desc->config_len);
 	memcpy(buf, lg_config(desc) + offset, len);
@@ -165,10 +167,12 @@ static void lg_get(struct virtio_device *vdev, unsigned int offset,
 
 /* Setting the contents is also trivial. */
 static void lg_set(struct virtio_device *vdev, unsigned int offset,
-		   const void *buf, unsigned len)
+		   const void *buf, unsigned len, unsigned access_size)
 {
 	struct lguest_device_desc *desc = to_lgdev(vdev)->desc;
 
+	len *= access_size;
+
 	/* Check they didn't ask for more than the length of the config! */
 	BUG_ON(offset + len > desc->config_len);
 	memcpy(lg_config(desc) + offset, buf, len);
diff --git a/drivers/net/caif/caif_virtio.c b/drivers/net/caif/caif_virtio.c
index b9ed128..5ace13d 100644
--- a/drivers/net/caif/caif_virtio.c
+++ b/drivers/net/caif/caif_virtio.c
@@ -689,7 +689,7 @@ static int cfv_probe(struct virtio_device *vdev)
 #define GET_VIRTIO_CONFIG_OPS(_v, _var, _f) \
 	((_v)->config->get(_v, offsetof(struct virtio_caif_transf_config, _f), \
 			   &_var, \
-			   FIELD_SIZEOF(struct virtio_caif_transf_config, _f)))
+			   1, FIELD_SIZEOF(struct virtio_caif_transf_config, _f)))
 
 	if (vdev->config->get) {
 		GET_VIRTIO_CONFIG_OPS(vdev, cfv->tx_hr, headroom);
diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index defec2b..54e9a22 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -853,7 +853,7 @@ static int virtnet_set_mac_address(struct net_device *dev, void *p)
 		}
 	} else if (virtio_has_feature(vdev, VIRTIO_NET_F_MAC)) {
 		vdev->config->set(vdev, offsetof(struct virtio_net_config, mac),
-				  addr->sa_data, dev->addr_len);
+				  addr->sa_data, dev->addr_len, 1);
 	}
 
 	eth_commit_mac_addr_change(dev, p);
diff --git a/drivers/remoteproc/remoteproc_virtio.c b/drivers/remoteproc/remoteproc_virtio.c
index b09c75c..40af32f 100644
--- a/drivers/remoteproc/remoteproc_virtio.c
+++ b/drivers/remoteproc/remoteproc_virtio.c
@@ -234,12 +234,14 @@ static void rproc_virtio_finalize_features(struct virtio_device *vdev)
 }
 
 static void rproc_virtio_get(struct virtio_device *vdev, unsigned offset,
-							void *buf, unsigned len)
+			     void *buf, unsigned len, unsigned access_size)
 {
 	struct rproc_vdev *rvdev = vdev_to_rvdev(vdev);
 	struct fw_rsc_vdev *rsc;
 	void *cfg;
 
+	len *= access_size;
+
 	rsc = (void *)rvdev->rproc->table_ptr + rvdev->rsc_offset;
 	cfg = &rsc->vring[rsc->num_of_vrings];
 
@@ -252,12 +254,14 @@ static void rproc_virtio_get(struct virtio_device *vdev, unsigned offset,
 }
 
 static void rproc_virtio_set(struct virtio_device *vdev, unsigned offset,
-		      const void *buf, unsigned len)
+			     const void *buf, unsigned len, unsigned access_size)
 {
 	struct rproc_vdev *rvdev = vdev_to_rvdev(vdev);
 	struct fw_rsc_vdev *rsc;
 	void *cfg;
 
+	len *= access_size;
+
 	rsc = (void *)rvdev->rproc->table_ptr + rvdev->rsc_offset;
 	cfg = &rsc->vring[rsc->num_of_vrings];
 
diff --git a/drivers/s390/kvm/kvm_virtio.c b/drivers/s390/kvm/kvm_virtio.c
index af2166f..119b106 100644
--- a/drivers/s390/kvm/kvm_virtio.c
+++ b/drivers/s390/kvm/kvm_virtio.c
@@ -115,19 +115,23 @@ static void kvm_finalize_features(struct virtio_device *vdev)
  * Reading and writing elements in config space
  */
 static void kvm_get(struct virtio_device *vdev, unsigned int offset,
-		   void *buf, unsigned len)
+		    void *buf, unsigned len, unsigned access_size)
 {
 	struct kvm_device_desc *desc = to_kvmdev(vdev)->desc;
 
+	len *= access_size;
+
 	BUG_ON(offset + len > desc->config_len);
 	memcpy(buf, kvm_vq_configspace(desc) + offset, len);
 }
 
 static void kvm_set(struct virtio_device *vdev, unsigned int offset,
-		   const void *buf, unsigned len)
+		    const void *buf, unsigned len, unsigned access_size)
 {
 	struct kvm_device_desc *desc = to_kvmdev(vdev)->desc;
 
+	len *= access_size;
+
 	BUG_ON(offset + len > desc->config_len);
 	memcpy(kvm_vq_configspace(desc) + offset, buf, len);
 }
diff --git a/drivers/s390/kvm/virtio_ccw.c b/drivers/s390/kvm/virtio_ccw.c
index 779dc51..2c36788 100644
--- a/drivers/s390/kvm/virtio_ccw.c
+++ b/drivers/s390/kvm/virtio_ccw.c
@@ -477,13 +477,16 @@ out_free:
 }
 
 static void virtio_ccw_get_config(struct virtio_device *vdev,
-				  unsigned int offset, void *buf, unsigned len)
+				  unsigned int offset, void *buf,
+				  unsigned len, unsigned access_size)
 {
 	struct virtio_ccw_device *vcdev = to_vc_device(vdev);
 	int ret;
 	struct ccw1 *ccw;
 	void *config_area;
 
+	len *= access_size;
+
 	ccw = kzalloc(sizeof(*ccw), GFP_DMA | GFP_KERNEL);
 	if (!ccw)
 		return;
@@ -511,12 +514,14 @@ out_free:
 
 static void virtio_ccw_set_config(struct virtio_device *vdev,
 				  unsigned int offset, const void *buf,
-				  unsigned len)
+				  unsigned len, unsigned access_size)
 {
 	struct virtio_ccw_device *vcdev = to_vc_device(vdev);
 	struct ccw1 *ccw;
 	void *config_area;
 
+	len *= access_size;
+
 	ccw = kzalloc(sizeof(*ccw), GFP_DMA | GFP_KERNEL);
 	if (!ccw)
 		return;
diff --git a/drivers/scsi/virtio_scsi.c b/drivers/scsi/virtio_scsi.c
index 74b88ef..652d64e 100644
--- a/drivers/scsi/virtio_scsi.c
+++ b/drivers/scsi/virtio_scsi.c
@@ -712,7 +712,7 @@ static struct scsi_host_template virtscsi_host_template_multi = {
 		typeof(((struct virtio_scsi_config *)0)->fld) __val; \
 		vdev->config->get(vdev, \
 				  offsetof(struct virtio_scsi_config, fld), \
-				  &__val, sizeof(__val)); \
+				  &__val, 1, sizeof(__val));		\
 		__val; \
 	})
 
@@ -721,7 +721,7 @@ static struct scsi_host_template virtscsi_host_template_multi = {
 		typeof(((struct virtio_scsi_config *)0)->fld) __val = (val); \
 		vdev->config->set(vdev, \
 				  offsetof(struct virtio_scsi_config, fld), \
-				  &__val, sizeof(__val)); \
+				  &__val, 1, sizeof(__val));		\
 	})
 
 static void __virtscsi_set_affinity(struct virtio_scsi *vscsi, bool affinity)
diff --git a/drivers/virtio/virtio_balloon.c b/drivers/virtio/virtio_balloon.c
index 1f572c0..ccac818 100644
--- a/drivers/virtio/virtio_balloon.c
+++ b/drivers/virtio/virtio_balloon.c
@@ -272,23 +272,21 @@ static void virtballoon_changed(struct virtio_device *vdev)
 
 static inline s64 towards_target(struct virtio_balloon *vb)
 {
-	__le32 v;
 	s64 target;
 
 	vb->vdev->config->get(vb->vdev,
 			      offsetof(struct virtio_balloon_config, num_pages),
-			      &v, sizeof(v));
-	target = le32_to_cpu(v);
+			      &target, 1, sizeof(target));
 	return target - vb->num_pages;
 }
 
 static void update_balloon_size(struct virtio_balloon *vb)
 {
-	__le32 actual = cpu_to_le32(vb->num_pages);
+	u32 actual = vb->num_pages;
 
 	vb->vdev->config->set(vb->vdev,
 			      offsetof(struct virtio_balloon_config, actual),
-			      &actual, sizeof(actual));
+			      &actual, 1, sizeof(actual));
 }
 
 static int balloon(void *_vballoon)
diff --git a/drivers/virtio/virtio_mmio.c b/drivers/virtio/virtio_mmio.c
index 57f24fd..bbc4410 100644
--- a/drivers/virtio/virtio_mmio.c
+++ b/drivers/virtio/virtio_mmio.c
@@ -101,7 +101,7 @@
 #include <linux/virtio_mmio.h>
 #include <linux/virtio_ring.h>
 
-
+#include <asm-generic/io-64-nonatomic-lo-hi.h>
 
 /* The alignment to use between consumer and producer parts of vring.
  * Currently hardcoded to the page size. */
@@ -168,25 +168,93 @@ static void vm_finalize_features(struct virtio_device *vdev)
 }
 
 static void vm_get(struct virtio_device *vdev, unsigned offset,
-		   void *buf, unsigned len)
+		   void *buf, unsigned len, unsigned access_size)
 {
 	struct virtio_mmio_device *vm_dev = to_virtio_mmio_device(vdev);
-	u8 *ptr = buf;
 	int i;
 
-	for (i = 0; i < len; i++)
-		ptr[i] = readb(vm_dev->base + VIRTIO_MMIO_CONFIG + offset + i);
+	switch (access_size) {
+	case 1: {
+		u8 *ptr = buf;
+
+		for (i = 0; i < len; i++)
+			ptr[i] = readb(vm_dev->base + VIRTIO_MMIO_CONFIG + offset + i);
+
+		break;
+	}
+	case 2: {
+		u16 *ptr = buf;
+
+		for (i = 0; i < len; i++)
+			ptr[i] = readw(vm_dev->base + VIRTIO_MMIO_CONFIG + offset + i);
+
+		break;
+	}
+	case 4: {
+		u32 *ptr = buf;
+
+		for (i = 0; i < len; i++)
+			ptr[i] = readl(vm_dev->base + VIRTIO_MMIO_CONFIG + offset + i);
+
+		break;
+	}
+	case 8: {
+		u64 *ptr = buf;
+
+		for (i = 0; i < len; i++)
+			ptr[i] = readq(vm_dev->base + VIRTIO_MMIO_CONFIG + offset + i);
+
+		break;
+	}
+	default:
+		pr_err("virtio: illegal access size %d\n", access_size);
+		BUG();
+	}
 }
 
 static void vm_set(struct virtio_device *vdev, unsigned offset,
-		   const void *buf, unsigned len)
+		   const void *buf, unsigned len, unsigned access_size)
 {
 	struct virtio_mmio_device *vm_dev = to_virtio_mmio_device(vdev);
-	const u8 *ptr = buf;
 	int i;
 
-	for (i = 0; i < len; i++)
-		writeb(ptr[i], vm_dev->base + VIRTIO_MMIO_CONFIG + offset + i);
+	switch (access_size) {
+	case 1: {
+		const u8 *ptr = buf;
+
+		for (i = 0; i < len; i++)
+			writeb(ptr[i], vm_dev->base + VIRTIO_MMIO_CONFIG + offset + i);
+
+		break;
+	}
+	case 2: {
+		const u16 *ptr = buf;
+
+		for (i = 0; i < len; i++)
+			writew(ptr[i], vm_dev->base + VIRTIO_MMIO_CONFIG + offset + i);
+
+		break;
+	}
+	case 4: {
+		const u32 *ptr = buf;
+
+		for (i = 0; i < len; i++)
+			writel(ptr[i], vm_dev->base + VIRTIO_MMIO_CONFIG + offset + i);
+
+		break;
+	}
+	case 8: {
+		const u64 *ptr = buf;
+
+		for (i = 0; i < len; i++)
+			writeq(ptr[i], vm_dev->base + VIRTIO_MMIO_CONFIG + offset + i);
+
+		break;
+	}
+	default:
+		pr_err("virtio: illegal access size %d\n", access_size);
+		BUG();
+	}
 }
 
 static u8 vm_get_status(struct virtio_device *vdev)
diff --git a/drivers/virtio/virtio_pci.c b/drivers/virtio/virtio_pci.c
index 98917fc..f06671c 100644
--- a/drivers/virtio/virtio_pci.c
+++ b/drivers/virtio/virtio_pci.c
@@ -129,7 +129,7 @@ static void vp_finalize_features(struct virtio_device *vdev)
 
 /* virtio config->get() implementation */
 static void vp_get(struct virtio_device *vdev, unsigned offset,
-		   void *buf, unsigned len)
+		   void *buf, unsigned len, unsigned access_size)
 {
 	struct virtio_pci_device *vp_dev = to_vp_device(vdev);
 	void __iomem *ioaddr = vp_dev->ioaddr +
@@ -137,6 +137,8 @@ static void vp_get(struct virtio_device *vdev, unsigned offset,
 	u8 *ptr = buf;
 	int i;
 
+	len *= access_size;
+
 	for (i = 0; i < len; i++)
 		ptr[i] = ioread8(ioaddr + i);
 }
@@ -144,7 +146,7 @@ static void vp_get(struct virtio_device *vdev, unsigned offset,
 /* the config->set() implementation.  it's symmetric to the config->get()
  * implementation */
 static void vp_set(struct virtio_device *vdev, unsigned offset,
-		   const void *buf, unsigned len)
+		   const void *buf, unsigned len, unsigned access_size)
 {
 	struct virtio_pci_device *vp_dev = to_vp_device(vdev);
 	void __iomem *ioaddr = vp_dev->ioaddr +
@@ -152,6 +154,8 @@ static void vp_set(struct virtio_device *vdev, unsigned offset,
 	const u8 *ptr = buf;
 	int i;
 
+	len *= access_size;
+
 	for (i = 0; i < len; i++)
 		iowrite8(ptr[i], ioaddr + i);
 }
diff --git a/include/linux/virtio_config.h b/include/linux/virtio_config.h
index 29b9104..69d1884 100644
--- a/include/linux/virtio_config.h
+++ b/include/linux/virtio_config.h
@@ -12,12 +12,14 @@
  *	vdev: the virtio_device
  *	offset: the offset of the configuration field
  *	buf: the buffer to write the field value into.
- *	len: the length of the buffer
+ *	len: the length of the buffer in access_size unit
+ *	access_size: access length
  * @set: write the value of a configuration field
  *	vdev: the virtio_device
  *	offset: the offset of the configuration field
  *	buf: the buffer to read the field value from.
- *	len: the length of the buffer
+ *	len: the length of the buffer in access_size unit
+ *	access_size: access length
  * @get_status: read the status byte
  *	vdev: the virtio_device
  *	Returns the status byte
@@ -55,9 +57,9 @@
 typedef void vq_callback_t(struct virtqueue *);
 struct virtio_config_ops {
 	void (*get)(struct virtio_device *vdev, unsigned offset,
-		    void *buf, unsigned len);
+		    void *buf, unsigned len, unsigned access_size);
 	void (*set)(struct virtio_device *vdev, unsigned offset,
-		    const void *buf, unsigned len);
+		    const void *buf, unsigned len, unsigned access_size);
 	u8 (*get_status)(struct virtio_device *vdev);
 	void (*set_status)(struct virtio_device *vdev, u8 status);
 	void (*reset)(struct virtio_device *vdev);
@@ -106,20 +108,21 @@ static inline bool virtio_has_feature(const struct virtio_device *vdev,
  * The return value is -ENOENT if the feature doesn't exist.  Otherwise
  * the config value is copied into whatever is pointed to by v. */
 #define virtio_config_val(vdev, fbit, offset, v) \
-	virtio_config_buf((vdev), (fbit), (offset), (v), sizeof(*v))
+	virtio_config_buf((vdev), (fbit), (offset), (v), 1, sizeof(*v))
 
 #define virtio_config_val_len(vdev, fbit, offset, v, len) \
-	virtio_config_buf((vdev), (fbit), (offset), (v), (len))
+	virtio_config_buf((vdev), (fbit), (offset), (v), (len), 1)
 
 static inline int virtio_config_buf(struct virtio_device *vdev,
 				    unsigned int fbit,
 				    unsigned int offset,
-				    void *buf, unsigned len)
+				    void *buf, unsigned len,
+				    unsigned access_size)
 {
 	if (!virtio_has_feature(vdev, fbit))
 		return -ENOENT;
 
-	vdev->config->get(vdev, offset, buf, len);
+	vdev->config->get(vdev, offset, buf, len, access_size);
 	return 0;
 }
 
diff --git a/net/9p/trans_virtio.c b/net/9p/trans_virtio.c
index 990afab..d12a2aa 100644
--- a/net/9p/trans_virtio.c
+++ b/net/9p/trans_virtio.c
@@ -546,7 +546,7 @@ static int p9_virtio_probe(struct virtio_device *vdev)
 	if (virtio_has_feature(vdev, VIRTIO_9P_MOUNT_TAG)) {
 		vdev->config->get(vdev,
 				offsetof(struct virtio_9p_config, tag_len),
-				&tag_len, sizeof(tag_len));
+				  &tag_len, 1, sizeof(tag_len));
 	} else {
 		err = -EINVAL;
 		goto out_free_vq;
@@ -557,7 +557,7 @@ static int p9_virtio_probe(struct virtio_device *vdev)
 		goto out_free_vq;
 	}
 	vdev->config->get(vdev, offsetof(struct virtio_9p_config, tag),
-			tag, tag_len);
+			  tag, tag_len, 1);
 	chan->tag = tag;
 	chan->tag_len = tag_len;
 	err = sysfs_create_file(&(vdev->dev.kobj), &dev_attr_mount_tag.attr);
-- 
1.8.2.3

^ permalink raw reply related	[flat|nested] 86+ messages in thread
* Re: [PATCH 0/3] virtio-mmio: handle BE guests on LE hosts
  2013-10-11 14:36 ` Marc Zyngier
@ 2013-10-12 18:28   ` Michael S. Tsirkin
  -1 siblings, 0 replies; 86+ messages in thread
From: Michael S. Tsirkin @ 2013-10-12 18:28 UTC (permalink / raw)
  To: Marc Zyngier; +Cc: linux-arm-kernel, kvmarm, kvm, Rusty Russell, Pawel Moll

On Fri, Oct 11, 2013 at 03:36:08PM +0100, Marc Zyngier wrote:
> This small patch series adds just enough kernel infrastructure and
> fixes to allow a BE guest to use virtio-mmio on a LE host, provided
> that the host actually supports such madness.
> 
> This has been tested on arm64, with some fixes to KVM and a set of
> changes to kvmtool, 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: Rusty Russell <rusty@rustcorp.com.au>
> Cc: Michael S. Tsirkin <mst@redhat.com>
> Cc: Pawel Moll <pawel.moll@arm.com>

We are changing the spec to make everything LE instead of
the native endian.

I think that'll fix the issue in a cleaner way.

> Marc Zyngier (3):
>   virtio: let the guest report its endianess if advertized by the host
>   virtio: mmio: fix signature checking for BE guests
>   virtio: mmio: access configuration space as little-endian
> 
>  drivers/block/virtio_blk.c             |  6 +--
>  drivers/char/virtio_console.c          |  4 +-
>  drivers/lguest/lguest_device.c         |  8 +++-
>  drivers/net/caif/caif_virtio.c         |  2 +-
>  drivers/net/virtio_net.c               |  2 +-
>  drivers/remoteproc/remoteproc_virtio.c |  8 +++-
>  drivers/s390/kvm/kvm_virtio.c          |  8 +++-
>  drivers/s390/kvm/virtio_ccw.c          |  9 +++-
>  drivers/scsi/virtio_scsi.c             |  4 +-
>  drivers/virtio/virtio_balloon.c        |  8 ++--
>  drivers/virtio/virtio_mmio.c           | 88 ++++++++++++++++++++++++++++++----
>  drivers/virtio/virtio_pci.c            |  8 +++-
>  drivers/virtio/virtio_ring.c           |  8 ++++
>  include/linux/virtio_config.h          | 19 ++++----
>  include/uapi/linux/virtio_ring.h       |  8 ++++
>  net/9p/trans_virtio.c                  |  4 +-
>  16 files changed, 150 insertions(+), 44 deletions(-)
> 
> -- 
> 1.8.2.3
> 

^ permalink raw reply	[flat|nested] 86+ messages in thread
* [PATCH 0/3] virtio-mmio: handle BE guests on LE hosts
@ 2013-10-12 18:28   ` Michael S. Tsirkin
  0 siblings, 0 replies; 86+ messages in thread
From: Michael S. Tsirkin @ 2013-10-12 18:28 UTC (permalink / raw)
  To: linux-arm-kernel

On Fri, Oct 11, 2013 at 03:36:08PM +0100, Marc Zyngier wrote:
> This small patch series adds just enough kernel infrastructure and
> fixes to allow a BE guest to use virtio-mmio on a LE host, provided
> that the host actually supports such madness.
> 
> This has been tested on arm64, with some fixes to KVM and a set of
> changes to kvmtool, 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: Rusty Russell <rusty@rustcorp.com.au>
> Cc: Michael S. Tsirkin <mst@redhat.com>
> Cc: Pawel Moll <pawel.moll@arm.com>

We are changing the spec to make everything LE instead of
the native endian.

I think that'll fix the issue in a cleaner way.

> Marc Zyngier (3):
>   virtio: let the guest report its endianess if advertized by the host
>   virtio: mmio: fix signature checking for BE guests
>   virtio: mmio: access configuration space as little-endian
> 
>  drivers/block/virtio_blk.c             |  6 +--
>  drivers/char/virtio_console.c          |  4 +-
>  drivers/lguest/lguest_device.c         |  8 +++-
>  drivers/net/caif/caif_virtio.c         |  2 +-
>  drivers/net/virtio_net.c               |  2 +-
>  drivers/remoteproc/remoteproc_virtio.c |  8 +++-
>  drivers/s390/kvm/kvm_virtio.c          |  8 +++-
>  drivers/s390/kvm/virtio_ccw.c          |  9 +++-
>  drivers/scsi/virtio_scsi.c             |  4 +-
>  drivers/virtio/virtio_balloon.c        |  8 ++--
>  drivers/virtio/virtio_mmio.c           | 88 ++++++++++++++++++++++++++++++----
>  drivers/virtio/virtio_pci.c            |  8 +++-
>  drivers/virtio/virtio_ring.c           |  8 ++++
>  include/linux/virtio_config.h          | 19 ++++----
>  include/uapi/linux/virtio_ring.h       |  8 ++++
>  net/9p/trans_virtio.c                  |  4 +-
>  16 files changed, 150 insertions(+), 44 deletions(-)
> 
> -- 
> 1.8.2.3
> 

^ permalink raw reply	[flat|nested] 86+ messages in thread
* Re: [PATCH 0/3] virtio-mmio: handle BE guests on LE hosts
  2013-10-11 14:36 ` Marc Zyngier
@ 2013-10-14  8:21   ` Rusty Russell
  -1 siblings, 0 replies; 86+ messages in thread
From: Rusty Russell @ 2013-10-14  8:21 UTC (permalink / raw)
  To: Marc Zyngier, linux-arm-kernel, kvmarm, kvm
  Cc: Michael S. Tsirkin, Pawel Moll

Marc Zyngier <marc.zyngier@arm.com> writes:
> This small patch series adds just enough kernel infrastructure and
> fixes to allow a BE guest to use virtio-mmio on a LE host, provided
> that the host actually supports such madness.
>
> This has been tested on arm64, with some fixes to KVM and a set of
> changes to kvmtool, both which I am posting separately.

OK, so I already have a patch which supports config space accessors.
I've posted the series below, and since you want it, I'll put it in
virtio-next after more testing and rebasing...

But we won't be using feature bits for endianness, since we're defining
endian in the 1.0 spec (whether LE-everywhere or LE-for-mmio-and-PCI is
still debated).  Since we need to guess for backwards compat anyway,
let's keep doing this until v1.0?

(Yeah, I made this mess with "native endian".  I promise I have learnt
my lesson).

Thanks,
Rusty.

Subject: virtio_config: introduce size-based accessors.

This lets the us do endian conversion if necessary, and insulates the
drivers from that change.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>

diff --git a/include/linux/virtio_config.h b/include/linux/virtio_config.h
index 29b9104..e62acdd 100644
--- a/include/linux/virtio_config.h
+++ b/include/linux/virtio_config.h
@@ -162,5 +135,139 @@ int virtqueue_set_affinity(struct virtqueue *vq, int cpu)
 	return 0;
 }
 
+/* Config space accessors. */
+#define virtio_cread(vdev, structname, member, ptr)			\
+	do {								\
+		/* Must match the member's type, and be integer */	\
+		if (!typecheck(typeof((((structname*)0)->member)), *(ptr))) \
+			(*ptr) = 1;					\
+									\
+		switch (sizeof(*ptr)) {					\
+		case 1:							\
+			*(ptr) = virtio_cread8(vdev,			\
+					       offsetof(structname, member)); \
+			break;						\
+		case 2:							\
+			*(ptr) = virtio_cread16(vdev,			\
+						offsetof(structname, member)); \
+			break;						\
+		case 4:							\
+			*(ptr) = virtio_cread32(vdev,			\
+						offsetof(structname, member)); \
+			break;						\
+		case 8:							\
+			*(ptr) = virtio_cread64(vdev,			\
+						offsetof(structname, member)); \
+			break;						\
+		default:						\
+			BUG();						\
+		}							\
+	} while(0)
+
+/* Config space accessors. */
+#define virtio_cwrite(vdev, structname, member, ptr)			\
+	do {								\
+		/* Must match the member's type, and be integer */	\
+		if (!typecheck(typeof((((structname*)0)->member)), *(ptr))) \
+			BUG_ON((*ptr) == 1);				\
+									\
+		switch (sizeof(*ptr)) {					\
+		case 1:							\
+			virtio_cwrite8(vdev,				\
+				       offsetof(structname, member),	\
+				       *(ptr));				\
+			break;						\
+		case 2:							\
+			virtio_cwrite16(vdev,				\
+					offsetof(structname, member),	\
+					*(ptr));			\
+			break;						\
+		case 4:							\
+			virtio_cwrite32(vdev,				\
+					offsetof(structname, member),	\
+					*(ptr));			\
+			break;						\
+		case 8:							\
+			virtio_cwrite64(vdev,				\
+					offsetof(structname, member),	\
+					*(ptr));			\
+			break;						\
+		default:						\
+			BUG();						\
+		}							\
+	} while(0)
+
+static inline u8 virtio_cread8(struct virtio_device *vdev, unsigned int offset)
+{
+	u8 ret;
+	vdev->config->get(vdev, offset, &ret, sizeof(ret));
+	return ret;
+}
+
+static inline void virtio_cread_bytes(struct virtio_device *vdev,
+				      unsigned int offset,
+				      void *buf, size_t len)
+{
+	vdev->config->get(vdev, offset, buf, len);
+}
+
+static inline void virtio_cwrite8(struct virtio_device *vdev,
+				  unsigned int offset, u8 val)
+{
+	vdev->config->set(vdev, offset, &val, sizeof(val));
+}
+
+static inline u16 virtio_cread16(struct virtio_device *vdev,
+				 unsigned int offset)
+{
+	u16 ret;
+	vdev->config->get(vdev, offset, &ret, sizeof(ret));
+	return ret;
+}
+
+static inline void virtio_cwrite16(struct virtio_device *vdev,
+				   unsigned int offset, u16 val)
+{
+	vdev->config->set(vdev, offset, &val, sizeof(val));
+}
+
+static inline u32 virtio_cread32(struct virtio_device *vdev,
+				 unsigned int offset)
+{
+	u32 ret;
+	vdev->config->get(vdev, offset, &ret, sizeof(ret));
+	return ret;
+}
+
+static inline void virtio_cwrite32(struct virtio_device *vdev,
+				   unsigned int offset, u32 val)
+{
+	vdev->config->set(vdev, offset, &val, sizeof(val));
+}
+
+static inline u64 virtio_cread64(struct virtio_device *vdev,
+				 unsigned int offset)
+{
+	u64 ret;
+	vdev->config->get(vdev, offset, &ret, sizeof(ret));
+	return ret;
+}
+
+static inline void virtio_cwrite64(struct virtio_device *vdev,
+				   unsigned int offset, u64 val)
+{
+	vdev->config->set(vdev, offset, &val, sizeof(val));
+}
+
+/* Conditional config space accessors. */
+#define virtio_cread_feature(vdev, fbit, structname, member, ptr)	\
+	({								\
+		int _r = 0;						\
+		if (!virtio_has_feature(vdev, fbit))			\
+			_r = -ENOENT;					\
+		else							\
+			virtio_cread((vdev), structname, member, ptr);	\
+		_r;							\
+	})
 
 #endif /* _LINUX_VIRTIO_CONFIG_H */

Subject: virtio: use size-based config accessors.

This lets the transport do endian conversion if necessary, and insulates
the drivers from the difference.

Most drivers can use the simple helpers virtio_cread() and virtio_cwrite().

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>

diff --git a/drivers/block/virtio_blk.c b/drivers/block/virtio_blk.c
index 6472395..1b1bbd3 100644
--- a/drivers/block/virtio_blk.c
+++ b/drivers/block/virtio_blk.c
@@ -456,18 +456,15 @@ static int virtblk_ioctl(struct block_device *bdev, fmode_t mode,
 static int virtblk_getgeo(struct block_device *bd, struct hd_geometry *geo)
 {
 	struct virtio_blk *vblk = bd->bd_disk->private_data;
-	struct virtio_blk_geometry vgeo;
-	int err;
 
 	/* see if the host passed in geometry config */
-	err = virtio_config_val(vblk->vdev, VIRTIO_BLK_F_GEOMETRY,
-				offsetof(struct virtio_blk_config, geometry),
-				&vgeo);
-
-	if (!err) {
-		geo->heads = vgeo.heads;
-		geo->sectors = vgeo.sectors;
-		geo->cylinders = vgeo.cylinders;
+	if (virtio_has_feature(vblk->vdev, VIRTIO_BLK_F_GEOMETRY)) {
+		virtio_cread(vblk->vdev, struct virtio_blk_config,
+			     geometry.cylinders, &geo->cylinders);
+		virtio_cread(vblk->vdev, struct virtio_blk_config,
+			     geometry.heads, &geo->heads);
+		virtio_cread(vblk->vdev, struct virtio_blk_config,
+			     geometry.sectors, &geo->sectors);
 	} else {
 		/* some standard values, similar to sd */
 		geo->heads = 1 << 6;
@@ -529,8 +526,7 @@ static void virtblk_config_changed_work(struct work_struct *work)
 		goto done;
 
 	/* Host must always specify the capacity. */
-	vdev->config->get(vdev, offsetof(struct virtio_blk_config, capacity),
-			  &capacity, sizeof(capacity));
+	virtio_cread(vdev, struct virtio_blk_config, capacity, &capacity);
 
 	/* If capacity is too big, truncate with warning. */
 	if ((sector_t)capacity != capacity) {
@@ -608,9 +604,9 @@ static int virtblk_get_cache_mode(struct virtio_device *vdev)
 	u8 writeback;
 	int err;
 
-	err = virtio_config_val(vdev, VIRTIO_BLK_F_CONFIG_WCE,
-				offsetof(struct virtio_blk_config, wce),
-				&writeback);
+	err = virtio_cread_feature(vdev, VIRTIO_BLK_F_CONFIG_WCE,
+				   struct virtio_blk_config, wce,
+				   &writeback);
 	if (err)
 		writeback = virtio_has_feature(vdev, VIRTIO_BLK_F_WCE);
 
@@ -642,7 +638,6 @@ virtblk_cache_type_store(struct device *dev, struct device_attribute *attr,
 	struct virtio_blk *vblk = disk->private_data;
 	struct virtio_device *vdev = vblk->vdev;
 	int i;
-	u8 writeback;
 
 	BUG_ON(!virtio_has_feature(vblk->vdev, VIRTIO_BLK_F_CONFIG_WCE));
 	for (i = ARRAY_SIZE(virtblk_cache_types); --i >= 0; )
@@ -652,11 +647,7 @@ virtblk_cache_type_store(struct device *dev, struct device_attribute *attr,
 	if (i < 0)
 		return -EINVAL;
 
-	writeback = i;
-	vdev->config->set(vdev,
-			  offsetof(struct virtio_blk_config, wce),
-			  &writeback, sizeof(writeback));
-
+	virtio_cwrite8(vdev, offsetof(struct virtio_blk_config, wce), i);
 	virtblk_update_cache_mode(vdev);
 	return count;
 }
@@ -699,9 +690,9 @@ static int virtblk_probe(struct virtio_device *vdev)
 	index = err;
 
 	/* We need to know how many segments before we allocate. */
-	err = virtio_config_val(vdev, VIRTIO_BLK_F_SEG_MAX,
-				offsetof(struct virtio_blk_config, seg_max),
-				&sg_elems);
+	err = virtio_cread_feature(vdev, VIRTIO_BLK_F_SEG_MAX,
+				   struct virtio_blk_config, seg_max,
+				   &sg_elems);
 
 	/* We need at least one SG element, whatever they say. */
 	if (err || !sg_elems)
@@ -772,8 +763,7 @@ static int virtblk_probe(struct virtio_device *vdev)
 		set_disk_ro(vblk->disk, 1);
 
 	/* Host must always specify the capacity. */
-	vdev->config->get(vdev, offsetof(struct virtio_blk_config, capacity),
-			  &cap, sizeof(cap));
+	virtio_cread(vdev, struct virtio_blk_config, capacity, &cap);
 
 	/* If capacity is too big, truncate with warning. */
 	if ((sector_t)cap != cap) {
@@ -794,46 +784,45 @@ static int virtblk_probe(struct virtio_device *vdev)
 
 	/* Host can optionally specify maximum segment size and number of
 	 * segments. */
-	err = virtio_config_val(vdev, VIRTIO_BLK_F_SIZE_MAX,
-				offsetof(struct virtio_blk_config, size_max),
-				&v);
+	err = virtio_cread_feature(vdev, VIRTIO_BLK_F_SIZE_MAX,
+				   struct virtio_blk_config, size_max, &v);
 	if (!err)
 		blk_queue_max_segment_size(q, v);
 	else
 		blk_queue_max_segment_size(q, -1U);
 
 	/* Host can optionally specify the block size of the device */
-	err = virtio_config_val(vdev, VIRTIO_BLK_F_BLK_SIZE,
-				offsetof(struct virtio_blk_config, blk_size),
-				&blk_size);
+	err = virtio_cread_feature(vdev, VIRTIO_BLK_F_BLK_SIZE,
+				   struct virtio_blk_config, blk_size,
+				   &blk_size);
 	if (!err)
 		blk_queue_logical_block_size(q, blk_size);
 	else
 		blk_size = queue_logical_block_size(q);
 
 	/* Use topology information if available */
-	err = virtio_config_val(vdev, VIRTIO_BLK_F_TOPOLOGY,
-			offsetof(struct virtio_blk_config, physical_block_exp),
-			&physical_block_exp);
+	err = virtio_cread_feature(vdev, VIRTIO_BLK_F_TOPOLOGY,
+				   struct virtio_blk_config, physical_block_exp,
+				   &physical_block_exp);
 	if (!err && physical_block_exp)
 		blk_queue_physical_block_size(q,
 				blk_size * (1 << physical_block_exp));
 
-	err = virtio_config_val(vdev, VIRTIO_BLK_F_TOPOLOGY,
-			offsetof(struct virtio_blk_config, alignment_offset),
-			&alignment_offset);
+	err = virtio_cread_feature(vdev, VIRTIO_BLK_F_TOPOLOGY,
+				   struct virtio_blk_config, alignment_offset,
+				   &alignment_offset);
 	if (!err && alignment_offset)
 		blk_queue_alignment_offset(q, blk_size * alignment_offset);
 
-	err = virtio_config_val(vdev, VIRTIO_BLK_F_TOPOLOGY,
-			offsetof(struct virtio_blk_config, min_io_size),
-			&min_io_size);
+	err = virtio_cread_feature(vdev, VIRTIO_BLK_F_TOPOLOGY,
+				   struct virtio_blk_config, min_io_size,
+				   &min_io_size);
 	if (!err && min_io_size)
 		blk_queue_io_min(q, blk_size * min_io_size);
 
-	err = virtio_config_val(vdev, VIRTIO_BLK_F_TOPOLOGY,
-			offsetof(struct virtio_blk_config, opt_io_size),
-			&opt_io_size);
+	err = virtio_cread_feature(vdev, VIRTIO_BLK_F_TOPOLOGY,
+				   struct virtio_blk_config, opt_io_size,
+				   &opt_io_size);
 	if (!err && opt_io_size)
 		blk_queue_io_opt(q, blk_size * opt_io_size);
 
diff --git a/drivers/char/virtio_console.c b/drivers/char/virtio_console.c
index e6ba6b7..1735c38 100644
--- a/drivers/char/virtio_console.c
+++ b/drivers/char/virtio_console.c
@@ -1800,12 +1800,8 @@ static void config_intr(struct virtio_device *vdev)
 		struct port *port;
 		u16 rows, cols;
 
-		vdev->config->get(vdev,
-				  offsetof(struct virtio_console_config, cols),
-				  &cols, sizeof(u16));
-		vdev->config->get(vdev,
-				  offsetof(struct virtio_console_config, rows),
-				  &rows, sizeof(u16));
+		virtio_cread(vdev, struct virtio_console_config, cols, &cols);
+		virtio_cread(vdev, struct virtio_console_config, rows, &rows);
 
 		port = find_port_by_id(portdev, 0);
 		set_console_size(port, rows, cols);
@@ -1977,10 +1973,9 @@ static int virtcons_probe(struct virtio_device *vdev)
 
 	/* Don't test MULTIPORT at all if we're rproc: not a valid feature! */
 	if (!is_rproc_serial(vdev) &&
-	    virtio_config_val(vdev, VIRTIO_CONSOLE_F_MULTIPORT,
-				  offsetof(struct virtio_console_config,
-					   max_nr_ports),
-				  &portdev->config.max_nr_ports) == 0) {
+	    virtio_cread_feature(vdev, VIRTIO_CONSOLE_F_MULTIPORT,
+				 struct virtio_console_config, max_nr_ports,
+				 &portdev->config.max_nr_ports) == 0) {
 		multiport = true;
 	}
 
diff --git a/drivers/net/caif/caif_virtio.c b/drivers/net/caif/caif_virtio.c
index 8308dee..ef602e3 100644
--- a/drivers/net/caif/caif_virtio.c
+++ b/drivers/net/caif/caif_virtio.c
@@ -682,18 +682,19 @@ static int cfv_probe(struct virtio_device *vdev)
 		goto err;
 
 	/* Get the CAIF configuration from virtio config space, if available */
-#define GET_VIRTIO_CONFIG_OPS(_v, _var, _f) \
-	((_v)->config->get(_v, offsetof(struct virtio_caif_transf_config, _f), \
-			   &_var, \
-			   FIELD_SIZEOF(struct virtio_caif_transf_config, _f)))
-
 	if (vdev->config->get) {
-		GET_VIRTIO_CONFIG_OPS(vdev, cfv->tx_hr, headroom);
-		GET_VIRTIO_CONFIG_OPS(vdev, cfv->rx_hr, headroom);
-		GET_VIRTIO_CONFIG_OPS(vdev, cfv->tx_tr, tailroom);
-		GET_VIRTIO_CONFIG_OPS(vdev, cfv->rx_tr, tailroom);
-		GET_VIRTIO_CONFIG_OPS(vdev, cfv->mtu, mtu);
-		GET_VIRTIO_CONFIG_OPS(vdev, cfv->mru, mtu);
+		virtio_cread(vdev, struct virtio_caif_transf_config, headroom,
+			     &cfv->tx_hr);
+		virtio_cread(vdev, struct virtio_caif_transf_config, headroom,
+			     &cfv->rx_hr);
+		virtio_cread(vdev, struct virtio_caif_transf_config, tailroom,
+			     &cfv->tx_tr);
+		virtio_cread(vdev, struct virtio_caif_transf_config, tailroom,
+			     &cfv->rx_tr);
+		virtio_cread(vdev, struct virtio_caif_transf_config, mtu,
+			     &cfv->mtu);
+		virtio_cread(vdev, struct virtio_caif_transf_config, mtu,
+			     &cfv->mru);
 	} else {
 		cfv->tx_hr = CFV_DEF_HEADROOM;
 		cfv->rx_hr = CFV_DEF_HEADROOM;
diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index be70487..61c1592 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -829,8 +829,13 @@ static int virtnet_set_mac_address(struct net_device *dev, void *p)
 			return -EINVAL;
 		}
 	} else if (virtio_has_feature(vdev, VIRTIO_NET_F_MAC)) {
-		vdev->config->set(vdev, offsetof(struct virtio_net_config, mac),
-				  addr->sa_data, dev->addr_len);
+		unsigned int i;
+
+		/* Naturally, this has an atomicity problem. */
+		for (i = 0; i < dev->addr_len; i++)
+			virtio_cwrite8(vdev,
+				       offsetof(struct virtio_net_config, mac) +
+				       i, addr->sa_data[i]);
 	}
 
 	eth_commit_mac_addr_change(dev, p);
@@ -1239,9 +1244,8 @@ static void virtnet_config_changed_work(struct work_struct *work)
 	if (!vi->config_enable)
 		goto done;
 
-	if (virtio_config_val(vi->vdev, VIRTIO_NET_F_STATUS,
-			      offsetof(struct virtio_net_config, status),
-			      &v) < 0)
+	if (virtio_cread_feature(vi->vdev, VIRTIO_NET_F_STATUS,
+				 struct virtio_net_config, status, &v) < 0)
 		goto done;
 
 	if (v & VIRTIO_NET_S_ANNOUNCE) {
@@ -1463,9 +1467,9 @@ static int virtnet_probe(struct virtio_device *vdev)
 	u16 max_queue_pairs;
 
 	/* Find if host supports multiqueue virtio_net device */
-	err = virtio_config_val(vdev, VIRTIO_NET_F_MQ,
-				offsetof(struct virtio_net_config,
-				max_virtqueue_pairs), &max_queue_pairs);
+	err = virtio_cread_feature(vdev, VIRTIO_NET_F_MQ,
+				   struct virtio_net_config,
+				   max_virtqueue_pairs, &max_queue_pairs);
 
 	/* We need at least 2 queue's */
 	if (err || max_queue_pairs < VIRTIO_NET_CTRL_MQ_VQ_PAIRS_MIN ||
@@ -1513,9 +1517,11 @@ static int virtnet_probe(struct virtio_device *vdev)
 	}
 
 	/* Configuration may specify what MAC to use.  Otherwise random. */
-	if (virtio_config_val_len(vdev, VIRTIO_NET_F_MAC,
-				  offsetof(struct virtio_net_config, mac),
-				  dev->dev_addr, dev->addr_len) < 0)
+	if (virtio_has_feature(vdev, VIRTIO_NET_F_MAC))
+		virtio_cread_bytes(vdev,
+				   offsetof(struct virtio_net_config, mac),
+				   dev->dev_addr, dev->addr_len);
+	else
 		eth_hw_addr_random(dev);
 
 	/* Set up our device-specific information */
diff --git a/drivers/scsi/virtio_scsi.c b/drivers/scsi/virtio_scsi.c
index f679b8c..a20418f 100644
--- a/drivers/scsi/virtio_scsi.c
+++ b/drivers/scsi/virtio_scsi.c
@@ -547,19 +547,15 @@ static struct scsi_host_template virtscsi_host_template = {
 #define virtscsi_config_get(vdev, fld) \
 	({ \
 		typeof(((struct virtio_scsi_config *)0)->fld) __val; \
-		vdev->config->get(vdev, \
-				  offsetof(struct virtio_scsi_config, fld), \
-				  &__val, sizeof(__val)); \
+		virtio_cread(vdev, struct virtio_scsi_config, fld, &__val); \
 		__val; \
 	})
 
 #define virtscsi_config_set(vdev, fld, val) \
-	(void)({ \
+	do { \
 		typeof(((struct virtio_scsi_config *)0)->fld) __val = (val); \
-		vdev->config->set(vdev, \
-				  offsetof(struct virtio_scsi_config, fld), \
-				  &__val, sizeof(__val)); \
-	})
+		virtio_cwrite(vdev, struct virtio_scsi_config, fld, &__val); \
+	} while(0)
 
 static void virtscsi_init_vq(struct virtio_scsi_vq *virtscsi_vq,
 			     struct virtqueue *vq)
diff --git a/drivers/virtio/virtio_balloon.c b/drivers/virtio/virtio_balloon.c
index 8dab163..bbab952 100644
--- a/drivers/virtio/virtio_balloon.c
+++ b/drivers/virtio/virtio_balloon.c
@@ -273,9 +273,8 @@ static inline s64 towards_target(struct virtio_balloon *vb)
 	__le32 v;
 	s64 target;
 
-	vb->vdev->config->get(vb->vdev,
-			      offsetof(struct virtio_balloon_config, num_pages),
-			      &v, sizeof(v));
+	virtio_cread(vb->vdev, struct virtio_balloon_config, num_pages, &v);
+
 	target = le32_to_cpu(v);
 	return target - vb->num_pages;
 }
@@ -284,9 +283,8 @@ static void update_balloon_size(struct virtio_balloon *vb)
 {
 	__le32 actual = cpu_to_le32(vb->num_pages);
 
-	vb->vdev->config->set(vb->vdev,
-			      offsetof(struct virtio_balloon_config, actual),
-			      &actual, sizeof(actual));
+	virtio_cwrite(vb->vdev, struct virtio_balloon_config, num_pages,
+		      &actual);
 }
 
 static int balloon(void *_vballoon)
diff --git a/net/9p/trans_virtio.c b/net/9p/trans_virtio.c
index de2e950..d843d5d 100644
--- a/net/9p/trans_virtio.c
+++ b/net/9p/trans_virtio.c
@@ -514,9 +514,7 @@ static int p9_virtio_probe(struct virtio_device *vdev)
 
 	chan->inuse = false;
 	if (virtio_has_feature(vdev, VIRTIO_9P_MOUNT_TAG)) {
-		vdev->config->get(vdev,
-				offsetof(struct virtio_9p_config, tag_len),
-				&tag_len, sizeof(tag_len));
+		virtio_cread(vdev, struct virtio_9p_config, tag_len, &tag_len);
 	} else {
 		err = -EINVAL;
 		goto out_free_vq;
@@ -526,8 +524,9 @@ static int p9_virtio_probe(struct virtio_device *vdev)
 		err = -ENOMEM;
 		goto out_free_vq;
 	}
-	vdev->config->get(vdev, offsetof(struct virtio_9p_config, tag),
-			tag, tag_len);
+
+	virtio_cread_bytes(vdev, offsetof(struct virtio_9p_config, tag),
+			   tag, tag_len);
 	chan->tag = tag;
 	chan->tag_len = tag_len;
 	err = sysfs_create_file(&(vdev->dev.kobj), &dev_attr_mount_tag.attr);


Subject: virtio_config: remove virtio_config_val

The virtio_cread() functions should now be used.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>

diff --git a/include/linux/virtio_config.h b/include/linux/virtio_config.h
index 29b9104..e62acdd 100644
--- a/include/linux/virtio_config.h
+++ b/include/linux/virtio_config.h
@@ -96,33 +96,6 @@ static inline bool virtio_has_feature(const struct virtio_device *vdev,
 	return test_bit(fbit, vdev->features);
 }
 
-/**
- * virtio_config_val - look for a feature and get a virtio config entry.
- * @vdev: the virtio device
- * @fbit: the feature bit
- * @offset: the type to search for.
- * @v: a pointer to the value to fill in.
- *
- * The return value is -ENOENT if the feature doesn't exist.  Otherwise
- * the config value is copied into whatever is pointed to by v. */
-#define virtio_config_val(vdev, fbit, offset, v) \
-	virtio_config_buf((vdev), (fbit), (offset), (v), sizeof(*v))
-
-#define virtio_config_val_len(vdev, fbit, offset, v, len) \
-	virtio_config_buf((vdev), (fbit), (offset), (v), (len))
-
-static inline int virtio_config_buf(struct virtio_device *vdev,
-				    unsigned int fbit,
-				    unsigned int offset,
-				    void *buf, unsigned len)
-{
-	if (!virtio_has_feature(vdev, fbit))
-		return -ENOENT;
-
-	vdev->config->get(vdev, offset, buf, len);
-	return 0;
-}
-
 static inline
 struct virtqueue *virtio_find_single_vq(struct virtio_device *vdev,
 					vq_callback_t *c, const char *n)

^ permalink raw reply related	[flat|nested] 86+ messages in thread
* [PATCH 0/3] virtio-mmio: handle BE guests on LE hosts
@ 2013-10-14  8:21   ` Rusty Russell
  0 siblings, 0 replies; 86+ messages in thread
From: Rusty Russell @ 2013-10-14  8:21 UTC (permalink / raw)
  To: linux-arm-kernel

Marc Zyngier <marc.zyngier@arm.com> writes:
> This small patch series adds just enough kernel infrastructure and
> fixes to allow a BE guest to use virtio-mmio on a LE host, provided
> that the host actually supports such madness.
>
> This has been tested on arm64, with some fixes to KVM and a set of
> changes to kvmtool, both which I am posting separately.

OK, so I already have a patch which supports config space accessors.
I've posted the series below, and since you want it, I'll put it in
virtio-next after more testing and rebasing...

But we won't be using feature bits for endianness, since we're defining
endian in the 1.0 spec (whether LE-everywhere or LE-for-mmio-and-PCI is
still debated).  Since we need to guess for backwards compat anyway,
let's keep doing this until v1.0?

(Yeah, I made this mess with "native endian".  I promise I have learnt
my lesson).

Thanks,
Rusty.

Subject: virtio_config: introduce size-based accessors.

This lets the us do endian conversion if necessary, and insulates the
drivers from that change.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>

diff --git a/include/linux/virtio_config.h b/include/linux/virtio_config.h
index 29b9104..e62acdd 100644
--- a/include/linux/virtio_config.h
+++ b/include/linux/virtio_config.h
@@ -162,5 +135,139 @@ int virtqueue_set_affinity(struct virtqueue *vq, int cpu)
 	return 0;
 }
 
+/* Config space accessors. */
+#define virtio_cread(vdev, structname, member, ptr)			\
+	do {								\
+		/* Must match the member's type, and be integer */	\
+		if (!typecheck(typeof((((structname*)0)->member)), *(ptr))) \
+			(*ptr) = 1;					\
+									\
+		switch (sizeof(*ptr)) {					\
+		case 1:							\
+			*(ptr) = virtio_cread8(vdev,			\
+					       offsetof(structname, member)); \
+			break;						\
+		case 2:							\
+			*(ptr) = virtio_cread16(vdev,			\
+						offsetof(structname, member)); \
+			break;						\
+		case 4:							\
+			*(ptr) = virtio_cread32(vdev,			\
+						offsetof(structname, member)); \
+			break;						\
+		case 8:							\
+			*(ptr) = virtio_cread64(vdev,			\
+						offsetof(structname, member)); \
+			break;						\
+		default:						\
+			BUG();						\
+		}							\
+	} while(0)
+
+/* Config space accessors. */
+#define virtio_cwrite(vdev, structname, member, ptr)			\
+	do {								\
+		/* Must match the member's type, and be integer */	\
+		if (!typecheck(typeof((((structname*)0)->member)), *(ptr))) \
+			BUG_ON((*ptr) == 1);				\
+									\
+		switch (sizeof(*ptr)) {					\
+		case 1:							\
+			virtio_cwrite8(vdev,				\
+				       offsetof(structname, member),	\
+				       *(ptr));				\
+			break;						\
+		case 2:							\
+			virtio_cwrite16(vdev,				\
+					offsetof(structname, member),	\
+					*(ptr));			\
+			break;						\
+		case 4:							\
+			virtio_cwrite32(vdev,				\
+					offsetof(structname, member),	\
+					*(ptr));			\
+			break;						\
+		case 8:							\
+			virtio_cwrite64(vdev,				\
+					offsetof(structname, member),	\
+					*(ptr));			\
+			break;						\
+		default:						\
+			BUG();						\
+		}							\
+	} while(0)
+
+static inline u8 virtio_cread8(struct virtio_device *vdev, unsigned int offset)
+{
+	u8 ret;
+	vdev->config->get(vdev, offset, &ret, sizeof(ret));
+	return ret;
+}
+
+static inline void virtio_cread_bytes(struct virtio_device *vdev,
+				      unsigned int offset,
+				      void *buf, size_t len)
+{
+	vdev->config->get(vdev, offset, buf, len);
+}
+
+static inline void virtio_cwrite8(struct virtio_device *vdev,
+				  unsigned int offset, u8 val)
+{
+	vdev->config->set(vdev, offset, &val, sizeof(val));
+}
+
+static inline u16 virtio_cread16(struct virtio_device *vdev,
+				 unsigned int offset)
+{
+	u16 ret;
+	vdev->config->get(vdev, offset, &ret, sizeof(ret));
+	return ret;
+}
+
+static inline void virtio_cwrite16(struct virtio_device *vdev,
+				   unsigned int offset, u16 val)
+{
+	vdev->config->set(vdev, offset, &val, sizeof(val));
+}
+
+static inline u32 virtio_cread32(struct virtio_device *vdev,
+				 unsigned int offset)
+{
+	u32 ret;
+	vdev->config->get(vdev, offset, &ret, sizeof(ret));
+	return ret;
+}
+
+static inline void virtio_cwrite32(struct virtio_device *vdev,
+				   unsigned int offset, u32 val)
+{
+	vdev->config->set(vdev, offset, &val, sizeof(val));
+}
+
+static inline u64 virtio_cread64(struct virtio_device *vdev,
+				 unsigned int offset)
+{
+	u64 ret;
+	vdev->config->get(vdev, offset, &ret, sizeof(ret));
+	return ret;
+}
+
+static inline void virtio_cwrite64(struct virtio_device *vdev,
+				   unsigned int offset, u64 val)
+{
+	vdev->config->set(vdev, offset, &val, sizeof(val));
+}
+
+/* Conditional config space accessors. */
+#define virtio_cread_feature(vdev, fbit, structname, member, ptr)	\
+	({								\
+		int _r = 0;						\
+		if (!virtio_has_feature(vdev, fbit))			\
+			_r = -ENOENT;					\
+		else							\
+			virtio_cread((vdev), structname, member, ptr);	\
+		_r;							\
+	})
 
 #endif /* _LINUX_VIRTIO_CONFIG_H */

Subject: virtio: use size-based config accessors.

This lets the transport do endian conversion if necessary, and insulates
the drivers from the difference.

Most drivers can use the simple helpers virtio_cread() and virtio_cwrite().

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>

diff --git a/drivers/block/virtio_blk.c b/drivers/block/virtio_blk.c
index 6472395..1b1bbd3 100644
--- a/drivers/block/virtio_blk.c
+++ b/drivers/block/virtio_blk.c
@@ -456,18 +456,15 @@ static int virtblk_ioctl(struct block_device *bdev, fmode_t mode,
 static int virtblk_getgeo(struct block_device *bd, struct hd_geometry *geo)
 {
 	struct virtio_blk *vblk = bd->bd_disk->private_data;
-	struct virtio_blk_geometry vgeo;
-	int err;
 
 	/* see if the host passed in geometry config */
-	err = virtio_config_val(vblk->vdev, VIRTIO_BLK_F_GEOMETRY,
-				offsetof(struct virtio_blk_config, geometry),
-				&vgeo);
-
-	if (!err) {
-		geo->heads = vgeo.heads;
-		geo->sectors = vgeo.sectors;
-		geo->cylinders = vgeo.cylinders;
+	if (virtio_has_feature(vblk->vdev, VIRTIO_BLK_F_GEOMETRY)) {
+		virtio_cread(vblk->vdev, struct virtio_blk_config,
+			     geometry.cylinders, &geo->cylinders);
+		virtio_cread(vblk->vdev, struct virtio_blk_config,
+			     geometry.heads, &geo->heads);
+		virtio_cread(vblk->vdev, struct virtio_blk_config,
+			     geometry.sectors, &geo->sectors);
 	} else {
 		/* some standard values, similar to sd */
 		geo->heads = 1 << 6;
@@ -529,8 +526,7 @@ static void virtblk_config_changed_work(struct work_struct *work)
 		goto done;
 
 	/* Host must always specify the capacity. */
-	vdev->config->get(vdev, offsetof(struct virtio_blk_config, capacity),
-			  &capacity, sizeof(capacity));
+	virtio_cread(vdev, struct virtio_blk_config, capacity, &capacity);
 
 	/* If capacity is too big, truncate with warning. */
 	if ((sector_t)capacity != capacity) {
@@ -608,9 +604,9 @@ static int virtblk_get_cache_mode(struct virtio_device *vdev)
 	u8 writeback;
 	int err;
 
-	err = virtio_config_val(vdev, VIRTIO_BLK_F_CONFIG_WCE,
-				offsetof(struct virtio_blk_config, wce),
-				&writeback);
+	err = virtio_cread_feature(vdev, VIRTIO_BLK_F_CONFIG_WCE,
+				   struct virtio_blk_config, wce,
+				   &writeback);
 	if (err)
 		writeback = virtio_has_feature(vdev, VIRTIO_BLK_F_WCE);
 
@@ -642,7 +638,6 @@ virtblk_cache_type_store(struct device *dev, struct device_attribute *attr,
 	struct virtio_blk *vblk = disk->private_data;
 	struct virtio_device *vdev = vblk->vdev;
 	int i;
-	u8 writeback;
 
 	BUG_ON(!virtio_has_feature(vblk->vdev, VIRTIO_BLK_F_CONFIG_WCE));
 	for (i = ARRAY_SIZE(virtblk_cache_types); --i >= 0; )
@@ -652,11 +647,7 @@ virtblk_cache_type_store(struct device *dev, struct device_attribute *attr,
 	if (i < 0)
 		return -EINVAL;
 
-	writeback = i;
-	vdev->config->set(vdev,
-			  offsetof(struct virtio_blk_config, wce),
-			  &writeback, sizeof(writeback));
-
+	virtio_cwrite8(vdev, offsetof(struct virtio_blk_config, wce), i);
 	virtblk_update_cache_mode(vdev);
 	return count;
 }
@@ -699,9 +690,9 @@ static int virtblk_probe(struct virtio_device *vdev)
 	index = err;
 
 	/* We need to know how many segments before we allocate. */
-	err = virtio_config_val(vdev, VIRTIO_BLK_F_SEG_MAX,
-				offsetof(struct virtio_blk_config, seg_max),
-				&sg_elems);
+	err = virtio_cread_feature(vdev, VIRTIO_BLK_F_SEG_MAX,
+				   struct virtio_blk_config, seg_max,
+				   &sg_elems);
 
 	/* We need at least one SG element, whatever they say. */
 	if (err || !sg_elems)
@@ -772,8 +763,7 @@ static int virtblk_probe(struct virtio_device *vdev)
 		set_disk_ro(vblk->disk, 1);
 
 	/* Host must always specify the capacity. */
-	vdev->config->get(vdev, offsetof(struct virtio_blk_config, capacity),
-			  &cap, sizeof(cap));
+	virtio_cread(vdev, struct virtio_blk_config, capacity, &cap);
 
 	/* If capacity is too big, truncate with warning. */
 	if ((sector_t)cap != cap) {
@@ -794,46 +784,45 @@ static int virtblk_probe(struct virtio_device *vdev)
 
 	/* Host can optionally specify maximum segment size and number of
 	 * segments. */
-	err = virtio_config_val(vdev, VIRTIO_BLK_F_SIZE_MAX,
-				offsetof(struct virtio_blk_config, size_max),
-				&v);
+	err = virtio_cread_feature(vdev, VIRTIO_BLK_F_SIZE_MAX,
+				   struct virtio_blk_config, size_max, &v);
 	if (!err)
 		blk_queue_max_segment_size(q, v);
 	else
 		blk_queue_max_segment_size(q, -1U);
 
 	/* Host can optionally specify the block size of the device */
-	err = virtio_config_val(vdev, VIRTIO_BLK_F_BLK_SIZE,
-				offsetof(struct virtio_blk_config, blk_size),
-				&blk_size);
+	err = virtio_cread_feature(vdev, VIRTIO_BLK_F_BLK_SIZE,
+				   struct virtio_blk_config, blk_size,
+				   &blk_size);
 	if (!err)
 		blk_queue_logical_block_size(q, blk_size);
 	else
 		blk_size = queue_logical_block_size(q);
 
 	/* Use topology information if available */
-	err = virtio_config_val(vdev, VIRTIO_BLK_F_TOPOLOGY,
-			offsetof(struct virtio_blk_config, physical_block_exp),
-			&physical_block_exp);
+	err = virtio_cread_feature(vdev, VIRTIO_BLK_F_TOPOLOGY,
+				   struct virtio_blk_config, physical_block_exp,
+				   &physical_block_exp);
 	if (!err && physical_block_exp)
 		blk_queue_physical_block_size(q,
 				blk_size * (1 << physical_block_exp));
 
-	err = virtio_config_val(vdev, VIRTIO_BLK_F_TOPOLOGY,
-			offsetof(struct virtio_blk_config, alignment_offset),
-			&alignment_offset);
+	err = virtio_cread_feature(vdev, VIRTIO_BLK_F_TOPOLOGY,
+				   struct virtio_blk_config, alignment_offset,
+				   &alignment_offset);
 	if (!err && alignment_offset)
 		blk_queue_alignment_offset(q, blk_size * alignment_offset);
 
-	err = virtio_config_val(vdev, VIRTIO_BLK_F_TOPOLOGY,
-			offsetof(struct virtio_blk_config, min_io_size),
-			&min_io_size);
+	err = virtio_cread_feature(vdev, VIRTIO_BLK_F_TOPOLOGY,
+				   struct virtio_blk_config, min_io_size,
+				   &min_io_size);
 	if (!err && min_io_size)
 		blk_queue_io_min(q, blk_size * min_io_size);
 
-	err = virtio_config_val(vdev, VIRTIO_BLK_F_TOPOLOGY,
-			offsetof(struct virtio_blk_config, opt_io_size),
-			&opt_io_size);
+	err = virtio_cread_feature(vdev, VIRTIO_BLK_F_TOPOLOGY,
+				   struct virtio_blk_config, opt_io_size,
+				   &opt_io_size);
 	if (!err && opt_io_size)
 		blk_queue_io_opt(q, blk_size * opt_io_size);
 
diff --git a/drivers/char/virtio_console.c b/drivers/char/virtio_console.c
index e6ba6b7..1735c38 100644
--- a/drivers/char/virtio_console.c
+++ b/drivers/char/virtio_console.c
@@ -1800,12 +1800,8 @@ static void config_intr(struct virtio_device *vdev)
 		struct port *port;
 		u16 rows, cols;
 
-		vdev->config->get(vdev,
-				  offsetof(struct virtio_console_config, cols),
-				  &cols, sizeof(u16));
-		vdev->config->get(vdev,
-				  offsetof(struct virtio_console_config, rows),
-				  &rows, sizeof(u16));
+		virtio_cread(vdev, struct virtio_console_config, cols, &cols);
+		virtio_cread(vdev, struct virtio_console_config, rows, &rows);
 
 		port = find_port_by_id(portdev, 0);
 		set_console_size(port, rows, cols);
@@ -1977,10 +1973,9 @@ static int virtcons_probe(struct virtio_device *vdev)
 
 	/* Don't test MULTIPORT at all if we're rproc: not a valid feature! */
 	if (!is_rproc_serial(vdev) &&
-	    virtio_config_val(vdev, VIRTIO_CONSOLE_F_MULTIPORT,
-				  offsetof(struct virtio_console_config,
-					   max_nr_ports),
-				  &portdev->config.max_nr_ports) == 0) {
+	    virtio_cread_feature(vdev, VIRTIO_CONSOLE_F_MULTIPORT,
+				 struct virtio_console_config, max_nr_ports,
+				 &portdev->config.max_nr_ports) == 0) {
 		multiport = true;
 	}
 
diff --git a/drivers/net/caif/caif_virtio.c b/drivers/net/caif/caif_virtio.c
index 8308dee..ef602e3 100644
--- a/drivers/net/caif/caif_virtio.c
+++ b/drivers/net/caif/caif_virtio.c
@@ -682,18 +682,19 @@ static int cfv_probe(struct virtio_device *vdev)
 		goto err;
 
 	/* Get the CAIF configuration from virtio config space, if available */
-#define GET_VIRTIO_CONFIG_OPS(_v, _var, _f) \
-	((_v)->config->get(_v, offsetof(struct virtio_caif_transf_config, _f), \
-			   &_var, \
-			   FIELD_SIZEOF(struct virtio_caif_transf_config, _f)))
-
 	if (vdev->config->get) {
-		GET_VIRTIO_CONFIG_OPS(vdev, cfv->tx_hr, headroom);
-		GET_VIRTIO_CONFIG_OPS(vdev, cfv->rx_hr, headroom);
-		GET_VIRTIO_CONFIG_OPS(vdev, cfv->tx_tr, tailroom);
-		GET_VIRTIO_CONFIG_OPS(vdev, cfv->rx_tr, tailroom);
-		GET_VIRTIO_CONFIG_OPS(vdev, cfv->mtu, mtu);
-		GET_VIRTIO_CONFIG_OPS(vdev, cfv->mru, mtu);
+		virtio_cread(vdev, struct virtio_caif_transf_config, headroom,
+			     &cfv->tx_hr);
+		virtio_cread(vdev, struct virtio_caif_transf_config, headroom,
+			     &cfv->rx_hr);
+		virtio_cread(vdev, struct virtio_caif_transf_config, tailroom,
+			     &cfv->tx_tr);
+		virtio_cread(vdev, struct virtio_caif_transf_config, tailroom,
+			     &cfv->rx_tr);
+		virtio_cread(vdev, struct virtio_caif_transf_config, mtu,
+			     &cfv->mtu);
+		virtio_cread(vdev, struct virtio_caif_transf_config, mtu,
+			     &cfv->mru);
 	} else {
 		cfv->tx_hr = CFV_DEF_HEADROOM;
 		cfv->rx_hr = CFV_DEF_HEADROOM;
diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index be70487..61c1592 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -829,8 +829,13 @@ static int virtnet_set_mac_address(struct net_device *dev, void *p)
 			return -EINVAL;
 		}
 	} else if (virtio_has_feature(vdev, VIRTIO_NET_F_MAC)) {
-		vdev->config->set(vdev, offsetof(struct virtio_net_config, mac),
-				  addr->sa_data, dev->addr_len);
+		unsigned int i;
+
+		/* Naturally, this has an atomicity problem. */
+		for (i = 0; i < dev->addr_len; i++)
+			virtio_cwrite8(vdev,
+				       offsetof(struct virtio_net_config, mac) +
+				       i, addr->sa_data[i]);
 	}
 
 	eth_commit_mac_addr_change(dev, p);
@@ -1239,9 +1244,8 @@ static void virtnet_config_changed_work(struct work_struct *work)
 	if (!vi->config_enable)
 		goto done;
 
-	if (virtio_config_val(vi->vdev, VIRTIO_NET_F_STATUS,
-			      offsetof(struct virtio_net_config, status),
-			      &v) < 0)
+	if (virtio_cread_feature(vi->vdev, VIRTIO_NET_F_STATUS,
+				 struct virtio_net_config, status, &v) < 0)
 		goto done;
 
 	if (v & VIRTIO_NET_S_ANNOUNCE) {
@@ -1463,9 +1467,9 @@ static int virtnet_probe(struct virtio_device *vdev)
 	u16 max_queue_pairs;
 
 	/* Find if host supports multiqueue virtio_net device */
-	err = virtio_config_val(vdev, VIRTIO_NET_F_MQ,
-				offsetof(struct virtio_net_config,
-				max_virtqueue_pairs), &max_queue_pairs);
+	err = virtio_cread_feature(vdev, VIRTIO_NET_F_MQ,
+				   struct virtio_net_config,
+				   max_virtqueue_pairs, &max_queue_pairs);
 
 	/* We need@least 2 queue's */
 	if (err || max_queue_pairs < VIRTIO_NET_CTRL_MQ_VQ_PAIRS_MIN ||
@@ -1513,9 +1517,11 @@ static int virtnet_probe(struct virtio_device *vdev)
 	}
 
 	/* Configuration may specify what MAC to use.  Otherwise random. */
-	if (virtio_config_val_len(vdev, VIRTIO_NET_F_MAC,
-				  offsetof(struct virtio_net_config, mac),
-				  dev->dev_addr, dev->addr_len) < 0)
+	if (virtio_has_feature(vdev, VIRTIO_NET_F_MAC))
+		virtio_cread_bytes(vdev,
+				   offsetof(struct virtio_net_config, mac),
+				   dev->dev_addr, dev->addr_len);
+	else
 		eth_hw_addr_random(dev);
 
 	/* Set up our device-specific information */
diff --git a/drivers/scsi/virtio_scsi.c b/drivers/scsi/virtio_scsi.c
index f679b8c..a20418f 100644
--- a/drivers/scsi/virtio_scsi.c
+++ b/drivers/scsi/virtio_scsi.c
@@ -547,19 +547,15 @@ static struct scsi_host_template virtscsi_host_template = {
 #define virtscsi_config_get(vdev, fld) \
 	({ \
 		typeof(((struct virtio_scsi_config *)0)->fld) __val; \
-		vdev->config->get(vdev, \
-				  offsetof(struct virtio_scsi_config, fld), \
-				  &__val, sizeof(__val)); \
+		virtio_cread(vdev, struct virtio_scsi_config, fld, &__val); \
 		__val; \
 	})
 
 #define virtscsi_config_set(vdev, fld, val) \
-	(void)({ \
+	do { \
 		typeof(((struct virtio_scsi_config *)0)->fld) __val = (val); \
-		vdev->config->set(vdev, \
-				  offsetof(struct virtio_scsi_config, fld), \
-				  &__val, sizeof(__val)); \
-	})
+		virtio_cwrite(vdev, struct virtio_scsi_config, fld, &__val); \
+	} while(0)
 
 static void virtscsi_init_vq(struct virtio_scsi_vq *virtscsi_vq,
 			     struct virtqueue *vq)
diff --git a/drivers/virtio/virtio_balloon.c b/drivers/virtio/virtio_balloon.c
index 8dab163..bbab952 100644
--- a/drivers/virtio/virtio_balloon.c
+++ b/drivers/virtio/virtio_balloon.c
@@ -273,9 +273,8 @@ static inline s64 towards_target(struct virtio_balloon *vb)
 	__le32 v;
 	s64 target;
 
-	vb->vdev->config->get(vb->vdev,
-			      offsetof(struct virtio_balloon_config, num_pages),
-			      &v, sizeof(v));
+	virtio_cread(vb->vdev, struct virtio_balloon_config, num_pages, &v);
+
 	target = le32_to_cpu(v);
 	return target - vb->num_pages;
 }
@@ -284,9 +283,8 @@ static void update_balloon_size(struct virtio_balloon *vb)
 {
 	__le32 actual = cpu_to_le32(vb->num_pages);
 
-	vb->vdev->config->set(vb->vdev,
-			      offsetof(struct virtio_balloon_config, actual),
-			      &actual, sizeof(actual));
+	virtio_cwrite(vb->vdev, struct virtio_balloon_config, num_pages,
+		      &actual);
 }
 
 static int balloon(void *_vballoon)
diff --git a/net/9p/trans_virtio.c b/net/9p/trans_virtio.c
index de2e950..d843d5d 100644
--- a/net/9p/trans_virtio.c
+++ b/net/9p/trans_virtio.c
@@ -514,9 +514,7 @@ static int p9_virtio_probe(struct virtio_device *vdev)
 
 	chan->inuse = false;
 	if (virtio_has_feature(vdev, VIRTIO_9P_MOUNT_TAG)) {
-		vdev->config->get(vdev,
-				offsetof(struct virtio_9p_config, tag_len),
-				&tag_len, sizeof(tag_len));
+		virtio_cread(vdev, struct virtio_9p_config, tag_len, &tag_len);
 	} else {
 		err = -EINVAL;
 		goto out_free_vq;
@@ -526,8 +524,9 @@ static int p9_virtio_probe(struct virtio_device *vdev)
 		err = -ENOMEM;
 		goto out_free_vq;
 	}
-	vdev->config->get(vdev, offsetof(struct virtio_9p_config, tag),
-			tag, tag_len);
+
+	virtio_cread_bytes(vdev, offsetof(struct virtio_9p_config, tag),
+			   tag, tag_len);
 	chan->tag = tag;
 	chan->tag_len = tag_len;
 	err = sysfs_create_file(&(vdev->dev.kobj), &dev_attr_mount_tag.attr);


Subject: virtio_config: remove virtio_config_val

The virtio_cread() functions should now be used.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>

diff --git a/include/linux/virtio_config.h b/include/linux/virtio_config.h
index 29b9104..e62acdd 100644
--- a/include/linux/virtio_config.h
+++ b/include/linux/virtio_config.h
@@ -96,33 +96,6 @@ static inline bool virtio_has_feature(const struct virtio_device *vdev,
 	return test_bit(fbit, vdev->features);
 }
 
-/**
- * virtio_config_val - look for a feature and get a virtio config entry.
- * @vdev: the virtio device
- * @fbit: the feature bit
- * @offset: the type to search for.
- * @v: a pointer to the value to fill in.
- *
- * The return value is -ENOENT if the feature doesn't exist.  Otherwise
- * the config value is copied into whatever is pointed to by v. */
-#define virtio_config_val(vdev, fbit, offset, v) \
-	virtio_config_buf((vdev), (fbit), (offset), (v), sizeof(*v))
-
-#define virtio_config_val_len(vdev, fbit, offset, v, len) \
-	virtio_config_buf((vdev), (fbit), (offset), (v), (len))
-
-static inline int virtio_config_buf(struct virtio_device *vdev,
-				    unsigned int fbit,
-				    unsigned int offset,
-				    void *buf, unsigned len)
-{
-	if (!virtio_has_feature(vdev, fbit))
-		return -ENOENT;
-
-	vdev->config->get(vdev, offset, buf, len);
-	return 0;
-}
-
 static inline
 struct virtqueue *virtio_find_single_vq(struct virtio_device *vdev,
 					vq_callback_t *c, const char *n)

^ permalink raw reply related	[flat|nested] 86+ messages in thread
* Re: [PATCH 0/3] virtio-mmio: handle BE guests on LE hosts
  2013-10-12 18:28   ` Michael S. Tsirkin
@ 2013-10-14  8:24     ` Marc Zyngier
  -1 siblings, 0 replies; 86+ messages in thread
From: Marc Zyngier @ 2013-10-14  8:24 UTC (permalink / raw)
  To: Michael S. Tsirkin
  Cc: linux-arm-kernel, kvmarm, kvm, Rusty Russell, Pawel Moll

Hi Michael,

On 12/10/13 19:28, Michael S. Tsirkin wrote:
> On Fri, Oct 11, 2013 at 03:36:08PM +0100, Marc Zyngier wrote:
>> This small patch series adds just enough kernel infrastructure and
>> fixes to allow a BE guest to use virtio-mmio on a LE host, provided
>> that the host actually supports such madness.
>>
>> This has been tested on arm64, with some fixes to KVM and a set of
>> changes to kvmtool, 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: Rusty Russell <rusty@rustcorp.com.au>
>> Cc: Michael S. Tsirkin <mst@redhat.com>
>> Cc: Pawel Moll <pawel.moll@arm.com>
> 
> We are changing the spec to make everything LE instead of
> the native endian.
> 
> I think that'll fix the issue in a cleaner way.

While I agree that it would solve the issue completely, it would also
break all BE users. Is that really an option?

The whole goal of this series is to implement something that gracefully
falls back to what we have today, not breaking anything in the process.

Cheers,

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


^ permalink raw reply	[flat|nested] 86+ messages in thread
* [PATCH 0/3] virtio-mmio: handle BE guests on LE hosts
@ 2013-10-14  8:24     ` Marc Zyngier
  0 siblings, 0 replies; 86+ messages in thread
From: Marc Zyngier @ 2013-10-14  8:24 UTC (permalink / raw)
  To: linux-arm-kernel

Hi Michael,

On 12/10/13 19:28, Michael S. Tsirkin wrote:
> On Fri, Oct 11, 2013 at 03:36:08PM +0100, Marc Zyngier wrote:
>> This small patch series adds just enough kernel infrastructure and
>> fixes to allow a BE guest to use virtio-mmio on a LE host, provided
>> that the host actually supports such madness.
>>
>> This has been tested on arm64, with some fixes to KVM and a set of
>> changes to kvmtool, 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: Rusty Russell <rusty@rustcorp.com.au>
>> Cc: Michael S. Tsirkin <mst@redhat.com>
>> Cc: Pawel Moll <pawel.moll@arm.com>
> 
> We are changing the spec to make everything LE instead of
> the native endian.
> 
> I think that'll fix the issue in a cleaner way.

While I agree that it would solve the issue completely, it would also
break all BE users. Is that really an option?

The whole goal of this series is to implement something that gracefully
falls back to what we have today, not breaking anything in the process.

Cheers,

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

^ permalink raw reply	[flat|nested] 86+ messages in thread
* Re: [PATCH 3/3] virtio: mmio: access configuration space as little-endian
  2013-10-11 14:36   ` Marc Zyngier
@ 2013-10-14  8:44     ` Pawel Moll
  -1 siblings, 0 replies; 86+ messages in thread
From: Pawel Moll @ 2013-10-14  8:44 UTC (permalink / raw)
  To: Marc Zyngier
  Cc: linux-arm-kernel, kvmarm, kvm, Rusty Russell, Michael S. Tsirkin

On Fri, 2013-10-11 at 15:36 +0100, Marc Zyngier wrote:
> virtio_mmio defines the config space to be little-endian. 

This is not exactly true. The configuration *registers* (magic value,
queue selector etc.) are always LE, yes. The configuration *space* (the
free-form stuff starting at 0x100) is still guest-endian.

The spec-in-works makes everything LE and the driver will most likely
get accessors for the config space. Rusty had a go at this a while ago:

http://thread.gmane.org/gmane.linux.kernel.virtualization/19397

Paweł



^ permalink raw reply	[flat|nested] 86+ messages in thread
* [PATCH 3/3] virtio: mmio: access configuration space as little-endian
@ 2013-10-14  8:44     ` Pawel Moll
  0 siblings, 0 replies; 86+ messages in thread
From: Pawel Moll @ 2013-10-14  8:44 UTC (permalink / raw)
  To: linux-arm-kernel

On Fri, 2013-10-11 at 15:36 +0100, Marc Zyngier wrote:
> virtio_mmio defines the config space to be little-endian. 

This is not exactly true. The configuration *registers* (magic value,
queue selector etc.) are always LE, yes. The configuration *space* (the
free-form stuff starting at 0x100) is still guest-endian.

The spec-in-works makes everything LE and the driver will most likely
get accessors for the config space. Rusty had a go at this a while ago:

http://thread.gmane.org/gmane.linux.kernel.virtualization/19397

Pawe?

^ permalink raw reply	[flat|nested] 86+ messages in thread
* Re: [PATCH 2/3] virtio: mmio: fix signature checking for BE guests
  2013-10-11 14:36   ` Marc Zyngier
@ 2013-10-14  8:46     ` Pawel Moll
  -1 siblings, 0 replies; 86+ messages in thread
From: Pawel Moll @ 2013-10-14  8:46 UTC (permalink / raw)
  To: Marc Zyngier
  Cc: linux-arm-kernel, kvmarm, kvm, Rusty Russell, Michael S. Tsirkin

On Fri, 2013-10-11 at 15:36 +0100, Marc Zyngier wrote:
> As virtio-mmio config registers are specified to be little-endian,
> using readl() to read the magic value and then memcmp() to check it
> fails on BE (as readl() has an implicit swab).
> 
> Fix it by encoding the magic value as an integer instead of a string.
> 
> Cc: Rusty Russell <rusty@rustcorp.com.au>
> Cc: Michael S. Tsirkin <mst@redhat.com>
> Cc: Pawel Moll <pawel.moll@arm.com>
> Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
> ---
>  drivers/virtio/virtio_mmio.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/virtio/virtio_mmio.c b/drivers/virtio/virtio_mmio.c
> index 1ba0d68..57f24fd 100644
> --- a/drivers/virtio/virtio_mmio.c
> +++ b/drivers/virtio/virtio_mmio.c
> @@ -470,7 +470,7 @@ static int virtio_mmio_probe(struct platform_device *pdev)
>  
>  	/* Check magic value */
>  	magic = readl(vm_dev->base + VIRTIO_MMIO_MAGIC_VALUE);
> -	if (memcmp(&magic, "virt", 4) != 0) {
> +	if (magic != ('v' | 'i' << 8 | 'r' << 16 | 't' << 24)) {
>  		dev_warn(&pdev->dev, "Wrong magic value 0x%08lx!\n", magic);
>  		return -ENODEV;
>  	}

The new spec will clarify this:

* 0x000 | R | MagicValue
  Magic value. Must be 0x74726976 (a Little Endian equivalent
  of a "virt" string).

... but I like the 'v'i'r't' characters still being there :-)

Acked-by: Pawel Moll <pawel.moll@arm.com>

Paweł



^ permalink raw reply	[flat|nested] 86+ messages in thread
* [PATCH 2/3] virtio: mmio: fix signature checking for BE guests
@ 2013-10-14  8:46     ` Pawel Moll
  0 siblings, 0 replies; 86+ messages in thread
From: Pawel Moll @ 2013-10-14  8:46 UTC (permalink / raw)
  To: linux-arm-kernel

On Fri, 2013-10-11 at 15:36 +0100, Marc Zyngier wrote:
> As virtio-mmio config registers are specified to be little-endian,
> using readl() to read the magic value and then memcmp() to check it
> fails on BE (as readl() has an implicit swab).
> 
> Fix it by encoding the magic value as an integer instead of a string.
> 
> Cc: Rusty Russell <rusty@rustcorp.com.au>
> Cc: Michael S. Tsirkin <mst@redhat.com>
> Cc: Pawel Moll <pawel.moll@arm.com>
> Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
> ---
>  drivers/virtio/virtio_mmio.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/virtio/virtio_mmio.c b/drivers/virtio/virtio_mmio.c
> index 1ba0d68..57f24fd 100644
> --- a/drivers/virtio/virtio_mmio.c
> +++ b/drivers/virtio/virtio_mmio.c
> @@ -470,7 +470,7 @@ static int virtio_mmio_probe(struct platform_device *pdev)
>  
>  	/* Check magic value */
>  	magic = readl(vm_dev->base + VIRTIO_MMIO_MAGIC_VALUE);
> -	if (memcmp(&magic, "virt", 4) != 0) {
> +	if (magic != ('v' | 'i' << 8 | 'r' << 16 | 't' << 24)) {
>  		dev_warn(&pdev->dev, "Wrong magic value 0x%08lx!\n", magic);
>  		return -ENODEV;
>  	}

The new spec will clarify this:

* 0x000 | R | MagicValue
  Magic value. Must be 0x74726976 (a Little Endian equivalent
  of a "virt" string).

... but I like the 'v'i'r't' characters still being there :-)

Acked-by: Pawel Moll <pawel.moll@arm.com>

Pawe?

^ permalink raw reply	[flat|nested] 86+ messages in thread
* Re: [PATCH 0/3] virtio-mmio: handle BE guests on LE hosts
  2013-10-14  8:24     ` Marc Zyngier
@ 2013-10-14  8:59       ` Michael S. Tsirkin
  -1 siblings, 0 replies; 86+ messages in thread
From: Michael S. Tsirkin @ 2013-10-14  8:59 UTC (permalink / raw)
  To: Marc Zyngier; +Cc: Rusty Russell, Pawel Moll, kvmarm, linux-arm-kernel, kvm

On Mon, Oct 14, 2013 at 09:24:32AM +0100, Marc Zyngier wrote:
> Hi Michael,
> 
> On 12/10/13 19:28, Michael S. Tsirkin wrote:
> > On Fri, Oct 11, 2013 at 03:36:08PM +0100, Marc Zyngier wrote:
> >> This small patch series adds just enough kernel infrastructure and
> >> fixes to allow a BE guest to use virtio-mmio on a LE host, provided
> >> that the host actually supports such madness.
> >>
> >> This has been tested on arm64, with some fixes to KVM and a set of
> >> changes to kvmtool, 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: Rusty Russell <rusty@rustcorp.com.au>
> >> Cc: Michael S. Tsirkin <mst@redhat.com>
> >> Cc: Pawel Moll <pawel.moll@arm.com>
> > 
> > We are changing the spec to make everything LE instead of
> > the native endian.
> > 
> > I think that'll fix the issue in a cleaner way.
> 
> While I agree that it would solve the issue completely, it would also
> break all BE users. Is that really an option?

I proposed several ways to create "transitional devices"
which can detect and switch to old interface at run-time.

Pawel thinks that's not necessary so ...


> The whole goal of this series is to implement something that gracefully
> falls back to what we have today, not breaking anything in the process.
> 
> Cheers,
> 
> 	M.
> -- 
> Jazz is not dead. It just smells funny...

^ permalink raw reply	[flat|nested] 86+ messages in thread
* [PATCH 0/3] virtio-mmio: handle BE guests on LE hosts
@ 2013-10-14  8:59       ` Michael S. Tsirkin
  0 siblings, 0 replies; 86+ messages in thread
From: Michael S. Tsirkin @ 2013-10-14  8:59 UTC (permalink / raw)
  To: linux-arm-kernel

On Mon, Oct 14, 2013 at 09:24:32AM +0100, Marc Zyngier wrote:
> Hi Michael,
> 
> On 12/10/13 19:28, Michael S. Tsirkin wrote:
> > On Fri, Oct 11, 2013 at 03:36:08PM +0100, Marc Zyngier wrote:
> >> This small patch series adds just enough kernel infrastructure and
> >> fixes to allow a BE guest to use virtio-mmio on a LE host, provided
> >> that the host actually supports such madness.
> >>
> >> This has been tested on arm64, with some fixes to KVM and a set of
> >> changes to kvmtool, 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: Rusty Russell <rusty@rustcorp.com.au>
> >> Cc: Michael S. Tsirkin <mst@redhat.com>
> >> Cc: Pawel Moll <pawel.moll@arm.com>
> > 
> > We are changing the spec to make everything LE instead of
> > the native endian.
> > 
> > I think that'll fix the issue in a cleaner way.
> 
> While I agree that it would solve the issue completely, it would also
> break all BE users. Is that really an option?

I proposed several ways to create "transitional devices"
which can detect and switch to old interface at run-time.

Pawel thinks that's not necessary so ...


> The whole goal of this series is to implement something that gracefully
> falls back to what we have today, not breaking anything in the process.
> 
> Cheers,
> 
> 	M.
> -- 
> Jazz is not dead. It just smells funny...

^ permalink raw reply	[flat|nested] 86+ messages in thread
* Re: [PATCH 0/3] virtio-mmio: handle BE guests on LE hosts
  2013-10-14  8:59       ` Michael S. Tsirkin
@ 2013-10-14  9:04         ` Pawel Moll
  -1 siblings, 0 replies; 86+ messages in thread
From: Pawel Moll @ 2013-10-14  9:04 UTC (permalink / raw)
  To: Michael S. Tsirkin
  Cc: Marc Zyngier, linux-arm-kernel, kvmarm, kvm, Rusty Russell

On Mon, 2013-10-14 at 09:59 +0100, Michael S. Tsirkin wrote:
> On Mon, Oct 14, 2013 at 09:24:32AM +0100, Marc Zyngier wrote:
> > Hi Michael,
> > 
> > On 12/10/13 19:28, Michael S. Tsirkin wrote:
> > > On Fri, Oct 11, 2013 at 03:36:08PM +0100, Marc Zyngier wrote:
> > >> This small patch series adds just enough kernel infrastructure and
> > >> fixes to allow a BE guest to use virtio-mmio on a LE host, provided
> > >> that the host actually supports such madness.
> > >>
> > >> This has been tested on arm64, with some fixes to KVM and a set of
> > >> changes to kvmtool, 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: Rusty Russell <rusty@rustcorp.com.au>
> > >> Cc: Michael S. Tsirkin <mst@redhat.com>
> > >> Cc: Pawel Moll <pawel.moll@arm.com>
> > > 
> > > We are changing the spec to make everything LE instead of
> > > the native endian.
> > > 
> > > I think that'll fix the issue in a cleaner way.
> > 
> > While I agree that it would solve the issue completely, it would also
> > break all BE users. Is that really an option?
> 
> I proposed several ways to create "transitional devices"
> which can detect and switch to old interface at run-time.
> 
> Pawel thinks that's not necessary so ...

Don't wipe yourself with my name, please.

You forgot to mention that the devices are versioned and the behavior of
the legacy devices remains unchanged. No existing implementation will be
broken.

Paweł



^ permalink raw reply	[flat|nested] 86+ messages in thread
* [PATCH 0/3] virtio-mmio: handle BE guests on LE hosts
@ 2013-10-14  9:04         ` Pawel Moll
  0 siblings, 0 replies; 86+ messages in thread
From: Pawel Moll @ 2013-10-14  9:04 UTC (permalink / raw)
  To: linux-arm-kernel

On Mon, 2013-10-14 at 09:59 +0100, Michael S. Tsirkin wrote:
> On Mon, Oct 14, 2013 at 09:24:32AM +0100, Marc Zyngier wrote:
> > Hi Michael,
> > 
> > On 12/10/13 19:28, Michael S. Tsirkin wrote:
> > > On Fri, Oct 11, 2013 at 03:36:08PM +0100, Marc Zyngier wrote:
> > >> This small patch series adds just enough kernel infrastructure and
> > >> fixes to allow a BE guest to use virtio-mmio on a LE host, provided
> > >> that the host actually supports such madness.
> > >>
> > >> This has been tested on arm64, with some fixes to KVM and a set of
> > >> changes to kvmtool, 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: Rusty Russell <rusty@rustcorp.com.au>
> > >> Cc: Michael S. Tsirkin <mst@redhat.com>
> > >> Cc: Pawel Moll <pawel.moll@arm.com>
> > > 
> > > We are changing the spec to make everything LE instead of
> > > the native endian.
> > > 
> > > I think that'll fix the issue in a cleaner way.
> > 
> > While I agree that it would solve the issue completely, it would also
> > break all BE users. Is that really an option?
> 
> I proposed several ways to create "transitional devices"
> which can detect and switch to old interface at run-time.
> 
> Pawel thinks that's not necessary so ...

Don't wipe yourself with my name, please.

You forgot to mention that the devices are versioned and the behavior of
the legacy devices remains unchanged. No existing implementation will be
broken.

Pawe?

^ permalink raw reply	[flat|nested] 86+ messages in thread
* Re: [PATCH 2/3] virtio: mmio: fix signature checking for BE guests
  2013-10-14  8:46     ` Pawel Moll
@ 2013-10-14  9:11       ` Rusty Russell
  -1 siblings, 0 replies; 86+ messages in thread
From: Rusty Russell @ 2013-10-14  9:11 UTC (permalink / raw)
  To: Pawel Moll, Marc Zyngier
  Cc: linux-arm-kernel, kvmarm, kvm, Michael S. Tsirkin

Pawel Moll <pawel.moll@arm.com> writes:
> On Fri, 2013-10-11 at 15:36 +0100, Marc Zyngier wrote:
>> As virtio-mmio config registers are specified to be little-endian,
>> using readl() to read the magic value and then memcmp() to check it
>> fails on BE (as readl() has an implicit swab).
>> 
>> Fix it by encoding the magic value as an integer instead of a string.
>> 
>> Cc: Rusty Russell <rusty@rustcorp.com.au>
>> Cc: Michael S. Tsirkin <mst@redhat.com>
>> Cc: Pawel Moll <pawel.moll@arm.com>
>> Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
>> ---
>>  drivers/virtio/virtio_mmio.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>> 
>> diff --git a/drivers/virtio/virtio_mmio.c b/drivers/virtio/virtio_mmio.c
>> index 1ba0d68..57f24fd 100644
>> --- a/drivers/virtio/virtio_mmio.c
>> +++ b/drivers/virtio/virtio_mmio.c
>> @@ -470,7 +470,7 @@ static int virtio_mmio_probe(struct platform_device *pdev)
>>  
>>  	/* Check magic value */
>>  	magic = readl(vm_dev->base + VIRTIO_MMIO_MAGIC_VALUE);
>> -	if (memcmp(&magic, "virt", 4) != 0) {
>> +	if (magic != ('v' | 'i' << 8 | 'r' << 16 | 't' << 24)) {
>>  		dev_warn(&pdev->dev, "Wrong magic value 0x%08lx!\n", magic);
>>  		return -ENODEV;
>>  	}
>
> The new spec will clarify this:
>
> * 0x000 | R | MagicValue
>   Magic value. Must be 0x74726976 (a Little Endian equivalent
>   of a "virt" string).
>
> ... but I like the 'v'i'r't' characters still being there :-)
>
> Acked-by: Pawel Moll <pawel.moll@arm.com>

Applied, thanks.

Cheers,
Rusty.

^ permalink raw reply	[flat|nested] 86+ messages in thread
* [PATCH 2/3] virtio: mmio: fix signature checking for BE guests
@ 2013-10-14  9:11       ` Rusty Russell
  0 siblings, 0 replies; 86+ messages in thread
From: Rusty Russell @ 2013-10-14  9:11 UTC (permalink / raw)
  To: linux-arm-kernel

Pawel Moll <pawel.moll@arm.com> writes:
> On Fri, 2013-10-11 at 15:36 +0100, Marc Zyngier wrote:
>> As virtio-mmio config registers are specified to be little-endian,
>> using readl() to read the magic value and then memcmp() to check it
>> fails on BE (as readl() has an implicit swab).
>> 
>> Fix it by encoding the magic value as an integer instead of a string.
>> 
>> Cc: Rusty Russell <rusty@rustcorp.com.au>
>> Cc: Michael S. Tsirkin <mst@redhat.com>
>> Cc: Pawel Moll <pawel.moll@arm.com>
>> Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
>> ---
>>  drivers/virtio/virtio_mmio.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>> 
>> diff --git a/drivers/virtio/virtio_mmio.c b/drivers/virtio/virtio_mmio.c
>> index 1ba0d68..57f24fd 100644
>> --- a/drivers/virtio/virtio_mmio.c
>> +++ b/drivers/virtio/virtio_mmio.c
>> @@ -470,7 +470,7 @@ static int virtio_mmio_probe(struct platform_device *pdev)
>>  
>>  	/* Check magic value */
>>  	magic = readl(vm_dev->base + VIRTIO_MMIO_MAGIC_VALUE);
>> -	if (memcmp(&magic, "virt", 4) != 0) {
>> +	if (magic != ('v' | 'i' << 8 | 'r' << 16 | 't' << 24)) {
>>  		dev_warn(&pdev->dev, "Wrong magic value 0x%08lx!\n", magic);
>>  		return -ENODEV;
>>  	}
>
> The new spec will clarify this:
>
> * 0x000 | R | MagicValue
>   Magic value. Must be 0x74726976 (a Little Endian equivalent
>   of a "virt" string).
>
> ... but I like the 'v'i'r't' characters still being there :-)
>
> Acked-by: Pawel Moll <pawel.moll@arm.com>

Applied, thanks.

Cheers,
Rusty.

^ permalink raw reply	[flat|nested] 86+ messages in thread
* Re: [PATCH 0/3] virtio-mmio: handle BE guests on LE hosts
  2013-10-14  8:24     ` Marc Zyngier
@ 2013-10-14  9:13       ` Rusty Russell
  -1 siblings, 0 replies; 86+ messages in thread
From: Rusty Russell @ 2013-10-14  9:13 UTC (permalink / raw)
  To: Marc Zyngier, Michael S. Tsirkin
  Cc: linux-arm-kernel, kvmarm, kvm, Pawel Moll

Marc Zyngier <marc.zyngier@arm.com> writes:
> Hi Michael,
>
> On 12/10/13 19:28, Michael S. Tsirkin wrote:
>> On Fri, Oct 11, 2013 at 03:36:08PM +0100, Marc Zyngier wrote:
>>> This small patch series adds just enough kernel infrastructure and
>>> fixes to allow a BE guest to use virtio-mmio on a LE host, provided
>>> that the host actually supports such madness.
>>>
>>> This has been tested on arm64, with some fixes to KVM and a set of
>>> changes to kvmtool, 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: Rusty Russell <rusty@rustcorp.com.au>
>>> Cc: Michael S. Tsirkin <mst@redhat.com>
>>> Cc: Pawel Moll <pawel.moll@arm.com>
>> 
>> We are changing the spec to make everything LE instead of
>> the native endian.
>> 
>> I think that'll fix the issue in a cleaner way.
>
> While I agree that it would solve the issue completely, it would also
> break all BE users. Is that really an option?
>
> The whole goal of this series is to implement something that gracefully
> falls back to what we have today, not breaking anything in the process.

Pawel was in favor of incrementing the version number.

OTOH, clearly there are no big endian users at the moment, since they
fail the magic tests...

Cheers,
Rusty.

^ permalink raw reply	[flat|nested] 86+ messages in thread
* [PATCH 0/3] virtio-mmio: handle BE guests on LE hosts
@ 2013-10-14  9:13       ` Rusty Russell
  0 siblings, 0 replies; 86+ messages in thread
From: Rusty Russell @ 2013-10-14  9:13 UTC (permalink / raw)
  To: linux-arm-kernel

Marc Zyngier <marc.zyngier@arm.com> writes:
> Hi Michael,
>
> On 12/10/13 19:28, Michael S. Tsirkin wrote:
>> On Fri, Oct 11, 2013 at 03:36:08PM +0100, Marc Zyngier wrote:
>>> This small patch series adds just enough kernel infrastructure and
>>> fixes to allow a BE guest to use virtio-mmio on a LE host, provided
>>> that the host actually supports such madness.
>>>
>>> This has been tested on arm64, with some fixes to KVM and a set of
>>> changes to kvmtool, 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: Rusty Russell <rusty@rustcorp.com.au>
>>> Cc: Michael S. Tsirkin <mst@redhat.com>
>>> Cc: Pawel Moll <pawel.moll@arm.com>
>> 
>> We are changing the spec to make everything LE instead of
>> the native endian.
>> 
>> I think that'll fix the issue in a cleaner way.
>
> While I agree that it would solve the issue completely, it would also
> break all BE users. Is that really an option?
>
> The whole goal of this series is to implement something that gracefully
> falls back to what we have today, not breaking anything in the process.

Pawel was in favor of incrementing the version number.

OTOH, clearly there are no big endian users at the moment, since they
fail the magic tests...

Cheers,
Rusty.

^ permalink raw reply	[flat|nested] 86+ messages in thread
* Re: [PATCH 0/3] virtio-mmio: handle BE guests on LE hosts
  2013-10-14  9:04         ` Pawel Moll
@ 2013-10-14 10:46           ` Michael S. Tsirkin
  -1 siblings, 0 replies; 86+ messages in thread
From: Michael S. Tsirkin @ 2013-10-14 10:46 UTC (permalink / raw)
  To: Pawel Moll; +Cc: Marc Zyngier, Rusty Russell, kvmarm, linux-arm-kernel, kvm

On Mon, Oct 14, 2013 at 10:04:55AM +0100, Pawel Moll wrote:
> On Mon, 2013-10-14 at 09:59 +0100, Michael S. Tsirkin wrote:
> > On Mon, Oct 14, 2013 at 09:24:32AM +0100, Marc Zyngier wrote:
> > > Hi Michael,
> > > 
> > > On 12/10/13 19:28, Michael S. Tsirkin wrote:
> > > > On Fri, Oct 11, 2013 at 03:36:08PM +0100, Marc Zyngier wrote:
> > > >> This small patch series adds just enough kernel infrastructure and
> > > >> fixes to allow a BE guest to use virtio-mmio on a LE host, provided
> > > >> that the host actually supports such madness.
> > > >>
> > > >> This has been tested on arm64, with some fixes to KVM and a set of
> > > >> changes to kvmtool, 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: Rusty Russell <rusty@rustcorp.com.au>
> > > >> Cc: Michael S. Tsirkin <mst@redhat.com>
> > > >> Cc: Pawel Moll <pawel.moll@arm.com>
> > > > 
> > > > We are changing the spec to make everything LE instead of
> > > > the native endian.
> > > > 
> > > > I think that'll fix the issue in a cleaner way.
> > > 
> > > While I agree that it would solve the issue completely, it would also
> > > break all BE users. Is that really an option?
> > 
> > I proposed several ways to create "transitional devices"
> > which can detect and switch to old interface at run-time.
> > 
> > Pawel thinks that's not necessary so ...
> 
> Don't wipe yourself with my name, please.
> 
> You forgot to mention that the devices are versioned and the behavior of
> the legacy devices remains unchanged. No existing implementation will be
> broken.
> 
> Paweł
> 

I'm sorry if what I wrote was misleading.

What I meant is that under the proposed scheme, users with
existing v1 drivers must configure a v1 device explicitly.

According to the plan, drivers will be updated so they can
work with both v1 devices and new v2 devices.

But if you configure a v2 device, old drivers
will not work.

-- 
MST

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 86+ messages in thread
* [PATCH 0/3] virtio-mmio: handle BE guests on LE hosts
@ 2013-10-14 10:46           ` Michael S. Tsirkin
  0 siblings, 0 replies; 86+ messages in thread
From: Michael S. Tsirkin @ 2013-10-14 10:46 UTC (permalink / raw)
  To: linux-arm-kernel

On Mon, Oct 14, 2013 at 10:04:55AM +0100, Pawel Moll wrote:
> On Mon, 2013-10-14 at 09:59 +0100, Michael S. Tsirkin wrote:
> > On Mon, Oct 14, 2013 at 09:24:32AM +0100, Marc Zyngier wrote:
> > > Hi Michael,
> > > 
> > > On 12/10/13 19:28, Michael S. Tsirkin wrote:
> > > > On Fri, Oct 11, 2013 at 03:36:08PM +0100, Marc Zyngier wrote:
> > > >> This small patch series adds just enough kernel infrastructure and
> > > >> fixes to allow a BE guest to use virtio-mmio on a LE host, provided
> > > >> that the host actually supports such madness.
> > > >>
> > > >> This has been tested on arm64, with some fixes to KVM and a set of
> > > >> changes to kvmtool, 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: Rusty Russell <rusty@rustcorp.com.au>
> > > >> Cc: Michael S. Tsirkin <mst@redhat.com>
> > > >> Cc: Pawel Moll <pawel.moll@arm.com>
> > > > 
> > > > We are changing the spec to make everything LE instead of
> > > > the native endian.
> > > > 
> > > > I think that'll fix the issue in a cleaner way.
> > > 
> > > While I agree that it would solve the issue completely, it would also
> > > break all BE users. Is that really an option?
> > 
> > I proposed several ways to create "transitional devices"
> > which can detect and switch to old interface at run-time.
> > 
> > Pawel thinks that's not necessary so ...
> 
> Don't wipe yourself with my name, please.
> 
> You forgot to mention that the devices are versioned and the behavior of
> the legacy devices remains unchanged. No existing implementation will be
> broken.
> 
> Pawe?
> 

I'm sorry if what I wrote was misleading.

What I meant is that under the proposed scheme, users with
existing v1 drivers must configure a v1 device explicitly.

According to the plan, drivers will be updated so they can
work with both v1 devices and new v2 devices.

But if you configure a v2 device, old drivers
will not work.

-- 
MST

^ permalink raw reply	[flat|nested] 86+ messages in thread
* Re: [PATCH 0/3] virtio-mmio: handle BE guests on LE hosts
  2013-10-14 10:46           ` Michael S. Tsirkin
@ 2013-10-14 10:50             ` Pawel Moll
  -1 siblings, 0 replies; 86+ messages in thread
From: Pawel Moll @ 2013-10-14 10:50 UTC (permalink / raw)
  To: Michael S. Tsirkin
  Cc: Marc Zyngier, linux-arm-kernel, kvmarm, kvm, Rusty Russell

On Mon, 2013-10-14 at 11:46 +0100, Michael S. Tsirkin wrote:
> What I meant is that under the proposed scheme, users with
> existing v1 drivers must configure a v1 device explicitly.
> 
> According to the plan, drivers will be updated so they can
> work with both v1 devices and new v2 devices.
> 
> But if you configure a v2 device, old drivers
> will not work.

That's correct, yes. If qemu 2015(16? 17?).01 will provide v2 devices
only, kernel 3.11 is not going to recognize any of them.

What will happen in reality, though, is that the implementation of the
Linux v1/v2 driver will pre-date *any* v2 device by far. First
implementation of v1 devices appeared almost 2 years after the driver
appeared. It will take year for people to upgrade to v2, and probably
only after they experience problems with page-based addressing mode.

I think I can live with this.

Paweł



^ permalink raw reply	[flat|nested] 86+ messages in thread
* [PATCH 0/3] virtio-mmio: handle BE guests on LE hosts
@ 2013-10-14 10:50             ` Pawel Moll
  0 siblings, 0 replies; 86+ messages in thread
From: Pawel Moll @ 2013-10-14 10:50 UTC (permalink / raw)
  To: linux-arm-kernel

On Mon, 2013-10-14 at 11:46 +0100, Michael S. Tsirkin wrote:
> What I meant is that under the proposed scheme, users with
> existing v1 drivers must configure a v1 device explicitly.
> 
> According to the plan, drivers will be updated so they can
> work with both v1 devices and new v2 devices.
> 
> But if you configure a v2 device, old drivers
> will not work.

That's correct, yes. If qemu 2015(16? 17?).01 will provide v2 devices
only, kernel 3.11 is not going to recognize any of them.

What will happen in reality, though, is that the implementation of the
Linux v1/v2 driver will pre-date *any* v2 device by far. First
implementation of v1 devices appeared almost 2 years after the driver
appeared. It will take year for people to upgrade to v2, and probably
only after they experience problems with page-based addressing mode.

I think I can live with this.

Pawe?

^ permalink raw reply	[flat|nested] 86+ messages in thread
* Re: [PATCH 0/3] virtio-mmio: handle BE guests on LE hosts
  2013-10-14  8:21   ` Rusty Russell
@ 2013-10-14 12:36     ` Marc Zyngier
  -1 siblings, 0 replies; 86+ messages in thread
From: Marc Zyngier @ 2013-10-14 12:36 UTC (permalink / raw)
  To: Rusty Russell
  Cc: linux-arm-kernel, kvmarm, kvm, Michael S. Tsirkin, Pawel Moll

On 14/10/13 09:21, Rusty Russell wrote:
> Marc Zyngier <marc.zyngier@arm.com> writes:
>> This small patch series adds just enough kernel infrastructure and
>> fixes to allow a BE guest to use virtio-mmio on a LE host, provided
>> that the host actually supports such madness.
>>
>> This has been tested on arm64, with some fixes to KVM and a set of
>> changes to kvmtool, both which I am posting separately.
> 
> OK, so I already have a patch which supports config space accessors.
> I've posted the series below, and since you want it, I'll put it in
> virtio-next after more testing and rebasing...

Yes, that's definitely something I'd like to see being merged, as it
would allow me to drop a significant chunk of changes.

> But we won't be using feature bits for endianness, since we're defining
> endian in the 1.0 spec (whether LE-everywhere or LE-for-mmio-and-PCI is
> still debated).  Since we need to guess for backwards compat anyway,
> let's keep doing this until v1.0?

When is 1.0 going to happen? When will actual implementation of drivers
and devices show up in my favourite platform emulation?

Having a grand plan for the future is great, but I need something
working right now, or at least fairly soonish... And I need it to be
backward compatible, as none of the above is going to show up overnight.

> (Yeah, I made this mess with "native endian".  I promise I have learnt
> my lesson).

Paracetamol bill coming you way... ;-)

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


^ permalink raw reply	[flat|nested] 86+ messages in thread
* [PATCH 0/3] virtio-mmio: handle BE guests on LE hosts
@ 2013-10-14 12:36     ` Marc Zyngier
  0 siblings, 0 replies; 86+ messages in thread
From: Marc Zyngier @ 2013-10-14 12:36 UTC (permalink / raw)
  To: linux-arm-kernel

On 14/10/13 09:21, Rusty Russell wrote:
> Marc Zyngier <marc.zyngier@arm.com> writes:
>> This small patch series adds just enough kernel infrastructure and
>> fixes to allow a BE guest to use virtio-mmio on a LE host, provided
>> that the host actually supports such madness.
>>
>> This has been tested on arm64, with some fixes to KVM and a set of
>> changes to kvmtool, both which I am posting separately.
> 
> OK, so I already have a patch which supports config space accessors.
> I've posted the series below, and since you want it, I'll put it in
> virtio-next after more testing and rebasing...

Yes, that's definitely something I'd like to see being merged, as it
would allow me to drop a significant chunk of changes.

> But we won't be using feature bits for endianness, since we're defining
> endian in the 1.0 spec (whether LE-everywhere or LE-for-mmio-and-PCI is
> still debated).  Since we need to guess for backwards compat anyway,
> let's keep doing this until v1.0?

When is 1.0 going to happen? When will actual implementation of drivers
and devices show up in my favourite platform emulation?

Having a grand plan for the future is great, but I need something
working right now, or at least fairly soonish... And I need it to be
backward compatible, as none of the above is going to show up overnight.

> (Yeah, I made this mess with "native endian".  I promise I have learnt
> my lesson).

Paracetamol bill coming you way... ;-)

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

^ permalink raw reply	[flat|nested] 86+ messages in thread
* Re: [PATCH 0/3] virtio-mmio: handle BE guests on LE hosts
  2013-10-14 12:36     ` Marc Zyngier
@ 2013-10-14 12:51       ` Michael S. Tsirkin
  -1 siblings, 0 replies; 86+ messages in thread
From: Michael S. Tsirkin @ 2013-10-14 12:51 UTC (permalink / raw)
  To: Marc Zyngier; +Cc: Rusty Russell, linux-arm-kernel, kvmarm, kvm, Pawel Moll

On Mon, Oct 14, 2013 at 01:36:12PM +0100, Marc Zyngier wrote:
> On 14/10/13 09:21, Rusty Russell wrote:
> > Marc Zyngier <marc.zyngier@arm.com> writes:
> >> This small patch series adds just enough kernel infrastructure and
> >> fixes to allow a BE guest to use virtio-mmio on a LE host, provided
> >> that the host actually supports such madness.
> >>
> >> This has been tested on arm64, with some fixes to KVM and a set of
> >> changes to kvmtool, both which I am posting separately.
> > 
> > OK, so I already have a patch which supports config space accessors.
> > I've posted the series below, and since you want it, I'll put it in
> > virtio-next after more testing and rebasing...
> 
> Yes, that's definitely something I'd like to see being merged, as it
> would allow me to drop a significant chunk of changes.
> 
> > But we won't be using feature bits for endianness, since we're defining
> > endian in the 1.0 spec (whether LE-everywhere or LE-for-mmio-and-PCI is
> > still debated).  Since we need to guess for backwards compat anyway,
> > let's keep doing this until v1.0?
> 
> When is 1.0 going to happen? When will actual implementation of drivers
> and devices show up in my favourite platform emulation?
> 
> Having a grand plan for the future is great, but I need something
> working right now, or at least fairly soonish... And I need it to be
> backward compatible, as none of the above is going to show up overnight.
> > (Yeah, I made this mess with "native endian".  I promise I have learnt
> > my lesson).
> 
> Paracetamol bill coming you way... ;-)
> 
> 	M.
> -- 
> Jazz is not dead. It just smells funny...


You can help by reviewing the proposals here:

mmio specific parts:
https://www.oasis-open.org/apps/org/workgroup/virtio/email/archives/201310/msg00123.html

generic changes:
http://markmail.org/search/?q=subject%3A%22%5BPATCH%5D%20configuration%20space%20endian-ness%22+list%3A%22org.oasis-open.lists.virtio%22

http://markmail.org/search/?q=subject%3A%22%5BPATCH%5D%20virtqueue%20endian-ness%22+list%3A%22org.oasis-open.lists.virtio%22

These are incremental patches against the current draft -
reviewing it would also be helpful:

https://tools.oasis-open.org/version-control/svn/virtio

We discuss virtio implementation issues at
virtio-dev@lists.oasis-open.org

-- 
MST

^ permalink raw reply	[flat|nested] 86+ messages in thread
* [PATCH 0/3] virtio-mmio: handle BE guests on LE hosts
@ 2013-10-14 12:51       ` Michael S. Tsirkin
  0 siblings, 0 replies; 86+ messages in thread
From: Michael S. Tsirkin @ 2013-10-14 12:51 UTC (permalink / raw)
  To: linux-arm-kernel

On Mon, Oct 14, 2013 at 01:36:12PM +0100, Marc Zyngier wrote:
> On 14/10/13 09:21, Rusty Russell wrote:
> > Marc Zyngier <marc.zyngier@arm.com> writes:
> >> This small patch series adds just enough kernel infrastructure and
> >> fixes to allow a BE guest to use virtio-mmio on a LE host, provided
> >> that the host actually supports such madness.
> >>
> >> This has been tested on arm64, with some fixes to KVM and a set of
> >> changes to kvmtool, both which I am posting separately.
> > 
> > OK, so I already have a patch which supports config space accessors.
> > I've posted the series below, and since you want it, I'll put it in
> > virtio-next after more testing and rebasing...
> 
> Yes, that's definitely something I'd like to see being merged, as it
> would allow me to drop a significant chunk of changes.
> 
> > But we won't be using feature bits for endianness, since we're defining
> > endian in the 1.0 spec (whether LE-everywhere or LE-for-mmio-and-PCI is
> > still debated).  Since we need to guess for backwards compat anyway,
> > let's keep doing this until v1.0?
> 
> When is 1.0 going to happen? When will actual implementation of drivers
> and devices show up in my favourite platform emulation?
> 
> Having a grand plan for the future is great, but I need something
> working right now, or at least fairly soonish... And I need it to be
> backward compatible, as none of the above is going to show up overnight.
> > (Yeah, I made this mess with "native endian".  I promise I have learnt
> > my lesson).
> 
> Paracetamol bill coming you way... ;-)
> 
> 	M.
> -- 
> Jazz is not dead. It just smells funny...


You can help by reviewing the proposals here:

mmio specific parts:
https://www.oasis-open.org/apps/org/workgroup/virtio/email/archives/201310/msg00123.html

generic changes:
http://markmail.org/search/?q=subject%3A%22%5BPATCH%5D%20configuration%20space%20endian-ness%22+list%3A%22org.oasis-open.lists.virtio%22

http://markmail.org/search/?q=subject%3A%22%5BPATCH%5D%20virtqueue%20endian-ness%22+list%3A%22org.oasis-open.lists.virtio%22

These are incremental patches against the current draft -
reviewing it would also be helpful:

https://tools.oasis-open.org/version-control/svn/virtio

We discuss virtio implementation issues at
virtio-dev at lists.oasis-open.org

-- 
MST

^ permalink raw reply	[flat|nested] 86+ messages in thread
* Re: [PATCH 0/3] virtio-mmio: handle BE guests on LE hosts
  2013-10-11 14:36 ` Marc Zyngier
@ 2013-10-14 13:03   ` Paolo Bonzini
  -1 siblings, 0 replies; 86+ messages in thread
From: Paolo Bonzini @ 2013-10-14 13:03 UTC (permalink / raw)
  To: Marc Zyngier
  Cc: linux-arm-kernel, kvmarm, kvm, Rusty Russell, Michael S. Tsirkin,
	Pawel Moll

Il 11/10/2013 16:36, Marc Zyngier ha scritto:
> This small patch series adds just enough kernel infrastructure and
> fixes to allow a BE guest to use virtio-mmio on a LE host, provided
> that the host actually supports such madness.

More precisely, it allows the guest drivers to pick the endianness they
prefer.  Mixed-endian virtio works fine on QEMU with e.g. a mips guest
in emulation mode, because then any given QEMU binary will always use
the same endianness (e.g. big for qemu-system-mips).

As far as I understand, patches 2 and 3 are really separate bugfixes,
and they should go in irrespective of patch 1.

Paolo

> This has been tested on arm64, with some fixes to KVM and a set of
> changes to kvmtool, 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: Rusty Russell <rusty@rustcorp.com.au>
> Cc: Michael S. Tsirkin <mst@redhat.com>
> Cc: Pawel Moll <pawel.moll@arm.com>


^ permalink raw reply	[flat|nested] 86+ messages in thread
* [PATCH 0/3] virtio-mmio: handle BE guests on LE hosts
@ 2013-10-14 13:03   ` Paolo Bonzini
  0 siblings, 0 replies; 86+ messages in thread
From: Paolo Bonzini @ 2013-10-14 13:03 UTC (permalink / raw)
  To: linux-arm-kernel

Il 11/10/2013 16:36, Marc Zyngier ha scritto:
> This small patch series adds just enough kernel infrastructure and
> fixes to allow a BE guest to use virtio-mmio on a LE host, provided
> that the host actually supports such madness.

More precisely, it allows the guest drivers to pick the endianness they
prefer.  Mixed-endian virtio works fine on QEMU with e.g. a mips guest
in emulation mode, because then any given QEMU binary will always use
the same endianness (e.g. big for qemu-system-mips).

As far as I understand, patches 2 and 3 are really separate bugfixes,
and they should go in irrespective of patch 1.

Paolo

> This has been tested on arm64, with some fixes to KVM and a set of
> changes to kvmtool, 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: Rusty Russell <rusty@rustcorp.com.au>
> Cc: Michael S. Tsirkin <mst@redhat.com>
> Cc: Pawel Moll <pawel.moll@arm.com>

^ permalink raw reply	[flat|nested] 86+ messages in thread
* Re: [PATCH 0/3] virtio-mmio: handle BE guests on LE hosts
  2013-10-14 13:03   ` Paolo Bonzini
@ 2013-10-14 13:10     ` Alexander Graf
  -1 siblings, 0 replies; 86+ messages in thread
From: Alexander Graf @ 2013-10-14 13:10 UTC (permalink / raw)
  To: Paolo Bonzini
  Cc: Marc Zyngier, kvm@vger.kernel.org mailing list, Pawel Moll,
	Michael S. Tsirkin, kvmarm, linux-arm-kernel


On 14.10.2013, at 15:03, Paolo Bonzini <pbonzini@redhat.com> wrote:

> Il 11/10/2013 16:36, Marc Zyngier ha scritto:
>> This small patch series adds just enough kernel infrastructure and
>> fixes to allow a BE guest to use virtio-mmio on a LE host, provided
>> that the host actually supports such madness.
> 
> More precisely, it allows the guest drivers to pick the endianness they
> prefer.  Mixed-endian virtio works fine on QEMU with e.g. a mips guest
> in emulation mode, because then any given QEMU binary will always use
> the same endianness (e.g. big for qemu-system-mips).

We have the same problem (runtime switchable endianness) on PowerPC. IBM POWER is gaining Little Endian support in Linux now, so we could easily end up with an LE guest on a BE host.

IIRC the way we're going to solve this is to hack up virtio_is_big_endian() to evaluate the first CPU's endianness mode (which will always be the same as all other CPU's endianness mode due to hypercall restrictions).

You could do the same with ARM if you need to support guest kernels before the new virtio stuff is there. LE POWER is definitely already reality upstream.


Alex


^ permalink raw reply	[flat|nested] 86+ messages in thread
* [PATCH 0/3] virtio-mmio: handle BE guests on LE hosts
@ 2013-10-14 13:10     ` Alexander Graf
  0 siblings, 0 replies; 86+ messages in thread
From: Alexander Graf @ 2013-10-14 13:10 UTC (permalink / raw)
  To: linux-arm-kernel


On 14.10.2013, at 15:03, Paolo Bonzini <pbonzini@redhat.com> wrote:

> Il 11/10/2013 16:36, Marc Zyngier ha scritto:
>> This small patch series adds just enough kernel infrastructure and
>> fixes to allow a BE guest to use virtio-mmio on a LE host, provided
>> that the host actually supports such madness.
> 
> More precisely, it allows the guest drivers to pick the endianness they
> prefer.  Mixed-endian virtio works fine on QEMU with e.g. a mips guest
> in emulation mode, because then any given QEMU binary will always use
> the same endianness (e.g. big for qemu-system-mips).

We have the same problem (runtime switchable endianness) on PowerPC. IBM POWER is gaining Little Endian support in Linux now, so we could easily end up with an LE guest on a BE host.

IIRC the way we're going to solve this is to hack up virtio_is_big_endian() to evaluate the first CPU's endianness mode (which will always be the same as all other CPU's endianness mode due to hypercall restrictions).

You could do the same with ARM if you need to support guest kernels before the new virtio stuff is there. LE POWER is definitely already reality upstream.


Alex

^ permalink raw reply	[flat|nested] 86+ messages in thread
* Re: [PATCH 0/3] virtio-mmio: handle BE guests on LE hosts
  2013-10-14 13:10     ` Alexander Graf
@ 2013-10-14 13:13       ` Paolo Bonzini
  -1 siblings, 0 replies; 86+ messages in thread
From: Paolo Bonzini @ 2013-10-14 13:13 UTC (permalink / raw)
  To: Alexander Graf
  Cc: kvm@vger.kernel.org mailing list, Pawel Moll, Michael S. Tsirkin,
	Marc Zyngier, kvmarm, linux-arm-kernel

Il 14/10/2013 15:10, Alexander Graf ha scritto:
>> More precisely, it allows the guest drivers to pick the
>> endianness they prefer.  Mixed-endian virtio works fine on QEMU
>> with e.g. a mips guest in emulation mode, because then any given
>> QEMU binary will always use the same endianness (e.g. big for
>> qemu-system-mips).
> 
> We have the same problem (runtime switchable endianness) on PowerPC.
> IBM POWER is gaining Little Endian support in Linux now, so we could
> easily end up with an LE guest on a BE host.
> 
> IIRC the way we're going to solve this is to hack up
> virtio_is_big_endian() to evaluate the first CPU's endianness mode
> (which will always be the same as all other CPU's endianness mode due
> to hypercall restrictions).
> 
> You could do the same with ARM if you need to support guest kernels
> before the new virtio stuff is there. LE POWER is definitely already
> reality upstream.

And the virtio standard, when it becomes reality, will be LE-only except
for some s390 stuff that I don't claim I understand...

Paolo

^ permalink raw reply	[flat|nested] 86+ messages in thread
* [PATCH 0/3] virtio-mmio: handle BE guests on LE hosts
@ 2013-10-14 13:13       ` Paolo Bonzini
  0 siblings, 0 replies; 86+ messages in thread
From: Paolo Bonzini @ 2013-10-14 13:13 UTC (permalink / raw)
  To: linux-arm-kernel

Il 14/10/2013 15:10, Alexander Graf ha scritto:
>> More precisely, it allows the guest drivers to pick the
>> endianness they prefer.  Mixed-endian virtio works fine on QEMU
>> with e.g. a mips guest in emulation mode, because then any given
>> QEMU binary will always use the same endianness (e.g. big for
>> qemu-system-mips).
> 
> We have the same problem (runtime switchable endianness) on PowerPC.
> IBM POWER is gaining Little Endian support in Linux now, so we could
> easily end up with an LE guest on a BE host.
> 
> IIRC the way we're going to solve this is to hack up
> virtio_is_big_endian() to evaluate the first CPU's endianness mode
> (which will always be the same as all other CPU's endianness mode due
> to hypercall restrictions).
> 
> You could do the same with ARM if you need to support guest kernels
> before the new virtio stuff is there. LE POWER is definitely already
> reality upstream.

And the virtio standard, when it becomes reality, will be LE-only except
for some s390 stuff that I don't claim I understand...

Paolo

^ permalink raw reply	[flat|nested] 86+ messages in thread
* Re: [PATCH 0/3] virtio-mmio: handle BE guests on LE hosts
  2013-10-14 13:10     ` Alexander Graf
@ 2013-10-14 13:24       ` Marc Zyngier
  -1 siblings, 0 replies; 86+ messages in thread
From: Marc Zyngier @ 2013-10-14 13:24 UTC (permalink / raw)
  To: Alexander Graf
  Cc: Paolo Bonzini, kvm@vger.kernel.org mailing list, Pawel Moll,
	Michael S. Tsirkin, kvmarm, linux-arm-kernel

On 14/10/13 14:10, Alexander Graf wrote:
> 
> On 14.10.2013, at 15:03, Paolo Bonzini <pbonzini@redhat.com> wrote:
> 
>> Il 11/10/2013 16:36, Marc Zyngier ha scritto:
>>> This small patch series adds just enough kernel infrastructure and
>>> fixes to allow a BE guest to use virtio-mmio on a LE host, provided
>>> that the host actually supports such madness.
>>
>> More precisely, it allows the guest drivers to pick the endianness they
>> prefer.  Mixed-endian virtio works fine on QEMU with e.g. a mips guest
>> in emulation mode, because then any given QEMU binary will always use
>> the same endianness (e.g. big for qemu-system-mips).
> 
> We have the same problem (runtime switchable endianness) on PowerPC. IBM POWER is gaining Little Endian support in Linux now, so we could easily end up with an LE guest on a BE host.
> 
> IIRC the way we're going to solve this is to hack up virtio_is_big_endian() to evaluate the first CPU's endianness mode (which will always be the same as all other CPU's endianness mode due to hypercall restrictions).

I have implemented something similar for MMIO emulation in KVM/arm
(except that I only care about the faulting CPU).

See my initial patch for that:
https://lists.cs.columbia.edu/pipermail/kvmarm/2013-October/007359.html

That doesn't really change the non-trapping virtio accesses, though.
Where is this virtio_is_big_endian() thing?

> You could do the same with ARM if you need to support guest kernels before the new virtio stuff is there. LE POWER is definitely already reality upstream.

So is BE ARM support...

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


^ permalink raw reply	[flat|nested] 86+ messages in thread
* [PATCH 0/3] virtio-mmio: handle BE guests on LE hosts
@ 2013-10-14 13:24       ` Marc Zyngier
  0 siblings, 0 replies; 86+ messages in thread
From: Marc Zyngier @ 2013-10-14 13:24 UTC (permalink / raw)
  To: linux-arm-kernel

On 14/10/13 14:10, Alexander Graf wrote:
> 
> On 14.10.2013, at 15:03, Paolo Bonzini <pbonzini@redhat.com> wrote:
> 
>> Il 11/10/2013 16:36, Marc Zyngier ha scritto:
>>> This small patch series adds just enough kernel infrastructure and
>>> fixes to allow a BE guest to use virtio-mmio on a LE host, provided
>>> that the host actually supports such madness.
>>
>> More precisely, it allows the guest drivers to pick the endianness they
>> prefer.  Mixed-endian virtio works fine on QEMU with e.g. a mips guest
>> in emulation mode, because then any given QEMU binary will always use
>> the same endianness (e.g. big for qemu-system-mips).
> 
> We have the same problem (runtime switchable endianness) on PowerPC. IBM POWER is gaining Little Endian support in Linux now, so we could easily end up with an LE guest on a BE host.
> 
> IIRC the way we're going to solve this is to hack up virtio_is_big_endian() to evaluate the first CPU's endianness mode (which will always be the same as all other CPU's endianness mode due to hypercall restrictions).

I have implemented something similar for MMIO emulation in KVM/arm
(except that I only care about the faulting CPU).

See my initial patch for that:
https://lists.cs.columbia.edu/pipermail/kvmarm/2013-October/007359.html

That doesn't really change the non-trapping virtio accesses, though.
Where is this virtio_is_big_endian() thing?

> You could do the same with ARM if you need to support guest kernels before the new virtio stuff is there. LE POWER is definitely already reality upstream.

So is BE ARM support...

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

^ permalink raw reply	[flat|nested] 86+ messages in thread
* Re: [PATCH 0/3] virtio-mmio: handle BE guests on LE hosts
  2013-10-14 13:24       ` Marc Zyngier
@ 2013-10-14 13:29         ` Paolo Bonzini
  -1 siblings, 0 replies; 86+ messages in thread
From: Paolo Bonzini @ 2013-10-14 13:29 UTC (permalink / raw)
  To: Marc Zyngier
  Cc: kvm@vger.kernel.org mailing list, Pawel Moll, Michael S. Tsirkin,
	Alexander Graf, kvmarm, linux-arm-kernel

Il 14/10/2013 15:24, Marc Zyngier ha scritto:
> See my initial patch for that:
> https://lists.cs.columbia.edu/pipermail/kvmarm/2013-October/007359.html
> 
> That doesn't really change the non-trapping virtio accesses, though.
> Where is this virtio_is_big_endian() thing?

In QEMU.

Paolo

^ permalink raw reply	[flat|nested] 86+ messages in thread
* [PATCH 0/3] virtio-mmio: handle BE guests on LE hosts
@ 2013-10-14 13:29         ` Paolo Bonzini
  0 siblings, 0 replies; 86+ messages in thread
From: Paolo Bonzini @ 2013-10-14 13:29 UTC (permalink / raw)
  To: linux-arm-kernel

Il 14/10/2013 15:24, Marc Zyngier ha scritto:
> See my initial patch for that:
> https://lists.cs.columbia.edu/pipermail/kvmarm/2013-October/007359.html
> 
> That doesn't really change the non-trapping virtio accesses, though.
> Where is this virtio_is_big_endian() thing?

In QEMU.

Paolo

^ permalink raw reply	[flat|nested] 86+ messages in thread
* Re: [PATCH 0/3] virtio-mmio: handle BE guests on LE hosts
  2013-10-14 13:24       ` Marc Zyngier
@ 2013-10-14 13:39         ` Alexander Graf
  -1 siblings, 0 replies; 86+ messages in thread
From: Alexander Graf @ 2013-10-14 13:39 UTC (permalink / raw)
  To: Marc Zyngier
  Cc: Paolo Bonzini, kvm@vger.kernel.org mailing list, Pawel Moll,
	Michael S. Tsirkin, kvmarm, linux-arm-kernel


On 14.10.2013, at 15:24, Marc Zyngier <marc.zyngier@arm.com> wrote:

> On 14/10/13 14:10, Alexander Graf wrote:
>> 
>> On 14.10.2013, at 15:03, Paolo Bonzini <pbonzini@redhat.com> wrote:
>> 
>>> Il 11/10/2013 16:36, Marc Zyngier ha scritto:
>>>> This small patch series adds just enough kernel infrastructure and
>>>> fixes to allow a BE guest to use virtio-mmio on a LE host, provided
>>>> that the host actually supports such madness.
>>> 
>>> More precisely, it allows the guest drivers to pick the endianness they
>>> prefer.  Mixed-endian virtio works fine on QEMU with e.g. a mips guest
>>> in emulation mode, because then any given QEMU binary will always use
>>> the same endianness (e.g. big for qemu-system-mips).
>> 
>> We have the same problem (runtime switchable endianness) on PowerPC. IBM POWER is gaining Little Endian support in Linux now, so we could easily end up with an LE guest on a BE host.
>> 
>> IIRC the way we're going to solve this is to hack up virtio_is_big_endian() to evaluate the first CPU's endianness mode (which will always be the same as all other CPU's endianness mode due to hypercall restrictions).
> 
> I have implemented something similar for MMIO emulation in KVM/arm
> (except that I only care about the faulting CPU).
> 
> See my initial patch for that:
> https://lists.cs.columbia.edu/pipermail/kvmarm/2013-October/007359.html
> 
> That doesn't really change the non-trapping virtio accesses, though.
> Where is this virtio_is_big_endian() thing?

It's in QEMU's exec.c. It only gets used for config space access that goes through PCI though. Is there any other place where virtio specifies native endianness today?


Alex


^ permalink raw reply	[flat|nested] 86+ messages in thread
* [PATCH 0/3] virtio-mmio: handle BE guests on LE hosts
@ 2013-10-14 13:39         ` Alexander Graf
  0 siblings, 0 replies; 86+ messages in thread
From: Alexander Graf @ 2013-10-14 13:39 UTC (permalink / raw)
  To: linux-arm-kernel


On 14.10.2013, at 15:24, Marc Zyngier <marc.zyngier@arm.com> wrote:

> On 14/10/13 14:10, Alexander Graf wrote:
>> 
>> On 14.10.2013, at 15:03, Paolo Bonzini <pbonzini@redhat.com> wrote:
>> 
>>> Il 11/10/2013 16:36, Marc Zyngier ha scritto:
>>>> This small patch series adds just enough kernel infrastructure and
>>>> fixes to allow a BE guest to use virtio-mmio on a LE host, provided
>>>> that the host actually supports such madness.
>>> 
>>> More precisely, it allows the guest drivers to pick the endianness they
>>> prefer.  Mixed-endian virtio works fine on QEMU with e.g. a mips guest
>>> in emulation mode, because then any given QEMU binary will always use
>>> the same endianness (e.g. big for qemu-system-mips).
>> 
>> We have the same problem (runtime switchable endianness) on PowerPC. IBM POWER is gaining Little Endian support in Linux now, so we could easily end up with an LE guest on a BE host.
>> 
>> IIRC the way we're going to solve this is to hack up virtio_is_big_endian() to evaluate the first CPU's endianness mode (which will always be the same as all other CPU's endianness mode due to hypercall restrictions).
> 
> I have implemented something similar for MMIO emulation in KVM/arm
> (except that I only care about the faulting CPU).
> 
> See my initial patch for that:
> https://lists.cs.columbia.edu/pipermail/kvmarm/2013-October/007359.html
> 
> That doesn't really change the non-trapping virtio accesses, though.
> Where is this virtio_is_big_endian() thing?

It's in QEMU's exec.c. It only gets used for config space access that goes through PCI though. Is there any other place where virtio specifies native endianness today?


Alex

^ permalink raw reply	[flat|nested] 86+ messages in thread
* Re: [PATCH 0/3] virtio-mmio: handle BE guests on LE hosts
  2013-10-14 13:39         ` Alexander Graf
@ 2013-10-14 13:49           ` Marc Zyngier
  -1 siblings, 0 replies; 86+ messages in thread
From: Marc Zyngier @ 2013-10-14 13:49 UTC (permalink / raw)
  To: Alexander Graf
  Cc: Paolo Bonzini, kvm@vger.kernel.org mailing list, Pawel Moll,
	Michael S. Tsirkin, kvmarm, linux-arm-kernel

On 14/10/13 14:39, Alexander Graf wrote:
> 
> On 14.10.2013, at 15:24, Marc Zyngier <marc.zyngier@arm.com> wrote:
> 
>> On 14/10/13 14:10, Alexander Graf wrote:
>>>
>>> On 14.10.2013, at 15:03, Paolo Bonzini <pbonzini@redhat.com> wrote:
>>>
>>>> Il 11/10/2013 16:36, Marc Zyngier ha scritto:
>>>>> This small patch series adds just enough kernel infrastructure and
>>>>> fixes to allow a BE guest to use virtio-mmio on a LE host, provided
>>>>> that the host actually supports such madness.
>>>>
>>>> More precisely, it allows the guest drivers to pick the endianness they
>>>> prefer.  Mixed-endian virtio works fine on QEMU with e.g. a mips guest
>>>> in emulation mode, because then any given QEMU binary will always use
>>>> the same endianness (e.g. big for qemu-system-mips).
>>>
>>> We have the same problem (runtime switchable endianness) on PowerPC. IBM POWER is gaining Little Endian support in Linux now, so we could easily end up with an LE guest on a BE host.
>>>
>>> IIRC the way we're going to solve this is to hack up virtio_is_big_endian() to evaluate the first CPU's endianness mode (which will always be the same as all other CPU's endianness mode due to hypercall restrictions).
>>
>> I have implemented something similar for MMIO emulation in KVM/arm
>> (except that I only care about the faulting CPU).
>>
>> See my initial patch for that:
>> https://lists.cs.columbia.edu/pipermail/kvmarm/2013-October/007359.html
>>
>> That doesn't really change the non-trapping virtio accesses, though.
>> Where is this virtio_is_big_endian() thing?
> 
> It's in QEMU's exec.c. It only gets used for config space access that goes through PCI though. Is there any other place where virtio specifies native endianness today?

That's the main problem. Today's virtio flavour doesn't specify anything
about endianness, and that is what I'm adding. Or rather (as Paolo put
it), the prefered endianness of the virtio driver.

So once (and if) this flags are in place, you always know what you're
dealing with. And because it is virtio-centric, you can implement it in
an architecture independent way.

Also, most of my life revolves around kvmtool. QEMU is hardly on my
radar, these days (for reasons that are neither technical, nor relevant
to this forum). So it is important to me that the solution is platform
emulation agnostic.

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


^ permalink raw reply	[flat|nested] 86+ messages in thread
* [PATCH 0/3] virtio-mmio: handle BE guests on LE hosts
@ 2013-10-14 13:49           ` Marc Zyngier
  0 siblings, 0 replies; 86+ messages in thread
From: Marc Zyngier @ 2013-10-14 13:49 UTC (permalink / raw)
  To: linux-arm-kernel

On 14/10/13 14:39, Alexander Graf wrote:
> 
> On 14.10.2013, at 15:24, Marc Zyngier <marc.zyngier@arm.com> wrote:
> 
>> On 14/10/13 14:10, Alexander Graf wrote:
>>>
>>> On 14.10.2013, at 15:03, Paolo Bonzini <pbonzini@redhat.com> wrote:
>>>
>>>> Il 11/10/2013 16:36, Marc Zyngier ha scritto:
>>>>> This small patch series adds just enough kernel infrastructure and
>>>>> fixes to allow a BE guest to use virtio-mmio on a LE host, provided
>>>>> that the host actually supports such madness.
>>>>
>>>> More precisely, it allows the guest drivers to pick the endianness they
>>>> prefer.  Mixed-endian virtio works fine on QEMU with e.g. a mips guest
>>>> in emulation mode, because then any given QEMU binary will always use
>>>> the same endianness (e.g. big for qemu-system-mips).
>>>
>>> We have the same problem (runtime switchable endianness) on PowerPC. IBM POWER is gaining Little Endian support in Linux now, so we could easily end up with an LE guest on a BE host.
>>>
>>> IIRC the way we're going to solve this is to hack up virtio_is_big_endian() to evaluate the first CPU's endianness mode (which will always be the same as all other CPU's endianness mode due to hypercall restrictions).
>>
>> I have implemented something similar for MMIO emulation in KVM/arm
>> (except that I only care about the faulting CPU).
>>
>> See my initial patch for that:
>> https://lists.cs.columbia.edu/pipermail/kvmarm/2013-October/007359.html
>>
>> That doesn't really change the non-trapping virtio accesses, though.
>> Where is this virtio_is_big_endian() thing?
> 
> It's in QEMU's exec.c. It only gets used for config space access that goes through PCI though. Is there any other place where virtio specifies native endianness today?

That's the main problem. Today's virtio flavour doesn't specify anything
about endianness, and that is what I'm adding. Or rather (as Paolo put
it), the prefered endianness of the virtio driver.

So once (and if) this flags are in place, you always know what you're
dealing with. And because it is virtio-centric, you can implement it in
an architecture independent way.

Also, most of my life revolves around kvmtool. QEMU is hardly on my
radar, these days (for reasons that are neither technical, nor relevant
to this forum). So it is important to me that the solution is platform
emulation agnostic.

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

^ permalink raw reply	[flat|nested] 86+ messages in thread
* Re: [PATCH 0/3] virtio-mmio: handle BE guests on LE hosts
  2013-10-14 13:49           ` Marc Zyngier
@ 2013-10-14 14:05             ` Michael S. Tsirkin
  -1 siblings, 0 replies; 86+ messages in thread
From: Michael S. Tsirkin @ 2013-10-14 14:05 UTC (permalink / raw)
  To: Marc Zyngier
  Cc: Alexander Graf, Paolo Bonzini, kvm@vger.kernel.org mailing list,
	Pawel Moll, kvmarm, linux-arm-kernel

On Mon, Oct 14, 2013 at 02:49:10PM +0100, Marc Zyngier wrote:
> On 14/10/13 14:39, Alexander Graf wrote:
> > 
> > On 14.10.2013, at 15:24, Marc Zyngier <marc.zyngier@arm.com> wrote:
> > 
> >> On 14/10/13 14:10, Alexander Graf wrote:
> >>>
> >>> On 14.10.2013, at 15:03, Paolo Bonzini <pbonzini@redhat.com> wrote:
> >>>
> >>>> Il 11/10/2013 16:36, Marc Zyngier ha scritto:
> >>>>> This small patch series adds just enough kernel infrastructure and
> >>>>> fixes to allow a BE guest to use virtio-mmio on a LE host, provided
> >>>>> that the host actually supports such madness.
> >>>>
> >>>> More precisely, it allows the guest drivers to pick the endianness they
> >>>> prefer.  Mixed-endian virtio works fine on QEMU with e.g. a mips guest
> >>>> in emulation mode, because then any given QEMU binary will always use
> >>>> the same endianness (e.g. big for qemu-system-mips).
> >>>
> >>> We have the same problem (runtime switchable endianness) on PowerPC. IBM POWER is gaining Little Endian support in Linux now, so we could easily end up with an LE guest on a BE host.
> >>>
> >>> IIRC the way we're going to solve this is to hack up virtio_is_big_endian() to evaluate the first CPU's endianness mode (which will always be the same as all other CPU's endianness mode due to hypercall restrictions).
> >>
> >> I have implemented something similar for MMIO emulation in KVM/arm
> >> (except that I only care about the faulting CPU).
> >>
> >> See my initial patch for that:
> >> https://lists.cs.columbia.edu/pipermail/kvmarm/2013-October/007359.html
> >>
> >> That doesn't really change the non-trapping virtio accesses, though.
> >> Where is this virtio_is_big_endian() thing?
> > 
> > It's in QEMU's exec.c. It only gets used for config space access that goes through PCI though. Is there any other place where virtio specifies native endianness today?
> 
> That's the main problem. Today's virtio flavour doesn't specify anything
> about endianness, and that is what I'm adding. Or rather (as Paolo put
> it), the prefered endianness of the virtio driver.
> 
> So once (and if) this flags are in place, you always know what you're
> dealing with. And because it is virtio-centric, you can implement it in
> an architecture independent way.
> 
> Also, most of my life revolves around kvmtool. QEMU is hardly on my
> radar, these days (for reasons that are neither technical, nor relevant
> to this forum). So it is important to me that the solution is platform
> emulation agnostic.
> 
> 	M.

f you like, you should be able to implement virtio_is_big_endian
in kvmtool too.

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

^ permalink raw reply	[flat|nested] 86+ messages in thread
* [PATCH 0/3] virtio-mmio: handle BE guests on LE hosts
@ 2013-10-14 14:05             ` Michael S. Tsirkin
  0 siblings, 0 replies; 86+ messages in thread
From: Michael S. Tsirkin @ 2013-10-14 14:05 UTC (permalink / raw)
  To: linux-arm-kernel

On Mon, Oct 14, 2013 at 02:49:10PM +0100, Marc Zyngier wrote:
> On 14/10/13 14:39, Alexander Graf wrote:
> > 
> > On 14.10.2013, at 15:24, Marc Zyngier <marc.zyngier@arm.com> wrote:
> > 
> >> On 14/10/13 14:10, Alexander Graf wrote:
> >>>
> >>> On 14.10.2013, at 15:03, Paolo Bonzini <pbonzini@redhat.com> wrote:
> >>>
> >>>> Il 11/10/2013 16:36, Marc Zyngier ha scritto:
> >>>>> This small patch series adds just enough kernel infrastructure and
> >>>>> fixes to allow a BE guest to use virtio-mmio on a LE host, provided
> >>>>> that the host actually supports such madness.
> >>>>
> >>>> More precisely, it allows the guest drivers to pick the endianness they
> >>>> prefer.  Mixed-endian virtio works fine on QEMU with e.g. a mips guest
> >>>> in emulation mode, because then any given QEMU binary will always use
> >>>> the same endianness (e.g. big for qemu-system-mips).
> >>>
> >>> We have the same problem (runtime switchable endianness) on PowerPC. IBM POWER is gaining Little Endian support in Linux now, so we could easily end up with an LE guest on a BE host.
> >>>
> >>> IIRC the way we're going to solve this is to hack up virtio_is_big_endian() to evaluate the first CPU's endianness mode (which will always be the same as all other CPU's endianness mode due to hypercall restrictions).
> >>
> >> I have implemented something similar for MMIO emulation in KVM/arm
> >> (except that I only care about the faulting CPU).
> >>
> >> See my initial patch for that:
> >> https://lists.cs.columbia.edu/pipermail/kvmarm/2013-October/007359.html
> >>
> >> That doesn't really change the non-trapping virtio accesses, though.
> >> Where is this virtio_is_big_endian() thing?
> > 
> > It's in QEMU's exec.c. It only gets used for config space access that goes through PCI though. Is there any other place where virtio specifies native endianness today?
> 
> That's the main problem. Today's virtio flavour doesn't specify anything
> about endianness, and that is what I'm adding. Or rather (as Paolo put
> it), the prefered endianness of the virtio driver.
> 
> So once (and if) this flags are in place, you always know what you're
> dealing with. And because it is virtio-centric, you can implement it in
> an architecture independent way.
> 
> Also, most of my life revolves around kvmtool. QEMU is hardly on my
> radar, these days (for reasons that are neither technical, nor relevant
> to this forum). So it is important to me that the solution is platform
> emulation agnostic.
> 
> 	M.

f you like, you should be able to implement virtio_is_big_endian
in kvmtool too.

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

^ permalink raw reply	[flat|nested] 86+ messages in thread
* Re: [PATCH 0/3] virtio-mmio: handle BE guests on LE hosts
  2013-10-14 14:05             ` Michael S. Tsirkin
@ 2013-10-14 14:13               ` Marc Zyngier
  -1 siblings, 0 replies; 86+ messages in thread
From: Marc Zyngier @ 2013-10-14 14:13 UTC (permalink / raw)
  To: Michael S. Tsirkin
  Cc: Alexander Graf, Paolo Bonzini, kvm@vger.kernel.org mailing list,
	Pawel Moll, kvmarm, linux-arm-kernel

On 14/10/13 15:05, Michael S. Tsirkin wrote:
> On Mon, Oct 14, 2013 at 02:49:10PM +0100, Marc Zyngier wrote:
>> On 14/10/13 14:39, Alexander Graf wrote:
>>>
>>> On 14.10.2013, at 15:24, Marc Zyngier <marc.zyngier@arm.com> wrote:
>>>
>>>> On 14/10/13 14:10, Alexander Graf wrote:
>>>>>
>>>>> On 14.10.2013, at 15:03, Paolo Bonzini <pbonzini@redhat.com> wrote:
>>>>>
>>>>>> Il 11/10/2013 16:36, Marc Zyngier ha scritto:
>>>>>>> This small patch series adds just enough kernel infrastructure and
>>>>>>> fixes to allow a BE guest to use virtio-mmio on a LE host, provided
>>>>>>> that the host actually supports such madness.
>>>>>>
>>>>>> More precisely, it allows the guest drivers to pick the endianness they
>>>>>> prefer.  Mixed-endian virtio works fine on QEMU with e.g. a mips guest
>>>>>> in emulation mode, because then any given QEMU binary will always use
>>>>>> the same endianness (e.g. big for qemu-system-mips).
>>>>>
>>>>> We have the same problem (runtime switchable endianness) on PowerPC. IBM POWER is gaining Little Endian support in Linux now, so we could easily end up with an LE guest on a BE host.
>>>>>
>>>>> IIRC the way we're going to solve this is to hack up virtio_is_big_endian() to evaluate the first CPU's endianness mode (which will always be the same as all other CPU's endianness mode due to hypercall restrictions).
>>>>
>>>> I have implemented something similar for MMIO emulation in KVM/arm
>>>> (except that I only care about the faulting CPU).
>>>>
>>>> See my initial patch for that:
>>>> https://lists.cs.columbia.edu/pipermail/kvmarm/2013-October/007359.html
>>>>
>>>> That doesn't really change the non-trapping virtio accesses, though.
>>>> Where is this virtio_is_big_endian() thing?
>>>
>>> It's in QEMU's exec.c. It only gets used for config space access that goes through PCI though. Is there any other place where virtio specifies native endianness today?
>>
>> That's the main problem. Today's virtio flavour doesn't specify anything
>> about endianness, and that is what I'm adding. Or rather (as Paolo put
>> it), the prefered endianness of the virtio driver.
>>
>> So once (and if) this flags are in place, you always know what you're
>> dealing with. And because it is virtio-centric, you can implement it in
>> an architecture independent way.
>>
>> Also, most of my life revolves around kvmtool. QEMU is hardly on my
>> radar, these days (for reasons that are neither technical, nor relevant
>> to this forum). So it is important to me that the solution is platform
>> emulation agnostic.
>>
>> 	M.
> 
> f you like, you should be able to implement virtio_is_big_endian
> in kvmtool too.

Sure. And I imagine this traps back into the kernel to read some
register and find out what the endianness of the accessing CPU is?

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


^ permalink raw reply	[flat|nested] 86+ messages in thread
* [PATCH 0/3] virtio-mmio: handle BE guests on LE hosts
@ 2013-10-14 14:13               ` Marc Zyngier
  0 siblings, 0 replies; 86+ messages in thread
From: Marc Zyngier @ 2013-10-14 14:13 UTC (permalink / raw)
  To: linux-arm-kernel

On 14/10/13 15:05, Michael S. Tsirkin wrote:
> On Mon, Oct 14, 2013 at 02:49:10PM +0100, Marc Zyngier wrote:
>> On 14/10/13 14:39, Alexander Graf wrote:
>>>
>>> On 14.10.2013, at 15:24, Marc Zyngier <marc.zyngier@arm.com> wrote:
>>>
>>>> On 14/10/13 14:10, Alexander Graf wrote:
>>>>>
>>>>> On 14.10.2013, at 15:03, Paolo Bonzini <pbonzini@redhat.com> wrote:
>>>>>
>>>>>> Il 11/10/2013 16:36, Marc Zyngier ha scritto:
>>>>>>> This small patch series adds just enough kernel infrastructure and
>>>>>>> fixes to allow a BE guest to use virtio-mmio on a LE host, provided
>>>>>>> that the host actually supports such madness.
>>>>>>
>>>>>> More precisely, it allows the guest drivers to pick the endianness they
>>>>>> prefer.  Mixed-endian virtio works fine on QEMU with e.g. a mips guest
>>>>>> in emulation mode, because then any given QEMU binary will always use
>>>>>> the same endianness (e.g. big for qemu-system-mips).
>>>>>
>>>>> We have the same problem (runtime switchable endianness) on PowerPC. IBM POWER is gaining Little Endian support in Linux now, so we could easily end up with an LE guest on a BE host.
>>>>>
>>>>> IIRC the way we're going to solve this is to hack up virtio_is_big_endian() to evaluate the first CPU's endianness mode (which will always be the same as all other CPU's endianness mode due to hypercall restrictions).
>>>>
>>>> I have implemented something similar for MMIO emulation in KVM/arm
>>>> (except that I only care about the faulting CPU).
>>>>
>>>> See my initial patch for that:
>>>> https://lists.cs.columbia.edu/pipermail/kvmarm/2013-October/007359.html
>>>>
>>>> That doesn't really change the non-trapping virtio accesses, though.
>>>> Where is this virtio_is_big_endian() thing?
>>>
>>> It's in QEMU's exec.c. It only gets used for config space access that goes through PCI though. Is there any other place where virtio specifies native endianness today?
>>
>> That's the main problem. Today's virtio flavour doesn't specify anything
>> about endianness, and that is what I'm adding. Or rather (as Paolo put
>> it), the prefered endianness of the virtio driver.
>>
>> So once (and if) this flags are in place, you always know what you're
>> dealing with. And because it is virtio-centric, you can implement it in
>> an architecture independent way.
>>
>> Also, most of my life revolves around kvmtool. QEMU is hardly on my
>> radar, these days (for reasons that are neither technical, nor relevant
>> to this forum). So it is important to me that the solution is platform
>> emulation agnostic.
>>
>> 	M.
> 
> f you like, you should be able to implement virtio_is_big_endian
> in kvmtool too.

Sure. And I imagine this traps back into the kernel to read some
register and find out what the endianness of the accessing CPU is?

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

^ permalink raw reply	[flat|nested] 86+ messages in thread
* Re: [PATCH 0/3] virtio-mmio: handle BE guests on LE hosts
  2013-10-14 14:13               ` Marc Zyngier
@ 2013-10-14 14:16                 ` Alexander Graf
  -1 siblings, 0 replies; 86+ messages in thread
From: Alexander Graf @ 2013-10-14 14:16 UTC (permalink / raw)
  To: Marc Zyngier
  Cc: Michael S. Tsirkin, Paolo Bonzini,
	kvm@vger.kernel.org mailing list, Pawel Moll, kvmarm,
	linux-arm-kernel


On 14.10.2013, at 16:13, Marc Zyngier <marc.zyngier@arm.com> wrote:

> On 14/10/13 15:05, Michael S. Tsirkin wrote:
>> On Mon, Oct 14, 2013 at 02:49:10PM +0100, Marc Zyngier wrote:
>>> On 14/10/13 14:39, Alexander Graf wrote:
>>>> 
>>>> On 14.10.2013, at 15:24, Marc Zyngier <marc.zyngier@arm.com> wrote:
>>>> 
>>>>> On 14/10/13 14:10, Alexander Graf wrote:
>>>>>> 
>>>>>> On 14.10.2013, at 15:03, Paolo Bonzini <pbonzini@redhat.com> wrote:
>>>>>> 
>>>>>>> Il 11/10/2013 16:36, Marc Zyngier ha scritto:
>>>>>>>> This small patch series adds just enough kernel infrastructure and
>>>>>>>> fixes to allow a BE guest to use virtio-mmio on a LE host, provided
>>>>>>>> that the host actually supports such madness.
>>>>>>> 
>>>>>>> More precisely, it allows the guest drivers to pick the endianness they
>>>>>>> prefer.  Mixed-endian virtio works fine on QEMU with e.g. a mips guest
>>>>>>> in emulation mode, because then any given QEMU binary will always use
>>>>>>> the same endianness (e.g. big for qemu-system-mips).
>>>>>> 
>>>>>> We have the same problem (runtime switchable endianness) on PowerPC. IBM POWER is gaining Little Endian support in Linux now, so we could easily end up with an LE guest on a BE host.
>>>>>> 
>>>>>> IIRC the way we're going to solve this is to hack up virtio_is_big_endian() to evaluate the first CPU's endianness mode (which will always be the same as all other CPU's endianness mode due to hypercall restrictions).
>>>>> 
>>>>> I have implemented something similar for MMIO emulation in KVM/arm
>>>>> (except that I only care about the faulting CPU).
>>>>> 
>>>>> See my initial patch for that:
>>>>> https://lists.cs.columbia.edu/pipermail/kvmarm/2013-October/007359.html
>>>>> 
>>>>> That doesn't really change the non-trapping virtio accesses, though.
>>>>> Where is this virtio_is_big_endian() thing?
>>>> 
>>>> It's in QEMU's exec.c. It only gets used for config space access that goes through PCI though. Is there any other place where virtio specifies native endianness today?
>>> 
>>> That's the main problem. Today's virtio flavour doesn't specify anything
>>> about endianness, and that is what I'm adding. Or rather (as Paolo put
>>> it), the prefered endianness of the virtio driver.
>>> 
>>> So once (and if) this flags are in place, you always know what you're
>>> dealing with. And because it is virtio-centric, you can implement it in
>>> an architecture independent way.
>>> 
>>> Also, most of my life revolves around kvmtool. QEMU is hardly on my
>>> radar, these days (for reasons that are neither technical, nor relevant
>>> to this forum). So it is important to me that the solution is platform
>>> emulation agnostic.
>>> 
>>> 	M.
>> 
>> f you like, you should be able to implement virtio_is_big_endian
>> in kvmtool too.
> 
> Sure. And I imagine this traps back into the kernel to read some
> register and find out what the endianness of the accessing CPU is?

Not yet. To be exact, it does the below today. But all virtio device emulation is 100% guest endianness unaware. This helper is the only piece of code where it gets any idea what endianness the guest has. So by checking for references to it in the code you know where endianness is an issue. And that's only in the config space.


Alex


bool virtio_is_big_endian(void)
{
#if defined(TARGET_WORDS_BIGENDIAN)
    return true;
#else
    return false;
#endif
}


^ permalink raw reply	[flat|nested] 86+ messages in thread
* [PATCH 0/3] virtio-mmio: handle BE guests on LE hosts
@ 2013-10-14 14:16                 ` Alexander Graf
  0 siblings, 0 replies; 86+ messages in thread
From: Alexander Graf @ 2013-10-14 14:16 UTC (permalink / raw)
  To: linux-arm-kernel


On 14.10.2013, at 16:13, Marc Zyngier <marc.zyngier@arm.com> wrote:

> On 14/10/13 15:05, Michael S. Tsirkin wrote:
>> On Mon, Oct 14, 2013 at 02:49:10PM +0100, Marc Zyngier wrote:
>>> On 14/10/13 14:39, Alexander Graf wrote:
>>>> 
>>>> On 14.10.2013, at 15:24, Marc Zyngier <marc.zyngier@arm.com> wrote:
>>>> 
>>>>> On 14/10/13 14:10, Alexander Graf wrote:
>>>>>> 
>>>>>> On 14.10.2013, at 15:03, Paolo Bonzini <pbonzini@redhat.com> wrote:
>>>>>> 
>>>>>>> Il 11/10/2013 16:36, Marc Zyngier ha scritto:
>>>>>>>> This small patch series adds just enough kernel infrastructure and
>>>>>>>> fixes to allow a BE guest to use virtio-mmio on a LE host, provided
>>>>>>>> that the host actually supports such madness.
>>>>>>> 
>>>>>>> More precisely, it allows the guest drivers to pick the endianness they
>>>>>>> prefer.  Mixed-endian virtio works fine on QEMU with e.g. a mips guest
>>>>>>> in emulation mode, because then any given QEMU binary will always use
>>>>>>> the same endianness (e.g. big for qemu-system-mips).
>>>>>> 
>>>>>> We have the same problem (runtime switchable endianness) on PowerPC. IBM POWER is gaining Little Endian support in Linux now, so we could easily end up with an LE guest on a BE host.
>>>>>> 
>>>>>> IIRC the way we're going to solve this is to hack up virtio_is_big_endian() to evaluate the first CPU's endianness mode (which will always be the same as all other CPU's endianness mode due to hypercall restrictions).
>>>>> 
>>>>> I have implemented something similar for MMIO emulation in KVM/arm
>>>>> (except that I only care about the faulting CPU).
>>>>> 
>>>>> See my initial patch for that:
>>>>> https://lists.cs.columbia.edu/pipermail/kvmarm/2013-October/007359.html
>>>>> 
>>>>> That doesn't really change the non-trapping virtio accesses, though.
>>>>> Where is this virtio_is_big_endian() thing?
>>>> 
>>>> It's in QEMU's exec.c. It only gets used for config space access that goes through PCI though. Is there any other place where virtio specifies native endianness today?
>>> 
>>> That's the main problem. Today's virtio flavour doesn't specify anything
>>> about endianness, and that is what I'm adding. Or rather (as Paolo put
>>> it), the prefered endianness of the virtio driver.
>>> 
>>> So once (and if) this flags are in place, you always know what you're
>>> dealing with. And because it is virtio-centric, you can implement it in
>>> an architecture independent way.
>>> 
>>> Also, most of my life revolves around kvmtool. QEMU is hardly on my
>>> radar, these days (for reasons that are neither technical, nor relevant
>>> to this forum). So it is important to me that the solution is platform
>>> emulation agnostic.
>>> 
>>> 	M.
>> 
>> f you like, you should be able to implement virtio_is_big_endian
>> in kvmtool too.
> 
> Sure. And I imagine this traps back into the kernel to read some
> register and find out what the endianness of the accessing CPU is?

Not yet. To be exact, it does the below today. But all virtio device emulation is 100% guest endianness unaware. This helper is the only piece of code where it gets any idea what endianness the guest has. So by checking for references to it in the code you know where endianness is an issue. And that's only in the config space.


Alex


bool virtio_is_big_endian(void)
{
#if defined(TARGET_WORDS_BIGENDIAN)
    return true;
#else
    return false;
#endif
}

^ permalink raw reply	[flat|nested] 86+ messages in thread
* Re: [PATCH 0/3] virtio-mmio: handle BE guests on LE hosts
  2013-10-14 14:16                 ` Alexander Graf
@ 2013-10-14 14:52                   ` Marc Zyngier
  -1 siblings, 0 replies; 86+ messages in thread
From: Marc Zyngier @ 2013-10-14 14:52 UTC (permalink / raw)
  To: Alexander Graf
  Cc: Michael S. Tsirkin, Paolo Bonzini,
	kvm@vger.kernel.org mailing list, Pawel Moll, kvmarm,
	linux-arm-kernel

On 14/10/13 15:16, Alexander Graf wrote:
> 
> On 14.10.2013, at 16:13, Marc Zyngier <marc.zyngier@arm.com> wrote:
> 
>> On 14/10/13 15:05, Michael S. Tsirkin wrote:
>>> On Mon, Oct 14, 2013 at 02:49:10PM +0100, Marc Zyngier wrote:
>>>> On 14/10/13 14:39, Alexander Graf wrote:
>>>>>
>>>>> On 14.10.2013, at 15:24, Marc Zyngier <marc.zyngier@arm.com> wrote:
>>>>>
>>>>>> On 14/10/13 14:10, Alexander Graf wrote:
>>>>>>>
>>>>>>> On 14.10.2013, at 15:03, Paolo Bonzini <pbonzini@redhat.com> wrote:
>>>>>>>
>>>>>>>> Il 11/10/2013 16:36, Marc Zyngier ha scritto:
>>>>>>>>> This small patch series adds just enough kernel infrastructure and
>>>>>>>>> fixes to allow a BE guest to use virtio-mmio on a LE host, provided
>>>>>>>>> that the host actually supports such madness.
>>>>>>>>
>>>>>>>> More precisely, it allows the guest drivers to pick the endianness they
>>>>>>>> prefer.  Mixed-endian virtio works fine on QEMU with e.g. a mips guest
>>>>>>>> in emulation mode, because then any given QEMU binary will always use
>>>>>>>> the same endianness (e.g. big for qemu-system-mips).
>>>>>>>
>>>>>>> We have the same problem (runtime switchable endianness) on PowerPC. IBM POWER is gaining Little Endian support in Linux now, so we could easily end up with an LE guest on a BE host.
>>>>>>>
>>>>>>> IIRC the way we're going to solve this is to hack up virtio_is_big_endian() to evaluate the first CPU's endianness mode (which will always be the same as all other CPU's endianness mode due to hypercall restrictions).
>>>>>>
>>>>>> I have implemented something similar for MMIO emulation in KVM/arm
>>>>>> (except that I only care about the faulting CPU).
>>>>>>
>>>>>> See my initial patch for that:
>>>>>> https://lists.cs.columbia.edu/pipermail/kvmarm/2013-October/007359.html
>>>>>>
>>>>>> That doesn't really change the non-trapping virtio accesses, though.
>>>>>> Where is this virtio_is_big_endian() thing?
>>>>>
>>>>> It's in QEMU's exec.c. It only gets used for config space access that goes through PCI though. Is there any other place where virtio specifies native endianness today?
>>>>
>>>> That's the main problem. Today's virtio flavour doesn't specify anything
>>>> about endianness, and that is what I'm adding. Or rather (as Paolo put
>>>> it), the prefered endianness of the virtio driver.
>>>>
>>>> So once (and if) this flags are in place, you always know what you're
>>>> dealing with. And because it is virtio-centric, you can implement it in
>>>> an architecture independent way.
>>>>
>>>> Also, most of my life revolves around kvmtool. QEMU is hardly on my
>>>> radar, these days (for reasons that are neither technical, nor relevant
>>>> to this forum). So it is important to me that the solution is platform
>>>> emulation agnostic.
>>>>
>>>> 	M.
>>>
>>> f you like, you should be able to implement virtio_is_big_endian
>>> in kvmtool too.
>>
>> Sure. And I imagine this traps back into the kernel to read some
>> register and find out what the endianness of the accessing CPU is?
> 
> Not yet. To be exact, it does the below today. But all virtio device
> emulation is 100% guest endianness unaware. This helper is the only
> piece of code where it gets any idea what endianness the guest has. So
> by checking for references to it in the code you know where endianness
> is an issue. And that's only in the config space.

Only config space? How do you deal with virtio ring descriptors, for
example?

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


^ permalink raw reply	[flat|nested] 86+ messages in thread
* [PATCH 0/3] virtio-mmio: handle BE guests on LE hosts
@ 2013-10-14 14:52                   ` Marc Zyngier
  0 siblings, 0 replies; 86+ messages in thread
From: Marc Zyngier @ 2013-10-14 14:52 UTC (permalink / raw)
  To: linux-arm-kernel

On 14/10/13 15:16, Alexander Graf wrote:
> 
> On 14.10.2013, at 16:13, Marc Zyngier <marc.zyngier@arm.com> wrote:
> 
>> On 14/10/13 15:05, Michael S. Tsirkin wrote:
>>> On Mon, Oct 14, 2013 at 02:49:10PM +0100, Marc Zyngier wrote:
>>>> On 14/10/13 14:39, Alexander Graf wrote:
>>>>>
>>>>> On 14.10.2013, at 15:24, Marc Zyngier <marc.zyngier@arm.com> wrote:
>>>>>
>>>>>> On 14/10/13 14:10, Alexander Graf wrote:
>>>>>>>
>>>>>>> On 14.10.2013, at 15:03, Paolo Bonzini <pbonzini@redhat.com> wrote:
>>>>>>>
>>>>>>>> Il 11/10/2013 16:36, Marc Zyngier ha scritto:
>>>>>>>>> This small patch series adds just enough kernel infrastructure and
>>>>>>>>> fixes to allow a BE guest to use virtio-mmio on a LE host, provided
>>>>>>>>> that the host actually supports such madness.
>>>>>>>>
>>>>>>>> More precisely, it allows the guest drivers to pick the endianness they
>>>>>>>> prefer.  Mixed-endian virtio works fine on QEMU with e.g. a mips guest
>>>>>>>> in emulation mode, because then any given QEMU binary will always use
>>>>>>>> the same endianness (e.g. big for qemu-system-mips).
>>>>>>>
>>>>>>> We have the same problem (runtime switchable endianness) on PowerPC. IBM POWER is gaining Little Endian support in Linux now, so we could easily end up with an LE guest on a BE host.
>>>>>>>
>>>>>>> IIRC the way we're going to solve this is to hack up virtio_is_big_endian() to evaluate the first CPU's endianness mode (which will always be the same as all other CPU's endianness mode due to hypercall restrictions).
>>>>>>
>>>>>> I have implemented something similar for MMIO emulation in KVM/arm
>>>>>> (except that I only care about the faulting CPU).
>>>>>>
>>>>>> See my initial patch for that:
>>>>>> https://lists.cs.columbia.edu/pipermail/kvmarm/2013-October/007359.html
>>>>>>
>>>>>> That doesn't really change the non-trapping virtio accesses, though.
>>>>>> Where is this virtio_is_big_endian() thing?
>>>>>
>>>>> It's in QEMU's exec.c. It only gets used for config space access that goes through PCI though. Is there any other place where virtio specifies native endianness today?
>>>>
>>>> That's the main problem. Today's virtio flavour doesn't specify anything
>>>> about endianness, and that is what I'm adding. Or rather (as Paolo put
>>>> it), the prefered endianness of the virtio driver.
>>>>
>>>> So once (and if) this flags are in place, you always know what you're
>>>> dealing with. And because it is virtio-centric, you can implement it in
>>>> an architecture independent way.
>>>>
>>>> Also, most of my life revolves around kvmtool. QEMU is hardly on my
>>>> radar, these days (for reasons that are neither technical, nor relevant
>>>> to this forum). So it is important to me that the solution is platform
>>>> emulation agnostic.
>>>>
>>>> 	M.
>>>
>>> f you like, you should be able to implement virtio_is_big_endian
>>> in kvmtool too.
>>
>> Sure. And I imagine this traps back into the kernel to read some
>> register and find out what the endianness of the accessing CPU is?
> 
> Not yet. To be exact, it does the below today. But all virtio device
> emulation is 100% guest endianness unaware. This helper is the only
> piece of code where it gets any idea what endianness the guest has. So
> by checking for references to it in the code you know where endianness
> is an issue. And that's only in the config space.

Only config space? How do you deal with virtio ring descriptors, for
example?

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

^ permalink raw reply	[flat|nested] 86+ messages in thread
* Re: [PATCH 0/3] virtio-mmio: handle BE guests on LE hosts
  2013-10-14 14:52                   ` Marc Zyngier
@ 2013-10-14 14:56                     ` Paolo Bonzini
  -1 siblings, 0 replies; 86+ messages in thread
From: Paolo Bonzini @ 2013-10-14 14:56 UTC (permalink / raw)
  To: Marc Zyngier
  Cc: Alexander Graf, Michael S. Tsirkin,
	kvm@vger.kernel.org mailing list, Pawel Moll, kvmarm,
	linux-arm-kernel

Il 14/10/2013 16:52, Marc Zyngier ha scritto:
> > > Sure. And I imagine this traps back into the kernel to read some
> > > register and find out what the endianness of the accessing CPU is?
> > 
> > Not yet. To be exact, it does the below today. But all virtio device
> > emulation is 100% guest endianness unaware. This helper is the only
> > piece of code where it gets any idea what endianness the guest has. So
> > by checking for references to it in the code you know where endianness
> > is an issue. And that's only in the config space.
> 
> Only config space? How do you deal with virtio ring descriptors, for
> example?

They also use guest endianness, but do not use virtio_is_big_endian()
(yet?) so Alex missed them.

Paolo

^ permalink raw reply	[flat|nested] 86+ messages in thread
* [PATCH 0/3] virtio-mmio: handle BE guests on LE hosts
@ 2013-10-14 14:56                     ` Paolo Bonzini
  0 siblings, 0 replies; 86+ messages in thread
From: Paolo Bonzini @ 2013-10-14 14:56 UTC (permalink / raw)
  To: linux-arm-kernel

Il 14/10/2013 16:52, Marc Zyngier ha scritto:
> > > Sure. And I imagine this traps back into the kernel to read some
> > > register and find out what the endianness of the accessing CPU is?
> > 
> > Not yet. To be exact, it does the below today. But all virtio device
> > emulation is 100% guest endianness unaware. This helper is the only
> > piece of code where it gets any idea what endianness the guest has. So
> > by checking for references to it in the code you know where endianness
> > is an issue. And that's only in the config space.
> 
> Only config space? How do you deal with virtio ring descriptors, for
> example?

They also use guest endianness, but do not use virtio_is_big_endian()
(yet?) so Alex missed them.

Paolo

^ permalink raw reply	[flat|nested] 86+ messages in thread
* Re: [PATCH 0/3] virtio-mmio: handle BE guests on LE hosts
  2013-10-14 14:56                     ` Paolo Bonzini
@ 2013-10-14 15:12                       ` Marc Zyngier
  -1 siblings, 0 replies; 86+ messages in thread
From: Marc Zyngier @ 2013-10-14 15:12 UTC (permalink / raw)
  To: Paolo Bonzini
  Cc: Alexander Graf, Michael S. Tsirkin,
	kvm@vger.kernel.org mailing list, Pawel Moll, kvmarm,
	linux-arm-kernel

On 14/10/13 15:56, Paolo Bonzini wrote:
> Il 14/10/2013 16:52, Marc Zyngier ha scritto:
>>>> Sure. And I imagine this traps back into the kernel to read some
>>>> register and find out what the endianness of the accessing CPU is?
>>>
>>> Not yet. To be exact, it does the below today. But all virtio device
>>> emulation is 100% guest endianness unaware. This helper is the only
>>> piece of code where it gets any idea what endianness the guest has. So
>>> by checking for references to it in the code you know where endianness
>>> is an issue. And that's only in the config space.
>>
>> Only config space? How do you deal with virtio ring descriptors, for
>> example?
> 
> They also use guest endianness, but do not use virtio_is_big_endian()
> (yet?) so Alex missed them.

Yeah, I thought as much. There is a whole bunch of things that need byte
swapping, both at the virtio level itself, and at the device level as well.

Grep-ing for __u{16,32,64} through include/uapi/linux/virtio* shows the
extent of the disaster.

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


^ permalink raw reply	[flat|nested] 86+ messages in thread
* [PATCH 0/3] virtio-mmio: handle BE guests on LE hosts
@ 2013-10-14 15:12                       ` Marc Zyngier
  0 siblings, 0 replies; 86+ messages in thread
From: Marc Zyngier @ 2013-10-14 15:12 UTC (permalink / raw)
  To: linux-arm-kernel

On 14/10/13 15:56, Paolo Bonzini wrote:
> Il 14/10/2013 16:52, Marc Zyngier ha scritto:
>>>> Sure. And I imagine this traps back into the kernel to read some
>>>> register and find out what the endianness of the accessing CPU is?
>>>
>>> Not yet. To be exact, it does the below today. But all virtio device
>>> emulation is 100% guest endianness unaware. This helper is the only
>>> piece of code where it gets any idea what endianness the guest has. So
>>> by checking for references to it in the code you know where endianness
>>> is an issue. And that's only in the config space.
>>
>> Only config space? How do you deal with virtio ring descriptors, for
>> example?
> 
> They also use guest endianness, but do not use virtio_is_big_endian()
> (yet?) so Alex missed them.

Yeah, I thought as much. There is a whole bunch of things that need byte
swapping, both at the virtio level itself, and at the device level as well.

Grep-ing for __u{16,32,64} through include/uapi/linux/virtio* shows the
extent of the disaster.

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

^ permalink raw reply	[flat|nested] 86+ messages in thread
* Re: [PATCH 0/3] virtio-mmio: handle BE guests on LE hosts
  2013-10-14 15:12                       ` Marc Zyngier
@ 2013-10-14 15:22                         ` Paolo Bonzini
  -1 siblings, 0 replies; 86+ messages in thread
From: Paolo Bonzini @ 2013-10-14 15:22 UTC (permalink / raw)
  To: Marc Zyngier
  Cc: kvm@vger.kernel.org mailing list, Pawel Moll, Michael S. Tsirkin,
	Alexander Graf, kvmarm, linux-arm-kernel

Il 14/10/2013 17:12, Marc Zyngier ha scritto:
> On 14/10/13 15:56, Paolo Bonzini wrote:
>> Il 14/10/2013 16:52, Marc Zyngier ha scritto:
>>>>> Sure. And I imagine this traps back into the kernel to read some
>>>>> register and find out what the endianness of the accessing CPU is?
>>>>
>>>> Not yet. To be exact, it does the below today. But all virtio device
>>>> emulation is 100% guest endianness unaware. This helper is the only
>>>> piece of code where it gets any idea what endianness the guest has. So
>>>> by checking for references to it in the code you know where endianness
>>>> is an issue. And that's only in the config space.
>>>
>>> Only config space? How do you deal with virtio ring descriptors, for
>>> example?
>>
>> They also use guest endianness, but do not use virtio_is_big_endian()
>> (yet?) so Alex missed them.
> 
> Yeah, I thought as much. There is a whole bunch of things that need byte
> swapping, both at the virtio level itself, and at the device level as well.
> 
> Grep-ing for __u{16,32,64} through include/uapi/linux/virtio* shows the
> extent of the disaster.

Devices are fine in QEMU, it's only the "generic" parts (rings) that are
missing AFAICT.

Paolo

^ permalink raw reply	[flat|nested] 86+ messages in thread
* [PATCH 0/3] virtio-mmio: handle BE guests on LE hosts
@ 2013-10-14 15:22                         ` Paolo Bonzini
  0 siblings, 0 replies; 86+ messages in thread
From: Paolo Bonzini @ 2013-10-14 15:22 UTC (permalink / raw)
  To: linux-arm-kernel

Il 14/10/2013 17:12, Marc Zyngier ha scritto:
> On 14/10/13 15:56, Paolo Bonzini wrote:
>> Il 14/10/2013 16:52, Marc Zyngier ha scritto:
>>>>> Sure. And I imagine this traps back into the kernel to read some
>>>>> register and find out what the endianness of the accessing CPU is?
>>>>
>>>> Not yet. To be exact, it does the below today. But all virtio device
>>>> emulation is 100% guest endianness unaware. This helper is the only
>>>> piece of code where it gets any idea what endianness the guest has. So
>>>> by checking for references to it in the code you know where endianness
>>>> is an issue. And that's only in the config space.
>>>
>>> Only config space? How do you deal with virtio ring descriptors, for
>>> example?
>>
>> They also use guest endianness, but do not use virtio_is_big_endian()
>> (yet?) so Alex missed them.
> 
> Yeah, I thought as much. There is a whole bunch of things that need byte
> swapping, both at the virtio level itself, and at the device level as well.
> 
> Grep-ing for __u{16,32,64} through include/uapi/linux/virtio* shows the
> extent of the disaster.

Devices are fine in QEMU, it's only the "generic" parts (rings) that are
missing AFAICT.

Paolo

^ permalink raw reply	[flat|nested] 86+ messages in thread
* Re: [PATCH 0/3] virtio-mmio: handle BE guests on LE hosts
  2013-10-14 15:22                         ` Paolo Bonzini
@ 2013-10-14 15:36                           ` Marc Zyngier
  -1 siblings, 0 replies; 86+ messages in thread
From: Marc Zyngier @ 2013-10-14 15:36 UTC (permalink / raw)
  To: Paolo Bonzini
  Cc: Alexander Graf, Michael S. Tsirkin,
	kvm@vger.kernel.org mailing list, Pawel Moll, kvmarm,
	linux-arm-kernel

On 14/10/13 16:22, Paolo Bonzini wrote:
> Il 14/10/2013 17:12, Marc Zyngier ha scritto:
>> On 14/10/13 15:56, Paolo Bonzini wrote:
>>> Il 14/10/2013 16:52, Marc Zyngier ha scritto:
>>>>>> Sure. And I imagine this traps back into the kernel to read some
>>>>>> register and find out what the endianness of the accessing CPU is?
>>>>>
>>>>> Not yet. To be exact, it does the below today. But all virtio device
>>>>> emulation is 100% guest endianness unaware. This helper is the only
>>>>> piece of code where it gets any idea what endianness the guest has. So
>>>>> by checking for references to it in the code you know where endianness
>>>>> is an issue. And that's only in the config space.
>>>>
>>>> Only config space? How do you deal with virtio ring descriptors, for
>>>> example?
>>>
>>> They also use guest endianness, but do not use virtio_is_big_endian()
>>> (yet?) so Alex missed them.
>>
>> Yeah, I thought as much. There is a whole bunch of things that need byte
>> swapping, both at the virtio level itself, and at the device level as well.
>>
>> Grep-ing for __u{16,32,64} through include/uapi/linux/virtio* shows the
>> extent of the disaster.
> 
> Devices are fine in QEMU, it's only the "generic" parts (rings) that are
> missing AFAICT.

So if I understand correctly how it works, target endianness is set at
compile time, and you have a BE specific QEMU?

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


^ permalink raw reply	[flat|nested] 86+ messages in thread
* [PATCH 0/3] virtio-mmio: handle BE guests on LE hosts
@ 2013-10-14 15:36                           ` Marc Zyngier
  0 siblings, 0 replies; 86+ messages in thread
From: Marc Zyngier @ 2013-10-14 15:36 UTC (permalink / raw)
  To: linux-arm-kernel

On 14/10/13 16:22, Paolo Bonzini wrote:
> Il 14/10/2013 17:12, Marc Zyngier ha scritto:
>> On 14/10/13 15:56, Paolo Bonzini wrote:
>>> Il 14/10/2013 16:52, Marc Zyngier ha scritto:
>>>>>> Sure. And I imagine this traps back into the kernel to read some
>>>>>> register and find out what the endianness of the accessing CPU is?
>>>>>
>>>>> Not yet. To be exact, it does the below today. But all virtio device
>>>>> emulation is 100% guest endianness unaware. This helper is the only
>>>>> piece of code where it gets any idea what endianness the guest has. So
>>>>> by checking for references to it in the code you know where endianness
>>>>> is an issue. And that's only in the config space.
>>>>
>>>> Only config space? How do you deal with virtio ring descriptors, for
>>>> example?
>>>
>>> They also use guest endianness, but do not use virtio_is_big_endian()
>>> (yet?) so Alex missed them.
>>
>> Yeah, I thought as much. There is a whole bunch of things that need byte
>> swapping, both at the virtio level itself, and at the device level as well.
>>
>> Grep-ing for __u{16,32,64} through include/uapi/linux/virtio* shows the
>> extent of the disaster.
> 
> Devices are fine in QEMU, it's only the "generic" parts (rings) that are
> missing AFAICT.

So if I understand correctly how it works, target endianness is set at
compile time, and you have a BE specific QEMU?

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

^ permalink raw reply	[flat|nested] 86+ messages in thread
* Re: [PATCH 0/3] virtio-mmio: handle BE guests on LE hosts
  2013-10-14 15:22                         ` Paolo Bonzini
@ 2013-10-14 15:45                           ` Anup Patel
  -1 siblings, 0 replies; 86+ messages in thread
From: Anup Patel @ 2013-10-14 15:45 UTC (permalink / raw)
  To: Paolo Bonzini
  Cc: Marc Zyngier, kvm@vger.kernel.org mailing list, Pawel Moll,
	Michael S. Tsirkin, kvmarm, linux-arm-kernel

On Mon, Oct 14, 2013 at 8:52 PM, Paolo Bonzini <pbonzini@redhat.com> wrote:
> Il 14/10/2013 17:12, Marc Zyngier ha scritto:
>> On 14/10/13 15:56, Paolo Bonzini wrote:
>>> Il 14/10/2013 16:52, Marc Zyngier ha scritto:
>>>>>> Sure. And I imagine this traps back into the kernel to read some
>>>>>> register and find out what the endianness of the accessing CPU is?
>>>>>
>>>>> Not yet. To be exact, it does the below today. But all virtio device
>>>>> emulation is 100% guest endianness unaware. This helper is the only
>>>>> piece of code where it gets any idea what endianness the guest has. So
>>>>> by checking for references to it in the code you know where endianness
>>>>> is an issue. And that's only in the config space.
>>>>
>>>> Only config space? How do you deal with virtio ring descriptors, for
>>>> example?
>>>
>>> They also use guest endianness, but do not use virtio_is_big_endian()
>>> (yet?) so Alex missed them.
>>
>> Yeah, I thought as much. There is a whole bunch of things that need byte
>> swapping, both at the virtio level itself, and at the device level as well.
>>
>> Grep-ing for __u{16,32,64} through include/uapi/linux/virtio* shows the
>> extent of the disaster.
>
> Devices are fine in QEMU, it's only the "generic" parts (rings) that are
> missing AFAICT.

We need to take care of endianness of "device" specific descriptors in
a VirtIO device.

For example:
In VirtIO Net device, the guest sends "struct virtio_net_hdr" for each Tx
packet which describes the Tx offloads needed for packet and other info.

>
> Paolo
>
> _______________________________________________
> kvmarm mailing list
> kvmarm@lists.cs.columbia.edu
> https://lists.cs.columbia.edu/cucslists/listinfo/kvmarm

--
Anup

^ permalink raw reply	[flat|nested] 86+ messages in thread
* [PATCH 0/3] virtio-mmio: handle BE guests on LE hosts
@ 2013-10-14 15:45                           ` Anup Patel
  0 siblings, 0 replies; 86+ messages in thread
From: Anup Patel @ 2013-10-14 15:45 UTC (permalink / raw)
  To: linux-arm-kernel

On Mon, Oct 14, 2013 at 8:52 PM, Paolo Bonzini <pbonzini@redhat.com> wrote:
> Il 14/10/2013 17:12, Marc Zyngier ha scritto:
>> On 14/10/13 15:56, Paolo Bonzini wrote:
>>> Il 14/10/2013 16:52, Marc Zyngier ha scritto:
>>>>>> Sure. And I imagine this traps back into the kernel to read some
>>>>>> register and find out what the endianness of the accessing CPU is?
>>>>>
>>>>> Not yet. To be exact, it does the below today. But all virtio device
>>>>> emulation is 100% guest endianness unaware. This helper is the only
>>>>> piece of code where it gets any idea what endianness the guest has. So
>>>>> by checking for references to it in the code you know where endianness
>>>>> is an issue. And that's only in the config space.
>>>>
>>>> Only config space? How do you deal with virtio ring descriptors, for
>>>> example?
>>>
>>> They also use guest endianness, but do not use virtio_is_big_endian()
>>> (yet?) so Alex missed them.
>>
>> Yeah, I thought as much. There is a whole bunch of things that need byte
>> swapping, both at the virtio level itself, and at the device level as well.
>>
>> Grep-ing for __u{16,32,64} through include/uapi/linux/virtio* shows the
>> extent of the disaster.
>
> Devices are fine in QEMU, it's only the "generic" parts (rings) that are
> missing AFAICT.

We need to take care of endianness of "device" specific descriptors in
a VirtIO device.

For example:
In VirtIO Net device, the guest sends "struct virtio_net_hdr" for each Tx
packet which describes the Tx offloads needed for packet and other info.

>
> Paolo
>
> _______________________________________________
> kvmarm mailing list
> kvmarm at lists.cs.columbia.edu
> https://lists.cs.columbia.edu/cucslists/listinfo/kvmarm

--
Anup

^ permalink raw reply	[flat|nested] 86+ messages in thread
* Re: [PATCH 0/3] virtio-mmio: handle BE guests on LE hosts
  2013-10-14 15:36                           ` Marc Zyngier
@ 2013-10-14 16:50                             ` Paolo Bonzini
  -1 siblings, 0 replies; 86+ messages in thread
From: Paolo Bonzini @ 2013-10-14 16:50 UTC (permalink / raw)
  To: Marc Zyngier
  Cc: kvm@vger.kernel.org mailing list, Pawel Moll, Michael S. Tsirkin,
	Alexander Graf, kvmarm, linux-arm-kernel

Il 14/10/2013 17:36, Marc Zyngier ha scritto:
>> > 
>> > Devices are fine in QEMU, it's only the "generic" parts (rings) that are
>> > missing AFAICT.
> So if I understand correctly how it works, target endianness is set at
> compile time, and you have a BE specific QEMU?

Yes.  Though as Alex said, this will have to change for PPC
little-endian support.

Paolo

^ permalink raw reply	[flat|nested] 86+ messages in thread
* [PATCH 0/3] virtio-mmio: handle BE guests on LE hosts
@ 2013-10-14 16:50                             ` Paolo Bonzini
  0 siblings, 0 replies; 86+ messages in thread
From: Paolo Bonzini @ 2013-10-14 16:50 UTC (permalink / raw)
  To: linux-arm-kernel

Il 14/10/2013 17:36, Marc Zyngier ha scritto:
>> > 
>> > Devices are fine in QEMU, it's only the "generic" parts (rings) that are
>> > missing AFAICT.
> So if I understand correctly how it works, target endianness is set at
> compile time, and you have a BE specific QEMU?

Yes.  Though as Alex said, this will have to change for PPC
little-endian support.

Paolo

^ permalink raw reply	[flat|nested] 86+ messages in thread
* Re: [PATCH 0/3] virtio-mmio: handle BE guests on LE hosts
  2013-10-14 16:50                             ` Paolo Bonzini
@ 2013-10-14 17:10                               ` Michael S. Tsirkin
  -1 siblings, 0 replies; 86+ messages in thread
From: Michael S. Tsirkin @ 2013-10-14 17:10 UTC (permalink / raw)
  To: Paolo Bonzini
  Cc: Marc Zyngier, Alexander Graf, kvm@vger.kernel.org mailing list,
	Pawel Moll, kvmarm, linux-arm-kernel

On Mon, Oct 14, 2013 at 06:50:48PM +0200, Paolo Bonzini wrote:
> Il 14/10/2013 17:36, Marc Zyngier ha scritto:
> >> > 
> >> > Devices are fine in QEMU, it's only the "generic" parts (rings) that are
> >> > missing AFAICT.
> > So if I understand correctly how it works, target endianness is set at
> > compile time, and you have a BE specific QEMU?
> 
> Yes.  Though as Alex said, this will have to change for PPC
> little-endian support.
> 
> Paolo

Or we'll just say that this platform requires virtio 1.0.



^ permalink raw reply	[flat|nested] 86+ messages in thread
* [PATCH 0/3] virtio-mmio: handle BE guests on LE hosts
@ 2013-10-14 17:10                               ` Michael S. Tsirkin
  0 siblings, 0 replies; 86+ messages in thread
From: Michael S. Tsirkin @ 2013-10-14 17:10 UTC (permalink / raw)
  To: linux-arm-kernel

On Mon, Oct 14, 2013 at 06:50:48PM +0200, Paolo Bonzini wrote:
> Il 14/10/2013 17:36, Marc Zyngier ha scritto:
> >> > 
> >> > Devices are fine in QEMU, it's only the "generic" parts (rings) that are
> >> > missing AFAICT.
> > So if I understand correctly how it works, target endianness is set at
> > compile time, and you have a BE specific QEMU?
> 
> Yes.  Though as Alex said, this will have to change for PPC
> little-endian support.
> 
> Paolo

Or we'll just say that this platform requires virtio 1.0.

^ permalink raw reply	[flat|nested] 86+ messages in thread
* Re: [PATCH 0/3] virtio-mmio: handle BE guests on LE hosts
  2013-10-14 10:50             ` Pawel Moll
@ 2013-10-14 23:02               ` Rusty Russell
  -1 siblings, 0 replies; 86+ messages in thread
From: Rusty Russell @ 2013-10-14 23:02 UTC (permalink / raw)
  To: Pawel Moll, Michael S. Tsirkin
  Cc: Marc Zyngier, linux-arm-kernel, kvmarm, kvm

Pawel Moll <pawel.moll@arm.com> writes:
> On Mon, 2013-10-14 at 11:46 +0100, Michael S. Tsirkin wrote:
>> What I meant is that under the proposed scheme, users with
>> existing v1 drivers must configure a v1 device explicitly.
>> 
>> According to the plan, drivers will be updated so they can
>> work with both v1 devices and new v2 devices.
>> 
>> But if you configure a v2 device, old drivers
>> will not work.
>
> That's correct, yes. If qemu 2015(16? 17?).01 will provide v2 devices
> only, kernel 3.11 is not going to recognize any of them.
>
> What will happen in reality, though, is that the implementation of the
> Linux v1/v2 driver will pre-date *any* v2 device by far. First
> implementation of v1 devices appeared almost 2 years after the driver
> appeared. It will take year for people to upgrade to v2, and probably
> only after they experience problems with page-based addressing mode.
>
> I think I can live with this.

Yes, this makes sense.  In particular, MMIO doesn't have any users you
don't control at this point.

I expect it to be less than 2 years to get into qemu, but there'll be no
compelling reason to use v2 devices for at least that long.

Cheers,
Rusty.

^ permalink raw reply	[flat|nested] 86+ messages in thread
* [PATCH 0/3] virtio-mmio: handle BE guests on LE hosts
@ 2013-10-14 23:02               ` Rusty Russell
  0 siblings, 0 replies; 86+ messages in thread
From: Rusty Russell @ 2013-10-14 23:02 UTC (permalink / raw)
  To: linux-arm-kernel

Pawel Moll <pawel.moll@arm.com> writes:
> On Mon, 2013-10-14 at 11:46 +0100, Michael S. Tsirkin wrote:
>> What I meant is that under the proposed scheme, users with
>> existing v1 drivers must configure a v1 device explicitly.
>> 
>> According to the plan, drivers will be updated so they can
>> work with both v1 devices and new v2 devices.
>> 
>> But if you configure a v2 device, old drivers
>> will not work.
>
> That's correct, yes. If qemu 2015(16? 17?).01 will provide v2 devices
> only, kernel 3.11 is not going to recognize any of them.
>
> What will happen in reality, though, is that the implementation of the
> Linux v1/v2 driver will pre-date *any* v2 device by far. First
> implementation of v1 devices appeared almost 2 years after the driver
> appeared. It will take year for people to upgrade to v2, and probably
> only after they experience problems with page-based addressing mode.
>
> I think I can live with this.

Yes, this makes sense.  In particular, MMIO doesn't have any users you
don't control at this point.

I expect it to be less than 2 years to get into qemu, but there'll be no
compelling reason to use v2 devices for at least that long.

Cheers,
Rusty.

^ permalink raw reply	[flat|nested] 86+ messages in thread
* Re: [PATCH 0/3] virtio-mmio: handle BE guests on LE hosts
  2013-10-14 17:10                               ` Michael S. Tsirkin
@ 2013-10-14 23:23                                 ` Rusty Russell
  -1 siblings, 0 replies; 86+ messages in thread
From: Rusty Russell @ 2013-10-14 23:23 UTC (permalink / raw)
  To: Michael S. Tsirkin, Paolo Bonzini
  Cc: kvm@vger.kernel.org mailing list, Pawel Moll, kvmarm, linux-arm-kernel

"Michael S. Tsirkin" <mst@redhat.com> writes:
> On Mon, Oct 14, 2013 at 06:50:48PM +0200, Paolo Bonzini wrote:
>> Il 14/10/2013 17:36, Marc Zyngier ha scritto:
>> >> > 
>> >> > Devices are fine in QEMU, it's only the "generic" parts (rings) that are
>> >> > missing AFAICT.
>> > So if I understand correctly how it works, target endianness is set at
>> > compile time, and you have a BE specific QEMU?
>> 
>> Yes.  Though as Alex said, this will have to change for PPC
>> little-endian support.
>> 
>> Paolo
>
> Or we'll just say that this platform requires virtio 1.0.

To come full circle, I have implemented virtio support for BE PPC in
qemu :) And we'll be supporting pre-1.0 for the moment. I'm awaiting
some ppc-specific hooks, but it could be merged without them.

It's icky: we don't really want to use the current endianness of the
CPU, since in theory a BE kernel could be running an LE process.  So at
the time of virtio device reset (the first thing the Linux drivers do)
we read the endianness of the interrupt entry point, which is presumably
the OS endianness.

The result looks like this.  In virtio_reset(void *opaque):

        /* We assume all devices are the same endian. */
        virtio_byteswap = virtio_get_byteswap();

And accessors like this are used in all virtio devices and routines:

	/* Initialized by virtio_get_byteswap() at any virtio device reset. */
	extern bool virtio_byteswap;
	
	static inline uint16_t virtio_lduw_phys(hwaddr pa)
	{
	    if (virtio_byteswap) {
	        return bswap16(lduw_phys(pa));
	    }
	    return lduw_phys(pa);
	    
	}
	
	static inline uint32_t virtio_ldl_phys(hwaddr pa)
	{
	    if (virtio_byteswap) {
	        return bswap32(ldl_phys(pa));
	    }
	    return ldl_phys(pa);
	}
	
	static inline uint64_t virtio_ldq_phys(hwaddr pa)
	{
	    if (virtio_byteswap) {
	        return bswap64(ldq_phys(pa));
	    }
	    return ldq_phys(pa);
	}
	
	static inline void virtio_stw_phys(hwaddr pa, uint16_t value)
	{
	    if (virtio_byteswap) {
	        stw_phys(pa, bswap16(value));
	    } else {
	        stw_phys(pa, value);
	    }
	}
	
	static inline void virtio_stl_phys(hwaddr pa, uint32_t value)
	{
	    if (virtio_byteswap) {
	        stl_phys(pa, bswap32(value));
	    } else {
	        stl_phys(pa, value);
	    }
	}
	
	static inline void virtio_stw_p(void *ptr, uint16_t v)
	{
	    if (virtio_byteswap) {
	        stw_p(ptr, bswap16(v));
	    } else {
	        stw_p(ptr, v);
	    }
	}
	
	static inline void virtio_stl_p(void *ptr, uint32_t v)
	{
	    if (virtio_byteswap) {
	        stl_p(ptr, bswap32(v));
	    } else {
	        stl_p(ptr, v);
	    }
	}
	
	static inline void virtio_stq_p(void *ptr, uint64_t v)
	{
	    if (virtio_byteswap) {
	        stq_p(ptr, bswap64(v));
	    } else {
	        stq_p(ptr, v);
	    }
	}
	
	static inline int virtio_lduw_p(const void *ptr)
	{
	    if (virtio_byteswap) {
	        return bswap16(lduw_p(ptr));
	    } else {
	        return lduw_p(ptr);
	    }
	}
	
	static inline int virtio_ldl_p(const void *ptr)
	{
	    if (virtio_byteswap) {
	        return bswap32(ldl_p(ptr));
	    } else {
	        return ldl_p(ptr);
	    }
	}
	
	static inline uint64_t virtio_ldq_p(const void *ptr)
	{
	    if (virtio_byteswap) {
	        return bswap64(ldq_p(ptr));
	    } else {
	        return ldq_p(ptr);
	    }
	}
	
	static inline uint32_t virtio_tswap32(uint32_t s)
	{
	    if (virtio_byteswap) {
	        return bswap32(tswap32(s));
	    } else {
	        return tswap32(s);
	    }
	}
	
	static inline void virtio_tswap32s(uint32_t *s)
	{
	    tswap32s(s);
	    if (virtio_byteswap) {
	        *s = bswap32(*s);
	    }
	}

There are obvious optimizations to this, such as making virtio_byteswap
a compile time constant on non-endian-ambivalent targets, but this is
the current approach.

Cheers,
Rusty.

^ permalink raw reply	[flat|nested] 86+ messages in thread
* [PATCH 0/3] virtio-mmio: handle BE guests on LE hosts
@ 2013-10-14 23:23                                 ` Rusty Russell
  0 siblings, 0 replies; 86+ messages in thread
From: Rusty Russell @ 2013-10-14 23:23 UTC (permalink / raw)
  To: linux-arm-kernel

"Michael S. Tsirkin" <mst@redhat.com> writes:
> On Mon, Oct 14, 2013 at 06:50:48PM +0200, Paolo Bonzini wrote:
>> Il 14/10/2013 17:36, Marc Zyngier ha scritto:
>> >> > 
>> >> > Devices are fine in QEMU, it's only the "generic" parts (rings) that are
>> >> > missing AFAICT.
>> > So if I understand correctly how it works, target endianness is set at
>> > compile time, and you have a BE specific QEMU?
>> 
>> Yes.  Though as Alex said, this will have to change for PPC
>> little-endian support.
>> 
>> Paolo
>
> Or we'll just say that this platform requires virtio 1.0.

To come full circle, I have implemented virtio support for BE PPC in
qemu :) And we'll be supporting pre-1.0 for the moment. I'm awaiting
some ppc-specific hooks, but it could be merged without them.

It's icky: we don't really want to use the current endianness of the
CPU, since in theory a BE kernel could be running an LE process.  So at
the time of virtio device reset (the first thing the Linux drivers do)
we read the endianness of the interrupt entry point, which is presumably
the OS endianness.

The result looks like this.  In virtio_reset(void *opaque):

        /* We assume all devices are the same endian. */
        virtio_byteswap = virtio_get_byteswap();

And accessors like this are used in all virtio devices and routines:

	/* Initialized by virtio_get_byteswap() at any virtio device reset. */
	extern bool virtio_byteswap;
	
	static inline uint16_t virtio_lduw_phys(hwaddr pa)
	{
	    if (virtio_byteswap) {
	        return bswap16(lduw_phys(pa));
	    }
	    return lduw_phys(pa);
	    
	}
	
	static inline uint32_t virtio_ldl_phys(hwaddr pa)
	{
	    if (virtio_byteswap) {
	        return bswap32(ldl_phys(pa));
	    }
	    return ldl_phys(pa);
	}
	
	static inline uint64_t virtio_ldq_phys(hwaddr pa)
	{
	    if (virtio_byteswap) {
	        return bswap64(ldq_phys(pa));
	    }
	    return ldq_phys(pa);
	}
	
	static inline void virtio_stw_phys(hwaddr pa, uint16_t value)
	{
	    if (virtio_byteswap) {
	        stw_phys(pa, bswap16(value));
	    } else {
	        stw_phys(pa, value);
	    }
	}
	
	static inline void virtio_stl_phys(hwaddr pa, uint32_t value)
	{
	    if (virtio_byteswap) {
	        stl_phys(pa, bswap32(value));
	    } else {
	        stl_phys(pa, value);
	    }
	}
	
	static inline void virtio_stw_p(void *ptr, uint16_t v)
	{
	    if (virtio_byteswap) {
	        stw_p(ptr, bswap16(v));
	    } else {
	        stw_p(ptr, v);
	    }
	}
	
	static inline void virtio_stl_p(void *ptr, uint32_t v)
	{
	    if (virtio_byteswap) {
	        stl_p(ptr, bswap32(v));
	    } else {
	        stl_p(ptr, v);
	    }
	}
	
	static inline void virtio_stq_p(void *ptr, uint64_t v)
	{
	    if (virtio_byteswap) {
	        stq_p(ptr, bswap64(v));
	    } else {
	        stq_p(ptr, v);
	    }
	}
	
	static inline int virtio_lduw_p(const void *ptr)
	{
	    if (virtio_byteswap) {
	        return bswap16(lduw_p(ptr));
	    } else {
	        return lduw_p(ptr);
	    }
	}
	
	static inline int virtio_ldl_p(const void *ptr)
	{
	    if (virtio_byteswap) {
	        return bswap32(ldl_p(ptr));
	    } else {
	        return ldl_p(ptr);
	    }
	}
	
	static inline uint64_t virtio_ldq_p(const void *ptr)
	{
	    if (virtio_byteswap) {
	        return bswap64(ldq_p(ptr));
	    } else {
	        return ldq_p(ptr);
	    }
	}
	
	static inline uint32_t virtio_tswap32(uint32_t s)
	{
	    if (virtio_byteswap) {
	        return bswap32(tswap32(s));
	    } else {
	        return tswap32(s);
	    }
	}
	
	static inline void virtio_tswap32s(uint32_t *s)
	{
	    tswap32s(s);
	    if (virtio_byteswap) {
	        *s = bswap32(*s);
	    }
	}

There are obvious optimizations to this, such as making virtio_byteswap
a compile time constant on non-endian-ambivalent targets, but this is
the current approach.

Cheers,
Rusty.

^ permalink raw reply	[flat|nested] 86+ messages in thread
* Re: [PATCH 0/3] virtio-mmio: handle BE guests on LE hosts
  2013-10-14 23:23                                 ` Rusty Russell
@ 2013-10-15  6:38                                   ` Michael S. Tsirkin
  -1 siblings, 0 replies; 86+ messages in thread
From: Michael S. Tsirkin @ 2013-10-15  6:38 UTC (permalink / raw)
  To: Rusty Russell
  Cc: linux-arm-kernel, Paolo Bonzini, Pawel Moll,
	kvm@vger.kernel.org mailing list, kvmarm

On Tue, Oct 15, 2013 at 09:53:17AM +1030, Rusty Russell wrote:
> "Michael S. Tsirkin" <mst@redhat.com> writes:
> > On Mon, Oct 14, 2013 at 06:50:48PM +0200, Paolo Bonzini wrote:
> >> Il 14/10/2013 17:36, Marc Zyngier ha scritto:
> >> >> > 
> >> >> > Devices are fine in QEMU, it's only the "generic" parts (rings) that are
> >> >> > missing AFAICT.
> >> > So if I understand correctly how it works, target endianness is set at
> >> > compile time, and you have a BE specific QEMU?
> >> 
> >> Yes.  Though as Alex said, this will have to change for PPC
> >> little-endian support.
> >> 
> >> Paolo
> >
> > Or we'll just say that this platform requires virtio 1.0.
> 
> To come full circle, I have implemented virtio support for BE PPC in
> qemu :) And we'll be supporting pre-1.0 for the moment. I'm awaiting
> some ppc-specific hooks, but it could be merged without them.
> 
> It's icky: we don't really want to use the current endianness of the
> CPU, since in theory a BE kernel could be running an LE process.  So at
> the time of virtio device reset (the first thing the Linux drivers do)
> we read the endianness of the interrupt entry point, which is presumably
> the OS endianness.

Indeed, ick.
The next thing after reset is status write:

        /* We always start by resetting the device, in case a previous
         * driver messed it up.  This also tests that code path a
         * little. */
        dev->config->reset(dev);

        /* Acknowledge that we've seen the device. */
        add_status(dev, VIRTIO_CONFIG_S_ACKNOWLEDGE);

and mmio happens to use a 32 bit register for this:

	static void vm_set_status(struct virtio_device *vdev, u8 status)
	{
		struct virtio_mmio_device *vm_dev = to_virtio_mmio_device(vdev);

		/* We should never be setting status to 0. */
		BUG_ON(status == 0);

		writel(status, vm_dev->base + VIRTIO_MMIO_STATUS);
	}

so if we only care about mmio, we can use the status write
to detect endian-ness: low byte set means LE, high byte set
means BE, which seems cleaner.

Hmm?

-- 
MST

^ permalink raw reply	[flat|nested] 86+ messages in thread
* [PATCH 0/3] virtio-mmio: handle BE guests on LE hosts
@ 2013-10-15  6:38                                   ` Michael S. Tsirkin
  0 siblings, 0 replies; 86+ messages in thread
From: Michael S. Tsirkin @ 2013-10-15  6:38 UTC (permalink / raw)
  To: linux-arm-kernel

On Tue, Oct 15, 2013 at 09:53:17AM +1030, Rusty Russell wrote:
> "Michael S. Tsirkin" <mst@redhat.com> writes:
> > On Mon, Oct 14, 2013 at 06:50:48PM +0200, Paolo Bonzini wrote:
> >> Il 14/10/2013 17:36, Marc Zyngier ha scritto:
> >> >> > 
> >> >> > Devices are fine in QEMU, it's only the "generic" parts (rings) that are
> >> >> > missing AFAICT.
> >> > So if I understand correctly how it works, target endianness is set at
> >> > compile time, and you have a BE specific QEMU?
> >> 
> >> Yes.  Though as Alex said, this will have to change for PPC
> >> little-endian support.
> >> 
> >> Paolo
> >
> > Or we'll just say that this platform requires virtio 1.0.
> 
> To come full circle, I have implemented virtio support for BE PPC in
> qemu :) And we'll be supporting pre-1.0 for the moment. I'm awaiting
> some ppc-specific hooks, but it could be merged without them.
> 
> It's icky: we don't really want to use the current endianness of the
> CPU, since in theory a BE kernel could be running an LE process.  So at
> the time of virtio device reset (the first thing the Linux drivers do)
> we read the endianness of the interrupt entry point, which is presumably
> the OS endianness.

Indeed, ick.
The next thing after reset is status write:

        /* We always start by resetting the device, in case a previous
         * driver messed it up.  This also tests that code path a
         * little. */
        dev->config->reset(dev);

        /* Acknowledge that we've seen the device. */
        add_status(dev, VIRTIO_CONFIG_S_ACKNOWLEDGE);

and mmio happens to use a 32 bit register for this:

	static void vm_set_status(struct virtio_device *vdev, u8 status)
	{
		struct virtio_mmio_device *vm_dev = to_virtio_mmio_device(vdev);

		/* We should never be setting status to 0. */
		BUG_ON(status == 0);

		writel(status, vm_dev->base + VIRTIO_MMIO_STATUS);
	}

so if we only care about mmio, we can use the status write
to detect endian-ness: low byte set means LE, high byte set
means BE, which seems cleaner.

Hmm?

-- 
MST

^ permalink raw reply	[flat|nested] 86+ messages in thread
* Re: [PATCH 0/3] virtio-mmio: handle BE guests on LE hosts
  2013-10-15  6:38                                   ` Michael S. Tsirkin
@ 2013-10-15  9:19                                     ` Marc Zyngier
  -1 siblings, 0 replies; 86+ messages in thread
From: Marc Zyngier @ 2013-10-15  9:19 UTC (permalink / raw)
  To: Michael S. Tsirkin
  Cc: Rusty Russell, linux-arm-kernel, Paolo Bonzini, Pawel Moll,
	kvm@vger.kernel.org mailing list, kvmarm

On 15/10/13 07:38, Michael S. Tsirkin wrote:
> On Tue, Oct 15, 2013 at 09:53:17AM +1030, Rusty Russell wrote:
>> "Michael S. Tsirkin" <mst@redhat.com> writes:
>>> On Mon, Oct 14, 2013 at 06:50:48PM +0200, Paolo Bonzini wrote:
>>>> Il 14/10/2013 17:36, Marc Zyngier ha scritto:
>>>>>>>
>>>>>>> Devices are fine in QEMU, it's only the "generic" parts (rings) that are
>>>>>>> missing AFAICT.
>>>>> So if I understand correctly how it works, target endianness is set at
>>>>> compile time, and you have a BE specific QEMU?
>>>>
>>>> Yes.  Though as Alex said, this will have to change for PPC
>>>> little-endian support.
>>>>
>>>> Paolo
>>>
>>> Or we'll just say that this platform requires virtio 1.0.
>>
>> To come full circle, I have implemented virtio support for BE PPC in
>> qemu :) And we'll be supporting pre-1.0 for the moment. I'm awaiting
>> some ppc-specific hooks, but it could be merged without them.
>>
>> It's icky: we don't really want to use the current endianness of the
>> CPU, since in theory a BE kernel could be running an LE process.  So at
>> the time of virtio device reset (the first thing the Linux drivers do)
>> we read the endianness of the interrupt entry point, which is presumably
>> the OS endianness.
> 
> Indeed, ick.
> The next thing after reset is status write:
> 
>         /* We always start by resetting the device, in case a previous
>          * driver messed it up.  This also tests that code path a
>          * little. */
>         dev->config->reset(dev);
> 
>         /* Acknowledge that we've seen the device. */
>         add_status(dev, VIRTIO_CONFIG_S_ACKNOWLEDGE);
> 
> and mmio happens to use a 32 bit register for this:
> 
> 	static void vm_set_status(struct virtio_device *vdev, u8 status)
> 	{
> 		struct virtio_mmio_device *vm_dev = to_virtio_mmio_device(vdev);
> 
> 		/* We should never be setting status to 0. */
> 		BUG_ON(status == 0);
> 
> 		writel(status, vm_dev->base + VIRTIO_MMIO_STATUS);
> 	}
> 
> so if we only care about mmio, we can use the status write
> to detect endian-ness: low byte set means LE, high byte set
> means BE, which seems cleaner.
> 
> Hmm?

Don't think it works. MMIO is LE only, so the interesting byte will
always be at the same location (writel will do the swap).

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


^ permalink raw reply	[flat|nested] 86+ messages in thread
* [PATCH 0/3] virtio-mmio: handle BE guests on LE hosts
@ 2013-10-15  9:19                                     ` Marc Zyngier
  0 siblings, 0 replies; 86+ messages in thread
From: Marc Zyngier @ 2013-10-15  9:19 UTC (permalink / raw)
  To: linux-arm-kernel

On 15/10/13 07:38, Michael S. Tsirkin wrote:
> On Tue, Oct 15, 2013 at 09:53:17AM +1030, Rusty Russell wrote:
>> "Michael S. Tsirkin" <mst@redhat.com> writes:
>>> On Mon, Oct 14, 2013 at 06:50:48PM +0200, Paolo Bonzini wrote:
>>>> Il 14/10/2013 17:36, Marc Zyngier ha scritto:
>>>>>>>
>>>>>>> Devices are fine in QEMU, it's only the "generic" parts (rings) that are
>>>>>>> missing AFAICT.
>>>>> So if I understand correctly how it works, target endianness is set at
>>>>> compile time, and you have a BE specific QEMU?
>>>>
>>>> Yes.  Though as Alex said, this will have to change for PPC
>>>> little-endian support.
>>>>
>>>> Paolo
>>>
>>> Or we'll just say that this platform requires virtio 1.0.
>>
>> To come full circle, I have implemented virtio support for BE PPC in
>> qemu :) And we'll be supporting pre-1.0 for the moment. I'm awaiting
>> some ppc-specific hooks, but it could be merged without them.
>>
>> It's icky: we don't really want to use the current endianness of the
>> CPU, since in theory a BE kernel could be running an LE process.  So at
>> the time of virtio device reset (the first thing the Linux drivers do)
>> we read the endianness of the interrupt entry point, which is presumably
>> the OS endianness.
> 
> Indeed, ick.
> The next thing after reset is status write:
> 
>         /* We always start by resetting the device, in case a previous
>          * driver messed it up.  This also tests that code path a
>          * little. */
>         dev->config->reset(dev);
> 
>         /* Acknowledge that we've seen the device. */
>         add_status(dev, VIRTIO_CONFIG_S_ACKNOWLEDGE);
> 
> and mmio happens to use a 32 bit register for this:
> 
> 	static void vm_set_status(struct virtio_device *vdev, u8 status)
> 	{
> 		struct virtio_mmio_device *vm_dev = to_virtio_mmio_device(vdev);
> 
> 		/* We should never be setting status to 0. */
> 		BUG_ON(status == 0);
> 
> 		writel(status, vm_dev->base + VIRTIO_MMIO_STATUS);
> 	}
> 
> so if we only care about mmio, we can use the status write
> to detect endian-ness: low byte set means LE, high byte set
> means BE, which seems cleaner.
> 
> Hmm?

Don't think it works. MMIO is LE only, so the interesting byte will
always be at the same location (writel will do the swap).

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

^ permalink raw reply	[flat|nested] 86+ messages in thread
* Re: [PATCH 0/3] virtio-mmio: handle BE guests on LE hosts
  2013-10-14 12:36     ` Marc Zyngier
@ 2013-10-17  0:27       ` Rusty Russell
  -1 siblings, 0 replies; 86+ messages in thread
From: Rusty Russell @ 2013-10-17  0:27 UTC (permalink / raw)
  To: Marc Zyngier
  Cc: linux-arm-kernel, kvmarm, kvm, Michael S. Tsirkin, Pawel Moll

Marc Zyngier <marc.zyngier@arm.com> writes:
> On 14/10/13 09:21, Rusty Russell wrote:
>> Marc Zyngier <marc.zyngier@arm.com> writes:
>>> This small patch series adds just enough kernel infrastructure and
>>> fixes to allow a BE guest to use virtio-mmio on a LE host, provided
>>> that the host actually supports such madness.
>>>
>>> This has been tested on arm64, with some fixes to KVM and a set of
>>> changes to kvmtool, both which I am posting separately.
>> 
>> OK, so I already have a patch which supports config space accessors.
>> I've posted the series below, and since you want it, I'll put it in
>> virtio-next after more testing and rebasing...
>
> Yes, that's definitely something I'd like to see being merged, as it
> would allow me to drop a significant chunk of changes.

OK, I've just put them into virtio-next.

>> But we won't be using feature bits for endianness, since we're defining
>> endian in the 1.0 spec (whether LE-everywhere or LE-for-mmio-and-PCI is
>> still debated).  Since we need to guess for backwards compat anyway,
>> let's keep doing this until v1.0?
>
> When is 1.0 going to happen? When will actual implementation of drivers
> and devices show up in my favourite platform emulation?

1.0 won't be finalized until early next year.  We aim to publish the
first draft next month, but noone should finalize implementations until
the feedback process is complete.

> Having a grand plan for the future is great, but I need something
> working right now, or at least fairly soonish... And I need it to be
> backward compatible, as none of the above is going to show up overnight.

Well, mmio BE won't work at all right now due to the signature check
being wrong in Linux.  So there are two choices: (1) fix it, and use
heuristics to figure out if the guest is BE, or (2) say there's no BE
mmio, and wait for 1.0.

I don't know what your timeline is: you might need to chat with Pawel
internally.

BTW, for qemu and PPC (though virtio-pci, not mmio) I look at the guest
interrupt delivery endian bit upon any virtio device reset to guess
endian.  Since Linux guests reset the device before doing anything else,
this works well, and supports crazy stuff like "kexec a kernel in the
other endian".

>> (Yeah, I made this mess with "native endian".  I promise I have learnt
>> my lesson).
>
> Paracetamol bill coming you way... ;-)

Now I just have to figure out where to send my equivalent bill...

Cheers,
Rusty.

^ permalink raw reply	[flat|nested] 86+ messages in thread
* [PATCH 0/3] virtio-mmio: handle BE guests on LE hosts
@ 2013-10-17  0:27       ` Rusty Russell
  0 siblings, 0 replies; 86+ messages in thread
From: Rusty Russell @ 2013-10-17  0:27 UTC (permalink / raw)
  To: linux-arm-kernel

Marc Zyngier <marc.zyngier@arm.com> writes:
> On 14/10/13 09:21, Rusty Russell wrote:
>> Marc Zyngier <marc.zyngier@arm.com> writes:
>>> This small patch series adds just enough kernel infrastructure and
>>> fixes to allow a BE guest to use virtio-mmio on a LE host, provided
>>> that the host actually supports such madness.
>>>
>>> This has been tested on arm64, with some fixes to KVM and a set of
>>> changes to kvmtool, both which I am posting separately.
>> 
>> OK, so I already have a patch which supports config space accessors.
>> I've posted the series below, and since you want it, I'll put it in
>> virtio-next after more testing and rebasing...
>
> Yes, that's definitely something I'd like to see being merged, as it
> would allow me to drop a significant chunk of changes.

OK, I've just put them into virtio-next.

>> But we won't be using feature bits for endianness, since we're defining
>> endian in the 1.0 spec (whether LE-everywhere or LE-for-mmio-and-PCI is
>> still debated).  Since we need to guess for backwards compat anyway,
>> let's keep doing this until v1.0?
>
> When is 1.0 going to happen? When will actual implementation of drivers
> and devices show up in my favourite platform emulation?

1.0 won't be finalized until early next year.  We aim to publish the
first draft next month, but noone should finalize implementations until
the feedback process is complete.

> Having a grand plan for the future is great, but I need something
> working right now, or at least fairly soonish... And I need it to be
> backward compatible, as none of the above is going to show up overnight.

Well, mmio BE won't work at all right now due to the signature check
being wrong in Linux.  So there are two choices: (1) fix it, and use
heuristics to figure out if the guest is BE, or (2) say there's no BE
mmio, and wait for 1.0.

I don't know what your timeline is: you might need to chat with Pawel
internally.

BTW, for qemu and PPC (though virtio-pci, not mmio) I look at the guest
interrupt delivery endian bit upon any virtio device reset to guess
endian.  Since Linux guests reset the device before doing anything else,
this works well, and supports crazy stuff like "kexec a kernel in the
other endian".

>> (Yeah, I made this mess with "native endian".  I promise I have learnt
>> my lesson).
>
> Paracetamol bill coming you way... ;-)

Now I just have to figure out where to send my equivalent bill...

Cheers,
Rusty.

^ permalink raw reply	[flat|nested] 86+ messages in thread
* Re: [PATCH 2/3] virtio: mmio: fix signature checking for BE guests
  2013-10-14  9:11       ` Rusty Russell
@ 2013-11-05  3:36         ` Rusty Russell
  -1 siblings, 0 replies; 86+ messages in thread
From: Rusty Russell @ 2013-11-05  3:36 UTC (permalink / raw)
  To: Pawel Moll, Marc Zyngier
  Cc: linux-arm-kernel, kvmarm, kvm, Michael S. Tsirkin

Rusty Russell <rusty@rustcorp.com.au> writes:
> Pawel Moll <pawel.moll@arm.com> writes:
>> On Fri, 2013-10-11 at 15:36 +0100, Marc Zyngier wrote:
>>> As virtio-mmio config registers are specified to be little-endian,
>>> using readl() to read the magic value and then memcmp() to check it
>>> fails on BE (as readl() has an implicit swab).
>>> 
>>> Fix it by encoding the magic value as an integer instead of a string.
>>> 
>>> Cc: Rusty Russell <rusty@rustcorp.com.au>
>>> Cc: Michael S. Tsirkin <mst@redhat.com>
>>> Cc: Pawel Moll <pawel.moll@arm.com>
>>> Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
>>> ---
>>>  drivers/virtio/virtio_mmio.c | 2 +-
>>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>> 
>>> diff --git a/drivers/virtio/virtio_mmio.c b/drivers/virtio/virtio_mmio.c
>>> index 1ba0d68..57f24fd 100644
>>> --- a/drivers/virtio/virtio_mmio.c
>>> +++ b/drivers/virtio/virtio_mmio.c
>>> @@ -470,7 +470,7 @@ static int virtio_mmio_probe(struct platform_device *pdev)
>>>  
>>>  	/* Check magic value */
>>>  	magic = readl(vm_dev->base + VIRTIO_MMIO_MAGIC_VALUE);
>>> -	if (memcmp(&magic, "virt", 4) != 0) {
>>> +	if (magic != ('v' | 'i' << 8 | 'r' << 16 | 't' << 24)) {
>>>  		dev_warn(&pdev->dev, "Wrong magic value 0x%08lx!\n", magic);
>>>  		return -ENODEV;
>>>  	}
>>
>> The new spec will clarify this:
>>
>> * 0x000 | R | MagicValue
>>   Magic value. Must be 0x74726976 (a Little Endian equivalent
>>   of a "virt" string).
>>
>> ... but I like the 'v'i'r't' characters still being there :-)
>>
>> Acked-by: Pawel Moll <pawel.moll@arm.com>
>
> Applied, thanks.

OK, I *said* applied, but left it in my pending queue.

Pawel, do you want to open this can of worms?  If so, I'l merge this
now.

Cheers,
Rusty.

^ permalink raw reply	[flat|nested] 86+ messages in thread
* [PATCH 2/3] virtio: mmio: fix signature checking for BE guests
@ 2013-11-05  3:36         ` Rusty Russell
  0 siblings, 0 replies; 86+ messages in thread
From: Rusty Russell @ 2013-11-05  3:36 UTC (permalink / raw)
  To: linux-arm-kernel

Rusty Russell <rusty@rustcorp.com.au> writes:
> Pawel Moll <pawel.moll@arm.com> writes:
>> On Fri, 2013-10-11 at 15:36 +0100, Marc Zyngier wrote:
>>> As virtio-mmio config registers are specified to be little-endian,
>>> using readl() to read the magic value and then memcmp() to check it
>>> fails on BE (as readl() has an implicit swab).
>>> 
>>> Fix it by encoding the magic value as an integer instead of a string.
>>> 
>>> Cc: Rusty Russell <rusty@rustcorp.com.au>
>>> Cc: Michael S. Tsirkin <mst@redhat.com>
>>> Cc: Pawel Moll <pawel.moll@arm.com>
>>> Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
>>> ---
>>>  drivers/virtio/virtio_mmio.c | 2 +-
>>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>> 
>>> diff --git a/drivers/virtio/virtio_mmio.c b/drivers/virtio/virtio_mmio.c
>>> index 1ba0d68..57f24fd 100644
>>> --- a/drivers/virtio/virtio_mmio.c
>>> +++ b/drivers/virtio/virtio_mmio.c
>>> @@ -470,7 +470,7 @@ static int virtio_mmio_probe(struct platform_device *pdev)
>>>  
>>>  	/* Check magic value */
>>>  	magic = readl(vm_dev->base + VIRTIO_MMIO_MAGIC_VALUE);
>>> -	if (memcmp(&magic, "virt", 4) != 0) {
>>> +	if (magic != ('v' | 'i' << 8 | 'r' << 16 | 't' << 24)) {
>>>  		dev_warn(&pdev->dev, "Wrong magic value 0x%08lx!\n", magic);
>>>  		return -ENODEV;
>>>  	}
>>
>> The new spec will clarify this:
>>
>> * 0x000 | R | MagicValue
>>   Magic value. Must be 0x74726976 (a Little Endian equivalent
>>   of a "virt" string).
>>
>> ... but I like the 'v'i'r't' characters still being there :-)
>>
>> Acked-by: Pawel Moll <pawel.moll@arm.com>
>
> Applied, thanks.

OK, I *said* applied, but left it in my pending queue.

Pawel, do you want to open this can of worms?  If so, I'l merge this
now.

Cheers,
Rusty.

^ permalink raw reply	[flat|nested] 86+ messages in thread
* Re: [PATCH 2/3] virtio: mmio: fix signature checking for BE guests
  2013-11-05  3:36         ` Rusty Russell
@ 2013-11-05 10:45           ` Pawel Moll
  -1 siblings, 0 replies; 86+ messages in thread
From: Pawel Moll @ 2013-11-05 10:45 UTC (permalink / raw)
  To: Rusty Russell
  Cc: Marc Zyngier, linux-arm-kernel, kvmarm, kvm, Michael S. Tsirkin

On Tue, 2013-11-05 at 03:36 +0000, Rusty Russell wrote:
> Rusty Russell <rusty@rustcorp.com.au> writes:
> > Pawel Moll <pawel.moll@arm.com> writes:
> >> On Fri, 2013-10-11 at 15:36 +0100, Marc Zyngier wrote:
> >>> As virtio-mmio config registers are specified to be little-endian,
> >>> using readl() to read the magic value and then memcmp() to check it
> >>> fails on BE (as readl() has an implicit swab).
> >>> 
> >>> Fix it by encoding the magic value as an integer instead of a string.
> >>> 
> >>> Cc: Rusty Russell <rusty@rustcorp.com.au>
> >>> Cc: Michael S. Tsirkin <mst@redhat.com>
> >>> Cc: Pawel Moll <pawel.moll@arm.com>
> >>> Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
> >>> ---
> >>>  drivers/virtio/virtio_mmio.c | 2 +-
> >>>  1 file changed, 1 insertion(+), 1 deletion(-)
> >>> 
> >>> diff --git a/drivers/virtio/virtio_mmio.c b/drivers/virtio/virtio_mmio.c
> >>> index 1ba0d68..57f24fd 100644
> >>> --- a/drivers/virtio/virtio_mmio.c
> >>> +++ b/drivers/virtio/virtio_mmio.c
> >>> @@ -470,7 +470,7 @@ static int virtio_mmio_probe(struct platform_device *pdev)
> >>>  
> >>>  	/* Check magic value */
> >>>  	magic = readl(vm_dev->base + VIRTIO_MMIO_MAGIC_VALUE);
> >>> -	if (memcmp(&magic, "virt", 4) != 0) {
> >>> +	if (magic != ('v' | 'i' << 8 | 'r' << 16 | 't' << 24)) {
> >>>  		dev_warn(&pdev->dev, "Wrong magic value 0x%08lx!\n", magic);
> >>>  		return -ENODEV;
> >>>  	}
> >>
> >> The new spec will clarify this:
> >>
> >> * 0x000 | R | MagicValue
> >>   Magic value. Must be 0x74726976 (a Little Endian equivalent
> >>   of a "virt" string).
> >>
> >> ... but I like the 'v'i'r't' characters still being there :-)
> >>
> >> Acked-by: Pawel Moll <pawel.moll@arm.com>
> >
> > Applied, thanks.
> 
> OK, I *said* applied, but left it in my pending queue.
> 
> Pawel, do you want to open this can of worms?  If so, I'l merge this
> now.

This particular can is empty - all worms already escaped :-)

It's a bugfix, not changing anything for the existing hosts. So - yes
please, do merge this one.

Thanks

Paweł



^ permalink raw reply	[flat|nested] 86+ messages in thread
* [PATCH 2/3] virtio: mmio: fix signature checking for BE guests
@ 2013-11-05 10:45           ` Pawel Moll
  0 siblings, 0 replies; 86+ messages in thread
From: Pawel Moll @ 2013-11-05 10:45 UTC (permalink / raw)
  To: linux-arm-kernel

On Tue, 2013-11-05 at 03:36 +0000, Rusty Russell wrote:
> Rusty Russell <rusty@rustcorp.com.au> writes:
> > Pawel Moll <pawel.moll@arm.com> writes:
> >> On Fri, 2013-10-11 at 15:36 +0100, Marc Zyngier wrote:
> >>> As virtio-mmio config registers are specified to be little-endian,
> >>> using readl() to read the magic value and then memcmp() to check it
> >>> fails on BE (as readl() has an implicit swab).
> >>> 
> >>> Fix it by encoding the magic value as an integer instead of a string.
> >>> 
> >>> Cc: Rusty Russell <rusty@rustcorp.com.au>
> >>> Cc: Michael S. Tsirkin <mst@redhat.com>
> >>> Cc: Pawel Moll <pawel.moll@arm.com>
> >>> Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
> >>> ---
> >>>  drivers/virtio/virtio_mmio.c | 2 +-
> >>>  1 file changed, 1 insertion(+), 1 deletion(-)
> >>> 
> >>> diff --git a/drivers/virtio/virtio_mmio.c b/drivers/virtio/virtio_mmio.c
> >>> index 1ba0d68..57f24fd 100644
> >>> --- a/drivers/virtio/virtio_mmio.c
> >>> +++ b/drivers/virtio/virtio_mmio.c
> >>> @@ -470,7 +470,7 @@ static int virtio_mmio_probe(struct platform_device *pdev)
> >>>  
> >>>  	/* Check magic value */
> >>>  	magic = readl(vm_dev->base + VIRTIO_MMIO_MAGIC_VALUE);
> >>> -	if (memcmp(&magic, "virt", 4) != 0) {
> >>> +	if (magic != ('v' | 'i' << 8 | 'r' << 16 | 't' << 24)) {
> >>>  		dev_warn(&pdev->dev, "Wrong magic value 0x%08lx!\n", magic);
> >>>  		return -ENODEV;
> >>>  	}
> >>
> >> The new spec will clarify this:
> >>
> >> * 0x000 | R | MagicValue
> >>   Magic value. Must be 0x74726976 (a Little Endian equivalent
> >>   of a "virt" string).
> >>
> >> ... but I like the 'v'i'r't' characters still being there :-)
> >>
> >> Acked-by: Pawel Moll <pawel.moll@arm.com>
> >
> > Applied, thanks.
> 
> OK, I *said* applied, but left it in my pending queue.
> 
> Pawel, do you want to open this can of worms?  If so, I'l merge this
> now.

This particular can is empty - all worms already escaped :-)

It's a bugfix, not changing anything for the existing hosts. So - yes
please, do merge this one.

Thanks

Pawe?

^ permalink raw reply	[flat|nested] 86+ messages in thread
* Re: [PATCH 2/3] virtio: mmio: fix signature checking for BE guests
  2013-11-05 10:45           ` Pawel Moll
@ 2013-11-07  0:36             ` Rusty Russell
  -1 siblings, 0 replies; 86+ messages in thread
From: Rusty Russell @ 2013-11-07  0:36 UTC (permalink / raw)
  To: Pawel Moll
  Cc: Marc Zyngier, linux-arm-kernel, kvmarm, kvm, Michael S. Tsirkin

Pawel Moll <pawel.moll@arm.com> writes:
> On Tue, 2013-11-05 at 03:36 +0000, Rusty Russell wrote:
> This particular can is empty - all worms already escaped :-)

I just thought that if you wait for 1.0, it will always be
little-endian, and if the current qemu only supported little-endian your
life might be simpler inside qemu.  But I'm sure Marc doesn't want to
wait :)

> It's a bugfix, not changing anything for the existing hosts. So - yes
> please, do merge this one.

Done, thanks.

Cheers,
Rusty.

^ permalink raw reply	[flat|nested] 86+ messages in thread
* [PATCH 2/3] virtio: mmio: fix signature checking for BE guests
@ 2013-11-07  0:36             ` Rusty Russell
  0 siblings, 0 replies; 86+ messages in thread
From: Rusty Russell @ 2013-11-07  0:36 UTC (permalink / raw)
  To: linux-arm-kernel

Pawel Moll <pawel.moll@arm.com> writes:
> On Tue, 2013-11-05 at 03:36 +0000, Rusty Russell wrote:
> This particular can is empty - all worms already escaped :-)

I just thought that if you wait for 1.0, it will always be
little-endian, and if the current qemu only supported little-endian your
life might be simpler inside qemu.  But I'm sure Marc doesn't want to
wait :)

> It's a bugfix, not changing anything for the existing hosts. So - yes
> please, do merge this one.

Done, thanks.

Cheers,
Rusty.

^ permalink raw reply	[flat|nested] 86+ messages in thread
end of thread, other threads:[~2013-11-07  0:55 UTC | newest]

Thread overview: 86+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-10-11 14:36 [PATCH 0/3] virtio-mmio: handle BE guests on LE hosts Marc Zyngier
2013-10-11 14:36 ` Marc Zyngier
2013-10-11 14:36 ` [PATCH 1/3] virtio: let the guest report its endianess if advertized by the host Marc Zyngier
2013-10-11 14:36   ` Marc Zyngier
2013-10-11 14:36 ` [PATCH 2/3] virtio: mmio: fix signature checking for BE guests Marc Zyngier
2013-10-11 14:36   ` Marc Zyngier
2013-10-14  8:46   ` Pawel Moll
2013-10-14  8:46     ` Pawel Moll
2013-10-14  9:11     ` Rusty Russell
2013-10-14  9:11       ` Rusty Russell
2013-11-05  3:36       ` Rusty Russell
2013-11-05  3:36         ` Rusty Russell
2013-11-05 10:45         ` Pawel Moll
2013-11-05 10:45           ` Pawel Moll
2013-11-07  0:36           ` Rusty Russell
2013-11-07  0:36             ` Rusty Russell
2013-10-11 14:36 ` [PATCH 3/3] virtio: mmio: access configuration space as little-endian Marc Zyngier
2013-10-11 14:36   ` Marc Zyngier
2013-10-14  8:44   ` Pawel Moll
2013-10-14  8:44     ` Pawel Moll
2013-10-12 18:28 ` [PATCH 0/3] virtio-mmio: handle BE guests on LE hosts Michael S. Tsirkin
2013-10-12 18:28   ` Michael S. Tsirkin
2013-10-14  8:24   ` Marc Zyngier
2013-10-14  8:24     ` Marc Zyngier
2013-10-14  8:59     ` Michael S. Tsirkin
2013-10-14  8:59       ` Michael S. Tsirkin
2013-10-14  9:04       ` Pawel Moll
2013-10-14  9:04         ` Pawel Moll
2013-10-14 10:46         ` Michael S. Tsirkin
2013-10-14 10:46           ` Michael S. Tsirkin
2013-10-14 10:50           ` Pawel Moll
2013-10-14 10:50             ` Pawel Moll
2013-10-14 23:02             ` Rusty Russell
2013-10-14 23:02               ` Rusty Russell
2013-10-14  9:13     ` Rusty Russell
2013-10-14  9:13       ` Rusty Russell
2013-10-14  8:21 ` Rusty Russell
2013-10-14  8:21   ` Rusty Russell
2013-10-14 12:36   ` Marc Zyngier
2013-10-14 12:36     ` Marc Zyngier
2013-10-14 12:51     ` Michael S. Tsirkin
2013-10-14 12:51       ` Michael S. Tsirkin
2013-10-17  0:27     ` Rusty Russell
2013-10-17  0:27       ` Rusty Russell
2013-10-14 13:03 ` Paolo Bonzini
2013-10-14 13:03   ` Paolo Bonzini
2013-10-14 13:10   ` Alexander Graf
2013-10-14 13:10     ` Alexander Graf
2013-10-14 13:13     ` Paolo Bonzini
2013-10-14 13:13       ` Paolo Bonzini
2013-10-14 13:24     ` Marc Zyngier
2013-10-14 13:24       ` Marc Zyngier
2013-10-14 13:29       ` Paolo Bonzini
2013-10-14 13:29         ` Paolo Bonzini
2013-10-14 13:39       ` Alexander Graf
2013-10-14 13:39         ` Alexander Graf
2013-10-14 13:49         ` Marc Zyngier
2013-10-14 13:49           ` Marc Zyngier
2013-10-14 14:05           ` Michael S. Tsirkin
2013-10-14 14:05             ` Michael S. Tsirkin
2013-10-14 14:13             ` Marc Zyngier
2013-10-14 14:13               ` Marc Zyngier
2013-10-14 14:16               ` Alexander Graf
2013-10-14 14:16                 ` Alexander Graf
2013-10-14 14:52                 ` Marc Zyngier
2013-10-14 14:52                   ` Marc Zyngier
2013-10-14 14:56                   ` Paolo Bonzini
2013-10-14 14:56                     ` Paolo Bonzini
2013-10-14 15:12                     ` Marc Zyngier
2013-10-14 15:12                       ` Marc Zyngier
2013-10-14 15:22                       ` Paolo Bonzini
2013-10-14 15:22                         ` Paolo Bonzini
2013-10-14 15:36                         ` Marc Zyngier
2013-10-14 15:36                           ` Marc Zyngier
2013-10-14 16:50                           ` Paolo Bonzini
2013-10-14 16:50                             ` Paolo Bonzini
2013-10-14 17:10                             ` Michael S. Tsirkin
2013-10-14 17:10                               ` Michael S. Tsirkin
2013-10-14 23:23                               ` Rusty Russell
2013-10-14 23:23                                 ` Rusty Russell
2013-10-15  6:38                                 ` Michael S. Tsirkin
2013-10-15  6:38                                   ` Michael S. Tsirkin
2013-10-15  9:19                                   ` Marc Zyngier
2013-10-15  9:19                                     ` Marc Zyngier
2013-10-14 15:45                         ` Anup Patel
2013-10-14 15:45                           ` Anup Patel

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.