Download raw body.
Virtio: Prefer 1.x over 0.9 (please test)
Hi,
tl;dr
Please test this diff if you run openbsd on non-qemu/non-vmd hypervisors
that use virtio, for example on bhyve or VirtualBox. Or if you have
unusual virtio configurations.
The virtio 1.x code has been in openbsd for several years. I think it is
time to make the drivers attach with 1.x instead of 0.9 if the hypervisor
offers both. This is important as only 1.x supports more than 32 feature
bits and virtio-network's multiqueue feature uses one of the high bits.
This only affects virtio_pci, as there are no virtio_mmio devices that
support both versions. The diff also tweaks the matching code to be more
compliant with the virtio 1.x standard which says "Drivers MUST match any
PCI Revision ID value". The diff should not change anything on openbsd
vmd, which only offers 0.9.
It could lead to a slight performance degradation in vioblk(4) which uses
the notify-on-empty feature as an optimization. That feature exists in 0.9
only. However, I did some simple benchmarks and the performance difference
was lost in the noise. I also have a diff that switches vioblk(4) to use
the event-index feature instead, but that needs some more polishing and
testing.
I have tested the diff on openbsd vmd and on qemu/kvm with various
configurations. If you run openbsd on other hypervisors that use virtio
devices, like bhyve or VirtualBox, I would be interested in test results.
Also if you run it on qemu with unusual virtio setups, like vhost-user. If
all of the virtio devices still attach successfully with the diff,
everything should be fine.
Comments welcome.
Thanks in advance.
Cheers,
Stefan
diff --git a/share/man/man4/virtio.4 b/share/man/man4/virtio.4
index 017dc55cddb..f6d80d6ee6f 100644
--- a/share/man/man4/virtio.4
+++ b/share/man/man4/virtio.4
@@ -60,9 +60,8 @@ The
driver conforms to the virtio 0.9.5 specification.
The virtio 1.0 standard is only supported for PCI devices.
.Pp
-By default 0.9 is preferred over 1.0.
-This can be changed by setting the bit 0x4 in the flags.
-Setting the bit 0x8 in the flags disables 1.0 support completely.
+By default 1.x is preferred over 0.9.
+This can be changed by setting the bit 0x8 in the flags.
.Sh SEE ALSO
.Xr intro 4
.Sh HISTORY
diff --git a/sys/dev/pci/virtio_pci.c b/sys/dev/pci/virtio_pci.c
index 2c59e899cad..e9dad0e2fff 100644
--- a/sys/dev/pci/virtio_pci.c
+++ b/sys/dev/pci/virtio_pci.c
@@ -362,8 +362,7 @@ virtio_pci_match(struct device *parent, void *match, void *aux)
return 1;
/* virtio 1.0 */
if (PCI_PRODUCT(pa->pa_id) >= 0x1040 &&
- PCI_PRODUCT(pa->pa_id) <= 0x107f &&
- PCI_REVISION(pa->pa_class) == 1)
+ PCI_PRODUCT(pa->pa_id) <= 0x107f)
return 1;
return 0;
}
@@ -595,21 +594,24 @@ virtio_pci_attach(struct device *parent, struct device *self, void *aux)
struct pci_attach_args *pa = (struct pci_attach_args *)aux;
pci_chipset_tag_t pc = pa->pa_pc;
pcitag_t tag = pa->pa_tag;
- int revision, ret = ENODEV;
+ int revision, product, vendor, ret = ENODEV, flags;
pcireg_t id;
struct virtio_pci_attach_args vpa = { { 0 }, pa };
revision = PCI_REVISION(pa->pa_class);
- switch (revision) {
- case 0:
- /* subsystem ID shows what I am */
+ product = PCI_PRODUCT(pa->pa_id);
+ vendor = PCI_VENDOR(pa->pa_id);
+ if (vendor == PCI_VENDOR_OPENBSD ||
+ (product >= 0x1000 && product <= 0x103f && revision == 0)) {
+ /* OpenBSD VMMCI and virtio 0.9 */
id = PCI_PRODUCT(pci_conf_read(pc, tag, PCI_SUBSYS_ID_REG));
- break;
- case 1:
- id = PCI_PRODUCT(pa->pa_id) - 0x1040;
- break;
- default:
- printf("unknown revision 0x%02x; giving up\n", revision);
+ } else if (product >= 0x1040 && product <= 0x107f) {
+ /* virtio 1.0 */
+ id = product - 0x1040;
+ revision = 1;
+ } else {
+ printf("unknown device prod 0x%04x rev 0x%02x; giving up\n",
+ product, revision);
return;
}
@@ -637,15 +639,15 @@ virtio_pci_attach(struct device *parent, struct device *self, void *aux)
M_DEVBUF, M_WAITOK | M_ZERO);
vsc->sc_ops = &virtio_pci_ops;
- if ((vsc->sc_dev.dv_cfdata->cf_flags & VIRTIO_CF_NO_VERSION_1) == 0 &&
- (revision == 1 ||
- (vsc->sc_dev.dv_cfdata->cf_flags & VIRTIO_CF_PREFER_VERSION_1))) {
+ flags = vsc->sc_dev.dv_cfdata->cf_flags;
+ if ((flags & VIRTIO_CF_PREFER_VERSION_09) == 0)
ret = virtio_pci_attach_10(sc, pa);
- }
if (ret != 0 && revision == 0) {
/* revision 0 means 0.9 only or both 0.9 and 1.0 */
ret = virtio_pci_attach_09(sc, pa);
}
+ if (ret != 0 && (flags & VIRTIO_CF_PREFER_VERSION_09))
+ ret = virtio_pci_attach_10(sc, pa);
if (ret != 0) {
printf(": Cannot attach (%d)\n", ret);
goto free;
diff --git a/sys/dev/pv/virtiovar.h b/sys/dev/pv/virtiovar.h
index 029f5743845..692af968557 100644
--- a/sys/dev/pv/virtiovar.h
+++ b/sys/dev/pv/virtiovar.h
@@ -82,8 +82,7 @@
/* flags for config(8) */
#define VIRTIO_CF_NO_INDIRECT 1
#define VIRTIO_CF_NO_EVENT_IDX 2
-#define VIRTIO_CF_PREFER_VERSION_1 4
-#define VIRTIO_CF_NO_VERSION_1 8
+#define VIRTIO_CF_PREFER_VERSION_09 8
struct virtio_attach_args {
int va_devid; /* virtio device id */
Virtio: Prefer 1.x over 0.9 (please test)