Conversation
| transaction carries the WID through the interconnect and all elements | ||
| on the path toward the targeted resource until access permissions can | ||
| be checked. The method of propagating and checking the WID on busses | ||
| The method of propagating and checking the WID on busses |
There was a problem hiding this comment.
I'd rather you keep the original description which explains the tagging/propagation/checking
There was a problem hiding this comment.
It is explained two paragraphs earlier, I consolidated the text. ("[...] meaning that emitted transactions will be examined by a WID-aware interface somewhere along the transaction path to enforce such permission checking.")
There was a problem hiding this comment.
either option looks fine to me - this is maybe just stylistic preference. I read the same information from both versions.
There was a problem hiding this comment.
Yeah, the same information is there. I just reorganized the text a little to consolidate it.
| assign WIDs to its privilege modes and masks to control the values that | ||
| are authorized for use with the principle that no privilege mode can change | ||
| its own WID. The CSRs for these extensions are listed in the Table below. | ||
| The CSRs for the RISC-V Worlds extensions are listed in the following table. |
There was a problem hiding this comment.
Please keep original text.
The purpose of the 2 tables is to emphasize the principle of "no privilege mode can change its own WID".
The second table adds the extensions which provides additional capability, not enforcing this principle.
There was a problem hiding this comment.
I disagree for a couple of reasons. First, the way you've documented things, the splitting of the table refers to CSRs that are "optional" (and by inference, CSRs that are not optional), which has nothing to do with the principle you're mentioning, and indeed it makes no sense (none of these CSRs is any more optional or mandatory than the others, at least not without defining a profile, which is not in scope here). Secondly, the principle you mention is not really pertinent to the purpose of the section, which is to itemize the CSRs introduced by the extensions. That principle does of course motivate the architecture, and it is already mentioned in the appropriate place. But it makes little sense to have separate CSR tables based on whether a CSR is deemed to serve that principle.
There was a problem hiding this comment.
My goal was to emphasize on the fact that no privilege mode can change its own WID, nothing else.
We can update the leading text to make the intend of the 2 tables clearer. While you disagree, I think it is indeed useful to explain the architecture motivation.
There was a problem hiding this comment.
The architecture motivation is described earlier, in an appropriate section. Here we are in the section that itemizes the CSRs introduced by the extensions. There is no good rationale for dividing those CSRs into two separate tables based on whether or not they enforce a design principle. If you wish to list the CSRs that illustrate the design principle, please list them in the section that explains that design principle.
There was a problem hiding this comment.
IMO this section is just the CSR list, and architectural motivations and operation of the (all optional) extensions are listed in their relevant sections. It doesn't make sense to me to split the CSR tables into an arbitrary grouping - M mode CSRs and S mode CSRs could equally be split into separate tables for instance. To me this split implies a baseline for Worlds, plus options - but I dont believe there is consensus on such a baseline ? Or maybe there is ?
There was a problem hiding this comment.
No I don't think there's any concept of a baseline/options. I think what Perrine is after is better handled by embellishing the previous section's description of the "no privilege can change its own WID" principle.
| addresses may support multiple contexts, with each context potentially being in | ||
| a different world. As the WID to associate with outgoing transactions is | ||
| typically determined by a control register, a given (software or hardware) | ||
| context can only be present in one world at a time. |
There was a problem hiding this comment.
As the WID to associate with outgoing transactions is
typically determined by a control register, a given (software or hardware)
context can only be present in one world at a time.
A software context running on a hart agent or a hardware context on a device agent can only be present in one world at a time.
There was a problem hiding this comment.
I think what I wrote here explains the situation better. Please observe that there is nothing to prevent you from defining an agent that does not have the "one world at a time" behavior. E.g. if an agent is forwarding transactions and performs a transform on the input WID to generate the output WID, then the "one world at a time" notion doesn't apply at all. I've tried to clarify that what is meant by "one world at a time" stems from the fact that the given "context" (which is a loose terms and could mean almost anything) is usually tagging outgoing transactions with a WID determined by a register field. Hence, the "one world" that the context is currently in is determined by whatever WID is stored in the register. The "context" might be per-privilege level (in the case of a hart) or per-channel (in the case of a device that has multiple initiator interfaces each with their own WID configuration). But if you're in a situation where the output WID is not derived from a register value, then the "one world at a time" idea may not apply.
There was a problem hiding this comment.
My hiccup is only on the "typically determined by a control register" portion.
There was a problem hiding this comment.
What is it about the "typically determined by a control register" portion that poses a problem? The "one world at a time" behavior is "typical" only because the outgoing WID for transactions is "typically" determined by a register. At least that's how it seems to me after thinking about it. I could have tried to do it by negation, by suggesting that the one-world-at-a-time behavior occurs when the outgoing WID is "not determined by the properties of the transaction itself". Or I could have tried to do it by introducing the concept of a "marker" (which is basically what the "control register(s)" are). Either way, the one-world-at-a-time concept needed to be explained because it isn't universal and it's not a mandate of the spec so much as it is a consequence of a typical usage pattern, of which the ISA is the obvious example. Without my change, the reader is left with the impression that an agent must be temporally bound to a single world, which isn't explained (and isn't necessarily true).
There was a problem hiding this comment.
As wid may be determined by default values, wires, a simple statement that 'a software context running on a hart agent or a hardware context on a device agent can only be present in one world at a time.' makes sense. I think even if later entity changes WID or merges worlds, this statement is still true. There is no notion of splitting a WID dependent on target for instance, that would allow a context to exist in two worlds simultaneously.
There was a problem hiding this comment.
@andrewdellow, the issue is that "an agent can only be in one world at a time" statement is only true if there is a setting (i.e. a register) that determines WIDs for processed transactions, that's why I added a note to that affect. If an agent is forwarding transactions (e.g. like a IOPMP instance) and the output WID is determined from the input WID, then the "only in one world at a time" statement doesn't make sense any more and in fact is outright misleading.
- "The RISC-V Worlds" is not a thing, remove the "The" - change definition of Worlds from "execution contexts and resources" to "system partitions consisting of execution contexts and resources" - "read and write" --> "load, store, and fetch" - clarify up-front that WIDs are associated with transactions - De-mystify and simplify the "one world at a time" stuff - Re-order some text, and consolidate the explanation(s) of transaction checking. - Pull the cache note into a distinct paragraph Signed-off-by: Geoffrey Thorpe <gthorpe@mips.com>
- move mwid-related notes to the Smwid bullet - simplify the Smwdeleg language Signed-off-by: Geoffrey Thorpe <gthorpe@mips.com>
All the extensions are optional, so splitting some of the CSRs into a separate "optional" table is misleading. Also, the extensions have just been introduced in the previous section, so no need to repeat their descriptions. Signed-off-by: Geoffrey Thorpe <gthorpe@mips.com>
- Name the extension (in bold) as we do when describing the other extensions. - Clarify the WID field in the CSR and remove the erroneous "writable" qualifier. - minor language tweaks. Signed-off-by: Geoffrey Thorpe <gthorpe@mips.com>
- highlight the extension name consistently. - consolidate and simplify the description of the WID field. - minor language tweaks. Signed-off-by: Geoffrey Thorpe <gthorpe@mips.com>
- fix missing highlighting of extension names - simplify the description of slwid's WID encoding - explain why permission checks (e.g. outside the hart) might not be able to raise exceptions, to make the text easier to follow. - minor language tweaks Signed-off-by: Geoffrey Thorpe <gthorpe@mips.com>
b587404 to
780dc17
Compare
Please see the individual commits for detail about the changes.