diff mbox series

[v3,3/5] sandbox: Detect the host bit size automatically

Message ID 20210802005613.2267669-4-sjg@chromium.org
State Superseded
Delegated to: Tom Rini
Headers show
Series Fix compiler warnings for 32-bit ARM | expand

Commit Message

Simon Glass Aug. 2, 2021, 12:56 a.m. UTC
At present if you build sandbox on a 32-bit host a lot of errors are
produced. This is because CONFIG_HOST_64BIT is enabled by default.

It is quite annoying to have to change that manually before building
sandbox. It is also quite confusing for new users.

Add a way to detect the setting and add the appropriate
CONFIG_HOST_64BIT=y or CONFIG_HOST_32BIT=y to the defconfig, to avoid
this issue.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

Changes in v3:
- Use 'bitness' instead of 'bit'

Changes in v2:
- Drop patches previously applied
- Put all the packages in gcc.rst

 Makefile                 | 11 ++++++++++-
 arch/sandbox/Kconfig     | 13 ++++++++-----
 scripts/kconfig/Makefile | 14 +++++++++++++-
 3 files changed, 31 insertions(+), 7 deletions(-)

Comments

Pali Rohár Nov. 1, 2021, 7:04 a.m. UTC | #1
On Sunday 01 August 2021 18:56:11 Simon Glass wrote:
> At present if you build sandbox on a 32-bit host a lot of errors are
> produced. This is because CONFIG_HOST_64BIT is enabled by default.
> 
> It is quite annoying to have to change that manually before building
> sandbox. It is also quite confusing for new users.
> 
> Add a way to detect the setting and add the appropriate
> CONFIG_HOST_64BIT=y or CONFIG_HOST_32BIT=y to the defconfig, to avoid
> this issue.
> 
> Signed-off-by: Simon Glass <sjg@chromium.org>
> ---
> 
> Changes in v3:
> - Use 'bitness' instead of 'bit'
> 
> Changes in v2:
> - Drop patches previously applied
> - Put all the packages in gcc.rst
> 
>  Makefile                 | 11 ++++++++++-
>  arch/sandbox/Kconfig     | 13 ++++++++-----
>  scripts/kconfig/Makefile | 14 +++++++++++++-
>  3 files changed, 31 insertions(+), 7 deletions(-)
> 
> diff --git a/Makefile b/Makefile
> index 269e353a28a..57c3e9a77df 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -565,8 +565,17 @@ export KBUILD_DEFCONFIG KBUILD_KCONFIG
>  config: scripts_basic outputmakefile FORCE
>  	$(Q)$(MAKE) $(build)=scripts/kconfig $@
>  
> +# If nothing is specified explicitly, detect the bit size for sandbox,
> +# referred to from arch/sandbox/Kconfig
> +# Add it to the end of the defconfig file
>  %config: scripts_basic outputmakefile FORCE
> -	$(Q)$(MAKE) $(build)=scripts/kconfig $@
> +	$(Q)if test -f $(srctree)/configs/$@ && \
> +		! grep -Eq "CONFIG_HOST_(32|64)BIT=y" $(srctree)/configs/$@; then \
> +		echo '#include <stdio.h>\nint main(void) { printf("CONFIG_HOST_%dBIT=y\\n", __WORDSIZE); return 0; }' \
> +			 | gcc -x c - -o get_word_size; \

Should not Makefile use user's specified compiler instead of hardcoded 'gcc'?

