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

Update simple serial commands + AES & KMAC bitstreams & binaries #212

Merged
merged 4 commits into from
Nov 14, 2023

Conversation

vogelpi
Copy link
Collaborator

@vogelpi vogelpi commented Nov 9, 2023

Change simple serial command identifiers

This commit aligns the simple serial command identifiers in
capture.py with the latest changes lowRISC/opentitan@4da6006

In particular the commands change as follows:

  • Setting the fixed key / plaintext for fvsr experiments is now 'f'. Previously this was using 't'. This affects AES, KMAC and SHA3.
  • Doing a fvsr batch encrypt on AES is now triggered using 'e' (previously 'f').

@vogelpi vogelpi requested review from wettermo and johannheyszl and removed request for andreaskurth November 9, 2023 15:50
@vogelpi vogelpi marked this pull request as draft November 9, 2023 15:51
@vogelpi
Copy link
Collaborator Author

vogelpi commented Nov 9, 2023

@nasahlpa @vrozic @johannheyszl after merging lowRISC/opentitan#20248 which allows us to select the trigger type (precise hardware-generated as now vs software controlled like on the real target) some of the simple serial command identifiers changed. This means when you want to generate a binary and bitstream on top of 20248, you also need to include the changes in this PR.

@vogelpi
Copy link
Collaborator Author

vogelpi commented Nov 9, 2023

I've marked this as draft because it fails CI (we need to update the binaries, bitstreams and the command identifiers in sync).

Copy link
Collaborator

@johannheyszl johannheyszl left a comment

Choose a reason for hiding this comment

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

Thanks @vogelpi LGTM

I guess this illustrates nicely how the future OTTF-based commands will improve since the multi-letter commands have better readability and space :)

Copy link
Member

@nasahlpa nasahlpa left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!

@vogelpi
Copy link
Collaborator Author

vogelpi commented Nov 10, 2023

Ironically, the CW310 capture which should have failed before didn't fail CI. The capture was aborted due to a mismatch between device and host (as expected due the command identifiers not being inline) but CI wouldn't fail. We should investigate this at some point. For reference, I've attached the log file.
log.txt

@nasahlpa
Copy link
Member

Ironically, the CW310 capture which should have failed before didn't fail CI. The capture was aborted due to a mismatch between device and host (as expected due the command identifiers not being inline) but CI wouldn't fail. We should investigate this at some point. For reference, I've attached the log file. log.txt

Thanks for pointing this out! I had a look into this - the "Capture traces" CI test calls a bash script to capture traces. Although the capturing fails, the script is not correctly passing the error to the CI. Instead of calling bash scrips in azure-pipelines.yml, we should directly call the capture Python script. With the simple capture scripts from #207, this should then be easier.

@vogelpi vogelpi marked this pull request as ready for review November 14, 2023 09:54
@vogelpi vogelpi force-pushed the update-commands branch 2 times, most recently from 0406907 to 6b4d9ee Compare November 14, 2023 12:59
This commit aligns the simple serial command identifiers in
capture.py with the latest changes lowRISC/opentitan@4da6006

Signed-off-by: Pirmin Vogel <vogelpi@lowrisc.org>
Signed-off-by: Pirmin Vogel <vogelpi@lowrisc.org>
To save some logic resources on the CW305, the ECC and scrambling have
been disabled for flash_ctrl, and SPI Host has been stubbed out. In
addition, switching off the masking has been re-enabled by modifying
the masking PRNG.

The bitstream and binary for the CW305 have been generated from
https://github.com/vogelpi/opentitan/tree/ot-sca_2023-11-10_03c5fee_cw305

Signed-off-by: Pirmin Vogel <vogelpi@lowrisc.org>
To save logic resources and allow enabling KMAC masking, the following
hardware changes have been implemented:
- ECC and scrambling have been disabled for flash_ctrl.
- AES masking has been disabled.
- OTBN has been stubbed out

The bitstream and binary for the CW305 have been generated from
https://github.com/vogelpi/opentitan/tree/ot-sca_2023-11-10_03c5fee_cw310

Signed-off-by: Pirmin Vogel <vogelpi@lowrisc.org>
@vogelpi vogelpi changed the title Update simple serial commands Update simple serial commands + AES & KMAC bitstreams & binaries Nov 14, 2023
@vogelpi
Copy link
Collaborator Author

vogelpi commented Nov 14, 2023

Ironically, the CW310 capture which should have failed before didn't fail CI. The capture was aborted due to a mismatch between device and host (as expected due the command identifiers not being inline) but CI wouldn't fail. We should investigate this at some point. For reference, I've attached the log file. log.txt

Thanks for pointing this out! I had a look into this - the "Capture traces" CI test calls a bash script to capture traces. Although the capturing fails, the script is not correctly passing the error to the CI. Instead of calling bash scrips in azure-pipelines.yml, we should directly call the capture Python script. With the simple capture scripts from #207, this should then be easier.

Thanks for your feedback on this Pascal. IIRC there are ways to pass errors through the scripts back in to CI. In general, the advantage of having a script is to be able to quickly re-run the job locally without having to reverse engineer the actual commands running in CI. But I agree, let's look into this after the restructuring, it will make things easier.

@vogelpi
Copy link
Collaborator Author

vogelpi commented Nov 14, 2023

I've now also updated the binaries for AES and KMAC. CI passes, I am merging this.

@vogelpi vogelpi merged commit 035826b into lowRISC:master Nov 14, 2023
5 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.

None yet

4 participants