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

Wrong mstatus, mtval value when Xiangshan executes an illegal instruction. #3860

Closed
4 tasks done
ha0lyu opened this issue Nov 11, 2024 · 8 comments
Closed
4 tasks done
Labels
bug report Bugs to be confirmed

Comments

@ha0lyu
Copy link
Contributor

ha0lyu commented Nov 11, 2024

Before start

  • I have read the RISC-V ISA Manual and this is not a RISC-V ISA question. 我已经阅读过 RISC-V 指令集手册,这不是一个指令集本身的问题。
  • I have read the XiangShan Documents. 我已经阅读过香山文档。
  • I have searched the previous issues and did not find anything relevant. 我已经搜索过之前的 issue,并没有找到相关的。
  • I have reviewed the commit messages from the relevant commit history. 我已经浏览过相关的提交历史和提交信息。

Describe the bug

When xiangshan execute an illegal instruction fld fa1, 160(a1), xiangshan gets wrong mstatus & mtval comparing with ready-to-run/spike-so, mtval is certainly wrong because it is also different from the result of NEMU. Please check the two log files for details.

Expected behavior

mstatus & mtval is right value.

To Reproduce

test.zip

Environment

  • XiangShan branch: master
  • XiangShan commit id: 4376b52
  • NEMU commit id: a4815f99
  • SPIKE commit id: 1.1.0-dev

Additional context

No response

@ha0lyu ha0lyu added the bug report Bugs to be confirmed label Nov 11, 2024
@ha0lyu ha0lyu changed the title Wrong mstatus, mtval value when Xiangshan executes illegal instruction. Wrong mstatus, mtval value when Xiangshan executes an illegal instruction. Nov 11, 2024
@ha0lyu
Copy link
Contributor Author

ha0lyu commented Nov 11, 2024

mstatus = 0x0000040a00001800 in Xiangshan & NEMU, it seems that the spike-so has a bug? Please check whether it is a bug of the spike-so.

@fly-1011
Copy link

hello@ha0lyu 😄

I have reviewed the test case and log information you submitted. In both NEMU and spike-so, the value of the mtval register is 0x31cc, which corresponds to the assembly instruction c.fld fa1, 160(a1). This issue is the same as the one I previously reported: #3864

Regarding the discrepancy you mentioned with the mstatus register, it may be due to the fact that both XiangShan and NEMU have implemented the SMDBLTRP extension (RISC-V M-mode Double Trap Extension), which is not implemented in Spike. You can find more details here: OpenXiangShan/NEMU#641 (comment)

@ha0lyu
Copy link
Contributor Author

ha0lyu commented Nov 11, 2024

Hi @fly-1011,
Thanks for your kindly reminder. Yes, this may be a same bug as you mentioned before. I didn't know that issue because I just checked issues of Xiangshan. Next time, please issue it in Xiangshan again.

Regarding the discrepancy you mentioned with the mstatus register, it may be due to the fact that both XiangShan and NEMU have implemented the SMDBLTRP extension (RISC-V M-mode Double Trap Extension), which is not implemented in Spike.

As for the error mstatus value, it seems that this is a bug in spike of DiffTest, which is a tool belongs to OpenXiangShan not the riscv-isa-sim: spike.

Let's keep this issue open until this bug fixed.

@NewPaulWalker
Copy link
Contributor

Thanks for @fly-1011 response.
The mismatch in mtval is the same issue as #3864.
As for the mismatch in mstatus between XiangShan and Spike, as I mentioned in OpenXiangShan/NEMU#641 (comment), it is because Spike has not implemented the SMDBLTRP extension yet.
Also, we do not currently plan to implement this extension on Spike.

@lewislzh
Copy link
Contributor

lewislzh commented Nov 12, 2024

For sm/ssbltrp, there are some implementation-defined inconsistencies between Xiangshan and Spike. Therefore, the relevant extensions are not enabled for spike-so. Additionally, since smdbltrp is based on the smrnmi extension, which is also mostly vestigial in Spike a5fdc4efbf9f79a890fe5d1c144880548b011d90, we currently see no need to conduct a diff for the above extensions with Spike. In the future, there may be plans to align the behavior of Spike with Xiangshan. If you’re interested, you are welcome to submit a PR to the Xiangshan Spike repository.

@lewislzh
Copy link
Contributor

I have completed the implementation of the SMRNMI extension and the dbltrp-related extension under XiangShan’s Spike 3870 , 54, 658 . There should be no mstatus.mdt-related issues in future Spike diffs.

@ha0lyu
Copy link
Contributor Author

ha0lyu commented Nov 14, 2024

Hi @lewislzh,
I really appreciate what you have done.
I have tested it in the newest version, it works! No more mstatus different for now.
However, this test case raise mcause different:

sh      s2, 1259(s0)   # test case
-------------------
privilegeMode: 3
 mcause different at pc = 0x0080000030, right= 0x0000000000000007, wrong = 0x0000000000000006

I ran this test case, the mcause value is 6 in the official spike, which is the same as XiangShan. But in spike-diff, it is 7, nemu-diff reported nothing.

Thanks.

@NewPaulWalker
Copy link
Contributor

This seems to be a new issue. Please create a new issue for it.

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

No branches or pull requests

4 participants