[PATCH RFC v1 00/10] archs/random: fallback to using sched_clock() if no cycle counter

[PATCH RFC v1 00/10] archs/random: fallback to using sched_clock() if no cycle counter

[Jason A. Donenfeld]

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. In that case, what are we left
with?

By my first guess, we have ktime_get_boottime_ns(), jiffies, and
sched_clock(). It seems like sched_clock() has already done a lot of
work in being always available with some incrementing value, falling
back to jiffies as necessary. So this series goes with that as a
fallback, for when the architecture doesn't define random_get_entropy in
its own way and when there's no working cycle counter.

Another option would be falling back to different things on different
platforms. For example, Arnd mentioned to me that on m68k,
ktime_get_ns() might be better than sched_clock(), because it doesn't
use CONFIG_GENERIC_SCHED_CLOCK and therefore is only as good as jiffies.
Maybe there are other considerations there as well.

This is a bit involved with plumbing asm/ headers, which is why this is
an RFC. There are a few ways of skinning that cat. The patchset also
tries to fill in the various cases where an arch only sometimes has a
cycle counter and sometimes doesn't. When possible, it tries to make the
decisions at compile time, but sometimes runtime decisions are
necessary.

Please let me know if you think this is sane. And if you have a
different candidate than sched_clock(), I'd be interested to learn about
that. In particular, I'd value input here from Thomas or somebody else
who has looked at timekeeping across less common platforms.

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.

Thanks,
Jason

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: Thomas Gleixner <tglx@linutronix.de>
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: 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 (10):
  random: use sched_clock() for random_get_entropy() if no get_cycles()
  m68k: use sched_clock() for random_get_entropy() instead of zero
  riscv: use sched_clock() for random_get_entropy() instead of zero
  mips: use sched_clock() for random_get_entropy() instead of zero
  arm: use sched_clock() for random_get_entropy() instead of zero
  x86: use sched_clock() for random_get_entropy() instead of zero
  arm64: use sched_clock() for random_get_entropy() instead of zero
  um: use sched_clock() for random_get_entropy() instead of zero
  sparc: use sched_clock() for random_get_entropy() instead of zero
  xtensa: use sched_clock() for random_get_entropy() instead of zero

 arch/arm/include/asm/timex.h      | 11 +++++++++++
 arch/arm64/include/asm/timex.h    |  9 +++++++++
 arch/m68k/include/asm/timex.h     |  4 +++-
 arch/mips/include/asm/timex.h     |  3 ++-
 arch/riscv/include/asm/timex.h    |  3 ++-
 arch/sparc/include/asm/timex_32.h |  4 +---
 arch/um/include/asm/timex.h       |  9 ++-------
 arch/x86/include/asm/tsc.h        | 11 +++++++++++
 arch/xtensa/include/asm/timex.h   |  6 ++----
 include/linux/timex.h             |  6 ++++++
 10 files changed, 49 insertions(+), 17 deletions(-)

-- 
2.35.1

[PATCH RFC v1 01/10] random: use sched_clock() for random_get_entropy() if no get_cycles()

[Jason A. Donenfeld]

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 sched_clock() would be
preferable, because that always needs to return _something_, even
falling back to jiffies eventually. It's not as though sched_clock() 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 | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/include/linux/timex.h b/include/linux/timex.h
index 5745c90c8800..bd78f784762e 100644
--- a/include/linux/timex.h
+++ b/include/linux/timex.h
@@ -61,6 +61,7 @@
 #include <linux/compiler.h>
 #include <linux/types.h>
 #include <linux/param.h>
+#include <linux/sched/clock.h>
 
 #include <asm/timex.h>
 
@@ -74,8 +75,13 @@
  *
  * 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 sched_clock().
  */
+#ifdef get_cycles
 #define random_get_entropy()	((unsigned long)get_cycles())
+#else
+#define random_get_entropy()	((unsigned long)sched_clock())
 #endif
 
 /*
-- 
2.35.1

[PATCH RFC v1 02/10] m68k: use sched_clock() for random_get_entropy() instead of zero

[Jason A. Donenfeld]

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 sched_clock() would be preferable, because
that always needs to return _something_, even falling back to jiffies
eventually. It's not as though sched_clock() 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: Arnd Bergmann <arnd@arndb.de>
Cc: Geert Uytterhoeven <geert@linux-m68k.org>
Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
---
 arch/m68k/include/asm/timex.h | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/arch/m68k/include/asm/timex.h b/arch/m68k/include/asm/timex.h
index 6a21d9358280..2cd0942097f8 100644
--- a/arch/m68k/include/asm/timex.h
+++ b/arch/m68k/include/asm/timex.h
@@ -7,6 +7,8 @@
 #ifndef _ASMm68K_TIMEX_H
 #define _ASMm68K_TIMEX_H
 
+#include <linux/sched/clock.h>
+
 #ifdef CONFIG_COLDFIRE
 /*
  * CLOCK_TICK_RATE should give the underlying frequency of the tick timer
@@ -35,7 +37,7 @@ static inline unsigned long random_get_entropy(void)
 {
 	if (mach_random_get_entropy)
 		return mach_random_get_entropy();
-	return 0;
+	return sched_clock();
 }
 #define random_get_entropy	random_get_entropy
 
-- 
2.35.1

[PATCH RFC v1 03/10] riscv: use sched_clock() for random_get_entropy() instead of zero

[Jason A. Donenfeld]

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 sched_clock() would be preferable, because
that always needs to return _something_, even falling back to jiffies
eventually. It's not as though sched_clock() 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: 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 | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/arch/riscv/include/asm/timex.h b/arch/riscv/include/asm/timex.h
index 507cae273bc6..5b802755ca3a 100644
--- a/arch/riscv/include/asm/timex.h
+++ b/arch/riscv/include/asm/timex.h
@@ -7,6 +7,7 @@
 #define _ASM_RISCV_TIMEX_H
 
 #include <asm/csr.h>
+#include <linux/sched/clock.h>
 
 typedef unsigned long cycles_t;
 
@@ -41,7 +42,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 sched_clock();
 	return get_cycles();
 }
 #define random_get_entropy()	random_get_entropy()
-- 
2.35.1

[PATCH RFC v1 04/10] mips: use sched_clock() for random_get_entropy() instead of zero

[Jason A. Donenfeld]

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 sched_clock() would be preferable, because
that always needs to return _something_, even falling back to jiffies
eventually. It's not as though sched_clock() 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: 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 | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/arch/mips/include/asm/timex.h b/arch/mips/include/asm/timex.h
index b05bb70a2e46..1de8ded08bb7 100644
--- a/arch/mips/include/asm/timex.h
+++ b/arch/mips/include/asm/timex.h
@@ -12,6 +12,7 @@
 #ifdef __KERNEL__
 
 #include <linux/compiler.h>
+#include <linux/sched/clock.h>
 
 #include <asm/cpu.h>
 #include <asm/cpu-features.h>
@@ -94,7 +95,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 sched_clock();	/* no usable register */
 }
 #define random_get_entropy random_get_entropy
 
-- 
2.35.1

[PATCH RFC v1 05/10] arm: use sched_clock() for random_get_entropy() instead of zero

[Jason A. Donenfeld]

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 sched_clock() would be preferable, because
that always needs to return _something_, even falling back to jiffies
eventually. It's not as though sched_clock() 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: 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 | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/arch/arm/include/asm/timex.h b/arch/arm/include/asm/timex.h
index 7c3b3671d6c2..1c51580ee55d 100644
--- a/arch/arm/include/asm/timex.h
+++ b/arch/arm/include/asm/timex.h
@@ -9,7 +9,18 @@
 #ifndef _ASMARM_TIMEX_H
 #define _ASMARM_TIMEX_H
 
+#include <linux/sched/clock.h>
+
 typedef unsigned long cycles_t;
 #define get_cycles()	({ cycles_t c; read_current_timer(&c) ? 0 : c; })
 
+static inline unsigned long random_get_entropy(void)
+{
+	unsigned long ret = get_cycles();
+	if (ret)
+		return ret;
+	return sched_clock();
+}
+#define random_get_entropy random_get_entropy
+
 #endif
-- 
2.35.1

[PATCH RFC v1 06/10] x86: use sched_clock() for random_get_entropy() instead of zero

