linux-kernel - Re: [EXT] Re: [PATCH] i2c: imx: Defer probing if EDMA not available

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Thu, 12 Dec 2019 10:58:57 +0000
From:   Russell King - ARM Linux admin <linux@...linux.org.uk>
To:     Peng Ma <peng.ma@....com>
Cc:     "shawnguo@...nel.org" <shawnguo@...nel.org>,
        "s.hauer@...gutronix.de" <s.hauer@...gutronix.de>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "linux@...pel-privat.de" <linux@...pel-privat.de>,
        Abel Vesa <abel.vesa@....com>,
        Aisheng Dong <aisheng.dong@....com>,
        Anson Huang <anson.huang@....com>,
        Bogdan Florin Vlad <bogdan.vlad@....com>,
        BOUGH CHEN <haibo.chen@....com>,
        Clark Wang <xiaoning.wang@....com>,
        Daniel Baluta <daniel.baluta@....com>,
        Fancy Fang <chen.fang@....com>, Han Xu <han.xu@....com>,
        Horia Geanta <horia.geanta@....com>,
        Iuliana Prodan <iuliana.prodan@....com>,
        Jacky Bai <ping.bai@....com>,
        Joakim Zhang <qiangqing.zhang@....com>,
        Jun Li <jun.li@....com>, Leo Zhang <leo.zhang@....com>,
        Leonard Crestez <leonard.crestez@....com>,
        Mircea Pop <mircea.pop@....com>,
        Mirela Rabulea <mirela.rabulea@....com>,
        Peng Fan <peng.fan@....com>, Peter Chen <peter.chen@....com>,
        Ranjani Vaidyanathan <ranjani.vaidyanathan@....com>,
        Robert Chiras <robert.chiras@....com>,
        Robin Gong <yibin.gong@....com>,
        Shenwei Wang <shenwei.wang@....com>,
        Viorel Suman <viorel.suman@....com>,
        Ying Liu <victor.liu@....com>,
        Zening Wang <zening.wang@....com>,
        "kernel@...gutronix.de" <kernel@...gutronix.de>,
        "festevam@...il.com" <festevam@...il.com>,
        "linux-arm-kernel@...ts.infradead.org" 
        <linux-arm-kernel@...ts.infradead.org>,
        "linux-i2c@...r.kernel.org" <linux-i2c@...r.kernel.org>
Subject: Re: [EXT] Re: [PATCH] i2c: imx: Defer probing if EDMA not available

On Thu, Dec 12, 2019 at 03:09:32AM +0000, Peng Ma wrote:
> Hello Russell,
> 
> Thanks very much for your strict guidance and comments.
> I realized it is hard to us that we want to i2c used edma when edma
> probe after i2c probe.

I have no problem with that aim.  I'm just very concerned by the
proposed implementation, especially when it has already been proven
to cause regressions in the kernel. I seem to remember that the
infinite loop caused other issues, such as the system being unable
to complete booting.

> I look forward to discussing with you as below, if you like.
> Thanks.
> 
> You say I could do this:
> "So, if you want to do this (and yes, I'd also encourage it to be
> conditional on EDMA being built-in, as I2C is commonly used as a way
> to get at RTCs, which are read before kernel modules can be loaded)
> then you MUST move
> i2c_imx_dma_request() before
> i2c_add_numbered_adapter() to avoid the infinite loop."
> 
> Even if I do this, It's hard to avoid the infinite loop of i2c probe caused by EDMA(build-in) initialization failure.

It isn't clear what you mean here.

If EDMA fails to probe (because fsl_edma_probe() returns an error other
than EPROBE_DEFER) then of_dma_find_controller() will return NULL. That
will be propagated down through i2c_imx_dma_request(). This is no
different from the case where EDMA is built as a module. It is also no
different from the case where EDMA hasn't yet been probed.

If i2c_imx_dma_request() is placed after i2c_add_numbered_adapter(),
and EPROBE_DEFER is propagated out of i2c_imx_probe(), then _yes_, it
will cause an infinite loop, because you are replicating the exact
conditions that caused the attempt to propagate i2c_imx_dma_request()'s
return value to be reverted last time - which brought the kernel to a
grinding halt.

If i2c_imx_dma_request() is placed before i2c_add_numbered_adapter(),
then there is no infinite deferred probing loop - yes, i2c_imx_probe()
will be called as a result of other drivers successfully probing, and
each time it will return EPROBE_DEFER, but the _key_ point is that
the action of i2c_imx_probe() will not _self trigger_ the deferred
probing _and_ place itself onto the deferred probe list.

Please, rather than continuing to send emails arguing over this point,
investigate the stated issue with some practical tests:

1. Make i2c_imx_probe() propagate i2c_imx_dma_request()'s return value,
   as it did in the original patch.
2. Build i2c-imx into the kernel.
3. Build edma as a module.
4. Build and test boot the kernel and check what happens.
5. Move i2c_imx_dma_request() before i2c_add_numbered_adapter()
6. Build and test boot the resulting kernel and note any differences.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up
According to speedtest.net: 11.9Mbps down 500kbps up