Skip to content

trap on accesses to mcounteren or menvcfg - #335

Open
MikeOpenHWGroup wants to merge 1 commit into
openhwfoundation:mainfrom
MikeOpenHWGroup:cs_registers
Open

MikeOpenHWGroup wants to merge 1 commit into
openhwfoundation:mainfrom
MikeOpenHWGroup:cs_registers

Conversation

@MikeOpenHWGroup

Copy link
Copy Markdown

The CVE2 does not support U-mode so access to mcounteren or menvcfg should throw an illegal instruction trap.

This fix "works" and passes an extensive regression. However, it is very awkwardly implemented because we have only disabled the logic for U-mode, not removed it.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟢 Approval recommended

The CSR read decode now correctly raises illegal-instruction traps for menvcfg(*) and mcounteren when U-mode is disabled, matching the stated requirement and existing illegal-CSR handling patterns.

Pull request overview

This PR updates CVE2’s CSR access behavior to match the RISC-V Privileged spec requirement that menvcfg/menvcfgh and mcounteren must be treated as non-existent (and therefore trap) when U-mode is not supported.

Changes:

  • Make reads of CSR_MENVCFG/CSR_MENVCFGH raise illegal_csr when UmodeEnabled==0.
  • Make reads of CSR_MCOUNTEREN raise illegal_csr when UmodeEnabled==0.
  • Add inline spec-referencing comments explaining the “CSR should not exist without U-mode” behavior.
File summaries
File Description
rtl/cve2_cs_registers.sv Gates menvcfg(*) and mcounteren CSR reads on UmodeEnabled, trapping when U-mode is absent.
Review details
  • Files reviewed: 1/1 changed files
  • Comments generated: 0
  • Review effort level: Lite

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

@cairo-caplan cairo-caplan left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

3 participants