From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751208AbdK3UcC (ORCPT ); Thu, 30 Nov 2017 15:32:02 -0500 Received: from mx2.suse.de ([195.135.220.15]:49568 "EHLO mx2.suse.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750921AbdK3UcA (ORCPT ); Thu, 30 Nov 2017 15:32:00 -0500 Date: Thu, 30 Nov 2017 21:31:58 +0100 From: "Luis R. Rodriguez" To: Greg KH , shuah@kernel.org Cc: "Luis R. Rodriguez" , akpm@linux-foundation.org, keescook@chromium.org, mfuzzey@parkeon.com, zohar@linux.vnet.ibm.com, dhowells@redhat.com, pali.rohar@gmail.com, tiwai@suse.de, arend.vanspriel@broadcom.com, zajec5@gmail.com, nbroeking@me.com, markivx@codeaurora.org, stephen.boyd@linaro.org, broonie@kernel.org, dmitry.torokhov@gmail.com, dwmw2@infradead.org, torvalds@linux-foundation.org, Abhay_Salunke@dell.com, bjorn.andersson@linaro.org, jewalt@lgsinnovations.com, linux-kernel@vger.kernel.org, linux-fsdevel@vger.kernel.org Subject: Re: [PATCH v2 16/23] test_firmware: enable custom fallback testing on limited kernel configs Message-ID: <20171130203158.GM729@wotan.suse.de> References: <20171120182409.27348-1-mcgrof@kernel.org> <20171120182409.27348-17-mcgrof@kernel.org> <20171129102235.GB11522@kroah.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20171129102235.GB11522@kroah.com> User-Agent: Mutt/1.6.0 (2016-04-01) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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 > > --- > > 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