[PATCH] ARM: imx: hab: panic on authentication failure

[Marek Vasut]

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 at 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 at denx.de>
>>>>> Cc: Fabio Estevam <festevam at gmail.com>
>>>>> Cc: NXP i.MX U-Boot Team <uboot-imx at nxp.com>
>>>>> Cc: Peng Fan <peng.fan at nxp.com>
>>>>> Cc: Stefano Babic <sbabic at denx.de>
>>>>
>>>> This is a better behavior indeed:
>>>>
>>>> Reviewed-by: Fabio Estevam <festevam at 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 ?