[PATCH net-next V4] net: axienet: Clear stale AXI DMA TX/RX status before re-enabling interrupts

[PATCH net-next V4] net: axienet: Clear stale AXI DMA TX/RX status before re-enabling interrupts

[Kusuma Vasana]

The AXI DMA interrupt line is level-sensitive: it asserts whenever
the IOC (XAXIDMA_IRQ_IOC_MASK) or DELAY (XAXIDMA_IRQ_DELAY_MASK) bits
in the status register (XAXIDMA_TX_SR_OFFSET / XAXIDMA_RX_SR_OFFSET)
are set and their corresponding enable bits in the control register
are active.

During TX/RX, interrupts are disabled in the control register while
NAPI runs. Completions arriving in this window cause hardware to latch
IOC/DELAY into the status register regardless of the control register
mask state. After NAPI completion, re-enabling interrupts immediately
re-asserts the IRQ line due to these stale status register bits, even
when no new work is pending.

This results in a stale interrupt and redundant NAPI poll cycle with
no new work pending, causing unnecessary CPU processing.

In the initial driver, the status register was cleared after polling
all packets, which naturally consumed any status accumulated during
processing. In later versions of the driver, status register clearing
was moved to the ISR before polling begins, leaving no mechanism to
clear status bits that arrive during the NAPI poll window.

Clear the status register IOC/DELAY bits before re-enabling interrupts
and recheck the next BD to catch race-window completions. Both are
skipped on budget exhaustion. On the RX side, both are additionally
skipped on refill failure.

Signed-off-by: Kusuma Vasana <kusuma.vasana@amd.com>
---
Changes in v4:
- Read back SR after W1C so the clear is posted before the BD recheck.
- Document that poll may return @budget on that recheck.
- Drop the incorrect "leave IRQ armed" wording for RX refill failure.

Changes in v3:
- Updated the commit description.
- Restricted TX/RX status-register clear and completion recheck to the
  NAPI completion path, with RX only when refill succeeds.
- Rechecked descriptor completion after status-register clear to handle
  race-window completions.

Changes in v2:
-Added net-next prefix in the subject
-Updated the commit description

---
 .../net/ethernet/xilinx/xilinx_axienet_main.c | 29 +++++++++++++++++--
 1 file changed, 27 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/xilinx/xilinx_axienet_main.c b/drivers/net/ethernet/xilinx/xilinx_axienet_main.c
index fcf517069d16..4d8169fc39cd 100644
--- a/drivers/net/ethernet/xilinx/xilinx_axienet_main.c
+++ b/drivers/net/ethernet/xilinx/xilinx_axienet_main.c
@@ -986,7 +986,8 @@ axienet_start_xmit_dmaengine(struct sk_buff *skb, struct net_device *ndev)
  * @napi:	Pointer to NAPI structure.
  * @budget:	Max number of TX packets to process.
  *
- * Return: Number of TX packets processed.
+ * Return: Number of TX packets processed, or @budget to continue polling
+ * after a post-clear completion recheck.
  *
  * This function is invoked from the NAPI processing to notify the completion
  * of transmit operation. It clears fields in the corresponding Tx BDs and
@@ -1018,6 +1019,17 @@ static int axienet_tx_poll(struct napi_struct *napi, int budget)
 			netif_wake_queue(ndev);
 	}
 
