Develop schema for common CSR implementation-defined behaviors to avoid creating a parameter for each one #185
Replies: 7 comments 4 replies
-
|
My first post was to help define requirements. This post is an attempt at starting a discussion about potential solutions. The one I have in mind at the moment is to have the presence of extensions determine what CSRs and CSR fields are present but not their behavior. If just the presence of an extension isn't enough to define the presence of CSRs and/or CSR fields, then add parameters to extensions to capture this (I'm guessing this is already the case). I see things like "definedBy: Zicntr" in the mcycle CSR so that is clearly stating that the CSR existence is a function of the presence of an extension. I don't see "definedBy: " yet in the CSR fields but maybe that could be added? Instead of adding more parameters to capture the behavior of CSRs and/or CSR fields, have a data-driven schema that defines the behavior of CSR fields for common cases. There is already a field's type in the CSR field schema (which is usually constant but can be a function). Could we also add the field's value if it is RO (ReadOnly) to replace parameters like VENDOR_ID_BANK and VENDOR_ID_OFFSET and the CSR field's width to replace parameters like MTVAL_WIDTH. If the field type isn't constant, the CSR field YAML could have an array of allowed types and then an implementation constrains it. So, instead of type being a function like below, it is just an array [CsrFieldType::RW, CsrFieldType::RO] and then UDB creates a CSR field parameter to allow an implementation to constrain it. So, besides extension parameters, there'd also be the concept of CSR field parameters which can be created automatically by UDB when it detects they are needed. This would get rid of the MUTABLE_MISA_* parameters. |
Beta Was this translation helpful? Give feedback.
-
It is already there. It's only used when a field is defined by an extension different than the CSR it belongs to. See mstatus for plenty of examples.
Note that the field's value when read-only is already present and used in the schema-- it's the We can't add a specific value to the implementation-independent description under arch/ because we don't know what it should be. You can add a specific value using the overlay mechanism (under cfgs/), but that's not helpful to generate things like the ISA docs. There might be some way of specifying read-only values that doesn't involve explicit parameter creation, but the parameter would have to exist nonetheless with an implicit name (e.g., you still need to be able to name it to configure the C++ model or generate implementation docs). I'd have to think about whether or not that ultimately would improve the situation.
Yes, but we lose information with that scheme. As defined below, you can tell when the field should be RO or RW. With a simple array, you don't have that information.
|
Beta Was this translation helpful? Give feedback.
-
|
Interesting idea. I don't know of a document that (bidirectionally) maps
supported extensions to the presence of CSRs.
That list should exist.
Obviously, it isn't 1:1; there are CSRs that are required by more than one
extension.
But even there, it isn't the entire CSR that is required; it may only be a
specific field of a CSR that is required.
I'm not sure about the reverse: are there CSRs that no (named) extension
requires (in which case we need to give it a name)?
The Mmode "extension" is the only one I can think of in that category.
Obviously, CSRs can have more than one legal format, and can be a mixture
of RO and RW fields
The fields themselves can be a mixture of RO and RW subfields,
and RW subfields may have more than one legal range of values supported.
It's more complex than it needs to be, but most of that complexity will
never show up in a real implementation.
…On Thu, Oct 24, 2024 at 1:25 PM Derek Hower ***@***.***> wrote:
My first post was to help define requirements. This post is an attempt at
starting a discussion about potential solutions.
The one I have in mind at the moment is to have the presence of extensions
determine what CSRs and CSR fields are present but not their behavior. If
just the presence of an extension isn't enough to define the presence of
CSRs and/or CSR fields, then add parameters to extensions to capture this
(I'm guessing this is already the case). I see things like "definedBy:
Zicntr" in the mcycle CSR so that is clearly stating that the CSR existence
is a function of the presence of an extension. I don't see "definedBy: "
yet in the CSR fields but maybe that could be added?
It is already there. It's only used when a field is defined by an
extension different than the CSR it belongs to. See mstatus for plenty of
examples.
Instead of adding more parameters to capture the behavior of CSRs and/or
CSR fields, have a data-driven schema that defines the behavior of CSR
fields for common cases. There is already a field's type in the CSR field
schema (which is usually constant but can be a function). Could we also add
the field's value if it is RO (ReadOnly) to replace parameters like
VENDOR_ID_BANK and VENDOR_ID_OFFSET and the CSR field's width to replace
parameters like MTVAL_WIDTH.
Note that the field's value when read-only is already present and used in
the schema-- it's the reset_value key.
We can't add a specific value to the implementation-independent
description under arch/ because we don't know what it should be. You can
add a specific value using the overlay mechanism (under cfgs/), but that's
not helpful to generate things like the ISA docs. There might be some way
of specifying read-only values that doesn't involve explicit parameter
creation, but the parameter would have to exist nonetheless with an
implicit name (e.g., you still need to be able to name it to configure the
C++ model or generate implementation docs). I'd have to think about whether
or not that ultimately would improve the situation.
If the field type isn't constant, the CSR field YAML could have an array
of allowed types and then an implementation constrains it. So, instead of
type being a function like below, it is just an array [CsrFieldType::RW,
CsrFieldType::RO] and then UDB creates a CSR field parameter to allow an
implementation to constrain it. So, besides extension parameters, there'd
also be the concept of CSR field parameters which can be created
automatically by UDB when it detects they are needed. This would get rid of
the MUTABLE_MISC_* parameters.
Yes, but we loose information with that scheme. As defined below, you can
tell *when* the field should be RO or RW. With a simple array, you don't
have that information.
C:
location: 2
description: |
Indicates support for the `C` (compressed) extension.
[when,"MUTABLE_MISA_C == true"]
Writing 0 to this field will cause all compressed instructions to raise an `IllegalInstruction` exception.
Additionally, IALIGN becomes 32.
type(): |
return (implemented?(ExtensionName::C) && MUTABLE_MISA_C) ? CsrFieldType::RW : CsrFieldType::RO;
reset_value(): |
return implemented?(ExtensionName::C) ? 1 : 0;
—
Reply to this email directly, view it on GitHub
<#185 (reply in thread)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AHPXVJTAKUWUPJ7GS7H6SFTZ5FJVFAVCNFSM6AAAAABQRZGQUKVHI2DSMVQWIX3LMV43URDJONRXK43TNFXW4Q3PNVWWK3TUHMYTCMBUGUZTGMI>
.
You are receiving this because you were mentioned.Message ID:
<riscv-software-src/riscv-unified-db/repo-discussions/185/comments/11045331
@github.com>
|
Beta Was this translation helpful? Give feedback.
-
|
This is the structure that riscv-config allows....I think
On Fri, Oct 25, 2024 at 1:44 PM Allen Baum ***@***.***>
wrote:
… Interesting idea. I don't know of a document that (bidirectionally) maps
supported extensions to the presence of CSRs.
That list should exist.
Obviously, it isn't 1:1; there are CSRs that are required by more than one
extension.
But even there, it isn't the entire CSR that is required; it may only be a
specific field of a CSR that is required.
I'm not sure about the reverse: are there CSRs that no (named) extension
requires (in which case we need to give it a name)?
The Mmode "extension" is the only one I can think of in that category.
Obviously, CSRs can have more than one legal format, and can be a mixture
of RO and RW fields
The fields themselves can be a mixture of RO and RW subfields,
and RW subfields may have more than one legal range of values supported.
It's more complex than it needs to be, but most of that complexity will
never show up in a real implementation.
On Thu, Oct 24, 2024 at 1:25 PM Derek Hower ***@***.***>
wrote:
> My first post was to help define requirements. This post is an attempt at
> starting a discussion about potential solutions.
>
> The one I have in mind at the moment is to have the presence of
> extensions determine what CSRs and CSR fields are present but not their
> behavior. If just the presence of an extension isn't enough to define the
> presence of CSRs and/or CSR fields, then add parameters to extensions to
> capture this (I'm guessing this is already the case). I see things like
> "definedBy: Zicntr" in the mcycle CSR so that is clearly stating that the
> CSR existence is a function of the presence of an extension. I don't see
> "definedBy: " yet in the CSR fields but maybe that could be added?
>
> It is already there. It's only used when a field is defined by an
> extension different than the CSR it belongs to. See mstatus for plenty of
> examples.
>
> Instead of adding more parameters to capture the behavior of CSRs and/or
> CSR fields, have a data-driven schema that defines the behavior of CSR
> fields for common cases. There is already a field's type in the CSR field
> schema (which is usually constant but can be a function). Could we also add
> the field's value if it is RO (ReadOnly) to replace parameters like
> VENDOR_ID_BANK and VENDOR_ID_OFFSET and the CSR field's width to replace
> parameters like MTVAL_WIDTH.
>
> Note that the field's value when read-only is already present and used in
> the schema-- it's the reset_value key.
>
> We can't add a specific value to the implementation-independent
> description under arch/ because we don't know what it should be. You can
> add a specific value using the overlay mechanism (under cfgs/), but that's
> not helpful to generate things like the ISA docs. There might be some way
> of specifying read-only values that doesn't involve explicit parameter
> creation, but the parameter would have to exist nonetheless with an
> implicit name (e.g., you still need to be able to name it to configure the
> C++ model or generate implementation docs). I'd have to think about whether
> or not that ultimately would improve the situation.
>
> If the field type isn't constant, the CSR field YAML could have an array
> of allowed types and then an implementation constrains it. So, instead of
> type being a function like below, it is just an array [CsrFieldType::RW,
> CsrFieldType::RO] and then UDB creates a CSR field parameter to allow an
> implementation to constrain it. So, besides extension parameters, there'd
> also be the concept of CSR field parameters which can be created
> automatically by UDB when it detects they are needed. This would get rid of
> the MUTABLE_MISC_* parameters.
>
> Yes, but we loose information with that scheme. As defined below, you can
> tell *when* the field should be RO or RW. With a simple array, you don't
> have that information.
>
> C:
> location: 2
> description: |
> Indicates support for the `C` (compressed) extension.
>
> [when,"MUTABLE_MISA_C == true"]
> Writing 0 to this field will cause all compressed instructions to raise an `IllegalInstruction` exception.
> Additionally, IALIGN becomes 32.
> type(): |
> return (implemented?(ExtensionName::C) && MUTABLE_MISA_C) ? CsrFieldType::RW : CsrFieldType::RO;
> reset_value(): |
> return implemented?(ExtensionName::C) ? 1 : 0;
>
> —
> Reply to this email directly, view it on GitHub
> <#185 (reply in thread)>,
> or unsubscribe
> <https://github.com/notifications/unsubscribe-auth/AHPXVJTAKUWUPJ7GS7H6SFTZ5FJVFAVCNFSM6AAAAABQRZGQUKVHI2DSMVQWIX3LMV43URDJONRXK43TNFXW4Q3PNVWWK3TUHMYTCMBUGUZTGMI>
> .
> You are receiving this because you were mentioned.Message ID:
> <riscv-software-src/riscv-unified-db/repo-discussions/185/comments/11045331
> @github.com>
>
|
Beta Was this translation helpful? Give feedback.
-
|
Allen, I don't see any link to the riscv-config structure. Is this the link you were intending to provide: https://riscv-config.readthedocs.io/en/stable/yaml-specs.html#csr-template |
Beta Was this translation helpful? Give feedback.
-
|
I'm trying to find examples that use the riscv-config schema. I've found https://github.com/riscv-software-src/riscv-config/blob/dev/examples/rv32i_isa.yaml but it doesn't seem to have information like the description or address for CSRs. Also, is there a YAML file somewhere for each CSR and then a configuration YAML that describes how a particular CSR is configured for a particular implementation? This is what UDB does but does riscv-config follow this approach too? So far it just looks like there is one YAML file that describes the CSR and how it exists in a particular implementation. |
Beta Was this translation helpful? Give feedback.
-
|
I see an ISA YAML but not an architecture YAML (that is implementation-independent) and then a configuration YAML that describes an implementation and references those architecture YAML files.
|
Beta Was this translation helpful? Give feedback.
-
|
riscv-config was designed to be a configuration file, not a database, so
anything that is defined by the architecture and can't be configured
shouldn't be part of it.
The address of a CSR can be mapped to its name (and vice versa) elsewhere -
that's the spec, it doesn't change, it can't be configured (see encoding.h)
The description of a CSR can also be mapped to its name (and vice versa)
else - that's also the spec, it doesn't change, it can't be configured (see
priv/unpriv spec)
riscv-config describes the configuration, which includes
- whether the CSR exists, and possibly what happens if you try to access
the address if it doesn't
- what the reset value is (if any)
- and dependency states that select the format)
- field values (including which bits are RW, which are rdonly, WARL
illegal->legal mappings)
etc.
All the other named and unnamed options should also be in this.
The architectural YAML is basically the ISA string at the beginning - that
by itself defines most of the configuratoin.
Are are supposed to be rules that define the legal combinations of those
options (e.g. can't have D without M, etc.
In theory, any TG that defines an extension is responsible for updating
those rules, but I don't think we have enforced that.
…On Tue, Nov 12, 2024 at 7:36 AM james-ball-qualcomm < ***@***.***> wrote:
I'm trying to find examples that use the riscv-config schema. I've found
https://github.com/riscv-software-src/riscv-config/blob/dev/examples/rv32i_isa.yaml
but it doesn't seem to have information like the description or address for
CSRs. Also, is there a YAML file somewhere for each CSR and then a
configuration YAML that describes how a particular CSR is configured for a
particular implementation? This is what UDB does but does riscv-config
follow this approach too? So far it just looks like there is one YAML file
that describes the CSR and how it exists in a particular implementation.
—
Reply to this email directly, view it on GitHub
<#185 (reply in thread)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AHPXVJR6NTOWPFF25W6AEM32AIOANAVCNFSM6AAAAABQRZGQUKVHI2DSMVQWIX3LMV43URDJONRXK43TNFXW4Q3PNVWWK3TUHMYTCMRSHAYTQOI>
.
You are receiving this because you were mentioned.Message ID:
<riscv-software-src/riscv-unified-db/repo-discussions/185/comments/11228189
@github.com>
|
Beta Was this translation helpful? Give feedback.
-
|
@dhower-qc, Is there some fundamental reason why a parameter must always be associated with an extension in UDB? I'm imagining adding CSR parameters to UDB in addition to the existing extension parameters. There could be a common base class for both types. It would mean a significant enhancement to UDB to support CSR parameters but it might be a good investment. |
Beta Was this translation helpful? Give feedback.
-
|
I've created slides to help organize my thoughts (and share them with others) for a solution to this issue. |
Beta Was this translation helpful? Give feedback.
-
There are many many implementation-defined behaviors that affect which CSRs are present, which CSR fields are present, and the allowed behavior of the CSR fields. Currently UDB requires extensions to create parameters to define an implementation's CSR field behavior. Instead, I can imagine we follow the riscv-config approach and have a general schema to capture these CSR field implementation-dependent behaviors without create a new parameter for each one. If the condition is too complex to have a data-driven schema represent it, then we'd allow writing an execution-driven (probably IDL) solution to define it.
Here's some examples of existing parameters that could potentially be replaced with a more general schema.
trap is taken into M-mode with exception-specific information to assist software in handling the
trap (e.g., address associated with exception).
Must be greater than or equal to max(PHYS_ADDR_WIDTH, VA_SIZE
Beta Was this translation helpful? Give feedback.
All reactions