Skip to content

Conversation

AlexJones0
Copy link

@AlexJones0 AlexJones0 commented Sep 8, 2025

Fixes behaviour of the AES when operating in manual mode - the presence of input data via the FIFO and the validity of the (sideloaded) key is not checked when performing a manual AES operation. These conditions are only checked when operating in automatic mode.

See relevant RTL: https://github.com/lowRISC/opentitan/blob/83b8114beceb0685b8b2a7ef7e0274aa2a8393ae/hw/ip/aes/rtl/aes_control_fsm.sv#L192-L216

Also see relevant SW test (clears the keymgr sideload key, invalidating it, and then expects a manual mode AES operation to throw no errors): https://github.com/lowRISC/opentitan/blob/df2110dc5caa7df2856747dae886a2ed50208570/sw/device/tests/keymgr_sideload_aes_test.c#L154-L169

See https://github.com/lowRISC/opentitan/blob/83b8114beceb0685b8b2a7ef7e0274aa2a8393ae/hw/ip/aes/rtl/aes_control_fsm.sv#L194-L216

Fixes behaviour of the AES when operating in manual mode - the presence
of input data via the FIFO and the validity of the (sideloaded) key is
not checked when performing a manual AES operation. These conditions are
only checked when operating in automatic mode.

Signed-off-by: Alex Jones <alex.jones@lowrisc.org>
Copy link

@rivos-eblot rivos-eblot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems ok to me, although I did not cross check with RTL ( sorry, too busy :-( )

@AlexJones0
Copy link
Author

Seems ok to me, although I did not cross check with RTL ( sorry, too busy :-( )

No worries, sorry for the recent flood of PRs 😅

@jwnrt if you get a chance, could you check this?

Copy link

@jwnrt jwnrt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I remember this discussion about AES. It looks correct to me.

@AlexJones0 AlexJones0 merged commit f97adba into lowRISC:ot-earlgrey-9.2.0 Sep 10, 2025
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants