[PATCH 0/3] KVM: x86: fix restart_apic_timer

[PATCH 0/3] KVM: x86: fix restart_apic_timer

[Radim Krčmář]

Various bugs that incorrectly injected a timer interrupt.

Going to work on kvm-unit-tests for this too.


Radim Krčmář (3):
  KVM: x86: handle 0 write to TSC_DEADLINE MSR
  KVM: x86: really disarm lapic timer when clearing TMICT
  KVM: x86: thoroughly disarm LAPIC timer around TSC deadline switch

 arch/x86/kvm/lapic.c | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

-- 
2.14.2

[PATCH 1/3] KVM: x86: handle 0 write to TSC_DEADLINE MSR

[Radim Krčmář]

0 should disable the timer, but start_hv_timer will recognize it as an
expired timer instead.

Signed-off-by: Radim Krčmář <rkrcmar@redhat.com>
---
 arch/x86/kvm/lapic.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/arch/x86/kvm/lapic.c b/arch/x86/kvm/lapic.c
index 2c9e88a82738..39c1ae11ce1d 100644
--- a/arch/x86/kvm/lapic.c
+++ b/arch/x86/kvm/lapic.c
@@ -1549,6 +1549,9 @@ static bool start_hv_timer(struct kvm_lapic *apic)
 	if (!apic_lvtt_period(apic) && atomic_read(&ktimer->pending))
 		return false;
 
+	if (!ktimer->tscdeadline)
+		return false;
+
 	r = kvm_x86_ops->set_hv_timer(apic->vcpu, ktimer->tscdeadline);
 	if (r < 0)
 		return false;
-- 
2.14.2

Re: [PATCH 1/3] KVM: x86: handle 0 write to TSC_DEADLINE MSR

[Wanpeng Li]

2017-10-07 1:25 GMT+08:00 Radim Krčmář <rkrcmar@redhat.com>:
> 0 should disable the timer, but start_hv_timer will recognize it as an
> expired timer instead.
>
> Signed-off-by: Radim Krčmář <rkrcmar@redhat.com>

Reviewed-by: Wanpeng Li <wanpeng.li@hotmail.com>

> ---
>  arch/x86/kvm/lapic.c | 3 +++
>  1 file changed, 3 insertions(+)
>
> diff --git a/arch/x86/kvm/lapic.c b/arch/x86/kvm/lapic.c
> index 2c9e88a82738..39c1ae11ce1d 100644
> --- a/arch/x86/kvm/lapic.c
> +++ b/arch/x86/kvm/lapic.c
> @@ -1549,6 +1549,9 @@ static bool start_hv_timer(struct kvm_lapic *apic)
>         if (!apic_lvtt_period(apic) && atomic_read(&ktimer->pending))
>                 return false;
>
> +       if (!ktimer->tscdeadline)
> +               return false;
> +
>         r = kvm_x86_ops->set_hv_timer(apic->vcpu, ktimer->tscdeadline);
>         if (r < 0)
>                 return false;
> --
> 2.14.2
>

[PATCH 2/3] KVM: x86: really disarm lapic timer when clearing TMICT

[Radim Krčmář]

preemption timer only looks at tscdeadline and could inject already
disarmed timer.

Signed-off-by: Radim Krčmář <rkrcmar@redhat.com>
---
 arch/x86/kvm/lapic.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/arch/x86/kvm/lapic.c b/arch/x86/kvm/lapic.c
index 39c1ae11ce1d..96ade848ae0b 100644
--- a/arch/x86/kvm/lapic.c
+++ b/arch/x86/kvm/lapic.c
@@ -1489,8 +1489,10 @@ static bool set_target_expiration(struct kvm_lapic *apic)
 	apic->lapic_timer.period = (u64)kvm_lapic_get_reg(apic, APIC_TMICT)
 		* APIC_BUS_CYCLE_NS * apic->divide_count;
 
-	if (!apic->lapic_timer.period)
+	if (!apic->lapic_timer.period) {
+		apic->lapic_timer.tscdeadline = 0;
 		return false;
+	}
 
 	limit_periodic_timer_frequency(apic);
 
-- 
2.14.2

Re: [PATCH 2/3] KVM: x86: really disarm lapic timer when clearing TMICT

[Wanpeng Li]

2017-10-07 1:25 GMT+08:00 Radim Krčmář <rkrcmar@redhat.com>:
> preemption timer only looks at tscdeadline and could inject already
> disarmed timer.
>
> Signed-off-by: Radim Krčmář <rkrcmar@redhat.com>

Reviewed-by: Wanpeng Li <wanpeng.li@hotmail.com>

> ---
>  arch/x86/kvm/lapic.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/arch/x86/kvm/lapic.c b/arch/x86/kvm/lapic.c
> index 39c1ae11ce1d..96ade848ae0b 100644
> --- a/arch/x86/kvm/lapic.c
> +++ b/arch/x86/kvm/lapic.c
> @@ -1489,8 +1489,10 @@ static bool set_target_expiration(struct kvm_lapic *apic)
>         apic->lapic_timer.period = (u64)kvm_lapic_get_reg(apic, APIC_TMICT)
>                 * APIC_BUS_CYCLE_NS * apic->divide_count;
>
> -       if (!apic->lapic_timer.period)
> +       if (!apic->lapic_timer.period) {
> +               apic->lapic_timer.tscdeadline = 0;
>                 return false;
> +       }
>
>         limit_periodic_timer_frequency(apic);
>
> --
> 2.14.2
>