+	/* Clear stale IOC/DELAY bits, flush the clear, and re-check for
+	 * race-window completions. Skipped on budget exhaustion.
+	 */
+	if (packets < budget) {
+		axienet_dma_out32(lp, XAXIDMA_TX_SR_OFFSET,
+				  XAXIDMA_IRQ_IOC_MASK | XAXIDMA_IRQ_DELAY_MASK);
+		axienet_dma_in32(lp, XAXIDMA_TX_SR_OFFSET);
+		if (lp->tx_bd_v[lp->tx_bd_ci].status & XAXIDMA_BD_STS_COMPLETE_MASK)
+			return budget;
+	}
+
 	if (packets < budget && napi_complete_done(napi, packets)) {
 		/* Re-enable TX completion interrupts. This should
 		 * cause an immediate interrupt if any TX packets are
@@ -1204,7 +1216,8 @@ static void axienet_dma_rx_cb(void *data, const struct dmaengine_result *result)
  * @napi:	Pointer to NAPI structure.
  * @budget:	Max number of RX packets to process.
  *
- * Return: Number of RX packets processed.
+ * Return: Number of RX packets processed, or @budget to continue polling
+ * after a post-clear completion recheck.
  */
 static int axienet_rx_poll(struct napi_struct *napi, int budget)
 {
@@ -1301,6 +1314,18 @@ static int axienet_rx_poll(struct napi_struct *napi, int budget)
 	if (tail_p)
 		axienet_dma_out_addr(lp, XAXIDMA_RX_TDESC_OFFSET, tail_p);
 
+	/* Clear stale IOC/DELAY bits, flush the clear, and re-check for
+	 * race-window completions. Skipped on budget exhaustion and on
+	 * refill failure (cur_p->skb == NULL).
+	 */
+	if (packets < budget && cur_p->skb) {
+		axienet_dma_out32(lp, XAXIDMA_RX_SR_OFFSET,
+				  XAXIDMA_IRQ_IOC_MASK | XAXIDMA_IRQ_DELAY_MASK);
+		axienet_dma_in32(lp, XAXIDMA_RX_SR_OFFSET);
+		if (cur_p->status & XAXIDMA_BD_STS_COMPLETE_MASK)
+			return budget;
+	}
+
 	if (packets < budget && napi_complete_done(napi, packets)) {
 		if (READ_ONCE(lp->rx_dim_enabled)) {
 			struct dim_sample sample = {
-- 
2.43.0

Re: [PATCH net-next V4] net: axienet: Clear stale AXI DMA TX/RX status before re-enabling interrupts

[Daniel Machon]

Hi Kusuma,

A few comments:

> The AXI DMA interrupt line is level-sensitive: it asserts whenever
> the IOC (XAXIDMA_IRQ_IOC_MASK) or DELAY (XAXIDMA_IRQ_DELAY_MASK) bits
> in the status register (XAXIDMA_TX_SR_OFFSET / XAXIDMA_RX_SR_OFFSET)
> are set and their corresponding enable bits in the control register
> are active.
> 
> During TX/RX, interrupts are disabled in the control register while
> NAPI runs. Completions arriving in this window cause hardware to latch
> IOC/DELAY into the status register regardless of the control register
> mask state. After NAPI completion, re-enabling interrupts immediately
> re-asserts the IRQ line due to these stale status register bits, even
> when no new work is pending.
> 
> This results in a stale interrupt and redundant NAPI poll cycle with
> no new work pending, causing unnecessary CPU processing.
> 
> In the initial driver, the status register was cleared after polling
> all packets, which naturally consumed any status accumulated during
> processing. In later versions of the driver, status register clearing
> was moved to the ISR before polling begins, leaving no mechanism to
> clear status bits that arrive during the NAPI poll window.
> 
> Clear the status register IOC/DELAY bits before re-enabling interrupts
> and recheck the next BD to catch race-window completions. Both are
> skipped on budget exhaustion. On the RX side, both are additionally
> skipped on refill failure.
> 
> Signed-off-by: Kusuma Vasana <kusuma.vasana@amd.com>
> ---
> Changes in v4:
> - Read back SR after W1C so the clear is posted before the BD recheck.
> - Document that poll may return @budget on that recheck.
> - Drop the incorrect "leave IRQ armed" wording for RX refill failure.
> 
> Changes in v3:
> - Updated the commit description.
> - Restricted TX/RX status-register clear and completion recheck to the
>   NAPI completion path, with RX only when refill succeeds.
> - Rechecked descriptor completion after status-register clear to handle
>   race-window completions.
> 
> Changes in v2:
> -Added net-next prefix in the subject
> -Updated the commit description
> 
> ---
>  .../net/ethernet/xilinx/xilinx_axienet_main.c | 29 +++++++++++++++++--
>  1 file changed, 27 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/net/ethernet/xilinx/xilinx_axienet_main.c b/drivers/net/ethernet/xilinx/xilinx_axienet_main.c
> index fcf517069d16..4d8169fc39cd 100644
> --- a/drivers/net/ethernet/xilinx/xilinx_axienet_main.c
> +++ b/drivers/net/ethernet/xilinx/xilinx_axienet_main.c
> @@ -986,7 +986,8 @@ axienet_start_xmit_dmaengine(struct sk_buff *skb, struct net_device *ndev)
>   * @napi:      Pointer to NAPI structure.
>   * @budget:    Max number of TX packets to process.
>   *
> - * Return: Number of TX packets processed.
> + * Return: Number of TX packets processed, or @budget to continue polling
> + * after a post-clear completion recheck.
>   *
>   * This function is invoked from the NAPI processing to notify the completion
>   * of transmit operation. It clears fields in the corresponding Tx BDs and
> @@ -1018,6 +1019,17 @@ static int axienet_tx_poll(struct napi_struct *napi, int budget)
>                         netif_wake_queue(ndev);
>         }
> 
> +       /* Clear stale IOC/DELAY bits, flush the clear, and re-check for
> +        * race-window completions. Skipped on budget exhaustion.
> +        */
> +       if (packets < budget) {
> +               axienet_dma_out32(lp, XAXIDMA_TX_SR_OFFSET,
> +                                 XAXIDMA_IRQ_IOC_MASK | XAXIDMA_IRQ_DELAY_MASK);
> +               axienet_dma_in32(lp, XAXIDMA_TX_SR_OFFSET);
> +               if (lp->tx_bd_v[lp->tx_bd_ci].status & XAXIDMA_BD_STS_COMPLETE_MASK)
> +                       return budget;
> +       }
> +

The patch does not apply on my tree.

This part was reworked in f9d58127380a ("net: axienet: do not report TX
completions as NAPI work") to not return budget-sized work, it now just returns
0.

>         if (packets < budget && napi_complete_done(napi, packets)) {
>                 /* Re-enable TX completion interrupts. This should
>                  * cause an immediate interrupt if any TX packets are
> @@ -1204,7 +1216,8 @@ static void axienet_dma_rx_cb(void *data, const struct dmaengine_result *result)
>   * @napi:      Pointer to NAPI structure.
>   * @budget:    Max number of RX packets to process.
>   *
> - * Return: Number of RX packets processed.
> + * Return: Number of RX packets processed, or @budget to continue polling
> + * after a post-clear completion recheck.
>   */
>  static int axienet_rx_poll(struct napi_struct *napi, int budget)
>  {
> @@ -1301,6 +1314,18 @@ static int axienet_rx_poll(struct napi_struct *napi, int budget)
>         if (tail_p)
>                 axienet_dma_out_addr(lp, XAXIDMA_RX_TDESC_OFFSET, tail_p);
> 
> +       /* Clear stale IOC/DELAY bits, flush the clear, and re-check for
> +        * race-window completions. Skipped on budget exhaustion and on
> +        * refill failure (cur_p->skb == NULL).
> +        */
> +       if (packets < budget && cur_p->skb) {
> +               axienet_dma_out32(lp, XAXIDMA_RX_SR_OFFSET,
> +                                 XAXIDMA_IRQ_IOC_MASK | XAXIDMA_IRQ_DELAY_MASK);
> +               axienet_dma_in32(lp, XAXIDMA_RX_SR_OFFSET);
> +               if (cur_p->status & XAXIDMA_BD_STS_COMPLETE_MASK)
> +                       return budget;
> +       }
> +
>         if (packets < budget && napi_complete_done(napi, packets)) {
>                 if (READ_ONCE(lp->rx_dim_enabled)) {
>

I think the RX side looks good.

>struct dim_sample sample = {
> --
> 2.43.0
> 
> 

/Daniel