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
245 stars 49 forks source link

Clearing interrupt pending bits #27

Closed marceg closed 4 years ago

marceg commented 5 years ago

As with most interrupt controllers (that I've worked with), the clearing of the pending bit for an edge-triggered interrupt can be optimized by clearing it automatically when software commits to handle the interrupt.

There are two such points in CLIC:

This saves two instructions in the vectored case, and three instructions in the non-vectored case.

Note that example code shows sw x0, INTERRUPT_FLAG, a0 in both cases. This expands to an AUIPC; SW sequence and works for the vectored case (except that sw needs to be sb, if I understand the memory map). Spelling out the sequence clarifies instruction count and timing (and using LUI makes the code relocatable):

LUI  a0, %hi(clicintip)        # Section 14.2 mentions this LUI
SB   x0, INTERRUPT_NUMBER(a0)  # for vectored, interrupt number is implied

The non-vectored case needs to add the interrupt number to the clicintip[] address (which requires an extra register thus a bit of re-ordering of surrounding instructions to get a1) and takes up 3 instructions:

LUI  a1, %hi(clicintip) # Section 14.2 mentions this LUI
ADD  a1, a1, a0         # for non-vectored, interrupt number in a0
SB   x0, 0(a1)

Related areas the spec might clarify:

aswaterman commented 5 years ago

The assembler pseudoinstruction sw x0, INTERRUPT_FLAG, a0 is syntactic sugar for

1: auipc a0, %pcrel_hi(INTERRUPT_FLAG)
   sw x0, %pcrel_lo(1b)(a0)
marceg commented 5 years ago

Ah, thank you. (I didn't find any documentation that covers this syntax.) I updated my comment accordingly.

David-Horner commented 4 years ago

As you requested the proposal now automatic clears clicintip for edge-triggered interrupts at entry for Selective Hardware Vectoring (SHV) mode and on executing a modifying mnxti instruction typically just before exiting that interrupt handling.

For level triggered interrupts the clicintip state should be cleared by writing to the device causing the interrupt.

I believe the revised text now answers your request for clarification: "The method for clearing edge-triggered interrupts, in the early parts of the spec (which right now mention clicintip[] without further details)."

For clarification request "Whether this pending bit can be set (triggering the interrupt) by writing a 1 to it.". It is partly answered as clicintip can be directly written: "clicintip[i] is a read-write register. Software-based (direct) writes to these pending bits have priority over hardware-based writes (triggers)." and "Note: During normal operation, software does not need to clear pending bits because CLIC hardware already supports automatic clearing of pending bits for edge-triggered interrupts. As for level-triggered interrupts, they should be cleared at interrupt sources (devices) so no need to clear the pending bits. Therefore, software usually only needs to modify pending bits in the initialization process or testing. "

What isn't answered is if software setting clicintip triggers an interrupt. As far as I can tell the text is silent on this point.

I am surprised and disappointed that your remaining question was not answered. "Whether the pending bit is the lsbit (bit 0) of its one-byte register." I will raise a separate issue including this last concern. Sifive used bit zero for its original proposal.

David-Horner commented 4 years ago

Re: "Whether this pending bit can be set triggering the interrupt"

The spec does imply it is possible: "To select an interrupt to present to the core, the CLIC hardware combines the valid bits in clicintattr.mode and clicintctl to form an unsigned value, then picks the global maximum across all pending-and-enabled interrupts based on this value."

However. the spec should be stipulate whether software setting the pending bit does indeed make the interrupt pending.

For consistency within the model I believe it should.

Kevin-Andes commented 4 years ago

Pending bit is located in bit 0 (LSB). Writing pending bits with SW will definitely set/clear edge-triggered interrupts. In contrast, for level-triggered interrupts, SW writing to pending bits is ignored completely.

Kevin-Andes commented 4 years ago

As a side note, in the course of our group discussion, some members expressed that setting pending bits to activate level-triggered interrupts is a useful feature for testing. Another suggestion was to define it as “undefined” to give each company the freedom to decide whether they need this testing feature. However, we finally concluded that writing to pending bits for level-triggered interrupts will be ignored for the following reasons:

  1. This “sticky-bit” behavior is against the traditional concept of level-triggering, so it can create confusion for users.
  2. To have this sticky property, an extra latch is needed so the HW cost will increase.
  3. SW interrupt handler will need an extra step to manually clear this bit in addition to the mandatory clearance of the external level-device. This extra step may be easily forgotten and also add more instructions and latency.
  4. Defining the writing behavior entirely as “undefined” can create nightmare for compatibility and compliance tests.
  5. Lastly, users can easily add a simple external testing module if they really need this testing feature.
Kevin-Andes commented 4 years ago

Clarified in the spec with commit 740e39a.