Hi Pawel > -----Original Message----- > From: Paweł Kochanowski <[email protected]> > Sent: Tuesday, April 8, 2025 4:30 PM > To: Gaurav Jain <[email protected]>; [email protected] > Cc: Priyanka Jain <[email protected]>; Gabriel Nesteruk > <[email protected]>; Pankaj Gupta <[email protected]>; Horia Geanta > <[email protected]> > Subject: RE: [EXT] [PATCH] crypto: fsl - Fix RNG generation for lengths > greater > than 16 bytes > > [You don't often get email from [email protected]. Learn why this is > important > at https://aka.ms/LearnAboutSenderIdentification ] > > Caution: This is an external email. Please take care when clicking links or > opening > attachments. When in doubt, report the message using the 'Report this email' > button > > > 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. 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 am also not sure how the swapped descriptor worked second time. > The effect that we see is that when leaving the command executing this > sequence > we get an exception in "unrelated" place.. After your change you stop seeing this exception? Are you sure it is related to rng descriptor? > > This is what we see when printing descriptor in the caam_rng_read_one on > LS1046A: > > First iteration: > 0: B0800005 > 1: 82500002 > 2: 60340010 > 3: 0 > 4: FBC94DC0 4th word is seqoutptr: sgf pre ext (ews). Have you also modified the descriptor ? Regards Gaurav > > Second iteration: > 0: 050080B0 > 1: 02005082 > 2: 10003460 > 3: 0 > 4: C04DC9FB > > BR, > Pawel. > > > From: Gaurav Jain <[email protected]> > > Sent: Tuesday, April 8, 2025 12:09 PM > > > > Can you explain more details about this issue. Where it is being > > modified in > > run_descriptor_jr() ? > > Regards > > Gaurav > > > > > -----Original Message----- > > > From: Pawel Kochanowski <[email protected]> > > > Sent: Monday, April 7, 2025 6:17 PM > > > To: [email protected] > > > Cc: Priyanka Jain <[email protected]>; Gaurav Jain > > > <[email protected]>; Gabriel Nesteruk <[email protected]>; Paweł > > > Kochanowski <[email protected]> > > > Subject: [EXT] [PATCH] crypto: fsl - Fix RNG generation for lengths > > > greater than 16 bytes > > > > > > [You don't often get email from [email protected]. Learn why this > > > is important at https://aka.ms/LearnAboutSenderIdentification ] > > > > > > Caution: This is an external email. Please take care when clicking > > > links or opening attachments. When in doubt, report the message > > > using the > > 'Report this email' > > > button > > > > > > > > > From: Gabriel Nesteruk <[email protected]> > > > > > > Reinitialize the descriptor for each RNG job, as it may be modified > > > by run_descriptor_jr(). > > > Failing to do so can result in memory corruption when > > > dm_rng_read() is called a second time on NXP devices with > > > CONFIG_SYS_FSL_SEC_BE enabled. > > > > > > Signed-off-by: Gabriel Nesteruk <[email protected]> > > > Signed-off-by: Pawel Kochanowski <[email protected]> > > > --- > > > drivers/crypto/fsl/rng.c | 14 ++++++-------- > > > 1 file changed, 6 insertions(+), 8 deletions(-) > > > > > > diff --git a/drivers/crypto/fsl/rng.c b/drivers/crypto/fsl/rng.c > > > index > > > 06364948052..440b26e3c94 100644 > > > --- a/drivers/crypto/fsl/rng.c > > > +++ b/drivers/crypto/fsl/rng.c > > > @@ -27,8 +27,14 @@ struct caam_rng_priv { static int > > > caam_rng_read_one(struct caam_rng_priv *priv) { > > > int size = ALIGN(CAAM_RNG_MAX_FIFO_STORE_SIZE, > > > ARCH_DMA_MINALIGN); > > > + ulong rng_desc_size = ALIGN(CAAM_RNG_DESC_LEN, > > > + ARCH_DMA_MINALIGN); > > > int ret; > > > > > > + inline_cnstr_jobdesc_rng(priv->desc, priv->data, > > > + CAAM_RNG_MAX_FIFO_STORE_SIZE); > > > + flush_dcache_range((unsigned long)priv->desc, > > > + (unsigned long)priv->desc + rng_desc_size); > > > + > > > ret = run_descriptor_jr(priv->desc); > > > if (ret < 0) > > > return -EIO; > > > @@ -63,14 +69,6 @@ static int caam_rng_read(struct udevice *dev, > > > void *data, size_t len) > > > > > > static int caam_rng_probe(struct udevice *dev) { > > > - struct caam_rng_priv *priv = dev_get_priv(dev); > > > - ulong size = ALIGN(CAAM_RNG_DESC_LEN, ARCH_DMA_MINALIGN); > > > - > > > - inline_cnstr_jobdesc_rng(priv->desc, priv->data, > > > - CAAM_RNG_MAX_FIFO_STORE_SIZE); > > > - flush_dcache_range((unsigned long)priv->desc, > > > - (unsigned long)priv->desc + size); > > > - > > > return 0; > > > } > > > > > > -- > > > 2.43.0
