CLICINTCTLBITS May Not Always Be Fixed

[billhuffman]

Section 3.7 says that CLICINTCTLBITS is a fixed parameter. But I think it may vary. If cliccfg.nmbits is writable, then when it changes, the number of bits in the interrupt priority tree allocated to the mode in which the interrupt should be taken will vary. CLICINTCTLBITS will then vary when cliccfg.nmbits is changed to utilize the remaining bits for level and priority.

[dansmathers]

CLICINTCTLBITS is supposed to represent the number of implemented bits in clicintctl[i]. E.g, some smaller implementations may only want to have 4 flops per interrupt allocated to selecting level/priority instead of the full 8.
The spec just describes when there are fewer than 8 bits implemented what the possible level/priority values the hardwired values take.
there can't really be a dynamic logic area saving since this is fixed at implementation time.

Are you thinking of an adjustable ctlbits option for a power saving option where the clic user could dynamically adjust which bits the priority encoder compares? So the area implemented is the max 8 flops per interrupt but different use cases could attempt to reduce the dynamic power of the priority tree decoding?

[billhuffman]

Yes, it is the number of bits implemented in clickintctl[i]. But that can vary. For simplicity, let's assume no priority. Just privileges and levels. And let's assume we have four flops per interrupt and four-wide comparators in the tree.

If cliccfg.nmbits=0 as it is at reset, we have 15 interrupt levels available. CLICINTCTLBITS = 4.

If we then write cliccfg.nmbits=1, one of the flops and comparator bits is used for the privilege the interrupt will be taken into. That leaves three bits and allows 7 levels of interrupts. CLICINTCTLBITS=3.

If we require CLICINTCTLBITS=3 in both cases, we restrict the use of the flops and comparator bits in an unfortunate way.

[dansmathers]

I implemented it as a comparison of {priv_of_interrupt,cliccintctl[CLICINTCTLBITS implemented]}
so cliccfg.nmbits only affects the value of priv_of_interrupt.
e.g, nmbits=0 then priv_of_interrupt is 2'b11 (machine). nmbits=1, priv_of_interrupt is either 2'b00 (user) or 2'b11 (machine) based on cliccintattr.mode setting.
So the priority tree comparison is always 2+CLICINTCTLBITS wide

Here is the CLIC doc section I referenced for this:
the CLIC hardware combines the valid bits in clicintattr.mode and clicintctl to form an unsigned integer, then picks the global maximum across all pending-and-enabled interrupts based on this value

So I don't think the intent is when cliccfg.nmbits=1, one of the clicintctl bits is used for privilege.
Does that make sense?
Any suggestions on how to clarify the doc to make that more clear?

[billhuffman]

I think the implementation you describe is legal. But it wastes expensive bits when, say, it is desired to support user interrupts, but in many situations they won't be used. When they're not used, there could have been twice as many interrupt levels. So, no problem with that implementation. But the more efficient one shouldn't be forbidden.

[dansmathers]

Hi Bill. can you give another example? I think I'm not understanding your point. In my implementation and understanding of the spec, when CLICINTCTLBITS is 4, the implemented size of clicintctl is 4 bits and we have 16 level/priority bits always. machine has 16 level/priority, supervisor has 16 level/priority, user has 16 level/priority. The mode of the interrupt comes from clicintattr.mode. if the implementation only supports m-mode, 2 bits can be saved there by hardcoding mode to 11. If 2 modes are implemented, only 1 bit has to be implemented there. if 3 modes are implemented, 2-bits are used on each interrupt for mode. Are you suggesting a way to repurpose unused clicintattr.mode bits as level/priority bits instead of priv-mode in the level/priority decoder if the implementation supports 3 modes but only 1 or 2 modes will be used by the application?

[dansmathers]

by the way, I don't see a section 3.7. are we looking at the same version of the spec?

[billhuffman]

I think another chapter has been added to the spec and what was 3.7 a few weeks back is now 4.7.

Maybe what I'm suggesting is what you said in your last sentence - depends on what you meant by it. :-)

If we take the implementation you describe, I believe it has 6 bits of comparison for each stage of the interrupt priority tree, right? 2 of those bits are coming from clicintattr[i].mode and 4 are coming from clicintctl[i]. I'm proposing that the same 6 flops per interrupt and the same 6 comparator bits per tree stage can also be used to support 1 bit of clicintattr[i].mode and 5 bits of clicintctl[i] or 0 bits of clicintattr[i].mode and 6 bits of clicintctl[i] depending on the setting of cliccfg.nmbits.

This requires no logic per interrupt and no logic in the interrupt priority tree. The logic is in the creation of what's written to or read from clicintattr[i] and clicintctl[i] and in the interpretation of the output of the interrupt priority tree. For example, the 6 bits that come out are parsed as 2 and 4, 1 and 5, or 0 and 6 for the three cases.

[kasanovic]

Discussion in TG meeting converged on assuming that any change to CLICINTCLTBITS would happen before normal operation, so the text of spec should be OK (will look for any places that assume this parameter is always hardwired).
Modified title of section to drop "implementation" from parameters b4f66df

[billhuffman]

I realized later in the meeting that I think we've lost something in some statements when we moved the mode to clicintattr[i]. There are some places that don't comprehend sharing bits between mode and level/priority.

[billhuffman]

Section 4.2.2 of the CLIC spec says:

Although the interrupt level is an 8-bit unsigned integer, the number of bits actually assigned or implemented can be fewer than 8. As described above, the number of bits assigned is specified in cliccfg.nlbits. The number of bits actually implemented can be derived from cliccfg.nlbits and a fixed parameter clicinfo.CLICINTCTLBITS (with value between 0 to 8) which specifies bits implemented for both interrupt level and priority.

This seems to forbid what I think is an obvious implementation. Let’s say that the interrupt comparator tree is four bits wide. This can support M-mode interrupts only with 16 levels, or M/S-mode interrupts with 8 levels each or M/S/U-mode interrupts with 4 levels each. This would be varied by setting cliccfg to 0x08, 0x26, or 0x44. All three settings would use the same four bits in different ways. This implementation seems to be forbidden because the parameter clicinfo.CLICINTCTLBITS is not fixed.

Perhaps clicinfo.CLICINTCTLBITS was intended to include all three of mode, interrupt level, and priority.

The table just before the end of the section doesn’t seem to allow for mode either.

Then there’s a related issue that there could be an implementation where the interrupt comparator tree is 10 bits wide. It can have 256 levels for each of M, S, and U modes. That implementation takes a lot of logic for comparators, but it seems like it is intended to be legal. It seems to be counted out by the description.

Section 4.3 says:

The CLICINTCTLBITS field specifies how many hardware bits are actually implemented in the clicintctl registers, with 0 ≤ CLICINTCTLBITS ≤ 8. The implemented bits are kept left-justified in the most-significant bits of each 8-bit clicintctl[i] register, with the lower unimplemented bits treated as hardwired to 1.

It seems like it should say how many bits are implemented in clicattr.mode plus clickintctl with a range 0 =< somebitcount =< 10. The justification in clicintctl[i] depends on nmbits. The width of the comparator tree minus nmbits needs to fit in clicintctl[i] or else we define which bits do fit into clickintctl[i].

Section 4.7 makes the same sort of assumptions. And Section 6.

Assuming we want to trade off bits between all three of mode, level, and priority, either we need to make CLICINTCTLBITS include mode or we need to not make it fixed and create another fixed value to include all three.

[Kevin-Andes]

he definition in the current spec is valid and counts the number of bits in the comparator tree slightly differently. It is actually simpler: how many interrupt levels you need (e.g., 8 levels) plus how many modes you need (e.g., 2 modes). In this example, the comparator tree needs 3 (for level) + 1 (for mode) = 4 bits.

We made the number of modes an independent parameter because it is determined by the platform and requirement of your application (e.g., RTOS only needs 1 mode, Linux needs 3 modes). On the other hand, the number of interrupt levels is determined by how many high/medium/low interrupts (or devices) exist in the system. So this 2 numbers don’t really have much dependency.

In the oldest version of spec, clicintctl includes vector, mode, level and priority. However, many members complained it was too restricting and difficult to use because changing one parameter will affect all the others. Therefore, we decoupled the mode to make more room and make it easier to program.

Thanks.

   Regards,
   Kevin

[dansmathers]

