Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <baf7adb1-68c4-54f3-c1f6-e5ccf9258719@intel.com>
Date:   Mon, 14 Nov 2022 11:49:53 -0800
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101
 Firefox/91.0 Thunderbird/91.11.0
Subject: Re: ld.lld: error: undefined symbol: firmware_upload_register
Content-Language: en-US
To:     Randy Dunlap <rdunlap@infradead.org>,
        kernel test robot <lkp@intel.com>,
        Luis Chamberlain <mcgrof@kernel.org>,
        Greg Kroah-Hartman <gregkh@linuxfoundation.org>
CC:     <llvm@lists.linux.dev>, <oe-kbuild-all@lists.linux.dev>,
        <linux-kernel@vger.kernel.org>, Xu Yilun <yilun.xu@intel.com>
References: <202211092004.GI7IL067-lkp@intel.com>
 <d81630d5-756c-4067-fc1b-47e5f56cad33@infradead.org>
From:   Russ Weight <russell.h.weight@intel.com>
In-Reply-To: <d81630d5-756c-4067-fc1b-47e5f56cad33@infradead.org>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: 7bit
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?ZCtJMFAzZzNvRm1DMEthUHdXQ1A4VEUzZmhZTGpML1V1c21lMkhJdHN5OEsx?=
 =?utf-8?B?VmpUMU1CMDUvbDJUeHJKL0RkR3VlcU9iVHBDd1YvZDRHWno4M1NjUDAwb3BQ?=
 =?utf-8?B?VHJ1NnIwTm91RHVkZGVNU2k4WVYwZURFZGJNV2dVdEF1ZVd1NENQL1ZOMlU3?=
 =?utf-8?B?R042WDExY1ViV2IrVE5ncXUzVFZ3NGFPeFlVRTQ2SFBpSkxpTEJsUEhPSWpt?=
 =?utf-8?B?Tk5VYTBBYjlSOWJ2TUhBdklnTnV6UEhxNWRVTnBFemxLTEhhbEVsMk9ic2tv?=
 =?utf-8?B?alRidVhxS2tQemZMUTZhRE1iRHNWaFArM3QrakJYLzlRZThRS0YzaDBpclpq?=
 =?utf-8?B?QzA5bk1CVlRnZWxJMXZzWFVjR0lHekdkMEJiZjQzRm1adXhEUWI2dFlpRmdD?=
 =?utf-8?B?cDhwNkcvQmNBdTFBbS83c1NrYzVUMWh4MC8yN2NvTFE5d3huREttSHI5WGgv?=
 =?utf-8?B?Y292c3pJZmFTeWlhS3BSSTd2bnBuVmlISlhvYXBUd0pUSUFWWFF0enV3WmVL?=
 =?utf-8?B?YVQrQUJwVGx4UThFMWRPYTJ2K2hXT1dDbTF4NCt5Wk9qeDRrZ2xTcDhJVEw1?=
 =?utf-8?B?dUk1MU5ka3RUc2JYM2RKMklKdmZYemdzNEROQ2M0M3hubkR6ZWRDTWdOSWtQ?=
 =?utf-8?B?cEd4UVJGSzRLUTFPNk5KV2Jjay9ydERMRVRMVjUrUmhxOUhEL0x0NFd5ZmtP?=
 =?utf-8?B?TDN3dk9zZmVnSEsrZjlkT0wyMTYzWU9hWmNSTW0vZFV1aWNHVmoyT1pZWnFQ?=
 =?utf-8?B?OHJsTWtQbDEwQ0h4SGhGMTZacHpMcG5TQlpNdWNib0grcngyQnQ0dW9DTFgw?=
 =?utf-8?B?SVIwZUxDdUhvekJPTjgxb0k4NzJ5QTdjaFk4Y0dBbUxTUm41bXBXOFZmWXVq?=
 =?utf-8?B?U1pNS0xCYlZzNU1FQXBqWGZwVGY5Nk9pbndjUmNoOEtXSWVxV2JRVGNkbFJW?=
 =?utf-8?B?NUtLYStwSkx6RjBkNGJwN1RtVDZoZG9PVWh4b1JFSVpoTFUvQkVNNUJRWEFs?=
 =?utf-8?B?c1haTE9ubmxqdVlqNU93RTVEY2FjemJsK0tHcEFsekJRWWlDVzNHYlZhMFFU?=
 =?utf-8?B?NlJPeGVSbXgrVm1HV2x4NzFyRGgrajVud0RVY0RhZ2FGMzkyWGVROFNwVTJL?=
 =?utf-8?B?eXB5UXEzM3FyT2JTaXJoTFVaa2MyL1V5T1JFeEt5dU4wSnlsMU14elNCU1BM?=
 =?utf-8?B?dEdSUmMxRUlZRGRjRFkveXZFYStCby9EQUJyT3pCcHk2dTdHU0pFTFdvcjlZ?=
 =?utf-8?B?bXh1bnRrSmhDTmNISHFHc2gzS1k0VzVOYVhaT2VMYnlBeC9xZktCZEo1a05M?=
 =?utf-8?B?TEdJZStXT1JzYVc3UFF6SHRRM08xd2RQM250WEMxSVFPM0N2K1MyWWhhWUlY?=
 =?utf-8?B?TUQ3SjduMFBKZjFCdU93Y3YrdXBjRFJMWVJyV1gzMzlVR1BRcnZWTFBySGNW?=
 =?utf-8?B?SDgyNWRaRFlZbGNYUVNDVWxEeGZZcFI3Kzl0L0EzWi9haTEwOTNsNFhXbU9V?=
 =?utf-8?B?QmRJVnpEMUhHU3BRN0xxakRCK0J0ZUlvUm1MZ212cmt5Y2Q1WTd3OU5iM2Zx?=
 =?utf-8?B?Y01zUTVvT3dFYVMvdVQ3MFhDTUFTU0ljeWtFMFFxL0FrdmNwL2o5dGduWW02?=
 =?utf-8?B?V2R4MUl4NFFwbXNzd2IrOTNsVHVJOUVmQ3Bwa1IzUkFrWmRLU28zQytYMVR0?=
 =?utf-8?B?TURnL2ljSTMyWHg3SDV6VmJTdzJpMXMzVG5aN3lXcnZIQzZtZEpCM1FZRCtY?=
 =?utf-8?B?MUN5aVBxc0c2c2padGdwOHhjNWNLbDhYdS9BTm93c3hIcHVRSlJjWks4RGFL?=
 =?utf-8?B?TFZ1b3lvQXpMeExOVGFCRW5oQlJKSmdBTVowRUNKaWQxL0dEdXlsZmtMSEpw?=
 =?utf-8?B?THdDN291UnlRbWtDdlBuRXU5Wk51M3NJREVoby9KQUd2K0p2NjFRVUNHVnU0?=
 =?utf-8?B?Yk9ocFljU082THhBSndVbi9Sanl6aGtnT2F6QVIzbDZvRmhyMWNmUEV5MDZO?=
 =?utf-8?B?ci9EMzN3STRTTDJuU0NDL0o4MUpielRlc1ZKMnVJeWNNOHlxaXdmMUFya1ZQ?=
 =?utf-8?B?KzhCdzlMMHF3SkN6anR6RkJkcFVNTnVYVnViOVdRL2ZUd0tBaWJBbXdoN3hv?=
 =?utf-8?B?ZkFYc2dTL1dkd0EwZUxZb21kWERSV2dUcUMrQWxmS2p1a0VTTExjODdKU1JZ?=
 =?utf-8?B?NGc9PQ==?=
X-MS-Exchange-CrossTenant-Network-Message-Id: e362f517-1932-41bc-f297-08dac6796856
X-MS-Exchange-CrossTenant-AuthSource: DM5PR11MB1899.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 14 Nov 2022 19:49:58.2560
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 46c98d88-e344-4ed4-8496-4ed7712e255d
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: irHIP9gOUk29D9F0nczdgeTdTOQTzQ38B/bgxjbOLjj82DdCO+N1yhnsGtI7XNsqW/98grMb6W6HOt+US9vWXTeR0f2Y/uyjVFmrMfXxqeI=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: IA0PR11MB7377
X-OriginatorOrg: intel.com
X-Spam-Status: No, score=-4.4 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH,
        DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,NICE_REPLY_A,
        RCVD_IN_DNSWL_MED,RCVD_IN_MSPIKE_H3,RCVD_IN_MSPIKE_WL,SPF_HELO_NONE,
        SPF_NONE autolearn=ham autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on
        lindbergh.monkeyblade.net
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org


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