Re: [dpdk-dev] [PATCH 1/2] vhost: fix guest notification setting

["Xia, Chenbo" <chenbo.xia@intel.com>]

> -----Original Message-----
> From: Maxime Coquelin <maxime.coquelin@redhat.com>
> Sent: Thursday, July 23, 2020 9:09 PM
> To: dev@dpdk.org; matan@mellanox.com; Xia, Chenbo
> <chenbo.xia@intel.com>; Liu, Yong <yong.liu@intel.com>; Wang, Yinan
> <yinan.wang@intel.com>
> Cc: thomas@monjalon.net; Yigit, Ferruh <ferruh.yigit@intel.com>;
> david.marchand@redhat.com; Maxime Coquelin
> <maxime.coquelin@redhat.com>
> Subject: [PATCH 1/2] vhost: fix guest notification setting
> 
> If rte_vhost_enable_guest_notification is called before the virtqueue is ready,
> the configuration is lost.
> 
> This patch fixes this by saving the guest notification enablement value requested
> by the application, and apply it before the virtqueue is made ready to the
> application.
> 
> Fixes: 604052ae5395 ("net/vhost: support queue update")
> 
> Reported-by: Yinan Wang <yinan.wang@intel.com>
> Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com>
> ---
>  lib/librte_vhost/vhost.c      | 24 ++++++++++++++++++++----
>  lib/librte_vhost/vhost.h      |  5 +++++
>  lib/librte_vhost/vhost_user.c | 11 ++++++++---
>  3 files changed, 33 insertions(+), 7 deletions(-)
> 
> diff --git a/lib/librte_vhost/vhost.c b/lib/librte_vhost/vhost.c index
> 14b3e253e8..8f20a0818f 100644
> --- a/lib/librte_vhost/vhost.c
> +++ b/lib/librte_vhost/vhost.c
> @@ -534,6 +534,7 @@ init_vring_queue(struct virtio_net *dev, uint32_t
> vring_idx)
> 
>  	vq->kickfd = VIRTIO_UNINITIALIZED_EVENTFD;
>  	vq->callfd = VIRTIO_UNINITIALIZED_EVENTFD;
> +	vq->notif_enable = VIRTIO_UNINITIALIZED_NOTIF;
> 
>  	vhost_user_iotlb_init(dev, vring_idx);
>  	/* Backends are set to -1 indicating an inactive device. */ @@ -1311,6
> +1312,23 @@ vhost_enable_notify_packed(struct virtio_net *dev,
>  	return 0;
>  }
> 
> +int
> +vhost_enable_guest_notification(struct virtio_net *dev,
> +		struct vhost_virtqueue *vq, int enable) {
> +	/*
> +	 * If the virtqueue is not ready yet, it will be applied
> +	 * when it will become ready.
> +	 */
> +	if (!vq->ready)
> +		return 0;
> +
> +	if (vq_is_packed(dev))
> +		return vhost_enable_notify_packed(dev, vq, enable);
> +	else
> +		return vhost_enable_notify_split(dev, vq, enable); }
> +
>  int
>  rte_vhost_enable_guest_notification(int vid, uint16_t queue_id, int enable)
> { @@ -1325,10 +1343,8 @@ rte_vhost_enable_guest_notification(int vid,
> uint16_t queue_id, int enable)
> 
>  	rte_spinlock_lock(&vq->access_lock);
> 
> -	if (vq_is_packed(dev))
> -		ret = vhost_enable_notify_packed(dev, vq, enable);
> -	else
> -		ret = vhost_enable_notify_split(dev, vq, enable);
> +	vq->notif_enable = enable;
> +	ret = vhost_enable_guest_notification(dev, vq, enable);
> 
>  	rte_spinlock_unlock(&vq->access_lock);
> 
> diff --git a/lib/librte_vhost/vhost.h b/lib/librte_vhost/vhost.h index
> 0f7212f888..a29c6638e2 100644
> --- a/lib/librte_vhost/vhost.h
> +++ b/lib/librte_vhost/vhost.h
> @@ -164,6 +164,9 @@ struct vhost_virtqueue {
>  	int			enabled;
>  	int			access_ok;
>  	int			ready;
> +	int			notif_enable;
> +#define VIRTIO_UNINITIALIZED_NOTIF	(-1)
> +
>  	rte_spinlock_t		access_lock;
> 
>  	/* Used to notify the guest (trigger interrupt) */ @@ -668,6 +671,8 @@
> void vhost_enable_dequeue_zero_copy(int vid);  void
> vhost_set_builtin_virtio_net(int vid, bool enable);  void vhost_enable_extbuf(int
> vid);  void vhost_enable_linearbuf(int vid);
> +int vhost_enable_guest_notification(struct virtio_net *dev,
> +		struct vhost_virtqueue *vq, int enable);
> 
>  struct vhost_device_ops const *vhost_driver_callback_get(const char *path);
> 
> diff --git a/lib/librte_vhost/vhost_user.c b/lib/librte_vhost/vhost_user.c index
> 9ddeae3622..c3c924faec 100644
> --- a/lib/librte_vhost/vhost_user.c
> +++ b/lib/librte_vhost/vhost_user.c
> @@ -235,6 +235,11 @@ vhost_user_notify_queue_state(struct virtio_net *dev,
> uint16_t index,
>  			      int enable)
>  {
>  	struct rte_vdpa_device *vdpa_dev = dev->vdpa_dev;
> +	struct vhost_virtqueue *vq = dev->virtqueue[index];
> +
> +	/* Configure guest notifications on enable */
> +	if (enable && vq->notif_enable != VIRTIO_UNINITIALIZED_NOTIF)
> +		vhost_enable_guest_notification(dev, vq, vq->notif_enable);
> 
>  	if (vdpa_dev && vdpa_dev->ops->set_vring_state)
>  		vdpa_dev->ops->set_vring_state(dev->vid, index, enable); @@ -
> 1640,8 +1645,8 @@ vhost_user_set_vring_call(struct virtio_net **pdev, struct
> VhostUserMsg *msg,
>  	vq = dev->virtqueue[file.index];
> 
>  	if (vq->ready) {
> -		vhost_user_notify_queue_state(dev, file.index, 0);
>  		vq->ready = 0;
> +		vhost_user_notify_queue_state(dev, file.index, 0);
>  	}
> 
>  	if (vq->callfd >= 0)
> @@ -1903,8 +1908,8 @@ vhost_user_set_vring_kick(struct virtio_net **pdev,
> struct VhostUserMsg *msg,
>  	}
> 
>  	if (vq->ready) {
> -		vhost_user_notify_queue_state(dev, file.index, 0);
>  		vq->ready = 0;
> +		vhost_user_notify_queue_state(dev, file.index, 0);
>  	}
> 
>  	if (vq->kickfd >= 0)
> @@ -2917,8 +2922,8 @@ vhost_user_msg_handler(int vid, int fd)
>  		bool cur_ready = vq_is_ready(dev, vq);
> 
>  		if (cur_ready != (vq && vq->ready)) {
> -			vhost_user_notify_queue_state(dev, i, cur_ready);
>  			vq->ready = cur_ready;
> +			vhost_user_notify_queue_state(dev, i, cur_ready);
>  		}
>  	}
> 
> --
> 2.26.2

Reviewed-by: Chenbo Xia <chenbo.xia@intel.com>