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.

Sign up for GitHub

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

Jump to bottom

Adding LMS KAT to RT boot #1507

Merged
merged 1 commit into from
May 20, 2024
Merged

Adding LMS KAT to RT boot #1507

merged 1 commit into from
May 20, 2024

Conversation

nquarton
Copy link
Contributor

No description provided.

@nquarton
Copy link
Contributor Author

nquarton commented May 10, 2024 •
edited

This is the non-ideal approach of always running the LMS KAT at the start of RT FW (before processing commands). Ideally, this would be run exactly once as/if needed before an LMS operation is performed in runtime. This gets a little tricky in rust though as it requires static state to be shared about whether or not the KAT has been run.

This approach adds approximately 1.25 ms of boot time at the start of runtime on 1.1 HW and around 3ms on 1.0 HW.

This is updated to add state in the LMS driver to track whether or not this has been run.

@sree-revoori1
Copy link
Contributor

Can we add a bool to the drivers struct indicating whether or not the KAT has been run instead?

@nquarton
Copy link
Contributor Author

Can we add a bool to the drivers struct indicating whether or not the KAT has been run instead?

I did consider that. It's an accessible place to put some state. The only issue is the drivers don't bring in KATs as a crate. So either

  1. The runtime code needs to enforce this rule, check/update the state, and run the KAT. Certainly possible but feels a little disaggregated.
  2. The drivers need to have a circular dependency on the KATs (is this okay/allowed?)

But if people still like this path (or I'm missing a better option) then I can absolutely do this.

@nquarton nquarton force-pushed the lms_rerun_kat_in_rt branch 2 times, most recently from ca39363 to 785c378 Compare May 13, 2024 19:04
@nquarton
Copy link
Contributor Author

nquarton commented May 13, 2024 •
edited

I have updated this implementation to store state in the LMS driver.

This solution isn't fully ideal because:

  1. The driver cannot handle running the KAT because it would create a cyclic dependency. This means the caller (only one in this case) must call the KAT's new execute_once function first.
  2. This affects ROM (since it touches the driver) and technically ROM doesn't need this functionality. This may be okay at this stage since ROM 1.1 has not been declared.

@nquarton nquarton marked this pull request as ready for review May 13, 2024 23:26
@nquarton nquarton linked an issue May 13, 2024 that may be closed by this pull request
LMS KAT must run before any LMS operations in FW #1470
Closed
jhand2
jhand2 previously approved these changes May 16, 2024
View reviewed changes
mhatrevi
mhatrevi previously approved these changes May 16, 2024
View reviewed changes
@nquarton nquarton dismissed stale reviews from mhatrevi and jhand2 via 0b67f33 May 16, 2024 18:18
@JohnTraverAmd JohnTraverAmd merged commit dd54a5e into chipsalliance:main May 20, 2024
10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jhand2 jhand2 jhand2 approved these changes

@mhatrevi mhatrevi mhatrevi approved these changes

@fdamato fdamato Awaiting requested review from fdamato fdamato is a code owner

@rusty1968 rusty1968 Awaiting requested review from rusty1968 rusty1968 is a code owner

@bluegate010 bluegate010 Awaiting requested review from bluegate010 bluegate010 is a code owner

@ajisaxena ajisaxena Awaiting requested review from ajisaxena ajisaxena is a code owner

@vsonims vsonims Awaiting requested review from vsonims vsonims is a code owner

@korran korran Awaiting requested review from korran korran is a code owner

@JohnTraverAmd JohnTraverAmd Awaiting requested review from JohnTraverAmd JohnTraverAmd is a code owner

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

LMS KAT must run before any LMS operations in FW
5 participants
@nquarton @sree-revoori1 @jhand2 @mhatrevi @JohnTraverAmd