Re: [PATCH v4 11/29] KVM: arm64: Expose FEAT_NV2p1 to NV guests

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Missing context synchronization event (`isb()`) after writing to a control-plane system register (`CNTKCTL_EL1` / `CNTHCTL_EL2`).
--

--- Patch [11]: [PATCH v4 11/29] KVM: arm64: Expose FEAT_NV2p1 to NV guests ---
commit 5b82c2dde5c66c45b12e75de220aab8518c99896
Author: Marc Zyngier <[email protected]>

KVM: arm64: Expose FEAT_NV2p1 to NV guests

This commit exposes FEAT_NV2p1 to nested virtualization guests. By reducing
the number of traps and updating timer configurations, it brings valuable
performance improvements to NV guests.

> 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: High]
Does this write to a control-plane system register require an isb() before
returning?

The ARM64 architecture generally requires an isb() as the very next
instruction after writing to control-plane registers like cnthctl_el2
and cntkctl_el1 to ensure subsequent instructions do not execute with
a stale configuration.

Because sysreg_clear_set() expands to a hardware write without an isb(),
timer_set_traps() returns immediately after the write. Could the CPU execute
subsequent instructions in the caller with stale pipeline state before a
context synchronization event occurs?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=11