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

[Heinrich Schuchardt <xypron.glpk@gmx.de>]

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