adding email thread discussion:
After the discussion we had today, I think it would be reasonable to leave the decoupling of setting clicintattr[i].mode from setting clicintctl[i] to solve the problem you mention, Kevin. But still make the parameter in clicinfo cover them all. I think the complaint was probably because software didin’t want always to include the mode in the value it wrote to clicintctl[i].

[dansmathers]

If we need one parameter to cover both interrupt levels and modes, then maybe we can define something like CLICCOMPAREBITS to avoid potential confusion

[dansmathers]

So if implemented CLICCOMPAREBITS is 10, when cliccfg.nmbits is set to 0 (machine-only), would bottom 2-implemented bits no longer be accessible via clicintctrl so they would need to be cleared when cliccfg.nmbits change to avoid hidden priority behavior? Or easier to just cap CLICCOMPAREBITS at 8. so max 256 levels when cliccfg.nmbits is 0, max 128 levels when cliccfg.nmbits is 1, max 64 levels when cliccfg.nmbits is 2?

[billhuffman]

For reasons of what we, as an IP provider, will be asked for, I’d prefer to cap it at 10 rather than 8. I think the natural thing is that the low two bits are the ones that aren’t used and they should be zero to avoid strange behavior. However, I think it’s reasonable to say that they’re set to zero when clicintctl[i] is written rather than when cliccfg.nmbits is changed. After all, if clicintctl[i] isn’t written after cliccfg.nmbits is changed, the results will be pretty strange anyway. Bill From: Dan Smathers ***@***.***> Sent: Tuesday, July 20, 2021 2:45 PM To: riscv/riscv-fast-interrupt ***@***.***> Cc: Bill Huffman ***@***.***>; Author ***@***.***> Subject: Re: [riscv/riscv-fast-interrupt] CLICINTCTLBITS May Not Always Be Fixed (#158) EXTERNAL MAIL So if implemented CLICCOMPAREBITS is 10, when cliccfg.nmbits is set to 0 (machine-only), would bottom 2-implemented bits no longer be accessible via clicintctrl so they would need to be cleared when cliccfg.nmbits change to avoid hidden priority behavior? Or easier to just cap CLICCOMPAREBITS at 8. so max 256 levels when cliccfg.nmbits is 0, max 128 levels when cliccfg.nmbits is 1, max 64 levels when cliccfg.nmbits is 2? — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub<https://urldefense.com/v3/__https:/github.com/riscv/riscv-fast-interrupt/issues/158*issuecomment-883613923__;Iw!!EHscmS1ygiU1lA!QCK48tSBMXyjXBF6YakdvlMQzKUFHdyGZBna38gh-zBUI0D0E8CRtd8Z1HpWOYQ$>, or unsubscribe<https://urldefense.com/v3/__https:/github.com/notifications/unsubscribe-auth/AKXXKKGPNJHRFBMLEGIZA63TYW73PANCNFSM46MPDACA__;!!EHscmS1ygiU1lA!QCK48tSBMXyjXBF6YakdvlMQzKUFHdyGZBna38gh-zBUI0D0E8CRtd8ZGs-jKTs$>.

[Kevin-Andes]

On a second thought, it is probably more confusing to define something like CLICCOMPAREBITS to be 10 bits. This is because 8-bit clicintctl already caps the interrupt level to be 8 bits and thus the remaining 2 bits cannot be used for interrupt level anyway. In other words, we cannot freely assign all these comparator bits to "level" or "mode."

Therefore, I think the current definition of CLICINTCTLBITS is fine, and efficient hardware can implement the sharing of level/mode implicitly (without the need to explicitly telling SW users as they probably don't care).

[billhuffman]

I would like to follow the previous couple of comments with CLICCOMPAREBITS between 0 and 10 inclusive.

[Kevin-Andes]

This dynamic sharing between interrupt mode and level bits is an OPTIONAL hardware optimization that only needed by some implementations (while other implementations may prefer to fix them). In addition, hardware can easily implement this feature implicitly without modifying current existing programming interface. Therefore, our spec should maintain a simple friendly programming interface (i.e., separate mode and interrupt level fields) and not burden programmers with calculation for implicit effects of optional sharing.

Nevertheless, with the new relatively inexpensive discovery method, we can add a parameter to let people know if any sharing exists and how many sharing bits are available. This is just read-only information and not changing our existing programming interface.

[billhuffman]

I agree with everything you say. I think the effort here is to have the names and readable constants defined so that all implementations look obvious to software. Bill From: Kevin-Andes ***@***.***> Sent: Tuesday, August 3, 2021 11:27 AM To: riscv/riscv-fast-interrupt ***@***.***> Cc: Bill Huffman ***@***.***>; Author ***@***.***> Subject: Re: [riscv/riscv-fast-interrupt] CLICINTCTLBITS May Not Always Be Fixed (#158) EXTERNAL MAIL This dynamic sharing between interrupt mode and level bits is an OPTIONAL hardware optimization that only needed by some implementations (while other implementations may prefer to fix them). In addition, hardware can easily implement this feature implicitly without modifying current existing programming interface. Therefore, our spec should maintain a simple friendly programming interface (i.e., separate mode and interrupt level fields) and not burden programmers with calculation for implicit effects of optional sharing. Nevertheless, with the new relatively inexpensive discovery method, we can add a parameter to let people know if any sharing exists and how many sharing bits are available. This is just read-only information and not changing our existing programming interface. — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub<https://urldefense.com/v3/__https:/github.com/riscv/riscv-fast-interrupt/issues/158*issuecomment-891941856__;Iw!!EHscmS1ygiU1lA!U5T-6iJBDfIg8AMfFtsd55AxTYCTCqcFuLzrhdlTOy787v-ix1kCseQ0F-D6SsA$>, or unsubscribe<https://urldefense.com/v3/__https:/github.com/notifications/unsubscribe-auth/AKXXKKEPZ3CDEIADBECL7DLT3ADGBANCNFSM46MPDACA__;!!EHscmS1ygiU1lA!U5T-6iJBDfIg8AMfFtsd55AxTYCTCqcFuLzrhdlTOy787v-ix1kCseQ0KcF2boU$>. Triage notifications on the go with GitHub Mobile for iOS<https://urldefense.com/v3/__https:/apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675__;!!EHscmS1ygiU1lA!U5T-6iJBDfIg8AMfFtsd55AxTYCTCqcFuLzrhdlTOy787v-ix1kCseQ0ITtfg1c$> or Android<https://urldefense.com/v3/__https:/play.google.com/store/apps/details?id=com.github.android&utm_campaign=notification-email__;!!EHscmS1ygiU1lA!U5T-6iJBDfIg8AMfFtsd55AxTYCTCqcFuLzrhdlTOy787v-ix1kCseQ0cx1FY0M$>.

[kasanovic]

Total bits is CLICINTMLPBITS, which is a read-only parameter on all implementations. MLP=Mode+Level+Priority

[kasanovic]

More parameters are needed to describe how the total bits can be allocated to M/L/P uses. Some implementations might fix the allocation, and this can be discovered by WARL accesses to cliccfg.nmbits/nlbits. This does make an assumption that the number of priority bits can be determined by npbits = max(0, min(CLICINTMLPBITS-cliccfg.nmbits-cliccfg.nlbits, 8-cliccfg.nlbits)).

Some custom uses might allow CLICINTMLPBITS to be modified to reserve some per-interrupt control bits for custom uses, but this should not require separate cliccfg.npbits field (assuming always allow sharing of priority and level bits).

[billhuffman]

The spec doesn't seem to be updated for any of this. CLICINTMLPBITS doesn't appear, at least in the current pdf. There are half a dozen or so places where CLICINTCTLBITS is assumed to be only level+priority bits, which is fine if we have CLICINTMLPBITS somewhere. But if CLICINTMLPBITS is constant, as I assume it is, then CLICINTCTLBITS is NOT constant as the spec says. Also, section 4.2.2 has a non-normative comment that the number of level bits can be determined by CLICINTCTLBITS - nmbits. We also have to figure out where CLICINTMLPBITS appears - in clicinfo? Somewhere else? It's computed by adding CLICINTCTLBITS and nmbits?

[dansmathers]

From Allen Baum email: adding to this issue since it is related. Also depends on how issue #96 is resolved.
The spec sec 4.2.2 says
The number of available level bits can be determined by subtracting
the number of mode bits from CLICINTCTLBITS.

Shouldn't that be

The number of available level bits can be determined by
the min(number of level bits , CLICINTCTLBITS)

which matches all the examples

[dansmathers]

closed by #369