Hi Markku, On Wed, 29 Jul 2026, Markku Rossi wrote:
> >Synopsis: virtio-gpu does not work with virtio_mmio.c > >Category: kernel > >Environment: > System : OpenBSD 7.9 > Details : OpenBSD 7.9 (GOEMU) #1: Wed Jul 29 07:01:37 CEST 2026 > [email protected]: > /usr/src/sys/arch/ris > cv64/compile/GOEMU > > Architecture: OpenBSD.riscv64 > Machine : riscv64 > >Description: > > I am developing my own RISC-V 64 emulator > (https://github.com/markkurossi/riscv). The emulator implements only > the VirtIO MMIO drivers. On OpenBSD, the VirtIO GPU (virtio-gpu) > device does not initialize with the `virtio_mmio.c' driver. The > `viogpu.c' driver checks: > > if (!vsc->sc_version_1) { > printf(": requires virtio version 1\n"); > goto err; > } > > The `vsc->sc_version_1' field is initialized in the `virtio_pci.c' > file but not in the `virtio_mmio.c' file. The `virtio_mmio.c' does a > version check and a trivial fix is to initialize the > `vsc->sc_version_1' field if `sc->sc_version == 2' (patch below). There is more stuff missing for correct virtio 1.x mmio support. Your diff alone did not make vio(4) attached at virtio-mmio work on linux/KVM/arm64. Does the diff below work for you? Cheers, Stefan diff --git a/sys/dev/fdt/virtio_mmio.c b/sys/dev/fdt/virtio_mmio.c index f673c7dcd4c..de7c529b989 100644 --- a/sys/dev/fdt/virtio_mmio.c +++ b/sys/dev/fdt/virtio_mmio.c @@ -119,7 +119,6 @@ struct virtio_mmio_softc { void *sc_ih; int sc_config_offset; - uint32_t sc_version; }; struct virtio_mmio_attach_args { @@ -183,12 +182,16 @@ virtio_mmio_setup_queue(struct virtio_softc *vsc, struct virtqueue *vq, vq->vq_index); bus_space_write_4(sc->sc_iot, sc->sc_ioh, VIRTIO_MMIO_QUEUE_NUM, bus_space_read_4(sc->sc_iot, sc->sc_ioh, VIRTIO_MMIO_QUEUE_NUM_MAX)); - if (sc->sc_version == 1) { + if (!vsc->sc_version_1) { bus_space_write_4(sc->sc_iot, sc->sc_ioh, VIRTIO_MMIO_QUEUE_ALIGN, PAGE_SIZE); bus_space_write_4(sc->sc_iot, sc->sc_ioh, VIRTIO_MMIO_QUEUE_PFN, addr / VIRTIO_PAGE_SIZE); } else { + if (addr == 0) { + bus_space_write_4(sc->sc_iot, sc->sc_ioh, + VIRTIO_MMIO_QUEUE_READY, 0); + } bus_space_write_4(sc->sc_iot, sc->sc_ioh, VIRTIO_MMIO_QUEUE_DESC_LOW, addr); bus_space_write_4(sc->sc_iot, sc->sc_ioh, @@ -205,8 +208,10 @@ virtio_mmio_setup_queue(struct virtio_softc *vsc, struct virtqueue *vq, bus_space_write_4(sc->sc_iot, sc->sc_ioh, VIRTIO_MMIO_QUEUE_USED_HIGH, (addr + vq->vq_usedoffset) >> 32); - bus_space_write_4(sc->sc_iot, sc->sc_ioh, - VIRTIO_MMIO_QUEUE_READY, 1); + if (addr != 0) { + bus_space_write_4(sc->sc_iot, sc->sc_ioh, + VIRTIO_MMIO_QUEUE_READY, 1); + } } } @@ -259,7 +264,7 @@ virtio_mmio_attach(struct device *parent, struct device *self, void *aux) struct fdt_attach_args *faa = aux; struct virtio_mmio_softc *sc = (struct virtio_mmio_softc *)self; struct virtio_softc *vsc = &sc->sc_sc; - uint32_t id, magic; + uint32_t id, magic, version; struct virtio_mmio_attach_args vma = { { 0 }, faa }; if (faa->fa_nreg < 1) { @@ -281,11 +286,20 @@ virtio_mmio_attach(struct device *parent, struct device *self, void *aux) return; } - sc->sc_version = bus_space_read_4(sc->sc_iot, sc->sc_ioh, + version = bus_space_read_4(sc->sc_iot, sc->sc_ioh, VIRTIO_MMIO_VERSION); - if (sc->sc_version < 1 || sc->sc_version > 2) { - printf(": unknown version 0x%02x; giving up\n", sc->sc_version); - return; + switch (version) { + case 1: + /* virtio 0.9 */ + break; + case 2: + /* virtio 1.x */ + vsc->sc_version_1 = 1; + break; + default: + printf(": unknown version 0x%02x; giving up\n", + version); + return; } id = bus_space_read_4(sc->sc_iot, sc->sc_ioh, VIRTIO_MMIO_DEVICE_ID); @@ -297,7 +311,7 @@ virtio_mmio_attach(struct device *parent, struct device *self, void *aux) if (id == 0) return; - if (sc->sc_version == 1) + if (!vsc->sc_version_1) bus_space_write_4(sc->sc_iot, sc->sc_ioh, VIRTIO_MMIO_GUEST_PAGE_SIZE, PAGE_SIZE); @@ -408,10 +422,20 @@ virtio_mmio_negotiate_features(struct virtio_softc *vsc, } vsc->sc_driver_features |= VIRTIO_F_ANY_LAYOUT; + if (vsc->sc_version_1) { + vsc->sc_driver_features |= VIRTIO_F_VERSION_1; + /* notify on empty is 0.9 only */ + vsc->sc_driver_features &= ~VIRTIO_F_NOTIFY_ON_EMPTY; + } + bus_space_write_4(sc->sc_iot, sc->sc_ioh, VIRTIO_MMIO_HOST_FEATURES_SEL, 0); host = bus_space_read_4(sc->sc_iot, sc->sc_ioh, - VIRTIO_MMIO_HOST_FEATURES); + VIRTIO_MMIO_HOST_FEATURES); + bus_space_write_4(sc->sc_iot, sc->sc_ioh, + VIRTIO_MMIO_HOST_FEATURES_SEL, 1); + host |= (uint64_t)bus_space_read_4(sc->sc_iot, sc->sc_ioh, + VIRTIO_MMIO_HOST_FEATURES) << 32; neg = host & vsc->sc_driver_features; #if VIRTIO_DEBUG if (guest_feature_names) @@ -420,7 +444,33 @@ virtio_mmio_negotiate_features(struct virtio_softc *vsc, bus_space_write_4(sc->sc_iot, sc->sc_ioh, VIRTIO_MMIO_GUEST_FEATURES_SEL, 0); bus_space_write_4(sc->sc_iot, sc->sc_ioh, - VIRTIO_MMIO_GUEST_FEATURES, neg); + VIRTIO_MMIO_GUEST_FEATURES, neg); + bus_space_write_4(sc->sc_iot, sc->sc_ioh, + VIRTIO_MMIO_GUEST_FEATURES_SEL, 1); + bus_space_write_4(sc->sc_iot, sc->sc_ioh, + VIRTIO_MMIO_GUEST_FEATURES, neg >> 32); + + if (vsc->sc_version_1) { + virtio_mmio_set_status(vsc, + VIRTIO_CONFIG_DEVICE_STATUS_FEATURES_OK); + if ((virtio_mmio_get_status(vsc) & + VIRTIO_CONFIG_DEVICE_STATUS_FEATURES_OK) == 0) { + printf("%s: Feature negotiation failed\n", + vsc->sc_dev.dv_xname); + virtio_mmio_set_status(vsc, + VIRTIO_CONFIG_DEVICE_STATUS_FAILED); + return ENXIO; + } + if ((neg & VIRTIO_F_VERSION_1) == 0) { +#if VIRTIO_DEBUG + printf("%s: Host rejected Version_1\n", __func__); +#endif + virtio_mmio_set_status(vsc, + VIRTIO_CONFIG_DEVICE_STATUS_FAILED); + return EINVAL; + } + } + vsc->sc_active_features = neg; if (neg & VIRTIO_F_RING_INDIRECT_DESC) vsc->sc_indirect = 1; @@ -519,18 +569,22 @@ virtio_mmio_intr(void *arg) { struct virtio_mmio_softc *sc = arg; struct virtio_softc *vsc = &sc->sc_sc; - int isr, r = 0; + int isr, r = 0, handled = 0; /* check and ack the interrupt */ isr = bus_space_read_4(sc->sc_iot, sc->sc_ioh, VIRTIO_MMIO_INTERRUPT_STATUS); - bus_space_write_4(sc->sc_iot, sc->sc_ioh, - VIRTIO_MMIO_INTERRUPT_ACK, isr); if ((isr & VIRTIO_MMIO_INT_CONFIG) && - (vsc->sc_config_change != NULL)) + (vsc->sc_config_change != NULL)) { r = (vsc->sc_config_change)(vsc); - if ((isr & VIRTIO_MMIO_INT_VRING)) + handled |= VIRTIO_MMIO_INT_CONFIG; + } + if ((isr & VIRTIO_MMIO_INT_VRING)) { r |= virtio_check_vqs(vsc); + handled |= VIRTIO_MMIO_INT_VRING; + } + bus_space_write_4(sc->sc_iot, sc->sc_ioh, + VIRTIO_MMIO_INTERRUPT_ACK, handled); return r; } diff --git a/sys/dev/pv/if_vio.c b/sys/dev/pv/if_vio.c index 8942b02857a..cb1dd8bf003 100644 --- a/sys/dev/pv/if_vio.c +++ b/sys/dev/pv/if_vio.c @@ -685,6 +685,8 @@ negotiate: */ want_tso = 0; virtio_reset(vsc); + virtio_set_status(vsc, VIRTIO_CONFIG_DEVICE_STATUS_ACK); + virtio_set_status(vsc, VIRTIO_CONFIG_DEVICE_STATUS_DRIVER); goto negotiate; } @@ -702,6 +704,8 @@ negotiate: device_mtu = 0; want_mtu = 0; virtio_reset(vsc); + virtio_set_status(vsc, VIRTIO_CONFIG_DEVICE_STATUS_ACK); + virtio_set_status(vsc, VIRTIO_CONFIG_DEVICE_STATUS_DRIVER); goto negotiate; } }
