diff mbox series

[v3] fsl-imx: add simple RTC emulation for i.MX6 and i.MX7 boards

Message ID CAC8KSA16N+DsCzPVE64NvqxQZZfst67prPbu=nzPTfHiFCEgdw@mail.gmail.com
State New
Headers show
Series [v3] fsl-imx: add simple RTC emulation for i.MX6 and i.MX7 boards | expand

Commit Message

Nikita Ostrenkov Dec. 14, 2023, 11:49 a.m. UTC
Signed-off-by: Nikita Ostrenkov <n.ostrenkov@gmail.com>
---
 hw/misc/imx7_snvs.c         | 91 ++++++++++++++++++++++++++++++++++---
 hw/misc/trace-events        |  4 +-
 include/hw/misc/imx7_snvs.h |  7 ++-
 3 files changed, 92 insertions(+), 10 deletions(-)

Comments

Peter Maydell Dec. 14, 2023, 5:23 p.m. UTC | #1
On Thu, 14 Dec 2023 at 11:49, Nikita Ostrenkov <n.ostrenkov@gmail.com> wrote:
>
> Signed-off-by: Nikita Ostrenkov <n.ostrenkov@gmail.com>
> ---
>  hw/misc/imx7_snvs.c         | 91 ++++++++++++++++++++++++++++++++++---
>  hw/misc/trace-events        |  4 +-
>  include/hw/misc/imx7_snvs.h |  7 ++-
>  3 files changed, 92 insertions(+), 10 deletions(-)

Hi; this doesn't compile for me:
../../hw/misc/imx7_snvs.c:139:5: error: implicit declaration of
function 'qemu_get_timedate' is invalid in C99
[-Werror,-Wimplicit-function-declaration]
    qemu_get_timedate(&tm, 0);
    ^
../../hw/misc/imx7_snvs.c:140:22: error: implicit declaration of
function 'mktimegm' is invalid in C99
[-Werror,-Wimplicit-function-declaration]
    s->tick_offset = mktimegm(&tm) -
                     ^

How have you been testing it? This looks like a missing
include line, so I'm wondering if you've been testing it
against an older version of QEMU rather than the current
head-of-git? (If I fix that error then there's another
one after it because the include line to get mktimegm() is
missing too.)

Also, your email client has unfortunately mangled the patch in
a couple of ways:
 * it's sent it as combined HTML/text, not as plain text only
 * it has wrapped some long lines
 * it has sent it base64 encoded

If you're planning to submit more QEMU patches in future,
it would be worth looking at the notes in
https://www.qemu.org/docs/master/devel/submitting-a-patch.html#submitting-your-patches
about 'git send-email'. For a single patch, I can fix stuff
up by hand at this end, but it's a bit awkward.

Other than that, the patch looks good to me.

thanks
-- PMM
Nikita Ostrenkov Dec. 14, 2023, 9:45 p.m. UTC | #2
Sorry again. Honestly, I was in a hurry.

Unfortunately, I'm currently using a different PC, and git publish doesn't
work on it (some issues with SSL certificates). I sent the first and second
versions of the patch from my main home PC through this way, but,
regrettably, I had to manually send the third version via the Gmail web
client.

You're right. I tested the patch on qemu v8.1.3. However, I made the
changes on the master branch.

Tomorrow, I will definitely test the patch on the master branch and try to
resolve the issues with git publish. I'll send the fourth version of the
patch.

Thanks!

чт, 14 дек. 2023 г., 20:23 Peter Maydell <peter.maydell@linaro.org>:

> On Thu, 14 Dec 2023 at 11:49, Nikita Ostrenkov <n.ostrenkov@gmail.com>
> wrote:
> >
> > Signed-off-by: Nikita Ostrenkov <n.ostrenkov@gmail.com>
> > ---
> >  hw/misc/imx7_snvs.c         | 91 ++++++++++++++++++++++++++++++++++---
> >  hw/misc/trace-events        |  4 +-
> >  include/hw/misc/imx7_snvs.h |  7 ++-
> >  3 files changed, 92 insertions(+), 10 deletions(-)
>
> Hi; this doesn't compile for me:
> ../../hw/misc/imx7_snvs.c:139:5: error: implicit declaration of
> function 'qemu_get_timedate' is invalid in C99
> [-Werror,-Wimplicit-function-declaration]
>     qemu_get_timedate(&tm, 0);
>     ^
> ../../hw/misc/imx7_snvs.c:140:22: error: implicit declaration of
> function 'mktimegm' is invalid in C99
> [-Werror,-Wimplicit-function-declaration]
>     s->tick_offset = mktimegm(&tm) -
>                      ^
>
> How have you been testing it? This looks like a missing
> include line, so I'm wondering if you've been testing it
> against an older version of QEMU rather than the current
> head-of-git? (If I fix that error then there's another
> one after it because the include line to get mktimegm() is
> missing too.)
>
> Also, your email client has unfortunately mangled the patch in
> a couple of ways:
>  * it's sent it as combined HTML/text, not as plain text only
>  * it has wrapped some long lines
>  * it has sent it base64 encoded
>
> If you're planning to submit more QEMU patches in future,
> it would be worth looking at the notes in
>
> https://www.qemu.org/docs/master/devel/submitting-a-patch.html#submitting-your-patches
> about 'git send-email'. For a single patch, I can fix stuff
> up by hand at this end, but it's a bit awkward.
>
> Other than that, the patch looks good to me.
>
> thanks
> -- PMM
>
diff mbox series

Patch

diff --git a/hw/misc/imx7_snvs.c b/hw/misc/imx7_snvs.c
index a245f96cd4..7769c39499 100644
--- a/hw/misc/imx7_snvs.c
+++ b/hw/misc/imx7_snvs.c
@@ -13,28 +13,98 @@ 
  */

 #include "qemu/osdep.h"
+#include "qemu/bitops.h"
+#include "qemu/timer.h"
+#include "migration/vmstate.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 const VMStateDescription vmstate_imx7_snvs = {
+    .name = TYPE_IMX7_SNVS,
+    .version_id = 1,
+    .minimum_version_id = 1,
+    .fields = (VMStateField[]) {
+        VMSTATE_UINT64(tick_offset, IMX7SNVSState),
+        VMSTATE_UINT64(lpcr, IMX7SNVSState),
+        VMSTATE_END_OF_LIST()
+    }
+};
+
+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 = IMX7_SNVS(opaque);
+    uint64_t ret = 0;
+
+    switch (offset) {
+    case SNVS_LPSRTCMR:
+        ret = extract64(imx7_snvs_get_count(s), 32, 15);
+        break;
+    case SNVS_LPSRTCLR:
+        ret = extract64(imx7_snvs_get_count(s), 0, 32);
+        break;
+    case SNVS_LPCR:
+        ret = s->lpcr;
+        break;
+    }

-    return 0;
+    trace_imx7_snvs_read(offset, ret, size);
+
+    return ret;
+}
+
+static void imx7_snvs_reset(DeviceState *dev)
+{
+    IMX7SNVSState *s = IMX7_SNVS(dev);
+
+    s->lpcr = 0;
 }

 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 = IMX7_SNVS(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 = deposit64(snvs_count, 32, 32, v);
+        break;
+    case SNVS_LPSRTCLR:
+        new_value = deposit64(snvs_count, 0, 32, v);
+        break;
+    case SNVS_LPCR: {
+        s->lpcr = v;
+
+        const uint32_t mask  = SNVS_LPCR_TOP | SNVS_LPCR_DP_EN;
+
+        if ((v & 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,17 +129,24 @@  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;
 }

 static void imx7_snvs_class_init(ObjectClass *klass, void *data)
 {
     DeviceClass *dc = DEVICE_CLASS(klass);

+    dc->reset = imx7_snvs_reset;
+    dc->vmsd = &vmstate_imx7_snvs;
     dc->desc  = "i.MX7 Secure Non-Volatile Storage Module";
 }

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..1272076086 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,9 @@  struct IMX7SNVSState {
     SysBusDevice parent_obj;

     MemoryRegion mmio;
+
+    uint64_t tick_offset;
+    uint64_t lpcr;
 };

 #endif /* IMX7_SNVS_H */