On Thu, Jun 11, 2020 at 10:31:32PM +0300, Vladimir Oltean wrote:
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.
Note that today CONFIG_SECURE_BOOT is not defined anywhere and the commit you mention next replaced the only places that set CONFIG_SECURE_BOOT with CONFIG_IMX_HAB.
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>
The problem is that SECURE_BOOT is very generic. We have quite a few different "secure boot" implementations in the tree and another pointed out what a bad name this one is. And just to be clear, I'm the only one (intentionally) touching non-i.MX spots here.
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.
OK. I (and others on the thread at the time) were asking for someone to group things right and provide a new symbol. What's in there is what we got, but more details are always better as there were a few cases that didn't get updated.
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).
So for this patch here it's a few instances of CONFIG_CSF_SIZE on i.MX files, a change to S32V that looks quite a lot like i.MX (the file notes as much) and a layerscape change to CONFIG_U_BOOT_HDR_SIZE. I'm quite happy to spin v2 dropping the layerscape part out and waiting to see what Eddy says for S32V. We have a CONFIG_NXP_ESBC symbol today, would that make sense to use in the check on include/configs/ls1021atsn.h and top-level Makefile for not making u-boot.pbl sometimes? Thanks again!