> On Aug. 14, 2014, 9:28 a.m., Andreas Sandberg wrote: > > src/mem/multi_level_page_table.hh, line 57 > > <http://reviews.gem5.org/r/2312/diff/2/?file=40429#file40429line57> > > > > In general, I prefer having a abstract base classes for interfaces > > since that makes documentation and compile-time testing easier and the code > > becomes more self-documenting. I don't think the performance impact in this > > case would be noticeable. > > > > Could you either refactor the code so that the ISA code inherits from > > MultiLevelPageTable instead of using the template? (Or at the very least > > document the ISAOps interface.) > > Alexandru Dutu wrote: > I have issues with the header files and cyclic dependencies. We'll see if > I can get a version with inheritance.
I have tried to refactor the page table code to have an abstract base, and remembered trying this in the past. It seems that this variant requires a lot more code to be changed because of the introduced cyclic dependencies (page_table.hh -> tlb.hh -> arch/x86/pagetable.hh -> multi_level_page_table.hh). TLBEntry and PageTableEntry have to be move from arch/x86/pagetable.hh etc. In the end there are still thousands of lines of unresolved BitUnion64 and TheISA because of the way headers are included in sim/process.cc. - Alexandru ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://reviews.gem5.org/r/2312/#review5245 ----------------------------------------------------------- On Aug. 25, 2014, 9:08 p.m., Alexandru Dutu wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://reviews.gem5.org/r/2312/ > ----------------------------------------------------------- > > (Updated Aug. 25, 2014, 9:08 p.m.) > > > Review request for Default. > > > Repository: gem5 > > > Description > ------- > > Changeset 10264:5cf3d07a2e8b > --------------------------- > Mem: adding a multi-level page table class > This patch defines a multi-level page table class that stores the page table > in > system memory, consistent with ISA specifications. In this way, cpu models > that > use the actual hardware to execute (e.g. KvmCPU), are able to traverse the > page > table. > > > Diffs > ----- > > src/mem/multi_level_page_table.hh PRE-CREATION > src/mem/multi_level_page_table.cc PRE-CREATION > src/mem/multi_level_page_table_impl.hh PRE-CREATION > src/mem/page_table.hh c00b5ba43967e7e48a28b7ddc48c9f4afaf2ab76 > src/mem/page_table.cc c00b5ba43967e7e48a28b7ddc48c9f4afaf2ab76 > src/mem/se_translating_port_proxy.hh > c00b5ba43967e7e48a28b7ddc48c9f4afaf2ab76 > src/sim/process.hh c00b5ba43967e7e48a28b7ddc48c9f4afaf2ab76 > src/sim/process.cc c00b5ba43967e7e48a28b7ddc48c9f4afaf2ab76 > > Diff: http://reviews.gem5.org/r/2312/diff/ > > > Testing > ------- > > Regressions passed. > > > Thanks, > > Alexandru Dutu > > _______________________________________________ gem5-dev mailing list gem5-dev@gem5.org http://m5sim.org/mailman/listinfo/gem5-dev
