Re: [RFC] firmware: annotate thou shalt not request fw on init or probe

[Luis R. Rodriguez]

On Thu, Sep 03, 2015 at 07:56:33AM +0200, Julia Lawall wrote:
> 
> 
> On Thu, 3 Sep 2015, Luis R. Rodriguez wrote:
> 
> > On Sat, Aug 29, 2015 at 06:18:20PM +0200, Julia Lawall wrote:
> > > > +@ defines_module_init exists @
> > > > +declarer name module_init;
> > > > +identifier init;
> > > > +@@
> > > > +
> > > > +module_init(init);
> > > > +
> > > > +@ has_probe depends on defines_module_init @
> > > > +identifier drv_calls, drv_probe;
> > > > +type bus_driver;
> > > > +identifier probe_op =~ "(probe)";
> > > > +@@
> > > > +
> > > > +bus_driver drv_calls = {
> > > > +   .probe_op = drv_probe,
> > > > +};
> > > 
> > > I'm not sure that this is enough.  For example, there is the macro
> > > platform_driver_probe that initializes probe fields.  There is likewise
> > > module_platform_driver, which is a top-level declaration that encapsulates
> > > the module_init and the definition of the module_init function, which in
> > > turn calls platform_driver_probe.  There is also module_platform_driver,
> > > which encapsulates the module_init, but not the initialization of the 
> > > probe
> > > field.  Are you concerned with any of these cases?
> > 
> > Yes, and also it would seem this would only capture simple one level of
> > routine indirection, for instance if probe called bar() and it was within
> > bar() that the driver code called a fw request call, that would not be 
> > picked
> > up, correct?
> 
> By default, Coccinelle is not interprocedural.  You can encode that in the 
> script, though.
> 
> Probably the most convenient approach would be to start with the the call, 
> and then work backward to the entry point.  I have code to do this, if and 
> when it turns out to be useful.

FYI folks, thanks to Coccinelle 1.0.5 this is now easily possible with
Python integration, a follow up patch will be submitted that uses this
mechanism to do a proper full search on the kernel.

  Luis

Re: [RFC] firmware: annotate thou shalt not request fw on init or probe

[Luis R. Rodriguez]

On Thu, Sep 03, 2015 at 07:56:33AM +0200, Julia Lawall wrote:
> 
> 
> On Thu, 3 Sep 2015, Luis R. Rodriguez wrote:
> 
> > On Sat, Aug 29, 2015 at 06:18:20PM +0200, Julia Lawall wrote:
> > > > +@ defines_module_init exists @
> > > > +declarer name module_init;
> > > > +identifier init;
> > > > +@@
> > > > +
> > > > +module_init(init);
> > > > +
> > > > +@ has_probe depends on defines_module_init @
> > > > +identifier drv_calls, drv_probe;
> > > > +type bus_driver;
> > > > +identifier probe_op =~ "(probe)";
> > > > +@@
> > > > +
> > > > +bus_driver drv_calls = {
> > > > +   .probe_op = drv_probe,
> > > > +};
> > > 
> > > I'm not sure that this is enough.  For example, there is the macro
> > > platform_driver_probe that initializes probe fields.  There is likewise
> > > module_platform_driver, which is a top-level declaration that encapsulates
> > > the module_init and the definition of the module_init function, which in
> > > turn calls platform_driver_probe.  There is also module_platform_driver,
> > > which encapsulates the module_init, but not the initialization of the 
> > > probe
> > > field.  Are you concerned with any of these cases?
> > 
> > Yes, and also it would seem this would only capture simple one level of
> > routine indirection, for instance if probe called bar() and it was within
> > bar() that the driver code called a fw request call, that would not be 
> > picked
> > up, correct?
> 
> By default, Coccinelle is not interprocedural.  You can encode that in the 
> script, though.
> 
> Probably the most convenient approach would be to start with the the call, 
> and then work backward to the entry point.  I have code to do this, if and 
> when it turns out to be useful.

FYI folks, thanks to Coccinelle 1.0.5 this is now easily possible with
Python integration, a follow up patch will be submitted that uses this
mechanism to do a proper full search on the kernel.

  Luis

Re: [Cocci] [RFC] firmware: annotate thou shalt not request fw on init or probe

[SF Markus Elfring]

> +@ calls_fw_on_init depends on defines_module_init @
> +identifier defines_module_init.init;
> +position p1;
> +@@
> +
> +init(void)
> +{
> + ...

Would it make sense to use the specification "when any"
for such a SmPL ellipsis?
http://coccinelle.lip6.fr/docs/main_grammar004.html


> +(
> +request_firmware@p1(...)
> +|
> +request_firmware_nowait@p1(...)
> +|
> +request_firmware_direct@p1(...)
> +)
> +...
> +}


Are there any more source code variants to check today?

Regards,
Markus
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [Cocci] [RFC] firmware: annotate thou shalt not request fw on init or probe

[SF Markus Elfring]

> +@ calls_fw_on_init depends on defines_module_init @
> +identifier defines_module_init.init;
> +position p1;
> +@@
> +
> +init(void)
> +{
> + ...

Would it make sense to use the specification "when any"
for such a SmPL ellipsis?
http://coccinelle.lip6.fr/docs/main_grammar004.html


> +(
> +request_firmware@p1(...)
> +|
> +request_firmware_nowait@p1(...)
> +|
> +request_firmware_direct@p1(...)
> +)
> +...
> +}


Are there any more source code variants to check today?

Regards,
Markus
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [RFC] firmware: annotate thou shalt not request fw on init or probe

[Julia Lawall]

On Thu, 3 Sep 2015, Luis R. Rodriguez wrote:

> On Sat, Aug 29, 2015 at 06:18:20PM +0200, Julia Lawall wrote:
> > > +@ defines_module_init exists @
> > > +declarer name module_init;
> > > +identifier init;
> > > +@@
> > > +
> > > +module_init(init);
> > > +
> > > +@ has_probe depends on defines_module_init @
> > > +identifier drv_calls, drv_probe;
> > > +type bus_driver;
> > > +identifier probe_op =~ "(probe)";
> > > +@@
> > > +
> > > +bus_driver drv_calls = {
> > > + .probe_op = drv_probe,
> > > +};
> > 
> > I'm not sure that this is enough.  For example, there is the macro
> > platform_driver_probe that initializes probe fields.  There is likewise
> > module_platform_driver, which is a top-level declaration that encapsulates
> > the module_init and the definition of the module_init function, which in
> > turn calls platform_driver_probe.  There is also module_platform_driver,
> > which encapsulates the module_init, but not the initialization of the probe
> > field.  Are you concerned with any of these cases?
> 
> Yes, and also it would seem this would only capture simple one level of
> routine indirection, for instance if probe called bar() and it was within
> bar() that the driver code called a fw request call, that would not be picked
> up, correct?

By default, Coccinelle is not interprocedural.  You can encode that in the 
script, though.

Probably the most convenient approach would be to start with the the call, 
and then work backward to the entry point.  I have code to do this, if and 
when it turns out to be useful.

julia


> If true then the hunt is yet even more complex. The discussion that prompted 
> me
> to send this is still unfolding though [0] and it seems we may want to allow
> for these type of calls within probe in the end but in order to vet for 
> drivers
> that fw is available through the direct filesystem lookup we may need help 
> from
> userspace. As that discussion unfolds it will be good to keep in mind what
> effort we'd need to hunt all users down for now.
> 
> [0] 
> http://lkml.kernel.org/r/CAB=NE6UBRa0K7=PomJzKxsoj4GzAqkYrkp=o+ufvvu2fwm2...@mail.gmail.com
> 
>   Luis
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [RFC] firmware: annotate thou shalt not request fw on init or probe

[Luis R. Rodriguez]

On Tue, Sep 01, 2015 at 01:28:27PM -0400, Josh Boyer wrote:
> On Fri, Aug 28, 2015 at 9:18 PM, Luis R. Rodriguez
>  wrote:
> > From: "Luis R. Rodriguez" 
> >
> > We are phasing out the usermode helper from the kernel,
> > systemd already ripped support for this a while ago, the
> > only remaining valid user is the Dell rbu driver. The
> 
> Actually, I don't think that is accurate any longer.  The Dell rbu
> driver certainly requires this, but it seems that the following
> drivers select FW_LOADER_USER_HELPER_FALLBACK as well:
> 
> CONFIG_DRM_STI
> CONFIG_LEDS_LP55XX_COMMON
> 
> Why they do that, I have no idea.  It's kind of disappointing.

What The Flying-hipster-typewriters! Indeed. With more reason to
put the user mode helper to sleep sooner rather than later.

  Luis
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [RFC] firmware: annotate thou shalt not request fw on init or probe

[Luis R. Rodriguez]

On Sat, Aug 29, 2015 at 06:18:20PM +0200, Julia Lawall wrote:
> > +@ defines_module_init exists @
> > +declarer name module_init;
> > +identifier init;
> > +@@
> > +
> > +module_init(init);
> > +
> > +@ has_probe depends on defines_module_init @
> > +identifier drv_calls, drv_probe;
> > +type bus_driver;
> > +identifier probe_op =~ "(probe)";
> > +@@
> > +
> > +bus_driver drv_calls = {
> > +   .probe_op = drv_probe,
> > +};
> 
> I'm not sure that this is enough.  For example, there is the macro
> platform_driver_probe that initializes probe fields.  There is likewise
> module_platform_driver, which is a top-level declaration that encapsulates
> the module_init and the definition of the module_init function, which in
> turn calls platform_driver_probe.  There is also module_platform_driver,
> which encapsulates the module_init, but not the initialization of the probe
> field.  Are you concerned with any of these cases?

Yes, and also it would seem this would only capture simple one level of
routine indirection, for instance if probe called bar() and it was within
bar() that the driver code called a fw request call, that would not be picked
up, correct?

If true then the hunt is yet even more complex. The discussion that prompted me
to send this is still unfolding though [0] and it seems we may want to allow
for these type of calls within probe in the end but in order to vet for drivers
that fw is available through the direct filesystem lookup we may need help from
userspace. As that discussion unfolds it will be good to keep in mind what
effort we'd need to hunt all users down for now.

[0] 
http://lkml.kernel.org/r/CAB=NE6UBRa0K7=PomJzKxsoj4GzAqkYrkp=o+ufvvu2fwm2...@mail.gmail.com

  Luis
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [RFC] firmware: annotate thou shalt not request fw on init or probe

[Luis R. Rodriguez]

On Sat, Aug 29, 2015 at 06:18:20PM +0200, Julia Lawall wrote:
> > +@ defines_module_init exists @
> > +declarer name module_init;
> > +identifier init;
> > +@@
> > +
> > +module_init(init);
> > +
> > +@ has_probe depends on defines_module_init @
> > +identifier drv_calls, drv_probe;
> > +type bus_driver;
> > +identifier probe_op =~ "(probe)";
> > +@@
> > +
> > +bus_driver drv_calls = {
> > +   .probe_op = drv_probe,
> > +};
> 
> I'm not sure that this is enough.  For example, there is the macro
> platform_driver_probe that initializes probe fields.  There is likewise
> module_platform_driver, which is a top-level declaration that encapsulates
> the module_init and the definition of the module_init function, which in
> turn calls platform_driver_probe.  There is also module_platform_driver,
> which encapsulates the module_init, but not the initialization of the probe
> field.  Are you concerned with any of these cases?

Yes, and also it would seem this would only capture simple one level of
routine indirection, for instance if probe called bar() and it was within
bar() that the driver code called a fw request call, that would not be picked
up, correct?

If true then the hunt is yet even more complex. The discussion that prompted me
to send this is still unfolding though [0] and it seems we may want to allow
for these type of calls within probe in the end but in order to vet for drivers
that fw is available through the direct filesystem lookup we may need help from
userspace. As that discussion unfolds it will be good to keep in mind what
effort we'd need to hunt all users down for now.

[0] 
http://lkml.kernel.org/r/CAB=NE6UBRa0K7=PomJzKxsoj4GzAqkYrkp=o+ufvvu2fwm2...@mail.gmail.com

  Luis
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [RFC] firmware: annotate thou shalt not request fw on init or probe

[Luis R. Rodriguez]

On Tue, Sep 01, 2015 at 01:28:27PM -0400, Josh Boyer wrote:
> On Fri, Aug 28, 2015 at 9:18 PM, Luis R. Rodriguez
>  wrote:
> > From: "Luis R. Rodriguez" 
> >
> > We are phasing out the usermode helper from the kernel,
> > systemd already ripped support for this a while ago, the
> > only remaining valid user is the Dell rbu driver. The
> 
> Actually, I don't think that is accurate any longer.  The Dell rbu
> driver certainly requires this, but it seems that the following
> drivers select FW_LOADER_USER_HELPER_FALLBACK as well:
> 
> CONFIG_DRM_STI
> CONFIG_LEDS_LP55XX_COMMON
> 
> Why they do that, I have no idea.  It's kind of disappointing.

What The Flying-hipster-typewriters! Indeed. With more reason to
put the user mode helper to sleep sooner rather than later.

  Luis
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [RFC] firmware: annotate thou shalt not request fw on init or probe

[Julia Lawall]

On Thu, 3 Sep 2015, Luis R. Rodriguez wrote:

> On Sat, Aug 29, 2015 at 06:18:20PM +0200, Julia Lawall wrote:
> > > +@ defines_module_init exists @
> > > +declarer name module_init;
> > > +identifier init;
> > > +@@
> > > +
> > > +module_init(init);
> > > +
> > > +@ has_probe depends on defines_module_init @
> > > +identifier drv_calls, drv_probe;
> > > +type bus_driver;
> > > +identifier probe_op =~ "(probe)";
> > > +@@
> > > +
> > > +bus_driver drv_calls = {
> > > + .probe_op = drv_probe,
> > > +};
> > 
> > I'm not sure that this is enough.  For example, there is the macro
> > platform_driver_probe that initializes probe fields.  There is likewise
> > module_platform_driver, which is a top-level declaration that encapsulates
> > the module_init and the definition of the module_init function, which in
> > turn calls platform_driver_probe.  There is also module_platform_driver,
> > which encapsulates the module_init, but not the initialization of the probe
> > field.  Are you concerned with any of these cases?
> 
> Yes, and also it would seem this would only capture simple one level of
> routine indirection, for instance if probe called bar() and it was within
> bar() that the driver code called a fw request call, that would not be picked
> up, correct?

By default, Coccinelle is not interprocedural.  You can encode that in the 
script, though.

Probably the most convenient approach would be to start with the the call, 
and then work backward to the entry point.  I have code to do this, if and 
when it turns out to be useful.

julia


> If true then the hunt is yet even more complex. The discussion that prompted 
> me
> to send this is still unfolding though [0] and it seems we may want to allow
> for these type of calls within probe in the end but in order to vet for 
> drivers
> that fw is available through the direct filesystem lookup we may need help 
> from
> userspace. As that discussion unfolds it will be good to keep in mind what
> effort we'd need to hunt all users down for now.
> 
> [0] 
> http://lkml.kernel.org/r/CAB=NE6UBRa0K7=PomJzKxsoj4GzAqkYrkp=o+ufvvu2fwm2...@mail.gmail.com
> 
>   Luis
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [RFC] firmware: annotate thou shalt not request fw on init or probe

[Josh Boyer]

On Fri, Aug 28, 2015 at 9:18 PM, Luis R. Rodriguez
 wrote:
> From: "Luis R. Rodriguez" 
>
> We are phasing out the usermode helper from the kernel,
> systemd already ripped support for this a while ago, the
> only remaining valid user is the Dell rbu driver. The

Actually, I don't think that is accurate any longer.  The Dell rbu
driver certainly requires this, but it seems that the following
drivers select FW_LOADER_USER_HELPER_FALLBACK as well:

CONFIG_DRM_STI
CONFIG_LEDS_LP55XX_COMMON

Why they do that, I have no idea.  It's kind of disappointing.

josh
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [RFC] firmware: annotate thou shalt not request fw on init or probe

[Josh Boyer]

On Fri, Aug 28, 2015 at 9:18 PM, Luis R. Rodriguez
 wrote:
