----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://reviews.gem5.org/r/3207/#review7720 -----------------------------------------------------------
Ship it! I have been testing this a bit with the hello world program in SE mode for x86. It seems fine to me. Thanks for cleaning this up! - Alexandru Dutu On Dec. 3, 2015, 9:53 a.m., Boris Shingarov wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://reviews.gem5.org/r/3207/ > ----------------------------------------------------------- > > (Updated Dec. 3, 2015, 9:53 a.m.) > > > Review request for Default. > > > Repository: gem5 > > > Description > ------- > > arm: remote GDB: rationalize structure of register offsets > > Currently, the wire format of register values in g- and G-packets is > modelled using a union of uint8/16/32/64 arrays. The offset positions > of each register are expressed as a "register count" scaled according > to the width of the register in question. This results in counter- > intuitive and error-prone "register count arithmetic", and some > formats would even be altogether unrepresentable in such model, e.g. > a 64-bit register following a 32-bit one would have a fractional index > in the regs64 array. > Another difficulty is that the array is allocated before the actual > architecture of the workload is known (and therefore before the correct > size for the array can be calculated). > > With this patch I propose a simpler mechanism for expressing the > register set structure. In the new code, GdbRegCache is an abstract > class; its subclasses contain straightforward structs reflecting the > register representation. The determination whether to use e.g. the > AArch32 vs. AArch64 register set (or SPARCv8 vs SPARCv9, etc.) is made > by polymorphically dispatching getregs() to the concrete subclass. > The subclass is not instantiated until it is needed for actual > g-/G-packet processing, when the mode is already known. > > This patch is not meant to be merged in on its own, because it changes > the contract between src/base/remote_gdb.* and src/arch/*/remote_gdb.*, > so as it stands right now, it would break the other architectures. > In this patch only the base and the ARM code are provided for review; > once we agree on the structure, I will provide src/arch/*/remote_gdb.* > for the other architectures; those patches could then be merged in > together. > > > Diffs > ----- > > src/arch/arm/remote_gdb.cc 135c16fa409d > src/arch/mips/remote_gdb.hh 135c16fa409d > src/arch/mips/remote_gdb.cc 135c16fa409d > src/arch/power/remote_gdb.hh 135c16fa409d > src/arch/power/remote_gdb.cc 135c16fa409d > src/arch/sparc/remote_gdb.hh 135c16fa409d > src/arch/sparc/remote_gdb.cc 135c16fa409d > src/arch/x86/remote_gdb.hh 135c16fa409d > src/arch/x86/remote_gdb.cc 135c16fa409d > src/base/remote_gdb.hh 135c16fa409d > src/base/remote_gdb.cc 135c16fa409d > src/arch/alpha/kgdb.h 135c16fa409d > src/arch/alpha/remote_gdb.hh 135c16fa409d > src/arch/alpha/remote_gdb.cc 135c16fa409d > src/arch/arm/remote_gdb.hh 135c16fa409d > > Diff: http://reviews.gem5.org/r/3207/diff/ > > > Testing > ------- > > Tested in SE mode with both AArch32 and AArch64, using gdb 7.10 configured > with --target=arm-linux-gnueabihf and --target=aarch64-linux-gnu, as well as > with our internal gdb RSP client. > > > Thanks, > > Boris Shingarov > > _______________________________________________ gem5-dev mailing list [email protected] http://m5sim.org/mailman/listinfo/gem5-dev
