| 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: <20150829183802.480b1425@tom-T450>
Date: Sat, 29 Aug 2015 18:38:02 +0800
From: Ming Lei <ming.lei@...onical.com>
To: Arend van Spriel <arend@...adcom.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 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;
+
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