alacrityvm-next: clearly specify ioq endianness

alacrityvm-next: clearly specify ioq endianness

[Ira W. Snyder]

This is a respin of the previous patch series against alacrityvm.git's
linux-next branch, to facilitate easier merging with the linux-next tree.

The SHM_SIGNAL and IOQ code is meant to be used for communication channels
between systems with different bit widths and operating systems.
Conceivably, this could include systems with different endianness as well,
for example, using QEMU on x86 to emulate a powerpc.

This patch series annotates all shared structures as little endian, and
fixes up all users. This means that there is no overhead in the most common
case, x86-on-x86. The byteswaps compile out.

[RESEND: added LKML to CC per Greg Haskins' request]

 drivers/net/vbus-enet.c    |   36 ++++++++++++++++++------------------
 drivers/vbus/bus-proxy.c   |    2 +-
 drivers/vbus/pci-bridge.c  |   18 +++++++++---------
 include/linux/ioq.h        |   24 ++++++++++++------------
 include/linux/shm_signal.h |    8 ++++----
 lib/ioq.c                  |   28 ++++++++++++++++------------
 6 files changed, 60 insertions(+), 56 deletions(-)

Thanks,
Ira

[PATCH] vbus: fix lots of sparse "dubious signed bitfield" warnings

[Ira W. Snyder]

The sparse utility gave tons of warnings about signed bitfields. A simple
inspection shows that they are all used as booleans, so convert them to the
correct type.

Signed-off-by: Ira W. Snyder <iws@ovro.caltech.edu>
---
 drivers/vbus/pci-bridge.c |    2 +-
 include/linux/ioq.h       |    6 +++---
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/vbus/pci-bridge.c b/drivers/vbus/pci-bridge.c
index 9e37df1..aa683ae 100644
--- a/drivers/vbus/pci-bridge.c
+++ b/drivers/vbus/pci-bridge.c
@@ -43,7 +43,7 @@ struct vbus_pci {
 	struct vbus_pci_regs     *regs;
 	struct vbus_pci_signals  *signals;
 	int                       irq;
-	int                       enabled:1;
+	bool                      enabled;
 	struct {
 		struct dentry    *fs;
 		int               events;
diff --git a/include/linux/ioq.h b/include/linux/ioq.h
index eba1021..f04dfb4 100644
--- a/include/linux/ioq.h
+++ b/include/linux/ioq.h
@@ -112,9 +112,9 @@ struct ioq_iterator {
 	struct ioq_ring_idx   *idx;
 	u32                    pos;
 	struct ioq_ring_desc  *desc;
-	int                    update:1;
-	int                    dualidx:1;
-	int                    flipowner:1;
+	bool                   update;
+	bool                   dualidx;
+	bool                   flipowner;
 };
 
 struct ioq_notifier {
-- 
1.5.4.3

[PATCH] vbus: pci-bridge: fix sparse warnings

[Ira W. Snyder]

The sparse tool caught many missing static declarations. Add them.

There are still many address space errors, but I'm unsure about the best
way to fix these.

Signed-off-by: Ira W. Snyder <iws@ovro.caltech.edu>
---
 drivers/vbus/pci-bridge.c |   10 +++++-----
 1 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/vbus/pci-bridge.c b/drivers/vbus/pci-bridge.c
index aa683ae..078b8f4 100644
--- a/drivers/vbus/pci-bridge.c
+++ b/drivers/vbus/pci-bridge.c
@@ -66,7 +66,7 @@ struct vbus_pci_device {
 	struct work_struct       drop;
 };
 
-DEFINE_PER_CPU(struct vbus_pci_fastcall_desc, vbus_pci_percpu_fastcall)
+static DEFINE_PER_CPU(struct vbus_pci_fastcall_desc, vbus_pci_percpu_fastcall)
 ____cacheline_aligned;
 
 /*
@@ -123,7 +123,7 @@ vbus_pci_buscall(unsigned long nr, void *data, unsigned long len)
 	return ret;
 }
 
-struct vbus_pci_device *
+static struct vbus_pci_device *
 to_dev(struct vbus_device_proxy *vdev)
 {
 	return container_of(vdev, struct vbus_pci_device, vdev);
@@ -412,7 +412,7 @@ vbus_pci_device_release(struct vbus_device_proxy *vdev)
 	kfree(_dev);
 }
 
-struct vbus_device_proxy_ops vbus_pci_device_ops = {
+static struct vbus_device_proxy_ops vbus_pci_device_ops = {
 	.open    = vbus_pci_device_open,
 	.close   = vbus_pci_device_close,
 	.shm     = vbus_pci_device_shm,
@@ -667,7 +667,7 @@ static struct ioq_notifier eventq_notifier = {
 };
 
 /* Injected whenever the host issues an ioq_signal() on the eventq */
-irqreturn_t
+static irqreturn_t
 eventq_intr(int irq, void *dev)
 {
 	vbus_pci.stats.qnotify++;
@@ -995,7 +995,7 @@ static struct pci_driver vbus_pci_driver = {
 	.remove   = vbus_pci_remove,
 };
 
-int __init
+static int __init
 vbus_pci_init(void)
 {
 	memset(&vbus_pci, 0, sizeof(vbus_pci));
-- 
1.5.4.3

[PATCH] vbus-enet: fix sparse warnings

[Ira W. Snyder]

Fix the following sparse warnings:
drivers/net/vbus-enet.c:411:9: warning: Using plain integer as NULL pointer
drivers/net/vbus-enet.c:1250:24: warning: Using plain integer as NULL pointer
drivers/net/vbus-enet.c:1326:24: warning: Using plain integer as NULL pointer

Signed-off-by: Ira W. Snyder <iws@ovro.caltech.edu>
---
 drivers/net/vbus-enet.c |    6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/net/vbus-enet.c b/drivers/net/vbus-enet.c
index 4e9ecac..8232215 100644
--- a/drivers/net/vbus-enet.c
+++ b/drivers/net/vbus-enet.c
@@ -408,7 +408,7 @@ tx_setup(struct vbus_enet_priv *priv)
 		priv->pmtd.pool = pool;
 
 		ret = dev->ops->shm(dev, NULL, shmid, 0, pool, poollen,
-				    0, NULL, 0);
+				    NULL, NULL, 0);
 		BUG_ON(ret < 0);
 	}
 
@@ -1239,7 +1239,7 @@ vbus_enet_evq_negcap(struct vbus_enet_priv *priv, unsigned long count)
 		priv->evq.pool = pool;
 
 		ret = dev->ops->shm(dev, NULL, query.dpid, 0,
-				    pool, poollen, 0, NULL, 0);
+				    pool, poollen, NULL, NULL, 0);
 		if (ret < 0)
 			return ret;
 
@@ -1315,7 +1315,7 @@ vbus_enet_l4ro_negcap(struct vbus_enet_priv *priv, unsigned long count)
 		 * pre-mapped descriptor pool
 		 */
 		ret = dev->ops->shm(dev, NULL, query.dpid, 0,
-				    pool, poollen, 0, NULL, 0);
+				    pool, poollen, NULL, NULL, 0);
 		if (ret < 0) {
 			printk(KERN_ERR "Error registering L4RO pool: %d\n",
 			       ret);
-- 
1.5.4.3

[PATCH] shm_signal: clearly specify endianness

[Ira W. Snyder]

The shm_signal code uses structures which are designed to be shared between
disparate systems, such as 32-bit and 64-bit, as well as Linux and Windows.

Since shm_signal is primarily intended to be used by qemu/kvm, which
supports virtual guests with a different CPU architecture than the host,
clearly define an endianness for the shared structures.

The endianness is defined to be little-endian, to avoid byte swapping in
the most common case: x86.

Signed-off-by: Ira W. Snyder <iws@ovro.caltech.edu>
---
 include/linux/shm_signal.h |    8 ++++----
 1 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/include/linux/shm_signal.h b/include/linux/shm_signal.h
index 21cf750..b2efd72 100644
--- a/include/linux/shm_signal.h
+++ b/include/linux/shm_signal.h
@@ -32,8 +32,8 @@
  *-----------
  */
 
-#define SHM_SIGNAL_MAGIC 0x58fa39df
-#define SHM_SIGNAL_VER   1
+#define SHM_SIGNAL_MAGIC cpu_to_le32(0x58fa39df)
+#define SHM_SIGNAL_VER   cpu_to_le32(1)
 
 struct shm_signal_irq {
 	__u8                  enabled;
@@ -47,8 +47,8 @@ enum shm_signal_locality {
 };
 
 struct shm_signal_desc {
-	__u32                 magic;
-	__u32                 ver;
+	__le32                 magic;
+	__le32                 ver;
 	struct shm_signal_irq irq[2];
 };
 
-- 
1.5.4.3

[PATCH] ioq: clearly specify endianness

[Ira W. Snyder]

The IOQ code uses structures which are designed to be shared between
disparate systems, such as 32-bit and 64-bit, as well as Linux and Windows.

Since IOQ is primarily intended to be used by qemu/kvm, which support
virtual guests with a different CPU architecture than the host, clearly
define the endianness for the shared structures.

The endianness is defined to be little-endian, to avoid byte swapping in
the most common case: x86.

Note that the cookie member was not changed to have a fixed endianness.
This is because it is only intended for use by one side of the IOQ.

Signed-off-by: Ira W. Snyder <iws@ovro.caltech.edu>
---
 drivers/net/vbus-enet.c   |   30 +++++++++++++++---------------
 drivers/vbus/bus-proxy.c  |    2 +-
 drivers/vbus/pci-bridge.c |    6 +++---
 include/linux/ioq.h       |   18 +++++++++---------
 lib/ioq.c                 |   28 ++++++++++++++++------------
 5 files changed, 44 insertions(+), 40 deletions(-)

diff --git a/drivers/net/vbus-enet.c b/drivers/net/vbus-enet.c
index 8232215..94b86d4 100644
--- a/drivers/net/vbus-enet.c
+++ b/drivers/net/vbus-enet.c
@@ -175,8 +175,8 @@ rxdesc_alloc(struct vbus_enet_priv *priv, struct ioq_ring_desc *desc, size_t len
 		iov->len     = len;
 	} else {
 		desc->cookie = (u64)(unsigned long)skb;
-		desc->ptr    = (u64)__pa(skb->data);
-		desc->len    = len; /* total length  */
+		desc->ptr    = cpu_to_le64(__pa(skb->data));
+		desc->len    = cpu_to_le64(len); /* total length  */
 	}
 
 	desc->valid  = 1;
@@ -212,8 +212,8 @@ rx_pageq_refill(struct vbus_enet_priv *priv, gfp_t gfp_mask)
 
 		added = 1;
 		iter.desc->cookie = (u64)(unsigned long)page;
-		iter.desc->ptr    = (u64)__pa(page_address(page));
-		iter.desc->len    = PAGE_SIZE;
+		iter.desc->ptr    = cpu_to_le64(__pa(page_address(page)));
+		iter.desc->len    = cpu_to_le64(PAGE_SIZE);
 
 		ret = ioq_iter_push(&iter, 0);
 		BUG_ON(ret < 0);
@@ -257,9 +257,9 @@ rx_setup(struct vbus_enet_priv *priv)
 			size_t offset = (i * SG_DESC_SIZE);
 			void *addr = &priv->l4ro.pool[offset];
 
-			iter.desc->ptr    = (u64)offset;
+			iter.desc->ptr    = cpu_to_le64(offset);
 			iter.desc->cookie = (u64)(unsigned long)addr;
-			iter.desc->len    = SG_DESC_SIZE;
+			iter.desc->len    = cpu_to_le64(SG_DESC_SIZE);
 		}
 
 		rxdesc_alloc(priv, iter.desc, priv->dev->mtu);
@@ -428,17 +428,17 @@ tx_setup(struct vbus_enet_priv *priv)
 			size_t offset = (i * SG_DESC_SIZE);
 
 			vsg = (struct venet_sg *)&priv->pmtd.pool[offset];
-			iter.desc->ptr = (u64)offset;
+			iter.desc->ptr = cpu_to_le64(offset);
 		} else {
 			vsg = kzalloc(SG_DESC_SIZE, GFP_KERNEL);
 			if (!vsg)
 				return -ENOMEM;
 
-			iter.desc->ptr = (u64)__pa(vsg);
+			iter.desc->ptr = cpu_to_le64(__pa(vsg));
 		}
 
 		iter.desc->cookie = (u64)(unsigned long)vsg;
-		iter.desc->len    = SG_DESC_SIZE;
+		iter.desc->len    = cpu_to_le64(SG_DESC_SIZE);
 
 		ret = ioq_iter_seek(&iter, ioq_seek_next, 0, 0);
 		BUG_ON(ret < 0);
@@ -708,7 +708,7 @@ vbus_enet_flat_import(struct vbus_enet_priv *priv, struct ioq_ring_desc *desc)
 		return NULL;
 	}
 
-	skb_put(skb, desc->len);
+	skb_put(skb, le64_to_cpu(desc->len));
 
 	return skb;
 }
