[PATCH] KVM: x86: Clear apic tsc-deadline after deadline

[PATCH] KVM: x86: Clear apic tsc-deadline after deadline

[Nadav Amit]

Intel SDM 10.5.4.1 says "When the timer generates an interrupt, it disarms
itself and clears the IA32_TSC_DEADLINE MSR".

This patch clears the MSR upon timer interrupt delivery which delivered on
deadline mode.  Since the MSR may be reconfigured while an interrupt is
pending, causing the new value to be overriden, pending timer interrupts are
checked before setting a new deadline.

Signed-off-by: Nadav Amit <namit@cs.technion.ac.il>
---
 arch/x86/kvm/lapic.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/arch/x86/kvm/lapic.c b/arch/x86/kvm/lapic.c
index 08e8a89..666c086 100644
--- a/arch/x86/kvm/lapic.c
+++ b/arch/x86/kvm/lapic.c
@@ -1352,6 +1352,9 @@ void kvm_set_lapic_tscdeadline_msr(struct kvm_vcpu *vcpu, u64 data)
 		return;
 
 	hrtimer_cancel(&apic->lapic_timer.timer);
+	/* Inject here so clearing tscdeadline won't override new value */
+	if (apic_has_pending_timer(vcpu))
+		kvm_inject_apic_timer_irqs(vcpu);
 	apic->lapic_timer.tscdeadline = data;
 	start_apic_timer(apic);
 }
@@ -1639,6 +1642,8 @@ void kvm_inject_apic_timer_irqs(struct kvm_vcpu *vcpu)
 
 	if (atomic_read(&apic->lapic_timer.pending) > 0) {
 		kvm_apic_local_deliver(apic, APIC_LVTT);
+		if (apic_lvtt_tscdeadline(apic))
+			apic->lapic_timer.tscdeadline = 0;
 		atomic_set(&apic->lapic_timer.pending, 0);
 	}
 }
-- 
1.9.1

[PATCH kvm-unit-tests 1/2] x86: Use host CPU parameter for apic test

[Nadav Amit]

Currently, the TSC deadline test never runs, since TSC deadline is disabled
unless the host cpu parameter is used. This patch changes the apic test to use
the qemu host cpu parameter.

Signed-off-by: Nadav Amit <namit@cs.technion.ac.il>
---
 x86/unittests.cfg | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/x86/unittests.cfg b/x86/unittests.cfg
index 6d3e23a..f692b2b 100644
--- a/x86/unittests.cfg
+++ b/x86/unittests.cfg
@@ -9,7 +9,7 @@
 [apic]
 file = apic.flat
 smp = 2
-extra_params = -cpu qemu64,+x2apic
+extra_params = -cpu host,+x2apic
 arch = x86_64
 
 [smptest]
-- 
1.9.1

[PATCH kvm-unit-tests 2/2] x86: Check deadline counter is cleared after interrupt

[Nadav Amit]

Once the local-apic timer is configured to use TSC deadline, the deadline
should be cleared after the deadline passes.  This patch adds a check of this
behavior.

Signed-off-by: Nadav Amit <namit@cs.technion.ac.il>
---
 x86/apic.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/x86/apic.c b/x86/apic.c
index 487c248..3f463a5 100644
--- a/x86/apic.c
+++ b/x86/apic.c
@@ -35,6 +35,7 @@ static void start_tsc_deadline_timer(void)
     wrmsr(MSR_IA32_TSCDEADLINE, rdmsr(MSR_IA32_TSC));
     asm volatile ("nop");
     report("tsc deadline timer", tdt_count == 1);
+    report("tsc deadline timer clearing", rdmsr(MSR_IA32_TSCDEADLINE) == 0);
 }
 
 static int enable_tsc_deadline_timer(void)
-- 
1.9.1

Re: [PATCH kvm-unit-tests 1/2] x86: Use host CPU parameter for apic test

[Paolo Bonzini]

Il 18/08/2014 21:43, Nadav Amit ha scritto:
> Currently, the TSC deadline test never runs, since TSC deadline is disabled
> unless the host cpu parameter is used. This patch changes the apic test to use
> the qemu host cpu parameter.

Better use
> Signed-off-by: Nadav Amit <namit@cs.technion.ac.il>
> ---
>  x86/unittests.cfg | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/x86/unittests.cfg b/x86/unittests.cfg
> index 6d3e23a..f692b2b 100644
> --- a/x86/unittests.cfg
> +++ b/x86/unittests.cfg
> @@ -9,7 +9,7 @@
>  [apic]
>  file = apic.flat
>  smp = 2
> -extra_params = -cpu qemu64,+x2apic
> +extra_params = -cpu host,+x2apic
>  arch = x86_64
>  
>  [smptest]
> 

Eduardo, I think we should add tsc_deadline to QEMU instead?

Paolo

Re: [PATCH kvm-unit-tests 2/2] x86: Check deadline counter is cleared after interrupt

[Paolo Bonzini]

Il 18/08/2014 21:43, Nadav Amit ha scritto:
> Once the local-apic timer is configured to use TSC deadline, the deadline
> should be cleared after the deadline passes.  This patch adds a check of this
> behavior.
> 
> Signed-off-by: Nadav Amit <namit@cs.technion.ac.il>
> ---
>  x86/apic.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/x86/apic.c b/x86/apic.c
> index 487c248..3f463a5 100644
> --- a/x86/apic.c
> +++ b/x86/apic.c
> @@ -35,6 +35,7 @@ static void start_tsc_deadline_timer(void)
>      wrmsr(MSR_IA32_TSCDEADLINE, rdmsr(MSR_IA32_TSC));
>      asm volatile ("nop");
>      report("tsc deadline timer", tdt_count == 1);
> +    report("tsc deadline timer clearing", rdmsr(MSR_IA32_TSCDEADLINE) == 0);
>  }
>  
>  static int enable_tsc_deadline_timer(void)
> 

