2021-07-20 15:44:45

by Andre Przywara

[permalink] [raw]
Subject: [PATCH v2] hwrng: Add Arm SMCCC TRNG based driver

The "Arm True Random Number Generator Firmware Interface"[1] provides
an SMCCC based interface to a true hardware random number generator.
So far we are using that in arch_get_random_seed(), but it might be
useful to expose the entropy through the /dev/hwrng device as well. This
allows to assess the quality of the implementation, by using "rngtest"
from the rng-tools package, for example.

Add a simple platform driver implementing the hw_random interface.
We unconditionally instantiate the platform device in the driver file,
then probe for the existence of the SMCCC TRNG implementation in the
driver's probe routine.
Since the firmware takes care about serialisation, this can happily
coexist with the arch_get_random_seed() bits.

Signed-off-by: Andre Przywara <[email protected]>

[1] https://developer.arm.com/documentation/den0098/latest/
---
Changelog v1 ... v2:
- fix building as a module
- de-register device upon exit
- mention module name in Kconfig

drivers/char/hw_random/Kconfig | 14 ++
drivers/char/hw_random/Makefile | 1 +
drivers/char/hw_random/arm_smccc_trng.c | 171 ++++++++++++++++++++++++
3 files changed, 186 insertions(+)
create mode 100644 drivers/char/hw_random/arm_smccc_trng.c

diff --git a/drivers/char/hw_random/Kconfig b/drivers/char/hw_random/Kconfig
index 3f166c8a4099..239eca4d6805 100644
--- a/drivers/char/hw_random/Kconfig
+++ b/drivers/char/hw_random/Kconfig
@@ -524,6 +524,20 @@ config HW_RANDOM_XIPHERA
To compile this driver as a module, choose M here: the
module will be called xiphera-trng.

+config HW_RANDOM_ARM_SMCCC_TRNG
+ tristate "Arm SMCCC TRNG firmware interface support"
+ depends on HAVE_ARM_SMCCC_DISCOVERY
+ default HW_RANDOM
+ help
+ Say 'Y' to enable the True Random Number Generator driver using
+ the Arm SMCCC TRNG firmware interface. This reads entropy from
+ higher exception levels (firmware, hypervisor). Uses SMCCC for
+ communicating with the firmware:
+ https://developer.arm.com/documentation/den0098/latest/
+
+ To compile this driver as a module, choose M here: the
+ module will be called arm_smccc_trng.
+
endif # HW_RANDOM

