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:
> > Hi,
> > 
> > Is it bad if the olatch counts up if the host's wall clock
> > is set backward?  Or if the olatch makes a big jump for
> > the same reason?
> > 
> > Serious question: I don't have any EPT hardware to test
> > this (sorry).
> > 
> > While here, it looks like we can (a) roll the olatch-writing
> > parts of i8253_do_readback into a loop, and (b) use the
> > timespecsub macro from sys/time.h, to make the code briefer.
> > 
> > Anyone down to test?
> > 
> > --
> > Scott Cheloha
> > 
> 
> 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

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 */

Reply via email to