1 month bump.

> On Mar 9, 2018, at 2:31 PM, Scott Cheloha <scottchel...@gmail.com> wrote:
> 
>> 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 */

Reply via email to