[PATCH net] net: airoha: Add missing cleanup bits in airoha_qdma_cleanup_rx_queue()

[PATCH net] net: airoha: Add missing cleanup bits in airoha_qdma_cleanup_rx_queue()

[Lorenzo Bianconi]

In order to properly cleanup hw rx QDMA queues and bring the device to
the initial state, reset rx DMA queue head/tail index. Moreover, reset
queued DMA descriptor fields.

Fixes: 23020f049327 ("net: airoha: Introduce ethernet support for EN7581 SoC")
Tested-by: Madhur Agrawal <Madhur.Agrawal@airoha.com>
Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org>
---
 drivers/net/ethernet/airoha/airoha_eth.c | 18 +++++++++++++++++-
 1 file changed, 17 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/airoha/airoha_eth.c b/drivers/net/ethernet/airoha/airoha_eth.c
index 56cf9a926a83da38bb03373ced9728f1fecdbce7..c2a54dbcbb0d576da298f2a36a354afaef89b240 100644
--- a/drivers/net/ethernet/airoha/airoha_eth.c
+++ b/drivers/net/ethernet/airoha/airoha_eth.c
@@ -794,18 +794,34 @@ static int airoha_qdma_init_rx_queue(struct airoha_queue *q,
 
 static void airoha_qdma_cleanup_rx_queue(struct airoha_queue *q)
 {
-	struct airoha_eth *eth = q->qdma->eth;
+	struct airoha_qdma *qdma = q->qdma;
+	struct airoha_eth *eth = qdma->eth;
+	int qid = q - &qdma->q_rx[0];
 
 	while (q->queued) {
 		struct airoha_queue_entry *e = &q->entry[q->tail];
+		struct airoha_qdma_desc *desc = &q->desc[q->tail];
 		struct page *page = virt_to_head_page(e->buf);
 
 		dma_sync_single_for_cpu(eth->dev, e->dma_addr, e->dma_len,
 					page_pool_get_dma_dir(q->page_pool));
 		page_pool_put_full_page(q->page_pool, page, false);
+		/* 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);
+		WRITE_ONCE(desc->msg3, 0);
+
 		q->tail = (q->tail + 1) % q->ndesc;
 		q->queued--;
 	}
+
+	q->head = q->tail;
+	airoha_qdma_rmw(qdma, REG_RX_DMA_IDX(qid), RX_RING_DMA_IDX_MASK,
+			FIELD_PREP(RX_RING_DMA_IDX_MASK, q->tail));
 }
 
 static int airoha_qdma_init_rx(struct airoha_qdma *qdma)

---
base-commit: ae05340ccaa9d347fe85415609e075545bec589f
change-id: 20260326-airoha_qdma_cleanup_rx_queue-fix-5efd8e392bd0

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

Re: [PATCH net] net: airoha: Add missing cleanup bits in airoha_qdma_cleanup_rx_queue()

[Jakub Kicinski]

On Fri, 27 Mar 2026 10:48:21 +0100 Lorenzo Bianconi wrote:
> In order to properly cleanup hw rx QDMA queues and bring the device to
> the initial state, reset rx DMA queue head/tail index. Moreover, reset
> queued DMA descriptor fields.
> 
> Fixes: 23020f049327 ("net: airoha: Introduce ethernet support for EN7581 SoC")
> Tested-by: Madhur Agrawal <Madhur.Agrawal@airoha.com>
> Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org>

Take a look at sashiko, please:
https://sashiko.dev/#/patchset/20260327-airoha_qdma_cleanup_rx_queue-fix-v1-1-369d6ab1511a@kernel.org

Looks somewhat orthogonal to the current patch but probably worth
fixing.

Re: [PATCH net] net: airoha: Add missing cleanup bits in airoha_qdma_cleanup_rx_queue()

[patchwork-bot+netdevbpf]

Hello:

This patch was applied to netdev/net.git (main)
by Jakub Kicinski <kuba@kernel.org>:

On Fri, 27 Mar 2026 10:48:21 +0100 you wrote:
> In order to properly cleanup hw rx QDMA queues and bring the device to
> the initial state, reset rx DMA queue head/tail index. Moreover, reset
> queued DMA descriptor fields.
> 
> Fixes: 23020f049327 ("net: airoha: Introduce ethernet support for EN7581 SoC")
> Tested-by: Madhur Agrawal <Madhur.Agrawal@airoha.com>
> Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org>
> 
> [...]

Here is the summary with links:
  - [net] net: airoha: Add missing cleanup bits in airoha_qdma_cleanup_rx_queue()
    https://git.kernel.org/netdev/net/c/514aac359987

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html

Re: [PATCH net] net: airoha: Add missing cleanup bits in airoha_qdma_cleanup_rx_queue()

[Lorenzo Bianconi]

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

> On Fri, 27 Mar 2026 10:48:21 +0100 Lorenzo Bianconi wrote:
> > In order to properly cleanup hw rx QDMA queues and bring the device to
> > the initial state, reset rx DMA queue head/tail index. Moreover, reset
> > queued DMA descriptor fields.
> > 
> > Fixes: 23020f049327 ("net: airoha: Introduce ethernet support for EN7581 SoC")
> > Tested-by: Madhur Agrawal <Madhur.Agrawal@airoha.com>
> > Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org>
> 
> Take a look at sashiko, please:
> https://sashiko.dev/#/patchset/20260327-airoha_qdma_cleanup_rx_queue-fix-v1-1-369d6ab1511a@kernel.org
> 
> Looks somewhat orthogonal to the current patch but probably worth
> fixing.

Hi Jakub,

thx for pointing me to the sashiko's issues.

1- Could this code execute while the interface is still administratively up and
   the hardware DMA engines are actively receiving packets?
   Looking at the teardown paths, airoha_hw_cleanup() is called before
   unregister_netdev(). Unmapping buffers and zeroing descriptor addresses
   while the hardware might still be actively writing to them could cause
   physical memory corruption and IOMMU faults.
   Should unregister_netdev() (which quiesces the DMA via ndo_stop) be called
   before airoha_hw_cleanup() to avoid this?

I think the issue described above is already fixed in the following commit
available in net-next:

commit b1c803d5c8167026791abfaed96fd3e6a1fcd750
Author: Lorenzo Bianconi <lorenzo@kernel.org>
Date:   Sat Mar 21 15:41:44 2026 +0100

    net: airoha: Rework the code flow in airoha_remove() and in airoha_probe() error path
    
    As suggested by Simon in [0], rework the code flow in airoha_remove()
    and in the airoha_probe() error path in order to rely on a more common
    approach un-registering configured net-devices first and destroying the
    hw resources at the end of the code.
    Introduce airoha_qdma_cleanup routine to release QDMA resources.
    
    [0] https://lore.kernel.org/netdev/20251214-airoha-fix-dev-registration-v1-1-860e027ad4c6@kernel.org/
    
    Suggested-by: Simon Horman <horms@kernel.org>
    Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org>
    Reviewed-by: Simon Horman <horms@kernel.org>
    Link: https://patch.msgid.link/20260321-airoha-remove-rework-v2-1-16c7bade5fe5@kernel.org
    Signed-off-by: Paolo Abeni <pabeni@redhat.com>

How can we go ahead on this?

2- This is a pre-existing issue, but while reviewing this cleanup path, I
   noticed a potential NULL pointer dereference if initialization fails earlier.
   If devm_kzalloc() or dmam_alloc_coherent() fails in airoha_qdma_init_rx_queue(),
   it returns an error before netif_napi_add() is called, leaving the embedded
   q->napi struct zero-filled.
   However, q->ndesc is set earlier in that function. Since q->ndesc is now
   non-zero, the error cleanup path will try to disable and delete this
   uninitialized NAPI structure, leading to a crash in napi_disable() when it
   calls hrtimer_cancel() on the uninitialized timer.
   Could we defer setting q->ndesc until after the allocations succeed?

I think it is fine to set 'q->ndesc' at the end of airoha_qdma_init_rx_queue()
routine but, considering net codebase, it seems the issue can't occur since if
airoha_qdma_init_rx_queue() fails as described above, airoha_probe() will jump
to error_hw_cleanup and netif_napi_del() in airoha_hw_cleanup() will return if
NAPI_STATE_LISTED is not set in __netif_napi_del_locked().
Am I missing something?

3- Is there a missing reset for the CPU producer index (REG_RX_CPU_IDX) here?
   The hardware DMA relies on the gap between the CPU and DMA indices to
   identify valid descriptors. By rewinding the DMA consumer index (REG_RX_DMA_IDX)
   to q->tail while leaving the CPU index at its old, advanced value, could
   this create a phantom gap of descriptors?
   If the DMA engine fetches these zeroed descriptors (with addr = 0), it might
   attempt to write incoming packets to physical address 0x0. Both indices might
   need to be synchronized.

I will post a fix for it.

Regards,
Lorenzo

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