Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[hmac,rtl] Handle incomplete message size after cmd.hash_stop #21710

Closed
gdessouky opened this issue Feb 27, 2024 · 6 comments
Closed

[hmac,rtl] Handle incomplete message size after cmd.hash_stop #21710

gdessouky opened this issue Feb 27, 2024 · 6 comments
Assignees
Labels

Comments

@gdessouky
Copy link
Contributor

Description

With the PR #21307 merged, HMAC now supports the save & restore feature, where context switching of the HMAC across different SW threads can occur at block size granularity. SW can set the CMD.hash_stop bit when the size of the input data provided so far is not yet multiple of block size. HMAC will have to wait until the SW completes the feeding the input data to reach a multiple of block size, and HMAC will hang indefinitely (and thus block the IP for other SW threads), if SW doesn't do that.

It will be good to look into we can mitigate this, e.g. we can indicate an error to the SW, where the SW will then have to react by providing the missing data, or/otherwise fill the message buffer with zeros/randomness and discard the result (since hashed message is not the message intended by SW).

@andreaskurth

@gdessouky gdessouky added this to the Earlgrey-PROD.M3 milestone Feb 27, 2024
@gdessouky gdessouky changed the title [hmac, rtl] Handle incomplete message size after cmd.hash_stop [hmac,rtl] Handle incomplete message size after cmd.hash_stop Feb 27, 2024
@andreaskurth andreaskurth added the prodc-integration ProdC Integration Issues label Mar 22, 2024
@andreaskurth
Copy link
Contributor

Indicating an error to SW and letting it handle that may be sufficient. We have to check if HW implements such an error

@andreaskurth
Copy link
Contributor

@gdessouky: Can SW not simply read the msg_length registers to find out if HMAC has received data for a full block, so that context switch / save & restore is available?

@gdessouky
Copy link
Contributor Author

gdessouky commented May 23, 2024

@gdessouky: Can SW not simply read the msg_length registers to find out if HMAC has received data for a full block, so that context switch / save & restore is available?

Yes, because signaling an error to SW after CMD.hash_stop is triggered if the message length is not multiple of block size eventually opens up questions: when do we signal the error? Do we allow a timeout after CMD.hash_stop is triggered? It gets messy.

The more robust way would be to just to block CMD.hash_stop altogether if message length is not multiple of block size already, either in HW or SW. In SW, the driver would need to check before triggering CMD.hash_stop that message length is already multiple of block size, otherwise HMAC remains occupied with this thread until the message length is complete. What do you think?

Implementing it in HW should be easy; same way we signal invalid configurations, but covering it in DV would need to wait till beyond M4 I think. Otherwise, a SW implementation will work - @ballifatih, do you see any reason why this can't be checked in the driver implementation?

@ballifatih
Copy link
Contributor

In SW, the driver would need to check before triggering CMD.hash_stop that message length is already multiple of block size, otherwise HMAC remains occupied with this thread until the message length is complete. What do you think?

This is reasonable. In the current driver implementation, we make sure that HW always receives full blocks and the driver only hits stop button after full blocks are fed. Unless there is a bug, SW does not issue stop before it is done feeding msg bytes. Only the very last block is a non-complete block, in which case process is invoked.

The error reporting discussed here is nice (mainly for debugging and having more robust driver), but I would assign low priority to it.

@gdessouky
Copy link
Contributor Author

Great, so to summarize, we agree this doesn't require HW changes, and can be ensured in the SW driver implementation. Error reporting (from the SW driver) may be added if we see a need for it.

@andreaskurth
Copy link
Contributor

Perfect. Since this issue is about a potential HW change, which we agreed is not needed, I'm closing it.

@andreaskurth andreaskurth closed this as not planned Won't fix, can't repro, duplicate, stale May 24, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants