[Qemu-devel] [PATCH] Change RTC time drift IRQ re-injection

[Qemu-devel] [PATCH] Change RTC time drift IRQ re-injection

[Gleb Natapov]

Currently IRQ are reinjected as soon as they are acknowledged to
the RTC, but Windows sometimes do acknowledgement in a loop with
global interrupt disabled waiting for interrupt to be cleared and
it does not mask RTC vector in PIC/APIC while doing this. In such
situation interrupt injection always fails and RTC interrupt is never
cleared.

Instead of reinjecting coalesced IRQs on acknowledgement the patch below
reinjects them by accelerating RTC clock a bit. This way RTC interrupt
is not constantly raced after coalesced interrupt.

Signed-off-by: Gleb Natapov <gleb@redhat.com>
diff --git a/hw/mc146818rtc.c b/hw/mc146818rtc.c
index 5b08d72..22b7a7f 100644
--- a/hw/mc146818rtc.c
+++ b/hw/mc146818rtc.c
@@ -71,6 +71,7 @@ struct RTCState {
 #ifdef TARGET_I386
     uint32_t irq_coalesced;
     uint32_t period;
+    QEMUTimer *coalesced_timer;
 #endif
     QEMUTimer *second_timer;
     QEMUTimer *second_timer2;
@@ -91,6 +92,37 @@ static void rtc_irq_raise(qemu_irq irq) {
 static void rtc_set_time(RTCState *s);
 static void rtc_copy_date(RTCState *s);
 
+#ifdef TARGET_I386
+static void rtc_coalesced_timer_update(RTCState *s)
+{
+    if (s->irq_coalesced == 0) {
+        qemu_del_timer(s->coalesced_timer);
+    } else {
+        /* divide each RTC interval to 2 - 8 smaller intervals */
+        int c = MIN(s->irq_coalesced, 7) + 1; 
+        int64_t next_clock = qemu_get_clock(vm_clock) +
+		muldiv64(s->period / c, ticks_per_sec, 32768);
+        qemu_mod_timer(s->coalesced_timer, next_clock);
+    }
+}
+
+static void rtc_coalesced_timer(void *opaque)
+{
+    RTCState *s = opaque;
+
+    if (s->irq_coalesced != 0) {
+        apic_reset_irq_delivered();
+        s->cmos_data[RTC_REG_C] |= 0xc0;
+        rtc_irq_raise(s->irq);
+        if (apic_get_irq_delivered()) {
+            s->irq_coalesced--;
+        }
+    }
+
+    rtc_coalesced_timer_update(s);
+}
+#endif
+
 static void rtc_timer_update(RTCState *s, int64_t current_time)
 {
     int period_code, period;
@@ -132,13 +164,17 @@ static void rtc_periodic_timer(void *opaque)
     RTCState *s = opaque;
 
     rtc_timer_update(s, s->next_periodic_time);
+    s->cmos_data[RTC_REG_C] |= 0xc0;
 #ifdef TARGET_I386
-    if ((s->cmos_data[RTC_REG_C] & 0xc0) && rtc_td_hack) {
-        s->irq_coalesced++;
-        return;
-    }
+    if(rtc_td_hack) {
+        apic_reset_irq_delivered();
+        rtc_irq_raise(s->irq);
+        if (!apic_get_irq_delivered()) {
+            s->irq_coalesced++;
+            rtc_coalesced_timer_update(s);
+        }
+    } else
 #endif
-    s->cmos_data[RTC_REG_C] |= 0xc0;
     rtc_irq_raise(s->irq);
 }
 
@@ -402,15 +438,6 @@ 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:
@@ -512,6 +539,7 @@ static void rtc_save_td(QEMUFile *f, void *opaque)
 
     qemu_put_be32(f, s->irq_coalesced);
     qemu_put_be32(f, s->period);
+    qemu_put_timer(f, s->coalesced_timer);
 }
 
 static int rtc_load_td(QEMUFile *f, void *opaque, int version_id)
@@ -523,6 +551,7 @@ static int rtc_load_td(QEMUFile *f, void *opaque, int version_id)
 
     s->irq_coalesced = qemu_get_be32(f);
     s->period = qemu_get_be32(f);
+    qemu_get_timer(f, s->coalesced_timer);
     return 0;
 }
 #endif
@@ -544,6 +573,10 @@ RTCState *rtc_init(int base, qemu_irq irq, int base_year)
 
     s->periodic_timer = qemu_new_timer(vm_clock,
                                        rtc_periodic_timer, s);
