mbox series

[v6,00/17] archs/random: fallback to best raw ktime when no cycle counter

Message ID 20220423212623.1957011-1-Jason@zx2c4.com
Headers show
Series archs/random: fallback to best raw ktime when no cycle counter | expand

Message

Jason A. Donenfeld April 23, 2022, 9:26 p.m. UTC
[Preface for v6: if you're an arch maintainer, a simple Acked-by would
 be appreciated if this looks okay.]

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 C0 random register, which isn't quite 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, OpenRISC, 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 v5->v6:
- Use cpu_feature_enabled() instead of boot_cpu_has() on x86.
- OpenRISC support.
- Define missing `#define get_cycles get_cycles` on various platforms.

Changes v4->v5:
- Do not prototype symbol with 'extern', according to style guide.
- On MIPS, combine random_get_entropy_fallback() with the c0 random
  register in a way that matches the format of the c0 random value, so
  that we get the best of a high precision cycle counter and of larger
  period timer, joined together. As a result, Thomas Bogendoerfer's
  ack on v4 of patch 4 has been dropped, since this is a substantial
  change.

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: Stephen Boyd <sboyd@kernel.org>
Cc: Dinh Nguyen <dinguyen@kernel.org>
Cc: Jonas Bonn <jonas@southpole.se>
Cc: Stefan Kristiansson <stefan.kristiansson@saunalahti.fi>
Cc: Stafford Horne <shorne@gmail.com>
Cc: Heiko Carstens <hca@linux.ibm.com>
Cc: Vasily Gorbik <gor@linux.ibm.com>
Cc: Alexander Gordeev <agordeev@linux.ibm.com>
Cc: Christian Borntraeger <borntraeger@linux.ibm.com>
Cc: Sven Schnelle <svens@linux.ibm.com>
Cc: Helge Deller <deller@gmx.de>
Cc: Richard Henderson <rth@twiddle.net>
Cc: Ivan Kokshaysky <ink@jurassic.park.msu.ru>
Cc: Matt Turner <mattst88@gmail.com>
Cc: Michael Ellerman <mpe@ellerman.id.au>
Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Cc: Paul Mackerras <paulus@samba.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
Cc: openrisc@lists.librecores.org
Cc: linux-ia64@vger.kernel.org
Cc: linux-s390@vger.kernel.org
Cc: linux-parisc@vger.kernel.org
Cc: linuxppc-dev@lists.ozlabs.org

Jason A. Donenfeld (17):
  ia64: define get_cycles macro for arch-override
  s390: define get_cycles macro for arch-override
  parisc: define get_cycles macro for arch-override
  alpha: define get_cycles macro for arch-override
  powerpc: define get_cycles macro for arch-override
  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 just c0 random
  arm: use fallback for random_get_entropy() instead of zero
  openrisc: 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/alpha/include/asm/timex.h    |  1 +
 arch/arm/include/asm/timex.h      |  1 +
 arch/ia64/include/asm/timex.h     |  1 +
 arch/m68k/include/asm/timex.h     |  2 +-
 arch/mips/include/asm/timex.h     | 17 +++---
 arch/nios2/include/asm/timex.h    |  3 ++
 arch/openrisc/include/asm/timex.h |  3 ++
 arch/parisc/include/asm/timex.h   |  3 +-
 arch/powerpc/include/asm/timex.h  |  1 +
 arch/riscv/include/asm/timex.h    |  2 +-
 arch/s390/include/asm/timex.h     |  1 +
 arch/sparc/include/asm/timex_32.h |  4 +-
 arch/um/include/asm/timex.h       |  9 +---
 arch/x86/include/asm/timex.h      | 10 ++++
 arch/x86/include/asm/tsc.h        |  4 +-
 arch/xtensa/include/asm/timex.h   |  6 +--
 drivers/char/random.c             | 89 ++++++++++---------------------
 include/linux/timex.h             |  8 +++
 kernel/time/timekeeping.c         | 10 ++++
 19 files changed, 87 insertions(+), 88 deletions(-)

Comments

Heiko Carstens April 25, 2022, 9:43 a.m. UTC | #1
On Sat, Apr 23, 2022 at 11:26:08PM +0200, Jason A. Donenfeld wrote:
> S390x defines a get_cycles() function, but it forgot to do the usual
> `#define get_cycles get_cycles` dance, making it impossible for generic
> code to see if an arch-specific function was defined.
> 
> Cc: Thomas Gleixner <tglx@linutronix.de>
> Cc: Arnd Bergmann <arnd@arndb.de>
> Cc: Heiko Carstens <hca@linux.ibm.com>
> Cc: Vasily Gorbik <gor@linux.ibm.com>
> Cc: Alexander Gordeev <agordeev@linux.ibm.com>
> Cc: Christian Borntraeger <borntraeger@linux.ibm.com>
> Cc: Sven Schnelle <svens@linux.ibm.com>
> Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
> ---
>  arch/s390/include/asm/timex.h | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/arch/s390/include/asm/timex.h b/arch/s390/include/asm/timex.h
> index 2cfce42aa7fc..ce878e85b6e4 100644
> --- a/arch/s390/include/asm/timex.h
> +++ b/arch/s390/include/asm/timex.h
> @@ -197,6 +197,7 @@ static inline cycles_t get_cycles(void)
>  {
>  	return (cycles_t) get_tod_clock() >> 2;
>  }
> +#define get_cycles get_cycles

As far as I can tell this doesn't change anything, since the
asm-generic timex.h header file is not included/used at all on s390
(and if it would, this would have resulted in a compile error).

FWIW, the compiled code also tells me that the s390 specific
get_cycles() version is already used.

Is any of your subsequent patches making sure that the asm generic
header file gets included everywhere? Otherwise I don't see the point
of this patch.
Jason A. Donenfeld April 25, 2022, 9:48 a.m. UTC | #2
On 4/25/22, Heiko Carstens <hca@linux.ibm.com> wrote:
> Is any of your subsequent patches making sure that the asm generic
> header file gets included everywhere? Otherwise I don't see the point
> of this patch.
>

Yes; patch 6 requires this as a prereq. I'm not doing this arbitrarily.

Jason
Heiko Carstens April 25, 2022, 10:21 a.m. UTC | #3
On Mon, Apr 25, 2022 at 11:48:34AM +0200, Jason A. Donenfeld wrote:
> On 4/25/22, Heiko Carstens <hca@linux.ibm.com> wrote:
> > Is any of your subsequent patches making sure that the asm generic
> > header file gets included everywhere? Otherwise I don't see the point
> > of this patch.
> 
> Yes; patch 6 requires this as a prereq. I'm not doing this arbitrarily.

Ok, that was not obvious to me, especially since I was only cc'ed for
this patch and assumed this was actually a bug fix.
Thanks for clarifying.

Acked-by: Heiko Carstens <hca@linux.ibm.com>