| Message ID | 20231112092232.131-1-n.ostrenkov@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | fsl-imx: Add simple RTC emulation for i.MX6 and i.MX7 boards | expand |
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(-) > > diff --git a/hw/misc/imx7_snvs.c b/hw/misc/imx7_snvs.c > index a245f96cd4..7ef3e4901a 100644 > --- a/hw/misc/imx7_snvs.c > +++ b/hw/misc/imx7_snvs.c > @@ -13,29 +13,74 @@ > */ > > #include "qemu/osdep.h" > +#include "qemu/timer.h" > #include "hw/misc/imx7_snvs.h" > #include "qemu/module.h" > +#include "sysemu/sysemu.h" > #include "sysemu/runstate.h" > #include "trace.h" > > +#define RTC_FREQ 32768ULL > + > +static uint64_t imx7_snvs_get_count(IMX7SNVSState *s) > +{ > + int64_t now_ms = qemu_clock_get_ns(rtc_clock) / 1000000; > + return s->tick_offset + now_ms * RTC_FREQ / 1000; This kind of clock-to-ticks calculation should generally be done with muldiv64() to avoid possible overflows: int64_t ticks = muldiv64(qemu_clock_get_ns(rtc_clock), RTC_FREQ, NANOSECONDS_PER_SECOND); return s->tick_offset + ticks; > +} > + > static uint64_t imx7_snvs_read(void *opaque, hwaddr offset, unsigned size) > { > - trace_imx7_snvs_read(offset, 0); > + IMX7SNVSState *s = opaque; > + uint64_t ret = 0; > + > + switch (offset) { > + case SNVS_LPSRTCMR: > + ret = (imx7_snvs_get_count(s) >> 32) & 0x7fffU; > + break; > + case SNVS_LPSRTCLR: > + ret = imx7_snvs_get_count(s) & 0xffffffffU; > + break; > + case SNVS_LPCR: > + ret = s->lpcr; > + break; > + } > + > + trace_imx7_snvs_read(offset, ret, size); > > - return 0; > + return ret; > } > > static void imx7_snvs_write(void *opaque, hwaddr offset, > uint64_t v, unsigned size) > { > - const uint32_t value = v; > - const uint32_t mask = SNVS_LPCR_TOP | SNVS_LPCR_DP_EN; > + trace_imx7_snvs_write(offset, v, size); > > - trace_imx7_snvs_write(offset, value); > + IMX7SNVSState *s = opaque; > > - if (offset == SNVS_LPCR && ((value & mask) == mask)) { > - qemu_system_shutdown_request(SHUTDOWN_CAUSE_GUEST_SHUTDOWN); > + uint64_t new_value = 0; > + > + switch (offset) { > + case SNVS_LPSRTCMR: > + new_value = (imx7_snvs_get_count(s) & 0xffffffffU) | (v << 32); > + break; > + case SNVS_LPSRTCLR: > + new_value = (imx7_snvs_get_count(s) & 0x7fff00000000ULL) | v; > + break; > + case SNVS_LPCR: { > + s->lpcr = v; > + > + const uint32_t value = v; > + const uint32_t mask = SNVS_LPCR_TOP | SNVS_LPCR_DP_EN; > + > + if ((value & mask) == mask) > + qemu_system_shutdown_request(SHUTDOWN_CAUSE_GUEST_SHUTDOWN); > + > + break; > + } > } > + > + if (offset == SNVS_LPSRTCMR || offset == SNVS_LPSRTCLR) > + s->tick_offset += new_value - imx7_snvs_get_count(s); Our coding standard requires braces on all if() statements, even single line ones. I think for this update-the-count handling we should call imx7_snvs_get_count() only once, and then use that value both in constructing new_value and also here where we calculate the tick_offset. > } I think you need to initialise s->tick_offset in the device init routine, similar to what the pl031 device does. > static const struct MemoryRegionOps imx7_snvs_ops = { > diff --git a/hw/misc/trace-events b/hw/misc/trace-events > index 05ff692441..85725506bf 100644 > --- a/hw/misc/trace-events > +++ b/hw/misc/trace-events > @@ -116,8 +116,8 @@ imx7_gpr_read(uint64_t offset) "addr 0x%08" PRIx64 > imx7_gpr_write(uint64_t offset, uint64_t value) "addr 0x%08" PRIx64 "value 0x%08" PRIx64 > > # imx7_snvs.c > -imx7_snvs_read(uint64_t offset, uint32_t value) "addr 0x%08" PRIx64 "value 0x%08" PRIx32 > -imx7_snvs_write(uint64_t offset, uint32_t value) "addr 0x%08" PRIx64 "value 0x%08" PRIx32 > +imx7_snvs_read(uint64_t offset, uint64_t value, unsigned size) "i.MX SNVS read: offset 0x%08" PRIx64 " value 0x%08" PRIx64 " size %u" > +imx7_snvs_write(uint64_t offset, uint64_t value, unsigned size) "i.MX SNVS write: offset 0x%08" PRIx64 " value 0x%08" PRIx64 " size %u" > > # mos6522.c > mos6522_set_counter(int index, unsigned int val) "T%d.counter=%d" > diff --git a/include/hw/misc/imx7_snvs.h b/include/hw/misc/imx7_snvs.h > index 14a1d6fe6b..406c1fe97f 100644 > --- a/include/hw/misc/imx7_snvs.h > +++ b/include/hw/misc/imx7_snvs.h > @@ -20,7 +20,9 @@ > enum IMX7SNVSRegisters { > SNVS_LPCR = 0x38, > SNVS_LPCR_TOP = BIT(6), > - SNVS_LPCR_DP_EN = BIT(5) > + SNVS_LPCR_DP_EN = BIT(5), > + SNVS_LPSRTCMR = 0x050, /* Secure Real Time Counter MSB Register */ > + SNVS_LPSRTCLR = 0x054, /* Secure Real Time Counter LSB Register */ > }; > > #define TYPE_IMX7_SNVS "imx7.snvs" > @@ -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. > }; > > #endif /* IMX7_SNVS_H */ > -- thanks -- PMM
Hi! Thanks for your feedback. I've considered your comments and I'm sending a new patch. вт, 12 дек. 2023 г. в 17:18, Peter Maydell <peter.maydell@linaro.org>: > 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(-) > > > > diff --git a/hw/misc/imx7_snvs.c b/hw/misc/imx7_snvs.c > > index a245f96cd4..7ef3e4901a 100644 > > --- a/hw/misc/imx7_snvs.c > > +++ b/hw/misc/imx7_snvs.c > > @@ -13,29 +13,74 @@ > > */ > > > > #include "qemu/osdep.h" > > +#include "qemu/timer.h" > > #include "hw/misc/imx7_snvs.h" > > #include "qemu/module.h" > > +#include "sysemu/sysemu.h" > > #include "sysemu/runstate.h" > > #include "trace.h" > > > > +#define RTC_FREQ 32768ULL > > + > > +static uint64_t imx7_snvs_get_count(IMX7SNVSState *s) > > +{ > > + int64_t now_ms = qemu_clock_get_ns(rtc_clock) / 1000000; > > + return s->tick_offset + now_ms * RTC_FREQ / 1000; > > This kind of clock-to-ticks calculation should generally > be done with muldiv64() to avoid possible overflows: > > int64_t ticks = muldiv64(qemu_clock_get_ns(rtc_clock), RTC_FREQ, > NANOSECONDS_PER_SECOND); > return s->tick_offset + ticks; > > > +} > > + > > static uint64_t imx7_snvs_read(void *opaque, hwaddr offset, unsigned > size) > > { > > - trace_imx7_snvs_read(offset, 0); > > + IMX7SNVSState *s = opaque; > > + uint64_t ret = 0; > > + > > + switch (offset) { > > + case SNVS_LPSRTCMR: > > + ret = (imx7_snvs_get_count(s) >> 32) & 0x7fffU; > > + break; > > + case SNVS_LPSRTCLR: > > + ret = imx7_snvs_get_count(s) & 0xffffffffU; > > + break; > > + case SNVS_LPCR: > > + ret = s->lpcr; > > + break; > > + } > > + > > + trace_imx7_snvs_read(offset, ret, size); > > > > - return 0; > > + return ret; > > } > > > > static void imx7_snvs_write(void *opaque, hwaddr offset, > > uint64_t v, unsigned size) > > { > > - const uint32_t value = v; > > - const uint32_t mask = SNVS_LPCR_TOP | SNVS_LPCR_DP_EN; > > + trace_imx7_snvs_write(offset, v, size); > > > > - trace_imx7_snvs_write(offset, value); > > + IMX7SNVSState *s = opaque; > > > > - if (offset == SNVS_LPCR && ((value & mask) == mask)) { > > - qemu_system_shutdown_request(SHUTDOWN_CAUSE_GUEST_SHUTDOWN); > > + uint64_t new_value = 0; > > + > > + switch (offset) { > > + case SNVS_LPSRTCMR: > > + new_value = (imx7_snvs_get_count(s) & 0xffffffffU) | (v << 32); > > + break; > > + case SNVS_LPSRTCLR: > > + new_value = (imx7_snvs_get_count(s) & 0x7fff00000000ULL) | v; > > + break; > > + case SNVS_LPCR: { > > + s->lpcr = v; > > + > > + const uint32_t value = v; > > + const uint32_t mask = SNVS_LPCR_TOP | SNVS_LPCR_DP_EN; > > + > > + if ((value & mask) == mask) > > + qemu_system_shutdown_request(SHUTDOWN_CAUSE_GUEST_SHUTDOWN); > > + > > + break; > > + } > > } > > + > > + if (offset == SNVS_LPSRTCMR || offset == SNVS_LPSRTCLR) > > + s->tick_offset += new_value - imx7_snvs_get_count(s); > > Our coding standard requires braces on all if() statements, > even single line ones. > > I think for this update-the-count handling we should call > imx7_snvs_get_count() only once, and then use that value > both in constructing new_value and also here where we > calculate the tick_offset. > > > } > > I think you need to initialise s->tick_offset in the > device init routine, similar to what the pl031 device does. > > > static const struct MemoryRegionOps imx7_snvs_ops = { > > diff --git a/hw/misc/trace-events b/hw/misc/trace-events > > index 05ff692441..85725506bf 100644 > > --- a/hw/misc/trace-events > > +++ b/hw/misc/trace-events > > @@ -116,8 +116,8 @@ imx7_gpr_read(uint64_t offset) "addr 0x%08" PRIx64 > > imx7_gpr_write(uint64_t offset, uint64_t value) "addr 0x%08" PRIx64 > "value 0x%08" PRIx64 > > > > # imx7_snvs.c > > -imx7_snvs_read(uint64_t offset, uint32_t value) "addr 0x%08" PRIx64 > "value 0x%08" PRIx32 > > -imx7_snvs_write(uint64_t offset, uint32_t value) "addr 0x%08" PRIx64 > "value 0x%08" PRIx32 > > +imx7_snvs_read(uint64_t offset, uint64_t value, unsigned size) "i.MX > SNVS read: offset 0x%08" PRIx64 " value 0x%08" PRIx64 " size %u" > > +imx7_snvs_write(uint64_t offset, uint64_t value, unsigned size) "i.MX > SNVS write: offset 0x%08" PRIx64 " value 0x%08" PRIx64 " size %u" > > > > # mos6522.c > > mos6522_set_counter(int index, unsigned int val) "T%d.counter=%d" > > diff --git a/include/hw/misc/imx7_snvs.h b/include/hw/misc/imx7_snvs.h > > index 14a1d6fe6b..406c1fe97f 100644 > > --- a/include/hw/misc/imx7_snvs.h > > +++ b/include/hw/misc/imx7_snvs.h > > @@ -20,7 +20,9 @@ > > enum IMX7SNVSRegisters { > > SNVS_LPCR = 0x38, > > SNVS_LPCR_TOP = BIT(6), > > - SNVS_LPCR_DP_EN = BIT(5) > > + SNVS_LPCR_DP_EN = BIT(5), > > + SNVS_LPSRTCMR = 0x050, /* Secure Real Time Counter MSB Register */ > > + SNVS_LPSRTCLR = 0x054, /* Secure Real Time Counter LSB Register */ > > }; > > > > #define TYPE_IMX7_SNVS "imx7.snvs" > > @@ -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. > > > }; > > > > #endif /* IMX7_SNVS_H */ > > -- > > thanks > -- PMM >
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? > >> }; >> >> #endif /* IMX7_SNVS_H */ >> -- > > thanks > -- PMM >
On Wed, 13 Dec 2023 at 17:17, Philippe Mathieu-Daudé <philmd@linaro.org> wrote: > > 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? The lpcr register state needs to be reset. thanks -- PMM
On 13/12/23 18:20, Peter Maydell wrote: > On Wed, 13 Dec 2023 at 17:17, Philippe Mathieu-Daudé <philmd@linaro.org> wrote: >> >> 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? > > The lpcr register state needs to be reset. Oh, I missed its access in imx7_snvs_read(). > thanks > -- PMM
diff --git a/hw/misc/imx7_snvs.c b/hw/misc/imx7_snvs.c index a245f96cd4..7ef3e4901a 100644 --- a/hw/misc/imx7_snvs.c +++ b/hw/misc/imx7_snvs.c @@ -13,29 +13,74 @@ */ #include "qemu/osdep.h" +#include "qemu/timer.h" #include "hw/misc/imx7_snvs.h" #include "qemu/module.h" +#include "sysemu/sysemu.h" #include "sysemu/runstate.h" #include "trace.h" +#define RTC_FREQ 32768ULL + +static uint64_t imx7_snvs_get_count(IMX7SNVSState *s) +{ + int64_t now_ms = qemu_clock_get_ns(rtc_clock) / 1000000; + return s->tick_offset + now_ms * RTC_FREQ / 1000; +} + static uint64_t imx7_snvs_read(void *opaque, hwaddr offset, unsigned size) { - trace_imx7_snvs_read(offset, 0); + IMX7SNVSState *s = opaque; + uint64_t ret = 0; + + switch (offset) { + case SNVS_LPSRTCMR: + ret = (imx7_snvs_get_count(s) >> 32) & 0x7fffU; + break; + case SNVS_LPSRTCLR: + ret = imx7_snvs_get_count(s) & 0xffffffffU; + break; + case SNVS_LPCR: + ret = s->lpcr; + break; + } + + trace_imx7_snvs_read(offset, ret, size); - return 0; + return ret; } static void imx7_snvs_write(void *opaque, hwaddr offset, uint64_t v, unsigned size) { - const uint32_t value = v; - const uint32_t mask = SNVS_LPCR_TOP | SNVS_LPCR_DP_EN; + trace_imx7_snvs_write(offset, v, size); - trace_imx7_snvs_write(offset, value); + IMX7SNVSState *s = opaque; - if (offset == SNVS_LPCR && ((value & mask) == mask)) { - qemu_system_shutdown_request(SHUTDOWN_CAUSE_GUEST_SHUTDOWN); + uint64_t new_value = 0; + + switch (offset) { + case SNVS_LPSRTCMR: + new_value = (imx7_snvs_get_count(s) & 0xffffffffU) | (v << 32); + break; + case SNVS_LPSRTCLR: + new_value = (imx7_snvs_get_count(s) & 0x7fff00000000ULL) | v; + break; + case SNVS_LPCR: { + s->lpcr = v; + + const uint32_t value = v; + const uint32_t mask = SNVS_LPCR_TOP | SNVS_LPCR_DP_EN; + + if ((value & mask) == mask) + qemu_system_shutdown_request(SHUTDOWN_CAUSE_GUEST_SHUTDOWN); + + break; + } } + + if (offset == SNVS_LPSRTCMR || offset == SNVS_LPSRTCLR) + s->tick_offset += new_value - imx7_snvs_get_count(s); } static const struct MemoryRegionOps imx7_snvs_ops = { diff --git a/hw/misc/trace-events b/hw/misc/trace-events index 05ff692441..85725506bf 100644 --- a/hw/misc/trace-events +++ b/hw/misc/trace-events @@ -116,8 +116,8 @@ imx7_gpr_read(uint64_t offset) "addr 0x%08" PRIx64 imx7_gpr_write(uint64_t offset, uint64_t value) "addr 0x%08" PRIx64 "value 0x%08" PRIx64 # imx7_snvs.c -imx7_snvs_read(uint64_t offset, uint32_t value) "addr 0x%08" PRIx64 "value 0x%08" PRIx32 -imx7_snvs_write(uint64_t offset, uint32_t value) "addr 0x%08" PRIx64 "value 0x%08" PRIx32 +imx7_snvs_read(uint64_t offset, uint64_t value, unsigned size) "i.MX SNVS read: offset 0x%08" PRIx64 " value 0x%08" PRIx64 " size %u" +imx7_snvs_write(uint64_t offset, uint64_t value, unsigned size) "i.MX SNVS write: offset 0x%08" PRIx64 " value 0x%08" PRIx64 " size %u" # mos6522.c mos6522_set_counter(int index, unsigned int val) "T%d.counter=%d" diff --git a/include/hw/misc/imx7_snvs.h b/include/hw/misc/imx7_snvs.h index 14a1d6fe6b..406c1fe97f 100644 --- a/include/hw/misc/imx7_snvs.h +++ b/include/hw/misc/imx7_snvs.h @@ -20,7 +20,9 @@ enum IMX7SNVSRegisters { SNVS_LPCR = 0x38, SNVS_LPCR_TOP = BIT(6), - SNVS_LPCR_DP_EN = BIT(5) + SNVS_LPCR_DP_EN = BIT(5), + SNVS_LPSRTCMR = 0x050, /* Secure Real Time Counter MSB Register */ + SNVS_LPSRTCLR = 0x054, /* Secure Real Time Counter LSB Register */ }; #define TYPE_IMX7_SNVS "imx7.snvs" @@ -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; + int64_t tick_offset; + + uint64_t lpcr; }; #endif /* IMX7_SNVS_H */
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(-)