linux-kernel - Re: [PATCH 2/3] arm64: defconfig: Select CONFIG_RESET

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]

Message-ID: <CAK8P3a1NoyMGNK90WzbgJ7opPbiVL8vwnMEuBeeNPNyvB+5sqQ@mail.gmail.com>
Date:   Mon, 29 Jun 2020 14:30:45 +0200
From:   Arnd Bergmann <arnd@...db.de>
To:     Anson Huang <anson.huang@....com>
Cc:     Russell King - ARM Linux <linux@...linux.org.uk>,
        Shawn Guo <shawnguo@...nel.org>,
        Sascha Hauer <s.hauer@...gutronix.de>,
        Sascha Hauer <kernel@...gutronix.de>,
        Fabio Estevam <festevam@...il.com>,
        Catalin Marinas <catalin.marinas@....com>,
        Will Deacon <will@...nel.org>,
        Philipp Zabel <p.zabel@...gutronix.de>,
        Stefan Agner <stefan.agner@...adex.com>,
        "oleksandr.suvorov@...adex.com" <oleksandr.suvorov@...adex.com>,
        Bjorn Andersson <bjorn.andersson@...aro.org>,
        Leo Li <leoyang.li@....com>, Vinod Koul <vkoul@...nel.org>,
        Geert Uytterhoeven <geert+renesas@...der.be>,
        Olof Johansson <olof@...om.net>,
        Linux ARM <linux-arm-kernel@...ts.infradead.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        dl-linux-imx <linux-imx@....com>
Subject: Re: [PATCH 2/3] arm64: defconfig: Select CONFIG_RESET_IMX7 by default

On Mon, Jun 29, 2020 at 2:09 PM Anson Huang <anson.huang@....com> wrote:
> >
> > It sounds like your patch 1/3 is not ready to be merged then.
> >
> > Please make sure that loading it later does not break other drivers that depend
> > on it. Other drivers don't have to be able to deal with missing dependencies if
> > this one is never loaded or disabled at compile-time. However before you
> > make it possible to turn this into a loadable module, anything that depends on
> > it must be able to deal with the modules getting loaded in a random order
> > first.
> >
> I searched all driver which uses this reset driver, looks like ONLY i.MX6 PCIe is using it and
> it ONLY supports built-in.

Ok, thanks for researching this.

> and inside this driver, it does NOT support defer probe etc.,
> since I am NOT sure when this PCIe driver will add module support, so do you think if I
> can make PCI_IMX6 select RESET_IMX7, then it won't break the PCIe function even
> RESET_IMX7 is set to =m in defconfig, as when PCI_IMX6 is enabled as =y, RESET_IMX7 will
> be also =y.

Yes, I think this can work as a short-term workaround, though ideally the
PCIe driver would also become a loadable module and also support deferred
probing.

Having loadable PCIe drivers has traditionally been problematic in Linux,
but Rob Herring has recently improved this in the series containing patch
0c59c06a7c90 ("PCI: host-generic: Support building as modules"), which
was also intended to help with Android GKI.

As i.MX uses the designware PCI core support, this may require some
more changes in PCIE_DW before the i.MX specific part can be
a loadable module, but it should no longer require changes to the
PCI core code. Turning the driver into a loadable module is probably
not even that hard, but making it possible to unload definitely requires
adding a proper .remove callback to properly unregister the PCIe
host bridge.

I also see a DECLARE_PCI_FIXUP() and a fault handler hook in
the pci-imx6.c driver, which probably need to get moved into
a separate built-in file with a few changes.

And then I noticed a bug in the driver: it hooks the abort handler
from an initcall whenever the driver is built into the kernel, regardless
of which machine it is actually running on!

Regarding deferrer probing, the PCIe host currently relies on
the clk controller,  the regulator and (on imx7d) the reset driver
to be probed first. I think making it support deferred probing for
all three should be straightforward, most importantly this means
not printing an error and returning -EPROBE_DEFER when not
all dependencies are there yet.

       Arnd