From mboxrd@z Thu Jan 1 00:00:00 1970 From: Zang Hongyong Subject: Re: [Qemu-devel] [PATCH] virtio-serial: Allow one MSI-X vector per virtqueue Date: Mon, 19 Dec 2011 16:49:30 +0800 Message-ID: <4EEEFA9A.7080008@huawei.com> 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> Mime-Version: 1.0 Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: QUOTED-PRINTABLE Cc: qemu-devel@nongnu.org, kvm@vger.kernel.org, rusty@rustcorp.com.au, aliguori@us.ibm.com, xiaowei.yang@huawei.com, hanweidong@huawei.com, wusongwei@huawei.com, jiangningyu@huawei.com, "Michael S. Tsirkin" To: Amit Shah Return-path: Received: from szxga04-in.huawei.com ([119.145.14.67]:35112 "EHLO szxga04-in.huawei.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750930Ab1LSIuV (ORCPT ); Mon, 19 Dec 2011 03:50:21 -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 <0LWG00B880JVII@szxga04-in.huawei.com> for kvm@vger.kernel.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 kvm@vger.kernel.org; Mon, 19 Dec 2011 16:50:19 +0800 (CST) In-reply-to: <20111219072628.GB3139@amit-x200.redhat.com> Sender: kvm-owner@vger.kernel.org List-ID: =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 se= t msi-x >>>> with one vector per queue. But it fails and eventually all virtio-= serial >>>> ports share one MSI-X vector. Because every virtio-serial port has= *two* >>>> virtqueues, virtio-serial needs (port+1)*2 vectors other than (por= t+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 defi= ned in >>>> qemu: msix.c, all the queues still share one MSI-X vector as befor= e.) >>>> >>>> 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_UNSPE= CIFIED >>>> - ? proxy->serial.max_virts= erial_ports + 1 >>>> + ? (proxy->serial.max_virt= serial_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 the = max >>> allowed, virtio_init_pci() will end up using a shared vector instea= d >>> of separate ones. >>> >> Hi Amit, >> If the nvectors exceeds the max, msix_init() will return -EINVAL in = QEMU, >> and the front-end driver in Guest will use regular interrupt instead >> 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. > > Amit > > . > Yup, if the vectors exceeds the max, msix_init() should try to share on= e=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.