LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
* [PATCH 0/5] firmware: Add support for loading compressed files
@ 2019-05-20  9:26 Takashi Iwai
  2019-05-20  9:26 ` [PATCH 1/5] firmware: Free temporary page table after vmapping Takashi Iwai
                   ` (7 more replies)
  0 siblings, 8 replies; 20+ messages in thread
From: Takashi Iwai @ 2019-05-20  9:26 UTC (permalink / raw)
  To: Luis Chamberlain; +Cc: Greg Kroah-Hartman, Rafael J . Wysocki, linux-kernel

Hi,

this is a patch set to add the support for loading compressed firmware
files.

The primary motivation is to reduce the storage size; e.g. currently
the amount of /lib/firmware on my machine counts up to 419MB, and this
can be reduced to 130MB file compression.  No bad deal.

The feature adds only fallback to the compressed file, so it should
work as it was as long as the normal firmware file is present.  The
f/w loader decompresses the content, so that there is no change needed
in the caller side.

Currently only XZ format is supported.  A caveat is that the kernel XZ
helper code supports only CRC32 (or none) integrity check type, so
you'll have to compress the files via xz -C crc32 option.

The patch set begins with a few other improvements and refactoring,
followed by the compression support.

In addition to this, dracut needs a small fix to deal with the *.xz
files.

Also, the latest patchset is found in topic/fw-decompress branch of my
sound.git tree:
  git://git.kernel.org/pub/scm/linux/kernel/git/tiwai/sound.git


thanks,

Takashi

===

Takashi Iwai (5):
  firmware: Free temporary page table after vmapping
  firmware: Unify the paged buffer release helper
  firmware: Use kvmalloc for page tables
  firmware: Factor out the paged buffer handling code
  firmware: Add support for loading compressed files

 drivers/base/firmware_loader/Kconfig    |  18 +++
 drivers/base/firmware_loader/fallback.c |  63 ++--------
 drivers/base/firmware_loader/firmware.h |  16 ++-
 drivers/base/firmware_loader/main.c     | 212 +++++++++++++++++++++++++++++---
 4 files changed, 235 insertions(+), 74 deletions(-)

-- 
2.16.4


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

* [PATCH 1/5] firmware: Free temporary page table after vmapping
  2019-05-20  9:26 [PATCH 0/5] firmware: Add support for loading compressed files Takashi Iwai
@ 2019-05-20  9:26 ` Takashi Iwai
  2019-05-20  9:26 ` [PATCH 2/5] firmware: Unify the paged buffer release helper Takashi Iwai
                   ` (6 subsequent siblings)
  7 siblings, 0 replies; 20+ messages in thread
From: Takashi Iwai @ 2019-05-20  9:26 UTC (permalink / raw)
  To: Luis Chamberlain; +Cc: Greg Kroah-Hartman, Rafael J . Wysocki, linux-kernel

Once after performing vmap() to map the S/G pages, our own page table
becomes superfluous since the pages can be released via vfree()
automatically.  Let's change the buffer release code and discard the
page table array for saving some memory.

Signed-off-by: Takashi Iwai <tiwai@suse.de>
---
 drivers/base/firmware_loader/fallback.c | 7 ++++++-
 drivers/base/firmware_loader/main.c     | 6 +++---
 2 files changed, 9 insertions(+), 4 deletions(-)

diff --git a/drivers/base/firmware_loader/fallback.c b/drivers/base/firmware_loader/fallback.c
index f962488546b6..a0a1856aac84 100644
--- a/drivers/base/firmware_loader/fallback.c
+++ b/drivers/base/firmware_loader/fallback.c
@@ -222,7 +222,7 @@ static ssize_t firmware_loading_show(struct device *dev,
 /* one pages buffer should be mapped/unmapped only once */
 static int map_fw_priv_pages(struct fw_priv *fw_priv)
 {
-	if (!fw_priv->is_paged_buf)
+	if (!fw_priv->pages)
 		return 0;
 
 	vunmap(fw_priv->data);
@@ -230,6 +230,11 @@ static int map_fw_priv_pages(struct fw_priv *fw_priv)
 			     PAGE_KERNEL_RO);
 	if (!fw_priv->data)
 		return -ENOMEM;
+
+	/* page table is no longer needed after mapping, let's free */
+	vfree(fw_priv->pages);
+	fw_priv->pages = NULL;
+
 	return 0;
 }
 
diff --git a/drivers/base/firmware_loader/main.c b/drivers/base/firmware_loader/main.c
index 7eaaf5ee5ba6..aed1a7c56713 100644
--- a/drivers/base/firmware_loader/main.c
+++ b/drivers/base/firmware_loader/main.c
@@ -252,13 +252,13 @@ static void __free_fw_priv(struct kref *ref)
 	spin_unlock(&fwc->lock);
 
 #ifdef CONFIG_FW_LOADER_USER_HELPER
-	if (fw_priv->is_paged_buf) {
+	if (fw_priv->pages) {
+		/* free leftover pages */
 		int i;
-		vunmap(fw_priv->data);
 		for (i = 0; i < fw_priv->nr_pages; i++)
 			__free_page(fw_priv->pages[i]);
 		vfree(fw_priv->pages);
-	} else
+	}
 #endif
 	if (!fw_priv->allocated_size)
 		vfree(fw_priv->data);
-- 
2.16.4


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

* [PATCH 2/5] firmware: Unify the paged buffer release helper
  2019-05-20  9:26 [PATCH 0/5] firmware: Add support for loading compressed files Takashi Iwai
  2019-05-20  9:26 ` [PATCH 1/5] firmware: Free temporary page table after vmapping Takashi Iwai
@ 2019-05-20  9:26 ` Takashi Iwai
  2019-05-20  9:26 ` [PATCH 3/5] firmware: Use kvmalloc for page tables Takashi Iwai
                   ` (5 subsequent siblings)
  7 siblings, 0 replies; 20+ messages in thread
From: Takashi Iwai @ 2019-05-20  9:26 UTC (permalink / raw)
  To: Luis Chamberlain; +Cc: Greg Kroah-Hartman, Rafael J . Wysocki, linux-kernel

Use a common helper to release the paged buffer resources.
This is rather a preparation for the upcoming decompression support.

Signed-off-by: Takashi Iwai <tiwai@suse.de>
---
 drivers/base/firmware_loader/fallback.c |  8 +-------
 drivers/base/firmware_loader/firmware.h |  6 ++++++
 drivers/base/firmware_loader/main.c     | 27 ++++++++++++++++++---------
 3 files changed, 25 insertions(+), 16 deletions(-)

diff --git a/drivers/base/firmware_loader/fallback.c b/drivers/base/firmware_loader/fallback.c
index a0a1856aac84..8970a5315e85 100644
--- a/drivers/base/firmware_loader/fallback.c
+++ b/drivers/base/firmware_loader/fallback.c
@@ -259,7 +259,6 @@ static ssize_t firmware_loading_store(struct device *dev,
 	struct fw_priv *fw_priv;
 	ssize_t written = count;
 	int loading = simple_strtol(buf, NULL, 10);
-	int i;
 
 	mutex_lock(&fw_lock);
 	fw_priv = fw_sysfs->fw_priv;
@@ -270,12 +269,7 @@ static ssize_t firmware_loading_store(struct device *dev,
 	case 1:
 		/* discarding any previous partial load */
 		if (!fw_sysfs_done(fw_priv)) {
-			for (i = 0; i < fw_priv->nr_pages; i++)
-				__free_page(fw_priv->pages[i]);
-			vfree(fw_priv->pages);
-			fw_priv->pages = NULL;
-			fw_priv->page_array_size = 0;
-			fw_priv->nr_pages = 0;
+			fw_free_paged_buf(fw_priv);
 			fw_state_start(fw_priv);
 		}
 		break;
diff --git a/drivers/base/firmware_loader/firmware.h b/drivers/base/firmware_loader/firmware.h
index 4c1395f8e7ed..d20d4e7f9e71 100644
--- a/drivers/base/firmware_loader/firmware.h
+++ b/drivers/base/firmware_loader/firmware.h
@@ -133,4 +133,10 @@ static inline void fw_state_done(struct fw_priv *fw_priv)
 int assign_fw(struct firmware *fw, struct device *device,
 	      enum fw_opt opt_flags);
 
+#ifdef CONFIG_FW_LOADER_USER_HELPER
+void fw_free_paged_buf(struct fw_priv *fw_priv);
+#else
+static inline void fw_free_paged_buf(struct fw_priv *fw_priv) {}
+#endif
+
 #endif /* __FIRMWARE_LOADER_H */
diff --git a/drivers/base/firmware_loader/main.c b/drivers/base/firmware_loader/main.c
index aed1a7c56713..083fc3e4f2fd 100644
--- a/drivers/base/firmware_loader/main.c
+++ b/drivers/base/firmware_loader/main.c
@@ -251,15 +251,7 @@ static void __free_fw_priv(struct kref *ref)
 	list_del(&fw_priv->list);
 	spin_unlock(&fwc->lock);
 
-#ifdef CONFIG_FW_LOADER_USER_HELPER
-	if (fw_priv->pages) {
-		/* free leftover pages */
-		int i;
-		for (i = 0; i < fw_priv->nr_pages; i++)
-			__free_page(fw_priv->pages[i]);
-		vfree(fw_priv->pages);
-	}
-#endif
+	fw_free_paged_buf(fw_priv); /* free leftover pages */
 	if (!fw_priv->allocated_size)
 		vfree(fw_priv->data);
 	kfree_const(fw_priv->fw_name);
@@ -274,6 +266,23 @@ static void free_fw_priv(struct fw_priv *fw_priv)
 		spin_unlock(&fwc->lock);
 }
 
+#ifdef CONFIG_FW_LOADER_USER_HELPER
+void fw_free_paged_buf(struct fw_priv *fw_priv)
+{
+	int i;
+
+	if (!fw_priv->pages)
+		return;
+
+	for (i = 0; i < fw_priv->nr_pages; i++)
+		__free_page(fw_priv->pages[i]);
+	vfree(fw_priv->pages);
+	fw_priv->pages = NULL;
+	fw_priv->page_array_size = 0;
+	fw_priv->nr_pages = 0;
+}
+#endif
+
 /* direct firmware loading support */
 static char fw_path_para[256];
 static const char * const fw_path[] = {
-- 
2.16.4


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

* [PATCH 3/5] firmware: Use kvmalloc for page tables
  2019-05-20  9:26 [PATCH 0/5] firmware: Add support for loading compressed files Takashi Iwai
  2019-05-20  9:26 ` [PATCH 1/5] firmware: Free temporary page table after vmapping Takashi Iwai
  2019-05-20  9:26 ` [PATCH 2/5] firmware: Unify the paged buffer release helper Takashi Iwai
@ 2019-05-20  9:26 ` Takashi Iwai
  2019-05-20  9:26 ` [PATCH 4/5] firmware: Factor out the paged buffer handling code Takashi Iwai
                   ` (4 subsequent siblings)
  7 siblings, 0 replies; 20+ messages in thread
