summaryrefslogtreecommitdiff
path: root/src
diff options
context:
space:
mode:
authorSteve Reinhardt <steve.reinhardt@amd.com>2010-03-23 08:50:57 -0700
committerSteve Reinhardt <steve.reinhardt@amd.com>2010-03-23 08:50:57 -0700
commit4d77ea7a5783d1de87a8eb804b17a6ef352998ce (patch)
treefbf1fd950e46885fdca93abbaf689f5131752995 /src
parentd484e1b334c6fd3f2721a2a4628c2324ed14fd08 (diff)
downloadgem5-4d77ea7a5783d1de87a8eb804b17a6ef352998ce.tar.xz
cpu: fix exec tracing memory corruption bug
Accessing traceData (to call setAddress() and/or setData()) after initiating a timing translation was causing crashes, since a failed translation could delete the traceData object before returning. It turns out that there was never a need to access traceData after initiating the translation, as the traced data was always available earlier; this ordering was merely historical. Furthermore, traceData->setAddress() and traceData->setData() were being called both from the CPU model and the ISA definition, often redundantly. This patch standardizes all setAddress and setData calls for memory instructions to be in the CPU models and not in the ISA definition. It also moves those calls above the translation calls to eliminate the crashes.
Diffstat (limited to 'src')
-rw-r--r--src/arch/alpha/isa/mem.isa6
-rw-r--r--src/arch/arm/isa/formats/mem.isa2
-rw-r--r--src/arch/mips/isa/formats/mem.isa8
-rw-r--r--src/arch/mips/isa/formats/util.isa3
-rw-r--r--src/arch/power/isa/formats/mem.isa2
-rw-r--r--src/arch/power/isa/formats/util.isa3
-rw-r--r--src/cpu/inorder/resources/cache_unit.cc9
-rw-r--r--src/cpu/simple/atomic.cc7
-rw-r--r--src/cpu/simple/base.cc21
-rw-r--r--src/cpu/simple/base.hh12
-rw-r--r--src/cpu/simple/timing.cc22
11 files changed, 43 insertions, 52 deletions
diff --git a/src/arch/alpha/isa/mem.isa b/src/arch/alpha/isa/mem.isa
index b1703221f..efff0eac7 100644
--- a/src/arch/alpha/isa/mem.isa
+++ b/src/arch/alpha/isa/mem.isa
@@ -275,7 +275,6 @@ def template StoreExecute {{
if (fault == NoFault) {
fault = xc->write((uint%(mem_acc_size)d_t&)Mem, EA,
memAccessFlags, NULL);
- if (traceData) { traceData->setData(Mem); }
}
if (fault == NoFault) {
@@ -310,7 +309,6 @@ def template StoreCondExecute {{
if (fault == NoFault) {
fault = xc->write((uint%(mem_acc_size)d_t&)Mem, EA,
memAccessFlags, &write_result);
- if (traceData) { traceData->setData(Mem); }
}
if (fault == NoFault) {
@@ -344,7 +342,6 @@ def template StoreInitiateAcc {{
if (fault == NoFault) {
fault = xc->write((uint%(mem_acc_size)d_t&)Mem, EA,
memAccessFlags, NULL);
- if (traceData) { traceData->setData(Mem); }
}
return fault;
@@ -478,9 +475,6 @@ def LoadStoreBase(name, Name, ea_code, memacc_code, mem_flags, inst_flags,
mem_flags = makeList(mem_flags)
inst_flags = makeList(inst_flags)
- # add hook to get effective addresses into execution trace output.
- ea_code += '\nif (traceData) { traceData->setAddr(EA); }\n'
-
# Some CPU models execute the memory operation as an atomic unit,
# while others want to separate them into an effective address
# computation and a memory access operation. As a result, we need
diff --git a/src/arch/arm/isa/formats/mem.isa b/src/arch/arm/isa/formats/mem.isa
index 0b0a4c9fa..2f66ca54e 100644
--- a/src/arch/arm/isa/formats/mem.isa
+++ b/src/arch/arm/isa/formats/mem.isa
@@ -172,7 +172,6 @@ def template StoreExecute {{
if (fault == NoFault) {
fault = xc->write((uint%(mem_acc_size)d_t&)Mem, EA,
memAccessFlags, NULL);
- if (traceData) { traceData->setData(Mem); }
}
if (fault == NoFault) {
@@ -204,7 +203,6 @@ def template StoreInitiateAcc {{
if (fault == NoFault) {
fault = xc->write((uint%(mem_acc_size)d_t&)Mem, EA,
memAccessFlags, NULL);
- if (traceData) { traceData->setData(Mem); }
}
// Need to write back any potential address register update
diff --git a/src/arch/mips/isa/formats/mem.isa b/src/arch/mips/isa/formats/mem.isa
index 161a52b06..411cc5fda 100644
--- a/src/arch/mips/isa/formats/mem.isa
+++ b/src/arch/mips/isa/formats/mem.isa
@@ -305,7 +305,6 @@ def template StoreExecute {{
if (fault == NoFault) {
fault = xc->write((uint%(mem_acc_size)d_t&)Mem, EA,
memAccessFlags, NULL);
- if (traceData) { traceData->setData(Mem); }
}
if (fault == NoFault) {
@@ -342,7 +341,6 @@ def template StoreFPExecute {{
if (fault == NoFault) {
fault = xc->write((uint%(mem_acc_size)d_t&)Mem, EA,
memAccessFlags, NULL);
- if (traceData) { traceData->setData(Mem); }
}
if (fault == NoFault) {
@@ -377,7 +375,6 @@ def template StoreCondExecute {{
if (fault == NoFault) {
fault = xc->write((uint%(mem_acc_size)d_t&)Mem, EA,
memAccessFlags, &write_result);
- if (traceData) { traceData->setData(Mem); }
}
if (fault == NoFault) {
@@ -411,7 +408,6 @@ def template StoreInitiateAcc {{
if (fault == NoFault) {
fault = xc->write((uint%(mem_acc_size)d_t&)Mem, EA,
memAccessFlags, NULL);
- if (traceData) { traceData->setData(Mem); }
}
return fault;
@@ -435,8 +431,6 @@ def template StoreCompleteAcc {{
if (fault == NoFault) {
%(op_wb)s;
-
- if (traceData) { traceData->setData(getMemData(xc, pkt)); }
}
return fault;
@@ -459,8 +453,6 @@ def template StoreCompleteAcc {{
if (fault == NoFault) {
%(op_wb)s;
-
- if (traceData) { traceData->setData(getMemData(xc, pkt)); }
}
return fault;
diff --git a/src/arch/mips/isa/formats/util.isa b/src/arch/mips/isa/formats/util.isa
index a6edffeda..708338074 100644
--- a/src/arch/mips/isa/formats/util.isa
+++ b/src/arch/mips/isa/formats/util.isa
@@ -38,9 +38,6 @@ def LoadStoreBase(name, Name, ea_code, memacc_code, mem_flags, inst_flags,
mem_flags = makeList(mem_flags)
inst_flags = makeList(inst_flags)
- # add hook to get effective addresses into execution trace output.
- ea_code += '\nif (traceData) { traceData->setAddr(EA); }\n'
-
# Some CPU models execute the memory operation as an atomic unit,
# while others want to separate them into an effective address
# computation and a memory access operation. As a result, we need
diff --git a/src/arch/power/isa/formats/mem.isa b/src/arch/power/isa/formats/mem.isa
index 1be49c2f7..3bcf0633a 100644
--- a/src/arch/power/isa/formats/mem.isa
+++ b/src/arch/power/isa/formats/mem.isa
@@ -166,7 +166,6 @@ def template StoreExecute {{
if (fault == NoFault) {
fault = xc->write((uint%(mem_acc_size)d_t&)Mem, EA,
memAccessFlags, NULL);
- if (traceData) { traceData->setData(Mem); }
}
if (fault == NoFault) {
@@ -196,7 +195,6 @@ def template StoreInitiateAcc {{
if (fault == NoFault) {
fault = xc->write((uint%(mem_acc_size)d_t&)Mem, EA,
memAccessFlags, NULL);
- if (traceData) { traceData->setData(Mem); }
}
// Need to write back any potential address register update
diff --git a/src/arch/power/isa/formats/util.isa b/src/arch/power/isa/formats/util.isa
index ab1e530b2..8fd7f7daa 100644
--- a/src/arch/power/isa/formats/util.isa
+++ b/src/arch/power/isa/formats/util.isa
@@ -97,9 +97,6 @@ def LoadStoreBase(name, Name, ea_code, memacc_code, mem_flags, inst_flags,
mem_flags = makeList(mem_flags)
inst_flags = makeList(inst_flags)
- # add hook to get effective addresses into execution trace output.
- ea_code += '\nif (traceData) { traceData->setAddr(EA); }\n'
-
# Generate InstObjParams for the memory access.
iop = InstObjParams(name, Name, base_class,
{'ea_code': ea_code,
diff --git a/src/cpu/inorder/resources/cache_unit.cc b/src/cpu/inorder/resources/cache_unit.cc
index 376ea8d26..d12f11a2c 100644
--- a/src/cpu/inorder/resources/cache_unit.cc
+++ b/src/cpu/inorder/resources/cache_unit.cc
@@ -443,6 +443,10 @@ CacheUnit::read(DynInstPtr inst, Addr addr, T &data, unsigned flags)
//The size of the data we're trying to read.
int dataSize = sizeof(T);
+ if (inst->traceData) {
+ inst->traceData->setAddr(addr);
+ }
+
if (inst->split2ndAccess) {
dataSize = inst->split2ndSize;
cache_req->splitAccess = true;
@@ -541,6 +545,11 @@ CacheUnit::write(DynInstPtr inst, T data, Addr addr, unsigned flags,
//The size of the data we're trying to read.
int dataSize = sizeof(T);
+ if (inst->traceData) {
+ inst->traceData->setAddr(addr);
+ inst->traceData->setData(data);
+ }
+
if (inst->split2ndAccess) {
dataSize = inst->split2ndSize;
cache_req->splitAccess = true;
diff --git a/src/cpu/simple/atomic.cc b/src/cpu/simple/atomic.cc
index 05b4ca3e2..7740434d8 100644
--- a/src/cpu/simple/atomic.cc
+++ b/src/cpu/simple/atomic.cc
@@ -451,6 +451,7 @@ AtomicSimpleCPU::write(T data, Addr addr, unsigned flags, uint64_t *res)
if (traceData) {
traceData->setAddr(addr);
+ traceData->setData(data);
}
//The block size of our peer.
@@ -530,12 +531,6 @@ AtomicSimpleCPU::write(T data, Addr addr, unsigned flags, uint64_t *res)
//stop now.
if (fault != NoFault || secondAddr <= addr)
{
- // If the write needs to have a fault on the access, consider
- // calling changeStatus() and changing it to "bad addr write"
- // or something.
- if (traceData) {
- traceData->setData(gtoh(data));
- }
if (req->isLocked() && fault == NoFault) {
assert(locked);
locked = false;
diff --git a/src/cpu/simple/base.cc b/src/cpu/simple/base.cc
index 0104e1b1f..17ba6a10b 100644
--- a/src/cpu/simple/base.cc
+++ b/src/cpu/simple/base.cc
@@ -205,6 +205,27 @@ change_thread_state(ThreadID tid, int activate, int priority)
{
}
+void
+BaseSimpleCPU::prefetch(Addr addr, unsigned flags)
+{
+ if (traceData) {
+ traceData->setAddr(addr);
+ }
+
+ // need to do this...
+}
+
+void
+BaseSimpleCPU::writeHint(Addr addr, int size, unsigned flags)
+{
+ if (traceData) {
+ traceData->setAddr(addr);
+ }
+
+ // need to do this...
+}
+
+
Fault
BaseSimpleCPU::copySrcTranslate(Addr src)
{
diff --git a/src/cpu/simple/base.hh b/src/cpu/simple/base.hh
index 39961fb88..87e211521 100644
--- a/src/cpu/simple/base.hh
+++ b/src/cpu/simple/base.hh
@@ -232,16 +232,8 @@ class BaseSimpleCPU : public BaseCPU
Addr getEA() { panic("BaseSimpleCPU::getEA() not implemented\n");
M5_DUMMY_RETURN}
- void prefetch(Addr addr, unsigned flags)
- {
- // need to do this...
- }
-
- void writeHint(Addr addr, int size, unsigned flags)
- {
- // need to do this...
- }
-
+ void prefetch(Addr addr, unsigned flags);
+ void writeHint(Addr addr, int size, unsigned flags);
Fault copySrcTranslate(Addr src);
diff --git a/src/cpu/simple/timing.cc b/src/cpu/simple/timing.cc
index 221cb0d0d..7583c09e6 100644
--- a/src/cpu/simple/timing.cc
+++ b/src/cpu/simple/timing.cc
@@ -426,6 +426,10 @@ TimingSimpleCPU::read(Addr addr, T &data, unsigned flags)
int data_size = sizeof(T);
BaseTLB::Mode mode = BaseTLB::Read;
+ if (traceData) {
+ traceData->setAddr(addr);
+ }
+
RequestPtr req = new Request(asid, addr, data_size,
flags, pc, _cpuId, tid);
@@ -460,11 +464,6 @@ TimingSimpleCPU::read(Addr addr, T &data, unsigned flags)
thread->dtb->translateTiming(req, tc, translation, mode);
}
- if (traceData) {
- traceData->setData(data);
- traceData->setAddr(addr);
- }
-
return NoFault;
}
@@ -548,6 +547,11 @@ TimingSimpleCPU::write(T data, Addr addr, unsigned flags, uint64_t *res)
int data_size = sizeof(T);
BaseTLB::Mode mode = BaseTLB::Write;
+ if (traceData) {
+ traceData->setAddr(addr);
+ traceData->setData(data);
+ }
+
RequestPtr req = new Request(asid, addr, data_size,
flags, pc, _cpuId, tid);
@@ -584,13 +588,7 @@ TimingSimpleCPU::write(T data, Addr addr, unsigned flags, uint64_t *res)
thread->dtb->translateTiming(req, tc, translation, mode);
}
- if (traceData) {
- traceData->setAddr(req->getVaddr());
- traceData->setData(data);
- }
-
- // If the write needs to have a fault on the access, consider calling
- // changeStatus() and changing it to "bad addr write" or something.
+ // Translation faults will be returned via finishTranslation()
return NoFault;
}