Skip to content

[manuf] Move default perso extensions outside of extension - #31269

Merged
pamaury merged 1 commit into
lowRISC:earlgrey_1.0.0from
pamaury:fix_default_prov
Sep 16, 2026
Merged

pamaury merged 1 commit into
lowRISC:earlgrey_1.0.0from
pamaury:fix_default_prov

Conversation

@pamaury

@pamaury pamaury commented Sep 9, 2026 •

Copy link
Copy Markdown
Contributor

Those files are used in the provisioning inputs of the OT repository unconditionally. This means that when the provisioning extension module gets overriden by the manufacturer, references like @provisioning_exts//:default_ft_ext_lib will now point to manufacturer code instead of the "default"/example provisioning code. At best, this can produce a compile error and at worse produce a non-functional perso/rom_ext when building the emulation skus.

This commit moves those default files outside of the extension so that they can be referenced even when overriding the extension.

Context: without this, using the ot-sku repository as a provisioning extension breaks all emulation skus.

name = "default_perso_fw_ext",
srcs = ["default_personalize_ext.c"],
deps = [
"@lowrisc_opentitan//sw/device/lib/dif:flash_ctrl",

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.

Note: technically, all those @lowrisc_opentitan references are not needed since package is part of lowrisc_opentitan. I kept it since it was present and it at least provided a very easy file to copy-paste to extension, so might be worth keeping?

@pamaury
pamaury marked this pull request as ready for review September 9, 2026 13:37
@pamaury
pamaury requested review from a team and cfrantz as code owners September 9, 2026 13:37
@pamaury
pamaury requested a review from a team September 9, 2026 13:37
@pamaury
pamaury force-pushed the fix_default_prov branch 3 times, most recently from df88333 to e6013a5 Compare September 11, 2026 14:45
@cfrantz

cfrantz commented Sep 11, 2026

Copy link
Copy Markdown
Contributor

I think this change affects the ML-DSA work by @sasdf and @xorptr; I think change is a move in the right direction: we want the default ML-DSA perso extension to be library code that can be re-used by downstream SKU configs.

@xorptr

xorptr commented Sep 11, 2026

Copy link
Copy Markdown
Contributor

I think this change affects the ML-DSA work by @sasdf and @xorptr; I think change is a move in the right direction: we want the default ML-DSA perso extension to be library code that can be re-used by downstream SKU configs.

I agree. Currently, the downstream code has alias targets that point to build targets with source files that are copies of the upstream default extension code. For ML-DSA default extension alias in the downstream code, I added an alias target that points back to the upstream @lowrisc_opentitan default extension target.

Having this changes would allow removing these alias targets in the downstream code.

@pamaury

pamaury commented Sep 11, 2026

Copy link
Copy Markdown
Contributor Author

This PR only moves the files so it should be relatively harmless but if you have work in progress and want to avoid rebase conflicts, we can keep this open until all MLDSA changes are done.

@xorptr

xorptr commented Sep 11, 2026 •

Copy link
Copy Markdown
Contributor

This PR only moves the files so it should be relatively harmless but if you have work in progress and want to avoid rebase conflicts, we can keep this open until all MLDSA changes are done.

I would prefer to merge this after reviews and use it downstream

@AlexJones0 AlexJones0 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM after my comments are addressed. Thanks @pamaury.

Comment thread sw/host/provisioning/ft_ext_lib/BUILD Outdated
Comment thread sw/device/silicon_creator/manuf/default/BUILD.bazel Outdated
@pamaury
pamaury force-pushed the fix_default_prov branch 2 times, most recently from adcf6cf to 802b175 Compare September 14, 2026 13:43
@sasdf sasdf added the CI:Rerun Rerun failed CI jobs label Sep 15, 2026
@github-actions github-actions Bot removed the CI:Rerun Rerun failed CI jobs label Sep 15, 2026
Those files are used in the provisioning inputs of the OT
repository unconditionally. This means that when the provisioning
extension module gets overriden by the manufacturer, references like
@provisioning_exts//:default_ft_ext_lib will now point to manufacturer
code instead of the "default"/example provisioning code. At best, this
can produce a compile error and at worse produce a non-functional
perso/rom_ext when building the emulation skus.

This commit moves those default files outside of the extension so that
they can be referenced even when overriding the extension.

Signed-off-by: Amaury Pouly <amaury.pouly@opentitan.org>
@pamaury

pamaury commented Sep 15, 2026

Copy link
Copy Markdown
Contributor Author

I have rebased since there were some recent changes but I tested locally and it works.

@pamaury
pamaury requested a review from rswarbrick as a code owner September 16, 2026 10:03
@pamaury pamaury changed the title [manuf] Move default perso extensions outside of extension [DO NOT MERGE][manuf] Move default perso extensions outside of extension Sep 16, 2026
@pamaury

pamaury commented Sep 16, 2026

Copy link
Copy Markdown
Contributor Author

I have tried cherry-picking #30906 to see if it helps with the USB problems.

@pamaury pamaury changed the title [DO NOT MERGE][manuf] Move default perso extensions outside of extension [manuf] Move default perso extensions outside of extension Sep 16, 2026
@pamaury

pamaury commented Sep 16, 2026

Copy link
Copy Markdown
Contributor Author

CI failures are due to some CI problems with the CW310. All manufacturing tests passed on the CW340 so this should be safe to merge.

@pamaury
pamaury merged commit 327152d into lowRISC:earlgrey_1.0.0 Sep 16, 2026
147 of 201 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