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

arch-arm: Do not issue memory requests for uncacheable prefetches #1140

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

Conversation

arichardson
Copy link
Contributor

@arichardson arichardson commented May 15, 2024

Currently, a prefetch instruction will result in an infinite sleep where
the CPU never wakes back up as it is expecting a dcache response even
though this will not be delivered for prefetches. Avoid this problem by
rejecting uncacheable prefetches in the MMU translation logic.

This allows my test workload from #1139
to continue running beyond strlen(). Tested using Atomic,Timing,Minor
and O3 CPU. See #1139 for the test
case that was used.

Fixes: #1139

Change-Id: Ic44bdb87f4099b11a7f9c6c99768a12fbef5842e

@ivanaamit ivanaamit added the cpu-simple gem5's Simple CPU label May 16, 2024
@ivanaamit
Copy link
Contributor

Hi @arichardson, there are a couple of tests that are failing. Could you please fix that before this can be reviewed/merged? Thank you.

@giactra
Copy link
Contributor

giactra commented May 21, 2024

Hi @arichardson. Software prefetching has a hacky implementation in gem5 which makes it buggy in some cases.
I have already seen several PRs trying to put a patch on a specific problem without really solving the underlying issue.

While this PR fixes your run, I am afraid is not enough to fix SW prefetching properly. That would probably require to handle cpu,arch,mem-cache,mem-ruby implementation of SW prefetches at the same time.
For example in the PR you force the CPU into running but this is simply because the CPU never gets a prefetch response message. However, the gem5 CPU does require a response for the prefetch (even if in real CPUs it does not make sense) as you can see from the SoftPFReq definition:

/* SoftPFReq */
{ {IsRead, IsRequest, IsSWPrefetch, NeedsResponse},
        SoftPFResp, "SoftPFReq" },

So why does your CPU never gets a response? It does so because your data prefetch targets uncacheable data (as you can see from the debug log you posted). Therefore:

  1. It should have never been issued in the first place
  2. It never gets a dummy response from the cache: https://github.com/gem5/gem5/blob/stable/src/mem/cache/cache.cc#L380 (it is uncacheable)

So I believe a partial solution which would make things work for you would be related to point 1, which is: discard a SW prefetch if it targets uncacheable data as data won't be stored anyway...

Currently, a prefetch instruction will result in an infinite sleep where
the CPU never wakes back up as it is expecting a dcache response even
though this will not be delivered for prefetches. Avoid this problem by
rejecting uncacheable prefetches in the MMU translation logic.

This allows my test workload from gem5#1139
to continue running beyond strlen(). Tested using Atomic,Timing,Minor
and O3 CPU. See gem5#1139 for the test
case that was used.

Fixes: gem5#1139

Change-Id: Ic44bdb87f4099b11a7f9c6c99768a12fbef5842e
@arichardson arichardson changed the title cpu-simple: Do not wait for a cache response for prefetches arch-arm: Do not issue memory requests for uncacheable prefetches Jun 6, 2024
@arichardson
Copy link
Contributor Author

I believe I have a better fix for this now, scoped to the Arm MMU code. The new change allows my new reduced test case to pass for all CPU models.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cpu-simple gem5's Simple CPU
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants