Hi Peter,
On 12/12/23 15:18, Peter Maydell wrote:
On Sun, 12 Nov 2023 at 09:22, Nikita Ostrenkov <n.ostrenkov@gmail.com> wrote:
Hi; thanks for this patch, and sorry I haven't got round
to reviewing it earlier.
Signed-off-by: Nikita Ostrenkov <n.ostrenkov@gmail.com>
---
hw/misc/imx7_snvs.c | 59 ++++++++++++++++++++++++++++++++-----
hw/misc/trace-events | 4 +--
include/hw/misc/imx7_snvs.h | 14 ++++++++-
3 files changed, 67 insertions(+), 10 deletions(-)
@@ -31,6 +33,16 @@ struct IMX7SNVSState {
SysBusDevice parent_obj;
MemoryRegion mmio;
+
+ /*
+ * Needed to preserve the tick_count across migration, even if the
+ * absolute value of the rtc_clock is different on the source and
+ * destination.
+ */
+ int64_t tick_offset_vmstate;
You don't need tick_offset_vmstate -- it is only in the p031
RTC device as a backwards-compatibility thing for older versions
of QEMU. Migrating tick_offset alone is sufficient in a new
device. (It seems to have been unfortunately copied-and-pasted
into the goldfish RTC device; we should probably fix that bug.)
+ int64_t tick_offset;
+
+ uint64_t lpcr;
We've now added state to this device, which means that it needs
a VMState structure to handle migration, and it needs a reset
function.
I just noticed your v1 review after reviewing v2. Indeed
'tick_offset' need to be migrated. Now about reset(), RTC
are somehow different. When resetting a machine in the same
QEMU process I'd expect the RTC offset to not be reset.
Could this be clarified by adding a reset handler with no
code but a comment /* RTC state is usually kept by CMOS
battery and is not reset */ maybe?