Re: [PATCH] tests: Build correct sandbox configuration on 32bit

["Michal Suchánek" <msuchanek@suse.de>]

On Fri, Oct 14, 2022 at 05:05:26AM +0200, Heinrich Schuchardt wrote:
> On 10/13/22 22:28, Michal Suchanek wrote:
> > Currently sandbox configuration defautls to 64bit and there is no
> > automation for building 32bit sandbox on 32bit hosts.
> > 
> > cpp does not know about target specification, code needs to be compiled
> > to determine integer width.
> > 
> > Add a test program that prints the integer width, and a make target that
> > aligns the sandbox configuration with the result.
> > 
> > Signed-off-by: Michal Suchanek <msuchanek@suse.de>
> > ---
> > 
> >   Makefile              |  6 ++++++
> >   doc/arch/sandbox.rst  | 16 +++++++++++-----
> >   test/py/conftest.py   |  1 +
> >   tools/Makefile        |  2 ++
> >   tools/bits-per-long.c | 14 ++++++++++++++
> >   5 files changed, 34 insertions(+), 5 deletions(-)
> >   create mode 100644 tools/bits-per-long.c
> > 
> > diff --git a/Makefile b/Makefile
> > index 3866cc62f9..e5463573f3 100644
> > --- a/Makefile
> > +++ b/Makefile
> > @@ -2166,6 +2166,12 @@ tools-all: envtools tools ;
> >   cross_tools: export CROSS_BUILD_TOOLS=y
> >   cross_tools: tools ;
> > 
> > +PHONY += set_host_bits
> > +set_host_bits: tools
> > +	$(Q)sed -i -e /CONFIG_HOST_$$($(objtree)/tools/bits-per-long)BIT/d $(KCONFIG_CONFIG)
> > +	$(Q)sed -i -E -e "s/CONFIG_HOST_(..)BIT=y/# CONFIG_HOST_\1BIT is not set/" $(KCONFIG_CONFIG)
> > +	$(Q)echo CONFIG_HOST_$$($(objtree)/tools/bits-per-long)BIT=y >> $(KCONFIG_CONFIG)
> > +
> >   .PHONY : CHANGELOG
> >   CHANGELOG:
> >   	git log --no-merges U-Boot-1_1_5.. | \
> > diff --git a/doc/arch/sandbox.rst b/doc/arch/sandbox.rst
> > index 068d4a3be4..d751205eba 100644
> > --- a/doc/arch/sandbox.rst
> > +++ b/doc/arch/sandbox.rst
> > @@ -33,9 +33,11 @@ machines.
> > 
> >   There are two versions of the sandbox: One using 32-bit-wide integers, and one
> >   using 64-bit-wide integers. The 32-bit version can be build and run on either
> > -32 or 64-bit hosts by either selecting or deselecting CONFIG_SANDBOX_32BIT; by
> > -default, the sandbox it built for a 32-bit host. The sandbox using 64-bit-wide
> > -integers can only be built on 64-bit hosts.
> > +32 or 64-bit hosts by either selecting or deselecting HOST_64BIT; by
> > +default, the sandbox it built for a 64-bit host. The sandbox using 64-bit-wide
> > +integers can only be built on 64-bit hosts. There is no automation for ensuring
> > +32bit build on 32bit hosts - use ``make set_host_bits`` to adjust the sandbox
> > +config.
> > 
> >   Note that standalone/API support is not available at present.
> > 
> > @@ -51,7 +53,9 @@ Basic Operation
> > 
> >   To run sandbox U-Boot use something like::
> > 
> > -   make sandbox_defconfig all
> > +   make sandbox_defconfig
> > +   make set_host_bits
> > +   make all
> 
> Thanks for addressing the problem of sandbox bitness.
> 
> We should not make building the sandbox more complicated. You could
> integrate building set_host_bits into an existing target like u-boot.cfg:.
> 
> Overall an approach with an external program is too complicated.
> CONFIG_HOST_32BIT and CONFIG_HOST_64BIT are used to define
> CONFIG_SANDBOX_BITS_PER_LONG.
And for making SANDBOX64 depend on 64bit build.
> 
> We could add
> 
> #ifndef __LP64__
> #undef SANDBOX_BITS_PER_LONG
> #define SANDBOX_BITS_PER_LONG 32
> #endif

If we are willing to depend on this define which is clearly named as
compiler-internal we could do similar to cc-option to run something like
$(CC) -dM -E - < /dev/null | grep -q _LP64

> 
> to the top of arch/sandbox/include/asm/posix_types.h and use
> 
> #if defined(CONFIG_HOST_64BIT) && defined(__LP64__)
> 
> in drivers/misc/swap_case.c to solve the problem. This demonstrates that
> CONFIG_HOST_32BIT and CONFIG_HOST_64BIT are superfluous symbols.

Not really.

Thanks

Michal