riscv_machine_timer driver don't follow the spec ?

[Dolu1990]

The mtime isn't read properly in the following code :
https://github.com/zephyrproject-rtos/zephyr/blob/da699e4a3b5f1baf1305331bbea0f8697117a8b9/drivers/timer/riscv_machine_timer.c#L51#L52

I use Volume I: RISC-V User-Level ISA V2.2 -> Figure 2.5: Sample code for reading the 64-bit cycle counter in RV32 as reference.

[AnttiLukats]

interesting, if there is real problem with 1.13 and the contest entry must use 1.13 without patchces?

[Dolu1990]

@AnttiLukats There is 32 bits implementation of the mtime which isn't impacted by the bug :
https://github.com/SpinalHDL/riscvSoftcoreContest/blob/6fc1ac40b3332699cd87060d70d71cc67c5c9290/src/main/scala/riscvSoftcoreContest/Peripherals.scala#L18
I tested it, it work fine

[brouhaha]

Just encountered this problem myself. The comment "This also works for other implementations" should be "This also works for some other implementations." In particular, it can fail on an implementation that doesn't have a cache, or for which mtime is in an uncacheable region. (In fact, a RISC-V implementation where mtime is cached is broken, unless any cache line containing mtime is automatically updated or invalidated whenever mtime is incremented!)

Following the figure 2.5 sample code should work for all implementations, whether or not cache is used. Here's a proposed replacement for the section that reads rtc:

        u32_t hi1, hi2, low;
        do
        {
          hi1 = mtime->val_high;
          low = mtime->val_low;
          hi2 = mtime->val_high;
        } while (hi1 != hi2);
        rtc = ((u64_t)hi1 << 32) | low;

[brouhaha]

@Dolu1990 If I understand the code you pointed to, you're only implementing a 32-bit mtime in your RISC-V core, which both violates the RISC-V specification and doesn't solve this problem.

[nategraff-sifive]

Closed by #10556

[Dolu1990]

@brouhaha Sorry, just seen your message now :/

So, in the Igloo2Perf and Up5kPerf, i implemented a 32 bits mtime and use the mtime zephyr driver without any changes. Implementing it as a 32 bit time even avoid the 64 bits issue that was in zephyr 1.13

For the Up5kArea design, i implemented my own 20 bit timer / zephyr driver to reduce the area usage, if i remember well it reduce about 100 lut the occupancy.

In general, i try to stick to the absolute minimum requirements to get the software run, For instance, there is the Csr configuration i used to be compliant with the contest :
https://github.com/SpinalHDL/VexRiscvSoftcoreContest2018/blob/master/hardware/scala/riscvSoftcoreContest/Up5kArea.scala#L138

There is a looooot of "useless" things turned off.