search

riscv / riscv-isa-manual

RISC-V Instruction Set Manual
https://riscv.org/
Creative Commons Attribution 4.0 International
3.68k stars 643 forks source link

Machine mode only and user CSRs #543

Open jm4rtin opened 4 years ago

jm4rtin commented 4 years ago

Perhaps I missed it but does the privileged specification explicitly state that an implementation must implement user (or supervisor) mode in order for the CSRs in the user (or supervisor) space (bits 9:8 encoded as 00 for user CSRs) to be implemented? Put another way, if an implementation only has machine mode, should access to any user (or supervisor) CSR throw an illegal instruction exception?

Secondly, if there is no such hard rule, should the read-only shadows of the mcycle, minstret, mhpmcountrn, mcycleh, minstreth and mhpmcountern CSRs: cycle, instret, hpmcountern, cycleh, instreth and hpmcountern respectively be accessible in a machine mode only implementation?

In section 3.1.12 the specification states:

When the CY, TM, IR, or HPMn bit in the mcounteren register is clear, attempts to read the cycle, time, instret, or hpmcountern register while executing in S-mode or U-mode will cause an illegal instruction exception.

In this case the read to the read-only shadow CSRs has to be in machine mode (because there is no user mode) and so this does not apply; therefore, if the read-only shadow CSRs were implemented they should respond the same way as the machine mode version.

The question is, if an implementation is machine mode only (no user or supervisor mode), do the read-only shadow CSRs for the HPM exist or do they throw an illegal instruction exception? I can't find anything in the specification to clearly indicate the proper behavior.

aswaterman commented 4 years ago

If you think of those CSRs as unprivileged CSRs, rather than U-mode CSRs, it becomes a bit clearer that such CSRs can exist even without U-mode.

The counters are actually an orthogonal extension described in the unprivileged ISA manual. Since they're defined in that spec, it's implicit that they can exist irrespective of the privileged architecture configuration.

My recommendation would be that if you're providing mcycle, you should also provide cycle. The cost is nearly nil, and it's easier for software. But you can have a conforming implementation either way.

allenjbaum commented 4 years ago

Another nice corner case for compliance.

Is the reasoning simply if they appear in the unprivileged spec they may (but NOT must) be implemented?

I’d be much happier if it were just one or the other since instead of implementation specific. Now we have to configure the formal model to match the implementation behavior for each of the CSRs. I wonder what the formal model does for this? I’m sure it only does one or the other of the behaviors.

-Allen

On Jul 17, 2020, at 2:23 PM, Andrew Waterman notifications@github.com wrote:

If you think of those CSRs as unprivileged CSRs, rather than U-mode CSRs, it becomes a bit clearer that such CSRs can exist even without U-mode.

The counters are actually an orthogonal extension described in the unprivileged ISA manual. Since they're defined in that spec, it's implicit that they can exist irrespective of the privileged architecture configuration.

My recommendation would be that if you're providing mcycle, you should also provide cycle. The cost is nearly nil, and it's easier for software. But you can have a conforming implementation either way.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub, or unsubscribe.

aswaterman commented 4 years ago

It’s not that they appear in the unpriv spec; it’s that the counters are a separate extension (which should perhaps be named - Zicount?)

On Fri, Jul 17, 2020 at 8:52 PM Allen Baum notifications@github.com wrote:

Another nice corner case for compliance.

Is the reasoning simply if they appear in the unprivileged spec they may (but NOT must) be implemented?

I’d be much happier if it were just one or the other since instead of implementation specific. Now we have to configure the formal model to match the implementation behavior for each of the CSRs. I wonder what the formal model does for this? I’m sure it only does one or the other of the behaviors.

-Allen

On Jul 17, 2020, at 2:23 PM, Andrew Waterman notifications@github.com wrote:

If you think of those CSRs as unprivileged CSRs, rather than U-mode CSRs, it becomes a bit clearer that such CSRs can exist even without U-mode.

The counters are actually an orthogonal extension described in the unprivileged ISA manual. Since they're defined in that spec, it's implicit that they can exist irrespective of the privileged architecture configuration.

My recommendation would be that if you're providing mcycle, you should also provide cycle. The cost is nearly nil, and it's easier for software. But you can have a conforming implementation either way.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub, or unsubscribe.

— You are receiving this because you commented.

Reply to this email directly, view it on GitHub https://github.com/riscv/riscv-isa-manual/issues/543#issuecomment-660419158, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAH3XQUPHZ5TGUAZR7LWEM3R4EMARANCNFSM4O6Y4V5A .

allenjbaum commented 4 years ago

Ah. chapter 10 is a separate section in the spec for counters - but nowhere does it indicate, or even hint, that it is actually a separate extension (and therefore isn't required to be implemented), as opposed to a base ISA feature. Am I looking at an outdated version?. It also says that the H versions that RV32I must implement to get to the high half of the 64b cycle/time/instret counters are not required for RV64I

On Sat, Jul 18, 2020 at 8:53 AM Andrew Waterman notifications@github.com wrote:

It’s not that they appear in the unpriv spec; it’s that the counters are a separate extension (which should perhaps be named - Zicount?)

On Fri, Jul 17, 2020 at 8:52 PM Allen Baum notifications@github.com wrote:

Another nice corner case for compliance.

Is the reasoning simply if they appear in the unprivileged spec they may (but NOT must) be implemented?

I’d be much happier if it were just one or the other since instead of implementation specific. Now we have to configure the formal model to match the implementation behavior for each of the CSRs. I wonder what the formal model does for this? I’m sure it only does one or the other of the behaviors.

-Allen

On Jul 17, 2020, at 2:23 PM, Andrew Waterman <notifications@github.com

wrote:

If you think of those CSRs as unprivileged CSRs, rather than U-mode CSRs, it becomes a bit clearer that such CSRs can exist even without U-mode.

The counters are actually an orthogonal extension described in the unprivileged ISA manual. Since they're defined in that spec, it's implicit that they can exist irrespective of the privileged architecture configuration.

My recommendation would be that if you're providing mcycle, you should also provide cycle. The cost is nearly nil, and it's easier for software. But you can have a conforming implementation either way.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub, or unsubscribe.

— You are receiving this because you commented.

Reply to this email directly, view it on GitHub < https://github.com/riscv/riscv-isa-manual/issues/543#issuecomment-660419158 , or unsubscribe < https://github.com/notifications/unsubscribe-auth/AAH3XQUPHZ5TGUAZR7LWEM3R4EMARANCNFSM4O6Y4V5A

.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/riscv/riscv-isa-manual/issues/543#issuecomment-660501719, or unsubscribe https://github.com/notifications/unsubscribe-auth/AHPXVJTIXSFABO6QYV7GPS3R4HAO5ANCNFSM4O6Y4V5A .

aswaterman commented 4 years ago

The high parts should not be implemented in RV64I

On Sun, Jul 19, 2020 at 1:09 AM Allen Baum notifications@github.com wrote:

Ah. chapter 10 is a separate section in the spec for counters - but nowhere does it indicate, or even hint, that it is actually a separate extension (and therefore isn't required to be implemented), as opposed to a base ISA feature. Am I looking at an outdated version?. It also says that the H versions that RV32I must implement to get to the high half of the 64b cycle/time/instret counters are not required for RV64I

  • which implies they are optional for RV64i, as opposed to being prohibited. Why not simply prohibit them?

On Sat, Jul 18, 2020 at 8:53 AM Andrew Waterman notifications@github.com wrote:

It’s not that they appear in the unpriv spec; it’s that the counters are a separate extension (which should perhaps be named - Zicount?)

On Fri, Jul 17, 2020 at 8:52 PM Allen Baum notifications@github.com wrote:

Another nice corner case for compliance.

Is the reasoning simply if they appear in the unprivileged spec they may (but NOT must) be implemented?

I’d be much happier if it were just one or the other since instead of implementation specific. Now we have to configure the formal model to match the implementation behavior for each of the CSRs. I wonder what the formal model does for this? I’m sure it only does one or the other of the behaviors.

-Allen

On Jul 17, 2020, at 2:23 PM, Andrew Waterman < notifications@github.com

wrote:

If you think of those CSRs as unprivileged CSRs, rather than U-mode CSRs, it becomes a bit clearer that such CSRs can exist even without U-mode.

The counters are actually an orthogonal extension described in the unprivileged ISA manual. Since they're defined in that spec, it's implicit that they can exist irrespective of the privileged architecture configuration.

My recommendation would be that if you're providing mcycle, you should also provide cycle. The cost is nearly nil, and it's easier for software. But you can have a conforming implementation either way.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub, or unsubscribe.

— You are receiving this because you commented.

Reply to this email directly, view it on GitHub <

https://github.com/riscv/riscv-isa-manual/issues/543#issuecomment-660419158

, or unsubscribe <

https://github.com/notifications/unsubscribe-auth/AAH3XQUPHZ5TGUAZR7LWEM3R4EMARANCNFSM4O6Y4V5A

.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub < https://github.com/riscv/riscv-isa-manual/issues/543#issuecomment-660501719 , or unsubscribe < https://github.com/notifications/unsubscribe-auth/AHPXVJTIXSFABO6QYV7GPS3R4HAO5ANCNFSM4O6Y4V5A

.

— You are receiving this because you commented.

Reply to this email directly, view it on GitHub https://github.com/riscv/riscv-isa-manual/issues/543#issuecomment-660606520, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAH3XQU32BDYOPR3BQYFLB3R4KS4HANCNFSM4O6Y4V5A .

allenjbaum commented 4 years ago

There is still that word there - should not, or must not? "Should" not doesn't change the meaning I am objecting to.

Also: If I'm reading unpriv spec chapter 9 correctly, Zicsr is specifically is there for timer/counter CSR ops. The CSR instructions themselves (CSRR[W|SC][ I] ) are defined nowhere else. Since Mmode requires CSR ops, and Mmode is required, that seems to imply that zicsr is always required, and thus so are the counter/timers.

I know better, of course, but if this is not the case, then this chapter should be split into a section that describe only counter/timer CSRs, and another with the description of the instructions.

If zicsr only refers to counter/timer CSRs, then tell me if my interpretation below is flawed: The D and F extensions and require CSRs, so

This leaves only the umode counter/timer CSRs, which appear to be part of the draft "Counters" extension (e.g. zicnt) and could

On Sun, Jul 19, 2020 at 3:12 PM Andrew Waterman notifications@github.com wrote:

The high parts should not be implemented in RV64I

On Sun, Jul 19, 2020 at 1:09 AM Allen Baum notifications@github.com wrote:

Ah. chapter 10 is a separate section in the spec for counters - but nowhere does it indicate, or even hint, that it is actually a separate extension (and therefore isn't required to be implemented), as opposed to a base ISA feature. Am I looking at an outdated version?. It also says that the H versions that RV32I must implement to get to the high half of the 64b cycle/time/instret counters are not required for RV64I

  • which implies they are optional for RV64i, as opposed to being prohibited. Why not simply prohibit them?

On Sat, Jul 18, 2020 at 8:53 AM Andrew Waterman < notifications@github.com> wrote:

It’s not that they appear in the unpriv spec; it’s that the counters are a separate extension (which should perhaps be named - Zicount?)

On Fri, Jul 17, 2020 at 8:52 PM Allen Baum notifications@github.com wrote:

Another nice corner case for compliance.

Is the reasoning simply if they appear in the unprivileged spec they may (but NOT must) be implemented?

I’d be much happier if it were just one or the other since instead of implementation specific. Now we have to configure the formal model to match the implementation behavior for each of the CSRs. I wonder what the formal model does for this? I’m sure it only does one or the other of the behaviors.

-Allen

On Jul 17, 2020, at 2:23 PM, Andrew Waterman < notifications@github.com

wrote:

If you think of those CSRs as unprivileged CSRs, rather than U-mode CSRs, it becomes a bit clearer that such CSRs can exist even without U-mode.

The counters are actually an orthogonal extension described in the unprivileged ISA manual. Since they're defined in that spec, it's implicit that they can exist irrespective of the privileged architecture configuration.

My recommendation would be that if you're providing mcycle, you should also provide cycle. The cost is nearly nil, and it's easier for software. But you can have a conforming implementation either way.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub, or unsubscribe.

— You are receiving this because you commented.

Reply to this email directly, view it on GitHub <

https://github.com/riscv/riscv-isa-manual/issues/543#issuecomment-660419158

, or unsubscribe <

https://github.com/notifications/unsubscribe-auth/AAH3XQUPHZ5TGUAZR7LWEM3R4EMARANCNFSM4O6Y4V5A

.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub <

https://github.com/riscv/riscv-isa-manual/issues/543#issuecomment-660501719

, or unsubscribe <

https://github.com/notifications/unsubscribe-auth/AHPXVJTIXSFABO6QYV7GPS3R4HAO5ANCNFSM4O6Y4V5A

.

— You are receiving this because you commented.

Reply to this email directly, view it on GitHub < https://github.com/riscv/riscv-isa-manual/issues/543#issuecomment-660606520 , or unsubscribe < https://github.com/notifications/unsubscribe-auth/AAH3XQU32BDYOPR3BQYFLB3R4KS4HANCNFSM4O6Y4V5A

.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/riscv/riscv-isa-manual/issues/543#issuecomment-660717341, or unsubscribe https://github.com/notifications/unsubscribe-auth/AHPXVJRBK54TKKNHWAQ5DADR4NVURANCNFSM4O6Y4V5A .

aswaterman commented 4 years ago

Can't really say "must not" for this sort of thing, because of the possibility of non-conforming extensions.

The unprivileged spec explicitly admits the possibility of alternative privileged architectures, so "Zicsr is always required" isn't strictly speaking true. If you restrict the goal of compliance testing to systems that implement the current RISC-V privileged architecture, then it is true that omitting Zicsr is noncompliant.

aswaterman commented 4 years ago

BTW, RDTIMEH/RDCYCLEH/RDINSTRETH are described as "RV32I-only", which I believe addresses your concern.

allenjbaum commented 4 years ago

My pendantic hat is one very tightly here: IF that is the case, why bother to use the word "must" anywhere in the priv spec? That word alone is used 73 times. Require is used at least 41 times, though many of them don't mean "must" in context. IF those registers are described as RV32i only, then why does the spec merely say these " instructions are not required in RV64I" instead of "these are RV32i only". Those two statements are not equivalent.

The priv spec should be a description of the priv spec, not a possibly non-conforming priv spec.

In other news: I said that the CSR ops and the timer/counters should be separated. As I scroll further down, I see that, yea, they are. Oops. MY confusion was because the spec says

"The counters and timers are no longer considered mandatory parts of the standard base ISAs, and so the CSR instructions required to access them have been moved out of the base ISA chapter into this separate chapter"

That was what confused me. The same reasoning could be applied to FP CSRs, but that wasn't sufficient to move them to a separate extension.

Are the "CSR instructions required to access them" specifically the ones that address the counter/timers (and by extension, FP CSRs)? Or are they all CSR ops?

If Mmode is required, and Mmode requires zicsr then (like all the other base ops) the implication is that zicsr is always required. So the obvious question is why it needs to be a separate extension instead of being part of the base spec, which is also always required?. The statement above does not answer that question.

On Sun, Jul 19, 2020 at 6:04 PM Andrew Waterman notifications@github.com wrote:

Can't really say "must not" for this sort of thing, because of the possibility of non-conforming extensions.

The unprivileged spec explicitly admits the possibility of alternative privileged architectures, so "Zicsr is always required" isn't strictly speaking true. If you restrict the goal of compliance testing to systems that implement the current RISC-V privileged architecture, then it is true that omitting Zicsr is noncompliant.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/riscv/riscv-isa-manual/issues/543#issuecomment-660743353, or unsubscribe https://github.com/notifications/unsubscribe-auth/AHPXVJSS7RKTPP4ZMEWFMQDR4OJ3NANCNFSM4O6Y4V5A .

aswaterman commented 4 years ago

If Mmode is required, and Mmode requires zicsr then (like all the other base ops) the implication is that zicsr is always required. So the obvious question is why it needs to be a separate extension instead of being part of the base spec, which is also always required?. The statement above does not answer that question.

Already answered above: The unprivileged spec explicitly admits the possibility of alternative privileged architectures, so "Zicsr is always required" isn't strictly speaking true.

aswaterman commented 4 years ago

That was what confused me. The same reasoning could be applied to FP CSRs, but that wasn't sufficient to move them to a separate extension.

The first paragraph of the F extension chapter says that F requires Zicsr.

aswaterman commented 4 years ago

IF those registers are described as RV32i only, then why does the spec merely say these " instructions are not required in RV64I" instead of "these are RV32i only".

It does say they are RV32I-only:

jm4rtin commented 4 years ago

So if I implement just the standard stuff (rv32iZicsr) with only machine mode (I may have omitted some other extension or two that was important - sorry if I did) and wish to leave the privileged HPM CSRs tied to 0 does that mean I am implementing "Zicount" with none of the optional features? If I am implementing "Zicount" does that mean that both the privileged and unprivileged CSRs should exist? If I don't implement "Zicount" then neither privileged and unprivileged CSRs should exist? Or is there still the possibility of the privileged but not unprivileged CSRs existing in a "only standard" configuration?

I get that idea that certain extensions should be able to stand on their own and co-exist with other non-standard extensions (or variants of the standard extensions); however, if you're using just the standard extensions it would be good to have some clarity and a definitive determination on if these CSRs are implemented in some fashion or not instead of a "recommendation".

jscheid-ventana commented 3 years ago

Is this stuck on "should" vs "must"? Aren't non-conforming extensions definitionally violating or "overriding" some requirement, therefore it's appropriate to have extensions simply state their requirements clearly?

allenjbaum commented 3 years ago

I interpret this as you stated: mcycle and minstret are required to be compatible, assuming RV32i priv mode requirements carry over to RV32E. If you don't want to be priv mode compatible, you can drop ziCSR and with it mcycle, minstret, interrupts, traps, etc. etc. - or keep everything except mcycle and minstret.

On Tue, Aug 10, 2021 at 11:55 AM Joshua Scheid @.***> wrote:

Is this stuck on "should" vs "must"? Aren't non-conforming extensions definitionally violating or "overriding" some requirement, therefore it's appropriate to have extensions simply state their requirements clearly?

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/riscv/riscv-isa-manual/issues/543#issuecomment-896235174, or unsubscribe https://github.com/notifications/unsubscribe-auth/AHPXVJSJ3H5PC4NRN7XCPTDT4FY2ZANCNFSM4O6Y4V5A . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&utm_campaign=notification-email .

kersten1 commented 4 months ago

I'm closing this one in favor of issue #560 https://github.com/riscv/riscv-isa-manual/issues/560

Please reopen if the discussion needs to continue.