2012-05-23 12:32:14

by Peter Korsgaard

[permalink] [raw]
Subject: [PATCH] hw_random: atmel-rng: fix data valid check

Brown paper bag: Data valid is LSB of the ISR (status register), and NOT
of ODATA (current random data word)!

With this, rngtest is a lot happier. Before:

rngtest 3
Copyright (c) 2004 by Henrique de Moraes Holschuh
This is free software; see the source for copying conditions. There is NO warr.

rngtest: starting FIPS tests...
rngtest: bits received from input: 20000032
rngtest: FIPS 140-2 successes: 3
rngtest: FIPS 140-2 failures: 997
rngtest: FIPS 140-2(2001-10-10) Monobit: 604
rngtest: FIPS 140-2(2001-10-10) Poker: 996
rngtest: FIPS 140-2(2001-10-10) Runs: 36
rngtest: FIPS 140-2(2001-10-10) Long run: 0
rngtest: FIPS 140-2(2001-10-10) Continuous run: 117
rngtest: input channel speed: (min=622.371; avg=23682.481; max=28224.350)Kibitss
rngtest: FIPS tests speed: (min=12.361; avg=12.718; max=12.861)Mibits/s
rngtest: Program run time: 2331696 microsecondsx

After:
rngtest 3
Copyright (c) 2004 by Henrique de Moraes Holschuh
This is free software; see the source for copying conditions. There is NO warr.

rngtest: starting FIPS tests...
rngtest: bits received from input: 20000032
rngtest: FIPS 140-2 successes: 999
rngtest: FIPS 140-2 failures: 1
rngtest: FIPS 140-2(2001-10-10) Monobit: 0
rngtest: FIPS 140-2(2001-10-10) Poker: 0
rngtest: FIPS 140-2(2001-10-10) Runs: 1
rngtest: FIPS 140-2(2001-10-10) Long run: 0
rngtest: FIPS 140-2(2001-10-10) Continuous run: 0
rngtest: input channel speed: (min=777.363; avg=43588.270; max=47870.711)Kibitss
rngtest: FIPS tests speed: (min=11.943; avg=12.716; max=12.844)Mibits/s
rngtest: Program run time: 1955282 microseconds

Cc: [email protected]
Signed-off-by: Peter Korsgaard <[email protected]>
Reported-by: George Pontis <[email protected]>
---
drivers/char/hw_random/atmel-rng.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/char/hw_random/atmel-rng.c b/drivers/char/hw_random/atmel-rng.c
index f518b99..d7ab920 100644
--- a/drivers/char/hw_random/atmel-rng.c
+++ b/drivers/char/hw_random/atmel-rng.c
@@ -34,7 +34,7 @@ static int atmel_trng_read(struct hwrng *rng, void *buf, size_t max,
u32 *data = buf;

/* data ready? */
- if (readl(trng->base + TRNG_ODATA) & 1) {
+ if (readl(trng->base + TRNG_ISR) & 1) {
*data = readl(trng->base + TRNG_ODATA);
return 4;
} else
--
1.7.10


2012-05-23 13:00:58

by Nicolas Ferre

[permalink] [raw]
Subject: Re: [PATCH] hw_random: atmel-rng: fix data valid check

On 05/23/2012 02:31 PM, Peter Korsgaard :
> Brown paper bag: Data valid is LSB of the ISR (status register), and NOT
> of ODATA (current random data word)!
>
> With this, rngtest is a lot happier. Before:
>
> rngtest 3
> Copyright (c) 2004 by Henrique de Moraes Holschuh
> This is free software; see the source for copying conditions. There is NO warr.
>
> rngtest: starting FIPS tests...
> rngtest: bits received from input: 20000032
> rngtest: FIPS 140-2 successes: 3
> rngtest: FIPS 140-2 failures: 997
> rngtest: FIPS 140-2(2001-10-10) Monobit: 604
> rngtest: FIPS 140-2(2001-10-10) Poker: 996
> rngtest: FIPS 140-2(2001-10-10) Runs: 36
> rngtest: FIPS 140-2(2001-10-10) Long run: 0
> rngtest: FIPS 140-2(2001-10-10) Continuous run: 117
> rngtest: input channel speed: (min=622.371; avg=23682.481; max=28224.350)Kibitss
> rngtest: FIPS tests speed: (min=12.361; avg=12.718; max=12.861)Mibits/s
> rngtest: Program run time: 2331696 microsecondsx
>
> After:
> rngtest 3
> Copyright (c) 2004 by Henrique de Moraes Holschuh
> This is free software; see the source for copying conditions. There is NO warr.
>
> rngtest: starting FIPS tests...
> rngtest: bits received from input: 20000032
> rngtest: FIPS 140-2 successes: 999
> rngtest: FIPS 140-2 failures: 1
> rngtest: FIPS 140-2(2001-10-10) Monobit: 0
> rngtest: FIPS 140-2(2001-10-10) Poker: 0
> rngtest: FIPS 140-2(2001-10-10) Runs: 1
> rngtest: FIPS 140-2(2001-10-10) Long run: 0
> rngtest: FIPS 140-2(2001-10-10) Continuous run: 0
> rngtest: input channel speed: (min=777.363; avg=43588.270; max=47870.711)Kibitss
> rngtest: FIPS tests speed: (min=11.943; avg=12.716; max=12.844)Mibits/s
> rngtest: Program run time: 1955282 microseconds
>
> Cc: [email protected]
> Signed-off-by: Peter Korsgaard <[email protected]>
> Reported-by: George Pontis <[email protected]>

Indeed, yes ;-)

Acked-by: Nicolas Ferre <[email protected]>

> ---
> drivers/char/hw_random/atmel-rng.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/char/hw_random/atmel-rng.c b/drivers/char/hw_random/atmel-rng.c
> index f518b99..d7ab920 100644
> --- a/drivers/char/hw_random/atmel-rng.c
> +++ b/drivers/char/hw_random/atmel-rng.c
> @@ -34,7 +34,7 @@ static int atmel_trng_read(struct hwrng *rng, void *buf, size_t max,
> u32 *data = buf;
>
> /* data ready? */
> - if (readl(trng->base + TRNG_ODATA) & 1) {
> + if (readl(trng->base + TRNG_ISR) & 1) {
> *data = readl(trng->base + TRNG_ODATA);
> return 4;
> } else


--
Nicolas Ferre

2012-06-11 16:16:01

by Nicolas Ferre

[permalink] [raw]
Subject: Re: [PATCH] hw_random: atmel-rng: fix data valid check

On 05/23/2012 03:00 PM, Nicolas Ferre :
> On 05/23/2012 02:31 PM, Peter Korsgaard :
>> Brown paper bag: Data valid is LSB of the ISR (status register), and NOT
>> of ODATA (current random data word)!
>>
>> With this, rngtest is a lot happier. Before:
>>
>> rngtest 3
>> Copyright (c) 2004 by Henrique de Moraes Holschuh
>> This is free software; see the source for copying conditions. There is NO warr.
>>
>> rngtest: starting FIPS tests...
>> rngtest: bits received from input: 20000032
>> rngtest: FIPS 140-2 successes: 3
>> rngtest: FIPS 140-2 failures: 997
>> rngtest: FIPS 140-2(2001-10-10) Monobit: 604
>> rngtest: FIPS 140-2(2001-10-10) Poker: 996
>> rngtest: FIPS 140-2(2001-10-10) Runs: 36
>> rngtest: FIPS 140-2(2001-10-10) Long run: 0
>> rngtest: FIPS 140-2(2001-10-10) Continuous run: 117
>> rngtest: input channel speed: (min=622.371; avg=23682.481; max=28224.350)Kibitss
>> rngtest: FIPS tests speed: (min=12.361; avg=12.718; max=12.861)Mibits/s
>> rngtest: Program run time: 2331696 microsecondsx
>>
>> After:
>> rngtest 3
>> Copyright (c) 2004 by Henrique de Moraes Holschuh
>> This is free software; see the source for copying conditions. There is NO warr.
>>
>> rngtest: starting FIPS tests...
>> rngtest: bits received from input: 20000032
>> rngtest: FIPS 140-2 successes: 999
>> rngtest: FIPS 140-2 failures: 1
>> rngtest: FIPS 140-2(2001-10-10) Monobit: 0
>> rngtest: FIPS 140-2(2001-10-10) Poker: 0
>> rngtest: FIPS 140-2(2001-10-10) Runs: 1
>> rngtest: FIPS 140-2(2001-10-10) Long run: 0
>> rngtest: FIPS 140-2(2001-10-10) Continuous run: 0
>> rngtest: input channel speed: (min=777.363; avg=43588.270; max=47870.711)Kibitss
>> rngtest: FIPS tests speed: (min=11.943; avg=12.716; max=12.844)Mibits/s
>> rngtest: Program run time: 1955282 microseconds
>>
>> Cc: [email protected]
>> Signed-off-by: Peter Korsgaard <[email protected]>
>> Reported-by: George Pontis <[email protected]>
>
> Indeed, yes ;-)
>
> Acked-by: Nicolas Ferre <[email protected]>

Hi Herbert,

It seems that this patch did not make it in your tree before the "other
one" from Peter: in linus' tree: 121daad (hwrng: atmel-rng - fix race
condition leading to repeated bits).

So, do you want me to rebase this patch because it will not apply anymore?

Thanks for your help, best regards,

>> ---
>> drivers/char/hw_random/atmel-rng.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/char/hw_random/atmel-rng.c b/drivers/char/hw_random/atmel-rng.c
>> index f518b99..d7ab920 100644
>> --- a/drivers/char/hw_random/atmel-rng.c
>> +++ b/drivers/char/hw_random/atmel-rng.c
>> @@ -34,7 +34,7 @@ static int atmel_trng_read(struct hwrng *rng, void *buf, size_t max,
>> u32 *data = buf;
>>
>> /* data ready? */
>> - if (readl(trng->base + TRNG_ODATA) & 1) {
>> + if (readl(trng->base + TRNG_ISR) & 1) {
>> *data = readl(trng->base + TRNG_ODATA);
>> return 4;
>> } else
>
>


--
Nicolas Ferre

2012-06-12 00:17:21

by Herbert Xu

[permalink] [raw]
Subject: Re: [PATCH] hw_random: atmel-rng: fix data valid check

On Mon, Jun 11, 2012 at 06:15:53PM +0200, Nicolas Ferre wrote:
>
> It seems that this patch did not make it in your tree before the "other
> one" from Peter: in linus' tree: 121daad (hwrng: atmel-rng - fix race
> condition leading to repeated bits).
>
> So, do you want me to rebase this patch because it will not apply anymore?

Oops, I'll fix this up.

Thanks!
--
Email: Herbert Xu <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt