From patchwork Mon Mar 28 12:17:54 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: David Marchand X-Patchwork-Id: 108930 X-Patchwork-Delegate: maxime.coquelin@redhat.com Return-Path: X-Original-To: patchwork@inbox.dpdk.org Delivered-To: patchwork@inbox.dpdk.org Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124]) by inbox.dpdk.org (Postfix) with ESMTP id B338EA0501; Mon, 28 Mar 2022 14:18:25 +0200 (CEST) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id A657D42871; Mon, 28 Mar 2022 14:18:25 +0200 (CEST) Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.129.124]) by mails.dpdk.org (Postfix) with ESMTP id DA1DF4286B for ; Mon, 28 Mar 2022 14:18:24 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1648469904; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=lkJkvPaJYAV9qFfSvXhPBD+zekg3wpjSt6cj2KPpQDI=; b=OmJJy/yvmbOnwY7A0AxfZyTJnDKwwclMNd98Hh8yAqRF95sLI4cITEqLzCJLcPfBpDj1oX /5QMsOL1arBXZwklAAWN8mKw5JLavOOeNzvKPsMnD58+G0GCNv6GJppGm4ZomMnjWoCKGv sF0PqV8ji77hQXHadXJYtyozPo8YARQ= Received: from mimecast-mx02.redhat.com (mimecast-mx02.redhat.com [66.187.233.88]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id us-mta-193-yal6jOXjMwqTF8dqeVOR3g-1; Mon, 28 Mar 2022 08:18:21 -0400 X-MC-Unique: yal6jOXjMwqTF8dqeVOR3g-1 Received: from smtp.corp.redhat.com (int-mx08.intmail.prod.int.rdu2.redhat.com [10.11.54.8]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx02.redhat.com (Postfix) with ESMTPS id A7DD1805F0E; Mon, 28 Mar 2022 12:18:20 +0000 (UTC) Received: from dmarchan.remote.csb (unknown [10.40.195.27]) by smtp.corp.redhat.com (Postfix) with ESMTP id 9A74FC15D58; Mon, 28 Mar 2022 12:18:16 +0000 (UTC) From: David Marchand To: dev@dpdk.org Cc: maxime.coquelin@redhat.com, chenbo.xia@intel.com, stable@dpdk.org, Yuanhan Liu , Stefan Hajnoczi Subject: [RFC PATCH 1/5] vhost: fix missing virtqueue lock protection Date: Mon, 28 Mar 2022 14:17:54 +0200 Message-Id: <20220328121758.26632-2-david.marchand@redhat.com> In-Reply-To: <20220328121758.26632-1-david.marchand@redhat.com> References: <20220328121758.26632-1-david.marchand@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.85 on 10.11.54.8 Authentication-Results: relay.mimecast.com; auth=pass smtp.auth=CUSA124A263 smtp.mailfrom=david.marchand@redhat.com X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org From: Maxime Coquelin This patch ensures virtqueue metadata are not being modified while rte_vhost_vring_call() is executed. Fixes: 6c299bb7322f ("vhost: introduce vring call API") Cc: stable@dpdk.org Signed-off-by: Maxime Coquelin Reviewed-by: David Marchand --- This is the same as: https://patchwork.dpdk.org/project/dpdk/patch/20220324124638.32672-2-maxime.coquelin@redhat.com/ --- lib/vhost/vhost.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/lib/vhost/vhost.c b/lib/vhost/vhost.c index bc88148347..2f96a28dac 100644 --- a/lib/vhost/vhost.c +++ b/lib/vhost/vhost.c @@ -1291,11 +1291,15 @@ rte_vhost_vring_call(int vid, uint16_t vring_idx) if (!vq) return -1; + rte_spinlock_lock(&vq->access_lock); + if (vq_is_packed(dev)) vhost_vring_call_packed(dev, vq); else vhost_vring_call_split(dev, vq); + rte_spinlock_unlock(&vq->access_lock); + return 0; } From patchwork Mon Mar 28 12:17:55 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: David Marchand X-Patchwork-Id: 108931 X-Patchwork-Delegate: maxime.coquelin@redhat.com Return-Path: X-Original-To: patchwork@inbox.dpdk.org Delivered-To: patchwork@inbox.dpdk.org Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124]) by inbox.dpdk.org (Postfix) with ESMTP id B84E3A0501; Mon, 28 Mar 2022 14:18:31 +0200 (CEST) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id A9FEB42883; Mon, 28 Mar 2022 14:18:31 +0200 (CEST) Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by mails.dpdk.org (Postfix) with ESMTP id 077B74286B for ; Mon, 28 Mar 2022 14:18:30 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1648469910; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=Ao1AX8/uhDpNCScyjHYF3PjMGF/k/0J4FghpD1kXg2Q=; b=TzELGkI/Q7vH+sLuSnwhLjVcMarane5KOS4zoNuqKa+vDAyL18eISNhw+1jwXDcJJIzY28 3Gnz3FHrUxua/m2w43iU9D6BQx/fLKbXEnlGyJyYgiGAesksauqp59AicLA8qAZZyUEvhq xCR6+JaYWmt63UJPZEo5DpR6Do/vgz4= Received: from mimecast-mx02.redhat.com (mimecast-mx02.redhat.com [66.187.233.88]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id us-mta-482-oBT3UyUfOzWCHzzrD_evkA-1; Mon, 28 Mar 2022 08:18:27 -0400 X-MC-Unique: oBT3UyUfOzWCHzzrD_evkA-1 Received: from smtp.corp.redhat.com (int-mx08.intmail.prod.int.rdu2.redhat.com [10.11.54.8]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx02.redhat.com (Postfix) with ESMTPS id E63DE85A5A8; Mon, 28 Mar 2022 12:18:26 +0000 (UTC) Received: from dmarchan.remote.csb (unknown [10.40.195.27]) by smtp.corp.redhat.com (Postfix) with ESMTP id 17D00C15D57; Mon, 28 Mar 2022 12:18:25 +0000 (UTC) From: David Marchand To: dev@dpdk.org Cc: maxime.coquelin@redhat.com, chenbo.xia@intel.com Subject: [RFC PATCH 2/5] vhost: annotate virtqueue access lock Date: Mon, 28 Mar 2022 14:17:55 +0200 Message-Id: <20220328121758.26632-3-david.marchand@redhat.com> In-Reply-To: <20220328121758.26632-1-david.marchand@redhat.com> References: <20220328121758.26632-1-david.marchand@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.85 on 10.11.54.8 Authentication-Results: relay.mimecast.com; auth=pass smtp.auth=CUSA124A263 smtp.mailfrom=david.marchand@redhat.com X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Introduce a wrapper around spinlocks and annotate the access_lock lock protecting vq. One small change is required in the code: vhost_poll_enqueue_completed was getting a queue_id to get hold of the vq, while its callers already knew of the vq. vq is now directly passed. Signed-off-by: David Marchand --- lib/vhost/meson.build | 3 ++ lib/vhost/vhost.c | 30 +++++++++--------- lib/vhost/vhost.h | 70 ++++++++++++++++++++++++++++++++++++++++-- lib/vhost/vhost_user.c | 14 +++++---- lib/vhost/virtio_net.c | 33 ++++++++++++-------- 5 files changed, 115 insertions(+), 35 deletions(-) diff --git a/lib/vhost/meson.build b/lib/vhost/meson.build index bc7272053b..6e0bf9e7e8 100644 --- a/lib/vhost/meson.build +++ b/lib/vhost/meson.build @@ -17,6 +17,9 @@ elif (toolchain == 'icc' and cc.version().version_compare('>=16.0.0')) endif dpdk_conf.set('RTE_LIBRTE_VHOST_POSTCOPY', cc.has_header('linux/userfaultfd.h')) cflags += '-fno-strict-aliasing' +if cc.has_argument('-Wthread-safety') + cflags += '-Wthread-safety' +endif sources = files( 'fd_man.c', 'iotlb.c', diff --git a/lib/vhost/vhost.c b/lib/vhost/vhost.c index 2f96a28dac..863cd9131e 100644 --- a/lib/vhost/vhost.c +++ b/lib/vhost/vhost.c @@ -622,7 +622,7 @@ alloc_vring_queue(struct virtio_net *dev, uint32_t vring_idx) dev->virtqueue[i] = vq; init_vring_queue(dev, i); - rte_spinlock_init(&vq->access_lock); + vhost_spinlock_init(&vq->access_lock); vq->avail_wrap_counter = 1; vq->used_wrap_counter = 1; vq->signalled_used_valid = false; @@ -1291,14 +1291,14 @@ rte_vhost_vring_call(int vid, uint16_t vring_idx) if (!vq) return -1; - rte_spinlock_lock(&vq->access_lock); + vhost_spinlock_lock(&vq->access_lock); if (vq_is_packed(dev)) vhost_vring_call_packed(dev, vq); else vhost_vring_call_split(dev, vq); - rte_spinlock_unlock(&vq->access_lock); + vhost_spinlock_unlock(&vq->access_lock); return 0; } @@ -1321,7 +1321,7 @@ rte_vhost_avail_entries(int vid, uint16_t queue_id) if (!vq) return 0; - rte_spinlock_lock(&vq->access_lock); + vhost_spinlock_lock(&vq->access_lock); if (unlikely(!vq->enabled || vq->avail == NULL)) goto out; @@ -1329,7 +1329,7 @@ rte_vhost_avail_entries(int vid, uint16_t queue_id) ret = *(volatile uint16_t *)&vq->avail->idx - vq->last_used_idx; out: - rte_spinlock_unlock(&vq->access_lock); + vhost_spinlock_unlock(&vq->access_lock); return ret; } @@ -1413,12 +1413,12 @@ rte_vhost_enable_guest_notification(int vid, uint16_t queue_id, int enable) if (!vq) return -1; - rte_spinlock_lock(&vq->access_lock); + vhost_spinlock_lock(&vq->access_lock); vq->notif_enable = enable; ret = vhost_enable_guest_notification(dev, vq, enable); - rte_spinlock_unlock(&vq->access_lock); + vhost_spinlock_unlock(&vq->access_lock); return ret; } @@ -1475,7 +1475,7 @@ rte_vhost_rx_queue_count(int vid, uint16_t qid) if (vq == NULL) return 0; - rte_spinlock_lock(&vq->access_lock); + vhost_spinlock_lock(&vq->access_lock); if (unlikely(!vq->enabled || vq->avail == NULL)) goto out; @@ -1483,7 +1483,7 @@ rte_vhost_rx_queue_count(int vid, uint16_t qid) ret = *((volatile uint16_t *)&vq->avail->idx) - vq->last_avail_idx; out: - rte_spinlock_unlock(&vq->access_lock); + vhost_spinlock_unlock(&vq->access_lock); return ret; } @@ -1708,9 +1708,9 @@ rte_vhost_async_channel_register(int vid, uint16_t queue_id) if (unlikely(vq == NULL || !dev->async_copy)) return -1; - rte_spinlock_lock(&vq->access_lock); + vhost_spinlock_lock(&vq->access_lock); ret = async_channel_register(vid, queue_id); - rte_spinlock_unlock(&vq->access_lock); + vhost_spinlock_unlock(&vq->access_lock); return ret; } @@ -1758,7 +1758,7 @@ rte_vhost_async_channel_unregister(int vid, uint16_t queue_id) if (!vq->async) return ret; - if (!rte_spinlock_trylock(&vq->access_lock)) { + if (vhost_spinlock_trylock(&vq->access_lock) == 0) { VHOST_LOG_CONFIG(ERR, "(%s) failed to unregister async channel, virtqueue busy.\n", dev->ifname); return -1; @@ -1774,7 +1774,7 @@ rte_vhost_async_channel_unregister(int vid, uint16_t queue_id) vhost_free_async_mem(vq); out: - rte_spinlock_unlock(&vq->access_lock); + vhost_spinlock_unlock(&vq->access_lock); return ret; } @@ -1894,7 +1894,7 @@ rte_vhost_async_get_inflight(int vid, uint16_t queue_id) if (!vq->async) return ret; - if (!rte_spinlock_trylock(&vq->access_lock)) { + if (vhost_spinlock_trylock(&vq->access_lock) == 0) { VHOST_LOG_CONFIG(DEBUG, "(%s) failed to check in-flight packets. virtqueue busy.\n", dev->ifname); @@ -1902,7 +1902,7 @@ rte_vhost_async_get_inflight(int vid, uint16_t queue_id) } ret = vq->async->pkts_inflight_n; - rte_spinlock_unlock(&vq->access_lock); + vhost_spinlock_unlock(&vq->access_lock); return ret; } diff --git a/lib/vhost/vhost.h b/lib/vhost/vhost.h index a9edc271aa..619f073fb2 100644 --- a/lib/vhost/vhost.h +++ b/lib/vhost/vhost.h @@ -85,6 +85,71 @@ for (iter = val; iter < num; iter++) #endif +#ifndef __has_feature +#define __vhost_has_feature(x) 0 +#else +#define __vhost_has_feature __has_feature +#endif + +#if __vhost_has_feature(c_thread_safety_attributes) +#define VHOST_NO_THREAD_SAFETY_ANALYSIS \ + __attribute__((no_thread_safety_analysis)) +#define VHOST_LOCKABLE \ + __attribute__((lockable)) + +#define VHOST_SPINLOCK_REQUIRES(...) \ + __attribute__((exclusive_locks_required(__VA_ARGS__))) +#define VHOST_SPINLOCK_ACQUIRES(...) \ + __attribute__((exclusive_lock_function(__VA_ARGS__))) +#define VHOST_SPINLOCK_TRYLOCK(ret, ...) \ + __attribute__((exclusive_trylock_function(ret, __VA_ARGS__))) +#define VHOST_SPINLOCK_RELEASES(...) \ + __attribute__((unlock_function(__VA_ARGS__))) + +#else +#define VHOST_NO_THREAD_SAFETY_ANALYSIS +#define VHOST_LOCKABLE + +#define VHOST_SPINLOCK_REQUIRES(...) +#define VHOST_SPINLOCK_ACQUIRES(...) +#define VHOST_SPINLOCK_TRYLOCK(...) +#define VHOST_SPINLOCK_RELEASES(...) +#endif + +typedef struct VHOST_LOCKABLE { + rte_spinlock_t l; +} vhost_spinlock_t; + +static __rte_always_inline void +vhost_spinlock_init(vhost_spinlock_t *l) +{ + rte_spinlock_init(&l->l); +} + +static __rte_always_inline void +vhost_spinlock_lock(vhost_spinlock_t *l) + VHOST_SPINLOCK_ACQUIRES(l) + VHOST_NO_THREAD_SAFETY_ANALYSIS +{ + rte_spinlock_lock(&l->l); +} + +static __rte_always_inline int +vhost_spinlock_trylock(vhost_spinlock_t *l) + VHOST_SPINLOCK_TRYLOCK(1, l) + VHOST_NO_THREAD_SAFETY_ANALYSIS +{ + return rte_spinlock_trylock(&l->l); +} + +static __rte_always_inline void +vhost_spinlock_unlock(vhost_spinlock_t *l) + VHOST_SPINLOCK_RELEASES(l) + VHOST_NO_THREAD_SAFETY_ANALYSIS +{ + rte_spinlock_unlock(&l->l); +} + /** * Structure contains buffer address, length and descriptor index * from vring to do scatter RX. @@ -255,8 +320,7 @@ struct vhost_virtqueue { bool access_ok; bool ready; - rte_spinlock_t access_lock; - + vhost_spinlock_t access_lock; union { struct vring_used_elem *shadow_used_split; @@ -834,6 +898,7 @@ vhost_need_event(uint16_t event_idx, uint16_t new_idx, uint16_t old) static __rte_always_inline void vhost_vring_call_split(struct virtio_net *dev, struct vhost_virtqueue *vq) + VHOST_SPINLOCK_REQUIRES(vq->access_lock) { /* Flush used->idx update before we read avail->flags. */ rte_atomic_thread_fence(__ATOMIC_SEQ_CST); @@ -872,6 +937,7 @@ vhost_vring_call_split(struct virtio_net *dev, struct vhost_virtqueue *vq) static __rte_always_inline void vhost_vring_call_packed(struct virtio_net *dev, struct vhost_virtqueue *vq) + VHOST_SPINLOCK_REQUIRES(vq->access_lock) { uint16_t old, new, off, off_wrap; bool signalled_used_valid, kick = false; diff --git a/lib/vhost/vhost_user.c b/lib/vhost/vhost_user.c index 1d390677fa..caf4cf14b5 100644 --- a/lib/vhost/vhost_user.c +++ b/lib/vhost/vhost_user.c @@ -2571,9 +2571,9 @@ vhost_user_iotlb_msg(struct virtio_net **pdev, len, imsg->perm); if (is_vring_iotlb(dev, vq, imsg)) { - rte_spinlock_lock(&vq->access_lock); + vhost_spinlock_lock(&vq->access_lock); *pdev = dev = translate_ring_addresses(dev, i); - rte_spinlock_unlock(&vq->access_lock); + vhost_spinlock_unlock(&vq->access_lock); } } break; @@ -2588,9 +2588,9 @@ vhost_user_iotlb_msg(struct virtio_net **pdev, imsg->size); if (is_vring_iotlb(dev, vq, imsg)) { - rte_spinlock_lock(&vq->access_lock); + vhost_spinlock_lock(&vq->access_lock); vring_invalidate(dev, vq); - rte_spinlock_unlock(&vq->access_lock); + vhost_spinlock_unlock(&vq->access_lock); } } break; @@ -2909,6 +2909,7 @@ vhost_user_check_and_alloc_queue_pair(struct virtio_net *dev, static void vhost_user_lock_all_queue_pairs(struct virtio_net *dev) + VHOST_NO_THREAD_SAFETY_ANALYSIS { unsigned int i = 0; unsigned int vq_num = 0; @@ -2917,7 +2918,7 @@ vhost_user_lock_all_queue_pairs(struct virtio_net *dev) struct vhost_virtqueue *vq = dev->virtqueue[i]; if (vq) { - rte_spinlock_lock(&vq->access_lock); + vhost_spinlock_lock(&vq->access_lock); vq_num++; } i++; @@ -2926,6 +2927,7 @@ vhost_user_lock_all_queue_pairs(struct virtio_net *dev) static void vhost_user_unlock_all_queue_pairs(struct virtio_net *dev) + VHOST_NO_THREAD_SAFETY_ANALYSIS { unsigned int i = 0; unsigned int vq_num = 0; @@ -2934,7 +2936,7 @@ vhost_user_unlock_all_queue_pairs(struct virtio_net *dev) struct vhost_virtqueue *vq = dev->virtqueue[i]; if (vq) { - rte_spinlock_unlock(&vq->access_lock); + vhost_spinlock_unlock(&vq->access_lock); vq_num++; } i++; diff --git a/lib/vhost/virtio_net.c b/lib/vhost/virtio_net.c index 5f432b0d77..25bdb1479d 100644 --- a/lib/vhost/virtio_net.c +++ b/lib/vhost/virtio_net.c @@ -1246,6 +1246,7 @@ vhost_enqueue_single_packed(struct virtio_net *dev, static __rte_noinline uint32_t virtio_dev_rx_split(struct virtio_net *dev, struct vhost_virtqueue *vq, struct rte_mbuf **pkts, uint32_t count) + VHOST_SPINLOCK_REQUIRES(vq->access_lock) { uint32_t pkt_idx = 0; uint16_t num_buffers; @@ -1441,6 +1442,7 @@ virtio_dev_rx_packed(struct virtio_net *dev, struct vhost_virtqueue *__rte_restrict vq, struct rte_mbuf **__rte_restrict pkts, uint32_t count) + VHOST_SPINLOCK_REQUIRES(vq->access_lock) { uint32_t pkt_idx = 0; @@ -1488,7 +1490,7 @@ virtio_dev_rx(struct virtio_net *dev, uint16_t queue_id, vq = dev->virtqueue[queue_id]; - rte_spinlock_lock(&vq->access_lock); + vhost_spinlock_lock(&vq->access_lock); if (unlikely(!vq->enabled)) goto out_access_unlock; @@ -1514,7 +1516,7 @@ virtio_dev_rx(struct virtio_net *dev, uint16_t queue_id, vhost_user_iotlb_rd_unlock(vq); out_access_unlock: - rte_spinlock_unlock(&vq->access_lock); + vhost_spinlock_unlock(&vq->access_lock); return nb_tx; } @@ -1955,11 +1957,11 @@ write_back_completed_descs_packed(struct vhost_virtqueue *vq, } static __rte_always_inline uint16_t -vhost_poll_enqueue_completed(struct virtio_net *dev, uint16_t queue_id, +vhost_poll_enqueue_completed(struct virtio_net *dev, struct vhost_virtqueue *vq, struct rte_mbuf **pkts, uint16_t count, int16_t dma_id, uint16_t vchan_id) + VHOST_SPINLOCK_REQUIRES(vq->access_lock) { - struct vhost_virtqueue *vq = dev->virtqueue[queue_id]; struct vhost_async *async = vq->async; struct async_inflight_info *pkts_info = async->pkts_info; uint16_t nr_cpl_pkts = 0; @@ -2050,7 +2052,7 @@ rte_vhost_poll_enqueue_completed(int vid, uint16_t queue_id, vq = dev->virtqueue[queue_id]; - if (!rte_spinlock_trylock(&vq->access_lock)) { + if (vhost_spinlock_trylock(&vq->access_lock) == 0) { VHOST_LOG_DATA(DEBUG, "(%s) %s: virtqueue %u is busy.\n", dev->ifname, __func__, queue_id); return 0; @@ -2062,10 +2064,10 @@ rte_vhost_poll_enqueue_completed(int vid, uint16_t queue_id, goto out; } - n_pkts_cpl = vhost_poll_enqueue_completed(dev, queue_id, pkts, count, dma_id, vchan_id); + n_pkts_cpl = vhost_poll_enqueue_completed(dev, vq, pkts, count, dma_id, vchan_id); out: - rte_spinlock_unlock(&vq->access_lock); + vhost_spinlock_unlock(&vq->access_lock); return n_pkts_cpl; } @@ -2074,6 +2076,7 @@ uint16_t rte_vhost_clear_queue_thread_unsafe(int vid, uint16_t queue_id, struct rte_mbuf **pkts, uint16_t count, int16_t dma_id, uint16_t vchan_id) + VHOST_NO_THREAD_SAFETY_ANALYSIS { struct virtio_net *dev = get_device(vid); struct vhost_virtqueue *vq; @@ -2104,7 +2107,7 @@ rte_vhost_clear_queue_thread_unsafe(int vid, uint16_t queue_id, return 0; } - n_pkts_cpl = vhost_poll_enqueue_completed(dev, queue_id, pkts, count, dma_id, vchan_id); + n_pkts_cpl = vhost_poll_enqueue_completed(dev, vq, pkts, count, dma_id, vchan_id); return n_pkts_cpl; } @@ -2132,7 +2135,7 @@ virtio_dev_rx_async_submit(struct virtio_net *dev, uint16_t queue_id, vq = dev->virtqueue[queue_id]; - rte_spinlock_lock(&vq->access_lock); + vhost_spinlock_lock(&vq->access_lock); if (unlikely(!vq->enabled || !vq->async)) goto out_access_unlock; @@ -2160,7 +2163,7 @@ virtio_dev_rx_async_submit(struct virtio_net *dev, uint16_t queue_id, vhost_user_iotlb_rd_unlock(vq); out_access_unlock: - rte_spinlock_unlock(&vq->access_lock); + vhost_spinlock_unlock(&vq->access_lock); return nb_tx; } @@ -2679,6 +2682,7 @@ static uint16_t virtio_dev_tx_split(struct virtio_net *dev, struct vhost_virtqueue *vq, struct rte_mempool *mbuf_pool, struct rte_mbuf **pkts, uint16_t count, bool legacy_ol_flags) + VHOST_SPINLOCK_REQUIRES(vq->access_lock) { uint16_t i; uint16_t free_entries; @@ -2774,6 +2778,7 @@ static uint16_t virtio_dev_tx_split_legacy(struct virtio_net *dev, struct vhost_virtqueue *vq, struct rte_mempool *mbuf_pool, struct rte_mbuf **pkts, uint16_t count) + VHOST_SPINLOCK_REQUIRES(vq->access_lock) { return virtio_dev_tx_split(dev, vq, mbuf_pool, pkts, count, true); } @@ -2783,6 +2788,7 @@ static uint16_t virtio_dev_tx_split_compliant(struct virtio_net *dev, struct vhost_virtqueue *vq, struct rte_mempool *mbuf_pool, struct rte_mbuf **pkts, uint16_t count) + VHOST_SPINLOCK_REQUIRES(vq->access_lock) { return virtio_dev_tx_split(dev, vq, mbuf_pool, pkts, count, false); } @@ -2982,6 +2988,7 @@ virtio_dev_tx_packed(struct virtio_net *dev, struct rte_mbuf **__rte_restrict pkts, uint32_t count, bool legacy_ol_flags) + VHOST_SPINLOCK_REQUIRES(vq->access_lock) { uint32_t pkt_idx = 0; @@ -3025,6 +3032,7 @@ static uint16_t virtio_dev_tx_packed_legacy(struct virtio_net *dev, struct vhost_virtqueue *__rte_restrict vq, struct rte_mempool *mbuf_pool, struct rte_mbuf **__rte_restrict pkts, uint32_t count) + VHOST_SPINLOCK_REQUIRES(vq->access_lock) { return virtio_dev_tx_packed(dev, vq, mbuf_pool, pkts, count, true); } @@ -3034,6 +3042,7 @@ static uint16_t virtio_dev_tx_packed_compliant(struct virtio_net *dev, struct vhost_virtqueue *__rte_restrict vq, struct rte_mempool *mbuf_pool, struct rte_mbuf **__rte_restrict pkts, uint32_t count) + VHOST_SPINLOCK_REQUIRES(vq->access_lock) { return virtio_dev_tx_packed(dev, vq, mbuf_pool, pkts, count, false); } @@ -3065,7 +3074,7 @@ rte_vhost_dequeue_burst(int vid, uint16_t queue_id, vq = dev->virtqueue[queue_id]; - if (unlikely(rte_spinlock_trylock(&vq->access_lock) == 0)) + if (unlikely(vhost_spinlock_trylock(&vq->access_lock) == 0)) return 0; if (unlikely(!vq->enabled)) { @@ -3134,7 +3143,7 @@ rte_vhost_dequeue_burst(int vid, uint16_t queue_id, vhost_user_iotlb_rd_unlock(vq); out_access_unlock: - rte_spinlock_unlock(&vq->access_lock); + vhost_spinlock_unlock(&vq->access_lock); if (unlikely(rarp_mbuf != NULL)) count += 1; From patchwork Mon Mar 28 12:17:56 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: David Marchand X-Patchwork-Id: 108932 X-Patchwork-Delegate: maxime.coquelin@redhat.com Return-Path: X-Original-To: patchwork@inbox.dpdk.org Delivered-To: patchwork@inbox.dpdk.org Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124]) by inbox.dpdk.org (Postfix) with ESMTP id 10B56A0501; Mon, 28 Mar 2022 14:18:52 +0200 (CEST) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 012B242880; Mon, 28 Mar 2022 14:18:52 +0200 (CEST) Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by mails.dpdk.org (Postfix) with ESMTP id 760AC41104 for ; Mon, 28 Mar 2022 14:18:50 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1648469930; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=fIkl5t5r6G/U+xVmXJurnpkXlWG5lPaLX2htjlnLEFE=; b=MF8Z8v/CPwNkj1dB+rQbr1FSCJBVAWSiSD9DSuSZrlkcW1dTbV2XkNzrctZIBIYAgjMiyG 6KDAjlo42fYtI+q/8TmpaM2eeDKBVEKz+MsatZoU4P+Lxub4say04pdrRFGlE9xSNcpF5D h0Nq6N+H3lSMeqHyRUKS7F/jHzCPM0c= Received: from mimecast-mx02.redhat.com (mimecast-mx02.redhat.com [66.187.233.88]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id us-mta-272-gsDjG94sP6STym-V7HFinA-1; Mon, 28 Mar 2022 08:18:46 -0400 X-MC-Unique: gsDjG94sP6STym-V7HFinA-1 Received: from smtp.corp.redhat.com (int-mx08.intmail.prod.int.rdu2.redhat.com [10.11.54.8]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx02.redhat.com (Postfix) with ESMTPS id 2D0B0101AA46; Mon, 28 Mar 2022 12:18:46 +0000 (UTC) Received: from dmarchan.remote.csb (unknown [10.40.195.27]) by smtp.corp.redhat.com (Postfix) with ESMTP id D5848C15D40; Mon, 28 Mar 2022 12:18:44 +0000 (UTC) From: David Marchand To: dev@dpdk.org Cc: maxime.coquelin@redhat.com, chenbo.xia@intel.com, stable@dpdk.org, Patrick Fu , Jiayu Hu Subject: [RFC PATCH 3/5] vhost: fix async access Date: Mon, 28 Mar 2022 14:17:56 +0200 Message-Id: <20220328121758.26632-4-david.marchand@redhat.com> In-Reply-To: <20220328121758.26632-1-david.marchand@redhat.com> References: <20220328121758.26632-1-david.marchand@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.85 on 10.11.54.8 Authentication-Results: relay.mimecast.com; auth=pass smtp.auth=CUSA124A263 smtp.mailfrom=david.marchand@redhat.com X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org vq->async is protected with vq->access_lock. Fixes: eb666d24085f ("vhost: fix async unregister deadlock") Fixes: 0c0935c5f794 ("vhost: allow to check in-flight packets for async vhost") Cc: stable@dpdk.org Signed-off-by: David Marchand --- This has been reported by Jiayu too: https://patchwork.dpdk.org/project/dpdk/patch/20220328020754.1155063-1-jiayu.hu@intel.com/ --- lib/vhost/vhost.c | 25 ++++++++++--------------- 1 file changed, 10 insertions(+), 15 deletions(-) diff --git a/lib/vhost/vhost.c b/lib/vhost/vhost.c index 863cd9131e..9e6ca506ff 100644 --- a/lib/vhost/vhost.c +++ b/lib/vhost/vhost.c @@ -1753,27 +1753,23 @@ rte_vhost_async_channel_unregister(int vid, uint16_t queue_id) if (vq == NULL) return ret; - ret = 0; - - if (!vq->async) - return ret; - if (vhost_spinlock_trylock(&vq->access_lock) == 0) { VHOST_LOG_CONFIG(ERR, "(%s) failed to unregister async channel, virtqueue busy.\n", dev->ifname); - return -1; + return ret; } - if (vq->async->pkts_inflight_n) { + if (!vq->async) { + ret = 0; + } else if (vq->async->pkts_inflight_n) { VHOST_LOG_CONFIG(ERR, "(%s) failed to unregister async channel.\n", dev->ifname); VHOST_LOG_CONFIG(ERR, "(%s) inflight packets must be completed before unregistration.\n", dev->ifname); - ret = -1; - goto out; + } else { + vhost_free_async_mem(vq); + ret = 0; } - vhost_free_async_mem(vq); -out: vhost_spinlock_unlock(&vq->access_lock); return ret; @@ -1891,9 +1887,6 @@ rte_vhost_async_get_inflight(int vid, uint16_t queue_id) if (vq == NULL) return ret; - if (!vq->async) - return ret; - if (vhost_spinlock_trylock(&vq->access_lock) == 0) { VHOST_LOG_CONFIG(DEBUG, "(%s) failed to check in-flight packets. virtqueue busy.\n", @@ -1901,7 +1894,9 @@ rte_vhost_async_get_inflight(int vid, uint16_t queue_id) return ret; } - ret = vq->async->pkts_inflight_n; + if (vq->async) + ret = vq->async->pkts_inflight_n; + vhost_spinlock_unlock(&vq->access_lock); return ret; From patchwork Mon Mar 28 12:17:57 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: David Marchand X-Patchwork-Id: 108933 X-Patchwork-Delegate: maxime.coquelin@redhat.com Return-Path: X-Original-To: patchwork@inbox.dpdk.org Delivered-To: patchwork@inbox.dpdk.org Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124]) by inbox.dpdk.org (Postfix) with ESMTP id 9054FA0501; Mon, 28 Mar 2022 14:18:56 +0200 (CEST) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id C0A6642889; Mon, 28 Mar 2022 14:18:54 +0200 (CEST) Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by mails.dpdk.org (Postfix) with ESMTP id 62E894288D for ; Mon, 28 Mar 2022 14:18:53 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1648469932; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=/GJD5m8DFNuCwYxoSMk0aT0MiueqxaNs3vkc7BbLlh4=; b=Rq8kJtnBFQ4Lrao7h5ZHbbwLs6a41UvPBcgGm8GHCo8HL43hjtJKHfTBayGKkky0rklihG vSttMwo7uj+FN2Ex/2N6copdHyRoYadt0uuFACTTCxJTOi5EGjo857TTox7pvwN0qF2huM hj5uAm/hT6zP814HZbbWES0ReIpYhBI= Received: from mimecast-mx02.redhat.com (mimecast-mx02.redhat.com [66.187.233.88]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id us-mta-12-_ZS0dJeTOyCxZaPJWbOosA-1; Mon, 28 Mar 2022 08:18:50 -0400 X-MC-Unique: _ZS0dJeTOyCxZaPJWbOosA-1 Received: from smtp.corp.redhat.com (int-mx08.intmail.prod.int.rdu2.redhat.com [10.11.54.8]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx02.redhat.com (Postfix) with ESMTPS id 8B32D802819; Mon, 28 Mar 2022 12:18:50 +0000 (UTC) Received: from dmarchan.remote.csb (unknown [10.40.195.27]) by smtp.corp.redhat.com (Postfix) with ESMTP id AF632C15D40; Mon, 28 Mar 2022 12:18:49 +0000 (UTC) From: David Marchand To: dev@dpdk.org Cc: maxime.coquelin@redhat.com, chenbo.xia@intel.com Subject: [RFC PATCH 4/5] vhost: annotate async locking requirement Date: Mon, 28 Mar 2022 14:17:57 +0200 Message-Id: <20220328121758.26632-5-david.marchand@redhat.com> In-Reply-To: <20220328121758.26632-1-david.marchand@redhat.com> References: <20220328121758.26632-1-david.marchand@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.85 on 10.11.54.8 Authentication-Results: relay.mimecast.com; auth=pass smtp.auth=CUSA124A263 smtp.mailfrom=david.marchand@redhat.com X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Instrument that the async field of a virtqueue must be protected by a lock. Signed-off-by: David Marchand --- lib/vhost/vhost.c | 14 +++++++++----- lib/vhost/vhost.h | 5 ++++- lib/vhost/vhost_user.c | 2 ++ lib/vhost/virtio_net.c | 14 ++++++++++++++ 4 files changed, 29 insertions(+), 6 deletions(-) diff --git a/lib/vhost/vhost.c b/lib/vhost/vhost.c index 9e6ca506ff..ea28323367 100644 --- a/lib/vhost/vhost.c +++ b/lib/vhost/vhost.c @@ -334,6 +334,7 @@ cleanup_device(struct virtio_net *dev, int destroy) static void vhost_free_async_mem(struct vhost_virtqueue *vq) + VHOST_SPINLOCK_REQUIRES(vq->access_lock) { if (!vq->async) return; @@ -352,6 +353,7 @@ vhost_free_async_mem(struct vhost_virtqueue *vq) void free_vq(struct virtio_net *dev, struct vhost_virtqueue *vq) + VHOST_NO_THREAD_SAFETY_ANALYSIS { if (vq_is_packed(dev)) rte_free(vq->shadow_used_packed); @@ -1622,10 +1624,10 @@ rte_vhost_extern_callback_register(int vid, } static __rte_always_inline int -async_channel_register(int vid, uint16_t queue_id) +async_channel_register(struct virtio_net *dev, struct vhost_virtqueue *vq, + uint16_t queue_id) + VHOST_SPINLOCK_REQUIRES(vq->access_lock) { - struct virtio_net *dev = get_device(vid); - struct vhost_virtqueue *vq = dev->virtqueue[queue_id]; struct vhost_async *async; int node = vq->numa_node; @@ -1709,7 +1711,7 @@ rte_vhost_async_channel_register(int vid, uint16_t queue_id) return -1; vhost_spinlock_lock(&vq->access_lock); - ret = async_channel_register(vid, queue_id); + ret = async_channel_register(dev, vq, queue_id); vhost_spinlock_unlock(&vq->access_lock); return ret; @@ -1717,6 +1719,7 @@ rte_vhost_async_channel_register(int vid, uint16_t queue_id) int rte_vhost_async_channel_register_thread_unsafe(int vid, uint16_t queue_id) + VHOST_NO_THREAD_SAFETY_ANALYSIS { struct vhost_virtqueue *vq; struct virtio_net *dev = get_device(vid); @@ -1732,7 +1735,7 @@ rte_vhost_async_channel_register_thread_unsafe(int vid, uint16_t queue_id) if (unlikely(vq == NULL || !dev->async_copy)) return -1; - return async_channel_register(vid, queue_id); + return async_channel_register(dev, vq, queue_id); } int @@ -1777,6 +1780,7 @@ rte_vhost_async_channel_unregister(int vid, uint16_t queue_id) int rte_vhost_async_channel_unregister_thread_unsafe(int vid, uint16_t queue_id) + VHOST_NO_THREAD_SAFETY_ANALYSIS { struct vhost_virtqueue *vq; struct virtio_net *dev = get_device(vid); diff --git a/lib/vhost/vhost.h b/lib/vhost/vhost.h index 619f073fb2..4b301ec152 100644 --- a/lib/vhost/vhost.h +++ b/lib/vhost/vhost.h @@ -96,6 +96,8 @@ __attribute__((no_thread_safety_analysis)) #define VHOST_LOCKABLE \ __attribute__((lockable)) +#define VHOST_GUARDED \ + __attribute__((guarded_var)) #define VHOST_SPINLOCK_REQUIRES(...) \ __attribute__((exclusive_locks_required(__VA_ARGS__))) @@ -109,6 +111,7 @@ #else #define VHOST_NO_THREAD_SAFETY_ANALYSIS #define VHOST_LOCKABLE +#define VHOST_GUARDED #define VHOST_SPINLOCK_REQUIRES(...) #define VHOST_SPINLOCK_ACQUIRES(...) @@ -363,7 +366,7 @@ struct vhost_virtqueue { struct rte_vhost_resubmit_info *resubmit_inflight; uint64_t global_counter; - struct vhost_async *async; + struct vhost_async *async VHOST_GUARDED; int notif_enable; #define VIRTIO_UNINITIALIZED_NOTIF (-1) diff --git a/lib/vhost/vhost_user.c b/lib/vhost/vhost_user.c index caf4cf14b5..35f1e23995 100644 --- a/lib/vhost/vhost_user.c +++ b/lib/vhost/vhost_user.c @@ -2199,6 +2199,8 @@ static int vhost_user_set_vring_enable(struct virtio_net **pdev, struct vhu_msg_context *ctx, int main_fd __rte_unused) + /* vq->access_lock is taken in vhost_user_lock_all_queue_pairs() */ + VHOST_NO_THREAD_SAFETY_ANALYSIS { struct virtio_net *dev = *pdev; bool enable = !!ctx->msg.payload.state.num; diff --git a/lib/vhost/virtio_net.c b/lib/vhost/virtio_net.c index 25bdb1479d..9bdba992dd 100644 --- a/lib/vhost/virtio_net.c +++ b/lib/vhost/virtio_net.c @@ -51,6 +51,7 @@ static __rte_always_inline int64_t vhost_async_dma_transfer_one(struct virtio_net *dev, struct vhost_virtqueue *vq, int16_t dma_id, uint16_t vchan_id, uint16_t flag_idx, struct vhost_iov_iter *pkt) + VHOST_SPINLOCK_REQUIRES(vq->access_lock) { struct async_dma_vchan_info *dma_info = &dma_copy_track[dma_id].vchans[vchan_id]; uint16_t ring_mask = dma_info->ring_mask; @@ -99,6 +100,7 @@ static __rte_always_inline uint16_t vhost_async_dma_transfer(struct virtio_net *dev, struct vhost_virtqueue *vq, int16_t dma_id, uint16_t vchan_id, uint16_t head_idx, struct vhost_iov_iter *pkts, uint16_t nr_pkts) + VHOST_SPINLOCK_REQUIRES(vq->access_lock) { struct async_dma_vchan_info *dma_info = &dma_copy_track[dma_id].vchans[vchan_id]; int64_t ret, nr_copies = 0; @@ -1000,6 +1002,7 @@ static __rte_always_inline int async_mbuf_to_desc_seg(struct virtio_net *dev, struct vhost_virtqueue *vq, struct rte_mbuf *m, uint32_t mbuf_offset, uint64_t buf_iova, uint32_t cpy_len) + VHOST_SPINLOCK_REQUIRES(vq->access_lock) { struct vhost_async *async = vq->async; uint64_t mapped_len; @@ -1057,6 +1060,7 @@ static __rte_always_inline int mbuf_to_desc(struct virtio_net *dev, struct vhost_virtqueue *vq, struct rte_mbuf *m, struct buf_vector *buf_vec, uint16_t nr_vec, uint16_t num_buffers, bool is_async) + VHOST_SPINLOCK_REQUIRES(vq->access_lock) { uint32_t vec_idx = 0; uint32_t mbuf_offset, mbuf_avail; @@ -1186,6 +1190,7 @@ vhost_enqueue_single_packed(struct virtio_net *dev, struct rte_mbuf *pkt, struct buf_vector *buf_vec, uint16_t *nr_descs) + VHOST_SPINLOCK_REQUIRES(vq->access_lock) { uint16_t nr_vec = 0; uint16_t avail_idx = vq->last_avail_idx; @@ -1417,6 +1422,7 @@ static __rte_always_inline int16_t virtio_dev_rx_single_packed(struct virtio_net *dev, struct vhost_virtqueue *vq, struct rte_mbuf *pkt) + VHOST_SPINLOCK_REQUIRES(vq->access_lock) { struct buf_vector buf_vec[BUF_VECTOR_MAX]; uint16_t nr_descs = 0; @@ -1541,6 +1547,7 @@ rte_vhost_enqueue_burst(int vid, uint16_t queue_id, static __rte_always_inline uint16_t async_get_first_inflight_pkt_idx(struct vhost_virtqueue *vq) + VHOST_SPINLOCK_REQUIRES(vq->access_lock) { struct vhost_async *async = vq->async; @@ -1587,6 +1594,7 @@ static __rte_noinline uint32_t virtio_dev_rx_async_submit_split(struct virtio_net *dev, struct vhost_virtqueue *vq, uint16_t queue_id, struct rte_mbuf **pkts, uint32_t count, int16_t dma_id, uint16_t vchan_id) + VHOST_SPINLOCK_REQUIRES(vq->access_lock) { struct buf_vector buf_vec[BUF_VECTOR_MAX]; uint32_t pkt_idx = 0; @@ -1691,6 +1699,7 @@ vhost_enqueue_async_packed(struct virtio_net *dev, struct buf_vector *buf_vec, uint16_t *nr_descs, uint16_t *nr_buffers) + VHOST_SPINLOCK_REQUIRES(vq->access_lock) { uint16_t nr_vec = 0; uint16_t avail_idx = vq->last_avail_idx; @@ -1748,6 +1757,7 @@ vhost_enqueue_async_packed(struct virtio_net *dev, static __rte_always_inline int16_t virtio_dev_rx_async_packed(struct virtio_net *dev, struct vhost_virtqueue *vq, struct rte_mbuf *pkt, uint16_t *nr_descs, uint16_t *nr_buffers) + VHOST_SPINLOCK_REQUIRES(vq->access_lock) { struct buf_vector buf_vec[BUF_VECTOR_MAX]; @@ -1766,6 +1776,7 @@ virtio_dev_rx_async_packed(struct virtio_net *dev, struct vhost_virtqueue *vq, static __rte_always_inline void dma_error_handler_packed(struct vhost_virtqueue *vq, uint16_t slot_idx, uint32_t nr_err, uint32_t *pkt_idx) + VHOST_SPINLOCK_REQUIRES(vq->access_lock) { uint16_t descs_err = 0; uint16_t buffers_err = 0; @@ -1793,6 +1804,7 @@ static __rte_noinline uint32_t virtio_dev_rx_async_submit_packed(struct virtio_net *dev, struct vhost_virtqueue *vq, uint16_t queue_id, struct rte_mbuf **pkts, uint32_t count, int16_t dma_id, uint16_t vchan_id) + VHOST_SPINLOCK_REQUIRES(vq->access_lock) { uint32_t pkt_idx = 0; uint32_t remained = count; @@ -1863,6 +1875,7 @@ virtio_dev_rx_async_submit_packed(struct virtio_net *dev, struct vhost_virtqueue static __rte_always_inline void write_back_completed_descs_split(struct vhost_virtqueue *vq, uint16_t n_descs) + VHOST_SPINLOCK_REQUIRES(vq->access_lock) { struct vhost_async *async = vq->async; uint16_t nr_left = n_descs; @@ -1895,6 +1908,7 @@ write_back_completed_descs_split(struct vhost_virtqueue *vq, uint16_t n_descs) static __rte_always_inline void write_back_completed_descs_packed(struct vhost_virtqueue *vq, uint16_t n_buffers) + VHOST_SPINLOCK_REQUIRES(vq->access_lock) { struct vhost_async *async = vq->async; uint16_t from = async->last_buffer_idx_packed; From patchwork Mon Mar 28 12:17:58 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: David Marchand X-Patchwork-Id: 108934 X-Patchwork-Delegate: maxime.coquelin@redhat.com Return-Path: X-Original-To: patchwork@inbox.dpdk.org Delivered-To: patchwork@inbox.dpdk.org Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124]) by inbox.dpdk.org (Postfix) with ESMTP id D5A92A0501; Mon, 28 Mar 2022 14:19:01 +0200 (CEST) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 8CC4742884; Mon, 28 Mar 2022 14:19:00 +0200 (CEST) Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.129.124]) by mails.dpdk.org (Postfix) with ESMTP id 1CD18427F8 for ; Mon, 28 Mar 2022 14:18:59 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1648469938; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=GPgHRqfKB43mibrcRaSetlatDxGXfwcaAlNWGJJVbHA=; b=Lmwzz5/RUL2YW3kWqOOwsbOZizJqdougHM1v42CkmGfnsk4EQ3XIY5iHbZNdP0LlHC5qY+ ZsC1avMPuVZJf3cMK4rWSvBELjSYfbMQslXBYNYBQFIV43eT/9rGr9N4CWvnnOGbcTD3Zg iViW5I/ewOts3ijNRywHEO4RPykI1OY= Received: from mimecast-mx02.redhat.com (mimecast-mx02.redhat.com [66.187.233.88]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id us-mta-223-uqq-3VaLOEaribxOpwR0UQ-1; Mon, 28 Mar 2022 08:18:55 -0400 X-MC-Unique: uqq-3VaLOEaribxOpwR0UQ-1 Received: from smtp.corp.redhat.com (int-mx08.intmail.prod.int.rdu2.redhat.com [10.11.54.8]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx02.redhat.com (Postfix) with ESMTPS id 2B782803D42; Mon, 28 Mar 2022 12:18:55 +0000 (UTC) Received: from dmarchan.remote.csb (unknown [10.40.195.27]) by smtp.corp.redhat.com (Postfix) with ESMTP id 28C72C07F50; Mon, 28 Mar 2022 12:18:54 +0000 (UTC) From: David Marchand To: dev@dpdk.org Cc: maxime.coquelin@redhat.com, chenbo.xia@intel.com Subject: [RFC PATCH 5/5] vhost: annotate IOTLB locks Date: Mon, 28 Mar 2022 14:17:58 +0200 Message-Id: <20220328121758.26632-6-david.marchand@redhat.com> In-Reply-To: <20220328121758.26632-1-david.marchand@redhat.com> References: <20220328121758.26632-1-david.marchand@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.85 on 10.11.54.8 Authentication-Results: relay.mimecast.com; auth=pass smtp.auth=CUSA124A263 smtp.mailfrom=david.marchand@redhat.com X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org The IOTLB code uses r/w locks. Introduce a wrapper around this type of locks and annotate iotlb_lock and iotlb_pending_lock. clang does not support conditionally held locks, so always take iotlb locks regardless of VIRTIO_F_IOMMU_PLATFORM feature. vdpa and vhost_crypto code are annotated though they end up not taking a IOTLB lock and have been marked with a FIXME. Signed-off-by: David Marchand --- lib/vhost/iotlb.c | 36 +++++++-------- lib/vhost/iotlb.h | 24 ---------- lib/vhost/vdpa.c | 1 + lib/vhost/vhost.c | 16 +++---- lib/vhost/vhost.h | 94 +++++++++++++++++++++++++++++++++++----- lib/vhost/vhost_crypto.c | 7 +++ lib/vhost/vhost_user.c | 16 +++++-- lib/vhost/virtio_net.c | 49 ++++++++++++++++----- 8 files changed, 164 insertions(+), 79 deletions(-) diff --git a/lib/vhost/iotlb.c b/lib/vhost/iotlb.c index 5a5ba8b82a..e5367d3af9 100644 --- a/lib/vhost/iotlb.c +++ b/lib/vhost/iotlb.c @@ -30,14 +30,14 @@ vhost_user_iotlb_pending_remove_all(struct vhost_virtqueue *vq) { struct vhost_iotlb_entry *node, *temp_node; - rte_rwlock_write_lock(&vq->iotlb_pending_lock); + vhost_rwlock_write_lock(&vq->iotlb_pending_lock); RTE_TAILQ_FOREACH_SAFE(node, &vq->iotlb_pending_list, next, temp_node) { TAILQ_REMOVE(&vq->iotlb_pending_list, node, next); rte_mempool_put(vq->iotlb_pool, node); } - rte_rwlock_write_unlock(&vq->iotlb_pending_lock); + vhost_rwlock_write_unlock(&vq->iotlb_pending_lock); } bool @@ -47,7 +47,7 @@ vhost_user_iotlb_pending_miss(struct vhost_virtqueue *vq, uint64_t iova, struct vhost_iotlb_entry *node; bool found = false; - rte_rwlock_read_lock(&vq->iotlb_pending_lock); + vhost_rwlock_read_lock(&vq->iotlb_pending_lock); TAILQ_FOREACH(node, &vq->iotlb_pending_list, next) { if ((node->iova == iova) && (node->perm == perm)) { @@ -56,7 +56,7 @@ vhost_user_iotlb_pending_miss(struct vhost_virtqueue *vq, uint64_t iova, } } - rte_rwlock_read_unlock(&vq->iotlb_pending_lock); + vhost_rwlock_read_unlock(&vq->iotlb_pending_lock); return found; } @@ -89,11 +89,11 @@ vhost_user_iotlb_pending_insert(struct virtio_net *dev, struct vhost_virtqueue * node->iova = iova; node->perm = perm; - rte_rwlock_write_lock(&vq->iotlb_pending_lock); + vhost_rwlock_write_lock(&vq->iotlb_pending_lock); TAILQ_INSERT_TAIL(&vq->iotlb_pending_list, node, next); - rte_rwlock_write_unlock(&vq->iotlb_pending_lock); + vhost_rwlock_write_unlock(&vq->iotlb_pending_lock); } void @@ -102,7 +102,7 @@ vhost_user_iotlb_pending_remove(struct vhost_virtqueue *vq, { struct vhost_iotlb_entry *node, *temp_node; - rte_rwlock_write_lock(&vq->iotlb_pending_lock); + vhost_rwlock_write_lock(&vq->iotlb_pending_lock); RTE_TAILQ_FOREACH_SAFE(node, &vq->iotlb_pending_list, next, temp_node) { @@ -116,7 +116,7 @@ vhost_user_iotlb_pending_remove(struct vhost_virtqueue *vq, rte_mempool_put(vq->iotlb_pool, node); } - rte_rwlock_write_unlock(&vq->iotlb_pending_lock); + vhost_rwlock_write_unlock(&vq->iotlb_pending_lock); } static void @@ -124,7 +124,7 @@ vhost_user_iotlb_cache_remove_all(struct vhost_virtqueue *vq) { struct vhost_iotlb_entry *node, *temp_node; - rte_rwlock_write_lock(&vq->iotlb_lock); + vhost_rwlock_write_lock(&vq->iotlb_lock); RTE_TAILQ_FOREACH_SAFE(node, &vq->iotlb_list, next, temp_node) { TAILQ_REMOVE(&vq->iotlb_list, node, next); @@ -133,7 +133,7 @@ vhost_user_iotlb_cache_remove_all(struct vhost_virtqueue *vq) vq->iotlb_cache_nr = 0; - rte_rwlock_write_unlock(&vq->iotlb_lock); + vhost_rwlock_write_unlock(&vq->iotlb_lock); } static void @@ -142,7 +142,7 @@ vhost_user_iotlb_cache_random_evict(struct vhost_virtqueue *vq) struct vhost_iotlb_entry *node, *temp_node; int entry_idx; - rte_rwlock_write_lock(&vq->iotlb_lock); + vhost_rwlock_write_lock(&vq->iotlb_lock); entry_idx = rte_rand() % vq->iotlb_cache_nr; @@ -156,7 +156,7 @@ vhost_user_iotlb_cache_random_evict(struct vhost_virtqueue *vq) entry_idx--; } - rte_rwlock_write_unlock(&vq->iotlb_lock); + vhost_rwlock_write_unlock(&vq->iotlb_lock); } void @@ -190,7 +190,7 @@ vhost_user_iotlb_cache_insert(struct virtio_net *dev, struct vhost_virtqueue *vq new_node->size = size; new_node->perm = perm; - rte_rwlock_write_lock(&vq->iotlb_lock); + vhost_rwlock_write_lock(&vq->iotlb_lock); TAILQ_FOREACH(node, &vq->iotlb_list, next) { /* @@ -213,7 +213,7 @@ vhost_user_iotlb_cache_insert(struct virtio_net *dev, struct vhost_virtqueue *vq unlock: vhost_user_iotlb_pending_remove(vq, iova, size, perm); - rte_rwlock_write_unlock(&vq->iotlb_lock); + vhost_rwlock_write_unlock(&vq->iotlb_lock); } @@ -226,7 +226,7 @@ vhost_user_iotlb_cache_remove(struct vhost_virtqueue *vq, if (unlikely(!size)) return; - rte_rwlock_write_lock(&vq->iotlb_lock); + vhost_rwlock_write_lock(&vq->iotlb_lock); RTE_TAILQ_FOREACH_SAFE(node, &vq->iotlb_list, next, temp_node) { /* Sorted list */ @@ -240,7 +240,7 @@ vhost_user_iotlb_cache_remove(struct vhost_virtqueue *vq, } } - rte_rwlock_write_unlock(&vq->iotlb_lock); + vhost_rwlock_write_unlock(&vq->iotlb_lock); } uint64_t @@ -312,8 +312,8 @@ vhost_user_iotlb_init(struct virtio_net *dev, int vq_index) socket = 0; #endif - rte_rwlock_init(&vq->iotlb_lock); - rte_rwlock_init(&vq->iotlb_pending_lock); + vhost_rwlock_init(&vq->iotlb_lock); + vhost_rwlock_init(&vq->iotlb_pending_lock); TAILQ_INIT(&vq->iotlb_list); TAILQ_INIT(&vq->iotlb_pending_list); diff --git a/lib/vhost/iotlb.h b/lib/vhost/iotlb.h index 8d0ff7473b..96ec4d608f 100644 --- a/lib/vhost/iotlb.h +++ b/lib/vhost/iotlb.h @@ -9,30 +9,6 @@ #include "vhost.h" -static __rte_always_inline void -vhost_user_iotlb_rd_lock(struct vhost_virtqueue *vq) -{ - rte_rwlock_read_lock(&vq->iotlb_lock); -} - -static __rte_always_inline void -vhost_user_iotlb_rd_unlock(struct vhost_virtqueue *vq) -{ - rte_rwlock_read_unlock(&vq->iotlb_lock); -} - -static __rte_always_inline void -vhost_user_iotlb_wr_lock(struct vhost_virtqueue *vq) -{ - rte_rwlock_write_lock(&vq->iotlb_lock); -} - -static __rte_always_inline void -vhost_user_iotlb_wr_unlock(struct vhost_virtqueue *vq) -{ - rte_rwlock_write_unlock(&vq->iotlb_lock); -} - void vhost_user_iotlb_cache_insert(struct virtio_net *dev, struct vhost_virtqueue *vq, uint64_t iova, uint64_t uaddr, uint64_t size, uint8_t perm); diff --git a/lib/vhost/vdpa.c b/lib/vhost/vdpa.c index 8fa2153023..406f13288b 100644 --- a/lib/vhost/vdpa.c +++ b/lib/vhost/vdpa.c @@ -130,6 +130,7 @@ rte_vdpa_unregister_device(struct rte_vdpa_device *dev) int rte_vdpa_relay_vring_used(int vid, uint16_t qid, void *vring_m) + VHOST_NO_THREAD_SAFETY_ANALYSIS /* FIXME: requires iotlb_lock? */ { struct virtio_net *dev = get_device(vid); uint16_t idx, idx_m, desc_id; diff --git a/lib/vhost/vhost.c b/lib/vhost/vhost.c index ea28323367..466a24dabe 100644 --- a/lib/vhost/vhost.c +++ b/lib/vhost/vhost.c @@ -50,7 +50,7 @@ __vhost_iova_to_vva(struct virtio_net *dev, struct vhost_virtqueue *vq, * which could cause a deadlock with QEMU if an IOTLB update * is being handled. We can safely unlock here to avoid it. */ - vhost_user_iotlb_rd_unlock(vq); + vhost_rwlock_read_unlock(&vq->iotlb_lock); vhost_user_iotlb_pending_insert(dev, vq, iova, perm); if (vhost_user_iotlb_miss(dev, iova, perm)) { @@ -59,7 +59,7 @@ __vhost_iova_to_vva(struct virtio_net *dev, struct vhost_virtqueue *vq, vhost_user_iotlb_pending_remove(vq, iova, 1, perm); } - vhost_user_iotlb_rd_lock(vq); + vhost_rwlock_read_lock(&vq->iotlb_lock); } return 0; @@ -383,6 +383,7 @@ free_device(struct virtio_net *dev) static __rte_always_inline int log_translate(struct virtio_net *dev, struct vhost_virtqueue *vq) + VHOST_RDLOCK_REQUIRES(vq->iotlb_lock) { if (likely(!(vq->ring_addrs.flags & (1 << VHOST_VRING_F_LOG)))) return 0; @@ -434,6 +435,7 @@ translate_log_addr(struct virtio_net *dev, struct vhost_virtqueue *vq, /* Caller should have iotlb_lock read-locked */ static int vring_translate_split(struct virtio_net *dev, struct vhost_virtqueue *vq) + VHOST_RDLOCK_REQUIRES(vq->iotlb_lock) { uint64_t req_size, size; @@ -473,6 +475,7 @@ vring_translate_split(struct virtio_net *dev, struct vhost_virtqueue *vq) /* Caller should have iotlb_lock read-locked */ static int vring_translate_packed(struct virtio_net *dev, struct vhost_virtqueue *vq) + VHOST_RDLOCK_REQUIRES(vq->iotlb_lock) { uint64_t req_size, size; @@ -506,7 +509,6 @@ vring_translate_packed(struct virtio_net *dev, struct vhost_virtqueue *vq) int vring_translate(struct virtio_net *dev, struct vhost_virtqueue *vq) { - if (!(dev->features & (1ULL << VIRTIO_F_IOMMU_PLATFORM))) return -1; @@ -527,19 +529,13 @@ vring_translate(struct virtio_net *dev, struct vhost_virtqueue *vq) } void -vring_invalidate(struct virtio_net *dev, struct vhost_virtqueue *vq) +vring_invalidate(struct virtio_net *dev __rte_unused, struct vhost_virtqueue *vq) { - if (dev->features & (1ULL << VIRTIO_F_IOMMU_PLATFORM)) - vhost_user_iotlb_wr_lock(vq); - vq->access_ok = false; vq->desc = NULL; vq->avail = NULL; vq->used = NULL; vq->log_guest_addr = 0; - - if (dev->features & (1ULL << VIRTIO_F_IOMMU_PLATFORM)) - vhost_user_iotlb_wr_unlock(vq); } static void diff --git a/lib/vhost/vhost.h b/lib/vhost/vhost.h index 4b301ec152..08ace9994a 100644 --- a/lib/vhost/vhost.h +++ b/lib/vhost/vhost.h @@ -108,6 +108,19 @@ #define VHOST_SPINLOCK_RELEASES(...) \ __attribute__((unlock_function(__VA_ARGS__))) +#define VHOST_RDLOCK_REQUIRES(...) \ + __attribute__((shared_locks_required(__VA_ARGS__))) +#define VHOST_RDLOCK_ACQUIRES(...) \ + __attribute__((shared_lock_function(__VA_ARGS__))) +#define VHOST_RDLOCK_RELEASES(...) \ + __attribute__((unlock_function(__VA_ARGS__))) +#define VHOST_WRLOCK_REQUIRES(...) \ + __attribute__((exclusive_locks_required(__VA_ARGS__))) +#define VHOST_WRLOCK_ACQUIRES(...) \ + __attribute__((exclusive_lock_function(__VA_ARGS__))) +#define VHOST_WRLOCK_RELEASES(...) \ + __attribute__((unlock_function(__VA_ARGS__))) + #else #define VHOST_NO_THREAD_SAFETY_ANALYSIS #define VHOST_LOCKABLE @@ -117,6 +130,13 @@ #define VHOST_SPINLOCK_ACQUIRES(...) #define VHOST_SPINLOCK_TRYLOCK(...) #define VHOST_SPINLOCK_RELEASES(...) + +#define VHOST_RDLOCK_ACQUIRES(...) +#define VHOST_RDLOCK_REQUIRES(...) +#define VHOST_RDLOCK_RELEASES(...) +#define VHOST_WRLOCK_REQUIRES(...) +#define VHOST_WRLOCK_ACQUIRES(...) +#define VHOST_WRLOCK_RELEASES(...) #endif typedef struct VHOST_LOCKABLE { @@ -153,6 +173,48 @@ vhost_spinlock_unlock(vhost_spinlock_t *l) rte_spinlock_unlock(&l->l); } +typedef struct VHOST_LOCKABLE { + rte_rwlock_t l; +} vhost_rwlock_t; + +static __rte_always_inline void +vhost_rwlock_init(vhost_rwlock_t *l) +{ + rte_rwlock_init(&l->l); +} + +static __rte_always_inline void +vhost_rwlock_read_lock(vhost_rwlock_t *l) + VHOST_RDLOCK_ACQUIRES(l) + VHOST_NO_THREAD_SAFETY_ANALYSIS +{ + rte_rwlock_read_lock(&l->l); +} + +static __rte_always_inline void +vhost_rwlock_read_unlock(vhost_rwlock_t *l) + VHOST_RDLOCK_RELEASES(l) + VHOST_NO_THREAD_SAFETY_ANALYSIS +{ + rte_rwlock_read_unlock(&l->l); +} + +static __rte_always_inline void +vhost_rwlock_write_lock(vhost_rwlock_t *l) + VHOST_WRLOCK_ACQUIRES(l) + VHOST_NO_THREAD_SAFETY_ANALYSIS +{ + rte_rwlock_write_lock(&l->l); +} + +static __rte_always_inline void +vhost_rwlock_write_unlock(vhost_rwlock_t *l) + VHOST_WRLOCK_RELEASES(l) + VHOST_NO_THREAD_SAFETY_ANALYSIS +{ + rte_rwlock_write_unlock(&l->l); +} + /** * Structure contains buffer address, length and descriptor index * from vring to do scatter RX. @@ -346,8 +408,8 @@ struct vhost_virtqueue { uint64_t log_guest_addr; struct log_cache_entry *log_cache; - rte_rwlock_t iotlb_lock; - rte_rwlock_t iotlb_pending_lock; + vhost_rwlock_t iotlb_lock; + vhost_rwlock_t iotlb_pending_lock; struct rte_mempool *iotlb_pool; TAILQ_HEAD(, vhost_iotlb_entry) iotlb_list; TAILQ_HEAD(, vhost_iotlb_entry) iotlb_pending_list; @@ -592,12 +654,15 @@ void __vhost_log_cache_write(struct virtio_net *dev, uint64_t addr, uint64_t len); void __vhost_log_cache_write_iova(struct virtio_net *dev, struct vhost_virtqueue *vq, - uint64_t iova, uint64_t len); + uint64_t iova, uint64_t len) + VHOST_RDLOCK_REQUIRES(vq->iotlb_lock); void __vhost_log_cache_sync(struct virtio_net *dev, struct vhost_virtqueue *vq); + void __vhost_log_write(struct virtio_net *dev, uint64_t addr, uint64_t len); void __vhost_log_write_iova(struct virtio_net *dev, struct vhost_virtqueue *vq, - uint64_t iova, uint64_t len); + uint64_t iova, uint64_t len) + VHOST_RDLOCK_REQUIRES(vq->iotlb_lock); static __rte_always_inline void vhost_log_write(struct virtio_net *dev, uint64_t addr, uint64_t len) @@ -647,6 +712,7 @@ vhost_log_used_vring(struct virtio_net *dev, struct vhost_virtqueue *vq, static __rte_always_inline void vhost_log_cache_write_iova(struct virtio_net *dev, struct vhost_virtqueue *vq, uint64_t iova, uint64_t len) + VHOST_RDLOCK_REQUIRES(vq->iotlb_lock) { if (likely(!(dev->features & (1ULL << VHOST_F_LOG_ALL)))) return; @@ -660,6 +726,7 @@ vhost_log_cache_write_iova(struct virtio_net *dev, struct vhost_virtqueue *vq, static __rte_always_inline void vhost_log_write_iova(struct virtio_net *dev, struct vhost_virtqueue *vq, uint64_t iova, uint64_t len) + VHOST_RDLOCK_REQUIRES(vq->iotlb_lock) { if (likely(!(dev->features & (1ULL << VHOST_F_LOG_ALL)))) return; @@ -863,18 +930,23 @@ struct rte_vhost_device_ops const *vhost_driver_callback_get(const char *path); void vhost_backend_cleanup(struct virtio_net *dev); uint64_t __vhost_iova_to_vva(struct virtio_net *dev, struct vhost_virtqueue *vq, - uint64_t iova, uint64_t *len, uint8_t perm); -void *vhost_alloc_copy_ind_table(struct virtio_net *dev, - struct vhost_virtqueue *vq, - uint64_t desc_addr, uint64_t desc_len); -int vring_translate(struct virtio_net *dev, struct vhost_virtqueue *vq); + uint64_t iova, uint64_t *len, uint8_t perm) + VHOST_RDLOCK_REQUIRES(vq->iotlb_lock); +void *vhost_alloc_copy_ind_table(struct virtio_net *dev, struct vhost_virtqueue *vq, + uint64_t desc_addr, uint64_t desc_len) + VHOST_RDLOCK_REQUIRES(vq->iotlb_lock); +int vring_translate(struct virtio_net *dev, struct vhost_virtqueue *vq) + VHOST_RDLOCK_REQUIRES(vq->iotlb_lock); uint64_t translate_log_addr(struct virtio_net *dev, struct vhost_virtqueue *vq, - uint64_t log_addr); -void vring_invalidate(struct virtio_net *dev, struct vhost_virtqueue *vq); + uint64_t log_addr) + VHOST_RDLOCK_REQUIRES(vq->iotlb_lock); +void vring_invalidate(struct virtio_net *dev, struct vhost_virtqueue *vq) + VHOST_WRLOCK_REQUIRES(vq->iotlb_lock); static __rte_always_inline uint64_t vhost_iova_to_vva(struct virtio_net *dev, struct vhost_virtqueue *vq, uint64_t iova, uint64_t *len, uint8_t perm) + VHOST_RDLOCK_REQUIRES(vq->iotlb_lock) { if (!(dev->features & (1ULL << VIRTIO_F_IOMMU_PLATFORM))) return rte_vhost_va_from_guest_pa(dev->mem, iova, len); diff --git a/lib/vhost/vhost_crypto.c b/lib/vhost/vhost_crypto.c index b1c0eb6a0f..2d0a1250fb 100644 --- a/lib/vhost/vhost_crypto.c +++ b/lib/vhost/vhost_crypto.c @@ -506,6 +506,7 @@ static __rte_always_inline struct virtio_crypto_inhdr * reach_inhdr(struct vhost_crypto_data_req *vc_req, struct vhost_crypto_desc *head, uint32_t max_n_descs) + VHOST_RDLOCK_REQUIRES(vc_req->vq->iotlb_lock) { struct virtio_crypto_inhdr *inhdr; struct vhost_crypto_desc *last = head + (max_n_descs - 1); @@ -552,6 +553,7 @@ static __rte_always_inline void * get_data_ptr(struct vhost_crypto_data_req *vc_req, struct vhost_crypto_desc *cur_desc, uint8_t perm) + VHOST_RDLOCK_REQUIRES(vc_req->vq->iotlb_lock) { void *data; uint64_t dlen = cur_desc->len; @@ -570,6 +572,7 @@ copy_data(void *dst_data, struct vhost_crypto_data_req *vc_req, struct vhost_crypto_desc *head, struct vhost_crypto_desc **cur_desc, uint32_t size, uint32_t max_n_descs) + VHOST_RDLOCK_REQUIRES(vc_req->vq->iotlb_lock) { struct vhost_crypto_desc *desc = *cur_desc; uint64_t remain, addr, dlen, len; @@ -718,6 +721,7 @@ prepare_write_back_data(struct vhost_crypto_data_req *vc_req, uint32_t offset, uint64_t write_back_len, uint32_t max_n_descs) + VHOST_RDLOCK_REQUIRES(vc_req->vq->iotlb_lock) { struct vhost_crypto_writeback_data *wb_data, *head; struct vhost_crypto_desc *desc = *cur_desc; @@ -838,6 +842,7 @@ prepare_sym_cipher_op(struct vhost_crypto *vcrypto, struct rte_crypto_op *op, struct virtio_crypto_cipher_data_req *cipher, struct vhost_crypto_desc *head, uint32_t max_n_descs) + VHOST_RDLOCK_REQUIRES(vc_req->vq->iotlb_lock) { struct vhost_crypto_desc *desc = head; struct vhost_crypto_writeback_data *ewb = NULL; @@ -990,6 +995,7 @@ prepare_sym_chain_op(struct vhost_crypto *vcrypto, struct rte_crypto_op *op, struct virtio_crypto_alg_chain_data_req *chain, struct vhost_crypto_desc *head, uint32_t max_n_descs) + VHOST_RDLOCK_REQUIRES(vc_req->vq->iotlb_lock) { struct vhost_crypto_desc *desc = head, *digest_desc; struct vhost_crypto_writeback_data *ewb = NULL, *ewb2 = NULL; @@ -1172,6 +1178,7 @@ vhost_crypto_process_one_req(struct vhost_crypto *vcrypto, struct vhost_virtqueue *vq, struct rte_crypto_op *op, struct vring_desc *head, struct vhost_crypto_desc *descs, uint16_t desc_idx) + VHOST_NO_THREAD_SAFETY_ANALYSIS /* FIXME: requires iotlb_lock? */ { struct vhost_crypto_data_req *vc_req = rte_mbuf_to_priv(op->sym->m_src); struct rte_cryptodev_sym_session *session; diff --git a/lib/vhost/vhost_user.c b/lib/vhost/vhost_user.c index 35f1e23995..0482c00801 100644 --- a/lib/vhost/vhost_user.c +++ b/lib/vhost/vhost_user.c @@ -754,10 +754,10 @@ ring_addr_to_vva(struct virtio_net *dev, struct vhost_virtqueue *vq, if (dev->features & (1ULL << VIRTIO_F_IOMMU_PLATFORM)) { uint64_t vva; - vhost_user_iotlb_rd_lock(vq); + vhost_rwlock_read_lock(&vq->iotlb_lock); vva = vhost_iova_to_vva(dev, vq, ra, size, VHOST_ACCESS_RW); - vhost_user_iotlb_rd_unlock(vq); + vhost_rwlock_read_unlock(&vq->iotlb_lock); return vva; } @@ -770,9 +770,9 @@ log_addr_to_gpa(struct virtio_net *dev, struct vhost_virtqueue *vq) { uint64_t log_gpa; - vhost_user_iotlb_rd_lock(vq); + vhost_rwlock_read_lock(&vq->iotlb_lock); log_gpa = translate_log_addr(dev, vq, vq->ring_addrs.log_guest_addr); - vhost_user_iotlb_rd_unlock(vq); + vhost_rwlock_read_unlock(&vq->iotlb_lock); return log_gpa; } @@ -927,7 +927,9 @@ vhost_user_set_vring_addr(struct virtio_net **pdev, */ memcpy(&vq->ring_addrs, addr, sizeof(*addr)); + vhost_rwlock_write_lock(&vq->iotlb_lock); vring_invalidate(dev, vq); + vhost_rwlock_write_unlock(&vq->iotlb_lock); if ((vq->enabled && (dev->features & (1ULL << VHOST_USER_F_PROTOCOL_FEATURES))) || @@ -1437,7 +1439,9 @@ vhost_user_set_mem_table(struct virtio_net **pdev, * need to be translated again as virtual addresses have * changed. */ + vhost_rwlock_write_lock(&vq->iotlb_lock); vring_invalidate(dev, vq); + vhost_rwlock_write_unlock(&vq->iotlb_lock); dev = translate_ring_addresses(dev, i); if (!dev) { @@ -2186,7 +2190,9 @@ vhost_user_get_vring_base(struct virtio_net **pdev, vhost_user_iotlb_flush_all(vq); + vhost_rwlock_write_lock(&vq->iotlb_lock); vring_invalidate(dev, vq); + vhost_rwlock_write_unlock(&vq->iotlb_lock); return RTE_VHOST_MSG_RESULT_REPLY; } @@ -2591,7 +2597,9 @@ vhost_user_iotlb_msg(struct virtio_net **pdev, if (is_vring_iotlb(dev, vq, imsg)) { vhost_spinlock_lock(&vq->access_lock); + vhost_rwlock_write_lock(&vq->iotlb_lock); vring_invalidate(dev, vq); + vhost_rwlock_write_unlock(&vq->iotlb_lock); vhost_spinlock_unlock(&vq->access_lock); } } diff --git a/lib/vhost/virtio_net.c b/lib/vhost/virtio_net.c index 9bdba992dd..ec342e772d 100644 --- a/lib/vhost/virtio_net.c +++ b/lib/vhost/virtio_net.c @@ -180,6 +180,7 @@ vhost_async_dma_check_completed(struct virtio_net *dev, int16_t dma_id, uint16_t static inline void do_data_copy_enqueue(struct virtio_net *dev, struct vhost_virtqueue *vq) + VHOST_RDLOCK_REQUIRES(vq->iotlb_lock) { struct batch_copy_elem *elem = vq->batch_copy_elems; uint16_t count = vq->batch_copy_nb_elems; @@ -526,6 +527,7 @@ vhost_shadow_enqueue_single_packed(struct virtio_net *dev, uint16_t *id, uint16_t *count, uint16_t num_buffers) + VHOST_RDLOCK_REQUIRES(vq->iotlb_lock) { vhost_shadow_enqueue_packed(vq, len, id, count, num_buffers); @@ -607,6 +609,7 @@ static __rte_always_inline int map_one_desc(struct virtio_net *dev, struct vhost_virtqueue *vq, struct buf_vector *buf_vec, uint16_t *vec_idx, uint64_t desc_iova, uint64_t desc_len, uint8_t perm) + VHOST_RDLOCK_REQUIRES(vq->iotlb_lock) { uint16_t vec_id = *vec_idx; @@ -644,6 +647,7 @@ fill_vec_buf_split(struct virtio_net *dev, struct vhost_virtqueue *vq, uint32_t avail_idx, uint16_t *vec_idx, struct buf_vector *buf_vec, uint16_t *desc_chain_head, uint32_t *desc_chain_len, uint8_t perm) + VHOST_RDLOCK_REQUIRES(vq->iotlb_lock) { uint16_t idx = vq->avail->ring[avail_idx & (vq->size - 1)]; uint16_t vec_id = *vec_idx; @@ -727,6 +731,7 @@ reserve_avail_buf_split(struct virtio_net *dev, struct vhost_virtqueue *vq, uint32_t size, struct buf_vector *buf_vec, uint16_t *num_buffers, uint16_t avail_head, uint16_t *nr_vec) + VHOST_RDLOCK_REQUIRES(vq->iotlb_lock) { uint16_t cur_idx; uint16_t vec_idx = 0; @@ -777,6 +782,7 @@ fill_vec_buf_packed_indirect(struct virtio_net *dev, struct vhost_virtqueue *vq, struct vring_packed_desc *desc, uint16_t *vec_idx, struct buf_vector *buf_vec, uint32_t *len, uint8_t perm) + VHOST_RDLOCK_REQUIRES(vq->iotlb_lock) { uint16_t i; uint32_t nr_descs; @@ -835,6 +841,7 @@ fill_vec_buf_packed(struct virtio_net *dev, struct vhost_virtqueue *vq, uint16_t avail_idx, uint16_t *desc_count, struct buf_vector *buf_vec, uint16_t *vec_idx, uint16_t *buf_id, uint32_t *len, uint8_t perm) + VHOST_RDLOCK_REQUIRES(vq->iotlb_lock) { bool wrap_counter = vq->avail_wrap_counter; struct vring_packed_desc *descs = vq->desc_packed; @@ -900,6 +907,7 @@ static __rte_noinline void copy_vnet_hdr_to_desc(struct virtio_net *dev, struct vhost_virtqueue *vq, struct buf_vector *buf_vec, struct virtio_net_hdr_mrg_rxbuf *hdr) + VHOST_RDLOCK_REQUIRES(vq->iotlb_lock) { uint64_t len; uint64_t remain = dev->vhost_hlen; @@ -1036,6 +1044,7 @@ static __rte_always_inline void sync_mbuf_to_desc_seg(struct virtio_net *dev, struct vhost_virtqueue *vq, struct rte_mbuf *m, uint32_t mbuf_offset, uint64_t buf_addr, uint64_t buf_iova, uint32_t cpy_len) + VHOST_RDLOCK_REQUIRES(vq->iotlb_lock) { struct batch_copy_elem *batch_copy = vq->batch_copy_elems; @@ -1061,6 +1070,7 @@ mbuf_to_desc(struct virtio_net *dev, struct vhost_virtqueue *vq, struct rte_mbuf *m, struct buf_vector *buf_vec, uint16_t nr_vec, uint16_t num_buffers, bool is_async) VHOST_SPINLOCK_REQUIRES(vq->access_lock) + VHOST_RDLOCK_REQUIRES(vq->iotlb_lock) { uint32_t vec_idx = 0; uint32_t mbuf_offset, mbuf_avail; @@ -1191,6 +1201,7 @@ vhost_enqueue_single_packed(struct virtio_net *dev, struct buf_vector *buf_vec, uint16_t *nr_descs) VHOST_SPINLOCK_REQUIRES(vq->access_lock) + VHOST_RDLOCK_REQUIRES(vq->iotlb_lock) { uint16_t nr_vec = 0; uint16_t avail_idx = vq->last_avail_idx; @@ -1252,6 +1263,7 @@ static __rte_noinline uint32_t virtio_dev_rx_split(struct virtio_net *dev, struct vhost_virtqueue *vq, struct rte_mbuf **pkts, uint32_t count) VHOST_SPINLOCK_REQUIRES(vq->access_lock) + VHOST_RDLOCK_REQUIRES(vq->iotlb_lock) { uint32_t pkt_idx = 0; uint16_t num_buffers; @@ -1309,6 +1321,7 @@ virtio_dev_rx_sync_batch_check(struct virtio_net *dev, struct rte_mbuf **pkts, uint64_t *desc_addrs, uint64_t *lens) + VHOST_RDLOCK_REQUIRES(vq->iotlb_lock) { bool wrap_counter = vq->avail_wrap_counter; struct vring_packed_desc *descs = vq->desc_packed; @@ -1360,6 +1373,7 @@ virtio_dev_rx_batch_packed_copy(struct virtio_net *dev, struct rte_mbuf **pkts, uint64_t *desc_addrs, uint64_t *lens) + VHOST_RDLOCK_REQUIRES(vq->iotlb_lock) { uint32_t buf_offset = sizeof(struct virtio_net_hdr_mrg_rxbuf); struct virtio_net_hdr_mrg_rxbuf *hdrs[PACKED_BATCH_SIZE]; @@ -1401,6 +1415,7 @@ static __rte_always_inline int virtio_dev_rx_sync_batch_packed(struct virtio_net *dev, struct vhost_virtqueue *vq, struct rte_mbuf **pkts) + VHOST_RDLOCK_REQUIRES(vq->iotlb_lock) { uint64_t desc_addrs[PACKED_BATCH_SIZE]; uint64_t lens[PACKED_BATCH_SIZE]; @@ -1423,6 +1438,7 @@ virtio_dev_rx_single_packed(struct virtio_net *dev, struct vhost_virtqueue *vq, struct rte_mbuf *pkt) VHOST_SPINLOCK_REQUIRES(vq->access_lock) + VHOST_RDLOCK_REQUIRES(vq->iotlb_lock) { struct buf_vector buf_vec[BUF_VECTOR_MAX]; uint16_t nr_descs = 0; @@ -1449,6 +1465,7 @@ virtio_dev_rx_packed(struct virtio_net *dev, struct rte_mbuf **__rte_restrict pkts, uint32_t count) VHOST_SPINLOCK_REQUIRES(vq->access_lock) + VHOST_RDLOCK_REQUIRES(vq->iotlb_lock) { uint32_t pkt_idx = 0; @@ -1501,8 +1518,7 @@ virtio_dev_rx(struct virtio_net *dev, uint16_t queue_id, if (unlikely(!vq->enabled)) goto out_access_unlock; - if (dev->features & (1ULL << VIRTIO_F_IOMMU_PLATFORM)) - vhost_user_iotlb_rd_lock(vq); + vhost_rwlock_read_lock(&vq->iotlb_lock); if (unlikely(!vq->access_ok)) if (unlikely(vring_translate(dev, vq) < 0)) @@ -1518,8 +1534,7 @@ virtio_dev_rx(struct virtio_net *dev, uint16_t queue_id, nb_tx = virtio_dev_rx_split(dev, vq, pkts, count); out: - if (dev->features & (1ULL << VIRTIO_F_IOMMU_PLATFORM)) - vhost_user_iotlb_rd_unlock(vq); + vhost_rwlock_read_unlock(&vq->iotlb_lock); out_access_unlock: vhost_spinlock_unlock(&vq->access_lock); @@ -1595,6 +1610,7 @@ virtio_dev_rx_async_submit_split(struct virtio_net *dev, struct vhost_virtqueue uint16_t queue_id, struct rte_mbuf **pkts, uint32_t count, int16_t dma_id, uint16_t vchan_id) VHOST_SPINLOCK_REQUIRES(vq->access_lock) + VHOST_RDLOCK_REQUIRES(vq->iotlb_lock) { struct buf_vector buf_vec[BUF_VECTOR_MAX]; uint32_t pkt_idx = 0; @@ -1700,6 +1716,7 @@ vhost_enqueue_async_packed(struct virtio_net *dev, uint16_t *nr_descs, uint16_t *nr_buffers) VHOST_SPINLOCK_REQUIRES(vq->access_lock) + VHOST_RDLOCK_REQUIRES(vq->iotlb_lock) { uint16_t nr_vec = 0; uint16_t avail_idx = vq->last_avail_idx; @@ -1758,6 +1775,7 @@ static __rte_always_inline int16_t virtio_dev_rx_async_packed(struct virtio_net *dev, struct vhost_virtqueue *vq, struct rte_mbuf *pkt, uint16_t *nr_descs, uint16_t *nr_buffers) VHOST_SPINLOCK_REQUIRES(vq->access_lock) + VHOST_RDLOCK_REQUIRES(vq->iotlb_lock) { struct buf_vector buf_vec[BUF_VECTOR_MAX]; @@ -1805,6 +1823,7 @@ virtio_dev_rx_async_submit_packed(struct virtio_net *dev, struct vhost_virtqueue uint16_t queue_id, struct rte_mbuf **pkts, uint32_t count, int16_t dma_id, uint16_t vchan_id) VHOST_SPINLOCK_REQUIRES(vq->access_lock) + VHOST_RDLOCK_REQUIRES(vq->iotlb_lock) { uint32_t pkt_idx = 0; uint32_t remained = count; @@ -2154,8 +2173,7 @@ virtio_dev_rx_async_submit(struct virtio_net *dev, uint16_t queue_id, if (unlikely(!vq->enabled || !vq->async)) goto out_access_unlock; - if (dev->features & (1ULL << VIRTIO_F_IOMMU_PLATFORM)) - vhost_user_iotlb_rd_lock(vq); + vhost_rwlock_read_lock(&vq->iotlb_lock); if (unlikely(!vq->access_ok)) if (unlikely(vring_translate(dev, vq) < 0)) @@ -2173,8 +2191,7 @@ virtio_dev_rx_async_submit(struct virtio_net *dev, uint16_t queue_id, pkts, count, dma_id, vchan_id); out: - if (dev->features & (1ULL << VIRTIO_F_IOMMU_PLATFORM)) - vhost_user_iotlb_rd_unlock(vq); + vhost_rwlock_read_unlock(&vq->iotlb_lock); out_access_unlock: vhost_spinlock_unlock(&vq->access_lock); @@ -2697,6 +2714,7 @@ virtio_dev_tx_split(struct virtio_net *dev, struct vhost_virtqueue *vq, struct rte_mempool *mbuf_pool, struct rte_mbuf **pkts, uint16_t count, bool legacy_ol_flags) VHOST_SPINLOCK_REQUIRES(vq->access_lock) + VHOST_RDLOCK_REQUIRES(vq->iotlb_lock) { uint16_t i; uint16_t free_entries; @@ -2793,6 +2811,7 @@ virtio_dev_tx_split_legacy(struct virtio_net *dev, struct vhost_virtqueue *vq, struct rte_mempool *mbuf_pool, struct rte_mbuf **pkts, uint16_t count) VHOST_SPINLOCK_REQUIRES(vq->access_lock) + VHOST_RDLOCK_REQUIRES(vq->iotlb_lock) { return virtio_dev_tx_split(dev, vq, mbuf_pool, pkts, count, true); } @@ -2803,6 +2822,7 @@ virtio_dev_tx_split_compliant(struct virtio_net *dev, struct vhost_virtqueue *vq, struct rte_mempool *mbuf_pool, struct rte_mbuf **pkts, uint16_t count) VHOST_SPINLOCK_REQUIRES(vq->access_lock) + VHOST_RDLOCK_REQUIRES(vq->iotlb_lock) { return virtio_dev_tx_split(dev, vq, mbuf_pool, pkts, count, false); } @@ -2814,6 +2834,7 @@ vhost_reserve_avail_batch_packed(struct virtio_net *dev, uint16_t avail_idx, uintptr_t *desc_addrs, uint16_t *ids) + VHOST_RDLOCK_REQUIRES(vq->iotlb_lock) { bool wrap = vq->avail_wrap_counter; struct vring_packed_desc *descs = vq->desc_packed; @@ -2883,6 +2904,7 @@ virtio_dev_tx_batch_packed(struct virtio_net *dev, struct vhost_virtqueue *vq, struct rte_mbuf **pkts, bool legacy_ol_flags) + VHOST_RDLOCK_REQUIRES(vq->iotlb_lock) { uint16_t avail_idx = vq->last_avail_idx; uint32_t buf_offset = sizeof(struct virtio_net_hdr_mrg_rxbuf); @@ -2929,6 +2951,7 @@ vhost_dequeue_single_packed(struct virtio_net *dev, uint16_t *buf_id, uint16_t *desc_count, bool legacy_ol_flags) + VHOST_RDLOCK_REQUIRES(vq->iotlb_lock) { struct buf_vector buf_vec[BUF_VECTOR_MAX]; uint32_t buf_len; @@ -2972,6 +2995,7 @@ virtio_dev_tx_single_packed(struct virtio_net *dev, struct rte_mempool *mbuf_pool, struct rte_mbuf *pkts, bool legacy_ol_flags) + VHOST_RDLOCK_REQUIRES(vq->iotlb_lock) { uint16_t buf_id, desc_count = 0; @@ -3003,6 +3027,7 @@ virtio_dev_tx_packed(struct virtio_net *dev, uint32_t count, bool legacy_ol_flags) VHOST_SPINLOCK_REQUIRES(vq->access_lock) + VHOST_RDLOCK_REQUIRES(vq->iotlb_lock) { uint32_t pkt_idx = 0; @@ -3047,6 +3072,7 @@ virtio_dev_tx_packed_legacy(struct virtio_net *dev, struct vhost_virtqueue *__rte_restrict vq, struct rte_mempool *mbuf_pool, struct rte_mbuf **__rte_restrict pkts, uint32_t count) VHOST_SPINLOCK_REQUIRES(vq->access_lock) + VHOST_RDLOCK_REQUIRES(vq->iotlb_lock) { return virtio_dev_tx_packed(dev, vq, mbuf_pool, pkts, count, true); } @@ -3057,6 +3083,7 @@ virtio_dev_tx_packed_compliant(struct virtio_net *dev, struct vhost_virtqueue *__rte_restrict vq, struct rte_mempool *mbuf_pool, struct rte_mbuf **__rte_restrict pkts, uint32_t count) VHOST_SPINLOCK_REQUIRES(vq->access_lock) + VHOST_RDLOCK_REQUIRES(vq->iotlb_lock) { return virtio_dev_tx_packed(dev, vq, mbuf_pool, pkts, count, false); } @@ -3096,8 +3123,7 @@ rte_vhost_dequeue_burst(int vid, uint16_t queue_id, goto out_access_unlock; } - if (dev->features & (1ULL << VIRTIO_F_IOMMU_PLATFORM)) - vhost_user_iotlb_rd_lock(vq); + vhost_rwlock_read_lock(&vq->iotlb_lock); if (unlikely(!vq->access_ok)) if (unlikely(vring_translate(dev, vq) < 0)) { @@ -3153,8 +3179,7 @@ rte_vhost_dequeue_burst(int vid, uint16_t queue_id, } out: - if (dev->features & (1ULL << VIRTIO_F_IOMMU_PLATFORM)) - vhost_user_iotlb_rd_unlock(vq); + vhost_rwlock_read_unlock(&vq->iotlb_lock); out_access_unlock: vhost_spinlock_unlock(&vq->access_lock);