From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga11.intel.com (mga11.intel.com [192.55.52.93]) by dpdk.org (Postfix) with ESMTP id 220BF12A8 for ; Thu, 2 Jul 2015 11:16:25 +0200 (CEST) Received: from fmsmga003.fm.intel.com ([10.253.24.29]) by fmsmga102.fm.intel.com with ESMTP; 02 Jul 2015 02:16:26 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.15,391,1432623600"; d="scan'208";a="517703374" Received: from pgsmsx108.gar.corp.intel.com ([10.221.44.103]) by FMSMGA003.fm.intel.com with ESMTP; 02 Jul 2015 02:16:23 -0700 Received: from shsmsx104.ccr.corp.intel.com (10.239.4.70) by PGSMSX108.gar.corp.intel.com (10.221.44.103) with Microsoft SMTP Server (TLS) id 14.3.224.2; Thu, 2 Jul 2015 17:15:50 +0800 Received: from shsmsx101.ccr.corp.intel.com ([169.254.1.246]) by SHSMSX104.ccr.corp.intel.com ([169.254.5.129]) with mapi id 14.03.0224.002; Thu, 2 Jul 2015 17:15:48 +0800 From: "Xie, Huawei" To: "Ouyang, Changchun" Thread-Topic: [dpdk-dev] [PATCH] virtio: fix the vq size issue Thread-Index: AdC0FhKoXdj2uCB1RLi+8Fm6XGN5OA== Date: Thu, 2 Jul 2015 09:15:48 +0000 Message-ID: References: <1435736930-26737-1-git-send-email-changchun.ouyang@intel.com> Accept-Language: en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-originating-ip: [10.239.127.40] Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Cc: "dev@dpdk.org" Subject: Re: [dpdk-dev] [PATCH] virtio: fix the vq size issue X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: patches and discussions about DPDK List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 02 Jul 2015 09:16:26 -0000 On 7/2/2015 10:16 AM, Ouyang, Changchun wrote:=0A= >=0A= >> -----Original Message-----=0A= >> From: Xie, Huawei=0A= >> Sent: Thursday, July 2, 2015 10:02 AM=0A= >> To: Ouyang, Changchun; dev@dpdk.org; Thomas Monjalon=0A= >> Subject: Re: [dpdk-dev] [PATCH] virtio: fix the vq size issue=0A= >>=0A= >> On 7/2/2015 8:29 AM, Ouyang, Changchun wrote:=0A= >>> Hi huawei,=0A= >>>=0A= >>>> -----Original Message-----=0A= >>>> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Xie, Huawei=0A= >>>> Sent: Wednesday, July 1, 2015 11:53 PM=0A= >>>> To: dev@dpdk.org; Thomas Monjalon=0A= >>>> Subject: Re: [dpdk-dev] [PATCH] virtio: fix the vq size issue=0A= >>>>=0A= >>>> On 7/1/2015 3:49 PM, Ouyang Changchun wrote:=0A= >>>>> This commit breaks virtio basic packets rx functionality:=0A= >>>>> d78deadae4dca240e85054bf2d604a801676becc=0A= >>>>>=0A= >>>>> The QEMU use 256 as default vring size, also use this default value= =0A= >>>>> to calculate the virtio avail ring base address and used ring base=0A= >>>>> address, and vhost in the backend use the ring base address to do=0A= >>>>> packet=0A= >>>> IO.=0A= >>>>> Virtio spec also says the queue size in PCI configuration is=0A= >>>>> read-only, so virtio front end can't change it. just need use the=0A= >>>>> read-only value to allocate space for vring and calculate the avail= =0A= >>>>> and used ring base address. Otherwise, the avail and used ring base= =0A= >>>> address will be different between host and guest, accordingly, packet= =0A= >>>> IO can't work normally.=0A= >>>> virtio driver could still use the vq_size to initialize avail ring=0A= >>>> and use ring so that they still have the same base address.=0A= >>>> The other issue is vhost use index & (vq->size -1) to index the ring.= =0A= >>> I am not sure what is your clear message here, Vhost has no choice but= =0A= >>> use vq->size -1 to index the ring, It is qemu that always use 256 as=0A= >>> the vq size, and set the avail and used ring base address, It also=0A= >>> tells vhost the vq size is 256.=0A= >> I mean "the same base address issue" could be resolved, but we still cou= ldn't=0A= >> stop vhost using idx & vq->size -1 to index the ring.=0A= >>=0A= > Then this patch will resolve this avail ring base address issue.=0A= I mean different ring base isn't the root cause. The commit message=0A= which states that this register is read only is simple and enough.=0A= =0A= >>>> Thomas:=0A= >>>> This fix works but introduces slight change with original code. Could= =0A= >>>> we just rollback that commit?=0A= >>> What's your major concern for the slight change here?=0A= >>> just removing the unnecessary check for nb_desc itself.=0A= >>> So I think no issue for the slight change.=0A= >> No major concern. It is better if this patch just rollbacks that commit = without=0A= >> introduce extra change if not necessary.=0A= >> The original code set nb_desc to vq_size, though it isn't used later.=0A= >>=0A= > I prefer to have the slight change to remove unnecessary setting.=0A= >=0A= >>> Thanks=0A= >>> Changchun=0A= >>>=0A= >>>=0A= >>>=0A= >>>=0A= >=0A= >=0A= =0A=