| Message ID | 113c061817fc2caa934930182da7ab63@m101.nthu.edu.tw (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v2] hw/intc/sifive_clint: Fix expiration time logic | expand |
On Sun, Aug 29, 2021 at 12:27 PM s101062801 <s101062801@m101.nthu.edu.tw> wrote: > > After timecmp is modified, the value is converted into nanosecond, > and pass to timer_mod. However, timer_mod perceives the value as > a signed integer. An example that goes wrong is as follows: > > OpenSBI v0.9 initializes the cold boot hart's timecmp to > 0xffffffff_ffffffff. timer_mod then interprets the product of the > following calculation as a negative value. As a result, the clint > timer is pulled out of timerlist because it looks like it has > expired, which cause the MIP.MTIP is set before any real timer > interrupt. > > Resolves: https://gitlab.com/qemu-project/qemu/-/issues/493 > Signed-off-by: Quey-Liang Kao <s101062801@m101.nthu.edu.tw> > --- > v2: > - Fix the calculation for next. > - Link to issue 493. > - I saw David's after I made this patch. Yet I want to correct the > error > in v1. Hello, Thanks for the patch! As David's was sent first I am going to apply that instead of this one. Sorry about that. Please feel free to send more QEMU patches in the future or to review other people's patches. Hopefully we will see you again :) Alistair > --- > hw/intc/sifive_clint.c | 9 ++++++--- > 1 file changed, 6 insertions(+), 3 deletions(-) > > diff --git a/hw/intc/sifive_clint.c b/hw/intc/sifive_clint.c > index 0f41e5ea1c..78f01d17d3 100644 > --- a/hw/intc/sifive_clint.c > +++ b/hw/intc/sifive_clint.c > @@ -44,6 +44,7 @@ static void sifive_clint_write_timecmp(RISCVCPU *cpu, > uint64_t value, > { > uint64_t next; > uint64_t diff; > + uint64_t now; > > uint64_t rtc_r = cpu_riscv_read_rtc(timebase_freq); > > @@ -59,9 +60,11 @@ static void sifive_clint_write_timecmp(RISCVCPU *cpu, > uint64_t value, > riscv_cpu_update_mip(cpu, MIP_MTIP, BOOL_TO_MASK(0)); > diff = cpu->env.timecmp - rtc_r; > /* back to ns (note args switched in muldiv64) */ > - next = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL) + > - muldiv64(diff, NANOSECONDS_PER_SECOND, timebase_freq); > - timer_mod(cpu->env.timer, next); > + now = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL); > + next = now + muldiv64(diff, NANOSECONDS_PER_SECOND, timebase_freq); > + timer_mod(cpu->env.timer, (next <= now) ? > + (int64_t)((1ULL << 63) - 1) : > + next); > } > > /* > -- > 2.32.0 >
diff --git a/hw/intc/sifive_clint.c b/hw/intc/sifive_clint.c index 0f41e5ea1c..78f01d17d3 100644 --- a/hw/intc/sifive_clint.c +++ b/hw/intc/sifive_clint.c @@ -44,6 +44,7 @@ static void sifive_clint_write_timecmp(RISCVCPU *cpu, uint64_t value, { uint64_t next; uint64_t diff; + uint64_t now; uint64_t rtc_r = cpu_riscv_read_rtc(timebase_freq);
After timecmp is modified, the value is converted into nanosecond, and pass to timer_mod. However, timer_mod perceives the value as a signed integer. An example that goes wrong is as follows: OpenSBI v0.9 initializes the cold boot hart's timecmp to 0xffffffff_ffffffff. timer_mod then interprets the product of the following calculation as a negative value. As a result, the clint timer is pulled out of timerlist because it looks like it has expired, which cause the MIP.MTIP is set before any real timer interrupt. Resolves: https://gitlab.com/qemu-project/qemu/-/issues/493 Signed-off-by: Quey-Liang Kao <s101062801@m101.nthu.edu.tw> --- v2: - Fix the calculation for next. - Link to issue 493. - I saw David's after I made this patch. Yet I want to correct the error in v1. --- hw/intc/sifive_clint.c | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) @@ -59,9 +60,11 @@ static void sifive_clint_write_timecmp(RISCVCPU *cpu, uint64_t value, riscv_cpu_update_mip(cpu, MIP_MTIP, BOOL_TO_MASK(0)); diff = cpu->env.timecmp - rtc_r; /* back to ns (note args switched in muldiv64) */ - next = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL) + - muldiv64(diff, NANOSECONDS_PER_SECOND, timebase_freq); - timer_mod(cpu->env.timer, next); + now = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL); + next = now + muldiv64(diff, NANOSECONDS_PER_SECOND, timebase_freq); + timer_mod(cpu->env.timer, (next <= now) ? + (int64_t)((1ULL << 63) - 1) : + next); } /* -- 2.32.0