> On July 14, 2014, 4:42 p.m., Andreas Sandberg wrote: > > I have two high-level comments that need to be fixed: > > > > * This is should be four different commits (configuration, CPUID updates, > > segment initialization, kvm, process). > > * Write a proper commit message (see http://www.m5sim.org/Commit_Access). > > Specifically, include a short one-line summary and a longer description of > > what the patch does and why. > > > > I'm also a bit unsure about the design of this patch. From RB2312, it > > /seems/ like the useArchPT flag is supposed to enable page tables in SE > > emulation mode, however in this patch you seem to overload it to mean "use > > magic syscall redirection". Could you clean that up a bit? > > > > Could you redesign the syscall & fault handler to use the memory mapped > > m5ops interface instead? That way, the code would in the kvm CPU would be > > much cleaner and you'd get rid of the magic ports. As a side effect, we > > could potentially scrap the SE-specific bits of the ISA (this is definitely > > something I'd like to see long term). > >
Thank you for your review! If I break this in four different commits, the simulator might not compile after each commit just after all of them are applied. That does not sound acceptable to me. > On July 14, 2014, 4:42 p.m., Andreas Sandberg wrote: > > src/arch/x86/system.hh, line 95 > > <http://reviews.gem5.org/r/2313/diff/1/?file=40380#file40380line95> > > > > Shouldn't this be in process.hh since it is SE specific? These get used in places where it does not make sense to include process.hh and it makes sense to include system.hh (i.e. mem/ and cpu/). > On July 14, 2014, 4:42 p.m., Andreas Sandberg wrote: > > src/arch/x86/system.hh, line 101 > > <http://reviews.gem5.org/r/2313/diff/1/?file=40380#file40380line101> > > > > SE-specific? Same as above. > On July 14, 2014, 4:42 p.m., Andreas Sandberg wrote: > > src/cpu/kvm/x86_cpu.cc, line 1409 > > <http://reviews.gem5.org/r/2313/diff/1/?file=40385#file40385line1409> > > > > Is this really needed here? Actually, this is not needed anymore. I went through 3 modes of exit out of KVM for syscall emulation and entering to continue execution. This got left here from the previous 2 modes. > On July 14, 2014, 4:42 p.m., Andreas Sandberg wrote: > > configs/example/se.py, line 66 > > <http://reviews.gem5.org/r/2313/diff/1/?file=40376#file40376line66> > > > > You need to make more checks here as this patch only adds SE support(?) > > for X86. This only checks whether KvmCPU is in use, I will add more checks to line 200. - Alexandru ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://reviews.gem5.org/r/2313/#review5187 ----------------------------------------------------------- On July 11, 2014, 4:01 p.m., Alexandru Dutu wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://reviews.gem5.org/r/2313/ > ----------------------------------------------------------- > > (Updated July 11, 2014, 4:01 p.m.) > > > Review request for Default. > > > Repository: gem5 > > > Description > ------- > > Changeset 10254:661c482f7d58 > --------------------------- > Making KvmCPU model usable in SE mode. > > > Diffs > ----- > > configs/example/se.py c625a3c51bac050879457e666dd83299a36d761b > src/arch/x86/cpuid.cc c625a3c51bac050879457e666dd83299a36d761b > src/arch/x86/process.cc c625a3c51bac050879457e666dd83299a36d761b > src/arch/x86/regs/misc.hh c625a3c51bac050879457e666dd83299a36d761b > src/arch/x86/system.hh c625a3c51bac050879457e666dd83299a36d761b > src/arch/x86/system.cc c625a3c51bac050879457e666dd83299a36d761b > src/cpu/kvm/base.hh c625a3c51bac050879457e666dd83299a36d761b > src/cpu/kvm/base.cc c625a3c51bac050879457e666dd83299a36d761b > src/cpu/kvm/x86_cpu.hh c625a3c51bac050879457e666dd83299a36d761b > src/cpu/kvm/x86_cpu.cc c625a3c51bac050879457e666dd83299a36d761b > > Diff: http://reviews.gem5.org/r/2313/diff/ > > > Testing > ------- > > Quick regressions passed. > > > Thanks, > > Alexandru Dutu > > _______________________________________________ gem5-dev mailing list [email protected] http://m5sim.org/mailman/listinfo/gem5-dev
