The platform's RNG must be available before random_init() in order to be useful for initial seeding, which in turn means that it needs to be called from setup_arch(), rather than from an init call. Fortunately, each platform already has a setup_arch function pointer, which means it's easy to wire this up. This commit also removes some noisy log messages that don't add much.
Cc: stable@vger.kernel.org Cc: Michael Ellerman mpe@ellerman.id.au Cc: Christophe Leroy christophe.leroy@csgroup.eu Fixes: a4da0d50b2a0 ("powerpc: Implement arch_get_random_long/int() for powernv") Signed-off-by: Jason A. Donenfeld Jason@zx2c4.com --- arch/powerpc/platforms/powernv/powernv.h | 2 ++ arch/powerpc/platforms/powernv/rng.c | 18 +++++------------- arch/powerpc/platforms/powernv/setup.c | 2 ++ 3 files changed, 9 insertions(+), 13 deletions(-)
diff --git a/arch/powerpc/platforms/powernv/powernv.h b/arch/powerpc/platforms/powernv/powernv.h index e297bf4abfcb..fd3f5e1eb10b 100644 --- a/arch/powerpc/platforms/powernv/powernv.h +++ b/arch/powerpc/platforms/powernv/powernv.h @@ -42,4 +42,6 @@ ssize_t memcons_copy(struct memcons *mc, char *to, loff_t pos, size_t count); u32 __init memcons_get_size(struct memcons *mc); struct memcons *__init memcons_init(struct device_node *node, const char *mc_prop_name);
+void powernv_rng_init(void); + #endif /* _POWERNV_H */ diff --git a/arch/powerpc/platforms/powernv/rng.c b/arch/powerpc/platforms/powernv/rng.c index e3d44b36ae98..c86bf097e407 100644 --- a/arch/powerpc/platforms/powernv/rng.c +++ b/arch/powerpc/platforms/powernv/rng.c @@ -17,6 +17,7 @@ #include <asm/prom.h> #include <asm/machdep.h> #include <asm/smp.h> +#include "powernv.h"
#define DARN_ERR 0xFFFFFFFFFFFFFFFFul
@@ -84,24 +85,20 @@ static int powernv_get_random_darn(unsigned long *v) return 1; }
-static int __init initialise_darn(void) +static void __init initialise_darn(void) { unsigned long val; int i;
if (!cpu_has_feature(CPU_FTR_ARCH_300)) - return -ENODEV; + return;
for (i = 0; i < 10; i++) { if (powernv_get_random_darn(&val)) { ppc_md.get_random_seed = powernv_get_random_darn; - return 0; + return; } } - - pr_warn("Unable to use DARN for get_random_seed()\n"); - - return -EIO; }
int powernv_get_random_long(unsigned long *v) @@ -163,14 +160,12 @@ static __init int rng_create(struct device_node *dn)
rng_init_per_cpu(rng, dn);
- pr_info_once("Registering arch random hook.\n"); - ppc_md.get_random_seed = powernv_get_random_long;
return 0; }
-static __init int rng_init(void) +void __init powernv_rng_init(void) { struct device_node *dn; int rc; @@ -188,7 +183,4 @@ static __init int rng_init(void) }
initialise_darn(); - - return 0; } -machine_subsys_initcall(powernv, rng_init); diff --git a/arch/powerpc/platforms/powernv/setup.c b/arch/powerpc/platforms/powernv/setup.c index 824c3ad7a0fa..a5fcb6796b22 100644 --- a/arch/powerpc/platforms/powernv/setup.c +++ b/arch/powerpc/platforms/powernv/setup.c @@ -203,6 +203,8 @@ static void __init pnv_setup_arch(void) pnv_check_guarded_cores();
/* XXX PMCS */ + + powernv_rng_init(); }
static void __init pnv_init(void)
Le 11/06/2022 à 17:10, Jason A. Donenfeld a écrit :
The platform's RNG must be available before random_init() in order to be useful for initial seeding, which in turn means that it needs to be called from setup_arch(), rather than from an init call. Fortunately, each platform already has a setup_arch function pointer, which means it's easy to wire this up. This commit also removes some noisy log messages that don't add much.
Cc: stable@vger.kernel.org Cc: Michael Ellerman mpe@ellerman.id.au Cc: Christophe Leroy christophe.leroy@csgroup.eu Fixes: a4da0d50b2a0 ("powerpc: Implement arch_get_random_long/int() for powernv") Signed-off-by: Jason A. Donenfeld Jason@zx2c4.com
Reviewed-by: Christophe Leroy christophe.leroy@csgroup.eu
arch/powerpc/platforms/powernv/powernv.h | 2 ++ arch/powerpc/platforms/powernv/rng.c | 18 +++++------------- arch/powerpc/platforms/powernv/setup.c | 2 ++ 3 files changed, 9 insertions(+), 13 deletions(-)
diff --git a/arch/powerpc/platforms/powernv/powernv.h b/arch/powerpc/platforms/powernv/powernv.h index e297bf4abfcb..fd3f5e1eb10b 100644 --- a/arch/powerpc/platforms/powernv/powernv.h +++ b/arch/powerpc/platforms/powernv/powernv.h @@ -42,4 +42,6 @@ ssize_t memcons_copy(struct memcons *mc, char *to, loff_t pos, size_t count); u32 __init memcons_get_size(struct memcons *mc); struct memcons *__init memcons_init(struct device_node *node, const char *mc_prop_name); +void powernv_rng_init(void);
- #endif /* _POWERNV_H */
diff --git a/arch/powerpc/platforms/powernv/rng.c b/arch/powerpc/platforms/powernv/rng.c index e3d44b36ae98..c86bf097e407 100644 --- a/arch/powerpc/platforms/powernv/rng.c +++ b/arch/powerpc/platforms/powernv/rng.c @@ -17,6 +17,7 @@ #include <asm/prom.h> #include <asm/machdep.h> #include <asm/smp.h> +#include "powernv.h" #define DARN_ERR 0xFFFFFFFFFFFFFFFFul @@ -84,24 +85,20 @@ static int powernv_get_random_darn(unsigned long *v) return 1; } -static int __init initialise_darn(void) +static void __init initialise_darn(void) { unsigned long val; int i; if (!cpu_has_feature(CPU_FTR_ARCH_300))
return -ENODEV;
return;
for (i = 0; i < 10; i++) { if (powernv_get_random_darn(&val)) { ppc_md.get_random_seed = powernv_get_random_darn;
return 0;
} }return;
- pr_warn("Unable to use DARN for get_random_seed()\n");
- return -EIO; }
int powernv_get_random_long(unsigned long *v) @@ -163,14 +160,12 @@ static __init int rng_create(struct device_node *dn) rng_init_per_cpu(rng, dn);
- pr_info_once("Registering arch random hook.\n");
- ppc_md.get_random_seed = powernv_get_random_long;
return 0; } -static __init int rng_init(void) +void __init powernv_rng_init(void) { struct device_node *dn; int rc; @@ -188,7 +183,4 @@ static __init int rng_init(void) } initialise_darn();
- return 0; }
-machine_subsys_initcall(powernv, rng_init); diff --git a/arch/powerpc/platforms/powernv/setup.c b/arch/powerpc/platforms/powernv/setup.c index 824c3ad7a0fa..a5fcb6796b22 100644 --- a/arch/powerpc/platforms/powernv/setup.c +++ b/arch/powerpc/platforms/powernv/setup.c @@ -203,6 +203,8 @@ static void __init pnv_setup_arch(void) pnv_check_guarded_cores(); /* XXX PMCS */
- powernv_rng_init(); }
static void __init pnv_init(void)
"Jason A. Donenfeld" Jason@zx2c4.com writes:
The platform's RNG must be available before random_init() in order to be useful for initial seeding, which in turn means that it needs to be called from setup_arch(), rather than from an init call. Fortunately, each platform already has a setup_arch function pointer, which means it's easy to wire this up. This commit also removes some noisy log messages that don't add much.
Cc: stable@vger.kernel.org Cc: Michael Ellerman mpe@ellerman.id.au Cc: Christophe Leroy christophe.leroy@csgroup.eu Fixes: a4da0d50b2a0 ("powerpc: Implement arch_get_random_long/int() for powernv") Signed-off-by: Jason A. Donenfeld Jason@zx2c4.com
arch/powerpc/platforms/powernv/powernv.h | 2 ++ arch/powerpc/platforms/powernv/rng.c | 18 +++++------------- arch/powerpc/platforms/powernv/setup.c | 2 ++ 3 files changed, 9 insertions(+), 13 deletions(-)
...
diff --git a/arch/powerpc/platforms/powernv/setup.c b/arch/powerpc/platforms/powernv/setup.c index 824c3ad7a0fa..a5fcb6796b22 100644 --- a/arch/powerpc/platforms/powernv/setup.c +++ b/arch/powerpc/platforms/powernv/setup.c @@ -203,6 +203,8 @@ static void __init pnv_setup_arch(void) pnv_check_guarded_cores(); /* XXX PMCS */
- powernv_rng_init();
}
This crashes on power8 because it's too early to call kzalloc() in rng_create(), and it's also too early to setup the percpu variables in there.
I'll rework it and post a v4.
cheers
Hi Michael,
On Sun, Jun 19, 2022 at 1:49 PM Michael Ellerman mpe@ellerman.id.au wrote:
This crashes on power8 because it's too early to call kzalloc() in rng_create(), and it's also too early to setup the percpu variables in there.
I'll rework it and post a v4.
Oh, darn. Sorry about that. Thanks for reworking it.
Jason
The platform's RNG must be available before random_init() in order to be useful for initial seeding, which in turn means that it needs to be called from setup_arch(), rather than from an init call. Fortunately, each platform already has a setup_arch function pointer, which means we can wire it up that way. Complicating things, however, is that POWER8 systems need some per-cpu state and kmalloc, which isn't available at this stage. So we split things into an early phase and a late phase, with the early phase working well enough to seed the RNG with a spinlock, before later getting fast per-cpu allocations. This commit also removes some noisy log messages that don't add much.
Cc: stable@vger.kernel.org Cc: Michael Ellerman mpe@ellerman.id.au Reviewed-by: Christophe Leroy christophe.leroy@csgroup.eu Fixes: a4da0d50b2a0 ("powerpc: Implement arch_get_random_long/int() for powernv") Signed-off-by: Jason A. Donenfeld Jason@zx2c4.com --- arch/powerpc/platforms/powernv/powernv.h | 2 + arch/powerpc/platforms/powernv/rng.c | 68 ++++++++++++++++++------ arch/powerpc/platforms/powernv/setup.c | 2 + 3 files changed, 55 insertions(+), 17 deletions(-)
diff --git a/arch/powerpc/platforms/powernv/powernv.h b/arch/powerpc/platforms/powernv/powernv.h index e297bf4abfcb..fd3f5e1eb10b 100644 --- a/arch/powerpc/platforms/powernv/powernv.h +++ b/arch/powerpc/platforms/powernv/powernv.h @@ -42,4 +42,6 @@ ssize_t memcons_copy(struct memcons *mc, char *to, loff_t pos, size_t count); u32 __init memcons_get_size(struct memcons *mc); struct memcons *__init memcons_init(struct device_node *node, const char *mc_prop_name);
+void powernv_rng_init(void); + #endif /* _POWERNV_H */ diff --git a/arch/powerpc/platforms/powernv/rng.c b/arch/powerpc/platforms/powernv/rng.c index e3d44b36ae98..c1beced9c32c 100644 --- a/arch/powerpc/platforms/powernv/rng.c +++ b/arch/powerpc/platforms/powernv/rng.c @@ -17,6 +17,7 @@ #include <asm/prom.h> #include <asm/machdep.h> #include <asm/smp.h> +#include "powernv.h"
#define DARN_ERR 0xFFFFFFFFFFFFFFFFul
@@ -28,6 +29,12 @@ struct powernv_rng {
static DEFINE_PER_CPU(struct powernv_rng *, powernv_rng);
+static struct { + struct powernv_rng rng; + spinlock_t lock; +} early_state __initdata = { + .lock = __SPIN_LOCK_UNLOCKED(powernv_early_rng) +};
int powernv_hwrng_present(void) { @@ -84,7 +91,7 @@ static int powernv_get_random_darn(unsigned long *v) return 1; }
-static int __init initialise_darn(void) +static int __init initialize_darn(void) { unsigned long val; int i; @@ -98,10 +105,18 @@ static int __init initialise_darn(void) return 0; } } + return -EIO; +}
- pr_warn("Unable to use DARN for get_random_seed()\n"); +static int __init powernv_get_random_long_early(unsigned long *v) +{ + unsigned long flags;
- return -EIO; + spin_lock_irqsave(&early_state.lock, flags); + *v = rng_whiten(&early_state.rng, in_be64(early_state.rng.regs)); + spin_unlock_irqrestore(&early_state.lock, flags); + + return 1; }
int powernv_get_random_long(unsigned long *v) @@ -163,32 +178,51 @@ static __init int rng_create(struct device_node *dn)
rng_init_per_cpu(rng, dn);
- pr_info_once("Registering arch random hook.\n"); - ppc_md.get_random_seed = powernv_get_random_long;
return 0; }
-static __init int rng_init(void) +void __init powernv_rng_init(void) +{ + struct device_node *dn; + struct resource res; + + /* Prefer darn over the rest. */ + if (!initialize_darn()) + return; + + dn = of_find_compatible_node(NULL, NULL, "ibm,power-rng"); + if (!dn) + return; + if (of_address_to_resource(dn, 0, &res)) + return; + early_state.rng.regs_real = (void __iomem *)res.start; + early_state.rng.regs = of_iomap(dn, 0); + if (!early_state.rng.regs) + return; + early_state.rng.mask = in_be64(early_state.rng.regs); + ppc_md.get_random_seed = powernv_get_random_long_early; +} + +static __init int powernv_rng_late_init(void) { struct device_node *dn; - int rc; + + /* + * If this didn't get initialized early on, then we're using darn, + * or this isn't available at all, so return early. + */ + if (ppc_md.get_random_seed != powernv_get_random_long_early) + return 0; + ppc_md.get_random_seed = NULL;
for_each_compatible_node(dn, NULL, "ibm,power-rng") { - rc = rng_create(dn); - if (rc) { - pr_err("Failed creating rng for %pOF (%d).\n", - dn, rc); + if (rng_create(dn)) continue; - } - /* Create devices for hwrng driver */ of_platform_device_create(dn, NULL, NULL); } - - initialise_darn(); - return 0; } -machine_subsys_initcall(powernv, rng_init); +machine_subsys_initcall(powernv, powernv_rng_late_init); diff --git a/arch/powerpc/platforms/powernv/setup.c b/arch/powerpc/platforms/powernv/setup.c index 824c3ad7a0fa..a5fcb6796b22 100644 --- a/arch/powerpc/platforms/powernv/setup.c +++ b/arch/powerpc/platforms/powernv/setup.c @@ -203,6 +203,8 @@ static void __init pnv_setup_arch(void) pnv_check_guarded_cores();
/* XXX PMCS */ + + powernv_rng_init(); }
static void __init pnv_init(void)
The platform's RNG must be available before random_init() in order to be useful for initial seeding, which in turn means that it needs to be called from setup_arch(), rather than from an init call. Fortunately, each platform already has a setup_arch function pointer, which means we can wire it up that way. Complicating things, however, is that POWER8 systems need some per-cpu state and kmalloc, which isn't available at this stage. So we split things up into an early phase and a later opportunistic phase. This commit also removes some noisy log messages that don't add much.
Cc: stable@vger.kernel.org Cc: Michael Ellerman mpe@ellerman.id.au Reviewed-by: Christophe Leroy christophe.leroy@csgroup.eu Fixes: a4da0d50b2a0 ("powerpc: Implement arch_get_random_long/int() for powernv") Signed-off-by: Jason A. Donenfeld Jason@zx2c4.com --- arch/powerpc/platforms/powernv/powernv.h | 2 + arch/powerpc/platforms/powernv/rng.c | 47 ++++++++++++++++-------- arch/powerpc/platforms/powernv/setup.c | 2 + 3 files changed, 35 insertions(+), 16 deletions(-)
diff --git a/arch/powerpc/platforms/powernv/powernv.h b/arch/powerpc/platforms/powernv/powernv.h index e297bf4abfcb..fd3f5e1eb10b 100644 --- a/arch/powerpc/platforms/powernv/powernv.h +++ b/arch/powerpc/platforms/powernv/powernv.h @@ -42,4 +42,6 @@ ssize_t memcons_copy(struct memcons *mc, char *to, loff_t pos, size_t count); u32 __init memcons_get_size(struct memcons *mc); struct memcons *__init memcons_init(struct device_node *node, const char *mc_prop_name);
+void powernv_rng_init(void); + #endif /* _POWERNV_H */ diff --git a/arch/powerpc/platforms/powernv/rng.c b/arch/powerpc/platforms/powernv/rng.c index e3d44b36ae98..e332e72e1857 100644 --- a/arch/powerpc/platforms/powernv/rng.c +++ b/arch/powerpc/platforms/powernv/rng.c @@ -17,6 +17,7 @@ #include <asm/prom.h> #include <asm/machdep.h> #include <asm/smp.h> +#include "powernv.h"
#define DARN_ERR 0xFFFFFFFFFFFFFFFFul
@@ -28,7 +29,6 @@ struct powernv_rng {
static DEFINE_PER_CPU(struct powernv_rng *, powernv_rng);
- int powernv_hwrng_present(void) { struct powernv_rng *rng; @@ -98,9 +98,6 @@ static int __init initialise_darn(void) return 0; } } - - pr_warn("Unable to use DARN for get_random_seed()\n"); - return -EIO; }
@@ -163,32 +160,50 @@ static __init int rng_create(struct device_node *dn)
rng_init_per_cpu(rng, dn);
- pr_info_once("Registering arch random hook.\n"); - ppc_md.get_random_seed = powernv_get_random_long;
return 0; }
-static __init int rng_init(void) +static int __init powernv_get_random_long_early(unsigned long *v) { struct device_node *dn; - int rc; + + if (!slab_is_available()) + return 0; + + if (cmpxchg(&ppc_md.get_random_seed, powernv_get_random_long_early, + NULL) != powernv_get_random_long_early) + return 0;
for_each_compatible_node(dn, NULL, "ibm,power-rng") { - rc = rng_create(dn); - if (rc) { - pr_err("Failed creating rng for %pOF (%d).\n", - dn, rc); + if (rng_create(dn)) continue; - } - /* Create devices for hwrng driver */ of_platform_device_create(dn, NULL, NULL); }
- initialise_darn(); + if (!ppc_md.get_random_seed) + return 0; + return ppc_md.get_random_seed(v); +} + +void __init powernv_rng_init(void) +{ + /* Prefer darn over the rest. */ + if (!initialise_darn()) + return; + + if (of_find_compatible_node(NULL, NULL, "ibm,power-rng")) + ppc_md.get_random_seed = powernv_get_random_long_early; +}
+static int __init powernv_rng_late_init(void) +{ + unsigned long v; + /* In case it wasn't called during init for some other reason. */ + if (ppc_md.get_random_seed == powernv_get_random_long_early) + powernv_get_random_long_early(&v); return 0; } -machine_subsys_initcall(powernv, rng_init); +machine_subsys_initcall(powernv, powernv_rng_late_init); diff --git a/arch/powerpc/platforms/powernv/setup.c b/arch/powerpc/platforms/powernv/setup.c index 824c3ad7a0fa..a5fcb6796b22 100644 --- a/arch/powerpc/platforms/powernv/setup.c +++ b/arch/powerpc/platforms/powernv/setup.c @@ -203,6 +203,8 @@ static void __init pnv_setup_arch(void) pnv_check_guarded_cores();
/* XXX PMCS */ + + powernv_rng_init(); }
static void __init pnv_init(void)
Le 21/06/2022 à 16:08, Jason A. Donenfeld a écrit :
The platform's RNG must be available before random_init() in order to be useful for initial seeding, which in turn means that it needs to be called from setup_arch(), rather than from an init call. Fortunately, each platform already has a setup_arch function pointer, which means we can wire it up that way. Complicating things, however, is that POWER8 systems need some per-cpu state and kmalloc, which isn't available at this stage. So we split things up into an early phase and a later opportunistic phase. This commit also removes some noisy log messages that don't add much.
Regarding the kmalloc(), I have not looked at it in details, but usually you can use memblock_alloc() when kmalloc is not available yet.
Christophe
Hi Christophe,
On Tue, Jun 21, 2022 at 06:33:11PM +0000, Christophe Leroy wrote:
Le 21/06/2022 à 16:08, Jason A. Donenfeld a écrit :
The platform's RNG must be available before random_init() in order to be useful for initial seeding, which in turn means that it needs to be called from setup_arch(), rather than from an init call. Fortunately, each platform already has a setup_arch function pointer, which means we can wire it up that way. Complicating things, however, is that POWER8 systems need some per-cpu state and kmalloc, which isn't available at this stage. So we split things up into an early phase and a later opportunistic phase. This commit also removes some noisy log messages that don't add much.
Regarding the kmalloc(), I have not looked at it in details, but usually you can use memblock_alloc() when kmalloc is not available yet.
That seems a bit excessive, especially as those allocations are long lived. And we don't even *need* it that early, but just before random_init(). Michael is running this v5 on the test rig overnight, so we'll learn in the Australian morning whether this finally did the trick (I hope).
Jason
Le 21/06/2022 à 20:47, Jason A. Donenfeld a écrit :
Hi Christophe,
On Tue, Jun 21, 2022 at 06:33:11PM +0000, Christophe Leroy wrote:
Le 21/06/2022 à 16:08, Jason A. Donenfeld a écrit :
The platform's RNG must be available before random_init() in order to be useful for initial seeding, which in turn means that it needs to be called from setup_arch(), rather than from an init call. Fortunately, each platform already has a setup_arch function pointer, which means we can wire it up that way. Complicating things, however, is that POWER8 systems need some per-cpu state and kmalloc, which isn't available at this stage. So we split things up into an early phase and a later opportunistic phase. This commit also removes some noisy log messages that don't add much.
Regarding the kmalloc(), I have not looked at it in details, but usually you can use memblock_alloc() when kmalloc is not available yet.
That seems a bit excessive, especially as those allocations are long lived. And we don't even *need* it that early, but just before random_init(). Michael is running this v5 on the test rig overnight, so we'll learn in the Australian morning whether this finally did the trick (I hope).
The fact that they are long lived make them a good candidate for memblock_alloc().
But fair enough, if they are not required that early then just do it later.
Christophe
Christophe Leroy christophe.leroy@csgroup.eu writes:
Le 21/06/2022 à 20:47, Jason A. Donenfeld a écrit :
On Tue, Jun 21, 2022 at 06:33:11PM +0000, Christophe Leroy wrote:
Le 21/06/2022 à 16:08, Jason A. Donenfeld a écrit :
The platform's RNG must be available before random_init() in order to be useful for initial seeding, which in turn means that it needs to be called from setup_arch(), rather than from an init call. Fortunately, each platform already has a setup_arch function pointer, which means we can wire it up that way. Complicating things, however, is that POWER8 systems need some per-cpu state and kmalloc, which isn't available at this stage. So we split things up into an early phase and a later opportunistic phase. This commit also removes some noisy log messages that don't add much.
Regarding the kmalloc(), I have not looked at it in details, but usually you can use memblock_alloc() when kmalloc is not available yet.
That seems a bit excessive, especially as those allocations are long lived. And we don't even *need* it that early, but just before random_init(). Michael is running this v5 on the test rig overnight, so we'll learn in the Australian morning whether this finally did the trick (I hope).
The fact that they are long lived make them a good candidate for memblock_alloc().
But fair enough, if they are not required that early then just do it later.
memblock works but then we trip on ioremap vs early_ioremap.
Fixing that is a bit of a pain as we'd have to stop using of_iomap() and we'd also need to switch the mappings to ioremap() later in boot.
We'd also have to defer the percpu initialisation.
So it's all just a bit of a pain when we actually only need to get the hook ready before random_init() which is called much later in boot when slab/ioremap/percpu are all ready.
cheers
On Tue, 21 Jun 2022 16:08:49 +0200, Jason A. Donenfeld wrote:
The platform's RNG must be available before random_init() in order to be useful for initial seeding, which in turn means that it needs to be called from setup_arch(), rather than from an init call. Fortunately, each platform already has a setup_arch function pointer, which means we can wire it up that way. Complicating things, however, is that POWER8 systems need some per-cpu state and kmalloc, which isn't available at this stage. So we split things up into an early phase and a later opportunistic phase. This commit also removes some noisy log messages that don't add much.
[...]
Applied to powerpc/fixes.
[1/1] powerpc/powernv: wire up rng during setup_arch https://git.kernel.org/powerpc/c/f3eac426657d985b97c92fa5f7ae1d43f04721f3
cheers
linux-stable-mirror@lists.linaro.org