* [PATCH] ARM: imx: hab: panic on authentication failure @ 2020-05-30 18:29 Marek Vasut 2020-05-30 18:31 ` Fabio Estevam 2020-08-04 8:52 ` sbabic at denx.de 0 siblings, 2 replies; 10+ messages in thread From: Marek Vasut @ 2020-05-30 18:29 UTC (permalink / raw) To: u-boot 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 <marex@denx.de> Cc: Fabio Estevam <festevam@gmail.com> Cc: NXP i.MX U-Boot Team <uboot-imx@nxp.com> Cc: Peng Fan <peng.fan@nxp.com> Cc: Stefano Babic <sbabic@denx.de> --- arch/arm/mach-imx/spl.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/arch/arm/mach-imx/spl.c b/arch/arm/mach-imx/spl.c index 1a231c67f5..76a5f7aca6 100644 --- a/arch/arm/mach-imx/spl.c +++ b/arch/arm/mach-imx/spl.c @@ -293,8 +293,7 @@ __weak void __noreturn jump_to_image_no_args(struct spl_image_info *spl_image) CSF_PAD_SIZE, offset)) { image_entry(); } else { - puts("spl: ERROR: image authentication fail\n"); - hang(); + panic("spl: ERROR: image authentication fail\n"); } } } @@ -320,8 +319,7 @@ void board_spl_fit_post_load(ulong load_addr, size_t length) if (imx_hab_authenticate_image(load_addr, offset + IVT_SIZE + CSF_PAD_SIZE, offset)) { - puts("spl: ERROR: image authentication unsuccessful\n"); - hang(); + panic("spl: ERROR: image authentication unsuccessful\n"); } } #endif -- 2.25.1 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH] ARM: imx: hab: panic on authentication failure 2020-05-30 18:29 [PATCH] ARM: imx: hab: panic on authentication failure Marek Vasut @ 2020-05-30 18:31 ` Fabio Estevam 2020-05-30 20:14 ` Patrick Wildt 2020-08-04 8:52 ` sbabic at denx.de 1 sibling, 1 reply; 10+ messages in thread From: Fabio Estevam @ 2020-05-30 18:31 UTC (permalink / raw) To: u-boot Hi Marek, [Adding Breno] On Sat, May 30, 2020 at 3:29 PM Marek Vasut <marex@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 <marex@denx.de> > Cc: Fabio Estevam <festevam@gmail.com> > Cc: NXP i.MX U-Boot Team <uboot-imx@nxp.com> > Cc: Peng Fan <peng.fan@nxp.com> > Cc: Stefano Babic <sbabic@denx.de> This is a better behavior indeed: Reviewed-by: Fabio Estevam <festevam@gmail.com> ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH] ARM: imx: hab: panic on authentication failure 2020-05-30 18:31 ` Fabio Estevam @ 2020-05-30 20:14 ` Patrick Wildt 2020-05-30 20:29 ` Marek Vasut 0 siblings, 1 reply; 10+ messages in thread From: Patrick Wildt @ 2020-05-30 20:14 UTC (permalink / raw) To: u-boot 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 <marex@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 <marex@denx.de> > > Cc: Fabio Estevam <festevam@gmail.com> > > Cc: NXP i.MX U-Boot Team <uboot-imx@nxp.com> > > Cc: Peng Fan <peng.fan@nxp.com> > > Cc: Stefano Babic <sbabic@denx.de> > > This is a better behavior indeed: > > Reviewed-by: Fabio Estevam <festevam@gmail.com> What about this? Have you ignored this patch for a reason? :/ https://marc.info/?l=u-boot&m=159069441005730&w=2 ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH] ARM: imx: hab: panic on authentication failure 2020-05-30 20:14 ` Patrick Wildt @ 2020-05-30 20:29 ` Marek Vasut 2020-05-30 20:53 ` Patrick Wildt 0 siblings, 1 reply; 10+ messages in thread From: Marek Vasut @ 2020-05-30 20:29 UTC (permalink / raw) To: u-boot 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 <marex@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 <marex@denx.de> >>> Cc: Fabio Estevam <festevam@gmail.com> >>> Cc: NXP i.MX U-Boot Team <uboot-imx@nxp.com> >>> Cc: Peng Fan <peng.fan@nxp.com> >>> Cc: Stefano Babic <sbabic@denx.de> >> >> This is a better behavior indeed: >> >> Reviewed-by: Fabio Estevam <festevam@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. 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 ? ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH] ARM: imx: hab: panic on authentication failure 2020-05-30 20:29 ` Marek Vasut @ 2020-05-30 20:53 ` Patrick Wildt 2020-05-31 15:38 ` Marek Vasut 0 siblings, 1 reply; 10+ messages in thread From: Patrick Wildt @ 2020-05-30 20:53 UTC (permalink / raw) To: u-boot 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 <marex@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 <marex@denx.de> > >>> Cc: Fabio Estevam <festevam@gmail.com> > >>> Cc: NXP i.MX U-Boot Team <uboot-imx@nxp.com> > >>> Cc: Peng Fan <peng.fan@nxp.com> > >>> Cc: Stefano Babic <sbabic@denx.de> > >> > >> This is a better behavior indeed: > >> > >> Reviewed-by: Fabio Estevam <festevam@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. Which errno would you like to see? Best regards, Patrick ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH] ARM: imx: hab: panic on authentication failure 2020-05-30 20:53 ` Patrick Wildt @ 2020-05-31 15:38 ` Marek Vasut 2020-05-31 15:53 ` Patrick Wildt 0 siblings, 1 reply; 10+ messages in thread From: Marek Vasut @ 2020-05-31 15:38 UTC (permalink / raw) To: u-boot 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 <marex@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 <marex@denx.de> >>>>> Cc: Fabio Estevam <festevam@gmail.com> >>>>> Cc: NXP i.MX U-Boot Team <uboot-imx@nxp.com> >>>>> Cc: Peng Fan <peng.fan@nxp.com> >>>>> Cc: Stefano Babic <sbabic@denx.de> >>>> >>>> This is a better behavior indeed: >>>> >>>> Reviewed-by: Fabio Estevam <festevam@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 ? ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH] ARM: imx: hab: panic on authentication failure 2020-05-31 15:38 ` Marek Vasut @ 2020-05-31 15:53 ` Patrick Wildt 2020-05-31 16:51 ` Marek Vasut 0 siblings, 1 reply; 10+ messages in thread From: Patrick Wildt @ 2020-05-31 15:53 UTC (permalink / raw) To: u-boot 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 <marex@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 <marex@denx.de> > >>>>> Cc: Fabio Estevam <festevam@gmail.com> > >>>>> Cc: NXP i.MX U-Boot Team <uboot-imx@nxp.com> > >>>>> Cc: Peng Fan <peng.fan@nxp.com> > >>>>> Cc: Stefano Babic <sbabic@denx.de> > >>>> > >>>> This is a better behavior indeed: > >>>> > >>>> Reviewed-by: Fabio Estevam <festevam@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. 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. Patrick ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH] ARM: imx: hab: panic on authentication failure 2020-05-31 15:53 ` Patrick Wildt @ 2020-05-31 16:51 ` Marek Vasut 2020-05-31 17:02 ` Patrick Wildt 0 siblings, 1 reply; 10+ messages in thread From: Marek Vasut @ 2020-05-31 16:51 UTC (permalink / raw) To: u-boot 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 <marex@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 <marex@denx.de> >>>>>>> Cc: Fabio Estevam <festevam@gmail.com> >>>>>>> Cc: NXP i.MX U-Boot Team <uboot-imx@nxp.com> >>>>>>> Cc: Peng Fan <peng.fan@nxp.com> >>>>>>> Cc: Stefano Babic <sbabic@denx.de> >>>>>> >>>>>> This is a better behavior indeed: >>>>>> >>>>>> Reviewed-by: Fabio Estevam <festevam@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. > 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. ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH] ARM: imx: hab: panic on authentication failure 2020-05-31 16:51 ` Marek Vasut @ 2020-05-31 17:02 ` Patrick Wildt 0 siblings, 0 replies; 10+ messages in thread From: Patrick Wildt @ 2020-05-31 17:02 UTC (permalink / raw) To: u-boot 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 <marex@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 <marex@denx.de> > >>>>>>> Cc: Fabio Estevam <festevam@gmail.com> > >>>>>>> Cc: NXP i.MX U-Boot Team <uboot-imx@nxp.com> > >>>>>>> Cc: Peng Fan <peng.fan@nxp.com> > >>>>>>> Cc: Stefano Babic <sbabic@denx.de> > >>>>>> > >>>>>> This is a better behavior indeed: > >>>>>> > >>>>>> Reviewed-by: Fabio Estevam <festevam@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! ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH] ARM: imx: hab: panic on authentication failure 2020-05-30 18:29 [PATCH] ARM: imx: hab: panic on authentication failure Marek Vasut 2020-05-30 18:31 ` Fabio Estevam @ 2020-08-04 8:52 ` sbabic at denx.de 1 sibling, 0 replies; 10+ messages in thread From: sbabic at denx.de @ 2020-08-04 8:52 UTC (permalink / raw) To: u-boot > 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 <marex@denx.de> > Cc: Fabio Estevam <festevam@gmail.com> > Cc: NXP i.MX U-Boot Team <uboot-imx@nxp.com> > Cc: Peng Fan <peng.fan@nxp.com> > Cc: Stefano Babic <sbabic@denx.de> > Reviewed-by: Fabio Estevam <festevam@gmail.com> Applied to u-boot-imx, master, thanks ! Best regards, Stefano Babic -- ===================================================================== DENX Software Engineering GmbH, Managing Director: Wolfgang Denk HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: +49-8142-66989-53 Fax: +49-8142-66989-80 Email: sbabic at denx.de ===================================================================== ^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2020-08-04 8:52 UTC | newest] Thread overview: 10+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2020-05-30 18:29 [PATCH] ARM: imx: hab: panic on authentication failure Marek Vasut 2020-05-30 18:31 ` Fabio Estevam 2020-05-30 20:14 ` Patrick Wildt 2020-05-30 20:29 ` Marek Vasut 2020-05-30 20:53 ` Patrick Wildt 2020-05-31 15:38 ` Marek Vasut 2020-05-31 15:53 ` Patrick Wildt 2020-05-31 16:51 ` Marek Vasut 2020-05-31 17:02 ` Patrick Wildt 2020-08-04 8:52 ` sbabic at denx.de
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox