[PATCH 25/30] arm: imx: Finish migration from CONFIG_SECURE_BOOT to CONFIG_IMX_HAB

[Vladimir Oltean]

Hi Tom,

On Wed, 10 Jun 2020 at 23:17, Tom Rini <trini at konsulko.com> wrote:
>
> There are a few remaining places where we say CONFIG_SECURE_BOOT rather
> than CONFIG_IMX HAB.  Update these instances.
>
> Cc: Stefano Babic <sbabic at denx.de>
> Cc: Fabio Estevam <festevam at gmail.com>
> Cc: NXP i.MX U-Boot Team <uboot-imx at nxp.com>
> Cc: Eddy Petrișor <eddy.petrisor at gmail.com>
> Cc: Shawn Guo <shawnguo at kernel.org>
> Cc: Vladimir Oltean <olteanv at gmail.com>
> Cc: Priyanka Jain <priyanka.jain at nxp.com>
> Fixes: d714a75fd4dc ("imx: replace CONFIG_SECURE_BOOT with CONFIG_IMX_HAB")
> Signed-off-by: Tom Rini <trini at konsulko.com>
> ---
> Note that we have one place left for CONFIG_SECURE_BOOT being in use but
> I think that is shared with PowerPC so I don't think IMX_HAB is the
> right name.  But perhaps I'm wrong about it being used for PowerPC?

NACK on this patch.
I'm not actually sure what were the cross-architecture problems with
the CONFIG_SECURE_BOOT name that mandated Stefano to write this patch:

commit d714a75fd4dcfb0eb8b7e1dd29f43e07113cec0b
Author: Stefano Babic <sbabic at denx.de>
Date:   Fri Sep 20 08:47:53 2019 +0200

    imx: replace CONFIG_SECURE_BOOT with CONFIG_IMX_HAB

    CONFIG_SECURE_BOOT is too generic and forbids to use it for cross
    architecture purposes. If Secure Boot is required for imx, this means to
    enable and use the HAB processor in the soc.

    Signed-off-by: Stefano Babic <sbabic at denx.de>

but going the full way and grouping Layerscape, QorIQ and S32V secure
boot implementations together with a boot ROM feature available only
on i.MX 50, 53, 6, 7, 8M and 8MM is demonstrably incorrect.

I think the correct solution (beside leaving the CONFIG_SECURE_BOOT
name alone) would be to merge it, for the Layerscape (ls*) and PowerPC
instances, with CONFIG_CHAIN_OF_TRUST (defined under
board/freescale/common/Kconfig). But you or Stefano might argue that
CHAIN_OF_TRUST is still too generic for a name, and in that case,
maybe the whole thing can be renamed to CONFIG_FSL_ESBC (ESBC ==
"External Secure Boot Code", aka image validation code executed by the
bootloader as opposed to the [internal] boot ROM).

By the way, I have no idea what is the correct solution for S32V.

> ---
>  arch/arm/mach-imx/spl_qspi.cfg            | 2 +-
>  board/ea/mx7ulp_com/imximage.cfg          | 2 +-
>  board/freescale/s32v234evb/s32v234evb.cfg | 2 +-
>  board/novtech/meerkat96/imximage.cfg      | 2 +-
>  include/configs/ls1021atsn.h              | 4 ++--
>  5 files changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/arch/arm/mach-imx/spl_qspi.cfg b/arch/arm/mach-imx/spl_qspi.cfg
> index 88956e626ffd..1e39ae2f01f7 100644
> --- a/arch/arm/mach-imx/spl_qspi.cfg
> +++ b/arch/arm/mach-imx/spl_qspi.cfg
> @@ -12,6 +12,6 @@ BOOT_FROM     qspi
>  /*
>   * Secure boot support
>   */
> -#ifdef CONFIG_SECURE_BOOT
> +#ifdef CONFIG_IMX_HAB
>  CSF CONFIG_CSF_SIZE
>  #endif
> diff --git a/board/ea/mx7ulp_com/imximage.cfg b/board/ea/mx7ulp_com/imximage.cfg
> index d298d17c1e92..1b218996aea9 100644
> --- a/board/ea/mx7ulp_com/imximage.cfg
> +++ b/board/ea/mx7ulp_com/imximage.cfg
> @@ -28,7 +28,7 @@ BOOT_FROM     sd
>  PLUGIN board/freescale/mx7ulp_evk/plugin.bin 0x2F020000
>  #else
>
> -#ifdef CONFIG_SECURE_BOOT
> +#ifdef CONFIG_IMX_HAB
>  CSF CONFIG_CSF_SIZE
>  #endif
>  /*
> diff --git a/board/freescale/s32v234evb/s32v234evb.cfg b/board/freescale/s32v234evb/s32v234evb.cfg
> index 7881512139d0..d7f722006312 100644
> --- a/board/freescale/s32v234evb/s32v234evb.cfg
> +++ b/board/freescale/s32v234evb/s32v234evb.cfg
> @@ -23,6 +23,6 @@ BOOT_FROM sd
>   */
>
>
> -#ifdef CONFIG_SECURE_BOOT
> +#ifdef CONFIG_IMX_HAB
>  SECURE_BOOT
>  #endif
> diff --git a/board/novtech/meerkat96/imximage.cfg b/board/novtech/meerkat96/imximage.cfg
> index 3bd8cc55e53c..86275b84d9c8 100644
> --- a/board/novtech/meerkat96/imximage.cfg
> +++ b/board/novtech/meerkat96/imximage.cfg
> @@ -25,7 +25,7 @@ BOOT_FROM     sd
>  /*
>   * Secure boot support
>   */
> -#ifdef CONFIG_SECURE_BOOT
> +#ifdef CONFIG_IMX_HAB
>  CSF CONFIG_CSF_SIZE
>  #endif
>
> diff --git a/include/configs/ls1021atsn.h b/include/configs/ls1021atsn.h
> index e76e54e97fc9..efa708a239ea 100644
> --- a/include/configs/ls1021atsn.h
> +++ b/include/configs/ls1021atsn.h
> @@ -60,9 +60,9 @@
>  #define CONFIG_SYS_FSL_PBL_RCW \
>                 "board/freescale/ls1021atsn/ls102xa_rcw_sd.cfg"
>
> -#ifdef CONFIG_SECURE_BOOT
> +#ifdef CONFIG_IMX_HAB
>  #define CONFIG_U_BOOT_HDR_SIZE         (16 << 10)
> -#endif /* ifdef CONFIG_SECURE_BOOT */
> +#endif /* ifdef CONFIG_IMX_HAB */
>
>  #define CONFIG_SPL_MAX_SIZE            0x1a000
>  #define CONFIG_SPL_STACK               0x1001d000
> --
> 2.17.1
>

Thanks,
-Vladimir