On Fri, Jun 12, 2020 at 12:02:24AM +0300, Vladimir Oltean wrote: > On Thu, 11 Jun 2020 at 23:30, Tom Rini <[email protected]> wrote: > > > > 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 <[email protected]> wrote: > > > > > > > > There are a few remaining places where we say CONFIG_SECURE_BOOT rather > > > > than CONFIG_IMX HAB. Update these instances. > > > > > > > > Cc: Stefano Babic <[email protected]> > > > > Cc: Fabio Estevam <[email protected]> > > > > Cc: NXP i.MX U-Boot Team <[email protected]> > > > > Cc: Eddy Petrișor <[email protected]> > > > > Cc: Shawn Guo <[email protected]> > > > > Cc: Vladimir Oltean <[email protected]> > > > > Cc: Priyanka Jain <[email protected]> > > > > Fixes: d714a75fd4dc ("imx: replace CONFIG_SECURE_BOOT with > > > > CONFIG_IMX_HAB") > > > > Signed-off-by: Tom Rini <[email protected]> > > > > --- > > > > 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. > > > > Actually looks like the ls1021atsn_sdcard_SECURE_BOOT_defconfig that > defined it (via CONFIG_SYS_EXTRA_OPTIONS) hasn't made it upstream yet, > so it's a bit unfair to say it, but things under CONFIG_SECURE_BOOT > are not dead code as you want to imply.
Ah, sorry, I was trying to fix inadvertently broken code. > > > 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 <[email protected]> > > > 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 <[email protected]> > > > > 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. > > > > Ok, agree that it's a bad name for something that lives under > board/freescale/. > > > > 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! > > > > Yes, yes, for ls1021atsn.h and for the top-level Makefile, > CONFIG_NXP_ESBC is exactly what is needed! For the top-level Makefile > in particular, I believe it was missed during this conversion: > > commit 5536c3c9d0d10c1a4e440e71eac389df3a3dbfa7 > Author: Udit Agarwal <[email protected]> > Date: Thu Nov 7 16:11:32 2019 +0000 > > freescale/layerscape: Rename the config CONFIG_SECURE_BOOT name > > Rename CONFIG_SECURE_BOOT to CONFIG_NXP_ESBC to avoid conflict > with UEFI secure boot. > > Signed-off-by: Udit Agarwal <[email protected]> > Signed-off-by: Priyanka Jain <[email protected]> > > I'm not sure how I missed it during my previous reply. Ah-ha! I'll catch this in v2, thanks! -- Tom
signature.asc
Description: PGP signature
