Re: [PATCH] mmc: sdhci-msm: Disable broken 64-bit DMA on MSM8916

[Stephan Gerhold <stephan@gerhold.net>]

On Thu, May 18, 2023 at 11:48:55AM +0200, Konrad Dybcio wrote:
> On 18.05.2023 11:39, Stephan Gerhold wrote:
> > While SDHCI claims to support 64-bit DMA on MSM8916 it does not seem to
> > be properly functional. It is not immediately obvious because SDHCI is
> > usually used with IOMMU bypassed on this SoC, and all physical memory
> > has 32-bit addresses. But when trying to enable the IOMMU it quickly
> > fails with an error such as the following:
> > 
> >   arm-smmu 1e00000.iommu: Unhandled context fault:
> >     fsr=0x402, iova=0xfffff200, fsynr=0xe0000, cbfrsynra=0x140, cb=3
> >   mmc1: ADMA error: 0x02000000
> >   mmc1: sdhci: ============ SDHCI REGISTER DUMP ===========
> >   mmc1: sdhci: Sys addr:  0x00000000 | Version:  0x00002e02
> >   mmc1: sdhci: Blk size:  0x00000008 | Blk cnt:  0x00000000
> >   mmc1: sdhci: Argument:  0x00000000 | Trn mode: 0x00000013
> >   mmc1: sdhci: Present:   0x03f80206 | Host ctl: 0x00000019
> >   mmc1: sdhci: Power:     0x0000000f | Blk gap:  0x00000000
> >   mmc1: sdhci: Wake-up:   0x00000000 | Clock:    0x00000007
> >   mmc1: sdhci: Timeout:   0x0000000a | Int stat: 0x00000001
> >   mmc1: sdhci: Int enab:  0x03ff900b | Sig enab: 0x03ff100b
> >   mmc1: sdhci: ACmd stat: 0x00000000 | Slot int: 0x00000000
> >   mmc1: sdhci: Caps:      0x322dc8b2 | Caps_1:   0x00008007
> >   mmc1: sdhci: Cmd:       0x0000333a | Max curr: 0x00000000
> >   mmc1: sdhci: Resp[0]:   0x00000920 | Resp[1]:  0x5b590000
> >   mmc1: sdhci: Resp[2]:   0xe6487f80 | Resp[3]:  0x0a404094
> >   mmc1: sdhci: Host ctl2: 0x00000008
> >   mmc1: sdhci: ADMA Err:  0x00000001 | ADMA Ptr: 0x0000000ffffff224
> >   mmc1: sdhci_msm: ----------- VENDOR REGISTER DUMP -----------
> >   mmc1: sdhci_msm: DLL sts: 0x00000000 | DLL cfg:  0x60006400 | DLL cfg2: 0x00000000
> >   mmc1: sdhci_msm: DLL cfg3: 0x00000000 | DLL usr ctl:  0x00000000 | DDR cfg: 0x00000000
> >   mmc1: sdhci_msm: Vndr func: 0x00018a9c | Vndr func2 : 0xf88018a8 Vndr func3: 0x00000000
> >   mmc1: sdhci: ============================================
> >   mmc1: sdhci: fffffffff200: DMA 0x0000ffffffffe100, LEN 0x0008, Attr=0x21
> >   mmc1: sdhci: fffffffff20c: DMA 0x0000000000000000, LEN 0x0000, Attr=0x03
> > 
> > Looking closely it's obvious that only the 32-bit part of the address
> > (0xfffff200) arrives at the SMMU, the higher 16-bit (0xffff...) get
> > lost somewhere. This might not be a limitation of the SDHCI itself but
> > perhaps the bus/interconnect it is connected to, or even the connection
> > to the SMMU.
> > 
> > Work around this by setting SDHCI_QUIRK2_BROKEN_64_BIT_DMA to avoid
> > using 64-bit addresses.
> > 
> > Signed-off-by: Stephan Gerhold <stephan@gerhold.net>
> > ---
> Would using 64bit address translation not require you to use (dma-)ranges
> with a greater-than-default size, like we do on newer platforms? Did you
> test that by chance?
> 

No, there is no "dma-ranges" in msm8916.dtsi. It seems to use 64-bit
(virtual) addresses by default for SDHCI, limited to 48-bit by the SMMU.

I tried limiting dma-ranges to 32-bit in msm8916.dtsi /soc@0 like
dma-ranges = <0 0 0 0xffffffff>; This seems to work as well as an
alternative to this patch, although it causes several annoying
"Invalid size 0xffffffff for dma-range(s)" warnings. I know the
0xffffffff isn't valid but I don't see how I would actually specify
2^32 there without rewriting the entire msm8916.dtsi to use
#address/size-cells = <2> instead of <1>.

It's not entirely clear to me where the actual limitation is coming from
here. It could be specific to the SDHCI (in which case this patch makes
more sense) or a general limitation of the bus/interconnect (in which
case dma-ranges might make more sense).

In any case 64-bit DMA is broken for SDHCI on MSM8916 so I would say
this patch is still the right thing to do. It's pointless to waste extra
cycles and memory to manage 64-bit pointers for SDHCI ADMA 64-bit, when
they can never work successfully on this SoC. The dma-ranges approach
doesn't change that, while SDHCI_QUIRK2_BROKEN_64_BIT_DMA actually makes
it use ADMA in 32-bit mode.

Thanks,
Stephan