pv clock: kvm is incompatible with xen :-(

pv clock: kvm is incompatible with xen :-(

[Gerd Hoffmann]

  Hi,

Tried to use kvmclock with xenner and noticed that the kvmclock
(MSR_KVM_SYSTEM_TIME msr) is incompatible with xen.

kvm guests do this to translate the tsc delta into nsecs:

   #define get_clock(cpu, field) per_cpu(hv_clock, cpu).field

   static inline u64 kvm_get_delta(u64 last_tsc)
   {
           int cpu = smp_processor_id();
           u64 delta = native_read_tsc() - last_tsc;
           return (delta * get_clock(cpu, tsc_to_system_mul)) >> 22;
   }

whereas xen guests do this (64bit version):

   static inline u64 scale_delta(u64 delta, u32 mul_frac, int shift)
   {
           u64 product;

           if (shift < 0)
                   delta >>= -shift;
           else
                   delta <<= shift;

           __asm__ (
                   "mul %%rdx ; shrd $32,%%rdx,%%rax"
                   : "=a" (product) : "0" (delta), "d" ((u64)mul_frac));
           return product;
   }

Note that xen does a 64bit multiply (of the 64bit delta and the 32bit
factor) yielding a 128bit result, then picking bits 32-95 for the 64bit
return value.  In contrast kvm does a simple 64bit multiply, which is
equivalent to using the lowest 64 bits.  Thus kvm is off by factor 2^32,
and that without even considering the ordering of two (shift + multiply)
operations and any rounding errors ...

cheers,
  Gerd

-- 
http://kraxel.fedorapeople.org/xenner/

-------------------------------------------------------------------------
This SF.net email is sponsored by the 2008 JavaOne(SM) Conference 
Don't miss this year's exciting event. There's still time to save $100. 
Use priority code J8TL2D2. 
http://ad.doubleclick.net/clk;198757673;13503038;p?http://java.sun.com/javaone

Re: pv clock: kvm is incompatible with xen :-(

[Avi Kivity]

Gerd Hoffmann wrote:
>   Hi,
>
> Tried to use kvmclock with xenner and noticed that the kvmclock
> (MSR_KVM_SYSTEM_TIME msr) is incompatible with xen.
>
>   

Patches are welcome, especially as kvmclock isn't merged yet, so there 
are no backward compatibility issues.

-- 
Any sufficiently difficult bug is indistinguishable from a feature.


-------------------------------------------------------------------------
This SF.net email is sponsored by the 2008 JavaOne(SM) Conference 
Don't miss this year's exciting event. There's still time to save $100. 
Use priority code J8TL2D2. 
http://ad.doubleclick.net/clk;198757673;13503038;p?http://java.sun.com/javaone

Re: pv clock: kvm is incompatible with xen :-(

[Gerd Hoffmann]

[-- Attachment #1: Type: text/plain, Size: 1169 bytes --]

Avi Kivity wrote:
> Gerd Hoffmann wrote:
>>   Hi,
>>
>> Tried to use kvmclock with xenner and noticed that the kvmclock
>> (MSR_KVM_SYSTEM_TIME msr) is incompatible with xen.
> 
> Patches are welcome, especially as kvmclock isn't merged yet, so there
> are no backward compatibility issues.

Great, so I'll happily go break kvm guests ;)

Patch revision #1 attached.  It changes the way the tsc-delta-scaling
fields are calculated to be compatible with xen.  Code is taken from
xenner (which got it from xen) and adapted a bit.  Host only, kvm guest
side not done (yet).

With that patch applied xen guests with pv clock enabled happily boot to
the login prompt, without complains about time going backwards.  Fine.

Wall clock is off a few hours though.  Oops.

I think the way wall clock and system clock work together in xen (Jeremy
correct me if I'm wrong) is that the wall clock specifies the point in
time where the system clock started going.  As kvm fills in host system
time into the guest system time fields the guest wall clock fields
should be filled with the host boot time timestamp I'd say.

Comments?

  Gerd

-- 
http://kraxel.fedorapeople.org/xenner/

[-- Attachment #2: kvmclock-1.diff --]
[-- Type: text/x-patch, Size: 1664 bytes --]

diff -up kvm-65/kernel/x86.c.fix kvm-65/kernel/x86.c
--- kvm-65/kernel/x86.c.fix	2008-04-06 21:23:07.000000000 +0200
+++ kvm-65/kernel/x86.c	2008-04-11 15:18:23.000000000 +0200
@@ -548,6 +548,44 @@ static void kvm_write_guest_time(struct 
 	mark_page_dirty(v->kvm, vcpu->time >> PAGE_SHIFT);
 }
 
+static uint32_t div_frac(uint32_t dividend, uint32_t divisor)
+{
+	uint32_t quotient, remainder;
+	
+	__asm__ ( "divl %4"
+		  : "=a" (quotient), "=d" (remainder)
+		  : "0" (0), "1" (dividend), "r" (divisor) );
+	return quotient;
+}
+
+static void kvm_set_time_scale(uint32_t tsc_khz, struct kvm_vcpu_time_info *hv_clock)
+{
+	uint64_t nsecs = 1000000000LL;
+	int32_t  shift = 0;
+	uint64_t tps64;
+	uint32_t tps32;
+	
+	tps64 = tsc_khz * 1000LL;
+	while (tps64 > nsecs*2) {
+		tps64 >>= 1;
+		shift--;
+	}
+
+	tps32 = (uint32_t)tps64;
+	while (tps32 <= (uint32_t)nsecs) {
+		tps32 <<= 1;
+		shift++;
+	}
+
+	hv_clock->tsc_shift = shift;
+	hv_clock->tsc_to_system_mul = div_frac(nsecs, tps32);
+
+#if 0
+	printk(KERN_DEBUG "%s: tsc_khz %u, tsc_shift %d, tsc_mul %u\n",
+	       __FUNCTION__, tsc_khz, hv_clock->tsc_shift,
+	       hv_clock->tsc_to_system_mul);
+#endif
+}
 
 int kvm_set_msr_common(struct kvm_vcpu *vcpu, u32 msr, u64 data)
 {
@@ -596,9 +634,7 @@ int kvm_set_msr_common(struct kvm_vcpu *
 		/* ...but clean it before doing the actual write */
 		vcpu->arch.time_offset = data & ~(PAGE_MASK | 1);
 
-		vcpu->arch.hv_clock.tsc_to_system_mul =
-					clocksource_khz2mult(kvm_tsc_khz, 22);
-		vcpu->arch.hv_clock.tsc_shift = 22;
+		kvm_set_time_scale(kvm_tsc_khz, &vcpu->arch.hv_clock);
 
 		down_read(&current->mm->mmap_sem);
 		vcpu->arch.time_page =

[-- Attachment #3: Type: text/plain, Size: 320 bytes --]

-------------------------------------------------------------------------
This SF.net email is sponsored by the 2008 JavaOne(SM) Conference 
Don't miss this year's exciting event. There's still time to save $100. 
Use priority code J8TL2D2. 
http://ad.doubleclick.net/clk;198757673;13503038;p?http://java.sun.com/javaone

[-- Attachment #4: Type: text/plain, Size: 158 bytes --]

_______________________________________________
kvm-devel mailing list
kvm-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/kvm-devel

Re: pv clock: kvm is incompatible with xen :-(

[Gerd Hoffmann]

[-- Attachment #1: Type: text/plain, Size: 992 bytes --]

Gerd Hoffmann wrote:

> Wall clock is off a few hours though.  Oops.
> 
> I think the way wall clock and system clock work together in xen (Jeremy
> correct me if I'm wrong) is that the wall clock specifies the point in
> time where the system clock started going.  As kvm fills in host system
> time into the guest system time fields the guest wall clock fields
> should be filled with the host boot time timestamp I'd say.

Following up myself with a quick&dirty patch to tackle this issue too.
This one calculates the boot time.  That should be solveable better,
include/linux/time.h lists two functions which sound promising:

  extern void getboottime(struct timespec *ts);
  extern void monotonic_to_bootbased(struct timespec *ts);

Neither of them is available to modules though, so I can't test without
rebooting my laptop ...

monotonic_to_bootbased() sounds like we would get hosts ntp adjustments
in the guests for free.

cheers,
  Gerd

-- 
http://kraxel.fedorapeople.org/xenner/

[-- Attachment #2: kvmclock-2.diff --]
[-- Type: text/x-patch, Size: 2348 bytes --]

diff -up kvm-65/kernel/x86.c.fix kvm-65/kernel/x86.c
--- kvm-65/kernel/x86.c.fix	2008-04-06 21:23:07.000000000 +0200
+++ kvm-65/kernel/x86.c	2008-04-11 16:17:23.000000000 +0200
@@ -490,7 +490,7 @@ static void kvm_write_wall_clock(struct 
 {
 	static int version;
 	struct kvm_wall_clock wc;
-	struct timespec wc_ts;
+	struct timespec now,sys,boot;
 
 	if (!wall_clock)
 		return;
@@ -499,9 +499,11 @@ static void kvm_write_wall_clock(struct 
 
 	kvm_write_guest(kvm, wall_clock, &version, sizeof(version));
 
-	wc_ts = current_kernel_time();
-	wc.wc_sec = wc_ts.tv_sec;
-	wc.wc_nsec = wc_ts.tv_nsec;
+	now = current_kernel_time();
+	ktime_get_ts(&sys);
+	boot = ns_to_timespec(timespec_to_ns(&now) - timespec_to_ns(&sys));
+	wc.wc_sec = boot.tv_sec;
+	wc.wc_nsec = boot.tv_nsec;
 	wc.wc_version = version;
 
 	kvm_write_guest(kvm, wall_clock, &wc, sizeof(wc));
@@ -548,6 +550,44 @@ static void kvm_write_guest_time(struct 
 	mark_page_dirty(v->kvm, vcpu->time >> PAGE_SHIFT);
 }
 
+static uint32_t div_frac(uint32_t dividend, uint32_t divisor)
+{
+	uint32_t quotient, remainder;
+	
+	__asm__ ( "divl %4"
+		  : "=a" (quotient), "=d" (remainder)
+		  : "0" (0), "1" (dividend), "r" (divisor) );
+	return quotient;
+}
+
+static void kvm_set_time_scale(uint32_t tsc_khz, struct kvm_vcpu_time_info *hv_clock)
+{
+	uint64_t nsecs = 1000000000LL;
+	int32_t  shift = 0;
+	uint64_t tps64;
+	uint32_t tps32;
+	
+	tps64 = tsc_khz * 1000LL;
+	while (tps64 > nsecs*2) {
+		tps64 >>= 1;
+		shift--;
+	}
+
+	tps32 = (uint32_t)tps64;
+	while (tps32 <= (uint32_t)nsecs) {
+		tps32 <<= 1;
+		shift++;
+	}
+
+	hv_clock->tsc_shift = shift;
+	hv_clock->tsc_to_system_mul = div_frac(nsecs, tps32);
+
+#if 0
+	printk(KERN_DEBUG "%s: tsc_khz %u, tsc_shift %d, tsc_mul %u\n",
+	       __FUNCTION__, tsc_khz, hv_clock->tsc_shift,
+	       hv_clock->tsc_to_system_mul);
+#endif
+}
 
 int kvm_set_msr_common(struct kvm_vcpu *vcpu, u32 msr, u64 data)
 {
@@ -596,9 +636,7 @@ int kvm_set_msr_common(struct kvm_vcpu *
 		/* ...but clean it before doing the actual write */
 		vcpu->arch.time_offset = data & ~(PAGE_MASK | 1);
 
-		vcpu->arch.hv_clock.tsc_to_system_mul =
-					clocksource_khz2mult(kvm_tsc_khz, 22);
-		vcpu->arch.hv_clock.tsc_shift = 22;
+		kvm_set_time_scale(kvm_tsc_khz, &vcpu->arch.hv_clock);
 
 		down_read(&current->mm->mmap_sem);
 		vcpu->arch.time_page =

[-- Attachment #3: Type: text/plain, Size: 320 bytes --]

-------------------------------------------------------------------------
This SF.net email is sponsored by the 2008 JavaOne(SM) Conference 
Don't miss this year's exciting event. There's still time to save $100. 
Use priority code J8TL2D2. 
http://ad.doubleclick.net/clk;198757673;13503038;p?http://java.sun.com/javaone

[-- Attachment #4: Type: text/plain, Size: 158 bytes --]

_______________________________________________
kvm-devel mailing list
kvm-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/kvm-devel

Re: pv clock: kvm is incompatible with xen :-(

[Jeremy Fitzhardinge]

Gerd Hoffmann wrote:
> Wall clock is off a few hours though.  Oops.
>
> I think the way wall clock and system clock work together in xen (Jeremy
> correct me if I'm wrong) is that the wall clock specifies the point in
> time where the system clock started going.  As kvm fills in host system
> time into the guest system time fields the guest wall clock fields
> should be filled with the host boot time timestamp I'd say.
>   

Yes.  The wallclock field in the shared info structure is the wallclock 
time at boot; you compute the current time by adding the system 
timestamp to it.   System time changes are effected by retroactively 
changing the boot time of the machine, though that can also change 
because of suspend/resume/migrate.

In general the kernel only reads the wallclock time at boot, and then 
maintains it for itself from then on.  I think.

    J

-------------------------------------------------------------------------
This SF.net email is sponsored by the 2008 JavaOne(SM) Conference 
Don't miss this year's exciting event. There's still time to save $100. 
Use priority code J8TL2D2. 
http://ad.doubleclick.net/clk;198757673;13503038;p?http://java.sun.com/javaone

Re: pv clock: kvm is incompatible with xen :-(

[Gerd Hoffmann]

Jeremy Fitzhardinge wrote:
> Gerd Hoffmann wrote:
>> Wall clock is off a few hours though.  Oops.
>>
>> I think the way wall clock and system clock work together in xen (Jeremy
>> correct me if I'm wrong) is that the wall clock specifies the point in
>> time where the system clock started going.  As kvm fills in host system
>> time into the guest system time fields the guest wall clock fields
>> should be filled with the host boot time timestamp I'd say.
>>   
> 
> Yes.  The wallclock field in the shared info structure is the wallclock
> time at boot; you compute the current time by adding the system
> timestamp to it.   System time changes are effected by retroactively
> changing the boot time of the machine, though that can also change
> because of suspend/resume/migrate.
> 
> In general the kernel only reads the wallclock time at boot, and then
> maintains it for itself from then on.  I think.

Thanks.

I'm looking at the guest side of the issue right now, trying to identify
common code, and while doing so noticed that xen does the
version-check-loop in both get_time_values_from_xen(void) and
xen_clocksource_read(void), and I can't see any obvious reason for that.
 The loop in xen_clocksource_read(void) is not needed IMHO.  Can I drop it?

cheers,
  Gerd

-- 
http://kraxel.fedorapeople.org/xenner/

-------------------------------------------------------------------------
This SF.net email is sponsored by the 2008 JavaOne(SM) Conference 
Don't miss this year's exciting event. There's still time to save $100. 
Use priority code J8TL2D2. 
http://ad.doubleclick.net/clk;198757673;13503038;p?http://java.sun.com/javaone

Re: pv clock: kvm is incompatible with xen :-(

[Jeremy Fitzhardinge]

Gerd Hoffmann wrote:
> I'm looking at the guest side of the issue right now, trying to identify
> common code, and while doing so noticed that xen does the
> version-check-loop in both get_time_values_from_xen(void) and
> xen_clocksource_read(void), and I can't see any obvious reason for that.
>  The loop in xen_clocksource_read(void) is not needed IMHO.  Can I drop it?
>   

No.  The get_nsec_offset() needs to be atomic with respect to the 
get_time_values() parameters.  There could be a loopless 
__get_time_values() for use in this case, but given that it almost never 
loops, I don't think its worthwhile.

    J

-------------------------------------------------------------------------
This SF.net email is sponsored by the 2008 JavaOne(SM) Conference 
Don't miss this year's exciting event. There's still time to save $100. 
Use priority code J8TL2D2. 
http://ad.doubleclick.net/clk;198757673;13503038;p?http://java.sun.com/javaone

Re: pv clock: kvm is incompatible with xen :-(

[Gerd Hoffmann]

Jeremy Fitzhardinge wrote:
> Gerd Hoffmann wrote:
>> I'm looking at the guest side of the issue right now, trying to identify
>> common code, and while doing so noticed that xen does the
>> version-check-loop in both get_time_values_from_xen(void) and
>> xen_clocksource_read(void), and I can't see any obvious reason for that.
>>  The loop in xen_clocksource_read(void) is not needed IMHO.  Can I
>> drop it?
> 
> No.  The get_nsec_offset() needs to be atomic with respect to the
> get_time_values() parameters.

Hmm, I somehow fail to see a case where it could be non-atomic ...

get_time_values() copies a consistent snapshot, thus
xen_clocksource_read() doesn't race against xen updating the fields.
The snapshot is in a per-cpu variable, thus it doesn't race against
other guest vcpus running get_time_values() at the same time.

> There could be a loopless
> __get_time_values() for use in this case, but given that it almost never
> loops, I don't think its worthwhile.

"in this case" ???  I'm confused.  There is only a single user of
get_nsec_offset(), which is xen_clocksource_read() ...

cheers,
  Gerd

-- 
http://kraxel.fedorapeople.org/xenner/

-------------------------------------------------------------------------
This SF.net email is sponsored by the 2008 JavaOne(SM) Conference 
Don't miss this year's exciting event. There's still time to save $100. 
Use priority code J8TL2D2. 
http://ad.doubleclick.net/clk;198757673;13503038;p?http://java.sun.com/javaone

Re: pv clock: kvm is incompatible with xen :-(

[Jeremy Fitzhardinge]

Gerd Hoffmann wrote:
> Hmm, I somehow fail to see a case where it could be non-atomic ...
>
> get_time_values() copies a consistent snapshot, thus
> xen_clocksource_read() doesn't race against xen updating the fields.
> The snapshot is in a per-cpu variable, thus it doesn't race against
> other guest vcpus running get_time_values() at the same time.
>   

Xen could change the parameters in the instant after get_time_values().  
That change could be as a result of suspend-resume, so the parameters 
and the tsc could be wildly different.  It's definitely an edge-case, 
but it's easy enough to deal with.

>> There could be a loopless
>> __get_time_values() for use in this case, but given that it almost never
>> loops, I don't think its worthwhile.
>>     
>
> "in this case" ???  I'm confused.  There is only a single user of
> get_nsec_offset(), which is xen_clocksource_read() ...
>   

Sure, but get_time_values() has several other callers.  If 
xen_clocksource_read() had its own loop to make sure the read_tsc is 
atomic with respect to get_time_values, then get_time_values itself 
needn't loop.  But, given that it only loops in the very rare case that 
it races with Xen updating those parameters, it doesn't seem to make 
much difference either way.

    J

-------------------------------------------------------------------------
This SF.net email is sponsored by the 2008 JavaOne(SM) Conference 
Don't miss this year's exciting event. There's still time to save $100. 
Use priority code J8TL2D2. 
http://ad.doubleclick.net/clk;198757673;13503038;p?http://java.sun.com/javaone

Re: pv clock: kvm is incompatible with xen :-(

[Gerd Hoffmann]

Jeremy Fitzhardinge wrote:
> Xen could change the parameters in the instant after get_time_values(). 
> That change could be as a result of suspend-resume, so the parameters
> and the tsc could be wildly different.

Ah, ok, forgot the rdtsc in the picture.  With that in mind I fully
agree that the loop is needed.  I think kvm guests can even hit that one
with the vcpu migrating to a different physical cpu, so we better handle
it correctly ;)

> Sure, but get_time_values() has several other callers.

Not really.  There are only two calls, one in clocksource_read() and one
in the init path.  The later is superfluous I think because
clocksource_read() is the only user of the shadowed time info.

cheers,
  Gerd

-- 
http://kraxel.fedorapeople.org/xenner/

-------------------------------------------------------------------------
This SF.net email is sponsored by the 2008 JavaOne(SM) Conference 
Don't miss this year's exciting event. There's still time to save $100. 
Use priority code J8TL2D2. 
http://ad.doubleclick.net/clk;198757673;13503038;p?http://java.sun.com/javaone

Re: pv clock: kvm is incompatible with xen :-(

[Jeremy Fitzhardinge]

Gerd Hoffmann wrote:
> Jeremy Fitzhardinge wrote:
>   
>> Xen could change the parameters in the instant after get_time_values(). 
>> That change could be as a result of suspend-resume, so the parameters
>> and the tsc could be wildly different.
>>     
>
> Ah, ok, forgot the rdtsc in the picture.  With that in mind I fully
> agree that the loop is needed.  I think kvm guests can even hit that one
> with the vcpu migrating to a different physical cpu, so we better handle
> it correctly ;)
>   

Yes, same with Xen.

>> Sure, but get_time_values() has several other callers.
>>     
>
> Not really.  There are only two calls, one in clocksource_read() and one
> in the init path.  The later is superfluous I think because
> clocksource_read() is the only user of the shadowed time info.
>   

Hm.  It doesn't look like shadow_time needs to be a static percpu at 
all.  It could just be a local to clocksource_read, I think.

    J

-------------------------------------------------------------------------
This SF.net email is sponsored by the 2008 JavaOne(SM) Conference 
Don't miss this year's exciting event. There's still time to save $100. 
Use priority code J8TL2D2. 
http://ad.doubleclick.net/clk;198757673;13503038;p?http://java.sun.com/javaone

Re: pv clock: kvm is incompatible with xen :-(

[Gerd Hoffmann]

Jeremy Fitzhardinge wrote:
> Gerd Hoffmann wrote:
>> Not really.  There are only two calls, one in clocksource_read() and one
>> in the init path.  The later is superfluous I think because
>> clocksource_read() is the only user of the shadowed time info.
> 
> Hm.  It doesn't look like shadow_time needs to be a static percpu at 
> all.  It could just be a local to clocksource_read, I think.

Good point, one more cleanup.

thanks,
  Gerd

-- 
http://kraxel.fedorapeople.org/xenner/

-------------------------------------------------------------------------
This SF.net email is sponsored by the 2008 JavaOne(SM) Conference 
Don't miss this year's exciting event. There's still time to save $100. 
Use priority code J8TL2D2. 
http://ad.doubleclick.net/clk;198757673;13503038;p?http://java.sun.com/javaone

Re: pv clock: kvm is incompatible with xen :-(

[Glauber Costa]

Gerd Hoffmann wrote:
> Jeremy Fitzhardinge wrote:
>> Xen could change the parameters in the instant after get_time_values(). 
>> That change could be as a result of suspend-resume, so the parameters
>> and the tsc could be wildly different.
> 
> Ah, ok, forgot the rdtsc in the picture.  With that in mind I fully
> agree that the loop is needed.  I think kvm guests can even hit that one
> with the vcpu migrating to a different physical cpu, so we better handle
> it correctly ;)

It's probably not needed for kvm, since we update everything everytime 
we get scheduled in the host side, which would cover the case for 
migration between physical cpus. But it's probably okay to do it to get 
a common denominator with xen, if needed.



-------------------------------------------------------------------------
This SF.net email is sponsored by the 2008 JavaOne(SM) Conference 
Don't miss this year's exciting event. There's still time to save $100. 
Use priority code J8TL2D2. 
http://ad.doubleclick.net/clk;198757673;13503038;p?http://java.sun.com/javaone

Re: pv clock: kvm is incompatible with xen :-(

[Gerd Hoffmann]

Glauber Costa wrote:
> Gerd Hoffmann wrote:
>> Jeremy Fitzhardinge wrote:
>>> Xen could change the parameters in the instant after
>>> get_time_values(). That change could be as a result of
>>> suspend-resume, so the parameters
>>> and the tsc could be wildly different.
>>
>> Ah, ok, forgot the rdtsc in the picture.  With that in mind I fully
>> agree that the loop is needed.  I think kvm guests can even hit that one
>> with the vcpu migrating to a different physical cpu, so we better handle
>> it correctly ;)
> 
> It's probably not needed for kvm, since we update everything everytime
> we get scheduled in the host side, which would cover the case for
> migration between physical cpus. 

No, it wouldn't.  The corner case we must catch is: guest reads time
info, kvm reschedules the guest to another pcpu, guest reads the tsc.
The time info used by the guest for the tsc delta is stale then, it
belongs to the previous pcpu.

cheers,
  Gerd

-- 
http://kraxel.fedorapeople.org/xenner/

-------------------------------------------------------------------------
This SF.net email is sponsored by the 2008 JavaOne(SM) Conference 
Don't miss this year's exciting event. There's still time to save $100. 
Use priority code J8TL2D2. 
http://ad.doubleclick.net/clk;198757673;13503038;p?http://java.sun.com/javaone

Re: pv clock: kvm is incompatible with xen :-(

[Glauber Costa]

Gerd Hoffmann wrote:
> Glauber Costa wrote:
>> Gerd Hoffmann wrote:
>>> Jeremy Fitzhardinge wrote:
>>>> Xen could change the parameters in the instant after
>>>> get_time_values(). That change could be as a result of
>>>> suspend-resume, so the parameters
>>>> and the tsc could be wildly different.
>>> Ah, ok, forgot the rdtsc in the picture.  With that in mind I fully
>>> agree that the loop is needed.  I think kvm guests can even hit that one
>>> with the vcpu migrating to a different physical cpu, so we better handle
>>> it correctly ;)
>> It's probably not needed for kvm, since we update everything everytime
>> we get scheduled in the host side, which would cover the case for
>> migration between physical cpus. 
> 
> No, it wouldn't.  The corner case we must catch is: guest reads time
> info, kvm reschedules the guest to another pcpu, guest reads the tsc.
> The time info used by the guest for the tsc delta is stale then, it
> belongs to the previous pcpu.
> 
> cheers,
>   Gerd
> 
Agreed.

-------------------------------------------------------------------------
This SF.net email is sponsored by the 2008 JavaOne(SM) Conference 
Don't miss this year's exciting event. There's still time to save $100. 
Use priority code J8TL2D2. 
http://ad.doubleclick.net/clk;198757673;13503038;p?http://java.sun.com/javaone