* [Qemu-devel] [RISU PATCH v3 01/10] risu: a bit more verbosity when running
2016-12-09 11:48 [Qemu-devel] [RISU PATCH v3 00/10] Record/payback patches Alex Bennée
@ 2016-12-09 11:48 ` Alex Bennée
2016-12-16 18:27 ` Peter Maydell
2016-12-09 11:48 ` [Qemu-devel] [RISU PATCH v3 02/10] aarch64: add hand-coded risu skeleton for directed testing Alex Bennée
` (10 subsequent siblings)
11 siblings, 1 reply; 20+ messages in thread
From: Alex Bennée @ 2016-12-09 11:48 UTC (permalink / raw)
To: peter.maydell; +Cc: qemu-devel, joserz, Alex Bennée
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(-)
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)
--
2.11.0
^ permalink raw reply related [flat|nested] 20+ messages in thread* Re: [Qemu-devel] [RISU PATCH v3 01/10] risu: a bit more verbosity when running
2016-12-09 11:48 ` [Qemu-devel] [RISU PATCH v3 01/10] risu: a bit more verbosity when running Alex Bennée
@ 2016-12-16 18:27 ` Peter Maydell
0 siblings, 0 replies; 20+ messages in thread
From: Peter Maydell @ 2016-12-16 18:27 UTC (permalink / raw)
To: Alex Bennée; +Cc: QEMU Developers, Jose Ricardo Ziviani
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
^ permalink raw reply [flat|nested] 20+ messages in thread
* [Qemu-devel] [RISU PATCH v3 02/10] aarch64: add hand-coded risu skeleton for directed testing
2016-12-09 11:48 [Qemu-devel] [RISU PATCH v3 00/10] Record/payback patches Alex Bennée
2016-12-09 11:48 ` [Qemu-devel] [RISU PATCH v3 01/10] risu: a bit more verbosity when running Alex Bennée
@ 2016-12-09 11:48 ` Alex Bennée
2016-12-16 18:33 ` Peter Maydell
2016-12-09 11:48 ` [Qemu-devel] [RISU PATCH v3 03/10] risu: paramterise send/receive functions Alex Bennée
` (9 subsequent siblings)
11 siblings, 1 reply; 20+ messages in thread
From: Alex Bennée @ 2016-12-09 11:48 UTC (permalink / raw)
To: peter.maydell; +Cc: qemu-devel, joserz, Alex Bennée
Sometimes you want absolute control over your test set-up to feed
explicit values into the test. This started as an experiment but might
be useful for further developing tests.
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
---
Makefile | 7 ++
aarch64_simd_handcoded.risu.S | 208 ++++++++++++++++++++++++++++++++++++++++++
2 files changed, 215 insertions(+)
create mode 100644 aarch64_simd_handcoded.risu.S
diff --git a/Makefile b/Makefile
index bfa8cac..4202c35 100644
--- a/Makefile
+++ b/Makefile
@@ -36,6 +36,13 @@ $(PROG): $(OBJS)
%.risu.asm: %.risu.bin
${OBJDUMP} -b binary -m $(ARCH) -D $^ > $@
+# hand-coded tests
+%.risu.bin: %.risu.elf
+ $(OBJCOPY) -O binary $< $@
+
+%.risu.elf: %.risu.S
+ ${AS} -o $@ $^
+
%.o: %.c $(HDRS)
$(CC) $(CPPFLAGS) $(CFLAGS) -o $@ -c $<
diff --git a/aarch64_simd_handcoded.risu.S b/aarch64_simd_handcoded.risu.S
new file mode 100644
index 0000000..61bd11a
--- /dev/null
+++ b/aarch64_simd_handcoded.risu.S
@@ -0,0 +1,208 @@
+/*
+ Hand coded RISU tests for aarch64
+
+ Sometimes you want slightly more than random instructions and you
+ want a specifically crafted test but within RISU's framework.
+
+ This file offers such a thing.
+ # So the last nibble indicates the desired operation:
+my $OP_COMPARE = 0; # compare registers
+my $OP_TESTEND = 1; # end of test, stop
+my $OP_SETMEMBLOCK = 2; # r0 is address of memory block (8192 bytes)
+my $OP_GETMEMBLOCK = 3; # add the address of memory block to r0
+my $OP_COMPAREMEM = 4; # compare memory block
+
+ */
+
+.macro risuop_comp
+ .word 0x00005af0
+.endm
+.macro risuop_testend
+ .word 0x00005af1
+.endm
+
+ .org 0x0
+
+//.globl .data
+ mov x0, #0x0 // #0
+ msr fpsr, x0
+ mov x0, #0x0 // #0
+ msr fpcr, x0
+ mrs x0, nzcv
+ eor w0, w0, #0xf0000000
+ msr nzcv, x0
+ adr x0, _q0
+ eor x0, x0, #0xf
+ b reg_setup
+
+ /*
+
+ This is the of block of data used for ld/st and setting up vector regs
+ Each .word is 32bits of data
+
+ */
+ .align 16
+
+_q0: .word 0x70000000, 0xffffffff, 0x80000000, 0xffffffff
+_q1: .word 0x90000000, 0x00000000, 0xa0000000, 0x00000000
+_q2: .word 0xffff0000, 0x00000000, 0xeeee0000, 0x00000000
+_q3: .word 0xffffffff, 0xffffffff, 0xffffffff, 0xffffffff
+_q4: .word 0x80000000, 0x00000000, 0xf0000000, 0x00000000
+_q5: .word 0xffff0000, 0x00000000, 0xeeee0000, 0x00000000
+_q6: .word 0x00000000, 0x00000000, 0x00000000, 0x00000000
+_q7: .word 0x00000000, 0x00000000, 0x00000000, 0x00000000
+
+_q8: .word 0x00000000, 0x00000000, 0x00000000, 0x00000000
+_q9: .word 0x00000000, 0x00000000, 0x00000000, 0x00000000
+_q10: .word 0x00000000, 0x00000000, 0x00000000, 0x00000000
+_q11: .word 0x00000000, 0x00000000, 0x00000000, 0x00000000
+_q12: .word 0x00000000, 0x00000000, 0x00000000, 0x00000000
+_q13: .word 0x00000000, 0x00000000, 0x00000000, 0x00000000
+_q14: .word 0x00000000, 0x00000000, 0x00000000, 0x00000000
+_q15: .word 0x00000000, 0x00000000, 0x00000000, 0x00000000
+
+_q16: .word 0x00000000, 0x00000000, 0x00000000, 0x00000000
+_q17: .word 0x00000000, 0x00000000, 0x00000000, 0x00000000
+_q18: .word 0x00000000, 0x00000000, 0x00000000, 0x00000000
+_q19: .word 0x00000000, 0x00000000, 0x00000000, 0x00000000
+_q20: .word 0x00000000, 0x00000000, 0x00000000, 0x00000000
+_q21: .word 0x00000000, 0x00000000, 0x00000000, 0x00000000
+_q22: .word 0x00000000, 0x00000000, 0x00000000, 0x00000000
+_q23: .word 0x00000000, 0x00000000, 0x00000000, 0x00000000
+
+_q24: .word 0x00000000, 0x00000000, 0x00000000, 0x00000000
+_q25: .word 0x00000000, 0x00000000, 0x00000000, 0x00000000
+_q26: .word 0x00000000, 0x00000000, 0x00000000, 0x00000000
+_q27: .word 0x00000000, 0x00000000, 0x00000000, 0x00000000
+_q28: .word 0x00000000, 0x00000000, 0x00000000, 0x00000000
+_q29: .word 0x00000000, 0x00000000, 0x00000000, 0x00000000
+_q30: .word 0x00000000, 0x00000000, 0x00000000, 0x00000000
+_q31: .word 0x00000000, 0x00000000, 0x00000000, 0x00000000
+
+ .align 16
+
+ /* Setup the register state */
+reg_setup:
+ ldp q0, q1, [x0],#32
+ ldp q2, q3, [x0],#16
+ ldp q4, q5, [x0],#16
+ ldp q6, q7, [x0],#16
+ ldp q8, q9, [x0],#16
+ ldp q10, q11, [x0],#16
+ ldp q12, q13, [x0],#16
+ ldp q14, q15, [x0],#16
+ ldp q16, q17, [x0],#16
+ ldp q18, q19, [x0],#16
+ ldp q20, q21, [x0],#16
+ ldp q22, q23, [x0],#16
+ ldp q24, q25, [x0],#16
+ ldp q26, q27, [x0],#16
+ ldp q28, q29, [x0],#16
+ ldp q30, q31, [x0],#16
+
+ /* Set-up integer registers */
+ mov x0, #0
+ mov x1, #0
+ mov x2, #0
+ mov x3, #0
+ mov x4, #0
+ mov x5, #0
+ mov x6, #0
+ mov x7, #0
+ mov x8, #0
+ mov x9, #0
+ mov x10, #0
+ mov x11, #0
+ mov x12, #0
+ mov x13, #0
+ mov x14, #0
+ mov x15, #0
+ mov x16, #0
+ mov x17, #0
+ mov x18, #0
+ mov x19, #0
+ mov x20, #0
+ mov x21, #0
+ mov x22, #0
+ mov x23, #0
+ mov x24, #0
+ mov x25, #0
+ mov x26, #0
+ mov x26, #0
+ mov x27, #0
+ mov x28, #0
+ mov x29, #0
+ mov x30, #0
+
+ risuop_comp
+
+ /* Testing ursra */
+
+ ursra v16.2d, v0.2d, #64
+ risuop_comp
+ ursra v17.2d, v1.2d, #64
+ risuop_comp
+ ursra v18.2d, v2.2d, #64
+ risuop_comp
+ ursra v19.2d, v3.2d, #64
+ risuop_comp
+ ursra v20.2d, v4.2d, #64
+ risuop_comp
+ ursra v21.2d, v5.2d, #64
+ risuop_comp
+ ursra v22.2d, v6.2d, #64
+ risuop_comp
+ ursra v23.2d, v7.2d, #64
+ risuop_comp
+ ursra v24.2d, v8.2d, #64
+ risuop_comp
+ ursra v25.2d, v9.2d, #64
+ risuop_comp
+ ursra v26.2d, v10.2d, #64
+ risuop_comp
+ ursra v27.2d, v11.2d, #64
+ risuop_comp
+ ursra v28.2d, v12.2d, #64
+ risuop_comp
+ ursra v29.2d, v13.2d, #64
+ risuop_comp
+ ursra v30.2d, v14.2d, #64
+ risuop_comp
+ ursra v31.2d, v15.2d, #64
+ risuop_comp
+
+ /* second pass */
+ ursra v16.2d, v0.2d, #64
+ risuop_comp
+ ursra v17.2d, v1.2d, #64
+ risuop_comp
+ ursra v18.2d, v2.2d, #64
+ risuop_comp
+ ursra v19.2d, v3.2d, #64
+ risuop_comp
+ ursra v20.2d, v4.2d, #64
+ risuop_comp
+ ursra v21.2d, v5.2d, #64
+ risuop_comp
+ ursra v22.2d, v6.2d, #64
+ risuop_comp
+ ursra v23.2d, v7.2d, #64
+ risuop_comp
+ ursra v24.2d, v8.2d, #64
+ risuop_comp
+ ursra v25.2d, v9.2d, #64
+ risuop_comp
+ ursra v26.2d, v10.2d, #64
+ risuop_comp
+ ursra v27.2d, v11.2d, #64
+ risuop_comp
+ ursra v28.2d, v12.2d, #64
+ risuop_comp
+ ursra v29.2d, v13.2d, #64
+ risuop_comp
+ ursra v30.2d, v14.2d, #64
+ risuop_comp
+ ursra v31.2d, v15.2d, #64
+ risuop_comp
+
+ risuop_testend /* test end */
--
2.11.0
^ permalink raw reply related [flat|nested] 20+ messages in thread* Re: [Qemu-devel] [RISU PATCH v3 02/10] aarch64: add hand-coded risu skeleton for directed testing
2016-12-09 11:48 ` [Qemu-devel] [RISU PATCH v3 02/10] aarch64: add hand-coded risu skeleton for directed testing Alex Bennée
@ 2016-12-16 18:33 ` Peter Maydell
0 siblings, 0 replies; 20+ messages in thread
From: Peter Maydell @ 2016-12-16 18:33 UTC (permalink / raw)
To: Alex Bennée; +Cc: QEMU Developers, Jose Ricardo Ziviani
On 9 December 2016 at 11:48, Alex Bennée <alex.bennee@linaro.org> wrote:
> Sometimes you want absolute control over your test set-up to feed
> explicit values into the test. This started as an experiment but might
> be useful for further developing tests.
>
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
I really don't like this patch, I'm afraid. I'd rather not have
it in the tree, because it implies that we expect that people
might want to write (or contribute back to us) handcoded test cases.
I don't think we should be encouraging that at all.
I only wrote the test_i386.s and test_arm.s originally because I hadn't
yet written the risugen script and wanted to test the C code I'd written.
Perhaps we should delete them now as being obsolete...
thanks
-- PMM
^ permalink raw reply [flat|nested] 20+ messages in thread
* [Qemu-devel] [RISU PATCH v3 03/10] risu: paramterise send/receive functions
2016-12-09 11:48 [Qemu-devel] [RISU PATCH v3 00/10] Record/payback patches Alex Bennée
2016-12-09 11:48 ` [Qemu-devel] [RISU PATCH v3 01/10] risu: a bit more verbosity when running Alex Bennée
2016-12-09 11:48 ` [Qemu-devel] [RISU PATCH v3 02/10] aarch64: add hand-coded risu skeleton for directed testing Alex Bennée
@ 2016-12-09 11:48 ` Alex Bennée
2016-12-16 18:54 ` Peter Maydell
2016-12-09 11:48 ` [Qemu-devel] [RISU PATCH v3 04/10] risu: add simple trace and replay support Alex Bennée
` (8 subsequent siblings)
11 siblings, 1 reply; 20+ messages in thread
From: Alex Bennée @ 2016-12-09 11:48 UTC (permalink / raw)
To: peter.maydell; +Cc: qemu-devel, joserz, Alex Bennée
This is a precursor to record/playback support. Instead of passing the
socket fd we now pass helper functions for reading/writing and
responding. This will allow us to do the rest of the record/playback
work without hacking up the arch specific stuff.
I've also added a header packet with pc/risu op in it so we can keep
better track of how things are going.
---
v3
- new for v3
- arm, aarch64, ppc64
---
risu.c | 23 +++++++-
risu.h | 11 +++-
risu_aarch64.c | 115 ++++++++++++++++++++++++--------------
risu_arm.c | 147 +++++++++++++++++++++++++++++++------------------
risu_ppc64le.c | 127 ++++++++++++++++++++++++++----------------
risu_reginfo_aarch64.h | 7 +++
risu_reginfo_arm.h | 6 ++
risu_reginfo_ppc64le.h | 6 ++
8 files changed, 295 insertions(+), 147 deletions(-)
diff --git a/risu.c b/risu.c
index bcdc219..22571cd 100644
--- a/risu.c
+++ b/risu.c
@@ -47,9 +47,28 @@ void report_test_status(void *pc)
}
}
+/* Master functions */
+
+int read_sock(void *ptr, size_t bytes)
+{
+ return recv_data_pkt(master_socket, ptr, bytes);
+}
+
+void respond_sock(int r)
+{
+ send_response_byte(master_socket, r);
+}
+
+/* Apprentice function */
+
+int write_sock(void *ptr, size_t bytes)
+{
+ return send_data_pkt(apprentice_socket, ptr, bytes);
+}
+
void master_sigill(int sig, siginfo_t *si, void *uc)
{
- switch (recv_and_compare_register_info(master_socket, uc))
+ switch (recv_and_compare_register_info(read_sock, respond_sock, uc))
{
case 0:
/* match OK */
@@ -63,7 +82,7 @@ void master_sigill(int sig, siginfo_t *si, void *uc)
void apprentice_sigill(int sig, siginfo_t *si, void *uc)
{
- switch (send_register_info(apprentice_socket, uc))
+ switch (send_register_info(write_sock, uc))
{
case 0:
/* match OK */
diff --git a/risu.h b/risu.h
index e4bb323..b0178df 100644
--- a/risu.h
+++ b/risu.h
@@ -40,17 +40,24 @@ extern int ismaster;
/* Interface provided by CPU-specific code: */
+/* To keep the read/write logic from multiplying across all arches
+ * we wrap up the function here to keep all the changes in one place
+ */
+typedef int (*write_fn) (void *ptr, size_t bytes);
+typedef int (*read_fn) (void *ptr, size_t bytes);
+typedef void (*respond_fn) (int response);
+
/* Send the register information from the struct ucontext down the socket.
* Return the response code from the master.
* NB: called from a signal handler.
*/
-int send_register_info(int sock, void *uc);
+int send_register_info(write_fn write_fn, void *uc);
/* Read register info from the socket and compare it with that from the
* ucontext. Return 0 for match, 1 for end-of-test, 2 for mismatch.
* NB: called from a signal handler.
*/
-int recv_and_compare_register_info(int sock, void *uc);
+int recv_and_compare_register_info(read_fn read_fn, respond_fn respond, void *uc);
/* Print a useful report on the status of the last comparison
* done in recv_and_compare_register_info(). This is called on
diff --git a/risu_aarch64.c b/risu_aarch64.c
index 1595604..c4c0d4d 100644
--- a/risu_aarch64.c
+++ b/risu_aarch64.c
@@ -50,21 +50,30 @@ static int get_risuop(uint32_t insn)
return (key != risukey) ? -1 : op;
}
-int send_register_info(int sock, void *uc)
+int send_register_info(write_fn write_fn, void *uc)
{
struct reginfo ri;
- int op;
+ trace_header_t header;
+ int op, r = 0;
+
reginfo_init(&ri, uc);
op = get_risuop(ri.faulting_insn);
+ /* Write a header with PC/op to keep in sync */
+ header.pc = ri.pc;
+ header.risu_op = op;
+ if (write_fn(&header, sizeof(header)) != 0) {
+ fprintf(stderr,"%s: failed header write\n", __func__);
+ return -1;
+ }
+
switch (op) {
- case OP_COMPARE:
case OP_TESTEND:
- default:
- /* Do a simple register compare on (a) explicit request
- * (b) end of test (c) a non-risuop UNDEF
- */
- return send_data_pkt(sock, &ri, sizeof(ri));
+ if (write_fn(&ri, sizeof(ri)) != 0) {
+ fprintf(stderr,"%s: failed last write\n", __func__);
+ }
+ r = 1;
+ break;
case OP_SETMEMBLOCK:
memblock = (void *)ri.regs[0];
break;
@@ -72,10 +81,16 @@ int send_register_info(int sock, void *uc)
set_x0(uc, ri.regs[0] + (uintptr_t)memblock);
break;
case OP_COMPAREMEM:
- return send_data_pkt(sock, memblock, MEMBLOCKLEN);
+ return write_fn(memblock, MEMBLOCKLEN);
break;
+ case OP_COMPARE:
+ default:
+ /* Do a simple register compare on (a) explicit request
+ * (b) end of test (c) a non-risuop UNDEF
+ */
+ return write_fn(&ri, sizeof(ri));
}
- return 0;
+ return r;
}
/* Read register info from the socket and compare it with that from the
@@ -86,51 +101,69 @@ int send_register_info(int sock, void *uc)
* that says whether it's register or memory data, so if the two
* sides get out of sync then we will fail obscurely.
*/
-int recv_and_compare_register_info(int sock, void *uc)
+int recv_and_compare_register_info(read_fn read_fn, respond_fn resp_fn, void *uc)
{
int resp = 0, op;
+ trace_header_t header;
reginfo_init(&master_ri, uc);
op = get_risuop(master_ri.faulting_insn);
- switch (op) {
- case OP_COMPARE:
- case OP_TESTEND:
- default:
- /* Do a simple register compare on (a) explicit request
- * (b) end of test (c) a non-risuop UNDEF
- */
- if (recv_data_pkt(sock, &apprentice_ri, sizeof(apprentice_ri))) {
- packet_mismatch = 1;
- resp = 2;
-
- } else if (!reginfo_is_eq(&master_ri, &apprentice_ri)) {
- /* register mismatch */
- resp = 2;
-
- } else if (op == OP_TESTEND) {
- resp = 1;
- }
- send_response_byte(sock, resp);
- break;
- case OP_SETMEMBLOCK:
+ if (read_fn(&header, sizeof(header)) != 0) {
+ fprintf(stderr,"%s: failed header read\n", __func__);
+ return -1;
+ }
+
+ if (header.risu_op == op ) {
+
+ /* send OK for the header */
+ resp_fn(0);
+
+ switch (op) {
+ case OP_COMPARE:
+ case OP_TESTEND:
+ default:
+ /* Do a simple register compare on (a) explicit request
+ * (b) end of test (c) a non-risuop UNDEF
+ */
+ if (read_fn(&apprentice_ri, sizeof(apprentice_ri))) {
+ packet_mismatch = 1;
+ resp = 2;
+
+ } else if (!reginfo_is_eq(&master_ri, &apprentice_ri)) {
+ /* register mismatch */
+ resp = 2;
+
+ } else if (op == OP_TESTEND) {
+ resp = 1;
+ }
+ resp_fn(resp);
+ break;
+ case OP_SETMEMBLOCK:
memblock = (void *)master_ri.regs[0];
break;
- case OP_GETMEMBLOCK:
+ case OP_GETMEMBLOCK:
set_x0(uc, master_ri.regs[0] + (uintptr_t)memblock);
break;
- case OP_COMPAREMEM:
- mem_used = 1;
- if (recv_data_pkt(sock, apprentice_memblock, MEMBLOCKLEN)) {
- packet_mismatch = 1;
- resp = 2;
- } else if (memcmp(memblock, apprentice_memblock, MEMBLOCKLEN) != 0) {
+ case OP_COMPAREMEM:
+ mem_used = 1;
+ if (read_fn(apprentice_memblock, MEMBLOCKLEN)) {
+ packet_mismatch = 1;
+ resp = 2;
+ } else if (memcmp(memblock, apprentice_memblock, MEMBLOCKLEN) != 0) {
/* memory mismatch */
resp = 2;
}
- send_response_byte(sock, resp);
+ resp_fn(resp);
break;
- }
+ }
+ } else {
+ fprintf(stderr, "out of sync %lx/%lx %d/%d\n",
+ master_ri.pc, header.pc,
+ op, header.risu_op);
+ resp = 2;
+ resp_fn(resp);
+ }
return resp;
}
diff --git a/risu_arm.c b/risu_arm.c
index c3fe3d3..474729c 100644
--- a/risu_arm.c
+++ b/risu_arm.c
@@ -73,22 +73,31 @@ static int get_risuop(uint32_t insn, int isz)
}
-int send_register_info(int sock, void *uc)
+int send_register_info(write_fn write_fn, void *uc)
{
struct reginfo ri;
- int op;
+ trace_header_t header;
+ int op, r = 0;
+
reginfo_init(&ri, uc);
op = get_risuop(ri.faulting_insn, ri.faulting_insn_size);
+ /* Write a header with PC/op to keep in sync */
+ header.pc = ri.gpreg[15];
+ header.risu_op = op;
+ if (write_fn(&header, sizeof(header)) != 0) {
+ fprintf(stderr,"%s: failed header write\n", __func__);
+ return -1;
+ }
+
switch (op)
{
- case OP_COMPARE:
case OP_TESTEND:
- default:
- /* Do a simple register compare on (a) explicit request
- * (b) end of test (c) a non-risuop UNDEF
- */
- return send_data_pkt(sock, &ri, sizeof(ri));
+ if (write_fn(&ri, sizeof(ri)) != 0) {
+ fprintf(stderr,"%s: failed last write\n", __func__);
+ }
+ r = 1;
+ break;
case OP_SETMEMBLOCK:
memblock = (void *)ri.gpreg[0];
break;
@@ -96,10 +105,18 @@ int send_register_info(int sock, void *uc)
set_r0(uc, ri.gpreg[0] + (uintptr_t)memblock);
break;
case OP_COMPAREMEM:
- return send_data_pkt(sock, memblock, MEMBLOCKLEN);
+ r = write_fn(memblock, MEMBLOCKLEN);
+ break;
+ case OP_COMPARE:
+ default:
+ /* Do a simple register compare on (a) explicit request
+ * (b) end of test (c) a non-risuop UNDEF
+ */
+ r = write_fn(&ri, sizeof(ri));
break;
}
- return 0;
+
+ return r;
}
/* Read register info from the socket and compare it with that from the
@@ -110,58 +127,78 @@ int send_register_info(int sock, void *uc)
* that says whether it's register or memory data, so if the two
* sides get out of sync then we will fail obscurely.
*/
-int recv_and_compare_register_info(int sock, void *uc)
+int recv_and_compare_register_info(read_fn read_fn, respond_fn resp_fn, void *uc)
{
int resp = 0, op;
+ trace_header_t header;
reginfo_init(&master_ri, uc);
op = get_risuop(master_ri.faulting_insn, master_ri.faulting_insn_size);
- switch (op)
- {
- case OP_COMPARE:
- case OP_TESTEND:
- default:
- /* Do a simple register compare on (a) explicit request
- * (b) end of test (c) a non-risuop UNDEF
- */
- if (recv_data_pkt(sock, &apprentice_ri, sizeof(apprentice_ri)))
- {
- packet_mismatch = 1;
- resp = 2;
- }
- else if (memcmp(&master_ri, &apprentice_ri, sizeof(master_ri)) != 0)
- {
- /* register mismatch */
- resp = 2;
- }
- else if (op == OP_TESTEND)
- {
- resp = 1;
- }
- send_response_byte(sock, resp);
- break;
- case OP_SETMEMBLOCK:
- memblock = (void *)master_ri.gpreg[0];
- break;
- case OP_GETMEMBLOCK:
- set_r0(uc, master_ri.gpreg[0] + (uintptr_t)memblock);
- break;
- case OP_COMPAREMEM:
- mem_used = 1;
- if (recv_data_pkt(sock, apprentice_memblock, MEMBLOCKLEN))
- {
- packet_mismatch = 1;
- resp = 2;
- }
- else if (memcmp(memblock, apprentice_memblock, MEMBLOCKLEN) != 0)
- {
- /* memory mismatch */
- resp = 2;
- }
- send_response_byte(sock, resp);
- break;
+ if (read_fn(&header, sizeof(header)) != 0) {
+ fprintf(stderr,"%s: failed header read\n", __func__);
+ return -1;
}
+
+ if ( header.pc == master_ri.gpreg[15] &&
+ header.risu_op == op ) {
+
+ /* send OK for the header */
+ resp_fn(0);
+
+ switch (op)
+ {
+ case OP_COMPARE:
+ case OP_TESTEND:
+ default:
+ /* Do a simple register compare on (a) explicit request
+ * (b) end of test (c) a non-risuop UNDEF
+ */
+ if (read_fn(&apprentice_ri, sizeof(apprentice_ri)))
+ {
+ packet_mismatch = 1;
+ resp = 2;
+ }
+ else if (memcmp(&master_ri, &apprentice_ri, sizeof(master_ri)) != 0)
+ {
+ /* register mismatch */
+ resp = 2;
+ }
+ else if (op == OP_TESTEND)
+ {
+ resp = 1;
+ }
+ resp_fn(resp);
+ break;
+ case OP_SETMEMBLOCK:
+ memblock = (void *)master_ri.gpreg[0];
+ break;
+ case OP_GETMEMBLOCK:
+ set_r0(uc, master_ri.gpreg[0] + (uintptr_t)memblock);
+ break;
+ case OP_COMPAREMEM:
+ mem_used = 1;
+ if (read_fn(apprentice_memblock, MEMBLOCKLEN))
+ {
+ packet_mismatch = 1;
+ resp = 2;
+ }
+ else if (memcmp(memblock, apprentice_memblock, MEMBLOCKLEN) != 0)
+ {
+ /* memory mismatch */
+ resp = 2;
+ }
+ resp_fn(resp);
+ break;
+ }
+ } else {
+ fprintf(stderr, "out of sync %x/%x %d/%d\n",
+ master_ri.gpreg[15], header.pc,
+ op, header.risu_op);
+ resp = 2;
+ resp_fn(resp);
+ }
+
return resp;
}
diff --git a/risu_ppc64le.c b/risu_ppc64le.c
index 9c1fafd..f156ed8 100644
--- a/risu_ppc64le.c
+++ b/risu_ppc64le.c
@@ -44,19 +44,30 @@ static int get_risuop(uint32_t insn)
return (key != risukey) ? -1 : op;
}
-int send_register_info(int sock, void *uc)
+int send_register_info(write_fn write_fn, void *uc)
{
struct reginfo ri;
- int op;
+ trace_header_t header;
+ int op, r = 0;
reginfo_init(&ri, uc);
op = get_risuop(ri.faulting_insn);
+ /* Write a header with PC/op to keep in sync */
+ header.pc = ri.nip;
+ header.risu_op = op;
+ if (write_fn(&header, sizeof(header)) != 0) {
+ fprintf(stderr,"%s: failed header write\n", __func__);
+ return -1;
+ }
+
switch (op) {
- case OP_COMPARE:
case OP_TESTEND:
- default:
- return send_data_pkt(sock, &ri, sizeof(ri));
+ if (write_fn(&ri, sizeof(ri)) != 0) {
+ fprintf(stderr,"%s: failed last write\n", __func__);
+ }
+ r = 1;
+ break;
case OP_SETMEMBLOCK:
memblock = (void*)ri.gregs[0];
break;
@@ -64,57 +75,79 @@ int send_register_info(int sock, void *uc)
set_x0(uc, ri.gregs[0] + (uintptr_t)memblock);
break;
case OP_COMPAREMEM:
- return send_data_pkt(sock, memblock, MEMBLOCKLEN);
+ return write_fn(memblock, MEMBLOCKLEN);
break;
+ case OP_COMPARE:
+ default:
+ return write_fn(&ri, sizeof(ri));
}
- return 0;
+ return r;
}
/* Read register info from the socket and compare it with that from the
* ucontext. Return 0 for match, 1 for end-of-test, 2 for mismatch.
* NB: called from a signal handler.
*/
-int recv_and_compare_register_info(int sock, void *uc)
+int recv_and_compare_register_info(read_fn read_fn, respond_fn resp_fn, void *uc)
{
- int resp = 0;
- int op;
-
- reginfo_init(&master_ri, uc);
- op = get_risuop(master_ri.faulting_insn);
-
- switch (op) {
- case OP_COMPARE:
- case OP_TESTEND:
- default:
- if (recv_data_pkt(sock, &apprentice_ri, sizeof(apprentice_ri))) {
- packet_mismatch = 1;
- resp = 2;
- } else if (!reginfo_is_eq(&master_ri, &apprentice_ri, uc)) {
- resp = 2;
- }
- else if (op == OP_TESTEND) {
- resp = 1;
- }
- send_response_byte(sock, resp);
- break;
- case OP_SETMEMBLOCK:
- memblock = (void*)master_ri.gregs[0];
- break;
- case OP_GETMEMBLOCK:
- set_x0(uc, master_ri.gregs[0] + (uintptr_t)memblock);
- break;
- case OP_COMPAREMEM:
- mem_used = 1;
- if (recv_data_pkt(sock, apprentice_memblock, MEMBLOCKLEN)) {
- packet_mismatch = 1;
- resp = 2;
- } else if (memcmp(memblock, apprentice_memblock, MEMBLOCKLEN) != 0) {
- resp = 2;
- }
- send_response_byte(sock, resp);
- break;
- }
- return resp;
+ int resp = 0;
+ int op;
+ trace_header_t header;
+
+ reginfo_init(&master_ri, uc);
+ op = get_risuop(master_ri.faulting_insn);
+
+ if (read_fn(&header, sizeof(header)) != 0) {
+ fprintf(stderr,"%s: failed header read\n", __func__);
+ return -1;
+ }
+
+ if (header.risu_op == op ) {
+
+ /* send OK for the header */
+ resp_fn(0);
+
+ switch (op) {
+ case OP_COMPARE:
+ case OP_TESTEND:
+ default:
+ if (read_fn(&apprentice_ri, sizeof(apprentice_ri))) {
+ packet_mismatch = 1;
+ resp = 2;
+ } else if (!reginfo_is_eq(&master_ri, &apprentice_ri, uc)) {
+ resp = 2;
+ }
+ else if (op == OP_TESTEND) {
+ resp = 1;
+ }
+ resp_fn(resp);
+ break;
+ case OP_SETMEMBLOCK:
+ memblock = (void*)master_ri.gregs[0];
+ break;
+ case OP_GETMEMBLOCK:
+ set_x0(uc, master_ri.gregs[0] + (uintptr_t)memblock);
+ break;
+ case OP_COMPAREMEM:
+ mem_used = 1;
+ if (read_fn(apprentice_memblock, MEMBLOCKLEN)) {
+ packet_mismatch = 1;
+ resp = 2;
+ } else if (memcmp(memblock, apprentice_memblock, MEMBLOCKLEN) != 0) {
+ resp = 2;
+ }
+ resp_fn(resp);
+ break;
+ }
+ } else {
+ fprintf(stderr, "out of sync %lx/%lx %d/%d\n",
+ master_ri.nip, header.pc,
+ op, header.risu_op);
+ resp = 2;
+ resp_fn(resp);
+ }
+
+ return resp;
}
/* Print a useful report on the status of the last comparison
diff --git a/risu_reginfo_aarch64.h b/risu_reginfo_aarch64.h
index 166b76c..db51cb2 100644
--- a/risu_reginfo_aarch64.h
+++ b/risu_reginfo_aarch64.h
@@ -28,6 +28,13 @@ struct reginfo
__uint128_t vregs[32];
};
+typedef struct
+{
+ uint64_t pc;
+ uint32_t risu_op;
+} trace_header_t;
+
+
/* initialize structure from a ucontext */
void reginfo_init(struct reginfo *ri, ucontext_t *uc);
diff --git a/risu_reginfo_arm.h b/risu_reginfo_arm.h
index 80c28c6..7e7e408 100644
--- a/risu_reginfo_arm.h
+++ b/risu_reginfo_arm.h
@@ -23,6 +23,12 @@ struct reginfo
uint32_t fpscr;
};
+typedef struct
+{
+ uint32_t pc;
+ uint32_t risu_op;
+} trace_header_t;
+
/* initialize a reginfo structure with data from uc */
void reginfo_init(struct reginfo *ri, ucontext_t *uc);
diff --git a/risu_reginfo_ppc64le.h b/risu_reginfo_ppc64le.h
index abe6002..49b4938 100644
--- a/risu_reginfo_ppc64le.h
+++ b/risu_reginfo_ppc64le.h
@@ -25,6 +25,12 @@ struct reginfo
vrregset_t vrregs;
};
+typedef struct
+{
+ uint64_t pc;
+ uint32_t risu_op;
+} trace_header_t;
+
/* initialize structure from a ucontext */
void reginfo_init(struct reginfo *ri, ucontext_t *uc);
--
2.11.0
^ permalink raw reply related [flat|nested] 20+ messages in thread* Re: [Qemu-devel] [RISU PATCH v3 03/10] risu: paramterise send/receive functions
2016-12-09 11:48 ` [Qemu-devel] [RISU PATCH v3 03/10] risu: paramterise send/receive functions Alex Bennée
@ 2016-12-16 18:54 ` Peter Maydell
0 siblings, 0 replies; 20+ messages in thread
From: Peter Maydell @ 2016-12-16 18:54 UTC (permalink / raw)
To: Alex Bennée; +Cc: QEMU Developers, Jose Ricardo Ziviani
On 9 December 2016 at 11:48, Alex Bennée <alex.bennee@linaro.org> wrote:
("parameterise" in subject)
> This is a precursor to record/playback support. Instead of passing the
> socket fd we now pass helper functions for reading/writing and
> responding. This will allow us to do the rest of the record/playback
> work without hacking up the arch specific stuff.
>
> I've also added a header packet with pc/risu op in it so we can keep
> better track of how things are going.
"also" tends to mean "I know this ought to be two patches really".
Can I ask you to split it up? I don't want to be too picky for
testing tool code, but this is a pretty big patch so I think it's
worth doing.
Missing signed-off-by line.
> ---
> v3
> - new for v3
> - arm, aarch64, ppc64
> ---
> risu.c | 23 +++++++-
> risu.h | 11 +++-
> risu_aarch64.c | 115 ++++++++++++++++++++++++--------------
> risu_arm.c | 147 +++++++++++++++++++++++++++++++------------------
> risu_ppc64le.c | 127 ++++++++++++++++++++++++++----------------
> risu_reginfo_aarch64.h | 7 +++
> risu_reginfo_arm.h | 6 ++
> risu_reginfo_ppc64le.h | 6 ++
> 8 files changed, 295 insertions(+), 147 deletions(-)
>
> diff --git a/risu.c b/risu.c
> index bcdc219..22571cd 100644
> --- a/risu.c
> +++ b/risu.c
> @@ -47,9 +47,28 @@ void report_test_status(void *pc)
> }
> }
>
> +/* Master functions */
> +
> +int read_sock(void *ptr, size_t bytes)
> +{
> + return recv_data_pkt(master_socket, ptr, bytes);
> +}
> +
> +void respond_sock(int r)
> +{
> + send_response_byte(master_socket, r);
> +}
> +
> +/* Apprentice function */
> +
> +int write_sock(void *ptr, size_t bytes)
> +{
> + return send_data_pkt(apprentice_socket, ptr, bytes);
> +}
> +
> void master_sigill(int sig, siginfo_t *si, void *uc)
> {
> - switch (recv_and_compare_register_info(master_socket, uc))
> + switch (recv_and_compare_register_info(read_sock, respond_sock, uc))
> {
> case 0:
> /* match OK */
> @@ -63,7 +82,7 @@ void master_sigill(int sig, siginfo_t *si, void *uc)
>
> void apprentice_sigill(int sig, siginfo_t *si, void *uc)
> {
> - switch (send_register_info(apprentice_socket, uc))
> + switch (send_register_info(write_sock, uc))
> {
> case 0:
> /* match OK */
> diff --git a/risu.h b/risu.h
> index e4bb323..b0178df 100644
> --- a/risu.h
> +++ b/risu.h
> @@ -40,17 +40,24 @@ extern int ismaster;
>
> /* Interface provided by CPU-specific code: */
>
> +/* To keep the read/write logic from multiplying across all arches
> + * we wrap up the function here to keep all the changes in one place
> + */
> +typedef int (*write_fn) (void *ptr, size_t bytes);
> +typedef int (*read_fn) (void *ptr, size_t bytes);
> +typedef void (*respond_fn) (int response);
I'm going to ask you to indulge a style preference:
typedefs for function pointers should be the type of the function:
typedef int write_fn(void *ptr, size_t bytes);
and then when you're dealing with it you use the '*', eg
int send_register_info(write_fn *writefn, void *uc);
I think it's clearer because it means the type name fits what it
is: a "write_fn" is a function, and a "write_fn *" is a pointer to
a function.
> diff --git a/risu_reginfo_aarch64.h b/risu_reginfo_aarch64.h
> index 166b76c..db51cb2 100644
> --- a/risu_reginfo_aarch64.h
> +++ b/risu_reginfo_aarch64.h
> @@ -28,6 +28,13 @@ struct reginfo
> __uint128_t vregs[32];
> };
>
> +typedef struct
> +{
> + uint64_t pc;
> + uint32_t risu_op;
> +} trace_header_t;
> +
> +
> /* initialize structure from a ucontext */
> void reginfo_init(struct reginfo *ri, ucontext_t *uc);
>
> diff --git a/risu_reginfo_arm.h b/risu_reginfo_arm.h
> index 80c28c6..7e7e408 100644
> --- a/risu_reginfo_arm.h
> +++ b/risu_reginfo_arm.h
> @@ -23,6 +23,12 @@ struct reginfo
> uint32_t fpscr;
> };
>
> +typedef struct
> +{
> + uint32_t pc;
> + uint32_t risu_op;
> +} trace_header_t;
Can we just transfer the PC as 64 bits on all architectures,
so we don't need to define a struct for each one?
> +
> /* initialize a reginfo structure with data from uc */
> void reginfo_init(struct reginfo *ri, ucontext_t *uc);
>
> diff --git a/risu_reginfo_ppc64le.h b/risu_reginfo_ppc64le.h
> index abe6002..49b4938 100644
> --- a/risu_reginfo_ppc64le.h
> +++ b/risu_reginfo_ppc64le.h
> @@ -25,6 +25,12 @@ struct reginfo
> vrregset_t vrregs;
> };
>
> +typedef struct
> +{
> + uint64_t pc;
> + uint32_t risu_op;
> +} trace_header_t;
> +
> /* initialize structure from a ucontext */
> void reginfo_init(struct reginfo *ri, ucontext_t *uc);
>
> --
> 2.11.0
thanks
-- PMM
^ permalink raw reply [flat|nested] 20+ messages in thread
* [Qemu-devel] [RISU PATCH v3 04/10] risu: add simple trace and replay support
2016-12-09 11:48 [Qemu-devel] [RISU PATCH v3 00/10] Record/payback patches Alex Bennée
` (2 preceding siblings ...)
2016-12-09 11:48 ` [Qemu-devel] [RISU PATCH v3 03/10] risu: paramterise send/receive functions Alex Bennée
@ 2016-12-09 11:48 ` Alex Bennée
2016-12-16 19:00 ` Peter Maydell
2016-12-09 11:48 ` [Qemu-devel] [RISU PATCH v3 05/10] risu: add support compressed tracefiles Alex Bennée
` (7 subsequent siblings)
11 siblings, 1 reply; 20+ messages in thread
From: Alex Bennée @ 2016-12-09 11:48 UTC (permalink / raw)
To: peter.maydell; +Cc: qemu-devel, joserz, Alex Bennée
This adds a very dumb and easily breakable trace and replay support. In
--master mode the various risu ops trigger a write of register/memory
state into a binary file which can be played back to an apprentice.
Currently there is no validation of the image source so feeding the
wrong image will fail straight away.
The trace files will get very big for any appreciable sized test file
and this will be addressed in later patches.
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
---
v2
- moved read/write functions into main risu.c
- cleaned up formatting
- report more in apprentice --trace mode
v3
- fix options parsing
- re-factored so no need for copy & paste
---
risu.c | 108 +++++++++++++++++++++++++++++++++++++++++++++++----------
risu_aarch64.c | 5 ++-
risu_arm.c | 4 +--
3 files changed, 93 insertions(+), 24 deletions(-)
diff --git a/risu.c b/risu.c
index 22571cd..173dd3c 100644
--- a/risu.c
+++ b/risu.c
@@ -31,6 +31,7 @@
void *memblock = 0;
int apprentice_socket, master_socket;
+int trace_file = 0;
sigjmp_buf jmpbuf;
@@ -40,10 +41,12 @@ 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);
+ if (ismaster || trace_file) {
+ executed_tests += 1;
+ if (executed_tests % 100 == 0) {
+ fprintf(stderr,"Executed %ld test instructions (pc=%p)\r",
+ executed_tests, pc);
+ }
}
}
@@ -54,6 +57,12 @@ int read_sock(void *ptr, size_t bytes)
return recv_data_pkt(master_socket, ptr, bytes);
}
+int write_trace(void *ptr, size_t bytes)
+{
+ size_t res = write(trace_file, ptr, bytes);
+ return (res == bytes) ? 0 : 1;
+}
+
void respond_sock(int r)
{
send_response_byte(master_socket, r);
@@ -66,26 +75,62 @@ int write_sock(void *ptr, size_t bytes)
return send_data_pkt(apprentice_socket, ptr, bytes);
}
+int read_trace(void *ptr, size_t bytes)
+{
+ size_t res = read(trace_file, ptr, bytes);
+ return (res == bytes) ? 0 : 1;
+}
+
+void respond_trace(int r)
+{
+ switch (r) {
+ case 0: /* test ok */
+ case 1: /* end of test */
+ break;
+ default:
+ fprintf(stderr,"%s: got response of %d\n", __func__, r);
+ break;
+ }
+}
+
void master_sigill(int sig, siginfo_t *si, void *uc)
{
- switch (recv_and_compare_register_info(read_sock, respond_sock, uc))
+ int r;
+
+ if (trace_file) {
+ r = send_register_info(write_trace, uc);
+ } else {
+ r = recv_and_compare_register_info(read_sock, respond_sock, uc);
+ }
+
+ switch (r)
{
case 0:
- /* match OK */
+ /* All OK */
advance_pc(uc);
return;
default:
/* mismatch, or end of test */
siglongjmp(jmpbuf, 1);
+ /* never */
+ return;
}
}
void apprentice_sigill(int sig, siginfo_t *si, void *uc)
{
- switch (send_register_info(write_sock, uc))
+ int r;
+
+ if (trace_file) {
+ r = recv_and_compare_register_info(read_trace, respond_trace, uc);
+ } else {
+ r = send_register_info(write_sock, uc);
+ }
+
+ switch (r)
{
case 0:
- /* match OK */
+ /* All OK */
advance_pc(uc);
return;
case 1:
@@ -94,6 +139,9 @@ void apprentice_sigill(int sig, siginfo_t *si, void *uc)
exit(0);
default:
/* mismatch */
+ if (trace_file) {
+ report_match_status();
+ }
exit(1);
}
}
@@ -155,7 +203,13 @@ int master(int sock)
{
if (sigsetjmp(jmpbuf, 1))
{
- return report_match_status();
+ if (trace_file) {
+ close(trace_file);
+ fprintf(stderr,"\nDone...\n");
+ return 0;
+ } else {
+ return report_match_status();
+ }
}
master_socket = sock;
set_sigill_handler(&master_sigill);
@@ -184,6 +238,7 @@ void usage (void)
fprintf(stderr, "between master and apprentice risu processes.\n\n");
fprintf(stderr, "Options:\n");
fprintf(stderr, " --master Be the master (server)\n");
+ fprintf(stderr, " -t, --trace=FILE Record/playback trace file\n");
fprintf(stderr, " -h, --host=HOST Specify master host machine (apprentice only)\n");
fprintf(stderr, " -p, --port=PORT Specify the port to connect to/listen on (default 9191)\n");
}
@@ -194,6 +249,7 @@ int main(int argc, char **argv)
uint16_t port = 9191;
char *hostname = "localhost";
char *imgfile;
+ char *trace_fn = NULL;
int sock;
// TODO clean this up later
@@ -204,13 +260,14 @@ int main(int argc, char **argv)
{
{ "help", no_argument, 0, '?'},
{ "master", no_argument, &ismaster, 1 },
+ { "trace", required_argument, 0, 't' },
{ "host", required_argument, 0, 'h' },
{ "port", required_argument, 0, 'p' },
{ "test-fp-exc", no_argument, &test_fp_exc, 1 },
{ 0,0,0,0 }
};
int optidx = 0;
- int c = getopt_long(argc, argv, "h:p:", longopts, &optidx);
+ int c = getopt_long(argc, argv, "h:p:t:", longopts, &optidx);
if (c == -1)
{
break;
@@ -223,6 +280,11 @@ int main(int argc, char **argv)
/* flag set by getopt_long, do nothing */
break;
}
+ case 't':
+ {
+ trace_fn = optarg;
+ break;
+ }
case 'h':
{
hostname = optarg;
@@ -253,18 +315,28 @@ int main(int argc, char **argv)
}
load_image(imgfile);
-
+
if (ismaster)
{
- fprintf(stderr, "master port %d\n", port);
- sock = master_connect(port);
- return master(sock);
+ if (trace_fn)
+ {
+ trace_file = open(trace_fn, O_WRONLY|O_CREAT, S_IRWXU);
+ } else {
+ fprintf(stderr, "master port %d\n", port);
+ sock = master_connect(port);
+ }
+ return master(sock);
}
else
- {
- fprintf(stderr, "apprentice host %s port %d\n", hostname, port);
- sock = apprentice_connect(hostname, port);
- return apprentice(sock);
+ {
+ if (trace_fn)
+ {
+ trace_file = open(trace_fn, O_RDONLY);
+ } else {
+ fprintf(stderr, "apprentice host %s port %d\n", hostname, port);
+ sock = apprentice_connect(hostname, port);
+ }
+ return apprentice(sock);
}
}
diff --git a/risu_aarch64.c b/risu_aarch64.c
index c4c0d4d..7f9f612 100644
--- a/risu_aarch64.c
+++ b/risu_aarch64.c
@@ -13,6 +13,7 @@
#include <stdio.h>
#include <ucontext.h>
#include <string.h>
+#include <unistd.h>
#include "risu.h"
#include "risu_reginfo_aarch64.h"
@@ -28,9 +29,7 @@ void advance_pc(void *vuc)
{
ucontext_t *uc = vuc;
uc->uc_mcontext.pc += 4;
- if (ismaster) {
- report_test_status((void *) uc->uc_mcontext.pc);
- }
+ 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 474729c..77bdcac 100644
--- a/risu_arm.c
+++ b/risu_arm.c
@@ -50,9 +50,7 @@ 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);
- }
+ report_test_status((void *) uc->uc_mcontext.arm_pc);
}
static void set_r0(void *vuc, uint32_t r0)
--
2.11.0
^ permalink raw reply related [flat|nested] 20+ messages in thread* Re: [Qemu-devel] [RISU PATCH v3 04/10] risu: add simple trace and replay support
2016-12-09 11:48 ` [Qemu-devel] [RISU PATCH v3 04/10] risu: add simple trace and replay support Alex Bennée
@ 2016-12-16 19:00 ` Peter Maydell
0 siblings, 0 replies; 20+ messages in thread
From: Peter Maydell @ 2016-12-16 19:00 UTC (permalink / raw)
To: Alex Bennée; +Cc: QEMU Developers, Jose Ricardo Ziviani
On 9 December 2016 at 11:48, Alex Bennée <alex.bennee@linaro.org> wrote:
> This adds a very dumb and easily breakable trace and replay support. In
> --master mode the various risu ops trigger a write of register/memory
> state into a binary file which can be played back to an apprentice.
> Currently there is no validation of the image source so feeding the
> wrong image will fail straight away.
>
> The trace files will get very big for any appreciable sized test file
> and this will be addressed in later patches.
>
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
Most of this looks good.
> ---
>
> v2
> - moved read/write functions into main risu.c
> - cleaned up formatting
> - report more in apprentice --trace mode
> v3
> - fix options parsing
> - re-factored so no need for copy & paste
> ---
> risu.c | 108 +++++++++++++++++++++++++++++++++++++++++++++++----------
> risu_aarch64.c | 5 ++-
> risu_arm.c | 4 +--
> 3 files changed, 93 insertions(+), 24 deletions(-)
>
> diff --git a/risu.c b/risu.c
> index 22571cd..173dd3c 100644
> --- a/risu.c
> +++ b/risu.c
> @@ -31,6 +31,7 @@
> void *memblock = 0;
>
> int apprentice_socket, master_socket;
> +int trace_file = 0;
>
> sigjmp_buf jmpbuf;
>
> @@ -40,10 +41,12 @@ 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);
> + if (ismaster || trace_file) {
> + executed_tests += 1;
> + if (executed_tests % 100 == 0) {
> + fprintf(stderr,"Executed %ld test instructions (pc=%p)\r",
> + executed_tests, pc);
> + }
> }
> }
>
> @@ -54,6 +57,12 @@ int read_sock(void *ptr, size_t bytes)
> return recv_data_pkt(master_socket, ptr, bytes);
> }
>
> +int write_trace(void *ptr, size_t bytes)
> +{
> + size_t res = write(trace_file, ptr, bytes);
> + return (res == bytes) ? 0 : 1;
> +}
> +
> void respond_sock(int r)
> {
> send_response_byte(master_socket, r);
> @@ -66,26 +75,62 @@ int write_sock(void *ptr, size_t bytes)
> return send_data_pkt(apprentice_socket, ptr, bytes);
> }
>
> +int read_trace(void *ptr, size_t bytes)
> +{
> + size_t res = read(trace_file, ptr, bytes);
> + return (res == bytes) ? 0 : 1;
> +}
> +
> +void respond_trace(int r)
> +{
> + switch (r) {
> + case 0: /* test ok */
> + case 1: /* end of test */
> + break;
> + default:
> + fprintf(stderr,"%s: got response of %d\n", __func__, r);
Is this a "something is broken/can't happen" case?
Anyway, you're in a signal handler so shouldn't use fprintf.
> + break;
> + }
> +}
> +
> void master_sigill(int sig, siginfo_t *si, void *uc)
> {
> - switch (recv_and_compare_register_info(read_sock, respond_sock, uc))
> + int r;
> +
> + if (trace_file) {
> + r = send_register_info(write_trace, uc);
> + } else {
> + r = recv_and_compare_register_info(read_sock, respond_sock, uc);
> + }
> +
> + switch (r)
> {
> case 0:
> - /* match OK */
> + /* All OK */
> advance_pc(uc);
> return;
> default:
> /* mismatch, or end of test */
> siglongjmp(jmpbuf, 1);
> + /* never */
> + return;
Is your compiler complaining about this? You should probably
get a better compiler (others will likely complain that the
return is dead, because siglongjmp is marked 'noreturn' in the
system headers).
> }
> }
>
> void apprentice_sigill(int sig, siginfo_t *si, void *uc)
> {
> - switch (send_register_info(write_sock, uc))
> + int r;
> +
> + if (trace_file) {
> + r = recv_and_compare_register_info(read_trace, respond_trace, uc);
> + } else {
> + r = send_register_info(write_sock, uc);
> + }
> +
> + switch (r)
> {
> case 0:
> - /* match OK */
> + /* All OK */
> advance_pc(uc);
> return;
> case 1:
> @@ -94,6 +139,9 @@ void apprentice_sigill(int sig, siginfo_t *si, void *uc)
> exit(0);
> default:
> /* mismatch */
> + if (trace_file) {
> + report_match_status();
> + }
> exit(1);
> }
> }
> @@ -155,7 +203,13 @@ int master(int sock)
> {
> if (sigsetjmp(jmpbuf, 1))
> {
> - return report_match_status();
> + if (trace_file) {
> + close(trace_file);
> + fprintf(stderr,"\nDone...\n");
> + return 0;
> + } else {
> + return report_match_status();
> + }
> }
> master_socket = sock;
> set_sigill_handler(&master_sigill);
> @@ -184,6 +238,7 @@ void usage (void)
> fprintf(stderr, "between master and apprentice risu processes.\n\n");
> fprintf(stderr, "Options:\n");
> fprintf(stderr, " --master Be the master (server)\n");
> + fprintf(stderr, " -t, --trace=FILE Record/playback trace file\n");
> fprintf(stderr, " -h, --host=HOST Specify master host machine (apprentice only)\n");
> fprintf(stderr, " -p, --port=PORT Specify the port to connect to/listen on (default 9191)\n");
> }
> @@ -194,6 +249,7 @@ int main(int argc, char **argv)
> uint16_t port = 9191;
> char *hostname = "localhost";
> char *imgfile;
> + char *trace_fn = NULL;
> int sock;
>
> // TODO clean this up later
> @@ -204,13 +260,14 @@ int main(int argc, char **argv)
> {
> { "help", no_argument, 0, '?'},
> { "master", no_argument, &ismaster, 1 },
> + { "trace", required_argument, 0, 't' },
> { "host", required_argument, 0, 'h' },
> { "port", required_argument, 0, 'p' },
> { "test-fp-exc", no_argument, &test_fp_exc, 1 },
> { 0,0,0,0 }
> };
> int optidx = 0;
> - int c = getopt_long(argc, argv, "h:p:", longopts, &optidx);
> + int c = getopt_long(argc, argv, "h:p:t:", longopts, &optidx);
> if (c == -1)
> {
> break;
> @@ -223,6 +280,11 @@ int main(int argc, char **argv)
> /* flag set by getopt_long, do nothing */
> break;
> }
> + case 't':
> + {
> + trace_fn = optarg;
> + break;
> + }
> case 'h':
> {
> hostname = optarg;
> @@ -253,18 +315,28 @@ int main(int argc, char **argv)
> }
>
> load_image(imgfile);
> -
> +
> if (ismaster)
> {
> - fprintf(stderr, "master port %d\n", port);
> - sock = master_connect(port);
> - return master(sock);
> + if (trace_fn)
> + {
> + trace_file = open(trace_fn, O_WRONLY|O_CREAT, S_IRWXU);
> + } else {
> + fprintf(stderr, "master port %d\n", port);
> + sock = master_connect(port);
> + }
GNU coding standards indentation ? I know risu's indenting
is a bit of a mess but we have not quite sunk that low yet :-)
> + return master(sock);
> }
> else
> - {
> - fprintf(stderr, "apprentice host %s port %d\n", hostname, port);
> - sock = apprentice_connect(hostname, port);
> - return apprentice(sock);
> + {
> + if (trace_fn)
> + {
> + trace_file = open(trace_fn, O_RDONLY);
> + } else {
> + fprintf(stderr, "apprentice host %s port %d\n", hostname, port);
> + sock = apprentice_connect(hostname, port);
> + }
> + return apprentice(sock);
> }
> }
>
> diff --git a/risu_aarch64.c b/risu_aarch64.c
> index c4c0d4d..7f9f612 100644
> --- a/risu_aarch64.c
> +++ b/risu_aarch64.c
> @@ -13,6 +13,7 @@
> #include <stdio.h>
> #include <ucontext.h>
> #include <string.h>
> +#include <unistd.h>
Why this include?
>
> #include "risu.h"
> #include "risu_reginfo_aarch64.h"
> @@ -28,9 +29,7 @@ void advance_pc(void *vuc)
> {
> ucontext_t *uc = vuc;
> uc->uc_mcontext.pc += 4;
> - if (ismaster) {
> - report_test_status((void *) uc->uc_mcontext.pc);
> - }
> + 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 474729c..77bdcac 100644
> --- a/risu_arm.c
> +++ b/risu_arm.c
> @@ -50,9 +50,7 @@ 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);
> - }
> + report_test_status((void *) uc->uc_mcontext.arm_pc);
> }
These bits should just vanish if you fix or drop the
earlier patch that added report_test_status() calls here.
>
> static void set_r0(void *vuc, uint32_t r0)
> --
> 2.11.0
thanks
-- PMM
^ permalink raw reply [flat|nested] 20+ messages in thread
* [Qemu-devel] [RISU PATCH v3 05/10] risu: add support compressed tracefiles
2016-12-09 11:48 [Qemu-devel] [RISU PATCH v3 00/10] Record/payback patches Alex Bennée
` (3 preceding siblings ...)
2016-12-09 11:48 ` [Qemu-devel] [RISU PATCH v3 04/10] risu: add simple trace and replay support Alex Bennée
@ 2016-12-09 11:48 ` Alex Bennée
2016-12-16 19:06 ` Peter Maydell
2016-12-09 11:48 ` [Qemu-devel] [RISU PATCH v3 06/10] risu_aarch64: it's -> it is Alex Bennée
` (6 subsequent siblings)
11 siblings, 1 reply; 20+ messages in thread
From: Alex Bennée @ 2016-12-09 11:48 UTC (permalink / raw)
To: peter.maydell; +Cc: qemu-devel, joserz, Alex Bennée
This uses the magic of zlib's gzread/write interface to wrap the
tracefile in compression. The code changes are tiny. I spent more time
messing about with the configure/linker stuff to auto-detect bits.
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
---
Makefile | 3 ++-
configure | 55 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
risu.c | 15 ++++++++++-----
3 files changed, 67 insertions(+), 6 deletions(-)
diff --git a/Makefile b/Makefile
index 4202c35..4a2ef02 100644
--- a/Makefile
+++ b/Makefile
@@ -14,6 +14,7 @@
include Makefile.in
CFLAGS ?= -g -Wall
+LDFLAGS += -lz
PROG=risu
SRCS=risu.c comms.c risu_$(ARCH).c risu_reginfo_$(ARCH).c
@@ -31,7 +32,7 @@ all: $(PROG) $(BINS)
dump: $(RISU_ASMS)
$(PROG): $(OBJS)
- $(CC) $(STATIC) $(CFLAGS) -o $@ $^
+ $(CC) $(STATIC) $(CFLAGS) -o $@ $^ $(LDFLAGS)
%.risu.asm: %.risu.bin
${OBJDUMP} -b binary -m $(ARCH) -D $^ > $@
diff --git a/configure b/configure
index f81bdb5..d11680f 100755
--- a/configure
+++ b/configure
@@ -6,6 +6,10 @@ compile() {
$CC $CFLAGS -c -o ${1}.o ${1}.c 2>/dev/null
}
+link() {
+ $LD $LDFLAGS -l${2} -o ${1} ${1}.o 2>/dev/null
+}
+
check_define() {
c=${tmp_dir}/check_define_${1}
cat > ${c}.c <<EOF
@@ -34,6 +38,50 @@ guess_arch() {
fi
}
+check_type() {
+ c=${tmp_dir}/check_type_${1}
+ cat > ${c}.c <<EOF
+#include <inttypes.h>
+#include <stdint.h>
+#include <sys/types.h>
+#include <sys/socket.h>
+
+int main(void) { $1 thisone; return 0; }
+EOF
+ compile $c
+}
+
+check_lib() {
+ c=${tmp_dir}/check_lib${1}
+ cat > ${c}.c <<EOF
+#include <stdint.h>
+#include <$2.h>
+
+int main(void) { $3; return 0; }
+EOF
+ compile $c && link $c $1
+}
+
+generate_config() {
+ cfg=config.h
+ echo "generating config.h..."
+
+ echo "/* config.h - generated by the 'configure' script */" > $cfg
+ echo "#ifndef CONFIG_H" >> $cfg
+ echo "#define CONFIG_H 1" >> $cfg
+
+ if check_type uintptr_t ; then
+ echo "#define HAVE_UINTPTR_T 1" >> $cfg
+ fi
+ if check_type socklen_t ; then
+ echo "#define HAVE_SOCKLEN_T 1" >> $cfg
+ fi
+
+ echo "#endif /* CONFIG_H */" >> $cfg
+
+ echo "...done"
+}
+
generate_makefilein() {
m=Makefile.in
echo "generating Makefile.in..."
@@ -93,6 +141,7 @@ done
CC="${CC-${CROSS_PREFIX}gcc}"
AS="${AS-${CROSS_PREFIX}as}"
+LD="${LD-${CROSS_PREFIX}ld}"
OBJCOPY="${OBJCOPY-${CROSS_PREFIX}objcopy}"
OBJDUMP="${OBJDUMP-${CROSS_PREFIX}objdump}"
@@ -100,6 +149,12 @@ if test "x${ARCH}" = "x"; then
guess_arch
fi
+if ! check_lib z zlib "zlibVersion()"; then
+ echo "Cannot find libz compression library"
+ exit 1
+fi
+
+generate_config
generate_makefilein
echo "type 'make' to start the build"
diff --git a/risu.c b/risu.c
index 173dd3c..3670bb1 100644
--- a/risu.c
+++ b/risu.c
@@ -25,6 +25,7 @@
#include <sys/mman.h>
#include <fcntl.h>
#include <string.h>
+#include <zlib.h>
#include "risu.h"
@@ -32,6 +33,7 @@ void *memblock = 0;
int apprentice_socket, master_socket;
int trace_file = 0;
+gzFile gz_trace_file;
sigjmp_buf jmpbuf;
@@ -59,7 +61,7 @@ int read_sock(void *ptr, size_t bytes)
int write_trace(void *ptr, size_t bytes)
{
- size_t res = write(trace_file, ptr, bytes);
+ size_t res = gzwrite(gz_trace_file, ptr, bytes);
return (res == bytes) ? 0 : 1;
}
@@ -77,7 +79,7 @@ int write_sock(void *ptr, size_t bytes)
int read_trace(void *ptr, size_t bytes)
{
- size_t res = read(trace_file, ptr, bytes);
+ size_t res = gzread(gz_trace_file, ptr, bytes);
return (res == bytes) ? 0 : 1;
}
@@ -203,9 +205,10 @@ int master(int sock)
{
if (sigsetjmp(jmpbuf, 1))
{
- if (trace_file) {
- close(trace_file);
- fprintf(stderr,"\nDone...\n");
+ if (trace_file)
+ {
+ gzclose(gz_trace_file);
+ fprintf(stderr,"Done...\n");
return 0;
} else {
return report_match_status();
@@ -321,6 +324,7 @@ int main(int argc, char **argv)
if (trace_fn)
{
trace_file = open(trace_fn, O_WRONLY|O_CREAT, S_IRWXU);
+ gz_trace_file = gzdopen(trace_file, "wb9");
} else {
fprintf(stderr, "master port %d\n", port);
sock = master_connect(port);
@@ -332,6 +336,7 @@ int main(int argc, char **argv)
if (trace_fn)
{
trace_file = open(trace_fn, O_RDONLY);
+ gz_trace_file = gzdopen(trace_file, "rb");
} else {
fprintf(stderr, "apprentice host %s port %d\n", hostname, port);
sock = apprentice_connect(hostname, port);
--
2.11.0
^ permalink raw reply related [flat|nested] 20+ messages in thread* Re: [Qemu-devel] [RISU PATCH v3 05/10] risu: add support compressed tracefiles
2016-12-09 11:48 ` [Qemu-devel] [RISU PATCH v3 05/10] risu: add support compressed tracefiles Alex Bennée
@ 2016-12-16 19:06 ` Peter Maydell
0 siblings, 0 replies; 20+ messages in thread
From: Peter Maydell @ 2016-12-16 19:06 UTC (permalink / raw)
To: Alex Bennée; +Cc: QEMU Developers, Jose Ricardo Ziviani
On 9 December 2016 at 11:48, Alex Bennée <alex.bennee@linaro.org> wrote:
> This uses the magic of zlib's gzread/write interface to wrap the
> tracefile in compression. The code changes are tiny. I spent more time
> messing about with the configure/linker stuff to auto-detect bits.
>
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> ---
> Makefile | 3 ++-
> configure | 55 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
> risu.c | 15 ++++++++++-----
> 3 files changed, 67 insertions(+), 6 deletions(-)
>
> diff --git a/Makefile b/Makefile
> index 4202c35..4a2ef02 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -14,6 +14,7 @@
> include Makefile.in
>
> CFLAGS ?= -g -Wall
> +LDFLAGS += -lz
>
> PROG=risu
> SRCS=risu.c comms.c risu_$(ARCH).c risu_reginfo_$(ARCH).c
> @@ -31,7 +32,7 @@ all: $(PROG) $(BINS)
> dump: $(RISU_ASMS)
>
> $(PROG): $(OBJS)
> - $(CC) $(STATIC) $(CFLAGS) -o $@ $^
> + $(CC) $(STATIC) $(CFLAGS) -o $@ $^ $(LDFLAGS)
(I think there's a stray space at the end of this line.)
>
> %.risu.asm: %.risu.bin
> ${OBJDUMP} -b binary -m $(ARCH) -D $^ > $@
> diff --git a/configure b/configure
> index f81bdb5..d11680f 100755
> --- a/configure
> +++ b/configure
> @@ -6,6 +6,10 @@ compile() {
> $CC $CFLAGS -c -o ${1}.o ${1}.c 2>/dev/null
> }
>
> +link() {
> + $LD $LDFLAGS -l${2} -o ${1} ${1}.o 2>/dev/null
> +}
I would ask you to get the shell quoting right,
but I see we don't do that in the existing configure
code, so never mind.
> +
> check_define() {
> c=${tmp_dir}/check_define_${1}
> cat > ${c}.c <<EOF
> @@ -34,6 +38,50 @@ guess_arch() {
> fi
> }
>
> +check_type() {
> + c=${tmp_dir}/check_type_${1}
> + cat > ${c}.c <<EOF
> +#include <inttypes.h>
> +#include <stdint.h>
> +#include <sys/types.h>
> +#include <sys/socket.h>
> +
> +int main(void) { $1 thisone; return 0; }
> +EOF
> + compile $c
> +}
> +
> +check_lib() {
> + c=${tmp_dir}/check_lib${1}
> + cat > ${c}.c <<EOF
> +#include <stdint.h>
> +#include <$2.h>
> +
> +int main(void) { $3; return 0; }
> +EOF
> + compile $c && link $c $1
> +}
> +
> +generate_config() {
> + cfg=config.h
> + echo "generating config.h..."
> +
> + echo "/* config.h - generated by the 'configure' script */" > $cfg
> + echo "#ifndef CONFIG_H" >> $cfg
> + echo "#define CONFIG_H 1" >> $cfg
> +
> + if check_type uintptr_t ; then
> + echo "#define HAVE_UINTPTR_T 1" >> $cfg
> + fi
> + if check_type socklen_t ; then
> + echo "#define HAVE_SOCKLEN_T 1" >> $cfg
> + fi
In what circumstances would we not have uintptr_t and
socklen_t types? We know we're building for Linux, after all.
> +
> + echo "#endif /* CONFIG_H */" >> $cfg
> +
> + echo "...done"
> +}
> +
> generate_makefilein() {
> m=Makefile.in
> echo "generating Makefile.in..."
> @@ -93,6 +141,7 @@ done
>
> CC="${CC-${CROSS_PREFIX}gcc}"
> AS="${AS-${CROSS_PREFIX}as}"
> +LD="${LD-${CROSS_PREFIX}ld}"
> OBJCOPY="${OBJCOPY-${CROSS_PREFIX}objcopy}"
> OBJDUMP="${OBJDUMP-${CROSS_PREFIX}objdump}"
>
> @@ -100,6 +149,12 @@ if test "x${ARCH}" = "x"; then
> guess_arch
> fi
>
> +if ! check_lib z zlib "zlibVersion()"; then
> + echo "Cannot find libz compression library"
> + exit 1
> +fi
> +
> +generate_config
> generate_makefilein
>
> echo "type 'make' to start the build"
> diff --git a/risu.c b/risu.c
> index 173dd3c..3670bb1 100644
> --- a/risu.c
> +++ b/risu.c
> @@ -25,6 +25,7 @@
> #include <sys/mman.h>
> #include <fcntl.h>
> #include <string.h>
> +#include <zlib.h>
>
> #include "risu.h"
>
> @@ -32,6 +33,7 @@ void *memblock = 0;
>
> int apprentice_socket, master_socket;
> int trace_file = 0;
> +gzFile gz_trace_file;
>
> sigjmp_buf jmpbuf;
>
> @@ -59,7 +61,7 @@ int read_sock(void *ptr, size_t bytes)
>
> int write_trace(void *ptr, size_t bytes)
> {
> - size_t res = write(trace_file, ptr, bytes);
> + size_t res = gzwrite(gz_trace_file, ptr, bytes);
> return (res == bytes) ? 0 : 1;
> }
>
> @@ -77,7 +79,7 @@ int write_sock(void *ptr, size_t bytes)
>
> int read_trace(void *ptr, size_t bytes)
> {
> - size_t res = read(trace_file, ptr, bytes);
> + size_t res = gzread(gz_trace_file, ptr, bytes);
> return (res == bytes) ? 0 : 1;
> }
>
> @@ -203,9 +205,10 @@ int master(int sock)
> {
> if (sigsetjmp(jmpbuf, 1))
> {
> - if (trace_file) {
> - close(trace_file);
> - fprintf(stderr,"\nDone...\n");
> + if (trace_file)
> + {
> + gzclose(gz_trace_file);
> + fprintf(stderr,"Done...\n");
> return 0;
> } else {
> return report_match_status();
> @@ -321,6 +324,7 @@ int main(int argc, char **argv)
> if (trace_fn)
> {
> trace_file = open(trace_fn, O_WRONLY|O_CREAT, S_IRWXU);
> + gz_trace_file = gzdopen(trace_file, "wb9");
> } else {
> fprintf(stderr, "master port %d\n", port);
> sock = master_connect(port);
> @@ -332,6 +336,7 @@ int main(int argc, char **argv)
> if (trace_fn)
> {
> trace_file = open(trace_fn, O_RDONLY);
> + gz_trace_file = gzdopen(trace_file, "rb");
> } else {
> fprintf(stderr, "apprentice host %s port %d\n", hostname, port);
> sock = apprentice_connect(hostname, port);
> --
> 2.11.0
Changes to the C code are fine.
thanks
-- PMM
^ permalink raw reply [flat|nested] 20+ messages in thread
* [Qemu-devel] [RISU PATCH v3 06/10] risu_aarch64: it's -> it is
2016-12-09 11:48 [Qemu-devel] [RISU PATCH v3 00/10] Record/payback patches Alex Bennée
` (4 preceding siblings ...)
2016-12-09 11:48 ` [Qemu-devel] [RISU PATCH v3 05/10] risu: add support compressed tracefiles Alex Bennée
@ 2016-12-09 11:48 ` Alex Bennée
2016-12-16 18:36 ` Peter Maydell
2016-12-09 11:48 ` [Qemu-devel] [RISU PATCH v3 07/10] risugen: remove grocer's apostrophe from REs Alex Bennée
` (5 subsequent siblings)
11 siblings, 1 reply; 20+ messages in thread
From: Alex Bennée @ 2016-12-09 11:48 UTC (permalink / raw)
To: peter.maydell; +Cc: qemu-devel, joserz, Alex Bennée
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
---
risu_aarch64.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/risu_aarch64.c b/risu_aarch64.c
index 7f9f612..8e3539e 100644
--- a/risu_aarch64.c
+++ b/risu_aarch64.c
@@ -97,7 +97,7 @@ int send_register_info(write_fn write_fn, void *uc)
* NB: called from a signal handler.
*
* We don't have any kind of identifying info in the incoming data
- * that says whether it's register or memory data, so if the two
+ * that says whether it is register or memory data, so if the two
* sides get out of sync then we will fail obscurely.
*/
int recv_and_compare_register_info(read_fn read_fn, respond_fn resp_fn, void *uc)
--
2.11.0
^ permalink raw reply related [flat|nested] 20+ messages in thread* Re: [Qemu-devel] [RISU PATCH v3 06/10] risu_aarch64: it's -> it is
2016-12-09 11:48 ` [Qemu-devel] [RISU PATCH v3 06/10] risu_aarch64: it's -> it is Alex Bennée
@ 2016-12-16 18:36 ` Peter Maydell
0 siblings, 0 replies; 20+ messages in thread
From: Peter Maydell @ 2016-12-16 18:36 UTC (permalink / raw)
To: Alex Bennée; +Cc: QEMU Developers, Jose Ricardo Ziviani
On 9 December 2016 at 11:48, Alex Bennée <alex.bennee@linaro.org> wrote:
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> ---
> risu_aarch64.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/risu_aarch64.c b/risu_aarch64.c
> index 7f9f612..8e3539e 100644
> --- a/risu_aarch64.c
> +++ b/risu_aarch64.c
> @@ -97,7 +97,7 @@ int send_register_info(write_fn write_fn, void *uc)
> * NB: called from a signal handler.
> *
> * We don't have any kind of identifying info in the incoming data
> - * that says whether it's register or memory data, so if the two
> + * that says whether it is register or memory data, so if the two
> * sides get out of sync then we will fail obscurely.
> */
> int recv_and_compare_register_info(read_fn read_fn, respond_fn resp_fn, void *uc)
Applied to risu master, thanks.
-- PMM
^ permalink raw reply [flat|nested] 20+ messages in thread
* [Qemu-devel] [RISU PATCH v3 07/10] risugen: remove grocer's apostrophe from REs
2016-12-09 11:48 [Qemu-devel] [RISU PATCH v3 00/10] Record/payback patches Alex Bennée
` (5 preceding siblings ...)
2016-12-09 11:48 ` [Qemu-devel] [RISU PATCH v3 06/10] risu_aarch64: it's -> it is Alex Bennée
@ 2016-12-09 11:48 ` Alex Bennée
2016-12-16 18:37 ` Peter Maydell
2016-12-09 11:48 ` [Qemu-devel] [RISU PATCH v3 08/10] new: generate_all.sh script Alex Bennée
` (4 subsequent siblings)
11 siblings, 1 reply; 20+ messages in thread
From: Alex Bennée @ 2016-12-09 11:48 UTC (permalink / raw)
To: peter.maydell; +Cc: qemu-devel, joserz, Alex Bennée
This also fixes perl-modes confusion about escaped strings.
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
---
risugen | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/risugen b/risugen
index a604fe5..77a550b 100755
--- a/risugen
+++ b/risugen
@@ -288,7 +288,7 @@ Valid options:
'VMULL' will match 'VMULL A1' and 'VMULL A2' but not
'VMULL_scalar A1'. This is generally what you wanted.
--not-pattern re[,re...] : exclude patterns matching regular expression.
- These RE's are applied after the matching pattern which
+ These REs are applied after the matching pattern which
is useful if you want to exclude a specific instruction from
a general set you have excluded.
--no-fp : disable floating point: no fp init, randomization etc.
--
2.11.0
^ permalink raw reply related [flat|nested] 20+ messages in thread* Re: [Qemu-devel] [RISU PATCH v3 07/10] risugen: remove grocer's apostrophe from REs
2016-12-09 11:48 ` [Qemu-devel] [RISU PATCH v3 07/10] risugen: remove grocer's apostrophe from REs Alex Bennée
@ 2016-12-16 18:37 ` Peter Maydell
0 siblings, 0 replies; 20+ messages in thread
From: Peter Maydell @ 2016-12-16 18:37 UTC (permalink / raw)
To: Alex Bennée; +Cc: QEMU Developers, Jose Ricardo Ziviani
On 9 December 2016 at 11:48, Alex Bennée <alex.bennee@linaro.org> wrote:
> This also fixes perl-modes confusion about escaped strings.
Missing apostrophe: should be "perl-mode's" :-)
>
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
Applied to risu master with that fixed; thanks.
-- PMM
> ---
> risugen | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/risugen b/risugen
> index a604fe5..77a550b 100755
> --- a/risugen
> +++ b/risugen
> @@ -288,7 +288,7 @@ Valid options:
> 'VMULL' will match 'VMULL A1' and 'VMULL A2' but not
> 'VMULL_scalar A1'. This is generally what you wanted.
> --not-pattern re[,re...] : exclude patterns matching regular expression.
> - These RE's are applied after the matching pattern which
> + These REs are applied after the matching pattern which
> is useful if you want to exclude a specific instruction from
> a general set you have excluded.
> --no-fp : disable floating point: no fp init, randomization etc.
> --
> 2.11.0
>
^ permalink raw reply [flat|nested] 20+ messages in thread
* [Qemu-devel] [RISU PATCH v3 08/10] new: generate_all.sh script
2016-12-09 11:48 [Qemu-devel] [RISU PATCH v3 00/10] Record/payback patches Alex Bennée
` (6 preceding siblings ...)
2016-12-09 11:48 ` [Qemu-devel] [RISU PATCH v3 07/10] risugen: remove grocer's apostrophe from REs Alex Bennée
@ 2016-12-09 11:48 ` Alex Bennée
2016-12-09 11:48 ` [Qemu-devel] [RISU PATCH v3 09/10] new: record_traces.sh helper script Alex Bennée
` (3 subsequent siblings)
11 siblings, 0 replies; 20+ messages in thread
From: Alex Bennée @ 2016-12-09 11:48 UTC (permalink / raw)
To: peter.maydell; +Cc: qemu-devel, joserz, Alex Bennée
A simple script to cut up a full .risu file and generate binaries for
the whole set. Call with risu file and instruction count and directory:
./generate_all.sh aarch64.risu 4 20000 testcases.aarch64
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
---
generate_all.sh | 55 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 55 insertions(+)
create mode 100755 generate_all.sh
diff --git a/generate_all.sh b/generate_all.sh
new file mode 100755
index 0000000..4c490be
--- /dev/null
+++ b/generate_all.sh
@@ -0,0 +1,55 @@
+#!/bin/sh
+#
+# Generate All Patterns
+#
+# Work through a given .risu file and generate full coverage
+#
+
+BASE_SHELL_PARAMS="./risugen"
+
+risufile=$1
+divisor=$2
+insns=$3
+dir=$4
+
+if test -z "$risufile"; then
+ echo "Need to specify a risu defintiion file"
+ exit 1
+fi
+
+if test -z "$divisor"; then
+ divisor=4
+fi
+
+if test -n "$insns"; then
+ BASE_SHELL_PARAMS="${BASE_SHELL_PARAMS} --numinsns $insns"
+fi
+
+if test -n "dir"; then
+ mkdir -p $dir
+else
+ dir="./"
+fi
+
+ALL_INSNS=$(cat $risufile | ag "^\w" | cut -f 1 -d " " | sort)
+COUNT=$(cat $risufile | ag "^\w" | cut -f 1 -d " " | wc -l)
+set -- $ALL_INSNS
+
+GROUP=$((COUNT / $divisor))
+
+while test $# -gt 0 ; do
+ INSN_PATTERNS=""
+ I_FILE="$dir/insn_"
+ for i in `seq 1 $divisor`; do
+ I=$1
+ if test -n "${I}"; then
+ shift
+ INSN_PATTERNS="${INSN_PATTERNS} --pattern ${I}"
+ I_FILE="${I_FILE}${I}_"
+ fi
+ done
+ I_FILE="${I_FILE}_INC.risu.bin"
+ CMD="$BASE_SHELL_PARAMS ${INSN_PATTERNS} $risufile ${I_FILE}"
+ echo "Running: $CMD"
+ $CMD
+done
--
2.11.0
^ permalink raw reply related [flat|nested] 20+ messages in thread* [Qemu-devel] [RISU PATCH v3 09/10] new: record_traces.sh helper script
2016-12-09 11:48 [Qemu-devel] [RISU PATCH v3 00/10] Record/payback patches Alex Bennée
` (7 preceding siblings ...)
2016-12-09 11:48 ` [Qemu-devel] [RISU PATCH v3 08/10] new: generate_all.sh script Alex Bennée
@ 2016-12-09 11:48 ` Alex Bennée
2016-12-09 11:48 ` [Qemu-devel] [RISU PATCH v3 10/10] new: run_risu.sh script Alex Bennée
` (2 subsequent siblings)
11 siblings, 0 replies; 20+ messages in thread
From: Alex Bennée @ 2016-12-09 11:48 UTC (permalink / raw)
To: peter.maydell; +Cc: qemu-devel, joserz, Alex Bennée
A simple script to run through a bunch of binaries and generate their
trace files.
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
---
v3
- allow overriding of RISU binary
---
record_traces.sh | 20 ++++++++++++++++++++
1 file changed, 20 insertions(+)
create mode 100755 record_traces.sh
diff --git a/record_traces.sh b/record_traces.sh
new file mode 100755
index 0000000..78ce47f
--- /dev/null
+++ b/record_traces.sh
@@ -0,0 +1,20 @@
+#!/bin/sh
+#
+# Record traces
+#
+# A risu helper script to batch process a bunch of binaries and record their outputs
+#
+set -e
+
+if test -z "$RISU"; then
+ script_dir=$(CDPATH= cd -- "$(dirname -- "$0")" && pwd -P)
+ RISU=${script_dir}/risu
+fi
+
+for f in $@; do
+ echo "Running risu against $f"
+ t="$f.trace"
+ ${RISU} --master $f -t $t
+ echo "Checking trace file OK"
+ ${RISU} $f -t $t
+done
--
2.11.0
^ permalink raw reply related [flat|nested] 20+ messages in thread* [Qemu-devel] [RISU PATCH v3 10/10] new: run_risu.sh script
2016-12-09 11:48 [Qemu-devel] [RISU PATCH v3 00/10] Record/payback patches Alex Bennée
` (8 preceding siblings ...)
2016-12-09 11:48 ` [Qemu-devel] [RISU PATCH v3 09/10] new: record_traces.sh helper script Alex Bennée
@ 2016-12-09 11:48 ` Alex Bennée
2016-12-16 19:09 ` [Qemu-devel] [RISU PATCH v3 00/10] Record/payback patches Peter Maydell
2016-12-23 1:41 ` joserz
11 siblings, 0 replies; 20+ messages in thread
From: Alex Bennée @ 2016-12-09 11:48 UTC (permalink / raw)
To: peter.maydell; +Cc: qemu-devel, joserz, Alex Bennée
A simple script to work through running all of a bunch of files with
record/playback traces. Dumps a summary and the number of failed tests
at the end.
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
---
v3
- tweak to allow specifying RISU binary
---
run_risu.sh | 53 +++++++++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 53 insertions(+)
create mode 100755 run_risu.sh
diff --git a/run_risu.sh b/run_risu.sh
new file mode 100755
index 0000000..7740d6f
--- /dev/null
+++ b/run_risu.sh
@@ -0,0 +1,53 @@
+#!/bin/bash
+#
+# Run risu against a set of binaries + trace files
+#
+#
+#set -e
+
+passed=()
+failed=()
+missing=()
+
+if test -z "$RISU"; then
+ script_dir=$(CDPATH= cd -- "$(dirname -- "$0")" && pwd -P)
+ RISU=${script_dir}/risu
+fi
+
+for f in $@; do
+ t="$f.trace"
+ echo "Running $f against $t"
+ if [ -e $t ]; then
+ ${QEMU} ${RISU} $f -t $t
+ if [ $? == 0 ]; then
+ passed=( "${passed[@]}" $f )
+ else
+ failed=( "${failed[@]}" $f )
+ fi
+ else
+ missing=( "${missing[@]}" $f )
+ fi
+done
+
+if test ${#missing[@]} -gt 0; then
+ echo "Tests missing ${#missing[@]} trace files:"
+ for m in "${missing[@]}"; do
+ echo "$m"
+ done
+fi
+
+if test ${#passed[@]} -gt 0; then
+ echo "Passed ${#passed[@]} tests:"
+ for p in "${passed[@]}"; do
+ echo "$p"
+ done
+fi
+
+if test ${#failed[@]} -gt 0; then
+ echo "Failed ${#failed[@]} tests:"
+ for f in "${failed[@]}"; do
+ echo "$f"
+ done
+fi
+
+exit ${#failed[@]}
--
2.11.0
^ permalink raw reply related [flat|nested] 20+ messages in thread* Re: [Qemu-devel] [RISU PATCH v3 00/10] Record/payback patches
2016-12-09 11:48 [Qemu-devel] [RISU PATCH v3 00/10] Record/payback patches Alex Bennée
` (9 preceding siblings ...)
2016-12-09 11:48 ` [Qemu-devel] [RISU PATCH v3 10/10] new: run_risu.sh script Alex Bennée
@ 2016-12-16 19:09 ` Peter Maydell
2016-12-23 1:41 ` joserz
11 siblings, 0 replies; 20+ messages in thread
From: Peter Maydell @ 2016-12-16 19:09 UTC (permalink / raw)
To: Alex Bennée; +Cc: QEMU Developers, Jose Ricardo Ziviani
On 9 December 2016 at 11:48, Alex Bennée <alex.bennee@linaro.org> wrote:
> I did a bunch of tweaking to see if I could abstract the hackage for
> record/replay a bit more. With a little bit of light re-factoring of
> the send and recv functions I can move all the specific send/recv
> logic into the main risu.c file and avoid having copy and pasted
> functions for each architecture.
>
> I've also introduced a header which makes tracking where you are in
> the recorded stream a little easier when debugging.
>
> It should be noted the functions:
>
> int send_register_info(write_fn write_fn, void *uc);
> int recv_and_compare_register_info(read_fn read_fn, respond_fn respond, void *uc);
>
> are pretty identical for each arch. I did consider folding even more
> of the logic into the common code but I wasn't sure if that would
> loose flexibility in the future for other architectures so I left it
> as is.
>
> I've make the same changes to ppc64 although I've been compiling and
> testing under a qemu linux-user environment which might explain why
> I'm seeing crashes when I try and run it. I would appreciate the PPC
> guys trying it on real hardware and debugging if needed.
>
> For reference I'm using a linux-user powered docker image (built with
> the debian-bootstrap recipe in the qemu.git tree) for cross compiling:
>
> docker run --rm -it -v /home/alex/lsrc/qemu/risu.git:/src --user=alex:alex -w /src debian:ppc64el make
>
> There have been some minor clean-ups to the helper scripts mainly to
> aid testing by allowing the override of the RISU binary (I have
> risu-arm, risu-arm64 and risu-ppc64 in my development directory).
> Notes in the --- comments on each patch.
This generally looks good -- I've applied the two trivial typo
fixes, and commented on the other patches except for the new
scripts. I don't think there's much more work to do to get these
applied.
Could we have a patch which updates README to document the
new functionality?
I won't have a chance to look at the scripts until next year, I
expect, but from a quick glance they seem to be reasonably
separate from the actual record/replay implementation.
thanks
-- PMM
^ permalink raw reply [flat|nested] 20+ messages in thread* Re: [Qemu-devel] [RISU PATCH v3 00/10] Record/payback patches
2016-12-09 11:48 [Qemu-devel] [RISU PATCH v3 00/10] Record/payback patches Alex Bennée
` (10 preceding siblings ...)
2016-12-16 19:09 ` [Qemu-devel] [RISU PATCH v3 00/10] Record/payback patches Peter Maydell
@ 2016-12-23 1:41 ` joserz
11 siblings, 0 replies; 20+ messages in thread
From: joserz @ 2016-12-23 1:41 UTC (permalink / raw)
To: Alex Bennée; +Cc: peter.maydell, qemu-devel
On Fri, Dec 09, 2016 at 11:48:20AM +0000, Alex Bennée wrote:
> Hi,
>
> I did a bunch of tweaking to see if I could abstract the hackage for
> record/replay a bit more. With a little bit of light re-factoring of
> the send and recv functions I can move all the specific send/recv
> logic into the main risu.c file and avoid having copy and pasted
> functions for each architecture.
>
> I've also introduced a header which makes tracking where you are in
> the recorded stream a little easier when debugging.
>
> It should be noted the functions:
>
> int send_register_info(write_fn write_fn, void *uc);
> int recv_and_compare_register_info(read_fn read_fn, respond_fn respond, void *uc);
>
> are pretty identical for each arch. I did consider folding even more
> of the logic into the common code but I wasn't sure if that would
> loose flexibility in the future for other architectures so I left it
> as is.
>
> I've make the same changes to ppc64 although I've been compiling and
> testing under a qemu linux-user environment which might explain why
> I'm seeing crashes when I try and run it. I would appreciate the PPC
> guys trying it on real hardware and debugging if needed.
Sure, I'm starting working on it. I'll try the real hardware, softmmu, and linux-user. I have an idea about what leads to the crash (already have a fix for it). As soon as I have your feature working for ppc I'll send the patches here.
Thanks Alex!
>
> For reference I'm using a linux-user powered docker image (built with
> the debian-bootstrap recipe in the qemu.git tree) for cross compiling:
>
> docker run --rm -it -v /home/alex/lsrc/qemu/risu.git:/src --user=alex:alex -w /src debian:ppc64el make
>
> There have been some minor clean-ups to the helper scripts mainly to
> aid testing by allowing the override of the RISU binary (I have
> risu-arm, risu-arm64 and risu-ppc64 in my development directory).
> Notes in the --- comments on each patch.
>
> Alex Bennée (10):
> risu: a bit more verbosity when running
> aarch64: add hand-coded risu skeleton for directed testing
> risu: paramterise send/receive functions
> risu: add simple trace and replay support
> risu: add support compressed tracefiles
> risu_aarch64: it's -> it is
> risugen: remove grocer's apostrophe from REs
> new: generate_all.sh script
> new: record_traces.sh helper script
> new: run_risu.sh script
>
> Makefile | 10 +-
> aarch64_simd_handcoded.risu.S | 208 ++++++++++++++++++++++++++++++++++++++++++
> configure | 55 +++++++++++
> generate_all.sh | 55 +++++++++++
> record_traces.sh | 20 ++++
> risu.c | 139 ++++++++++++++++++++++++----
> risu.h | 14 ++-
> risu_aarch64.c | 119 +++++++++++++++---------
> risu_arm.c | 148 +++++++++++++++++++-----------
> risu_ppc64le.c | 127 ++++++++++++++++----------
> risu_reginfo_aarch64.h | 7 ++
> risu_reginfo_arm.h | 6 ++
> risu_reginfo_ppc64le.h | 6 ++
> risugen | 2 +-
> run_risu.sh | 53 +++++++++++
> 15 files changed, 805 insertions(+), 164 deletions(-)
> create mode 100644 aarch64_simd_handcoded.risu.S
> create mode 100755 generate_all.sh
> create mode 100755 record_traces.sh
> create mode 100755 run_risu.sh
>
> --
> 2.11.0
>
^ permalink raw reply [flat|nested] 20+ messages in thread