[PATCH] hw/timer/etraxfs_timer: Add vmstate for ETRAX timers

[PATCH] hw/timer/etraxfs_timer: Add vmstate for ETRAX timers

[Philippe Mathieu-Daudé]

Add the vmstate for the ETRAX timers.
This is in theory a migration compatibility break
for the 'AXIS devboard 88' CRIS machine.

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 hw/timer/etraxfs_timer.c | 34 +++++++++++++++++++++++++++++++++-
 1 file changed, 33 insertions(+), 1 deletion(-)

diff --git a/hw/timer/etraxfs_timer.c b/hw/timer/etraxfs_timer.c
index 4ba662190de..139e5b86a44 100644
--- a/hw/timer/etraxfs_timer.c
+++ b/hw/timer/etraxfs_timer.c
@@ -26,6 +26,7 @@
 #include "hw/sysbus.h"
 #include "sysemu/reset.h"
 #include "sysemu/runstate.h"
+#include "migration/vmstate.h"
 #include "qemu/module.h"
 #include "qemu/timer.h"
 #include "hw/irq.h"
@@ -64,7 +65,7 @@ struct ETRAXTimerState {
     ptimer_state *ptimer_t1;
     ptimer_state *ptimer_wd;
 
-    int wd_hits;
+    uint32_t wd_hits;
 
     /* Control registers.  */
     uint32_t rw_tmr0_div;
@@ -83,6 +84,36 @@ struct ETRAXTimerState {
     uint32_t r_masked_intr;
 };
 
+static const VMStateDescription vmstate_etraxfs = {
+    .name = "etraxfs",
+    .version_id = 0,
+    .minimum_version_id = 0,
+    .fields = (VMStateField[]) {
+        VMSTATE_PTIMER(ptimer_t0, ETRAXTimerState),
+        VMSTATE_PTIMER(ptimer_t1, ETRAXTimerState),
+        VMSTATE_PTIMER(ptimer_wd, ETRAXTimerState),
+
+        VMSTATE_UINT32(wd_hits, ETRAXTimerState),
+
+        VMSTATE_UINT32(rw_tmr0_div, ETRAXTimerState),
+        VMSTATE_UINT32(r_tmr0_data, ETRAXTimerState),
+        VMSTATE_UINT32(rw_tmr0_ctrl, ETRAXTimerState),
+
+        VMSTATE_UINT32(rw_tmr1_div, ETRAXTimerState),
+        VMSTATE_UINT32(r_tmr1_data, ETRAXTimerState),
+        VMSTATE_UINT32(rw_tmr1_ctrl, ETRAXTimerState),
+
+        VMSTATE_UINT32(rw_wd_ctrl, ETRAXTimerState),
+
+        VMSTATE_UINT32(rw_intr_mask, ETRAXTimerState),
+        VMSTATE_UINT32(rw_ack_intr, ETRAXTimerState),
+        VMSTATE_UINT32(r_intr, ETRAXTimerState),
+        VMSTATE_UINT32(r_masked_intr, ETRAXTimerState),
+
+        VMSTATE_END_OF_LIST()
+    }
+};
+
 static uint64_t
 timer_read(void *opaque, hwaddr addr, unsigned int size)
 {
@@ -357,6 +388,7 @@ static void etraxfs_timer_class_init(ObjectClass *klass, void *data)
     ResettableClass *rc = RESETTABLE_CLASS(klass);
 
     dc->realize = etraxfs_timer_realize;
+    dc->vmsd = &vmstate_etraxfs;
     rc->phases.enter = etraxfs_timer_reset_enter;
     rc->phases.hold = etraxfs_timer_reset_hold;
 }
-- 
2.31.1

Re: [PATCH] hw/timer/etraxfs_timer: Add vmstate for ETRAX timers

[Philippe Mathieu-Daudé]

ping?

On 11/6/21 11:56, Philippe Mathieu-Daudé wrote:
> Add the vmstate for the ETRAX timers.
> This is in theory a migration compatibility break
> for the 'AXIS devboard 88' CRIS machine.
> 
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
>  hw/timer/etraxfs_timer.c | 34 +++++++++++++++++++++++++++++++++-
>  1 file changed, 33 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/timer/etraxfs_timer.c b/hw/timer/etraxfs_timer.c
> index 4ba662190de..139e5b86a44 100644
> --- a/hw/timer/etraxfs_timer.c
> +++ b/hw/timer/etraxfs_timer.c
> @@ -26,6 +26,7 @@
>  #include "hw/sysbus.h"
>  #include "sysemu/reset.h"
>  #include "sysemu/runstate.h"
> +#include "migration/vmstate.h"
>  #include "qemu/module.h"
>  #include "qemu/timer.h"
>  #include "hw/irq.h"
> @@ -64,7 +65,7 @@ struct ETRAXTimerState {
>      ptimer_state *ptimer_t1;
>      ptimer_state *ptimer_wd;
>  
> -    int wd_hits;
> +    uint32_t wd_hits;
>  
>      /* Control registers.  */
>      uint32_t rw_tmr0_div;
> @@ -83,6 +84,36 @@ struct ETRAXTimerState {
>      uint32_t r_masked_intr;
>  };
>  
> +static const VMStateDescription vmstate_etraxfs = {
> +    .name = "etraxfs",
> +    .version_id = 0,
> +    .minimum_version_id = 0,
> +    .fields = (VMStateField[]) {
> +        VMSTATE_PTIMER(ptimer_t0, ETRAXTimerState),
> +        VMSTATE_PTIMER(ptimer_t1, ETRAXTimerState),
> +        VMSTATE_PTIMER(ptimer_wd, ETRAXTimerState),
> +
> +        VMSTATE_UINT32(wd_hits, ETRAXTimerState),
> +
> +        VMSTATE_UINT32(rw_tmr0_div, ETRAXTimerState),
> +        VMSTATE_UINT32(r_tmr0_data, ETRAXTimerState),
> +        VMSTATE_UINT32(rw_tmr0_ctrl, ETRAXTimerState),
> +
> +        VMSTATE_UINT32(rw_tmr1_div, ETRAXTimerState),
> +        VMSTATE_UINT32(r_tmr1_data, ETRAXTimerState),
> +        VMSTATE_UINT32(rw_tmr1_ctrl, ETRAXTimerState),
> +
> +        VMSTATE_UINT32(rw_wd_ctrl, ETRAXTimerState),
> +
> +        VMSTATE_UINT32(rw_intr_mask, ETRAXTimerState),
> +        VMSTATE_UINT32(rw_ack_intr, ETRAXTimerState),
> +        VMSTATE_UINT32(r_intr, ETRAXTimerState),
> +        VMSTATE_UINT32(r_masked_intr, ETRAXTimerState),
> +
> +        VMSTATE_END_OF_LIST()
> +    }
> +};
> +
>  static uint64_t
>  timer_read(void *opaque, hwaddr addr, unsigned int size)
>  {
> @@ -357,6 +388,7 @@ static void etraxfs_timer_class_init(ObjectClass *klass, void *data)
>      ResettableClass *rc = RESETTABLE_CLASS(klass);
>  
>      dc->realize = etraxfs_timer_realize;
> +    dc->vmsd = &vmstate_etraxfs;
>      rc->phases.enter = etraxfs_timer_reset_enter;
>      rc->phases.hold = etraxfs_timer_reset_hold;
>  }
> 

Re: [PATCH] hw/timer/etraxfs_timer: Add vmstate for ETRAX timers

[Richard Henderson]

On 12/17/21 3:37 PM, Philippe Mathieu-Daudé wrote:
> ping?
> 
> On 11/6/21 11:56, Philippe Mathieu-Daudé wrote:
>> Add the vmstate for the ETRAX timers.
>> This is in theory a migration compatibility break
>> for the 'AXIS devboard 88' CRIS machine.
>>
>> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
>> ---
>>   hw/timer/etraxfs_timer.c | 34 +++++++++++++++++++++++++++++++++-
>>   1 file changed, 33 insertions(+), 1 deletion(-)


In that it matches another similar timer device:
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>


>> +static const VMStateDescription vmstate_etraxfs = {
>> +    .name = "etraxfs",
>> +    .version_id = 0,
>> +    .minimum_version_id = 0,
>> +    .fields = (VMStateField[]) {
>> +        VMSTATE_PTIMER(ptimer_t0, ETRAXTimerState),
>> +        VMSTATE_PTIMER(ptimer_t1, ETRAXTimerState),
>> +        VMSTATE_PTIMER(ptimer_wd, ETRAXTimerState),
>> +
>> +        VMSTATE_UINT32(wd_hits, ETRAXTimerState),
>> +
>> +        VMSTATE_UINT32(rw_tmr0_div, ETRAXTimerState),
>> +        VMSTATE_UINT32(r_tmr0_data, ETRAXTimerState),
>> +        VMSTATE_UINT32(rw_tmr0_ctrl, ETRAXTimerState),
>> +
>> +        VMSTATE_UINT32(rw_tmr1_div, ETRAXTimerState),
>> +        VMSTATE_UINT32(r_tmr1_data, ETRAXTimerState),
>> +        VMSTATE_UINT32(rw_tmr1_ctrl, ETRAXTimerState),
>> +
>> +        VMSTATE_UINT32(rw_wd_ctrl, ETRAXTimerState),
>> +
>> +        VMSTATE_UINT32(rw_intr_mask, ETRAXTimerState),
>> +        VMSTATE_UINT32(rw_ack_intr, ETRAXTimerState),
>> +        VMSTATE_UINT32(r_intr, ETRAXTimerState),
>> +        VMSTATE_UINT32(r_masked_intr, ETRAXTimerState),
>> +
>> +        VMSTATE_END_OF_LIST()
>> +    }
>> +};

What I don't understand is how these controls get applied to qemu_irq after vmload, here 
or in any other device.  It seems like we should have some post_load hook that calls 
timer_update_irq, etc.


r~

Re: [PATCH] hw/timer/etraxfs_timer: Add vmstate for ETRAX timers

[Laurent Vivier]

Le 18/12/2021 à 03:28, Richard Henderson a écrit :
> On 12/17/21 3:37 PM, Philippe Mathieu-Daudé wrote:
>> ping?
>>
>> On 11/6/21 11:56, Philippe Mathieu-Daudé wrote:
>>> Add the vmstate for the ETRAX timers.
>>> This is in theory a migration compatibility break
>>> for the 'AXIS devboard 88' CRIS machine.
>>>
>>> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
>>> ---
>>>   hw/timer/etraxfs_timer.c | 34 +++++++++++++++++++++++++++++++++-
>>>   1 file changed, 33 insertions(+), 1 deletion(-)
> 
> 
> In that it matches another similar timer device:
> Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
> 
> 
>>> +static const VMStateDescription vmstate_etraxfs = {
>>> +    .name = "etraxfs",
>>> +    .version_id = 0,
>>> +    .minimum_version_id = 0,
>>> +    .fields = (VMStateField[]) {
>>> +        VMSTATE_PTIMER(ptimer_t0, ETRAXTimerState),
>>> +        VMSTATE_PTIMER(ptimer_t1, ETRAXTimerState),
>>> +        VMSTATE_PTIMER(ptimer_wd, ETRAXTimerState),
>>> +
>>> +        VMSTATE_UINT32(wd_hits, ETRAXTimerState),
>>> +
>>> +        VMSTATE_UINT32(rw_tmr0_div, ETRAXTimerState),
>>> +        VMSTATE_UINT32(r_tmr0_data, ETRAXTimerState),
>>> +        VMSTATE_UINT32(rw_tmr0_ctrl, ETRAXTimerState),
>>> +
>>> +        VMSTATE_UINT32(rw_tmr1_div, ETRAXTimerState),
>>> +        VMSTATE_UINT32(r_tmr1_data, ETRAXTimerState),
>>> +        VMSTATE_UINT32(rw_tmr1_ctrl, ETRAXTimerState),
>>> +
>>> +        VMSTATE_UINT32(rw_wd_ctrl, ETRAXTimerState),
>>> +
>>> +        VMSTATE_UINT32(rw_intr_mask, ETRAXTimerState),
>>> +        VMSTATE_UINT32(rw_ack_intr, ETRAXTimerState),
>>> +        VMSTATE_UINT32(r_intr, ETRAXTimerState),
>>> +        VMSTATE_UINT32(r_masked_intr, ETRAXTimerState),
>>> +
>>> +        VMSTATE_END_OF_LIST()
>>> +    }
>>> +};
> 
> What I don't understand is how these controls get applied to qemu_irq after vmload, here or in any 
> other device.  It seems like we should have some post_load hook that calls timer_update_irq, etc.
> 

FWIW, in VMSTATE_PTIMER(), we use a vmstate_ptimer struct that registers a vmstate_info_timer with 
VMSTATE_TIMER_PTR(). vmstate_info_timer uses timer_get() to update or delete the timer when it is 
loaded.

Applied to my trivial-patches branch.

Thanks,
Laurent

Re: [PATCH] hw/timer/etraxfs_timer: Add vmstate for ETRAX timers

[Peter Maydell]

On Sat, 18 Dec 2021 at 02:28, Richard Henderson
<richard.henderson@linaro.org> wrote:
> What I don't understand is how these controls get applied to qemu_irq after vmload, here
> or in any other device.  It seems like we should have some post_load hook that calls
> timer_update_irq, etc.

Very late answer, but: we don't need to call qemu_set_irq() on
qemu_irqs outbound from a device after a vmload, because IRQ
lines themselves have no state. The device on the other end of
the irq line also loads its own state, and typically that includes
its internal variables which it uses to track whether its input
lines are 0 or 1.

-- PMM