Moving CLIC memory mapped registers to indirect CSRs

[christian-herber-nxp]

I wanted to bring up the possibility of migrating the memory mapped registers to indirectly accessed CSRs. I assume that the reason for using memory mapped registers was the number of registers being too high for regular CSRs, but I am happy to learn from somebody with a longer history on that.

With the indirect CSRs on the way, I would think this is a nicer way of integrating those registers, which would also be more consistent with AIA. Also, with well defined CSR indeces, you no longer have to discover the base addresses of the CLIC memory regions.

Is this something which could be considered?

[dansmathers]

adding link to indirect CSR github: https://github.com/riscv/riscv-indirect-csr-access

[dansmathers]

Discussed in last 10 minutes of 8/29/2023 meeting. Probably something that should be discussed more.

[dansmathers]

Pros: Similar to AIA. ton of registers, nice to have as CSRs but don't want to use limited CSR space. indirect CSRs opens up this option. don't like to use pmp to separate clic memory regions since pmp regions are expensive. easier to implement indirect CSRs in SAIL/spike?
can limit corner cases vs. using memory interconnect? for software consistency/debugger could remove CLIC base addr from parameter and have it be the same between implementations. stability/portability.

Cons: probably delay path to ratification? already some implementations using memory mapped solutions? could we have a way to coexist?

sub extension to clic to use indirect CSRs? single way is best for one piece of reference software.

if we were to implement, would we pack the enables or keep the same memory format? CSR instructions allow setting easily. configuring at system startup if packed would be easier.

[christian-herber-nxp]

One question I have around the indirect CSRs is related to the user registers. The indirect CSR access is only defined for M and S mode. CLIC is sort of an outlier with User mode support. Could this be an issue?

[christian-herber-nxp]

This is a proposal for a potential CSR map for CLIC using S*csrind.

• I would expect the CSRs which are currently directly accesible to stay unchanged.
• xcliccfg is a single 32-bit register, can be mapped to a single indirect CSR
• clicintip and clicintie should be collapsed into individual bits of CSRs, analogous to AIA
• clicinttrig[0-31] will be indirectly accessed CSRs
• clicintattr[i] and clicintctl[i] are both only one byte and can be merged into i indirect CSR - alternative is to map two sources into one CSR, but I think it is better to have a more straight forward mapping and then 16-bit reserved space in the the CSR

The corresponding indirect CSR map would look the following:

Address	Register	Access
0x0	cliccfg	R/W
0x6+i	clicinttrig[i]	R/W
0x26+i	reserved (bits XLEN-1:16), clicintattr[i], clicintctl[i]	R/W
0x1026+k*2	clicintip[k]	R or RW
0x1027+k*2	clicintie[k]	RW

where k is in the range from 0 to 127, and each clicint{ip,ie}[k] register contains 32 sources.

The CSR map is not optimized to be small when there are few interrupt sources implemented, as the entire space anyway needs to be reserved for CLIC.

If my math checks out, this needs 0x1125 indirect CSRs in the indirect address space.

[christian-herber-nxp]

Giving some background to the proposal above: Addressing: The address layout is optimized to accesses that are multiples of 6, as there are 6 xiregs. E.g. a xiselect value of 1 would map the first 6 clicinttrig registers and so on. One thing that i would change to my proposal is not to interleave cliintip and clicintie, so that the ie bits can be configured on block at start up.

[christian-herber-nxp]

@kasanovic I believe a CSR-only version of CLIC should work. The memory-mapped register in IMSIC really comes from the need for MSIs. In CLIC, I would expect that some of the interrupt lines going towards CLIC can be triggered by other components of the SoC, including other harts. How other cores can trigger those can be outside of the scope of CLIC. E.g. in a multicore system, a PLIC/APLIC could be in place to do this job.

[dansmathers]

Closed by pull #364

[dansmathers]

copying comments from pull #364 and moving to issue for easier tracking:

Silabs-ArjanB 2/2/2024

The original memory mapped option seems completely removed. No parameter for this option seems to have been introduced. There is a statement that "A CSR accessible via one method may or may not be accessible via the other method such as memory-mapped access". This "other method" seems however not specified. I would really prefer if the memory mapped options stays explicitly specified.

Silabs-ArjanB 2/3/2024

I think furthermore that mandating indirect CSR access just increases the interrupt latency without any real benefit over using a memory mapped strategy. The reason I think interrupt latency is negatively affected is that the miselect CSR adds state that now needs to be saved and restored (which conflicts with the goal of having an extension that improves interrupt latency). My proposal would be to mark indirect CSR access as a possible post-v1.0 addition as opposed to a pre-v1.0 replacement of a memory mapped strategy.

