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
This commit is contained in:
Jessica Clarke 2021-01-21 01:03:44 +00:00
parent 0e72f2c541
commit be79a2c60f
4 changed files with 46 additions and 12 deletions

View File

@ -171,6 +171,48 @@ DEFINE_CLASS_0(virtio_mmio, vtmmio_driver, vtmmio_methods,
MODULE_VERSION(virtio_mmio, 1); 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 static int
vtmmio_setup_intr(device_t dev, enum intr_type type) 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); 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); vtmmio_reset(sc);

View File

@ -55,6 +55,7 @@ struct vtmmio_softc {
void *ih; void *ih;
}; };
int vtmmio_probe(device_t);
int vtmmio_attach(device_t); int vtmmio_attach(device_t);
#define VIRTIO_MMIO_MAGIC_VALUE 0x000 #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_QUEUE_USED_HIGH 0x0a4 /* requires version 2 */
#define VIRTIO_MMIO_CONFIG_GENERATION 0x100 /* requires version 2 */ #define VIRTIO_MMIO_CONFIG_GENERATION 0x100 /* requires version 2 */
#define VIRTIO_MMIO_CONFIG 0x100 #define VIRTIO_MMIO_CONFIG 0x100
#define VIRTIO_MMIO_MAGIC_VIRT 0x74726976
#define VIRTIO_MMIO_INT_VRING (1 << 0) #define VIRTIO_MMIO_INT_VRING (1 << 0)
#define VIRTIO_MMIO_INT_CONFIG (1 << 1) #define VIRTIO_MMIO_INT_CONFIG (1 << 1)
#define VIRTIO_MMIO_VRING_ALIGN 4096 #define VIRTIO_MMIO_VRING_ALIGN 4096

View File

@ -81,6 +81,5 @@ vtmmio_acpi_probe(device_t dev)
if (!acpi_MatchHid(h, "LNRO0005")) if (!acpi_MatchHid(h, "LNRO0005"))
return (ENXIO); return (ENXIO);
device_set_desc(dev, "VirtIO MMIO adapter"); return (vtmmio_probe(dev));
return (BUS_PROBE_DEFAULT);
} }

View File

@ -97,8 +97,7 @@ vtmmio_fdt_probe(device_t dev)
if (!ofw_bus_is_compatible(dev, "virtio,mmio")) if (!ofw_bus_is_compatible(dev, "virtio,mmio"))
return (ENXIO); return (ENXIO);
device_set_desc(dev, "VirtIO MMIO adapter"); return (vtmmio_probe(dev));
return (BUS_PROBE_DEFAULT);
} }
static int static int