KVM: arm/arm64: Properly handle arch-timer IRQs after vtimer_save_state (36e5cfd4) · Commits · 戴 / test

The recent timer rework was assuming that once the timer was disabled, we should no longer see any interrupts from the timer. This assumption turns out to not be true, and instead we have to handle the case when the timer ISR runs even after the timer has been disabled. This requires a couple of changes: First, we should never overwrite the cached guest state of the timer control register when the ISR runs, because KVM may have disabled its timers when doing vcpu_put(), even though the guest still had the timer enabled. Second, we shouldn't assume that the timer is actually firing just because we see an interrupt, but we should check the actual state of the timer in the timer control register to understand if the hardware timer is really firing or not. We also add an ISB to vtimer_save_state() to ensure the timer is actually disabled once we enable interrupts, which should clarify the intention of the implementation, and reduce the risk of unwanted interrupts. Fixes: ("KVM: arm/arm64: Avoid timer save/restore in vcpu entry/exit") Reported-by:

Marc Zyngier <marc.zyngier@arm.com> Reported-by:

Jia He <hejianet@gmail.com> Reviewed-by:

Marc Zyngier <marc.zyngier@arm.com> Tested-by:

Marc Zyngier <marc.zyngier@arm.com> Signed-off-by:

Christoffer Dall <christoffer.dall@linaro.org>

virt/kvm/arm/arch_timer.c

+15 −7

Original line number	Original line	Diff line number	Diff line
	@@ -92,16 +92,23 @@ static irqreturn_t kvm_arch_timer_handler(int irq, void *dev_id)
	{		{
	struct kvm_vcpu vcpu = (struct kvm_vcpu **)dev_id;		struct kvm_vcpu vcpu = (struct kvm_vcpu **)dev_id;
	struct arch_timer_context *vtimer;		struct arch_timer_context *vtimer;
			u32 cnt_ctl;

	if (!vcpu) {		/*
	pr_warn_once("Spurious arch timer IRQ on non-VCPU thread\n");		* We may see a timer interrupt after vcpu_put() has been called which
	return IRQ_NONE;		* sets the CPU's vcpu pointer to NULL, because even though the timer
	}		* has been disabled in vtimer_save_state(), the hardware interrupt
	vtimer = vcpu_vtimer(vcpu);		* signal may not have been retired from the interrupt controller yet.
			*/
			if (!vcpu)
			return IRQ_HANDLED;

			vtimer = vcpu_vtimer(vcpu);
	if (!vtimer->irq.level) {		if (!vtimer->irq.level) {
	vtimer->cnt_ctl = read_sysreg_el0(cntv_ctl);		cnt_ctl = read_sysreg_el0(cntv_ctl);
	if (kvm_timer_irq_can_fire(vtimer))		cnt_ctl &= ARCH_TIMER_CTRL_ENABLE \| ARCH_TIMER_CTRL_IT_STAT \|
			ARCH_TIMER_CTRL_IT_MASK;
			if (cnt_ctl == (ARCH_TIMER_CTRL_ENABLE \| ARCH_TIMER_CTRL_IT_STAT))
	kvm_timer_update_irq(vcpu, true, vtimer);		kvm_timer_update_irq(vcpu, true, vtimer);
	}		}

	@@ -355,6 +362,7 @@ static void vtimer_save_state(struct kvm_vcpu *vcpu)

	/* Disable the virtual timer */		/* Disable the virtual timer */
	write_sysreg_el0(0, cntv_ctl);		write_sysreg_el0(0, cntv_ctl);
			isb();

	vtimer->loaded = false;		vtimer->loaded = false;
	out:		out:

Admin message