[PATCH v2] wifi: rtw88: sdio: Fix unhandled RX request interrupt storm

[PATCH v2] wifi: rtw88: sdio: Fix unhandled RX request interrupt storm

[Alastair D'Silva]

In rtw_sdio_handle_interrupt(), the HISR status register is cleared using
Write-1-to-Clear (W1C) semantics. However, the driver masks out the
REG_SDIO_HISR_RX_REQUEST bit in the local 'hisr' variable before writing
it back, causing a 0 to be written to that bit. This prevents the RX
request interrupt from being acknowledged and cleared in hardware, trapping
the CPU core in an infinite interrupt storm loop (starving the RCU preempt
kthread and locking up the system).

Masking out the bit in software was originally intended to allow
budget-limited polling without losing interrupts. However, because the
status register was never acknowledged, the hardware interrupt line
remained asserted continuously.

Resolve this by adopting the standard interrupt masking pattern:
1. Mask interrupts via rtw_sdio_disable_interrupt(rtwdev) upon entry.
2. Acknowledge pending status bits via rtw_write32(rtwdev, REG_SDIO_HISR,
   hisr) without clearing REG_SDIO_HISR_RX_REQUEST from the writeback mask.
3. Service the pending interrupt events (rtw_sdio_rx_isr).
4. Re-enable interrupts via rtw_sdio_enable_interrupt(rtwdev).

Any packet arriving over the air while interrupts are masked latches
REG_SDIO_HISR_RX_REQUEST in hardware. Unmasking HIMR restores the interrupt
mask and immediately re-asserts the SDIO interrupt line (DAT[1]), causing
ksdioirqd to run another pass and drain the new packets.

Fixes: 65371a3f14e7 ("wifi: rtw88: sdio: Add HCI implementation for SDIO based chipsets")
Cc: stable@vger.kernel.org
Signed-off-by: Alastair D'Silva <alastair@d-silva.org>
---
v2:
  - Drop the complex drain-and-recheck inside rtw_sdio_rx_isr() and budget
    bypass logic from v1.
  - Implement clean HIMR masking (rtw_sdio_disable_interrupt() on entry,
    rtw_sdio_enable_interrupt() on exit), matching the PCI driver model as
    suggested by Ping-Ke Shih and the Realtek internal team.
  - Verified on physical RTL8821CS hardware (Allwinner H618 / Mellow Fly-C5).

Empirical Testbed & Timing Measurements:
----------------------------------------
To address concerns regarding bus overhead and packet loss during the masked
window, rtw_sdio_handle_interrupt() was instrumented with nanosecond ktime
timestamps on an Allwinner H618 board with RTL8821CS SDIO Wi-Fi under
continuous saturation traffic (38.0 Mbps TCP / 34.4 Mbps UDP with 0% loss):

1. Bus Overhead (average over 18,660 interrupts):
   - Disable HIMR:     13.88 us (min: 10.00 us, max: 74.83 us)
   - Acknowledge HISR: 13.69 us (min: 8.91 us, max: 39.33 us)
   - RX ISR work:     204.78 us (min: 48.66 us, max: 5.07 ms)
   - Enable HIMR:      14.46 us (min: 9.54 us, max: 52.79 us)
   - Total ISR time:  280.32 us
   Toggling HIMR accounts for ~28.35 us (~10% of total ISR duration) and is
   negligible compared to payload data transfers over SDIO.

2. Hardware Latching & No FIFO Stall:
   - In 1,251 out of 18,660 interrupts (6.7%), new packets arrived over the
     air while interrupts were masked during FIFO drainage.
   - Reading REG_SDIO_HISR immediately prior to unmasking confirmed that
     REG_SDIO_HISR_RX_REQUEST was latched in hardware in every instance.
   - Unmasking HIMR immediately re-asserted DAT[1], causing ksdioirqd to
     service the newly arrived packets. Zero packets were left stuck in FIFO.
   - Injecting artificial delays up to 2000 us before unmasking increased
     masked arrivals to 16.4% while maintaining 0% extra packet loss and
     zero stalls.

 drivers/net/wireless/realtek/rtw88/sdio.c | 13 +++++++++----
 1 file changed, 9 insertions(+), 4 deletions(-)

diff --git a/drivers/net/wireless/realtek/rtw88/sdio.c b/drivers/net/wireless/realtek/rtw88/sdio.c
index 5b40d74b16ee..ff707f207a83 100644
--- a/drivers/net/wireless/realtek/rtw88/sdio.c
+++ b/drivers/net/wireless/realtek/rtw88/sdio.c
@@ -1087,16 +1087,21 @@ static void rtw_sdio_handle_interrupt(struct sdio_func *sdio_func)
 	rtwsdio->irq_thread = current;
 
 	hisr = rtw_read32(rtwdev, REG_SDIO_HISR);
+	if (!hisr)
+		goto out;
+
+	rtw_sdio_disable_interrupt(rtwdev);
+	rtw_write32(rtwdev, REG_SDIO_HISR, hisr);
 
 	if (hisr & REG_SDIO_HISR_TXERR)
 		rtw_sdio_tx_err_isr(rtwdev);
-	if (hisr & REG_SDIO_HISR_RX_REQUEST) {
-		hisr &= ~REG_SDIO_HISR_RX_REQUEST;
+	if (hisr & REG_SDIO_HISR_RX_REQUEST)
 		rtw_sdio_rx_isr(rtwdev);
-	}
 
-	rtw_write32(rtwdev, REG_SDIO_HISR, hisr);
+	/* Unmasking HIMR re-asserts the IRQ line if new packets arrived */
+	rtw_sdio_enable_interrupt(rtwdev);
 
+out:
 	rtwsdio->irq_thread = NULL;
 }
 
