linux-kernel - Re: [PATCH v9 1/5] firmware: add extensible driver data params

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 for Android: free password hash cracker in your pocket

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

Date:   Tue, 13 Jun 2017 16:12:17 +0200
From:   Rafał Miłecki <rafal@...ecki.pl>
To:     Greg KH <gregkh@...uxfoundation.org>
Cc:     "Luis R. Rodriguez" <mcgrof@...nel.org>, wagi@...om.org,
        dwmw2@...radead.org, arend.vanspriel@...adcom.com,
        rjw@...ysocki.net, yi1.li@...ux.intel.com,
        atull@...nsource.altera.com, moritz.fischer@...us.com,
        pmladek@...e.com, johannes.berg@...el.com,
        emmanuel.grumbach@...el.com, luciano.coelho@...el.com,
        kvalo@...eaurora.org, luto@...nel.org,
        torvalds@...ux-foundation.org, keescook@...omium.org,
        takahiro.akashi@...aro.org, dhowells@...hat.com, pjones@...hat.com,
        hdegoede@...hat.com, alan@...ux.intel.com, tytso@....edu,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH v9 1/5] firmware: add extensible driver data params

On 06/13/2017 03:17 PM, Greg KH wrote:
> On Tue, Jun 13, 2017 at 12:31:04PM +0200, Rafał Miłecki wrote:
>> On 2017-06-13 11:05, Greg KH wrote:
>>> On Mon, Jun 05, 2017 at 02:39:33PM -0700, Luis R. Rodriguez wrote:
>>>> As the firmware API evolves we keep extending functions with more
>>>> arguments.
>>>> Stop this nonsense by proving an extensible data structure which can
>>>> be used
>>>> to represent both user parameters and private internal parameters.
>>>
>>> Let's take a simple C function interface and make it a more complex
>>> data-driven interface that is impossible to understand and obviously
>>> understand how it is to be used and works!
>>>
>>> :(
>>>
>>> Seriously, why?  Why are we extending any of this at all?  This series
>>> adds a ton of new "features" and complexity, but for absolutely no gain.
>>>
>>> Oh, I take it back, you removed 29 lines from the iwlwifi driver.
>>>
>>> That's still not worth it at all, you have yet to sell me on this whole
>>> complex beast.  I can't see why we need it, and if I, one of the few
>>> people who thinks they actually understand this kernel interface, can't
>>> see it, how can you sell it to someone else?
>>>
>>> Sorry, but no, I'm still not going to take this series until you show
>>> some _REAL_ benefit for it.
>>
>> FWIW I saw (or maybe still see?) a need to extend request_firmware* API to
>> allow silencing a warning if firmware file is missing.
>>
>> I even sent a trivial patch adding support for this:
>> [PATCH V4 1/2] firmware: add more flexible request_firmware_async function
>> https://patchwork.kernel.org/patch/9588787/
>> (I think it still applies) but it got rejected due to Luis's big rework.
>
> Can you resend this series if it still does apply?

Sure, if you think it's worth trying, I'll do that!


> And what exact warning is this silencing?  Normally we want the warning
> there, as that implies that something is wrong if the firmware file that
> a driver is asking for is not present.  That way the user can know to go
> fix it up, right?

It's because brcmfmac looks for NVRAM in two places: /lib/firmware/ and
platform NVRAM. It's supposed to silence
[   10.801506] brcmfmac 0000:01:00.0: Direct firmware load for brcm/brcmfmac43602-pcie.txt failed with error -2
in case there is platform NVRAM present.

For more details please take a look at:
[PATCH V4 2/2] brcmfmac: don't warn user about NVRAM if fallback to platform one succeeds
https://patchwork.kernel.org/patch/9588791/