> +		extra=$$(./get_word_size); \
> +	fi; \
> +	$(MAKE) $(build)=scripts/kconfig EXTRA_DEFCONFIG=$$extra $@
>  
>  else
>  # ===========================================================================
> diff --git a/arch/sandbox/Kconfig b/arch/sandbox/Kconfig
> index f83282d9d56..0332aaee798 100644
> --- a/arch/sandbox/Kconfig
> +++ b/arch/sandbox/Kconfig
> @@ -35,12 +35,12 @@ config SYS_CONFIG_NAME
>  
>  choice
>  	prompt "Run sandbox on 32/64-bit host"
> -	default HOST_64BIT
> +	default HOST_DETECT_BITNESS
>  	help
> -	  Sandbox can be built on 32-bit and 64-bit hosts.
> -	  The default is to build on a 64-bit host and run
> -	  on a 64-bit host. If you want to run sandbox on
> -	  a 32-bit host, change it here.
> +	  Sandbox can be built on 32-bit and 64-bit hosts. This is generally
> +	  auto-detected but you can force a particular word size here. If you
> +	  see strange warnings about SANDBOX_BITS_PER_LONG then you may have
> +	  selected the wrong value.
>  
>  config HOST_32BIT
>  	bool "32-bit host"
> @@ -49,6 +49,9 @@ config HOST_32BIT
>  config HOST_64BIT
>  	bool "64-bit host"
>  
> +config HOST_DETECT_BITNESS
> +	bool "Auto-detect host bitness"
> +
>  endchoice
>  
>  config SANDBOX_CRASH_RESET
> diff --git a/scripts/kconfig/Makefile b/scripts/kconfig/Makefile
> index d52128425ce..8e14db7ade3 100644
> --- a/scripts/kconfig/Makefile
> +++ b/scripts/kconfig/Makefile
> @@ -12,6 +12,9 @@ PHONY += xconfig gconfig menuconfig config localmodconfig localyesconfig \
>  #  Set SRCARCH to .. fake this Makefile.
>  SRCARCH := ..
>  
> +# For U-Boot, EXTRA_DEFCONFIG contains a line to be added to the defconfig
> +# before processing. If empty, no line is added.
> +
>  ifdef KBUILD_KCONFIG
>  Kconfig := $(KBUILD_KCONFIG)
>  else
> @@ -92,8 +95,17 @@ else
>  endif
>  endif
>  
> +# If EXTRA_DEFCONFIG is defined, add it to the end of the defconfig, before
> +# processing. This allows the caller to change a setting on the fly
>  %_defconfig: $(obj)/conf
> -	$(Q)$< $(silent) --defconfig=arch/$(SRCARCH)/configs/$@ $(Kconfig)
> +	$(Q)defconfig="$(srctree)/arch/$(SRCARCH)/configs/$@"; \
> +	if [ -n "$(EXTRA_DEFCONFIG)" ]; then \
> +		cat $$defconfig >defconfig_tmp; \
> +		echo $(EXTRA_DEFCONFIG) >>defconfig_tmp; \
> +		$< $(silent) --defconfig=defconfig_tmp $(Kconfig); \
> +	else \
> +		$< $(silent) --defconfig=$$defconfig $(Kconfig); \
> +	fi
>  
>  # Added for U-Boot (backward compatibility)
>  %_config: %_defconfig
> -- 
> 2.32.0.554.ge1b32706d8-goog
>
Tom Rini Nov. 1, 2021, 1:13 p.m. UTC | #2
On Mon, Nov 01, 2021 at 08:04:44AM +0100, Pali Rohár wrote:
> On Sunday 01 August 2021 18:56:11 Simon Glass wrote:
> > At present if you build sandbox on a 32-bit host a lot of errors are
> > produced. This is because CONFIG_HOST_64BIT is enabled by default.
> > 
> > It is quite annoying to have to change that manually before building
> > sandbox. It is also quite confusing for new users.
> > 
> > Add a way to detect the setting and add the appropriate
> > CONFIG_HOST_64BIT=y or CONFIG_HOST_32BIT=y to the defconfig, to avoid
> > this issue.
> > 
> > Signed-off-by: Simon Glass <sjg@chromium.org>
> > ---
> > 
> > Changes in v3:
> > - Use 'bitness' instead of 'bit'
> > 
> > Changes in v2:
> > - Drop patches previously applied
> > - Put all the packages in gcc.rst
> > 
> >  Makefile                 | 11 ++++++++++-
> >  arch/sandbox/Kconfig     | 13 ++++++++-----
> >  scripts/kconfig/Makefile | 14 +++++++++++++-
> >  3 files changed, 31 insertions(+), 7 deletions(-)
> > 
> > diff --git a/Makefile b/Makefile
> > index 269e353a28a..57c3e9a77df 100644
> > --- a/Makefile
> > +++ b/Makefile
> > @@ -565,8 +565,17 @@ export KBUILD_DEFCONFIG KBUILD_KCONFIG
> >  config: scripts_basic outputmakefile FORCE
> >  	$(Q)$(MAKE) $(build)=scripts/kconfig $@
> >  
> > +# If nothing is specified explicitly, detect the bit size for sandbox,
> > +# referred to from arch/sandbox/Kconfig
> > +# Add it to the end of the defconfig file
> >  %config: scripts_basic outputmakefile FORCE
> > -	$(Q)$(MAKE) $(build)=scripts/kconfig $@
> > +	$(Q)if test -f $(srctree)/configs/$@ && \
> > +		! grep -Eq "CONFIG_HOST_(32|64)BIT=y" $(srctree)/configs/$@; then \
> > +		echo '#include <stdio.h>\nint main(void) { printf("CONFIG_HOST_%dBIT=y\\n", __WORDSIZE); return 0; }' \
> > +			 | gcc -x c - -o get_word_size; \
> 
> Should not Makefile use user's specified compiler instead of hardcoded 'gcc'?

