[2/3] vhost: fix vring addr handling during live migration

Message ID 20190819113457.15569-3-tiwei.bie@intel.com (mailing list archive)
State Accepted, archived
Delegated to: Maxime Coquelin
Headers
Series Some fixes for vhost |

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/Intel-compilation fail Compilation issues

Commit Message

Tiwei Bie Aug. 19, 2019, 11:34 a.m. UTC
  When live migration starts, QEMU will set ring addrs again for
each virtqueue. In this case, we should try to translate ring
addrs after we invalidating the ring, otherwise virtqueues can
be enabled with the addrs untranslated. Besides, also leverage
the access_ok flag in non-IOMMU case to prevent the data path
accessing invalidated virtqueues.

Fixes: 5a4933e56be4 ("vhost: postpone ring address translations at kick time only")
Cc: stable@dpdk.org

Reported-by: Yilong Lv <lvyilong.lyl@alibaba-inc.com>
Signed-off-by: Tiwei Bie <tiwei.bie@intel.com>
---
 lib/librte_vhost/vhost.c      |  3 +--
 lib/librte_vhost/vhost_user.c | 12 ++++++++++--
 2 files changed, 11 insertions(+), 4 deletions(-)
  

Comments

Maxime Coquelin Sept. 27, 2019, 7:54 a.m. UTC | #1
On 8/19/19 1:34 PM, Tiwei Bie wrote:
> When live migration starts, QEMU will set ring addrs again for
> each virtqueue. In this case, we should try to translate ring
> addrs after we invalidating the ring, otherwise virtqueues can
> be enabled with the addrs untranslated. Besides, also leverage
> the access_ok flag in non-IOMMU case to prevent the data path
> accessing invalidated virtqueues.
> 
> Fixes: 5a4933e56be4 ("vhost: postpone ring address translations at kick time only")
> Cc: stable@dpdk.org
> 
> Reported-by: Yilong Lv <lvyilong.lyl@alibaba-inc.com>
> Signed-off-by: Tiwei Bie <tiwei.bie@intel.com>
> ---
>  lib/librte_vhost/vhost.c      |  3 +--
>  lib/librte_vhost/vhost_user.c | 12 ++++++++++--
>  2 files changed, 11 insertions(+), 4 deletions(-)
> 


Reviewed-by: Maxime Coquelin <maxime.coquelin@redhat.com>

Thanks,
Maxime
  

Patch

diff --git a/lib/librte_vhost/vhost.c b/lib/librte_vhost/vhost.c
index 981837b5d..77be16069 100644
--- a/lib/librte_vhost/vhost.c
+++ b/lib/librte_vhost/vhost.c
@@ -358,7 +358,7 @@  vring_translate(struct virtio_net *dev, struct vhost_virtqueue *vq)
 {
 
 	if (!(dev->features & (1ULL << VIRTIO_F_IOMMU_PLATFORM)))
-		goto out;
+		return -1;
 
 	if (vq_is_packed(dev)) {
 		if (vring_translate_packed(dev, vq) < 0)
@@ -367,7 +367,6 @@  vring_translate(struct virtio_net *dev, struct vhost_virtqueue *vq)
 		if (vring_translate_split(dev, vq) < 0)
 			return -1;
 	}
-out:
 	vq->access_ok = 1;
 
 	return 0;
diff --git a/lib/librte_vhost/vhost_user.c b/lib/librte_vhost/vhost_user.c
index ea43ab139..381250cbc 100644
--- a/lib/librte_vhost/vhost_user.c
+++ b/lib/librte_vhost/vhost_user.c
@@ -622,6 +622,7 @@  translate_ring_addresses(struct virtio_net *dev, int vq_index)
 			return dev;
 		}
 
+		vq->access_ok = 1;
 		return dev;
 	}
 
@@ -680,6 +681,7 @@  translate_ring_addresses(struct virtio_net *dev, int vq_index)
 	}
 
 	vq->log_guest_addr = addr->log_guest_addr;
+	vq->access_ok = 1;
 
 	VHOST_LOG_DEBUG(VHOST_CONFIG, "(%d) mapped address desc: %p\n",
 			dev->vid, vq->desc);
@@ -704,6 +706,7 @@  vhost_user_set_vring_addr(struct virtio_net **pdev, struct VhostUserMsg *msg,
 	struct virtio_net *dev = *pdev;
 	struct vhost_virtqueue *vq;
 	struct vhost_vring_addr *addr = &msg->payload.addr;
+	bool access_ok;
 
 	if (dev->mem == NULL)
 		return RTE_VHOST_MSG_RESULT_ERR;
@@ -711,6 +714,8 @@  vhost_user_set_vring_addr(struct virtio_net **pdev, struct VhostUserMsg *msg,
 	/* addr->index refers to the queue index. The txq 1, rxq is 0. */
 	vq = dev->virtqueue[msg->payload.addr.index];
 
+	access_ok = vq->access_ok;
+
 	/*
 	 * Rings addresses should not be interpreted as long as the ring is not
 	 * started and enabled
@@ -719,8 +724,9 @@  vhost_user_set_vring_addr(struct virtio_net **pdev, struct VhostUserMsg *msg,
 
 	vring_invalidate(dev, vq);
 
-	if (vq->enabled && (dev->features &
-				(1ULL << VHOST_USER_F_PROTOCOL_FEATURES))) {
+	if ((vq->enabled && (dev->features &
+				(1ULL << VHOST_USER_F_PROTOCOL_FEATURES))) ||
+			access_ok) {
 		dev = translate_ring_addresses(dev, msg->payload.addr.index);
 		if (!dev)
 			return RTE_VHOST_MSG_RESULT_ERR;
@@ -1325,6 +1331,8 @@  vhost_user_get_vring_base(struct virtio_net **pdev,
 	msg->size = sizeof(msg->payload.state);
 	msg->fd_num = 0;
 
+	vring_invalidate(dev, vq);
+
 	return RTE_VHOST_MSG_RESULT_REPLY;
 }