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

fixed TL refill request when TL request is not accepted #3704

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

HakamAtassi
Copy link

@HakamAtassi HakamAtassi commented Jan 3, 2025

Related issue:

Type of change: bug report

Impact: API modification

Development Phase: implementation

Release Notes

There is a peculiarity in the rocket chip I$ that seems to be a bug. Namely, it seems that the I$ is coded such that when a miss takes place, the I$ only requests a refill from the TL bus once, and achieves this by checking if the previous cycle has a miss and the I$ is currently missing. This works okay most of the time but doesnt account for the TL.A channel not accepting the request.

ex:
I$_Bug

What I suspect the offending line of code is (line 380 in icache.scala):

image

Waveform after the change:

image

edit:
A more complete pic of the code for context.
image

Copy link

linux-foundation-easycla bot commented Jan 3, 2025

CLA Signed


The committers listed above are authorized under a signed CLA.

@jerryz123
Copy link
Contributor

I'm not sure this is a needed fix. If the A channel does not accept, it is the responsibility of the frontend to request the address again from the Icache.

@HakamAtassi
Copy link
Author

Hey,

Thanks for the response. As far as I'm aware, the I$ doesnt provide a mechanism for the frontend to identify if an accepted I$ request was never accepted by the TL bus. The current frontend implementation I'm working with effectively holds an address on the I$ until it receives its corresponding response. What may result is the following situation:

  • You place a request from the I$.
  • The I$ accepts it (req.fire).
  • One cycle later, the cache detects a miss, place a request on the TL bus, but that request is never accepted.
  • The frontend continues to request from the I$.
  • I$ doesn't request from TL again because it did the previous cycle (but the TL request wasn't accepted).
  • Frontend stalls.

Either the frontend does in-fact have access to a signal to notify it that the TL request was not accepted, the way I'm handling frontend requests is wrong, or the cache should somehow keep track of which requests are accepted on its TL side and which aren't. My change just allows for multiple requests for a refill if the previous one wasn't accepted.

Let me know what you think. Take care.

@jerryz123
Copy link
Contributor

I think you should avoid holding the same address on the ICache until it gets a response, as the ICache is designed to be used in a pipelined design. You should fire the request into the ICache once, check on the next cycle if a response was provided (a hit), and reissue a request if no response was provided.

I realize your request for changing the ICache behavior is reasonable, and I can't immediately spot any potential problem, but I'm concerned about it introducing a subtle bug elsewhere that I don't have the bandwidth to resolve.

If you truly desire the described behavior, you can fork the ICache implementation and simplify it to your needs.

@HakamAtassi
Copy link
Author

HakamAtassi commented Jan 3, 2025

Hey,

That's reasonable. In my implementation I keep a buffer of the previous request and re-request the prev_pc if it never got a response, which effectively holds the address constant on the I$.

You make a fair point though. I'll keep this open if I or anyone else does end up thoroughly verifying things. Thanks again.

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

Successfully merging this pull request may close these issues.

2 participants