Commit a6fa60f5614352eb53fc57b23f1984fed4e71eac

Authored by James Hogan
Committed by Greg Kroah-Hartman
1 parent 9e01c02f98

MIPS: KVM: Fix timer IRQ race when writing CP0_Compare

commit b45bacd2d048f405c7760e5cc9b60dd67708734f upstream.

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: e30492bbe95a ("MIPS: KVM: Rewrite count/compare timer emulation")
Signed-off-by: James 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
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

Showing 3 changed files with 29 additions and 36 deletions Side-by-side Diff

arch/mips/include/asm/kvm_host.h
... ... @@ -784,7 +784,7 @@
784 784  
785 785 uint32_t kvm_mips_read_count(struct kvm_vcpu *vcpu);
786 786 void kvm_mips_write_count(struct kvm_vcpu *vcpu, uint32_t count);
787   -void kvm_mips_write_compare(struct kvm_vcpu *vcpu, uint32_t compare);
  787 +void kvm_mips_write_compare(struct kvm_vcpu *vcpu, uint32_t compare, bool ack);
788 788 void kvm_mips_init_count(struct kvm_vcpu *vcpu);
789 789 int kvm_mips_set_count_ctl(struct kvm_vcpu *vcpu, s64 count_ctl);
790 790 int kvm_mips_set_count_resume(struct kvm_vcpu *vcpu, s64 count_resume);
arch/mips/kvm/emulate.c
... ... @@ -438,32 +438,6 @@
438 438 }
439 439  
440 440 /**
441   - * kvm_mips_update_hrtimer() - Update next expiry time of hrtimer.
442   - * @vcpu: Virtual CPU.
443   - *
444   - * Recalculates and updates the expiry time of the hrtimer. This can be used
445   - * after timer parameters have been altered which do not depend on the time that
446   - * the change occurs (in those cases kvm_mips_freeze_hrtimer() and
447   - * kvm_mips_resume_hrtimer() are used directly).
448   - *
449   - * It is guaranteed that no timer interrupts will be lost in the process.
450   - *
451   - * Assumes !kvm_mips_count_disabled(@vcpu) (guest CP0_Count timer is running).
452   - */
453   -static void kvm_mips_update_hrtimer(struct kvm_vcpu *vcpu)
454   -{
455   - ktime_t now;
456   - uint32_t count;
457   -
458   - /*
459   - * freeze_hrtimer takes care of a timer interrupts <= count, and
460   - * resume_hrtimer the hrtimer takes care of a timer interrupts > count.
461   - */
462   - now = kvm_mips_freeze_hrtimer(vcpu, &count);
463   - kvm_mips_resume_hrtimer(vcpu, now, count);
464   -}
465   -
466   -/**
467 441 * kvm_mips_write_count() - Modify the count and update timer.
468 442 * @vcpu: Virtual CPU.
469 443 * @count: Guest CP0_Count value to set.
470 444  
471 445  
472 446  
473 447  
474 448  
475 449  
476 450  
... ... @@ -558,23 +532,42 @@
558 532 * kvm_mips_write_compare() - Modify compare and update timer.
559 533 * @vcpu: Virtual CPU.
560 534 * @compare: New CP0_Compare value.
  535 + * @ack: Whether to acknowledge timer interrupt.
561 536 *
562 537 * Update CP0_Compare to a new value and update the timeout.
  538 + * If @ack, atomically acknowledge any pending timer interrupt, otherwise ensure
  539 + * any pending timer interrupt is preserved.
563 540 */
564   -void kvm_mips_write_compare(struct kvm_vcpu *vcpu, uint32_t compare)
  541 +void kvm_mips_write_compare(struct kvm_vcpu *vcpu, uint32_t compare, bool ack)
565 542 {
566 543 struct mips_coproc *cop0 = vcpu->arch.cop0;
  544 + int dc;
  545 + u32 old_compare = kvm_read_c0_guest_compare(cop0);
  546 + ktime_t now;
  547 + uint32_t count;
567 548  
568 549 /* if unchanged, must just be an ack */
569   - if (kvm_read_c0_guest_compare(cop0) == compare)
  550 + if (old_compare == compare) {
  551 + if (!ack)
  552 + return;
  553 + kvm_mips_callbacks->dequeue_timer_int(vcpu);
  554 + kvm_write_c0_guest_compare(cop0, compare);
570 555 return;
  556 + }
571 557  
572   - /* Update compare */
  558 + /* freeze_hrtimer() takes care of timer interrupts <= count */
  559 + dc = kvm_mips_count_disabled(vcpu);
  560 + if (!dc)
  561 + now = kvm_mips_freeze_hrtimer(vcpu, &count);
  562 +
  563 + if (ack)
  564 + kvm_mips_callbacks->dequeue_timer_int(vcpu);
  565 +
573 566 kvm_write_c0_guest_compare(cop0, compare);
574 567  
575   - /* Update timeout if count enabled */
576   - if (!kvm_mips_count_disabled(vcpu))
577   - kvm_mips_update_hrtimer(vcpu);
  568 + /* resume_hrtimer() takes care of timer interrupts > count */
  569 + if (!dc)
  570 + kvm_mips_resume_hrtimer(vcpu, now, count);
578 571 }
579 572  
580 573 /**
581 574  
... ... @@ -1113,9 +1106,9 @@
1113 1106  
1114 1107 /* If we are writing to COMPARE */
1115 1108 /* Clear pending timer interrupt, if any */
1116   - kvm_mips_callbacks->dequeue_timer_int(vcpu);
1117 1109 kvm_mips_write_compare(vcpu,
1118   - vcpu->arch.gprs[rt]);
  1110 + vcpu->arch.gprs[rt],
  1111 + true);
1119 1112 } else if ((rd == MIPS_CP0_STATUS) && (sel == 0)) {
1120 1113 unsigned int old_val, val, change;
1121 1114  
arch/mips/kvm/trap_emul.c
... ... @@ -547,7 +547,7 @@
547 547 kvm_mips_write_count(vcpu, v);
548 548 break;
549 549 case KVM_REG_MIPS_CP0_COMPARE:
550   - kvm_mips_write_compare(vcpu, v);
  550 + kvm_mips_write_compare(vcpu, v, false);
551 551 break;
552 552 case KVM_REG_MIPS_CP0_CAUSE:
553 553 /*