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

Implement full mtval #712

Merged
merged 20 commits into from
Oct 15, 2024
Merged

Implement full mtval #712

merged 20 commits into from
Oct 15, 2024

Conversation

piotro888
Copy link
Member

@piotro888 piotro888 commented Jun 14, 2024

Implements full mtval CSR.

Required some tricks. This CSR is optional to fill for all exception types, so if some case becomes too problematic in the future, it can be set to 0.

Boom and XiangShan doesn't support setting it to instruction bytes in case of Illegal Instruction, but it was added to coreblocks at currently little cost. It is helpful, because it could speed up emulation of missing instructions (possibly in OpenSBI?)
XiangShan supports setting mtval to upper half of instruction address in case of fault on misaligned instruction crossing fetch blocks, hard to tell if supported in Boom.
Other cases are handled by both processors.

@piotro888 piotro888 marked this pull request as ready for review August 12, 2024 09:37
@piotro888 piotro888 added this to the Implement machine mode ISA milestone Aug 12, 2024
@piotro888 piotro888 added the enhancement New feature or request label Aug 12, 2024
coreblocks/func_blocks/csr/csr.py Outdated Show resolved Hide resolved
coreblocks/frontend/decoder/instr_decoder.py Outdated Show resolved Hide resolved
@@ -95,7 +95,9 @@ def elaborate(self, platform):
"rl_s1": Mux(instr_decoder.rs1_v & (~exception_override), instr_decoder.rs1, 0),
"rl_s2": Mux(instr_decoder.rs2_v & (~exception_override), instr_decoder.rs2, 0),
},
"imm": instr_decoder.imm,
"imm": Mux(
~exception_override, instr_decoder.imm, Mux(raw.access_fault, raw.access_fault, raw.instr)
Copy link
Contributor

Choose a reason for hiding this comment

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

Shouldn't be raw.pc in case when raw.access_fault is true?

Copy link
Member Author

Choose a reason for hiding this comment

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

It is correct, Exception FU uses information from access_fault to determine correct PC. Switched to enum and documented this mux

coreblocks/interface/layouts.py Outdated Show resolved Hide resolved
test/asm/mtval.asm Show resolved Hide resolved
@tilk
Copy link
Member

tilk commented Oct 5, 2024

Waiting until comments are addressed.

@tilk tilk merged commit 8f2072f into kuznia-rdzeni:master Oct 15, 2024
13 checks passed
github-actions bot pushed a commit that referenced this pull request Oct 15, 2024
tilk pushed a commit to kuznia-rdzeni/transactron that referenced this pull request Nov 25, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants