Re: [PATCH v10 2/3] arm64: random: Add data to pool from setup_arch()

From: Mark Rutland <mark.rutland@arm.com>
To: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: Catalin Marinas <catalin.marinas@arm.com>,
	Richard Henderson <richard.henderson@linaro.org>,
	Mark Brown <broonie@kernel.org>, Will Deacon <will@kernel.org>,
	linux-arm-kernel <linux-arm-kernel@lists.infradead.org>
Subject: Re: [PATCH v10 2/3] arm64: random: Add data to pool from setup_arch()
Date: Wed, 15 Jan 2020 10:11:08 +0000	[thread overview]
Message-ID: <20200115101107.GA32549@lakrids.cambridge.arm.com> (raw)
In-Reply-To: <CAKv+Gu9=L6hPSHbvf1qHa7N9hyJ7m7KSYJHBf1em8E0Db_Lghw@mail.gmail.com>

On Wed, Jan 15, 2020 at 10:22:03AM +0100, Ard Biesheuvel wrote:
> On Wed, 15 Jan 2020 at 10:16, Will Deacon <will@kernel.org> wrote:
> >
> > On Wed, Jan 15, 2020 at 08:48:46AM +0100, Ard Biesheuvel wrote:
> > > On Fri, 10 Jan 2020 at 13:23, Mark Brown <broonie@kernel.org> wrote:
> > > >
> > > > Since the arm64 ARCH_RANDOM implementation is not available until
> > > > cpufeature has determined the system capabilities it can't be used by
> > > > the generic random code to initialize the entropy pool for early use.
> > > > Instead explicitly add some data to the pool from setup_arch() if the
> > > > boot CPU supports v8.5-RNG, this is the point recommended by the generic
> > > > code.
> > > >
> > > > Note that we are only adding data here, it will be mixed into the pool
> > > > but won't be credited as entropy. There are currently no suitable
> > > > interfaces for that at present - extending the random code to provide
> > > > those will be done as a future step. Providing data is better than not
> > > > doing so as it will still provide an increase in variation in the output
> > > > from the random code and there will be no impact on the rate at which
> > > > entropy is credited compared to what we have without this patch.
> > > >
> > >
> > > This is slightly unfortunate, as this way, we lose the ability to use
> > > random.trust_cpu=1 to get the entropy credited and initialize CRNG
> > > early.
> >
> > Agreed. Do you think we should wait for that support before merging the
> > series? Given that I don't know of any CPUs implementing this extension,
> > we can probably afford not to rush this in.
> 
> In a previous iteration, we did have a functional
> arch_get_random_seed_long() early on, which would solve this issue
> without even needing a patch like this.
> 
> Perhaps Mark (Rutland) can give a recap of his concerns at the time?

It meant that the common runtime path had code that was only ever meant
to run at boot time, and would also run on secondary CPUs until we
finalized the caps, so they'd behave inconsistently across boot and
hotplug paths. I was concerned that this was messy and would be painful
to reason about and debug.

My suggestion was that we either:

(a) Had the arch code explicitly inject the entropy in the primary setup
    path, as these patches do, or;

(b) Had a new callback (e.g. __early_arch_get_random_seed_long()) that
    the core random code only called during its initialization, separate
    to the runtime paths.

Thanks,
Mark.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel