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

RVFI misrepresents OBI transactions? #410

Closed
silabs-robin opened this issue Feb 27, 2023 · 5 comments
Closed

RVFI misrepresents OBI transactions? #410

silabs-robin opened this issue Feb 27, 2023 · 5 comments
Labels
Component:Other Non-RTL, non-documentation (e.g. bhv, sva) Status:Resolved Issue has been resolved, but closure is pending on git merge and/or issuer confirmation Type:Bug For bugs in any content (RTL, Documentation, etc.)

Comments

@silabs-robin
Copy link
Contributor

Component

Component:Other: For any other issues

Steps to Reproduce

  1. Cvverif hash f7f2d5
  2. My fork https://github.com/silabs-robin/core-v-verif
  3. tcf_cvverif
  4. a_obi_vs_rvfi

If I there is nothing I have overlooked, it seems like rvfi_mem_* reports memory writes that didn't happen.
In the example below it seems like there is an MPU error, so the bus access is denied, but RVFI says the mem got updated.

image
@Silabs-ArjanB Silabs-ArjanB added Component:Other Non-RTL, non-documentation (e.g. bhv, sva) Type:Bug For bugs in any content (RTL, Documentation, etc.) labels Feb 27, 2023
@Silabs-ArjanB
Copy link
Contributor

It looks like an RVFI bug indeed (it ignores the related valid signal and acts as if a memory write actually happened).

@silabs-robin I would assume that rvfi_trap.trap gets asserted as well, right?

@silabs-robin
Copy link
Contributor Author

I would assume that rvfi_trap.trap gets asserted as well, right?

Yes.

image

@silabs-robin silabs-robin mentioned this issue Feb 28, 2023
Merged
halfdan-dolva pushed a commit to halfdan-dolva/cv32e40s that referenced this issue Mar 13, 2023
@Silabs-ArjanB
Merge pull request #747 from silabs-oysteink/silabs-oysteink_issue-410
32e8406 
Fix for issue openhwgroup#410
@Silabs-ArjanB
Copy link
Contributor

Should be fixed by openhwgroup/cv32e40x#816 (once merged from 40X to 40S)

@Silabs-ArjanB Silabs-ArjanB added the Status:Resolved Issue has been resolved, but closure is pending on git merge and/or issuer confirmation label Mar 27, 2023
silabs-oysteink added a commit to silabs-oysteink/cv32e40s that referenced this issue Mar 29, 2023
@silabs-oysteink
Only updating rvfi_mem_rmask and rvfi_mem_wmask if a transaction trul…
2790c50 
…y went into the write buffer (not blocked by WPT, MPU or alignment checker).

Fixes issue openhwgroup#410 on cv32e40s.

Signed-off-by: Oystein Knauserud <Oystein.Knauserud@silabs.com>
silabs-oysteink pushed a commit to silabs-oysteink/cv32e40s that referenced this issue Mar 29, 2023
@Silabs-ArjanB
Merge pull request #816 from silabs-oysteink/silabs-oysteink_s-issue-410
c6f4759 
Fix for issue openhwgroup#410 on cv32e40s
@silabs-oysteink
Copy link
Contributor

@silabs-robin Issue should now be resolved, can you check and close if you can confirm?

@silabs-robin
Copy link
Contributor Author

I am seeing a bound of more than double where the original CEX was found, and we have reports of a bound of "26 cycles / 12k seconds ".
Therefore, I consider the problem fixed.

(If there exists any further problem on deeper depths then I will create a new issue for that separately.)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Component:Other Non-RTL, non-documentation (e.g. bhv, sva) Status:Resolved Issue has been resolved, but closure is pending on git merge and/or issuer confirmation Type:Bug For bugs in any content (RTL, Documentation, etc.)
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@Silabs-ArjanB @silabs-oysteink @silabs-robin