Re: [PATCH v8 1/3] arm64: Implement archrandom.h for ARMv8.5-RNG

[Mark Rutland <mark.rutland@arm.com>]

On Tue, Jan 07, 2020 at 06:35:38PM +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>
> ---
>  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..364177954bef
> --- /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)

Nit: missing spacing here, this last line should be:

	: "=r" (*v), "=r (ok)

Otherwise, the patch looks good to me. With that fixed:

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

Thanks for sorting this out!

Mark.

> +	:
> +	: "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