Module Name:    src
Committed By:   reinoud
Date:           Fri Feb  5 19:18:23 UTC 2021

Modified Files:
        src/sys/dev/pci: virtio.c virtio_pci.c virtiovar.h

Log Message:
Second round of cleaning up endian code. No more tailored code to maintain.


To generate a diff of this commit:
cvs rdiff -u -r1.45 -r1.46 src/sys/dev/pci/virtio.c
cvs rdiff -u -r1.27 -r1.28 src/sys/dev/pci/virtio_pci.c
cvs rdiff -u -r1.19 -r1.20 src/sys/dev/pci/virtiovar.h

Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.

Modified files:

Index: src/sys/dev/pci/virtio.c
diff -u src/sys/dev/pci/virtio.c:1.45 src/sys/dev/pci/virtio.c:1.46
--- src/sys/dev/pci/virtio.c:1.45	Thu Jan 28 15:43:12 2021
+++ src/sys/dev/pci/virtio.c	Fri Feb  5 19:18:23 2021
@@ -1,4 +1,4 @@
-/*	$NetBSD: virtio.c,v 1.45 2021/01/28 15:43:12 reinoud Exp $	*/
+/*	$NetBSD: virtio.c,v 1.46 2021/02/05 19:18:23 reinoud Exp $	*/
 
 /*
  * Copyright (c) 2020 The NetBSD Foundation, Inc.
@@ -28,7 +28,7 @@
  */
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: virtio.c,v 1.45 2021/01/28 15:43:12 reinoud Exp $");
+__KERNEL_RCSID(0, "$NetBSD: virtio.c,v 1.46 2021/02/05 19:18:23 reinoud Exp $");
 
 #include <sys/param.h>
 #include <sys/systm.h>
@@ -201,32 +201,27 @@ virtio_read_device_config_4(struct virti
 	return val;
 }
 
