Skip to content

Cv32e40p sail 0.11#1385

Open
karabambus wants to merge 14 commits intoriscv:act4from
karabambus:cv32e40p-sail-0.11
Open

Cv32e40p sail 0.11#1385
karabambus wants to merge 14 commits intoriscv:act4from
karabambus:cv32e40p-sail-0.11

Conversation

@karabambus
Copy link
Copy Markdown
Contributor

@karabambus karabambus commented Apr 27, 2026

Verified by running on DUT.

@karabambus karabambus marked this pull request as ready for review April 30, 2026 14:26
@karabambus
Copy link
Copy Markdown
Contributor Author

Added comments for parameters that are schema-required but are not relevant to our core. I see there are multiple issues open across ACT4 and Sail repo. If you find it useful and not redundant I can open issue where I can track this parameters.

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Updates the CV32E40P v2 Sail configuration to align with Sail 0.11 schema/behavior and to better reflect the DUT’s implemented privilege/memory/CSR behavior during reference-model comparison.

Changes:

  • Expand/update Sail config fields (misaligned access policy, memory regions, reservation, max_time_to_wait, additional schema-required knobs).
  • Refine reserved-behavior and CSR capability annotations for M-mode-only CV32E40P (no PMP/MMU/A/V, mtval hardwired to 0, etc.).
  • Add RVMODEL_MTVEC_ALIGN to match the core’s mtvec base alignment requirement.

Reviewed changes

Copilot reviewed 4 out of 5 changed files in this pull request and generated 3 comments.

File Description
config/cores/cve4/cv32e40p-v2-rv32imcf/sail.json Sail 0.11 config/schema updates + richer memory map and exception/reserved-behavior settings for the F-enabled variant
config/cores/cve4/cv32e40p-v2-rv32imc/sail.json Same Sail 0.11 config/schema updates for the non-F variant
config/cores/cve4/cv32e40p-v2-rv32imc/rvmodel_macros.h Define mtvec alignment (RVMODEL_MTVEC_ALIGN) for trap handler placement correctness

Comment thread config/cores/cve4/cv32e40p-v2-rv32imcf/sail.json Outdated
Comment thread config/cores/cve4/cv32e40p-v2-rv32imcf/sail.json Outdated
Comment thread config/cores/cve4/cv32e40p-v2-rv32imc/sail.json Outdated
karabambus and others added 2 commits May 3, 2026 08:59
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Signed-off-by: Marin Radic <mradic07@gmail.com>
Copy link
Copy Markdown
Collaborator

@jordancarlin jordancarlin left a comment

Choose a reason for hiding this comment

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

LGTM, but I would like a review from @MikeOpenHWGroup before merging since he is more familiar with these cores.

@jordancarlin
Copy link
Copy Markdown
Collaborator

Added comments for parameters that are schema-required but are not relevant to our core. I see there are multiple issues open across ACT4 and Sail repo. If you find it useful and not redundant I can open issue where I can track this parameters.

This is a limitation of the Sail JSON schema (it requires all keys to be defined), and I don't think this is something that will be changing any time soon, so probably no need for an issue.

Comment thread config/cores/cve4/cv32e40p-v2-rv32imc/sail.json Outdated
Comment thread config/cores/cve4/cv32e40p-v2-rv32imc/sail.json Outdated
Comment thread config/cores/cve4/cv32e40p-v2-rv32imc/sail.json Outdated
Comment thread config/cores/cve4/cv32e40p-v2-rv32imc/sail.json
@MikeOpenHWGroup
Copy link
Copy Markdown
Contributor

For the most part, the technical content LGTM as well. I have made a couple of minor comments on specific files, please have a look.

I also have a few comments about style:

  1. There are multiple comment lines in the core json file that follow this pattern:
// SCHEMA-REQUIRED: cv32e40p has no <feature xxx>
// Value has no effect on DUT comparison.

It is not clear exactly what SCHEMA-REQUIRED means. I suspect it means, "this field in the json file is required by the ACT4 schema". Since this term is not mentioned in the ACT4 documentation it would be a good idea to add a comment field at the top of the json file explaining what SCHEMA-REQUIRED means and why it is needed.

  1. Unless it is absolutely unavoidable, it is dangerous to explicitly reference RTL source code. Instead, a reference to the User Manual is more appropriate. For example, instead of:
// mtval is hardwired to 0 in cv32e40p — cv32e40p_cs_registers.sv:346,516.

it would be more appropriate to point the reader to the User Manual:

// mtval is hardwired to 0 in cv32e40p, see the CSR Descriptions chapter in the User Manual.

@karabambus karabambus requested a review from MikeOpenHWGroup May 6, 2026 21:31
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.

4 participants