Thanks, applying this patch.

Paolo

Re: [PATCH] KVM: x86: Clear apic tsc-deadline after deadline

[Paolo Bonzini]

Il 18/08/2014 21:42, Nadav Amit ha scritto:
> Intel SDM 10.5.4.1 says "When the timer generates an interrupt, it disarms
> itself and clears the IA32_TSC_DEADLINE MSR".
> 
> This patch clears the MSR upon timer interrupt delivery which delivered on
> deadline mode.  Since the MSR may be reconfigured while an interrupt is
> pending, causing the new value to be overriden, pending timer interrupts are
> checked before setting a new deadline.
> 
> Signed-off-by: Nadav Amit <namit@cs.technion.ac.il>
> ---
>  arch/x86/kvm/lapic.c | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/arch/x86/kvm/lapic.c b/arch/x86/kvm/lapic.c
> index 08e8a89..666c086 100644
> --- a/arch/x86/kvm/lapic.c
> +++ b/arch/x86/kvm/lapic.c
> @@ -1352,6 +1352,9 @@ void kvm_set_lapic_tscdeadline_msr(struct kvm_vcpu *vcpu, u64 data)
>  		return;
>  
>  	hrtimer_cancel(&apic->lapic_timer.timer);
> +	/* Inject here so clearing tscdeadline won't override new value */
> +	if (apic_has_pending_timer(vcpu))
> +		kvm_inject_apic_timer_irqs(vcpu);
>  	apic->lapic_timer.tscdeadline = data;
>  	start_apic_timer(apic);
>  }
> @@ -1639,6 +1642,8 @@ void kvm_inject_apic_timer_irqs(struct kvm_vcpu *vcpu)
>  
>  	if (atomic_read(&apic->lapic_timer.pending) > 0) {
>  		kvm_apic_local_deliver(apic, APIC_LVTT);
> +		if (apic_lvtt_tscdeadline(apic))
> +			apic->lapic_timer.tscdeadline = 0;
>  		atomic_set(&apic->lapic_timer.pending, 0);
>  	}
>  }
> 

Applied, thanks.  Also applied patch 2 to kvm-unit-tests.

Paolo

Re: [PATCH kvm-unit-tests 1/2] x86: Use host CPU parameter for apic test

[Eduardo Habkost]

On Tue, Aug 19, 2014 at 11:51:05AM +0200, Paolo Bonzini wrote:
> Il 18/08/2014 21:43, Nadav Amit ha scritto:
> > Currently, the TSC deadline test never runs, since TSC deadline is disabled
> > unless the host cpu parameter is used. This patch changes the apic test to use
> > the qemu host cpu parameter.
> 
> Better use
> > Signed-off-by: Nadav Amit <namit@cs.technion.ac.il>
> > ---
> >  x86/unittests.cfg | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/x86/unittests.cfg b/x86/unittests.cfg
> > index 6d3e23a..f692b2b 100644
> > --- a/x86/unittests.cfg
> > +++ b/x86/unittests.cfg
> > @@ -9,7 +9,7 @@
> >  [apic]
> >  file = apic.flat
> >  smp = 2
> > -extra_params = -cpu qemu64,+x2apic
> > +extra_params = -cpu host,+x2apic
> >  arch = x86_64
> >  
> >  [smptest]
> > 
> 
> Eduardo, I think we should add tsc_deadline to QEMU instead?

"+tsc-deadline" is supported by QEMU since v1.1.0.

-- 
Eduardo

Re: [PATCH kvm-unit-tests 1/2] x86: Use host CPU parameter for apic test

[Paolo Bonzini]

Il 19/08/2014 15:18, Eduardo Habkost ha scritto:
> On Tue, Aug 19, 2014 at 11:51:05AM +0200, Paolo Bonzini wrote:
>> Il 18/08/2014 21:43, Nadav Amit ha scritto:
>>> Currently, the TSC deadline test never runs, since TSC deadline is disabled
>>> unless the host cpu parameter is used. This patch changes the apic test to use
>>> the qemu host cpu parameter.
>>
>> Better use
>>> Signed-off-by: Nadav Amit <namit@cs.technion.ac.il>
>>> ---
>>>  x86/unittests.cfg | 2 +-
>>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/x86/unittests.cfg b/x86/unittests.cfg
>>> index 6d3e23a..f692b2b 100644
>>> --- a/x86/unittests.cfg
>>> +++ b/x86/unittests.cfg
>>> @@ -9,7 +9,7 @@
>>>  [apic]
>>>  file = apic.flat
>>>  smp = 2
>>> -extra_params = -cpu qemu64,+x2apic
>>> +extra_params = -cpu host,+x2apic
>>>  arch = x86_64
>>>  
>>>  [smptest]
>>>
>>
>> Eduardo, I think we should add tsc_deadline to QEMU instead?
> 
> "+tsc-deadline" is supported by QEMU since v1.1.0.

Thanks, I applied this:

diff --git a/x86/unittests.cfg b/x86/unittests.cfg
index 6d3e23a..0123944 100644
--- a/x86/unittests.cfg
+++ b/x86/unittests.cfg
@@ -9,7 +9,7 @@
 [apic]
 file = apic.flat
 smp = 2
-extra_params = -cpu qemu64,+x2apic
+extra_params = -cpu qemu64,+x2apic,+tsc-deadline
 arch = x86_64

 [smptest]

Paolo