linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/3] firmware_loader: few selftest fixes
@ 2019-02-07 19:05 Luis R. Rodriguez
  2019-02-07 19:06 ` [PATCH 1/3] Revert "selftests: firmware: add CONFIG_FW_LOADER_USER_HELPER_FALLBACK to config" Luis R. Rodriguez
                   ` (3 more replies)
  0 siblings, 4 replies; 6+ messages in thread
From: Luis R. Rodriguez @ 2019-02-07 19:05 UTC (permalink / raw)
  To: gregkh
  Cc: akpm, josh, rishabhb, kubakici, maco, andy.gross, david.brown,
	bjorn.andersson, linux-wireless, keescook, shuah, mfuzzey, zohar,
	dhowells, pali.rohar, tiwai, arend.vanspriel, zajec5, nbroeking,
	markivx, broonie, dmitry.torokhov, dwmw2, torvalds,
	Abhay_Salunke, jewalt, cantabile.desu, ast, andresx7, dan.rue,
	brendanhiggins, yzaikin, linux-kernel, linux-fsdevel,
	Luis Chamberlain

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


^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH 1/3] Revert "selftests: firmware: add CONFIG_FW_LOADER_USER_HELPER_FALLBACK to config"
  2019-02-07 19:05 [PATCH 0/3] firmware_loader: few selftest fixes Luis R. Rodriguez
@ 2019-02-07 19:06 ` Luis R. Rodriguez
  2019-02-07 19:06 ` [PATCH 2/3] Revert "selftests: firmware: remove use of non-standard diff -Z option" Luis R. Rodriguez
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 6+ messages in thread
From: Luis R. Rodriguez @ 2019-02-07 19:06 UTC (permalink / raw)
  To: gregkh
  Cc: akpm, josh, rishabhb, kubakici, maco, andy.gross, david.brown,
	bjorn.andersson, linux-wireless, keescook, shuah, mfuzzey, zohar,
	dhowells, pali.rohar, tiwai, arend.vanspriel, zajec5, nbroeking,
	markivx, broonie, dmitry.torokhov, dwmw2, torvalds,
	Abhay_Salunke, jewalt, cantabile.desu, ast, andresx7, dan.rue,
	brendanhiggins, yzaikin, linux-kernel, linux-fsdevel,
	Luis Chamberlain

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


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH 2/3] Revert "selftests: firmware: remove use of non-standard diff -Z option"
  2019-02-07 19:05 [PATCH 0/3] firmware_loader: few selftest fixes Luis R. Rodriguez
  2019-02-07 19:06 ` [PATCH 1/3] Revert "selftests: firmware: add CONFIG_FW_LOADER_USER_HELPER_FALLBACK to config" Luis R. Rodriguez
@ 2019-02-07 19:06 ` Luis R. Rodriguez
  2019-02-07 19:06 ` [PATCH 3/3] selftests: firmware: fix verify_reqs() return value Luis R. Rodriguez
  2019-02-08 17:57 ` [PATCH 0/3] firmware_loader: few selftest fixes shuah
  3 siblings, 0 replies; 6+ messages in thread
From: Luis R. Rodriguez @ 2019-02-07 19:06 UTC (permalink / raw)
  To: gregkh
  Cc: akpm, josh, rishabhb, kubakici, maco, andy.gross, david.brown,
	bjorn.andersson, linux-wireless, keescook, shuah, mfuzzey, zohar,
	dhowells, pali.rohar, tiwai, arend.vanspriel, zajec5, nbroeking,
	markivx, broonie, dmitry.torokhov, dwmw2, torvalds,
	Abhay_Salunke, jewalt, cantabile.desu, ast, andresx7, dan.rue,
	brendanhiggins, yzaikin, linux-kernel, linux-fsdevel,
	Luis Chamberlain

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


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH 3/3] selftests: firmware: fix verify_reqs() return value
  2019-02-07 19:05 [PATCH 0/3] firmware_loader: few selftest fixes Luis R. Rodriguez
  2019-02-07 19:06 ` [PATCH 1/3] Revert "selftests: firmware: add CONFIG_FW_LOADER_USER_HELPER_FALLBACK to config" Luis R. Rodriguez
  2019-02-07 19:06 ` [PATCH 2/3] Revert "selftests: firmware: remove use of non-standard diff -Z option" Luis R. Rodriguez
