caliptra_ss_fuse_ctrl_integrity_check fix by antmarzam · Pull Request #727 · chipsalliance/caliptra-ss

antmarzam · 2025-09-16T14:18:45Z

Expected mask in wait_dai_op_idle now masks all partition errors.

The error mask within wait_dai_op_idle now masks only partition errors and nothing else.

moidx · 2025-09-16T16:15:42Z

...on/test_suites/caliptra_ss_fuse_ctrl_integrity_check/caliptra_ss_fuse_ctrl_integrity_check.c


-    wait_dai_op_idle(0x3FFFF);
+    // Masking all partition errors - since they should all be sert after forcing a different digest
+    wait_dai_op_idle(0x7FFFFF);


nit: Is there an autogenerated constant that can be used to dynamically calculate this mask?

Integrators will have the ability to re-generate the memory map. We will ask them to re-run the caliptra-ss test suite to make sure there are no issues. This is the reason we should aim to make the test cases resilient to memory map changes.

Makes sense - thanks.
We've already added the autogenerated constant to fuse_ctrl.c, so the same should be applied here.

@antmarzam , I think you've now replaced the literal here with the constant right? If yes, this should be flagged to re-trigger a review. Can you please clarify?

Yes - hardcoded values have been swapped with autogen constant.
I just re-trigger a review!

Thanks @antmarzam , also for triggering the pipeline as requested by Emre.

martin-velay

LGTM thanks Antonio for fixing the issues

ekarabu · 2025-09-18T15:15:12Z

src/integration/test_suites/caliptra_ss_fuse_ctrl_init_fail/caliptra_ss_fuse_ctrl_init_fail.c

        CMD_FC_LCC_UNCORRECTABLE_FAULT
    };

+    const uint32_t partition_lower_addr_bound = 0x40;


assign UDS base addr

@ekarabu , would you mind pointing us to where these auto-generated base addresses live in C-land please? The team knows where to find the values in the RTL sources but not on the C side. Can it be that these header files are auto-generated at compile time and then only live in the build folder? I've seen this in other projects as well. I I understand why it may be done this way but it makes it a bit harder to find out the names of the constants in C land.

please check *mmap.h file

Thanks @ekarabu I believe I'm using now the correct autogen constant - please let me know if you're happy with it

Thanks @ekarabu for the pointer, it's appreciated.

ekarabu · 2025-09-18T15:15:32Z

src/integration/test_suites/caliptra_ss_fuse_ctrl_init_fail/caliptra_ss_fuse_ctrl_init_fail.c

    };

+    const uint32_t partition_lower_addr_bound = 0x40;
+    const uint32_t partition_upper_addr_bound = 0xF0;


Assign FE3's Zeroization base address

I've replaced the harcoded constant for the autogen value:

caliptra-ss/src/integration/test_suites/caliptra_ss_fuse_ctrl_init_fail/caliptra_ss_fuse_ctrl_init_fail.c

Lines 45 to 46 in 478e8f6

const uint32_t partition_lower_addr_bound = partitions[SECRET_MANUF_PARTITION].address;

const uint32_t partition_upper_addr_bound = partitions[SECRET_PROD_PARTITION_3].zer_address;

ekarabu · 2025-09-18T19:25:27Z

Please ensure your branch is up-to-date and run a promote-pipeline to qualify your PR to be merged!

antmarzam · 2025-09-19T08:18:32Z

Please ensure your branch is up-to-date and run a promote-pipeline to qualify your PR to be merged!

Thanks Emre - I rebased with the latest main and just started a promote pipeline

rswarbrick · 2025-09-19T09:34:37Z

src/integration/test_suites/caliptra_ss_fuse_ctrl_init_fail/caliptra_ss_fuse_ctrl_init_fail.c

    reset_fc_lcc_rtl();
    wait_dai_op_idle(
-        fault == CMD_FC_LCC_CORRECTABLE_FAULT ? 1 << partition.index : 0x3FFFF
+                     fault == CMD_FC_LCC_CORRECTABLE_FAULT ? 1 << partition.index : OTP_CTRL_STATUS_LIFE_CYCLE_ERROR_MASK - 1 //0x3FFFFFF


@antmarzam: I think you probably want to drop the commented constant from this line? I'd also either get rid of the newline after the ( or put the indentation back to what it was.

Good spot - thanks

Fix to the address bounds in the partition and to the expected error sent to `wait_dai_op_idle()` This came from the head of PR chipsalliance#727.

rswarbrick · 2025-09-21T15:02:04Z

Hi @antmarzam! As well as the tidy-up I'm suggesting above, do you think you could pull the commits of mine from #740? These all seem related, and maybe this would be a way to land the change atomically.

rswarbrick · 2025-09-23T12:39:41Z

Boo. Looks like this just failed CI. The results that caused it to fail:

  -smoke_test_fc_key_revocation.yml ...degraded
  -smoke_test_fc_ocp_lock_zeroization.yml ...degraded
  -smoke_test_fc_pk_volatile_lock.yml ...degraded
  -smoke_test_fc_prov_lcc_tokens.yml ...degraded
  -smoke_test_fc_registers.yml ...degraded
  -smoke_test_lcc_registers.yml ...degraded
  -smoke_test_lcc_scrap.yml ...degraded
  -smoke_test_mbox0_csr_zeros_after_release.yml ...degraded
  -smoke_test_mci_axi_miss.yml ...degraded
  -smoke_test_mci_brkpoint_axi.yml ...degraded
  -smoke_test_mci_soc_config_always_enable.yml ...degraded
  -smoke_test_mci_soc_config_diff_mcu.yml ...degraded
  -smoke_test_mci_soc_config_disable.yml ...degraded
  -smoke_test_mcu_mbox0.yml ...degraded
  -smoke_test_mcu_mbox0_csrs_warm_rst.yml ...degraded
  -smoke_test_mcu_mbox0_ecc.yml ...degraded
  -smoke_test_mcu_mbox0_invalid_axi.yml ...degraded
  -smoke_test_mcu_mbox0_strb_wr_csr.yml ...degraded
  -smoke_test_mcu_mbox0_write_user_lock.yml ...degraded
  -smoke_test_mcu_mbox0_zeroize.yml ...degraded
  -smoke_test_mcu_no_rom_config.yml ...degraded
  -smoke_test_mcu_no_rom_config_brkpoint.yml ...degraded
  -smoke_test_mcu_sram_byte_write.yml ...degraded
  -smoke_test_mcu_sram_ecc_db.yml ...degraded
  -smoke_test_mcu_sram_ecc_sb.yml ...degraded
  -smoke_test_mcu_sram_execution_region.yml ...degraded
  -smoke_test_mcu_sram_execution_region_max_size.yml ...degraded
  -smoke_test_mcu_sram_execution_region_random_sram_config_axi_user.yml ...degraded
  -smoke_test_mcu_sram_protected_region.yml ...degraded
  -smoke_test_mcu_trace_buffer.yml ...degraded
  -smoke_test_mcu_trace_buffer_63.yml ...degraded
  -smoke_test_mcu_trace_buffer_64.yml ...degraded
  -smoke_test_mcu_trace_buffer_no_debug.yml ...degraded
  -smoke_test_mcu_trace_buffer_random.yml ...degraded
  -smoke_test_mcu_trace_buffer_single.yml ...degraded
  -smoke_test_wdt.yml ...degraded

I'll take a look at this now.

rswarbrick · 2025-09-23T14:13:41Z

Well that's embarrassing: it's a build failure. Tidying it up now.

andreaskurth · 2025-09-23T17:11:21Z

@rswarbrick, could you please add the test added in PR #685 to the L1 regression set, in this PR?

rswarbrick · 2025-09-23T18:45:13Z

@andreaskurth: Sadly not because I still can't push to cailptra-ss. It's extremely frustrating. What's worse, I'm deep in a maze of irritating linker problems (because fuse_ctrl_mmap.h defines symbols, which means you can't currently include it in more than one compilation object).

@ekarabu: I suggest you grab #685 (that Andreas pointed to) and I will have this PR working by tomorrow. Grump, grump, grump!

rswarbrick · 2025-09-23T19:10:25Z

Ok, I've finally got everything working (this was rather hard!)

There's a working version of this PR at https://github.com/rswarbrick/caliptra-ss/tree/new-727. If someone would like to force-push that to this PR, that would be brilliant. If not, I hope that I'll be a member of chipsalliance at last by tomorrow.

This avoids fuse_ctrl_mmap.h defining something that actually gets instantiated (and defines a symbol). Without fixing that, if more than one compilation unit includes the header then link will fail because more than one object file defines "partitions" and various fuse index arrays.

Expected mask in `wait_dai_op_idle` now masks all partition errors. The error mask within `wait_dai_op_idle` now masks only partition errors and nothing else.

Fix to the address bounds in the partition and to the expected error sent to `wait_dai_op_idle()`

No functional change (but hopefully it makes things a bit clearer: I found these ranges a bit hard to reason about).

The value in caliptra_ss_fuse_ctrl_manuf_prod_prov hadn't yet been updated for the change in 275521b. This change should make it so that test and also caliptra_ss_fuse_ctrl_init_fail get a magic number from a single (documented) place. Apply the same change to caliptra_ss_fuse_ctrl_test_unlocked0_prov.

rswarbrick · 2025-09-24T13:26:57Z

I've been looking at this more carefully and I'm rather unconvinced by the two "fix" commits in this series. I will open an orthogonal PR with the other changes (which I think are easier to be certain about).

rswarbrick · 2025-09-30T18:19:03Z

I think the changes we need have been implemented in other PRs, so I'm going to close this one now.

antmarzam requested review from andrea-caforio, ekarabu and martin-velay September 16, 2025 14:18

moidx reviewed Sep 16, 2025

View reviewed changes

antmarzam force-pushed the lowrisc_tlt_regression_fixes branch from 6d049d8 to cbb8ca9 Compare September 16, 2025 16:29

martin-velay previously approved these changes Sep 17, 2025

View reviewed changes

antmarzam dismissed martin-velay’s stale review via 412dc6b September 17, 2025 11:04

antmarzam force-pushed the lowrisc_tlt_regression_fixes branch from b08a54c to 412dc6b Compare September 17, 2025 11:04

ekarabu reviewed Sep 18, 2025

View reviewed changes

antmarzam force-pushed the lowrisc_tlt_regression_fixes branch from 412dc6b to 478e8f6 Compare September 19, 2025 08:00

antmarzam requested review from ekarabu, martin-velay, moidx and vogelpi September 19, 2025 08:00

antmarzam force-pushed the lowrisc_tlt_regression_fixes branch from 478e8f6 to 0a8046f Compare September 19, 2025 08:15

rswarbrick reviewed Sep 19, 2025

View reviewed changes

This was referenced Sep 19, 2025

[fuse_ctrl,dv] Slightly tidy up some tests about the Caliptra "secret" addresses #740

Closed

[fuse_ctrl,regression] The caliptra_ss_fuse_ctrl_integrity_check fails in the nightly regression #741

Closed

antmarzam force-pushed the lowrisc_tlt_regression_fixes branch from 0a8046f to 2905fc5 Compare September 21, 2025 17:35

rswarbrick mentioned this pull request Sep 23, 2025

[Val] FC Test uses local register definitions #703

Closed

marnovandermaas force-pushed the lowrisc_tlt_regression_fixes branch from 40b168b to e3cf8f7 Compare September 23, 2025 11:05

rswarbrick force-pushed the lowrisc_tlt_regression_fixes branch from e3cf8f7 to 7438b43 Compare September 24, 2025 08:58

rswarbrick and others added 5 commits September 24, 2025 11:48

caliptra_ss_fuse_ctrl_integrity_check fix

553c53f

Expected mask in `wait_dai_op_idle` now masks all partition errors. The error mask within `wait_dai_op_idle` now masks only partition errors and nothing else.

caliptra_ss_fuse_ctrl_init_fail fix

9eb58b0

Fix to the address bounds in the partition and to the expected error sent to `wait_dai_op_idle()`

[fuse_ctrl] Add some documentation for address range type in package

302ea24

No functional change (but hopefully it makes things a bit clearer: I found these ranges a bit hard to reason about).

rswarbrick force-pushed the lowrisc_tlt_regression_fixes branch from 7438b43 to 42c81b8 Compare September 24, 2025 12:15

rswarbrick marked this pull request as draft September 24, 2025 13:27

rswarbrick mentioned this pull request Sep 24, 2025

[fuse_ctrl,dv] Assorted cleanups to how FC tests look at magic registers #750

Merged

rswarbrick closed this Sep 30, 2025

rswarbrick deleted the lowrisc_tlt_regression_fixes branch October 15, 2025 16:31

	const uint32_t partition_lower_addr_bound = partitions[SECRET_MANUF_PARTITION].address;
	const uint32_t partition_upper_addr_bound = partitions[SECRET_PROD_PARTITION_3].zer_address;

Conversation

antmarzam commented Sep 16, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

martin-velay left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ekarabu Sep 18, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ekarabu commented Sep 18, 2025

Uh oh!

antmarzam commented Sep 19, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rswarbrick commented Sep 21, 2025

Uh oh!

rswarbrick commented Sep 23, 2025

Uh oh!

rswarbrick commented Sep 23, 2025

Uh oh!

andreaskurth commented Sep 23, 2025

Uh oh!

rswarbrick commented Sep 23, 2025

Uh oh!

rswarbrick commented Sep 23, 2025

Uh oh!

rswarbrick commented Sep 24, 2025

Uh oh!

rswarbrick commented Sep 30, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants

ekarabu Sep 18, 2025 •

edited

Loading