[PATCH 0/2] RTC support for QEMU RISC-V virt machine

[PATCH 0/2] RTC support for QEMU RISC-V virt machine

[Anup Patel]

This series adds RTC device to QEMU RISC-V virt machine. We have
selected Goldfish RTC device model for this. It's a pretty simple
synthetic device with few MMIO registers and no dependency external
clock. The driver for Goldfish RTC is already available in Linux so
we just need to enable it in Kconfig for RISCV and also update Linux
defconfigs.

We have tested this series with Linux-5.3 plus defconfig changes
available in 'goldfish_rtc_v1' branch of:
https://github.com/avpatel/linux.git

Anup Patel (2):
  hw: timer: Add Goldfish RTC device
  riscv: virt: Use Goldfish RTC device

 hw/riscv/Kconfig                |   1 +
 hw/riscv/virt.c                 |  15 +++
 hw/timer/Kconfig                |   3 +
 hw/timer/Makefile.objs          |   1 +
 hw/timer/goldfish_rtc.c         | 221 ++++++++++++++++++++++++++++++++
 include/hw/riscv/virt.h         |   2 +
 include/hw/timer/goldfish_rtc.h |  45 +++++++
 7 files changed, 288 insertions(+)
 create mode 100644 hw/timer/goldfish_rtc.c
 create mode 100644 include/hw/timer/goldfish_rtc.h

--
2.17.1

[PATCH 0/2] RTC support for QEMU RISC-V virt machine

[Anup Patel]

This series adds RTC device to QEMU RISC-V virt machine. We have
selected Goldfish RTC device model for this. It's a pretty simple
synthetic device with few MMIO registers and no dependency external
clock. The driver for Goldfish RTC is already available in Linux so
we just need to enable it in Kconfig for RISCV and also update Linux
defconfigs.

We have tested this series with Linux-5.3 plus defconfig changes
available in 'goldfish_rtc_v1' branch of:
https://github.com/avpatel/linux.git

Anup Patel (2):
  hw: timer: Add Goldfish RTC device
  riscv: virt: Use Goldfish RTC device

 hw/riscv/Kconfig                |   1 +
 hw/riscv/virt.c                 |  15 +++
 hw/timer/Kconfig                |   3 +
 hw/timer/Makefile.objs          |   1 +
 hw/timer/goldfish_rtc.c         | 221 ++++++++++++++++++++++++++++++++
 include/hw/riscv/virt.h         |   2 +
 include/hw/timer/goldfish_rtc.h |  45 +++++++
 7 files changed, 288 insertions(+)
 create mode 100644 hw/timer/goldfish_rtc.c
 create mode 100644 include/hw/timer/goldfish_rtc.h

--
2.17.1

[PATCH 1/2] hw: timer: Add Goldfish RTC device

[Anup Patel]

This patch adds model for Google Goldfish virtual platform RTC device.

We will be adding Goldfish RTC device to the QEMU RISC-V virt machine
for providing real date-time to Guest Linux. The corresponding Linux
driver for Goldfish RTC device is already available in upstream Linux.

For now, VM migration support is not available for Goldfish RTC device
but it will be added later when we implement VM migration for KVM RISC-V.

Signed-off-by: Anup Patel <anup.patel@wdc.com>
---
 hw/timer/Kconfig                |   3 +
 hw/timer/Makefile.objs          |   1 +
 hw/timer/goldfish_rtc.c         | 221 ++++++++++++++++++++++++++++++++
 include/hw/timer/goldfish_rtc.h |  45 +++++++
 4 files changed, 270 insertions(+)
 create mode 100644 hw/timer/goldfish_rtc.c
 create mode 100644 include/hw/timer/goldfish_rtc.h

diff --git a/hw/timer/Kconfig b/hw/timer/Kconfig
index eefc95f35e..bd1831fbe8 100644
--- a/hw/timer/Kconfig
+++ b/hw/timer/Kconfig
@@ -58,3 +58,6 @@ config CMSDK_APB_TIMER
 config CMSDK_APB_DUALTIMER
     bool
     select PTIMER
+
+config GOLDFISH_RTC
+    bool
diff --git a/hw/timer/Makefile.objs b/hw/timer/Makefile.objs
index 123d92c969..5dc6f880af 100644
--- a/hw/timer/Makefile.objs
+++ b/hw/timer/Makefile.objs
@@ -47,3 +47,4 @@ common-obj-$(CONFIG_SUN4V_RTC) += sun4v-rtc.o
 common-obj-$(CONFIG_CMSDK_APB_TIMER) += cmsdk-apb-timer.o
 common-obj-$(CONFIG_CMSDK_APB_DUALTIMER) += cmsdk-apb-dualtimer.o
 common-obj-$(CONFIG_MSF2) += mss-timer.o
+common-obj-$(CONFIG_GOLDFISH_RTC) += goldfish_rtc.o
diff --git a/hw/timer/goldfish_rtc.c b/hw/timer/goldfish_rtc.c
new file mode 100644
index 0000000000..a3349d7cb4
--- /dev/null
+++ b/hw/timer/goldfish_rtc.c
@@ -0,0 +1,221 @@
+/*
+ * Goldfish virtual platform RTC
+ *
+ * Copyright (C) 2019 Western Digital Corporation or its affiliates.
+ *
+ * For more details on Google Goldfish virtual platform refer:
+ * https://android.googlesource.com/platform/external/qemu/+/master/docs/GOLDFISH-VIRTUAL-HARDWARE.TXT
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms and conditions of the GNU General Public License,
+ * version 2 or later, as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
+ * more details.
+ *
+ * You should have received a copy of the GNU General Public License along with
+ * this program.  If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#include "qemu/osdep.h"
+#include "qemu-common.h"
+#include "hw/timer/goldfish_rtc.h"
+#include "hw/irq.h"
+#include "hw/qdev-properties.h"
+#include "hw/sysbus.h"
+#include "qemu/timer.h"
+#include "sysemu/sysemu.h"
+#include "qemu/cutils.h"
+#include "qemu/log.h"
+
+#define RTC_TIME_LOW            0x00
+#define RTC_TIME_HIGH           0x04
+#define RTC_ALARM_LOW           0x08
+#define RTC_ALARM_HIGH          0x0c
+#define RTC_IRQ_ENABLED         0x10
+#define RTC_CLEAR_ALARM         0x14
+#define RTC_ALARM_STATUS        0x18
+#define RTC_CLEAR_INTERRUPT     0x1c
+
+static void goldfish_rtc_update(GoldfishRTCState *s)
+{
+    qemu_set_irq(s->irq, (s->irq_pending & s->irq_enabled) ? 1 : 0);
+}
+
+static void goldfish_rtc_interrupt(void *opaque)
+{
+    GoldfishRTCState *s = (GoldfishRTCState *)opaque;
+
+    s->alarm_running = 0;
+    s->irq_pending = 1;
+    goldfish_rtc_update(s);
+}
+
+static uint64_t goldfish_rtc_get_count(GoldfishRTCState *s)
+{
+    return s->tick_offset + (uint64_t)qemu_clock_get_ns(rtc_clock);
+}
+
+static void goldfish_rtc_clear_alarm(GoldfishRTCState *s)
+{
+    timer_del(s->timer);
+    s->alarm_running = 0;
+}
+
+static void goldfish_rtc_set_alarm(GoldfishRTCState *s)
+{
+    uint64_t ticks = goldfish_rtc_get_count(s);
+    uint64_t event = s->alarm_next;
+
+    if (event <= ticks) {
+        timer_del(s->timer);
+        goldfish_rtc_interrupt(s);
+    } else {
+        int64_t now = qemu_clock_get_ns(rtc_clock);
+        timer_mod(s->timer, now + (event - ticks));
+        s->alarm_running = 1;
+    }
+}
+
+static uint64_t goldfish_rtc_read(void *opaque, hwaddr offset,
+                                  unsigned size)
+{
+    GoldfishRTCState *s = (GoldfishRTCState *)opaque;
+    uint64_t r;
+
+    switch (offset) {
+    case RTC_TIME_LOW:
+        r = goldfish_rtc_get_count(s) & 0xffffffff;
+        break;
+    case RTC_TIME_HIGH:
+        r = goldfish_rtc_get_count(s) >> 32;
+        break;
+    case RTC_ALARM_LOW:
+        r = s->alarm_next & 0xffffffff;
+        break;
+    case RTC_ALARM_HIGH:
+        r = s->alarm_next >> 32;
+        break;
+    case RTC_IRQ_ENABLED:
+        r = s->irq_enabled;
+        break;
+    case RTC_ALARM_STATUS:
+        r = s->alarm_running;
+        break;
+    default:
+        qemu_log_mask(LOG_GUEST_ERROR,
+                      "goldfish_rtc_read: Bad offset 0x%x\n", (int)offset);
+        r = 0;
+        break;
+    }
+
+    return r;
+}
+
+static void goldfish_rtc_write(void *opaque, hwaddr offset,
+                               uint64_t value, unsigned size)
+{
+    uint64_t current_tick, new_tick;
+    GoldfishRTCState *s = (GoldfishRTCState *)opaque;
+
+    switch (offset) {
+    case RTC_TIME_LOW:
+        current_tick = goldfish_rtc_get_count(s);
+        new_tick = current_tick & (0xffffffffULL << 32);
+        new_tick |= value;
+        s->tick_offset += new_tick - current_tick;
+        break;
+    case RTC_TIME_HIGH:
+        current_tick = goldfish_rtc_get_count(s);
+        new_tick = current_tick & 0xffffffffULL;
+        new_tick |= (value << 32);
+        s->tick_offset += new_tick - current_tick;
+        break;
+    case RTC_ALARM_LOW:
+        s->alarm_next &= (0xffffffffULL << 32);
+        s->alarm_next |= value;
+        goldfish_rtc_set_alarm(s);
+        break;
+    case RTC_ALARM_HIGH:
+        s->alarm_next &= 0xffffffffULL;
+        s->alarm_next |= (value << 32);
+        break;
+    case RTC_IRQ_ENABLED:
+        s->irq_enabled = (uint32_t)(value & 0x1);
+        goldfish_rtc_update(s);
+        break;
+    case RTC_CLEAR_ALARM:
+        goldfish_rtc_clear_alarm(s);
+        break;
+    case RTC_CLEAR_INTERRUPT:
+        s->irq_pending = 0;
+        goldfish_rtc_update(s);
+        break;
+    default:
+        qemu_log_mask(LOG_GUEST_ERROR,
+                      "goldfish_rtc_write: Bad offset 0x%x\n", (int)offset);
+        break;
+    }
+}
+
+static const MemoryRegionOps goldfish_rtc_ops = {
+    .read = goldfish_rtc_read,
+    .write = goldfish_rtc_write,
+    .endianness = DEVICE_LITTLE_ENDIAN,
+    .valid = {
+        .min_access_size = 4,
+        .max_access_size = 4
+    }
+};
+
+static void goldfish_rtc_init(Object *obj)
+{
+    GoldfishRTCState *s = GOLDFISH_RTC(obj);
+    SysBusDevice *dev = SYS_BUS_DEVICE(obj);
+    struct tm tm;
+
+    memory_region_init_io(&s->iomem, obj, &goldfish_rtc_ops, s,
+                          "goldfish_rtc", 0x1000);
+    sysbus_init_mmio(dev, &s->iomem);
+
+    sysbus_init_irq(dev, &s->irq);
+
+    qemu_get_timedate(&tm, 0);
+    s->tick_offset = mktimegm(&tm);
+    s->tick_offset *= NANOSECONDS_PER_SECOND;
+    s->tick_offset -= qemu_clock_get_ns(rtc_clock);
+
+    s->timer = timer_new_ns(rtc_clock, goldfish_rtc_interrupt, s);
+}
+
+static Property goldfish_rtc_properties[] = {
+    DEFINE_PROP_UINT64("tick-offset", GoldfishRTCState, tick_offset, 0),
+    DEFINE_PROP_UINT64("alarm-next", GoldfishRTCState, alarm_next, 0),
+    DEFINE_PROP_UINT32("alarm-running", GoldfishRTCState, alarm_running, 0),
+    DEFINE_PROP_UINT32("irq-pending", GoldfishRTCState, irq_pending, 0),
+    DEFINE_PROP_UINT32("irq-enabled", GoldfishRTCState, irq_enabled, 0),
+    DEFINE_PROP_END_OF_LIST(),
+};
+
+static void goldfish_rtc_class_init(ObjectClass *klass, void *data)
+{
+    DeviceClass *dc = DEVICE_CLASS(klass);
+    dc->props = goldfish_rtc_properties;
+}
+
+static const TypeInfo goldfish_rtc_info = {
+    .name          = TYPE_GOLDFISH_RTC,
+    .parent        = TYPE_SYS_BUS_DEVICE,
+    .instance_size = sizeof(GoldfishRTCState),
+    .instance_init = goldfish_rtc_init,
+    .class_init    = goldfish_rtc_class_init,
+};
+
+static void goldfish_rtc_register_types(void)
+{
+    type_register_static(&goldfish_rtc_info);
+}
+
+type_init(goldfish_rtc_register_types)
diff --git a/include/hw/timer/goldfish_rtc.h b/include/hw/timer/goldfish_rtc.h
new file mode 100644
index 0000000000..f96a5f5158
--- /dev/null
+++ b/include/hw/timer/goldfish_rtc.h
@@ -0,0 +1,45 @@
+/*
+ * Goldfish virtual platform RTC
+ *
+ * Copyright (C) 2019 Western Digital Corporation or its affiliates.
+ *
+ * For more details on Google Goldfish virtual platform refer:
+ * https://android.googlesource.com/platform/external/qemu/+/master/docs/GOLDFISH-VIRTUAL-HARDWARE.TXT
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms and conditions of the GNU General Public License,
+ * version 2 or later, as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
+ * more details.
+ *
+ * You should have received a copy of the GNU General Public License along with
+ * this program.  If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#ifndef HW_TIMER_GOLDFISH_RTC_H
+#define HW_TIMER_GOLDFISH_RTC_H
+
+#include "hw/sysbus.h"
+
+#define TYPE_GOLDFISH_RTC "goldfish_rtc"
+#define GOLDFISH_RTC(obj) \
+OBJECT_CHECK(GoldfishRTCState, (obj), TYPE_GOLDFISH_RTC)
+
+typedef struct GoldfishRTCState {
+    SysBusDevice parent_obj;
+
+    MemoryRegion iomem;
+    QEMUTimer *timer;
+    qemu_irq irq;
+
+    uint64_t tick_offset;
+    uint64_t alarm_next;
+    uint32_t alarm_running;
+    uint32_t irq_pending;
+    uint32_t irq_enabled;
+} GoldfishRTCState;
+
+#endif
-- 
2.17.1

[PATCH 1/2] hw: timer: Add Goldfish RTC device

[Anup Patel]

This patch adds model for Google Goldfish virtual platform RTC device.

We will be adding Goldfish RTC device to the QEMU RISC-V virt machine
for providing real date-time to Guest Linux. The corresponding Linux
driver for Goldfish RTC device is already available in upstream Linux.

For now, VM migration support is not available for Goldfish RTC device
but it will be added later when we implement VM migration for KVM RISC-V.

Signed-off-by: Anup Patel <anup.patel@wdc.com>
---
 hw/timer/Kconfig                |   3 +
 hw/timer/Makefile.objs          |   1 +
 hw/timer/goldfish_rtc.c         | 221 ++++++++++++++++++++++++++++++++
 include/hw/timer/goldfish_rtc.h |  45 +++++++
 4 files changed, 270 insertions(+)
 create mode 100644 hw/timer/goldfish_rtc.c
 create mode 100644 include/hw/timer/goldfish_rtc.h

diff --git a/hw/timer/Kconfig b/hw/timer/Kconfig
index eefc95f35e..bd1831fbe8 100644
--- a/hw/timer/Kconfig
+++ b/hw/timer/Kconfig
@@ -58,3 +58,6 @@ config CMSDK_APB_TIMER
 config CMSDK_APB_DUALTIMER
     bool
     select PTIMER
+
+config GOLDFISH_RTC
+    bool
diff --git a/hw/timer/Makefile.objs b/hw/timer/Makefile.objs
index 123d92c969..5dc6f880af 100644
--- a/hw/timer/Makefile.objs
+++ b/hw/timer/Makefile.objs
@@ -47,3 +47,4 @@ common-obj-$(CONFIG_SUN4V_RTC) += sun4v-rtc.o
 common-obj-$(CONFIG_CMSDK_APB_TIMER) += cmsdk-apb-timer.o
 common-obj-$(CONFIG_CMSDK_APB_DUALTIMER) += cmsdk-apb-dualtimer.o
 common-obj-$(CONFIG_MSF2) += mss-timer.o
+common-obj-$(CONFIG_GOLDFISH_RTC) += goldfish_rtc.o
diff --git a/hw/timer/goldfish_rtc.c b/hw/timer/goldfish_rtc.c
new file mode 100644
index 0000000000..a3349d7cb4
--- /dev/null
+++ b/hw/timer/goldfish_rtc.c
@@ -0,0 +1,221 @@
+/*
+ * Goldfish virtual platform RTC
+ *
+ * Copyright (C) 2019 Western Digital Corporation or its affiliates.
+ *
+ * For more details on Google Goldfish virtual platform refer:
+ * https://android.googlesource.com/platform/external/qemu/+/master/docs/GOLDFISH-VIRTUAL-HARDWARE.TXT
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms and conditions of the GNU General Public License,
+ * version 2 or later, as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
+ * more details.
+ *
+ * You should have received a copy of the GNU General Public License along with
+ * this program.  If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#include "qemu/osdep.h"
+#include "qemu-common.h"
+#include "hw/timer/goldfish_rtc.h"
+#include "hw/irq.h"
+#include "hw/qdev-properties.h"
+#include "hw/sysbus.h"
+#include "qemu/timer.h"
+#include "sysemu/sysemu.h"
+#include "qemu/cutils.h"
+#include "qemu/log.h"
+
+#define RTC_TIME_LOW            0x00
+#define RTC_TIME_HIGH           0x04
+#define RTC_ALARM_LOW           0x08
+#define RTC_ALARM_HIGH          0x0c
+#define RTC_IRQ_ENABLED         0x10
+#define RTC_CLEAR_ALARM         0x14
+#define RTC_ALARM_STATUS        0x18
+#define RTC_CLEAR_INTERRUPT     0x1c
+
+static void goldfish_rtc_update(GoldfishRTCState *s)
+{
+    qemu_set_irq(s->irq, (s->irq_pending & s->irq_enabled) ? 1 : 0);
+}
+
+static void goldfish_rtc_interrupt(void *opaque)
+{
+    GoldfishRTCState *s = (GoldfishRTCState *)opaque;
+
+    s->alarm_running = 0;
+    s->irq_pending = 1;
+    goldfish_rtc_update(s);
+}
+
+static uint64_t goldfish_rtc_get_count(GoldfishRTCState *s)
+{
+    return s->tick_offset + (uint64_t)qemu_clock_get_ns(rtc_clock);
+}
+
+static void goldfish_rtc_clear_alarm(GoldfishRTCState *s)
+{
+    timer_del(s->timer);
+    s->alarm_running = 0;
+}
+
+static void goldfish_rtc_set_alarm(GoldfishRTCState *s)
+{
+    uint64_t ticks = goldfish_rtc_get_count(s);
+    uint64_t event = s->alarm_next;
+
+    if (event <= ticks) {
+        timer_del(s->timer);
+        goldfish_rtc_interrupt(s);
+    } else {
+        int64_t now = qemu_clock_get_ns(rtc_clock);
+        timer_mod(s->timer, now + (event - ticks));
+        s->alarm_running = 1;
+    }
+}
+
+static uint64_t goldfish_rtc_read(void *opaque, hwaddr offset,
+                                  unsigned size)
+{
+    GoldfishRTCState *s = (GoldfishRTCState *)opaque;
+    uint64_t r;
+
+    switch (offset) {
+    case RTC_TIME_LOW:
+        r = goldfish_rtc_get_count(s) & 0xffffffff;
+        break;
+    case RTC_TIME_HIGH:
+        r = goldfish_rtc_get_count(s) >> 32;
+        break;
+    case RTC_ALARM_LOW:
+        r = s->alarm_next & 0xffffffff;
+        break;
+    case RTC_ALARM_HIGH:
+        r = s->alarm_next >> 32;
+        break;
+    case RTC_IRQ_ENABLED:
+        r = s->irq_enabled;
+        break;
+    case RTC_ALARM_STATUS:
+        r = s->alarm_running;
+        break;
+    default:
+        qemu_log_mask(LOG_GUEST_ERROR,
+                      "goldfish_rtc_read: Bad offset 0x%x\n", (int)offset);
+        r = 0;
+        break;
+    }
+
+    return r;
+}
+
+static void goldfish_rtc_write(void *opaque, hwaddr offset,
+                               uint64_t value, unsigned size)
+{
+    uint64_t current_tick, new_tick;
+    GoldfishRTCState *s = (GoldfishRTCState *)opaque;
+
+    switch (offset) {
+    case RTC_TIME_LOW:
+        current_tick = goldfish_rtc_get_count(s);
+        new_tick = current_tick & (0xffffffffULL << 32);
+        new_tick |= value;
+        s->tick_offset += new_tick - current_tick;
+        break;
+    case RTC_TIME_HIGH:
+        current_tick = goldfish_rtc_get_count(s);
+        new_tick = current_tick & 0xffffffffULL;
+        new_tick |= (value << 32);
+        s->tick_offset += new_tick - current_tick;
+        break;
+    case RTC_ALARM_LOW:
+        s->alarm_next &= (0xffffffffULL << 32);
+        s->alarm_next |= value;
+        goldfish_rtc_set_alarm(s);
+        break;
+    case RTC_ALARM_HIGH:
+        s->alarm_next &= 0xffffffffULL;
+        s->alarm_next |= (value << 32);
+        break;
+    case RTC_IRQ_ENABLED:
+        s->irq_enabled = (uint32_t)(value & 0x1);
+        goldfish_rtc_update(s);
+        break;
+    case RTC_CLEAR_ALARM:
+        goldfish_rtc_clear_alarm(s);
+        break;
+    case RTC_CLEAR_INTERRUPT:
+        s->irq_pending = 0;
+        goldfish_rtc_update(s);
+        break;
+    default:
+        qemu_log_mask(LOG_GUEST_ERROR,
+                      "goldfish_rtc_write: Bad offset 0x%x\n", (int)offset);
+        break;
+    }
+}
+
+static const MemoryRegionOps goldfish_rtc_ops = {
+    .read = goldfish_rtc_read,
+    .write = goldfish_rtc_write,
+    .endianness = DEVICE_LITTLE_ENDIAN,
+    .valid = {
+        .min_access_size = 4,
+        .max_access_size = 4
+    }
+};
+
+static void goldfish_rtc_init(Object *obj)
+{
+    GoldfishRTCState *s = GOLDFISH_RTC(obj);
+    SysBusDevice *dev = SYS_BUS_DEVICE(obj);
+    struct tm tm;
+
+    memory_region_init_io(&s->iomem, obj, &goldfish_rtc_ops, s,
+                          "goldfish_rtc", 0x1000);
+    sysbus_init_mmio(dev, &s->iomem);
+
+    sysbus_init_irq(dev, &s->irq);
+
+    qemu_get_timedate(&tm, 0);
+    s->tick_offset = mktimegm(&tm);
+    s->tick_offset *= NANOSECONDS_PER_SECOND;
+    s->tick_offset -= qemu_clock_get_ns(rtc_clock);
+
+    s->timer = timer_new_ns(rtc_clock, goldfish_rtc_interrupt, s);
+}
+
+static Property goldfish_rtc_properties[] = {
+    DEFINE_PROP_UINT64("tick-offset", GoldfishRTCState, tick_offset, 0),
+    DEFINE_PROP_UINT64("alarm-next", GoldfishRTCState, alarm_next, 0),
+    DEFINE_PROP_UINT32("alarm-running", GoldfishRTCState, alarm_running, 0),
+    DEFINE_PROP_UINT32("irq-pending", GoldfishRTCState, irq_pending, 0),
+    DEFINE_PROP_UINT32("irq-enabled", GoldfishRTCState, irq_enabled, 0),
+    DEFINE_PROP_END_OF_LIST(),
+};
+
+static void goldfish_rtc_class_init(ObjectClass *klass, void *data)
+{
+    DeviceClass *dc = DEVICE_CLASS(klass);
+    dc->props = goldfish_rtc_properties;
+}
+
+static const TypeInfo goldfish_rtc_info = {
+    .name          = TYPE_GOLDFISH_RTC,
+    .parent        = TYPE_SYS_BUS_DEVICE,
+    .instance_size = sizeof(GoldfishRTCState),
+    .instance_init = goldfish_rtc_init,
+    .class_init    = goldfish_rtc_class_init,
+};
+
+static void goldfish_rtc_register_types(void)
+{
+    type_register_static(&goldfish_rtc_info);
+}
+
+type_init(goldfish_rtc_register_types)
diff --git a/include/hw/timer/goldfish_rtc.h b/include/hw/timer/goldfish_rtc.h
new file mode 100644
index 0000000000..f96a5f5158
--- /dev/null
+++ b/include/hw/timer/goldfish_rtc.h
@@ -0,0 +1,45 @@
+/*
+ * Goldfish virtual platform RTC
+ *
+ * Copyright (C) 2019 Western Digital Corporation or its affiliates.
+ *
+ * For more details on Google Goldfish virtual platform refer:
+ * https://android.googlesource.com/platform/external/qemu/+/master/docs/GOLDFISH-VIRTUAL-HARDWARE.TXT
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms and conditions of the GNU General Public License,
+ * version 2 or later, as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
+ * more details.
+ *
+ * You should have received a copy of the GNU General Public License along with
+ * this program.  If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#ifndef HW_TIMER_GOLDFISH_RTC_H
+#define HW_TIMER_GOLDFISH_RTC_H
+
+#include "hw/sysbus.h"
+
+#define TYPE_GOLDFISH_RTC "goldfish_rtc"
+#define GOLDFISH_RTC(obj) \
+OBJECT_CHECK(GoldfishRTCState, (obj), TYPE_GOLDFISH_RTC)
+
+typedef struct GoldfishRTCState {
+    SysBusDevice parent_obj;
+
+    MemoryRegion iomem;
+    QEMUTimer *timer;
+    qemu_irq irq;
+
+    uint64_t tick_offset;
+    uint64_t alarm_next;
+    uint32_t alarm_running;
+    uint32_t irq_pending;
+    uint32_t irq_enabled;
+} GoldfishRTCState;
+
+#endif
-- 
2.17.1

[PATCH 2/2] riscv: virt: Use Goldfish RTC device

[Anup Patel]

We extend QEMU RISC-V virt machine by adding Goldfish RTC device
to it. This will allow Guest Linux to sync it's local date/time
with Host date/time via RTC device.

Signed-off-by: Anup Patel <anup.patel@wdc.com>
---
 hw/riscv/Kconfig        |  1 +
 hw/riscv/virt.c         | 15 +++++++++++++++
 include/hw/riscv/virt.h |  2 ++
 3 files changed, 18 insertions(+)

diff --git a/hw/riscv/Kconfig b/hw/riscv/Kconfig
index fb19b2df3a..b33753c780 100644
--- a/hw/riscv/Kconfig
+++ b/hw/riscv/Kconfig
@@ -34,6 +34,7 @@ config RISCV_VIRT
     select PCI
     select HART
     select SERIAL
+    select GOLDFISH_RTC
     select VIRTIO_MMIO
     select PCI_EXPRESS_GENERIC_BRIDGE
     select SIFIVE
diff --git a/hw/riscv/virt.c b/hw/riscv/virt.c
index d36f5625ec..95c42ab993 100644
--- a/hw/riscv/virt.c
+++ b/hw/riscv/virt.c
@@ -57,6 +57,7 @@ static const struct MemmapEntry {
     [VIRT_DEBUG] =       {        0x0,         0x100 },
     [VIRT_MROM] =        {     0x1000,       0x11000 },
     [VIRT_TEST] =        {   0x100000,        0x1000 },
+    [VIRT_RTC] =         {   0x101000,        0x1000 },
     [VIRT_CLINT] =       {  0x2000000,       0x10000 },
     [VIRT_PLIC] =        {  0xc000000,     0x4000000 },
     [VIRT_UART0] =       { 0x10000000,         0x100 },
@@ -310,6 +311,17 @@ static void create_fdt(RISCVVirtState *s, const struct MemmapEntry *memmap,
     qemu_fdt_setprop_cell(fdt, nodename, "interrupt-parent", plic_phandle);
     qemu_fdt_setprop_cell(fdt, nodename, "interrupts", UART0_IRQ);
 
+    nodename = g_strdup_printf("/rtc@%lx",
+        (long)memmap[VIRT_RTC].base);
+    qemu_fdt_add_subnode(fdt, nodename);
+    qemu_fdt_setprop_string(fdt, nodename, "compatible",
+        "google,goldfish-rtc");
+    qemu_fdt_setprop_cells(fdt, nodename, "reg",
+        0x0, memmap[VIRT_RTC].base,
+        0x0, memmap[VIRT_RTC].size);
+    qemu_fdt_setprop_cell(fdt, nodename, "interrupt-parent", plic_phandle);
+    qemu_fdt_setprop_cell(fdt, nodename, "interrupts", RTC_IRQ);
+
     qemu_fdt_add_subnode(fdt, "/chosen");
     qemu_fdt_setprop_string(fdt, "/chosen", "stdout-path", nodename);
     if (cmdline) {
@@ -496,6 +508,9 @@ static void riscv_virt_board_init(MachineState *machine)
         0, qdev_get_gpio_in(DEVICE(s->plic), UART0_IRQ), 399193,
         serial_hd(0), DEVICE_LITTLE_ENDIAN);
 
+    sysbus_create_simple("goldfish_rtc", memmap[VIRT_RTC].base,
+        qdev_get_gpio_in(DEVICE(s->plic), RTC_IRQ));
+
     g_free(plic_hart_config);
 }
 
diff --git a/include/hw/riscv/virt.h b/include/hw/riscv/virt.h
index 6e5fbe5d3b..e6423258d3 100644
--- a/include/hw/riscv/virt.h
+++ b/include/hw/riscv/virt.h
@@ -37,6 +37,7 @@ enum {
     VIRT_DEBUG,
     VIRT_MROM,
     VIRT_TEST,
+    VIRT_RTC,
     VIRT_CLINT,
     VIRT_PLIC,
     VIRT_UART0,
@@ -49,6 +50,7 @@ enum {
 
 enum {
     UART0_IRQ = 10,
+    RTC_IRQ = 11,
     VIRTIO_IRQ = 1, /* 1 to 8 */
     VIRTIO_COUNT = 8,
     PCIE_IRQ = 0x20, /* 32 to 35 */
