[PATCH rtw-next] wifi: rtl8xxxu: isolate the USB register buffer

[PATCH rtw-next] wifi: rtl8xxxu: isolate the USB register buffer

[Kim Wooseok]

I encountered reconnect failures on four Raspberry Pis in my project.
While investigating the failure on a Raspberry Pi 5 after an AP power
cycle, I found that RF_MODE had been corrupted. Restoring its initialized
value restored reception without reloading the driver.

The USB register buffer shared a cache line with CPU-written mutex state.
This patch allocates the buffer separately while keeping the existing
register access and locking.

I tested the separate-buffer approach on the Pi's Linux 6.18.34 kernel.
During repeated scan requests, wireless ping traffic kept receiving
replies and the RF_MODE and HSSI values stayed stable. I also built the
submission's rtl8xxxu driver for ARM64 with W=1; sparse reported no
diagnostics in core.c.

Signed-off-by: Kim Wooseok <5mghybrid@khu.ac.kr>

---
Kim Wooseok (1):
      wifi: rtl8xxxu: isolate the USB register buffer from CPU state

 drivers/net/wireless/realtek/rtl8xxxu/core.c     | 35 ++++++++++++++++--------
 drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h |  2 +-
 2 files changed, 24 insertions(+), 13 deletions(-)
---
base-commit: 83de3a16c7b37064a59305040ba9b0f93b832794
change-id: 20261010-rtl8xxxu-usb-dma-c94f476e1047

Best regards,
--  
Kim Wooseok <5mghybrid@khu.ac.kr>

[PATCH rtw-next] wifi: rtl8xxxu: isolate the USB register buffer

[Kim Wooseok via B4 Relay]

I encountered reconnect failures on four Raspberry Pis in my project.
While investigating the failure on a Raspberry Pi 5 after an AP power
cycle, I found that RF_MODE had been corrupted. Restoring its initialized
value restored reception without reloading the driver.

The USB register buffer shared a cache line with CPU-written mutex state.
This patch allocates the buffer separately while keeping the existing
register access and locking.

I tested the separate-buffer approach on the Pi's Linux 6.18.34 kernel.
During repeated scan requests, wireless ping traffic kept receiving
replies and the RF_MODE and HSSI values stayed stable. I also built the
submission's rtl8xxxu driver for ARM64 with W=1; sparse reported no
diagnostics in core.c.

Signed-off-by: Kim Wooseok <5mghybrid@khu.ac.kr>

---
Kim Wooseok (1):
      wifi: rtl8xxxu: isolate the USB register buffer from CPU state

 drivers/net/wireless/realtek/rtl8xxxu/core.c     | 35 ++++++++++++++++--------
 drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h |  2 +-
 2 files changed, 24 insertions(+), 13 deletions(-)
---
base-commit: 83de3a16c7b37064a59305040ba9b0f93b832794
change-id: 20261010-rtl8xxxu-usb-dma-c94f476e1047

Best regards,
--  
Kim Wooseok <5mghybrid@khu.ac.kr>

[PATCH rtw-next] wifi: rtl8xxxu: isolate the USB register buffer from CPU state

[Kim Wooseok]

An RTL8192EU failed to reconnect on a Raspberry Pi after an AP power
cycle. RF_MODE contained a corrupted value, and restoring its initialized
value restored reception.

The register access helpers pass usb_buf directly to usb_control_msg(),
but the buffer is embedded next to usb_buf_mutex. On the affected Pi,
it shares a cache line with the mutex's waiter state. A contending task
updates that state while the USB transfer uses the buffer. This violates
the cache-line isolation required for non-coherent DMA.

Allocate the buffer once in probe and reuse it, avoiding an allocation
for every register access. Keep the existing register access helpers and
mutex protection. Free the buffer on probe failure and after power_off()
in disconnect, since shutdown still uses the register access helpers.

Fixes: 26f1fad29ad9 ("New driver: rtl8xxxu (mac80211)")
Cc: stable@vger.kernel.org
Signed-off-by: Kim Wooseok <5mghybrid@khu.ac.kr>
---
I tested the separate-buffer approach on an RTL8192EU connected to a
Raspberry Pi 5, using the existing Linux 6.18.34 kernel. While repeatedly
requesting scans, wireless ping traffic kept receiving replies and the
RF_MODE and HSSI register values remained stable.

The submission is based on rtw-next. An ARM64 build of the rtl8xxxu
driver with W=1 completed without warnings, and sparse reported no
diagnostics in core.c.
---
 drivers/net/wireless/realtek/rtl8xxxu/core.c     | 35 ++++++++++++++++--------
 drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h |  2 +-
 2 files changed, 24 insertions(+), 13 deletions(-)

diff --git a/drivers/net/wireless/realtek/rtl8xxxu/core.c b/drivers/net/wireless/realtek/rtl8xxxu/core.c
index 3d0c22db3..604bb1e5b 100644
--- a/drivers/net/wireless/realtek/rtl8xxxu/core.c
+++ b/drivers/net/wireless/realtek/rtl8xxxu/core.c
@@ -622,9 +622,9 @@ u8 rtl8xxxu_read8(struct rtl8xxxu_priv *priv, u16 addr)
 	mutex_lock(&priv->usb_buf_mutex);
 	len = usb_control_msg(udev, usb_rcvctrlpipe(udev, 0),
 			      REALTEK_USB_CMD_REQ, REALTEK_USB_READ,
-			      addr, 0, &priv->usb_buf.val8, sizeof(u8),
+			      addr, 0, &priv->usb_buf->val8, sizeof(u8),
 			      RTW_USB_CONTROL_MSG_TIMEOUT);
-	data = priv->usb_buf.val8;
+	data = priv->usb_buf->val8;
 	mutex_unlock(&priv->usb_buf_mutex);
 
 	if (rtl8xxxu_debug & RTL8XXXU_DEBUG_REG_READ)
@@ -645,9 +645,9 @@ u16 rtl8xxxu_read16(struct rtl8xxxu_priv *priv, u16 addr)
 	mutex_lock(&priv->usb_buf_mutex);
 	len = usb_control_msg(udev, usb_rcvctrlpipe(udev, 0),
 			      REALTEK_USB_CMD_REQ, REALTEK_USB_READ,
-			      addr, 0, &priv->usb_buf.val16, sizeof(u16),
+			      addr, 0, &priv->usb_buf->val16, sizeof(u16),
 			      RTW_USB_CONTROL_MSG_TIMEOUT);
-	data = le16_to_cpu(priv->usb_buf.val16);
+	data = le16_to_cpu(priv->usb_buf->val16);
 	mutex_unlock(&priv->usb_buf_mutex);
 
 	if (rtl8xxxu_debug & RTL8XXXU_DEBUG_REG_READ)
@@ -668,9 +668,9 @@ u32 rtl8xxxu_read32(struct rtl8xxxu_priv *priv, u16 addr)
 	mutex_lock(&priv->usb_buf_mutex);
 	len = usb_control_msg(udev, usb_rcvctrlpipe(udev, 0),
 			      REALTEK_USB_CMD_REQ, REALTEK_USB_READ,
-			      addr, 0, &priv->usb_buf.val32, sizeof(u32),
+			      addr, 0, &priv->usb_buf->val32, sizeof(u32),
 			      RTW_USB_CONTROL_MSG_TIMEOUT);
-	data = le32_to_cpu(priv->usb_buf.val32);
+	data = le32_to_cpu(priv->usb_buf->val32);
 	mutex_unlock(&priv->usb_buf_mutex);
 
 	if (rtl8xxxu_debug & RTL8XXXU_DEBUG_REG_READ)
@@ -688,10 +688,10 @@ int rtl8xxxu_write8(struct rtl8xxxu_priv *priv, u16 addr, u8 val)
 		addr |= 0x8000;
 
 	mutex_lock(&priv->usb_buf_mutex);
