| Message ID | 4a387c1b-4818-8293-738e-4017e21c0e07@gmx.de |
|---|---|
| State | New |
| Headers | show |
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. > With the following additional change the RNG driver is loaded on the > Wandboard: > > diff --git a/arch/arm/mach-imx/mx6/soc.c b/arch/arm/mach-imx/mx6/soc.c > index 19ca382649..e129286065 100644 > --- a/arch/arm/mach-imx/mx6/soc.c > +++ b/arch/arm/mach-imx/mx6/soc.c > @@ -22,6 +22,7 @@ > #include <asm/arch/mxc_hdmi.h> > #include <asm/arch/crm_regs.h> > #include <dm.h> > +#include <fsl_sec.h> > #include <imx_thermal.h> > #include <mmc.h> > > @@ -691,6 +692,15 @@ void imx_setup_hdmi(void) > } > #endif > > +#ifdef CONFIG_ARCH_MISC_INIT > +int arch_misc_init(void) > +{ > +#ifdef CONFIG_FSL_CAAM > + sec_init(); > +#endif > + return 0; > +} > +#endif > > /* > * gpr_init() function is common for boards using MX6S, MX6DL, MX6D, > > > But the RNG driver seems to require some changes to work on the i.MX6: > > => rng > CACHE: Misaligned operation at range [8e596f68, 8e596f78] > CACHE: Misaligned operation at range [8e596f68, 8e596f78] > ERROR: v7_outer_cache_inval_range - start address is not aligned - > 0x8e596f68 > ERROR: v7_outer_cache_inval_range - stop address is not aligned - > 0x8e596f78 > CACHE: Misaligned operation at range [8e596f68, 8e596f78] > CACHE: Misaligned operation at range [8e596f68, 8e596f78] > ERROR: v7_outer_cache_inval_range - start address is not aligned - > 0x8e596f68 > ERROR: v7_outer_cache_inval_range - stop address is not aligned - > 0x8e596f78 > CACHE: Misaligned operation at range [8e596f68, 8e596f78] > CACHE: Misaligned operation at range [8e596f68, 8e596f78] > ERROR: v7_outer_cache_inval_range - start address is not aligned - > 0x8e596f68 > ERROR: v7_outer_cache_inval_range - stop address is not aligned - > 0x8e596f78 > CACHE: Misaligned operation at range [8e596f68, 8e596f78] > CACHE: Misaligned operation at range [8e596f68, 8e596f78] > ERROR: v7_outer_cache_inval_range - start address is not aligned - > 0x8e596f68 > ERROR: v7_outer_cache_inval_range - stop address is not aligned - > 0x8e596f78 > 00000000: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 > ................ > 00000010: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 > ................ > 00000020: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 > ................ > 00000030: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 > ................ > => Could you please trace where this happens? The start address should be aligned, the end is likely not aligned. I presumed it is part of the dcache code to take care of the rounding. Of course I can do the rounding before the invalidation/flushing. It seems to be another discepancy between the architectures. Still doesn't explain why the start address is unaligned. -michael
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? Best regards Heinrich > >> With the following additional change the RNG driver is loaded on the >> Wandboard: >> >> diff --git a/arch/arm/mach-imx/mx6/soc.c b/arch/arm/mach-imx/mx6/soc.c >> index 19ca382649..e129286065 100644 >> --- a/arch/arm/mach-imx/mx6/soc.c >> +++ b/arch/arm/mach-imx/mx6/soc.c >> @@ -22,6 +22,7 @@ >> ?#include <asm/arch/mxc_hdmi.h> >> ?#include <asm/arch/crm_regs.h> >> ?#include <dm.h> >> +#include <fsl_sec.h> >> ?#include <imx_thermal.h> >> ?#include <mmc.h> >> >> @@ -691,6 +692,15 @@ void imx_setup_hdmi(void) >> ?} >> ?#endif >> >> +#ifdef CONFIG_ARCH_MISC_INIT >> +int arch_misc_init(void) >> +{ >> +#ifdef CONFIG_FSL_CAAM >> +?????? sec_init(); >> +#endif >> +?????? return 0; >> +} >> +#endif >> >> ?/* >> ? * gpr_init() function is common for boards using MX6S, MX6DL, MX6D, >> >> >> But the RNG driver seems to require some changes to work on the i.MX6: >> >> => rng >> CACHE: Misaligned operation at range [8e596f68, 8e596f78] >> CACHE: Misaligned operation at range [8e596f68, 8e596f78] >> ERROR: v7_outer_cache_inval_range - start address is not aligned - >> 0x8e596f68 >> ERROR: v7_outer_cache_inval_range - stop address is not aligned - >> 0x8e596f78 >> CACHE: Misaligned operation at range [8e596f68, 8e596f78] >> CACHE: Misaligned operation at range [8e596f68, 8e596f78] >> ERROR: v7_outer_cache_inval_range - start address is not aligned - >> 0x8e596f68 >> ERROR: v7_outer_cache_inval_range - stop address is not aligned - >> 0x8e596f78 >> CACHE: Misaligned operation at range [8e596f68, 8e596f78] >> CACHE: Misaligned operation at range [8e596f68, 8e596f78] >> ERROR: v7_outer_cache_inval_range - start address is not aligned - >> 0x8e596f68 >> ERROR: v7_outer_cache_inval_range - stop address is not aligned - >> 0x8e596f78 >> CACHE: Misaligned operation at range [8e596f68, 8e596f78] >> CACHE: Misaligned operation at range [8e596f68, 8e596f78] >> ERROR: v7_outer_cache_inval_range - start address is not aligned - >> 0x8e596f68 >> ERROR: v7_outer_cache_inval_range - stop address is not aligned - >> 0x8e596f78 >> 00000000: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00? >> ................ >> 00000010: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00? >> ................ >> 00000020: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00? >> ................ >> 00000030: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00? >> ................ >> => > > Could you please trace where this happens? The start address should > be aligned, the end is likely not aligned. I presumed it is part of > the dcache code to take care of the rounding. Of course I can > do the rounding before the invalidation/flushing. It seems to be > another discepancy between the architectures. Still doesn't explain > why the start address is unaligned. > > -michael
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? yes, because HAS_SEC is a hardware feature, why should that be dependant on a feature which is selected by the user? But I don't know if there are any side effects. Also I don't know if the SEC is available in all SoC of the imx6/7 series. -michael
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. With the following additional change the RNG driver is loaded on the Wandboard: diff --git a/arch/arm/mach-imx/mx6/soc.c b/arch/arm/mach-imx/mx6/soc.c index 19ca382649..e129286065 100644 --- a/arch/arm/mach-imx/mx6/soc.c +++ b/arch/arm/mach-imx/mx6/soc.c @@ -22,6 +22,7 @@ #include <asm/arch/mxc_hdmi.h> #include <asm/arch/crm_regs.h> #include <dm.h> +#include <fsl_sec.h> #include <imx_thermal.h> #include <mmc.h> @@ -691,6 +692,15 @@ void imx_setup_hdmi(void) } #endif +#ifdef CONFIG_ARCH_MISC_INIT +int arch_misc_init(void) +{ +#ifdef CONFIG_FSL_CAAM + sec_init(); +#endif + return 0; +} +#endif /* * gpr_init() function is common for boards using MX6S, MX6DL, MX6D,