> On Sept. 27, 2016, 8:53 p.m., Andreas Hansson wrote: > > Thanks again for getting this in shape. There are a few minor questions and > > comment on the patch, the most important one is related to the copyrights. > > > > Also, I assume you have used hg copy for the files that are more or less > > 1:1 copies from other architectures?
I didn't; I used cp and then hg add. Does hg copy do anything different? > On Sept. 27, 2016, 8:53 p.m., Andreas Hansson wrote: > > src/arch/riscv/faults.hh, line 139 > > <http://reviews.gem5.org/r/3624/diff/5/?file=58091#file58091line139> > > > > Could you perhaps elaborate on why this is ok as is? Currently this only supports SE mode, which doesn't have elevated privilege (which is also why the eret instruction is not implemented). That is a reminder to me of how I intended for that fault to change when elevated privilege is implemented (or a hint to the person who does it if I don't). Is it not appropriate to include it? > On Sept. 27, 2016, 8:53 p.m., Andreas Hansson wrote: > > src/arch/riscv/isa/base.isa, line 3 > > <http://reviews.gem5.org/r/3624/diff/5/?file=58099#file58099line3> > > > > ? Is that a legal entity? Some of the files in this project were originally from a GitHub project by Austin Harris. In those cases, I left whatever copyright was originally there in addition to adding mine if I felt it was appropriate. > On Sept. 27, 2016, 8:53 p.m., Andreas Hansson wrote: > > src/arch/riscv/tlb.cc, line 3 > > <http://reviews.gem5.org/r/3624/diff/5/?file=58133#file58133line3> > > > > I am really surprise that this and a few other files do not have a Uni > > Virginia copyright. Is that intentional? Files that I copied from other ISAs, and only made minor changes like names, I didn't feel merited that addition, so I left them as they are. > On Sept. 27, 2016, 8:53 p.m., Andreas Hansson wrote: > > src/arch/riscv/pagetable.hh, line 81 > > <http://reviews.gem5.org/r/3624/diff/5/?file=58119#file58119line81> > > > > What are the implications? > > > > Also, should this class not inherint from Serializable? This is copied from MIPS, with some name changes. I had been focused on getting RISC-V running, and haven't gotten to properly implementing the TLB yet. > On Sept. 27, 2016, 8:53 p.m., Andreas Hansson wrote: > > src/arch/riscv/mmapped_ipr.hh, line 43 > > <http://reviews.gem5.org/r/3624/diff/5/?file=58118#file58118line43> > > > > RISCV has mmapped IPRs? This file was from the original GitHub project, and appears to be required by the CPU models. > On Sept. 27, 2016, 8:53 p.m., Andreas Hansson wrote: > > src/arch/riscv/tlb.cc, line 58 > > <http://reviews.gem5.org/r/3624/diff/5/?file=58133#file58133line58> > > > > Not convention See my comment for pagetable.hh. - Alec ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://reviews.gem5.org/r/3624/#review8737 ----------------------------------------------------------- On Sept. 16, 2016, 4:49 p.m., Alec Roelke wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://reviews.gem5.org/r/3624/ > ----------------------------------------------------------- > > (Updated Sept. 16, 2016, 4:49 p.m.) > > > Review request for Default. > > > Repository: gem5 > > > Description > ------- > > Changeset 11620:3da42ffa5b4b > --------------------------- > arch: [Patch 1/5] Added RISC-V base instruction set RV64I > > First of five patches adding RISC-V to GEM5. This patch introduces the > base 64-bit ISA (RV64I) in src/arch/riscv for use with syscall emulation. > The multiply, floating point, and atomic memory instructions will be added > in additional patches, as well as support for more detailed CPU models. > The loader is also modified to be able to parse RISC-V ELF files, and a > "Hello world!" example for RISC-V is added to test-progs. > > Patch 2 will implement the multiply extension, RV64M; patch 3 will implement > the floating point (single- and double-precision) extensions, RV64FD; > patch 4 will implement the atomic memory instructions, RV64A, and patch 5 > will add support for timing, minor, and detailed CPU models that is missing > from the first four patches (such as handling locked memory). > > [Removed several unused parameters and imports from RiscvInterrupts.py, > RiscvISA.py, and RiscvSystem.py.] > [Fixed copyright information in RISC-V files copied from elsewhere that had > ARM licenses attached.] > [Reorganized instruction definitions in decoder.isa so that they are sorted > by opcode in preparation for the addition of ISA extensions M, A, F, D.] > Signed-off by: Alec Roelke > > > Diffs > ----- > > src/arch/riscv/stacktrace.hh PRE-CREATION > src/arch/riscv/mmapped_ipr.hh PRE-CREATION > src/arch/riscv/pagetable.hh PRE-CREATION > src/arch/riscv/pagetable.cc PRE-CREATION > build_opts/RISCV PRE-CREATION > ext/libelf/elf_common.h 8bc53d5565ba > src/arch/riscv/RiscvISA.py PRE-CREATION > src/arch/riscv/RiscvInterrupts.py PRE-CREATION > src/arch/riscv/RiscvSystem.py PRE-CREATION > src/arch/riscv/RiscvTLB.py PRE-CREATION > src/arch/riscv/SConscript PRE-CREATION > src/arch/riscv/SConsopts PRE-CREATION > src/arch/riscv/decoder.hh PRE-CREATION > src/arch/riscv/decoder.cc PRE-CREATION > src/arch/riscv/faults.hh PRE-CREATION > src/arch/riscv/faults.cc PRE-CREATION > src/arch/riscv/idle_event.hh PRE-CREATION > src/arch/riscv/idle_event.cc PRE-CREATION > src/arch/riscv/interrupts.hh PRE-CREATION > src/arch/riscv/interrupts.cc PRE-CREATION > src/arch/riscv/isa.hh PRE-CREATION > src/arch/riscv/isa.cc PRE-CREATION > src/arch/riscv/isa/base.isa PRE-CREATION > src/arch/riscv/isa/bitfields.isa PRE-CREATION > src/arch/riscv/isa/decoder.isa PRE-CREATION > src/arch/riscv/isa/formats/basic.isa PRE-CREATION > src/arch/riscv/isa/formats/formats.isa PRE-CREATION > src/arch/riscv/isa/formats/mem.isa PRE-CREATION > src/arch/riscv/isa/formats/type.isa PRE-CREATION > src/arch/riscv/isa/formats/unknown.isa PRE-CREATION > src/arch/riscv/isa/includes.isa PRE-CREATION > src/arch/riscv/isa/main.isa PRE-CREATION > src/arch/riscv/isa/operands.isa PRE-CREATION > src/arch/riscv/isa_traits.hh PRE-CREATION > src/arch/riscv/kernel_stats.hh PRE-CREATION > src/arch/riscv/linux/linux.hh PRE-CREATION > src/arch/riscv/linux/linux.cc PRE-CREATION > src/arch/riscv/linux/process.hh PRE-CREATION > src/arch/riscv/linux/process.cc PRE-CREATION > src/arch/riscv/locked_mem.hh PRE-CREATION > src/arch/riscv/microcode_rom.hh PRE-CREATION > src/arch/riscv/pra_constants.hh PRE-CREATION > src/arch/riscv/process.hh PRE-CREATION > src/arch/riscv/process.cc PRE-CREATION > src/arch/riscv/pseudo_inst.hh PRE-CREATION > src/arch/riscv/registers.hh PRE-CREATION > src/arch/riscv/remote_gdb.hh PRE-CREATION > src/arch/riscv/remote_gdb.cc PRE-CREATION > src/arch/riscv/tlb.hh PRE-CREATION > src/arch/riscv/tlb.cc PRE-CREATION > src/arch/riscv/types.hh PRE-CREATION > src/arch/riscv/utility.hh PRE-CREATION > src/arch/riscv/vtophys.hh PRE-CREATION > src/base/loader/elf_object.cc 8bc53d5565ba > src/base/loader/object_file.hh 8bc53d5565ba > src/cpu/BaseCPU.py 8bc53d5565ba > src/sim/process.cc 8bc53d5565ba > tests/test-progs/hello/bin/riscv/linux/hello 8bc53d5565ba > src/arch/riscv/stacktrace.cc PRE-CREATION > src/arch/riscv/system.hh PRE-CREATION > src/arch/riscv/system.cc PRE-CREATION > > Diff: http://reviews.gem5.org/r/3624/diff/ > > > Testing > ------- > > > Thanks, > > Alec Roelke > > _______________________________________________ gem5-dev mailing list [email protected] http://m5sim.org/mailman/listinfo/gem5-dev
