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

[hmac,dv] FIFO empty interrupt and test mode pins #23749

Merged
merged 2 commits into from
Jul 1, 2024
Merged

[hmac,dv] FIFO empty interrupt and test mode pins #23749

merged 2 commits into from
Jul 1, 2024

Conversation

martin-velay
Copy link
Contributor

@martin-velay martin-velay requested a review from a team as a code owner June 20, 2024 14:04
@martin-velay martin-velay requested review from rswarbrick, gdessouky, hcallahan-lowrisc and antmarzam and removed request for a team June 20, 2024 14:04
@martin-velay
Copy link
Contributor Author

martin-velay commented Jun 20, 2024 •
edited
Loading

Up to this commit 259fbc7, in the current state, the regression is passing.
NOTE: and with this RTL fix #23739

@martin-velay martin-velay marked this pull request as draft June 20, 2024 14:06
rswarbrick
rswarbrick reviewed Jun 21, 2024
View reviewed changes
hw/ip/hmac/dv/env/hmac_scoreboard.sv Outdated Show resolved Hide resolved
hw/ip/hmac/dv/env/hmac_scoreboard.sv Outdated Show resolved Hide resolved
hw/ip/hmac/dv/env/hmac_scoreboard.sv Show resolved Hide resolved
hw/ip/hmac/dv/env/hmac_scoreboard.sv Outdated Show resolved Hide resolved
hw/ip/hmac/dv/env/hmac_scoreboard.sv Outdated Show resolved Hide resolved
@martin-velay martin-velay force-pushed the fifo_empty_dv branch 9 times, most recently from b077560 to b1002e0 Compare June 28, 2024 14:22
@martin-velay
Copy link
Contributor Author

@vogelpi I have added a temporary workaround to be able to merge without taking the related RTL fix from this PR #23739.

Now, this PR could be merged into the master and won't break anything.

@martin-velay martin-velay force-pushed the fifo_empty_dv branch 2 times, most recently from 55e2027 to 44048e9 Compare July 1, 2024 08:14
@martin-velay
Copy link
Contributor Author

@vogelpi, as it has been decided to merge the RTL fix from this PR #23739 , I have reverted the proposed workaround mentioned above.
If needed, it could be reestablished, see this commit: 55e2027

@martin-velay martin-velay marked this pull request as ready for review July 1, 2024 08:17
@martin-velay martin-velay added Component:DV DV issue: testbench, test case, etc. IP:hmac labels Jul 1, 2024
@martin-velay martin-velay mentioned this pull request Jul 1, 2024
Merged
@rswarbrick
Copy link
Contributor

Hi @martin-velay! Would you mind rebasing this over recently merged changes?

@martin-velay @rswarbrick
[hmac,dv] FIFO empty interrupt and test mode pins
2dccf11 
  - Verify FIFO empty interrupt register to tackle issue lowRISC#21815
  - Add checks on interrupt pins when test mode is triggered via the
    dedicated register INTR_TEST

Signed-off-by: Martin Velay <[email protected]>
@martin-velay @rswarbrick
[hmac,dv] Refactor reset handling
c7ca60d 
  - moving reset handler in a common location
  - add function/task/class names when ending it to improve readability

Signed-off-by: Martin Velay <[email protected]>
vogelpi
vogelpi approved these changes Jul 1, 2024
View reviewed changes
Copy link
Contributor

@vogelpi vogelpi left a comment

Choose a reason for hiding this comment

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

LGTM, when rebasing, would mind correcting the commit messages? One commit contains two sign-off lines.

@rswarbrick
Copy link
Contributor

Sorry, I jumped the gun! It turns out that the rebasing wasn't too hard. Happy for either of us to merge, but maybe it makes sense to wait long enough to check that I didn't accidentally break the compile.

@vogelpi
Copy link
Contributor

vogelpi commented Jul 1, 2024

Thanks @rswarbrick . The DV checks passed already. What's left are the FPGA tests which are not affected by this. I am thus merging the PR now.

@vogelpi vogelpi merged commit 1be44e2 into lowRISC:master Jul 1, 2024
23 of 29 checks passed
@andreaskurth andreaskurth mentioned this pull request Jul 5, 2024
Closed
andreaskurth added a commit to andreaskurth/opentitan that referenced this pull request Jul 5, 2024
@andreaskurth
[hmac/dv] Map tests to fifo_empty_status_interrupt testpoint
91a6a3d 
All vseqs based on `hmac_base_vseq` enable the FIFO empty status
interrupt, and the scoreboard checks this interrupt (since lowRISC#23749), so
this testpoint gets covered by all tests based on `hmac_base_vseq`.

Signed-off-by: Andreas Kurth <[email protected]>
@andreaskurth andreaskurth mentioned this pull request Jul 5, 2024
Merged
andreaskurth added a commit to andreaskurth/opentitan that referenced this pull request Jul 5, 2024
@andreaskurth
[hmac/dv] Map tests to fifo_empty_status_interrupt testpoint
7ca457b 
All vseqs based on `hmac_base_vseq` enable the FIFO empty status
interrupt, and the scoreboard checks this interrupt (since lowRISC#23749), so
this testpoint gets covered by all tests based on `hmac_base_vseq`.

Signed-off-by: Andreas Kurth <[email protected]>
andreaskurth added a commit that referenced this pull request Jul 6, 2024
@andreaskurth
[hmac/dv] Map tests to fifo_empty_status_interrupt testpoint
c9a3203 
All vseqs based on `hmac_base_vseq` enable the FIFO empty status
interrupt, and the scoreboard checks this interrupt (since #23749), so
this testpoint gets covered by all tests based on `hmac_base_vseq`.

Signed-off-by: Andreas Kurth <[email protected]>
@andreaskurth andreaskurth mentioned this pull request Jul 16, 2024
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@rswarbrick rswarbrick rswarbrick left review comments

@vogelpi vogelpi vogelpi approved these changes

@gdessouky gdessouky Awaiting requested review from gdessouky

@hcallahan-lowrisc hcallahan-lowrisc Awaiting requested review from hcallahan-lowrisc

@antmarzam antmarzam Awaiting requested review from antmarzam

Assignees
No one assigned
Labels
Component:DV DV issue: testbench, test case, etc. IP:hmac
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@martin-velay @rswarbrick @vogelpi