[PATCH net 0/2] ionic: fix completion descriptor access

[PATCH net 0/2] ionic: fix completion descriptor access

[Eric Joyner]

The first patch adds the missing dma_rmb() between the completion
publish checks and the reads of the descriptor payloads, suggested by
the Sashiko review from [1] for the second patch.

The second locates the completion within its slot from cq->desc_size
rather than a fixed stride, which is necessary when IONIC_Q_F_2X_CQ_DESC
(double-sized completion descriptors) is used (e.g. for hardware
timestamps).

[1] https://lore.kernel.org/netdev/20260507155928.2537928-1-kuba@kernel.org/

Eric Joyner (1):
  ionic: add missing dma_rmb() after the completion publish check

Prabu Thayalan (1):
  ionic: fix completion descriptor access with 2x desc size

 .../net/ethernet/pensando/ionic/ionic_main.c  |  4 +++
 .../net/ethernet/pensando/ionic/ionic_txrx.c  | 31 +++++++++++--------
 2 files changed, 22 insertions(+), 13 deletions(-)


base-commit: 53658c6f3682967a5e76ed4bc7462c4bdcddaec3
-- 
2.43.0

[PATCH net 1/2] ionic: add missing dma_rmb() after the completion publish check

[Eric Joyner]

Each completion service routine tests a device-written publish flag and
then reads the rest of the descriptor with nothing ordering those loads.
A control dependency does not order loads, so a weakly ordered CPU may
satisfy the payload reads from a cache line state observed before the
flag became valid. Add the barrier to all four completion paths.

Fixes: 1d062b7b6f64 ("ionic: Add basic adminq support")
Fixes: 0f3154e6bcb3 ("ionic: Add Tx and Rx handling")
Fixes: 77ceb68e29cc ("ionic: Add notifyq support")
Assisted-by: Claude:claude-opus-5
Signed-off-by: Eric Joyner <eric.joyner@amd.com>
Reviewed-by: Brett Creeley <brett.creeley@amd.com>
---
 drivers/net/ethernet/pensando/ionic/ionic_main.c | 4 ++++
 drivers/net/ethernet/pensando/ionic/ionic_txrx.c | 4 ++++
 2 files changed, 8 insertions(+)

diff --git a/drivers/net/ethernet/pensando/ionic/ionic_main.c b/drivers/net/ethernet/pensando/ionic/ionic_main.c
index 6e6f3ed07271..10501be9ef95 100644
--- a/drivers/net/ethernet/pensando/ionic/ionic_main.c
+++ b/drivers/net/ethernet/pensando/ionic/ionic_main.c
@@ -269,6 +269,8 @@ bool ionic_notifyq_service(struct ionic_cq *cq)
 	if ((s64)(eid - lif->last_eid) <= 0)
 		return false;
 
+	dma_rmb();
+
 	lif->last_eid = eid;
 
 	dev_dbg(lif->ionic->dev, "notifyq event:\n");
@@ -314,6 +316,8 @@ bool ionic_adminq_service(struct ionic_cq *cq)
 	if (!color_match(comp->color, cq->done_color))
 		return false;
 
+	dma_rmb();
+
 	/* check for empty queue */
 	if (q->tail_idx == q->head_idx)
 		return false;
diff --git a/drivers/net/ethernet/pensando/ionic/ionic_txrx.c b/drivers/net/ethernet/pensando/ionic/ionic_txrx.c
index 301ebee2fdc5..5b58460350be 100644
--- a/drivers/net/ethernet/pensando/ionic/ionic_txrx.c
+++ b/drivers/net/ethernet/pensando/ionic/ionic_txrx.c
@@ -734,6 +734,8 @@ static bool __ionic_rx_service(struct ionic_cq *cq, struct bpf_prog *xdp_prog)
 	if (!color_match(comp->pkt_type_color, cq->done_color))
 		return false;
 
+	dma_rmb();
+
 	/* check for empty queue */
 	if (q->tail_idx == q->head_idx)
 		return false;
@@ -1249,6 +1251,8 @@ static bool ionic_tx_service(struct ionic_cq *cq,
 	if (!color_match(comp->color, cq->done_color))
 		return false;
 
+	dma_rmb();
+
 	/* clean the related q entries, there could be
 	 * several q entries completed for each cq completion
 	 */
-- 
2.43.0

[PATCH net 2/2] ionic: fix completion descriptor access with 2x desc size

[Eric Joyner]

From: Prabu Thayalan <prabu.ponrajthayalan@amd.com>

The old ionic_rx_service() and ionic_tx_service() used array
indexing to access completion descriptors:

    comp = &((struct ionic_rxq_comp *)cq->base)[cq->tail_idx];

This assumes the stride is sizeof(struct ionic_rxq_comp) = 16 bytes.
However, when the IONIC_Q_F_2X_CQ_DESC flag is set, the actual
completion descriptor size is 32 bytes (2 * sizeof(comp)), and the
completion itself is located at the end of that 32-byte slot. Array
indexing with a 16-byte stride would access the wrong offset.

Use pointer arithmetic that accounts for the actual descriptor size
from cq->desc_size:

    comp = cq->base +
           cq->desc_size * cq->tail_idx +
           cq->desc_size - sizeof(*comp);

This correctly calculates the completion location regardless of
descriptor size. For the common case where desc_size equals
sizeof(*comp), use array indexing in a likely() fast path to avoid
performance regression.

Fixes: 65e548f6b0ff ("ionic: remove the cq_info to save more memory")
Signed-off-by: Prabu Thayalan <prabu.ponrajthayalan@amd.com>
Signed-off-by: Eric Joyner <eric.joyner@amd.com>
Reviewed-by: Brett Creeley <brett.creeley@amd.com>
---
 .../net/ethernet/pensando/ionic/ionic_txrx.c  | 27 ++++++++++---------
 1 file changed, 14 insertions(+), 13 deletions(-)

diff --git a/drivers/net/ethernet/pensando/ionic/ionic_txrx.c b/drivers/net/ethernet/pensando/ionic/ionic_txrx.c
index 5b58460350be..05938689248d 100644
--- a/drivers/net/ethernet/pensando/ionic/ionic_txrx.c
+++ b/drivers/net/ethernet/pensando/ionic/ionic_txrx.c
@@ -701,11 +701,7 @@ static void ionic_rx_clean(struct ionic_queue *q,
 		__le64 *cq_desc_hwstamp;
 		u64 hwstamp;
 
-		cq_desc_hwstamp =
-			(void *)comp +
-			qcq->cq.desc_size -
-			sizeof(struct ionic_rxq_comp) -
-			IONIC_HWSTAMP_CQ_NEGOFFSET;
+		cq_desc_hwstamp = (void *)comp - IONIC_HWSTAMP_CQ_NEGOFFSET;
 
 		hwstamp = le64_to_cpu(*cq_desc_hwstamp);
 
@@ -729,7 +725,12 @@ static bool __ionic_rx_service(struct ionic_cq *cq, struct bpf_prog *xdp_prog)
 	struct ionic_queue *q = cq->bound_q;
 	struct ionic_rxq_comp *comp;
 
-	comp = &((struct ionic_rxq_comp *)cq->base)[cq->tail_idx];
+	if (likely(cq->desc_size == sizeof(*comp)))
+		comp = &((struct ionic_rxq_comp *)cq->base)[cq->tail_idx];
+	else
+		comp = cq->base +
+		       cq->desc_size * cq->tail_idx +
+		       cq->desc_size - sizeof(*comp);
 
 	if (!color_match(comp->pkt_type_color, cq->done_color))
 		return false;
@@ -1182,7 +1183,6 @@ static void ionic_tx_clean(struct ionic_queue *q,
 			   bool in_napi)
 {
 	struct ionic_tx_stats *stats = q_to_tx_stats(q);
-	struct ionic_qcq *qcq = q_to_qcq(q);
 	struct sk_buff *skb;
 
 	if (desc_info->xdpf) {
@@ -1207,11 +1207,7 @@ static void ionic_tx_clean(struct ionic_queue *q,
 			__le64 *cq_desc_hwstamp;
 			u64 hwstamp;
 
-			cq_desc_hwstamp =
-				(void *)comp +
-				qcq->cq.desc_size -
-				sizeof(struct ionic_txq_comp) -
-				IONIC_HWSTAMP_CQ_NEGOFFSET;
+			cq_desc_hwstamp = (void *)comp - IONIC_HWSTAMP_CQ_NEGOFFSET;
 
 			hwstamp = le64_to_cpu(*cq_desc_hwstamp);
 
@@ -1246,7 +1242,12 @@ static bool ionic_tx_service(struct ionic_cq *cq,
 	unsigned int pkts = 0;
 	u16 index;
 
-	comp = &((struct ionic_txq_comp *)cq->base)[cq->tail_idx];
+	if (likely(cq->desc_size == sizeof(*comp)))
+		comp = &((struct ionic_txq_comp *)cq->base)[cq->tail_idx];
+	else
+		comp = cq->base +
+		       cq->desc_size * cq->tail_idx +
+		       cq->desc_size - sizeof(*comp);
 
 	if (!color_match(comp->color, cq->done_color))
 		return false;
-- 
2.43.0