| Message ID | 20200522023440.26261-1-richard.henderson@linaro.org |
|---|---|
| Headers | show |
| Series | risu cleanups and improvements | expand |
Just a note that I'm assuming this is still on Alex's plate to review. Ping me if it gets reviews and is ready to apply. thanks -- PMM On Fri, 22 May 2020 at 03:35, Richard Henderson <richard.henderson@linaro.org> wrote: > > Version 3 changes the --dump option to --fulldump and --diffdump, > after an off-hand suggestion by Alex. > > These are now mode options, similar to --master. Which means that > dumping is an orthogonal apprentice type, which means that we can > dump from a socket. I'm not sure that will be useful as such, but > I think it makes main be a bit cleaner. > > If using old trace files with the new risu, you get > > Unexpected magic number: 0x000078 > > If for somehow you use different risu for master and apprentice on > sockets, the apprentice will hang waiting for data that the master > will never write. This is less than helpful, but should be trivial > to avoid. > > While cleaning up the interface for reginfo_dump_mismatch(), I > noticed some bugs on the ppc64 side. > > The patches without reviews are: > > 0014-Merge-reginfo.c-into-risu.c.patch > 0015-Rearrange-reginfo-and-memblock-buffers.patch > 0016-Split-out-recv_register_info.patch > 0017-Add-magic-and-size-to-the-trace-header.patch > 0018-Compute-reginfo_size-based-on-the-reginfo.patch > 0019-aarch64-Reorg-sve-reginfo-to-save-space.patch > 0020-aarch64-Use-arch_init-to-configure-sve.patch > 0021-ppc64-Use-uint64_t-to-represent-double.patch > 0022-Standardize-reginfo_dump_mismatch-printing.patch > 0023-Add-fulldump-and-diffdup-options.patch > 0024-Remove-return-value-from-reginfo_dump.patch > 0025-ppc64-Clean-up-reginfo-handling.patch > > most of which are new, and those that aren't new have had > significant modifications. > > > r~ > > > Richard Henderson (25): > Use bool for tracing variables > Unify master_fd and apprentice_fd to comm_fd > Hoist trace file and socket opening > Adjust tracefile open for write > Use EXIT_FAILURE, EXIT_SUCCESS > Make some risu.c symbols static > Add enum RisuOp > Add enum RisuResult > Unify i/o functions and use RisuResult > Pass non-OK result back through siglongjmp > Always write for --master > Simplify syncing with master > Split RES_MISMATCH for registers and memory > Merge reginfo.c into risu.c > Rearrange reginfo and memblock buffers > Split out recv_register_info > Add magic and size to the trace header > Compute reginfo_size based on the reginfo > aarch64: Reorg sve reginfo to save space > aarch64: Use arch_init to configure sve > ppc64: Use uint64_t to represent double > Standardize reginfo_dump_mismatch printing > Add --fulldump and --diffdup options > Remove return value from reginfo_dump > ppc64: Clean up reginfo handling > > Makefile | 2 +- > risu.h | 103 +++---- > risu_reginfo_aarch64.h | 16 +- > risu_reginfo_ppc64.h | 3 +- > comms.c | 34 +-- > reginfo.c | 183 ----------- > risu.c | 676 ++++++++++++++++++++++++++++++----------- > risu_aarch64.c | 6 +- > risu_arm.c | 6 +- > risu_i386.c | 4 +- > risu_m68k.c | 4 +- > risu_ppc64.c | 4 +- > risu_reginfo_aarch64.c | 212 +++++++------ > risu_reginfo_arm.c | 32 +- > risu_reginfo_i386.c | 22 +- > risu_reginfo_m68k.c | 37 +-- > risu_reginfo_ppc64.c | 183 +++++------ > 17 files changed, 803 insertions(+), 724 deletions(-) > delete mode 100644 reginfo.c > > -- > 2.20.1
Peter Maydell <peter.maydell@linaro.org> writes: > Just a note that I'm assuming this is still on Alex's plate to review. > Ping me if it gets reviews and is ready to apply. Thanks for the reminder - it had dropped off my radar. I'll have a quick scan through now. -- Alex Bennée
Richard Henderson <richard.henderson@linaro.org> writes: > Version 3 changes the --dump option to --fulldump and --diffdump, > after an off-hand suggestion by Alex. > > These are now mode options, similar to --master. Which means that > dumping is an orthogonal apprentice type, which means that we can > dump from a socket. I'm not sure that will be useful as such, but > I think it makes main be a bit cleaner. Hmm recording traces I ran into a difference, need to track down if its a master or apprentice bug (both are native): ./builds/arm64/risu aarch64-all-v8dot0/insn_LDAPR__INC.risu.bin -t aarch64-all-v8dot0/insn_LDAPR__INC.risu.bin.trace fails with: loading test image aarch64-all-v8dot0/insn_LDAPR__INC.risu.bin... starting apprentice image at 0xffff8548c000 starting image Mismatch reg after 4 checkpoints master reginfo: faulting insn 38bfc1f4 . . . mismatch detail (master : apprentice): X15 : 0000ffff9eba41dc vs 0000ffff8548c1dc > > If using old trace files with the new risu, you get > > Unexpected magic number: 0x000078 > > If for somehow you use different risu for master and apprentice on > sockets, the apprentice will hang waiting for data that the master > will never write. This is less than helpful, but should be trivial > to avoid. > > While cleaning up the interface for reginfo_dump_mismatch(), I > noticed some bugs on the ppc64 side. > > The patches without reviews are: > > 0014-Merge-reginfo.c-into-risu.c.patch > 0015-Rearrange-reginfo-and-memblock-buffers.patch > 0016-Split-out-recv_register_info.patch > 0017-Add-magic-and-size-to-the-trace-header.patch > 0018-Compute-reginfo_size-based-on-the-reginfo.patch > 0019-aarch64-Reorg-sve-reginfo-to-save-space.patch > 0020-aarch64-Use-arch_init-to-configure-sve.patch > 0021-ppc64-Use-uint64_t-to-represent-double.patch > 0022-Standardize-reginfo_dump_mismatch-printing.patch > 0023-Add-fulldump-and-diffdup-options.patch > 0024-Remove-return-value-from-reginfo_dump.patch > 0025-ppc64-Clean-up-reginfo-handling.patch > > most of which are new, and those that aren't new have had > significant modifications. > > > r~ > > > Richard Henderson (25): > Use bool for tracing variables > Unify master_fd and apprentice_fd to comm_fd > Hoist trace file and socket opening > Adjust tracefile open for write > Use EXIT_FAILURE, EXIT_SUCCESS > Make some risu.c symbols static > Add enum RisuOp > Add enum RisuResult > Unify i/o functions and use RisuResult > Pass non-OK result back through siglongjmp > Always write for --master > Simplify syncing with master > Split RES_MISMATCH for registers and memory > Merge reginfo.c into risu.c > Rearrange reginfo and memblock buffers > Split out recv_register_info > Add magic and size to the trace header > Compute reginfo_size based on the reginfo > aarch64: Reorg sve reginfo to save space > aarch64: Use arch_init to configure sve > ppc64: Use uint64_t to represent double > Standardize reginfo_dump_mismatch printing > Add --fulldump and --diffdup options > Remove return value from reginfo_dump > ppc64: Clean up reginfo handling > > Makefile | 2 +- > risu.h | 103 +++---- > risu_reginfo_aarch64.h | 16 +- > risu_reginfo_ppc64.h | 3 +- > comms.c | 34 +-- > reginfo.c | 183 ----------- > risu.c | 676 ++++++++++++++++++++++++++++++----------- > risu_aarch64.c | 6 +- > risu_arm.c | 6 +- > risu_i386.c | 4 +- > risu_m68k.c | 4 +- > risu_ppc64.c | 4 +- > risu_reginfo_aarch64.c | 212 +++++++------ > risu_reginfo_arm.c | 32 +- > risu_reginfo_i386.c | 22 +- > risu_reginfo_m68k.c | 37 +-- > risu_reginfo_ppc64.c | 183 +++++------ > 17 files changed, 803 insertions(+), 724 deletions(-) > delete mode 100644 reginfo.c -- Alex Bennée
Alex Bennée <alex.bennee@linaro.org> writes: > Richard Henderson <richard.henderson@linaro.org> writes: > >> Version 3 changes the --dump option to --fulldump and --diffdump, >> after an off-hand suggestion by Alex. >> >> These are now mode options, similar to --master. Which means that >> dumping is an orthogonal apprentice type, which means that we can >> dump from a socket. I'm not sure that will be useful as such, but >> I think it makes main be a bit cleaner. > > Hmm recording traces I ran into a difference, need to track down if its > a master or apprentice bug (both are native): > > ./builds/arm64/risu aarch64-all-v8dot0/insn_LDAPR__INC.risu.bin -t aarch64-all-v8dot0/insn_LDAPR__INC.risu.bin.trace > > fails with: > > loading test image aarch64-all-v8dot0/insn_LDAPR__INC.risu.bin... > starting apprentice image at 0xffff8548c000 > starting image > Mismatch reg after 4 checkpoints > master reginfo: > faulting insn 38bfc1f4 > . > . > . > mismatch detail (master : apprentice): > X15 : 0000ffff9eba41dc vs 0000ffff8548c1dc This is sort of bogus due to LDAPR being a v8.3 instruction so we fault and x15 is a pointer into the memblock which isn't stable. If it had executed we would have nixed the absolution address to and offset before the OP_COMPARE. I think for OP_SIGILL the only thing that can be truly valid would be to compare PCs so we can check the apprentice also faults on the same SIGILL? > >> >> If using old trace files with the new risu, you get >> >> Unexpected magic number: 0x000078 >> >> If for somehow you use different risu for master and apprentice on >> sockets, the apprentice will hang waiting for data that the master >> will never write. This is less than helpful, but should be trivial >> to avoid. >> >> While cleaning up the interface for reginfo_dump_mismatch(), I >> noticed some bugs on the ppc64 side. >> >> The patches without reviews are: >> >> 0014-Merge-reginfo.c-into-risu.c.patch >> 0015-Rearrange-reginfo-and-memblock-buffers.patch >> 0016-Split-out-recv_register_info.patch >> 0017-Add-magic-and-size-to-the-trace-header.patch >> 0018-Compute-reginfo_size-based-on-the-reginfo.patch >> 0019-aarch64-Reorg-sve-reginfo-to-save-space.patch >> 0020-aarch64-Use-arch_init-to-configure-sve.patch >> 0021-ppc64-Use-uint64_t-to-represent-double.patch >> 0022-Standardize-reginfo_dump_mismatch-printing.patch >> 0023-Add-fulldump-and-diffdup-options.patch >> 0024-Remove-return-value-from-reginfo_dump.patch >> 0025-ppc64-Clean-up-reginfo-handling.patch >> >> most of which are new, and those that aren't new have had >> significant modifications. >> >> >> r~ >> >> >> Richard Henderson (25): >> Use bool for tracing variables >> Unify master_fd and apprentice_fd to comm_fd >> Hoist trace file and socket opening >> Adjust tracefile open for write >> Use EXIT_FAILURE, EXIT_SUCCESS >> Make some risu.c symbols static >> Add enum RisuOp >> Add enum RisuResult >> Unify i/o functions and use RisuResult >> Pass non-OK result back through siglongjmp >> Always write for --master >> Simplify syncing with master >> Split RES_MISMATCH for registers and memory >> Merge reginfo.c into risu.c >> Rearrange reginfo and memblock buffers >> Split out recv_register_info >> Add magic and size to the trace header >> Compute reginfo_size based on the reginfo >> aarch64: Reorg sve reginfo to save space >> aarch64: Use arch_init to configure sve >> ppc64: Use uint64_t to represent double >> Standardize reginfo_dump_mismatch printing >> Add --fulldump and --diffdup options >> Remove return value from reginfo_dump >> ppc64: Clean up reginfo handling >> >> Makefile | 2 +- >> risu.h | 103 +++---- >> risu_reginfo_aarch64.h | 16 +- >> risu_reginfo_ppc64.h | 3 +- >> comms.c | 34 +-- >> reginfo.c | 183 ----------- >> risu.c | 676 ++++++++++++++++++++++++++++++----------- >> risu_aarch64.c | 6 +- >> risu_arm.c | 6 +- >> risu_i386.c | 4 +- >> risu_m68k.c | 4 +- >> risu_ppc64.c | 4 +- >> risu_reginfo_aarch64.c | 212 +++++++------ >> risu_reginfo_arm.c | 32 +- >> risu_reginfo_i386.c | 22 +- >> risu_reginfo_m68k.c | 37 +-- >> risu_reginfo_ppc64.c | 183 +++++------ >> 17 files changed, 803 insertions(+), 724 deletions(-) >> delete mode 100644 reginfo.c -- Alex Bennée
On 6/23/20 2:00 AM, Alex Bennée wrote: > > Richard Henderson <richard.henderson@linaro.org> writes: > >> Version 3 changes the --dump option to --fulldump and --diffdump, >> after an off-hand suggestion by Alex. >> >> These are now mode options, similar to --master. Which means that >> dumping is an orthogonal apprentice type, which means that we can >> dump from a socket. I'm not sure that will be useful as such, but >> I think it makes main be a bit cleaner. > > Hmm recording traces I ran into a difference, need to track down if its > a master or apprentice bug (both are native): > > ./builds/arm64/risu aarch64-all-v8dot0/insn_LDAPR__INC.risu.bin -t aarch64-all-v8dot0/insn_LDAPR__INC.risu.bin.trace > > fails with: > > loading test image aarch64-all-v8dot0/insn_LDAPR__INC.risu.bin... > starting apprentice image at 0xffff8548c000 > starting image > Mismatch reg after 4 checkpoints > master reginfo: > faulting insn 38bfc1f4 > . > . > . > mismatch detail (master : apprentice): > X15 : 0000ffff9eba41dc vs 0000ffff8548c1dc Did this work before? I can't see how. Your v8dot0 string is suspicious, because LDAPR is a v8.3 instruction. Are you sure you're testing what you think you are? r~