[crypto] ML-DSA-87 verify (1/24) by andrea-caforio · Pull Request #29299 · lowRISC/opentitan

andrea-caforio · 2026-02-11T16:20:08Z

First PR of the series adding package documentation and Bazel build files.

This is a series of PRs that in their composition result in FIPS-204-compliant OTBN implementation of ML-DSA-87 verify.

Resources

Preamble

doc

Number-theoretic transform

NTT
INTT

Polynomial arithmetic

poly_add, poly_sub, poly_mul
poly_mul_add

XOF

xof_init, xof_poll, xof_finish
xof_absorb
xof_squeeze

Rounding

shift_left
decompose

Reduction

reduce

Infinity norm

norm_check

Sampling

rej_ntt_poly, expand_a
sample_in-ball
challenge_hash

Encoding

decode_z
decode_t1
decode_hint
encode_w1

Vector operations

sig_decode
norm_check_z
A*z, c * t1, Az - ct1
use_hint

Epilogue

app

sw/otbn/crypto/mldsa87/mldsa87.s

etterli

Looks good to me except that the memory increase should probably be an atomic commit which also touches the RTL and DV. The reason is that otherwise the DV and potentially IBEX SW breaks.

I think increasing the memories should touch the same files as in this commit: etterli/opentitan-otbn-pqc-isa@b0ce32d

etterli · 2026-02-11T17:09:56Z

hw/ip/otbn/data/otbn.hjson

          Note that DMEM is actually 4kiB in size, but only the first 3kiB of
          the memory is visible through this register interface.


Suggested change

TODO: Document the .bss and .scratchpad split

Note that DMEM is actually 32kiB in size, but only the first XkiB of

the memory is visible through this register interface.

andrea-caforio · 2026-02-12T07:29:14Z

Looks good to me except that the memory increase should probably be an atomic commit which also touches the RTL and DV. The reason is that otherwise the DV and potentially IBEX SW breaks.

I think increasing the memories should touch the same files as in this commit: etterli/opentitan-otbn-pqc-isa@b0ce32d

Correct.

nasahlpa

Thanks Andi, this looks good to me.

Could you please:

Increase the memory sizes in the documentation as well
Fix the linting issue in otbn_reg_pkg.sv

nasahlpa · 2026-02-12T07:45:21Z

hw/ip/otbn/doc/registers.md

-| otbn.[`IMEM`](#imem)                           | 0x4000   |     8192 | Instruction Memory Access                       |
-| otbn.[`DMEM`](#dmem)                           | 0x8000   |     3072 | Data Memory Access                              |
+| otbn.[`IMEM`](#imem)                           | 0x8000   |    16384 | Instruction Memory Access                       |
+| otbn.[`DMEM`](#dmem)                           | 0x10000  |    32768 | Data Memory Access                              |


Can you please also update the memory sizes here:
https://opentitan.org/book/hw/ip/otbn/doc/theory_of_operation.html#memories

This is actually autogenerated. I am working on a proper PR which increases the memories. It takes more than just adding the changes I previously mentioned.

Please note that if you move the start address of DMEM (or IMEM), you introduce a backwards incompatible change with respect to Ibex software. If we do this, we have to not just do a minor version increase to 1.2.0 but a major one 2.0.0. IIRC, we should be able to increase DMEM to 32 KiB and IMEM to 16 KiB without changing start addresses.

Yes, we can increase both memories to 32KiB without changing the offsets. This is implemented in #29318 .

vogelpi

Thanks @andrea-caforio for the PR, it's great to see this! I've left some comments

vogelpi · 2026-02-14T14:08:41Z

sw/otbn/crypto/mldsa87/mldsa87.s

@@ -0,0 +1,94 @@
+/* Copyright lowRISC contributors (OpenTitan project). */


Should this go into a README.md instead?

It's a README.md now.

vogelpi · 2026-02-14T14:12:22Z

hw/ip/otbn/doc/registers.md

-| otbn.[`IMEM`](#imem)                           | 0x4000   |     8192 | Instruction Memory Access                       |
-| otbn.[`DMEM`](#dmem)                           | 0x8000   |     3072 | Data Memory Access                              |
+| otbn.[`IMEM`](#imem)                           | 0x8000   |    16384 | Instruction Memory Access                       |
+| otbn.[`DMEM`](#dmem)                           | 0x10000  |    32768 | Data Memory Access                              |


Please note that if you move the start address of DMEM (or IMEM), you introduce a backwards incompatible change with respect to Ibex software. If we do this, we have to not just do a minor version increase to 1.2.0 but a major one 2.0.0. IIRC, we should be able to increase DMEM to 32 KiB and IMEM to 16 KiB without changing start addresses.

vogelpi · 2026-02-14T14:13:42Z

hw/ip/otbn/data/otbn.hjson

+          TODO: Document the .bss and .scratchpad split
+          Note that DMEM is actually 32kiB in size, but only the first XkiB of


Ideally we can make a sensible choice for this right from the beginning. It may require some internal discussion first though.

#29318 goes with a 16\16 KiB split. This should be sufficient to transfer any PQC related data.

I removed the commit that adjusts the DMEM/IMEM sizes.
It is handled in #29318.

etterli · 2026-02-16T08:04:22Z

@vogelpi Thanks for explaining the address offset and version problematic. I created a separate PR which increases the IMEM and DMEM. See #29318.

@andrea-caforio

h-filali

Thanks @andrea-caforio. I mainly double checked the documentation part. Looks very clean, I like it!

sw/otbn/crypto/mldsa87/mldsa87.s

h-filali · 2026-02-17T16:17:57Z

sw/otbn/crypto/mldsa87/mldsa87.s

+
+The implementation is structured hierarchically where the bottom layer consists
+of routines that operate on polynomials in Z_q[X] / (X^256 + 1) composed of
+256 24-bit coefficients in the ring Z_q each occupying one 32-bit memory word


A bit nitpicky but is "occupying one 32-bit memory word" always true?

Yes, always.

h-filali · 2026-02-17T16:23:13Z

sw/otbn/crypto/mldsa87/mldsa87.s

+
+/*
+
+This directory contains the FIPS-204-compliant and hardened OpenTitan OTBN


Only SCA hardened AFAIK.

I'm projecting here. :-)

This file documents the high-level implementation choices and should be the initial contact point when navigating to the `mldsa87` directory. Signed-off-by: Andrea Caforio <andrea.caforio@lowrisc.org>

Two Bazel build files are required for the ML-DSA-87 apps: One for the sources files and another one for the unit tests. Signed-off-by: Andrea Caforio <andrea.caforio@lowrisc.org>

nasahlpa

Thanks for addressing the comments. The IMEM and DMEM size increase will be done in #29318.

andrea-caforio self-assigned this Feb 11, 2026

andrea-caforio added the SW:cryptolib Crypto library label Feb 11, 2026

andrea-caforio requested review from andreaskurth, etterli, h-filali, johannheyszl, nasahlpa, siemen11 and vogelpi February 11, 2026 16:20

nasahlpa reviewed Feb 11, 2026

View reviewed changes

sw/otbn/crypto/mldsa87/mldsa87.s Outdated Show resolved Hide resolved

etterli reviewed Feb 11, 2026

View reviewed changes

andrea-caforio force-pushed the mldsa87-verify-1 branch from 0d2d6bb to 85f561a Compare February 12, 2026 07:27

andrea-caforio requested a review from a team as a code owner February 12, 2026 07:27

andrea-caforio requested review from hcallahan-lowrisc and removed request for a team February 12, 2026 07:27

andrea-caforio force-pushed the mldsa87-verify-1 branch from 85f561a to c50f149 Compare February 12, 2026 07:38

nasahlpa reviewed Feb 12, 2026

View reviewed changes

vogelpi reviewed Feb 14, 2026

View reviewed changes

h-filali approved these changes Feb 17, 2026

View reviewed changes

andrea-caforio added 2 commits February 18, 2026 08:46

[crypto] ML-DSA-87 package documentation

5f01766

This file documents the high-level implementation choices and should be the initial contact point when navigating to the `mldsa87` directory. Signed-off-by: Andrea Caforio <andrea.caforio@lowrisc.org>

[crypto] ML-DSA-87 Bazel files

f37a11e

Two Bazel build files are required for the ML-DSA-87 apps: One for the sources files and another one for the unit tests. Signed-off-by: Andrea Caforio <andrea.caforio@lowrisc.org>

andrea-caforio force-pushed the mldsa87-verify-1 branch from c50f149 to f37a11e Compare February 18, 2026 07:47

etterli approved these changes Feb 18, 2026

View reviewed changes

nasahlpa approved these changes Feb 18, 2026

View reviewed changes

nasahlpa added this pull request to the merge queue Feb 18, 2026

Merged via the queue into lowRISC:master with commit 59a9333 Feb 18, 2026
46 checks passed

andrea-caforio mentioned this pull request Feb 18, 2026

[crypto] ML-DSA-87: forward number-theoretic transform (2/24) #29333

Merged

This was referenced Feb 18, 2026

[crypto] ML-DSA-87: backward number-theoretic transform (3/24) #29334

Merged

[crypto] ML-DSA-87: Polynomial arithmetic (mul, add, sub) (4/24) #29335

Merged

[crypto] ML-DSA-87: Polynomial arithmetic (mul-add) (5/24) #29345

Open

		Note that DMEM is actually 4kiB in size, but only the first 3kiB of
		the memory is visible through this register interface.

		@@ -0,0 +1,94 @@
		/* Copyright lowRISC contributors (OpenTitan project). */

		TODO: Document the .bss and .scratchpad split
		Note that DMEM is actually 32kiB in size, but only the first XkiB of


		/*

		This directory contains the FIPS-204-compliant and hardened OpenTitan OTBN

Comments

Conversation

andrea-caforio commented Feb 11, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Resources

Preamble

Number-theoretic transform

Polynomial arithmetic

XOF

Rounding

Reduction

Infinity norm

Sampling

Encoding

Vector operations

Epilogue

Uh oh!

Uh oh!

etterli left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

andrea-caforio commented Feb 12, 2026

Uh oh!

nasahlpa 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!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

vogelpi 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!

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!

etterli commented Feb 16, 2026

Uh oh!

h-filali left a comment

Choose a reason for hiding this comment

Uh oh!

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!

nasahlpa left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

andrea-caforio commented Feb 11, 2026 •

edited

Loading