riscv / riscv-fast-interrupt

Proposal for a RISC-V Core-Local Interrupt Controller (CLIC)
https://jira.riscv.org/browse/RVG-63
Creative Commons Attribution 4.0 International
247 stars 49 forks source link

Require immediate evaluation of clicintie and clicintip #378

Open Silabs-ArjanB opened 8 months ago

Silabs-ArjanB commented 8 months ago

The section CLIC Interrupt Pending (clicintip) contains the following text which, in my opinion, is outdated now that certain CLIC registers are accessible via indirect CSR access:

The conditions for an interrupt trap to occur must be evaluated in a bounded amount of time from when an interrupt becomes, or ceases to be, pending in clicintip, but unlike the MIP/MIE CSRs, there is no requirement that clicintie or clicintip are evaluated immediately following an explicit store to clicintip or clicintie.

I would suggest that this is replaced by:

The conditions for an interrupt trap to occur must be evaluated in a bounded amount of time from when an interrupt becomes, or ceases to be, pending in clicintip, and just like the MIP/MIE CSRs, there is a requirement that clicintie or clicintip are evaluated immediately following an explicit write to clicintip or clicintie.

The original text was only there because clicintip and clicintie were memory mapped. Now it makes a lot more sense to have behavior compatible with CLINT behavior.

dansmathers commented 8 months ago

Can you provide more information on what you are concerned about and why you would like the tighter restriction besides being consistent with CLINT behavior? Implementations can choose to implement a tighter restriction.

In 20230926 TG meeting discussion we discussed why we wouldn't want the requirement you are proposing. (also discussed in #364)

[With] Fixed csr so pipeline can figure out conflicts. [With] indirect csrs, don’t know select register. value may not be ready yet. so things in ind csrs can’t cause pipeline events. so means ind csrs aren’t critical timing. so flexible but more constrained on what they can do. just like AIA, it is a good candidate. so ind csrs is like a local memory map

Silabs-ArjanB commented 8 months ago

I am concerned about the unneeded incompatibility with CLINT from a software paradigm point of view, From a software point of view it is a good feature if an explicit write to a CSR will cause an interrupt immediately (and not some instructions later). This might make verification easier as well.

I don't know how you define 'pipeline events', but the reasoning provided does not sound okay to me. Indirect CSRs might not be timing critical, but that does not in any way imply that there effect should not be precise. This should not be any more difficult to implement than the complexity already imposed on CLINT implementations (and yes, obviously the select register value might need forwarding, but that is nothing special either). And using your own argument that indirect CSRs are not timing critical you could also let a change in the select register take many cycles to prevent the need for forwarding. The thing that matters is the effect after the indirect CSR write, not the performance of the CSR instructions leading up to the CSR write.

dansmathers commented 8 months ago

Sorry I don't have more details about what @kasanovic meant in the TG discussion on 20230926. I was taking notes while Krste was talking and my notes are incomplete.

I was hoping you would have something more concrete (benefits/issues) that would require the tighter restriction.

This should not be any more difficult to implement than the complexity already imposed on CLINT implementations

I'm not sure this statement is accurate. CLINT implementations compare a handful of bits in mip/mie then check mideleg/current_priv/mstatus.xie. The CLIC interrupt comparison tree can have a lot of levels of logic. CLIC supports up to 4096 interrupts. Even smaller numbers of interrupts like 64 can generate lots of levels of logic. For all clicintie/ip set, CLIC selects the highest priv (clicintattr.mode) and level (clicintctl), compares to xintthresh/mintstatus.xil, current_priv/mstatus.xie.

I don't see any perceived benefits or incompatibility with CLINT from a software paradigm without the restriction. clicintip/ie don't appear in any of the interrupt handler examples in the CLIC spec so I don't see any software incompatibility there.
Edge-triggered interrupts can be cleared using shv entry or xnxti. Level interrupts need to be cleared at the source so wouldn't use the indirect CSRs.

Can you provide a more concrete example of how what you are proposing would help/is needed and why it needs to be implemented by everyone? Again, your implementation is free to be more restrictive than CLIC spec.

Silabs-ArjanB commented 8 months ago

Hi @dansmathers I do not have a more concrete example. For me this is only about the, in my opinion, unneeded difference between CLIC and CLINT. Yes, we are free to be more restrictive than the CLIC spec, but that mostly means more fragmentation for us as we can't count on all CPUs that we might want to use implementing this.

dansmathers commented 8 months ago

