[Qemu-devel] [PATCH v5 0/3] Add and connect the ZynqMP RTC

[Qemu-devel] [PATCH v5 0/3] Add and connect the ZynqMP RTC

[Alistair Francis]

V5:
 - Recalculate tick_offset after migration

V4:
 - Use the RegisterAccessInfo .unimp functionality

V3:
 - Store an offset value
 - Use mktimegm()
 - Log unimplemented writes

V2:
 - Delete unused realise function
 - Add cover letter
 - Convert DB_PRINT() macro to trace


Alistair Francis (3):
  xlnx-zynqmp-rtc: Initial commit
  xlnx-zynqmp-rtc: Add basic time support
  xlnx-zynqmp: Connect the RTC device

 include/hw/arm/xlnx-zynqmp.h       |   2 +
 include/hw/timer/xlnx-zynqmp-rtc.h |  87 ++++++++++++
 hw/arm/xlnx-zynqmp.c               |  14 ++
 hw/timer/xlnx-zynqmp-rtc.c         | 266 +++++++++++++++++++++++++++++++++++++
 hw/timer/Makefile.objs             |   1 +
 hw/timer/trace-events              |   3 +
 6 files changed, 373 insertions(+)
 create mode 100644 include/hw/timer/xlnx-zynqmp-rtc.h
 create mode 100644 hw/timer/xlnx-zynqmp-rtc.c

-- 
2.14.1

[Qemu-devel] [PATCH v5 1/3] xlnx-zynqmp-rtc: Initial commit

[Alistair Francis]

Initial commit of the ZynqMP RTC device.

Signed-off-by: Alistair Francis <alistair.francis@xilinx.com>
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---

v5:
 - Don't use intermediate val
V2:
 - Delete unused realise function
 - Remove DB_PRINT()

 include/hw/timer/xlnx-zynqmp-rtc.h |  84 +++++++++++++++
 hw/timer/xlnx-zynqmp-rtc.c         | 214 +++++++++++++++++++++++++++++++++++++
 hw/timer/Makefile.objs             |   1 +
 3 files changed, 299 insertions(+)
 create mode 100644 include/hw/timer/xlnx-zynqmp-rtc.h
 create mode 100644 hw/timer/xlnx-zynqmp-rtc.c

