Hi Tom,
On Wed, 10 Jun 2020 at 23:17, Tom Rini trini@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@denx.de Cc: Fabio Estevam festevam@gmail.com Cc: NXP i.MX U-Boot Team uboot-imx@nxp.com Cc: Eddy Petrișor eddy.petrisor@gmail.com Cc: Shawn Guo shawnguo@kernel.org Cc: Vladimir Oltean olteanv@gmail.com Cc: Priyanka Jain priyanka.jain@nxp.com Fixes: d714a75fd4dc ("imx: replace CONFIG_SECURE_BOOT with CONFIG_IMX_HAB") Signed-off-by: Tom Rini trini@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@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@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