Re: [PATCH v4 04/11] mips: use fallback for random_get_entropy() instead of zero

Maciej W. Rozycki
2022-04-15T12:26:54Z

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