Re: [PATCH] KVM: arm64: Sync SCTLR_EL1 before a pVM's CPU_ON reads its endianness

[Fuad Tabba]

On Wed, 11 Nov 2026 at 11:15, Marc Zyngier <maz@xxxxxxxxxx> wrote:
>
> On Wed, 11 Nov 2026 08:27:46 +0000,
> Fuad Tabba <fuad.tabba@xxxxxxxxx> wrote:
> >
> > pKVM handles a protected guest's PSCI CPU_ON at EL2 before the guest's
> > EL1 sysregs are saved, so kvm_vcpu_is_be() reads a stale SCTLR_EL1. A
> > guest that changes SCTLR_EL1.EE and calls CPU_ON before its next exit
> > to the host starts the target vCPU with the old endianness.
> >
> > Sync SCTLR_EL1 before the read, as inject_sync64() does.
> >
> > Fixes: 9d4ec80be2eb4 ("KVM: arm64: Handle PSCI calls for protected VMs at EL2")
> > Signed-off-by: Fuad Tabba <fuad.tabba@xxxxxxxxx>
> > ---
> >
> > Notes:
> >     inject_sync64() is the only other reader on a protected guest's exit
> >     path of a guest-writable EL1 sysreg's copy, and it syncs that register
> >     first. Saving SCTLR_EL1 before the in-loop exit handlers instead would
> >     put the save on every protected exit for two readers. A non-protected
> >     guest's CPU_ON is handled by the host, after the sysregs are saved.
> >
> >  arch/arm64/kvm/hyp/nvhe/pkvm.c | 3 +++
> >  1 file changed, 3 insertions(+)
> >
> > diff --git a/arch/arm64/kvm/hyp/nvhe/pkvm.c b/arch/arm64/kvm/hyp/nvhe/pkvm.c
> > index fc0475f0877d1..e421f45383e0d 100644
> > --- a/arch/arm64/kvm/hyp/nvhe/pkvm.c
> > +++ b/arch/arm64/kvm/hyp/nvhe/pkvm.c
> > @@ -1321,6 +1321,9 @@ static bool pvm_psci_vcpu_on(struct pkvm_hyp_vcpu *hyp_vcpu)
> >               goto error;
> >       }
> >
> > +     /* Sync SCTLR_EL1 for kvm_vcpu_is_be(), as HVCs are handled before sysregs are resync'ed. */
> > +     __vcpu_assign_sys_reg(&hyp_vcpu->vcpu, SCTLR_EL1, read_sysreg_el1(SYS_SCTLR));
> > +
> >       reset_state = &target->vcpu.arch.reset_state;
> >       WRITE_ONCE(reset_state->pc, smccc_get_arg2(&hyp_vcpu->vcpu));
> >       WRITE_ONCE(reset_state->r0, smccc_get_arg3(&hyp_vcpu->vcpu));
> >
>
> I think it'd make more sense to have a global primitive that pKVM
> calls to synchronise whatever affects the control flow of a vcpu (yes,
> control flow for PSCI_ON is a bit abusive, but hey...). It's a bit
> more invasive, but at least we have a single spot to deal with this
> stuff, and it doesn't affect the hot path.
>
> Something like the hack below.

Yes, better to have it in a single spot. Will respin with that.

Cheers,
/fuad