Hi > -----Original Message----- > From: Maxime Coquelin [mailto:maxime.coque...@redhat.com] > Sent: Tuesday, September 3, 2019 3:35 PM > To: Wang, Xiao W <xiao.w.w...@intel.com>; Ye, Xiaolong > <xiaolong...@intel.com> > Cc: Bie, Tiwei <tiwei....@intel.com>; Wang, Zhihong > <zhihong.w...@intel.com>; amore...@redhat.com; dev@dpdk.org; > jfreim...@redhat.com; sta...@dpdk.org > Subject: Re: [PATCH 02/15] vhost: configure vDPA as soon as the device is > ready > > > > On 9/2/19 11:02 AM, Wang, Xiao W wrote: > > Hi, > > > >> -----Original Message----- > >> From: Ye, Xiaolong > >> Sent: Monday, September 2, 2019 4:35 PM > >> To: Maxime Coquelin <maxime.coque...@redhat.com> > >> Cc: Bie, Tiwei <tiwei....@intel.com>; Wang, Zhihong > >> <zhihong.w...@intel.com>; amore...@redhat.com; Wang, Xiao W > >> <xiao.w.w...@intel.com>; dev@dpdk.org; jfreim...@redhat.com; > >> sta...@dpdk.org > >> Subject: Re: [PATCH 02/15] vhost: configure vDPA as soon as the device is > >> ready > >> > >> On 08/29, Maxime Coquelin wrote: > >>> There might not have any VHOST_USER_SET_VRING_CALL requests > >>> sent once virtio device is ready. When it happens, the vDPA > >>> device's dev_conf() callback may never be called. > >>> > >>> Fixes: 9f9014512822 ("vhost: configure vDPA device after set vring call > >> message") > >>> Cc: sta...@dpdk.org > >>> Cc: xiaolong...@intel.com > >>> > >>> Signed-off-by: Maxime Coquelin <maxime.coque...@redhat.com> > >>> --- > >>> lib/librte_vhost/vhost_user.c | 3 +-- > >>> 1 file changed, 1 insertion(+), 2 deletions(-) > >>> > >>> diff --git a/lib/librte_vhost/vhost_user.c b/lib/librte_vhost/vhost_user.c > >>> index 0b72648a5..b1ea80c52 100644 > >>> --- a/lib/librte_vhost/vhost_user.c > >>> +++ b/lib/librte_vhost/vhost_user.c > >>> @@ -2112,8 +2112,7 @@ vhost_user_msg_handler(int vid, int fd) > >>> did = dev->vdpa_dev_id; > >>> vdpa_dev = rte_vdpa_get_device(did); > >>> if (vdpa_dev && virtio_is_ready(dev) && > >>> - !(dev->flags & VIRTIO_DEV_VDPA_CONFIGURED) && > >>> - msg.request.master == > >> VHOST_USER_SET_VRING_CALL) { > >>> + !(dev->flags & VIRTIO_DEV_VDPA_CONFIGURED)) { > > > > In the early beginning of vhost user messages, there seems to be a > VHOST_USER_SET_VRING_CALL with invalid call fd, > > not sure if QEMU has any update on this point. > > If the virtio_is_ready() is based on that invalid call fd, then > > vdpa_dev_conf() > cannot setup interrupt properly. > > I think that's why in our previous implementation, we wait for the real > > call fd > and then call dev_conf(). > > I think that when we receive the first SET_VRING_CALL request from Qemu, > virtio_is_ready() should be false because the rings addresses won't be > received yet.
In the last phase of vhost communication, there're usually a sequence of messages "SET_VRING_KICK" and "SET_VRING_CALL". With this patch, at the arrival of "SET_VRING_KICK", virtio_is_ready() will return true, and the invalid call fd will get involved into dev_con(). I'm not sure if the invalid callfd implementation still exist in latest QEMU, if yes, We have to handle this case. > > Did it fixed a real issue, or was it based on code/logs review? In an old implementation with QEMU 2.6, I met the issue. To verify it, you need to Setup a VM (maybe nested) and verify if virtio can get an interrupt. BRs, Xiao > > Thanks for the explanations, > Maxime > > > BRs, > > Xiao > >
