1 month bump.
> On Mar 9, 2018, at 2:31 PM, Scott Cheloha <[email protected]> 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 */
