> On 2010-11-21 01:19:58, Gabe Black wrote: > > src/cpu/base_dyn_inst.hh, line 229 > > <http://reviews.m5sim.org/r/321/diff/1/?file=5387#file5387line229> > > > > Please change to "Is the physical address valid (translation was > > successful)?"
fixed > On 2010-11-21 01:19:58, Gabe Black wrote: > > src/cpu/base_dyn_inst.hh, line 232 > > <http://reviews.m5sim.org/r/321/diff/1/?file=5387#file5387line232> > > > > Please change to "Is address translation finished, faulting or not?" fixed > On 2010-11-21 01:19:58, Gabe Black wrote: > > src/cpu/base_dyn_inst.hh, line 973 > > <http://reviews.m5sim.org/r/321/diff/1/?file=5387#file5387line973> > > > > 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. fixed > On 2010-11-21 01:19:58, Gabe Black wrote: > > src/cpu/base_dyn_inst.hh, line 1025 > > <http://reviews.m5sim.org/r/321/diff/1/?file=5387#file5387line1025> > > > > 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. It's used to make sure that the instruction doesn't execute early, so yes it is in this case. See lsq_unit.hh:561 and blame Kevin if you don't like it. > On 2010-11-21 01:19:58, Gabe Black wrote: > > src/cpu/o3/lsq_unit_impl.hh, line 458 > > <http://reviews.m5sim.org/r/321/diff/1/?file=5394#file5394line458> > > > > 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. True... fixed. > On 2010-11-21 01:19:58, Gabe Black wrote: > > src/cpu/o3/lsq_unit_impl.hh, line 476 > > <http://reviews.m5sim.org/r/321/diff/1/?file=5394#file5394line476> > > > > Why is this code being removed? It's being re-located.... See the memdepordering change. Since the translation might not be done yet we can't check for a violation here. > On 2010-11-21 01:19:58, Gabe Black wrote: > > src/cpu/o3/lsq_unit_impl.hh, line 565 > > <http://reviews.m5sim.org/r/321/diff/1/?file=5394#file5394line565> > > > > 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. You asked where the dependency checking was going and here it is. > On 2010-11-21 01:19:58, Gabe Black wrote: > > src/cpu/o3/lsq_unit_impl.hh, line 582 > > <http://reviews.m5sim.org/r/321/diff/1/?file=5394#file5394line582> > > > > The else and the if shouldn't be on two different lines. fixed... > On 2010-11-21 01:19:58, Gabe Black wrote: > > src/cpu/o3/lsq_unit_impl.hh, line 627 > > <http://reviews.m5sim.org/r/321/diff/1/?file=5394#file5394line627> > > > > Shouldn't be on a new line. fixed. - Ali ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://reviews.m5sim.org/r/321/#review500 ----------------------------------------------------------- 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
