public inbox for u-boot@lists.denx.de
 help / color / mirror / Atom feed
* [PATCH v5] rockchip: board: Increase rng-seed size to make it sufficient for modern Linux
@ 2024-10-15 21:28 Alex Shumsky
  2024-10-16  1:22 ` Dragan Simic
  0 siblings, 1 reply; 3+ messages in thread
From: Alex Shumsky @ 2024-10-15 21:28 UTC (permalink / raw)
  To: u-boot
  Cc: Alex Shumsky, Dragan Simic, Marek Vasut, Ben Wolsieffer,
	Caleb Connolly, Chris Morgan, Devarsh Thakkar, Jagan Teki,
	Janne Grunau, Jiaxun Yang, Jonas Karlman, Joshua Watt, Kever Yang,
	Mattijs Korpershoek, Nikhil M Jain, Philipp Tomsich,
	Philipp Tomsich, Quentin Schulz, Rasmus Villemoes, Simon Glass,
	Stefan Roese, Tim Harvey, Tom Rini

Increase rng-seed size to make Linux happy and initialize rng pool instantly.
Linux 5.19+ requires 32 bytes of entropy to initialize random pool, but u-boot
currently provides only 8 bytes.
Linux 5.18 and probably some versions before it used to require 64 bytes.
Bump min value to 64 bytes to be on a safe side.

Boot with 8 byte rng-seed (Linux 6.11):
    # dmesg | grep crng
    [   12.089286] random: crng init done
Boot with 32 byte rng-seed (Linux 6.11):
    # dmesg | grep crng
    [    0.000000] random: crng init done

Linux source references:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/char/random.c?h=v5.19#n551
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/char/random.c?h=v5.18#n236

Signed-off-by: Alex Shumsky <alexthreed@gmail.com>
Fixes: d2048bafae40 ("rockchip: board: Add board_rng_seed() for all Rockchip devices")
Reviewed-by: Dragan Simic <dsimic@manjaro.org>
Reviewed-by: Marek Vasut <marex@denx.de>
---

Changes in v5:
- add docs

Changes in v4:
- fix typos and code style

Changes in v3:
- reword warning
- increase default rng_seed_size to support older linux as well
- reword commit message

Changes in v2:
- add env config knob rng_seed_size
- add warning for small rng_seed_size
- 12-character commit SHA in Fixes

 arch/arm/mach-rockchip/board.c | 11 ++++++++++-
 common/Kconfig                 |  3 +++
 doc/usage/environment.rst      |  5 +++++
 include/fdt_support.h          |  3 ++-
 4 files changed, 20 insertions(+), 2 deletions(-)

diff --git a/arch/arm/mach-rockchip/board.c b/arch/arm/mach-rockchip/board.c
index 3fadf7e412..ca6404776f 100644
--- a/arch/arm/mach-rockchip/board.c
+++ b/arch/arm/mach-rockchip/board.c
@@ -472,9 +472,18 @@ __weak int misc_init_r(void)
 __weak int board_rng_seed(struct abuf *buf)
 {
 	struct udevice *dev;
-	size_t len = 0x8;
+	ulong len = env_get_ulong("rng_seed_size", 10, 64);
 	u64 *data;
 
+	if (len < 64) {
+		/*
+		 * rng_seed_size should be at least 32 bytes for Linux 5.19+,
+		 * or 64 for older Linux kernel versions
+		 */
+		log_warning("Value for rng_seed_size too low (%lu) and likely insufficient for the Linux RNG initialization\n",
+			    len);
+	}
+
 	data = malloc(len);
 	if (!data) {
 		printf("Out of memory\n");
diff --git a/common/Kconfig b/common/Kconfig
index 957de0c5c0..c1d06f5045 100644
--- a/common/Kconfig
+++ b/common/Kconfig
@@ -927,6 +927,9 @@ config BOARD_RNG_SEED
 	  new seed for use on subsequent boots, and whether or not the
 	  kernel should account any entropy from the given seed.
 
+	  Default seed size (64 bytes) can be overridden by a decimal environment
+	  variable rng_seed_size.
+
 endmenu
 
 menu "Update support"
diff --git a/doc/usage/environment.rst b/doc/usage/environment.rst
index cc33d3ec0f..253b25a002 100644
--- a/doc/usage/environment.rst
+++ b/doc/usage/environment.rst
@@ -323,6 +323,11 @@ netretry
     Useful on scripts which control the retry operation
     themselves.
 
+rng_seed_size
+    Size of random value added to device tree-node /chosen/rng-seed.
+    This variable is given as a hexadecimal number.
+    If unset, 64 bytes is used as the default.
+
 silent_linux
     If set then Linux will be told to boot silently, by
     adding 'console=' to its command line. If "yes" it will be
diff --git a/include/fdt_support.h b/include/fdt_support.h
index 741e2360c2..02561eab15 100644
--- a/include/fdt_support.h
+++ b/include/fdt_support.h
@@ -202,7 +202,8 @@ int ft_board_setup(void *blob, struct bd_info *bd);
  *
  * This function is called if CONFIG_BOARD_RNG_SEED is set, and must
  * be provided by the board. It should return, via @buf, some suitable
- * seed value to pass to the kernel.
+ * seed value to pass to the kernel. Seed size could be set in a decimal
+ * environment variable rng_seed_size and defaulted to 64 bytes.
  *
  * @param buf         A struct abuf for returning the seed and its size.
  * @return            0 if ok, negative on error.
-- 
2.34.1


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

* Re: [PATCH v5] rockchip: board: Increase rng-seed size to make it sufficient for modern Linux
  2024-10-15 21:28 [PATCH v5] rockchip: board: Increase rng-seed size to make it sufficient for modern Linux Alex Shumsky
@ 2024-10-16  1:22 ` Dragan Simic
  2024-10-16 10:05   ` Alex Shumsky
  0 siblings, 1 reply; 3+ messages in thread
From: Dragan Simic @ 2024-10-16  1:22 UTC (permalink / raw)
  To: Alex Shumsky
  Cc: u-boot, Marek Vasut, Ben Wolsieffer, Caleb Connolly, Chris Morgan,
	Devarsh Thakkar, Jagan Teki, Janne Grunau, Jiaxun Yang,
	Jonas Karlman, Joshua Watt, Kever Yang, Mattijs Korpershoek,
	Nikhil M Jain, Philipp Tomsich, Philipp Tomsich, Quentin Schulz,
	Rasmus Villemoes, Simon Glass, Stefan Roese, Tim Harvey, Tom Rini

Hello Alex,

Thanks for the v5.  Please see a few comments below.

On 2024-10-15 23:28, Alex Shumsky wrote:
> Increase rng-seed size to make Linux happy and initialize rng pool 
> instantly.
> Linux 5.19+ requires 32 bytes of entropy to initialize random pool, but 
> u-boot
> currently provides only 8 bytes.
> Linux 5.18 and probably some versions before it used to require 64 
> bytes.
> Bump min value to 64 bytes to be on a safe side.
> 
> Boot with 8 byte rng-seed (Linux 6.11):
>     # dmesg | grep crng
>     [   12.089286] random: crng init done
> Boot with 32 byte rng-seed (Linux 6.11):
>     # dmesg | grep crng
>     [    0.000000] random: crng init done
> 
> Linux source references:
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/char/random.c?h=v5.19#n551
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/char/random.c?h=v5.18#n236
> 
> Signed-off-by: Alex Shumsky <alexthreed@gmail.com>
> Fixes: d2048bafae40 ("rockchip: board: Add board_rng_seed() for all
> Rockchip devices")
> Reviewed-by: Dragan Simic <dsimic@manjaro.org>
> Reviewed-by: Marek Vasut <marex@denx.de>
> ---
> 
> Changes in v5:
> - add docs
> 
> Changes in v4:
> - fix typos and code style
> 
> Changes in v3:
> - reword warning
> - increase default rng_seed_size to support older linux as well
> - reword commit message
> 
> Changes in v2:
> - add env config knob rng_seed_size
> - add warning for small rng_seed_size
> - 12-character commit SHA in Fixes
> 
>  arch/arm/mach-rockchip/board.c | 11 ++++++++++-
>  common/Kconfig                 |  3 +++
>  doc/usage/environment.rst      |  5 +++++
>  include/fdt_support.h          |  3 ++-
>  4 files changed, 20 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/arm/mach-rockchip/board.c 
> b/arch/arm/mach-rockchip/board.c
> index 3fadf7e412..ca6404776f 100644
> --- a/arch/arm/mach-rockchip/board.c
> +++ b/arch/arm/mach-rockchip/board.c
> @@ -472,9 +472,18 @@ __weak int misc_init_r(void)
>  __weak int board_rng_seed(struct abuf *buf)
>  {
>  	struct udevice *dev;
> -	size_t len = 0x8;
> +	ulong len = env_get_ulong("rng_seed_size", 10, 64);
>  	u64 *data;
> 
> +	if (len < 64) {
> +		/*
> +		 * rng_seed_size should be at least 32 bytes for Linux 5.19+,
> +		 * or 64 for older Linux kernel versions
> +		 */
> +		log_warning("Value for rng_seed_size too low (%lu) and likely
> insufficient for the Linux RNG initialization\n",

Please consider changing the message to:

   "Value for rng_seed_size (%lu) too low, Linux kernel RNG
   may fail to initialize early\n"

It's more compact, which is always good, and should be a bit
more precise that way.

> +			    len);
> +	}
> +
>  	data = malloc(len);
>  	if (!data) {
>  		printf("Out of memory\n");
> diff --git a/common/Kconfig b/common/Kconfig
> index 957de0c5c0..c1d06f5045 100644
> --- a/common/Kconfig
> +++ b/common/Kconfig
> @@ -927,6 +927,9 @@ config BOARD_RNG_SEED
>  	  new seed for use on subsequent boots, and whether or not the
>  	  kernel should account any entropy from the given seed.
> 
> +	  Default seed size (64 bytes) can be overridden by a decimal 
> environment
> +	  variable rng_seed_size.

Maybe reflow a bit to follow the column width above.

> +
>  endmenu
> 
>  menu "Update support"
> diff --git a/doc/usage/environment.rst b/doc/usage/environment.rst
> index cc33d3ec0f..253b25a002 100644
> --- a/doc/usage/environment.rst
> +++ b/doc/usage/environment.rst
> @@ -323,6 +323,11 @@ netretry
>      Useful on scripts which control the retry operation
>      themselves.
> 
> +rng_seed_size
> +    Size of random value added to device tree-node /chosen/rng-seed.
> +    This variable is given as a hexadecimal number.

We've come to a conclusion it's a decimal number, right? :)
So, please s/hexadecimal/decimal/

> +    If unset, 64 bytes is used as the default.
> +
>  silent_linux
>      If set then Linux will be told to boot silently, by
>      adding 'console=' to its command line. If "yes" it will be
> diff --git a/include/fdt_support.h b/include/fdt_support.h
> index 741e2360c2..02561eab15 100644
> --- a/include/fdt_support.h
> +++ b/include/fdt_support.h
> @@ -202,7 +202,8 @@ int ft_board_setup(void *blob, struct bd_info *bd);
>   *
>   * This function is called if CONFIG_BOARD_RNG_SEED is set, and must
>   * be provided by the board. It should return, via @buf, some suitable
> - * seed value to pass to the kernel.
> + * seed value to pass to the kernel. Seed size could be set in a 
> decimal
> + * environment variable rng_seed_size and defaulted to 64 bytes.

s/defaulted/it defaults/

>   *
>   * @param buf         A struct abuf for returning the seed and its 
> size.
>   * @return            0 if ok, negative on error.

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

* Re: [PATCH v5] rockchip: board: Increase rng-seed size to make it sufficient for modern Linux
  2024-10-16  1:22 ` Dragan Simic
@ 2024-10-16 10:05   ` Alex Shumsky
  0 siblings, 0 replies; 3+ messages in thread
From: Alex Shumsky @ 2024-10-16 10:05 UTC (permalink / raw)
  To: Dragan Simic
  Cc: u-boot, Marek Vasut, Ben Wolsieffer, Caleb Connolly, Chris Morgan,
	Devarsh Thakkar, Jagan Teki, Janne Grunau, Jiaxun Yang,
	Jonas Karlman, Joshua Watt, Kever Yang, Mattijs Korpershoek,
	Nikhil M Jain, Philipp Tomsich, Philipp Tomsich, Quentin Schulz,
	Rasmus Villemoes, Simon Glass, Stefan Roese, Tim Harvey, Tom Rini

On Wed, Oct 16, 2024 at 4:22 AM Dragan Simic <dsimic@manjaro.org> wrote:
>
> Hello Alex,
>
> Thanks for the v5.  Please see a few comments below.
>
> On 2024-10-15 23:28, Alex Shumsky wrote:
> > Increase rng-seed size to make Linux happy and initialize rng pool
> > instantly.
> > Linux 5.19+ requires 32 bytes of entropy to initialize random pool, but
> > u-boot
> > currently provides only 8 bytes.
> > Linux 5.18 and probably some versions before it used to require 64
> > bytes.
> > Bump min value to 64 bytes to be on a safe side.
> >
> > Boot with 8 byte rng-seed (Linux 6.11):
> >     # dmesg | grep crng
> >     [   12.089286] random: crng init done
> > Boot with 32 byte rng-seed (Linux 6.11):
> >     # dmesg | grep crng
> >     [    0.000000] random: crng init done
> >
> > Linux source references:
> > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/char/random.c?h=v5.19#n551
> > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/char/random.c?h=v5.18#n236
> >
> > Signed-off-by: Alex Shumsky <alexthreed@gmail.com>
> > Fixes: d2048bafae40 ("rockchip: board: Add board_rng_seed() for all
> > Rockchip devices")
> > Reviewed-by: Dragan Simic <dsimic@manjaro.org>
> > Reviewed-by: Marek Vasut <marex@denx.de>
> > ---
> >
> > Changes in v5:
> > - add docs
> >
> > Changes in v4:
> > - fix typos and code style
> >
> > Changes in v3:
> > - reword warning
> > - increase default rng_seed_size to support older linux as well
> > - reword commit message
> >
> > Changes in v2:
> > - add env config knob rng_seed_size
> > - add warning for small rng_seed_size
> > - 12-character commit SHA in Fixes
> >
> >  arch/arm/mach-rockchip/board.c | 11 ++++++++++-
> >  common/Kconfig                 |  3 +++
> >  doc/usage/environment.rst      |  5 +++++
> >  include/fdt_support.h          |  3 ++-
> >  4 files changed, 20 insertions(+), 2 deletions(-)
> >
> > diff --git a/arch/arm/mach-rockchip/board.c
> > b/arch/arm/mach-rockchip/board.c
> > index 3fadf7e412..ca6404776f 100644
> > --- a/arch/arm/mach-rockchip/board.c
> > +++ b/arch/arm/mach-rockchip/board.c
> > @@ -472,9 +472,18 @@ __weak int misc_init_r(void)
> >  __weak int board_rng_seed(struct abuf *buf)
> >  {
> >       struct udevice *dev;
> > -     size_t len = 0x8;
> > +     ulong len = env_get_ulong("rng_seed_size", 10, 64);
> >       u64 *data;
> >
> > +     if (len < 64) {
> > +             /*
> > +              * rng_seed_size should be at least 32 bytes for Linux 5.19+,
> > +              * or 64 for older Linux kernel versions
> > +              */
> > +             log_warning("Value for rng_seed_size too low (%lu) and likely
> > insufficient for the Linux RNG initialization\n",
>
> Please consider changing the message to:
>
>    "Value for rng_seed_size (%lu) too low, Linux kernel RNG
>    may fail to initialize early\n"
>
> It's more compact, which is always good, and should be a bit
> more precise that way.
>
> > +                         len);
> > +     }
> > +
> >       data = malloc(len);
> >       if (!data) {
> >               printf("Out of memory\n");
> > diff --git a/common/Kconfig b/common/Kconfig
> > index 957de0c5c0..c1d06f5045 100644
> > --- a/common/Kconfig
> > +++ b/common/Kconfig
> > @@ -927,6 +927,9 @@ config BOARD_RNG_SEED
> >         new seed for use on subsequent boots, and whether or not the
> >         kernel should account any entropy from the given seed.
> >
> > +       Default seed size (64 bytes) can be overridden by a decimal
> > environment
> > +       variable rng_seed_size.
>
> Maybe reflow a bit to follow the column width above.
>
> > +
> >  endmenu
> >
> >  menu "Update support"
> > diff --git a/doc/usage/environment.rst b/doc/usage/environment.rst
> > index cc33d3ec0f..253b25a002 100644
> > --- a/doc/usage/environment.rst
> > +++ b/doc/usage/environment.rst
> > @@ -323,6 +323,11 @@ netretry
> >      Useful on scripts which control the retry operation
> >      themselves.
> >
> > +rng_seed_size
> > +    Size of random value added to device tree-node /chosen/rng-seed.
> > +    This variable is given as a hexadecimal number.
>
> We've come to a conclusion it's a decimal number, right? :)
> So, please s/hexadecimal/decimal/
>
> > +    If unset, 64 bytes is used as the default.
> > +
> >  silent_linux
> >      If set then Linux will be told to boot silently, by
> >      adding 'console=' to its command line. If "yes" it will be
> > diff --git a/include/fdt_support.h b/include/fdt_support.h
> > index 741e2360c2..02561eab15 100644
> > --- a/include/fdt_support.h
> > +++ b/include/fdt_support.h
> > @@ -202,7 +202,8 @@ int ft_board_setup(void *blob, struct bd_info *bd);
> >   *
> >   * This function is called if CONFIG_BOARD_RNG_SEED is set, and must
> >   * be provided by the board. It should return, via @buf, some suitable
> > - * seed value to pass to the kernel.
> > + * seed value to pass to the kernel. Seed size could be set in a
> > decimal
> > + * environment variable rng_seed_size and defaulted to 64 bytes.
>
> s/defaulted/it defaults/
>

Thanks, fixed. I'm not good at wording.

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

end of thread, other threads:[~2024-10-16 10:07 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-10-15 21:28 [PATCH v5] rockchip: board: Increase rng-seed size to make it sufficient for modern Linux Alex Shumsky
2024-10-16  1:22 ` Dragan Simic
2024-10-16 10:05   ` Alex Shumsky

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox