[PATCH net v4 0/2] net: airoha: Fix airoha_qdma_cleanup_tx_queue() processing

[PATCH net v4 0/2] net: airoha: Fix airoha_qdma_cleanup_tx_queue() processing

[Lorenzo Bianconi]

Add missing bits in airoha_qdma_cleanup_tx_queue routine.
Fix airoha_qdma_cleanup_tx_queue processing errors intorduced in commit
'3f47e67dff1f7 ("net: airoha: Add the capability to consume out-of-order
DMA tx descriptors")'.

---
Changes in v4:
- Drop patch 2/3 to move entries to queue head in case of DMA mapping
  failure in airoha_dev_xmit().
- Link to v3: https://lore.kernel.org/r/20260416-airoha_qdma_cleanup_tx_queue-fix-net-v3-0-2b69f5788580@kernel.org

Changes in v3:
- Move ndesc initialization fix in a dedicated patch.
- Add patch 2/3 to move entries to queue head in case of DMA mapping
  failure in airoha_dev_xmit().
- Cosmetics.
- Link to v2: https://lore.kernel.org/r/20260414-airoha_qdma_cleanup_tx_queue-fix-net-v2-1-875de57cc022@kernel.org

Changes in v2:
- Move q->ndesc initialization at end of airoha_qdma_init_tx routine in
  order to avoid any possible NULL pointer dereference in
  airoha_qdma_cleanup_tx_queue()
- Check if q->tx_list is empty in airoha_qdma_cleanup_tx_queue()
- Link to v1: https://lore.kernel.org/r/20260410-airoha_qdma_cleanup_tx_queue-fix-net-v1-1-b7171c8f1e78@kernel.org

---
Lorenzo Bianconi (2):
      net: airoha: Move ndesc initialization at end of airoha_qdma_init_tx()
      net: airoha: Add missing bits in airoha_qdma_cleanup_tx_queue()

 drivers/net/ethernet/airoha/airoha_eth.c | 40 +++++++++++++++++++++++++++-----
 1 file changed, 34 insertions(+), 6 deletions(-)
---
base-commit: 82c21069028c5db3463f851ae8ac9cc2e38a3827
change-id: 20260410-airoha_qdma_cleanup_tx_queue-fix-net-93375f5ee80f

Best regards,
-- 
Lorenzo Bianconi <lorenzo@kernel.org>

[PATCH net v4 1/2] net: airoha: Move ndesc initialization at end of airoha_qdma_init_tx()

[Lorenzo Bianconi]

If queue entry list allocation fails in airoha_qdma_init_tx_queue routine,
airoha_qdma_cleanup_tx_queue() will trigger a NULL pointer dereference
accessing the queue entry array. The issue is due to the early ndesc
initialization in airoha_qdma_init_tx_queue(). Fix the issue moving ndesc
initialization at end of airoha_qdma_init_tx routine.

Fixes: 3f47e67dff1f7 ("net: airoha: Add the capability to consume out-of-order DMA tx descriptors")
Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org>
---
 drivers/net/ethernet/airoha/airoha_eth.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/net/ethernet/airoha/airoha_eth.c b/drivers/net/ethernet/airoha/airoha_eth.c
index e1ab15f1ee7d..690bfaf8d7d9 100644
--- a/drivers/net/ethernet/airoha/airoha_eth.c
+++ b/drivers/net/ethernet/airoha/airoha_eth.c
@@ -954,27 +954,27 @@ static int airoha_qdma_init_tx_queue(struct airoha_queue *q,
 	dma_addr_t dma_addr;
 
 	spin_lock_init(&q->lock);
-	q->ndesc = size;
 	q->qdma = qdma;
 	q->free_thr = 1 + MAX_SKB_FRAGS;
 	INIT_LIST_HEAD(&q->tx_list);
 
-	q->entry = devm_kzalloc(eth->dev, q->ndesc * sizeof(*q->entry),
+	q->entry = devm_kzalloc(eth->dev, size * sizeof(*q->entry),
 				GFP_KERNEL);
 	if (!q->entry)
 		return -ENOMEM;
 
-	q->desc = dmam_alloc_coherent(eth->dev, q->ndesc * sizeof(*q->desc),
+	q->desc = dmam_alloc_coherent(eth->dev, size * sizeof(*q->desc),
 				      &dma_addr, GFP_KERNEL);
 	if (!q->desc)
 		return -ENOMEM;
 
-	for (i = 0; i < q->ndesc; i++) {
+	for (i = 0; i < size; i++) {
 		u32 val = FIELD_PREP(QDMA_DESC_DONE_MASK, 1);
 
 		list_add_tail(&q->entry[i].list, &q->tx_list);
 		WRITE_ONCE(q->desc[i].ctrl, cpu_to_le32(val));
 	}
+	q->ndesc = size;
 
 	/* xmit ring drop default setting */
 	airoha_qdma_set(qdma, REG_TX_RING_BLOCKING(qid),

-- 
2.53.0

[PATCH net v4 2/2] net: airoha: Add missing bits in airoha_qdma_cleanup_tx_queue()

[Lorenzo Bianconi]

Similar to airoha_qdma_cleanup_rx_queue(), reset DMA TX descriptors in
airoha_qdma_cleanup_tx_queue routine. Moreover, reset TX_DMA_IDX to
TX_CPU_IDX to notify the NIC the QDMA TX ring is empty.

Fixes: 23020f0493270 ("net: airoha: Introduce ethernet support for EN7581 SoC")
Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org>
---
 drivers/net/ethernet/airoha/airoha_eth.c | 32 ++++++++++++++++++++++++++++++--
 1 file changed, 30 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/airoha/airoha_eth.c b/drivers/net/ethernet/airoha/airoha_eth.c
index 690bfaf8d7d9..6d9f82c677a0 100644
--- a/drivers/net/ethernet/airoha/airoha_eth.c
+++ b/drivers/net/ethernet/airoha/airoha_eth.c
@@ -1039,12 +1039,15 @@ static int airoha_qdma_init_tx(struct airoha_qdma *qdma)
 
 static void airoha_qdma_cleanup_tx_queue(struct airoha_queue *q)
 {
-	struct airoha_eth *eth = q->qdma->eth;
-	int i;
+	struct airoha_qdma *qdma = q->qdma;
+	struct airoha_eth *eth = qdma->eth;
+	int i, qid = q - &qdma->q_tx[0];
+	u16 index = 0;
 
 	spin_lock_bh(&q->lock);
 	for (i = 0; i < q->ndesc; i++) {
 		struct airoha_queue_entry *e = &q->entry[i];
+		struct airoha_qdma_desc *desc = &q->desc[i];
 
 		if (!e->dma_addr)
 			continue;
@@ -1055,8 +1058,33 @@ static void airoha_qdma_cleanup_tx_queue(struct airoha_queue *q)
 		e->dma_addr = 0;
 		e->skb = NULL;
 		list_add_tail(&e->list, &q->tx_list);
+
+		/* Reset DMA descriptor */
+		WRITE_ONCE(desc->ctrl, 0);
+		WRITE_ONCE(desc->addr, 0);
+		WRITE_ONCE(desc->data, 0);
+		WRITE_ONCE(desc->msg0, 0);
+		WRITE_ONCE(desc->msg1, 0);
+		WRITE_ONCE(desc->msg2, 0);
+
 		q->queued--;
 	}
+
+	if (!list_empty(&q->tx_list)) {
+		struct airoha_queue_entry *e;
+
+		e = list_first_entry(&q->tx_list, struct airoha_queue_entry,
+				     list);
+		index = e - q->entry;
+	}
+	/* Set TX_DMA_IDX to TX_CPU_IDX to notify the hw the QDMA TX ring is
+	 * empty.
+	 */
+	airoha_qdma_rmw(qdma, REG_TX_CPU_IDX(qid), TX_RING_CPU_IDX_MASK,
+			FIELD_PREP(TX_RING_CPU_IDX_MASK, index));
+	airoha_qdma_rmw(qdma, REG_TX_DMA_IDX(qid), TX_RING_DMA_IDX_MASK,
+			FIELD_PREP(TX_RING_DMA_IDX_MASK, index));
+
 	spin_unlock_bh(&q->lock);
 }
 

-- 
2.53.0

Re: [PATCH net v4 0/2] net: airoha: Fix airoha_qdma_cleanup_tx_queue() processing

[Simon Horman]

On Fri, Apr 17, 2026 at 08:36:30AM +0200, Lorenzo Bianconi wrote:
> Add missing bits in airoha_qdma_cleanup_tx_queue routine.
> Fix airoha_qdma_cleanup_tx_queue processing errors intorduced in commit
> '3f47e67dff1f7 ("net: airoha: Add the capability to consume out-of-order
> DMA tx descriptors")'.
> 
> ---
> Changes in v4:
> - Drop patch 2/3 to move entries to queue head in case of DMA mapping
>   failure in airoha_dev_xmit().
> - Link to v3: https://lore.kernel.org/r/20260416-airoha_qdma_cleanup_tx_queue-fix-net-v3-0-2b69f5788580@kernel.org
> 
> Changes in v3:
> - Move ndesc initialization fix in a dedicated patch.
> - Add patch 2/3 to move entries to queue head in case of DMA mapping
>   failure in airoha_dev_xmit().
> - Cosmetics.
> - Link to v2: https://lore.kernel.org/r/20260414-airoha_qdma_cleanup_tx_queue-fix-net-v2-1-875de57cc022@kernel.org
> 
> Changes in v2:
> - Move q->ndesc initialization at end of airoha_qdma_init_tx routine in
>   order to avoid any possible NULL pointer dereference in
>   airoha_qdma_cleanup_tx_queue()
> - Check if q->tx_list is empty in airoha_qdma_cleanup_tx_queue()
> - Link to v1: https://lore.kernel.org/r/20260410-airoha_qdma_cleanup_tx_queue-fix-net-v1-1-b7171c8f1e78@kernel.org
> 
> ---
> Lorenzo Bianconi (2):
>       net: airoha: Move ndesc initialization at end of airoha_qdma_init_tx()
>       net: airoha: Add missing bits in airoha_qdma_cleanup_tx_queue()
> 
>  drivers/net/ethernet/airoha/airoha_eth.c | 40 +++++++++++++++++++++++++++-----
>  1 file changed, 34 insertions(+), 6 deletions(-)

Thanks for the updates, this series looks good to me:

Reviewed-by: Simon Horman <horms@kernel.org>


As is the way of things, Sashiko has some feedback on this patchset.
Please consider looking over it for potential follow-up changes.
I do not think that needs to block progress of this patchset.

Re: [PATCH net v4 2/2] net: airoha: Add missing bits in airoha_qdma_cleanup_tx_queue()

[Paolo Abeni]

On 4/17/26 8:36 AM, Lorenzo Bianconi wrote:
> @@ -1055,8 +1058,33 @@ static void airoha_qdma_cleanup_tx_queue(struct airoha_queue *q)
>  		e->dma_addr = 0;
>  		e->skb = NULL;
>  		list_add_tail(&e->list, &q->tx_list);
> +
> +		/* Reset DMA descriptor */
> +		WRITE_ONCE(desc->ctrl, 0);
> +		WRITE_ONCE(desc->addr, 0);
> +		WRITE_ONCE(desc->data, 0);
> +		WRITE_ONCE(desc->msg0, 0);
> +		WRITE_ONCE(desc->msg1, 0);
> +		WRITE_ONCE(desc->msg2, 0);

Sashiko has some complains on this patch that look legit to me.

Also the pre-existing issues mentioned WRT patch 1/2 makes such patch
IMHO almost ineffective, I think you should address them in the same series.

Note that you should have commented on sashiko review on the ML, it
would have saved a significant amount of time on us.

/P

Re: [PATCH net v4 2/2] net: airoha: Add missing bits in airoha_qdma_cleanup_tx_queue()

[Lorenzo Bianconi]

[-- Attachment #1: Type: text/plain, Size: 1670 bytes --]

On Apr 21, Paolo Abeni wrote:
> On 4/17/26 8:36 AM, Lorenzo Bianconi wrote:
> > @@ -1055,8 +1058,33 @@ static void airoha_qdma_cleanup_tx_queue(struct airoha_queue *q)
> >  		e->dma_addr = 0;
> >  		e->skb = NULL;
> >  		list_add_tail(&e->list, &q->tx_list);
> > +
> > +		/* Reset DMA descriptor */
> > +		WRITE_ONCE(desc->ctrl, 0);
> > +		WRITE_ONCE(desc->addr, 0);
> > +		WRITE_ONCE(desc->data, 0);
> > +		WRITE_ONCE(desc->msg0, 0);
> > +		WRITE_ONCE(desc->msg1, 0);
> > +		WRITE_ONCE(desc->msg2, 0);
> 
> Sashiko has some complains on this patch that look legit to me.

Hi Paolo,

I looked at the issue reported by Sashiko for patch 2/2 and I will send
a dedicated patch to address it but I guess this problem is not strictly
related to this patch since we already have the reported issue upstream
even without this patch (please consider dma_unmap_single() in
airoha_qdma_cleanup_tx_queue()).
Moreover, in this patch I want to just add missing bits in
airoha_qdma_cleanup_tx_queue().

> 
> Also the pre-existing issues mentioned WRT patch 1/2 makes such patch
> IMHO almost ineffective, I think you should address them in the same series.

The issue reported by Sashiko for patch 1/2 are already addressed in the
following upstream series:
https://patchwork.kernel.org/project/netdevbpf/cover/20260420-airoha_qdma_init_rx_queue-fix-v2-0-d99347e5c18d@kernel.org/

> 
> Note that you should have commented on sashiko review on the ML, it
> would have saved a significant amount of time on us.

I guess the main issue here is Sashiko is not currently sending the feedbacks
on the ML, right?

Regards,
Lorenzo

> 
> /P
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH net v4 2/2] net: airoha: Add missing bits in airoha_qdma_cleanup_tx_queue()

[Lorenzo Bianconi]

[-- Attachment #1: Type: text/plain, Size: 1904 bytes --]

> On 4/17/26 8:36 AM, Lorenzo Bianconi wrote:
> > @@ -1055,8 +1058,33 @@ static void airoha_qdma_cleanup_tx_queue(struct airoha_queue *q)
> >  		e->dma_addr = 0;
> >  		e->skb = NULL;
> >  		list_add_tail(&e->list, &q->tx_list);
> > +
> > +		/* Reset DMA descriptor */
> > +		WRITE_ONCE(desc->ctrl, 0);
> > +		WRITE_ONCE(desc->addr, 0);
> > +		WRITE_ONCE(desc->data, 0);
> > +		WRITE_ONCE(desc->msg0, 0);
> > +		WRITE_ONCE(desc->msg1, 0);
> > +		WRITE_ONCE(desc->msg2, 0);
> 
> Sashiko has some complains on this patch that look legit to me.
> 
> Also the pre-existing issues mentioned WRT patch 1/2 makes such patch
> IMHO almost ineffective, I think you should address them in the same series.
> 
> Note that you should have commented on sashiko review on the ML, it
> would have saved a significant amount of time on us.

Since this series is marked as 'Changes Requested', it is not clear to me what
next steps are. I guess we have two possible approach here:

1) - Post patch 1/2 ("net: airoha: Move ndesc initialization at
     end of airoha_qdma_init_tx()") with the series available upstream
     (not merged yet) in [0] where I am fixing similar issues for
     airoha_qdma_init_rx_queue() and airoha_qdma_tx_irq_init().
   - Post patch 2/2 ("net: airoha: Add missing bits in
     airoha_qdma_cleanup_tx_queue()") with a fix for airoha_ndo_stop() waiting
     for TX/RX DMA engine to complete before running
     airoha_qdma_cleanup_tx_queue().

2) - Since all the issues rised by Sashiko are not strictly related to this
     series and they are already fixed in pending patches, just apply the fixes
     separately without the needs to repost this series.

Which approach do you prefer?

Regards,
Lorenzo

[0] https://patchwork.kernel.org/project/netdevbpf/cover/20260420-airoha_qdma_init_rx_queue-fix-v2-0-d99347e5c18d@kernel.org/

> 
> /P
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH net v4 2/2] net: airoha: Add missing bits in airoha_qdma_cleanup_tx_queue()

[Paolo Abeni]

On 4/21/26 7:15 PM, Lorenzo Bianconi wrote:
>> On 4/17/26 8:36 AM, Lorenzo Bianconi wrote:
>>> @@ -1055,8 +1058,33 @@ static void airoha_qdma_cleanup_tx_queue(struct airoha_queue *q)
>>>  		e->dma_addr = 0;
>>>  		e->skb = NULL;
>>>  		list_add_tail(&e->list, &q->tx_list);
>>> +
>>> +		/* Reset DMA descriptor */
>>> +		WRITE_ONCE(desc->ctrl, 0);
>>> +		WRITE_ONCE(desc->addr, 0);
>>> +		WRITE_ONCE(desc->data, 0);
>>> +		WRITE_ONCE(desc->msg0, 0);
>>> +		WRITE_ONCE(desc->msg1, 0);
>>> +		WRITE_ONCE(desc->msg2, 0);
>>
>> Sashiko has some complains on this patch that look legit to me.
>>
>> Also the pre-existing issues mentioned WRT patch 1/2 makes such patch
>> IMHO almost ineffective, I think you should address them in the same series.
>>
>> Note that you should have commented on sashiko review on the ML, it
>> would have saved a significant amount of time on us.
> 
> Since this series is marked as 'Changes Requested', it is not clear to me what
> next steps are. I guess we have two possible approach here:
> 
> 1) - Post patch 1/2 ("net: airoha: Move ndesc initialization at
>      end of airoha_qdma_init_tx()") with the series available upstream
>      (not merged yet) in [0] where I am fixing similar issues for
>      airoha_qdma_init_rx_queue() and airoha_qdma_tx_irq_init().
>    - Post patch 2/2 ("net: airoha: Add missing bits in
>      airoha_qdma_cleanup_tx_queue()") with a fix for airoha_ndo_stop() waiting
>      for TX/RX DMA engine to complete before running
>      airoha_qdma_cleanup_tx_queue().
> 
> 2) - Since all the issues rised by Sashiko are not strictly related to this
>      series and they are already fixed in pending patches, just apply the fixes
>      separately without the needs to repost this series.

Given the current flood on the ML I think option 2 could be the better.
Note that the tree is currently in Jakub's hands and he can very legitly
disagree.

/P