| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <c96c620d-a8a7-2aa6-452e-65e3e1fe79ab@seco.com>
Date: Tue, 20 Sep 2022 19:05:27 -0400
From: Sean Anderson <sean.anderson@...o.com>
To: Leo Li <leoyang.li@....com>, Robin Murphy <robin.murphy@....com>,
Oleksij Rempel <linux@...pel-privat.de>,
Pengutronix Kernel Team <kernel@...gutronix.de>,
"linux-i2c@...r.kernel.org" <linux-i2c@...r.kernel.org>,
linux-arm-kernel <linux-arm-kernel@...ts.infradead.org>,
Vinod Koul <vkoul@...nel.org>,
"dmaengine@...r.kernel.org" <dmaengine@...r.kernel.org>,
Laurentiu Tudor <laurentiu.tudor@....com>
Cc: Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
"dri-devel@...ts.freedesktop.org" <dri-devel@...ts.freedesktop.org>,
Christian König <christian.koenig@....com>,
"linaro-mm-sig@...ts.linaro.org" <linaro-mm-sig@...ts.linaro.org>,
Shawn Guo <shawnguo@...nel.org>,
Sumit Semwal <sumit.semwal@...aro.org>,
Joy Zou <joy.zou@....com>,
"linux-media@...r.kernel.org" <linux-media@...r.kernel.org>
Subject: Re: [BUG] ls1046a: eDMA does not transfer data from I2C
On 9/20/22 6:49 PM, Leo Li wrote:
>
>
>> -----Original Message-----
>> From: Sean Anderson <sean.anderson@...o.com>
>> Sent: Tuesday, September 20, 2022 11:21 AM
>> To: Robin Murphy <robin.murphy@....com>; Oleksij Rempel
>> <linux@...pel-privat.de>; Pengutronix Kernel Team
>> <kernel@...gutronix.de>; linux-i2c@...r.kernel.org; linux-arm-kernel
>> <linux-arm-kernel@...ts.infradead.org>; Vinod Koul <vkoul@...nel.org>;
>> dmaengine@...r.kernel.org; Leo Li <leoyang.li@....com>; Laurentiu Tudor
>> <laurentiu.tudor@....com>
>> Cc: Linux Kernel Mailing List <linux-kernel@...r.kernel.org>; dri-
>> devel@...ts.freedesktop.org; Christian König <christian.koenig@....com>;
>> linaro-mm-sig@...ts.linaro.org; Shawn Guo <shawnguo@...nel.org>; Sumit
>> Semwal <sumit.semwal@...aro.org>; Joy Zou <joy.zou@....com>; linux-
>> media@...r.kernel.org
>> Subject: Re: [BUG] ls1046a: eDMA does not transfer data from I2C
>>
>>
>>
>> On 9/20/22 11:44 AM, Sean Anderson wrote:
>> >
>> >
>> > On 9/20/22 11:24 AM, Sean Anderson wrote:
>> >>
>> >>
>> >> On 9/20/22 6:07 AM, Robin Murphy wrote:
>> >>> On 2022-09-19 23:24, Sean Anderson wrote:
>> >>>> Hi all,
>> >>>>
>> >>>> I discovered a bug in either imx_i2c or fsl-edma on the LS1046A
>> >>>> where no data is read in i2c_imx_dma_read except for the last two
>> >>>> bytes (which are not read using DMA). This is perhaps best
>> >>>> illustrated with the following example:
>> >>>>
>> >>>> # hexdump -C /sys/bus/nvmem/devices/0-00540/nvmem
>> >>>> [ 308.914884] i2c i2c-0: ffff000809380000 0x0000000889380000
>> 0x00000000f5401000 ffff000075401000
>> >>>> [ 308.923529] src= 2180004 dst=f5401000 attr= 0 soff= 0 nbytes=1
>> slast= 0
>> >>>> [ 308.923529] citer= 7e biter= 7e doff= 1 dlast_sga= 0
>> >>>> [ 308.923529] major_int=1 disable_req=1 enable_sg=0 [ 308.942113]
>> >>>> fsl-edma 2c00000.edma: vchan 000000001b4371fc: txd
>> >>>> 00000000d9dd26c5[4]: submitted [ 308.974049] fsl-edma
>> >>>> 2c00000.edma: txd 00000000d9dd26c5[4]: marked complete [
>> >>>> 308.981339] i2c i2c-0: ffff000809380000 = [2e 2e 2f 2e 2e 2f 2e 2e
>> >>>> 2f 64 65 76 69 63 65 73 2f 70 6c 61 74 66 6f 72 6d 2f 73 6f 63 2f 32 31 38 30
>> 30 30 30 2e 69 32 63 2f 69 32 63 2d 30 2f 30 2d 30 30 35 34 2f 30 2d 30 30 35 34
>> 30 00 00] [ 309.002226] i2c i2c-0: ffff000075401000 = [2e 2e 2f 2e 2e 2f 2e 2e 2f
>> 64 65 76 69 63 65 73 2f 70 6c 61 74 66 6f 72 6d 2f 73 6f 63 2f 32 31 38 30 30 30 30
>> 2e 69 32 63 2f 69 32 63 2d 30 2f 30 2d 30 30 35 34 2f 30 2d 30 30 35 34 30 00 00]
>> [ 309.024649] i2c i2c-0: ffff000809380080 0x0000000889380080
>> 0x00000000f5401800 ffff000075401800
>> >>>> [ 309.033270] src= 2180004 dst=f5401800 attr= 0 soff= 0 nbytes=1
>> slast= 0
>> >>>> [ 309.033270] citer= 7e biter= 7e doff= 1 dlast_sga= 0
>> >>>> [ 309.033270] major_int=1 disable_req=1 enable_sg=0 [ 309.051633]
>> >>>> fsl-edma 2c00000.edma: vchan 000000001b4371fc: txd
>> >>>> 00000000d9dd26c5[5]: submitted [ 309.083526] fsl-edma
>> >>>> 2c00000.edma: txd 00000000d9dd26c5[5]: marked complete [
>> >>>> 309.090807] i2c i2c-0: ffff000809380080 = [00 00 00 00 00 00 00 00
>> >>>> 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
>> >>>> 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
>> >>>> 00 00 00 00 00 00 00 00 00 00 00 00] [ 309.111694] i2c i2c-0:
>> >>>> ffff000075401800 = [00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
>> >>>> 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
>> >>>> 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
>> >>>> 00 00 00 00]
>> >>>> 00000000 2e 2e 2f 2e 2e 2f 2e 2e 2f 64 65 76 69 63 65 73
>> >>>> |../../../devices|
>> >>>> 00000010 2f 70 6c 61 74 66 6f 72 6d 2f 73 6f 63 2f 32 31
>> >>>> |/platform/soc/21|
>> >>>> 00000020 38 30 30 30 30 2e 69 32 63 2f 69 32 63 2d 30 2f
>> >>>> |80000.i2c/i2c-0/|
>> >>>> 00000030 30 2d 30 30 35 34 2f 30 2d 30 30 35 34 30 00 00
>> >>>> |0-0054/0-00540..|
>> >>>> 00000040 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
>> >>>> |................|
>> >>>> *
>> >>>> 00000070 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ff ff
>> >>>> |................|
>> >>>> 00000080 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
>> >>>> |................|
>> >>>> *
>> >>>> 000000f0 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ff 5b
>> >>>> |...............[|
>> >>>> 00000100
>> >>>>
>> >>>> (patch with my debug prints appended below)
>> >>>>
>> >>>> Despite the DMA completing successfully, no data was copied into
>> >>>> the buffer, leaving the original (now junk) contents. I probed the
>> >>>> I2C bus with an oscilloscope, and I verified that the transfer did indeed
>> occur.
>> >>>> The timing between submission and completion seems reasonable for
>> >>>> the bus speed (50 kHz for whatever reason).
>> >>>>
>> >>>> I had a look over the I2C driver, and nothing looked obviously
>> >>>> incorrect. If anyone has ideas on what to try, I'm more than willing.
>> >>>
>> >>> Is the DMA controller cache-coherent? I see the mainline LS1046A DT
>> doesn't have a "dma-coherent" property for it, but the behaviour is entirely
>> consistent with that being wrong - dma_map_single() cleans the cache,
>> coherent DMA write hits the still-present cache lines, dma_unmap_single()
>> invalidates the cache, and boom, the data is gone and you read back the
>> previous content of the buffer that was cleaned out to DRAM beforehand.
>> >>
>> >> I've tried both with and without [1] applied. I also tried removing
>> >> the call to dma_unmap_single, but to no effect.
>> >
>> > Actually, I wasn't updating my device tree like I thought...
>> >
>> > Turns out I2C works only *without* this patch.
>> >
>> > So maybe the eDMA is not coherent?
>>
>> It seems like this might be the case. From the reference manual:
>>
>> > All transactions from eDMA are tagged as snoop configuration if the
>> > SCFG_SNPCNFGCR[eDMASNP] bit is set. Refer Snoop Configuration
>> Register
>> > (SCFG_SNPCNFGCR) for details.
>>
>> But there is no such bit in this register on the LS1046A. On the LS1043A, this
>> bit does exist, but on the LS1046A it is reserved. I read the register, and
>> found the bit was 0. Perhaps eDMA was intended to be coherent, but there
>> was a hardware bug?
>
> Thanks for the findings. I don't know the reason why this bit is reserved on LS1046a either. It should have a similar design as LS1043a.
Funnily enough, this bit is not set for the LS1043A either [1]. So maybe
this is a U-Boot bug? I'll test this tomorrow.
[1] https://source.denx.de/u-boot/u-boot/-/blob/master/arch/arm/cpu/armv8/fsl-layerscape/soc.c#L680
>>
>> If this is the case, then I think dma-coherent should be left out of the top-
>> level /soc node. Instead, the qdma, sata, usb, and emmc nodes should have
>> dma-coherent added.
>>
>> Li/Laurentiu, what are your thoughts?
>
> Then looks like it is not correct to say all devices on the bus is coherent. But as we have the new "dma-noncoherent" property now and most of the devices are actually coherent, we probably can keep the bus as dma-coherent and mark exceptions with dma-noncoherent?
Neat, I didn't know about that.
Yes, that sounds good. For the moment, only i2c0 uses DMA, so we will
only need it there. At some point, someone should send a patch enabling
it for the rest of the I2C devices, as well as the LPUARTs and SPIs.
--Sean
Powered by blists - more mailing lists