On 2/13/24 10:16, Stefan Roese wrote:

Hi Max,

mostly some nitpicking comments below.

On 2/11/24 14:04, Max Resch wrote:

...

A RNG driver for Armada 3720 boards running the Turris Mox rWTM firmware from CZ.NIC in the secure processor.

Signed-off-by: Max Resch resch.max@gmail.com

Changes in v4:   - wrongful/missing git rebase

Changes in v3:   - More meaningful variable names in accordance with review

Changes in v2:   - Removed ring buffer implementation

drivers/rng/Kconfig           |   8 +++   drivers/rng/Makefile          |   1 +   drivers/rng/turris_rwtm_rng.c | 122 ++++++++++++++++++++++++++++++++++   3 files changed, 131 insertions(+)   create mode 100644 drivers/rng/turris_rwtm_rng.c

diff --git a/drivers/rng/Kconfig b/drivers/rng/Kconfig index a89c899568..cd72852a47 100644 --- a/drivers/rng/Kconfig +++ b/drivers/rng/Kconfig @@ -105,4 +105,12 @@ config RNG_JH7110       help         Enable True Random Number Generator in StarFive JH7110 SoCs. +config RNG_TURRIS_RWTM +    bool "Turris Mox TRNG in Secure Processor" +    depends on DM_RNG && ARMADA_3700 +    help +      Use TRNG in Turris Mox Secure Processor Firmware. Can be used +      on other Armada-3700 devices (like EspressoBin) if Secure +      Firmware from CZ.NIC is used.

endif diff --git a/drivers/rng/Makefile b/drivers/rng/Makefile index 7e64c4cdfc..ecae1a3da3 100644 --- a/drivers/rng/Makefile +++ b/drivers/rng/Makefile @@ -17,3 +17,4 @@ obj-$(CONFIG_RNG_SMCCC_TRNG) += smccc_trng.o   obj-$(CONFIG_RNG_ARM_RNDR) += arm_rndr.o   obj-$(CONFIG_TPM_RNG) += tpm_rng.o   obj-$(CONFIG_RNG_JH7110) += jh7110_rng.o +obj-$(CONFIG_RNG_TURRIS_RWTM) += turris_rwtm_rng.o diff --git a/drivers/rng/turris_rwtm_rng.c b/drivers/rng/turris_rwtm_rng.c new file mode 100644 index 0000000000..ec2cb0bca3 --- /dev/null +++ b/drivers/rng/turris_rwtm_rng.c @@ -0,0 +1,122 @@ +// SPDX-License-Identifier: GPL-2.0-or-later OR BSD-3-Clause +/*

• • Copyright (c) 2024, Max Resch
• */

+#include <dm.h> +#include <malloc.h> +#include <rng.h> +#include <asm/dma-mapping.h> +#include <asm/types.h> +#include <mach/mbox.h>

+/* size of entropy buffer */ +#define RNG_BUFFER_SIZE    128U

+struct turris_rwtm_rng_priv { +    phys_addr_t buffer; +};

+static int turris_rwtm_rng_fill_entropy(phys_addr_t entropy, size_t size) +{ +    u32 args[3] = { 1, (u32)entropy, size };

On a device with memory above 4 GiB this (u32) conversion may point to a different location than rwtm_rng_priv.

Is the Armada 3700 family restricted to below 4 GiB?

Should we add a check in the probe function?

Best regards

Heinrich

...

+    int ret;

+    /* flush data cache */ +    flush_dcache_range(entropy, entropy + size);

+    /* +     * get entropy +     * args[0] = 1 copies BYTES array in args[1] of length args[2] +     */ +    ret = mbox_do_cmd(MBOX_CMD_GET_RANDOM, args, 3, NULL, 0); +    if (ret < 0) +        return ret;

+    /* invalidate data cache */ +    invalidate_dcache_range(entropy, entropy + size);

+    return 0; +}

+static int turris_rwtm_rng_random_read(struct udevice *dev, void *data, size_t count) +{ +    struct turris_rwtm_rng_priv *priv = dev_get_priv(dev); +    phys_addr_t phys; +    size_t size; +    int ret;

+    phys = priv->buffer;

+    while (count) { +        size = min_t(size_t, RNG_BUFFER_SIZE, count);

+        ret = turris_rwtm_rng_fill_entropy(phys, size);

Return code check missing here.

...

+        memcpy(data, (void *)phys, size); +        count -= size; +        data = (u8 *)data + size; +    }

+    return 0; +}

+static int turris_rwtm_rng_probe(struct udevice *dev) +{ +    struct turris_rwtm_rng_priv *priv = dev_get_priv(dev); +    u32 args[] = { 0 }; +    int ret;

+    /* +     * check if the random command is supported +     * args[0] = 0 would copy 16 DWORDS entropy to out but we ignore them +     */ +    ret = mbox_do_cmd(MBOX_CMD_GET_RANDOM, args, ARRAY_SIZE(args), NULL, 0);

Please drop this empty line before the ret check.

...

+    if (ret < 0) +        return ret;

+    /* entropy buffer */ +    priv->buffer = 0;

+    /* buffer address need to be aligned */ +    dma_alloc_coherent(RNG_BUFFER_SIZE, (unsigned long *)&priv->buffer); +    if (!priv->buffer) +        return -ENOMEM;

+    return 0; +}

+static int turris_rwtm_rng_remove(struct udevice *dev) +{ +    struct turris_rwtm_rng_priv *priv = dev_get_priv(dev); +    phys_addr_t phys = priv->buffer;

+    dma_free_coherent((void *)phys);

+    return 0; +}

+static const struct dm_rng_ops turris_rwtm_rng_ops = { +    .read = turris_rwtm_rng_random_read, +};

+/*

• • only Turris MOX firmware has the RNG but allow all probable

devices to be

• • probed the default firmware will just reject the probe
• */

+static const struct udevice_id turris_rwtm_rng_match[] = { +    { .compatible = "cznic,turris-mox-rwtm" }, +    { .compatible = "marvell,armada-3700-rwtm-firmware" }, +    {}, +};

+U_BOOT_DRIVER(turris_rwtm_rng) = { +    .name    = "turris-rwtm-rng", +    .id    = UCLASS_RNG, +    .of_match = turris_rwtm_rng_match, +    .ops    = &turris_rwtm_rng_ops, +    .probe    = turris_rwtm_rng_probe, +    .remove = turris_rwtm_rng_remove, +    .priv_auto = sizeof(struct turris_rwtm_rng_priv), +};

Thanks, Stefan