Enable FIT image to be loaded beyond 32-bit space
Subsystems (3)
v5 cover Enable FIT image to be loaded beyond 32-bit space
This set fixes compiling warnings for sandbox on 32-bit host (Ubuntu), convert ulong to phys_addr_t for image handling. The purpose is to fix image addresses so FIT image can be put beyond 32-bit space. The challenge is to keep 32-bit host tool (eg mkimage) working. Using unsigned long long as phys_addr_t for host is the trick.
This patchset is tested by compiling for selected powerpc, arm, armv8 and sandbox target. It is verified on selected platforms, including p1021rdb (e500v2)
p4080ds (e500mc)
t4240qds (e6500)
ls1021aqds (armv7)
ls2080ardb (armv8) with 32- and 64-git address images
Need help to test and probably fix on other platforms. Mips probably has compiling warning for arch/mips/lib/bootm.c.
Changes in v5:
Add comment to explain casting from 64-bit to 32-bit on 32-bit
targets with 36-bit or more physical address space
Revert a change of using pointer in bootm_host_load_image() Using new PRIpa macro instead of casting in fit_image_load() Add change to arch/x86/lib/bootm.c
Revise commit message
Drop % sign in PRIpa macro
Use %#08llx instead of %08llx when applicable
New patch split from fixing load and entry address patch Split the common function into another patch.
Revise commit subject.
Update commit message as suggested by Simon.
Updated cover letter with testing report.
Changes in v4:
New patch, separated from fixing FIT image.
New patch to fix compiling warnings for sandbox built on 32-bit host Still need to change CONFIG_SANDBOX_BITS_PER_LONG to 32 to avoid these warnings
include/asm-generic/bitops/__fls.h:17:2: warning: left shift
count >= width of type [enabled by default]
include/asm-generic/bitops/__fls.h:19:3: warning: left shift
count >= width of type [enabled by default]
include/asm-generic/bitops/__fls.h:22:2: warning: left shift
count >= width of type [enabled by default]
include/asm-generic/bitops/__fls.h:26:2: warning: left shift
count >= width of type [enabled by default]
include/asm-generic/bitops/__fls.h:30:2: warning: left shift
count >= width of type [enabled by default]
include/asm-generic/bitops/__fls.h:34:2: warning: left shift
count >= width of type [enabled by default]
include/asm-generic/bitops/__fls.h:38:2: warning: left shift
count >= width of type [enabled by default]
Separate ulong to phys_addr_t change to another patch.
Changes in v3:
Define PRIpa for host and target in common/image-fit.c so printf works properly for 32-, 64-bit targets and host tools.
Changes in v2:
Make a common function for both load and entry addresses. Simplify calculation of addresses in a similar way as fdtdec_get_number() fdtdec_get_number() is not used, or too many files need to be included
and/or twisted for host tool
Continue to use %08llx for print format for load and entry addresses
because %pa does not always work for host tool (mkimage)
York Sun (4):
common: Convert ulong to phys_addr_t for image addresses
sandbox: Fix compiling warning
common: image-fit: Use a common function to get address
common: image-fit: Fix load and entry addresses in FIT image
arch/powerpc/lib/bootm.c | 4 +- arch/sandbox/cpu/cpu.c | 3 +- arch/sandbox/include/asm/types.h | 9 ++++- arch/sandbox/lib/bootm.c | 3 +- arch/sandbox/lib/pci_io.c | 2 +- arch/x86/lib/bootm.c | 15 +++++--- cmd/bootm.c | 11 +++--- cmd/lzmadec.c | 5 ++- cmd/ximg.c | 21 +++++++--- common/bootm.c | 21 +++++----- common/bootm_os.c | 14 ++++--- common/image-android.c | 10 ++--- common/image-fdt.c | 16 ++++---- common/image-fit.c | 79 +++++++++++++++++++++----------------- common/image.c | 17 ++++---- disk/part_efi.c | 2 +- include/bootm.h | 6 +-- include/image.h | 40 ++++++++++++------- lib/hashtable.c | 2 +- tools/default_image.c | 2 +- 20 files changed, 166 insertions(+), 116 deletions(-)
Signature · 1 line
1.7.9.5
RFC PATCH v5 1/4 common: Convert ulong to phys_addr_t for image addresses
When dealing with image addresses, ulong has been used. Some files are used by both host and target. It is OK for the target, but not always enough for host tools including mkimage. This patch replaces "ulong" with "phys_addr_t" to make sure addresses are correct for both the target and the host.
This issue was found on 32-bit host when compiling for 64-bit target to support images with address higher than 32-bit space.
---
Changes in v5:
Add comment to explain casting from 64-bit to 32-bit on 32-bit
targets with 36-bit or more physical address space
Revert a change of using pointer in bootm_host_load_image() Using new PRIpa macro instead of casting in fit_image_load() Add change to arch/x86/lib/bootm.c
Changes in v4:
New patch, separated from fixing FIT image.
Changes in v3: None
Changes in v2: None
arch/powerpc/lib/bootm.c | 4 ++-- arch/x86/lib/bootm.c | 15 +++++++++------ cmd/bootm.c | 11 ++++++----- cmd/ximg.c | 21 +++++++++++++++------ common/bootm.c | 21 +++++++++++---------- common/bootm_os.c | 14 ++++++++------ common/image-android.c | 10 +++++----- common/image-fdt.c | 16 ++++++++-------- common/image-fit.c | 25 +++++++++++++------------ common/image.c | 17 ++++++++++------- include/bootm.h | 6 +++--- include/image.h | 40 +++++++++++++++++++++++++++------------- tools/default_image.c | 2 +- 13 files changed, 118 insertions(+), 84 deletions(-)
arch/powerpc/lib/bootm.c +2 -2
@@ -47,7 +47,7 @@ static void boot_jump_linux(bootm_headers_t *images)#endif kernel = (void (*)(bd_t *,ulong ,ulong ,ulong , -ulong ,ulong ,ulong ))images->ep; +ulong ,ulong ,ulong ))(uintptr_t )images->ep;debug ("## Transferring control to Linux (at address %08lx) ...\n" , (ulong )kernel);
@@ -335,7 +335,7 @@ void boot_jump_vxworks(bootm_headers_t *images)WATCHDOG_RESET (); ((void (*)(void *,ulong ,ulong ,ulong , -ulong ,ulong ,ulong ))images->ep)(images->ft_addr, +ulong ,ulong ,ulong ))(uintptr_t )images->ep)(images->ft_addr, 0, 0, EPAPR_MAGIC, getenv_bootm_mapsize(), 0, 0); }#endif
arch/x86/lib/bootm.c +9 -6
@@ -71,6 +71,8 @@ static int boot_prep_linux(bootm_headers_t *images)char *cmd_line_dest =NULL ;image_header_t *hdr;int is_zimage =0 ; +phys_addr_t os_data; +ulong os_len;void *data =NULL ;size_t len;int ret;
@@ -87,11 +89,10 @@ static int boot_prep_linux(bootm_headers_t *images)if (images->legacy_hdr_valid) { hdr = images->legacy_hdr_os;if (image_check_type (hdr,IH_TYPE_MULTI )) { -ulong os_data, os_len;/* if multi-part image, we need to get first subimage */ image_multi_getimg (hdr,0 , &os_data, &os_len); - data = (void *)os_data; + data = (void *)(uintptr_t )os_data; len = os_len; }else {/* otherwise get image data */
@@ -121,14 +122,15 @@ static int boot_prep_linux(bootm_headers_t *images) cmd_line_dest = base_ptr +COMMAND_LINE_OFFSET ; images->ep = (ulong )base_ptr; }else if (images->ep) { - cmd_line_dest = (void *)images->ep +COMMAND_LINE_OFFSET ; + cmd_line_dest = (void *)(uintptr_t )images->ep + +COMMAND_LINE_OFFSET ; }else {printf ("## Kernel loading failed (missing x86 kernel setup) ...\n" ); goto error; } -printf ("Setup at %#08lx\n" , images->ep); - ret =setup_zimage ((void *)images->ep, cmd_line_dest, +printf ("Setup at %#08" PRIpa"\n" , images->ep); + ret =setup_zimage ((void *)(uintptr_t )images->ep, cmd_line_dest,0 , images->rd_start, images->rd_end - images->rd_start);
@@ -187,7 +189,8 @@ int boot_linux_kernel(ulong setup_base, ulong load_address, bool image_64bit)/* Subcommand: GO */ static int boot_jump_linux (bootm_headers_t *images) { -debug ("## Transferring control to Linux (at address %08lx, kernel %08lx) ...\n" , +debug ("## Transferring control to Linux (at address %#08" PRIpa +", kernel %#08" PRIpa") ...\n" , images->ep, images->os.load); return boot_linux_kernel(images->ep, images->os.load,
cmd/bootm.c +6 -5
@@ -566,8 +566,8 @@ static int bootz_start(cmd_tbl_t *cmdtp, int flag, int argc, load_addr); }else { images->ep =simple_strtoul (argv[0 ],NULL ,16 ); -debug ("* kernel: cmdline image address = 0x%08lx\n" , - images->ep); +debug ("* kernel: cmdline image address = %#08" PRIpa"\n" , + images->ep); } ret =bootz_setup (images->ep, &zi_start, &zi_end);
@@ -676,7 +676,8 @@ static int booti_setup(bootm_headers_t *images)if (images->ep != dst) {void *src; -debug ("Moving Image from 0x%lx to 0x%llx\n" , images->ep, dst); +debug ("Moving Image from 0x%" PRIpa" to 0x%llx\n" , + images->ep, dst); src = (void *)images->ep; images->ep = dst;
@@ -705,8 +706,8 @@ static int booti_start(cmd_tbl_t *cmdtp, int flag, int argc, load_addr); }else { images->ep =simple_strtoul (argv[0 ],NULL ,16 ); -debug ("* kernel: cmdline image address = 0x%08lx\n" , - images->ep); +debug ("* kernel: cmdline image address = %#08" PRIpa"\n" , + images->ep); } ret =booti_setup (images);
cmd/ximg.c +15 -6
@@ -33,7 +33,16 @@ do_imgextract(cmd_tbl_t * cmdtp, int flag, int argc, char * const argv[]) {ulong addr = load_addr;ulong dest =0 ; -ulong data, len; +/* +* In this function, data is decalred as phys_addr_t type. +* On some systems (eg. ARM, PowerPC) phys_addr_t can be +* "unsigned long", or "unsigned long long", depending on +* CONFIG_PHYS_64BIT. It is safe to cast 64-bit phys_addr_t +* to 32-bit pointer for image handling because the actual +* address the image is loaded is within 32-bit space. +*/ +phys_addr_t data; +ulong len;int verify;int part =0 ;#if defined(CONFIG_IMAGE_FORMAT_LEGACY )
@@ -173,7 +182,7 @@ do_imgextract(cmd_tbl_t * cmdtp, int flag, int argc, char * const argv[]) return 1; } - data = (ulong )fit_data; + data = (phys_addr_t )(uintptr_t )fit_data; len = (ulong )fit_len;break ;#endif
@@ -190,7 +199,7 @@ do_imgextract(cmd_tbl_t * cmdtp, int flag, int argc, char * const argv[])size_t l = len;size_t tail;void *to = (void *) dest; -void *from = (void *)data; +void *from = (void *)(uintptr_t )data;printf (" Loading part %d ... " , part);
@@ -205,14 +214,14 @@ do_imgextract(cmd_tbl_t * cmdtp, int flag, int argc, char * const argv[]) }#else /* !(CONFIG_HW_WATCHDOG || CONFIG_WATCHDOG) */ printf (" Loading part %d ... " , part); -memmove ((char *) dest, (char *)data, len); +memmove ((char *)dest, (char *)(uintptr_t )data, len);#endif /* CONFIG_HW_WATCHDOG || CONFIG_WATCHDOG */ break ;#ifdef CONFIG_GZIP case IH_COMP_GZIP :printf (" Uncompressing part %d ... " , part);if (gunzip ((void *) dest, unc_len, - (uchar *) data, &len) !=0 ) { + (uchar *)(uintptr_t )data, &len) !=0 ) {puts ("GUNZIP ERROR - image not loaded\n" );return 1 ; }
@@ -231,7 +240,7 @@ do_imgextract(cmd_tbl_t * cmdtp, int flag, int argc, char * const argv[])*/ i =BZ2_bzBuffToBuffDecompress (map_sysmem (ntohl (hdr->ih_load),0 ), - &unc_len, (char *)data, len, + &unc_len, (char *)(uintptr_t )data, len,CONFIG_SYS_MALLOC_LEN < (4096 *1024 ),0 );if (i !=BZ_OK ) {
common/bootm.c +11 -10
@@ -43,7 +43,7 @@ DECLARE_GLOBAL_DATA_PTR;static const void *boot_get_kernel (cmd_tbl_t *cmdtp,int flag,int argc,char *const argv[],bootm_headers_t *images, -ulong *os_data,ulong *os_len); +phys_addr_t *os_data,ulong *os_len);#ifdef CONFIG_LMB static void boot_start_lmb (bootm_headers_t *images)
@@ -325,9 +325,9 @@ static int handle_decomp_error(int comp_type, size_t uncomp_size,return BOOTM_ERR_RESET ; } -int bootm_decomp_image (int comp,ulong load,ulong image_start,int type, -void *load_buf,void *image_buf,ulong image_len, -uint unc_len,ulong *load_end) +int bootm_decomp_image (int comp,phys_addr_t load,phys_addr_t image_start, +int type,void *load_buf,void *image_buf, +ulong image_len,uint unc_len,ulong *load_end) {int ret =0 ;
@@ -767,7 +767,7 @@ static image_header_t *image_get_kernel(ulong img_addr, int verify)/** * boot_get_kernel - find kernel image -* @os_data: pointer to a ulong variable, will hold os data start address +* @os_data: pointer to a phys_addr_t variable, will hold os data start address * @os_len: pointer to a ulong variable, will hold os data length * * boot_get_kernel() tries to find a kernel image, verifies its integrity
@@ -779,7 +779,7 @@ static image_header_t *image_get_kernel(ulong img_addr, int verify)*/ static const void *boot_get_kernel (cmd_tbl_t *cmdtp,int flag,int argc,char *const argv[],bootm_headers_t *images, -ulong *os_data,ulong *os_len) +phys_addr_t *os_data,ulong *os_len) {#if defined(CONFIG_IMAGE_FORMAT_LEGACY )image_header_t *hdr;
@@ -879,7 +879,7 @@ static const void *boot_get_kernel(cmd_tbl_t *cmdtp, int flag, int argc,return NULL ; } -debug (" kernel data at 0x%08lx, len = 0x%08lx (%ld)\n" , +debug (" kernel data@%#08" PRIpa", len = 0x%08lx (%ld)\n" , *os_data, *os_len, *os_len);return buf;
@@ -894,7 +894,8 @@ void memmove_wd(void *to, void *from, size_t len, ulong chunksz)static int bootm_host_load_image (const void *fit,int req_image_type) {const char *fit_uname_config =NULL ; -ulong data, len; +phys_addr_t data; +ulong len;bootm_headers_t images;int noffset;ulong load_end;
@@ -924,8 +925,8 @@ static int bootm_host_load_image(const void *fit, int req_image_type)/* Allow the image to expand by a factor of 4, should be safe */ load_buf =malloc ((1 <<20 ) + len *4 ); ret =bootm_decomp_image (imape_comp,0 , data, image_type, load_buf, - (void *)data, len,CONFIG_SYS_BOOTM_LEN , - &load_end); + (void *)(uintptr_t )data, len, +CONFIG_SYS_BOOTM_LEN , &load_end);free (load_buf);if (ret && ret !=BOOTM_ERR_UNIMPLEMENTED )
common/bootm_os.c +8 -6
@@ -26,7 +26,7 @@ static int do_bootm_standalone(int flag, int argc, char * const argv[],setenv_hex ("filesize" , images->os.image_len);return 0 ; } - appl = (int (*)(int ,char *const []))images->ep; + appl = (int (*)(int ,char *const []))(uintptr_t )images->ep;appl (argc, argv);return 0 ; }
@@ -55,7 +55,8 @@ static int do_bootm_netbsd(int flag, int argc, char * const argv[], {void (*loader)(bd_t *,image_header_t *,char *,char *);image_header_t *os_hdr, *hdr; -ulong kernel_data, kernel_len; +phys_addr_t kernel_data; +ulong kernel_len;char *consdev;char *cmdline;
@@ -113,7 +114,8 @@ static int do_bootm_netbsd(int flag, int argc, char * const argv[], cmdline ="" ; } - loader = (void (*)(bd_t *,image_header_t *,char *,char *))images->ep; + loader = (void (*)(bd_t *,image_header_t *,char *,char *)) + (uintptr_t )images->ep;printf ("## Transferring control to NetBSD stage-2 loader (at address %08lx) ...\n" , (ulong )loader);
@@ -171,7 +173,7 @@ static int do_bootm_rtems(int flag, int argc, char * const argv[], }#endif - entry_point = (void (*)(bd_t *))images->ep; + entry_point = (void (*)(bd_t *))(uintptr_t )images->ep;printf ("## Transferring control to RTEMS (at address %08lx) ...\n" , (ulong )entry_point);
@@ -252,7 +254,7 @@ static int do_bootm_plan9(int flag, int argc, char * const argv[], } } - entry_point = (void (*)(void ))images->ep; + entry_point = (void (*)(void ))(uintptr_t )images->ep;printf ("## Transferring control to Plan 9 (at address %08lx) ...\n" , (ulong )entry_point);
@@ -364,7 +366,7 @@ static int do_bootm_qnxelf(int flag, int argc, char * const argv[], }#endif -sprintf (str,"%lx" , images->ep);/* write entry-point into string */ +sprintf (str,"%" PRIpa, images->ep);/* write entry-point into string */ local_args[0 ] = argv[0 ]; local_args[1 ] = str;/* and provide it via the arguments */ do_bootelf (NULL ,0 ,2 , local_args);
common/image-android.c +5 -5
@@ -38,7 +38,7 @@ static ulong android_image_get_kernel_addr(const struct andr_img_hdr *hdr)* @hdr: Pointer to image header, which is at the start * of the image. * @verify: Checksum verification flag. Currently unimplemented. -* @os_data: Pointer to a ulong variable, will hold os data start +* @os_data: Pointer to a phys_addr_t variable, will hold os data start * address. * @os_len: Pointer to a ulong variable, will hold os data length. *
@@ -49,7 +49,7 @@ static ulong android_image_get_kernel_addr(const struct andr_img_hdr *hdr)* otherwise on failure. */ int android_image_get_kernel (const struct andr_img_hdr *hdr,int verify, -ulong *os_data,ulong *os_len) +phys_addr_t *os_data,ulong *os_len) {u32 kernel_addr =android_image_get_kernel_addr (hdr);
@@ -93,7 +93,7 @@ int android_image_get_kernel(const struct andr_img_hdr *hdr, int verify,setenv ("bootargs" , newbootargs);if (os_data) { - *os_data = (ulong )hdr; + *os_data = (phys_addr_t )hdr; *os_data += hdr->page_size; }if (os_len)
@@ -128,7 +128,7 @@ ulong android_image_get_kload(const struct andr_img_hdr *hdr) }int android_image_get_ramdisk (const struct andr_img_hdr *hdr, -ulong *rd_data,ulong *rd_len) +phys_addr_t *rd_data,ulong *rd_len) {if (!hdr->ramdisk_size) { *rd_data = *rd_len =0 ;
@@ -138,7 +138,7 @@ int android_image_get_ramdisk(const struct andr_img_hdr *hdr,printf ("RAM disk load addr 0x%08x size %u KiB\n" , hdr->ramdisk_addr,DIV_ROUND_UP (hdr->ramdisk_size,1024 )); - *rd_data = (unsigned long )hdr; + *rd_data = (phys_addr_t )hdr; *rd_data += hdr->page_size; *rd_data +=ALIGN (hdr->kernel_size, hdr->page_size);
common/image-fdt.c +8 -8
@@ -223,11 +223,14 @@ error:int boot_get_fdt (int flag,int argc,char *const argv[],uint8_t arch,bootm_headers_t *images,char **of_flat_tree,ulong *of_size) { +phys_addr_t load;#if defined(CONFIG_IMAGE_FORMAT_LEGACY )const image_header_t *fdt_hdr; -ulong load, load_end; +phys_addr_t load_end;ulong image_start, image_data, image_end;#endif +phys_addr_t fdt_data; +ulong fdt_len;ulong fdt_addr;char *fdt_blob =NULL ;void *buf;
@@ -236,6 +239,7 @@ int boot_get_fdt(int flag, int argc, char * const argv[], uint8_t arch,const char *fit_uname_fdt =NULL ;ulong default_addr;int fdt_noffset; +ulong len;#endif const char *select =NULL ;int ok_no_fdt =0 ;
@@ -335,10 +339,10 @@ int boot_get_fdt(int flag, int argc, char * const argv[], uint8_t arch, goto error; } -debug (" Loading FDT from 0x%08lx to 0x%08lx\n" , +debug (" Loading FDT from 0x%08lx to %#08" PRIpa"\n" , image_data, load); -memmove ((void *)load, +memmove ((void *)(uintptr_t )load, (void *)image_data,image_get_data_size (fdt_hdr));
@@ -354,8 +358,6 @@ int boot_get_fdt(int flag, int argc, char * const argv[], uint8_t arch,#if defined(CONFIG_FIT )/* check FDT blob vs FIT blob */ if (fit_check_format (buf)) { -ulong load, len; - fdt_noffset = fit_image_load(images, fdt_addr, &fit_uname_fdt, &fit_uname_config,
@@ -389,8 +391,6 @@ int boot_get_fdt(int flag, int argc, char * const argv[], uint8_t arch, }else if (images->legacy_hdr_valid &&image_check_type (&images->legacy_hdr_os_copy,IH_TYPE_MULTI )) { -ulong fdt_data, fdt_len; -/* * Now check if we have a legacy multi-component image, * get second entry data start address and len.
@@ -401,7 +401,7 @@ int boot_get_fdt(int flag, int argc, char * const argv[], uint8_t arch,image_multi_getimg (images->legacy_hdr_os,2 , &fdt_data, &fdt_len);if (fdt_len) { - fdt_blob = (char *)fdt_data; + fdt_blob = (char *)(uintptr_t )fdt_data;printf (" Booting using the fdt at 0x%p\n" , fdt_blob);if (fdt_check_header (fdt_blob) !=0 ) {
common/image-fit.c +13 -12
@@ -358,7 +358,7 @@ void fit_image_print(const void *fit, int image_noffset, const char *p)char *desc;uint8_t type, arch, os, comp;size_t size; -ulong load, entry; +phys_addr_t load, entry;const void *data;int noffset;int ndepth;
@@ -428,7 +428,7 @@ void fit_image_print(const void *fit, int image_noffset, const char *p)if (ret)printf ("unavailable\n" );else -printf ("0x%08lx\n" , load); +printf ("%#08" PRIpa"\n" , load); } if ((type == IH_TYPE_KERNEL) || (type == IH_TYPE_STANDALONE) ||
@@ -438,7 +438,7 @@ void fit_image_print(const void *fit, int image_noffset, const char *p)if (ret)printf ("unavailable\n" );else -printf ("0x%08lx\n" , entry); +printf ("%#08" PRIpa"\n" , entry); }/* Process all hash subnodes of the component image node */
@@ -679,7 +679,7 @@ int fit_image_get_comp(const void *fit, int noffset, uint8_t *comp)* fit_image_get_load() - get load addr property for given component image node * @fit: pointer to the FIT format image header * @noffset: component image node offset -* @load: pointer to the uint32_t, will hold load address +* @load: pointer to the phys_addr_t, will hold load address * * fit_image_get_load() finds load address property in a given component * image node. If the property is found, its value is returned to the caller.
@@ -688,7 +688,7 @@ int fit_image_get_comp(const void *fit, int noffset, uint8_t *comp)* 0, on success * -1, on failure */ -int fit_image_get_load (const void *fit,int noffset,ulong *load) +int fit_image_get_load (const void *fit,int noffset,phys_addr_t *load) {int len;const uint32_t *data;
@@ -707,7 +707,7 @@ int fit_image_get_load(const void *fit, int noffset, ulong *load)* fit_image_get_entry() - get entry point address property * @fit: pointer to the FIT format image header * @noffset: component image node offset -* @entry: pointer to the uint32_t, will hold entry point address +* @entry: pointer to the phys_addr_t, will hold entry point address * * This gets the entry point address property for a given component image * node.
@@ -720,7 +720,7 @@ int fit_image_get_load(const void *fit, int noffset, ulong *load)* 0, on success * -1, on failure */ -int fit_image_get_entry (const void *fit,int noffset,ulong *entry) +int fit_image_get_entry (const void *fit,int noffset,phys_addr_t *entry) {int len;const uint32_t *data;
@@ -1556,7 +1556,7 @@ static const char *fit_get_image_type_property(int type)int fit_image_load (bootm_headers_t *images,ulong addr,const char **fit_unamep,const char **fit_uname_configp,int arch,int image_type,int bootstage_id, -enum fit_load_op load_op,ulong *datap,ulong *lenp) +enum fit_load_op load_op,phys_addr_t *datap,ulong *lenp) {int cfg_noffset, noffset;const char *fit_uname;
@@ -1565,7 +1565,8 @@ int fit_image_load(bootm_headers_t *images, ulong addr,const void *buf;size_t size;int type_ok, os_ok; -ulong load, data, len; +phys_addr_t load; +ulong data, len;uint8_t os;const char *prop_name;int ret;
@@ -1721,7 +1722,7 @@ int fit_image_load(bootm_headers_t *images, ulong addr, } }else if (load_op !=FIT_LOAD_OPTIONAL_NON_ZERO || load) {ulong image_start, image_end; -ulong load_end; +phys_addr_t load_end;void *dst;/*
@@ -1738,7 +1739,7 @@ int fit_image_load(bootm_headers_t *images, ulong addr,return -EXDEV ; } -printf (" Loading %s from 0x%08lx to 0x%08lx\n" , +printf (" Loading %s from 0x%08lx to %#08" PRIpa"\n" , prop_name, data, load); dst =map_sysmem (load, len);
@@ -1758,7 +1759,7 @@ int fit_image_load(bootm_headers_t *images, ulong addr, }int boot_get_setup_fit (bootm_headers_t *images,uint8_t arch, -ulong *setup_start,ulong *setup_len) +phys_addr_t *setup_start,ulong *setup_len) {int noffset;ulong addr;
common/image.c +10 -7
@@ -232,7 +232,7 @@ ulong image_multi_count(const image_header_t *hdr)* image_multi_getimg - get component data address and size * @hdr: pointer to the header of the multi component image * @idx: index of the requested component -* @data: pointer to a ulong variable, will hold component data address +* @data: pointer to a phys_addr_t variable, will hold component data address * @len: pointer to a ulong variable, will hold component size * * image_multi_getimg() returns size and data address for the requested
@@ -246,7 +246,7 @@ ulong image_multi_count(const image_header_t *hdr)* 0 in data and len, if idx is out of range */ void image_multi_getimg (const image_header_t *hdr,ulong idx, -ulong *data,ulong *len) +phys_addr_t *data,ulong *len) {int i;uint32_t *size;
@@ -326,7 +326,8 @@ void image_print_contents(const void *ptr)if (image_check_type (hdr,IH_TYPE_MULTI ) ||image_check_type (hdr,IH_TYPE_SCRIPT )) {int i; -ulong data, len; +phys_addr_t data; +ulong len;ulong count =image_multi_count (hdr);printf ("%sContents:\n" , p);
@@ -342,7 +343,7 @@ void image_print_contents(const void *ptr)* if planning to do something with * multiple files */ -printf ("%s Offset = 0x%08lx\n" , p, data); +printf ("%s Offset = %#08" PRIpa, p, data); } } }
@@ -895,7 +896,8 @@ int boot_get_ramdisk(int argc, char * const argv[], bootm_headers_t *images, uint8_t arch, ulong *rd_start, ulong *rd_end) { ulong rd_addr, rd_load; -ulong rd_data, rd_len; +phys_addr_t rd_data; +ulong rd_len;#if defined(CONFIG_IMAGE_FORMAT_LEGACY )const image_header_t *rd_hdr;#endif
@@ -1182,7 +1184,7 @@ error:#endif /* CONFIG_SYS_BOOT_RAMDISK_HIGH */ int boot_get_setup (bootm_headers_t *images,uint8_t arch, -ulong *setup_start,ulong *setup_len) +phys_addr_t *setup_start,ulong *setup_len) {#if defined(CONFIG_FIT )return boot_get_setup_fit (images, arch, setup_start, setup_len);
@@ -1204,7 +1206,8 @@ int boot_get_loadable(int argc, char * const argv[], bootm_headers_t *images,* These two variables are requirements for fit_image_load, but * their values are not used */ -ulong img_data, img_len; +phys_addr_t img_data; +ulong img_len;void *buf;int loadables_index;int conf_noffset;
include/bootm.h +3 -3
@@ -69,8 +69,8 @@ void arch_preboot_os(void);* @unc_len: Available space for decompression * @return 0 if OK, -ve on error (BOOTM_ERR_...) */ -int bootm_decomp_image (int comp,ulong load,ulong image_start,int type, -void *load_buf,void *image_buf,ulong image_len, -uint unc_len,ulong *load_end); +int bootm_decomp_image (int comp,phys_addr_t load,phys_addr_t image_start, +int type,void *load_buf,void *image_buf, +ulong image_len,uint unc_len,ulong *load_end);#endif
include/image.h +27 -13
@@ -33,11 +33,24 @@ struct lmb;#define IMAGE_ENABLE_IGNORE 0#define IMAGE_INDENT_STRING "" +/* Be able to hold physical address */ +typedef unsigned long long phys_addr_t ; +typedef unsigned long long phys_size_t ; +#define PRIpa "llx" +#else #include <lmb.h> #include <asm/u-boot.h> #include <command.h> +#include <linux/types.h> +#if defined(CONFIG_PHYS_64BIT ) || \ + (BITS_PER_LONG >32 ) || \ + defined(CONFIG_X86 ) +#define PRIpa "llx" +#else +#define PRIpa "lx" +#endif /* Take notice of the 'ignore' property for hashes */ #define IMAGE_ENABLE_IGNORE 1
@@ -288,9 +301,10 @@ typedef struct image_header { } image_header_t;typedef struct image_info { -ulong start, end;/* start/end of blob */ -ulong image_start, image_len;/* start of image within blob, len of image */ -ulong load;/* load addr for the image */ +phys_addr_t start, end;/* start/end of blob */ +phys_addr_t image_start;/* start of image within blob */ +ulong image_len;/* len of image */ +phys_addr_t load;/* load addr for the image */ uint8_t comp, type, os;/* compression, type of image, os type */ uint8_t arch;/* CPU architecture */ }image_info_t ;
@@ -331,7 +345,7 @@ typedef struct bootm_headers {#ifndef USE_HOSTCC image_info_t os;/* os image info */ -ulong ep;/* entry point of OS */ +phys_addr_t ep;/* entry point of OS */ ulong rd_start, rd_end;/* ramdisk start/end */
@@ -450,8 +464,8 @@ enum fit_load_op { FIT_LOAD_REQUIRED, /* Must be provided */ }; -int boot_get_setup (bootm_headers_t *images,uint8_t arch,ulong *setup_start, -ulong *setup_len); +int boot_get_setup (bootm_headers_t *images,uint8_t arch, +phys_addr_t *setup_start,ulong *setup_len);#ifndef USE_HOSTCC /* Image format types, returned by _get_format() routine */
@@ -499,7 +513,7 @@ int boot_get_loadable(int argc, char * const argv[], bootm_headers_t *images,#endif /* !USE_HOSTCC */ int boot_get_setup_fit (bootm_headers_t *images,uint8_t arch, -ulong *setup_start,ulong *setup_len); +phys_addr_t *setup_start,ulong *setup_len);/** * fit_image_load() - load an image from a FIT
@@ -534,7 +548,7 @@ int boot_get_setup_fit(bootm_headers_t *images, uint8_t arch,int fit_image_load (bootm_headers_t *images,ulong addr,const char **fit_unamep,const char **fit_uname_configp,int arch,int image_type,int bootstage_id, -enum fit_load_op load_op,ulong *datap,ulong *lenp); +enum fit_load_op load_op,phys_addr_t *datap,ulong *lenp);#ifndef USE_HOSTCC /**
@@ -703,7 +717,7 @@ static inline int image_check_os(const image_header_t *hdr, uint8_t os)ulong image_multi_count (const image_header_t *hdr);void image_multi_getimg (const image_header_t *hdr,ulong idx, -ulong *data,ulong *len); +phys_addr_t *data,ulong *len);void image_print_contents (const void *hdr);
@@ -845,8 +859,8 @@ int fit_image_get_os(const void *fit, int noffset, uint8_t *os);int fit_image_get_arch (const void *fit,int noffset,uint8_t *arch);int fit_image_get_type (const void *fit,int noffset,uint8_t *type);int fit_image_get_comp (const void *fit,int noffset,uint8_t *comp); -int fit_image_get_load (const void *fit,int noffset,ulong *load); -int fit_image_get_entry (const void *fit,int noffset,ulong *entry); +int fit_image_get_load (const void *fit,int noffset,phys_addr_t *load); +int fit_image_get_entry (const void *fit,int noffset,phys_addr_t *entry);int fit_image_get_data (const void *fit,int noffset,const void **data,size_t *size);
@@ -1122,9 +1136,9 @@ static inline int fit_image_check_target_arch(const void *fdt, int node)struct andr_img_hdr ;int android_image_check_header (const struct andr_img_hdr *hdr);int android_image_get_kernel (const struct andr_img_hdr *hdr,int verify, -ulong *os_data,ulong *os_len); +phys_addr_t *os_data,ulong *os_len);int android_image_get_ramdisk (const struct andr_img_hdr *hdr, -ulong *rd_data,ulong *rd_len); +phys_addr_t *rd_data,ulong *rd_len);ulong android_image_get_end (const struct andr_img_hdr *hdr);ulong android_image_get_kload (const struct andr_img_hdr *hdr);
tools/default_image.c +1 -1
@@ -134,7 +134,7 @@ static void image_set_header(void *ptr, struct stat *sbuf, int ifd,static int image_extract_subimage (void *ptr,struct image_tool_params *params) {const image_header_t *hdr = (const image_header_t *)ptr; -ulong file_data; +phys_addr_t file_data;ulong file_len;if (image_check_type (hdr,IH_TYPE_MULTI )) {
Signature · 1 line
1.7.9.5
RFC PATCH v5 1/4 common: Convert ulong to phys_addr_t for image addresses
Dear York Sun,
Quoted 5 lines · In message <[email protected]> you wrote:
When dealing with image addresses, ulong has been used. Some files are used by both host and target. It is OK for the target, but not always enough for host tools including mkimage. This patch replaces "ulong" with "
phys_addr_t" to make sure addresses are correct for both the target and the host.
You talk here about using "phys_addr_t"...
arch/powerpc/lib/bootm.c static void boot_jump_linux(bootm_headers_t *images) · line 50
Quoted 2 lines
arch/powerpc/lib/bootm.c +1
-1- ulong, ulong, ulong))images->ep; + ulong, ulong, ulong))(uintptr_t)images->ep;
...but here you use uintptr_t , hich is something different?
arch/powerpc/lib/bootm.c void boot_jump_vxworks(bootm_headers_t *images) · line 338
Quoted 2 lines
arch/powerpc/lib/bootm.c +1
-1- ulong, ulong, ulong))images->ep)(images->ft_addr, + ulong, ulong, ulong))(uintptr_t)images->ep)(images->ft_addr,
Ditto.
arch/x86/lib/bootm.c static int boot_prep_linux(bootm_headers_t *images) · line 90
Quoted 10 lines
arch/x86/lib/bootm.c +2
-1+phys_addr_t os_data; +ulong os_len;void *data =NULL ;size_t len;int ret;@@ -87,11 +89,10 @@ static int boot_prep_linux(bootm_headers_t *images)if (images->legacy_hdr_valid) { hdr = images->legacy_hdr_os;if (image_check_type (hdr,IH_TYPE_MULTI )) { -ulong os_data, os_len;
Why do you moe the declarations out of this block? The variables are only used within this block so there is no need for a wider scope?
arch/x86/lib/bootm.c static int boot_prep_linux(bootm_headers_t *images) · line 95
Quoted 2 lines
arch/x86/lib/bootm.c +1
-1- data = (void *)os_data; + data = (void *)(uintptr_t )os_data;
This double cast looks scary to me, and you don;t explain it in the commit message. Why exactly is this needed?
arch/x86/lib/bootm.c static int boot_prep_linux(bootm_headers_t *images) · line 126
Quoted 3 lines
arch/x86/lib/bootm.c +2
-1- cmd_line_dest = (void *)images->ep +COMMAND_LINE_OFFSET ; + cmd_line_dest = (void *)(uintptr_t )images->ep + +COMMAND_LINE_OFFSET ;
Ditto.
arch/x86/lib/bootm.c static int boot_prep_linux(bootm_headers_t *images) · line 132
Quoted 3 lines
arch/x86/lib/bootm.c +1
-2-printf ("Setup at %#08lx\n" , images->ep); - ret = setup_zimage((void *)images->ep, cmd_line_dest, +printf ("Setup at %#08" PRIpa"\n" , images->ep);
This is really ugly...
arch/x86/lib/bootm.c static int boot_prep_linux(bootm_headers_t *images) · line 133
Quoted 1 line
arch/x86/lib/bootm.c +1
-0+ ret = setup_zimage((void *)(uintptr_t)images->ep, cmd_line_dest,
See before.
arch/x86/lib/bootm.c int boot_linux_kernel(ulong setup_base, ulong load_address, bool image_64bit) · line 193
Quoted 3 lines
arch/x86/lib/bootm.c +2
-1- debug("## Transferring control to Linux (at address %08lx, kernel %08lx) ...\n", + debug("## Transferring control to Linux (at address %#08" PRIpa + ", kernel %#08" PRIpa ") ...\n",
See before...
cmd/bootm.c static int bootz_start(cmd_tbl_t *cmdtp, int flag, int argc, · line 570
Quoted 4 lines
cmd/bootm.c +2
-2-debug ("* kernel: cmdline image address = 0x%08lx\n" , - images->ep); +debug ("* kernel: cmdline image address = %#08" PRIpa"\n" , + images->ep);
Ditto. etc. etc.
cmd/ximg.c do_imgextract(cmd_tbl_t * cmdtp, int flag, int argc, char * const argv[]) · line 37
Quoted 2 lines
cmd/ximg.c +2
-0+/* +* In this function, data is decalred as phys_addr_t type.
s/decalred/declared/
cmd/ximg.c do_imgextract(cmd_tbl_t * cmdtp, int flag, int argc, char * const argv[]) · line 42
Quoted 5 lines
cmd/ximg.c +5
-0+* On some systems (eg. ARM, PowerPC) phys_addr_t can be +* "unsigned long", or "unsigned long long", depending on +* CONFIG_PHYS_64BIT. It is safe to cast 64-bit phys_addr_t +* to 32-bit pointer for image handling because the actual +* address the image is loaded is within 32-bit space.
Who guarantees that?
cmd/ximg.c do_imgextract(cmd_tbl_t * cmdtp, int flag, int argc, char * const argv[]) · line 185
Quoted 2 lines
cmd/ximg.c +1
-1- data = (ulong )fit_data; + data = (phys_addr_t )(uintptr_t )fit_data;
This double cast looks strange to me. Why is it needed?
cmd/ximg.c do_imgextract(cmd_tbl_t * cmdtp, int flag, int argc, char * const argv[]) · line 202
Quoted 2 lines
cmd/ximg.c +1
-1-void *from = (void *)data; +void *from = (void *)(uintptr_t )data;
Ditto.
cmd/ximg.c do_imgextract(cmd_tbl_t * cmdtp, int flag, int argc, char * const argv[]) · line 217
Quoted 2 lines
cmd/ximg.c +1
-1-memmove ((char *) dest, (char *)data, len); +memmove ((char *)dest, (char *)(uintptr_t )data, len);
Ditto. etc. etc.
All these double casts look somewhat wrong to me. Why are they needed?
Best regards,
Wolfgang Denk
Signature · 7 lines
DENX Software Engineering GmbH, Managing Director: Wolfgang Denk HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de Nearly everyone is in favor of going to heaven but too many are hoping they'll live long enough to see an easing of the entrance requirements. Never appeal to a man's "better nature." he might not have one.
RFC PATCH v5 1/4 common: Convert ulong to phys_addr_t for image addresses
Quoted 93 lines · Wolfgang Denk
Dear York Sun,
Quoted 5 lines · In message <[email protected]> you wrote:
When dealing with image addresses, ulong has been used. Some files are used by both host and target. It is OK for the target, but not always enough for host tools including mkimage. This patch replaces "ulong" with "
phys_addr_t" to make sure addresses are correct for both the target and the host.You talk here about using "
phys_addr_t"...arch/powerpc/lib/bootm.c static void boot_jump_linux(bootm_headers_t *images) · line 50
Quoted 2 lines
arch/powerpc/lib/bootm.c +1
-1- ulong, ulong, ulong))images->ep; + ulong, ulong, ulong))(uintptr_t)images->ep;...but here you use
uintptr_t, hich is something different?arch/powerpc/lib/bootm.c void boot_jump_vxworks(bootm_headers_t *images) · line 338
Quoted 2 lines
arch/powerpc/lib/bootm.c +1
-1- ulong, ulong, ulong))images->ep)(images->ft_addr, + ulong, ulong, ulong))(uintptr_t)images->ep)(images->ft_addr,Ditto.
arch/x86/lib/bootm.c static int boot_prep_linux(bootm_headers_t *images) · line 90
Quoted 10 lines
arch/x86/lib/bootm.c +2
-1+phys_addr_t os_data; +ulong os_len;void *data =NULL ;size_t len;int ret;@@ -87,11 +89,10 @@ static int boot_prep_linux(bootm_headers_t *images)if (images->legacy_hdr_valid) { hdr = images->legacy_hdr_os;if (image_check_type (hdr,IH_TYPE_MULTI )) { -ulong os_data, os_len;Why do you moe the declarations out of this block? The variables are only used within this block so there is no need for a wider scope?
arch/x86/lib/bootm.c static int boot_prep_linux(bootm_headers_t *images) · line 95
Quoted 2 lines
arch/x86/lib/bootm.c +1
-1- data = (void *)os_data; + data = (void *)(uintptr_t )os_data;This double cast looks scary to me, and you don;t explain it in the commit message. Why exactly is this needed?
arch/x86/lib/bootm.c static int boot_prep_linux(bootm_headers_t *images) · line 126
Quoted 3 lines
arch/x86/lib/bootm.c +2
-1- cmd_line_dest = (void *)images->ep +COMMAND_LINE_OFFSET ; + cmd_line_dest = (void *)(uintptr_t )images->ep + +COMMAND_LINE_OFFSET ;Ditto.
arch/x86/lib/bootm.c static int boot_prep_linux(bootm_headers_t *images) · line 132
Quoted 3 lines
arch/x86/lib/bootm.c +1
-2-printf ("Setup at %#08lx\n" , images->ep); - ret = setup_zimage((void *)images->ep, cmd_line_dest, +printf ("Setup at %#08" PRIpa"\n" , images->ep);This is really ugly...
arch/x86/lib/bootm.c static int boot_prep_linux(bootm_headers_t *images) · line 133
Quoted 1 line
arch/x86/lib/bootm.c +1
-0+ ret = setup_zimage((void *)(uintptr_t)images->ep, cmd_line_dest,See before.
arch/x86/lib/bootm.c int boot_linux_kernel(ulong setup_base, ulong load_address, bool image_64bit) · line 193
Quoted 3 lines
arch/x86/lib/bootm.c +2
-1- debug("## Transferring control to Linux (at address %08lx, kernel %08lx) ...\n", + debug("## Transferring control to Linux (at address %#08" PRIpa + ", kernel %#08" PRIpa ") ...\n",See before...
cmd/bootm.c static int bootz_start(cmd_tbl_t *cmdtp, int flag, int argc, · line 570
Quoted 4 lines
cmd/bootm.c +2
-2-debug ("* kernel: cmdline image address = 0x%08lx\n" , - images->ep); +debug ("* kernel: cmdline image address = %#08" PRIpa"\n" , + images->ep);Ditto. etc. etc.
cmd/ximg.c do_imgextract(cmd_tbl_t * cmdtp, int flag, int argc, char * const argv[]) · line 37
Quoted 2 lines
cmd/ximg.c +2
-0+/* +* In this function, data is decalred as phys_addr_t type. s/decalred/declared/
cmd/ximg.c do_imgextract(cmd_tbl_t * cmdtp, int flag, int argc, char * const argv[]) · line 42
Quoted 5 lines
cmd/ximg.c +5
-0+* On some systems (eg. ARM, PowerPC) phys_addr_t can be +* "unsigned long", or "unsigned long long", depending on +* CONFIG_PHYS_64BIT. It is safe to cast 64-bit phys_addr_t +* to 32-bit pointer for image handling because the actual +* address the image is loaded is within 32-bit space. Who guarantees that?
cmd/ximg.c do_imgextract(cmd_tbl_t * cmdtp, int flag, int argc, char * const argv[]) · line 185
Quoted 2 lines
cmd/ximg.c +1
-1- data = (ulong )fit_data; + data = (phys_addr_t )(uintptr_t )fit_data;This double cast looks strange to me. Why is it needed?
cmd/ximg.c do_imgextract(cmd_tbl_t * cmdtp, int flag, int argc, char * const argv[]) · line 202
Quoted 2 lines
cmd/ximg.c +1
-1-void *from = (void *)data; +void *from = (void *)(uintptr_t )data;Ditto.
Quoted 8 lines
cmd/ximg.c do_imgextract(cmd_tbl_t * cmdtp, int flag, int argc, char * const argv[]) · line 217
Quoted 2 lines
cmd/ximg.c +1
-1-memmove ((char *) dest, (char *)data, len); +memmove ((char *)dest, (char *)(uintptr_t )data, len);Ditto. etc. etc.
All these double casts look somewhat wrong to me. Why are they needed?
Dear Wolfgang,
I can use some serious help here. What I am really trying to achieve is the last two patches in this set. I didn't want to use replace ulong with phys_addr_t. I am not proud with the change I proposed, but I didn't come up with a smarter solution. My specific trouble is to build ARMv8 targets on 32-bit Ubuntu host. Some code is shared between the target and host tool (mkimage). I started from small changes, but it gets wider and wider when I tried to get rid of the compiling warnings.
York
RFC PATCH v5 1/4 common: Convert ulong to phys_addr_t for image addresses
Hi York,
Quoted 113 lines · york sun
Quoted 93 lines · Wolfgang Denk
Dear York Sun,
Quoted 5 lines · In message <[email protected]> you wrote:
When dealing with image addresses, ulong has been used. Some files are used by both host and target. It is OK for the target, but not always enough for host tools including mkimage. This patch replaces "ulong" with "
phys_addr_t" to make sure addresses are correct for both the target and the host.You talk here about using "
phys_addr_t"...arch/powerpc/lib/bootm.c static void boot_jump_linux(bootm_headers_t *images) · line 50
Quoted 2 lines
arch/powerpc/lib/bootm.c +1
-1- ulong, ulong, ulong))images->ep; + ulong, ulong, ulong))(uintptr_t)images->ep;...but here you use
uintptr_t, hich is something different?arch/powerpc/lib/bootm.c void boot_jump_vxworks(bootm_headers_t *images) · line 338
Quoted 2 lines
arch/powerpc/lib/bootm.c +1
-1- ulong, ulong, ulong))images->ep)(images->ft_addr, + ulong, ulong, ulong))(uintptr_t)images->ep)(images->ft_addr,Ditto.
arch/x86/lib/bootm.c static int boot_prep_linux(bootm_headers_t *images) · line 90
Quoted 10 lines
arch/x86/lib/bootm.c +2
-1+phys_addr_t os_data; +ulong os_len;void *data =NULL ;size_t len;int ret;@@ -87,11 +89,10 @@ static int boot_prep_linux(bootm_headers_t *images)if (images->legacy_hdr_valid) { hdr = images->legacy_hdr_os;if (image_check_type (hdr,IH_TYPE_MULTI )) { -ulong os_data, os_len;Why do you moe the declarations out of this block? The variables are only used within this block so there is no need for a wider scope?
arch/x86/lib/bootm.c static int boot_prep_linux(bootm_headers_t *images) · line 95
Quoted 2 lines
arch/x86/lib/bootm.c +1
-1- data = (void *)os_data; + data = (void *)(uintptr_t )os_data;This double cast looks scary to me, and you don;t explain it in the commit message. Why exactly is this needed?
arch/x86/lib/bootm.c static int boot_prep_linux(bootm_headers_t *images) · line 126
Quoted 3 lines
arch/x86/lib/bootm.c +2
-1- cmd_line_dest = (void *)images->ep +COMMAND_LINE_OFFSET ; + cmd_line_dest = (void *)(uintptr_t )images->ep + +COMMAND_LINE_OFFSET ;Ditto.
arch/x86/lib/bootm.c static int boot_prep_linux(bootm_headers_t *images) · line 132
Quoted 3 lines
arch/x86/lib/bootm.c +1
-2-printf ("Setup at %#08lx\n" , images->ep); - ret = setup_zimage((void *)images->ep, cmd_line_dest, +printf ("Setup at %#08" PRIpa"\n" , images->ep);This is really ugly...
arch/x86/lib/bootm.c static int boot_prep_linux(bootm_headers_t *images) · line 133
Quoted 1 line
arch/x86/lib/bootm.c +1
-0+ ret = setup_zimage((void *)(uintptr_t)images->ep, cmd_line_dest,See before.
arch/x86/lib/bootm.c int boot_linux_kernel(ulong setup_base, ulong load_address, bool image_64bit) · line 193
Quoted 3 lines
arch/x86/lib/bootm.c +2
-1- debug("## Transferring control to Linux (at address %08lx, kernel %08lx) ...\n", + debug("## Transferring control to Linux (at address %#08" PRIpa + ", kernel %#08" PRIpa ") ...\n",See before...
cmd/bootm.c static int bootz_start(cmd_tbl_t *cmdtp, int flag, int argc, · line 570
Quoted 4 lines
cmd/bootm.c +2
-2-debug ("* kernel: cmdline image address = 0x%08lx\n" , - images->ep); +debug ("* kernel: cmdline image address = %#08" PRIpa"\n" , + images->ep);Ditto. etc. etc.
cmd/ximg.c do_imgextract(cmd_tbl_t * cmdtp, int flag, int argc, char * const argv[]) · line 37
Quoted 2 lines
cmd/ximg.c +2
-0+/* +* In this function, data is decalred as phys_addr_t type. s/decalred/declared/
cmd/ximg.c do_imgextract(cmd_tbl_t * cmdtp, int flag, int argc, char * const argv[]) · line 42
Quoted 5 lines
cmd/ximg.c +5
-0+* On some systems (eg. ARM, PowerPC) phys_addr_t can be +* "unsigned long", or "unsigned long long", depending on +* CONFIG_PHYS_64BIT. It is safe to cast 64-bit phys_addr_t +* to 32-bit pointer for image handling because the actual +* address the image is loaded is within 32-bit space. Who guarantees that?
cmd/ximg.c do_imgextract(cmd_tbl_t * cmdtp, int flag, int argc, char * const argv[]) · line 185
Quoted 2 lines
cmd/ximg.c +1
-1- data = (ulong )fit_data; + data = (phys_addr_t )(uintptr_t )fit_data;This double cast looks strange to me. Why is it needed?
cmd/ximg.c do_imgextract(cmd_tbl_t * cmdtp, int flag, int argc, char * const argv[]) · line 202
Quoted 2 lines
cmd/ximg.c +1
-1-void *from = (void *)data; +void *from = (void *)(uintptr_t )data;Ditto.
Quoted 8 lines
cmd/ximg.c do_imgextract(cmd_tbl_t * cmdtp, int flag, int argc, char * const argv[]) · line 217
Quoted 2 lines
cmd/ximg.c +1
-1-memmove ((char *) dest, (char *)data, len); +memmove ((char *)dest, (char *)(uintptr_t )data, len);Ditto. etc. etc.
All these double casts look somewhat wrong to me. Why are they needed?
Dear Wolfgang,
I can use some serious help here. What I am really trying to achieve is the last two patches in this set. I didn't want to use replace ulong with
phys_addr_t. I am not proud with the change I proposed, but I didn't come up with a smarter solution. My specific trouble is to build ARMv8 targets on 32-bit Ubuntu host. Some code is shared between the target and host tool (mkimage). I started from small changes, but it gets wider and wider when I tried to get rid of the compiling warnings.
Quoted 2 lines
York
I suggest just documenting it better with comments and in the commit message. It's mostly the same comment I made.
One concern is that if you cast to uintptr_t on a 32-bit host machine, won't you end up dropping the top 32 bits?
Regards,
Simon
RFC PATCH v5 1/4 common: Convert ulong to phys_addr_t for image addresses
Quoted 116 lines · Simon Glass
Hi York,
Quoted 113 lines · york sun
Quoted 93 lines · Wolfgang Denk
Dear York Sun,
Quoted 5 lines · In message <[email protected]> you wrote:
When dealing with image addresses, ulong has been used. Some files are used by both host and target. It is OK for the target, but not always enough for host tools including mkimage. This patch replaces "ulong" with "
phys_addr_t" to make sure addresses are correct for both the target and the host.You talk here about using "
phys_addr_t"...arch/powerpc/lib/bootm.c static void boot_jump_linux(bootm_headers_t *images) · line 50
Quoted 2 lines
arch/powerpc/lib/bootm.c +1
-1- ulong, ulong, ulong))images->ep; + ulong, ulong, ulong))(uintptr_t)images->ep;...but here you use
uintptr_t, hich is something different?arch/powerpc/lib/bootm.c void boot_jump_vxworks(bootm_headers_t *images) · line 338
Quoted 2 lines
arch/powerpc/lib/bootm.c +1
-1- ulong, ulong, ulong))images->ep)(images->ft_addr, + ulong, ulong, ulong))(uintptr_t)images->ep)(images->ft_addr,Ditto.
arch/x86/lib/bootm.c static int boot_prep_linux(bootm_headers_t *images) · line 90
Quoted 10 lines
arch/x86/lib/bootm.c +2
-1+phys_addr_t os_data; +ulong os_len;void *data =NULL ;size_t len;int ret;@@ -87,11 +89,10 @@ static int boot_prep_linux(bootm_headers_t *images)if (images->legacy_hdr_valid) { hdr = images->legacy_hdr_os;if (image_check_type (hdr,IH_TYPE_MULTI )) { -ulong os_data, os_len;Why do you moe the declarations out of this block? The variables are only used within this block so there is no need for a wider scope?
arch/x86/lib/bootm.c static int boot_prep_linux(bootm_headers_t *images) · line 95
Quoted 2 lines
arch/x86/lib/bootm.c +1
-1- data = (void *)os_data; + data = (void *)(uintptr_t )os_data;This double cast looks scary to me, and you don;t explain it in the commit message. Why exactly is this needed?
arch/x86/lib/bootm.c static int boot_prep_linux(bootm_headers_t *images) · line 126
Quoted 3 lines
arch/x86/lib/bootm.c +2
-1- cmd_line_dest = (void *)images->ep +COMMAND_LINE_OFFSET ; + cmd_line_dest = (void *)(uintptr_t )images->ep + +COMMAND_LINE_OFFSET ;Ditto.
arch/x86/lib/bootm.c static int boot_prep_linux(bootm_headers_t *images) · line 132
Quoted 3 lines
arch/x86/lib/bootm.c +1
-2-printf ("Setup at %#08lx\n" , images->ep); - ret = setup_zimage((void *)images->ep, cmd_line_dest, +printf ("Setup at %#08" PRIpa"\n" , images->ep);This is really ugly...
arch/x86/lib/bootm.c static int boot_prep_linux(bootm_headers_t *images) · line 133
Quoted 1 line
arch/x86/lib/bootm.c +1
-0+ ret = setup_zimage((void *)(uintptr_t)images->ep, cmd_line_dest,See before.
arch/x86/lib/bootm.c int boot_linux_kernel(ulong setup_base, ulong load_address, bool image_64bit) · line 193
Quoted 3 lines
arch/x86/lib/bootm.c +2
-1- debug("## Transferring control to Linux (at address %08lx, kernel %08lx) ...\n", + debug("## Transferring control to Linux (at address %#08" PRIpa + ", kernel %#08" PRIpa ") ...\n",See before...
cmd/bootm.c static int bootz_start(cmd_tbl_t *cmdtp, int flag, int argc, · line 570
Quoted 4 lines
cmd/bootm.c +2
-2-debug ("* kernel: cmdline image address = 0x%08lx\n" , - images->ep); +debug ("* kernel: cmdline image address = %#08" PRIpa"\n" , + images->ep);Ditto. etc. etc.
cmd/ximg.c do_imgextract(cmd_tbl_t * cmdtp, int flag, int argc, char * const argv[]) · line 37
Quoted 2 lines
cmd/ximg.c +2
-0+/* +* In this function, data is decalred as phys_addr_t type. s/decalred/declared/
cmd/ximg.c do_imgextract(cmd_tbl_t * cmdtp, int flag, int argc, char * const argv[]) · line 42
Quoted 5 lines
cmd/ximg.c +5
-0+* On some systems (eg. ARM, PowerPC) phys_addr_t can be +* "unsigned long", or "unsigned long long", depending on +* CONFIG_PHYS_64BIT. It is safe to cast 64-bit phys_addr_t +* to 32-bit pointer for image handling because the actual +* address the image is loaded is within 32-bit space. Who guarantees that?
cmd/ximg.c do_imgextract(cmd_tbl_t * cmdtp, int flag, int argc, char * const argv[]) · line 185
Quoted 2 lines
cmd/ximg.c +1
-1- data = (ulong )fit_data; + data = (phys_addr_t )(uintptr_t )fit_data;This double cast looks strange to me. Why is it needed?
cmd/ximg.c do_imgextract(cmd_tbl_t * cmdtp, int flag, int argc, char * const argv[]) · line 202
Quoted 2 lines
cmd/ximg.c +1
-1-void *from = (void *)data; +void *from = (void *)(uintptr_t )data;Ditto.
Quoted 8 lines
cmd/ximg.c do_imgextract(cmd_tbl_t * cmdtp, int flag, int argc, char * const argv[]) · line 217
Quoted 2 lines
cmd/ximg.c +1
-1-memmove ((char *) dest, (char *)data, len); +memmove ((char *)dest, (char *)(uintptr_t )data, len);Ditto. etc. etc.
All these double casts look somewhat wrong to me. Why are they needed?
Dear Wolfgang,
I can use some serious help here. What I am really trying to achieve is the last two patches in this set. I didn't want to use replace ulong with
phys_addr_t. I am not proud with the change I proposed, but I didn't come up with a smarter solution. My specific trouble is to build ARMv8 targets on 32-bit Ubuntu host. Some code is shared between the target and host tool (mkimage). I started from small changes, but it gets wider and wider when I tried to get rid of the compiling warnings.
Quoted 9 lines
Quoted 2 lines
York
I suggest just documenting it better with comments and in the commit message. It's mostly the same comment I made.
One concern is that if you cast to
uintptr_ton a 32-bit host machine, won't you end up dropping the top 32 bits?
Simon,
Some code is shared between targets and hosts, but not all. The ugly casting is used by targets only. Maybe I should take another look at the issue and back away from converting ulong to phys_addr_t totally. It is only broken on 32-bit host tool and seems nobody really cares. Sandbox is also broken on 32-bit host (compiling warnings), and yet no one complains. Am I the only one living in old age? I can upgrade to 64-bit Ubuntu and we all can forget about this mess I created. (Getting exhausted here...)
York
RFC PATCH v5 1/4 common: Convert ulong to phys_addr_t for image addresses
Hi York,
Quoted 129 lines · york sun
Quoted 116 lines · Simon Glass
Hi York,
Quoted 113 lines · york sun
Quoted 93 lines · Wolfgang Denk
Dear York Sun,
Quoted 5 lines · In message <[email protected]> you wrote:
When dealing with image addresses, ulong has been used. Some files are used by both host and target. It is OK for the target, but not always enough for host tools including mkimage. This patch replaces "ulong" with "
phys_addr_t" to make sure addresses are correct for both the target and the host.You talk here about using "
phys_addr_t"...arch/powerpc/lib/bootm.c static void boot_jump_linux(bootm_headers_t *images) · line 50
Quoted 2 lines
arch/powerpc/lib/bootm.c +1
-1- ulong, ulong, ulong))images->ep; + ulong, ulong, ulong))(uintptr_t)images->ep;...but here you use
uintptr_t, hich is something different?arch/powerpc/lib/bootm.c void boot_jump_vxworks(bootm_headers_t *images) · line 338
Quoted 2 lines
arch/powerpc/lib/bootm.c +1
-1- ulong, ulong, ulong))images->ep)(images->ft_addr, + ulong, ulong, ulong))(uintptr_t)images->ep)(images->ft_addr,Ditto.
arch/x86/lib/bootm.c static int boot_prep_linux(bootm_headers_t *images) · line 90
Quoted 10 lines
arch/x86/lib/bootm.c +2
-1+phys_addr_t os_data; +ulong os_len;void *data =NULL ;size_t len;int ret;@@ -87,11 +89,10 @@ static int boot_prep_linux(bootm_headers_t *images)if (images->legacy_hdr_valid) { hdr = images->legacy_hdr_os;if (image_check_type (hdr,IH_TYPE_MULTI )) { -ulong os_data, os_len;Why do you moe the declarations out of this block? The variables are only used within this block so there is no need for a wider scope?
arch/x86/lib/bootm.c static int boot_prep_linux(bootm_headers_t *images) · line 95
Quoted 2 lines
arch/x86/lib/bootm.c +1
-1- data = (void *)os_data; + data = (void *)(uintptr_t )os_data;This double cast looks scary to me, and you don;t explain it in the commit message. Why exactly is this needed?
arch/x86/lib/bootm.c static int boot_prep_linux(bootm_headers_t *images) · line 126
Quoted 3 lines
arch/x86/lib/bootm.c +2
-1- cmd_line_dest = (void *)images->ep +COMMAND_LINE_OFFSET ; + cmd_line_dest = (void *)(uintptr_t )images->ep + +COMMAND_LINE_OFFSET ;Ditto.
arch/x86/lib/bootm.c static int boot_prep_linux(bootm_headers_t *images) · line 132
Quoted 3 lines
arch/x86/lib/bootm.c +1
-2-printf ("Setup at %#08lx\n" , images->ep); - ret = setup_zimage((void *)images->ep, cmd_line_dest, +printf ("Setup at %#08" PRIpa"\n" , images->ep);This is really ugly...
arch/x86/lib/bootm.c static int boot_prep_linux(bootm_headers_t *images) · line 133
Quoted 1 line
arch/x86/lib/bootm.c +1
-0+ ret = setup_zimage((void *)(uintptr_t)images->ep, cmd_line_dest,See before.
arch/x86/lib/bootm.c int boot_linux_kernel(ulong setup_base, ulong load_address, bool image_64bit) · line 193
Quoted 3 lines
arch/x86/lib/bootm.c +2
-1- debug("## Transferring control to Linux (at address %08lx, kernel %08lx) ...\n", + debug("## Transferring control to Linux (at address %#08" PRIpa + ", kernel %#08" PRIpa ") ...\n",See before...
cmd/bootm.c static int bootz_start(cmd_tbl_t *cmdtp, int flag, int argc, · line 570
Quoted 4 lines
cmd/bootm.c +2
-2-debug ("* kernel: cmdline image address = 0x%08lx\n" , - images->ep); +debug ("* kernel: cmdline image address = %#08" PRIpa"\n" , + images->ep);Ditto. etc. etc.
cmd/ximg.c do_imgextract(cmd_tbl_t * cmdtp, int flag, int argc, char * const argv[]) · line 37
Quoted 2 lines
cmd/ximg.c +2
-0+/* +* In this function, data is decalred as phys_addr_t type. s/decalred/declared/
cmd/ximg.c do_imgextract(cmd_tbl_t * cmdtp, int flag, int argc, char * const argv[]) · line 42
Quoted 5 lines
cmd/ximg.c +5
-0+* On some systems (eg. ARM, PowerPC) phys_addr_t can be +* "unsigned long", or "unsigned long long", depending on +* CONFIG_PHYS_64BIT. It is safe to cast 64-bit phys_addr_t +* to 32-bit pointer for image handling because the actual +* address the image is loaded is within 32-bit space. Who guarantees that?
cmd/ximg.c do_imgextract(cmd_tbl_t * cmdtp, int flag, int argc, char * const argv[]) · line 185
Quoted 2 lines
cmd/ximg.c +1
-1- data = (ulong )fit_data; + data = (phys_addr_t )(uintptr_t )fit_data;This double cast looks strange to me. Why is it needed?
cmd/ximg.c do_imgextract(cmd_tbl_t * cmdtp, int flag, int argc, char * const argv[]) · line 202
Quoted 2 lines
cmd/ximg.c +1
-1-void *from = (void *)data; +void *from = (void *)(uintptr_t )data;Ditto.
Quoted 8 lines
cmd/ximg.c do_imgextract(cmd_tbl_t * cmdtp, int flag, int argc, char * const argv[]) · line 217
Quoted 2 lines
cmd/ximg.c +1
-1-memmove ((char *) dest, (char *)data, len); +memmove ((char *)dest, (char *)(uintptr_t )data, len);Ditto. etc. etc.
All these double casts look somewhat wrong to me. Why are they needed?
Dear Wolfgang,
I can use some serious help here. What I am really trying to achieve is the last two patches in this set. I didn't want to use replace ulong with
phys_addr_t. I am not proud with the change I proposed, but I didn't come up with a smarter solution. My specific trouble is to build ARMv8 targets on 32-bit Ubuntu host. Some code is shared between the target and host tool (mkimage). I started from small changes, but it gets wider and wider when I tried to get rid of the compiling warnings.Quoted 9 lines
Quoted 2 lines
York
I suggest just documenting it better with comments and in the commit message. It's mostly the same comment I made.
One concern is that if you cast to
uintptr_ton a 32-bit host machine, won't you end up dropping the top 32 bits?Simon,
Quoted 7 lines
Some code is shared between targets and hosts, but not all. The ugly casting is used by targets only. Maybe I should take another look at the issue and back away from converting ulong to
phys_addr_ttotally. It is only broken on 32-bit host tool and seems nobody really cares. Sandbox is also broken on 32-bit host (compiling warnings), and yet no one complains. Am I the only one living in old age? I can upgrade to 64-bit Ubuntu and we all can forget about this mess I created. (Getting exhausted here...)
Well maybe. You could instead just print an error and quit when trying to use 64-bit images on a 32-bit host machine. I think that would be acceptable these days.
Regards,
Simon
RFC PATCH v5 1/4 common: Convert ulong to phys_addr_t for image addresses
On 02/26/2016 10:05 AM, Simon Glass wrote:
<snip>
Quoted 34 lines
Quoted 26 lines
Quoted 14 lines
Quoted 14 lines
Quoted 3 lines
All these double casts look somewhat wrong to me. Why are they needed?
Dear Wolfgang,
I can use some serious help here. What I am really trying to achieve is the last two patches in this set. I didn't want to use replace ulong with
phys_addr_t. I am not proud with the change I proposed, but I didn't come up with a smarter solution. My specific trouble is to build ARMv8 targets on 32-bit Ubuntu host. Some code is shared between the target and host tool (mkimage). I started from small changes, but it gets wider and wider when I tried to get rid of the compiling warnings.Quoted 9 lines
Quoted 2 lines
York
I suggest just documenting it better with comments and in the commit message. It's mostly the same comment I made.
One concern is that if you cast to
uintptr_ton a 32-bit host machine, won't you end up dropping the top 32 bits?Simon,
Quoted 7 lines
Some code is shared between targets and hosts, but not all. The ugly casting is used by targets only. Maybe I should take another look at the issue and back away from converting ulong to
phys_addr_ttotally. It is only broken on 32-bit host tool and seems nobody really cares. Sandbox is also broken on 32-bit host (compiling warnings), and yet no one complains. Am I the only one living in old age? I can upgrade to 64-bit Ubuntu and we all can forget about this mess I created. (Getting exhausted here...)
Quoted 4 lines
Well maybe. You could instead just print an error and quit when trying to use 64-bit images on a 32-bit host machine. I think that would be acceptable these days.
Sounds good to me. Let me try to rework the patch set.
York
RFC PATCH v5 2/4 sandbox: Fix compiling warning
In reply to York SunFix the following compiling warning. Some only occur on 32-bit hosts
disk/part_efi.c: In function 'alloc_read_gpt_entries':disk/part_efi.c:894:2: warning: format '%zu' expects argument of
type 'size_t', but argument 5 has type 'long unsigned int' [-Wformat]disk/part_efi.c:907:4: warning: format '%zX' expects argument of
type 'size_t', but argument 3 has type 'long unsigned int' [-Wformat]cmd/lzmadec.c: In function 'do_lzmadec':cmd/lzmadec.c:39:12: warning: passing argument 2 of
'lzmaBuffToBuffDecompress' from incompatible pointer type [enabled by default]include/lzma/../../lib/lzma/LzmaTools.h:17:12: note: expected
'SizeT *' but argument is of type 'long unsigned int *'lib/hashtable.c: In function 'hexport_r':lib/hashtable.c:605:2: warning: format '%zu' expects argument of
type 'size_t', but argument 5 has type 'long unsigned int' [-Wformat]lib/hashtable.c:661:5: warning: format '%zu' expects argument of
type 'size_t', but argument 2 has type 'long unsigned int' [-Wformat]lib/hashtable.c:661:5: warning: format '%zu' expects argument of
type 'size_t', but argument 3 has type 'long unsigned int' [-Wformat]lib/hashtable.c: In function 'himport_r':lib/hashtable.c:793:3: warning: format '%zu' expects argument of
type 'size_t', but argument 2 has type 'long unsigned int' [-Wformat]
---
Changes in v5:
Revise commit message
Drop % sign in PRIpa macro
Use %#08llx instead of %08llx when applicable
Changes in v4:
New patch to fix compiling warnings for sandbox built on 32-bit host Still need to change CONFIG_SANDBOX_BITS_PER_LONG to 32 to avoid these warnings
include/asm-generic/bitops/__fls.h:17:2: warning: left shift
count >= width of type [enabled by default]
include/asm-generic/bitops/__fls.h:19:3: warning: left shift
count >= width of type [enabled by default]
include/asm-generic/bitops/__fls.h:22:2: warning: left shift
count >= width of type [enabled by default]
include/asm-generic/bitops/__fls.h:26:2: warning: left shift
count >= width of type [enabled by default]
include/asm-generic/bitops/__fls.h:30:2: warning: left shift
count >= width of type [enabled by default]
include/asm-generic/bitops/__fls.h:34:2: warning: left shift
count >= width of type [enabled by default]
include/asm-generic/bitops/__fls.h:38:2: warning: left shift
count >= width of type [enabled by default]
Changes in v3: None
Changes in v2: None
arch/sandbox/cpu/cpu.c | 3 ++- arch/sandbox/include/asm/types.h | 9 +++++++-- arch/sandbox/lib/bootm.c | 3 ++- arch/sandbox/lib/pci_io.c | 2 +- cmd/lzmadec.c | 5 +++-- disk/part_efi.c | 2 +- lib/hashtable.c | 2 +- 7 files changed, 17 insertions(+), 9 deletions(-)
arch/sandbox/cpu/cpu.c +2 -1
@@ -62,7 +62,8 @@ void *map_physmem(phys_addr_t paddr, unsigned long len, unsigned long flags) map_dev =NULL ;if (enable_pci_map && !pci_map_physmem (paddr, &len, &map_dev, &ptr)) {if (plen != len) { -printf ("%s: Warning: partial map at %x, wanted %lx, got %lx\n" , +printf ("%s: Warning: partial map at %" PRIpa +", wanted %lx, got %lx\n" , __func__, paddr, len, plen); } map_len = len;
arch/sandbox/include/asm/types.h +7 -2
@@ -53,8 +53,13 @@ typedef __UINT64_TYPE__ u64;#define BITS_PER_LONG CONFIG_SANDBOX_BITS_PER_LONGtypedef unsigned long dma_addr_t ; -typedef u32 phys_addr_t ; -typedef u32 phys_size_t ; +#if defined(CONFIG_PHYS_64BIT ) || (BITS_PER_LONG >32 ) +/* To be consistent with print format %llx */ +typedef unsigned long long phys_addr_t ; +#else +typedef unsigned long phys_addr_t ; +#endif +typedef unsigned long phys_size_t ;#endif /* __KERNEL__ */
arch/sandbox/lib/bootm.c +2 -1
@@ -54,7 +54,8 @@ int do_bootm_linux(int flag, int argc, char *argv[], bootm_headers_t *images) {if (flag & (BOOTM_STATE_OS_GO |BOOTM_STATE_OS_FAKE_GO )) {bootstage_mark (BOOTSTAGE_ID_RUN_OS ); -printf ("## Transferring control to Linux (at address %08lx)...\n" , +printf ("## Transferring control to Linux (at address %#08" PRIpa +")...\n" , images->ep);reset_cpu (0 ); }
arch/sandbox/lib/pci_io.c +1 -1
@@ -35,7 +35,7 @@ int pci_map_physmem(phys_addr_t paddr, unsigned long *lenp,return 0 ; } -debug ("%s: failed: addr=%x\n" , __func__, paddr); +debug ("%s: failed: addr=%" PRIpa"\n" , __func__, paddr);return -ENOSYS ; }
cmd/lzmadec.c +3 -2
@@ -20,7 +20,7 @@static int do_lzmadec (cmd_tbl_t *cmdtp,int flag,int argc,char *const argv[]) {unsigned long src, dst; -unsigned long src_len = ~0UL , dst_len = ~0UL ; +SizeT src_len = ~0UL , dst_len = ~0UL ;int ret;switch (argc) {
@@ -40,7 +40,8 @@ static int do_lzmadec(cmd_tbl_t *cmdtp, int flag, int argc, char *const argv[])if (ret !=SZ_OK )return 1 ; -printf ("Uncompressed size: %ld = 0x%lX\n" , src_len, src_len); +printf ("Uncompressed size: %ld = 0x%lX\n" , + (ulong )src_len, (ulong )src_len);setenv_hex ("filesize" , src_len);return 0 ;
disk/part_efi.c +1 -1
@@ -10,7 +10,6 @@* when CONFIG_SYS_64BIT_LBA is not defined, lbaint_t is 32 bits; this * limits the maximum size of addressable storage to < 2 Terra Bytes */ -#include <asm/unaligned.h> #include <common.h> #include <command.h> #include <ide.h>
@@ -19,6 +18,7 @@#include <memalign.h> #include <part_efi.h> #include <linux/ctype.h> +#include <asm/unaligned.h> DECLARE_GLOBAL_DATA_PTR ;
lib/hashtable.c +1 -1
@@ -13,6 +13,7 @@* SPDX-License-Identifier: LGPL-2.1+ */ +#include <common.h> #include <errno.h> #include <malloc.h>
@@ -29,7 +30,6 @@# endif # endif #else /* U-Boot build */ -# include <common.h> # include <linux/string.h> # include <linux/ctype.h> #endif
Signature · 1 line
1.7.9.5
RFC PATCH v5 3/4 common: image-fit: Use a common function to get address
In reply to York SunFIT image supports load address and entry address. Getting these addresses can use a common function.
---
Changes in v5:
New patch split from fixing load and entry address patch
Changes in v4: None
Changes in v3: None
Changes in v2: None
common/image-fit.c | 42 +++++++++++++++++++----------------------- 1 file changed, 19 insertions(+), 23 deletions(-)
common/image-fit.c +19 -23
@@ -433,7 +433,7 @@ void fit_image_print(const void *fit, int image_noffset, const char *p)if ((type ==IH_TYPE_KERNEL ) || (type ==IH_TYPE_STANDALONE ) || (type ==IH_TYPE_RAMDISK )) { -fit_image_get_entry (fit, image_noffset, &entry); + ret =fit_image_get_entry (fit, image_noffset, &entry);printf ("%s Entry Point: " , p);if (ret)printf ("unavailable\n" );
@@ -675,6 +675,22 @@ int fit_image_get_comp(const void *fit, int noffset, uint8_t *comp)return 0 ; } +static int fit_image_get_address (const void *fit,int noffset,char *name, +phys_addr_t *load) +{ +int len; +const uint32_t *data; + + data =fdt_getprop (fit, noffset, name, &len); +if (data ==NULL ) { +fit_get_debug (fit, noffset, name, len); +return -1 ; + } + + *load =uimage_to_cpu (*data); + +return 0 ; +}/** * fit_image_get_load() - get load addr property for given component image node * @fit: pointer to the FIT format image header
@@ -690,17 +706,7 @@ int fit_image_get_comp(const void *fit, int noffset, uint8_t *comp)*/ int fit_image_get_load (const void *fit,int noffset,phys_addr_t *load) { -int len; -const uint32_t *data; - - data =fdt_getprop (fit, noffset,FIT_LOAD_PROP , &len); -if (data ==NULL ) { -fit_get_debug (fit, noffset,FIT_LOAD_PROP , len); -return -1 ; - } - - *load =uimage_to_cpu (*data); -return 0 ; +return fit_image_get_address (fit, noffset,FIT_LOAD_PROP , load); }/**
@@ -722,17 +728,7 @@ int fit_image_get_load(const void *fit, int noffset, phys_addr_t *load)*/ int fit_image_get_entry (const void *fit,int noffset,phys_addr_t *entry) { -int len; -const uint32_t *data; - - data =fdt_getprop (fit, noffset,FIT_ENTRY_PROP , &len); -if (data ==NULL ) { -fit_get_debug (fit, noffset,FIT_ENTRY_PROP , len); -return -1 ; - } - - *entry =uimage_to_cpu (*data); -return 0 ; +return fit_image_get_address (fit, noffset,FIT_ENTRY_PROP , entry); }/**
Signature · 1 line
1.7.9.5
RFC PATCH v5 4/4 common: image-fit: Fix load and entry addresses in FIT image
In reply to York SunFIT image supports more than 32 bits in addresses by using #address-cell field. Fixing 64-bit support by using this field.
---
Changes in v5:
Split the common function into another patch.
Revise commit subject.
Update commit message as suggested by Simon.
Updated cover letter with testing report.
Changes in v4:
Separate ulong to phys_addr_t change to another patch.
Changes in v3:
Define PRIpa for host and target in common/image-fit.c so printf works properly for 32-, 64-bit targets and host tools.
Changes in v2:
Make a common function for both load and entry addresses. Simplify calculation of addresses in a similar way as fdtdec_get_number() fdtdec_get_number() is not used, or too many files need to be included
and/or twisted for host tool
Continue to use %08llx for print format for load and entry addresses
because %pa does not always work for host tool (mkimage)
common/image-fit.c | 22 +++++++++++++++++----- 1 file changed, 17 insertions(+), 5 deletions(-)
common/image-fit.c +17 -5
@@ -678,16 +678,28 @@ int fit_image_get_comp(const void *fit, int noffset, uint8_t *comp)static int fit_image_get_address (const void *fit,int noffset,char *name,phys_addr_t *load) { -int len; -const uint32_t *data; +int len, cell_len; +const fdt32_t *cell; +unsigned long long load64 =0 ; - data =fdt_getprop (fit, noffset, name, &len); -if (data ==NULL ) { + cell =fdt_getprop (fit, noffset, name, &len); +if (cell ==NULL ) {fit_get_debug (fit, noffset, name, len);return -1 ; } - *load =uimage_to_cpu (*data); +if (len >sizeof (phys_addr_t)) { +printf ("Unsupported %s address size\n" , name); +return -1 ; + } + + cell_len = len >>2 ; +/* Use load64 to avoid compiling warning for 32-bit target */ +while (cell_len--) { + load64 = (load64 <<32 ) |uimage_to_cpu (*cell); + cell++; + } + *load = (phys_addr_t )load64;return 0 ; }
Signature · 1 line
1.7.9.5