From eeedf63c86bf9ee72eecdbedf66665b9bbbdcf4c Mon Sep 17 00:00:00 2001 From: Boris Shingarov Date: Tue, 7 Apr 2020 16:26:24 -0400 Subject: [PATCH] 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 Maintainer: Jason Lowe-Power Tested-by: kokoro --- src/cpu/simple/atomic.cc | 2 ++ src/cpu/simple/base.cc | 9 ++++++--- src/cpu/simple/base.hh | 1 + src/cpu/simple/timing.cc | 2 ++ 4 files changed, 11 insertions(+), 3 deletions(-) diff --git a/src/cpu/simple/atomic.cc b/src/cpu/simple/atomic.cc index 4b9b773c57..12accc3f6d 100644 --- a/src/cpu/simple/atomic.cc +++ b/src/cpu/simple/atomic.cc @@ -646,6 +646,8 @@ AtomicSimpleCPU::tick() return; } + serviceInstCountEvents(); + Fault fault = NoFault; TheISA::PCState pcState = thread->pcState(); diff --git a/src/cpu/simple/base.cc b/src/cpu/simple/base.cc index 0a4595ce47..135094fc30 100644 --- a/src/cpu/simple/base.cc +++ b/src/cpu/simple/base.cc @@ -302,6 +302,12 @@ BaseSimpleCPU::setupFetchRequest(const RequestPtr &req) 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 @@ BaseSimpleCPU::preExecute() 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(); diff --git a/src/cpu/simple/base.hh b/src/cpu/simple/base.hh index 811713864b..cee786d005 100644 --- a/src/cpu/simple/base.hh +++ b/src/cpu/simple/base.hh @@ -130,6 +130,7 @@ class BaseSimpleCPU : public BaseCPU public: void checkForInterrupts(); void setupFetchRequest(const RequestPtr &req); + void serviceInstCountEvents(); void preExecute(); void postExecute(); void advancePC(const Fault &fault); diff --git a/src/cpu/simple/timing.cc b/src/cpu/simple/timing.cc index ad0e0390dd..76bc1af67b 100644 --- a/src/cpu/simple/timing.cc +++ b/src/cpu/simple/timing.cc @@ -805,6 +805,8 @@ TimingSimpleCPU::advanceInst(const Fault &fault) 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,