-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
cpu: Don't change to suspend if the thread status is halted #1039
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
In our gem5 model, there are four types represent thread context: Active, Suspend, Halting and Halted https://github.com/gem5/gem5/blob/5641c5e4642f7d44651f783bdb018d3cf8ba01b5/src/cpu/thread_context.hh#L99-L117 When initializing the gem5 instance, all of the thread contexts are set Halted. The status of thread context will not be active until the Workload initializes start up, except the StubWorkload. So if the user uses the StubWorkload, and the CPU is connected with the model_reset port. The thread context of the CPU will be activated possibly. The following is the steps of activating thread context of the CPU without Workload[1] initialization or lower model_reset port[2]. 1. Raise the model_reset port (Change the state from Halted to Suspend) https://github.com/gem5/gem5/blob/5641c5e4642f7d44651f783bdb018d3cf8ba01b5/src/cpu/base.cc#L671-L673 2. Post the interrupt to CPU (Change the state from Suspend to Active) https://github.com/gem5/gem5/blob/5641c5e4642f7d44651f783bdb018d3cf8ba01b5/src/cpu/base.cc#L231-L239 Implementation of wakeup SimpleCPU: https://github.com/gem5/gem5/blob/5641c5e4642f7d44651f783bdb018d3cf8ba01b5/src/cpu/simple/base.cc#L251-L259 MinorCPU: https://github.com/gem5/gem5/blob/5641c5e4642f7d44651f783bdb018d3cf8ba01b5/src/cpu/minor/cpu.cc#L143-L151 O3CPU: https://github.com/gem5/gem5/blob/5641c5e4642f7d44651f783bdb018d3cf8ba01b5/src/cpu/o3/cpu.cc#L1337-L1346 This CL fixed the issue when raising the model reset port to CPU(let CPU sleep) if the CPU is not activated by workload. If the CPU status is halted, it's should not change to Suspend to avoid wake up Reference The model_reset is introduced in the CL: https://gem5-review.googlesource.com/c/public/gem5/+/67574/4 [1] Activate by workload (ARM example): https://github.com/gem5/gem5/blob/5641c5e4642f7d44651f783bdb018d3cf8ba01b5/src/arch/arm/fs_workload.cc#L101-L114 [2] Lower the model_reset: https://github.com/gem5/gem5/blob/5641c5e4642f7d44651f783bdb018d3cf8ba01b5/src/cpu/base.cc#L191-L192 https://github.com/gem5/gem5/blob/5641c5e4642f7d44651f783bdb018d3cf8ba01b5/src/cpu/base.cc#L674-L685 Change-Id: I5bfc0b7491d14369fff77b98b71c0ac763fb7c42
rogerchang23424
force-pushed
the
fix-cpu-halted
branch
from
April 26, 2024 02:55
89883e7
to
306294f
Compare
Hi @giactra, I noticed that you self-requested a review for this PR. Do you have time to look at it? Thanks. |
BobbyRBruce
approved these changes
May 13, 2024
BobbyRBruce
pushed a commit
to BobbyRBruce/gem5
that referenced
this pull request
May 25, 2024
In our gem5 model, there are four types represent thread context: Active, Suspend, Halting and Halted https://github.com/gem5/gem5/blob/5641c5e4642f7d44651f783bdb018d3cf8ba01b5/src/cpu/thread_context.hh#L99-L117 When initializing the gem5 instance, all of the thread contexts are set Halted. The status of thread context will not be active until the Workload initializes start up, except the StubWorkload. So if the user uses the StubWorkload, and the CPU is connected with the model_reset port. The thread context of the CPU will be activated possibly. The following is the steps of activating thread context of the CPU without Workload[1] initialization or lower model_reset port[2]. 1. Raise the model_reset port (Change the state from Halted to Suspend) https://github.com/gem5/gem5/blob/5641c5e4642f7d44651f783bdb018d3cf8ba01b5/src/cpu/base.cc#L671-L673 2. Post the interrupt to CPU (Change the state from Suspend to Active) https://github.com/gem5/gem5/blob/5641c5e4642f7d44651f783bdb018d3cf8ba01b5/src/cpu/base.cc#L231-L239 Implementation of wakeup SimpleCPU: https://github.com/gem5/gem5/blob/5641c5e4642f7d44651f783bdb018d3cf8ba01b5/src/cpu/simple/base.cc#L251-L259 MinorCPU: https://github.com/gem5/gem5/blob/5641c5e4642f7d44651f783bdb018d3cf8ba01b5/src/cpu/minor/cpu.cc#L143-L151 O3CPU: https://github.com/gem5/gem5/blob/5641c5e4642f7d44651f783bdb018d3cf8ba01b5/src/cpu/o3/cpu.cc#L1337-L1346 This CL fixed the issue when raising the model reset port to CPU(let CPU sleep) if the CPU is not activated by workload. If the CPU status is halted, it's should not change to Suspend to avoid wake up Reference The model_reset is introduced in the CL: https://gem5-review.googlesource.com/c/public/gem5/+/67574/4 [1] Activate by workload (ARM example): https://github.com/gem5/gem5/blob/5641c5e4642f7d44651f783bdb018d3cf8ba01b5/src/arch/arm/fs_workload.cc#L101-L114 [2] Lower the model_reset: https://github.com/gem5/gem5/blob/5641c5e4642f7d44651f783bdb018d3cf8ba01b5/src/cpu/base.cc#L191-L192 https://github.com/gem5/gem5/blob/5641c5e4642f7d44651f783bdb018d3cf8ba01b5/src/cpu/base.cc#L674-L685 Change-Id: I5bfc0b7491d14369fff77b98b71c0ac763fb7c42
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
In our gem5 model, there are four types represent thread context: Active, Suspend, Halting and Halted
gem5/src/cpu/thread_context.hh
Lines 99 to 117 in 5641c5e
When initializing the gem5 instance, all of the thread contexts are set Halted. The status of thread context will not be active until the Workload initializes start up, except the StubWorkload. So if the user uses the StubWorkload, and the CPU is connected with the model_reset port. The thread context of the CPU will be activated possibly.
The following is the steps of activating thread context of the CPU without Workload[1] initialization or lower model_reset port[2].
Raise the model_reset port (Change the state from Halted to Suspend)
gem5/src/cpu/base.cc
Lines 671 to 673 in 5641c5e
Post the interrupt to CPU (Change the state from Suspend to Active)
gem5/src/cpu/base.cc
Lines 231 to 239 in 5641c5e
Implementation of wakeup
SimpleCPU:
gem5/src/cpu/simple/base.cc
Lines 251 to 259 in 5641c5e
MinorCPU:
gem5/src/cpu/minor/cpu.cc
Lines 143 to 151 in 5641c5e
O3CPU:
gem5/src/cpu/o3/cpu.cc
Lines 1337 to 1346 in 5641c5e
This CL fixed the issue when raising the model reset port to CPU(let CPU sleep) if the CPU is not activated by workload. If the CPU status is halted, it's should not change to Suspend to avoid wake up
Reference
The model_reset is introduced in the CL:
https://gem5-review.googlesource.com/c/public/gem5/+/67574/4
[1] Activate by workload (ARM example):
gem5/src/arch/arm/fs_workload.cc
Lines 101 to 114 in 5641c5e
[2] Lower the model_reset:
gem5/src/cpu/base.cc
Lines 191 to 192 in 5641c5e
gem5/src/cpu/base.cc
Lines 674 to 685 in 5641c5e
Change-Id: I5bfc0b7491d14369fff77b98b71c0ac763fb7c42