| Message ID | 20180918081013.26660-1-alex.kiernan@gmail.com |
|---|---|
| State | Rejected |
| Delegated to: | Wolfgang Denk |
| Headers | show |
| Series | [U-Boot] bootm: fdt: Use panic() instead of hang() when presented with a bad image | expand |
On 09/18/2018 02:10 AM, Alex Kiernan wrote: > When the image which bootm is given can't be booted, call panic with > the error message rather than printf/hang so that we can recover from > broken images via a bootcount mechanism. If hang on failure is still > required then CONFIG_PANIC_HANG can still be enabled. This sounds plausible to me, but I think you'll want to send this patch To/Cc someone who owns that file. At least Tom Rini (now CC'd hence quoting the entire patch for context). > Signed-off-by: Alex Kiernan <alex.kiernan@gmail.com> > --- > > arch/arm/lib/bootm.c | 6 ++---- > 1 file changed, 2 insertions(+), 4 deletions(-) > > diff --git a/arch/arm/lib/bootm.c b/arch/arm/lib/bootm.c > index c3c1d2fdfa..a5ed12df74 100644 > --- a/arch/arm/lib/bootm.c > +++ b/arch/arm/lib/bootm.c > @@ -230,8 +230,7 @@ static void boot_prep_linux(bootm_headers_t *images) > #ifdef CONFIG_OF_LIBFDT > debug("using: FDT\n"); > if (image_setup_linux(images)) { > - printf("FDT creation failed! hanging..."); > - hang(); > + panic("FDT creation failed!\n"); > } > #endif > } else if (BOOTM_ENABLE_TAGS) { > @@ -264,8 +263,7 @@ static void boot_prep_linux(bootm_headers_t *images) > setup_board_tags(¶ms); > setup_end_tag(gd->bd); > } else { > - printf("FDT and ATAGS support not compiled in - hanging\n"); > - hang(); > + panic("FDT and ATAGS support not compiled in\n"); > } > }
Dear Alex, In message <20180918081013.26660-1-alex.kiernan@gmail.com> you wrote: > When the image which bootm is given can't be booted, call panic with > the error message rather than printf/hang so that we can recover from > broken images via a bootcount mechanism. If hang on failure is still > required then CONFIG_PANIC_HANG can still be enabled. I wonder if the failing of the FDT creation is a reason to panic / reboot at all? The point of no return is architecture dependent - for example, on Power architecture it is usuallywhen the kernel image gests decompressed/written to RAM, as then the exception vectors get overwritten, so no return to U-Boot is possible. But the FDT is always (I think ?) just written to an area of RAM that is not critical for the execution of U-Boot itself - so when this fails, why can we not simply return to U-Boot with an eroor indication? I feel we should use panic/hang only as a last resort, when no other recovery is possible? Best regards, Wolfgang Denk
On Fri, Sep 21, 2018 at 1:43 PM Wolfgang Denk <wd@denx.de> wrote: > > Dear Alex, > > In message <20180918081013.26660-1-alex.kiernan@gmail.com> you wrote: > > When the image which bootm is given can't be booted, call panic with > > the error message rather than printf/hang so that we can recover from > > broken images via a bootcount mechanism. If hang on failure is still > > required then CONFIG_PANIC_HANG can still be enabled. > > I wonder if the failing of the FDT creation is a reason to panic / > reboot at all? The point of no return is architecture dependent - > for example, on Power architecture it is usuallywhen the kernel > image gests decompressed/written to RAM, as then the exception > vectors get overwritten, so no return to U-Boot is possible. > > But the FDT is always (I think ?) just written to an area of RAM > that is not critical for the execution of U-Boot itself - so when > this fails, why can we not simply return to U-Boot with an eroor > indication? > > I feel we should use panic/hang only as a last resort, when no other > recovery is possible? > Thinking it through, that sounds reasonable to me... the reason I ended up looking at this code was because I deployed a broken FIT image, then got stuck here. But if bootm had returned with a failure I could have done useful recovery actions without needing a reboot.
diff --git a/arch/arm/lib/bootm.c b/arch/arm/lib/bootm.c index c3c1d2fdfa..a5ed12df74 100644 --- a/arch/arm/lib/bootm.c +++ b/arch/arm/lib/bootm.c @@ -230,8 +230,7 @@ static void boot_prep_linux(bootm_headers_t *images) #ifdef CONFIG_OF_LIBFDT debug("using: FDT\n"); if (image_setup_linux(images)) { - printf("FDT creation failed! hanging..."); - hang(); + panic("FDT creation failed!\n"); } #endif } else if (BOOTM_ENABLE_TAGS) { @@ -264,8 +263,7 @@ static void boot_prep_linux(bootm_headers_t *images) setup_board_tags(¶ms); setup_end_tag(gd->bd); } else { - printf("FDT and ATAGS support not compiled in - hanging\n"); - hang(); + panic("FDT and ATAGS support not compiled in\n"); } }
When the image which bootm is given can't be booted, call panic with the error message rather than printf/hang so that we can recover from broken images via a bootcount mechanism. If hang on failure is still required then CONFIG_PANIC_HANG can still be enabled. Signed-off-by: Alex Kiernan <alex.kiernan@gmail.com> --- arch/arm/lib/bootm.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-)