Commit b45bacd2 authored by James Hogan's avatar James Hogan Committed by Paolo Bonzini

MIPS: KVM: Fix timer IRQ race when writing CP0_Compare

Writing CP0_Compare clears the timer interrupt pending bit
(CP0_Cause.TI), but this wasn't being done atomically. If a timer
interrupt raced with the write of the guest CP0_Compare, the timer
interrupt could end up being pending even though the new CP0_Compare is
nowhere near CP0_Count.

We were already updating the hrtimer expiry with
kvm_mips_update_hrtimer(), which used both kvm_mips_freeze_hrtimer() and
kvm_mips_resume_hrtimer(). Close the race window by expanding out
kvm_mips_update_hrtimer(), and clearing CP0_Cause.TI and setting
CP0_Compare between the freeze and resume. Since the pending timer
interrupt should not be cleared when CP0_Compare is written via the KVM
user API, an ack argument is added to distinguish the source of the
write.

Fixes: e30492bb ("MIPS: KVM: Rewrite count/compare timer emulation")
Signed-off-by: default avatarJames Hogan <james.hogan@imgtec.com>
Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: "Radim KrčmáÅ" <rkrcmar@redhat.com>
Cc: Ralf Baechle <ralf@linux-mips.org>
Cc: linux-mips@linux-mips.org
Cc: kvm@vger.kernel.org
Cc: <stable@vger.kernel.org> # 3.16.x-
Signed-off-by: default avatarPaolo Bonzini <pbonzini@redhat.com>
parent 4355c44f
...@@ -747,7 +747,7 @@ extern enum emulation_result kvm_mips_complete_mmio_load(struct kvm_vcpu *vcpu, ...@@ -747,7 +747,7 @@ extern enum emulation_result kvm_mips_complete_mmio_load(struct kvm_vcpu *vcpu,
uint32_t kvm_mips_read_count(struct kvm_vcpu *vcpu); uint32_t kvm_mips_read_count(struct kvm_vcpu *vcpu);
void kvm_mips_write_count(struct kvm_vcpu *vcpu, uint32_t count); void kvm_mips_write_count(struct kvm_vcpu *vcpu, uint32_t count);
void kvm_mips_write_compare(struct kvm_vcpu *vcpu, uint32_t compare); void kvm_mips_write_compare(struct kvm_vcpu *vcpu, uint32_t compare, bool ack);
void kvm_mips_init_count(struct kvm_vcpu *vcpu); void kvm_mips_init_count(struct kvm_vcpu *vcpu);
int kvm_mips_set_count_ctl(struct kvm_vcpu *vcpu, s64 count_ctl); int kvm_mips_set_count_ctl(struct kvm_vcpu *vcpu, s64 count_ctl);
int kvm_mips_set_count_resume(struct kvm_vcpu *vcpu, s64 count_resume); int kvm_mips_set_count_resume(struct kvm_vcpu *vcpu, s64 count_resume);
......
...@@ -437,32 +437,6 @@ static void kvm_mips_resume_hrtimer(struct kvm_vcpu *vcpu, ...@@ -437,32 +437,6 @@ static void kvm_mips_resume_hrtimer(struct kvm_vcpu *vcpu,
hrtimer_start(&vcpu->arch.comparecount_timer, expire, HRTIMER_MODE_ABS); hrtimer_start(&vcpu->arch.comparecount_timer, expire, HRTIMER_MODE_ABS);
} }
/**
* kvm_mips_update_hrtimer() - Update next expiry time of hrtimer.
* @vcpu: Virtual CPU.
*
* Recalculates and updates the expiry time of the hrtimer. This can be used
* after timer parameters have been altered which do not depend on the time that
* the change occurs (in those cases kvm_mips_freeze_hrtimer() and
* kvm_mips_resume_hrtimer() are used directly).
*
* It is guaranteed that no timer interrupts will be lost in the process.
*
* Assumes !kvm_mips_count_disabled(@vcpu) (guest CP0_Count timer is running).
*/
static void kvm_mips_update_hrtimer(struct kvm_vcpu *vcpu)
{
ktime_t now;
uint32_t count;
/*
* freeze_hrtimer takes care of a timer interrupts <= count, and
* resume_hrtimer the hrtimer takes care of a timer interrupts > count.
*/
now = kvm_mips_freeze_hrtimer(vcpu, &count);
kvm_mips_resume_hrtimer(vcpu, now, count);
}
/** /**
* kvm_mips_write_count() - Modify the count and update timer. * kvm_mips_write_count() - Modify the count and update timer.
* @vcpu: Virtual CPU. * @vcpu: Virtual CPU.
...@@ -558,23 +532,42 @@ int kvm_mips_set_count_hz(struct kvm_vcpu *vcpu, s64 count_hz) ...@@ -558,23 +532,42 @@ int kvm_mips_set_count_hz(struct kvm_vcpu *vcpu, s64 count_hz)
* kvm_mips_write_compare() - Modify compare and update timer. * kvm_mips_write_compare() - Modify compare and update timer.
* @vcpu: Virtual CPU. * @vcpu: Virtual CPU.
* @compare: New CP0_Compare value. * @compare: New CP0_Compare value.
* @ack: Whether to acknowledge timer interrupt.
* *
* Update CP0_Compare to a new value and update the timeout. * Update CP0_Compare to a new value and update the timeout.
* If @ack, atomically acknowledge any pending timer interrupt, otherwise ensure
* any pending timer interrupt is preserved.
*/ */
void kvm_mips_write_compare(struct kvm_vcpu *vcpu, uint32_t compare) void kvm_mips_write_compare(struct kvm_vcpu *vcpu, uint32_t compare, bool ack)
{ {
struct mips_coproc *cop0 = vcpu->arch.cop0; struct mips_coproc *cop0 = vcpu->arch.cop0;
int dc;
u32 old_compare = kvm_read_c0_guest_compare(cop0);
ktime_t now;
uint32_t count;
/* if unchanged, must just be an ack */ /* if unchanged, must just be an ack */
if (kvm_read_c0_guest_compare(cop0) == compare) if (old_compare == compare) {
if (!ack)
return; return;
kvm_mips_callbacks->dequeue_timer_int(vcpu);
kvm_write_c0_guest_compare(cop0, compare);
return;
}
/* freeze_hrtimer() takes care of timer interrupts <= count */
dc = kvm_mips_count_disabled(vcpu);
if (!dc)
now = kvm_mips_freeze_hrtimer(vcpu, &count);
if (ack)
kvm_mips_callbacks->dequeue_timer_int(vcpu);
/* Update compare */
kvm_write_c0_guest_compare(cop0, compare); kvm_write_c0_guest_compare(cop0, compare);
/* Update timeout if count enabled */ /* resume_hrtimer() takes care of timer interrupts > count */
if (!kvm_mips_count_disabled(vcpu)) if (!dc)
kvm_mips_update_hrtimer(vcpu); kvm_mips_resume_hrtimer(vcpu, now, count);
} }
/** /**
...@@ -1113,9 +1106,9 @@ enum emulation_result kvm_mips_emulate_CP0(uint32_t inst, uint32_t *opc, ...@@ -1113,9 +1106,9 @@ enum emulation_result kvm_mips_emulate_CP0(uint32_t inst, uint32_t *opc,
/* If we are writing to COMPARE */ /* If we are writing to COMPARE */
/* Clear pending timer interrupt, if any */ /* Clear pending timer interrupt, if any */
kvm_mips_callbacks->dequeue_timer_int(vcpu);
kvm_mips_write_compare(vcpu, kvm_mips_write_compare(vcpu,
vcpu->arch.gprs[rt]); vcpu->arch.gprs[rt],
true);
} else if ((rd == MIPS_CP0_STATUS) && (sel == 0)) { } else if ((rd == MIPS_CP0_STATUS) && (sel == 0)) {
unsigned int old_val, val, change; unsigned int old_val, val, change;
......
...@@ -546,7 +546,7 @@ static int kvm_trap_emul_set_one_reg(struct kvm_vcpu *vcpu, ...@@ -546,7 +546,7 @@ static int kvm_trap_emul_set_one_reg(struct kvm_vcpu *vcpu,
kvm_mips_write_count(vcpu, v); kvm_mips_write_count(vcpu, v);
break; break;
case KVM_REG_MIPS_CP0_COMPARE: case KVM_REG_MIPS_CP0_COMPARE:
kvm_mips_write_compare(vcpu, v); kvm_mips_write_compare(vcpu, v, false);
break; break;
case KVM_REG_MIPS_CP0_CAUSE: case KVM_REG_MIPS_CP0_CAUSE:
/* /*
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment