[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v2] fsl-imx: add simple RTC emulation for i.MX6 and i.MX7 boa
From: |
Philippe Mathieu-Daudé |
Subject: |
Re: [PATCH v2] fsl-imx: add simple RTC emulation for i.MX6 and i.MX7 boards |
Date: |
Wed, 13 Dec 2023 16:53:52 +0100 |
User-agent: |
Mozilla Thunderbird |
Hi Nikita,
On 13/12/23 16:24, Nikita Ostrenkov wrote:
Signed-off-by: Nikita Ostrenkov <n.ostrenkov@gmail.com>
---
hw/misc/imx7_snvs.c | 70 +++++++++++++++++++++++++++++++++----
hw/misc/trace-events | 4 +--
include/hw/misc/imx7_snvs.h | 7 +++-
3 files changed, 71 insertions(+), 10 deletions(-)
diff --git a/hw/misc/imx7_snvs.c b/hw/misc/imx7_snvs.c
index a245f96cd4..98fe51aa66 100644
--- a/hw/misc/imx7_snvs.c
+++ b/hw/misc/imx7_snvs.c
@@ -13,28 +13,79 @@
*/
#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 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;
- return 0;
+ switch (offset) {
+ case SNVS_LPSRTCMR:
+ ret = (imx7_snvs_get_count(s) >> 32) & 0x7fffU;
Please have a look at extract64() which might make your code
easier to read. It is declared in include/qemu/bitops.h.
+ break;
+ case SNVS_LPSRTCLR:
+ ret = imx7_snvs_get_count(s) & 0xffffffffU;
Ditto.
+ break;
+ case SNVS_LPCR:
+ ret = s->lpcr;
+ break;
+ }
+
+ trace_imx7_snvs_read(offset, ret, size);
+
+ 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);
+
+ IMX7SNVSState *s = opaque;
- trace_imx7_snvs_write(offset, value);
+ uint64_t new_value = 0, snvs_count = 0;
- if (offset == SNVS_LPCR && ((value & mask) == mask)) {
- qemu_system_shutdown_request(SHUTDOWN_CAUSE_GUEST_SHUTDOWN);
+ if (offset == SNVS_LPSRTCMR || offset == SNVS_LPSRTCLR) {
+ snvs_count = imx7_snvs_get_count(s);
+ }
+
+ switch (offset) {
+ case SNVS_LPSRTCMR:
+ new_value = (snvs_count & 0xffffffffU) | (v << 32);
Here the equivalent is deposit64().
+ break;
+ case SNVS_LPSRTCLR:
+ new_value = (snvs_count & 0x7fff00000000ULL) | v;
Ditto.
+ break;
+ case SNVS_LPCR: {
+ s->lpcr = v;
+
+ const uint32_t value = v;
'value' is not really needed.
+ 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 - snvs_count;
}
}
@@ -59,11 +110,16 @@ static void imx7_snvs_init(Object *obj)
{
SysBusDevice *sd = SYS_BUS_DEVICE(obj);
IMX7SNVSState *s = IMX7_SNVS(obj);
+ struct tm tm;
memory_region_init_io(&s->mmio, obj, &imx7_snvs_ops, s,
TYPE_IMX7_SNVS, 0x1000);
sysbus_init_mmio(sd, &s->mmio);
+
+ qemu_get_timedate(&tm, 0);
+ s->tick_offset = mktimegm(&tm) -
+ qemu_clock_get_ns(rtc_clock) / NANOSECONDS_PER_SECOND;
So 'tick_offset' should be saved for migration. But I wonder
about replay, do we need this to be a property so user can set
and offset to emulate a RTC in host past/future? Maybe out of
scope, but still please add VMSTATE_INT64(tick_offset).
}
static void imx7_snvs_class_init(ObjectClass *klass, void *data)
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..26c497b8ed 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),
(FWIW SNVS_LPCR_TOP/SNVS_LPCR_DP_EN aren't IMX7SNVSRegisters enum.)
+ 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,9 @@ struct IMX7SNVSState {
SysBusDevice parent_obj;
MemoryRegion mmio;
+
+ int64_t tick_offset;
+ uint64_t lpcr;
Why do we need 'lpcr' in the instance state? It doesn't seem used.
};
#endif /* IMX7_SNVS_H */
Modulo few comments, the patch LGTM!
Regards,
Phil.