On Wed, Jan 30, 2019 at 10:53 AM Fabien Chouteau <chout...@adacore.com> wrote:
>
> Writing a high value in timecmp leads to an integer overflow. This patch
> modifies the code to detect such case, and use the maximum integer value
> as the next trigger for the timer.
>
> Signed-off-by: Fabien Chouteau <chout...@adacore.com>
> ---
> hw/riscv/sifive_clint.c | 31 ++++++++++++++++++++++++++-----
> 1 file changed, 26 insertions(+), 5 deletions(-)
>
> diff --git a/hw/riscv/sifive_clint.c b/hw/riscv/sifive_clint.c
> index d4c159e937..1ca1f8c75e 100644
> --- a/hw/riscv/sifive_clint.c
> +++ b/hw/riscv/sifive_clint.c
> @@ -38,8 +38,10 @@ static uint64_t cpu_riscv_read_rtc(void)
> */
> static void sifive_clint_write_timecmp(RISCVCPU *cpu, uint64_t value)
> {
> - uint64_t next;
> uint64_t diff;
> + uint64_t lapse_ns;
> + uint64_t clock_ns;
> + int64_t next_ns;
>
> uint64_t rtc_r = cpu_riscv_read_rtc();
>
> @@ -54,10 +56,29 @@ static void sifive_clint_write_timecmp(RISCVCPU *cpu,
> uint64_t value)
> /* otherwise, set up the future timer interrupt */
> 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, SIFIVE_CLINT_TIMEBASE_FREQ);
> - timer_mod(cpu->env.timer, next);
> +
> + /*
> + * How many nanoseconds until the next trigger (note args switched in
> + * muldiv64)
> + */
> + lapse_ns = muldiv64(diff,
> + NANOSECONDS_PER_SECOND,
> + SIFIVE_CLINT_TIMEBASE_FREQ);
> +
> + /* Current time in nanoseconds */
> + clock_ns = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL);
> +
> + if ((G_MAXINT64 - clock_ns) <= lapse_ns) {
> + /*
> + * clock + lapse would overflow on 64bit. The highest 64bit value is
> + * used as the next trigger time.
> + */
> + next_ns = G_MAXINT64;
> + } else {
> + next_ns = clock_ns + lapse_ns;
> + }
Can you describe what this fixes?
Won't an overflow be ok as we then just wrap around anyway? I guess
there is a problem if we want a value so large that we wrap around
past our current time though.
Alistair
> +
> + timer_mod(cpu->env.timer, next_ns);
> }
>
> /*
> --
> 2.17.1
>
>