[PATCH 3/3] KVM: x86: thoroughly disarm LAPIC timer around TSC deadline switch

[Radim Krčmář]

Our routines look at tscdeadline and period when deciding state of a
timer.  The timer is disarmed when switching between TSC deadline and
other modes, so we should set everything to disarmed state.

Signed-off-by: Radim Krčmář <rkrcmar@redhat.com>
---
 arch/x86/kvm/lapic.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/arch/x86/kvm/lapic.c b/arch/x86/kvm/lapic.c
index 96ade848ae0b..a778f1ae2927 100644
--- a/arch/x86/kvm/lapic.c
+++ b/arch/x86/kvm/lapic.c
@@ -1330,8 +1330,10 @@ static void apic_update_lvtt(struct kvm_lapic *apic)
 	if (apic->lapic_timer.timer_mode != timer_mode) {
 		if (apic_lvtt_tscdeadline(apic) != (timer_mode ==
 				APIC_LVT_TIMER_TSCDEADLINE)) {
-			kvm_lapic_set_reg(apic, APIC_TMICT, 0);
 			hrtimer_cancel(&apic->lapic_timer.timer);
+			kvm_lapic_set_reg(apic, APIC_TMICT, 0);
+			apic->lapic_timer.period = 0;
+			apic->lapic_timer.tscdeadline = 0;
 		}
 		apic->lapic_timer.timer_mode = timer_mode;
 		limit_periodic_timer_frequency(apic);
-- 
2.14.2

Re: [PATCH 3/3] KVM: x86: thoroughly disarm LAPIC timer around TSC deadline switch

[Wanpeng Li]

2017-10-07 1:25 GMT+08:00 Radim Krčmář <rkrcmar@redhat.com>:
> Our routines look at tscdeadline and period when deciding state of a
> timer.  The timer is disarmed when switching between TSC deadline and
> other modes, so we should set everything to disarmed state.
>
> Signed-off-by: Radim Krčmář <rkrcmar@redhat.com>

Reviewed-by: Wanpeng Li <wanpeng.li@hotmail.com>

> ---
>  arch/x86/kvm/lapic.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/arch/x86/kvm/lapic.c b/arch/x86/kvm/lapic.c
> index 96ade848ae0b..a778f1ae2927 100644
> --- a/arch/x86/kvm/lapic.c
> +++ b/arch/x86/kvm/lapic.c
> @@ -1330,8 +1330,10 @@ static void apic_update_lvtt(struct kvm_lapic *apic)
>         if (apic->lapic_timer.timer_mode != timer_mode) {
>                 if (apic_lvtt_tscdeadline(apic) != (timer_mode ==
>                                 APIC_LVT_TIMER_TSCDEADLINE)) {
> -                       kvm_lapic_set_reg(apic, APIC_TMICT, 0);
>                         hrtimer_cancel(&apic->lapic_timer.timer);
> +                       kvm_lapic_set_reg(apic, APIC_TMICT, 0);
> +                       apic->lapic_timer.period = 0;
> +                       apic->lapic_timer.tscdeadline = 0;
>                 }
>                 apic->lapic_timer.timer_mode = timer_mode;
>                 limit_periodic_timer_frequency(apic);
> --
> 2.14.2
>

Re: [PATCH 0/3] KVM: x86: fix restart_apic_timer

[Wanpeng Li]

2017-10-07 1:25 GMT+08:00 Radim Krčmář <rkrcmar@redhat.com>:
> Various bugs that incorrectly injected a timer interrupt.
>
> Going to work on kvm-unit-tests for this too.

Btw, there is a testcase which I posted before also can be used to
test apic timer mode transition.
https://patchwork.kernel.org/patch/9976971/

Regards,
Wanpeng Li

>
>
> Radim Krčmář (3):
>   KVM: x86: handle 0 write to TSC_DEADLINE MSR
>   KVM: x86: really disarm lapic timer when clearing TMICT
>   KVM: x86: thoroughly disarm LAPIC timer around TSC deadline switch
>
>  arch/x86/kvm/lapic.c | 11 +++++++++--
>  1 file changed, 9 insertions(+), 2 deletions(-)
>
> --
> 2.14.2
>

Re: [PATCH 0/3] KVM: x86: fix restart_apic_timer

[Paolo Bonzini]

On 06/10/2017 19:25, Radim Krčmář wrote:
> Various bugs that incorrectly injected a timer interrupt.
> 
> Going to work on kvm-unit-tests for this too.
> 
> 
> Radim Krčmář (3):
>   KVM: x86: handle 0 write to TSC_DEADLINE MSR
>   KVM: x86: really disarm lapic timer when clearing TMICT
>   KVM: x86: thoroughly disarm LAPIC timer around TSC deadline switch
> 
>  arch/x86/kvm/lapic.c | 11 +++++++++--
>  1 file changed, 9 insertions(+), 2 deletions(-)
> 

Queued, thanks.

Paolo