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

[Heinrich Schuchardt]

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 at 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.

We could add

#ifndef __LP64__
#undef SANDBOX_BITS_PER_LONG
#define SANDBOX_BITS_PER_LONG 32
#endif

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.

Eliminating them and only using __LP64__ is the right approach.

@Simon:
We should add sandbox_defconfig built with -m32 to our Gitlab CI testing
after fixing the incompatibilities in the unit tests.

>      ./u-boot
>
>   Note: If you get errors about 'sdl-config: Command not found' you may need to
> @@ -59,7 +63,9 @@ install libsdl2.0-dev or similar to get SDL support. Alternatively you can
>   build sandbox without SDL (i.e. no display/keyboard support) by removing
>   the CONFIG_SANDBOX_SDL line in include/configs/sandbox.h or using::
>
> -   make sandbox_defconfig all NO_SDL=1
> +   make sandbox_defconfig
> +   make set_host_bits
> +   make all NO_SDL=1
>      ./u-boot
>
>   U-Boot will start on your computer, showing a sandbox emulation of the serial
> diff --git a/test/py/conftest.py b/test/py/conftest.py
> index 304e93164a..3d1fd6883a 100644
> --- a/test/py/conftest.py
> +++ b/test/py/conftest.py
> @@ -104,6 +104,7 @@ def run_build(config, source_dir, build_dir, board_type, log):
>               o_opt = ''
>           cmds = (
>               ['make', o_opt, '-s', board_type + '_defconfig'],
> +            ['make', o_opt, '-s', 'set_host_bits'],
>               ['make', o_opt, '-s', '-j{}'.format(os.cpu_count())],
>           )
>           name = 'make'
> diff --git a/tools/Makefile b/tools/Makefile
> index 34a1aa7a8b..d6b585953d 100644
> --- a/tools/Makefile
> +++ b/tools/Makefile
> @@ -68,6 +68,8 @@ HOSTCFLAGS_img2srec.o := -pedantic
>   hostprogs-$(CONFIG_XWAY_SWAP_BYTES) += xway-swap-bytes
>   HOSTCFLAGS_xway-swap-bytes.o := -pedantic
>
> +hostprogs-y += bits-per-long
> +
>   hostprogs-y += mkenvimage
>   mkenvimage-objs := mkenvimage.o os_support.o lib/crc32.o
>
> diff --git a/tools/bits-per-long.c b/tools/bits-per-long.c
> new file mode 100644
> index 0000000000..7630e1623f
> --- /dev/null
> +++ b/tools/bits-per-long.c
> @@ -0,0 +1,14 @@
> +// SPDX-License-Identifier: GPL-2.0
> +#include <stdio.h>
> +
> +int main(int argc, char **argv)
> +{
> +	unsigned long testvar = ~0UL;
> +	unsigned int i;
> +
> +	for (i = 0; testvar; i++, testvar >>= 1)
> +		;
> +
> +	return printf("%u\n", i);

return printf("%zd\n", 8 * sizeof(long));

> +}
> +

Please avoid empty lines at the end files.

Best regards

Heinrich