[Jason A. Donenfeld]

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 sched_clock() would be preferable, because
that always needs to return _something_, even falling back to jiffies
eventually. It's not as though sched_clock() 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: Arnd Bergmann <arnd@arndb.de>
Cc: x86@kernel.org
Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
---
 arch/x86/include/asm/tsc.h | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/arch/x86/include/asm/tsc.h b/arch/x86/include/asm/tsc.h
index 01a300a9700b..b0c0b2b9e0f7 100644
--- a/arch/x86/include/asm/tsc.h
+++ b/arch/x86/include/asm/tsc.h
@@ -5,6 +5,7 @@
 #ifndef _ASM_X86_TSC_H
 #define _ASM_X86_TSC_H
 
+#include <linux/sched/clock.h>
 #include <asm/processor.h>
 #include <asm/cpufeature.h>
 
@@ -28,6 +29,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 sched_clock();
+#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

[PATCH RFC v1 07/10] arm64: use sched_clock() for random_get_entropy() instead of zero

[Jason A. Donenfeld]

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 sched_clock() would be preferable, because
that always needs to return _something_, even falling back to jiffies
eventually. It's not as though sched_clock() 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_ARM_ARCH_TIMER=n, then get_cycles() will return 0, so we only
need the fallback code for that case.

Cc: Arnd Bergmann <arnd@arndb.de>
Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: Will Deacon <will@kernel.org>
Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
---
 arch/arm64/include/asm/timex.h | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/arch/arm64/include/asm/timex.h b/arch/arm64/include/asm/timex.h
index cf59ce91b22d..bfebd2e1ce45 100644
--- a/arch/arm64/include/asm/timex.h
+++ b/arch/arm64/include/asm/timex.h
@@ -13,6 +13,15 @@
  */
 #define get_cycles()	arch_timer_read_counter()
 
+#ifndef CONFIG_ARM_ARCH_TIMER
+/*
+ * The default implementation of random_get_entropy() calls get_cycles(),
+ * which will return 0 if CONFIG_ARM_ARCH_TIMER=n, so we fall back to
+ * sched_clock() here. Not a great situation, but better than nothing.
+ */
+#define random_get_entropy() ((unsigned long)sched_clock())
+#endif
+
 #include <asm-generic/timex.h>
 
 #endif
-- 
2.35.1

[PATCH RFC v1 08/10] um: use sched_clock() for random_get_entropy() instead of zero

[Jason A. Donenfeld]

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 sched_clock() would be preferable, because
that always needs to return _something_, even falling back to jiffies
eventually. It's not as though sched_clock() 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: 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

[PATCH RFC v1 09/10] sparc: use sched_clock() for random_get_entropy() instead of zero

[Jason A. Donenfeld]

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 sched_clock() would be preferable, because
that always needs to return _something_, even falling back to jiffies
eventually. It's not as though sched_clock() 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: 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

[PATCH RFC v1 10/10] xtensa: use sched_clock() for random_get_entropy() instead of zero

[Jason A. Donenfeld]

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 sched_clock() would be preferable, because
that always needs to return _something_, even falling back to jiffies
eventually. It's not as though sched_clock() 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: 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

Re: [PATCH RFC v1 07/10] arm64: use sched_clock() for random_get_entropy() instead of zero

[Mark Rutland]

On Fri, Apr 08, 2022 at 08:21:42PM +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 sched_clock() would be preferable, because
> that always needs to return _something_, even falling back to jiffies
> eventually. It's not as though sched_clock() 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_ARM_ARCH_TIMER=n, then get_cycles() will return 0, so we only
> need the fallback code for that case.

In arch/arm64/Kconfig we unconditionally select CONFIG_ARM_ARCH_TIMER,
so that configuration shouldn't be possible, and I don't think this
patch is necessary.

On arm64 we depend on the architected timer in a bunch of places, so
anyone hacking that out has bigger problems.

Thanks,
Mark.

