firmware_loader: built-in API and make x86 use it
Subsystems (11)
Luis R. Rodriguez
maintainerv2 cover firmware_loader: built-in API and make x86 use it
From: Luis Chamberlain <[email protected]>
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].
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
Signature · 1 line
2.30.2
Luis R. Rodriguez
maintainerPATCH v2 1/10 firmware_loader: formalize built-in firmware API
From: Luis Chamberlain <[email protected]>
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.
--- 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
drivers/base/firmware_loader/builtin/Makefile +4 -2
@@ -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))
drivers/base/firmware_loader/builtin/main.c new file +100 -0
100 lines not on this page · raw message
drivers/base/firmware_loader/firmware.h +17 -0
@@ -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);
drivers/base/firmware_loader/main.c +1 -77
90 lines not on this page · raw message
include/linux/firmware.h +15 -0
@@ -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);
Signature · 1 line
2.30.2
Luis R. Rodriguez
maintainerPATCH v2 2/10 firmware_loader: remove old DECLARE_BUILTIN_FIRMWARE()
In reply to Luis R. RodriguezFrom: Luis Chamberlain <[email protected]>
This was never used upstream. Time to get rid of it. We don't carry around unused baggage.
--- include/linux/firmware.h | 13 ------------- 1 file changed, 13 deletions(-)
include/linux/firmware.h +0 -13
@@ -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)
Signature · 1 line
2.30.2
Luis R. Rodriguez
maintainerPATCH v2 3/10 x86/microcode: Use the firmware_loader built-in API
In reply to Luis R. RodriguezFrom: Borislav Petkov <[email protected]>
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.
--- 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(-)
arch/x86/include/asm/microcode.h +0 -3
@@ -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 */
arch/x86/kernel/cpu/microcode/amd.c +10 -4
@@ -456,17 +456,23 @@ apply_microcode_early_amd(u32 cpuid_1_eax, void *ucode, size_t size, bool save_pstatic 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)
arch/x86/kernel/cpu/microcode/core.c +0 -17
@@ -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;
arch/x86/kernel/cpu/microcode/intel.c +8 -1
@@ -456,6 +456,7 @@ static void save_mc_for_early(struct ucode_cpu_info *uci, u8 *mc, unsigned int sstatic 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; }/*
Signature · 1 line
2.30.2
Luis R. Rodriguez
maintainerPATCH v2 4/10 firmware_loader: move struct builtin_fw to the only place used
In reply to Luis R. RodriguezFrom: Luis Chamberlain <[email protected]>
Now that x86 doesn't abuse picking at internals to the firmware loader move out the built-in firmware struct to its only user.
--- drivers/base/firmware_loader/builtin/main.c | 6 ++++++ include/linux/firmware.h | 6 ------ 2 files changed, 6 insertions(+), 6 deletions(-)
drivers/base/firmware_loader/builtin/main.c +6 -0
@@ -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[];
include/linux/firmware.h +0 -6
@@ -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,
Signature · 1 line
2.30.2
Luis R. Rodriguez
maintainerPATCH v2 5/10 vmlinux.lds.h: wrap built-in firmware support under FW_LOADER
In reply to Luis R. RodriguezFrom: Luis Chamberlain <[email protected]>
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.
--- include/asm-generic/vmlinux.lds.h | 20 +++++++++++++------- 1 file changed, 13 insertions(+), 7 deletions(-)
include/asm-generic/vmlinux.lds.h +13 -7
@@ -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); \
Signature · 1 line
2.30.2
Luis R. Rodriguez
maintainerPATCH v2 6/10 x86/build: Tuck away built-in firmware under FW_LOADER
In reply to Luis R. RodriguezFrom: Luis Chamberlain <[email protected]>
When FW_LOADER is modular or disabled we don't use it. Update x86 relocs to reflect that.
--- arch/x86/tools/relocs.c | 2 ++ 1 file changed, 2 insertions(+)
arch/x86/tools/relocs.c +2 -0
@@ -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|"
Signature · 1 line
2.30.2
Luis R. Rodriguez
maintainerPATCH v2 7/10 firmware_loader: rename EXTRA_FIRMWARE and EXTRA_FIRMWARE_DIR
In reply to Luis R. RodriguezFrom: Luis Chamberlain <[email protected]>
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.
--- .../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(-)
Documentation/driver-api/firmware/built-in-fw.rst +3 -3
@@ -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
Documentation/x86/microcode.rst +4 -4
@@ -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::
arch/x86/Kconfig +2 -2
@@ -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"
drivers/base/firmware_loader/Kconfig +19 -10
@@ -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"
drivers/base/firmware_loader/builtin/Makefile +3 -3
@@ -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,%,$@)
drivers/staging/media/av7110/Kconfig +2 -2
@@ -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.
Signature · 1 line
2.30.2
Greg KH
maintainerRe: [PATCH v2 07/10] firmware_loader: rename EXTRA_FIRMWARE and EXTRA_FIRMWARE_DIR
Quoted 121 lines · Luis R. Rodriguez
From: Luis Chamberlain <[email protected]>
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_loaderand to make it easier to understand what they are for.Signed-off-by Luis Chamberlain <[email protected]>--- .../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(-)Documentation/driver-api/firmware/built-in-fw.rst +3
-3@@ -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'tDocumentation/x86/microcode.rst +4
-4@@ -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::arch/x86/Kconfig +2
-2@@ -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"drivers/base/firmware_loader/Kconfig +13
-6@@ -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.
drivers/base/firmware_loader/Kconfig config EXTRA_FIRMWARE · line 72
Quoted 8 lines
drivers/base/firmware_loader/Kconfig +2
-2@@ -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
Luis R. Rodriguez
maintainerPATCH v2 8/10 firmware_loader: move builtin build helper to shared library
In reply to Luis R. RodriguezFrom: Luis Chamberlain <[email protected]>
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.
--- 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
drivers/base/firmware_loader/builtin/Makefile +2 -32
@@ -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))
drivers/base/firmware_loader/builtin/lib.Makefile new file +32 -0
@@ -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))
Signature · 1 line
2.30.2
Greg KH
maintainerRe: [PATCH v2 08/10] firmware_loader: move builtin build helper to shared library
Quoted 25 lines · Luis R. Rodriguez
From: Luis Chamberlain <[email protected]>
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_FILESlist andFW_LOADER_BUILTIN_DIRin 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 theMakefilejust makes this very difficult to read.
Quoted 7 lines
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
Makefileand demo how to use it outside of the defaultFW_LOADER_BUILTIN_DIRandFW_LOADER_BUILTIN_FILES.Signed-off-by Luis Chamberlain <[email protected]>
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
Luis Chamberlain
maintainerRe: [PATCH v2 08/10] firmware_loader: move builtin build helper to shared library
Quoted 32 lines · Greg KH
Quoted 31 lines · Luis R. Rodriguez
From: Luis Chamberlain <[email protected]>
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_FILESlist andFW_LOADER_BUILTIN_DIRin 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 theMakefilejust 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
Makefileand demo how to use it outside of the defaultFW_LOADER_BUILTIN_DIRandFW_LOADER_BUILTIN_FILES.
Quoted 7 lines
Quoted 1 line
Signed-off-by Luis Chamberlain <[email protected]>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
Luis R. Rodriguez
maintainerPATCH v2 9/10 test_firmware: move a few test knobs out to its library
In reply to Luis R. RodriguezFrom: Luis Chamberlain <[email protected]>
These will be used by other tests cases in other files so move them to the library.
--- .../testing/selftests/firmware/fw_filesystem.sh | 16 ---------------- tools/testing/selftests/firmware/fw_lib.sh | 17 +++++++++++++++++ 2 files changed, 17 insertions(+), 16 deletions(-)
tools/testing/selftests/firmware/fw_filesystem.sh +0 -16
@@ -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
tools/testing/selftests/firmware/fw_lib.sh +17 -0
@@ -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 +}
Signature · 1 line
2.30.2
Luis R. Rodriguez
maintainerPATCH v2 10/10 test_firmware: add support for testing built-in firmware
In reply to Luis R. RodriguezFrom: Luis Chamberlain <[email protected]>
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.
--- 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
drivers/base/firmware_loader/Makefile +1 -0
@@ -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
drivers/base/firmware_loader/builtin/Makefile +1 -0
@@ -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).
drivers/base/firmware_loader/test-builtin/.gitignore new file +3 -0
@@ -0,0 +1,3 @@ +# SPDX-License-Identifier: GPL-2.0 +*.gen.S +*.bin
drivers/base/firmware_loader/test-builtin/Makefile new file +18 -0
@@ -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)
lib/Kconfig.debug +33 -0
@@ -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
lib/test_firmware.c +51 -1
94 lines not on this page · raw message
tools/testing/selftests/firmware/fw_builtin.sh new file +69 -0
69 lines not on this page · raw message
tools/testing/selftests/firmware/fw_lib.sh +7 -0
@@ -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"
tools/testing/selftests/firmware/fw_run_tests.sh +2 -0
@@ -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
Signature · 1 line
2.30.2