[crypto/mlkem] Add main subroutines for ml-kem1024#30719
Conversation
4b0ff34 to
af2819f
Compare
andrea-caforio
left a comment
andrea-caforio
left a comment
There was a problem hiding this comment.
Thanks @siemen11. ML-KEM is on the map now. :-) I did an initial review, haven't checked some of the logic-heavy parts yet.
|
|
||
| /* Basecase even products c0 = a0*b0 + a1*b1*gamma */ | ||
| bn.mulvm.8S w3, w0, w1 | ||
| bn.mulvm.8S w4, w3, w2 |
There was a problem hiding this comment.
Does this work without a dummy addition in between? Our Montgomery multiplier does not do the conditional subtraction, so 2 subsequent multiplications might be incorrect.
There was a problem hiding this comment.
The difference is that ML-KEM works on 12 bit values, not like ML-DSA with 24 bits, so we can leave the overflow unhandled for a bit
There was a problem hiding this comment.
I see. The following addition is not a problem either I guess? Just to verify for myself because the operands for the addition are together larger than 2q but the addition is only well-defined for < 2q.
There was a problem hiding this comment.
The way I understand it is that the outputs from bn.mulvm are strictly bounded by q, so the sum of any two multiplication outputs is at most 2q
There was a problem hiding this comment.
The output of the Montgomery mutiplier used to be in [0, q[ but that was removed in later versions of the SIMD extension.
There was a problem hiding this comment.
The way that I understand it is that, we have a, b smaller than q and q = 3329, a 12 bit prime.
When we call bn.mulvm.8S, we calculate
r = (ab + mq)/2^32 <= floor((a*b - q)/2^32) + q (because m <= w^32 - 1, the Mont constant)
But because a, b < q, that floor is actually equal to 0, so r stays still smaller than q
Specifically, this is also mentioned in the docu:
"Note that when chaining multiplications, the conditional subtraction can be postponed until after the last multiplication in case the initial inputs are in [0, 2q[ and q < (2^d)/4 holds."
We have d = 32, and we have (2^d) / 4 = (2^32) / 4 = 2^30 and q = 2^12
| .data | ||
| .balign 32 | ||
|
|
||
| _expand_buf: |
There was a problem hiding this comment.
Is this really needed? Why not expand directly in the output buffer?
80cbc75 to
faa95fb
Compare
c1e3212 to
2f58500
Compare
andrea-caforio
left a comment
andrea-caforio
left a comment
There was a problem hiding this comment.
Nice @siemen11. This looks clean.
| bn.rshi w3, w2, w3 >> 11 | ||
| bn.rshi w2, w1, w2 >> 11 | ||
| bn.rshi w1, w31, w1 >> 11 | ||
| nop |
There was a problem hiding this comment.
| nop |
|
|
||
| jal x1, decode_5 | ||
|
|
||
| lw x7, -20(x31) |
There was a problem hiding this comment.
Might be cleaner to pass the address as parameter to the routine.
| li x4, 1 | ||
| bn.lid x4, 0(x2) | ||
|
|
||
| /* Prepare constant vector w2 = [1665, 1665, ..., 1665]. */ |
There was a problem hiding this comment.
This constant can also be stored in DMEM.
|
|
||
| /* Division by 3329 via shift-subtract */ | ||
| addi x9, x0, 0 | ||
| .irp bit, 11, 10, 9, 8, 7, 6, 5, 4, 3, 2, 1, 0 |
There was a problem hiding this comment.
For later, if IMEM becomes an issue convert this to a loop. This section alone expands to 384B of instructions.
8edcd36 to
18cb044
Compare
18cb044 to
222df8a
Compare
Add the main arithmetic and subroutines for the ml-kem1024 in the otbn. With thanks to Andrea Caforio <andrea.caforio@lowrisc.org> Signed-off-by: Siemen Dhooghe <sdhooghe@google.com>
2 participants
Add the main arithmetic and subroutines for the ml-kem1024 in the otbn.