> From: "Luis R. Rodriguez" 
>
> We are phasing out the usermode helper from the kernel,
> systemd already ripped support for this a while ago, the
> only remaining valid user is the Dell rbu driver. The

Actually, I don't think that is accurate any longer.  The Dell rbu
driver certainly requires this, but it seems that the following
drivers select FW_LOADER_USER_HELPER_FALLBACK as well:

CONFIG_DRM_STI
CONFIG_LEDS_LP55XX_COMMON

Why they do that, I have no idea.  It's kind of disappointing.

josh
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [RFC] firmware: annotate thou shalt not request fw on init or probe

[Julia Lawall]

> +@ defines_module_init exists @
> +declarer name module_init;
> +identifier init;
> +@@
> +
> +module_init(init);
> +
> +@ has_probe depends on defines_module_init @
> +identifier drv_calls, drv_probe;
> +type bus_driver;
> +identifier probe_op =~ "(probe)";
> +@@
> +
> +bus_driver drv_calls = {
> + .probe_op = drv_probe,
> +};

I'm not sure that this is enough.  For example, there is the macro
platform_driver_probe that initializes probe fields.  There is likewise
module_platform_driver, which is a top-level declaration that encapsulates
the module_init and the definition of the module_init function, which in
turn calls platform_driver_probe.  There is also module_platform_driver,
which encapsulates the module_init, but not the initialization of the probe
field.  Are you concerned with any of these cases?

julia
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [RFC] firmware: annotate thou shalt not request fw on init or probe

[Julia Lawall]

 +@ defines_module_init exists @
 +declarer name module_init;
 +identifier init;
 +@@
 +
 +module_init(init);
 +
 +@ has_probe depends on defines_module_init @
 +identifier drv_calls, drv_probe;
 +type bus_driver;
 +identifier probe_op =~ (probe);
 +@@
 +
 +bus_driver drv_calls = {
 + .probe_op = drv_probe,
 +};

I'm not sure that this is enough.  For example, there is the macro
platform_driver_probe that initializes probe fields.  There is likewise
module_platform_driver, which is a top-level declaration that encapsulates
the module_init and the definition of the module_init function, which in
turn calls platform_driver_probe.  There is also module_platform_driver,
which encapsulates the module_init, but not the initialization of the probe
field.  Are you concerned with any of these cases?

julia
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[RFC] firmware: annotate thou shalt not request fw on init or probe

[Luis R. Rodriguez]

From: "Luis R. Rodriguez" 

We are phasing out the usermode helper from the kernel,
systemd already ripped support for this a while ago, the
only remaining valid user is the Dell rbu driver. The
firmware is now being read directly from the filesystem
by the kernel. What this means is that if you have a
device driver that needs firmware early, when it is
built-in to the kernel the firmware may not yet be
available. Folks building drivers that need firmware
early should either include it as part of the kernel or
stuff it into the initramfs used to boot.

In particular since we are accessing the firmware directly
folks cannot expect new found firmware on a filesystem
after we switch off from an initramfs with pivot_root().
Supporting such dynamic changes to load drivers would
be possible but adds complexity, instead lets document
the expectations properly and add a grammar rule to enable
folks to check / validate / police if drivers are using
the request firmware API early on init or probe.

The SmPL rule used to check for the probe routine is
loose and is currently defined through a regexp, that
can easily be extended to any other known bus probe
routine names.

Thou shalt not make firmware calls early on init or probe.

I spot only 2 offender right now.

mcgrof@ergon ~/linux-next (git::20150805-pend-all)$ export 
COCCI=scripts/coccinelle/api/request_firmware.cocci
mcgrof@ergon ~/linux-next (git::20150805-pend-all)$ make coccicheck MODE=report

Please check for false positives in the output before submitting a patch.
When using "patch" mode, carefully review the patch before submitting
it.

./drivers/fmc/fmc-write-eeprom.c:136:7-23: ERROR: driver call request firmware 
call on its probe routine on line 136.
./drivers/tty/serial/rp2.c:796:6-29: ERROR: driver call request firmware call 
on its probe routine on line 796.

Cc: Ming Lei 
Cc: Jonathan Corbet 
Cc: Julia Lawall 
Cc: Gilles Muller 
Cc: Nicolas Palix 
Cc: Michal Marek 
Cc: linux-...@vger.kernel.org
Cc: co...@systeme.lip6.fr
Cc: Alessandro Rubini 
Cc: Kevin Cernekee 
Cc: Greg Kroah-Hartman 
Cc: Jiri Slaby 
Cc: linux-ser...@vger.kernel.org
Signed-off-by: Luis R. Rodriguez 
---
 Documentation/firmware_class/README   | 24 +--
 drivers/base/Kconfig  |  2 +-
 scripts/coccinelle/api/request_firmware.cocci | 90 +++
 3 files changed, 110 insertions(+), 6 deletions(-)
 create mode 100644 scripts/coccinelle/api/request_firmware.cocci

diff --git a/Documentation/firmware_class/README 
b/Documentation/firmware_class/README
index 71f86859d7d8..7c59f4d07f1d 100644
--- a/Documentation/firmware_class/README
+++ b/Documentation/firmware_class/README
@@ -33,7 +33,7 @@
than 256, user should pass 'firmware_class.path=$CUSTOMIZED_PATH'
if firmware_class is built in kernel(the general situation)
 
- 2), userspace:
+ 2), userspace: (DEPRECATED)
- /sys/class/firmware/xxx/{loading,data} appear.
- hotplug gets called with a firmware identifier in $FIRMWARE
  and the usual hotplug environment.
@@ -41,14 +41,14 @@
 
  3), kernel: Discard any previous partial load.
 
- 4), userspace:
+ 4), userspace: (DEPRECATED)
- hotplug: cat appropriate_firmware_image > \
/sys/class/firmware/xxx/data
 
  5), kernel: grows a buffer in PAGE_SIZE increments to hold the image as it
 comes in.
 
- 6), userspace:
+ 6), userspace: (DEPRECATED)
- hotplug: echo 0 > /sys/class/firmware/xxx/loading
 
  7), kernel: request_firmware() returns and the driver has the firmware
@@ -66,8 +66,8 @@
copy_fw_to_device(fw_entry->data, fw_entry->size);
 release_firmware(fw_entry);
 
- Sample/simple hotplug script:
- 
+ Sample/simple hotplug script: (DEPRECATED)
+ ==
 
# Both $DEVPATH and $FIRMWARE are already provided in the environment.
 
@@ -93,6 +93,20 @@
user contexts to request firmware asynchronously, but can't be called
in atomic contexts.
 
+Requirements:
+=
+
+You should avoid at all costs requesting firmware on both init and probe paths
+of your device driver. Reason for this is as usermod helper functionality is
+being deprecated we will only have direct firmware access. This means that
+any routines requesting firmware will need the filesystem which contains the
+firmware available and mounted. Device drivers init and probe paths can be
+called early on prior to /lib/firmware being available. If you might need
+access to firmware early you should consider requiring your device driver to
+be only available as a module, this however as its own set of limitations.
+
+Folks building drivers that need firmware early should either include it as
+part of the kernel or stuff it into the initramfs used to boot.
 
  about in-kernel persistence:
  ---
diff --git a/drivers/base/Kconfig 

[RFC] firmware: annotate thou shalt not request fw on init or probe

[Luis R. Rodriguez]

From: Luis R. Rodriguez mcg...@suse.com

We are phasing out the usermode helper from the kernel,
systemd already ripped support for this a while ago, the
only remaining valid user is the Dell rbu driver. The
firmware is now being read directly from the filesystem
by the kernel. What this means is that if you have a
device driver that needs firmware early, when it is
built-in to the kernel the firmware may not yet be
available. Folks building drivers that need firmware
early should either include it as part of the kernel or
stuff it into the initramfs used to boot.

In particular since we are accessing the firmware directly
folks cannot expect new found firmware on a filesystem
after we switch off from an initramfs with pivot_root().
Supporting such dynamic changes to load drivers would
be possible but adds complexity, instead lets document
the expectations properly and add a grammar rule to enable
folks to check / validate / police if drivers are using
the request firmware API early on init or probe.

