From c9ad4a15d6b1460ea2b9c1515739f56f81ea9b57 Mon Sep 17 00:00:00 2001 From: Kevin Lim Date: Tue, 23 May 2006 16:59:13 -0400 Subject: [PATCH] Cleanup checker. cpu/checker/cpu.cc: Cleanup checker, give more useful warning messages. Also fix bug cpu/checker/cpu.hh: Cleanup checker, use forward declaration instead of include. --HG-- extra : convert_revision : 8f231199a0a75788218320cdbcc7f70441e5d574 --- cpu/checker/cpu.cc | 321 +++++++++++++++------------------------------ cpu/checker/cpu.hh | 16 +-- 2 files changed, 111 insertions(+), 226 deletions(-) diff --git a/cpu/checker/cpu.cc b/cpu/checker/cpu.cc index f76f1e063..08ab5d5c8 100644 --- a/cpu/checker/cpu.cc +++ b/cpu/checker/cpu.cc @@ -1,5 +1,5 @@ /* - * Copyright (c) 2002-2005 The Regents of The University of Michigan + * Copyright (c) 2006 The Regents of The University of Michigan * All rights reserved. * * Redistribution and use in source and binary forms, with or without @@ -26,41 +26,17 @@ * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. */ -//#include -#include -//#include -#include -#include #include -//#include #include -//#include "base/cprintf.hh" -//#include "base/inifile.hh" -//#include "base/loader/symtab.hh" -#include "base/misc.hh" -//#include "base/pollevent.hh" -//#include "base/range.hh" #include "base/refcnt.hh" -//#include "base/stats/events.hh" #include "cpu/base.hh" #include "cpu/base_dyn_inst.hh" #include "cpu/checker/cpu.hh" #include "cpu/cpu_exec_context.hh" #include "cpu/exec_context.hh" -//#include "cpu/exetrace.hh" -//#include "cpu/profile.hh" -#include "cpu/sampler/sampler.hh" -//#include "cpu/smt.hh" #include "cpu/static_inst.hh" -//#include "kern/kernel_stats.hh" -#include "mem/base_mem.hh" -#include "mem/mem_interface.hh" #include "sim/byteswap.hh" -#include "sim/builder.hh" -//#include "sim/debug.hh" -//#include "sim/host.hh" -//#include "sim/sim_events.hh" #include "sim/sim_object.hh" #include "sim/stats.hh" @@ -72,15 +48,8 @@ #include "cpu/ozone/simple_impl.hh" #if FULL_SYSTEM -#include "base/remote_gdb.hh" -#include "mem/functional/memory_control.hh" -#include "mem/functional/physical.hh" #include "sim/system.hh" -#include "arch/tlb.hh" -#include "arch/stacktrace.hh" #include "arch/vtophys.hh" -#else // !FULL_SYSTEM -#include "mem/functional/functional.hh" #endif // FULL_SYSTEM using namespace std; @@ -90,17 +59,6 @@ using namespace AlphaISA; void CheckerCPU::init() { -/* - BaseCPU::init(); -#if FULL_SYSTEM - for (int i = 0; i < execContexts.size(); ++i) { - ExecContext *xc = execContexts[i]; - - // initialize CPU, including PC - TheISA::initCPU(xc, xc->readCpuId()); - } -#endif -*/ } CheckerCPU::CheckerCPU(Params *p) @@ -151,6 +109,8 @@ CheckerCPU::setMemory(FunctionalMemory *mem) xcProxy = cpuXC->getProxy(); execContexts.push_back(xcProxy); memReq->xc = xcProxy; + delete cpuXC->kernelStats; + cpuXC->kernelStats = NULL; } #endif } @@ -168,6 +128,8 @@ CheckerCPU::setSystem(System *system) xcProxy = cpuXC->getProxy(); execContexts.push_back(xcProxy); memReq->xc = xcProxy; + delete cpuXC->kernelStats; + cpuXC->kernelStats = NULL; } } #endif @@ -197,82 +159,15 @@ CheckerCPU::unserialize(Checkpoint *cp, const string §ion) Fault CheckerCPU::copySrcTranslate(Addr src) { - static bool no_warn = true; - int blk_size = 64; - // Only support block sizes of 64 atm. - assert(blk_size == 64); - int offset = src & (blk_size - 1); - - // Make sure block doesn't span page - if (no_warn && - (src & PageMask) != ((src + blk_size) & PageMask) && - (src >> 40) != 0xfffffc) { - warn("Copied block source spans pages %x.", src); - no_warn = false; - } - - memReq->reset(src & ~(blk_size - 1), blk_size); - - // translate to physical address - Fault fault = cpuXC->translateDataReadReq(memReq); - - if (fault == NoFault) { - cpuXC->copySrcAddr = src; - cpuXC->copySrcPhysAddr = memReq->paddr + offset; - } else { - assert(!fault->isAlignmentFault()); - - cpuXC->copySrcAddr = 0; - cpuXC->copySrcPhysAddr = 0; - } - return fault; + panic("Unimplemented!"); } Fault CheckerCPU::copy(Addr dest) { - static bool no_warn = true; - int blk_size = 64; - // Only support block sizes of 64 atm. - assert(blk_size == 64); - uint8_t data[blk_size]; - //assert(cpuXC->copySrcAddr); - int offset = dest & (blk_size - 1); - - // Make sure block doesn't span page - if (no_warn && - (dest & PageMask) != ((dest + blk_size) & PageMask) && - (dest >> 40) != 0xfffffc) { - no_warn = false; - warn("Copied block destination spans pages %x. ", dest); - } - - memReq->reset(dest & ~(blk_size -1), blk_size); - // translate to physical address - Fault fault = cpuXC->translateDataWriteReq(memReq); - - if (fault == NoFault) { - Addr dest_addr = memReq->paddr + offset; - // Need to read straight from memory since we have more than 8 bytes. - memReq->paddr = cpuXC->copySrcPhysAddr; - cpuXC->mem->read(memReq, data); - memReq->paddr = dest_addr; - cpuXC->mem->write(memReq, data); - memReq->cmd = Copy; - memReq->completionEvent = NULL; - memReq->paddr = cpuXC->copySrcPhysAddr; - memReq->dest = dest_addr; - memReq->size = 64; - memReq->time = curTick; - memReq->flags &= ~INST_READ; - } - else - assert(!fault->isAlignmentFault()); - - return fault; + panic("Unimplemented!"); } -// precise architected memory state accessor macros template Fault CheckerCPU::read(Addr addr, T &data, unsigned flags) @@ -280,17 +175,15 @@ CheckerCPU::read(Addr addr, T &data, unsigned flags) memReq->reset(addr, sizeof(T), flags); // translate to physical address - // Should I probe the DTB? Or should I just take the physical address - // and assume correct translation? translateDataReadReq(memReq); - // if we have a cache, do cache access too memReq->cmd = Read; memReq->completionEvent = NULL; memReq->time = curTick; memReq->flags &= ~INST_READ; if (!(memReq->flags & UNCACHEABLE)) { + // Access memory to see if we have the same data cpuXC->read(memReq, data); } else { // Assume the data is correct if it's an uncached access @@ -350,29 +243,34 @@ CheckerCPU::write(T data, Addr addr, unsigned flags, uint64_t *res) // translate to physical address cpuXC->translateDataWriteReq(memReq); - if ((!(unverifiedReq->flags & LOCKED) || - ((unverifiedReq->flags & LOCKED) && - unverifiedReq->result == 1)) && - !(unverifiedReq->flags & UNCACHEABLE)) { - // do functional access -// cpuXC->read(memReq, data); - - memReq->cmd = Write; -// memcpy(memReq->data,(uint8_t *)&data,memReq->size); - T inst_data; - memcpy(&inst_data, unverifiedReq->data, sizeof(T)); + // Can compare the write data and result only if it's cacheable, + // not a store conditional, or is a store conditional that + // succeeded. + // @todo: Verify that actual memory matches up with these values. + // Right now it only verifies that the instruction data is the + // same as what was in the request that got sent to memory; there + // is no verification that it is the same as what is in memory. + // This is because the LSQ would have to be snooped in the CPU to + // verify this data. + if (unverifiedReq && + !(unverifiedReq->flags & UNCACHEABLE) && + (!(unverifiedReq->flags & LOCKED) || + ((unverifiedReq->flags & LOCKED) && + unverifiedReq->result == 1))) { +#if 0 + memReq->cmd = Read; memReq->completionEvent = NULL; memReq->time = curTick; memReq->flags &= ~INST_READ; + cpuXC->read(memReq, inst_data); +#endif + T inst_data; + memcpy(&inst_data, unverifiedReq->data, sizeof(T)); - // Hard to verify this as the data writes back after the - // instruction commits. May only be able to check that the - // value produced from execute() matches the value produced - // from the instruction's first execution. if (data != inst_data) { - warn("Store value does not match value in memory! " + warn("%lli: Store value does not match value in memory! " "Instruction: %#x, memory: %#x", - inst_data, data); + curTick, inst_data, data); handleError(); } } @@ -436,19 +334,6 @@ CheckerCPU::dbg_vtophys(Addr addr) } #endif // FULL_SYSTEM -#if FULL_SYSTEM -void -CheckerCPU::post_interrupt(int int_num, int index) -{ - BaseCPU::post_interrupt(int_num, index); - - if (cpuXC->status() == ExecContext::Suspended) { - DPRINTF(IPI,"Suspended Processor awoke\n"); - cpuXC->activate(); - } -} -#endif // FULL_SYSTEM - bool CheckerCPU::translateInstReq(MemReqPtr &req) { @@ -466,15 +351,16 @@ CheckerCPU::translateDataReadReq(MemReqPtr &req) cpuXC->translateDataReadReq(req); if (req->vaddr != unverifiedReq->vaddr) { - warn("Request virtual addresses do not match! Inst: %#x, checker:" - " %#x", - unverifiedReq->vaddr, req->vaddr); + warn("%lli: Request virtual addresses do not match! Inst: %#x, " + "checker: %#x", + curTick, unverifiedReq->vaddr, req->vaddr); + handleError(); } req->paddr = unverifiedReq->paddr; if (checkFlags(req)) { - warn("Request flags do not match! Inst: %#x, checker: %#x", - unverifiedReq->flags, req->flags); + warn("%lli: Request flags do not match! Inst: %#x, checker: %#x", + curTick, unverifiedReq->flags, req->flags); handleError(); } } @@ -485,15 +371,16 @@ CheckerCPU::translateDataWriteReq(MemReqPtr &req) cpuXC->translateDataWriteReq(req); if (req->vaddr != unverifiedReq->vaddr) { - warn("Request virtual addresses do not match! Inst: %#x, checker:" - " %#x", - unverifiedReq->vaddr, req->vaddr); + warn("%lli: Request virtual addresses do not match! Inst: %#x, " + "checker: %#x", + curTick, unverifiedReq->vaddr, req->vaddr); + handleError(); } req->paddr = unverifiedReq->paddr; if (checkFlags(req)) { - warn("Request flags do not match! Inst: %#x, checker: %#x", - unverifiedReq->flags, req->flags); + warn("%lli: Request flags do not match! Inst: %#x, checker: %#x", + curTick, unverifiedReq->flags, req->flags); handleError(); } } @@ -512,13 +399,17 @@ CheckerCPU::checkFlags(MemReqPtr &req) } } -/* start simulation, program loaded, processor precise state initialized */ template void Checker::tick(DynInstPtr &completed_inst) { DynInstPtr inst; + // Either check this instruction, or add it to a list of + // instructions waiting to be checked. Instructions must be + // checked in program order, so if a store has committed yet not + // completed, there may be some instructions that are waiting + // behind it that have completed and must be checked. if (!instList.empty()) { if (youngestSN < completed_inst->seqNum) { DPRINTF(Checker, "Adding instruction [sn:%lli] PC:%#x to list.\n", @@ -547,16 +438,17 @@ Checker::tick(DynInstPtr &completed_inst) inst = completed_inst; youngestSN = completed_inst->seqNum; } else { -// panic("SN already seen yet the list is empty!"); return; } } } + // Try to check all instructions that are completed, ending if we + // run out of instructions to check or if an instruction is not + // yet completed. while (1) { DPRINTF(Checker, "Processing instruction [sn:%lli] PC:%#x.\n", inst->seqNum, inst->readPC()); -// verifyInst = completed_inst; unverifiedResult.integer = inst->readIntResult(); unverifiedReq = inst->req; numCycles++; @@ -569,15 +461,9 @@ Checker::tick(DynInstPtr &completed_inst) cpuXC->setFloatRegDouble(ZeroReg, 0.0); #endif // TARGET_ALPHA - // Try to fetch an instruction - - // set up memory request for instruction fetch -#if FULL_SYSTEM -#define IFETCH_FLAGS(pc) ((pc) & 1) ? PHYSICAL : 0 -#else -#define IFETCH_FLAGS(pc) 0 -#endif - + // Check if any recent PC changes match up with anything we + // expect to happen. This is mostly to check if traps or + // PC-based events have occurred in both the checker and CPU. if (changedPC) { DPRINTF(Checker, "Changed PC recently to %#x\n", cpuXC->readPC()); @@ -585,9 +471,9 @@ Checker::tick(DynInstPtr &completed_inst) if (newPC == cpuXC->readPC()) { DPRINTF(Checker, "Changed PC matches expected PC\n"); } else { - warn("Changed PC does not match expected PC, changed: %#x, " - "expected: %#x", - cpuXC->readPC(), newPC); + warn("%lli: Changed PC does not match expected PC, " + "changed: %#x, expected: %#x", + curTick, cpuXC->readPC(), newPC); handleError(); } willChangePC = false; @@ -600,6 +486,15 @@ Checker::tick(DynInstPtr &completed_inst) changedNextPC = false; } + // Try to fetch the instruction + +#if FULL_SYSTEM +#define IFETCH_FLAGS(pc) ((pc) & 1) ? PHYSICAL : 0 +#else +#define IFETCH_FLAGS(pc) 0 +#endif + + // set up memory request for instruction fetch memReq->cmd = Read; memReq->reset(cpuXC->readPC() & ~3, sizeof(uint32_t), IFETCH_FLAGS(cpuXC->readPC())); @@ -608,8 +503,13 @@ Checker::tick(DynInstPtr &completed_inst) if (!succeeded) { if (inst->getFault() == NoFault) { - warn("Instruction PC %#x was not found in the ITB!", - cpuXC->readPC()); + // In this case the instruction was not a dummy + // instruction carrying an ITB fault. In the single + // threaded case the ITB should still be able to + // translate this instruction; in the SMT case it's + // possible that its ITB entry was kicked out. + warn("%lli: Instruction PC %#x was not found in the ITB!", + curTick, cpuXC->readPC()); handleError(); // go to the next instruction @@ -618,20 +518,18 @@ Checker::tick(DynInstPtr &completed_inst) return; } else { + // The instruction is carrying an ITB fault. Handle + // the fault and see if our results match the CPU on + // the next tick(). fault = inst->getFault(); } } if (fault == NoFault) { -// fault = cpuXC->mem->read(memReq, machInst); cpuXC->mem->read(memReq, machInst); - // If we've got a valid instruction (i.e., no fault on instruction - // fetch), then execute it. - - // keep an instruction count + // keep an instruction count numInst++; -// numInsts++; // decode the instruction machInst = gtoh(machInst); @@ -639,7 +537,8 @@ Checker::tick(DynInstPtr &completed_inst) // Checks both the machine instruction and the PC. validateInst(inst); - curStaticInst = StaticInst::decode(makeExtMI(machInst, cpuXC->readPC())); + curStaticInst = StaticInst::decode(makeExtMI(machInst, + cpuXC->readPC())); #if FULL_SYSTEM cpuXC->setInst(machInst); @@ -660,10 +559,6 @@ Checker::tick(DynInstPtr &completed_inst) // Checks to make sure instrution results are correct. validateExecution(inst); -// if (curStaticInst->isMemRef()) { -// numMemRefs++; -// } - if (curStaticInst->isLoad()) { ++numLoad; } @@ -693,6 +588,9 @@ Checker::tick(DynInstPtr &completed_inst) } #if FULL_SYSTEM + // @todo: Determine if these should happen only if the + // instruction hasn't faulted. In the SimpleCPU case this may + // not be true, but in the O3 or Ozone case this may be true. Addr oldpc; int count = 0; do { @@ -707,10 +605,12 @@ Checker::tick(DynInstPtr &completed_inst) } #endif - // Checks PC, next PC. Optionally can check all registers. (Or just those + // @todo: Optionally can check all registers. (Or just those // that have been modified). validateState(); + // Continue verifying instructions if there's another completed + // instruction waiting to be verified. if (instList.empty()) { break; } else if (instList.front()->isCompleted()) { @@ -726,7 +626,6 @@ template void Checker::switchOut(Sampler *s) { - sampler = s; instList.clear(); } @@ -734,15 +633,6 @@ template void Checker::takeOverFrom(BaseCPU *oldCPU) { -// BaseCPU::takeOverFrom(oldCPU); - - // if any of this CPU's ExecContexts are active, mark the CPU as - // running and schedule its tick event. -/* - for (int i = 0; i < execContexts.size(); ++i) { - ExecContext *xc = execContexts[i]; - } -*/ } template @@ -750,20 +640,22 @@ void Checker::validateInst(DynInstPtr &inst) { if (inst->readPC() != cpuXC->readPC()) { - warn("PCs do not match! Inst: %#x, checker: %#x", - inst->readPC(), cpuXC->readPC()); + warn("%lli: PCs do not match! Inst: %#x, checker: %#x", + curTick, inst->readPC(), cpuXC->readPC()); if (changedPC) { - warn("Changed PCs recently, may not be an error"); + warn("%lli: Changed PCs recently, may not be an error", + curTick); } else { handleError(); } } - if (static_cast(inst->staticInst->machInst) != - machInst) { - warn("Binary instructions do not match! Inst: %#x, checker: %#x", - static_cast(inst->staticInst->machInst), - machInst); + MachInst mi = static_cast(inst->staticInst->machInst); + + if (mi != machInst) { + warn("%lli: Binary instructions do not match! Inst: %#x, " + "checker: %#x", + curTick, mi, machInst); handleError(); } } @@ -773,10 +665,11 @@ void Checker::validateExecution(DynInstPtr &inst) { if (inst->numDestRegs()) { + // @todo: Support more destination registers. if (inst->isUnverifiable()) { - // @todo: Support more destination registers. - // Grab the result from the instruction and write it to the - // register. + // Unverifiable instructions assume they were executed + // properly by the CPU. Grab the result from the + // instruction and write it to the register. RegIndex idx = inst->destRegIdx(0); if (idx < TheISA::FP_Base_DepTag) { cpuXC->setIntReg(idx, inst->readIntResult()); @@ -786,16 +679,17 @@ Checker::validateExecution(DynInstPtr &inst) cpuXC->setMiscReg(idx, inst->readIntResult()); } } else if (result.integer != inst->readIntResult()) { - warn("Instruction results do not match! (May not be integer results) " - "Inst: %#x, checker: %#x", - inst->readIntResult(), result.integer); + warn("%lli: Instruction results do not match! (Results may not " + "actually be integers) Inst: %#x, checker: %#x", + curTick, inst->readIntResult(), result.integer); handleError(); } } if (inst->readNextPC() != cpuXC->readNextPC()) { - warn("Instruction next PCs do not match! Inst: %#x, checker: %#x", - inst->readNextPC(), cpuXC->readNextPC()); + warn("%lli: Instruction next PCs do not match! Inst: %#x, " + "checker: %#x", + curTick, inst->readNextPC(), cpuXC->readNextPC()); handleError(); } @@ -810,9 +704,10 @@ Checker::validateExecution(DynInstPtr &inst) if (inst->xcBase()->readMiscReg(misc_reg_idx) != cpuXC->readMiscReg(misc_reg_idx)) { - warn("Misc reg idx %i (side effect) does not match! Inst: %#x, " - "checker: %#x", - misc_reg_idx, inst->xcBase()->readMiscReg(misc_reg_idx), + warn("%lli: Misc reg idx %i (side effect) does not match! " + "Inst: %#x, checker: %#x", + curTick, misc_reg_idx, + inst->xcBase()->readMiscReg(misc_reg_idx), cpuXC->readMiscReg(misc_reg_idx)); handleError(); } diff --git a/cpu/checker/cpu.hh b/cpu/checker/cpu.hh index 678e888df..37fe59d95 100644 --- a/cpu/checker/cpu.hh +++ b/cpu/checker/cpu.hh @@ -1,5 +1,5 @@ /* - * Copyright (c) 2002-2005 The Regents of The University of Michigan + * Copyright (c) 2006 The Regents of The University of Michigan * All rights reserved. * * Redistribution and use in source and binary forms, with or without @@ -39,7 +39,6 @@ #include "cpu/base_dyn_inst.hh" #include "cpu/cpu_exec_context.hh" #include "cpu/pc_event.hh" -#include "cpu/sampler/sampler.hh" #include "cpu/static_inst.hh" #include "sim/eventq.hh" @@ -63,6 +62,7 @@ class BaseDynInst; class ExecContext; class MemInterface; class Checkpoint; +class Sampler; class CheckerCPU : public BaseCPU { @@ -86,8 +86,6 @@ class CheckerCPU : public BaseCPU }; public: - void post_interrupt(int int_num, int index); - CheckerCPU(Params *p); virtual ~CheckerCPU(); @@ -111,8 +109,6 @@ class CheckerCPU : public BaseCPU #if FULL_SYSTEM Addr dbg_vtophys(Addr addr); - - bool interval_stats; #endif union Result { @@ -129,11 +125,6 @@ class CheckerCPU : public BaseCPU // Refcounted pointer to the one memory request. MemReqPtr memReq; - // Pointer to the sampler that is telling us to switchover. - // Used to signal the completion of the pipe drain and schedule - // the next switchover - Sampler *sampler; - StaticInstPtr curStaticInst; // number of simulated instructions @@ -284,6 +275,7 @@ class CheckerCPU : public BaseCPU bool simPalCheck(int palFunc) { return cpuXC->simPalCheck(palFunc); } #else // Assume that the normal CPU's call to syscall was successful. + // The checker's state would have already been updated by the syscall. void syscall() { } #endif @@ -307,8 +299,6 @@ class CheckerCPU : public BaseCPU bool exitOnError; InstSeqNum youngestSN; -// std::map storeBuff; -// typedef std::map::iterator map_it; }; template -- 2.30.2