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 13:07:39 +0000
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
Message-ID: <Y2EaG7Zo2jiyKUWB@FVFF77S0Q05N>
References: <20221101115616.232884-1-Jason@zx2c4.com>
 <20221101122527.323843-1-Jason@zx2c4.com>
 <Y2ESilMCF9eeffW6@FVFF77S0Q05N>
 <Y2EZAsRA8uS+ppnn@zx2c4.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <Y2EZAsRA8uS+ppnn@zx2c4.com>
Precedence: bulk

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.