-	priv->usb_buf.val8 = val;
+	priv->usb_buf->val8 = val;
 	ret = usb_control_msg(udev, usb_sndctrlpipe(udev, 0),
 			      REALTEK_USB_CMD_REQ, REALTEK_USB_WRITE,
-			      addr, 0, &priv->usb_buf.val8, sizeof(u8),
+			      addr, 0, &priv->usb_buf->val8, sizeof(u8),
 			      RTW_USB_CONTROL_MSG_TIMEOUT);
 
 	mutex_unlock(&priv->usb_buf_mutex);
@@ -711,10 +711,10 @@ int rtl8xxxu_write16(struct rtl8xxxu_priv *priv, u16 addr, u16 val)
 		addr |= 0x8000;
 
 	mutex_lock(&priv->usb_buf_mutex);
-	priv->usb_buf.val16 = cpu_to_le16(val);
+	priv->usb_buf->val16 = cpu_to_le16(val);
 	ret = usb_control_msg(udev, usb_sndctrlpipe(udev, 0),
 			      REALTEK_USB_CMD_REQ, REALTEK_USB_WRITE,
-			      addr, 0, &priv->usb_buf.val16, sizeof(u16),
+			      addr, 0, &priv->usb_buf->val16, sizeof(u16),
 			      RTW_USB_CONTROL_MSG_TIMEOUT);
 	mutex_unlock(&priv->usb_buf_mutex);
 
@@ -733,10 +733,10 @@ int rtl8xxxu_write32(struct rtl8xxxu_priv *priv, u16 addr, u32 val)
 		addr |= 0x8000;
 
 	mutex_lock(&priv->usb_buf_mutex);
-	priv->usb_buf.val32 = cpu_to_le32(val);
+	priv->usb_buf->val32 = cpu_to_le32(val);
 	ret = usb_control_msg(udev, usb_sndctrlpipe(udev, 0),
 			      REALTEK_USB_CMD_REQ, REALTEK_USB_WRITE,
-			      addr, 0, &priv->usb_buf.val32, sizeof(u32),
+			      addr, 0, &priv->usb_buf->val32, sizeof(u32),
 			      RTW_USB_CONTROL_MSG_TIMEOUT);
 	mutex_unlock(&priv->usb_buf_mutex);
 