+/*
+ * The Virtio spec explicitly tells that reading and writing 8 bytes are not
+ * considered atomic and no triggers may be connected to reading or writing
+ * it. This allows for reading byte-by-byte.
+ */
 uint64_t
 virtio_read_device_config_8(struct virtio_softc *sc, int index) {
 	bus_space_tag_t	   iot = sc->sc_devcfg_iot;
 	bus_space_handle_t ioh = sc->sc_devcfg_ioh;
-	uint64_t val, val_0, val_1, val_l, val_h;
+	union {
+		uint64_t u64;
+		uint8_t  b[8];
+	} v;
+	uint64_t val;
+
+	for (int i = 0; i < 8; i++)
+		v.b[i] = bus_space_read_1(iot, ioh, index + i);
+	val = v.u64;
 
-	val_0 = bus_space_read_4(iot, ioh, index);
-	val_1 = bus_space_read_4(iot, ioh, index + 4);
-	if (BYTE_ORDER != sc->sc_bus_endian) {
-		val_l = bswap32(val_1);
-		val_h = bswap32(val_0);
-	} else {
-		val_l = val_0;
-		val_h = val_1;
-	}
-
-#ifdef AARCH64EB_PROBLEM
-	/* XXX see comment at virtio_pci.c */
-	if (sc->sc_aarch64eb_bus_problem) {
-		val_l = val_1;
-		val_h = val_0;
-	}
-#endif
-
-	val = val_h << 32;
-	val |= val_l;
+	if (BYTE_ORDER != sc->sc_struct_endian)
+		val = bswap64(val);
 
 	DPRINTFR("read_8", "%08lx", val, index, 8);
 	DPRINTFR2("read_8 low ", "%08x",
@@ -308,34 +303,32 @@ virtio_write_device_config_4(struct virt
 	bus_space_write_4(iot, ioh, index, value);
 }
 
+/*
+ * The Virtio spec explicitly tells that reading and writing 8 bytes are not
+ * considered atomic and no triggers may be connected to reading or writing
+ * it. This allows for writing byte-by-byte. For good measure it is stated to
+ * always write lsb first just in case of a hypervisor bug.
+ */
 void
 virtio_write_device_config_8(struct virtio_softc *sc, int index, uint64_t value)
 {
 	bus_space_tag_t	   iot = sc->sc_devcfg_iot;
 	bus_space_handle_t ioh = sc->sc_devcfg_ioh;
-	uint64_t val_0, val_1, val_l, val_h;
-
-	val_l = BUS_ADDR_LO32(value);
-	val_h = BUS_ADDR_HI32(value);
-
-	if (BYTE_ORDER != sc->sc_bus_endian) {
-		val_0 = bswap32(val_h);
-		val_1 = bswap32(val_l);
-	} else {
-		val_0 = val_l;
-		val_1 = val_h;
-	}
-
-#ifdef AARCH64EB_PROBLEM
-	/* XXX see comment at virtio_pci.c */
-	if (sc->sc_aarch64eb_bus_problem) {
-		val_0 = val_h;
-		val_1 = val_l;
-	}
-#endif
-
-	bus_space_write_4(iot, ioh, index, val_0);
-	bus_space_write_4(iot, ioh, index + 4, val_1);
+	union {
+		uint64_t u64;
+		uint8_t  b[8];
+	} v;
+
+	if (BYTE_ORDER != sc->sc_struct_endian)
+		value = bswap64(value);
+
+	v.u64 = value;
+	if (sc->sc_struct_endian == LITTLE_ENDIAN)
+		for (int i = 0; i < 8; i++)
+			bus_space_write_1(iot, ioh, index + i, v.b[i]);
+ 	else
+		for (int i = 7; i >= 0; i--)
+			bus_space_write_1(iot, ioh, index + i, v.b[i]);
 }
 
 /*

Index: src/sys/dev/pci/virtio_pci.c
diff -u src/sys/dev/pci/virtio_pci.c:1.27 src/sys/dev/pci/virtio_pci.c:1.28
--- src/sys/dev/pci/virtio_pci.c:1.27	Thu Jan 28 15:43:12 2021
+++ src/sys/dev/pci/virtio_pci.c	Fri Feb  5 19:18:23 2021
@@ -1,4 +1,4 @@
-/* $NetBSD: virtio_pci.c,v 1.27 2021/01/28 15:43:12 reinoud Exp $ */
+/* $NetBSD: virtio_pci.c,v 1.28 2021/02/05 19:18:23 reinoud Exp $ */
 
 /*
  * Copyright (c) 2020 The NetBSD Foundation, Inc.
@@ -28,7 +28,7 @@
  */
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: virtio_pci.c,v 1.27 2021/01/28 15:43:12 reinoud Exp $");
+__KERNEL_RCSID(0, "$NetBSD: virtio_pci.c,v 1.28 2021/02/05 19:18:23 reinoud Exp $");
 
 #include <sys/param.h>
 #include <sys/systm.h>
@@ -127,15 +127,11 @@ static int	virtio_pci_setup_intx_interru
  * suddenly read BIG_ENDIAN where it should stay LITTLE_ENDIAN. The data read
  * 1 byte at a time seem OK but reading bigger lengths result in swapped
  * endian. This is most notable on reading 8 byters since we can't use
- * bus_space_{read,write}_8() and it has to be patched there explicitly. We
- * define the AARCH64EB_PROBLEM to signal we're vulnerable to this and set the
- * accompanied sc->sc_aarch64_eb_bus_problem variable when attaching using the
- * IO space.
+ * bus_space_{read,write}_8().
  */
 
 #if defined(__aarch64__) && BYTE_ORDER == BIG_ENDIAN
-	/* source of AARCH64EB_PROBLEM */
-#	define READ_ENDIAN_09	BIG_ENDIAN	/* XXX bug, should be LITTLE_ENDIAN */
+#	define READ_ENDIAN_09	BIG_ENDIAN	/* should be LITTLE_ENDIAN */
 #	define READ_ENDIAN_10	BIG_ENDIAN
 #	define STRUCT_ENDIAN_09	BIG_ENDIAN
 #	define STRUCT_ENDIAN_10	LITTLE_ENDIAN
@@ -369,9 +365,6 @@ virtio_pci_attach_09(device_t self, void
 	sc->sc_ops = &virtio_pci_ops_09;
 	sc->sc_bus_endian    = READ_ENDIAN_09;
 	sc->sc_struct_endian = STRUCT_ENDIAN_09;
-#if defined(__aarch64__) && BYTE_ORDER == BIG_ENDIAN
-	sc->sc_aarch64eb_bus_problem = true;
-#endif
 	return 0;
 }
 

Index: src/sys/dev/pci/virtiovar.h
diff -u src/sys/dev/pci/virtiovar.h:1.19 src/sys/dev/pci/virtiovar.h:1.20
--- src/sys/dev/pci/virtiovar.h:1.19	Thu Jan 28 15:43:12 2021
+++ src/sys/dev/pci/virtiovar.h	Fri Feb  5 19:18:23 2021
@@ -1,4 +1,4 @@
-/*	$NetBSD: virtiovar.h,v 1.19 2021/01/28 15:43:12 reinoud Exp $	*/
+/*	$NetBSD: virtiovar.h,v 1.20 2021/02/05 19:18:23 reinoud Exp $	*/
 
 /*
  * Copyright (c) 2010 Minoura Makoto.
@@ -148,9 +148,6 @@ struct virtio_softc {
 	const struct virtio_ops *sc_ops;
 	bus_dma_tag_t		sc_dmat;
 
-#define AARCH64EB_PROBLEM	/* see comment in virtio_pci.c */
-	bool			sc_aarch64eb_bus_problem;
-
 	int			sc_bus_endian;
 	int			sc_struct_endian;
 

Reply via email to