Some small improvements:
- Use RISC-V names for exception codes (disARM them)
- Remove undefined S-mode exception codes (machine and hypervisor ecall)
- Apply style(9) to some condition checks
Details
Details
- Reviewers
kp jrtc27 br jhb - Commits
- rS367020: riscv: improve exception code naming
Diff Detail
Diff Detail
- Repository
- rS FreeBSD src repository - subversion
- Lint
Lint Not Applicable - Unit
Tests Not Applicable
Event Timeline
Comment Actions
I'm also considering moving the SCAUSE* definitions to machine/trap.h which is an empty header on riscv. I think that riscvreg.h is a sensible place for them, but I find myself searching trap.h first whenever I need to look up an exception code by number. Thoughts?
| sys/riscv/include/riscvreg.h | ||
|---|---|---|
| 40–54 ↗ | (On Diff #78641) | Whilst at it, please make FAULT_FETCH etc consistent with INST_PAGE_FAULT (and I guess the INST vs FETCH too?). Also the normal faults are called "access faults" in the spec so they should gain ACCESS in the same way the page faults are PAGE_FAULT. |
| 41 ↗ | (On Diff #78641) | This name doesn't make sense; SCAUSE is the whole thing. You want SCAUSE_EXCP_MASK or SCAUSE_EXCEPTION_MASK. |
| sys/riscv/include/riscvreg.h | ||
|---|---|---|
| 41 ↗ | (On Diff #78641) | The field is named as the Exception Code in the spec, so I went with SCAUSE_CODE. I can add a MASK suffix to both of these if desired, but I feel it's clear enough from their usage. |
| sys/riscv/include/riscvreg.h | ||
|---|---|---|
| 44 ↗ | (On Diff #78647) | The description is "Illegal instruction"; the old name was better IMO (but can be ILLEGAL_INST if you want to match the other faults). |
| sys/riscv/include/riscvreg.h | ||
|---|---|---|
| 44 ↗ | (On Diff #78647) | Yeah, I had that thought too, but this way it lines up with the other two instruction traps here and SCAUSE_INST_PAGE_FAULT further down. I don't particularly object to either form, there's arguments for both. |
| sys/riscv/include/riscvreg.h | ||
|---|---|---|
| 44 ↗ | (On Diff #78647) | It does not matter to me. I'll make it ILLEGAL_INSTRUCTION for the commit since that's what the spec has and it's distinct enough from the other INST_* codes. Any thoughts on my question of moving these definitions to machine/trap.h? |
| sys/riscv/include/riscvreg.h | ||
|---|---|---|
| 44 ↗ | (On Diff #78647) | Hm. I feel like so long as they're talking about SCAUSE directly then they belong in riscvreg.h, especially SCAUSE_INTR/CODE. What you _could_ do though is have SCAUSE_INTR and SCAUSE_CODE here, and put EXC_CODE_* in trap.h (yeah more renaming..)? |
| sys/riscv/include/riscvreg.h | ||
|---|---|---|
| 44 ↗ | (On Diff #78647) | Okay yeah, agreed. I'll leave them here since it's better to have the definitions in one place. |