Re: [PATCH v2 2/3] KVM: arm64: Fix timer offsets for non-protected VMs

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

Pre-existing issues:
- [High] Missing Context Synchronization Event (`isb()`) after writing to control-plane system register `cnthctl_el2`.
--

--- Patch [2]: [PATCH v2 2/3] KVM: arm64: Fix timer offsets for non-protected VMs ---
commit 0ed46cb1bdd2dc0d545a0cd2b0b6f500a46a254f
Author: Mostafa Saleh <[email protected]>

KVM: arm64: Fix timer offsets for non-protected VMs

This commit fixes an issue where timer offsets for non-protected guests
were not correctly applied in pKVM. It corrects the protected VM check
in __timer_enable_traps() and initializes the previously null timer offsets.

> diff --git a/arch/arm64/kvm/hyp/nvhe/timer-sr.c b/arch/arm64/kvm/hyp/nvhe/timer-sr.c
> index ff176f4ce7deb..51b4f5010b660 100644
> --- a/arch/arm64/kvm/hyp/nvhe/timer-sr.c
> +++ b/arch/arm64/kvm/hyp/nvhe/timer-sr.c
> @@ -45,11 +45,11 @@ void __timer_enable_traps(struct kvm_vcpu *vcpu)
[ ... ]
>  	clr = CNTHCTL_EL1PCEN;
> -	if (is_protected_kvm_enabled() ||
> -	    !kern_hyp_va(vcpu->kvm)->arch.timer_data.poffset)
> +	if (vcpu_is_protected(vcpu) ||
> +	    !timer_get_offset(vcpu_ptimer(vcpu)))
>  		set |= CNTHCTL_EL1PCTEN;
>  	else
>  		clr |= CNTHCTL_EL1PCTEN;

[Severity: High]
This is a pre-existing issue, but do __timer_enable_traps() and
__timer_disable_traps() in arch/arm64/kvm/hyp/nvhe/timer-sr.c need an
explicit isb() after writing to cnthctl_el2?

Looking at the end of __timer_enable_traps(), it updates cnthctl_el2 and
returns:

arch/arm64/kvm/hyp/nvhe/timer-sr.c:__timer_enable_traps() {
    ...
    sysreg_clear_set(cnthctl_el2, clr, set);
}

The same pattern exists in __timer_disable_traps().

Because sysreg_clear_set() does not embed a Context Synchronization Event,
the CPU executes the subsequent RET instruction (and any caller instructions
before an eventual isb() or ERET) against an architecturally undefined
pipeline state.

Could this missing immediate context synchronization event after a
control-plane system register write lead to unexpected traps or memory
safety violations?

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