Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix the check of the register Rd is not equal to zero for c.lwsp instruction and Update IO rang addresses #115

Closed
wants to merge 2 commits into from

Conversation

Bill94l
Copy link

@Bill94l Bill94l commented Jul 4, 2024

Hi,

Please find the modification for c.lwsp below. Kindly make the necessary adjustments for the remaining compressed instructions that require a non-zero check of the RS and RD registers

Thanks :D

@Bill94l Bill94l changed the title Fix the check of the register Rd is not equal to zero for c.lwsp instruction Fix the check of the register Rd is not equal to zero for c.lwsp instruction and Update IO rang addresses Jul 8, 2024
@Dolu1990
Copy link
Member

Dolu1990 commented Jul 9, 2024

Hi ^^
So, apparently the issue with a(31) is that some tests in the nax/machine regressions get :

2024-07-08T13:01:31.5430063Z *** mmio address
2024-07-08T13:01:31.5430625Z DUT=1230 REF=1ffffff0 ***
2024-07-08T13:01:31.5431081Z
2024-07-08T13:01:31.5431333Z TIME=4636
2024-07-08T13:01:31.5431848Z LAST PC COMMIT=8000064c
2024-07-08T13:01:31.5432476Z INCOMING SPIKE PC=ffffffff8000064e
2024-07-08T13:01:31.5433134Z ROB_ID=x3b
2024-07-08T13:01:31.5433623Z FAILURE machine

ext/NaxSoftware/baremetal/machine/build/rv32imafdc/machine.elf

So my guess is that there is a missmatch of the memory mapping between RVLS and the DUT, making things go south on :
#define MM_FAULT_ADDRESS 0x00001230
trap_setup
li x1, MM_FAULT_ADDRESS
lw x1, 0(x1) //load fault
trap_handle

So here we have 2 conflicting desire :

  • Testing that a cached memory load which fault is catched by Nax (unlike the store)
  • Keeping things in sync with RVLS / implementation non-specific RISC-V

So far what i did is to avoid simulating cases where spike would trap the fault and vex would ignore it.
I think we would need to extends RVLS / Spike to allow specifying memory region where store or load trap are ignored by the hardware ?

@Bill94l
Copy link
Author

Bill94l commented Jul 12, 2024

Hi,

Do you have a suggestion on how to extend spike or RVLS to add memory regions?

Here's what I found on the riscv-isa-sim github, but I don't know if it can help

riscv-software-src/riscv-isa-sim#186 (comment)

Thanks :D

@Dolu1990
Copy link
Member

So, RVLS is already informed by the simulation of what memory region exists and if the are cached or io.
https://github.com/SpinalHDL/rvls/blob/main/src/hart.hpp#L67
Would need to add more information to it like :
allowFetch, allowStore, allowLoad

It depend which RVLS frontend you use, for instance if you use the text file one, it is decoded here :
https://github.com/SpinalHDL/rvls/blob/b5c15e129f8168c2317b2c129aef91adf935bfeb/src/ascii_frontend.cpp#L114

Else through JNI =>
https://github.com/SpinalHDL/rvls/blob/main/src/jni_frontend.cpp#L151

Then the testbench could give those additional information

Remain the other side, would need to update the mmio_xxx function in https://github.com/SpinalHDL/rvls/blob/main/src/hart.cpp#L49C15-L49C25 to check those added flags in the regions.

Important thing for such update is to go step by step, probably starting from the testbench and then going layer after layer step by step.

@Dolu1990
Copy link
Member

I mean, on thing about the fault behaviour missmatch betwen spike and Nax is store fault.
Nax => not trap on store fault

So, would need to update https://github.com/SpinalHDL/rvls/blob/b5c15e129f8168c2317b2c129aef91adf935bfeb/src/hart.cpp#L86 to not trap on non-io store.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants