From 5730d12167e0a0834d14b6332a4dd31d673bf73b Mon Sep 17 00:00:00 2001 From: Ben Marshall Date: Thu, 18 Feb 2021 13:22:45 +0000 Subject: [PATCH] scalar-crypto: Fix RV32 sha512 instructions. These instructions are RV32 only. Previously, they zero-extended their 32-bit result to 64-bits, to match the Spike implementation detail that the X registers are always 64-bits long. This exposed a data dependant problem when the instruction results fed into the add and sltu instructions. The lack of sign extension on the sha512*, combined with the presence of sign extension on the add, meant sltu would (as it is currently implemented) produce the wrong result. There were two potential fixes: 1) Sign extend from 32-bits to XLEN the result of the SHA512 instructions. 2) Change the SLTU implementation to truncate RS1/RS2 to be XLEN bits before it does the comparison. This patch implements option 1, because I didn't want to mess with a base ISA instruction. However, this leaves the implementation detail open to cause problems for people in the future. Fixing this is outside the scope of this commit. On branch scalar-crypto-fix Changes to be committed: modified: riscv/insns/sha512sig0h.h modified: riscv/insns/sha512sig0l.h modified: riscv/insns/sha512sig1h.h modified: riscv/insns/sha512sig1l.h modified: riscv/insns/sha512sum0r.h modified: riscv/insns/sha512sum1r.h --- riscv/insns/sha512sig0h.h | 8 ++++---- riscv/insns/sha512sig0l.h | 8 ++++---- riscv/insns/sha512sig1h.h | 8 ++++---- riscv/insns/sha512sig1l.h | 8 ++++---- riscv/insns/sha512sum0r.h | 8 ++++---- riscv/insns/sha512sum1r.h | 8 ++++---- 6 files changed, 24 insertions(+), 24 deletions(-) diff --git a/riscv/insns/sha512sig0h.h b/riscv/insns/sha512sig0h.h index c28716f8..7cd5f2f1 100644 --- a/riscv/insns/sha512sig0h.h +++ b/riscv/insns/sha512sig0h.h @@ -2,8 +2,8 @@ require_rv32; require_extension('K'); -uint32_t result = - ((uint32_t)RS1 >> 1) ^ ((uint32_t)RS1 >> 7) ^ ((uint32_t)RS1 >> 8) ^ - ((uint32_t)RS2 << 31) ^ ((uint32_t)RS2 << 24); +reg_t result = + (zext32(RS1) >> 1) ^ (zext32(RS1) >> 7) ^ (zext32(RS1) >> 8) ^ + (zext32(RS2) << 31) ^ (zext32(RS2) << 24); -WRITE_RD(zext_xlen(result)); +WRITE_RD(sext_xlen(result)); diff --git a/riscv/insns/sha512sig0l.h b/riscv/insns/sha512sig0l.h index d0a6db25..99d7aa9a 100644 --- a/riscv/insns/sha512sig0l.h +++ b/riscv/insns/sha512sig0l.h @@ -2,8 +2,8 @@ require_rv32; require_extension('K'); -uint32_t result = - ((uint32_t)RS1 >> 1) ^ ((uint32_t)RS1 >> 7) ^ ((uint32_t)RS1 >> 8) ^ - ((uint32_t)RS2 << 31) ^ ((uint32_t)RS2 << 25) ^ ((uint32_t)RS2 << 24); +reg_t result = + (zext32(RS1) >> 1) ^ (zext32(RS1) >> 7) ^ (zext32(RS1) >> 8) ^ + (zext32(RS2) << 31) ^ (zext32(RS2) << 25) ^ (zext32(RS2) << 24); -WRITE_RD(zext_xlen(result)); +WRITE_RD(sext_xlen(result)); diff --git a/riscv/insns/sha512sig1h.h b/riscv/insns/sha512sig1h.h index 18ba85fe..ed3fa32d 100644 --- a/riscv/insns/sha512sig1h.h +++ b/riscv/insns/sha512sig1h.h @@ -2,8 +2,8 @@ require_rv32; require_extension('K'); -uint32_t result = - ((uint32_t)RS1 << 3) ^ ((uint32_t)RS1 >> 6) ^ ((uint32_t)RS1 >> 19) ^ - ((uint32_t)RS2 >> 29) ^ ((uint32_t)RS2 << 13); +reg_t result = + (zext32(RS1) << 3) ^ (zext32(RS1) >> 6) ^ (zext32(RS1) >> 19) ^ + (zext32(RS2) >> 29) ^ (zext32(RS2) << 13); -WRITE_RD(zext_xlen(result)); +WRITE_RD(sext_xlen(result)); diff --git a/riscv/insns/sha512sig1l.h b/riscv/insns/sha512sig1l.h index 691ef32e..84694d1e 100644 --- a/riscv/insns/sha512sig1l.h +++ b/riscv/insns/sha512sig1l.h @@ -2,8 +2,8 @@ require_rv32; require_extension('K'); -uint32_t result = - ((uint32_t)RS1 << 3) ^ ((uint32_t)RS1 >> 6) ^ ((uint32_t)RS1 >> 19) ^ - ((uint32_t)RS2 >> 29) ^ ((uint32_t)RS2 << 26) ^ ((uint32_t)RS2 << 13); +reg_t result = + (zext32(RS1) << 3) ^ (zext32(RS1) >> 6) ^ (zext32(RS1) >> 19) ^ + (zext32(RS2) >> 29) ^ (zext32(RS2) << 26) ^ (zext32(RS2) << 13); -WRITE_RD(zext_xlen(result)); +WRITE_RD(sext_xlen(result)); diff --git a/riscv/insns/sha512sum0r.h b/riscv/insns/sha512sum0r.h index e2772e33..87afee96 100644 --- a/riscv/insns/sha512sum0r.h +++ b/riscv/insns/sha512sum0r.h @@ -2,8 +2,8 @@ require_rv32; require_extension('K'); -uint32_t result = - ((uint32_t)RS1 << 25) ^ ((uint32_t)RS1 << 30) ^ ((uint32_t)RS1 >> 28) ^ - ((uint32_t)RS2 >> 7) ^ ((uint32_t)RS2 >> 2) ^ ((uint32_t)RS2 << 4); +reg_t result = + (zext32(RS1) << 25) ^ (zext32(RS1) << 30) ^ (zext32(RS1) >> 28) ^ + (zext32(RS2) >> 7) ^ (zext32(RS2) >> 2) ^ (zext32(RS2) << 4); -WRITE_RD(zext_xlen(result)); +WRITE_RD(sext_xlen(result)); diff --git a/riscv/insns/sha512sum1r.h b/riscv/insns/sha512sum1r.h index e5dfaa98..1e526967 100644 --- a/riscv/insns/sha512sum1r.h +++ b/riscv/insns/sha512sum1r.h @@ -2,8 +2,8 @@ require_rv32; require_extension('K'); -uint32_t result = - ((uint32_t)RS1 << 23) ^ ((uint32_t)RS1 >> 14) ^ ((uint32_t)RS1 >> 18) ^ - ((uint32_t)RS2 >> 9) ^ ((uint32_t)RS2 << 18) ^ ((uint32_t)RS2 << 14); +reg_t result = + (zext32(RS1) << 23) ^ (zext32(RS1) >> 14) ^ (zext32(RS1) >> 18) ^ + (zext32(RS2) >> 9) ^ (zext32(RS2) << 18) ^ (zext32(RS2) << 14); -WRITE_RD(zext_xlen(result)); +WRITE_RD(sext_xlen(result));