[PATCH v3 0/5] ARM: arm64: Add SMCCC TRNG entropy service

[PATCH v3 0/5] ARM: arm64: Add SMCCC TRNG entropy service

[Andre Przywara]

Hi,

an update to v2 with some fixes and a few tweaks. Ard's patch [1] should
significantly reduce the frequency of arch_get_random_seed_long() calls,
not sure if that is enough the appease the concerns about the
potentially long latency of SMC calls. I also dropped the direct
arch_get_random() call in KVM for the same reason. An alternative could
be to just use the SMC in the _early() versions, but then we would lose
the SMCCC entropy source for the periodic reseeds. This could be mitigated
by using a hwrng driver [2] and rngd.
The only other non-minor change to v2 is the addition of using the SMCCC
call in the _early() variant. For a changelog see below.

Sudeep: patch 1/5 is a prerequisite for all other patches, which
themselves could be considered separate and need to go via different trees.
If we could agree on that one now and get that merged, it would help the
handling of the other patches going forward.

Cheers,
Andre
==============================

The ARM architected TRNG firmware interface, described in ARM spec
DEN0098[3], defines an ARM SMCCC based interface to a true random number
generator, provided by firmware.

This series collects all the patches implementing this in various
places: as a user feeding into the ARCH_RANDOM pool, both for ARM and
arm64, and as a service provider for KVM guests.

Patch 1 introduces the interface definition used by all three entities.
Patch 2 prepares the Arm SMCCC firmware driver to probe for the
interface. This patch is needed to avoid a later dependency on *two*
patches (there might be a better solution to this problem).

Patch 3 implements the ARM part, patch 4 is the arm64 version.
The final patch 5 adds support to provide random numbers to KVM guests.

This was tested on:
- QEMU -kernel (no SMCCC, regression test)
- Juno w/ prototype of the h/w Trusted RNG support
- mainline KVM (SMCCC, but no TRNG: regression test)
- ARM and arm64 KVM guests, using the KVM service in patch 5/5

Based on v5.10-rc3, please let me know if I should rebased on something
else. A git repo is accessible at:
https://gitlab.arm.com/linux-arm/linux-ap/-/commits/smccc-trng/v3/

Cheers,
Andre

[1] http://lists.infradead.org/pipermail/linux-arm-kernel/2020-November/615446.html
[2] https://gitlab.arm.com/linux-arm/linux-ap/-/commit/87e3722f437
[3] https://developer.arm.com/documentation/den0098/latest/

Changelog v2 ... v3:
- ARM: fix compilation with randconfig
- arm64: use SMCCC call also in arch_get_random_seed_long_early()
- KVM: comment on return value usage
- KVM: use more interesting UUID (enjoy, Marc!)
- KVM: use bitmaps instead of open coded long arrays
- KVM: drop direct usage of arch_get_random() interface

Changelog "v1" ... v2:
- trigger ARCH_RANDOM initialisation from the SMCCC firmware driver
- use a single bool in smccc.c to hold the initialisation state for arm64
- handle endianess correctly in the KVM provider

Andre Przywara (2):
  firmware: smccc: Introduce SMCCC TRNG framework
  arm64: Add support for SMCCC TRNG entropy source

Ard Biesheuvel (3):
  firmware: smccc: Add SMCCC TRNG function call IDs
  ARM: implement support for SMCCC TRNG entropy source
  KVM: arm64: implement the TRNG hypervisor call

 arch/arm/Kconfig                    |  4 ++
 arch/arm/include/asm/archrandom.h   | 74 +++++++++++++++++++++++++
 arch/arm64/include/asm/archrandom.h | 79 +++++++++++++++++++++++----
 arch/arm64/include/asm/kvm_host.h   |  2 +
 arch/arm64/kvm/Makefile             |  2 +-
 arch/arm64/kvm/hypercalls.c         |  6 ++
 arch/arm64/kvm/trng.c               | 85 +++++++++++++++++++++++++++++
 drivers/firmware/smccc/smccc.c      |  5 ++
 include/linux/arm-smccc.h           | 31 +++++++++++
 9 files changed, 277 insertions(+), 11 deletions(-)
 create mode 100644 arch/arm/include/asm/archrandom.h
 create mode 100644 arch/arm64/kvm/trng.c

-- 
2.17.1

[PATCH v3 1/5] firmware: smccc: Add SMCCC TRNG function call IDs

[Andre Przywara]

From: Ard Biesheuvel <ardb@kernel.org>

The ARM architected TRNG firmware interface, described in ARM spec
DEN0098, define an ARM SMCCC based interface to a true random number
generator, provided by firmware.

Add the definitions of the SMCCC functions as defined by the spec.

Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
Signed-off-by: Andre Przywara <andre.przywara@arm.com>
---
 include/linux/arm-smccc.h | 31 +++++++++++++++++++++++++++++++
 1 file changed, 31 insertions(+)

diff --git a/include/linux/arm-smccc.h b/include/linux/arm-smccc.h
index f860645f6512..62c54234576c 100644
--- a/include/linux/arm-smccc.h
+++ b/include/linux/arm-smccc.h
@@ -102,6 +102,37 @@
 			   ARM_SMCCC_OWNER_STANDARD_HYP,	\
 			   0x21)
 
+/* TRNG entropy source calls (defined by ARM DEN0098) */
+#define ARM_SMCCC_TRNG_VERSION					\
+	ARM_SMCCC_CALL_VAL(ARM_SMCCC_FAST_CALL,			\
+			   ARM_SMCCC_SMC_32,			\
+			   ARM_SMCCC_OWNER_STANDARD,		\
+			   0x50)
+
+#define ARM_SMCCC_TRNG_FEATURES					\
+	ARM_SMCCC_CALL_VAL(ARM_SMCCC_FAST_CALL,			\
+			   ARM_SMCCC_SMC_32,			\
+			   ARM_SMCCC_OWNER_STANDARD,		\
+			   0x51)
+
+#define ARM_SMCCC_TRNG_GET_UUID					\
+	ARM_SMCCC_CALL_VAL(ARM_SMCCC_FAST_CALL,			\
+			   ARM_SMCCC_SMC_32,			\
+			   ARM_SMCCC_OWNER_STANDARD,		\
+			   0x52)
+
+#define ARM_SMCCC_TRNG_RND32					\
+	ARM_SMCCC_CALL_VAL(ARM_SMCCC_FAST_CALL,			\
+			   ARM_SMCCC_SMC_32,			\
+			   ARM_SMCCC_OWNER_STANDARD,		\
+			   0x53)
+
+#define ARM_SMCCC_TRNG_RND64					\
+	ARM_SMCCC_CALL_VAL(ARM_SMCCC_FAST_CALL,			\
+			   ARM_SMCCC_SMC_64,			\
+			   ARM_SMCCC_OWNER_STANDARD,		\
+			   0x53)
+
 /*
  * Return codes defined in ARM DEN 0070A
  * ARM DEN 0070A is now merged/consolidated into ARM DEN 0028 C
-- 
2.17.1

[PATCH v3 2/5] firmware: smccc: Introduce SMCCC TRNG framework

[Andre Przywara]

The ARM DEN0098 document describe an SMCCC based firmware service to
deliver hardware generated random numbers. Its existence is advertised
according to the SMCCC v1.1 specification.

Add a (dummy) call to probe functions implemented in each architecture
(ARM and arm64), to determine the existence of this interface.
For now this return false, but this will be overwritten by each
architecture's support patch.

Signed-off-by: Andre Przywara <andre.przywara@arm.com>
---
 arch/arm/include/asm/archrandom.h   | 10 ++++++++++
 arch/arm64/include/asm/archrandom.h | 12 ++++++++++++
 drivers/firmware/smccc/smccc.c      |  5 +++++
 3 files changed, 27 insertions(+)
 create mode 100644 arch/arm/include/asm/archrandom.h

diff --git a/arch/arm/include/asm/archrandom.h b/arch/arm/include/asm/archrandom.h
new file mode 100644
index 000000000000..a8e84ca5c2ee
--- /dev/null
+++ b/arch/arm/include/asm/archrandom.h
@@ -0,0 +1,10 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef _ASM_ARCHRANDOM_H
+#define _ASM_ARCHRANDOM_H
+
+static inline bool __init smccc_probe_trng(void)
+{
+	return false;
+}
+
+#endif /* _ASM_ARCHRANDOM_H */
diff --git a/arch/arm64/include/asm/archrandom.h b/arch/arm64/include/asm/archrandom.h
index ffb1a40d5475..abe07c21da8e 100644
--- a/arch/arm64/include/asm/archrandom.h
+++ b/arch/arm64/include/asm/archrandom.h
@@ -8,6 +8,11 @@
 #include <linux/kernel.h>
 #include <asm/cpufeature.h>
 
+static inline bool __init smccc_probe_trng(void)
+{
+	return false;
+}
+
 static inline bool __arm64_rndr(unsigned long *v)
 {
 	bool ok;
@@ -79,5 +84,12 @@ arch_get_random_seed_long_early(unsigned long *v)
 }
 #define arch_get_random_seed_long_early arch_get_random_seed_long_early
 
+#else /* !CONFIG_ARCH_RANDOM */
+
+static inline bool __init smccc_probe_trng(void)
+{
+	return false;
+}
+
 #endif /* CONFIG_ARCH_RANDOM */
 #endif /* _ASM_ARCHRANDOM_H */
diff --git a/drivers/firmware/smccc/smccc.c b/drivers/firmware/smccc/smccc.c
index 00c88b809c0c..74c84b9559f6 100644
--- a/drivers/firmware/smccc/smccc.c
+++ b/drivers/firmware/smccc/smccc.c
@@ -7,14 +7,19 @@
 
 #include <linux/init.h>
 #include <linux/arm-smccc.h>
+#include <asm/archrandom.h>
 
 static u32 smccc_version = ARM_SMCCC_VERSION_1_0;
 static enum arm_smccc_conduit smccc_conduit = SMCCC_CONDUIT_NONE;
 
+bool __ro_after_init smccc_trng_available = false;
+
 void __init arm_smccc_version_init(u32 version, enum arm_smccc_conduit conduit)
 {
 	smccc_version = version;
 	smccc_conduit = conduit;
+
+	smccc_trng_available = smccc_probe_trng();
 }
 
 enum arm_smccc_conduit arm_smccc_1_1_get_conduit(void)
-- 
2.17.1

[PATCH v3 3/5] ARM: implement support for SMCCC TRNG entropy source

[Andre Przywara]

From: Ard Biesheuvel <ardb@kernel.org>

Implement arch_get_random_seed_*() for ARM based on the firmware
or hypervisor provided entropy source described in ARM DEN0098.

This will make the kernel's random number generator consume entropy
provided by this interface, at early boot, and periodically at
runtime when reseeding.

Cc: Linus Walleij <linus.walleij@linaro.org>
Cc: Russell King <linux@armlinux.org.uk>
Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
[Andre: rework to be initialised by the SMCCC firmware driver]
Signed-off-by: Andre Przywara <andre.przywara@arm.com>
---
 arch/arm/Kconfig                  |  4 ++
 arch/arm/include/asm/archrandom.h | 64 +++++++++++++++++++++++++++++++
 2 files changed, 68 insertions(+)

diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
index fe2f17eb2b50..a6d2d66f4110 100644
--- a/arch/arm/Kconfig
+++ b/arch/arm/Kconfig
@@ -1667,6 +1667,10 @@ config STACKPROTECTOR_PER_TASK
 	  Enable this option to switch to a different method that uses a
 	  different canary value for each task.
 
+config ARCH_RANDOM
+	def_bool y
+	depends on HAVE_ARM_SMCCC_DISCOVERY
+
 endmenu
 
 menu "Boot options"
diff --git a/arch/arm/include/asm/archrandom.h b/arch/arm/include/asm/archrandom.h
index a8e84ca5c2ee..f3e96a5b65f8 100644
--- a/arch/arm/include/asm/archrandom.h
+++ b/arch/arm/include/asm/archrandom.h
@@ -2,9 +2,73 @@
 #ifndef _ASM_ARCHRANDOM_H
 #define _ASM_ARCHRANDOM_H
 
+#ifdef CONFIG_ARCH_RANDOM
+
+#include <linux/arm-smccc.h>
+#include <linux/kernel.h>
+
+#define ARM_SMCCC_TRNG_MIN_VERSION     0x10000UL
+
+extern bool smccc_trng_available;
+
+static inline bool __init smccc_probe_trng(void)
+{
+	struct arm_smccc_res res;
+
+	arm_smccc_1_1_invoke(ARM_SMCCC_TRNG_VERSION, &res);
+	if ((s32)res.a0 < 0)
+		return false;
+	if (res.a0 >= ARM_SMCCC_TRNG_MIN_VERSION) {
+		/* double check that the 32-bit flavor is available */
+		arm_smccc_1_1_invoke(ARM_SMCCC_TRNG_FEATURES,
+				     ARM_SMCCC_TRNG_RND32,
+				     &res);
+		if ((s32)res.a0 >= 0)
+			return true;
+	}
+
+	return false;
+}
+
+static inline bool __must_check arch_get_random_long(unsigned long *v)
+{
+	return false;
+}
+
+static inline bool __must_check arch_get_random_int(unsigned int *v)
+{
+	return false;
+}
+
+static inline bool __must_check arch_get_random_seed_long(unsigned long *v)
+{
+	struct arm_smccc_res res;
+
+	if (smccc_trng_available) {
+		arm_smccc_1_1_invoke(ARM_SMCCC_TRNG_RND32, 8 * sizeof(*v), &res);
+
+		if (res.a0 != 0)
+			return false;
+
+		*v = res.a3;
+		return true;
+	}
+
+	return false;
+}
+
+static inline bool __must_check arch_get_random_seed_int(unsigned int *v)
+{
+	return arch_get_random_seed_long((unsigned long *)v);
+}
+
+
+#else /* !CONFIG_ARCH_RANDOM */
+
 static inline bool __init smccc_probe_trng(void)
 {
 	return false;
 }
 
+#endif /* CONFIG_ARCH_RANDOM */
 #endif /* _ASM_ARCHRANDOM_H */
-- 
2.17.1

[PATCH v3 4/5] arm64: Add support for SMCCC TRNG entropy source

[Andre Przywara]

The ARM architected TRNG firmware interface, described in ARM spec
DEN0098, defines an ARM SMCCC based interface to a true random number
generator, provided by firmware.
This can be discovered via the SMCCC >=v1.1 interface, and provides
up to 192 bits of entropy per call.

Hook this SMC call into arm64's arch_get_random_*() implementation,
coming to the rescue when the CPU does not implement the ARM v8.5 RNG
system registers.

For the detection, we piggy back on the PSCI/SMCCC discovery (which gives
us the conduit to use (hvc/smc)), then try to call the
ARM_SMCCC_TRNG_VERSION function, which returns -1 if this interface is
not implemented.

Signed-off-by: Andre Przywara <andre.przywara@arm.com>
---
 arch/arm64/include/asm/archrandom.h | 69 ++++++++++++++++++++++++-----
 1 file changed, 58 insertions(+), 11 deletions(-)

diff --git a/arch/arm64/include/asm/archrandom.h b/arch/arm64/include/asm/archrandom.h
index abe07c21da8e..fe34bfd30caa 100644
--- a/arch/arm64/include/asm/archrandom.h
+++ b/arch/arm64/include/asm/archrandom.h
@@ -4,13 +4,24 @@
 
 #ifdef CONFIG_ARCH_RANDOM
 
+#include <linux/arm-smccc.h>
 #include <linux/bug.h>
 #include <linux/kernel.h>
 #include <asm/cpufeature.h>
 
+#define ARM_SMCCC_TRNG_MIN_VERSION	0x10000UL
+
+extern bool smccc_trng_available;
+
 static inline bool __init smccc_probe_trng(void)
 {
-	return false;
+	struct arm_smccc_res res;
+
+	arm_smccc_1_1_invoke(ARM_SMCCC_TRNG_VERSION, &res);
+	if ((s32)res.a0 < 0)
+		return false;
+
+	return res.a0 >= ARM_SMCCC_TRNG_MIN_VERSION;
 }
 
 static inline bool __arm64_rndr(unsigned long *v)
@@ -43,26 +54,52 @@ static inline bool __must_check arch_get_random_int(unsigned int *v)
 
 static inline bool __must_check arch_get_random_seed_long(unsigned long *v)
 {
+	struct arm_smccc_res res;
+
 	/*
 	 * Only support the generic interface after we have detected
 	 * the system wide capability, avoiding complexity with the
 	 * cpufeature code and with potential scheduling between CPUs
 	 * with and without the feature.
 	 */
-	if (!cpus_have_const_cap(ARM64_HAS_RNG))
-		return false;
+	if (cpus_have_const_cap(ARM64_HAS_RNG))
+		return __arm64_rndr(v);
 
-	return __arm64_rndr(v);
-}
+	if (smccc_trng_available) {
+		arm_smccc_1_1_invoke(ARM_SMCCC_TRNG_RND64, 64, &res);
+		if ((int)res.a0 < 0)
+			return false;
 
+		*v = res.a3;
+		return true;
+	}
+
+	return false;
+}
 
 static inline bool __must_check arch_get_random_seed_int(unsigned int *v)
 {
+	struct arm_smccc_res res;
 	unsigned long val;
-	bool ok = arch_get_random_seed_long(&val);
 
-	*v = val;
-	return ok;
+	if (cpus_have_const_cap(ARM64_HAS_RNG)) {
+		if (arch_get_random_seed_long(&val)) {
+			*v = val;
+			return true;
+		}
+		return false;
+	}
+
+	if (smccc_trng_available) {
+		arm_smccc_1_1_invoke(ARM_SMCCC_TRNG_RND64, 32, &res);
+		if ((int)res.a0 < 0)
+			return false;
+
+		*v = res.a3 & GENMASK(31, 0);
+		return true;
+	}
+
+	return false;
 }
 
 static inline bool __init __early_cpu_has_rndr(void)
@@ -77,10 +114,20 @@ arch_get_random_seed_long_early(unsigned long *v)
 {
 	WARN_ON(system_state != SYSTEM_BOOTING);
 
-	if (!__early_cpu_has_rndr())
-		return false;
+	if (__early_cpu_has_rndr())
+		return __arm64_rndr(v);
+
+	if (smccc_trng_available) {
+		struct arm_smccc_res res;
 
-	return __arm64_rndr(v);
+		arm_smccc_1_1_invoke(ARM_SMCCC_TRNG_RND64, 64, &res);
+		if ((int)res.a0 >= 0) {
+			*v = res.a3;
+			return true;
+		}
+	}
+
+	return false;
 }
 #define arch_get_random_seed_long_early arch_get_random_seed_long_early
 
-- 
2.17.1

[PATCH v3 5/5] KVM: arm64: implement the TRNG hypervisor call

[Andre Przywara]

From: Ard Biesheuvel <ardb@kernel.org>

Provide a hypervisor implementation of the ARM architected TRNG firmware
interface described in ARM spec DEN0098. All function IDs are implemented,
including both 32-bit and 64-bit versions of the TRNG_RND service, which
is the centerpiece of the API.

The API is backed by the kernel's entropy pool only, to avoid guests
draining more precious direct entropy sources.

Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
[Andre: minor fixes, drop arch_get_random() usage]
Signed-off-by: Andre Przywara <andre.przywara@arm.com>
---
 arch/arm64/include/asm/kvm_host.h |  2 +
 arch/arm64/kvm/Makefile           |  2 +-
 arch/arm64/kvm/hypercalls.c       |  6 +++
 arch/arm64/kvm/trng.c             | 85 +++++++++++++++++++++++++++++++
 4 files changed, 94 insertions(+), 1 deletion(-)
 create mode 100644 arch/arm64/kvm/trng.c

diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h
index 781d029b8aa8..615932bacf76 100644
--- a/arch/arm64/include/asm/kvm_host.h
+++ b/arch/arm64/include/asm/kvm_host.h
@@ -652,4 +652,6 @@ bool kvm_arm_vcpu_is_finalized(struct kvm_vcpu *vcpu);
 #define kvm_arm_vcpu_sve_finalized(vcpu) \
 	((vcpu)->arch.flags & KVM_ARM64_VCPU_SVE_FINALIZED)
 
+int kvm_trng_call(struct kvm_vcpu *vcpu);
+
 #endif /* __ARM64_KVM_HOST_H__ */
diff --git a/arch/arm64/kvm/Makefile b/arch/arm64/kvm/Makefile
index 1504c81fbf5d..a510037e3270 100644
--- a/arch/arm64/kvm/Makefile
+++ b/arch/arm64/kvm/Makefile
@@ -16,7 +16,7 @@ kvm-y := $(KVM)/kvm_main.o $(KVM)/coalesced_mmio.o $(KVM)/eventfd.o \
 	 inject_fault.o regmap.o va_layout.o handle_exit.o \
 	 guest.o debug.o reset.o sys_regs.o \
 	 vgic-sys-reg-v3.o fpsimd.o pmu.o \
-	 aarch32.o arch_timer.o \
+	 aarch32.o arch_timer.o trng.o \
 	 vgic/vgic.o vgic/vgic-init.o \
 	 vgic/vgic-irqfd.o vgic/vgic-v2.o \
 	 vgic/vgic-v3.o vgic/vgic-v4.o \
diff --git a/arch/arm64/kvm/hypercalls.c b/arch/arm64/kvm/hypercalls.c
index 25ea4ecb6449..ead21b98b620 100644
--- a/arch/arm64/kvm/hypercalls.c
+++ b/arch/arm64/kvm/hypercalls.c
@@ -71,6 +71,12 @@ int kvm_hvc_call_handler(struct kvm_vcpu *vcpu)
 		if (gpa != GPA_INVALID)
 			val = gpa;
 		break;
+	case ARM_SMCCC_TRNG_VERSION:
+	case ARM_SMCCC_TRNG_FEATURES:
+	case ARM_SMCCC_TRNG_GET_UUID:
+	case ARM_SMCCC_TRNG_RND32:
+	case ARM_SMCCC_TRNG_RND64:
+		return kvm_trng_call(vcpu);
 	default:
 		return kvm_psci_call(vcpu);
 	}
diff --git a/arch/arm64/kvm/trng.c b/arch/arm64/kvm/trng.c
new file mode 100644
index 000000000000..99bdd7103c9c
--- /dev/null
+++ b/arch/arm64/kvm/trng.c
@@ -0,0 +1,85 @@
+// SPDX-License-Identifier: GPL-2.0
+// Copyright (C) 2020 Arm Ltd.
+
+#include <linux/arm-smccc.h>
+#include <linux/kvm_host.h>
+
+#include <asm/kvm_emulate.h>
+
+#include <kvm/arm_hypercalls.h>
+
+#define ARM_SMCCC_TRNG_VERSION_1_0	0x10000UL
+
+/* Those values are deliberately separate from the generic SMCCC definitions. */
+#define TRNG_SUCCESS			0UL
+#define TRNG_NOT_SUPPORTED		((unsigned long)-1)
+#define TRNG_INVALID_PARAMETER		((unsigned long)-2)
+#define TRNG_NO_ENTROPY			((unsigned long)-3)
+
+#define TRNG_MAX_BITS64			192
+
+static const uuid_t arm_smc_trng_uuid __aligned(4) = UUID_INIT(
+	0x0d21e000, 0x4384, 0x11eb, 0x80, 0x70, 0x52, 0x44, 0x55, 0x4e, 0x5a, 0x4c);
+
+static int kvm_trng_do_rnd(struct kvm_vcpu *vcpu, int size)
+{
+	DECLARE_BITMAP(bits, TRNG_MAX_BITS64);
+	u32 num_bits = smccc_get_arg1(vcpu);
+	int i;
+
+	if (num_bits > 3 * size) {
+		smccc_set_retval(vcpu, TRNG_INVALID_PARAMETER, 0, 0, 0);
+		return 1;
+	}
+
+	/* get as many bits as we need to fulfil the request */
+	for (i = 0; i < DIV_ROUND_UP(num_bits, BITS_PER_LONG); i++)
+		bits[i] = get_random_long();
+
+	bitmap_clear(bits, num_bits, TRNG_MAX_BITS64 - num_bits);
+
+	if (size == 32)
+		smccc_set_retval(vcpu, TRNG_SUCCESS, lower_32_bits(bits[1]),
+				 upper_32_bits(bits[0]), lower_32_bits(bits[0]));
+	else
+		smccc_set_retval(vcpu, TRNG_SUCCESS, bits[2], bits[1], bits[0]);
+
+	memzero_explicit(bits, sizeof(bits));
+	return 1;
+}
+
+int kvm_trng_call(struct kvm_vcpu *vcpu)
+{
+	const __le32 *u = (__le32 *)arm_smc_trng_uuid.b;
+	u32 func_id = smccc_get_function(vcpu);
+	unsigned long val = TRNG_NOT_SUPPORTED;
+	int size = 64;
+
+	switch (func_id) {
+	case ARM_SMCCC_TRNG_VERSION:
+		val = ARM_SMCCC_TRNG_VERSION_1_0;
+		break;
+	case ARM_SMCCC_TRNG_FEATURES:
+		switch (smccc_get_arg1(vcpu)) {
+		case ARM_SMCCC_TRNG_VERSION:
+		case ARM_SMCCC_TRNG_FEATURES:
+		case ARM_SMCCC_TRNG_GET_UUID:
+		case ARM_SMCCC_TRNG_RND32:
+		case ARM_SMCCC_TRNG_RND64:
+			val = TRNG_SUCCESS;
+		}
+		break;
+	case ARM_SMCCC_TRNG_GET_UUID:
+		smccc_set_retval(vcpu, le32_to_cpu(u[0]), le32_to_cpu(u[1]),
+				 le32_to_cpu(u[2]), le32_to_cpu(u[3]));
+		return 1;
+	case ARM_SMCCC_TRNG_RND32:
+		size = 32;
+		fallthrough;
+	case ARM_SMCCC_TRNG_RND64:
+		return kvm_trng_do_rnd(vcpu, size);
+	}
+
+	smccc_set_retval(vcpu, val, 0, 0, 0);
+	return 1;
+}
-- 
2.17.1

Re: [PATCH v3 0/5] ARM: arm64: Add SMCCC TRNG entropy service

[Ard Biesheuvel]

On Fri, 13 Nov 2020 at 19:24, Andre Przywara <andre.przywara@arm.com> wrote:
>
> Hi,
>
> an update to v2 with some fixes and a few tweaks. Ard's patch [1] should
> significantly reduce the frequency of arch_get_random_seed_long() calls,
> not sure if that is enough the appease the concerns about the
> potentially long latency of SMC calls. I also dropped the direct
> arch_get_random() call in KVM for the same reason. An alternative could
> be to just use the SMC in the _early() versions, but then we would lose
> the SMCCC entropy source for the periodic reseeds. This could be mitigated
> by using a hwrng driver [2] and rngd.
> The only other non-minor change to v2 is the addition of using the SMCCC
> call in the _early() variant. For a changelog see below.
>
> Sudeep: patch 1/5 is a prerequisite for all other patches, which
> themselves could be considered separate and need to go via different trees.
> If we could agree on that one now and get that merged, it would help the
> handling of the other patches going forward.
>
> Cheers,
> Andre
> ==============================
>
> The ARM architected TRNG firmware interface, described in ARM spec
> DEN0098[3], defines an ARM SMCCC based interface to a true random number
> generator, provided by firmware.
>
> This series collects all the patches implementing this in various
> places: as a user feeding into the ARCH_RANDOM pool, both for ARM and
> arm64, and as a service provider for KVM guests.
>
> Patch 1 introduces the interface definition used by all three entities.
> Patch 2 prepares the Arm SMCCC firmware driver to probe for the
> interface. This patch is needed to avoid a later dependency on *two*
> patches (there might be a better solution to this problem).
>
> Patch 3 implements the ARM part, patch 4 is the arm64 version.
> The final patch 5 adds support to provide random numbers to KVM guests.
>
> This was tested on:
> - QEMU -kernel (no SMCCC, regression test)
> - Juno w/ prototype of the h/w Trusted RNG support
> - mainline KVM (SMCCC, but no TRNG: regression test)
> - ARM and arm64 KVM guests, using the KVM service in patch 5/5
>
> Based on v5.10-rc3, please let me know if I should rebased on something
> else. A git repo is accessible at:
> https://gitlab.arm.com/linux-arm/linux-ap/-/commits/smccc-trng/v3/
>
> Cheers,
> Andre
>
> [1] http://lists.infradead.org/pipermail/linux-arm-kernel/2020-November/615446.html
> [2] https://gitlab.arm.com/linux-arm/linux-ap/-/commit/87e3722f437
> [3] https://developer.arm.com/documentation/den0098/latest/
>
> Changelog v2 ... v3:
> - ARM: fix compilation with randconfig
> - arm64: use SMCCC call also in arch_get_random_seed_long_early()
> - KVM: comment on return value usage
> - KVM: use more interesting UUID (enjoy, Marc!)

UUIDs are constructed using certain rules, so probably better to
refrain from playing games with them here.

If Marc wants an easter egg, he will have to wait until Easter.

> - KVM: use bitmaps instead of open coded long arrays
> - KVM: drop direct usage of arch_get_random() interface
>
> Changelog "v1" ... v2:
> - trigger ARCH_RANDOM initialisation from the SMCCC firmware driver
> - use a single bool in smccc.c to hold the initialisation state for arm64
> - handle endianess correctly in the KVM provider
>
> Andre Przywara (2):
>   firmware: smccc: Introduce SMCCC TRNG framework
>   arm64: Add support for SMCCC TRNG entropy source
>
> Ard Biesheuvel (3):
>   firmware: smccc: Add SMCCC TRNG function call IDs
>   ARM: implement support for SMCCC TRNG entropy source
>   KVM: arm64: implement the TRNG hypervisor call
>
>  arch/arm/Kconfig                    |  4 ++
>  arch/arm/include/asm/archrandom.h   | 74 +++++++++++++++++++++++++
>  arch/arm64/include/asm/archrandom.h | 79 +++++++++++++++++++++++----
>  arch/arm64/include/asm/kvm_host.h   |  2 +
>  arch/arm64/kvm/Makefile             |  2 +-
>  arch/arm64/kvm/hypercalls.c         |  6 ++
>  arch/arm64/kvm/trng.c               | 85 +++++++++++++++++++++++++++++
>  drivers/firmware/smccc/smccc.c      |  5 ++
>  include/linux/arm-smccc.h           | 31 +++++++++++
>  9 files changed, 277 insertions(+), 11 deletions(-)
>  create mode 100644 arch/arm/include/asm/archrandom.h
>  create mode 100644 arch/arm64/kvm/trng.c
>
> --
> 2.17.1
>

Re: [PATCH v3 0/5] ARM: arm64: Add SMCCC TRNG entropy service

[André Przywara]

On 13/11/2020 23:05, Ard Biesheuvel wrote:
> On Fri, 13 Nov 2020 at 19:24, Andre Przywara <andre.przywara@arm.com> wrote:
>>
>> Hi,
>>
>> an update to v2 with some fixes and a few tweaks. Ard's patch [1] should
>> significantly reduce the frequency of arch_get_random_seed_long() calls,
>> not sure if that is enough the appease the concerns about the
>> potentially long latency of SMC calls. I also dropped the direct
>> arch_get_random() call in KVM for the same reason. An alternative could
>> be to just use the SMC in the _early() versions, but then we would lose
>> the SMCCC entropy source for the periodic reseeds. This could be mitigated
>> by using a hwrng driver [2] and rngd.
>> The only other non-minor change to v2 is the addition of using the SMCCC
>> call in the _early() variant. For a changelog see below.
>>
>> Sudeep: patch 1/5 is a prerequisite for all other patches, which
>> themselves could be considered separate and need to go via different trees.
>> If we could agree on that one now and get that merged, it would help the
>> handling of the other patches going forward.
>>
>> Cheers,
>> Andre
>> ==============================
>>
>> The ARM architected TRNG firmware interface, described in ARM spec
>> DEN0098[3], defines an ARM SMCCC based interface to a true random number
>> generator, provided by firmware.
>>
>> This series collects all the patches implementing this in various
>> places: as a user feeding into the ARCH_RANDOM pool, both for ARM and
>> arm64, and as a service provider for KVM guests.
>>
>> Patch 1 introduces the interface definition used by all three entities.
>> Patch 2 prepares the Arm SMCCC firmware driver to probe for the
>> interface. This patch is needed to avoid a later dependency on *two*
>> patches (there might be a better solution to this problem).
>>
>> Patch 3 implements the ARM part, patch 4 is the arm64 version.
>> The final patch 5 adds support to provide random numbers to KVM guests.
>>
>> This was tested on:
>> - QEMU -kernel (no SMCCC, regression test)
>> - Juno w/ prototype of the h/w Trusted RNG support
>> - mainline KVM (SMCCC, but no TRNG: regression test)
>> - ARM and arm64 KVM guests, using the KVM service in patch 5/5
>>
>> Based on v5.10-rc3, please let me know if I should rebased on something
>> else. A git repo is accessible at:
>> https://gitlab.arm.com/linux-arm/linux-ap/-/commits/smccc-trng/v3/
>>
>> Cheers,
>> Andre
>>
>> [1] http://lists.infradead.org/pipermail/linux-arm-kernel/2020-November/615446.html
>> [2] https://gitlab.arm.com/linux-arm/linux-ap/-/commit/87e3722f437
>> [3] https://developer.arm.com/documentation/den0098/latest/
>>
>> Changelog v2 ... v3:
>> - ARM: fix compilation with randconfig
>> - arm64: use SMCCC call also in arch_get_random_seed_long_early()
>> - KVM: comment on return value usage
>> - KVM: use more interesting UUID (enjoy, Marc!)
> 
> UUIDs are constructed using certain rules, so probably better to
> refrain from playing games with them here.

Hey, it's a valid variant 1 version 1 UUID (otherwise this would be no
real easter egg). uuidgen -t should be able to generate this one. I
found it too easy to use the random variant, but we can revert to that.

> If Marc wants an easter egg, he will have to wait until Easter.

Don't mess with your maintainer ;-)

Cheers,
Andre

> 
>> - KVM: use bitmaps instead of open coded long arrays
>> - KVM: drop direct usage of arch_get_random() interface
>>
>> Changelog "v1" ... v2:
>> - trigger ARCH_RANDOM initialisation from the SMCCC firmware driver
>> - use a single bool in smccc.c to hold the initialisation state for arm64
>> - handle endianess correctly in the KVM provider
>>
>> Andre Przywara (2):
>>   firmware: smccc: Introduce SMCCC TRNG framework
>>   arm64: Add support for SMCCC TRNG entropy source
>>
>> Ard Biesheuvel (3):
>>   firmware: smccc: Add SMCCC TRNG function call IDs
>>   ARM: implement support for SMCCC TRNG entropy source
>>   KVM: arm64: implement the TRNG hypervisor call
>>
>>  arch/arm/Kconfig                    |  4 ++
>>  arch/arm/include/asm/archrandom.h   | 74 +++++++++++++++++++++++++
>>  arch/arm64/include/asm/archrandom.h | 79 +++++++++++++++++++++++----
>>  arch/arm64/include/asm/kvm_host.h   |  2 +
>>  arch/arm64/kvm/Makefile             |  2 +-
>>  arch/arm64/kvm/hypercalls.c         |  6 ++
>>  arch/arm64/kvm/trng.c               | 85 +++++++++++++++++++++++++++++
>>  drivers/firmware/smccc/smccc.c      |  5 ++
>>  include/linux/arm-smccc.h           | 31 +++++++++++
>>  9 files changed, 277 insertions(+), 11 deletions(-)
>>  create mode 100644 arch/arm/include/asm/archrandom.h
>>  create mode 100644 arch/arm64/kvm/trng.c
>>
>> --
>> 2.17.1
>>

Re: [PATCH v3 1/5] firmware: smccc: Add SMCCC TRNG function call IDs

[Linus Walleij]

On Fri, Nov 13, 2020 at 7:24 PM Andre Przywara <andre.przywara@arm.com> wrote:

> From: Ard Biesheuvel <ardb@kernel.org>
>
> The ARM architected TRNG firmware interface, described in ARM spec
> DEN0098, define an ARM SMCCC based interface to a true random number
> generator, provided by firmware.
>
> Add the definitions of the SMCCC functions as defined by the spec.
>
> Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
> Signed-off-by: Andre Przywara <andre.przywara@arm.com>

Reviewed-by: Linus Walleij <linus.walleij@linaro.org>

Yours,
Linus Walleij

Re: [PATCH v3 2/5] firmware: smccc: Introduce SMCCC TRNG framework

[Linus Walleij]

On Fri, Nov 13, 2020 at 7:24 PM Andre Przywara <andre.przywara@arm.com> wrote:

> The ARM DEN0098 document describe an SMCCC based firmware service to
> deliver hardware generated random numbers. Its existence is advertised
> according to the SMCCC v1.1 specification.
>
> Add a (dummy) call to probe functions implemented in each architecture
> (ARM and arm64), to determine the existence of this interface.
> For now this return false, but this will be overwritten by each
> architecture's support patch.
>
> Signed-off-by: Andre Przywara <andre.przywara@arm.com>

Reviewed-by: Linus Walleij <linus.walleij@linaro.org>

Yours,
Linus Walleij

Re: [PATCH v3 3/5] ARM: implement support for SMCCC TRNG entropy source

[Linus Walleij]

On Fri, Nov 13, 2020 at 7:24 PM Andre Przywara <andre.przywara@arm.com> wrote:

> From: Ard Biesheuvel <ardb@kernel.org>
>
> Implement arch_get_random_seed_*() for ARM based on the firmware
> or hypervisor provided entropy source described in ARM DEN0098.
>
> This will make the kernel's random number generator consume entropy
> provided by this interface, at early boot, and periodically at
> runtime when reseeding.
>
> Cc: Linus Walleij <linus.walleij@linaro.org>
> Cc: Russell King <linux@armlinux.org.uk>
> Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
> [Andre: rework to be initialised by the SMCCC firmware driver]
> Signed-off-by: Andre Przywara <andre.przywara@arm.com>

Excellent idea,
Reviewed-by: Linus Walleij <linus.walleij@linaro.org>

Yours,
Linus Walleij

Re: [PATCH v3 4/5] arm64: Add support for SMCCC TRNG entropy source

[Ard Biesheuvel]

On Fri, 13 Nov 2020 at 19:24, Andre Przywara <andre.przywara@arm.com> wrote:
>
> The ARM architected TRNG firmware interface, described in ARM spec
> DEN0098, defines an ARM SMCCC based interface to a true random number
> generator, provided by firmware.
> This can be discovered via the SMCCC >=v1.1 interface, and provides
> up to 192 bits of entropy per call.
>
> Hook this SMC call into arm64's arch_get_random_*() implementation,
> coming to the rescue when the CPU does not implement the ARM v8.5 RNG
> system registers.
>
> For the detection, we piggy back on the PSCI/SMCCC discovery (which gives
> us the conduit to use (hvc/smc)), then try to call the
> ARM_SMCCC_TRNG_VERSION function, which returns -1 if this interface is
> not implemented.
>
> Signed-off-by: Andre Przywara <andre.przywara@arm.com>
> ---
>  arch/arm64/include/asm/archrandom.h | 69 ++++++++++++++++++++++++-----
>  1 file changed, 58 insertions(+), 11 deletions(-)
>
> diff --git a/arch/arm64/include/asm/archrandom.h b/arch/arm64/include/asm/archrandom.h
> index abe07c21da8e..fe34bfd30caa 100644
> --- a/arch/arm64/include/asm/archrandom.h
> +++ b/arch/arm64/include/asm/archrandom.h
> @@ -4,13 +4,24 @@
>
>  #ifdef CONFIG_ARCH_RANDOM
>
> +#include <linux/arm-smccc.h>
>  #include <linux/bug.h>
>  #include <linux/kernel.h>
>  #include <asm/cpufeature.h>
>
> +#define ARM_SMCCC_TRNG_MIN_VERSION     0x10000UL
> +
> +extern bool smccc_trng_available;
> +
>  static inline bool __init smccc_probe_trng(void)
>  {
> -       return false;
> +       struct arm_smccc_res res;
> +
> +       arm_smccc_1_1_invoke(ARM_SMCCC_TRNG_VERSION, &res);
> +       if ((s32)res.a0 < 0)
> +               return false;
> +
> +       return res.a0 >= ARM_SMCCC_TRNG_MIN_VERSION;
>  }
>
>  static inline bool __arm64_rndr(unsigned long *v)
> @@ -43,26 +54,52 @@ static inline bool __must_check arch_get_random_int(unsigned int *v)
>
>  static inline bool __must_check arch_get_random_seed_long(unsigned long *v)
>  {
> +       struct arm_smccc_res res;
> +
>         /*
>          * Only support the generic interface after we have detected
>          * the system wide capability, avoiding complexity with the
>          * cpufeature code and with potential scheduling between CPUs
>          * with and without the feature.
>          */
> -       if (!cpus_have_const_cap(ARM64_HAS_RNG))
> -               return false;
> +       if (cpus_have_const_cap(ARM64_HAS_RNG))
> +               return __arm64_rndr(v);
>
> -       return __arm64_rndr(v);
> -}
> +       if (smccc_trng_available) {
> +               arm_smccc_1_1_invoke(ARM_SMCCC_TRNG_RND64, 64, &res);
> +               if ((int)res.a0 < 0)
> +                       return false;
>
> +               *v = res.a3;
> +               return true;
> +       }
> +
> +       return false;
> +}
>

I think we should be more rigorous here in how we map the concepts of
random seeds and random numbers onto the various sources.

First of all, assuming my patch dropping the call to
arch_get_random_seed_long() from add_interrupt_randomness() gets
accepted, we should switch to RNDRRS here, and implement the non-seed
variants using RNDR.

However, this is still semantically inaccurate: RNDRRS does not return
a random *seed*, it returns a number drawn from a freshly seeded
pseudo-random sequence. This means that the TRNG interface, if
implemented, is a better choice, and so we should try it first. Note
that on platforms that don't implement both, only one of these will be
available in the first place. But on platforms that *do* implement
both, the firmware interface may actually be less wasteful in terms of
resources: the TRNG interface returns every bit drawn from the
underlying entropy source, whereas RNDRRS uses ~500 bits of entropy to
reseed a DRBG that gets used only once to draw a single 64-bit number.
And the cost of the SMCCC call in terms of CPU time is charged to the
caller, which is appropriate here.

Then, I don't think we should ever return false without even trying if
RNDRRS is available if the SMCCC invocation fails.

Something like this perhaps?

if (smccc_trng_available) {
  arm_smccc_1_1_invoke(ARM_SMCCC_TRNG_RND64, 64, &res);
  if ((int)res.a0 >= 0) {
    *v = res.a3;
    return true;
  }
}

if (cpus_have_const_cap(ARM64_HAS_RNG))
   return __arm64_rndrrs(v);

return false;

(and something similar 2x below)


>  static inline bool __must_check arch_get_random_seed_int(unsigned int *v)
>  {
> +       struct arm_smccc_res res;
>         unsigned long val;
> -       bool ok = arch_get_random_seed_long(&val);
>
> -       *v = val;
> -       return ok;
> +       if (cpus_have_const_cap(ARM64_HAS_RNG)) {
> +               if (arch_get_random_seed_long(&val)) {
> +                       *v = val;
> +                       return true;
> +               }
> +               return false;
> +       }
> +
> +       if (smccc_trng_available) {
> +               arm_smccc_1_1_invoke(ARM_SMCCC_TRNG_RND64, 32, &res);
> +               if ((int)res.a0 < 0)
> +                       return false;
> +
> +               *v = res.a3 & GENMASK(31, 0);
> +               return true;
> +       }
> +
> +       return false;
>  }
>
>  static inline bool __init __early_cpu_has_rndr(void)
> @@ -77,10 +114,20 @@ arch_get_random_seed_long_early(unsigned long *v)
>  {
>         WARN_ON(system_state != SYSTEM_BOOTING);
>
> -       if (!__early_cpu_has_rndr())
> -               return false;
> +       if (__early_cpu_has_rndr())
> +               return __arm64_rndr(v);
> +
> +       if (smccc_trng_available) {
> +               struct arm_smccc_res res;
>
> -       return __arm64_rndr(v);
> +               arm_smccc_1_1_invoke(ARM_SMCCC_TRNG_RND64, 64, &res);
> +               if ((int)res.a0 >= 0) {
> +                       *v = res.a3;
> +                       return true;
> +               }
> +       }
> +
> +       return false;
>  }
>  #define arch_get_random_seed_long_early arch_get_random_seed_long_early
>
> --
> 2.17.1
>

Re: [PATCH v3 4/5] arm64: Add support for SMCCC TRNG entropy source

[André Przywara]

On 19/11/2020 13:41, Ard Biesheuvel wrote:

Hi,

> On Fri, 13 Nov 2020 at 19:24, Andre Przywara <andre.przywara@arm.com> wrote:
>>
>> The ARM architected TRNG firmware interface, described in ARM spec
>> DEN0098, defines an ARM SMCCC based interface to a true random number
>> generator, provided by firmware.
>> This can be discovered via the SMCCC >=v1.1 interface, and provides
>> up to 192 bits of entropy per call.
>>
>> Hook this SMC call into arm64's arch_get_random_*() implementation,
>> coming to the rescue when the CPU does not implement the ARM v8.5 RNG
>> system registers.
>>
>> For the detection, we piggy back on the PSCI/SMCCC discovery (which gives
>> us the conduit to use (hvc/smc)), then try to call the
>> ARM_SMCCC_TRNG_VERSION function, which returns -1 if this interface is
>> not implemented.
>>
>> Signed-off-by: Andre Przywara <andre.przywara@arm.com>
>> ---
>>  arch/arm64/include/asm/archrandom.h | 69 ++++++++++++++++++++++++-----
>>  1 file changed, 58 insertions(+), 11 deletions(-)
>>
>> diff --git a/arch/arm64/include/asm/archrandom.h b/arch/arm64/include/asm/archrandom.h
>> index abe07c21da8e..fe34bfd30caa 100644
>> --- a/arch/arm64/include/asm/archrandom.h
>> +++ b/arch/arm64/include/asm/archrandom.h
>> @@ -4,13 +4,24 @@
>>
>>  #ifdef CONFIG_ARCH_RANDOM
>>
>> +#include <linux/arm-smccc.h>
>>  #include <linux/bug.h>
>>  #include <linux/kernel.h>
>>  #include <asm/cpufeature.h>
>>
>> +#define ARM_SMCCC_TRNG_MIN_VERSION     0x10000UL
>> +
>> +extern bool smccc_trng_available;
>> +
>>  static inline bool __init smccc_probe_trng(void)
>>  {
>> -       return false;
>> +       struct arm_smccc_res res;
>> +
>> +       arm_smccc_1_1_invoke(ARM_SMCCC_TRNG_VERSION, &res);
>> +       if ((s32)res.a0 < 0)
>> +               return false;
>> +
>> +       return res.a0 >= ARM_SMCCC_TRNG_MIN_VERSION;
>>  }
>>
>>  static inline bool __arm64_rndr(unsigned long *v)
>> @@ -43,26 +54,52 @@ static inline bool __must_check arch_get_random_int(unsigned int *v)
>>
>>  static inline bool __must_check arch_get_random_seed_long(unsigned long *v)
>>  {
>> +       struct arm_smccc_res res;
>> +
>>         /*
>>          * Only support the generic interface after we have detected
>>          * the system wide capability, avoiding complexity with the
>>          * cpufeature code and with potential scheduling between CPUs
>>          * with and without the feature.
>>          */
>> -       if (!cpus_have_const_cap(ARM64_HAS_RNG))
>> -               return false;
>> +       if (cpus_have_const_cap(ARM64_HAS_RNG))
>> +               return __arm64_rndr(v);
>>
>> -       return __arm64_rndr(v);
>> -}
>> +       if (smccc_trng_available) {
>> +               arm_smccc_1_1_invoke(ARM_SMCCC_TRNG_RND64, 64, &res);
>> +               if ((int)res.a0 < 0)
>> +                       return false;
>>
>> +               *v = res.a3;
>> +               return true;
>> +       }
>> +
>> +       return false;
>> +}
>>
> 
> I think we should be more rigorous here in how we map the concepts of
> random seeds and random numbers onto the various sources.
> 
> First of all, assuming my patch dropping the call to
> arch_get_random_seed_long() from add_interrupt_randomness() gets
> accepted, we should switch to RNDRRS here, and implement the non-seed
> variants using RNDR.

I agree (and have a patch ready), but that seems independent from this
series.

> However, this is still semantically inaccurate: RNDRRS does not return
> a random *seed*, it returns a number drawn from a freshly seeded
> pseudo-random sequence. This means that the TRNG interface, if
> implemented, is a better choice, and so we should try it first. Note
> that on platforms that don't implement both, only one of these will be
> available in the first place. But on platforms that *do* implement
> both, the firmware interface may actually be less wasteful in terms of
> resources: the TRNG interface returns every bit drawn from the
> underlying entropy source, whereas RNDRRS uses ~500 bits of entropy to
> reseed a DRBG that gets used only once to draw a single 64-bit number.

I am not sure I share your enthusiasm about the quality of the actual
TRNG firmware implementations, but we can go with this for now.
Maybe if we see bad implementations in the future, we can revisit this,
and have some tuneables? Or a command line option to ignore the SMCCC
interface? Or use the UUID mechanism for that?

> And the cost of the SMCCC call in terms of CPU time is charged to the
> caller, which is appropriate here.

This still leaves the problem that the core might be stuck in EL3 for an
unknown period of time, impeding our realtime efforts.
Do we have some ball park number of a number of cycles spent in EL3
still being acceptable? That could serve as a guideline for firmware
implementations?

> Then, I don't think we should ever return false without even trying if
> RNDRRS is available if the SMCCC invocation fails.

That's a good point.

> Something like this perhaps?
> 
> if (smccc_trng_available) {
>   arm_smccc_1_1_invoke(ARM_SMCCC_TRNG_RND64, 64, &res);
>   if ((int)res.a0 >= 0) {
>     *v = res.a3;
>     return true;
>   }
> }

yeah, the fall-through here is a good idea.

> 
> if (cpus_have_const_cap(ARM64_HAS_RNG))
>    return __arm64_rndrrs(v);
> 
> return false;

So I wonder if we have a trade-off here between the performance of the
RNDRRS entropy source and the latency of the firmware implementation.
If the RNDR entropy source delivers Mbits/s (the Juno h/w definitely
does), we might just not care about throwing away some (or even the
majority) of it.

On the other hand the Juno TRNG hardware for instance spends already
hundreds of cycles on a single 32-bit MMIO read alone, just to transfer
the bits into the CPU. Having a rather large pool could avoid paying
this price on every SMC call, but I don't see a nice way of allowing
TF-A to fill this pool, when Linux thinks it can spare the time.

So I am a bit uneasy about unconditionally preferring the SMCCC
implementation over the RNDRRS instruction.

Cheers,
Andre

> (and something similar 2x below)
> 
> 
>>  static inline bool __must_check arch_get_random_seed_int(unsigned int *v)
>>  {
>> +       struct arm_smccc_res res;
>>         unsigned long val;
>> -       bool ok = arch_get_random_seed_long(&val);
>>
>> -       *v = val;
>> -       return ok;
>> +       if (cpus_have_const_cap(ARM64_HAS_RNG)) {
>> +               if (arch_get_random_seed_long(&val)) {
>> +                       *v = val;
>> +                       return true;
>> +               }
>> +               return false;
>> +       }
>> +
>> +       if (smccc_trng_available) {
>> +               arm_smccc_1_1_invoke(ARM_SMCCC_TRNG_RND64, 32, &res);
>> +               if ((int)res.a0 < 0)
>> +                       return false;
>> +
>> +               *v = res.a3 & GENMASK(31, 0);
>> +               return true;
>> +       }
>> +
>> +       return false;
>>  }
>>
>>  static inline bool __init __early_cpu_has_rndr(void)
>> @@ -77,10 +114,20 @@ arch_get_random_seed_long_early(unsigned long *v)
>>  {
>>         WARN_ON(system_state != SYSTEM_BOOTING);
>>
>> -       if (!__early_cpu_has_rndr())
>> -               return false;
>> +       if (__early_cpu_has_rndr())
>> +               return __arm64_rndr(v);
>> +
>> +       if (smccc_trng_available) {
>> +               struct arm_smccc_res res;
>>
>> -       return __arm64_rndr(v);
>> +               arm_smccc_1_1_invoke(ARM_SMCCC_TRNG_RND64, 64, &res);
>> +               if ((int)res.a0 >= 0) {
>> +                       *v = res.a3;
>> +                       return true;
>> +               }
>> +       }
>> +
>> +       return false;
>>  }
>>  #define arch_get_random_seed_long_early arch_get_random_seed_long_early
>>
>> --
>> 2.17.1
>>

Re: [PATCH v3 4/5] arm64: Add support for SMCCC TRNG entropy source

[Ard Biesheuvel]

On Fri, 20 Nov 2020 at 11:52, André Przywara <andre.przywara@arm.com> wrote:
>
> On 19/11/2020 13:41, Ard Biesheuvel wrote:
>
> Hi,
>
> > On Fri, 13 Nov 2020 at 19:24, Andre Przywara <andre.przywara@arm.com> wrote:
> >>
> >> The ARM architected TRNG firmware interface, described in ARM spec
> >> DEN0098, defines an ARM SMCCC based interface to a true random number
> >> generator, provided by firmware.
> >> This can be discovered via the SMCCC >=v1.1 interface, and provides
> >> up to 192 bits of entropy per call.
> >>
> >> Hook this SMC call into arm64's arch_get_random_*() implementation,
> >> coming to the rescue when the CPU does not implement the ARM v8.5 RNG
> >> system registers.
> >>
> >> For the detection, we piggy back on the PSCI/SMCCC discovery (which gives
> >> us the conduit to use (hvc/smc)), then try to call the
> >> ARM_SMCCC_TRNG_VERSION function, which returns -1 if this interface is
> >> not implemented.
> >>
> >> Signed-off-by: Andre Przywara <andre.przywara@arm.com>
> >> ---
> >>  arch/arm64/include/asm/archrandom.h | 69 ++++++++++++++++++++++++-----
> >>  1 file changed, 58 insertions(+), 11 deletions(-)
> >>
> >> diff --git a/arch/arm64/include/asm/archrandom.h b/arch/arm64/include/asm/archrandom.h
> >> index abe07c21da8e..fe34bfd30caa 100644
> >> --- a/arch/arm64/include/asm/archrandom.h
> >> +++ b/arch/arm64/include/asm/archrandom.h
> >> @@ -4,13 +4,24 @@
> >>
> >>  #ifdef CONFIG_ARCH_RANDOM
> >>
> >> +#include <linux/arm-smccc.h>
> >>  #include <linux/bug.h>
> >>  #include <linux/kernel.h>
> >>  #include <asm/cpufeature.h>
> >>
> >> +#define ARM_SMCCC_TRNG_MIN_VERSION     0x10000UL
> >> +
> >> +extern bool smccc_trng_available;
> >> +
> >>  static inline bool __init smccc_probe_trng(void)
> >>  {
> >> -       return false;
> >> +       struct arm_smccc_res res;
> >> +
> >> +       arm_smccc_1_1_invoke(ARM_SMCCC_TRNG_VERSION, &res);
> >> +       if ((s32)res.a0 < 0)
> >> +               return false;
> >> +
> >> +       return res.a0 >= ARM_SMCCC_TRNG_MIN_VERSION;
> >>  }
> >>
> >>  static inline bool __arm64_rndr(unsigned long *v)
> >> @@ -43,26 +54,52 @@ static inline bool __must_check arch_get_random_int(unsigned int *v)
> >>
> >>  static inline bool __must_check arch_get_random_seed_long(unsigned long *v)
> >>  {
> >> +       struct arm_smccc_res res;
> >> +
> >>         /*
> >>          * Only support the generic interface after we have detected
> >>          * the system wide capability, avoiding complexity with the
> >>          * cpufeature code and with potential scheduling between CPUs
> >>          * with and without the feature.
> >>          */
> >> -       if (!cpus_have_const_cap(ARM64_HAS_RNG))
> >> -               return false;
> >> +       if (cpus_have_const_cap(ARM64_HAS_RNG))
> >> +               return __arm64_rndr(v);
> >>
> >> -       return __arm64_rndr(v);
> >> -}
> >> +       if (smccc_trng_available) {
> >> +               arm_smccc_1_1_invoke(ARM_SMCCC_TRNG_RND64, 64, &res);
> >> +               if ((int)res.a0 < 0)
> >> +                       return false;
> >>
> >> +               *v = res.a3;
> >> +               return true;
> >> +       }
> >> +
> >> +       return false;
> >> +}
> >>
> >
> > I think we should be more rigorous here in how we map the concepts of
> > random seeds and random numbers onto the various sources.
> >
> > First of all, assuming my patch dropping the call to
> > arch_get_random_seed_long() from add_interrupt_randomness() gets
> > accepted, we should switch to RNDRRS here, and implement the non-seed
> > variants using RNDR.
>
> I agree (and have a patch ready), but that seems independent from this
> series.
>

Well, it will conflict, but other than that, I agree it is orthogonal.

> > However, this is still semantically inaccurate: RNDRRS does not return
> > a random *seed*, it returns a number drawn from a freshly seeded
> > pseudo-random sequence. This means that the TRNG interface, if
> > implemented, is a better choice, and so we should try it first. Note
> > that on platforms that don't implement both, only one of these will be
> > available in the first place. But on platforms that *do* implement
> > both, the firmware interface may actually be less wasteful in terms of
> > resources: the TRNG interface returns every bit drawn from the
> > underlying entropy source, whereas RNDRRS uses ~500 bits of entropy to
> > reseed a DRBG that gets used only once to draw a single 64-bit number.
>
> I am not sure I share your enthusiasm about the quality of the actual
> TRNG firmware implementations, but we can go with this for now.
> Maybe if we see bad implementations in the future, we can revisit this,
> and have some tuneables? Or a command line option to ignore the SMCCC
> interface? Or use the UUID mechanism for that?
>

This is not about good or bad implementations. It is about mapping the
OS's request for a seed onto a privileged entropy source if one is
available, instead of onto a non-privileged instruction which is
documented as producing the output of a pseudo random number
generator, not of an entropy source. And again, it remains to be seen
whether any systems will turn up that implement both, but if they do,
it likely means that the SMC call provides access to the entropy
source that is also used to seed the RNDRRS DRBG, and in this case,
the SMC call is more appropriate.

> > And the cost of the SMCCC call in terms of CPU time is charged to the
> > caller, which is appropriate here.
>
> This still leaves the problem that the core might be stuck in EL3 for an
> unknown period of time, impeding our realtime efforts.

I agree that there may be quality of implementation issues here, but
the call is documented as non-blocking, i.e., it either returns the
result immediately, or it signals failure.

> Do we have some ball park number of a number of cycles spent in EL3
> still being acceptable? That could serve as a guideline for firmware
> implementations?
>

Realtime is typically about having a bounded worst case, and so I
think the non-blocking requirement is what is most important here. How
many cycles or nanoseconds are spent exactly is of a lesser concern
IMO.

> > Then, I don't think we should ever return false without even trying if
> > RNDRRS is available if the SMCCC invocation fails.
>
> That's a good point.
>
> > Something like this perhaps?
> >
> > if (smccc_trng_available) {
> >   arm_smccc_1_1_invoke(ARM_SMCCC_TRNG_RND64, 64, &res);
> >   if ((int)res.a0 >= 0) {
> >     *v = res.a3;
> >     return true;
> >   }
> > }
>
> yeah, the fall-through here is a good idea.
>
> >
> > if (cpus_have_const_cap(ARM64_HAS_RNG))
> >    return __arm64_rndrrs(v);
> >
> > return false;
>
> So I wonder if we have a trade-off here between the performance of the
> RNDRRS entropy source and the latency of the firmware implementation.
> If the RNDR entropy source delivers Mbits/s (the Juno h/w definitely
> does), we might just not care about throwing away some (or even the
> majority) of it.
>

The important distinction here is that you can only waste your own CPU
cycles, but you can potentially waste a shared resource by hitting on
RNDRRS (and the architecture does not give any guidance whatsoever on
the permitted failure rate of RNDRRS)

But perhaps we should save this discussion for another time, when we
have actual hardware to test this on. In the meantime, I am fine with
the current precedence of RNDR[RS] over the SMC call.

> On the other hand the Juno TRNG hardware for instance spends already
> hundreds of cycles on a single 32-bit MMIO read alone, just to transfer
> the bits into the CPU. Having a rather large pool could avoid paying
> this price on every SMC call, but I don't see a nice way of allowing
> TF-A to fill this pool, when Linux thinks it can spare the time.
>
> So I am a bit uneasy about unconditionally preferring the SMCCC
> implementation over the RNDRRS instruction.
>
> Cheers,
> Andre
>
> > (and something similar 2x below)
> >
> >
> >>  static inline bool __must_check arch_get_random_seed_int(unsigned int *v)
> >>  {
> >> +       struct arm_smccc_res res;
> >>         unsigned long val;
> >> -       bool ok = arch_get_random_seed_long(&val);
> >>
> >> -       *v = val;
> >> -       return ok;
> >> +       if (cpus_have_const_cap(ARM64_HAS_RNG)) {
> >> +               if (arch_get_random_seed_long(&val)) {
> >> +                       *v = val;
> >> +                       return true;
> >> +               }
> >> +               return false;
> >> +       }
> >> +
> >> +       if (smccc_trng_available) {
> >> +               arm_smccc_1_1_invoke(ARM_SMCCC_TRNG_RND64, 32, &res);
> >> +               if ((int)res.a0 < 0)
> >> +                       return false;
> >> +
> >> +               *v = res.a3 & GENMASK(31, 0);
> >> +               return true;
> >> +       }
> >> +
> >> +       return false;
> >>  }
> >>
> >>  static inline bool __init __early_cpu_has_rndr(void)
> >> @@ -77,10 +114,20 @@ arch_get_random_seed_long_early(unsigned long *v)
> >>  {
> >>         WARN_ON(system_state != SYSTEM_BOOTING);
> >>
> >> -       if (!__early_cpu_has_rndr())
> >> -               return false;
> >> +       if (__early_cpu_has_rndr())
> >> +               return __arm64_rndr(v);
> >> +
> >> +       if (smccc_trng_available) {
> >> +               struct arm_smccc_res res;
> >>
> >> -       return __arm64_rndr(v);
> >> +               arm_smccc_1_1_invoke(ARM_SMCCC_TRNG_RND64, 64, &res);
> >> +               if ((int)res.a0 >= 0) {
> >> +                       *v = res.a3;
> >> +                       return true;
> >> +               }
> >> +       }
> >> +
> >> +       return false;
> >>  }
> >>  #define arch_get_random_seed_long_early arch_get_random_seed_long_early
> >>
> >> --
> >> 2.17.1
> >>
>