From patchwork Sat Jun 27 19:25:28 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Michael Walle X-Patchwork-Id: 243026 List-Id: U-Boot discussion From: michael at walle.cc (Michael Walle) Date: Sat, 27 Jun 2020 21:25:28 +0200 Subject: [PATCH v3 0/6] crypto/fsl: add RNG support In-Reply-To: <98d5562d-e26f-3b52-8b0e-a7a99ce80a5f@gmx.de> References: <20200625121905.4475-1-michael@walle.cc> <98665946-466d-a7ab-8818-73b3889f0c18@gmx.de> <4a387c1b-4818-8293-738e-4017e21c0e07@gmx.de> <98d5562d-e26f-3b52-8b0e-a7a99ce80a5f@gmx.de> Message-ID: <7ef2d07567133c776222ae1d5678a192@walle.cc> Am 2020-06-26 18:26, schrieb Heinrich Schuchardt: > On 6/25/20 11:01 PM, Michael Walle wrote: >> Am 2020-06-25 18:03, schrieb Heinrich Schuchardt: >>> On 25.06.20 16:36, Heinrich Schuchardt wrote: >>>> On 25.06.20 14:18, Michael Walle wrote: >>>>> First, improve the compatibility on newer Era CAAMs. These >>>>> introduced new >>>>> version registers. Secondly, add RNG support for the CAAM. This way >>>>> we get >>>>> random number generator support for EFI for free and KASLR will >>>>> work >>>>> with >>>>> ARM64 kernels booted with bootefi. >>>>> >>>> >>>> It seems that a Kconfig dependency at least on >>>> CONFIG_SYS_FSL_HAS_SEC >>>> which itself depends on CONFIG_IMX_HAB is missing: >>>> >>>> wandboard_defconfig + FSL_CAAM + DM_RNG gives me a bunch of errors >>>> >>>> drivers/crypto/fsl/jr.c: In function ?start_jr0?: >>>> drivers/crypto/fsl/jr.c:47:2: error: unknown type name ?ccsr_sec_t?; >>>> did >>>> you mean ?pci_dev_t?? >>>> ? ccsr_sec_t *sec = (void *)SEC_ADDR(sec_idx); >>>> ? ^~~~~~~~~~ >>>> ? pci_dev_t >>>> In file included from ./arch/arm/include/asm/byteorder.h:29, >>>> ???????????????? from include/linux/libfdt_env.h:15, >>>> ???????????????? from include/linux/libfdt.h:6, >>>> ???????????????? from include/fdtdec.h:17, >>>> ???????????????? from include/asm-generic/global_data.h:23, >>>> ???????????????? from ./arch/arm/include/asm/global_data.h:87, >>>> ???????????????? from include/common.h:26, >>>> ???????????????? from drivers/crypto/fsl/jr.c:8: >>>> drivers/crypto/fsl/jr.c:48:29: error: request for member ?ctpr_ms? >>>> in >>>> something not a structure or union >>>> ? u32 ctpr_ms = sec_in32(&sec->ctpr_ms); >>>> ???????????????????????????? ^~ >>>> >>>> But if I enable IMX_HAB booting fails with: "hab fuse not enabled". >>>> >>>> Why should I need to enable the HAB fuse to use the random number >>>> generator on the i.MX6? >>>> >>> >>> With this change I can build the RNG driver for the i.MX6 Wandboard: >>> >>> diff --git a/drivers/crypto/fsl/Kconfig b/drivers/crypto/fsl/Kconfig >>> index 5ed6140da3..84783ea987 100644 >>> --- a/drivers/crypto/fsl/Kconfig >>> +++ b/drivers/crypto/fsl/Kconfig >>> @@ -37,7 +37,6 @@ config SYS_FSL_SEC_BE >>> >>> ?config SYS_FSL_SEC_COMPAT >>> ??????? int "Freescale Secure Boot compatibility" >>> -?????? depends on SYS_FSL_HAS_SEC >>> ??????? default 2 if SYS_FSL_SEC_COMPAT_2 >>> ??????? default 4 if SYS_FSL_SEC_COMPAT_4 >>> ??????? default 5 if SYS_FSL_SEC_COMPAT_5 >>> >>> Even if you do not plan to support the i.MX6, I would recommend this >>> change to separate HAB and RNG. >> >> I don't think this is the correct place. Rather the architecture >> should >> set SYS_FSL_HAS_SEC if it actually has the SEC. I mean it already sets >> the COMPAT level but not the actual config which indicates it has one. >> At the moment it depends on IMX_HAB; I don't know the reasoning behind >> this. But I don't see how this would be part of this series. > > ARCH_MX7 (arch/arm/Kconfig) has: > select SYS_FSL_HAS_SEC if IMX_HAB > > So according to your suggestion this should be changed to > > select SYS_FSL_HAS_SEC ? > > And the same added to ARCH_MX6? > Also I don't know why the SYS_FSL_SEC_COMPAT is user selectable: -michael --- a/drivers/crypto/fsl/Kconfig +++ b/drivers/crypto/fsl/Kconfig @@ -36,7 +36,7 @@ config SYS_FSL_SEC_BE bool "Big-endian access to Freescale Secure Boot" config SYS_FSL_SEC_COMPAT - int "Freescale Secure Boot compatibility" + int depends on SYS_FSL_HAS_SEC default 2 if SYS_FSL_SEC_COMPAT_2 default 4 if SYS_FSL_SEC_COMPAT_4