riscv-isa-manual icon indicating copy to clipboard operation
riscv-isa-manual copied to clipboard

adding counter overflow interrupt facility

Open allenjbaum opened this issue 6 years ago • 2 comments

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.

allenjbaum avatar Oct 19 '18 23:10 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 [email protected] wrote:

@aswaterman 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 https://github.com/riscv/riscv-isa-manual/pull/244#pullrequestreview-179784340, or mute the thread https://github.com/notifications/unsubscribe-auth/Ad96phQMWHVOD2ZQPeyHlh-Fp-D6U7eJks5uz9vNgaJpZM4Xxmh1 .

allenjbaum avatar Nov 29 '18 16:11 allenjbaum

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.

aswaterman avatar Nov 29 '18 18:11 aswaterman

@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?

kersten1 avatar Mar 18 '24 15:03 kersten1

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)?

gfavor avatar Mar 18 '24 18:03 gfavor

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

allenjbaum avatar Mar 18 '24 20:03 allenjbaum

So maybe we just close this PR?

kersten1 avatar Mar 19 '24 00:03 kersten1

The only part of this that can't currently be done in a standardized way is mhpmevent access to instret and cycle, c.f. https://github.com/riscv/riscv-smcntrpmf/issues/1#issuecomment-1637036334

sorear avatar Mar 19 '24 04:03 sorear

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.

gfavor avatar Mar 19 '24 05:03 gfavor

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.

ved-rivos avatar Mar 19 '24 07:03 ved-rivos

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

kersten1 avatar Mar 26 '24 19:03 kersten1