----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://reviews.m5sim.org/r/385/#review642 -----------------------------------------------------------
I think you forgot some files so this I suppose this is only a partial review. It looks like this could be cleanly split into three different changes, and the fact that you have sub-commit messages for those independent parts suggests that you already knew that. It's best to split it up and commit it as separate changes. src/arch/x86/vtophys.cc <http://reviews.m5sim.org/r/385/#comment863> Better wording might be "Need access to page tables." src/arch/x86/vtophys.cc <http://reviews.m5sim.org/r/385/#comment865> Having a temporary variable here seems unnecessary unless it's to prevent having to wrap the next line. It's not a big deal, though. src/arch/x86/vtophys.cc <http://reviews.m5sim.org/r/385/#comment864> This is very suspicious. The request size was set to 0 when you constructed the request object, so this is anding the original address with -1. That doesn't do anything, so you're really just oring the addresses together. The TLB will already have taken care of any page offset/page number munging that you need. Actually, this whole function is suspect (not because of your code) since there's no guarantee code/data and/or different forms of data will be translated the same, or that flags aren't important. - Gabe On 2011-01-06 15:59:24, Brad Beckmann wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://reviews.m5sim.org/r/385/ > ----------------------------------------------------------- > > (Updated 2011-01-06 15:59:24) > > > Review request for Default, Ali Saidi, Gabe Black, Steve Reinhardt, and > Nathan Binkert. > > > Summary > ------- > > x86: page table walker functional support > > src/arch/x86/pagetable_walker.hh: Added method to functionally walk page table > src/arch/x86/pagetable_walker.cc: Added method to functionally walk page table > src/arch/x86/tlb.cc: Added method to return pointer to walker > src/arch/x86/tlb.hh: Added method to return pointer to walker > src/arch/x86/vtophys.cc: Calls walker to look up virt. to phys. page mapping > > > Diffs > ----- > > src/arch/x86/vtophys.cc 9f9e10967912 > > Diff: http://reviews.m5sim.org/r/385/diff > > > Testing > ------- > > > Thanks, > > Brad > >
_______________________________________________ m5-dev mailing list m5-dev@m5sim.org http://m5sim.org/mailman/listinfo/m5-dev
