Re: [PATCH v3 09/12] arm64: cpufeature: Ensure atomic updates to system_cpucaps bitmap

[Jinjie Ruan <ruanjinjie@huawei.com>]

On 7/7/2026 1:40 AM, Will Deacon wrote:
> On Wed, Jun 24, 2026 at 05:25:34PM +0800, Jinjie Ruan wrote:
>> Parallel CPU bringup allows multiple secondary CPUs to concurrently
>> execute update_cpu_capabilities() during early boot.
>>
>> The current non-atomic __set_bit() and __clear_bit() helpers perform
>> unserialized updates on the shared global bitmap, risking data races
>> and feature flag erasure.
>>
>> Upgrade these operations to set_bit() and clear_bit() to ensure all
>> concurrent modifications are properly serialized via arm64 atomics.
>>
>> Signed-off-by: Jinjie Ruan <ruanjinjie@huawei.com>
>> ---
>>  arch/arm64/kernel/cpufeature.c | 4 ++--
>>  1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/arch/arm64/kernel/cpufeature.c b/arch/arm64/kernel/cpufeature.c
>> index be75e60d56ca..a1a13f3e01ed 100644
>> --- a/arch/arm64/kernel/cpufeature.c
>> +++ b/arch/arm64/kernel/cpufeature.c
>> @@ -3548,7 +3548,7 @@ static void update_cpu_capabilities(u16 scope_mask)
>>  
>>  		if (!caps->matches(caps, cpucap_default_scope(caps))) {
>>  			if (match_all)
>> -				__clear_bit(caps->capability, system_cpucaps);
>> +				clear_bit(caps->capability, system_cpucaps);
>>  			continue;
>>  		}
>>  
>> @@ -3559,7 +3559,7 @@ static void update_cpu_capabilities(u16 scope_mask)
>>  		if (!match_all && caps->desc && !caps->cpus)
>>  			pr_info("detected: %s\n", caps->desc);
>>  
>> -		__set_bit(caps->capability, system_cpucaps);
>> +		set_bit(caps->capability, system_cpucaps);
>>  
>>  		if (boot_cpu && (caps->type & SCOPE_BOOT_CPU))
>>  			set_bit(caps->capability, boot_cpucaps);
> 
> I don't think this is sufficient. Even if we use atomic updates for the
> low-level bits, the logic here which tries to avoid re-probing features
> that have already been detected isn't going to work correctly if it's
> running concurrently with itself.
> 
> I think the best bet is probably to move update_cpu_capabilities() out
> of check_local_cpu_capabilities() and call it after cpuhp_ap_sync_alive()
> when the system capabilities are not yet finalised. WDYT? That means

Hi Will,

I have also considered this direction, and I fully agree that it is a
much more robust solution than merely making the bit operations atomic.

- Concurrency is eliminated completely: After moving
update_cpu_capabilities() to after cpuhp_ap_sync_alive(), the subsequent
CPU state transitions (e.g., from the AP synchronization phase to the
final online state) are strictly serialised by the cpuhp state machine.
This means system_cpucaps will no longer be modified concurrently. The
original race condition simply disappears.

- No dead-wait on the boot CPU: I also verified that
update_cpu_capabilities() never calls cpu_die_early() — it only updates
system-wide capability bitmaps and does not trigger any error path that
would cause a secondary CPU to fail or die. Therefore, moving its
invocation to a later point (after the sync point) will not introduce
any risk of the boot CPU waiting indefinitely for a secondary CPU that
might have panicked or got stuck.

Thus, I think your suggestion is the correct way forward.

Best regards,
Jinjie

> we'd have something akin to setup_boot_cpu_features() for the secondary
> CPUs.
> 
> Will