> 
> Cc: Arnd Bergmann <arnd@arndb.de>
> Cc: Catalin Marinas <catalin.marinas@arm.com>
> Cc: Will Deacon <will@kernel.org>
> Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
> ---
>  arch/arm64/include/asm/timex.h | 9 +++++++++
>  1 file changed, 9 insertions(+)
> 
> diff --git a/arch/arm64/include/asm/timex.h b/arch/arm64/include/asm/timex.h
> index cf59ce91b22d..bfebd2e1ce45 100644
> --- a/arch/arm64/include/asm/timex.h
> +++ b/arch/arm64/include/asm/timex.h
> @@ -13,6 +13,15 @@
>   */
>  #define get_cycles()	arch_timer_read_counter()
>  
> +#ifndef CONFIG_ARM_ARCH_TIMER
> +/*
> + * The default implementation of random_get_entropy() calls get_cycles(),
> + * which will return 0 if CONFIG_ARM_ARCH_TIMER=n, so we fall back to
> + * sched_clock() here. Not a great situation, but better than nothing.
> + */
> +#define random_get_entropy() ((unsigned long)sched_clock())
> +#endif
> +
>  #include <asm-generic/timex.h>
>  
>  #endif
> -- 
> 2.35.1
> 
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH RFC v1 07/10] arm64: use sched_clock() for random_get_entropy() instead of zero

[Jason A. Donenfeld]

Hi Mark,

On Fri, Apr 08, 2022 at 07:33:53PM +0100, Mark Rutland wrote:
> On Fri, Apr 08, 2022 at 08:21:42PM +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 sched_clock() would be preferable, because
> > that always needs to return _something_, even falling back to jiffies
> > eventually. It's not as though sched_clock() 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_ARM_ARCH_TIMER=n, then get_cycles() will return 0, so we only
> > need the fallback code for that case.
> 
> In arch/arm64/Kconfig we unconditionally select CONFIG_ARM_ARCH_TIMER,
> so that configuration shouldn't be possible, and I don't think this
> patch is necessary.
> 
> On arm64 we depend on the architected timer in a bunch of places, so
> anyone hacking that out has bigger problems.

Thanks for the tip. I'll drop this patch from the series.

Jason

Re: [PATCH RFC v1 00/10] archs/random: fallback to using sched_clock() if no cycle counter

[Thomas Gleixner]

Jason,

On Fri, Apr 08 2022 at 20:21, Jason A. Donenfeld wrote:
> 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. In that case, what are we left
> with?
>
> By my first guess, we have ktime_get_boottime_ns(), jiffies, and
> sched_clock(). It seems like sched_clock() has already done a lot of
> work in being always available with some incrementing value, falling
> back to jiffies as necessary. So this series goes with that as a
> fallback, for when the architecture doesn't define random_get_entropy in
> its own way and when there's no working cycle counter.

sched_clock() is a halfways sane option, but yes as Arnd pointed out:

> Another option would be falling back to different things on different
> platforms. For example, Arnd mentioned to me that on m68k,
> ktime_get_ns() might be better than sched_clock(), because it doesn't
> use CONFIG_GENERIC_SCHED_CLOCK and therefore is only as good as
> jiffies.

ktime_get_ns() or the slightly faster variant ktime_get_mono_fast_ns()
are usable. In the worst case they are jiffies driven too, but systems
with jiffies clocksource are pretty much museum pieces.

It's slightly slower than get_cycles() and a get_cyles() based
sched_clock(), but you get the most granular clock of the platform
automatically, which has it's charm too :)

The bad news is that depending on the clocksource frequency the lower
bits might never change. Always true for clocksource jiffies.
sched_clock() has the same issue.

But the below uncompiled hack gives you access to the 'best' clocksource
of a machine, i.e. the one which the platform decided to be the one
which is giving the best resolution. The minimal bitwidth of that is
AFAICT 20 bits. In the jiffies case this will at least advance every
tick.

The price, e.g. on x86 would be that RDTSC would be invoked via an
indirect function call. Not the end of the world...

Thanks,

        tglx
---
--- a/kernel/time/timekeeping.c
+++ b/kernel/time/timekeeping.c
@@ -646,6 +646,11 @@ static void halt_fast_timekeeper(const s
 	update_fast_timekeeper(&tkr_dummy, &tk_fast_raw);
 }
 
