Skip to content

Master candidate dcls - #3421

Open
valentinThomazic wants to merge 1 commit into
openhwgroup:master_candidatefrom
ThalesSiliconSecurity:master_candidate_dcls
Open

Master candidate dcls#3421
valentinThomazic wants to merge 1 commit into
openhwgroup:master_candidatefrom
ThalesSiliconSecurity:master_candidate_dcls

Conversation

@valentinThomazic

@valentinThomazic valentinThomazic commented Jul 30, 2026

Copy link
Copy Markdown
Contributor

Proposed base of work for common DCLS implementation.

Elaboration top becomes cva6_top which instantiate either a single CVA6 or a CVA6 DCLS with its logic control.

The `dcls_logic` module shifts the main core inputs by a configurable delay to drive the shadow core. it also performs the core outputs comparison.

The `dcls_common` instantiates common modules depending on whether they are configured to be shared or not. Both cores either instantiate the module or send their control signals and receive data from the dcls common modules at the top.

It compares the received control inputs from both cores and outputs and send the result to the cores, shifted by n cycles for the shadow core.

The comparators output synchronous alarms.

Current options are:

  • Configurable delay between the 2 cores
  • Common Regfile (without integrity or error correction)
  • Common BHT

Target cv32a60x is set with:

  • 2 Cycle of delay
  • Common Regfile & Common BHT

Others configurations are left unchanged and function as before

@cainria

cainria commented Jul 30, 2026

Copy link
Copy Markdown
Member

⚠️ Verilator was not run in this PR ⚠️

It was a default-branch workflow, but due to new GitHub security rules, we switched to a per-branch workflow #3415

We may want to cherry-pick #3415 in master_candidate before or after merging this PR, so that Verible checks the code in that branch too! (#3403 might be required first to avoid git conflicts)
EDIT: #3436 is needed too to freeze Verible version since a recent release changed the expected formatting.

@cainria cainria left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Nice contribution! I like the way you handled the common register file.

Comment thread core/dcls_delay_ff.sv

@cainria cainria Jul 30, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This looks like a duplicate of shift_reg from pulp-platform, which is used in CVA6 load unit.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

You are right, I will use it instead.
Using this module also gives the option of having no delay between the cores.

Comment thread core/dcls_logic.sv
Comment on lines +62 to +79
dcls_comparator #(
.data_t(core_outputs_t)
) i_dcls_comparator (
.clk_i,
.rst_ni (rst_shadow_no),
.main_i (main_outputs_delayed),
.shadow_i(shadow_outputs_i),
.alarm_o (alarm_o[0])
);
dcls_comparator #(
.data_t(core_outputs_t)
) i_dcls_comparator_dup (
.clk_i,
.rst_ni (rst_shadow_no),
.main_i (main_outputs_delayed),
.shadow_i(shadow_outputs_i),
.alarm_o (alarm_o[1])
);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Could we use a for loop here? Or would it prevent us from telling the synthesis tool to not merge these duplicates instances?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I tested it and it still synthesizes both comparators, at least with our synthesis flow.
I will use a generate loop then

@zarubaf
zarubaf removed their request for review August 13, 2026 05:19
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.

2 participants