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

[Fuad Tabba]

On Mon, 22 Jun 2026 at 09:49, Vincent Donnefort <vdonnefort@xxxxxxxxxx> wrote:
>
> [...]
>
> > > > diff --git a/arch/arm64/kvm/handle_exit.c b/arch/arm64/kvm/handle_exit.c
> > > > index 54aedf93c78b..8963621bcdd1 100644
> > > > --- a/arch/arm64/kvm/handle_exit.c
> > > > +++ b/arch/arm64/kvm/handle_exit.c
> > > > @@ -422,6 +422,20 @@ static int handle_trap_exceptions(struct kvm_vcpu *vcpu)
> > > >  {
> > > >       int handled;
> > > >
> > > > +     /*
> > > > +      * If we run a non-protected VM when protection is enabled
> > > > +      * system-wide, resync the state from the hypervisor and mark
> > > > +      * it as dirty on the host side if it wasn't dirty already
> > > > +      * (which could happen if preemption has taken place).
> > > > +      */
> > > > +     if (is_protected_kvm_enabled() && !kvm_vm_is_protected(vcpu->kvm)) {
> > > > +             guard(preempt)();
> > > > +             if (!(vcpu_get_flag(vcpu, PKVM_HOST_STATE_DIRTY))) {
> > > > +                     kvm_call_hyp_nvhe(__pkvm_vcpu_sync_state);
> > > > +                     vcpu_set_flag(vcpu, PKVM_HOST_STATE_DIRTY);
> > > > +             }
> > > > +     }
> > > > +
> > >
> > > Could we remove this update here and let handle_exit_early() do the sync
> > > regardless of the SError injection? One of the main point of handle_exit_early()
> > > is to do things under !prempt().
> >
> > Agreed on the move: handle_exit_early() is already preempt-off, so the
> > guard() goes away. Not on every exit though. handle_exit_early() runs
> > on every exit, and sync_hyp_vcpu() only copies PC/PSTATE/fault back
> > for a non-protected guest; the GPRs and sysregs cross solely via
> > __pkvm_vcpu_sync_state. Syncing unconditionally would pull the full
> > context back on plain IRQ exits, which is the copy this patch avoids.
> > So I will gate it on trap-or-SError and drop the
> > handle_trap_exceptions() block.
> >
> > >
> > >
> > > >       /*
> > > >        * See ARM ARM B1.14.1: "Hyp traps on instructions
> > > >        * that fail their condition code check"
> > > > @@ -489,6 +503,22 @@ int handle_exit(struct kvm_vcpu *vcpu, int exception_index)
> > > >  /* For exit types that need handling before we can be preempted */
> > > >  void handle_exit_early(struct kvm_vcpu *vcpu, int exception_index)
> > > >  {
> > > > +     bool inject_serror = ARM_SERROR_PENDING(exception_index) ||
> > > > +             ARM_EXCEPTION_CODE(exception_index) == ARM_EXCEPTION_EL1_SERROR;
> > > > +
> > > > +     /*
> > > > +      * An SError injected below writes the host ctxt; for a non-protected
> > > > +      * guest, sync from the hyp vCPU and keep it dirty so it isn't dropped.
> > > > +      */
> > > > +     if (is_protected_kvm_enabled()) {
> > >
> > > Should we test !kvm_vm_is_protected(vcpu->kvm) here, as the
> > > PKVM_HOST_STATE_DIRTY is only updated for p-guests everywhere else?
> >
> > Yes. The flag is only ever set for non-protected guests, so clearing it
> > for a protected one is a no-op, but gating it matches the invariant.
> >
> > Both fold into one block in handle_exit_early():
> >
> >       if (is_protected_kvm_enabled() && !kvm_vm_is_protected(vcpu->kvm)) {
> >               if (inject_serror ||
> >                   ARM_EXCEPTION_CODE(exception_index) == ARM_EXCEPTION_TRAP) {
> >                       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);
> >               }
> >       }
> >
> > I will fold this into the next respin.
>
> Ah yes of course, I was hoping we could just have a switch here, just like
> handle_exit() does, but that's not possible because of ARM_SERROR_PENDING().
>
> Perhaps it would look cleaner if done in a separate function
> handle_exit_pkvm_state()?

Agreed, that reads better. Pulling it into handle_exit_pkvm_state()
also gets inject_serror out of handle_exit_early(), which only used it
for this. Done for the next respin.

Cheers,
/fuad

>
>
> >
> > Thanks for the reviews!
> > /fuad
> >
> > >
> > > > +             vcpu_clear_flag(vcpu, PKVM_HOST_STATE_DIRTY);
> > > > +
> > > > +             if (inject_serror && !kvm_vm_is_protected(vcpu->kvm)) {
> > > > +                     kvm_call_hyp_nvhe(__pkvm_vcpu_sync_state);
> > > > +                     vcpu_set_flag(vcpu, PKVM_HOST_STATE_DIRTY);
> > > > +             }
> > > > +     }
> > > > +
> > > >       if (ARM_SERROR_PENDING(exception_index)) {
> > > >               if (this_cpu_has_cap(ARM64_HAS_RAS_EXTN)) {
> > > >                       u64 disr = kvm_vcpu_get_disr(vcpu);
> > >
> > > [...]