From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124]) by inbox.dpdk.org (Postfix) with ESMTP id 8BD1F41C3D; Wed, 8 Feb 2023 10:57:00 +0100 (CET) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 6DB4040DFD; Wed, 8 Feb 2023 10:57:00 +0100 (CET) Received: from out28-218.mail.aliyun.com (out28-218.mail.aliyun.com [115.124.28.218]) by mails.dpdk.org (Postfix) with ESMTP id 08CFE40141 for ; Wed, 8 Feb 2023 10:56:58 +0100 (CET) X-Alimail-AntiSpam: AC=CONTINUE; BC=0.07526142|-1; CH=green; DM=|CONTINUE|false|; DS=CONTINUE|ham_system_inform|0.00623628-0.00136405-0.9924; FP=0|0|0|0|0|-1|-1|-1; HT=ay29a033018047208; MF=chenh@yusur.tech; NM=1; PH=DS; RN=7; RT=7; SR=0; TI=SMTPD_---.RFWyRk8_1675850215; Received: from 192.168.10.190(mailfrom:chenh@yusur.tech fp:SMTPD_---.RFWyRk8_1675850215) by smtp.aliyun-inc.com; Wed, 08 Feb 2023 17:56:56 +0800 Message-ID: <9ed018e6-17bf-2e8e-3349-6a52bd89cb2f@yusur.tech> Date: Wed, 8 Feb 2023 17:56:12 +0800 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:102.0) Gecko/20100101 Thunderbird/102.6.1 Subject: Re: [PATCH] vhost: fix vdpa driver multi-queue initialization failure To: Maxime Coquelin , dev@dpdk.org Cc: zy@yusur.tech, huangml@yusur.tech, Chenbo Xia , xiao.w.wang@intel.com, vsrivast@xilinx.com References: <20230128030603.1168073-1-chenh@yusur.tech> <0f2f9d8c-486b-06cb-dd79-00de982f0f82@redhat.com> From: Hao Chen In-Reply-To: <0f2f9d8c-486b-06cb-dd79-00de982f0f82@redhat.com> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org On 2023/2/3 18:38, Maxime Coquelin wrote: > > > On 1/28/23 04:06, Hao Chen wrote: >> When 'virtio_is_Ready' returns 1, 'vdpa_dev->ops->dev_conf' will be >> called, and the vdpa driver called 'rte_vhost_get_vhost_vring' to >> get the vring addr info from 'vhost_devices->virtqueue[]'. >> virtio_is_ready's nr_vring is VIRTIO_BUILTIN_NUM_VQS_TO_BE_READY(2), >> multi-queue's nr_vring is greater than 2. >> Only 'vhost_devices->virtqueue[0]' and 'vhost_devices->virtqueue[1]' >> are obtained in the for loop. Other queues of multiple queues cannot >> obtain the corresponding 'vhost_devices->virtqueue[i]', which will >> cause 'vdpa_dev->ops->dev_conf' to obtain QEMU vring addr information >> from 'vhost_devices->virtqueue[i]' failed. >> Here, nr_ving is modified to dev->nr_vring, so that multiple queues >> can obtain the corresponding 'vhost_devices->virtqueue[i]'. >> >> Signed-off-by: Hao Chen >> --- >>   lib/vhost/vhost_user.c | 2 +- >>   1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/lib/vhost/vhost_user.c b/lib/vhost/vhost_user.c >> index 9902ae9944..9249eafc06 100644 >> --- a/lib/vhost/vhost_user.c >> +++ b/lib/vhost/vhost_user.c >> @@ -1473,7 +1473,7 @@ virtio_is_ready(struct virtio_net *dev) >>       if (dev->nr_vring < nr_vring) >>           return 0; >>   -    for (i = 0; i < nr_vring; i++) { >> +    for (i = 0; i < dev->nr_vring; i++) { >>           vq = dev->virtqueue[i]; >>             if (!vq_is_ready(dev, vq)) > > (reiterate my reply since the patch was posted twice, and I replied to > the one discarded in PW) > > Nack. > > We can start processing the vrings as soon as the first queue pair is > initialized. I think you may want to rely on > vdpa_dev->ops->set_vring_state to initialize other vrings. > Does this mean that the queue should be initialized and enabled in vdpa_dev->ops->set_vring_state, not in vdpa_dev->ops->dev_conf? > What is the vDPA driver/device affected by this issue? After tracking the code, I found that the dpdk-vdpa example's log shows the calling order of multiple queues as follow: " VHOST_USER_SET_VRING_ADDR VHOST_USER_SET_VRING_KICK vdpa_dev->ops->set_vring_state  (vq_idx=0) VHOST_USER_SET_VRING_CALL vdpa_dev->ops->set_vring_state  (vq_idx=0) ... VHOST_USER_SET_VRING_ADDR VHOST_USER_SET_VRING_KICK vdpa_dev->ops->set_vring_state  (vq_idx=1) VHOST_USER_SET_VRING_CALL vdpa_dev->ops->set_vring_state  (vq_idx=1) " then virtio_is_ready return 1, vdpa_dev->ops->dev_conf is called. After that, other queues will call vdpa_dev->ops->set_vring_state (vq_idx=i) The dev->virtqueue[]->desc/available/used is assigned in VHOST_USER_SET_VRING_KICK's translate_ring_addresses() , so vdpa driver should not initialize all queues in vdpa_dev->ops->dev_conf , otherwise other queue's dev->virtqueue[i]->desc/available/used are NULL , vring's hpa to gpa will fail by rte_vhost_get_vhost_vring(). dev->virtqueue[i]->desc/available/used should be accessed after queue(i)'s VHOST_USER_SET_VRING_KICK is called. I see that all of ifc/sfc vdpa driver's queues acessed all queues's dev->virtqueue[i]->desc/available/used in vdpa_dev->ops->dev_conf, maybe they should move the code? > > Regards, > Maxime >