[PATCH v2 00/10] firmware_loader: built-in API and make x86 use it

[PATCH v2 00/10] firmware_loader: built-in API and make x86 use it

[Luis R. Rodriguez]

From: Luis Chamberlain <mcgrof@kernel.org>

Changes in this v2:

 o drops two patches which Greg has already merged
 o drop one patch which added the new kconfig symbol FW_LOADER_BUILTIN
   which Greg didn't like, and so instead we rely only on the FW_LOADER
   symbol. If anyone cringes during review because of this, just keep in mind,
   *this* is *why* I added the symbol in the first patch series.

I've ran the firmware selftests test and found no issues. I have also
let 0 day grind on this and it found no issues. This is all based on
linux-next next-20211020, I have a branch 20211020-firmware-builtin
which has these changes in case anyone wants this in a git tree [0].

[0] https://git.kernel.org/pub/scm/linux/kernel/git/mcgrof/linux-next.git/log/?h=20211020-firmware-builtin

Borislav Petkov (1):
  x86/microcode: Use the firmware_loader built-in API

Luis Chamberlain (9):
  firmware_loader: formalize built-in firmware API
  firmware_loader: remove old DECLARE_BUILTIN_FIRMWARE()
  firmware_loader: move struct builtin_fw to the only place used
  vmlinux.lds.h: wrap built-in firmware support under FW_LOADER
  x86/build: Tuck away built-in firmware under FW_LOADER
  firmware_loader: rename EXTRA_FIRMWARE and EXTRA_FIRMWARE_DIR
  firmware_loader: move builtin build helper to shared library
  test_firmware: move a few test knobs out to its library
  test_firmware: add support for testing built-in firmware

 .../driver-api/firmware/built-in-fw.rst       |   6 +-
 Documentation/x86/microcode.rst               |   8 +-
 arch/x86/Kconfig                              |   4 +-
 arch/x86/include/asm/microcode.h              |   3 -
 arch/x86/kernel/cpu/microcode/amd.c           |  14 ++-
 arch/x86/kernel/cpu/microcode/core.c          |  17 ---
 arch/x86/kernel/cpu/microcode/intel.c         |   9 +-
 arch/x86/tools/relocs.c                       |   2 +
 drivers/base/firmware_loader/Kconfig          |  29 +++--
 drivers/base/firmware_loader/Makefile         |   1 +
 drivers/base/firmware_loader/builtin/Makefile |  43 ++-----
 .../base/firmware_loader/builtin/lib.Makefile |  32 ++++++
 drivers/base/firmware_loader/builtin/main.c   | 106 ++++++++++++++++++
 drivers/base/firmware_loader/firmware.h       |  17 +++
 drivers/base/firmware_loader/main.c           |  78 +------------
 .../firmware_loader/test-builtin/.gitignore   |   3 +
 .../firmware_loader/test-builtin/Makefile     |  18 +++
 drivers/staging/media/av7110/Kconfig          |   4 +-
 include/asm-generic/vmlinux.lds.h             |  20 ++--
 include/linux/firmware.h                      |  30 +++--
 lib/Kconfig.debug                             |  33 ++++++
 lib/test_firmware.c                           |  52 ++++++++-
 .../testing/selftests/firmware/fw_builtin.sh  |  69 ++++++++++++
 .../selftests/firmware/fw_filesystem.sh       |  16 ---
 tools/testing/selftests/firmware/fw_lib.sh    |  24 ++++
 .../selftests/firmware/fw_run_tests.sh        |   2 +
 26 files changed, 441 insertions(+), 199 deletions(-)
 create mode 100644 drivers/base/firmware_loader/builtin/lib.Makefile
 create mode 100644 drivers/base/firmware_loader/builtin/main.c
 create mode 100644 drivers/base/firmware_loader/test-builtin/.gitignore
 create mode 100644 drivers/base/firmware_loader/test-builtin/Makefile
 create mode 100755 tools/testing/selftests/firmware/fw_builtin.sh

-- 
2.30.2

[PATCH v2 01/10] firmware_loader: formalize built-in firmware API

[Luis R. Rodriguez]

From: Luis Chamberlain <mcgrof@kernel.org>

Formalize the built-in firmware with a proper API. This can later
be used by other callers where all they need is built-in firmware.

We export the firmware_request_builtin() call for now only
under the TEST_FIRMWARE symbol namespace as there are no
direct modular users for it. If they pop up they are free
to export it generally. Built-in code always gets access to
the callers and we'll demonstrate a hidden user which has been
lurking in the kernel for a while and the reason why using a
proper API was better long term.

Reviewed-by: Borislav Petkov <bp@suse.de>
Signed-off-by: Luis Chamberlain <mcgrof@kernel.org>
---
 drivers/base/firmware_loader/builtin/Makefile |   6 +-
 drivers/base/firmware_loader/builtin/main.c   | 100 ++++++++++++++++++
 drivers/base/firmware_loader/firmware.h       |  17 +++
 drivers/base/firmware_loader/main.c           |  78 +-------------
 include/linux/firmware.h                      |  15 +++
 5 files changed, 137 insertions(+), 79 deletions(-)
 create mode 100644 drivers/base/firmware_loader/builtin/main.c

diff --git a/drivers/base/firmware_loader/builtin/Makefile b/drivers/base/firmware_loader/builtin/Makefile
index 101754ad48d9..eb4be452062a 100644
--- a/drivers/base/firmware_loader/builtin/Makefile
+++ b/drivers/base/firmware_loader/builtin/Makefile
@@ -1,11 +1,13 @@
 # SPDX-License-Identifier: GPL-2.0
+obj-y  += main.o
 
 # Create $(fwdir) from $(CONFIG_EXTRA_FIRMWARE_DIR) -- if it doesn't have a
 # leading /, it's relative to $(srctree).
 fwdir := $(subst $(quote),,$(CONFIG_EXTRA_FIRMWARE_DIR))
 fwdir := $(addprefix $(srctree)/,$(filter-out /%,$(fwdir)))$(filter /%,$(fwdir))
 
-obj-y  := $(addsuffix .gen.o, $(subst $(quote),,$(CONFIG_EXTRA_FIRMWARE)))
+firmware  := $(addsuffix .gen.o, $(subst $(quote),,$(CONFIG_EXTRA_FIRMWARE)))
+obj-y += $(firmware)
 
 FWNAME    = $(patsubst $(obj)/%.gen.S,%,$@)
 FWSTR     = $(subst $(comma),_,$(subst /,_,$(subst .,_,$(subst -,_,$(FWNAME)))))
@@ -34,7 +36,7 @@ $(obj)/%.gen.S: FORCE
 	$(call filechk,fwbin)
 
 # The .o files depend on the binaries directly; the .S files don't.
-$(addprefix $(obj)/, $(obj-y)): $(obj)/%.gen.o: $(fwdir)/%
+$(addprefix $(obj)/, $(firmware)): $(obj)/%.gen.o: $(fwdir)/%
 
 targets := $(patsubst $(obj)/%,%, \
                                 $(shell find $(obj) -name \*.gen.S 2>/dev/null))
diff --git a/drivers/base/firmware_loader/builtin/main.c b/drivers/base/firmware_loader/builtin/main.c
new file mode 100644
index 000000000000..d85626b2fdf5
--- /dev/null
+++ b/drivers/base/firmware_loader/builtin/main.c
@@ -0,0 +1,100 @@
+// SPDX-License-Identifier: GPL-2.0
+/* Builtin firmware support */
+
+#include <linux/firmware.h>
+#include "../firmware.h"
+
+/* Only if FW_LOADER=y */
+#ifdef CONFIG_FW_LOADER
+
+extern struct builtin_fw __start_builtin_fw[];
+extern struct builtin_fw __end_builtin_fw[];
+
+static bool fw_copy_to_prealloc_buf(struct firmware *fw,
+				    void *buf, size_t size)
+{
+	if (!buf)
+		return true;
+	if (size < fw->size)
+		return false;
+	memcpy(buf, fw->data, fw->size);
+	return true;
+}
+
+/**
+ * firmware_request_builtin() - load builtin firmware
+ * @fw: pointer to firmware struct
+ * @name: name of firmware file
+ *
+ * Some use cases in the kernel have a requirement so that no memory allocator
+ * is involved as these calls take place early in boot process. An example is
+ * the x86 CPU microcode loader. In these cases all the caller wants is to see
+ * if the firmware was built-in and if so use it right away. This can be used
+ * for such cases.
+ *
+ * This looks for the firmware in the built-in kernel. Only if the kernel was
+ * built-in with the firmware you are looking for will this return successfully.
+ *
+ * Callers of this API do not need to use release_firmware() as the pointer to
+ * the firmware is expected to be provided locally on the stack of the caller.
+ **/
+bool firmware_request_builtin(struct firmware *fw, const char *name)
+{
+	struct builtin_fw *b_fw;
+
+	if (!fw)
+		return false;
+
+	for (b_fw = __start_builtin_fw; b_fw != __end_builtin_fw; b_fw++) {
+		if (strcmp(name, b_fw->name) == 0) {
+			fw->size = b_fw->size;
+			fw->data = b_fw->data;
+			return true;
+		}
+	}
+
+	return false;
+}
+EXPORT_SYMBOL_NS_GPL(firmware_request_builtin, TEST_FIRMWARE);
+
+/**
+ * firmware_request_builtin_buf() - load builtin firmware into optional buffer
+ * @fw: pointer to firmware struct
+ * @name: name of firmware file
+ * @buf: If set this lets you use a pre-allocated buffer so that the built-in
+ *	firmware into is copied into. This field can be NULL. It is used by
+ *	callers such as request_firmware_into_buf() and
+ *	request_partial_firmware_into_buf()
+ * @size: if buf was provided, the max size of the allocated buffer available.
+ *	If the built-in firmware does not fit into the pre-allocated @buf this
+ *	call will fail.
+ *
+ * This looks for the firmware in the built-in kernel. Only if the kernel was
+ * built-in with the firmware you are looking for will this call possibly
+ * succeed. If you passed a @buf the firmware will be copied into it *iff* the
+ * built-in firmware fits into the pre-allocated buffer size specified in
+ * @size.
+ *
+ * This caller is to be used internally by the firmware_loader only.
+ **/
+bool firmware_request_builtin_buf(struct firmware *fw, const char *name,
+				  void *buf, size_t size)
+{
+	if (!firmware_request_builtin(fw, name))
+		return false;
+
+	return fw_copy_to_prealloc_buf(fw, buf, size);
+}
+
+bool firmware_is_builtin(const struct firmware *fw)
+{
+	struct builtin_fw *b_fw;
+
+	for (b_fw = __start_builtin_fw; b_fw != __end_builtin_fw; b_fw++)
+		if (fw->data == b_fw->data)
+			return true;
+
+	return false;
+}
+
+#endif
diff --git a/drivers/base/firmware_loader/firmware.h b/drivers/base/firmware_loader/firmware.h
index a3014e9e2c85..2889f446ad41 100644
--- a/drivers/base/firmware_loader/firmware.h
+++ b/drivers/base/firmware_loader/firmware.h
@@ -151,6 +151,23 @@ static inline void fw_state_done(struct fw_priv *fw_priv)
 
 int assign_fw(struct firmware *fw, struct device *device);
 
