[PATCH v3 0/6] crypto/fsl: add RNG support

[Michael Walle]

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