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

Unexpected Modification of xstatus WPRI Field During menvcfg Reads/Writes #3934

Closed
4 tasks done
youzi27 opened this issue Nov 25, 2024 · 6 comments · Fixed by #3955, OpenXiangShan/NEMU#688, OpenXiangShan/riscv-isa-sim#60 or OpenXiangShan/NEMU#685
Assignees
Labels
bug report Bugs to be confirmed

Comments

@youzi27
Copy link

youzi27 commented Nov 25, 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

Hi there,

When performing carefully crafted reads and writes(csrrs) to the menvcfg register in M-mode on XiangShan, the WPRI (Reserved Writes Preserve Values, Reads Ignore Values) field in xstatus is unexpectedly modified to 1. This behavior is not observed in either NEMU or SPIKE.

Screenshots
image

Expected behavior

  • The WPRI field in xstatus should remain unmodified during reads or writes to menvcfg, as specified by the RISC-V privileged architecture specification.
  • Behavior should be consistent with other simulators such as NEMU and SPIKE.

To Reproduce

testcase: test.zip

Environment

Additional context

No response

@youzi27 youzi27 added the bug report Bugs to be confirmed label Nov 25, 2024
@NewPaulWalker
Copy link
Contributor

Hi @youzi27
@lewislzh is working on this issue.

@lewislzh
Copy link
Contributor

lewislzh commented Nov 26, 2024

@youzi27
Thank you for your issue. First, this is not a WPRI field. We recommend updating to the latest version of the manual for reference. This bit is the sdt field, which is used to control the S-mode double trap behavior. The manual specifies that the readability and writability of the sdt field are influenced by the DTE field in envcfg. When the writability of sdt changes due to a modification of DTE, the sdt field only needs to acquire an unspecified but legal value (i.e., an implementation-defined legal value).

In your example, this is exactly the case: writing to menvcfg happens to enable DTE. Therefore, strictly speaking, the behavior of NEMU, SPIKE, and XiangShan complies with the manual and is not a bug.

XiangShan's implementation treats the sdt field as WARL (Write Any Read Legal) when it becomes read-only due to the DTE field being disabled. In this case, writes to sdt are ignored, but reads return valid values. Consequently, when DTE is disabled, reading sdt (or having it read by difftest) always returns 0 (i.e., software perceives it as 0). However, the value in the register (hardware architectural state) may still change. Once DTE is enabled, the value of sdt read by difftest immediately reflects the register's value. In your example, sstatus.sdt was written as 1 before writing menvcfg, so after DTE is enabled, XiangShan's behavior immediately reflects this value as 1.

SPIKE, on the other hand, implements this read-only field as WLRL (Write Limited Read Legal), which means writes are also constrained to 0. As a result, SPIKE retains the value of 0 even after DTE is enabled. NEMU's implementation aligns with XiangShan's, but it directly reads the sstatus register value without using a proper read function, leading to a mismatch during difftest.

We will work to unify the implementations of NEMU and SPIKE to align with XiangShan in the future to ensure they pass difftest successfully.

@youzi27
Copy link
Author

youzi27 commented Nov 27, 2024

Hi @lewislzh ,

Thank you for your patient explanation. The RISC-V privileged version I was using (2024-02-13) lacked an explanation about SDT, so I have updated my privileged specification.

However, there are still some questions I hope you can clarify. According to the privileged specification I referenced, my testcase occurred in M-mode.

  1. Does menvcfg.DTE affect operations in M-mode?
  2. Why would the readability/writability of mstatus.SDT in M-mode be influenced by menvcfg.DTE? I could not find relevant provisions in the privileged specification.

I would greatly appreciate it if you could provide further clarification. Thank you!

@youzi27
Copy link
Author

youzi27 commented Nov 27, 2024

Fortunately, I found similar questions and answers for your reference (riscv/riscv-isa-manual#1623). I look forward to your further explanation. Thank you very much.

@lewislzh
Copy link
Contributor

Fortunately, I found similar questions and answers for your reference ([riscv/riscv-isa-manual#1623]
(riscv/riscv-isa-manual#1623)). I look forward to your further explanation. Thank you very much.

Thank you for your issue. The read/write accessibility of mstatus.sdt is indeed not controlled by DTE, which conflicts with the specification. We will address this issue in a future fix. The behavior of sstatus.sdt aligns with the specification, and we are currently working on aligning the behavior of NEMU and SPIKE with XiangShan.

@youzi27
Copy link
Author

youzi27 commented Nov 27, 2024

Thank you for your response. I look forward to your fix for this issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment