Re: [PATCH 26/28] KVM: arm64: Add FEAT_NV3 detection

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Writes to the SYS_NVHCR_EL2 control system register are not followed by an immediate isb().
- [High] Writes to CNTKCTL_EL1 and CNTHCTL_EL2 control registers using sysreg_clear_set() lack an immediate isb().
--

commit b72de4ae5666d0aaa35b94915af5eab1152df83b
Author: Marc Zyngier <maz@kernel.org>
KVM: arm64: Add FEAT_NV3 detection

This commit adds the detection code for FEAT_NV3 to the cpufeature
capabilities. This engages the FEAT_NV3 fast-path when the hardware
supports it.

> diff --git a/arch/arm64/kernel/cpufeature.c b/arch/arm64/kernel/cpufeature.c
> index c9c124b0ccc8e..6ae1c816e2010 100644
> --- a/arch/arm64/kernel/cpufeature.c
> +++ b/arch/arm64/kernel/cpufeature.c
> @@ -2627,6 +2627,13 @@ static const struct arm64_cpu_capabilities arm64_features[] = {
>  		.matches = has_cpuid_feature,
>  		ARM64_CPUID_FIELDS(ID_AA64MMFR4_EL1, NV_frac, NV2P1)
>  	},

[Severity: High]
Could the system operate under a stale configuration when updating timer
control registers based on the ARM64_HAS_NV2P1 capability?

In arch/arm64/kvm/arch_timer.c:timer_set_traps(), the control registers
are modified without an immediate isb() following the update:

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 expands to a read-modify-write sequence that
does not include an isb() as the very next instruction. Is an explicit
barrier required here to prevent undefined pipeline state?

> +	{
> +		.desc = "FEAT_NV3",
> +		.capability = ARM64_HAS_NV3,
> +		.type = ARM64_CPUCAP_SYSTEM_FEATURE,
> +		.matches = has_cpuid_feature,
> +		ARM64_CPUID_FIELDS(ID_AA64MMFR4_EL1, NV_frac, NV3)
> +	},

[Severity: High]
Does enabling the ARM64_HAS_NV3 capability expose missing synchronization
when writing the SYS_NVHCR_EL2 control register?

Looking at __compute_hcr() in arch/arm64/kvm/hyp/vhe/switch.c, the guest
view is published to SYS_NVHCR_EL2 without an immediate isb():

arch/arm64/kvm/hyp/vhe/switch.c:__compute_hcr()
		if (cpus_have_final_cap(ARM64_HAS_NV3) && vcpu_el2_e2h_is_set(vcpu))
			write_sysreg_s(__vcpu_sys_reg(vcpu, HCR_EL2), SYS_NVHCR_EL2);
		else
			__vcpu_assign_sys_reg(vcpu, NVHCR_EL2, __vcpu_sys_reg(vcpu, HCR_EL2));

Similarly, the L2 view of HCR_EL2 is restored without an immediate isb()
before returning:

arch/arm64/kvm/hyp/include/hyp/sysreg-sr.h:__sysreg_restore_el1_state()
	if (is_nested_nv3_ctxt(ctxt_to_vcpu(ctxt)))
		write_sysreg_s(ctxt_sys_reg(ctxt, NVHCR_EL2), SYS_NVHCR_EL2);
}

Could the CPU operate under a stale configuration for several cycles,
leading to unpredictable behavior or unexpected traps?

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

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