linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v4 00/11] archs/random: fallback to best raw ktime when no cycle counter
@ 2022-04-13 11:54 Jason A. Donenfeld
  2022-04-13 11:54 ` [PATCH v4 01/11] timekeeping: add raw clock fallback for random_get_entropy() Jason A. Donenfeld
                   ` (10 more replies)
  0 siblings, 11 replies; 34+ messages in thread
From: Jason A. Donenfeld @ 2022-04-13 11:54 UTC (permalink / raw)
  To: linux-kernel, linux-crypto, tglx, arnd
  Cc: Jason A. Donenfeld, Theodore Ts'o, Dominik Brodowski,
	Russell King, Catalin Marinas, Will Deacon, Geert Uytterhoeven,
	Thomas Bogendoerfer, Paul Walmsley, Palmer Dabbelt, Albert Ou,
	David S . Miller, Richard Weinberger, Anton Ivanov,
	Johannes Berg, Ingo Molnar, Borislav Petkov, Dave Hansen,
	H . Peter Anvin, Chris Zankel, Max Filippov, John Stultz,
	Stephen Boyd, Dinh Nguyen, linux-arm-kernel, linux-m68k,
	linux-mips, linux-riscv, sparclinux, linux-um, x86, linux-xtensa

Hi folks,

The RNG uses a function called random_get_entropy() basically anytime
that it needs to timestamp an event. For example, an interrupt comes in,
and we mix a random_get_entropy() into the entropy pool somehow.
Somebody mashes their keyboard or moves their mouse around? We mix a
random_get_entropy() into the entropy pool. It's one of the main
varieties of input.

Unfortunately, it's always 0 on a few platforms. The RNG has accumulated
various hacks to deal with this, but in general it's not great. Surely
we can do better than 0. In fact, *anything* that's not the same exact
value all the time would be better than 0. Even a counter that
increments once per hour would be better than 0! I think you get the
idea.

On most platforms, random_get_entropy() is aliased to get_cycles(),
which makes sense for platforms where get_cycles() is defined. RDTSC,
for example, has all the characteristics we care about for this
function: it's fast to acquire (i.e. acceptable in an irq handler),
pretty high precision, available, forms a 2-monotone distribution, etc.
But for platforms without that, what is the next best thing?

Sometimes the next best thing is architecture-defined. For example,
really old MIPS has the CP0 random register, which isn't a cycle
counter, but is at least something. However, some platforms don't even
have an architecture-defined fallback.

Fortunately, the timekeeping subsystem has already solved this problem
of trying to determine what the least bad clock is on constrained
systems, falling back to jiffies in the worst case. By exporting the raw
clock, we can get a decent fallback function for when there's no cycle
counter or architecture-specific function.

This series makes the RNG more useful on: m68k, RISC-V, MIPS, ARM32,
NIOS II, SPARC32, Xtensa, and Usermode Linux. Previously these platforms
would, in certain circumstances, but out of luck with regards to having
any type of event timestamping source in the RNG.

Finally, note that this series isn't about "jitter entropy" or other
ways of initializing the RNG. That's a different topic for a different
thread. Please don't let this discussion veer off into that. Here, I'm
just trying to find a good fallback counter/timer for platforms without
get_cycles(), a question with limited scope.

If this (or a future revision) looks good to you all and receives the
requisite acks, my plan was to take these through the random.git tree
for 5.19, so that I can then build on top of it.

Thanks,
Jason

Changes v3->v4:
- Use EXPORT_SYMBOL_GPL instead of EXPORT_SYMBOL.

Changes v2->v3:
- Name the fallback function random_get_entropy_fallback(), so that it
  can be changed out as needed.
- Include header with prototype in timekeeping.c to avoid compiler
  warning.
- Export fallback function symbol.

Changes v1->v2:
- Use ktime_read_raw_clock() instead of sched_clock(), per Thomas'
  suggestion.
- Drop arm64 change.
- Cleanup header inclusion ordering problem.

Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Arnd Bergmann <arnd@arndb.de>
Cc: Theodore Ts'o <tytso@mit.edu>
Cc: Dominik Brodowski <linux@dominikbrodowski.net>
Cc: Russell King <linux@armlinux.org.uk>
Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: Will Deacon <will@kernel.org>
Cc: Geert Uytterhoeven <geert@linux-m68k.org>
Cc: Thomas Bogendoerfer <tsbogend@alpha.franken.de>
Cc: Paul Walmsley <paul.walmsley@sifive.com>
Cc: Palmer Dabbelt <palmer@dabbelt.com>
Cc: Albert Ou <aou@eecs.berkeley.edu>
Cc: David S. Miller <davem@davemloft.net>
Cc: Richard Weinberger <richard@nod.at>
Cc: Anton Ivanov <anton.ivanov@cambridgegreys.com>
Cc: Johannes Berg <johannes@sipsolutions.net>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: Borislav Petkov <bp@alien8.de>
Cc: Dave Hansen <dave.hansen@linux.intel.com>
Cc: H. Peter Anvin <hpa@zytor.com>
Cc: Chris Zankel <chris@zankel.net>
Cc: Max Filippov <jcmvbkbc@gmail.com>
Cc: John Stultz <john.stultz@linaro.org>
Cc: Stephen Boyd <sboyd@kernel.org>
Cc: Dinh Nguyen <dinguyen@kernel.org>
Cc: linux-arm-kernel@lists.infradead.org
Cc: linux-m68k@lists.linux-m68k.org
Cc: linux-mips@vger.kernel.org
Cc: linux-riscv@lists.infradead.org
Cc: sparclinux@vger.kernel.org
Cc: linux-um@lists.infradead.org
Cc: x86@kernel.org
Cc: linux-xtensa@linux-xtensa.org

Jason A. Donenfeld (11):
  timekeeping: add raw clock fallback for random_get_entropy()
  m68k: use fallback for random_get_entropy() instead of zero
  riscv: use fallback for random_get_entropy() instead of zero
  mips: use fallback for random_get_entropy() instead of zero
  arm: use fallback for random_get_entropy() instead of zero
  nios2: use fallback for random_get_entropy() instead of zero
  x86: use fallback for random_get_entropy() instead of zero
  um: use fallback for random_get_entropy() instead of zero
  sparc: use fallback for random_get_entropy() instead of zero
  xtensa: use fallback for random_get_entropy() instead of zero
  random: insist on random_get_entropy() existing in order to simplify

 arch/arm/include/asm/timex.h      |  1 +
 arch/m68k/include/asm/timex.h     |  2 +-
 arch/mips/include/asm/timex.h     |  2 +-
 arch/nios2/include/asm/timex.h    |  2 +
 arch/riscv/include/asm/timex.h    |  2 +-
 arch/sparc/include/asm/timex_32.h |  4 +-
 arch/um/include/asm/timex.h       |  9 +---
 arch/x86/include/asm/tsc.h        | 10 ++++
 arch/xtensa/include/asm/timex.h   |  6 +--
 drivers/char/random.c             | 87 ++++++++++---------------------
 include/linux/timex.h             |  8 +++
 kernel/time/timekeeping.c         | 10 ++++
 12 files changed, 67 insertions(+), 76 deletions(-)

-- 
2.35.1


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

* [PATCH v4 01/11] timekeeping: add raw clock fallback for random_get_entropy()
  2022-04-13 11:54 [PATCH v4 00/11] archs/random: fallback to best raw ktime when no cycle counter Jason A. Donenfeld
@ 2022-04-13 11:54 ` Jason A. Donenfeld
  2022-04-13 14:32   ` Rob Herring
  2022-04-14 10:12   ` Russell King (Oracle)
  2022-04-13 11:54 ` [PATCH v4 02/11] m68k: use fallback for random_get_entropy() instead of zero Jason A. Donenfeld
                   ` (9 subsequent siblings)
  10 siblings, 2 replies; 34+ messages in thread
From: Jason A. Donenfeld @ 2022-04-13 11:54 UTC (permalink / raw)
  To: linux-kernel, linux-crypto, tglx, arnd
  Cc: Jason A. Donenfeld, Theodore Ts'o, Dominik Brodowski,
	Russell King, Catalin Marinas, Will Deacon, Geert Uytterhoeven,
	Thomas Bogendoerfer, Paul Walmsley, Palmer Dabbelt, Albert Ou,
	David S . Miller, Richard Weinberger, Anton Ivanov,
	Johannes Berg, Ingo Molnar, Borislav Petkov, Dave Hansen,
	H . Peter Anvin, Chris Zankel, Max Filippov, John Stultz,
	Stephen Boyd, Dinh Nguyen, linux-arm-kernel, linux-m68k,
	linux-mips, linux-riscv, sparclinux, linux-um, x86, linux-xtensa

The addition of random_get_entropy_fallback() provides access to
whichever time source has the highest frequency, which is useful for
gathering entropy on platforms without available cycle counters. It's
not necessarily as good as being able to quickly access a cycle counter
that the CPU has, but it's still something, even when it falls back to
being jiffies-based.

In the event that a given arch does not define get_cycles(), falling
back to the get_cycles() default implementation that returns 0 is really
not the best we can do. Instead, at least calling
random_get_entropy_fallback() would be preferable, because that always
needs to return _something_, even falling back to jiffies eventually.
It's not as though random_get_entropy_fallback() is super high precision
or guaranteed to be entropic, but basically anything that's not zero all
the time is better than returning zero all the time.

Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Arnd Bergmann <arnd@arndb.de>
Cc: Theodore Ts'o <tytso@mit.edu>
Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
---
 include/linux/timex.h     |  8 ++++++++
 kernel/time/timekeeping.c | 10 ++++++++++
 2 files changed, 18 insertions(+)

diff --git a/include/linux/timex.h b/include/linux/timex.h
index 5745c90c8800..fbbe34226044 100644
--- a/include/linux/timex.h
+++ b/include/linux/timex.h
@@ -62,6 +62,8 @@
 #include <linux/types.h>
 #include <linux/param.h>
 
+extern unsigned long random_get_entropy_fallback(void);
+
 #include <asm/timex.h>
 
 #ifndef random_get_entropy
@@ -74,8 +76,14 @@
  *
  * By default we use get_cycles() for this purpose, but individual
  * architectures may override this in their asm/timex.h header file.
+ * If a given arch does not have get_cycles(), then we fallback to
+ * using random_get_entropy_fallback().
  */
+#ifdef get_cycles
 #define random_get_entropy()	((unsigned long)get_cycles())
+#else
+#define random_get_entropy()	random_get_entropy_fallback()
+#endif
 #endif
 
 /*
diff --git a/kernel/time/timekeeping.c b/kernel/time/timekeeping.c
index dcdcb85121e4..7cd2ec239cae 100644
--- a/kernel/time/timekeeping.c
+++ b/kernel/time/timekeeping.c
@@ -17,6 +17,7 @@
 #include <linux/clocksource.h>
 #include <linux/jiffies.h>
 #include <linux/time.h>
+#include <linux/timex.h>
 #include <linux/tick.h>
 #include <linux/stop_machine.h>
 #include <linux/pvclock_gtod.h>
@@ -2380,6 +2381,15 @@ static int timekeeping_validate_timex(const struct __kernel_timex *txc)
 	return 0;
 }
 
+/**
+ * random_get_entropy_fallback - Returns the raw clock source value,
+ * used by random.c for platforms with no valid random_get_entropy().
+ */
+unsigned long random_get_entropy_fallback(void)
+{
+	return tk_clock_read(&tk_core.timekeeper.tkr_mono);
+}
+EXPORT_SYMBOL_GPL(random_get_entropy_fallback);
 
 /**
  * do_adjtimex() - Accessor function to NTP __do_adjtimex function
-- 
2.35.1


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

* [PATCH v4 02/11] m68k: use fallback for random_get_entropy() instead of zero
  2022-04-13 11:54 [PATCH v4 00/11] archs/random: fallback to best raw ktime when no cycle counter Jason A. Donenfeld
  2022-04-13 11:54 ` [PATCH v4 01/11] timekeeping: add raw clock fallback for random_get_entropy() Jason A. Donenfeld
@ 2022-04-13 11:54 ` Jason A. Donenfeld
  2022-04-13 11:54 ` [PATCH v4 03/11] riscv: " Jason A. Donenfeld
                   ` (8 subsequent siblings)
  10 siblings, 0 replies; 34+ messages in thread
From: Jason A. Donenfeld @ 2022-04-13 11:54 UTC (permalink / raw)
  To: linux-kernel, linux-crypto, tglx, arnd
  Cc: Jason A. Donenfeld, Theodore Ts'o, Dominik Brodowski,
	Russell King, Catalin Marinas, Will Deacon, Geert Uytterhoeven,
	Thomas Bogendoerfer, Paul Walmsley, Palmer Dabbelt, Albert Ou,
	David S . Miller, Richard Weinberger, Anton Ivanov,
	Johannes Berg, Ingo Molnar, Borislav Petkov, Dave Hansen,
	H . Peter Anvin, Chris Zankel, Max Filippov, John Stultz,
	Stephen Boyd, Dinh Nguyen, linux-arm-kernel, linux-m68k,
	linux-mips, linux-riscv, sparclinux, linux-um, x86, linux-xtensa

In the event that random_get_entropy() can't access a cycle counter or
similar, falling back to returning 0 is really not the best we can do.
Instead, at least calling random_get_entropy_fallback() would be
preferable, because that always needs to return _something_, even
falling back to jiffies eventually. It's not as though
random_get_entropy_fallback() is super high precision or guaranteed to
be entropic, but basically anything that's not zero all the time is
better than returning zero all the time.

Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Arnd Bergmann <arnd@arndb.de>
Acked-by: Geert Uytterhoeven <geert@linux-m68k.org>
Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
---
 arch/m68k/include/asm/timex.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/m68k/include/asm/timex.h b/arch/m68k/include/asm/timex.h
index 6a21d9358280..f4a7a340f4ca 100644
--- a/arch/m68k/include/asm/timex.h
+++ b/arch/m68k/include/asm/timex.h
@@ -35,7 +35,7 @@ static inline unsigned long random_get_entropy(void)
 {
 	if (mach_random_get_entropy)
 		return mach_random_get_entropy();
-	return 0;
+	return random_get_entropy_fallback();
 }
 #define random_get_entropy	random_get_entropy
 
-- 
2.35.1


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

* [PATCH v4 03/11] riscv: use fallback for random_get_entropy() instead of zero
  2022-04-13 11:54 [PATCH v4 00/11] archs/random: fallback to best raw ktime when no cycle counter Jason A. Donenfeld
  2022-04-13 11:54 ` [PATCH v4 01/11] timekeeping: add raw clock fallback for random_get_entropy() Jason A. Donenfeld
  2022-04-13 11:54 ` [PATCH v4 02/11] m68k: use fallback for random_get_entropy() instead of zero Jason A. Donenfeld
@ 2022-04-13 11:54 ` Jason A. Donenfeld
  2022-04-13 14:40   ` Rob Herring
  2022-04-13 11:54 ` [PATCH v4 04/11] mips: " Jason A. Donenfeld
                   ` (7 subsequent siblings)
  10 siblings, 1 reply; 34+ messages in thread
From: Jason A. Donenfeld @ 2022-04-13 11:54 UTC (permalink / raw)
  To: linux-kernel, linux-crypto, tglx, arnd
  Cc: Jason A. Donenfeld, Theodore Ts'o, Dominik Brodowski,
	Russell King, Catalin Marinas, Will Deacon, Geert Uytterhoeven,
	Thomas Bogendoerfer, Paul Walmsley, Palmer Dabbelt, Albert Ou,
	David S . Miller, Richard Weinberger, Anton Ivanov,
	Johannes Berg, Ingo Molnar, Borislav Petkov, Dave Hansen,
	H . Peter Anvin, Chris Zankel, Max Filippov, John Stultz,
	Stephen Boyd, Dinh Nguyen, linux-arm-kernel, linux-m68k,
	linux-mips, linux-riscv, sparclinux, linux-um, x86, linux-xtensa

In the event that random_get_entropy() can't access a cycle counter or
similar, falling back to returning 0 is really not the best we can do.
Instead, at least calling random_get_entropy_fallback() would be
preferable, because that always needs to return _something_, even
falling back to jiffies eventually. It's not as though
random_get_entropy_fallback() is super high precision or guaranteed to
be entropic, but basically anything that's not zero all the time is
better than returning zero all the time.

Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Arnd Bergmann <arnd@arndb.de>
Cc: Paul Walmsley <paul.walmsley@sifive.com>
Cc: Palmer Dabbelt <palmer@dabbelt.com>
Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
---
 arch/riscv/include/asm/timex.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/riscv/include/asm/timex.h b/arch/riscv/include/asm/timex.h
index 507cae273bc6..d6a7428f6248 100644
--- a/arch/riscv/include/asm/timex.h
+++ b/arch/riscv/include/asm/timex.h
@@ -41,7 +41,7 @@ static inline u32 get_cycles_hi(void)
 static inline unsigned long random_get_entropy(void)
 {
 	if (unlikely(clint_time_val == NULL))
-		return 0;
+		return random_get_entropy_fallback();
 	return get_cycles();
 }
 #define random_get_entropy()	random_get_entropy()
-- 
2.35.1


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

* [PATCH v4 04/11] mips: use fallback for random_get_entropy() instead of zero
  2022-04-13 11:54 [PATCH v4 00/11] archs/random: fallback to best raw ktime when no cycle counter Jason A. Donenfeld
                   ` (2 preceding siblings ...)
  2022-04-13 11:54 ` [PATCH v4 03/11] riscv: " Jason A. Donenfeld
@ 2022-04-13 11:54 ` Jason A. Donenfeld
  2022-04-13 12:25   ` Thomas Bogendoerfer
  2022-04-13 11:54 ` [PATCH v4 05/11] arm: " Jason A. Donenfeld
                   ` (6 subsequent siblings)
  10 siblings, 1 reply; 34+ messages in thread
From: Jason A. Donenfeld @ 2022-04-13 11:54 UTC (permalink / raw)
  To: linux-kernel, linux-crypto, tglx, arnd
  Cc: Jason A. Donenfeld, Theodore Ts'o, Dominik Brodowski,
	Russell King, Catalin Marinas, Will Deacon, Geert Uytterhoeven,
	Thomas Bogendoerfer, Paul Walmsley, Palmer Dabbelt, Albert Ou,
	David S . Miller, Richard Weinberger, Anton Ivanov,
	Johannes Berg, Ingo Molnar, Borislav Petkov, Dave Hansen,
	H . Peter Anvin, Chris Zankel, Max Filippov, John Stultz,
	Stephen Boyd, Dinh Nguyen, linux-arm-kernel, linux-m68k,
	linux-mips, linux-riscv, sparclinux, linux-um, x86, linux-xtensa

In the event that random_get_entropy() can't access a cycle counter or
similar, falling back to returning 0 is really not the best we can do.
Instead, at least calling random_get_entropy_fallback() would be
preferable, because that always needs to return _something_, even
falling back to jiffies eventually. It's not as though
random_get_entropy_fallback() is super high precision or guaranteed to
be entropic, but basically anything that's not zero all the time is
better than returning zero all the time.

Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Arnd Bergmann <arnd@arndb.de>
Cc: Thomas Bogendoerfer <tsbogend@alpha.franken.de>
Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
---
 arch/mips/include/asm/timex.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/mips/include/asm/timex.h b/arch/mips/include/asm/timex.h
index b05bb70a2e46..abc60a6395e3 100644
--- a/arch/mips/include/asm/timex.h
+++ b/arch/mips/include/asm/timex.h
@@ -94,7 +94,7 @@ static inline unsigned long random_get_entropy(void)
 	else if (likely(imp != PRID_IMP_R6000 && imp != PRID_IMP_R6000A))
 		return read_c0_random();
 	else
-		return 0;	/* no usable register */
+		return random_get_entropy_fallback();	/* no usable register */
 }
 #define random_get_entropy random_get_entropy
 
-- 
2.35.1


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

* [PATCH v4 05/11] arm: use fallback for random_get_entropy() instead of zero
  2022-04-13 11:54 [PATCH v4 00/11] archs/random: fallback to best raw ktime when no cycle counter Jason A. Donenfeld
                   ` (3 preceding siblings ...)
  2022-04-13 11:54 ` [PATCH v4 04/11] mips: " Jason A. Donenfeld
@ 2022-04-13 11:54 ` Jason A. Donenfeld
  2022-04-13 11:54 ` [PATCH v4 06/11] nios2: " Jason A. Donenfeld
                   ` (5 subsequent siblings)
  10 siblings, 0 replies; 34+ messages in thread
From: Jason A. Donenfeld @ 2022-04-13 11:54 UTC (permalink / raw)
  To: linux-kernel, linux-crypto, tglx, arnd
  Cc: Jason A. Donenfeld, Theodore Ts'o, Dominik Brodowski,
	Russell King, Catalin Marinas, Will Deacon, Geert Uytterhoeven,
	Thomas Bogendoerfer, Paul Walmsley, Palmer Dabbelt, Albert Ou,
	David S . Miller, Richard Weinberger, Anton Ivanov,
	Johannes Berg, Ingo Molnar, Borislav Petkov, Dave Hansen,
	H . Peter Anvin, Chris Zankel, Max Filippov, John Stultz,
	Stephen Boyd, Dinh Nguyen, linux-arm-kernel, linux-m68k,
	linux-mips, linux-riscv, sparclinux, linux-um, x86, linux-xtensa

In the event that random_get_entropy() can't access a cycle counter or
similar, falling back to returning 0 is really not the best we can do.
Instead, at least calling random_get_entropy_fallback() would be
preferable, because that always needs to return _something_, even
falling back to jiffies eventually. It's not as though
random_get_entropy_fallback() is super high precision or guaranteed to
be entropic, but basically anything that's not zero all the time is
better than returning zero all the time.

Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Arnd Bergmann <arnd@arndb.de>
Cc: Russell King <linux@armlinux.org.uk>
Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
---
 arch/arm/include/asm/timex.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm/include/asm/timex.h b/arch/arm/include/asm/timex.h
index 7c3b3671d6c2..6d1337c169cd 100644
--- a/arch/arm/include/asm/timex.h
+++ b/arch/arm/include/asm/timex.h
@@ -11,5 +11,6 @@
 
 typedef unsigned long cycles_t;
 #define get_cycles()	({ cycles_t c; read_current_timer(&c) ? 0 : c; })
+#define random_get_entropy() (((unsigned long)get_cycles()) ?: random_get_entropy_fallback())
 
 #endif
-- 
2.35.1


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

* [PATCH v4 06/11] nios2: use fallback for random_get_entropy() instead of zero
  2022-04-13 11:54 [PATCH v4 00/11] archs/random: fallback to best raw ktime when no cycle counter Jason A. Donenfeld
                   ` (4 preceding siblings ...)
  2022-04-13 11:54 ` [PATCH v4 05/11] arm: " Jason A. Donenfeld
@ 2022-04-13 11:54 ` Jason A. Donenfeld
  2022-05-23 13:58   ` Dinh Nguyen
  2022-04-13 11:54 ` [PATCH v4 07/11] x86: " Jason A. Donenfeld
                   ` (4 subsequent siblings)
  10 siblings, 1 reply; 34+ messages in thread
From: Jason A. Donenfeld @ 2022-04-13 11:54 UTC (permalink / raw)
  To: linux-kernel, linux-crypto, tglx, arnd
  Cc: Jason A. Donenfeld, Theodore Ts'o, Dominik Brodowski,
	Russell King, Catalin Marinas, Will Deacon, Geert Uytterhoeven,
	Thomas Bogendoerfer, Paul Walmsley, Palmer Dabbelt, Albert Ou,
	David S . Miller, Richard Weinberger, Anton Ivanov,
	Johannes Berg, Ingo Molnar, Borislav Petkov, Dave Hansen,
	H . Peter Anvin, Chris Zankel, Max Filippov, John Stultz,
	Stephen Boyd, Dinh Nguyen, linux-arm-kernel, linux-m68k,
	linux-mips, linux-riscv, sparclinux, linux-um, x86, linux-xtensa

In the event that random_get_entropy() can't access a cycle counter or
similar, falling back to returning 0 is really not the best we can do.
Instead, at least calling random_get_entropy_fallback() would be
preferable, because that always needs to return _something_, even
falling back to jiffies eventually. It's not as though
random_get_entropy_fallback() is super high precision or guaranteed to
be entropic, but basically anything that's not zero all the time is
better than returning zero all the time.

Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Arnd Bergmann <arnd@arndb.de>
Cc: Dinh Nguyen <dinguyen@kernel.org>
Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
---
 arch/nios2/include/asm/timex.h | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/arch/nios2/include/asm/timex.h b/arch/nios2/include/asm/timex.h
index a769f871b28d..d9a3f426cdda 100644
--- a/arch/nios2/include/asm/timex.h
+++ b/arch/nios2/include/asm/timex.h
@@ -9,4 +9,6 @@ typedef unsigned long cycles_t;
 
 extern cycles_t get_cycles(void);
 
+#define random_get_entropy() (((unsigned long)get_cycles()) ?: random_get_entropy_fallback())
+
 #endif
-- 
2.35.1


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

* [PATCH v4 07/11] x86: use fallback for random_get_entropy() instead of zero
  2022-04-13 11:54 [PATCH v4 00/11] archs/random: fallback to best raw ktime when no cycle counter Jason A. Donenfeld
                   ` (5 preceding siblings ...)
  2022-04-13 11:54 ` [PATCH v4 06/11] nios2: " Jason A. Donenfeld
@ 2022-04-13 11:54 ` Jason A. Donenfeld
  2022-04-13 11:54 ` [PATCH v4 08/11] um: " Jason A. Donenfeld
                   ` (3 subsequent siblings)
  10 siblings, 0 replies; 34+ messages in thread
From: Jason A. Donenfeld @ 2022-04-13 11:54 UTC (permalink / raw)
  To: linux-kernel, linux-crypto, tglx, arnd
  Cc: Jason A. Donenfeld, Theodore Ts'o, Dominik Brodowski,
	Russell King, Catalin Marinas, Will Deacon, Geert Uytterhoeven,
	Thomas Bogendoerfer, Paul Walmsley, Palmer Dabbelt, Albert Ou,
	David S . Miller, Richard Weinberger, Anton Ivanov,
	Johannes Berg, Ingo Molnar, Borislav Petkov, Dave Hansen,
	H . Peter Anvin, Chris Zankel, Max Filippov, John Stultz,
	Stephen Boyd, Dinh Nguyen, linux-arm-kernel, linux-m68k,
	linux-mips, linux-riscv, sparclinux, linux-um, x86, linux-xtensa

In the event that random_get_entropy() can't access a cycle counter or
similar, falling back to returning 0 is really not the best we can do.
Instead, at least calling random_get_entropy_fallback() would be
preferable, because that always needs to return _something_, even
falling back to jiffies eventually. It's not as though
random_get_entropy_fallback() is super high precision or guaranteed to
be entropic, but basically anything that's not zero all the time is
better than returning zero all the time.

If CONFIG_X86_TSC=n, then it's possible that we're running on a 486 with
no RDTSC, so we only need the fallback code for that case.

Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Arnd Bergmann <arnd@arndb.de>
Cc: x86@kernel.org
Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
---
 arch/x86/include/asm/tsc.h | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/arch/x86/include/asm/tsc.h b/arch/x86/include/asm/tsc.h
index 01a300a9700b..194dc1e3f77c 100644
--- a/arch/x86/include/asm/tsc.h
+++ b/arch/x86/include/asm/tsc.h
@@ -28,6 +28,16 @@ static inline cycles_t get_cycles(void)
 	return rdtsc();
 }
 
+static inline unsigned long random_get_entropy(void)
+{
+#ifndef CONFIG_X86_TSC
+	if (!boot_cpu_has(X86_FEATURE_TSC))
+		return random_get_entropy_fallback();
+#endif
+	return rdtsc();
+}
+#define random_get_entropy random_get_entropy
+
 extern struct system_counterval_t convert_art_to_tsc(u64 art);
 extern struct system_counterval_t convert_art_ns_to_tsc(u64 art_ns);
 
-- 
2.35.1


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

* [PATCH v4 08/11] um: use fallback for random_get_entropy() instead of zero
  2022-04-13 11:54 [PATCH v4 00/11] archs/random: fallback to best raw ktime when no cycle counter Jason A. Donenfeld
                   ` (6 preceding siblings ...)
  2022-04-13 11:54 ` [PATCH v4 07/11] x86: " Jason A. Donenfeld
@ 2022-04-13 11:54 ` Jason A. Donenfeld
  2022-04-13 11:54 ` [PATCH v4 09/11] sparc: " Jason A. Donenfeld
                   ` (2 subsequent siblings)
  10 siblings, 0 replies; 34+ messages in thread
From: Jason A. Donenfeld @ 2022-04-13 11:54 UTC (permalink / raw)
  To: linux-kernel, linux-crypto, tglx, arnd
  Cc: Jason A. Donenfeld, Theodore Ts'o, Dominik Brodowski,
	Russell King, Catalin Marinas, Will Deacon, Geert Uytterhoeven,
	Thomas Bogendoerfer, Paul Walmsley, Palmer Dabbelt, Albert Ou,
	David S . Miller, Richard Weinberger, Anton Ivanov,
	Johannes Berg, Ingo Molnar, Borislav Petkov, Dave Hansen,
	H . Peter Anvin, Chris Zankel, Max Filippov, John Stultz,
	Stephen Boyd, Dinh Nguyen, linux-arm-kernel, linux-m68k,
	linux-mips, linux-riscv, sparclinux, linux-um, x86, linux-xtensa

