[PATCH 0/2] Allow virtio-mmio to work on m68k nommu 68000

[PATCH 0/2] Allow virtio-mmio to work on m68k nommu 68000

[Daniel Palmer]

- First patch just cleans up sizeof usage so that checkpatch complains
  less about the second patch.

- Second patch replaces uses of readl()/writel() with raw io access +
  explict endian conversion to avoid getting the wrong values where
  readl()/writel() are abnormal like m68k nommu for non-coldfire.

Tested on QEMU + patches to make a 68000 virt machine possible.

Daniel Palmer (2):
  virtio-mmio: Replace sizeof x with sizeof(x)
  virtio-mmio: Use raw io accessors to avoid arch issues

 drivers/virtio/virtio_mmio.c | 140 ++++++++++++++++++-----------------
 1 file changed, 74 insertions(+), 66 deletions(-)

-- 
2.51.0

[PATCH 1/2] virtio-mmio: Replace sizeof x with sizeof(x)

[Daniel Palmer]

checkpatch wants the braces.

Signed-off-by: Daniel Palmer <daniel@thingy.jp>
---
 drivers/virtio/virtio_mmio.c | 24 ++++++++++++------------
 1 file changed, 12 insertions(+), 12 deletions(-)

diff --git a/drivers/virtio/virtio_mmio.c b/drivers/virtio/virtio_mmio.c
index 595c2274fbb5..dda915b2ac9f 100644
--- a/drivers/virtio/virtio_mmio.c
+++ b/drivers/virtio/virtio_mmio.c
@@ -152,21 +152,21 @@ static void vm_get(struct virtio_device *vdev, unsigned int offset,
 	switch (len) {
 	case 1:
 		b = readb(base + offset);
-		memcpy(buf, &b, sizeof b);
+		memcpy(buf, &b, sizeof(b));
 		break;
 	case 2:
 		w = cpu_to_le16(readw(base + offset));
-		memcpy(buf, &w, sizeof w);
+		memcpy(buf, &w, sizeof(w));
 		break;
 	case 4:
 		l = cpu_to_le32(readl(base + offset));
-		memcpy(buf, &l, sizeof l);
+		memcpy(buf, &l, sizeof(l));
 		break;
 	case 8:
 		l = cpu_to_le32(readl(base + offset));
-		memcpy(buf, &l, sizeof l);
-		l = cpu_to_le32(ioread32(base + offset + sizeof l));
-		memcpy(buf + sizeof l, &l, sizeof l);
+		memcpy(buf, &l, sizeof(l));
+		l = cpu_to_le32(ioread32(base + offset + sizeof(l)));
+		memcpy(buf + sizeof(l), &l, sizeof(l));
 		break;
 	default:
 		BUG();
@@ -194,22 +194,22 @@ static void vm_set(struct virtio_device *vdev, unsigned int offset,
 
 	switch (len) {
 	case 1:
-		memcpy(&b, buf, sizeof b);
+		memcpy(&b, buf, sizeof(b));
 		writeb(b, base + offset);
 		break;
 	case 2:
-		memcpy(&w, buf, sizeof w);
+		memcpy(&w, buf, sizeof(w));
 		writew(le16_to_cpu(w), base + offset);
 		break;
 	case 4:
-		memcpy(&l, buf, sizeof l);
+		memcpy(&l, buf, sizeof(l));
 		writel(le32_to_cpu(l), base + offset);
 		break;
 	case 8:
-		memcpy(&l, buf, sizeof l);
+		memcpy(&l, buf, sizeof(l));
 		writel(le32_to_cpu(l), base + offset);
-		memcpy(&l, buf + sizeof l, sizeof l);
-		writel(le32_to_cpu(l), base + offset + sizeof l);
+		memcpy(&l, buf + sizeof(l), sizeof(l));
+		writel(le32_to_cpu(l), base + offset + sizeof(l));
 		break;
 	default:
 		BUG();
-- 
2.51.0

[PATCH 2/2] virtio-mmio: Use raw io accessors to avoid arch issues

[Daniel Palmer]

virtio-mmio registers are always little endian even on machines
where peripheral registers are normally big endian.

At least on nommu, non-coldfire, m68k (see: arch/m68k/include/asm/io_no.h)
readl() and friends are defined to the raw versions. So the value from
the bus is returned as is.

Since virtio-mmio is a bit special and will always be little endian
even if that is different to the machine's convention use the raw
accessors and do the endian swap, where needed, in house.

There are some places in the code where the little endian values are
actually wanted and we currently swap those back again. If we don't
swap them in the first place maybe that saves some unneeded swapping?

Signed-off-by: Daniel Palmer <daniel@thingy.jp>
---
 drivers/virtio/virtio_mmio.c | 120 +++++++++++++++++++----------------
 1 file changed, 64 insertions(+), 56 deletions(-)

diff --git a/drivers/virtio/virtio_mmio.c b/drivers/virtio/virtio_mmio.c
index dda915b2ac9f..4050533cc976 100644
--- a/drivers/virtio/virtio_mmio.c
+++ b/drivers/virtio/virtio_mmio.c
@@ -70,7 +70,14 @@
 #include <uapi/linux/virtio_mmio.h>
 #include <linux/virtio_ring.h>
 
-
+/*
+ * The virtio registers are always little endian, even on machines
+ * where everything else is big endian. readl()/write() could use the
+ * value from the bus as-is or endian swap it from little endian.
+ * To avoid confusion use the raw accessors and do the swap here.
+ */
+#define virtio_mmio_readl(addr)		le32_to_cpu(__raw_readl(addr))
+#define virtio_mmio_writel(value, addr)	__raw_writel(cpu_to_le32(value), addr)
 
 /* The alignment to use between consumer and producer parts of vring.
  * Currently hardcoded to the page size. */
@@ -96,12 +103,12 @@ static u64 vm_get_features(struct virtio_device *vdev)
 	struct virtio_mmio_device *vm_dev = to_virtio_mmio_device(vdev);
 	u64 features;
 
-	writel(1, vm_dev->base + VIRTIO_MMIO_DEVICE_FEATURES_SEL);
-	features = readl(vm_dev->base + VIRTIO_MMIO_DEVICE_FEATURES);
+	virtio_mmio_writel(1, vm_dev->base + VIRTIO_MMIO_DEVICE_FEATURES_SEL);
+	features = virtio_mmio_readl(vm_dev->base + VIRTIO_MMIO_DEVICE_FEATURES);
 	features <<= 32;
 
-	writel(0, vm_dev->base + VIRTIO_MMIO_DEVICE_FEATURES_SEL);
-	features |= readl(vm_dev->base + VIRTIO_MMIO_DEVICE_FEATURES);
+	virtio_mmio_writel(0, vm_dev->base + VIRTIO_MMIO_DEVICE_FEATURES_SEL);
+	features |= virtio_mmio_readl(vm_dev->base + VIRTIO_MMIO_DEVICE_FEATURES);
 
 	return features;
 }
@@ -120,12 +127,12 @@ static int vm_finalize_features(struct virtio_device *vdev)
 		return -EINVAL;
 	}
 
-	writel(1, vm_dev->base + VIRTIO_MMIO_DRIVER_FEATURES_SEL);
-	writel((u32)(vdev->features >> 32),
+	virtio_mmio_writel(1, vm_dev->base + VIRTIO_MMIO_DRIVER_FEATURES_SEL);
+	virtio_mmio_writel((u32)(vdev->features >> 32),
 			vm_dev->base + VIRTIO_MMIO_DRIVER_FEATURES);
 
-	writel(0, vm_dev->base + VIRTIO_MMIO_DRIVER_FEATURES_SEL);
-	writel((u32)vdev->features,
+	virtio_mmio_writel(0, vm_dev->base + VIRTIO_MMIO_DRIVER_FEATURES_SEL);
+	virtio_mmio_writel((u32)vdev->features,
 			vm_dev->base + VIRTIO_MMIO_DRIVER_FEATURES);
 
 	return 0;
@@ -155,17 +162,17 @@ static void vm_get(struct virtio_device *vdev, unsigned int offset,
 		memcpy(buf, &b, sizeof(b));
 		break;
 	case 2:
-		w = cpu_to_le16(readw(base + offset));
+		w = __raw_readw(base + offset);
 		memcpy(buf, &w, sizeof(w));
 		break;
 	case 4:
-		l = cpu_to_le32(readl(base + offset));
+		l = __raw_readl(base + offset);
 		memcpy(buf, &l, sizeof(l));
 		break;
 	case 8:
-		l = cpu_to_le32(readl(base + offset));
+		l = __raw_readl(base + offset);
 		memcpy(buf, &l, sizeof(l));
-		l = cpu_to_le32(ioread32(base + offset + sizeof(l)));
+		l = __raw_readl(base + offset + sizeof(l));
 		memcpy(buf + sizeof(l), &l, sizeof(l));
 		break;
 	default:
@@ -199,17 +206,17 @@ static void vm_set(struct virtio_device *vdev, unsigned int offset,
 		break;
 	case 2:
 		memcpy(&w, buf, sizeof(w));
-		writew(le16_to_cpu(w), base + offset);
+		__raw_writew(w, base + offset);
 		break;
 	case 4:
 		memcpy(&l, buf, sizeof(l));
-		writel(le32_to_cpu(l), base + offset);
+		__raw_writel(l, base + offset);
 		break;
 	case 8:
 		memcpy(&l, buf, sizeof(l));
-		writel(le32_to_cpu(l), base + offset);
+		__raw_writel(l, base + offset);
 		memcpy(&l, buf + sizeof(l), sizeof(l));
-		writel(le32_to_cpu(l), base + offset + sizeof(l));
+		__raw_writel(l, base + offset + sizeof(l));
 		break;
 	default:
 		BUG();
@@ -223,14 +230,14 @@ static u32 vm_generation(struct virtio_device *vdev)
 	if (vm_dev->version == 1)
 		return 0;
 	else
-		return readl(vm_dev->base + VIRTIO_MMIO_CONFIG_GENERATION);
+		return virtio_mmio_readl(vm_dev->base + VIRTIO_MMIO_CONFIG_GENERATION);
 }
 
 static u8 vm_get_status(struct virtio_device *vdev)
 {
 	struct virtio_mmio_device *vm_dev = to_virtio_mmio_device(vdev);
 
-	return readl(vm_dev->base + VIRTIO_MMIO_STATUS) & 0xff;
+	return virtio_mmio_readl(vm_dev->base + VIRTIO_MMIO_STATUS) & 0xff;
 }
 
 static void vm_set_status(struct virtio_device *vdev, u8 status)
@@ -245,7 +252,7 @@ static void vm_set_status(struct virtio_device *vdev, u8 status)
 	 * that the cache coherent memory writes have completed
 	 * before writing to the MMIO region.
 	 */
-	writel(status, vm_dev->base + VIRTIO_MMIO_STATUS);
+	virtio_mmio_writel(status, vm_dev->base + VIRTIO_MMIO_STATUS);
 }
 
 static void vm_reset(struct virtio_device *vdev)
@@ -253,7 +260,7 @@ static void vm_reset(struct virtio_device *vdev)
 	struct virtio_mmio_device *vm_dev = to_virtio_mmio_device(vdev);
 
 	/* 0 status means a reset. */
-	writel(0, vm_dev->base + VIRTIO_MMIO_STATUS);
+	virtio_mmio_writel(0, vm_dev->base + VIRTIO_MMIO_STATUS);
 }
 
 
@@ -267,7 +274,7 @@ static bool vm_notify(struct virtqueue *vq)
 
 	/* We write the queue's selector into the notification register to
 	 * signal the other end */
-	writel(vq->index, vm_dev->base + VIRTIO_MMIO_QUEUE_NOTIFY);
+	virtio_mmio_writel(vq->index, vm_dev->base + VIRTIO_MMIO_QUEUE_NOTIFY);
 	return true;
 }
 
@@ -276,7 +283,7 @@ static bool vm_notify_with_data(struct virtqueue *vq)
 	struct virtio_mmio_device *vm_dev = to_virtio_mmio_device(vq->vdev);
 	u32 data = vring_notification_data(vq);
 
-	writel(data, vm_dev->base + VIRTIO_MMIO_QUEUE_NOTIFY);
+	virtio_mmio_writel(data, vm_dev->base + VIRTIO_MMIO_QUEUE_NOTIFY);
 
 	return true;
 }
@@ -290,8 +297,8 @@ static irqreturn_t vm_interrupt(int irq, void *opaque)
 	irqreturn_t ret = IRQ_NONE;
 
 	/* Read and acknowledge interrupts */
-	status = readl(vm_dev->base + VIRTIO_MMIO_INTERRUPT_STATUS);
-	writel(status, vm_dev->base + VIRTIO_MMIO_INTERRUPT_ACK);
+	status = virtio_mmio_readl(vm_dev->base + VIRTIO_MMIO_INTERRUPT_STATUS);
+	virtio_mmio_writel(status, vm_dev->base + VIRTIO_MMIO_INTERRUPT_ACK);
 
 	if (unlikely(status & VIRTIO_MMIO_INT_CONFIG)) {
 		virtio_config_changed(&vm_dev->vdev);
@@ -314,12 +321,12 @@ static void vm_del_vq(struct virtqueue *vq)
 	unsigned int index = vq->index;
 
 	/* Select and deactivate the queue */
-	writel(index, vm_dev->base + VIRTIO_MMIO_QUEUE_SEL);
+	virtio_mmio_writel(index, vm_dev->base + VIRTIO_MMIO_QUEUE_SEL);
 	if (vm_dev->version == 1) {
-		writel(0, vm_dev->base + VIRTIO_MMIO_QUEUE_PFN);
+		virtio_mmio_writel(0, vm_dev->base + VIRTIO_MMIO_QUEUE_PFN);
 	} else {
-		writel(0, vm_dev->base + VIRTIO_MMIO_QUEUE_READY);
-		WARN_ON(readl(vm_dev->base + VIRTIO_MMIO_QUEUE_READY));
+		virtio_mmio_writel(0, vm_dev->base + VIRTIO_MMIO_QUEUE_READY);
+		WARN_ON(virtio_mmio_readl(vm_dev->base + VIRTIO_MMIO_QUEUE_READY));
 	}
 
 	vring_del_virtqueue(vq);
@@ -362,16 +369,17 @@ static struct virtqueue *vm_setup_vq(struct virtio_device *vdev, unsigned int in
 		return NULL;
 
 	/* Select the queue we're interested in */
-	writel(index, vm_dev->base + VIRTIO_MMIO_QUEUE_SEL);
+	virtio_mmio_writel(index, vm_dev->base + VIRTIO_MMIO_QUEUE_SEL);
 
 	/* Queue shouldn't already be set up. */
-	if (readl(vm_dev->base + (vm_dev->version == 1 ?
+	if (virtio_mmio_readl(vm_dev->base + (vm_dev->version == 1 ?
 			VIRTIO_MMIO_QUEUE_PFN : VIRTIO_MMIO_QUEUE_READY))) {
 		err = -ENOENT;
 		goto error_available;
 	}
 
-	num = readl(vm_dev->base + VIRTIO_MMIO_QUEUE_NUM_MAX);
+	num = virtio_mmio_readl(vm_dev->base + VIRTIO_MMIO_QUEUE_NUM_MAX);
+
 	if (num == 0) {
 		err = -ENOENT;
 		goto error_new_virtqueue;
@@ -388,7 +396,7 @@ static struct virtqueue *vm_setup_vq(struct virtio_device *vdev, unsigned int in
 	vq->num_max = num;
 
 	/* Activate the queue */
-	writel(virtqueue_get_vring_size(vq), vm_dev->base + VIRTIO_MMIO_QUEUE_NUM);
+	virtio_mmio_writel(virtqueue_get_vring_size(vq), vm_dev->base + VIRTIO_MMIO_QUEUE_NUM);
 	if (vm_dev->version == 1) {
 		u64 q_pfn = virtqueue_get_desc_addr(vq) >> PAGE_SHIFT;
 
@@ -405,27 +413,27 @@ static struct virtqueue *vm_setup_vq(struct virtio_device *vdev, unsigned int in
 			goto error_bad_pfn;
 		}
 
-		writel(PAGE_SIZE, vm_dev->base + VIRTIO_MMIO_QUEUE_ALIGN);
-		writel(q_pfn, vm_dev->base + VIRTIO_MMIO_QUEUE_PFN);
+		virtio_mmio_writel(PAGE_SIZE, vm_dev->base + VIRTIO_MMIO_QUEUE_ALIGN);
+		virtio_mmio_writel(q_pfn, vm_dev->base + VIRTIO_MMIO_QUEUE_PFN);
 	} else {
 		u64 addr;
 
 		addr = virtqueue_get_desc_addr(vq);
-		writel((u32)addr, vm_dev->base + VIRTIO_MMIO_QUEUE_DESC_LOW);
-		writel((u32)(addr >> 32),
+		virtio_mmio_writel((u32)addr, vm_dev->base + VIRTIO_MMIO_QUEUE_DESC_LOW);
+		virtio_mmio_writel((u32)(addr >> 32),
 				vm_dev->base + VIRTIO_MMIO_QUEUE_DESC_HIGH);
 
 		addr = virtqueue_get_avail_addr(vq);
-		writel((u32)addr, vm_dev->base + VIRTIO_MMIO_QUEUE_AVAIL_LOW);
-		writel((u32)(addr >> 32),
+		virtio_mmio_writel((u32)addr, vm_dev->base + VIRTIO_MMIO_QUEUE_AVAIL_LOW);
+		virtio_mmio_writel((u32)(addr >> 32),
 				vm_dev->base + VIRTIO_MMIO_QUEUE_AVAIL_HIGH);
 
 		addr = virtqueue_get_used_addr(vq);
-		writel((u32)addr, vm_dev->base + VIRTIO_MMIO_QUEUE_USED_LOW);
-		writel((u32)(addr >> 32),
+		virtio_mmio_writel((u32)addr, vm_dev->base + VIRTIO_MMIO_QUEUE_USED_LOW);
+		virtio_mmio_writel((u32)(addr >> 32),
 				vm_dev->base + VIRTIO_MMIO_QUEUE_USED_HIGH);
 
-		writel(1, vm_dev->base + VIRTIO_MMIO_QUEUE_READY);
+		virtio_mmio_writel(1, vm_dev->base + VIRTIO_MMIO_QUEUE_READY);
 	}
 
 	return vq;
@@ -434,10 +442,10 @@ static struct virtqueue *vm_setup_vq(struct virtio_device *vdev, unsigned int in
 	vring_del_virtqueue(vq);
 error_new_virtqueue:
 	if (vm_dev->version == 1) {
-		writel(0, vm_dev->base + VIRTIO_MMIO_QUEUE_PFN);
+		virtio_mmio_writel(0, vm_dev->base + VIRTIO_MMIO_QUEUE_PFN);
 	} else {
-		writel(0, vm_dev->base + VIRTIO_MMIO_QUEUE_READY);
-		WARN_ON(readl(vm_dev->base + VIRTIO_MMIO_QUEUE_READY));
+		virtio_mmio_writel(0, vm_dev->base + VIRTIO_MMIO_QUEUE_READY);
+		WARN_ON(virtio_mmio_readl(vm_dev->base + VIRTIO_MMIO_QUEUE_READY));
 	}
 error_available:
 	return ERR_PTR(err);
@@ -496,11 +504,11 @@ static bool vm_get_shm_region(struct virtio_device *vdev,
 	u64 len, addr;
 
 	/* Select the region we're interested in */
-	writel(id, vm_dev->base + VIRTIO_MMIO_SHM_SEL);
+	virtio_mmio_writel(id, vm_dev->base + VIRTIO_MMIO_SHM_SEL);
 
 	/* Read the region size */
-	len = (u64) readl(vm_dev->base + VIRTIO_MMIO_SHM_LEN_LOW);
-	len |= (u64) readl(vm_dev->base + VIRTIO_MMIO_SHM_LEN_HIGH) << 32;
+	len = (u64) virtio_mmio_readl(vm_dev->base + VIRTIO_MMIO_SHM_LEN_LOW);
+	len |= (u64) virtio_mmio_readl(vm_dev->base + VIRTIO_MMIO_SHM_LEN_HIGH) << 32;
 
 	region->len = len;
 
@@ -511,8 +519,8 @@ static bool vm_get_shm_region(struct virtio_device *vdev,
 		return false;
 
 	/* Read the region base address */
-	addr = (u64) readl(vm_dev->base + VIRTIO_MMIO_SHM_BASE_LOW);
-	addr |= (u64) readl(vm_dev->base + VIRTIO_MMIO_SHM_BASE_HIGH) << 32;
+	addr = (u64) virtio_mmio_readl(vm_dev->base + VIRTIO_MMIO_SHM_BASE_LOW);
+	addr |= (u64) virtio_mmio_readl(vm_dev->base + VIRTIO_MMIO_SHM_BASE_HIGH) << 32;
 
 	region->addr = addr;
 
@@ -548,7 +556,7 @@ static int virtio_mmio_restore(struct device *dev)
 	struct virtio_mmio_device *vm_dev = dev_get_drvdata(dev);
 
 	if (vm_dev->version == 1)
-		writel(PAGE_SIZE, vm_dev->base + VIRTIO_MMIO_GUEST_PAGE_SIZE);
+		virtio_mmio_writel(PAGE_SIZE, vm_dev->base + VIRTIO_MMIO_GUEST_PAGE_SIZE);
 
 	return virtio_device_restore(&vm_dev->vdev);
 }
@@ -591,7 +599,7 @@ static int virtio_mmio_probe(struct platform_device *pdev)
 	}
 
 	/* Check magic value */
-	magic = readl(vm_dev->base + VIRTIO_MMIO_MAGIC_VALUE);
+	magic = virtio_mmio_readl(vm_dev->base + VIRTIO_MMIO_MAGIC_VALUE);
 	if (magic != ('v' | 'i' << 8 | 'r' << 16 | 't' << 24)) {
 		dev_warn(&pdev->dev, "Wrong magic value 0x%08lx!\n", magic);
 		rc = -ENODEV;
@@ -599,7 +607,7 @@ static int virtio_mmio_probe(struct platform_device *pdev)
 	}
 
 	/* Check device version */
-	vm_dev->version = readl(vm_dev->base + VIRTIO_MMIO_VERSION);
+	vm_dev->version = virtio_mmio_readl(vm_dev->base + VIRTIO_MMIO_VERSION);
 	if (vm_dev->version < 1 || vm_dev->version > 2) {
 		dev_err(&pdev->dev, "Version %ld not supported!\n",
 				vm_dev->version);
@@ -607,7 +615,7 @@ static int virtio_mmio_probe(struct platform_device *pdev)
 		goto free_vm_dev;
 	}
 
-	vm_dev->vdev.id.device = readl(vm_dev->base + VIRTIO_MMIO_DEVICE_ID);
+	vm_dev->vdev.id.device = virtio_mmio_readl(vm_dev->base + VIRTIO_MMIO_DEVICE_ID);
 	if (vm_dev->vdev.id.device == 0) {
 		/*
 		 * virtio-mmio device with an ID 0 is a (dummy) placeholder
@@ -616,10 +624,10 @@ static int virtio_mmio_probe(struct platform_device *pdev)
 		rc = -ENODEV;
 		goto free_vm_dev;
 	}
-	vm_dev->vdev.id.vendor = readl(vm_dev->base + VIRTIO_MMIO_VENDOR_ID);
+	vm_dev->vdev.id.vendor = virtio_mmio_readl(vm_dev->base + VIRTIO_MMIO_VENDOR_ID);
 
 	if (vm_dev->version == 1) {
-		writel(PAGE_SIZE, vm_dev->base + VIRTIO_MMIO_GUEST_PAGE_SIZE);
+		virtio_mmio_writel(PAGE_SIZE, vm_dev->base + VIRTIO_MMIO_GUEST_PAGE_SIZE);
 
 		rc = dma_set_mask(&pdev->dev, DMA_BIT_MASK(64));
 		/*
-- 
2.51.0

Re: [PATCH 2/2] virtio-mmio: Use raw io accessors to avoid arch issues

[Arnd Bergmann]

On Sat, Mar 14, 2026, at 04:06, Daniel Palmer wrote:
> virtio-mmio registers are always little endian even on machines
> where peripheral registers are normally big endian.
>
> At least on nommu, non-coldfire, m68k (see: arch/m68k/include/asm/io_no.h)
> readl() and friends are defined to the raw versions. So the value from
> the bus is returned as is.
>
> Since virtio-mmio is a bit special and will always be little endian
> even if that is different to the machine's convention use the raw
> accessors and do the endian swap, where needed, in house.
>
> There are some places in the code where the little endian values are
> actually wanted and we currently swap those back again. If we don't
> swap them in the first place maybe that saves some unneeded swapping?
>
> Signed-off-by: Daniel Palmer <daniel@thingy.jp>

I don't think this is safe to do in portable code

> -
> +/*
> + * The virtio registers are always little endian, even on machines
> + * where everything else is big endian. readl()/write() could use the
> + * value from the bus as-is or endian swap it from little endian.
> + * To avoid confusion use the raw accessors and do the swap here.
> + */
> +#define virtio_mmio_readl(addr)		le32_to_cpu(__raw_readl(addr))
> +#define virtio_mmio_writel(value, addr)	__raw_writel(cpu_to_le32(value), addr)

The __raw accessors are not well-defined across architectures and
don't technically guarantee anything about the access beyond being
usable as part of a memory access to byte data behind a bridge.
In particular using these for MMIO registers (real or emulated)
misses the bits about

 - atomicity of the access
 - emulating them from virtualization host
 - actual endianess
 - ordering against memory or MMIO accesses

I think a better approach would be to change the dragonball
specific device drivers to just use iowrite32_be/ioread32_be
instead of writel/readl, and make the behavior consistent
with the rest of Linux (including io_mm.h and coldfire),
using the generic definition:

diff --git a/arch/m68k/include/asm/io_no.h b/arch/m68k/include/asm/io_no.h
index 516371d5587a..c39db8798ef2 100644
--- a/arch/m68k/include/asm/io_no.h
+++ b/arch/m68k/include/asm/io_no.h
@@ -96,15 +96,6 @@ static inline void writel(u32 value, volatile void __iomem *addr)
 		__raw_writel(swab32(value), addr);
 }
 
-#else
-
-#define readb __raw_readb
-#define readw __raw_readw
-#define readl __raw_readl
-#define writeb __raw_writeb
-#define writew __raw_writew
-#define writel __raw_writel
-
 #endif /* IOMEMBASE */
 
 #if defined(CONFIG_PCI)

Can you try if this works?

   Arnd

Re: [PATCH 2/2] virtio-mmio: Use raw io accessors to avoid arch issues

[Daniel Palmer]

Hi Arnd,

On Sat, 14 Mar 2026 at 16:42, Arnd Bergmann <arnd@kernel.org> wrote:
> I don't think this is safe to do in portable code

:(

> diff --git a/arch/m68k/include/asm/io_no.h b/arch/m68k/include/asm/io_no.h
> index 516371d5587a..c39db8798ef2 100644
> --- a/arch/m68k/include/asm/io_no.h
> +++ b/arch/m68k/include/asm/io_no.h
> @@ -96,15 +96,6 @@ static inline void writel(u32 value, volatile void __iomem *addr)
>                 __raw_writel(swab32(value), addr);
>  }
>
> -#else
> -
> -#define readb __raw_readb
> -#define readw __raw_readw
> -#define readl __raw_readl
> -#define writeb __raw_writeb
> -#define writew __raw_writew
> -#define writel __raw_writel
> -
>  #endif /* IOMEMBASE */
>
>  #if defined(CONFIG_PCI)
>
> Can you try if this works?

This builds but doesn't boot on my devicetree'd dragonball branch. I
don't think it's possible to boot the stuff that is in mainline (and
you wouldn't know if it booted or not due to the lack of a serial
driver). I think I can fix my stuff so it works with the changes in
readl() etc but I think there are a few other people that have their
own 68000 trees for weird custom boards and I guess this might break
their stuff. Not saying we should force a change onto virtio-mmio
because of weird hobby projects. Not sure what to do.

Thanks,

Daniel

Re: [PATCH 2/2] virtio-mmio: Use raw io accessors to avoid arch issues

[Arnd Bergmann]

On Sat, Mar 14, 2026, at 09:32, Daniel Palmer wrote:
> On Sat, 14 Mar 2026 at 16:42, Arnd Bergmann <arnd@kernel.org> wrote:
>> -
>> -#define readb __raw_readb
>> -#define readw __raw_readw
>> -#define readl __raw_readl
>> -#define writeb __raw_writeb
>> -#define writew __raw_writew
>> -#define writel __raw_writel
>> -
>>  #endif /* IOMEMBASE */
>>
>>  #if defined(CONFIG_PCI)
>>
>> Can you try if this works?
>
> This builds but doesn't boot on my devicetree'd dragonball branch. I
> don't think it's possible to boot the stuff that is in mainline (and
> you wouldn't know if it booted or not due to the lack of a serial
> driver). I think I can fix my stuff so it works with the changes in
> readl() etc but I think there are a few other people that have their
> own 68000 trees for weird custom boards and I guess this might break
> their stuff. Not saying we should force a change onto virtio-mmio
> because of weird hobby projects. Not sure what to do.

I don't think you need to worry about breaking stuff that is not
upstream: it is the expected cost of maintaining custom patches,
and you probably know who those people are so you can warn them.

I tried to find dragonball specific drivers in the source tree
but couldn't find anything that actually uses readl/writel.
Since it didn't boot for you, there is clearly something, but
I wonder if most of that is just your own out-of-tree code that
works around the endianess bug in asm/io.h by having the opposite
bug in the driver, i.e. using readl() or ioread32() to read a
big-endian word where it should have been ioread32_be().

     Arnd