Re: [PATCH v5] random: remove early archrandom abstraction

From: Mark Rutland <mark.rutland@arm.com>
To: "Jason A. Donenfeld" <Jason@zx2c4.com>
Cc: Catalin Marinas <catalin.marinas@arm.com>,
	linux-arm-kernel@lists.infradead.org,
	linux-kernel@vger.kernel.org, Will Deacon <will@kernel.org>,
	Ard Biesheuvel <ardb@kernel.org>,
	Jean-Philippe Brucker <jean-philippe@linaro.org>
Subject: Re: [PATCH v5] random: remove early archrandom abstraction
Date: Tue, 1 Nov 2022 13:07:39 +0000	[thread overview]
Message-ID: <Y2EaG7Zo2jiyKUWB@FVFF77S0Q05N> (raw)
In-Reply-To: <Y2EZAsRA8uS+ppnn@zx2c4.com>

On Tue, Nov 01, 2022 at 02:02:58PM +0100, Jason A. Donenfeld wrote:
> Hi Mark,
> 
> On Tue, Nov 01, 2022 at 12:36:07PM +0000, Mark Rutland wrote:
> > Hi Jason,
> > 
> > Sorry for joining this late...
> > 
> > On Tue, Nov 01, 2022 at 01:25:28PM +0100, Jason A. Donenfeld wrote:
> > > The arch_get_random*_early() abstraction is not completely useful and
> > > adds complexity, because it's not a given that there will be no calls to
> > > arch_get_random*() between random_init_early(), which uses
> > > arch_get_random*_early(), and init_cpu_features(). During that gap,
> > > crng_reseed() might be called, which uses arch_get_random*(), since it's
> > > mostly not init code.
> > 
> > The original rationale for arch_get_random*_early() was just to seed the RNG
> > more robustly rather than to feed every possible arch_get_random() call made
> > early in the boot flow, and the rationale for having a separate functions was
> > that it was trivial to see by inspection that it was (only) called in the
> > expected places.
> > 
> > I'm not wedded to arch_get_random*_early() specifically, but I do think that
> > having arch_get_random() behave differently depending on which phase of boot
> > we're in has more scope for error than having a separate call of some sort.
> > 
> > Other than removing the lines below, what chages is this going to permit?
> 
> Firstly, the issue with the API is having to remember to use it! There's
> already been a bug from forgetting to use the _early() call during some
> refactoring, and I doubt it'll be the last.
> 
> But also, functions such as crng_reseed()->extract_entropy() wind up
> being called in both early contexts and normal contexts. It's not
> feasible to have different paths there, so by having two functions,
> we miss out on having access during early boot.
> 
> So I don't want a separate call, both for the API complexity reasons,
> and because it doesn't really work as intended in the end.

If it's too painful to use separate paths, then fair enough. Thanks for the
summary!

Mark.

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