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

	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.