Skip to content

Commit

Permalink
KVM: arm64: nv: Always evaluate HCR_EL2 using sanitising accessors
Browse files Browse the repository at this point in the history
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: Joey Gouly <Joey.Gouly@arm.com>
Tested-by: Joey Gouly <joey.gouly@arm.com>
Reviewed-by: Joey Gouly <joey.gouly@arm.com>
Link: https://lore.kernel.org/r/20250112165029.1181056-2-maz@kernel.org
Signed-off-by: Marc Zyngier <maz@kernel.org>
  • Loading branch information
Marc Zyngier committed Jan 14, 2025
1 parent 40384c8 commit c139b6d
Show file tree
Hide file tree
Showing 2 changed files with 17 additions and 23 deletions.
36 changes: 15 additions & 21 deletions arch/arm64/include/asm/kvm_emulate.h
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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)
Expand Down
4 changes: 2 additions & 2 deletions arch/arm64/kvm/hyp/vhe/sysreg-sr.c
Original file line number Diff line number Diff line change
Expand Up @@ -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)) {
Expand Down Expand Up @@ -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);
Expand Down

0 comments on commit c139b6d

Please sign in to comment.