Skip to content
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

Fixed CSR Instruction Handlers and Use New Register Macros #65

Merged
merged 19 commits into from
Feb 12, 2025

Conversation

kathlenemagnus
Copy link
Collaborator

@kathlenemagnus kathlenemagnus commented Feb 7, 2025

Set of fixes needed to get the rv64mi-p-csr arch test to pass.

  • Use new register macros in all instruction handlers
  • Set some CSR read-only fields at boot based on the configured XLEN
  • Added a way to override CSR register values through a JSON file
  • Made several fixes to the CSR instruction handlers
  • Improved Instruction Logger to better support exceptions and to print CSR values for CSR instructions

// TODO cnyce
(void)state;
(void)xlen_val;
return 42949672960;
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@colby-nyce FYI the value here (0xa00000000) is setting the CSR fields that set the XLEN for user mode (uxl) and supervisor mode (sxl). The same value is being written to MSTATUS and SSTATUS in AtlasState::boot.

@@ -152,6 +153,32 @@ namespace atlas
}
}

void AtlasState::onBindTreeLate_()
{
// Write initial values to CSR registers
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I'm not committed to this method of providing CSR values. Open to suggestions. I've used complex, Sparta parameters in the past, but I've found that they can be hard to read in the yaml file sometimes.

Copy link
Contributor

Choose a reason for hiding this comment

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

Could always create an Action group that are an initialization methods. The first call of run would run theses methods then they "move themselves out of the way" returning to normal operation.

@@ -0,0 +1,2 @@
{
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Format would be like this:

{
    "mstatus": "0xa00000000",
    "sstatus": "0x200000000"
}

core/AtlasInst.hpp Outdated Show resolved Hide resolved
core/AtlasInst.hpp Outdated Show resolved Hide resolved
@@ -152,6 +153,32 @@ namespace atlas
}
}

void AtlasState::onBindTreeLate_()
{
// Write initial values to CSR registers
Copy link
Contributor

Choose a reason for hiding this comment

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

Could always create an Action group that are an initialization methods. The first call of run would run theses methods then they "move themselves out of the way" returning to normal operation.

core/AtlasState.cpp Outdated Show resolved Hide resolved
core/AtlasInst.hpp Outdated Show resolved Hide resolved
core/AtlasInst.hpp Outdated Show resolved Hide resolved
core/AtlasState.cpp Outdated Show resolved Hide resolved
}

const auto rd_val = sext(old, state->getXlen());
insn->getRd()->write(rd_val); // dmiWrite?
WRITE_INT_REG(rd, csr_val);
Copy link
Contributor

Choose a reason for hiding this comment

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

I am wondering how rd==x0 is taken care of. I believe if rd==x0, no value is written. Is that checked in WRITE_INT_REG?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes, it is taken care of in the macro. If reg_ident == 0, then the write does not occur.

@kathlenemagnus kathlenemagnus merged commit 2c5d2a9 into sparcians:main Feb 12, 2025
5 checks passed
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.

6 participants