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.

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

[dv] Clocking block usage and guidelines #23795

Open
GregAC opened this issue Jun 25, 2024 · 0 comments
Open

[dv] Clocking block usage and guidelines #23795

GregAC opened this issue Jun 25, 2024 · 0 comments
Labels
Component:DV DV issue: testbench, test case, etc. Milestone:V3 Type:Enhancement Feature requests, enhancements

Comments

@GregAC
Copy link
Contributor

GregAC commented Jun 25, 2024

Description

I recently discovered an issue with how we were using clocking blocks in the Ibex testbench. This lead to a problem in our memory agent meaning we never stimulated a single cycle response (where you get an rvalid the cycle immediately after a granted request). I have fixed the issue and thankfully there are no new failures in regression (The fix is here lowRISC/ibex#2177)

However we should consider how we are using clocking block across the OpenTitan project. The root of the issue is the event you synchronize on, there's the raw clocking event (i.e. use a @(posedge clk) statement to synchronize) or there's the clocking block event (use a @(interface.clocking_block) statement to synchronize).

The clocking block event fires once the clocking block variables have been updated with their latest values. The raw clocking event happens before this. So if you synchronize on the raw clocking event you're effectively observing things one cycle late (you look at the values before they've updated). There is also a potential race where you can't be certain if the clocking block variables have or have not been updated yet but I think in practice you'll always end up observing the values pre update when you sync on the raw clocking value.

In cases where you're purely observing things to send to a scoreboard using the raw clocking event may not matter in practice though when you want to react to observations it adds an inherent one cycle delay (which is what we had in the Ibex testbench so could never achieve single-cycle response).

We should write some usage guidelines for clocking blocks in the DV style guide (https://github.com/lowRISC/style-guides/blob/master/DVCodingStyle.md) and also review clocking block usage across OpenTitan. I don't propose we change usage right now as I think it'll be massively disruptive (as scoreboards will currently be written expecting the extra cycle of delay on observations) but we should aim to move towards proper synchronization.

The paper here: https://assets.website-files.com/63f4bb21bd5303fe472ad00e/64958c5c4f777e440b2ba235_paper51_taming_tb_timing_FINAL_fixes.pdf goes into great detail about correct clocking block usage and proposes some guidelines that we could adopt.

@GregAC GregAC added Component:DV DV issue: testbench, test case, etc. Type:Enhancement Feature requests, enhancements labels Jun 25, 2024
@andreaskurth andreaskurth added this to the Earlgrey-PROD.M7 milestone Jun 25, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Component:DV DV issue: testbench, test case, etc. Milestone:V3 Type:Enhancement Feature requests, enhancements
Projects
None yet
Development

No branches or pull requests

2 participants