Reorganize and fix problems (returns) in the bootm command.
PATCH 6/17 Reorganize and fix problems (returns) in the bootm command.
Do *NOT* return after the "point of no return" has been passed.
If something goes wrong, the board must be reset after that point.
Move the "Transferring control to Linux" debug message back to where it
belongs: just before transferring control to linux.
--- common/cmd_bootm.c | 130 +++++++++++++++++++++++++-------------------------- 1 files changed, 64 insertions(+), 66 deletions(-)
common/cmd_bootm.c +64 -66
@@ -45,8 +45,8 @@DECLARE_GLOBAL_DATA_PTR ; -/*cmd_boot.c*/ -extern int do_reset (cmd_tbl_t *cmdtp,int flag,int argc,char *argv[]); +/*cmd_boot.c*/ +extern int do_reset (cmd_tbl_t *cmdtp,int flag,int argc,char *argv[]);#if (CONFIG_COMMANDS &CFG_CMD_DATE ) || defined(CONFIG_TIMESTAMP )#include <rtc.h>
@@ -763,8 +763,8 @@ do_bootm_linux (cmd_tbl_t *cmdtp, int flag,if ((ntohl (hdr->ih_load) < ((unsigned long )hdr +ntohl (hdr->ih_size) +sizeof (hdr))) && ((ntohl (hdr->ih_load) +ntohl (hdr->ih_size)) > (unsigned long )hdr)) { -printf ("ERROR: Load address overwrites Flat Device Tree uImage\n" ); -return ; +puts ("ERROR: Load address overwrites Flat Device Tree uImage\nMust RESET board to recover\n" ); +do_reset (cmdtp, flag, argc, argv); }printf (" Verifying Checksum ... " );
@@ -773,34 +773,34 @@ do_bootm_linux (cmd_tbl_t *cmdtp, int flag, header.ih_hcrc =0 ;if (checksum !=crc32 (0 , (uchar *)&header,sizeof (image_header_t))) { -printf ("ERROR: Flat Device Tree header checksum is invalid\n" ); -return ; +puts ("ERROR: Flat Device Tree header checksum is invalid\nMust RESET board to recover\n" ); +do_reset (cmdtp, flag, argc, argv); } checksum =ntohl (hdr->ih_dcrc); addr = (ulong)((uchar *)(hdr) +sizeof (image_header_t));if (checksum !=crc32 (0 , (uchar *)addr,ntohl (hdr->ih_size))) { -printf ("ERROR: Flat Device Tree checksum is invalid\n" ); -return ; +puts ("ERROR: Flat Device Tree checksum is invalid\nMust RESET board to recover\n" ); +do_reset (cmdtp, flag, argc, argv); }printf ("OK\n" );if (ntohl (hdr->ih_type) !=IH_TYPE_FLATDT ) { -printf ("ERROR: uImage not Flat Device Tree type\n" ); -return ; +puts ("ERROR: uImage not Flat Device Tree type\nMust RESET board to recover\n" ); +do_reset (cmdtp, flag, argc, argv); }if (ntohl (hdr->ih_comp) !=IH_COMP_NONE ) { -printf ("ERROR: uImage is not uncompressed\n" ); -return ; +puts ("ERROR: uImage is not uncompressed\nMust RESET board to recover\n" ); +do_reset (cmdtp, flag, argc, argv); }#if defined(CONFIG_OF_LIBFDT )if (fdt_check_header (of_flat_tree +sizeof (image_header_t)) !=0 ) {#else if (*((ulong *)(of_flat_tree +sizeof (image_header_t))) !=OF_DT_HEADER ) {#endif -printf ("ERROR: uImage data is not a flat device tree\n" ); -return ; +puts ("ERROR: uImage data is not a flat device tree\nMust RESET board to recover\n" ); +do_reset (cmdtp, flag, argc, argv); } memmove((void *)ntohl(hdr->ih_load),
@@ -808,8 +808,8 @@ do_bootm_linux (cmd_tbl_t *cmdtp, int flag, ntohl(hdr->ih_size)); of_flat_tree = (char *)ntohl (hdr->ih_load); }else { -printf ("Did not find a flat flat device tree at address %08lX\n" , of_flat_tree); -return ; +puts ("Did not find a flat flat device tree\nMust RESET board to recover\n" ); +do_reset (cmdtp, flag, argc, argv); }printf (" Booting using flat device tree at 0x%x\n" , of_flat_tree);
@@ -840,8 +840,8 @@ do_bootm_linux (cmd_tbl_t *cmdtp, int flag,#else if (((struct boot_param_header *)of_data)->magic !=OF_DT_HEADER ) {#endif -printf ("ERROR: image is not a flat device tree\n" ); -return ; +puts ("ERROR: image is not a flat device tree\nMust RESET board to recover\n" ); +do_reset (cmdtp, flag, argc, argv); }#if defined(CONFIG_OF_LIBFDT )
@@ -849,8 +849,8 @@ do_bootm_linux (cmd_tbl_t *cmdtp, int flag,#else if (((struct boot_param_header *)of_data)->totalsize !=ntohl (len_ptr[2 ])) {#endif -printf ("ERROR: flat device tree size does not agree with image\n" ); -return ; +puts ("ERROR: flat device tree size does not agree with image\nMust RESET board to recover\n" ); +do_reset (cmdtp, flag, argc, argv); } }#endif
@@ -923,15 +923,6 @@ do_bootm_linux (cmd_tbl_t *cmdtp, int flag, initrd_end =0 ; } -debug ("## Transferring control to Linux (at address %08lx) ...\n" , - (ulong )kernel); - -SHOW_BOOT_PROGRESS (15 ); - -#if defined(CFG_INIT_RAM_LOCK ) && !defined(CONFIG_E500 ) -unlock_ram_in_cache (); -#endif -#if defined(CONFIG_OF_LIBFDT )/* move of_flat_tree if needed */ if (of_data) {
@@ -960,19 +951,19 @@ do_bootm_linux (cmd_tbl_t *cmdtp, int flag,* if the user wants it (the logic is in the subroutines). */ if (fdt_chosen (of_flat_tree, initrd_start, initrd_end,0 ) <0 ) { -printf ("Failed creating the /chosen node (0x%08X), aborting.\n" , of_flat_tree); -return ; +puts ("ERROR: Failed creating the /chosen node, aborting.\nMust RESET board to recover\n" ); +do_reset (cmdtp, flag, argc, argv); }#ifdef CONFIG_OF_HAS_UBOOT_ENV if (fdt_env (of_flat_tree) <0 ) { -printf ("Failed creating the /u-boot-env node, aborting.\n" ); -return ; +puts ("ERROR: Failed creating the /u-boot-env node, aborting.\nMust RESET board to recover\n" ); +do_reset (cmdtp, flag, argc, argv); }#endif #ifdef CONFIG_OF_HAS_BD_T if (fdt_bd_t (of_flat_tree) <0 ) { -printf ("Failed creating the /bd_t node, aborting.\n" ); -return ; +puts ("ERROR: Failed creating the /bd_t node, aborting.\nMust RESET board to recover\n" ); +do_reset (cmdtp, flag, argc, argv); }#endif }
@@ -997,55 +988,62 @@ do_bootm_linux (cmd_tbl_t *cmdtp, int flag,memmove ((void *)of_start, (void *)of_data, of_len); }#endif - -/* -* Linux Kernel Parameters (passing board info data): -* r3: ptr to board info data -* r4: initrd_start or 0 if no initrd -* r5: initrd_end - unused if r4 is 0 -* r6: Start of command line string -* r7: End of command line string -*/ -#if defined(CONFIG_OF_FLAT_TREE ) || defined(CONFIG_OF_LIBFDT ) -if (!of_flat_tree)/* no device tree; boot old style */ -#endif - (*kernel) (kbd, initrd_start, initrd_end, cmd_start, cmd_end); -/* does not return */ - -#if defined(CONFIG_OF_FLAT_TREE ) || defined(CONFIG_OF_LIBFDT ) -/* -* Linux Kernel Parameters (passing device tree): -* r3: ptr to OF flat tree, followed by the board info data -* r4: physical pointer to the kernel itself -* r5: NULL -* r6: NULL -* r7: NULL -*/ #if defined(CONFIG_OF_FLAT_TREE )ft_setup (of_flat_tree, kbd, initrd_start, initrd_end);/* ft_dump_blob(of_flat_tree); */ #endif #if defined(CONFIG_OF_LIBFDT )if (fdt_chosen (of_flat_tree, initrd_start, initrd_end,0 ) <0 ) { -printf ("Failed creating the /chosen node (0x%08X), aborting.\n" , of_flat_tree); -return ; +puts ("ERROR: Failed to create the /chosen node, aborting.\nMust RESET board to recover\n" ); +do_reset (cmdtp, flag, argc, argv); }#ifdef CONFIG_OF_HAS_UBOOT_ENV if (fdt_env (of_flat_tree) <0 ) { -printf ("Failed creating the /u-boot-env node, aborting.\n" ); -return ; +puts ("ERROR: Failed to create the /u-boot-env node, aborting.\nMust RESET board to recover\n" ); +do_reset (cmdtp, flag, argc, argv); }#endif #ifdef CONFIG_OF_HAS_BD_T if (fdt_bd_t (of_flat_tree) <0 ) { -printf ("Failed creating the /bd_t node, aborting.\n" ); -return ; +puts ("ERROR: Failed to create the /bd_t node, aborting.\nMust RESET board to recover\n" ); +do_reset (cmdtp, flag, argc, argv); }#endif #endif /* if defined(CONFIG_OF_LIBFDT) */ - (*kernel) ((bd_t *)of_flat_tree, (ulong )kernel,0 ,0 ,0 ); +debug ("## Transferring control to Linux (at address %08lx) ...\n" , + (ulong )kernel); + +SHOW_BOOT_PROGRESS (15 ); + +#if defined(CFG_INIT_RAM_LOCK ) && !defined(CONFIG_E500 ) +unlock_ram_in_cache ();#endif + +#if defined(CONFIG_OF_FLAT_TREE ) || defined(CONFIG_OF_LIBFDT ) +if (of_flat_tree) {/* device tree; boot new style */ +/* +* Linux Kernel Parameters (passing device tree): +* r3: ptr to flattened device tree, followed by the board info data +* r4: physical pointer to the kernel itself +* r5: NULL +* r6: NULL +* r7: NULL +*/ + (*kernel) ((bd_t *)of_flat_tree, (ulong )kernel,0 ,0 ,0 ); +/* does not return */ + } +#endif +/* +* Linux Kernel Parameters (passing board info data): +* r3: ptr to board info data +* r4: initrd_start or 0 if no initrd +* r5: initrd_end - unused if r4 is 0 +* r6: Start of command line string +* r7: End of command line string +*/ + (*kernel) (kbd, initrd_start, initrd_end, cmd_start, cmd_end); +/* does not return */ }#endif /* CONFIG_PPC */
Signature · 1 line
1.4.4.4
PATCH 6/17 Reorganize and fix problems (returns) in the bootm command.
Quoted 4 lines · Jerry Van Baren
Do *NOT* return after the "point of no return" has been passed.
If something goes wrong, the board must be reset after that point.
Move the "Transferring control to Linux" debug message back to where it
belongs: just before transferring control to linux.
Why is it necessary to reset the board at this point? The failure paths here seem to be things like invalid checksums and the like. Why are these tests after a point of no return?
I would think that if any of these failures are hit, you would *not* want to reset the board so you can figure out what was going on.
Cheers,
g.
Signature · 4 lines
Grant Likely, B.Sc., P.Eng. Secret Lab Technologies Ltd. grant.likely at secretlab.ca (403) 399-0195
PATCH 6/17 Reorganize and fix problems (returns) in the bootm command.
Quoted 10 lines · Grant Likely
Quoted 4 lines · Jerry Van Baren
Do *NOT* return after the "point of no return" has been passed.
If something goes wrong, the board must be reset after that point.
Move the "Transferring control to Linux" debug message back to where it
belongs: just before transferring control to linux.Why is it necessary to reset the board at this point? The failure paths here seem to be things like invalid checksums and the like. Why are these tests after a point of no return?
Quoted 5 lines
I would think that if any of these failures are hit, you would *not* want to reset the board so you can figure out what was going on.
Cheers,
g.
We've smashed our underpinnings by uncompressing linux over our interrupt vectors before the checksum catches the problem. There is no way back other than a reset. The error I fixed (in a couple of places) was returns. FWIIW, my experience with the erroneous returns is that it sort of works, but not right and then funky stuff happens. :-/
I presume the comment was a general one. If you have any specific reset vs. return questions, hollar and I'll justify them or eat crow.
Thanks,
gvb
PATCH 6/17 Reorganize and fix problems (returns) in the bootm command.
Quoted 17 lines · Jerry Van Baren
Quoted 10 lines · Grant Likely
Quoted 4 lines · Jerry Van Baren
Do *NOT* return after the "point of no return" has been passed.
If something goes wrong, the board must be reset after that point.
Move the "Transferring control to Linux" debug message back to where it
belongs: just before transferring control to linux.Why is it necessary to reset the board at this point? The failure paths here seem to be things like invalid checksums and the like. Why are these tests after a point of no return?
Quoted 5 lines
I would think that if any of these failures are hit, you would *not* want to reset the board so you can figure out what was going on.
Cheers,
g.
Quoted 8 lines
We've smashed our underpinnings by uncompressing linux over our interrupt vectors before the checksum catches the problem. There is no way back other than a reset. The error I fixed (in a couple of places) was returns. FWIIW, my experience with the erroneous returns is that it sort of works, but not right and then funky stuff happens. :-/
I presume the comment was a general one. If you have any specific reset vs. return questions, hollar and I'll justify them or eat crow.
Ah, okay that makes sense. So the obvious next question is: can any of those tests be performed before uncompressing Linux? If so, then they probably should be moved. If not...
Quoted 4 lines
Thanks,
gvb
Signature · 4 lines
Grant Likely, B.Sc., P.Eng. Secret Lab Technologies Ltd. grant.likely at secretlab.ca (403) 399-0195
PATCH 6/17 Reorganize and fix problems (returns) in the bootm command.
Quoted 22 lines · Grant Likely
Quoted 21 lines · Jerry Van Baren
Quoted 9 lines · Grant Likely
Quoted 4 lines · Jerry Van Baren
Do *NOT* return after the "point of no return" has been passed.
If something goes wrong, the board must be reset after that point.
Move the "Transferring control to Linux" debug message back to where it
belongs: just before transferring control to linux.Why is it necessary to reset the board at this point? The failure paths here seem to be things like invalid checksums and the like. Why are these tests after a point of no return?
Quoted 5 lines
I would think that if any of these failures are hit, you would *not* want to reset the board so you can figure out what was going on.
Cheers,
g.We've smashed our underpinnings by uncompressing linux over our interrupt vectors before the checksum catches the problem. There is no way back other than a reset. The error I fixed (in a couple of places) was returns. FWIIW, my experience with the erroneous returns is that it sort of works, but not right and then funky stuff happens. :-/
Quoted 8 lines
Quoted 2 lines
I presume the comment was a general one. If you have any specific reset vs. return questions, hollar and I'll justify them or eat crow.
Ah, okay that makes sense. So the obvious next question is: can any of those tests be performed before uncompressing Linux? If so, then they probably should be moved. If not...
Acked-by Grant Likely <[email protected]>
Sorry, no. The CRC is over the uncompressed data, and for good reason - it will detect if Something Bad[tm] happened during the decompression. The good news is, the reset-to-recover path /should/ never be taken.
gvb
PATCH 6/17 Reorganize and fix problems (returns) in the bootm command.
Quoted 2 lines · In message <[email protected]> you wrote:
Sorry, no. The CRC is over the uncompressed data, and for good reason -
Umm... no, it is over the data blob, which usually is compressed.
Best regards,
Wolfgang Denk
Signature · 6 lines
DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel 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 Every program has at least one bug and can be shortened by at least one instruction - from which, by induction, one can deduce that every program can be reduced to one instruction which doesn't work.
PATCH 6/17 Reorganize and fix problems (returns) in the bootm command.
In reply to Jerry Van BarenQuoted 3 lines · Jerry Van Baren
We've smashed our underpinnings by uncompressing linux over our interrupt vectors before the checksum catches the problem. There is no way back other than a reset.
Can you include this explanation as a comment in the code at the exact point this occurs?
Signature · 2 lines
Timur Tabi Linux Kernel Developer @ Freescale
PATCH 6/17 Reorganize and fix problems (returns) in the bootm command.
Quoted 7 lines · Timur Tabi
Quoted 3 lines · Jerry Van Baren
We've smashed our underpinnings by uncompressing linux over our interrupt vectors before the checksum catches the problem. There is no way back other than a reset.
Can you include this explanation as a comment in the code at the exact point this occurs?
It is already in the original code.
<http://www.denx.de/cgi-bin/gitweb.cgi?p=u-boot.git;a=blob;f=common/cmd_bootm.c;h=a6499e8dd9bc3fc853606567c847d0a064f02e55;hb=HEAD#l306>
gvb
PATCH 6/17 Reorganize and fix problems (returns) in the bootm command.
In reply to Timur TabiQuoted 6 lines · In message <[email protected]> you wrote:
Quoted 3 lines · Jerry Van Baren
We've smashed our underpinnings by uncompressing linux over our interrupt vectors before the checksum catches the problem. There is no way back other than a reset.
Can you include this explanation as a comment in the code at the exact point this occurs?
Ummm... what exactly is not clear enough with this comment in "common/cmd_bootm.c":
/*
* We have reached the point of no return: we are going to
* overwrite all exception vector code, so we cannot easily
* recover from any failures any more...
*/
??? Do you need additional explanations? What should be added?
Best regards,
Wolfgang Denk
Signature · 7 lines
DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel 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 "Logic and practical information do not seem to apply here." "You admit that?" "To deny the facts would be illogical, Doctor" -- Spock and McCoy, "A Piece of the Action", stardate unknown
PATCH 6/17 Reorganize and fix problems (returns) in the bootm command.
Quoted 17 lines · Wolfgang Denk
Quoted 6 lines · In message <[email protected]> you wrote:
Quoted 3 lines · Jerry Van Baren
We've smashed our underpinnings by uncompressing linux over our interrupt vectors before the checksum catches the problem. There is no way back other than a reset.
Can you include this explanation as a comment in the code at the exact point this occurs?
Ummm... what exactly is not clear enough with this comment in "
common/cmd_bootm.c":/*
* We have reached the point of no return: we are going to
* overwrite all exception vector code, so we cannot easily
* recover from any failures any more...
*/
Quoted 5 lines
??? Do you need additional explanations? What should be added?
Best regards,
Wolfgang Denk
Maybe if we enhanced the comment so it catches the eye of your average geek. Perhaps one of these:
<http://www.chris.com/ASCII/art/html/nakedladies.html>
Of course, we would have to do #ifdef MALE around it...
;-)
gvb (who missed the comment himself, the first time around)
PATCH 6/17 Reorganize and fix problems (returns) in the bootm command.
In reply to Wolfgang DenkQuoted 7 lines · Wolfgang Denk
/*
* We have reached the point of no return: we are going to
* overwrite all exception vector code, so we cannot easily
* recover from any failures any more...
*/??? Do you need additional explanations? What should be added?
I read only the patch, not the code in cmd_bootm.c. And since someone else asked why the patch was needed, I assumed that there was no comment or any other documentation explaining the need for the patch.