+u32 ktime_read_raw_clock(void)
+{
+	return tk_clock_read(&tk_core.timekeeper.tkr_mono);
+}
+
 static RAW_NOTIFIER_HEAD(pvclock_gtod_chain);
 
 static void update_pvclock_gtod(struct timekeeper *tk, bool was_set)

Re: [PATCH RFC v1 00/10] archs/random: fallback to using sched_clock() if no cycle counter

[Jason A. Donenfeld]

Hi Thomas,

On Sun, Apr 10, 2022 at 01:29:32AM +0200, Thomas Gleixner wrote:
> But the below uncompiled hack gives you access to the 'best' clocksource
> of a machine, i.e. the one which the platform decided to be the one
> which is giving the best resolution. The minimal bitwidth of that is
> AFAICT 20 bits. In the jiffies case this will at least advance every
> tick.

Oh, huh, that's pretty cool. I can try to make a commit out of that. Are
you suggesting I use this as the fallback for all platforms that
currently return zero, or just for m68k per Arnd's suggestion, and then
use sched_clock() for the others? It sounds to me like you're saying
this would be best for all of them. If so, that'd be quite nice.


> The price, e.g. on x86 would be that RDTSC would be invoked via an
> indirect function call. Not the end of the world...

Well on x86, random_get_entropy() is overridden in the arch/ code to
call get_cycles(). So this would really just be for 486 and for other
architectures with no cycle counter that are currently returning zero.
However, this brings up a good point: if your proposed
ktime_read_raw_clock() function really is so nice, should it be used
everywhere unconditionally with no arch-specific overrides? On x86, is
it really guaranteed to be RDTSC, and not, say, some off-core HPET
situation? And is this acceptable to call from the hard irq handler?

Not yet having too much knowledge, I'm tentatively leaning toward the
safe side, of just using ktime_read_raw_clock() in the current places
that return zero all the time -- that is, for the purpose this patchset
has.

Jason

Re: [PATCH RFC v1 00/10] archs/random: fallback to using sched_clock() if no cycle counter

[Thomas Gleixner]

Jason,

On Sun, Apr 10 2022 at 16:25, Jason A. Donenfeld wrote:
> On Sun, Apr 10, 2022 at 01:29:32AM +0200, Thomas Gleixner wrote:
>> But the below uncompiled hack gives you access to the 'best' clocksource
>> of a machine, i.e. the one which the platform decided to be the one
>> which is giving the best resolution. The minimal bitwidth of that is
>> AFAICT 20 bits. In the jiffies case this will at least advance every
>> tick.
>
> Oh, huh, that's pretty cool. I can try to make a commit out of that. Are
> you suggesting I use this as the fallback for all platforms that
> currently return zero, or just for m68k per Arnd's suggestion, and then
> use sched_clock() for the others? It sounds to me like you're saying
> this would be best for all of them. If so, that'd be quite nice.

It's the best in terms of timekeeping. Not the fastest :)

>> The price, e.g. on x86 would be that RDTSC would be invoked via an
>> indirect function call. Not the end of the world...
>
> Well on x86, random_get_entropy() is overridden in the arch/ code to
> call get_cycles(). So this would really just be for 486 and for other
> architectures with no cycle counter that are currently returning zero.
> However, this brings up a good point: if your proposed
> ktime_read_raw_clock() function really is so nice, should it be used
> everywhere unconditionally with no arch-specific overrides? On x86, is
> it really guaranteed to be RDTSC, and not, say, some off-core HPET
> situation? And is this acceptable to call from the hard irq handler?

No, that's the sad part. On system where TSC is unstable (for whatever
reason) this might fallback to some off-core clock (HPET, PMTIMER).
The good news is that this is mostly affecting older systems. After 20+
years of complaining the hardware people seem to have figured out that a
fast accessible and realiable clocksource is something useful. :)

> Not yet having too much knowledge, I'm tentatively leaning toward the
> safe side, of just using ktime_read_raw_clock() in the current places
> that return zero all the time -- that is, for the purpose this patchset
> has.

That's probably a good approach and it's init/runtime discoverable.

Thanks,

        tglx

Re: [PATCH RFC v1 00/10] archs/random: fallback to using sched_clock() if no cycle counter

[Jason A. Donenfeld]

