----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://reviews.m5sim.org/r/321/#review500 -----------------------------------------------------------
src/cpu/base_dyn_inst.hh <http://reviews.m5sim.org/r/321/#comment722> Please change to "Is the physical address valid (translation was successful)?" src/cpu/base_dyn_inst.hh <http://reviews.m5sim.org/r/321/#comment723> Please change to "Is address translation finished, faulting or not?" src/cpu/base_dyn_inst.hh <http://reviews.m5sim.org/r/321/#comment724> data may not need to be on the next line. It might need to be if the lines above are juuuust on the very edge. src/cpu/base_dyn_inst.hh <http://reviews.m5sim.org/r/321/#comment725> MachineCheckFaults aren't for uncached loads, they're for accesses that fall off the end of the world and cause a machine check. To the best of my knowledge these won't be returned immediately since there would be a delay getting to them through the memory system, but something in the middle might use the fault to signal a strange error. src/cpu/base_dyn_inst_impl.hh <http://reviews.m5sim.org/r/321/#comment726> A new copyright might not be justified here. src/cpu/o3/lsq_unit.hh <http://reviews.m5sim.org/r/321/#comment727> Copyright probably not justified again. src/cpu/o3/lsq_unit_impl.hh <http://reviews.m5sim.org/r/321/#comment728> I don't think this is true. A fault could have happened by this point, especially when there isn't a table walker. initiateAcc itself could fault without even performing an access if, for instance, it was a privileged instruction executed in a non-privileged mode. src/cpu/o3/lsq_unit_impl.hh <http://reviews.m5sim.org/r/321/#comment729> Why is this code being removed? src/cpu/o3/lsq_unit_impl.hh <http://reviews.m5sim.org/r/321/#comment731> I don't totally follow what's going on here. Why are you putting these in a new structure instead of checking them in place? This might be fine, but I don't understand what's going on so I'm not sure if there's a flaw in the logic. One thing I'd be especially careful of here are translations finishing out of order and breaking the ordering checks. src/cpu/o3/lsq_unit_impl.hh <http://reviews.m5sim.org/r/321/#comment730> The else and the if shouldn't be on two different lines. src/cpu/o3/lsq_unit_impl.hh <http://reviews.m5sim.org/r/321/#comment732> Shouldn't be on a new line. - Gabe On 2010-11-19 16:13:39, Ali Saidi wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://reviews.m5sim.org/r/321/ > ----------------------------------------------------------- > > (Updated 2010-11-19 16:13:39) > > > Review request for Default. > > > Summary > ------- > > O3: Make the DTLB translateTiming() a split transaction. > ISAs that have hardware page table walkers need this. For those ISAs, > initiateTranslation can return with NoFault, but with the translation > unfinished. > > In the BaseDynInst::read() function, actual memory(cache) access calling > cpu->read() used to occur right after initiateTranslation() function call. > This > access has been moved to the finishTranslation() so that it occurs when the > translation finishes, either by TLB hit or TLB Miss followed by hardware > page-walk. Faults occured during translation is detected in > finishTranslation() > and handled during the IEW stage tick()->handleTranslationFault(). > > This is a work in progress, but I want to make sure there aren't any issues... > > > Diffs > ----- > > src/arch/arm/tlb.cc 6286bb50127e > src/cpu/base_dyn_inst.hh 6286bb50127e > src/cpu/base_dyn_inst_impl.hh 6286bb50127e > src/cpu/o3/commit_impl.hh 6286bb50127e > src/cpu/o3/iew_impl.hh 6286bb50127e > src/cpu/o3/lsq.hh 6286bb50127e > src/cpu/o3/lsq_impl.hh 6286bb50127e > src/cpu/o3/lsq_unit.hh 6286bb50127e > src/cpu/o3/lsq_unit_impl.hh 6286bb50127e > > Diff: http://reviews.m5sim.org/r/321/diff > > > Testing > ------- > > > Thanks, > > Ali > >
_______________________________________________ m5-dev mailing list [email protected] http://m5sim.org/mailman/listinfo/m5-dev
