[PATCH] arm64: archrandom: avoid trapping ID register read in __cpu_has_rng()

[PATCH] arm64: archrandom: avoid trapping ID register read in __cpu_has_rng()

[Aman Priyadarshi]

__cpu_has_rng() has an early-boot fallback, taken before the
ARM64_HAS_RNG alternative is patched, that calls
this_cpu_has_cap(ARM64_HAS_RNG). With SCOPE_LOCAL_CPU that resolves the
capability by reading ID_AA64ISAR0_EL1 directly from hardware via
__read_sysreg_by_encoding(), on every invocation.

Until the CRNG is seeded, crng_make_state() routes every get_random_*()
through extract_entropy(), which drains architectural entropy via
arch_get_random_seed_longs()/arch_get_random_longs() and so calls
__cpu_has_rng() several times per request. On a direct (non-EFI) boot
there is no bootloader seed, so the CRNG stays unseeded for much of
boot and essentially every early randomness consumer takes this path.

Under virtualization this is costly: the hypervisor traps guest
accesses to the ID registers (HCR_EL2.TID3), making each read a vmexit,
producing ~200k trapped ID_AA64ISAR0_EL1 reads during boot.

The register value is invariant, so read the sanitised feature register
instead. read_sanitised_ftr_reg() returns the cached value from
arm64_ftr_regs[] with no sysreg access, and hence no trap. That array
is populated by cpuinfo_store_boot_cpu() in smp_prepare_boot_cpu(),
before the first early RNG use in random_init_early(), so it is always
valid here.

With this change the trapped reads drop from ~200k to handful number of
times, and the boot time drops roughly by 6.3% in the test environment.

Fixes: 2c03e16f4499 ("random: remove early archrandom abstraction")
Cc: Jason A. Donenfeld <Jason@zx2c4.com>
Cc: Ard Biesheuvel <ardb@kernel.org>
Cc: Will Deacon <will@kernel.org>
Signed-off-by: Aman Priyadarshi <amanp@apple.com>
---
 arch/arm64/include/asm/archrandom.h | 18 ++++++++++++++++--
 1 file changed, 16 insertions(+), 2 deletions(-)

diff --git a/arch/arm64/include/asm/archrandom.h b/arch/arm64/include/asm/archrandom.h
index 8babfbe31f95..8067e9a35641 100644
--- a/arch/arm64/include/asm/archrandom.h
+++ b/arch/arm64/include/asm/archrandom.h
@@ -61,8 +61,22 @@ static inline bool __arm64_rndrrs(unsigned long *v)
 
 static __always_inline bool __cpu_has_rng(void)
 {
-	if (unlikely(!system_capabilities_finalized() && !preemptible()))
-		return this_cpu_has_cap(ARM64_HAS_RNG);
+	if (unlikely(!system_capabilities_finalized() && !preemptible())) {
+		/*
+		 * Until the ARM64_HAS_RNG alternative is patched we can't use
+		 * the static-branch form, so consult the feature register
+		 * directly. Don't use this_cpu_has_cap() here: it reads
+		 * ID_AA64ISAR0_EL1 from hardware on every call, under
+		 * virtualization each ID register read traps to the hypervisor
+		 * (HCR_EL2.TID3) -- producing a storm of vmexits during boot.
+		 * The sanitised value is cached in memory.
+		 */
+		u64 isar0 = read_sanitised_ftr_reg(SYS_ID_AA64ISAR0_EL1);
+
+		return cpuid_feature_extract_unsigned_field(isar0,
+				ID_AA64ISAR0_EL1_RNDR_SHIFT) >=
+			ID_AA64ISAR0_EL1_RNDR_IMP;
+	}
 	return alternative_has_cap_unlikely(ARM64_HAS_RNG);
 }
 
-- 
2.54.0 (Apple Git-156)

Re: [PATCH] arm64: archrandom: avoid trapping ID register read in __cpu_has_rng()

[Will Deacon]

Hi Aman,

On Mon, Jul 20, 2026 at 03:06:15PM +0100, Aman Priyadarshi wrote:
> __cpu_has_rng() has an early-boot fallback, taken before the
> ARM64_HAS_RNG alternative is patched, that calls
> this_cpu_has_cap(ARM64_HAS_RNG). With SCOPE_LOCAL_CPU that resolves the
> capability by reading ID_AA64ISAR0_EL1 directly from hardware via
> __read_sysreg_by_encoding(), on every invocation.
> 
> Until the CRNG is seeded, crng_make_state() routes every get_random_*()
> through extract_entropy(), which drains architectural entropy via
> arch_get_random_seed_longs()/arch_get_random_longs() and so calls
> __cpu_has_rng() several times per request. On a direct (non-EFI) boot
> there is no bootloader seed, so the CRNG stays unseeded for much of
> boot and essentially every early randomness consumer takes this path.
> 
> Under virtualization this is costly: the hypervisor traps guest
> accesses to the ID registers (HCR_EL2.TID3), making each read a vmexit,
> producing ~200k trapped ID_AA64ISAR0_EL1 reads during boot.
> 
> The register value is invariant, so read the sanitised feature register
> instead. read_sanitised_ftr_reg() returns the cached value from
> arm64_ftr_regs[] with no sysreg access, and hence no trap. That array
> is populated by cpuinfo_store_boot_cpu() in smp_prepare_boot_cpu(),
> before the first early RNG use in random_init_early(), so it is always
> valid here.
> 
> With this change the trapped reads drop from ~200k to handful number of
> times, and the boot time drops roughly by 6.3% in the test environment.

Yikes, that's quite a compelling performance improvement.