Hi Thomas,

On Sun, Apr 10, 2022 at 10:57 PM Thomas Gleixner <tglx@linutronix.de> wrote:
> > Not yet having too much knowledge, I'm tentatively leaning toward the
> > safe side, of just using ktime_read_raw_clock() in the current places
> > that return zero all the time -- that is, for the purpose this patchset
> > has.
>
> That's probably a good approach and it's init/runtime discoverable.

Alright then. I'll send out a v2 and we'll see how that looks.

Jason

Re: [PATCH RFC v1 00/10] archs/random: fallback to using sched_clock() if no cycle counter

[Eric Biggers]

On Fri, Apr 08, 2022 at 08:21:35PM +0200, Jason A. Donenfeld wrote:
> By my first guess, we have ktime_get_boottime_ns(), jiffies, and
> sched_clock(). It seems like sched_clock() has already done a lot of
> work in being always available with some incrementing value, falling
> back to jiffies as necessary. So this series goes with that as a
> fallback, for when the architecture doesn't define random_get_entropy in
> its own way and when there's no working cycle counter.

Won't this interact badly with how try_to_generate_entropy() (a.k.a. the "Linus
Jitter Dance") detects the presence of an appropriate timer currently?

        stack.cycles = random_get_entropy();

        /* Slow counter - or none. Don't even bother */
        if (stack.cycles == random_get_entropy())
                return;

So if random_get_entropy() always returns 0, then try_to_generate_entropy()
won't run.  However, if random_get_entropy() is even just a low-precision timer,
then try_to_generate_entropy() will have a chance of running, since the timer
might change between the two calls to random_get_entropy().  And if
try_to_generate_entropy() does run, then it credits 1 bit of entropy for every
iteration, regardless of the timer's precision.

This is an existing problem, but this patchset will make it worse, as it changes
a lot of cases from "no timer" to "low precision timer".

Perhaps try_to_generate_entropy() should check the timer at least 3 times and
verify that it changed each time?

- Eric

Re: [PATCH RFC v1 00/10] archs/random: fallback to using sched_clock() if no cycle counter

[Jason A. Donenfeld]

Hi Eric,

On 4/11/22, Eric Biggers <ebiggers@kernel.org> wrote:
> On Fri, Apr 08, 2022 at 08:21:35PM +0200, Jason A. Donenfeld wrote:
>> By my first guess, we have ktime_get_boottime_ns(), jiffies, and
>> sched_clock(). It seems like sched_clock() has already done a lot of
>> work in being always available with some incrementing value, falling
>> back to jiffies as necessary. So this series goes with that as a
>> fallback, for when the architecture doesn't define random_get_entropy in
>> its own way and when there's no working cycle counter.
>
> Won't this interact badly with how try_to_generate_entropy() (a.k.a. the
> "Linus
> Jitter Dance") detects the presence of an appropriate timer currently?
>
>         stack.cycles = random_get_entropy();
>
>         /* Slow counter - or none. Don't even bother */
>         if (stack.cycles == random_get_entropy())
>                 return;
>
> So if random_get_entropy() always returns 0, then try_to_generate_entropy()
> won't run.  However, if random_get_entropy() is even just a low-precision
> timer,
> then try_to_generate_entropy() will have a chance of running, since the
> timer
> might change between the two calls to random_get_entropy().  And if
> try_to_generate_entropy() does run, then it credits 1 bit of entropy for
> every
> iteration, regardless of the timer's precision.
>
> This is an existing problem, but this patchset will make it worse, as it
> changes
> a lot of cases from "no timer" to "low precision timer".
>
> Perhaps try_to_generate_entropy() should check the timer at least 3 times
> and
> verify that it changed each time?

What you've identified is actually already the case for platforms
where the cycle counter is already just slow (and there are a few such
platforms; my odroid C2 even exhibits this). As you identified, the
cycle counter might already be too slow, yet we get [un]lucky and
reach this code right on the cusp or a change.

So the problem isn't new here, per say, for this patchset. But indeed
perhaps we should consider adjusting the heuristics for that a bit in
a separate patch. Your check three times idea seems like a good
starting point, if you want to send a patch and we can poke at it.

Jason