Replace the env->rdtime_fn function pointer with an instance of the
RISCVCPUTimeSrcIf QOM interface.
This allows to remove the dependency on the ACLINT in the
riscv_timer_write_timecmp function:
- This dependency was buggy because env->rdtime_fn_arg was an opaque
pointer and was converted in riscv_timer_write_timecmp to a ACLINT
without dynamic type check.
- This will allow to have time sources provided by other devices than
an ACLINT.
Signed-off-by: Luc Michel <luc.michel@amd.com>
---
target/riscv/cpu.h | 8 +++-----
hw/intc/riscv_aclint.c | 2 +-
target/riscv/cpu_helper.c | 7 -------
target/riscv/csr.c | 24 ++++++++++++------------
target/riscv/time_helper.c | 15 +++++++++------
5 files changed, 25 insertions(+), 31 deletions(-)
diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
index 36e7f100374..368b9e2532d 100644
--- a/target/riscv/cpu.h
+++ b/target/riscv/cpu.h
@@ -458,13 +458,12 @@ struct CPUArchState {
struct CPUWatchpoint *cpu_watchpoint[RV_MAX_TRIGGERS];
QEMUTimer *itrigger_timer[RV_MAX_TRIGGERS];
int64_t last_icount;
bool itrigger_enabled;
- /* machine specific rdtime callback */
- uint64_t (*rdtime_fn)(void *);
- void *rdtime_fn_arg;
+ /* machine specific time source interface */
+ RISCVCPUTimeSrcIf *time_src;
/* machine specific AIA ireg read-modify-write callback */
#define AIA_MAKE_IREG(__isel, __priv, __virt, __vgein, __xlen) \
((((__xlen) & 0xff) << 24) | \
(((__vgein) & 0x3f) << 20) | \
@@ -642,12 +641,11 @@ int riscv_cpu_claim_interrupts(RISCVCPU *cpu, uint64_t interrupts);
uint64_t riscv_cpu_update_mip(CPURISCVState *env, uint64_t mask,
uint64_t value);
void riscv_cpu_set_rnmi(RISCVCPU *cpu, uint32_t irq, bool level);
void riscv_cpu_interrupt(CPURISCVState *env);
#define BOOL_TO_MASK(x) (-!!(x)) /* helper for riscv_cpu_update_mip value */
-void riscv_cpu_set_rdtime_fn(CPURISCVState *env, uint64_t (*fn)(void *),
- void *arg);
+void riscv_cpu_set_time_src(CPURISCVState *env, RISCVCPUTimeSrcIf *src);
void riscv_cpu_set_aia_ireg_rmw_fn(CPURISCVState *env, uint32_t priv,
int (*rmw_fn)(void *arg,
target_ulong reg,
target_ulong *val,
target_ulong new_val,
diff --git a/hw/intc/riscv_aclint.c b/hw/intc/riscv_aclint.c
index 4a4449d9d2f..8d001a5eb20 100644
--- a/hw/intc/riscv_aclint.c
+++ b/hw/intc/riscv_aclint.c
@@ -418,11 +418,11 @@ DeviceState *riscv_aclint_mtimer_create(hwaddr addr, hwaddr size,
if (!env) {
g_free(cb);
continue;
}
if (provide_rdtime) {
- riscv_cpu_set_rdtime_fn(env, riscv_aclint_mtimer_get_ticks, dev);
+ riscv_cpu_set_time_src(env, RISCV_CPU_TIME_SRC_IF(dev));
}
cb->s = s;
cb->num = i;
s->timers[i] = timer_new_ns(QEMU_CLOCK_VIRTUAL,
diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
index c4fb68b5de8..f642ec80b31 100644
--- a/target/riscv/cpu_helper.c
+++ b/target/riscv/cpu_helper.c
@@ -757,17 +757,10 @@ uint64_t riscv_cpu_update_mip(CPURISCVState *env, uint64_t mask, uint64_t value)
riscv_cpu_interrupt(env);
return old;
}
-void riscv_cpu_set_rdtime_fn(CPURISCVState *env, uint64_t (*fn)(void *),
- void *arg)
-{
- env->rdtime_fn = fn;
- env->rdtime_fn_arg = arg;
-}
-
void riscv_cpu_set_aia_ireg_rmw_fn(CPURISCVState *env, uint32_t priv,
int (*rmw_fn)(void *arg,
target_ulong reg,
target_ulong *val,
target_ulong new_val,
diff --git a/target/riscv/csr.c b/target/riscv/csr.c
index 5c91658c3dc..583f646460c 100644
--- a/target/riscv/csr.c
+++ b/target/riscv/csr.c
@@ -572,11 +572,11 @@ static RISCVException sstateen(CPURISCVState *env, int csrno)
static RISCVException sstc(CPURISCVState *env, int csrno)
{
bool hmode_check = false;
- if (!riscv_cpu_cfg(env)->ext_sstc || !env->rdtime_fn) {
+ if (!riscv_cpu_cfg(env)->ext_sstc || !env->time_src) {
return RISCV_EXCP_ILLEGAL_INST;
}
if ((csrno == CSR_VSTIMECMP) || (csrno == CSR_VSTIMECMPH)) {
hmode_check = true;
@@ -1634,28 +1634,28 @@ static RISCVException read_scountovf(CPURISCVState *env, int csrno,
static RISCVException read_time(CPURISCVState *env, int csrno,
target_ulong *val)
{
uint64_t delta = env->virt_enabled ? env->htimedelta : 0;
- if (!env->rdtime_fn) {
+ if (!env->time_src) {
return RISCV_EXCP_ILLEGAL_INST;
}
- *val = env->rdtime_fn(env->rdtime_fn_arg) + delta;
+ *val = riscv_cpu_time_src_get_ticks(env->time_src) + delta;
return RISCV_EXCP_NONE;
}
static RISCVException read_timeh(CPURISCVState *env, int csrno,
target_ulong *val)
{
uint64_t delta = env->virt_enabled ? env->htimedelta : 0;
- if (!env->rdtime_fn) {
+ if (!env->time_src) {
return RISCV_EXCP_ILLEGAL_INST;
}
- *val = (env->rdtime_fn(env->rdtime_fn_arg) + delta) >> 32;
+ *val = (riscv_cpu_time_src_get_ticks(env->time_src) + delta) >> 32;
return RISCV_EXCP_NONE;
}
static RISCVException read_vstimecmp(CPURISCVState *env, int csrno,
target_ulong *val)
@@ -4965,60 +4965,60 @@ static RISCVException write_hgatp(CPURISCVState *env, int csrno,
}
static RISCVException read_htimedelta(CPURISCVState *env, int csrno,
target_ulong *val)
{
- if (!env->rdtime_fn) {
+ if (!env->time_src) {
return RISCV_EXCP_ILLEGAL_INST;
}
*val = env->htimedelta;
return RISCV_EXCP_NONE;
}
static RISCVException write_htimedelta(CPURISCVState *env, int csrno,
target_ulong val, uintptr_t ra)
{
- if (!env->rdtime_fn) {
+ if (!env->time_src) {
return RISCV_EXCP_ILLEGAL_INST;
}
if (riscv_cpu_mxl(env) == MXL_RV32) {
env->htimedelta = deposit64(env->htimedelta, 0, 32, (uint64_t)val);
} else {
env->htimedelta = val;
}
- if (riscv_cpu_cfg(env)->ext_sstc && env->rdtime_fn) {
+ if (riscv_cpu_cfg(env)->ext_sstc) {
riscv_timer_write_timecmp(env, env->vstimer, env->vstimecmp,
env->htimedelta, MIP_VSTIP);
}
return RISCV_EXCP_NONE;
}
static RISCVException read_htimedeltah(CPURISCVState *env, int csrno,
target_ulong *val)
{
- if (!env->rdtime_fn) {
+ if (!env->time_src) {
return RISCV_EXCP_ILLEGAL_INST;
}
*val = env->htimedelta >> 32;
return RISCV_EXCP_NONE;
}
static RISCVException write_htimedeltah(CPURISCVState *env, int csrno,
target_ulong val, uintptr_t ra)
{
- if (!env->rdtime_fn) {
+ if (!env->time_src) {
return RISCV_EXCP_ILLEGAL_INST;
}
env->htimedelta = deposit64(env->htimedelta, 32, 32, (uint64_t)val);
- if (riscv_cpu_cfg(env)->ext_sstc && env->rdtime_fn) {
+ if (riscv_cpu_cfg(env)->ext_sstc) {
riscv_timer_write_timecmp(env, env->vstimer, env->vstimecmp,
env->htimedelta, MIP_VSTIP);
}
return RISCV_EXCP_NONE;
@@ -5825,11 +5825,11 @@ riscv_csr_operations csr_ops[CSR_TABLE_SIZE] = {
[CSR_CYCLEH] = { "cycleh", ctr32, read_hpmcounterh },
[CSR_INSTRETH] = { "instreth", ctr32, read_hpmcounterh },
/*
* In privileged mode, the monitor will have to emulate TIME CSRs only if
- * rdtime callback is not provided by machine/platform emulation.
+ * CPU time source interface is not provided by machine/platform emulation.
*/
[CSR_TIME] = { "time", ctr, read_time },
[CSR_TIMEH] = { "timeh", ctr32, read_timeh },
/* Crypto Extension */
diff --git a/target/riscv/time_helper.c b/target/riscv/time_helper.c
index dc0777607ab..7b493b7a233 100644
--- a/target/riscv/time_helper.c
+++ b/target/riscv/time_helper.c
@@ -17,11 +17,10 @@
*/
#include "qemu/osdep.h"
#include "cpu_bits.h"
#include "time_helper.h"
-#include "hw/intc/riscv_aclint.h"
static void riscv_vstimer_cb(void *opaque)
{
RISCVCPU *cpu = opaque;
CPURISCVState *env = &cpu->env;
@@ -42,28 +41,27 @@ static void riscv_stimer_cb(void *opaque)
void riscv_timer_write_timecmp(CPURISCVState *env, QEMUTimer *timer,
uint64_t timecmp, uint64_t delta,
uint32_t timer_irq)
{
uint64_t diff, ns_diff, next;
- RISCVAclintMTimerState *mtimer = env->rdtime_fn_arg;
uint32_t timebase_freq;
uint64_t rtc_r;
- if (!riscv_cpu_cfg(env)->ext_sstc || !env->rdtime_fn ||
- !env->rdtime_fn_arg || !get_field(env->menvcfg, MENVCFG_STCE)) {
+ if (!riscv_cpu_cfg(env)->ext_sstc || !env->time_src ||
+ !get_field(env->menvcfg, MENVCFG_STCE)) {
/* S/VS Timer IRQ depends on sstc extension, rdtime_fn(), and STCE. */
return;
}
if (timer_irq == MIP_VSTIP &&
(!riscv_has_ext(env, RVH) || !get_field(env->henvcfg, HENVCFG_STCE))) {
/* VS Timer IRQ also depends on RVH and henvcfg.STCE. */
return;
}
- timebase_freq = mtimer->timebase_freq;
- rtc_r = env->rdtime_fn(env->rdtime_fn_arg) + delta;
+ timebase_freq = riscv_cpu_time_src_get_tick_freq(env->time_src);
+ rtc_r = riscv_cpu_time_src_get_ticks(env->time_src) + delta;
if (timecmp <= rtc_r) {
/*
* If we're setting an stimecmp value in the "past",
* immediately raise the timer interrupt
@@ -199,10 +197,15 @@ void riscv_timer_init(RISCVCPU *cpu)
env->vstimer = timer_new_ns(QEMU_CLOCK_VIRTUAL, &riscv_vstimer_cb, cpu);
env->vstimecmp = 0;
}
+void riscv_cpu_set_time_src(CPURISCVState *env, RISCVCPUTimeSrcIf *src)
+{
+ env->time_src = src;
+}
+
static const TypeInfo riscv_cpu_time_src_if_info = {
.name = TYPE_RISCV_CPU_TIME_SRC_IF,
.parent = TYPE_INTERFACE,
.class_size = sizeof(RISCVCPUTimeSrcIfClass),
};
--
2.51.0On 7/11/25 11:23, Luc Michel wrote:
> Replace the env->rdtime_fn function pointer with an instance of the
> RISCVCPUTimeSrcIf QOM interface.
>
> This allows to remove the dependency on the ACLINT in the
> riscv_timer_write_timecmp function:
> - This dependency was buggy because env->rdtime_fn_arg was an opaque
> pointer and was converted in riscv_timer_write_timecmp to a ACLINT
> without dynamic type check.
> - This will allow to have time sources provided by other devices than
> an ACLINT.
>
> Signed-off-by: Luc Michel <luc.michel@amd.com>
> ---
> target/riscv/cpu.h | 8 +++-----
> hw/intc/riscv_aclint.c | 2 +-
> target/riscv/cpu_helper.c | 7 -------
> target/riscv/csr.c | 24 ++++++++++++------------
> target/riscv/time_helper.c | 15 +++++++++------
> 5 files changed, 25 insertions(+), 31 deletions(-)
> +void riscv_cpu_set_time_src(CPURISCVState *env, RISCVCPUTimeSrcIf *src)
> +{
> + env->time_src = src;
Worth asserting time_src is NULL? Regardless, good cleanup:
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> +}
© 2016 - 2025 Red Hat, Inc.