Il 19/03/2012 07:13, Zhang, Yang Z ha scritto:
> Current RTC emulation uses periodic timer(2 timers per second) to
> update RTC clock. And it will stop CPU staying at deep C-state for
> long period. Our experience shows the Pkg C6 residency reduced 6%
> when running 64 idle guest. The following patch stop the two periodic
> timer and only updating RTC clock when guest try to read it.

I attach a patch that fixes some problems with divider reset and in
general simplifies the logic.  Even with the patch, however, I still see
failures in my test case unfortunately.  Probably there are rounding
errors somewhere.

Also (minor change) you need to use rtc_clock instead of host_clock.

I'm afraid that we're hitting a wall. :(  The problem I have is that the
logic in your patch is very complex and, without understanding it well,
I'm afraid we'll never be able to fix it completely (while the old
inefficient one is buggy but it _can_ be fixed).

By the way in general the SET bit and the divider should be handled in
the same way, because both stop the updates.  That is, in general there
should be a function like

static inline bool rtc_running(RTCState *s)
{
    return (!(s->cmos_data[RTC_REG_B] & REG_B_SET) &&
            (s->cmos_data[RTC_REG_A] & 0x70) == 0x20);
}

that is used instead of testing REG_B_SET.  I pushed some work along
these lines at git://github.com/bonzini/qemu.git (branch rtc-intel), but
it does not really improve the situation with respect to rounding errors
so the bug must be elsewhere.

Paolo
>From 198afe37adb532738a55dd32ef8bd533c2493c65 Mon Sep 17 00:00:00 2001
From: Paolo Bonzini <pbonz...@redhat.com>
Date: Tue, 20 Mar 2012 12:21:00 +0100
Subject: [PATCH] fixes

Signed-off-by: Paolo Bonzini <pbonz...@redhat.com>
---
 hw/mc146818rtc.c |   34 ++++++++++++++++------------------
 1 files changed, 16 insertions(+), 18 deletions(-)

diff --git a/hw/mc146818rtc.c b/hw/mc146818rtc.c
index 22a9f40..f94ddbb 100644
--- a/hw/mc146818rtc.c
+++ b/hw/mc146818rtc.c
@@ -123,8 +123,6 @@ static void rtc_set_cmos(RTCState *s);
 static inline int rtc_from_bcd(RTCState *s, int a);
 static uint64_t get_next_alarm(RTCState *s);
 
-static int32_t divider_reset;
-
 static uint64_t get_guest_rtc_us(RTCState *s)
 {
     int64_t host_usec, offset_usec, guest_usec;
@@ -489,7 +487,8 @@ static void rtc_update_timer2(void *opaque)
     RTCState *s = opaque;
     int32_t alarm_fired;
 
-    if (!(s->cmos_data[RTC_REG_B] & REG_B_SET)) {
+    if (!(s->cmos_data[RTC_REG_B] & REG_B_SET) &&
+        (s->cmos_data[RTC_REG_A] & 0x70) == 0x20) {
         s->cmos_data[RTC_REG_C] |= REG_C_UF;
         if (check_alarm(s)) {
             s->cmos_data[RTC_REG_C] |= REG_C_AF;
@@ -561,16 +560,16 @@ static void cmos_ioport_write(void *opaque, uint32_t addr, uint32_t data)
         case RTC_REG_A:
             /* when the divider reset is removed, the first update cycle
              * begins one-half second later*/
-            if (((s->cmos_data[RTC_REG_A] & 0x60) == 0x60) &&
-                                    ((data & 0x70) >> 4) <= 2) {
-                divider_reset = 1;
-                if (!(s->cmos_data[RTC_REG_B] & REG_B_SET)) {
-                    rtc_calibrate_time(s);
-                    rtc_set_offset(s, 500000);
-                    s->cmos_data[RTC_REG_A] &= ~REG_A_UIP;
-                    check_update_timer(s);
-                    divider_reset = 0;
-                }
+            if ((data & 0x60) == 0x60 &&
+                (s->cmos_data[RTC_REG_A] & 0x70) <= 0x20) {
+                rtc_calibrate_time(s);
+                rtc_set_cmos(s);
+                s->cmos_data[RTC_REG_A] &= ~REG_A_UIP;
+	    } else if ((s->cmos_data[RTC_REG_A] & 0x60) == 0x60 &&
+                       (data & 0x70) <= 0x20) {
+                rtc_set_time(s);
+                rtc_set_offset(s, 500000);
+                check_update_timer(s);
             }
             /* UIP bit is read only */
             s->cmos_data[RTC_REG_A] = (data & ~REG_A_UIP) |
@@ -591,11 +590,7 @@ static void cmos_ioport_write(void *opaque, uint32_t addr, uint32_t data)
                 /* if disabling set mode, update the time */
                 if (s->cmos_data[RTC_REG_B] & REG_B_SET) {
                     rtc_set_time(s);
-                    if (divider_reset == 1) {
-                        rtc_set_offset(s, 500000);
-                        s->cmos_data[RTC_REG_A] &= ~REG_A_UIP;
-                        divider_reset = 0;
-                    } else {
+                    if (((s->cmos_data[RTC_REG_A] & 0x70) >> 4) <= 2) {
                         rtc_set_offset(s, 0);
                     }
                 }
@@ -709,6 +704,9 @@ static int update_in_progress(RTCState *s)
     if (s->cmos_data[RTC_REG_B] & REG_B_SET) {
         return 0;
     }
+    if ((s->cmos_data[RTC_REG_A] & 0x60) == 0x60) {
+        return 0;
+    }
     guest_usec = get_guest_rtc_us(s);
     /* UIP bit will be set at last 244us of every second. */
     if ((guest_usec % USEC_PER_SEC) >= (USEC_PER_SEC - 244)) {
-- 
1.7.7.6

Reply via email to