On Sun, Feb 18, 2018 at 09:34:50PM -0800, Mike Larkin wrote: > On Sun, Feb 18, 2018 at 07:42:10PM -0600, Scott Cheloha wrote: > > On Sun, Feb 18, 2018 at 02:44:43PM -0800, Mike Larkin wrote: > > > On Sun, Feb 18, 2018 at 12:00:01PM -0600, Scott Cheloha wrote: > > > > [...] > > > > > > Before I read through this, can you briefly explain what this is trying > > > to fix? > > > > The i8253 is a down counter. If we drive it with the system wall clock > > it could "count up" or make relatively big jumps if the superuser changes > > the host's wall clock with date(1). Driving it with the monotonic clock > > should, in principle, ensure it always decrements and that it decrements > > relatively evenly. > > > > I'm not sure what exactly the guest does if the output latch on the PIC > > increments or jumps between two readbacks, though. > > > > Here's a diff without any refactoring. But there's bugs in your timeval > > normalization code [1], so I think using the subtraction macros in > > sys/time.h > > would be a good idea, eventually. > > > > -Scott > > > > [1] tv_usec > 1000000 > > > > should be > > > > tv_usec >= 1000000 > > > > Alright, seems like this is an improvement at least in readability. I don't > believe that the jumping around of the PIT counter would cause serious issues > even if it went backwards (aside from perhaps keeping slightly less accurate > time but we know we have issues there with reading the PIT anyway, due to its > very small period). > > Thanks for spotting the tv_usec thing also. > > I'd like to get a few reports that this doesn't break anything before oks > though. I'll load it on my x230 where I run a bunch of VMs and see what > happens..
Did this work? -Scott > PS, I realize this is w/o refactoring, I like the other one more so that's > the one I'll use > > -ml > > > > > Index: usr.sbin/vmd/i8253.c > > =================================================================== > > RCS file: /cvs/src/usr.sbin/vmd/i8253.c,v > > retrieving revision 1.17 > > diff -u -p -r1.17 i8253.c > > --- usr.sbin/vmd/i8253.c 14 Aug 2017 19:46:44 -0000 1.17 > > +++ usr.sbin/vmd/i8253.c 19 Feb 2018 01:36:44 -0000 > > @@ -25,6 +25,7 @@ > > #include <event.h> > > #include <string.h> > > #include <stddef.h> > > +#include <time.h> > > #include <unistd.h> > > > > #include "i8253.h" > > @@ -53,7 +54,7 @@ void > > i8253_init(uint32_t vm_id) > > { > > memset(&i8253_channel, 0, sizeof(struct i8253_channel)); > > - gettimeofday(&i8253_channel[0].tv, NULL); > > + clock_gettime(CLOCK_MONOTONIC, &i8253_channel[0].ts); > > i8253_channel[0].start = 0xFFFF; > > i8253_channel[0].mode = TIMER_INTTC; > > i8253_channel[0].last_r = 1; > > @@ -86,7 +87,7 @@ i8253_init(uint32_t vm_id) > > void > > i8253_do_readback(uint32_t data) > > { > > - struct timeval now, delta; > > + struct timespec now, delta; > > uint64_t ns, ticks; > > > > /* bits are inverted here - !TIMER_RB_STATUS == enable chan readback */ > > @@ -99,19 +100,19 @@ i8253_do_readback(uint32_t data) > > /* !TIMER_RB_COUNT == enable counter readback */ > > if (data & ~TIMER_RB_COUNT) { > > if (data & TIMER_RB_C0) { > > - gettimeofday(&now, NULL); > > - delta.tv_sec = now.tv_sec - i8253_channel[0].tv.tv_sec; > > - delta.tv_usec = now.tv_usec - > > - i8253_channel[0].tv.tv_usec; > > - if (delta.tv_usec < 0) { > > + clock_gettime(CLOCK_MONOTONIC, &now); > > + delta.tv_sec = now.tv_sec - i8253_channel[0].ts.tv_sec; > > + delta.tv_nsec = now.tv_nsec - > > + i8253_channel[0].ts.tv_nsec; > > + if (delta.tv_nsec < 0) { > > delta.tv_sec--; > > - delta.tv_usec += 1000000; > > + delta.tv_nsec += 1000000000; > > } > > - if (delta.tv_usec > 1000000) { > > + if (delta.tv_nsec >= 1000000000) { > > delta.tv_sec++; > > - delta.tv_usec -= 1000000; > > + delta.tv_nsec -= 1000000000; > > } > > - ns = delta.tv_usec * 1000 + delta.tv_sec * 1000000000; > > + ns = delta.tv_nsec + delta.tv_sec * 1000000000; > > ticks = ns / NS_PER_TICK; > > if (i8253_channel[0].start) > > i8253_channel[0].olatch = > > @@ -122,19 +123,19 @@ i8253_do_readback(uint32_t data) > > } > > > > if (data & TIMER_RB_C1) { > > - gettimeofday(&now, NULL); > > - delta.tv_sec = now.tv_sec - i8253_channel[1].tv.tv_sec; > > - delta.tv_usec = now.tv_usec - > > - i8253_channel[1].tv.tv_usec; > > - if (delta.tv_usec < 0) { > > + clock_gettime(CLOCK_MONOTONIC, &now); > > + delta.tv_sec = now.tv_sec - i8253_channel[1].ts.tv_sec; > > + delta.tv_nsec = now.tv_nsec - > > + i8253_channel[1].ts.tv_nsec; > > + if (delta.tv_nsec < 0) { > > delta.tv_sec--; > > - delta.tv_usec += 1000000; > > + delta.tv_nsec += 1000000000; > > } > > - if (delta.tv_usec > 1000000) { > > + if (delta.tv_nsec >= 1000000000) { > > delta.tv_sec++; > > - delta.tv_usec -= 1000000; > > + delta.tv_nsec -= 1000000000; > > } > > - ns = delta.tv_usec * 1000 + delta.tv_sec * 1000000000; > > + ns = delta.tv_nsec + delta.tv_sec * 1000000000; > > ticks = ns / NS_PER_TICK; > > if (i8253_channel[1].start) > > i8253_channel[1].olatch = > > @@ -145,19 +146,19 @@ i8253_do_readback(uint32_t data) > > } > > > > if (data & TIMER_RB_C2) { > > - gettimeofday(&now, NULL); > > - delta.tv_sec = now.tv_sec - i8253_channel[2].tv.tv_sec; > > - delta.tv_usec = now.tv_usec - > > - i8253_channel[2].tv.tv_usec; > > - if (delta.tv_usec < 0) { > > + clock_gettime(CLOCK_MONOTONIC, &now); > > + delta.tv_sec = now.tv_sec - i8253_channel[2].ts.tv_sec; > > + delta.tv_nsec = now.tv_nsec - > > + i8253_channel[2].ts.tv_nsec; > > + if (delta.tv_nsec < 0) { > > delta.tv_sec--; > > - delta.tv_usec += 1000000; > > + delta.tv_nsec += 1000000000; > > } > > - if (delta.tv_usec > 1000000) { > > + if (delta.tv_nsec >= 1000000000) { > > delta.tv_sec++; > > - delta.tv_usec -= 1000000; > > + delta.tv_nsec -= 1000000000; > > } > > - ns = delta.tv_usec * 1000 + delta.tv_sec * 1000000000; > > + ns = delta.tv_nsec + delta.tv_sec * 1000000000; > > ticks = ns / NS_PER_TICK; > > if (i8253_channel[2].start) > > i8253_channel[2].olatch = > > @@ -188,7 +189,7 @@ vcpu_exit_i8253(struct vm_run_params *vr > > uint32_t out_data; > > uint8_t sel, rw, data, mode; > > uint64_t ns, ticks; > > - struct timeval now, delta; > > + struct timespec now, delta; > > union vm_exit *vei = vrp->vrp_exit; > > > > get_input_data(vei, &out_data); > > @@ -216,21 +217,20 @@ vcpu_exit_i8253(struct vm_run_params *vr > > * rate. > > */ > > if (rw == TIMER_LATCH) { > > - gettimeofday(&now, NULL); > > + clock_gettime(CLOCK_MONOTONIC, &now); > > delta.tv_sec = now.tv_sec - > > - i8253_channel[sel].tv.tv_sec; > > - delta.tv_usec = now.tv_usec - > > - i8253_channel[sel].tv.tv_usec; > > - if (delta.tv_usec < 0) { > > + i8253_channel[sel].ts.tv_sec; > > + delta.tv_nsec = now.tv_nsec - > > + i8253_channel[sel].ts.tv_nsec; > > + if (delta.tv_nsec < 0) { > > delta.tv_sec--; > > - delta.tv_usec += 1000000; > > + delta.tv_nsec += 1000000000; > > } > > - if (delta.tv_usec > 1000000) { > > + if (delta.tv_nsec >= 1000000000) { > > delta.tv_sec++; > > - delta.tv_usec -= 1000000; > > + delta.tv_nsec -= 1000000000; > > } > > - ns = delta.tv_usec * 1000 + > > - delta.tv_sec * 1000000000; > > + ns = delta.tv_nsec + delta.tv_sec * 1000000000; > > ticks = ns / NS_PER_TICK; > > if (i8253_channel[sel].start) { > > i8253_channel[sel].olatch = > > Index: usr.sbin/vmd/i8253.h > > =================================================================== > > RCS file: /cvs/src/usr.sbin/vmd/i8253.h,v > > retrieving revision 1.7 > > diff -u -p -r1.7 i8253.h > > --- usr.sbin/vmd/i8253.h 9 Jul 2017 00:51:40 -0000 1.7 > > +++ usr.sbin/vmd/i8253.h 19 Feb 2018 01:36:44 -0000 > > @@ -29,7 +29,7 @@ > > > > /* i8253 registers */ > > struct i8253_channel { > > - struct timeval tv; /* timer start time */ > > + struct timespec ts; /* timer start time */ > > uint16_t start; /* starting value */ > > uint16_t olatch; /* output latch */ > > uint16_t ilatch; /* input latch */