config UML_RANDOM
diff --git a/drivers/char/hw_random/Makefile b/drivers/char/hw_random/Makefile
index 8933fada74f2..a5a1c765a394 100644
--- a/drivers/char/hw_random/Makefile
+++ b/drivers/char/hw_random/Makefile
@@ -45,3 +45,4 @@ obj-$(CONFIG_HW_RANDOM_OPTEE) += optee-rng.o
obj-$(CONFIG_HW_RANDOM_NPCM) += npcm-rng.o
obj-$(CONFIG_HW_RANDOM_CCTRNG) += cctrng.o
obj-$(CONFIG_HW_RANDOM_XIPHERA) += xiphera-trng.o
+obj-$(CONFIG_HW_RANDOM_ARM_SMCCC_TRNG) += arm_smccc_trng.o
diff --git a/drivers/char/hw_random/arm_smccc_trng.c b/drivers/char/hw_random/arm_smccc_trng.c
new file mode 100644
index 000000000000..3bd510a98882
--- /dev/null
+++ b/drivers/char/hw_random/arm_smccc_trng.c
@@ -0,0 +1,171 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Randomness driver for the ARM SMCCC TRNG Firmware Interface
+ * https://developer.arm.com/documentation/den0098/latest/
+ *
+ * Copyright (C) 2020 Arm Ltd.
+ *
+ * The ARM TRNG firmware interface specifies a protocol to read entropy
+ * from a higher exception level, to abstract from any machine specific
+ * implemenations and allow easier use in hypervisors.
+ *
+ * The firmware interface is realised using the SMCCC specification.
+ */
+
+#include <linux/bits.h>
+#include <linux/device.h>
+#include <linux/hw_random.h>
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <linux/arm-smccc.h>
+
+#ifdef CONFIG_ARM64
+#define ARM_SMCCC_TRNG_RND ARM_SMCCC_TRNG_RND64
+#define MAX_BITS_PER_CALL (3 * 64UL)
+#else
+#define ARM_SMCCC_TRNG_RND ARM_SMCCC_TRNG_RND32
+#define MAX_BITS_PER_CALL (3 * 32UL)
+#endif
+
+/* We don't want to allow the firmware to stall us forever. */
+#define SMCCC_TRNG_MAX_TRIES 20
+
+#define SMCCC_RET_TRNG_INVALID_PARAMETER -2
+#define SMCCC_RET_TRNG_NO_ENTROPY -3
+
+static struct platform_device *smccc_trng_pdev;
+
+static int smccc_trng_init(struct hwrng *rng)
+{
+ return 0;
+}
+
+static int copy_from_registers(char *buf, struct arm_smccc_res *res,
+ size_t bytes)
+{
+ unsigned int chunk, copied;
+
+ if (bytes == 0)
+ return 0;
+
+ chunk = min(bytes, sizeof(long));
+ memcpy(buf, &res->a3, chunk);
+ copied = chunk;
+ if (copied >= bytes)
+ return copied;
+
+ chunk = min((bytes - copied), sizeof(long));
+ memcpy(&buf[copied], &res->a2, chunk);
+ copied += chunk;
+ if (copied >= bytes)
+ return copied;
+
+ chunk = min((bytes - copied), sizeof(long));
+ memcpy(&buf[copied], &res->a1, chunk);
+
+ return copied + chunk;
+}
+
+static int smccc_trng_read(struct hwrng *rng, void *data, size_t max, bool wait)
+{
+ struct arm_smccc_res res;
+ u8 *buf = data;
+ unsigned int copied = 0;
+ int tries = 0;
+
+ while (copied < max) {
+ size_t bits = min_t(size_t, (max - copied) * BITS_PER_BYTE,
+ MAX_BITS_PER_CALL);
+
+ arm_smccc_1_1_invoke(ARM_SMCCC_TRNG_RND, bits, &res);
+ if ((int)res.a0 < 0)
+ return (int)res.a0;
+
+ switch ((int)res.a0) {
+ case SMCCC_RET_SUCCESS:
+ copied += copy_from_registers(buf + copied, &res,
+ bits / BITS_PER_BYTE);
+ tries = 0;
+ break;
+ case SMCCC_RET_TRNG_NO_ENTROPY:
+ if (!wait)
+ return copied;
+ tries++;
+ if (tries >= SMCCC_TRNG_MAX_TRIES)
+ return copied;
+ cond_resched();
+ break;
+ }
+ }
+
+ return copied;
+}
+
+static int smccc_trng_probe(struct platform_device *pdev)
+{
+ struct arm_smccc_res res;
+ struct hwrng *trng;
+ u32 version;
+ int ret;
+
+ /* We need ARM SMCCC v1.1, with its autodetection feature. */
+ if (arm_smccc_get_version() < ARM_SMCCC_VERSION_1_1)
+ return -ENODEV;
+
+ arm_smccc_1_1_invoke(ARM_SMCCC_TRNG_VERSION, &res);
+ if ((int)res.a0 < 0)
+ return -ENODEV;
+ version = res.a0;
+
+ arm_smccc_1_1_invoke(ARM_SMCCC_TRNG_FEATURES,
+ ARM_SMCCC_TRNG_RND, &res);
+ if ((int)res.a0 < 0)
+ return -ENODEV;
+
+ trng = devm_kzalloc(&pdev->dev, sizeof(*trng), GFP_KERNEL);
+ if (!trng)
+ return -ENOMEM;
+
+ trng->name = "smccc_trng";
+ trng->init = smccc_trng_init;
+ trng->read = smccc_trng_read;
+
+ platform_set_drvdata(pdev, trng);
+ ret = devm_hwrng_register(&pdev->dev, trng);
+ if (ret)
+ return -ENOENT;
+
+ dev_info(&pdev->dev,
+ "ARM SMCCC TRNG firmware random number generator v%d.%d\n",
+ version >> 16, version & 0xffff);
+
+ return 0;
+}
+
+static struct platform_driver smccc_trng_driver = {
+ .driver = {
+ .name = "smccc_trng",
+ },
+ .probe = smccc_trng_probe,
+};
+
+static int __init smccc_trng_driver_init(void)
+{
+ smccc_trng_pdev = platform_device_register_simple("smccc_trng",
+ -1, NULL, 0);
+ if (IS_ERR(smccc_trng_pdev))
+ return PTR_ERR(smccc_trng_pdev);
+
+ return platform_driver_register(&smccc_trng_driver);
+}
+module_init(smccc_trng_driver_init);
+
+static void __exit smccc_trng_driver_exit(void)
+{
+ platform_driver_unregister(&smccc_trng_driver);
+ platform_device_unregister(smccc_trng_pdev);
+}
+module_exit(smccc_trng_driver_exit);
+
+MODULE_AUTHOR("Andre Przywara");
+MODULE_LICENSE("GPL");
--
2.17.6


