I wanted to fix it, but we never really found a way to do that, at least that everybody could agree on. In my opinion, the timing simple CPU needs a subtle but fundamental rework to avoid this class of bugs. Also, that'll take more time than I personally have available these days, so I'm not going to be able to just jump in and fix it myself.
Gabe Steve Reinhardt wrote: > This stack is kind of funky... I'm not sure why Stq::initiateAcc would > call into TimingSimpleCPU::completeIfetch. > > I went back and looked at that ExecEnable bug discussion though, and I > recall that problem... sad to see that after all the discussion we had > almost a year ago the bug is still not fixed (as far as I can tell). > This patch you made here does look like it could be addressing the > same or a similar issue. > > Steve > > On Thu, Mar 18, 2010 at 6:56 PM, Beckmann, Brad <[email protected]> wrote: > >> I don't have exact Valgrind stack anymore, but below is the approximate call >> stack (it is actually a subset of another call stack that I wasn't able to >> completely solve...more on that later). Basically if I remember correctly, >> the request is deleted within sendData. >> >> TimingSimpleCPU::sendData(RefCountingPtr<FaultBase>, Request*, unsigned >> char*, unsigned long*, bool) ==27646== by >> TimingSimpleCPU::DataTranslation::finish(RefCountingPtr<FaultBase>, >> Request*, ThreadContext*, BaseTLB::Mode) (/ >> tmp/bbeckman/m5/build/ALPHA_SE_MOESI_hammer/cpu/simple/timing.hh:139) >> AlphaISA::TLB::translateTiming(Request*, ThreadContext*, >> BaseTLB::Translation*, BaseTLB::Mode) (/tmp/bbeckman/m >> 5/build/ALPHA_SE_MOESI_hammer/arch/alpha/tlb.cc:603) >> RefCountingPtr<FaultBase> TimingSimpleCPU::write<unsigned long>(unsigned >> long, unsigned long, unsigned, unsigne >> d long*) >> (/tmp/bbeckman/m5/build/ALPHA_SE_MOESI_hammer/cpu/simple/timing.cc:599) >> AlphaISAInst::Stq::initiateAcc(TimingSimpleCPU*, Trace::InstRecord*) const >> (/tmp/bbeckman/m5/build/ALPHA_SE_MOE >> SI_hammer/arch/alpha/timing_simple_cpu_exec.cc:1953) >> TimingSimpleCPU::completeIfetch(Packet*) >> (/tmp/bbeckman/m5/build/ALPHA_SE_MOESI_hammer/cpu/simple/timing.cc:769) >> TimingSimpleCPU::IcachePort::recvTiming(Packet*) >> (/tmp/bbeckman/m5/build/ALPHA_SE_MOESI_hammer/cpu/simple/timing.cc:832) >> Port::sendTiming(Packet*) >> (/tmp/bbeckman/m5/build/ALPHA_SE_MOESI_hammer/mem/port.hh:186) >> Bus::recvTiming(Packet*) >> (/tmp/bbeckman/m5/build/ALPHA_SE_MOESI_hammer/mem/bus.cc:243) >> >> >> Now the bug I wasn't able to completely fix involved the bug identified in >> the old thread: "Memory corruption in m5 dev repository whenusing >> --trace-flags="ExecEnable"". That bug as a similar problem where the trace >> data is deleted within the write function and then referenced later in >> initiateAcc, however it is even more confusing because it deals with two >> different pointers to traceData. I have a patch that has a heavyweight fix >> for that bug, but I didn't send it out for review because I couldn't figure >> out how to get it to compile with the O3 cpu model. If you're interested, I >> can send that out as well. >> >> Brad >> >> >> -----Original Message----- >> From: [email protected] [mailto:[email protected]] On Behalf >> Of Steve Reinhardt >> Sent: Thursday, March 18, 2010 4:24 PM >> To: M5 Developer List >> Subject: Re: [m5-dev] [PATCH 07 of 31] m5: Fixed request read bug flagged by >> Valgrind >> >> It seems very odd that translateTiming would delete the request >> object... can you point out where that happens? I couldn't find it in >> the alpha or x86 tlb code. >> >> Thanks, >> >> Steve >> >> On Thu, Mar 18, 2010 at 3:46 PM, Brad Beckmann <[email protected]> wrote: >> >>> # HG changeset patch >>> # User Brad Beckmann <[email protected]> >>> # Date 1268941825 25200 >>> # Node ID c2db9da78da715a22d76e92e0cabbced82dcef9f >>> # Parent d657b9a0875113dbb453986391661fb3c7fa669c >>> m5: Fixed request read bug flagged by Valgrind >>> >>> Previously the recording of an uncached read occurred after the request was >>> possibly deleted within the translateTiming function. >>> >>> diff --git a/src/cpu/simple/timing.cc b/src/cpu/simple/timing.cc >>> --- a/src/cpu/simple/timing.cc >>> +++ b/src/cpu/simple/timing.cc >>> @@ -432,6 +432,10 @@ >>> Addr split_addr = roundDown(addr + data_size - 1, block_size); >>> assert(split_addr <= addr || split_addr - addr < block_size); >>> >>> + // This will need a new way to tell if it's hooked up to a cache or >>> not. >>> + if (req->isUncacheable()) >>> + recordEvent("Uncached Write"); >>> + >>> _status = DTBWaitResponse; >>> if (split_addr > addr) { >>> RequestPtr req1, req2; >>> @@ -461,10 +465,6 @@ >>> traceData->setAddr(addr); >>> } >>> >>> - // This will need a new way to tell if it has a dcache attached. >>> - if (req->isUncacheable()) >>> - recordEvent("Uncached Read"); >>> - >>> return NoFault; >>> } >>> >>> @@ -510,7 +510,6 @@ >>> return read(addr, *(uint32_t*)&data, flags); >>> } >>> >>> - >>> template<> >>> Fault >>> TimingSimpleCPU::read(Addr addr, int32_t &data, unsigned flags) >>> @@ -555,6 +554,10 @@ >>> Addr split_addr = roundDown(addr + data_size - 1, block_size); >>> assert(split_addr <= addr || split_addr - addr < block_size); >>> >>> + // This will need a new way to tell if it's hooked up to a cache or >>> not. >>> + if (req->isUncacheable()) >>> + recordEvent("Uncached Write"); >>> + >>> T *dataP = new T; >>> *dataP = TheISA::htog(data); >>> _status = DTBWaitResponse; >>> @@ -586,10 +589,6 @@ >>> traceData->setData(data); >>> } >>> >>> - // This will need a new way to tell if it's hooked up to a cache or >>> not. >>> - if (req->isUncacheable()) >>> - recordEvent("Uncached Write"); >>> - >>> // If the write needs to have a fault on the access, consider calling >>> // changeStatus() and changing it to "bad addr write" or something. >>> return NoFault; >>> >>> _______________________________________________ >>> m5-dev mailing list >>> [email protected] >>> http://m5sim.org/mailman/listinfo/m5-dev >>> >>> >> _______________________________________________ >> m5-dev mailing list >> [email protected] >> http://m5sim.org/mailman/listinfo/m5-dev >> >> >> _______________________________________________ >> m5-dev mailing list >> [email protected] >> http://m5sim.org/mailman/listinfo/m5-dev >> >> > _______________________________________________ > m5-dev mailing list > [email protected] > http://m5sim.org/mailman/listinfo/m5-dev > _______________________________________________ m5-dev mailing list [email protected] http://m5sim.org/mailman/listinfo/m5-dev
