Re: [PATCH] KVM: arm64: Fix hvhe and broken CNTVOFF_EL2

On Thu, 6 Aug 2026 at 16:32, <[email protected]> wrote:
>
> Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
>
> New issues:
> - [High] Virtual timer traps are inadvertently disabled for non-protected guests under pKVM on hardware with broken CNTVOFF_EL2, breaking their configured timer offsets.
>
> Pre-existing issues:
> - [High] `__timer_enable_traps()` uses the global `is_protected_kvm_enabled()` check and an unsynced `poffset`, unconditionally disabling physical timer traps for non-protected VMs under pKVM.

This only runs on broken-CNTVOFF hardware, where the workaround pins
CNTVOFF_EL2=0 for all guests. Dropping the trap changes nothing
observable.


/fuad

> --
>
> --- Patch [1]: [PATCH] KVM: arm64: Fix hvhe and broken CNTVOFF_EL2 ---
> commit d5e405a74fba9d73744f97970471012a4473ce3c
> Author: Mostafa Saleh <[email protected]>
> Subject: KVM: arm64: Fix hvhe and broken CNTVOFF_EL2
>
> The commit fixes a guest kernel panic when launching a protected VM with
> protected hvhe mode on hardware with a broken CNTVOFF_EL2. It achieves
> this by conditionally enabling virtual timer traps only when the guest
> actually has a non-zero virtual timer offset.
>
> > diff --git a/arch/arm64/kvm/hyp/nvhe/timer-sr.c b/arch/arm64/kvm/hyp/nvhe/timer-sr.c
> > index ff176f4ce7deb..98b6e37ee8fa4 100644
> > --- a/arch/arm64/kvm/hyp/nvhe/timer-sr.c
> > +++ b/arch/arm64/kvm/hyp/nvhe/timer-sr.c
> [ ... ]
>
> [Severity: High]
> This is a pre-existing issue, but does the physical timer trap logic earlier
> in __timer_enable_traps() improperly grant all guests under pKVM access to
> the physical timer?
>
> Looking at the physical timer access check in this function:
>
>         if (is_protected_kvm_enabled() ||
>             !kern_hyp_va(vcpu->kvm)->arch.timer_data.poffset)
>                 set |= CNTHCTL_EL1PCTEN;
>
> The is_protected_kvm_enabled() static key reflects host pKVM support and
> applies to all guests. Additionally, the fallback poffset check evaluates to
> true under pKVM because the EL2-private hyp_vm->kvm is never populated with
> the host's poffset during init_pkvm_hyp_vm().
>
> Can this cause non-protected VMs on older hardware without CNTPOFF_EL2 to
> bypass physical timer offset emulation and read the raw physical timer
> directly?
>
> > @@ -63,7 +64,7 @@ void __timer_enable_traps(struct kvm_vcpu *vcpu)
> >        * Trap the virtual counter/timer if we have a broken cntvoff
> >        * implementation.
> >        */
> > -     if (has_broken_cntvoff())
> > +     if (has_broken_cntvoff() && hyp_timer_get_offset(vcpu_vtimer(vcpu)))
>
> [Severity: High]
> Could this condition inadvertently disable virtual timer traps for
> non-protected guests under pKVM?
>
> The new check evaluates hyp_timer_get_offset(vcpu_vtimer(vcpu)). However,
> under pKVM (nVHE), the EL2-private pkvm_hyp_vcpu structure's timer_cpu state
> is never synchronized from the host vCPU during init_pkvm_hyp_vcpu() or
> flush_hyp_vcpu().
>
> Since ctxt->offset.vm_offset remains NULL, hyp_timer_get_offset() will
> unconditionally return 0. Will this allow any non-protected guest with a
> configured virtual timer offset to bypass the trap and incorrectly observe an
> offset of 0?
>
> >               set |= CNTHCTL_EL1TVT | CNTHCTL_EL1TVCT;
> >
> >       sysreg_clear_set(cnthctl_el2, clr, set);
>
> --
> Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=1
>