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

Improve description of software examples #430

Open
mark-honman opened this issue Nov 5, 2024 · 1 comment
Open

Improve description of software examples #430

mark-honman opened this issue Nov 5, 2024 · 1 comment
Labels
v1.0 resolve for 1.0

Comments

@mark-honman
Copy link

mark-honman commented Nov 5, 2024

Comments in the software examples for preemptible interrupt handlers are misleading because CLIC-enhanced mcause contains a mix of fields that pertain to the interrupt to be handled (interrupt and exccode) and information from the Interrupted Context (OIC).

It would be useful to explain that the essential requirement for a handler to be preemptible is that it should save mpp, mpie, mpil, and mepc prior to enabling interrupts, and immediately prior to exit it must disable interrupts and restore this information which is needed to return control to the Interrupted Context. A preemptible interrupt handler will usually also save mcause.interrupt and mcause.exccode if it needs to refer to those values in the course of its execution, as the contents of these fields will be overwritten by a preempting interrupt.

Since mpp, mpie, mpil have aliases in mcause, the only CSRs that must be saved and restored with interrupts disabled by a preemptible interrupt handler are mcause and mepc.

Specific issues with comments/notes in the examples:
A.2
-(minor) - "read cause" should be "read cause and interrupted-context information" (similiar for "put cause back" comment).
B.1

  • (major) - "Get mcause of interrupted context", "Save mcause of interrupted context" are completely misleading.
    (had me wondering "how the XYZZY does it achieve THAT?").
  • (major) - "Call C ABI routine, a0 has interrupt ID encoded." Incorrect, at this stage a0 contains the address of the interrupt handling routine. The contents of a1 are as described in the mnxti pseudo-code: VTBASE + XLEN/8 * clic.id
  • (medium) note 2 talks of saving OIC's mepc and mcause values, which is misleading because the mepc and mcause values available to the new interrupt handler are values in its context rather than in the OIC. Perhaps better to say "save mepc and mpp, mpie, mpil values needed to return to the OIC"?
  • (enhancement) - at note 12, may be worth mentioning that mpil may have changed if the handler has been preempted. (see note A below)
  • (query) - the last-gasp check of mnxti in the exit critical section just complicates the example... considering how few instructions there are between the service-loop exit and the mnxti access in the critical section. Yes this is a useful trick if the hart implementation is non-pipelined but IMO that would be better addressed in a follow-up example.

Note A: Section 3.10.7 Returns from Handlers says that the xret instruction does not modify the xcause.xpil field. This implies that if a software vectoring dispatcher or handler at level L is preempted, when the preempting handler returns the value of mpil will be L in place of the value it had prior to preemption. If there available interrupts at levels < L and greater than the value mpil had prior to preemption, they will not be offered by mnxti in either the preempted or preempting handler. [has this scenario been previously considered?]

@mark-honman
Copy link
Author

mark-honman commented Nov 13, 2024

Also SW example related - in Appendix C the example code has a line
csrci zero, mstatus, MIE

should be
csrrci zero, mstatus, MIE
or
csrci mstatus, MIE

@jb-brelot-nxp jb-brelot-nxp added the v1.0 resolve for 1.0 label Dec 9, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
v1.0 resolve for 1.0
Projects
None yet
Development

No branches or pull requests

2 participants