[PATCH 1/3] boot: fdt: Change type of env_get_bootm_low() to phys_addr_t

[Heinrich Schuchardt]

On 17.03.24 17:58, Marek Vasut wrote:
> On 3/17/24 10:08 AM, Heinrich Schuchardt wrote:
>> On 3/17/24 07:16, Marek Vasut wrote:
>>> Change type of ulong env_get_bootm_low() to phys_addr_t
>>> env_get_bootm_low().
>>> The PPC/LS systems already treat env_get_bootm_low() result as
>>> phys_addr_t,
>>> while the function itself still returns ulong. This is potentially
>>> dangerous
>>> on 64bit systems, where ulong might not be large enough to hold the
>>> content
>>
>> Isn't long always 64bit when building with gcc or llvm?
>
> C99 spec says:
>
> 5.2.4.2.1 Sizes of integer types <limits.h>
> ...
> - maximum value for an object of type unsigned long int
> ULONG_MAX 4294967295 // 2^32 - 1
> ...
>
> Simpler table:
> https://en.wikipedia.org/wiki/C_data_types
>
> So, to answer the question, it might, but we cannot rely on that.
>
> Also, phys_addr_t represents physical addresses, which this is.
>
> [...]
>
>>> @@ -553,8 +554,8 @@ int boot_ramdisk_high(struct lmb *lmb, ulong
>>> rd_data, ulong rd_len,
>>>           initrd_high = env_get_bootm_mapsize() + env_get_bootm_low();
>>>       }
>>>
>>> -    debug("## initrd_high = 0x%08lx, copy_to_ram = %d\n",
>>> -          initrd_high, initrd_copy_to_ram);
>>> +    debug("## initrd_high = 0x%p, copy_to_ram = %d\n",
>>> +          (void *)(uintptr_t)initrd_high, initrd_copy_to_ram);
>>
>> Void * may be narrower than phys_addr_t.
>
> When would that happen, on PAE systems ?
>
>> Shouldn't we convert to unsigned long long for printing.
>
> Printing phys_addr_t always confuses me. I was under the impression that
> turning the value into uintptr_t (platform pointer type represented as
> integer) and then actually using that as a pointer for printing should
> not suffer from any type width problems. Does it ?

As you already remarked on LPAE this may happen.

Though you may not be able load initrd outside the address range of
void* the variables might exceed it.

Best regards

Heinrich

>
> Since this is a debug print, I can just upcast it to u64.