+#ifdef TARGET_I386
+    if (rtc_td_hack)
+        s->coalesced_timer = qemu_new_timer(vm_clock, rtc_coalesced_timer, s);
+#endif
     s->second_timer = qemu_new_timer(vm_clock,
                                      rtc_update_second, s);
     s->second_timer2 = qemu_new_timer(vm_clock,
--
			Gleb.

Re: [Qemu-devel] [PATCH] Change RTC time drift IRQ re-injection

[Anthony Liguori]

Gleb Natapov wrote:
> Currently IRQ are reinjected as soon as they are acknowledged to
> the RTC, but Windows sometimes do acknowledgement in a loop with
> global interrupt disabled waiting for interrupt to be cleared and
> it does not mask RTC vector in PIC/APIC while doing this. In such
> situation interrupt injection always fails and RTC interrupt is never
> cleared.
>
> Instead of reinjecting coalesced IRQs on acknowledgement the patch below
> reinjects them by accelerating RTC clock a bit. This way RTC interrupt
> is not constantly raced after coalesced interrupt.
>
> Signed-off-by: Gleb Natapov <gleb@redhat.com>
>              s->cmos_data[RTC_REG_C] = 0x00;
>              break;
>          default:
> @@ -512,6 +539,7 @@ static void rtc_save_td(QEMUFile *f, void *opaque)
>  
>      qemu_put_be32(f, s->irq_coalesced);
>      qemu_put_be32(f, s->period);
> +    qemu_put_timer(f, s->coalesced_timer);
>  }
>  
>  static int rtc_load_td(QEMUFile *f, void *opaque, int version_id)
> @@ -523,6 +551,7 @@ static int rtc_load_td(QEMUFile *f, void *opaque, int version_id)
>  
>      s->irq_coalesced = qemu_get_be32(f);
>      s->period = qemu_get_be32(f);
> +    qemu_get_timer(f, s->coalesced_timer);
>      return 0;
>   

If you're adding something to the savevm format, you have to increment 
the version_id and handle older versions correctly.


-- 
Regards,

Anthony Liguori

Re: [Qemu-devel] [PATCH] Change RTC time drift IRQ re-injection

[Dor Laor]

Anthony Liguori wrote:
> Gleb Natapov wrote:
>> Currently IRQ are reinjected as soon as they are acknowledged to
>> the RTC, but Windows sometimes do acknowledgement in a loop with
>> global interrupt disabled waiting for interrupt to be cleared and
>> it does not mask RTC vector in PIC/APIC while doing this. In such
>> situation interrupt injection always fails and RTC interrupt is never
>> cleared.
>>
>> Instead of reinjecting coalesced IRQs on acknowledgement the patch below
>> reinjects them by accelerating RTC clock a bit. This way RTC interrupt
>> is not constantly raced after coalesced interrupt.
>>
>> Signed-off-by: Gleb Natapov <gleb@redhat.com>
>>              s->cmos_data[RTC_REG_C] = 0x00;
>>              break;
>>          default:
>> @@ -512,6 +539,7 @@ static void rtc_save_td(QEMUFile *f, void *opaque)
>>  
>>      qemu_put_be32(f, s->irq_coalesced);
>>      qemu_put_be32(f, s->period);
>> +    qemu_put_timer(f, s->coalesced_timer);
>>  }
>>  
>>  static int rtc_load_td(QEMUFile *f, void *opaque, int version_id)
>> @@ -523,6 +551,7 @@ static int rtc_load_td(QEMUFile *f, void *opaque, 
>> int version_id)
>>  
>>      s->irq_coalesced = qemu_get_be32(f);
>>      s->period = qemu_get_be32(f);
>> +    qemu_get_timer(f, s->coalesced_timer);
>>      return 0;
>>   
>
> If you're adding something to the savevm format, you have to increment 
> the version_id and handle older versions correctly.
>
>
Actually, it is not a must to save this boolean, you can assume it is 
set and schedule it for the current time on the load.

Re: [Qemu-devel] [PATCH] Change RTC time drift IRQ re-injection

[Gleb Natapov]

On Sun, Apr 19, 2009 at 05:06:42PM +0300, Dor Laor wrote:
> Anthony Liguori wrote:
>> Gleb Natapov wrote:
>>> Currently IRQ are reinjected as soon as they are acknowledged to
>>> the RTC, but Windows sometimes do acknowledgement in a loop with
>>> global interrupt disabled waiting for interrupt to be cleared and
>>> it does not mask RTC vector in PIC/APIC while doing this. In such
>>> situation interrupt injection always fails and RTC interrupt is never
>>> cleared.
>>>
>>> Instead of reinjecting coalesced IRQs on acknowledgement the patch below
>>> reinjects them by accelerating RTC clock a bit. This way RTC interrupt
>>> is not constantly raced after coalesced interrupt.
>>>
>>> Signed-off-by: Gleb Natapov <gleb@redhat.com>
>>>              s->cmos_data[RTC_REG_C] = 0x00;
>>>              break;
>>>          default:
>>> @@ -512,6 +539,7 @@ static void rtc_save_td(QEMUFile *f, void *opaque)
>>>       qemu_put_be32(f, s->irq_coalesced);
>>>      qemu_put_be32(f, s->period);
>>> +    qemu_put_timer(f, s->coalesced_timer);
>>>  }
>>>   static int rtc_load_td(QEMUFile *f, void *opaque, int version_id)
>>> @@ -523,6 +551,7 @@ static int rtc_load_td(QEMUFile *f, void *opaque, 
>>> int version_id)
>>>       s->irq_coalesced = qemu_get_be32(f);
>>>      s->period = qemu_get_be32(f);
>>> +    qemu_get_timer(f, s->coalesced_timer);
>>>      return 0;
>>>   
>>
>> If you're adding something to the savevm format, you have to increment  
>> the version_id and handle older versions correctly.
>>
>>
> Actually, it is not a must to save this boolean, you can assume it is  
> set and schedule it for the current time on the load.
>
This is not boolean. This is timer. But you are right, there is no point
is saving it. It can be restarted after migration.

--
			Gleb.