Re: [PATCH v7 00/17] mm, dma, arm64: Reduce ARCH_KMALLOC_MINALIGN to 8

[John Ernberg <john.ernberg@actia.se>]

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

On 6/12/23 5:31 PM, Catalin Marinas wrote:
> Hi,
> 
> That's v7 of the series reducing the kmalloc() minimum alignment on
> arm64 to 8 (from 128). There's no new/different functionality, mostly
> cosmetic changes and acks/tested-bys.
> 
> Andrew, if there are no further comments or objections to this version,
> are you ok to take the series through the mm tree? The arm64 changes are
> fairly small. Alternatively, I can push it into linux-next now to give
> it some wider exposure and decide whether to upstream it when the
> merging window opens. Thanks.
> 
> The updated patches are also available on this branch:
> 
> git://git.kernel.org/pub/scm/linux/kernel/git/arm64/linux devel/kmalloc-minalign
> 

(I trimmed the recipients and added linux-usb and imx mailing list, + 
cdns3 maintainers)

Hi Catalin, lists,

Seen on Linux 6.12.20, it is not trivial for us to test later kernels so
if the issue is potentially fixed we are more than happy to cherry-pick
the potential fixes and give them a go.

Having an SMSC9512 (smsc95xx) USB Ethernet/Hub chip attached to the armv8
SoC iMX8QXP over the Cadence USB3 USB2 interface (cdns3-imx) will since
the patch set at [0] cause random interrupt storms over the SMSC9512 INT
EP.

The reason for the storm is that the async URBs queued at [1] right before
the interrupt configuration [2] in the driver.
With [0] applied, those async URBs are likely clobbering any URB located
after them in memory somewhere in the xhci memory space.
The memory corruption only happens if there is more than one URB in the
queue at the same time, making these async URBs a good trigger of the
problem.
If we force those URBs to be sync or use the hack inlined below, the
problem goes away.

The content of read_buf in the interrupt configuration read at [2] looks
to be the lo-part of a pointer +-20 bytes distance from the pointers
present in the async URBs queued from [1] when we dumped the URB structures
instead of the expected register contents.

Attached are 3 usbmon captures, with some annotations, which we thought
might contain useful hints.

* usb-annotated-bad.pcapng shows the URB that will get clobbered is queued
   at position 280, and completes at 282.

* usb-annotated-good.pcapng shows the URB that gets clobbered in the bad
   case is queued at position 282 and completes at 283.

The above captures are from a kernel where the hack has not been applied.

* usb-annotated-hackfix.pcapng shows the URB that gets clobbered in the
   bad case is queued at position 238 and completes at 240.

This was captured after the hack fix inlined below was applied.

We have no reason to believe [0] is actually causing this, but rather
the improvements uncovered a pre-existing issue with the Cadence 3
integration in the iMX8QXP, as otherwise USB would be some level of broken
for absolutely everyone on ARM64 since 2+ years.

The reason we did not notice the issue earlier is that until very recently
we have been stuck on the NXP vendoring of 6.1 which pre-dates [0].

We determined [0] uncovers the breakage by replacing the host/ and cdns3/
code with the NXP 6.1 vendor kernel code in a Frankenstein fashion and
still ending up with the same USB breakage.

Running KASAN on this doesn't trigger anything, so it's not a KASAN
detectable issue.

Our knowledge on how to progress on debugging this has unfortunately come
to an end. We'd deeply appreciate any suggestion or possible patches to
test.

Best regards // John Ernberg

[0]: 
https://lore.kernel.org/linux-mm/20230612153201.554742-1-catalin.marinas@arm.com/
[1]: 
https://elixir.bootlin.com/linux/v6.12.19/source/drivers/net/usb/smsc95xx.c#L525
[2]: 
https://elixir.bootlin.com/linux/v6.12.19/source/drivers/net/usb/smsc95xx.c#L1045

------------------->8-----------
diff --git a/arch/arm64/include/asm/cache.h b/arch/arm64/include/asm/cache.h
index 06a4670bdb0b..7356801faa52 100644
--- a/arch/arm64/include/asm/cache.h
+++ b/arch/arm64/include/asm/cache.h
@@ -33,7 +33,7 @@
   * the CPU.
   */
  #define ARCH_DMA_MINALIGN      (128)
-#define ARCH_KMALLOC_MINALIGN  (8)
+#define ARCH_KMALLOC_MINALIGN  (128)

  #ifndef __ASSEMBLY__

[-- Attachment #2: usb-annotated-bad.pcapng --]
[-- Type: application/x-pcapng, Size: 62472 bytes --]

[-- Attachment #3: usb-annotated-good.pcapng --]
[-- Type: application/x-pcapng, Size: 52692 bytes --]

[-- Attachment #4: usb-annotated-hackfix.pcapng --]
[-- Type: application/x-pcapng, Size: 58324 bytes --]