+#ifdef CONFIG_FW_LOADER
+bool firmware_is_builtin(const struct firmware *fw);
+bool firmware_request_builtin_buf(struct firmware *fw, const char *name,
+				  void *buf, size_t size);
+#else /* module case */
+static inline bool firmware_is_builtin(const struct firmware *fw)
+{
+	return false;
+}
+static inline bool firmware_request_builtin_buf(struct firmware *fw,
+						const char *name,
+						void *buf, size_t size)
+{
+	return false;
+}
+#endif
+
 #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);
diff --git a/drivers/base/firmware_loader/main.c b/drivers/base/firmware_loader/main.c
index d95b5fe5f700..94d1789a233e 100644
--- a/drivers/base/firmware_loader/main.c
+++ b/drivers/base/firmware_loader/main.c
@@ -93,82 +93,6 @@ DEFINE_MUTEX(fw_lock);
 
 static struct firmware_cache fw_cache;
 
-/* Builtin firmware support */
-
-#ifdef CONFIG_FW_LOADER
-
-extern struct builtin_fw __start_builtin_fw[];
-extern struct builtin_fw __end_builtin_fw[];
-
-static bool fw_copy_to_prealloc_buf(struct firmware *fw,
-				    void *buf, size_t size)
-{
-	if (!buf)
-		return true;
-	if (size < fw->size)
-		return false;
-	memcpy(buf, fw->data, fw->size);
-	return true;
-}
-
-static bool firmware_request_builtin(struct firmware *fw, const char *name)
-{
-	struct builtin_fw *b_fw;
-
-	if (!fw)
-		return false;
-
-	for (b_fw = __start_builtin_fw; b_fw != __end_builtin_fw; b_fw++) {
-		if (strcmp(name, b_fw->name) == 0) {
-			fw->size = b_fw->size;
-			fw->data = b_fw->data;
-			return true;
-		}
-	}
-
-	return false;
-}
-
-static bool firmware_request_builtin_buf(struct firmware *fw, const char *name,
-					 void *buf, size_t size)
-{
-	if (!firmware_request_builtin(fw, name))
-		return false;
-	return fw_copy_to_prealloc_buf(fw, buf, size);
-}
-
-static bool fw_is_builtin_firmware(const struct firmware *fw)
-{
-	struct builtin_fw *b_fw;
-
-	for (b_fw = __start_builtin_fw; b_fw != __end_builtin_fw; b_fw++)
-		if (fw->data == b_fw->data)
-			return true;
-
-	return false;
-}
-
-#else /* Module case - no builtin firmware support */
-
-static inline bool firmware_request_builtin(struct firmware *fw,
-					    const char *name)
-{
-	return false;
-}
-
-static inline bool firmware_request_builtin_buf(struct firmware *fw,
-						const char *name, void *buf,
-						size_t size)
-{
-	return false;
-}
-
-static inline bool fw_is_builtin_firmware(const struct firmware *fw)
-{
-	return false;
-}
-#endif
-
 static void fw_state_init(struct fw_priv *fw_priv)
 {
 	struct fw_state *fw_st = &fw_priv->fw_st;
@@ -1068,7 +992,7 @@ EXPORT_SYMBOL(request_partial_firmware_into_buf);
 void release_firmware(const struct firmware *fw)
 {
 	if (fw) {
-		if (!fw_is_builtin_firmware(fw))
+		if (!firmware_is_builtin(fw))
 			firmware_free_data(fw);
 		kfree(fw);
 	}
diff --git a/include/linux/firmware.h b/include/linux/firmware.h
index 25109192cebe..d743a8d1c2fe 100644
--- a/include/linux/firmware.h
+++ b/include/linux/firmware.h
@@ -20,12 +20,19 @@ struct firmware {
 struct module;
 struct device;
 
+/*
+ * Built-in firmware functionality is only available if FW_LOADER=y, but not
+ * FW_LOADER=m
+ */
+#ifdef CONFIG_FW_LOADER
 struct builtin_fw {
 	char *name;
 	void *data;
 	unsigned long size;
 };
 
+bool firmware_request_builtin(struct firmware *fw, const char *name);
+
 /* We have to play tricks here much like stringify() to get the
    __COUNTER__ macro to be expanded as we want it */
 #define __fw_concat1(x, y) x##y
@@ -38,6 +45,14 @@ struct builtin_fw {
 	static const struct builtin_fw __fw_concat(__builtin_fw,__COUNTER__) \
 	__used __section(".builtin_fw") = { name, blob, size }
 
+#else
+static inline bool firmware_request_builtin(struct firmware *fw,
+					    const char *name)
+{
+	return false;
+}
+#endif
+
 #if defined(CONFIG_FW_LOADER) || (defined(CONFIG_FW_LOADER_MODULE) && defined(MODULE))
 int request_firmware(const struct firmware **fw, const char *name,
 		     struct device *device);
-- 
2.30.2

[PATCH v2 02/10] firmware_loader: remove old DECLARE_BUILTIN_FIRMWARE()

[Luis R. Rodriguez]

From: Luis Chamberlain <mcgrof@kernel.org>

This was never used upstream. Time to get rid of it. We
don't carry around unused baggage.

Reviewed-by: Borislav Petkov <bp@suse.de>
Signed-off-by: Luis Chamberlain <mcgrof@kernel.org>
---
 include/linux/firmware.h | 13 -------------
 1 file changed, 13 deletions(-)

diff --git a/include/linux/firmware.h b/include/linux/firmware.h
index d743a8d1c2fe..34e8d5844fa0 100644
--- a/include/linux/firmware.h
+++ b/include/linux/firmware.h
@@ -32,19 +32,6 @@ struct builtin_fw {
 };
 
 bool firmware_request_builtin(struct firmware *fw, const char *name);
-
-/* We have to play tricks here much like stringify() to get the
-   __COUNTER__ macro to be expanded as we want it */
-#define __fw_concat1(x, y) x##y
-#define __fw_concat(x, y) __fw_concat1(x, y)
-
-#define DECLARE_BUILTIN_FIRMWARE(name, blob)				     \
-	DECLARE_BUILTIN_FIRMWARE_SIZE(name, &(blob), sizeof(blob))
-
-#define DECLARE_BUILTIN_FIRMWARE_SIZE(name, blob, size)			     \
-	static const struct builtin_fw __fw_concat(__builtin_fw,__COUNTER__) \
-	__used __section(".builtin_fw") = { name, blob, size }
-
 #else
 static inline bool firmware_request_builtin(struct firmware *fw,
 					    const char *name)
-- 
2.30.2

[PATCH v2 03/10] x86/microcode: Use the firmware_loader built-in API

[Luis R. Rodriguez]

From: Borislav Petkov <bp@suse.de>

The microcode loader has been looping through __start_builtin_fw down to
__end_builtin_fw to look for possibly built-in firmware for microcode
updates.

Now that the firmware loader code has exported an API for looping
through the kernel's built-in firmware section, use it and drop the x86
implementation in favor.

Signed-off-by: Borislav Petkov <bp@suse.de>
Signed-off-by: Luis Chamberlain <mcgrof@kernel.org>
---
 arch/x86/include/asm/microcode.h      |  3 ---
 arch/x86/kernel/cpu/microcode/amd.c   | 14 ++++++++++----
 arch/x86/kernel/cpu/microcode/core.c  | 17 -----------------
 arch/x86/kernel/cpu/microcode/intel.c |  9 ++++++++-
 4 files changed, 18 insertions(+), 25 deletions(-)

diff --git a/arch/x86/include/asm/microcode.h b/arch/x86/include/asm/microcode.h
index ab45a220fac4..d6bfdfb0f0af 100644
--- a/arch/x86/include/asm/microcode.h
+++ b/arch/x86/include/asm/microcode.h
@@ -130,14 +130,11 @@ static inline unsigned int x86_cpuid_family(void)
 extern void __init load_ucode_bsp(void);
 extern void load_ucode_ap(void);
 void reload_early_microcode(void);
-extern bool get_builtin_firmware(struct cpio_data *cd, const char *name);
 extern bool initrd_gone;
 #else
 static inline void __init load_ucode_bsp(void)			{ }
 static inline void load_ucode_ap(void)				{ }
 static inline void reload_early_microcode(void)			{ }
-static inline bool
-get_builtin_firmware(struct cpio_data *cd, const char *name)	{ return false; }
 #endif
 
 #endif /* _ASM_X86_MICROCODE_H */
diff --git a/arch/x86/kernel/cpu/microcode/amd.c b/arch/x86/kernel/cpu/microcode/amd.c
index 3d4a48336084..8b2fcdfa6d31 100644
--- a/arch/x86/kernel/cpu/microcode/amd.c
+++ b/arch/x86/kernel/cpu/microcode/amd.c
@@ -456,17 +456,23 @@ apply_microcode_early_amd(u32 cpuid_1_eax, void *ucode, size_t size, bool save_p
 
 static bool get_builtin_microcode(struct cpio_data *cp, unsigned int family)
 {
-#ifdef CONFIG_X86_64
 	char fw_name[36] = "amd-ucode/microcode_amd.bin";
+	struct firmware fw;
+
+	if (IS_ENABLED(CONFIG_X86_32))
+		return false;
 
 	if (family >= 0x15)
 		snprintf(fw_name, sizeof(fw_name),
 			 "amd-ucode/microcode_amd_fam%.2xh.bin", family);
 
-	return get_builtin_firmware(cp, fw_name);
-#else
+	if (firmware_request_builtin(&fw, fw_name)) {
+		cp->size = fw.size;
+		cp->data = (void *)fw.data;
+		return true;
+	}
+
 	return false;
-#endif
 }
 
 static void __load_ucode_amd(unsigned int cpuid_1_eax, struct cpio_data *ret)
diff --git a/arch/x86/kernel/cpu/microcode/core.c b/arch/x86/kernel/cpu/microcode/core.c
index efb69be41ab1..f955d25076ba 100644
--- a/arch/x86/kernel/cpu/microcode/core.c
+++ b/arch/x86/kernel/cpu/microcode/core.c
@@ -140,23 +140,6 @@ static bool __init check_loader_disabled_bsp(void)
 	return *res;
 }
 
-extern struct builtin_fw __start_builtin_fw[];
-extern struct builtin_fw __end_builtin_fw[];
-
-bool get_builtin_firmware(struct cpio_data *cd, const char *name)
-{
-	struct builtin_fw *b_fw;
-
-	for (b_fw = __start_builtin_fw; b_fw != __end_builtin_fw; b_fw++) {
-		if (!strcmp(name, b_fw->name)) {
-			cd->size = b_fw->size;
-			cd->data = b_fw->data;
-			return true;
-		}
-	}
-	return false;
-}
-
 void __init load_ucode_bsp(void)
 {
 	unsigned int cpuid_1_eax;
diff --git a/arch/x86/kernel/cpu/microcode/intel.c b/arch/x86/kernel/cpu/microcode/intel.c
index 7e8e07bddd5f..d28a9f8f3fec 100644
--- a/arch/x86/kernel/cpu/microcode/intel.c
+++ b/arch/x86/kernel/cpu/microcode/intel.c
@@ -456,6 +456,7 @@ static void save_mc_for_early(struct ucode_cpu_info *uci, u8 *mc, unsigned int s
 static bool load_builtin_intel_microcode(struct cpio_data *cp)
 {
 	unsigned int eax = 1, ebx, ecx = 0, edx;
+	struct firmware fw;
 	char name[30];
 
 	if (IS_ENABLED(CONFIG_X86_32))
@@ -466,7 +467,13 @@ static bool load_builtin_intel_microcode(struct cpio_data *cp)
 	sprintf(name, "intel-ucode/%02x-%02x-%02x",
 		      x86_family(eax), x86_model(eax), x86_stepping(eax));
 
-	return get_builtin_firmware(cp, name);
+	if (firmware_request_builtin(&fw, name)) {
+		cp->size = fw.size;
+		cp->data = (void *)fw.data;
+		return true;
+	}
+
+	return false;
 }
 
 /*
-- 
2.30.2

[PATCH v2 04/10] firmware_loader: move struct builtin_fw to the only place used

[Luis R. Rodriguez]

From: Luis Chamberlain <mcgrof@kernel.org>

Now that x86 doesn't abuse picking at internals to the firmware
loader move out the built-in firmware struct to its only user.

Reviewed-by: Borislav Petkov <bp@suse.de>
Signed-off-by: Luis Chamberlain <mcgrof@kernel.org>
---
 drivers/base/firmware_loader/builtin/main.c | 6 ++++++
 include/linux/firmware.h                    | 6 ------
 2 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/base/firmware_loader/builtin/main.c b/drivers/base/firmware_loader/builtin/main.c
index d85626b2fdf5..a065c3150897 100644
--- a/drivers/base/firmware_loader/builtin/main.c
+++ b/drivers/base/firmware_loader/builtin/main.c
@@ -7,6 +7,12 @@
 /* Only if FW_LOADER=y */
 #ifdef CONFIG_FW_LOADER
 
+struct builtin_fw {
+	char *name;
+	void *data;
+	unsigned long size;
+};
+
 extern struct builtin_fw __start_builtin_fw[];
 extern struct builtin_fw __end_builtin_fw[];
 
diff --git a/include/linux/firmware.h b/include/linux/firmware.h
index 34e8d5844fa0..3b057dfc8284 100644
--- a/include/linux/firmware.h
+++ b/include/linux/firmware.h
@@ -25,12 +25,6 @@ struct device;
  * FW_LOADER=m
  */
 #ifdef CONFIG_FW_LOADER
-struct builtin_fw {
-	char *name;
-	void *data;
-	unsigned long size;
-};
-
 bool firmware_request_builtin(struct firmware *fw, const char *name);
 #else
 static inline bool firmware_request_builtin(struct firmware *fw,
-- 
2.30.2

[PATCH v2 05/10] vmlinux.lds.h: wrap built-in firmware support under FW_LOADER

[Luis R. Rodriguez]

From: Luis Chamberlain <mcgrof@kernel.org>

The firmware loader built-in firmware is only available when FW_LOADER
is built-in, so tuck away the sections for built-in firmware under it.

This ensures no oddball user tries to uses these sections without
first enabling FW_LOADER=y.

Reviewed-by: Borislav Petkov <bp@suse.de>
Signed-off-by: Luis Chamberlain <mcgrof@kernel.org>
---
 include/asm-generic/vmlinux.lds.h | 20 +++++++++++++-------
 1 file changed, 13 insertions(+), 7 deletions(-)

diff --git a/include/asm-generic/vmlinux.lds.h b/include/asm-generic/vmlinux.lds.h
index 8771c435f34b..62351669add4 100644
--- a/include/asm-generic/vmlinux.lds.h
+++ b/include/asm-generic/vmlinux.lds.h
@@ -476,13 +476,7 @@
 		__end_pci_fixups_suspend_late = .;			\
 	}								\
 									\
-	/* Built-in firmware blobs */					\
-	.builtin_fw : AT(ADDR(.builtin_fw) - LOAD_OFFSET) ALIGN(8) {	\
-		__start_builtin_fw = .;					\
-		KEEP(*(.builtin_fw))					\
-		__end_builtin_fw = .;					\
-	}								\
-									\
+	FW_LOADER_BUILT_IN_DATA						\
 	TRACEDATA							\
 									\
 	PRINTK_INDEX							\
@@ -886,6 +880,18 @@
 #define ORC_UNWIND_TABLE
 #endif
 
+/* Built-in firmware blobs */
+#ifdef CONFIG_FW_LOADER
+#define FW_LOADER_BUILT_IN_DATA						\
+	.builtin_fw : AT(ADDR(.builtin_fw) - LOAD_OFFSET) ALIGN(8) {	\
+		__start_builtin_fw = .;					\
+		KEEP(*(.builtin_fw))					\
+		__end_builtin_fw = .;					\
+	}
+#else
+#define FW_LOADER_BUILT_IN_DATA
+#endif
+
 #ifdef CONFIG_PM_TRACE
 #define TRACEDATA							\
 	. = ALIGN(4);							\
-- 
2.30.2

[PATCH v2 06/10] x86/build: Tuck away built-in firmware under FW_LOADER

[Luis R. Rodriguez]

From: Luis Chamberlain <mcgrof@kernel.org>

When FW_LOADER is modular or disabled we don't use it.
Update x86 relocs to reflect that.

Reviewed-by: Borislav Petkov <bp@suse.de>
Signed-off-by: Luis Chamberlain <mcgrof@kernel.org>
---
 arch/x86/tools/relocs.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/arch/x86/tools/relocs.c b/arch/x86/tools/relocs.c
index 27c82207d387..ab3554d4aa06 100644
--- a/arch/x86/tools/relocs.c
+++ b/arch/x86/tools/relocs.c
@@ -63,7 +63,9 @@ static const char * const sym_regex_kernel[S_NSYMTYPES] = {
 	"(__parainstructions|__alt_instructions)(_end)?|"
 	"(__iommu_table|__apicdrivers|__smp_locks)(_end)?|"
 	"__(start|end)_pci_.*|"
+#if CONFIG_FW_LOADER_BUILTIN
 	"__(start|end)_builtin_fw|"
+#endif
 	"__(start|stop)___ksymtab(_gpl)?|"
 	"__(start|stop)___kcrctab(_gpl)?|"
 	"__(start|stop)___param|"
-- 
2.30.2

[PATCH v2 07/10] firmware_loader: rename EXTRA_FIRMWARE and EXTRA_FIRMWARE_DIR

[Luis R. Rodriguez]

From: Luis Chamberlain <mcgrof@kernel.org>

Now that we've tied loose ends on the built-in firmware API,
rename the kconfig symbols for it to reflect more that they are
associated to the firmware_loader and to make it easier to
understand what they are for.

Signed-off-by: Luis Chamberlain <mcgrof@kernel.org>
---
 .../driver-api/firmware/built-in-fw.rst       |  6 ++--
 Documentation/x86/microcode.rst               |  8 ++---
 arch/x86/Kconfig                              |  4 +--
 drivers/base/firmware_loader/Kconfig          | 29 ++++++++++++-------
 drivers/base/firmware_loader/builtin/Makefile |  6 ++--
 drivers/staging/media/av7110/Kconfig          |  4 +--
 6 files changed, 33 insertions(+), 24 deletions(-)

diff --git a/Documentation/driver-api/firmware/built-in-fw.rst b/Documentation/driver-api/firmware/built-in-fw.rst
index bc1c961bace1..a9a0ab8c9512 100644
--- a/Documentation/driver-api/firmware/built-in-fw.rst
+++ b/Documentation/driver-api/firmware/built-in-fw.rst
@@ -8,11 +8,11 @@ the filesystem. Instead, firmware can be looked for inside the kernel
 directly. You can enable built-in firmware using the kernel configuration
 options:
 
-  * CONFIG_EXTRA_FIRMWARE
-  * CONFIG_EXTRA_FIRMWARE_DIR
+  * CONFIG_FW_LOADER_BUILTIN_FILES
+  * CONFIG_FW_LOADER_BUILTIN_DIR
 
 There are a few reasons why you might want to consider building your firmware
-into the kernel with CONFIG_EXTRA_FIRMWARE:
+into the kernel with CONFIG_FW_LOADER_BUILTIN_FILES:
 
 * Speed
 * Firmware is needed for accessing the boot device, and the user doesn't
diff --git a/Documentation/x86/microcode.rst b/Documentation/x86/microcode.rst
index a320d37982ed..2cacc7f60014 100644
--- a/Documentation/x86/microcode.rst
+++ b/Documentation/x86/microcode.rst
@@ -114,13 +114,13 @@ Builtin microcode
 =================
 
 The loader supports also loading of a builtin microcode supplied through
-the regular builtin firmware method CONFIG_EXTRA_FIRMWARE. Only 64-bit is
-currently supported.
+the regular builtin firmware method using CONFIG_FW_LOADER_BUILTIN and
+CONFIG_FW_LOADER_BUILTIN_FILES. Only 64-bit is currently supported.
 
 Here's an example::
 
-  CONFIG_EXTRA_FIRMWARE="intel-ucode/06-3a-09 amd-ucode/microcode_amd_fam15h.bin"
-  CONFIG_EXTRA_FIRMWARE_DIR="/lib/firmware"
+  CONFIG_FW_LOADER_BUILTIN_FILES="intel-ucode/06-3a-09 amd-ucode/microcode_amd_fam15h.bin"
+  CONFIG_FW_LOADER_BUILTIN_DIR="/lib/firmware"
 
 This basically means, you have the following tree structure locally::
 
diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index 999907dd7544..cfb09dc7f21b 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -1315,8 +1315,8 @@ config MICROCODE
 	  initrd for microcode blobs.
 
 	  In addition, you can build the microcode into the kernel. For that you
-	  need to add the vendor-supplied microcode to the CONFIG_EXTRA_FIRMWARE
-	  config option.
+	  need to add the vendor-supplied microcode to the configuration option
+	  CONFIG_FW_LOADER_BUILTIN_FILES
 
 config MICROCODE_INTEL
 	bool "Intel microcode loading support"
diff --git a/drivers/base/firmware_loader/Kconfig b/drivers/base/firmware_loader/Kconfig
index 5b24f3959255..2dc3e137d903 100644
--- a/drivers/base/firmware_loader/Kconfig
+++ b/drivers/base/firmware_loader/Kconfig
@@ -22,14 +22,14 @@ config FW_LOADER
 	  You typically want this built-in (=y) but you can also enable this
 	  as a module, in which case the firmware_class module will be built.
 	  You also want to be sure to enable this built-in if you are going to
-	  enable built-in firmware (CONFIG_EXTRA_FIRMWARE).
+	  enable built-in firmware (CONFIG_FW_LOADER_BUILTIN_FILES).
 
 if FW_LOADER
 
 config FW_LOADER_PAGED_BUF
 	bool
 
-config EXTRA_FIRMWARE
+config FW_LOADER_BUILTIN_FILES
 	string "Build named firmware blobs into the kernel binary"
 	help
 	  Device drivers which require firmware can typically deal with
@@ -43,14 +43,21 @@ config EXTRA_FIRMWARE
 	  in boot and cannot rely on the firmware being placed in an initrd or
 	  initramfs.
 
-	  This option is a string and takes the (space-separated) names of the
+	  Support for built-in firmware is not supported if you are using
+	  the firmware loader as a module.
+
+	  This option is a string and takes the space-separated names of the
 	  firmware files -- the same names that appear in MODULE_FIRMWARE()
 	  and request_firmware() in the source. These files should exist under
-	  the directory specified by the EXTRA_FIRMWARE_DIR option, which is
+	  the directory specified by the FW_LOADER_BUILTIN_DIR option, which is
 	  /lib/firmware by default.
 
-	  For example, you might set CONFIG_EXTRA_FIRMWARE="usb8388.bin", copy
-	  the usb8388.bin file into /lib/firmware, and build the kernel. Then
+	  For example, you might have set:
+
+	  CONFIG_FW_LOADER_BUILTIN_FILES="usb8388.bin"
+
+	  After this you would copy the usb8388.bin file into directory
+	  specified by FW_LOADER_BUILTIN_DIR and build the kernel. Then
 	  any request_firmware("usb8388.bin") will be satisfied internally
 	  inside the kernel without ever looking at your filesystem at runtime.
 
@@ -60,13 +67,15 @@ config EXTRA_FIRMWARE
 	  image since it combines both GPL and non-GPL work. You should
 	  consult a lawyer of your own before distributing such an image.
 
-config EXTRA_FIRMWARE_DIR
-	string "Firmware blobs root directory"
+config FW_LOADER_BUILTIN_DIR
+	string "Directory with firmware to be built-in to the kernel"
 	depends on EXTRA_FIRMWARE != ""
 	default "/lib/firmware"
 	help
-	  This option controls the directory in which the kernel build system
-	  looks for the firmware files listed in the EXTRA_FIRMWARE option.
+	  This option specifies the directory which the kernel build system
+	  will use to look for the firmware files which are going to be
+	  built into the kernel using the space-separated
+	  FW_LOADER_BUILTIN_FILES entries.
 
 config FW_LOADER_USER_HELPER
 	bool "Enable the firmware sysfs fallback mechanism"
diff --git a/drivers/base/firmware_loader/builtin/Makefile b/drivers/base/firmware_loader/builtin/Makefile
index eb4be452062a..7cdd0b5c7384 100644
--- a/drivers/base/firmware_loader/builtin/Makefile
+++ b/drivers/base/firmware_loader/builtin/Makefile
@@ -1,12 +1,12 @@
 # SPDX-License-Identifier: GPL-2.0
 obj-y  += main.o
 
-# Create $(fwdir) from $(CONFIG_EXTRA_FIRMWARE_DIR) -- if it doesn't have a
+# Create $(fwdir) from $(CONFIG_FW_LOADER_BUILTIN_DIR) -- if it doesn't have a
 # leading /, it's relative to $(srctree).
-fwdir := $(subst $(quote),,$(CONFIG_EXTRA_FIRMWARE_DIR))
+fwdir := $(subst $(quote),,$(CONFIG_FW_LOADER_BUILTIN_DIR))
 fwdir := $(addprefix $(srctree)/,$(filter-out /%,$(fwdir)))$(filter /%,$(fwdir))
 
-firmware  := $(addsuffix .gen.o, $(subst $(quote),,$(CONFIG_EXTRA_FIRMWARE)))
+firmware  := $(addsuffix .gen.o, $(subst $(quote),,$(CONFIG_FW_LOADER_BUILTIN_FILES)))
 obj-y += $(firmware)
 
 FWNAME    = $(patsubst $(obj)/%.gen.S,%,$@)
diff --git a/drivers/staging/media/av7110/Kconfig b/drivers/staging/media/av7110/Kconfig
index 9faf9d2d4001..87c7702f72f6 100644
--- a/drivers/staging/media/av7110/Kconfig
+++ b/drivers/staging/media/av7110/Kconfig
@@ -31,8 +31,8 @@ config DVB_AV7110
 	  or /lib/firmware (depending on configuration of firmware hotplug).
 
 	  Alternatively, you can download the file and use the kernel's
-	  EXTRA_FIRMWARE configuration option to build it into your
-	  kernel image by adding the filename to the EXTRA_FIRMWARE
+	  FW_LOADER_BUILTIN_FILES configuration option to build it into your
+	  kernel image by adding the filename to the FW_LOADER_BUILTIN_FILES
 	  configuration option string.
 
 	  Say Y if you own such a card and want to use it.
-- 
2.30.2

[PATCH v2 08/10] firmware_loader: move builtin build helper to shared library

[Luis R. Rodriguez]

From: Luis Chamberlain <mcgrof@kernel.org>

If we wanted to use a different directory for building target
builtin firmware it is easier if we just have a shared library
Makefile, and each target directory can then just include it and
populate the respective needed variables. This reduces clutter,
makes things easier to read, and also most importantly allows
us to not have to try to magically adjust only one target
kconfig symbol for built-in firmware files. Trying to do this
can easily end up causing odd build issues if the user is not
careful.

As an example issue, if we are going to try to extend the
FW_LOADER_BUILTIN_FILES list and FW_LOADER_BUILTIN_DIR in case
of a new test firmware builtin support currently our only option
would be modify the defaults of each of these in case test firmware
builtin support was enabled. Defaults however won't augment a prior
setting, and so if FW_LOADER_BUILTIN_DIR="/lib/firmware" and you
and want this to be changed to something like
FW_LOADER_BUILTIN_DIR="drivers/base/firmware_loader/test-builtin"
the change will not take effect as a prior build already had it
set, and the build would fail. Trying to augment / append the
variables in the Makefile just makes this very difficult to
read.

Using a library let's us split up possible built-in targets so
that the user does not have to be involved. This will be used
in a subsequent patch which will add another user to this
built-in firmware library Makefile and demo how to use it outside
of the default FW_LOADER_BUILTIN_DIR and FW_LOADER_BUILTIN_FILES.

Signed-off-by: Luis Chamberlain <mcgrof@kernel.org>
---
 drivers/base/firmware_loader/builtin/Makefile | 34 ++-----------------
 .../base/firmware_loader/builtin/lib.Makefile | 32 +++++++++++++++++
 2 files changed, 34 insertions(+), 32 deletions(-)
 create mode 100644 drivers/base/firmware_loader/builtin/lib.Makefile

diff --git a/drivers/base/firmware_loader/builtin/Makefile b/drivers/base/firmware_loader/builtin/Makefile
index 7cdd0b5c7384..9b0dc193f6c7 100644
--- a/drivers/base/firmware_loader/builtin/Makefile
+++ b/drivers/base/firmware_loader/builtin/Makefile
@@ -1,4 +1,6 @@
 # SPDX-License-Identifier: GPL-2.0
+include $(srctree)/drivers/base/firmware_loader/builtin/lib.Makefile
+
 obj-y  += main.o
 
 # Create $(fwdir) from $(CONFIG_FW_LOADER_BUILTIN_DIR) -- if it doesn't have a
@@ -8,35 +10,3 @@ fwdir := $(addprefix $(srctree)/,$(filter-out /%,$(fwdir)))$(filter /%,$(fwdir))
 
 firmware  := $(addsuffix .gen.o, $(subst $(quote),,$(CONFIG_FW_LOADER_BUILTIN_FILES)))
 obj-y += $(firmware)
-
-FWNAME    = $(patsubst $(obj)/%.gen.S,%,$@)
-FWSTR     = $(subst $(comma),_,$(subst /,_,$(subst .,_,$(subst -,_,$(FWNAME)))))
-ASM_WORD  = $(if $(CONFIG_64BIT),.quad,.long)
-ASM_ALIGN = $(if $(CONFIG_64BIT),3,2)
-PROGBITS  = $(if $(CONFIG_ARM),%,@)progbits
-
-filechk_fwbin = \
-	echo "/* Generated by $(src)/Makefile */"		;\
-	echo "    .section .rodata"				;\
-	echo "    .p2align 4"					;\
-	echo "_fw_$(FWSTR)_bin:"				;\
-	echo "    .incbin \"$(fwdir)/$(FWNAME)\""		;\
-	echo "_fw_end:"						;\
-	echo "    .section .rodata.str,\"aMS\",$(PROGBITS),1"	;\
-	echo "    .p2align $(ASM_ALIGN)"			;\
-	echo "_fw_$(FWSTR)_name:"				;\
-	echo "    .string \"$(FWNAME)\""			;\
-	echo "    .section .builtin_fw,\"a\",$(PROGBITS)"	;\
-	echo "    .p2align $(ASM_ALIGN)"			;\
-	echo "    $(ASM_WORD) _fw_$(FWSTR)_name"		;\
-	echo "    $(ASM_WORD) _fw_$(FWSTR)_bin"			;\
-	echo "    $(ASM_WORD) _fw_end - _fw_$(FWSTR)_bin"
-
-$(obj)/%.gen.S: FORCE
-	$(call filechk,fwbin)
-
-# The .o files depend on the binaries directly; the .S files don't.
-$(addprefix $(obj)/, $(firmware)): $(obj)/%.gen.o: $(fwdir)/%
-
-targets := $(patsubst $(obj)/%,%, \
-                                $(shell find $(obj) -name \*.gen.S 2>/dev/null))
diff --git a/drivers/base/firmware_loader/builtin/lib.Makefile b/drivers/base/firmware_loader/builtin/lib.Makefile
new file mode 100644
index 000000000000..e979a67acfa7
--- /dev/null
+++ b/drivers/base/firmware_loader/builtin/lib.Makefile
@@ -0,0 +1,32 @@
+# SPDX-License-Identifier: GPL-2.0
+FWNAME    = $(patsubst $(obj)/%.gen.S,%,$@)
+FWSTR     = $(subst $(comma),_,$(subst /,_,$(subst .,_,$(subst -,_,$(FWNAME)))))
+ASM_WORD  = $(if $(CONFIG_64BIT),.quad,.long)
+ASM_ALIGN = $(if $(CONFIG_64BIT),3,2)
+PROGBITS  = $(if $(CONFIG_ARM),%,@)progbits
+
+filechk_fwbin = \
+	echo "/* Generated by $(src)/Makefile */"		;\
+	echo "    .section .rodata"				;\
+	echo "    .p2align 4"					;\
+	echo "_fw_$(FWSTR)_bin:"				;\
+	echo "    .incbin \"$(fwdir)/$(FWNAME)\""		;\
+	echo "_fw_end:"						;\
+	echo "    .section .rodata.str,\"aMS\",$(PROGBITS),1"	;\
+	echo "    .p2align $(ASM_ALIGN)"			;\
+	echo "_fw_$(FWSTR)_name:"				;\
+	echo "    .string \"$(FWNAME)\""			;\
+	echo "    .section .builtin_fw,\"a\",$(PROGBITS)"	;\
+	echo "    .p2align $(ASM_ALIGN)"			;\
+	echo "    $(ASM_WORD) _fw_$(FWSTR)_name"		;\
+	echo "    $(ASM_WORD) _fw_$(FWSTR)_bin"			;\
+	echo "    $(ASM_WORD) _fw_end - _fw_$(FWSTR)_bin"
+
+$(obj)/%.gen.S: FORCE
+	$(call filechk,fwbin)
+
+# The .o files depend on the binaries directly; the .S files don't.
+$(addprefix $(obj)/, $(firmware)): $(obj)/%.gen.o: $(fwdir)/%
+
+targets := $(patsubst $(obj)/%,%, \
+                                $(shell find $(obj) -name \*.gen.S 2>/dev/null))
-- 
2.30.2

[PATCH v2 09/10] test_firmware: move a few test knobs out to its library

[Luis R. Rodriguez]

From: Luis Chamberlain <mcgrof@kernel.org>

These will be used by other tests cases in other files so
move them to the library.

Signed-off-by: Luis Chamberlain <mcgrof@kernel.org>
---
 .../testing/selftests/firmware/fw_filesystem.sh | 16 ----------------
 tools/testing/selftests/firmware/fw_lib.sh      | 17 +++++++++++++++++
 2 files changed, 17 insertions(+), 16 deletions(-)

diff --git a/tools/testing/selftests/firmware/fw_filesystem.sh b/tools/testing/selftests/firmware/fw_filesystem.sh
index c2a2a100114b..7d763b303057 100755
--- a/tools/testing/selftests/firmware/fw_filesystem.sh
+++ b/tools/testing/selftests/firmware/fw_filesystem.sh
@@ -118,27 +118,11 @@ test_config_present()
 	fi
 }
 
-# Defaults :
-#
-# send_uevent: 1
-# sync_direct: 0
-# name: test-firmware.bin
-# num_requests: 4
-config_reset()
-{
-	echo 1 >  $DIR/reset
-}
-
 release_all_firmware()
 {
 	echo 1 >  $DIR/release_all_firmware
 }
 
-config_set_name()
-{
-	echo -n $1 >  $DIR/config_name
-}
-
 config_set_into_buf()
 {
 	echo 1 >  $DIR/config_into_buf
diff --git a/tools/testing/selftests/firmware/fw_lib.sh b/tools/testing/selftests/firmware/fw_lib.sh
index 5b8c0fedee76..31b71fe11dc5 100755
--- a/tools/testing/selftests/firmware/fw_lib.sh
+++ b/tools/testing/selftests/firmware/fw_lib.sh
@@ -221,3 +221,20 @@ kconfig_has()
 		fi
 	fi
 }
+
+# Defaults :
+#
+# send_uevent: 1
+# sync_direct: 0
+# name: test-firmware.bin
+# num_requests: 4
+config_reset()
+{
+	echo 1 >  $DIR/reset
+}
+
+
+config_set_name()
+{
+	echo -n $1 >  $DIR/config_name
+}
-- 
2.30.2

[PATCH v2 10/10] test_firmware: add support for testing built-in firmware

[Luis R. Rodriguez]

From: Luis Chamberlain <mcgrof@kernel.org>

This adds some basic knobs to let us test built-in firmware
support. We test to ensure built-in firmware is indeed used,
and that it matches the contents we expect. Likewise we test
that a file that should not be built-in was not present.

For older kernels with older test_firmware drivers (yes some
folks do test selftests this way and we support it), the new
built-in test will simply bail out early.

Reviewed-by: Borislav Petkov <bp@suse.de>
Signed-off-by: Luis Chamberlain <mcgrof@kernel.org>
---
 drivers/base/firmware_loader/Makefile         |  1 +
 drivers/base/firmware_loader/builtin/Makefile |  1 +
 .../firmware_loader/test-builtin/.gitignore   |  3 +
 .../firmware_loader/test-builtin/Makefile     | 18 +++++
 lib/Kconfig.debug                             | 33 +++++++++
 lib/test_firmware.c                           | 52 +++++++++++++-
 .../testing/selftests/firmware/fw_builtin.sh  | 69 +++++++++++++++++++
 tools/testing/selftests/firmware/fw_lib.sh    |  7 ++
 .../selftests/firmware/fw_run_tests.sh        |  2 +
 9 files changed, 185 insertions(+), 1 deletion(-)
 create mode 100644 drivers/base/firmware_loader/test-builtin/.gitignore
 create mode 100644 drivers/base/firmware_loader/test-builtin/Makefile
 create mode 100755 tools/testing/selftests/firmware/fw_builtin.sh

diff --git a/drivers/base/firmware_loader/Makefile b/drivers/base/firmware_loader/Makefile
index e87843408fe6..dbeba0fa315d 100644
--- a/drivers/base/firmware_loader/Makefile
+++ b/drivers/base/firmware_loader/Makefile
@@ -1,6 +1,7 @@
 # SPDX-License-Identifier: GPL-2.0
 # Makefile for the Linux firmware loader
 
+obj-$(CONFIG_TEST_FIRMWARE_BUILTIN) += test-builtin/
 obj-$(CONFIG_FW_LOADER_USER_HELPER) += fallback_table.o
 obj-$(CONFIG_FW_LOADER)	+= firmware_class.o
 firmware_class-objs := main.o
diff --git a/drivers/base/firmware_loader/builtin/Makefile b/drivers/base/firmware_loader/builtin/Makefile
index 9b0dc193f6c7..baad7777974b 100644
--- a/drivers/base/firmware_loader/builtin/Makefile
+++ b/drivers/base/firmware_loader/builtin/Makefile
@@ -2,6 +2,7 @@
 include $(srctree)/drivers/base/firmware_loader/builtin/lib.Makefile
 
 obj-y  += main.o
+obj-$(CONFIG_TEST_BUILTIN_FIRMWARE)  += test-builtin-firmware.bin.gen.o
 
 # Create $(fwdir) from $(CONFIG_FW_LOADER_BUILTIN_DIR) -- if it doesn't have a
 # leading /, it's relative to $(srctree).
diff --git a/drivers/base/firmware_loader/test-builtin/.gitignore b/drivers/base/firmware_loader/test-builtin/.gitignore
new file mode 100644
index 000000000000..1d46553f50a0
--- /dev/null
+++ b/drivers/base/firmware_loader/test-builtin/.gitignore
@@ -0,0 +1,3 @@
+# SPDX-License-Identifier: GPL-2.0
+*.gen.S
+*.bin
diff --git a/drivers/base/firmware_loader/test-builtin/Makefile b/drivers/base/firmware_loader/test-builtin/Makefile
new file mode 100644
index 000000000000..04204ad7ede1
--- /dev/null
+++ b/drivers/base/firmware_loader/test-builtin/Makefile
@@ -0,0 +1,18 @@
+# SPDX-License-Identifier: GPL-2.0
+include $(srctree)/drivers/base/firmware_loader/builtin/lib.Makefile
+
+extra-y := test-builtin-firmware.bin
+$(obj)/test-builtin-firmware.bin: FORCE
+	@$(kecho) "  GEN     $@"
+	@(set -e;			\
+	(				\
+		echo 'ABCD0123';	\
+	) > $@)
+
+# Create $(fwdir) from $(CONFIG_TEST_FIRMWARE_BUILTIN_DIR) -- if it doesn't
+# have a leading /, it's relative to $(srctree).
+fwdir := $(subst $(quote),,$(CONFIG_TEST_FIRMWARE_BUILTIN_DIR))
+fwdir := $(addprefix $(srctree)/,$(filter-out /%,$(fwdir)))$(filter /%,$(fwdir))
+
+firmware  := $(addsuffix .gen.o, $(subst $(quote),,$(CONFIG_TEST_FIRMWARE_BUILTIN_FILES)))
+obj-y += $(firmware)
diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
index 3266fef7cb53..b8af41619fc3 100644
--- a/lib/Kconfig.debug
+++ b/lib/Kconfig.debug
@@ -2333,6 +2333,39 @@ config TEST_FIRMWARE
 
 	  If unsure, say N.
 
+config TEST_FIRMWARE_BUILTIN
+	bool "Allow building built-in test firmware"
+	depends on TEST_FIRMWARE
+	help
+	  If enabled, firmware will be built into to the kernel so that
+	  loading it with the test_firmware driver can be tested. Disabling
+	  this will just mean the test firmware scripts won't find any
+	  built-in firmware. Enabling this will make the kernel generate the
+	  file test-builtin-firmware.bin inside local directory
+	  drivers/base/firmware_loader/test-builtin. This file will then
+	  be available whenever the request firmware API is used.
+
+	  Enabling this functionlity essentially overrides the location where
+	  you specify to find built-in firmware, and so should only be enabled
+	  if you don't need to build firmware into your kernel for full
+	  funcionality.
+
+	  This should be disabled on production kernels as otherwise you'll
+	  end up with a test firmware stuck into your final kernel image and
+	  with default built-in firmware support.
+
+	  If unsure, say N.
+
+config TEST_FIRMWARE_BUILTIN_FILES
+	string
+	depends on TEST_FIRMWARE_BUILTIN
+	default "test-builtin-firmware.bin"
+
+config TEST_FIRMWARE_BUILTIN_DIR
+	string
+	depends on TEST_FIRMWARE_BUILTIN
+	default "drivers/base/firmware_loader/test-builtin"
+
 config TEST_SYSCTL
 	tristate "sysctl test driver"
 	depends on PROC_SYSCTL
diff --git a/lib/test_firmware.c b/lib/test_firmware.c
index 1bccd6cd5f48..134f8c78d2d4 100644
--- a/lib/test_firmware.c
+++ b/lib/test_firmware.c
@@ -34,6 +34,7 @@ MODULE_IMPORT_NS(TEST_FIRMWARE);
 
 static DEFINE_MUTEX(test_fw_mutex);
 static const struct firmware *test_firmware;
+static struct firmware builtin_test_firmware;
 
 struct test_batched_req {
 	u8 idx;
@@ -58,6 +59,10 @@ struct test_batched_req {
  * @sync_direct: when the sync trigger is used if this is true
  *	request_firmware_direct() will be used instead.
  * @send_uevent: whether or not to send a uevent for async requests
+ * @is_builtin: used only internally to determine if the firmware was found
+ *	to be built-in to the kernel using only the API call
+ *	firmware_request_builtin(). We treat this specially as we are
+ *	responsible for the firmware struct.
  * @num_requests: number of requests to try per test case. This is trigger
  *	specific.
  * @reqs: stores all requests information
@@ -99,6 +104,7 @@ struct test_config {
 	bool partial;
 	bool sync_direct;
 	bool send_uevent;
+	bool is_builtin;
 	u8 num_requests;
 	u8 read_fw_idx;
 
@@ -120,7 +126,11 @@ static ssize_t test_fw_misc_read(struct file *f, char __user *buf,
 	ssize_t rc = 0;
 
 	mutex_lock(&test_fw_mutex);
-	if (test_firmware)
+	if (test_fw_config->is_builtin)
+		rc = simple_read_from_buffer(buf, size, offset,
+					     builtin_test_firmware.data,
+					     builtin_test_firmware.size);
+	else if (test_firmware)
 		rc = simple_read_from_buffer(buf, size, offset,
 					     test_firmware->data,
 					     test_firmware->size);
@@ -194,6 +204,7 @@ static int __test_firmware_config_init(void)
 	test_fw_config->buf_size = TEST_FIRMWARE_BUF_SIZE;
 	test_fw_config->file_offset = 0;
 	test_fw_config->partial = false;
+	test_fw_config->is_builtin = false;
 	test_fw_config->sync_direct = false;
 	test_fw_config->req_firmware = request_firmware;
 	test_fw_config->test_result = 0;
@@ -1051,6 +1062,44 @@ static ssize_t read_firmware_show(struct device *dev,
 }
 static DEVICE_ATTR_RO(read_firmware);
 
+/*
+ * In order to test this, set CONFIG_FW_LOADER_BUILTIN_FILES to a firmware file
+ * which will be built into the kernel image. Then echo the name into the
+ * "trigger_request_builtin" sysfs file of this module.
+ */
+static ssize_t trigger_request_builtin_store(struct device *dev,
+					     struct device_attribute *attr,
+					     const char *buf, size_t count)
+{
+	int rc = -ENOENT;
+
+	if (!test_fw_config->name) {
+		pr_warn("unconfigured firmware settings\n");
+		return rc;
+	}
+
+	pr_info("loading builtin '%s'\n", test_fw_config->name);
+
+	mutex_lock(&test_fw_mutex);
+
+	if (firmware_request_builtin(&builtin_test_firmware,
+				     test_fw_config->name)) {
+		/* This let's us diff against the firmware */
+		test_fw_config->is_builtin = true;
+		pr_info("loaded: %zu\n", builtin_test_firmware.size);
+		rc = count;
+		goto out;
+	}
+
+	pr_info("load of '%s' failed\n", test_fw_config->name);
+
+out:
+	mutex_unlock(&test_fw_mutex);
+
+	return rc;
+}
+static DEVICE_ATTR_WO(trigger_request_builtin);
+
 #define TEST_FW_DEV_ATTR(name)          &dev_attr_##name.attr
 
 static struct attribute *test_dev_attrs[] = {
@@ -1082,6 +1131,7 @@ static struct attribute *test_dev_attrs[] = {
 	TEST_FW_DEV_ATTR(release_all_firmware),
 	TEST_FW_DEV_ATTR(test_result),
 	TEST_FW_DEV_ATTR(read_firmware),
+	TEST_FW_DEV_ATTR(trigger_request_builtin),
 	NULL,
 };
 
diff --git a/tools/testing/selftests/firmware/fw_builtin.sh b/tools/testing/selftests/firmware/fw_builtin.sh
new file mode 100755
index 000000000000..44e4198ed88f
--- /dev/null
+++ b/tools/testing/selftests/firmware/fw_builtin.sh
@@ -0,0 +1,69 @@
+#!/bin/bash
+# SPDX-License-Identifier: GPL-2.0
+# Test loading built-in firmware
+set -e
+
+TEST_REQS_FW_SYSFS_FALLBACK="no"
+TEST_REQS_FW_SET_CUSTOM_PATH="yes"
+TEST_DIR=$(dirname $0)
+source $TEST_DIR/fw_lib.sh
+
+check_mods
+check_setup
+verify_reqs
+setup_tmp_file
+
+trap "test_finish" EXIT
+
+if [ "$HAS_FW_LOADER_USER_HELPER" = "yes" ]; then
+	# Turn down the timeout so failures don't take so long.
+	echo 1 >/sys/class/firmware/timeout
+fi
+
+# built-in firmware support can be optional to test
+if [[ "$HAS_FW_LOADER_BUILTIN" != "yes" || "$HAS_TEST_FIRMWARE_BUILTIN" != "yes" ]]; then
+	exit $ksft_skip
+fi
+
+echo "Testing builtin firmware API ... "
+
+config_trigger_builtin()
+{
+	echo -n 1 > $DIR/trigger_request_builtin
+}
+
+test_builtin_firmware()
+{
+	echo -n "Testing firmware_request_builtin() ... "
+	config_reset
+	config_set_name $TEST_FIRMWARE_BUILTIN_FILENAME
+	config_trigger_builtin
+	echo OK
+	# Verify the contents are what we expect.
+	echo -n "Verifying file integrity ..."
+	if ! diff -q "$FW" /dev/test_firmware >/dev/null ; then
+		echo "$0: firmware loaded content differs" >&2
+		exit 1
+	else
+		echo "firmware content matches what we expect - OK"
+	fi
+}
+
+test_builtin_firmware_nofile()
+{
+	echo -n "Testing firmware_request_builtin() with fake file... "
+	config_reset
+	config_set_name fake-${TEST_FIRMWARE_BUILTIN_FILENAME}
+	if config_trigger_builtin 2> /dev/null; then
+		echo "$0: firmware shouldn't have loaded" >&2
+	fi
+	echo "OK"
+}
+
+test_builtin_firmware
+test_builtin_firmware_nofile
+
+# Ensure test_fw_config->is_builtin is set back to false
+# otherwise we won't be able to diff against the right target
+# firmware for other tests.
+config_reset
diff --git a/tools/testing/selftests/firmware/fw_lib.sh b/tools/testing/selftests/firmware/fw_lib.sh
index 31b71fe11dc5..bbae33d9f5ed 100755
--- a/tools/testing/selftests/firmware/fw_lib.sh
+++ b/tools/testing/selftests/firmware/fw_lib.sh
@@ -15,6 +15,10 @@ TEST_DIR=$(dirname $0)
 # To reproduce rename this to test-firmware.bin
 TEST_FIRMWARE_INTO_BUF_FILENAME=test-firmware-into-buf.bin
 
+# We should use a different filename for built-in firmware otherwise
+# we'd always have the file present.
+TEST_FIRMWARE_BUILTIN_FILENAME=test-builtin-firmware.bin
+
 # Kselftest framework requirement - SKIP code is 4.
 ksft_skip=4
 
@@ -63,6 +67,9 @@ 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_USER_HELPER="$(kconfig_has CONFIG_FW_LOADER_USER_HELPER=y)"
+	HAS_FW_LOADER_BUILTIN="$(kconfig_has CONFIG_FW_LOADER=y)"
+	HAS_TEST_FIRMWARE_BUILTIN="$(kconfig_has CONFIG_TEST_FIRMWARE_BUILTIN=y)"
 	PROC_FW_IGNORE_SYSFS_FALLBACK="0"
 	PROC_FW_FORCE_SYSFS_FALLBACK="0"
 
diff --git a/tools/testing/selftests/firmware/fw_run_tests.sh b/tools/testing/selftests/firmware/fw_run_tests.sh
index 777377078d5e..08a9bf043333 100755
--- a/tools/testing/selftests/firmware/fw_run_tests.sh
+++ b/tools/testing/selftests/firmware/fw_run_tests.sh
@@ -65,6 +65,8 @@ echo "Running namespace test: "
 $TEST_DIR/fw_namespace $DIR/trigger_request
 echo "OK"
 
+$TEST_DIR/fw_builtin.sh
+
 if [ -f $FW_FORCE_SYSFS_FALLBACK ]; then
 	run_test_config_0001
 	run_test_config_0002
-- 
2.30.2

Re: [PATCH v2 08/10] firmware_loader: move builtin build helper to shared library

[Greg KH]

On Thu, Oct 21, 2021 at 08:58:41AM -0700, Luis R. Rodriguez wrote:
> From: Luis Chamberlain <mcgrof@kernel.org>
> 
> If we wanted to use a different directory for building target
> builtin firmware it is easier if we just have a shared library
> Makefile, and each target directory can then just include it and
> populate the respective needed variables. This reduces clutter,
> makes things easier to read, and also most importantly allows
> us to not have to try to magically adjust only one target
> kconfig symbol for built-in firmware files. Trying to do this
> can easily end up causing odd build issues if the user is not
> careful.
> 
> As an example issue, if we are going to try to extend the
> FW_LOADER_BUILTIN_FILES list and FW_LOADER_BUILTIN_DIR in case
> of a new test firmware builtin support currently our only option
> would be modify the defaults of each of these in case test firmware
> builtin support was enabled. Defaults however won't augment a prior
> setting, and so if FW_LOADER_BUILTIN_DIR="/lib/firmware" and you
> and want this to be changed to something like
> FW_LOADER_BUILTIN_DIR="drivers/base/firmware_loader/test-builtin"
> the change will not take effect as a prior build already had it
> set, and the build would fail. Trying to augment / append the
> variables in the Makefile just makes this very difficult to
> read.
> 
> Using a library let's us split up possible built-in targets so
> that the user does not have to be involved. This will be used
> in a subsequent patch which will add another user to this
> built-in firmware library Makefile and demo how to use it outside
> of the default FW_LOADER_BUILTIN_DIR and FW_LOADER_BUILTIN_FILES.
> 
> Signed-off-by: Luis Chamberlain <mcgrof@kernel.org>

I'm sorry, but I do not understand the need for this change at all.  You
are now building this as a library, but what uses this library?  The
patches after this series are just testing patches, to verify that the
code previous in this series is working correctly, it should not depend
on a new library that only the testing code requires, right?

confused,

greg k-h

Re: [PATCH v2 07/10] firmware_loader: rename EXTRA_FIRMWARE and EXTRA_FIRMWARE_DIR

[Greg KH]

On Thu, Oct 21, 2021 at 08:58:40AM -0700, Luis R. Rodriguez wrote:
> From: Luis Chamberlain <mcgrof@kernel.org>
> 
> Now that we've tied loose ends on the built-in firmware API,
> rename the kconfig symbols for it to reflect more that they are
> associated to the firmware_loader and to make it easier to
> understand what they are for.
> 
> Signed-off-by: Luis Chamberlain <mcgrof@kernel.org>
> ---
>  .../driver-api/firmware/built-in-fw.rst       |  6 ++--
>  Documentation/x86/microcode.rst               |  8 ++---
>  arch/x86/Kconfig                              |  4 +--
>  drivers/base/firmware_loader/Kconfig          | 29 ++++++++++++-------
>  drivers/base/firmware_loader/builtin/Makefile |  6 ++--
>  drivers/staging/media/av7110/Kconfig          |  4 +--
>  6 files changed, 33 insertions(+), 24 deletions(-)
> 
> diff --git a/Documentation/driver-api/firmware/built-in-fw.rst b/Documentation/driver-api/firmware/built-in-fw.rst
> index bc1c961bace1..a9a0ab8c9512 100644
> --- a/Documentation/driver-api/firmware/built-in-fw.rst
> +++ b/Documentation/driver-api/firmware/built-in-fw.rst
> @@ -8,11 +8,11 @@ the filesystem. Instead, firmware can be looked for inside the kernel
>  directly. You can enable built-in firmware using the kernel configuration
>  options:
>  
> -  * CONFIG_EXTRA_FIRMWARE
> -  * CONFIG_EXTRA_FIRMWARE_DIR
> +  * CONFIG_FW_LOADER_BUILTIN_FILES
> +  * CONFIG_FW_LOADER_BUILTIN_DIR
>  
>  There are a few reasons why you might want to consider building your firmware
> -into the kernel with CONFIG_EXTRA_FIRMWARE:
> +into the kernel with CONFIG_FW_LOADER_BUILTIN_FILES:
>  
>  * Speed
>  * Firmware is needed for accessing the boot device, and the user doesn't
> diff --git a/Documentation/x86/microcode.rst b/Documentation/x86/microcode.rst
> index a320d37982ed..2cacc7f60014 100644
> --- a/Documentation/x86/microcode.rst
> +++ b/Documentation/x86/microcode.rst
> @@ -114,13 +114,13 @@ Builtin microcode
>  =================
>  
>  The loader supports also loading of a builtin microcode supplied through
> -the regular builtin firmware method CONFIG_EXTRA_FIRMWARE. Only 64-bit is
> -currently supported.
> +the regular builtin firmware method using CONFIG_FW_LOADER_BUILTIN and
> +CONFIG_FW_LOADER_BUILTIN_FILES. Only 64-bit is currently supported.
>  
>  Here's an example::
>  
> -  CONFIG_EXTRA_FIRMWARE="intel-ucode/06-3a-09 amd-ucode/microcode_amd_fam15h.bin"
> -  CONFIG_EXTRA_FIRMWARE_DIR="/lib/firmware"
> +  CONFIG_FW_LOADER_BUILTIN_FILES="intel-ucode/06-3a-09 amd-ucode/microcode_amd_fam15h.bin"
> +  CONFIG_FW_LOADER_BUILTIN_DIR="/lib/firmware"
>  
>  This basically means, you have the following tree structure locally::
>  
> diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
> index 999907dd7544..cfb09dc7f21b 100644
> --- a/arch/x86/Kconfig
> +++ b/arch/x86/Kconfig
> @@ -1315,8 +1315,8 @@ config MICROCODE
>  	  initrd for microcode blobs.
>  
>  	  In addition, you can build the microcode into the kernel. For that you
> -	  need to add the vendor-supplied microcode to the CONFIG_EXTRA_FIRMWARE
> -	  config option.
> +	  need to add the vendor-supplied microcode to the configuration option
> +	  CONFIG_FW_LOADER_BUILTIN_FILES
>  
>  config MICROCODE_INTEL
>  	bool "Intel microcode loading support"
> diff --git a/drivers/base/firmware_loader/Kconfig b/drivers/base/firmware_loader/Kconfig
> index 5b24f3959255..2dc3e137d903 100644
> --- a/drivers/base/firmware_loader/Kconfig
> +++ b/drivers/base/firmware_loader/Kconfig
> @@ -22,14 +22,14 @@ config FW_LOADER
>  	  You typically want this built-in (=y) but you can also enable this
>  	  as a module, in which case the firmware_class module will be built.
>  	  You also want to be sure to enable this built-in if you are going to
> -	  enable built-in firmware (CONFIG_EXTRA_FIRMWARE).
> +	  enable built-in firmware (CONFIG_FW_LOADER_BUILTIN_FILES).
>  
>  if FW_LOADER
>  
>  config FW_LOADER_PAGED_BUF
>  	bool
>  
> -config EXTRA_FIRMWARE
> +config FW_LOADER_BUILTIN_FILES
>  	string "Build named firmware blobs into the kernel binary"
>  	help
>  	  Device drivers which require firmware can typically deal with
> @@ -43,14 +43,21 @@ config EXTRA_FIRMWARE
>  	  in boot and cannot rely on the firmware being placed in an initrd or
>  	  initramfs.
>  
> -	  This option is a string and takes the (space-separated) names of the
> +	  Support for built-in firmware is not supported if you are using
> +	  the firmware loader as a module.
> +
> +	  This option is a string and takes the space-separated names of the
>  	  firmware files -- the same names that appear in MODULE_FIRMWARE()
>  	  and request_firmware() in the source. These files should exist under
> -	  the directory specified by the EXTRA_FIRMWARE_DIR option, which is
> +	  the directory specified by the FW_LOADER_BUILTIN_DIR option, which is
>  	  /lib/firmware by default.
>  
> -	  For example, you might set CONFIG_EXTRA_FIRMWARE="usb8388.bin", copy
> -	  the usb8388.bin file into /lib/firmware, and build the kernel. Then
> +	  For example, you might have set:
> +
> +	  CONFIG_FW_LOADER_BUILTIN_FILES="usb8388.bin"
> +
> +	  After this you would copy the usb8388.bin file into directory
> +	  specified by FW_LOADER_BUILTIN_DIR and build the kernel. Then
>  	  any request_firmware("usb8388.bin") will be satisfied internally
>  	  inside the kernel without ever looking at your filesystem at runtime.
>  
> @@ -60,13 +67,15 @@ config EXTRA_FIRMWARE
>  	  image since it combines both GPL and non-GPL work. You should
>  	  consult a lawyer of your own before distributing such an image.
>  
> -config EXTRA_FIRMWARE_DIR
> -	string "Firmware blobs root directory"
> +config FW_LOADER_BUILTIN_DIR
> +	string "Directory with firmware to be built-in to the kernel"
>  	depends on EXTRA_FIRMWARE != ""

You forgot to update this dependency :(

I took the first 6 patches here, this one needs work.

thanks,

greg k-h

Re: [PATCH v2 08/10] firmware_loader: move builtin build helper to shared library

[Luis Chamberlain]

On Fri, Oct 22, 2021 at 02:13:38PM +0200, Greg KH wrote:
> On Thu, Oct 21, 2021 at 08:58:41AM -0700, Luis R. Rodriguez wrote:
> > From: Luis Chamberlain <mcgrof@kernel.org>
> > 
> > If we wanted to use a different directory for building target
> > builtin firmware it is easier if we just have a shared library
> > Makefile, and each target directory can then just include it and
> > populate the respective needed variables. This reduces clutter,
> > makes things easier to read, and also most importantly allows
> > us to not have to try to magically adjust only one target
> > kconfig symbol for built-in firmware files. Trying to do this
> > can easily end up causing odd build issues if the user is not
> > careful.
> > 
> > As an example issue, if we are going to try to extend the
> > FW_LOADER_BUILTIN_FILES list and FW_LOADER_BUILTIN_DIR in case
> > of a new test firmware builtin support currently our only option
> > would be modify the defaults of each of these in case test firmware
> > builtin support was enabled. Defaults however won't augment a prior
> > setting, and so if FW_LOADER_BUILTIN_DIR="/lib/firmware" and you
> > and want this to be changed to something like
> > FW_LOADER_BUILTIN_DIR="drivers/base/firmware_loader/test-builtin"
> > the change will not take effect as a prior build already had it
> > set, and the build would fail. Trying to augment / append the
> > variables in the Makefile just makes this very difficult to
> > read.
> > 
> > Using a library let's us split up possible built-in targets so
> > that the user does not have to be involved. This will be used
> > in a subsequent patch which will add another user to this
> > built-in firmware library Makefile and demo how to use it outside
> > of the default FW_LOADER_BUILTIN_DIR and FW_LOADER_BUILTIN_FILES.
> > 
> > Signed-off-by: Luis Chamberlain <mcgrof@kernel.org>
> 
> I'm sorry, but I do not understand the need for this change at all.  You
> are now building this as a library, but what uses this library?  The
> patches after this series are just testing patches, to verify that the
> code previous in this series is working correctly, it should not depend
> on a new library that only the testing code requires, right?

The last patch adds support to test built-in firmware, but most kernels
will have and do want EXTRA_FIRMWARE="", and so there cannot be anything
that can be tested. And so we need aother two pair of kconfig symbols
which are independent to test built-in firmware. The reason for this is
explained in the commit log, if we try to augment the EXTRA_FIRMWARE
when enabling testing built-in firmware we can easily end up with odd
build issues.

So this patch moves the logic to enable us to re-use the same built-in
magic for two independent kconfig test symbols.

  Luis