Download raw body.
virtio: Fix condition for split rx buffers, negotiate VIRTIO_F_ANY_LAYOUT
Hi,
In 0.9-only time, the VIO_HAVE_MRG_RXBUF macro checked just for the
VIRTIO_NET_F_MRG_RXBUF feature. The meaning was later changed to
(version_1 || VIRTIO_NET_F_MRG_RXBUF), but the new meaning is only correct
for one use of the macro. The buffer chaining must check for the MRG_RXBUF
feature exclusively.
On the other hand, the check if we have to split the header from the rest
of the buffer in the rx queue is a workaround for old kvm versions. The
standard has since then gained the ANY_LAYOUT feature flag to turn off
this workaround. According to the virtio 1.x standard, we should accept
VIRTIO_F_ANY_LAYOUT if it is offered for transitional devices. ANY_LAYOUT
is implicit if VERSION_1 has been negotiated.
Since accepting ANY_LAYOUT only relaxes the requirements for us, we can
simply accept it globally for all virtio device types. vioblk(4) and
vioscsi(4) unconditionally use the strict buffer layout required for
legacy devices without ANY_LAYOUT, anyway.
Problem noticed by helg@
ok?
Cheers,
Stefan
diff --git a/sys/dev/fdt/virtio_mmio.c b/sys/dev/fdt/virtio_mmio.c
index db5f5381d0e..179988771f5 100644
--- a/sys/dev/fdt/virtio_mmio.c
+++ b/sys/dev/fdt/virtio_mmio.c
@@ -407,6 +407,7 @@ virtio_mmio_negotiate_features(struct virtio_softc *vsc,
vsc->sc_driver_features &= ~(VIRTIO_F_RING_EVENT_IDX);
}
+ vsc->sc_driver_features |= VIRTIO_F_ANY_LAYOUT;
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,
diff --git a/sys/dev/pci/virtio_pci.c b/sys/dev/pci/virtio_pci.c
index 0f7d0d1bbcc..b61a162ddc7 100644
--- a/sys/dev/pci/virtio_pci.c
+++ b/sys/dev/pci/virtio_pci.c
@@ -798,6 +798,7 @@ virtio_pci_negotiate_features(struct virtio_softc *vsc,
uint64_t host, negotiated;
vsc->sc_active_features = 0;
+ vsc->sc_driver_features |= VIRTIO_F_ANY_LAYOUT;
/*
* We enable indirect descriptors by default. They can be switched
diff --git a/sys/dev/pv/if_vio.c b/sys/dev/pv/if_vio.c
index 90d7ad8522b..d5c0767162d 100644
--- a/sys/dev/pv/if_vio.c
+++ b/sys/dev/pv/if_vio.c
@@ -295,8 +295,6 @@ struct vio_softc {
#define VIO_DMAMEM_SYNC(vsc, sc, p, size, flags) \
bus_dmamap_sync((vsc)->sc_dmat, (sc)->sc_dma_map, \
VIO_DMAMEM_OFFSET((sc), (p)), (size), (flags))
-#define VIO_HAVE_MRG_RXBUF(sc) \
- ((sc)->sc_hdr_size == sizeof(struct virtio_net_hdr))
/* vioq N uses the rx/tx vq pair 2*N and 2*N + 1 */
#define VIO_VQ2Q(sc, vq) (&sc->sc_q[vq->vq_index/2])
@@ -1398,7 +1396,9 @@ vio_populate_rx_mbufs(struct vio_softc *sc, struct vio_queue *vioq)
int r, done = 0;
u_int slots;
struct virtqueue *vq = vioq->viq_rxvq;
- int mrg_rxbuf = VIO_HAVE_MRG_RXBUF(sc);
+ int no_split = virtio_has_feature(vsc, VIRTIO_NET_F_MRG_RXBUF) ||
+ virtio_has_feature(vsc, VIRTIO_F_VERSION_1) ||
+ virtio_has_feature(vsc, VIRTIO_F_ANY_LAYOUT);
MUTEX_ASSERT_LOCKED(&vioq->viq_rxmtx);
for (slots = if_rxr_get(&vioq->viq_rxring, vq->vq_num);
@@ -1418,7 +1418,7 @@ vio_populate_rx_mbufs(struct vio_softc *sc, struct vio_queue *vioq)
}
}
r = virtio_enqueue_reserve(vq, slot,
- vioq->viq_rxdmamaps[slot]->dm_nsegs + (mrg_rxbuf ? 0 : 1));
+ vioq->viq_rxdmamaps[slot]->dm_nsegs + (no_split ? 0 : 1));
if (r != 0) {
vio_free_rx_mbuf(sc, vioq, slot);
break;
@@ -1426,13 +1426,13 @@ vio_populate_rx_mbufs(struct vio_softc *sc, struct vio_queue *vioq)
bus_dmamap_sync(vsc->sc_dmat, vioq->viq_rxdmamaps[slot], 0,
vioq->viq_rxdmamaps[slot]->dm_mapsize,
BUS_DMASYNC_PREREAD);
- if (mrg_rxbuf) {
+ if (no_split) {
virtio_enqueue(vq, slot, vioq->viq_rxdmamaps[slot], 0);
} else {
/*
- * Buggy kvm wants a buffer of exactly the size of
- * the header in this case, so we have to split in
- * two.
+ * Legacy devices without VIRTIO_F_ANY_LAYOUT want a
+ * buffer of exactly the size of the header in this
+ * case, so we have to split in two.
*/
virtio_enqueue_p(vq, slot, vioq->viq_rxdmamaps[slot],
0, sc->sc_hdr_size, 0);
@@ -1526,7 +1526,7 @@ vio_rxeof(struct vio_queue *vioq)
vioq->viq_ifiq->ifiq_idx;
SET(m->m_pkthdr.csum_flags, M_FLOWID);
}
- if (VIO_HAVE_MRG_RXBUF(sc))
+ if (virtio_has_feature(vsc, VIRTIO_NET_F_MRG_RXBUF))
bufs_left = hdr->num_buffers - 1;
else
bufs_left = 0;
virtio: Fix condition for split rx buffers, negotiate VIRTIO_F_ANY_LAYOUT