Add nmi bit to etrigger.#408
Conversation
Fixes #407.
| <field name="0" bits="XLEN-7:11" access="R" reset="0" /> | ||
| <field name="nmi" bits="10" access="R/W" reset="0"> | ||
| When this optional bit is set, non-maskable interrupts cause this | ||
| trigger to fire just as it does for other exceptions. |
There was a problem hiding this comment.
what does "just as it does for other exceptions" mean? Why do you need to add that clarification, why not just say "trigger to fire".
There was a problem hiding this comment.
I wonder if this bit belongs in the Interrupt Trigger register rather than Exception Trigger. It seems like nmi is more like interrupts than it is like exceptions, but I read the discussion in the Issue and understand the reasoning.
Is the intention to trigger on all nmi's regardless of M/S/U mode, or would the mode have to match an enabled mode to trigger?
I would suggest text along the lines of "When this optional bit is set, enable this trigger for non-maskable interrupts that are taken from M, S, or U mode."
There was a problem hiding this comment.
I assumed nmi would still depend on the m/s/u bits, for consistency with the other sources that can cause triggers in this register. It also affords a little more flexibility, although I'm not sure when somebody would want to trigger e.g. on NMIs from U mode but not other modes. I'm still leaning towards consistency. What do you think?
There was a problem hiding this comment.
Sounds like we should discuss at the next meeting. NMIs "are only used for hardware error conditions" according to the Priv Spec, so it makes more sense to me to respond to any NMI regardless of mode enables.
There was a problem hiding this comment.
In today's meeting we agreed to ignore m/s/u for nmi. I've updated this PR.
The rationale is that nobody can think of a reason that somebody might want to only trigger on NMI in a specific mode, and by ignoring those bits it's possible to use a single trigger to trigger on e.g. illegal instructions in U mode, but on all NMIs.
Ernie Edgar (ernie-sifive)
left a comment
Ernie Edgar (ernie-sifive)
left a comment
There was a problem hiding this comment.
This looks great, thanks for making this change.
3 participants
Fixes #407.