-- 
2.53.0

Re: [PATCH v2] wifi: rtw88: sdio: Fix unhandled RX request interrupt storm

[Alastair D'Silva]

On Wed, 2026-09-30 at 19:07 +1000, Alastair D'Silva wrote:
> In rtw_sdio_handle_interrupt(), the HISR status register is cleared using
> Write-1-to-Clear (W1C) semantics. However, the driver masks out the
> REG_SDIO_HISR_RX_REQUEST bit in the local 'hisr' variable before writing
> it back, causing a 0 to be written to that bit. This prevents the RX
> request interrupt from being acknowledged and cleared in hardware, trapping
> the CPU core in an infinite interrupt storm loop (starving the RCU preempt
> kthread and locking up the system).
> 
<snip>
> Fixes: 65371a3f14e7 ("wifi: rtw88: sdio: Add HCI implementation for SDIO based chipsets")
> Cc: stable@vger.kernel.org
> Signed-off-by: Alastair D'Silva <alastair@d-silva.org>
> 

Apologies, I omitted the required attribution tag per Documentation/process/coding-assistants.rst:

Assisted-by: LLM

Maintainers, please feel free to add this when applying, or let me know if you would prefer a v3
respin.

Cheers,

-- 
Alastair D'Silva

Re: [PATCH v2] wifi: rtw88: sdio: Fix unhandled RX request interrupt storm

[Luka Gejak]

Hi Alastair,

This breaks the drain loop on the 8051 parts. The early write clears
REG_SDIO_HISR_RX_REQUEST before rtw_sdio_rx_isr() runs, but the 8051 branch
of the loop uses that bit to decide whether another request is pending:

> +	rtw_sdio_disable_interrupt(rtwdev);
> +	rtw_write32(rtwdev, REG_SDIO_HISR, hisr);
> [...]
> +	if (hisr & REG_SDIO_HISR_RX_REQUEST)
>  		rtw_sdio_rx_isr(rtwdev);

		if (rtw_chip_wcpu_8051(rtwdev)) {
			hisr = rtw_read32(rtwdev, REG_SDIO_HISR);
		} else {
			hisr = REG_SDIO_HISR_RX_REQUEST;
		}
	} while (total_rx_bytes < SZ_64K && hisr & REG_SDIO_HISR_RX_REQUEST);

In the v1 thread Ping-Ke relayed that the hardware clears the bit once the
RX buffer is empty, and that a software clear means no new interrupt unless
a new packet arrives. So the first re-read after the write returns 0, the
loop stops after one request, and the rest of the FIFO waits for the next
packet.

The 8821CS cannot show this, since 3081 parts never read HISR there.
RTL8723CS, RTL8723DS and the RTL8723BS once its glue lands are the 8051
SDIO parts, so please run the test on one of those, or leave RX_REQUEST out
of the early write and let the hardware drop it.

This also needs a rebase for rtw-next. The 8723BS mask sits between this
block and the old write:

	if (rtw_is_8723bs(rtwdev))
		hisr &= RTW_SDIO_HISR_CLEAR_MASK;

	rtw_write32(rtwdev, REG_SDIO_HISR, hisr);

Moving the write up as posted puts the raw value back, undefined bits
included, which is the resume storm on 8723BS that the mask exists to
avoid.

One smaller thing, the early write also consumes anything already queued
when the handler starts, so what the 64K budget leaves behind waits for the
next packet too, on 3081 as well.

Best regards,
Luka Gejak