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

[Bryan O'Donoghue]

On 11/01/18 19:34, Breno Matheus Lima wrote:
> 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.

So, at least on the MX7 - bootrom->authenticate_image() won't run unless 
the board is in locked mode, I suspect that's the same behavior on the MX6.

> 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;

Right, what you want is for uboot->authenticate_image() to return zero 
(OK) when the part is unlocked.

If this were the first time we were adding this code in, I'd say that 
would be a crazy thing to do because

1. bootrom->authenticate_image() will return an error
2. uboot->authenticate_image() will not even call 
bootrom->authenticate_image()

However since you already have a dependency on this behavior - I'll 
respin this code ....

---
bod