@ 2019-02-07 19:06 ` Luis R. Rodriguez
  2019-02-08 17:57 ` [PATCH 0/3] firmware_loader: few selftest fixes shuah
  3 siblings, 0 replies; 6+ messages in thread
From: Luis R. Rodriguez @ 2019-02-07 19:06 UTC (permalink / raw)
  To: gregkh
  Cc: akpm, josh, rishabhb, kubakici, maco, andy.gross, david.brown,
	bjorn.andersson, linux-wireless, keescook, shuah, mfuzzey, zohar,
	dhowells, pali.rohar, tiwai, arend.vanspriel, zajec5, nbroeking,
	markivx, broonie, dmitry.torokhov, dwmw2, torvalds,
	Abhay_Salunke, jewalt, cantabile.desu, ast, andresx7, dan.rue,
	brendanhiggins, yzaikin, linux-kernel, linux-fsdevel,
	Luis Chamberlain, stable

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


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH 0/3] firmware_loader: few selftest fixes
  2019-02-07 19:05 [PATCH 0/3] firmware_loader: few selftest fixes Luis R. Rodriguez
                   ` (2 preceding siblings ...)
  2019-02-07 19:06 ` [PATCH 3/3] selftests: firmware: fix verify_reqs() return value Luis R. Rodriguez
@ 2019-02-08 17:57 ` shuah
  2019-02-09  8:25   ` Greg KH
  3 siblings, 1 reply; 6+ messages in thread
From: shuah @ 2019-02-08 17:57 UTC (permalink / raw)
  To: Luis R. Rodriguez, gregkh
  Cc: akpm, josh, rishabhb, kubakici, maco, andy.gross, david.brown,
	bjorn.andersson, linux-wireless, keescook, mfuzzey, zohar,
	dhowells, pali.rohar, tiwai, arend.vanspriel, zajec5, nbroeking,
	markivx, broonie, dmitry.torokhov, dwmw2, torvalds,
	Abhay_Salunke, jewalt, cantabile.desu, ast, andresx7, dan.rue,
	brendanhiggins, yzaikin, linux-kernel, linux-fsdevel, shuah

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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH 0/3] firmware_loader: few selftest fixes
  2019-02-08 17:57 ` [PATCH 0/3] firmware_loader: few selftest fixes shuah
@ 2019-02-09  8:25   ` Greg KH
  0 siblings, 0 replies; 6+ messages in thread
From: Greg KH @ 2019-02-09  8:25 UTC (permalink / raw)
  To: shuah
  Cc: Luis R. Rodriguez, akpm, josh, rishabhb, kubakici, maco,
	andy.gross, david.brown, bjorn.andersson, linux-wireless,
	keescook, mfuzzey, zohar, dhowells, pali.rohar, tiwai,
	arend.vanspriel, zajec5, nbroeking, markivx, broonie,
	dmitry.torokhov, dwmw2, torvalds, Abhay_Salunke, jewalt,
	cantabile.desu, ast, andresx7, dan.rue, brendanhiggins, yzaikin,
	linux-kernel, linux-fsdevel

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

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2019-02-09  8:25 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-02-07 19:05 [PATCH 0/3] firmware_loader: few selftest fixes Luis R. Rodriguez
2019-02-07 19:06 ` [PATCH 1/3] Revert "selftests: firmware: add CONFIG_FW_LOADER_USER_HELPER_FALLBACK to config" Luis R. Rodriguez
2019-02-07 19:06 ` [PATCH 2/3] Revert "selftests: firmware: remove use of non-standard diff -Z option" Luis R. Rodriguez
2019-02-07 19:06 ` [PATCH 3/3] selftests: firmware: fix verify_reqs() return value Luis R. Rodriguez
2019-02-08 17:57 ` [PATCH 0/3] firmware_loader: few selftest fixes shuah
2019-02-09  8:25   ` Greg KH

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).