[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <baf7adb1-68c4-54f3-c1f6-e5ccf9258719@intel.com>
Date: Mon, 14 Nov 2022 11:49:53 -0800
From: Russ Weight <russell.h.weight@...el.com>
To: Randy Dunlap <rdunlap@...radead.org>,
kernel test robot <lkp@...el.com>,
Luis Chamberlain <mcgrof@...nel.org>,
Greg Kroah-Hartman <gregkh@...uxfoundation.org>
CC: <llvm@...ts.linux.dev>, <oe-kbuild-all@...ts.linux.dev>,
<linux-kernel@...r.kernel.org>, Xu Yilun <yilun.xu@...el.com>
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 <lkp@...el.com>
>>
>> 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
Powered by blists - more mailing lists