* [RFC PATCH] hw/intc: Make RISC-V ACLINT mtime MMIO register writable
@ 2022-01-26 9:54 frank.chang
2022-02-01 2:34 ` Alistair Francis
0 siblings, 1 reply; 3+ messages in thread
From: frank.chang @ 2022-01-26 9:54 UTC (permalink / raw)
To: qemu-devel
Cc: Anup Patel, qemu-riscv, Frank Chang, Bin Meng, Palmer Dabbelt,
Alistair Francis, LIU Zhiwei
From: Frank Chang <frank.chang@sifive.com>
RISC-V privilege spec defines that mtime is exposed as a memory-mapped
machine-mode read-write register. However, as QEMU uses host monotonic
timer as timer source, this makes mtime to be read-only in RISC-V
ACLINT.
This patch makes mtime to be writable by recording the time delta value
between the mtime value to be written and the timer value at the time
mtime is written. Time delta value is then added back whenever the timer
value is retrieved.
Signed-off-by: Frank Chang <frank.chang@sifive.com>
---
hw/intc/riscv_aclint.c | 58 ++++++++++++++++++++++------------
include/hw/intc/riscv_aclint.h | 1 +
target/riscv/cpu.h | 8 ++---
target/riscv/cpu_helper.c | 4 +--
4 files changed, 44 insertions(+), 27 deletions(-)
diff --git a/hw/intc/riscv_aclint.c b/hw/intc/riscv_aclint.c
index f1a5d3d284..ffbe211a3d 100644
--- a/hw/intc/riscv_aclint.c
+++ b/hw/intc/riscv_aclint.c
@@ -38,12 +38,18 @@ typedef struct riscv_aclint_mtimer_callback {
int num;
} riscv_aclint_mtimer_callback;
-static uint64_t cpu_riscv_read_rtc(uint32_t timebase_freq)
+static uint64_t cpu_riscv_read_rtc_raw(uint32_t timebase_freq)
{
return muldiv64(qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL),
timebase_freq, NANOSECONDS_PER_SECOND);
}
+static uint64_t cpu_riscv_read_rtc(void *opaque)
+{
+ RISCVAclintMTimerState *mtimer = opaque;
+ return cpu_riscv_read_rtc_raw(mtimer->timebase_freq) + mtimer->time_delta;
+}
+
/*
* Called when timecmp is written to update the QEMU timer or immediately
* trigger timer interrupt if mtimecmp <= current timer value.
@@ -51,13 +57,13 @@ static uint64_t cpu_riscv_read_rtc(uint32_t timebase_freq)
static void riscv_aclint_mtimer_write_timecmp(RISCVAclintMTimerState *mtimer,
RISCVCPU *cpu,
int hartid,
- uint64_t value,
- uint32_t timebase_freq)
+ uint64_t value)
{
+ uint32_t timebase_freq = mtimer->timebase_freq;
uint64_t next;
uint64_t diff;
- uint64_t rtc_r = cpu_riscv_read_rtc(timebase_freq);
+ uint64_t rtc_r = cpu_riscv_read_rtc(mtimer);
cpu->env.timecmp = value;
if (cpu->env.timecmp <= rtc_r) {
@@ -140,10 +146,10 @@ static uint64_t riscv_aclint_mtimer_read(void *opaque, hwaddr addr,
}
} else if (addr == mtimer->time_base) {
/* time_lo */
- return cpu_riscv_read_rtc(mtimer->timebase_freq) & 0xFFFFFFFF;
+ return cpu_riscv_read_rtc(mtimer) & 0xFFFFFFFF;
} else if (addr == mtimer->time_base + 4) {
/* time_hi */
- return (cpu_riscv_read_rtc(mtimer->timebase_freq) >> 32) & 0xFFFFFFFF;
+ return (cpu_riscv_read_rtc(mtimer) >> 32) & 0xFFFFFFFF;
}
qemu_log_mask(LOG_UNIMP,
@@ -156,6 +162,7 @@ static void riscv_aclint_mtimer_write(void *opaque, hwaddr addr,
uint64_t value, unsigned size)
{
RISCVAclintMTimerState *mtimer = opaque;
+ int i;
if (addr >= mtimer->timecmp_base &&
addr < (mtimer->timecmp_base + (mtimer->num_harts << 3))) {
@@ -170,31 +177,40 @@ static void riscv_aclint_mtimer_write(void *opaque, hwaddr addr,
/* timecmp_lo */
uint64_t timecmp_hi = env->timecmp >> 32;
riscv_aclint_mtimer_write_timecmp(mtimer, RISCV_CPU(cpu), hartid,
- timecmp_hi << 32 | (value & 0xFFFFFFFF),
- mtimer->timebase_freq);
+ timecmp_hi << 32 | (value & 0xFFFFFFFF));
return;
} else if ((addr & 0x7) == 4) {
/* timecmp_hi */
uint64_t timecmp_lo = env->timecmp;
riscv_aclint_mtimer_write_timecmp(mtimer, RISCV_CPU(cpu), hartid,
- value << 32 | (timecmp_lo & 0xFFFFFFFF),
- mtimer->timebase_freq);
+ value << 32 | (timecmp_lo & 0xFFFFFFFF));
} else {
qemu_log_mask(LOG_UNIMP,
"aclint-mtimer: invalid timecmp write: %08x",
(uint32_t)addr);
}
return;
- } else if (addr == mtimer->time_base) {
- /* time_lo */
- qemu_log_mask(LOG_UNIMP,
- "aclint-mtimer: time_lo write not implemented");
- return;
- } else if (addr == mtimer->time_base + 4) {
- /* time_hi */
- qemu_log_mask(LOG_UNIMP,
- "aclint-mtimer: time_hi write not implemented");
- return;
+ } else if (addr == mtimer->time_base || addr == mtimer->time_base + 4) {
+ uint64_t rtc_r = cpu_riscv_read_rtc_raw(mtimer->timebase_freq);
+
+ if (addr == mtimer->time_base) {
+ /* time_lo */
+ mtimer->time_delta = ((rtc_r & ~0xFFFFFFFFULL) | value) - rtc_r;
+ } else {
+ /* time_hi */
+ mtimer->time_delta = (value << 32 | (rtc_r & 0xFFFFFFFF)) - rtc_r;
+ }
+
+ /* Check if timer interrupt is triggered for each hart. */
+ for (i = 0; i < mtimer->num_harts; i++) {
+ CPUState *cpu = qemu_get_cpu(mtimer->hartid_base + i);
+ CPURISCVState *env = cpu ? cpu->env_ptr : NULL;
+ if (!env) {
+ continue;
+ }
+ riscv_aclint_mtimer_write_timecmp(mtimer, RISCV_CPU(cpu),
+ i, env->timecmp);
+ }
}
qemu_log_mask(LOG_UNIMP,
@@ -299,7 +315,7 @@ DeviceState *riscv_aclint_mtimer_create(hwaddr addr, hwaddr size,
continue;
}
if (provide_rdtime) {
- riscv_cpu_set_rdtime_fn(env, cpu_riscv_read_rtc, timebase_freq);
+ riscv_cpu_set_rdtime_fn(env, cpu_riscv_read_rtc, dev);
}
cb->s = RISCV_ACLINT_MTIMER(dev);
diff --git a/include/hw/intc/riscv_aclint.h b/include/hw/intc/riscv_aclint.h
index 229bd08d25..26d4048687 100644
--- a/include/hw/intc/riscv_aclint.h
+++ b/include/hw/intc/riscv_aclint.h
@@ -31,6 +31,7 @@
typedef struct RISCVAclintMTimerState {
/*< private >*/
SysBusDevice parent_obj;
+ uint64_t time_delta;
/*< public >*/
MemoryRegion mmio;
diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
index 55635d68d5..46cac9df76 100644
--- a/target/riscv/cpu.h
+++ b/target/riscv/cpu.h
@@ -249,8 +249,8 @@ struct CPURISCVState {
target_ulong mseccfg;
/* machine specific rdtime callback */
- uint64_t (*rdtime_fn)(uint32_t);
- uint32_t rdtime_fn_arg;
+ uint64_t (*rdtime_fn)(void *);
+ void *rdtime_fn_arg;
/* True if in debugger mode. */
bool debugger;
@@ -413,8 +413,8 @@ void riscv_cpu_swap_hypervisor_regs(CPURISCVState *env);
int riscv_cpu_claim_interrupts(RISCVCPU *cpu, uint32_t interrupts);
uint32_t riscv_cpu_update_mip(RISCVCPU *cpu, uint32_t mask, uint32_t value);
#define BOOL_TO_MASK(x) (-!!(x)) /* helper for riscv_cpu_update_mip value */
-void riscv_cpu_set_rdtime_fn(CPURISCVState *env, uint64_t (*fn)(uint32_t),
- uint32_t arg);
+void riscv_cpu_set_rdtime_fn(CPURISCVState *env, uint64_t (*fn)(void *),
+ void *arg);
#endif
void riscv_cpu_set_mode(CPURISCVState *env, target_ulong newpriv);
diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
index 327a2c4f1d..73bf1bafa7 100644
--- a/target/riscv/cpu_helper.c
+++ b/target/riscv/cpu_helper.c
@@ -345,8 +345,8 @@ uint32_t riscv_cpu_update_mip(RISCVCPU *cpu, uint32_t mask, uint32_t value)
return old;
}
-void riscv_cpu_set_rdtime_fn(CPURISCVState *env, uint64_t (*fn)(uint32_t),
- uint32_t arg)
+void riscv_cpu_set_rdtime_fn(CPURISCVState *env, uint64_t (*fn)(void *),
+ void *arg)
{
env->rdtime_fn = fn;
env->rdtime_fn_arg = arg;
--
2.31.1
^ permalink raw reply related [flat|nested] 3+ messages in thread* Re: [RFC PATCH] hw/intc: Make RISC-V ACLINT mtime MMIO register writable
2022-01-26 9:54 [RFC PATCH] hw/intc: Make RISC-V ACLINT mtime MMIO register writable frank.chang
@ 2022-02-01 2:34 ` Alistair Francis
2022-02-09 2:36 ` Frank Chang
0 siblings, 1 reply; 3+ messages in thread
From: Alistair Francis @ 2022-02-01 2:34 UTC (permalink / raw)
To: Frank Chang
Cc: Anup Patel, open list:RISC-V, Bin Meng,
qemu-devel@nongnu.org Developers, Palmer Dabbelt,
Alistair Francis, LIU Zhiwei
On Wed, Jan 26, 2022 at 7:55 PM <frank.chang@sifive.com> wrote:
>
> From: Frank Chang <frank.chang@sifive.com>
>
> RISC-V privilege spec defines that mtime is exposed as a memory-mapped
> machine-mode read-write register. However, as QEMU uses host monotonic
> timer as timer source, this makes mtime to be read-only in RISC-V
> ACLINT.
>
> This patch makes mtime to be writable by recording the time delta value
> between the mtime value to be written and the timer value at the time
> mtime is written. Time delta value is then added back whenever the timer
> value is retrieved.
>
> Signed-off-by: Frank Chang <frank.chang@sifive.com>
> ---
> hw/intc/riscv_aclint.c | 58 ++++++++++++++++++++++------------
> include/hw/intc/riscv_aclint.h | 1 +
> target/riscv/cpu.h | 8 ++---
> target/riscv/cpu_helper.c | 4 +--
> 4 files changed, 44 insertions(+), 27 deletions(-)
>
> diff --git a/hw/intc/riscv_aclint.c b/hw/intc/riscv_aclint.c
> index f1a5d3d284..ffbe211a3d 100644
> --- a/hw/intc/riscv_aclint.c
> +++ b/hw/intc/riscv_aclint.c
> @@ -38,12 +38,18 @@ typedef struct riscv_aclint_mtimer_callback {
> int num;
> } riscv_aclint_mtimer_callback;
>
> -static uint64_t cpu_riscv_read_rtc(uint32_t timebase_freq)
> +static uint64_t cpu_riscv_read_rtc_raw(uint32_t timebase_freq)
> {
> return muldiv64(qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL),
> timebase_freq, NANOSECONDS_PER_SECOND);
> }
>
> +static uint64_t cpu_riscv_read_rtc(void *opaque)
> +{
> + RISCVAclintMTimerState *mtimer = opaque;
> + return cpu_riscv_read_rtc_raw(mtimer->timebase_freq) + mtimer->time_delta;
> +}
> +
> /*
> * Called when timecmp is written to update the QEMU timer or immediately
> * trigger timer interrupt if mtimecmp <= current timer value.
> @@ -51,13 +57,13 @@ static uint64_t cpu_riscv_read_rtc(uint32_t timebase_freq)
> static void riscv_aclint_mtimer_write_timecmp(RISCVAclintMTimerState *mtimer,
> RISCVCPU *cpu,
> int hartid,
> - uint64_t value,
> - uint32_t timebase_freq)
> + uint64_t value)
> {
> + uint32_t timebase_freq = mtimer->timebase_freq;
> uint64_t next;
> uint64_t diff;
>
> - uint64_t rtc_r = cpu_riscv_read_rtc(timebase_freq);
> + uint64_t rtc_r = cpu_riscv_read_rtc(mtimer);
>
> cpu->env.timecmp = value;
> if (cpu->env.timecmp <= rtc_r) {
> @@ -140,10 +146,10 @@ static uint64_t riscv_aclint_mtimer_read(void *opaque, hwaddr addr,
> }
> } else if (addr == mtimer->time_base) {
> /* time_lo */
> - return cpu_riscv_read_rtc(mtimer->timebase_freq) & 0xFFFFFFFF;
> + return cpu_riscv_read_rtc(mtimer) & 0xFFFFFFFF;
> } else if (addr == mtimer->time_base + 4) {
> /* time_hi */
> - return (cpu_riscv_read_rtc(mtimer->timebase_freq) >> 32) & 0xFFFFFFFF;
> + return (cpu_riscv_read_rtc(mtimer) >> 32) & 0xFFFFFFFF;
> }
>
> qemu_log_mask(LOG_UNIMP,
> @@ -156,6 +162,7 @@ static void riscv_aclint_mtimer_write(void *opaque, hwaddr addr,
> uint64_t value, unsigned size)
> {
> RISCVAclintMTimerState *mtimer = opaque;
> + int i;
>
> if (addr >= mtimer->timecmp_base &&
> addr < (mtimer->timecmp_base + (mtimer->num_harts << 3))) {
> @@ -170,31 +177,40 @@ static void riscv_aclint_mtimer_write(void *opaque, hwaddr addr,
> /* timecmp_lo */
> uint64_t timecmp_hi = env->timecmp >> 32;
> riscv_aclint_mtimer_write_timecmp(mtimer, RISCV_CPU(cpu), hartid,
> - timecmp_hi << 32 | (value & 0xFFFFFFFF),
> - mtimer->timebase_freq);
> + timecmp_hi << 32 | (value & 0xFFFFFFFF));
> return;
> } else if ((addr & 0x7) == 4) {
> /* timecmp_hi */
> uint64_t timecmp_lo = env->timecmp;
> riscv_aclint_mtimer_write_timecmp(mtimer, RISCV_CPU(cpu), hartid,
> - value << 32 | (timecmp_lo & 0xFFFFFFFF),
> - mtimer->timebase_freq);
> + value << 32 | (timecmp_lo & 0xFFFFFFFF));
> } else {
> qemu_log_mask(LOG_UNIMP,
> "aclint-mtimer: invalid timecmp write: %08x",
> (uint32_t)addr);
> }
> return;
> - } else if (addr == mtimer->time_base) {
> - /* time_lo */
> - qemu_log_mask(LOG_UNIMP,
> - "aclint-mtimer: time_lo write not implemented");
> - return;
> - } else if (addr == mtimer->time_base + 4) {
> - /* time_hi */
> - qemu_log_mask(LOG_UNIMP,
> - "aclint-mtimer: time_hi write not implemented");
> - return;
> + } else if (addr == mtimer->time_base || addr == mtimer->time_base + 4) {
> + uint64_t rtc_r = cpu_riscv_read_rtc_raw(mtimer->timebase_freq);
> +
> + if (addr == mtimer->time_base) {
> + /* time_lo */
> + mtimer->time_delta = ((rtc_r & ~0xFFFFFFFFULL) | value) - rtc_r;
> + } else {
> + /* time_hi */
> + mtimer->time_delta = (value << 32 | (rtc_r & 0xFFFFFFFF)) - rtc_r;
> + }
We should be checking the size here, for RV64 a 64-bit memory access
to mtime or mtimecmp can occur.
Alistair
> +
> + /* Check if timer interrupt is triggered for each hart. */
> + for (i = 0; i < mtimer->num_harts; i++) {
> + CPUState *cpu = qemu_get_cpu(mtimer->hartid_base + i);
> + CPURISCVState *env = cpu ? cpu->env_ptr : NULL;
> + if (!env) {
> + continue;
> + }
> + riscv_aclint_mtimer_write_timecmp(mtimer, RISCV_CPU(cpu),
> + i, env->timecmp);
> + }
> }
>
> qemu_log_mask(LOG_UNIMP,
> @@ -299,7 +315,7 @@ DeviceState *riscv_aclint_mtimer_create(hwaddr addr, hwaddr size,
> continue;
> }
> if (provide_rdtime) {
> - riscv_cpu_set_rdtime_fn(env, cpu_riscv_read_rtc, timebase_freq);
> + riscv_cpu_set_rdtime_fn(env, cpu_riscv_read_rtc, dev);
> }
>
> cb->s = RISCV_ACLINT_MTIMER(dev);
> diff --git a/include/hw/intc/riscv_aclint.h b/include/hw/intc/riscv_aclint.h
> index 229bd08d25..26d4048687 100644
> --- a/include/hw/intc/riscv_aclint.h
> +++ b/include/hw/intc/riscv_aclint.h
> @@ -31,6 +31,7 @@
> typedef struct RISCVAclintMTimerState {
> /*< private >*/
> SysBusDevice parent_obj;
> + uint64_t time_delta;
>
> /*< public >*/
> MemoryRegion mmio;
> diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
> index 55635d68d5..46cac9df76 100644
> --- a/target/riscv/cpu.h
> +++ b/target/riscv/cpu.h
> @@ -249,8 +249,8 @@ struct CPURISCVState {
> target_ulong mseccfg;
>
> /* machine specific rdtime callback */
> - uint64_t (*rdtime_fn)(uint32_t);
> - uint32_t rdtime_fn_arg;
> + uint64_t (*rdtime_fn)(void *);
> + void *rdtime_fn_arg;
>
> /* True if in debugger mode. */
> bool debugger;
> @@ -413,8 +413,8 @@ void riscv_cpu_swap_hypervisor_regs(CPURISCVState *env);
> int riscv_cpu_claim_interrupts(RISCVCPU *cpu, uint32_t interrupts);
> uint32_t riscv_cpu_update_mip(RISCVCPU *cpu, uint32_t mask, uint32_t value);
> #define BOOL_TO_MASK(x) (-!!(x)) /* helper for riscv_cpu_update_mip value */
> -void riscv_cpu_set_rdtime_fn(CPURISCVState *env, uint64_t (*fn)(uint32_t),
> - uint32_t arg);
> +void riscv_cpu_set_rdtime_fn(CPURISCVState *env, uint64_t (*fn)(void *),
> + void *arg);
> #endif
> void riscv_cpu_set_mode(CPURISCVState *env, target_ulong newpriv);
>
> diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
> index 327a2c4f1d..73bf1bafa7 100644
> --- a/target/riscv/cpu_helper.c
> +++ b/target/riscv/cpu_helper.c
> @@ -345,8 +345,8 @@ uint32_t riscv_cpu_update_mip(RISCVCPU *cpu, uint32_t mask, uint32_t value)
> return old;
> }
>
> -void riscv_cpu_set_rdtime_fn(CPURISCVState *env, uint64_t (*fn)(uint32_t),
> - uint32_t arg)
> +void riscv_cpu_set_rdtime_fn(CPURISCVState *env, uint64_t (*fn)(void *),
> + void *arg)
> {
> env->rdtime_fn = fn;
> env->rdtime_fn_arg = arg;
> --
> 2.31.1
>
>
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [RFC PATCH] hw/intc: Make RISC-V ACLINT mtime MMIO register writable
2022-02-01 2:34 ` Alistair Francis
@ 2022-02-09 2:36 ` Frank Chang
0 siblings, 0 replies; 3+ messages in thread
From: Frank Chang @ 2022-02-09 2:36 UTC (permalink / raw)
To: Alistair Francis
Cc: Anup Patel, open list:RISC-V, Bin Meng,
qemu-devel@nongnu.org Developers, Palmer Dabbelt,
Alistair Francis, LIU Zhiwei
[-- Attachment #1: Type: text/plain, Size: 9428 bytes --]
On Tue, Feb 1, 2022 at 10:34 AM Alistair Francis <alistair23@gmail.com>
wrote:
> On Wed, Jan 26, 2022 at 7:55 PM <frank.chang@sifive.com> wrote:
> >
> > From: Frank Chang <frank.chang@sifive.com>
> >
> > RISC-V privilege spec defines that mtime is exposed as a memory-mapped
> > machine-mode read-write register. However, as QEMU uses host monotonic
> > timer as timer source, this makes mtime to be read-only in RISC-V
> > ACLINT.
> >
> > This patch makes mtime to be writable by recording the time delta value
> > between the mtime value to be written and the timer value at the time
> > mtime is written. Time delta value is then added back whenever the timer
> > value is retrieved.
> >
> > Signed-off-by: Frank Chang <frank.chang@sifive.com>
> > ---
> > hw/intc/riscv_aclint.c | 58 ++++++++++++++++++++++------------
> > include/hw/intc/riscv_aclint.h | 1 +
> > target/riscv/cpu.h | 8 ++---
> > target/riscv/cpu_helper.c | 4 +--
> > 4 files changed, 44 insertions(+), 27 deletions(-)
> >
> > diff --git a/hw/intc/riscv_aclint.c b/hw/intc/riscv_aclint.c
> > index f1a5d3d284..ffbe211a3d 100644
> > --- a/hw/intc/riscv_aclint.c
> > +++ b/hw/intc/riscv_aclint.c
> > @@ -38,12 +38,18 @@ typedef struct riscv_aclint_mtimer_callback {
> > int num;
> > } riscv_aclint_mtimer_callback;
> >
> > -static uint64_t cpu_riscv_read_rtc(uint32_t timebase_freq)
> > +static uint64_t cpu_riscv_read_rtc_raw(uint32_t timebase_freq)
> > {
> > return muldiv64(qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL),
> > timebase_freq, NANOSECONDS_PER_SECOND);
> > }
> >
> > +static uint64_t cpu_riscv_read_rtc(void *opaque)
> > +{
> > + RISCVAclintMTimerState *mtimer = opaque;
> > + return cpu_riscv_read_rtc_raw(mtimer->timebase_freq) +
> mtimer->time_delta;
> > +}
> > +
> > /*
> > * Called when timecmp is written to update the QEMU timer or
> immediately
> > * trigger timer interrupt if mtimecmp <= current timer value.
> > @@ -51,13 +57,13 @@ static uint64_t cpu_riscv_read_rtc(uint32_t
> timebase_freq)
> > static void riscv_aclint_mtimer_write_timecmp(RISCVAclintMTimerState
> *mtimer,
> > RISCVCPU *cpu,
> > int hartid,
> > - uint64_t value,
> > - uint32_t timebase_freq)
> > + uint64_t value)
> > {
> > + uint32_t timebase_freq = mtimer->timebase_freq;
> > uint64_t next;
> > uint64_t diff;
> >
> > - uint64_t rtc_r = cpu_riscv_read_rtc(timebase_freq);
> > + uint64_t rtc_r = cpu_riscv_read_rtc(mtimer);
> >
> > cpu->env.timecmp = value;
> > if (cpu->env.timecmp <= rtc_r) {
> > @@ -140,10 +146,10 @@ static uint64_t riscv_aclint_mtimer_read(void
> *opaque, hwaddr addr,
> > }
> > } else if (addr == mtimer->time_base) {
> > /* time_lo */
> > - return cpu_riscv_read_rtc(mtimer->timebase_freq) & 0xFFFFFFFF;
> > + return cpu_riscv_read_rtc(mtimer) & 0xFFFFFFFF;
> > } else if (addr == mtimer->time_base + 4) {
> > /* time_hi */
> > - return (cpu_riscv_read_rtc(mtimer->timebase_freq) >> 32) &
> 0xFFFFFFFF;
> > + return (cpu_riscv_read_rtc(mtimer) >> 32) & 0xFFFFFFFF;
> > }
> >
> > qemu_log_mask(LOG_UNIMP,
> > @@ -156,6 +162,7 @@ static void riscv_aclint_mtimer_write(void *opaque,
> hwaddr addr,
> > uint64_t value, unsigned size)
> > {
> > RISCVAclintMTimerState *mtimer = opaque;
> > + int i;
> >
> > if (addr >= mtimer->timecmp_base &&
> > addr < (mtimer->timecmp_base + (mtimer->num_harts << 3))) {
> > @@ -170,31 +177,40 @@ static void riscv_aclint_mtimer_write(void
> *opaque, hwaddr addr,
> > /* timecmp_lo */
> > uint64_t timecmp_hi = env->timecmp >> 32;
> > riscv_aclint_mtimer_write_timecmp(mtimer, RISCV_CPU(cpu),
> hartid,
> > - timecmp_hi << 32 | (value & 0xFFFFFFFF),
> > - mtimer->timebase_freq);
> > + timecmp_hi << 32 | (value & 0xFFFFFFFF));
> > return;
> > } else if ((addr & 0x7) == 4) {
> > /* timecmp_hi */
> > uint64_t timecmp_lo = env->timecmp;
> > riscv_aclint_mtimer_write_timecmp(mtimer, RISCV_CPU(cpu),
> hartid,
> > - value << 32 | (timecmp_lo & 0xFFFFFFFF),
> > - mtimer->timebase_freq);
> > + value << 32 | (timecmp_lo & 0xFFFFFFFF));
> > } else {
> > qemu_log_mask(LOG_UNIMP,
> > "aclint-mtimer: invalid timecmp write: %08x",
> > (uint32_t)addr);
> > }
> > return;
> > - } else if (addr == mtimer->time_base) {
> > - /* time_lo */
> > - qemu_log_mask(LOG_UNIMP,
> > - "aclint-mtimer: time_lo write not implemented");
> > - return;
> > - } else if (addr == mtimer->time_base + 4) {
> > - /* time_hi */
> > - qemu_log_mask(LOG_UNIMP,
> > - "aclint-mtimer: time_hi write not implemented");
> > - return;
> > + } else if (addr == mtimer->time_base || addr == mtimer->time_base +
> 4) {
> > + uint64_t rtc_r = cpu_riscv_read_rtc_raw(mtimer->timebase_freq);
> > +
> > + if (addr == mtimer->time_base) {
> > + /* time_lo */
> > + mtimer->time_delta = ((rtc_r & ~0xFFFFFFFFULL) | value) -
> rtc_r;
> > + } else {
> > + /* time_hi */
> > + mtimer->time_delta = (value << 32 | (rtc_r & 0xFFFFFFFF)) -
> rtc_r;
> > + }
>
> We should be checking the size here, for RV64 a 64-bit memory access
> to mtime or mtimecmp can occur.
>
> Alistair
>
From Privilege v1.12 spec:
In RV32, memory-mapped writes to mtimecmp modify only one 32-bit part of
the register.
For RV64, naturally aligned 64-bit memory accesses to the mtime and
mtimecmp registers are
additionally supported and are atomic.
It seems that both mtimecmp and mtime should check the sizes of 4 and 8-bit
memory accesses.
I will refine that in my next patch.
Regards,
Frank Chang
>
> > +
> > + /* Check if timer interrupt is triggered for each hart. */
> > + for (i = 0; i < mtimer->num_harts; i++) {
> > + CPUState *cpu = qemu_get_cpu(mtimer->hartid_base + i);
> > + CPURISCVState *env = cpu ? cpu->env_ptr : NULL;
> > + if (!env) {
> > + continue;
> > + }
> > + riscv_aclint_mtimer_write_timecmp(mtimer, RISCV_CPU(cpu),
> > + i, env->timecmp);
> > + }
> > }
> >
> > qemu_log_mask(LOG_UNIMP,
> > @@ -299,7 +315,7 @@ DeviceState *riscv_aclint_mtimer_create(hwaddr addr,
> hwaddr size,
> > continue;
> > }
> > if (provide_rdtime) {
> > - riscv_cpu_set_rdtime_fn(env, cpu_riscv_read_rtc,
> timebase_freq);
> > + riscv_cpu_set_rdtime_fn(env, cpu_riscv_read_rtc, dev);
> > }
> >
> > cb->s = RISCV_ACLINT_MTIMER(dev);
> > diff --git a/include/hw/intc/riscv_aclint.h
> b/include/hw/intc/riscv_aclint.h
> > index 229bd08d25..26d4048687 100644
> > --- a/include/hw/intc/riscv_aclint.h
> > +++ b/include/hw/intc/riscv_aclint.h
> > @@ -31,6 +31,7 @@
> > typedef struct RISCVAclintMTimerState {
> > /*< private >*/
> > SysBusDevice parent_obj;
> > + uint64_t time_delta;
> >
> > /*< public >*/
> > MemoryRegion mmio;
> > diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
> > index 55635d68d5..46cac9df76 100644
> > --- a/target/riscv/cpu.h
> > +++ b/target/riscv/cpu.h
> > @@ -249,8 +249,8 @@ struct CPURISCVState {
> > target_ulong mseccfg;
> >
> > /* machine specific rdtime callback */
> > - uint64_t (*rdtime_fn)(uint32_t);
> > - uint32_t rdtime_fn_arg;
> > + uint64_t (*rdtime_fn)(void *);
> > + void *rdtime_fn_arg;
> >
> > /* True if in debugger mode. */
> > bool debugger;
> > @@ -413,8 +413,8 @@ void riscv_cpu_swap_hypervisor_regs(CPURISCVState
> *env);
> > int riscv_cpu_claim_interrupts(RISCVCPU *cpu, uint32_t interrupts);
> > uint32_t riscv_cpu_update_mip(RISCVCPU *cpu, uint32_t mask, uint32_t
> value);
> > #define BOOL_TO_MASK(x) (-!!(x)) /* helper for riscv_cpu_update_mip
> value */
> > -void riscv_cpu_set_rdtime_fn(CPURISCVState *env, uint64_t
> (*fn)(uint32_t),
> > - uint32_t arg);
> > +void riscv_cpu_set_rdtime_fn(CPURISCVState *env, uint64_t (*fn)(void *),
> > + void *arg);
> > #endif
> > void riscv_cpu_set_mode(CPURISCVState *env, target_ulong newpriv);
> >
> > diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
> > index 327a2c4f1d..73bf1bafa7 100644
> > --- a/target/riscv/cpu_helper.c
> > +++ b/target/riscv/cpu_helper.c
> > @@ -345,8 +345,8 @@ uint32_t riscv_cpu_update_mip(RISCVCPU *cpu,
> uint32_t mask, uint32_t value)
> > return old;
> > }
> >
> > -void riscv_cpu_set_rdtime_fn(CPURISCVState *env, uint64_t
> (*fn)(uint32_t),
> > - uint32_t arg)
> > +void riscv_cpu_set_rdtime_fn(CPURISCVState *env, uint64_t (*fn)(void *),
> > + void *arg)
> > {
> > env->rdtime_fn = fn;
> > env->rdtime_fn_arg = arg;
> > --
> > 2.31.1
> >
> >
>
[-- Attachment #2: Type: text/html, Size: 12333 bytes --]
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2022-02-09 2:40 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2022-01-26 9:54 [RFC PATCH] hw/intc: Make RISC-V ACLINT mtime MMIO register writable frank.chang
2022-02-01 2:34 ` Alistair Francis
2022-02-09 2:36 ` Frank Chang
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).