* [PATCH] vhost: fix vdpa driver multi-queue initialization failure @ 2023-01-28 3:06 Hao Chen 2023-01-28 3:06 ` Hao Chen 2023-02-03 10:38 ` Maxime Coquelin 0 siblings, 2 replies; 5+ messages in thread From: Hao Chen @ 2023-01-28 3:06 UTC (permalink / raw) To: dev; +Cc: zy, huangml, Maxime Coquelin, Chenbo Xia 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 <chenh@yusur.tech> --- 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)) -- 2.27.0 ^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH] vhost: fix vdpa driver multi-queue initialization failure 2023-01-28 3:06 [PATCH] vhost: fix vdpa driver multi-queue initialization failure Hao Chen @ 2023-01-28 3:06 ` Hao Chen 2023-02-01 12:23 ` Maxime Coquelin 2023-02-03 10:38 ` Maxime Coquelin 1 sibling, 1 reply; 5+ messages in thread From: Hao Chen @ 2023-01-28 3:06 UTC (permalink / raw) To: dev; +Cc: zy, huangml, Maxime Coquelin, Chenbo Xia 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 <chenh@yusur.tech> --- 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)) -- 2.27.0 ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] vhost: fix vdpa driver multi-queue initialization failure 2023-01-28 3:06 ` Hao Chen @ 2023-02-01 12:23 ` Maxime Coquelin 0 siblings, 0 replies; 5+ messages in thread From: Maxime Coquelin @ 2023-02-01 12:23 UTC (permalink / raw) To: Hao Chen, dev; +Cc: zy, huangml, Chenbo Xia 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 <chenh@yusur.tech> > --- > 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)) 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. What is the vDPA driver/device affected by this issue? Regards, Maxime ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] vhost: fix vdpa driver multi-queue initialization failure 2023-01-28 3:06 [PATCH] vhost: fix vdpa driver multi-queue initialization failure Hao Chen 2023-01-28 3:06 ` Hao Chen @ 2023-02-03 10:38 ` Maxime Coquelin 2023-02-08 9:56 ` Hao Chen 1 sibling, 1 reply; 5+ messages in thread From: Maxime Coquelin @ 2023-02-03 10:38 UTC (permalink / raw) To: Hao Chen, dev; +Cc: zy, huangml, Chenbo Xia 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 <chenh@yusur.tech> > --- > 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. What is the vDPA driver/device affected by this issue? Regards, Maxime ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] vhost: fix vdpa driver multi-queue initialization failure 2023-02-03 10:38 ` Maxime Coquelin @ 2023-02-08 9:56 ` Hao Chen 0 siblings, 0 replies; 5+ messages in thread From: Hao Chen @ 2023-02-08 9:56 UTC (permalink / raw) To: Maxime Coquelin, dev; +Cc: zy, huangml, Chenbo Xia, xiao.w.wang, vsrivast 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 <chenh@yusur.tech> >> --- >> 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 > ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2023-02-08 9:57 UTC | newest] Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2023-01-28 3:06 [PATCH] vhost: fix vdpa driver multi-queue initialization failure Hao Chen 2023-01-28 3:06 ` Hao Chen 2023-02-01 12:23 ` Maxime Coquelin 2023-02-03 10:38 ` Maxime Coquelin 2023-02-08 9:56 ` Hao Chen
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).