CLINT: 32-bit reads of mtime/mtimecmp upper half return the low word (read_req address shift is commented out)

Summary

In clint/clint.bsv, the read_req method's intra-register address shift is commented out, so a 32-bit (Word) read of the upper half of a 64-bit mtime/mtimecmp returns the lower 32 bits instead of bits [63:32]. The write_req path keeps the equivalent shift active, so the two are asymmetric — writes mask correctly, reads do not.

This breaks any RISC-V-standard driver that reads mtime as two 32-bit halves (SiFive CLINT drivers, Linux timer-riscv, Tock's sifive::clint): reading the high word at mtimereg + 4 yields the low word, so the reconstructed 64-bit time is corrupt (high word ≈ low word). Full 64-bit (DWord) reads are unaffected, which masks the bug on pure-64-bit access paths.

Location (current master, commit 5426057c)

In read_req, both lines are commented out:

// Bit#(6) shift_amt=zeroExtend(addr[2:0])<<3;
// temp = temp>>shift_amt;

while the matching shift in write_req is live:

Bit#(6) shift_amt=zeroExtend(addr[2:0])<<3;
mask=mask<<shift_amt;

So a Word read falls through to temp = duplicate(temp[31:0]), always returning the low 32 bits regardless of addr[2].

Reproduction

With global interrupts disabled (no trap), read mtime twice as two 32-bit halves (mtimereg and mtimereg + 4) and print them — the high word equals the low word (we observed 0x00016aef00016ae9, where the halves should differ). Seen on the C-Class c64 SoC driving a Tock OS RV64 machine-timer driver.

Suggested fix

Restore the address-based shift in read_req (uncomment the two lines), mirroring the masking already done in write_req, so a sub-DWord read selects the addressed slice. Happy to send a merge request if that is preferred.