Sorry I didn't respond yesterday. I was sick and went right to bed after
work. This is probably ok, although I haven't looked at it in great
depth. If you boot the x86 disk image all the way to the login prompt,
one of the startup scripts will fail. If it fails and prints an error
message once, the RTC is updating. If it fails and prints the same error
message twice, it's not. After I dealt with it in M5, I noticed that
script failing on a real machine with the error message once, so I think
having one is a defect in the script, not in our implementation.
Basically what happens, I think, is that this executable waits for a
second boundary before it does whatever it's going to do. The first one
happens arbitrarily in the middle of a second, and so if time is
progressing, it gets to one without waiting too long. Then it
immediately does a second (as in after first) operation. Since that one
started about as far as it could from the end of the second and the
executable doesn't wait quite long enough, it thinks the second isn't
going to change and gives up.
Gabe
Steve Reinhardt wrote:
> Brad's issues with the timer device and checkpointing reminded me that
> I wasn't really happy with that code; in particular, the redundant
> storage in both clock_data[] and curTime seemed bound to cause
> problems when they got out of sync, and in fact Brad's checkpointing
> fix was incomplete in that it didn't save and restore curTime.
>
> I tweaked the code to get rid of curTime; now clock_data is the only
> state, and we convert to/from a struct tm only temporarily as needed.
> This code passes the regressions, but since there's no X86_FS
> regression and that might be the only config that cares, I didn't want
> to just check it in without getting some feedback.
>
> On a separate but related note, I also think that we can get rid of
> the mkutctime() call. We really just need a reversible conversion
> from 'struct tm' to 'time_t' and back, and although mkutctime/gmtime
> provides that, I don't see why mktime/localtime wouldn't do just as
> well (while being simpler). Thoughts?
>
> Steve
>
> -------------------
>
> diff -r 9c33426d404a src/dev/mc146818.hh
> --- a/src/dev/mc146818.hh Mon Oct 19 17:29:34 2009 -0400
> +++ b/src/dev/mc146818.hh Tue Oct 20 22:18:27 2009 -0700
> @@ -91,27 +91,33 @@
> /** RTC tick event */
> RTCTickEvent tickEvent;
>
> + struct ClockFields {
> + uint8_t sec;
> + uint8_t sec_alrm;
> + uint8_t min;
> + uint8_t min_alrm;
> + uint8_t hour;
> + uint8_t hour_alrm;
> + uint8_t wday;
> + uint8_t mday;
> + uint8_t mon;
> + uint8_t year;
> +
> + void setBCDFromBinary(const ClockFields *);
> + void setBinaryFromBCD(const ClockFields *);
> +
> + void setTime(const struct tm time);
> + void getTime(struct tm *time);
> + };
> +
> /** Data for real-time clock function */
> union {
> uint8_t clock_data[10];
> -
> - struct {
> - uint8_t sec;
> - uint8_t sec_alrm;
> - uint8_t min;
> - uint8_t min_alrm;
> - uint8_t hour;
> - uint8_t hour_alrm;
> - uint8_t wday;
> - uint8_t mday;
> - uint8_t mon;
> - uint8_t year;
> - };
> + struct ClockFields clock_fields;
> };
>
> - struct tm curTime;
> -
> void setTime(const struct tm time);
> + void getTime(struct tm *time);
>
> /** RTC status register A */
> uint8_t stat_regA;
> diff -r 9c33426d404a src/dev/mc146818.cc
> --- a/src/dev/mc146818.cc Mon Oct 19 17:29:34 2009 -0400
> +++ b/src/dev/mc146818.cc Tue Oct 20 22:18:27 2009 -0700
> @@ -62,33 +62,90 @@
> }
>
> void
> +MC146818::ClockFields::setBCDFromBinary(const ClockFields *src)
> +{
> + // note that possibly src == this for in-place conversion
> + sec = bcdize(src->sec);
> + min = bcdize(src->min);
> + hour = bcdize(src->hour);
> + wday = src->wday; // wday < 8, so no need to convert
> + mday = bcdize(src->mday);
> + mon = bcdize(src->mon);
> + // The datasheet says that the year field can be either BCD or
> + // years since 1900. Linux seems to be happy with years since
> + // 1900.
> + year = bcdize(src->year % 100);
> +}
> +
> +void
> +MC146818::ClockFields::setBinaryFromBCD(const ClockFields *src)
> +{
> + // note that possibly src == this for in-place conversion
> + sec = unbcdize(src->sec);
> + min = unbcdize(src->min);
> + hour = unbcdize(src->hour);
> + wday = src->wday; // wday < 8, so no need to convert
> + mday = unbcdize(src->mday);
> + mon = unbcdize(src->mon);
> + // binary year is years since 1900, not just year % 100
> + // assume range of 1970-2069
> + year = unbcdize(src->year);
> + if (year < 70)
> + year += 100;
> +}
> +
> +void
> +MC146818::ClockFields::setTime(const struct tm time)
> +{
> + sec = time.tm_sec;
> + min = time.tm_min;
> + hour = time.tm_hour;
> + // Datasheet says 1 is sunday
> + wday = time.tm_wday + 1;
> + mday = time.tm_mday;
> + // Unix is 0-11 for month, data sheet says start at 1
> + mon = time.tm_mon + 1;
> + year = time.tm_year % 1900;
> +}
> +
> +void
> +MC146818::ClockFields::getTime(struct tm *time)
> +{
> + // inverse of setTime
> + time->tm_sec = sec;
> + time->tm_min = min;
> + time->tm_hour = hour;
> + time->tm_wday = wday - 1;
> + time->tm_mday = mday;
> + time->tm_mon = mon - 1;
> + time->tm_year = year + 1900;
> +}
> +
> +void
> MC146818::setTime(const struct tm time)
> {
> - curTime = time;
> - year = time.tm_year;
> - // Unix is 0-11 for month, data seet says start at 1
> - mon = time.tm_mon + 1;
> - mday = time.tm_mday;
> - hour = time.tm_hour;
> - min = time.tm_min;
> - sec = time.tm_sec;
> -
> - // Datasheet says 1 is sunday
> - wday = time.tm_wday + 1;
> + clock_fields.setTime(time);
>
> if (!(stat_regB & RTCB_BIN)) {
> - // The datasheet says that the year field can be either BCD or
> - // years since 1900. Linux seems to be happy with years since
> - // 1900.
> - year = bcdize(year % 100);
> - mon = bcdize(mon);
> - mday = bcdize(mday);
> - hour = bcdize(hour);
> - min = bcdize(min);
> - sec = bcdize(sec);
> + clock_fields.setBCDFromBinary(&clock_fields);
> }
> }
>
> +void
> +MC146818::getTime(struct tm *time)
> +{
> + // inverse of setTime
> + if (!(stat_regB & RTCB_BIN)) {
> + // convert temporarily to binary
> + ClockFields tmp_clock_fields;
> + tmp_clock_fields.setBinaryFromBCD(&clock_fields);
> + tmp_clock_fields.getTime(time);
> + } else {
> + clock_fields.getTime(time);
> + }
> +}
> +
> +
> MC146818::MC146818(EventManager *em, const string &n, const struct tm
> time,
> bool bcd, Tick frequency)
> : EventManager(em), _name(n), event(this, frequency), tickEvent(this)
> @@ -112,13 +169,6 @@
> {
> if (addr < RTC_STAT_REGA) {
> clock_data[addr] = data;
> - curTime.tm_sec = unbcdize(sec);
> - curTime.tm_min = unbcdize(min);
> - curTime.tm_hour = unbcdize(hour);
> - curTime.tm_mday = unbcdize(mday);
> - curTime.tm_mon = unbcdize(mon) - 1;
> - curTime.tm_year = ((unbcdize(year) + 50) % 100) + 1950;
> - curTime.tm_wday = unbcdize(wday) - 1;
> } else {
> switch (addr) {
> case RTC_STAT_REGA:
> @@ -196,6 +246,8 @@
> {
> if (stat_regB & RTCB_NO_UPDT)
> return;
> + struct tm curTime;
> + getTime(&curTime);
> time_t calTime = mkutctime(&curTime);
> calTime++;
> setTime(*gmtime(&calTime));
>
> ------------------------------------------------------------------------
>
> _______________________________________________
> m5-dev mailing list
> [email protected]
> http://m5sim.org/mailman/listinfo/m5-dev
>
_______________________________________________
m5-dev mailing list
[email protected]
http://m5sim.org/mailman/listinfo/m5-dev
