[PATCH 12/13] net/ena/base: remove x86 Tx LLQ write barrier

[PATCH 12/13] net/ena/base: remove x86 Tx LLQ write barrier

[Shai Brandes]

ena_com_write_bounce_buffer_to_dev() issues a wmb() before copying each
LLQ entry from the bounce buffer to the write-combining (WC) LLQ memory
region. It was intended to ensure the bounce buffer is fully written
before the copy reads it. That is never needed: a CPU always observes
its own prior stores, on every architecture. wmb() also orders stores
against later stores, not against later loads, so it could not provide
that guarantee anyway.

The barrier is needed on ARM for a different reason: it orders one LLQ
entry's stores to the device ahead of the next entry's. The copy uses
relaxed stores, which on ARM may reach the device partially and out of
order while the burst is still being written. The barrier in the
doorbell write cannot prevent this, as it only orders the entries
against the doorbell, not against each other. With the barrier removed,
c8gn at 32 queues fails with IPP_REASS_ERROR_PRE_VALID_DB ("Data Out of
Window") before the doorbell is written.

On x86 the barrier is redundant. WC stores are weakly ordered there
too, so it never ordered entries against each other. The entries reach
the device before the doorbell because of the memory type, not a fence:
each entry is written as complete 64-byte WC lines, and on Intel the UC
doorbell write flushes pending WC buffers, while on AMD each 64-byte WC
line is flushed once it is fully written. The per-entry SFENCE made the
CPU wait for the previous entry to reach the device before copying the
next one, defeating the purpose of write combining.

Introduce arm_wmb(), a write barrier on ARM and a no-op elsewhere, and
use it in place of wmb() before the bounce buffer copy.

Signed-off-by: Amit Bernstein <amitbern@amazon.com>
Signed-off-by: Shai Brandes <shaibran@amazon.com>
---
 doc/guides/rel_notes/release_26_11.rst |  2 ++
 drivers/net/ena/base/ena_eth_com.c     | 23 ++++++++++++++++++++---
 drivers/net/ena/base/ena_plat_dpdk.h   |  8 ++++++++
 3 files changed, 30 insertions(+), 3 deletions(-)

diff --git a/doc/guides/rel_notes/release_26_11.rst b/doc/guides/rel_notes/release_26_11.rst
index 05281c3d11..c6c8719d2b 100644
--- a/doc/guides/rel_notes/release_26_11.rst
+++ b/doc/guides/rel_notes/release_26_11.rst
@@ -95,6 +95,8 @@ New Features
 * **Updated Amazon ENA (Elastic Network Adapter) ethernet driver.**
 
   * Added Rx flow steering support using the generic flow API.
+  * Restrict the Tx LLQ write barrier to ARM, where it is required for correct entry ordering,
+    and remove it on x86, where it is redundant and limits throughput.
 
 * **Updated Intel iavf driver.**
 
diff --git a/drivers/net/ena/base/ena_eth_com.c b/drivers/net/ena/base/ena_eth_com.c
index ea0fb374db..5dbb432acc 100644
--- a/drivers/net/ena/base/ena_eth_com.c
+++ b/drivers/net/ena/base/ena_eth_com.c
@@ -132,10 +132,27 @@ static int ena_com_write_bounce_buffer_to_dev(struct ena_com_io_sq *io_sq,
 			    io_sq->qid, io_sq->llq_runtime.entries_in_tx_burst_left);
 	}
 
-	/* Make sure everything was written into the bounce buffer before
-	 * writing the bounce buffer to the device
+	/* Ensure the previous LLQ entry reaches the device before this one.
+	 *
+	 * The original wmb() was intended to ensure the bounce buffer is
+	 * fully written before it is copied to the device. This is not
+	 * required: a CPU always observes its own prior stores, on every
+	 * architecture. Moreover, wmb() orders stores against later stores,
+	 * not against later loads, so it could not provide this guarantee.
+	 *
+	 * The copy below uses relaxed stores to the WC LLQ region: plain
+	 * stores with no barrier, which the CPU may send to the device in any
+	 * order. On ARM, this means an entry may reach the device partially
+	 * and out of order before the doorbell is written. In addition, the
+	 * barrier in the doorbell write cannot prevent this, as it only orders
+	 * the entries against the doorbell, not against each other.
+	 *
+	 * On x86, this barrier is not needed. Each entry is written as complete
+	 * 64-byte WC lines, all of which reach the device before the
+	 * doorbell: on Intel, the UC doorbell write flushes pending WC
+	 * buffers; on AMD, each line is flushed once it is fully written.
 	 */
-	wmb();
+	arm_wmb();
 
 	/* The line is completed. Copy it to dev */
 	ENA_MEMCPY_TO_DEVICE_64(io_sq->desc_addr.pbuf_dev_addr + dst_offset,
diff --git a/drivers/net/ena/base/ena_plat_dpdk.h b/drivers/net/ena/base/ena_plat_dpdk.h
index 4a6d53d0a2..12f1643a85 100644
--- a/drivers/net/ena/base/ena_plat_dpdk.h
+++ b/drivers/net/ena/base/ena_plat_dpdk.h
@@ -73,6 +73,14 @@ typedef RTE_ATOMIC(int32_t) ena_atomic32_t;
 #define rmb rte_rmb
 #define mb rte_mb
 #define mmiowb rte_io_wmb
+
+/* Write memory barrier for ARM architectures - covers 32 and 64 bit ARM */
+#if defined(RTE_ARCH_ARM)
+#define arm_wmb rte_wmb
+#else
+static inline void arm_wmb(void) { }
+#endif
+
 #define __iomem
 
 #ifndef READ_ONCE
-- 
2.17.1