HRNG in CAAM isn't working properly on IMX6 SoloX

* HRNG in CAAM isn't working properly on IMX6 SoloX
@ 2021-08-30 11:21 Fredrik Yhlen
  2021-08-30 11:49 ` Fabio Estevam
  0 siblings, 1 reply; 5+ messages in thread
From: Fredrik Yhlen @ 2021-08-30 11:21 UTC (permalink / raw)
  To: horia.geanta
  Cc: aymen.sghaier, herbert, davem, linux-crypto, linux-kernel,
	andrew.smirnov

Hi,

We're having problems with hwrng on a board with imx6sx (soloX) running Linux
5.10.x. mainline, and I have tracked it down to this commit
'358ba762d9f1d4ba99ab31ef12bc28014b22f4c9' as being the culprit.

The caam_jr driver spits out lots of messages when attempting to read from /dev/hwrng:
```
[29717.629041] hwrng: no data available
[29727.859008] caam_jr 2101000.jr: 20003c5b: CCB: desc idx 60: RNG: Hardware error
```

```
caam_jr 2101000.jr0: 2000025b: CCB: desc idx 2: RNG: Hardware error.
caam_jr 2101000.jr0: 20003c5b: CCB: desc idx 60: RNG: Hardware error.
caam_jr 2101000.jr0: 20003c5b: CCB: desc idx 60: RNG: Hardware error.
caam_jr 2101000.jr0: 20003c5b: CCB: desc idx 60: RNG: Hardware error.
caam_jr 2101000.jr0: 20003c5b: CCB: desc idx 60: RNG: Hardware error.
caam_jr 2101000.jr0: 20003c5b: CCB: desc idx 60: RNG: Hardware error.
```

This also happens on Boundary's Nitrogen6_soloX board when running the same
kernel, and likewise with their latest Yocto release that uses 5.4.100 linux-imx kernel.

```
root@nitrogen6sx:~# dd if=/dev/hwrng of=/tmp/random bs=256 count=10
[  113.940735] caam_jr 2101000.jr0: 20003c5b: CCB: desc idx 60: RNG: Hardware error
dd: /dev/hwrng: Invalid argument
root@nitrogen6sx:~# rm /tmp/random
root@nitrogen6sx:~# dd if=/dev/hwrng of=/tmp/random bs=256 count=10
[  125.300823] caam_jr 2101000.jr0: 20003c5b: CCB: desc idx 60: RNG: Hardware error
dd: /dev/hwrng: Invalid argument
root@nitrogen6sx:~# du -hs /tmp/random
0       /tmp/random
root@nitrogen6sx:~# ls -l /tmp/random
-rw-r--r--    1 root     root             0 Dec 16 17:27 /tmp/random
root@nitrogen6sx:~#
```

And then no data is available from /dev/hwrng.

The problem occurs when adding OP_ALG_PR_ON(prediction resistance) when setting up
job descriptor for reading new random data in caamrng.c. There are also
some confusing parts about this commit that I'm not too sure about.

1. It's adding a conditional variable named 'pr_support', but I guess this only
indicates if the MC(Management Complex) supports prediction resistance,
since the following check can be bypassed when 'pr_support' is false.

    /*
     * If SEC has RNG version >= 4 and RNG state handle has not been
     * already instantiated, do RNG instantiation
     * In case of SoCs with Management Complex, RNG is managed by MC f/w.
     */
    if (!(ctrlpriv->mc_en && pr_support) && rng_vid >= 4) {


This will eventually lead to the following chain call: caam_probe() -> instantiate_rng() ->
build_instantiation_desc(), where OP_ALG_PR_ON will be used through DECO.

static void build_instantiation_desc(u32 *desc, int handle, int do_sk)
{
    u32 *jump_cmd, op_flags;

    init_job_desc(desc, 0);

    op_flags = OP_TYPE_CLASS1_ALG | OP_ALG_ALGSEL_RNG |
            (handle << OP_ALG_AAI_SHIFT) | OP_ALG_AS_INIT |
            OP_ALG_PR_ON;
    ...
    ...
    ...

Shouldn't it be named 'mc_pr_support' instead, or something similar?

2. PR is unconditionally used in caamrng.c(caam_jr module) when
reading new RNG data. Should this be the case?

Removing OP_ALG_PR_ON in caam_init_desc() from drivers/crypto/caam/caamrng.c
seems to fix the problem we're experiencing, here's an example:
```

diff --git a/drivers/crypto/caam/caamrng.c b/drivers/crypto/caam/caamrng.c
index 77d048dfe5d0..f085a80b1b3c 100644
--- a/drivers/crypto/caam/caamrng.c
+++ b/drivers/crypto/caam/caamrng.c
@@ -67,8 +67,7 @@ static u32 *caam_init_desc(u32 *desc, dma_addr_t dst_dma)
 {
        init_job_desc(desc, 0); /* + 1 cmd_sz */
        /* Generate random bytes: + 1 cmd_sz */
-       append_operation(desc, OP_ALG_ALGSEL_RNG | OP_TYPE_CLASS1_ALG |
-                        OP_ALG_PR_ON);
+       append_operation(desc, OP_ALG_ALGSEL_RNG | OP_TYPE_CLASS1_ALG);
        /* Store bytes: + 1 cmd_sz + caam_ptr_sz  */
        append_fifo_store(desc, dst_dma,
                          CAAM_RNG_MAX_FIFO_STORE_SIZE, FIFOST_TYPE_RNGSTORE);
```

Best regards,
Fredrik

^ permalink raw reply related	[flat|nested] 5+ messages in thread