diff mbox

[U-Boot,V2,2/3] lib: rand: add call to hw_rand() - hardware random number generator

Message ID 216028592db390f90b2c9583c08f83978ee183a0.1394038415.git.p.marczak@samsung.com
State Changes Requested
Delegated to: Tom Rini
Headers show

Commit Message

Przemyslaw Marczak March 5, 2014, 4:57 p.m. UTC
Changes:
- lib/rand.c: add call to hw_rand() (depends on CONFIG_RAND_HW_ACCEL)
- include/common.h: add hw_rand() declaration.

Signed-off-by: Przemyslaw Marczak <p.marczak@samsung.com>
cc: Michael Walle <michael@walle.cc>
cc: Tom Rini <trini@ti.com>
---
Changes v2:
- move function hw_rand() from rand() to rand_r() and ignore its argument

 include/common.h |    3 +++
 lib/rand.c       |    3 +++
 2 files changed, 6 insertions(+)

Comments

Tom Rini March 5, 2014, 9:22 p.m. UTC | #1
On Wed, Mar 05, 2014 at 05:57:46PM +0100, Przemyslaw Marczak wrote:

> Changes:
> - lib/rand.c: add call to hw_rand() (depends on CONFIG_RAND_HW_ACCEL)
> - include/common.h: add hw_rand() declaration.
> 
> Signed-off-by: Przemyslaw Marczak <p.marczak@samsung.com>
> cc: Michael Walle <michael@walle.cc>
> cc: Tom Rini <trini@ti.com>
> ---
> Changes v2:
> - move function hw_rand() from rand() to rand_r() and ignore its argument
> 
>  include/common.h |    3 +++
>  lib/rand.c       |    3 +++
>  2 files changed, 6 insertions(+)
> 
> diff --git a/include/common.h b/include/common.h
> index 96a45a6..58e2fbc 100644
> --- a/include/common.h
> +++ b/include/common.h
> @@ -836,6 +836,9 @@ void srand(unsigned int seed);
>  unsigned int rand(void);
>  unsigned int rand_r(unsigned int *seedp);
>  #endif
> +#ifdef CONFIG_RAND_HW_ACCEL
> +unsigned int hw_rand(void);
> +#endif
>  
>  /* common/console.c */
>  int	console_init_f(void);	/* Before relocation; uses the serial  stuff	*/
> diff --git a/lib/rand.c b/lib/rand.c
> index 5c367e1..0617063 100644
> --- a/lib/rand.c
> +++ b/lib/rand.c
> @@ -14,6 +14,9 @@ static unsigned int y = 1U;
>  
>  unsigned int rand_r(unsigned int *seedp)
>  {
> +#ifdef CONFIG_RAND_HW_ACCEL
> +	return hw_rand();
> +#endif
>  	*seedp ^= (*seedp << 13);
>  	*seedp ^= (*seedp >> 17);
>  	*seedp ^= (*seedp << 5);

This doesn't already generate warnings about unreachable code?

I hate to say at but I think we should add lib/hw_rand.c which does:
void srand(uint seed) {}
unsigned int rand(void) { return hw_rand(); }
unsigned int rand_r(unsigned int *seedp) { seedp = hw_rand(); return
*seedp;}

(please double check how hw_rand() returns and net/link_local.c if we
can really avoid using the callers pointer...).

And then in correct Kbuild fashion, something like
randsrc-y ?= rand.o
randsrc-$(CONFIG_RAND_HW_ACCEL) = hw_rand.o

(The above is probably wrong, help please Masahiro :))
Masahiro Yamada March 6, 2014, 4:58 a.m. UTC | #2
Hello Tom,

