Linux-ARM-Kernel Archive on lore.kernel.org
 help / color / Atom feed
* [PATCH v10 0/3] ARMv8.5-RNG support
@ 2020-01-10 12:23 Mark Brown
  2020-01-10 12:23 ` [PATCH v10 1/3] arm64: Implement archrandom.h for ARMv8.5-RNG Mark Brown
                   ` (2 more replies)
  0 siblings, 3 replies; 30+ messages in thread
From: Mark Brown @ 2020-01-10 12:23 UTC (permalink / raw)
  To: Will Deacon, Catalin Marinas
  Cc: Mark Rutland, Mark Brown, Richard Henderson, linux-arm-kernel,
	Ard Biesheuvel

This series is based on Richard Henderson's previous v7, it addresses
review comments from that version by splitting the boot and runtime
interaction with the core random code so that they are completely
separate and adds a new change that uses the v8.5-RNG extension to seed
KASLR when ARCH_RANDOM is enabled.

v10:
 - Spell out that we're adding data not entropy from setup_arch() in
   the commit message for patch 2.
v9:
 - Make another static inline helper for early feature checks.
 - Add init annotations.
 - Use xor to add RNDR output to seed for KASLR.

Mark Brown (2):
  arm64: random: Add data to pool from setup_arch()
  arm64: Use v8.5-RNG entropy for KASLR seed

Richard Henderson (1):
  arm64: Implement archrandom.h for ARMv8.5-RNG

 Documentation/arm64/cpu-feature-registers.rst |  2 +
 arch/arm64/Kconfig                            | 12 +++
 arch/arm64/include/asm/archrandom.h           | 97 +++++++++++++++++++
 arch/arm64/include/asm/cpucaps.h              |  3 +-
 arch/arm64/include/asm/sysreg.h               |  4 +
 arch/arm64/kernel/cpufeature.c                | 13 +++
 arch/arm64/kernel/kaslr.c                     | 11 +++
 arch/arm64/kernel/setup.c                     |  2 +
 8 files changed, 143 insertions(+), 1 deletion(-)
 create mode 100644 arch/arm64/include/asm/archrandom.h

-- 
2.20.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 30+ messages in thread

* [PATCH v10 1/3] arm64: Implement archrandom.h for ARMv8.5-RNG
  2020-01-10 12:23 [PATCH v10 0/3] ARMv8.5-RNG support Mark Brown
@ 2020-01-10 12:23 ` Mark Brown
  2020-01-14 17:44   ` Will Deacon
  2020-01-10 12:23 ` [PATCH v10 2/3] arm64: random: Add data to pool from setup_arch() Mark Brown
  2020-01-10 12:23 ` [PATCH v10 3/3] arm64: Use v8.5-RNG entropy for KASLR seed Mark Brown
  2 siblings, 1 reply; 30+ messages in thread
From: Mark Brown @ 2020-01-10 12:23 UTC (permalink / raw)
  To: Will Deacon, Catalin Marinas
  Cc: Mark Rutland, Mark Brown, Richard Henderson, linux-arm-kernel,
	Ard Biesheuvel

From: Richard Henderson <richard.henderson@linaro.org>

Expose the ID_AA64ISAR0.RNDR field to userspace, as the RNG system
registers are always available at EL0.

Implement arch_get_random_seed_long using RNDR.  Given that the
TRNG is likely to be a shared resource between cores, and VMs,
do not explicitly force re-seeding with RNDRRS.  In order to avoid
code complexity and potential issues with hetrogenous systems only
provide values after cpufeature has finalized the system capabilities.

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
[Modified to only function after cpufeature has finalized the system
capabilities and move all the code into the header -- broonie]
Signed-off-by: Mark Brown <broonie@kernel.org>
Reviewed-by: Mark Rutland <mark.rutland@arm.com>
---
 Documentation/arm64/cpu-feature-registers.rst |  2 +
 arch/arm64/Kconfig                            | 12 ++++
 arch/arm64/include/asm/archrandom.h           | 67 +++++++++++++++++++
 arch/arm64/include/asm/cpucaps.h              |  3 +-
 arch/arm64/include/asm/sysreg.h               |  4 ++
 arch/arm64/kernel/cpufeature.c                | 13 ++++
 6 files changed, 100 insertions(+), 1 deletion(-)
 create mode 100644 arch/arm64/include/asm/archrandom.h

diff --git a/Documentation/arm64/cpu-feature-registers.rst b/Documentation/arm64/cpu-feature-registers.rst
index b6e44884e3ad..ce320785fb0c 100644
--- a/Documentation/arm64/cpu-feature-registers.rst
+++ b/Documentation/arm64/cpu-feature-registers.rst
@@ -117,6 +117,8 @@ infrastructure:
      +------------------------------+---------+---------+
      | Name                         |  bits   | visible |
      +------------------------------+---------+---------+
+     | RNDR                         | [63-60] |    y    |
+     +------------------------------+---------+---------+
      | TS                           | [55-52] |    y    |
      +------------------------------+---------+---------+
      | FHM                          | [51-48] |    y    |
diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
index b1b4476ddb83..835f8158220e 100644
--- a/arch/arm64/Kconfig
+++ b/arch/arm64/Kconfig
@@ -1484,6 +1484,18 @@ config ARM64_PTR_AUTH
 
 endmenu
 
+menu "ARMv8.5 architectural features"
+
+config ARCH_RANDOM
+	bool "Enable support for random number generation"
+	default y
+	help
+	  Random number generation (part of the ARMv8.5 Extensions)
+	  provides a high bandwidth, cryptographically secure
+	  hardware random number generator.
+
+endmenu
+
 config ARM64_SVE
 	bool "ARM Scalable Vector Extension support"
 	default y
diff --git a/arch/arm64/include/asm/archrandom.h b/arch/arm64/include/asm/archrandom.h
new file mode 100644
index 000000000000..5ea5a1ce5a5f
--- /dev/null
+++ b/arch/arm64/include/asm/archrandom.h
@@ -0,0 +1,67 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef _ASM_ARCHRANDOM_H
+#define _ASM_ARCHRANDOM_H
+
+#ifdef CONFIG_ARCH_RANDOM
+
+#include <linux/random.h>
+#include <asm/cpufeature.h>
+
+static inline bool __arm64_rndr(unsigned long *v)
+{
+	bool ok;
+
+	/*
+	 * Reads of RNDR set PSTATE.NZCV to 0b0000 on success,
+	 * and set PSTATE.NZCV to 0b0100 otherwise.
+	 */
+	asm volatile(
+		__mrs_s("%0", SYS_RNDR_EL0) "\n"
+	"	cset %w1, ne\n"
+	: "=r" (*v), "=r" (ok)
+	:
+	: "cc");
+
+	return ok;
+}
+
+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)
+{
+	/*
+	 * 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;
+
+	return __arm64_rndr(v);
+}
+
+
+static inline bool __must_check arch_get_random_seed_int(unsigned int *v)
+{
+	unsigned long val;
+	bool ok = arch_get_random_seed_long(&val);
+
+	*v = val;
+	return ok;
+}
+
+#else
+
+static inline bool __arm64_rndr(unsigned long *v) { return false; }
+
+#endif /* CONFIG_ARCH_RANDOM */
+#endif /* _ASM_ARCHRANDOM_H */
diff --git a/arch/arm64/include/asm/cpucaps.h b/arch/arm64/include/asm/cpucaps.h
index b92683871119..515f4fbcbf91 100644
--- a/arch/arm64/include/asm/cpucaps.h
+++ b/arch/arm64/include/asm/cpucaps.h
@@ -56,7 +56,8 @@
 #define ARM64_WORKAROUND_CAVIUM_TX2_219_PRFM	46
 #define ARM64_WORKAROUND_1542419		47
 #define ARM64_WORKAROUND_1319367		48
+#define ARM64_HAS_RNG				49
 
-#define ARM64_NCAPS				49
+#define ARM64_NCAPS				50
 
 #endif /* __ASM_CPUCAPS_H */
diff --git a/arch/arm64/include/asm/sysreg.h b/arch/arm64/include/asm/sysreg.h
index 6e919fafb43d..5e718f279469 100644
--- a/arch/arm64/include/asm/sysreg.h
+++ b/arch/arm64/include/asm/sysreg.h
@@ -365,6 +365,9 @@
 #define SYS_CTR_EL0			sys_reg(3, 3, 0, 0, 1)
 #define SYS_DCZID_EL0			sys_reg(3, 3, 0, 0, 7)
 
+#define SYS_RNDR_EL0			sys_reg(3, 3, 2, 4, 0)
+#define SYS_RNDRRS_EL0			sys_reg(3, 3, 2, 4, 1)
+
 #define SYS_PMCR_EL0			sys_reg(3, 3, 9, 12, 0)
 #define SYS_PMCNTENSET_EL0		sys_reg(3, 3, 9, 12, 1)
 #define SYS_PMCNTENCLR_EL0		sys_reg(3, 3, 9, 12, 2)
@@ -539,6 +542,7 @@
 			 ENDIAN_SET_EL1 | SCTLR_EL1_UCI  | SCTLR_EL1_RES1)
 
 /* id_aa64isar0 */
+#define ID_AA64ISAR0_RNDR_SHIFT		60
 #define ID_AA64ISAR0_TS_SHIFT		52
 #define ID_AA64ISAR0_FHM_SHIFT		48
 #define ID_AA64ISAR0_DP_SHIFT		44
diff --git a/arch/arm64/kernel/cpufeature.c b/arch/arm64/kernel/cpufeature.c
index 04cf64e9f0c9..0fea85228956 100644
--- a/arch/arm64/kernel/cpufeature.c
+++ b/arch/arm64/kernel/cpufeature.c
@@ -119,6 +119,7 @@ static void cpu_enable_cnp(struct arm64_cpu_capabilities const *cap);
  * sync with the documentation of the CPU feature register ABI.
  */
 static const struct arm64_ftr_bits ftr_id_aa64isar0[] = {
+	ARM64_FTR_BITS(FTR_VISIBLE, FTR_STRICT, FTR_LOWER_SAFE, ID_AA64ISAR0_RNDR_SHIFT, 4, 0),
 	ARM64_FTR_BITS(FTR_VISIBLE, FTR_STRICT, FTR_LOWER_SAFE, ID_AA64ISAR0_TS_SHIFT, 4, 0),
 	ARM64_FTR_BITS(FTR_VISIBLE, FTR_STRICT, FTR_LOWER_SAFE, ID_AA64ISAR0_FHM_SHIFT, 4, 0),
 	ARM64_FTR_BITS(FTR_VISIBLE, FTR_STRICT, FTR_LOWER_SAFE, ID_AA64ISAR0_DP_SHIFT, 4, 0),
@@ -1566,6 +1567,18 @@ static const struct arm64_cpu_capabilities arm64_features[] = {
 		.sign = FTR_UNSIGNED,
 		.min_field_value = 1,
 	},
+#endif
+#ifdef CONFIG_ARCH_RANDOM
+	{
+		.desc = "Random Number Generator",
+		.capability = ARM64_HAS_RNG,
+		.type = ARM64_CPUCAP_SYSTEM_FEATURE,
+		.matches = has_cpuid_feature,
+		.sys_reg = SYS_ID_AA64ISAR0_EL1,
+		.field_pos = ID_AA64ISAR0_RNDR_SHIFT,
+		.sign = FTR_UNSIGNED,
+		.min_field_value = 1,
+	},
 #endif
 	{},
 };
