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

["Vasana, Kusuma" <Kusuma.Vasana@amd.com>]

On 7/25/2026 4:49 AM, Jakub Kicinski wrote:
> This is an AI-generated review of your patch. The human sending this
> email has considered the AI review valid, or at least plausible.
> ---
> net: axienet: Clear stale AXI DMA TX/RX status before re-enabling interrupts
> 
> This patch clears the AXI DMA IOC/DELAY status bits at the end of the TX
> and RX NAPI poll handlers to avoid a redundant stale interrupt caused by
> IOC/DELAY bits latched into the status register while interrupts were
> masked in the control register during NAPI. The clear is performed
> unconditionally, before the napi_complete_done() / CR re-enable step.
> 
>> diff --git a/drivers/net/ethernet/xilinx/xilinx_axienet_main.c b/drivers/net/ethernet/xilinx/xilinx_axienet_main.c
>> index fcf517069d16..29050c8d04e2 100644
>> --- a/drivers/net/ethernet/xilinx/xilinx_axienet_main.c
>> +++ b/drivers/net/ethernet/xilinx/xilinx_axienet_main.c
>> @@ -1018,6 +1018,13 @@ static int axienet_tx_poll(struct napi_struct *napi, int budget)
>>   			netif_wake_queue(ndev);
>>   	}
>>   
>> +	/* Clear stale IOC/DELAY bits that may have latched during the
>> +	 * poll window to prevent a stale interrupt when there is no
>> +	 * work pending.
>> +	 */
>> +	axienet_dma_out32(lp, XAXIDMA_TX_SR_OFFSET,
>> +			  XAXIDMA_IRQ_IOC_MASK | XAXIDMA_IRQ_DELAY_MASK);
>> +
>>   	if (packets < budget && napi_complete_done(napi, packets)) {
>>   		/* Re-enable TX completion interrupts. This should
>>   		 * cause an immediate interrupt if any TX packets are
> 
> [High]
> Can this sequence race with a late completion arriving after the poll
> loop has exited but before the SR write?
> 
> The AXI DMA engine writes cur_p->status and latches SR.IOC asynchronously
> from CPU execution. The poll loop's exit condition reads cur_p->status,
> and between that read and the unconditional SR IOC/DELAY clear that this
> hunk adds, the hardware can set COMPLETE_MASK on the next BD and latch
> SR.IOC for it. That legitimate notification would then be wiped by the
> new W1C write.
> 
> Since napi_schedule() is only called from the ISR, and the ISR only fires
> when the IRQ line asserts, napi_complete_done() will succeed here even
> though a new completion has appeared. The subsequent CR re-enable would
> then observe SR.IOC=0, so the level-sensitive line does not re-assert,
> and the completed BD is stranded until the next unrelated completion or
> DELAY-timer expiry moves things forward.
> 
> Before this patch, any SR bit latched during the poll window (whether
> stale or from a late completion) re-asserted the IRQ line after CR
> re-enable, which the changelog frames purely as an unnecessary stale
> interrupt:
> 
>      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.
> 
> Did that same behavior also serve as the recovery path for a completion
> that lands in the SR-clear/CR-reenable window? If so, would it be worth
> either re-checking cur_p->status after the SR clear and skipping
> napi_complete_done() when new work has appeared, or performing the CR
> re-enable before the SR clear so a late latch still asserts the line?
> 
> NAPIF_STATE_MISSED does not appear to help here because it requires a
> napi_schedule() call, which only the ISR performs.
> 
> For configurations where axienet_calc_cr() does arm the DELAY timer
> (coalesce count > 1), the stranded BD is recovered when DELAY expires:
> 
>      /* Only set interrupt delay timer if not generating an interrupt on
>       * every packet
>       */
> 
> Under DIM with low traffic the coalesce count can settle at 1, in which
> case DELAY is not armed and there is no bounded recovery for the last
> BD of a burst until another packet arrives. Is that concern valid, or is
> there another mechanism that ensures the last BD is drained?
> 
>> @@ -1293,6 +1300,13 @@ static int axienet_rx_poll(struct napi_struct *napi, int budget)
>>   		cur_p = &lp->rx_bd_v[lp->rx_bd_ci];
>>   	}
>>   
>> +	/* Clear stale IOC/DELAY bits that may have latched during the
>> +	 * poll window to prevent a stale interrupt when there is no
>> +	 * work pending.
>> +	 */
>> +	axienet_dma_out32(lp, XAXIDMA_RX_SR_OFFSET,
>> +			  XAXIDMA_IRQ_IOC_MASK | XAXIDMA_IRQ_DELAY_MASK);
>> +
> 
> The same question applies to the RX path: a completion arriving after
> the while loop's cur_p->status check but before this SR write would have
> its SR.IOC latch cleared, and the subsequent CR re-enable would then not
> re-assert the line. Is there a reason the RX path is not exposed to the
> same race?


Thanks for the review. I agree and will add a BD status recheck after 
clearing the SR to handle any completions that arrive during the race 
window. I will address this in V3.

Thanks,
Kusuma