From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([140.186.70.92]:46438) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1RcYvZ-0007fv-5b for qemu-devel@nongnu.org; Mon, 19 Dec 2011 03:50:34 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1RcYvT-0003co-1V for qemu-devel@nongnu.org; Mon, 19 Dec 2011 03:50:33 -0500 Received: from szxga04-in.huawei.com ([119.145.14.67]:35133) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1RcYvS-0003cV-JP for qemu-devel@nongnu.org; Mon, 19 Dec 2011 03:50:27 -0500 Received: from huawei.com (szxga04-in [172.24.2.12]) by szxga04-in.huawei.com (iPlanet Messaging Server 5.2 HotFix 2.14 (built Aug 8 2006)) with ESMTP id <0LWG003I20JV13@szxga04-in.huawei.com> for qemu-devel@nongnu.org; Mon, 19 Dec 2011 16:50:19 +0800 (CST) Received: from szxrg01-dlp.huawei.com ([172.24.2.119]) by szxga04-in.huawei.com (iPlanet Messaging Server 5.2 HotFix 2.14 (built Aug 8 2006)) with ESMTP id <0LWG00F500JT91@szxga04-in.huawei.com> for qemu-devel@nongnu.org; Mon, 19 Dec 2011 16:50:19 +0800 (CST) Date: Mon, 19 Dec 2011 16:49:30 +0800 From: Zang Hongyong In-reply-to: <20111219072628.GB3139@amit-x200.redhat.com> Message-id: <4EEEFA9A.7080008@huawei.com> MIME-version: 1.0 Content-type: text/plain; format=flowed; charset=UTF-8 Content-transfer-encoding: QUOTED-PRINTABLE References: <1323998066-2396-1-git-send-email-zanghongyong@huawei.com> <20111216093945.GB3071@amit-x200.redhat.com> <4EEED527.4070103@huawei.com> <20111219072628.GB3139@amit-x200.redhat.com> Subject: Re: [Qemu-devel] [PATCH] virtio-serial: Allow one MSI-X vector per virtqueue List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Amit Shah Cc: aliguori@us.ibm.com, wusongwei@huawei.com, kvm@vger.kernel.org, hanweidong@huawei.com, rusty@rustcorp.com.au, qemu-devel@nongnu.org, xiaowei.yang@huawei.com, "Michael S. Tsirkin" , jiangningyu@huawei.com =E4=BA=8E 2011/12/19,=E6=98=9F=E6=9C=9F=E4=B8=80 15:26, Amit Shah = =E5=86=99=E9=81=93: > On (Mon) 19 Dec 2011 [14:09:43], Zang Hongyong wrote: >> =E4=BA=8E 2011/12/16,=E6=98=9F=E6=9C=9F=E4=BA=94 17:39, Amit Shah = =E5=86=99=E9=81=93: >>> On (Fri) 16 Dec 2011 [09:14:26], zanghongyong@huawei.com wrote: >>>> From: Hongyong Zang >>>> >>>> In pci_enable_msix(), the guest's virtio-serial driver tries to = set msi-x >>>> with one vector per queue. But it fails and eventually all virti= o-serial >>>> ports share one MSI-X vector. Because every virtio-serial port h= as *two* >>>> virtqueues, virtio-serial needs (port+1)*2 vectors other than (p= ort+1). >>> Ouch, good catch. >>> >>> One comment below: >>> >>>> This patch allows every virtqueue to have its own MSI-X vector. >>>> (When the MSI-X vectors needed are more than MSIX_MAX_ENTRIES de= fined in >>>> qemu: msix.c, all the queues still share one MSI-X vector as bef= ore.) >>>> >>>> Signed-off-by: Hongyong Zang >>>> --- >>>> hw/virtio-pci.c | 5 ++++- >>>> 1 files changed, 4 insertions(+), 1 deletions(-) >>>> >>>> diff --git a/hw/virtio-pci.c b/hw/virtio-pci.c >>>> index 77b75bc..2c9c6fb 100644 >>>> --- a/hw/virtio-pci.c >>>> +++ b/hw/virtio-pci.c >>>> @@ -718,8 +718,11 @@ static int virtio_serial_init_pci(PCIDevice= *pci_dev) >>>> return -1; >>>> } >>>> vdev->nvectors =3D proxy->nvectors =3D=3D DEV_NVECTORS_UNS= PECIFIED >>>> - ? proxy->serial.max_vir= tserial_ports + 1 >>>> + ? (proxy->serial.max_vi= rtserial_ports + 1) * 2 >>>> : proxy->nvectors; >>>> + /*msix.c: #define MSIX_MAX_ENTRIES 32*/ >>>> + if (vdev->nvectors> 32) >>>> + vdev->nvectors =3D 32; >>> This change isn't needed: if the proxy->nvectors value exceeds th= e max >>> allowed, virtio_init_pci() will end up using a shared vector inst= ead >>> of separate ones. >>> >> Hi Amit, >> If the nvectors exceeds the max, msix_init() will return -EINVAL i= n QEMU, >> and the front-end driver in Guest will use regular interrupt inste= ad >> of MSI-X. > In that case, I believe msix_init() should be changed to attempt to > share interrupts instead of drivers doing this by themselves. > > =09=09Amit > > . > Yup, if the vectors exceeds the max, msix_init() should try to share = one=20 vector or use the max vectors directly. Or we may change the MSIX_MAX_ENTRIES defined in misx.c: It allocs one page for the msix table(reserve second half of the page= =20 for pending bits), and one table entry only needs four DWORDs in PCI= =20 SPEC, so I think the max entries could be 128 instead of 32.