From: Takashi Iwai @ 2019-05-20  9:26 UTC (permalink / raw)
  To: Luis Chamberlain; +Cc: Greg Kroah-Hartman, Rafael J . Wysocki, linux-kernel

This is a minor optimization to use kvmalloc() variant for allocating
the page table for the SG-buffer.  They aren't so big in general, so
kmalloc() would fit often better.

Signed-off-by: Takashi Iwai <tiwai@suse.de>
---
 drivers/base/firmware_loader/fallback.c | 7 ++++---
 drivers/base/firmware_loader/main.c     | 2 +-
 2 files changed, 5 insertions(+), 4 deletions(-)

diff --git a/drivers/base/firmware_loader/fallback.c b/drivers/base/firmware_loader/fallback.c
index 8970a5315e85..b5cd96fd0e77 100644
--- a/drivers/base/firmware_loader/fallback.c
+++ b/drivers/base/firmware_loader/fallback.c
@@ -232,7 +232,7 @@ static int map_fw_priv_pages(struct fw_priv *fw_priv)
 		return -ENOMEM;
 
 	/* page table is no longer needed after mapping, let's free */
-	vfree(fw_priv->pages);
+	kvfree(fw_priv->pages);
 	fw_priv->pages = NULL;
 
 	return 0;
@@ -397,7 +397,8 @@ static int fw_realloc_pages(struct fw_sysfs *fw_sysfs, int min_size)
 					 fw_priv->page_array_size * 2);
 		struct page **new_pages;
 
-		new_pages = vmalloc(array_size(new_array_size, sizeof(void *)));
+		new_pages = kvmalloc_array(new_array_size, sizeof(void *),
+					   GFP_KERNEL);
 		if (!new_pages) {
 			fw_load_abort(fw_sysfs);
 			return -ENOMEM;
@@ -406,7 +407,7 @@ static int fw_realloc_pages(struct fw_sysfs *fw_sysfs, int min_size)
 		       fw_priv->page_array_size * sizeof(void *));
 		memset(&new_pages[fw_priv->page_array_size], 0, sizeof(void *) *
 		       (new_array_size - fw_priv->page_array_size));
-		vfree(fw_priv->pages);
+		kvfree(fw_priv->pages);
 		fw_priv->pages = new_pages;
 		fw_priv->page_array_size = new_array_size;
 	}
diff --git a/drivers/base/firmware_loader/main.c b/drivers/base/firmware_loader/main.c
index 083fc3e4f2fd..2e74a1b73dae 100644
--- a/drivers/base/firmware_loader/main.c
+++ b/drivers/base/firmware_loader/main.c
@@ -276,7 +276,7 @@ void fw_free_paged_buf(struct fw_priv *fw_priv)
 
 	for (i = 0; i < fw_priv->nr_pages; i++)
 		__free_page(fw_priv->pages[i]);
-	vfree(fw_priv->pages);
+	kvfree(fw_priv->pages);
 	fw_priv->pages = NULL;
 	fw_priv->page_array_size = 0;
 	fw_priv->nr_pages = 0;
-- 
2.16.4


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

* [PATCH 4/5] firmware: Factor out the paged buffer handling code
  2019-05-20  9:26 [PATCH 0/5] firmware: Add support for loading compressed files Takashi Iwai
                   ` (2 preceding siblings ...)
  2019-05-20  9:26 ` [PATCH 3/5] firmware: Use kvmalloc for page tables Takashi Iwai
@ 2019-05-20  9:26 ` Takashi Iwai
  2019-05-20  9:26 ` [PATCH 5/5] firmware: Add support for loading compressed files Takashi Iwai
                   ` (3 subsequent siblings)
  7 siblings, 0 replies; 20+ messages in thread
From: Takashi Iwai @ 2019-05-20  9:26 UTC (permalink / raw)
  To: Luis Chamberlain; +Cc: Greg Kroah-Hartman, Rafael J . Wysocki, linux-kernel

This is merely a preparation for the upcoming compressed firmware
support and no functional changes.  It moves the code to handle the
paged buffer allocation and mapping out of fallback.c into the main
code, so that they can be used commonly.

Signed-off-by: Takashi Iwai <tiwai@suse.de>
---
 drivers/base/firmware_loader/fallback.c | 61 ++++-----------------------------
 drivers/base/firmware_loader/firmware.h |  4 +++
 drivers/base/firmware_loader/main.c     | 52 ++++++++++++++++++++++++++++
 3 files changed, 63 insertions(+), 54 deletions(-)

diff --git a/drivers/base/firmware_loader/fallback.c b/drivers/base/firmware_loader/fallback.c
index b5cd96fd0e77..80b20f6c494f 100644
--- a/drivers/base/firmware_loader/fallback.c
+++ b/drivers/base/firmware_loader/fallback.c
@@ -219,25 +219,6 @@ static ssize_t firmware_loading_show(struct device *dev,
 	return sprintf(buf, "%d\n", loading);
 }
 
-/* one pages buffer should be mapped/unmapped only once */
-static int map_fw_priv_pages(struct fw_priv *fw_priv)
-{
-	if (!fw_priv->pages)
-		return 0;
-
-	vunmap(fw_priv->data);
-	fw_priv->data = vmap(fw_priv->pages, fw_priv->nr_pages, 0,
-			     PAGE_KERNEL_RO);
-	if (!fw_priv->data)
-		return -ENOMEM;
-
-	/* page table is no longer needed after mapping, let's free */
-	kvfree(fw_priv->pages);
-	fw_priv->pages = NULL;
-
-	return 0;
-}
-
 /**
  * firmware_loading_store() - set value in the 'loading' control file
  * @dev: device pointer
@@ -283,7 +264,7 @@ static ssize_t firmware_loading_store(struct device *dev,
 			 * see the mapped 'buf->data' once the loading
 			 * is completed.
 			 * */
-			rc = map_fw_priv_pages(fw_priv);
+			rc = fw_map_paged_buf(fw_priv);
 			if (rc)
 				dev_err(dev, "%s: map pages failed\n",
 					__func__);
@@ -388,41 +369,13 @@ static ssize_t firmware_data_read(struct file *filp, struct kobject *kobj,
 
 static int fw_realloc_pages(struct fw_sysfs *fw_sysfs, int min_size)
 {
-	struct fw_priv *fw_priv= fw_sysfs->fw_priv;
-	int pages_needed = PAGE_ALIGN(min_size) >> PAGE_SHIFT;
-
-	/* If the array of pages is too small, grow it... */
-	if (fw_priv->page_array_size < pages_needed) {
-		int new_array_size = max(pages_needed,
-					 fw_priv->page_array_size * 2);
-		struct page **new_pages;
+	int err;
 
-		new_pages = kvmalloc_array(new_array_size, sizeof(void *),
-					   GFP_KERNEL);
-		if (!new_pages) {
-			fw_load_abort(fw_sysfs);
-			return -ENOMEM;
-		}
-		memcpy(new_pages, fw_priv->pages,
-		       fw_priv->page_array_size * sizeof(void *));
-		memset(&new_pages[fw_priv->page_array_size], 0, sizeof(void *) *
-		       (new_array_size - fw_priv->page_array_size));
-		kvfree(fw_priv->pages);
-		fw_priv->pages = new_pages;
-		fw_priv->page_array_size = new_array_size;
-	}
-
-	while (fw_priv->nr_pages < pages_needed) {
-		fw_priv->pages[fw_priv->nr_pages] =
-			alloc_page(GFP_KERNEL | __GFP_HIGHMEM);
-
-		if (!fw_priv->pages[fw_priv->nr_pages]) {
-			fw_load_abort(fw_sysfs);
-			return -ENOMEM;
-		}
-		fw_priv->nr_pages++;
-	}
-	return 0;
+	err = fw_grow_paged_buf(fw_sysfs->fw_priv,
+				PAGE_ALIGN(min_size) >> PAGE_SHIFT);
+	if (err)
+		fw_load_abort(fw_sysfs);
+	return err;
 }
 
 /**
diff --git a/drivers/base/firmware_loader/firmware.h b/drivers/base/firmware_loader/firmware.h
index d20d4e7f9e71..35f4e58b2d98 100644
--- a/drivers/base/firmware_loader/firmware.h
+++ b/drivers/base/firmware_loader/firmware.h
@@ -135,8 +135,12 @@ int assign_fw(struct firmware *fw, struct device *device,
 
 #ifdef CONFIG_FW_LOADER_USER_HELPER
 void fw_free_paged_buf(struct fw_priv *fw_priv);
+int fw_grow_paged_buf(struct fw_priv *fw_priv, int pages_needed);
+int fw_map_paged_buf(struct fw_priv *fw_priv);
 #else
 static inline void fw_free_paged_buf(struct fw_priv *fw_priv) {}
+int fw_grow_paged_buf(struct fw_priv *fw_priv, int pages_needed) { return -ENXIO; }
+int fw_map_paged_buf(struct fw_priv *fw_priv) { return -ENXIO; }
 #endif
 
 #endif /* __FIRMWARE_LOADER_H */
diff --git a/drivers/base/firmware_loader/main.c b/drivers/base/firmware_loader/main.c
index 2e74a1b73dae..7e12732f4705 100644
--- a/drivers/base/firmware_loader/main.c
+++ b/drivers/base/firmware_loader/main.c
@@ -281,6 +281,58 @@ void fw_free_paged_buf(struct fw_priv *fw_priv)
 	fw_priv->page_array_size = 0;
 	fw_priv->nr_pages = 0;
 }
+
+int fw_grow_paged_buf(struct fw_priv *fw_priv, int pages_needed)
+{
+	/* If the array of pages is too small, grow it */
+	if (fw_priv->page_array_size < pages_needed) {
+		int new_array_size = max(pages_needed,
+					 fw_priv->page_array_size * 2);
+		struct page **new_pages;
+
+		new_pages = kvmalloc_array(new_array_size, sizeof(void *),
+					   GFP_KERNEL);
+		if (!new_pages)
+			return -ENOMEM;
+		memcpy(new_pages, fw_priv->pages,
+		       fw_priv->page_array_size * sizeof(void *));
+		memset(&new_pages[fw_priv->page_array_size], 0, sizeof(void *) *
+		       (new_array_size - fw_priv->page_array_size));
+		kvfree(fw_priv->pages);
+		fw_priv->pages = new_pages;
+		fw_priv->page_array_size = new_array_size;
+	}
+
+	while (fw_priv->nr_pages < pages_needed) {
+		fw_priv->pages[fw_priv->nr_pages] =
+			alloc_page(GFP_KERNEL | __GFP_HIGHMEM);
+
+		if (!fw_priv->pages[fw_priv->nr_pages])
+			return -ENOMEM;
+		fw_priv->nr_pages++;
+	}
+
+	return 0;
+}
+
+int fw_map_paged_buf(struct fw_priv *fw_priv)
+{
+	/* one pages buffer should be mapped/unmapped only once */
+	if (!fw_priv->pages)
+		return 0;
+
+	vunmap(fw_priv->data);
+	fw_priv->data = vmap(fw_priv->pages, fw_priv->nr_pages, 0,
+			     PAGE_KERNEL_RO);
+	if (!fw_priv->data)
+		return -ENOMEM;
+
+	/* page table is no longer needed after mapping, let's free */
+	kvfree(fw_priv->pages);
+	fw_priv->pages = NULL;
+
+	return 0;
+}
 #endif
 
 /* direct firmware loading support */
-- 
2.16.4


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

* [PATCH 5/5] firmware: Add support for loading compressed files
  2019-05-20  9:26 [PATCH 0/5] firmware: Add support for loading compressed files Takashi Iwai
                   ` (3 preceding siblings ...)
  2019-05-20  9:26 ` [PATCH 4/5] firmware: Factor out the paged buffer handling code Takashi Iwai
@ 2019-05-20  9:26 ` Takashi Iwai
  2019-05-20  9:39 ` [PATCH 0/5] " Greg Kroah-Hartman
                   ` (2 subsequent siblings)
  7 siblings, 0 replies; 20+ messages in thread
