Re: [PATCH v6 13/17] x86: use fallback for random_get_entropy() instead of zero

From: Thomas Gleixner
Date: Mon Apr 25 2022 - 08:35:52 EST

On Sat, Apr 23 2022 at 23:26, Jason A. Donenfeld wrote:

Please follow the guidelines of the tip maintainers when touching x86

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

There are also 586 CPUs which lack TSC.

> While we're at it, fix up both the new function and the get_cycles()
> function from which its derived to use cpu_feature_enabled() rather
> than boot_cpu_has().

Lot's of 'we' here. We are not doing anything.

> +static inline unsigned long random_get_entropy(void)
> +{
> +#ifndef CONFIG_X86_TSC
> + if (!cpu_feature_enabled(X86_FEATURE_TSC))
> + return random_get_entropy_fallback();
> +#endif

Please get rid of this ifdeffery. While you are right, that anything
with CONFIG_X86_TSC=y should have a TSC, there is virt ....

cpu_feature_enabled() is runtime patched and only evaluated before
alternative patching, so the win of this ifdef is marginally, if even

We surely can think about making TSC mandatory, but not selectively in a
particalur context.