> diff --git a/arch/arm64/include/asm/archrandom.h b/arch/arm64/include/asm/archrandom.h
> index 8babfbe31f95..8067e9a35641 100644
> --- a/arch/arm64/include/asm/archrandom.h
> +++ b/arch/arm64/include/asm/archrandom.h
> @@ -61,8 +61,22 @@ static inline bool __arm64_rndrrs(unsigned long *v)
>  
>  static __always_inline bool __cpu_has_rng(void)
>  {
> -	if (unlikely(!system_capabilities_finalized() && !preemptible()))
> -		return this_cpu_has_cap(ARM64_HAS_RNG);
> +	if (unlikely(!system_capabilities_finalized() && !preemptible())) {
> +		/*
> +		 * Until the ARM64_HAS_RNG alternative is patched we can't use
> +		 * the static-branch form, so consult the feature register
> +		 * directly. Don't use this_cpu_has_cap() here: it reads
> +		 * ID_AA64ISAR0_EL1 from hardware on every call, under
> +		 * virtualization each ID register read traps to the hypervisor
> +		 * (HCR_EL2.TID3) -- producing a storm of vmexits during boot.
> +		 * The sanitised value is cached in memory.
> +		 */
> +		u64 isar0 = read_sanitised_ftr_reg(SYS_ID_AA64ISAR0_EL1);
> +
> +		return cpuid_feature_extract_unsigned_field(isar0,
> +				ID_AA64ISAR0_EL1_RNDR_SHIFT) >=
> +			ID_AA64ISAR0_EL1_RNDR_IMP;
> +	}

You can probably rewrite this a little more cleanly along the lines of
the (not even compile-tested) diff below. I was about to do that, but
then I got a bit confused by the whole thing. The preemptible() check is
presumably not needed if we're accessing the in-memory feature registers
rather than the per-CPU id registers, but then how do you handle races
with concurrent updates to the "safe value" made by CPUs concurrently
coming online?

Will

--->8

diff --git a/arch/arm64/include/asm/archrandom.h b/arch/arm64/include/asm/archrandom.h
index 8babfbe31f95..1c6ccc1776cd 100644
--- a/arch/arm64/include/asm/archrandom.h
+++ b/arch/arm64/include/asm/archrandom.h
@@ -61,8 +61,17 @@ static inline bool __arm64_rndrrs(unsigned long *v)

 static __always_inline bool __cpu_has_rng(void)
 {
-       if (unlikely(!system_capabilities_finalized() && !preemptible()))
-               return this_cpu_has_cap(ARM64_HAS_RNG);
+       if (unlikely(!system_capabilities_finalized() && !preemptible())) {
+               /*
+                * Query the in-memory sanitised value to avoid a potential
+                * trap when accessing the ID register under a hypervisor.
+                */
+               u64 isar0 = read_sanitised_ftr_reg(SYS_ID_AA64ISAR0_EL1);
+               u64 rndr = SYS_FIELD_GET(ID_AA64ISAR0_EL1, RNDR, isar0);
+
+               return rndr >= ID_AA64ISAR0_EL1_RNDR_IMP;
+       }
+
        return alternative_has_cap_unlikely(ARM64_HAS_RNG);
 }

Re: [PATCH] arm64: archrandom: avoid trapping ID register read in __cpu_has_rng()

[Aman Priyadarshi]

Hi Will,

Thank you for reviewing the patch!

> On 31 Jul 2026, at 15:43, Will Deacon <will@kernel.org> wrote:
> 
> Hi Aman,
> 
> On Mon, Jul 20, 2026 at 03:06:15PM +0100, Aman Priyadarshi wrote:
>> __cpu_has_rng() has an early-boot fallback, taken before the
>> ARM64_HAS_RNG alternative is patched, that calls
>> this_cpu_has_cap(ARM64_HAS_RNG). With SCOPE_LOCAL_CPU that resolves the
>> capability by reading ID_AA64ISAR0_EL1 directly from hardware via
>> __read_sysreg_by_encoding(), on every invocation.
>> 
>> Until the CRNG is seeded, crng_make_state() routes every get_random_*()
>> through extract_entropy(), which drains architectural entropy via
>> arch_get_random_seed_longs()/arch_get_random_longs() and so calls
>> __cpu_has_rng() several times per request. On a direct (non-EFI) boot
>> there is no bootloader seed, so the CRNG stays unseeded for much of
>> boot and essentially every early randomness consumer takes this path.
>> 
>> Under virtualization this is costly: the hypervisor traps guest
>> accesses to the ID registers (HCR_EL2.TID3), making each read a vmexit,
>> producing ~200k trapped ID_AA64ISAR0_EL1 reads during boot.
>> 
>> The register value is invariant, so read the sanitised feature register
>> instead. read_sanitised_ftr_reg() returns the cached value from
>> arm64_ftr_regs[] with no sysreg access, and hence no trap. That array
>> is populated by cpuinfo_store_boot_cpu() in smp_prepare_boot_cpu(),
>> before the first early RNG use in random_init_early(), so it is always
>> valid here.
>> 
>> With this change the trapped reads drop from ~200k to handful number of
>> times, and the boot time drops roughly by 6.3% in the test environment.
> 
> Yikes, that's quite a compelling performance improvement.
> 
>> diff --git a/arch/arm64/include/asm/archrandom.h b/arch/arm64/include/asm/archrandom.h
>> index 8babfbe31f95..8067e9a35641 100644
>> --- a/arch/arm64/include/asm/archrandom.h
>> +++ b/arch/arm64/include/asm/archrandom.h
>> @@ -61,8 +61,22 @@ static inline bool __arm64_rndrrs(unsigned long *v)
>> 
>> static __always_inline bool __cpu_has_rng(void)
>> {
>> - if (unlikely(!system_capabilities_finalized() && !preemptible()))
>> - return this_cpu_has_cap(ARM64_HAS_RNG);
>> + if (unlikely(!system_capabilities_finalized() && !preemptible())) {
>> + /*
>> + * Until the ARM64_HAS_RNG alternative is patched we can't use
>> + * the static-branch form, so consult the feature register
>> + * directly. Don't use this_cpu_has_cap() here: it reads
>> + * ID_AA64ISAR0_EL1 from hardware on every call, under
>> + * virtualization each ID register read traps to the hypervisor
>> + * (HCR_EL2.TID3) -- producing a storm of vmexits during boot.
>> + * The sanitised value is cached in memory.
>> + */
>> + u64 isar0 = read_sanitised_ftr_reg(SYS_ID_AA64ISAR0_EL1);
>> +
>> + return cpuid_feature_extract_unsigned_field(isar0,
>> + ID_AA64ISAR0_EL1_RNDR_SHIFT) >=
>> + ID_AA64ISAR0_EL1_RNDR_IMP;
>> + }
> 
> You can probably rewrite this a little more cleanly along the lines of
> the (not even compile-tested) diff below. I was about to do that, but
> then I got a bit confused by the whole thing. The preemptible() check is
> presumably not needed if we're accessing the in-memory feature registers
> rather than the per-CPU id registers, but then how do you handle races
> with concurrent updates to the "safe value" made by CPUs concurrently
> coming online?

I kept preemptible() check for this exact reason: the updates made by CPUs
concurrently coming online will always take the downgrade path (a secondary
CPU can clear RNDR, never set it), and therefore by taking the non-preemptible
branch I can guarantee that the pinned CPU supports the said feature.
I agree, this assumes that a secondary CPU folds its own ID registers into sys_val
before it can ever be a randomness consumer, but looking at the code that seems
the case, please feel free to correct me.
Besides, in my opinion, it's hard to argue correctness of this code without
preemptible() check.

> 
> Will
> 
> --->8
> 
> diff --git a/arch/arm64/include/asm/archrandom.h b/arch/arm64/include/asm/archrandom.h
> index 8babfbe31f95..1c6ccc1776cd 100644
> --- a/arch/arm64/include/asm/archrandom.h
> +++ b/arch/arm64/include/asm/archrandom.h
> @@ -61,8 +61,17 @@ static inline bool __arm64_rndrrs(unsigned long *v)
> 
> static __always_inline bool __cpu_has_rng(void)
> {
> -       if (unlikely(!system_capabilities_finalized() && !preemptible()))
> -               return this_cpu_has_cap(ARM64_HAS_RNG);
> +       if (unlikely(!system_capabilities_finalized() && !preemptible())) {
> +               /*
> +                * Query the in-memory sanitised value to avoid a potential
> +                * trap when accessing the ID register under a hypervisor.
> +                */
> +               u64 isar0 = read_sanitised_ftr_reg(SYS_ID_AA64ISAR0_EL1);
> +               u64 rndr = SYS_FIELD_GET(ID_AA64ISAR0_EL1, RNDR, isar0);
> +
> +               return rndr >= ID_AA64ISAR0_EL1_RNDR_IMP;
> +       }
> +
>        return alternative_has_cap_unlikely(ARM64_HAS_RNG);
> }

Agreed, this looks cleaner. Thanks!

- Aman Priyadarshi

Re: [PATCH] arm64: archrandom: avoid trapping ID register read in __cpu_has_rng()

[Will Deacon]

On Fri, Jul 31, 2026 at 06:26:44PM +0100, Aman Priyadarshi wrote:
> > On 31 Jul 2026, at 15:43, Will Deacon <will@kernel.org> wrote:
> > On Mon, Jul 20, 2026 at 03:06:15PM +0100, Aman Priyadarshi wrote:
> >> diff --git a/arch/arm64/include/asm/archrandom.h b/arch/arm64/include/asm/archrandom.h
> >> index 8babfbe31f95..8067e9a35641 100644
> >> --- a/arch/arm64/include/asm/archrandom.h
> >> +++ b/arch/arm64/include/asm/archrandom.h
> >> @@ -61,8 +61,22 @@ static inline bool __arm64_rndrrs(unsigned long *v)
> >> 
> >> static __always_inline bool __cpu_has_rng(void)
> >> {
> >> - if (unlikely(!system_capabilities_finalized() && !preemptible()))
> >> - return this_cpu_has_cap(ARM64_HAS_RNG);
> >> + if (unlikely(!system_capabilities_finalized() && !preemptible())) {
> >> + /*
> >> + * Until the ARM64_HAS_RNG alternative is patched we can't use
> >> + * the static-branch form, so consult the feature register
> >> + * directly. Don't use this_cpu_has_cap() here: it reads
> >> + * ID_AA64ISAR0_EL1 from hardware on every call, under
> >> + * virtualization each ID register read traps to the hypervisor
> >> + * (HCR_EL2.TID3) -- producing a storm of vmexits during boot.
> >> + * The sanitised value is cached in memory.
> >> + */
> >> + u64 isar0 = read_sanitised_ftr_reg(SYS_ID_AA64ISAR0_EL1);
> >> +
> >> + return cpuid_feature_extract_unsigned_field(isar0,
> >> + ID_AA64ISAR0_EL1_RNDR_SHIFT) >=
> >> + ID_AA64ISAR0_EL1_RNDR_IMP;
> >> + }
> > 
> > You can probably rewrite this a little more cleanly along the lines of
> > the (not even compile-tested) diff below. I was about to do that, but
> > then I got a bit confused by the whole thing. The preemptible() check is
> > presumably not needed if we're accessing the in-memory feature registers
> > rather than the per-CPU id registers, but then how do you handle races
> > with concurrent updates to the "safe value" made by CPUs concurrently
> > coming online?
> 
> I kept preemptible() check for this exact reason: the updates made by CPUs
> concurrently coming online will always take the downgrade path (a secondary
> CPU can clear RNDR, never set it), and therefore by taking the non-preemptible
> branch I can guarantee that the pinned CPU supports the said feature.
> I agree, this assumes that a secondary CPU folds its own ID registers into sys_val
> before it can ever be a randomness consumer, but looking at the code that seems
> the case, please feel free to correct me.
> Besides, in my opinion, it's hard to argue correctness of this code without
> preemptible() check.

My point is that this change introduces a data race on 'reg->sys_val' for
the ID_AA64ISAR0_EL1 entry in the arm64_ftr_regs array.

Will

Re: [PATCH] arm64: archrandom: avoid trapping ID register read in __cpu_has_rng()

[Aman Priyadarshi]

> On 4 Aug 2026, at 15:56, Will Deacon <will@kernel.org> wrote:
> 
> On Fri, Jul 31, 2026 at 06:26:44PM +0100, Aman Priyadarshi wrote:
>>> On 31 Jul 2026, at 15:43, Will Deacon <will@kernel.org> wrote:
>>> On Mon, Jul 20, 2026 at 03:06:15PM +0100, Aman Priyadarshi wrote:
>>>> diff --git a/arch/arm64/include/asm/archrandom.h b/arch/arm64/include/asm/archrandom.h
>>>> index 8babfbe31f95..8067e9a35641 100644
>>>> --- a/arch/arm64/include/asm/archrandom.h
>>>> +++ b/arch/arm64/include/asm/archrandom.h
>>>> @@ -61,8 +61,22 @@ static inline bool __arm64_rndrrs(unsigned long *v)
>>>> 
>>>> static __always_inline bool __cpu_has_rng(void)
>>>> {
>>>> - if (unlikely(!system_capabilities_finalized() && !preemptible()))
>>>> - return this_cpu_has_cap(ARM64_HAS_RNG);
>>>> + if (unlikely(!system_capabilities_finalized() && !preemptible())) {
>>>> + /*
>>>> + * Until the ARM64_HAS_RNG alternative is patched we can't use
>>>> + * the static-branch form, so consult the feature register
>>>> + * directly. Don't use this_cpu_has_cap() here: it reads
>>>> + * ID_AA64ISAR0_EL1 from hardware on every call, under
>>>> + * virtualization each ID register read traps to the hypervisor
>>>> + * (HCR_EL2.TID3) -- producing a storm of vmexits during boot.
>>>> + * The sanitised value is cached in memory.
>>>> + */
>>>> + u64 isar0 = read_sanitised_ftr_reg(SYS_ID_AA64ISAR0_EL1);
>>>> +
>>>> + return cpuid_feature_extract_unsigned_field(isar0,
>>>> + ID_AA64ISAR0_EL1_RNDR_SHIFT) >=
>>>> + ID_AA64ISAR0_EL1_RNDR_IMP;
>>>> + }
>>> 
>>> You can probably rewrite this a little more cleanly along the lines of
>>> the (not even compile-tested) diff below. I was about to do that, but
>>> then I got a bit confused by the whole thing. The preemptible() check is
>>> presumably not needed if we're accessing the in-memory feature registers
>>> rather than the per-CPU id registers, but then how do you handle races
>>> with concurrent updates to the "safe value" made by CPUs concurrently
>>> coming online?
>> 
>> I kept preemptible() check for this exact reason: the updates made by CPUs
>> concurrently coming online will always take the downgrade path (a secondary
>> CPU can clear RNDR, never set it), and therefore by taking the non-preemptible
>> branch I can guarantee that the pinned CPU supports the said feature.
>> I agree, this assumes that a secondary CPU folds its own ID registers into sys_val
>> before it can ever be a randomness consumer, but looking at the code that seems
>> the case, please feel free to correct me.
>> Besides, in my opinion, it's hard to argue correctness of this code without
>> preemptible() check.
> 
> My point is that this change introduces a data race on 'reg->sys_val' for
> the ID_AA64ISAR0_EL1 entry in the arm64_ftr_regs array.
> 
> Will

Agreed, you're right. My reasoning was that existing read_sanitised_ftr_reg() callers
already race with sys_val updates during hotplug CPU bringup, so this wasn't a
new problem. But I agree it's not much of a defence.
It looks like an easy fix, though: mark the reader and the writer. What do you think
of the patch below? I'm happy to post it as a separate patch ahead of this fix once
you confirm it works for you.

- Aman Priyadarshi

--->8

diff --git a/arch/arm64/kernel/cpufeature.c b/arch/arm64/kernel/cpufeature.c
index 9a22df0c5120..e1c10a23da3c 100644
--- a/arch/arm64/kernel/cpufeature.c
+++ b/arch/arm64/kernel/cpufeature.c
@@ -1235,18 +1235,20 @@ void __init init_cpu_features(struct cpuinfo_arm64 *info)
 static void update_cpu_ftr_reg(struct arm64_ftr_reg *reg, u64 new)
 {
        const struct arm64_ftr_bits *ftrp;
+       u64 sys_val = reg->sys_val;
 
        for (ftrp = reg->ftr_bits; ftrp->width; ftrp++) {
-               s64 ftr_cur = arm64_ftr_value(ftrp, reg->sys_val);
+               s64 ftr_cur = arm64_ftr_value(ftrp, sys_val);
                s64 ftr_new = arm64_ftr_value(ftrp, new);
 
                if (ftr_cur == ftr_new)
                        continue;
                /* Find a safe value */
                ftr_new = arm64_ftr_safe_value(ftrp, ftr_new, ftr_cur);
-               reg->sys_val = arm64_ftr_set_value(ftrp, reg->sys_val, ftr_new);
+               sys_val = arm64_ftr_set_value(ftrp, sys_val, ftr_new);
        }
 
+       WRITE_ONCE(reg->sys_val, sys_val);
 }
 
 static int check_update_ftr_reg(u32 sys_id, int cpu, u64 val, u64 boot)
@@ -1526,7 +1528,8 @@ u64 read_sanitised_ftr_reg(u32 id)
 
        if (!regp)
                return 0;
-       return regp->sys_val;
+
+       return READ_ONCE(regp->sys_val);
 }
 EXPORT_SYMBOL_GPL(read_sanitised_ftr_reg);

Re: [PATCH] arm64: archrandom: avoid trapping ID register read in __cpu_has_rng()

[Will Deacon]

On Tue, Aug 04, 2026 at 04:50:35PM +0100, Aman Priyadarshi wrote:
> > On 4 Aug 2026, at 15:56, Will Deacon <will@kernel.org> wrote:
> > On Fri, Jul 31, 2026 at 06:26:44PM +0100, Aman Priyadarshi wrote:
> >>> On 31 Jul 2026, at 15:43, Will Deacon <will@kernel.org> wrote:
> >>> On Mon, Jul 20, 2026 at 03:06:15PM +0100, Aman Priyadarshi wrote:
> >>>> diff --git a/arch/arm64/include/asm/archrandom.h b/arch/arm64/include/asm/archrandom.h
> >>>> index 8babfbe31f95..8067e9a35641 100644
> >>>> --- a/arch/arm64/include/asm/archrandom.h
> >>>> +++ b/arch/arm64/include/asm/archrandom.h
> >>>> @@ -61,8 +61,22 @@ static inline bool __arm64_rndrrs(unsigned long *v)
> >>>> 
> >>>> static __always_inline bool __cpu_has_rng(void)
> >>>> {
> >>>> - if (unlikely(!system_capabilities_finalized() && !preemptible()))
> >>>> - return this_cpu_has_cap(ARM64_HAS_RNG);
> >>>> + if (unlikely(!system_capabilities_finalized() && !preemptible())) {
> >>>> + /*
> >>>> + * Until the ARM64_HAS_RNG alternative is patched we can't use
> >>>> + * the static-branch form, so consult the feature register
> >>>> + * directly. Don't use this_cpu_has_cap() here: it reads
> >>>> + * ID_AA64ISAR0_EL1 from hardware on every call, under
> >>>> + * virtualization each ID register read traps to the hypervisor
> >>>> + * (HCR_EL2.TID3) -- producing a storm of vmexits during boot.
> >>>> + * The sanitised value is cached in memory.
> >>>> + */
> >>>> + u64 isar0 = read_sanitised_ftr_reg(SYS_ID_AA64ISAR0_EL1);
> >>>> +
> >>>> + return cpuid_feature_extract_unsigned_field(isar0,
> >>>> + ID_AA64ISAR0_EL1_RNDR_SHIFT) >=
> >>>> + ID_AA64ISAR0_EL1_RNDR_IMP;
> >>>> + }
> >>> 
> >>> You can probably rewrite this a little more cleanly along the lines of
> >>> the (not even compile-tested) diff below. I was about to do that, but
> >>> then I got a bit confused by the whole thing. The preemptible() check is
> >>> presumably not needed if we're accessing the in-memory feature registers
> >>> rather than the per-CPU id registers, but then how do you handle races
> >>> with concurrent updates to the "safe value" made by CPUs concurrently
> >>> coming online?
> >> 
> >> I kept preemptible() check for this exact reason: the updates made by CPUs
> >> concurrently coming online will always take the downgrade path (a secondary
> >> CPU can clear RNDR, never set it), and therefore by taking the non-preemptible
> >> branch I can guarantee that the pinned CPU supports the said feature.
> >> I agree, this assumes that a secondary CPU folds its own ID registers into sys_val
> >> before it can ever be a randomness consumer, but looking at the code that seems
> >> the case, please feel free to correct me.
> >> Besides, in my opinion, it's hard to argue correctness of this code without
> >> preemptible() check.
> > 
> > My point is that this change introduces a data race on 'reg->sys_val' for
> > the ID_AA64ISAR0_EL1 entry in the arm64_ftr_regs array.
> 
> Agreed, you're right. My reasoning was that existing read_sanitised_ftr_reg() callers
> already race with sys_val updates during hotplug CPU bringup, so this wasn't a
> new problem. But I agree it's not much of a defence.

I think system_capabilities_finalized() is supposed to handle that race:

  * If the capabilities are not finalised, you shouldn't query the sanitised
    feature registers and they will be updated by CPUs coming online.

  * If the capabailities are finalised, you can query the sanitised feature
    registers and CPUs coming online must be compatible with them.

That's not to say we're bug-free here, though. The hw_breakpoint code
queries the sanitised view of SYS_ID_AA64DFR0_EL1 before it should and
you're trying to add another caller in the random code.

> It looks like an easy fix, though: mark the reader and the writer. What do you think
> of the patch below? I'm happy to post it as a separate patch ahead of this fix once
> you confirm it works for you.

I don't think we should try to make this thread safe. It would be much
better if this_cpu_has_cap() could query the saved id registers in
this_cpu_ptr(&cpu_data) instead of reading the id registers again.

Something along the lines of the untested and incomplete diff below...

Will

--->8

diff --git a/arch/arm64/include/asm/cpufeature.h b/arch/arm64/include/asm/cpufeature.h
index a57870fa96db..b72e9e22ca79 100644
--- a/arch/arm64/include/asm/cpufeature.h
+++ b/arch/arm64/include/asm/cpufeature.h
@@ -641,7 +641,6 @@ void __init setup_user_features(void);
 void check_local_cpu_capabilities(void);
 
 u64 read_sanitised_ftr_reg(u32 id);
-u64 __read_sysreg_by_encoding(u32 sys_id);
 
 static inline bool cpu_supports_mixed_endian_el0(void)
 {
diff --git a/arch/arm64/kernel/cpufeature.c b/arch/arm64/kernel/cpufeature.c
index 9a22df0c5120..6462863fb78b 100644
--- a/arch/arm64/kernel/cpufeature.c
+++ b/arch/arm64/kernel/cpufeature.c
@@ -1533,11 +1533,7 @@ EXPORT_SYMBOL_GPL(read_sanitised_ftr_reg);
 #define read_sysreg_case(r)    \
        case r:         val = read_sysreg_s(r); break;
 
-/*
- * __read_sysreg_by_encoding() - Used by a STARTING cpu before cpuinfo is populated.
- * Read the system register on the current CPU
- */
-u64 __read_sysreg_by_encoding(u32 sys_id)
+static u64 __read_sysreg_by_encoding(struct cpuinfo_arm64 *info, u32 sys_id)
 {
        struct arm64_ftr_reg *regp;
        u64 val;
@@ -1578,7 +1574,9 @@ u64 __read_sysreg_by_encoding(u32 sys_id)
        read_sysreg_case(SYS_ID_AA64MMFR2_EL1);
        read_sysreg_case(SYS_ID_AA64MMFR3_EL1);
        read_sysreg_case(SYS_ID_AA64MMFR4_EL1);
-       read_sysreg_case(SYS_ID_AA64ISAR0_EL1);
+       case SYS_ID_AA64ISAR0_EL1:
+               val = info->reg_id_aa64isar0;
+               break;
        read_sysreg_case(SYS_ID_AA64ISAR1_EL1);
        read_sysreg_case(SYS_ID_AA64ISAR2_EL1);
        read_sysreg_case(SYS_ID_AA64ISAR3_EL1);
@@ -1639,11 +1637,19 @@ feature_matches(u64 reg, const struct arm64_cpu_capabilities *entry)
 static u64
 read_scoped_sysreg(const struct arm64_cpu_capabilities *entry, int scope)
 {
-       WARN_ON(scope == SCOPE_LOCAL_CPU && preemptible());
-       if (scope == SCOPE_SYSTEM)
+       switch (scope) {
+       case SCOPE_SYSTEM:
                return read_sanitised_ftr_reg(entry->sys_reg);
-       else
-               return __read_sysreg_by_encoding(entry->sys_reg);
+       case SCOPE_LOCAL_CPU:
+               WARN_ON(preemptible());
+               fallthrough;
+       case SCOPE_BOOT_CPU:
+               return __read_sysreg_by_encoding(this_cpu_ptr(&cpu_data),
+                                                entry->sys_reg);
+       default:
+               BUG();
+               return 0;
+       }
 }
 
 static bool
@@ -2232,7 +2238,7 @@ static bool has_address_auth_cpucap(const struct arm64_cpu_capabilities *entry,
        if (scope & SCOPE_BOOT_CPU)
                return boot_val >= entry->min_field_value;
        /* Now check for the secondary CPUs with SCOPE_LOCAL_CPU scope */
-       sec_val = cpuid_feature_extract_field(__read_sysreg_by_encoding(entry->sys_reg),
+       sec_val = cpuid_feature_extract_field(read_scoped_sysreg(entry, SCOPE_LOCAL_CPU),
                                              entry->field_pos, entry->sign);
        return (sec_val >= entry->min_field_value) && (sec_val == boot_val);
 }