lapic.timer_mode was not properly initialized after migration, which
broke few useful things, like login, by making every sleep eternal.
Fix this by calling apic_update_lvtt in kvm_apic_post_state_restore.
There are other slowpaths that update lvtt, so this patch makes sure
something similar doesn't happen again by calling apic_update_lvtt
after every modification.
Cc: [email protected]
Fixes: f30ebc312ca9 ("KVM: x86: optimize some accesses to LVTT and SPIV")
Signed-off-by: Radim Krčmář <[email protected]>
---
arch/x86/kvm/lapic.c | 26 ++++++++++++++++----------
1 file changed, 16 insertions(+), 10 deletions(-)
diff --git a/arch/x86/kvm/lapic.c b/arch/x86/kvm/lapic.c
index beeef05bb4d9..36e9de1b4127 100644
--- a/arch/x86/kvm/lapic.c
+++ b/arch/x86/kvm/lapic.c
@@ -1103,6 +1103,17 @@ static void update_divide_count(struct kvm_lapic *apic)
apic->divide_count);
}
+static void apic_update_lvtt(struct kvm_lapic *apic)
+{
+ u32 timer_mode = kvm_apic_get_reg(apic, APIC_LVTT) &
+ apic->lapic_timer.timer_mode_mask;
+
+ if (apic->lapic_timer.timer_mode != timer_mode) {
+ apic->lapic_timer.timer_mode = timer_mode;
+ hrtimer_cancel(&apic->lapic_timer.timer);
+ }
+}
+
static void apic_timer_expired(struct kvm_lapic *apic)
{
struct kvm_vcpu *vcpu = apic->vcpu;
@@ -1311,6 +1322,7 @@ static int apic_reg_write(struct kvm_lapic *apic, u32 reg, u32 val)
apic_set_reg(apic, APIC_LVTT + 0x10 * i,
lvt_val | APIC_LVT_MASKED);
}
+ apic_update_lvtt(apic);
atomic_set(&apic->lapic_timer.pending, 0);
}
@@ -1343,20 +1355,13 @@ static int apic_reg_write(struct kvm_lapic *apic, u32 reg, u32 val)
break;
- case APIC_LVTT: {
- u32 timer_mode = val & apic->lapic_timer.timer_mode_mask;
-
- if (apic->lapic_timer.timer_mode != timer_mode) {
- apic->lapic_timer.timer_mode = timer_mode;
- hrtimer_cancel(&apic->lapic_timer.timer);
- }
-
+ case APIC_LVTT:
if (!kvm_apic_sw_enabled(apic))
val |= APIC_LVT_MASKED;
val &= (apic_lvt_mask[0] | apic->lapic_timer.timer_mode_mask);
apic_set_reg(apic, APIC_LVTT, val);
+ apic_update_lvtt(apic);
break;
- }
case APIC_TMICT:
if (apic_lvtt_tscdeadline(apic))
@@ -1588,7 +1593,7 @@ void kvm_lapic_reset(struct kvm_vcpu *vcpu, bool init_event)
for (i = 0; i < APIC_LVT_NUM; i++)
apic_set_reg(apic, APIC_LVTT + 0x10 * i, APIC_LVT_MASKED);
- apic->lapic_timer.timer_mode = 0;
+ apic_update_lvtt(apic);
if (!(vcpu->kvm->arch.disabled_quirks & KVM_QUIRK_LINT0_REENABLED))
apic_set_reg(apic, APIC_LVT0,
SET_APIC_DELIVERY_MODE(0, APIC_MODE_EXTINT));
@@ -1816,6 +1821,7 @@ void kvm_apic_post_state_restore(struct kvm_vcpu *vcpu,
apic_update_ppr(apic);
hrtimer_cancel(&apic->lapic_timer.timer);
+ apic_update_lvtt(apic);
update_divide_count(apic);
start_apic_timer(apic);
apic->irr_pending = true;
--
2.4.2
On 05/06/2015 20:57, Radim Krčmář wrote:
> lapic.timer_mode was not properly initialized after migration, which
> broke few useful things, like login, by making every sleep eternal.
>
> Fix this by calling apic_update_lvtt in kvm_apic_post_state_restore.
>
> There are other slowpaths that update lvtt, so this patch makes sure
> something similar doesn't happen again by calling apic_update_lvtt
> after every modification.
>
> Cc: [email protected]
> Fixes: f30ebc312ca9 ("KVM: x86: optimize some accesses to LVTT and SPIV")
> Signed-off-by: Radim Krčmář <[email protected]>
> ---
> arch/x86/kvm/lapic.c | 26 ++++++++++++++++----------
> 1 file changed, 16 insertions(+), 10 deletions(-)
>
> diff --git a/arch/x86/kvm/lapic.c b/arch/x86/kvm/lapic.c
> index beeef05bb4d9..36e9de1b4127 100644
> --- a/arch/x86/kvm/lapic.c
> +++ b/arch/x86/kvm/lapic.c
> @@ -1103,6 +1103,17 @@ static void update_divide_count(struct kvm_lapic *apic)
> apic->divide_count);
> }
>
> +static void apic_update_lvtt(struct kvm_lapic *apic)
> +{
> + u32 timer_mode = kvm_apic_get_reg(apic, APIC_LVTT) &
> + apic->lapic_timer.timer_mode_mask;
> +
> + if (apic->lapic_timer.timer_mode != timer_mode) {
> + apic->lapic_timer.timer_mode = timer_mode;
> + hrtimer_cancel(&apic->lapic_timer.timer);
> + }
> +}
> +
> static void apic_timer_expired(struct kvm_lapic *apic)
> {
> struct kvm_vcpu *vcpu = apic->vcpu;
> @@ -1311,6 +1322,7 @@ static int apic_reg_write(struct kvm_lapic *apic, u32 reg, u32 val)
> apic_set_reg(apic, APIC_LVTT + 0x10 * i,
> lvt_val | APIC_LVT_MASKED);
> }
> + apic_update_lvtt(apic);
> atomic_set(&apic->lapic_timer.pending, 0);
>
> }
> @@ -1343,20 +1355,13 @@ static int apic_reg_write(struct kvm_lapic *apic, u32 reg, u32 val)
>
> break;
>
> - case APIC_LVTT: {
> - u32 timer_mode = val & apic->lapic_timer.timer_mode_mask;
> -
> - if (apic->lapic_timer.timer_mode != timer_mode) {
> - apic->lapic_timer.timer_mode = timer_mode;
> - hrtimer_cancel(&apic->lapic_timer.timer);
> - }
> -
> + case APIC_LVTT:
> if (!kvm_apic_sw_enabled(apic))
> val |= APIC_LVT_MASKED;
> val &= (apic_lvt_mask[0] | apic->lapic_timer.timer_mode_mask);
> apic_set_reg(apic, APIC_LVTT, val);
> + apic_update_lvtt(apic);
> break;
> - }
>
> case APIC_TMICT:
> if (apic_lvtt_tscdeadline(apic))
> @@ -1588,7 +1593,7 @@ void kvm_lapic_reset(struct kvm_vcpu *vcpu, bool init_event)
>
> for (i = 0; i < APIC_LVT_NUM; i++)
> apic_set_reg(apic, APIC_LVTT + 0x10 * i, APIC_LVT_MASKED);
> - apic->lapic_timer.timer_mode = 0;
> + apic_update_lvtt(apic);
> if (!(vcpu->kvm->arch.disabled_quirks & KVM_QUIRK_LINT0_REENABLED))
> apic_set_reg(apic, APIC_LVT0,
> SET_APIC_DELIVERY_MODE(0, APIC_MODE_EXTINT));
> @@ -1816,6 +1821,7 @@ void kvm_apic_post_state_restore(struct kvm_vcpu *vcpu,
>
> apic_update_ppr(apic);
> hrtimer_cancel(&apic->lapic_timer.timer);
> + apic_update_lvtt(apic);
> update_divide_count(apic);
> start_apic_timer(apic);
> apic->irr_pending = true;
>
Marcelo, if you have some free cycles feel free to apply this to
kvm/master and send it to Linus sometime next week. I cannot do it on
Monday and I'll be on vacation afterwards. (I'll be back as soon as
June 16th so I didn't plan on a formal handoff, but I think it's better
to have this in 4.1. The merge window conflicts with Linus's own
vacation and might be delayed).
And thanks Radim for the fix, it looks good.
Paolo