adding counter overflow interrupt facility

[allenjbaum]

This adds a counter overflow facility for all counters. The timer is grandfathered in as it effectively already has an overflow interrupt. This piggybacks on that interrupt cause, with new CSR that identifies which counter(s) overflowed. It is completely backwards compatible with existing implementations.

[aswaterman]

Sorry for taking so long to get back to you on this, @allenjbaum! This is an important feature.

We like the approach of defining the mcip CSR to hold the overflow bits, and we agree with your remark that there's no need for separate enable bits.

Although overloading the xTIP bits is architecturally elegant, we think this would be a step backwards in practice. In many systems, the timer interrupt is an important performance case. We don't want to add extra instructions to demultiplex the timer interrupt from the counter interrupts, especially since
counter-overflow exceptions don't occur in most normal use cases.

We have some ideas on how to address this deficiency without having to allocate another 3 bits in the mie/mip CSRs, which we'll write up later. No need to rush to solve this problem prior to ratification.

[allenjbaum]

There is some rush here however, as we intend to ship something that may have a conflicting definition... so if you have a different idea, please lay it out (much) sooner rather than later.

…

On Thu, Nov 29, 2018 at 5:06 AM Andrew Waterman ***@***.***> wrote: ***@***.**** requested changes on this pull request. Sorry for taking so long to get back to you on this, @allenjbaum <https://github.com/allenjbaum>! This is an important feature. We like the approach of defining the mcip CSR to hold the overflow bits, and we agree with your remark that there's no need for separate enable bits. Although overloading the xTIP bits is architecturally elegant, we think this would be a step backwards in practice. In many systems, the timer interrupt is an important performance case. We don't want to add extra instructions to demultiplex the timer interrupt from the counter interrupts, especially since counter-overflow exceptions don't occur in most normal use cases. We have some ideas on how to address this deficiency without having to allocate another 3 bits in the mie/mip CSRs, which we'll write up later. No need to rush to solve this problem prior to ratification. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#244 (review)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/Ad96phQMWHVOD2ZQPeyHlh-Fp-D6U7eJks5uz9vNgaJpZM4Xxmh1> .

[aswaterman]

If you’re planning to ship it soon, I strongly advise making it an M-mode-only feature. It’s virtually certain to be at least slightly different than what gets ratified, but if that nonconformance is isolated to M-mode, then standard software is a lot less likely to notice/care.

[kersten1]

@allenjbaum Not sure what needs to happen with this one. Obviously, it can't go forward as is because we are using adoc now. There is an issue attached to this PR that is at least from 2021. Can we port the basic info to this issue and close this PR?

[gfavor]

What is the issue that remains valid and given the counter overflow extensions that have been done since way back (both Sscofpmf and the more recent extensoin by Beeman which I off-hand forget the name of)?

[allenjbaum]

It's ratified now.
Any concerns we had are either resolved or moot.

[kersten1]

So maybe we just close this PR?

[sorear]

The only part of this that can't currently be done in a standardized way is mhpmevent access to instret and cycle, c.f. riscvarchive/riscv-smcntrpmf#1 (comment)

[gfavor]

It's unclear what is the remaining issue preventing closure of this PR? The instret and cycle counters don't have associated event CSRs since they are fixed event counters, Smcntrpmf provides mode-based filtering for these two counters, and there has not been a justified intent to try and add overflow interrupts to these counters for a few reasons (that I don't remember off-hand), as well as the programmable counters can be used for this purpose if needed.

[ved-rivos]

and there has not been a justified intent to try and add overflow interrupts to these counters for a few reasons (that I don't remember off-hand)

Use of overflow interrupts typically involves setting these counters close to overflow such that the overflow interrupt can be used as a sampling event. With instret and cycles there were uses that sampled these counters periodically for purposes such as IPC estimation and hence these could not be used in this mode for sampling. The equivalent events can be counted on the programmable counters for the sample-on-overflow uses. I believe this was the reason they were excluded.

[kersten1]

Based on comments, I'm closing this one. If there is something still relevant, please reopen and I'll get the info moved ASAP.