Re: [PATCH v4 1/1] arm64: Implement archrandom.h for ARMv8.5-RNG

[Ard Biesheuvel <ard.biesheuvel@linaro.org>]

On Fri, 8 Nov 2019 at 14:03, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
>
> On Fri, 8 Nov 2019 at 12:10, Richard Henderson
> <richard.henderson@linaro.org> wrote:
> >
> > On 11/6/19 10:30 PM, Ard Biesheuvel wrote:
> > > On Wed, 6 Nov 2019 at 15:20, Richard Henderson
> > > <richard.henderson@linaro.org> wrote:
> > >> +static inline bool has_random(void)
> > >> +{
> > >> +       /*
> > >> +        * We "have" RNG if either
> > >> +        * (1) every cpu in the system has RNG, or
> > >> +        * (2) in a non-preemptable context, current cpu has RNG.
> > >> +        * Case 1 is the expected case when RNG is deployed, but
> > >> +        * case 2 is present as a backup in case some big/little
> > >> +        * system only has RNG on big cpus, we can still add entropy
> > >> +        * from the interrupt handler of the big cpus.
> > >
> > > I don't understand the reference to the interrupt handler here.
> >
> > To add_interrupt_randomness(), invoked by handle_irq_event_percpu().
> > Better if I reword the above to include the function name?
> >
>
> This is one of the several places where arch_random_get_seed_long() is
> called, so if you are going to single it out like that, it does make
> sense to clarify that.
>

Looking more carefully at that code, it seems we call
arch_get_random_seed_long() NR_CPUS times per second, and assuming
that our RNDRRS sysreg will be reseeded from a resource that is shared
between all the cores, I am now wondering if this is such a good fit
after all, especially in the context of virtualization and
accessibility of both sysregs all the way down to EL0.

I propose we go with RNDR instead, at least for the time being, and
once actual hardware appears, we can try to figure out how these
pieces best fit together.


> > > It is
> > > worth mentioning though that this arrangement permits
> > > rand_initialize() to use the instructions regardless of whether they
> > > are implemented only by the boot CPU or by all of them.
> >
> > Yes, I'll include that.
> >
> >
> > r~