> >  unsigned int rand_r(unsigned int *seedp)
> >  {
> > +#ifdef CONFIG_RAND_HW_ACCEL
> > +	return hw_rand();
> > +#endif
> >  	*seedp ^= (*seedp << 13);
> >  	*seedp ^= (*seedp >> 17);
> >  	*seedp ^= (*seedp << 5);
> 
> This doesn't already generate warnings about unreachable code?
> 
> I hate to say at but I think we should add lib/hw_rand.c which does:
> void srand(uint seed) {}
> unsigned int rand(void) { return hw_rand(); }
> unsigned int rand_r(unsigned int *seedp) { seedp = hw_rand(); return
> *seedp;}
> 
> (please double check how hw_rand() returns and net/link_local.c if we
> can really avoid using the callers pointer...).
> 
> And then in correct Kbuild fashion, something like
> randsrc-y ?= rand.o
> randsrc-$(CONFIG_RAND_HW_ACCEL) = hw_rand.o
> 
> (The above is probably wrong, help please Masahiro :))

In the first place, I don't like the idea to add a new function
hw_rand() very much.

I think lib/rand.c is just one implementation among
some possible choices.
drivers/crypto/ace_sha.c is another implementation.

How about treating them in the same way?

I mean,  srand(), rand(), rand_r() 
should be defined in drivers/crypto/ace_sha.c
rather than adding lib/hw_rand.c.


drivers/crypto/ace_sha.c is like this:

void srand(uint seed) {}
unsigned int rand(void)
{
         <your hardware rand implementation>
}
unsigned int rand_r(unsigned int *seedp)
{
       return rand();
}




lib/Makefile will be changed like this:

  --- a/lib/Makefile
  +++ b/lib/Makefile
  @@ -62,8 +62,6 @@ obj-y += time.o
   obj-$(CONFIG_TRACE) += trace.o
   obj-$(CONFIG_BOOTP_PXE) += uuid.o
   obj-y += vsprintf.o
  -obj-$(CONFIG_RANDOM_MACADDR) += rand.o
  -obj-$(CONFIG_BOOTP_RANDOM_DELAY) += rand.o
  -obj-$(CONFIG_CMD_LINK_LOCAL) += rand.o
  +obj-$(CONFIG_SW_RAND) += rand.o
 


It looks like CONFIG_SW_RAND (or CONFIG_EXYNOS_ACE_SHA)
must be added to the following boards.

include/configs/lsxl.h                (for CONFIG_RANDOM_MACADDR)
include/configs/MERGERBOX.h  (for CONFIG_BOOTP_RAMDOM_DELAY)
include/configs/MVBC_P.h         (for CONFIG_BOOTP_RAMDOM_DELAY)
include/configs/MVBLM7.h         (for CONFIG_BOOTP_RAMDOM_DELAY)
include/configs/MVSMR.h          (for CONFIG_BOOTP_RAMDOM_DELAY)
include/configs/bfin_adi_common.h  (for CONFIG_BOOTP_RAMDOM_DELAY)
include/configs/sacsng.h            (for CONFIG_BOOTP_RAMDOM_DELAY)
include/config_uncmd_spl.h       (for CONFIG_CMD_LINK_LOCAL)
include/configs/a3m071.h          (for CONFIG_CMD_LINK_LOCAL)


Best Regards
Masahiro Yamada
diff mbox

Patch

diff --git a/include/common.h b/include/common.h
index 96a45a6..58e2fbc 100644
--- a/include/common.h
+++ b/include/common.h
@@ -836,6 +836,9 @@  void srand(unsigned int seed);
 unsigned int rand(void);
 unsigned int rand_r(unsigned int *seedp);
 #endif
+#ifdef CONFIG_RAND_HW_ACCEL
+unsigned int hw_rand(void);
+#endif
 
 /* common/console.c */
 int	console_init_f(void);	/* Before relocation; uses the serial  stuff	*/
diff --git a/lib/rand.c b/lib/rand.c
index 5c367e1..0617063 100644
--- a/lib/rand.c
+++ b/lib/rand.c
@@ -14,6 +14,9 @@  static unsigned int y = 1U;
 
 unsigned int rand_r(unsigned int *seedp)
 {
+#ifdef CONFIG_RAND_HW_ACCEL
+	return hw_rand();
+#endif
 	*seedp ^= (*seedp << 13);
 	*seedp ^= (*seedp >> 17);
 	*seedp ^= (*seedp << 5);