Hi Russell, > -----Original Message----- > From: Russell King <rmk@xxxxxxxxxxxxxxx> On Behalf Of Russell King > Sent: Sunday, September 22, 2019 6:27 PM > To: Robin Murphy <robin.murphy@xxxxxxx>; dann frazier > <dann.frazier@xxxxxxxxxxxxx>; Will Deacon <will.deacon@xxxxxxx>; Nicolin > Chen <nicoleotsuka@xxxxxxxxx>; Y.b. Lu <yangbo.lu@xxxxxxx>; Christoph > Hellwig <hch@xxxxxx> > Cc: Adrian Hunter <adrian.hunter@xxxxxxxxx>; Ulf Hansson > <ulf.hansson@xxxxxxxxxx>; linux-mmc@xxxxxxxxxxxxxxx > Subject: [PATCH 2/3] mmc: sdhci-of-esdhc: set DMA snooping based on DMA > coherence > > We must not unconditionally set the DMA snoop bit; if the DMA API is > assuming that the device is not DMA coherent, and the device snoops the CPU > caches, the device can see stale cache lines brought in by speculative prefetch. > > This leads to the device seeing stale data, potentially resulting in corrupted > data transfers. Commonly, this results in a descriptor fetch error such as: > > mmc0: ADMA error > mmc0: sdhci: ============ SDHCI REGISTER DUMP =========== > mmc0: sdhci: Sys addr: 0x00000000 | Version: 0x00002202 > mmc0: sdhci: Blk size: 0x00000008 | Blk cnt: 0x00000001 > mmc0: sdhci: Argument: 0x00000000 | Trn mode: 0x00000013 > mmc0: sdhci: Present: 0x01f50008 | Host ctl: 0x00000038 > mmc0: sdhci: Power: 0x00000003 | Blk gap: 0x00000000 > mmc0: sdhci: Wake-up: 0x00000000 | Clock: 0x000040d8 > mmc0: sdhci: Timeout: 0x00000003 | Int stat: 0x00000001 > mmc0: sdhci: Int enab: 0x037f108f | Sig enab: 0x037f108b > mmc0: sdhci: ACmd stat: 0x00000000 | Slot int: 0x00002202 > mmc0: sdhci: Caps: 0x35fa0000 | Caps_1: 0x0000af00 > mmc0: sdhci: Cmd: 0x0000333a | Max curr: 0x00000000 > mmc0: sdhci: Resp[0]: 0x00000920 | Resp[1]: 0x001d8a33 > mmc0: sdhci: Resp[2]: 0x325b5900 | Resp[3]: 0x3f400e00 > mmc0: sdhci: Host ctl2: 0x00000000 > mmc0: sdhci: ADMA Err: 0x00000009 | ADMA Ptr: 0x000000236d43820c > mmc0: sdhci: ============================================ > mmc0: error -5 whilst initialising SD card > > but can lead to other errors, and potentially direct the SDHCI controller to > read/write data to other memory locations (e.g. if a valid descriptor is visible > to the device in a stale cache line.) > > Fix this by ensuring that the DMA snoop bit corresponds with the behaviour of > the DMA API. Since the driver currently only supports DT, use > of_dma_is_coherent(). Note that device_get_dma_attr() can not be used as > that risks re-introducing this bug if/when the driver is converted to ACPI. [Y.b. Lu] May I have your suggestion how to check this condition with ACPI? Although we didn’t support APCI on upstream for this driver now, as I know we had already had ACPI used internally and would be upstreamed. Thanks. > > Signed-off-by: Russell King <rmk+kernel@xxxxxxxxxxxxxxx> > --- > drivers/mmc/host/sdhci-of-esdhc.c | 7 ++++++- > 1 file changed, 6 insertions(+), 1 deletion(-) > > diff --git a/drivers/mmc/host/sdhci-of-esdhc.c > b/drivers/mmc/host/sdhci-of-esdhc.c > index 4dd43b1adf2c..74de5e8c45c8 100644 > --- a/drivers/mmc/host/sdhci-of-esdhc.c > +++ b/drivers/mmc/host/sdhci-of-esdhc.c > @@ -495,7 +495,12 @@ static int esdhc_of_enable_dma(struct sdhci_host > *host) > dma_set_mask_and_coherent(dev, DMA_BIT_MASK(40)); > > value = sdhci_readl(host, ESDHC_DMA_SYSCTL); > - value |= ESDHC_DMA_SNOOP; > + > + if (of_dma_is_coherent(dev->of_node)) > + value |= ESDHC_DMA_SNOOP; > + else > + value &= ~ESDHC_DMA_SNOOP; > + > sdhci_writel(host, value, ESDHC_DMA_SYSCTL); > return 0; > } > -- > 2.7.4
