On Sat, Apr 23 2022 at 23:26, Jason A. Donenfeld wrote: Please follow the guidelines of the tip maintainers when touching x86 code. https://www.kernel.org/doc/html/latest/process/maintainer-tip.html#patch-subject > 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. https://www.kernel.org/doc/html/latest/process/maintainer-tip.html#changelog > +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 noticable. We surely can think about making TSC mandatory, but not selectively in a particalur context. Thanks, tglx
