Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Porting #SF patch #565 Real Time Clock /CMOS fix #4

Merged
merged 26 commits into from
Dec 1, 2023

Conversation

stlintel
Copy link
Contributor

by Michele Giacomone

Detailed description:

-Observed issues

Due to some limitations only dates between 1980 and 2038 can be
used in a reliable way.
Also, bochs incorrectly assumes a linear correspondence between
the data returned by the <time.h> functions localtime() and
mktime(), and isn't setting the latter properly.
Bochs keeps its internal time value dependent to these functions
after setup, assuming that their internal settings won't change
on the go - which is not the case.
In my OS, and in my timezone, this leads to incorrect startup values
for 5 months each year and unreliable values if the simulation is
kept going for a long time. (a feedback between localtime() and
mktime() is created which keeps shifting back the time)
Also, the RTC simulation is not realistic since the clock fixes
itself across DST changes, without updating any DST related flag,
a behavior that no guest OS expects.

-Proposed fix

This is implemented in such way that no bochs' previous behavior
is changed, a part from the broken ones, with legacy in mind
== the user can keep using bochs exactly as before knowing nothing
of this patch

+Make the internal s.timeval variable a Bit64s, so it can fit all
values that the cmos can correctly represent, reported below:
MIN setting -62167219200 => 0000/01/01 SAT 0:00:00
MAX BCD setting 253402300799 => 9999/12/31 FRI 23:59:59
MAX BIN setting 745690751999 => 25599/12/31 FRI 23:59:59
And then fix each reference to these so it can handle such values
And make bochs correctly wrap around for under/overflows, so that
only the most significant bits of the century are lost.

+Do the same thing to the bochs time0 parameter, so all the above
values can be chosen at startup (despite being now legal values,
1 and 2 will still be treated as "local" and "utc"). Note that
normally only BCD settings are valid since bochs' CMOS defaults
to such operating mode - the only way to use the binary range
is by loading a cmos memory map.

+Make the internal s.timeval variable independent from external
factors. This means providing a small set of time handling
functions, contained in "iodev/utctime.h", which must work in
any environment in which bochs compiles, accessing no external
resource. This also means that after startup, s.timeval will only
be changed internally, and no call to the OS time functions will
be made.

+Make the internal s.timeval variable timezone independent, to
have a linear correlation between its values and valid CMOS
settings. To make it easier, s.timeval is gonna be treated as
if the current timezone was UTC: so,
- if the user selects UTC as time0, s.timeval will become current
time(NULL)
- if the user selects localtime, s.timeval will be computed as
the value which will display the same broken down time as
localtime(&now)
- if the user inputs a time formatted string the proper s.timeval
to displayed will be easily calculated,
- if the user inputs a starting time value, s.timeval will be
computed as the value which will display the same broken down
time as localtime(&user_input) to ensure the same operation as
before.
A "tz=utc" is displayed when bochs prints out the current time
value, to warn users about the difference in meaning between the
internally kept time value and the value they can set through
the "time0=" parameter. This might be changed to communicate
instead the time value they can input to get the same setting,
but performing such calculation (except for the startup time)
suffers from all the mktime()/localtime() problems listed above
so I did not do it.
The range of "time0" is automatically adjusted so all users in
all time zones can set any legal value despite "time0=" having a
local meaning.

A thorough explanation of what I did and why can be found in the
"iodev/utctime.h" library header.

@stlintel
Copy link
Contributor Author

I hope someone who have capability / expertise of compiling Bochs BIOS could help testing this PR.
I trust the author and believe this has added value to the project but still somehow need to validate it.

@stlintel stlintel force-pushed the bochs/real_time_cmos_fix_patch branch from f0007d7 to 477a90b Compare August 26, 2022 18:22
stlintel and others added 3 commits October 10, 2023 14:54
by Michele Giacomone

Detailed description:

  -Observed issues

   Due to some limitations only dates between 1980 and 2038 can be
   used in a reliable way.
   Also, bochs incorrectly assumes a linear correspondence between
   the data returned by the <time.h> functions localtime() and
   mktime(), and isn't setting the latter properly.
   Bochs keeps its internal time value dependent to these functions
   after setup, assuming that their internal settings won't change
   on the go - which is not the case.
   In my OS, and in my timezone, this leads to incorrect startup values
   for 5 months each year and unreliable values if the simulation is
   kept going for a long time. (a feedback between localtime() and
   mktime() is created which keeps shifting back the time)
   Also, the RTC simulation is not realistic since the clock fixes
   itself across DST changes, without updating any DST related flag,
   a behavior that no guest OS expects.

  -Proposed fix

   This is implemented in such way that no bochs' previous behavior
   is changed, a part from the broken ones, with legacy in mind
   == the user can keep using bochs exactly as before knowing nothing
      of this patch

   +Make the internal s.timeval variable a Bit64s, so it can fit all
    values that the cmos can correctly represent, reported below:
    MIN     setting  -62167219200 =>  0000/01/01 SAT  0:00:00
    MAX BCD setting  253402300799 =>  9999/12/31 FRI 23:59:59
    MAX BIN setting  745690751999 => 25599/12/31 FRI 23:59:59
    And then fix each reference to these so it can handle such values
    And make bochs correctly wrap around for under/overflows, so that
    only the most significant bits of the century are lost.

   +Do the same thing to the bochs time0 parameter, so all the above
    values can be chosen at startup (despite being now legal values,
    1 and 2 will still be treated as "local" and "utc"). Note that
    normally only BCD settings are valid since bochs' CMOS defaults
    to such operating mode - the only way to use the binary range
    is by loading a cmos memory map.

   +Make the internal s.timeval variable independent from external
    factors. This means providing a small set of time handling
    functions, contained in "iodev/utctime.h", which must work in
    any environment in which bochs compiles, accessing no external
    resource. This also means that after startup, s.timeval will only
    be changed internally, and no call to the OS time functions will
    be made.

   +Make the internal s.timeval variable timezone independent, to
    have a linear correlation between its values and valid CMOS
    settings. To make it easier, s.timeval is gonna be treated as
    if the current timezone was UTC: so,
     - if the user selects UTC as time0, s.timeval will become current
       time(NULL)
     - if the user selects localtime, s.timeval will be computed as
       the value which will display the same broken down time as
       localtime(&now)
     - if the user inputs a time formatted string the proper s.timeval
       to displayed will be easily calculated,
     - if the user inputs a starting time value, s.timeval will be
       computed as the value which will display the same broken down
       time as localtime(&user_input) to ensure the same operation as
       before.
    A "tz=utc" is displayed when bochs prints out the current time
    value, to warn users about the difference in meaning between the
    internally kept time value and the value they can set through
    the "time0=" parameter. This might be changed to communicate
    instead the time value they can input to get the same setting,
    but performing such calculation (except for the startup time)
    suffers from all the mktime()/localtime() problems listed above
    so I did not do it.
    The range of "time0" is automatically adjusted so all users in
    all time zones can set any legal value despite "time0=" having a
    local meaning.

  A thorough explanation of what I did and why can be found in the
  "iodev/utctime.h" library header.
…never used and could be removed from configure script
@stlintel stlintel force-pushed the bochs/real_time_cmos_fix_patch branch from 477a90b to 3e913fa Compare October 10, 2023 15:43
@stlintel stlintel force-pushed the master branch 2 times, most recently from a170ca4 to 32b3f65 Compare October 10, 2023 17:06
Shwartsman and others added 4 commits November 19, 2023 23:09
by Michele Giacomone

Detailed description:

  -Observed issues

   Due to some limitations only dates between 1980 and 2038 can be
   used in a reliable way.
   Also, bochs incorrectly assumes a linear correspondence between
   the data returned by the <time.h> functions localtime() and
   mktime(), and isn't setting the latter properly.
   Bochs keeps its internal time value dependent to these functions
   after setup, assuming that their internal settings won't change
   on the go - which is not the case.
   In my OS, and in my timezone, this leads to incorrect startup values
   for 5 months each year and unreliable values if the simulation is
   kept going for a long time. (a feedback between localtime() and
   mktime() is created which keeps shifting back the time)
   Also, the RTC simulation is not realistic since the clock fixes
   itself across DST changes, without updating any DST related flag,
   a behavior that no guest OS expects.

  -Proposed fix

   This is implemented in such way that no bochs' previous behavior
   is changed, a part from the broken ones, with legacy in mind
   == the user can keep using bochs exactly as before knowing nothing
      of this patch

   +Make the internal s.timeval variable a Bit64s, so it can fit all
    values that the cmos can correctly represent, reported below:
    MIN     setting  -62167219200 =>  0000/01/01 SAT  0:00:00
    MAX BCD setting  253402300799 =>  9999/12/31 FRI 23:59:59
    MAX BIN setting  745690751999 => 25599/12/31 FRI 23:59:59
    And then fix each reference to these so it can handle such values
    And make bochs correctly wrap around for under/overflows, so that
    only the most significant bits of the century are lost.

   +Do the same thing to the bochs time0 parameter, so all the above
    values can be chosen at startup (despite being now legal values,
    1 and 2 will still be treated as "local" and "utc"). Note that
    normally only BCD settings are valid since bochs' CMOS defaults
    to such operating mode - the only way to use the binary range
    is by loading a cmos memory map.

   +Make the internal s.timeval variable independent from external
    factors. This means providing a small set of time handling
    functions, contained in "iodev/utctime.h", which must work in
    any environment in which bochs compiles, accessing no external
    resource. This also means that after startup, s.timeval will only
    be changed internally, and no call to the OS time functions will
    be made.

   +Make the internal s.timeval variable timezone independent, to
    have a linear correlation between its values and valid CMOS
    settings. To make it easier, s.timeval is gonna be treated as
    if the current timezone was UTC: so,
     - if the user selects UTC as time0, s.timeval will become current
       time(NULL)
     - if the user selects localtime, s.timeval will be computed as
       the value which will display the same broken down time as
       localtime(&now)
     - if the user inputs a time formatted string the proper s.timeval
       to displayed will be easily calculated,
     - if the user inputs a starting time value, s.timeval will be
       computed as the value which will display the same broken down
       time as localtime(&user_input) to ensure the same operation as
       before.
    A "tz=utc" is displayed when bochs prints out the current time
    value, to warn users about the difference in meaning between the
    internally kept time value and the value they can set through
    the "time0=" parameter. This might be changed to communicate
    instead the time value they can input to get the same setting,
    but performing such calculation (except for the startup time)
    suffers from all the mktime()/localtime() problems listed above
    so I did not do it.
    The range of "time0" is automatically adjusted so all users in
    all time zones can set any legal value despite "time0=" having a
    local meaning.

  A thorough explanation of what I did and why can be found in the
  "iodev/utctime.h" library header.
…never used and could be removed from configure script
@stlintel stlintel force-pushed the bochs/real_time_cmos_fix_patch branch from 3e913fa to f98c542 Compare November 19, 2023 21:10
@stlintel stlintel force-pushed the master branch 2 times, most recently from ca0b00a to f4f41cf Compare November 21, 2023 18:26
@vruppert
Copy link
Contributor

I'm interested in testing this patch, but I'm unable to resolve the issues above. The conflicting file is not a part of the original patch.

@stlintel
Copy link
Contributor Author

I'm interested in testing this patch, but I'm unable to resolve the issues above. The conflicting file is not a part of the original patch.

Resolved

Disabled including utc_time.h in config.cc to make Bochs compile with plugins
disabled. I don't know whether or not the author wanted to use his own functions
there. Added check for mktime() in configure.ac once again, since at least the
hdimage code is using it for coherency checks. To be continued.
- Added missing ASM function 'imodu' for the BIOS code (untested)
- Added makefile dependency for utctime.h
- Some text cleanups
This reverts commit 692c06a.

Reverted commit of additional BIOS files.
- Added missing ASM function 'imodu' for the BIOS code (untested)
- Added makefile dependency for utctime.h
- Some text cleanups
- Since git doesn't support SVN keywords, use current build date only for now.
- Added generated BIOS files to .gitignore.
- Minor other changes.
- Fixed 12/24 hour handling in 'Get CMOS time' function (INT 1A AH=02).
- Recompiled BIOS images.
- Fixed 12/24 hour handling in 'Set CMOS time' function (INT 1A AH=03).
- Fixed bcd2bin and bin2bcd conversion functions.
- Recompiled BIOS images.
Now using format specifier FMT_LL (defined in osdep.h) for 64 bit values in different platforms.
@vruppert
Copy link
Contributor

From my point of view these changes are ready to review and test. I guess the original author never compiled a BIOS, but now this part of the patch also works as expected.

@vruppert vruppert self-requested a review December 1, 2023 21:50
@vruppert vruppert merged commit ffc722f into master Dec 1, 2023
@vruppert vruppert deleted the bochs/real_time_cmos_fix_patch branch December 1, 2023 21:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants