On Tue, May 22, 2018 at 5:15 PM, Michael Clark <m...@sifive.com> wrote:
> A missing shift made updates to the low order bits
> of timecmp erroneously copy the old low order bits
> into the high order bits of the 64-bit timecmp
> register. Add the missing shift and rename timecmp
> local variables to timecmp_hi and timecmp_lo.
>
> This bug didn't show up as the low order bits are
> usually written first followed by the high order
> bits meaning the high order bits contained an invalid
> value between the timecmp_lo and timecmp_hi update.
>
> Cc: Palmer Dabbelt <pal...@sifive.com>
> Cc: Sagar Karandikar <sag...@eecs.berkeley.edu>
> Cc: Bastian Koppelmann <kbast...@mail.uni-paderborn.de>
> Cc: Alistair Francis <alistair.fran...@wdc.com>
> Co-Authored-by: Johannes Haring <johannes.har...@gmx.net>
> Signed-off-by: Michael Clark <m...@sifive.com>
Reviewed-by: Alistair Francis <alistair.fran...@wdc.com>
Alistair
> ---
> hw/riscv/sifive_clint.c | 8 ++++----
> 1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/hw/riscv/sifive_clint.c b/hw/riscv/sifive_clint.c
> index 0d2fd52487e6..d4c159e93736 100644
> --- a/hw/riscv/sifive_clint.c
> +++ b/hw/riscv/sifive_clint.c
> @@ -146,15 +146,15 @@ static void sifive_clint_write(void *opaque, hwaddr
> addr, uint64_t value,
> error_report("clint: invalid timecmp hartid: %zu", hartid);
> } else if ((addr & 0x7) == 0) {
> /* timecmp_lo */
> - uint64_t timecmp = env->timecmp;
> + uint64_t timecmp_hi = env->timecmp >> 32;
> sifive_clint_write_timecmp(RISCV_CPU(cpu),
> - timecmp << 32 | (value & 0xFFFFFFFF));
> + timecmp_hi << 32 | (value & 0xFFFFFFFF));
> return;
> } else if ((addr & 0x7) == 4) {
> /* timecmp_hi */
> - uint64_t timecmp = env->timecmp;
> + uint64_t timecmp_lo = env->timecmp;
> sifive_clint_write_timecmp(RISCV_CPU(cpu),
> - value << 32 | (timecmp & 0xFFFFFFFF));
> + value << 32 | (timecmp_lo & 0xFFFFFFFF));
> } else {
> error_report("clint: invalid timecmp write: %08x",
> (uint32_t)addr);
> }
> --
> 2.7.0
>
>