diff --git a/include/hw/timer/xlnx-zynqmp-rtc.h b/include/hw/timer/xlnx-zynqmp-rtc.h
new file mode 100644
index 0000000000..87649836cc
--- /dev/null
+++ b/include/hw/timer/xlnx-zynqmp-rtc.h
@@ -0,0 +1,84 @@
+/*
+ * QEMU model of the Xilinx ZynqMP Real Time Clock (RTC).
+ *
+ * Copyright (c) 2017 Xilinx Inc.
+ *
+ * Written-by: Alistair Francis <alistair.francis@xilinx.com>
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ */
+
+#include "hw/register.h"
+
+#define TYPE_XLNX_ZYNQMP_RTC "xlnx-zynmp.rtc"
+
+#define XLNX_ZYNQMP_RTC(obj) \
+     OBJECT_CHECK(XlnxZynqMPRTC, (obj), TYPE_XLNX_ZYNQMP_RTC)
+
+REG32(SET_TIME_WRITE, 0x0)
+REG32(SET_TIME_READ, 0x4)
+REG32(CALIB_WRITE, 0x8)
+    FIELD(CALIB_WRITE, FRACTION_EN, 20, 1)
+    FIELD(CALIB_WRITE, FRACTION_DATA, 16, 4)
+    FIELD(CALIB_WRITE, MAX_TICK, 0, 16)
+REG32(CALIB_READ, 0xc)
+    FIELD(CALIB_READ, FRACTION_EN, 20, 1)
+    FIELD(CALIB_READ, FRACTION_DATA, 16, 4)
+    FIELD(CALIB_READ, MAX_TICK, 0, 16)
+REG32(CURRENT_TIME, 0x10)
+REG32(CURRENT_TICK, 0x14)
+    FIELD(CURRENT_TICK, VALUE, 0, 16)
+REG32(ALARM, 0x18)
+REG32(RTC_INT_STATUS, 0x20)
+    FIELD(RTC_INT_STATUS, ALARM, 1, 1)
+    FIELD(RTC_INT_STATUS, SECONDS, 0, 1)
+REG32(RTC_INT_MASK, 0x24)
+    FIELD(RTC_INT_MASK, ALARM, 1, 1)
+    FIELD(RTC_INT_MASK, SECONDS, 0, 1)
+REG32(RTC_INT_EN, 0x28)
+    FIELD(RTC_INT_EN, ALARM, 1, 1)
+    FIELD(RTC_INT_EN, SECONDS, 0, 1)
+REG32(RTC_INT_DIS, 0x2c)
+    FIELD(RTC_INT_DIS, ALARM, 1, 1)
+    FIELD(RTC_INT_DIS, SECONDS, 0, 1)
+REG32(ADDR_ERROR, 0x30)
+    FIELD(ADDR_ERROR, STATUS, 0, 1)
+REG32(ADDR_ERROR_INT_MASK, 0x34)
+    FIELD(ADDR_ERROR_INT_MASK, MASK, 0, 1)
+REG32(ADDR_ERROR_INT_EN, 0x38)
+    FIELD(ADDR_ERROR_INT_EN, MASK, 0, 1)
+REG32(ADDR_ERROR_INT_DIS, 0x3c)
+    FIELD(ADDR_ERROR_INT_DIS, MASK, 0, 1)
+REG32(CONTROL, 0x40)
+    FIELD(CONTROL, BATTERY_DISABLE, 31, 1)
+    FIELD(CONTROL, OSC_CNTRL, 24, 4)
+    FIELD(CONTROL, SLVERR_ENABLE, 0, 1)
+REG32(SAFETY_CHK, 0x50)
+
+#define XLNX_ZYNQMP_RTC_R_MAX (R_SAFETY_CHK + 1)
+
+typedef struct XlnxZynqMPRTC {
+    SysBusDevice parent_obj;
+    MemoryRegion iomem;
+    qemu_irq irq_rtc_int;
+    qemu_irq irq_addr_error_int;
+
+    uint32_t regs[XLNX_ZYNQMP_RTC_R_MAX];
+    RegisterInfo regs_info[XLNX_ZYNQMP_RTC_R_MAX];
+} XlnxZynqMPRTC;
diff --git a/hw/timer/xlnx-zynqmp-rtc.c b/hw/timer/xlnx-zynqmp-rtc.c
new file mode 100644
index 0000000000..707f145027
--- /dev/null
+++ b/hw/timer/xlnx-zynqmp-rtc.c
@@ -0,0 +1,214 @@
+/*
+ * QEMU model of the Xilinx ZynqMP Real Time Clock (RTC).
+ *
+ * Copyright (c) 2017 Xilinx Inc.
+ *
+ * Written-by: Alistair Francis <alistair.francis@xilinx.com>
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ */
+
+#include "qemu/osdep.h"
+#include "hw/sysbus.h"
+#include "hw/register.h"
+#include "qemu/bitops.h"
+#include "qemu/log.h"
+#include "hw/timer/xlnx-zynqmp-rtc.h"
+
+#ifndef XLNX_ZYNQMP_RTC_ERR_DEBUG
+#define XLNX_ZYNQMP_RTC_ERR_DEBUG 0
+#endif
+
+static void rtc_int_update_irq(XlnxZynqMPRTC *s)
+{
+    bool pending = s->regs[R_RTC_INT_STATUS] & ~s->regs[R_RTC_INT_MASK];
+    qemu_set_irq(s->irq_rtc_int, pending);
+}
+
+static void addr_error_int_update_irq(XlnxZynqMPRTC *s)
+{
+    bool pending = s->regs[R_ADDR_ERROR] & ~s->regs[R_ADDR_ERROR_INT_MASK];
+    qemu_set_irq(s->irq_addr_error_int, pending);
+}
+
+static void rtc_int_status_postw(RegisterInfo *reg, uint64_t val64)
+{
+    XlnxZynqMPRTC *s = XLNX_ZYNQMP_RTC(reg->opaque);
+    rtc_int_update_irq(s);
+}
+
+static uint64_t rtc_int_en_prew(RegisterInfo *reg, uint64_t val64)
+{
+    XlnxZynqMPRTC *s = XLNX_ZYNQMP_RTC(reg->opaque);
+
+    s->regs[R_RTC_INT_MASK] &= (uint32_t) ~val64;
+    rtc_int_update_irq(s);
+    return 0;
+}
+
+static uint64_t rtc_int_dis_prew(RegisterInfo *reg, uint64_t val64)
+{
+    XlnxZynqMPRTC *s = XLNX_ZYNQMP_RTC(reg->opaque);
+
+    s->regs[R_RTC_INT_MASK] |= (uint32_t) val64;
+    rtc_int_update_irq(s);
+    return 0;
+}
+
+static void addr_error_postw(RegisterInfo *reg, uint64_t val64)
+{
+    XlnxZynqMPRTC *s = XLNX_ZYNQMP_RTC(reg->opaque);
+    addr_error_int_update_irq(s);
+}
+
+static uint64_t addr_error_int_en_prew(RegisterInfo *reg, uint64_t val64)
+{
+    XlnxZynqMPRTC *s = XLNX_ZYNQMP_RTC(reg->opaque);
+
+    s->regs[R_ADDR_ERROR_INT_MASK] &= (uint32_t) ~val64;
+    addr_error_int_update_irq(s);
+    return 0;
+}
+
+static uint64_t addr_error_int_dis_prew(RegisterInfo *reg, uint64_t val64)
+{
+    XlnxZynqMPRTC *s = XLNX_ZYNQMP_RTC(reg->opaque);
+
+    s->regs[R_ADDR_ERROR_INT_MASK] |= (uint32_t) val64;
+    addr_error_int_update_irq(s);
+    return 0;
+}
+
+static const RegisterAccessInfo rtc_regs_info[] = {
+    {   .name = "SET_TIME_WRITE",  .addr = A_SET_TIME_WRITE,
+    },{ .name = "SET_TIME_READ",  .addr = A_SET_TIME_READ,
+        .ro = 0xffffffff,
+    },{ .name = "CALIB_WRITE",  .addr = A_CALIB_WRITE,
+    },{ .name = "CALIB_READ",  .addr = A_CALIB_READ,
+        .ro = 0x1fffff,
+    },{ .name = "CURRENT_TIME",  .addr = A_CURRENT_TIME,
+        .ro = 0xffffffff,
+    },{ .name = "CURRENT_TICK",  .addr = A_CURRENT_TICK,
+        .ro = 0xffff,
+    },{ .name = "ALARM",  .addr = A_ALARM,
+    },{ .name = "RTC_INT_STATUS",  .addr = A_RTC_INT_STATUS,
+        .w1c = 0x3,
+        .post_write = rtc_int_status_postw,
+    },{ .name = "RTC_INT_MASK",  .addr = A_RTC_INT_MASK,
+        .reset = 0x3,
+        .ro = 0x3,
+    },{ .name = "RTC_INT_EN",  .addr = A_RTC_INT_EN,
+        .pre_write = rtc_int_en_prew,
+    },{ .name = "RTC_INT_DIS",  .addr = A_RTC_INT_DIS,
+        .pre_write = rtc_int_dis_prew,
+    },{ .name = "ADDR_ERROR",  .addr = A_ADDR_ERROR,
+        .w1c = 0x1,
+        .post_write = addr_error_postw,
+    },{ .name = "ADDR_ERROR_INT_MASK",  .addr = A_ADDR_ERROR_INT_MASK,
+        .reset = 0x1,
+        .ro = 0x1,
+    },{ .name = "ADDR_ERROR_INT_EN",  .addr = A_ADDR_ERROR_INT_EN,
+        .pre_write = addr_error_int_en_prew,
+    },{ .name = "ADDR_ERROR_INT_DIS",  .addr = A_ADDR_ERROR_INT_DIS,
+        .pre_write = addr_error_int_dis_prew,
+    },{ .name = "CONTROL",  .addr = A_CONTROL,
+        .reset = 0x1000000,
+        .rsvd = 0x70fffffe,
+    },{ .name = "SAFETY_CHK",  .addr = A_SAFETY_CHK,
+    }
+};
+
+static void rtc_reset(DeviceState *dev)
+{
+    XlnxZynqMPRTC *s = XLNX_ZYNQMP_RTC(dev);
+    unsigned int i;
+
+    for (i = 0; i < ARRAY_SIZE(s->regs_info); ++i) {
+        register_reset(&s->regs_info[i]);
+    }
+
+    rtc_int_update_irq(s);
+    addr_error_int_update_irq(s);
+}
+
+static const MemoryRegionOps rtc_ops = {
+    .read = register_read_memory,
+    .write = register_write_memory,
+    .endianness = DEVICE_LITTLE_ENDIAN,
+    .valid = {
+        .min_access_size = 4,
+        .max_access_size = 4,
+    },
+};
+
+static void rtc_init(Object *obj)
+{
+    XlnxZynqMPRTC *s = XLNX_ZYNQMP_RTC(obj);
+    SysBusDevice *sbd = SYS_BUS_DEVICE(obj);
+    RegisterInfoArray *reg_array;
+
+    memory_region_init(&s->iomem, obj, TYPE_XLNX_ZYNQMP_RTC,
+                       XLNX_ZYNQMP_RTC_R_MAX * 4);
+    reg_array =
+        register_init_block32(DEVICE(obj), rtc_regs_info,
+                              ARRAY_SIZE(rtc_regs_info),
+                              s->regs_info, s->regs,
+                              &rtc_ops,
+                              XLNX_ZYNQMP_RTC_ERR_DEBUG,
+                              XLNX_ZYNQMP_RTC_R_MAX * 4);
+    memory_region_add_subregion(&s->iomem,
+                                0x0,
+                                &reg_array->mem);
+    sysbus_init_mmio(sbd, &s->iomem);
+    sysbus_init_irq(sbd, &s->irq_rtc_int);
+    sysbus_init_irq(sbd, &s->irq_addr_error_int);
+}
+
+static const VMStateDescription vmstate_rtc = {
+    .name = TYPE_XLNX_ZYNQMP_RTC,
+    .version_id = 1,
+    .minimum_version_id = 1,
+    .fields = (VMStateField[]) {
+        VMSTATE_UINT32_ARRAY(regs, XlnxZynqMPRTC, XLNX_ZYNQMP_RTC_R_MAX),
+        VMSTATE_END_OF_LIST(),
+    }
+};
+
+static void rtc_class_init(ObjectClass *klass, void *data)
+{
+    DeviceClass *dc = DEVICE_CLASS(klass);
+
+    dc->reset = rtc_reset;
+    dc->vmsd = &vmstate_rtc;
+}
+
+static const TypeInfo rtc_info = {
+    .name          = TYPE_XLNX_ZYNQMP_RTC,
+    .parent        = TYPE_SYS_BUS_DEVICE,
+    .instance_size = sizeof(XlnxZynqMPRTC),
+    .class_init    = rtc_class_init,
+    .instance_init = rtc_init,
+};
+
+static void rtc_register_types(void)
+{
+    type_register_static(&rtc_info);
+}
+
+type_init(rtc_register_types)
diff --git a/hw/timer/Makefile.objs b/hw/timer/Makefile.objs
index 8c19eac3b6..8b27a4b7ef 100644
--- a/hw/timer/Makefile.objs
+++ b/hw/timer/Makefile.objs
@@ -21,6 +21,7 @@ common-obj-$(CONFIG_IMX) += imx_epit.o
 common-obj-$(CONFIG_IMX) += imx_gpt.o
 common-obj-$(CONFIG_LM32) += lm32_timer.o
 common-obj-$(CONFIG_MILKYMIST) += milkymist-sysctl.o
