ot_kmac: Implement error processing + misc. fixes #171
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
(leaving this one to @loiclefort , I do not know the KMAC) |
AlexJones0
force-pushed
the
kmac_improvements
branch
from
8e37156 to
dc99081
Compare
loiclefort
reviewed
Sep 3, 2025
AlexJones0
force-pushed
the
kmac_improvements
branch
2 times, most recently
from
b67c626 to
9e4c912
Compare
loiclefort
reviewed
Sep 3, 2025
As is explained in comments in the code, the current RTL actually exposes the entirety of the internal Keccak state, including the resultant capacity that is not a valid part of the digest, meaning that this capacity should be visible through the state registers. This is the case for all operations, except for SW-initiated operations that use a sideloaded key for KMAC. In this case, HW does indeed zero the capacity as one might expect - see: https://github.com/lowRISC/opentitan/blob/a14e715d0f425bc4986be526bc52bdeb901756bf/hw/ip/kmac/rtl/kmac_app.sv#L727-L728 Signed-off-by: Alex Jones <alex.jones@lowrisc.org>
The KMAC app request API already errors if we have an existing request pending from an APP, but there is an edge case with re-entrancy. Consider: 1. A device calls a kmac->app_request(data). 2. The KMAC processes the data and responds, calling dev_app->fn(resp) 3. As a result of the response, the device wants to process more KMAC requests, so it sends another app request. 4. KMAC thinks the app is not pending but still sees the current app set, so it thinks it is receiving more data. It will try and process that data and then then transition to idle, but the data that was expected to be processed has not been processed. This re-entrancy should be allowed because we are essentially just trying to "queue" an operation now that the previous one is completed, and we want it to run after the KMAC is idle again (i.e. state reset). So, by simply modifying the existing check we can use the existing app request/pending mechanisms to allow safe re-entrancy (also without potential for stack issues). Signed-off-by: Alex Jones <alex.jones@lowrisc.org>
The RTL actually does error even in SW-initiated KMAC commands that are making use of an invalid sideloaded key, with the `app_id` that is reported in the error code just defaulting to zero. Signed-off-by: Alex Jones <alex.jones@lowrisc.org>
Previously, the KMAC `CMD.ERR_PROCESSED` bit was just clearing the
alerts, but not actually doing anything to the internal KMAC state.
This commit implements full error handling for both HW and SW operation
errors. The logic is the same for both types - upon an erroneous
command, the user must write the `err_processed` bit, upon which the
KMAC will flush its internal state by sending a `Process` command,
followed by an `Absorb` command when done. The key differences are:
- For HW app interface errors: as soon as all the data is received, HW
is responded to so we don't stall HW, but the FSM won't return to
idle until SW acknowledges the error. If SW acknowledges the error
before the HW request data is finished sending, then KMAC first waits
for the full request, then responds, then returns to idle.
- For SW app interface errors: the internal KMAC state is flushed as
soon as the error is processed, ignoring any further message feed or
commands and immediately processing, and then finishing ('Done')
when absorbed, before returning to idle.
Signed-off-by: Alex Jones <alex.jones@lowrisc.org>
AlexJones0
force-pushed
the
kmac_improvements
branch
from
9e4c912 to
5d59b42
Compare
loiclefort
approved these changes
Sep 4, 2025
jwnrt
approved these changes
Sep 4, 2025
This was referenced Sep 15, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR contains a a few misc. fixes/improvements based on issues encountered whilst running OpenTitan tests - these changes are explained in the commit messages.
The largest change of this PR is the fourth commit, which introduces error handling through software setting of the
CMD.ERR_PROCESSEDbit. The logic here is based primarily on the RTL's error handling FSM states inkmac_app.sv