2022-11-09 12:52:59

by kernel test robot

[permalink] [raw]
Subject: ld.lld: error: undefined symbol: firmware_upload_register

tree: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git master
head: f141df371335645ce29a87d9683a3f79fba7fd67
commit: 5cd339b370e29b04b85fbb83f40496991465318e fpga: m10bmc-sec: add max10 secure update functions
date: 5 months ago
config: mips-buildonly-randconfig-r004-20221109
compiler: clang version 16.0.0 (https://github.com/llvm/llvm-project 463da45892e2d2a262277b91b96f5f8c05dc25d0)
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# install mips cross compiling tool for clang build
# apt-get install binutils-mips-linux-gnu
# https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=5cd339b370e29b04b85fbb83f40496991465318e
git remote add linus https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
git fetch --no-tags linus master
git checkout 5cd339b370e29b04b85fbb83f40496991465318e
# save the config file
mkdir build_dir && cp config build_dir/.config
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=mips SHELL=/bin/bash

If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <[email protected]>

All errors (new ones prefixed by >>):

>> ld.lld: error: undefined symbol: firmware_upload_register
>>> referenced by intel-m10-bmc-sec-update.c
>>> fpga/intel-m10-bmc-sec-update.o:(m10bmc_sec_probe) in archive drivers/built-in.a
--
>> ld.lld: error: undefined symbol: firmware_upload_unregister
>>> referenced by intel-m10-bmc-sec-update.c
>>> fpga/intel-m10-bmc-sec-update.o:(m10bmc_sec_remove) in archive drivers/built-in.a

--
0-DAY CI Kernel Test Service
https://01.org/lkp


Attachments:
(No filename) (1.84 kB)
config (156.35 kB)
Download all attachments

2022-11-11 22:18:00

by Randy Dunlap

[permalink] [raw]
Subject: Re: ld.lld: error: undefined symbol: firmware_upload_register

Hi--

On 11/9/22 04:30, kernel test robot wrote:
> tree: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git master
> head: f141df371335645ce29a87d9683a3f79fba7fd67
> commit: 5cd339b370e29b04b85fbb83f40496991465318e fpga: m10bmc-sec: add max10 secure update functions
> date: 5 months ago
> config: mips-buildonly-randconfig-r004-20221109
> compiler: clang version 16.0.0 (https://github.com/llvm/llvm-project 463da45892e2d2a262277b91b96f5f8c05dc25d0)
> reproduce (this is a W=1 build):
> wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
> chmod +x ~/bin/make.cross
> # install mips cross compiling tool for clang build
> # apt-get install binutils-mips-linux-gnu
> # https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=5cd339b370e29b04b85fbb83f40496991465318e
> git remote add linus https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
> git fetch --no-tags linus master
> git checkout 5cd339b370e29b04b85fbb83f40496991465318e
> # save the config file
> mkdir build_dir && cp config build_dir/.config
> COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=mips SHELL=/bin/bash
>
> If you fix the issue, kindly add following tag where applicable
> | Reported-by: kernel test robot <[email protected]>
>
> All errors (new ones prefixed by >>):
>
>>> ld.lld: error: undefined symbol: firmware_upload_register
> >>> referenced by intel-m10-bmc-sec-update.c
> >>> fpga/intel-m10-bmc-sec-update.o:(m10bmc_sec_probe) in archive drivers/built-in.a
> --
>>> ld.lld: error: undefined symbol: firmware_upload_unregister
> >>> referenced by intel-m10-bmc-sec-update.c
> >>> fpga/intel-m10-bmc-sec-update.o:(m10bmc_sec_remove) in archive drivers/built-in.a


This happens when (bool) CONFIG_FW_UPLOAD=y and CONFIG_FW_LOADER=m.
Ah, it's the old "bool depending on a tristate" problem.

There may be some Makefile fix for this - I don't know.

Or the Kconfig fix works AFAICT (below). Change FW_UPLOAD to a tristate
and modify 2 header files for exports/inlines.

Luis -- how was FW_LOADER=m and FW_UPLOAD=y supposed to work?

Thanks.
--
---
drivers/base/firmware_loader/Kconfig | 2 +-
drivers/base/firmware_loader/sysfs.h | 2 +-
include/linux/firmware.h | 2 +-
3 files changed, 3 insertions(+), 3 deletions(-)

diff -- a/drivers/base/firmware_loader/Kconfig b/drivers/base/firmware_loader/Kconfig
--- a/drivers/base/firmware_loader/Kconfig
+++ b/drivers/base/firmware_loader/Kconfig
@@ -204,7 +204,7 @@ config FW_CACHE
If unsure, say Y.

config FW_UPLOAD
- bool "Enable users to initiate firmware updates using sysfs"
+ tristate "Enable users to initiate firmware updates using sysfs"
select FW_LOADER_SYSFS
select FW_LOADER_PAGED_BUF
help
diff -- a/drivers/base/firmware_loader/sysfs.h b/drivers/base/firmware_loader/sysfs.h
--- a/drivers/base/firmware_loader/sysfs.h
+++ b/drivers/base/firmware_loader/sysfs.h
@@ -99,7 +99,7 @@ struct fw_sysfs *
fw_create_instance(struct firmware *firmware, const char *fw_name,
struct device *device, u32 opt_flags);

-#ifdef CONFIG_FW_UPLOAD
+#if IS_ENABLED(CONFIG_FW_UPLOAD)
extern struct device_attribute dev_attr_status;
extern struct device_attribute dev_attr_error;
extern struct device_attribute dev_attr_cancel;
diff -- a/include/linux/firmware.h b/include/linux/firmware.h
--- a/include/linux/firmware.h
+++ b/include/linux/firmware.h
@@ -170,7 +170,7 @@ static inline int request_partial_firmwa

#endif

-#ifdef CONFIG_FW_UPLOAD
+#if IS_ENABLED(CONFIG_FW_UPLOAD)

struct fw_upload *
firmware_upload_register(struct module *module, struct device *parent,



2022-11-14 20:12:04

by Russ Weight

[permalink] [raw]
Subject: Re: ld.lld: error: undefined symbol: firmware_upload_register



On 11/11/22 13:59, Randy Dunlap wrote:
> Hi--
>
> On 11/9/22 04:30, kernel test robot wrote:
>> tree: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git master
>> head: f141df371335645ce29a87d9683a3f79fba7fd67
>> commit: 5cd339b370e29b04b85fbb83f40496991465318e fpga: m10bmc-sec: add max10 secure update functions
>> date: 5 months ago
>> config: mips-buildonly-randconfig-r004-20221109
>> compiler: clang version 16.0.0 (https://github.com/llvm/llvm-project 463da45892e2d2a262277b91b96f5f8c05dc25d0)
>> reproduce (this is a W=1 build):
>> wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
>> chmod +x ~/bin/make.cross
>> # install mips cross compiling tool for clang build
>> # apt-get install binutils-mips-linux-gnu
>> # https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=5cd339b370e29b04b85fbb83f40496991465318e
>> git remote add linus https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
>> git fetch --no-tags linus master
>> git checkout 5cd339b370e29b04b85fbb83f40496991465318e
>> # save the config file
>> mkdir build_dir && cp config build_dir/.config
>> COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=mips SHELL=/bin/bash
>>
>> If you fix the issue, kindly add following tag where applicable
>> | Reported-by: kernel test robot <[email protected]>
>>
>> All errors (new ones prefixed by >>):
>>
>>>> ld.lld: error: undefined symbol: firmware_upload_register
>> >>> referenced by intel-m10-bmc-sec-update.c
>> >>> fpga/intel-m10-bmc-sec-update.o:(m10bmc_sec_probe) in archive drivers/built-in.a
>> --
>>>> ld.lld: error: undefined symbol: firmware_upload_unregister
>> >>> referenced by intel-m10-bmc-sec-update.c
>> >>> fpga/intel-m10-bmc-sec-update.o:(m10bmc_sec_remove) in archive drivers/built-in.a
>
> This happens when (bool) CONFIG_FW_UPLOAD=y and CONFIG_FW_LOADER=m.
> Ah, it's the old "bool depending on a tristate" problem.
>
> There may be some Makefile fix for this - I don't know.
>
> Or the Kconfig fix works AFAICT (below). Change FW_UPLOAD to a tristate
> and modify 2 header files for exports/inlines.
>
> Luis -- how was FW_LOADER=m and FW_UPLOAD=y supposed to work?

Hi Randy - thanks for chasing this down.

I used FW_LOADER_USER_HELPER as a pattern when I created FW_UPLOAD. It is
also defined as bool. Both of these are defined within "if FW_LOADER" which,
as you pointed out, is defined as tristate. The corresponding Makefile
gives a hint as to how these all play together:

obj-$(CONFIG_FW_LOADER) += firmware_class.o
firmware_class-objs := main.o
firmware_class-$(CONFIG_FW_LOADER_USER_HELPER) += fallback.o
firmware_class-$(CONFIG_EFI_EMBEDDED_FIRMWARE) += fallback_platform.o
firmware_class-$(CONFIG_FW_LOADER_SYSFS) += sysfs.o
firmware_class-$(CONFIG_FW_UPLOAD) += sysfs_upload.o

So FW_LOADER may be selected as a module, but the individual firmware_class
objects are enabled with bool configs.

I think the implementation of FW_UPLOAD config is consistent with the
other config options and that the error is caused by an incorrect
dependency for FPGA_M10_BMC_SEC_UPDATE.

I looked at how FW_LOADER_USER_HELPER is referenced as a dependency in
other Kconfig files and I found two instances, both of which do:

select FW_LOADER
select FW_LOADER_USER_HELPER

So I believe the right fix for this bug is to change the dependency for
FPGA_M10_BMC_SEC_UPDATE from:

depends on MFD_INTEL_M10_BMC_CORE && FW_UPLOAD

to

depends on MFD_INTEL_M10_BMC_CORE
select FW_LOADER
select FW_UPLOAD

I'll prepare and test a patch. Please let me know if you disagree.

Thanks,
- Russ