----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://reviews.gem5.org/r/3207/#review7584 -----------------------------------------------------------
src/base/remote_gdb.hh (line 156) <http://reviews.gem5.org/r/3207/#comment6516> would like to see doxygen documentation for these methods Overall this seems like a sensible approach to me - Ali Saidi On Nov. 13, 2015, 5:21 p.m., Boris Shingarov wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://reviews.gem5.org/r/3207/ > ----------------------------------------------------------- > > (Updated Nov. 13, 2015, 5:21 p.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.hh 7a9eeecf2b52 > src/arch/arm/remote_gdb.cc 7a9eeecf2b52 > src/base/remote_gdb.hh 7a9eeecf2b52 > src/base/remote_gdb.cc 7a9eeecf2b52 > > 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 gem5-dev@gem5.org http://m5sim.org/mailman/listinfo/gem5-dev
