[PATCH] sandbox: Eliminate CONFIG_HOST_32/64BIT

[Heinrich Schuchardt]

On 10/14/22 13:42, Michal Suchánek wrote:
> On Fri, Oct 14, 2022 at 01:33:33PM +0200, Heinrich Schuchardt wrote:
>> On 10/14/22 10:33, Michal Suchánek wrote:
>>> Hello,
>>>
>>> On Fri, Oct 14, 2022 at 08:40:52AM +0200, Heinrich Schuchardt wrote:
>>>> From: Heinrich Schuchardt <xypron.glpk at gmx.de>
>>>>
>>>> Building sandbox_defconfig on 32bit systems requires manual configuration.
>>>> we should void this.
>>>>
>>>> The compiler provides symbol __LP64__. This is enough to know if the host
>>>> is a 64bit or a 32bit system.
>>>>
>>>> Reported-by: Michal Suchanek <msuchanek at suse.de>
>>>> Signed-off-by: Heinrich Schuchardt <xypron.glpk at gmx.de>
>>>> ---
>>>>    arch/sandbox/Kconfig                   | 24 ------------------------
>>>>    arch/sandbox/include/asm/posix_types.h |  8 +++++++-
>>>>    drivers/misc/swap_case.c               |  2 +-
>>>>    include/linux/bitops.h                 |  2 +-
>>>>    4 files changed, 9 insertions(+), 27 deletions(-)
>>>>
>>>> diff --git a/arch/sandbox/Kconfig b/arch/sandbox/Kconfig
>>>> index 852a7c8bf2..c7668233b9 100644
>>>> --- a/arch/sandbox/Kconfig
>>>> +++ b/arch/sandbox/Kconfig
>>>> @@ -13,7 +13,6 @@ config SYS_CPU
>>>>    config SANDBOX64
>>>>    	bool "Use 64-bit addresses"
>>>>    	select PHYS_64BIT
>>>> -	select HOST_64BIT
>>>
>>> eliminating the Kconfig option makes it no longer possible SANDBOX64
>>> depend on building 64bit.
>>
>> Looking at include/linux/bitops.h and drivers/misc/swap_case.c it was always
>> wrong to use HOST_64BIT=y when building sandbox64_defconfig on an ILP32
>> system.
> 
> And it will continue to be wrong when there is no config option that
> SANDBOX64 can depend on so it cannot be enabled on 32bit.

Before and after this patch you can build sandbox64 on ilp32. Kconfig is 
not meant to stop you from building.

The raison d'etre of phys_addr_t is that it can be wider than void *. 
sandbox64 can be booted on ilp32 but has bugs that need attention.

Best regards

Heinrich

> 
> Thanks
> 
> Michal
> 
>>
>> Best regards
>>
>> Heinrich
>>
>>>
>>> Thanks
>>>
>>> Michal
>>>>    config SANDBOX_RAM_SIZE_MB
>>>>    	int "RAM size in MiB"
>>>> @@ -41,24 +40,6 @@ 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
>>>> -
>>>> -config HOST_64BIT
>>>> -	bool "64-bit host"
>>>> -
>>>> -endchoice
>>>> -
>>>>    config SANDBOX_CRASH_RESET
>>>>    	bool "Reset on crash"
>>>>    	help
>>>> @@ -68,11 +49,6 @@ config SANDBOX_CRASH_RESET
>>>>    	  test suites like the UEFI self certification test which continue
>>>>    	  with the next test after a crash.
>>>> -config SANDBOX_BITS_PER_LONG
>>>> -	int
>>>> -	default 32 if HOST_32BIT
>>>> -	default 64 if HOST_64BIT
>>>> -
>>>>    config SYS_FDT_LOAD_ADDR
>>>>    	hex "Address at which to load devicetree"
>>>>    	default 0x100
>>>> diff --git a/arch/sandbox/include/asm/posix_types.h b/arch/sandbox/include/asm/posix_types.h
>>>> index ec18ed7e3c..0129e91122 100644
>>>> --- a/arch/sandbox/include/asm/posix_types.h
>>>> +++ b/arch/sandbox/include/asm/posix_types.h
>>>> @@ -19,6 +19,12 @@
>>>>     * assume GCC is being used.
>>>>     */
>>>> +#ifdef __LP64__
>>>> +#define SANDBOX_BITS_PER_LONG 64
>>>> +#else
>>>> +#define SANDBOX_BITS_PER_LONG 32
>>>> +#endif
>>>> +
>>>>    typedef unsigned short		__kernel_dev_t;
>>>>    typedef unsigned long		__kernel_ino_t;
>>>>    typedef unsigned short		__kernel_mode_t;
>>>> @@ -28,7 +34,7 @@ typedef int			__kernel_pid_t;
>>>>    typedef unsigned short		__kernel_ipc_pid_t;
>>>>    typedef unsigned short		__kernel_uid_t;
>>>>    typedef unsigned short		__kernel_gid_t;
>>>> -#if CONFIG_SANDBOX_BITS_PER_LONG == 32
>>>> +#if SANDBOX_BITS_PER_LONG == 32
>>>>    typedef unsigned int		__kernel_size_t;
>>>>    typedef int			__kernel_ssize_t;
>>>>    typedef int			__kernel_ptrdiff_t;
>>>> diff --git a/drivers/misc/swap_case.c b/drivers/misc/swap_case.c
>>>> index 7093ad1cd4..205ed81750 100644
>>>> --- a/drivers/misc/swap_case.c
>>>> +++ b/drivers/misc/swap_case.c
>>>> @@ -331,7 +331,7 @@ static int sandbox_swap_case_map_physmem(struct udevice *dev,
>>>>    			*ptrp = &pci_ea_bar2_magic;
>>>>    			*lenp = PCI_CAP_EA_SIZE_LO;
>>>>    			break;
>>>> -#ifdef CONFIG_HOST_64BIT
>>>> +#ifdef __LP64__
>>>>    		/*
>>>>    		 * This cannot be work on a 32-bit machine since *lenp is ulong
>>>>    		 * which is 32-bits, but it needs to have a 64-bit value
>>>> diff --git a/include/linux/bitops.h b/include/linux/bitops.h
>>>> index d2e5ca026e..6456f5ad45 100644
>>>> --- a/include/linux/bitops.h
>>>> +++ b/include/linux/bitops.h
>>>> @@ -28,7 +28,7 @@
>>>>     */
>>>>    #ifdef CONFIG_SANDBOX
>>>>    #define GENMASK(h, l) \
>>>> -	(((~0UL) << (l)) & (~0UL >> (CONFIG_SANDBOX_BITS_PER_LONG - 1 - (h))))
>>>> +	(((~0UL) << (l)) & (~0UL >> (SANDBOX_BITS_PER_LONG - 1 - (h))))
>>>>    #else
>>>>    #define GENMASK(h, l) \
>>>>    	(((~0UL) << (l)) & (~0UL >> (BITS_PER_LONG - 1 - (h))))
>>>> -- 
>>>> 2.37.2
>>>>
>>