Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Tue, 1 Nov 2022 14:02:58 +0100
From:   "Jason A. Donenfeld" <Jason@zx2c4.com>
To:     Mark Rutland <mark.rutland@arm.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
Message-ID: <Y2EZAsRA8uS+ppnn@zx2c4.com>
References: <20221101115616.232884-1-Jason@zx2c4.com>
 <20221101122527.323843-1-Jason@zx2c4.com>
 <Y2ESilMCF9eeffW6@FVFF77S0Q05N>
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
In-Reply-To: <Y2ESilMCF9eeffW6@FVFF77S0Q05N>
Precedence: bulk

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.

Jason