linux-kernel - Re: [PATCH v1 5/5] driver core: Set fw

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: <33f643b9-9f6d-e59a-64e7-2d2b7d3e71a4@roeck-us.net>
Date:   Wed, 10 Feb 2021 07:10:38 -0800
From:   Guenter Roeck <linux@...ck-us.net>
To:     Saravana Kannan <saravanak@...gle.com>
Cc:     Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        "Rafael J. Wysocki" <rafael@...nel.org>,
        Android Kernel Team <kernel-team@...roid.com>,
        LKML <linux-kernel@...r.kernel.org>,
        Jisheng Zhang <Jisheng.Zhang@...aptics.com>,
        Kevin Hilman <khilman@...libre.com>,
        John Stultz <john.stultz@...aro.org>,
        Nicolas Saenz Julienne <nsaenzjulienne@...e.de>,
        Marc Zyngier <maz@...nel.org>
Subject: Re: [PATCH v1 5/5] driver core: Set fw_devlink=on by default

On 2/10/21 12:20 AM, Saravana Kannan wrote:
> On Tue, Feb 9, 2021 at 9:54 PM Guenter Roeck <linux@...ck-us.net> wrote:
>>
>> On Thu, Dec 17, 2020 at 07:17:03PM -0800, Saravana Kannan wrote:
>>> Cyclic dependencies in some firmware was one of the last remaining
>>> reasons fw_devlink=on couldn't be set by default. Now that cyclic
>>> dependencies don't block probing, set fw_devlink=on by default.
>>>
>>> Setting fw_devlink=on by default brings a bunch of benefits (currently,
>>> only for systems with device tree firmware):
>>> * Significantly cuts down deferred probes.
>>> * Device probe is effectively attempted in graph order.
>>> * Makes it much easier to load drivers as modules without having to
>>>   worry about functional dependencies between modules (depmod is still
>>>   needed for symbol dependencies).
>>>
>>> If this patch prevents some devices from probing, it's very likely due
>>> to the system having one or more device drivers that "probe"/set up a
>>> device (DT node with compatible property) without creating a struct
>>> device for it.  If we hit such cases, the device drivers need to be
>>> fixed so that they populate struct devices and probe them like normal
>>> device drivers so that the driver core is aware of the devices and their
>>> status. See [1] for an example of such a case.
>>>
>>> [1] - https://lore.kernel.org/lkml/CAGETcx9PiX==mLxB9PO8Myyk6u2vhPVwTMsA5NkD-ywH5xhusw@mail.gmail.com/
>>> Signed-off-by: Saravana Kannan <saravanak@...gle.com>
>>
>> This patch breaks nios2 boot tests in qemu. The system gets stuck when
>> trying to reboot. Reverting this patch fixes the problem. Bisect log
>> is attached.
> 
> Thanks for the report Guenter. Can you please try this series?
> https://lore.kernel.org/lkml/20210205222644.2357303-1-saravanak@google.com/
> 

Not this week. I have lots of reviews to complete before the end of the week,
with the 5.12 commit window coming up.

Given the number of problems observed, I personally think that it is way
too early for this patch. We'll have no end of problems if it is applied
to the upstream kernel in the next commit window. Of course, that is just
my personal opinion.

Thanks,
Guenter