Re: [PATCH v2 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:
- [Medium] Missing immediate Context Synchronization Event (isb) after writing to control-plane system registers cnthctl_el2 and cntkctl_el1.
--

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

KVM: arm64: Relax CNTHCTL_EL2 handling when FEAT_NV2p1 is present

This commit relaxes the handling of CNTHCTL_EL2 when FEAT_NV2p1 is present.
It removes the need for a split approach to access bits and instead relies
on the normal CNTKCTL_EL1 accessor.

> diff --git a/arch/arm64/kvm/arch_timer.c b/arch/arm64/kvm/arch_timer.c
> index 4155fe89b58a1..db60facad9f3c 100644
> --- a/arch/arm64/kvm/arch_timer.c
> +++ b/arch/arm64/kvm/arch_timer.c
> @@ -876,8 +876,14 @@ static void timer_set_traps(struct kvm_vcpu *vcpu, struct timer_map *map)
>  	assign_clear_set_bit(tvt02, CNTHCTL_EL1NVVCT, clr, set);
>  	assign_clear_set_bit(tpt02, CNTHCTL_EL1NVPCT, clr, set);
>  
> -	/* This only happens on VHE, so use the CNTHCTL_EL2 accessor. */
> -	sysreg_clear_set(cnthctl_el2, clr, set);
> +	/*
> +	 * This only happens on VHE, so use the CNTHCTL_EL2 accessor, unless
> +	 * we are sure CNTKCTL_EL1 is completely stateful with FEAT_NV2p1.
> +	 */
> +	if (!cpus_have_final_cap(ARM64_HAS_NV2P1))
> +		sysreg_clear_set(cnthctl_el2, clr, set);
> +	else
> +		sysreg_clear_set(cntkctl_el1, clr, set);

[Severity: Medium]
Does this code need an immediate context synchronization event after writing
to these control-plane system registers?

Modifying cntkctl_el1 (and cnthctl_el2) via sysreg_clear_set() expands to a
conditional write_sysreg() without an immediate isb() barrier. The function
returns immediately after, which may allow intervening instructions to execute
with architecturally undefined pipeline state before a context synchronization
event occurs.

This path is reachable during normal VM execution when loading vCPU timer
state. While the cnthctl_el2 write is pre-existing, the cntkctl_el1 write
was newly introduced here, which could introduce a regression.

>  }
>  
>  void kvm_timer_vcpu_load(struct kvm_vcpu *vcpu)

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