Re: [PATCH] crypto: caam - skip RNG selftest when i.MX6 is in normal world

[Ahmad Fatoum <a.fatoum@pengutronix.de>]

Hi,

On 10/29/25 10:55 AM, Marco Felsch wrote:
> On 25-10-29, Ahmad Fatoum wrote:
>> On an i.MX6 running under OP-TEE, the CAAM selftest fails as it needs to
>> run in the secure world[1].
>>
>> We can detect that barebox is running in the normal world on an i.MX6,
>> so use that info to gracefully skip the test.
>>
>> This is not enough to get the CAAM working (The DECO registers may not
>> be accessible to barebox), but that is arguably a more useful error
>> message than an -EBUSY.
>>
>> [1]: Scroll down to "Job Ring TZ assignment issue"
>> https://community.nxp.com/t5/i-MX-Security/RNG-self-test-errors-on-select-i-MX-device-revisions/ta-p/1102970
>>
>> Reported-by: Fabian Pflug <f.pflug@pengutronix.de>
>> Cc: Marco Felsch <m.felsch@pengutronix.de>
>> Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
>> ---
>>  arch/arm/mach-imx/imx6.c            |  2 +-
>>  drivers/crypto/caam/rng_self_test.c | 13 ++++++++++++-
>>  include/mach/imx/imx6.h             |  2 ++
>>  3 files changed, 15 insertions(+), 2 deletions(-)
>>
>> diff --git a/arch/arm/mach-imx/imx6.c b/arch/arm/mach-imx/imx6.c
>> index 0cf2c17d6b90..29a56de56293 100644
>> --- a/arch/arm/mach-imx/imx6.c
>> +++ b/arch/arm/mach-imx/imx6.c
>> @@ -254,7 +254,7 @@ int imx6_devices_init(void)
>>  	return 0;
>>  }
>>  
>> -static bool imx6_cannot_write_l2x0(void)
>> +bool imx6_cannot_write_l2x0(void)
>>  {
>>  	void __iomem *l2x0_base = IOMEM(0x00a02000);
>>  	u32 val;
>> diff --git a/drivers/crypto/caam/rng_self_test.c b/drivers/crypto/caam/rng_self_test.c
>> index 5216ecef4451..a42d9cad1639 100644
>> --- a/drivers/crypto/caam/rng_self_test.c
>> +++ b/drivers/crypto/caam/rng_self_test.c
>> @@ -47,6 +47,8 @@
>>  #include <dma.h>
>>  #include <common.h>
>>  #include <linux/kernel.h>
>> +#include <mach/imx/generic.h>
>> +#include <mach/imx/imx6.h>
>>  
>>  #include "error.h"
>>  #include "regs.h"
>> @@ -143,7 +145,7 @@ static void rng_self_test_done(struct device *dev, u32 *desc, u32 err,
>>   * * i.MX6DLS silicon revision 1.4
>>   * * i.MX6SX silicon revision 1.4
>>   * * i.MX6UL silicon revision 1.2
>> - * * i.MX67SD silicon revision 1.3
>> + * * i.MX7SD silicon revision 1.3
>>   *
>>   */
>>  int caam_rng_self_test(struct device *dev, const u8 caam_era, const u8 rngvid,
>> @@ -176,6 +178,15 @@ int caam_rng_self_test(struct device *dev, const u8 caam_era, const u8 rngvid,
>>  		return -EINVAL;
>>  	}
>>  
>> +	if (cpu_is_mx6() && imx6_cannot_write_l2x0()) {
> 
> Nit:
> Would be nice to have an in_secure_mode() function to make the purpose
> more clear. However, change is correct and you added a comment, so
> 
> Reviewed-by: Marco Felsch <m.felsch@pengutronix.de>

A generic in_secure_mode() function must necessarily be tristate for
architectural reasons: yes, no, don't know.

I guess we could do that, but yes, out of scope for this fix.

Cheers,
Ahmad



> 
> 
>> +		/* If we enter this if clause, we are likely running
>> +		 * under OP-TEE, so there is no point in continuing
>> +		 * and getting -EBUSY waiting for job completion
>> +		 */
>> +		pr_info("skipping test in normal world\n");
>> +		return 0;
>> +	}
>> +
>>  	result = dma_alloc(sizeof(*result) * result_size);
>>  	desc = dma_alloc(sizeof(*desc) * desc_size);
>>  
>> diff --git a/include/mach/imx/imx6.h b/include/mach/imx/imx6.h
>> index 109df9b811c5..7b3cedd80f66 100644
>> --- a/include/mach/imx/imx6.h
>> +++ b/include/mach/imx/imx6.h
>> @@ -135,4 +135,6 @@ int imx6_cpu_revision(void);
>>  
>>  u64 imx6_uid(void);
>>  
>> +bool imx6_cannot_write_l2x0(void);
>> +
>>  #endif /* __MACH_IMX6_H */
>> -- 
>> 2.47.3
>>
>>
> 

-- 
Pengutronix e.K.                  |                             |
Steuerwalder Str. 21              | http://www.pengutronix.de/  |
31137 Hildesheim, Germany         | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686  | Fax:   +49-5121-206917-5555 |