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

[tvla] Fix plotting issues with new database and metadata format #270

Merged
merged 2 commits into from
Jan 8, 2024

Conversation

wettermo
Copy link
Contributor

@wettermo wettermo commented Jan 5, 2024

This commit fixes incompatibility of tvla plotting with the new database and metadata format.

For this, additional metadata is stored in aes, otbn, kmac and sha3 capture. Also, a new wrapper function for plotting is added to the tvla script. Finally, aes, otbn, kmac and sha3 config files are adjusted.

Further, this PR adds code to also include metadata in plots of aes specific tests.

Through this PR two tasks in #72 are addressed:

  • At the moment, TVLA figures contain meta-data only for general TVLA test. We should enable the same for AES specific TVLA tests.
  • At the moment, tvla.py cannot generate some figures if data is loaded from the new database format.

@wettermo wettermo requested review from m-temp, nasahlpa and vrozic and removed request for andreaskurth, nasahlpa, vogelpi and vrozic January 5, 2024 11:42
@wettermo wettermo force-pushed the tvla-fix-plot-restructure branch 2 times, most recently from 48650e4 to 6d6c3ad Compare January 5, 2024 11:50
This commit fixes incompatibility of tvla plotting with the new database
and metadata format.
For this, additional metadata is stored in aes and otbn capture. Also, a
new wrapper function for plotting is added to the tvla script. Finally,
aes and otbn config files are adjusted.
Further, this commit adds code to also include metadata in plots of aes
specific test.

Signed-off-by: Moritz Wettermann <[email protected]>
@wettermo wettermo force-pushed the tvla-fix-plot-restructure branch from 6d6c3ad to f653abd Compare January 5, 2024 12:03
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.

Thanks for adapting all the scripts!

Copy link
Contributor

@vrozic vrozic left a comment

Choose a reason for hiding this comment

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

Thanks @wettermo
This looks good to me.
The CI failure is unrelated to this PR, so I think it can be safely merged.

@wettermo
Copy link
Contributor Author

wettermo commented Jan 8, 2024

Thanks @nasahlpa and @vrozic for the reviews!

@wettermo
Copy link
Contributor Author

wettermo commented Jan 8, 2024

I'm merging this PR, even though CI fails, because CI failure is unrelated to this PR.

@wettermo wettermo merged commit 645b703 into lowRISC:master Jan 8, 2024
6 of 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