christian-herber-nxp 2/3/2024

Interesting point regarding the additional state. In most cases, however, I would argue that ISRs would not need to access indirect CSRs, and then no state needs to be saved. Given the fact that indirect CSR accesses are a possibility now, I think they are the best choice for CLIC, and we want to have the best possible version of CLIC. Of course I understand that it might be desired to maintain the memory mapped option in the interest of existing implementations, but from software point of view that would essentially lead to two kinds of CLICs (fueling any concerns regarding fragmentation).

tovine 3/9/2024

I realize I'm a bit late to this discussion, but does this mean that the previous memory-mapped version of the CLIC spec is no longer to be supported at all, or will it still remain an option to choose one or the other? This is a major change that suddenly leaves existing implementations (of the memory-mapped version) incompatible and out in the cold, and will inevitably lead to ecosystem fragmentation that's quite unfortunate.

[dansmathers]

This line was added to the CLIC spec to say that implementations can continue to access via memory-mapped access but they need to support the indirect CSR access method.

A CSR accessible via indirect CSR access may or may not be accessible via another method such as memory-mapped access.

It is recognized that it is a big change and I think we are all concerned that having two options for accessing the clic registers will lead to fragmentation. It's possible that those fragmentation worries are overstated because as pointed out above, the clicintie/ip/attr/ctrl registers don't need to be accessed within interrupt handlers. They will mainly be accessed during setup/mode changes which would probably be highly custom among implementations anyway. But the intent is it should be easy/low impact to add indirect CSR access to existing implementations. If you have an existing CLIC design and see issues with adding indirect CSR access to the clic registers, can you describe them?

[tovine]

This fragmentation is already upon us by introducing this change, just one example I could find in open source software here on GitHub, from the Zephyr RTOS project: https://github.com/zephyrproject-rtos/zephyr/blob/main/drivers/interrupt_controller/intc_nuclei_eclic.c Many more hits can be found in that repo: https://github.com/search?q=repo%3Azephyrproject-rtos%2Fzephyr%20clic&type=code

The current/old memory-mapped CLIC spec is already implemented in currently shipping devices, so whether we like it or not it will have to be supported by software going forward. I suspect removing it from the spec altogether would lead to a lot of confusion.

While I do see the appeal of doing it in a similar way to the AIA spec (and the other benefits mentioned earlier in this thread), making a fundamental change like this after having kept the old memory mapped approach relatively stable for several years already is quite a disruptive move (for example this CLIC driver in Zephyr was added in December 2021), and I suspect the inertia of the old implementations will force a forked SW ecosystem in this case.

[christian-herber-nxp]

Implementations done before ratification cannot prohibit further spec development. Otherwise we also would not have the vector 1.0 spec as it is today. SW like Zephyr will be able to deal with it, especially as this only affects the config code, not the handlers. I expect both versions to be supported there. After all, HW will drive what is available in an RTOS more than a standard. The statement that this leads to a forked ecosystem is in my opinion not true. It simply means there will be multiple versions of driver support, as is the case with almost any device going through product generations. Also, CLIC might not stand still, and there may be changes beyond v1.0 which require further changes to the SW.

[tovine]

Fair enough, as long as the legacy documentation is available and easy to find for the people developing software support then I won't complain. It could for example be a link in the document to an old tag of the spec before this change.

[dansmathers]

From [RISC-V] [tech-chairs] ARC meeting minutes 2024/3/12: The [CLIC] spec was received and AR has begun reviewing the specifications. ARC noticed the email threads on retaining the alternative memory-mapped interface to the CLIC, so will work with the TG on clarifying the path forward.

[jnk0le]

Interesting point regarding the additional state. In most cases, however, I would argue that ISRs would not need to access indirect CSRs, and then no state needs to be saved.

Only if it's callee saved (or compiler does ipra) if we are about to call standard C functions.

Are those CSRs even subject to calling conventions BTW?

[dansmathers]

copying tovine comment on pull request #392 here to help with tracking.

Seems like indirect CSR support is still mandatory, meaning that existing (memory-mapped-only) implementations are technically not compliant. What's the best course of action here? In my head it doesn't make a lot of sense for an implementation to offer both access methods, so maybe have it either or, and define two separate extensions e.g. Smclicmem or Smclicind?