Re: kern/58049: vioif(4) panics if the virtq size is too small

[Tetsuya Isaki <isaki%pastel-flower.jp@localhost>]

At Mon, 18 Mar 2024 08:51:45 -0700,
Jason Thorpe wrote:
> When the DMA map is created, do we not yet know the max number of
> segments?  Or is the problem that different queues might have
> different segment counts and the DMA maps are shared between them?
> In a perfect world, this extra test isn't needed because the bus_dma
> back-end honors the device's maximum segment count.

Thank you for comment.
I have found the cause of this problem.  This is the revised patch.

--- src/sys/dev/pci/if_vioif.c
+++ src/sys/dev/pci/if_vioif.c
@@ -1280,12 +1280,14 @@ vioif_alloc_mems(struct vioif_softc *sc)
 
 		struct virtio_net_hdr *hdrs;
 		int dir;
+		int nsegs;
 
 		dir = VIOIF_NETQ_DIR(qid);
 		netq = &sc->sc_netqs[qid];
 		vq_num = netq->netq_vq->vq_num;
 		maps = netq->netq_maps;
 		hdrs = netq->netq_maps_kva;
+		nsegs = uimin(dmaparams[dir].dma_nsegs, vq_num - 1/*hdr*/);
 
 		for (i = 0; i < vq_num; i++) {
 			maps[i].vnm_hdr = &hdrs[i];
@@ -1297,7 +1299,7 @@ vioif_alloc_mems(struct vioif_softc *sc)
 				goto err_reqs;
 
 			r = vioif_dmamap_create(sc, &maps[i].vnm_mbuf_map,
-			    dmaparams[dir].dma_size, dmaparams[dir].dma_nsegs,
+			    dmaparams[dir].dma_size, nsegs,
 			    dmaparams[dir].msg_payload);
 			if (r != 0)
 				goto err_reqs;


vioif_net_load_mbuf() (in vioif_send_common_locked()) loaded more
than 8 segments(fragments) despite (my) virtio queue size was 8.
So I tried to defragment in the previous patch.  But it was not
correct as you pointed out.

This is because vioif_alloc_mems() told bus_dma that it was able to
handle up to hardcoded VIRTIO_NET_TX_MAXNSEGS (=16) segments during
initialization, despite my virtio queue size was 8(!).
If the virtq size is less than or equal to VIRTIO_NET_TX_MAXNSEGS,
the max number of dmamap segments for payload must be virtq size - 1.

Thanks,
---
Tetsuya Isaki <isaki%pastel-flower.jp@localhost / isaki%NetBSD.org@localhost>