[entropy_src/dv] Test whether noise source symbols are not dropped #21821
Conversation
h-filali
requested review from
hcallahan-lowrisc,
vogelpi and
andreaskurth
and removed request for
a team and
hcallahan-lowrisc
| end else begin | ||
| observe_fifo_drops++; | ||
| msg = $sformatf("Dropped word: %d\n", observe_fifo_drops); | ||
| fork |
There was a problem hiding this comment.
Our of curiosity: why do we need the fork/join_none here?
There was a problem hiding this comment.
I had to add a backdoor read which is time consuming. This is inside of a function so time consuming operations aren't allowed here. So I had two possible solutions either we turn the function into a task or put the time consuming part inside a fork. I went for the latter since in my experience we want the process_tl_access task to consume as little time as possible (I ran into issues in the past because of this).
What do you think should I leave it, turn it into a function or add a comment explaining this?
There was a problem hiding this comment.
Thanks for adding the comment and for explaining. I think that's sufficient. I wasn't aware that forks are even possible inside functions :-)
| // We need to make sure that we can read out 1024 contiguous symbols, that's why | ||
| // we set the probability for 128 seeds to 40 pct (this is exactly 1024 symbols | ||
| // when using all 4 lanes). |
There was a problem hiding this comment.
Sorry, I can't follow. Would you mind explaining what seed_cnt is please? I couldn't find this really in the code. I know it was there before and you haven't invented the concept but you seem to understand it :-)
There was a problem hiding this comment.
So the way I understood it, the seed_cnt used to be the number of 32 bit words extracted in the vseq. In the 4 lane mode one 32 bit word contains 8 symbols. 128 times 8 symbols is exactly the 1024 symbols we want to read out contiguously. I'll write it a bit more clear.
There was a problem hiding this comment.
I just noticed. For other vseqs it's the number of bundles read from the observe FIFO. I'll add a new variable for this and keep the seed_cnt as is. For our case here I think it's better to deal in words rather than bundles.
There was a problem hiding this comment.
Okay, this sounds good. Thanks for explaining.
h-filali
force-pushed
the
es-observe-entropy-match
branch
2 times, most recently
from
701d9d3
to
e5c14c3
Compare
|
@vogelpi thanks for reviewing. |
This commit adds changes to checks for comparing entropy in the observe FIFO to the actual noise from the noise source. The scoreboard now knows when pushed into the observe FIFO is dropped and when it isnt. This commit also revives a vseq that was unused and changes some settings. Furthermore, the task reading entropy from the observe FIFO now has optional checks to make sure that the entropy is contiguous. Signed-off-by: Hakim Filali <[email protected]>
vogelpi
force-pushed
the
es-observe-entropy-match
branch
from
e5c14c3
to
c2ae606
Compare
|
I've now rebased this on master. There was an issue not related to this PR. |
|
I am merging this. There was one failure in the SiVal tests but it's spurious and unrelated to this PR. This PR only touches DV files. |
This PR adds checks for the comparison between entropy coming in from the noise source and the entropy going into the observe FIFO.
For further information check the commit message.
This reasolves #20953
I had to change the used vseq for the fw_ov test from the rng_vseq to the fw_ov_vseq.
We need to discuss whether I should add a new test instead.
I also still need to set the cfg for the fw_ov test appropriately once discussed.