From: Luis Chamberlain <mcgrof@kernel.org> Greg, I've found that Dan's patches really broke firmware testing. I've identified a proper fix for the issue he found, its the last patch. This series reverts his two patches which break testing, and fixes the issue he was running into. I leave it to him as exercise to add a busybox bash quirk for the bash issue he found with diff. His patches are merged on v5.0-rc1 as such these should go to Linus tree as well. They are regressions on v5.0-rc1. Please let me know if there are any questions. Luis Luis Chamberlain (3): Revert "selftests: firmware: add CONFIG_FW_LOADER_USER_HELPER_FALLBACK to config" Revert "selftests: firmware: remove use of non-standard diff -Z option" selftests: firmware: fix verify_reqs() return value tools/testing/selftests/firmware/config | 1 - tools/testing/selftests/firmware/fw_filesystem.sh | 9 ++++++--- tools/testing/selftests/firmware/fw_lib.sh | 2 +- 3 files changed, 7 insertions(+), 5 deletions(-) -- 2.18.0
From: Luis Chamberlain <mcgrof@kernel.org> This reverts commit 7492902e8d22b568463897fa967c0886764cf034. The commit tried to address an issue discovered by Dan where he got a message saying: 'usermode helper disabled so ignoring test'. Dans's commit is forcing CONFIG_FW_LOADER_USER_HELPER_FALLBACK but just having CONFIG_FW_LOADER_USER_HELPER suffices to emulate the_FALLBACK functionality. Dan's commit is trying to fix an issue which is hidden from a previous commit. That issue will be addressed properly next. Signed-off-by: Luis Chamberlain <mcgrof@kernel.org> --- tools/testing/selftests/firmware/config | 1 - 1 file changed, 1 deletion(-) diff --git a/tools/testing/selftests/firmware/config b/tools/testing/selftests/firmware/config index 913a25a4a32b..bf634dda0720 100644 --- a/tools/testing/selftests/firmware/config +++ b/tools/testing/selftests/firmware/config @@ -1,6 +1,5 @@ CONFIG_TEST_FIRMWARE=y CONFIG_FW_LOADER=y CONFIG_FW_LOADER_USER_HELPER=y -CONFIG_FW_LOADER_USER_HELPER_FALLBACK=y CONFIG_IKCONFIG=y CONFIG_IKCONFIG_PROC=y -- 2.18.0
From: Luis Chamberlain <mcgrof@kernel.org> This reverts commit f70b472e937bb659a7b7a14e64f07308e230888c. This breaks testing on Debian, and this patch was NACKed anyway. The proper way to address this is a quirk for busybox as that is where the issue is present. Signed-off-by: Luis Chamberlain <mcgrof@kernel.org> --- tools/testing/selftests/firmware/fw_filesystem.sh | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/tools/testing/selftests/firmware/fw_filesystem.sh b/tools/testing/selftests/firmware/fw_filesystem.sh index 466cf2f91ba0..a4320c4b44dc 100755 --- a/tools/testing/selftests/firmware/fw_filesystem.sh +++ b/tools/testing/selftests/firmware/fw_filesystem.sh @@ -155,8 +155,11 @@ read_firmwares() { for i in $(seq 0 3); do config_set_read_fw_idx $i - # Verify the contents match - if ! diff -q "$FW" $DIR/read_firmware 2>/dev/null ; then + # Verify the contents are what we expect. + # -Z required for now -- check for yourself, md5sum + # on $FW and DIR/read_firmware will yield the same. Even + # cmp agrees, so something is off. + if ! diff -q -Z "$FW" $DIR/read_firmware 2>/dev/null ; then echo "request #$i: firmware was not loaded" >&2 exit 1 fi @@ -168,7 +171,7 @@ read_firmwares_expect_nofile() for i in $(seq 0 3); do config_set_read_fw_idx $i # Ensures contents differ - if diff -q "$FW" $DIR/read_firmware 2>/dev/null ; then + if diff -q -Z "$FW" $DIR/read_firmware 2>/dev/null ; then echo "request $i: file was not expected to match" >&2 exit 1 fi -- 2.18.0
From: Luis Chamberlain <mcgrof@kernel.org> commit a6a9be9270c87 ("selftests: firmware: return Kselftest Skip code for skipped tests") by Shuah modified failures to return the special error code of $ksft_skip (4). We have a corner case issue where we *do* want to verify_reqs(). Cc: <stable@vger.kernel.org> # >= 4.18 Fixes: a6a9be9270c87 ("selftests: firmware: return Kselftest Skip code for for skipped tests") Signed-off-by: Luis Chamberlain <mcgrof@kernel.org> --- tools/testing/selftests/firmware/fw_lib.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/testing/selftests/firmware/fw_lib.sh b/tools/testing/selftests/firmware/fw_lib.sh index 6c5f1b2ffb74..1cbb12e284a6 100755 --- a/tools/testing/selftests/firmware/fw_lib.sh +++ b/tools/testing/selftests/firmware/fw_lib.sh @@ -91,7 +91,7 @@ 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 $ksft_skip + exit 0 fi fi } -- 2.18.0
On 2/7/19 12:05 PM, Luis R. Rodriguez wrote:
> From: Luis Chamberlain <mcgrof@kernel.org>
>
> Greg,
>
> I've found that Dan's patches really broke firmware testing. I've
> identified a proper fix for the issue he found, its the last patch. This
> series reverts his two patches which break testing, and fixes the issue
> he was running into. I leave it to him as exercise to add a busybox bash
> quirk for the bash issue he found with diff.
>
> His patches are merged on v5.0-rc1 as such these should go to Linus
> tree as well. They are regressions on v5.0-rc1. Please let me know if
> there are any questions.
>
> Luis
>
> Luis Chamberlain (3):
> Revert "selftests: firmware: add CONFIG_FW_LOADER_USER_HELPER_FALLBACK
> to config"
> Revert "selftests: firmware: remove use of non-standard diff -Z
> option"
> selftests: firmware: fix verify_reqs() return value
>
> tools/testing/selftests/firmware/config | 1 -
> tools/testing/selftests/firmware/fw_filesystem.sh | 9 ++++++---
> tools/testing/selftests/firmware/fw_lib.sh | 2 +-
> 3 files changed, 7 insertions(+), 5 deletions(-)
>
Luis: Thanks for finding and fixing the problems. My apologies
for pulling the problem patch in by mistake in the first place. :(
Greg! Do you plan to take these through firmware or do you want
me to get these in through kselftest tree. Either way is fine.
Acked-by: Shuah Khan <shuah@kernel.org>
thanks,
-- Shuah
On Fri, Feb 08, 2019 at 10:57:35AM -0700, shuah wrote:
> On 2/7/19 12:05 PM, Luis R. Rodriguez wrote:
> > From: Luis Chamberlain <mcgrof@kernel.org>
> >
> > Greg,
> >
> > I've found that Dan's patches really broke firmware testing. I've
> > identified a proper fix for the issue he found, its the last patch. This
> > series reverts his two patches which break testing, and fixes the issue
> > he was running into. I leave it to him as exercise to add a busybox bash
> > quirk for the bash issue he found with diff.
> >
> > His patches are merged on v5.0-rc1 as such these should go to Linus
> > tree as well. They are regressions on v5.0-rc1. Please let me know if
> > there are any questions.
> >
> > Luis
> >
> > Luis Chamberlain (3):
> > Revert "selftests: firmware: add CONFIG_FW_LOADER_USER_HELPER_FALLBACK
> > to config"
> > Revert "selftests: firmware: remove use of non-standard diff -Z
> > option"
> > selftests: firmware: fix verify_reqs() return value
> >
> > tools/testing/selftests/firmware/config | 1 -
> > tools/testing/selftests/firmware/fw_filesystem.sh | 9 ++++++---
> > tools/testing/selftests/firmware/fw_lib.sh | 2 +-
> > 3 files changed, 7 insertions(+), 5 deletions(-)
> >
>
> Luis: Thanks for finding and fixing the problems. My apologies
> for pulling the problem patch in by mistake in the first place. :(
>
> Greg! Do you plan to take these through firmware or do you want
> me to get these in through kselftest tree. Either way is fine.
>
> Acked-by: Shuah Khan <shuah@kernel.org>
I already queued these up in my tree yesterday, so no need to add them
to yours.
thanks,
greg k-h