From: Takashi Iwai @ 2019-05-20  9:26 UTC (permalink / raw)
  To: Luis Chamberlain; +Cc: Greg Kroah-Hartman, Rafael J . Wysocki, linux-kernel

This patch adds the support for loading compressed firmware files.
The primary motivation is to reduce the storage size; e.g. currently
the files in /lib/firmware on my machine counts up to 419MB, while
they can be reduced to 130MB by file compression.

The patch introduces a new kconfig option CONFIG_FW_LOADER_COMPRESS.
Even with this option set, the firmware loader still tries to load the
original firmware file as-is at first, but then falls back to the file
with ".xz" extension when it's not found, and the decompressed file
content is returned to the caller of request_firmware().  So, no
change is needed for the rest.

Currently only XZ format is supported.  A caveat is that the kernel XZ
helper code supports only CRC32 (or none) integrity check type, so
you'll have to compress the files via xz -C crc32 option.

Since we can't determine the expanded size immediately, the patch
re-uses the paged buffer that was used for the user-mode fallback, and
puts the decompressed content page by page as well as a new kconfig for
enabling the paged buffer support.

Signed-off-by: Takashi Iwai <tiwai@suse.de>
---
 drivers/base/firmware_loader/Kconfig    |  18 +++++
 drivers/base/firmware_loader/firmware.h |   8 +-
 drivers/base/firmware_loader/main.c     | 135 +++++++++++++++++++++++++++++---
 3 files changed, 149 insertions(+), 12 deletions(-)

diff --git a/drivers/base/firmware_loader/Kconfig b/drivers/base/firmware_loader/Kconfig
index 38f2da6f5c2b..cc86d51d2999 100644
--- a/drivers/base/firmware_loader/Kconfig
+++ b/drivers/base/firmware_loader/Kconfig
@@ -26,6 +26,9 @@ config FW_LOADER
 
 if FW_LOADER
 
+config FW_LOADER_PAGED_BUF
+	bool
+
 config EXTRA_FIRMWARE
 	string "Build named firmware blobs into the kernel binary"
 	help
@@ -67,6 +70,7 @@ config EXTRA_FIRMWARE_DIR
 
 config FW_LOADER_USER_HELPER
 	bool "Enable the firmware sysfs fallback mechanism"
+	select FW_LOADER_PAGED_BUF
 	help
 	  This option enables a sysfs loading facility to enable firmware
 	  loading to the kernel through userspace as a fallback mechanism
@@ -151,5 +155,19 @@ config FW_LOADER_USER_HELPER_FALLBACK
 
 	  If you are unsure about this, say N here.
 
+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
+	  trying to load the raw file at first.
+
+	  Currently only XZ-compressed files are supported, and they have to
+	  be compressed with either none or crc32 integrity check type (the
+	  -C crc32 option of xz command).
+
 endif # FW_LOADER
 endmenu
diff --git a/drivers/base/firmware_loader/firmware.h b/drivers/base/firmware_loader/firmware.h
index 35f4e58b2d98..7048a41973ed 100644
--- a/drivers/base/firmware_loader/firmware.h
+++ b/drivers/base/firmware_loader/firmware.h
@@ -64,12 +64,14 @@ struct fw_priv {
 	void *data;
 	size_t size;
 	size_t allocated_size;
-#ifdef CONFIG_FW_LOADER_USER_HELPER
+#ifdef CONFIG_FW_LOADER_PAGED_BUF
 	bool is_paged_buf;
-	bool need_uevent;
 	struct page **pages;
 	int nr_pages;
 	int page_array_size;
+#endif
+#ifdef CONFIG_FW_LOADER_USER_HELPER
+	bool need_uevent;
 	struct list_head pending_list;
 #endif
 	const char *fw_name;
@@ -133,7 +135,7 @@ static inline void fw_state_done(struct fw_priv *fw_priv)
 int assign_fw(struct firmware *fw, struct device *device,
 	      enum fw_opt opt_flags);
 
-#ifdef CONFIG_FW_LOADER_USER_HELPER
+#ifdef CONFIG_FW_LOADER_PAGED_BUF
 void fw_free_paged_buf(struct fw_priv *fw_priv);
 int fw_grow_paged_buf(struct fw_priv *fw_priv, int pages_needed);
 int fw_map_paged_buf(struct fw_priv *fw_priv);
diff --git a/drivers/base/firmware_loader/main.c b/drivers/base/firmware_loader/main.c
index 7e12732f4705..b070d6b4f4f1 100644
--- a/drivers/base/firmware_loader/main.c
+++ b/drivers/base/firmware_loader/main.c
@@ -33,6 +33,7 @@
 #include <linux/syscore_ops.h>
 #include <linux/reboot.h>
 #include <linux/security.h>
+#include <linux/xz.h>
 
 #include <generated/utsrelease.h>
 
@@ -266,7 +267,7 @@ static void free_fw_priv(struct fw_priv *fw_priv)
 		spin_unlock(&fwc->lock);
 }
 
-#ifdef CONFIG_FW_LOADER_USER_HELPER
+#ifdef CONFIG_FW_LOADER_PAGED_BUF
 void fw_free_paged_buf(struct fw_priv *fw_priv)
 {
 	int i;
@@ -335,6 +336,98 @@ int fw_map_paged_buf(struct fw_priv *fw_priv)
 }
 #endif
 
