Re: [PATCH v3 12/28] arm64: Add FEAT_NV2p1 detection

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Missing isb() immediately after writing to control-plane system registers.
--

--- Patch [12]: [PATCH v3 12/28] arm64: Add FEAT_NV2p1 detection ---
commit 5488b94ad37f884fedfaca076980f395c30a72bf
Author: Marc Zyngier <maz@kernel.org>

arm64: Add FEAT_NV2p1 detection
    
This commit adds the necessary NV2p1 probing to the cpufeature
infrastructure.

> diff --git a/arch/arm64/kernel/cpufeature.c b/arch/arm64/kernel/cpufeature.c
> index 9a22df0c5120f..c9c124b0ccc8e 100644
> --- a/arch/arm64/kernel/cpufeature.c
> +++ b/arch/arm64/kernel/cpufeature.c

[ ... ]

> @@ -2620,6 +2620,13 @@ static const struct arm64_cpu_capabilities arm64_features[] = {
>  			{ /* Sentinel */ }
>  		},
>  	},
> +	{
> +		.desc = "FEAT_NV2p1",
> +		.capability = ARM64_HAS_NV2P1,
> +		.type = ARM64_CPUCAP_SYSTEM_FEATURE,
> +		.matches = has_cpuid_feature,
> +		ARM64_CPUID_FIELDS(ID_AA64MMFR4_EL1, NV_frac, NV2P1)
> +	},

[Severity: High]
Are we missing an isb() after writing to control-plane system registers when
this capability is enabled?

Looking at arch/arm64/kvm/arch_timer.c in timer_set_traps(), there is a
missing isb() after the system register writes:

arch/arm64/kvm/arch_timer.c:timer_set_traps() {
    ...
	if (!cpus_have_final_cap(ARM64_HAS_NV2P1))
		sysreg_clear_set(cnthctl_el2, clr, set);
	else
		sysreg_clear_set(cntkctl_el1, clr, set);
}

The sysreg_clear_set macro evaluates to a conditional write but lacks an
immediate isb(), and the function returns shortly after.

Similarly, in arch/arm64/kvm/sys_regs.c, are we missing synchronization before
the break?

arch/arm64/kvm/sys_regs.c:vcpu_write_sys_reg() {
    ...
		case CNTHCTL_EL2:
			write_sysreg_el1(val, SYS_CNTKCTL);
			break;
		case CPTR_EL2:
			write_sysreg_el1(val, SYS_CPACR);
			break;
    ...
}

The writes to SYS_CPACR and SYS_CNTKCTL are followed by a break and a
subsequent function return, rather than an immediate isb().

Without immediate synchronization, the CPU might operate under a stale
configuration for several cycles, which could cause unexpected traps or break
memory safety semantics.

>  	{
>  		.capability = ARM64_HAS_32BIT_EL0_DO_NOT_USE,
>  		.type = ARM64_CPUCAP_SYSTEM_FEATURE,

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260722074234.3330999-1-maz@kernel.org?part=12