@@ -7851,6 +7851,13 @@ static int rtl8xxxu_probe(struct usb_interface *interface,
 
 	usb_set_intfdata(interface, hw);
 
+	/* Keep the USB register buffer separate from driver state. */
+	priv->usb_buf = kzalloc_obj(*priv->usb_buf);
+	if (!priv->usb_buf) {
+		ret = -ENOMEM;
+		goto err_set_intfdata;
+	}
+
 	ret = rtl8xxxu_parse_usb(priv, interface);
 	if (ret)
 		goto err_set_intfdata;
@@ -7971,7 +7978,9 @@ static int rtl8xxxu_probe(struct usb_interface *interface,
 err_set_intfdata:
 	usb_set_intfdata(interface, NULL);
 
+	kfree(priv->usb_buf);
 	kfree(priv->fw_data);
+
 	mutex_destroy(&priv->usb_buf_mutex);
 	mutex_destroy(&priv->syson_indirect_access_mutex);
 	mutex_destroy(&priv->h2c_mutex);
@@ -7999,7 +8008,9 @@ static void rtl8xxxu_disconnect(struct usb_interface *interface)
 
 	dev_info(&priv->udev->dev, "disconnecting\n");
 
+	kfree(priv->usb_buf);
 	kfree(priv->fw_data);
+
 	mutex_destroy(&priv->usb_buf_mutex);
 	mutex_destroy(&priv->syson_indirect_access_mutex);
 	mutex_destroy(&priv->h2c_mutex);
diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h
index c6953051d..8cbadcf71 100644
--- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h
+++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h
@@ -1891,7 +1891,7 @@ struct rtl8xxxu_priv {
 		__le32 val32;
 		__le16 val16;
 		u8 val8;
-	} usb_buf;
+	} *usb_buf;
 	union {
 		u8 raw[EFUSE_MAP_LEN];
 		struct rtl8723au_efuse efuse8723;

-- 
2.53.0

[PATCH rtw-next] wifi: rtl8xxxu: isolate the USB register buffer from CPU state

[Kim Wooseok via B4 Relay]

From: Kim Wooseok <5mghybrid@khu.ac.kr>

An RTL8192EU failed to reconnect on a Raspberry Pi after an AP power
cycle. RF_MODE contained a corrupted value, and restoring its initialized
value restored reception.

The register access helpers pass usb_buf directly to usb_control_msg(),
but the buffer is embedded next to usb_buf_mutex. On the affected Pi,
it shares a cache line with the mutex's waiter state. A contending task
updates that state while the USB transfer uses the buffer. This violates
the cache-line isolation required for non-coherent DMA.

Allocate the buffer once in probe and reuse it, avoiding an allocation
for every register access. Keep the existing register access helpers and
mutex protection. Free the buffer on probe failure and after power_off()
in disconnect, since shutdown still uses the register access helpers.

Fixes: 26f1fad29ad9 ("New driver: rtl8xxxu (mac80211)")
Cc: stable@vger.kernel.org
Signed-off-by: Kim Wooseok <5mghybrid@khu.ac.kr>
---
I tested the separate-buffer approach on an RTL8192EU connected to a
Raspberry Pi 5, using the existing Linux 6.18.34 kernel. While repeatedly
requesting scans, wireless ping traffic kept receiving replies and the
RF_MODE and HSSI register values remained stable.

The submission is based on rtw-next. An ARM64 build of the rtl8xxxu
driver with W=1 completed without warnings, and sparse reported no
diagnostics in core.c.
---
 drivers/net/wireless/realtek/rtl8xxxu/core.c     | 35 ++++++++++++++++--------
 drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h |  2 +-
 2 files changed, 24 insertions(+), 13 deletions(-)

diff --git a/drivers/net/wireless/realtek/rtl8xxxu/core.c b/drivers/net/wireless/realtek/rtl8xxxu/core.c
index 3d0c22db3..604bb1e5b 100644
--- a/drivers/net/wireless/realtek/rtl8xxxu/core.c
+++ b/drivers/net/wireless/realtek/rtl8xxxu/core.c
@@ -622,9 +622,9 @@ u8 rtl8xxxu_read8(struct rtl8xxxu_priv *priv, u16 addr)
 	mutex_lock(&priv->usb_buf_mutex);
 	len = usb_control_msg(udev, usb_rcvctrlpipe(udev, 0),
 			      REALTEK_USB_CMD_REQ, REALTEK_USB_READ,
-			      addr, 0, &priv->usb_buf.val8, sizeof(u8),
+			      addr, 0, &priv->usb_buf->val8, sizeof(u8),
 			      RTW_USB_CONTROL_MSG_TIMEOUT);
-	data = priv->usb_buf.val8;
+	data = priv->usb_buf->val8;
 	mutex_unlock(&priv->usb_buf_mutex);
 
 	if (rtl8xxxu_debug & RTL8XXXU_DEBUG_REG_READ)
@@ -645,9 +645,9 @@ u16 rtl8xxxu_read16(struct rtl8xxxu_priv *priv, u16 addr)
 	mutex_lock(&priv->usb_buf_mutex);
 	len = usb_control_msg(udev, usb_rcvctrlpipe(udev, 0),
 			      REALTEK_USB_CMD_REQ, REALTEK_USB_READ,
-			      addr, 0, &priv->usb_buf.val16, sizeof(u16),
+			      addr, 0, &priv->usb_buf->val16, sizeof(u16),
 			      RTW_USB_CONTROL_MSG_TIMEOUT);
-	data = le16_to_cpu(priv->usb_buf.val16);
+	data = le16_to_cpu(priv->usb_buf->val16);
 	mutex_unlock(&priv->usb_buf_mutex);
 
 	if (rtl8xxxu_debug & RTL8XXXU_DEBUG_REG_READ)
@@ -668,9 +668,9 @@ u32 rtl8xxxu_read32(struct rtl8xxxu_priv *priv, u16 addr)
 	mutex_lock(&priv->usb_buf_mutex);
 	len = usb_control_msg(udev, usb_rcvctrlpipe(udev, 0),
 			      REALTEK_USB_CMD_REQ, REALTEK_USB_READ,
-			      addr, 0, &priv->usb_buf.val32, sizeof(u32),
+			      addr, 0, &priv->usb_buf->val32, sizeof(u32),
 			      RTW_USB_CONTROL_MSG_TIMEOUT);
-	data = le32_to_cpu(priv->usb_buf.val32);
+	data = le32_to_cpu(priv->usb_buf->val32);
 	mutex_unlock(&priv->usb_buf_mutex);
 
 	if (rtl8xxxu_debug & RTL8XXXU_DEBUG_REG_READ)
@@ -688,10 +688,10 @@ int rtl8xxxu_write8(struct rtl8xxxu_priv *priv, u16 addr, u8 val)
 		addr |= 0x8000;
 
 	mutex_lock(&priv->usb_buf_mutex);
-	priv->usb_buf.val8 = val;
+	priv->usb_buf->val8 = val;
 	ret = usb_control_msg(udev, usb_sndctrlpipe(udev, 0),
 			      REALTEK_USB_CMD_REQ, REALTEK_USB_WRITE,
-			      addr, 0, &priv->usb_buf.val8, sizeof(u8),
+			      addr, 0, &priv->usb_buf->val8, sizeof(u8),
 			      RTW_USB_CONTROL_MSG_TIMEOUT);
 
 	mutex_unlock(&priv->usb_buf_mutex);
@@ -711,10 +711,10 @@ int rtl8xxxu_write16(struct rtl8xxxu_priv *priv, u16 addr, u16 val)
 		addr |= 0x8000;
 
 	mutex_lock(&priv->usb_buf_mutex);
-	priv->usb_buf.val16 = cpu_to_le16(val);
+	priv->usb_buf->val16 = cpu_to_le16(val);
 	ret = usb_control_msg(udev, usb_sndctrlpipe(udev, 0),
 			      REALTEK_USB_CMD_REQ, REALTEK_USB_WRITE,
-			      addr, 0, &priv->usb_buf.val16, sizeof(u16),
+			      addr, 0, &priv->usb_buf->val16, sizeof(u16),
 			      RTW_USB_CONTROL_MSG_TIMEOUT);
 	mutex_unlock(&priv->usb_buf_mutex);
 
@@ -733,10 +733,10 @@ int rtl8xxxu_write32(struct rtl8xxxu_priv *priv, u16 addr, u32 val)
 		addr |= 0x8000;
 
 	mutex_lock(&priv->usb_buf_mutex);
-	priv->usb_buf.val32 = cpu_to_le32(val);
+	priv->usb_buf->val32 = cpu_to_le32(val);
 	ret = usb_control_msg(udev, usb_sndctrlpipe(udev, 0),
 			      REALTEK_USB_CMD_REQ, REALTEK_USB_WRITE,
-			      addr, 0, &priv->usb_buf.val32, sizeof(u32),
+			      addr, 0, &priv->usb_buf->val32, sizeof(u32),
 			      RTW_USB_CONTROL_MSG_TIMEOUT);
 	mutex_unlock(&priv->usb_buf_mutex);
 
@@ -7851,6 +7851,13 @@ static int rtl8xxxu_probe(struct usb_interface *interface,
 
 	usb_set_intfdata(interface, hw);
 
+	/* Keep the USB register buffer separate from driver state. */
+	priv->usb_buf = kzalloc_obj(*priv->usb_buf);
+	if (!priv->usb_buf) {
+		ret = -ENOMEM;
+		goto err_set_intfdata;
+	}
+
 	ret = rtl8xxxu_parse_usb(priv, interface);
 	if (ret)
 		goto err_set_intfdata;
@@ -7971,7 +7978,9 @@ static int rtl8xxxu_probe(struct usb_interface *interface,
 err_set_intfdata:
 	usb_set_intfdata(interface, NULL);
 
+	kfree(priv->usb_buf);
 	kfree(priv->fw_data);
+
 	mutex_destroy(&priv->usb_buf_mutex);
 	mutex_destroy(&priv->syson_indirect_access_mutex);
 	mutex_destroy(&priv->h2c_mutex);
@@ -7999,7 +8008,9 @@ static void rtl8xxxu_disconnect(struct usb_interface *interface)
 
 	dev_info(&priv->udev->dev, "disconnecting\n");
 
+	kfree(priv->usb_buf);
 	kfree(priv->fw_data);
+
 	mutex_destroy(&priv->usb_buf_mutex);
 	mutex_destroy(&priv->syson_indirect_access_mutex);
 	mutex_destroy(&priv->h2c_mutex);
diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h
index c6953051d..8cbadcf71 100644
--- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h
+++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h
@@ -1891,7 +1891,7 @@ struct rtl8xxxu_priv {
 		__le32 val32;
 		__le16 val16;
 		u8 val8;
-	} usb_buf;
+	} *usb_buf;
 	union {
 		u8 raw[EFUSE_MAP_LEN];
 		struct rtl8723au_efuse efuse8723;

-- 
2.53.0

RE: [PATCH rtw-next] wifi: rtl8xxxu: isolate the USB register buffer from CPU state

[Ping-Ke Shih]

Kim Wooseok via B4 Relay <devnull+5mghybrid.khu.ac.kr@kernel.org> wrote:
> From: Kim Wooseok <5mghybrid@khu.ac.kr>
> 
> An RTL8192EU failed to reconnect on a Raspberry Pi after an AP power
> cycle. RF_MODE contained a corrupted value, and restoring its initialized
> value restored reception.
> 
> The register access helpers pass usb_buf directly to usb_control_msg(),
> but the buffer is embedded next to usb_buf_mutex. On the affected Pi,
> it shares a cache line with the mutex's waiter state. A contending task
> updates that state while the USB transfer uses the buffer. This violates
> the cache-line isolation required for non-coherent DMA.

Cool. How did you dig that?

> 
> Allocate the buffer once in probe and reuse it, avoiding an allocation
> for every register access. Keep the existing register access helpers and
> mutex protection. Free the buffer on probe failure and after power_off()
> in disconnect, since shutdown still uses the register access helpers.
> 
> Fixes: 26f1fad29ad9 ("New driver: rtl8xxxu (mac80211)")
> Cc: stable@vger.kernel.org
> Signed-off-by: Kim Wooseok <5mghybrid@khu.ac.kr>

Acked-by: Ping-Ke Shih <pkshih@realtek.com>

Re: [PATCH rtw-next] wifi: rtl8xxxu: isolate the USB register buffer from CPU state

[‍김우석[학생](전자정보대학 전자공학과)]

Hi Ping-Ke,

> Cool. How did you dig that?

I was repeatedly power-cycling the AP to test recovery of a persistent
queue in my project. After fixing a separate NetworkManager issue,
some reconnect failures remained, so I started looking at the driver.

I was testing with four Raspberry Pis, and different units would hit
the same failure across runs. On a failing unit, RF_MODE was corrupted,
and restoring its initialized value brought reception back.

I narrowed it down to register corruption during scanning, then traced
the register access path. That led me to usb_buf sharing a cache line
with the mutex's waiter state, which can change while a USB transfer
is in progress.

With the buffer allocated separately, repeated scans no longer
corrupted the register values, and ping traffic kept receiving replies.

Wooseok

RE: [PATCH rtw-next] wifi: rtl8xxxu: isolate the USB register buffer from CPU state

[Ping-Ke Shih]

‍김우석[학생](전자정보대학 전자공학과) <5mghybrid@khu.ac.kr> wrote:
> Hi Ping-Ke,
> 
> > Cool. How did you dig that?
> 
> I was repeatedly power-cycling the AP to test recovery of a persistent
> queue in my project. After fixing a separate NetworkManager issue,
> some reconnect failures remained, so I started looking at the driver.
> 
> I was testing with four Raspberry Pis, and different units would hit
> the same failure across runs. On a failing unit, RF_MODE was corrupted,
> and restoring its initialized value brought reception back.
> 
> I narrowed it down to register corruption during scanning, then traced
> the register access path. That led me to usb_buf sharing a cache line
> with the mutex's waiter state, which can change while a USB transfer
> is in progress.

Does it mean that data in a cache line can't be modified while a USB
transfer? Even the data isn't a part of transferring data.
How can it affect the transfer? 

(I'm not a cache-line expert. Just try to understand the cause).

Ping-Ke

Re: [PATCH rtw-next] wifi: rtl8xxxu: isolate the USB register buffer from CPU state

[Kim Wooseok]

Hi Ping-Ke,

> Does it mean that data in a cache line can't be modified while a USB
> transfer? Even the data isn't a part of transferring data.
> How can it affect the transfer?

Yes, in this non-coherent DMA case, even modifying bytes outside the
transfer buffer can cause corruption if they share its cache line.

On the affected Pi, the 4-byte usb_buf shares a 64-byte cache line with
the mutex's waiter state.

The important detail is that a task waiting for the mutex still updates
its wait list before acquiring the lock. So the mutex serializes access
to usb_buf, but its own waiter state can change during the transfer.

For example, suppose the buffer initially contains 10:

1. Task A holds usb_buf_mutex and starts a USB register read.
2. Task B tries to acquire the same mutex and joins its wait list.
   This can bring the shared cache line into the CPU cache, including
   the old buffer value 10. Updating the list makes that line dirty.
3. The USB controller writes the new value 20 to RAM via DMA. Since
   this DMA is not cache-coherent, the cached copy still contains 10.
4. If the dirty cache line is then written back, the old buffer value
   10 can be written back along with the updated wait list, overwriting
   the value 20 from DMA.

Task B never directly touched usb_buf, but the writeback operates on
the shared cache line. A subsequent register read-modify-write can
then propagate that incorrect value into the hardware configuration.

I think one possible overlap during a rescan is between the
channel-change path in rtl8xxxu_config() and the periodic
rtl8xxxu_watchdog_callback(). The watchdog runs every two seconds
and may update frequency correction settings through
rtl8xxxu_track_cfo(). Both paths use the same register buffer and
usb_buf_mutex, so the watchdog could join the wait list while the
channel-change path is waiting for a USB read to finish.

Allocating usb_buf separately removes that sharing. The mutex still
serializes register accesses, while updates to its wait list no longer
affect the cache line containing the DMA buffer.

Wooseok

RE: [PATCH rtw-next] wifi: rtl8xxxu: isolate the USB register buffer from CPU state

[Ping-Ke Shih]

Kim Wooseok <5mghybrid@khu.ac.kr> wrote:
> 
> For example, suppose the buffer initially contains 10:
> 
> 1. Task A holds usb_buf_mutex and starts a USB register read.
> 2. Task B tries to acquire the same mutex and joins its wait list.
>    This can bring the shared cache line into the CPU cache, including
>    the old buffer value 10. Updating the list makes that line dirty.
> 3. The USB controller writes the new value 20 to RAM via DMA. Since
>    this DMA is not cache-coherent, the cached copy still contains 10.
> 4. If the dirty cache line is then written back, the old buffer value
>    10 can be written back along with the updated wait list, overwriting
>    the value 20 from DMA.

Thanks for the detail and this example, which is clear to me. :)

Ping-Ke