[U-Boot] [PATCH v5 03/24] arm: imx: hab: Optimise flow of authenticate_image on is_enabled fail

[Breno Matheus Lima]

Hi Bryan,

2018-01-08 15:36 GMT-02:00 Bryan O'Donoghue <bryan.odonoghue at linaro.org>:
> There is no need to call is_enabled() twice in authenticate_image - it does
> nothing but add an additional layer of indentation.
>
> We can check for is_enabled() at the start of the function and return the
> result code directly.
>
> Signed-off-by: Bryan O'Donoghue <bryan.odonoghue at linaro.org>
> Cc: Stefano Babic <sbabic at denx.de>
> Cc: Fabio Estevam <fabio.estevam at nxp.com>
> Cc: Peng Fan <peng.fan at nxp.com>
> Cc: Albert Aribaud <albert.u.boot at aribaud.net>
> Cc: Sven Ebenfeld <sven.ebenfeld at gmail.com>
> Cc: George McCollister <george.mccollister at gmail.com>
> Cc: Breno Matheus Lima <brenomatheus at gmail.com>
> ---
>  arch/arm/mach-imx/hab.c | 138 ++++++++++++++++++++++++------------------------
>  1 file changed, 69 insertions(+), 69 deletions(-)
>
> diff --git a/arch/arm/mach-imx/hab.c b/arch/arm/mach-imx/hab.c
> index 9fe6d43..6f86c02 100644
> --- a/arch/arm/mach-imx/hab.c
> +++ b/arch/arm/mach-imx/hab.c
> @@ -428,91 +428,91 @@ int authenticate_image(uint32_t ddr_start, uint32_t image_size)
>         hab_rvt_entry = hab_rvt_entry_p;
>         hab_rvt_exit = hab_rvt_exit_p;
>
> -       if (is_hab_enabled()) {
> -               printf("\nAuthenticate image from DDR location 0x%x...\n",
> -                      ddr_start);
> +       if (!is_hab_enabled()) {
> +               puts("hab fuse not enabled\n");
> +               return result;
> +       }

Just another comment in your series, I was working on an open i.MX6UL
EVK and noticed that we may have problem with SPL boards in open mode:

U-Boot SPL 2018.01-rc3-39088-g59b5c4e (Jan 11 2018 - 16:32:52)
Trying to boot from MMC1
hab fuse not enabled
spl: ERROR:  image authentication unsuccessful
### ERROR ### Please RESET the board ###

The imx_hab_authenticate_image() is returning an authentication
failure in this case and we cannot boot u-boot-ivt.img. In this way is
not possible to run hab_status before closing the device.

I think the following fix this issue:

--- a/arch/arm/mach-imx/hab.c
+++ b/arch/arm/mach-imx/hab.c
@@ -492,7 +492,7 @@ int imx_hab_authenticate_image(uint32_t ddr_start,
uint32_t image_size,

        if (!imx_hab_is_enabled()) {
                puts("hab fuse not enabled\n");
-               return result;
+               goto hab_exit;
        }

        printf("\nAuthenticate image from DDR location 0x%x...\n",
@@ -594,7 +594,8 @@ hab_exit_failure_print_status:
 hab_caam_clock_disable:
        hab_caam_clock_enable(0);

-       if (load_addr != 0)
+hab_exit:
+       if (load_addr != 0 || (!imx_hab_is_enabled()))
                result = 0;

Can you please check if is possible to add this on your series? If is
too late I can submit a patch.

Thanks,
Breno Lima