[Qemu-devel] [PATCH 0/3] Fix autotest-exposed RTC bug 1058225

[Qemu-devel] [PATCH 0/3] Fix autotest-exposed RTC bug 1058225

[Paolo Bonzini]

> Very easy to reproduce:
>
> 1) Build the latest qemu.git (we've captured this on internal automated
> testing, verified manually), the commit for reference is:
>
> 14:07:02 INFO | git commit ID is
> 6f8fd2530e9a530f237240daf1c981fa5df7f978 (tag v1.2.0-461-g6f8fd25)
>
> 2) Install a linux guest in it (caught with RHEL 6.2, verified with
> Fedora 17)
>
> 3) In the linux guest, set the hardware clock with hwclock:
>
> /sbin/hwclock --set --date "2/2/80 03:04:00"
>
> 4) Verify if hardware clock was set back to the eighties:
>
> LC_ALL=C /sbin/hwclock
>
> 5) Observe amazed that hwclock reports a date in the year 2043:
>
> 14:09:34 INFO |          ('hwclock', 'FAIL', 2, "Failed to set hwclock
> back to the eighties. Output of hwclock is 'Sun Dec 27 20:35:46 2043
> -0.489664 seconds'")

The testcase of patch 1 exposes the bug on an unpatched QEMU.

Paolo

Paolo Bonzini (3):
  rtc: fix overflow in mktimegm
  rtc: map CMOS index 0x37 to 0x32 on read and writes
  rtc: implement century byte

 cutils.c              |  2 +-
 hw/mc146818rtc.c      | 40 ++++++++++++++++++----------
 hw/mc146818rtc_regs.h |  4 +++
 tests/rtc-test.c      | 73 +++++++++++++++++++++++++++++++++++++++++++++++++++
 4 file modificati, 104 inserzioni(+), 15 rimozioni(-)

-- 
1.7.12

[Qemu-devel] [PATCH 1/3] rtc: fix overflow in mktimegm

[Paolo Bonzini]

When setting a date in 1980, Linux is actually disregarding the century
byte and setting the year to 2080.  This causes a year-2038 overflow
in mktimegm.  Fix this by doing the days-to-seconds computation in
64-bit math.

Reported-by: Lucas Meneghel Rodrigues <lookkas@gmail.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 cutils.c         |  2 +-
 tests/rtc-test.c | 45 +++++++++++++++++++++++++++++++++++++++++++++
 2 file modificati, 46 inserzioni(+). 1 rimozione(-)

diff --git a/cutils.c b/cutils.c
index 8ef648f..8edd8fa 100644
--- a/cutils.c
+++ b/cutils.c
@@ -115,7 +115,7 @@ time_t mktimegm(struct tm *tm)
         m += 12;
         y--;
     }
-    t = 86400 * (d + (153 * m - 457) / 5 + 365 * y + y / 4 - y / 100 + 
+    t = 86400ULL * (d + (153 * m - 457) / 5 + 365 * y + y / 4 - y / 100 + 
                  y / 400 - 719469);
     t += 3600 * tm->tm_hour + 60 * tm->tm_min + tm->tm_sec;
     return t;
diff --git a/tests/rtc-test.c b/tests/rtc-test.c
index f23ac3a..2b9aa63 100644
--- a/tests/rtc-test.c
+++ b/tests/rtc-test.c
@@ -179,6 +179,50 @@ static void check_time(int wiggle)
 
 static int wiggle = 2;
 
+static void set_year(void)
+{
+    /* Set BCD mode */
+    cmos_write(RTC_REG_B, cmos_read(RTC_REG_B) & ~REG_B_DM);
+    cmos_write(RTC_REG_A, 0x76);
+    cmos_write(RTC_YEAR, 0x11);
+    cmos_write(RTC_MONTH, 0x02);
+    cmos_write(RTC_DAY_OF_MONTH, 0x02);
+    cmos_write(RTC_HOURS, 0x02);
+    cmos_write(RTC_MINUTES, 0x04);
+    cmos_write(RTC_SECONDS, 0x58);
+    cmos_write(RTC_REG_A, 0x26);
+
+    g_assert_cmpint(cmos_read(RTC_HOURS), ==, 0x02);
+    g_assert_cmpint(cmos_read(RTC_MINUTES), ==, 0x04);
+    g_assert_cmpint(cmos_read(RTC_SECONDS), >=, 0x58);
+    g_assert_cmpint(cmos_read(RTC_DAY_OF_MONTH), ==, 0x02);
+    g_assert_cmpint(cmos_read(RTC_MONTH), ==, 0x02);
+    g_assert_cmpint(cmos_read(RTC_YEAR), ==, 0x11);
+
+    /* Set a date in 2080 to ensure there is no year-2038 overflow.  */
+    cmos_write(RTC_REG_A, 0x76);
+    cmos_write(RTC_YEAR, 0x80);
+    cmos_write(RTC_REG_A, 0x26);
+
+    g_assert_cmpint(cmos_read(RTC_HOURS), ==, 0x02);
+    g_assert_cmpint(cmos_read(RTC_MINUTES), ==, 0x04);
+    g_assert_cmpint(cmos_read(RTC_SECONDS), >=, 0x58);
+    g_assert_cmpint(cmos_read(RTC_DAY_OF_MONTH), ==, 0x02);
+    g_assert_cmpint(cmos_read(RTC_MONTH), ==, 0x02);
+    g_assert_cmpint(cmos_read(RTC_YEAR), ==, 0x80);
+
+    cmos_write(RTC_REG_A, 0x76);
+    cmos_write(RTC_YEAR, 0x11);
+    cmos_write(RTC_REG_A, 0x26);
+
+    g_assert_cmpint(cmos_read(RTC_HOURS), ==, 0x02);
+    g_assert_cmpint(cmos_read(RTC_MINUTES), ==, 0x04);
+    g_assert_cmpint(cmos_read(RTC_SECONDS), >=, 0x58);
+    g_assert_cmpint(cmos_read(RTC_DAY_OF_MONTH), ==, 0x02);
+    g_assert_cmpint(cmos_read(RTC_MONTH), ==, 0x02);
+    g_assert_cmpint(cmos_read(RTC_YEAR), ==, 0x11);
+}
+
 static void bcd_check_time(void)
 {
     /* Set BCD mode */
@@ -269,6 +313,7 @@ int main(int argc, char **argv)
     qtest_add_func("/rtc/bcd/check-time", bcd_check_time);
     qtest_add_func("/rtc/dec/check-time", dec_check_time);
     qtest_add_func("/rtc/alarm-time", alarm_time);
+    qtest_add_func("/rtc/set-year", set_year);
     qtest_add_func("/rtc/fuzz-registers", fuzz_registers);
     ret = g_test_run();
 
-- 
1.7.12

[Qemu-devel] [PATCH 2/3] rtc: map CMOS index 0x37 to 0x32 on read and writes

[Paolo Bonzini]

QEMU's attempt to implement the century byte cover two possible places
for the byte.  A common one on modern chipsets is 0x32, but QEMU also
stores the value in 0x37 (apparently for IBM PS/2 compatibility---it's
only been 25 years).  To simplify the implementation of the century
byte, store it only at 0x32 but remap transparently 0x37 to 0x32 when
reading and writing from CMOS.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 hw/mc146818rtc.c      | 15 +++++++++------
 hw/mc146818rtc_regs.h |  4 ++++
 2 file modificati, 13 inserzioni(+), 6 rimozioni(-)

diff --git a/hw/mc146818rtc.c b/hw/mc146818rtc.c
index d63554f..a7d20d5 100644
--- a/hw/mc146818rtc.c
+++ b/hw/mc146818rtc.c
@@ -399,6 +399,10 @@ static void cmos_ioport_write(void *opaque, uint32_t addr, uint32_t data)
             s->cmos_data[s->cmos_index] = data;
             check_update_timer(s);
             break;
+	case RTC_IBM_PS2_CENTURY_BYTE:
+            s->cmos_index = RTC_CENTURY;
+            /* fall through */
+        case RTC_CENTURY:
         case RTC_SECONDS:
         case RTC_MINUTES:
         case RTC_HOURS:
@@ -598,6 +602,10 @@ static uint32_t cmos_ioport_read(void *opaque, uint32_t addr)
         return 0xff;
     } else {
         switch(s->cmos_index) {
+	case RTC_IBM_PS2_CENTURY_BYTE:
+            s->cmos_index = RTC_CENTURY;
+            /* fall through */
+        case RTC_CENTURY:
         case RTC_SECONDS:
         case RTC_MINUTES:
         case RTC_HOURS:
@@ -661,10 +669,6 @@ void rtc_set_memory(ISADevice *dev, int addr, int val)
         s->cmos_data[addr] = val;
 }
 
-/* PC cmos mappings */
-#define REG_IBM_CENTURY_BYTE        0x32
-#define REG_IBM_PS2_CENTURY_BYTE    0x37
-
 static void rtc_set_date_from_host(ISADevice *dev)
 {
     RTCState *s = DO_UPCAST(RTCState, dev, dev);
@@ -681,8 +685,7 @@ static void rtc_set_date_from_host(ISADevice *dev)
     rtc_set_cmos(s, &tm);
 
     val = rtc_to_bcd(s, (tm.tm_year / 100) + 19);
-    rtc_set_memory(dev, REG_IBM_CENTURY_BYTE, val);
-    rtc_set_memory(dev, REG_IBM_PS2_CENTURY_BYTE, val);
+    rtc_set_memory(dev, RTC_CENTURY, val);
 }
 
 static int rtc_post_load(void *opaque, int version_id)
diff --git a/hw/mc146818rtc_regs.h b/hw/mc146818rtc_regs.h
index fc10076..ccdee42 100644
--- a/hw/mc146818rtc_regs.h
+++ b/hw/mc146818rtc_regs.h
@@ -44,6 +44,10 @@
 #define RTC_REG_C               12
 #define RTC_REG_D               13
 
+/* PC cmos mappings */
+#define RTC_CENTURY              0x32
+#define RTC_IBM_PS2_CENTURY_BYTE 0x37
+
 #define REG_A_UIP 0x80
 
 #define REG_B_SET  0x80
-- 
1.7.12

[Qemu-devel] [PATCH 3/3] rtc: implement century byte

[Paolo Bonzini]

Implement the century byte in the RTC emulation, and test that it works.
This leads to some annoying compatibility code because we need to treat
a value of 2000 for the base_year property as "use the century byte
properly" (which would be a value of 0).

The century byte will now be always-zero, rather than always-20,
for the MIPS Magnum machine whose base_year is 1980.  Commit 42fc73a
(Support epoch of 1980 in RTC emulation for MIPS Magnum, 2009-01-24)
correctly said:

    With an epoch of 1980 and a year of 2009, one could argue that [the
    century byte] should hold either 0, 1, 19 or 20.  NT 3.50 on MIPS
    does not read the century byte.

so I picked the simplest and most sensible implementation which is to
return 0 for 1980-2079, 1 for 2080-2179 and so on.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 hw/mc146818rtc.c | 27 ++++++++++++++++++---------
 tests/rtc-test.c | 32 ++++++++++++++++++++++++++++++--
 2 file modificati, 48 inserzioni(+), 11 rimozioni(-)

diff --git a/hw/mc146818rtc.c b/hw/mc146818rtc.c
index a7d20d5..332a77d 100644
--- a/hw/mc146818rtc.c
+++ b/hw/mc146818rtc.c
@@ -519,7 +519,9 @@ static void rtc_get_time(RTCState *s, struct tm *tm)
     tm->tm_wday = rtc_from_bcd(s, s->cmos_data[RTC_DAY_OF_WEEK]) - 1;
     tm->tm_mday = rtc_from_bcd(s, s->cmos_data[RTC_DAY_OF_MONTH]);
     tm->tm_mon = rtc_from_bcd(s, s->cmos_data[RTC_MONTH]) - 1;
-    tm->tm_year = rtc_from_bcd(s, s->cmos_data[RTC_YEAR]) + s->base_year - 1900;
+    tm->tm_year =
+        rtc_from_bcd(s, s->cmos_data[RTC_YEAR]) + s->base_year +
+        rtc_from_bcd(s, s->cmos_data[RTC_CENTURY]) * 100 - 1900;
 }
 
 static void rtc_set_time(RTCState *s)
@@ -552,10 +554,9 @@ static void rtc_set_cmos(RTCState *s, const struct tm *tm)
     s->cmos_data[RTC_DAY_OF_WEEK] = rtc_to_bcd(s, tm->tm_wday + 1);
     s->cmos_data[RTC_DAY_OF_MONTH] = rtc_to_bcd(s, tm->tm_mday);
     s->cmos_data[RTC_MONTH] = rtc_to_bcd(s, tm->tm_mon + 1);
-    year = (tm->tm_year - s->base_year) % 100;
-    if (year < 0)
-        year += 100;
-    s->cmos_data[RTC_YEAR] = rtc_to_bcd(s, year);
+    year = tm->tm_year + 1900 - s->base_year;
+    s->cmos_data[RTC_YEAR] = rtc_to_bcd(s, year % 100);
+    s->cmos_data[RTC_CENTURY] = rtc_to_bcd(s, year / 100);
 }
 
 static void rtc_update_time(RTCState *s)
@@ -673,7 +674,6 @@ static void rtc_set_date_from_host(ISADevice *dev)
 {
     RTCState *s = DO_UPCAST(RTCState, dev, dev);
     struct tm tm;
-    int val;
 
     qemu_get_timedate(&tm, 0);
 
@@ -683,9 +683,6 @@ static void rtc_set_date_from_host(ISADevice *dev)
 
     /* set the CMOS date */
     rtc_set_cmos(s, &tm);
-
-    val = rtc_to_bcd(s, (tm.tm_year / 100) + 19);
-    rtc_set_memory(dev, RTC_CENTURY, val);
 }
 
 static int rtc_post_load(void *opaque, int version_id)
@@ -810,6 +807,18 @@ static int rtc_initfn(ISADevice *dev)
     s->cmos_data[RTC_REG_C] = 0x00;
     s->cmos_data[RTC_REG_D] = 0x80;
 
+    /* This is for historical reasons.  The default base year qdev property
+     * was set to 2000 for most machine types before the century byte was
+     * implemented.
+     *
+     * This if statement means that the century byte will be always 0
+     * (at least until 2079...) for base_year = 1980, but will be set
+     * correctly for base_year = 2000.
+     */
+    if (s->base_year == 2000) {
+        s->base_year = 0;
+    }
+
     rtc_set_date_from_host(dev);
 
 #ifdef TARGET_I386
diff --git a/tests/rtc-test.c b/tests/rtc-test.c
index 2b9aa63..7fdc94a 100644
--- a/tests/rtc-test.c
+++ b/tests/rtc-test.c
@@ -179,12 +179,13 @@ static void check_time(int wiggle)
 
 static int wiggle = 2;
 
-static void set_year(void)
+static void set_year_20xx(void)
 {
     /* Set BCD mode */
     cmos_write(RTC_REG_B, cmos_read(RTC_REG_B) & ~REG_B_DM);
     cmos_write(RTC_REG_A, 0x76);
     cmos_write(RTC_YEAR, 0x11);
+    cmos_write(RTC_CENTURY, 0x20);
     cmos_write(RTC_MONTH, 0x02);
     cmos_write(RTC_DAY_OF_MONTH, 0x02);
     cmos_write(RTC_HOURS, 0x02);
@@ -198,6 +199,7 @@ static void set_year(void)
     g_assert_cmpint(cmos_read(RTC_DAY_OF_MONTH), ==, 0x02);
     g_assert_cmpint(cmos_read(RTC_MONTH), ==, 0x02);
     g_assert_cmpint(cmos_read(RTC_YEAR), ==, 0x11);
+    g_assert_cmpint(cmos_read(RTC_CENTURY), ==, 0x20);
 
     /* Set a date in 2080 to ensure there is no year-2038 overflow.  */
     cmos_write(RTC_REG_A, 0x76);
@@ -210,6 +212,7 @@ static void set_year(void)
     g_assert_cmpint(cmos_read(RTC_DAY_OF_MONTH), ==, 0x02);
     g_assert_cmpint(cmos_read(RTC_MONTH), ==, 0x02);
     g_assert_cmpint(cmos_read(RTC_YEAR), ==, 0x80);
+    g_assert_cmpint(cmos_read(RTC_CENTURY), ==, 0x20);
 
     cmos_write(RTC_REG_A, 0x76);
     cmos_write(RTC_YEAR, 0x11);
@@ -221,6 +224,30 @@ static void set_year(void)
     g_assert_cmpint(cmos_read(RTC_DAY_OF_MONTH), ==, 0x02);
     g_assert_cmpint(cmos_read(RTC_MONTH), ==, 0x02);
     g_assert_cmpint(cmos_read(RTC_YEAR), ==, 0x11);
+    g_assert_cmpint(cmos_read(RTC_CENTURY), ==, 0x20);
+}
+
+static void set_year_1980(void)
+{
+    /* Set BCD mode */
+    cmos_write(RTC_REG_B, cmos_read(RTC_REG_B) & ~REG_B_DM);
+    cmos_write(RTC_REG_A, 0x76);
+    cmos_write(RTC_YEAR, 0x80);
+    cmos_write(RTC_CENTURY, 0x19);
+    cmos_write(RTC_MONTH, 0x02);
+    cmos_write(RTC_DAY_OF_MONTH, 0x02);
+    cmos_write(RTC_HOURS, 0x02);
+    cmos_write(RTC_MINUTES, 0x04);
+    cmos_write(RTC_SECONDS, 0x58);
+    cmos_write(RTC_REG_A, 0x26);
+
+    g_assert_cmpint(cmos_read(RTC_HOURS), ==, 0x02);
+    g_assert_cmpint(cmos_read(RTC_MINUTES), ==, 0x04);
+    g_assert_cmpint(cmos_read(RTC_SECONDS), >=, 0x58);
+    g_assert_cmpint(cmos_read(RTC_DAY_OF_MONTH), ==, 0x02);
+    g_assert_cmpint(cmos_read(RTC_MONTH), ==, 0x02);
+    g_assert_cmpint(cmos_read(RTC_YEAR), ==, 0x80);
+    g_assert_cmpint(cmos_read(RTC_CENTURY), ==, 0x19);
 }
 
 static void bcd_check_time(void)
@@ -313,7 +340,8 @@ int main(int argc, char **argv)
     qtest_add_func("/rtc/bcd/check-time", bcd_check_time);
     qtest_add_func("/rtc/dec/check-time", dec_check_time);
     qtest_add_func("/rtc/alarm-time", alarm_time);
-    qtest_add_func("/rtc/set-year", set_year);
+    qtest_add_func("/rtc/set-year/20xx", set_year_20xx);
+    qtest_add_func("/rtc/set-year/1980", set_year_1980);
     qtest_add_func("/rtc/fuzz-registers", fuzz_registers);
     ret = g_test_run();
 
-- 
1.7.12

Re: [Qemu-devel] [PATCH 1/3] rtc: fix overflow in mktimegm

[Anthony Liguori]

Paolo Bonzini <pbonzini@redhat.com> writes:

> When setting a date in 1980, Linux is actually disregarding the century
> byte and setting the year to 2080.  This causes a year-2038 overflow
> in mktimegm.  Fix this by doing the days-to-seconds computation in
> 64-bit math.
>
> Reported-by: Lucas Meneghel Rodrigues <lookkas@gmail.com>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>

Applied. Thanks.

Regards,

Anthony Liguori

> ---
>  cutils.c         |  2 +-
>  tests/rtc-test.c | 45 +++++++++++++++++++++++++++++++++++++++++++++
>  2 file modificati, 46 inserzioni(+). 1 rimozione(-)
>
> diff --git a/cutils.c b/cutils.c
> index 8ef648f..8edd8fa 100644
> --- a/cutils.c
> +++ b/cutils.c
> @@ -115,7 +115,7 @@ time_t mktimegm(struct tm *tm)
>          m += 12;
>          y--;
>      }
> -    t = 86400 * (d + (153 * m - 457) / 5 + 365 * y + y / 4 - y / 100 + 
> +    t = 86400ULL * (d + (153 * m - 457) / 5 + 365 * y + y / 4 - y / 100 + 
>                   y / 400 - 719469);
>      t += 3600 * tm->tm_hour + 60 * tm->tm_min + tm->tm_sec;
>      return t;
> diff --git a/tests/rtc-test.c b/tests/rtc-test.c
> index f23ac3a..2b9aa63 100644
> --- a/tests/rtc-test.c
> +++ b/tests/rtc-test.c
> @@ -179,6 +179,50 @@ static void check_time(int wiggle)
>  
>  static int wiggle = 2;
>  
> +static void set_year(void)
> +{
> +    /* Set BCD mode */
> +    cmos_write(RTC_REG_B, cmos_read(RTC_REG_B) & ~REG_B_DM);
> +    cmos_write(RTC_REG_A, 0x76);
> +    cmos_write(RTC_YEAR, 0x11);
> +    cmos_write(RTC_MONTH, 0x02);
> +    cmos_write(RTC_DAY_OF_MONTH, 0x02);
> +    cmos_write(RTC_HOURS, 0x02);
> +    cmos_write(RTC_MINUTES, 0x04);
> +    cmos_write(RTC_SECONDS, 0x58);
> +    cmos_write(RTC_REG_A, 0x26);
> +
> +    g_assert_cmpint(cmos_read(RTC_HOURS), ==, 0x02);
> +    g_assert_cmpint(cmos_read(RTC_MINUTES), ==, 0x04);
> +    g_assert_cmpint(cmos_read(RTC_SECONDS), >=, 0x58);
> +    g_assert_cmpint(cmos_read(RTC_DAY_OF_MONTH), ==, 0x02);
> +    g_assert_cmpint(cmos_read(RTC_MONTH), ==, 0x02);
> +    g_assert_cmpint(cmos_read(RTC_YEAR), ==, 0x11);
> +
> +    /* Set a date in 2080 to ensure there is no year-2038 overflow.  */
> +    cmos_write(RTC_REG_A, 0x76);
> +    cmos_write(RTC_YEAR, 0x80);
> +    cmos_write(RTC_REG_A, 0x26);
> +
> +    g_assert_cmpint(cmos_read(RTC_HOURS), ==, 0x02);
> +    g_assert_cmpint(cmos_read(RTC_MINUTES), ==, 0x04);
> +    g_assert_cmpint(cmos_read(RTC_SECONDS), >=, 0x58);
> +    g_assert_cmpint(cmos_read(RTC_DAY_OF_MONTH), ==, 0x02);
> +    g_assert_cmpint(cmos_read(RTC_MONTH), ==, 0x02);
> +    g_assert_cmpint(cmos_read(RTC_YEAR), ==, 0x80);
> +
> +    cmos_write(RTC_REG_A, 0x76);
> +    cmos_write(RTC_YEAR, 0x11);
> +    cmos_write(RTC_REG_A, 0x26);
> +
> +    g_assert_cmpint(cmos_read(RTC_HOURS), ==, 0x02);
> +    g_assert_cmpint(cmos_read(RTC_MINUTES), ==, 0x04);
> +    g_assert_cmpint(cmos_read(RTC_SECONDS), >=, 0x58);
> +    g_assert_cmpint(cmos_read(RTC_DAY_OF_MONTH), ==, 0x02);
> +    g_assert_cmpint(cmos_read(RTC_MONTH), ==, 0x02);
> +    g_assert_cmpint(cmos_read(RTC_YEAR), ==, 0x11);
> +}
> +
>  static void bcd_check_time(void)
>  {
>      /* Set BCD mode */
> @@ -269,6 +313,7 @@ int main(int argc, char **argv)
>      qtest_add_func("/rtc/bcd/check-time", bcd_check_time);
>      qtest_add_func("/rtc/dec/check-time", dec_check_time);
>      qtest_add_func("/rtc/alarm-time", alarm_time);
> +    qtest_add_func("/rtc/set-year", set_year);
>      qtest_add_func("/rtc/fuzz-registers", fuzz_registers);
>      ret = g_test_run();
>  
> -- 
> 1.7.12