[EXT] [PATCH] crypto: fsl - Fix RNG generation for lengths greater than 16 bytes

[Horia Geanta]

On 4/9/2025 9:19 AM, Gaurav Jain wrote:
> Hi Pawel
> 
>> From: Paweł Kochanowski <pkochanowski at sii.pl>
>>
>> Hi Gaurav,
>>
>> What we see is that the jr_enqueue() called by run_descriptor_jr() swaps the
>> endianness of the descriptor in place (by modifying the data pointed by
>> desc_add):
>>
>>         /* The descriptor must be submitted to SEC block as per endianness
>>          * of the SEC Block.
>>          * So, if the endianness of Core and SEC block is different, each word
>>          * of the descriptor will be byte-swapped.
>>          */
>>         for (i = 0; i < length; i++) {
>>                 desc_word = desc_addr[i];
>>                 sec_out32((uint32_t *)&desc_addr[i], desc_word);
>>         }
>>
>> So the sequence looks like this:
>> 1. caam_rng_probe sets correct descriptor in ` caam_rng_priv *priv` 2.
>> caam_rng_read is called with 32B 3. caam_rng_read_one->run_descriptor_jr() is
>> called to generate 16B with `priv->desc` containing valid descriptor 4. The
>> descriptor is swapped in jr_enqueue() before passing it to job ring
> 
> I see you are right. In Linux, caam endianness is handled at the time of creating the descriptor.
> But In Uboot, caam endianness is not handled at the time of descriptor creation.
> 
IMO the U-Boot approach is broken.
The handling of endianness must be done at creation time, only then we know
how to interpret the data and perform adequate byteswaps.
U-Boot makes the assumption that everything should be 4-byte swapped,
but there are cases when the swap has to be done 8-byte wide (for example
address pointers or 8-byte immediate data).

> 5. The loop in
>> caam_rng_read is called second time, this time the `priv->desc` contain swapped
>> data.
>>
>> Interesting thing is that the job still succeeds and that some data are present in
>> the buffers, but maybe swapped descriptor can also be a valid one?
> I agree that the descriptor should be reinitialized for each RNG job.
I would advise against this, considering what I said above.
Handling endianness should be done only once.

Regards,
Horia