DPDK patches and discussions
 help / color / mirror / Atom feed
* [PATCH] vhost: fix vring addr update with vDPA
@ 2024-03-07 10:34 David Marchand
  2024-03-11 10:16 ` Maxime Coquelin
  2024-03-13 14:45 ` David Marchand
  0 siblings, 2 replies; 5+ messages in thread
From: David Marchand @ 2024-03-07 10:34 UTC (permalink / raw)
  To: dev; +Cc: ssimma, Maxime Coquelin, Chenbo Xia, Eelco Chaudron

For vDPA devices, vq are not locked once the device has been configured
at runtime.

On the other hand, we need to hold the vq lock to evaluate vq->access_ok,
invalidate vring addresses and translate them.

Move vring address update earlier and, when vDPA is configured, skip parts
which expect lock to be taken.

Bugzilla ID: 1394
Fixes: 741dc052eaf9 ("vhost: annotate virtqueue access checks")

Signed-off-by: David Marchand <david.marchand@redhat.com>
---
No change, just resending for CI.

---
 lib/vhost/vhost_user.c | 14 +++++++++-----
 1 file changed, 9 insertions(+), 5 deletions(-)

diff --git a/lib/vhost/vhost_user.c b/lib/vhost/vhost_user.c
index 3aba32c95a..7fe1687f08 100644
--- a/lib/vhost/vhost_user.c
+++ b/lib/vhost/vhost_user.c
@@ -986,17 +986,20 @@ vhost_user_set_vring_addr(struct virtio_net **pdev,
 	/* addr->index refers to the queue index. The txq 1, rxq is 0. */
 	vq = dev->virtqueue[ctx->msg.payload.addr.index];
 
-	/* vhost_user_lock_all_queue_pairs locked all qps */
-	VHOST_USER_ASSERT_LOCK(dev, vq, VHOST_USER_SET_VRING_ADDR);
-
-	access_ok = vq->access_ok;
-
 	/*
 	 * Rings addresses should not be interpreted as long as the ring is not
 	 * started and enabled
 	 */
 	memcpy(&vq->ring_addrs, addr, sizeof(*addr));
 
+	if (dev->flags & VIRTIO_DEV_VDPA_CONFIGURED)
+		goto out;
+
+	/* vhost_user_lock_all_queue_pairs locked all qps */
+	VHOST_USER_ASSERT_LOCK(dev, vq, VHOST_USER_SET_VRING_ADDR);
+
+	access_ok = vq->access_ok;
+
 	vring_invalidate(dev, vq);
 
 	if ((vq->enabled && (dev->features &
@@ -1006,6 +1009,7 @@ vhost_user_set_vring_addr(struct virtio_net **pdev,
 		*pdev = dev;
 	}
 
+out:
 	return RTE_VHOST_MSG_RESULT_OK;
 }
 
-- 
2.43.0


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] vhost: fix vring addr update with vDPA
  2024-03-07 10:34 [PATCH] vhost: fix vring addr update with vDPA David Marchand
@ 2024-03-11 10:16 ` Maxime Coquelin
  2024-03-13 14:45 ` David Marchand
  1 sibling, 0 replies; 5+ messages in thread
From: Maxime Coquelin @ 2024-03-11 10:16 UTC (permalink / raw)
  To: David Marchand, dev; +Cc: ssimma, Chenbo Xia, Eelco Chaudron



On 3/7/24 11:34, David Marchand wrote:
> For vDPA devices, vq are not locked once the device has been configured
> at runtime.
> 
> On the other hand, we need to hold the vq lock to evaluate vq->access_ok,
> invalidate vring addresses and translate them.
> 
> Move vring address update earlier and, when vDPA is configured, skip parts
> which expect lock to be taken.
> 
> Bugzilla ID: 1394
> Fixes: 741dc052eaf9 ("vhost: annotate virtqueue access checks")
> 
> Signed-off-by: David Marchand <david.marchand@redhat.com>
> ---
> No change, just resending for CI.
> 
> ---
>   lib/vhost/vhost_user.c | 14 +++++++++-----
>   1 file changed, 9 insertions(+), 5 deletions(-)
> 
> diff --git a/lib/vhost/vhost_user.c b/lib/vhost/vhost_user.c
> index 3aba32c95a..7fe1687f08 100644
> --- a/lib/vhost/vhost_user.c
> +++ b/lib/vhost/vhost_user.c
> @@ -986,17 +986,20 @@ vhost_user_set_vring_addr(struct virtio_net **pdev,
>   	/* addr->index refers to the queue index. The txq 1, rxq is 0. */
>   	vq = dev->virtqueue[ctx->msg.payload.addr.index];
>   
> -	/* vhost_user_lock_all_queue_pairs locked all qps */
> -	VHOST_USER_ASSERT_LOCK(dev, vq, VHOST_USER_SET_VRING_ADDR);
> -
> -	access_ok = vq->access_ok;
> -
>   	/*
>   	 * Rings addresses should not be interpreted as long as the ring is not
>   	 * started and enabled
>   	 */
>   	memcpy(&vq->ring_addrs, addr, sizeof(*addr));
>   
> +	if (dev->flags & VIRTIO_DEV_VDPA_CONFIGURED)
> +		goto out;
> +
> +	/* vhost_user_lock_all_queue_pairs locked all qps */
> +	VHOST_USER_ASSERT_LOCK(dev, vq, VHOST_USER_SET_VRING_ADDR);
> +
> +	access_ok = vq->access_ok;
> +
>   	vring_invalidate(dev, vq);
>   
>   	if ((vq->enabled && (dev->features &
> @@ -1006,6 +1009,7 @@ vhost_user_set_vring_addr(struct virtio_net **pdev,
>   		*pdev = dev;
>   	}
>   
> +out:
>   	return RTE_VHOST_MSG_RESULT_OK;
>   }
>   

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

Thanks,
Maxime


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] vhost: fix vring addr update with vDPA
  2024-03-07 10:34 [PATCH] vhost: fix vring addr update with vDPA David Marchand
  2024-03-11 10:16 ` Maxime Coquelin
@ 2024-03-13 14:45 ` David Marchand
  1 sibling, 0 replies; 5+ messages in thread
From: David Marchand @ 2024-03-13 14:45 UTC (permalink / raw)
  To: David Marchand; +Cc: dev, ssimma, Maxime Coquelin, Chenbo Xia, Eelco Chaudron

On Thu, Mar 7, 2024 at 11:35 AM David Marchand
<david.marchand@redhat.com> wrote:
>
> For vDPA devices, vq are not locked once the device has been configured
> at runtime.
>
> On the other hand, we need to hold the vq lock to evaluate vq->access_ok,
> invalidate vring addresses and translate them.
>
> Move vring address update earlier and, when vDPA is configured, skip parts
> which expect lock to be taken.
>
> Bugzilla ID: 1394
> Fixes: 741dc052eaf9 ("vhost: annotate virtqueue access checks")
>
> Signed-off-by: David Marchand <david.marchand@redhat.com>

Applied, thanks.


-- 
David Marchand


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] vhost: fix vring addr update with vDPA
  2024-03-05  9:13 David Marchand
@ 2024-03-05 13:28 ` David Marchand
  0 siblings, 0 replies; 5+ messages in thread
From: David Marchand @ 2024-03-05 13:28 UTC (permalink / raw)
  To: dev; +Cc: ssimma, Maxime Coquelin, Chenbo Xia, Eelco Chaudron

On Tue, Mar 5, 2024 at 10:13 AM David Marchand
<david.marchand@redhat.com> wrote:
>
> For vDPA devices, vq are not locked once the device has been configured
> at runtime.
>
> On the other hand, we need to hold the vq lock to evaluate vq->access_ok,
> invalidate vring addresses and translate them.
>
> Move vring address update earlier and, when vDPA is configured, skip parts
> which expect lock to be taken.
>
> Bugzilla ID: 1394
> Fixes: 741dc052eaf9 ("vhost: annotate virtqueue access checks")
>
> Signed-off-by: David Marchand <david.marchand@redhat.com>

Recheck-request: iol-testing


-- 
David Marchand


^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH] vhost: fix vring addr update with vDPA
@ 2024-03-05  9:13 David Marchand
  2024-03-05 13:28 ` David Marchand
  0 siblings, 1 reply; 5+ messages in thread
From: David Marchand @ 2024-03-05  9:13 UTC (permalink / raw)
  To: dev; +Cc: ssimma, Maxime Coquelin, Chenbo Xia, Eelco Chaudron

For vDPA devices, vq are not locked once the device has been configured
at runtime.

On the other hand, we need to hold the vq lock to evaluate vq->access_ok,
invalidate vring addresses and translate them.

Move vring address update earlier and, when vDPA is configured, skip parts
which expect lock to be taken.

Bugzilla ID: 1394
Fixes: 741dc052eaf9 ("vhost: annotate virtqueue access checks")

Signed-off-by: David Marchand <david.marchand@redhat.com>
---
 lib/vhost/vhost_user.c | 14 +++++++++-----
 1 file changed, 9 insertions(+), 5 deletions(-)

diff --git a/lib/vhost/vhost_user.c b/lib/vhost/vhost_user.c
index 3aba32c95a..7fe1687f08 100644
--- a/lib/vhost/vhost_user.c
+++ b/lib/vhost/vhost_user.c
@@ -986,17 +986,20 @@ vhost_user_set_vring_addr(struct virtio_net **pdev,
 	/* addr->index refers to the queue index. The txq 1, rxq is 0. */
 	vq = dev->virtqueue[ctx->msg.payload.addr.index];
 
-	/* vhost_user_lock_all_queue_pairs locked all qps */
-	VHOST_USER_ASSERT_LOCK(dev, vq, VHOST_USER_SET_VRING_ADDR);
-
-	access_ok = vq->access_ok;
-
 	/*
 	 * Rings addresses should not be interpreted as long as the ring is not
 	 * started and enabled
 	 */
 	memcpy(&vq->ring_addrs, addr, sizeof(*addr));
 
+	if (dev->flags & VIRTIO_DEV_VDPA_CONFIGURED)
+		goto out;
+
+	/* vhost_user_lock_all_queue_pairs locked all qps */
+	VHOST_USER_ASSERT_LOCK(dev, vq, VHOST_USER_SET_VRING_ADDR);
+
+	access_ok = vq->access_ok;
+
 	vring_invalidate(dev, vq);
 
 	if ((vq->enabled && (dev->features &
@@ -1006,6 +1009,7 @@ vhost_user_set_vring_addr(struct virtio_net **pdev,
 		*pdev = dev;
 	}
 
+out:
 	return RTE_VHOST_MSG_RESULT_OK;
 }
 
-- 
2.43.0


^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2024-03-13 14:46 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-03-07 10:34 [PATCH] vhost: fix vring addr update with vDPA David Marchand
2024-03-11 10:16 ` Maxime Coquelin
2024-03-13 14:45 ` David Marchand
  -- strict thread matches above, loose matches on Subject: below --
2024-03-05  9:13 David Marchand
2024-03-05 13:28 ` David Marchand

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).