Received-SPF: softfail (google.com: domain of transitioning linux-crypto-owner@vger.kernel.org does not designate 23.128.96.19 as permitted sender) client-ip=23.128.96.19;
Date:   Wed, 27 Apr 2022 03:29:49 +0200
From:   "Jason A. Donenfeld" <Jason@zx2c4.com>
To:     "Maciej W. Rozycki" <macro@orcam.me.uk>
Cc:     linux-kernel@vger.kernel.org, linux-crypto@vger.kernel.org,
        tglx@linutronix.de, arnd@arndb.de,
        Thomas Bogendoerfer <tsbogend@alpha.franken.de>
Subject: Re: [PATCH v6 09/17] mips: use fallback for random_get_entropy()
 instead of just c0 random
Message-ID: <YmicjVbkcppfzE1E@zx2c4.com>
References: <20220423212623.1957011-1-Jason@zx2c4.com>
 <20220423212623.1957011-10-Jason@zx2c4.com>
 <alpine.DEB.2.21.2204250113440.9383@angie.orcam.me.uk>
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
In-Reply-To: <alpine.DEB.2.21.2204250113440.9383@angie.orcam.me.uk>
Precedence: bulk

Hey Maciej,

On Tue, Apr 26, 2022 at 09:28:39PM +0100, Maciej W. Rozycki wrote:
> On Sat, 23 Apr 2022, Jason A. Donenfeld wrote:
> 
> > diff --git a/arch/mips/include/asm/timex.h b/arch/mips/include/asm/timex.h
> > index b05bb70a2e46..8cfa485d19e6 100644
> > --- a/arch/mips/include/asm/timex.h
> > +++ b/arch/mips/include/asm/timex.h
> [...]
> > +	if (cpu_has_3kex)
> > +		c0_random = (read_c0_random() >> 8) & 0x3f;
> 
>  Hmm, I wonder whether we do need to mask the contents of the register out 
> here given that known implementations return zeros in reserved bits.  Even 
> though R3000 documentation I have access to makes no guarantee as to the 
> values of the reserved bits here I think we can safely proceed according 
> to what systems we do actually support do (even though it only saves one 
> instruction).
> 
> >  	else
> > -		return 0;	/* no usable register */
> > +		c0_random = read_c0_random() & 0x3f;
> 
>  Here the architecture guarantees unused bits to be zero, but the number 
> of them varies between implementations.  However we'll only ever use this 
> leg for the R4000/R4400 processors, which have 48 TLB entries, and for the 
> Ingenic XBurst cores, which I have now verified in documentation (which 
> user-reported dumps from /proc/cpuinfo are consistent with) that have 32 
> TLB entries.  So I think this mask operation can go as well.
> 
>  I guess these updates can be made with a follow-up change though.

There is lots of optimization potential on a few fronts we've identified
in this thread. Let's save these for a follow-up. I'd rather this
initial one be at least somewhat simple, so that as it gets optimized,
it'll be easy to handle regressions. Also, it probably makes sense for
you to send the patches for these, since you have both the hardware
chops and the hardware itself to assess these ideas. I am interested in
the topic though, so please do CC me.

Jason