-- 
2.17.1

[PATCH 2/2] riscv: virt: Use Goldfish RTC device

[Anup Patel]

We extend QEMU RISC-V virt machine by adding Goldfish RTC device
to it. This will allow Guest Linux to sync it's local date/time
with Host date/time via RTC device.

Signed-off-by: Anup Patel <anup.patel@wdc.com>
---
 hw/riscv/Kconfig        |  1 +
 hw/riscv/virt.c         | 15 +++++++++++++++
 include/hw/riscv/virt.h |  2 ++
 3 files changed, 18 insertions(+)

diff --git a/hw/riscv/Kconfig b/hw/riscv/Kconfig
index fb19b2df3a..b33753c780 100644
--- a/hw/riscv/Kconfig
+++ b/hw/riscv/Kconfig
@@ -34,6 +34,7 @@ config RISCV_VIRT
     select PCI
     select HART
     select SERIAL
+    select GOLDFISH_RTC
     select VIRTIO_MMIO
     select PCI_EXPRESS_GENERIC_BRIDGE
     select SIFIVE
diff --git a/hw/riscv/virt.c b/hw/riscv/virt.c
index d36f5625ec..95c42ab993 100644
--- a/hw/riscv/virt.c
+++ b/hw/riscv/virt.c
@@ -57,6 +57,7 @@ static const struct MemmapEntry {
     [VIRT_DEBUG] =       {        0x0,         0x100 },
     [VIRT_MROM] =        {     0x1000,       0x11000 },
     [VIRT_TEST] =        {   0x100000,        0x1000 },
+    [VIRT_RTC] =         {   0x101000,        0x1000 },
     [VIRT_CLINT] =       {  0x2000000,       0x10000 },
     [VIRT_PLIC] =        {  0xc000000,     0x4000000 },
     [VIRT_UART0] =       { 0x10000000,         0x100 },
@@ -310,6 +311,17 @@ static void create_fdt(RISCVVirtState *s, const struct MemmapEntry *memmap,
     qemu_fdt_setprop_cell(fdt, nodename, "interrupt-parent", plic_phandle);
     qemu_fdt_setprop_cell(fdt, nodename, "interrupts", UART0_IRQ);
 
+    nodename = g_strdup_printf("/rtc@%lx",
+        (long)memmap[VIRT_RTC].base);
+    qemu_fdt_add_subnode(fdt, nodename);
+    qemu_fdt_setprop_string(fdt, nodename, "compatible",
+        "google,goldfish-rtc");
+    qemu_fdt_setprop_cells(fdt, nodename, "reg",
+        0x0, memmap[VIRT_RTC].base,
+        0x0, memmap[VIRT_RTC].size);
+    qemu_fdt_setprop_cell(fdt, nodename, "interrupt-parent", plic_phandle);
+    qemu_fdt_setprop_cell(fdt, nodename, "interrupts", RTC_IRQ);
+
     qemu_fdt_add_subnode(fdt, "/chosen");
     qemu_fdt_setprop_string(fdt, "/chosen", "stdout-path", nodename);
     if (cmdline) {
@@ -496,6 +508,9 @@ static void riscv_virt_board_init(MachineState *machine)
         0, qdev_get_gpio_in(DEVICE(s->plic), UART0_IRQ), 399193,
         serial_hd(0), DEVICE_LITTLE_ENDIAN);
 
+    sysbus_create_simple("goldfish_rtc", memmap[VIRT_RTC].base,
+        qdev_get_gpio_in(DEVICE(s->plic), RTC_IRQ));
+
     g_free(plic_hart_config);
 }
 
diff --git a/include/hw/riscv/virt.h b/include/hw/riscv/virt.h
index 6e5fbe5d3b..e6423258d3 100644
--- a/include/hw/riscv/virt.h
+++ b/include/hw/riscv/virt.h
@@ -37,6 +37,7 @@ enum {
     VIRT_DEBUG,
     VIRT_MROM,
     VIRT_TEST,
+    VIRT_RTC,
     VIRT_CLINT,
     VIRT_PLIC,
     VIRT_UART0,
@@ -49,6 +50,7 @@ enum {
 
 enum {
     UART0_IRQ = 10,
+    RTC_IRQ = 11,
     VIRTIO_IRQ = 1, /* 1 to 8 */
     VIRTIO_COUNT = 8,
     PCIE_IRQ = 0x20, /* 32 to 35 */
-- 
2.17.1

Re: [PATCH 1/2] hw: timer: Add Goldfish RTC device

[Peter Maydell]

On Tue, 24 Sep 2019 at 09:45, Anup Patel <Anup.Patel@wdc.com> wrote:
>
> This patch adds model for Google Goldfish virtual platform RTC device.
>
> We will be adding Goldfish RTC device to the QEMU RISC-V virt machine
> for providing real date-time to Guest Linux. The corresponding Linux
> driver for Goldfish RTC device is already available in upstream Linux.
>
> For now, VM migration support is not available for Goldfish RTC device
> but it will be added later when we implement VM migration for KVM RISC-V.
>
> Signed-off-by: Anup Patel <anup.patel@wdc.com>
> ---
> +
> +static Property goldfish_rtc_properties[] = {
> +    DEFINE_PROP_UINT64("tick-offset", GoldfishRTCState, tick_offset, 0),
> +    DEFINE_PROP_UINT64("alarm-next", GoldfishRTCState, alarm_next, 0),
> +    DEFINE_PROP_UINT32("alarm-running", GoldfishRTCState, alarm_running, 0),
> +    DEFINE_PROP_UINT32("irq-pending", GoldfishRTCState, irq_pending, 0),
> +    DEFINE_PROP_UINT32("irq-enabled", GoldfishRTCState, irq_enabled, 0),
> +    DEFINE_PROP_END_OF_LIST(),
> +};

What are all these properties trying to do ?

> +
> +static void goldfish_rtc_class_init(ObjectClass *klass, void *data)
> +{
> +    DeviceClass *dc = DEVICE_CLASS(klass);
> +    dc->props = goldfish_rtc_properties;

Missing reset function.

If you don't want to implement migration support now
you should at least put in something that block migration.
(Personally I prefer to just write the vmstate, it's as
easy as writing the code to block migrations.)

thanks
-- PMM

Re: [PATCH 1/2] hw: timer: Add Goldfish RTC device

[Peter Maydell]

On Tue, 24 Sep 2019 at 09:45, Anup Patel <Anup.Patel@wdc.com> wrote:
>
> This patch adds model for Google Goldfish virtual platform RTC device.
>
> We will be adding Goldfish RTC device to the QEMU RISC-V virt machine
> for providing real date-time to Guest Linux. The corresponding Linux
> driver for Goldfish RTC device is already available in upstream Linux.
>
> For now, VM migration support is not available for Goldfish RTC device
> but it will be added later when we implement VM migration for KVM RISC-V.
>
> Signed-off-by: Anup Patel <anup.patel@wdc.com>
> ---
> +
> +static Property goldfish_rtc_properties[] = {
> +    DEFINE_PROP_UINT64("tick-offset", GoldfishRTCState, tick_offset, 0),
> +    DEFINE_PROP_UINT64("alarm-next", GoldfishRTCState, alarm_next, 0),
> +    DEFINE_PROP_UINT32("alarm-running", GoldfishRTCState, alarm_running, 0),
> +    DEFINE_PROP_UINT32("irq-pending", GoldfishRTCState, irq_pending, 0),
> +    DEFINE_PROP_UINT32("irq-enabled", GoldfishRTCState, irq_enabled, 0),
> +    DEFINE_PROP_END_OF_LIST(),
> +};

What are all these properties trying to do ?

> +
> +static void goldfish_rtc_class_init(ObjectClass *klass, void *data)
> +{
> +    DeviceClass *dc = DEVICE_CLASS(klass);
> +    dc->props = goldfish_rtc_properties;

Missing reset function.

If you don't want to implement migration support now
you should at least put in something that block migration.
(Personally I prefer to just write the vmstate, it's as
easy as writing the code to block migrations.)

thanks
-- PMM

RE: [PATCH 1/2] hw: timer: Add Goldfish RTC device

[Anup Patel]

> -----Original Message-----
> From: Peter Maydell <peter.maydell@linaro.org>
> Sent: Tuesday, September 24, 2019 3:21 PM
> To: Anup Patel <Anup.Patel@wdc.com>
> Cc: Palmer Dabbelt <palmer@sifive.com>; Alistair Francis
> <Alistair.Francis@wdc.com>; Sagar Karandikar <sagark@eecs.berkeley.edu>;
> Bastian Koppelmann <kbastian@mail.uni-paderborn.de>; Atish Patra
> <Atish.Patra@wdc.com>; qemu-riscv@nongnu.org; qemu-
> devel@nongnu.org; Anup Patel <anup@brainfault.org>
> Subject: Re: [PATCH 1/2] hw: timer: Add Goldfish RTC device
> 
> On Tue, 24 Sep 2019 at 09:45, Anup Patel <Anup.Patel@wdc.com> wrote:
> >
> > This patch adds model for Google Goldfish virtual platform RTC device.
> >
> > We will be adding Goldfish RTC device to the QEMU RISC-V virt machine
> > for providing real date-time to Guest Linux. The corresponding Linux
> > driver for Goldfish RTC device is already available in upstream Linux.
> >
> > For now, VM migration support is not available for Goldfish RTC device
> > but it will be added later when we implement VM migration for KVM RISC-
> V.
> >
> > Signed-off-by: Anup Patel <anup.patel@wdc.com>
> > ---
> > +
> > +static Property goldfish_rtc_properties[] = {
> > +    DEFINE_PROP_UINT64("tick-offset", GoldfishRTCState, tick_offset, 0),
> > +    DEFINE_PROP_UINT64("alarm-next", GoldfishRTCState, alarm_next, 0),
> > +    DEFINE_PROP_UINT32("alarm-running", GoldfishRTCState,
> alarm_running, 0),
> > +    DEFINE_PROP_UINT32("irq-pending", GoldfishRTCState, irq_pending,
> 0),
> > +    DEFINE_PROP_UINT32("irq-enabled", GoldfishRTCState, irq_enabled,
> 0),
> > +    DEFINE_PROP_END_OF_LIST(),
> > +};
> 
> What are all these properties trying to do ?

Argh, I forgot to remove these before sending.

I will drop these in next revision.

> 
> > +
> > +static void goldfish_rtc_class_init(ObjectClass *klass, void *data) {
> > +    DeviceClass *dc = DEVICE_CLASS(klass);
> > +    dc->props = goldfish_rtc_properties;
> 
> Missing reset function.

Sure, I will add it. I got confused because I saw reset function in HPET
but not in PL031.

> 
> If you don't want to implement migration support now you should at least
> put in something that block migration.
> (Personally I prefer to just write the vmstate, it's as easy as writing the code
> to block migrations.)

Okay, I will add vmstate.

Is there a way to unit test the vmstate part ?
OR
Are you fine if we test-n-fix it later ?

> 
> thanks
> -- PMM

Regards,
Anup

RE: [PATCH 1/2] hw: timer: Add Goldfish RTC device

[Anup Patel]

> -----Original Message-----
> From: Peter Maydell <peter.maydell@linaro.org>
> Sent: Tuesday, September 24, 2019 3:21 PM
> To: Anup Patel <Anup.Patel@wdc.com>
> Cc: Palmer Dabbelt <palmer@sifive.com>; Alistair Francis
> <Alistair.Francis@wdc.com>; Sagar Karandikar <sagark@eecs.berkeley.edu>;
> Bastian Koppelmann <kbastian@mail.uni-paderborn.de>; Atish Patra
> <Atish.Patra@wdc.com>; qemu-riscv@nongnu.org; qemu-
> devel@nongnu.org; Anup Patel <anup@brainfault.org>
> Subject: Re: [PATCH 1/2] hw: timer: Add Goldfish RTC device
> 
> On Tue, 24 Sep 2019 at 09:45, Anup Patel <Anup.Patel@wdc.com> wrote:
> >
> > This patch adds model for Google Goldfish virtual platform RTC device.
> >
> > We will be adding Goldfish RTC device to the QEMU RISC-V virt machine
> > for providing real date-time to Guest Linux. The corresponding Linux
> > driver for Goldfish RTC device is already available in upstream Linux.
> >
> > For now, VM migration support is not available for Goldfish RTC device
> > but it will be added later when we implement VM migration for KVM RISC-
> V.
> >
> > Signed-off-by: Anup Patel <anup.patel@wdc.com>
> > ---
> > +
> > +static Property goldfish_rtc_properties[] = {
> > +    DEFINE_PROP_UINT64("tick-offset", GoldfishRTCState, tick_offset, 0),
> > +    DEFINE_PROP_UINT64("alarm-next", GoldfishRTCState, alarm_next, 0),
> > +    DEFINE_PROP_UINT32("alarm-running", GoldfishRTCState,
> alarm_running, 0),
> > +    DEFINE_PROP_UINT32("irq-pending", GoldfishRTCState, irq_pending,
> 0),
> > +    DEFINE_PROP_UINT32("irq-enabled", GoldfishRTCState, irq_enabled,
> 0),
> > +    DEFINE_PROP_END_OF_LIST(),
> > +};
> 
> What are all these properties trying to do ?

Argh, I forgot to remove these before sending.

I will drop these in next revision.

> 
> > +
> > +static void goldfish_rtc_class_init(ObjectClass *klass, void *data) {
> > +    DeviceClass *dc = DEVICE_CLASS(klass);
> > +    dc->props = goldfish_rtc_properties;
> 
> Missing reset function.

Sure, I will add it. I got confused because I saw reset function in HPET
but not in PL031.

> 
> If you don't want to implement migration support now you should at least
> put in something that block migration.
> (Personally I prefer to just write the vmstate, it's as easy as writing the code
> to block migrations.)

Okay, I will add vmstate.

Is there a way to unit test the vmstate part ?
OR
Are you fine if we test-n-fix it later ?

> 
> thanks
> -- PMM

Regards,
Anup

Re: [PATCH 1/2] hw: timer: Add Goldfish RTC device

[Peter Maydell]

On Tue, 24 Sep 2019 at 12:17, Anup Patel <Anup.Patel@wdc.com> wrote:
>
>
>
> > -----Original Message-----
> > From: Peter Maydell <peter.maydell@linaro.org>
> > Sent: Tuesday, September 24, 2019 3:21 PM
> > To: Anup Patel <Anup.Patel@wdc.com>
> > Cc: Palmer Dabbelt <palmer@sifive.com>; Alistair Francis
> > <Alistair.Francis@wdc.com>; Sagar Karandikar <sagark@eecs.berkeley.edu>;
> > Bastian Koppelmann <kbastian@mail.uni-paderborn.de>; Atish Patra
> > <Atish.Patra@wdc.com>; qemu-riscv@nongnu.org; qemu-
> > devel@nongnu.org; Anup Patel <anup@brainfault.org>
> > Subject: Re: [PATCH 1/2] hw: timer: Add Goldfish RTC device
> >
> > On Tue, 24 Sep 2019 at 09:45, Anup Patel <Anup.Patel@wdc.com> wrote:
> > >
> > > This patch adds model for Google Goldfish virtual platform RTC device.
> > >
> > > We will be adding Goldfish RTC device to the QEMU RISC-V virt machine
> > > for providing real date-time to Guest Linux. The corresponding Linux
> > > driver for Goldfish RTC device is already available in upstream Linux.
> > >
> > > For now, VM migration support is not available for Goldfish RTC device
> > > but it will be added later when we implement VM migration for KVM RISC-
> > V.
> > >
> > > Signed-off-by: Anup Patel <anup.patel@wdc.com>

> > > ---
> > > +
> > > +static Property goldfish_rtc_properties[] = {
> > > +    DEFINE_PROP_UINT64("tick-offset", GoldfishRTCState, tick_offset, 0),
> > > +    DEFINE_PROP_UINT64("alarm-next", GoldfishRTCState, alarm_next, 0),
> > > +    DEFINE_PROP_UINT32("alarm-running", GoldfishRTCState,
> > alarm_running, 0),
> > > +    DEFINE_PROP_UINT32("irq-pending", GoldfishRTCState, irq_pending,
> > 0),
> > > +    DEFINE_PROP_UINT32("irq-enabled", GoldfishRTCState, irq_enabled,
> > 0),
> > > +    DEFINE_PROP_END_OF_LIST(),
> > > +};
> >
> > What are all these properties trying to do ?
>
> Argh, I forgot to remove these before sending.
>
> I will drop these in next revision.
>
> >
> > > +
> > > +static void goldfish_rtc_class_init(ObjectClass *klass, void *data) {
> > > +    DeviceClass *dc = DEVICE_CLASS(klass);
> > > +    dc->props = goldfish_rtc_properties;
> >
> > Missing reset function.
>
> Sure, I will add it. I got confused because I saw reset function in HPET
> but not in PL031.

Yeah, the lack of reset is a bug in the pl031. I did have
a draft set of patches where I tried to fix that, but I
got stuck because it was a bit unclear what I ought to be
resetting. In a real hardware pl031 there is no persistent
storage of the RTC value -- it's just a 1Hz counter, really,
and guest firmware will write to it on startup (from some
other real RTC). In QEMU we use it as a sort of "RTC for a
VM", and back it with the QEMU RTC clock, so it doesn't
behave quite as the hardware does but more like "view that
you'd see from a combination of h/w and firmware behaviour".

TLDR: don't use the pl031 code as a good model of how to do an RTC,
it has some definite flaws. x86 or ppc RTC handling is
probably a better place to look.

Another random question -- is there an existing RTC device
we could put in the riscv board rather than adding a new
model of this goldfish device? Put another way, what are
the merits of using goldfish in particular?

We should probably document this device in docs/specs since it's
a pure-virtual device. (You have a link to the URL for the
GOLDFISH-VIRTUAL-HARDWARE.TXT for the android emulator, but that
doesn't seem to match the implementation here -- the doc says the
alarm registers are non-functional but this code does something with
them.)

> > If you don't want to implement migration support now you should at least
> > put in something that block migration.
> > (Personally I prefer to just write the vmstate, it's as easy as writing the code
> > to block migrations.)
>
> Okay, I will add vmstate.
>
> Is there a way to unit test the vmstate part ?
> OR
> Are you fine if we test-n-fix it later ?

Test and fix later is fine. That said, I've just remembered
that for an RTC migration is potentially a tricky area
because you typically don't want a vmsave-vmload to
give you the same RTC value after load that there was on
store, you want it to still be following the host RTC.
We got that wrong on the PL031 and had to put in some ugly
workarounds to maintain migration compatibility when we fixed it.
So maybe it would be better to mark as non-migratable for now.

thanks
-- PMM

Re: [PATCH 1/2] hw: timer: Add Goldfish RTC device

[Peter Maydell]

On Tue, 24 Sep 2019 at 12:17, Anup Patel <Anup.Patel@wdc.com> wrote:
>
>
>
> > -----Original Message-----
> > From: Peter Maydell <peter.maydell@linaro.org>
> > Sent: Tuesday, September 24, 2019 3:21 PM
> > To: Anup Patel <Anup.Patel@wdc.com>
> > Cc: Palmer Dabbelt <palmer@sifive.com>; Alistair Francis
> > <Alistair.Francis@wdc.com>; Sagar Karandikar <sagark@eecs.berkeley.edu>;
> > Bastian Koppelmann <kbastian@mail.uni-paderborn.de>; Atish Patra
> > <Atish.Patra@wdc.com>; qemu-riscv@nongnu.org; qemu-
> > devel@nongnu.org; Anup Patel <anup@brainfault.org>
> > Subject: Re: [PATCH 1/2] hw: timer: Add Goldfish RTC device
> >
> > On Tue, 24 Sep 2019 at 09:45, Anup Patel <Anup.Patel@wdc.com> wrote:
> > >
> > > This patch adds model for Google Goldfish virtual platform RTC device.
> > >
> > > We will be adding Goldfish RTC device to the QEMU RISC-V virt machine
> > > for providing real date-time to Guest Linux. The corresponding Linux
> > > driver for Goldfish RTC device is already available in upstream Linux.
> > >
> > > For now, VM migration support is not available for Goldfish RTC device
> > > but it will be added later when we implement VM migration for KVM RISC-
> > V.
> > >
> > > Signed-off-by: Anup Patel <anup.patel@wdc.com>

> > > ---
> > > +
> > > +static Property goldfish_rtc_properties[] = {
> > > +    DEFINE_PROP_UINT64("tick-offset", GoldfishRTCState, tick_offset, 0),
> > > +    DEFINE_PROP_UINT64("alarm-next", GoldfishRTCState, alarm_next, 0),
> > > +    DEFINE_PROP_UINT32("alarm-running", GoldfishRTCState,
> > alarm_running, 0),
> > > +    DEFINE_PROP_UINT32("irq-pending", GoldfishRTCState, irq_pending,
> > 0),
> > > +    DEFINE_PROP_UINT32("irq-enabled", GoldfishRTCState, irq_enabled,
> > 0),
> > > +    DEFINE_PROP_END_OF_LIST(),
> > > +};
> >
> > What are all these properties trying to do ?
>
> Argh, I forgot to remove these before sending.
>
> I will drop these in next revision.
>
> >
> > > +
> > > +static void goldfish_rtc_class_init(ObjectClass *klass, void *data) {
> > > +    DeviceClass *dc = DEVICE_CLASS(klass);
> > > +    dc->props = goldfish_rtc_properties;
> >
> > Missing reset function.
>
> Sure, I will add it. I got confused because I saw reset function in HPET
> but not in PL031.

Yeah, the lack of reset is a bug in the pl031. I did have
a draft set of patches where I tried to fix that, but I
got stuck because it was a bit unclear what I ought to be
resetting. In a real hardware pl031 there is no persistent
storage of the RTC value -- it's just a 1Hz counter, really,
and guest firmware will write to it on startup (from some
other real RTC). In QEMU we use it as a sort of "RTC for a
VM", and back it with the QEMU RTC clock, so it doesn't
behave quite as the hardware does but more like "view that
you'd see from a combination of h/w and firmware behaviour".

TLDR: don't use the pl031 code as a good model of how to do an RTC,
it has some definite flaws. x86 or ppc RTC handling is
probably a better place to look.

Another random question -- is there an existing RTC device
we could put in the riscv board rather than adding a new
model of this goldfish device? Put another way, what are
the merits of using goldfish in particular?

We should probably document this device in docs/specs since it's
a pure-virtual device. (You have a link to the URL for the
GOLDFISH-VIRTUAL-HARDWARE.TXT for the android emulator, but that
doesn't seem to match the implementation here -- the doc says the
alarm registers are non-functional but this code does something with
them.)

> > If you don't want to implement migration support now you should at least
> > put in something that block migration.
> > (Personally I prefer to just write the vmstate, it's as easy as writing the code
> > to block migrations.)
>
> Okay, I will add vmstate.
>
> Is there a way to unit test the vmstate part ?
> OR
> Are you fine if we test-n-fix it later ?

Test and fix later is fine. That said, I've just remembered
that for an RTC migration is potentially a tricky area
because you typically don't want a vmsave-vmload to
give you the same RTC value after load that there was on
store, you want it to still be following the host RTC.
We got that wrong on the PL031 and had to put in some ugly
workarounds to maintain migration compatibility when we fixed it.
So maybe it would be better to mark as non-migratable for now.

thanks
-- PMM

RE: [PATCH 1/2] hw: timer: Add Goldfish RTC device

[Anup Patel]

> -----Original Message-----
> From: Peter Maydell <peter.maydell@linaro.org>
> Sent: Tuesday, September 24, 2019 5:02 PM
> To: Anup Patel <Anup.Patel@wdc.com>
> Cc: Palmer Dabbelt <palmer@sifive.com>; Alistair Francis
> <Alistair.Francis@wdc.com>; Sagar Karandikar <sagark@eecs.berkeley.edu>;
> Bastian Koppelmann <kbastian@mail.uni-paderborn.de>; Atish Patra
> <Atish.Patra@wdc.com>; qemu-riscv@nongnu.org; qemu-
> devel@nongnu.org; Anup Patel <anup@brainfault.org>
> Subject: Re: [PATCH 1/2] hw: timer: Add Goldfish RTC device
> 
> On Tue, 24 Sep 2019 at 12:17, Anup Patel <Anup.Patel@wdc.com> wrote:
> >
> >
> >
> > > -----Original Message-----
> > > From: Peter Maydell <peter.maydell@linaro.org>
> > > Sent: Tuesday, September 24, 2019 3:21 PM
> > > To: Anup Patel <Anup.Patel@wdc.com>
> > > Cc: Palmer Dabbelt <palmer@sifive.com>; Alistair Francis
> > > <Alistair.Francis@wdc.com>; Sagar Karandikar
> > > <sagark@eecs.berkeley.edu>; Bastian Koppelmann
> > > <kbastian@mail.uni-paderborn.de>; Atish Patra
> <Atish.Patra@wdc.com>;
> > > qemu-riscv@nongnu.org; qemu- devel@nongnu.org; Anup Patel
> > > <anup@brainfault.org>
> > > Subject: Re: [PATCH 1/2] hw: timer: Add Goldfish RTC device
> > >
> > > On Tue, 24 Sep 2019 at 09:45, Anup Patel <Anup.Patel@wdc.com> wrote:
> > > >
> > > > This patch adds model for Google Goldfish virtual platform RTC device.
> > > >
> > > > We will be adding Goldfish RTC device to the QEMU RISC-V virt
> > > > machine for providing real date-time to Guest Linux. The
> > > > corresponding Linux driver for Goldfish RTC device is already available in
> upstream Linux.
> > > >
> > > > For now, VM migration support is not available for Goldfish RTC
> > > > device but it will be added later when we implement VM migration
> > > > for KVM RISC-
> > > V.
> > > >
> > > > Signed-off-by: Anup Patel <anup.patel@wdc.com>
> 
> > > > ---
> > > > +
> > > > +static Property goldfish_rtc_properties[] = {
> > > > +    DEFINE_PROP_UINT64("tick-offset", GoldfishRTCState, tick_offset,
> 0),
> > > > +    DEFINE_PROP_UINT64("alarm-next", GoldfishRTCState, alarm_next,
> 0),
> > > > +    DEFINE_PROP_UINT32("alarm-running", GoldfishRTCState,
> > > alarm_running, 0),
> > > > +    DEFINE_PROP_UINT32("irq-pending", GoldfishRTCState,
> > > > + irq_pending,
> > > 0),
> > > > +    DEFINE_PROP_UINT32("irq-enabled", GoldfishRTCState,
> > > > + irq_enabled,
> > > 0),
> > > > +    DEFINE_PROP_END_OF_LIST(),
> > > > +};
> > >
> > > What are all these properties trying to do ?
> >
> > Argh, I forgot to remove these before sending.
> >
> > I will drop these in next revision.
> >
> > >
> > > > +
> > > > +static void goldfish_rtc_class_init(ObjectClass *klass, void *data) {
> > > > +    DeviceClass *dc = DEVICE_CLASS(klass);
> > > > +    dc->props = goldfish_rtc_properties;
> > >
> > > Missing reset function.
> >
> > Sure, I will add it. I got confused because I saw reset function in
> > HPET but not in PL031.
> 
> Yeah, the lack of reset is a bug in the pl031. I did have a draft set of patches
> where I tried to fix that, but I got stuck because it was a bit unclear what I
> ought to be resetting. In a real hardware pl031 there is no persistent storage
> of the RTC value -- it's just a 1Hz counter, really, and guest firmware will write
> to it on startup (from some other real RTC). In QEMU we use it as a sort of
> "RTC for a VM", and back it with the QEMU RTC clock, so it doesn't behave
> quite as the hardware does but more like "view that you'd see from a
> combination of h/w and firmware behaviour".
> 
> TLDR: don't use the pl031 code as a good model of how to do an RTC, it has
> some definite flaws. x86 or ppc RTC handling is probably a better place to
> look.
> 
> Another random question -- is there an existing RTC device we could put in
> the riscv board rather than adding a new model of this goldfish device? Put
> another way, what are the merits of using goldfish in particular?

As of now, there is no RTC device used across SOCs. Atleast nothing that I
am aware of.

I just wanted a very simple RTC for RISC-V virt machine and found goldfish
RTC to be really useful. Although other Goldfish devices are not very
attractive and we generally have an equivalent VirtIO device.

Ideally, we should have a VirtIO RTC thingy but not sure if RTC fits
in VirtIO ring programming model.

> 
> We should probably document this device in docs/specs since it's a pure-
> virtual device. (You have a link to the URL for the GOLDFISH-VIRTUAL-
> HARDWARE.TXT for the android emulator, but that doesn't seem to match
> the implementation here -- the doc says the alarm registers are non-
> functional but this code does something with
> them.)

The implementation matches corresponding driver available in upstream
Linux. We should certainly have a more complete document under
docs/sepcs (like you suggested) for this device.

> 
> > > If you don't want to implement migration support now you should at
> > > least put in something that block migration.
> > > (Personally I prefer to just write the vmstate, it's as easy as
> > > writing the code to block migrations.)
> >
> > Okay, I will add vmstate.
> >
> > Is there a way to unit test the vmstate part ?
> > OR
> > Are you fine if we test-n-fix it later ?
> 
> Test and fix later is fine. That said, I've just remembered that for an RTC
> migration is potentially a tricky area because you typically don't want a
> vmsave-vmload to give you the same RTC value after load that there was on
> store, you want it to still be following the host RTC.
> We got that wrong on the PL031 and had to put in some ugly workarounds to
> maintain migration compatibility when we fixed it.
> So maybe it would be better to mark as non-migratable for now.

Yes, I saw the workaround in PL031 and the backward compatibility bits.

This is first version of Goldfish RTC vmstate so we don't need the PL031
backward compatibility bits.

I will try implement this in v2 and you can have a look.

Regards,
Anup

RE: [PATCH 1/2] hw: timer: Add Goldfish RTC device

[Anup Patel]

> -----Original Message-----
> From: Peter Maydell <peter.maydell@linaro.org>
> Sent: Tuesday, September 24, 2019 5:02 PM
> To: Anup Patel <Anup.Patel@wdc.com>
> Cc: Palmer Dabbelt <palmer@sifive.com>; Alistair Francis
> <Alistair.Francis@wdc.com>; Sagar Karandikar <sagark@eecs.berkeley.edu>;
> Bastian Koppelmann <kbastian@mail.uni-paderborn.de>; Atish Patra
> <Atish.Patra@wdc.com>; qemu-riscv@nongnu.org; qemu-
> devel@nongnu.org; Anup Patel <anup@brainfault.org>
> Subject: Re: [PATCH 1/2] hw: timer: Add Goldfish RTC device
> 
> On Tue, 24 Sep 2019 at 12:17, Anup Patel <Anup.Patel@wdc.com> wrote:
> >
> >
> >
> > > -----Original Message-----
> > > From: Peter Maydell <peter.maydell@linaro.org>
> > > Sent: Tuesday, September 24, 2019 3:21 PM
> > > To: Anup Patel <Anup.Patel@wdc.com>
> > > Cc: Palmer Dabbelt <palmer@sifive.com>; Alistair Francis
> > > <Alistair.Francis@wdc.com>; Sagar Karandikar
> > > <sagark@eecs.berkeley.edu>; Bastian Koppelmann
> > > <kbastian@mail.uni-paderborn.de>; Atish Patra
> <Atish.Patra@wdc.com>;
> > > qemu-riscv@nongnu.org; qemu- devel@nongnu.org; Anup Patel
> > > <anup@brainfault.org>
> > > Subject: Re: [PATCH 1/2] hw: timer: Add Goldfish RTC device
> > >
> > > On Tue, 24 Sep 2019 at 09:45, Anup Patel <Anup.Patel@wdc.com> wrote:
> > > >
> > > > This patch adds model for Google Goldfish virtual platform RTC device.
> > > >
> > > > We will be adding Goldfish RTC device to the QEMU RISC-V virt
> > > > machine for providing real date-time to Guest Linux. The
> > > > corresponding Linux driver for Goldfish RTC device is already available in
> upstream Linux.
> > > >
> > > > For now, VM migration support is not available for Goldfish RTC
> > > > device but it will be added later when we implement VM migration
> > > > for KVM RISC-
> > > V.
> > > >
> > > > Signed-off-by: Anup Patel <anup.patel@wdc.com>
> 
> > > > ---
> > > > +
> > > > +static Property goldfish_rtc_properties[] = {
> > > > +    DEFINE_PROP_UINT64("tick-offset", GoldfishRTCState, tick_offset,
> 0),
> > > > +    DEFINE_PROP_UINT64("alarm-next", GoldfishRTCState, alarm_next,
> 0),
> > > > +    DEFINE_PROP_UINT32("alarm-running", GoldfishRTCState,
> > > alarm_running, 0),
> > > > +    DEFINE_PROP_UINT32("irq-pending", GoldfishRTCState,
> > > > + irq_pending,
> > > 0),
> > > > +    DEFINE_PROP_UINT32("irq-enabled", GoldfishRTCState,
> > > > + irq_enabled,
> > > 0),
> > > > +    DEFINE_PROP_END_OF_LIST(),
> > > > +};
> > >
> > > What are all these properties trying to do ?
> >
> > Argh, I forgot to remove these before sending.
> >
> > I will drop these in next revision.
> >
> > >
> > > > +
> > > > +static void goldfish_rtc_class_init(ObjectClass *klass, void *data) {
> > > > +    DeviceClass *dc = DEVICE_CLASS(klass);
> > > > +    dc->props = goldfish_rtc_properties;
> > >
> > > Missing reset function.
> >
> > Sure, I will add it. I got confused because I saw reset function in
> > HPET but not in PL031.
> 
> Yeah, the lack of reset is a bug in the pl031. I did have a draft set of patches
> where I tried to fix that, but I got stuck because it was a bit unclear what I
> ought to be resetting. In a real hardware pl031 there is no persistent storage
> of the RTC value -- it's just a 1Hz counter, really, and guest firmware will write
> to it on startup (from some other real RTC). In QEMU we use it as a sort of
> "RTC for a VM", and back it with the QEMU RTC clock, so it doesn't behave
> quite as the hardware does but more like "view that you'd see from a
> combination of h/w and firmware behaviour".
> 
> TLDR: don't use the pl031 code as a good model of how to do an RTC, it has
> some definite flaws. x86 or ppc RTC handling is probably a better place to
> look.
> 
> Another random question -- is there an existing RTC device we could put in
> the riscv board rather than adding a new model of this goldfish device? Put
> another way, what are the merits of using goldfish in particular?

As of now, there is no RTC device used across SOCs. Atleast nothing that I
am aware of.

I just wanted a very simple RTC for RISC-V virt machine and found goldfish
RTC to be really useful. Although other Goldfish devices are not very
attractive and we generally have an equivalent VirtIO device.

Ideally, we should have a VirtIO RTC thingy but not sure if RTC fits
in VirtIO ring programming model.

> 
> We should probably document this device in docs/specs since it's a pure-
> virtual device. (You have a link to the URL for the GOLDFISH-VIRTUAL-
> HARDWARE.TXT for the android emulator, but that doesn't seem to match
> the implementation here -- the doc says the alarm registers are non-
> functional but this code does something with
> them.)

The implementation matches corresponding driver available in upstream
Linux. We should certainly have a more complete document under
docs/sepcs (like you suggested) for this device.

> 
> > > If you don't want to implement migration support now you should at
> > > least put in something that block migration.
> > > (Personally I prefer to just write the vmstate, it's as easy as
> > > writing the code to block migrations.)
> >
> > Okay, I will add vmstate.
> >
> > Is there a way to unit test the vmstate part ?
> > OR
> > Are you fine if we test-n-fix it later ?
> 
> Test and fix later is fine. That said, I've just remembered that for an RTC
> migration is potentially a tricky area because you typically don't want a
> vmsave-vmload to give you the same RTC value after load that there was on
> store, you want it to still be following the host RTC.
> We got that wrong on the PL031 and had to put in some ugly workarounds to
> maintain migration compatibility when we fixed it.
> So maybe it would be better to mark as non-migratable for now.

Yes, I saw the workaround in PL031 and the backward compatibility bits.

This is first version of Goldfish RTC vmstate so we don't need the PL031
backward compatibility bits.

I will try implement this in v2 and you can have a look.

Regards,
Anup

Re: [PATCH 0/2] RTC support for QEMU RISC-V virt machine

[Richard W.M. Jones]

On Tue, Sep 24, 2019 at 08:42:36AM +0000, Anup Patel wrote:
> This series adds RTC device to QEMU RISC-V virt machine. We have
> selected Goldfish RTC device model for this. It's a pretty simple
> synthetic device with few MMIO registers and no dependency external
> clock. The driver for Goldfish RTC is already available in Linux so
> we just need to enable it in Kconfig for RISCV and also update Linux
> defconfigs.
> 
> We have tested this series with Linux-5.3 plus defconfig changes
> available in 'goldfish_rtc_v1' branch of:
> https://github.com/avpatel/linux.git

Why was this device chosen instead of kvm-clock?

Rich.

> Anup Patel (2):
>   hw: timer: Add Goldfish RTC device
>   riscv: virt: Use Goldfish RTC device
> 
>  hw/riscv/Kconfig                |   1 +
>  hw/riscv/virt.c                 |  15 +++
>  hw/timer/Kconfig                |   3 +
>  hw/timer/Makefile.objs          |   1 +
>  hw/timer/goldfish_rtc.c         | 221 ++++++++++++++++++++++++++++++++
>  include/hw/riscv/virt.h         |   2 +
>  include/hw/timer/goldfish_rtc.h |  45 +++++++
>  7 files changed, 288 insertions(+)
>  create mode 100644 hw/timer/goldfish_rtc.c
>  create mode 100644 include/hw/timer/goldfish_rtc.h
> 
> --
> 2.17.1

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-top is 'top' for virtual machines.  Tiny program with many
powerful monitoring features, net stats, disk stats, logging, etc.
http://people.redhat.com/~rjones/virt-top

Re: [PATCH 0/2] RTC support for QEMU RISC-V virt machine

[Richard W.M. Jones]

On Tue, Sep 24, 2019 at 08:42:36AM +0000, Anup Patel wrote:
> This series adds RTC device to QEMU RISC-V virt machine. We have
> selected Goldfish RTC device model for this. It's a pretty simple
> synthetic device with few MMIO registers and no dependency external
> clock. The driver for Goldfish RTC is already available in Linux so
> we just need to enable it in Kconfig for RISCV and also update Linux
> defconfigs.
> 
> We have tested this series with Linux-5.3 plus defconfig changes
> available in 'goldfish_rtc_v1' branch of:
> https://github.com/avpatel/linux.git

Why was this device chosen instead of kvm-clock?

Rich.

> Anup Patel (2):
>   hw: timer: Add Goldfish RTC device
>   riscv: virt: Use Goldfish RTC device
> 
>  hw/riscv/Kconfig                |   1 +
>  hw/riscv/virt.c                 |  15 +++
>  hw/timer/Kconfig                |   3 +
>  hw/timer/Makefile.objs          |   1 +
>  hw/timer/goldfish_rtc.c         | 221 ++++++++++++++++++++++++++++++++
>  include/hw/riscv/virt.h         |   2 +
>  include/hw/timer/goldfish_rtc.h |  45 +++++++
>  7 files changed, 288 insertions(+)
>  create mode 100644 hw/timer/goldfish_rtc.c
>  create mode 100644 include/hw/timer/goldfish_rtc.h
> 
> --
> 2.17.1

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-top is 'top' for virtual machines.  Tiny program with many
powerful monitoring features, net stats, disk stats, logging, etc.
http://people.redhat.com/~rjones/virt-top

RE: [PATCH 0/2] RTC support for QEMU RISC-V virt machine

[Anup Patel]

> -----Original Message-----
> From: Richard W.M. Jones <rjones@redhat.com>
> Sent: Friday, September 27, 2019 5:21 PM
> To: Anup Patel <Anup.Patel@wdc.com>
> Cc: Palmer Dabbelt <palmer@sifive.com>; Alistair Francis
> <Alistair.Francis@wdc.com>; Sagar Karandikar <sagark@eecs.berkeley.edu>;
> Bastian Koppelmann <kbastian@mail.uni-paderborn.de>; Atish Patra
> <Atish.Patra@wdc.com>; qemu-riscv@nongnu.org; qemu-
> devel@nongnu.org; Anup Patel <anup@brainfault.org>
> Subject: Re: [PATCH 0/2] RTC support for QEMU RISC-V virt machine
> 
> 
> On Tue, Sep 24, 2019 at 08:42:36AM +0000, Anup Patel wrote:
> > This series adds RTC device to QEMU RISC-V virt machine. We have
> > selected Goldfish RTC device model for this. It's a pretty simple
> > synthetic device with few MMIO registers and no dependency external
> > clock. The driver for Goldfish RTC is already available in Linux so we
> > just need to enable it in Kconfig for RISCV and also update Linux
> > defconfigs.
> >
> > We have tested this series with Linux-5.3 plus defconfig changes
> > available in 'goldfish_rtc_v1' branch of:
> > https://github.com/avpatel/linux.git
> 
> Why was this device chosen instead of kvm-clock?

We need a RTC device which worked fine in TCG mode (even without
KVM). The KVMCLOCK is PTP clock which depends on KVM hypercalls.

On ARM virt machine, we have PL031 so instead of that we have
Goldfish RTC on RISC-V virt machine.

Regards,
Anup

> 
> Rich.
> 
> > Anup Patel (2):
> >   hw: timer: Add Goldfish RTC device
> >   riscv: virt: Use Goldfish RTC device
> >
> >  hw/riscv/Kconfig                |   1 +
> >  hw/riscv/virt.c                 |  15 +++
> >  hw/timer/Kconfig                |   3 +
> >  hw/timer/Makefile.objs          |   1 +
> >  hw/timer/goldfish_rtc.c         | 221
> ++++++++++++++++++++++++++++++++
> >  include/hw/riscv/virt.h         |   2 +
> >  include/hw/timer/goldfish_rtc.h |  45 +++++++
> >  7 files changed, 288 insertions(+)
> >  create mode 100644 hw/timer/goldfish_rtc.c  create mode 100644
> > include/hw/timer/goldfish_rtc.h
> >
> > --
> > 2.17.1
> 
> --
> Richard Jones, Virtualization Group, Red Hat
> http://people.redhat.com/~rjones Read my programming and virtualization
> blog: http://rwmj.wordpress.com virt-top is 'top' for virtual machines.  Tiny
> program with many powerful monitoring features, net stats, disk stats,
> logging, etc.
> http://people.redhat.com/~rjones/virt-top

RE: [PATCH 0/2] RTC support for QEMU RISC-V virt machine

[Anup Patel]

> -----Original Message-----
> From: Richard W.M. Jones <rjones@redhat.com>
> Sent: Friday, September 27, 2019 5:21 PM
> To: Anup Patel <Anup.Patel@wdc.com>
> Cc: Palmer Dabbelt <palmer@sifive.com>; Alistair Francis
> <Alistair.Francis@wdc.com>; Sagar Karandikar <sagark@eecs.berkeley.edu>;
> Bastian Koppelmann <kbastian@mail.uni-paderborn.de>; Atish Patra
> <Atish.Patra@wdc.com>; qemu-riscv@nongnu.org; qemu-
> devel@nongnu.org; Anup Patel <anup@brainfault.org>
> Subject: Re: [PATCH 0/2] RTC support for QEMU RISC-V virt machine
> 
> 
> On Tue, Sep 24, 2019 at 08:42:36AM +0000, Anup Patel wrote:
> > This series adds RTC device to QEMU RISC-V virt machine. We have
> > selected Goldfish RTC device model for this. It's a pretty simple
> > synthetic device with few MMIO registers and no dependency external
> > clock. The driver for Goldfish RTC is already available in Linux so we
> > just need to enable it in Kconfig for RISCV and also update Linux
> > defconfigs.
> >
> > We have tested this series with Linux-5.3 plus defconfig changes
> > available in 'goldfish_rtc_v1' branch of:
> > https://github.com/avpatel/linux.git
> 
> Why was this device chosen instead of kvm-clock?

We need a RTC device which worked fine in TCG mode (even without
KVM). The KVMCLOCK is PTP clock which depends on KVM hypercalls.

On ARM virt machine, we have PL031 so instead of that we have
Goldfish RTC on RISC-V virt machine.

Regards,
Anup

> 
> Rich.
> 
> > Anup Patel (2):
> >   hw: timer: Add Goldfish RTC device
> >   riscv: virt: Use Goldfish RTC device
> >
> >  hw/riscv/Kconfig                |   1 +
> >  hw/riscv/virt.c                 |  15 +++
> >  hw/timer/Kconfig                |   3 +
> >  hw/timer/Makefile.objs          |   1 +
> >  hw/timer/goldfish_rtc.c         | 221
> ++++++++++++++++++++++++++++++++
> >  include/hw/riscv/virt.h         |   2 +
> >  include/hw/timer/goldfish_rtc.h |  45 +++++++
> >  7 files changed, 288 insertions(+)
> >  create mode 100644 hw/timer/goldfish_rtc.c  create mode 100644
> > include/hw/timer/goldfish_rtc.h
> >
> > --
> > 2.17.1
> 
> --
> Richard Jones, Virtualization Group, Red Hat
> http://people.redhat.com/~rjones Read my programming and virtualization
> blog: http://rwmj.wordpress.com virt-top is 'top' for virtual machines.  Tiny
> program with many powerful monitoring features, net stats, disk stats,
> logging, etc.
> http://people.redhat.com/~rjones/virt-top

Re: [PATCH 0/2] RTC support for QEMU RISC-V virt machine

[Richard W.M. Jones]

On Fri, Sep 27, 2019 at 12:05:43PM +0000, Anup Patel wrote:
> 
> 
> > -----Original Message-----
> > From: Richard W.M. Jones <rjones@redhat.com>
> > Sent: Friday, September 27, 2019 5:21 PM
> > To: Anup Patel <Anup.Patel@wdc.com>
> > Cc: Palmer Dabbelt <palmer@sifive.com>; Alistair Francis
> > <Alistair.Francis@wdc.com>; Sagar Karandikar <sagark@eecs.berkeley.edu>;
> > Bastian Koppelmann <kbastian@mail.uni-paderborn.de>; Atish Patra
> > <Atish.Patra@wdc.com>; qemu-riscv@nongnu.org; qemu-
> > devel@nongnu.org; Anup Patel <anup@brainfault.org>
> > Subject: Re: [PATCH 0/2] RTC support for QEMU RISC-V virt machine
> > 
> > 
> > On Tue, Sep 24, 2019 at 08:42:36AM +0000, Anup Patel wrote:
> > > This series adds RTC device to QEMU RISC-V virt machine. We have
> > > selected Goldfish RTC device model for this. It's a pretty simple
> > > synthetic device with few MMIO registers and no dependency external
> > > clock. The driver for Goldfish RTC is already available in Linux so we
> > > just need to enable it in Kconfig for RISCV and also update Linux
> > > defconfigs.
> > >
> > > We have tested this series with Linux-5.3 plus defconfig changes
> > > available in 'goldfish_rtc_v1' branch of:
> > > https://github.com/avpatel/linux.git
> > 
> > Why was this device chosen instead of kvm-clock?
> 
> We need a RTC device which worked fine in TCG mode (even without
> KVM). The KVMCLOCK is PTP clock which depends on KVM hypercalls.
> 
> On ARM virt machine, we have PL031 so instead of that we have
> Goldfish RTC on RISC-V virt machine.

Could we not make kvm-clock work on TCG (I wasn't aware that it needed
actual KVM - I wonder how timekeeping works on TCG?)

Alternately why not use PL031 here?

The reason I'm asking this is because adding a new virtual device
means we have to change this all the way up the stack (libvirt,
virt-*) _and_ have special cases everywhere just for RISC-V.  That's a
load of extra work for everyone.

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-builder quickly builds VMs from scratch
http://libguestfs.org/virt-builder.1.html

Re: [PATCH 0/2] RTC support for QEMU RISC-V virt machine

[Richard W.M. Jones]

On Fri, Sep 27, 2019 at 12:05:43PM +0000, Anup Patel wrote:
> 
> 
> > -----Original Message-----
> > From: Richard W.M. Jones <rjones@redhat.com>
> > Sent: Friday, September 27, 2019 5:21 PM
> > To: Anup Patel <Anup.Patel@wdc.com>
> > Cc: Palmer Dabbelt <palmer@sifive.com>; Alistair Francis
> > <Alistair.Francis@wdc.com>; Sagar Karandikar <sagark@eecs.berkeley.edu>;
> > Bastian Koppelmann <kbastian@mail.uni-paderborn.de>; Atish Patra
> > <Atish.Patra@wdc.com>; qemu-riscv@nongnu.org; qemu-
> > devel@nongnu.org; Anup Patel <anup@brainfault.org>
> > Subject: Re: [PATCH 0/2] RTC support for QEMU RISC-V virt machine
> > 
> > 
> > On Tue, Sep 24, 2019 at 08:42:36AM +0000, Anup Patel wrote:
> > > This series adds RTC device to QEMU RISC-V virt machine. We have
> > > selected Goldfish RTC device model for this. It's a pretty simple
> > > synthetic device with few MMIO registers and no dependency external
> > > clock. The driver for Goldfish RTC is already available in Linux so we
> > > just need to enable it in Kconfig for RISCV and also update Linux
> > > defconfigs.
> > >
> > > We have tested this series with Linux-5.3 plus defconfig changes
> > > available in 'goldfish_rtc_v1' branch of:
> > > https://github.com/avpatel/linux.git
> > 
> > Why was this device chosen instead of kvm-clock?
> 
> We need a RTC device which worked fine in TCG mode (even without
> KVM). The KVMCLOCK is PTP clock which depends on KVM hypercalls.
> 
> On ARM virt machine, we have PL031 so instead of that we have
> Goldfish RTC on RISC-V virt machine.

Could we not make kvm-clock work on TCG (I wasn't aware that it needed
actual KVM - I wonder how timekeeping works on TCG?)

Alternately why not use PL031 here?

The reason I'm asking this is because adding a new virtual device
means we have to change this all the way up the stack (libvirt,
virt-*) _and_ have special cases everywhere just for RISC-V.  That's a
load of extra work for everyone.

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-builder quickly builds VMs from scratch
http://libguestfs.org/virt-builder.1.html

RE: [PATCH 0/2] RTC support for QEMU RISC-V virt machine

[Anup Patel]

> -----Original Message-----
> From: Richard W.M. Jones <rjones@redhat.com>
> Sent: Friday, September 27, 2019 6:01 PM
> To: Anup Patel <Anup.Patel@wdc.com>
> Cc: Palmer Dabbelt <palmer@sifive.com>; Alistair Francis
> <Alistair.Francis@wdc.com>; Sagar Karandikar <sagark@eecs.berkeley.edu>;
> Bastian Koppelmann <kbastian@mail.uni-paderborn.de>; Atish Patra
> <Atish.Patra@wdc.com>; qemu-riscv@nongnu.org; qemu-
> devel@nongnu.org; Anup Patel <anup@brainfault.org>
> Subject: Re: [PATCH 0/2] RTC support for QEMU RISC-V virt machine
> 
> On Fri, Sep 27, 2019 at 12:05:43PM +0000, Anup Patel wrote:
> >
> >
> > > -----Original Message-----
> > > From: Richard W.M. Jones <rjones@redhat.com>
> > > Sent: Friday, September 27, 2019 5:21 PM
> > > To: Anup Patel <Anup.Patel@wdc.com>
> > > Cc: Palmer Dabbelt <palmer@sifive.com>; Alistair Francis
> > > <Alistair.Francis@wdc.com>; Sagar Karandikar
> > > <sagark@eecs.berkeley.edu>; Bastian Koppelmann
> > > <kbastian@mail.uni-paderborn.de>; Atish Patra
> <Atish.Patra@wdc.com>;
> > > qemu-riscv@nongnu.org; qemu- devel@nongnu.org; Anup Patel
> > > <anup@brainfault.org>
> > > Subject: Re: [PATCH 0/2] RTC support for QEMU RISC-V virt machine
> > >
> > >
> > > On Tue, Sep 24, 2019 at 08:42:36AM +0000, Anup Patel wrote:
> > > > This series adds RTC device to QEMU RISC-V virt machine. We have
> > > > selected Goldfish RTC device model for this. It's a pretty simple
> > > > synthetic device with few MMIO registers and no dependency
> > > > external clock. The driver for Goldfish RTC is already available
> > > > in Linux so we just need to enable it in Kconfig for RISCV and
> > > > also update Linux defconfigs.
> > > >
> > > > We have tested this series with Linux-5.3 plus defconfig changes
> > > > available in 'goldfish_rtc_v1' branch of:
> > > > https://github.com/avpatel/linux.git
> > >
> > > Why was this device chosen instead of kvm-clock?
> >
> > We need a RTC device which worked fine in TCG mode (even without
> KVM).
> > The KVMCLOCK is PTP clock which depends on KVM hypercalls.
> >
> > On ARM virt machine, we have PL031 so instead of that we have Goldfish
> > RTC on RISC-V virt machine.
> 
> Could we not make kvm-clock work on TCG (I wasn't aware that it needed
> actual KVM - I wonder how timekeeping works on TCG?)
> 
> Alternately why not use PL031 here?

PL031 requires input clock.

Of course, we can provide fake input clock (i.e. some random
fixed clock) to make it work but it will be a HACK.

Also, it will be really strange to hook an ARM device into RISC-V
virt machine. On other hand, Goldfish para-virt devices are used
across architectures x86, ARM and MIPS.

> 
> The reason I'm asking this is because adding a new virtual device means we
> have to change this all the way up the stack (libvirt,
> virt-*) _and_ have special cases everywhere just for RISC-V.  That's a load of
> extra work for everyone.

The RTC devices are pretty much transparent in usage. I am not sure
why libvirt will require to know about type of RTC device.

Regards,
Anup

RE: [PATCH 0/2] RTC support for QEMU RISC-V virt machine

[Anup Patel]

> -----Original Message-----
> From: Richard W.M. Jones <rjones@redhat.com>
> Sent: Friday, September 27, 2019 6:01 PM
> To: Anup Patel <Anup.Patel@wdc.com>
> Cc: Palmer Dabbelt <palmer@sifive.com>; Alistair Francis
> <Alistair.Francis@wdc.com>; Sagar Karandikar <sagark@eecs.berkeley.edu>;
> Bastian Koppelmann <kbastian@mail.uni-paderborn.de>; Atish Patra
> <Atish.Patra@wdc.com>; qemu-riscv@nongnu.org; qemu-
> devel@nongnu.org; Anup Patel <anup@brainfault.org>
> Subject: Re: [PATCH 0/2] RTC support for QEMU RISC-V virt machine
> 
> On Fri, Sep 27, 2019 at 12:05:43PM +0000, Anup Patel wrote:
> >
> >
> > > -----Original Message-----
> > > From: Richard W.M. Jones <rjones@redhat.com>
> > > Sent: Friday, September 27, 2019 5:21 PM
> > > To: Anup Patel <Anup.Patel@wdc.com>
> > > Cc: Palmer Dabbelt <palmer@sifive.com>; Alistair Francis
> > > <Alistair.Francis@wdc.com>; Sagar Karandikar
> > > <sagark@eecs.berkeley.edu>; Bastian Koppelmann
> > > <kbastian@mail.uni-paderborn.de>; Atish Patra
> <Atish.Patra@wdc.com>;
> > > qemu-riscv@nongnu.org; qemu- devel@nongnu.org; Anup Patel
> > > <anup@brainfault.org>
> > > Subject: Re: [PATCH 0/2] RTC support for QEMU RISC-V virt machine
> > >
> > >
> > > On Tue, Sep 24, 2019 at 08:42:36AM +0000, Anup Patel wrote:
> > > > This series adds RTC device to QEMU RISC-V virt machine. We have
> > > > selected Goldfish RTC device model for this. It's a pretty simple
> > > > synthetic device with few MMIO registers and no dependency
> > > > external clock. The driver for Goldfish RTC is already available
> > > > in Linux so we just need to enable it in Kconfig for RISCV and
> > > > also update Linux defconfigs.
> > > >
> > > > We have tested this series with Linux-5.3 plus defconfig changes
> > > > available in 'goldfish_rtc_v1' branch of:
> > > > https://github.com/avpatel/linux.git
> > >
> > > Why was this device chosen instead of kvm-clock?
> >
> > We need a RTC device which worked fine in TCG mode (even without
> KVM).
> > The KVMCLOCK is PTP clock which depends on KVM hypercalls.
> >
> > On ARM virt machine, we have PL031 so instead of that we have Goldfish
> > RTC on RISC-V virt machine.
> 
> Could we not make kvm-clock work on TCG (I wasn't aware that it needed
> actual KVM - I wonder how timekeeping works on TCG?)
> 
> Alternately why not use PL031 here?

PL031 requires input clock.

Of course, we can provide fake input clock (i.e. some random
fixed clock) to make it work but it will be a HACK.

Also, it will be really strange to hook an ARM device into RISC-V
virt machine. On other hand, Goldfish para-virt devices are used
across architectures x86, ARM and MIPS.

> 
> The reason I'm asking this is because adding a new virtual device means we
> have to change this all the way up the stack (libvirt,
> virt-*) _and_ have special cases everywhere just for RISC-V.  That's a load of
> extra work for everyone.

The RTC devices are pretty much transparent in usage. I am not sure
why libvirt will require to know about type of RTC device.

Regards,
Anup