+/*
+ * compressed firmware support
+ */
+#ifdef CONFIG_FW_LOADER_COMPRESS
+/* single-shot decompression onto the pre-allocated buffer */
+static enum xz_ret fw_decompress_single(struct fw_priv *fw_priv,
+					struct xz_dec *xz_dec,
+					struct xz_buf *xz_buf)
+{
+	enum xz_ret xz_ret;
+
+	xz_buf->out_pos = 0;
+	xz_buf->out = fw_priv->data;
+	xz_buf->out_size = fw_priv->allocated_size;
+	xz_ret = xz_dec_run(xz_dec, xz_buf);
+	fw_priv->size = xz_buf->out_pos;
+	return xz_ret;
+}
+
+/* decompression on paged buffer and map it */
+static enum xz_ret fw_decompress_pages(struct fw_priv *fw_priv,
+				       struct xz_dec *xz_dec,
+				       struct xz_buf *xz_buf)
+{
+	struct page *page;
+	enum xz_ret xz_ret;
+
+	fw_priv->is_paged_buf = true;
+	fw_priv->size = 0;
+	do {
+		if (fw_grow_paged_buf(fw_priv, fw_priv->nr_pages + 1))
+			return XZ_MEM_ERROR;
+
+		page = fw_priv->pages[fw_priv->nr_pages - 1];
+		xz_buf->out = kmap(page);
+		xz_buf->out_pos = 0;
+		xz_buf->out_size = PAGE_SIZE;
+		xz_ret = xz_dec_run(xz_dec, xz_buf);
+		kunmap(page);
+		fw_priv->size += xz_buf->out_pos;
+	} while (xz_ret == XZ_OK);
+
+	if (xz_ret == XZ_STREAM_END) {
+		if (fw_map_paged_buf(fw_priv))
+			return XZ_MEM_ERROR;
+	}
+
+	return xz_ret;
+}
+
+static int fw_decompress_buffer(struct device *dev, struct fw_priv *fw_priv,
+				size_t in_size, const void *in_buffer)
+{
+	struct xz_dec *xz_dec;
+	struct xz_buf xz_buf;
+	enum xz_ret xz_ret;
+	bool singleshot = false;
+
+	/* if the buffer is pre-allocaed, we can perform in single-shot mode */
+	if (fw_priv->data)
+		singleshot = true;
+
+	xz_dec = xz_dec_init(singleshot ? XZ_SINGLE : XZ_DYNALLOC, (u32)-1);
+	if (!xz_dec) {
+		dev_warn(dev, "f/w decompression init failed\n");
+		return -ENOMEM;
+	}
+
+	xz_buf.in_size = in_size;
+	xz_buf.in = in_buffer;
+	xz_buf.in_pos = 0;
+	if (singleshot)
+		xz_ret = fw_decompress_single(fw_priv, xz_dec, &xz_buf);
+	else
+		xz_ret = fw_decompress_pages(fw_priv, xz_dec, &xz_buf);
+	xz_dec_end(xz_dec);
+
+	if (xz_ret != XZ_STREAM_END) {
+		dev_warn(dev, "f/w decompression failed (xz_ret=%d)\n", xz_ret);
+		return xz_ret == XZ_MEM_ERROR ? -ENOMEM : -EINVAL;
+	}
+
+	return 0;
+}
+#else /* CONFIG_FW_LOADER_COMPRESS */
+static inline int fw_decompress_buffer(struct device *dev,  struct fw_priv *fw,
+				       size_t in_size, const void *in_buffer)
+{
+	return -ENXIO;
+}
+#endif /* CONFIG_FW_LOADER_COMPRESS */
+
 /* direct firmware loading support */
 static char fw_path_para[256];
 static const char * const fw_path[] = {
@@ -354,7 +447,8 @@ module_param_string(path, fw_path_para, sizeof(fw_path_para), 0644);
 MODULE_PARM_DESC(path, "customized firmware image search path with a higher priority than default path");
 
 static int
-fw_get_filesystem_firmware(struct device *device, struct fw_priv *fw_priv)
+fw_get_filesystem_firmware(struct device *device, struct fw_priv *fw_priv,
+			   bool decompress)
 {
 	loff_t size;
 	int i, len;
@@ -362,9 +456,12 @@ fw_get_filesystem_firmware(struct device *device, struct fw_priv *fw_priv)
 	char *path;
 	enum kernel_read_file_id id = READING_FIRMWARE;
 	size_t msize = INT_MAX;
+	void *buffer = NULL;
+	const char *suffix = decompress ? ".xz" : "";
 
 	/* Already populated data member means we're loading into a buffer */
-	if (fw_priv->data) {
+	if (!decompress && fw_priv->data) {
+		buffer = fw_priv->data;
 		id = READING_FIRMWARE_PREALLOC_BUFFER;
 		msize = fw_priv->allocated_size;
 	}
@@ -378,15 +475,15 @@ fw_get_filesystem_firmware(struct device *device, struct fw_priv *fw_priv)
 		if (!fw_path[i][0])
 			continue;
 
-		len = snprintf(path, PATH_MAX, "%s/%s",
-			       fw_path[i], fw_priv->fw_name);
+		len = snprintf(path, PATH_MAX, "%s/%s%s",
+			       fw_path[i], fw_priv->fw_name, suffix);
 		if (len >= PATH_MAX) {
 			rc = -ENAMETOOLONG;
 			break;
 		}
 
 		fw_priv->size = 0;
-		rc = kernel_read_file_from_path(path, &fw_priv->data, &size,
+		rc = kernel_read_file_from_path(path, &buffer, &size,
 						msize, id);
 		if (rc) {
 			if (rc != -ENOENT)
@@ -397,8 +494,25 @@ fw_get_filesystem_firmware(struct device *device, struct fw_priv *fw_priv)
 					 path);
 			continue;
 		}
-		dev_dbg(device, "direct-loading %s\n", fw_priv->fw_name);
-		fw_priv->size = size;
+		if (decompress) {
+			dev_dbg(device, "f/w decompressing %s\n",
+				fw_priv->fw_name);
+			rc = fw_decompress_buffer(device, fw_priv, size,
+						  buffer);
+			/* discard the superfluous original content */
+			vfree(buffer);
+			buffer = NULL;
+			if (rc) {
+				fw_free_paged_buf(fw_priv);
+				continue;
+			}
+		} else {
+			dev_dbg(device, "direct-loading %s\n",
+				fw_priv->fw_name);
+			if (!fw_priv->data)
+				fw_priv->data = buffer;
+			fw_priv->size = size;
+		}
 		fw_state_done(fw_priv);
 		break;
 	}
@@ -645,7 +759,10 @@ _request_firmware(const struct firmware **firmware_p, const char *name,
 	if (ret <= 0) /* error or already assigned */
 		goto out;
 
-	ret = fw_get_filesystem_firmware(device, fw->priv);
+	ret = fw_get_filesystem_firmware(device, fw->priv, false);
+	if (IS_ENABLED(CONFIG_FW_LOADER_COMPRESS) && ret == -ENOENT)
+		ret = fw_get_filesystem_firmware(device, fw->priv, true);
+
 	if (ret) {
 		if (!(opt_flags & FW_OPT_NO_WARN))
 			dev_warn(device,
-- 
2.16.4


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

* Re: [PATCH 0/5] firmware: Add support for loading compressed files
  2019-05-20  9:26 [PATCH 0/5] firmware: Add support for loading compressed files Takashi Iwai
                   ` (4 preceding siblings ...)
  2019-05-20  9:26 ` [PATCH 5/5] firmware: Add support for loading compressed files Takashi Iwai
@ 2019-05-20  9:39 ` Greg Kroah-Hartman
  2019-05-20  9:56   ` Takashi Iwai
  2019-05-20 14:39   ` Takashi Iwai
  2019-05-28  5:25 ` Takashi Iwai
  2019-06-10 17:21 ` Greg Kroah-Hartman
  7 siblings, 2 replies; 20+ messages in thread
From: Greg Kroah-Hartman @ 2019-05-20  9:39 UTC (permalink / raw)
  To: Takashi Iwai; +Cc: Luis Chamberlain, Rafael J . Wysocki, linux-kernel

On Mon, May 20, 2019 at 11:26:42AM +0200, Takashi Iwai wrote:
> Hi,
> 
> this is a patch set to add the support for loading compressed firmware
> files.
> 
> The primary motivation is to reduce the storage size; e.g. currently
> the amount of /lib/firmware on my machine counts up to 419MB, and this
> can be reduced to 130MB file compression.  No bad deal.
> 
> The feature adds only fallback to the compressed file, so it should
> work as it was as long as the normal firmware file is present.  The
> f/w loader decompresses the content, so that there is no change needed
> in the caller side.
> 
> Currently only XZ format is supported.  A caveat is that the kernel XZ
> helper code supports only CRC32 (or none) integrity check type, so
> you'll have to compress the files via xz -C crc32 option.
> 
> The patch set begins with a few other improvements and refactoring,
> followed by the compression support.
> 
> In addition to this, dracut needs a small fix to deal with the *.xz
> files.
> 
> Also, the latest patchset is found in topic/fw-decompress branch of my
> sound.git tree:
>   git://git.kernel.org/pub/scm/linux/kernel/git/tiwai/sound.git

After a quick review, these all look good to me, nice job.

One recommendation, can we add support for testing this to the
tools/testing/selftests/firmware/ tests?  And you did run those
regression tests to verify that you didn't get any of the config options
messed up, right? :)

thanks,

greg k-h

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

* Re: [PATCH 0/5] firmware: Add support for loading compressed files
  2019-05-20  9:39 ` [PATCH 0/5] " Greg Kroah-Hartman
@ 2019-05-20  9:56   ` Takashi Iwai
  2019-05-21  5:30     ` Takashi Iwai
  2019-05-20 14:39   ` Takashi Iwai
  1 sibling, 1 reply; 20+ messages in thread
From: Takashi Iwai @ 2019-05-20  9:56 UTC (permalink / raw)
  To: Greg Kroah-Hartman; +Cc: Luis Chamberlain, Rafael J . Wysocki, linux-kernel

On Mon, 20 May 2019 11:39:29 +0200,
Greg Kroah-Hartman wrote:
> 
> On Mon, May 20, 2019 at 11:26:42AM +0200, Takashi Iwai wrote:
> > Hi,
> > 
> > this is a patch set to add the support for loading compressed firmware
> > files.
> > 
> > The primary motivation is to reduce the storage size; e.g. currently
> > the amount of /lib/firmware on my machine counts up to 419MB, and this
> > can be reduced to 130MB file compression.  No bad deal.
> > 
> > The feature adds only fallback to the compressed file, so it should
> > work as it was as long as the normal firmware file is present.  The
> > f/w loader decompresses the content, so that there is no change needed
> > in the caller side.
> > 
> > Currently only XZ format is supported.  A caveat is that the kernel XZ
> > helper code supports only CRC32 (or none) integrity check type, so
> > you'll have to compress the files via xz -C crc32 option.
> > 
> > The patch set begins with a few other improvements and refactoring,
> > followed by the compression support.
> > 
> > In addition to this, dracut needs a small fix to deal with the *.xz
> > files.
> > 
> > Also, the latest patchset is found in topic/fw-decompress branch of my
> > sound.git tree:
> >   git://git.kernel.org/pub/scm/linux/kernel/git/tiwai/sound.git
> 
> After a quick review, these all look good to me, nice job.
> 
> One recommendation, can we add support for testing this to the
> tools/testing/selftests/firmware/ tests?  And you did run those
> regression tests to verify that you didn't get any of the config options
> messed up, right? :)

Oh, do you believe I'm a so modern person who lets computer working on
everything? ;)  I only tested manually, so far, this will be my
homework today.

Thanks for pointing to it!


Takashi

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

* Re: [PATCH 0/5] firmware: Add support for loading compressed files
  2019-05-20  9:39 ` [PATCH 0/5] " Greg Kroah-Hartman
  2019-05-20  9:56   ` Takashi Iwai
@ 2019-05-20 14:39   ` Takashi Iwai
  2019-05-20 15:18     ` Takashi Iwai
  1 sibling, 1 reply; 20+ messages in thread
From: Takashi Iwai @ 2019-05-20 14:39 UTC (permalink / raw)
  To: Greg Kroah-Hartman; +Cc: Luis Chamberlain, Rafael J . Wysocki, linux-kernel

On Mon, 20 May 2019 11:39:29 +0200,
Greg Kroah-Hartman wrote:
> 
> On Mon, May 20, 2019 at 11:26:42AM +0200, Takashi Iwai wrote:
> > Hi,
> > 
> > this is a patch set to add the support for loading compressed firmware
> > files.
> > 
> > The primary motivation is to reduce the storage size; e.g. currently
> > the amount of /lib/firmware on my machine counts up to 419MB, and this
> > can be reduced to 130MB file compression.  No bad deal.
> > 
> > The feature adds only fallback to the compressed file, so it should
> > work as it was as long as the normal firmware file is present.  The
> > f/w loader decompresses the content, so that there is no change needed
> > in the caller side.
> > 
> > Currently only XZ format is supported.  A caveat is that the kernel XZ
> > helper code supports only CRC32 (or none) integrity check type, so
> > you'll have to compress the files via xz -C crc32 option.
> > 
> > The patch set begins with a few other improvements and refactoring,
> > followed by the compression support.
> > 
> > In addition to this, dracut needs a small fix to deal with the *.xz
> > files.
> > 
> > Also, the latest patchset is found in topic/fw-decompress branch of my
> > sound.git tree:
> >   git://git.kernel.org/pub/scm/linux/kernel/git/tiwai/sound.git
> 
> After a quick review, these all look good to me, nice job.
> 
> One recommendation, can we add support for testing this to the
> tools/testing/selftests/firmware/ tests?  And you did run those
> regression tests to verify that you didn't get any of the config options
> messed up, right? :)

Now I've been testing the firmware selftest, and this turned out to be
surprisingly difficult on my system.  By some reason, the test always
fails at the point triggering the request (line 58 of
fw_filesystem.sh):

  if ! echo -n "$NAME" >"$DIR"/trigger_request ; then
	....

Judging from the strace output, this echo writes only the first byte
of $NAME.  Then kernfs write op is invoked and it deals this one byte
input as if a whole argument were passed, leading to an error.

My temporary workaround was to replace the all "echo" call with
"/usr/bin/echo".

Then it hits a similar write error at the places like:

	echo 1 > $DIR/config_sync_direct

This could be worked around by adding -n option to echo.

Finally, I noticed that the user-fallback doesn't work on my system
any longer and the test stopped.  This is expected, so it implies that
all direct loading tests passed.

FWIW, my system is openSUSE Leap 15.1.  Does anyone experience a
similar problem?


BTW, about adding the new tests for the compressed file support.
Would we want to add a new proc entry for toggling the behavior, like
other tested stuff?  Or just check CONFIG_FW_LOADER_COMPRESS and test
the compressed files conditionally?


thanks,

Takashi

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

* Re: [PATCH 0/5] firmware: Add support for loading compressed files
  2019-05-20 14:39   ` Takashi Iwai
@ 2019-05-20 15:18     ` Takashi Iwai
  2019-05-20 16:22       ` Takashi Iwai
  0 siblings, 1 reply; 20+ messages in thread
From: Takashi Iwai @ 2019-05-20 15:18 UTC (permalink / raw)
  To: Greg Kroah-Hartman; +Cc: Luis Chamberlain, Rafael J . Wysocki, linux-kernel

On Mon, 20 May 2019 16:39:37 +0200,
Takashi Iwai wrote:
> 
> On Mon, 20 May 2019 11:39:29 +0200,
> Greg Kroah-Hartman wrote:
> > 
> > On Mon, May 20, 2019 at 11:26:42AM +0200, Takashi Iwai wrote:
> > > Hi,
> > > 
> > > this is a patch set to add the support for loading compressed firmware
> > > files.
> > > 
> > > The primary motivation is to reduce the storage size; e.g. currently
> > > the amount of /lib/firmware on my machine counts up to 419MB, and this
> > > can be reduced to 130MB file compression.  No bad deal.
> > > 
> > > The feature adds only fallback to the compressed file, so it should
> > > work as it was as long as the normal firmware file is present.  The
> > > f/w loader decompresses the content, so that there is no change needed
> > > in the caller side.
> > > 
> > > Currently only XZ format is supported.  A caveat is that the kernel XZ
> > > helper code supports only CRC32 (or none) integrity check type, so
> > > you'll have to compress the files via xz -C crc32 option.
> > > 
> > > The patch set begins with a few other improvements and refactoring,
> > > followed by the compression support.
> > > 
> > > In addition to this, dracut needs a small fix to deal with the *.xz
> > > files.
> > > 
> > > Also, the latest patchset is found in topic/fw-decompress branch of my
> > > sound.git tree:
> > >   git://git.kernel.org/pub/scm/linux/kernel/git/tiwai/sound.git
> > 
> > After a quick review, these all look good to me, nice job.
> > 
> > One recommendation, can we add support for testing this to the
> > tools/testing/selftests/firmware/ tests?  And you did run those
> > regression tests to verify that you didn't get any of the config options
> > messed up, right? :)
> 
> Now I've been testing the firmware selftest, and this turned out to be
> surprisingly difficult on my system.  By some reason, the test always
> fails at the point triggering the request (line 58 of
> fw_filesystem.sh):
> 
>   if ! echo -n "$NAME" >"$DIR"/trigger_request ; then
> 	....
> 
> Judging from the strace output, this echo writes only the first byte
> of $NAME.  Then kernfs write op is invoked and it deals this one byte
> input as if a whole argument were passed, leading to an error.
> 
> My temporary workaround was to replace the all "echo" call with
> "/usr/bin/echo".
> 
> Then it hits a similar write error at the places like:
> 
> 	echo 1 > $DIR/config_sync_direct
> 
> This could be worked around by adding -n option to echo.
> 
> Finally, I noticed that the user-fallback doesn't work on my system
> any longer and the test stopped.  This is expected, so it implies that
> all direct loading tests passed.
> 
> FWIW, my system is openSUSE Leap 15.1.  Does anyone experience a
> similar problem?

This seems to be a regression on 5.2-rc1.
The tests on 4.20 worked fine.  5.1 worked, but gave the error at
fallback test instead of skipping.  This is likely another regression,
but irrelevant with the major issue as above.

Now bisecting...


Takashi

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

* Re: [PATCH 0/5] firmware: Add support for loading compressed files
  2019-05-20 15:18     ` Takashi Iwai
@ 2019-05-20 16:22       ` Takashi Iwai
  2019-05-20 17:26         ` shuah
  0 siblings, 1 reply; 20+ messages in thread
From: Takashi Iwai @ 2019-05-20 16:22 UTC (permalink / raw)
  To: Shuah Khan
  Cc: Greg Kroah-Hartman, Luis Chamberlain, Rafael J . Wysocki, linux-kernel

On Mon, 20 May 2019 17:18:48 +0200,
Takashi Iwai wrote:
> 
> On Mon, 20 May 2019 16:39:37 +0200,
> Takashi Iwai wrote:
> > 
> > On Mon, 20 May 2019 11:39:29 +0200,
> > Greg Kroah-Hartman wrote:
> > > 
> > > On Mon, May 20, 2019 at 11:26:42AM +0200, Takashi Iwai wrote:
> > > > Hi,
> > > > 
> > > > this is a patch set to add the support for loading compressed firmware
> > > > files.
> > > > 
> > > > The primary motivation is to reduce the storage size; e.g. currently
> > > > the amount of /lib/firmware on my machine counts up to 419MB, and this
> > > > can be reduced to 130MB file compression.  No bad deal.
> > > > 
> > > > The feature adds only fallback to the compressed file, so it should
> > > > work as it was as long as the normal firmware file is present.  The
> > > > f/w loader decompresses the content, so that there is no change needed
> > > > in the caller side.
> > > > 
> > > > Currently only XZ format is supported.  A caveat is that the kernel XZ
> > > > helper code supports only CRC32 (or none) integrity check type, so
> > > > you'll have to compress the files via xz -C crc32 option.
> > > > 
> > > > The patch set begins with a few other improvements and refactoring,
> > > > followed by the compression support.
> > > > 
> > > > In addition to this, dracut needs a small fix to deal with the *.xz
> > > > files.
> > > > 
> > > > Also, the latest patchset is found in topic/fw-decompress branch of my
> > > > sound.git tree:
> > > >   git://git.kernel.org/pub/scm/linux/kernel/git/tiwai/sound.git
> > > 
> > > After a quick review, these all look good to me, nice job.
> > > 
> > > One recommendation, can we add support for testing this to the
> > > tools/testing/selftests/firmware/ tests?  And you did run those
> > > regression tests to verify that you didn't get any of the config options
> > > messed up, right? :)
> > 
> > Now I've been testing the firmware selftest, and this turned out to be
> > surprisingly difficult on my system.  By some reason, the test always
> > fails at the point triggering the request (line 58 of
> > fw_filesystem.sh):
> > 
> >   if ! echo -n "$NAME" >"$DIR"/trigger_request ; then
> > 	....
> > 
> > Judging from the strace output, this echo writes only the first byte
> > of $NAME.  Then kernfs write op is invoked and it deals this one byte
> > input as if a whole argument were passed, leading to an error.
> > 
> > My temporary workaround was to replace the all "echo" call with
> > "/usr/bin/echo".
> > 
> > Then it hits a similar write error at the places like:
> > 
> > 	echo 1 > $DIR/config_sync_direct
> > 
> > This could be worked around by adding -n option to echo.
> > 
> > Finally, I noticed that the user-fallback doesn't work on my system
> > any longer and the test stopped.  This is expected, so it implies that
> > all direct loading tests passed.
> > 
> > FWIW, my system is openSUSE Leap 15.1.  Does anyone experience a
> > similar problem?
> 
> This seems to be a regression on 5.2-rc1.
> The tests on 4.20 worked fine.  5.1 worked, but gave the error at
> fallback test instead of skipping.  This is likely another regression,
> but irrelevant with the major issue as above.
> 
> Now bisecting...

Still in bisection, but it's timeout, I'll have to leave now...

FWIW, the regression seems to have been introduced around the latest
kselftest merge.
The commit 4c7b63a32d54 is bad, and the commit 9cbda1bddb4c good.

Shuah, could you check it?


thanks,

Takashi

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

* Re: [PATCH 0/5] firmware: Add support for loading compressed files
  2019-05-20 16:22       ` Takashi Iwai
@ 2019-05-20 17:26         ` shuah
  2019-05-20 18:07           ` Takashi Iwai
  0 siblings, 1 reply; 20+ messages in thread
From: shuah @ 2019-05-20 17:26 UTC (permalink / raw)
  To: Takashi Iwai, Kees Cook, shuah
  Cc: Greg Kroah-Hartman, Luis Chamberlain, Rafael J . Wysocki, linux-kernel

On 5/20/19 10:22 AM, Takashi Iwai wrote:
> On Mon, 20 May 2019 17:18:48 +0200,
> Takashi Iwai wrote:
>>
>> On Mon, 20 May 2019 16:39:37 +0200,
>> Takashi Iwai wrote:
>>>
>>> On Mon, 20 May 2019 11:39:29 +0200,
>>> Greg Kroah-Hartman wrote:
>>>>
>>>> On Mon, May 20, 2019 at 11:26:42AM +0200, Takashi Iwai wrote:
>>>>> Hi,
>>>>>
>>>>> this is a patch set to add the support for loading compressed firmware
>>>>> files.
>>>>>
>>>>> The primary motivation is to reduce the storage size; e.g. currently
>>>>> the amount of /lib/firmware on my machine counts up to 419MB, and this
>>>>> can be reduced to 130MB file compression.  No bad deal.
>>>>>
>>>>> The feature adds only fallback to the compressed file, so it should
>>>>> work as it was as long as the normal firmware file is present.  The
>>>>> f/w loader decompresses the content, so that there is no change needed
>>>>> in the caller side.
>>>>>
>>>>> Currently only XZ format is supported.  A caveat is that the kernel XZ
>>>>> helper code supports only CRC32 (or none) integrity check type, so
>>>>> you'll have to compress the files via xz -C crc32 option.
>>>>>
>>>>> The patch set begins with a few other improvements and refactoring,
>>>>> followed by the compression support.
>>>>>
>>>>> In addition to this, dracut needs a small fix to deal with the *.xz
>>>>> files.
>>>>>
>>>>> Also, the latest patchset is found in topic/fw-decompress branch of my
>>>>> sound.git tree:
>>>>>    git://git.kernel.org/pub/scm/linux/kernel/git/tiwai/sound.git
>>>>
>>>> After a quick review, these all look good to me, nice job.
>>>>
>>>> One recommendation, can we add support for testing this to the
>>>> tools/testing/selftests/firmware/ tests?  And you did run those
>>>> regression tests to verify that you didn't get any of the config options
>>>> messed up, right? :)
>>>
>>> Now I've been testing the firmware selftest, and this turned out to be
>>> surprisingly difficult on my system.  By some reason, the test always
>>> fails at the point triggering the request (line 58 of
>>> fw_filesystem.sh):
>>>
>>>    if ! echo -n "$NAME" >"$DIR"/trigger_request ; then
>>> 	....
>>>
>>> Judging from the strace output, this echo writes only the first byte
>>> of $NAME.  Then kernfs write op is invoked and it deals this one byte
>>> input as if a whole argument were passed, leading to an error.
>>>
>>> My temporary workaround was to replace the all "echo" call with
>>> "/usr/bin/echo".
>>>
>>> Then it hits a similar write error at the places like:
>>>
>>> 	echo 1 > $DIR/config_sync_direct
>>>
>>> This could be worked around by adding -n option to echo.
>>>
>>> Finally, I noticed that the user-fallback doesn't work on my system
>>> any longer and the test stopped.  This is expected, so it implies that
>>> all direct loading tests passed.
>>>
>>> FWIW, my system is openSUSE Leap 15.1.  Does anyone experience a
>>> similar problem?
>>
>> This seems to be a regression on 5.2-rc1.
>> The tests on 4.20 worked fine.  5.1 worked, but gave the error at
>> fallback test instead of skipping.  This is likely another regression,
>> but irrelevant with the major issue as above.
>>
>> Now bisecting...
> 
> Still in bisection, but it's timeout, I'll have to leave now...
> 
> FWIW, the regression seems to have been introduced around the latest
> kselftest merge.
> The commit 4c7b63a32d54 is bad, and the commit 9cbda1bddb4c good.
> 
> Shuah, could you check it?
> 
> 

Kees,

Could this be related to your selftest Makefile test run output
refactoring work?

I did a quick test on 5.2 after my first kselftest update without
the refactor work - firmware test worked.

I am thinking this is related to

eff3ee303d0d
selftests: Extract single-test shell logic from lib.mk

bash vs. sh difference in "echo" command behavior is the cause
is my best guess. Will you be able to take a look at this?

sudo make -C tools/testing/selftests/firmware/ run_tests

will show the difference.

thanks,
-- Shuah

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

* Re: [PATCH 0/5] firmware: Add support for loading compressed files
  2019-05-20 17:26         ` shuah
@ 2019-05-20 18:07           ` Takashi Iwai
  2019-05-20 18:59             ` Takashi Iwai
  0 siblings, 1 reply; 20+ messages in thread
From: Takashi Iwai @ 2019-05-20 18:07 UTC (permalink / raw)
  To: shuah
  Cc: Kees Cook, Greg Kroah-Hartman, Luis Chamberlain,
	Rafael J . Wysocki, linux-kernel

On Mon, 20 May 2019 19:26:59 +0200,
shuah wrote:
> 
> On 5/20/19 10:22 AM, Takashi Iwai wrote:
> > On Mon, 20 May 2019 17:18:48 +0200,
> > Takashi Iwai wrote:
> >>
> >> On Mon, 20 May 2019 16:39:37 +0200,
> >> Takashi Iwai wrote:
> >>>
> >>> On Mon, 20 May 2019 11:39:29 +0200,
> >>> Greg Kroah-Hartman wrote:
> >>>>
> >>>> On Mon, May 20, 2019 at 11:26:42AM +0200, Takashi Iwai wrote:
> >>>>> Hi,
> >>>>>
> >>>>> this is a patch set to add the support for loading compressed firmware
> >>>>> files.
> >>>>>
> >>>>> The primary motivation is to reduce the storage size; e.g. currently
> >>>>> the amount of /lib/firmware on my machine counts up to 419MB, and this
> >>>>> can be reduced to 130MB file compression.  No bad deal.
> >>>>>
> >>>>> The feature adds only fallback to the compressed file, so it should
> >>>>> work as it was as long as the normal firmware file is present.  The
> >>>>> f/w loader decompresses the content, so that there is no change needed
> >>>>> in the caller side.
> >>>>>
> >>>>> Currently only XZ format is supported.  A caveat is that the kernel XZ
> >>>>> helper code supports only CRC32 (or none) integrity check type, so
> >>>>> you'll have to compress the files via xz -C crc32 option.
> >>>>>
> >>>>> The patch set begins with a few other improvements and refactoring,
> >>>>> followed by the compression support.
> >>>>>
> >>>>> In addition to this, dracut needs a small fix to deal with the *.xz
> >>>>> files.
> >>>>>
> >>>>> Also, the latest patchset is found in topic/fw-decompress branch of my
> >>>>> sound.git tree:
> >>>>>    git://git.kernel.org/pub/scm/linux/kernel/git/tiwai/sound.git
> >>>>
> >>>> After a quick review, these all look good to me, nice job.
> >>>>
> >>>> One recommendation, can we add support for testing this to the
> >>>> tools/testing/selftests/firmware/ tests?  And you did run those
> >>>> regression tests to verify that you didn't get any of the config options
> >>>> messed up, right? :)
> >>>
> >>> Now I've been testing the firmware selftest, and this turned out to be
> >>> surprisingly difficult on my system.  By some reason, the test always
> >>> fails at the point triggering the request (line 58 of
> >>> fw_filesystem.sh):
> >>>
> >>>    if ! echo -n "$NAME" >"$DIR"/trigger_request ; then
> >>> 	....
> >>>
> >>> Judging from the strace output, this echo writes only the first byte
> >>> of $NAME.  Then kernfs write op is invoked and it deals this one byte
> >>> input as if a whole argument were passed, leading to an error.
> >>>
> >>> My temporary workaround was to replace the all "echo" call with
> >>> "/usr/bin/echo".
> >>>
> >>> Then it hits a similar write error at the places like:
> >>>
> >>> 	echo 1 > $DIR/config_sync_direct
> >>>
> >>> This could be worked around by adding -n option to echo.
> >>>
> >>> Finally, I noticed that the user-fallback doesn't work on my system
> >>> any longer and the test stopped.  This is expected, so it implies that
> >>> all direct loading tests passed.
> >>>
> >>> FWIW, my system is openSUSE Leap 15.1.  Does anyone experience a
> >>> similar problem?
> >>
> >> This seems to be a regression on 5.2-rc1.
> >> The tests on 4.20 worked fine.  5.1 worked, but gave the error at
> >> fallback test instead of skipping.  This is likely another regression,
> >> but irrelevant with the major issue as above.
> >>
> >> Now bisecting...
> >
> > Still in bisection, but it's timeout, I'll have to leave now...
> >
> > FWIW, the regression seems to have been introduced around the latest
> > kselftest merge.
> > The commit 4c7b63a32d54 is bad, and the commit 9cbda1bddb4c good.
> >
> > Shuah, could you check it?
> >
> >
> 
> Kees,
> 
> Could this be related to your selftest Makefile test run output
> refactoring work?
> 
> I did a quick test on 5.2 after my first kselftest update without
> the refactor work - firmware test worked.
> 
> I am thinking this is related to
> 
> eff3ee303d0d
> selftests: Extract single-test shell logic from lib.mk

OK, I could identify the culprit commit.  It's 5c069b6dedef
    selftests: Move test output to diagnostic lines

It seems that stdbuf usage caused a regression, at least on my
system.


Takashi

> 
> bash vs. sh difference in "echo" command behavior is the cause
> is my best guess. Will you be able to take a look at this?
> 
> sudo make -C tools/testing/selftests/firmware/ run_tests
> 
> will show the difference.
> 
> thanks,
> -- Shuah
> 

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

* Re: [PATCH 0/5] firmware: Add support for loading compressed files
  2019-05-20 18:07           ` Takashi Iwai
@ 2019-05-20 18:59             ` Takashi Iwai
  2019-05-20 21:50               ` Kees Cook
  0 siblings, 1 reply; 20+ messages in thread
From: Takashi Iwai @ 2019-05-20 18:59 UTC (permalink / raw)
  To: shuah
  Cc: Kees Cook, Greg Kroah-Hartman, Luis Chamberlain,
	Rafael J . Wysocki, linux-kernel

On Mon, 20 May 2019 20:07:25 +0200,
Takashi Iwai wrote:
> 
> On Mon, 20 May 2019 19:26:59 +0200,
> shuah wrote:
> > 
> > On 5/20/19 10:22 AM, Takashi Iwai wrote:
> > > On Mon, 20 May 2019 17:18:48 +0200,
> > > Takashi Iwai wrote:
> > >>
> > >> On Mon, 20 May 2019 16:39:37 +0200,
> > >> Takashi Iwai wrote:
> > >>>
> > >>> On Mon, 20 May 2019 11:39:29 +0200,
> > >>> Greg Kroah-Hartman wrote:
> > >>>>
> > >>>> On Mon, May 20, 2019 at 11:26:42AM +0200, Takashi Iwai wrote:
> > >>>>> Hi,
> > >>>>>
> > >>>>> this is a patch set to add the support for loading compressed firmware
> > >>>>> files.
> > >>>>>
> > >>>>> The primary motivation is to reduce the storage size; e.g. currently
> > >>>>> the amount of /lib/firmware on my machine counts up to 419MB, and this
> > >>>>> can be reduced to 130MB file compression.  No bad deal.
> > >>>>>
> > >>>>> The feature adds only fallback to the compressed file, so it should
> > >>>>> work as it was as long as the normal firmware file is present.  The
> > >>>>> f/w loader decompresses the content, so that there is no change needed
> > >>>>> in the caller side.
> > >>>>>
> > >>>>> Currently only XZ format is supported.  A caveat is that the kernel XZ
> > >>>>> helper code supports only CRC32 (or none) integrity check type, so
> > >>>>> you'll have to compress the files via xz -C crc32 option.
> > >>>>>
> > >>>>> The patch set begins with a few other improvements and refactoring,
> > >>>>> followed by the compression support.
> > >>>>>
> > >>>>> In addition to this, dracut needs a small fix to deal with the *.xz
> > >>>>> files.
> > >>>>>
> > >>>>> Also, the latest patchset is found in topic/fw-decompress branch of my
> > >>>>> sound.git tree:
> > >>>>>    git://git.kernel.org/pub/scm/linux/kernel/git/tiwai/sound.git
> > >>>>
> > >>>> After a quick review, these all look good to me, nice job.
> > >>>>
> > >>>> One recommendation, can we add support for testing this to the
> > >>>> tools/testing/selftests/firmware/ tests?  And you did run those
> > >>>> regression tests to verify that you didn't get any of the config options
> > >>>> messed up, right? :)
> > >>>
> > >>> Now I've been testing the firmware selftest, and this turned out to be
> > >>> surprisingly difficult on my system.  By some reason, the test always
> > >>> fails at the point triggering the request (line 58 of
> > >>> fw_filesystem.sh):
> > >>>
> > >>>    if ! echo -n "$NAME" >"$DIR"/trigger_request ; then
> > >>> 	....
> > >>>
> > >>> Judging from the strace output, this echo writes only the first byte
> > >>> of $NAME.  Then kernfs write op is invoked and it deals this one byte
> > >>> input as if a whole argument were passed, leading to an error.
> > >>>
> > >>> My temporary workaround was to replace the all "echo" call with
> > >>> "/usr/bin/echo".
> > >>>
> > >>> Then it hits a similar write error at the places like:
> > >>>
> > >>> 	echo 1 > $DIR/config_sync_direct
> > >>>
> > >>> This could be worked around by adding -n option to echo.
> > >>>
> > >>> Finally, I noticed that the user-fallback doesn't work on my system
> > >>> any longer and the test stopped.  This is expected, so it implies that
> > >>> all direct loading tests passed.
> > >>>
> > >>> FWIW, my system is openSUSE Leap 15.1.  Does anyone experience a
> > >>> similar problem?
> > >>
> > >> This seems to be a regression on 5.2-rc1.
> > >> The tests on 4.20 worked fine.  5.1 worked, but gave the error at
> > >> fallback test instead of skipping.  This is likely another regression,
> > >> but irrelevant with the major issue as above.
> > >>
> > >> Now bisecting...
> > >
> > > Still in bisection, but it's timeout, I'll have to leave now...
> > >
> > > FWIW, the regression seems to have been introduced around the latest
> > > kselftest merge.
> > > The commit 4c7b63a32d54 is bad, and the commit 9cbda1bddb4c good.
> > >
> > > Shuah, could you check it?
> > >
> > >
> > 
> > Kees,
> > 
> > Could this be related to your selftest Makefile test run output
> > refactoring work?
> > 
> > I did a quick test on 5.2 after my first kselftest update without
> > the refactor work - firmware test worked.
> > 
> > I am thinking this is related to
> > 
> > eff3ee303d0d
> > selftests: Extract single-test shell logic from lib.mk
> 
> OK, I could identify the culprit commit.  It's 5c069b6dedef
>     selftests: Move test output to diagnostic lines
> 
> It seems that stdbuf usage caused a regression, at least on my
> system.

So the problem is obvious: the commit above adjusts the stdout to be
unbuffered via stdbuf, hence each invocation like
  echo -n abc > /sys/....

would become writes of "a", "b" and "c", instead of "abc".

Although we can work around it in each test unit, I'm afraid that
enforcing the unbuffered stdio is too fragile for scripts like the
above case.


thanks,

Takashi

> 
> 
> Takashi
> 
> > 
> > bash vs. sh difference in "echo" command behavior is the cause
> > is my best guess. Will you be able to take a look at this?
> > 
> > sudo make -C tools/testing/selftests/firmware/ run_tests
> > 
> > will show the difference.
> > 
> > thanks,
> > -- Shuah
> > 

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

* Re: [PATCH 0/5] firmware: Add support for loading compressed files
  2019-05-20 18:59             ` Takashi Iwai
@ 2019-05-20 21:50               ` Kees Cook
  0 siblings, 0 replies; 20+ messages in thread
From: Kees Cook @ 2019-05-20 21:50 UTC (permalink / raw)
  To: Takashi Iwai
  Cc: shuah, Greg Kroah-Hartman, Luis Chamberlain, Rafael J . Wysocki,
	linux-kernel

On Mon, May 20, 2019 at 08:59:14PM +0200, Takashi Iwai wrote:
> So the problem is obvious: the commit above adjusts the stdout to be
> unbuffered via stdbuf, hence each invocation like
>   echo -n abc > /sys/....
> 
> would become writes of "a", "b" and "c", instead of "abc".
> 
> Although we can work around it in each test unit, I'm afraid that
> enforcing the unbuffered stdio is too fragile for scripts like the
> above case.

Oh this is nasty. Looks like stdbuf overrides all child processes too...
yeah, that's very broken. Let me try to see if I can find an
alternative.

Shuah, in the meantime, if you want a fix to restore test behavior,
but regress output flushing, this will work:

diff --git a/tools/testing/selftests/kselftest/runner.sh b/tools/testing/selftests/kselftest/runner.sh
index eff3ee303d0d..a529c19240fc 100644
--- a/tools/testing/selftests/kselftest/runner.sh
+++ b/tools/testing/selftests/kselftest/runner.sh
@@ -27,11 +27,11 @@ tap_prefix()
 # If stdbuf is unavailable, we must fall back to line-at-a-time piping.
 tap_unbuffer()
 {
-	if ! which stdbuf >/dev/null ; then
+	#if ! which asdfstdbuf >/dev/null ; then
 		"$@"
-	else
-		stdbuf -i0 -o0 -e0 "$@"
-	fi
+	#else
+	#	stdbuf -i0 -o0 -e0 "$@"
+	#fi
 }
 
 run_one()

Some tests will no longer show their output until they're entirely done,
but at least no test pass/fail results should regress.

I'll keep looking at solutions...

-- 
Kees Cook

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

* Re: [PATCH 0/5] firmware: Add support for loading compressed files
  2019-05-20  9:56   ` Takashi Iwai
@ 2019-05-21  5:30     ` Takashi Iwai
  0 siblings, 0 replies; 20+ messages in thread
From: Takashi Iwai @ 2019-05-21  5:30 UTC (permalink / raw)
  To: Greg Kroah-Hartman; +Cc: Luis Chamberlain, Rafael J . Wysocki, linux-kernel

On Mon, 20 May 2019 11:56:07 +0200,
Takashi Iwai wrote:
> 
> On Mon, 20 May 2019 11:39:29 +0200,
> Greg Kroah-Hartman wrote:
> > 
> > On Mon, May 20, 2019 at 11:26:42AM +0200, Takashi Iwai wrote:
> > > Hi,
> > > 
> > > this is a patch set to add the support for loading compressed firmware
> > > files.
> > > 
> > > The primary motivation is to reduce the storage size; e.g. currently
> > > the amount of /lib/firmware on my machine counts up to 419MB, and this
> > > can be reduced to 130MB file compression.  No bad deal.
> > > 
> > > The feature adds only fallback to the compressed file, so it should
> > > work as it was as long as the normal firmware file is present.  The
> > > f/w loader decompresses the content, so that there is no change needed
> > > in the caller side.
> > > 
> > > Currently only XZ format is supported.  A caveat is that the kernel XZ
> > > helper code supports only CRC32 (or none) integrity check type, so
> > > you'll have to compress the files via xz -C crc32 option.
> > > 
> > > The patch set begins with a few other improvements and refactoring,
> > > followed by the compression support.
> > > 
> > > In addition to this, dracut needs a small fix to deal with the *.xz
> > > files.
> > > 
> > > Also, the latest patchset is found in topic/fw-decompress branch of my
> > > sound.git tree:
> > >   git://git.kernel.org/pub/scm/linux/kernel/git/tiwai/sound.git
> > 
> > After a quick review, these all look good to me, nice job.
> > 
> > One recommendation, can we add support for testing this to the
> > tools/testing/selftests/firmware/ tests?  And you did run those
> > regression tests to verify that you didn't get any of the config options
> > messed up, right? :)
> 
> Oh, do you believe I'm a so modern person who lets computer working on
> everything? ;)  I only tested manually, so far, this will be my
> homework today.