+common-obj-$(CONFIG_XLNX_ZYNQMP) += xlnx-zynqmp-rtc.o
 
 obj-$(CONFIG_ALTERA_TIMER) += altera_timer.o
 obj-$(CONFIG_EXYNOS4) += exynos4210_mct.o
-- 
2.14.1

[Qemu-devel] [PATCH v5 2/3] xlnx-zynqmp-rtc: Add basic time support

[Alistair Francis]

Allow the guest to determine the time set from the QEMU command line.

This includes adding a trace event to debug the new time.

Signed-off-by: Alistair Francis <alistair.francis@xilinx.com>
---
V5:
 - Recalculate tick_offset after migration
V4:
 - Use the .unimp property
V3:
 - Store an offset value
 - Use mktimegm()
 - Log unimplemented writes
V2:
 - Convert DB_PRINT() macro to trace

 include/hw/timer/xlnx-zynqmp-rtc.h |  3 +++
 hw/timer/xlnx-zynqmp-rtc.c         | 52 ++++++++++++++++++++++++++++++++++++++
 hw/timer/trace-events              |  3 +++
 3 files changed, 58 insertions(+)

diff --git a/include/hw/timer/xlnx-zynqmp-rtc.h b/include/hw/timer/xlnx-zynqmp-rtc.h
index 87649836cc..2867563bdd 100644
--- a/include/hw/timer/xlnx-zynqmp-rtc.h
+++ b/include/hw/timer/xlnx-zynqmp-rtc.h
@@ -79,6 +79,9 @@ typedef struct XlnxZynqMPRTC {
     qemu_irq irq_rtc_int;
     qemu_irq irq_addr_error_int;
 
+    struct tm current_tm;
+    uint32_t tick_offset;
+
     uint32_t regs[XLNX_ZYNQMP_RTC_R_MAX];
     RegisterInfo regs_info[XLNX_ZYNQMP_RTC_R_MAX];
 } XlnxZynqMPRTC;