In the event that random_get_entropy() can't access a cycle counter or
similar, falling back to returning 0 is really not the best we can do.
Instead, at least calling random_get_entropy_fallback() would be
preferable, because that always needs to return _something_, even
falling back to jiffies eventually. It's not as though
random_get_entropy_fallback() is super high precision or guaranteed to
be entropic, but basically anything that's not zero all the time is
better than returning zero all the time.

This is accomplished by just including the asm-generic code like on
other architectures, which means we can get rid of the empty stub
function here.

Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Arnd Bergmann <arnd@arndb.de>
Cc: Richard Weinberger <richard@nod.at>
Cc: Anton Ivanov <anton.ivanov@cambridgegreys.com>
Cc: Johannes Berg <johannes@sipsolutions.net>
Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
---
 arch/um/include/asm/timex.h | 9 ++-------
 1 file changed, 2 insertions(+), 7 deletions(-)

diff --git a/arch/um/include/asm/timex.h b/arch/um/include/asm/timex.h
index e392a9a5bc9b..9f27176adb26 100644
--- a/arch/um/include/asm/timex.h
+++ b/arch/um/include/asm/timex.h
@@ -2,13 +2,8 @@
 #ifndef __UM_TIMEX_H
 #define __UM_TIMEX_H
 
-typedef unsigned long cycles_t;
-
-static inline cycles_t get_cycles (void)
-{
-	return 0;
-}
-
 #define CLOCK_TICK_RATE (HZ)
 
+#include <asm-generic/timex.h>
+
 #endif
-- 
2.35.1


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

* [PATCH v4 09/11] sparc: use fallback for random_get_entropy() instead of zero
  2022-04-13 11:54 [PATCH v4 00/11] archs/random: fallback to best raw ktime when no cycle counter Jason A. Donenfeld
                   ` (7 preceding siblings ...)
  2022-04-13 11:54 ` [PATCH v4 08/11] um: " Jason A. Donenfeld
@ 2022-04-13 11:54 ` Jason A. Donenfeld
  2022-04-13 11:54 ` [PATCH v4 10/11] xtensa: " Jason A. Donenfeld
  2022-04-13 11:54 ` [PATCH v4 11/11] random: insist on random_get_entropy() existing in order to simplify Jason A. Donenfeld
  10 siblings, 0 replies; 34+ messages in thread
From: Jason A. Donenfeld @ 2022-04-13 11:54 UTC (permalink / raw)
  To: linux-kernel, linux-crypto, tglx, arnd
  Cc: Jason A. Donenfeld, Theodore Ts'o, Dominik Brodowski,
	Russell King, Catalin Marinas, Will Deacon, Geert Uytterhoeven,
	Thomas Bogendoerfer, Paul Walmsley, Palmer Dabbelt, Albert Ou,
	David S . Miller, Richard Weinberger, Anton Ivanov,
	Johannes Berg, Ingo Molnar, Borislav Petkov, Dave Hansen,
	H . Peter Anvin, Chris Zankel, Max Filippov, John Stultz,
	Stephen Boyd, Dinh Nguyen, linux-arm-kernel, linux-m68k,
	linux-mips, linux-riscv, sparclinux, linux-um, x86, linux-xtensa

In the event that random_get_entropy() can't access a cycle counter or
similar, falling back to returning 0 is really not the best we can do.
Instead, at least calling random_get_entropy_fallback() would be
preferable, because that always needs to return _something_, even
falling back to jiffies eventually. It's not as though
random_get_entropy_fallback() is super high precision or guaranteed to
be entropic, but basically anything that's not zero all the time is
better than returning zero all the time.

This is accomplished by just including the asm-generic code like on
other architectures, which means we can get rid of the empty stub
function here.

Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Arnd Bergmann <arnd@arndb.de>
Cc: David S. Miller <davem@davemloft.net>
Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
---
 arch/sparc/include/asm/timex_32.h | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/arch/sparc/include/asm/timex_32.h b/arch/sparc/include/asm/timex_32.h
index 542915b46209..f86326a6f89e 100644
--- a/arch/sparc/include/asm/timex_32.h
+++ b/arch/sparc/include/asm/timex_32.h
@@ -9,8 +9,6 @@
 
 #define CLOCK_TICK_RATE	1193180 /* Underlying HZ */
 
-/* XXX Maybe do something better at some point... -DaveM */
-typedef unsigned long cycles_t;
-#define get_cycles()	(0)
+#include <asm-generic/timex.h>
 
 #endif
-- 
2.35.1


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

* [PATCH v4 10/11] xtensa: use fallback for random_get_entropy() instead of zero
  2022-04-13 11:54 [PATCH v4 00/11] archs/random: fallback to best raw ktime when no cycle counter Jason A. Donenfeld
                   ` (8 preceding siblings ...)
  2022-04-13 11:54 ` [PATCH v4 09/11] sparc: " Jason A. Donenfeld
@ 2022-04-13 11:54 ` Jason A. Donenfeld
  2022-04-13 11:54 ` [PATCH v4 11/11] random: insist on random_get_entropy() existing in order to simplify Jason A. Donenfeld
  10 siblings, 0 replies; 34+ messages in thread
From: Jason A. Donenfeld @ 2022-04-13 11:54 UTC (permalink / raw)
  To: linux-kernel, linux-crypto, tglx, arnd
  Cc: Jason A. Donenfeld, Theodore Ts'o, Dominik Brodowski,
	Russell King, Catalin Marinas, Will Deacon, Geert Uytterhoeven,
	Thomas Bogendoerfer, Paul Walmsley, Palmer Dabbelt, Albert Ou,
	David S . Miller, Richard Weinberger, Anton Ivanov,
	Johannes Berg, Ingo Molnar, Borislav Petkov, Dave Hansen,
	H . Peter Anvin, Chris Zankel, Max Filippov, John Stultz,
	Stephen Boyd, Dinh Nguyen, linux-arm-kernel, linux-m68k,
	linux-mips, linux-riscv, sparclinux, linux-um, x86, linux-xtensa

In the event that random_get_entropy() can't access a cycle counter or
similar, falling back to returning 0 is really not the best we can do.
Instead, at least calling random_get_entropy_fallback() would be
preferable, because that always needs to return _something_, even
falling back to jiffies eventually. It's not as though
random_get_entropy_fallback() is super high precision or guaranteed to
be entropic, but basically anything that's not zero all the time is
better than returning zero all the time.

This is accomplished by just including the asm-generic code like on
other architectures, which means we can get rid of the empty stub
function here.

Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Arnd Bergmann <arnd@arndb.de>
Cc: Max Filippov <jcmvbkbc@gmail.com>
Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
---
 arch/xtensa/include/asm/timex.h | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/arch/xtensa/include/asm/timex.h b/arch/xtensa/include/asm/timex.h
index 233ec75e60c6..3f2462f2d027 100644
--- a/arch/xtensa/include/asm/timex.h
+++ b/arch/xtensa/include/asm/timex.h
@@ -29,10 +29,6 @@
 
 extern unsigned long ccount_freq;
 
-typedef unsigned long long cycles_t;
-
-#define get_cycles()	(0)
-
 void local_timer_setup(unsigned cpu);
 
 /*
@@ -59,4 +55,6 @@ static inline void set_linux_timer (unsigned long ccompare)
 	xtensa_set_sr(ccompare, SREG_CCOMPARE + LINUX_TIMER);
 }
 
+#include <asm-generic/timex.h>
+
 #endif	/* _XTENSA_TIMEX_H */
-- 
2.35.1


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

* [PATCH v4 11/11] random: insist on random_get_entropy() existing in order to simplify
  2022-04-13 11:54 [PATCH v4 00/11] archs/random: fallback to best raw ktime when no cycle counter Jason A. Donenfeld
                   ` (9 preceding siblings ...)
  2022-04-13 11:54 ` [PATCH v4 10/11] xtensa: " Jason A. Donenfeld
@ 2022-04-13 11:54 ` Jason A. Donenfeld
  10 siblings, 0 replies; 34+ messages in thread
From: Jason A. Donenfeld @ 2022-04-13 11:54 UTC (permalink / raw)
  To: linux-kernel, linux-crypto, tglx, arnd
  Cc: Jason A. Donenfeld, Theodore Ts'o, Dominik Brodowski,
	Russell King, Catalin Marinas, Will Deacon, Geert Uytterhoeven,
	Thomas Bogendoerfer, Paul Walmsley, Palmer Dabbelt, Albert Ou,
	David S . Miller, Richard Weinberger, Anton Ivanov,
	Johannes Berg, Ingo Molnar, Borislav Petkov, Dave Hansen,
	H . Peter Anvin, Chris Zankel, Max Filippov, John Stultz,
	Stephen Boyd, Dinh Nguyen, linux-arm-kernel, linux-m68k,
	linux-mips, linux-riscv, sparclinux, linux-um, x86, linux-xtensa

All platforms are now guaranteed to provide some value for
random_get_entropy(). In case some bug leads to this not being so, we
print a warning, because that indicates that something is really very
wrong (and likely other things are impacted too). This should never be
hit, but it's a good and cheap way of finding out if something ever is
problematic.

Since we now have viable fallback code for random_get_entropy() on all
platforms, which is, in the worst case, not worse than jiffies, we can
count on getting the best possible value out of it. That means there's
no longer a use for using jiffies as entropy input. It also means we no
longer have a reason for doing the round-robin register flow in the IRQ
handler, which was always of fairly dubious value.

Instead we can greatly simplify the IRQ handler inputs and also unify
the construction between 64-bits and 32-bits. We now collect the cycle
counter and the return address, since those are the two things that
matter. Because the return address and the irq number are likely
related, to the extent we mix in the irq number, we can just xor it into
the top unchanging bytes of the return address, rather than the bottom
changing bytes of the cycle counter as before. Then, we can do a fixed 2
rounds of SipHash/HSipHash. Finally, we use the same construction of
hashing only half of the [H]SipHash state on 32-bit and 64-bit. We're
not actually discarding any entropy, since that entropy is carried
through until the next time. And more importantly, it lets us do the
same sponge-like construction everywhere.

Cc: Theodore Ts'o <tytso@mit.edu>
Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
---
 drivers/char/random.c | 87 ++++++++++++++-----------------------------
 1 file changed, 28 insertions(+), 59 deletions(-)

diff --git a/drivers/char/random.c b/drivers/char/random.c
index 6b01b2be9dd4..8931b3fac1d3 100644
--- a/drivers/char/random.c
+++ b/drivers/char/random.c
@@ -1005,6 +1005,9 @@ int __init rand_initialize(void)
 		urandom_warning.interval = 0;
 		unseeded_warning.interval = 0;
 	}
+
+	WARN(!random_get_entropy(), "Missing cycle counter and fallback timer; RNG "
+				    "entropy collection will consequently suffer.");
 	return 0;
 }
 
@@ -1018,15 +1021,14 @@ int __init rand_initialize(void)
  */
 void add_device_randomness(const void *buf, size_t size)
 {
-	unsigned long cycles = random_get_entropy();
-	unsigned long flags, now = jiffies;
+	unsigned long entropy = random_get_entropy();
+	unsigned long flags;
 
 	if (crng_init == 0 && size)
 		crng_pre_init_inject(buf, size, false);
 
 	spin_lock_irqsave(&input_pool.lock, flags);
-	_mix_pool_bytes(&cycles, sizeof(cycles));
-	_mix_pool_bytes(&now, sizeof(now));
+	_mix_pool_bytes(&entropy, sizeof(entropy));
 	_mix_pool_bytes(buf, size);
 	spin_unlock_irqrestore(&input_pool.lock, flags);
 }
@@ -1049,12 +1051,11 @@ struct timer_rand_state {
  */
 static void add_timer_randomness(struct timer_rand_state *state, unsigned int num)
 {
-	unsigned long cycles = random_get_entropy(), now = jiffies, flags;
+	unsigned long entropy = random_get_entropy(), now = jiffies, flags;
 	long delta, delta2, delta3;
 
 	spin_lock_irqsave(&input_pool.lock, flags);
-	_mix_pool_bytes(&cycles, sizeof(cycles));
-	_mix_pool_bytes(&now, sizeof(now));
+	_mix_pool_bytes(&entropy, sizeof(entropy));
 	_mix_pool_bytes(&num, sizeof(num));
 	spin_unlock_irqrestore(&input_pool.lock, flags);
 
@@ -1216,7 +1217,6 @@ struct fast_pool {
 	unsigned long pool[4];
 	unsigned long last;
 	unsigned int count;
-	u16 reg_idx;
 };
 
 static DEFINE_PER_CPU(struct fast_pool, irq_randomness) = {
@@ -1236,11 +1236,11 @@ static DEFINE_PER_CPU(struct fast_pool, irq_randomness) = {
  * and therefore this has no security on its own. s represents the
  * 128 or 256-bit SipHash state, while v represents a 128-bit input.
  */
-static void fast_mix(unsigned long s[4], const unsigned long *v)
+static void fast_mix(unsigned long s[4], const unsigned long v[2])
 {
 	size_t i;
 
-	for (i = 0; i < 16 / sizeof(long); ++i) {
+	for (i = 0; i < 2; ++i) {
 		s[3] ^= v[i];
 #ifdef CONFIG_64BIT
 		s[0] += s[1]; s[1] = rol64(s[1], 13); s[1] ^= s[0]; s[0] = rol64(s[0], 32);
@@ -1280,33 +1280,17 @@ int random_online_cpu(unsigned int cpu)
 }
 #endif
 
-static unsigned long get_reg(struct fast_pool *f, struct pt_regs *regs)
-{
-	unsigned long *ptr = (unsigned long *)regs;
-	unsigned int idx;
-
-	if (regs == NULL)
-		return 0;
-	idx = READ_ONCE(f->reg_idx);
-	if (idx >= sizeof(struct pt_regs) / sizeof(unsigned long))
-		idx = 0;
-	ptr += idx++;
-	WRITE_ONCE(f->reg_idx, idx);
-	return *ptr;
-}
-
 static void mix_interrupt_randomness(struct work_struct *work)
 {
 	struct fast_pool *fast_pool = container_of(work, struct fast_pool, mix);
 	/*
-	 * The size of the copied stack pool is explicitly 16 bytes so that we
-	 * tax mix_pool_byte()'s compression function the same amount on all
-	 * platforms. This means on 64-bit we copy half the pool into this,
-	 * while on 32-bit we copy all of it. The entropy is supposed to be
-	 * sufficiently dispersed between bits that in the sponge-like
-	 * half case, on average we don't wind up "losing" some.
+	 * The size of the copied stack pool is explicitly 2 longs so that we
+	 * only ever ingest half of the siphash output each time, retaining
+	 * the other half as the next "key" that carries over. The entropy is
+	 * supposed to be sufficiently dispersed between bits so on average
+	 * we don't wind up "losing" some.
 	 */
-	u8 pool[16];
+	unsigned long pool[2];
 
 	/* Check to see if we're running on the wrong CPU due to hotplug. */
 	local_irq_disable();
@@ -1338,36 +1322,21 @@ static void mix_interrupt_randomness(struct work_struct *work)
 void add_interrupt_randomness(int irq)
 {
 	enum { MIX_INFLIGHT = 1U << 31 };
-	unsigned long cycles = random_get_entropy(), now = jiffies;
+	unsigned long cycles = random_get_entropy();
 	struct fast_pool *fast_pool = this_cpu_ptr(&irq_randomness);
 	struct pt_regs *regs = get_irq_regs();
 	unsigned int new_count;
-	union {
-		u32 u32[4];
-		u64 u64[2];
-		unsigned long longs[16 / sizeof(long)];
-	} irq_data;
-
-	if (cycles == 0)
-		cycles = get_reg(fast_pool, regs);
-
-	if (sizeof(unsigned long) == 8) {
-		irq_data.u64[0] = cycles ^ rol64(now, 32) ^ irq;
-		irq_data.u64[1] = regs ? instruction_pointer(regs) : _RET_IP_;
-	} else {
-		irq_data.u32[0] = cycles ^ irq;
-		irq_data.u32[1] = now;
-		irq_data.u32[2] = regs ? instruction_pointer(regs) : _RET_IP_;
-		irq_data.u32[3] = get_reg(fast_pool, regs);
-	}
 
-	fast_mix(fast_pool->pool, irq_data.longs);
+	fast_mix(fast_pool->pool, (unsigned long[2]){
+		(regs ? instruction_pointer(regs) : _RET_IP_) ^ swab(irq),
+		cycles
+	});
 	new_count = ++fast_pool->count;
 
 	if (new_count & MIX_INFLIGHT)
 		return;
 
-	if (new_count < 64 && (!time_after(now, fast_pool->last + HZ) ||
+	if (new_count < 64 && (!time_is_before_jiffies(fast_pool->last + HZ) ||
 			       unlikely(crng_init == 0)))
 		return;
 
@@ -1403,28 +1372,28 @@ static void entropy_timer(struct timer_list *t)
 static void try_to_generate_entropy(void)
 {
 	struct {
-		unsigned long cycles;
+		unsigned long entropy;
 		struct timer_list timer;
 	} stack;
 
-	stack.cycles = random_get_entropy();
+	stack.entropy = random_get_entropy();
 
 	/* Slow counter - or none. Don't even bother */
-	if (stack.cycles == random_get_entropy())
+	if (stack.entropy == random_get_entropy())
 		return;
 
 	timer_setup_on_stack(&stack.timer, entropy_timer, 0);
 	while (!crng_ready() && !signal_pending(current)) {
 		if (!timer_pending(&stack.timer))
 			mod_timer(&stack.timer, jiffies + 1);
-		mix_pool_bytes(&stack.cycles, sizeof(stack.cycles));
+		mix_pool_bytes(&stack.entropy, sizeof(stack.entropy));
 		schedule();
-		stack.cycles = random_get_entropy();
+		stack.entropy = random_get_entropy();
 	}
 
 	del_timer_sync(&stack.timer);
 	destroy_timer_on_stack(&stack.timer);
-	mix_pool_bytes(&stack.cycles, sizeof(stack.cycles));
+	mix_pool_bytes(&stack.entropy, sizeof(stack.entropy));
 }
 
 
-- 
2.35.1


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

* Re: [PATCH v4 04/11] mips: use fallback for random_get_entropy() instead of zero
  2022-04-13 11:54 ` [PATCH v4 04/11] mips: " Jason A. Donenfeld
@ 2022-04-13 12:25   ` Thomas Bogendoerfer
  2022-04-13 12:46     ` Maciej W. Rozycki
  0 siblings, 1 reply; 34+ messages in thread
From: Thomas Bogendoerfer @ 2022-04-13 12:25 UTC (permalink / raw)
  To: Jason A. Donenfeld
  Cc: linux-kernel, linux-crypto, tglx, arnd, Theodore Ts'o,
	Dominik Brodowski, Russell King, Catalin Marinas, Will Deacon,
	Geert Uytterhoeven, Paul Walmsley, Palmer Dabbelt, Albert Ou,
	David S . Miller, Richard Weinberger, Anton Ivanov,
	Johannes Berg, Ingo Molnar, Borislav Petkov, Dave Hansen,
	H . Peter Anvin, Chris Zankel, Max Filippov, John Stultz,
	Stephen Boyd, Dinh Nguyen, linux-arm-kernel, linux-m68k,
	linux-mips, linux-riscv, sparclinux, linux-um, x86, linux-xtensa

On Wed, Apr 13, 2022 at 01:54:04PM +0200, Jason A. Donenfeld wrote:
> In the event that random_get_entropy() can't access a cycle counter or
> similar, falling back to returning 0 is really not the best we can do.
> Instead, at least calling random_get_entropy_fallback() would be
> preferable, because that always needs to return _something_, even
> falling back to jiffies eventually. It's not as though
> random_get_entropy_fallback() is super high precision or guaranteed to
> be entropic, but basically anything that's not zero all the time is
> better than returning zero all the time.
> 
> Cc: Thomas Gleixner <tglx@linutronix.de>
> Cc: Arnd Bergmann <arnd@arndb.de>
> Cc: Thomas Bogendoerfer <tsbogend@alpha.franken.de>
> Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
> ---
>  arch/mips/include/asm/timex.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/arch/mips/include/asm/timex.h b/arch/mips/include/asm/timex.h
> index b05bb70a2e46..abc60a6395e3 100644
> --- a/arch/mips/include/asm/timex.h
> +++ b/arch/mips/include/asm/timex.h
> @@ -94,7 +94,7 @@ static inline unsigned long random_get_entropy(void)
>  	else if (likely(imp != PRID_IMP_R6000 && imp != PRID_IMP_R6000A))
>  		return read_c0_random();
>  	else
> -		return 0;	/* no usable register */
> +		return random_get_entropy_fallback();	/* no usable register */
>  }
>  #define random_get_entropy random_get_entropy
>  
> -- 
> 2.35.1

Acked-by: Thomas Bogendoerfer <tsbogend@alpha.franken.de>

-- 
Crap can work. Given enough thrust pigs will fly, but it's not necessarily a
good idea.                                                [ RFC1925, 2.3 ]

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

* Re: [PATCH v4 04/11] mips: use fallback for random_get_entropy() instead of zero
  2022-04-13 12:25   ` Thomas Bogendoerfer
@ 2022-04-13 12:46     ` Maciej W. Rozycki
  2022-04-13 22:35       ` Jason A. Donenfeld
  0 siblings, 1 reply; 34+ messages in thread
From: Maciej W. Rozycki @ 2022-04-13 12:46 UTC (permalink / raw)
  To: Thomas Bogendoerfer
  Cc: Jason A. Donenfeld, linux-kernel, linux-crypto, tglx, arnd,
	Theodore Ts'o, Dominik Brodowski, Russell King,
	Catalin Marinas, Will Deacon, Geert Uytterhoeven, Paul Walmsley,
	Palmer Dabbelt, Albert Ou, David S . Miller, Richard Weinberger,
	Anton Ivanov, Johannes Berg, Ingo Molnar, Borislav Petkov,
	Dave Hansen, H . Peter Anvin, Chris Zankel, Max Filippov,
	John Stultz, Stephen Boyd, Dinh Nguyen, linux-arm-kernel,
	linux-m68k, linux-mips, linux-riscv, sparclinux, linux-um, x86,
	linux-xtensa

On Wed, 13 Apr 2022, Thomas Bogendoerfer wrote:

> > diff --git a/arch/mips/include/asm/timex.h b/arch/mips/include/asm/timex.h
> > index b05bb70a2e46..abc60a6395e3 100644
> > --- a/arch/mips/include/asm/timex.h
> > +++ b/arch/mips/include/asm/timex.h
> > @@ -94,7 +94,7 @@ static inline unsigned long random_get_entropy(void)
> >  	else if (likely(imp != PRID_IMP_R6000 && imp != PRID_IMP_R6000A))
> >  		return read_c0_random();
> >  	else
> > -		return 0;	/* no usable register */
> > +		return random_get_entropy_fallback();	/* no usable register */
> >  }
> >  #define random_get_entropy random_get_entropy
> >  
> > -- 
> > 2.35.1
> 
> Acked-by: Thomas Bogendoerfer <tsbogend@alpha.franken.de>

 Or we could drop the PRID_IMP_R6000/A check and the final `else' clause 
entirely, as we don't even pretend to support the R6k at all anymore, and 
this is the final reference remaining.  For one we no longer handle the 
CPU in `cpu_probe_legacy' so any attempt to boot on such a CPU would 
inevitably fail as no CPU options would be set (we probably should have a 
`panic' or suchlike as the default case for the switch statement there).

 Therefore I'm all for removing this piece instead, complementing commit 
3b2db173f012 ("MIPS: Remove unused R6000 support"), where it should have 
really happened.

  Maciej

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

* Re: [PATCH v4 01/11] timekeeping: add raw clock fallback for random_get_entropy()
  2022-04-13 11:54 ` [PATCH v4 01/11] timekeeping: add raw clock fallback for random_get_entropy() Jason A. Donenfeld
@ 2022-04-13 14:32   ` Rob Herring
  2022-04-13 22:38     ` Jason A. Donenfeld
  2022-04-14 10:12   ` Russell King (Oracle)
  1 sibling, 1 reply; 34+ messages in thread
From: Rob Herring @ 2022-04-13 14:32 UTC (permalink / raw)
  To: Jason A. Donenfeld
  Cc: linux-kernel, open list:HARDWARE RANDOM NUMBER GENERATOR CORE,
	Thomas Gleixner, Arnd Bergmann, Theodore Ts'o,
	Dominik Brodowski, Russell King, Catalin Marinas, Will Deacon,
	Geert Uytterhoeven, Thomas Bogendoerfer, Paul Walmsley,
	Palmer Dabbelt, Albert Ou, David S . Miller, Richard Weinberger,
	Anton Ivanov, Johannes Berg, Ingo Molnar, Borislav Petkov,
	Dave Hansen, H . Peter Anvin, Chris Zankel, Max Filippov,
	John Stultz, Stephen Boyd, Dinh Nguyen, linux-arm-kernel,
	linux-m68k, open list:MIPS, linux-riscv, sparclinux, linux-um,
	X86 ML, linux-xtensa

On Wed, Apr 13, 2022 at 6:55 AM Jason A. Donenfeld <Jason@zx2c4.com> wrote:
>
> The addition of random_get_entropy_fallback() provides access to
> whichever time source has the highest frequency, which is useful for
> gathering entropy on platforms without available cycle counters. It's
> not necessarily as good as being able to quickly access a cycle counter
> that the CPU has, but it's still something, even when it falls back to
> being jiffies-based.
>
> In the event that a given arch does not define get_cycles(), falling
> back to the get_cycles() default implementation that returns 0 is really
> not the best we can do. Instead, at least calling
> random_get_entropy_fallback() would be preferable, because that always
> needs to return _something_, even falling back to jiffies eventually.
> It's not as though random_get_entropy_fallback() is super high precision
> or guaranteed to be entropic, but basically anything that's not zero all
> the time is better than returning zero all the time.
>
> Cc: Thomas Gleixner <tglx@linutronix.de>
> Cc: Arnd Bergmann <arnd@arndb.de>
> Cc: Theodore Ts'o <tytso@mit.edu>
> Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
> ---
>  include/linux/timex.h     |  8 ++++++++
>  kernel/time/timekeeping.c | 10 ++++++++++
>  2 files changed, 18 insertions(+)
>
> diff --git a/include/linux/timex.h b/include/linux/timex.h
> index 5745c90c8800..fbbe34226044 100644
> --- a/include/linux/timex.h
> +++ b/include/linux/timex.h
> @@ -62,6 +62,8 @@
>  #include <linux/types.h>
>  #include <linux/param.h>
>
> +extern unsigned long random_get_entropy_fallback(void);
> +
>  #include <asm/timex.h>
>
>  #ifndef random_get_entropy
> @@ -74,8 +76,14 @@
>   *
>   * By default we use get_cycles() for this purpose, but individual
>   * architectures may override this in their asm/timex.h header file.
> + * If a given arch does not have get_cycles(), then we fallback to

'does not have a usable get_cycles(), ...' as clearly some arches have
get_cycles() and yet still need a fallback.

Why not handle the 'if get_cycles() returns 0 do the fallback' within
a weak random_get_entropy() function? Then more arches don't need any
random_get_entropy() implementation.

Rob

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

* Re: [PATCH v4 03/11] riscv: use fallback for random_get_entropy() instead of zero
  2022-04-13 11:54 ` [PATCH v4 03/11] riscv: " Jason A. Donenfeld
@ 2022-04-13 14:40   ` Rob Herring
  2022-04-13 22:40     ` Jason A. Donenfeld
  0 siblings, 1 reply; 34+ messages in thread
From: Rob Herring @ 2022-04-13 14:40 UTC (permalink / raw)
  To: Jason A. Donenfeld
  Cc: linux-kernel, open list:HARDWARE RANDOM NUMBER GENERATOR CORE,
	Thomas Gleixner, Arnd Bergmann, Theodore Ts'o,
	Dominik Brodowski, Russell King, Catalin Marinas, Will Deacon,
	Geert Uytterhoeven, Thomas Bogendoerfer, Paul Walmsley,
	Palmer Dabbelt, Albert Ou, David S . Miller, Richard Weinberger,
	Anton Ivanov, Johannes Berg, Ingo Molnar, Borislav Petkov,
	Dave Hansen, H . Peter Anvin, Chris Zankel, Max Filippov,
	John Stultz, Stephen Boyd, Dinh Nguyen, linux-arm-kernel,
	linux-m68k, open list:MIPS, linux-riscv, sparclinux, linux-um,
	X86 ML, linux-xtensa

On Wed, Apr 13, 2022 at 6:56 AM Jason A. Donenfeld <Jason@zx2c4.com> wrote:
>
> In the event that random_get_entropy() can't access a cycle counter or
> similar, falling back to returning 0 is really not the best we can do.
> Instead, at least calling random_get_entropy_fallback() would be
> preferable, because that always needs to return _something_, even
> falling back to jiffies eventually. It's not as though
> random_get_entropy_fallback() is super high precision or guaranteed to
> be entropic, but basically anything that's not zero all the time is
> better than returning zero all the time.
>
> Cc: Thomas Gleixner <tglx@linutronix.de>
> Cc: Arnd Bergmann <arnd@arndb.de>
> Cc: Paul Walmsley <paul.walmsley@sifive.com>
> Cc: Palmer Dabbelt <palmer@dabbelt.com>
> Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
> ---
>  arch/riscv/include/asm/timex.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/arch/riscv/include/asm/timex.h b/arch/riscv/include/asm/timex.h
> index 507cae273bc6..d6a7428f6248 100644
> --- a/arch/riscv/include/asm/timex.h
> +++ b/arch/riscv/include/asm/timex.h
> @@ -41,7 +41,7 @@ static inline u32 get_cycles_hi(void)
>  static inline unsigned long random_get_entropy(void)
>  {
>         if (unlikely(clint_time_val == NULL))

Moving this check to get_cycles() implementation would eliminate the
RiscV implementation of random_get_entropy() if you follow my other
suggestion.

I guess there's some advantage to skipping a NULL check every time for
get_cycles(), but really the register read time will be much slower
than an added check.

> -               return 0;
> +               return random_get_entropy_fallback();
>         return get_cycles();
>  }
>  #define random_get_entropy()   random_get_entropy()

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

* Re: [PATCH v4 04/11] mips: use fallback for random_get_entropy() instead of zero
  2022-04-13 12:46     ` Maciej W. Rozycki
@ 2022-04-13 22:35       ` Jason A. Donenfeld
  2022-04-14  1:16         ` Maciej W. Rozycki
  0 siblings, 1 reply; 34+ messages in thread
From: Jason A. Donenfeld @ 2022-04-13 22:35 UTC (permalink / raw)
  To: Maciej W. Rozycki
  Cc: Thomas Bogendoerfer, LKML, Linux Crypto Mailing List,
	Thomas Gleixner, Arnd Bergmann, Theodore Ts'o,
	Dominik Brodowski, Russell King, Catalin Marinas, Will Deacon,
	Geert Uytterhoeven, Paul Walmsley, Palmer Dabbelt, Albert Ou,
	David S . Miller, Richard Weinberger, Anton Ivanov,
	Johannes Berg, Ingo Molnar, Borislav Petkov, Dave Hansen,
	H . Peter Anvin, Chris Zankel, Max Filippov, John Stultz,
	Stephen Boyd, Dinh Nguyen, linux-arm-kernel, linux-m68k,
	open list:BROADCOM NVRAM DRIVER, linux-riscv, sparclinux,
	linux-um, X86 ML, linux-xtensa

Hi Maciej,

On Wed, Apr 13, 2022 at 2:46 PM Maciej W. Rozycki <macro@orcam.me.uk> wrote:
>
> On Wed, 13 Apr 2022, Thomas Bogendoerfer wrote:
>
> > > diff --git a/arch/mips/include/asm/timex.h b/arch/mips/include/asm/timex.h
> > > index b05bb70a2e46..abc60a6395e3 100644
> > > --- a/arch/mips/include/asm/timex.h
> > > +++ b/arch/mips/include/asm/timex.h
> > > @@ -94,7 +94,7 @@ static inline unsigned long random_get_entropy(void)
> > >     else if (likely(imp != PRID_IMP_R6000 && imp != PRID_IMP_R6000A))
> > >             return read_c0_random();
> > >     else
> > > -           return 0;       /* no usable register */
> > > +           return random_get_entropy_fallback();   /* no usable register */
> > >  }
> > >  #define random_get_entropy random_get_entropy
> > >
> > > --
> > > 2.35.1
> >
> > Acked-by: Thomas Bogendoerfer <tsbogend@alpha.franken.de>
>
>  Or we could drop the PRID_IMP_R6000/A check and the final `else' clause
> entirely, as we don't even pretend to support the R6k at all anymore, and
> this is the final reference remaining.  For one we no longer handle the
> CPU in `cpu_probe_legacy' so any attempt to boot on such a CPU would
> inevitably fail as no CPU options would be set (we probably should have a
> `panic' or suchlike as the default case for the switch statement there).
>
>  Therefore I'm all for removing this piece instead, complementing commit
> 3b2db173f012 ("MIPS: Remove unused R6000 support"), where it should have
> really happened.

That's fine with me, if that's what Thomas wants to do, and I can
submit a v5 with that in it. Indeed, from our previous conversations,
I'm aware that the `else` there is probably never hit.

However, one thing that I've been thinking about is that the c0 random
register is actually kind of garbage. In my fuzzy decade-old memory of
MIPS, I believe the c0 random register starts at the maximum number of
TLB entries (16?), and then counts down cyclically, decrementing once
per CPU cycle. Is that right?

If it is, there are some real pros and cons here to consider:
- Pro: decrementing each CPU cycle means pretty good granularity
- Con: wrapping at, like, 16 or something really is very limited, to
the point of being almost bad

Meanwhile, on systems without the c0 cycles counter, what is the
actual resolution of random_get_entropy_fallback()? Is this just
falling back to jiffies?

IF (a) the fallback is jiffies AND (b) c0 wraps at 16, then actually,
what would be really nice would be something like:

    return (jiffies << 4) | read_c0_random();

It seems like that would give us something somewhat more ideal than
the status quo. Still crap, of course, but undoubtedly better.

Unfortunately, I don't know enough about whether assumptions (a) and
(b) hold for all hardware that doesn't have the c0 cycle counter. Can
you or Thomas confirm/deny this? And if it is more nuanced than my
optimistic assumption above, can we think of some scheme together that
nicely combines jiffies or the fallback function with the c0 random
register that would be sensible?

Jason

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

* Re: [PATCH v4 01/11] timekeeping: add raw clock fallback for random_get_entropy()
  2022-04-13 14:32   ` Rob Herring
@ 2022-04-13 22:38     ` Jason A. Donenfeld
  2022-04-14 20:41       ` Rob Herring
  0 siblings, 1 reply; 34+ messages in thread
From: Jason A. Donenfeld @ 2022-04-13 22:38 UTC (permalink / raw)
  To: Rob Herring
  Cc: linux-kernel, open list:HARDWARE RANDOM NUMBER GENERATOR CORE,
	Thomas Gleixner, Arnd Bergmann, Theodore Ts'o,
	Dominik Brodowski, Russell King, Catalin Marinas, Will Deacon,
	Geert Uytterhoeven, Thomas Bogendoerfer, Paul Walmsley,
	Palmer Dabbelt, Albert Ou, David S . Miller, Richard Weinberger,
	Anton Ivanov, Johannes Berg, Ingo Molnar, Borislav Petkov,
	Dave Hansen, H . Peter Anvin, Chris Zankel, Max Filippov,
	John Stultz, Stephen Boyd, Dinh Nguyen, linux-arm-kernel,
	linux-m68k, open list:MIPS, linux-riscv, sparclinux, linux-um,
	X86 ML, linux-xtensa

Hi Rob,

On Wed, Apr 13, 2022 at 4:32 PM Rob Herring <robh@kernel.org> wrote:
> 'does not have a usable get_cycles(), ...' as clearly some arches have
> get_cycles() and yet still need a fallback.
>
> Why not handle the 'if get_cycles() returns 0 do the fallback' within
> a weak random_get_entropy() function? Then more arches don't need any
> random_get_entropy() implementation.

No, this doesn't really work. Actually, most archs don't need a
random_get_entropy() function, because it exists in asm-generic doing
the thing we want. So that's taken care of. But weak functions as you
suggested would be quite suboptimal, because on, e.g. x86, what we
have now gets inlined into a single rdtsc instruction. Also, the
relation between get_cycles() and random_get_entropy() doesn't always
hold; some archs may not have a working get_cycles() function but do
have a path for a random_get_entropy(). Etc, etc. So I'm pretty sure
that this commit is really the most simple and optimal thing to do. I
really don't want to go the weak functions route.

Jason

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

* Re: [PATCH v4 03/11] riscv: use fallback for random_get_entropy() instead of zero
  2022-04-13 14:40   ` Rob Herring
@ 2022-04-13 22:40     ` Jason A. Donenfeld
  0 siblings, 0 replies; 34+ messages in thread
From: Jason A. Donenfeld @ 2022-04-13 22:40 UTC (permalink / raw)
  To: Rob Herring
  Cc: linux-kernel, open list:HARDWARE RANDOM NUMBER GENERATOR CORE,
	Thomas Gleixner, Arnd Bergmann, Theodore Ts'o,
	Dominik Brodowski, Russell King, Catalin Marinas, Will Deacon,
	Geert Uytterhoeven, Thomas Bogendoerfer, Paul Walmsley,
	Palmer Dabbelt, Albert Ou, David S . Miller, Richard Weinberger,
	Anton Ivanov, Johannes Berg, Ingo Molnar, Borislav Petkov,
	Dave Hansen, H . Peter Anvin, Chris Zankel, Max Filippov,
	John Stultz, Stephen Boyd, Dinh Nguyen, linux-arm-kernel,
	linux-m68k, open list:MIPS, linux-riscv, sparclinux, linux-um,
	X86 ML, linux-xtensa

Hi Rob,

On Wed, Apr 13, 2022 at 4:40 PM Rob Herring <robh@kernel.org> wrote:
> Moving this check to get_cycles() implementation would eliminate the
> RiscV implementation of random_get_entropy() if you follow my other
> suggestion.

Not an option. random_get_entropy() != get_cycles(). Sometimes these
are different functions. Returning random_get_entropy_fallback() from
get_cycles(), for example, wouldn't make any sense.

Jason

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

* Re: [PATCH v4 04/11] mips: use fallback for random_get_entropy() instead of zero
  2022-04-13 22:35       ` Jason A. Donenfeld
@ 2022-04-14  1:16         ` Maciej W. Rozycki
  2022-04-14  9:27           ` Jason A. Donenfeld
  0 siblings, 1 reply; 34+ messages in thread
From: Maciej W. Rozycki @ 2022-04-14  1:16 UTC (permalink / raw)
  To: Jason A. Donenfeld
  Cc: Thomas Bogendoerfer, LKML, Linux Crypto Mailing List,
	Thomas Gleixner, Arnd Bergmann, Theodore Ts'o,
	Dominik Brodowski, Russell King, Catalin Marinas, Will Deacon,
	Geert Uytterhoeven, Paul Walmsley, Palmer Dabbelt, Albert Ou,
	David S . Miller, Richard Weinberger, Anton Ivanov,
	Johannes Berg, Ingo Molnar, Borislav Petkov, Dave Hansen,
	H . Peter Anvin, Chris Zankel, Max Filippov, John Stultz,
	Stephen Boyd, Dinh Nguyen, linux-arm-kernel, linux-m68k,
	open list:BROADCOM NVRAM DRIVER, linux-riscv, sparclinux,
	linux-um, X86 ML, linux-xtensa

Hi Jason,

> However, one thing that I've been thinking about is that the c0 random
> register is actually kind of garbage. In my fuzzy decade-old memory of
> MIPS, I believe the c0 random register starts at the maximum number of
> TLB entries (16?), and then counts down cyclically, decrementing once
> per CPU cycle. Is that right?

 Yes, for the relevant CPUs the range is 63-8 << 8 for R3k machines and 
47-0 (the lower bound can be higher if wired entries are used, which I 
think we occasionally do) for R4k machines with a buggy CP0 counter.  So 
there are either 56 or up to 48 distinct CP0 Random register values.

> If it is, there are some real pros and cons here to consider:
> - Pro: decrementing each CPU cycle means pretty good granularity
> - Con: wrapping at, like, 16 or something really is very limited, to
> the point of being almost bad
> 
> Meanwhile, on systems without the c0 cycles counter, what is the
> actual resolution of random_get_entropy_fallback()? Is this just
> falling back to jiffies?

 It depends on the exact system.  Some have a 32-bit high-resolution 
counter in the chipset (arch/mips/kernel/csrc-ioasic.c) giving like 25MHz 
resolution, some have nothing but jiffies.

> IF (a) the fallback is jiffies AND (b) c0 wraps at 16, then actually,
> what would be really nice would be something like:
> 
>     return (jiffies << 4) | read_c0_random();
> 
> It seems like that would give us something somewhat more ideal than
> the status quo. Still crap, of course, but undoubtedly better.

 It seems like a reasonable idea to me, but the details would have to be 
sorted out, because where a chipset high-resolution counter is available 
we want to factor it in, and otherwise we need to extract the right bits 
from the CP0 Random register, either 13:8 for the R3k or 5:0 for the R4k.

  Maciej

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

* Re: [PATCH v4 04/11] mips: use fallback for random_get_entropy() instead of zero
  2022-04-14  1:16         ` Maciej W. Rozycki
@ 2022-04-14  9:27           ` Jason A. Donenfeld
  2022-04-15 12:26             ` Maciej W. Rozycki
  0 siblings, 1 reply; 34+ messages in thread
From: Jason A. Donenfeld @ 2022-04-14  9:27 UTC (permalink / raw)
  To: Maciej W. Rozycki
  Cc: Thomas Bogendoerfer, LKML, Linux Crypto Mailing List,
	Thomas Gleixner, Arnd Bergmann, Theodore Ts'o,
	Dominik Brodowski, Russell King, Catalin Marinas, Will Deacon,
	Geert Uytterhoeven, Paul Walmsley, Palmer Dabbelt, Albert Ou,
	David S . Miller, Richard Weinberger, Anton Ivanov,
	Johannes Berg, Ingo Molnar, Borislav Petkov, Dave Hansen,
	H . Peter Anvin, Chris Zankel, Max Filippov, John Stultz,
	Stephen Boyd, Dinh Nguyen, linux-arm-kernel, linux-m68k,
	open list:BROADCOM NVRAM DRIVER, linux-riscv, sparclinux,
	linux-um, X86 ML, linux-xtensa

Hi Maciej,

On Thu, Apr 14, 2022 at 02:16:18AM +0100, Maciej W. Rozycki wrote:
>  Yes, for the relevant CPUs the range is 63-8 << 8 for R3k machines and 
> 47-0 (the lower bound can be higher if wired entries are used, which I 
> think we occasionally do) for R4k machines with a buggy CP0 counter.  So 
> there are either 56 or up to 48 distinct CP0 Random register values.

Ahh interesting, so it varies a bit, but it remains rather small.

>  It depends on the exact system.  Some have a 32-bit high-resolution 
> counter in the chipset (arch/mips/kernel/csrc-ioasic.c) giving like 25MHz 
> resolution, some have nothing but jiffies.

Alright, so there _are_ machines with no c0 cycles but with a good
clock. Yet, 25MHz is still less than the cpu cycle, so this c0 random
ORing trick remains useful perhaps.

>  It seems like a reasonable idea to me, but the details would have to be 
> sorted out, because where a chipset high-resolution counter is available 
> we want to factor it in, and otherwise we need to extract the right bits 
> from the CP0 Random register, either 13:8 for the R3k or 5:0 for the R4k.

One thing we could do here that would seemingly cover all the cases
without losing _that_ much would be:

    return (random_get_entropy_fallback() << 13) | ((1<<13) - read_c0_random());

Or in case the 13 turns out to be wrong on some hardware, we could
mitigate the effect with:

    return (random_get_entropy_fallback() << 13) ^ ((1<<13) - read_c0_random());

As mentioned in the 1/xx patch of this series,
random_get_entropy_fallback() should call the highest resolution thing.
We then shave off the least-changing bits and stuff in the
faster-changing bits from read_c0_random(). Then, in order to keep it
counting up instead of down, we do the subtraction there.

What do you think of this plan?

Jason

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

* Re: [PATCH v4 01/11] timekeeping: add raw clock fallback for random_get_entropy()
  2022-04-13 11:54 ` [PATCH v4 01/11] timekeeping: add raw clock fallback for random_get_entropy() Jason A. Donenfeld
  2022-04-13 14:32   ` Rob Herring
@ 2022-04-14 10:12   ` Russell King (Oracle)
  2022-04-14 11:56     ` Jason A. Donenfeld
  1 sibling, 1 reply; 34+ messages in thread
From: Russell King (Oracle) @ 2022-04-14 10:12 UTC (permalink / raw)
  To: Jason A. Donenfeld
  Cc: linux-kernel, linux-crypto, tglx, arnd, Theodore Ts'o,
	Dominik Brodowski, Catalin Marinas, Will Deacon,
	Geert Uytterhoeven, Thomas Bogendoerfer, Paul Walmsley,
	Palmer Dabbelt, Albert Ou, David S . Miller, Richard Weinberger,
	Anton Ivanov, Johannes Berg, Ingo Molnar, Borislav Petkov,
	Dave Hansen, H . Peter Anvin, Chris Zankel, Max Filippov,
	John Stultz, Stephen Boyd, Dinh Nguyen, linux-arm-kernel,
	linux-m68k, linux-mips, linux-riscv, sparclinux, linux-um, x86,
	linux-xtensa

On Wed, Apr 13, 2022 at 01:54:01PM +0200, Jason A. Donenfeld wrote:
> The addition of random_get_entropy_fallback() provides access to
> whichever time source has the highest frequency, which is useful for
> gathering entropy on platforms without available cycle counters. It's
> not necessarily as good as being able to quickly access a cycle counter
> that the CPU has, but it's still something, even when it falls back to
> being jiffies-based.
> 
> In the event that a given arch does not define get_cycles(), falling
> back to the get_cycles() default implementation that returns 0 is really
> not the best we can do. Instead, at least calling
> random_get_entropy_fallback() would be preferable, because that always
> needs to return _something_, even falling back to jiffies eventually.
> It's not as though random_get_entropy_fallback() is super high precision
> or guaranteed to be entropic, but basically anything that's not zero all
> the time is better than returning zero all the time.
> 
> Cc: Thomas Gleixner <tglx@linutronix.de>
> Cc: Arnd Bergmann <arnd@arndb.de>
> Cc: Theodore Ts'o <tytso@mit.edu>
> Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
> ---
>  include/linux/timex.h     |  8 ++++++++
>  kernel/time/timekeeping.c | 10 ++++++++++
>  2 files changed, 18 insertions(+)
> 
> diff --git a/include/linux/timex.h b/include/linux/timex.h
> index 5745c90c8800..fbbe34226044 100644
> --- a/include/linux/timex.h
> +++ b/include/linux/timex.h
> @@ -62,6 +62,8 @@
>  #include <linux/types.h>
>  #include <linux/param.h>
>  
> +extern unsigned long random_get_entropy_fallback(void);

Hi

I'm surprised this didn't trigger checkpatch to warn. From
coding-style:

6.1) Function prototypes
Do not use the ``extern`` keyword with function declarations as this makes
lines longer and isn't strictly necessary.

Thanks!

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!

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

* Re: [PATCH v4 01/11] timekeeping: add raw clock fallback for random_get_entropy()
  2022-04-14 10:12   ` Russell King (Oracle)
@ 2022-04-14 11:56     ` Jason A. Donenfeld
  0 siblings, 0 replies; 34+ messages in thread
From: Jason A. Donenfeld @ 2022-04-14 11:56 UTC (permalink / raw)
  To: Russell King (Oracle)
  Cc: LKML, Linux Crypto Mailing List, Thomas Gleixner, Arnd Bergmann,
	Theodore Ts'o, Dominik Brodowski, Catalin Marinas,
	Will Deacon, Geert Uytterhoeven, Thomas Bogendoerfer,
	Paul Walmsley, Palmer Dabbelt, Albert Ou, David S . Miller,
	Richard Weinberger, Anton Ivanov, Johannes Berg, Ingo Molnar,
	Borislav Petkov, Dave Hansen, H . Peter Anvin, Chris Zankel,
	Max Filippov, John Stultz, Stephen Boyd, Dinh Nguyen,
	linux-arm-kernel, linux-m68k, open list:BROADCOM NVRAM DRIVER,
	linux-riscv, sparclinux, linux-um, X86 ML, linux-xtensa

Hi Russell,

On Thu, Apr 14, 2022 at 12:12 PM Russell King (Oracle)
<linux@armlinux.org.uk> wrote:
> I'm surprised this didn't trigger checkpatch to warn. From
> coding-style:
>
> 6.1) Function prototypes
> Do not use the ``extern`` keyword with function declarations as this makes
> lines longer and isn't strictly necessary.

Okay, will do for v+1.

Jason

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

* Re: [PATCH v4 01/11] timekeeping: add raw clock fallback for random_get_entropy()
  2022-04-13 22:38     ` Jason A. Donenfeld
@ 2022-04-14 20:41       ` Rob Herring
  2022-04-14 21:49         ` H. Peter Anvin
  0 siblings, 1 reply; 34+ messages in thread
From: Rob Herring @ 2022-04-14 20:41 UTC (permalink / raw)
  To: Jason A. Donenfeld
  Cc: linux-kernel, open list:HARDWARE RANDOM NUMBER GENERATOR CORE,
	Thomas Gleixner, Arnd Bergmann, Theodore Ts'o,
	Dominik Brodowski, Russell King, Catalin Marinas, Will Deacon,
	Geert Uytterhoeven, Thomas Bogendoerfer, Paul Walmsley,
	Palmer Dabbelt, Albert Ou, David S . Miller, Richard Weinberger,
	Anton Ivanov, Johannes Berg, Ingo Molnar, Borislav Petkov,
	Dave Hansen, H . Peter Anvin, Chris Zankel, Max Filippov,
	John Stultz, Stephen Boyd, Dinh Nguyen, linux-arm-kernel,
	linux-m68k, open list:MIPS, linux-riscv, sparclinux, linux-um,
	X86 ML, linux-xtensa

On Thu, Apr 14, 2022 at 12:38:49AM +0200, Jason A. Donenfeld wrote:
> Hi Rob,
> 
> On Wed, Apr 13, 2022 at 4:32 PM Rob Herring <robh@kernel.org> wrote:
> > 'does not have a usable get_cycles(), ...' as clearly some arches have
> > get_cycles() and yet still need a fallback.
> >
> > Why not handle the 'if get_cycles() returns 0 do the fallback' within
> > a weak random_get_entropy() function? Then more arches don't need any
> > random_get_entropy() implementation.
> 
> No, this doesn't really work. Actually, most archs don't need a
> random_get_entropy() function, because it exists in asm-generic doing
> the thing we want. So that's taken care of. But weak functions as you
> suggested would be quite suboptimal, because on, e.g. x86, what we
> have now gets inlined into a single rdtsc instruction. Also, the
> relation between get_cycles() and random_get_entropy() doesn't always
> hold; some archs may not have a working get_cycles() function but do
> have a path for a random_get_entropy(). Etc, etc. So I'm pretty sure
> that this commit is really the most simple and optimal thing to do. I
> really don't want to go the weak functions route.

Is random_get_entropy() a hot path?


It doesn't have to be a weak function, but look at it this way. We have 
the following possibilities for what random_get_entropy() does:

- get_cycles()
- get_cycles() but returns 0 sometimes
- returns 0
- something else

You're handling the 3rd case.

For the 2nd case, that's riscv, arm, nios2, and x86. That's not a lot, 
but is 2 or 3 of the most widely used architectures. Is it really too 
much to ask to support the 2nd case in the generic code/header?

Rob

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

* Re: [PATCH v4 01/11] timekeeping: add raw clock fallback for random_get_entropy()
  2022-04-14 20:41       ` Rob Herring
@ 2022-04-14 21:49         ` H. Peter Anvin
  0 siblings, 0 replies; 34+ messages in thread
From: H. Peter Anvin @ 2022-04-14 21:49 UTC (permalink / raw)
  To: Rob Herring, Jason A. Donenfeld
  Cc: linux-kernel, open list:HARDWARE RANDOM NUMBER GENERATOR CORE,
	Thomas Gleixner, Arnd Bergmann, Theodore Ts'o,
	Dominik Brodowski, Russell King, Catalin Marinas, Will Deacon,
	Geert Uytterhoeven, Thomas Bogendoerfer, Paul Walmsley,
	Palmer Dabbelt, Albert Ou, David S . Miller, Richard Weinberger,
	Anton Ivanov, Johannes Berg, Ingo Molnar, Borislav Petkov,
	Dave Hansen, Chris Zankel, Max Filippov, John Stultz,
	Stephen Boyd, Dinh Nguyen, linux-arm-kernel, linux-m68k,
	open list:MIPS, linux-riscv, sparclinux, linux-um, X86 ML,
	linux-xtensa

On April 14, 2022 1:41:38 PM PDT, Rob Herring <robh@kernel.org> wrote:
>On Thu, Apr 14, 2022 at 12:38:49AM +0200, Jason A. Donenfeld wrote:
>> Hi Rob,
>> 
>> On Wed, Apr 13, 2022 at 4:32 PM Rob Herring <robh@kernel.org> wrote:
>> > 'does not have a usable get_cycles(), ...' as clearly some arches have
>> > get_cycles() and yet still need a fallback.
>> >
>> > Why not handle the 'if get_cycles() returns 0 do the fallback' within
>> > a weak random_get_entropy() function? Then more arches don't need any
>> > random_get_entropy() implementation.
>> 
>> No, this doesn't really work. Actually, most archs don't need a
>> random_get_entropy() function, because it exists in asm-generic doing
>> the thing we want. So that's taken care of. But weak functions as you
>> suggested would be quite suboptimal, because on, e.g. x86, what we
>> have now gets inlined into a single rdtsc instruction. Also, the
>> relation between get_cycles() and random_get_entropy() doesn't always
>> hold; some archs may not have a working get_cycles() function but do
>> have a path for a random_get_entropy(). Etc, etc. So I'm pretty sure
>> that this commit is really the most simple and optimal thing to do. I
>> really don't want to go the weak functions route.
>
>Is random_get_entropy() a hot path?
>
>
>It doesn't have to be a weak function, but look at it this way. We have 
>the following possibilities for what random_get_entropy() does:
>
>- get_cycles()
>- get_cycles() but returns 0 sometimes
>- returns 0
>- something else
>
>You're handling the 3rd case.
>
>For the 2nd case, that's riscv, arm, nios2, and x86. That's not a lot, 
>but is 2 or 3 of the most widely used architectures. Is it really too 
>much to ask to support the 2nd case in the generic code/header?
>
>Rob

It goes into interrupts, which means it is latency critical.

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

* Re: [PATCH v4 04/11] mips: use fallback for random_get_entropy() instead of zero
  2022-04-14  9:27           ` Jason A. Donenfeld
@ 2022-04-15 12:26             ` Maciej W. Rozycki
  2022-04-16 11:09               ` Jason A. Donenfeld
  2022-04-18  7:10               ` Thomas Bogendoerfer
  0 siblings, 2 replies; 34+ messages in thread
From: Maciej W. Rozycki @ 2022-04-15 12:26 UTC (permalink / raw)
  To: Jason A. Donenfeld
  Cc: Thomas Bogendoerfer, LKML, Linux Crypto Mailing List,
	Thomas Gleixner, Arnd Bergmann, Theodore Ts'o,
	Dominik Brodowski, Russell King, Catalin Marinas, Will Deacon,
	Geert Uytterhoeven, Paul Walmsley, Palmer Dabbelt, Albert Ou,
	David S . Miller, Richard Weinberger, Anton Ivanov,
	Johannes Berg, Ingo Molnar, Borislav Petkov, Dave Hansen,
	H . Peter Anvin, Chris Zankel, Max Filippov, John Stultz,
	Stephen Boyd, Dinh Nguyen, linux-arm-kernel, linux-m68k,
	open list:BROADCOM NVRAM DRIVER, linux-riscv, sparclinux,
	linux-um, X86 ML, linux-xtensa

Hi Jason,

> >  It depends on the exact system.  Some have a 32-bit high-resolution 
> > counter in the chipset (arch/mips/kernel/csrc-ioasic.c) giving like 25MHz 
> > resolution, some have nothing but jiffies.
> 
> Alright, so there _are_ machines with no c0 cycles but with a good
> clock. Yet, 25MHz is still less than the cpu cycle, so this c0 random
> ORing trick remains useful perhaps.

 It's not much less than the CPU cycle really, given that the R3k CPUs are 
clocked at up to 40MHz in the systems concerned and likewise the buggy R4k 
CPUs run at up to 60MHz (and mind that their CP0 Count register increments 
at half the clock rate, so the rate is up to 30MHz anyway).  The overhead 
of the calculation is more than that, let alone the latency and issue rate 
of an uncached MMIO access to the chipset register.

 Also the systems I have in mind and that lack a counter in the chipset 
actually can make use of the buggy CP0 timer, because it's only when CP0 
timer interrupts are used that the erratum matters, but they use a DS1287 
RTC interrupt instead unconditionally as the clock event (see the comment 
at the bottom of arch/mips/dec/time.c).  But this has not been factored in 
with `can_use_mips_counter' (should it just check for `mips_hpt_frequency' 
being zero perhaps, meaning the timer interrupt not being used?).

 Thomas, do you happen to know if any of the SGI systems that we support 
had buggy early R4k chips?

> >  It seems like a reasonable idea to me, but the details would have to be 
> > sorted out, because where a chipset high-resolution counter is available 
> > we want to factor it in, and otherwise we need to extract the right bits 
> > from the CP0 Random register, either 13:8 for the R3k or 5:0 for the R4k.
> 
> One thing we could do here that would seemingly cover all the cases
> without losing _that_ much would be:
> 
>     return (random_get_entropy_fallback() << 13) | ((1<<13) - read_c0_random());

 Except this would have to be:

    return (random_get_entropy_fallback() << 14) | ((1<<14) - read_c0_random());

of course, as bit 13 is still one of the active ones in the R3k CP0 Random 
register.

> Or in case the 13 turns out to be wrong on some hardware, we could
> mitigate the effect with:
> 
>     return (random_get_entropy_fallback() << 13) ^ ((1<<13) - read_c0_random());

 There are two variants only of the CP0 Random register that we can ever 
encounter, as it's been de-facto standardised in early 1990s already and 
then written down in the MIPSr1 architecture specification ~2000.  So I 
think it may make sense to actually handle them both explictitly with 
individual calculations, possibly conditionalised on a CONFIG setting or 
`cpu_has_3kex', because kernels that support the two variants of the MMU 
architecture are mutually incompatible.

 Ah, there's that buggy non-compliant JZ4740 chip too.  I guess we can 
figure out how many CP0 Random bits it implements, though it may be worth 
noting that architecturally the register is not required to decrement, so 
again it may be good to double-check how the JZ4740 selects the values 
there.

 I think the check for a buggy CP0 timer in `can_use_mips_counter' should 
also be qualified with !(CONFIG_CPU_MIPS32 || CONFIG_CPU_MIPS64), which 
will reduce the function to a constant 1 for the overwhelming majority of 
systems out there, without a need to refer to CP0 PRId every time.

> As mentioned in the 1/xx patch of this series,
> random_get_entropy_fallback() should call the highest resolution thing.
> We then shave off the least-changing bits and stuff in the
> faster-changing bits from read_c0_random(). Then, in order to keep it
> counting up instead of down, we do the subtraction there.

 Isn't it going to be an issue for an entropy source that the distribution 
of values obtained from the CP0 Random bit-field is not even, that is some 
values from the 6-bit range will never appear?

> What do you think of this plan?

 Otherwise it makes absolute sense to me.

  Maciej

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

* Re: [PATCH v4 04/11] mips: use fallback for random_get_entropy() instead of zero
  2022-04-15 12:26             ` Maciej W. Rozycki
@ 2022-04-16 11:09               ` Jason A. Donenfeld
  2022-04-16 14:44                 ` Maciej W. Rozycki
  2022-04-18  7:10               ` Thomas Bogendoerfer
  1 sibling, 1 reply; 34+ messages in thread
From: Jason A. Donenfeld @ 2022-04-16 11:09 UTC (permalink / raw)
  To: Maciej W. Rozycki
  Cc: Thomas Bogendoerfer, LKML, Linux Crypto Mailing List,
	Thomas Gleixner, Arnd Bergmann, Theodore Ts'o,
	Dominik Brodowski, Russell King, Catalin Marinas, Will Deacon,
	Geert Uytterhoeven, Paul Walmsley, Palmer Dabbelt, Albert Ou,
	David S . Miller, Richard Weinberger, Anton Ivanov,
	Johannes Berg, Ingo Molnar, Borislav Petkov, Dave Hansen,
	H . Peter Anvin, Chris Zankel, Max Filippov, John Stultz,
	Stephen Boyd, Dinh Nguyen, linux-arm-kernel, linux-m68k,
	open list:BROADCOM NVRAM DRIVER, linux-riscv, sparclinux,
	linux-um, X86 ML, linux-xtensa

Hi Maciej,

On Fri, Apr 15, 2022 at 2:26 PM Maciej W. Rozycki <macro@orcam.me.uk> wrote:
>     return (random_get_entropy_fallback() << 14) | ((1<<14) - read_c0_random());
>
> of course, as bit 13 is still one of the active ones in the R3k CP0 Random
> register.

Ah, thanks, will do that.

>  There are two variants only of the CP0 Random register that we can ever
> encounter, as it's been de-facto standardised in early 1990s already and
> then written down in the MIPSr1 architecture specification ~2000.  So I
> think it may make sense to actually handle them both explictitly with
> individual calculations, possibly conditionalised on a CONFIG setting or
> `cpu_has_3kex', because kernels that support the two variants of the MMU
> architecture are mutually incompatible.

Okay, I can give this a shot, but this certainly isn't my forté. It
may ultimately wind up being simpler for you to just send some code of
what you envision for this, but if I understand your idea correctly,
what you're saying is something like:

static inline unsigned long random_get_entropy(void)
{
        unsigned int prid = read_c0_prid();
        unsigned int imp = prid & PRID_IMP_MASK;
        unsigned int c0_random;

        if (can_use_mips_counter(prid))
                return read_c0_count();

        if (cpu_has_3kex)
                c0_random = (read_c0_random() >> 8) & 0x3f;
        else
                c0_random = read_c0_random() & 0x3f;
        return (random_get_entropy_fallback() << 6) | (0x3f - c0_random);
}

What do you think of that? Some tweak I'm missing?

>  Isn't it going to be an issue for an entropy source that the distribution
> of values obtained from the CP0 Random bit-field is not even, that is some
> values from the 6-bit range will never appear?

It's the same situation without inverting the order: instead of some
bits on the top never happening, some bits on the bottom never happen
instead. In general, counters don't form uniform distributions anyway,
since the lower bits change faster, and neither are they independent,
since one sample in large part depends on the previous. This is just
sort of the nature of the beast, and the code that calls
random_get_entropy() deals with this appropriately (by, at the moment,
just hashing all the bits).

Jason

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

* Re: [PATCH v4 04/11] mips: use fallback for random_get_entropy() instead of zero
  2022-04-16 11:09               ` Jason A. Donenfeld
@ 2022-04-16 14:44                 ` Maciej W. Rozycki
  2022-04-16 22:54                   ` Jason A. Donenfeld
  0 siblings, 1 reply; 34+ messages in thread
From: Maciej W. Rozycki @ 2022-04-16 14:44 UTC (permalink / raw)
  To: Jason A. Donenfeld
  Cc: Thomas Bogendoerfer, LKML, Linux Crypto Mailing List,
	Thomas Gleixner, Arnd Bergmann, Theodore Ts'o,
	Dominik Brodowski, Russell King, Catalin Marinas, Will Deacon,
	Geert Uytterhoeven, Paul Walmsley, Palmer Dabbelt, Albert Ou,
	David S . Miller, Richard Weinberger, Anton Ivanov,
	Johannes Berg, Ingo Molnar, Borislav Petkov, Dave Hansen,
	H . Peter Anvin, Chris Zankel, Max Filippov, John Stultz,
	Stephen Boyd, Dinh Nguyen, linux-arm-kernel, linux-m68k,
	open list:BROADCOM NVRAM DRIVER, linux-riscv, sparclinux,
	linux-um, X86 ML, linux-xtensa

Hi Jason,

> >  There are two variants only of the CP0 Random register that we can ever
> > encounter, as it's been de-facto standardised in early 1990s already and
> > then written down in the MIPSr1 architecture specification ~2000.  So I
> > think it may make sense to actually handle them both explictitly with
> > individual calculations, possibly conditionalised on a CONFIG setting or
> > `cpu_has_3kex', because kernels that support the two variants of the MMU
> > architecture are mutually incompatible.
> 
> Okay, I can give this a shot, but this certainly isn't my forté. It
> may ultimately wind up being simpler for you to just send some code of
> what you envision for this, but if I understand your idea correctly,
> what you're saying is something like:
> 
> static inline unsigned long random_get_entropy(void)
> {
>         unsigned int prid = read_c0_prid();
>         unsigned int imp = prid & PRID_IMP_MASK;
>         unsigned int c0_random;
> 
>         if (can_use_mips_counter(prid))
>                 return read_c0_count();
> 
>         if (cpu_has_3kex)
>                 c0_random = (read_c0_random() >> 8) & 0x3f;
>         else
>                 c0_random = read_c0_random() & 0x3f;
>         return (random_get_entropy_fallback() << 6) | (0x3f - c0_random);
> }
> 
> What do you think of that? Some tweak I'm missing?

 It certainly looks good to me.  Do you have a way I could verify how this 
function performs?  If so, then I could put it through my systems as I can 
cover all the cases handled here.

 Any improvements I previously discussed can then be made locally in the 
MIPS port as follow-up changes.

> >  Isn't it going to be an issue for an entropy source that the distribution
> > of values obtained from the CP0 Random bit-field is not even, that is some
> > values from the 6-bit range will never appear?
> 
> It's the same situation without inverting the order: instead of some
> bits on the top never happening, some bits on the bottom never happen
> instead. In general, counters don't form uniform distributions anyway,
> since the lower bits change faster, and neither are they independent,
> since one sample in large part depends on the previous. This is just
> sort of the nature of the beast, and the code that calls
> random_get_entropy() deals with this appropriately (by, at the moment,
> just hashing all the bits).

 OK then, thanks for your clarification.

  Maciej

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

* Re: [PATCH v4 04/11] mips: use fallback for random_get_entropy() instead of zero
  2022-04-16 14:44                 ` Maciej W. Rozycki
@ 2022-04-16 22:54                   ` Jason A. Donenfeld
  0 siblings, 0 replies; 34+ messages in thread
From: Jason A. Donenfeld @ 2022-04-16 22:54 UTC (permalink / raw)
  To: Maciej W. Rozycki
  Cc: Thomas Bogendoerfer, LKML, Linux Crypto Mailing List,
	Thomas Gleixner, Arnd Bergmann, Theodore Ts'o,
	Dominik Brodowski, Russell King, Catalin Marinas, Will Deacon,
	Geert Uytterhoeven, Paul Walmsley, Palmer Dabbelt, Albert Ou,
	David S . Miller, Richard Weinberger, Anton Ivanov,
	Johannes Berg, Ingo Molnar, Borislav Petkov, Dave Hansen,
	H . Peter Anvin, Chris Zankel, Max Filippov, John Stultz,
	Stephen Boyd, Dinh Nguyen, linux-arm-kernel, linux-m68k,
	open list:BROADCOM NVRAM DRIVER, linux-riscv, sparclinux,
	linux-um, X86 ML, linux-xtensa

Hi Maciej,

On Sat, Apr 16, 2022 at 03:44:53PM +0100, Maciej W. Rozycki wrote:
> > Okay, I can give this a shot, but this certainly isn't my forté. It
> > may ultimately wind up being simpler for you to just send some code of
> > what you envision for this, but if I understand your idea correctly,
> > what you're saying is something like:
> > 
> > static inline unsigned long random_get_entropy(void)
> > {
> >         unsigned int prid = read_c0_prid();
> >         unsigned int imp = prid & PRID_IMP_MASK;
> >         unsigned int c0_random;
> > 
> >         if (can_use_mips_counter(prid))
> >                 return read_c0_count();
> > 
> >         if (cpu_has_3kex)
> >                 c0_random = (read_c0_random() >> 8) & 0x3f;
> >         else
> >                 c0_random = read_c0_random() & 0x3f;
> >         return (random_get_entropy_fallback() << 6) | (0x3f - c0_random);
> > }
> > 
> > What do you think of that? Some tweak I'm missing?
> 
>  It certainly looks good to me.  Do you have a way I could verify how this 
> function performs?  If so, then I could put it through my systems as I can 
> cover all the cases handled here.

Oh, awesome about the test rig. Below is a little patch that adds some
printf code to init, calling the above sequence 70 times in a busy loop
and then 30 times after that with a scheduler 1 ms delay in there,
printing lots of various about the above calculation. Hopefully that's
enough information that it'll be possible to notice if something looks
really off when we squint at it.

Jason

-------------------8<-----------------------------------------------------

diff --git a/drivers/char/random.c b/drivers/char/random.c
index 3a293f919af9..0b32203aa47f 100644
--- a/drivers/char/random.c
+++ b/drivers/char/random.c
@@ -48,6 +48,7 @@
 #include <linux/ptrace.h>
 #include <linux/workqueue.h>
 #include <linux/irq.h>
+#include <linux/delay.h>
 #include <linux/ratelimit.h>
 #include <linux/syscalls.h>
 #include <linux/completion.h>
@@ -1781,6 +1782,26 @@ static struct ctl_table random_table[] = {
  */
 static int __init random_sysctls_init(void)
 {
+
+	int i;
+	for (i = 0; i < 100; ++i) {
+		if (can_use_mips_counter(read_c0_prid()))
+			pr_err("SARU %d c0 count: %08x\n", i, read_c0_count());
+		else {
+			unsigned int c0_random = read_c0_random(), c0_random_mask;
+			unsigned long fallback = random_get_entropy_fallback(), out;
+			if (cpu_has_3kex)
+				c0_random_mask = (c0_random >> 8) & 0x3f;
+			else
+				c0_random_mask = c0_random & 0x3f;
+			out = (fallback << 6) | (0x3f - c0_random_mask);
+			pr_err("SARU: %d (%08x >> n) & 0x3f = %08x, inverted = %08x, fallback = %08lx, fallback << 6 = %08lx, combined = %08lx\n",
+			       i, c0_random, c0_random_mask, 0x3f - c0_random_mask, fallback, fallback << 6, out);
+		}
+		if (i > 70)
+			msleep(1);
+	}
+
 	register_sysctl_init("kernel/random", random_table);
 	return 0;
 }
diff --git a/include/linux/timex.h b/include/linux/timex.h
index 5745c90c8800..3871b06bd302 100644
--- a/include/linux/timex.h
+++ b/include/linux/timex.h
@@ -62,6 +62,8 @@
 #include <linux/types.h>
 #include <linux/param.h>

+unsigned long random_get_entropy_fallback(void);
+
 #include <asm/timex.h>

 #ifndef random_get_entropy
@@ -74,8 +76,14 @@
  *
  * By default we use get_cycles() for this purpose, but individual
  * architectures may override this in their asm/timex.h header file.
+ * If a given arch does not have get_cycles(), then we fallback to
+ * using random_get_entropy_fallback().
  */
+#ifdef get_cycles
 #define random_get_entropy()	((unsigned long)get_cycles())
+#else
+#define random_get_entropy()	random_get_entropy_fallback()
+#endif
 #endif

 /*
diff --git a/kernel/time/timekeeping.c b/kernel/time/timekeeping.c
index dcdcb85121e4..7cd2ec239cae 100644
--- a/kernel/time/timekeeping.c
+++ b/kernel/time/timekeeping.c
@@ -17,6 +17,7 @@
 #include <linux/clocksource.h>
 #include <linux/jiffies.h>
 #include <linux/time.h>
+#include <linux/timex.h>
 #include <linux/tick.h>
 #include <linux/stop_machine.h>
 #include <linux/pvclock_gtod.h>
@@ -2380,6 +2381,15 @@ static int timekeeping_validate_timex(const struct __kernel_timex *txc)
 	return 0;
 }

+/**
+ * random_get_entropy_fallback - Returns the raw clock source value,
+ * used by random.c for platforms with no valid random_get_entropy().
+ */
+unsigned long random_get_entropy_fallback(void)
+{
+	return tk_clock_read(&tk_core.timekeeper.tkr_mono);
+}
+EXPORT_SYMBOL_GPL(random_get_entropy_fallback);

 /**
  * do_adjtimex() - Accessor function to NTP __do_adjtimex function


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

* Re: [PATCH v4 04/11] mips: use fallback for random_get_entropy() instead of zero
  2022-04-15 12:26             ` Maciej W. Rozycki
  2022-04-16 11:09               ` Jason A. Donenfeld
@ 2022-04-18  7:10               ` Thomas Bogendoerfer
  2022-04-23 23:33                 ` Maciej W. Rozycki
  1 sibling, 1 reply; 34+ messages in thread
From: Thomas Bogendoerfer @ 2022-04-18  7:10 UTC (permalink / raw)
  To: Maciej W. Rozycki
  Cc: Jason A. Donenfeld, LKML, Linux Crypto Mailing List,
	Thomas Gleixner, Arnd Bergmann, Theodore Ts'o,
	Dominik Brodowski, Russell King, Catalin Marinas, Will Deacon,
	Geert Uytterhoeven, Paul Walmsley, Palmer Dabbelt, Albert Ou,
	David S . Miller, Richard Weinberger, Anton Ivanov,
	Johannes Berg, Ingo Molnar, Borislav Petkov, Dave Hansen,
	H . Peter Anvin, Chris Zankel, Max Filippov, John Stultz,
	Stephen Boyd, Dinh Nguyen, linux-arm-kernel, linux-m68k,
	open list:BROADCOM NVRAM DRIVER, linux-riscv, sparclinux,
	linux-um, X86 ML, linux-xtensa

On Fri, Apr 15, 2022 at 01:26:48PM +0100, Maciej W. Rozycki wrote:
> Hi Jason,
> 
> > >  It depends on the exact system.  Some have a 32-bit high-resolution 
> > > counter in the chipset (arch/mips/kernel/csrc-ioasic.c) giving like 25MHz 
> > > resolution, some have nothing but jiffies.
> > 
> > Alright, so there _are_ machines with no c0 cycles but with a good
> > clock. Yet, 25MHz is still less than the cpu cycle, so this c0 random
> > ORing trick remains useful perhaps.
> 
>  It's not much less than the CPU cycle really, given that the R3k CPUs are 
> clocked at up to 40MHz in the systems concerned and likewise the buggy R4k 
> CPUs run at up to 60MHz (and mind that their CP0 Count register increments 
> at half the clock rate, so the rate is up to 30MHz anyway).  The overhead 
> of the calculation is more than that, let alone the latency and issue rate 
> of an uncached MMIO access to the chipset register.
> 
>  Also the systems I have in mind and that lack a counter in the chipset 
> actually can make use of the buggy CP0 timer, because it's only when CP0 
> timer interrupts are used that the erratum matters, but they use a DS1287 
> RTC interrupt instead unconditionally as the clock event (see the comment 
> at the bottom of arch/mips/dec/time.c).  But this has not been factored in 
> with `can_use_mips_counter' (should it just check for `mips_hpt_frequency' 
> being zero perhaps, meaning the timer interrupt not being used?).
> 
>  Thomas, do you happen to know if any of the SGI systems that we support 
> had buggy early R4k chips?

IP22 has probably seen all buggy MIPS chips produced, so yes I even own
Indy/Indigo2 CPU boards with early R4k chips.

Thomas.

-- 
Crap can work. Given enough thrust pigs will fly, but it's not necessarily a
good idea.                                                [ RFC1925, 2.3 ]

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

* Re: [PATCH v4 04/11] mips: use fallback for random_get_entropy() instead of zero
  2022-04-18  7:10               ` Thomas Bogendoerfer
@ 2022-04-23 23:33                 ` Maciej W. Rozycki
  2022-04-24  8:15                   ` Jason A. Donenfeld
  0 siblings, 1 reply; 34+ messages in thread
From: Maciej W. Rozycki @ 2022-04-23 23:33 UTC (permalink / raw)
  To: Thomas Bogendoerfer
  Cc: Jason A. Donenfeld, LKML, Linux Crypto Mailing List,
	Thomas Gleixner, Arnd Bergmann, Theodore Ts'o,
	Dominik Brodowski, Russell King, Catalin Marinas, Will Deacon,
	Geert Uytterhoeven, Paul Walmsley, Palmer Dabbelt, Albert Ou,
	David S . Miller, Richard Weinberger, Anton Ivanov,
	Johannes Berg, Ingo Molnar, Borislav Petkov, Dave Hansen,
	H . Peter Anvin, Chris Zankel, Max Filippov, John Stultz,
	Stephen Boyd, Dinh Nguyen, linux-arm-kernel, linux-m68k,
	open list:BROADCOM NVRAM DRIVER, linux-riscv, sparclinux,
	linux-um, X86 ML, linux-xtensa

On Mon, 18 Apr 2022, Thomas Bogendoerfer wrote:

> >  Also the systems I have in mind and that lack a counter in the chipset 
> > actually can make use of the buggy CP0 timer, because it's only when CP0 
> > timer interrupts are used that the erratum matters, but they use a DS1287 
> > RTC interrupt instead unconditionally as the clock event (see the comment 
> > at the bottom of arch/mips/dec/time.c).  But this has not been factored in 
> > with `can_use_mips_counter' (should it just check for `mips_hpt_frequency' 
> > being zero perhaps, meaning the timer interrupt not being used?).
> > 
> >  Thomas, do you happen to know if any of the SGI systems that we support 
> > had buggy early R4k chips?
> 
> IP22 has probably seen all buggy MIPS chips produced, so yes I even own
> Indy/Indigo2 CPU boards with early R4k chips.

 Do they actually use the CP0 timer as a clock event device?  Do they have 
an alternative high-precision timer available?

 In the course of verifying this change I have noticed my DECstation
5000/260, which has a high-precision timer in the chipset available as a 
clock source device, does register the CP0 timer as a clock source device 
regardless.  Upon a closer inspection I have noticed that the CP0 timer 
interrupt is non-functional in this machine, which I have then confirmed 
as a valid CPU hardware configuration via the TimIntDis/TimerIntDis (the 
R4k CPU manual is inconsistent in naming here) boot-mode bit.  It allows 
IP7 to be used as an external interrupt source instead.  I used not to be 
aware of the presence of this boot-mode bit.

 I find this arrangement odd, because IP7 used to be wired internally as 
the FPU interrupt with the 5000/240's CPU module, so it's not usable as an 
external interrupt anyway with this system's mainboard.

 That means however that this machine (and possibly the 5000/150 as well, 
but I'll have to verify that once I get at the KN04 CPU module I have in a 
drawer at my other place) can use the CP0 timer as a clock source device 
unconditionally.  I think this discovery asks for code optimisation, which 
I'll try to cook up sometime.

 I don't expect the IP22 to have a similar arrangement with the CP0 timer 
interrupt given that the CPU was an in-house design at SGI, but who knows?  
Do you?

  Maciej

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

* Re: [PATCH v4 04/11] mips: use fallback for random_get_entropy() instead of zero
  2022-04-23 23:33                 ` Maciej W. Rozycki
@ 2022-04-24  8:15                   ` Jason A. Donenfeld
  2022-04-24 10:51                     ` Maciej W. Rozycki
  0 siblings, 1 reply; 34+ messages in thread
From: Jason A. Donenfeld @ 2022-04-24  8:15 UTC (permalink / raw)
  To: Maciej W. Rozycki
  Cc: Thomas Bogendoerfer, LKML, Linux Crypto Mailing List,
	Thomas Gleixner, Arnd Bergmann, Theodore Ts'o,
	Dominik Brodowski, Russell King, Catalin Marinas, Will Deacon,
	Geert Uytterhoeven, Paul Walmsley, Palmer Dabbelt, Albert Ou,
	David S . Miller, Richard Weinberger, Anton Ivanov,
	Johannes Berg, Ingo Molnar, Borislav Petkov, Dave Hansen,
	H . Peter Anvin, Chris Zankel, Max Filippov, John Stultz,
	Stephen Boyd, Dinh Nguyen, linux-arm-kernel, linux-m68k,
	open list:BROADCOM NVRAM DRIVER, linux-riscv, sparclinux,
	linux-um, X86 ML, linux-xtensa

On Sun, Apr 24, 2022 at 12:33:44AM +0100, Maciej W. Rozycki wrote:
> unconditionally.  I think this discovery asks for code optimisation, which 
> I'll try to cook up sometime.

At some point too, by the way, we might also consider putting that into
a .c file rather than a static inline in the .h, since that function is
starting to get sort of big.

Jason

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

* Re: [PATCH v4 04/11] mips: use fallback for random_get_entropy() instead of zero
  2022-04-24  8:15                   ` Jason A. Donenfeld
@ 2022-04-24 10:51                     ` Maciej W. Rozycki
  0 siblings, 0 replies; 34+ messages in thread
From: Maciej W. Rozycki @ 2022-04-24 10:51 UTC (permalink / raw)
  To: Jason A. Donenfeld
  Cc: Thomas Bogendoerfer, LKML, Linux Crypto Mailing List,
	Thomas Gleixner, Arnd Bergmann, Theodore Ts'o,
	Dominik Brodowski, Russell King, Catalin Marinas, Will Deacon,
	Geert Uytterhoeven, Paul Walmsley, Palmer Dabbelt, Albert Ou,
	David S . Miller, Richard Weinberger, Anton Ivanov,
	Johannes Berg, Ingo Molnar, Borislav Petkov, Dave Hansen,
	H . Peter Anvin, Chris Zankel, Max Filippov, John Stultz,
	Stephen Boyd, Dinh Nguyen, linux-arm-kernel, linux-m68k,
	open list:BROADCOM NVRAM DRIVER, linux-riscv, sparclinux,
	linux-um, X86 ML, linux-xtensa

On Sun, 24 Apr 2022, Jason A. Donenfeld wrote:

> > unconditionally.  I think this discovery asks for code optimisation, which 
> > I'll try to cook up sometime.
> 
> At some point too, by the way, we might also consider putting that into
> a .c file rather than a static inline in the .h, since that function is
> starting to get sort of big.

 This code is supposed to produce one to a couple of machine instructions 
for the majority of configurations.  This is because the conditionals used 
are usually compile-time constants.  Therefore I think it will be good to 
continue having it as `static inline' functions.  Cf. the analysis in 
commit 06947aaaf9bf ("MIPS: Implement random_get_entropy with CP0 
Random").

 If this code does expand to a longer sequence for some platforms, then 
either they need to be verified whether they can be optimised (just as I 
note here for the DEC systems) or we can consider making these functions 
`extern inline' instead, with out-of-line code available from a .a file in 
case the compiler decides the code is too large for inlining to be worth 
doing after all.  Though I don't expect the latter case to be required 
really.

  Maciej

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

* Re: [PATCH v4 06/11] nios2: use fallback for random_get_entropy() instead of zero
  2022-04-13 11:54 ` [PATCH v4 06/11] nios2: " Jason A. Donenfeld
@ 2022-05-23 13:58   ` Dinh Nguyen
  0 siblings, 0 replies; 34+ messages in thread
From: Dinh Nguyen @ 2022-05-23 13:58 UTC (permalink / raw)
  To: Jason A. Donenfeld, linux-kernel, linux-crypto, tglx, arnd
  Cc: Theodore Ts'o, Dominik Brodowski, Russell King,
	Catalin Marinas, Will Deacon, Geert Uytterhoeven,
	Thomas Bogendoerfer, Paul Walmsley, Palmer Dabbelt, Albert Ou,
	David S . Miller, Richard Weinberger, Anton Ivanov,
	Johannes Berg, Ingo Molnar, Borislav Petkov, Dave Hansen,
	H . Peter Anvin, Chris Zankel, Max Filippov, John Stultz,
	Stephen Boyd, linux-arm-kernel, linux-m68k, linux-mips,
	linux-riscv, sparclinux, linux-um, x86, linux-xtensa



On 4/13/22 06:54, Jason A. Donenfeld wrote:
> In the event that random_get_entropy() can't access a cycle counter or
> similar, falling back to returning 0 is really not the best we can do.
> Instead, at least calling random_get_entropy_fallback() would be
> preferable, because that always needs to return _something_, even
> falling back to jiffies eventually. It's not as though
> random_get_entropy_fallback() is super high precision or guaranteed to
> be entropic, but basically anything that's not zero all the time is
> better than returning zero all the time.
> 
> Cc: Thomas Gleixner <tglx@linutronix.de>
> Cc: Arnd Bergmann <arnd@arndb.de>
> Cc: Dinh Nguyen <dinguyen@kernel.org>
> Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
> ---
>   arch/nios2/include/asm/timex.h | 2 ++
>   1 file changed, 2 insertions(+)
> 
> diff --git a/arch/nios2/include/asm/timex.h b/arch/nios2/include/asm/timex.h
> index a769f871b28d..d9a3f426cdda 100644
> --- a/arch/nios2/include/asm/timex.h
> +++ b/arch/nios2/include/asm/timex.h
> @@ -9,4 +9,6 @@ typedef unsigned long cycles_t;
>   
>   extern cycles_t get_cycles(void);
>   
> +#define random_get_entropy() (((unsigned long)get_cycles()) ?: random_get_entropy_fallback())
> +
>   #endif

Acked-by: Dinh Nguyen <dinguyen@kernel.org>

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

end of thread, other threads:[~2022-05-23 13:58 UTC | newest]

Thread overview: 34+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-04-13 11:54 [PATCH v4 00/11] archs/random: fallback to best raw ktime when no cycle counter Jason A. Donenfeld
2022-04-13 11:54 ` [PATCH v4 01/11] timekeeping: add raw clock fallback for random_get_entropy() Jason A. Donenfeld
2022-04-13 14:32   ` Rob Herring
2022-04-13 22:38     ` Jason A. Donenfeld
2022-04-14 20:41       ` Rob Herring
2022-04-14 21:49         ` H. Peter Anvin
2022-04-14 10:12   ` Russell King (Oracle)
2022-04-14 11:56     ` Jason A. Donenfeld
2022-04-13 11:54 ` [PATCH v4 02/11] m68k: use fallback for random_get_entropy() instead of zero Jason A. Donenfeld
2022-04-13 11:54 ` [PATCH v4 03/11] riscv: " Jason A. Donenfeld
2022-04-13 14:40   ` Rob Herring
2022-04-13 22:40     ` Jason A. Donenfeld
2022-04-13 11:54 ` [PATCH v4 04/11] mips: " Jason A. Donenfeld
2022-04-13 12:25   ` Thomas Bogendoerfer
2022-04-13 12:46     ` Maciej W. Rozycki
2022-04-13 22:35       ` Jason A. Donenfeld
2022-04-14  1:16         ` Maciej W. Rozycki
2022-04-14  9:27           ` Jason A. Donenfeld
2022-04-15 12:26             ` Maciej W. Rozycki
2022-04-16 11:09               ` Jason A. Donenfeld
2022-04-16 14:44                 ` Maciej W. Rozycki
2022-04-16 22:54                   ` Jason A. Donenfeld
2022-04-18  7:10               ` Thomas Bogendoerfer
2022-04-23 23:33                 ` Maciej W. Rozycki
2022-04-24  8:15                   ` Jason A. Donenfeld
2022-04-24 10:51                     ` Maciej W. Rozycki
2022-04-13 11:54 ` [PATCH v4 05/11] arm: " Jason A. Donenfeld
2022-04-13 11:54 ` [PATCH v4 06/11] nios2: " Jason A. Donenfeld
2022-05-23 13:58   ` Dinh Nguyen
2022-04-13 11:54 ` [PATCH v4 07/11] x86: " Jason A. Donenfeld
2022-04-13 11:54 ` [PATCH v4 08/11] um: " Jason A. Donenfeld
2022-04-13 11:54 ` [PATCH v4 09/11] sparc: " Jason A. Donenfeld
2022-04-13 11:54 ` [PATCH v4 10/11] xtensa: " Jason A. Donenfeld
2022-04-13 11:54 ` [PATCH v4 11/11] random: insist on random_get_entropy() existing in order to simplify Jason A. Donenfeld

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).