kern/47780: if_vioif doesn't work with QEMU >= 1.4.0 due to features negotiation issues

[aurelien%aurel32.net@localhost]

>Number:         47780
>Category:       kern
>Synopsis:       if_vioif doesn't work with QEMU >= 1.4.0 due to features 
>negotiation issues
>Confidential:   no
>Severity:       serious
>Priority:       medium
>Responsible:    kern-bug-people
>State:          open
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Mon Apr 29 06:45:00 +0000 2013
>Originator:     Aurelien Jarno
>Release:        6.0.1
>Organization:
>Environment:
NetBSD netbsd.aurel32.net 6.0.1 NetBSD 6.0.1 (GENERIC) #0: Mon Apr 29 02:07:25 
CEST 2013  
aurel32%netbsd.aurel32.net@localhost:/usr/src/sys/arch/amd64/compile/GENERIC 
amd64
>Description:
NetBSD includes paravirtualized network drivers to run under QEMU/KVM. While 
they work fine with QEMU < 1.4.0, they fail on QEMU >= 1.4.0 due to features 
negotiation issues on the NetBSD side. In vioif_attach() the negotiation is 
working correctly, however the features set is not saved. Later when the device 
is reset calling vioif_stop() and vioif_start(), the features are renogociated, 
using sc->sc_features which has not be initialized and thus equal to 0. This 
causes the host to drop support for some features, while the NetBSD guest later 
tries to use them. This in turn causes protocol issues (in this case NetBSD 
tries to write to the control queue, which has been disabled by the host), and 
causes QEMU to abort.

The attached patch fixes the issue.
>How-To-Repeat:
Run NetBSD under QEMU 1.4.0 with a virtio network interface:

  qemu-system-x86-64 -cdrom NetBSD-6.0.1-i386.iso -net nic,model=virtio -net 
user

And set and IP address on the virtio interface. This will causes QEMU to abort 
as NetBSD is trying to do forbidden things with the virtio network interface.
>Fix:
Index: if_vioif.c
===================================================================
RCS file: /cvsroot/src/sys/dev/pci/if_vioif.c,v
retrieving revision 1.2
diff -u -r1.2 if_vioif.c
--- if_vioif.c  19 Nov 2011 12:32:54 -0000      1.2
+++ if_vioif.c  29 Apr 2013 06:11:43 -0000
@@ -459,7 +459,6 @@
 {
        struct vioif_softc *sc = device_private(self);
        struct virtio_softc *vsc = device_private(parent);
-       uint32_t features;
        struct ifnet *ifp = &sc->sc_ethercom.ec_if;
 
        if (vsc->sc_child != NULL) {
@@ -478,13 +477,13 @@
        vsc->sc_config_change = 0;
        vsc->sc_intrhand = virtio_vq_intr;
 
-       features = virtio_negotiate_features(vsc,
-                                            (VIRTIO_NET_F_MAC |
-                                             VIRTIO_NET_F_STATUS |
-                                             VIRTIO_NET_F_CTRL_VQ |
-                                             VIRTIO_NET_F_CTRL_RX |
-                                             VIRTIO_F_NOTIFY_ON_EMPTY));
-       if (features & VIRTIO_NET_F_MAC) {
+       sc->sc_features = virtio_negotiate_features(vsc,
+                                                   (VIRTIO_NET_F_MAC |
+                                                    VIRTIO_NET_F_STATUS |
+                                                    VIRTIO_NET_F_CTRL_VQ |
+                                                    VIRTIO_NET_F_CTRL_RX |
+                                                    VIRTIO_F_NOTIFY_ON_EMPTY));
+       if (sc->sc_features & VIRTIO_NET_F_MAC) {
                sc->sc_mac[0] = virtio_read_device_config_1(vsc,
                                                    VIRTIO_NET_CONFIG_MAC+0);
                sc->sc_mac[1] = virtio_read_device_config_1(vsc,
@@ -544,8 +543,8 @@
        sc->sc_vq[1].vq_done = vioif_tx_vq_done;
        virtio_start_vq_intr(vsc, &sc->sc_vq[0]);
        virtio_stop_vq_intr(vsc, &sc->sc_vq[1]); /* not urgent; do it later */
-       if ((features & VIRTIO_NET_F_CTRL_VQ)
-           && (features & VIRTIO_NET_F_CTRL_RX)) {
+       if ((sc->sc_features & VIRTIO_NET_F_CTRL_VQ)
+           && (sc->sc_features & VIRTIO_NET_F_CTRL_RX)) {
                if (virtio_alloc_vq(vsc, &sc->sc_vq[2], 2,
                                    NBPG, 1, "control") == 0) {
                        sc->sc_vq[2].vq_done = vioif_ctrl_vq_done;