diff --git a/hw/timer/xlnx-zynqmp-rtc.c b/hw/timer/xlnx-zynqmp-rtc.c
index 707f145027..1d229870c5 100644
--- a/hw/timer/xlnx-zynqmp-rtc.c
+++ b/hw/timer/xlnx-zynqmp-rtc.c
@@ -29,6 +29,10 @@
 #include "hw/register.h"
 #include "qemu/bitops.h"
 #include "qemu/log.h"
+#include "hw/ptimer.h"
+#include "qemu/cutils.h"
+#include "sysemu/sysemu.h"
+#include "trace.h"
 #include "hw/timer/xlnx-zynqmp-rtc.h"
 
 #ifndef XLNX_ZYNQMP_RTC_ERR_DEBUG
@@ -47,6 +51,19 @@ static void addr_error_int_update_irq(XlnxZynqMPRTC *s)
     qemu_set_irq(s->irq_addr_error_int, pending);
 }
 
+static uint32_t rtc_get_count(XlnxZynqMPRTC *s)
+{
+    int64_t now = qemu_clock_get_ns(rtc_clock);
+    return s->tick_offset + now / NANOSECONDS_PER_SECOND;
+}
+
+static uint64_t current_time_postr(RegisterInfo *reg, uint64_t val64)
+{
+    XlnxZynqMPRTC *s = XLNX_ZYNQMP_RTC(reg->opaque);
+
+    return rtc_get_count(s);
+}
+
 static void rtc_int_status_postw(RegisterInfo *reg, uint64_t val64)
 {
     XlnxZynqMPRTC *s = XLNX_ZYNQMP_RTC(reg->opaque);
@@ -97,13 +114,17 @@ static uint64_t addr_error_int_dis_prew(RegisterInfo *reg, uint64_t val64)
 
 static const RegisterAccessInfo rtc_regs_info[] = {
     {   .name = "SET_TIME_WRITE",  .addr = A_SET_TIME_WRITE,
+        .unimp = MAKE_64BIT_MASK(0, 32),
     },{ .name = "SET_TIME_READ",  .addr = A_SET_TIME_READ,
         .ro = 0xffffffff,
+        .post_read = current_time_postr,
     },{ .name = "CALIB_WRITE",  .addr = A_CALIB_WRITE,
+        .unimp = MAKE_64BIT_MASK(0, 32),
     },{ .name = "CALIB_READ",  .addr = A_CALIB_READ,
         .ro = 0x1fffff,
     },{ .name = "CURRENT_TIME",  .addr = A_CURRENT_TIME,
         .ro = 0xffffffff,
+        .post_read = current_time_postr,
     },{ .name = "CURRENT_TICK",  .addr = A_CURRENT_TICK,
         .ro = 0xffff,
     },{ .name = "ALARM",  .addr = A_ALARM,
@@ -143,6 +164,10 @@ static void rtc_reset(DeviceState *dev)
         register_reset(&s->regs_info[i]);
     }
 
+    trace_xlnx_zynqmp_rtc_gettime(s->current_tm.tm_year, s->current_tm.tm_mon,
+                                  s->current_tm.tm_mday, s->current_tm.tm_hour,
+                                  s->current_tm.tm_min, s->current_tm.tm_sec);
+
     rtc_int_update_irq(s);
     addr_error_int_update_irq(s);
 }
@@ -178,14 +203,41 @@ static void rtc_init(Object *obj)
     sysbus_init_mmio(sbd, &s->iomem);
     sysbus_init_irq(sbd, &s->irq_rtc_int);
     sysbus_init_irq(sbd, &s->irq_addr_error_int);
+
+    qemu_get_timedate(&s->current_tm, 0);
+    s->tick_offset = mktimegm(&s->current_tm) -
+        qemu_clock_get_ns(rtc_clock) / NANOSECONDS_PER_SECOND;
+}
+
+static int rtc_post_load(void *opaque, int version_id)
+{
+    XlnxZynqMPRTC *s = opaque;
+
+    /* The tick_offset is added to the current time to determine the guest
+     * time. After migration we don't want to use the original time as that
+     * will indicate to the guest that time has passed, so we need to
+     * recalculate the tick_offset here.
+     */
+    s->tick_offset = mktimegm(&s->current_tm) -
+        qemu_clock_get_ns(rtc_clock) / NANOSECONDS_PER_SECOND;
+
+    return 0;
 }
 
 static const VMStateDescription vmstate_rtc = {
     .name = TYPE_XLNX_ZYNQMP_RTC,
     .version_id = 1,
     .minimum_version_id = 1,
+    .post_load = rtc_post_load,
     .fields = (VMStateField[]) {
         VMSTATE_UINT32_ARRAY(regs, XlnxZynqMPRTC, XLNX_ZYNQMP_RTC_R_MAX),
+        VMSTATE_INT32(current_tm.tm_sec, XlnxZynqMPRTC),
+        VMSTATE_INT32(current_tm.tm_min, XlnxZynqMPRTC),
+        VMSTATE_INT32(current_tm.tm_hour, XlnxZynqMPRTC),
+        VMSTATE_INT32(current_tm.tm_wday, XlnxZynqMPRTC),
+        VMSTATE_INT32(current_tm.tm_mday, XlnxZynqMPRTC),
+        VMSTATE_INT32(current_tm.tm_mon, XlnxZynqMPRTC),
+        VMSTATE_INT32(current_tm.tm_year, XlnxZynqMPRTC),
         VMSTATE_END_OF_LIST(),
     }
 };
diff --git a/hw/timer/trace-events b/hw/timer/trace-events
index 640722b5d1..e6e042fddb 100644
--- a/hw/timer/trace-events
+++ b/hw/timer/trace-events
@@ -60,3 +60,6 @@ systick_write(uint64_t addr, uint32_t value, unsigned size) "systick write addr
 cmsdk_apb_timer_read(uint64_t offset, uint64_t data, unsigned size) "CMSDK APB timer read: offset 0x%" PRIx64 " data 0x%" PRIx64 " size %u"
 cmsdk_apb_timer_write(uint64_t offset, uint64_t data, unsigned size) "CMSDK APB timer write: offset 0x%" PRIx64 " data 0x%" PRIx64 " size %u"
 cmsdk_apb_timer_reset(void) "CMSDK APB timer: reset"
+
+# hw/timer/xlnx-zynqmp-rtc.c
+xlnx_zynqmp_rtc_gettime(int year, int month, int day, int hour, int min, int sec) "Get time from host: %d-%d-%d %2d:%02d:%02d"
-- 
2.14.1

[Qemu-devel] [PATCH v5 3/3] xlnx-zynqmp: Connect the RTC device

[Alistair Francis]

Signed-off-by: Alistair Francis <alistair.francis@xilinx.com>
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---

 include/hw/arm/xlnx-zynqmp.h |  2 ++
 hw/arm/xlnx-zynqmp.c         | 14 ++++++++++++++
 2 files changed, 16 insertions(+)

diff --git a/include/hw/arm/xlnx-zynqmp.h b/include/hw/arm/xlnx-zynqmp.h
index 3e6fb9b7bd..9e8c9e18dd 100644
--- a/include/hw/arm/xlnx-zynqmp.h
+++ b/include/hw/arm/xlnx-zynqmp.h
@@ -28,6 +28,7 @@
 #include "hw/ssi/xilinx_spips.h"
 #include "hw/dma/xlnx_dpdma.h"
 #include "hw/display/xlnx_dp.h"
+#include "hw/timer/xlnx-zynqmp-rtc.h"
 
 #define TYPE_XLNX_ZYNQMP "xlnx,zynqmp"
 #define XLNX_ZYNQMP(obj) OBJECT_CHECK(XlnxZynqMPState, (obj), \
@@ -90,6 +91,7 @@ typedef struct XlnxZynqMPState {
     XlnxZynqMPQSPIPS qspi;
     XlnxDPState dp;
     XlnxDPDMAState dpdma;
+    XlnxZynqMPRTC rtc;
 
     char *boot_cpu;
     ARMCPU *boot_cpu_ptr;
diff --git a/hw/arm/xlnx-zynqmp.c b/hw/arm/xlnx-zynqmp.c
index 325642058b..deef583c2a 100644
--- a/hw/arm/xlnx-zynqmp.c
+++ b/hw/arm/xlnx-zynqmp.c
@@ -50,6 +50,9 @@
 #define DPDMA_ADDR          0xfd4c0000
 #define DPDMA_IRQ           116
 
+#define RTC_ADDR            0xffa60000
+#define RTC_IRQ             26
+
 static const uint64_t gem_addr[XLNX_ZYNQMP_NUM_GEMS] = {
     0xFF0B0000, 0xFF0C0000, 0xFF0D0000, 0xFF0E0000,
 };
@@ -183,6 +186,9 @@ static void xlnx_zynqmp_init(Object *obj)
 
     object_initialize(&s->dpdma, sizeof(s->dpdma), TYPE_XLNX_DPDMA);
     qdev_set_parent_bus(DEVICE(&s->dpdma), sysbus_get_default());
+
+    object_initialize(&s->rtc, sizeof(s->rtc), TYPE_XLNX_ZYNQMP_RTC);
+    qdev_set_parent_bus(DEVICE(&s->rtc), sysbus_get_default());
 }
 
 static void xlnx_zynqmp_realize(DeviceState *dev, Error **errp)
