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