| Message ID | 20230830113230.3925868-1-andre.przywara@arm.com |
|---|---|
| State | Accepted |
| Commit | 31565bb0aa2d76b6941e96bcdbd204bae49ca828 |
| Delegated to: | Tom Rini |
| Headers | show |
| Series | driver: rng: Add DM_RNG interface for ARMv8.5 RNDR registers | expand |
Hi Andre, On Wed, 30 Aug 2023 at 05:32, Andre Przywara <andre.przywara@arm.com> wrote: > > The ARMv8.5 architecture extension defines architectural RNDR/RNDRRS > system registers, that provide 64 bits worth of randomness on every > read. Since it's an extension, and implementing it is optional, there is > a field in the ID_AA64ISAR0_EL1 ID register to query the availability > of those registers. > > Add a UCLASS_RNG driver that returns entropy via repeated reads from > those system registers, if the extension is implemented. > The driver always binds, but checks the availability in the probe() > routine. > > This helps systems which suffer from low boot entropy, since U-Boot can > provide entropy via the generic UEFI entropy gathering protocol to the OS, > at an early stage. > > Signed-off-by: Andre Przywara <andre.przywara@arm.com> > --- > Hi Simon, > > not sure if this is the expected way to model a driver which autodetects > its device at runtime. It somewhat works, but always lists the bound > device, even if the registers are not supported. If I do the check in bind > instead, it fails booting when this returns -ENODEV, since it expects > the fixed device to always bind successfully, I guess? > Is there any other way of modeling this? And before you say your > favourite two letters: this is not a DT job, since it can be safely > auto-detected in an architectural way. > > Thanks, > Andre > > arch/arm/include/asm/system.h | 1 + > drivers/rng/Kconfig | 6 +++ > drivers/rng/Makefile | 1 + > drivers/rng/arm_rndr.c | 82 +++++++++++++++++++++++++++++++++++ > 4 files changed, 90 insertions(+) > create mode 100644 drivers/rng/arm_rndr.c Reviewed-by: Simon Glass <sjg@chromium.org> nit below > > diff --git a/arch/arm/include/asm/system.h b/arch/arm/include/asm/system.h > index 87d1c77e8b1..0eae857e73a 100644 > --- a/arch/arm/include/asm/system.h > +++ b/arch/arm/include/asm/system.h > @@ -84,6 +84,7 @@ > #define HCR_EL2_HCD_DIS (1 << 29) /* Hypervisor Call disabled */ > #define HCR_EL2_AMO_EL2 (1 << 5) /* Route SErrors to EL2 */ > > +#define ID_AA64ISAR0_EL1_RNDR (0xFUL << 60) /* RNDR random registers */ > /* > * ID_AA64ISAR1_EL1 bits definitions > */ > diff --git a/drivers/rng/Kconfig b/drivers/rng/Kconfig > index 5deb5db5b71..72788d479cc 100644 > --- a/drivers/rng/Kconfig > +++ b/drivers/rng/Kconfig > @@ -76,6 +76,12 @@ config RNG_SMCCC_TRNG > Enable random number generator for platforms that support Arm > SMCCC TRNG interface. > > +config RNG_ARM_RNDR > + bool "Generic ARMv8.5 RNDR register" > + depends on DM_RNG && ARM64 > + help > + Use the ARMv8.5 RNDR register to provide random numbers. > + > config TPM_RNG > bool "Enable random number generator on TPM device" > depends on TPM > diff --git a/drivers/rng/Makefile b/drivers/rng/Makefile > index 78f61051acf..572fa7a0c66 100644 > --- a/drivers/rng/Makefile > +++ b/drivers/rng/Makefile > @@ -13,4 +13,5 @@ obj-$(CONFIG_RNG_STM32MP1) += stm32mp1_rng.o > obj-$(CONFIG_RNG_ROCKCHIP) += rockchip_rng.o > obj-$(CONFIG_RNG_IPROC200) += iproc_rng200.o > obj-$(CONFIG_RNG_SMCCC_TRNG) += smccc_trng.o > +obj-$(CONFIG_RNG_ARM_RNDR) += arm_rndr.o > obj-$(CONFIG_TPM_RNG) += tpm_rng.o > diff --git a/drivers/rng/arm_rndr.c b/drivers/rng/arm_rndr.c > new file mode 100644 > index 00000000000..55989743eae > --- /dev/null > +++ b/drivers/rng/arm_rndr.c > @@ -0,0 +1,82 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* > + * Copyright (c) 2023, Arm Ltd. > + * > + * Use the (optional) ARMv8.5 RNDR register to provide random numbers to > + * U-Boot's UCLASS_RNG users. > + * Detection is done at runtime using the CPU ID registers. > + */ > + > +#define LOG_CATEGORY UCLASS_RNG > + > +#include <common.h> > +#include <dm.h> > +#include <linux/kernel.h> > +#include <rng.h> > +#include <asm/system.h> > + > +#define DRIVER_NAME "arm-rndr" > + > +static bool cpu_has_rndr(void) > +{ > + uint64_t reg; > + > + __asm__ volatile("mrs %0, ID_AA64ISAR0_EL1\n" : "=r" (reg)); > + return !!(reg & ID_AA64ISAR0_EL1_RNDR); > +} > + > +/* > + * The system register name is RNDR, but this isn't widely known among older > + * toolchains, and also triggers errors because of it being an architecture > + * extension. Since we check the availability of the register before, it's > + * fine to use here, though. > + */ > +#define RNDR "S3_3_C2_C4_0" > + > +static uint64_t read_rndr(void) > +{ > + uint64_t reg; > + > + __asm__ volatile("mrs %0, " RNDR "\n" : "=r" (reg)); > + > + return reg; > +} > + > +static int arm_rndr_read(struct udevice *dev, void *data, size_t len) > +{ > + uint64_t random; > + > + while (len) { > + int tocopy = min(sizeof(uint64_t), len); > + > + random = read_rndr(); > + memcpy(data, &random, tocopy); > + len -= tocopy; > + data += tocopy; > + } > + > + return 0; > +} > + > +static const struct dm_rng_ops arm_rndr_ops = { > + .read = arm_rndr_read, > +}; > + > +static int arm_rndr_probe(struct udevice *dev) > +{ > + if (!cpu_has_rndr()) > + return -ENODEV; Can you put this in a bind() function instead, if not too expensive? > + > + return 0; > +} > + > +U_BOOT_DRIVER(arm_rndr) = { > + .name = DRIVER_NAME, > + .id = UCLASS_RNG, > + .ops = &arm_rndr_ops, > + .probe = arm_rndr_probe, > +}; > + > +U_BOOT_DRVINFO(cpu_arm_rndr) = { > + .name = DRIVER_NAME, > +}; > -- > 2.25.1 > Regards, Simon
On Wed, 30 Aug 2023 20:49:18 -0600 Simon Glass <sjg@chromium.org> wrote: Hi Simon, > On Wed, 30 Aug 2023 at 05:32, Andre Przywara <andre.przywara@arm.com> wrote: > > > > The ARMv8.5 architecture extension defines architectural RNDR/RNDRRS > > system registers, that provide 64 bits worth of randomness on every > > read. Since it's an extension, and implementing it is optional, there is > > a field in the ID_AA64ISAR0_EL1 ID register to query the availability > > of those registers. > > > > Add a UCLASS_RNG driver that returns entropy via repeated reads from > > those system registers, if the extension is implemented. > > The driver always binds, but checks the availability in the probe() > > routine. > > > > This helps systems which suffer from low boot entropy, since U-Boot can > > provide entropy via the generic UEFI entropy gathering protocol to the OS, > > at an early stage. > > > > Signed-off-by: Andre Przywara <andre.przywara@arm.com> > > --- > > Hi Simon, > > > > not sure if this is the expected way to model a driver which autodetects > > its device at runtime. It somewhat works, but always lists the bound > > device, even if the registers are not supported. If I do the check in bind > > instead, it fails booting when this returns -ENODEV, since it expects > > the fixed device to always bind successfully, I guess? > > Is there any other way of modeling this? And before you say your > > favourite two letters: this is not a DT job, since it can be safely > > auto-detected in an architectural way. > > > > Thanks, > > Andre > > > > arch/arm/include/asm/system.h | 1 + > > drivers/rng/Kconfig | 6 +++ > > drivers/rng/Makefile | 1 + > > drivers/rng/arm_rndr.c | 82 +++++++++++++++++++++++++++++++++++ > > 4 files changed, 90 insertions(+) > > create mode 100644 drivers/rng/arm_rndr.c > > Reviewed-by: Simon Glass <sjg@chromium.org> thanks! > nit below > > > > > diff --git a/arch/arm/include/asm/system.h b/arch/arm/include/asm/system.h > > index 87d1c77e8b1..0eae857e73a 100644 > > --- a/arch/arm/include/asm/system.h > > +++ b/arch/arm/include/asm/system.h > > @@ -84,6 +84,7 @@ > > #define HCR_EL2_HCD_DIS (1 << 29) /* Hypervisor Call disabled */ > > #define HCR_EL2_AMO_EL2 (1 << 5) /* Route SErrors to EL2 */ > > > > +#define ID_AA64ISAR0_EL1_RNDR (0xFUL << 60) /* RNDR random registers */ > > /* > > * ID_AA64ISAR1_EL1 bits definitions > > */ > > diff --git a/drivers/rng/Kconfig b/drivers/rng/Kconfig > > index 5deb5db5b71..72788d479cc 100644 > > --- a/drivers/rng/Kconfig > > +++ b/drivers/rng/Kconfig > > @@ -76,6 +76,12 @@ config RNG_SMCCC_TRNG > > Enable random number generator for platforms that support Arm > > SMCCC TRNG interface. > > > > +config RNG_ARM_RNDR > > + bool "Generic ARMv8.5 RNDR register" > > + depends on DM_RNG && ARM64 > > + help > > + Use the ARMv8.5 RNDR register to provide random numbers. > > + > > config TPM_RNG > > bool "Enable random number generator on TPM device" > > depends on TPM > > diff --git a/drivers/rng/Makefile b/drivers/rng/Makefile > > index 78f61051acf..572fa7a0c66 100644 > > --- a/drivers/rng/Makefile > > +++ b/drivers/rng/Makefile > > @@ -13,4 +13,5 @@ obj-$(CONFIG_RNG_STM32MP1) += stm32mp1_rng.o > > obj-$(CONFIG_RNG_ROCKCHIP) += rockchip_rng.o > > obj-$(CONFIG_RNG_IPROC200) += iproc_rng200.o > > obj-$(CONFIG_RNG_SMCCC_TRNG) += smccc_trng.o > > +obj-$(CONFIG_RNG_ARM_RNDR) += arm_rndr.o > > obj-$(CONFIG_TPM_RNG) += tpm_rng.o > > diff --git a/drivers/rng/arm_rndr.c b/drivers/rng/arm_rndr.c > > new file mode 100644 > > index 00000000000..55989743eae > > --- /dev/null > > +++ b/drivers/rng/arm_rndr.c > > @@ -0,0 +1,82 @@ > > +// SPDX-License-Identifier: GPL-2.0 > > +/* > > + * Copyright (c) 2023, Arm Ltd. > > + * > > + * Use the (optional) ARMv8.5 RNDR register to provide random numbers to > > + * U-Boot's UCLASS_RNG users. > > + * Detection is done at runtime using the CPU ID registers. > > + */ > > + > > +#define LOG_CATEGORY UCLASS_RNG > > + > > +#include <common.h> > > +#include <dm.h> > > +#include <linux/kernel.h> > > +#include <rng.h> > > +#include <asm/system.h> > > + > > +#define DRIVER_NAME "arm-rndr" > > + > > +static bool cpu_has_rndr(void) > > +{ > > + uint64_t reg; > > + > > + __asm__ volatile("mrs %0, ID_AA64ISAR0_EL1\n" : "=r" (reg)); > > + return !!(reg & ID_AA64ISAR0_EL1_RNDR); > > +} > > + > > +/* > > + * The system register name is RNDR, but this isn't widely known among older > > + * toolchains, and also triggers errors because of it being an architecture > > + * extension. Since we check the availability of the register before, it's > > + * fine to use here, though. > > + */ > > +#define RNDR "S3_3_C2_C4_0" > > + > > +static uint64_t read_rndr(void) > > +{ > > + uint64_t reg; > > + > > + __asm__ volatile("mrs %0, " RNDR "\n" : "=r" (reg)); > > + > > + return reg; > > +} > > + > > +static int arm_rndr_read(struct udevice *dev, void *data, size_t len) > > +{ > > + uint64_t random; > > + > > + while (len) { > > + int tocopy = min(sizeof(uint64_t), len); > > + > > + random = read_rndr(); > > + memcpy(data, &random, tocopy); > > + len -= tocopy; > > + data += tocopy; > > + } > > + > > + return 0; > > +} > > + > > +static const struct dm_rng_ops arm_rndr_ops = { > > + .read = arm_rndr_read, > > +}; > > + > > +static int arm_rndr_probe(struct udevice *dev) > > +{ > > + if (!cpu_has_rndr()) > > + return -ENODEV; > > Can you put this in a bind() function instead, if not too expensive? That's the point I was wondering about (see my message to you above, after the S-o-B): it seems more natural there, but stops the boot. Apparently DM expects those fixed devices (U_BOOT_DRVINFO) to always succeed their bind() calls? Because if I put it in bind, I get: ================ No match for driver 'arm-rndr' initcall sequence 00000000fefc4528 failed at call 000000008801d104 (err=-19) ### ERROR ### Please RESET the board ### ================ if the feature is not available. Any suggestions how to use bind and avoid that crash are highly welcome. And regarding "expensive": This is reading a fixed, read-only system register. There is probably nothing cheaper than that on a modern chip. Cheers, Andre > > + > > + return 0; > > +} > > + > > +U_BOOT_DRIVER(arm_rndr) = { > > + .name = DRIVER_NAME, > > + .id = UCLASS_RNG, > > + .ops = &arm_rndr_ops, > > + .probe = arm_rndr_probe, > > +}; > > + > > +U_BOOT_DRVINFO(cpu_arm_rndr) = { > > + .name = DRIVER_NAME, > > +}; > > -- > > 2.25.1 > > > > Regards, > Simon
Hi Andre, On Thu, 31 Aug 2023 at 06:43, Andre Przywara <andre.przywara@arm.com> wrote: > > On Wed, 30 Aug 2023 20:49:18 -0600 > Simon Glass <sjg@chromium.org> wrote: > > Hi Simon, > > > On Wed, 30 Aug 2023 at 05:32, Andre Przywara <andre.przywara@arm.com> wrote: > > > > > > The ARMv8.5 architecture extension defines architectural RNDR/RNDRRS > > > system registers, that provide 64 bits worth of randomness on every > > > read. Since it's an extension, and implementing it is optional, there is > > > a field in the ID_AA64ISAR0_EL1 ID register to query the availability > > > of those registers. > > > > > > Add a UCLASS_RNG driver that returns entropy via repeated reads from > > > those system registers, if the extension is implemented. > > > The driver always binds, but checks the availability in the probe() > > > routine. > > > > > > This helps systems which suffer from low boot entropy, since U-Boot can > > > provide entropy via the generic UEFI entropy gathering protocol to the OS, > > > at an early stage. > > > > > > Signed-off-by: Andre Przywara <andre.przywara@arm.com> > > > --- > > > Hi Simon, > > > > > > not sure if this is the expected way to model a driver which autodetects > > > its device at runtime. It somewhat works, but always lists the bound > > > device, even if the registers are not supported. If I do the check in bind > > > instead, it fails booting when this returns -ENODEV, since it expects > > > the fixed device to always bind successfully, I guess? > > > Is there any other way of modeling this? And before you say your > > > favourite two letters: this is not a DT job, since it can be safely > > > auto-detected in an architectural way. > > > > > > Thanks, > > > Andre > > > > > > arch/arm/include/asm/system.h | 1 + > > > drivers/rng/Kconfig | 6 +++ > > > drivers/rng/Makefile | 1 + > > > drivers/rng/arm_rndr.c | 82 +++++++++++++++++++++++++++++++++++ > > > 4 files changed, 90 insertions(+) > > > create mode 100644 drivers/rng/arm_rndr.c > > > > Reviewed-by: Simon Glass <sjg@chromium.org> > > thanks! > > > nit below > > > > > > > > diff --git a/arch/arm/include/asm/system.h b/arch/arm/include/asm/system.h > > > index 87d1c77e8b1..0eae857e73a 100644 > > > --- a/arch/arm/include/asm/system.h > > > +++ b/arch/arm/include/asm/system.h > > > @@ -84,6 +84,7 @@ > > > #define HCR_EL2_HCD_DIS (1 << 29) /* Hypervisor Call disabled */ > > > #define HCR_EL2_AMO_EL2 (1 << 5) /* Route SErrors to EL2 */ > > > > > > +#define ID_AA64ISAR0_EL1_RNDR (0xFUL << 60) /* RNDR random registers */ > > > /* > > > * ID_AA64ISAR1_EL1 bits definitions > > > */ > > > diff --git a/drivers/rng/Kconfig b/drivers/rng/Kconfig > > > index 5deb5db5b71..72788d479cc 100644 > > > --- a/drivers/rng/Kconfig > > > +++ b/drivers/rng/Kconfig > > > @@ -76,6 +76,12 @@ config RNG_SMCCC_TRNG > > > Enable random number generator for platforms that support Arm > > > SMCCC TRNG interface. > > > > > > +config RNG_ARM_RNDR > > > + bool "Generic ARMv8.5 RNDR register" > > > + depends on DM_RNG && ARM64 > > > + help > > > + Use the ARMv8.5 RNDR register to provide random numbers. > > > + > > > config TPM_RNG > > > bool "Enable random number generator on TPM device" > > > depends on TPM > > > diff --git a/drivers/rng/Makefile b/drivers/rng/Makefile > > > index 78f61051acf..572fa7a0c66 100644 > > > --- a/drivers/rng/Makefile > > > +++ b/drivers/rng/Makefile > > > @@ -13,4 +13,5 @@ obj-$(CONFIG_RNG_STM32MP1) += stm32mp1_rng.o > > > obj-$(CONFIG_RNG_ROCKCHIP) += rockchip_rng.o > > > obj-$(CONFIG_RNG_IPROC200) += iproc_rng200.o > > > obj-$(CONFIG_RNG_SMCCC_TRNG) += smccc_trng.o > > > +obj-$(CONFIG_RNG_ARM_RNDR) += arm_rndr.o > > > obj-$(CONFIG_TPM_RNG) += tpm_rng.o > > > diff --git a/drivers/rng/arm_rndr.c b/drivers/rng/arm_rndr.c > > > new file mode 100644 > > > index 00000000000..55989743eae > > > --- /dev/null > > > +++ b/drivers/rng/arm_rndr.c > > > @@ -0,0 +1,82 @@ > > > +// SPDX-License-Identifier: GPL-2.0 > > > +/* > > > + * Copyright (c) 2023, Arm Ltd. > > > + * > > > + * Use the (optional) ARMv8.5 RNDR register to provide random numbers to > > > + * U-Boot's UCLASS_RNG users. > > > + * Detection is done at runtime using the CPU ID registers. > > > + */ > > > + > > > +#define LOG_CATEGORY UCLASS_RNG > > > + > > > +#include <common.h> > > > +#include <dm.h> > > > +#include <linux/kernel.h> > > > +#include <rng.h> > > > +#include <asm/system.h> > > > + > > > +#define DRIVER_NAME "arm-rndr" > > > + > > > +static bool cpu_has_rndr(void) > > > +{ > > > + uint64_t reg; > > > + > > > + __asm__ volatile("mrs %0, ID_AA64ISAR0_EL1\n" : "=r" (reg)); > > > + return !!(reg & ID_AA64ISAR0_EL1_RNDR); > > > +} > > > + > > > +/* > > > + * The system register name is RNDR, but this isn't widely known among older > > > + * toolchains, and also triggers errors because of it being an architecture > > > + * extension. Since we check the availability of the register before, it's > > > + * fine to use here, though. > > > + */ > > > +#define RNDR "S3_3_C2_C4_0" > > > + > > > +static uint64_t read_rndr(void) > > > +{ > > > + uint64_t reg; > > > + > > > + __asm__ volatile("mrs %0, " RNDR "\n" : "=r" (reg)); > > > + > > > + return reg; > > > +} > > > + > > > +static int arm_rndr_read(struct udevice *dev, void *data, size_t len) > > > +{ > > > + uint64_t random; > > > + > > > + while (len) { > > > + int tocopy = min(sizeof(uint64_t), len); > > > + > > > + random = read_rndr(); > > > + memcpy(data, &random, tocopy); > > > + len -= tocopy; > > > + data += tocopy; > > > + } > > > + > > > + return 0; > > > +} > > > + > > > +static const struct dm_rng_ops arm_rndr_ops = { > > > + .read = arm_rndr_read, > > > +}; > > > + > > > +static int arm_rndr_probe(struct udevice *dev) > > > +{ > > > + if (!cpu_has_rndr()) > > > + return -ENODEV; > > > > Can you put this in a bind() function instead, if not too expensive? > > That's the point I was wondering about (see my message to you above, after > the S-o-B): it seems more natural there, but stops the boot. > Apparently DM expects those fixed devices (U_BOOT_DRVINFO) to always > succeed their bind() calls? Because if I put it in bind, I get: > ================ > No match for driver 'arm-rndr' > initcall sequence 00000000fefc4528 failed at call 000000008801d104 (err=-19) > ### ERROR ### Please RESET the board ### > ================ > if the feature is not available. > Any suggestions how to use bind and avoid that crash are highly welcome. An #ifdef is the only thing I can suggest. Why not add the node to the DT? > > And regarding "expensive": This is reading a fixed, read-only system > register. There is probably nothing cheaper than that on a modern chip. No I meant expensive in terms of code size, but I don't think a bind() function matters here. [..] Regards, Simon
On Thu, 31 Aug 2023 13:01:57 -0600 Simon Glass <sjg@chromium.org> wrote: Hi Simon, > On Thu, 31 Aug 2023 at 06:43, Andre Przywara <andre.przywara@arm.com> wrote: > > > > On Wed, 30 Aug 2023 20:49:18 -0600 > > Simon Glass <sjg@chromium.org> wrote: > > > > Hi Simon, > > > > > On Wed, 30 Aug 2023 at 05:32, Andre Przywara <andre.przywara@arm.com> wrote: > > > > > > > > The ARMv8.5 architecture extension defines architectural RNDR/RNDRRS > > > > system registers, that provide 64 bits worth of randomness on every > > > > read. Since it's an extension, and implementing it is optional, there is > > > > a field in the ID_AA64ISAR0_EL1 ID register to query the availability > > > > of those registers. > > > > > > > > Add a UCLASS_RNG driver that returns entropy via repeated reads from > > > > those system registers, if the extension is implemented. > > > > The driver always binds, but checks the availability in the probe() > > > > routine. > > > > > > > > This helps systems which suffer from low boot entropy, since U-Boot can > > > > provide entropy via the generic UEFI entropy gathering protocol to the OS, > > > > at an early stage. > > > > > > > > Signed-off-by: Andre Przywara <andre.przywara@arm.com> > > > > --- > > > > Hi Simon, > > > > > > > > not sure if this is the expected way to model a driver which autodetects > > > > its device at runtime. It somewhat works, but always lists the bound > > > > device, even if the registers are not supported. If I do the check in bind > > > > instead, it fails booting when this returns -ENODEV, since it expects > > > > the fixed device to always bind successfully, I guess? > > > > Is there any other way of modeling this? And before you say your > > > > favourite two letters: this is not a DT job, since it can be safely > > > > auto-detected in an architectural way. > > > > > > > > Thanks, > > > > Andre > > > > > > > > arch/arm/include/asm/system.h | 1 + > > > > drivers/rng/Kconfig | 6 +++ > > > > drivers/rng/Makefile | 1 + > > > > drivers/rng/arm_rndr.c | 82 +++++++++++++++++++++++++++++++++++ > > > > 4 files changed, 90 insertions(+) > > > > create mode 100644 drivers/rng/arm_rndr.c > > > > > > Reviewed-by: Simon Glass <sjg@chromium.org> > > > > thanks! > > > > > nit below > > > > > > > > > > > diff --git a/arch/arm/include/asm/system.h b/arch/arm/include/asm/system.h > > > > index 87d1c77e8b1..0eae857e73a 100644 > > > > --- a/arch/arm/include/asm/system.h > > > > +++ b/arch/arm/include/asm/system.h > > > > @@ -84,6 +84,7 @@ > > > > #define HCR_EL2_HCD_DIS (1 << 29) /* Hypervisor Call disabled */ > > > > #define HCR_EL2_AMO_EL2 (1 << 5) /* Route SErrors to EL2 */ > > > > > > > > +#define ID_AA64ISAR0_EL1_RNDR (0xFUL << 60) /* RNDR random registers */ > > > > /* > > > > * ID_AA64ISAR1_EL1 bits definitions > > > > */ > > > > diff --git a/drivers/rng/Kconfig b/drivers/rng/Kconfig > > > > index 5deb5db5b71..72788d479cc 100644 > > > > --- a/drivers/rng/Kconfig > > > > +++ b/drivers/rng/Kconfig > > > > @@ -76,6 +76,12 @@ config RNG_SMCCC_TRNG > > > > Enable random number generator for platforms that support Arm > > > > SMCCC TRNG interface. > > > > > > > > +config RNG_ARM_RNDR > > > > + bool "Generic ARMv8.5 RNDR register" > > > > + depends on DM_RNG && ARM64 > > > > + help > > > > + Use the ARMv8.5 RNDR register to provide random numbers. > > > > + > > > > config TPM_RNG > > > > bool "Enable random number generator on TPM device" > > > > depends on TPM > > > > diff --git a/drivers/rng/Makefile b/drivers/rng/Makefile > > > > index 78f61051acf..572fa7a0c66 100644 > > > > --- a/drivers/rng/Makefile > > > > +++ b/drivers/rng/Makefile > > > > @@ -13,4 +13,5 @@ obj-$(CONFIG_RNG_STM32MP1) += stm32mp1_rng.o > > > > obj-$(CONFIG_RNG_ROCKCHIP) += rockchip_rng.o > > > > obj-$(CONFIG_RNG_IPROC200) += iproc_rng200.o > > > > obj-$(CONFIG_RNG_SMCCC_TRNG) += smccc_trng.o > > > > +obj-$(CONFIG_RNG_ARM_RNDR) += arm_rndr.o > > > > obj-$(CONFIG_TPM_RNG) += tpm_rng.o > > > > diff --git a/drivers/rng/arm_rndr.c b/drivers/rng/arm_rndr.c > > > > new file mode 100644 > > > > index 00000000000..55989743eae > > > > --- /dev/null > > > > +++ b/drivers/rng/arm_rndr.c > > > > @@ -0,0 +1,82 @@ > > > > +// SPDX-License-Identifier: GPL-2.0 > > > > +/* > > > > + * Copyright (c) 2023, Arm Ltd. > > > > + * > > > > + * Use the (optional) ARMv8.5 RNDR register to provide random numbers to > > > > + * U-Boot's UCLASS_RNG users. > > > > + * Detection is done at runtime using the CPU ID registers. > > > > + */ > > > > + > > > > +#define LOG_CATEGORY UCLASS_RNG > > > > + > > > > +#include <common.h> > > > > +#include <dm.h> > > > > +#include <linux/kernel.h> > > > > +#include <rng.h> > > > > +#include <asm/system.h> > > > > + > > > > +#define DRIVER_NAME "arm-rndr" > > > > + > > > > +static bool cpu_has_rndr(void) > > > > +{ > > > > + uint64_t reg; > > > > + > > > > + __asm__ volatile("mrs %0, ID_AA64ISAR0_EL1\n" : "=r" (reg)); > > > > + return !!(reg & ID_AA64ISAR0_EL1_RNDR); > > > > +} > > > > + > > > > +/* > > > > + * The system register name is RNDR, but this isn't widely known among older > > > > + * toolchains, and also triggers errors because of it being an architecture > > > > + * extension. Since we check the availability of the register before, it's > > > > + * fine to use here, though. > > > > + */ > > > > +#define RNDR "S3_3_C2_C4_0" > > > > + > > > > +static uint64_t read_rndr(void) > > > > +{ > > > > + uint64_t reg; > > > > + > > > > + __asm__ volatile("mrs %0, " RNDR "\n" : "=r" (reg)); > > > > + > > > > + return reg; > > > > +} > > > > + > > > > +static int arm_rndr_read(struct udevice *dev, void *data, size_t len) > > > > +{ > > > > + uint64_t random; > > > > + > > > > + while (len) { > > > > + int tocopy = min(sizeof(uint64_t), len); > > > > + > > > > + random = read_rndr(); > > > > + memcpy(data, &random, tocopy); > > > > + len -= tocopy; > > > > + data += tocopy; > > > > + } > > > > + > > > > + return 0; > > > > +} > > > > + > > > > +static const struct dm_rng_ops arm_rndr_ops = { > > > > + .read = arm_rndr_read, > > > > +}; > > > > + > > > > +static int arm_rndr_probe(struct udevice *dev) > > > > +{ > > > > + if (!cpu_has_rndr()) > > > > + return -ENODEV; > > > > > > Can you put this in a bind() function instead, if not too expensive? > > > > That's the point I was wondering about (see my message to you above, after > > the S-o-B): it seems more natural there, but stops the boot. > > Apparently DM expects those fixed devices (U_BOOT_DRVINFO) to always > > succeed their bind() calls? Because if I put it in bind, I get: > > ================ > > No match for driver 'arm-rndr' > > initcall sequence 00000000fefc4528 failed at call 000000008801d104 (err=-19) > > ### ERROR ### Please RESET the board ### > > ================ > > if the feature is not available. > > Any suggestions how to use bind and avoid that crash are highly welcome. > > An #ifdef is the only thing I can suggest. > > Why not add the node to the DT? Because the RNDR system registers are architected and perfectly discoverable, so out of scope for a devicetree. The registers are arguably not a device, but a CPU feature, though I found it useful to represent them as a UCLASS_RNG device, since this ties in neatly and automatically to the rest of U-Boot, specifically the UEFI loader, which makes use of it to provide early entropy to the OS. So either I leave it as it is, living with the fact that it always shows as "bound" to the driver, even when the feature doesn't exist, or I add extra code to the efi_rng.c's getrng() function to discover the CPU feature there. But the latter sounds arbitrary, and would add arch specific bits to the generic EFI code, plus would also leave out the other UCLASS_RNG users like kaslrseed. > > And regarding "expensive": This is reading a fixed, read-only system > > register. There is probably nothing cheaper than that on a modern chip. > > No I meant expensive in terms of code size, but I don't think a bind() > function matters here. This driver mostly consists of boilerplate code, so I am not worried about code size at all, and we are not particularly constrained in size. The feature check itself is typically like three or four assembly instructions. Cheers, Andre
On Wed, Aug 30, 2023 at 12:32:30PM +0100, Andre Przywara wrote: > The ARMv8.5 architecture extension defines architectural RNDR/RNDRRS > system registers, that provide 64 bits worth of randomness on every > read. Since it's an extension, and implementing it is optional, there is > a field in the ID_AA64ISAR0_EL1 ID register to query the availability > of those registers. > > Add a UCLASS_RNG driver that returns entropy via repeated reads from > those system registers, if the extension is implemented. > The driver always binds, but checks the availability in the probe() > routine. > > This helps systems which suffer from low boot entropy, since U-Boot can > provide entropy via the generic UEFI entropy gathering protocol to the OS, > at an early stage. > > Signed-off-by: Andre Przywara <andre.przywara@arm.com> > Reviewed-by: Simon Glass <sjg@chromium.org> Applied to u-boot/master, thanks!
diff --git a/arch/arm/include/asm/system.h b/arch/arm/include/asm/system.h index 87d1c77e8b1..0eae857e73a 100644 --- a/arch/arm/include/asm/system.h +++ b/arch/arm/include/asm/system.h @@ -84,6 +84,7 @@ #define HCR_EL2_HCD_DIS (1 << 29) /* Hypervisor Call disabled */ #define HCR_EL2_AMO_EL2 (1 << 5) /* Route SErrors to EL2 */ +#define ID_AA64ISAR0_EL1_RNDR (0xFUL << 60) /* RNDR random registers */ /* * ID_AA64ISAR1_EL1 bits definitions */ diff --git a/drivers/rng/Kconfig b/drivers/rng/Kconfig index 5deb5db5b71..72788d479cc 100644 --- a/drivers/rng/Kconfig +++ b/drivers/rng/Kconfig @@ -76,6 +76,12 @@ config RNG_SMCCC_TRNG Enable random number generator for platforms that support Arm SMCCC TRNG interface. +config RNG_ARM_RNDR + bool "Generic ARMv8.5 RNDR register" + depends on DM_RNG && ARM64 + help + Use the ARMv8.5 RNDR register to provide random numbers. + config TPM_RNG bool "Enable random number generator on TPM device" depends on TPM diff --git a/drivers/rng/Makefile b/drivers/rng/Makefile index 78f61051acf..572fa7a0c66 100644 --- a/drivers/rng/Makefile +++ b/drivers/rng/Makefile @@ -13,4 +13,5 @@ obj-$(CONFIG_RNG_STM32MP1) += stm32mp1_rng.o obj-$(CONFIG_RNG_ROCKCHIP) += rockchip_rng.o obj-$(CONFIG_RNG_IPROC200) += iproc_rng200.o obj-$(CONFIG_RNG_SMCCC_TRNG) += smccc_trng.o +obj-$(CONFIG_RNG_ARM_RNDR) += arm_rndr.o obj-$(CONFIG_TPM_RNG) += tpm_rng.o diff --git a/drivers/rng/arm_rndr.c b/drivers/rng/arm_rndr.c new file mode 100644 index 00000000000..55989743eae --- /dev/null +++ b/drivers/rng/arm_rndr.c @@ -0,0 +1,82 @@ +// SPDX-License-Identifier: GPL-2.0 +/* + * Copyright (c) 2023, Arm Ltd. + * + * Use the (optional) ARMv8.5 RNDR register to provide random numbers to + * U-Boot's UCLASS_RNG users. + * Detection is done at runtime using the CPU ID registers. + */ + +#define LOG_CATEGORY UCLASS_RNG + +#include <common.h> +#include <dm.h> +#include <linux/kernel.h> +#include <rng.h> +#include <asm/system.h> + +#define DRIVER_NAME "arm-rndr" + +static bool cpu_has_rndr(void) +{ + uint64_t reg; + + __asm__ volatile("mrs %0, ID_AA64ISAR0_EL1\n" : "=r" (reg)); + return !!(reg & ID_AA64ISAR0_EL1_RNDR); +} + +/* + * The system register name is RNDR, but this isn't widely known among older + * toolchains, and also triggers errors because of it being an architecture + * extension. Since we check the availability of the register before, it's + * fine to use here, though. + */ +#define RNDR "S3_3_C2_C4_0" + +static uint64_t read_rndr(void) +{ + uint64_t reg; + + __asm__ volatile("mrs %0, " RNDR "\n" : "=r" (reg)); + + return reg; +} + +static int arm_rndr_read(struct udevice *dev, void *data, size_t len) +{ + uint64_t random; + + while (len) { + int tocopy = min(sizeof(uint64_t), len); + + random = read_rndr(); + memcpy(data, &random, tocopy); + len -= tocopy; + data += tocopy; + } + + return 0; +} + +static const struct dm_rng_ops arm_rndr_ops = { + .read = arm_rndr_read, +}; + +static int arm_rndr_probe(struct udevice *dev) +{ + if (!cpu_has_rndr()) + return -ENODEV; + + return 0; +} + +U_BOOT_DRIVER(arm_rndr) = { + .name = DRIVER_NAME, + .id = UCLASS_RNG, + .ops = &arm_rndr_ops, + .probe = arm_rndr_probe, +}; + +U_BOOT_DRVINFO(cpu_arm_rndr) = { + .name = DRIVER_NAME, +};
The ARMv8.5 architecture extension defines architectural RNDR/RNDRRS system registers, that provide 64 bits worth of randomness on every read. Since it's an extension, and implementing it is optional, there is a field in the ID_AA64ISAR0_EL1 ID register to query the availability of those registers. Add a UCLASS_RNG driver that returns entropy via repeated reads from those system registers, if the extension is implemented. The driver always binds, but checks the availability in the probe() routine. This helps systems which suffer from low boot entropy, since U-Boot can provide entropy via the generic UEFI entropy gathering protocol to the OS, at an early stage. Signed-off-by: Andre Przywara <andre.przywara@arm.com> --- Hi Simon, not sure if this is the expected way to model a driver which autodetects its device at runtime. It somewhat works, but always lists the bound device, even if the registers are not supported. If I do the check in bind instead, it fails booting when this returns -ENODEV, since it expects the fixed device to always bind successfully, I guess? Is there any other way of modeling this? And before you say your favourite two letters: this is not a DT job, since it can be safely auto-detected in an architectural way. Thanks, Andre arch/arm/include/asm/system.h | 1 + drivers/rng/Kconfig | 6 +++ drivers/rng/Makefile | 1 + drivers/rng/arm_rndr.c | 82 +++++++++++++++++++++++++++++++++++ 4 files changed, 90 insertions(+) create mode 100644 drivers/rng/arm_rndr.c