| Message ID | CAB88-qOuB+uR6epr7h5F76A9qNOB358WikXDfmpvn-ceW71p7Q@mail.gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v2,1/3] hw/watchdog: wdt_ibex_aon.c: Implement the watchdog for the OpenTitan | expand |
On 22/9/22 17:58, Tyler Ng wrote: > 1. Adds fields to hold the value of mtime in timer_upper0 and timer_lower0. > > 2. Changes the read and write functions to use the mtime fields. > > 3. Updates the value of mtime in update_mtime() by extrapolating the > time elapsed. This will need to change if/when the prescalar is > implemented. > > 4. Adds a qtest for the ibex timer. > > Signed-off-by: Tyler Ng <tkng@rivosinc.com> > --- > hw/timer/ibex_timer.c | 98 +++++++++++++------ > include/hw/timer/ibex_timer.h | 6 ++ > tests/qtest/ibex-timer-test.c | 178 ++++++++++++++++++++++++++++++++++ > tests/qtest/meson.build | 3 +- > 4 files changed, 256 insertions(+), 29 deletions(-) > create mode 100644 tests/qtest/ibex-timer-test.c > -static void ibex_timer_update_irqs(IbexTimerState *s) > +/* > + * The goal of this function is to: > + * 1. Check if the timer is enabled. If not, return false, > + * 2. Calculate the amount of time that has passed since. > + * 3. Extrapolate the number of ticks that have passed, and add it to `mtime`. > + * 4. Return true. > + */ > +static bool update_mtime(IbexTimerState *s) > { > - uint64_t value = s->timer_compare_lower0 | > - ((uint64_t)s->timer_compare_upper0 << 32); > - uint64_t next, diff; > - uint64_t now = cpu_riscv_read_rtc(s->timebase_freq); > - > if (!(s->timer_ctrl & R_CTRL_ACTIVE_MASK)) { > - /* Timer isn't active */ > + return false; > + } > + /* Get the time then extrapolate the number of ticks that have elapsed */ > + uint64_t mtime = get_mtime(s); > + int64_t now = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL); > + int64_t elapsed = now - s->timer_last_update; > + if (elapsed < 0) { > + /* We jumped back in time. */ > + mtime -= muldiv64((uint64_t)(-elapsed), s->timebase_freq, > + NANOSECONDS_PER_SECOND); > + } else { > + mtime += muldiv64(elapsed, s->timebase_freq, NANOSECONDS_PER_SECOND); > + } > + s->timer_lower0 = mtime & 0xffffffff; > + s->timer_upper0 = (mtime >> 32) & 0xffffffff; Could use extract64(mtime, 0, 32) and extract64(mtime, 32, 32); > + /* update last-checkpoint timestamp */ > + s->timer_last_update = now; > + return true; > +}
> -----Original Message----- > From: Qemu-devel <qemu-devel-bounces+eddie.dong=intel.com@nongnu.org> > On Behalf Of Tyler Ng > Sent: Thursday, September 22, 2022 8:59 AM > To: open list:RISC-V <qemu-riscv@nongnu.org>; qemu-devel@nongnu.org > Developers <qemu-devel@nongnu.org> > Cc: Alistair Francis <Alistair.Francis@wdc.com>; Meng, Bin > <bin.meng@windriver.com>; Thomas Huth <thuth@redhat.com>; Paolo > Bonzini <pbonzini@redhat.com>; Palmer Dabbelt <palmer@dabbelt.com>; > Laurent Vivier <lvivier@redhat.com> > Subject: [PATCH v2 3/3] hw/timer: ibex_timer.c: Add support for writes to > mtime > > 1. Adds fields to hold the value of mtime in timer_upper0 and timer_lower0. > > 2. Changes the read and write functions to use the mtime fields. > > 3. Updates the value of mtime in update_mtime() by extrapolating the time > elapsed. This will need to change if/when the prescalar is implemented. > > 4. Adds a qtest for the ibex timer. > > Signed-off-by: Tyler Ng <tkng@rivosinc.com> > --- > hw/timer/ibex_timer.c | 98 +++++++++++++------ > include/hw/timer/ibex_timer.h | 6 ++ > tests/qtest/ibex-timer-test.c | 178 ++++++++++++++++++++++++++++++++++ > tests/qtest/meson.build | 3 +- > 4 files changed, 256 insertions(+), 29 deletions(-) create mode 100644 > tests/qtest/ibex-timer-test.c > > diff --git a/hw/timer/ibex_timer.c b/hw/timer/ibex_timer.c index > d8b8e4e1f6..4230d60e85 100644 > --- a/hw/timer/ibex_timer.c > +++ b/hw/timer/ibex_timer.c > @@ -52,28 +52,56 @@ REG32(INTR_STATE, 0x118) REG32(INTR_TEST, > 0x11C) > FIELD(INTR_TEST, T_0, 0, 1) > > -static uint64_t cpu_riscv_read_rtc(uint32_t timebase_freq) > +static inline uint64_t get_mtime(void *opaque) > { > - return muldiv64(qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL), > - timebase_freq, NANOSECONDS_PER_SECOND); > + IbexTimerState *s = opaque; > + return (s->timer_lower0) | ((uint64_t) s->timer_upper0 << 32); > } > > -static void ibex_timer_update_irqs(IbexTimerState *s) > +/* > + * The goal of this function is to: > + * 1. Check if the timer is enabled. If not, return false, > + * 2. Calculate the amount of time that has passed since. > + * 3. Extrapolate the number of ticks that have passed, and add it to `mtime`. > + * 4. Return true. > + */ > +static bool update_mtime(IbexTimerState *s) > { > - uint64_t value = s->timer_compare_lower0 | > - ((uint64_t)s->timer_compare_upper0 << 32); So the hardware actually implements 64 bits register (used in 32 bits CPU), why not use an union for this? Same for: + uint32_t timer_lower0; + uint32_t timer_upper0; > - uint64_t next, diff; > - uint64_t now = cpu_riscv_read_rtc(s->timebase_freq); > - > if (!(s->timer_ctrl & R_CTRL_ACTIVE_MASK)) { > - /* Timer isn't active */ > + return false; > + } > + /* Get the time then extrapolate the number of ticks that have elapsed */ > + uint64_t mtime = get_mtime(s); > + int64_t now = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL); > + int64_t elapsed = now - s->timer_last_update; > + if (elapsed < 0) { > + /* We jumped back in time. */ > + mtime -= muldiv64((uint64_t)(-elapsed), s->timebase_freq, > + NANOSECONDS_PER_SECOND); > + } else { > + mtime += muldiv64(elapsed, s->timebase_freq, > NANOSECONDS_PER_SECOND); > + } > + s->timer_lower0 = mtime & 0xffffffff; > + s->timer_upper0 = (mtime >> 32) & 0xffffffff; > + /* update last-checkpoint timestamp */ > + s->timer_last_update = now; > + return true; > +} > + > +static void ibex_timer_update_irqs(IbexTimerState *s) { > + if (!update_mtime(s)) { > + /* Timer is not enabled? */ > return; > } > + uint64_t mtimecmp = s->timer_compare_lower0 | > + ((uint64_t)s->timer_compare_upper0 << 32); > + uint64_t mtime = get_mtime(s); > > /* Update the CPUs mtimecmp */ > - s->mtimecmp = value; > + s->mtimecmp = mtimecmp; > > - if (s->mtimecmp <= now) { > + if (s->mtimecmp <= mtime) { > /* > * If the mtimecmp was in the past raise the interrupt now. > */ > @@ -84,18 +112,17 @@ static void ibex_timer_update_irqs(IbexTimerState > *s) > } > return; > } > - > - /* Setup a timer to trigger the interrupt in the future */ > + /* Update timers: setup a timer to trigger the interrupt in the > + future */ > qemu_irq_lower(s->m_timer_irq); > qemu_set_irq(s->irq, false); > - > - diff = s->mtimecmp - now; > - next = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL) + > - muldiv64(diff, > - NANOSECONDS_PER_SECOND, > - s->timebase_freq); > - > - if (next < qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL)) { > + /* Compute the difference, and set a timer for the next update. */ > + const uint64_t diff = mtimecmp - mtime; > + const int64_t now = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL); > + const uint64_t towait = muldiv64(diff, NANOSECONDS_PER_SECOND, > + s->timebase_freq); > + /* timer_mod takes in a int64_t, not uint64_t! Need to saturate it */ > + const int64_t next = now + towait; > + if (next < now) { > /* We overflowed the timer, just set it as large as we can */ > timer_mod(s->mtimer, 0x7FFFFFFFFFFFFFFF); > } else { > @@ -124,11 +151,13 @@ static void ibex_timer_reset(DeviceState *dev) > > s->timer_ctrl = 0x00000000; > s->timer_cfg0 = 0x00010000; > + s->timer_lower0 = 0x00000000; > + s->timer_upper0 = 0x00000000; > s->timer_compare_lower0 = 0xFFFFFFFF; > s->timer_compare_upper0 = 0xFFFFFFFF; > s->timer_intr_enable = 0x00000000; > s->timer_intr_state = 0x00000000; > - > + s->timer_last_update = 0x00000000; > ibex_timer_update_irqs(s); > } > > @@ -136,7 +165,6 @@ static uint64_t ibex_timer_read(void *opaque, > hwaddr addr, > unsigned int size) { > IbexTimerState *s = opaque; > - uint64_t now = cpu_riscv_read_rtc(s->timebase_freq); > uint64_t retvalue = 0; > > switch (addr >> 2) { > @@ -151,10 +179,12 @@ static uint64_t ibex_timer_read(void *opaque, > hwaddr addr, > retvalue = s->timer_cfg0; > break; > case R_LOWER0: > - retvalue = now; > + update_mtime(s); > + retvalue = s->timer_lower0; > break; > case R_UPPER0: > - retvalue = now >> 32; > + update_mtime(s); > + retvalue = s->timer_upper0; > break; > case R_COMPARE_LOWER0: > retvalue = s->timer_compare_lower0; @@ -192,17 +222,26 @@ static > void ibex_timer_write(void *opaque, hwaddr addr, > qemu_log_mask(LOG_UNIMP, "Alert triggering not supported"); > break; > case R_CTRL: > + if (!(s->timer_ctrl & R_CTRL_ACTIVE_MASK)) { > + s->timer_last_update = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL); > + } > s->timer_ctrl = val; > + /* We must update IRQs, because the QEMU timer gets updated here. > */ > + ibex_timer_update_irqs(s); > break; > case R_CFG0: > qemu_log_mask(LOG_UNIMP, "Changing prescale or step not > supported"); > s->timer_cfg0 = val; > break; > case R_LOWER0: > - qemu_log_mask(LOG_UNIMP, "Changing timer value is not supported"); > + s->timer_lower0 = val; > + s->timer_last_update = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL); > + ibex_timer_update_irqs(s); > break; > case R_UPPER0: > - qemu_log_mask(LOG_UNIMP, "Changing timer value is not supported"); > + s->timer_upper0 = val; > + s->timer_last_update = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL); > + ibex_timer_update_irqs(s); > break; > case R_COMPARE_LOWER0: > s->timer_compare_lower0 = val; > @@ -259,6 +298,9 @@ static const VMStateDescription vmstate_ibex_timer > = { > VMSTATE_UINT32(timer_compare_upper0, IbexTimerState), > VMSTATE_UINT32(timer_intr_enable, IbexTimerState), > VMSTATE_UINT32(timer_intr_state, IbexTimerState), > + VMSTATE_INT64(timer_last_update, IbexTimerState), > + VMSTATE_UINT32(timer_lower0, IbexTimerState), > + VMSTATE_UINT32(timer_upper0, IbexTimerState), > VMSTATE_END_OF_LIST() > } > }; > diff --git a/include/hw/timer/ibex_timer.h b/include/hw/timer/ibex_timer.h > index 41f5c82a92..15c16035a8 100644 > --- a/include/hw/timer/ibex_timer.h > +++ b/include/hw/timer/ibex_timer.h > @@ -36,11 +36,17 @@ struct IbexTimerState { > uint64_t mtimecmp; > QEMUTimer *mtimer; /* Internal timer for M-mode interrupt */ > > + int64_t timer_last_update; /* Used for extrapolating mtime. */ > + > /* <public> */ > MemoryRegion mmio; > > uint32_t timer_ctrl; > uint32_t timer_cfg0; > + > + > + uint32_t timer_lower0; > + uint32_t timer_upper0; > uint32_t timer_compare_lower0; > uint32_t timer_compare_upper0; > uint32_t timer_intr_enable; > diff --git a/tests/qtest/ibex-timer-test.c b/tests/qtest/ibex-timer-test.c new > file mode 100644 index 0000000000..1c2dfb0b6c > --- /dev/null > +++ b/tests/qtest/ibex-timer-test.c > @@ -0,0 +1,178 @@ > +/* > + * Testing the Ibex Timer > + * > + * Copyright (c) 2022 Rivos Inc. > + * > + * 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 "libqtest.h" > +#include "qapi/qmp/qdict.h" > + > +#define TIMER_BASE_ADDR 0x40100000UL > +#define TIMER_ADDR(addr) (TIMER_BASE_ADDR + addr) #define > +TIMER_EXPIRED_IRQ 127 #define NANOS_PER_SECOND 1000000000LL > + > +#define A_ALERT_TEST 0x0 > +#define A_CTRL 0x4 > +#define A_CFG0 0x100 > +#define A_LOWER_0 0x104 > +#define A_UPPER_0 0x108 > +#define A_COMPARE_LOWER0 0x10C > +#define A_COMPARE_UPPER0 0x110 > +#define A_INTR_ENABLE 0x114 > +#define A_INTR_STATE 0x118 > +#define A_INTR_TEST 0x11C > + > +/* > + * Tests that regs get reset properly. > + */ > +static void test_reset(void) > +{ > + QTestState *test = qtest_init("-M opentitan"); > + qtest_irq_intercept_in(test, "/machine/soc/plic"); > + /* Regs are reset; skip WO regs */ > + g_assert_cmpuint(qtest_readl(test, TIMER_ADDR(A_CTRL)), ==, 0); > + g_assert_cmpuint(qtest_readl(test, TIMER_ADDR(A_INTR_ENABLE)), ==, 0); > + g_assert_cmpuint(qtest_readl(test, TIMER_ADDR(A_INTR_STATE)), ==, 0); > + g_assert_cmpuint(qtest_readl(test, TIMER_ADDR(A_CFG0)), ==, 0x10000); > + g_assert_cmpuint(qtest_readl(test, TIMER_ADDR(A_LOWER_0)), ==, 0); > + g_assert_cmpuint(qtest_readl(test, TIMER_ADDR(A_UPPER_0)), ==, 0); > + g_assert_cmpuint(qtest_readl(test, TIMER_ADDR(A_COMPARE_LOWER0)), > ==, > + UINT32_MAX); > + g_assert_cmpuint(qtest_readl(test, TIMER_ADDR(A_COMPARE_UPPER0)), > ==, > + UINT32_MAX); > + g_assert(!qtest_get_irq(test, TIMER_EXPIRED_IRQ)); > + qtest_quit(test); > +} > + > +/* > + * Test that writes worked. > + */ > +static void test_writes(void) > +{ > + QTestState *test = qtest_init("-M opentitan"); > + > + qtest_irq_intercept_in(test, "/machine/soc/plic"); > + > + /* Special regs that may/may not need to be tested yet */ > +/* > + g_assert_cmpuint(qtest_readl(test, TIMER_ADDR(A_ALERT_TEST)), ==, 0); > + g_assert_cmpuint(qtest_readl(test, TIMER_ADDR(A_INTR_TEST)), ==, 0); > + g_assert_cmpuint(qtest_readl(test, TIMER_ADDR(A_INTR_STATE)), ==, > +0); */ > + /* Write to ctrl */ > + qtest_writel(test, TIMER_ADDR(A_CTRL), 0x1); > + g_assert_cmpuint(qtest_readl(test, TIMER_ADDR(A_CTRL)), ==, 0x1); > + /* Write to intr_enable */ > + qtest_writel(test, TIMER_ADDR(A_INTR_ENABLE), 0x1); > + g_assert_cmpuint(qtest_readl(test, TIMER_ADDR(A_INTR_ENABLE)), ==, > +0x1); > + > + /* Writes to config? Though none of it is supported */ > + qtest_writel(test, TIMER_ADDR(A_CFG0), 0x20001); > + g_assert_cmpuint(qtest_readl(test, TIMER_ADDR(A_CFG0)), ==, > + 0x20001); > + > + /* Writes to mtime */ > + qtest_writel(test, TIMER_ADDR(A_LOWER_0), 0xdeaddad); > + g_assert_cmpuint(qtest_readl(test, TIMER_ADDR(A_LOWER_0)), ==, > 0xdeaddad); > + qtest_writel(test, TIMER_ADDR(A_UPPER_0), 0xdeaddad); > + g_assert_cmpuint(qtest_readl(test, TIMER_ADDR(A_UPPER_0)), ==, > + 0xdeaddad); > + > + /* Writes to mtimecmp */ > + qtest_writel(test, TIMER_ADDR(A_COMPARE_LOWER0), 0xdeaddad); > + g_assert_cmpuint(qtest_readl(test, TIMER_ADDR(A_COMPARE_LOWER0)), > ==, > + 0xdeaddad); > + qtest_writel(test, TIMER_ADDR(A_COMPARE_UPPER0), 0xdeaddad); > + g_assert_cmpuint(qtest_readl(test, TIMER_ADDR(A_COMPARE_UPPER0)), > ==, > + 0xdeaddad); > + qtest_quit(test); > +} > + > +/* > + * Test the standard operation of the timer. > + */ > +static void test_operation(void) > +{ > + /* A frequency of 1000000 Hz*/ > + QTestState *test = qtest_init( > + "-M opentitan " > + "-global driver=ibex-timer,property=timebase-freq,value=1000000" > + ); > + qtest_irq_intercept_in(test, "/machine/soc/plic"); > + /* Set mtimecmp; approx 1 second. */ > + qtest_writel(test, TIMER_ADDR(A_COMPARE_LOWER0), 1000000); > + qtest_writel(test, TIMER_ADDR(A_COMPARE_UPPER0), 0); > + > + /* Wait for some amount of time. Timer should not tick. */ > + qtest_clock_step(test, NANOS_PER_SECOND * 30); > + g_assert_cmpuint(qtest_readl(test, TIMER_ADDR(A_LOWER_0)), ==, 0); > + g_assert_cmpuint(qtest_readl(test, TIMER_ADDR(A_UPPER_0)), ==, 0); > + > + /* First, enable the timer. */ > + qtest_writel(test, TIMER_ADDR(A_CTRL), 0x1); > + qtest_writel(test, TIMER_ADDR(A_INTR_ENABLE), 0x1); > + > + /* The counter should remain at 0, and no interrupts. */ > + g_assert_cmpuint(qtest_readl(test, TIMER_ADDR(A_LOWER_0)), ==, 0); > + g_assert_cmpuint(qtest_readl(test, TIMER_ADDR(A_UPPER_0)), ==, 0); > + g_assert(!qtest_get_irq(test, TIMER_EXPIRED_IRQ)); > + > + /* Let it run for half a second. No interrupts. */ > + qtest_clock_step(test, NANOS_PER_SECOND / 2); > + g_assert_cmpuint(qtest_readl(test, TIMER_ADDR(A_LOWER_0)), <, > + qtest_readl(test, TIMER_ADDR(A_COMPARE_LOWER0))); > + g_assert_cmpuint(qtest_readl(test, TIMER_ADDR(A_UPPER_0)), <=, > + qtest_readl(test, TIMER_ADDR(A_COMPARE_UPPER0))); > + g_assert(!qtest_get_irq(test, TIMER_EXPIRED_IRQ)); > + > + /* Let it run for half a second again. Interrupt. */ > + qtest_clock_step(test, NANOS_PER_SECOND / 2); > + g_assert_cmpuint(qtest_readl(test, TIMER_ADDR(A_LOWER_0)), ==, > + qtest_readl(test, TIMER_ADDR(A_COMPARE_LOWER0))); > + g_assert_cmpuint(qtest_readl(test, TIMER_ADDR(A_UPPER_0)), ==, > + qtest_readl(test, TIMER_ADDR(A_COMPARE_UPPER0))); > + g_assert(qtest_get_irq(test, TIMER_EXPIRED_IRQ)); > + > + /* Handle interrupt, no more interrupt after. */ > + qtest_writel(test, TIMER_ADDR(A_LOWER_0), 0x0); > + qtest_writel(test, TIMER_ADDR(A_UPPER_0), 0x0); > + qtest_writel(test, TIMER_ADDR(A_INTR_STATE), 0x1); > + g_assert(!qtest_get_irq(test, TIMER_EXPIRED_IRQ)); > + > + /* One more second. */ > + qtest_clock_step(test, NANOS_PER_SECOND); > + g_assert_cmpuint(qtest_readl(test, TIMER_ADDR(A_LOWER_0)), ==, > + qtest_readl(test, TIMER_ADDR(A_COMPARE_LOWER0))); > + g_assert_cmpuint(qtest_readl(test, TIMER_ADDR(A_UPPER_0)), ==, > + qtest_readl(test, TIMER_ADDR(A_COMPARE_UPPER0))); > + g_assert(qtest_get_irq(test, TIMER_EXPIRED_IRQ)); > + > + qtest_quit(test); > +} > + > +int main(int argc, char **argv) > +{ > + g_test_init(&argc, &argv, NULL); > + qtest_add_func("ibex-timer/reset", test_reset); > + qtest_add_func("ibex-timer/writes", test_writes); > + qtest_add_func("ibex-timer/op", test_operation); > + return g_test_run(); > +} > diff --git a/tests/qtest/meson.build b/tests/qtest/meson.build index > fb63b8d3fa..7a769a79c5 100644 > --- a/tests/qtest/meson.build > +++ b/tests/qtest/meson.build > @@ -235,7 +235,8 @@ qtests_s390x = \ > 'migration-test'] > > qtests_riscv32 = \ > - ['ibex-aon-timer-test'] > + ['ibex-aon-timer-test', > + 'ibex-timer-test'] > > qos_test_ss = ss.source_set() > qos_test_ss.add( > -- > 2.34.1
On Thu, Sep 22, 2022 at 2:45 PM Philippe Mathieu-Daudé <f4bug@amsat.org> wrote: > On 22/9/22 17:58, Tyler Ng wrote: > > 1. Adds fields to hold the value of mtime in timer_upper0 and > timer_lower0. > > > > 2. Changes the read and write functions to use the mtime fields. > > > > 3. Updates the value of mtime in update_mtime() by extrapolating the > > time elapsed. This will need to change if/when the prescalar is > > implemented. > > > > 4. Adds a qtest for the ibex timer. > > > > Signed-off-by: Tyler Ng <tkng@rivosinc.com> > > --- > > hw/timer/ibex_timer.c | 98 +++++++++++++------ > > include/hw/timer/ibex_timer.h | 6 ++ > > tests/qtest/ibex-timer-test.c | 178 ++++++++++++++++++++++++++++++++++ > > tests/qtest/meson.build | 3 +- > > 4 files changed, 256 insertions(+), 29 deletions(-) > > create mode 100644 tests/qtest/ibex-timer-test.c > > > -static void ibex_timer_update_irqs(IbexTimerState *s) > > +/* > > + * The goal of this function is to: > > + * 1. Check if the timer is enabled. If not, return false, > > + * 2. Calculate the amount of time that has passed since. > > + * 3. Extrapolate the number of ticks that have passed, and add it to > `mtime`. > > + * 4. Return true. > > + */ > > +static bool update_mtime(IbexTimerState *s) > > { > > - uint64_t value = s->timer_compare_lower0 | > > - ((uint64_t)s->timer_compare_upper0 << 32); > > - uint64_t next, diff; > > - uint64_t now = cpu_riscv_read_rtc(s->timebase_freq); > > - > > if (!(s->timer_ctrl & R_CTRL_ACTIVE_MASK)) { > > - /* Timer isn't active */ > > + return false; > > + } > > + /* Get the time then extrapolate the number of ticks that have > elapsed */ > > + uint64_t mtime = get_mtime(s); > > + int64_t now = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL); > > + int64_t elapsed = now - s->timer_last_update; > > + if (elapsed < 0) { > > + /* We jumped back in time. */ > > + mtime -= muldiv64((uint64_t)(-elapsed), s->timebase_freq, > > + NANOSECONDS_PER_SECOND); > > + } else { > > + mtime += muldiv64(elapsed, s->timebase_freq, > NANOSECONDS_PER_SECOND); > > + } > > + s->timer_lower0 = mtime & 0xffffffff; > > + s->timer_upper0 = (mtime >> 32) & 0xffffffff; > > Could use extract64(mtime, 0, 32) and extract64(mtime, 32, 32); > > Didn't know about this function, thanks. > > + /* update last-checkpoint timestamp */ > > + s->timer_last_update = now; > > + return true; > > +} >
Hi Eddie, On Mon, Sep 26, 2022 at 2:06 PM Dong, Eddie <eddie.dong@intel.com> wrote: > > > > -----Original Message----- > > From: Qemu-devel <qemu-devel-bounces+eddie.dong=intel.com@nongnu.org> > > On Behalf Of Tyler Ng > > Sent: Thursday, September 22, 2022 8:59 AM > > To: open list:RISC-V <qemu-riscv@nongnu.org>; qemu-devel@nongnu.org > > Developers <qemu-devel@nongnu.org> > > Cc: Alistair Francis <Alistair.Francis@wdc.com>; Meng, Bin > > <bin.meng@windriver.com>; Thomas Huth <thuth@redhat.com>; Paolo > > Bonzini <pbonzini@redhat.com>; Palmer Dabbelt <palmer@dabbelt.com>; > > Laurent Vivier <lvivier@redhat.com> > > Subject: [PATCH v2 3/3] hw/timer: ibex_timer.c: Add support for writes to > > mtime > > > > 1. Adds fields to hold the value of mtime in timer_upper0 and > timer_lower0. > > > > 2. Changes the read and write functions to use the mtime fields. > > > > 3. Updates the value of mtime in update_mtime() by extrapolating the time > > elapsed. This will need to change if/when the prescalar is implemented. > > > > 4. Adds a qtest for the ibex timer. > > > > Signed-off-by: Tyler Ng <tkng@rivosinc.com> > > --- > > hw/timer/ibex_timer.c | 98 +++++++++++++------ > > include/hw/timer/ibex_timer.h | 6 ++ > > tests/qtest/ibex-timer-test.c | 178 ++++++++++++++++++++++++++++++++++ > > tests/qtest/meson.build | 3 +- > > 4 files changed, 256 insertions(+), 29 deletions(-) create mode 100644 > > tests/qtest/ibex-timer-test.c > > > > diff --git a/hw/timer/ibex_timer.c b/hw/timer/ibex_timer.c index > > d8b8e4e1f6..4230d60e85 100644 > > --- a/hw/timer/ibex_timer.c > > +++ b/hw/timer/ibex_timer.c > > @@ -52,28 +52,56 @@ REG32(INTR_STATE, 0x118) REG32(INTR_TEST, > > 0x11C) > > FIELD(INTR_TEST, T_0, 0, 1) > > > > -static uint64_t cpu_riscv_read_rtc(uint32_t timebase_freq) > > +static inline uint64_t get_mtime(void *opaque) > > { > > - return muldiv64(qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL), > > - timebase_freq, NANOSECONDS_PER_SECOND); > > + IbexTimerState *s = opaque; > > + return (s->timer_lower0) | ((uint64_t) s->timer_upper0 << 32); > > } > > > > -static void ibex_timer_update_irqs(IbexTimerState *s) > > +/* > > + * The goal of this function is to: > > + * 1. Check if the timer is enabled. If not, return false, > > + * 2. Calculate the amount of time that has passed since. > > + * 3. Extrapolate the number of ticks that have passed, and add it to > `mtime`. > > + * 4. Return true. > > + */ > > +static bool update_mtime(IbexTimerState *s) > > { > > - uint64_t value = s->timer_compare_lower0 | > > - ((uint64_t)s->timer_compare_upper0 << 32); > So the hardware actually implements 64 bits register (used in 32 bits > CPU), why not use an union for this? > Same for: > + uint32_t timer_lower0; > + uint32_t timer_upper0; > > I'm not too sure what a C union would do for us here? > > - uint64_t next, diff; > > - uint64_t now = cpu_riscv_read_rtc(s->timebase_freq); > > - > > if (!(s->timer_ctrl & R_CTRL_ACTIVE_MASK)) { > > - /* Timer isn't active */ > > + return false; > > + } > > + /* Get the time then extrapolate the number of ticks that have > elapsed */ > > + uint64_t mtime = get_mtime(s); > > + int64_t now = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL); > > + int64_t elapsed = now - s->timer_last_update; > > + if (elapsed < 0) { > > + /* We jumped back in time. */ > > + mtime -= muldiv64((uint64_t)(-elapsed), s->timebase_freq, > > + NANOSECONDS_PER_SECOND); > > + } else { > > + mtime += muldiv64(elapsed, s->timebase_freq, > > NANOSECONDS_PER_SECOND); > > + } > > + s->timer_lower0 = mtime & 0xffffffff; > > + s->timer_upper0 = (mtime >> 32) & 0xffffffff; > > + /* update last-checkpoint timestamp */ > > + s->timer_last_update = now; > > + return true; > > +} > > + > > +static void ibex_timer_update_irqs(IbexTimerState *s) { > > + if (!update_mtime(s)) { > > + /* Timer is not enabled? */ > > return; > > } > > + uint64_t mtimecmp = s->timer_compare_lower0 | > > + ((uint64_t)s->timer_compare_upper0 << 32); > > + uint64_t mtime = get_mtime(s); > > > > /* Update the CPUs mtimecmp */ > > - s->mtimecmp = value; > > + s->mtimecmp = mtimecmp; > > > > - if (s->mtimecmp <= now) { > > + if (s->mtimecmp <= mtime) { > > /* > > * If the mtimecmp was in the past raise the interrupt now. > > */ > > @@ -84,18 +112,17 @@ static void ibex_timer_update_irqs(IbexTimerState > > *s) > > } > > return; > > } > > - > > - /* Setup a timer to trigger the interrupt in the future */ > > + /* Update timers: setup a timer to trigger the interrupt in the > > + future */ > > qemu_irq_lower(s->m_timer_irq); > > qemu_set_irq(s->irq, false); > > - > > - diff = s->mtimecmp - now; > > - next = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL) + > > - muldiv64(diff, > > - NANOSECONDS_PER_SECOND, > > - s->timebase_freq); > > - > > - if (next < qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL)) { > > + /* Compute the difference, and set a timer for the next update. */ > > + const uint64_t diff = mtimecmp - mtime; > > + const int64_t now = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL); > > + const uint64_t towait = muldiv64(diff, NANOSECONDS_PER_SECOND, > > + s->timebase_freq); > > + /* timer_mod takes in a int64_t, not uint64_t! Need to saturate it > */ > > + const int64_t next = now + towait; > > + if (next < now) { > > /* We overflowed the timer, just set it as large as we can */ > > timer_mod(s->mtimer, 0x7FFFFFFFFFFFFFFF); > > } else { > > @@ -124,11 +151,13 @@ static void ibex_timer_reset(DeviceState *dev) > > > > s->timer_ctrl = 0x00000000; > > s->timer_cfg0 = 0x00010000; > > + s->timer_lower0 = 0x00000000; > > + s->timer_upper0 = 0x00000000; > > s->timer_compare_lower0 = 0xFFFFFFFF; > > s->timer_compare_upper0 = 0xFFFFFFFF; > > s->timer_intr_enable = 0x00000000; > > s->timer_intr_state = 0x00000000; > > - > > + s->timer_last_update = 0x00000000; > > ibex_timer_update_irqs(s); > > } > > > > @@ -136,7 +165,6 @@ static uint64_t ibex_timer_read(void *opaque, > > hwaddr addr, > > unsigned int size) { > > IbexTimerState *s = opaque; > > - uint64_t now = cpu_riscv_read_rtc(s->timebase_freq); > > uint64_t retvalue = 0; > > > > switch (addr >> 2) { > > @@ -151,10 +179,12 @@ static uint64_t ibex_timer_read(void *opaque, > > hwaddr addr, > > retvalue = s->timer_cfg0; > > break; > > case R_LOWER0: > > - retvalue = now; > > + update_mtime(s); > > + retvalue = s->timer_lower0; > > break; > > case R_UPPER0: > > - retvalue = now >> 32; > > + update_mtime(s); > > + retvalue = s->timer_upper0; > > break; > > case R_COMPARE_LOWER0: > > retvalue = s->timer_compare_lower0; @@ -192,17 +222,26 @@ static > > void ibex_timer_write(void *opaque, hwaddr addr, > > qemu_log_mask(LOG_UNIMP, "Alert triggering not supported"); > > break; > > case R_CTRL: > > + if (!(s->timer_ctrl & R_CTRL_ACTIVE_MASK)) { > > + s->timer_last_update = > qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL); > > + } > > s->timer_ctrl = val; > > + /* We must update IRQs, because the QEMU timer gets updated > here. > > */ > > + ibex_timer_update_irqs(s); > > break; > > case R_CFG0: > > qemu_log_mask(LOG_UNIMP, "Changing prescale or step not > > supported"); > > s->timer_cfg0 = val; > > break; > > case R_LOWER0: > > - qemu_log_mask(LOG_UNIMP, "Changing timer value is not > supported"); > > + s->timer_lower0 = val; > > + s->timer_last_update = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL); > > + ibex_timer_update_irqs(s); > > break; > > case R_UPPER0: > > - qemu_log_mask(LOG_UNIMP, "Changing timer value is not > supported"); > > + s->timer_upper0 = val; > > + s->timer_last_update = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL); > > + ibex_timer_update_irqs(s); > > break; > > case R_COMPARE_LOWER0: > > s->timer_compare_lower0 = val; > > @@ -259,6 +298,9 @@ static const VMStateDescription vmstate_ibex_timer > > = { > > VMSTATE_UINT32(timer_compare_upper0, IbexTimerState), > > VMSTATE_UINT32(timer_intr_enable, IbexTimerState), > > VMSTATE_UINT32(timer_intr_state, IbexTimerState), > > + VMSTATE_INT64(timer_last_update, IbexTimerState), > > + VMSTATE_UINT32(timer_lower0, IbexTimerState), > > + VMSTATE_UINT32(timer_upper0, IbexTimerState), > > VMSTATE_END_OF_LIST() > > } > > }; > > diff --git a/include/hw/timer/ibex_timer.h > b/include/hw/timer/ibex_timer.h > > index 41f5c82a92..15c16035a8 100644 > > --- a/include/hw/timer/ibex_timer.h > > +++ b/include/hw/timer/ibex_timer.h > > @@ -36,11 +36,17 @@ struct IbexTimerState { > > uint64_t mtimecmp; > > QEMUTimer *mtimer; /* Internal timer for M-mode interrupt */ > > > > + int64_t timer_last_update; /* Used for extrapolating mtime. */ > > + > > /* <public> */ > > MemoryRegion mmio; > > > > uint32_t timer_ctrl; > > uint32_t timer_cfg0; > > + > > + > > + uint32_t timer_lower0; > > + uint32_t timer_upper0; > > uint32_t timer_compare_lower0; > > uint32_t timer_compare_upper0; > > uint32_t timer_intr_enable; > > diff --git a/tests/qtest/ibex-timer-test.c > b/tests/qtest/ibex-timer-test.c new > > file mode 100644 index 0000000000..1c2dfb0b6c > > --- /dev/null > > +++ b/tests/qtest/ibex-timer-test.c > > @@ -0,0 +1,178 @@ > > +/* > > + * Testing the Ibex Timer > > + * > > + * Copyright (c) 2022 Rivos Inc. > > + * > > + * 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 "libqtest.h" > > +#include "qapi/qmp/qdict.h" > > + > > +#define TIMER_BASE_ADDR 0x40100000UL > > +#define TIMER_ADDR(addr) (TIMER_BASE_ADDR + addr) #define > > +TIMER_EXPIRED_IRQ 127 #define NANOS_PER_SECOND 1000000000LL > > + > > +#define A_ALERT_TEST 0x0 > > +#define A_CTRL 0x4 > > +#define A_CFG0 0x100 > > +#define A_LOWER_0 0x104 > > +#define A_UPPER_0 0x108 > > +#define A_COMPARE_LOWER0 0x10C > > +#define A_COMPARE_UPPER0 0x110 > > +#define A_INTR_ENABLE 0x114 > > +#define A_INTR_STATE 0x118 > > +#define A_INTR_TEST 0x11C > > + > > +/* > > + * Tests that regs get reset properly. > > + */ > > +static void test_reset(void) > > +{ > > + QTestState *test = qtest_init("-M opentitan"); > > + qtest_irq_intercept_in(test, "/machine/soc/plic"); > > + /* Regs are reset; skip WO regs */ > > + g_assert_cmpuint(qtest_readl(test, TIMER_ADDR(A_CTRL)), ==, 0); > > + g_assert_cmpuint(qtest_readl(test, TIMER_ADDR(A_INTR_ENABLE)), ==, > 0); > > + g_assert_cmpuint(qtest_readl(test, TIMER_ADDR(A_INTR_STATE)), ==, > 0); > > + g_assert_cmpuint(qtest_readl(test, TIMER_ADDR(A_CFG0)), ==, > 0x10000); > > + g_assert_cmpuint(qtest_readl(test, TIMER_ADDR(A_LOWER_0)), ==, 0); > > + g_assert_cmpuint(qtest_readl(test, TIMER_ADDR(A_UPPER_0)), ==, 0); > > + g_assert_cmpuint(qtest_readl(test, TIMER_ADDR(A_COMPARE_LOWER0)), > > ==, > > + UINT32_MAX); > > + g_assert_cmpuint(qtest_readl(test, TIMER_ADDR(A_COMPARE_UPPER0)), > > ==, > > + UINT32_MAX); > > + g_assert(!qtest_get_irq(test, TIMER_EXPIRED_IRQ)); > > + qtest_quit(test); > > +} > > + > > +/* > > + * Test that writes worked. > > + */ > > +static void test_writes(void) > > +{ > > + QTestState *test = qtest_init("-M opentitan"); > > + > > + qtest_irq_intercept_in(test, "/machine/soc/plic"); > > + > > + /* Special regs that may/may not need to be tested yet */ > > +/* > > + g_assert_cmpuint(qtest_readl(test, TIMER_ADDR(A_ALERT_TEST)), ==, > 0); > > + g_assert_cmpuint(qtest_readl(test, TIMER_ADDR(A_INTR_TEST)), ==, 0); > > + g_assert_cmpuint(qtest_readl(test, TIMER_ADDR(A_INTR_STATE)), ==, > > +0); */ > > + /* Write to ctrl */ > > + qtest_writel(test, TIMER_ADDR(A_CTRL), 0x1); > > + g_assert_cmpuint(qtest_readl(test, TIMER_ADDR(A_CTRL)), ==, 0x1); > > + /* Write to intr_enable */ > > + qtest_writel(test, TIMER_ADDR(A_INTR_ENABLE), 0x1); > > + g_assert_cmpuint(qtest_readl(test, TIMER_ADDR(A_INTR_ENABLE)), ==, > > +0x1); > > + > > + /* Writes to config? Though none of it is supported */ > > + qtest_writel(test, TIMER_ADDR(A_CFG0), 0x20001); > > + g_assert_cmpuint(qtest_readl(test, TIMER_ADDR(A_CFG0)), ==, > > + 0x20001); > > + > > + /* Writes to mtime */ > > + qtest_writel(test, TIMER_ADDR(A_LOWER_0), 0xdeaddad); > > + g_assert_cmpuint(qtest_readl(test, TIMER_ADDR(A_LOWER_0)), ==, > > 0xdeaddad); > > + qtest_writel(test, TIMER_ADDR(A_UPPER_0), 0xdeaddad); > > + g_assert_cmpuint(qtest_readl(test, TIMER_ADDR(A_UPPER_0)), ==, > > + 0xdeaddad); > > + > > + /* Writes to mtimecmp */ > > + qtest_writel(test, TIMER_ADDR(A_COMPARE_LOWER0), 0xdeaddad); > > + g_assert_cmpuint(qtest_readl(test, TIMER_ADDR(A_COMPARE_LOWER0)), > > ==, > > + 0xdeaddad); > > + qtest_writel(test, TIMER_ADDR(A_COMPARE_UPPER0), 0xdeaddad); > > + g_assert_cmpuint(qtest_readl(test, TIMER_ADDR(A_COMPARE_UPPER0)), > > ==, > > + 0xdeaddad); > > + qtest_quit(test); > > +} > > + > > +/* > > + * Test the standard operation of the timer. > > + */ > > +static void test_operation(void) > > +{ > > + /* A frequency of 1000000 Hz*/ > > + QTestState *test = qtest_init( > > + "-M opentitan " > > + "-global driver=ibex-timer,property=timebase-freq,value=1000000" > > + ); > > + qtest_irq_intercept_in(test, "/machine/soc/plic"); > > + /* Set mtimecmp; approx 1 second. */ > > + qtest_writel(test, TIMER_ADDR(A_COMPARE_LOWER0), 1000000); > > + qtest_writel(test, TIMER_ADDR(A_COMPARE_UPPER0), 0); > > + > > + /* Wait for some amount of time. Timer should not tick. */ > > + qtest_clock_step(test, NANOS_PER_SECOND * 30); > > + g_assert_cmpuint(qtest_readl(test, TIMER_ADDR(A_LOWER_0)), ==, 0); > > + g_assert_cmpuint(qtest_readl(test, TIMER_ADDR(A_UPPER_0)), ==, 0); > > + > > + /* First, enable the timer. */ > > + qtest_writel(test, TIMER_ADDR(A_CTRL), 0x1); > > + qtest_writel(test, TIMER_ADDR(A_INTR_ENABLE), 0x1); > > + > > + /* The counter should remain at 0, and no interrupts. */ > > + g_assert_cmpuint(qtest_readl(test, TIMER_ADDR(A_LOWER_0)), ==, 0); > > + g_assert_cmpuint(qtest_readl(test, TIMER_ADDR(A_UPPER_0)), ==, 0); > > + g_assert(!qtest_get_irq(test, TIMER_EXPIRED_IRQ)); > > + > > + /* Let it run for half a second. No interrupts. */ > > + qtest_clock_step(test, NANOS_PER_SECOND / 2); > > + g_assert_cmpuint(qtest_readl(test, TIMER_ADDR(A_LOWER_0)), <, > > + qtest_readl(test, TIMER_ADDR(A_COMPARE_LOWER0))); > > + g_assert_cmpuint(qtest_readl(test, TIMER_ADDR(A_UPPER_0)), <=, > > + qtest_readl(test, TIMER_ADDR(A_COMPARE_UPPER0))); > > + g_assert(!qtest_get_irq(test, TIMER_EXPIRED_IRQ)); > > + > > + /* Let it run for half a second again. Interrupt. */ > > + qtest_clock_step(test, NANOS_PER_SECOND / 2); > > + g_assert_cmpuint(qtest_readl(test, TIMER_ADDR(A_LOWER_0)), ==, > > + qtest_readl(test, TIMER_ADDR(A_COMPARE_LOWER0))); > > + g_assert_cmpuint(qtest_readl(test, TIMER_ADDR(A_UPPER_0)), ==, > > + qtest_readl(test, TIMER_ADDR(A_COMPARE_UPPER0))); > > + g_assert(qtest_get_irq(test, TIMER_EXPIRED_IRQ)); > > + > > + /* Handle interrupt, no more interrupt after. */ > > + qtest_writel(test, TIMER_ADDR(A_LOWER_0), 0x0); > > + qtest_writel(test, TIMER_ADDR(A_UPPER_0), 0x0); > > + qtest_writel(test, TIMER_ADDR(A_INTR_STATE), 0x1); > > + g_assert(!qtest_get_irq(test, TIMER_EXPIRED_IRQ)); > > + > > + /* One more second. */ > > + qtest_clock_step(test, NANOS_PER_SECOND); > > + g_assert_cmpuint(qtest_readl(test, TIMER_ADDR(A_LOWER_0)), ==, > > + qtest_readl(test, TIMER_ADDR(A_COMPARE_LOWER0))); > > + g_assert_cmpuint(qtest_readl(test, TIMER_ADDR(A_UPPER_0)), ==, > > + qtest_readl(test, TIMER_ADDR(A_COMPARE_UPPER0))); > > + g_assert(qtest_get_irq(test, TIMER_EXPIRED_IRQ)); > > + > > + qtest_quit(test); > > +} > > + > > +int main(int argc, char **argv) > > +{ > > + g_test_init(&argc, &argv, NULL); > > + qtest_add_func("ibex-timer/reset", test_reset); > > + qtest_add_func("ibex-timer/writes", test_writes); > > + qtest_add_func("ibex-timer/op", test_operation); > > + return g_test_run(); > > +} > > diff --git a/tests/qtest/meson.build b/tests/qtest/meson.build index > > fb63b8d3fa..7a769a79c5 100644 > > --- a/tests/qtest/meson.build > > +++ b/tests/qtest/meson.build > > @@ -235,7 +235,8 @@ qtests_s390x = \ > > 'migration-test'] > > > > qtests_riscv32 = \ > > - ['ibex-aon-timer-test'] > > + ['ibex-aon-timer-test', > > + 'ibex-timer-test'] > > > > qos_test_ss = ss.source_set() > > qos_test_ss.add( > > -- > > 2.34.1 > > -Tyler
From: Tyler Ng <tkng@rivosinc.com> Sent: Monday, September 26, 2022 4:38 PM To: Dong, Eddie <eddie.dong@intel.com> Cc: open list:RISC-V <qemu-riscv@nongnu.org>; qemu-devel@nongnu.org Developers <qemu-devel@nongnu.org>; Alistair Francis <Alistair.Francis@wdc.com>; Meng, Bin <bin.meng@windriver.com>; Thomas Huth <thuth@redhat.com>; Paolo Bonzini <pbonzini@redhat.com>; Palmer Dabbelt <palmer@dabbelt.com>; Laurent Vivier <lvivier@redhat.com> Subject: Re: [PATCH v2 3/3] hw/timer: ibex_timer.c: Add support for writes to mtime Hi Eddie, On Mon, Sep 26, 2022 at 2:06 PM Dong, Eddie <mailto:eddie.dong@intel.com> wrote: > -----Original Message----- > From: Qemu-devel <qemu-devel-bounces+eddie.dong=mailto:intel.com@nongnu.org> > On Behalf Of Tyler Ng > Sent: Thursday, September 22, 2022 8:59 AM > To: open list:RISC-V <mailto:qemu-riscv@nongnu.org>; mailto:qemu-devel@nongnu.org > Developers <mailto:qemu-devel@nongnu.org> > Cc: Alistair Francis <mailto:Alistair.Francis@wdc.com>; Meng, Bin > <mailto:bin.meng@windriver.com>; Thomas Huth <mailto:thuth@redhat.com>; Paolo > Bonzini <mailto:pbonzini@redhat.com>; Palmer Dabbelt <mailto:palmer@dabbelt.com>; > Laurent Vivier <mailto:lvivier@redhat.com> > Subject: [PATCH v2 3/3] hw/timer: ibex_timer.c: Add support for writes to > mtime > > 1. Adds fields to hold the value of mtime in timer_upper0 and timer_lower0. > > 2. Changes the read and write functions to use the mtime fields. > > 3. Updates the value of mtime in update_mtime() by extrapolating the time > elapsed. This will need to change if/when the prescalar is implemented. > > 4. Adds a qtest for the ibex timer. > > Signed-off-by: Tyler Ng <mailto:tkng@rivosinc.com> > --- > hw/timer/ibex_timer.c | 98 +++++++++++++------ > include/hw/timer/ibex_timer.h | 6 ++ > tests/qtest/ibex-timer-test.c | 178 ++++++++++++++++++++++++++++++++++ > tests/qtest/meson.build | 3 +- > 4 files changed, 256 insertions(+), 29 deletions(-) create mode 100644 > tests/qtest/ibex-timer-test.c > > diff --git a/hw/timer/ibex_timer.c b/hw/timer/ibex_timer.c index > d8b8e4e1f6..4230d60e85 100644 > --- a/hw/timer/ibex_timer.c > +++ b/hw/timer/ibex_timer.c > @@ -52,28 +52,56 @@ REG32(INTR_STATE, 0x118) REG32(INTR_TEST, > 0x11C) > FIELD(INTR_TEST, T_0, 0, 1) > > -static uint64_t cpu_riscv_read_rtc(uint32_t timebase_freq) > +static inline uint64_t get_mtime(void *opaque) > { > - return muldiv64(qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL), > - timebase_freq, NANOSECONDS_PER_SECOND); > + IbexTimerState *s = opaque; > + return (s->timer_lower0) | ((uint64_t) s->timer_upper0 << 32); > } > > -static void ibex_timer_update_irqs(IbexTimerState *s) > +/* > + * The goal of this function is to: > + * 1. Check if the timer is enabled. If not, return false, > + * 2. Calculate the amount of time that has passed since. > + * 3. Extrapolate the number of ticks that have passed, and add it to `mtime`. > + * 4. Return true. > + */ > +static bool update_mtime(IbexTimerState *s) > { > - uint64_t value = s->timer_compare_lower0 | > - ((uint64_t)s->timer_compare_upper0 << 32); So the hardware actually implements 64 bits register (used in 32 bits CPU), why not use an union for this? Same for: + uint32_t timer_lower0; + uint32_t timer_upper0; I'm not too sure what a C union would do for us here? I think what the hardware really implement is a 64 bits register, with 32 bits interface to access. struct IbexTimerState actually defines both of them: uint64_t mtimecmp; uint32_t timer_compare_lower0; uint32_t timer_compare_upper0; Using a union can better reflect this. Also, it can avoid the convert from 2 32-bits register to 64 bits, like the above code does. ibex_timer_update_irqs() also does this conversion. > - uint64_t next, diff; > - uint64_t now = cpu_riscv_read_rtc(s->timebase_freq); > - > if (!(s->timer_ctrl & R_CTRL_ACTIVE_MASK)) { > - /* Timer isn't active */ > + return false; > + } > + /* Get the time then extrapolate the number of ticks that have elapsed */ > + uint64_t mtime = get_mtime(s); > + int64_t now = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL); > + int64_t elapsed = now - s->timer_last_update; > + if (elapsed < 0) { > + /* We jumped back in time. */ > + mtime -= muldiv64((uint64_t)(-elapsed), s->timebase_freq, > + NANOSECONDS_PER_SECOND); > + } else { > + mtime += muldiv64(elapsed, s->timebase_freq, > NANOSECONDS_PER_SECOND); > + } > + s->timer_lower0 = mtime & 0xffffffff; > + s->timer_upper0 = (mtime >> 32) & 0xffffffff; > + /* update last-checkpoint timestamp */ > + s->timer_last_update = now; > + return true; > +} > + > +static void ibex_timer_update_irqs(IbexTimerState *s) { > + if (!update_mtime(s)) { > + /* Timer is not enabled? */ > return; > } > + uint64_t mtimecmp = s->timer_compare_lower0 | > + ((uint64_t)s->timer_compare_upper0 << 32); > + uint64_t mtime = get_mtime(s); > > /* Update the CPUs mtimecmp */ > - s->mtimecmp = value; > + s->mtimecmp = mtimecmp; > > - if (s->mtimecmp <= now) { > + if (s->mtimecmp <= mtime) { > /* > * If the mtimecmp was in the past raise the interrupt now. > */ > @@ -84,18 +112,17 @@ static void ibex_timer_update_irqs(IbexTimerState > *s) > } > return; > } > - > - /* Setup a timer to trigger the interrupt in the future */ > + /* Update timers: setup a timer to trigger the interrupt in the > + future */ > qemu_irq_lower(s->m_timer_irq); > qemu_set_irq(s->irq, false); > - > - diff = s->mtimecmp - now; > - next = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL) + > - muldiv64(diff, > - NANOSECONDS_PER_SECOND, > - s->timebase_freq); > - > - if (next < qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL)) { > + /* Compute the difference, and set a timer for the next update. */ > + const uint64_t diff = mtimecmp - mtime; > + const int64_t now = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL); > + const uint64_t towait = muldiv64(diff, NANOSECONDS_PER_SECOND, > + s->timebase_freq); > + /* timer_mod takes in a int64_t, not uint64_t! Need to saturate it */ > + const int64_t next = now + towait; > + if (next < now) { > /* We overflowed the timer, just set it as large as we can */ > timer_mod(s->mtimer, 0x7FFFFFFFFFFFFFFF); > } else { > @@ -124,11 +151,13 @@ static void ibex_timer_reset(DeviceState *dev) > > s->timer_ctrl = 0x00000000; > s->timer_cfg0 = 0x00010000; > + s->timer_lower0 = 0x00000000; > + s->timer_upper0 = 0x00000000; > s->timer_compare_lower0 = 0xFFFFFFFF; > s->timer_compare_upper0 = 0xFFFFFFFF; > s->timer_intr_enable = 0x00000000; > s->timer_intr_state = 0x00000000; > - > + s->timer_last_update = 0x00000000; > ibex_timer_update_irqs(s); > } > > @@ -136,7 +165,6 @@ static uint64_t ibex_timer_read(void *opaque, > hwaddr addr, > unsigned int size) { > IbexTimerState *s = opaque; > - uint64_t now = cpu_riscv_read_rtc(s->timebase_freq); > uint64_t retvalue = 0; > > switch (addr >> 2) { > @@ -151,10 +179,12 @@ static uint64_t ibex_timer_read(void *opaque, > hwaddr addr, > retvalue = s->timer_cfg0; > break; > case R_LOWER0: > - retvalue = now; > + update_mtime(s); > + retvalue = s->timer_lower0; > break; > case R_UPPER0: > - retvalue = now >> 32; > + update_mtime(s); > + retvalue = s->timer_upper0; > break; > case R_COMPARE_LOWER0: > retvalue = s->timer_compare_lower0; @@ -192,17 +222,26 @@ static > void ibex_timer_write(void *opaque, hwaddr addr, > qemu_log_mask(LOG_UNIMP, "Alert triggering not supported"); > break; > case R_CTRL: > + if (!(s->timer_ctrl & R_CTRL_ACTIVE_MASK)) { > + s->timer_last_update = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL); > + } > s->timer_ctrl = val; > + /* We must update IRQs, because the QEMU timer gets updated here. > */ > + ibex_timer_update_irqs(s); > break; > case R_CFG0: > qemu_log_mask(LOG_UNIMP, "Changing prescale or step not > supported"); > s->timer_cfg0 = val; > break; > case R_LOWER0: > - qemu_log_mask(LOG_UNIMP, "Changing timer value is not supported"); > + s->timer_lower0 = val; > + s->timer_last_update = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL); > + ibex_timer_update_irqs(s); > break; > case R_UPPER0: > - qemu_log_mask(LOG_UNIMP, "Changing timer value is not supported"); > + s->timer_upper0 = val; > + s->timer_last_update = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL); > + ibex_timer_update_irqs(s); > break; > case R_COMPARE_LOWER0: > s->timer_compare_lower0 = val; > @@ -259,6 +298,9 @@ static const VMStateDescription vmstate_ibex_timer > = { > VMSTATE_UINT32(timer_compare_upper0, IbexTimerState), > VMSTATE_UINT32(timer_intr_enable, IbexTimerState), > VMSTATE_UINT32(timer_intr_state, IbexTimerState), > + VMSTATE_INT64(timer_last_update, IbexTimerState), > + VMSTATE_UINT32(timer_lower0, IbexTimerState), > + VMSTATE_UINT32(timer_upper0, IbexTimerState), > VMSTATE_END_OF_LIST() > } > }; > diff --git a/include/hw/timer/ibex_timer.h b/include/hw/timer/ibex_timer.h > index 41f5c82a92..15c16035a8 100644 > --- a/include/hw/timer/ibex_timer.h > +++ b/include/hw/timer/ibex_timer.h > @@ -36,11 +36,17 @@ struct IbexTimerState { > uint64_t mtimecmp; > QEMUTimer *mtimer; /* Internal timer for M-mode interrupt */ > > + int64_t timer_last_update; /* Used for extrapolating mtime. */ > + > /* <public> */ > MemoryRegion mmio; > > uint32_t timer_ctrl; > uint32_t timer_cfg0; > + > + > + uint32_t timer_lower0; > + uint32_t timer_upper0; > uint32_t timer_compare_lower0; > uint32_t timer_compare_upper0; > uint32_t timer_intr_enable; > diff --git a/tests/qtest/ibex-timer-test.c b/tests/qtest/ibex-timer-test.c new > file mode 100644 index 0000000000..1c2dfb0b6c > --- /dev/null > +++ b/tests/qtest/ibex-timer-test.c > @@ -0,0 +1,178 @@ > +/* > + * Testing the Ibex Timer > + * > + * Copyright (c) 2022 Rivos Inc. > + * > + * 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 "libqtest.h" > +#include "qapi/qmp/qdict.h" > + > +#define TIMER_BASE_ADDR 0x40100000UL > +#define TIMER_ADDR(addr) (TIMER_BASE_ADDR + addr) #define > +TIMER_EXPIRED_IRQ 127 #define NANOS_PER_SECOND 1000000000LL > + > +#define A_ALERT_TEST 0x0 > +#define A_CTRL 0x4 > +#define A_CFG0 0x100 > +#define A_LOWER_0 0x104 > +#define A_UPPER_0 0x108 > +#define A_COMPARE_LOWER0 0x10C > +#define A_COMPARE_UPPER0 0x110 > +#define A_INTR_ENABLE 0x114 > +#define A_INTR_STATE 0x118 > +#define A_INTR_TEST 0x11C > + > +/* > + * Tests that regs get reset properly. > + */ > +static void test_reset(void) > +{ > + QTestState *test = qtest_init("-M opentitan"); > + qtest_irq_intercept_in(test, "/machine/soc/plic"); > + /* Regs are reset; skip WO regs */ > + g_assert_cmpuint(qtest_readl(test, TIMER_ADDR(A_CTRL)), ==, 0); > + g_assert_cmpuint(qtest_readl(test, TIMER_ADDR(A_INTR_ENABLE)), ==, 0); > + g_assert_cmpuint(qtest_readl(test, TIMER_ADDR(A_INTR_STATE)), ==, 0); > + g_assert_cmpuint(qtest_readl(test, TIMER_ADDR(A_CFG0)), ==, 0x10000); > + g_assert_cmpuint(qtest_readl(test, TIMER_ADDR(A_LOWER_0)), ==, 0); > + g_assert_cmpuint(qtest_readl(test, TIMER_ADDR(A_UPPER_0)), ==, 0); > + g_assert_cmpuint(qtest_readl(test, TIMER_ADDR(A_COMPARE_LOWER0)), > ==, > + UINT32_MAX); > + g_assert_cmpuint(qtest_readl(test, TIMER_ADDR(A_COMPARE_UPPER0)), > ==, > + UINT32_MAX); > + g_assert(!qtest_get_irq(test, TIMER_EXPIRED_IRQ)); > + qtest_quit(test); > +} > + > +/* > + * Test that writes worked. > + */ > +static void test_writes(void) > +{ > + QTestState *test = qtest_init("-M opentitan"); > + > + qtest_irq_intercept_in(test, "/machine/soc/plic"); > + > + /* Special regs that may/may not need to be tested yet */ > +/* > + g_assert_cmpuint(qtest_readl(test, TIMER_ADDR(A_ALERT_TEST)), ==, 0); > + g_assert_cmpuint(qtest_readl(test, TIMER_ADDR(A_INTR_TEST)), ==, 0); > + g_assert_cmpuint(qtest_readl(test, TIMER_ADDR(A_INTR_STATE)), ==, > +0); */ > + /* Write to ctrl */ > + qtest_writel(test, TIMER_ADDR(A_CTRL), 0x1); > + g_assert_cmpuint(qtest_readl(test, TIMER_ADDR(A_CTRL)), ==, 0x1); > + /* Write to intr_enable */ > + qtest_writel(test, TIMER_ADDR(A_INTR_ENABLE), 0x1); > + g_assert_cmpuint(qtest_readl(test, TIMER_ADDR(A_INTR_ENABLE)), ==, > +0x1); > + > + /* Writes to config? Though none of it is supported */ > + qtest_writel(test, TIMER_ADDR(A_CFG0), 0x20001); > + g_assert_cmpuint(qtest_readl(test, TIMER_ADDR(A_CFG0)), ==, > + 0x20001); > + > + /* Writes to mtime */ > + qtest_writel(test, TIMER_ADDR(A_LOWER_0), 0xdeaddad); > + g_assert_cmpuint(qtest_readl(test, TIMER_ADDR(A_LOWER_0)), ==, > 0xdeaddad); > + qtest_writel(test, TIMER_ADDR(A_UPPER_0), 0xdeaddad); > + g_assert_cmpuint(qtest_readl(test, TIMER_ADDR(A_UPPER_0)), ==, > + 0xdeaddad); > + > + /* Writes to mtimecmp */ > + qtest_writel(test, TIMER_ADDR(A_COMPARE_LOWER0), 0xdeaddad); > + g_assert_cmpuint(qtest_readl(test, TIMER_ADDR(A_COMPARE_LOWER0)), > ==, > + 0xdeaddad); > + qtest_writel(test, TIMER_ADDR(A_COMPARE_UPPER0), 0xdeaddad); > + g_assert_cmpuint(qtest_readl(test, TIMER_ADDR(A_COMPARE_UPPER0)), > ==, > + 0xdeaddad); > + qtest_quit(test); > +} > + > +/* > + * Test the standard operation of the timer. > + */ > +static void test_operation(void) > +{ > + /* A frequency of 1000000 Hz*/ > + QTestState *test = qtest_init( > + "-M opentitan " > + "-global driver=ibex-timer,property=timebase-freq,value=1000000" > + ); > + qtest_irq_intercept_in(test, "/machine/soc/plic"); > + /* Set mtimecmp; approx 1 second. */ > + qtest_writel(test, TIMER_ADDR(A_COMPARE_LOWER0), 1000000); > + qtest_writel(test, TIMER_ADDR(A_COMPARE_UPPER0), 0); > + > + /* Wait for some amount of time. Timer should not tick. */ > + qtest_clock_step(test, NANOS_PER_SECOND * 30); > + g_assert_cmpuint(qtest_readl(test, TIMER_ADDR(A_LOWER_0)), ==, 0); > + g_assert_cmpuint(qtest_readl(test, TIMER_ADDR(A_UPPER_0)), ==, 0); > + > + /* First, enable the timer. */ > + qtest_writel(test, TIMER_ADDR(A_CTRL), 0x1); > + qtest_writel(test, TIMER_ADDR(A_INTR_ENABLE), 0x1); > + > + /* The counter should remain at 0, and no interrupts. */ > + g_assert_cmpuint(qtest_readl(test, TIMER_ADDR(A_LOWER_0)), ==, 0); > + g_assert_cmpuint(qtest_readl(test, TIMER_ADDR(A_UPPER_0)), ==, 0); > + g_assert(!qtest_get_irq(test, TIMER_EXPIRED_IRQ)); > + > + /* Let it run for half a second. No interrupts. */ > + qtest_clock_step(test, NANOS_PER_SECOND / 2); > + g_assert_cmpuint(qtest_readl(test, TIMER_ADDR(A_LOWER_0)), <, > + qtest_readl(test, TIMER_ADDR(A_COMPARE_LOWER0))); > + g_assert_cmpuint(qtest_readl(test, TIMER_ADDR(A_UPPER_0)), <=, > + qtest_readl(test, TIMER_ADDR(A_COMPARE_UPPER0))); > + g_assert(!qtest_get_irq(test, TIMER_EXPIRED_IRQ)); > + > + /* Let it run for half a second again. Interrupt. */ > + qtest_clock_step(test, NANOS_PER_SECOND / 2); > + g_assert_cmpuint(qtest_readl(test, TIMER_ADDR(A_LOWER_0)), ==, > + qtest_readl(test, TIMER_ADDR(A_COMPARE_LOWER0))); > + g_assert_cmpuint(qtest_readl(test, TIMER_ADDR(A_UPPER_0)), ==, > + qtest_readl(test, TIMER_ADDR(A_COMPARE_UPPER0))); > + g_assert(qtest_get_irq(test, TIMER_EXPIRED_IRQ)); > + > + /* Handle interrupt, no more interrupt after. */ > + qtest_writel(test, TIMER_ADDR(A_LOWER_0), 0x0); > + qtest_writel(test, TIMER_ADDR(A_UPPER_0), 0x0); > + qtest_writel(test, TIMER_ADDR(A_INTR_STATE), 0x1); > + g_assert(!qtest_get_irq(test, TIMER_EXPIRED_IRQ)); > + > + /* One more second. */ > + qtest_clock_step(test, NANOS_PER_SECOND); > + g_assert_cmpuint(qtest_readl(test, TIMER_ADDR(A_LOWER_0)), ==, > + qtest_readl(test, TIMER_ADDR(A_COMPARE_LOWER0))); > + g_assert_cmpuint(qtest_readl(test, TIMER_ADDR(A_UPPER_0)), ==, > + qtest_readl(test, TIMER_ADDR(A_COMPARE_UPPER0))); > + g_assert(qtest_get_irq(test, TIMER_EXPIRED_IRQ)); > + > + qtest_quit(test); > +} > + > +int main(int argc, char **argv) > +{ > + g_test_init(&argc, &argv, NULL); > + qtest_add_func("ibex-timer/reset", test_reset); > + qtest_add_func("ibex-timer/writes", test_writes); > + qtest_add_func("ibex-timer/op", test_operation); > + return g_test_run(); > +} > diff --git a/tests/qtest/meson.build b/tests/qtest/meson.build index > fb63b8d3fa..7a769a79c5 100644 > --- a/tests/qtest/meson.build > +++ b/tests/qtest/meson.build > @@ -235,7 +235,8 @@ qtests_s390x = \ > 'migration-test'] > > qtests_riscv32 = \ > - ['ibex-aon-timer-test'] > + ['ibex-aon-timer-test', > + 'ibex-timer-test'] > > qos_test_ss = ss.source_set() > qos_test_ss.add( > -- > 2.34.1 -Tyler
On Wed, Sep 28, 2022 at 4:00 PM Dong, Eddie <eddie.dong@intel.com> wrote: > > > From: Tyler Ng <tkng@rivosinc.com> > Sent: Monday, September 26, 2022 4:38 PM > To: Dong, Eddie <eddie.dong@intel.com> > Cc: open list:RISC-V <qemu-riscv@nongnu.org>; qemu-devel@nongnu.org > Developers <qemu-devel@nongnu.org>; Alistair Francis < > Alistair.Francis@wdc.com>; Meng, Bin <bin.meng@windriver.com>; Thomas > Huth <thuth@redhat.com>; Paolo Bonzini <pbonzini@redhat.com>; Palmer > Dabbelt <palmer@dabbelt.com>; Laurent Vivier <lvivier@redhat.com> > Subject: Re: [PATCH v2 3/3] hw/timer: ibex_timer.c: Add support for writes > to mtime > > Hi Eddie, > > On Mon, Sep 26, 2022 at 2:06 PM Dong, Eddie <mailto:eddie.dong@intel.com> > wrote: > > > > -----Original Message----- > > From: Qemu-devel <qemu-devel-bounces+eddie.dong=mailto: > intel.com@nongnu.org> > > On Behalf Of Tyler Ng > > Sent: Thursday, September 22, 2022 8:59 AM > > To: open list:RISC-V <mailto:qemu-riscv@nongnu.org>; mailto: > qemu-devel@nongnu.org > > Developers <mailto:qemu-devel@nongnu.org> > > Cc: Alistair Francis <mailto:Alistair.Francis@wdc.com>; Meng, Bin > > <mailto:bin.meng@windriver.com>; Thomas Huth <mailto:thuth@redhat.com>; > Paolo > > Bonzini <mailto:pbonzini@redhat.com>; Palmer Dabbelt <mailto: > palmer@dabbelt.com>; > > Laurent Vivier <mailto:lvivier@redhat.com> > > Subject: [PATCH v2 3/3] hw/timer: ibex_timer.c: Add support for writes to > > mtime > > > > 1. Adds fields to hold the value of mtime in timer_upper0 and > timer_lower0. > > > > 2. Changes the read and write functions to use the mtime fields. > > > > 3. Updates the value of mtime in update_mtime() by extrapolating the time > > elapsed. This will need to change if/when the prescalar is implemented. > > > > 4. Adds a qtest for the ibex timer. > > > > Signed-off-by: Tyler Ng <mailto:tkng@rivosinc.com> > > --- > > hw/timer/ibex_timer.c | 98 +++++++++++++------ > > include/hw/timer/ibex_timer.h | 6 ++ > > tests/qtest/ibex-timer-test.c | 178 ++++++++++++++++++++++++++++++++++ > > tests/qtest/meson.build | 3 +- > > 4 files changed, 256 insertions(+), 29 deletions(-) create mode 100644 > > tests/qtest/ibex-timer-test.c > > > > diff --git a/hw/timer/ibex_timer.c b/hw/timer/ibex_timer.c index > > d8b8e4e1f6..4230d60e85 100644 > > --- a/hw/timer/ibex_timer.c > > +++ b/hw/timer/ibex_timer.c > > @@ -52,28 +52,56 @@ REG32(INTR_STATE, 0x118) REG32(INTR_TEST, > > 0x11C) > > FIELD(INTR_TEST, T_0, 0, 1) > > > > -static uint64_t cpu_riscv_read_rtc(uint32_t timebase_freq) > > +static inline uint64_t get_mtime(void *opaque) > > { > > - return muldiv64(qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL), > > - timebase_freq, NANOSECONDS_PER_SECOND); > > + IbexTimerState *s = opaque; > > + return (s->timer_lower0) | ((uint64_t) s->timer_upper0 << 32); > > } > > > > -static void ibex_timer_update_irqs(IbexTimerState *s) > > +/* > > + * The goal of this function is to: > > + * 1. Check if the timer is enabled. If not, return false, > > + * 2. Calculate the amount of time that has passed since. > > + * 3. Extrapolate the number of ticks that have passed, and add it to > `mtime`. > > + * 4. Return true. > > + */ > > +static bool update_mtime(IbexTimerState *s) > > { > > - uint64_t value = s->timer_compare_lower0 | > > - ((uint64_t)s->timer_compare_upper0 << 32); > So the hardware actually implements 64 bits register (used in 32 bits > CPU), why not use an union for this? > Same for: > + uint32_t timer_lower0; > + uint32_t timer_upper0; > I'm not too sure what a C union would do for us here? > > I think what the hardware really implement is a 64 bits register, with 32 > bits interface to access. > struct IbexTimerState actually defines both of them: > uint64_t mtimecmp; > uint32_t timer_compare_lower0; > uint32_t timer_compare_upper0; > > It seems that the 64-bit mtimecmp field was added between patch versions... Upon closer inspection I believe it duplicates functionality of the existing 32-bit fields. It probably should be removed. > Using a union can better reflect this. Also, it can avoid the convert from > 2 32-bits register to 64 bits, like the above code does. > ibex_timer_update_irqs() also does this conversion. > > It took me a bit of time, but now I think I understand what you mean: a union of 2 uint32_t's (perhaps packed into a struct or an array) and a uint64_t would make it easier to access the components, is that what you mean? That is pretty handy, thanks. -Tyler > > - uint64_t next, diff; > > - uint64_t now = cpu_riscv_read_rtc(s->timebase_freq); > > - > > if (!(s->timer_ctrl & R_CTRL_ACTIVE_MASK)) { > > - /* Timer isn't active */ > > + return false; > > + } > > + /* Get the time then extrapolate the number of ticks that have > elapsed */ > > + uint64_t mtime = get_mtime(s); > > + int64_t now = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL); > > + int64_t elapsed = now - s->timer_last_update; > > + if (elapsed < 0) { > > + /* We jumped back in time. */ > > + mtime -= muldiv64((uint64_t)(-elapsed), s->timebase_freq, > > + NANOSECONDS_PER_SECOND); > > + } else { > > + mtime += muldiv64(elapsed, s->timebase_freq, > > NANOSECONDS_PER_SECOND); > > + } > > + s->timer_lower0 = mtime & 0xffffffff; > > + s->timer_upper0 = (mtime >> 32) & 0xffffffff; > > + /* update last-checkpoint timestamp */ > > + s->timer_last_update = now; > > + return true; > > +} > > + > > +static void ibex_timer_update_irqs(IbexTimerState *s) { > > + if (!update_mtime(s)) { > > + /* Timer is not enabled? */ > > return; > > } > > + uint64_t mtimecmp = s->timer_compare_lower0 | > > + ((uint64_t)s->timer_compare_upper0 << 32); > > + uint64_t mtime = get_mtime(s); > > > > /* Update the CPUs mtimecmp */ > > - s->mtimecmp = value; > > + s->mtimecmp = mtimecmp; > > > > - if (s->mtimecmp <= now) { > > + if (s->mtimecmp <= mtime) { > > /* > > * If the mtimecmp was in the past raise the interrupt now. > > */ > > @@ -84,18 +112,17 @@ static void ibex_timer_update_irqs(IbexTimerState > > *s) > > } > > return; > > } > > - > > - /* Setup a timer to trigger the interrupt in the future */ > > + /* Update timers: setup a timer to trigger the interrupt in the > > + future */ > > qemu_irq_lower(s->m_timer_irq); > > qemu_set_irq(s->irq, false); > > - > > - diff = s->mtimecmp - now; > > - next = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL) + > > - muldiv64(diff, > > - NANOSECONDS_PER_SECOND, > > - s->timebase_freq); > > - > > - if (next < qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL)) { > > + /* Compute the difference, and set a timer for the next update. */ > > + const uint64_t diff = mtimecmp - mtime; > > + const int64_t now = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL); > > + const uint64_t towait = muldiv64(diff, NANOSECONDS_PER_SECOND, > > + s->timebase_freq); > > + /* timer_mod takes in a int64_t, not uint64_t! Need to saturate it > */ > > + const int64_t next = now + towait; > > + if (next < now) { > > /* We overflowed the timer, just set it as large as we can */ > > timer_mod(s->mtimer, 0x7FFFFFFFFFFFFFFF); > > } else { > > @@ -124,11 +151,13 @@ static void ibex_timer_reset(DeviceState *dev) > > > > s->timer_ctrl = 0x00000000; > > s->timer_cfg0 = 0x00010000; > > + s->timer_lower0 = 0x00000000; > > + s->timer_upper0 = 0x00000000; > > s->timer_compare_lower0 = 0xFFFFFFFF; > > s->timer_compare_upper0 = 0xFFFFFFFF; > > s->timer_intr_enable = 0x00000000; > > s->timer_intr_state = 0x00000000; > > - > > + s->timer_last_update = 0x00000000; > > ibex_timer_update_irqs(s); > > } > > > > @@ -136,7 +165,6 @@ static uint64_t ibex_timer_read(void *opaque, > > hwaddr addr, > > unsigned int size) { > > IbexTimerState *s = opaque; > > - uint64_t now = cpu_riscv_read_rtc(s->timebase_freq); > > uint64_t retvalue = 0; > > > > switch (addr >> 2) { > > @@ -151,10 +179,12 @@ static uint64_t ibex_timer_read(void *opaque, > > hwaddr addr, > > retvalue = s->timer_cfg0; > > break; > > case R_LOWER0: > > - retvalue = now; > > + update_mtime(s); > > + retvalue = s->timer_lower0; > > break; > > case R_UPPER0: > > - retvalue = now >> 32; > > + update_mtime(s); > > + retvalue = s->timer_upper0; > > break; > > case R_COMPARE_LOWER0: > > retvalue = s->timer_compare_lower0; @@ -192,17 +222,26 @@ static > > void ibex_timer_write(void *opaque, hwaddr addr, > > qemu_log_mask(LOG_UNIMP, "Alert triggering not supported"); > > break; > > case R_CTRL: > > + if (!(s->timer_ctrl & R_CTRL_ACTIVE_MASK)) { > > + s->timer_last_update = > qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL); > > + } > > s->timer_ctrl = val; > > + /* We must update IRQs, because the QEMU timer gets updated > here. > > */ > > + ibex_timer_update_irqs(s); > > break; > > case R_CFG0: > > qemu_log_mask(LOG_UNIMP, "Changing prescale or step not > > supported"); > > s->timer_cfg0 = val; > > break; > > case R_LOWER0: > > - qemu_log_mask(LOG_UNIMP, "Changing timer value is not > supported"); > > + s->timer_lower0 = val; > > + s->timer_last_update = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL); > > + ibex_timer_update_irqs(s); > > break; > > case R_UPPER0: > > - qemu_log_mask(LOG_UNIMP, "Changing timer value is not > supported"); > > + s->timer_upper0 = val; > > + s->timer_last_update = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL); > > + ibex_timer_update_irqs(s); > > break; > > case R_COMPARE_LOWER0: > > s->timer_compare_lower0 = val; > > @@ -259,6 +298,9 @@ static const VMStateDescription vmstate_ibex_timer > > = { > > VMSTATE_UINT32(timer_compare_upper0, IbexTimerState), > > VMSTATE_UINT32(timer_intr_enable, IbexTimerState), > > VMSTATE_UINT32(timer_intr_state, IbexTimerState), > > + VMSTATE_INT64(timer_last_update, IbexTimerState), > > + VMSTATE_UINT32(timer_lower0, IbexTimerState), > > + VMSTATE_UINT32(timer_upper0, IbexTimerState), > > VMSTATE_END_OF_LIST() > > } > > }; > > diff --git a/include/hw/timer/ibex_timer.h > b/include/hw/timer/ibex_timer.h > > index 41f5c82a92..15c16035a8 100644 > > --- a/include/hw/timer/ibex_timer.h > > +++ b/include/hw/timer/ibex_timer.h > > @@ -36,11 +36,17 @@ struct IbexTimerState { > > uint64_t mtimecmp; > > QEMUTimer *mtimer; /* Internal timer for M-mode interrupt */ > > > > + int64_t timer_last_update; /* Used for extrapolating mtime. */ > > + > > /* <public> */ > > MemoryRegion mmio; > > > > uint32_t timer_ctrl; > > uint32_t timer_cfg0; > > + > > + > > + uint32_t timer_lower0; > > + uint32_t timer_upper0; > > uint32_t timer_compare_lower0; > > uint32_t timer_compare_upper0; > > uint32_t timer_intr_enable; > > diff --git a/tests/qtest/ibex-timer-test.c > b/tests/qtest/ibex-timer-test.c new > > file mode 100644 index 0000000000..1c2dfb0b6c > > --- /dev/null > > +++ b/tests/qtest/ibex-timer-test.c > > @@ -0,0 +1,178 @@ > > +/* > > + * Testing the Ibex Timer > > + * > > + * Copyright (c) 2022 Rivos Inc. > > + * > > + * 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 "libqtest.h" > > +#include "qapi/qmp/qdict.h" > > + > > +#define TIMER_BASE_ADDR 0x40100000UL > > +#define TIMER_ADDR(addr) (TIMER_BASE_ADDR + addr) #define > > +TIMER_EXPIRED_IRQ 127 #define NANOS_PER_SECOND 1000000000LL > > + > > +#define A_ALERT_TEST 0x0 > > +#define A_CTRL 0x4 > > +#define A_CFG0 0x100 > > +#define A_LOWER_0 0x104 > > +#define A_UPPER_0 0x108 > > +#define A_COMPARE_LOWER0 0x10C > > +#define A_COMPARE_UPPER0 0x110 > > +#define A_INTR_ENABLE 0x114 > > +#define A_INTR_STATE 0x118 > > +#define A_INTR_TEST 0x11C > > + > > +/* > > + * Tests that regs get reset properly. > > + */ > > +static void test_reset(void) > > +{ > > + QTestState *test = qtest_init("-M opentitan"); > > + qtest_irq_intercept_in(test, "/machine/soc/plic"); > > + /* Regs are reset; skip WO regs */ > > + g_assert_cmpuint(qtest_readl(test, TIMER_ADDR(A_CTRL)), ==, 0); > > + g_assert_cmpuint(qtest_readl(test, TIMER_ADDR(A_INTR_ENABLE)), ==, > 0); > > + g_assert_cmpuint(qtest_readl(test, TIMER_ADDR(A_INTR_STATE)), ==, > 0); > > + g_assert_cmpuint(qtest_readl(test, TIMER_ADDR(A_CFG0)), ==, > 0x10000); > > + g_assert_cmpuint(qtest_readl(test, TIMER_ADDR(A_LOWER_0)), ==, 0); > > + g_assert_cmpuint(qtest_readl(test, TIMER_ADDR(A_UPPER_0)), ==, 0); > > + g_assert_cmpuint(qtest_readl(test, TIMER_ADDR(A_COMPARE_LOWER0)), > > ==, > > + UINT32_MAX); > > + g_assert_cmpuint(qtest_readl(test, TIMER_ADDR(A_COMPARE_UPPER0)), > > ==, > > + UINT32_MAX); > > + g_assert(!qtest_get_irq(test, TIMER_EXPIRED_IRQ)); > > + qtest_quit(test); > > +} > > + > > +/* > > + * Test that writes worked. > > + */ > > +static void test_writes(void) > > +{ > > + QTestState *test = qtest_init("-M opentitan"); > > + > > + qtest_irq_intercept_in(test, "/machine/soc/plic"); > > + > > + /* Special regs that may/may not need to be tested yet */ > > +/* > > + g_assert_cmpuint(qtest_readl(test, TIMER_ADDR(A_ALERT_TEST)), ==, > 0); > > + g_assert_cmpuint(qtest_readl(test, TIMER_ADDR(A_INTR_TEST)), ==, 0); > > + g_assert_cmpuint(qtest_readl(test, TIMER_ADDR(A_INTR_STATE)), ==, > > +0); */ > > + /* Write to ctrl */ > > + qtest_writel(test, TIMER_ADDR(A_CTRL), 0x1); > > + g_assert_cmpuint(qtest_readl(test, TIMER_ADDR(A_CTRL)), ==, 0x1); > > + /* Write to intr_enable */ > > + qtest_writel(test, TIMER_ADDR(A_INTR_ENABLE), 0x1); > > + g_assert_cmpuint(qtest_readl(test, TIMER_ADDR(A_INTR_ENABLE)), ==, > > +0x1); > > + > > + /* Writes to config? Though none of it is supported */ > > + qtest_writel(test, TIMER_ADDR(A_CFG0), 0x20001); > > + g_assert_cmpuint(qtest_readl(test, TIMER_ADDR(A_CFG0)), ==, > > + 0x20001); > > + > > + /* Writes to mtime */ > > + qtest_writel(test, TIMER_ADDR(A_LOWER_0), 0xdeaddad); > > + g_assert_cmpuint(qtest_readl(test, TIMER_ADDR(A_LOWER_0)), ==, > > 0xdeaddad); > > + qtest_writel(test, TIMER_ADDR(A_UPPER_0), 0xdeaddad); > > + g_assert_cmpuint(qtest_readl(test, TIMER_ADDR(A_UPPER_0)), ==, > > + 0xdeaddad); > > + > > + /* Writes to mtimecmp */ > > + qtest_writel(test, TIMER_ADDR(A_COMPARE_LOWER0), 0xdeaddad); > > + g_assert_cmpuint(qtest_readl(test, TIMER_ADDR(A_COMPARE_LOWER0)), > > ==, > > + 0xdeaddad); > > + qtest_writel(test, TIMER_ADDR(A_COMPARE_UPPER0), 0xdeaddad); > > + g_assert_cmpuint(qtest_readl(test, TIMER_ADDR(A_COMPARE_UPPER0)), > > ==, > > + 0xdeaddad); > > + qtest_quit(test); > > +} > > + > > +/* > > + * Test the standard operation of the timer. > > + */ > > +static void test_operation(void) > > +{ > > + /* A frequency of 1000000 Hz*/ > > + QTestState *test = qtest_init( > > + "-M opentitan " > > + "-global driver=ibex-timer,property=timebase-freq,value=1000000" > > + ); > > + qtest_irq_intercept_in(test, "/machine/soc/plic"); > > + /* Set mtimecmp; approx 1 second. */ > > + qtest_writel(test, TIMER_ADDR(A_COMPARE_LOWER0), 1000000); > > + qtest_writel(test, TIMER_ADDR(A_COMPARE_UPPER0), 0); > > + > > + /* Wait for some amount of time. Timer should not tick. */ > > + qtest_clock_step(test, NANOS_PER_SECOND * 30); > > + g_assert_cmpuint(qtest_readl(test, TIMER_ADDR(A_LOWER_0)), ==, 0); > > + g_assert_cmpuint(qtest_readl(test, TIMER_ADDR(A_UPPER_0)), ==, 0); > > + > > + /* First, enable the timer. */ > > + qtest_writel(test, TIMER_ADDR(A_CTRL), 0x1); > > + qtest_writel(test, TIMER_ADDR(A_INTR_ENABLE), 0x1); > > + > > + /* The counter should remain at 0, and no interrupts. */ > > + g_assert_cmpuint(qtest_readl(test, TIMER_ADDR(A_LOWER_0)), ==, 0); > > + g_assert_cmpuint(qtest_readl(test, TIMER_ADDR(A_UPPER_0)), ==, 0); > > + g_assert(!qtest_get_irq(test, TIMER_EXPIRED_IRQ)); > > + > > + /* Let it run for half a second. No interrupts. */ > > + qtest_clock_step(test, NANOS_PER_SECOND / 2); > > + g_assert_cmpuint(qtest_readl(test, TIMER_ADDR(A_LOWER_0)), <, > > + qtest_readl(test, TIMER_ADDR(A_COMPARE_LOWER0))); > > + g_assert_cmpuint(qtest_readl(test, TIMER_ADDR(A_UPPER_0)), <=, > > + qtest_readl(test, TIMER_ADDR(A_COMPARE_UPPER0))); > > + g_assert(!qtest_get_irq(test, TIMER_EXPIRED_IRQ)); > > + > > + /* Let it run for half a second again. Interrupt. */ > > + qtest_clock_step(test, NANOS_PER_SECOND / 2); > > + g_assert_cmpuint(qtest_readl(test, TIMER_ADDR(A_LOWER_0)), ==, > > + qtest_readl(test, TIMER_ADDR(A_COMPARE_LOWER0))); > > + g_assert_cmpuint(qtest_readl(test, TIMER_ADDR(A_UPPER_0)), ==, > > + qtest_readl(test, TIMER_ADDR(A_COMPARE_UPPER0))); > > + g_assert(qtest_get_irq(test, TIMER_EXPIRED_IRQ)); > > + > > + /* Handle interrupt, no more interrupt after. */ > > + qtest_writel(test, TIMER_ADDR(A_LOWER_0), 0x0); > > + qtest_writel(test, TIMER_ADDR(A_UPPER_0), 0x0); > > + qtest_writel(test, TIMER_ADDR(A_INTR_STATE), 0x1); > > + g_assert(!qtest_get_irq(test, TIMER_EXPIRED_IRQ)); > > + > > + /* One more second. */ > > + qtest_clock_step(test, NANOS_PER_SECOND); > > + g_assert_cmpuint(qtest_readl(test, TIMER_ADDR(A_LOWER_0)), ==, > > + qtest_readl(test, TIMER_ADDR(A_COMPARE_LOWER0))); > > + g_assert_cmpuint(qtest_readl(test, TIMER_ADDR(A_UPPER_0)), ==, > > + qtest_readl(test, TIMER_ADDR(A_COMPARE_UPPER0))); > > + g_assert(qtest_get_irq(test, TIMER_EXPIRED_IRQ)); > > + > > + qtest_quit(test); > > +} > > + > > +int main(int argc, char **argv) > > +{ > > + g_test_init(&argc, &argv, NULL); > > + qtest_add_func("ibex-timer/reset", test_reset); > > + qtest_add_func("ibex-timer/writes", test_writes); > > + qtest_add_func("ibex-timer/op", test_operation); > > + return g_test_run(); > > +} > > diff --git a/tests/qtest/meson.build b/tests/qtest/meson.build index > > fb63b8d3fa..7a769a79c5 100644 > > --- a/tests/qtest/meson.build > > +++ b/tests/qtest/meson.build > > @@ -235,7 +235,8 @@ qtests_s390x = \ > > 'migration-test'] > > > > qtests_riscv32 = \ > > - ['ibex-aon-timer-test'] > > + ['ibex-aon-timer-test', > > + 'ibex-timer-test'] > > > > qos_test_ss = ss.source_set() > > qos_test_ss.add( > > -- > > 2.34.1 > -Tyler >
Using a union can better reflect this. Also, it can avoid the convert from 2 32-bits register to 64 bits, like the above code does. ibex_timer_update_irqs() also does this conversion. It took me a bit of time, but now I think I understand what you mean: a union of 2 uint32_t's (perhaps packed into a struct or an array) and a uint64_t would make it easier to access the components, is that what you mean? That is pretty handy, thanks. YES. You decide
diff --git a/hw/timer/ibex_timer.c b/hw/timer/ibex_timer.c index d8b8e4e1f6..4230d60e85 100644 --- a/hw/timer/ibex_timer.c +++ b/hw/timer/ibex_timer.c @@ -52,28 +52,56 @@ REG32(INTR_STATE, 0x118) REG32(INTR_TEST, 0x11C) FIELD(INTR_TEST, T_0, 0, 1) -static uint64_t cpu_riscv_read_rtc(uint32_t timebase_freq) +static inline uint64_t get_mtime(void *opaque) { - return muldiv64(qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL), - timebase_freq, NANOSECONDS_PER_SECOND); + IbexTimerState *s = opaque; + return (s->timer_lower0) | ((uint64_t) s->timer_upper0 << 32); } -static void ibex_timer_update_irqs(IbexTimerState *s) +/* + * The goal of this function is to: + * 1. Check if the timer is enabled. If not, return false, + * 2. Calculate the amount of time that has passed since. + * 3. Extrapolate the number of ticks that have passed, and add it to `mtime`. + * 4. Return true. + */ +static bool update_mtime(IbexTimerState *s) { - uint64_t value = s->timer_compare_lower0 | - ((uint64_t)s->timer_compare_upper0 << 32); - uint64_t next, diff; - uint64_t now = cpu_riscv_read_rtc(s->timebase_freq); - if (!(s->timer_ctrl & R_CTRL_ACTIVE_MASK)) { - /* Timer isn't active */ + return false; + } + /* Get the time then extrapolate the number of ticks that have elapsed */ + uint64_t mtime = get_mtime(s); + int64_t now = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL); + int64_t elapsed = now - s->timer_last_update; + if (elapsed < 0) { + /* We jumped back in time. */ + mtime -= muldiv64((uint64_t)(-elapsed), s->timebase_freq, + NANOSECONDS_PER_SECOND); + } else { + mtime += muldiv64(elapsed, s->timebase_freq, NANOSECONDS_PER_SECOND); + } + s->timer_lower0 = mtime & 0xffffffff; + s->timer_upper0 = (mtime >> 32) & 0xffffffff; + /* update last-checkpoint timestamp */ + s->timer_last_update = now; + return true; +} + +static void ibex_timer_update_irqs(IbexTimerState *s) +{ + if (!update_mtime(s)) { + /* Timer is not enabled? */ return; } + uint64_t mtimecmp = s->timer_compare_lower0 | + ((uint64_t)s->timer_compare_upper0 << 32); + uint64_t mtime = get_mtime(s); /* Update the CPUs mtimecmp */ - s->mtimecmp = value; + s->mtimecmp = mtimecmp; - if (s->mtimecmp <= now) { + if (s->mtimecmp <= mtime) { /* * If the mtimecmp was in the past raise the interrupt now. */ @@ -84,18 +112,17 @@ static void ibex_timer_update_irqs(IbexTimerState *s) } return; } - - /* Setup a timer to trigger the interrupt in the future */ + /* Update timers: setup a timer to trigger the interrupt in the future */ qemu_irq_lower(s->m_timer_irq); qemu_set_irq(s->irq, false); - - diff = s->mtimecmp - now; - next = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL) + - muldiv64(diff, - NANOSECONDS_PER_SECOND, - s->timebase_freq); - - if (next < qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL)) { + /* Compute the difference, and set a timer for the next update. */ + const uint64_t diff = mtimecmp - mtime; + const int64_t now = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL); + const uint64_t towait = muldiv64(diff, NANOSECONDS_PER_SECOND, + s->timebase_freq); + /* timer_mod takes in a int64_t, not uint64_t! Need to saturate it */ + const int64_t next = now + towait; + if (next < now) { /* We overflowed the timer, just set it as large as we can */ timer_mod(s->mtimer, 0x7FFFFFFFFFFFFFFF); } else { @@ -124,11 +151,13 @@ static void ibex_timer_reset(DeviceState *dev) s->timer_ctrl = 0x00000000; s->timer_cfg0 = 0x00010000; + s->timer_lower0 = 0x00000000; + s->timer_upper0 = 0x00000000; s->timer_compare_lower0 = 0xFFFFFFFF; s->timer_compare_upper0 = 0xFFFFFFFF; s->timer_intr_enable = 0x00000000; s->timer_intr_state = 0x00000000; - + s->timer_last_update = 0x00000000; ibex_timer_update_irqs(s); } @@ -136,7 +165,6 @@ static uint64_t ibex_timer_read(void *opaque, hwaddr addr, unsigned int size) { IbexTimerState *s = opaque; - uint64_t now = cpu_riscv_read_rtc(s->timebase_freq); uint64_t retvalue = 0; switch (addr >> 2) { @@ -151,10 +179,12 @@ static uint64_t ibex_timer_read(void *opaque, hwaddr addr, retvalue = s->timer_cfg0; break; case R_LOWER0: - retvalue = now; + update_mtime(s); + retvalue = s->timer_lower0; break; case R_UPPER0: - retvalue = now >> 32; + update_mtime(s); + retvalue = s->timer_upper0; break; case R_COMPARE_LOWER0: retvalue = s->timer_compare_lower0; @@ -192,17 +222,26 @@ static void ibex_timer_write(void *opaque, hwaddr addr, qemu_log_mask(LOG_UNIMP, "Alert triggering not supported"); break; case R_CTRL: + if (!(s->timer_ctrl & R_CTRL_ACTIVE_MASK)) { + s->timer_last_update = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL); + } s->timer_ctrl = val; + /* We must update IRQs, because the QEMU timer gets updated here. */ + ibex_timer_update_irqs(s); break; case R_CFG0: qemu_log_mask(LOG_UNIMP, "Changing prescale or step not supported"); s->timer_cfg0 = val; break; case R_LOWER0: - qemu_log_mask(LOG_UNIMP, "Changing timer value is not supported"); + s->timer_lower0 = val; + s->timer_last_update = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL); + ibex_timer_update_irqs(s); break; case R_UPPER0: - qemu_log_mask(LOG_UNIMP, "Changing timer value is not supported"); + s->timer_upper0 = val; + s->timer_last_update = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL); + ibex_timer_update_irqs(s); break; case R_COMPARE_LOWER0: s->timer_compare_lower0 = val; @@ -259,6 +298,9 @@ static const VMStateDescription vmstate_ibex_timer = { VMSTATE_UINT32(timer_compare_upper0, IbexTimerState), VMSTATE_UINT32(timer_intr_enable, IbexTimerState), VMSTATE_UINT32(timer_intr_state, IbexTimerState), + VMSTATE_INT64(timer_last_update, IbexTimerState), + VMSTATE_UINT32(timer_lower0, IbexTimerState), + VMSTATE_UINT32(timer_upper0, IbexTimerState), VMSTATE_END_OF_LIST() } }; diff --git a/include/hw/timer/ibex_timer.h b/include/hw/timer/ibex_timer.h index 41f5c82a92..15c16035a8 100644 --- a/include/hw/timer/ibex_timer.h +++ b/include/hw/timer/ibex_timer.h @@ -36,11 +36,17 @@ struct IbexTimerState { uint64_t mtimecmp; QEMUTimer *mtimer; /* Internal timer for M-mode interrupt */ + int64_t timer_last_update; /* Used for extrapolating mtime. */ + /* <public> */ MemoryRegion mmio; uint32_t timer_ctrl; uint32_t timer_cfg0; + + + uint32_t timer_lower0; + uint32_t timer_upper0; uint32_t timer_compare_lower0; uint32_t timer_compare_upper0; uint32_t timer_intr_enable; diff --git a/tests/qtest/ibex-timer-test.c b/tests/qtest/ibex-timer-test.c new file mode 100644 index 0000000000..1c2dfb0b6c --- /dev/null +++ b/tests/qtest/ibex-timer-test.c @@ -0,0 +1,178 @@ +/* + * Testing the Ibex Timer + * + * Copyright (c) 2022 Rivos Inc. + * + * 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 "libqtest.h" +#include "qapi/qmp/qdict.h" + +#define TIMER_BASE_ADDR 0x40100000UL +#define TIMER_ADDR(addr) (TIMER_BASE_ADDR + addr) +#define TIMER_EXPIRED_IRQ 127 +#define NANOS_PER_SECOND 1000000000LL + +#define A_ALERT_TEST 0x0 +#define A_CTRL 0x4 +#define A_CFG0 0x100 +#define A_LOWER_0 0x104 +#define A_UPPER_0 0x108 +#define A_COMPARE_LOWER0 0x10C +#define A_COMPARE_UPPER0 0x110 +#define A_INTR_ENABLE 0x114 +#define A_INTR_STATE 0x118 +#define A_INTR_TEST 0x11C + +/* + * Tests that regs get reset properly. + */ +static void test_reset(void) +{ + QTestState *test = qtest_init("-M opentitan"); + qtest_irq_intercept_in(test, "/machine/soc/plic"); + /* Regs are reset; skip WO regs */ + g_assert_cmpuint(qtest_readl(test, TIMER_ADDR(A_CTRL)), ==, 0); + g_assert_cmpuint(qtest_readl(test, TIMER_ADDR(A_INTR_ENABLE)), ==, 0); + g_assert_cmpuint(qtest_readl(test, TIMER_ADDR(A_INTR_STATE)), ==, 0); + g_assert_cmpuint(qtest_readl(test, TIMER_ADDR(A_CFG0)), ==, 0x10000); + g_assert_cmpuint(qtest_readl(test, TIMER_ADDR(A_LOWER_0)), ==, 0); + g_assert_cmpuint(qtest_readl(test, TIMER_ADDR(A_UPPER_0)), ==, 0); + g_assert_cmpuint(qtest_readl(test, TIMER_ADDR(A_COMPARE_LOWER0)), ==, + UINT32_MAX); + g_assert_cmpuint(qtest_readl(test, TIMER_ADDR(A_COMPARE_UPPER0)), ==, + UINT32_MAX); + g_assert(!qtest_get_irq(test, TIMER_EXPIRED_IRQ)); + qtest_quit(test); +} + +/* + * Test that writes worked. + */ +static void test_writes(void) +{ + QTestState *test = qtest_init("-M opentitan"); + + qtest_irq_intercept_in(test, "/machine/soc/plic"); + + /* Special regs that may/may not need to be tested yet */ +/* + g_assert_cmpuint(qtest_readl(test, TIMER_ADDR(A_ALERT_TEST)), ==, 0); + g_assert_cmpuint(qtest_readl(test, TIMER_ADDR(A_INTR_TEST)), ==, 0); + g_assert_cmpuint(qtest_readl(test, TIMER_ADDR(A_INTR_STATE)), ==, 0); +*/ + /* Write to ctrl */ + qtest_writel(test, TIMER_ADDR(A_CTRL), 0x1); + g_assert_cmpuint(qtest_readl(test, TIMER_ADDR(A_CTRL)), ==, 0x1); + /* Write to intr_enable */ + qtest_writel(test, TIMER_ADDR(A_INTR_ENABLE), 0x1); + g_assert_cmpuint(qtest_readl(test, TIMER_ADDR(A_INTR_ENABLE)), ==, 0x1); + + /* Writes to config? Though none of it is supported */ + qtest_writel(test, TIMER_ADDR(A_CFG0), 0x20001); + g_assert_cmpuint(qtest_readl(test, TIMER_ADDR(A_CFG0)), ==, 0x20001); + + /* Writes to mtime */ + qtest_writel(test, TIMER_ADDR(A_LOWER_0), 0xdeaddad); + g_assert_cmpuint(qtest_readl(test, TIMER_ADDR(A_LOWER_0)), ==, 0xdeaddad); + qtest_writel(test, TIMER_ADDR(A_UPPER_0), 0xdeaddad); + g_assert_cmpuint(qtest_readl(test, TIMER_ADDR(A_UPPER_0)), ==, 0xdeaddad); + + /* Writes to mtimecmp */ + qtest_writel(test, TIMER_ADDR(A_COMPARE_LOWER0), 0xdeaddad); + g_assert_cmpuint(qtest_readl(test, TIMER_ADDR(A_COMPARE_LOWER0)), ==, + 0xdeaddad); + qtest_writel(test, TIMER_ADDR(A_COMPARE_UPPER0), 0xdeaddad); + g_assert_cmpuint(qtest_readl(test, TIMER_ADDR(A_COMPARE_UPPER0)), ==, + 0xdeaddad); + qtest_quit(test); +} + +/* + * Test the standard operation of the timer. + */ +static void test_operation(void) +{ + /* A frequency of 1000000 Hz*/ + QTestState *test = qtest_init( + "-M opentitan " + "-global driver=ibex-timer,property=timebase-freq,value=1000000" + ); + qtest_irq_intercept_in(test, "/machine/soc/plic"); + /* Set mtimecmp; approx 1 second. */ + qtest_writel(test, TIMER_ADDR(A_COMPARE_LOWER0), 1000000); + qtest_writel(test, TIMER_ADDR(A_COMPARE_UPPER0), 0); + + /* Wait for some amount of time. Timer should not tick. */ + qtest_clock_step(test, NANOS_PER_SECOND * 30); + g_assert_cmpuint(qtest_readl(test, TIMER_ADDR(A_LOWER_0)), ==, 0); + g_assert_cmpuint(qtest_readl(test, TIMER_ADDR(A_UPPER_0)), ==, 0); + + /* First, enable the timer. */ + qtest_writel(test, TIMER_ADDR(A_CTRL), 0x1); + qtest_writel(test, TIMER_ADDR(A_INTR_ENABLE), 0x1); + + /* The counter should remain at 0, and no interrupts. */ + g_assert_cmpuint(qtest_readl(test, TIMER_ADDR(A_LOWER_0)), ==, 0); + g_assert_cmpuint(qtest_readl(test, TIMER_ADDR(A_UPPER_0)), ==, 0); + g_assert(!qtest_get_irq(test, TIMER_EXPIRED_IRQ)); + + /* Let it run for half a second. No interrupts. */ + qtest_clock_step(test, NANOS_PER_SECOND / 2); + g_assert_cmpuint(qtest_readl(test, TIMER_ADDR(A_LOWER_0)), <, + qtest_readl(test, TIMER_ADDR(A_COMPARE_LOWER0))); + g_assert_cmpuint(qtest_readl(test, TIMER_ADDR(A_UPPER_0)), <=, + qtest_readl(test, TIMER_ADDR(A_COMPARE_UPPER0))); + g_assert(!qtest_get_irq(test, TIMER_EXPIRED_IRQ)); + + /* Let it run for half a second again. Interrupt. */ + qtest_clock_step(test, NANOS_PER_SECOND / 2); + g_assert_cmpuint(qtest_readl(test, TIMER_ADDR(A_LOWER_0)), ==, + qtest_readl(test, TIMER_ADDR(A_COMPARE_LOWER0))); + g_assert_cmpuint(qtest_readl(test, TIMER_ADDR(A_UPPER_0)), ==, + qtest_readl(test, TIMER_ADDR(A_COMPARE_UPPER0))); + g_assert(qtest_get_irq(test, TIMER_EXPIRED_IRQ)); + + /* Handle interrupt, no more interrupt after. */ + qtest_writel(test, TIMER_ADDR(A_LOWER_0), 0x0); + qtest_writel(test, TIMER_ADDR(A_UPPER_0), 0x0); + qtest_writel(test, TIMER_ADDR(A_INTR_STATE), 0x1); + g_assert(!qtest_get_irq(test, TIMER_EXPIRED_IRQ)); + + /* One more second. */ + qtest_clock_step(test, NANOS_PER_SECOND); + g_assert_cmpuint(qtest_readl(test, TIMER_ADDR(A_LOWER_0)), ==, + qtest_readl(test, TIMER_ADDR(A_COMPARE_LOWER0))); + g_assert_cmpuint(qtest_readl(test, TIMER_ADDR(A_UPPER_0)), ==, + qtest_readl(test, TIMER_ADDR(A_COMPARE_UPPER0))); + g_assert(qtest_get_irq(test, TIMER_EXPIRED_IRQ)); + + qtest_quit(test); +} + +int main(int argc, char **argv) +{ + g_test_init(&argc, &argv, NULL); + qtest_add_func("ibex-timer/reset", test_reset); + qtest_add_func("ibex-timer/writes", test_writes); + qtest_add_func("ibex-timer/op", test_operation); + return g_test_run(); +} diff --git a/tests/qtest/meson.build b/tests/qtest/meson.build index fb63b8d3fa..7a769a79c5 100644 --- a/tests/qtest/meson.build +++ b/tests/qtest/meson.build @@ -235,7 +235,8 @@ qtests_s390x = \ 'migration-test'] qtests_riscv32 = \ - ['ibex-aon-timer-test'] + ['ibex-aon-timer-test', + 'ibex-timer-test'] qos_test_ss = ss.source_set() qos_test_ss.add(
1. Adds fields to hold the value of mtime in timer_upper0 and timer_lower0. 2. Changes the read and write functions to use the mtime fields. 3. Updates the value of mtime in update_mtime() by extrapolating the time elapsed. This will need to change if/when the prescalar is implemented. 4. Adds a qtest for the ibex timer. Signed-off-by: Tyler Ng <tkng@rivosinc.com> --- hw/timer/ibex_timer.c | 98 +++++++++++++------ include/hw/timer/ibex_timer.h | 6 ++ tests/qtest/ibex-timer-test.c | 178 ++++++++++++++++++++++++++++++++++ tests/qtest/meson.build | 3 +- 4 files changed, 256 insertions(+), 29 deletions(-) create mode 100644 tests/qtest/ibex-timer-test.c -- 2.34.1