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

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

[Fuad Tabba]

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@linux.dev>
---

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));

base-commit: 7475ea8b58616b33c04afb976cf0fbaad9dd63fb
-- 
2.47.3

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

[Marc Zyngier]

On Wed, 11 Nov 2026 08:27:46 +0000,
Fuad Tabba <fuad.tabba@linux.dev> 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@linux.dev>
> ---
> 
> 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.

	M.

diff --git a/arch/arm64/kvm/hyp/include/nvhe/pkvm.h b/arch/arm64/kvm/hyp/include/nvhe/pkvm.h
index 0261b2bc681f7..ef05265626b7c 100644
--- a/arch/arm64/kvm/hyp/include/nvhe/pkvm.h
+++ b/arch/arm64/kvm/hyp/include/nvhe/pkvm.h
@@ -134,6 +134,7 @@ void kvm_init_pvm_id_regs(struct kvm_vcpu *vcpu);
 void kvm_reset_pvm_sys_regs(struct kvm_vcpu *vcpu);
 int kvm_check_pvm_sysreg_table(void);
 
+void pkvm_sync_before_exception(struct kvm_vcpu *vcpu);
 int pkvm_reset_vcpu(struct pkvm_hyp_vcpu *hyp_vcpu);
 struct pkvm_hyp_vcpu *pkvm_mpidr_to_hyp_vcpu(struct pkvm_hyp_vm *vm, unsigned long mpidr);
 #endif /* __ARM64_KVM_NVHE_PKVM_H__ */
diff --git a/arch/arm64/kvm/hyp/nvhe/pkvm.c b/arch/arm64/kvm/hyp/nvhe/pkvm.c
index fc0475f0877d1..85da570ae1032 100644
--- a/arch/arm64/kvm/hyp/nvhe/pkvm.c
+++ b/arch/arm64/kvm/hyp/nvhe/pkvm.c
@@ -1321,6 +1321,8 @@ static bool pvm_psci_vcpu_on(struct pkvm_hyp_vcpu *hyp_vcpu)
 		goto error;
 	}
 
+	pkvm_sync_before_exception(&hyp_vcpu->vcpu);
+
 	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));
diff --git a/arch/arm64/kvm/hyp/nvhe/sys_regs.c b/arch/arm64/kvm/hyp/nvhe/sys_regs.c
index 32e82653ebf7e..f28a96d0e50de 100644
--- a/arch/arm64/kvm/hyp/nvhe/sys_regs.c
+++ b/arch/arm64/kvm/hyp/nvhe/sys_regs.c
@@ -254,7 +254,7 @@ static u64 pvm_calc_id_reg(const struct kvm_vcpu *vcpu, u32 id)
 	}
 }
 
-static void inject_sync64(struct kvm_vcpu *vcpu, u64 esr)
+void pkvm_sync_before_exception(struct kvm_vcpu *vcpu)
 {
 	*vcpu_pc(vcpu) = read_sysreg_el2(SYS_ELR);
 	*vcpu_cpsr(vcpu) = read_sysreg_el2(SYS_SPSR);
@@ -265,6 +265,11 @@ static void inject_sync64(struct kvm_vcpu *vcpu, u64 esr)
 	 */
 	__vcpu_assign_sys_reg(vcpu, VBAR_EL1, read_sysreg_el1(SYS_VBAR));
 	__vcpu_assign_sys_reg(vcpu, SCTLR_EL1, read_sysreg_el1(SYS_SCTLR));
+}
+
+static void inject_sync64(struct kvm_vcpu *vcpu, u64 esr)
+{
+	pkvm_sync_before_exception(vcpu);
 
 	kvm_pend_exception(vcpu, EXCEPT_AA64_EL1_SYNC);
 

-- 
Without deviation from the norm, progress is not possible.

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@kernel.org> wrote:
>
> On Wed, 11 Nov 2026 08:27:46 +0000,
> Fuad Tabba <fuad.tabba@linux.dev> 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@linux.dev>
> > ---
> >
> > 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