linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] hwrng: cn10k: Add extended trng register support
@ 2023-05-18  6:47 Bharat Bhushan
  2023-05-24  9:56 ` Herbert Xu
  0 siblings, 1 reply; 3+ messages in thread
From: Bharat Bhushan @ 2023-05-18  6:47 UTC (permalink / raw)
  To: olivia, herbert, Jason, linux-crypto, linux-kernel, sgoutham
  Cc: Bharat Bhushan

The way random data is read from hardware has changed from
Octeon CN10KA-B0 and later SoCs onwards. A new set of registers
have been added to read random data and to verify whether the
read data is valid or not. This patch extends and uses
RNM_PF_TRNG_DAT and RNM_PF_TRNG_STS CSRs to read random number
and status for the applicable silicon variants.

Signed-off-by: Bharat Bhushan <bbhushan2@marvell.com>
---
 drivers/char/hw_random/cn10k-rng.c | 64 ++++++++++++++++++++++++++++--
 1 file changed, 60 insertions(+), 4 deletions(-)

diff --git a/drivers/char/hw_random/cn10k-rng.c b/drivers/char/hw_random/cn10k-rng.c
index c1193f85982c..42e44705320c 100644
--- a/drivers/char/hw_random/cn10k-rng.c
+++ b/drivers/char/hw_random/cn10k-rng.c
@@ -23,14 +23,49 @@
 #define RNM_PF_RANDOM		0x400
 #define RNM_TRNG_RESULT		0x408
 
+/* Extended TRNG Read and Status Registers */
+#define RNM_PF_TRNG_DAT		0x1000
+#define RNM_PF_TRNG_RES		0x1008
+
 struct cn10k_rng {
 	void __iomem *reg_base;
 	struct hwrng ops;
 	struct pci_dev *pdev;
+	/* Octeon CN10K-A A0/A1, CNF10K-A A0/A1 and CNF10K-B A0/B0
+	 * does not support extended TRNG registers
+	 */
+	bool extended_trng_regs;
 };
 
 #define PLAT_OCTEONTX_RESET_RNG_EBG_HEALTH_STATE     0xc2000b0f
 
+#define PCI_SUBSYS_DEVID_CN10K_A_RNG	0xB900
+#define PCI_SUBSYS_DEVID_CNF10K_A_RNG	0xBA00
+#define PCI_SUBSYS_DEVID_CNF10K_B_RNG	0xBC00
+
+static bool cn10k_is_extended_trng_regs_supported(struct pci_dev *pdev)
+{
+	/* CN10K-A A0/A1 */
+	if ((pdev->subsystem_device == PCI_SUBSYS_DEVID_CN10K_A_RNG) &&
+	    (!pdev->revision || (pdev->revision & 0xff) == 0x50 ||
+	     (pdev->revision & 0xff) == 0x51))
+		return false;
+
+	/* CNF10K-A A0 */
+	if ((pdev->subsystem_device == PCI_SUBSYS_DEVID_CNF10K_A_RNG) &&
+	    (!pdev->revision || (pdev->revision & 0xff) == 0x60 ||
+	     (pdev->revision & 0xff) == 0x61))
+		return false;
+
+	/* CNF10K-B A0/B0 */
+	if ((pdev->subsystem_device == PCI_SUBSYS_DEVID_CNF10K_B_RNG) &&
+	    (!pdev->revision || (pdev->revision & 0xff) == 0x70 ||
+	     (pdev->revision & 0xff) == 0x74))
+		return false;
+
+	return true;
+}
+
 static unsigned long reset_rng_health_state(struct cn10k_rng *rng)
 {
 	struct arm_smccc_res res;
@@ -63,9 +98,22 @@ static int check_rng_health(struct cn10k_rng *rng)
 	return 0;
 }
 
-static void cn10k_read_trng(struct cn10k_rng *rng, u64 *value)
+static size_t cn10k_read_trng(struct cn10k_rng *rng, u64 *value)
 {
+	u16 retry_count = 0;
 	u64 upper, lower;
+	u64 status;
+
+	if (rng->extended_trng_regs) {
+		do {
+			*value = readq(rng->reg_base + RNM_PF_TRNG_DAT);
+			if (*value)
+				return 8;
+			status = readq(rng->reg_base + RNM_PF_TRNG_RES);
+			if (!status && (retry_count++ > 0x1000))
+				return 0;
+		} while (!status);
+	}
 
 	*value = readq(rng->reg_base + RNM_PF_RANDOM);
 
@@ -82,6 +130,7 @@ static void cn10k_read_trng(struct cn10k_rng *rng, u64 *value)
 
 		*value = (upper & 0xFFFFFFFF00000000) | (lower & 0xFFFFFFFF);
 	}
+	return 8;
 }
 
 static int cn10k_rng_read(struct hwrng *hwrng, void *data,
@@ -100,7 +149,9 @@ static int cn10k_rng_read(struct hwrng *hwrng, void *data,
 	size = max;
 
 	while (size >= 8) {
-		cn10k_read_trng(rng, &value);
+		err = cn10k_read_trng(rng, &value);
+		if (!err)
+			goto out;
 
 		*((u64 *)pos) = value;
 		size -= 8;
@@ -108,7 +159,9 @@ static int cn10k_rng_read(struct hwrng *hwrng, void *data,
 	}
 
 	if (size > 0) {
-		cn10k_read_trng(rng, &value);
+		err = cn10k_read_trng(rng, &value);
+		if (!err)
+			goto out;
 
 		while (size > 0) {
 			*pos = (u8)value;
@@ -118,6 +171,7 @@ static int cn10k_rng_read(struct hwrng *hwrng, void *data,
 		}
 	}
 
+out:
 	return max - size;
 }
 
@@ -144,9 +198,11 @@ static int cn10k_rng_probe(struct pci_dev *pdev, const struct pci_device_id *id)
 	if (!rng->ops.name)
 		return -ENOMEM;
 
-	rng->ops.read    = cn10k_rng_read;
+	rng->ops.read = cn10k_rng_read;
 	rng->ops.priv = (unsigned long)rng;
 
+	rng->extended_trng_regs = cn10k_is_extended_trng_regs_supported(pdev);
+
 	reset_rng_health_state(rng);
 
 	err = devm_hwrng_register(&pdev->dev, &rng->ops);
-- 
2.17.1


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

* Re: [PATCH] hwrng: cn10k: Add extended trng register support
  2023-05-18  6:47 [PATCH] hwrng: cn10k: Add extended trng register support Bharat Bhushan
@ 2023-05-24  9:56 ` Herbert Xu
  2023-05-24 11:56   ` [EXT] " Bharat Bhushan
  0 siblings, 1 reply; 3+ messages in thread
From: Herbert Xu @ 2023-05-24  9:56 UTC (permalink / raw)
  To: Bharat Bhushan; +Cc: olivia, Jason, linux-crypto, linux-kernel, sgoutham

On Thu, May 18, 2023 at 12:17:34PM +0530, Bharat Bhushan wrote:
>
> -static void cn10k_read_trng(struct cn10k_rng *rng, u64 *value)
> +static size_t cn10k_read_trng(struct cn10k_rng *rng, u64 *value)

Since the return value is either 0 or 8, why did you pick size_t
as the return value?

>  {
> +	u16 retry_count = 0;
>  	u64 upper, lower;
> +	u64 status;
> +
> +	if (rng->extended_trng_regs) {
> +		do {
> +			*value = readq(rng->reg_base + RNM_PF_TRNG_DAT);
> +			if (*value)
> +				return 8;
> +			status = readq(rng->reg_base + RNM_PF_TRNG_RES);
> +			if (!status && (retry_count++ > 0x1000))
> +				return 0;
> +		} while (!status);
> +	}
>  
>  	*value = readq(rng->reg_base + RNM_PF_RANDOM);
>  
> @@ -82,6 +130,7 @@ static void cn10k_read_trng(struct cn10k_rng *rng, u64 *value)
>  
>  		*value = (upper & 0xFFFFFFFF00000000) | (lower & 0xFFFFFFFF);
>  	}
> +	return 8;
>  }
>  
>  static int cn10k_rng_read(struct hwrng *hwrng, void *data,
> @@ -100,7 +149,9 @@ static int cn10k_rng_read(struct hwrng *hwrng, void *data,
>  	size = max;
>  
>  	while (size >= 8) {
> -		cn10k_read_trng(rng, &value);
> +		err = cn10k_read_trng(rng, &value);
> +		if (!err)
> +			goto out;

It appears that err is either 8 or 0, so it's not really an error.
Could you plesae use a more meaningful name for this variable?

Cheers,
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

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

* RE: [EXT] Re: [PATCH] hwrng: cn10k: Add extended trng register support
  2023-05-24  9:56 ` Herbert Xu
@ 2023-05-24 11:56   ` Bharat Bhushan
  0 siblings, 0 replies; 3+ messages in thread
From: Bharat Bhushan @ 2023-05-24 11:56 UTC (permalink / raw)
  To: Herbert Xu
  Cc: olivia, Jason, linux-crypto, linux-kernel, Sunil Kovvuri Goutham



> -----Original Message-----
> From: Herbert Xu <herbert@gondor.apana.org.au>
> Sent: Wednesday, May 24, 2023 3:26 PM
> To: Bharat Bhushan <bbhushan2@marvell.com>
> Cc: olivia@selenic.com; Jason@zx2c4.com; linux-crypto@vger.kernel.org; linux-
> kernel@vger.kernel.org; Sunil Kovvuri Goutham <sgoutham@marvell.com>
> Subject: [EXT] Re: [PATCH] hwrng: cn10k: Add extended trng register support
> 
> External Email
> 
> ----------------------------------------------------------------------
> On Thu, May 18, 2023 at 12:17:34PM +0530, Bharat Bhushan wrote:
> >
> > -static void cn10k_read_trng(struct cn10k_rng *rng, u64 *value)
> > +static size_t cn10k_read_trng(struct cn10k_rng *rng, u64 *value)
> 
> Since the return value is either 0 or 8, why did you pick size_t as the return value?

This function returns number of valid bytes available in "value". 0 means no valid data. size_t is definitely wrong.
I think I will make this function to return bool, true when data is valid and false when data is not valid.

Thanks
-Bharat

> 
> >  {
> > +	u16 retry_count = 0;
> >  	u64 upper, lower;
> > +	u64 status;
> > +
> > +	if (rng->extended_trng_regs) {
> > +		do {
> > +			*value = readq(rng->reg_base + RNM_PF_TRNG_DAT);
> > +			if (*value)
> > +				return 8;
> > +			status = readq(rng->reg_base + RNM_PF_TRNG_RES);
> > +			if (!status && (retry_count++ > 0x1000))
> > +				return 0;
> > +		} while (!status);
> > +	}
> >
> >  	*value = readq(rng->reg_base + RNM_PF_RANDOM);
> >
> > @@ -82,6 +130,7 @@ static void cn10k_read_trng(struct cn10k_rng *rng,
> > u64 *value)
> >
> >  		*value = (upper & 0xFFFFFFFF00000000) | (lower & 0xFFFFFFFF);
> >  	}
> > +	return 8;
> >  }
> >
> >  static int cn10k_rng_read(struct hwrng *hwrng, void *data, @@ -100,7
> > +149,9 @@ static int cn10k_rng_read(struct hwrng *hwrng, void *data,
> >  	size = max;
> >
> >  	while (size >= 8) {
> > -		cn10k_read_trng(rng, &value);
> > +		err = cn10k_read_trng(rng, &value);
> > +		if (!err)
> > +			goto out;
> 
> It appears that err is either 8 or 0, so it's not really an error.
> Could you plesae use a more meaningful name for this variable?
> 
> Cheers,
> --
> Email: Herbert Xu <herbert@gondor.apana.org.au> Home Page:
> https://urldefense.proofpoint.com/v2/url?u=http-3A__gondor.apana.org.au_-
> 7Eherbert_&d=DwIBAg&c=nKjWec2b6R0mOyPaz7xtfQ&r=PAAlWswPe7d8gHlGb
> CLmy2YezyK7O3Hv_t2heGnouBw&m=Ya8SJ6OhypttG2itpa39dd7kXwEXGgbUOJ2
> Hqi95w7MetfxqRKKFX7aluHaLZpoc&s=pnRAp3oSzsNrbEAHZdLM6Eb9M4tDMxEd
> WLzANKejHwU&e=
> PGP Key: https://urldefense.proofpoint.com/v2/url?u=http-
> 3A__gondor.apana.org.au_-
> 7Eherbert_pubkey.txt&d=DwIBAg&c=nKjWec2b6R0mOyPaz7xtfQ&r=PAAlWswP
> e7d8gHlGbCLmy2YezyK7O3Hv_t2heGnouBw&m=Ya8SJ6OhypttG2itpa39dd7kXwE
> XGgbUOJ2Hqi95w7MetfxqRKKFX7aluHaLZpoc&s=pLpT-
> xdKvVvCI4sp5r8r5qYoKeEx537Gnlkq1dpTrYY&e=

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

end of thread, other threads:[~2023-05-24 11:57 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-05-18  6:47 [PATCH] hwrng: cn10k: Add extended trng register support Bharat Bhushan
2023-05-24  9:56 ` Herbert Xu
2023-05-24 11:56   ` [EXT] " Bharat Bhushan

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).