After fixing the regression in kselftest, I could verify and confirm
that no regression was introduced by my patchset.

Also, below is the patch to add tests for the compressed firmware
load.  I'll add to the series at the next respin, if needed.


thanks,

Takashi

-- 8< --
From: Takashi Iwai <tiwai@suse.de>
Subject: [PATCH] selftests: firmware: Add compressed firmware tests

This patch adds the test cases for checking compressed firmware load.
Two more cases are added to fw_filesystem.sh:
- Both a plain file and an xz file are present, and load the former
- Only an xz file is present, and load without '.xz' suffix

The tests are enabled only when CONFIG_FW_LOADER_COMPRESS is enabled
and xz program is installed.

Signed-off-by: Takashi Iwai <tiwai@suse.de>
---
 tools/testing/selftests/firmware/fw_filesystem.sh | 73 +++++++++++++++++++----
 tools/testing/selftests/firmware/fw_lib.sh        |  7 +++
 tools/testing/selftests/firmware/fw_run_tests.sh  |  1 +
 3 files changed, 71 insertions(+), 10 deletions(-)

diff --git a/tools/testing/selftests/firmware/fw_filesystem.sh b/tools/testing/selftests/firmware/fw_filesystem.sh
index a4320c4b44dc..f901076aa2ea 100755
--- a/tools/testing/selftests/firmware/fw_filesystem.sh
+++ b/tools/testing/selftests/firmware/fw_filesystem.sh
@@ -153,13 +153,18 @@ config_set_read_fw_idx()
 
 read_firmwares()
 {
+	if [ "$1" = "xzonly" ]; then
+		fwfile="${FW}-orig"
+	else
+		fwfile="$FW"
+	fi
 	for i in $(seq 0 3); do
 		config_set_read_fw_idx $i
 		# 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
+		if ! diff -q -Z "$fwfile" $DIR/read_firmware 2>/dev/null ; then
 			echo "request #$i: firmware was not loaded" >&2
 			exit 1
 		fi
@@ -246,17 +251,17 @@ test_request_firmware_nowait_custom_nofile()
 
 test_batched_request_firmware()
 {
-	echo -n "Batched request_firmware() try #$1: "
+	echo -n "Batched request_firmware() $2 try #$1: "
 	config_reset
 	config_trigger_sync
-	read_firmwares
+	read_firmwares $2
 	release_all_firmware
 	echo "OK"
 }
 
 test_batched_request_firmware_direct()
 {
-	echo -n "Batched request_firmware_direct() try #$1: "
+	echo -n "Batched request_firmware_direct() $2 try #$1: "
 	config_reset
 	config_set_sync_direct
 	config_trigger_sync
@@ -266,7 +271,7 @@ test_batched_request_firmware_direct()
 
 test_request_firmware_nowait_uevent()
 {
-	echo -n "Batched request_firmware_nowait(uevent=true) try #$1: "
+	echo -n "Batched request_firmware_nowait(uevent=true) $2 try #$1: "
 	config_reset
 	config_trigger_async
 	release_all_firmware
@@ -275,11 +280,16 @@ test_request_firmware_nowait_uevent()
 
 test_request_firmware_nowait_custom()
 {
-	echo -n "Batched request_firmware_nowait(uevent=false) try #$1: "
+	echo -n "Batched request_firmware_nowait(uevent=false) $2 try #$1: "
 	config_reset
 	config_unset_uevent
 	RANDOM_FILE_PATH=$(setup_random_file)
 	RANDOM_FILE="$(basename $RANDOM_FILE_PATH)"
+	if [ "$2" = "both" ]; then
+		xz -9 -C crc32 -k $RANDOM_FILE_PATH
+	elif [ "$2" = "xzonly" ]; then
+		xz -9 -C crc32 $RANDOM_FILE_PATH
+	fi
 	config_set_name $RANDOM_FILE
 	config_trigger_async
 	release_all_firmware
@@ -294,19 +304,19 @@ test_config_present
 echo
 echo "Testing with the file present..."
 for i in $(seq 1 5); do
-	test_batched_request_firmware $i
+	test_batched_request_firmware $i normal
 done
 
 for i in $(seq 1 5); do
-	test_batched_request_firmware_direct $i
+	test_batched_request_firmware_direct $i normal
 done
 
 for i in $(seq 1 5); do
-	test_request_firmware_nowait_uevent $i
+	test_request_firmware_nowait_uevent $i normal
 done
 
 for i in $(seq 1 5); do
-	test_request_firmware_nowait_custom $i
+	test_request_firmware_nowait_custom $i normal
 done
 
 # Test for file not found, errors are expected, the failure would be
@@ -329,4 +339,47 @@ for i in $(seq 1 5); do
 	test_request_firmware_nowait_custom_nofile $i
 done
 
+test "$HAS_FW_LOADER_COMPRESS" != "yes" && exit 0
+
+# test with both files present
+xz -9 -C crc32 -k $FW
+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_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
+
+# test with only xz file present
+mv "$FW" "${FW}-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_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
+
 exit 0
diff --git a/tools/testing/selftests/firmware/fw_lib.sh b/tools/testing/selftests/firmware/fw_lib.sh
index 1cbb12e284a6..f236cc295450 100755
--- a/tools/testing/selftests/firmware/fw_lib.sh
+++ b/tools/testing/selftests/firmware/fw_lib.sh
@@ -50,6 +50,7 @@ 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)"
 	PROC_FW_IGNORE_SYSFS_FALLBACK="0"
 	PROC_FW_FORCE_SYSFS_FALLBACK="0"
 
@@ -84,6 +85,12 @@ check_setup()
 	fi
 
 	OLD_FWPATH="$(cat /sys/module/firmware_class/parameters/path)"
+
+	if [ "$HAS_FW_LOADER_COMPRESS" = "yes" ]; then
+		if ! which xz 2> /dev/null > /dev/null; then
+			HAS_FW_LOADER_COMPRESS=""
+		fi
+	fi
 }
 
 verify_reqs()
diff --git a/tools/testing/selftests/firmware/fw_run_tests.sh b/tools/testing/selftests/firmware/fw_run_tests.sh
index cffdd4eb0a57..8e14d555c197 100755
--- a/tools/testing/selftests/firmware/fw_run_tests.sh
+++ b/tools/testing/selftests/firmware/fw_run_tests.sh
@@ -11,6 +11,7 @@ source $TEST_DIR/fw_lib.sh
 
 export HAS_FW_LOADER_USER_HELPER=""
 export HAS_FW_LOADER_USER_HELPER_FALLBACK=""
+export HAS_FW_LOADER_COMPRESS=""
 
 run_tests()
 {
-- 
2.16.4


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

* Re: [PATCH 0/5] firmware: Add support for loading compressed files
  2019-05-20  9:26 [PATCH 0/5] firmware: Add support for loading compressed files Takashi Iwai
                   ` (5 preceding siblings ...)
  2019-05-20  9:39 ` [PATCH 0/5] " Greg Kroah-Hartman
@ 2019-05-28  5:25 ` Takashi Iwai
  2019-06-10 17:21 ` Greg Kroah-Hartman
  7 siblings, 0 replies; 20+ messages in thread
From: Takashi Iwai @ 2019-05-28  5:25 UTC (permalink / raw)
  To: Luis Chamberlain; +Cc: Greg Kroah-Hartman, Rafael J . Wysocki, linux-kernel

On Mon, 20 May 2019 11:26:42 +0200,
Takashi Iwai wrote:
> 
> Hi,
> 
> this is a patch set to add the support for loading compressed firmware
> files.
> 
> The primary motivation is to reduce the storage size; e.g. currently
> the amount of /lib/firmware on my machine counts up to 419MB, and this
> can be reduced to 130MB file compression.  No bad deal.
> 
> The feature adds only fallback to the compressed file, so it should
> work as it was as long as the normal firmware file is present.  The
> f/w loader decompresses the content, so that there is no change needed
> in the caller side.
> 
> Currently only XZ format is supported.  A caveat is that the kernel XZ
> helper code supports only CRC32 (or none) integrity check type, so
> you'll have to compress the files via xz -C crc32 option.
> 
> The patch set begins with a few other improvements and refactoring,
> followed by the compression support.
> 
> In addition to this, dracut needs a small fix to deal with the *.xz
> files.
> 
> Also, the latest patchset is found in topic/fw-decompress branch of my
> sound.git tree:
>   git://git.kernel.org/pub/scm/linux/kernel/git/tiwai/sound.git
> 
> 
> thanks,
> 
> Takashi

Luis, any comments on the patchset?


thanks,

Takashi

> 
> ===
> 
> Takashi Iwai (5):
>   firmware: Free temporary page table after vmapping
>   firmware: Unify the paged buffer release helper
>   firmware: Use kvmalloc for page tables
>   firmware: Factor out the paged buffer handling code
>   firmware: Add support for loading compressed files
> 
>  drivers/base/firmware_loader/Kconfig    |  18 +++
>  drivers/base/firmware_loader/fallback.c |  63 ++--------
>  drivers/base/firmware_loader/firmware.h |  16 ++-
>  drivers/base/firmware_loader/main.c     | 212 +++++++++++++++++++++++++++++---
>  4 files changed, 235 insertions(+), 74 deletions(-)
> 
> -- 
> 2.16.4
> 

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

* Re: [PATCH 0/5] firmware: Add support for loading compressed files
  2019-05-20  9:26 [PATCH 0/5] firmware: Add support for loading compressed files Takashi Iwai
                   ` (6 preceding siblings ...)
  2019-05-28  5:25 ` Takashi Iwai
@ 2019-06-10 17:21 ` Greg Kroah-Hartman
  2019-06-10 17:30   ` Takashi Iwai
  7 siblings, 1 reply; 20+ messages in thread
From: Greg Kroah-Hartman @ 2019-06-10 17:21 UTC (permalink / raw)
  To: Takashi Iwai; +Cc: Luis Chamberlain, Rafael J . Wysocki, linux-kernel

On Mon, May 20, 2019 at 11:26:42AM +0200, Takashi Iwai wrote:
> Hi,
> 
> this is a patch set to add the support for loading compressed firmware
> files.
> 
> The primary motivation is to reduce the storage size; e.g. currently
> the amount of /lib/firmware on my machine counts up to 419MB, and this
> can be reduced to 130MB file compression.  No bad deal.
> 
> The feature adds only fallback to the compressed file, so it should
> work as it was as long as the normal firmware file is present.  The
> f/w loader decompresses the content, so that there is no change needed
> in the caller side.
> 
> Currently only XZ format is supported.  A caveat is that the kernel XZ
> helper code supports only CRC32 (or none) integrity check type, so
> you'll have to compress the files via xz -C crc32 option.
> 
> The patch set begins with a few other improvements and refactoring,
> followed by the compression support.
> 
> In addition to this, dracut needs a small fix to deal with the *.xz
> files.
> 
> Also, the latest patchset is found in topic/fw-decompress branch of my
> sound.git tree:
>   git://git.kernel.org/pub/scm/linux/kernel/git/tiwai/sound.git

I've applied the first 3 patches to my tree, as they were sane and good
cleanups.

I'll wait for a test-case and a resend of the second two before taking
them {hint} :)

thanks,

greg k-h

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

* Re: [PATCH 0/5] firmware: Add support for loading compressed files
  2019-06-10 17:21 ` Greg Kroah-Hartman
@ 2019-06-10 17:30   ` Takashi Iwai
  2019-06-10 17:48     ` Greg Kroah-Hartman
  0 siblings, 1 reply; 20+ messages in thread
From: Takashi Iwai @ 2019-06-10 17:30 UTC (permalink / raw)
  To: Greg Kroah-Hartman; +Cc: Luis Chamberlain, Rafael J . Wysocki, linux-kernel

On Mon, 10 Jun 2019 19:21:30 +0200,
Greg Kroah-Hartman wrote:
> 
> On Mon, May 20, 2019 at 11:26:42AM +0200, Takashi Iwai wrote:
> > Hi,
> > 
> > this is a patch set to add the support for loading compressed firmware
> > files.
> > 
> > The primary motivation is to reduce the storage size; e.g. currently
> > the amount of /lib/firmware on my machine counts up to 419MB, and this
> > can be reduced to 130MB file compression.  No bad deal.
> > 
> > The feature adds only fallback to the compressed file, so it should
> > work as it was as long as the normal firmware file is present.  The
> > f/w loader decompresses the content, so that there is no change needed
> > in the caller side.
> > 
> > Currently only XZ format is supported.  A caveat is that the kernel XZ
> > helper code supports only CRC32 (or none) integrity check type, so
> > you'll have to compress the files via xz -C crc32 option.
> > 
> > The patch set begins with a few other improvements and refactoring,
> > followed by the compression support.
> > 
> > In addition to this, dracut needs a small fix to deal with the *.xz
> > files.
> > 
> > Also, the latest patchset is found in topic/fw-decompress branch of my
> > sound.git tree:
> >   git://git.kernel.org/pub/scm/linux/kernel/git/tiwai/sound.git
> 
> I've applied the first 3 patches to my tree, as they were sane and good
> cleanups.

Great, thanks!

> I'll wait for a test-case and a resend of the second two before taking
> them {hint} :)

The patch for test_firmware was already sent, and it includes the test
of the compressed firmware file.  In anyway, I'm going to resubmit the
rest tomorrow.


Takashi

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

* Re: [PATCH 0/5] firmware: Add support for loading compressed files
  2019-06-10 17:30   ` Takashi Iwai
@ 2019-06-10 17:48     ` Greg Kroah-Hartman
  0 siblings, 0 replies; 20+ messages in thread
From: Greg Kroah-Hartman @ 2019-06-10 17:48 UTC (permalink / raw)
  To: Takashi Iwai; +Cc: Luis Chamberlain, Rafael J . Wysocki, linux-kernel

On Mon, Jun 10, 2019 at 07:30:31PM +0200, Takashi Iwai wrote:
> On Mon, 10 Jun 2019 19:21:30 +0200,
> Greg Kroah-Hartman wrote:
> > 
> > On Mon, May 20, 2019 at 11:26:42AM +0200, Takashi Iwai wrote:
> > > Hi,
> > > 
> > > this is a patch set to add the support for loading compressed firmware
> > > files.
> > > 
> > > The primary motivation is to reduce the storage size; e.g. currently
> > > the amount of /lib/firmware on my machine counts up to 419MB, and this
> > > can be reduced to 130MB file compression.  No bad deal.
> > > 
> > > The feature adds only fallback to the compressed file, so it should
> > > work as it was as long as the normal firmware file is present.  The
> > > f/w loader decompresses the content, so that there is no change needed
> > > in the caller side.
> > > 
> > > Currently only XZ format is supported.  A caveat is that the kernel XZ
> > > helper code supports only CRC32 (or none) integrity check type, so
> > > you'll have to compress the files via xz -C crc32 option.
> > > 
> > > The patch set begins with a few other improvements and refactoring,
> > > followed by the compression support.
> > > 
> > > In addition to this, dracut needs a small fix to deal with the *.xz
> > > files.
> > > 
> > > Also, the latest patchset is found in topic/fw-decompress branch of my
> > > sound.git tree:
> > >   git://git.kernel.org/pub/scm/linux/kernel/git/tiwai/sound.git
> > 
> > I've applied the first 3 patches to my tree, as they were sane and good
> > cleanups.
> 
> Great, thanks!
> 
> > I'll wait for a test-case and a resend of the second two before taking
> > them {hint} :)
> 
> The patch for test_firmware was already sent, and it includes the test
> of the compressed firmware file.  In anyway, I'm going to resubmit the
> rest tomorrow.

Ah, sorry, I seem to have missed that.  A new series would be best,
thanks!

greg k-h

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

end of thread, other threads:[~2019-06-10 17:48 UTC | newest]

Thread overview: 20+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-05-20  9:26 [PATCH 0/5] firmware: Add support for loading compressed files Takashi Iwai
2019-05-20  9:26 ` [PATCH 1/5] firmware: Free temporary page table after vmapping Takashi Iwai
2019-05-20  9:26 ` [PATCH 2/5] firmware: Unify the paged buffer release helper Takashi Iwai
2019-05-20  9:26 ` [PATCH 3/5] firmware: Use kvmalloc for page tables Takashi Iwai
2019-05-20  9:26 ` [PATCH 4/5] firmware: Factor out the paged buffer handling code Takashi Iwai
2019-05-20  9:26 ` [PATCH 5/5] firmware: Add support for loading compressed files Takashi Iwai
2019-05-20  9:39 ` [PATCH 0/5] " Greg Kroah-Hartman
2019-05-20  9:56   ` Takashi Iwai
2019-05-21  5:30     ` Takashi Iwai
2019-05-20 14:39   ` Takashi Iwai
2019-05-20 15:18     ` Takashi Iwai
2019-05-20 16:22       ` Takashi Iwai
2019-05-20 17:26         ` shuah
2019-05-20 18:07           ` Takashi Iwai
2019-05-20 18:59             ` Takashi Iwai
2019-05-20 21:50               ` Kees Cook
2019-05-28  5:25 ` Takashi Iwai
2019-06-10 17:21 ` Greg Kroah-Hartman
2019-06-10 17:30   ` Takashi Iwai
2019-06-10 17:48     ` Greg Kroah-Hartman

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).