| 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: <55E2BE0E.1070907@broadcom.com>
Date: Sun, 30 Aug 2015 10:25:50 +0200
From: Arend van Spriel <arend@...adcom.com>
To: Ming Lei <ming.lei@...onical.com>
CC: Takashi Iwai <tiwai@...e.de>,
"Luis R. Rodriguez" <mcgrof@...e.com>,
"Linus Torvalds" <torvalds@...ux-foundation.org>,
Liam Girdwood <liam.r.girdwood@...ux.intel.com>,
"Jie, Yang" <yang.jie@...el.com>,
"Dmitry Torokhov" <dmitry.torokhov@...il.com>,
"joonas.lahtinen@...ux.intel.com" <joonas.lahtinen@...ux.intel.com>,
Tom Gundersen <teg@...m.no>, Al Viro <viro@...iv.linux.org.uk>,
Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
Kay Sievers <kay@...y.org>,
David Woodhouse <dwmw2@...radead.org>,
"Luis Rodriguez" <mcgrof@...not-panic.com>,
lkml <linux-kernel@...r.kernel.org>,
yalin wang <yalin.wang2010@...il.com>, <tom.leiming@...il.com>
Subject: Re: Problems loading firmware using built-in drivers with kernels
that use initramfs.
On 08/29/2015 12:38 PM, Ming Lei wrote:
> On Sat, 29 Aug 2015 10:50:22 +0200
> Arend van Spriel <arend@...adcom.com> wrote:
>
>> On 08/29/2015 09:11 AM, Takashi Iwai wrote:
>>> On Sat, 29 Aug 2015 06:09:01 +0200,
>>> Ming Lei wrote:
>>>>
>>>> On Sat, Aug 29, 2015 at 9:11 AM, Luis R. Rodriguez <mcgrof@...e.com> wrote:
>>>>> On Thu, Aug 27, 2015 at 08:55:13AM +0800, Ming Lei wrote:
>>>>>> On Thu, Aug 27, 2015 at 2:07 AM, Linus Torvalds
>>>>>> <torvalds@...ux-foundation.org> wrote:
>>>>>>> On Wed, Aug 26, 2015 at 1:06 AM, Liam Girdwood
>>>>>>> <liam.r.girdwood@...ux.intel.com> wrote:
>>>>>>>>
>>>>>>>> I think the options are to either :-
>>>>>>>>
>>>>>>>> 1) Don not support audio DSP drivers using topology data as built-in
>>>>>>>> drivers. Audio is not really a critical system required for booting
>>>>>>>> anyway.
>>>>>>>
>>>>>>> Yes, forcing it to be a module and not letting people compile it in by
>>>>>>> mistake (and then not have it work) is an option.
>>>>>>>
>>>>>>> That said, there are situations where people don't want to use
>>>>>>> modules. I used to eschew them for security reasons, for example - now
>>>>>>> I instead just do a one-time temporary key. But others may have other
>>>>>>> reasons to try to avoid modules.
>>>>>>>
>>>>>>>> 2) Create a default PCM for every driver that has topology data on the
>>>>>>>> assumption that every sound card will at least 1 PCM. This PCM can then
>>>>>>>> be re-configured when the FW is loaded.
>>>>>>>
>>>>>>> That would seem to be the better option if it is reasonably implementable.
>>>>>>>
>>>>>>> Of course, some kind of timer-based retry (limited *somehow*) of the
>>>>>>> fw loading could work too, but smells really really hacky.
>>>>>>
>>>>>> Yeah, years ago, we discussed to use -EPROBE_DEFER for the situation,
>>>>>> which should be one kind of fix, but looks there were objections at that time.
>>>>>
>>>>> That would still be a hack. I'll note there is also asynchronous probe support
>>>>> now but to use that would also be a hack for this issue. We don't want to
>>>>
>>>> If we think firmware as one kind of resources like regulators, gpio and others,
>>>> PROBE_DEFER is one good match for firmware loading case, and
>>>> it has been used by lots of drivers, so why can't it be used for
>>>> firmware loading?
>>>>
>>>> One problem is that we need to convert drivers into returning -EPROBE_DEFER
>>>> in case of request failure, and that may involve some work, but which
>>>> should be mechanical.
>>>
>>> I find such a delaying mechanism not so bad, too. It's very
>>> straightforward, at least, no big pain in the transition in the driver
>>> side.
>>
>> Not sure how this is going to work with request_firmware_nowait(). We
>> use that in our drivers to get rid of ~60 sec. delay in probe and
>> consequently boot time when built-in. So basically we return 0 on probe
>> lacking better knowledge. Guess we can always move back to
>> request_firmware calls when defer_probe support is available.
>
> How about the following untested draft patch?
>
> diff --git a/drivers/base/dd.c b/drivers/base/dd.c
> index be0eb46..f66912f 100644
> --- a/drivers/base/dd.c
> +++ b/drivers/base/dd.c
> @@ -171,6 +171,12 @@ static void driver_deferred_probe_trigger(void)
> queue_work(deferred_wq, &deferred_probe_work);
> }
>
> +void driver_trigger_fw_load()
> +{
> + driver_deferred_probe_trigger();
> +}
> +EXPORT_SYMBOL_GPL(driver_trigger_fw_load);
> +
> /**
> * deferred_probe_initcall() - Enable probing of deferred devices
> *
> diff --git a/drivers/base/firmware_class.c b/drivers/base/firmware_class.c
> index 8524450..f879a07 100644
> --- a/drivers/base/firmware_class.c
> +++ b/drivers/base/firmware_class.c
> @@ -1132,6 +1132,11 @@ _request_firmware(const struct firmware **firmware_p, const char *name,
> if (ret <= 0) /* error or already assigned */
> goto out;
>
> + if (system_state == SYSTEM_BOOTING) {
> + ret = -EPROBE_DEFER;
> + goto out;
> + }
> +
> ret = 0;
> timeout = firmware_loading_timeout();
> if (opt_flags & FW_OPT_NOWAIT) {
> @@ -1311,6 +1316,9 @@ request_firmware_nowait(
> {
> struct firmware_work *fw_work;
>
> + if (system_state == SYSTEM_BOOTING)
> + return -EPROBE_DEFER;
> +
Does this mean a built-in driver can not get firmware from initramfs or
built in the kernel early. Seems a bit too aggressive. The problem
stated in this thread is when the firmware is not on initramfs but only
on the rootfs.
Regards,
Arend
> fw_work = kzalloc(sizeof(struct firmware_work), gfp);
> if (!fw_work)
> return -ENOMEM;
> diff --git a/include/linux/device.h b/include/linux/device.h
> index 5d7bc63..1c189fe 100644
> --- a/include/linux/device.h
> +++ b/include/linux/device.h
> @@ -289,6 +289,7 @@ extern struct device_driver *driver_find(const char *name,
> struct bus_type *bus);
> extern int driver_probe_done(void);
> extern void wait_for_device_probe(void);
> +extern void driver_trigger_fw_load(void);
>
>
> /* sysfs interface for exporting driver attributes */
> diff --git a/init/main.c b/init/main.c
> index 9e64d70..be8411b 100644
> --- a/init/main.c
> +++ b/init/main.c
> @@ -943,6 +943,9 @@ static int __ref kernel_init(void *unused)
>
> flush_delayed_fput();
>
> + /* trigger probe for request_firmware and its no_wait pair */
> + driver_trigger_fw_load();
> +
> if (ramdisk_execute_command) {
> ret = run_init_process(ramdisk_execute_command);
> if (!ret)
>
>
>
> Thanks,
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists