* [PATCH v3 01/20] test_firmware: add simple firmware firmware test library
2018-03-10 14:14 [PATCH v3 00/20] firmware: development for v4.17 Luis R. Rodriguez
@ 2018-03-10 14:14 ` Luis R. Rodriguez
2018-03-10 14:14 ` [PATCH v3 02/20] test_firmware: enable custom fallback testing on limited kernel configs Luis R. Rodriguez
` (19 subsequent siblings)
20 siblings, 0 replies; 39+ messages in thread
From: Luis R. Rodriguez @ 2018-03-10 14:14 UTC (permalink / raw)
To: gregkh
Cc: akpm, cantabile.desu, kubakici, linux-wireless, keescook, shuah,
mfuzzey, zohar, dhowells, pali.rohar, tiwai, arend.vanspriel,
zajec5, nbroeking, markivx, broonie, dmitry.torokhov, dwmw2,
torvalds, Abhay_Salunke, bjorn.andersson, jewalt, oneukum, ast,
andresx7, linux-kernel, linux-fsdevel, Luis R. Rodriguez
We'll expland on this later, for now just add basic module checker.
While at it, move this all to use /bin/bash as we'll have much more
flexibility with it.
Signed-off-by: Luis R. Rodriguez <mcgrof@kernel.org>
---
tools/testing/selftests/firmware/fw_fallback.sh | 7 ++--
tools/testing/selftests/firmware/fw_filesystem.sh | 20 ++---------
tools/testing/selftests/firmware/fw_lib.sh | 44 +++++++++++++++++++++++
3 files changed, 51 insertions(+), 20 deletions(-)
create mode 100755 tools/testing/selftests/firmware/fw_lib.sh
diff --git a/tools/testing/selftests/firmware/fw_fallback.sh b/tools/testing/selftests/firmware/fw_fallback.sh
index 722cad91df74..755147a8c967 100755
--- a/tools/testing/selftests/firmware/fw_fallback.sh
+++ b/tools/testing/selftests/firmware/fw_fallback.sh
@@ -1,4 +1,4 @@
-#!/bin/sh
+#!/bin/bash
# SPDX-License-Identifier: GPL-2.0
# This validates that the kernel will fall back to using the fallback mechanism
# to load firmware it can't find on disk itself. We must request a firmware
@@ -6,9 +6,10 @@
# won't find so that we can do the load ourself manually.
set -e
-modprobe test_firmware
+TEST_DIR=$(dirname $0)
+source $TEST_DIR/fw_lib.sh
-DIR=/sys/devices/virtual/misc/test_firmware
+check_mods
# CONFIG_FW_LOADER_USER_HELPER has a sysfs class under /sys/class/firmware/
# These days no one enables CONFIG_FW_LOADER_USER_HELPER so check for that
diff --git a/tools/testing/selftests/firmware/fw_filesystem.sh b/tools/testing/selftests/firmware/fw_filesystem.sh
index f9508e1a4058..748f2f5737e9 100755
--- a/tools/testing/selftests/firmware/fw_filesystem.sh
+++ b/tools/testing/selftests/firmware/fw_filesystem.sh
@@ -1,4 +1,4 @@
-#!/bin/sh
+#!/bin/bash
# SPDX-License-Identifier: GPL-2.0
# This validates that the kernel will load firmware out of its list of
# firmware locations on disk. Since the user helper does similar work,
@@ -6,24 +6,10 @@
# know so we can be sure we're not accidentally testing the user helper.
set -e
-DIR=/sys/devices/virtual/misc/test_firmware
TEST_DIR=$(dirname $0)
+source $TEST_DIR/fw_lib.sh
-test_modprobe()
-{
- if [ ! -d $DIR ]; then
- echo "$0: $DIR not present"
- echo "You must have the following enabled in your kernel:"
- cat $TEST_DIR/config
- exit 1
- fi
-}
-
-trap "test_modprobe" EXIT
-
-if [ ! -d $DIR ]; then
- modprobe test_firmware
-fi
+check_mods
# CONFIG_FW_LOADER_USER_HELPER has a sysfs class under /sys/class/firmware/
# These days most distros enable CONFIG_FW_LOADER_USER_HELPER but disable
diff --git a/tools/testing/selftests/firmware/fw_lib.sh b/tools/testing/selftests/firmware/fw_lib.sh
new file mode 100755
index 000000000000..c14bbca7ecf9
--- /dev/null
+++ b/tools/testing/selftests/firmware/fw_lib.sh
@@ -0,0 +1,44 @@
+#!/bin/bash
+# SPDX-License-Identifier: GPL-2.0
+
+# Library of helpers for test scripts.
+set -e
+
+DIR=/sys/devices/virtual/misc/test_firmware
+
+PROC_CONFIG="/proc/config.gz"
+TEST_DIR=$(dirname $0)
+
+print_reqs_exit()
+{
+ echo "You must have the following enabled in your kernel:" >&2
+ cat $TEST_DIR/config >&2
+ exit 1
+}
+
+test_modprobe()
+{
+ if [ ! -d $DIR ]; then
+ print_reqs_exit
+ fi
+}
+
+check_mods()
+{
+ trap "test_modprobe" EXIT
+ if [ ! -d $DIR ]; then
+ modprobe test_firmware
+ fi
+ if [ ! -f $PROC_CONFIG ]; then
+ if modprobe configs 2>/dev/null; then
+ echo "Loaded configs module"
+ if [ ! -f $PROC_CONFIG ]; then
+ echo "You must have the following enabled in your kernel:" >&2
+ cat $TEST_DIR/config >&2
+ echo "Resorting to old heuristics" >&2
+ fi
+ else
+ echo "Failed to load configs module, using old heuristics" >&2
+ fi
+ fi
+}
--
2.16.2
^ permalink raw reply related [flat|nested] 39+ messages in thread* [PATCH v3 02/20] test_firmware: enable custom fallback testing on limited kernel configs
2018-03-10 14:14 [PATCH v3 00/20] firmware: development for v4.17 Luis R. Rodriguez
2018-03-10 14:14 ` [PATCH v3 01/20] test_firmware: add simple firmware firmware test library Luis R. Rodriguez
@ 2018-03-10 14:14 ` Luis R. Rodriguez
2018-03-10 14:14 ` [PATCH v3 03/20] test_firmware: replace syfs fallback check with kconfig_has helper Luis R. Rodriguez
` (18 subsequent siblings)
20 siblings, 0 replies; 39+ messages in thread
From: Luis R. Rodriguez @ 2018-03-10 14:14 UTC (permalink / raw)
To: gregkh
Cc: akpm, cantabile.desu, kubakici, linux-wireless, keescook, shuah,
mfuzzey, zohar, dhowells, pali.rohar, tiwai, arend.vanspriel,
zajec5, nbroeking, markivx, broonie, dmitry.torokhov, dwmw2,
torvalds, Abhay_Salunke, bjorn.andersson, jewalt, oneukum, ast,
andresx7, linux-kernel, linux-fsdevel, Luis R. Rodriguez
When a kernel is not built with:
CONFIG_HAS_FW_LOADER_USER_HELPER_FALLBACK=y
We don't currently enable testing fw_fallback.sh. For kernels that
still enable the fallback mechanism, its possible to use the async
request firmware API call request_firmware_nowait() using the custom
interface to use the fallback mechanism, so we should be able to test
this but we currently cannot.
We can enable testing without CONFIG_HAS_FW_LOADER_USER_HELPER_FALLBACK=y
by relying on /proc/config.gz (CONFIG_IKCONFIG_PROC), if present. If you
don't have this we'll have no option but to rely on old heuristics for now.
We stuff the new kconfig_has() helper into our shared library as we'll
later expando on its use elsewhere.
Acked-by: Kees Cook <keescook@chromium.org>
Signed-off-by: Luis R. Rodriguez <mcgrof@kernel.org>
---
tools/testing/selftests/firmware/config | 4 ++++
tools/testing/selftests/firmware/fw_fallback.sh | 6 +++++-
tools/testing/selftests/firmware/fw_lib.sh | 24 ++++++++++++++++++++++++
3 files changed, 33 insertions(+), 1 deletion(-)
diff --git a/tools/testing/selftests/firmware/config b/tools/testing/selftests/firmware/config
index c8137f70e291..bf634dda0720 100644
--- a/tools/testing/selftests/firmware/config
+++ b/tools/testing/selftests/firmware/config
@@ -1 +1,5 @@
CONFIG_TEST_FIRMWARE=y
+CONFIG_FW_LOADER=y
+CONFIG_FW_LOADER_USER_HELPER=y
+CONFIG_IKCONFIG=y
+CONFIG_IKCONFIG_PROC=y
diff --git a/tools/testing/selftests/firmware/fw_fallback.sh b/tools/testing/selftests/firmware/fw_fallback.sh
index 755147a8c967..bf850050e5e9 100755
--- a/tools/testing/selftests/firmware/fw_fallback.sh
+++ b/tools/testing/selftests/firmware/fw_fallback.sh
@@ -15,6 +15,7 @@ check_mods
# These days no one enables CONFIG_FW_LOADER_USER_HELPER so check for that
# as an indicator for CONFIG_FW_LOADER_USER_HELPER.
HAS_FW_LOADER_USER_HELPER=$(if [ -d /sys/class/firmware/ ]; then echo yes; else echo no; fi)
+HAS_FW_LOADER_USER_HELPER_FALLBACK=$(kconfig_has CONFIG_FW_LOADER_USER_HELPER_FALLBACK=y)
if [ "$HAS_FW_LOADER_USER_HELPER" = "yes" ]; then
OLD_TIMEOUT=$(cat /sys/class/firmware/timeout)
@@ -287,7 +288,10 @@ run_sysfs_custom_load_tests()
fi
}
-run_sysfs_main_tests
+if [ "$HAS_FW_LOADER_USER_HELPER_FALLBACK" = "yes" ]; then
+ run_sysfs_main_tests
+fi
+
run_sysfs_custom_load_tests
exit 0
diff --git a/tools/testing/selftests/firmware/fw_lib.sh b/tools/testing/selftests/firmware/fw_lib.sh
index c14bbca7ecf9..467567c758b9 100755
--- a/tools/testing/selftests/firmware/fw_lib.sh
+++ b/tools/testing/selftests/firmware/fw_lib.sh
@@ -42,3 +42,27 @@ check_mods()
fi
fi
}
+
+kconfig_has()
+{
+ if [ -f $PROC_CONFIG ]; then
+ if zgrep -q $1 $PROC_CONFIG 2>/dev/null; then
+ echo "yes"
+ else
+ echo "no"
+ fi
+ else
+ # We currently don't have easy heuristics to infer this
+ # so best we can do is just try to use the kernel assuming
+ # you had enabled it. This matches the old behaviour.
+ if [ "$1" = "CONFIG_FW_LOADER_USER_HELPER_FALLBACK=y" ]; then
+ echo "yes"
+ elif [ "$1" = "CONFIG_FW_LOADER_USER_HELPER=y" ]; then
+ if [ -d /sys/class/firmware/ ]; then
+ echo yes
+ else
+ echo no
+ fi
+ fi
+ fi
+}
--
2.16.2
^ permalink raw reply related [flat|nested] 39+ messages in thread* [PATCH v3 03/20] test_firmware: replace syfs fallback check with kconfig_has helper
2018-03-10 14:14 [PATCH v3 00/20] firmware: development for v4.17 Luis R. Rodriguez
2018-03-10 14:14 ` [PATCH v3 01/20] test_firmware: add simple firmware firmware test library Luis R. Rodriguez
2018-03-10 14:14 ` [PATCH v3 02/20] test_firmware: enable custom fallback testing on limited kernel configs Luis R. Rodriguez
@ 2018-03-10 14:14 ` Luis R. Rodriguez
2018-03-10 14:14 ` [PATCH v3 04/20] firmware: enable to split firmware_class into separate target files Luis R. Rodriguez
` (17 subsequent siblings)
20 siblings, 0 replies; 39+ messages in thread
From: Luis R. Rodriguez @ 2018-03-10 14:14 UTC (permalink / raw)
To: gregkh
Cc: akpm, cantabile.desu, kubakici, linux-wireless, keescook, shuah,
mfuzzey, zohar, dhowells, pali.rohar, tiwai, arend.vanspriel,
zajec5, nbroeking, markivx, broonie, dmitry.torokhov, dwmw2,
torvalds, Abhay_Salunke, bjorn.andersson, jewalt, oneukum, ast,
andresx7, linux-kernel, linux-fsdevel, Luis R. Rodriguez
Now that we have a kconfig checker just use that instead of relying
on testing a sysfs directory being present, since our requirements
are spelled out.
Acked-by: Kees Cook <keescook@chromium.org>
Signed-off-by: Luis R. Rodriguez <mcgrof@kernel.org>
---
tools/testing/selftests/firmware/fw_fallback.sh | 5 +----
1 file changed, 1 insertion(+), 4 deletions(-)
diff --git a/tools/testing/selftests/firmware/fw_fallback.sh b/tools/testing/selftests/firmware/fw_fallback.sh
index bf850050e5e9..323c921a2469 100755
--- a/tools/testing/selftests/firmware/fw_fallback.sh
+++ b/tools/testing/selftests/firmware/fw_fallback.sh
@@ -11,10 +11,7 @@ source $TEST_DIR/fw_lib.sh
check_mods
-# CONFIG_FW_LOADER_USER_HELPER has a sysfs class under /sys/class/firmware/
-# These days no one enables CONFIG_FW_LOADER_USER_HELPER so check for that
-# as an indicator for CONFIG_FW_LOADER_USER_HELPER.
-HAS_FW_LOADER_USER_HELPER=$(if [ -d /sys/class/firmware/ ]; then echo yes; else echo no; fi)
+HAS_FW_LOADER_USER_HELPER=$(kconfig_has CONFIG_FW_LOADER_USER_HELPER=y)
HAS_FW_LOADER_USER_HELPER_FALLBACK=$(kconfig_has CONFIG_FW_LOADER_USER_HELPER_FALLBACK=y)
if [ "$HAS_FW_LOADER_USER_HELPER" = "yes" ]; then
--
2.16.2
^ permalink raw reply related [flat|nested] 39+ messages in thread* [PATCH v3 04/20] firmware: enable to split firmware_class into separate target files
2018-03-10 14:14 [PATCH v3 00/20] firmware: development for v4.17 Luis R. Rodriguez
` (2 preceding siblings ...)
2018-03-10 14:14 ` [PATCH v3 03/20] test_firmware: replace syfs fallback check with kconfig_has helper Luis R. Rodriguez
@ 2018-03-10 14:14 ` Luis R. Rodriguez
2018-03-10 14:14 ` [PATCH v3 05/20] firmware: simplify CONFIG_FW_LOADER_USER_HELPER_FALLBACK further Luis R. Rodriguez
` (16 subsequent siblings)
20 siblings, 0 replies; 39+ messages in thread
From: Luis R. Rodriguez @ 2018-03-10 14:14 UTC (permalink / raw)
To: gregkh
Cc: akpm, cantabile.desu, kubakici, linux-wireless, keescook, shuah,
mfuzzey, zohar, dhowells, pali.rohar, tiwai, arend.vanspriel,
zajec5, nbroeking, markivx, broonie, dmitry.torokhov, dwmw2,
torvalds, Abhay_Salunke, bjorn.andersson, jewalt, oneukum, ast,
andresx7, linux-kernel, linux-fsdevel, Luis R. Rodriguez
The firmware loader code has grown quite a bit over the years.
The practice of stuffing everything we need into one file makes
the code hard to follow.
In order to split the firmware loader code into different components
we must pick a module name and a first object target file. We must
keep the firmware_class name to remain compatible with scripts which
have been relying on the sysfs loader path for years, so the old module
name stays. We can however rename the C file without affecting the
module name.
The firmware_class used to represent the idea that the code was a simple
sysfs firmware loader, provided by the struct class firmware_class.
The sysfs firmware loader used to be the default, today its only the
fallback mechanism.
This only renames the target code then to make emphasis of what the code
does these days. With this change new features can also use a new object
files.
Signed-off-by: Luis R. Rodriguez <mcgrof@kernel.org>
---
drivers/base/Makefile | 1 +
drivers/base/{firmware_class.c => firmware_loader.c} | 0
2 files changed, 1 insertion(+)
rename drivers/base/{firmware_class.c => firmware_loader.c} (100%)
diff --git a/drivers/base/Makefile b/drivers/base/Makefile
index e32a52490051..f261143fafbf 100644
--- a/drivers/base/Makefile
+++ b/drivers/base/Makefile
@@ -13,6 +13,7 @@ obj-$(CONFIG_HAS_DMA) += dma-mapping.o
obj-$(CONFIG_HAVE_GENERIC_DMA_COHERENT) += dma-coherent.o
obj-$(CONFIG_ISA_BUS_API) += isa.o
obj-$(CONFIG_FW_LOADER) += firmware_class.o
+firmware_class-objs := firmware_loader.o
obj-$(CONFIG_NUMA) += node.o
obj-$(CONFIG_MEMORY_HOTPLUG_SPARSE) += memory.o
ifeq ($(CONFIG_SYSFS),y)
diff --git a/drivers/base/firmware_class.c b/drivers/base/firmware_loader.c
similarity index 100%
rename from drivers/base/firmware_class.c
rename to drivers/base/firmware_loader.c
--
2.16.2
^ permalink raw reply related [flat|nested] 39+ messages in thread* [PATCH v3 05/20] firmware: simplify CONFIG_FW_LOADER_USER_HELPER_FALLBACK further
2018-03-10 14:14 [PATCH v3 00/20] firmware: development for v4.17 Luis R. Rodriguez
` (3 preceding siblings ...)
2018-03-10 14:14 ` [PATCH v3 04/20] firmware: enable to split firmware_class into separate target files Luis R. Rodriguez
@ 2018-03-10 14:14 ` Luis R. Rodriguez
2018-03-14 18:53 ` Greg KH
2018-03-10 14:14 ` [PATCH v3 06/20] firmware: use helpers for setting up a temporary cache timeout Luis R. Rodriguez
` (15 subsequent siblings)
20 siblings, 1 reply; 39+ messages in thread
From: Luis R. Rodriguez @ 2018-03-10 14:14 UTC (permalink / raw)
To: gregkh
Cc: akpm, cantabile.desu, kubakici, linux-wireless, keescook, shuah,
mfuzzey, zohar, dhowells, pali.rohar, tiwai, arend.vanspriel,
zajec5, nbroeking, markivx, broonie, dmitry.torokhov, dwmw2,
torvalds, Abhay_Salunke, bjorn.andersson, jewalt, oneukum, ast,
andresx7, linux-kernel, linux-fsdevel, Luis R. Rodriguez
All CONFIG_FW_LOADER_USER_HELPER_FALLBACK really is, is just a bool,
initailized at build time. Define it as such. This simplifies the
logic even further, removing now all explicit #ifdefs around the code.
Acked-by: Kees Cook <keescook@chromium.org>
Signed-off-by: Luis R. Rodriguez <mcgrof@kernel.org>
---
drivers/base/firmware_loader.c | 25 ++++++++++++++++++-------
1 file changed, 18 insertions(+), 7 deletions(-)
diff --git a/drivers/base/firmware_loader.c b/drivers/base/firmware_loader.c
index 7dd36ace6152..59dba794ce1a 100644
--- a/drivers/base/firmware_loader.c
+++ b/drivers/base/firmware_loader.c
@@ -266,6 +266,22 @@ static inline bool fw_state_is_aborted(struct fw_priv *fw_priv)
#ifdef CONFIG_FW_LOADER_USER_HELPER
+/**
+ * struct firmware_fallback_config - firmware fallback configuratioon settings
+ *
+ * Helps describe and fine tune the fallback mechanism.
+ *
+ * @force_sysfs_fallback: force the sysfs fallback mechanism to be used
+ * as if one had enabled CONFIG_FW_LOADER_USER_HELPER_FALLBACK=y.
+ */
+struct firmware_fallback_config {
+ bool force_sysfs_fallback;
+};
+
+static const struct firmware_fallback_config fw_fallback_config = {
+ .force_sysfs_fallback = IS_ENABLED(CONFIG_FW_LOADER_USER_HELPER_FALLBACK),
+};
+
static inline bool fw_sysfs_done(struct fw_priv *fw_priv)
{
return __fw_state_check(fw_priv, FW_STATUS_DONE);
@@ -1151,19 +1167,14 @@ static int fw_load_from_user_helper(struct firmware *firmware,
return ret;
}
-#ifdef CONFIG_FW_LOADER_USER_HELPER_FALLBACK
-static bool fw_force_sysfs_fallback(unsigned int opt_flags)
-{
- return true;
-}
-#else
static bool fw_force_sysfs_fallback(unsigned int opt_flags)
{
+ if (fw_fallback_config.force_sysfs_fallback)
+ return true;
if (!(opt_flags & FW_OPT_USERHELPER))
return false;
return true;
}
-#endif
static bool fw_run_sysfs_fallback(unsigned int opt_flags)
{
--
2.16.2
^ permalink raw reply related [flat|nested] 39+ messages in thread* Re: [PATCH v3 05/20] firmware: simplify CONFIG_FW_LOADER_USER_HELPER_FALLBACK further
2018-03-10 14:14 ` [PATCH v3 05/20] firmware: simplify CONFIG_FW_LOADER_USER_HELPER_FALLBACK further Luis R. Rodriguez
@ 2018-03-14 18:53 ` Greg KH
2018-03-14 22:36 ` Luis R. Rodriguez
0 siblings, 1 reply; 39+ messages in thread
From: Greg KH @ 2018-03-14 18:53 UTC (permalink / raw)
To: Luis R. Rodriguez
Cc: akpm, cantabile.desu, kubakici, linux-wireless, keescook, shuah,
mfuzzey, zohar, dhowells, pali.rohar, tiwai, arend.vanspriel,
zajec5, nbroeking, markivx, broonie, dmitry.torokhov, dwmw2,
torvalds, Abhay_Salunke, bjorn.andersson, jewalt, oneukum, ast,
andresx7, linux-kernel, linux-fsdevel
On Sat, Mar 10, 2018 at 06:14:46AM -0800, Luis R. Rodriguez wrote:
> All CONFIG_FW_LOADER_USER_HELPER_FALLBACK really is, is just a bool,
> initailized at build time. Define it as such. This simplifies the
> logic even further, removing now all explicit #ifdefs around the code.
>
> Acked-by: Kees Cook <keescook@chromium.org>
> Signed-off-by: Luis R. Rodriguez <mcgrof@kernel.org>
> ---
> drivers/base/firmware_loader.c | 25 ++++++++++++++++++-------
> 1 file changed, 18 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/base/firmware_loader.c b/drivers/base/firmware_loader.c
> index 7dd36ace6152..59dba794ce1a 100644
> --- a/drivers/base/firmware_loader.c
> +++ b/drivers/base/firmware_loader.c
> @@ -266,6 +266,22 @@ static inline bool fw_state_is_aborted(struct fw_priv *fw_priv)
>
> #ifdef CONFIG_FW_LOADER_USER_HELPER
>
> +/**
> + * struct firmware_fallback_config - firmware fallback configuratioon settings
> + *
> + * Helps describe and fine tune the fallback mechanism.
> + *
> + * @force_sysfs_fallback: force the sysfs fallback mechanism to be used
> + * as if one had enabled CONFIG_FW_LOADER_USER_HELPER_FALLBACK=y.
> + */
> +struct firmware_fallback_config {
> + bool force_sysfs_fallback;
> +};
This is C, why are you messing around with a structure and "getters and
setters" for a set of simple values?
> +static const struct firmware_fallback_config fw_fallback_config = {
> + .force_sysfs_fallback = IS_ENABLED(CONFIG_FW_LOADER_USER_HELPER_FALLBACK),
> +};
static bool force_sysfs_fallback = IS_ENABLED(CONFIG_FW_LOADER_USER_HELPER_FALLBACK);
Then just read/write the foolish thing, don't make this more complex
than is needed please.
There is a "getter and setters are evil" rant somewhere out there
online, if you really need me to dig up the old tired arguments :)
thanks,
greg k-h
> +
> static inline bool fw_sysfs_done(struct fw_priv *fw_priv)
> {
> return __fw_state_check(fw_priv, FW_STATUS_DONE);
> @@ -1151,19 +1167,14 @@ static int fw_load_from_user_helper(struct firmware *firmware,
> return ret;
> }
>
> -#ifdef CONFIG_FW_LOADER_USER_HELPER_FALLBACK
> -static bool fw_force_sysfs_fallback(unsigned int opt_flags)
> -{
> - return true;
> -}
> -#else
> static bool fw_force_sysfs_fallback(unsigned int opt_flags)
> {
> + if (fw_fallback_config.force_sysfs_fallback)
Ok, you directly access it here, but your later patches get a lot more
"complex" it seems...
thanks,
greg k-h
^ permalink raw reply [flat|nested] 39+ messages in thread* Re: [PATCH v3 05/20] firmware: simplify CONFIG_FW_LOADER_USER_HELPER_FALLBACK further
2018-03-14 18:53 ` Greg KH
@ 2018-03-14 22:36 ` Luis R. Rodriguez
0 siblings, 0 replies; 39+ messages in thread
From: Luis R. Rodriguez @ 2018-03-14 22:36 UTC (permalink / raw)
To: Greg KH
Cc: Andrew Morton, cantabile.desu, kubakici, linux-wireless,
Kees Cook, shuah, Martin Fuzzey, Mimi Zohar, David Howells,
Pali Rohár, Takashi Iwai, Arend Van Spriel,
Rafał Miłecki, Nicolas Broeking, Vikram Mulukutla,
Mark Brown, Dmitry Torokhov, David Woodhouse, Linus Torvalds,
Abhay Salunke, Bjorn Andersson, jewalt, oneukum,
Alexei Starovoitov, Andres Rodriguez, linux-kernel,
Linux FS Devel
On Wed, Mar 14, 2018 at 11:53 AM, Greg KH <gregkh@linuxfoundation.org> wrote:
> On Sat, Mar 10, 2018 at 06:14:46AM -0800, Luis R. Rodriguez wrote:
>> All CONFIG_FW_LOADER_USER_HELPER_FALLBACK really is, is just a bool,
>> initailized at build time. Define it as such. This simplifies the
>> logic even further, removing now all explicit #ifdefs around the code.
>>
>> Acked-by: Kees Cook <keescook@chromium.org>
>> Signed-off-by: Luis R. Rodriguez <mcgrof@kernel.org>
>> ---
>> drivers/base/firmware_loader.c | 25 ++++++++++++++++++-------
>> 1 file changed, 18 insertions(+), 7 deletions(-)
>>
>> diff --git a/drivers/base/firmware_loader.c b/drivers/base/firmware_loader.c
>> index 7dd36ace6152..59dba794ce1a 100644
>> --- a/drivers/base/firmware_loader.c
>> +++ b/drivers/base/firmware_loader.c
>> @@ -266,6 +266,22 @@ static inline bool fw_state_is_aborted(struct fw_priv *fw_priv)
>>
>> #ifdef CONFIG_FW_LOADER_USER_HELPER
>>
>> +/**
>> + * struct firmware_fallback_config - firmware fallback configuratioon settings
>> + *
>> + * Helps describe and fine tune the fallback mechanism.
>> + *
>> + * @force_sysfs_fallback: force the sysfs fallback mechanism to be used
>> + * as if one had enabled CONFIG_FW_LOADER_USER_HELPER_FALLBACK=y.
>> + */
>> +struct firmware_fallback_config {
>> + bool force_sysfs_fallback;
>> +};
>
> This is C, why are you messing around with a structure and "getters and
> setters" for a set of simple values?
>
>> +static const struct firmware_fallback_config fw_fallback_config = {
>> + .force_sysfs_fallback = IS_ENABLED(CONFIG_FW_LOADER_USER_HELPER_FALLBACK),
>> +};
>
> static bool force_sysfs_fallback = IS_ENABLED(CONFIG_FW_LOADER_USER_HELPER_FALLBACK);
For this case yes, but I later expand on this structure for all
fallback configuration items. So we have to start it somewhere.
> Then just read/write the foolish thing, don't make this more complex
> than is needed please.
Please read the final results and let me know what you think.
> There is a "getter and setters are evil" rant somewhere out there
> online, if you really need me to dig up the old tired arguments :)
I don't use them for sport, I use them given the fallback stuff *is*
not something the direct core firmware code should use openly, it will
really depend on if the fallback interface was enabled. As such we
provide wrapper for access into them.
Luis
^ permalink raw reply [flat|nested] 39+ messages in thread
* [PATCH v3 06/20] firmware: use helpers for setting up a temporary cache timeout
2018-03-10 14:14 [PATCH v3 00/20] firmware: development for v4.17 Luis R. Rodriguez
` (4 preceding siblings ...)
2018-03-10 14:14 ` [PATCH v3 05/20] firmware: simplify CONFIG_FW_LOADER_USER_HELPER_FALLBACK further Luis R. Rodriguez
@ 2018-03-10 14:14 ` Luis R. Rodriguez
2018-03-14 18:55 ` Greg KH
2018-03-10 14:14 ` [PATCH v3 07/20] firmware: move loading timeout under struct firmware_fallback_config Luis R. Rodriguez
` (14 subsequent siblings)
20 siblings, 1 reply; 39+ messages in thread
From: Luis R. Rodriguez @ 2018-03-10 14:14 UTC (permalink / raw)
To: gregkh
Cc: akpm, cantabile.desu, kubakici, linux-wireless, keescook, shuah,
mfuzzey, zohar, dhowells, pali.rohar, tiwai, arend.vanspriel,
zajec5, nbroeking, markivx, broonie, dmitry.torokhov, dwmw2,
torvalds, Abhay_Salunke, bjorn.andersson, jewalt, oneukum, ast,
andresx7, linux-kernel, linux-fsdevel, Luis R. Rodriguez
We only use the timeout for the firmware fallback mechanism
except for trying to set the timeout during the cache setup
for resume/suspend. For those cases, setting the timeout should
be a no-op, so just reflect this in code by adding helpers for it.
This change introduces no functional changes.
Acked-by: Kees Cook <keescook@chromium.org>
Signed-off-by: Luis R. Rodriguez <mcgrof@kernel.org>
---
drivers/base/firmware_loader.c | 49 ++++++++++++++++++++++++++----------------
1 file changed, 30 insertions(+), 19 deletions(-)
diff --git a/drivers/base/firmware_loader.c b/drivers/base/firmware_loader.c
index 59dba794ce1a..2d819875348d 100644
--- a/drivers/base/firmware_loader.c
+++ b/drivers/base/firmware_loader.c
@@ -191,13 +191,6 @@ static inline bool fw_is_builtin_firmware(const struct firmware *fw)
}
#endif
-static int loading_timeout = 60; /* In seconds */
-
-static inline long firmware_loading_timeout(void)
-{
- return loading_timeout > 0 ? loading_timeout * HZ : MAX_JIFFY_OFFSET;
-}
-
static void fw_state_init(struct fw_priv *fw_priv)
{
struct fw_state *fw_st = &fw_priv->fw_st;
@@ -282,6 +275,32 @@ static const struct firmware_fallback_config fw_fallback_config = {
.force_sysfs_fallback = IS_ENABLED(CONFIG_FW_LOADER_USER_HELPER_FALLBACK),
};
+static int old_timeout;
+static int loading_timeout = 60; /* In seconds */
+
+static inline long firmware_loading_timeout(void)
+{
+ return loading_timeout > 0 ? loading_timeout * HZ : MAX_JIFFY_OFFSET;
+}
+
+/*
+ * use small loading timeout for caching devices' firmware because all these
+ * firmware images have been loaded successfully at lease once, also system is
+ * ready for completing firmware loading now. The maximum size of firmware in
+ * current distributions is about 2M bytes, so 10 secs should be enough.
+ */
+static void fw_fallback_set_cache_timeout(void)
+{
+ old_timeout = loading_timeout;
+ loading_timeout = 10;
+}
+
+/* Restores the timeout to the value last configured during normal operation */
+static void fw_fallback_set_default_timeout(void)
+{
+ loading_timeout = old_timeout;
+}
+
static inline bool fw_sysfs_done(struct fw_priv *fw_priv)
{
return __fw_state_check(fw_priv, FW_STATUS_DONE);
@@ -1206,6 +1225,8 @@ static int fw_sysfs_fallback(struct firmware *fw, const char *name,
}
static inline void kill_pending_fw_fallback_reqs(bool only_kill_custom) { }
+static inline void fw_fallback_set_cache_timeout(void) { }
+static inline void fw_fallback_set_default_timeout(void) { }
static inline int register_sysfs_loader(void)
{
@@ -1752,7 +1773,6 @@ static void __device_uncache_fw_images(void)
static void device_cache_fw_images(void)
{
struct firmware_cache *fwc = &fw_cache;
- int old_timeout;
DEFINE_WAIT(wait);
pr_debug("%s\n", __func__);
@@ -1760,16 +1780,7 @@ static void device_cache_fw_images(void)
/* cancel uncache work */
cancel_delayed_work_sync(&fwc->work);
- /*
- * use small loading timeout for caching devices' firmware
- * because all these firmware images have been loaded
- * successfully at lease once, also system is ready for
- * completing firmware loading now. The maximum size of
- * firmware in current distributions is about 2M bytes,
- * so 10 secs should be enough.
- */
- old_timeout = loading_timeout;
- loading_timeout = 10;
+ fw_fallback_set_cache_timeout();
mutex_lock(&fw_lock);
fwc->state = FW_LOADER_START_CACHE;
@@ -1779,7 +1790,7 @@ static void device_cache_fw_images(void)
/* wait for completion of caching firmware for all devices */
async_synchronize_full_domain(&fw_cache_domain);
- loading_timeout = old_timeout;
+ fw_fallback_set_default_timeout();
}
/**
--
2.16.2
^ permalink raw reply related [flat|nested] 39+ messages in thread* Re: [PATCH v3 06/20] firmware: use helpers for setting up a temporary cache timeout
2018-03-10 14:14 ` [PATCH v3 06/20] firmware: use helpers for setting up a temporary cache timeout Luis R. Rodriguez
@ 2018-03-14 18:55 ` Greg KH
0 siblings, 0 replies; 39+ messages in thread
From: Greg KH @ 2018-03-14 18:55 UTC (permalink / raw)
To: Luis R. Rodriguez
Cc: akpm, cantabile.desu, kubakici, linux-wireless, keescook, shuah,
mfuzzey, zohar, dhowells, pali.rohar, tiwai, arend.vanspriel,
zajec5, nbroeking, markivx, broonie, dmitry.torokhov, dwmw2,
torvalds, Abhay_Salunke, bjorn.andersson, jewalt, oneukum, ast,
andresx7, linux-kernel, linux-fsdevel
On Sat, Mar 10, 2018 at 06:14:47AM -0800, Luis R. Rodriguez wrote:
> We only use the timeout for the firmware fallback mechanism
> except for trying to set the timeout during the cache setup
> for resume/suspend. For those cases, setting the timeout should
> be a no-op, so just reflect this in code by adding helpers for it.
>
> This change introduces no functional changes.
>
> Acked-by: Kees Cook <keescook@chromium.org>
> Signed-off-by: Luis R. Rodriguez <mcgrof@kernel.org>
> ---
> drivers/base/firmware_loader.c | 49 ++++++++++++++++++++++++++----------------
> 1 file changed, 30 insertions(+), 19 deletions(-)
>
> diff --git a/drivers/base/firmware_loader.c b/drivers/base/firmware_loader.c
> index 59dba794ce1a..2d819875348d 100644
> --- a/drivers/base/firmware_loader.c
> +++ b/drivers/base/firmware_loader.c
> @@ -191,13 +191,6 @@ static inline bool fw_is_builtin_firmware(const struct firmware *fw)
> }
> #endif
>
> -static int loading_timeout = 60; /* In seconds */
> -
> -static inline long firmware_loading_timeout(void)
> -{
> - return loading_timeout > 0 ? loading_timeout * HZ : MAX_JIFFY_OFFSET;
> -}
> -
> static void fw_state_init(struct fw_priv *fw_priv)
> {
> struct fw_state *fw_st = &fw_priv->fw_st;
> @@ -282,6 +275,32 @@ static const struct firmware_fallback_config fw_fallback_config = {
> .force_sysfs_fallback = IS_ENABLED(CONFIG_FW_LOADER_USER_HELPER_FALLBACK),
> };
>
> +static int old_timeout;
> +static int loading_timeout = 60; /* In seconds */
> +
> +static inline long firmware_loading_timeout(void)
> +{
> + return loading_timeout > 0 ? loading_timeout * HZ : MAX_JIFFY_OFFSET;
> +}
> +
> +/*
> + * use small loading timeout for caching devices' firmware because all these
> + * firmware images have been loaded successfully at lease once, also system is
> + * ready for completing firmware loading now. The maximum size of firmware in
> + * current distributions is about 2M bytes, so 10 secs should be enough.
> + */
> +static void fw_fallback_set_cache_timeout(void)
> +{
> + old_timeout = loading_timeout;
> + loading_timeout = 10;
> +}
> +
> +/* Restores the timeout to the value last configured during normal operation */
> +static void fw_fallback_set_default_timeout(void)
> +{
> + loading_timeout = old_timeout;
extra space?
Yeah, nit, I know :)
^ permalink raw reply [flat|nested] 39+ messages in thread
* [PATCH v3 07/20] firmware: move loading timeout under struct firmware_fallback_config
2018-03-10 14:14 [PATCH v3 00/20] firmware: development for v4.17 Luis R. Rodriguez
` (5 preceding siblings ...)
2018-03-10 14:14 ` [PATCH v3 06/20] firmware: use helpers for setting up a temporary cache timeout Luis R. Rodriguez
@ 2018-03-10 14:14 ` Luis R. Rodriguez
2018-03-14 18:56 ` Greg KH
2018-03-10 14:14 ` [PATCH v3 08/20] firmware: split firmware fallback functionality into its own file Luis R. Rodriguez
` (13 subsequent siblings)
20 siblings, 1 reply; 39+ messages in thread
From: Luis R. Rodriguez @ 2018-03-10 14:14 UTC (permalink / raw)
To: gregkh
Cc: akpm, cantabile.desu, kubakici, linux-wireless, keescook, shuah,
mfuzzey, zohar, dhowells, pali.rohar, tiwai, arend.vanspriel,
zajec5, nbroeking, markivx, broonie, dmitry.torokhov, dwmw2,
torvalds, Abhay_Salunke, bjorn.andersson, jewalt, oneukum, ast,
andresx7, linux-kernel, linux-fsdevel, Luis R. Rodriguez
The timeout is a fallback construct, so we can just stuff the
timeout configuration under struct firmware_fallback_config.
While at it, add a few helpers which vets the use of getting or
setting the timeout as an int. The main use of the timeout is
to set a timeout for completion, and that is used as an unsigned
long. There a few cases however where it makes sense to get or
set the timeout as an int, the helpers annotate these use cases
have been properly vetted for.
Acked-by: Kees Cook <keescook@chromium.org>
Signed-off-by: Luis R. Rodriguez <mcgrof@kernel.org>
---
drivers/base/firmware_loader.c | 46 ++++++++++++++++++++++++++++++------------
1 file changed, 33 insertions(+), 13 deletions(-)
diff --git a/drivers/base/firmware_loader.c b/drivers/base/firmware_loader.c
index 2d819875348d..9757f9fff01e 100644
--- a/drivers/base/firmware_loader.c
+++ b/drivers/base/firmware_loader.c
@@ -266,21 +266,38 @@ static inline bool fw_state_is_aborted(struct fw_priv *fw_priv)
*
* @force_sysfs_fallback: force the sysfs fallback mechanism to be used
* as if one had enabled CONFIG_FW_LOADER_USER_HELPER_FALLBACK=y.
+ * @old_timeout: for internal use
+ * @loading_timeout: the timeout to wait for the fallback mechanism before
+ * giving up, in seconds.
*/
struct firmware_fallback_config {
- bool force_sysfs_fallback;
+ const bool force_sysfs_fallback;
+ int old_timeout;
+ int loading_timeout;
};
-static const struct firmware_fallback_config fw_fallback_config = {
+static struct firmware_fallback_config fw_fallback_config = {
.force_sysfs_fallback = IS_ENABLED(CONFIG_FW_LOADER_USER_HELPER_FALLBACK),
+ .loading_timeout = 60,
+ .old_timeout = 60,
};
-static int old_timeout;
-static int loading_timeout = 60; /* In seconds */
+/* These getters are vetted to use int properly */
+static inline int __firmware_loading_timeout(void)
+{
+ return fw_fallback_config.loading_timeout;
+}
+
+/* These setters are vetted to use int properly */
+static void __fw_fallback_set_timeout(int timeout)
+{
+ fw_fallback_config.loading_timeout = timeout;
+}
static inline long firmware_loading_timeout(void)
{
- return loading_timeout > 0 ? loading_timeout * HZ : MAX_JIFFY_OFFSET;
+ return __firmware_loading_timeout() > 0 ?
+ __firmware_loading_timeout() * HZ : MAX_JIFFY_OFFSET;
}
/*
@@ -291,14 +308,14 @@ static inline long firmware_loading_timeout(void)
*/
static void fw_fallback_set_cache_timeout(void)
{
- old_timeout = loading_timeout;
- loading_timeout = 10;
+ fw_fallback_config.old_timeout = __firmware_loading_timeout();
+ __fw_fallback_set_timeout(10);
}
/* Restores the timeout to the value last configured during normal operation */
static void fw_fallback_set_default_timeout(void)
{
- loading_timeout = old_timeout;
+ __fw_fallback_set_timeout(fw_fallback_config.old_timeout);
}
static inline bool fw_sysfs_done(struct fw_priv *fw_priv)
@@ -677,7 +694,7 @@ static void kill_pending_fw_fallback_reqs(bool only_kill_custom)
static ssize_t timeout_show(struct class *class, struct class_attribute *attr,
char *buf)
{
- return sprintf(buf, "%d\n", loading_timeout);
+ return sprintf(buf, "%d\n", __firmware_loading_timeout());
}
/**
@@ -696,9 +713,12 @@ static ssize_t timeout_show(struct class *class, struct class_attribute *attr,
static ssize_t timeout_store(struct class *class, struct class_attribute *attr,
const char *buf, size_t count)
{
- loading_timeout = simple_strtol(buf, NULL, 10);
- if (loading_timeout < 0)
- loading_timeout = 0;
+ int tmp_loading_timeout = simple_strtol(buf, NULL, 10);
+
+ if (tmp_loading_timeout < 0)
+ tmp_loading_timeout = 0;
+
+ __fw_fallback_set_timeout(tmp_loading_timeout);
return count;
}
@@ -721,7 +741,7 @@ static int do_firmware_uevent(struct fw_sysfs *fw_sysfs, struct kobj_uevent_env
{
if (add_uevent_var(env, "FIRMWARE=%s", fw_sysfs->fw_priv->fw_name))
return -ENOMEM;
- if (add_uevent_var(env, "TIMEOUT=%i", loading_timeout))
+ if (add_uevent_var(env, "TIMEOUT=%i", __firmware_loading_timeout()))
return -ENOMEM;
if (add_uevent_var(env, "ASYNC=%d", fw_sysfs->nowait))
return -ENOMEM;
--
2.16.2
^ permalink raw reply related [flat|nested] 39+ messages in thread* Re: [PATCH v3 07/20] firmware: move loading timeout under struct firmware_fallback_config
2018-03-10 14:14 ` [PATCH v3 07/20] firmware: move loading timeout under struct firmware_fallback_config Luis R. Rodriguez
@ 2018-03-14 18:56 ` Greg KH
2018-03-14 22:33 ` Luis R. Rodriguez
0 siblings, 1 reply; 39+ messages in thread
From: Greg KH @ 2018-03-14 18:56 UTC (permalink / raw)
To: Luis R. Rodriguez
Cc: akpm, cantabile.desu, kubakici, linux-wireless, keescook, shuah,
mfuzzey, zohar, dhowells, pali.rohar, tiwai, arend.vanspriel,
zajec5, nbroeking, markivx, broonie, dmitry.torokhov, dwmw2,
torvalds, Abhay_Salunke, bjorn.andersson, jewalt, oneukum, ast,
andresx7, linux-kernel, linux-fsdevel
On Sat, Mar 10, 2018 at 06:14:48AM -0800, Luis R. Rodriguez wrote:
> The timeout is a fallback construct, so we can just stuff the
> timeout configuration under struct firmware_fallback_config.
Why? What does it matter?
> While at it, add a few helpers which vets the use of getting or
> setting the timeout as an int. The main use of the timeout is
> to set a timeout for completion, and that is used as an unsigned
> long. There a few cases however where it makes sense to get or
> set the timeout as an int, the helpers annotate these use cases
> have been properly vetted for.
This feels really odd to me. Why would you want to use it as an int,
just keep it the same "size" everywhere and it should be simpler and
easier to keep working correctly over time.
thanks,
greg k-h
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH v3 07/20] firmware: move loading timeout under struct firmware_fallback_config
2018-03-14 18:56 ` Greg KH
@ 2018-03-14 22:33 ` Luis R. Rodriguez
0 siblings, 0 replies; 39+ messages in thread
From: Luis R. Rodriguez @ 2018-03-14 22:33 UTC (permalink / raw)
To: Greg KH
Cc: Andrew Morton, cantabile.desu, kubakici, linux-wireless,
Kees Cook, shuah, Martin Fuzzey, Mimi Zohar, David Howells,
Pali Rohár, Takashi Iwai, Arend Van Spriel,
Rafał Miłecki, Nicolas Broeking, Vikram Mulukutla,
Mark Brown, Dmitry Torokhov, David Woodhouse, Linus Torvalds,
Abhay Salunke, Bjorn Andersson, jewalt, oneukum,
Alexei Starovoitov, Andres Rodriguez, linux-kernel,
Linux FS Devel
On Wed, Mar 14, 2018 at 11:56 AM, Greg KH <gregkh@linuxfoundation.org> wrote:
> On Sat, Mar 10, 2018 at 06:14:48AM -0800, Luis R. Rodriguez wrote:
>> The timeout is a fallback construct, so we can just stuff the
>> timeout configuration under struct firmware_fallback_config.
>
> Why? What does it matter?
Because we want to remove direct access to things which the direct
firmware loader should not care about, and instead have proper
wrappers so that the fallback code implements it when needed. Part of
the motivation for this then was to move all fallback code into its
own file therefore compartmentalizing it.
>> While at it, add a few helpers which vets the use of getting or
>> setting the timeout as an int. The main use of the timeout is
>> to set a timeout for completion, and that is used as an unsigned
>> long. There a few cases however where it makes sense to get or
>> set the timeout as an int, the helpers annotate these use cases
>> have been properly vetted for.
>
> This feels really odd to me. Why would you want to use it as an int,
> just keep it the same "size" everywhere and it should be simpler and
> easier to keep working correctly over time.
One is the input/output we provide for it uses ints all over so its
much easier to handle this as an int consistently in most places and
only deal with the long where needed. See above uses of
simple_strtol(), add_uevent_var(). Otherwise the inverse has to be
done. This was easier to deal with and vet for.
Luis
^ permalink raw reply [flat|nested] 39+ messages in thread
* [PATCH v3 08/20] firmware: split firmware fallback functionality into its own file
2018-03-10 14:14 [PATCH v3 00/20] firmware: development for v4.17 Luis R. Rodriguez
` (6 preceding siblings ...)
2018-03-10 14:14 ` [PATCH v3 07/20] firmware: move loading timeout under struct firmware_fallback_config Luis R. Rodriguez
@ 2018-03-10 14:14 ` Luis R. Rodriguez
2018-03-10 14:14 ` [PATCH v3 09/20] firmware: move firmware loader into its own directory Luis R. Rodriguez
` (12 subsequent siblings)
20 siblings, 0 replies; 39+ messages in thread
From: Luis R. Rodriguez @ 2018-03-10 14:14 UTC (permalink / raw)
To: gregkh
Cc: akpm, cantabile.desu, kubakici, linux-wireless, keescook, shuah,
mfuzzey, zohar, dhowells, pali.rohar, tiwai, arend.vanspriel,
zajec5, nbroeking, markivx, broonie, dmitry.torokhov, dwmw2,
torvalds, Abhay_Salunke, bjorn.andersson, jewalt, oneukum, ast,
andresx7, linux-kernel, linux-fsdevel, Luis R. Rodriguez
The firmware fallback code is optional. Split that code out to help
distinguish the fallback functionlity from othere core firmware loader
features. This should make it easier to maintain and review code
changes.
The reason for keeping the configuration onto a table which is built-in
if you enable firmware loading is so that we can later enable the kernel
after subsequent patches to tweak this configuration, even if the
firmware loader is modular.
This introduces no functional changes.
Signed-off-by: Luis R. Rodriguez <mcgrof@kernel.org>
---
drivers/base/Makefile | 4 +-
drivers/base/firmware_fallback.c | 661 +++++++++++++++++++++++++++
drivers/base/firmware_fallback.h | 61 +++
drivers/base/firmware_fallback_table.c | 29 ++
drivers/base/firmware_loader.c | 803 +--------------------------------
drivers/base/firmware_loader.h | 115 +++++
6 files changed, 874 insertions(+), 799 deletions(-)
create mode 100644 drivers/base/firmware_fallback.c
create mode 100644 drivers/base/firmware_fallback.h
create mode 100644 drivers/base/firmware_fallback_table.c
create mode 100644 drivers/base/firmware_loader.h
diff --git a/drivers/base/Makefile b/drivers/base/Makefile
index f261143fafbf..b946a408256d 100644
--- a/drivers/base/Makefile
+++ b/drivers/base/Makefile
@@ -5,7 +5,8 @@ obj-y := component.o core.o bus.o dd.o syscore.o \
driver.o class.o platform.o \
cpu.o firmware.o init.o map.o devres.o \
attribute_container.o transport_class.o \
- topology.o container.o property.o cacheinfo.o
+ topology.o container.o property.o cacheinfo.o \
+ firmware_fallback_table.o
obj-$(CONFIG_DEVTMPFS) += devtmpfs.o
obj-$(CONFIG_DMA_CMA) += dma-contiguous.o
obj-y += power/
@@ -14,6 +15,7 @@ obj-$(CONFIG_HAVE_GENERIC_DMA_COHERENT) += dma-coherent.o
obj-$(CONFIG_ISA_BUS_API) += isa.o
obj-$(CONFIG_FW_LOADER) += firmware_class.o
firmware_class-objs := firmware_loader.o
+firmware_class-$(CONFIG_FW_LOADER_USER_HELPER) += firmware_fallback.o
obj-$(CONFIG_NUMA) += node.o
obj-$(CONFIG_MEMORY_HOTPLUG_SPARSE) += memory.o
ifeq ($(CONFIG_SYSFS),y)
diff --git a/drivers/base/firmware_fallback.c b/drivers/base/firmware_fallback.c
new file mode 100644
index 000000000000..47690207e0ee
--- /dev/null
+++ b/drivers/base/firmware_fallback.c
@@ -0,0 +1,661 @@
+// SPDX-License-Identifier: GPL-2.0
+
+#include <linux/types.h>
+#include <linux/kconfig.h>
+#include <linux/list.h>
+#include <linux/slab.h>
+#include <linux/security.h>
+#include <linux/highmem.h>
+#include <linux/umh.h>
+
+#include "firmware_fallback.h"
+#include "firmware_loader.h"
+
+/*
+ * firmware fallback mechanism
+ */
+
+extern struct firmware_fallback_config fw_fallback_config;
+
+/* These getters are vetted to use int properly */
+static inline int __firmware_loading_timeout(void)
+{
+ return fw_fallback_config.loading_timeout;
+}
+
+/* These setters are vetted to use int properly */
+static void __fw_fallback_set_timeout(int timeout)
+{
+ fw_fallback_config.loading_timeout = timeout;
+}
+
+/*
+ * use small loading timeout for caching devices' firmware because all these
+ * firmware images have been loaded successfully at lease once, also system is
+ * ready for completing firmware loading now. The maximum size of firmware in
+ * current distributions is about 2M bytes, so 10 secs should be enough.
+ */
+void fw_fallback_set_cache_timeout(void)
+{
+ fw_fallback_config.old_timeout = __firmware_loading_timeout();
+ __fw_fallback_set_timeout(10);
+}
+
+/* Restores the timeout to the value last configured during normal operation */
+void fw_fallback_set_default_timeout(void)
+{
+ __fw_fallback_set_timeout(fw_fallback_config.old_timeout);
+}
+
+static long firmware_loading_timeout(void)
+{
+ return __firmware_loading_timeout() > 0 ?
+ __firmware_loading_timeout() * HZ : MAX_JIFFY_OFFSET;
+}
+
+static inline bool fw_sysfs_done(struct fw_priv *fw_priv)
+{
+ return __fw_state_check(fw_priv, FW_STATUS_DONE);
+}
+
+static inline bool fw_sysfs_loading(struct fw_priv *fw_priv)
+{
+ return __fw_state_check(fw_priv, FW_STATUS_LOADING);
+}
+
+static inline int fw_sysfs_wait_timeout(struct fw_priv *fw_priv, long timeout)
+{
+ return __fw_state_wait_common(fw_priv, timeout);
+}
+
+struct fw_sysfs {
+ bool nowait;
+ struct device dev;
+ struct fw_priv *fw_priv;
+ struct firmware *fw;
+};
+
+static struct fw_sysfs *to_fw_sysfs(struct device *dev)
+{
+ return container_of(dev, struct fw_sysfs, dev);
+}
+
+static void __fw_load_abort(struct fw_priv *fw_priv)
+{
+ /*
+ * There is a small window in which user can write to 'loading'
+ * between loading done and disappearance of 'loading'
+ */
+ if (fw_sysfs_done(fw_priv))
+ return;
+
+ list_del_init(&fw_priv->pending_list);
+ fw_state_aborted(fw_priv);
+}
+
+static void fw_load_abort(struct fw_sysfs *fw_sysfs)
+{
+ struct fw_priv *fw_priv = fw_sysfs->fw_priv;
+
+ __fw_load_abort(fw_priv);
+}
+
+static LIST_HEAD(pending_fw_head);
+
+void kill_pending_fw_fallback_reqs(bool only_kill_custom)
+{
+ struct fw_priv *fw_priv;
+ struct fw_priv *next;
+
+ mutex_lock(&fw_lock);
+ list_for_each_entry_safe(fw_priv, next, &pending_fw_head,
+ pending_list) {
+ if (!fw_priv->need_uevent || !only_kill_custom)
+ __fw_load_abort(fw_priv);
+ }
+ mutex_unlock(&fw_lock);
+}
+
+static ssize_t timeout_show(struct class *class, struct class_attribute *attr,
+ char *buf)
+{
+ return sprintf(buf, "%d\n", __firmware_loading_timeout());
+}
+
+/**
+ * firmware_timeout_store - set number of seconds to wait for firmware
+ * @class: device class pointer
+ * @attr: device attribute pointer
+ * @buf: buffer to scan for timeout value
+ * @count: number of bytes in @buf
+ *
+ * Sets the number of seconds to wait for the firmware. Once
+ * this expires an error will be returned to the driver and no
+ * firmware will be provided.
+ *
+ * Note: zero means 'wait forever'.
+ **/
+static ssize_t timeout_store(struct class *class, struct class_attribute *attr,
+ const char *buf, size_t count)
+{
+ int tmp_loading_timeout = simple_strtol(buf, NULL, 10);
+
+ if (tmp_loading_timeout < 0)
+ tmp_loading_timeout = 0;
+
+ __fw_fallback_set_timeout(tmp_loading_timeout);
+
+ return count;
+}
+static CLASS_ATTR_RW(timeout);
+
+static struct attribute *firmware_class_attrs[] = {
+ &class_attr_timeout.attr,
+ NULL,
+};
+ATTRIBUTE_GROUPS(firmware_class);
+
+static void fw_dev_release(struct device *dev)
+{
+ struct fw_sysfs *fw_sysfs = to_fw_sysfs(dev);
+
+ kfree(fw_sysfs);
+}
+
+static int do_firmware_uevent(struct fw_sysfs *fw_sysfs, struct kobj_uevent_env *env)
+{
+ if (add_uevent_var(env, "FIRMWARE=%s", fw_sysfs->fw_priv->fw_name))
+ return -ENOMEM;
+ if (add_uevent_var(env, "TIMEOUT=%i", __firmware_loading_timeout()))
+ return -ENOMEM;
+ if (add_uevent_var(env, "ASYNC=%d", fw_sysfs->nowait))
+ return -ENOMEM;
+
+ return 0;
+}
+
+static int firmware_uevent(struct device *dev, struct kobj_uevent_env *env)
+{
+ struct fw_sysfs *fw_sysfs = to_fw_sysfs(dev);
+ int err = 0;
+
+ mutex_lock(&fw_lock);
+ if (fw_sysfs->fw_priv)
+ err = do_firmware_uevent(fw_sysfs, env);
+ mutex_unlock(&fw_lock);
+ return err;
+}
+
+static struct class firmware_class = {
+ .name = "firmware",
+ .class_groups = firmware_class_groups,
+ .dev_uevent = firmware_uevent,
+ .dev_release = fw_dev_release,
+};
+
+int register_sysfs_loader(void)
+{
+ return class_register(&firmware_class);
+}
+
+void unregister_sysfs_loader(void)
+{
+ class_unregister(&firmware_class);
+}
+
+static ssize_t firmware_loading_show(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
+ struct fw_sysfs *fw_sysfs = to_fw_sysfs(dev);
+ int loading = 0;
+
+ mutex_lock(&fw_lock);
+ if (fw_sysfs->fw_priv)
+ loading = fw_sysfs_loading(fw_sysfs->fw_priv);
+ mutex_unlock(&fw_lock);
+
+ return sprintf(buf, "%d\n", loading);
+}
+
+/* Some architectures don't have PAGE_KERNEL_RO */
+#ifndef PAGE_KERNEL_RO
+#define PAGE_KERNEL_RO PAGE_KERNEL
+#endif
+
+/* one pages buffer should be mapped/unmapped only once */
+static int map_fw_priv_pages(struct fw_priv *fw_priv)
+{
+ if (!fw_priv->is_paged_buf)
+ return 0;
+
+ vunmap(fw_priv->data);
+ fw_priv->data = vmap(fw_priv->pages, fw_priv->nr_pages, 0,
+ PAGE_KERNEL_RO);
+ if (!fw_priv->data)
+ return -ENOMEM;
+ return 0;
+}
+
+/**
+ * firmware_loading_store - set value in the 'loading' control file
+ * @dev: device pointer
+ * @attr: device attribute pointer
+ * @buf: buffer to scan for loading control value
+ * @count: number of bytes in @buf
+ *
+ * The relevant values are:
+ *
+ * 1: Start a load, discarding any previous partial load.
+ * 0: Conclude the load and hand the data to the driver code.
+ * -1: Conclude the load with an error and discard any written data.
+ **/
+static ssize_t firmware_loading_store(struct device *dev,
+ struct device_attribute *attr,
+ const char *buf, size_t count)
+{
+ struct fw_sysfs *fw_sysfs = to_fw_sysfs(dev);
+ struct fw_priv *fw_priv;
+ ssize_t written = count;
+ int loading = simple_strtol(buf, NULL, 10);
+ int i;
+
+ mutex_lock(&fw_lock);
+ fw_priv = fw_sysfs->fw_priv;
+ if (fw_state_is_aborted(fw_priv))
+ goto out;
+
+ switch (loading) {
+ case 1:
+ /* discarding any previous partial load */
+ if (!fw_sysfs_done(fw_priv)) {
+ for (i = 0; i < fw_priv->nr_pages; i++)
+ __free_page(fw_priv->pages[i]);
+ vfree(fw_priv->pages);
+ fw_priv->pages = NULL;
+ fw_priv->page_array_size = 0;
+ fw_priv->nr_pages = 0;
+ fw_state_start(fw_priv);
+ }
+ break;
+ case 0:
+ if (fw_sysfs_loading(fw_priv)) {
+ int rc;
+
+ /*
+ * Several loading requests may be pending on
+ * one same firmware buf, so let all requests
+ * see the mapped 'buf->data' once the loading
+ * is completed.
+ * */
+ rc = map_fw_priv_pages(fw_priv);
+ if (rc)
+ dev_err(dev, "%s: map pages failed\n",
+ __func__);
+ else
+ rc = security_kernel_post_read_file(NULL,
+ fw_priv->data, fw_priv->size,
+ READING_FIRMWARE);
+
+ /*
+ * Same logic as fw_load_abort, only the DONE bit
+ * is ignored and we set ABORT only on failure.
+ */
+ list_del_init(&fw_priv->pending_list);
+ if (rc) {
+ fw_state_aborted(fw_priv);
+ written = rc;
+ } else {
+ fw_state_done(fw_priv);
+ }
+ break;
+ }
+ /* fallthrough */
+ default:
+ dev_err(dev, "%s: unexpected value (%d)\n", __func__, loading);
+ /* fallthrough */
+ case -1:
+ fw_load_abort(fw_sysfs);
+ break;
+ }
+out:
+ mutex_unlock(&fw_lock);
+ return written;
+}
+
+static DEVICE_ATTR(loading, 0644, firmware_loading_show, firmware_loading_store);
+
+static void firmware_rw_data(struct fw_priv *fw_priv, char *buffer,
+ loff_t offset, size_t count, bool read)
+{
+ if (read)
+ memcpy(buffer, fw_priv->data + offset, count);
+ else
+ memcpy(fw_priv->data + offset, buffer, count);
+}
+
+static void firmware_rw(struct fw_priv *fw_priv, char *buffer,
+ loff_t offset, size_t count, bool read)
+{
+ while (count) {
+ void *page_data;
+ int page_nr = offset >> PAGE_SHIFT;
+ int page_ofs = offset & (PAGE_SIZE-1);
+ int page_cnt = min_t(size_t, PAGE_SIZE - page_ofs, count);
+
+ page_data = kmap(fw_priv->pages[page_nr]);
+
+ if (read)
+ memcpy(buffer, page_data + page_ofs, page_cnt);
+ else
+ memcpy(page_data + page_ofs, buffer, page_cnt);
+
+ kunmap(fw_priv->pages[page_nr]);
+ buffer += page_cnt;
+ offset += page_cnt;
+ count -= page_cnt;
+ }
+}
+
+static ssize_t firmware_data_read(struct file *filp, struct kobject *kobj,
+ struct bin_attribute *bin_attr,
+ char *buffer, loff_t offset, size_t count)
+{
+ struct device *dev = kobj_to_dev(kobj);
+ struct fw_sysfs *fw_sysfs = to_fw_sysfs(dev);
+ struct fw_priv *fw_priv;
+ ssize_t ret_count;
+
+ mutex_lock(&fw_lock);
+ fw_priv = fw_sysfs->fw_priv;
+ if (!fw_priv || fw_sysfs_done(fw_priv)) {
+ ret_count = -ENODEV;
+ goto out;
+ }
+ if (offset > fw_priv->size) {
+ ret_count = 0;
+ goto out;
+ }
+ if (count > fw_priv->size - offset)
+ count = fw_priv->size - offset;
+
+ ret_count = count;
+
+ if (fw_priv->data)
+ firmware_rw_data(fw_priv, buffer, offset, count, true);
+ else
+ firmware_rw(fw_priv, buffer, offset, count, true);
+
+out:
+ mutex_unlock(&fw_lock);
+ return ret_count;
+}
+
+static int fw_realloc_pages(struct fw_sysfs *fw_sysfs, int min_size)
+{
+ struct fw_priv *fw_priv= fw_sysfs->fw_priv;
+ int pages_needed = PAGE_ALIGN(min_size) >> PAGE_SHIFT;
+
+ /* If the array of pages is too small, grow it... */
+ if (fw_priv->page_array_size < pages_needed) {
+ int new_array_size = max(pages_needed,
+ fw_priv->page_array_size * 2);
+ struct page **new_pages;
+
+ new_pages = vmalloc(new_array_size * sizeof(void *));
+ if (!new_pages) {
+ fw_load_abort(fw_sysfs);
+ return -ENOMEM;
+ }
+ memcpy(new_pages, fw_priv->pages,
+ fw_priv->page_array_size * sizeof(void *));
+ memset(&new_pages[fw_priv->page_array_size], 0, sizeof(void *) *
+ (new_array_size - fw_priv->page_array_size));
+ vfree(fw_priv->pages);
+ fw_priv->pages = new_pages;
+ fw_priv->page_array_size = new_array_size;
+ }
+
+ while (fw_priv->nr_pages < pages_needed) {
+ fw_priv->pages[fw_priv->nr_pages] =
+ alloc_page(GFP_KERNEL | __GFP_HIGHMEM);
+
+ if (!fw_priv->pages[fw_priv->nr_pages]) {
+ fw_load_abort(fw_sysfs);
+ return -ENOMEM;
+ }
+ fw_priv->nr_pages++;
+ }
+ return 0;
+}
+
+/**
+ * firmware_data_write - write method for firmware
+ * @filp: open sysfs file
+ * @kobj: kobject for the device
+ * @bin_attr: bin_attr structure
+ * @buffer: buffer being written
+ * @offset: buffer offset for write in total data store area
+ * @count: buffer size
+ *
+ * Data written to the 'data' attribute will be later handed to
+ * the driver as a firmware image.
+ **/
+static ssize_t firmware_data_write(struct file *filp, struct kobject *kobj,
+ struct bin_attribute *bin_attr,
+ char *buffer, loff_t offset, size_t count)
+{
+ struct device *dev = kobj_to_dev(kobj);
+ struct fw_sysfs *fw_sysfs = to_fw_sysfs(dev);
+ struct fw_priv *fw_priv;
+ ssize_t retval;
+
+ if (!capable(CAP_SYS_RAWIO))
+ return -EPERM;
+
+ mutex_lock(&fw_lock);
+ fw_priv = fw_sysfs->fw_priv;
+ if (!fw_priv || fw_sysfs_done(fw_priv)) {
+ retval = -ENODEV;
+ goto out;
+ }
+
+ if (fw_priv->data) {
+ if (offset + count > fw_priv->allocated_size) {
+ retval = -ENOMEM;
+ goto out;
+ }
+ firmware_rw_data(fw_priv, buffer, offset, count, false);
+ retval = count;
+ } else {
+ retval = fw_realloc_pages(fw_sysfs, offset + count);
+ if (retval)
+ goto out;
+
+ retval = count;
+ firmware_rw(fw_priv, buffer, offset, count, false);
+ }
+
+ fw_priv->size = max_t(size_t, offset + count, fw_priv->size);
+out:
+ mutex_unlock(&fw_lock);
+ return retval;
+}
+
+static struct bin_attribute firmware_attr_data = {
+ .attr = { .name = "data", .mode = 0644 },
+ .size = 0,
+ .read = firmware_data_read,
+ .write = firmware_data_write,
+};
+
+static struct attribute *fw_dev_attrs[] = {
+ &dev_attr_loading.attr,
+ NULL
+};
+
+static struct bin_attribute *fw_dev_bin_attrs[] = {
+ &firmware_attr_data,
+ NULL
+};
+
+static const struct attribute_group fw_dev_attr_group = {
+ .attrs = fw_dev_attrs,
+ .bin_attrs = fw_dev_bin_attrs,
+};
+
+static const struct attribute_group *fw_dev_attr_groups[] = {
+ &fw_dev_attr_group,
+ NULL
+};
+
+static struct fw_sysfs *
+fw_create_instance(struct firmware *firmware, const char *fw_name,
+ struct device *device, unsigned int opt_flags)
+{
+ struct fw_sysfs *fw_sysfs;
+ struct device *f_dev;
+
+ fw_sysfs = kzalloc(sizeof(*fw_sysfs), GFP_KERNEL);
+ if (!fw_sysfs) {
+ fw_sysfs = ERR_PTR(-ENOMEM);
+ goto exit;
+ }
+
+ fw_sysfs->nowait = !!(opt_flags & FW_OPT_NOWAIT);
+ fw_sysfs->fw = firmware;
+ f_dev = &fw_sysfs->dev;
+
+ device_initialize(f_dev);
+ dev_set_name(f_dev, "%s", fw_name);
+ f_dev->parent = device;
+ f_dev->class = &firmware_class;
+ f_dev->groups = fw_dev_attr_groups;
+exit:
+ return fw_sysfs;
+}
+
+/* load a firmware via user helper */
+static int _request_firmware_load(struct fw_sysfs *fw_sysfs,
+ unsigned int opt_flags, long timeout)
+{
+ int retval = 0;
+ struct device *f_dev = &fw_sysfs->dev;
+ struct fw_priv *fw_priv = fw_sysfs->fw_priv;
+
+ /* fall back on userspace loading */
+ if (!fw_priv->data)
+ fw_priv->is_paged_buf = true;
+
+ dev_set_uevent_suppress(f_dev, true);
+
+ retval = device_add(f_dev);
+ if (retval) {
+ dev_err(f_dev, "%s: device_register failed\n", __func__);
+ goto err_put_dev;
+ }
+
+ mutex_lock(&fw_lock);
+ list_add(&fw_priv->pending_list, &pending_fw_head);
+ mutex_unlock(&fw_lock);
+
+ if (opt_flags & FW_OPT_UEVENT) {
+ fw_priv->need_uevent = true;
+ dev_set_uevent_suppress(f_dev, false);
+ dev_dbg(f_dev, "firmware: requesting %s\n", fw_priv->fw_name);
+ kobject_uevent(&fw_sysfs->dev.kobj, KOBJ_ADD);
+ } else {
+ timeout = MAX_JIFFY_OFFSET;
+ }
+
+ retval = fw_sysfs_wait_timeout(fw_priv, timeout);
+ if (retval < 0) {
+ mutex_lock(&fw_lock);
+ fw_load_abort(fw_sysfs);
+ mutex_unlock(&fw_lock);
+ }
+
+ if (fw_state_is_aborted(fw_priv)) {
+ if (retval == -ERESTARTSYS)
+ retval = -EINTR;
+ else
+ retval = -EAGAIN;
+ } else if (fw_priv->is_paged_buf && !fw_priv->data)
+ retval = -ENOMEM;
+
+ device_del(f_dev);
+err_put_dev:
+ put_device(f_dev);
+ return retval;
+}
+
+static int fw_load_from_user_helper(struct firmware *firmware,
+ const char *name, struct device *device,
+ unsigned int opt_flags)
+{
+ struct fw_sysfs *fw_sysfs;
+ long timeout;
+ int ret;
+
+ timeout = firmware_loading_timeout();
+ if (opt_flags & FW_OPT_NOWAIT) {
+ timeout = usermodehelper_read_lock_wait(timeout);
+ if (!timeout) {
+ dev_dbg(device, "firmware: %s loading timed out\n",
+ name);
+ return -EBUSY;
+ }
+ } else {
+ ret = usermodehelper_read_trylock();
+ if (WARN_ON(ret)) {
+ dev_err(device, "firmware: %s will not be loaded\n",
+ name);
+ return ret;
+ }
+ }
+
+ fw_sysfs = fw_create_instance(firmware, name, device, opt_flags);
+ if (IS_ERR(fw_sysfs)) {
+ ret = PTR_ERR(fw_sysfs);
+ goto out_unlock;
+ }
+
+ fw_sysfs->fw_priv = firmware->priv;
+ ret = _request_firmware_load(fw_sysfs, opt_flags, timeout);
+
+ if (!ret)
+ ret = assign_fw(firmware, device, opt_flags);
+
+out_unlock:
+ usermodehelper_read_unlock();
+
+ return ret;
+}
+
+static bool fw_force_sysfs_fallback(unsigned int opt_flags)
+{
+ if (fw_fallback_config.force_sysfs_fallback)
+ return true;
+ if (!(opt_flags & FW_OPT_USERHELPER))
+ return false;
+ return true;
+}
+
+static bool fw_run_sysfs_fallback(unsigned int opt_flags)
+{
+ if ((opt_flags & FW_OPT_NOFALLBACK))
+ return false;
+
+ return fw_force_sysfs_fallback(opt_flags);
+}
+
+int fw_sysfs_fallback(struct firmware *fw, const char *name,
+ struct device *device,
+ unsigned int opt_flags,
+ int ret)
+{
+ if (!fw_run_sysfs_fallback(opt_flags))
+ return ret;
+
+ dev_warn(device, "Falling back to user helper\n");
+ return fw_load_from_user_helper(fw, name, device, opt_flags);
+}
diff --git a/drivers/base/firmware_fallback.h b/drivers/base/firmware_fallback.h
new file mode 100644
index 000000000000..550498c7fa4c
--- /dev/null
+++ b/drivers/base/firmware_fallback.h
@@ -0,0 +1,61 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef __FIRMWARE_FALLBACK_H
+#define __FIRMWARE_FALLBACK_H
+
+#include <linux/firmware.h>
+#include <linux/device.h>
+
+/**
+ * struct firmware_fallback_config - firmware fallback configuratioon settings
+ *
+ * Helps describe and fine tune the fallback mechanism.
+ *
+ * @force_sysfs_fallback: force the sysfs fallback mechanism to be used
+ * as if one had enabled CONFIG_FW_LOADER_USER_HELPER_FALLBACK=y.
+ * @old_timeout: for internal use
+ * @loading_timeout: the timeout to wait for the fallback mechanism before
+ * giving up, in seconds.
+ */
+struct firmware_fallback_config {
+ const bool force_sysfs_fallback;
+ int old_timeout;
+ int loading_timeout;
+};
+
+#ifdef CONFIG_FW_LOADER_USER_HELPER
+int fw_sysfs_fallback(struct firmware *fw, const char *name,
+ struct device *device,
+ unsigned int opt_flags,
+ int ret);
+void kill_pending_fw_fallback_reqs(bool only_kill_custom);
+
+void fw_fallback_set_cache_timeout(void);
+void fw_fallback_set_default_timeout(void);
+
+int register_sysfs_loader(void);
+void unregister_sysfs_loader(void);
+#else /* CONFIG_FW_LOADER_USER_HELPER */
+static inline int fw_sysfs_fallback(struct firmware *fw, const char *name,
+ struct device *device,
+ unsigned int opt_flags,
+ int ret)
+{
+ /* Keep carrying over the same error */
+ return ret;
+}
+
+static inline void kill_pending_fw_fallback_reqs(bool only_kill_custom) { }
+static inline void fw_fallback_set_cache_timeout(void) { }
+static inline void fw_fallback_set_default_timeout(void) { }
+
+static inline int register_sysfs_loader(void)
+{
+ return 0;
+}
+
+static inline void unregister_sysfs_loader(void)
+{
+}
+#endif /* CONFIG_FW_LOADER_USER_HELPER */
+
+#endif /* __FIRMWARE_FALLBACK_H */
diff --git a/drivers/base/firmware_fallback_table.c b/drivers/base/firmware_fallback_table.c
new file mode 100644
index 000000000000..53cc4e492520
--- /dev/null
+++ b/drivers/base/firmware_fallback_table.c
@@ -0,0 +1,29 @@
+// SPDX-License-Identifier: GPL-2.0
+
+#include <linux/types.h>
+#include <linux/kconfig.h>
+#include <linux/list.h>
+#include <linux/slab.h>
+#include <linux/security.h>
+#include <linux/highmem.h>
+#include <linux/umh.h>
+#include <linux/sysctl.h>
+
+#include "firmware_fallback.h"
+#include "firmware_loader.h"
+
+/*
+ * firmware fallback configuration table
+ */
+
+/* Module or buit-in */
+#ifdef CONFIG_FW_LOADER_USER_HELPER
+
+struct firmware_fallback_config fw_fallback_config = {
+ .force_sysfs_fallback = IS_ENABLED(CONFIG_FW_LOADER_USER_HELPER_FALLBACK),
+ .loading_timeout = 60,
+ .old_timeout = 60,
+};
+EXPORT_SYMBOL_GPL(fw_fallback_config);
+
+#endif
diff --git a/drivers/base/firmware_loader.c b/drivers/base/firmware_loader.c
index 9757f9fff01e..21dd31ef08ae 100644
--- a/drivers/base/firmware_loader.c
+++ b/drivers/base/firmware_loader.c
@@ -37,36 +37,13 @@
#include <generated/utsrelease.h>
#include "base.h"
+#include "firmware_loader.h"
+#include "firmware_fallback.h"
MODULE_AUTHOR("Manuel Estrada Sainz");
MODULE_DESCRIPTION("Multi purpose firmware loading support");
MODULE_LICENSE("GPL");
-enum fw_status {
- FW_STATUS_UNKNOWN,
- FW_STATUS_LOADING,
- FW_STATUS_DONE,
- FW_STATUS_ABORTED,
-};
-
-/*
- * Concurrent request_firmware() for the same firmware need to be
- * serialized. struct fw_state is simple state machine which hold the
- * state of the firmware loading.
- */
-struct fw_state {
- struct completion completion;
- enum fw_status status;
-};
-
-/* firmware behavior options */
-#define FW_OPT_UEVENT (1U << 0)
-#define FW_OPT_NOWAIT (1U << 1)
-#define FW_OPT_USERHELPER (1U << 2)
-#define FW_OPT_NO_WARN (1U << 3)
-#define FW_OPT_NOCACHE (1U << 4)
-#define FW_OPT_NOFALLBACK (1U << 5)
-
struct firmware_cache {
/* firmware_buf instance will be added into the below list */
spinlock_t lock;
@@ -89,25 +66,6 @@ struct firmware_cache {
#endif
};
-struct fw_priv {
- struct kref ref;
- struct list_head list;
- struct firmware_cache *fwc;
- struct fw_state fw_st;
- void *data;
- size_t size;
- size_t allocated_size;
-#ifdef CONFIG_FW_LOADER_USER_HELPER
- bool is_paged_buf;
- bool need_uevent;
- struct page **pages;
- int nr_pages;
- int page_array_size;
- struct list_head pending_list;
-#endif
- const char *fw_name;
-};
-
struct fw_cache_entry {
struct list_head list;
const char *name;
@@ -128,7 +86,7 @@ static inline struct fw_priv *to_fw_priv(struct kref *ref)
/* fw_lock could be moved to 'struct fw_sysfs' but since it is just
* guarding for corner cases a global lock should be OK */
-static DEFINE_MUTEX(fw_lock);
+DEFINE_MUTEX(fw_lock);
static struct firmware_cache fw_cache;
@@ -199,142 +157,11 @@ static void fw_state_init(struct fw_priv *fw_priv)
fw_st->status = FW_STATUS_UNKNOWN;
}
-static int __fw_state_wait_common(struct fw_priv *fw_priv, long timeout)
-{
- struct fw_state *fw_st = &fw_priv->fw_st;
- long ret;
-
- ret = wait_for_completion_killable_timeout(&fw_st->completion, timeout);
- if (ret != 0 && fw_st->status == FW_STATUS_ABORTED)
- return -ENOENT;
- if (!ret)
- return -ETIMEDOUT;
-
- return ret < 0 ? ret : 0;
-}
-
-static void __fw_state_set(struct fw_priv *fw_priv,
- enum fw_status status)
-{
- struct fw_state *fw_st = &fw_priv->fw_st;
-
- WRITE_ONCE(fw_st->status, status);
-
- if (status == FW_STATUS_DONE || status == FW_STATUS_ABORTED)
- complete_all(&fw_st->completion);
-}
-
-static inline void fw_state_start(struct fw_priv *fw_priv)
-{
- __fw_state_set(fw_priv, FW_STATUS_LOADING);
-}
-
-static inline void fw_state_done(struct fw_priv *fw_priv)
-{
- __fw_state_set(fw_priv, FW_STATUS_DONE);
-}
-
-static inline void fw_state_aborted(struct fw_priv *fw_priv)
-{
- __fw_state_set(fw_priv, FW_STATUS_ABORTED);
-}
-
static inline int fw_state_wait(struct fw_priv *fw_priv)
{
return __fw_state_wait_common(fw_priv, MAX_SCHEDULE_TIMEOUT);
}
-static bool __fw_state_check(struct fw_priv *fw_priv,
- enum fw_status status)
-{
- struct fw_state *fw_st = &fw_priv->fw_st;
-
- return fw_st->status == status;
-}
-
-static inline bool fw_state_is_aborted(struct fw_priv *fw_priv)
-{
- return __fw_state_check(fw_priv, FW_STATUS_ABORTED);
-}
-
-#ifdef CONFIG_FW_LOADER_USER_HELPER
-
-/**
- * struct firmware_fallback_config - firmware fallback configuratioon settings
- *
- * Helps describe and fine tune the fallback mechanism.
- *
- * @force_sysfs_fallback: force the sysfs fallback mechanism to be used
- * as if one had enabled CONFIG_FW_LOADER_USER_HELPER_FALLBACK=y.
- * @old_timeout: for internal use
- * @loading_timeout: the timeout to wait for the fallback mechanism before
- * giving up, in seconds.
- */
-struct firmware_fallback_config {
- const bool force_sysfs_fallback;
- int old_timeout;
- int loading_timeout;
-};
-
-static struct firmware_fallback_config fw_fallback_config = {
- .force_sysfs_fallback = IS_ENABLED(CONFIG_FW_LOADER_USER_HELPER_FALLBACK),
- .loading_timeout = 60,
- .old_timeout = 60,
-};
-
-/* These getters are vetted to use int properly */
-static inline int __firmware_loading_timeout(void)
-{
- return fw_fallback_config.loading_timeout;
-}
-
-/* These setters are vetted to use int properly */
-static void __fw_fallback_set_timeout(int timeout)
-{
- fw_fallback_config.loading_timeout = timeout;
-}
-
-static inline long firmware_loading_timeout(void)
-{
- return __firmware_loading_timeout() > 0 ?
- __firmware_loading_timeout() * HZ : MAX_JIFFY_OFFSET;
-}
-
-/*
- * use small loading timeout for caching devices' firmware because all these
- * firmware images have been loaded successfully at lease once, also system is
- * ready for completing firmware loading now. The maximum size of firmware in
- * current distributions is about 2M bytes, so 10 secs should be enough.
- */
-static void fw_fallback_set_cache_timeout(void)
-{
- fw_fallback_config.old_timeout = __firmware_loading_timeout();
- __fw_fallback_set_timeout(10);
-}
-
-/* Restores the timeout to the value last configured during normal operation */
-static void fw_fallback_set_default_timeout(void)
-{
- __fw_fallback_set_timeout(fw_fallback_config.old_timeout);
-}
-
-static inline bool fw_sysfs_done(struct fw_priv *fw_priv)
-{
- return __fw_state_check(fw_priv, FW_STATUS_DONE);
-}
-
-static inline bool fw_sysfs_loading(struct fw_priv *fw_priv)
-{
- return __fw_state_check(fw_priv, FW_STATUS_LOADING);
-}
-
-static inline int fw_sysfs_wait_timeout(struct fw_priv *fw_priv, long timeout)
-{
- return __fw_state_wait_common(fw_priv, timeout);
-}
-
-#endif /* CONFIG_FW_LOADER_USER_HELPER */
-
static int fw_cache_piggyback_on_request(const char *name);
static struct fw_priv *__allocate_fw_priv(const char *fw_name,
@@ -600,8 +427,8 @@ static int fw_add_devm_name(struct device *dev, const char *name)
}
#endif
-static int assign_fw(struct firmware *fw, struct device *device,
- unsigned int opt_flags)
+int assign_fw(struct firmware *fw, struct device *device,
+ unsigned int opt_flags)
{
struct fw_priv *fw_priv = fw->priv;
@@ -639,626 +466,6 @@ static int assign_fw(struct firmware *fw, struct device *device,
return 0;
}
-/*
- * user-mode helper code
- */
-#ifdef CONFIG_FW_LOADER_USER_HELPER
-struct fw_sysfs {
- bool nowait;
- struct device dev;
- struct fw_priv *fw_priv;
- struct firmware *fw;
-};
-
-static struct fw_sysfs *to_fw_sysfs(struct device *dev)
-{
- return container_of(dev, struct fw_sysfs, dev);
-}
-
-static void __fw_load_abort(struct fw_priv *fw_priv)
-{
- /*
- * There is a small window in which user can write to 'loading'
- * between loading done and disappearance of 'loading'
- */
- if (fw_sysfs_done(fw_priv))
- return;
-
- list_del_init(&fw_priv->pending_list);
- fw_state_aborted(fw_priv);
-}
-
-static void fw_load_abort(struct fw_sysfs *fw_sysfs)
-{
- struct fw_priv *fw_priv = fw_sysfs->fw_priv;
-
- __fw_load_abort(fw_priv);
-}
-
-static LIST_HEAD(pending_fw_head);
-
-static void kill_pending_fw_fallback_reqs(bool only_kill_custom)
-{
- struct fw_priv *fw_priv;
- struct fw_priv *next;
-
- mutex_lock(&fw_lock);
- list_for_each_entry_safe(fw_priv, next, &pending_fw_head,
- pending_list) {
- if (!fw_priv->need_uevent || !only_kill_custom)
- __fw_load_abort(fw_priv);
- }
- mutex_unlock(&fw_lock);
-}
-
-static ssize_t timeout_show(struct class *class, struct class_attribute *attr,
- char *buf)
-{
- return sprintf(buf, "%d\n", __firmware_loading_timeout());
-}
-
-/**
- * firmware_timeout_store - set number of seconds to wait for firmware
- * @class: device class pointer
- * @attr: device attribute pointer
- * @buf: buffer to scan for timeout value
- * @count: number of bytes in @buf
- *
- * Sets the number of seconds to wait for the firmware. Once
- * this expires an error will be returned to the driver and no
- * firmware will be provided.
- *
- * Note: zero means 'wait forever'.
- **/
-static ssize_t timeout_store(struct class *class, struct class_attribute *attr,
- const char *buf, size_t count)
-{
- int tmp_loading_timeout = simple_strtol(buf, NULL, 10);
-
- if (tmp_loading_timeout < 0)
- tmp_loading_timeout = 0;
-
- __fw_fallback_set_timeout(tmp_loading_timeout);
-
- return count;
-}
-static CLASS_ATTR_RW(timeout);
-
-static struct attribute *firmware_class_attrs[] = {
- &class_attr_timeout.attr,
- NULL,
-};
-ATTRIBUTE_GROUPS(firmware_class);
-
-static void fw_dev_release(struct device *dev)
-{
- struct fw_sysfs *fw_sysfs = to_fw_sysfs(dev);
-
- kfree(fw_sysfs);
-}
-
-static int do_firmware_uevent(struct fw_sysfs *fw_sysfs, struct kobj_uevent_env *env)
-{
- if (add_uevent_var(env, "FIRMWARE=%s", fw_sysfs->fw_priv->fw_name))
- return -ENOMEM;
- if (add_uevent_var(env, "TIMEOUT=%i", __firmware_loading_timeout()))
- return -ENOMEM;
- if (add_uevent_var(env, "ASYNC=%d", fw_sysfs->nowait))
- return -ENOMEM;
-
- return 0;
-}
-
-static int firmware_uevent(struct device *dev, struct kobj_uevent_env *env)
-{
- struct fw_sysfs *fw_sysfs = to_fw_sysfs(dev);
- int err = 0;
-
- mutex_lock(&fw_lock);
- if (fw_sysfs->fw_priv)
- err = do_firmware_uevent(fw_sysfs, env);
- mutex_unlock(&fw_lock);
- return err;
-}
-
-static struct class firmware_class = {
- .name = "firmware",
- .class_groups = firmware_class_groups,
- .dev_uevent = firmware_uevent,
- .dev_release = fw_dev_release,
-};
-
-static inline int register_sysfs_loader(void)
-{
- return class_register(&firmware_class);
-}
-
-static inline void unregister_sysfs_loader(void)
-{
- class_unregister(&firmware_class);
-}
-
-static ssize_t firmware_loading_show(struct device *dev,
- struct device_attribute *attr, char *buf)
-{
- struct fw_sysfs *fw_sysfs = to_fw_sysfs(dev);
- int loading = 0;
-
- mutex_lock(&fw_lock);
- if (fw_sysfs->fw_priv)
- loading = fw_sysfs_loading(fw_sysfs->fw_priv);
- mutex_unlock(&fw_lock);
-
- return sprintf(buf, "%d\n", loading);
-}
-
-/* Some architectures don't have PAGE_KERNEL_RO */
-#ifndef PAGE_KERNEL_RO
-#define PAGE_KERNEL_RO PAGE_KERNEL
-#endif
-
-/* one pages buffer should be mapped/unmapped only once */
-static int map_fw_priv_pages(struct fw_priv *fw_priv)
-{
- if (!fw_priv->is_paged_buf)
- return 0;
-
- vunmap(fw_priv->data);
- fw_priv->data = vmap(fw_priv->pages, fw_priv->nr_pages, 0,
- PAGE_KERNEL_RO);
- if (!fw_priv->data)
- return -ENOMEM;
- return 0;
-}
-
-/**
- * firmware_loading_store - set value in the 'loading' control file
- * @dev: device pointer
- * @attr: device attribute pointer
- * @buf: buffer to scan for loading control value
- * @count: number of bytes in @buf
- *
- * The relevant values are:
- *
- * 1: Start a load, discarding any previous partial load.
- * 0: Conclude the load and hand the data to the driver code.
- * -1: Conclude the load with an error and discard any written data.
- **/
-static ssize_t firmware_loading_store(struct device *dev,
- struct device_attribute *attr,
- const char *buf, size_t count)
-{
- struct fw_sysfs *fw_sysfs = to_fw_sysfs(dev);
- struct fw_priv *fw_priv;
- ssize_t written = count;
- int loading = simple_strtol(buf, NULL, 10);
- int i;
-
- mutex_lock(&fw_lock);
- fw_priv = fw_sysfs->fw_priv;
- if (fw_state_is_aborted(fw_priv))
- goto out;
-
- switch (loading) {
- case 1:
- /* discarding any previous partial load */
- if (!fw_sysfs_done(fw_priv)) {
- for (i = 0; i < fw_priv->nr_pages; i++)
- __free_page(fw_priv->pages[i]);
- vfree(fw_priv->pages);
- fw_priv->pages = NULL;
- fw_priv->page_array_size = 0;
- fw_priv->nr_pages = 0;
- fw_state_start(fw_priv);
- }
- break;
- case 0:
- if (fw_sysfs_loading(fw_priv)) {
- int rc;
-
- /*
- * Several loading requests may be pending on
- * one same firmware buf, so let all requests
- * see the mapped 'buf->data' once the loading
- * is completed.
- * */
- rc = map_fw_priv_pages(fw_priv);
- if (rc)
- dev_err(dev, "%s: map pages failed\n",
- __func__);
- else
- rc = security_kernel_post_read_file(NULL,
- fw_priv->data, fw_priv->size,
- READING_FIRMWARE);
-
- /*
- * Same logic as fw_load_abort, only the DONE bit
- * is ignored and we set ABORT only on failure.
- */
- list_del_init(&fw_priv->pending_list);
- if (rc) {
- fw_state_aborted(fw_priv);
- written = rc;
- } else {
- fw_state_done(fw_priv);
- }
- break;
- }
- /* fallthrough */
- default:
- dev_err(dev, "%s: unexpected value (%d)\n", __func__, loading);
- /* fallthrough */
- case -1:
- fw_load_abort(fw_sysfs);
- break;
- }
-out:
- mutex_unlock(&fw_lock);
- return written;
-}
-
-static DEVICE_ATTR(loading, 0644, firmware_loading_show, firmware_loading_store);
-
-static void firmware_rw_data(struct fw_priv *fw_priv, char *buffer,
- loff_t offset, size_t count, bool read)
-{
- if (read)
- memcpy(buffer, fw_priv->data + offset, count);
- else
- memcpy(fw_priv->data + offset, buffer, count);
-}
-
-static void firmware_rw(struct fw_priv *fw_priv, char *buffer,
- loff_t offset, size_t count, bool read)
-{
- while (count) {
- void *page_data;
- int page_nr = offset >> PAGE_SHIFT;
- int page_ofs = offset & (PAGE_SIZE-1);
- int page_cnt = min_t(size_t, PAGE_SIZE - page_ofs, count);
-
- page_data = kmap(fw_priv->pages[page_nr]);
-
- if (read)
- memcpy(buffer, page_data + page_ofs, page_cnt);
- else
- memcpy(page_data + page_ofs, buffer, page_cnt);
-
- kunmap(fw_priv->pages[page_nr]);
- buffer += page_cnt;
- offset += page_cnt;
- count -= page_cnt;
- }
-}
-
-static ssize_t firmware_data_read(struct file *filp, struct kobject *kobj,
- struct bin_attribute *bin_attr,
- char *buffer, loff_t offset, size_t count)
-{
- struct device *dev = kobj_to_dev(kobj);
- struct fw_sysfs *fw_sysfs = to_fw_sysfs(dev);
- struct fw_priv *fw_priv;
- ssize_t ret_count;
-
- mutex_lock(&fw_lock);
- fw_priv = fw_sysfs->fw_priv;
- if (!fw_priv || fw_sysfs_done(fw_priv)) {
- ret_count = -ENODEV;
- goto out;
- }
- if (offset > fw_priv->size) {
- ret_count = 0;
- goto out;
- }
- if (count > fw_priv->size - offset)
- count = fw_priv->size - offset;
-
- ret_count = count;
-
- if (fw_priv->data)
- firmware_rw_data(fw_priv, buffer, offset, count, true);
- else
- firmware_rw(fw_priv, buffer, offset, count, true);
-
-out:
- mutex_unlock(&fw_lock);
- return ret_count;
-}
-
-static int fw_realloc_pages(struct fw_sysfs *fw_sysfs, int min_size)
-{
- struct fw_priv *fw_priv= fw_sysfs->fw_priv;
- int pages_needed = PAGE_ALIGN(min_size) >> PAGE_SHIFT;
-
- /* If the array of pages is too small, grow it... */
- if (fw_priv->page_array_size < pages_needed) {
- int new_array_size = max(pages_needed,
- fw_priv->page_array_size * 2);
- struct page **new_pages;
-
- new_pages = vmalloc(new_array_size * sizeof(void *));
- if (!new_pages) {
- fw_load_abort(fw_sysfs);
- return -ENOMEM;
- }
- memcpy(new_pages, fw_priv->pages,
- fw_priv->page_array_size * sizeof(void *));
- memset(&new_pages[fw_priv->page_array_size], 0, sizeof(void *) *
- (new_array_size - fw_priv->page_array_size));
- vfree(fw_priv->pages);
- fw_priv->pages = new_pages;
- fw_priv->page_array_size = new_array_size;
- }
-
- while (fw_priv->nr_pages < pages_needed) {
- fw_priv->pages[fw_priv->nr_pages] =
- alloc_page(GFP_KERNEL | __GFP_HIGHMEM);
-
- if (!fw_priv->pages[fw_priv->nr_pages]) {
- fw_load_abort(fw_sysfs);
- return -ENOMEM;
- }
- fw_priv->nr_pages++;
- }
- return 0;
-}
-
-/**
- * firmware_data_write - write method for firmware
- * @filp: open sysfs file
- * @kobj: kobject for the device
- * @bin_attr: bin_attr structure
- * @buffer: buffer being written
- * @offset: buffer offset for write in total data store area
- * @count: buffer size
- *
- * Data written to the 'data' attribute will be later handed to
- * the driver as a firmware image.
- **/
-static ssize_t firmware_data_write(struct file *filp, struct kobject *kobj,
- struct bin_attribute *bin_attr,
- char *buffer, loff_t offset, size_t count)
-{
- struct device *dev = kobj_to_dev(kobj);
- struct fw_sysfs *fw_sysfs = to_fw_sysfs(dev);
- struct fw_priv *fw_priv;
- ssize_t retval;
-
- if (!capable(CAP_SYS_RAWIO))
- return -EPERM;
-
- mutex_lock(&fw_lock);
- fw_priv = fw_sysfs->fw_priv;
- if (!fw_priv || fw_sysfs_done(fw_priv)) {
- retval = -ENODEV;
- goto out;
- }
-
- if (fw_priv->data) {
- if (offset + count > fw_priv->allocated_size) {
- retval = -ENOMEM;
- goto out;
- }
- firmware_rw_data(fw_priv, buffer, offset, count, false);
- retval = count;
- } else {
- retval = fw_realloc_pages(fw_sysfs, offset + count);
- if (retval)
- goto out;
-
- retval = count;
- firmware_rw(fw_priv, buffer, offset, count, false);
- }
-
- fw_priv->size = max_t(size_t, offset + count, fw_priv->size);
-out:
- mutex_unlock(&fw_lock);
- return retval;
-}
-
-static struct bin_attribute firmware_attr_data = {
- .attr = { .name = "data", .mode = 0644 },
- .size = 0,
- .read = firmware_data_read,
- .write = firmware_data_write,
-};
-
-static struct attribute *fw_dev_attrs[] = {
- &dev_attr_loading.attr,
- NULL
-};
-
-static struct bin_attribute *fw_dev_bin_attrs[] = {
- &firmware_attr_data,
- NULL
-};
-
-static const struct attribute_group fw_dev_attr_group = {
- .attrs = fw_dev_attrs,
- .bin_attrs = fw_dev_bin_attrs,
-};
-
-static const struct attribute_group *fw_dev_attr_groups[] = {
- &fw_dev_attr_group,
- NULL
-};
-
-static struct fw_sysfs *
-fw_create_instance(struct firmware *firmware, const char *fw_name,
- struct device *device, unsigned int opt_flags)
-{
- struct fw_sysfs *fw_sysfs;
- struct device *f_dev;
-
- fw_sysfs = kzalloc(sizeof(*fw_sysfs), GFP_KERNEL);
- if (!fw_sysfs) {
- fw_sysfs = ERR_PTR(-ENOMEM);
- goto exit;
- }
-
- fw_sysfs->nowait = !!(opt_flags & FW_OPT_NOWAIT);
- fw_sysfs->fw = firmware;
- f_dev = &fw_sysfs->dev;
-
- device_initialize(f_dev);
- dev_set_name(f_dev, "%s", fw_name);
- f_dev->parent = device;
- f_dev->class = &firmware_class;
- f_dev->groups = fw_dev_attr_groups;
-exit:
- return fw_sysfs;
-}
-
-/* load a firmware via user helper */
-static int _request_firmware_load(struct fw_sysfs *fw_sysfs,
- unsigned int opt_flags, long timeout)
-{
- int retval = 0;
- struct device *f_dev = &fw_sysfs->dev;
- struct fw_priv *fw_priv = fw_sysfs->fw_priv;
-
- /* fall back on userspace loading */
- if (!fw_priv->data)
- fw_priv->is_paged_buf = true;
-
- dev_set_uevent_suppress(f_dev, true);
-
- retval = device_add(f_dev);
- if (retval) {
- dev_err(f_dev, "%s: device_register failed\n", __func__);
- goto err_put_dev;
- }
-
- mutex_lock(&fw_lock);
- list_add(&fw_priv->pending_list, &pending_fw_head);
- mutex_unlock(&fw_lock);
-
- if (opt_flags & FW_OPT_UEVENT) {
- fw_priv->need_uevent = true;
- dev_set_uevent_suppress(f_dev, false);
- dev_dbg(f_dev, "firmware: requesting %s\n", fw_priv->fw_name);
- kobject_uevent(&fw_sysfs->dev.kobj, KOBJ_ADD);
- } else {
- timeout = MAX_JIFFY_OFFSET;
- }
-
- retval = fw_sysfs_wait_timeout(fw_priv, timeout);
- if (retval < 0) {
- mutex_lock(&fw_lock);
- fw_load_abort(fw_sysfs);
- mutex_unlock(&fw_lock);
- }
-
- if (fw_state_is_aborted(fw_priv)) {
- if (retval == -ERESTARTSYS)
- retval = -EINTR;
- else
- retval = -EAGAIN;
- } else if (fw_priv->is_paged_buf && !fw_priv->data)
- retval = -ENOMEM;
-
- device_del(f_dev);
-err_put_dev:
- put_device(f_dev);
- return retval;
-}
-
-static int fw_load_from_user_helper(struct firmware *firmware,
- const char *name, struct device *device,
- unsigned int opt_flags)
-{
- struct fw_sysfs *fw_sysfs;
- long timeout;
- int ret;
-
- timeout = firmware_loading_timeout();
- if (opt_flags & FW_OPT_NOWAIT) {
- timeout = usermodehelper_read_lock_wait(timeout);
- if (!timeout) {
- dev_dbg(device, "firmware: %s loading timed out\n",
- name);
- return -EBUSY;
- }
- } else {
- ret = usermodehelper_read_trylock();
- if (WARN_ON(ret)) {
- dev_err(device, "firmware: %s will not be loaded\n",
- name);
- return ret;
- }
- }
-
- fw_sysfs = fw_create_instance(firmware, name, device, opt_flags);
- if (IS_ERR(fw_sysfs)) {
- ret = PTR_ERR(fw_sysfs);
- goto out_unlock;
- }
-
- fw_sysfs->fw_priv = firmware->priv;
- ret = _request_firmware_load(fw_sysfs, opt_flags, timeout);
-
- if (!ret)
- ret = assign_fw(firmware, device, opt_flags);
-
-out_unlock:
- usermodehelper_read_unlock();
-
- return ret;
-}
-
-static bool fw_force_sysfs_fallback(unsigned int opt_flags)
-{
- if (fw_fallback_config.force_sysfs_fallback)
- return true;
- if (!(opt_flags & FW_OPT_USERHELPER))
- return false;
- return true;
-}
-
-static bool fw_run_sysfs_fallback(unsigned int opt_flags)
-{
- if ((opt_flags & FW_OPT_NOFALLBACK))
- return false;
-
- return fw_force_sysfs_fallback(opt_flags);
-}
-
-static int fw_sysfs_fallback(struct firmware *fw, const char *name,
- struct device *device,
- unsigned int opt_flags,
- int ret)
-{
- if (!fw_run_sysfs_fallback(opt_flags))
- return ret;
-
- dev_warn(device, "Falling back to user helper\n");
- return fw_load_from_user_helper(fw, name, device, opt_flags);
-}
-#else /* CONFIG_FW_LOADER_USER_HELPER */
-static int fw_sysfs_fallback(struct firmware *fw, const char *name,
- struct device *device,
- unsigned int opt_flags,
- int ret)
-{
- /* Keep carrying over the same error */
- return ret;
-}
-
-static inline void kill_pending_fw_fallback_reqs(bool only_kill_custom) { }
-static inline void fw_fallback_set_cache_timeout(void) { }
-static inline void fw_fallback_set_default_timeout(void) { }
-
-static inline int register_sysfs_loader(void)
-{
- return 0;
-}
-
-static inline void unregister_sysfs_loader(void)
-{
-}
-
-#endif /* CONFIG_FW_LOADER_USER_HELPER */
-
/* prepare firmware and firmware_buf structs;
* return 0 if a firmware is already assigned, 1 if need to load one,
* or a negative error code
diff --git a/drivers/base/firmware_loader.h b/drivers/base/firmware_loader.h
new file mode 100644
index 000000000000..64acbb1a392c
--- /dev/null
+++ b/drivers/base/firmware_loader.h
@@ -0,0 +1,115 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef __FIRMWARE_LOADER_H
+#define __FIRMWARE_LOADER_H
+
+#include <linux/firmware.h>
+#include <linux/types.h>
+#include <linux/kref.h>
+#include <linux/list.h>
+#include <linux/completion.h>
+
+#include <generated/utsrelease.h>
+
+/* firmware behavior options */
+#define FW_OPT_UEVENT (1U << 0)
+#define FW_OPT_NOWAIT (1U << 1)
+#define FW_OPT_USERHELPER (1U << 2)
+#define FW_OPT_NO_WARN (1U << 3)
+#define FW_OPT_NOCACHE (1U << 4)
+#define FW_OPT_NOFALLBACK (1U << 5)
+
+enum fw_status {
+ FW_STATUS_UNKNOWN,
+ FW_STATUS_LOADING,
+ FW_STATUS_DONE,
+ FW_STATUS_ABORTED,
+};
+
+/*
+ * Concurrent request_firmware() for the same firmware need to be
+ * serialized. struct fw_state is simple state machine which hold the
+ * state of the firmware loading.
+ */
+struct fw_state {
+ struct completion completion;
+ enum fw_status status;
+};
+
+struct fw_priv {
+ struct kref ref;
+ struct list_head list;
+ struct firmware_cache *fwc;
+ struct fw_state fw_st;
+ void *data;
+ size_t size;
+ size_t allocated_size;
+#ifdef CONFIG_FW_LOADER_USER_HELPER
+ bool is_paged_buf;
+ bool need_uevent;
+ struct page **pages;
+ int nr_pages;
+ int page_array_size;
+ struct list_head pending_list;
+#endif
+ const char *fw_name;
+};
+
+extern struct mutex fw_lock;
+
+static inline bool __fw_state_check(struct fw_priv *fw_priv,
+ enum fw_status status)
+{
+ struct fw_state *fw_st = &fw_priv->fw_st;
+
+ return fw_st->status == status;
+}
+
+static inline int __fw_state_wait_common(struct fw_priv *fw_priv, long timeout)
+{
+ struct fw_state *fw_st = &fw_priv->fw_st;
+ long ret;
+
+ ret = wait_for_completion_killable_timeout(&fw_st->completion, timeout);
+ if (ret != 0 && fw_st->status == FW_STATUS_ABORTED)
+ return -ENOENT;
+ if (!ret)
+ return -ETIMEDOUT;
+
+ return ret < 0 ? ret : 0;
+}
+
+static inline void __fw_state_set(struct fw_priv *fw_priv,
+ enum fw_status status)
+{
+ struct fw_state *fw_st = &fw_priv->fw_st;
+
+ WRITE_ONCE(fw_st->status, status);
+
+ if (status == FW_STATUS_DONE || status == FW_STATUS_ABORTED)
+ complete_all(&fw_st->completion);
+}
+
+static inline void fw_state_aborted(struct fw_priv *fw_priv)
+{
+ __fw_state_set(fw_priv, FW_STATUS_ABORTED);
+}
+
+static inline bool fw_state_is_aborted(struct fw_priv *fw_priv)
+{
+ return __fw_state_check(fw_priv, FW_STATUS_ABORTED);
+}
+
+static inline void fw_state_start(struct fw_priv *fw_priv)
+{
+ __fw_state_set(fw_priv, FW_STATUS_LOADING);
+}
+
+static inline void fw_state_done(struct fw_priv *fw_priv)
+{
+ __fw_state_set(fw_priv, FW_STATUS_DONE);
+}
+
+int assign_fw(struct firmware *fw, struct device *device,
+ unsigned int opt_flags);
+
+#endif /* __FIRMWARE_LOADER_H */
--
2.16.2
^ permalink raw reply related [flat|nested] 39+ messages in thread* [PATCH v3 09/20] firmware: move firmware loader into its own directory
2018-03-10 14:14 [PATCH v3 00/20] firmware: development for v4.17 Luis R. Rodriguez
` (7 preceding siblings ...)
2018-03-10 14:14 ` [PATCH v3 08/20] firmware: split firmware fallback functionality into its own file Luis R. Rodriguez
@ 2018-03-10 14:14 ` Luis R. Rodriguez
2018-03-10 14:14 ` [PATCH v3 10/20] firmware: enable run time change of forcing fallback loader Luis R. Rodriguez
` (11 subsequent siblings)
20 siblings, 0 replies; 39+ messages in thread
From: Luis R. Rodriguez @ 2018-03-10 14:14 UTC (permalink / raw)
To: gregkh
Cc: akpm, cantabile.desu, kubakici, linux-wireless, keescook, shuah,
mfuzzey, zohar, dhowells, pali.rohar, tiwai, arend.vanspriel,
zajec5, nbroeking, markivx, broonie, dmitry.torokhov, dwmw2,
torvalds, Abhay_Salunke, bjorn.andersson, jewalt, oneukum, ast,
andresx7, linux-kernel, linux-fsdevel, Luis R. Rodriguez
This will make it much easier to manage as we manage to
keep trimming componnents down into their own files to more
easily manage and maintain this codebase.
Suggested-by: Kees Cook <keescook@chromium.org>
Signed-off-by: Luis R. Rodriguez <mcgrof@kernel.org>
---
MAINTAINERS | 2 +-
drivers/base/Makefile | 7 ++-----
drivers/base/firmware_loader/Makefile | 7 +++++++
drivers/base/{firmware_fallback.c => firmware_loader/fallback.c} | 4 ++--
drivers/base/{firmware_fallback.h => firmware_loader/fallback.h} | 0
.../fallback_table.c} | 4 ++--
drivers/base/{firmware_loader.h => firmware_loader/firmware.h} | 0
drivers/base/{firmware_loader.c => firmware_loader/main.c} | 8 ++++----
8 files changed, 18 insertions(+), 14 deletions(-)
create mode 100644 drivers/base/firmware_loader/Makefile
rename drivers/base/{firmware_fallback.c => firmware_loader/fallback.c} (99%)
rename drivers/base/{firmware_fallback.h => firmware_loader/fallback.h} (100%)
rename drivers/base/{firmware_fallback_table.c => firmware_loader/fallback_table.c} (90%)
rename drivers/base/{firmware_loader.h => firmware_loader/firmware.h} (100%)
rename drivers/base/{firmware_loader.c => firmware_loader/main.c} (99%)
diff --git a/MAINTAINERS b/MAINTAINERS
index e03a130902cd..6ddd6f4aaffa 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -5589,7 +5589,7 @@ M: Luis R. Rodriguez <mcgrof@kernel.org>
L: linux-kernel@vger.kernel.org
S: Maintained
F: Documentation/firmware_class/
-F: drivers/base/firmware*.c
+F: drivers/base/firmware_loader/
F: include/linux/firmware.h
FLASH ADAPTER DRIVER (IBM Flash Adapter 900GB Full Height PCI Flash Card)
diff --git a/drivers/base/Makefile b/drivers/base/Makefile
index b946a408256d..b9539abec675 100644
--- a/drivers/base/Makefile
+++ b/drivers/base/Makefile
@@ -5,17 +5,14 @@ obj-y := component.o core.o bus.o dd.o syscore.o \
driver.o class.o platform.o \
cpu.o firmware.o init.o map.o devres.o \
attribute_container.o transport_class.o \
- topology.o container.o property.o cacheinfo.o \
- firmware_fallback_table.o
+ topology.o container.o property.o cacheinfo.o
obj-$(CONFIG_DEVTMPFS) += devtmpfs.o
obj-$(CONFIG_DMA_CMA) += dma-contiguous.o
obj-y += power/
obj-$(CONFIG_HAS_DMA) += dma-mapping.o
obj-$(CONFIG_HAVE_GENERIC_DMA_COHERENT) += dma-coherent.o
obj-$(CONFIG_ISA_BUS_API) += isa.o
-obj-$(CONFIG_FW_LOADER) += firmware_class.o
-firmware_class-objs := firmware_loader.o
-firmware_class-$(CONFIG_FW_LOADER_USER_HELPER) += firmware_fallback.o
+obj-y += firmware_loader/
obj-$(CONFIG_NUMA) += node.o
obj-$(CONFIG_MEMORY_HOTPLUG_SPARSE) += memory.o
ifeq ($(CONFIG_SYSFS),y)
diff --git a/drivers/base/firmware_loader/Makefile b/drivers/base/firmware_loader/Makefile
new file mode 100644
index 000000000000..a97eeb0be1d8
--- /dev/null
+++ b/drivers/base/firmware_loader/Makefile
@@ -0,0 +1,7 @@
+# SPDX-License-Identifier: GPL-2.0
+# Makefile for the Linux firmware loader
+
+obj-y := fallback_table.o
+obj-$(CONFIG_FW_LOADER) += firmware_class.o
+firmware_class-objs := main.o
+firmware_class-$(CONFIG_FW_LOADER_USER_HELPER) += fallback.o
diff --git a/drivers/base/firmware_fallback.c b/drivers/base/firmware_loader/fallback.c
similarity index 99%
rename from drivers/base/firmware_fallback.c
rename to drivers/base/firmware_loader/fallback.c
index 47690207e0ee..9b65837256d6 100644
--- a/drivers/base/firmware_fallback.c
+++ b/drivers/base/firmware_loader/fallback.c
@@ -8,8 +8,8 @@
#include <linux/highmem.h>
#include <linux/umh.h>
-#include "firmware_fallback.h"
-#include "firmware_loader.h"
+#include "fallback.h"
+#include "firmware.h"
/*
* firmware fallback mechanism
diff --git a/drivers/base/firmware_fallback.h b/drivers/base/firmware_loader/fallback.h
similarity index 100%
rename from drivers/base/firmware_fallback.h
rename to drivers/base/firmware_loader/fallback.h
diff --git a/drivers/base/firmware_fallback_table.c b/drivers/base/firmware_loader/fallback_table.c
similarity index 90%
rename from drivers/base/firmware_fallback_table.c
rename to drivers/base/firmware_loader/fallback_table.c
index 53cc4e492520..981419044c7e 100644
--- a/drivers/base/firmware_fallback_table.c
+++ b/drivers/base/firmware_loader/fallback_table.c
@@ -9,8 +9,8 @@
#include <linux/umh.h>
#include <linux/sysctl.h>
-#include "firmware_fallback.h"
-#include "firmware_loader.h"
+#include "fallback.h"
+#include "firmware.h"
/*
* firmware fallback configuration table
diff --git a/drivers/base/firmware_loader.h b/drivers/base/firmware_loader/firmware.h
similarity index 100%
rename from drivers/base/firmware_loader.h
rename to drivers/base/firmware_loader/firmware.h
diff --git a/drivers/base/firmware_loader.c b/drivers/base/firmware_loader/main.c
similarity index 99%
rename from drivers/base/firmware_loader.c
rename to drivers/base/firmware_loader/main.c
index 21dd31ef08ae..c8966c84bd44 100644
--- a/drivers/base/firmware_loader.c
+++ b/drivers/base/firmware_loader/main.c
@@ -1,6 +1,6 @@
// SPDX-License-Identifier: GPL-2.0
/*
- * firmware_class.c - Multi purpose firmware loading support
+ * main.c - Multi purpose firmware loading support
*
* Copyright (c) 2003 Manuel Estrada Sainz
*
@@ -36,9 +36,9 @@
#include <generated/utsrelease.h>
-#include "base.h"
-#include "firmware_loader.h"
-#include "firmware_fallback.h"
+#include "../base.h"
+#include "firmware.h"
+#include "fallback.h"
MODULE_AUTHOR("Manuel Estrada Sainz");
MODULE_DESCRIPTION("Multi purpose firmware loading support");
--
2.16.2
^ permalink raw reply related [flat|nested] 39+ messages in thread* [PATCH v3 10/20] firmware: enable run time change of forcing fallback loader
2018-03-10 14:14 [PATCH v3 00/20] firmware: development for v4.17 Luis R. Rodriguez
` (8 preceding siblings ...)
2018-03-10 14:14 ` [PATCH v3 09/20] firmware: move firmware loader into its own directory Luis R. Rodriguez
@ 2018-03-10 14:14 ` Luis R. Rodriguez
2018-03-10 14:14 ` [PATCH v3 11/20] firmware: enable to force disable the fallback mechanism at run time Luis R. Rodriguez
` (10 subsequent siblings)
20 siblings, 0 replies; 39+ messages in thread
From: Luis R. Rodriguez @ 2018-03-10 14:14 UTC (permalink / raw)
To: gregkh
Cc: akpm, cantabile.desu, kubakici, linux-wireless, keescook, shuah,
mfuzzey, zohar, dhowells, pali.rohar, tiwai, arend.vanspriel,
zajec5, nbroeking, markivx, broonie, dmitry.torokhov, dwmw2,
torvalds, Abhay_Salunke, bjorn.andersson, jewalt, oneukum, ast,
andresx7, linux-kernel, linux-fsdevel, Luis R. Rodriguez
Currently one requires to test four kernel configurations to test the
firmware API completely:
0)
CONFIG_FW_LOADER=y
1)
o CONFIG_FW_LOADER=y
o CONFIG_FW_LOADER_USER_HELPER=y
2)
o CONFIG_FW_LOADER=y
o CONFIG_FW_LOADER_USER_HELPER=y
o CONFIG_FW_LOADER_USER_HELPER_FALLBACK=y
3) When CONFIG_FW_LOADER=m the built-in stuff is disabled, we have
no current tests for this.
We can reduce the requirements to three kernel configurations by making
fw_config.force_sysfs_fallback a proc knob we flip on off. For kernels that
disable CONFIG_IKCONFIG_PROC this can also enable one to inspect if
CONFIG_FW_LOADER_USER_HELPER_FALLBACK was enabled at build time by checking
the proc value at boot time.
Acked-by: Kees Cook <keescook@chromium.org>
Signed-off-by: Luis R. Rodriguez <mcgrof@kernel.org>
---
drivers/base/firmware_loader/fallback.c | 1 +
drivers/base/firmware_loader/fallback.h | 4 +++-
drivers/base/firmware_loader/fallback_table.c | 17 +++++++++++++++++
kernel/sysctl.c | 11 +++++++++++
4 files changed, 32 insertions(+), 1 deletion(-)
diff --git a/drivers/base/firmware_loader/fallback.c b/drivers/base/firmware_loader/fallback.c
index 9b65837256d6..45cc40933a47 100644
--- a/drivers/base/firmware_loader/fallback.c
+++ b/drivers/base/firmware_loader/fallback.c
@@ -7,6 +7,7 @@
#include <linux/security.h>
#include <linux/highmem.h>
#include <linux/umh.h>
+#include <linux/sysctl.h>
#include "fallback.h"
#include "firmware.h"
diff --git a/drivers/base/firmware_loader/fallback.h b/drivers/base/firmware_loader/fallback.h
index 550498c7fa4c..ca7e69a8417b 100644
--- a/drivers/base/firmware_loader/fallback.h
+++ b/drivers/base/firmware_loader/fallback.h
@@ -12,12 +12,14 @@
*
* @force_sysfs_fallback: force the sysfs fallback mechanism to be used
* as if one had enabled CONFIG_FW_LOADER_USER_HELPER_FALLBACK=y.
+ * Useful to help debug a CONFIG_FW_LOADER_USER_HELPER_FALLBACK=y
+ * functionality on a kernel where that config entry has been disabled.
* @old_timeout: for internal use
* @loading_timeout: the timeout to wait for the fallback mechanism before
* giving up, in seconds.
*/
struct firmware_fallback_config {
- const bool force_sysfs_fallback;
+ unsigned int force_sysfs_fallback;
int old_timeout;
int loading_timeout;
};
diff --git a/drivers/base/firmware_loader/fallback_table.c b/drivers/base/firmware_loader/fallback_table.c
index 981419044c7e..92365e053e30 100644
--- a/drivers/base/firmware_loader/fallback_table.c
+++ b/drivers/base/firmware_loader/fallback_table.c
@@ -19,6 +19,9 @@
/* Module or buit-in */
#ifdef CONFIG_FW_LOADER_USER_HELPER
+static unsigned int zero;
+static unsigned int one = 1;
+
struct firmware_fallback_config fw_fallback_config = {
.force_sysfs_fallback = IS_ENABLED(CONFIG_FW_LOADER_USER_HELPER_FALLBACK),
.loading_timeout = 60,
@@ -26,4 +29,18 @@ struct firmware_fallback_config fw_fallback_config = {
};
EXPORT_SYMBOL_GPL(fw_fallback_config);
+struct ctl_table firmware_config_table[] = {
+ {
+ .procname = "force_sysfs_fallback",
+ .data = &fw_fallback_config.force_sysfs_fallback,
+ .maxlen = sizeof(unsigned int),
+ .mode = 0644,
+ .proc_handler = proc_douintvec_minmax,
+ .extra1 = &zero,
+ .extra2 = &one,
+ },
+ { }
+};
+EXPORT_SYMBOL_GPL(firmware_config_table);
+
#endif
diff --git a/kernel/sysctl.c b/kernel/sysctl.c
index 6c68e771e11d..7a8aa6866764 100644
--- a/kernel/sysctl.c
+++ b/kernel/sysctl.c
@@ -253,6 +253,10 @@ extern struct ctl_table random_table[];
extern struct ctl_table epoll_table[];
#endif
+#ifdef CONFIG_FW_LOADER_USER_HELPER
+extern struct ctl_table firmware_config_table[];
+#endif
+
#ifdef HAVE_ARCH_PICK_MMAP_LAYOUT
int sysctl_legacy_va_layout;
#endif
@@ -748,6 +752,13 @@ static struct ctl_table kern_table[] = {
.mode = 0555,
.child = usermodehelper_table,
},
+#ifdef CONFIG_FW_LOADER_USER_HELPER
+ {
+ .procname = "firmware_config",
+ .mode = 0555,
+ .child = firmware_config_table,
+ },
+#endif
{
.procname = "overflowuid",
.data = &overflowuid,
--
2.16.2
^ permalink raw reply related [flat|nested] 39+ messages in thread* [PATCH v3 11/20] firmware: enable to force disable the fallback mechanism at run time
2018-03-10 14:14 [PATCH v3 00/20] firmware: development for v4.17 Luis R. Rodriguez
` (9 preceding siblings ...)
2018-03-10 14:14 ` [PATCH v3 10/20] firmware: enable run time change of forcing fallback loader Luis R. Rodriguez
@ 2018-03-10 14:14 ` Luis R. Rodriguez
2018-03-14 19:00 ` Greg KH
2018-03-10 14:14 ` [PATCH v3 12/20] test_firmware: expand on library with shared helpers Luis R. Rodriguez
` (9 subsequent siblings)
20 siblings, 1 reply; 39+ messages in thread
From: Luis R. Rodriguez @ 2018-03-10 14:14 UTC (permalink / raw)
To: gregkh
Cc: akpm, cantabile.desu, kubakici, linux-wireless, keescook, shuah,
mfuzzey, zohar, dhowells, pali.rohar, tiwai, arend.vanspriel,
zajec5, nbroeking, markivx, broonie, dmitry.torokhov, dwmw2,
torvalds, Abhay_Salunke, bjorn.andersson, jewalt, oneukum, ast,
andresx7, linux-kernel, linux-fsdevel, Luis R. Rodriguez
You currently need four different kernel builds to test the firmware
API fully. By adding a proc knob to force disable the fallback mechanism
completely we are able to reduce the amount of kernels you need built
to test the firmware API down to two.
Acked-by: Kees Cook <keescook@chromium.org>
Signed-off-by: Luis R. Rodriguez <mcgrof@kernel.org>
---
drivers/base/firmware_loader/fallback.c | 5 +++++
drivers/base/firmware_loader/fallback.h | 4 ++++
drivers/base/firmware_loader/fallback_table.c | 9 +++++++++
3 files changed, 18 insertions(+)
diff --git a/drivers/base/firmware_loader/fallback.c b/drivers/base/firmware_loader/fallback.c
index 45cc40933a47..d6838e7ec00c 100644
--- a/drivers/base/firmware_loader/fallback.c
+++ b/drivers/base/firmware_loader/fallback.c
@@ -643,6 +643,11 @@ static bool fw_force_sysfs_fallback(unsigned int opt_flags)
static bool fw_run_sysfs_fallback(unsigned int opt_flags)
{
+ if (fw_fallback_config.ignore_sysfs_fallback) {
+ pr_info_once("Ignoring firmware sysfs fallback due to debugfs knob\n");
+ return false;
+ }
+
if ((opt_flags & FW_OPT_NOFALLBACK))
return false;
diff --git a/drivers/base/firmware_loader/fallback.h b/drivers/base/firmware_loader/fallback.h
index ca7e69a8417b..dfebc644ed35 100644
--- a/drivers/base/firmware_loader/fallback.h
+++ b/drivers/base/firmware_loader/fallback.h
@@ -14,12 +14,16 @@
* as if one had enabled CONFIG_FW_LOADER_USER_HELPER_FALLBACK=y.
* Useful to help debug a CONFIG_FW_LOADER_USER_HELPER_FALLBACK=y
* functionality on a kernel where that config entry has been disabled.
+ * @ignore_sysfs_fallback: force to disable the sysfs fallback mechanism.
+ * This emulates the behaviour as if we had set the kernel
+ * config CONFIG_FW_LOADER_USER_HELPER=n.
* @old_timeout: for internal use
* @loading_timeout: the timeout to wait for the fallback mechanism before
* giving up, in seconds.
*/
struct firmware_fallback_config {
unsigned int force_sysfs_fallback;
+ unsigned int ignore_sysfs_fallback;
int old_timeout;
int loading_timeout;
};
diff --git a/drivers/base/firmware_loader/fallback_table.c b/drivers/base/firmware_loader/fallback_table.c
index 92365e053e30..7428659d8df9 100644
--- a/drivers/base/firmware_loader/fallback_table.c
+++ b/drivers/base/firmware_loader/fallback_table.c
@@ -39,6 +39,15 @@ struct ctl_table firmware_config_table[] = {
.extra1 = &zero,
.extra2 = &one,
},
+ {
+ .procname = "ignore_sysfs_fallback",
+ .data = &fw_fallback_config.ignore_sysfs_fallback,
+ .maxlen = sizeof(unsigned int),
+ .mode = 0644,
+ .proc_handler = proc_douintvec_minmax,
+ .extra1 = &zero,
+ .extra2 = &one,
+ },
{ }
};
EXPORT_SYMBOL_GPL(firmware_config_table);
--
2.16.2
^ permalink raw reply related [flat|nested] 39+ messages in thread* Re: [PATCH v3 11/20] firmware: enable to force disable the fallback mechanism at run time
2018-03-10 14:14 ` [PATCH v3 11/20] firmware: enable to force disable the fallback mechanism at run time Luis R. Rodriguez
@ 2018-03-14 19:00 ` Greg KH
2018-03-18 20:20 ` Luis R. Rodriguez
0 siblings, 1 reply; 39+ messages in thread
From: Greg KH @ 2018-03-14 19:00 UTC (permalink / raw)
To: Luis R. Rodriguez
Cc: akpm, cantabile.desu, kubakici, linux-wireless, keescook, shuah,
mfuzzey, zohar, dhowells, pali.rohar, tiwai, arend.vanspriel,
zajec5, nbroeking, markivx, broonie, dmitry.torokhov, dwmw2,
torvalds, Abhay_Salunke, bjorn.andersson, jewalt, oneukum, ast,
andresx7, linux-kernel, linux-fsdevel
On Sat, Mar 10, 2018 at 06:14:52AM -0800, Luis R. Rodriguez wrote:
> You currently need four different kernel builds to test the firmware
> API fully. By adding a proc knob to force disable the fallback mechanism
> completely we are able to reduce the amount of kernels you need built
> to test the firmware API down to two.
>
> Acked-by: Kees Cook <keescook@chromium.org>
> Signed-off-by: Luis R. Rodriguez <mcgrof@kernel.org>
> ---
> drivers/base/firmware_loader/fallback.c | 5 +++++
> drivers/base/firmware_loader/fallback.h | 4 ++++
> drivers/base/firmware_loader/fallback_table.c | 9 +++++++++
> 3 files changed, 18 insertions(+)
>
> diff --git a/drivers/base/firmware_loader/fallback.c b/drivers/base/firmware_loader/fallback.c
> index 45cc40933a47..d6838e7ec00c 100644
> --- a/drivers/base/firmware_loader/fallback.c
> +++ b/drivers/base/firmware_loader/fallback.c
> @@ -643,6 +643,11 @@ static bool fw_force_sysfs_fallback(unsigned int opt_flags)
>
> static bool fw_run_sysfs_fallback(unsigned int opt_flags)
> {
> + if (fw_fallback_config.ignore_sysfs_fallback) {
> + pr_info_once("Ignoring firmware sysfs fallback due to debugfs knob\n");
s/debugfs/sysctl/ right?
^ permalink raw reply [flat|nested] 39+ messages in thread* Re: [PATCH v3 11/20] firmware: enable to force disable the fallback mechanism at run time
2018-03-14 19:00 ` Greg KH
@ 2018-03-18 20:20 ` Luis R. Rodriguez
0 siblings, 0 replies; 39+ messages in thread
From: Luis R. Rodriguez @ 2018-03-18 20:20 UTC (permalink / raw)
To: Greg KH
Cc: Andrew Morton, Bitterblue of Monsea, kubakici, linux-wireless,
Kees Cook, shuah, Martin Fuzzey, Mimi Zohar, David Howells,
Pali Rohár, Takashi Iwai, Arend Van Spriel,
Rafał Miłecki, Nicolas Broeking, Vikram Mulukutla,
Mark Brown, Dmitry Torokhov, David Woodhouse, Linus Torvalds,
Abhay Salunke, Bjorn Andersson, jewalt, oneukum,
Alexei Starovoitov, Andres Rodriguez, linux-kernel,
Linux FS Devel
On Wed, Mar 14, 2018 at 12:00 PM, Greg KH <gregkh@linuxfoundation.org> wrote:
> On Sat, Mar 10, 2018 at 06:14:52AM -0800, Luis R. Rodriguez wrote:
>> You currently need four different kernel builds to test the firmware
>> API fully. By adding a proc knob to force disable the fallback mechanism
>> completely we are able to reduce the amount of kernels you need built
>> to test the firmware API down to two.
>>
>> Acked-by: Kees Cook <keescook@chromium.org>
>> Signed-off-by: Luis R. Rodriguez <mcgrof@kernel.org>
>> ---
>> drivers/base/firmware_loader/fallback.c | 5 +++++
>> drivers/base/firmware_loader/fallback.h | 4 ++++
>> drivers/base/firmware_loader/fallback_table.c | 9 +++++++++
>> 3 files changed, 18 insertions(+)
>>
>> diff --git a/drivers/base/firmware_loader/fallback.c b/drivers/base/firmware_loader/fallback.c
>> index 45cc40933a47..d6838e7ec00c 100644
>> --- a/drivers/base/firmware_loader/fallback.c
>> +++ b/drivers/base/firmware_loader/fallback.c
>> @@ -643,6 +643,11 @@ static bool fw_force_sysfs_fallback(unsigned int opt_flags)
>>
>> static bool fw_run_sysfs_fallback(unsigned int opt_flags)
>> {
>> + if (fw_fallback_config.ignore_sysfs_fallback) {
>> + pr_info_once("Ignoring firmware sysfs fallback due to debugfs knob\n");
>
> s/debugfs/sysctl/ right?
Indeed. Will respin with the other space fix. As for the other changes
let me know if you have further feedback or if I should just keep them
as-is.
Luis
^ permalink raw reply [flat|nested] 39+ messages in thread
* [PATCH v3 12/20] test_firmware: expand on library with shared helpers
2018-03-10 14:14 [PATCH v3 00/20] firmware: development for v4.17 Luis R. Rodriguez
` (10 preceding siblings ...)
2018-03-10 14:14 ` [PATCH v3 11/20] firmware: enable to force disable the fallback mechanism at run time Luis R. Rodriguez
@ 2018-03-10 14:14 ` Luis R. Rodriguez
2018-03-10 14:14 ` [PATCH v3 13/20] test_firmware: test three firmware kernel configs using a proc knob Luis R. Rodriguez
` (8 subsequent siblings)
20 siblings, 0 replies; 39+ messages in thread
From: Luis R. Rodriguez @ 2018-03-10 14:14 UTC (permalink / raw)
To: gregkh
Cc: akpm, cantabile.desu, kubakici, linux-wireless, keescook, shuah,
mfuzzey, zohar, dhowells, pali.rohar, tiwai, arend.vanspriel,
zajec5, nbroeking, markivx, broonie, dmitry.torokhov, dwmw2,
torvalds, Abhay_Salunke, bjorn.andersson, jewalt, oneukum, ast,
andresx7, linux-kernel, linux-fsdevel, Luis R. Rodriguez
This expands our library with as many things we could find which
both scripts we use share.
Signed-off-by: Luis R. Rodriguez <mcgrof@kernel.org>
---
tools/testing/selftests/firmware/fw_fallback.sh | 31 +++-----------
tools/testing/selftests/firmware/fw_filesystem.sh | 41 +++---------------
tools/testing/selftests/firmware/fw_lib.sh | 52 +++++++++++++++++++++++
3 files changed, 63 insertions(+), 61 deletions(-)
diff --git a/tools/testing/selftests/firmware/fw_fallback.sh b/tools/testing/selftests/firmware/fw_fallback.sh
index 323c921a2469..9337a0328627 100755
--- a/tools/testing/selftests/firmware/fw_fallback.sh
+++ b/tools/testing/selftests/firmware/fw_fallback.sh
@@ -6,30 +6,17 @@
# won't find so that we can do the load ourself manually.
set -e
+TEST_REQS_FW_SYSFS_FALLBACK="yes"
+TEST_REQS_FW_SET_CUSTOM_PATH="no"
TEST_DIR=$(dirname $0)
source $TEST_DIR/fw_lib.sh
check_mods
+check_setup
+verify_reqs
+setup_tmp_file
-HAS_FW_LOADER_USER_HELPER=$(kconfig_has CONFIG_FW_LOADER_USER_HELPER=y)
-HAS_FW_LOADER_USER_HELPER_FALLBACK=$(kconfig_has CONFIG_FW_LOADER_USER_HELPER_FALLBACK=y)
-
-if [ "$HAS_FW_LOADER_USER_HELPER" = "yes" ]; then
- OLD_TIMEOUT=$(cat /sys/class/firmware/timeout)
-else
- echo "usermode helper disabled so ignoring test"
- exit 0
-fi
-
-FWPATH=$(mktemp -d)
-FW="$FWPATH/test-firmware.bin"
-
-test_finish()
-{
- echo "$OLD_TIMEOUT" >/sys/class/firmware/timeout
- rm -f "$FW"
- rmdir "$FWPATH"
-}
+trap "test_finish" EXIT
load_fw()
{
@@ -168,12 +155,6 @@ load_fw_fallback_with_child()
return $RET
}
-trap "test_finish" EXIT
-
-# This is an unlikely real-world firmware content. :)
-echo "ABCD0123" >"$FW"
-NAME=$(basename "$FW")
-
test_syfs_timeout()
{
DEVPATH="$DIR"/"nope-$NAME"/loading
diff --git a/tools/testing/selftests/firmware/fw_filesystem.sh b/tools/testing/selftests/firmware/fw_filesystem.sh
index 748f2f5737e9..7f47877fa7fa 100755
--- a/tools/testing/selftests/firmware/fw_filesystem.sh
+++ b/tools/testing/selftests/firmware/fw_filesystem.sh
@@ -6,38 +6,15 @@
# know so we can be sure we're not accidentally testing the user helper.
set -e
+TEST_REQS_FW_SYSFS_FALLBACK="no"
+TEST_REQS_FW_SET_CUSTOM_PATH="yes"
TEST_DIR=$(dirname $0)
source $TEST_DIR/fw_lib.sh
check_mods
-
-# CONFIG_FW_LOADER_USER_HELPER has a sysfs class under /sys/class/firmware/
-# These days most distros enable CONFIG_FW_LOADER_USER_HELPER but disable
-# CONFIG_FW_LOADER_USER_HELPER_FALLBACK. We use /sys/class/firmware/ as an
-# indicator for CONFIG_FW_LOADER_USER_HELPER.
-HAS_FW_LOADER_USER_HELPER=$(if [ -d /sys/class/firmware/ ]; then echo yes; else echo no; fi)
-
-if [ "$HAS_FW_LOADER_USER_HELPER" = "yes" ]; then
- OLD_TIMEOUT=$(cat /sys/class/firmware/timeout)
-fi
-
-OLD_FWPATH=$(cat /sys/module/firmware_class/parameters/path)
-
-FWPATH=$(mktemp -d)
-FW="$FWPATH/test-firmware.bin"
-
-test_finish()
-{
- if [ "$HAS_FW_LOADER_USER_HELPER" = "yes" ]; then
- echo "$OLD_TIMEOUT" >/sys/class/firmware/timeout
- fi
- if [ "$OLD_FWPATH" = "" ]; then
- OLD_FWPATH=" "
- fi
- echo -n "$OLD_FWPATH" >/sys/module/firmware_class/parameters/path
- rm -f "$FW"
- rmdir "$FWPATH"
-}
+check_setup
+verify_reqs
+setup_tmp_file
trap "test_finish" EXIT
@@ -46,14 +23,6 @@ if [ "$HAS_FW_LOADER_USER_HELPER" = "yes" ]; then
echo 1 >/sys/class/firmware/timeout
fi
-# Set the kernel search path.
-echo -n "$FWPATH" >/sys/module/firmware_class/parameters/path
-
-# This is an unlikely real-world firmware content. :)
-echo "ABCD0123" >"$FW"
-
-NAME=$(basename "$FW")
-
if printf '\000' >"$DIR"/trigger_request 2> /dev/null; then
echo "$0: empty filename should not succeed" >&2
exit 1
diff --git a/tools/testing/selftests/firmware/fw_lib.sh b/tools/testing/selftests/firmware/fw_lib.sh
index 467567c758b9..e3cc4483fdba 100755
--- a/tools/testing/selftests/firmware/fw_lib.sh
+++ b/tools/testing/selftests/firmware/fw_lib.sh
@@ -43,6 +43,58 @@ check_mods()
fi
}
+check_setup()
+{
+ HAS_FW_LOADER_USER_HELPER="$(kconfig_has CONFIG_FW_LOADER_USER_HELPER=y)"
+ HAS_FW_LOADER_USER_HELPER_FALLBACK="$(kconfig_has CONFIG_FW_LOADER_USER_HELPER_FALLBACK=y)"
+
+ if [ "$HAS_FW_LOADER_USER_HELPER" = "yes" ]; then
+ OLD_TIMEOUT="$(cat /sys/class/firmware/timeout)"
+ fi
+
+ OLD_FWPATH="$(cat /sys/module/firmware_class/parameters/path)"
+}
+
+verify_reqs()
+{
+ if [ "$TEST_REQS_FW_SYSFS_FALLBACK" = "yes" ]; then
+ if [ ! "$HAS_FW_LOADER_USER_HELPER" = "yes" ]; then
+ echo "usermode helper disabled so ignoring test"
+ exit 0
+ fi
+ fi
+}
+
+setup_tmp_file()
+{
+ FWPATH=$(mktemp -d)
+ FW="$FWPATH/test-firmware.bin"
+ echo "ABCD0123" >"$FW"
+ NAME=$(basename "$FW")
+ if [ "$TEST_REQS_FW_SET_CUSTOM_PATH" = "yes" ]; then
+ echo -n "$FWPATH" >/sys/module/firmware_class/parameters/path
+ fi
+}
+
+test_finish()
+{
+ if [ "$HAS_FW_LOADER_USER_HELPER" = "yes" ]; then
+ echo "$OLD_TIMEOUT" >/sys/class/firmware/timeout
+ fi
+ if [ "$OLD_FWPATH" = "" ]; then
+ OLD_FWPATH=" "
+ fi
+ if [ "$TEST_REQS_FW_SET_CUSTOM_PATH" = "yes" ]; then
+ echo -n "$OLD_FWPATH" >/sys/module/firmware_class/parameters/path
+ fi
+ if [ -f $FW ]; then
+ rm -f "$FW"
+ fi
+ if [ -d $FWPATH ]; then
+ rm -rf "$FWPATH"
+ fi
+}
+
kconfig_has()
{
if [ -f $PROC_CONFIG ]; then
--
2.16.2
^ permalink raw reply related [flat|nested] 39+ messages in thread* [PATCH v3 13/20] test_firmware: test three firmware kernel configs using a proc knob
2018-03-10 14:14 [PATCH v3 00/20] firmware: development for v4.17 Luis R. Rodriguez
` (11 preceding siblings ...)
2018-03-10 14:14 ` [PATCH v3 12/20] test_firmware: expand on library with shared helpers Luis R. Rodriguez
@ 2018-03-10 14:14 ` Luis R. Rodriguez
2018-03-10 14:14 ` [PATCH v3 14/20] rename: _request_firmware_load() fw_load_sysfs_fallback() Luis R. Rodriguez
` (7 subsequent siblings)
20 siblings, 0 replies; 39+ messages in thread
From: Luis R. Rodriguez @ 2018-03-10 14:14 UTC (permalink / raw)
To: gregkh
Cc: akpm, cantabile.desu, kubakici, linux-wireless, keescook, shuah,
mfuzzey, zohar, dhowells, pali.rohar, tiwai, arend.vanspriel,
zajec5, nbroeking, markivx, broonie, dmitry.torokhov, dwmw2,
torvalds, Abhay_Salunke, bjorn.andersson, jewalt, oneukum, ast,
andresx7, linux-kernel, linux-fsdevel, Luis R. Rodriguez
Since we now have knobs to twiddle what used to be set on kernel
configurations we can build one base kernel configuration and modify
behaviour to mimic such kernel configurations to test them.
Provided you build a kernel with:
CONFIG_TEST_FIRMWARE=y
CONFIG_FW_LOADER=y
CONFIG_FW_LOADER_USER_HELPER=y
CONFIG_IKCONFIG=y
CONFIG_IKCONFIG_PROC=y
We should now be able test all possible kernel configurations
when FW_LOADER=y. Note that when FW_LOADER=m we just don't provide
the built-in functionality of the built-in firmware.
If you're on an old kernel and either don't have /proc/config.gz
(CONFIG_IKCONFIG_PROC) or haven't enabled CONFIG_FW_LOADER_USER_HELPER
we cannot run these dynamic tests, so just run both scripts just
as we used to before making blunt assumptions about your setup
and requirements exactly as we did before.
Acked-by: Kees Cook <keescook@chromium.org>
Signed-off-by: Luis R. Rodriguez <mcgrof@kernel.org>
---
tools/testing/selftests/firmware/Makefile | 2 +-
tools/testing/selftests/firmware/fw_lib.sh | 51 +++++++++++++++++
tools/testing/selftests/firmware/fw_run_tests.sh | 70 ++++++++++++++++++++++++
3 files changed, 122 insertions(+), 1 deletion(-)
create mode 100755 tools/testing/selftests/firmware/fw_run_tests.sh
diff --git a/tools/testing/selftests/firmware/Makefile b/tools/testing/selftests/firmware/Makefile
index 1894d625af2d..826f38d5dd19 100644
--- a/tools/testing/selftests/firmware/Makefile
+++ b/tools/testing/selftests/firmware/Makefile
@@ -3,7 +3,7 @@
# No binaries, but make sure arg-less "make" doesn't trigger "run_tests"
all:
-TEST_PROGS := fw_filesystem.sh fw_fallback.sh
+TEST_PROGS := fw_run_tests.sh
include ../lib.mk
diff --git a/tools/testing/selftests/firmware/fw_lib.sh b/tools/testing/selftests/firmware/fw_lib.sh
index e3cc4483fdba..98dceb847ba0 100755
--- a/tools/testing/selftests/firmware/fw_lib.sh
+++ b/tools/testing/selftests/firmware/fw_lib.sh
@@ -47,6 +47,34 @@ check_setup()
{
HAS_FW_LOADER_USER_HELPER="$(kconfig_has CONFIG_FW_LOADER_USER_HELPER=y)"
HAS_FW_LOADER_USER_HELPER_FALLBACK="$(kconfig_has CONFIG_FW_LOADER_USER_HELPER_FALLBACK=y)"
+ PROC_FW_IGNORE_SYSFS_FALLBACK="0"
+ PROC_FW_FORCE_SYSFS_FALLBACK="0"
+
+ if [ -z $PROC_SYS_DIR ]; then
+ PROC_SYS_DIR="/proc/sys/kernel"
+ fi
+
+ FW_PROC="${PROC_SYS_DIR}/firmware_config"
+ FW_FORCE_SYSFS_FALLBACK="$FW_PROC/force_sysfs_fallback"
+ FW_IGNORE_SYSFS_FALLBACK="$FW_PROC/ignore_sysfs_fallback"
+
+ if [ -f $FW_FORCE_SYSFS_FALLBACK ]; then
+ PROC_FW_FORCE_SYSFS_FALLBACK="$(cat $FW_FORCE_SYSFS_FALLBACK)"
+ fi
+
+ if [ -f $FW_IGNORE_SYSFS_FALLBACK ]; then
+ PROC_FW_IGNORE_SYSFS_FALLBACK="$(cat $FW_IGNORE_SYSFS_FALLBACK)"
+ fi
+
+ if [ "$PROC_FW_FORCE_SYSFS_FALLBACK" = "1" ]; then
+ HAS_FW_LOADER_USER_HELPER="yes"
+ HAS_FW_LOADER_USER_HELPER_FALLBACK="yes"
+ fi
+
+ if [ "$PROC_FW_IGNORE_SYSFS_FALLBACK" = "1" ]; then
+ HAS_FW_LOADER_USER_HELPER_FALLBACK="no"
+ HAS_FW_LOADER_USER_HELPER="no"
+ fi
if [ "$HAS_FW_LOADER_USER_HELPER" = "yes" ]; then
OLD_TIMEOUT="$(cat /sys/class/firmware/timeout)"
@@ -76,6 +104,28 @@ setup_tmp_file()
fi
}
+proc_set_force_sysfs_fallback()
+{
+ if [ -f $FW_FORCE_SYSFS_FALLBACK ]; then
+ echo -n $1 > $FW_FORCE_SYSFS_FALLBACK
+ check_setup
+ fi
+}
+
+proc_set_ignore_sysfs_fallback()
+{
+ if [ -f $FW_IGNORE_SYSFS_FALLBACK ]; then
+ echo -n $1 > $FW_IGNORE_SYSFS_FALLBACK
+ check_setup
+ fi
+}
+
+proc_restore_defaults()
+{
+ proc_set_force_sysfs_fallback 0
+ proc_set_ignore_sysfs_fallback 0
+}
+
test_finish()
{
if [ "$HAS_FW_LOADER_USER_HELPER" = "yes" ]; then
@@ -93,6 +143,7 @@ test_finish()
if [ -d $FWPATH ]; then
rm -rf "$FWPATH"
fi
+ proc_restore_defaults
}
kconfig_has()
diff --git a/tools/testing/selftests/firmware/fw_run_tests.sh b/tools/testing/selftests/firmware/fw_run_tests.sh
new file mode 100755
index 000000000000..06d638e9dc62
--- /dev/null
+++ b/tools/testing/selftests/firmware/fw_run_tests.sh
@@ -0,0 +1,70 @@
+#!/bin/bash
+# SPDX-License-Identifier: GPL-2.0
+
+# This runs all known tests across all known possible configurations we could
+# emulate in one run.
+
+set -e
+
+TEST_DIR=$(dirname $0)
+source $TEST_DIR/fw_lib.sh
+
+export HAS_FW_LOADER_USER_HELPER=""
+export HAS_FW_LOADER_USER_HELPER_FALLBACK=""
+
+run_tests()
+{
+ proc_set_force_sysfs_fallback $1
+ proc_set_ignore_sysfs_fallback $2
+ $TEST_DIR/fw_filesystem.sh
+
+ proc_set_force_sysfs_fallback $1
+ proc_set_ignore_sysfs_fallback $2
+ $TEST_DIR/fw_fallback.sh
+}
+
+run_test_config_0001()
+{
+ echo "-----------------------------------------------------"
+ echo "Running kernel configuration test 1 -- rare"
+ echo "Emulates:"
+ echo "CONFIG_FW_LOADER=y"
+ echo "CONFIG_FW_LOADER_USER_HELPER=n"
+ echo "CONFIG_FW_LOADER_USER_HELPER_FALLBACK=n"
+ run_tests 0 1
+}
+
+run_test_config_0002()
+{
+ echo "-----------------------------------------------------"
+ echo "Running kernel configuration test 2 -- distro"
+ echo "Emulates:"
+ echo "CONFIG_FW_LOADER=y"
+ echo "CONFIG_FW_LOADER_USER_HELPER=y"
+ echo "CONFIG_FW_LOADER_USER_HELPER_FALLBACK=n"
+ proc_set_ignore_sysfs_fallback 0
+ run_tests 0 0
+}
+
+run_test_config_0003()
+{
+ echo "-----------------------------------------------------"
+ echo "Running kernel configuration test 3 -- android"
+ echo "Emulates:"
+ echo "CONFIG_FW_LOADER=y"
+ echo "CONFIG_FW_LOADER_USER_HELPER=y"
+ echo "CONFIG_FW_LOADER_USER_HELPER_FALLBACK=y"
+ run_tests 1 0
+}
+
+check_mods
+check_setup
+
+if [ -f $FW_FORCE_SYSFS_FALLBACK ]; then
+ run_test_config_0001
+ run_test_config_0002
+ run_test_config_0003
+else
+ echo "Running basic kernel configuration, working with your config"
+ run_test
+fi
--
2.16.2
^ permalink raw reply related [flat|nested] 39+ messages in thread* [PATCH v3 14/20] rename: _request_firmware_load() fw_load_sysfs_fallback()
2018-03-10 14:14 [PATCH v3 00/20] firmware: development for v4.17 Luis R. Rodriguez
` (12 preceding siblings ...)
2018-03-10 14:14 ` [PATCH v3 13/20] test_firmware: test three firmware kernel configs using a proc knob Luis R. Rodriguez
@ 2018-03-10 14:14 ` Luis R. Rodriguez
2018-03-10 14:14 ` [PATCH v3 15/20] firmware: fix checking for return values for fw_add_devm_name() Luis R. Rodriguez
` (6 subsequent siblings)
20 siblings, 0 replies; 39+ messages in thread
From: Luis R. Rodriguez @ 2018-03-10 14:14 UTC (permalink / raw)
To: gregkh
Cc: akpm, cantabile.desu, kubakici, linux-wireless, keescook, shuah,
mfuzzey, zohar, dhowells, pali.rohar, tiwai, arend.vanspriel,
zajec5, nbroeking, markivx, broonie, dmitry.torokhov, dwmw2,
torvalds, Abhay_Salunke, bjorn.andersson, jewalt, oneukum, ast,
andresx7, linux-kernel, linux-fsdevel, Luis R. Rodriguez
This reflects much clearer what is being done.
While at it, kdoc'ify it.
Signed-off-by: Luis R. Rodriguez <mcgrof@kernel.org>
---
Documentation/driver-api/firmware/fallback-mechanisms.rst | 2 +-
drivers/base/firmware_loader/fallback.c | 13 ++++++++++---
2 files changed, 11 insertions(+), 4 deletions(-)
diff --git a/Documentation/driver-api/firmware/fallback-mechanisms.rst b/Documentation/driver-api/firmware/fallback-mechanisms.rst
index 4055ac76b288..f353783ae0be 100644
--- a/Documentation/driver-api/firmware/fallback-mechanisms.rst
+++ b/Documentation/driver-api/firmware/fallback-mechanisms.rst
@@ -112,7 +112,7 @@ Since a device is created for the sysfs interface to help load firmware as a
fallback mechanism userspace can be informed of the addition of the device by
relying on kobject uevents. The addition of the device into the device
hierarchy means the fallback mechanism for firmware loading has been initiated.
-For details of implementation refer to _request_firmware_load(), in particular
+For details of implementation refer to fw_load_sysfs_fallback(), in particular
on the use of dev_set_uevent_suppress() and kobject_uevent().
The kernel's kobject uevent mechanism is implemented in lib/kobject_uevent.c,
diff --git a/drivers/base/firmware_loader/fallback.c b/drivers/base/firmware_loader/fallback.c
index d6838e7ec00c..0a8ec7fec585 100644
--- a/drivers/base/firmware_loader/fallback.c
+++ b/drivers/base/firmware_loader/fallback.c
@@ -535,8 +535,15 @@ fw_create_instance(struct firmware *firmware, const char *fw_name,
return fw_sysfs;
}
-/* load a firmware via user helper */
-static int _request_firmware_load(struct fw_sysfs *fw_sysfs,
+/**
+ * fw_load_sysfs_fallback - load a firmware via the syfs fallback mechanism
+ * @fw_sysfs: firmware syfs information for the firmware to load
+ * @opt_flags: flags of options, FW_OPT_*
+ * @timeout: timeout to wait for the load
+ *
+ * In charge of constructing a sysfs fallback interface for firmware loading.
+ **/
+static int fw_load_sysfs_fallback(struct fw_sysfs *fw_sysfs,
unsigned int opt_flags, long timeout)
{
int retval = 0;
@@ -621,7 +628,7 @@ static int fw_load_from_user_helper(struct firmware *firmware,
}
fw_sysfs->fw_priv = firmware->priv;
- ret = _request_firmware_load(fw_sysfs, opt_flags, timeout);
+ ret = fw_load_sysfs_fallback(fw_sysfs, opt_flags, timeout);
if (!ret)
ret = assign_fw(firmware, device, opt_flags);
--
2.16.2
^ permalink raw reply related [flat|nested] 39+ messages in thread* [PATCH v3 15/20] firmware: fix checking for return values for fw_add_devm_name()
2018-03-10 14:14 [PATCH v3 00/20] firmware: development for v4.17 Luis R. Rodriguez
` (13 preceding siblings ...)
2018-03-10 14:14 ` [PATCH v3 14/20] rename: _request_firmware_load() fw_load_sysfs_fallback() Luis R. Rodriguez
@ 2018-03-10 14:14 ` Luis R. Rodriguez
2018-03-10 14:14 ` [PATCH v3 16/20] firmware: add helper to check to see if fw cache is setup Luis R. Rodriguez
` (5 subsequent siblings)
20 siblings, 0 replies; 39+ messages in thread
From: Luis R. Rodriguez @ 2018-03-10 14:14 UTC (permalink / raw)
To: gregkh
Cc: akpm, cantabile.desu, kubakici, linux-wireless, keescook, shuah,
mfuzzey, zohar, dhowells, pali.rohar, tiwai, arend.vanspriel,
zajec5, nbroeking, markivx, broonie, dmitry.torokhov, dwmw2,
torvalds, Abhay_Salunke, bjorn.andersson, jewalt, oneukum, ast,
andresx7, linux-kernel, linux-fsdevel, Luis R. Rodriguez
Currently fw_add_devm_name() returns 1 if the firmware cache
was already set. This makes it complicated for us to check for
correctness. It is actually non-fatal if the firmware cache
is already setup, so just return 0, and simplify the checkers.
fw_add_devm_name() adds device's name onto the devres for the
device so that prior to suspend we cache the firmware onto memory,
so that on resume the firmware is reliably available. We never
were checking for success for this call though, meaning in some
really rare cases we my have never setup the firmware cache for
a device, which could in turn make resume fail.
This is all theoretical, no known issues have been reported.
This small issue has been present way since the addition of the
devres firmware cache names on v3.7.
Fixes: f531f05ae9437 ("firmware loader: store firmware name into devres list")
Signed-off-by: Luis R. Rodriguez <mcgrof@kernel.org>
---
drivers/base/firmware_loader/main.c | 12 +++++++++---
1 file changed, 9 insertions(+), 3 deletions(-)
diff --git a/drivers/base/firmware_loader/main.c b/drivers/base/firmware_loader/main.c
index c8966c84bd44..f5046887e362 100644
--- a/drivers/base/firmware_loader/main.c
+++ b/drivers/base/firmware_loader/main.c
@@ -403,7 +403,7 @@ static int fw_add_devm_name(struct device *dev, const char *name)
fwn = fw_find_devm_name(dev, name);
if (fwn)
- return 1;
+ return 0;
fwn = devres_alloc(fw_name_devm_release, sizeof(struct fw_name_devm),
GFP_KERNEL);
@@ -431,6 +431,7 @@ int assign_fw(struct firmware *fw, struct device *device,
unsigned int opt_flags)
{
struct fw_priv *fw_priv = fw->priv;
+ int ret;
mutex_lock(&fw_lock);
if (!fw_priv->size || fw_state_is_aborted(fw_priv)) {
@@ -447,8 +448,13 @@ int assign_fw(struct firmware *fw, struct device *device,
*/
/* don't cache firmware handled without uevent */
if (device && (opt_flags & FW_OPT_UEVENT) &&
- !(opt_flags & FW_OPT_NOCACHE))
- fw_add_devm_name(device, fw_priv->fw_name);
+ !(opt_flags & FW_OPT_NOCACHE)) {
+ ret = fw_add_devm_name(device, fw_priv->fw_name);
+ if (ret) {
+ mutex_unlock(&fw_lock);
+ return ret;
+ }
+ }
/*
* After caching firmware image is started, let it piggyback
--
2.16.2
^ permalink raw reply related [flat|nested] 39+ messages in thread* [PATCH v3 16/20] firmware: add helper to check to see if fw cache is setup
2018-03-10 14:14 [PATCH v3 00/20] firmware: development for v4.17 Luis R. Rodriguez
` (14 preceding siblings ...)
2018-03-10 14:14 ` [PATCH v3 15/20] firmware: fix checking for return values for fw_add_devm_name() Luis R. Rodriguez
@ 2018-03-10 14:14 ` Luis R. Rodriguez
2018-03-10 14:14 ` [PATCH v3 17/20] test_firmware: modify custom fallback tests to use unique files Luis R. Rodriguez
` (4 subsequent siblings)
20 siblings, 0 replies; 39+ messages in thread
From: Luis R. Rodriguez @ 2018-03-10 14:14 UTC (permalink / raw)
To: gregkh
Cc: akpm, cantabile.desu, kubakici, linux-wireless, keescook, shuah,
mfuzzey, zohar, dhowells, pali.rohar, tiwai, arend.vanspriel,
zajec5, nbroeking, markivx, broonie, dmitry.torokhov, dwmw2,
torvalds, Abhay_Salunke, bjorn.andersson, jewalt, oneukum, ast,
andresx7, linux-kernel, linux-fsdevel, Luis R. Rodriguez
Add a helper to check if the firmware cache is already setup for a device.
This will be used later.
Signed-off-by: Luis R. Rodriguez <mcgrof@kernel.org>
---
drivers/base/firmware_loader/main.c | 14 ++++++++++++--
1 file changed, 12 insertions(+), 2 deletions(-)
diff --git a/drivers/base/firmware_loader/main.c b/drivers/base/firmware_loader/main.c
index f5046887e362..b569d8a09392 100644
--- a/drivers/base/firmware_loader/main.c
+++ b/drivers/base/firmware_loader/main.c
@@ -396,13 +396,23 @@ static struct fw_name_devm *fw_find_devm_name(struct device *dev,
return fwn;
}
-/* add firmware name into devres list */
-static int fw_add_devm_name(struct device *dev, const char *name)
+static bool fw_cache_is_setup(struct device *dev, const char *name)
{
struct fw_name_devm *fwn;
fwn = fw_find_devm_name(dev, name);
if (fwn)
+ return true;
+
+ return false;
+}
+
+/* add firmware name into devres list */
+static int fw_add_devm_name(struct device *dev, const char *name)
+{
+ struct fw_name_devm *fwn;
+
+ if (fw_cache_is_setup(dev, name))
return 0;
fwn = devres_alloc(fw_name_devm_release, sizeof(struct fw_name_devm),
--
2.16.2
^ permalink raw reply related [flat|nested] 39+ messages in thread* [PATCH v3 17/20] test_firmware: modify custom fallback tests to use unique files
2018-03-10 14:14 [PATCH v3 00/20] firmware: development for v4.17 Luis R. Rodriguez
` (15 preceding siblings ...)
2018-03-10 14:14 ` [PATCH v3 16/20] firmware: add helper to check to see if fw cache is setup Luis R. Rodriguez
@ 2018-03-10 14:14 ` Luis R. Rodriguez
2018-03-10 14:14 ` [PATCH v3 18/20] firmware: ensure the firmware cache is not used on incompatible calls Luis R. Rodriguez
` (3 subsequent siblings)
20 siblings, 0 replies; 39+ messages in thread
From: Luis R. Rodriguez @ 2018-03-10 14:14 UTC (permalink / raw)
To: gregkh
Cc: akpm, cantabile.desu, kubakici, linux-wireless, keescook, shuah,
mfuzzey, zohar, dhowells, pali.rohar, tiwai, arend.vanspriel,
zajec5, nbroeking, markivx, broonie, dmitry.torokhov, dwmw2,
torvalds, Abhay_Salunke, bjorn.andersson, jewalt, oneukum, ast,
andresx7, linux-kernel, linux-fsdevel, Luis R. Rodriguez
Users of the custom firmware fallback interface is are not supposed to
use the firmware cache interface, this can happen if for instance the
one of the APIs which use the firmware cache is used first with one
firmware file and then the request_firmware_nowait(uevent=false) API
is used with the same file.
We'll soon become strict about this on the firmware interface to reject
such calls later, so correct the test scripts to avoid such uses as well.
We address this on the tests scripts by simply using unique names when
testing the custom fallback interface.
Signed-off-by: Luis R. Rodriguez <mcgrof@kernel.org>
---
tools/testing/selftests/firmware/fw_fallback.sh | 20 ++++++++++++++------
tools/testing/selftests/firmware/fw_filesystem.sh | 11 +++++++++--
tools/testing/selftests/firmware/fw_lib.sh | 23 +++++++++++++++++++++++
3 files changed, 46 insertions(+), 8 deletions(-)
diff --git a/tools/testing/selftests/firmware/fw_fallback.sh b/tools/testing/selftests/firmware/fw_fallback.sh
index 9337a0328627..8e2e34a2ca69 100755
--- a/tools/testing/selftests/firmware/fw_fallback.sh
+++ b/tools/testing/selftests/firmware/fw_fallback.sh
@@ -238,8 +238,10 @@ run_sysfs_main_tests()
run_sysfs_custom_load_tests()
{
- if load_fw_custom "$NAME" "$FW" ; then
- if ! diff -q "$FW" /dev/test_firmware >/dev/null ; then
+ RANDOM_FILE_PATH=$(setup_random_file)
+ RANDOM_FILE="$(basename $RANDOM_FILE_PATH)"
+ if load_fw_custom "$RANDOM_FILE" "$RANDOM_FILE_PATH" ; then
+ if ! diff -q "$RANDOM_FILE_PATH" /dev/test_firmware >/dev/null ; then
echo "$0: firmware was not loaded" >&2
exit 1
else
@@ -247,8 +249,10 @@ run_sysfs_custom_load_tests()
fi
fi
- if load_fw_custom "$NAME" "$FW" ; then
- if ! diff -q "$FW" /dev/test_firmware >/dev/null ; then
+ RANDOM_FILE_PATH=$(setup_random_file)
+ RANDOM_FILE="$(basename $RANDOM_FILE_PATH)"
+ if load_fw_custom "$RANDOM_FILE" "$RANDOM_FILE_PATH" ; then
+ if ! diff -q "$RANDOM_FILE_PATH" /dev/test_firmware >/dev/null ; then
echo "$0: firmware was not loaded" >&2
exit 1
else
@@ -256,8 +260,12 @@ run_sysfs_custom_load_tests()
fi
fi
- if load_fw_custom_cancel "nope-$NAME" "$FW" ; then
- if diff -q "$FW" /dev/test_firmware >/dev/null ; then
+ RANDOM_FILE_REAL="$RANDOM_FILE_PATH"
+ FAKE_RANDOM_FILE_PATH=$(setup_random_file_fake)
+ FAKE_RANDOM_FILE="$(basename $FAKE_RANDOM_FILE_PATH)"
+
+ if load_fw_custom_cancel "$FAKE_RANDOM_FILE" "$RANDOM_FILE_REAL" ; then
+ if diff -q "$RANDOM_FILE_PATH" /dev/test_firmware >/dev/null ; then
echo "$0: firmware was expected to be cancelled" >&2
exit 1
else
diff --git a/tools/testing/selftests/firmware/fw_filesystem.sh b/tools/testing/selftests/firmware/fw_filesystem.sh
index 7f47877fa7fa..6452d2129cd9 100755
--- a/tools/testing/selftests/firmware/fw_filesystem.sh
+++ b/tools/testing/selftests/firmware/fw_filesystem.sh
@@ -230,10 +230,13 @@ test_wait_and_cancel_custom_load()
test_request_firmware_nowait_custom_nofile()
{
echo -n "Batched request_firmware_nowait(uevent=false) nofile try #$1: "
+ config_reset
config_unset_uevent
- config_set_name nope-test-firmware.bin
+ RANDOM_FILE_PATH=$(setup_random_file_fake)
+ RANDOM_FILE="$(basename $RANDOM_FILE_PATH)"
+ config_set_name $RANDOM_FILE
config_trigger_async &
- test_wait_and_cancel_custom_load nope-test-firmware.bin
+ test_wait_and_cancel_custom_load $RANDOM_FILE
wait
release_all_firmware
echo "OK"
@@ -271,7 +274,11 @@ test_request_firmware_nowait_uevent()
test_request_firmware_nowait_custom()
{
echo -n "Batched request_firmware_nowait(uevent=false) try #$1: "
+ config_reset
config_unset_uevent
+ RANDOM_FILE_PATH=$(setup_random_file)
+ RANDOM_FILE="$(basename $RANDOM_FILE_PATH)"
+ config_set_name $RANDOM_FILE
config_trigger_async
release_all_firmware
echo "OK"
diff --git a/tools/testing/selftests/firmware/fw_lib.sh b/tools/testing/selftests/firmware/fw_lib.sh
index 98dceb847ba0..9ea31b57d71a 100755
--- a/tools/testing/selftests/firmware/fw_lib.sh
+++ b/tools/testing/selftests/firmware/fw_lib.sh
@@ -104,6 +104,29 @@ setup_tmp_file()
fi
}
+__setup_random_file()
+{
+ RANDOM_FILE_PATH="$(mktemp -p $FWPATH)"
+ # mktemp says dry-run -n is unsafe, so...
+ if [[ "$1" = "fake" ]]; then
+ rm -rf $RANDOM_FILE_PATH
+ sync
+ else
+ echo "ABCD0123" >"$RANDOM_FILE_PATH"
+ fi
+ echo $RANDOM_FILE_PATH
+}
+
+setup_random_file()
+{
+ echo $(__setup_random_file)
+}
+
+setup_random_file_fake()
+{
+ echo $(__setup_random_file fake)
+}
+
proc_set_force_sysfs_fallback()
{
if [ -f $FW_FORCE_SYSFS_FALLBACK ]; then
--
2.16.2
^ permalink raw reply related [flat|nested] 39+ messages in thread* [PATCH v3 18/20] firmware: ensure the firmware cache is not used on incompatible calls
2018-03-10 14:14 [PATCH v3 00/20] firmware: development for v4.17 Luis R. Rodriguez
` (16 preceding siblings ...)
2018-03-10 14:14 ` [PATCH v3 17/20] test_firmware: modify custom fallback tests to use unique files Luis R. Rodriguez
@ 2018-03-10 14:14 ` Luis R. Rodriguez
2018-03-10 14:15 ` [PATCH v3 19/20] firmware: add request_firmware_cache() to help with cache on reboot Luis R. Rodriguez
` (2 subsequent siblings)
20 siblings, 0 replies; 39+ messages in thread
From: Luis R. Rodriguez @ 2018-03-10 14:14 UTC (permalink / raw)
To: gregkh
Cc: akpm, cantabile.desu, kubakici, linux-wireless, keescook, shuah,
mfuzzey, zohar, dhowells, pali.rohar, tiwai, arend.vanspriel,
zajec5, nbroeking, markivx, broonie, dmitry.torokhov, dwmw2,
torvalds, Abhay_Salunke, bjorn.andersson, jewalt, oneukum, ast,
andresx7, linux-kernel, linux-fsdevel, Luis R. Rodriguez
request_firmware_into_buf() explicitly disables the firmware cache,
meanwhile the firmware cache cannot be used when request_firmware_nowait()
is used without the uevent. Enforce a sanity check for this to avoid future
issues undocumented behaviours should misuses of the firmware cache
happen later.
One of the reasons we want to enforce this is the firmware cache is
used for helping with suspend/resume, and if incompatible calls use it
they can stall suspend.
Signed-off-by: Luis R. Rodriguez <mcgrof@kernel.org>
---
drivers/base/firmware_loader/main.c | 14 ++++++++++++++
1 file changed, 14 insertions(+)
diff --git a/drivers/base/firmware_loader/main.c b/drivers/base/firmware_loader/main.c
index b569d8a09392..2913bb0e5e7b 100644
--- a/drivers/base/firmware_loader/main.c
+++ b/drivers/base/firmware_loader/main.c
@@ -431,6 +431,11 @@ static int fw_add_devm_name(struct device *dev, const char *name)
return 0;
}
#else
+static bool fw_cache_is_setup(struct device *dev, const char *name)
+{
+ return false;
+}
+
static int fw_add_devm_name(struct device *dev, const char *name)
{
return 0;
@@ -672,6 +677,9 @@ request_firmware_into_buf(const struct firmware **firmware_p, const char *name,
{
int ret;
+ if (fw_cache_is_setup(device, name))
+ return -EOPNOTSUPP;
+
__module_get(THIS_MODULE);
ret = _request_firmware(firmware_p, name, device, buf, size,
FW_OPT_UEVENT | FW_OPT_NOCACHE);
@@ -769,6 +777,12 @@ request_firmware_nowait(
fw_work->opt_flags = FW_OPT_NOWAIT |
(uevent ? FW_OPT_UEVENT : FW_OPT_USERHELPER);
+ if (!uevent && fw_cache_is_setup(device, name)) {
+ kfree_const(fw_work->name);
+ kfree(fw_work);
+ return -EOPNOTSUPP;
+ }
+
if (!try_module_get(module)) {
kfree_const(fw_work->name);
kfree(fw_work);
--
2.16.2
^ permalink raw reply related [flat|nested] 39+ messages in thread* [PATCH v3 19/20] firmware: add request_firmware_cache() to help with cache on reboot
2018-03-10 14:14 [PATCH v3 00/20] firmware: development for v4.17 Luis R. Rodriguez
` (17 preceding siblings ...)
2018-03-10 14:14 ` [PATCH v3 18/20] firmware: ensure the firmware cache is not used on incompatible calls Luis R. Rodriguez
@ 2018-03-10 14:15 ` Luis R. Rodriguez
2018-03-20 8:30 ` Greg KH
2018-03-10 14:15 ` [PATCH v3 20/20] mt7601u: use request_firmware_cache() to address " Luis R. Rodriguez
2018-03-10 17:16 ` [PATCH v3 00/20] firmware: development for v4.17 Kees Cook
20 siblings, 1 reply; 39+ messages in thread
From: Luis R. Rodriguez @ 2018-03-10 14:15 UTC (permalink / raw)
To: gregkh
Cc: akpm, cantabile.desu, kubakici, linux-wireless, keescook, shuah,
mfuzzey, zohar, dhowells, pali.rohar, tiwai, arend.vanspriel,
zajec5, nbroeking, markivx, broonie, dmitry.torokhov, dwmw2,
torvalds, Abhay_Salunke, bjorn.andersson, jewalt, oneukum, ast,
andresx7, linux-kernel, linux-fsdevel, Luis R. Rodriguez
Some devices have an optimization in place to enable the firmware to
be retaineed during a system reboot, so after reboot the device can skip
requesting and loading the firmware. This can save up to 1s in load
time. The mt7601u 802.11 device happens to be such a device.
When these devices retain the firmware on a reboot and then suspend
they can miss looking for the firmware on resume. To help with this we
need a way to cache the firmware when such an optimization has taken
place.
Signed-off-by: Luis R. Rodriguez <mcgrof@kernel.org>
---
.../driver-api/firmware/request_firmware.rst | 14 +++++++++++++
drivers/base/firmware_loader/main.c | 24 ++++++++++++++++++++++
include/linux/firmware.h | 3 +++
3 files changed, 41 insertions(+)
diff --git a/Documentation/driver-api/firmware/request_firmware.rst b/Documentation/driver-api/firmware/request_firmware.rst
index cc0aea880824..b554f4338859 100644
--- a/Documentation/driver-api/firmware/request_firmware.rst
+++ b/Documentation/driver-api/firmware/request_firmware.rst
@@ -44,6 +44,20 @@ request_firmware_nowait
.. kernel-doc:: drivers/base/firmware_class.c
:functions: request_firmware_nowait
+Special optimizations on reboot
+===============================
+
+Some devices have an optimization in place to enable the firmware to be
+retained during system reboot. When such optimizations are used the driver
+author must ensure the firmware is still available on resume from suspend,
+this can be done with request_firmware_cache() insted of requesting for the
+firmare to be loaded.
+
+request_firmware_cache()
+-----------------------
+.. kernel-doc:: drivers/base/firmware_class.c
+ :functions: request_firmware_load
+
request firmware API expected driver use
========================================
diff --git a/drivers/base/firmware_loader/main.c b/drivers/base/firmware_loader/main.c
index 2913bb0e5e7b..04bf853f60e9 100644
--- a/drivers/base/firmware_loader/main.c
+++ b/drivers/base/firmware_loader/main.c
@@ -656,6 +656,30 @@ int request_firmware_direct(const struct firmware **firmware_p,
}
EXPORT_SYMBOL_GPL(request_firmware_direct);
+/**
+ * request_firmware_cache: - cache firmware for suspend so resume can use it
+ * @name: name of firmware file
+ * @device: device for which firmware should be cached for
+ *
+ * There are some devices with an optimization that enables the device to not
+ * require loading firmware on system reboot. This optimization may still
+ * require the firmware present on resume from suspend. This routine can be
+ * used to ensure the firmware is present on resume from suspend in these
+ * situations. This helper is not compatible with drivers which use
+ * request_firmware_into_buf() or request_firmware_nowait() with no uevent set.
+ **/
+int request_firmware_cache(struct device *device, const char *name)
+{
+ int ret;
+
+ mutex_lock(&fw_lock);
+ ret = fw_add_devm_name(device, name);
+ mutex_unlock(&fw_lock);
+
+ return ret;
+}
+EXPORT_SYMBOL_GPL(request_firmware_cache);
+
/**
* request_firmware_into_buf - load firmware into a previously allocated buffer
* @firmware_p: pointer to firmware image
diff --git a/include/linux/firmware.h b/include/linux/firmware.h
index d4508080348d..166867910ebc 100644
--- a/include/linux/firmware.h
+++ b/include/linux/firmware.h
@@ -85,4 +85,7 @@ static inline int request_firmware_into_buf(const struct firmware **firmware_p,
}
#endif
+
+int request_firmware_cache(struct device *device, const char *name);
+
#endif
--
2.16.2
^ permalink raw reply related [flat|nested] 39+ messages in thread* Re: [PATCH v3 19/20] firmware: add request_firmware_cache() to help with cache on reboot
2018-03-10 14:15 ` [PATCH v3 19/20] firmware: add request_firmware_cache() to help with cache on reboot Luis R. Rodriguez
@ 2018-03-20 8:30 ` Greg KH
2018-03-20 17:34 ` Luis R. Rodriguez
0 siblings, 1 reply; 39+ messages in thread
From: Greg KH @ 2018-03-20 8:30 UTC (permalink / raw)
To: Luis R. Rodriguez
Cc: akpm, cantabile.desu, kubakici, linux-wireless, keescook, shuah,
mfuzzey, zohar, dhowells, pali.rohar, tiwai, arend.vanspriel,
zajec5, nbroeking, markivx, broonie, dmitry.torokhov, dwmw2,
torvalds, Abhay_Salunke, bjorn.andersson, jewalt, oneukum, ast,
andresx7, linux-kernel, linux-fsdevel
On Sat, Mar 10, 2018 at 06:15:00AM -0800, Luis R. Rodriguez wrote:
> Some devices have an optimization in place to enable the firmware to
> be retaineed during a system reboot, so after reboot the device can skip
> requesting and loading the firmware. This can save up to 1s in load
> time. The mt7601u 802.11 device happens to be such a device.
>
> When these devices retain the firmware on a reboot and then suspend
> they can miss looking for the firmware on resume. To help with this we
> need a way to cache the firmware when such an optimization has taken
> place.
>
> Signed-off-by: Luis R. Rodriguez <mcgrof@kernel.org>
> ---
> .../driver-api/firmware/request_firmware.rst | 14 +++++++++++++
> drivers/base/firmware_loader/main.c | 24 ++++++++++++++++++++++
> include/linux/firmware.h | 3 +++
> 3 files changed, 41 insertions(+)
>
> diff --git a/Documentation/driver-api/firmware/request_firmware.rst b/Documentation/driver-api/firmware/request_firmware.rst
> index cc0aea880824..b554f4338859 100644
> --- a/Documentation/driver-api/firmware/request_firmware.rst
> +++ b/Documentation/driver-api/firmware/request_firmware.rst
> @@ -44,6 +44,20 @@ request_firmware_nowait
> .. kernel-doc:: drivers/base/firmware_class.c
> :functions: request_firmware_nowait
>
> +Special optimizations on reboot
> +===============================
> +
> +Some devices have an optimization in place to enable the firmware to be
> +retained during system reboot. When such optimizations are used the driver
> +author must ensure the firmware is still available on resume from suspend,
> +this can be done with request_firmware_cache() insted of requesting for the
> +firmare to be loaded.
> +
> +request_firmware_cache()
> +-----------------------
> +.. kernel-doc:: drivers/base/firmware_class.c
> + :functions: request_firmware_load
> +
> request firmware API expected driver use
> ========================================
>
> diff --git a/drivers/base/firmware_loader/main.c b/drivers/base/firmware_loader/main.c
> index 2913bb0e5e7b..04bf853f60e9 100644
> --- a/drivers/base/firmware_loader/main.c
> +++ b/drivers/base/firmware_loader/main.c
> @@ -656,6 +656,30 @@ int request_firmware_direct(const struct firmware **firmware_p,
> }
> EXPORT_SYMBOL_GPL(request_firmware_direct);
>
> +/**
> + * request_firmware_cache: - cache firmware for suspend so resume can use it
> + * @name: name of firmware file
> + * @device: device for which firmware should be cached for
> + *
> + * There are some devices with an optimization that enables the device to not
> + * require loading firmware on system reboot. This optimization may still
> + * require the firmware present on resume from suspend. This routine can be
> + * used to ensure the firmware is present on resume from suspend in these
> + * situations. This helper is not compatible with drivers which use
> + * request_firmware_into_buf() or request_firmware_nowait() with no uevent set.
> + **/
> +int request_firmware_cache(struct device *device, const char *name)
> +{
> + int ret;
> +
> + mutex_lock(&fw_lock);
> + ret = fw_add_devm_name(device, name);
> + mutex_unlock(&fw_lock);
> +
> + return ret;
> +}
> +EXPORT_SYMBOL_GPL(request_firmware_cache);
I know you are just following the existing naming scheme, but please
let's not continue the problem here. Can you prefix all of the firmware
exported symbols with "firmware_", and then the verb. So this would be
"firmware_request_cache", and the older function would be
"firmware_request_direct".
I've stopped here in the patch series, applying the others now, so feel
free to rebase and resend what I've missed, and the minor fixups for the
prior patches.
thanks,
greg k-h
^ permalink raw reply [flat|nested] 39+ messages in thread* Re: [PATCH v3 19/20] firmware: add request_firmware_cache() to help with cache on reboot
2018-03-20 8:30 ` Greg KH
@ 2018-03-20 17:34 ` Luis R. Rodriguez
2018-03-20 17:38 ` Greg KH
0 siblings, 1 reply; 39+ messages in thread
From: Luis R. Rodriguez @ 2018-03-20 17:34 UTC (permalink / raw)
To: Greg KH
Cc: Luis R. Rodriguez, akpm, cantabile.desu, kubakici,
linux-wireless, keescook, shuah, mfuzzey, zohar, dhowells,
pali.rohar, tiwai, arend.vanspriel, zajec5, nbroeking, markivx,
broonie, dmitry.torokhov, dwmw2, torvalds, Abhay_Salunke,
bjorn.andersson, jewalt, oneukum, ast, andresx7, linux-kernel,
linux-fsdevel
On Tue, Mar 20, 2018 at 09:30:55AM +0100, Greg KH wrote:
> On Sat, Mar 10, 2018 at 06:15:00AM -0800, Luis R. Rodriguez wrote:
> > +EXPORT_SYMBOL_GPL(request_firmware_cache);
>
> I know you are just following the existing naming scheme, but please
> let's not continue the problem here. Can you prefix all of the firmware
> exported symbols with "firmware_", and then the verb. So this would be
> "firmware_request_cache",
Sure.
> and the older function would be
> "firmware_request_direct".
Sure, so you want to also rename the old exported symbols, and add a macro
or static inline to enable use of the older names?
> I've stopped here in the patch series, applying the others now, so feel
> free to rebase and resend what I've missed, and the minor fixups for the
> prior patches.
Sure thing.
Luis
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH v3 19/20] firmware: add request_firmware_cache() to help with cache on reboot
2018-03-20 17:34 ` Luis R. Rodriguez
@ 2018-03-20 17:38 ` Greg KH
2018-03-20 18:24 ` Luis R. Rodriguez
0 siblings, 1 reply; 39+ messages in thread
From: Greg KH @ 2018-03-20 17:38 UTC (permalink / raw)
To: Luis R. Rodriguez
Cc: akpm, cantabile.desu, kubakici, linux-wireless, keescook, shuah,
mfuzzey, zohar, dhowells, pali.rohar, tiwai, arend.vanspriel,
zajec5, nbroeking, markivx, broonie, dmitry.torokhov, dwmw2,
torvalds, Abhay_Salunke, bjorn.andersson, jewalt, oneukum, ast,
andresx7, linux-kernel, linux-fsdevel
On Tue, Mar 20, 2018 at 05:34:09PM +0000, Luis R. Rodriguez wrote:
> On Tue, Mar 20, 2018 at 09:30:55AM +0100, Greg KH wrote:
> > On Sat, Mar 10, 2018 at 06:15:00AM -0800, Luis R. Rodriguez wrote:
> > > +EXPORT_SYMBOL_GPL(request_firmware_cache);
> >
> > I know you are just following the existing naming scheme, but please
> > let's not continue the problem here. Can you prefix all of the firmware
> > exported symbols with "firmware_", and then the verb. So this would be
> > "firmware_request_cache",
>
> Sure.
>
> > and the older function would be
> > "firmware_request_direct".
>
> Sure, so you want to also rename the old exported symbols, and add a macro
> or static inline to enable use of the older names?
Renaming is best, let's not keep them around for no good reason :)
thanks,
greg k-h
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH v3 19/20] firmware: add request_firmware_cache() to help with cache on reboot
2018-03-20 17:38 ` Greg KH
@ 2018-03-20 18:24 ` Luis R. Rodriguez
2018-03-20 18:54 ` Konstantin Ryabitsev
2018-03-21 10:04 ` Greg KH
0 siblings, 2 replies; 39+ messages in thread
From: Luis R. Rodriguez @ 2018-03-20 18:24 UTC (permalink / raw)
To: Greg KH, Linus Torvalds, Julia Lawall, Nicolas Palix,
Joe Perches, Takashi Iwai, Thomas Gleixner, Jiri Kosina,
Masahiro Yamada
Cc: Luis R. Rodriguez, akpm, cantabile.desu, kubakici,
linux-wireless, keescook, shuah, mfuzzey, zohar, dhowells,
pali.rohar, tiwai, arend.vanspriel, zajec5, nbroeking, markivx,
broonie, dmitry.torokhov, dwmw2, Abhay_Salunke, bjorn.andersson,
jewalt, oneukum, ast, andresx7, linux-kernel, linux-fsdevel,
Konstantin Ryabitsev
On Tue, Mar 20, 2018 at 06:38:01PM +0100, Greg KH wrote:
> On Tue, Mar 20, 2018 at 05:34:09PM +0000, Luis R. Rodriguez wrote:
> > On Tue, Mar 20, 2018 at 09:30:55AM +0100, Greg KH wrote:
> > > On Sat, Mar 10, 2018 at 06:15:00AM -0800, Luis R. Rodriguez wrote:
> > > > +EXPORT_SYMBOL_GPL(request_firmware_cache);
> > >
> > > I know you are just following the existing naming scheme, but please
> > > let's not continue the problem here. Can you prefix all of the firmware
> > > exported symbols with "firmware_", and then the verb. So this would be
> > > "firmware_request_cache",
> >
> > Sure.
> >
> > > and the older function would be
> > > "firmware_request_direct".
> >
> > Sure, so you want to also rename the old exported symbols, and add a macro
> > or static inline to enable use of the older names?
>
> Renaming is best, let's not keep them around for no good reason :)
Sure thing, I'll rename the old firmware calls.
Since that would be a cross-tree collateral evolution, as we have discussed
before over these at kernel summits, I'd prefer to leave that large set of
renames toward the end of the series, as the last patch. This way the
Coccinelle SmPL / sed script could be run at the very end, and in case of merge
conflicts re-run.
Which BTW... *long term*, I believe a possible way to address these cross-tree
collateral evolutions, due to new merge updates and conflicts, could be for us
to embed the SmPL / sed scripts into git notes, and in turn have an *optional* git
configuration which could attempt a merge conflict resolution through
alternative means, one of which -- if enabled -- could be to try to run the SmPL or
sed script in the attached git note (if you downloaded them). One of the other
benefits of this is the commit log is no longer visually impaired by such long
semantic patches or scripts, and gives us a nice way to pull these up through a
generic tooling mechanism. This could all just be optional, no one would have
to push or fetch these -- only if they wanted the possible added benefits of
having them.
*Iff* this seems sensible, this would only mean kernel.org would have to start
accepting git notes long term, for those who optionally want to push them or
fetch them.
FWIW I've confirmed with github that they now accepts git notes, the UI for
github just needs time to develop how to interact with them, for now they just
hide them. One of the difficulties with a UI interface for them is that by
default git notes *do not* use a namespace reflective for a branch or anything.
We could stick to a actual namespace for branch / commit:
"iso" for isomorphism
git note --ref branch/iso-coccinelle/ add commit-id
vi refs/notes/branch/iso-coccinelle/commit-id
git note --ref branch/iso-coccinelle/ edit commit-id
This is rather crude though so simple interface such as the following could
be added as well:
git smpl add sp.cocci commit-id
git smpl edit commit-id
git smpl list v4.13..v4.14
Similar thing could be done for crash dumps, if we wanted...
Since this still would need to be discussed, and if agreed then implemented, in
the meantime I can just do it the old school way of embedding the SmPL and/or
sed script into the commit log, but left as the last patch in the series.
Luis
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH v3 19/20] firmware: add request_firmware_cache() to help with cache on reboot
2018-03-20 18:24 ` Luis R. Rodriguez
@ 2018-03-20 18:54 ` Konstantin Ryabitsev
2018-03-20 19:51 ` Luis R. Rodriguez
2018-03-21 10:04 ` Greg KH
1 sibling, 1 reply; 39+ messages in thread
From: Konstantin Ryabitsev @ 2018-03-20 18:54 UTC (permalink / raw)
To: Luis R. Rodriguez, Greg KH, Linus Torvalds, Julia Lawall,
Nicolas Palix, Joe Perches, Takashi Iwai, Thomas Gleixner,
Jiri Kosina, Masahiro Yamada
Cc: akpm, cantabile.desu, kubakici, linux-wireless, keescook, shuah,
mfuzzey, zohar, dhowells, pali.rohar, arend.vanspriel, zajec5,
nbroeking, markivx, broonie, dmitry.torokhov, dwmw2,
Abhay_Salunke, bjorn.andersson, jewalt, oneukum, ast, andresx7,
linux-kernel, linux-fsdevel
[-- Attachment #1.1: Type: text/plain, Size: 1114 bytes --]
On 03/20/18 14:24, Luis R. Rodriguez wrote:
> *Iff* this seems sensible, this would only mean kernel.org would have to start
> accepting git notes long term, for those who optionally want to push them or
> fetch them.
We don't disallow them. You just need to make sure you're fetching and
pushing them, because they aren't by default. You can set this in your
kernel.org origin section of .git/config:
[remote origin]
...
fetch = +refs/notes/*:refs/notes/*
And then push them separately as "git push origin refs/notes/*".
Since frontends clone with --mirror, the notes will be available on all
git.kernel.org nodes (e.g. see
https://git.kernel.org/mricon/hook-test/c/a8d310d4c13)
If you do start using notes, I strongly suggest you pick a dedicated
refspace for it instead of putting things into the default
refs/notes/commits, e.g.:
git notes --ref crosstree [...]
This will create refs/notes/crosstree that has less of a chance to be
clobbered by someone else's use of notes.
Best,
--
Konstantin Ryabitsev
Director, IT Infrastructure Security
The Linux Foundation
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH v3 19/20] firmware: add request_firmware_cache() to help with cache on reboot
2018-03-20 18:54 ` Konstantin Ryabitsev
@ 2018-03-20 19:51 ` Luis R. Rodriguez
0 siblings, 0 replies; 39+ messages in thread
From: Luis R. Rodriguez @ 2018-03-20 19:51 UTC (permalink / raw)
To: Konstantin Ryabitsev
Cc: Luis R. Rodriguez, Greg KH, Linus Torvalds, Julia Lawall,
Nicolas Palix, Joe Perches, Takashi Iwai, Thomas Gleixner,
Jiri Kosina, Masahiro Yamada, akpm, cantabile.desu, kubakici,
linux-wireless, keescook, shuah, mfuzzey, zohar, dhowells,
pali.rohar, arend.vanspriel, zajec5, nbroeking, markivx, broonie,
dmitry.torokhov, dwmw2, Abhay_Salunke, bjorn.andersson, jewalt,
oneukum, ast, andresx7, linux-kernel, linux-fsdevel
On Tue, Mar 20, 2018 at 02:54:44PM -0400, Konstantin Ryabitsev wrote:
> On 03/20/18 14:24, Luis R. Rodriguez wrote:
> > *Iff* this seems sensible, this would only mean kernel.org would have to start
> > accepting git notes long term, for those who optionally want to push them or
> > fetch them.
>
> We don't disallow them. You just need to make sure you're fetching and
> pushing them, because they aren't by default. You can set this in your
> kernel.org origin section of .git/config:
>
> [remote origin]
> ...
> fetch = +refs/notes/*:refs/notes/*
>
> And then push them separately as "git push origin refs/notes/*".
Superb, thanks!
> Since frontends clone with --mirror, the notes will be available on all
> git.kernel.org nodes (e.g. see
> https://git.kernel.org/mricon/hook-test/c/a8d310d4c13)
Good to know thanks, and it looks good!
> If you do start using notes, I strongly suggest you pick a dedicated
> refspace for it instead of putting things into the default
> refs/notes/commits, e.g.:
>
> git notes --ref crosstree [...]
>
> This will create refs/notes/crosstree that has less of a chance to be
> clobbered by someone else's use of notes.
Indeed, the default is crap. I was suggesting perhaps having it also be
per branch, so:
git note --ref branch/iso-coccinelle/ add commit-id
If it was required sed I guess
git note --ref branch/sed/ add commit-id
And if we wanted a resolution handler which could manage the above for you:
git note --ref branch/merger-script add commit-id
Which could use the above two to run what is needed to mimic the commit.
Luis
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH v3 19/20] firmware: add request_firmware_cache() to help with cache on reboot
2018-03-20 18:24 ` Luis R. Rodriguez
2018-03-20 18:54 ` Konstantin Ryabitsev
@ 2018-03-21 10:04 ` Greg KH
1 sibling, 0 replies; 39+ messages in thread
From: Greg KH @ 2018-03-21 10:04 UTC (permalink / raw)
To: Luis R. Rodriguez
Cc: Linus Torvalds, Julia Lawall, Nicolas Palix, Joe Perches,
Takashi Iwai, Thomas Gleixner, Jiri Kosina, Masahiro Yamada,
akpm, cantabile.desu, kubakici, linux-wireless, keescook, shuah,
mfuzzey, zohar, dhowells, pali.rohar, arend.vanspriel, zajec5,
nbroeking, markivx, broonie, dmitry.torokhov, dwmw2,
Abhay_Salunke, bjorn.andersson, jewalt, oneukum, ast, andresx7,
linux-kernel, linux-fsdevel, Konstantin Ryabitsev
On Tue, Mar 20, 2018 at 06:24:09PM +0000, Luis R. Rodriguez wrote:
> On Tue, Mar 20, 2018 at 06:38:01PM +0100, Greg KH wrote:
> > On Tue, Mar 20, 2018 at 05:34:09PM +0000, Luis R. Rodriguez wrote:
> > > On Tue, Mar 20, 2018 at 09:30:55AM +0100, Greg KH wrote:
> > > > On Sat, Mar 10, 2018 at 06:15:00AM -0800, Luis R. Rodriguez wrote:
> > > > > +EXPORT_SYMBOL_GPL(request_firmware_cache);
> > > >
> > > > I know you are just following the existing naming scheme, but please
> > > > let's not continue the problem here. Can you prefix all of the firmware
> > > > exported symbols with "firmware_", and then the verb. So this would be
> > > > "firmware_request_cache",
> > >
> > > Sure.
> > >
> > > > and the older function would be
> > > > "firmware_request_direct".
> > >
> > > Sure, so you want to also rename the old exported symbols, and add a macro
> > > or static inline to enable use of the older names?
> >
> > Renaming is best, let's not keep them around for no good reason :)
>
> Sure thing, I'll rename the old firmware calls.
Ah, in looking at this closer, that might take a bit of time, as there
are a _lot_ of callers of request_firmware(). So maybe a macro/wrapper
is good for that one so we can propagate the changes through the
different subsystems and do this over a few kernel releases.
But for anything new, let's get it right the first time :)
thanks,
greg k-h
^ permalink raw reply [flat|nested] 39+ messages in thread
* [PATCH v3 20/20] mt7601u: use request_firmware_cache() to address cache on reboot
2018-03-10 14:14 [PATCH v3 00/20] firmware: development for v4.17 Luis R. Rodriguez
` (18 preceding siblings ...)
2018-03-10 14:15 ` [PATCH v3 19/20] firmware: add request_firmware_cache() to help with cache on reboot Luis R. Rodriguez
@ 2018-03-10 14:15 ` Luis R. Rodriguez
2018-03-13 4:43 ` Jakub Kicinski
2018-03-10 17:16 ` [PATCH v3 00/20] firmware: development for v4.17 Kees Cook
20 siblings, 1 reply; 39+ messages in thread
From: Luis R. Rodriguez @ 2018-03-10 14:15 UTC (permalink / raw)
To: gregkh
Cc: akpm, cantabile.desu, kubakici, linux-wireless, keescook, shuah,
mfuzzey, zohar, dhowells, pali.rohar, tiwai, arend.vanspriel,
zajec5, nbroeking, markivx, broonie, dmitry.torokhov, dwmw2,
torvalds, Abhay_Salunke, bjorn.andersson, jewalt, oneukum, ast,
andresx7, linux-kernel, linux-fsdevel, Luis R. Rodriguez
request_firmware_cache() will ensure the firmware is available on resume
from suspend if on reboot the device retains the firmware.
This optimization is in place given otherwise on reboot we have to
reload the firmware, the opmization saves us about max 1s, minimum 10ms.
Cantabile has reported back this fixes his woes with both suspend and
hibernation.
Reported-by: Cantabile <cantabile.desu@gmail.com>
Tested-by: Cantabile <cantabile.desu@gmail.com>
Signed-off-by: Luis R. Rodriguez <mcgrof@kernel.org>
---
drivers/net/wireless/mediatek/mt7601u/mcu.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/wireless/mediatek/mt7601u/mcu.c b/drivers/net/wireless/mediatek/mt7601u/mcu.c
index 65a8004418ea..b90456a4b4d7 100644
--- a/drivers/net/wireless/mediatek/mt7601u/mcu.c
+++ b/drivers/net/wireless/mediatek/mt7601u/mcu.c
@@ -421,7 +421,7 @@ static int mt7601u_load_firmware(struct mt7601u_dev *dev)
MT_USB_DMA_CFG_TX_BULK_EN));
if (firmware_running(dev))
- return 0;
+ return request_firmware_cache(dev->dev, MT7601U_FIRMWARE);
ret = request_firmware(&fw, MT7601U_FIRMWARE, dev->dev);
if (ret)
--
2.16.2
^ permalink raw reply related [flat|nested] 39+ messages in thread* Re: [PATCH v3 20/20] mt7601u: use request_firmware_cache() to address cache on reboot
2018-03-10 14:15 ` [PATCH v3 20/20] mt7601u: use request_firmware_cache() to address " Luis R. Rodriguez
@ 2018-03-13 4:43 ` Jakub Kicinski
0 siblings, 0 replies; 39+ messages in thread
From: Jakub Kicinski @ 2018-03-13 4:43 UTC (permalink / raw)
To: Luis R. Rodriguez
Cc: gregkh, akpm, cantabile.desu, linux-wireless, keescook, shuah,
mfuzzey, zohar, dhowells, pali.rohar, tiwai, arend.vanspriel,
zajec5, nbroeking, markivx, broonie, dmitry.torokhov, dwmw2,
torvalds, Abhay_Salunke, bjorn.andersson, jewalt, oneukum, ast,
andresx7, linux-kernel, linux-fsdevel
On Sat, 10 Mar 2018 06:15:01 -0800, Luis R. Rodriguez wrote:
> request_firmware_cache() will ensure the firmware is available on resume
> from suspend if on reboot the device retains the firmware.
>
> This optimization is in place given otherwise on reboot we have to
> reload the firmware, the opmization saves us about max 1s, minimum 10ms.
>
> Cantabile has reported back this fixes his woes with both suspend and
> hibernation.
>
> Reported-by: Cantabile <cantabile.desu@gmail.com>
> Tested-by: Cantabile <cantabile.desu@gmail.com>
> Signed-off-by: Luis R. Rodriguez <mcgrof@kernel.org>
FWIW:
Acked-by: Jakub Kicinski <kubakici@wp.pl>
Thanks Luis!!
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH v3 00/20] firmware: development for v4.17
2018-03-10 14:14 [PATCH v3 00/20] firmware: development for v4.17 Luis R. Rodriguez
` (19 preceding siblings ...)
2018-03-10 14:15 ` [PATCH v3 20/20] mt7601u: use request_firmware_cache() to address " Luis R. Rodriguez
@ 2018-03-10 17:16 ` Kees Cook
2018-03-14 17:44 ` Luis R. Rodriguez
20 siblings, 1 reply; 39+ messages in thread
From: Kees Cook @ 2018-03-10 17:16 UTC (permalink / raw)
To: Luis R. Rodriguez
Cc: Greg KH, Andrew Morton, Bitterblue of Monsea, Jakub Kicinski,
linux-wireless, Shuah Khan, Martin Fuzzey, Mimi Zohar,
David Howells, pali.rohar, Takashi Iwai, arend.vanspriel,
Rafał Miłecki, Nicolas Broeking, Vikram Mulukutla,
Mark Brown, Dmitry Torokhov, David Woodhouse, Linus Torvalds,
Abhay_Salunke, bjorn.andersson, jewalt, oneukum, ast, andresx7,
LKML, linux-fsdevel
On Sat, Mar 10, 2018 at 6:14 AM, Luis R. Rodriguez <mcgrof@kernel.org> wrote:
> Greg,
>
> Here's a respin of what I have queued up for v4.17 for the firmware API. It
> combines the cleanup I've been working on and the addition of the new API call
> request_firmware_cache() for fixing a corner case suspend issue on some type of
> cards with an optimization in place where the firmware is *not* needed on
> reboot.
>
> The cleanup work allows us to test the firmware API with one kernel
> configuration. I've addressed Kees' feedback on this respin and
> combined the code into drivers/base/firmware_class/.
>
> I've made one new test_firmware change in consideration for one firmware
> change, the patch "firmware: ensure the firmware cache is not used on
> incompatible calls" requires us to modify our tests scripts to use
> the APIs sanely as well.
>
> I've put up these changes on my git tree, refer to the branch
> "20180307-firmware-dev-for-v4.17" based on linux-next [0] and
> the same name based on Linus' tree [1].
>
> Questions, feedback, and specially rants are always welcomed.
This all looks good to me! Thanks for respinning. :)
-Kees
>
> [0] https://git.kernel.org/pub/scm/linux/kernel/git/mcgrof/linux-next.git/log/?h=20180307-firmware-dev-for-v4.17
> [1] https://git.kernel.org/pub/scm/linux/kernel/git/mcgrof/linux.git/log/?h=20180307-firmware-dev-for-v4.17
>
> Luis R. Rodriguez (20):
> test_firmware: add simple firmware firmware test library
> test_firmware: enable custom fallback testing on limited kernel
> configs
> test_firmware: replace syfs fallback check with kconfig_has helper
> firmware: enable to split firmware_class into separate target files
> firmware: simplify CONFIG_FW_LOADER_USER_HELPER_FALLBACK further
> firmware: use helpers for setting up a temporary cache timeout
> firmware: move loading timeout under struct firmware_fallback_config
> firmware: split firmware fallback functionality into its own file
> firmware: move firmware loader into its own directory
> firmware: enable run time change of forcing fallback loader
> firmware: enable to force disable the fallback mechanism at run time
> test_firmware: expand on library with shared helpers
> test_firmware: test three firmware kernel configs using a proc knob
> rename: _request_firmware_load() fw_load_sysfs_fallback()
> firmware: fix checking for return values for fw_add_devm_name()
> firmware: add helper to check to see if fw cache is setup
> test_firmware: modify custom fallback tests to use unique files
> firmware: ensure the firmware cache is not used on incompatible calls
> firmware: add request_firmware_cache() to help with cache on reboot
> mt7601u: use request_firmware_cache() to address cache on reboot
>
> .../driver-api/firmware/fallback-mechanisms.rst | 2 +-
> .../driver-api/firmware/request_firmware.rst | 14 +
> MAINTAINERS | 2 +-
> drivers/base/Makefile | 2 +-
> drivers/base/firmware_loader/Makefile | 7 +
> drivers/base/firmware_loader/fallback.c | 674 +++++++++++++++++
> drivers/base/firmware_loader/fallback.h | 67 ++
> drivers/base/firmware_loader/fallback_table.c | 55 ++
> drivers/base/firmware_loader/firmware.h | 115 +++
> .../{firmware_class.c => firmware_loader/main.c} | 833 ++-------------------
> drivers/net/wireless/mediatek/mt7601u/mcu.c | 2 +-
> include/linux/firmware.h | 3 +
> kernel/sysctl.c | 11 +
> tools/testing/selftests/firmware/Makefile | 2 +-
> tools/testing/selftests/firmware/config | 4 +
> tools/testing/selftests/firmware/fw_fallback.sh | 65 +-
> tools/testing/selftests/firmware/fw_filesystem.sh | 72 +-
> tools/testing/selftests/firmware/fw_lib.sh | 194 +++++
> tools/testing/selftests/firmware/fw_run_tests.sh | 70 ++
> 19 files changed, 1332 insertions(+), 862 deletions(-)
> create mode 100644 drivers/base/firmware_loader/Makefile
> create mode 100644 drivers/base/firmware_loader/fallback.c
> create mode 100644 drivers/base/firmware_loader/fallback.h
> create mode 100644 drivers/base/firmware_loader/fallback_table.c
> create mode 100644 drivers/base/firmware_loader/firmware.h
> rename drivers/base/{firmware_class.c => firmware_loader/main.c} (60%)
> create mode 100755 tools/testing/selftests/firmware/fw_lib.sh
> create mode 100755 tools/testing/selftests/firmware/fw_run_tests.sh
>
> --
> 2.16.2
>
--
Kees Cook
Pixel Security
^ permalink raw reply [flat|nested] 39+ messages in thread* Re: [PATCH v3 00/20] firmware: development for v4.17
2018-03-10 17:16 ` [PATCH v3 00/20] firmware: development for v4.17 Kees Cook
@ 2018-03-14 17:44 ` Luis R. Rodriguez
2018-03-14 17:58 ` Greg KH
0 siblings, 1 reply; 39+ messages in thread
From: Luis R. Rodriguez @ 2018-03-14 17:44 UTC (permalink / raw)
To: Greg KH, Andrew Morton
Cc: Luis R. Rodriguez, Kees Cook, Bitterblue of Monsea,
Jakub Kicinski, linux-wireless, Shuah Khan, Martin Fuzzey,
Mimi Zohar, David Howells, pali.rohar, Takashi Iwai,
arend.vanspriel, Rafał Miłecki, Nicolas Broeking,
Vikram Mulukutla, Mark Brown, Dmitry Torokhov, David Woodhouse,
Linus Torvalds, Abhay_Salunke, bjorn.andersson, jewalt, oneukum,
ast, andresx7, LKML, linux-fsdevel
On Sat, Mar 10, 2018 at 09:16:36AM -0800, Kees Cook wrote:
> On Sat, Mar 10, 2018 at 6:14 AM, Luis R. Rodriguez <mcgrof@kernel.org> wrote:
> > Greg,
> >
> > Here's a respin of what I have queued up for v4.17 for the firmware API. It
> > combines the cleanup I've been working on and the addition of the new API call
> > request_firmware_cache() for fixing a corner case suspend issue on some type of
> > cards with an optimization in place where the firmware is *not* needed on
> > reboot.
> >
> > The cleanup work allows us to test the firmware API with one kernel
> > configuration. I've addressed Kees' feedback on this respin and
> > combined the code into drivers/base/firmware_class/.
> >
> > I've made one new test_firmware change in consideration for one firmware
> > change, the patch "firmware: ensure the firmware cache is not used on
> > incompatible calls" requires us to modify our tests scripts to use
> > the APIs sanely as well.
> >
> > I've put up these changes on my git tree, refer to the branch
> > "20180307-firmware-dev-for-v4.17" based on linux-next [0] and
> > the same name based on Linus' tree [1].
> >
> > Questions, feedback, and specially rants are always welcomed.
>
> This all looks good to me! Thanks for respinning. :)
Greg,
I'd like to get these baked a bit through linux-next. Let me know if its
easier for you if I go through Andrew for the firmware API changes.
Luis
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH v3 00/20] firmware: development for v4.17
2018-03-14 17:44 ` Luis R. Rodriguez
@ 2018-03-14 17:58 ` Greg KH
0 siblings, 0 replies; 39+ messages in thread
From: Greg KH @ 2018-03-14 17:58 UTC (permalink / raw)
To: Luis R. Rodriguez
Cc: Andrew Morton, Kees Cook, Bitterblue of Monsea, Jakub Kicinski,
linux-wireless, Shuah Khan, Martin Fuzzey, Mimi Zohar,
David Howells, pali.rohar, Takashi Iwai, arend.vanspriel,
Rafał Miłecki, Nicolas Broeking, Vikram Mulukutla,
Mark Brown, Dmitry Torokhov, David Woodhouse, Linus Torvalds,
Abhay_Salunke, bjorn.andersson, jewalt, oneukum, ast, andresx7,
LKML, linux-fsdevel
On Wed, Mar 14, 2018 at 05:44:03PM +0000, Luis R. Rodriguez wrote:
> On Sat, Mar 10, 2018 at 09:16:36AM -0800, Kees Cook wrote:
> > On Sat, Mar 10, 2018 at 6:14 AM, Luis R. Rodriguez <mcgrof@kernel.org> wrote:
> > > Greg,
> > >
> > > Here's a respin of what I have queued up for v4.17 for the firmware API. It
> > > combines the cleanup I've been working on and the addition of the new API call
> > > request_firmware_cache() for fixing a corner case suspend issue on some type of
> > > cards with an optimization in place where the firmware is *not* needed on
> > > reboot.
> > >
> > > The cleanup work allows us to test the firmware API with one kernel
> > > configuration. I've addressed Kees' feedback on this respin and
> > > combined the code into drivers/base/firmware_class/.
> > >
> > > I've made one new test_firmware change in consideration for one firmware
> > > change, the patch "firmware: ensure the firmware cache is not used on
> > > incompatible calls" requires us to modify our tests scripts to use
> > > the APIs sanely as well.
> > >
> > > I've put up these changes on my git tree, refer to the branch
> > > "20180307-firmware-dev-for-v4.17" based on linux-next [0] and
> > > the same name based on Linus' tree [1].
> > >
> > > Questions, feedback, and specially rants are always welcomed.
> >
> > This all looks good to me! Thanks for respinning. :)
>
> Greg,
>
> I'd like to get these baked a bit through linux-next. Let me know if its
> easier for you if I go through Andrew for the firmware API changes.
I'm just about to queue up all of the firmware patches right now, so no
need to bother Andrew :)
thanks,
greg k-h
^ permalink raw reply [flat|nested] 39+ messages in thread