Commit c139b6d1 authored by Marc Zyngier's avatar Marc Zyngier
Browse files

KVM: arm64: nv: Always evaluate HCR_EL2 using sanitising accessors 



A lot of the NV code depends on HCR_EL2.{E2H,TGE}, and we assume
in places that at least HCR_EL2.E2H is invariant for a given guest.

However, we make a point in *not* using the sanitising accessor
that would enforce this, and are at the mercy of the guest doing
stupid things. Clearly, that's not good.

Rework the HCR_EL2 accessors to use __vcpu_sys_reg() instead,
guaranteeing that the RESx settings get applied, specially
when HCR_EL2.E2H is evaluated. This results in fewer accessors
overall.

Huge thanks to Joey who spent a long time tracking this bug down.

Reported-by: default avatarJoey Gouly <Joey.Gouly@arm.com>
Tested-by: default avatarJoey Gouly <joey.gouly@arm.com>
Reviewed-by: default avatarJoey Gouly <joey.gouly@arm.com>
Link: https://lore.kernel.org/r/20250112165029.1181056-2-maz@kernel.org


Signed-off-by: default avatarMarc Zyngier <maz@kernel.org>
parent 40384c84

arch/arm64/include/asm/kvm_emulate.h

+15 −21
Original line number Diff line number Diff line
@@ -184,29 +184,30 @@ static inline bool vcpu_is_el2(const struct kvm_vcpu *vcpu) 
	return vcpu_is_el2_ctxt(&vcpu->arch.ctxt);

}



static inline bool __vcpu_el2_e2h_is_set(const struct kvm_cpu_context *ctxt)

static inline bool vcpu_el2_e2h_is_set(const struct kvm_vcpu *vcpu)

{

	return (!cpus_have_final_cap(ARM64_HAS_HCR_NV1) ||

		(ctxt_sys_reg(ctxt, HCR_EL2) & HCR_E2H));

		(__vcpu_sys_reg(vcpu, HCR_EL2) & HCR_E2H));

}



static inline bool vcpu_el2_e2h_is_set(const struct kvm_vcpu *vcpu)

static inline bool vcpu_el2_tge_is_set(const struct kvm_vcpu *vcpu)

{

	return __vcpu_el2_e2h_is_set(&vcpu->arch.ctxt);

	return ctxt_sys_reg(&vcpu->arch.ctxt, HCR_EL2) & HCR_TGE;

}



static inline bool __vcpu_el2_tge_is_set(const struct kvm_cpu_context *ctxt)

static inline bool is_hyp_ctxt(const struct kvm_vcpu *vcpu)

{

	return ctxt_sys_reg(ctxt, HCR_EL2) & HCR_TGE;

}

	bool e2h, tge;

	u64 hcr;



static inline bool vcpu_el2_tge_is_set(const struct kvm_vcpu *vcpu)

{

	return __vcpu_el2_tge_is_set(&vcpu->arch.ctxt);

}

	if (!vcpu_has_nv(vcpu))

		return false;



	hcr = __vcpu_sys_reg(vcpu, HCR_EL2);



	e2h = (hcr & HCR_E2H);

	tge = (hcr & HCR_TGE);



static inline bool __is_hyp_ctxt(const struct kvm_cpu_context *ctxt)

{

	/*

	 * We are in a hypervisor context if the vcpu mode is EL2 or

	 * E2H and TGE bits are set. The latter means we are in the user space
@@ -215,14 +216,7 @@ static inline bool __is_hyp_ctxt(const struct kvm_cpu_context *ctxt) 
	 * Note that the HCR_EL2.{E2H,TGE}={0,1} isn't really handled in the

	 * rest of the KVM code, and will result in a misbehaving guest.

	 */

	return vcpu_is_el2_ctxt(ctxt) ||

		(__vcpu_el2_e2h_is_set(ctxt) && __vcpu_el2_tge_is_set(ctxt)) ||

		__vcpu_el2_tge_is_set(ctxt);

}



static inline bool is_hyp_ctxt(const struct kvm_vcpu *vcpu)

{

	return vcpu_has_nv(vcpu) && __is_hyp_ctxt(&vcpu->arch.ctxt);

	return vcpu_is_el2(vcpu) || (e2h && tge) || tge;

}



static inline bool vcpu_is_host_el0(const struct kvm_vcpu *vcpu)

arch/arm64/kvm/hyp/vhe/sysreg-sr.c

+2 −2
Original line number Diff line number Diff line
@@ -216,7 +216,7 @@ void __vcpu_load_switch_sysregs(struct kvm_vcpu *vcpu) 
	__sysreg32_restore_state(vcpu);

	__sysreg_restore_user_state(guest_ctxt);



	if (unlikely(__is_hyp_ctxt(guest_ctxt))) {

	if (unlikely(is_hyp_ctxt(vcpu))) {

		__sysreg_restore_vel2_state(vcpu);

	} else {

		if (vcpu_has_nv(vcpu)) {
@@ -260,7 +260,7 @@ void __vcpu_put_switch_sysregs(struct kvm_vcpu *vcpu) 


	host_ctxt = host_data_ptr(host_ctxt);



	if (unlikely(__is_hyp_ctxt(guest_ctxt)))

	if (unlikely(is_hyp_ctxt(vcpu)))

		__sysreg_save_vel2_state(vcpu);

	else

		__sysreg_save_el1_state(guest_ctxt);