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.pinch...@ideasonboard.com>
Signed-off-by: Marek Vasut <marek.vasut+rene...@mailbox.org>
---
Cc: Heinrich Schuchardt <xypron.g...@gmx.de>
Cc: Kuninori Morimoto <kuninori.morimoto...@renesas.com>
Cc: Laurent Pinchart <laurent.pinch...@ideasonboard.com>
Cc: Simon Glass <s...@chromium.org>
Cc: Tom Rini <tr...@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