I read through the AIA spec and didn't see any immediate evaluation requirement of eip/eie CSRs which use the AIA indirect CSR method. You could actually argue that adding an immediate evaluation requirement to CLIC would be an unneeded difference between CLIC and AIA and the CLIC indirect CSR access method is more similar to AIA than to priv spec MIP/MIE CSRs. Do you want to take a look at AIA and see if I missed something?

Silabs-ArjanB commented 8 months ago

I believe you if you say it is not in the AIA spec. Maybe the person who wrote the related part of the Privileged spec should indicate why this was actually important for CLINT then and why that same reasoning does not hold for CLIC / AIA.

dansmathers commented 8 months ago

@kasanovic or @aswaterman or @jhauser-us, can you chime in on this? Thanks.

jhauser-us commented 7 months ago

From the RISC-V Unprivileged spec, Section 11.1.1, "CSR Access Ordering":

Each RISC-V hart normally observes its own CSR accesses, including its implicit CSR accesses, as performed in program order. In particular, unless specified otherwise, a CSR access is performed after the execution of any prior instructions in program order whose behavior modifies or is modified by the CSR state and before the execution of any subsequent instructions in program order whose behavior modifies or is modified by the CSR state.

Because the AIA doesn't say otherwise, my interpretation has always been that any effects of an indirect register write to an IMSIC interrupt file's interrupt-pending or interrupt-enable bits will be immediately visible in registers such as mtopei and mip, at the start of the next executed instruction.

Dan Smathers wrote:

I read through the AIA spec and didn't see any immediate evaluation requirement of eip/eie CSRs which use the AIA indirect CSR method.

The AIA---or any other RISC-V extension document---doesn't need to say that the effects of specific CSR accesses are immediate, because that's supposed to be the default, "unless specified otherwise".

If I'm not mistaken, the AIA explicitly allows delays in interrupt delivery in only two places. First, in Section 3.5, "Memory region for an interrupt file":

When not ignored, writes to an interrupt file’s memory region are guaranteed to be reflected in the interrupt file eventually, but not necessarily immediately.

And in Section 4.8.2, concerning wired interrupts from an APLIC:

Each interrupt signal may be arbitrarily delayed traveling from the APLIC to the proper hart.

I don't offer an opinion about whether the CLIC should allow delays between writes to clicintie or clicintip and the visible effects of those writes. However, it is clear to me, if the CLIC wants to allow such delays, it must say so, just as the Privileged spec explicitly allows a "bounded amount of time" (with certain other constraints) before an interrupt trap actually occurs.

jhauser-us commented 7 months ago

I wrote:

If I'm not mistaken, the AIA explicitly allows delays in interrupt delivery in only two places. [...]

Also a third:

When an APLIC sends an MSI to a hart, there is an unspecified travel delay before the MSI is observed at the hart’s IMSIC.

dansmathers commented 7 months ago

CLIC spec does currently contain this qualification:

section 5.2 The conditions for an interrupt trap to occur must be evaluated in a bounded amount of time from when an interrupt becomes, or ceases to be, pending in clicintip, but unlike the MIP/MIE CSRs, there is no requirement that clicintie or clicintip are evaluated immediately following an explicit store to clicintip or clicintie.

So spec wise, CLIC is mostly ok except should probably also mention clicintctl, clicintattr in that statement above because it wouldn't make sense for those to be evaluated immediately but not clicintip/clicintie.

The question really comes down to if is it necessary to require a tighter constraint to match the behavior of explicit writes to MIP/MIE/MIDELEG CSRs. It seems like all the use cases are safely bounded by xRET and mstatus.

Any examples of why the tighter constraint would be necessary?

Seagate Internal


From: John Hauser @.> Sent: Sunday, March 31, 2024 3:04 PM To: riscv/riscv-fast-interrupt @.> Cc: Dan Smathers @.>; Mention @.> Subject: Re: [riscv/riscv-fast-interrupt] Require immediate evaluation of clicintie and clicintip (Issue #378)

This message has originated from an External Source. Please use proper judgment and caution when opening attachments, clicking links, or responding to this email.

I wrote:

If I'm not mistaken, the AIA explicitly allows delays in interrupt delivery in only two places. [...]

Also a third:

When an APLIC sends an MSI to a hart, there is an unspecified travel delay before the MSI is observed at the hart’s IMSIC.

— Reply to this email directly, view it on GitHubhttps://github.com/riscv/riscv-fast-interrupt/issues/378#issuecomment-2028904345, or unsubscribehttps://github.com/notifications/unsubscribe-auth/APFWH23XU5NIOLUPLTDJSNTY3B27JAVCNFSM6AAAAABEG4BXWOVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDAMRYHEYDIMZUGU. You are receiving this because you were mentioned.Message ID: @.***>

jb-brelot-nxp commented 2 months ago

This issue is still valid :

WE need to understand the requirement of immediate evaluation on privileged specification (MIE/ MIP ). If the reason is valid we need to make sure that if we are using indirect CSR then we need to have an update on the specification depending if we are supporting Indirect CSR or other.

james-ball-qualcomm commented 1 month ago

From reading through this thread, seems like the CLIC has the latitude to define this either way (immediate effect vs. allowing delayed effect) since we already have a clause allowing the latter (which is less restrictive than the former). So, an implementation would be free to have an immediate effect but portable software wouldn't be able to rely on this from implementation to implementation.

From another perspective (certification), allowing a delay effect is harder to certify but not a show stopper.

If SW wanted to support implementations that might have a delayed effect, how could SW be written to wait until the delayed effect is guaranteed to be in effect? Is this even a requirement? I wonder because Dan said in this thread that "all the use cases are safely bounded by xRET and mstatus". I assume this means that an xRET contains a FENCE-like behavior that requires writes to the CLIC indirect CSRs to be "in effect" before completing. Where is that specified? Would be good to look at it. Also, Dan, what did you mean about mstatus in the quote I have here from you?

james-ball-qualcomm commented 1 month ago

We discussed this at the fast interrupt meeting (Dan, Allen, JB involved in live discussion). Consensus is that HW should follow default of priv ISA manual (see John Hauser's comments in this issue) and require writes to indirect CSRs take effect immediately. This is easier for SW and not a burden for HW implementations we can imagine that include a CLIC.

I'll update the spec appropriately.

mark-honman commented 2 weeks ago

Can I take it to mean that "evaluated immediately" or "take effect immediately" mean that, if the clicintie bit for the currently highest-ranked interrupt is cleared by a CSRind write, if mnxti is accessed in the very next cycle it should no longer reflect interrupt that was highest-rank in the cycle that the clicintie bit was cleared? This is likely to degrade CLIC fMax in an FPGA implementation as the number of supported interrupts increases.

Silabs-ArjanB commented 2 weeks ago

Can I take it to mean that "evaluated immediately" or "take effect immediately" mean that, if the clicintie bit for the currently highest-ranked interrupt is cleared by a CSRind write, if mnxti is accessed in the very next cycle it should no longer reflect interrupt that was highest-rank in the cycle that the clicintie bit was cleared?

That is exactly the type of thing that would be guaranteed indeed.

This is likely to degrade CLIC fMax in an FPGA implementation as the number of supported interrupts increases.

No fMax performance degradation is needed at all. I realize that the effect of clicintie might take multiple cycles to realize (because of pipelining of the interrupt controller). That only implies that after a clicintie CSR write you might have to insert some stall cycles for the dependency to get resolved. That would increase the cycle count of a clicintie CSR write (which is maybe a don't care), but it definitely does not have to impact fMax.

mark-honman commented 2 weeks ago

That only implies that after a clicintie CSR write you might have to insert some stall cycles for the dependency to get resolved. That would increase the cycle count of a clicintie CSR write (which is maybe a don't care), but it definitely does not have to impact fMax.

Okay point taken, but it's still a performance hit... and complexity increase. To some extent the performance penalty of inserted stall cycles can be mitigated by stalling only mnxti acceses and holding off action based on the output of the filter tree until the enable change has finished propagating through it - but at the cost of yet more complexity.

I don't see the value in CLINT compatibility in this instance, as CLIC requires its own software implementation.

jhauser-us commented 2 weeks ago

Since this comment thread was started, the Architecture Review Committee has had a few internal discussions that have re-confirmed the blanket rule that the effects of register accesses must appear to software to be immediate unless explicitly stated otherwise, and furthermore re-confirmed that this rule applies as much to indirectly accessed registers as to any other RISC-V registers.

The CLIC spec is allowed to make exceptions to the default of immediate effects. However, the TG would be wise to carefully consider the consequences that any such relaxations may have on software. It can be hard to account for all the subtle ways that software may depend on having a fully consistent view of the machine.

In earlier comments above, I referenced three instances where the Advanced Interrupt Architecture allows a delay to be visible to software. You may find it useful to know that the ARC believes the following should be added to the list: When a hart has an IMSIC, changes in the state of the IMSIC's interrupt files are guaranteed to be reflected in mip.MEIP, mip.SEIP, and hgeip eventually, but not necessarily immediately. The ARC debated but decided against any other relaxations for the AIA. Note that in the Privileged ISA and in the AIA, there is a consistent theme that delays may be visible in the propagation of interrupt-pending signals, but rarely otherwise, even where interrupts are concerned.