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
Hi Maciej,
On Fri, Apr 15, 2022 at 2:26 PM Maciej W. Rozycki <[email protected]> wrote:
> 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.
Ah, thanks, will do that.
> 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.
Okay, I can give this a shot, but this certainly isn't my forté. It
may ultimately wind up being simpler for you to just send some code of
what you envision for this, but if I understand your idea correctly,
what you're saying is something like:
static inline unsigned long random_get_entropy(void)
{
unsigned int prid = read_c0_prid();
unsigned int imp = prid & PRID_IMP_MASK;
unsigned int c0_random;
if (can_use_mips_counter(prid))
return read_c0_count();
if (cpu_has_3kex)
c0_random = (read_c0_random() >> 8) & 0x3f;
else
c0_random = read_c0_random() & 0x3f;
return (random_get_entropy_fallback() << 6) | (0x3f - c0_random);
}
What do you think of that? Some tweak I'm missing?
> 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?
It's the same situation without inverting the order: instead of some
bits on the top never happening, some bits on the bottom never happen
instead. In general, counters don't form uniform distributions anyway,
since the lower bits change faster, and neither are they independent,
since one sample in large part depends on the previous. This is just
sort of the nature of the beast, and the code that calls
random_get_entropy() deals with this appropriately (by, at the moment,
just hashing all the bits).
Jason