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

[Heinrich Schuchardt]

On 10/15/22 19:53, Simon Glass wrote:
> Hi Michal,
>
> On Fri, 14 Oct 2022 at 14:53, Michal Suchanek <msuchanek at suse.de> wrote:
>>
>> Currently sandbox configuration defautls to 64bit and there is no
>> automation for building 32bit sandbox on 32bit hosts.
>>
>> Use _LP64 macro as heuristic for detecting 64bit targets.
>>
>> Signed-off-by: Michal Suchanek <msuchanek at suse.de>
>> ---
>>
>> Changes in v2:
>> simplify and move detection to kconfig
>>
>> ---
>>   arch/sandbox/Kconfig    | 18 +++---------------
>>   scripts/Kconfig.include |  4 ++++
>>   2 files changed, 7 insertions(+), 15 deletions(-)
>
> Reviewed-by: Simon Glass <sjg at chromium.org>
>
> My only question is whether we can allow building the 32-bit version
> on a 64-bit machine? That would need a separate option I think, to
> say:
>
> I don't want you to automatically determine HOST_32/64BIT. Instead,
> use 32 (or 64).
>
> This is along the lines of what Heinrich is saying, except that I
> strongly feel that we must do the right thing by default, as your
> patch does.

The whole point of phys_addr_t and phys_size_t is that it can be 64bit
or 32bit on ilp32.

With this patch we cannot build with CONFIG_PHYS_64BIT=y on ilp32 and
that is bad.

32 bit phys_addr_t on lp64 is irrelevant for actual hardware but this is
what we currently test with sandbox_defconfig on Gitlab CI.

My patch is ending up in the same behavior as Michal's patch except that
it allows to have 64bit phys_addr_t on ilp32.

>
>>
>> diff --git a/arch/sandbox/Kconfig b/arch/sandbox/Kconfig
>> index 852a7c8bf2..35508c6b29 100644
>> --- a/arch/sandbox/Kconfig
>> +++ b/arch/sandbox/Kconfig
>> @@ -13,7 +13,7 @@ config SYS_CPU
>>   config SANDBOX64
>>          bool "Use 64-bit addresses"
>>          select PHYS_64BIT
>> -       select HOST_64BIT
>> +       depends on HOST_64BIT

This line is utterly wrong.

Best regards

Heinrich

>>
>>   config SANDBOX_RAM_SIZE_MB
>>          int "RAM size in MiB"
>> @@ -41,23 +41,11 @@ config SYS_CONFIG_NAME
>>          default "sandbox_spl" if SANDBOX_SPL
>>          default "sandbox" if !SANDBOX_SPL
>>
>> -choice
>> -       prompt "Run sandbox on 32/64-bit host"
>> -       default HOST_64BIT
>> -       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.
>> -
>>   config HOST_32BIT
>> -       bool "32-bit host"
>> -       depends on !PHYS_64BIT
>> +       def_bool ! $(cc-define,_LP64)
>>
>>   config HOST_64BIT
>> -       bool "64-bit host"
>> -
>> -endchoice
>> +       def_bool $(cc-define,_LP64)
>>
>>   config SANDBOX_CRASH_RESET
>>          bool "Reset on crash"
>> diff --git a/scripts/Kconfig.include b/scripts/Kconfig.include
>> index dad5583451..b7598ca5d9 100644
>> --- a/scripts/Kconfig.include
>> +++ b/scripts/Kconfig.include
>> @@ -22,6 +22,10 @@ success = $(if-success,$(1),y,n)
>>   # Return y if the compiler supports <flag>, n otherwise
>>   cc-option = $(success,$(CC) -Werror $(1) -E -x c /dev/null -o /dev/null)
>>
>> +# $(cc-define,<macro>)
>> +# Return y if the compiler defines <macro>, n otherwise
>> +cc-define = $(success,$(CC) -dM -E -x c /dev/null | grep -q '^#define \<$(1)\>')
>> +
>>   # $(ld-option,<flag>)
>>   # Return y if the linker supports <flag>, n otherwise
>>   ld-option = $(success,$(LD) -v $(1))
>> --
>> 2.37.3
>>
>
> Regards,
> SImon