@@ -871,7 +871,7 @@ vbus_enet_tx_start(struct sk_buff *skb, struct net_device *dev)
 			iov->ptr = (u64)sg_phys(sg);
 		}
 
-		iter.desc->len = (u64)VSG_DESC_SIZE(vsg->count);
+		iter.desc->len = cpu_to_le64(VSG_DESC_SIZE(vsg->count));
 
 	} else {
 		/*
@@ -879,8 +879,8 @@ vbus_enet_tx_start(struct sk_buff *skb, struct net_device *dev)
 		 * ring.
 		 */
 		iter.desc->cookie = (u64)(unsigned long)skb;
-		iter.desc->len = (u64)skb->len;
-		iter.desc->ptr = (u64)__pa(skb->data);
+		iter.desc->len = cpu_to_le64(skb->len);
+		iter.desc->ptr = cpu_to_le64(__pa(skb->data));
 	}
 
 	iter.desc->valid  = 1;
@@ -1258,9 +1258,9 @@ vbus_enet_evq_negcap(struct vbus_enet_priv *priv, unsigned long count)
 			size_t offset = (i * query.evsize);
 			void *addr = &priv->evq.pool[offset];
 
-			iter.desc->ptr    = (u64)offset;
+			iter.desc->ptr    = cpu_to_le64(offset);
 			iter.desc->cookie = (u64)(unsigned long)addr;
