On 7/2/2015 10:16 AM, Ouyang, Changchun wrote: > >> -----Original Message----- >> From: Xie, Huawei >> Sent: Thursday, July 2, 2015 10:02 AM >> To: Ouyang, Changchun; dev at dpdk.org; Thomas Monjalon >> Subject: Re: [dpdk-dev] [PATCH] virtio: fix the vq size issue >> >> On 7/2/2015 8:29 AM, Ouyang, Changchun wrote: >>> Hi huawei, >>> >>>> -----Original Message----- >>>> From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Xie, Huawei >>>> Sent: Wednesday, July 1, 2015 11:53 PM >>>> To: dev at dpdk.org; Thomas Monjalon >>>> Subject: Re: [dpdk-dev] [PATCH] virtio: fix the vq size issue >>>> >>>> On 7/1/2015 3:49 PM, Ouyang Changchun wrote: >>>>> This commit breaks virtio basic packets rx functionality: >>>>> d78deadae4dca240e85054bf2d604a801676becc >>>>> >>>>> The QEMU use 256 as default vring size, also use this default value >>>>> to calculate the virtio avail ring base address and used ring base >>>>> address, and vhost in the backend use the ring base address to do >>>>> packet >>>> IO. >>>>> Virtio spec also says the queue size in PCI configuration is >>>>> read-only, so virtio front end can't change it. just need use the >>>>> read-only value to allocate space for vring and calculate the avail >>>>> and used ring base address. Otherwise, the avail and used ring base >>>> address will be different between host and guest, accordingly, packet >>>> IO can't work normally. >>>> virtio driver could still use the vq_size to initialize avail ring >>>> and use ring so that they still have the same base address. >>>> The other issue is vhost use index & (vq->size -1) to index the ring. >>> I am not sure what is your clear message here, Vhost has no choice but >>> use vq->size -1 to index the ring, It is qemu that always use 256 as >>> the vq size, and set the avail and used ring base address, It also >>> tells vhost the vq size is 256. >> I mean "the same base address issue" could be resolved, but we still couldn't >> stop vhost using idx & vq->size -1 to index the ring. >> > Then this patch will resolve this avail ring base address issue. I mean different ring base isn't the root cause. The commit message which states that this register is read only is simple and enough.
>>>> Thomas: >>>> This fix works but introduces slight change with original code. Could >>>> we just rollback that commit? >>> What's your major concern for the slight change here? >>> just removing the unnecessary check for nb_desc itself. >>> So I think no issue for the slight change. >> No major concern. It is better if this patch just rollbacks that commit >> without >> introduce extra change if not necessary. >> The original code set nb_desc to vq_size, though it isn't used later. >> > I prefer to have the slight change to remove unnecessary setting. > >>> Thanks >>> Changchun >>> >>> >>> >>> > >
