* KVM: x86: disable master clock if TSC is reset during suspend
@ 2014-05-14 7:26 Marcelo Tosatti
2014-05-14 9:09 ` Paolo Bonzini
0 siblings, 1 reply; 9+ messages in thread
From: Marcelo Tosatti @ 2014-05-14 7:26 UTC (permalink / raw)
To: kvm-devel; +Cc: Paolo Bonzini, Alexander Graf
Updating system_time from the kernel clock once master clock
has been enabled can result in time backwards event, in case
kernel clock frequency is lower than TSC frequency.
Disable master clock in case its necessary to update it
from the resume path.
Signed-off-by: Marcelo Tosatti <mtosatti@redhat.com>
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index de0931c..9f5dd40 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -106,6 +106,8 @@ EXPORT_SYMBOL_GPL(kvm_max_guest_tsc_khz);
static u32 tsc_tolerance_ppm = 250;
module_param(tsc_tolerance_ppm, uint, S_IRUGO | S_IWUSR);
+static bool suspend_backwards_tsc = false;
+
#define KVM_NR_SHARED_MSRS 16
struct kvm_shared_msrs_global {
@@ -1472,6 +1474,8 @@ static void pvclock_update_vm_gtod_copy(struct kvm *kvm)
&ka->master_cycle_now);
ka->use_master_clock = host_tsc_clocksource & vcpus_matched;
+ if (suspend_backwards_tsc)
+ ka->use_master_clock = 0;
if (ka->use_master_clock)
atomic_set(&kvm_guest_has_master_clock, 1);
@@ -6939,6 +6943,7 @@ int kvm_arch_hardware_enable(void *garbage)
*/
if (backwards_tsc) {
u64 delta_cyc = max_tsc - local_tsc;
+ suspend_backwards_tsc = true;
list_for_each_entry(kvm, &vm_list, vm_list) {
kvm_for_each_vcpu(i, vcpu, kvm) {
vcpu->arch.tsc_offset_adjustment += delta_cyc;
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: KVM: x86: disable master clock if TSC is reset during suspend
2014-05-14 7:26 KVM: x86: disable master clock if TSC is reset during suspend Marcelo Tosatti
@ 2014-05-14 9:09 ` Paolo Bonzini
2014-05-14 10:05 ` Marcelo Tosatti
2014-05-14 10:06 ` KVM: x86: disable master clock if TSC is reset during suspend (v2) Marcelo Tosatti
0 siblings, 2 replies; 9+ messages in thread
From: Paolo Bonzini @ 2014-05-14 9:09 UTC (permalink / raw)
To: Marcelo Tosatti, kvm-devel; +Cc: Alexander Graf
Il 14/05/2014 09:26, Marcelo Tosatti ha scritto:
>
> Updating system_time from the kernel clock once master clock
> has been enabled can result in time backwards event, in case
> kernel clock frequency is lower than TSC frequency.
>
> Disable master clock in case its necessary to update it
> from the resume path.
>
> Signed-off-by: Marcelo Tosatti <mtosatti@redhat.com>
>
> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> index de0931c..9f5dd40 100644
> --- a/arch/x86/kvm/x86.c
> +++ b/arch/x86/kvm/x86.c
> @@ -106,6 +106,8 @@ EXPORT_SYMBOL_GPL(kvm_max_guest_tsc_khz);
> static u32 tsc_tolerance_ppm = 250;
> module_param(tsc_tolerance_ppm, uint, S_IRUGO | S_IWUSR);
>
> +static bool suspend_backwards_tsc = false;
> +
> #define KVM_NR_SHARED_MSRS 16
>
> struct kvm_shared_msrs_global {
> @@ -1472,6 +1474,8 @@ static void pvclock_update_vm_gtod_copy(struct kvm *kvm)
> &ka->master_cycle_now);
>
> ka->use_master_clock = host_tsc_clocksource & vcpus_matched;
> + if (suspend_backwards_tsc)
> + ka->use_master_clock = 0;
Use an && instead of an "if"? (Also, why "&" in the other assignment of
ka->use_master_clock)?
> if (ka->use_master_clock)
> atomic_set(&kvm_guest_has_master_clock, 1);
> @@ -6939,6 +6943,7 @@ int kvm_arch_hardware_enable(void *garbage)
> */
> if (backwards_tsc) {
> u64 delta_cyc = max_tsc - local_tsc;
> + suspend_backwards_tsc = true;
> list_for_each_entry(kvm, &vm_list, vm_list) {
> kvm_for_each_vcpu(i, vcpu, kvm) {
> vcpu->arch.tsc_offset_adjustment += delta_cyc;
>
Why a global and not a variable inside kvm? The same question holds for
kvm_guest_has_master_clock.
Paolo
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: KVM: x86: disable master clock if TSC is reset during suspend
2014-05-14 9:09 ` Paolo Bonzini
@ 2014-05-14 10:05 ` Marcelo Tosatti
2014-05-14 10:06 ` KVM: x86: disable master clock if TSC is reset during suspend (v2) Marcelo Tosatti
1 sibling, 0 replies; 9+ messages in thread
From: Marcelo Tosatti @ 2014-05-14 10:05 UTC (permalink / raw)
To: Paolo Bonzini; +Cc: kvm-devel, Alexander Graf
On Wed, May 14, 2014 at 11:09:36AM +0200, Paolo Bonzini wrote:
> Il 14/05/2014 09:26, Marcelo Tosatti ha scritto:
> >
> >Updating system_time from the kernel clock once master clock
> >has been enabled can result in time backwards event, in case
> >kernel clock frequency is lower than TSC frequency.
> >
> >Disable master clock in case its necessary to update it
> >from the resume path.
> >
> >Signed-off-by: Marcelo Tosatti <mtosatti@redhat.com>
> >
> >diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> >index de0931c..9f5dd40 100644
> >--- a/arch/x86/kvm/x86.c
> >+++ b/arch/x86/kvm/x86.c
> >@@ -106,6 +106,8 @@ EXPORT_SYMBOL_GPL(kvm_max_guest_tsc_khz);
> > static u32 tsc_tolerance_ppm = 250;
> > module_param(tsc_tolerance_ppm, uint, S_IRUGO | S_IWUSR);
> >
> >+static bool suspend_backwards_tsc = false;
> >+
> > #define KVM_NR_SHARED_MSRS 16
> >
> > struct kvm_shared_msrs_global {
> >@@ -1472,6 +1474,8 @@ static void pvclock_update_vm_gtod_copy(struct kvm *kvm)
> > &ka->master_cycle_now);
> >
> > ka->use_master_clock = host_tsc_clocksource & vcpus_matched;
> >+ if (suspend_backwards_tsc)
> >+ ka->use_master_clock = 0;
>
> Use an && instead of an "if"? (Also, why "&" in the other
> assignment of ka->use_master_clock)?
I was asking myself the same.
> > if (ka->use_master_clock)
> > atomic_set(&kvm_guest_has_master_clock, 1);
> >@@ -6939,6 +6943,7 @@ int kvm_arch_hardware_enable(void *garbage)
> > */
> > if (backwards_tsc) {
> > u64 delta_cyc = max_tsc - local_tsc;
> >+ suspend_backwards_tsc = true;
> > list_for_each_entry(kvm, &vm_list, vm_list) {
> > kvm_for_each_vcpu(i, vcpu, kvm) {
> > vcpu->arch.tsc_offset_adjustment += delta_cyc;
> >
>
> Why a global and not a variable inside kvm? The same question holds
> for kvm_guest_has_master_clock.
Because these are global properties: reset-tsc-on-suspend is global,
kvm_guest_has_master_clock is global.
^ permalink raw reply [flat|nested] 9+ messages in thread
* KVM: x86: disable master clock if TSC is reset during suspend (v2)
2014-05-14 9:09 ` Paolo Bonzini
2014-05-14 10:05 ` Marcelo Tosatti
@ 2014-05-14 10:06 ` Marcelo Tosatti
2014-05-14 10:21 ` Paolo Bonzini
1 sibling, 1 reply; 9+ messages in thread
From: Marcelo Tosatti @ 2014-05-14 10:06 UTC (permalink / raw)
To: Paolo Bonzini; +Cc: kvm-devel, Alexander Graf
Updating system_time from the kernel clock once master clock
has been enabled can result in time backwards event, in case
kernel clock frequency is lower than TSC frequency.
Disable master clock in case its necessary to update it
from the resume path.
Signed-off-by: Marcelo Tosatti <mtosatti@redhat.com>
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index de0931c..32eec77 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -106,6 +106,8 @@ EXPORT_SYMBOL_GPL(kvm_max_guest_tsc_khz);
static u32 tsc_tolerance_ppm = 250;
module_param(tsc_tolerance_ppm, uint, S_IRUGO | S_IWUSR);
+static bool suspend_backwards_tsc = false;
+
#define KVM_NR_SHARED_MSRS 16
struct kvm_shared_msrs_global {
@@ -1471,7 +1473,8 @@ static void pvclock_update_vm_gtod_copy(struct kvm *kvm)
&ka->master_kernel_ns,
&ka->master_cycle_now);
- ka->use_master_clock = host_tsc_clocksource & vcpus_matched;
+ ka->use_master_clock = host_tsc_clocksource && vcpus_matched
+ && suspend_backwards_tsc;
if (ka->use_master_clock)
atomic_set(&kvm_guest_has_master_clock, 1);
@@ -6939,6 +6942,7 @@ int kvm_arch_hardware_enable(void *garbage)
*/
if (backwards_tsc) {
u64 delta_cyc = max_tsc - local_tsc;
+ suspend_backwards_tsc = true;
list_for_each_entry(kvm, &vm_list, vm_list) {
kvm_for_each_vcpu(i, vcpu, kvm) {
vcpu->arch.tsc_offset_adjustment += delta_cyc;
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: KVM: x86: disable master clock if TSC is reset during suspend (v2)
2014-05-14 10:06 ` KVM: x86: disable master clock if TSC is reset during suspend (v2) Marcelo Tosatti
@ 2014-05-14 10:21 ` Paolo Bonzini
2014-05-14 15:43 ` KVM: x86: disable master clock if TSC is reset during suspend (v3) Marcelo Tosatti
0 siblings, 1 reply; 9+ messages in thread
From: Paolo Bonzini @ 2014-05-14 10:21 UTC (permalink / raw)
To: Marcelo Tosatti; +Cc: kvm-devel, Alexander Graf
Il 14/05/2014 12:06, Marcelo Tosatti ha scritto:
>
> Updating system_time from the kernel clock once master clock
> has been enabled can result in time backwards event, in case
> kernel clock frequency is lower than TSC frequency.
>
> Disable master clock in case its necessary to update it
> from the resume path.
>
> Signed-off-by: Marcelo Tosatti <mtosatti@redhat.com>
>
> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> index de0931c..32eec77 100644
> --- a/arch/x86/kvm/x86.c
> +++ b/arch/x86/kvm/x86.c
> @@ -106,6 +106,8 @@ EXPORT_SYMBOL_GPL(kvm_max_guest_tsc_khz);
> static u32 tsc_tolerance_ppm = 250;
> module_param(tsc_tolerance_ppm, uint, S_IRUGO | S_IWUSR);
>
> +static bool suspend_backwards_tsc = false;
> +
> #define KVM_NR_SHARED_MSRS 16
>
> struct kvm_shared_msrs_global {
> @@ -1471,7 +1473,8 @@ static void pvclock_update_vm_gtod_copy(struct kvm *kvm)
> &ka->master_kernel_ns,
> &ka->master_cycle_now);
>
> - ka->use_master_clock = host_tsc_clocksource & vcpus_matched;
> + ka->use_master_clock = host_tsc_clocksource && vcpus_matched
> + && suspend_backwards_tsc;
Missing "!" here. Also, perhaps "backwards_tsc_observed" is a better
name for the variable?
Paolo
>
> if (ka->use_master_clock)
> atomic_set(&kvm_guest_has_master_clock, 1);
> @@ -6939,6 +6942,7 @@ int kvm_arch_hardware_enable(void *garbage)
> */
> if (backwards_tsc) {
> u64 delta_cyc = max_tsc - local_tsc;
> + suspend_backwards_tsc = true;
> list_for_each_entry(kvm, &vm_list, vm_list) {
> kvm_for_each_vcpu(i, vcpu, kvm) {
> vcpu->arch.tsc_offset_adjustment += delta_cyc;
>
^ permalink raw reply [flat|nested] 9+ messages in thread
* KVM: x86: disable master clock if TSC is reset during suspend (v3)
2014-05-14 10:21 ` Paolo Bonzini
@ 2014-05-14 15:43 ` Marcelo Tosatti
2014-05-14 16:00 ` Paolo Bonzini
0 siblings, 1 reply; 9+ messages in thread
From: Marcelo Tosatti @ 2014-05-14 15:43 UTC (permalink / raw)
To: Paolo Bonzini; +Cc: kvm-devel, Alexander Graf
Updating system_time from the kernel clock once master clock
has been enabled can result in time backwards event, in case
kernel clock frequency is lower than TSC frequency.
Disable master clock in case its necessary to update it
from the resume path.
Signed-off-by: Marcelo Tosatti <mtosatti@redhat.com>
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index de0931c..c098728 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -106,6 +106,8 @@ EXPORT_SYMBOL_GPL(kvm_max_guest_tsc_khz);
static u32 tsc_tolerance_ppm = 250;
module_param(tsc_tolerance_ppm, uint, S_IRUGO | S_IWUSR);
+static bool backwards_tsc_observed = false;
+
#define KVM_NR_SHARED_MSRS 16
struct kvm_shared_msrs_global {
@@ -1471,7 +1473,8 @@ static void pvclock_update_vm_gtod_copy(struct kvm *kvm)
&ka->master_kernel_ns,
&ka->master_cycle_now);
- ka->use_master_clock = host_tsc_clocksource & vcpus_matched;
+ ka->use_master_clock = host_tsc_clocksource && vcpus_matched
+ && !backwards_tsc_observed;
if (ka->use_master_clock)
atomic_set(&kvm_guest_has_master_clock, 1);
@@ -6939,6 +6942,7 @@ int kvm_arch_hardware_enable(void *garbage)
*/
if (backwards_tsc) {
u64 delta_cyc = max_tsc - local_tsc;
+ backwards_tsc_observed = true;
list_for_each_entry(kvm, &vm_list, vm_list) {
kvm_for_each_vcpu(i, vcpu, kvm) {
vcpu->arch.tsc_offset_adjustment += delta_cyc;
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: KVM: x86: disable master clock if TSC is reset during suspend (v3)
2014-05-14 15:43 ` KVM: x86: disable master clock if TSC is reset during suspend (v3) Marcelo Tosatti
@ 2014-05-14 16:00 ` Paolo Bonzini
2014-05-21 8:26 ` Alexander Graf
0 siblings, 1 reply; 9+ messages in thread
From: Paolo Bonzini @ 2014-05-14 16:00 UTC (permalink / raw)
To: Marcelo Tosatti; +Cc: kvm-devel, Alexander Graf
Il 14/05/2014 17:43, Marcelo Tosatti ha scritto:
>
> Updating system_time from the kernel clock once master clock
> has been enabled can result in time backwards event, in case
> kernel clock frequency is lower than TSC frequency.
>
> Disable master clock in case its necessary to update it
> from the resume path.
>
> Signed-off-by: Marcelo Tosatti <mtosatti@redhat.com>
>
> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> index de0931c..c098728 100644
> --- a/arch/x86/kvm/x86.c
> +++ b/arch/x86/kvm/x86.c
> @@ -106,6 +106,8 @@ EXPORT_SYMBOL_GPL(kvm_max_guest_tsc_khz);
> static u32 tsc_tolerance_ppm = 250;
> module_param(tsc_tolerance_ppm, uint, S_IRUGO | S_IWUSR);
>
> +static bool backwards_tsc_observed = false;
> +
> #define KVM_NR_SHARED_MSRS 16
>
> struct kvm_shared_msrs_global {
> @@ -1471,7 +1473,8 @@ static void pvclock_update_vm_gtod_copy(struct kvm *kvm)
> &ka->master_kernel_ns,
> &ka->master_cycle_now);
>
> - ka->use_master_clock = host_tsc_clocksource & vcpus_matched;
> + ka->use_master_clock = host_tsc_clocksource && vcpus_matched
> + && !backwards_tsc_observed;
>
> if (ka->use_master_clock)
> atomic_set(&kvm_guest_has_master_clock, 1);
> @@ -6939,6 +6942,7 @@ int kvm_arch_hardware_enable(void *garbage)
> */
> if (backwards_tsc) {
> u64 delta_cyc = max_tsc - local_tsc;
> + backwards_tsc_observed = true;
> list_for_each_entry(kvm, &vm_list, vm_list) {
> kvm_for_each_vcpu(i, vcpu, kvm) {
> vcpu->arch.tsc_offset_adjustment += delta_cyc;
>
Applying to kvm/master, thanks.
Paolo
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: KVM: x86: disable master clock if TSC is reset during suspend (v3)
2014-05-14 16:00 ` Paolo Bonzini
@ 2014-05-21 8:26 ` Alexander Graf
2014-05-21 10:01 ` Paolo Bonzini
0 siblings, 1 reply; 9+ messages in thread
From: Alexander Graf @ 2014-05-21 8:26 UTC (permalink / raw)
To: Paolo Bonzini, Marcelo Tosatti; +Cc: kvm-devel
On 14.05.14 18:00, Paolo Bonzini wrote:
> Il 14/05/2014 17:43, Marcelo Tosatti ha scritto:
>>
>> Updating system_time from the kernel clock once master clock
>> has been enabled can result in time backwards event, in case
>> kernel clock frequency is lower than TSC frequency.
>>
>> Disable master clock in case its necessary to update it
>> from the resume path.
>>
>> Signed-off-by: Marcelo Tosatti <mtosatti@redhat.com>
>>
>> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
>> index de0931c..c098728 100644
>> --- a/arch/x86/kvm/x86.c
>> +++ b/arch/x86/kvm/x86.c
>> @@ -106,6 +106,8 @@ EXPORT_SYMBOL_GPL(kvm_max_guest_tsc_khz);
>> static u32 tsc_tolerance_ppm = 250;
>> module_param(tsc_tolerance_ppm, uint, S_IRUGO | S_IWUSR);
>>
>> +static bool backwards_tsc_observed = false;
>> +
>> #define KVM_NR_SHARED_MSRS 16
>>
>> struct kvm_shared_msrs_global {
>> @@ -1471,7 +1473,8 @@ static void pvclock_update_vm_gtod_copy(struct
>> kvm *kvm)
>> &ka->master_kernel_ns,
>> &ka->master_cycle_now);
>>
>> - ka->use_master_clock = host_tsc_clocksource & vcpus_matched;
>> + ka->use_master_clock = host_tsc_clocksource && vcpus_matched
>> + && !backwards_tsc_observed;
>>
>> if (ka->use_master_clock)
>> atomic_set(&kvm_guest_has_master_clock, 1);
>> @@ -6939,6 +6942,7 @@ int kvm_arch_hardware_enable(void *garbage)
>> */
>> if (backwards_tsc) {
>> u64 delta_cyc = max_tsc - local_tsc;
>> + backwards_tsc_observed = true;
>> list_for_each_entry(kvm, &vm_list, vm_list) {
>> kvm_for_each_vcpu(i, vcpu, kvm) {
>> vcpu->arch.tsc_offset_adjustment += delta_cyc;
>>
>
> Applying to kvm/master, thanks.
Thanks. Can you please make sure to CC qemu-stable when sending a pull
request?
Alex
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: KVM: x86: disable master clock if TSC is reset during suspend (v3)
2014-05-21 8:26 ` Alexander Graf
@ 2014-05-21 10:01 ` Paolo Bonzini
0 siblings, 0 replies; 9+ messages in thread
From: Paolo Bonzini @ 2014-05-21 10:01 UTC (permalink / raw)
To: Alexander Graf, Marcelo Tosatti; +Cc: kvm-devel
Il 21/05/2014 10:26, Alexander Graf ha scritto:
>
> On 14.05.14 18:00, Paolo Bonzini wrote:
>> Il 14/05/2014 17:43, Marcelo Tosatti ha scritto:
>>>
>>> Updating system_time from the kernel clock once master clock
>>> has been enabled can result in time backwards event, in case
>>> kernel clock frequency is lower than TSC frequency.
>>>
>>> Disable master clock in case its necessary to update it
>>> from the resume path.
>>>
>>> Signed-off-by: Marcelo Tosatti <mtosatti@redhat.com>
>>>
>>> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
>>> index de0931c..c098728 100644
>>> --- a/arch/x86/kvm/x86.c
>>> +++ b/arch/x86/kvm/x86.c
>>> @@ -106,6 +106,8 @@ EXPORT_SYMBOL_GPL(kvm_max_guest_tsc_khz);
>>> static u32 tsc_tolerance_ppm = 250;
>>> module_param(tsc_tolerance_ppm, uint, S_IRUGO | S_IWUSR);
>>>
>>> +static bool backwards_tsc_observed = false;
>>> +
>>> #define KVM_NR_SHARED_MSRS 16
>>>
>>> struct kvm_shared_msrs_global {
>>> @@ -1471,7 +1473,8 @@ static void pvclock_update_vm_gtod_copy(struct
>>> kvm *kvm)
>>> &ka->master_kernel_ns,
>>> &ka->master_cycle_now);
>>>
>>> - ka->use_master_clock = host_tsc_clocksource & vcpus_matched;
>>> + ka->use_master_clock = host_tsc_clocksource && vcpus_matched
>>> + && !backwards_tsc_observed;
>>>
>>> if (ka->use_master_clock)
>>> atomic_set(&kvm_guest_has_master_clock, 1);
>>> @@ -6939,6 +6942,7 @@ int kvm_arch_hardware_enable(void *garbage)
>>> */
>>> if (backwards_tsc) {
>>> u64 delta_cyc = max_tsc - local_tsc;
>>> + backwards_tsc_observed = true;
>>> list_for_each_entry(kvm, &vm_list, vm_list) {
>>> kvm_for_each_vcpu(i, vcpu, kvm) {
>>> vcpu->arch.tsc_offset_adjustment += delta_cyc;
>>>
>>
>> Applying to kvm/master, thanks.
>
> Thanks. Can you please make sure to CC qemu-stable when sending a pull
> request?
Now I will.
Paolo :)
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2014-05-21 10:01 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-05-14 7:26 KVM: x86: disable master clock if TSC is reset during suspend Marcelo Tosatti
2014-05-14 9:09 ` Paolo Bonzini
2014-05-14 10:05 ` Marcelo Tosatti
2014-05-14 10:06 ` KVM: x86: disable master clock if TSC is reset during suspend (v2) Marcelo Tosatti
2014-05-14 10:21 ` Paolo Bonzini
2014-05-14 15:43 ` KVM: x86: disable master clock if TSC is reset during suspend (v3) Marcelo Tosatti
2014-05-14 16:00 ` Paolo Bonzini
2014-05-21 8:26 ` Alexander Graf
2014-05-21 10:01 ` Paolo Bonzini
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).