Re: [PATCH v3 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] KVM's FEAT_NV3 fast-path evaluates to false for all guests, causing stale SYS_NVHCR_EL2 state during L2 nested entry.
- [High] Writes to SYS_NVHCR_EL2 lack required immediate Context Synchronization Event (ISB).
--

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

This commit adds the FEAT_NV3 detection code to cpufeature.c by introducing
the ARM64_HAS_NV3 capability. It engages the FEAT_NV3 fast-path in KVM now
that the infrastructure is in place.

> 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,

[Severity: High]
Could engaging this capability break nested virtualization on NV3 hardware?

By enabling ARM64_HAS_NV3, the fast-path checks now depend on kvm_has_nv3(),
which appears to require the guest to advertise NV3:

arch/arm64/include/asm/kvm_emulate.h:kvm_has_nv3() {
    return (cpus_have_final_cap(ARM64_HAS_NESTED_VIRT) &&
            cpus_have_final_cap(ARM64_HAS_NV3) &&
            kvm_has_feat(kvm, ID_AA64MMFR4_EL1, NV_frac, NV3));
}

Since NV3 is explicitly hidden from guests (ID_AA64MMFR4_EL1.NV_frac is capped
at NV2P1), won't kvm_has_feat() always evaluate to false?

This would cause is_nested_nv3_ctxt() to always return false, skipping the
SYS_NVHCR_EL2 restore in L2 nested entry:

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);
    ...
}

Will the physical CPU execute the L2 guest using an uninitialized or stale
SYS_NVHCR_EL2 value?

[Severity: High]
Are there missing context synchronization events in this new fast-path?

When ARM64_HAS_NV3 is enabled, the code writes to the control-plane system
register 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));

    /* No ISB here */
    guest_hcr = kvm_vcpu_apply_reg_masks(vcpu, HCR_EL2, 0);
    ...
}

It also happens during state restore 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);
}
/* No ISB before return */

Since SYS_NVHCR_EL2 is a control-plane register, doesn't it require an isb()
as the very next instruction to prevent the CPU from executing subsequent
instructions with undefined pipeline state?

> +		.type = ARM64_CPUCAP_SYSTEM_FEATURE,
> +		.matches = has_cpuid_feature,
> +		ARM64_CPUID_FIELDS(ID_AA64MMFR4_EL1, NV_frac, NV3)
> +	},
>  	{
>  		.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=26