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

How should non-normative text be specified in CLIC spec #376

Open
christian-herber-nxp opened this issue Mar 5, 2024 · 5 comments
Open
Labels
v1.0 resolve for 1.0

Comments

@christian-herber-nxp
Copy link
Collaborator

In the section "Trampoline for Preemptible Inline Handlers":

as part of irq_enter, the following check is performed.
csrrsi a0, mnxti, MIE # Get highest interrupt, enable interrupts.
beqz a0, exit

Two questions:

  1. Is it really beneficial to check for a higher prio interrupt at this point? It doesn't really reduce a worst-case latency by a lot, e.g. if the interrupt arrives two cycles later, it still has to sit through the entire ISR. On the other hand, you are adding two instructions into the path of every interrupt.
  2. Checking for 0. I understand this could happen if a level based interrupt is retracted. But this would be a rare case, and accepting a short handler in this case would be better than having this branch instruction always.
@dansmathers
Copy link
Collaborator

dansmathers commented Mar 5, 2024

This is the preemptible example so it does need to enable mstatus.mie to allow higher level interrupts to preempt. I think the beqz is then really just error checking if the interrupt was retracted or an shv interrupt showed up.
So I think we could replace

csrrsi a0, mnxti, MIE # Get highest interrupt, enable interrupts.
beqz a0, exit

with
csrrsi x0, mstatus, MIE
if we wanted to avoid the extra beqz.

But it is just non-normative text so isn't too important. Would you like me to add a comment that we could avoid the beqz?

@christian-herber-nxp
Copy link
Collaborator Author

This is the preemptible example so it does need to enable mstatus.mie to allow higher level interrupts to preempt. I think the beqz is then really just error checking if the interrupt was retracted or an shv interrupt showed up.

I didn't think about the shv interrupt. That's actually a good point. On the other hand, if the shv interrupt is at the same level, it shouldn't really preempt a running non-shv interrupt, should it? It would be allowed, as the handler is yielding its priority free willingly, but it's not in the spirit of the levels and prios.

So I think we could replace

csrrsi a0, mnxti, MIE # Get highest interrupt, enable interrupts.
beqz a0, exit

with csrrsi x0, mstatus, MIE if we wanted to avoid the extra beqz.

But it is just normative text so isn't too important. Would you like me to add a comment that we could avoid the beqz?

you mean non-normative? Actually, what defines that something is normative or non-normative?
It might make sense to move all those software sections into the appendix.

@dansmathers
Copy link
Collaborator

sorry. yes, non-normative. I corrected my comment above. good point about moving to an appendix

@dansmathers
Copy link
Collaborator

I see both AIA and priv spec use inline commentary in italics to specify non-normative text.
e.g.
Commentary on our design decisions, implementation options, and application is formatted as
in this paragraph, and can be skipped if the reader is only interested in the specification itself.

No spec change is required, just noting that chapter 11 (interrupt handling software through Chapter 17 CLIC interrupt ID ordering recommendations) all should be in an appendix/inline commentary. I'll add a label of spec formatting to this issue that can be resolved during ratification phase but not gate freeze. Do you agree?

@dansmathers dansmathers added the v1.0 resolve for 1.0 label Mar 7, 2024
@christian-herber-nxp
Copy link
Collaborator Author

totally fine

@dansmathers dansmathers changed the title Trampoline for Preemptible Inline Handlers How should non-normative text be specified in CLIC spec Mar 8, 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