kern/58680: [PATCH] Wrong byte order of config space write in big-endian virtio over PCI

[gorg%gorgnet.net@localhost]

>Number:         58680
>Category:       kern
>Synopsis:       [PATCH] Wrong byte order of config space write in big-endian virtio over PCI
>Confidential:   no
>Severity:       serious
>Priority:       low
>Responsible:    kern-bug-people
>State:          open
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Thu Sep 19 23:00:00 +0000 2024
>Originator:     gorg%gorgnet.net@localhost
>Release:        NetBSD 10.99.12
>Organization:
>Environment:
System: NetBSD 10.99.12: Wed Sep 18 15:14:20 MDT 2024 evbarm
Architecture: aarch64
Machine: evbarm
>Description:
In virtio_pci.c, the function virtio_pci_bus_space_write_8 writes the first 4
bytes and second 4 bytes of an 8-byte value in the same or swapped order
within the device's config space depending on the endianness of the system it
is compiled for. However, the BUS_ADDR_LO32 and BUS_ADDR_HI32 always produce
the least-significant and most-significant 4 bytes of an 8-byte value,
respectively, regardless of the endianness of the system. Since
virtio_pci_bus_space_write_8 is always used only for writing to little-endian
parts of the config space, the least significant 4 bytes must always be written
to the first 4 bytes in the config space value, and the most significant 4
bytes must always be written to the last 4 bytes of the config space value.
Therefore, swapping the byte order produces incorrect behaviour on big-endian
systems.

The incorrect behaviour seems to have been introduced in revision 1.19 of
virtio_pci.c, which this change would mostly reverse:

http://cvsweb.netbsd.org/bsdweb.cgi/src/sys/dev/pci/virtio_pci.c?rev=1.19&content-type=text/x-cvsweb-markup&only_with_tag=MAIN
http://cvsweb.netbsd.org/bsdweb.cgi/src/sys/dev/pci/virtio_pci.c.diff?r1=1.19&r2=1.18&only_with_tag=MAIN

I suspect that this may be related to problem reports #57289 and #57290. This
issue was observed and this fix was made after the VIRTIO_F_ACCESS_PLATFORM
changed was added.

>How-To-Repeat:
One may use qemu to test virtio-over-PCI on a big endian system. For example:

qemu-system-sparc64 ... \
    -device virtio-blk-pci,bus=pciB,drive=... \
    ...

Depending on the memory layout of the particular kernel that is used, the
device may hang or qemu may produce errors about incorrect values being
written while the device fails to work.

>Fix:
Index: sys/dev/pci/virtio_pci.c
===================================================================
RCS file: /cvsroot/src/sys/dev/pci/virtio_pci.c,v
retrieving revision 1.54
diff -u -r1.54 virtio_pci.c
--- sys/dev/pci/virtio_pci.c	25 Jun 2024 14:55:23 -0000	1.54
+++ sys/dev/pci/virtio_pci.c	19 Sep 2024 22:01:17 -0000
@@ -755,13 +755,8 @@
 virtio_pci_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, BUS_ADDR_LO32(value));
 	bus_space_write_4(iot, ioh, offset + 4, BUS_ADDR_HI32(value));
-#else
-	bus_space_write_4(iot, ioh, offset, BUS_ADDR_HI32(value));
-	bus_space_write_4(iot, ioh, offset + 4, BUS_ADDR_LO32(value));
-#endif
 }
 
 static void