[PATCH 0/5] Firmware loader support for ZSTD-compressed files

[PATCH 0/5] Firmware loader support for ZSTD-compressed files

[Takashi Iwai]

Hi,

this is a revised patch set for RFC I posted some time ago (*).
Since the ZSTD usage became much more popular now, it makes sense to
have the consistent (de)compression support in the kernel, also for
the firmware files.  This patch set adds the support for ZSTD-
compressed firmware files as well as the extension of selftests, in
addition to a couple of relevant fixes in selftests.

(*) https://lore.kernel.org/r/20210127154939.13288-1-tiwai@suse.de


Takashi

===

Takashi Iwai (5):
  firmware: Add the support for ZSTD-compressed firmware files
  selftests: firmware: Use smaller dictionary for XZ compression
  selftests: firmware: Fix the request_firmware_into_buf() test for XZ
    format
  selftests: firmware: Simplify test patterns
  selftests: firmware: Add ZSTD compressed file tests

 drivers/base/firmware_loader/Kconfig          |  24 ++-
 drivers/base/firmware_loader/main.c           |  76 +++++++-
 .../selftests/firmware/fw_filesystem.sh       | 170 +++++++++---------
 tools/testing/selftests/firmware/fw_lib.sh    |  12 +-
 4 files changed, 182 insertions(+), 100 deletions(-)

-- 
2.31.1

[PATCH 1/5] firmware: Add the support for ZSTD-compressed firmware files

[Takashi Iwai]

As the growing demand on ZSTD compressions, there have been requests
for the support of ZSTD-compressed firmware files, so here it is:
this patch extends the firmware loader code to allow loading ZSTD
files.  The implementation is fairly straightforward, it just adds a
ZSTD decompression routine for the file expander.  (And the code is
even simpler than XZ thanks to the ZSTD API that gives the original
decompressed size from the header.)

Tested-by: Piotr Gorski <lucjan.lucjanov@gmail.com>
Link: https://lore.kernel.org/all/20210127154939.13288-1-tiwai@suse.de/
Signed-off-by: Takashi Iwai <tiwai@suse.de>
---
 drivers/base/firmware_loader/Kconfig | 24 ++++++---
 drivers/base/firmware_loader/main.c  | 76 ++++++++++++++++++++++++++--
 2 files changed, 91 insertions(+), 9 deletions(-)

diff --git a/drivers/base/firmware_loader/Kconfig b/drivers/base/firmware_loader/Kconfig
index 38f3b66bf52b..08bb50451a96 100644
--- a/drivers/base/firmware_loader/Kconfig
+++ b/drivers/base/firmware_loader/Kconfig
@@ -159,21 +159,33 @@ config FW_LOADER_USER_HELPER_FALLBACK
 
 config FW_LOADER_COMPRESS
 	bool "Enable compressed firmware support"
-	select FW_LOADER_PAGED_BUF
-	select XZ_DEC
 	help
 	  This option enables the support for loading compressed firmware
 	  files. The caller of firmware API receives the decompressed file
 	  content. The compressed file is loaded as a fallback, only after
 	  loading the raw file failed at first.
 
-	  Currently only XZ-compressed files are supported, and they have to
-	  be compressed with either none or crc32 integrity check type (pass
-	  "-C crc32" option to xz command).
-
 	  Compressed firmware support does not apply to firmware images
 	  that are built into the kernel image (CONFIG_EXTRA_FIRMWARE).
 
+if FW_LOADER_COMPRESS
+config FW_LOADER_COMPRESS_XZ
+	bool "Enable XZ-compressed firmware support"
+	select FW_LOADER_PAGED_BUF
+	select XZ_DEC
+	help
+	  This option adds the support for XZ-compressed files.
+	  The files have to be compressed with either none or crc32
+	  integrity check type (pass "-C crc32" option to xz command).
+
+config FW_LOADER_COMPRESS_ZSTD
+	bool "Enable ZSTD-compressed firmware support"
+	select ZSTD_DECOMPRESS
+	help
+	  This option adds the support for ZSTD-compressed files.
+
+endif # FW_LOADER_COMPRESS
+
 config FW_CACHE
 	bool "Enable firmware caching during suspend"
 	depends on PM_SLEEP
diff --git a/drivers/base/firmware_loader/main.c b/drivers/base/firmware_loader/main.c
index 94d1789a233e..74830aeec7f6 100644
--- a/drivers/base/firmware_loader/main.c
+++ b/drivers/base/firmware_loader/main.c
@@ -35,6 +35,7 @@
 #include <linux/syscore_ops.h>
 #include <linux/reboot.h>
 #include <linux/security.h>
+#include <linux/zstd.h>
 #include <linux/xz.h>
 
 #include <generated/utsrelease.h>
@@ -304,10 +305,74 @@ int fw_map_paged_buf(struct fw_priv *fw_priv)
 }
 #endif
 
+/*
+ * ZSTD-compressed firmware support
+ */
+#ifdef CONFIG_FW_LOADER_COMPRESS_ZSTD
+static int fw_decompress_zstd(struct device *dev, struct fw_priv *fw_priv,
+			      size_t in_size, const void *in_buffer)
+{
+	size_t len, out_size, workspace_size;
+	void *workspace, *out_buf;
+	zstd_dctx *ctx;
+	int err;
+
+	if (fw_priv->allocated_size) {
+		out_size = fw_priv->allocated_size;
+		out_buf = fw_priv->data;
+	} else {
+		zstd_frame_header params;
+
+		if (zstd_get_frame_header(&params, in_buffer, in_size) ||
+		    params.frameContentSize == ZSTD_CONTENTSIZE_UNKNOWN) {
+			dev_dbg(dev, "%s: invalid zstd header\n", __func__);
+			return -EINVAL;
+		}
+		out_size = params.frameContentSize;
+		out_buf = vzalloc(out_size);
+		if (!out_buf)
+			return -ENOMEM;
+	}
+
+	workspace_size = zstd_dctx_workspace_bound();
+	workspace = kvzalloc(workspace_size, GFP_KERNEL);
+	if (!workspace) {
+		err = -ENOMEM;
+		goto error;
+	}
+
+	ctx = zstd_init_dctx(workspace, workspace_size);
+	if (!ctx) {
+		dev_dbg(dev, "%s: failed to initialize context\n", __func__);
+		err = -EINVAL;
+		goto error;
+	}
+
+	len = zstd_decompress_dctx(ctx, out_buf, out_size, in_buffer, in_size);
+	if (zstd_is_error(len)) {
+		dev_dbg(dev, "%s: failed to decompress: %d\n", __func__,
+			zstd_get_error_code(len));
+		err = -EINVAL;
+		goto error;
+	}
+
+	if (!fw_priv->allocated_size)
+		fw_priv->data = out_buf;
+	fw_priv->size = len;
+	err = 0;
+
+ error:
+	kvfree(workspace);
+	if (err && !fw_priv->allocated_size)
+		vfree(out_buf);
+	return err;
+}
+#endif /* CONFIG_FW_LOADER_COMPRESS_ZSTD */
+
 /*
  * XZ-compressed firmware support
  */
