SpinalHDL / NaxRiscv

MIT License
267 stars 40 forks source link

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

Closed Bill94l closed 1 month ago

Bill94l commented 4 months ago

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

Dolu1990 commented 4 months ago

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 :

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 commented 4 months ago

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

https://github.com/riscv-software-src/riscv-isa-sim/pull/186#issue-307446561

Thanks :D

Dolu1990 commented 4 months ago

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 commented 4 months ago

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.