[dpdk-dev] [PATCH] vhost: translate addrs in set_vring_addr if ring already enabled

[dpdk-dev] [PATCH] vhost: translate addrs in set_vring_addr if ring already enabled

[Maxime Coquelin]

Commit 3ea7052f4b1b ("vhost: postpone rings addresses translation")
moves rings addresses translation at either vring kick or enable
time, depending on whether protocol features are enabled or not.
This is done not interpret ring information as long as the vring
is not fully initialized.

The problem is that with old QEMU versions, like v2.5, the ring
is enabled before addresses are sent, so addresses are never
translated.

This patch fixes the issue by doing the translation in
VHOST_USER_SET_VRING_ADDR handling if ring is already enabled.

Reported-by: Yao, Lei A <lei.a.yao@intel.com>
Fixes: 3ea7052f4b1b ("vhost: postpone rings addresses translation")
Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com>
---
 lib/librte_vhost/vhost_user.c | 66 +++++++++++++++++++++++++------------------
 1 file changed, 38 insertions(+), 28 deletions(-)

diff --git a/lib/librte_vhost/vhost_user.c b/lib/librte_vhost/vhost_user.c
index 76c4eeca5..1f6cba4b9 100644
--- a/lib/librte_vhost/vhost_user.c
+++ b/lib/librte_vhost/vhost_user.c
@@ -372,33 +372,6 @@ ring_addr_to_vva(struct virtio_net *dev, struct vhost_virtqueue *vq,
 	return qva_to_vva(dev, ra);
 }
 
-/*
- * The virtio device sends us the desc, used and avail ring addresses.
- * This function then converts these to our address space.
- */
-static int
-vhost_user_set_vring_addr(struct virtio_net *dev, VhostUserMsg *msg)
-{
-	struct vhost_virtqueue *vq;
-	struct vhost_vring_addr *addr = &msg->payload.addr;
-
-	if (dev->mem == NULL)
-		return -1;
-
-	/* addr->index refers to the queue index. The txq 1, rxq is 0. */
-	vq = dev->virtqueue[msg->payload.addr.index];
-
-	/*
-	 * Rings addresses should not be interpreted as long as the ring is not
-	 * started and enabled
-	 */
-	memcpy(&vq->ring_addrs, addr, sizeof(*addr));
-
-	vring_invalidate(dev, vq);
-
-	return 0;
-}
-
 static struct virtio_net *
 translate_ring_addresses(struct virtio_net *dev, int vq_index)
 {
@@ -464,6 +437,43 @@ translate_ring_addresses(struct virtio_net *dev, int vq_index)
 }
 
 /*
+ * The virtio device sends us the desc, used and avail ring addresses.
+ * This function then converts these to our address space.
+ */
+static int
+vhost_user_set_vring_addr(struct virtio_net **pdev, VhostUserMsg *msg)
+{
+	struct vhost_virtqueue *vq;
+	struct vhost_vring_addr *addr = &msg->payload.addr;
+	struct virtio_net *dev = *pdev;
+
+	if (dev->mem == NULL)
+		return -1;
+
+	/* addr->index refers to the queue index. The txq 1, rxq is 0. */
+	vq = dev->virtqueue[msg->payload.addr.index];
+
+	/*
+	 * Rings addresses should not be interpreted as long as the ring is not
+	 * started and enabled
+	 */
+	memcpy(&vq->ring_addrs, addr, sizeof(*addr));
+
+	vring_invalidate(dev, vq);
+
+	if (vq->enabled && (dev->features &
+				(1ULL << VHOST_USER_F_PROTOCOL_FEATURES))) {
+		dev = translate_ring_addresses(dev, msg->payload.state.index);
+		if (!dev)
+			return -1;
+
+		*pdev = dev;
+	}
+
+	return 0;
+}
+
+/*
  * The virtio device sends us the available ring last used index.
  */
 static int
@@ -1273,7 +1283,7 @@ vhost_user_msg_handler(int vid, int fd)
 		vhost_user_set_vring_num(dev, &msg);
 		break;
 	case VHOST_USER_SET_VRING_ADDR:
-		vhost_user_set_vring_addr(dev, &msg);
+		vhost_user_set_vring_addr(&dev, &msg);
 		break;
 	case VHOST_USER_SET_VRING_BASE:
 		vhost_user_set_vring_base(dev, &msg);
-- 
2.13.6

[dpdk-dev] [PATCH v2] vhost: translate addrs in set_vring_addr if ring already enabled

[Maxime Coquelin]

Commit 3ea7052f4b1b ("vhost: postpone rings addresses translation")
moves rings addresses translation at either vring kick or enable
time, depending on whether protocol features are enabled or not.
This is done not interpret ring information as long as the vring
is not fully initialized.

The problem is that with old QEMU versions, like v2.5, the ring
is enabled before addresses are sent, so addresses are never
translated.

This patch fixes the issue by doing the translation in
VHOST_USER_SET_VRING_ADDR handling if ring is already enabled.

Fixes: 3ea7052f4b1b ("vhost: postpone rings addresses translation")

Reported-by: Lei Yao <lei.a.yao@intel.com>
Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com>
---
v2: Fixed commit informations (Fixes:, Reported-by:)

 lib/librte_vhost/vhost_user.c | 66 +++++++++++++++++++++++++------------------
 1 file changed, 38 insertions(+), 28 deletions(-)

diff --git a/lib/librte_vhost/vhost_user.c b/lib/librte_vhost/vhost_user.c
index 76c4eeca5..1f6cba4b9 100644
--- a/lib/librte_vhost/vhost_user.c
+++ b/lib/librte_vhost/vhost_user.c
@@ -372,33 +372,6 @@ ring_addr_to_vva(struct virtio_net *dev, struct vhost_virtqueue *vq,
 	return qva_to_vva(dev, ra);
 }
 
-/*
- * The virtio device sends us the desc, used and avail ring addresses.
- * This function then converts these to our address space.
- */
-static int
-vhost_user_set_vring_addr(struct virtio_net *dev, VhostUserMsg *msg)
-{
-	struct vhost_virtqueue *vq;
-	struct vhost_vring_addr *addr = &msg->payload.addr;
-
-	if (dev->mem == NULL)
-		return -1;
-
-	/* addr->index refers to the queue index. The txq 1, rxq is 0. */
-	vq = dev->virtqueue[msg->payload.addr.index];
-
-	/*
-	 * Rings addresses should not be interpreted as long as the ring is not
-	 * started and enabled
-	 */
-	memcpy(&vq->ring_addrs, addr, sizeof(*addr));
-
-	vring_invalidate(dev, vq);
-
-	return 0;
-}
-
 static struct virtio_net *
 translate_ring_addresses(struct virtio_net *dev, int vq_index)
 {
@@ -464,6 +437,43 @@ translate_ring_addresses(struct virtio_net *dev, int vq_index)
 }
 
 /*
+ * The virtio device sends us the desc, used and avail ring addresses.
+ * This function then converts these to our address space.
+ */
+static int
+vhost_user_set_vring_addr(struct virtio_net **pdev, VhostUserMsg *msg)
+{
+	struct vhost_virtqueue *vq;
+	struct vhost_vring_addr *addr = &msg->payload.addr;
+	struct virtio_net *dev = *pdev;
+
+	if (dev->mem == NULL)
+		return -1;
+
+	/* addr->index refers to the queue index. The txq 1, rxq is 0. */
+	vq = dev->virtqueue[msg->payload.addr.index];
+
+	/*
+	 * Rings addresses should not be interpreted as long as the ring is not
+	 * started and enabled
+	 */
+	memcpy(&vq->ring_addrs, addr, sizeof(*addr));
+
+	vring_invalidate(dev, vq);
+
+	if (vq->enabled && (dev->features &
+				(1ULL << VHOST_USER_F_PROTOCOL_FEATURES))) {
+		dev = translate_ring_addresses(dev, msg->payload.state.index);
+		if (!dev)
+			return -1;
+
+		*pdev = dev;
+	}
+
+	return 0;
+}
+
+/*
  * The virtio device sends us the available ring last used index.
  */
 static int
@@ -1273,7 +1283,7 @@ vhost_user_msg_handler(int vid, int fd)
 		vhost_user_set_vring_num(dev, &msg);
 		break;
 	case VHOST_USER_SET_VRING_ADDR:
-		vhost_user_set_vring_addr(dev, &msg);
+		vhost_user_set_vring_addr(&dev, &msg);
 		break;
 	case VHOST_USER_SET_VRING_BASE:
 		vhost_user_set_vring_base(dev, &msg);
-- 
2.13.6

Re: [dpdk-dev] [PATCH v2] vhost: translate addrs in set_vring_addr if ring already enabled

[Yuanhan Liu]

On Mon, Oct 16, 2017 at 04:56:27PM +0200, Maxime Coquelin wrote:
> Commit 3ea7052f4b1b ("vhost: postpone rings addresses translation")
> moves rings addresses translation at either vring kick or enable
> time, depending on whether protocol features are enabled or not.
> This is done not interpret ring information as long as the vring
> is not fully initialized.
> 
> The problem is that with old QEMU versions, like v2.5, the ring
> is enabled before addresses are sent, so addresses are never
> translated.
> 
> This patch fixes the issue by doing the translation in
> VHOST_USER_SET_VRING_ADDR handling if ring is already enabled.
> 
> Fixes: 3ea7052f4b1b ("vhost: postpone rings addresses translation")
> 
> Reported-by: Lei Yao <lei.a.yao@intel.com>
> Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com>
> ---
> v2: Fixed commit informations (Fixes:, Reported-by:)

Applied to dpdk-next-virtio.

Thanks.

	--yliu