From: "Michael S. Tsirkin" <mst@redhat.com>
To: Ilya Maximets <i.maximets@samsung.com>
Cc: dev@dpdk.org, Maxime Coquelin <maxime.coquelin@redhat.com>,
Xiao Wang <xiao.w.wang@intel.com>,
Tiwei Bie <tiwei.bie@intel.com>,
Zhihong Wang <zhihong.wang@intel.com>,
jfreimann@redhat.com, Jason Wang <jasowang@redhat.com>,
xiaolong.ye@intel.com, alejandro.lucero@netronome.com
Subject: Re: [PATCH] net/virtio: add platform memory ordering feature support
Date: Fri, 14 Dec 2018 12:00:51 -0500 [thread overview]
Message-ID: <20181214120038-mutt-send-email-mst@kernel.org> (raw)
In-Reply-To: <20181214153812.3878-1-i.maximets@samsung.com>
On Fri, Dec 14, 2018 at 06:38:12PM +0300, Ilya Maximets wrote:
> VIRTIO_F_ORDER_PLATFORM is required to use proper memory barriers
> in case of HW vhost implementations like vDPA.
>
> DMA barriers (rte_cio_*) are sufficent for that purpose.
>
> Previously known as VIRTIO_F_IO_BARRIER.
>
> Signed-off-by: Ilya Maximets <i.maximets@samsung.com>
> ---
>
> Based on "[RFC] net/virtio: use real barriers for vDPA".
>
> RFC --> PATCH:
> * Dropped vendor-specific hack to determine if we need real barriers.
> * Added VIRTIO_F_ORDER_PLATFORM feature definition and checking.
>
> Note: Patch to change the name of the feature from VIRTIO_F_IO_BARRIER
> to VIRTIO_F_ORDER_PLATFORM is not merged yet:
> https://www.mail-archive.com/virtio-dev@lists.oasis-open.org/msg04114.html
>
> drivers/net/virtio/virtio_ethdev.c | 2 ++
> drivers/net/virtio/virtio_ethdev.h | 3 ++-
> drivers/net/virtio/virtio_pci.h | 7 ++++++
> drivers/net/virtio/virtio_rxtx.c | 14 ++++++------
> drivers/net/virtio/virtqueue.h | 35 +++++++++++++++++++++++++-----
> 5 files changed, 48 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/net/virtio/virtio_ethdev.c b/drivers/net/virtio/virtio_ethdev.c
> index cb2b2e0bf..5ae7a9650 100644
> --- a/drivers/net/virtio/virtio_ethdev.c
> +++ b/drivers/net/virtio/virtio_ethdev.c
> @@ -1474,6 +1474,8 @@ virtio_init_device(struct rte_eth_dev *eth_dev, uint64_t req_features)
> if (virtio_negotiate_features(hw, req_features) < 0)
> return -1;
>
> + hw->weak_barriers = !vtpci_with_feature(hw, VIRTIO_F_ORDER_PLATFORM);
> +
> if (!hw->virtio_user_dev) {
> pci_dev = RTE_ETH_DEV_TO_PCI(eth_dev);
> rte_eth_copy_pci_info(eth_dev, pci_dev);
> diff --git a/drivers/net/virtio/virtio_ethdev.h b/drivers/net/virtio/virtio_ethdev.h
> index e0f80e5a4..c098e5ac0 100644
> --- a/drivers/net/virtio/virtio_ethdev.h
> +++ b/drivers/net/virtio/virtio_ethdev.h
> @@ -34,7 +34,8 @@
> 1u << VIRTIO_RING_F_INDIRECT_DESC | \
> 1ULL << VIRTIO_F_VERSION_1 | \
> 1ULL << VIRTIO_F_IN_ORDER | \
> - 1ULL << VIRTIO_F_IOMMU_PLATFORM)
> + 1ULL << VIRTIO_F_IOMMU_PLATFORM | \
> + 1ULL << VIRTIO_F_ORDER_PLATFORM)
>
> #define VIRTIO_PMD_SUPPORTED_GUEST_FEATURES \
> (VIRTIO_PMD_DEFAULT_GUEST_FEATURES | \
> diff --git a/drivers/net/virtio/virtio_pci.h b/drivers/net/virtio/virtio_pci.h
> index e961a58ca..e2f096185 100644
> --- a/drivers/net/virtio/virtio_pci.h
> +++ b/drivers/net/virtio/virtio_pci.h
> @@ -128,6 +128,12 @@ struct virtnet_ctl;
> */
> #define VIRTIO_F_IN_ORDER 35
>
> +/*
> + * This feature indicates that memory accesses by the driver and the device
> + * are ordered in a way described by the platform.
> + */
> +#define VIRTIO_F_ORDER_PLATFORM 36
> +
> /* The Guest publishes the used index for which it expects an interrupt
> * at the end of the avail ring. Host should ignore the avail->flags field. */
> /* The Host publishes the avail index for which it expects a kick
> @@ -240,6 +246,7 @@ struct virtio_hw {
> uint8_t use_simple_rx;
> uint8_t use_inorder_rx;
> uint8_t use_inorder_tx;
> + uint8_t weak_barriers;
> bool has_tx_offload;
> bool has_rx_offload;
> uint16_t port_id;
> diff --git a/drivers/net/virtio/virtio_rxtx.c b/drivers/net/virtio/virtio_rxtx.c
> index cb8f89f18..66195bf47 100644
> --- a/drivers/net/virtio/virtio_rxtx.c
> +++ b/drivers/net/virtio/virtio_rxtx.c
> @@ -906,7 +906,7 @@ virtio_recv_pkts(void *rx_queue, struct rte_mbuf **rx_pkts, uint16_t nb_pkts)
>
> nb_used = VIRTQUEUE_NUSED(vq);
>
> - virtio_rmb();
> + virtio_rmb(hw->weak_barriers);
>
> num = likely(nb_used <= nb_pkts) ? nb_used : nb_pkts;
> if (unlikely(num > VIRTIO_MBUF_BURST_SZ))
> @@ -1017,7 +1017,7 @@ virtio_recv_mergeable_pkts_inorder(void *rx_queue,
> nb_used = RTE_MIN(nb_used, nb_pkts);
> nb_used = RTE_MIN(nb_used, VIRTIO_MBUF_BURST_SZ);
>
> - virtio_rmb();
> + virtio_rmb(hw->weak_barriers);
>
> PMD_RX_LOG(DEBUG, "used:%d", nb_used);
>
> @@ -1202,7 +1202,7 @@ virtio_recv_mergeable_pkts(void *rx_queue,
>
> nb_used = VIRTQUEUE_NUSED(vq);
>
> - virtio_rmb();
> + virtio_rmb(hw->weak_barriers);
>
> PMD_RX_LOG(DEBUG, "used:%d", nb_used);
>
> @@ -1365,7 +1365,7 @@ virtio_xmit_pkts(void *tx_queue, struct rte_mbuf **tx_pkts, uint16_t nb_pkts)
> PMD_TX_LOG(DEBUG, "%d packets to xmit", nb_pkts);
> nb_used = VIRTQUEUE_NUSED(vq);
>
> - virtio_rmb();
> + virtio_rmb(hw->weak_barriers);
> if (likely(nb_used > vq->vq_nentries - vq->vq_free_thresh))
> virtio_xmit_cleanup(vq, nb_used);
>
> @@ -1407,7 +1407,7 @@ virtio_xmit_pkts(void *tx_queue, struct rte_mbuf **tx_pkts, uint16_t nb_pkts)
> /* Positive value indicates it need free vring descriptors */
> if (unlikely(need > 0)) {
> nb_used = VIRTQUEUE_NUSED(vq);
> - virtio_rmb();
> + virtio_rmb(hw->weak_barriers);
> need = RTE_MIN(need, (int)nb_used);
>
> virtio_xmit_cleanup(vq, need);
> @@ -1463,7 +1463,7 @@ virtio_xmit_pkts_inorder(void *tx_queue,
> PMD_TX_LOG(DEBUG, "%d packets to xmit", nb_pkts);
> nb_used = VIRTQUEUE_NUSED(vq);
>
> - virtio_rmb();
> + virtio_rmb(hw->weak_barriers);
> if (likely(nb_used > vq->vq_nentries - vq->vq_free_thresh))
> virtio_xmit_cleanup_inorder(vq, nb_used);
>
> @@ -1511,7 +1511,7 @@ virtio_xmit_pkts_inorder(void *tx_queue,
> need = slots - vq->vq_free_cnt;
> if (unlikely(need > 0)) {
> nb_used = VIRTQUEUE_NUSED(vq);
> - virtio_rmb();
> + virtio_rmb(hw->weak_barriers);
> need = RTE_MIN(need, (int)nb_used);
>
> virtio_xmit_cleanup_inorder(vq, need);
> diff --git a/drivers/net/virtio/virtqueue.h b/drivers/net/virtio/virtqueue.h
> index 26518ed98..6b9055a1f 100644
> --- a/drivers/net/virtio/virtqueue.h
> +++ b/drivers/net/virtio/virtqueue.h
> @@ -19,15 +19,40 @@
> struct rte_mbuf;
>
> /*
> - * Per virtio_config.h in Linux.
> + * Per virtio_ring.h in Linux.
> * For virtio_pci on SMP, we don't need to order with respect to MMIO
> * accesses through relaxed memory I/O windows, so smp_mb() et al are
> * sufficient.
> *
> + * For using virtio to talk to real devices (eg. vDPA) we do need real
> + * barriers.
> */
> -#define virtio_mb() rte_smp_mb()
> -#define virtio_rmb() rte_smp_rmb()
> -#define virtio_wmb() rte_smp_wmb()
> +static inline void
> +virtio_mb(uint8_t weak_barriers)
> +{
> + if (weak_barriers)
> + rte_smp_mb();
> + else
> + rte_mb();
> +}
> +
Why doesn't rte_cio_rmb exit?
> +static inline void
> +virtio_rmb(uint8_t weak_barriers)
> +{
> + if (weak_barriers)
> + rte_smp_rmb();
> + else
> + rte_cio_rmb();
> +}
> +
> +static inline void
> +virtio_wmb(uint8_t weak_barriers)
> +{
> + if (weak_barriers)
> + rte_smp_wmb();
> + else
> + rte_cio_wmb();
> +}
>
> #ifdef RTE_PMD_PACKET_PREFETCH
> #define rte_packet_prefetch(p) rte_prefetch1(p)
> @@ -312,7 +337,7 @@ void vq_ring_free_inorder(struct virtqueue *vq, uint16_t desc_idx,
> static inline void
> vq_update_avail_idx(struct virtqueue *vq)
> {
> - virtio_wmb();
> + virtio_wmb(vq->hw->weak_barriers);
> vq->vq_ring.avail->idx = vq->vq_avail_idx;
> }
>
> --
> 2.17.1
next prev parent reply other threads:[~2018-12-14 17:00 UTC|newest]
Thread overview: 28+ messages / expand[flat|nested] mbox.gz Atom feed top
[not found] <CGME20181214153817eucas1p19a41cdd791879252e1f3a5d77c427845@eucas1p1.samsung.com>
2018-12-14 15:38 ` [PATCH] net/virtio: add platform memory ordering feature support Ilya Maximets
2018-12-14 17:00 ` Michael S. Tsirkin [this message]
2018-12-14 17:23 ` Ilya Maximets
2018-12-26 16:37 ` [PATCH v2] " Ilya Maximets
2018-12-27 10:07 ` Shahaf Shuler
2019-01-09 14:34 ` Ilya Maximets
2019-01-09 15:50 ` Michael S. Tsirkin
2019-01-10 20:36 ` Shahaf Shuler
2019-01-15 6:33 ` Shahaf Shuler
2019-01-15 8:29 ` Ilya Maximets
2019-01-15 8:55 ` Shahaf Shuler
2019-01-15 10:23 ` Ilya Maximets
2019-02-12 17:50 ` Michael S. Tsirkin
2019-01-09 14:50 ` [PATCH v3 0/3] Missing barriers and VIRTIO_F_ORDER_PLATFORM Ilya Maximets
2019-01-09 14:50 ` [PATCH v3 1/3] net/virtio: add missing barrier before reading the flags Ilya Maximets
2019-01-10 14:31 ` Maxime Coquelin
2019-01-09 14:50 ` [PATCH v3 2/3] net/virtio: update memory ordering comment for vq notify Ilya Maximets
2019-01-10 8:19 ` Gavin Hu (Arm Technology China)
2019-01-10 9:18 ` Maxime Coquelin
2019-01-10 9:55 ` Ilya Maximets
2019-01-10 14:56 ` Michael S. Tsirkin
2019-01-10 14:31 ` Maxime Coquelin
2019-01-09 14:50 ` [PATCH v3 3/3] net/virtio: add platform memory ordering feature support Ilya Maximets
2019-01-10 14:31 ` Maxime Coquelin
2019-01-09 14:55 ` [PATCH v3 0/3] Missing barriers and VIRTIO_F_ORDER_PLATFORM Michael S. Tsirkin
2019-01-09 15:24 ` Ilya Maximets
2019-01-09 16:53 ` Ferruh Yigit
2019-01-10 15:19 ` Maxime Coquelin
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20181214120038-mutt-send-email-mst@kernel.org \
--to=mst@redhat.com \
--cc=alejandro.lucero@netronome.com \
--cc=dev@dpdk.org \
--cc=i.maximets@samsung.com \
--cc=jasowang@redhat.com \
--cc=jfreimann@redhat.com \
--cc=maxime.coquelin@redhat.com \
--cc=tiwei.bie@intel.com \
--cc=xiao.w.wang@intel.com \
--cc=xiaolong.ye@intel.com \
--cc=zhihong.wang@intel.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.