[csrng, dv] Fixed csrng_regwen test - #31067
Conversation
| rand bit [MaxNumHwApps:0] int_state_read_enable; | ||
| bit [MaxNumHwApps:0] chk_int_state_read_enable; | ||
| rand bit [NumApps - 1:0] int_state_read_enable; | ||
| bit [NumApps - 1:0] chk_int_state_read_enable; |
There was a problem hiding this comment.
Could you do this by masking out the bottom cfg.m_num_hw_apps bits of each? It would probably be nicer to have this be inferred from the hardware in the tb, rather than set in a global parameter in the package.
Yes, I know that OpenTitan is full of that sort of ugliness! But we can do better :-)
There was a problem hiding this comment.
Thanks for this suggestion and explaining the reason behind the existence of MaxNumHwApps in person
a6886f4 to
de84782
Compare
|
Thanks @rswarbrick and @glaserf for helping with the review. I agree with this discussion_r3822488371. Instead of introducing a duplicate parameter, I've added |
de84782 to
8bd0d27
Compare
glaserf
left a comment
glaserf
left a comment
There was a problem hiding this comment.
Thanks @KinzaQamar for this PR and yet another improvement to CSRNGs dv!
I only have a small nit, see below.
The test was failing an expected vs actual value check in csrng_regwen_vseq. The actual value is the data read from int_state_read_enable register, which will return a 3-bit read data. But the comparison happened with a 5 bits wide expected chk_int_state_read_enable Also introduced a parameter MaxNumApps in csrng_env_cfg, as the int_state_read_enable register represents bits per instance (which also includes SW app). Signed-off-by: Kinza Qamar <kqzaman@lowrisc.org>
a6886f4 to
d85fb17
Compare
hcallahan-lowrisc
left a comment
hcallahan-lowrisc
left a comment
There was a problem hiding this comment.
Thanks for fixing this @KinzaQamar, and for following up on the reviewers' comments so quickly. LGTM!
4 participants
The test was failing an expected vs actual value check in csrng_regwen_vseq.sv#L61. The actual value is the data read from int_state_read_enable register, which will return a 3-bit read data. But the comparison happened with a 5 bits wide expected chk_int_state_read_enable