From: Alexander Bluhm <bluhm@openbsd.org>
Subject: Re: virtio: Support unused virtqueues
To: Stefan Fritsch <sf@openbsd.org>
Cc: tech@openbsd.org
Date: Wed, 8 Jan 2025 20:59:24 +0100

On Tue, Jan 07, 2025 at 09:09:30AM +0100, Stefan Fritsch wrote:
> Hi,
> 
> another small preparatory diff that we will need for vio(4) multiqueue, if 
> the hypervisor offers more packet queues than we want to use. The control 
> queue comes after the packet queues.
> 
> ok?

I have tested it.  OK bluhm@

> Cheers,
> Stefan
> 
> 
> diff --git a/sys/dev/pv/virtio.c b/sys/dev/pv/virtio.c
> index 69eb26386ba..2d0c2386b87 100644
> --- a/sys/dev/pv/virtio.c
> +++ b/sys/dev/pv/virtio.c
> @@ -167,6 +167,8 @@ virtio_attach_finish(struct virtio_softc *sc, struct virtio_attach_args *va)
>  	for (i = 0; i < sc->sc_nvqs; i++) {
>  		struct virtqueue *vq = &sc->sc_vqs[i];
>  
> +		if (vq->vq_num == 0)
> +			continue;
>  		virtio_setup_queue(sc, vq, vq->vq_dmamap->dm_segs[0].ds_addr);
>  	}
>  	virtio_set_status(sc, VIRTIO_CONFIG_DEVICE_STATUS_DRIVER_OK);
> @@ -185,9 +187,9 @@ virtio_reinit_start(struct virtio_softc *sc)
>  	for (i = 0; i < sc->sc_nvqs; i++) {
>  		int n;
>  		struct virtqueue *vq = &sc->sc_vqs[i];
> -		n = virtio_read_queue_size(sc, vq->vq_index);
> -		if (n == 0)	/* vq disappeared */
> +		if (vq->vq_num == 0)	/* not used */
>  			continue;
> +		n = virtio_read_queue_size(sc, vq->vq_index);
>  		if (n != vq->vq_num) {
>  			panic("%s: virtqueue size changed, vq index %d",
>  			    sc->sc_dev.dv_xname, vq->vq_index);
> @@ -274,8 +276,11 @@ virtio_check_vqs(struct virtio_softc *sc)
>  	int i, r = 0;
>  
>  	/* going backwards is better for if_vio */
> -	for (i = sc->sc_nvqs - 1; i >= 0; i--)
> +	for (i = sc->sc_nvqs - 1; i >= 0; i--) {
> +		if (sc->sc_vqs[i].vq_num == 0)	/* not used */
> +			continue;
>  		r |= virtio_check_vq(sc, &sc->sc_vqs[i]);
> +	}
>  
>  	return r;
>  }
> @@ -305,6 +310,7 @@ virtio_init_vq(struct virtio_softc *sc, struct virtqueue *vq)
>  	int i, j;
>  	int vq_size = vq->vq_num;
>  
> +	VIRTIO_ASSERT(vq_size > 0);
>  	memset(vq->vq_vaddr, 0, vq->vq_bytesize);
>  
>  	/* build the indirect descriptor chain */
> @@ -468,6 +474,11 @@ virtio_free_vq(struct virtio_softc *sc, struct virtqueue *vq)
>  	struct vq_entry *qe;
>  	int i = 0;
>  
> +	if (vq->vq_num == 0) {
> +		/* virtio_alloc_vq() was never called */
> +		return 0;
> +	}
> +
>  	/* device must be already deactivated */
>  	/* confirm the vq is empty */
>  	SLIST_FOREACH(qe, &vq->vq_freelist, qe_list) {
> @@ -1017,6 +1028,10 @@ virtio_vq_dump(struct virtqueue *vq)
>  #endif
>  	/* Common fields */
>  	printf(" + addr: %p\n", vq);
> +	if (vq->vq_num == 0) {
> +		printf(" + vq is unused\n");
> +		return;
> +	}
>  	printf(" + vq num: %d\n", vq->vq_num);
>  	printf(" + vq mask: 0x%X\n", vq->vq_mask);
>  	printf(" + vq index: %d\n", vq->vq_index);
> diff --git a/sys/dev/pv/virtiovar.h b/sys/dev/pv/virtiovar.h
> index bed9ff0ce09..207e43ce9b8 100644
> --- a/sys/dev/pv/virtiovar.h
> +++ b/sys/dev/pv/virtiovar.h
> @@ -103,7 +103,8 @@ struct vq_entry {
>  
>  struct virtqueue {
>  	struct virtio_softc	*vq_owner;
> -	unsigned int		vq_num;  /* queue size (# of entries) */
> +	unsigned int		vq_num;  /* queue size (# of entries),
> +					  * 0 if unused/non-existant */
>  	unsigned int		vq_mask; /* (1 << vq_num - 1) */
>  	int			vq_index; /* queue number (0, 1, ...) */
>  
> @@ -180,7 +181,7 @@ struct virtio_softc {
>  	int			 sc_indirect;
>  	int			 sc_version_1;
>  
> -	int			 sc_nvqs;	/* set by child */
> +	int			 sc_nvqs;	/* size of sc_vqs, set by child */
>  	struct virtqueue	*sc_vqs;	/* set by child */
>  
>  	struct device		*sc_child;	/* set by child,