From: Stefan Fritsch <sf@sfritsch.de>
Subject: Re: [PATCH] vio: Restore device status after negotiation reset
To: Jan Klemkow <jan@openbsd.org>
Cc: tech@openbsd.org
Date: Sat, 10 Oct 2026 13:22:11 +0200

On Sat, 10 Oct 2026, Jan Klemkow wrote:

> On Thu, Oct 08, 2026 at 12:06:47PM +0200, Leander Kohler wrote:
> > The TSO and MTU fallback paths reset the device before renegotiating
> > features. Reset clears all status bits, but these paths do not restore
> > ACKNOWLEDGE and DRIVER before setting FEATURES_OK and DRIVER_OK.
> > This violates the initialization sequence required by Virtio §3.1.1 [0].
> > 
> > We restore both bits after each fallback reset. This addresses one
> > incompatibility with the Cloud Hypervisor VMM, which requires the
> > complete initialization status before activating the device.
> > 
> > [0] https://docs.oasis-open.org/virtio/virtio/v1.4/cs01/virtio-v1.4-cs01.html
> 
> Thanks for your diff and pointing out this issue.  As far as I
> understand the spec and our code, the following diff should also do the
> trick and avoids redundant calls of ACK and DRIVER.  It also avoids
> general VirtIO device behavior in a specific driver.
> 
> ok?

ok sf@

> 
> Thanks,
> Jan
> 
> Index: dev/fdt/virtio_mmio.c
> ===================================================================
> RCS file: /cvs/src/sys/dev/fdt/virtio_mmio.c,v
> diff -u -p -r1.24 virtio_mmio.c
> --- dev/fdt/virtio_mmio.c	22 Dec 2025 20:24:49 -0000	1.24
> +++ dev/fdt/virtio_mmio.c	9 Oct 2026 22:01:53 -0000
> @@ -305,9 +305,7 @@ virtio_mmio_attach(struct device *parent
>  	vsc->sc_dmat = sc->sc_dmat;
>  	sc->sc_config_offset = VIRTIO_MMIO_CONFIG;
>  
> -	virtio_device_reset(vsc);
> -	virtio_mmio_set_status(vsc, VIRTIO_CONFIG_DEVICE_STATUS_ACK);
> -	virtio_mmio_set_status(vsc, VIRTIO_CONFIG_DEVICE_STATUS_DRIVER);
> +	virtio_reset(vsc);
>  
>  	vma.vma_va.va_devid = id;
>  	vma.vma_va.va_nintr = 1;
> Index: dev/pci/virtio_pci.c
> ===================================================================
> RCS file: /cvs/src/sys/dev/pci/virtio_pci.c,v
> diff -u -p -r1.53 virtio_pci.c
> --- dev/pci/virtio_pci.c	22 Dec 2025 20:24:49 -0000	1.53
> +++ dev/pci/virtio_pci.c	9 Oct 2026 22:03:21 -0000
> @@ -664,9 +664,7 @@ virtio_pci_attach(struct device *parent,
>  		goto free;
>  	}
>  
> -	virtio_device_reset(vsc);
> -	virtio_set_status(vsc, VIRTIO_CONFIG_DEVICE_STATUS_ACK);
> -	virtio_set_status(vsc, VIRTIO_CONFIG_DEVICE_STATUS_DRIVER);
> +	virtio_reset(vsc);
>  
>  	printf("\n");
>  	vpa.vpa_va.va_devid = id;
> Index: dev/pv/virtio.c
> ===================================================================
> RCS file: /cvs/src/sys/dev/pv/virtio.c,v
> diff -u -p -r1.39 virtio.c
> --- dev/pv/virtio.c	16 Sep 2025 12:18:10 -0000	1.39
> +++ dev/pv/virtio.c	9 Oct 2026 21:45:34 -0000
> @@ -152,6 +152,8 @@ virtio_reset(struct virtio_softc *sc)
>  {
>  	virtio_device_reset(sc);
>  	sc->sc_active_features = 0;
> +	virtio_set_status(sc, VIRTIO_CONFIG_DEVICE_STATUS_ACK);
> +	virtio_set_status(sc, VIRTIO_CONFIG_DEVICE_STATUS_DRIVER);
>  }
>  
>  int
> @@ -180,8 +182,6 @@ virtio_reinit_start(struct virtio_softc 
>  {
>  	int i;
>  
> -	virtio_set_status(sc, VIRTIO_CONFIG_DEVICE_STATUS_ACK);
> -	virtio_set_status(sc, VIRTIO_CONFIG_DEVICE_STATUS_DRIVER);
>  	virtio_negotiate_features(sc, NULL);
>  	sc->sc_ops->setup_intrs(sc);
>  	for (i = 0; i < sc->sc_nvqs; i++) {
> 
>