----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://reviews.m5sim.org/r/396/#review649 -----------------------------------------------------------
The code seems ok, but why do we need to have multiple outstanding page walks in timing mode again? src/arch/x86/pagetable_walker.hh <http://reviews.m5sim.org/r/396/#comment883> Why call this reqType instead of leaving it as mode? requests have types which are orthogonal to this, and it's called mode everywhere else. src/arch/x86/pagetable_walker.cc <http://reviews.m5sim.org/r/396/#comment880> These should use FastAlloc if at all possible since they're on a critical path and the heap is slow. src/arch/x86/pagetable_walker.cc <http://reviews.m5sim.org/r/396/#comment877> Memory leak. src/arch/x86/pagetable_walker.cc <http://reviews.m5sim.org/r/396/#comment879> Is letting translations pass each other realistic? I worry we're making our walker artificially powerful. These loops will also slow things down potentially. src/arch/x86/pagetable_walker.cc <http://reviews.m5sim.org/r/396/#comment881> Declare this where it's used. src/arch/x86/pagetable_walker.cc <http://reviews.m5sim.org/r/396/#comment882> Why is this pulled out into its own switch statement? That will slow down the code and makes things more complicated. - Gabe On 2011-01-06 16:12:34, Brad Beckmann wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://reviews.m5sim.org/r/396/ > ----------------------------------------------------------- > > (Updated 2011-01-06 16:12:34) > > > Review request for Default, Ali Saidi, Gabe Black, Steve Reinhardt, and > Nathan Binkert. > > > Summary > ------- > > x86: Timing support for pagetable walker > > Move page table walker state to its own object type, and make the > walker instantiate state for each outstanding walk. By storing the > states in a queue, the walker is able to handle multiple outstanding > timing requests. Note that functional walks use separate state > elements. > > > Diffs > ----- > > src/arch/x86/pagetable_walker.hh 9f9e10967912 > src/arch/x86/pagetable_walker.cc 9f9e10967912 > src/arch/x86/tlb.hh 9f9e10967912 > src/arch/x86/tlb.cc 9f9e10967912 > > Diff: http://reviews.m5sim.org/r/396/diff > > > Testing > ------- > > > Thanks, > > Brad > >
_______________________________________________ m5-dev mailing list m5-dev@m5sim.org http://m5sim.org/mailman/listinfo/m5-dev
