[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
RE: [PATCH v4 1/2] hw: rtc: Add Goldfish RTC device
|
From: |
Anup Patel |
|
Subject: |
RE: [PATCH v4 1/2] hw: rtc: Add Goldfish RTC device |
|
Date: |
Fri, 25 Oct 2019 03:53:39 +0000 |
> -----Original Message-----
> From: Alistair Francis <address@hidden>
> Sent: Friday, October 25, 2019 5:40 AM
> To: Anup Patel <address@hidden>
> Cc: Peter Maydell <address@hidden>; Palmer Dabbelt
> <address@hidden>; Alistair Francis <address@hidden>; Sagar
> Karandikar <address@hidden>; Bastian Koppelmann
> <address@hidden>; Atish Patra <address@hidden>;
> address@hidden; address@hidden; Anup Patel
> <address@hidden>
> Subject: Re: [PATCH v4 1/2] hw: rtc: Add Goldfish RTC device
>
> On Tue, Oct 22, 2019 at 11:37 PM Anup Patel <address@hidden> 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 available but untested for Goldfish
> > RTC device. It will be hardened in-future when we implement VM
> > migration for KVM RISC-V.
> >
> > Signed-off-by: Anup Patel <address@hidden>
> > ---
> > hw/rtc/Kconfig | 3 +
> > hw/rtc/Makefile.objs | 1 +
> > hw/rtc/goldfish_rtc.c | 288 ++++++++++++++++++++++++++++++++
> > hw/rtc/trace-events | 4 +
> > include/hw/timer/goldfish_rtc.h | 46 +++++
> > 5 files changed, 342 insertions(+)
> > create mode 100644 hw/rtc/goldfish_rtc.c create mode 100644
> > include/hw/timer/goldfish_rtc.h
> >
> > diff --git a/hw/rtc/Kconfig b/hw/rtc/Kconfig index
> > 45daa8d655..bafe6ac2c9 100644
> > --- a/hw/rtc/Kconfig
> > +++ b/hw/rtc/Kconfig
> > @@ -21,3 +21,6 @@ config MC146818RTC
> >
> > config SUN4V_RTC
> > bool
> > +
> > +config GOLDFISH_RTC
> > + bool
> > diff --git a/hw/rtc/Makefile.objs b/hw/rtc/Makefile.objs index
> > 8dc9fcd3a9..aa208d0d10 100644
> > --- a/hw/rtc/Makefile.objs
> > +++ b/hw/rtc/Makefile.objs
> > @@ -11,3 +11,4 @@ common-obj-$(CONFIG_EXYNOS4) +=
> exynos4210_rtc.o
> > obj-$(CONFIG_MC146818RTC) += mc146818rtc.o
> > common-obj-$(CONFIG_SUN4V_RTC) += sun4v-rtc.o
> > common-obj-$(CONFIG_ASPEED_SOC) += aspeed_rtc.o
> > +common-obj-$(CONFIG_GOLDFISH_RTC) += goldfish_rtc.o
> > diff --git a/hw/rtc/goldfish_rtc.c b/hw/rtc/goldfish_rtc.c new file
> > mode 100644 index 0000000000..f49a8e4489
> > --- /dev/null
> > +++ b/hw/rtc/goldfish_rtc.c
> > @@ -0,0 +1,288 @@
> > +/*
> > + * 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/doc
> s
> > +/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"
I think goldfish_rtc.h should be under include/hw/rtc directory.
I will move it.
> > +#include "migration/vmstate.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"
> > +
> > +#include "trace.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) {
> > + goldfish_rtc_clear_alarm(s);
> > + goldfish_rtc_interrupt(s);
> > + } else {
> > + /*
> > + * We should be setting timer expiry to:
> > + * qemu_clock_get_ns(rtc_clock) + (event - ticks)
> > + * but this is equivalent to:
> > + * event - s->tick_offset
> > + */
> > + timer_mod(s->timer, event - s->tick_offset);
> > + s->alarm_running = 1;
> > + }
> > +}
> > +
> > +static uint64_t goldfish_rtc_read(void *opaque, hwaddr offset,
> > + unsigned size) {
> > + GoldfishRTCState *s = (GoldfishRTCState *)opaque;
>
> This shouldn't be cast, use GOLDFISH_RTC(opaque) instead
Actually, GOLDFISH_RTC() macro is for QEMU objects.
Since, opaque is a void pointer we don't need any explicit
type-cast. I will update this accordingly.
>
> > + uint64_t r = 0;
> > +
> > + 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,
> > + "%s: Bad offset 0x%x\n", __func__, (uint32_t)offset);
> > + break;
> > + }
> > +
> > + trace_goldfish_rtc_read(offset, r);
> > +
> > + return r;
> > +}
> > +
> > +static void goldfish_rtc_write(void *opaque, hwaddr offset,
> > + uint64_t value, unsigned size) {
> > + GoldfishRTCState *s = (GoldfishRTCState *)opaque;
>
> Same applies here.
Same as above.
>
> > + uint64_t current_tick, new_tick;
> > +
> > + 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,
> > + "%s: Bad offset 0x%x\n", __func__, (uint32_t)offset);
> > + break;
> > + }
> > +
> > + trace_aspeed_rtc_write(offset, value);
>
> This must be a typo.
Argh, copy-paste error. I will fix this soon.
Regards,
Anup
>
> Alistair
>
> > +}
> > +
> > +static int goldfish_rtc_pre_save(void *opaque) {
> > + uint64_t delta;
> > + GoldfishRTCState *s = opaque;
> > +
> > + /*
> > + * We want to migrate this offset, which sounds straightforward.
> > + * Unfortunately, we cannot directly pass tick_offset because
> > + * rtc_clock on destination Host might not be same source Host.
> > + *
> > + * To tackle, this we pass tick_offset relative to vm_clock from
> > + * source Host and make it relative to rtc_clock at destination Host.
> > + */
> > + delta = qemu_clock_get_ns(rtc_clock) -
> > + qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL);
> > + s->tick_offset_vmstate = s->tick_offset + delta;
> > +
> > + return 0;
> > +}
> > +
> > +static int goldfish_rtc_post_load(void *opaque, int version_id) {
> > + uint64_t delta;
> > + GoldfishRTCState *s = opaque;
> > +
> > + /*
> > + * We extract tick_offset from tick_offset_vmstate by doing
> > + * reverse math compared to pre_save() function.
> > + */
> > + delta = qemu_clock_get_ns(rtc_clock) -
> > + qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL);
> > + s->tick_offset = s->tick_offset_vmstate - delta;
> > +
> > + return 0;
> > +}
> > +
> > +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 const VMStateDescription goldfish_rtc_vmstate = {
> > + .name = TYPE_GOLDFISH_RTC,
> > + .version_id = 1,
> > + .pre_save = goldfish_rtc_pre_save,
> > + .post_load = goldfish_rtc_post_load,
> > + .fields = (VMStateField[]) {
> > + VMSTATE_UINT64(tick_offset_vmstate, GoldfishRTCState),
> > + VMSTATE_UINT64(alarm_next, GoldfishRTCState),
> > + VMSTATE_UINT32(alarm_running, GoldfishRTCState),
> > + VMSTATE_UINT32(irq_pending, GoldfishRTCState),
> > + VMSTATE_UINT32(irq_enabled, GoldfishRTCState),
> > + VMSTATE_END_OF_LIST()
> > + }
> > +};
> > +
> > +static void goldfish_rtc_reset(DeviceState *dev) {
> > + GoldfishRTCState *s = GOLDFISH_RTC(dev);
> > + struct tm tm;
> > +
> > + timer_del(s->timer);
> > +
> > + 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->tick_offset_vmstate = 0;
> > + s->alarm_next = 0;
> > + s->alarm_running = 0;
> > + s->irq_pending = 0;
> > + s->irq_enabled = 0;
> > +}
> > +
> > +static void goldfish_rtc_realize(DeviceState *d, Error **errp) {
> > + SysBusDevice *dev = SYS_BUS_DEVICE(d);
> > + GoldfishRTCState *s = GOLDFISH_RTC(d);
> > +
> > + memory_region_init_io(&s->iomem, OBJECT(s), &goldfish_rtc_ops, s,
> > + "goldfish_rtc", 0x1000);
> > + sysbus_init_mmio(dev, &s->iomem);
> > +
> > + sysbus_init_irq(dev, &s->irq);
> > +
> > + s->timer = timer_new_ns(rtc_clock, goldfish_rtc_interrupt, s); }
> > +
> > +static void goldfish_rtc_class_init(ObjectClass *klass, void *data) {
> > + DeviceClass *dc = DEVICE_CLASS(klass);
> > +
> > + dc->realize = goldfish_rtc_realize;
> > + dc->reset = goldfish_rtc_reset;
> > + dc->vmsd = &goldfish_rtc_vmstate; }
> > +
> > +static const TypeInfo goldfish_rtc_info = {
> > + .name = TYPE_GOLDFISH_RTC,
> > + .parent = TYPE_SYS_BUS_DEVICE,
> > + .instance_size = sizeof(GoldfishRTCState),
> > + .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/hw/rtc/trace-events b/hw/rtc/trace-events index
> > d6749f4616..0bfaa26cb8 100644
> > --- a/hw/rtc/trace-events
> > +++ b/hw/rtc/trace-events
> > @@ -17,3 +17,7 @@ pl031_set_alarm(uint32_t ticks) "alarm set for %u
> ticks"
> > # aspeed-rtc.c
> > aspeed_rtc_read(uint64_t addr, uint64_t value) "addr 0x%02" PRIx64 "
> > value 0x%08" PRIx64 aspeed_rtc_write(uint64_t addr, uint64_t value)
> > "addr 0x%02" PRIx64 " value 0x%08" PRIx64
> > +
> > +# goldfish_rtc.c
> > +goldfish_rtc_read(uint64_t addr, uint64_t value) "addr 0x%02" PRIx64
> > +" value 0x%08" PRIx64 goldfish_rtc_write(uint64_t addr, uint64_t
> > +value) "addr 0x%02" PRIx64 " value 0x%08" PRIx64
> > diff --git a/include/hw/timer/goldfish_rtc.h
> > b/include/hw/timer/goldfish_rtc.h new file mode 100644 index
> > 0000000000..dfb6d70b86
> > --- /dev/null
> > +++ b/include/hw/timer/goldfish_rtc.h
> > @@ -0,0 +1,46 @@
> > +/*
> > + * 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/doc
> s
> > +/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 tick_offset_vmstate;
> > + uint64_t alarm_next;
> > + uint32_t alarm_running;
> > + uint32_t irq_pending;
> > + uint32_t irq_enabled;
> > +} GoldfishRTCState;
> > +
> > +#endif
> > --
> > 2.17.1
> >
> >