diff mbox

[2/4] i8254: Open-code timer restore

Message ID 77a9fa5aa9aae73889803315c291418a4d99505a.1328438750.git.jan.kiszka@web.de
State New
Headers show

Commit Message

Jan Kiszka Feb. 5, 2012, 10:46 a.m. UTC 
From: Jan Kiszka <jan.kiszka@siemens.com>

Same as for the APIC: To enable migration between accelerated and
non-accelerated models, we need to arm the channel 0 timer only inside
the emulated PIT model. The common code just saves/restores that timer
to the the next_transition_time field.

Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---
 hw/i8254.c        |   12 ++++++++++++
 hw/i8254_common.c |    6 +++---
 2 files changed, 15 insertions(+), 3 deletions(-)

Comments

Paolo Bonzini Feb. 5, 2012, 11:23 a.m. UTC | #1 
On 02/05/2012 11:46 AM, Jan Kiszka wrote:
> diff --git a/hw/i8254_common.c b/hw/i8254_common.c
> index 0601d88..b60fbda 100644
> --- a/hw/i8254_common.c
> +++ b/hw/i8254_common.c
> @@ -234,9 +234,8 @@ static int pit_load_old(QEMUFile *f, void *opaque, int version_id)
>           qemu_get_8s(f,&s->gate);
>           s->count_load_time = qemu_get_be64(f);
>           s->irq_disabled = 0;
> -        if (s->irq_timer) {
> +        if (i == 0) {
>               s->next_transition_time = qemu_get_be64(f);
> -            qemu_get_timer(f, s->irq_timer);
>           }
>       }
>       return 0;

You need to invoke the post load callback manually in the load_old 
callback; see vmstate_load_state:

     if  (version_id < vmsd->minimum_version_id) {
         return vmsd->load_state_old(f, opaque, version_id);
     }

I noticed that in apic_common's apic_load_old you don't have the bug, 
but on the other hand you're unconditionally loading into s->timer, so 
"old" migration to a destination with in-kernel APIC doesn't work:

     if (version_id >= 2) {
         qemu_get_timer(f, s->timer);
     }

Paolo
Jan Kiszka Feb. 5, 2012, 11:33 a.m. UTC | #2 
On 2012-02-05 12:23, Paolo Bonzini wrote:
> On 02/05/2012 11:46 AM, Jan Kiszka wrote:
>> diff --git a/hw/i8254_common.c b/hw/i8254_common.c
>> index 0601d88..b60fbda 100644
>> --- a/hw/i8254_common.c
>> +++ b/hw/i8254_common.c
>> @@ -234,9 +234,8 @@ static int pit_load_old(QEMUFile *f, void *opaque,
>> int version_id)
>>           qemu_get_8s(f,&s->gate);
>>           s->count_load_time = qemu_get_be64(f);
>>           s->irq_disabled = 0;
>> -        if (s->irq_timer) {
>> +        if (i == 0) {
>>               s->next_transition_time = qemu_get_be64(f);
>> -            qemu_get_timer(f, s->irq_timer);
>>           }
>>       }
>>       return 0;
> 
> You need to invoke the post load callback manually in the load_old
> callback; see vmstate_load_state:
> 
>     if  (version_id < vmsd->minimum_version_id) {
>         return vmsd->load_state_old(f, opaque, version_id);
>     }
> 
> I noticed that in apic_common's apic_load_old you don't have the bug,
> but on the other hand you're unconditionally loading into s->timer, so
> "old" migration to a destination with in-kernel APIC doesn't work:
> 
>     if (version_id >= 2) {
>         qemu_get_timer(f, s->timer);
>     }
> 

Hmm, true. The whole load_old is broken, in both APIC and PIT. Need to
call the post_load callbacks from there as well. Will fix, thanks.

Jan
diff mbox

Patch

diff --git a/hw/i8254.c b/hw/i8254.c
index f9f58e0..d41ee05 100644
--- a/hw/i8254.c
+++ b/hw/i8254.c
@@ -300,6 +300,17 @@  static const MemoryRegionOps pit_ioport_ops = {
     .old_portio = pit_portio
 };
 
+static void pit_post_load(PITCommonState *s)
+{
+    PITChannelState *sc = &s->channels[0];
+
+    if (sc->next_transition_time != -1) {
+        qemu_mod_timer(sc->irq_timer, sc->next_transition_time);
+    } else {
+        qemu_del_timer(sc->irq_timer);
+    }
+}
+
 static int pit_initfn(PITCommonState *pit)
 {
     PITChannelState *s;
@@ -329,6 +340,7 @@  static void pit_class_initfn(ObjectClass *klass, void *data)
     k->init = pit_initfn;
     k->set_channel_gate = pit_set_channel_gate;
     k->get_channel_info = pit_get_channel_info_common;
+    k->post_load = pit_post_load;
     dc->reset = pit_reset;
     dc->props = pit_properties;
 }
diff --git a/hw/i8254_common.c b/hw/i8254_common.c
index 0601d88..b60fbda 100644
--- a/hw/i8254_common.c
+++ b/hw/i8254_common.c
@@ -234,9 +234,8 @@  static int pit_load_old(QEMUFile *f, void *opaque, int version_id)
         qemu_get_8s(f, &s->gate);
         s->count_load_time = qemu_get_be64(f);
         s->irq_disabled = 0;
-        if (s->irq_timer) {
+        if (i == 0) {
             s->next_transition_time = qemu_get_be64(f);
-            qemu_get_timer(f, s->irq_timer);
         }
     }
     return 0;
@@ -275,7 +274,8 @@  static const VMStateDescription vmstate_pit_common = {
         VMSTATE_UINT32_V(channels[0].irq_disabled, PITCommonState, 3),
         VMSTATE_STRUCT_ARRAY(channels, PITCommonState, 3, 2,
                              vmstate_pit_channel, PITChannelState),
-        VMSTATE_TIMER(channels[0].irq_timer, PITCommonState),
+        VMSTATE_INT64(channels[0].next_transition_time,
+                      PITCommonState), /* formerly irq_timer */
         VMSTATE_END_OF_LIST()
     }
 };