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

[Marc Zyngier <maz@kernel.org>]

On Wed, 22 Jul 2026 09:53:31 +0100,
sashiko-bot@kernel.org wrote:
> 
> 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?

Yes. The enablement is in the following patch.

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

No. This simply results in the NV2 behaviour.

> 
> [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?

Nonsense.

	M.

-- 
Without deviation from the norm, progress is not possible.