diff mbox

[v1,23/30] RISC-V: Fix CLINT timecmp low 32-bit writes

Message ID 1527034517-7851-24-git-send-email-mjc@sifive.com (mailing list archive)
State New, archived
Headers show

Commit Message

Michael Clark May 23, 2018, 12:15 a.m. UTC
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 <palmer@sifive.com>
Cc: Sagar Karandikar <sagark@eecs.berkeley.edu>
Cc: Bastian Koppelmann <kbastian@mail.uni-paderborn.de>
Cc: Alistair Francis <Alistair.Francis@wdc.com>
Co-Authored-by: Johannes Haring <johannes.haring@gmx.net>
Signed-off-by: Michael Clark <mjc@sifive.com>
---
 hw/riscv/sifive_clint.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

Comments

Alistair Francis May 25, 2018, 10:40 p.m. UTC | #1
On Tue, May 22, 2018 at 5:15 PM, Michael Clark <mjc@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 <palmer@sifive.com>
> Cc: Sagar Karandikar <sagark@eecs.berkeley.edu>
> Cc: Bastian Koppelmann <kbastian@mail.uni-paderborn.de>
> Cc: Alistair Francis <Alistair.Francis@wdc.com>
> Co-Authored-by: Johannes Haring <johannes.haring@gmx.net>
> Signed-off-by: Michael Clark <mjc@sifive.com>

Reviewed-by: Alistair Francis <alistair.francis@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
>
>
diff mbox

Patch

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);
         }