The SmPL rule used to check for the probe routine is
loose and is currently defined through a regexp, that
can easily be extended to any other known bus probe
routine names.

Thou shalt not make firmware calls early on init or probe.

I spot only 2 offender right now.

mcgrof@ergon ~/linux-next (git::20150805-pend-all)$ export 
COCCI=scripts/coccinelle/api/request_firmware.cocci
mcgrof@ergon ~/linux-next (git::20150805-pend-all)$ make coccicheck MODE=report

Please check for false positives in the output before submitting a patch.
When using patch mode, carefully review the patch before submitting
it.

./drivers/fmc/fmc-write-eeprom.c:136:7-23: ERROR: driver call request firmware 
call on its probe routine on line 136.
./drivers/tty/serial/rp2.c:796:6-29: ERROR: driver call request firmware call 
on its probe routine on line 796.

Cc: Ming Lei ming@canonical.com
Cc: Jonathan Corbet cor...@lwn.net
Cc: Julia Lawall julia.law...@lip6.fr
Cc: Gilles Muller gilles.mul...@lip6.fr
Cc: Nicolas Palix nicolas.pa...@imag.fr
Cc: Michal Marek mma...@suse.com
Cc: linux-...@vger.kernel.org
Cc: co...@systeme.lip6.fr
Cc: Alessandro Rubini rub...@gnudd.com
Cc: Kevin Cernekee cerne...@gmail.com
Cc: Greg Kroah-Hartman gre...@linuxfoundation.org
Cc: Jiri Slaby jsl...@suse.com
Cc: linux-ser...@vger.kernel.org
Signed-off-by: Luis R. Rodriguez mcg...@suse.com
---
 Documentation/firmware_class/README   | 24 +--
 drivers/base/Kconfig  |  2 +-
 scripts/coccinelle/api/request_firmware.cocci | 90 +++
 3 files changed, 110 insertions(+), 6 deletions(-)
 create mode 100644 scripts/coccinelle/api/request_firmware.cocci

diff --git a/Documentation/firmware_class/README 
b/Documentation/firmware_class/README
index 71f86859d7d8..7c59f4d07f1d 100644
--- a/Documentation/firmware_class/README
+++ b/Documentation/firmware_class/README
@@ -33,7 +33,7 @@
than 256, user should pass 'firmware_class.path=$CUSTOMIZED_PATH'
if firmware_class is built in kernel(the general situation)
 
- 2), userspace:
+ 2), userspace: (DEPRECATED)
- /sys/class/firmware/xxx/{loading,data} appear.
- hotplug gets called with a firmware identifier in $FIRMWARE
  and the usual hotplug environment.
@@ -41,14 +41,14 @@
 
  3), kernel: Discard any previous partial load.
 
- 4), userspace:
+ 4), userspace: (DEPRECATED)
- hotplug: cat appropriate_firmware_image  \
/sys/class/firmware/xxx/data
 
  5), kernel: grows a buffer in PAGE_SIZE increments to hold the image as it
 comes in.
 
- 6), userspace:
+ 6), userspace: (DEPRECATED)
- hotplug: echo 0  /sys/class/firmware/xxx/loading
 
  7), kernel: request_firmware() returns and the driver has the firmware
@@ -66,8 +66,8 @@
copy_fw_to_device(fw_entry-data, fw_entry-size);
 release_firmware(fw_entry);
 
- Sample/simple hotplug script:
- 
+ Sample/simple hotplug script: (DEPRECATED)
+ ==
 
# Both $DEVPATH and $FIRMWARE are already provided in the environment.
 
@@ -93,6 +93,20 @@
user contexts to request firmware asynchronously, but can't be called
in atomic contexts.
 
+Requirements:
+=
+
+You should avoid at all costs requesting firmware on both init and probe paths
+of your device driver. Reason for this is as usermod helper functionality is
+being deprecated we will only have direct firmware access. This means that
+any routines requesting firmware will need the filesystem which contains the
+firmware available and mounted. Device drivers init and probe paths can be
+called early on prior to /lib/firmware being available. If you might need
+access to firmware early you should consider requiring your device driver to
+be only available as a module, this however as its own set of limitations.
+
+Folks building drivers that need firmware