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 <mcg...@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
