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

spr_bus_ack_o reset issue #127

Closed
Harshitha172000 opened this issue Jul 8, 2021 · 5 comments
Closed

spr_bus_ack_o reset issue #127

Harshitha172000 opened this issue Jul 8, 2021 · 5 comments

Comments

@Harshitha172000
Copy link
Member

This issue is applicable to the mor1kx_icache module. After resetting, the state moves to IDLE but spr_bus_ack_o can be high if spr strobe/write signals stays high during reset. This is possible as spr stb/we signals are not reset in the ctrl module.

Assertion Failed:

          always @(posedge clk)
               if ($past(rst) && f_past_valid)
                  assert (!spr_bus_ack_ic_i);

Trace showing issue:

image

stffrdhrn added a commit to stffrdhrn/mor1kx that referenced this issue Jul 9, 2021
Issue: openrisc#127

The reset and error cases override the state to IDLE, we should
check invalidate after this to avoid setting the spr_bus_ack_o
when we are not going to invalidate.

This was detected via formal verification.
@stffrdhrn
Copy link
Member

I posted a fix for this does it also fix formal?

@Harshitha172000
Copy link
Member Author

No, now I see this trace. With the reset, the state switches from INVALIDATE to IDLE and keeps spr_bus_ack_o high.

image

@stffrdhrn
Copy link
Member

OK, it doesn't cater for the case when spr_bus_ack_o is high to start with.

stffrdhrn added a commit to stffrdhrn/mor1kx that referenced this issue Jul 10, 2021
Issue: openrisc#127

The reset and error cases override the state to IDLE, but if we
are invalidating the spr_bus_ack_o stays set which should probably
not cause any problem but is showing up as an issue in formal
verification.

Ensure reset and error override invalidate and also reset the ack
output signal on reset.
@stffrdhrn
Copy link
Member

I pushed another update, how does it look now?

@Harshitha172000
Copy link
Member Author

It passes the test, the issue is resolved.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants