Re: CVS commit: src/sys/dev/pci

[christos%astron.com@localhost (Christos Zoulas)]

In article <20210121204833.9EBCFFA9E%cvs.NetBSD.org@localhost>,
Reinoud Zandijk <source-changes-d%NetBSD.org@localhost> wrote:
>-=-=-=-=-=-
>
>Module Name:	src
>Committed By:	reinoud
>Date:		Thu Jan 21 20:48:33 UTC 2021
>
>Modified Files:
>	src/sys/dev/pci: virtio_pci.c
>
>Log Message:
>Remove dependency on bus_space_write_8() for i386 and instead implement it as
>two bus_space_write_4()'s as allowed in the spec.

Isn't it better to do it this way so it always works (not just for little
endian)? We could even provide this in the MI bus.h if others need it
and don't care about the non-atomic transactions. I have not even compile
tested it.

christos

Index: virtio_pci.c
===================================================================
RCS file: /cvsroot/src/sys/dev/pci/virtio_pci.c,v
retrieving revision 1.18
diff -u -p -u -r1.18 virtio_pci.c
--- virtio_pci.c	21 Jan 2021 20:48:33 -0000	1.18
+++ virtio_pci.c	22 Jan 2021 04:46:24 -0000
@@ -34,6 +34,7 @@ __KERNEL_RCSID(0, "$NetBSD: virtio_pci.c
 #include <sys/systm.h>
 #include <sys/kmem.h>
 #include <sys/module.h>
+#include <sys/endian.h>
 #include <sys/interrupt.h>
 
 #include <sys/device.h>
@@ -731,9 +732,20 @@ virtio_pci_read_queue_size_10(struct vir
  * By definition little endian only in v1.0 and 8 byters are allowed to be
  * written as two 4 byters
  */
-#define bus_space_write_le_8(iot, ioh, reg, val) \
-	bus_space_write_4(iot, ioh, reg, ((uint64_t) (val)) & 0xffffffff); \
-	bus_space_write_4(iot, ioh, reg + 4, ((uint64_t) (val)) >> 32);
+#ifndef _LP64
+static __inline void
+bus_space_write_8(bus_space_tag_t iot, bus_space_handle_t ioh,
+     bus_size_t offset, uint64_t value)
+{
+#if _QUAD_HIGHWORD
+	bus_space_write_4(iot, ioh, offset, (uint32_t)(value & 0xffffffff));
+	bus_space_write_4(iot, ioh, offset + 4, (uint32_t)(value >> 32));
+#else
+	bus_space_write_4(iot, ioh, offset, (uint32_t)(value >> 32));
+	bus_space_write_4(iot, ioh, offset + 4, (uint32_t)(value & 0xffffffff));
+#endif
+}
+#endif /* !_LP64 */
 
 static void
 virtio_pci_setup_queue_10(struct virtio_softc *sc, uint16_t idx, uint64_t addr)
@@ -747,15 +759,15 @@ virtio_pci_setup_queue_10(struct virtio_
 	bus_space_write_2(iot, ioh, VIRTIO_CONFIG1_QUEUE_SELECT, vq->vq_index);
 	if (addr == 0) {
 		bus_space_write_2(iot, ioh, VIRTIO_CONFIG1_QUEUE_ENABLE, 0);
-		bus_space_write_le_8(iot, ioh, VIRTIO_CONFIG1_QUEUE_DESC,   0);
-		bus_space_write_le_8(iot, ioh, VIRTIO_CONFIG1_QUEUE_AVAIL,  0);
-		bus_space_write_le_8(iot, ioh, VIRTIO_CONFIG1_QUEUE_USED,   0);
+		bus_space_write_8(iot, ioh, VIRTIO_CONFIG1_QUEUE_DESC,   0);
+		bus_space_write_8(iot, ioh, VIRTIO_CONFIG1_QUEUE_AVAIL,  0);
+		bus_space_write_8(iot, ioh, VIRTIO_CONFIG1_QUEUE_USED,   0);
 	} else {
-		bus_space_write_le_8(iot, ioh,
+		bus_space_write_8(iot, ioh,
 			VIRTIO_CONFIG1_QUEUE_DESC, addr);
-		bus_space_write_le_8(iot, ioh,
+		bus_space_write_8(iot, ioh,
 			VIRTIO_CONFIG1_QUEUE_AVAIL, addr + vq->vq_availoffset);
-		bus_space_write_le_8(iot, ioh,
+		bus_space_write_8(iot, ioh,
 			VIRTIO_CONFIG1_QUEUE_USED, addr + vq->vq_usedoffset);
 		bus_space_write_2(iot, ioh,
 			VIRTIO_CONFIG1_QUEUE_ENABLE, 1);
@@ -771,7 +783,6 @@ virtio_pci_setup_queue_10(struct virtio_
 			VIRTIO_CONFIG1_QUEUE_MSIX_VECTOR, vec);
 	}
 }
-#undef bus_space_write_le_8
 
 static void
 virtio_pci_set_status_10(struct virtio_softc *sc, int status)