[v3,00/25] risu cleanups and improvements

Message ID 20200522023440.26261-1-richard.henderson@linaro.org
Headers show
Series
  • risu cleanups and improvements
Related show

Message

Richard Henderson May 22, 2020, 2:34 a.m.
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

Comments

Peter Maydell June 22, 2020, 7:17 p.m. | #1
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
Alex Bennée June 23, 2020, 8:23 a.m. | #2
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
Alex Bennée June 23, 2020, 9 a.m. | #3
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 June 23, 2020, 1:30 p.m. | #4
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
Richard Henderson June 23, 2020, 8:26 p.m. | #5
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~