Commit 6678791e authored by Marc Zyngier's avatar Marc Zyngier
Browse files

KVM: arm64: Add assignment-specific sysreg accessor 



Assigning a value to a system register doesn't do what it is
supposed to be doing if that register is one that has RESx bits.

The main problem is that we use __vcpu_sys_reg(), which can be used
both as a lvalue and rvalue. When used as a lvalue, the bit masking
occurs *before* the new value is assigned, meaning that we (1) do
pointless work on the old cvalue, and (2) potentially assign an
invalid value as we fail to apply the masks to it.

Fix this by providing a new __vcpu_assign_sys_reg() that does
what it says on the tin, and sanitises the *new* value instead of
the old one. This comes with a significant amount of churn.

Reviewed-by: default avatarMiguel Luis <miguel.luis@oracle.com>
Reviewed-by: default avatarOliver Upton <oliver.upton@linux.dev>
Link: https://lore.kernel.org/r/20250603070824.1192795-2-maz@kernel.org


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

arch/arm64/include/asm/kvm_host.h

+11 −0
Original line number Diff line number Diff line
@@ -1107,6 +1107,17 @@ static inline u64 *___ctxt_sys_reg(const struct kvm_cpu_context *ctxt, int r) 
#define ctxt_sys_reg(c,r)	(*__ctxt_sys_reg(c,r))



u64 kvm_vcpu_apply_reg_masks(const struct kvm_vcpu *, enum vcpu_sysreg, u64);



#define __vcpu_assign_sys_reg(v, r, val)				\

	do {								\

		const struct kvm_cpu_context *ctxt = &(v)->arch.ctxt;	\

		u64 __v = (val);					\

		if (vcpu_has_nv((v)) && (r) >= __SANITISED_REG_START__)	\

			__v = kvm_vcpu_apply_reg_masks((v), (r), __v);	\

									\

		ctxt_sys_reg(ctxt, (r)) = __v;				\

	} while (0)



#define __vcpu_sys_reg(v,r)						\

	(*({								\

		const struct kvm_cpu_context *ctxt = &(v)->arch.ctxt;	\

arch/arm64/kvm/arch_timer.c

+8 −8
Original line number Diff line number Diff line
@@ -108,16 +108,16 @@ static void timer_set_ctl(struct arch_timer_context *ctxt, u32 ctl) 


	switch(arch_timer_ctx_index(ctxt)) {

	case TIMER_VTIMER:

		__vcpu_sys_reg(vcpu, CNTV_CTL_EL0) = ctl;

		__vcpu_assign_sys_reg(vcpu, CNTV_CTL_EL0, ctl);

		break;

	case TIMER_PTIMER:

		__vcpu_sys_reg(vcpu, CNTP_CTL_EL0) = ctl;

		__vcpu_assign_sys_reg(vcpu, CNTP_CTL_EL0, ctl);

		break;

	case TIMER_HVTIMER:

		__vcpu_sys_reg(vcpu, CNTHV_CTL_EL2) = ctl;

		__vcpu_assign_sys_reg(vcpu, CNTHV_CTL_EL2, ctl);

		break;

	case TIMER_HPTIMER:

		__vcpu_sys_reg(vcpu, CNTHP_CTL_EL2) = ctl;

		__vcpu_assign_sys_reg(vcpu, CNTHP_CTL_EL2, ctl);

		break;

	default:

		WARN_ON(1);
@@ -130,16 +130,16 @@ static void timer_set_cval(struct arch_timer_context *ctxt, u64 cval) 


	switch(arch_timer_ctx_index(ctxt)) {

	case TIMER_VTIMER:

		__vcpu_sys_reg(vcpu, CNTV_CVAL_EL0) = cval;

		__vcpu_assign_sys_reg(vcpu, CNTV_CVAL_EL0, cval);

		break;

	case TIMER_PTIMER:

		__vcpu_sys_reg(vcpu, CNTP_CVAL_EL0) = cval;

		__vcpu_assign_sys_reg(vcpu, CNTP_CVAL_EL0, cval);

		break;

	case TIMER_HVTIMER:

		__vcpu_sys_reg(vcpu, CNTHV_CVAL_EL2) = cval;

		__vcpu_assign_sys_reg(vcpu, CNTHV_CVAL_EL2, cval);

		break;

	case TIMER_HPTIMER:

		__vcpu_sys_reg(vcpu, CNTHP_CVAL_EL2) = cval;

		__vcpu_assign_sys_reg(vcpu, CNTHP_CVAL_EL2, cval);

		break;

	default:

		WARN_ON(1);

arch/arm64/kvm/hyp/exception.c

+2 −2
Original line number Diff line number Diff line
@@ -37,7 +37,7 @@ static inline void __vcpu_write_sys_reg(struct kvm_vcpu *vcpu, u64 val, int reg) 
	if (unlikely(vcpu_has_nv(vcpu)))

		vcpu_write_sys_reg(vcpu, val, reg);

	else if (!__vcpu_write_sys_reg_to_cpu(val, reg))

		__vcpu_sys_reg(vcpu, reg) = val;

		__vcpu_assign_sys_reg(vcpu, reg, val);

}



static void __vcpu_write_spsr(struct kvm_vcpu *vcpu, unsigned long target_mode,
@@ -51,7 +51,7 @@ static void __vcpu_write_spsr(struct kvm_vcpu *vcpu, unsigned long target_mode, 
	} else if (has_vhe()) {

		write_sysreg_el1(val, SYS_SPSR);

	} else {

		__vcpu_sys_reg(vcpu, SPSR_EL1) = val;

		__vcpu_assign_sys_reg(vcpu, SPSR_EL1, val);

	}

}

arch/arm64/kvm/hyp/include/hyp/switch.h

+2 −2
Original line number Diff line number Diff line
@@ -45,7 +45,7 @@ static inline void __fpsimd_save_fpexc32(struct kvm_vcpu *vcpu) 
	if (!vcpu_el1_is_32bit(vcpu))

		return;



	__vcpu_sys_reg(vcpu, FPEXC32_EL2) = read_sysreg(fpexc32_el2);

	__vcpu_assign_sys_reg(vcpu, FPEXC32_EL2, read_sysreg(fpexc32_el2));

}



static inline void __activate_traps_fpsimd32(struct kvm_vcpu *vcpu)
@@ -457,7 +457,7 @@ static inline void fpsimd_lazy_switch_to_host(struct kvm_vcpu *vcpu) 
	 */

	if (vcpu_has_sve(vcpu)) {

		zcr_el1 = read_sysreg_el1(SYS_ZCR);

		__vcpu_sys_reg(vcpu, vcpu_sve_zcr_elx(vcpu)) = zcr_el1;

		__vcpu_assign_sys_reg(vcpu, vcpu_sve_zcr_elx(vcpu), zcr_el1);



		/*

		 * The guest's state is always saved using the guest's max VL.

arch/arm64/kvm/hyp/include/hyp/sysreg-sr.h

+3 −3
Original line number Diff line number Diff line
@@ -307,11 +307,11 @@ static inline void __sysreg32_save_state(struct kvm_vcpu *vcpu) 
	vcpu->arch.ctxt.spsr_irq = read_sysreg(spsr_irq);

	vcpu->arch.ctxt.spsr_fiq = read_sysreg(spsr_fiq);



	__vcpu_sys_reg(vcpu, DACR32_EL2) = read_sysreg(dacr32_el2);

	__vcpu_sys_reg(vcpu, IFSR32_EL2) = read_sysreg(ifsr32_el2);

	__vcpu_assign_sys_reg(vcpu, DACR32_EL2, read_sysreg(dacr32_el2));

	__vcpu_assign_sys_reg(vcpu, IFSR32_EL2, read_sysreg(ifsr32_el2));



	if (has_vhe() || kvm_debug_regs_in_use(vcpu))

		__vcpu_sys_reg(vcpu, DBGVCR32_EL2) = read_sysreg(dbgvcr32_el2);

		__vcpu_assign_sys_reg(vcpu, DBGVCR32_EL2, read_sysreg(dbgvcr32_el2));

}



static inline void __sysreg32_restore_state(struct kvm_vcpu *vcpu)