[kmac] Specify and implement a dynamic app interface extension by etterli · Pull Request #30141 · lowRISC/opentitan

etterli · 2026-05-18T13:59:55Z

This PR extends the current KMAC application interface so it supports changing the hashing configuration at runtime. The new interface mode is called 'dynamic'. The extension keeps full compatibility with the existing interface behavior, hereinafter referred to as 'static'.

The new interface behavior is described / specified in the hw/ip/kmac/doc/theory_of_operation.md.
The diagrams in the documentation are best viewed by building the documentation locally using:
util/site/build-docs.sh serve
(or if using VS code install the wavedrom and a mermaid extensions).

Note, this PR should not yet be merged, as the changes also directly break the keymgr, keymgr_dpe, rom_ctrl, and lc_ctrl IPs. The reason is that the types app_req_t and app_rsp_t are modified. This PR would also break the DV of kmac and the related IPs.

I will push commits resolving these problems but for now I think it would make sense to start reviewing the specification and core implementation of the interface.

etterli · 2026-05-18T15:56:34Z

The CI tests fail mainly due to the mentioned breaking changes in other IPs and missing top level updates.

rswarbrick

This is a partial review of the specification commit.

etterli

@rswarbrick Thank you for the first review. Incorporated the feedback.

etterli · 2026-05-19T07:36:33Z

+### Application Interfaces
+
+The IP has N instances of an application interface.
+Each of these interface can be either static or dynamic.


Good idea to split the two types. I will rewrite this once I came up with a solution to the wiring problem.

vogelpi

Thanks for the PR @etterli ! I've reviewed the doc + the first half of the RTL. Will continue later.

vogelpi · 2026-05-21T11:34:27Z

+### Application Interfaces
+
+The IP has N instances of an application interface.
+Each of these interface can be either static or dynamic.


I am not sure it's worth to invest more effort into that. Eventually, all interfaces will be dynamic but some will just always provide the same static configuration. The reason for splitting this into two variants is that we need the configurability in OTBN, but at the time being, we don't want to change LC_CTRL, OTP_CTRL, ROM_CTRL which also use an app interface.

etterli

Thanks @vogelpi for the review. I addressed some of the comments. Open points are:

Refactor / clean up the FSM
Adapt the error handling
Change the name of the dynamic config
Come up with a good naming for 'last' and 'finished'

etterli · 2026-05-22T14:39:29Z

Newest push contains

The dynamic configuration is now renamed to session config.
The last and finished flags are renamed to req_last and rsp_finish.

Open points are:

Refactor / clean up the FSM
Adapt the error handling

etterli · 2026-05-26T08:33:48Z

I have now adapted the error handling and refactored the FSM.

vogelpi · 2026-05-26T12:54:41Z

+Once SW has cleared the error, the interface then triggers a PROCESS command to bring the hashing engine back into the idle state.
+
+Case 3: If the key gets invalidated after the interface sent the PROCESS command (i.e., if in `StAppProcess` or `StAppWait`) the message is already complete and the PROCESS command may not be issued again.
+The app interface still sends the error response and a dynamic interface waits for the finish response which it responds to with a finish response which has the error flag set (= 1).


waits for the finish request which ...

Actually, you're talking about finish requests and finish responses here but the interface defines a last request and a finish response. I recommend to align the terminology with the signals in the interface.

I now have renamed all the finish requests to termination requests. Now we have the following requests / responses which should be quite easy to differentiate:

message requests

last requests / process commands

termination requests

digest responses

finish responses

Renaming these termination requests to also 'last requests' would have again been conflicting with the last requests.

Hm, it's getting a bit confusing. Maybe we can discuss this tomorrow. We probably don't have to change much to make this easier for others to understand.

vogelpi · 2026-05-26T12:58:44Z

+A dynamic interface also sends an immediate error response but then waits until a finish request arrives.
+The finish request is then answered with a finish response which has the error flag set (= 1).
+Both interface types then wait for SW to acknowledged the error by writing to the `error_processed` bit.
+Once SW has cleared the error, the interface then triggers a PROCESS command to bring the hashing engine back into the idle state.


This sounds good to me. Can you please ensure that for this PROCESS command, we enforce that the entropy_fast_process bit forwarded to the SHA3 engine is 0?

Which way to ensure this is preferred:

By checking that SW configured this correctly before starting a KMAC operation and raising a service rejected error

By overwriting this signal / The app interface sets the actual signal to the SHA3 engine? Is this possible at runtime?

Sorry, I misunderstand the request. It must clearly be set to 0 by the app interface at runtime, once we recover the error.

Btw, why should this be 0?

I implemented it so that the bit is overwritten for the duration of the error recovery.

@vogelpi Have you also checked the implementation thereof? Is it as you requested it?

Thanks for the reminder. I overlooked this comment initially.

The way you implemented this is fine. But to be on the safe side, I would drive the disable_entropy_fast_process signal to 1 in all error states. The result of this is that whenever we experience an error, any hashing operations of the SHA3 core will be done by updating the PRNG (hence the masking is fully enabled). For power saving, the entropy_fast_process bit got added originally but I see a slight risk of leaving this disabled during error handling. It could be used to trigger improperly masked hashing operations.

All error states now force this signal to 0.

vogelpi

Thanks @etterli for implementing the changes. I did another full pass over this PR and it looks really great. I have a few remaining comments, most of them are minor, or can be implemented with little effort. Please let me know if you have further questions.

etterli

Thank you @vogelpi for the review. I addressed your feedback and have one question regarding the fast entropy overwrite.

etterli · 2026-05-28T10:06:37Z

+Once the interface has sent the finish response, it will return to its idle state, ready to serve the next app request.
+
+When the app receives the finish response, it must check it for errors.
+The reason is that there could have been an error in the last digest but this was not reported immediately to satisfy to the valid locked-in property.


Added a small explanation.

etterli · 2026-05-28T10:08:56Z

+A dynamic interface also sends an immediate error response but then waits until a finish request arrives.
+The finish request is then answered with a finish response which has the error flag set (= 1).
+Both interface types then wait for SW to acknowledged the error by writing to the `error_processed` bit.
+Once SW has cleared the error, the interface then triggers a PROCESS command to bring the hashing engine back into the idle state.


@vogelpi Have you also checked the implementation thereof? Is it as you requested it?

etterli · 2026-05-28T10:26:42Z

-    AppKMAC   = 2
+  // Application interface type. Either use a compile-time defined configuration or the app
+  // provides a hashing configuration at runtime.
+  typedef enum logic {


@vogelpi Can you maybe clarify this?

rswarbrick

I haven't managed to get GitHub to show me any other responses you sent on this PR today (apparently, that's not possible for some reason).

These are responses to lines that are still visible on the pending changes :-)

rswarbrick · 2026-05-28T11:15:53Z

+If this error occurs, the app interface will not send a response or handle any message requests.
+
+##### Service rejected error
+This error occurs when the configuration is invalid (both dynamic and static) or a KMAC operation is requested when the entropy is not ready.


But that requires testing... Since we don't care, it's better not to specify that this happens.

rswarbrick · 2026-05-28T11:16:40Z

+As of this no data is ever pushed into the hashing engine.
+After the last message request, the app interface then immediately sends a response with garbage data and the error flag set.
+A static interface then directly returns into the Idle state without waiting for SW to set the `error_processed` bit.
+A dynamic interface waits until a termination request is received and answers with a finish acknowledgment response.


Sounds reasonable to me. But it needs documenting!

rswarbrick · 2026-05-28T11:18:14Z

-    AppKMAC   = 2
+  // Application interface type. Either use a compile-time defined configuration or the app
+  // provides a hashing configuration at runtime.
+  typedef enum logic {


I really hope that you don't want to be able to specify that the parameter is 'x... I strongly believe that values that are boolean should be encoded as bit, rather than logic.

etterli · 2026-05-28T11:55:02Z

Hey @rswarbrick I went through all your initial comments and now only the relevant comments are still open. Here is also a list of direct links to all comments which are of interest to review my answer. Once the new tab opens, click the "load more" link. It will then jump to the comment (it takes a while).

#30141 (comment)
#30141 (comment)
#30141 (comment)
#30141 (comment)
#30141 (comment)
#30141 (comment)
#30141 (comment)
#30141 (comment)
#30141 (comment)
#30141 (comment)
#30141 (comment)

etterli · 2026-05-29T13:10:17Z

I addressed the following points:

Disabled the fast entropy in all error recovery states
Documented the Ready sources
The key error no longer drops the valid towards the FIFO

vogelpi · 2026-06-01T13:19:42Z

I addressed the following points:

* Disabled the fast entropy in all error recovery states

* Documented the Ready sources

* The key error no longer drops the valid towards the FIFO

Thanks @etterli , this looks great!

etterli · 2026-06-03T10:41:30Z

Rebased on master with some minor style fixes.

etterli · 2026-06-03T12:39:55Z

Rebased again on master which now includes #30229.

etterli · 2026-06-04T11:01:05Z

I changed the description how an app terminates the message phase. This includes the case how OTBN will end the phase. It was already implemented this way but not specified.

nasahlpa

Thanks Pascal - I again had a look into spec and RTL - which looks good to me. Great work!

Signed-off-by: Pascal Etterli <pascal.etterli@lowrisc.org>

etterli · 2026-06-08T14:51:11Z

Force-push rebased on lastest master.

etterli · 2026-06-09T20:30:04Z

Closing this PR in favor of #30143. This PR here now just serves as documentation of the specification and RTL implementation review.

etterli added the IP:otbn label May 18, 2026

etterli self-assigned this May 18, 2026

etterli force-pushed the otbn-kmac-if-rtl branch from 53a5031 to b68efc3 Compare May 18, 2026 14:26

etterli mentioned this pull request May 18, 2026

[kmac] Extend KMAC app interface with a dynamic interface type #30143

Merged

etterli requested review from h-filali, nasahlpa, rswarbrick and vogelpi May 18, 2026 15:56

rswarbrick reviewed May 18, 2026

View reviewed changes

etterli commented May 19, 2026

View reviewed changes

etterli force-pushed the otbn-kmac-if-rtl branch 4 times, most recently from b6b80ec to 4f86a12 Compare May 20, 2026 17:02

nasahlpa reviewed May 21, 2026

View reviewed changes

etterli force-pushed the otbn-kmac-if-rtl branch from 4f86a12 to 5cafde2 Compare May 21, 2026 15:13

vogelpi reviewed May 21, 2026

View reviewed changes

etterli force-pushed the otbn-kmac-if-rtl branch from 5cafde2 to ce544ec Compare May 22, 2026 12:04

etterli commented May 22, 2026

View reviewed changes

etterli force-pushed the otbn-kmac-if-rtl branch 2 times, most recently from 608c39f to 786c5e9 Compare May 22, 2026 14:38

nasahlpa reviewed May 26, 2026

View reviewed changes

etterli force-pushed the otbn-kmac-if-rtl branch from 786c5e9 to 1416e53 Compare May 26, 2026 08:32

etterli force-pushed the otbn-kmac-if-rtl branch 2 times, most recently from eface93 to 318d780 Compare May 26, 2026 12:05

vogelpi reviewed May 26, 2026

View reviewed changes

vogelpi reviewed May 28, 2026

View reviewed changes

Comment thread hw/ip/kmac/rtl/kmac_app.sv Outdated

vogelpi approved these changes May 28, 2026

View reviewed changes

etterli force-pushed the otbn-kmac-if-rtl branch from d3dbeac to 9c37024 Compare May 28, 2026 11:06

etterli commented May 28, 2026

View reviewed changes

rswarbrick reviewed May 28, 2026

View reviewed changes

etterli force-pushed the otbn-kmac-if-rtl branch from 9c37024 to e0c1a3c Compare May 28, 2026 11:38

etterli force-pushed the otbn-kmac-if-rtl branch from e0c1a3c to 651216d Compare May 28, 2026 12:48

vogelpi reviewed May 28, 2026

View reviewed changes

Comment thread hw/ip/kmac/rtl/kmac_app.sv Outdated

etterli force-pushed the otbn-kmac-if-rtl branch from 651216d to 6db70a2 Compare May 29, 2026 13:08

etterli force-pushed the otbn-kmac-if-rtl branch 2 times, most recently from 1585336 to f6b53e0 Compare June 3, 2026 10:41

etterli mentioned this pull request Jun 3, 2026

[otbn] Specify the OTBN side of the KMAC interface #30278

Merged

etterli force-pushed the otbn-kmac-if-rtl branch from f6b53e0 to bc3eafb Compare June 3, 2026 12:39

etterli force-pushed the otbn-kmac-if-rtl branch from bc3eafb to 7d905c3 Compare June 3, 2026 13:11

etterli mentioned this pull request Jun 3, 2026

[otbn] RTL implementation of the KMAC interface #30283

Merged

etterli force-pushed the otbn-kmac-if-rtl branch from 7d905c3 to c61901b Compare June 4, 2026 11:00

etterli force-pushed the otbn-kmac-if-rtl branch from c61901b to fdee384 Compare June 4, 2026 13:55

nasahlpa approved these changes Jun 5, 2026

View reviewed changes

etterli added 2 commits June 8, 2026 16:45

[kmac] Specify the dynamic app interface

c219d02

Signed-off-by: Pascal Etterli <pascal.etterli@lowrisc.org>

[kmac,rtl] Implement the dynamic app interface

c050aa3

Signed-off-by: Pascal Etterli <pascal.etterli@lowrisc.org>

etterli force-pushed the otbn-kmac-if-rtl branch from fdee384 to c050aa3 Compare June 8, 2026 14:50

etterli closed this Jun 9, 2026

Uh oh!

Conversation

etterli commented May 18, 2026

Uh oh!

etterli commented May 18, 2026

Uh oh!

rswarbrick left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

etterli left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

vogelpi left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

etterli left a comment • 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!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

etterli commented May 22, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

etterli commented May 26, 2026

etterli left a comment •

edited

Loading

etterli commented May 22, 2026 •

edited

Loading