linux-kernel - Re: ld.lld: error: undefined symbol: iosf_mbi

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <87frs6vj4m.wl-tiwai@suse.de>
Date: Thu, 18 Jul 2024 17:02:01 +0200
From: Takashi Iwai <tiwai@...e.de>
To: Pierre-Louis Bossart <pierre-louis.bossart@...ux.intel.com>
Cc: Takashi Iwai <tiwai@...e.de>,
	Nathan Chancellor <nathan@...nel.org>,
	kernel test robot <lkp@...el.com>,
	llvm@...ts.linux.dev,
	oe-kbuild-all@...ts.linux.dev,
	linux-kernel@...r.kernel.org,
	Kai Vehmanen <kai.vehmanen@...ux.intel.com>,
	Guennadi Liakhovetski <guennadi.liakhovetski@...ux.intel.com>,
	Bard Liao <bard.liao@...el.com>
Subject: Re: ld.lld: error: undefined symbol: iosf_mbi_available

On Thu, 18 Jul 2024 11:12:24 +0200,
Pierre-Louis Bossart wrote:
> 
> 
> 
> >>> --- a/sound/soc/sof/intel/Kconfig
> >>> +++ b/sound/soc/sof/intel/Kconfig
> >>> @@ -19,6 +19,7 @@ config SND_SOC_SOF_INTEL_ATOM_HIFI_EP
> >>>  	tristate
> >>>  	select SND_SOC_SOF_INTEL_COMMON
> >>>  	select SND_SOC_SOF_INTEL_HIFI_EP_IPC
> >>> +	select IOSF_MBI if X86 && PCI
> >>>  	help
> >>>  	  This option is not user-selectable but automagically handled by
> >>>  	  'select' statements at a higher level.
> >>> @@ -44,7 +45,6 @@ config SND_SOC_SOF_BAYTRAIL
> >>>  	select SND_SOC_SOF_INTEL_COMMON
> >>>  	select SND_SOC_SOF_INTEL_ATOM_HIFI_EP
> >>>  	select SND_SOC_SOF_ACPI_DEV
> >>> -	select IOSF_MBI if X86 && PCI
> >>>  	help
> >>>  	  This adds support for Sound Open Firmware for Intel(R) platforms
> >>>  	  using the Baytrail, Braswell or Cherrytrail processors.
> >>
> >> I don't think it's the 'right' fix Takashi.
> >>
> >> The problem is that we end-up using the iosf_mbi_read() routine by
> >> including the soc-intel-quirks.h header file blindly for all X66
> >> platforms - even when Baytrail is not used.
> >>
> >> Adding IOSF support for Tangiger doesn't seem right to me, it's not a
> >> real dependency.
> >>
> >> We can be more restrictive and only use the helper for Baytrail, and use
> >> a fallback if Baytrail is not used.
> >>
> >> diff --git a/sound/soc/intel/common/soc-intel-quirks.h
> >> b/sound/soc/intel/common/soc-intel-quirks.h
> >> index de4e550c5b34..ae67853f7e2e 100644
> >> --- a/sound/soc/intel/common/soc-intel-quirks.h
> >> +++ b/sound/soc/intel/common/soc-intel-quirks.h
> >> @@ -11,7 +11,9 @@
> >>
> >>  #include <linux/platform_data/x86/soc.h>
> >>
> >> -#if IS_ENABLED(CONFIG_X86)
> >> +#if IS_ENABLED(CONFIG_X86) && \
> >> +       (IS_ENABLED(CONFIG_SND_SOC_SOF_BAYTRAIL) || \
> >> +        IS_ENABLED(CONFIG_SND_SST_ATOM_HIFI2_PLATFORM_ACPI))
> >>
> >>  #include <linux/dmi.h>
> >>  #include <asm/iosf_mbi.h>
> >>
> >> also at https://github.com/thesofproject/linux/pull/5114
> > 
> > I'm afraid it's not enough, either.  It's included in
> > sound/soc/sof/intel/atom.c, and this one can be built-in by selected
> > from others while CONFIG_SND_SOC_SOF_BAYTRAIL=m.  And, the reverse
> > selection is done from CONFIG_SND_SOC_SOF_BAYTRAIL -- so
> > CONFIG_IOSF_MBI can be m as well, and this can lead to the unresolved
> > symbol from the built-in atom.c.
> 
> Fair point, I was only looking at the reported failure where Baytrail
> was completely disabled.
> 
> I am not sure though if it makes sense to split hair in N dimensions.
> Building Merrifield as y and Baytrail as m is a corner case that
> shouldn't exist at all. And it's only an academic compilation issue, in
> practice using 'y' would fail at run-time due to the usual firmware load
> dependencies...

Surely this kind of bug won't hit anyone in practical use, but it's
only about the randconfig failures.  The original report is in the
same category, after all.

Maybe another (rather easier) workaround would be to use
IS_REACHABLE(), something like below.  This should fix the original
issue and the potential mess-up of kconfig dependencies.


Takashi

--- a/sound/soc/intel/common/soc-intel-quirks.h
+++ b/sound/soc/intel/common/soc-intel-quirks.h
@@ -11,7 +11,7 @@
 
 #include <linux/platform_data/x86/soc.h>
 
-#if IS_ENABLED(CONFIG_X86)
+#if IS_REACHABLE(CONFIG_IOSF_MBI)
 
 #include <linux/dmi.h>
 #include <asm/iosf_mbi.h>