[PATCH 1/3] boot: fdt: Change type of env_get_bootm_low() to phys_addr_t
Heinrich Schuchardt
xypron.glpk at gmx.de
Sun Mar 17 10:08:19 CET 2024
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?
> of "bootm_low" environment variable. Fix it by using phys_addr_t, similar to
> what env_get_bootm_size() does, which returns phys_size_t .
>
> Reported-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> Signed-off-by: Marek Vasut <marek.vasut+renesas at mailbox.org>
> ---
> Cc: Heinrich Schuchardt <xypron.glpk at gmx.de>
> Cc: Kuninori Morimoto <kuninori.morimoto.gx at renesas.com>
> Cc: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> Cc: Simon Glass <sjg at chromium.org>
> Cc: Tom Rini <trini at konsulko.com>
> ---
> boot/bootm.c | 2 +-
> boot/image-board.c | 11 ++++++-----
> boot/image-fdt.c | 9 +++++----
> include/image.h | 2 +-
> 4 files changed, 13 insertions(+), 11 deletions(-)
>
> diff --git a/boot/bootm.c b/boot/bootm.c
> index d071537d692..2e818264f5f 100644
> --- a/boot/bootm.c
> +++ b/boot/bootm.c
> @@ -242,7 +242,7 @@ static int boot_get_kernel(const char *addr_fit, struct bootm_headers *images,
> #ifdef CONFIG_LMB
> static void boot_start_lmb(struct bootm_headers *images)
> {
> - ulong mem_start;
> + phys_addr_t mem_start;
> phys_size_t mem_size;
>
> mem_start = env_get_bootm_low();
Please, remove the conversion to phys_addr_t in the
lmb_init_and_reserve_range() invocation.
> diff --git a/boot/image-board.c b/boot/image-board.c
> index 75f6906cd56..447ced2678a 100644
> --- a/boot/image-board.c
> +++ b/boot/image-board.c
> @@ -107,12 +107,13 @@ static int on_loadaddr(const char *name, const char *value, enum env_op op,
> }
> U_BOOT_ENV_CALLBACK(loadaddr, on_loadaddr);
>
> -ulong env_get_bootm_low(void)
> +phys_addr_t env_get_bootm_low(void)
> {
> char *s = env_get("bootm_low");
> + phys_size_t tmp;
>
> if (s) {
> - ulong tmp = hextoul(s, NULL);
> + tmp = (phys_addr_t)simple_strtoull(s, NULL, 16);
In C the conversion is superfluous. Please, remove '(phys_addr_t)'.
Why do we need tmp?
return simple_strtoull(s, NULL, 16);
> return tmp;
> }
>
> @@ -538,7 +539,7 @@ int boot_ramdisk_high(struct lmb *lmb, ulong rd_data, ulong rd_len,
> ulong *initrd_start, ulong *initrd_end)
> {
> char *s;
> - ulong initrd_high;
> + phys_addr_t initrd_high;
> int initrd_copy_to_ram = 1;
>
> s = env_get("initrd_high");
> @@ -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. Shouldn't we convert to
unsigned long long for printing.
Best regards
Heinrich
>
> if (rd_data) {
> if (!initrd_copy_to_ram) { /* zero-copy ramdisk support */
> diff --git a/boot/image-fdt.c b/boot/image-fdt.c
> index 5e4aa9de0d2..boot/image-fdt.c
> @@ -160,9 +160,10 @@c2571b22244 100644
> --- a/boot/image-fdt.c
> +++ b/ int boot_relocate_fdt(struct lmb *lmb, char **of_flat_tree, ulong *of_size)
> {
> void *fdt_blob = *of_flat_tree;
> void *of_start = NULL;
> - u64 start, size, usable;
> + phys_addr_t start, size, usable;
> char *fdt_high;
> - ulong mapsize, low;
> + phys_addr_t low;
> + phys_size_t mapsize;
> ulong of_len = 0;
> int bank;
> int err;
> @@ -217,7 +218,7 @@ int boot_relocate_fdt(struct lmb *lmb, char **of_flat_tree, ulong *of_size)
> if (start + size < low)
> continue;
>
> - usable = min(start + size, (u64)(low + mapsize));
> + usable = min(start + size, low + mapsize);
>
> /*
> * At least part of this DRAM bank is usable, try
> @@ -233,7 +234,7 @@ int boot_relocate_fdt(struct lmb *lmb, char **of_flat_tree, ulong *of_size)
> * Reduce the mapping size in the next bank
> * by the size of attempt in current bank.
> */
> - mapsize -= usable - max(start, (u64)low);
> + mapsize -= usable - max(start, low);
> if (!mapsize)
> break;
> }
> diff --git a/include/image.h b/include/image.h
> index 21de70f0c9e..acffd17e0df 100644
> --- a/include/image.h
> +++ b/include/image.h
> @@ -946,7 +946,7 @@ static inline void image_set_name(struct legacy_img_hdr *hdr, const char *name)
> int image_check_hcrc(const struct legacy_img_hdr *hdr);
> int image_check_dcrc(const struct legacy_img_hdr *hdr);
> #ifndef USE_HOSTCC
> -ulong env_get_bootm_low(void);
> +phys_addr_t env_get_bootm_low(void);
> phys_size_t env_get_bootm_size(void);
> phys_size_t env_get_bootm_mapsize(void);
> #endif
More information about the U-Boot
mailing list