| Message ID | 20171120182409.27348-17-mcgrof@kernel.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Mon, Nov 20, 2017 at 10:24:02AM -0800, Luis R. Rodriguez wrote: > 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. > > Signed-off-by: Luis R. Rodriguez <mcgrof@kernel.org> > --- > tools/testing/selftests/firmware/config | 4 +++ > tools/testing/selftests/firmware/fw_fallback.sh | 45 ++++++++++++++++++++++++- > 2 files changed, 48 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 722cad91df74..a42e437363d9 100755 > --- a/tools/testing/selftests/firmware/fw_fallback.sh > +++ b/tools/testing/selftests/firmware/fw_fallback.sh > @@ -6,7 +6,46 @@ > # won't find so that we can do the load ourself manually. > set -e > > +PROC_CONFIG="/proc/config.gz" How does this work when you are running the test on a target that does not have this kernel build option? thanks, greg k-h
On Mon, Nov 20, 2017 at 10:24:02AM -0800, Luis R. Rodriguez wrote: > 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. > > Signed-off-by: Luis R. Rodriguez <mcgrof@kernel.org> > --- > tools/testing/selftests/firmware/config | 4 +++ > tools/testing/selftests/firmware/fw_fallback.sh | 45 ++++++++++++++++++++++++- > 2 files changed, 48 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 722cad91df74..a42e437363d9 100755 > --- a/tools/testing/selftests/firmware/fw_fallback.sh > +++ b/tools/testing/selftests/firmware/fw_fallback.sh > @@ -6,7 +6,46 @@ > # won't find so that we can do the load ourself manually. > set -e > > +PROC_CONFIG="/proc/config.gz" > +TEST_DIR=$(dirname $0) > + > modprobe test_firmware > +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 > + > +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 > +} Shouldn't these functions be part of the kselftest core so that all tests can take advantage of them instead of having to hand-roll them for every individual test? And is there no way at runtime to tell what the options are and just not run that type of test? thanks, greg k-h
On Mon, Nov 20, 2017 at 10:24:02AM -0800, Luis R. Rodriguez wrote: > 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. > > Signed-off-by: Luis R. Rodriguez <mcgrof@kernel.org> I've applied the patches up to here in this series. thanks, greg k-h
On Wed, Nov 29, 2017 at 11:22:35AM +0100, Greg KH wrote: > On Mon, Nov 20, 2017 at 10:24:02AM -0800, Luis R. Rodriguez wrote: > > 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. > > > > Signed-off-by: Luis R. Rodriguez <mcgrof@kernel.org> > > --- > > tools/testing/selftests/firmware/config | 4 +++ > > tools/testing/selftests/firmware/fw_fallback.sh | 45 ++++++++++++++++++++++++- > > 2 files changed, 48 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 722cad91df74..a42e437363d9 100755 > > --- a/tools/testing/selftests/firmware/fw_fallback.sh > > +++ b/tools/testing/selftests/firmware/fw_fallback.sh > > @@ -6,7 +6,46 @@ > > # won't find so that we can do the load ourself manually. > > set -e > > > > +PROC_CONFIG="/proc/config.gz" > > +TEST_DIR=$(dirname $0) > > + > > modprobe test_firmware > > +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 > > + > > +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 > > +} > > Shouldn't these functions be part of the kselftest core so that all > tests can take advantage of them instead of having to hand-roll them for > every individual test? At a first glance it would seem to be nice. Note that in our case we'd still need a way to work without CONFIG_IKCONFIG_PROC, hence that big else condition, to ensure it works for kernels without CONFIG_IKCONFIG_PROC set, so we'd still end up with our own fw_kconfig_has(), and if we had a generic helper it'd look very similar... something like: fw_kconfig_has() { if [ has_proc_config ]; then echo $(kconfig_has("$1")) 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 } Not sure if there is a big gain then for now. Shuah, what do you think? Is it worth it? Do we have a generic bash library we can use? If not, if I add one, so that other scripts can source it, where should I add it? > And is there no way at runtime to tell what the options are and just > not run that type of test? For CONFIG_FW_LOADER_USER_HELPER=y yes, its handled in that else condition. For CONFIG_FW_LOADER_USER_HELPER_FALLBACK=y though no, there is no way. The else condition has a big comment indicating there is no current *easy* run time heuristic possible. This remains true specially since we have to support these scripts to run on older kernels as well. Luis
On 11/30/2017 01:31 PM, Luis R. Rodriguez wrote: > On Wed, Nov 29, 2017 at 11:22:35AM +0100, Greg KH wrote: >> On Mon, Nov 20, 2017 at 10:24:02AM -0800, Luis R. Rodriguez wrote: >>> 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. >>> >>> Signed-off-by: Luis R. Rodriguez <mcgrof@kernel.org> >>> --- >>> tools/testing/selftests/firmware/config | 4 +++ >>> tools/testing/selftests/firmware/fw_fallback.sh | 45 ++++++++++++++++++++++++- >>> 2 files changed, 48 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 722cad91df74..a42e437363d9 100755 >>> --- a/tools/testing/selftests/firmware/fw_fallback.sh >>> +++ b/tools/testing/selftests/firmware/fw_fallback.sh >>> @@ -6,7 +6,46 @@ >>> # won't find so that we can do the load ourself manually. >>> set -e >>> >>> +PROC_CONFIG="/proc/config.gz" >>> +TEST_DIR=$(dirname $0) >>> + >>> modprobe test_firmware >>> +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 >>> + >>> +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 >>> +} >> >> Shouldn't these functions be part of the kselftest core so that all >> tests can take advantage of them instead of having to hand-roll them for >> every individual test? > > At a first glance it would seem to be nice. > > Note that in our case we'd still need a way to work without CONFIG_IKCONFIG_PROC, > hence that big else condition, to ensure it works for kernels without > CONFIG_IKCONFIG_PROC set, so we'd still end up with our own fw_kconfig_has(), > and if we had a generic helper it'd look very similar... something like: > > fw_kconfig_has() > { > if [ has_proc_config ]; then > echo $(kconfig_has("$1")) > 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 > } > > Not sure if there is a big gain then for now. > > Shuah, what do you think? Is it worth it? Do we have a generic bash library we > can use? If not, if I add one, so that other scripts can source it, where > should I add it? There is no generic script right now. It depends on whether or not a generic script will help. Can it made generic or test would still need their customizations anyway. Furthermore, this generic script will become a dependency for tests and will need to be included in the install targets and also when O=dir cases. hence, the question on how generic can the script be and whether are not tests can avoid customization. I am finding in the case of selftests the generic framework sometimes add more complexity. In some cases it is worth it like lib.mk, however it takes work to override it as well. I have some reservations about introducing a generic script for this. thanks, -- Shuah
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 722cad91df74..a42e437363d9 100755 --- a/tools/testing/selftests/firmware/fw_fallback.sh +++ b/tools/testing/selftests/firmware/fw_fallback.sh @@ -6,7 +6,46 @@ # won't find so that we can do the load ourself manually. set -e +PROC_CONFIG="/proc/config.gz" +TEST_DIR=$(dirname $0) + modprobe test_firmware +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 + +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 +} DIR=/sys/devices/virtual/misc/test_firmware @@ -14,6 +53,7 @@ DIR=/sys/devices/virtual/misc/test_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_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) @@ -286,7 +326,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
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. Signed-off-by: Luis R. Rodriguez <mcgrof@kernel.org> --- tools/testing/selftests/firmware/config | 4 +++ tools/testing/selftests/firmware/fw_fallback.sh | 45 ++++++++++++++++++++++++- 2 files changed, 48 insertions(+), 1 deletion(-)