-
Notifications
You must be signed in to change notification settings - Fork 781
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
[otp_ctrl] Increase Hamming distance in OTP commands #21953
Conversation
d3e5e6c
to
affa646
Compare
hw/ip/prim/rtl/prim_otp_pkg.sv
Outdated
parameter int ErrWidth = 3; | ||
|
||
// Encoding generated with: | ||
// $ ./util/design/sparse-fsm-encode.py -d 3 -m 5 -n 6 \ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I thought we typically aimed for higher hamming distances?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For FSM encodings, yes, we would typically do 4 or 5, but at least 3.
I am not sure this is really required in this setting for the command.
We can do 4, but the command needs to be 7bit wide then for this to work.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I increased the HD to 4
a050424
to
cd59108
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thanks!
cd59108
to
c50f44d
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thanks @msfschaffner !
In case you need to push once more, there is also a comment in hw/ip/prim_generic/rtl/prim_generic_otp.sv
mentioning the now outdated 3-bit commands.
For refernce, commit dc9da97 added the raw commands ignoring the integrity.
Fixes lowRISC#21948 Signed-off-by: Michael Schaffner <[email protected]>
c50f44d
to
1b39fc4
Compare
I patched that up, thanks! |
This a security patch for closing out D2S for OTP_CTRL.
Fixes #21948