Re: [Qemu-devel] [PATCH] Time drift again.

[Gleb Natapov]

On Tue, Dec 16, 2008 at 02:38:12PM +0200, Gleb Natapov wrote:
> On Tue, Dec 16, 2008 at 12:33:30PM +0000, Paul Brook wrote:
> > On Tuesday 16 December 2008, Gleb Natapov wrote:
> > > Hello,
> > >
> > >  After my last patch to fix interrupt coalescing was rejected
> > > on the basis that it is too intrusive we decided to make the
> > > fix much more localized and only fix the problem for RTC time
> > > source. Unfortunately it is impossible to fix the problem entirely
> > > inside RTC code like Andrzej proposed since Windows reads RTC
> > > register C more then once on each time interrupt so it is impossible
> > > to count reliably how many interrupt windows actually handled.
> > > Proposed solution is localized to I386 target and is disabled by
> > > default. To enable it "-rtc-td-hack" flag should be used.
> > 
> > You need documentation for the new option.
> > 
> Will add to the next patch.
> 

Here is the same patch with doc section.

Signed-off-by: Gleb Natapov <address@hidden>
diff --git a/hw/apic.c b/hw/apic.c
index a2915f8..dfe51d8 100644
--- a/hw/apic.c
+++ b/hw/apic.c
@@ -100,6 +100,8 @@ struct IOAPICState {
 static int apic_io_memory;
 static APICState *local_apics[MAX_APICS + 1];
 static int last_apic_id = 0;
+static int apic_irq_delivered;
+
 
 static void apic_init_ipi(APICState *s);
 static void apic_set_irq(APICState *s, int vector_num, int trigger_mode);
@@ -133,6 +135,14 @@ static inline void reset_bit(uint32_t *tab, int index)
     tab[i] &= ~mask;
 }
 
+static inline int get_bit(uint32_t *tab, int index)
+{
+    int i, mask;
+    i = index >> 5;
+    mask = 1 << (index & 0x1f);
+    return !!(tab[i] & mask);
+}
+
 static void apic_local_deliver(CPUState *env, int vector)
 {
     APICState *s = env->apic_state;
@@ -349,8 +359,20 @@ static void apic_update_irq(APICState *s)
     cpu_interrupt(s->cpu_env, CPU_INTERRUPT_HARD);
 }
 
+void apic_reset_irq_delivered(void)
+{
+    apic_irq_delivered = 0;
+}
+
+int apic_get_irq_delivered(void)
+{
+    return apic_irq_delivered;
+}
+
 static void apic_set_irq(APICState *s, int vector_num, int trigger_mode)
 {
+    apic_irq_delivered += !get_bit(s->irr, vector_num);
+
     set_bit(s->irr, vector_num);
     if (trigger_mode)
         set_bit(s->tmr, vector_num);
diff --git a/hw/mc146818rtc.c b/hw/mc146818rtc.c
index ac41a94..acaba91 100644
--- a/hw/mc146818rtc.c
+++ b/hw/mc146818rtc.c
@@ -65,6 +65,10 @@ struct RTCState {
     int64_t next_periodic_time;
     /* second update */
     int64_t next_second_time;
+#ifdef TARGET_I386
+    uint32_t irq_coalesced;
+    uint32_t period;
+#endif
     QEMUTimer *second_timer;
     QEMUTimer *second_timer2;
 };
@@ -84,12 +88,20 @@ static void rtc_timer_update(RTCState *s, int64_t 
current_time)
             period_code += 7;
         /* period in 32 Khz cycles */
         period = 1 << (period_code - 1);
+#ifdef TARGET_I386
+        if(period != s->period)
+            s->irq_coalesced = (s->irq_coalesced * s->period) / period;
+        s->period = period;
+#endif
         /* compute 32 khz clock */
         cur_clock = muldiv64(current_time, 32768, ticks_per_sec);
         next_irq_clock = (cur_clock & ~(period - 1)) + period;
         s->next_periodic_time = muldiv64(next_irq_clock, ticks_per_sec, 32768) 
+ 1;
         qemu_mod_timer(s->periodic_timer, s->next_periodic_time);
     } else {
+#ifdef TARGET_I386
+        s->irq_coalesced = 0;
+#endif
         qemu_del_timer(s->periodic_timer);
     }
 }
@@ -99,6 +111,12 @@ static void rtc_periodic_timer(void *opaque)
     RTCState *s = opaque;
 
     rtc_timer_update(s, s->next_periodic_time);
+#ifdef TARGET_I386
+    if ((s->cmos_data[RTC_REG_C] & 0xc0) && rtc_td_hack) {
+        s->irq_coalesced++;
+        return;
+    }
+#endif
     s->cmos_data[RTC_REG_C] |= 0xc0;
     qemu_irq_raise(s->irq);
 }
@@ -359,6 +377,15 @@ static uint32_t cmos_ioport_read(void *opaque, uint32_t 
addr)
         case RTC_REG_C:
             ret = s->cmos_data[s->cmos_index];
             qemu_irq_lower(s->irq);
+#ifdef TARGET_I386
+            if(s->irq_coalesced) {
+                apic_reset_irq_delivered();
+                qemu_irq_raise(s->irq);
+                if (apic_get_irq_delivered())
+                    s->irq_coalesced--;
+                break;
+            }
+#endif
             s->cmos_data[RTC_REG_C] = 0x00;
             break;
         default:
@@ -453,6 +480,28 @@ static int rtc_load(QEMUFile *f, void *opaque, int 
version_id)
     return 0;
 }
 
+#ifdef TARGET_I386
+static void rtc_save_td(QEMUFile *f, void *opaque)
+{
+    RTCState *s = opaque;
+
+    qemu_put_be32(f, s->irq_coalesced);
+    qemu_put_be32(f, s->period);
+}
+
+static int rtc_load_td(QEMUFile *f, void *opaque, int version_id)
+{
+    RTCState *s = opaque;
+
+    if (version_id != 1)
+        return -EINVAL;
+
+    s->irq_coalesced = qemu_get_be32(f);
+    s->period = qemu_get_be32(f);
+    return 0;
+}
+#endif
+
 RTCState *rtc_init(int base, qemu_irq irq)
 {
     RTCState *s;
@@ -483,6 +532,9 @@ RTCState *rtc_init(int base, qemu_irq irq)
     register_ioport_read(base, 2, 1, cmos_ioport_read, s);
 
     register_savevm("mc146818rtc", base, 1, rtc_save, rtc_load, s);
+#ifdef TARGET_I386
+    register_savevm("mc146818rtc-td", base, 1, rtc_save_td, rtc_load_td, s);
+#endif
     return s;
 }
 
@@ -589,5 +641,8 @@ RTCState *rtc_mm_init(target_phys_addr_t base, int 
it_shift, qemu_irq irq)
     cpu_register_physical_memory(base, 2 << it_shift, io_memory);
 
     register_savevm("mc146818rtc", base, 1, rtc_save, rtc_load, s);
+#ifdef TARGET_I386
+    register_savevm("mc146818rtc-td", base, 1, rtc_save_td, rtc_load_td, s);
+#endif
     return s;
 }
diff --git a/hw/pc.h b/hw/pc.h
index 39b220f..840f00a 100644
--- a/hw/pc.h
+++ b/hw/pc.h
@@ -46,6 +46,8 @@ void apic_deliver_pic_intr(CPUState *env, int level);
 int apic_get_interrupt(CPUState *env);
 IOAPICState *ioapic_init(void);
 void ioapic_set_irq(void *opaque, int vector, int level);
+void apic_reset_irq_delivered(void);
+int apic_get_irq_delivered(void);
 
 /* i8254.c */
 
diff --git a/qemu-doc.texi b/qemu-doc.texi
index 698e0d5..dfbddab 100644
--- a/qemu-doc.texi
+++ b/qemu-doc.texi
@@ -418,6 +418,11 @@ Use it when installing Windows 2000 to avoid a disk full 
bug. After
 Windows 2000 is installed, you no longer need this option (this option
 slows down the IDE transfers).
 
address@hidden -rtc-td-hack
+Use it if you experience time drift problem in Windows with ACPI HAL.
+This option will try to figure out how many timer interrupts were not
+processed by the Windows guest and will re-inject them.
+
 @item -option-rom @var{file}
 Load the contents of @var{file} as an option ROM.
 This option is useful to load things like EtherBoot.
diff --git a/sysemu.h b/sysemu.h
index 94cffaf..4a24f12 100644
--- a/sysemu.h
+++ b/sysemu.h
@@ -89,6 +89,7 @@ extern int graphic_depth;
 extern int nographic;
 extern const char *keyboard_layout;
 extern int win2k_install_hack;
+extern int rtc_td_hack;
 extern int alt_grab;
 extern int usb_enabled;
 extern int smp_cpus;
diff --git a/vl.c b/vl.c
index 0a02151..a515f53 100644
--- a/vl.c
+++ b/vl.c
@@ -220,6 +220,7 @@ CharDriverState *parallel_hds[MAX_PARALLEL_PORTS];
 #ifdef TARGET_I386
 int win2k_install_hack = 0;
 #endif
+int rtc_td_hack = 0;
 int usb_enabled = 0;
 int smp_cpus = 1;
 const char *vnc_display;
@@ -3869,6 +3870,7 @@ static void help(int exitcode)
 #ifdef TARGET_I386
            "-win2k-hack     use it when installing Windows 2000 to avoid a 
disk full bug\n"
 #endif
+           "-rtc-td-hack    use it to fix time drift in Windows ACPI HAL\n"
            "-usb            enable the USB driver (will be the default soon)\n"
            "-usbdevice name add the host or guest USB device 'name'\n"
 #if defined(TARGET_PPC) || defined(TARGET_SPARC)
@@ -4062,6 +4064,7 @@ enum {
     QEMU_OPTION_kernel_kqemu,
     QEMU_OPTION_enable_kvm,
     QEMU_OPTION_win2k_hack,
+    QEMU_OPTION_rtc_td_hack,
     QEMU_OPTION_usb,
     QEMU_OPTION_usbdevice,
     QEMU_OPTION_smp,
@@ -4169,6 +4172,7 @@ static const QEMUOption qemu_options[] = {
 #endif
     { "pidfile", HAS_ARG, QEMU_OPTION_pidfile },
     { "win2k-hack", 0, QEMU_OPTION_win2k_hack },
+    { "rtc-td-hack", 0, QEMU_OPTION_rtc_td_hack },
     { "usbdevice", HAS_ARG, QEMU_OPTION_usbdevice },
     { "smp", HAS_ARG, QEMU_OPTION_smp },
     { "vnc", HAS_ARG, QEMU_OPTION_vnc },
@@ -4976,6 +4980,9 @@ int main(int argc, char **argv, char **envp)
                 win2k_install_hack = 1;
                 break;
 #endif
+            case QEMU_OPTION_rtc_td_hack:
+                rtc_td_hack = 1;
+                break;
 #ifdef USE_KQEMU
             case QEMU_OPTION_no_kqemu:
                 kqemu_allowed = 0;
--
                        Gleb.