From be79a2c60fec0b4ead6369e6ce420a664a0dfa9d Mon Sep 17 00:00:00 2001
From: Jessica Clarke <jrtc27@FreeBSD.org>
Date: Thu, 21 Jan 2021 01:03:44 +0000
Subject: [PATCH] virtio_mmio: Fix V1 device probing spec conformance (section
 4.2.3.1.1)

We must check MagicValue not just Version before anything else, and then
we must check DeviceID and immediately abort if zero (and this must not
be an error).

Do all this when probing rather than at the start of attaching as that's
where this belongs, and provides a clear boundary between the device
detection and device initialisation parts of the specified driver
initialisation process. This also means we don't create empty device
instances for placeholder devices, reducing clutter on systems that
pre-allocate a large number, such as QEMU's AArch64 virt machine (which
provides 32).

Reviewed by:	bryanv
Differential Revision:	https://reviews.freebsd.org/D28070
---
 sys/dev/virtio/mmio/virtio_mmio.c      | 50 +++++++++++++++++++++-----
 sys/dev/virtio/mmio/virtio_mmio.h      |  2 ++
 sys/dev/virtio/mmio/virtio_mmio_acpi.c |  3 +-
 sys/dev/virtio/mmio/virtio_mmio_fdt.c  |  3 +-
 4 files changed, 46 insertions(+), 12 deletions(-)

diff --git a/sys/dev/virtio/mmio/virtio_mmio.c b/sys/dev/virtio/mmio/virtio_mmio.c
index c16ecd40a25..862272b917d 100644
--- a/sys/dev/virtio/mmio/virtio_mmio.c
+++ b/sys/dev/virtio/mmio/virtio_mmio.c
@@ -171,6 +171,48 @@ DEFINE_CLASS_0(virtio_mmio, vtmmio_driver, vtmmio_methods,
 
 MODULE_VERSION(virtio_mmio, 1);
 
+int
+vtmmio_probe(device_t dev)
+{
+	struct vtmmio_softc *sc;
+	int rid;
+	uint32_t magic, version;
+
+	sc = device_get_softc(dev);
+
+	rid = 0;
+	sc->res[0] = bus_alloc_resource_any(dev, SYS_RES_MEMORY, &rid,
+	    RF_ACTIVE);
+	if (sc->res[0] == NULL) {
+		device_printf(dev, "Cannot allocate memory window.\n");
+		return (ENXIO);
+	}
+
+	magic = vtmmio_read_config_4(sc, VIRTIO_MMIO_MAGIC_VALUE);
+	if (magic != VIRTIO_MMIO_MAGIC_VIRT) {
+		device_printf(dev, "Bad magic value %#x\n", magic);
+		bus_release_resource(dev, SYS_RES_MEMORY, rid, sc->res[0]);
+		return (ENXIO);
+	}
+
+	version = vtmmio_read_config_4(sc, VIRTIO_MMIO_VERSION);
+	if (version < 1 || version > 2) {
+		device_printf(dev, "Unsupported version: %#x\n", version);
+		bus_release_resource(dev, SYS_RES_MEMORY, rid, sc->res[0]);
+		return (ENXIO);
+	}
+
+	if (vtmmio_read_config_4(sc, VIRTIO_MMIO_DEVICE_ID) == 0) {
+		bus_release_resource(dev, SYS_RES_MEMORY, rid, sc->res[0]);
+		return (ENXIO);
+	}
+
+	bus_release_resource(dev, SYS_RES_MEMORY, rid, sc->res[0]);
+
+	device_set_desc(dev, "VirtIO MMIO adapter");
+	return (BUS_PROBE_DEFAULT);
+}
+
 static int
 vtmmio_setup_intr(device_t dev, enum intr_type type)
 {
@@ -225,14 +267,6 @@ vtmmio_attach(device_t dev)
 	}
 
 	sc->vtmmio_version = vtmmio_read_config_4(sc, VIRTIO_MMIO_VERSION);
-	if (sc->vtmmio_version < 1 || sc->vtmmio_version > 2) {
-		device_printf(dev, "Unsupported version: %x\n",
-		    sc->vtmmio_version);
-		bus_release_resource(dev, SYS_RES_MEMORY, 0,
-		    sc->res[0]);
-		sc->res[0] = NULL;
-		return (ENXIO);
-	}
 
 	vtmmio_reset(sc);
 
diff --git a/sys/dev/virtio/mmio/virtio_mmio.h b/sys/dev/virtio/mmio/virtio_mmio.h
index ce60cd4fa17..5eb56d86078 100644
--- a/sys/dev/virtio/mmio/virtio_mmio.h
+++ b/sys/dev/virtio/mmio/virtio_mmio.h
@@ -55,6 +55,7 @@ struct vtmmio_softc {
 	void				*ih;
 };
 
+int vtmmio_probe(device_t);
 int vtmmio_attach(device_t);
 
 #define	VIRTIO_MMIO_MAGIC_VALUE		0x000
@@ -84,6 +85,7 @@ int vtmmio_attach(device_t);
 #define	VIRTIO_MMIO_QUEUE_USED_HIGH	0x0a4	/* requires version 2 */
 #define	VIRTIO_MMIO_CONFIG_GENERATION	0x100	/* requires version 2 */
 #define	VIRTIO_MMIO_CONFIG		0x100
+#define	VIRTIO_MMIO_MAGIC_VIRT		0x74726976
 #define	VIRTIO_MMIO_INT_VRING		(1 << 0)
 #define	VIRTIO_MMIO_INT_CONFIG		(1 << 1)
 #define	VIRTIO_MMIO_VRING_ALIGN		4096
diff --git a/sys/dev/virtio/mmio/virtio_mmio_acpi.c b/sys/dev/virtio/mmio/virtio_mmio_acpi.c
index bd737a6e53d..0cb34f5aa87 100644
--- a/sys/dev/virtio/mmio/virtio_mmio_acpi.c
+++ b/sys/dev/virtio/mmio/virtio_mmio_acpi.c
@@ -81,6 +81,5 @@ vtmmio_acpi_probe(device_t dev)
 	if (!acpi_MatchHid(h, "LNRO0005"))
 		return (ENXIO);
 
-	device_set_desc(dev, "VirtIO MMIO adapter");
-	return (BUS_PROBE_DEFAULT);
+	return (vtmmio_probe(dev));
 }
diff --git a/sys/dev/virtio/mmio/virtio_mmio_fdt.c b/sys/dev/virtio/mmio/virtio_mmio_fdt.c
index a6867176a07..a188df70794 100644
--- a/sys/dev/virtio/mmio/virtio_mmio_fdt.c
+++ b/sys/dev/virtio/mmio/virtio_mmio_fdt.c
@@ -97,8 +97,7 @@ vtmmio_fdt_probe(device_t dev)
 	if (!ofw_bus_is_compatible(dev, "virtio,mmio"))
 		return (ENXIO);
 
-	device_set_desc(dev, "VirtIO MMIO adapter");
-	return (BUS_PROBE_DEFAULT);
+	return (vtmmio_probe(dev));
 }
 
 static int