Re: [PATCH v4 2/3] hwrng: add Rockchip SoC hwrng driver

From: Aurelien Jarno <aurelien@aurel32.net>
To: Krzysztof Kozlowski <krzk@kernel.org>
Cc: "Daniel Golle" <daniel@makrotopia.org>,
	"Olivia Mackall" <olivia@selenic.com>,
	"Herbert Xu" <herbert@gondor.apana.org.au>,
	"Rob Herring" <robh@kernel.org>,
	"Krzysztof Kozlowski" <krzk+dt@kernel.org>,
	"Conor Dooley" <conor+dt@kernel.org>,
	"Heiko Stuebner" <heiko@sntech.de>,
	"Philipp Zabel" <p.zabel@pengutronix.de>,
	"Uwe Kleine-König" <ukleinek@debian.org>,
	"Sebastian Reichel" <sebastian.reichel@collabora.com>,
	"Sascha Hauer" <s.hauer@pengutronix.de>,
	"Dragan Simic" <dsimic@manjaro.org>,
	"Martin Kaiser" <martin@kaiser.cx>,
	"Ard Biesheuvel" <ardb@kernel.org>,
	linux-crypto@vger.kernel.org, devicetree@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org,
	linux-rockchip@lists.infradead.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH v4 2/3] hwrng: add Rockchip SoC hwrng driver
Date: Sun, 23 Jun 2024 12:42:45 +0200	[thread overview]
Message-ID: <Znf8JeBA4mzVa0V1@aurel32.net> (raw)
In-Reply-To: <612bd49c-c44a-41f2-89e9-c96e62e52a0a@kernel.org>

Hi,

On 2024-06-23 09:00, Krzysztof Kozlowski wrote:
> On 23/06/2024 05:33, Daniel Golle wrote:
> > +
> > +	rk_rng->rng.name = dev_driver_string(dev);
> > +#ifndef CONFIG_PM
> > +	rk_rng->rng.init = rk_rng_init;
> > +	rk_rng->rng.cleanup = rk_rng_cleanup;
> > +#endif
> > +	rk_rng->rng.read = rk_rng_read;
> > +	rk_rng->rng.priv = (unsigned long) dev;
> > +	rk_rng->rng.quality = 900;
> 
> I doubt in this value. Usually SoC vendors do not provide datasheet with
> any reliable and verifiable (so one which could be proven by 3rd party)
> information. Can you provide a source? (and vendor downstream tree does
> not really count)

As the original author of the patch, I am the one who have chosen the
value. I did it as explained in the commit message:

| The TRNG device does not seem to have a signal conditionner and the FIPS
| 140-2 test returns a lot of failures. They can be reduced by increasing
| RK_RNG_SAMPLE_CNT, in a tradeoff between quality and speed. This value
| has been adjusted to get ~90% of successes and the quality value has
| been set accordingly.

It is also explained, admittedly more briefly, above the
RK_RNG_SAMPLE_CNT #define, as the commit messages are not really
relevant anymore once the patches are accepted:

| * TRNG collects osc ring output bit every RK_RNG_SAMPLE_CNT time. The value is
| * a tradeoff between speed and quality and has been adjusted to get a quality
| * of ~900 (~90% of FIPS 140-2 successes).
| */

The decision to adjust RK_RNG_SAMPLE_CNT to reach ~90% of FIPS 140-2
successes was based on the quality chosen by most hw_random drivers
currently in the kernel sources. The FIPS 140-2 tests were performed
using rngtest from the rng-tools project.

All that said, I am not an expert in that domain, so feel free to point
to the documentation or provide the correct method to determine the
quality.

Regards
Aurelien

[1] https://git.kernel.org/pub/scm/utils/kernel/rng-tools/rng-tools.git/

-- 
Aurelien Jarno                          GPG: 4096R/1DDD8C9B
aurelien@aurel32.net                     http://aurel32.net