-- 
2.20.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 30+ messages in thread

* [PATCH v10 2/3] arm64: random: Add data to pool from setup_arch()
  2020-01-10 12:23 [PATCH v10 0/3] ARMv8.5-RNG support Mark Brown
  2020-01-10 12:23 ` [PATCH v10 1/3] arm64: Implement archrandom.h for ARMv8.5-RNG Mark Brown
@ 2020-01-10 12:23 ` Mark Brown
  2020-01-10 12:35   ` Mark Rutland
                     ` (2 more replies)
  2020-01-10 12:23 ` [PATCH v10 3/3] arm64: Use v8.5-RNG entropy for KASLR seed Mark Brown
  2 siblings, 3 replies; 30+ messages in thread
From: Mark Brown @ 2020-01-10 12:23 UTC (permalink / raw)
  To: Will Deacon, Catalin Marinas
  Cc: Mark Rutland, Mark Brown, Richard Henderson, linux-arm-kernel,
	Ard Biesheuvel

Since the arm64 ARCH_RANDOM implementation is not available until
cpufeature has determined the system capabilities it can't be used by
the generic random code to initialize the entropy pool for early use.
Instead explicitly add some data to the pool from setup_arch() if the
boot CPU supports v8.5-RNG, this is the point recommended by the generic
code.

Note that we are only adding data here, it will be mixed into the pool
but won't be credited as entropy. There are currently no suitable
interfaces for that at present - extending the random code to provide
those will be done as a future step. Providing data is better than not
doing so as it will still provide an increase in variation in the output
from the random code and there will be no impact on the rate at which
entropy is credited compared to what we have without this patch.

Signed-off-by: Mark Brown <broonie@kernel.org>
---
 arch/arm64/include/asm/archrandom.h | 30 +++++++++++++++++++++++++++++
 arch/arm64/kernel/setup.c           |  2 ++
 2 files changed, 32 insertions(+)

diff --git a/arch/arm64/include/asm/archrandom.h b/arch/arm64/include/asm/archrandom.h
index 5ea5a1ce5a5f..2eb1db1f0bdf 100644
--- a/arch/arm64/include/asm/archrandom.h
+++ b/arch/arm64/include/asm/archrandom.h
@@ -59,9 +59,39 @@ static inline bool __must_check arch_get_random_seed_int(unsigned int *v)
 	return ok;
 }
 
+static inline bool __init __early_cpu_has_rndr(void)
+{
+	/* Open code as we run prior to the first call to cpufeature. */
+	unsigned long ftr = read_sysreg_s(SYS_ID_AA64ISAR0_EL1);
+	return (ftr >> ID_AA64ISAR0_RNDR_SHIFT) & 0xf;
+}
+
+/*
+ * Our ARCH_RANDOM implementation does not function until relatively
+ * late in the boot when cpufeature has detertmined system
+ * capabilities so the core code can't use arch_get_random*() to
+ * initialize, instead we call this function to inject data from
+ * setup_arch() if the boot CPU supports v8.5-RNG.
+ */
+static inline void __init arm64_add_early_rndr_entropy(void)
+{
+	unsigned long val;
+	int i;
+
+	if (!__early_cpu_has_rndr())
+		return;
+
+	/* Add multiple values to mirror the generic code. */
+	for (i = 0; i < 16; i++)
+		if (__arm64_rndr(&val))
+			add_device_randomness(&val, sizeof(val));
+}
+
 #else
 
 static inline bool __arm64_rndr(unsigned long *v) { return false; }
+static inline bool __init __early_cpu_has_rndr(void) { return false; }
+static inline void __init arm64_add_early_rndr_entropy(void) { }
 
 #endif /* CONFIG_ARCH_RANDOM */
 #endif /* _ASM_ARCHRANDOM_H */
diff --git a/arch/arm64/kernel/setup.c b/arch/arm64/kernel/setup.c
index 56f664561754..170842965a32 100644
--- a/arch/arm64/kernel/setup.c
+++ b/arch/arm64/kernel/setup.c
@@ -344,6 +344,8 @@ void __init setup_arch(char **cmdline_p)
 	/* Init percpu seeds for random tags after cpus are set up. */
 	kasan_init_tags();
 
+	arm64_add_early_rndr_entropy();
+
 #ifdef CONFIG_ARM64_SW_TTBR0_PAN
 	/*
 	 * Make sure init_thread_info.ttbr0 always generates translation
-- 
2.20.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 30+ messages in thread

* [PATCH v10 3/3] arm64: Use v8.5-RNG entropy for KASLR seed
  2020-01-10 12:23 [PATCH v10 0/3] ARMv8.5-RNG support Mark Brown
  2020-01-10 12:23 ` [PATCH v10 1/3] arm64: Implement archrandom.h for ARMv8.5-RNG Mark Brown
  2020-01-10 12:23 ` [PATCH v10 2/3] arm64: random: Add data to pool from setup_arch() Mark Brown
@ 2020-01-10 12:23 ` Mark Brown
  2020-01-10 12:35   ` Mark Rutland
  2020-01-13 19:09   ` Richard Henderson
  2 siblings, 2 replies; 30+ messages in thread
From: Mark Brown @ 2020-01-10 12:23 UTC (permalink / raw)
  To: Will Deacon, Catalin Marinas
  Cc: Mark Rutland, Mark Brown, Richard Henderson, linux-arm-kernel,
	Ard Biesheuvel

When seeding KALSR on a system where we have architecture level random
number generation make use of that entropy, mixing it in with the seed
passed by the bootloader. Since this is run very early in init before
feature detection is complete we open code rather than use archrandom.h.

Signed-off-by: Mark Brown <broonie@kernel.org>
---
 arch/arm64/kernel/kaslr.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/arch/arm64/kernel/kaslr.c b/arch/arm64/kernel/kaslr.c
index 2a11a962e571..53b8a4ee64ff 100644
--- a/arch/arm64/kernel/kaslr.c
+++ b/arch/arm64/kernel/kaslr.c
@@ -120,6 +120,17 @@ u64 __init kaslr_early_init(u64 dt_phys)
 		return 0;
 	}
 
+	/*
+	 * Mix in any entropy obtainable architecturally, open coded
+	 * since this runs extremely early.
+	 */
+	if (__early_cpu_has_rndr()) {
+		unsigned long raw;
+
+		if (__arm64_rndr(&raw))
+			seed ^= raw;
+	}
+
 	if (!seed) {
 		kaslr_status = KASLR_DISABLED_NO_SEED;
 		return 0;
-- 
2.20.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH v10 2/3] arm64: random: Add data to pool from setup_arch()
  2020-01-10 12:23 ` [PATCH v10 2/3] arm64: random: Add data to pool from setup_arch() Mark Brown
@ 2020-01-10 12:35   ` Mark Rutland
  2020-01-13 19:09   ` Richard Henderson
  2020-01-15  7:48   ` Ard Biesheuvel
  2 siblings, 0 replies; 30+ messages in thread
From: Mark Rutland @ 2020-01-10 12:35 UTC (permalink / raw)
  To: Mark Brown
  Cc: Catalin Marinas, Richard Henderson, Will Deacon,
	linux-arm-kernel, Ard Biesheuvel

On Fri, Jan 10, 2020 at 12:23:40PM +0000, Mark Brown wrote:
> Since the arm64 ARCH_RANDOM implementation is not available until
> cpufeature has determined the system capabilities it can't be used by
> the generic random code to initialize the entropy pool for early use.
> Instead explicitly add some data to the pool from setup_arch() if the
> boot CPU supports v8.5-RNG, this is the point recommended by the generic
> code.
> 
> Note that we are only adding data here, it will be mixed into the pool
> but won't be credited as entropy. There are currently no suitable
> interfaces for that at present - extending the random code to provide
> those will be done as a future step. Providing data is better than not
> doing so as it will still provide an increase in variation in the output
> from the random code and there will be no impact on the rate at which
> entropy is credited compared to what we have without this patch.
> 
> Signed-off-by: Mark Brown <broonie@kernel.org>

This is certainly better than the current state of things so:

Reviewed-by: Mark Rutland <mark.rutland@arm.com>

Thanks,
Mark.

> ---
>  arch/arm64/include/asm/archrandom.h | 30 +++++++++++++++++++++++++++++
>  arch/arm64/kernel/setup.c           |  2 ++
>  2 files changed, 32 insertions(+)
> 
> diff --git a/arch/arm64/include/asm/archrandom.h b/arch/arm64/include/asm/archrandom.h
> index 5ea5a1ce5a5f..2eb1db1f0bdf 100644
> --- a/arch/arm64/include/asm/archrandom.h
> +++ b/arch/arm64/include/asm/archrandom.h
> @@ -59,9 +59,39 @@ static inline bool __must_check arch_get_random_seed_int(unsigned int *v)
>  	return ok;
>  }
>  
> +static inline bool __init __early_cpu_has_rndr(void)
> +{
> +	/* Open code as we run prior to the first call to cpufeature. */
> +	unsigned long ftr = read_sysreg_s(SYS_ID_AA64ISAR0_EL1);
> +	return (ftr >> ID_AA64ISAR0_RNDR_SHIFT) & 0xf;
> +}
> +
> +/*
> + * Our ARCH_RANDOM implementation does not function until relatively
> + * late in the boot when cpufeature has detertmined system
> + * capabilities so the core code can't use arch_get_random*() to
> + * initialize, instead we call this function to inject data from
> + * setup_arch() if the boot CPU supports v8.5-RNG.
> + */
> +static inline void __init arm64_add_early_rndr_entropy(void)
> +{
> +	unsigned long val;
> +	int i;
> +
> +	if (!__early_cpu_has_rndr())
> +		return;
> +
> +	/* Add multiple values to mirror the generic code. */
> +	for (i = 0; i < 16; i++)
> +		if (__arm64_rndr(&val))
> +			add_device_randomness(&val, sizeof(val));
> +}
> +
>  #else
>  
>  static inline bool __arm64_rndr(unsigned long *v) { return false; }
> +static inline bool __init __early_cpu_has_rndr(void) { return false; }
> +static inline void __init arm64_add_early_rndr_entropy(void) { }
>  
>  #endif /* CONFIG_ARCH_RANDOM */
>  #endif /* _ASM_ARCHRANDOM_H */
> diff --git a/arch/arm64/kernel/setup.c b/arch/arm64/kernel/setup.c
> index 56f664561754..170842965a32 100644
> --- a/arch/arm64/kernel/setup.c
> +++ b/arch/arm64/kernel/setup.c
> @@ -344,6 +344,8 @@ void __init setup_arch(char **cmdline_p)
>  	/* Init percpu seeds for random tags after cpus are set up. */
>  	kasan_init_tags();
>  
> +	arm64_add_early_rndr_entropy();
> +
>  #ifdef CONFIG_ARM64_SW_TTBR0_PAN
>  	/*
>  	 * Make sure init_thread_info.ttbr0 always generates translation
> -- 
> 2.20.1
> 

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH v10 3/3] arm64: Use v8.5-RNG entropy for KASLR seed
  2020-01-10 12:23 ` [PATCH v10 3/3] arm64: Use v8.5-RNG entropy for KASLR seed Mark Brown
@ 2020-01-10 12:35   ` Mark Rutland
  2020-01-13 19:09   ` Richard Henderson
  1 sibling, 0 replies; 30+ messages in thread
From: Mark Rutland @ 2020-01-10 12:35 UTC (permalink / raw)
  To: Mark Brown
  Cc: Catalin Marinas, Richard Henderson, Will Deacon,
	linux-arm-kernel, Ard Biesheuvel

On Fri, Jan 10, 2020 at 12:23:41PM +0000, Mark Brown wrote:
> When seeding KALSR on a system where we have architecture level random
> number generation make use of that entropy, mixing it in with the seed
> passed by the bootloader. Since this is run very early in init before
> feature detection is complete we open code rather than use archrandom.h.
> 
> Signed-off-by: Mark Brown <broonie@kernel.org>

Reviewed-by: Mark Rutland <mark.rutland@arm.com>

Mark.

> ---
>  arch/arm64/kernel/kaslr.c | 11 +++++++++++
>  1 file changed, 11 insertions(+)
> 
> diff --git a/arch/arm64/kernel/kaslr.c b/arch/arm64/kernel/kaslr.c
> index 2a11a962e571..53b8a4ee64ff 100644
> --- a/arch/arm64/kernel/kaslr.c
> +++ b/arch/arm64/kernel/kaslr.c
> @@ -120,6 +120,17 @@ u64 __init kaslr_early_init(u64 dt_phys)
>  		return 0;
>  	}
>  
> +	/*
> +	 * Mix in any entropy obtainable architecturally, open coded
> +	 * since this runs extremely early.
> +	 */
> +	if (__early_cpu_has_rndr()) {
> +		unsigned long raw;
> +
> +		if (__arm64_rndr(&raw))
> +			seed ^= raw;
> +	}
> +
>  	if (!seed) {
>  		kaslr_status = KASLR_DISABLED_NO_SEED;
>  		return 0;
> -- 
> 2.20.1
> 

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH v10 2/3] arm64: random: Add data to pool from setup_arch()
  2020-01-10 12:23 ` [PATCH v10 2/3] arm64: random: Add data to pool from setup_arch() Mark Brown
  2020-01-10 12:35   ` Mark Rutland
@ 2020-01-13 19:09   ` Richard Henderson
  2020-01-15  7:48   ` Ard Biesheuvel
  2 siblings, 0 replies; 30+ messages in thread
From: Richard Henderson @ 2020-01-13 19:09 UTC (permalink / raw)
  To: Mark Brown, Will Deacon, Catalin Marinas
  Cc: Mark Rutland, linux-arm-kernel, Ard Biesheuvel

On 1/10/20 2:23 AM, Mark Brown wrote:
> Since the arm64 ARCH_RANDOM implementation is not available until
> cpufeature has determined the system capabilities it can't be used by
> the generic random code to initialize the entropy pool for early use.
> Instead explicitly add some data to the pool from setup_arch() if the
> boot CPU supports v8.5-RNG, this is the point recommended by the generic
> code.
> 
> Note that we are only adding data here, it will be mixed into the pool
> but won't be credited as entropy. There are currently no suitable
> interfaces for that at present - extending the random code to provide
> those will be done as a future step. Providing data is better than not
> doing so as it will still provide an increase in variation in the output
> from the random code and there will be no impact on the rate at which
> entropy is credited compared to what we have without this patch.
> 
> Signed-off-by: Mark Brown <broonie@kernel.org>
> ---
>  arch/arm64/include/asm/archrandom.h | 30 +++++++++++++++++++++++++++++
>  arch/arm64/kernel/setup.c           |  2 ++
>  2 files changed, 32 insertions(+)
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>


r~

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH v10 3/3] arm64: Use v8.5-RNG entropy for KASLR seed
  2020-01-10 12:23 ` [PATCH v10 3/3] arm64: Use v8.5-RNG entropy for KASLR seed Mark Brown
  2020-01-10 12:35   ` Mark Rutland
@ 2020-01-13 19:09   ` Richard Henderson
  1 sibling, 0 replies; 30+ messages in thread
From: Richard Henderson @ 2020-01-13 19:09 UTC (permalink / raw)
  To: Mark Brown, Will Deacon, Catalin Marinas
  Cc: Mark Rutland, linux-arm-kernel, Ard Biesheuvel

On 1/10/20 2:23 AM, Mark Brown wrote:
> When seeding KALSR on a system where we have architecture level random
> number generation make use of that entropy, mixing it in with the seed
> passed by the bootloader. Since this is run very early in init before
> feature detection is complete we open code rather than use archrandom.h.
> 
> Signed-off-by: Mark Brown <broonie@kernel.org>
> ---
>  arch/arm64/kernel/kaslr.c | 11 +++++++++++
>  1 file changed, 11 insertions(+)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>


r~

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH v10 1/3] arm64: Implement archrandom.h for ARMv8.5-RNG
  2020-01-10 12:23 ` [PATCH v10 1/3] arm64: Implement archrandom.h for ARMv8.5-RNG Mark Brown
@ 2020-01-14 17:44   ` Will Deacon
  2020-01-15  7:40     ` Ard Biesheuvel
  0 siblings, 1 reply; 30+ messages in thread
From: Will Deacon @ 2020-01-14 17:44 UTC (permalink / raw)
  To: Mark Brown
  Cc: Mark Rutland, Catalin Marinas, Richard Henderson,
	linux-arm-kernel, Ard Biesheuvel

On Fri, Jan 10, 2020 at 12:23:39PM +0000, Mark Brown wrote:
> From: Richard Henderson <richard.henderson@linaro.org>
> 
> Expose the ID_AA64ISAR0.RNDR field to userspace, as the RNG system
> registers are always available at EL0.
> 
> Implement arch_get_random_seed_long using RNDR.  Given that the
> TRNG is likely to be a shared resource between cores, and VMs,
> do not explicitly force re-seeding with RNDRRS.  In order to avoid
> code complexity and potential issues with hetrogenous systems only
> provide values after cpufeature has finalized the system capabilities.
> 
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> [Modified to only function after cpufeature has finalized the system
> capabilities and move all the code into the header -- broonie]
> Signed-off-by: Mark Brown <broonie@kernel.org>
> Reviewed-by: Mark Rutland <mark.rutland@arm.com>
> ---
>  Documentation/arm64/cpu-feature-registers.rst |  2 +
>  arch/arm64/Kconfig                            | 12 ++++
>  arch/arm64/include/asm/archrandom.h           | 67 +++++++++++++++++++
>  arch/arm64/include/asm/cpucaps.h              |  3 +-
>  arch/arm64/include/asm/sysreg.h               |  4 ++
>  arch/arm64/kernel/cpufeature.c                | 13 ++++
>  6 files changed, 100 insertions(+), 1 deletion(-)
>  create mode 100644 arch/arm64/include/asm/archrandom.h

In which case, should we also add an HWCAP for this?

Will

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH v10 1/3] arm64: Implement archrandom.h for ARMv8.5-RNG
  2020-01-14 17:44   ` Will Deacon
@ 2020-01-15  7:40     ` Ard Biesheuvel
  2020-01-15  9:16       ` Will Deacon
  0 siblings, 1 reply; 30+ messages in thread
From: Ard Biesheuvel @ 2020-01-15  7:40 UTC (permalink / raw)
  To: Will Deacon
  Cc: Mark Rutland, Catalin Marinas, Mark Brown, Richard Henderson,
	linux-arm-kernel

On Tue, 14 Jan 2020 at 18:44, Will Deacon <will@kernel.org> wrote:
>
> On Fri, Jan 10, 2020 at 12:23:39PM +0000, Mark Brown wrote:
> > From: Richard Henderson <richard.henderson@linaro.org>
> >
> > Expose the ID_AA64ISAR0.RNDR field to userspace, as the RNG system
> > registers are always available at EL0.
> >
> > Implement arch_get_random_seed_long using RNDR.  Given that the
> > TRNG is likely to be a shared resource between cores, and VMs,
> > do not explicitly force re-seeding with RNDRRS.  In order to avoid
> > code complexity and potential issues with hetrogenous systems only
> > provide values after cpufeature has finalized the system capabilities.
> >
> > Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> > [Modified to only function after cpufeature has finalized the system
> > capabilities and move all the code into the header -- broonie]
> > Signed-off-by: Mark Brown <broonie@kernel.org>
> > Reviewed-by: Mark Rutland <mark.rutland@arm.com>
> > ---
> >  Documentation/arm64/cpu-feature-registers.rst |  2 +
> >  arch/arm64/Kconfig                            | 12 ++++
> >  arch/arm64/include/asm/archrandom.h           | 67 +++++++++++++++++++
> >  arch/arm64/include/asm/cpucaps.h              |  3 +-
> >  arch/arm64/include/asm/sysreg.h               |  4 ++
> >  arch/arm64/kernel/cpufeature.c                | 13 ++++
> >  6 files changed, 100 insertions(+), 1 deletion(-)
> >  create mode 100644 arch/arm64/include/asm/archrandom.h
>
> In which case, should we also add an HWCAP for this?
>

Isn't this covered by the 'cpuid' HWCAP? We can't prevent EL0 from
accessing these system registers anyway, even if we wanted to.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH v10 2/3] arm64: random: Add data to pool from setup_arch()
  2020-01-10 12:23 ` [PATCH v10 2/3] arm64: random: Add data to pool from setup_arch() Mark Brown
  2020-01-10 12:35   ` Mark Rutland
  2020-01-13 19:09   ` Richard Henderson
@ 2020-01-15  7:48   ` Ard Biesheuvel
  2020-01-15  9:16     ` Will Deacon
  2 siblings, 1 reply; 30+ messages in thread
From: Ard Biesheuvel @ 2020-01-15  7:48 UTC (permalink / raw)
  To: Mark Brown
  Cc: Mark Rutland, Catalin Marinas, Richard Henderson, Will Deacon,
	linux-arm-kernel

On Fri, 10 Jan 2020 at 13:23, Mark Brown <broonie@kernel.org> wrote:
>
> Since the arm64 ARCH_RANDOM implementation is not available until
> cpufeature has determined the system capabilities it can't be used by
> the generic random code to initialize the entropy pool for early use.
> Instead explicitly add some data to the pool from setup_arch() if the
> boot CPU supports v8.5-RNG, this is the point recommended by the generic
> code.
>
> Note that we are only adding data here, it will be mixed into the pool
> but won't be credited as entropy. There are currently no suitable
> interfaces for that at present - extending the random code to provide
> those will be done as a future step. Providing data is better than not
> doing so as it will still provide an increase in variation in the output
> from the random code and there will be no impact on the rate at which
> entropy is credited compared to what we have without this patch.
>

This is slightly unfortunate, as this way, we lose the ability to use
random.trust_cpu=1 to get the entropy credited and initialize CRNG
early.

> Signed-off-by: Mark Brown <broonie@kernel.org>
> ---
>  arch/arm64/include/asm/archrandom.h | 30 +++++++++++++++++++++++++++++
>  arch/arm64/kernel/setup.c           |  2 ++
>  2 files changed, 32 insertions(+)
>
> diff --git a/arch/arm64/include/asm/archrandom.h b/arch/arm64/include/asm/archrandom.h
> index 5ea5a1ce5a5f..2eb1db1f0bdf 100644
> --- a/arch/arm64/include/asm/archrandom.h
> +++ b/arch/arm64/include/asm/archrandom.h
> @@ -59,9 +59,39 @@ static inline bool __must_check arch_get_random_seed_int(unsigned int *v)
>         return ok;
>  }
>
> +static inline bool __init __early_cpu_has_rndr(void)
> +{
> +       /* Open code as we run prior to the first call to cpufeature. */
> +       unsigned long ftr = read_sysreg_s(SYS_ID_AA64ISAR0_EL1);
> +       return (ftr >> ID_AA64ISAR0_RNDR_SHIFT) & 0xf;
> +}
> +
> +/*
> + * Our ARCH_RANDOM implementation does not function until relatively
> + * late in the boot when cpufeature has detertmined system

determined

> + * capabilities so the core code can't use arch_get_random*() to
> + * initialize, instead we call this function to inject data from
> + * setup_arch() if the boot CPU supports v8.5-RNG.
> + */
> +static inline void __init arm64_add_early_rndr_entropy(void)
> +{
> +       unsigned long val;
> +       int i;
> +
> +       if (!__early_cpu_has_rndr())
> +               return;
> +
> +       /* Add multiple values to mirror the generic code. */
> +       for (i = 0; i < 16; i++)
> +               if (__arm64_rndr(&val))
> +                       add_device_randomness(&val, sizeof(val));
> +}
> +
>  #else
>
>  static inline bool __arm64_rndr(unsigned long *v) { return false; }
> +static inline bool __init __early_cpu_has_rndr(void) { return false; }
> +static inline void __init arm64_add_early_rndr_entropy(void) { }
>
>  #endif /* CONFIG_ARCH_RANDOM */
>  #endif /* _ASM_ARCHRANDOM_H */
> diff --git a/arch/arm64/kernel/setup.c b/arch/arm64/kernel/setup.c
> index 56f664561754..170842965a32 100644
> --- a/arch/arm64/kernel/setup.c
> +++ b/arch/arm64/kernel/setup.c
> @@ -344,6 +344,8 @@ void __init setup_arch(char **cmdline_p)
>         /* Init percpu seeds for random tags after cpus are set up. */
>         kasan_init_tags();
>
> +       arm64_add_early_rndr_entropy();
> +
>  #ifdef CONFIG_ARM64_SW_TTBR0_PAN
>         /*
>          * Make sure init_thread_info.ttbr0 always generates translation
> --
> 2.20.1
>

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH v10 2/3] arm64: random: Add data to pool from setup_arch()
  2020-01-15  7:48   ` Ard Biesheuvel
@ 2020-01-15  9:16     ` Will Deacon
  2020-01-15  9:22       ` Ard Biesheuvel
  2020-01-15 12:07       ` Mark Brown
  0 siblings, 2 replies; 30+ messages in thread
From: Will Deacon @ 2020-01-15  9:16 UTC (permalink / raw)
  To: Ard Biesheuvel
  Cc: Mark Rutland, Catalin Marinas, Mark Brown, Richard Henderson,
	linux-arm-kernel

On Wed, Jan 15, 2020 at 08:48:46AM +0100, Ard Biesheuvel wrote:
> On Fri, 10 Jan 2020 at 13:23, Mark Brown <broonie@kernel.org> wrote:
> >
> > Since the arm64 ARCH_RANDOM implementation is not available until
> > cpufeature has determined the system capabilities it can't be used by
> > the generic random code to initialize the entropy pool for early use.
> > Instead explicitly add some data to the pool from setup_arch() if the
> > boot CPU supports v8.5-RNG, this is the point recommended by the generic
> > code.
> >
> > Note that we are only adding data here, it will be mixed into the pool
> > but won't be credited as entropy. There are currently no suitable
> > interfaces for that at present - extending the random code to provide
> > those will be done as a future step. Providing data is better than not
> > doing so as it will still provide an increase in variation in the output
> > from the random code and there will be no impact on the rate at which
> > entropy is credited compared to what we have without this patch.
> >
> 
> This is slightly unfortunate, as this way, we lose the ability to use
> random.trust_cpu=1 to get the entropy credited and initialize CRNG
> early.

Agreed. Do you think we should wait for that support before merging the
series? Given that I don't know of any CPUs implementing this extension,
we can probably afford not to rush this in.

Will

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH v10 1/3] arm64: Implement archrandom.h for ARMv8.5-RNG
  2020-01-15  7:40     ` Ard Biesheuvel
@ 2020-01-15  9:16       ` Will Deacon
  2020-01-15  9:24         ` Ard Biesheuvel
  0 siblings, 1 reply; 30+ messages in thread
From: Will Deacon @ 2020-01-15  9:16 UTC (permalink / raw)
  To: Ard Biesheuvel
  Cc: Mark Rutland, Catalin Marinas, Mark Brown, Richard Henderson,
	linux-arm-kernel

On Wed, Jan 15, 2020 at 08:40:46AM +0100, Ard Biesheuvel wrote:
> On Tue, 14 Jan 2020 at 18:44, Will Deacon <will@kernel.org> wrote:
> >
> > On Fri, Jan 10, 2020 at 12:23:39PM +0000, Mark Brown wrote:
> > > From: Richard Henderson <richard.henderson@linaro.org>
> > >
> > > Expose the ID_AA64ISAR0.RNDR field to userspace, as the RNG system
> > > registers are always available at EL0.
> > >
> > > Implement arch_get_random_seed_long using RNDR.  Given that the
> > > TRNG is likely to be a shared resource between cores, and VMs,
> > > do not explicitly force re-seeding with RNDRRS.  In order to avoid
> > > code complexity and potential issues with hetrogenous systems only
> > > provide values after cpufeature has finalized the system capabilities.
> > >
> > > Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> > > [Modified to only function after cpufeature has finalized the system
> > > capabilities and move all the code into the header -- broonie]
> > > Signed-off-by: Mark Brown <broonie@kernel.org>
> > > Reviewed-by: Mark Rutland <mark.rutland@arm.com>
> > > ---
> > >  Documentation/arm64/cpu-feature-registers.rst |  2 +
> > >  arch/arm64/Kconfig                            | 12 ++++
> > >  arch/arm64/include/asm/archrandom.h           | 67 +++++++++++++++++++
> > >  arch/arm64/include/asm/cpucaps.h              |  3 +-
> > >  arch/arm64/include/asm/sysreg.h               |  4 ++
> > >  arch/arm64/kernel/cpufeature.c                | 13 ++++
> > >  6 files changed, 100 insertions(+), 1 deletion(-)
> > >  create mode 100644 arch/arm64/include/asm/archrandom.h
> >
> > In which case, should we also add an HWCAP for this?
> >
> 
> Isn't this covered by the 'cpuid' HWCAP? We can't prevent EL0 from
> accessing these system registers anyway, even if we wanted to.

I see your argument, but I was just going on the side of consistency because
we're continuing to expose other features as HWCAPs when the capability is
just a proxy for the cpuid field. I was in favour of stopping the addition
of such HWCAPs years ago, but I couldn't convince Catalin ;)

The way I see it, we'll soon run out of HWCAP2 bits and then we'll have
our hand forced.

Will

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH v10 2/3] arm64: random: Add data to pool from setup_arch()
  2020-01-15  9:16     ` Will Deacon
@ 2020-01-15  9:22       ` Ard Biesheuvel
  2020-01-15 10:11         ` Mark Rutland
  2020-01-15 12:07       ` Mark Brown
  1 sibling, 1 reply; 30+ messages in thread
From: Ard Biesheuvel @ 2020-01-15  9:22 UTC (permalink / raw)
  To: Will Deacon
  Cc: Mark Rutland, Catalin Marinas, Mark Brown, Richard Henderson,
	linux-arm-kernel

On Wed, 15 Jan 2020 at 10:16, Will Deacon <will@kernel.org> wrote:
>
> On Wed, Jan 15, 2020 at 08:48:46AM +0100, Ard Biesheuvel wrote:
> > On Fri, 10 Jan 2020 at 13:23, Mark Brown <broonie@kernel.org> wrote:
> > >
> > > Since the arm64 ARCH_RANDOM implementation is not available until
> > > cpufeature has determined the system capabilities it can't be used by
> > > the generic random code to initialize the entropy pool for early use.
> > > Instead explicitly add some data to the pool from setup_arch() if the
> > > boot CPU supports v8.5-RNG, this is the point recommended by the generic
> > > code.
> > >
> > > Note that we are only adding data here, it will be mixed into the pool
> > > but won't be credited as entropy. There are currently no suitable
> > > interfaces for that at present - extending the random code to provide
> > > those will be done as a future step. Providing data is better than not
> > > doing so as it will still provide an increase in variation in the output
> > > from the random code and there will be no impact on the rate at which
> > > entropy is credited compared to what we have without this patch.
> > >
> >
> > This is slightly unfortunate, as this way, we lose the ability to use
> > random.trust_cpu=1 to get the entropy credited and initialize CRNG
> > early.
>
> Agreed. Do you think we should wait for that support before merging the
> series? Given that I don't know of any CPUs implementing this extension,
> we can probably afford not to rush this in.
>

In a previous iteration, we did have a functional
arch_get_random_seed_long() early on, which would solve this issue
without even needing a patch like this.

Perhaps Mark (Rutland) can give a recap of his concerns at the time?

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH v10 1/3] arm64: Implement archrandom.h for ARMv8.5-RNG
  2020-01-15  9:16       ` Will Deacon
@ 2020-01-15  9:24         ` Ard Biesheuvel
  2020-01-15 11:07           ` Mark Brown
  0 siblings, 1 reply; 30+ messages in thread
From: Ard Biesheuvel @ 2020-01-15  9:24 UTC (permalink / raw)
  To: Will Deacon
  Cc: Mark Rutland, Catalin Marinas, Mark Brown, Richard Henderson,
	linux-arm-kernel

On Wed, 15 Jan 2020 at 10:16, Will Deacon <will@kernel.org> wrote:
>
> On Wed, Jan 15, 2020 at 08:40:46AM +0100, Ard Biesheuvel wrote:
> > On Tue, 14 Jan 2020 at 18:44, Will Deacon <will@kernel.org> wrote:
> > >
> > > On Fri, Jan 10, 2020 at 12:23:39PM +0000, Mark Brown wrote:
> > > > From: Richard Henderson <richard.henderson@linaro.org>
> > > >
> > > > Expose the ID_AA64ISAR0.RNDR field to userspace, as the RNG system
> > > > registers are always available at EL0.
> > > >
> > > > Implement arch_get_random_seed_long using RNDR.  Given that the
> > > > TRNG is likely to be a shared resource between cores, and VMs,
> > > > do not explicitly force re-seeding with RNDRRS.  In order to avoid
> > > > code complexity and potential issues with hetrogenous systems only
> > > > provide values after cpufeature has finalized the system capabilities.
> > > >
> > > > Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> > > > [Modified to only function after cpufeature has finalized the system
> > > > capabilities and move all the code into the header -- broonie]
> > > > Signed-off-by: Mark Brown <broonie@kernel.org>
> > > > Reviewed-by: Mark Rutland <mark.rutland@arm.com>
> > > > ---
> > > >  Documentation/arm64/cpu-feature-registers.rst |  2 +
> > > >  arch/arm64/Kconfig                            | 12 ++++
> > > >  arch/arm64/include/asm/archrandom.h           | 67 +++++++++++++++++++
> > > >  arch/arm64/include/asm/cpucaps.h              |  3 +-
> > > >  arch/arm64/include/asm/sysreg.h               |  4 ++
> > > >  arch/arm64/kernel/cpufeature.c                | 13 ++++
> > > >  6 files changed, 100 insertions(+), 1 deletion(-)
> > > >  create mode 100644 arch/arm64/include/asm/archrandom.h
> > >
> > > In which case, should we also add an HWCAP for this?
> > >
> >
> > Isn't this covered by the 'cpuid' HWCAP? We can't prevent EL0 from
> > accessing these system registers anyway, even if we wanted to.
>
> I see your argument, but I was just going on the side of consistency because
> we're continuing to expose other features as HWCAPs when the capability is
> just a proxy for the cpuid field. I was in favour of stopping the addition
> of such HWCAPs years ago, but I couldn't convince Catalin ;)
>
> The way I see it, we'll soon run out of HWCAP2 bits and then we'll have
> our hand forced.
>

I don't have a strong opinion either way.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH v10 2/3] arm64: random: Add data to pool from setup_arch()
  2020-01-15  9:22       ` Ard Biesheuvel
@ 2020-01-15 10:11         ` Mark Rutland
  2020-01-15 14:01           ` Mark Brown
  0 siblings, 1 reply; 30+ messages in thread
From: Mark Rutland @ 2020-01-15 10:11 UTC (permalink / raw)
  To: Ard Biesheuvel
  Cc: Catalin Marinas, Richard Henderson, Mark Brown, Will Deacon,
	linux-arm-kernel

On Wed, Jan 15, 2020 at 10:22:03AM +0100, Ard Biesheuvel wrote:
> On Wed, 15 Jan 2020 at 10:16, Will Deacon <will@kernel.org> wrote:
> >
> > On Wed, Jan 15, 2020 at 08:48:46AM +0100, Ard Biesheuvel wrote:
> > > On Fri, 10 Jan 2020 at 13:23, Mark Brown <broonie@kernel.org> wrote:
> > > >
> > > > Since the arm64 ARCH_RANDOM implementation is not available until
> > > > cpufeature has determined the system capabilities it can't be used by
> > > > the generic random code to initialize the entropy pool for early use.
> > > > Instead explicitly add some data to the pool from setup_arch() if the
> > > > boot CPU supports v8.5-RNG, this is the point recommended by the generic
> > > > code.
> > > >
> > > > Note that we are only adding data here, it will be mixed into the pool
> > > > but won't be credited as entropy. There are currently no suitable
> > > > interfaces for that at present - extending the random code to provide
> > > > those will be done as a future step. Providing data is better than not
> > > > doing so as it will still provide an increase in variation in the output
> > > > from the random code and there will be no impact on the rate at which
> > > > entropy is credited compared to what we have without this patch.
> > > >
> > >
> > > This is slightly unfortunate, as this way, we lose the ability to use
> > > random.trust_cpu=1 to get the entropy credited and initialize CRNG
> > > early.
> >
> > Agreed. Do you think we should wait for that support before merging the
> > series? Given that I don't know of any CPUs implementing this extension,
> > we can probably afford not to rush this in.
> 
> In a previous iteration, we did have a functional
> arch_get_random_seed_long() early on, which would solve this issue
> without even needing a patch like this.
> 
> Perhaps Mark (Rutland) can give a recap of his concerns at the time?

It meant that the common runtime path had code that was only ever meant
to run at boot time, and would also run on secondary CPUs until we
finalized the caps, so they'd behave inconsistently across boot and
hotplug paths. I was concerned that this was messy and would be painful
to reason about and debug.

My suggestion was that we either:

(a) Had the arch code explicitly inject the entropy in the primary setup
    path, as these patches do, or;

(b) Had a new callback (e.g. __early_arch_get_random_seed_long()) that
    the core random code only called during its initialization, separate
    to the runtime paths.

Thanks,
Mark.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH v10 1/3] arm64: Implement archrandom.h for ARMv8.5-RNG
  2020-01-15  9:24         ` Ard Biesheuvel
@ 2020-01-15 11:07           ` Mark Brown
  2020-01-15 11:16             ` Will Deacon
  2020-01-15 14:26             ` Catalin Marinas
  0 siblings, 2 replies; 30+ messages in thread
From: Mark Brown @ 2020-01-15 11:07 UTC (permalink / raw)
  To: Ard Biesheuvel
  Cc: Mark Rutland, Catalin Marinas, Richard Henderson, Will Deacon,
	linux-arm-kernel

[-- Attachment #1.1: Type: text/plain, Size: 655 bytes --]

On Wed, Jan 15, 2020 at 10:24:21AM +0100, Ard Biesheuvel wrote:
> On Wed, 15 Jan 2020 at 10:16, Will Deacon <will@kernel.org> wrote:

> > I see your argument, but I was just going on the side of consistency because
> > we're continuing to expose other features as HWCAPs when the capability is
> > just a proxy for the cpuid field. I was in favour of stopping the addition
> > of such HWCAPs years ago, but I couldn't convince Catalin ;)

> > The way I see it, we'll soon run out of HWCAP2 bits and then we'll have
> > our hand forced.

> I don't have a strong opinion either way.

Me either, or at least not enough to object to doing it - Will?
Catalin?

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[-- Attachment #2: Type: text/plain, Size: 176 bytes --]

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH v10 1/3] arm64: Implement archrandom.h for ARMv8.5-RNG
  2020-01-15 11:07           ` Mark Brown
@ 2020-01-15 11:16             ` Will Deacon
  2020-01-15 14:26             ` Catalin Marinas
  1 sibling, 0 replies; 30+ messages in thread
From: Will Deacon @ 2020-01-15 11:16 UTC (permalink / raw)
  To: Mark Brown
  Cc: Mark Rutland, Catalin Marinas, Richard Henderson,
	linux-arm-kernel, Ard Biesheuvel

On Wed, Jan 15, 2020 at 11:07:20AM +0000, Mark Brown wrote:
> On Wed, Jan 15, 2020 at 10:24:21AM +0100, Ard Biesheuvel wrote:
> > On Wed, 15 Jan 2020 at 10:16, Will Deacon <will@kernel.org> wrote:
> 
> > > I see your argument, but I was just going on the side of consistency because
> > > we're continuing to expose other features as HWCAPs when the capability is
> > > just a proxy for the cpuid field. I was in favour of stopping the addition
> > > of such HWCAPs years ago, but I couldn't convince Catalin ;)
> 
> > > The way I see it, we'll soon run out of HWCAP2 bits and then we'll have
> > > our hand forced.
> 
> > I don't have a strong opinion either way.
> 
> Me either, or at least not enough to object to doing it - Will?
> Catalin?

Unless we all agree to stop adding new HWCAPs, then I think you may as well
add a new one here (i.e. I don't think this discussion should derail your
work).

Will

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH v10 2/3] arm64: random: Add data to pool from setup_arch()
  2020-01-15  9:16     ` Will Deacon
  2020-01-15  9:22       ` Ard Biesheuvel
@ 2020-01-15 12:07       ` Mark Brown
  2020-01-15 12:42         ` Will Deacon
  1 sibling, 1 reply; 30+ messages in thread
From: Mark Brown @ 2020-01-15 12:07 UTC (permalink / raw)
  To: Will Deacon
  Cc: Mark Rutland, Catalin Marinas, Richard Henderson,
	linux-arm-kernel, Ard Biesheuvel

[-- Attachment #1.1: Type: text/plain, Size: 2334 bytes --]

On Wed, Jan 15, 2020 at 09:16:16AM +0000, Will Deacon wrote:
> On Wed, Jan 15, 2020 at 08:48:46AM +0100, Ard Biesheuvel wrote:

> > > Note that we are only adding data here, it will be mixed into the pool
> > > but won't be credited as entropy. There are currently no suitable
> > > interfaces for that at present - extending the random code to provide

> > This is slightly unfortunate, as this way, we lose the ability to use
> > random.trust_cpu=1 to get the entropy credited and initialize CRNG
> > early.

Right.  OTOH that's a bit of a mess to do, I do have some
thoughts but it's a bit of a mess trying to do it tastefully,
especially when considering that you probably don't want an
interface that it's easy for something to misuse.  The effort
involved certainly seems large enough to handle separately.

> Agreed. Do you think we should wait for that support before merging the
> series? Given that I don't know of any CPUs implementing this extension,
> we can probably afford not to rush this in.

It's implemented in at least the fast models already, not checked
any of the other emulators, so there's some possibility of people
using it while developing other things and hopefully at least
some of the various CI systems will be including emulated
platforms with newer extensions in their coverage so might gain
some benefit from it.  Frankly the only reason I'm looking at
this at all is that I'd written patch 3 because I was getting fed
up with KASLR initialization being easily disabled when I was
trying to test E0PD on the models (especially before I added the
status print at boot to KASLR so this happened silently), having
this in mainline would've helped considerably when working on
that.

I don't see any downside to having the code in mainline as is,
even though it's not ideal it does make things better since if
for some reason anyone does end up running this code on a system
that has the feature they'll get at least some benefit from it
even if nothing else happens.  The bulk of the code isn't going
to change when the early init stuff gets improved and includes
tables like cpufeature.h that make it annoying to hold out of
tree, the bits that are going to change can just as well be
worked on incrementally as held out of tree entirely and having
the rest in means there's less friction doing that.

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[-- Attachment #2: Type: text/plain, Size: 176 bytes --]

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH v10 2/3] arm64: random: Add data to pool from setup_arch()
  2020-01-15 12:07       ` Mark Brown
@ 2020-01-15 12:42         ` Will Deacon
  2020-01-15 13:36           ` Ard Biesheuvel
  2020-01-15 15:40           ` Mark Brown
  0 siblings, 2 replies; 30+ messages in thread
From: Will Deacon @ 2020-01-15 12:42 UTC (permalink / raw)
  To: Mark Brown
  Cc: Mark Rutland, Catalin Marinas, Richard Henderson,
	linux-arm-kernel, Ard Biesheuvel

On Wed, Jan 15, 2020 at 12:07:03PM +0000, Mark Brown wrote:
> On Wed, Jan 15, 2020 at 09:16:16AM +0000, Will Deacon wrote:
> > On Wed, Jan 15, 2020 at 08:48:46AM +0100, Ard Biesheuvel wrote:
> 
> > > > Note that we are only adding data here, it will be mixed into the pool
> > > > but won't be credited as entropy. There are currently no suitable
> > > > interfaces for that at present - extending the random code to provide
> 
> > > This is slightly unfortunate, as this way, we lose the ability to use
> > > random.trust_cpu=1 to get the entropy credited and initialize CRNG
> > > early.
> 
> Right.  OTOH that's a bit of a mess to do, I do have some
> thoughts but it's a bit of a mess trying to do it tastefully,
> especially when considering that you probably don't want an
> interface that it's easy for something to misuse.  The effort
> involved certainly seems large enough to handle separately.

Maybe, but see below...

> > Agreed. Do you think we should wait for that support before merging the
> > series? Given that I don't know of any CPUs implementing this extension,
> > we can probably afford not to rush this in.
> 
> It's implemented in at least the fast models already, not checked
> any of the other emulators, so there's some possibility of people
> using it while developing other things and hopefully at least
> some of the various CI systems will be including emulated
> platforms with newer extensions in their coverage so might gain
> some benefit from it.  Frankly the only reason I'm looking at
> this at all is that I'd written patch 3 because I was getting fed
> up with KASLR initialization being easily disabled when I was
> trying to test E0PD on the models (especially before I added the
> status print at boot to KASLR so this happened silently), having
> this in mainline would've helped considerably when working on
> that.

I was thinking specifically about users on silicon rather than developers
on simulators. (I could stick this on a branch for developers if necessary).

> I don't see any downside to having the code in mainline as is,
> even though it's not ideal it does make things better since if
> for some reason anyone does end up running this code on a system
> that has the feature they'll get at least some benefit from it
> even if nothing else happens.  The bulk of the code isn't going
> to change when the early init stuff gets improved and includes
> tables like cpufeature.h that make it annoying to hold out of
> tree, the bits that are going to change can just as well be
> worked on incrementally as held out of tree entirely and having
> the rest in means there's less friction doing that.

The usual downside that comes from merging patches with promises of fixing
them up later is that the motivating task gets marked as "done" somewhere,
the developer gets given something else to do and the updates never
materialise. That's not a dig at you; it's just the way these things tend
to work (I've certainly been on both sides of that coin).

If there was an urgency to this, I'd suggest merging a form of Richard's
code, as it appears to solve the technical issue of credited entropy whilst
leaving some room for subsequent cleanup. However, I think that makes it
even less likely that anybody will come back to do the cleanup because the
code will be perfectly functional, so I'd prefer to wait for a complete
solution unless you think it's not achievable for 5.7.

I'd also really like Ard's ack on anything relating to RNGs.

Will

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH v10 2/3] arm64: random: Add data to pool from setup_arch()
  2020-01-15 12:42         ` Will Deacon
@ 2020-01-15 13:36           ` Ard Biesheuvel
  2020-01-15 17:04             ` Mark Brown
  2020-01-15 15:40           ` Mark Brown
  1 sibling, 1 reply; 30+ messages in thread
From: Ard Biesheuvel @ 2020-01-15 13:36 UTC (permalink / raw)
  To: Will Deacon
  Cc: Mark Rutland, Catalin Marinas, Mark Brown, Richard Henderson,
	linux-arm-kernel

On Wed, 15 Jan 2020 at 13:42, Will Deacon <will@kernel.org> wrote:
>
> On Wed, Jan 15, 2020 at 12:07:03PM +0000, Mark Brown wrote:
> > On Wed, Jan 15, 2020 at 09:16:16AM +0000, Will Deacon wrote:
> > > On Wed, Jan 15, 2020 at 08:48:46AM +0100, Ard Biesheuvel wrote:
> >
> > > > > Note that we are only adding data here, it will be mixed into the pool
> > > > > but won't be credited as entropy. There are currently no suitable
> > > > > interfaces for that at present - extending the random code to provide
> >
> > > > This is slightly unfortunate, as this way, we lose the ability to use
> > > > random.trust_cpu=1 to get the entropy credited and initialize CRNG
> > > > early.
> >
> > Right.  OTOH that's a bit of a mess to do, I do have some
> > thoughts but it's a bit of a mess trying to do it tastefully,
> > especially when considering that you probably don't want an
> > interface that it's easy for something to misuse.  The effort
> > involved certainly seems large enough to handle separately.
>
> Maybe, but see below...
>
> > > Agreed. Do you think we should wait for that support before merging the
> > > series? Given that I don't know of any CPUs implementing this extension,
> > > we can probably afford not to rush this in.
> >
> > It's implemented in at least the fast models already, not checked
> > any of the other emulators, so there's some possibility of people
> > using it while developing other things and hopefully at least
> > some of the various CI systems will be including emulated
> > platforms with newer extensions in their coverage so might gain
> > some benefit from it.  Frankly the only reason I'm looking at
> > this at all is that I'd written patch 3 because I was getting fed
> > up with KASLR initialization being easily disabled when I was
> > trying to test E0PD on the models (especially before I added the
> > status print at boot to KASLR so this happened silently), having
> > this in mainline would've helped considerably when working on
> > that.
>
> I was thinking specifically about users on silicon rather than developers
> on simulators. (I could stick this on a branch for developers if necessary).
>
> > I don't see any downside to having the code in mainline as is,
> > even though it's not ideal it does make things better since if
> > for some reason anyone does end up running this code on a system
> > that has the feature they'll get at least some benefit from it
> > even if nothing else happens.  The bulk of the code isn't going
> > to change when the early init stuff gets improved and includes
> > tables like cpufeature.h that make it annoying to hold out of
> > tree, the bits that are going to change can just as well be
> > worked on incrementally as held out of tree entirely and having
> > the rest in means there's less friction doing that.
>
> The usual downside that comes from merging patches with promises of fixing
> them up later is that the motivating task gets marked as "done" somewhere,
> the developer gets given something else to do and the updates never
> materialise. That's not a dig at you; it's just the way these things tend
> to work (I've certainly been on both sides of that coin).
>
> If there was an urgency to this, I'd suggest merging a form of Richard's
> code, as it appears to solve the technical issue of credited entropy whilst
> leaving some room for subsequent cleanup. However, I think that makes it
> even less likely that anybody will come back to do the cleanup because the
> code will be perfectly functional, so I'd prefer to wait for a complete
> solution unless you think it's not achievable for 5.7.
>
> I'd also really like Ard's ack on anything relating to RNGs.
>

Patches #1 and #3 are fine with me, modulo the HWCAP bit which I don't
deeply care about.

But the way this patch works around our workaround for mismatched RNG
caps between cores doesn't make sense to me.
arch_get_random_seed_long() should just have some out of line __init
path that gets invoked only during early boot, exactly how we are
using it in patch #3 to seed KASLR, where we don't care about whether
or not other CPUs have the extension. (Note that rand_initialize() is
called very early, way before the point where we have to care about
being scheduled from a CPU with RNG to one without)

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH v10 2/3] arm64: random: Add data to pool from setup_arch()
  2020-01-15 10:11         ` Mark Rutland
@ 2020-01-15 14:01           ` Mark Brown
  0 siblings, 0 replies; 30+ messages in thread
From: Mark Brown @ 2020-01-15 14:01 UTC (permalink / raw)
  To: Mark Rutland
  Cc: Catalin Marinas, Richard Henderson, Will Deacon,
	linux-arm-kernel, Ard Biesheuvel

[-- Attachment #1.1: Type: text/plain, Size: 1622 bytes --]

On Wed, Jan 15, 2020 at 10:11:08AM +0000, Mark Rutland wrote:
> On Wed, Jan 15, 2020 at 10:22:03AM +0100, Ard Biesheuvel wrote:

> > In a previous iteration, we did have a functional
> > arch_get_random_seed_long() early on, which would solve this issue
> > without even needing a patch like this.

> It meant that the common runtime path had code that was only ever meant
> to run at boot time, and would also run on secondary CPUs until we
> finalized the caps, so they'd behave inconsistently across boot and
> hotplug paths. I was concerned that this was messy and would be painful
> to reason about and debug.

> My suggestion was that we either:

> (a) Had the arch code explicitly inject the entropy in the primary setup
>     path, as these patches do, or;

These patches don't quite do that, they inject data but not
entropy so anything that is waiting for the pool to become fully
initialized will still end up waiting, though we do still get the
data mixed in.  There is currently no interface which allows one
to explicitly inject entropy as though from the architecture and
I'm not convinced that having one would be a good idea.

> (b) Had a new callback (e.g. __early_arch_get_random_seed_long()) that
>     the core random code only called during its initialization, separate
>     to the runtime paths.

This is definitely an option, but it is a bit ugly and as things
stand with random.c it would I think have to cope with possibly
running with multiple processors at which point we start to get
back to the complexity you were originally worried about just in
a code path that's less commonly executed.

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[-- Attachment #2: Type: text/plain, Size: 176 bytes --]

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH v10 1/3] arm64: Implement archrandom.h for ARMv8.5-RNG
  2020-01-15 11:07           ` Mark Brown
  2020-01-15 11:16             ` Will Deacon
@ 2020-01-15 14:26             ` Catalin Marinas
  2020-01-16  0:23               ` Richard Henderson
  1 sibling, 1 reply; 30+ messages in thread
From: Catalin Marinas @ 2020-01-15 14:26 UTC (permalink / raw)
  To: Mark Brown
  Cc: Mark Rutland, Richard Henderson, Will Deacon, linux-arm-kernel,
	Ard Biesheuvel

On Wed, Jan 15, 2020 at 11:07:20AM +0000, Mark Brown wrote:
> On Wed, Jan 15, 2020 at 10:24:21AM +0100, Ard Biesheuvel wrote:
> > On Wed, 15 Jan 2020 at 10:16, Will Deacon <will@kernel.org> wrote:
> 
> > > I see your argument, but I was just going on the side of consistency because
> > > we're continuing to expose other features as HWCAPs when the capability is
> > > just a proxy for the cpuid field. I was in favour of stopping the addition
> > > of such HWCAPs years ago, but I couldn't convince Catalin ;)
> 
> > > The way I see it, we'll soon run out of HWCAP2 bits and then we'll have
> > > our hand forced.
> 
> > I don't have a strong opinion either way.
> 
> Me either, or at least not enough to object to doing it - Will?
> Catalin?

Until the ifunc resolver can work with CPUID, I think we should keep
adding HWCAPn bits. We can revisit this with the toolchain people before
introducing HWCAP3.

-- 
Catalin

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH v10 2/3] arm64: random: Add data to pool from setup_arch()
  2020-01-15 12:42         ` Will Deacon
  2020-01-15 13:36           ` Ard Biesheuvel
@ 2020-01-15 15:40           ` Mark Brown
  1 sibling, 0 replies; 30+ messages in thread
From: Mark Brown @ 2020-01-15 15:40 UTC (permalink / raw)
  To: Will Deacon
  Cc: Mark Rutland, Catalin Marinas, Richard Henderson,
	linux-arm-kernel, Ard Biesheuvel

[-- Attachment #1.1: Type: text/plain, Size: 2925 bytes --]

On Wed, Jan 15, 2020 at 12:42:39PM +0000, Will Deacon wrote:
> On Wed, Jan 15, 2020 at 12:07:03PM +0000, Mark Brown wrote:

> > tables like cpufeature.h that make it annoying to hold out of
> > tree, the bits that are going to change can just as well be
> > worked on incrementally as held out of tree entirely and having
> > the rest in means there's less friction doing that.

> The usual downside that comes from merging patches with promises of fixing
> them up later is that the motivating task gets marked as "done" somewhere,
> the developer gets given something else to do and the updates never
> materialise. That's not a dig at you; it's just the way these things tend
> to work (I've certainly been on both sides of that coin).

It's certainly a way things can work, but it does assume that
there is an underlying motivating task that involves getting
things upstream which will cause people to do the additional work
rather than just wandering off and then potentially causing
someone else to redo the bit that was already done if they don't
notice the code in the list archives or spend time trying to
figure out if the original author will continue revising their
series.  We even had an awkward situation when I was at Linaro
where the original author of something we depended on was
continuing to work on their series but it was now a spare time
activity for them so progress was painfully slow, the worst of
both worlds.  The most common way this happens that I run into is
people implementing things for products who are doing the
upstreaming on the side.

It can also have the effect of discouraging people from trying to
do things in the first place, I know the likelyhood of scope
creep is one of the factors that influences how likely I am to
try to improve things I happen to notice while I'm working on
something else and I'm fairly sure other people make similar
assessments.

> If there was an urgency to this, I'd suggest merging a form of Richard's
> code, as it appears to solve the technical issue of credited entropy whilst
> leaving some room for subsequent cleanup. However, I think that makes it
> even less likely that anybody will come back to do the cleanup because the

I agree with your assessment of the likelyhood of cleanup, I
think an incomplete solution that doesn't credit entropy is more
robustly likely to get fixed since it causes an actual problem
that people will be motivated to fix as opposed to just being
ugly code.  I have no objection to merging Richard's
static_branch_likely() approach though.

> code will be perfectly functional, so I'd prefer to wait for a complete
> solution unless you think it's not achievable for 5.7.

It could happen but I wouldn't like to commit to getting
something in for v5.7, that's basically just a single release
given how near we are to the v5.6 merge window opening and I know
changes in the random code can sometimes take a while.

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[-- Attachment #2: Type: text/plain, Size: 176 bytes --]

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH v10 2/3] arm64: random: Add data to pool from setup_arch()
  2020-01-15 13:36           ` Ard Biesheuvel
@ 2020-01-15 17:04             ` Mark Brown
  2020-01-16 11:33               ` Will Deacon
  0 siblings, 1 reply; 30+ messages in thread
From: Mark Brown @ 2020-01-15 17:04 UTC (permalink / raw)
  To: Ard Biesheuvel
  Cc: Mark Rutland, Catalin Marinas, Richard Henderson, Will Deacon,
	linux-arm-kernel

[-- Attachment #1.1: Type: text/plain, Size: 1886 bytes --]

On Wed, Jan 15, 2020 at 02:36:32PM +0100, Ard Biesheuvel wrote:
> On Wed, 15 Jan 2020 at 13:42, Will Deacon <will@kernel.org> wrote:

> > I'd also really like Ard's ack on anything relating to RNGs.

> Patches #1 and #3 are fine with me, modulo the HWCAP bit which I don't
> deeply care about.

> But the way this patch works around our workaround for mismatched RNG
> caps between cores doesn't make sense to me.

I'd be totally happy to drop patch 2 entirely, it's a *bit*
marginal if it's useful - I mainly wrote it because it's so
trivial to do not because I think it's a wonderful idea.

> arch_get_random_seed_long() should just have some out of line __init
> path that gets invoked only during early boot, exactly how we are
> using it in patch #3 to seed KASLR, where we don't care about whether

Yes, I think that would be a good place to get to if we can - if
the early init thing is a separate call then we have to worry
about the callers always running from the right context which
sounds like trouble.  It's just trying to figure out a way to
write things which is clearly robust when looking at the arch
code by itself, and I don't want to completely discount the
possibility of a new interface from the random code to help with
that yet.

> or not other CPUs have the extension. (Note that rand_initialize() is
> called very early, way before the point where we have to care about
> being scheduled from a CPU with RNG to one without)

Everything is simple during rand_initialize(), though the actual
calls to get entropy that it does happen in crng_initialize()
which is also used to initialize the per-node pools for NUMA
systems but that should happen after the capabilities code has
run I think (pretty sure, but I need to check) so we can rely on
cpus_have_const_cap().  There's also calls in prandom_init()
which runs at core_initcall() so that should be fine too.

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[-- Attachment #2: Type: text/plain, Size: 176 bytes --]

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH v10 1/3] arm64: Implement archrandom.h for ARMv8.5-RNG
  2020-01-15 14:26             ` Catalin Marinas
@ 2020-01-16  0:23               ` Richard Henderson
  2020-01-16 11:02                 ` Catalin Marinas
  0 siblings, 1 reply; 30+ messages in thread
From: Richard Henderson @ 2020-01-16  0:23 UTC (permalink / raw)
  To: Catalin Marinas, Mark Brown
  Cc: Mark Rutland, Will Deacon, linux-arm-kernel, Ard Biesheuvel

On 1/15/20 4:26 AM, Catalin Marinas wrote:
> On Wed, Jan 15, 2020 at 11:07:20AM +0000, Mark Brown wrote:
>> On Wed, Jan 15, 2020 at 10:24:21AM +0100, Ard Biesheuvel wrote:
>>> On Wed, 15 Jan 2020 at 10:16, Will Deacon <will@kernel.org> wrote:
>>
>>>> I see your argument, but I was just going on the side of consistency because
>>>> we're continuing to expose other features as HWCAPs when the capability is
>>>> just a proxy for the cpuid field. I was in favour of stopping the addition
>>>> of such HWCAPs years ago, but I couldn't convince Catalin ;)
>>
>>>> The way I see it, we'll soon run out of HWCAP2 bits and then we'll have
>>>> our hand forced.
>>
>>> I don't have a strong opinion either way.
>>
>> Me either, or at least not enough to object to doing it - Will?
>> Catalin?
> 
> Until the ifunc resolver can work with CPUID, I think we should keep
> adding HWCAPn bits. We can revisit this with the toolchain people before
> introducing HWCAP3.

Why would the ifunc resolver not be able to use HWCAP_CPUID?

The first argument to the ifunc resolver, apparently since the beginning of
time (2013-11-25 7520ff8c744a), AT_HWCAP has been passed directly as the first
argument.

That means HWCAP_CPUID, present in AT_HWCAP, can always be tested directly.  At
which point one can access architected registers, with no dynamic linker
relocations, to make further decisions.  Admittedly there's a trap to the OS
involved, but there is *far* too much info in those registers to copy
everything to HWCAPn.

The current state of affairs, as of glibc-2.30, is that the first argument is
augmented to include a _IFUNC_ARG_HWCAP bit, which indicates the presence of a
second argument, a pointer to struct __ifunc_arg_t.  This struct does include a
size field, allowing the struct to be extended in future.

That said, speaking as a toolchain guy, you should conserve HWCAP2 bits so
that, by preference, you do not need to introduce AT_HWCAP3.  Or at least delay
adding it.


r~

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH v10 1/3] arm64: Implement archrandom.h for ARMv8.5-RNG
  2020-01-16  0:23               ` Richard Henderson
@ 2020-01-16 11:02                 ` Catalin Marinas
  2020-01-16 11:10                   ` Ard Biesheuvel
  0 siblings, 1 reply; 30+ messages in thread
From: Catalin Marinas @ 2020-01-16 11:02 UTC (permalink / raw)
  To: Richard Henderson
  Cc: Mark Rutland, Mark Brown, Will Deacon, linux-arm-kernel, Ard Biesheuvel

On Wed, Jan 15, 2020 at 02:23:39PM -1000, Richard Henderson wrote:
> On 1/15/20 4:26 AM, Catalin Marinas wrote:
> > On Wed, Jan 15, 2020 at 11:07:20AM +0000, Mark Brown wrote:
> >> On Wed, Jan 15, 2020 at 10:24:21AM +0100, Ard Biesheuvel wrote:
> >>> On Wed, 15 Jan 2020 at 10:16, Will Deacon <will@kernel.org> wrote:
> >>
> >>>> I see your argument, but I was just going on the side of consistency because
> >>>> we're continuing to expose other features as HWCAPs when the capability is
> >>>> just a proxy for the cpuid field. I was in favour of stopping the addition
> >>>> of such HWCAPs years ago, but I couldn't convince Catalin ;)
> >>
> >>>> The way I see it, we'll soon run out of HWCAP2 bits and then we'll have
> >>>> our hand forced.
> >>
> >>> I don't have a strong opinion either way.
> >>
> >> Me either, or at least not enough to object to doing it - Will?
> >> Catalin?
> > 
> > Until the ifunc resolver can work with CPUID, I think we should keep
> > adding HWCAPn bits. We can revisit this with the toolchain people before
> > introducing HWCAP3.
> 
> Why would the ifunc resolver not be able to use HWCAP_CPUID?

It can indeed check the HWCAP_CPUID but I haven't seen any plans to
implement the next part, actual use of an MRS instruction to read the
corresponding ID_AA64* regs. This MRS emulation was requested by (some
of) the toolchain people, even the architecture gained a feature to
simplify the emulation, but followed by complete silence from the
toolchain folk.

> That said, speaking as a toolchain guy, you should conserve HWCAP2 bits so
> that, by preference, you do not need to introduce AT_HWCAP3.  Or at least delay
> adding it.

We still have some time before AT_HWCAP3. Also, we have 32-bit spare in
both HWCAP and HWCAP2 which we can use. IIRC we didn't go into the top
32-bit of HWCAP because we were still debating whether ILP32 makes
sense (and now I'm 100% convinced it doesn't ;)).

-- 
Catalin

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH v10 1/3] arm64: Implement archrandom.h for ARMv8.5-RNG
  2020-01-16 11:02                 ` Catalin Marinas
@ 2020-01-16 11:10                   ` Ard Biesheuvel
  2020-01-16 11:40                     ` Catalin Marinas
  0 siblings, 1 reply; 30+ messages in thread
From: Ard Biesheuvel @ 2020-01-16 11:10 UTC (permalink / raw)
  To: Catalin Marinas
  Cc: Mark Rutland, Will Deacon, Mark Brown, Richard Henderson,
	linux-arm-kernel

On Thu, 16 Jan 2020 at 12:02, Catalin Marinas <catalin.marinas@arm.com> wrote:
>
> On Wed, Jan 15, 2020 at 02:23:39PM -1000, Richard Henderson wrote:
> > On 1/15/20 4:26 AM, Catalin Marinas wrote:
> > > On Wed, Jan 15, 2020 at 11:07:20AM +0000, Mark Brown wrote:
> > >> On Wed, Jan 15, 2020 at 10:24:21AM +0100, Ard Biesheuvel wrote:
> > >>> On Wed, 15 Jan 2020 at 10:16, Will Deacon <will@kernel.org> wrote:
> > >>
> > >>>> I see your argument, but I was just going on the side of consistency because
> > >>>> we're continuing to expose other features as HWCAPs when the capability is
> > >>>> just a proxy for the cpuid field. I was in favour of stopping the addition
> > >>>> of such HWCAPs years ago, but I couldn't convince Catalin ;)
> > >>
> > >>>> The way I see it, we'll soon run out of HWCAP2 bits and then we'll have
> > >>>> our hand forced.
> > >>
> > >>> I don't have a strong opinion either way.
> > >>
> > >> Me either, or at least not enough to object to doing it - Will?
> > >> Catalin?
> > >
> > > Until the ifunc resolver can work with CPUID, I think we should keep
> > > adding HWCAPn bits. We can revisit this with the toolchain people before
> > > introducing HWCAP3.
> >
> > Why would the ifunc resolver not be able to use HWCAP_CPUID?
>
> It can indeed check the HWCAP_CPUID but I haven't seen any plans to
> implement the next part, actual use of an MRS instruction to read the
> corresponding ID_AA64* regs. This MRS emulation was requested by (some
> of) the toolchain people, even the architecture gained a feature to
> simplify the emulation, but followed by complete silence from the
> toolchain folk.
>

But what infrastructure would the toolchain folks need to provide
here? An ifunc resolver would simply do

void generic_func(...);
void foo_func(...);

void *resolve_foo(long hwcap)
{
   if (hwcap & HWCAP_CPUID) {
       long l;
       asm ("mrs %1, ID_AA64_...") : "=r"(l));
       if (l has 'foo')
         return foo_func;
   }
   return generic_func;
}

so all that is needed for using ID registers to do ifunc resolution is
already there.


> > That said, speaking as a toolchain guy, you should conserve HWCAP2 bits so
> > that, by preference, you do not need to introduce AT_HWCAP3.  Or at least delay
> > adding it.
>
> We still have some time before AT_HWCAP3. Also, we have 32-bit spare in
> both HWCAP and HWCAP2 which we can use. IIRC we didn't go into the top
> 32-bit of HWCAP because we were still debating whether ILP32 makes
> sense (and now I'm 100% convinced it doesn't ;)).
>
> --
> Catalin

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH v10 2/3] arm64: random: Add data to pool from setup_arch()
  2020-01-15 17:04             ` Mark Brown
@ 2020-01-16 11:33               ` Will Deacon
  0 siblings, 0 replies; 30+ messages in thread
From: Will Deacon @ 2020-01-16 11:33 UTC (permalink / raw)
  To: Mark Brown
  Cc: Mark Rutland, Catalin Marinas, Richard Henderson,
	linux-arm-kernel, Ard Biesheuvel

On Wed, Jan 15, 2020 at 05:04:59PM +0000, Mark Brown wrote:
> On Wed, Jan 15, 2020 at 02:36:32PM +0100, Ard Biesheuvel wrote:
> > On Wed, 15 Jan 2020 at 13:42, Will Deacon <will@kernel.org> wrote:
> 
> > > I'd also really like Ard's ack on anything relating to RNGs.
> 
> > Patches #1 and #3 are fine with me, modulo the HWCAP bit which I don't
> > deeply care about.
> 
> > But the way this patch works around our workaround for mismatched RNG
> > caps between cores doesn't make sense to me.
> 
> I'd be totally happy to drop patch 2 entirely, it's a *bit*
> marginal if it's useful - I mainly wrote it because it's so
> trivial to do not because I think it's a wonderful idea.

OK, tell you what -- please resend just 1 and 3, with the HWCAP addition
(because we're not going to resolve that in time for 5.6) and I'll queue
them up.

Will

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH v10 1/3] arm64: Implement archrandom.h for ARMv8.5-RNG
  2020-01-16 11:10                   ` Ard Biesheuvel
@ 2020-01-16 11:40                     ` Catalin Marinas
  0 siblings, 0 replies; 30+ messages in thread
From: Catalin Marinas @ 2020-01-16 11:40 UTC (permalink / raw)
  To: Ard Biesheuvel
  Cc: Mark Rutland, Will Deacon, Mark Brown, Richard Henderson,
	linux-arm-kernel

On Thu, Jan 16, 2020 at 12:10:28PM +0100, Ard Biesheuvel wrote:
> On Thu, 16 Jan 2020 at 12:02, Catalin Marinas <catalin.marinas@arm.com> wrote:
> > On Wed, Jan 15, 2020 at 02:23:39PM -1000, Richard Henderson wrote:
> > > On 1/15/20 4:26 AM, Catalin Marinas wrote:
> > > > Until the ifunc resolver can work with CPUID, I think we should keep
> > > > adding HWCAPn bits. We can revisit this with the toolchain people before
> > > > introducing HWCAP3.
> > >
> > > Why would the ifunc resolver not be able to use HWCAP_CPUID?
> >
> > It can indeed check the HWCAP_CPUID but I haven't seen any plans to
> > implement the next part, actual use of an MRS instruction to read the
> > corresponding ID_AA64* regs. This MRS emulation was requested by (some
> > of) the toolchain people, even the architecture gained a feature to
> > simplify the emulation, but followed by complete silence from the
> > toolchain folk.
> 
> But what infrastructure would the toolchain folks need to provide
> here? An ifunc resolver would simply do
> 
> void generic_func(...);
> void foo_func(...);
> 
> void *resolve_foo(long hwcap)
> {
>    if (hwcap & HWCAP_CPUID) {
>        long l;
>        asm ("mrs %1, ID_AA64_...") : "=r"(l));
>        if (l has 'foo')
>          return foo_func;
>    }
>    return generic_func;
> }
> 
> so all that is needed for using ID registers to do ifunc resolution is
> already there.

If you write the resolver yourself, it should work. I was thinking of
function multiversioning (which I thought using ifunc behind the scenes)
but I'm not sure what the aarch64 support level is (in gcc or clang).

Anyway, I'm not aware of anyone using the MRS emulation (maybe they do
and I haven't heard). I guess it doesn't help that we keep adding HWCAP
bits ;).

-- 
Catalin

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 30+ messages in thread

end of thread, back to index

Thread overview: 30+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-01-10 12:23 [PATCH v10 0/3] ARMv8.5-RNG support Mark Brown
2020-01-10 12:23 ` [PATCH v10 1/3] arm64: Implement archrandom.h for ARMv8.5-RNG Mark Brown
2020-01-14 17:44   ` Will Deacon
2020-01-15  7:40     ` Ard Biesheuvel
2020-01-15  9:16       ` Will Deacon
2020-01-15  9:24         ` Ard Biesheuvel
2020-01-15 11:07           ` Mark Brown
2020-01-15 11:16             ` Will Deacon
2020-01-15 14:26             ` Catalin Marinas
2020-01-16  0:23               ` Richard Henderson
2020-01-16 11:02                 ` Catalin Marinas
2020-01-16 11:10                   ` Ard Biesheuvel
2020-01-16 11:40                     ` Catalin Marinas
2020-01-10 12:23 ` [PATCH v10 2/3] arm64: random: Add data to pool from setup_arch() Mark Brown
2020-01-10 12:35   ` Mark Rutland
2020-01-13 19:09   ` Richard Henderson
2020-01-15  7:48   ` Ard Biesheuvel
2020-01-15  9:16     ` Will Deacon
2020-01-15  9:22       ` Ard Biesheuvel
2020-01-15 10:11         ` Mark Rutland
2020-01-15 14:01           ` Mark Brown
2020-01-15 12:07       ` Mark Brown
2020-01-15 12:42         ` Will Deacon
2020-01-15 13:36           ` Ard Biesheuvel
2020-01-15 17:04             ` Mark Brown
2020-01-16 11:33               ` Will Deacon
2020-01-15 15:40           ` Mark Brown
2020-01-10 12:23 ` [PATCH v10 3/3] arm64: Use v8.5-RNG entropy for KASLR seed Mark Brown
2020-01-10 12:35   ` Mark Rutland
2020-01-13 19:09   ` Richard Henderson

Linux-ARM-Kernel Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-arm-kernel/0 linux-arm-kernel/git/0.git
	git clone --mirror https://lore.kernel.org/linux-arm-kernel/1 linux-arm-kernel/git/1.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-arm-kernel linux-arm-kernel/ https://lore.kernel.org/linux-arm-kernel \
		linux-arm-kernel@lists.infradead.org
	public-inbox-index linux-arm-kernel

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.infradead.lists.linux-arm-kernel


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git