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

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Writes to control-plane system registers (SYS_NVHCR_EL2 and SYS_VNCR_EL2) are missing the architecturally required `isb()` synchronization barrier.
--

--- Patch [26]: [PATCH v2 26/28] KVM: arm64: Add FEAT_NV3 detection ---
commit 1e08d22547f3313a77b075620f905436357c4b40
Author: Marc Zyngier <maz@kernel.org>

KVM: arm64: Add FEAT_NV3 detection
    
Now that everything is in place to engage the FEAT_NV3 fast-path,
add the detection code to cpufeature.c.

> 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)
>  	},
> +	{
> +		.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]
This isn't a bug introduced by this patch, but enabling FEAT_NV3 here exposes
pre-existing paths where control-plane system registers are written without
the architecturally required immediate isb() synchronization barrier.

For example, in 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);

and a bit later in the same function:

		write_sysreg_s(vcpu->arch.ctxt.vncr_array, SYS_VNCR_EL2);
	} else {

Also in arch/arm64/kvm/hyp/include/hyp/sysreg-sr.h:__sysreg_restore_el2_return_state():

	if (is_nested_nv3_ctxt(ctxt_to_vcpu(ctxt)))
		write_sysreg_s(ctxt_sys_reg(ctxt, NVHCR_EL2), SYS_NVHCR_EL2);
}

None of these writes are immediately followed by an isb(). Since they are
control-plane system registers, execution continues to evaluate masks or
return from the function under an architecturally undefined pipeline state.

Should there be an isb() immediately following these writes to
SYS_NVHCR_EL2 and SYS_VNCR_EL2?

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

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