* [PATCH v2] KVM: x86: update masterclock values on TSC writes
@ 2014-11-04 23:30 Marcelo Tosatti
2014-11-05 9:36 ` Paolo Bonzini
0 siblings, 1 reply; 6+ messages in thread
From: Marcelo Tosatti @ 2014-11-04 23:30 UTC (permalink / raw)
To: Paolo Bonzini; +Cc: kvm-devel
When the guest writes to the TSC, the masterclock TSC copy must be
updated as well along with the TSC_OFFSET update, otherwise a negative
tsc_timestamp is calculated at kvm_guest_time_update.
Signed-off-by: Marcelo Tosatti <mtosatti@redhat.com>
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index 0033df3..50b6f56 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -1237,21 +1237,21 @@ void kvm_track_tsc_matching(struct kvm_vcpu *vcpu)
{
#ifdef CONFIG_X86_64
bool vcpus_matched;
- bool do_request = false;
struct kvm_arch *ka = &vcpu->kvm->arch;
struct pvclock_gtod_data *gtod = &pvclock_gtod_data;
vcpus_matched = (ka->nr_vcpus_matched_tsc + 1 ==
atomic_read(&vcpu->kvm->online_vcpus));
- if (vcpus_matched && gtod->clock.vclock_mode == VCLOCK_TSC)
- if (!ka->use_master_clock)
- do_request = 1;
-
- if (!vcpus_matched && ka->use_master_clock)
- do_request = 1;
-
- if (do_request)
+ /*
+ * If the vcpus have matched TSCs and host clocksource is TSC,
+ * perform request to enable masterclock.
+ *
+ * If the masterclock is enabled, perform request to update
+ * masterclock values.
+ */
+ if ((vcpus_matched && gtod->clock.vclock_mode == VCLOCK_TSC) ||
+ ka->use_master_clock)
kvm_make_request(KVM_REQ_MASTERCLOCK_UPDATE, vcpu);
trace_kvm_track_tsc(vcpu->vcpu_id, ka->nr_vcpus_matched_tsc,
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH v2] KVM: x86: update masterclock values on TSC writes
2014-11-04 23:30 [PATCH v2] KVM: x86: update masterclock values on TSC writes Marcelo Tosatti
@ 2014-11-05 9:36 ` Paolo Bonzini
2014-11-05 18:19 ` Marcelo Tosatti
0 siblings, 1 reply; 6+ messages in thread
From: Paolo Bonzini @ 2014-11-05 9:36 UTC (permalink / raw)
To: Marcelo Tosatti; +Cc: kvm-devel
On 05/11/2014 00:30, Marcelo Tosatti wrote:
> + /*
> + * If the vcpus have matched TSCs and host clocksource is TSC,
> + * perform request to enable masterclock.
> + *
> + * If the masterclock is enabled, perform request to update
> + * masterclock values.
> + */
> + if ((vcpus_matched && gtod->clock.vclock_mode == VCLOCK_TSC) ||
> + ka->use_master_clock)
This is not an explanation, it is a literal translation from C to
English. :) Can you also explain the why, especially for the first half
of the condition?
Paolo
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2] KVM: x86: update masterclock values on TSC writes
2014-11-05 9:36 ` Paolo Bonzini
@ 2014-11-05 18:19 ` Marcelo Tosatti
2014-11-05 19:47 ` Paolo Bonzini
0 siblings, 1 reply; 6+ messages in thread
From: Marcelo Tosatti @ 2014-11-05 18:19 UTC (permalink / raw)
To: Paolo Bonzini; +Cc: kvm-devel
On Wed, Nov 05, 2014 at 10:36:05AM +0100, Paolo Bonzini wrote:
> On 05/11/2014 00:30, Marcelo Tosatti wrote:
> > + /*
> > + * If the vcpus have matched TSCs and host clocksource is TSC,
> > + * perform request to enable masterclock.
> > + *
> > + * If the masterclock is enabled, perform request to update
> > + * masterclock values.
> > + */
> > + if ((vcpus_matched && gtod->clock.vclock_mode == VCLOCK_TSC) ||
> > + ka->use_master_clock)
>
> This is not an explanation, it is a literal translation from C to
> English. :) Can you also explain the why, especially for the first half
> of the condition?
>
> Paolo
The comment on top of pvclock_update_vm_gtod_copy says:
"Rely on synchronization of host TSCs and guest TSCs for monotonicity."
Then with this patch
"If the vcpus have matched TSCs and host clocksource is TSC"
Is it sufficient to add
"Masterclock requires synchronized guest TSC and host clocksource TSC"
to this patch?
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2] KVM: x86: update masterclock values on TSC writes
2014-11-05 18:19 ` Marcelo Tosatti
@ 2014-11-05 19:47 ` Paolo Bonzini
2014-11-05 19:49 ` Paolo Bonzini
0 siblings, 1 reply; 6+ messages in thread
From: Paolo Bonzini @ 2014-11-05 19:47 UTC (permalink / raw)
To: Marcelo Tosatti; +Cc: kvm-devel
On 05/11/2014 19:19, Marcelo Tosatti wrote:
>>> > > + * If the vcpus have matched TSCs and host clocksource is TSC,
>>> > > + * perform request to enable masterclock.
>>> > > + *
>>> > > + * If the masterclock is enabled, perform request to update
>>> > > + * masterclock values.
>>> > > + */
>>> > > + if ((vcpus_matched && gtod->clock.vclock_mode == VCLOCK_TSC) ||
>>> > > + ka->use_master_clock)
>> >
>> > This is not an explanation, it is a literal translation from C to
>> > English. :) Can you also explain the why, especially for the first half
>> > of the condition?
>> >
>> > Paolo
> The comment on top of pvclock_update_vm_gtod_copy says:
>
> "Rely on synchronization of host TSCs and guest TSCs for monotonicity."
>
> Then with this patch
>
> "If the vcpus have matched TSCs and host clocksource is TSC"
>
> Is it sufficient to add
>
> "Masterclock requires synchronized guest TSC and host clocksource TSC"
> to this patch?
Got it now, thanks. I think I misread one "enable" as "update" in your
comment, sorry. I'll apply v2 tomorrow.
Paolo
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2] KVM: x86: update masterclock values on TSC writes
2014-11-05 19:47 ` Paolo Bonzini
@ 2014-11-05 19:49 ` Paolo Bonzini
2014-11-05 19:58 ` Marcelo Tosatti
0 siblings, 1 reply; 6+ messages in thread
From: Paolo Bonzini @ 2014-11-05 19:49 UTC (permalink / raw)
To: Marcelo Tosatti; +Cc: kvm-devel
On 05/11/2014 20:47, Paolo Bonzini wrote:
>
>
> On 05/11/2014 19:19, Marcelo Tosatti wrote:
>>>>>> + * If the vcpus have matched TSCs and host clocksource is TSC,
>>>>>> + * perform request to enable masterclock.
>>>>>> + *
>>>>>> + * If the masterclock is enabled, perform request to update
>>>>>> + * masterclock values.
>>>>>> + */
>>>>>> + if ((vcpus_matched && gtod->clock.vclock_mode == VCLOCK_TSC) ||
>>>>>> + ka->use_master_clock)
>>>>
>>>> This is not an explanation, it is a literal translation from C to
>>>> English. :) Can you also explain the why, especially for the first half
>>>> of the condition?
>>>>
>>>> Paolo
>> The comment on top of pvclock_update_vm_gtod_copy says:
>>
>> "Rely on synchronization of host TSCs and guest TSCs for monotonicity."
>>
>> Then with this patch
>>
>> "If the vcpus have matched TSCs and host clocksource is TSC"
>>
>> Is it sufficient to add
>>
>> "Masterclock requires synchronized guest TSC and host clocksource TSC"
>> to this patch?
>
> Got it now, thanks. I think I misread one "enable" as "update" in your
> comment, sorry. I'll apply v2 tomorrow.
Slightly reworded:
/*
* Once the masterclock is enabled, always perform request in
* order to update it.
*
* Until then, if the vcpus have matched TSCs and host clocksource
* is TSC, perform request to enable masterclock.
*/
if (ka->use_master_clock ||
(vcpus_matched && gtod->clock.vclock_mode == VCLOCK_TSC))
kvm_make_request(KVM_REQ_MASTERCLOCK_UPDATE, vcpu);
Paolo
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH v2] KVM: x86: update masterclock values on TSC writes
2014-11-05 19:49 ` Paolo Bonzini
@ 2014-11-05 19:58 ` Marcelo Tosatti
0 siblings, 0 replies; 6+ messages in thread
From: Marcelo Tosatti @ 2014-11-05 19:58 UTC (permalink / raw)
To: Paolo Bonzini; +Cc: kvm-devel
On Wed, Nov 05, 2014 at 08:49:38PM +0100, Paolo Bonzini wrote:
>
>
> On 05/11/2014 20:47, Paolo Bonzini wrote:
> >
> >
> > On 05/11/2014 19:19, Marcelo Tosatti wrote:
> >>>>>> + * If the vcpus have matched TSCs and host clocksource is TSC,
> >>>>>> + * perform request to enable masterclock.
> >>>>>> + *
> >>>>>> + * If the masterclock is enabled, perform request to update
> >>>>>> + * masterclock values.
> >>>>>> + */
> >>>>>> + if ((vcpus_matched && gtod->clock.vclock_mode == VCLOCK_TSC) ||
> >>>>>> + ka->use_master_clock)
> >>>>
> >>>> This is not an explanation, it is a literal translation from C to
> >>>> English. :) Can you also explain the why, especially for the first half
> >>>> of the condition?
> >>>>
> >>>> Paolo
> >> The comment on top of pvclock_update_vm_gtod_copy says:
> >>
> >> "Rely on synchronization of host TSCs and guest TSCs for monotonicity."
> >>
> >> Then with this patch
> >>
> >> "If the vcpus have matched TSCs and host clocksource is TSC"
> >>
> >> Is it sufficient to add
> >>
> >> "Masterclock requires synchronized guest TSC and host clocksource TSC"
> >> to this patch?
> >
> > Got it now, thanks. I think I misread one "enable" as "update" in your
> > comment, sorry. I'll apply v2 tomorrow.
>
> Slightly reworded:
>
> /*
> * Once the masterclock is enabled, always perform request in
> * order to update it.
> *
> * Until then, if the vcpus have matched TSCs and host clocksource
> * is TSC, perform request to enable masterclock.
> */
> if (ka->use_master_clock ||
> (vcpus_matched && gtod->clock.vclock_mode == VCLOCK_TSC))
> kvm_make_request(KVM_REQ_MASTERCLOCK_UPDATE, vcpu);
>
> Paolo
Looks good, thanks.
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2014-11-05 20:58 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-11-04 23:30 [PATCH v2] KVM: x86: update masterclock values on TSC writes Marcelo Tosatti
2014-11-05 9:36 ` Paolo Bonzini
2014-11-05 18:19 ` Marcelo Tosatti
2014-11-05 19:47 ` Paolo Bonzini
2014-11-05 19:49 ` Paolo Bonzini
2014-11-05 19:58 ` Marcelo Tosatti
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox