commit eeedf63c86bf9ee72eecdbedf66665b9bbbdcf4c
author Boris Shingarov <shingarov@labware.com> Tue Apr 07 16:26:24 2020 -0400
committer Boris Shingarov <shingarov@labware.com> Wed Jul 07 22:55:06 2021 +0000
tree 5b9d8d396df84a9ed96191e05c197b3828b88990
parent c046e612165e0e76ca2e689521f4e87fc8ab3480

cpu: Check for instruction-count events before fetch

Instruction fetch should not commence if there already is an
instruction-count event in the queue.

The most conspicuous scenario where this leads to obvious breakage,
is guest debugging.  Imagine the first bytes in the program pointed to
by _start are invalid instruction encoding, and we pass the --wait-gdb
flag.  Then in GDB we set $pc to point to valid instructions, and we
"continue".  gem5 will abort with "invalid instruction".

This is not how real targets behave: neither software- (e.g. ptrace)
based debuggers, nor low-level (e.g. OpenOCD or XMD connected over
JTAG to debug early initialization code eg when the MMU has not been
switched on yet, etc.)  Fetching should start from where $pc was set
to.  This patch tries to model this behavior.

Change-Id: Ibce6fdbbb082edf1073ae96745bc7867878f99ca
Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/27587
Reviewed-by: Jason Lowe-Power <power.jg@gmail.com>
Maintainer: Jason Lowe-Power <power.jg@gmail.com>
Tested-by: kokoro <noreply+kokoro@google.com>

src/cpu/simple/atomic.cc

diff --git a/src/cpu/simple/atomic.cc b/src/cpu/simple/atomic.cc
index 4b9b773..12accc3 100644
--- a/src/cpu/simple/atomic.cc
+++ b/src/cpu/simple/atomic.cc
@@ -646,6 +646,8 @@
             return;
         }
 
+        serviceInstCountEvents();
+
         Fault fault = NoFault;
 
         TheISA::PCState pcState = thread->pcState();

src/cpu/simple/base.cc

diff --git a/src/cpu/simple/base.cc b/src/cpu/simple/base.cc
index 0a4595c..135094f 100644
--- a/src/cpu/simple/base.cc
+++ b/src/cpu/simple/base.cc
@@ -302,6 +302,12 @@
                  instRequestorId(), instAddr);
 }
 
+void
+BaseSimpleCPU::serviceInstCountEvents()
+{
+    SimpleExecContext &t_info = *threadInfo[curThread];
+    t_info.thread->comInstEventQueue.serviceEvents(t_info.numInst);
+}
 
 void
 BaseSimpleCPU::preExecute()
@@ -316,9 +322,6 @@
     t_info.setPredicate(true);
     t_info.setMemAccPredicate(true);
 
-    // check for instruction-count-based events
-    thread->comInstEventQueue.serviceEvents(t_info.numInst);
-
     // decode the instruction
     TheISA::PCState pcState = thread->pcState();
 

src/cpu/simple/base.hh

diff --git a/src/cpu/simple/base.hh b/src/cpu/simple/base.hh
index 8117138..cee786d 100644
--- a/src/cpu/simple/base.hh
+++ b/src/cpu/simple/base.hh
@@ -130,6 +130,7 @@
   public:
     void checkForInterrupts();
     void setupFetchRequest(const RequestPtr &req);
+    void serviceInstCountEvents();
     void preExecute();
     void postExecute();
     void advancePC(const Fault &fault);

src/cpu/simple/timing.cc

diff --git a/src/cpu/simple/timing.cc b/src/cpu/simple/timing.cc
index ad0e039..76bc1af 100644
--- a/src/cpu/simple/timing.cc
+++ b/src/cpu/simple/timing.cc
@@ -805,6 +805,8 @@
     if (tryCompleteDrain())
         return;
 
+    serviceInstCountEvents();
+
     if (_status == BaseSimpleCPU::Running) {
         // kick off fetch of next instruction... callback from icache
         // response will cause that instruction to be executed,