Re: [PATCH 1/2] virtio-pci: add setup_vqs flag in vp_try_to_find_vqs

["Michael S. Tsirkin" <mst@redhat.com>]

On Thu, Jan 12, 2012 at 09:20:06AM +0800, zanghongyong@huawei.com wrote:
> From: Hongyong Zang <zanghongyong@huawei.com>
> 
> changes in vp_try_to_find_vqs:
> Virtio-serial's probe() calls it to request irqs and setup vqs of port0 and
> controls; add_port() calls it to set up vqs of io_port.
> it will not create virtqueue if the name is null.
> 
> Signed-off-by: Hongyong Zang <zanghongyong@huawei.com>

This looks like a fragile hack, to me.

virtio spec also implied that VQ initialization is done
before status is set to OK (during probe) and
devices might have relied on that. So if we want to change
that, I think we need a feature bit.

Besides, what about documentation? non pci transports?

> ---
>  drivers/virtio/virtio_pci.c |   17 +++++++++++++----
>  1 files changed, 13 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/virtio/virtio_pci.c b/drivers/virtio/virtio_pci.c
> index baabb79..1f98c36 100644
> --- a/drivers/virtio/virtio_pci.c
> +++ b/drivers/virtio/virtio_pci.c
> @@ -492,9 +492,11 @@ static void vp_del_vqs(struct virtio_device *vdev)
>  	list_for_each_entry_safe(vq, n, &vdev->vqs, list) {
>  		info = vq->priv;
>  		if (vp_dev->per_vq_vectors &&
> -			info->msix_vector != VIRTIO_MSI_NO_VECTOR)
> +			info->msix_vector != VIRTIO_MSI_NO_VECTOR) {
>  			free_irq(vp_dev->msix_entries[info->msix_vector].vector,
>  				 vq);
> +			vp_dev->msix_used_vectors--;
> +		}
>  		vp_del_vq(vq);
>  	}
>  	vp_dev->per_vq_vectors = false;
> @@ -511,7 +513,10 @@ static int vp_try_to_find_vqs(struct virtio_device *vdev, unsigned nvqs,
>  {
>  	struct virtio_pci_device *vp_dev = to_vp_device(vdev);
>  	u16 msix_vec;
> -	int i, err, nvectors, allocated_vectors;
> +	int i, err, nvectors;
> +
> +	if (vp_dev->msix_used_vectors)
> +		goto setup_vqs;

Please don't abuse goto in this way.

>  
>  	if (!use_msix) {
>  		/* Old style: one normal interrupt for change and all vqs. */
> @@ -536,12 +541,16 @@ static int vp_try_to_find_vqs(struct virtio_device *vdev, unsigned nvqs,
>  	}
>  
>  	vp_dev->per_vq_vectors = per_vq_vectors;
> -	allocated_vectors = vp_dev->msix_used_vectors;
> +
> +setup_vqs:
>  	for (i = 0; i < nvqs; ++i) {
> +		if (names[i] == NULL)

if (![names[i])

> +			continue;
> +
>  		if (!callbacks[i] || !vp_dev->msix_enabled)
>  			msix_vec = VIRTIO_MSI_NO_VECTOR;
>  		else if (vp_dev->per_vq_vectors)
> -			msix_vec = allocated_vectors++;
> +			msix_vec = vp_dev->msix_used_vectors++;
>  		else
>  			msix_vec = VP_MSIX_VQ_VECTOR;
>  		vqs[i] = setup_vq(vdev, i, callbacks[i], names[i], msix_vec);
> -- 
> 1.7.1
> 
> --
> To unsubscribe from this list: send the line "unsubscribe kvm" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html