search

riscv / sail-riscv

Sail RISC-V model
https://lists.riscv.org/g/tech-golden-model
Other
467 stars 169 forks source link

Add Sstc extension #570

Open Timmmm opened 1 month ago

Timmmm commented 1 month ago

This adds the stimecmp[h] CSRs. It is enabled by default.

The code is slightly unfortunately structured to handle CSRs that don't fit the standard permission checks. I added a TODO to improve it.

Timmmm commented 1 month ago

This is a rebase of #395 by @ved-rivos. The model structure has changed a bit so it required fairly extensive changes. I also added a command line flag.

github-actions[bot] commented 1 month ago

Test Results

396 tests  ±0   396 ✅ ±0   0s ⏱️ ±0s   4 suites ±0     0 💤 ±0    1 files   ±0     0 ❌ ±0 

Results for commit 81287b2d. ± Comparison against base commit a78ab633.

:recycle: This comment has been updated with latest results.

Alasdair commented 1 month ago

Yes, I think having a is_csr_accessible function might be a good idea

Timmmm commented 1 month ago

:point_up: rebased and fixed a missing & xlen == 32 on the stimecmph CSR that was found in testing.

Timmmm commented 1 month ago

@billmcspadden-riscv any idea who we could get to review this from a spec correctness point of view?

Timmmm commented 2 days ago

Rebased to handle the CSR rejigging. I had to move some code around to make the dependencies work. I also removed the platform_wfi() speed hack since it would no longer be correct with Sstc. I doubt it make much difference to speed in practice.

We've been using this code for a while now with no issues found. It seems like @jscheid-ventana was the original author of the spec - if you could take a look that would be great!

Otherwise I think we should just merge this.

Alasdair commented 2 days ago

I left some minor comments, but overall I also think this can just be merged.