Re: [PATCH v7 8/8] KVM: arm64: Implement lazy vCPU state sync for non-protected guests

Fuad Tabba <[email protected]> Wed, 29 Jul 2026 14:46:47 +0100
Newsgroups dev.linux.lists.sashiko-reviews,dev.linux.lists.kvmarm
Message-ID <CA+EHjTxCuAgBdRkACw26=F2-k98BrDxzwaLyhyT5uGN+-4-ZKQ@mail.gmail.com>
On Wed, 29 Jul 2026 at 14:36, <[email protected]> wrote:
>
> Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
> - [High] Missing HCR_EL2 state synchronization breaks virtual SError injection, causing an infinite exception loop.
> - [High] Lazy state sync fails to flush context after KVM_REQ_VCPU_RESET, causing vCPU resets (e.g., PSCI CPU_ON) to be ignored.

I think this covers it:

https://lore.kernel.org/all/CA+EHjTxiyUSJqRcadSCOmR+tELSsBiGaTYH9xxXh-g-xyuHZdA@mail.gmail.com/

/fuad

> --
>
> --- Patch [8]: [PATCH v7 8/8] KVM: arm64: Implement lazy vCPU state sync for non-protected guests ---
> commit 80df5c4ca2cca0a1726813fa129b353353482892
> Author: Fuad Tabba <[email protected]>
>
> KVM: arm64: Implement lazy vCPU state sync for non-protected guests
>
> This commit optimizes world switches for non-protected guests by deferring
> the copy of the vCPU register context between the host and hypervisor.
> State is now only synced on entry if marked dirty, and on exit only when
> explicitly needed by the host.
>
> > diff --git a/arch/arm64/kvm/handle_exit.c b/arch/arm64/kvm/handle_exit.c
> > --- a/arch/arm64/kvm/handle_exit.c
> > +++ b/arch/arm64/kvm/handle_exit.c
> > @@ -486,9 +486,32 @@ int handle_exit(struct kvm_vcpu *vcpu, int exception_index)
> >       }
> >  }
> >
> > +static void handle_exit_pkvm_state(struct kvm_vcpu *vcpu, int exception_index)
> > +{
> > +     int exception_code = ARM_EXCEPTION_CODE(exception_index);
> > +
> > +     if (!is_protected_kvm_enabled() || kvm_vm_is_protected(vcpu->kvm))
> > +             return;
> > +
> > +     /*
> > +      * Sync the context back when the host will read (trap) or write
> > +      * (SError) it. Preempt-off here, so the loaded hyp vCPU is stable.
> > +      */
> > +     if (exception_code == ARM_EXCEPTION_TRAP ||
> > +         exception_code == ARM_EXCEPTION_EL1_SERROR ||
> > +         ARM_SERROR_PENDING(exception_index)) {
> > +             kvm_call_hyp_nvhe(__pkvm_vcpu_sync_state);
> > +             vcpu_set_flag(vcpu, PKVM_HOST_STATE_DIRTY);
> > +     } else {
> > +             vcpu_clear_flag(vcpu, PKVM_HOST_STATE_DIRTY);
>
> [Severity: High]
> Is it possible for KVM_REQ_VCPU_RESET updates to be ignored due to this
> cleared flag?
>
> If a secondary vCPU is powered on (for example, via PSCI CPU_ON), it
> receives KVM_REQ_VCPU_RESET and gets kicked, leading to an exit with an
> IRQ. Since that exit takes this else branch and clears the dirty flag,
> and the reset request is processed afterward in kvm_vcpu_check_requests()
> which modifies the host-side vCPU context, wouldn't flush_hyp_vcpu() skip
> syncing this updated reset state because the flag remains cleared?
>
> This could result in the target vCPU resuming with its pre-IRQ stale
> context, eventually overwriting the new reset state on the next exit.
>
> > +     }
> > +}
> > +
>
> [ ... ]
>
> > diff --git a/arch/arm64/kvm/hyp/nvhe/hyp-main.c b/arch/arm64/kvm/hyp/nvhe/hyp-main.c
> > --- a/arch/arm64/kvm/hyp/nvhe/hyp-main.c
> > +++ b/arch/arm64/kvm/hyp/nvhe/hyp-main.c
> > @@ -203,9 +255,16 @@ static void sync_hyp_vcpu(struct pkvm_hyp_vcpu *hyp_vcpu)
> >       fpsimd_sve_sync(&hyp_vcpu->vcpu);
> >       sync_debug_state(hyp_vcpu);
> >
> > -     host_vcpu->arch.ctxt            = hyp_vcpu->vcpu.arch.ctxt;
> > -
> > -     host_vcpu->arch.hcr_el2         = hyp_vcpu->vcpu.arch.hcr_el2;
>
> [Severity: High]
> Does removing this HCR_EL2 assignment break virtual SError injection and
> cause an infinite exception loop?
>
> When the host injects a virtual SError into a non-protected guest under
> pKVM by setting HCR_VSE in the host-side vcpu->arch.hcr_el2, the hardware
> automatically clears the VSE bit in the physical HCR_EL2 register once the
> guest takes the exception.
>
> Without syncing this hardware-cleared state back to the host here, the
> host's HCR_VSE bit remains set. On the next entry, flush_hyp_vcpu() will
> repeatedly re-inject the stale HCR_VSE bit from the host into the hyp vCPU,
> locking up the guest.
>
> > +     if (pkvm_hyp_vcpu_is_protected(hyp_vcpu)) {
> > +             host_vcpu->arch.ctxt = hyp_vcpu->vcpu.arch.ctxt;
> > +     } else {
> > +             /*
> > +              * PC feeds trace_kvm_exit(), PSTATE.SS the host software-step
> > +              * machine, and both run before the next on-demand ctxt sync.
> > +              */
> > +             host_vcpu->arch.ctxt.regs.pc = hyp_vcpu->vcpu.arch.ctxt.regs.pc;
> > +             host_vcpu->arch.ctxt.regs.pstate = hyp_vcpu->vcpu.arch.ctxt.regs.pstate;
> > +     }
>
> --
> Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=8