| Message ID | 20161209114830.9158-2-alex.bennee@linaro.org |
|---|---|
| State | New |
| Headers | show |
On 9 December 2016 at 11:48, Alex Bennée <alex.bennee@linaro.org> wrote: > Before this is could seem a little quite when running as you had no > indication stuff was happening (or how fast). I only dump on the master > side as I want to minimise the amount of qemu logs to sift through. Yeah, more progress info would be good. (I used to have a patchset that made risu print '.' every so often but I dunno where that disappeared to.) > Signed-off-by: Alex Bennée <alex.bennee@linaro.org> > > -- > v3 > - use portable fmt string for image_start_address > - include arm dumping position > --- > risu.c | 15 +++++++++++++-- > risu.h | 3 +++ > risu_aarch64.c | 3 +++ > risu_arm.c | 3 +++ > 4 files changed, 22 insertions(+), 2 deletions(-) > > diff --git a/risu.c b/risu.c > index 7e42160..bcdc219 100644 > --- a/risu.c > +++ b/risu.c > @@ -37,6 +37,16 @@ sigjmp_buf jmpbuf; > /* Should we test for FP exception status bits? */ > int test_fp_exc = 0; > > +long executed_tests = 0; > +void report_test_status(void *pc) > +{ > + executed_tests += 1; > + if (executed_tests % 100 == 0) { > + fprintf(stderr,"Executed %ld test instructions (pc=%p)\r", > + executed_tests, pc); > + } I think this is too much info to be printing every 100 instructions, though. We should either just print '.'s or print info less often. Also, you're inside a signal handler, so trying to use libc stdio is probably a bad idea. With '.' you can at least stick to write() which is OK inside signal handlers. > +} > + > void master_sigill(int sig, siginfo_t *si, void *uc) > { > switch (recv_and_compare_register_info(master_socket, uc)) > @@ -61,6 +71,7 @@ void apprentice_sigill(int sig, siginfo_t *si, void *uc) > return; > case 1: > /* end of test */ > + fprintf(stderr, "\nend of test\n"); > exit(0); This is also inside a signal handler. (If that's too annoying you can do what master_sigill() does for this, and siglongjmp() out to somewhere more convenient.) > default: > /* mismatch */ The default case is about to do 'exit(1);' -- should we print something there too? > @@ -129,7 +140,7 @@ int master(int sock) > } > master_socket = sock; > set_sigill_handler(&master_sigill); > - fprintf(stderr, "starting image\n"); > + fprintf(stderr, "starting master image at 0x%"PRIxPTR"\n", image_start_address); > image_start(); > fprintf(stderr, "image returned unexpectedly\n"); > exit(1); > @@ -139,7 +150,7 @@ int apprentice(int sock) > { > apprentice_socket = sock; > set_sigill_handler(&apprentice_sigill); > - fprintf(stderr, "starting image\n"); > + fprintf(stderr, "starting apprentice image at 0x%"PRIxPTR"\n", image_start_address); > image_start(); > fprintf(stderr, "image returned unexpectedly\n"); > exit(1); > diff --git a/risu.h b/risu.h > index 26ed834..e4bb323 100644 > --- a/risu.h > +++ b/risu.h > @@ -26,6 +26,7 @@ extern uintptr_t image_start_address; > extern void *memblock; > > extern int test_fp_exc; > +extern int ismaster; > > /* Ops code under test can request from risu: */ > #define OP_COMPARE 0 > @@ -59,6 +60,8 @@ int recv_and_compare_register_info(int sock, void *uc); > */ > int report_match_status(void); > > +void report_test_status(void *pc); > + > /* Move the PC past this faulting insn by adjusting ucontext > */ > void advance_pc(void *uc); > diff --git a/risu_aarch64.c b/risu_aarch64.c > index 547f987..1595604 100644 > --- a/risu_aarch64.c > +++ b/risu_aarch64.c > @@ -28,6 +28,9 @@ void advance_pc(void *vuc) > { > ucontext_t *uc = vuc; > uc->uc_mcontext.pc += 4; > + if (ismaster) { > + report_test_status((void *) uc->uc_mcontext.pc); > + } > } I think it would be nicer to do the progress info directly from master_sigill()/apprentice_sigill(). That means that you don't have to change the advance_pc() function for every architecture, and you don't need a global for ismaster either, because you know by virtue of which function you were in. (If you're dead set on printing the guest PC we should add a uint64_t get_guest_pc(void *vuc) function for the progress info function to use, but I'd rather not bother.) > static void set_x0(void *vuc, uint64_t x0) > diff --git a/risu_arm.c b/risu_arm.c > index bdfb59b..c3fe3d3 100644 > --- a/risu_arm.c > +++ b/risu_arm.c > @@ -50,6 +50,9 @@ void advance_pc(void *vuc) > { > ucontext_t *uc = vuc; > uc->uc_mcontext.arm_pc += insnsize(uc); > + if (ismaster) { > + report_test_status((void *) uc->uc_mcontext.arm_pc); > + } > } > > static void set_r0(void *vuc, uint32_t r0) > -- > 2.11.0 thanks -- PMM
diff --git a/risu.c b/risu.c index 7e42160..bcdc219 100644 --- a/risu.c +++ b/risu.c @@ -37,6 +37,16 @@ sigjmp_buf jmpbuf; /* Should we test for FP exception status bits? */ int test_fp_exc = 0; +long executed_tests = 0; +void report_test_status(void *pc) +{ + executed_tests += 1; + if (executed_tests % 100 == 0) { + fprintf(stderr,"Executed %ld test instructions (pc=%p)\r", + executed_tests, pc); + } +} + void master_sigill(int sig, siginfo_t *si, void *uc) { switch (recv_and_compare_register_info(master_socket, uc)) @@ -61,6 +71,7 @@ void apprentice_sigill(int sig, siginfo_t *si, void *uc) return; case 1: /* end of test */ + fprintf(stderr, "\nend of test\n"); exit(0); default: /* mismatch */ @@ -129,7 +140,7 @@ int master(int sock) } master_socket = sock; set_sigill_handler(&master_sigill); - fprintf(stderr, "starting image\n"); + fprintf(stderr, "starting master image at 0x%"PRIxPTR"\n", image_start_address); image_start(); fprintf(stderr, "image returned unexpectedly\n"); exit(1); @@ -139,7 +150,7 @@ int apprentice(int sock) { apprentice_socket = sock; set_sigill_handler(&apprentice_sigill); - fprintf(stderr, "starting image\n"); + fprintf(stderr, "starting apprentice image at 0x%"PRIxPTR"\n", image_start_address); image_start(); fprintf(stderr, "image returned unexpectedly\n"); exit(1); diff --git a/risu.h b/risu.h index 26ed834..e4bb323 100644 --- a/risu.h +++ b/risu.h @@ -26,6 +26,7 @@ extern uintptr_t image_start_address; extern void *memblock; extern int test_fp_exc; +extern int ismaster; /* Ops code under test can request from risu: */ #define OP_COMPARE 0 @@ -59,6 +60,8 @@ int recv_and_compare_register_info(int sock, void *uc); */ int report_match_status(void); +void report_test_status(void *pc); + /* Move the PC past this faulting insn by adjusting ucontext */ void advance_pc(void *uc); diff --git a/risu_aarch64.c b/risu_aarch64.c index 547f987..1595604 100644 --- a/risu_aarch64.c +++ b/risu_aarch64.c @@ -28,6 +28,9 @@ void advance_pc(void *vuc) { ucontext_t *uc = vuc; uc->uc_mcontext.pc += 4; + if (ismaster) { + report_test_status((void *) uc->uc_mcontext.pc); + } } static void set_x0(void *vuc, uint64_t x0) diff --git a/risu_arm.c b/risu_arm.c index bdfb59b..c3fe3d3 100644 --- a/risu_arm.c +++ b/risu_arm.c @@ -50,6 +50,9 @@ void advance_pc(void *vuc) { ucontext_t *uc = vuc; uc->uc_mcontext.arm_pc += insnsize(uc); + if (ismaster) { + report_test_status((void *) uc->uc_mcontext.arm_pc); + } } static void set_r0(void *vuc, uint32_t r0)
Before this is could seem a little quite when running as you had no indication stuff was happening (or how fast). I only dump on the master side as I want to minimise the amount of qemu logs to sift through. Signed-off-by: Alex Bennée <alex.bennee@linaro.org> -- v3 - use portable fmt string for image_start_address - include arm dumping position --- risu.c | 15 +++++++++++++-- risu.h | 3 +++ risu_aarch64.c | 3 +++ risu_arm.c | 3 +++ 4 files changed, 22 insertions(+), 2 deletions(-) -- 2.11.0