And it should probably look a bit more like how we do CC_IS_GCC, etc.
diff mbox series

Patch

diff --git a/Makefile b/Makefile
index 269e353a28a..57c3e9a77df 100644
--- a/Makefile
+++ b/Makefile
@@ -565,8 +565,17 @@  export KBUILD_DEFCONFIG KBUILD_KCONFIG
 config: scripts_basic outputmakefile FORCE
 	$(Q)$(MAKE) $(build)=scripts/kconfig $@
 
+# If nothing is specified explicitly, detect the bit size for sandbox,
+# referred to from arch/sandbox/Kconfig
+# Add it to the end of the defconfig file
 %config: scripts_basic outputmakefile FORCE
-	$(Q)$(MAKE) $(build)=scripts/kconfig $@
+	$(Q)if test -f $(srctree)/configs/$@ && \
+		! grep -Eq "CONFIG_HOST_(32|64)BIT=y" $(srctree)/configs/$@; then \
+		echo '#include <stdio.h>\nint main(void) { printf("CONFIG_HOST_%dBIT=y\\n", __WORDSIZE); return 0; }' \
+			 | gcc -x c - -o get_word_size; \
+		extra=$$(./get_word_size); \
+	fi; \
+	$(MAKE) $(build)=scripts/kconfig EXTRA_DEFCONFIG=$$extra $@
 
 else
 # ===========================================================================
diff --git a/arch/sandbox/Kconfig b/arch/sandbox/Kconfig
index f83282d9d56..0332aaee798 100644
--- a/arch/sandbox/Kconfig
+++ b/arch/sandbox/Kconfig
@@ -35,12 +35,12 @@  config SYS_CONFIG_NAME
 
 choice
 	prompt "Run sandbox on 32/64-bit host"
-	default HOST_64BIT
+	default HOST_DETECT_BITNESS
 	help
-	  Sandbox can be built on 32-bit and 64-bit hosts.
-	  The default is to build on a 64-bit host and run
-	  on a 64-bit host. If you want to run sandbox on
-	  a 32-bit host, change it here.
+	  Sandbox can be built on 32-bit and 64-bit hosts. This is generally
+	  auto-detected but you can force a particular word size here. If you
+	  see strange warnings about SANDBOX_BITS_PER_LONG then you may have
+	  selected the wrong value.
 
 config HOST_32BIT
 	bool "32-bit host"
@@ -49,6 +49,9 @@  config HOST_32BIT
 config HOST_64BIT
 	bool "64-bit host"
 
+config HOST_DETECT_BITNESS
+	bool "Auto-detect host bitness"
+
 endchoice
 
 config SANDBOX_CRASH_RESET
diff --git a/scripts/kconfig/Makefile b/scripts/kconfig/Makefile
index d52128425ce..8e14db7ade3 100644
--- a/scripts/kconfig/Makefile
+++ b/scripts/kconfig/Makefile
@@ -12,6 +12,9 @@  PHONY += xconfig gconfig menuconfig config localmodconfig localyesconfig \
 #  Set SRCARCH to .. fake this Makefile.
 SRCARCH := ..
 
+# For U-Boot, EXTRA_DEFCONFIG contains a line to be added to the defconfig
+# before processing. If empty, no line is added.
+
 ifdef KBUILD_KCONFIG
 Kconfig := $(KBUILD_KCONFIG)
 else
@@ -92,8 +95,17 @@  else
 endif
 endif
 
+# If EXTRA_DEFCONFIG is defined, add it to the end of the defconfig, before
+# processing. This allows the caller to change a setting on the fly
 %_defconfig: $(obj)/conf
-	$(Q)$< $(silent) --defconfig=arch/$(SRCARCH)/configs/$@ $(Kconfig)
+	$(Q)defconfig="$(srctree)/arch/$(SRCARCH)/configs/$@"; \
+	if [ -n "$(EXTRA_DEFCONFIG)" ]; then \
+		cat $$defconfig >defconfig_tmp; \
+		echo $(EXTRA_DEFCONFIG) >>defconfig_tmp; \
+		$< $(silent) --defconfig=defconfig_tmp $(Kconfig); \
+	else \
+		$< $(silent) --defconfig=$$defconfig $(Kconfig); \
+	fi
 
 # Added for U-Boot (backward compatibility)
 %_config: %_defconfig