Re: ld.lld: error: undefined symbol: firmware_upload_register

[Russ Weight <russell.h.weight@intel.com>]

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@intel.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