-			iter.desc->len    = query.evsize;
+			iter.desc->len    = cpu_to_le64(query.evsize);
 
 			ret = ioq_iter_push(&iter, 0);
 			BUG_ON(ret < 0);
diff --git a/drivers/vbus/bus-proxy.c b/drivers/vbus/bus-proxy.c
index a318c67..4792842 100644
--- a/drivers/vbus/bus-proxy.c
+++ b/drivers/vbus/bus-proxy.c
@@ -217,7 +217,7 @@ int vbus_driver_ioq_alloc(struct vbus_device_proxy *dev, const char *name,
 
 	head->magic     = IOQ_RING_MAGIC;
 	head->ver	= IOQ_RING_VER;
-	head->count     = count;
+	head->count     = cpu_to_le32(count);
 
 	ret = dev->ops->shm(dev, name, id, prio, head, len,
 			    &head->signal, &signal, 0);
diff --git a/drivers/vbus/pci-bridge.c b/drivers/vbus/pci-bridge.c
index 078b8f4..0d51324 100644
--- a/drivers/vbus/pci-bridge.c
+++ b/drivers/vbus/pci-bridge.c
@@ -579,8 +579,8 @@ eventq_init(int qlen)
 		BUG_ON(iter.desc->valid);
 
 		desc->cookie = (u64)(unsigned long)event;
-		desc->ptr    = (u64)__pa(event);
-		desc->len    = len; /* total length  */
+		desc->ptr    = cpu_to_le64(__pa(event));
+		desc->len    = cpu_to_le64(len); /* total length */
 		desc->valid  = 1;
 
 		/*
@@ -798,7 +798,7 @@ _ioq_init(size_t ringsize, struct ioq *ioq, struct ioq_ops *ops)
 
 	head->magic     = IOQ_RING_MAGIC;
 	head->ver	= IOQ_RING_VER;
-	head->count     = ringsize;
+	head->count     = cpu_to_le32(ringsize);
 
 	_signal_init(signal, &head->signal, &eventq_signal_ops);
 
diff --git a/include/linux/ioq.h b/include/linux/ioq.h
index f04dfb4..7c6d6ca 100644
--- a/include/linux/ioq.h
+++ b/include/linux/ioq.h
@@ -52,18 +52,18 @@
  */
 struct ioq_ring_desc {
 	__u64                 cookie; /* for arbitrary use by north-side */
-	__u64                 ptr;
-	__u64                 len;
+	__le64                ptr;
+	__le64                len;
 	__u8                  valid;
 	__u8                  sown; /* South owned = 1, North owned = 0 */
 };
 
-#define IOQ_RING_MAGIC 0x47fa2fe4
-#define IOQ_RING_VER   4
+#define IOQ_RING_MAGIC cpu_to_le32(0x47fa2fe4)
+#define IOQ_RING_VER   cpu_to_le32(4)
 
 struct ioq_ring_idx {
-	__u32                 head;    /* 0 based index to head of ptr array */
-	__u32                 tail;    /* 0 based index to tail of ptr array */
+	__le32                head;    /* 0 based index to head of ptr array */
+	__le32                tail;    /* 0 based index to tail of ptr array */
 	__u8                  full;
 };
 
@@ -73,11 +73,11 @@ enum ioq_locality {
 };
 
 struct ioq_ring_head {
-	__u32                  magic;
-	__u32                  ver;
+	__le32                 magic;
+	__le32                 ver;
 	struct shm_signal_desc signal;
 	struct ioq_ring_idx    idx[2];
-	__u32                  count;
+	__le32                 count;
 	struct ioq_ring_desc   ring[1]; /* "count" elements will be allocated */
 };
 
diff --git a/lib/ioq.c b/lib/ioq.c
index d5e57be..4027848 100644
--- a/lib/ioq.c
+++ b/lib/ioq.c
@@ -71,10 +71,10 @@ int ioq_iter_seek(struct ioq_iterator *iter, enum ioq_seek_type type,
 		pos = modulo_inc(iter->pos, iter->ioq->count);
 		break;
 	case ioq_seek_tail:
-		pos = idx->tail;
+		pos = le32_to_cpu(idx->tail);
 		break;
 	case ioq_seek_head:
-		pos = idx->head;
+		pos = le32_to_cpu(idx->head);
 		break;
 	case ioq_seek_set:
 		if (offset >= iter->ioq->count)
@@ -91,19 +91,23 @@ EXPORT_SYMBOL_GPL(ioq_iter_seek);
 
 static int ioq_ring_count(struct ioq_ring_idx *idx, int count)
 {
-	if (idx->full && (idx->head == idx->tail))
+	u32 head = le32_to_cpu(idx->head);
+	u32 tail = le32_to_cpu(idx->tail);
+
+	if (idx->full && (head == tail))
 		return count;
-	else if (idx->tail >= idx->head)
-		return idx->tail - idx->head;
+	else if (tail >= head)
+		return tail - head;
 	else
-		return (idx->tail + count) - idx->head;
+		return (tail + count) - head;
 }
 
 static void idx_tail_push(struct ioq_ring_idx *idx, int count)
 {
-	u32 tail = modulo_inc(idx->tail, count);
+	u32 tail = modulo_inc(le32_to_cpu(idx->tail), count);
+	u32 head = le32_to_cpu(idx->head);
 
-	if (idx->head == tail) {
+	if (head == tail) {
 		rmb();
 
 		/*
@@ -116,7 +120,7 @@ static void idx_tail_push(struct ioq_ring_idx *idx, int count)
 		wmb();
 	}
 
-	idx->tail = tail;
+	idx->tail = cpu_to_le32(tail);
 }
 
 int ioq_iter_push(struct ioq_iterator *iter, int flags)
@@ -128,7 +132,7 @@ int ioq_iter_push(struct ioq_iterator *iter, int flags)
 	/*
 	 * Its only valid to push if we are currently pointed at the tail
 	 */
-	if (iter->pos != idx->tail || iter->desc->sown != iter->ioq->locale)
+	if (iter->pos != le32_to_cpu(idx->tail) || iter->desc->sown != iter->ioq->locale)
 		return -EINVAL;
 
 	idx_tail_push(idx, iter->ioq->count);
@@ -167,10 +171,10 @@ int ioq_iter_pop(struct ioq_iterator *iter,  int flags)
 	/*
 	 * Its only valid to pop if we are currently pointed at the head
 	 */
-	if (iter->pos != idx->head || iter->desc->sown != iter->ioq->locale)
+	if (iter->pos != le32_to_cpu(idx->head) || iter->desc->sown != iter->ioq->locale)
 		return -EINVAL;
 
-	idx->head = modulo_inc(idx->head, iter->ioq->count);
+	idx->head = cpu_to_le32(modulo_inc(le32_to_cpu(idx->head), iter->ioq->count));
 	wmb(); /* head must be visible before full */
 
 	if (idx->full) {
-- 
1.5.4.3

Re: [Alacrityvm-devel] alacrityvm-next: clearly specify ioq endianness

[Gregory Haskins]

[-- Attachment #1: Type: text/plain, Size: 778 bytes --]

On 12/8/09 3:57 PM, Ira W. Snyder wrote:
> 
> This is a respin of the previous patch series against alacrityvm.git's
> linux-next branch, to facilitate easier merging with the linux-next tree.
> 
> The SHM_SIGNAL and IOQ code is meant to be used for communication channels
> between systems with different bit widths and operating systems.
> Conceivably, this could include systems with different endianness as well,
> for example, using QEMU on x86 to emulate a powerpc.
> 
> This patch series annotates all shared structures as little endian, and
> fixes up all users. This means that there is no overhead in the most common
> case, x86-on-x86. The byteswaps compile out.
> 
> [RESEND: added LKML to CC per Greg Haskins' request]

Applied, thanks!

-Greg


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 267 bytes --]