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

[Michael Walle]

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:

--- 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

-michael