@@ -454,6 +460,14 @@ static void xlnx_zynqmp_realize(DeviceState *dev, Error **errp)
                              &error_abort);
     sysbus_mmio_map(SYS_BUS_DEVICE(&s->dpdma), 0, DPDMA_ADDR);
     sysbus_connect_irq(SYS_BUS_DEVICE(&s->dpdma), 0, gic_spi[DPDMA_IRQ]);
+
+    object_property_set_bool(OBJECT(&s->rtc), true, "realized", &err);
+    if (err) {
+        error_propagate(errp, err);
+        return;
+    }
+    sysbus_mmio_map(SYS_BUS_DEVICE(&s->rtc), 0, RTC_ADDR);
+    sysbus_connect_irq(SYS_BUS_DEVICE(&s->rtc), 0, gic_spi[RTC_IRQ]);
 }
 
 static Property xlnx_zynqmp_props[] = {
-- 
2.14.1

Re: [Qemu-devel] [PATCH v5 2/3] xlnx-zynqmp-rtc: Add basic time support

[Peter Maydell]

On 23 January 2018 at 22:24, Alistair Francis
<alistair.francis@xilinx.com> wrote:
> Allow the guest to determine the time set from the QEMU command line.
>
> This includes adding a trace event to debug the new time.
>
> Signed-off-by: Alistair Francis <alistair.francis@xilinx.com>
> ---
>  static const VMStateDescription vmstate_rtc = {
>      .name = TYPE_XLNX_ZYNQMP_RTC,
>      .version_id = 1,
>      .minimum_version_id = 1,
> +    .post_load = rtc_post_load,
>      .fields = (VMStateField[]) {
>          VMSTATE_UINT32_ARRAY(regs, XlnxZynqMPRTC, XLNX_ZYNQMP_RTC_R_MAX),
> +        VMSTATE_INT32(current_tm.tm_sec, XlnxZynqMPRTC),
> +        VMSTATE_INT32(current_tm.tm_min, XlnxZynqMPRTC),
> +        VMSTATE_INT32(current_tm.tm_hour, XlnxZynqMPRTC),
> +        VMSTATE_INT32(current_tm.tm_wday, XlnxZynqMPRTC),
> +        VMSTATE_INT32(current_tm.tm_mday, XlnxZynqMPRTC),
> +        VMSTATE_INT32(current_tm.tm_mon, XlnxZynqMPRTC),
> +        VMSTATE_INT32(current_tm.tm_year, XlnxZynqMPRTC),

I'm still not sure about having the current_tm struct fields
here rather than whatever the hardware's natural representation
of the current time is. Can you explain why you think this
is the best approach?

thanks
-- PMM

Re: [Qemu-devel] [PATCH v5 2/3] xlnx-zynqmp-rtc: Add basic time support

[Alistair Francis]

On Thu, Jan 25, 2018 at 3:36 AM, Peter Maydell <peter.maydell@linaro.org> wrote:
> On 23 January 2018 at 22:24, Alistair Francis
> <alistair.francis@xilinx.com> wrote:
>> Allow the guest to determine the time set from the QEMU command line.
>>
>> This includes adding a trace event to debug the new time.
>>
>> Signed-off-by: Alistair Francis <alistair.francis@xilinx.com>
>> ---
>>  static const VMStateDescription vmstate_rtc = {
>>      .name = TYPE_XLNX_ZYNQMP_RTC,
>>      .version_id = 1,
>>      .minimum_version_id = 1,
>> +    .post_load = rtc_post_load,
>>      .fields = (VMStateField[]) {
>>          VMSTATE_UINT32_ARRAY(regs, XlnxZynqMPRTC, XLNX_ZYNQMP_RTC_R_MAX),
>> +        VMSTATE_INT32(current_tm.tm_sec, XlnxZynqMPRTC),
>> +        VMSTATE_INT32(current_tm.tm_min, XlnxZynqMPRTC),
>> +        VMSTATE_INT32(current_tm.tm_hour, XlnxZynqMPRTC),
>> +        VMSTATE_INT32(current_tm.tm_wday, XlnxZynqMPRTC),
>> +        VMSTATE_INT32(current_tm.tm_mday, XlnxZynqMPRTC),
>> +        VMSTATE_INT32(current_tm.tm_mon, XlnxZynqMPRTC),
>> +        VMSTATE_INT32(current_tm.tm_year, XlnxZynqMPRTC),
>
> I'm still not sure about having the current_tm struct fields
> here rather than whatever the hardware's natural representation
> of the current time is. Can you explain why you think this
> is the best approach?

This means that the only thing we have to track is the time that the
user set, either through the QEMU command line arguments as an
override or the original starting time. Then from there we can always
calculate the correct offset based on our current time.

I don't see a nicer way, otherwise we need to recalculate the natural
representation before and after migration using something more
complex. This way it's always the same logic, we just run it at init
and after migration.

Alistair

>
> thanks
> -- PMM

Re: [Qemu-devel] [PATCH v5 2/3] xlnx-zynqmp-rtc: Add basic time support

[Peter Maydell]

On 26 January 2018 at 01:09, Alistair Francis
<alistair.francis@xilinx.com> wrote:
> On Thu, Jan 25, 2018 at 3:36 AM, Peter Maydell <peter.maydell@linaro.org> wrote:
>> I'm still not sure about having the current_tm struct fields
>> here rather than whatever the hardware's natural representation
>> of the current time is. Can you explain why you think this
>> is the best approach?
>
> This means that the only thing we have to track is the time that the
> user set, either through the QEMU command line arguments as an
> override or the original starting time. Then from there we can always
> calculate the correct offset based on our current time.
>
> I don't see a nicer way, otherwise we need to recalculate the natural
> representation before and after migration using something more
> complex. This way it's always the same logic, we just run it at init
> and after migration.

Doing it this way round means that it gets complicated when the
guest writes to the RTC, though. At the moment I can't see anywhere
that sets the current_tm fields except for reset and post-migration.
Isn't something missing here? I'm wondering if this choice of
migration state is going to paint us into a corner for handling
guest-writes-to-rtc.

thanks
-- PMM

Re: [Qemu-devel] [PATCH v5 2/3] xlnx-zynqmp-rtc: Add basic time support

[Alistair Francis]

On Thu, Feb 8, 2018 at 7:42 AM, Peter Maydell <peter.maydell@linaro.org> wrote:
> On 26 January 2018 at 01:09, Alistair Francis
> <alistair.francis@xilinx.com> wrote:
>> On Thu, Jan 25, 2018 at 3:36 AM, Peter Maydell <peter.maydell@linaro.org> wrote:
>>> I'm still not sure about having the current_tm struct fields
>>> here rather than whatever the hardware's natural representation
>>> of the current time is. Can you explain why you think this
>>> is the best approach?
>>
>> This means that the only thing we have to track is the time that the
>> user set, either through the QEMU command line arguments as an
>> override or the original starting time. Then from there we can always
>> calculate the correct offset based on our current time.
>>
>> I don't see a nicer way, otherwise we need to recalculate the natural
>> representation before and after migration using something more
>> complex. This way it's always the same logic, we just run it at init
>> and after migration.
>
> Doing it this way round means that it gets complicated when the
> guest writes to the RTC, though. At the moment I can't see anywhere
> that sets the current_tm fields except for reset and post-migration.
> Isn't something missing here? I'm wondering if this choice of
> migration state is going to paint us into a corner for handling
> guest-writes-to-rtc.

I don't have guest write support. The main focus was just allowing
users to set the time when they boot QEMU. We haven't seen any request
to allow guests to set the time, so I didn't bother.

Wouldn't adding guest write support just be as simple as converting
the value written to a current_tm field?

Alistair

>
> thanks
> -- PMM
>

Re: [Qemu-devel] [PATCH v5 2/3] xlnx-zynqmp-rtc: Add basic time support

[Peter Maydell]

On 8 February 2018 at 17:21, Alistair Francis
<alistair.francis@xilinx.com> wrote:
> On Thu, Feb 8, 2018 at 7:42 AM, Peter Maydell <peter.maydell@linaro.org> wrote:
>> Doing it this way round means that it gets complicated when the
>> guest writes to the RTC, though. At the moment I can't see anywhere
>> that sets the current_tm fields except for reset and post-migration.
>> Isn't something missing here? I'm wondering if this choice of
>> migration state is going to paint us into a corner for handling
>> guest-writes-to-rtc.
>
> I don't have guest write support. The main focus was just allowing
> users to set the time when they boot QEMU. We haven't seen any request
> to allow guests to set the time, so I didn't bother.
>
> Wouldn't adding guest write support just be as simple as converting
> the value written to a current_tm field?

Yes, but then why are we using the current_tm format at all?
It's not what the guest wants to see for reads, it's not what writes
would naturally be, and it's not what you're using at runtime (you
convert it immediately to a tick_offset and ignore the current_tm
stuff). I guess it can be made to work, it just seems like a weird
format to use for migration.

I've also just reread the patch and am a bit confused -- don't
you need some code that writes the correct value for the current
guest time to the current_tm fields before migration ?

thanks
-- PMM