-#ifdef CONFIG_FW_LOADER_COMPRESS
+#ifdef CONFIG_FW_LOADER_COMPRESS_XZ
 /* show an error and return the standard error code */
 static int fw_decompress_xz_error(struct device *dev, enum xz_ret xz_ret)
 {
@@ -401,7 +466,7 @@ static int fw_decompress_xz(struct device *dev, struct fw_priv *fw_priv,
 	else
 		return fw_decompress_xz_pages(dev, fw_priv, in_size, in_buffer);
 }
-#endif /* CONFIG_FW_LOADER_COMPRESS */
+#endif /* CONFIG_FW_LOADER_COMPRESS_XZ */
 
 /* direct firmware loading support */
 static char fw_path_para[256];
@@ -757,7 +822,12 @@ _request_firmware(const struct firmware **firmware_p, const char *name,
 	if (!(opt_flags & FW_OPT_PARTIAL))
 		nondirect = true;
 
-#ifdef CONFIG_FW_LOADER_COMPRESS
+#ifdef CONFIG_FW_LOADER_COMPRESS_ZSTD
+	if (ret == -ENOENT && nondirect)
+		ret = fw_get_filesystem_firmware(device, fw->priv, ".zst",
+						 fw_decompress_zstd);
+#endif
+#ifdef CONFIG_FW_LOADER_COMPRESS_XZ
 	if (ret == -ENOENT && nondirect)
 		ret = fw_get_filesystem_firmware(device, fw->priv, ".xz",
 						 fw_decompress_xz);
-- 
2.31.1

[PATCH 2/5] selftests: firmware: Use smaller dictionary for XZ compression

[Takashi Iwai]

The xz -9 option leads to an unnecessarily too large dictionary that
isn't really suitable for the kernel firmware loader.  Pass the
dictionary size explicitly, instead.

While we're at it, make the xz command call defined in $RUN_XZ for
simplicity.

Fixes: 108ae07c5036 ("selftests: firmware: Add compressed firmware tests")
Signed-off-by: Takashi Iwai <tiwai@suse.de>
---
 tools/testing/selftests/firmware/fw_filesystem.sh | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/tools/testing/selftests/firmware/fw_filesystem.sh b/tools/testing/selftests/firmware/fw_filesystem.sh
index c2a2a100114b..731f011def78 100755
--- a/tools/testing/selftests/firmware/fw_filesystem.sh
+++ b/tools/testing/selftests/firmware/fw_filesystem.sh
@@ -11,6 +11,8 @@ TEST_REQS_FW_SET_CUSTOM_PATH="yes"
 TEST_DIR=$(dirname $0)
 source $TEST_DIR/fw_lib.sh
 
+RUN_XZ="xz -C crc32 --lzma2=dict=2MiB"
+
 check_mods
 check_setup
 verify_reqs
@@ -410,9 +412,9 @@ test_request_firmware_nowait_custom()
 	RANDOM_FILE_PATH=$(setup_random_file)
 	RANDOM_FILE="$(basename $RANDOM_FILE_PATH)"
 	if [ "$2" = "both" ]; then
-		xz -9 -C crc32 -k $RANDOM_FILE_PATH
+		$RUN_XZ -k $RANDOM_FILE_PATH
 	elif [ "$2" = "xzonly" ]; then
-		xz -9 -C crc32 $RANDOM_FILE_PATH
+		$RUN_XZ $RANDOM_FILE_PATH
 	fi
 	config_set_name $RANDOM_FILE
 	config_trigger_async
@@ -501,7 +503,7 @@ test_request_partial_firmware_into_buf_nofile 2 10
 test "$HAS_FW_LOADER_COMPRESS" != "yes" && exit 0
 
 # test with both files present
-xz -9 -C crc32 -k $FW
+$RUN_XZ -k $FW
 config_set_name $NAME
 echo
 echo "Testing with both plain and xz files present..."
-- 
2.31.1

[PATCH 3/5] selftests: firmware: Fix the request_firmware_into_buf() test for XZ format

[Takashi Iwai]

The test uses a different firmware name, and we forgot to adapt for
the XZ compressed file tests.

Fixes: 1798045900b7 ("selftests: firmware: Add request_firmware_into_buf tests")
https://lore.kernel.org/all/20210127154939.13288-1-tiwai@suse.de/
Signed-off-by: Takashi Iwai <tiwai@suse.de>
---
 tools/testing/selftests/firmware/fw_filesystem.sh | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/tools/testing/selftests/firmware/fw_filesystem.sh b/tools/testing/selftests/firmware/fw_filesystem.sh
index 731f011def78..3ac09b401a83 100755
--- a/tools/testing/selftests/firmware/fw_filesystem.sh
+++ b/tools/testing/selftests/firmware/fw_filesystem.sh
@@ -504,6 +504,7 @@ test "$HAS_FW_LOADER_COMPRESS" != "yes" && exit 0
 
 # test with both files present
 $RUN_XZ -k $FW
+$RUN_XZ -k $FW_INTO_BUF
 config_set_name $NAME
 echo
 echo "Testing with both plain and xz files present..."
@@ -529,6 +530,7 @@ done
 
 # test with only xz file present
 mv "$FW" "${FW}-orig"
+mv "$FW_INTO_BUF" "${FW_INTO_BUF}-orig"
 echo
 echo "Testing with only xz file present..."
 for i in $(seq 1 5); do
-- 
2.31.1

[PATCH 4/5] selftests: firmware: Simplify test patterns

[Takashi Iwai]

The test patterns are almost same in three sequential tests.
Make the unified helper function for improving the readability.

Link: https://lore.kernel.org/all/20210127154939.13288-1-tiwai@suse.de/
Signed-off-by: Takashi Iwai <tiwai@suse.de>
---
 .../selftests/firmware/fw_filesystem.sh       | 106 +++++-------------
 1 file changed, 30 insertions(+), 76 deletions(-)

diff --git a/tools/testing/selftests/firmware/fw_filesystem.sh b/tools/testing/selftests/firmware/fw_filesystem.sh
index 3ac09b401a83..4a574be8b862 100755
--- a/tools/testing/selftests/firmware/fw_filesystem.sh
+++ b/tools/testing/selftests/firmware/fw_filesystem.sh
@@ -437,6 +437,32 @@ test_request_partial_firmware_into_buf()
 	echo "OK"
 }
 
+do_tests ()
+{
+	mode="$1"
+	suffix="$2"
+
+	for i in $(seq 1 5); do
+		test_batched_request_firmware$suffix $i $mode
+	done
+
+	for i in $(seq 1 5); do
+		test_batched_request_firmware_into_buf$suffix $i $mode
+	done
+
+	for i in $(seq 1 5); do
+		test_batched_request_firmware_direct$suffix $i $mode
+	done
+
+	for i in $(seq 1 5); do
+		test_request_firmware_nowait_uevent$suffix $i $mode
+	done
+
+	for i in $(seq 1 5); do
+		test_request_firmware_nowait_custom$suffix $i $mode
+	done
+}
+
 # Only continue if batched request triggers are present on the
 # test-firmware driver
 test_config_present
@@ -444,25 +470,7 @@ test_config_present
 # test with the file present
 echo
 echo "Testing with the file present..."
-for i in $(seq 1 5); do
-	test_batched_request_firmware $i normal
-done
-
-for i in $(seq 1 5); do
-	test_batched_request_firmware_into_buf $i normal
-done
-
-for i in $(seq 1 5); do
-	test_batched_request_firmware_direct $i normal
-done
-
-for i in $(seq 1 5); do
-	test_request_firmware_nowait_uevent $i normal
-done
-
-for i in $(seq 1 5); do
-	test_request_firmware_nowait_custom $i normal
-done
+do_tests normal
 
 # Partial loads cannot use fallback, so do not repeat tests.
 test_request_partial_firmware_into_buf 0 10
@@ -474,25 +482,7 @@ test_request_partial_firmware_into_buf 2 10
 # a hung task, which would require a hard reset.
 echo
 echo "Testing with the file missing..."
-for i in $(seq 1 5); do
-	test_batched_request_firmware_nofile $i
-done
-
-for i in $(seq 1 5); do
-	test_batched_request_firmware_into_buf_nofile $i
-done
-
-for i in $(seq 1 5); do
-	test_batched_request_firmware_direct_nofile $i
-done
-
-for i in $(seq 1 5); do
-	test_request_firmware_nowait_uevent_nofile $i
-done
-
-for i in $(seq 1 5); do
-	test_request_firmware_nowait_custom_nofile $i
-done
+do_tests nofile _nofile
 
 # Partial loads cannot use fallback, so do not repeat tests.
 test_request_partial_firmware_into_buf_nofile 0 10
@@ -508,49 +498,13 @@ $RUN_XZ -k $FW_INTO_BUF
 config_set_name $NAME
 echo
 echo "Testing with both plain and xz files present..."
-for i in $(seq 1 5); do
-	test_batched_request_firmware $i both
-done
-
-for i in $(seq 1 5); do
-	test_batched_request_firmware_into_buf $i both
-done
-
-for i in $(seq 1 5); do
-	test_batched_request_firmware_direct $i both
-done
-
-for i in $(seq 1 5); do
-	test_request_firmware_nowait_uevent $i both
-done
-
-for i in $(seq 1 5); do
-	test_request_firmware_nowait_custom $i both
-done
+do_tests both
 
 # test with only xz file present
 mv "$FW" "${FW}-orig"
 mv "$FW_INTO_BUF" "${FW_INTO_BUF}-orig"
 echo
 echo "Testing with only xz file present..."
-for i in $(seq 1 5); do
-	test_batched_request_firmware $i xzonly
-done
-
-for i in $(seq 1 5); do
-	test_batched_request_firmware_into_buf $i xzonly
-done
-
-for i in $(seq 1 5); do
-	test_batched_request_firmware_direct $i xzonly
-done
-
-for i in $(seq 1 5); do
-	test_request_firmware_nowait_uevent $i xzonly
-done
-
-for i in $(seq 1 5); do
-	test_request_firmware_nowait_custom $i xzonly
-done
+do_tests xzonly
 
 exit 0
-- 
2.31.1

[PATCH 5/5] selftests: firmware: Add ZSTD compressed file tests

[Takashi Iwai]

It's similar like XZ compressed files.  For the simplicity, both XZ
and ZSTD tests are done in a single function.  The format is specified
via $COMPRESS_FORMAT and the compression function is pre-defined.

Link: https://lore.kernel.org/r/20210127154939.13288-5-tiwai@suse.de
Signed-off-by: Takashi Iwai <tiwai@suse.de>
---
 .../selftests/firmware/fw_filesystem.sh       | 76 ++++++++++++++-----
 tools/testing/selftests/firmware/fw_lib.sh    | 12 ++-
 2 files changed, 65 insertions(+), 23 deletions(-)

diff --git a/tools/testing/selftests/firmware/fw_filesystem.sh b/tools/testing/selftests/firmware/fw_filesystem.sh
index 4a574be8b862..1a99aea0549e 100755
--- a/tools/testing/selftests/firmware/fw_filesystem.sh
+++ b/tools/testing/selftests/firmware/fw_filesystem.sh
@@ -12,6 +12,7 @@ TEST_DIR=$(dirname $0)
 source $TEST_DIR/fw_lib.sh
 
 RUN_XZ="xz -C crc32 --lzma2=dict=2MiB"
+RUN_ZSTD="zstd -q"
 
 check_mods
 check_setup
@@ -213,7 +214,7 @@ read_firmwares()
 	else
 		fwfile="$FW"
 	fi
-	if [ "$1" = "xzonly" ]; then
+	if [ "$1" = "componly" ]; then
 		fwfile="${fwfile}-orig"
 	fi
 	for i in $(seq 0 3); do
@@ -237,7 +238,7 @@ read_partial_firmwares()
 		fwfile="${FW}"
 	fi
 
-	if [ "$1" = "xzonly" ]; then
+	if [ "$1" = "componly" ]; then
 		fwfile="${fwfile}-orig"
 	fi
 
@@ -411,10 +412,8 @@ test_request_firmware_nowait_custom()
 	config_unset_uevent
 	RANDOM_FILE_PATH=$(setup_random_file)
 	RANDOM_FILE="$(basename $RANDOM_FILE_PATH)"
-	if [ "$2" = "both" ]; then
-		$RUN_XZ -k $RANDOM_FILE_PATH
-	elif [ "$2" = "xzonly" ]; then
-		$RUN_XZ $RANDOM_FILE_PATH
+	if [ -n "$2" -a "$2" != "normal" ]; then
+		compress_"$2"_"$COMPRESS_FORMAT" $RANDOM_FILE_PATH
 	fi
 	config_set_name $RANDOM_FILE
 	config_trigger_async
@@ -490,21 +489,58 @@ test_request_partial_firmware_into_buf_nofile 0 5
 test_request_partial_firmware_into_buf_nofile 1 6
 test_request_partial_firmware_into_buf_nofile 2 10
 
-test "$HAS_FW_LOADER_COMPRESS" != "yes" && exit 0
+test_request_firmware_compressed ()
+{
+	export COMPRESS_FORMAT="$1"
 
-# test with both files present
-$RUN_XZ -k $FW
-$RUN_XZ -k $FW_INTO_BUF
-config_set_name $NAME
-echo
-echo "Testing with both plain and xz files present..."
-do_tests both
+	# test with both files present
+	compress_both_"$COMPRESS_FORMAT" $FW
+	compress_both_"$COMPRESS_FORMAT" $FW_INTO_BUF
 
-# test with only xz file present
-mv "$FW" "${FW}-orig"
-mv "$FW_INTO_BUF" "${FW_INTO_BUF}-orig"
-echo
-echo "Testing with only xz file present..."
-do_tests xzonly
+	config_set_name $NAME
+	echo
+	echo "Testing with both plain and $COMPRESS_FORMAT files present..."
+	do_tests both
+
+	# test with only compressed file present
+	mv "$FW" "${FW}-orig"
+	mv "$FW_INTO_BUF" "${FW_INTO_BUF}-orig"
+
+	config_set_name $NAME
+	echo
+	echo "Testing with only $COMPRESS_FORMAT file present..."
+	do_tests componly
+
+	mv "${FW}-orig" "$FW"
+	mv "${FW_INTO_BUF}-orig" "$FW_INTO_BUF"
+}
+
+compress_both_XZ ()
+{
+	$RUN_XZ -k "$@"
+}
+
+compress_componly_XZ ()
+{
+	$RUN_XZ "$@"
+}
+
+compress_both_ZSTD ()
+{
+	$RUN_ZSTD -k "$@"
+}
+
+compress_componly_ZSTD ()
+{
+	$RUN_ZSTD --rm "$@"
+}
+
+if test "$HAS_FW_LOADER_COMPRESS_XZ" = "yes"; then
+	test_request_firmware_compressed XZ
+fi
+
+if test "$HAS_FW_LOADER_COMPRESS_ZSTD" = "yes"; then
+	test_request_firmware_compressed ZSTD
+fi
 
 exit 0
diff --git a/tools/testing/selftests/firmware/fw_lib.sh b/tools/testing/selftests/firmware/fw_lib.sh
index 5b8c0fedee76..3fa8282b053b 100755
--- a/tools/testing/selftests/firmware/fw_lib.sh
+++ b/tools/testing/selftests/firmware/fw_lib.sh
@@ -62,7 +62,8 @@ check_setup()
 {
 	HAS_FW_LOADER_USER_HELPER="$(kconfig_has CONFIG_FW_LOADER_USER_HELPER=y)"
 	HAS_FW_LOADER_USER_HELPER_FALLBACK="$(kconfig_has CONFIG_FW_LOADER_USER_HELPER_FALLBACK=y)"
-	HAS_FW_LOADER_COMPRESS="$(kconfig_has CONFIG_FW_LOADER_COMPRESS=y)"
+	HAS_FW_LOADER_COMPRESS_XZ="$(kconfig_has CONFIG_FW_LOADER_COMPRESS_XZ=y)"
+	HAS_FW_LOADER_COMPRESS_ZSTD="$(kconfig_has CONFIG_FW_LOADER_COMPRESS_ZSTD=y)"
 	PROC_FW_IGNORE_SYSFS_FALLBACK="0"
 	PROC_FW_FORCE_SYSFS_FALLBACK="0"
 
@@ -98,9 +99,14 @@ check_setup()
 
 	OLD_FWPATH="$(cat /sys/module/firmware_class/parameters/path)"
 
-	if [ "$HAS_FW_LOADER_COMPRESS" = "yes" ]; then
+	if [ "$HAS_FW_LOADER_COMPRESS_XZ" = "yes" ]; then
 		if ! which xz 2> /dev/null > /dev/null; then
-			HAS_FW_LOADER_COMPRESS=""
+			HAS_FW_LOADER_COMPRESS_XZ=""
+		fi
+	fi
+	if [ "$HAS_FW_LOADER_COMPRESS_ZSTD" = "yes" ]; then
+		if ! which zstd 2> /dev/null > /dev/null; then
+			HAS_FW_LOADER_COMPRESS_ZSTD=""
 		fi
 	fi
 }
-- 
2.31.1

Re: [PATCH 0/5] Firmware loader support for ZSTD-compressed files

[Luis Chamberlain]

On Thu, Apr 21, 2022 at 05:29:03PM +0200, Takashi Iwai wrote:
> Hi,
> 
> this is a revised patch set for RFC I posted some time ago (*).
> Since the ZSTD usage became much more popular now, it makes sense to
> have the consistent (de)compression support in the kernel, also for
> the firmware files.  This patch set adds the support for ZSTD-
> compressed firmware files as well as the extension of selftests, in
> addition to a couple of relevant fixes in selftests.
> 
> (*) https://lore.kernel.org/r/20210127154939.13288-1-tiwai@suse.de

Russ had posted a set of patches which this likely needs to be
rebased on top of. Russ however has to address one kernel splat
found by 0day, so I'd expect a new set and then perhaps this can
be based on that?

[0] https://lkml.kernel.org/r/20220419231658.664388-1-russell.h.weight@intel.com    

  Luis

Re: [PATCH 0/5] Firmware loader support for ZSTD-compressed files

[Takashi Iwai]

On Thu, 21 Apr 2022 18:48:05 +0200,
Luis Chamberlain wrote:
> 
> On Thu, Apr 21, 2022 at 05:29:03PM +0200, Takashi Iwai wrote:
> > Hi,
> > 
> > this is a revised patch set for RFC I posted some time ago (*).
> > Since the ZSTD usage became much more popular now, it makes sense to
> > have the consistent (de)compression support in the kernel, also for
> > the firmware files.  This patch set adds the support for ZSTD-
> > compressed firmware files as well as the extension of selftests, in
> > addition to a couple of relevant fixes in selftests.
> > 
> > (*) https://lore.kernel.org/r/20210127154939.13288-1-tiwai@suse.de
> 
> Russ had posted a set of patches which this likely needs to be
> rebased on top of. Russ however has to address one kernel splat
> found by 0day, so I'd expect a new set and then perhaps this can
> be based on that?

Sure, it should be fine, as the code change there is quite minimal.
Let me know if the base branch becomes ready.


thanks,

Takashi

> 
> [0] https://lkml.kernel.org/r/20220419231658.664388-1-russell.h.weight@intel.com    
> 
>   Luis
> 

Re: [PATCH 0/5] Firmware loader support for ZSTD-compressed files

[Greg Kroah-Hartman]

On Fri, Apr 22, 2022 at 08:38:07AM +0200, Takashi Iwai wrote:
> On Thu, 21 Apr 2022 18:48:05 +0200,
> Luis Chamberlain wrote:
> > 
> > On Thu, Apr 21, 2022 at 05:29:03PM +0200, Takashi Iwai wrote:
> > > Hi,
> > > 
> > > this is a revised patch set for RFC I posted some time ago (*).
> > > Since the ZSTD usage became much more popular now, it makes sense to
> > > have the consistent (de)compression support in the kernel, also for
> > > the firmware files.  This patch set adds the support for ZSTD-
> > > compressed firmware files as well as the extension of selftests, in
> > > addition to a couple of relevant fixes in selftests.
> > > 
> > > (*) https://lore.kernel.org/r/20210127154939.13288-1-tiwai@suse.de
> > 
> > Russ had posted a set of patches which this likely needs to be
> > rebased on top of. Russ however has to address one kernel splat
> > found by 0day, so I'd expect a new set and then perhaps this can
> > be based on that?
> 
> Sure, it should be fine, as the code change there is quite minimal.
> Let me know if the base branch becomes ready.

Your patches came in before Russ's, so I'll queue them up now first.

thanks,

greg k-h

Re: [PATCH 2/5] selftests: firmware: Use smaller dictionary for XZ compression

[Shuah Khan]

On 4/21/22 9:29 AM, Takashi Iwai wrote:
> The xz -9 option leads to an unnecessarily too large dictionary that
> isn't really suitable for the kernel firmware loader.  Pass the
> dictionary size explicitly, instead.
> 
> While we're at it, make the xz command call defined in $RUN_XZ for
> simplicity.
> 
> Fixes: 108ae07c5036 ("selftests: firmware: Add compressed firmware tests")
> Signed-off-by: Takashi Iwai <tiwai@suse.de>
> ---
>   tools/testing/selftests/firmware/fw_filesystem.sh | 8 +++++---
>   1 file changed, 5 insertions(+), 3 deletions(-)
> 
> diff --git a/tools/testing/selftests/firmware/fw_filesystem.sh b/tools/testing/selftests/firmware/fw_filesystem.sh
> index c2a2a100114b..731f011def78 100755
> --- a/tools/testing/selftests/firmware/fw_filesystem.sh
> +++ b/tools/testing/selftests/firmware/fw_filesystem.sh
> @@ -11,6 +11,8 @@ TEST_REQS_FW_SET_CUSTOM_PATH="yes"
>   TEST_DIR=$(dirname $0)
>   source $TEST_DIR/fw_lib.sh
>   
> +RUN_XZ="xz -C crc32 --lzma2=dict=2MiB"
> +
>   check_mods
>   check_setup
>   verify_reqs
> @@ -410,9 +412,9 @@ test_request_firmware_nowait_custom()
>   	RANDOM_FILE_PATH=$(setup_random_file)
>   	RANDOM_FILE="$(basename $RANDOM_FILE_PATH)"
>   	if [ "$2" = "both" ]; then
> -		xz -9 -C crc32 -k $RANDOM_FILE_PATH
> +		$RUN_XZ -k $RANDOM_FILE_PATH
>   	elif [ "$2" = "xzonly" ]; then
> -		xz -9 -C crc32 $RANDOM_FILE_PATH
> +		$RUN_XZ $RANDOM_FILE_PATH
>   	fi
>   	config_set_name $RANDOM_FILE
>   	config_trigger_async
> @@ -501,7 +503,7 @@ test_request_partial_firmware_into_buf_nofile 2 10
>   test "$HAS_FW_LOADER_COMPRESS" != "yes" && exit 0
>   
>   # test with both files present
> -xz -9 -C crc32 -k $FW
> +$RUN_XZ -k $FW
>   config_set_name $NAME
>   echo
>   echo "Testing with both plain and xz files present..."
> 

Thank you. Looks good to me.

Reviewed-by: Shuah Khan <skhan@linuxfoundation.org>

thanks,
-- Shuah

Re: [PATCH 3/5] selftests: firmware: Fix the request_firmware_into_buf() test for XZ format

[Shuah Khan]

On 4/21/22 9:29 AM, Takashi Iwai wrote:
> The test uses a different firmware name, and we forgot to adapt for
> the XZ compressed file tests.
> 
> Fixes: 1798045900b7 ("selftests: firmware: Add request_firmware_into_buf tests")
> https://lore.kernel.org/all/20210127154939.13288-1-tiwai@suse.de/
> Signed-off-by: Takashi Iwai <tiwai@suse.de>
> ---
>   tools/testing/selftests/firmware/fw_filesystem.sh | 2 ++
>   1 file changed, 2 insertions(+)
> 
> diff --git a/tools/testing/selftests/firmware/fw_filesystem.sh b/tools/testing/selftests/firmware/fw_filesystem.sh
> index 731f011def78..3ac09b401a83 100755
> --- a/tools/testing/selftests/firmware/fw_filesystem.sh
> +++ b/tools/testing/selftests/firmware/fw_filesystem.sh
> @@ -504,6 +504,7 @@ test "$HAS_FW_LOADER_COMPRESS" != "yes" && exit 0
>   
>   # test with both files present
>   $RUN_XZ -k $FW
> +$RUN_XZ -k $FW_INTO_BUF
>   config_set_name $NAME
>   echo
>   echo "Testing with both plain and xz files present..."
> @@ -529,6 +530,7 @@ done
>   
>   # test with only xz file present
>   mv "$FW" "${FW}-orig"
> +mv "$FW_INTO_BUF" "${FW_INTO_BUF}-orig"
>   echo
>   echo "Testing with only xz file present..."
>   for i in $(seq 1 5); do
> 

Thank you. Looks good to me.

Reviewed-by: Shuah Khan <skhan@linuxfoundation.org>

thanks,
-- Shuah

Re: [PATCH 4/5] selftests: firmware: Simplify test patterns

[Shuah Khan]

On 4/21/22 9:29 AM, Takashi Iwai wrote:
> The test patterns are almost same in three sequential tests.
> Make the unified helper function for improving the readability.
> 
> Link: https://lore.kernel.org/all/20210127154939.13288-1-tiwai@suse.de/
> Signed-off-by: Takashi Iwai <tiwai@suse.de>
> ---

Thank you. It helps with maintaining this code easier.

Reviewed-by: Shuah Khan <skhan@linuxfoundation.org>

thanks,
-- Shuah

Re: [PATCH 5/5] selftests: firmware: Add ZSTD compressed file tests

[Shuah Khan]

On 4/21/22 9:29 AM, Takashi Iwai wrote:
> It's similar like XZ compressed files.  For the simplicity, both XZ
> and ZSTD tests are done in a single function.  The format is specified
> via $COMPRESS_FORMAT and the compression function is pre-defined.
> 
> Link: https://lore.kernel.org/r/20210127154939.13288-5-tiwai@suse.de
> Signed-off-by: Takashi Iwai <tiwai@suse.de>
> ---
>   .../selftests/firmware/fw_filesystem.sh       | 76 ++++++++++++++-----
>   tools/testing/selftests/firmware/fw_lib.sh    | 12 ++-
>   2 files changed, 65 insertions(+), 23 deletions(-)
> 

Thank you.

Reviewed-by: Shuah Khan <skhan@linuxfoundation.org>

thanks,
-- Shuah