From mboxrd@z Thu Jan 1 00:00:00 1970 From: "Michael S. Tsirkin" Subject: Re: [PATCH 1/2] virtio-pci: add setup_vqs flag in vp_try_to_find_vqs Date: Wed, 1 Feb 2012 11:54:09 +0200 Message-ID: <20120201095407.GB27435@redhat.com> References: <1326331207-10339-1-git-send-email-zanghongyong@huawei.com> <1326331207-10339-2-git-send-email-zanghongyong@huawei.com> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Cc: kvm@vger.kernel.org, rusty@rustcorp.com.au, amit.shah@redhat.com, aliguori@us.ibm.com, xiaowei.yang@huawei.com, hanweidong@huawei.com, wusongwei@huawei.com, jiangningyu@huawei.com To: zanghongyong@huawei.com Return-path: Received: from mx1.redhat.com ([209.132.183.28]:61569 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753308Ab2BAJyQ (ORCPT ); Wed, 1 Feb 2012 04:54:16 -0500 Content-Disposition: inline In-Reply-To: <1326331207-10339-2-git-send-email-zanghongyong@huawei.com> Sender: kvm-owner@vger.kernel.org List-ID: On Thu, Jan 12, 2012 at 09:20:06AM +0800, zanghongyong@huawei.com wrote: > From: Hongyong Zang > > 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 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