2021-07-20 16:13:47

by Ard Biesheuvel

[permalink] [raw]
Subject: Re: [PATCH v2] hwrng: Add Arm SMCCC TRNG based driver

On Tue, 20 Jul 2021 at 17:22, Andre Przywara <[email protected]> wrote:
>
> The "Arm True Random Number Generator Firmware Interface"[1] provides
> an SMCCC based interface to a true hardware random number generator.
> So far we are using that in arch_get_random_seed(), but it might be
> useful to expose the entropy through the /dev/hwrng device as well. This
> allows to assess the quality of the implementation, by using "rngtest"
> from the rng-tools package, for example.
>
> Add a simple platform driver implementing the hw_random interface.
> We unconditionally instantiate the platform device in the driver file,
> then probe for the existence of the SMCCC TRNG implementation in the
> driver's probe routine.
> Since the firmware takes care about serialisation, this can happily
> coexist with the arch_get_random_seed() bits.
>
> Signed-off-by: Andre Przywara <[email protected]>
>
> [1] https://developer.arm.com/documentation/den0098/latest/
> ---
> Changelog v1 ... v2:
> - fix building as a module
> - de-register device upon exit
> - mention module name in Kconfig
>
> drivers/char/hw_random/Kconfig | 14 ++
> drivers/char/hw_random/Makefile | 1 +
> drivers/char/hw_random/arm_smccc_trng.c | 171 ++++++++++++++++++++++++
> 3 files changed, 186 insertions(+)
> create mode 100644 drivers/char/hw_random/arm_smccc_trng.c
>
> diff --git a/drivers/char/hw_random/Kconfig b/drivers/char/hw_random/Kconfig
> index 3f166c8a4099..239eca4d6805 100644
> --- a/drivers/char/hw_random/Kconfig
> +++ b/drivers/char/hw_random/Kconfig
> @@ -524,6 +524,20 @@ config HW_RANDOM_XIPHERA
> To compile this driver as a module, choose M here: the
> module will be called xiphera-trng.
>
> +config HW_RANDOM_ARM_SMCCC_TRNG
> + tristate "Arm SMCCC TRNG firmware interface support"
> + depends on HAVE_ARM_SMCCC_DISCOVERY
> + default HW_RANDOM
> + help
> + Say 'Y' to enable the True Random Number Generator driver using
> + the Arm SMCCC TRNG firmware interface. This reads entropy from
> + higher exception levels (firmware, hypervisor). Uses SMCCC for
> + communicating with the firmware:
> + https://developer.arm.com/documentation/den0098/latest/
> +
> + To compile this driver as a module, choose M here: the
> + module will be called arm_smccc_trng.
> +
> endif # HW_RANDOM
>
> config UML_RANDOM
> diff --git a/drivers/char/hw_random/Makefile b/drivers/char/hw_random/Makefile
> index 8933fada74f2..a5a1c765a394 100644
> --- a/drivers/char/hw_random/Makefile
> +++ b/drivers/char/hw_random/Makefile
> @@ -45,3 +45,4 @@ obj-$(CONFIG_HW_RANDOM_OPTEE) += optee-rng.o
> obj-$(CONFIG_HW_RANDOM_NPCM) += npcm-rng.o
> obj-$(CONFIG_HW_RANDOM_CCTRNG) += cctrng.o
> obj-$(CONFIG_HW_RANDOM_XIPHERA) += xiphera-trng.o
> +obj-$(CONFIG_HW_RANDOM_ARM_SMCCC_TRNG) += arm_smccc_trng.o
> diff --git a/drivers/char/hw_random/arm_smccc_trng.c b/drivers/char/hw_random/arm_smccc_trng.c
> new file mode 100644
> index 000000000000..3bd510a98882
> --- /dev/null
> +++ b/drivers/char/hw_random/arm_smccc_trng.c
> @@ -0,0 +1,171 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Randomness driver for the ARM SMCCC TRNG Firmware Interface
> + * https://developer.arm.com/documentation/den0098/latest/
> + *
> + * Copyright (C) 2020 Arm Ltd.
> + *
> + * The ARM TRNG firmware interface specifies a protocol to read entropy
> + * from a higher exception level, to abstract from any machine specific
> + * implemenations and allow easier use in hypervisors.
> + *
> + * The firmware interface is realised using the SMCCC specification.
> + */
> +
> +#include <linux/bits.h>
> +#include <linux/device.h>
> +#include <linux/hw_random.h>
> +#include <linux/module.h>
> +#include <linux/platform_device.h>
> +#include <linux/arm-smccc.h>
> +
> +#ifdef CONFIG_ARM64
> +#define ARM_SMCCC_TRNG_RND ARM_SMCCC_TRNG_RND64
> +#define MAX_BITS_PER_CALL (3 * 64UL)
> +#else
> +#define ARM_SMCCC_TRNG_RND ARM_SMCCC_TRNG_RND32
> +#define MAX_BITS_PER_CALL (3 * 32UL)
> +#endif
> +
> +/* We don't want to allow the firmware to stall us forever. */
> +#define SMCCC_TRNG_MAX_TRIES 20
> +
> +#define SMCCC_RET_TRNG_INVALID_PARAMETER -2
> +#define SMCCC_RET_TRNG_NO_ENTROPY -3
> +
> +static struct platform_device *smccc_trng_pdev;
> +

Can we split the platform device and the platform driver? The platform
device should be registered by the core code once it discovers the
SMCCC TRNG availabily. The platform driver can then use ordinary
module alias autoloading etc based on the platform device availabilty.

This requires the following to be added though

MODULE_ALIAS("platform: smccc_trng")

but all the smccc calls could be dropped from the probe function in that case.



> +static int smccc_trng_init(struct hwrng *rng)
> +{
> + return 0;
> +}
> +
> +static int copy_from_registers(char *buf, struct arm_smccc_res *res,
> + size_t bytes)
> +{
> + unsigned int chunk, copied;
> +
> + if (bytes == 0)
> + return 0;
> +
> + chunk = min(bytes, sizeof(long));
> + memcpy(buf, &res->a3, chunk);
> + copied = chunk;
> + if (copied >= bytes)
> + return copied;
> +
> + chunk = min((bytes - copied), sizeof(long));
> + memcpy(&buf[copied], &res->a2, chunk);
> + copied += chunk;
> + if (copied >= bytes)
> + return copied;
> +
> + chunk = min((bytes - copied), sizeof(long));
> + memcpy(&buf[copied], &res->a1, chunk);
> +
> + return copied + chunk;
> +}
> +
> +static int smccc_trng_read(struct hwrng *rng, void *data, size_t max, bool wait)
> +{
> + struct arm_smccc_res res;
> + u8 *buf = data;
> + unsigned int copied = 0;
> + int tries = 0;
> +
> + while (copied < max) {
> + size_t bits = min_t(size_t, (max - copied) * BITS_PER_BYTE,
> + MAX_BITS_PER_CALL);
> +
> + arm_smccc_1_1_invoke(ARM_SMCCC_TRNG_RND, bits, &res);
> + if ((int)res.a0 < 0)
> + return (int)res.a0;
> +
> + switch ((int)res.a0) {
> + case SMCCC_RET_SUCCESS:
> + copied += copy_from_registers(buf + copied, &res,
> + bits / BITS_PER_BYTE);
> + tries = 0;
> + break;
> + case SMCCC_RET_TRNG_NO_ENTROPY:
> + if (!wait)
> + return copied;
> + tries++;
> + if (tries >= SMCCC_TRNG_MAX_TRIES)
> + return copied;
> + cond_resched();
> + break;
> + }
> + }
> +
> + return copied;
> +}
> +
> +static int smccc_trng_probe(struct platform_device *pdev)
> +{
> + struct arm_smccc_res res;
> + struct hwrng *trng;
> + u32 version;
> + int ret;
> +
> + /* We need ARM SMCCC v1.1, with its autodetection feature. */
> + if (arm_smccc_get_version() < ARM_SMCCC_VERSION_1_1)
> + return -ENODEV;
> +
> + arm_smccc_1_1_invoke(ARM_SMCCC_TRNG_VERSION, &res);
> + if ((int)res.a0 < 0)
> + return -ENODEV;
> + version = res.a0;
> +
> + arm_smccc_1_1_invoke(ARM_SMCCC_TRNG_FEATURES,
> + ARM_SMCCC_TRNG_RND, &res);
> + if ((int)res.a0 < 0)
> + return -ENODEV;
> +
> + trng = devm_kzalloc(&pdev->dev, sizeof(*trng), GFP_KERNEL);
> + if (!trng)
> + return -ENOMEM;
> +
> + trng->name = "smccc_trng";
> + trng->init = smccc_trng_init;
> + trng->read = smccc_trng_read;
> +
> + platform_set_drvdata(pdev, trng);
> + ret = devm_hwrng_register(&pdev->dev, trng);
> + if (ret)
> + return -ENOENT;
> +
> + dev_info(&pdev->dev,
> + "ARM SMCCC TRNG firmware random number generator v%d.%d\n",
> + version >> 16, version & 0xffff);
> +
> + return 0;
> +}
> +
> +static struct platform_driver smccc_trng_driver = {
> + .driver = {
> + .name = "smccc_trng",
> + },
> + .probe = smccc_trng_probe,
> +};
> +
> +static int __init smccc_trng_driver_init(void)
> +{
> + smccc_trng_pdev = platform_device_register_simple("smccc_trng",
> + -1, NULL, 0);
> + if (IS_ERR(smccc_trng_pdev))
> + return PTR_ERR(smccc_trng_pdev);
> +
> + return platform_driver_register(&smccc_trng_driver);
> +}
> +module_init(smccc_trng_driver_init);
> +
> +static void __exit smccc_trng_driver_exit(void)
> +{
> + platform_driver_unregister(&smccc_trng_driver);
> + platform_device_unregister(smccc_trng_pdev);
> +}
> +module_exit(smccc_trng_driver_exit);
> +
> +MODULE_AUTHOR("Andre Przywara");
> +MODULE_LICENSE("GPL");
> --
> 2.17.6
>

2021-07-20 16:22:54

by Andre Przywara

[permalink] [raw]
Subject: Re: [PATCH v2] hwrng: Add Arm SMCCC TRNG based driver

On Tue, 20 Jul 2021 18:03:37 +0200
Ard Biesheuvel <[email protected]> wrote:

Hi Ard,

> On Tue, 20 Jul 2021 at 17:22, Andre Przywara <[email protected]> wrote:
> >
> > The "Arm True Random Number Generator Firmware Interface"[1] provides
> > an SMCCC based interface to a true hardware random number generator.
> > So far we are using that in arch_get_random_seed(), but it might be
> > useful to expose the entropy through the /dev/hwrng device as well. This
> > allows to assess the quality of the implementation, by using "rngtest"
> > from the rng-tools package, for example.
> >
> > Add a simple platform driver implementing the hw_random interface.
> > We unconditionally instantiate the platform device in the driver file,
> > then probe for the existence of the SMCCC TRNG implementation in the
> > driver's probe routine.
> > Since the firmware takes care about serialisation, this can happily
> > coexist with the arch_get_random_seed() bits.
> >
> > Signed-off-by: Andre Przywara <[email protected]>
> >
> > [1] https://developer.arm.com/documentation/den0098/latest/
> > ---
> > Changelog v1 ... v2:
> > - fix building as a module
> > - de-register device upon exit
> > - mention module name in Kconfig
> >
> > drivers/char/hw_random/Kconfig | 14 ++
> > drivers/char/hw_random/Makefile | 1 +
> > drivers/char/hw_random/arm_smccc_trng.c | 171 ++++++++++++++++++++++++
> > 3 files changed, 186 insertions(+)
> > create mode 100644 drivers/char/hw_random/arm_smccc_trng.c
> >
> > diff --git a/drivers/char/hw_random/Kconfig b/drivers/char/hw_random/Kconfig
> > index 3f166c8a4099..239eca4d6805 100644
> > --- a/drivers/char/hw_random/Kconfig
> > +++ b/drivers/char/hw_random/Kconfig
> > @@ -524,6 +524,20 @@ config HW_RANDOM_XIPHERA
> > To compile this driver as a module, choose M here: the
> > module will be called xiphera-trng.
> >
> > +config HW_RANDOM_ARM_SMCCC_TRNG
> > + tristate "Arm SMCCC TRNG firmware interface support"
> > + depends on HAVE_ARM_SMCCC_DISCOVERY
> > + default HW_RANDOM
> > + help
> > + Say 'Y' to enable the True Random Number Generator driver using
> > + the Arm SMCCC TRNG firmware interface. This reads entropy from
> > + higher exception levels (firmware, hypervisor). Uses SMCCC for
> > + communicating with the firmware:
> > + https://developer.arm.com/documentation/den0098/latest/
> > +
> > + To compile this driver as a module, choose M here: the
> > + module will be called arm_smccc_trng.
> > +
> > endif # HW_RANDOM
> >
> > config UML_RANDOM
> > diff --git a/drivers/char/hw_random/Makefile b/drivers/char/hw_random/Makefile
> > index 8933fada74f2..a5a1c765a394 100644
> > --- a/drivers/char/hw_random/Makefile
> > +++ b/drivers/char/hw_random/Makefile
> > @@ -45,3 +45,4 @@ obj-$(CONFIG_HW_RANDOM_OPTEE) += optee-rng.o
> > obj-$(CONFIG_HW_RANDOM_NPCM) += npcm-rng.o
> > obj-$(CONFIG_HW_RANDOM_CCTRNG) += cctrng.o
> > obj-$(CONFIG_HW_RANDOM_XIPHERA) += xiphera-trng.o
> > +obj-$(CONFIG_HW_RANDOM_ARM_SMCCC_TRNG) += arm_smccc_trng.o
> > diff --git a/drivers/char/hw_random/arm_smccc_trng.c b/drivers/char/hw_random/arm_smccc_trng.c
> > new file mode 100644
> > index 000000000000..3bd510a98882
> > --- /dev/null
> > +++ b/drivers/char/hw_random/arm_smccc_trng.c
> > @@ -0,0 +1,171 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * Randomness driver for the ARM SMCCC TRNG Firmware Interface
> > + * https://developer.arm.com/documentation/den0098/latest/
> > + *
> > + * Copyright (C) 2020 Arm Ltd.
> > + *
> > + * The ARM TRNG firmware interface specifies a protocol to read entropy
> > + * from a higher exception level, to abstract from any machine specific
> > + * implemenations and allow easier use in hypervisors.
> > + *
> > + * The firmware interface is realised using the SMCCC specification.
> > + */
> > +
> > +#include <linux/bits.h>
> > +#include <linux/device.h>
> > +#include <linux/hw_random.h>
> > +#include <linux/module.h>
> > +#include <linux/platform_device.h>
> > +#include <linux/arm-smccc.h>
> > +
> > +#ifdef CONFIG_ARM64
> > +#define ARM_SMCCC_TRNG_RND ARM_SMCCC_TRNG_RND64
> > +#define MAX_BITS_PER_CALL (3 * 64UL)
> > +#else
> > +#define ARM_SMCCC_TRNG_RND ARM_SMCCC_TRNG_RND32
> > +#define MAX_BITS_PER_CALL (3 * 32UL)
> > +#endif
> > +
> > +/* We don't want to allow the firmware to stall us forever. */
> > +#define SMCCC_TRNG_MAX_TRIES 20
> > +
> > +#define SMCCC_RET_TRNG_INVALID_PARAMETER -2
> > +#define SMCCC_RET_TRNG_NO_ENTROPY -3
> > +
> > +static struct platform_device *smccc_trng_pdev;
> > +
>
> Can we split the platform device and the platform driver? The platform
> device should be registered by the core code once it discovers the
> SMCCC TRNG availabily. The platform driver can then use ordinary
> module alias autoloading etc based on the platform device availabilty.

Yes, a similar idea was already brought up before. I think there is even
the potential for something like an artificial SMCCC "bus", where those
services presentable as devices could be auto-detected (by checking
known function IDs), the respective drivers would then probe
automatically?

I will try to implement your approach first, to see what it looks like.

Cheers,
Andre

>
> This requires the following to be added though
>
> MODULE_ALIAS("platform: smccc_trng")
>
> but all the smccc calls could be dropped from the probe function in that case.
>
>
>
> > +static int smccc_trng_init(struct hwrng *rng)
> > +{
> > + return 0;
> > +}
> > +
> > +static int copy_from_registers(char *buf, struct arm_smccc_res *res,
> > + size_t bytes)
> > +{
> > + unsigned int chunk, copied;
> > +
> > + if (bytes == 0)
> > + return 0;
> > +
> > + chunk = min(bytes, sizeof(long));
> > + memcpy(buf, &res->a3, chunk);
> > + copied = chunk;
> > + if (copied >= bytes)
> > + return copied;
> > +
> > + chunk = min((bytes - copied), sizeof(long));
> > + memcpy(&buf[copied], &res->a2, chunk);
> > + copied += chunk;
> > + if (copied >= bytes)
> > + return copied;
> > +
> > + chunk = min((bytes - copied), sizeof(long));
> > + memcpy(&buf[copied], &res->a1, chunk);
> > +
> > + return copied + chunk;
> > +}
> > +
> > +static int smccc_trng_read(struct hwrng *rng, void *data, size_t max, bool wait)
> > +{
> > + struct arm_smccc_res res;
> > + u8 *buf = data;
> > + unsigned int copied = 0;
> > + int tries = 0;
> > +
> > + while (copied < max) {
> > + size_t bits = min_t(size_t, (max - copied) * BITS_PER_BYTE,
> > + MAX_BITS_PER_CALL);
> > +
> > + arm_smccc_1_1_invoke(ARM_SMCCC_TRNG_RND, bits, &res);
> > + if ((int)res.a0 < 0)
> > + return (int)res.a0;
> > +
> > + switch ((int)res.a0) {
> > + case SMCCC_RET_SUCCESS:
> > + copied += copy_from_registers(buf + copied, &res,
> > + bits / BITS_PER_BYTE);
> > + tries = 0;
> > + break;
> > + case SMCCC_RET_TRNG_NO_ENTROPY:
> > + if (!wait)
> > + return copied;
> > + tries++;
> > + if (tries >= SMCCC_TRNG_MAX_TRIES)
> > + return copied;
> > + cond_resched();
> > + break;
> > + }
> > + }
> > +
> > + return copied;
> > +}
> > +
> > +static int smccc_trng_probe(struct platform_device *pdev)
> > +{
> > + struct arm_smccc_res res;
> > + struct hwrng *trng;
> > + u32 version;
> > + int ret;
> > +
> > + /* We need ARM SMCCC v1.1, with its autodetection feature. */
> > + if (arm_smccc_get_version() < ARM_SMCCC_VERSION_1_1)
> > + return -ENODEV;
> > +
> > + arm_smccc_1_1_invoke(ARM_SMCCC_TRNG_VERSION, &res);
> > + if ((int)res.a0 < 0)
> > + return -ENODEV;
> > + version = res.a0;
> > +
> > + arm_smccc_1_1_invoke(ARM_SMCCC_TRNG_FEATURES,
> > + ARM_SMCCC_TRNG_RND, &res);
> > + if ((int)res.a0 < 0)
> > + return -ENODEV;
> > +
> > + trng = devm_kzalloc(&pdev->dev, sizeof(*trng), GFP_KERNEL);
> > + if (!trng)
> > + return -ENOMEM;
> > +
> > + trng->name = "smccc_trng";
> > + trng->init = smccc_trng_init;
> > + trng->read = smccc_trng_read;
> > +
> > + platform_set_drvdata(pdev, trng);
> > + ret = devm_hwrng_register(&pdev->dev, trng);
> > + if (ret)
> > + return -ENOENT;
> > +
> > + dev_info(&pdev->dev,
> > + "ARM SMCCC TRNG firmware random number generator v%d.%d\n",
> > + version >> 16, version & 0xffff);
> > +
> > + return 0;
> > +}
> > +
> > +static struct platform_driver smccc_trng_driver = {
> > + .driver = {
> > + .name = "smccc_trng",
> > + },
> > + .probe = smccc_trng_probe,
> > +};
> > +
> > +static int __init smccc_trng_driver_init(void)
> > +{
> > + smccc_trng_pdev = platform_device_register_simple("smccc_trng",
> > + -1, NULL, 0);
> > + if (IS_ERR(smccc_trng_pdev))
> > + return PTR_ERR(smccc_trng_pdev);
> > +
> > + return platform_driver_register(&smccc_trng_driver);
> > +}
> > +module_init(smccc_trng_driver_init);
> > +
> > +static void __exit smccc_trng_driver_exit(void)
> > +{
> > + platform_driver_unregister(&smccc_trng_driver);
> > + platform_device_unregister(smccc_trng_pdev);
> > +}
> > +module_exit(smccc_trng_driver_exit);
> > +
> > +MODULE_AUTHOR("Andre Przywara");
> > +MODULE_LICENSE("GPL");
> > --
> > 2.17.6
> >

2021-07-21 11:22:39

by Mark Brown

[permalink] [raw]
Subject: Re: [PATCH v2] hwrng: Add Arm SMCCC TRNG based driver

On Wed, Jul 21, 2021 at 12:18:03AM +0100, Andre Przywara wrote:
> On Wed, 21 Jul 2021 08:02:42 +1000
> Benjamin Herrenschmidt <[email protected]> wrote:
> > On Tue, 2021-07-20 at 17:16 +0100, Andre Przywara wrote:

> > > Yes, a similar idea was already brought up before. I think there is even
> > > the potential for something like an artificial SMCCC "bus", where those
> > > services presentable as devices could be auto-detected (by checking
> > > known function IDs), the respective drivers would then probe
> > > automatically?

> > Sounds like a boot time killer...

> How so? To be clear, at the moment there is basically just the TRNG
> service we would probe for, maybe FF-A, then adding as we go. But in
> any case it would be just a handful, and querying is very quick
> (SMC/HVC, then just a switch/case on the other side, and ERET).
> Is there any particular scenario you are concerned about? Quick
> starting guests?

It's also worth pointing out that we're already doing the enumeration
part, making things a bus would just be about reorganising the code that
checks if services are present to a central place so it looks more Linux
style. If anything I'd guess that if we get to the point where things
are slow enough to worry about having that code in one central place
would make doing something about it easier (eg, adding a "list all
services" service or firmware binding).


Attachments:
(No filename) (1.40 kB)
signature.asc (499.00 B)
Download all attachments