| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <76091207-d81c-e737-34a2-884153251925@linaro.org>
Date: Tue, 29 Nov 2022 10:33:09 +0100
From: Krzysztof Kozlowski <krzysztof.kozlowski@...aro.org>
To: Aurelien Jarno <aurelien@...el32.net>,
Olivia Mackall <olivia@...enic.com>,
Herbert Xu <herbert@...dor.apana.org.au>,
Rob Herring <robh+dt@...nel.org>,
Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>,
Heiko Stuebner <heiko@...ech.de>,
Philipp Zabel <p.zabel@...gutronix.de>,
Lin Jinhan <troy.lin@...k-chips.com>
Cc: "open list:HARDWARE RANDOM NUMBER GENERATOR CORE"
<linux-crypto@...r.kernel.org>,
"open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS"
<devicetree@...r.kernel.org>,
"moderated list:ARM/Rockchip SoC support"
<linux-arm-kernel@...ts.infradead.org>,
"open list:ARM/Rockchip SoC support"
<linux-rockchip@...ts.infradead.org>,
open list <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v2 2/3] hwrng: add Rockchip SoC hwrng driver
On 28/11/2022 19:47, Aurelien Jarno wrote:
> Rockchip SoCs used to have a random number generator as part of their
> crypto device, and support for it has to be added to the corresponding
> driver. However newer Rockchip SoCs like the RK356x have an independent
> True Random Number Generator device. This patch adds a driver for it,
> greatly inspired from the downstream driver.
>
> 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.
>
> Signed-off-by: Aurelien Jarno <aurelien@...el32.net>
> ---
> drivers/char/hw_random/Kconfig | 14 ++
> drivers/char/hw_random/Makefile | 1 +
> drivers/char/hw_random/rockchip-rng.c | 250 ++++++++++++++++++++++++++
> 3 files changed, 265 insertions(+)
> create mode 100644 drivers/char/hw_random/rockchip-rng.c
>
> diff --git a/drivers/char/hw_random/Kconfig b/drivers/char/hw_random/Kconfig
> index 3da8e85f8aae..8e5c88504f72 100644
> --- a/drivers/char/hw_random/Kconfig
> +++ b/drivers/char/hw_random/Kconfig
> @@ -549,6 +549,20 @@ config HW_RANDOM_CN10K
> To compile this driver as a module, choose M here.
> The module will be called cn10k_rng. If unsure, say Y.
>
> +config HW_RANDOM_ROCKCHIP
> + tristate "Rockchip True Random Number Generator"
> + depends on HW_RANDOM && (ARCH_ROCKCHIP || COMPILE_TEST)
> + depends on HAS_IOMEM
> + default HW_RANDOM
> + help
> + This driver provides kernel-side support for the True Random Number
> + Generator hardware found on some Rockchip SoC like RK3566 or RK3568.
> +
> + To compile this driver as a module, choose M here: the
> + module will be called rockchip-rng.
> +
> + If unsure, say Y.
> +
> endif # HW_RANDOM
>
> config UML_RANDOM
> diff --git a/drivers/char/hw_random/Makefile b/drivers/char/hw_random/Makefile
> index 3e948cf04476..b7e989535fd6 100644
> --- a/drivers/char/hw_random/Makefile
> +++ b/drivers/char/hw_random/Makefile
> @@ -47,3 +47,4 @@ obj-$(CONFIG_HW_RANDOM_XIPHERA) += xiphera-trng.o
> obj-$(CONFIG_HW_RANDOM_ARM_SMCCC_TRNG) += arm_smccc_trng.o
> obj-$(CONFIG_HW_RANDOM_CN10K) += cn10k-rng.o
> obj-$(CONFIG_HW_RANDOM_POLARFIRE_SOC) += mpfs-rng.o
> +obj-$(CONFIG_HW_RANDOM_ROCKCHIP) += rockchip-rng.o
> diff --git a/drivers/char/hw_random/rockchip-rng.c b/drivers/char/hw_random/rockchip-rng.c
> new file mode 100644
> index 000000000000..18cdd91ad8c3
> --- /dev/null
> +++ b/drivers/char/hw_random/rockchip-rng.c
> @@ -0,0 +1,250 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * rockchip-rng.c True Random Number Generator driver for Rockchip SoCs
> + *
> + * Copyright (c) 2018, Fuzhou Rockchip Electronics Co., Ltd.
> + * Copyright (c) 2022, Aurelien Jarno
> + * Authors:
> + * Lin Jinhan <troy.lin@...k-chips.com>
> + * Aurelien Jarno <aurelien@...el32.net>
> + */
> +#include <linux/clk.h>
> +#include <linux/hw_random.h>
> +#include <linux/io.h>
> +#include <linux/iopoll.h>
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/of_platform.h>
> +#include <linux/pm_runtime.h>
> +#include <linux/reset.h>
> +#include <linux/slab.h>
> +
> +#define RK_RNG_AUTOSUSPEND_DELAY 100
> +#define RK_RNG_MAX_BYTE 32
> +#define RK_RNG_POLL_PERIOD_US 100
> +#define RK_RNG_POLL_TIMEOUT_US 10000
> +
> +/*
> + * 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).
> + */
> +#define RK_RNG_SAMPLE_CNT 1000
> +
> +/* TRNG registers from RK3568 TRM-Part2, section 5.4.1 */
> +#define TRNG_RST_CTL 0x0004
> +#define TRNG_RNG_CTL 0x0400
> +#define TRNG_RNG_CTL_LEN_64_BIT (0x00 << 4)
> +#define TRNG_RNG_CTL_LEN_128_BIT (0x01 << 4)
> +#define TRNG_RNG_CTL_LEN_192_BIT (0x02 << 4)
> +#define TRNG_RNG_CTL_LEN_256_BIT (0x03 << 4)
> +#define TRNG_RNG_CTL_OSC_RING_SPEED_0 (0x00 << 2)
> +#define TRNG_RNG_CTL_OSC_RING_SPEED_1 (0x01 << 2)
> +#define TRNG_RNG_CTL_OSC_RING_SPEED_2 (0x02 << 2)
> +#define TRNG_RNG_CTL_OSC_RING_SPEED_3 (0x03 << 2)
> +#define TRNG_RNG_CTL_ENABLE BIT(1)
> +#define TRNG_RNG_CTL_START BIT(0)
> +#define TRNG_RNG_SAMPLE_CNT 0x0404
> +#define TRNG_RNG_DOUT_0 0x0410
> +#define TRNG_RNG_DOUT_1 0x0414
> +#define TRNG_RNG_DOUT_2 0x0418
> +#define TRNG_RNG_DOUT_3 0x041c
> +#define TRNG_RNG_DOUT_4 0x0420
> +#define TRNG_RNG_DOUT_5 0x0424
> +#define TRNG_RNG_DOUT_6 0x0428
> +#define TRNG_RNG_DOUT_7 0x042c
> +
> +struct rk_rng {
> + struct hwrng rng;
> + void __iomem *base;
> + struct reset_control *rst;
> + int clk_num;
> + struct clk_bulk_data *clk_bulks;
> +};
> +
> +/* The mask determine the bits that are updated */
> +static void rk_rng_write_ctl(struct rk_rng *rng, u32 val, u32 mask)
> +{
> + writel_relaxed((mask << 16) | val, rng->base + TRNG_RNG_CTL);
> +}
> +
> +static int rk_rng_init(struct hwrng *rng)
> +{
> + struct rk_rng *rk_rng = container_of(rng, struct rk_rng, rng);
> + u32 reg;
> + int ret;
> +
> + /* start clocks */
> + ret = clk_bulk_prepare_enable(rk_rng->clk_num, rk_rng->clk_bulks);
> + if (ret < 0) {
> + dev_err((struct device *) rk_rng->rng.priv,
> + "Failed to enable clks %d\n", ret);
> + return ret;
> + }
> +
> + /* set the sample period */
> + writel(RK_RNG_SAMPLE_CNT, rk_rng->base + TRNG_RNG_SAMPLE_CNT);
> +
> + /* set osc ring speed and enable it */
> + reg = TRNG_RNG_CTL_LEN_256_BIT |
It's not reg. It's val.
> + TRNG_RNG_CTL_OSC_RING_SPEED_0 |
> + TRNG_RNG_CTL_ENABLE;
> + rk_rng_write_ctl(rk_rng, reg, 0xffff);
> +
> + return 0;
> +}
> +
> +static void rk_rng_cleanup(struct hwrng *rng)
> +{
> + struct rk_rng *rk_rng = container_of(rng, struct rk_rng, rng);
> + u32 reg;
> +
> + /* stop TRNG */
> + reg = 0;
It's not reg. It's val.
> + rk_rng_write_ctl(rk_rng, reg, 0xffff);
> +
> + /* stop clocks */
> + clk_bulk_disable_unprepare(rk_rng->clk_num, rk_rng->clk_bulks);
> +}
> +
> +static int rk_rng_read(struct hwrng *rng, void *buf, size_t max, bool wait)
> +{
> + struct rk_rng *rk_rng = container_of(rng, struct rk_rng, rng);
> + u32 reg;
> + int ret = 0;
> + int i;
> +
> + pm_runtime_get_sync((struct device *) rk_rng->rng.priv);
Missing error handling.
> +
> + /* Start collecting random data */
> + reg = TRNG_RNG_CTL_START;
This is not usefull. Just use it directly in write call. Actually this
is heavy confusing, as reg suggests address. This would be val instead...
> + rk_rng_write_ctl(rk_rng, reg, reg);
> +
> + ret = readl_poll_timeout(rk_rng->base + TRNG_RNG_CTL, reg,
> + !(reg & TRNG_RNG_CTL_START),
> + RK_RNG_POLL_PERIOD_US,
> + RK_RNG_POLL_TIMEOUT_US);
> + if (ret < 0)
> + goto out;
> +
> + /* Read random data stored in the registers */
> + ret = min_t(size_t, max, RK_RNG_MAX_BYTE);
> + for (i = 0; i < ret; i += 4) {
> + *(u32 *)(buf + i) = readl_relaxed(rk_rng->base + TRNG_RNG_DOUT_0 + i);
> + }
This cannot be just memcpy_fromio?
> +
> +out:
> + pm_runtime_mark_last_busy((struct device *) rk_rng->rng.priv);
> + pm_runtime_put_sync_autosuspend((struct device *) rk_rng->rng.priv);
> +
> + return ret;
> +}
> +
> +static int rk_rng_probe(struct platform_device *pdev)
> +{
> + struct device *dev = &pdev->dev;
> + struct rk_rng *rk_rng;
> + int ret;
> +
> + rk_rng = devm_kzalloc(dev, sizeof(struct rk_rng), GFP_KERNEL);
sizeof(*rk_rng)
> + if (!rk_rng)
> + return -ENOMEM;
> +
Best regards,
Krzysztof
Powered by blists - more mailing lists