On Sun, May 31, 2020 at 06:51:14PM +0200, Marek Vasut wrote: > On 5/31/20 5:53 PM, Patrick Wildt wrote: > > On Sun, May 31, 2020 at 05:38:05PM +0200, Marek Vasut wrote: > >> On 5/30/20 10:53 PM, Patrick Wildt wrote: > >>> On Sat, May 30, 2020 at 10:29:19PM +0200, Marek Vasut wrote: > >>>> On 5/30/20 10:14 PM, Patrick Wildt wrote: > >>>>> On Sat, May 30, 2020 at 03:31:29PM -0300, Fabio Estevam wrote: > >>>>>> Hi Marek, > >>>>>> > >>>>>> [Adding Breno] > >>>>>> > >>>>>> On Sat, May 30, 2020 at 3:29 PM Marek Vasut <ma...@denx.de> wrote: > >>>>>>> > >>>>>>> Instead of hang()ing the system and thus disallowing any automated > >>>>>>> recovery possibility from a HAB authentication failure, panic() . > >>>>>>> The panic() function can be configured to hang() the system after > >>>>>>> printing an error message, however the default is to reset the > >>>>>>> system instead. > >>>>>>> > >>>>>>> This allows redundant boot to work correctly. In case the primary > >>>>>>> or secondary image cannot be authenticated, the system reboots and > >>>>>>> bootrom can try to start the other one. > >>>>>>> > >>>>>>> Signed-off-by: Marek Vasut <ma...@denx.de> > >>>>>>> Cc: Fabio Estevam <feste...@gmail.com> > >>>>>>> Cc: NXP i.MX U-Boot Team <uboot-...@nxp.com> > >>>>>>> Cc: Peng Fan <peng....@nxp.com> > >>>>>>> Cc: Stefano Babic <sba...@denx.de> > >>>>>> > >>>>>> This is a better behavior indeed: > >>>>>> > >>>>>> Reviewed-by: Fabio Estevam <feste...@gmail.com> > >>>>> > >>>>> What about this? Have you ignored this patch for a reason? :/ > >>>>> > >>>>> https://marc.info/?l=u-boot&m=159069441005730&w=2 > >>>> > >>>> Yes, and the reason is I was not even aware of your patch, sorry. The CC > >>>> list in this mail should cover all the interested parties, so use it > >>>> when sending V2, or use patman. > >>> > >>> I already had 11 people on CC, but apparently I missed you. > >>> > >>>> The patch looks fine, one nit is that you should return errno.h return > >>>> value and another is that it changes the current behavior. Now that I > >>>> look at this imx code, board_spl_fit_post_load() should not even be in > >>>> arch/ , sigh, but that's for separate patch either way. > >>>> > >>>> So I think if you want to support this sort of fallback, you should make > >>>> the board_spl_fit_post_load() be in board/ files, with default __weak > >>>> implementation calling some arch_hab_authenticate...() which implements > >>>> current content of board_spl_fit_post_load(), and let boards decide how > >>>> to handle the fallback if it needs to be altered. > >>>> > >>>> Would that work ? > >>> > >>> I'm not sure. In comparison to the people from NXP who are paid to > >>> upstream their code and still don't do it correctly, I'm doing this > >>> in my spare time and I'm not sure I want to bikeshed all day long. > >>> > >>> I can send a V3 that replaces the -1 with EINVAL, EACCESS, EPERM or > >>> something the like. If you want to clean up after NXP, feel free to. > >> > >> In fact, what is it that you're trying to achieve with this fallback ? > >> What are you falling back to , another fallback fitImage ? > > > > Exactly. > > > > I have a device with a glued enclosure, with no external sources, apart > > from a serial console. If the SPL fails to load the U-Boot main image > > from eMMC, the only way to fix it is to destroy the case, open up the > > enclosure and short some lines. That's not really nice, since we'd have > > to get a new enclosure, new serial number label,... it's a hassle. > > Look for SRC PERSIST_SECONDARY_BOOT in your datasheet then. This will > let you use two copies of SPL, two copies of U-Boot, etc.
I'll have a look! > > If the SPL was gone as well, the BootROM would fall back to other > > sources. But with only one half of U-Boot missing, it would just hang. > > I'm sure that's also why you replace the hang() with a panic(). > > > > I thought that if the SPL is still fine, but only the U-Boot image was > > gone, why not make use of the spl_boot_list to try and boot from another > > source? Like yModem. For that I sent the following fix, also with many > > people CCd: > > > > https://marc.info/?l=u-boot&m=158893200030861&w=2 > > > > Now the board spl boot order can have eMMC as first, and yModem as > > second. If eMMC fails, it falls back to yModem. If none work, it > > though hang()s, doing > > > > puts(SPL_TPL_PROMPT "failed to boot from all boot devices\n"); > > hang(); > > > > Maybe you want this as panic instead? > > > > Anyway, I think this is nicer option for recovery, instead of simply > > hanging or rebooting. > > The problem is, this only works for fitImage and not for raw uImage. But > in your case, that is probably OK. > > So if you can just fix the errno return value to some -EINVAL (?) and > send a V3, I think that would be good. Ok, will do, thanks!
