Skip to content

Add CSR YAML files for Smcntrpmf: mcyclecfg and minstretcfg #595

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 10 commits into
base: main
Choose a base branch
from

Conversation

syedowaisalishah
Copy link
Contributor

@syedowaisalishah syedowaisalishah commented Apr 5, 2025

Implement CSR YAMLs :

@syedowaisalishah
Copy link
Contributor Author

@ThinkOpenly Thanks for the feedback! I’ve made the following updates:

  • Updated descriptions to match the ratified spec for mcyclecfg and minstretcfg.
  • Combined location_rv32 and location_rv64 into a single line.
  • Adjusted the inhibit bit descriptions to align with the spec wording.
  • Added base: 64 to both YAML files.
  • Ensured bit 63 is read-only zero
    Let me know if anything else needs attention!

@syedowaisalishah
Copy link
Contributor Author

@ThinkOpenly I'll update the PR by adding mcyclecfgh and minstretcfgh, using a table format for privilege modes, and ensuring consistency in the comments and formatting across the YAML files.Thanks for the suggestion, and sorry for the oversight!

Copy link
Collaborator

@ThinkOpenly ThinkOpenly left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will you be adding mcyclecfgh and minstretcfgh? These are the 32-bit (high) versions of these registers, so very similar definitions.

@syedowaisalishah
Copy link
Contributor Author

syedowaisalishah commented May 2, 2025

Thank you for the detailed feedback, and apologies for the earlier confusion. I've now made the following updates:

  • Added mcyclecfgh and minstretcfgh CSR YAML files as the 32-bit high halves for RV32.
  • Corrected mcyclecfg and minstretcfg:
    • Removed base: 64 at the register level.
    • Added base: 64 to individual fields at locations ≥ 32.
    • Converted field type attributes to type() functions based on privilege mode presence.
    • Updated reset_value fields to UNDEFINED_LEGAL.
    • Removed the redundant WPRI field definitions and related text.

Please let me know if anything still needs adjustment!

Copy link
Collaborator

@dhower-qc dhower-qc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Getting close.

The *INH fields should be definedBy the extension they control so that they appear to not be implemented if that extension isn't present.

For example

definedBy: Smcntrpmf
fields:
  SINH:
    definedBy: S
  VSINH:
    definedBy: H
  # ...

@syedowaisalishah syedowaisalishah requested a review from dhower-qc May 6, 2025 15:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add Smcntrpmf registers
3 participants