diff mbox series

[v2,2/7] MIPS: Add toolchain feature dependency for microMIPS smartMIPS

Message ID 20230414080701.15503-3-jiaxun.yang@flygoat.com (mailing list archive)
State Superseded
Headers show
Series MIPS: LLVM toolchain support for more CPUs | expand

Commit Message

Jiaxun Yang April 14, 2023, 8:06 a.m. UTC 
microMIPS smartMIPS kernel can only be compiled if they are supported
by toolchain.

Signed-off-by: Jiaxun Yang <jiaxun.yang@flygoat.com>
Reviewed-by: Nick Desaulniers <ndesaulniers@google.com>
---
 arch/mips/Kconfig | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

Comments

Thomas Bogendoerfer April 18, 2023, 1:08 p.m. UTC | #1 
On Fri, Apr 14, 2023 at 09:06:56AM +0100, Jiaxun Yang wrote:
> microMIPS smartMIPS kernel can only be compiled if they are supported
> by toolchain.
> 
> Signed-off-by: Jiaxun Yang <jiaxun.yang@flygoat.com>
> Reviewed-by: Nick Desaulniers <ndesaulniers@google.com>
> ---
>  arch/mips/Kconfig | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/mips/Kconfig b/arch/mips/Kconfig
> index 41ac4dc5aae4..0b270562c3eb 100644
> --- a/arch/mips/Kconfig
> +++ b/arch/mips/Kconfig
> @@ -2360,7 +2360,7 @@ config CPU_NEEDS_NO_SMARTMIPS_OR_MICROMIPS
>  	  Select this if you want neither microMIPS nor SmartMIPS support
>  
>  config CPU_HAS_SMARTMIPS
> -	depends on SYS_SUPPORTS_SMARTMIPS
> +	depends on SYS_SUPPORTS_SMARTMIPS && CC_HAS_SMARTMIPS
>  	bool "SmartMIPS"
>  	help
>  	  SmartMIPS is a extension of the MIPS32 architecture aimed at
> @@ -2373,6 +2373,7 @@ config CPU_HAS_SMARTMIPS
>  
>  config CPU_MICROMIPS
>  	depends on 32BIT && SYS_SUPPORTS_MICROMIPS && !CPU_MIPSR6
> +	depends on CC_HAS_MICROMIPS
>  	bool "microMIPS"
>  	help
>  	  When this option is enabled the kernel will be built using the

hmm, with this change the options will silently be dropped. I prefer
the error message, that the compiler doesn't support what is configured.

Thomas.
Nick Desaulniers April 19, 2023, 11:01 p.m. UTC | #2 
On Tue, Apr 18, 2023 at 6:13 AM Thomas Bogendoerfer
<tsbogend@alpha.franken.de> wrote:
>
> On Fri, Apr 14, 2023 at 09:06:56AM +0100, Jiaxun Yang wrote:
> > microMIPS smartMIPS kernel can only be compiled if they are supported
> > by toolchain.
> >
> > Signed-off-by: Jiaxun Yang <jiaxun.yang@flygoat.com>
> > Reviewed-by: Nick Desaulniers <ndesaulniers@google.com>

Question: won't the lack of this (patch and rest of the series) hurt
our ability to test randconfig builds of ARCH=mips with clang? See
also the 0day report from Boris:
https://lore.kernel.org/llvm/202304170748.Fg9VIgGd-lkp@intel.com/

i.e. randconfig will continue to select options that can't be built yet.

> > ---
> >  arch/mips/Kconfig | 3 ++-
> >  1 file changed, 2 insertions(+), 1 deletion(-)
> >
> > diff --git a/arch/mips/Kconfig b/arch/mips/Kconfig
> > index 41ac4dc5aae4..0b270562c3eb 100644
> > --- a/arch/mips/Kconfig
> > +++ b/arch/mips/Kconfig
> > @@ -2360,7 +2360,7 @@ config CPU_NEEDS_NO_SMARTMIPS_OR_MICROMIPS
> >         Select this if you want neither microMIPS nor SmartMIPS support
> >
> >  config CPU_HAS_SMARTMIPS
> > -     depends on SYS_SUPPORTS_SMARTMIPS
> > +     depends on SYS_SUPPORTS_SMARTMIPS && CC_HAS_SMARTMIPS
> >       bool "SmartMIPS"
> >       help
> >         SmartMIPS is a extension of the MIPS32 architecture aimed at
> > @@ -2373,6 +2373,7 @@ config CPU_HAS_SMARTMIPS
> >
> >  config CPU_MICROMIPS
> >       depends on 32BIT && SYS_SUPPORTS_MICROMIPS && !CPU_MIPSR6
> > +     depends on CC_HAS_MICROMIPS
> >       bool "microMIPS"
> >       help
> >         When this option is enabled the kernel will be built using the
>
> hmm, with this change the options will silently be dropped. I prefer
> the error message, that the compiler doesn't support what is configured.
>
> Thomas.
>
> --
> Crap can work. Given enough thrust pigs will fly, but it's not necessarily a
> good idea.                                                [ RFC1925, 2.3 ]
Jiaxun Yang April 20, 2023, 7:41 p.m. UTC | #3 
> 2023年4月20日 00:01，Nick Desaulniers <ndesaulniers@google.com> 写道：
> 
> On Tue, Apr 18, 2023 at 6:13 AM Thomas Bogendoerfer
> <tsbogend@alpha.franken.de> wrote:
>> 
>> On Fri, Apr 14, 2023 at 09:06:56AM +0100, Jiaxun Yang wrote:
>>> microMIPS smartMIPS kernel can only be compiled if they are supported
>>> by toolchain.
>>> 
>>> Signed-off-by: Jiaxun Yang <jiaxun.yang@flygoat.com>
>>> Reviewed-by: Nick Desaulniers <ndesaulniers@google.com>
> 
> Question: won't the lack of this (patch and rest of the series) hurt
> our ability to test randconfig builds of ARCH=mips with clang? See
> also the 0day report from Boris:
> https://lore.kernel.org/llvm/202304170748.Fg9VIgGd-lkp@intel.com/

Kconfig experts, Is there any way to generate warning based on Kconfig options?
So we can let users know there are something went wrong but still allow build to happen.

Thanks
- Jiaxun 

> 
> i.e. randconfig will continue to select options that can't be built yet.
> 
> -- 
> Thanks,
> ~Nick Desaulniers
Nathan Chancellor April 24, 2023, 5:03 p.m. UTC | #4 
On Thu, Apr 20, 2023 at 08:41:07PM +0100, Jiaxun Yang wrote:
> 
> 
> > 2023年4月20日 00:01，Nick Desaulniers <ndesaulniers@google.com> 写道：
> > 
> > On Tue, Apr 18, 2023 at 6:13 AM Thomas Bogendoerfer
> > <tsbogend@alpha.franken.de> wrote:
> >> 
> >> On Fri, Apr 14, 2023 at 09:06:56AM +0100, Jiaxun Yang wrote:
> >>> microMIPS smartMIPS kernel can only be compiled if they are supported
> >>> by toolchain.
> >>> 
> >>> Signed-off-by: Jiaxun Yang <jiaxun.yang@flygoat.com>
> >>> Reviewed-by: Nick Desaulniers <ndesaulniers@google.com>
> > 
> > Question: won't the lack of this (patch and rest of the series) hurt
> > our ability to test randconfig builds of ARCH=mips with clang? See
> > also the 0day report from Boris:
> > https://lore.kernel.org/llvm/202304170748.Fg9VIgGd-lkp@intel.com/
> 
> Kconfig experts, Is there any way to generate warning based on Kconfig options?
> So we can let users know there are something went wrong but still allow build to happen.

I do not think there is a way that this can be done within Kconfig
itself but you could do something like arch/x86/Makefile does for
RETPOLINE during archprepare and warn when an option is selected without
a dependency. I envision something like the following but it has not
even been compile tested:

diff --git a/arch/mips/Makefile b/arch/mips/Makefile
index 04e46ec24319..2c3be65ce3cc 100644
--- a/arch/mips/Makefile
+++ b/arch/mips/Makefile
@@ -399,6 +399,11 @@ ifdef CONFIG_MIPS32_O32
 	@$(kecho) '  Checking missing-syscalls for O32'
 	$(Q)$(MAKE) $(build)=. missing-syscalls missing_syscalls_flags="-mabi=32"
 endif
+ifdef CONFIG_CPU_HAS_SMARTMIPS
+ifndef CC_HAS_SMARTMIPS
+	$(warning CONFIG_CPU_HAS_SMARTMIPS selected without supported compiler, build may not work)
+endif
+endif
 
 install:
 	$(Q)install -D -m 755 vmlinux $(INSTALL_PATH)/vmlinux-$(KERNELRELEASE)

You could also make that depend on !CONFIG_COMPILE_TEST, as that is a
signal that the user does not intend to run this kernel.

Masahiro might have other comments.

Cheers,
Nathan
diff mbox series

Patch

diff --git a/arch/mips/Kconfig b/arch/mips/Kconfig
index 41ac4dc5aae4..0b270562c3eb 100644
--- a/arch/mips/Kconfig
+++ b/arch/mips/Kconfig
@@ -2360,7 +2360,7 @@  config CPU_NEEDS_NO_SMARTMIPS_OR_MICROMIPS
 	  Select this if you want neither microMIPS nor SmartMIPS support
 
 config CPU_HAS_SMARTMIPS
-	depends on SYS_SUPPORTS_SMARTMIPS
+	depends on SYS_SUPPORTS_SMARTMIPS && CC_HAS_SMARTMIPS
 	bool "SmartMIPS"
 	help
 	  SmartMIPS is a extension of the MIPS32 architecture aimed at
@@ -2373,6 +2373,7 @@  config CPU_HAS_SMARTMIPS
 
 config CPU_MICROMIPS
 	depends on 32BIT && SYS_SUPPORTS_MICROMIPS && !CPU_MIPSR6
+	depends on CC_HAS_MICROMIPS
 	bool "microMIPS"
 	help
 	  When this option is enabled the kernel will be built using the