vhost: fix vring addr update with vDPA

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

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
  

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.