* [RISU v2 01/17] Use bool for tracing variables
2020-05-19 2:53 [RISU v2 00/17] risu cleanups and improvements Richard Henderson
@ 2020-05-19 2:53 ` Richard Henderson
2020-05-19 15:52 ` Alex Bennée
2020-05-19 2:53 ` [RISU v2 02/17] Unify master_fd and apprentice_fd to comm_fd Richard Henderson
` (15 subsequent siblings)
16 siblings, 1 reply; 36+ messages in thread
From: Richard Henderson @ 2020-05-19 2:53 UTC (permalink / raw)
To: qemu-devel; +Cc: peter.maydell, alex.bennee
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
risu.h | 3 ++-
reginfo.c | 2 +-
risu.c | 8 ++++----
3 files changed, 7 insertions(+), 6 deletions(-)
diff --git a/risu.h b/risu.h
index 8d2d646..e2b4508 100644
--- a/risu.h
+++ b/risu.h
@@ -17,6 +17,7 @@
#include <ucontext.h>
#include <stdio.h>
#include <getopt.h>
+#include <stdbool.h>
/* Extra option processing for architectures */
extern const struct option * const arch_long_opts;
@@ -96,7 +97,7 @@ int recv_and_compare_register_info(read_fn read_fn,
* Should return 0 if it was a good match (ie end of test)
* and 1 for a mismatch.
*/
-int report_match_status(int trace);
+int report_match_status(bool trace);
/* Interface provided by CPU-specific code: */
diff --git a/reginfo.c b/reginfo.c
index dd42ae2..1b2a821 100644
--- a/reginfo.c
+++ b/reginfo.c
@@ -141,7 +141,7 @@ int recv_and_compare_register_info(read_fn read_fn,
* Should return 0 if it was a good match (ie end of test)
* and 1 for a mismatch.
*/
-int report_match_status(int trace)
+int report_match_status(bool trace)
{
int resp = 0;
fprintf(stderr, "match status...\n");
diff --git a/risu.c b/risu.c
index 01525d2..79b1092 100644
--- a/risu.c
+++ b/risu.c
@@ -31,7 +31,7 @@
void *memblock;
int apprentice_fd, master_fd;
-int trace;
+bool trace;
size_t signal_count;
#ifdef HAVE_ZLIB
@@ -228,7 +228,7 @@ int master(void)
signal_count);
return 0;
} else {
- return report_match_status(0);
+ return report_match_status(false);
}
}
set_sigill_handler(&master_sigill);
@@ -250,7 +250,7 @@ int apprentice(void)
#endif
close(apprentice_fd);
fprintf(stderr, "finished early after %zd checkpoints\n", signal_count);
- return report_match_status(1);
+ return report_match_status(true);
}
set_sigill_handler(&apprentice_sigill);
fprintf(stderr, "starting apprentice image at 0x%"PRIxPTR"\n",
@@ -344,7 +344,7 @@ int main(int argc, char **argv)
break;
case 't':
trace_fn = optarg;
- trace = 1;
+ trace = true;
break;
case 'h':
hostname = optarg;
--
2.20.1
^ permalink raw reply related [flat|nested] 36+ messages in thread* [RISU v2 02/17] Unify master_fd and apprentice_fd to comm_fd
2020-05-19 2:53 [RISU v2 00/17] risu cleanups and improvements Richard Henderson
2020-05-19 2:53 ` [RISU v2 01/17] Use bool for tracing variables Richard Henderson
@ 2020-05-19 2:53 ` Richard Henderson
2020-05-19 15:52 ` Alex Bennée
2020-05-19 2:53 ` [RISU v2 03/17] Hoist trace file opening Richard Henderson
` (14 subsequent siblings)
16 siblings, 1 reply; 36+ messages in thread
From: Richard Henderson @ 2020-05-19 2:53 UTC (permalink / raw)
To: qemu-devel; +Cc: peter.maydell, alex.bennee
Any one invocation cannot be both master and apprentice.
Let's use only one variable for the file descriptor.
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
risu.c | 40 ++++++++++++++++++++--------------------
1 file changed, 20 insertions(+), 20 deletions(-)
diff --git a/risu.c b/risu.c
index 79b1092..059348f 100644
--- a/risu.c
+++ b/risu.c
@@ -30,7 +30,7 @@
void *memblock;
-int apprentice_fd, master_fd;
+static int comm_fd;
bool trace;
size_t signal_count;
@@ -50,7 +50,7 @@ sigjmp_buf jmpbuf;
int read_sock(void *ptr, size_t bytes)
{
- return recv_data_pkt(master_fd, ptr, bytes);
+ return recv_data_pkt(comm_fd, ptr, bytes);
}
int write_trace(void *ptr, size_t bytes)
@@ -58,9 +58,9 @@ int write_trace(void *ptr, size_t bytes)
size_t res;
#ifdef HAVE_ZLIB
- if (master_fd == STDOUT_FILENO) {
+ if (comm_fd == STDOUT_FILENO) {
#endif
- res = write(master_fd, ptr, bytes);
+ res = write(comm_fd, ptr, bytes);
#ifdef HAVE_ZLIB
} else {
res = gzwrite(gz_trace_file, ptr, bytes);
@@ -71,14 +71,14 @@ int write_trace(void *ptr, size_t bytes)
void respond_sock(int r)
{
- send_response_byte(master_fd, r);
+ send_response_byte(comm_fd, r);
}
/* Apprentice function */
int write_sock(void *ptr, size_t bytes)
{
- return send_data_pkt(apprentice_fd, ptr, bytes);
+ return send_data_pkt(comm_fd, ptr, bytes);
}
int read_trace(void *ptr, size_t bytes)
@@ -86,9 +86,9 @@ int read_trace(void *ptr, size_t bytes)
size_t res;
#ifdef HAVE_ZLIB
- if (apprentice_fd == STDIN_FILENO) {
+ if (comm_fd == STDIN_FILENO) {
#endif
- res = read(apprentice_fd, ptr, bytes);
+ res = read(comm_fd, ptr, bytes);
#ifdef HAVE_ZLIB
} else {
res = gzread(gz_trace_file, ptr, bytes);
@@ -218,11 +218,11 @@ int master(void)
{
if (sigsetjmp(jmpbuf, 1)) {
#ifdef HAVE_ZLIB
- if (trace && master_fd != STDOUT_FILENO) {
+ if (trace && comm_fd != STDOUT_FILENO) {
gzclose(gz_trace_file);
}
#endif
- close(master_fd);
+ close(comm_fd);
if (trace) {
fprintf(stderr, "trace complete after %zd checkpoints\n",
signal_count);
@@ -244,11 +244,11 @@ int apprentice(void)
{
if (sigsetjmp(jmpbuf, 1)) {
#ifdef HAVE_ZLIB
- if (trace && apprentice_fd != STDIN_FILENO) {
+ if (trace && comm_fd != STDIN_FILENO) {
gzclose(gz_trace_file);
}
#endif
- close(apprentice_fd);
+ close(comm_fd);
fprintf(stderr, "finished early after %zd checkpoints\n", signal_count);
return report_match_status(true);
}
@@ -375,31 +375,31 @@ int main(int argc, char **argv)
if (ismaster) {
if (trace) {
if (strcmp(trace_fn, "-") == 0) {
- master_fd = STDOUT_FILENO;
+ comm_fd = STDOUT_FILENO;
} else {
- master_fd = open(trace_fn, O_WRONLY | O_CREAT, S_IRWXU);
+ comm_fd = open(trace_fn, O_WRONLY | O_CREAT, S_IRWXU);
#ifdef HAVE_ZLIB
- gz_trace_file = gzdopen(master_fd, "wb9");
+ gz_trace_file = gzdopen(comm_fd, "wb9");
#endif
}
} else {
fprintf(stderr, "master port %d\n", port);
- master_fd = master_connect(port);
+ comm_fd = master_connect(port);
}
return master();
} else {
if (trace) {
if (strcmp(trace_fn, "-") == 0) {
- apprentice_fd = STDIN_FILENO;
+ comm_fd = STDIN_FILENO;
} else {
- apprentice_fd = open(trace_fn, O_RDONLY);
+ comm_fd = open(trace_fn, O_RDONLY);
#ifdef HAVE_ZLIB
- gz_trace_file = gzdopen(apprentice_fd, "rb");
+ gz_trace_file = gzdopen(comm_fd, "rb");
#endif
}
} else {
fprintf(stderr, "apprentice host %s port %d\n", hostname, port);
- apprentice_fd = apprentice_connect(hostname, port);
+ comm_fd = apprentice_connect(hostname, port);
}
return apprentice();
}
--
2.20.1
^ permalink raw reply related [flat|nested] 36+ messages in thread* [RISU v2 03/17] Hoist trace file opening
2020-05-19 2:53 [RISU v2 00/17] risu cleanups and improvements Richard Henderson
2020-05-19 2:53 ` [RISU v2 01/17] Use bool for tracing variables Richard Henderson
2020-05-19 2:53 ` [RISU v2 02/17] Unify master_fd and apprentice_fd to comm_fd Richard Henderson
@ 2020-05-19 2:53 ` Richard Henderson
2020-05-19 16:50 ` Alex Bennée
2020-05-19 2:53 ` [RISU v2 04/17] Adjust tracefile open for write Richard Henderson
` (13 subsequent siblings)
16 siblings, 1 reply; 36+ messages in thread
From: Richard Henderson @ 2020-05-19 2:53 UTC (permalink / raw)
To: qemu-devel; +Cc: peter.maydell, alex.bennee
We will want to share this code with --dump.
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
risu.c | 37 +++++++++++++++++--------------------
1 file changed, 17 insertions(+), 20 deletions(-)
diff --git a/risu.c b/risu.c
index 059348f..1c66885 100644
--- a/risu.c
+++ b/risu.c
@@ -363,6 +363,21 @@ int main(int argc, char **argv)
}
}
+ if (trace) {
+ if (strcmp(trace_fn, "-") == 0) {
+ comm_fd = ismaster ? STDOUT_FILENO : STDIN_FILENO;
+ } else {
+ if (ismaster) {
+ comm_fd = open(trace_fn, O_WRONLY | O_CREAT, S_IRWXU);
+ } else {
+ comm_fd = open(trace_fn, O_RDONLY);
+ }
+#ifdef HAVE_ZLIB
+ gz_trace_file = gzdopen(comm_fd, ismaster ? "wb9" : "rb");
+#endif
+ }
+ }
+
imgfile = argv[optind];
if (!imgfile) {
fprintf(stderr, "Error: must specify image file name\n\n");
@@ -373,31 +388,13 @@ int main(int argc, char **argv)
load_image(imgfile);
if (ismaster) {
- if (trace) {
- if (strcmp(trace_fn, "-") == 0) {
- comm_fd = STDOUT_FILENO;
- } else {
- comm_fd = open(trace_fn, O_WRONLY | O_CREAT, S_IRWXU);
-#ifdef HAVE_ZLIB
- gz_trace_file = gzdopen(comm_fd, "wb9");
-#endif
- }
- } else {
+ if (!trace) {
fprintf(stderr, "master port %d\n", port);
comm_fd = master_connect(port);
}
return master();
} else {
- if (trace) {
- if (strcmp(trace_fn, "-") == 0) {
- comm_fd = STDIN_FILENO;
- } else {
- comm_fd = open(trace_fn, O_RDONLY);
-#ifdef HAVE_ZLIB
- gz_trace_file = gzdopen(comm_fd, "rb");
-#endif
- }
- } else {
+ if (!trace) {
fprintf(stderr, "apprentice host %s port %d\n", hostname, port);
comm_fd = apprentice_connect(hostname, port);
}
--
2.20.1
^ permalink raw reply related [flat|nested] 36+ messages in thread* Re: [RISU v2 03/17] Hoist trace file opening
2020-05-19 2:53 ` [RISU v2 03/17] Hoist trace file opening Richard Henderson
@ 2020-05-19 16:50 ` Alex Bennée
0 siblings, 0 replies; 36+ messages in thread
From: Alex Bennée @ 2020-05-19 16:50 UTC (permalink / raw)
To: Richard Henderson; +Cc: peter.maydell, qemu-devel
Richard Henderson <richard.henderson@linaro.org> writes:
> We will want to share this code with --dump.
>
> Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
> ---
> risu.c | 37 +++++++++++++++++--------------------
> 1 file changed, 17 insertions(+), 20 deletions(-)
>
> diff --git a/risu.c b/risu.c
> index 059348f..1c66885 100644
> --- a/risu.c
> +++ b/risu.c
> @@ -363,6 +363,21 @@ int main(int argc, char **argv)
> }
> }
>
> + if (trace) {
> + if (strcmp(trace_fn, "-") == 0) {
> + comm_fd = ismaster ? STDOUT_FILENO : STDIN_FILENO;
> + } else {
> + if (ismaster) {
> + comm_fd = open(trace_fn, O_WRONLY | O_CREAT, S_IRWXU);
> + } else {
> + comm_fd = open(trace_fn, O_RDONLY);
> + }
> +#ifdef HAVE_ZLIB
> + gz_trace_file = gzdopen(comm_fd, ismaster ? "wb9" : "rb");
> +#endif
> + }
> + }
> +
> imgfile = argv[optind];
> if (!imgfile) {
> fprintf(stderr, "Error: must specify image file name\n\n");
> @@ -373,31 +388,13 @@ int main(int argc, char **argv)
> load_image(imgfile);
>
> if (ismaster) {
> - if (trace) {
> - if (strcmp(trace_fn, "-") == 0) {
> - comm_fd = STDOUT_FILENO;
> - } else {
> - comm_fd = open(trace_fn, O_WRONLY | O_CREAT, S_IRWXU);
> -#ifdef HAVE_ZLIB
> - gz_trace_file = gzdopen(comm_fd, "wb9");
> -#endif
> - }
> - } else {
> + if (!trace) {
> fprintf(stderr, "master port %d\n", port);
> comm_fd = master_connect(port);
> }
> return master();
> } else {
> - if (trace) {
> - if (strcmp(trace_fn, "-") == 0) {
> - comm_fd = STDIN_FILENO;
> - } else {
> - comm_fd = open(trace_fn, O_RDONLY);
> -#ifdef HAVE_ZLIB
> - gz_trace_file = gzdopen(comm_fd, "rb");
> -#endif
> - }
> - } else {
> + if (!trace) {
> fprintf(stderr, "apprentice host %s port %d\n", hostname, port);
> comm_fd = apprentice_connect(hostname, port);
> }
--
Alex Bennée
^ permalink raw reply [flat|nested] 36+ messages in thread
* [RISU v2 04/17] Adjust tracefile open for write
2020-05-19 2:53 [RISU v2 00/17] risu cleanups and improvements Richard Henderson
` (2 preceding siblings ...)
2020-05-19 2:53 ` [RISU v2 03/17] Hoist trace file opening Richard Henderson
@ 2020-05-19 2:53 ` Richard Henderson
2020-05-19 18:24 ` Alex Bennée
2020-05-19 2:53 ` [RISU v2 05/17] Use EXIT_FAILURE, EXIT_SUCCESS Richard Henderson
` (12 subsequent siblings)
16 siblings, 1 reply; 36+ messages in thread
From: Richard Henderson @ 2020-05-19 2:53 UTC (permalink / raw)
To: qemu-devel; +Cc: peter.maydell, alex.bennee
Truncate the new output file. Rely on umask to remove
group+other file permissions, if desired.
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
risu.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/risu.c b/risu.c
index 1c66885..f404d8f 100644
--- a/risu.c
+++ b/risu.c
@@ -368,7 +368,7 @@ int main(int argc, char **argv)
comm_fd = ismaster ? STDOUT_FILENO : STDIN_FILENO;
} else {
if (ismaster) {
- comm_fd = open(trace_fn, O_WRONLY | O_CREAT, S_IRWXU);
+ comm_fd = open(trace_fn, O_WRONLY | O_CREAT | O_TRUNC, 0666);
} else {
comm_fd = open(trace_fn, O_RDONLY);
}
--
2.20.1
^ permalink raw reply related [flat|nested] 36+ messages in thread* Re: [RISU v2 04/17] Adjust tracefile open for write
2020-05-19 2:53 ` [RISU v2 04/17] Adjust tracefile open for write Richard Henderson
@ 2020-05-19 18:24 ` Alex Bennée
0 siblings, 0 replies; 36+ messages in thread
From: Alex Bennée @ 2020-05-19 18:24 UTC (permalink / raw)
To: Richard Henderson; +Cc: peter.maydell, qemu-devel
Richard Henderson <richard.henderson@linaro.org> writes:
> Truncate the new output file. Rely on umask to remove
> group+other file permissions, if desired.
>
> Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
> ---
> risu.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/risu.c b/risu.c
> index 1c66885..f404d8f 100644
> --- a/risu.c
> +++ b/risu.c
> @@ -368,7 +368,7 @@ int main(int argc, char **argv)
> comm_fd = ismaster ? STDOUT_FILENO : STDIN_FILENO;
> } else {
> if (ismaster) {
> - comm_fd = open(trace_fn, O_WRONLY | O_CREAT, S_IRWXU);
> + comm_fd = open(trace_fn, O_WRONLY | O_CREAT | O_TRUNC, 0666);
> } else {
> comm_fd = open(trace_fn, O_RDONLY);
> }
--
Alex Bennée
^ permalink raw reply [flat|nested] 36+ messages in thread
* [RISU v2 05/17] Use EXIT_FAILURE, EXIT_SUCCESS
2020-05-19 2:53 [RISU v2 00/17] risu cleanups and improvements Richard Henderson
` (3 preceding siblings ...)
2020-05-19 2:53 ` [RISU v2 04/17] Adjust tracefile open for write Richard Henderson
@ 2020-05-19 2:53 ` Richard Henderson
2020-05-19 18:24 ` Alex Bennée
2020-05-19 2:53 ` [RISU v2 06/17] Make some risu.c symbols static Richard Henderson
` (11 subsequent siblings)
16 siblings, 1 reply; 36+ messages in thread
From: Richard Henderson @ 2020-05-19 2:53 UTC (permalink / raw)
To: qemu-devel; +Cc: peter.maydell, alex.bennee
Some of the time we exit via the return value from main.
This can make it easier to tell what it is we're returning.
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
comms.c | 26 +++++++++++++-------------
risu.c | 22 +++++++++++-----------
risu_reginfo_aarch64.c | 4 ++--
risu_reginfo_i386.c | 2 +-
4 files changed, 27 insertions(+), 27 deletions(-)
diff --git a/comms.c b/comms.c
index 6946fd9..861e845 100644
--- a/comms.c
+++ b/comms.c
@@ -31,7 +31,7 @@ int apprentice_connect(const char *hostname, int port)
sock = socket(PF_INET, SOCK_STREAM, 0);
if (sock < 0) {
perror("socket");
- exit(1);
+ exit(EXIT_FAILURE);
}
struct hostent *hostinfo;
sa.sin_family = AF_INET;
@@ -39,12 +39,12 @@ int apprentice_connect(const char *hostname, int port)
hostinfo = gethostbyname(hostname);
if (!hostinfo) {
fprintf(stderr, "Unknown host %s\n", hostname);
- exit(1);
+ exit(EXIT_FAILURE);
}
sa.sin_addr = *(struct in_addr *) hostinfo->h_addr;
if (connect(sock, (struct sockaddr *) &sa, sizeof(sa)) < 0) {
perror("connect");
- exit(1);
+ exit(EXIT_FAILURE);
}
return sock;
}
@@ -56,13 +56,13 @@ int master_connect(int port)
sock = socket(PF_INET, SOCK_STREAM, 0);
if (sock < 0) {
perror("socket");
- exit(1);
+ exit(EXIT_FAILURE);
}
int sora = 1;
if (setsockopt(sock, SOL_SOCKET, SO_REUSEADDR, &sora, sizeof(sora)) !=
0) {
perror("setsockopt(SO_REUSEADDR)");
- exit(1);
+ exit(EXIT_FAILURE);
}
sa.sin_family = AF_INET;
@@ -70,11 +70,11 @@ int master_connect(int port)
sa.sin_addr.s_addr = htonl(INADDR_ANY);
if (bind(sock, (struct sockaddr *) &sa, sizeof(sa)) < 0) {
perror("bind");
- exit(1);
+ exit(EXIT_FAILURE);
}
if (listen(sock, 1) < 0) {
perror("listen");
- exit(1);
+ exit(EXIT_FAILURE);
}
/* Just block until we get a connection */
fprintf(stderr, "master: waiting for connection on port %d...\n",
@@ -84,7 +84,7 @@ int master_connect(int port)
int nsock = accept(sock, (struct sockaddr *) &csa, &csasz);
if (nsock < 0) {
perror("accept");
- exit(1);
+ exit(EXIT_FAILURE);
}
/* We're done with the server socket now */
close(sock);
@@ -104,7 +104,7 @@ static void recv_bytes(int sock, void *pkt, int pktlen)
continue;
}
perror("read failed");
- exit(1);
+ exit(EXIT_FAILURE);
}
pktlen -= i;
p += i;
@@ -127,7 +127,7 @@ static void recv_and_discard_bytes(int sock, int pktlen)
continue;
}
perror("read failed");
- exit(1);
+ exit(EXIT_FAILURE);
}
pktlen -= i;
}
@@ -186,12 +186,12 @@ int send_data_pkt(int sock, void *pkt, int pktlen)
if (safe_writev(sock, iov, 2) == -1) {
perror("writev failed");
- exit(1);
+ exit(EXIT_FAILURE);
}
if (read(sock, &resp, 1) != 1) {
perror("read failed");
- exit(1);
+ exit(EXIT_FAILURE);
}
return resp;
}
@@ -217,6 +217,6 @@ void send_response_byte(int sock, int resp)
unsigned char r = resp;
if (write(sock, &r, 1) != 1) {
perror("write failed");
- exit(1);
+ exit(EXIT_FAILURE);
}
}
diff --git a/risu.c b/risu.c
index f404d8f..979341c 100644
--- a/risu.c
+++ b/risu.c
@@ -153,13 +153,13 @@ void apprentice_sigill(int sig, siginfo_t *si, void *uc)
return;
case 1:
/* end of test */
- exit(0);
+ exit(EXIT_SUCCESS);
default:
/* mismatch */
if (trace) {
siglongjmp(jmpbuf, 1);
}
- exit(1);
+ exit(EXIT_FAILURE);
}
}
@@ -173,7 +173,7 @@ static void set_sigill_handler(void (*fn) (int, siginfo_t *, void *))
sigemptyset(&sa.sa_mask);
if (sigaction(SIGILL, &sa, 0) != 0) {
perror("sigaction");
- exit(1);
+ exit(EXIT_FAILURE);
}
}
@@ -190,11 +190,11 @@ void load_image(const char *imgfile)
int fd = open(imgfile, O_RDONLY);
if (fd < 0) {
fprintf(stderr, "failed to open image file %s\n", imgfile);
- exit(1);
+ exit(EXIT_FAILURE);
}
if (fstat(fd, &st) != 0) {
perror("fstat");
- exit(1);
+ exit(EXIT_FAILURE);
}
size_t len = st.st_size;
void *addr;
@@ -207,7 +207,7 @@ void load_image(const char *imgfile)
0);
if (!addr) {
perror("mmap");
- exit(1);
+ exit(EXIT_FAILURE);
}
close(fd);
image_start = addr;
@@ -226,7 +226,7 @@ int master(void)
if (trace) {
fprintf(stderr, "trace complete after %zd checkpoints\n",
signal_count);
- return 0;
+ return EXIT_SUCCESS;
} else {
return report_match_status(false);
}
@@ -237,7 +237,7 @@ int master(void)
fprintf(stderr, "starting image\n");
image_start();
fprintf(stderr, "image returned unexpectedly\n");
- exit(1);
+ return EXIT_FAILURE;
}
int apprentice(void)
@@ -258,7 +258,7 @@ int apprentice(void)
fprintf(stderr, "starting image\n");
image_start();
fprintf(stderr, "image returned unexpectedly\n");
- exit(1);
+ return EXIT_FAILURE;
}
int ismaster;
@@ -355,7 +355,7 @@ int main(int argc, char **argv)
break;
case '?':
usage();
- exit(1);
+ return EXIT_FAILURE;
default:
assert(c >= FIRST_ARCH_OPT);
process_arch_opt(c, optarg);
@@ -382,7 +382,7 @@ int main(int argc, char **argv)
if (!imgfile) {
fprintf(stderr, "Error: must specify image file name\n\n");
usage();
- exit(1);
+ return EXIT_FAILURE;
}
load_image(imgfile);
diff --git a/risu_reginfo_aarch64.c b/risu_reginfo_aarch64.c
index 00d1c8b..028c690 100644
--- a/risu_reginfo_aarch64.c
+++ b/risu_reginfo_aarch64.c
@@ -51,7 +51,7 @@ void process_arch_opt(int opt, const char *arg)
if (test_sve <= 0 || test_sve > SVE_VQ_MAX) {
fprintf(stderr, "Invalid value for VQ (1-%d)\n", SVE_VQ_MAX);
- exit(1);
+ exit(EXIT_FAILURE);
}
want = sve_vl_from_vq(test_sve);
got = prctl(PR_SVE_SET_VL, want);
@@ -62,7 +62,7 @@ void process_arch_opt(int opt, const char *arg)
fprintf(stderr, "Unsupported value for VQ (%d != %d)\n",
test_sve, (int)sve_vq_from_vl(got));
}
- exit(1);
+ exit(EXIT_FAILURE);
}
#else
abort();
diff --git a/risu_reginfo_i386.c b/risu_reginfo_i386.c
index 194e0ad..60fc239 100644
--- a/risu_reginfo_i386.c
+++ b/risu_reginfo_i386.c
@@ -69,7 +69,7 @@ void process_arch_opt(int opt, const char *arg)
fprintf(stderr,
"Unable to parse '%s' in '%s' into an xfeatures integer mask\n",
endptr, arg);
- exit(1);
+ exit(EXIT_FAILURE);
}
}
}
--
2.20.1
^ permalink raw reply related [flat|nested] 36+ messages in thread* [RISU v2 06/17] Make some risu.c symbols static
2020-05-19 2:53 [RISU v2 00/17] risu cleanups and improvements Richard Henderson
` (4 preceding siblings ...)
2020-05-19 2:53 ` [RISU v2 05/17] Use EXIT_FAILURE, EXIT_SUCCESS Richard Henderson
@ 2020-05-19 2:53 ` Richard Henderson
2020-05-19 18:25 ` Alex Bennée
2020-05-19 2:53 ` [RISU v2 07/17] Add enum RisuOp Richard Henderson
` (10 subsequent siblings)
16 siblings, 1 reply; 36+ messages in thread
From: Richard Henderson @ 2020-05-19 2:53 UTC (permalink / raw)
To: qemu-devel; +Cc: peter.maydell, alex.bennee
These are unused in other translation units.
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
risu.c | 24 ++++++++++++------------
1 file changed, 12 insertions(+), 12 deletions(-)
diff --git a/risu.c b/risu.c
index 979341c..24ada8f 100644
--- a/risu.c
+++ b/risu.c
@@ -31,18 +31,18 @@
void *memblock;
static int comm_fd;
-bool trace;
-size_t signal_count;
+static bool trace;
+static size_t signal_count;
#ifdef HAVE_ZLIB
#include <zlib.h>
-gzFile gz_trace_file;
+static gzFile gz_trace_file;
#define TRACE_TYPE "compressed"
#else
#define TRACE_TYPE "uncompressed"
#endif
-sigjmp_buf jmpbuf;
+static sigjmp_buf jmpbuf;
#define ARRAY_SIZE(x) (sizeof(x) / sizeof((x)[0]))
@@ -113,7 +113,7 @@ void respond_trace(int r)
}
}
-void master_sigill(int sig, siginfo_t *si, void *uc)
+static void master_sigill(int sig, siginfo_t *si, void *uc)
{
int r;
signal_count++;
@@ -135,7 +135,7 @@ void master_sigill(int sig, siginfo_t *si, void *uc)
}
}
-void apprentice_sigill(int sig, siginfo_t *si, void *uc)
+static void apprentice_sigill(int sig, siginfo_t *si, void *uc)
{
int r;
signal_count++;
@@ -180,9 +180,9 @@ static void set_sigill_handler(void (*fn) (int, siginfo_t *, void *))
typedef void entrypoint_fn(void);
uintptr_t image_start_address;
-entrypoint_fn *image_start;
+static entrypoint_fn *image_start;
-void load_image(const char *imgfile)
+static void load_image(const char *imgfile)
{
/* Load image file into memory as executable */
struct stat st;
@@ -214,7 +214,7 @@ void load_image(const char *imgfile)
image_start_address = (uintptr_t) addr;
}
-int master(void)
+static int master(void)
{
if (sigsetjmp(jmpbuf, 1)) {
#ifdef HAVE_ZLIB
@@ -240,7 +240,7 @@ int master(void)
return EXIT_FAILURE;
}
-int apprentice(void)
+static int apprentice(void)
{
if (sigsetjmp(jmpbuf, 1)) {
#ifdef HAVE_ZLIB
@@ -261,9 +261,9 @@ int apprentice(void)
return EXIT_FAILURE;
}
-int ismaster;
+static int ismaster;
-void usage(void)
+static void usage(void)
{
fprintf(stderr,
"Usage: risu [--master] [--host <ip>] [--port <port>] <image file>"
--
2.20.1
^ permalink raw reply related [flat|nested] 36+ messages in thread* [RISU v2 07/17] Add enum RisuOp
2020-05-19 2:53 [RISU v2 00/17] risu cleanups and improvements Richard Henderson
` (5 preceding siblings ...)
2020-05-19 2:53 ` [RISU v2 06/17] Make some risu.c symbols static Richard Henderson
@ 2020-05-19 2:53 ` Richard Henderson
2020-05-19 18:39 ` Alex Bennée
2020-05-19 2:53 ` [RISU v2 08/17] Add enum RisuResult Richard Henderson
` (9 subsequent siblings)
16 siblings, 1 reply; 36+ messages in thread
From: Richard Henderson @ 2020-05-19 2:53 UTC (permalink / raw)
To: qemu-devel; +Cc: peter.maydell, alex.bennee
Formalize the set of defines, plus -1, into an enum.
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
risu.h | 23 +++++++++++++++--------
reginfo.c | 32 +++++++++++++++++++-------------
risu_aarch64.c | 6 +++---
risu_arm.c | 6 +++---
risu_i386.c | 4 ++--
risu_m68k.c | 4 ++--
risu_ppc64.c | 4 ++--
7 files changed, 46 insertions(+), 33 deletions(-)
diff --git a/risu.h b/risu.h
index e2b4508..a7aa929 100644
--- a/risu.h
+++ b/risu.h
@@ -45,11 +45,17 @@ extern uintptr_t image_start_address;
extern void *memblock;
/* Ops code under test can request from risu: */
-#define OP_COMPARE 0
-#define OP_TESTEND 1
-#define OP_SETMEMBLOCK 2
-#define OP_GETMEMBLOCK 3
-#define OP_COMPAREMEM 4
+typedef enum {
+ /* Any other sigill besides the destignated undefined insn. */
+ OP_SIGILL = -1,
+
+ /* These are generated by the designated undefined insn. */
+ OP_COMPARE = 0,
+ OP_TESTEND = 1,
+ OP_SETMEMBLOCK = 2,
+ OP_GETMEMBLOCK = 3,
+ OP_COMPAREMEM = 4,
+} RisuOp;
/* The memory block should be this long */
#define MEMBLOCKLEN 8192
@@ -114,10 +120,11 @@ void set_ucontext_paramreg(void *vuc, uint64_t value);
/* Return the value of the parameter register from a reginfo. */
uint64_t get_reginfo_paramreg(struct reginfo *ri);
-/* Return the risu operation number we have been asked to do,
- * or -1 if this was a SIGILL for a non-risuop insn.
+/*
+ * Return the risu operation number we have been asked to do,
+ * or OP_SIGILL if this was a SIGILL for a non-risuop insn.
*/
-int get_risuop(struct reginfo *ri);
+RisuOp get_risuop(struct reginfo *ri);
/* Return the PC from a reginfo */
uintptr_t get_pc(struct reginfo *ri);
diff --git a/reginfo.c b/reginfo.c
index 1b2a821..2d67c93 100644
--- a/reginfo.c
+++ b/reginfo.c
@@ -11,7 +11,7 @@
#include <stdio.h>
#include <string.h>
-
+#include <stdlib.h>
#include "risu.h"
struct reginfo master_ri, apprentice_ri;
@@ -25,7 +25,7 @@ int send_register_info(write_fn write_fn, void *uc)
{
struct reginfo ri;
trace_header_t header;
- int op;
+ RisuOp op;
reginfo_init(&ri, uc);
op = get_risuop(&ri);
@@ -38,11 +38,18 @@ int send_register_info(write_fn write_fn, void *uc)
}
switch (op) {
+ case OP_COMPARE:
case OP_TESTEND:
- write_fn(&ri, reginfo_size());
- /* if we are tracing write_fn will return 0 unlike a remote
- end, hence we force return of 1 here */
- return 1;
+ case OP_SIGILL:
+ /*
+ * Do a simple register compare on (a) explicit request
+ * (b) end of test (c) a non-risuop UNDEF
+ */
+ if (write_fn(&ri, reginfo_size()) != 0) {
+ return -1;
+ }
+ /* For OP_TEST_END, force return 1 to exit. */
+ return op == OP_TESTEND;
case OP_SETMEMBLOCK:
memblock = (void *)(uintptr_t)get_reginfo_paramreg(&ri);
break;
@@ -53,12 +60,8 @@ int send_register_info(write_fn write_fn, void *uc)
case OP_COMPAREMEM:
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, reginfo_size());
+ abort();
}
return 0;
}
@@ -74,8 +77,9 @@ int send_register_info(write_fn write_fn, void *uc)
int recv_and_compare_register_info(read_fn read_fn,
respond_fn resp_fn, void *uc)
{
- int resp = 0, op;
+ int resp = 0;
trace_header_t header;
+ RisuOp op;
reginfo_init(&master_ri, uc);
op = get_risuop(&master_ri);
@@ -97,7 +101,7 @@ int recv_and_compare_register_info(read_fn read_fn,
switch (op) {
case OP_COMPARE:
case OP_TESTEND:
- default:
+ case OP_SIGILL:
/* Do a simple register compare on (a) explicit request
* (b) end of test (c) a non-risuop UNDEF
*/
@@ -130,6 +134,8 @@ int recv_and_compare_register_info(read_fn read_fn,
}
resp_fn(resp);
break;
+ default:
+ abort();
}
return resp;
diff --git a/risu_aarch64.c b/risu_aarch64.c
index 492d141..f8a8412 100644
--- a/risu_aarch64.c
+++ b/risu_aarch64.c
@@ -29,16 +29,16 @@ uint64_t get_reginfo_paramreg(struct reginfo *ri)
return ri->regs[0];
}
-int get_risuop(struct reginfo *ri)
+RisuOp get_risuop(struct reginfo *ri)
{
/* Return the risuop we have been asked to do
- * (or -1 if this was a SIGILL for a non-risuop insn)
+ * (or OP_SIGILL if this was a SIGILL for a non-risuop insn)
*/
uint32_t insn = ri->faulting_insn;
uint32_t op = insn & 0xf;
uint32_t key = insn & ~0xf;
uint32_t risukey = 0x00005af0;
- return (key != risukey) ? -1 : op;
+ return (key != risukey) ? OP_SIGILL : op;
}
uintptr_t get_pc(struct reginfo *ri)
diff --git a/risu_arm.c b/risu_arm.c
index 5fcb2a5..a20bf73 100644
--- a/risu_arm.c
+++ b/risu_arm.c
@@ -56,17 +56,17 @@ uint64_t get_reginfo_paramreg(struct reginfo *ri)
return ri->gpreg[0];
}
-int get_risuop(struct reginfo *ri)
+RisuOp get_risuop(struct reginfo *ri)
{
/* Return the risuop we have been asked to do
- * (or -1 if this was a SIGILL for a non-risuop insn)
+ * (or OP_SIGILL if this was a SIGILL for a non-risuop insn)
*/
uint32_t insn = ri->faulting_insn;
int isz = ri->faulting_insn_size;
uint32_t op = insn & 0xf;
uint32_t key = insn & ~0xf;
uint32_t risukey = (isz == 2) ? 0xdee0 : 0xe7fe5af0;
- return (key != risukey) ? -1 : op;
+ return (key != risukey) ? OP_SIGILL : op;
}
uintptr_t get_pc(struct reginfo *ri)
diff --git a/risu_i386.c b/risu_i386.c
index 9962b8f..127e816 100644
--- a/risu_i386.c
+++ b/risu_i386.c
@@ -38,12 +38,12 @@ uint64_t get_reginfo_paramreg(struct reginfo *ri)
return ri->gregs[REG_E(AX)];
}
-int get_risuop(struct reginfo *ri)
+RisuOp get_risuop(struct reginfo *ri)
{
if ((ri->faulting_insn & 0xf8ffff) == 0xc0b90f) { /* UD1 %xxx,%eax */
return (ri->faulting_insn >> 16) & 7;
}
- return -1;
+ return OP_SIGILL;
}
uintptr_t get_pc(struct reginfo *ri)
diff --git a/risu_m68k.c b/risu_m68k.c
index 1056eef..acdd57a 100644
--- a/risu_m68k.c
+++ b/risu_m68k.c
@@ -25,13 +25,13 @@ uint64_t get_reginfo_paramreg(struct reginfo *ri)
return ri->gregs[R_A0];
}
-int get_risuop(struct reginfo *ri)
+RisuOp get_risuop(struct reginfo *ri)
{
uint32_t insn = ri->faulting_insn;
uint32_t op = insn & 0xf;
uint32_t key = insn & ~0xf;
uint32_t risukey = 0x4afc7000;
- return (key != risukey) ? -1 : op;
+ return (key != risukey) ? OP_SIGILL : op;
}
uintptr_t get_pc(struct reginfo *ri)
diff --git a/risu_ppc64.c b/risu_ppc64.c
index a3028f7..9df8d58 100644
--- a/risu_ppc64.c
+++ b/risu_ppc64.c
@@ -32,13 +32,13 @@ uint64_t get_reginfo_paramreg(struct reginfo *ri)
return ri->gregs[0];
}
-int get_risuop(struct reginfo *ri)
+RisuOp get_risuop(struct reginfo *ri)
{
uint32_t insn = ri->faulting_insn;
uint32_t op = insn & 0xf;
uint32_t key = insn & ~0xf;
uint32_t risukey = 0x00005af0;
- return (key != risukey) ? -1 : op;
+ return (key != risukey) ? OP_SIGILL : op;
}
uintptr_t get_pc(struct reginfo *ri)
--
2.20.1
^ permalink raw reply related [flat|nested] 36+ messages in thread* [RISU v2 08/17] Add enum RisuResult
2020-05-19 2:53 [RISU v2 00/17] risu cleanups and improvements Richard Henderson
` (6 preceding siblings ...)
2020-05-19 2:53 ` [RISU v2 07/17] Add enum RisuOp Richard Henderson
@ 2020-05-19 2:53 ` Richard Henderson
2020-05-19 19:06 ` Alex Bennée
2020-05-19 2:53 ` [RISU v2 09/17] Unify i/o functions and use RisuResult Richard Henderson
` (8 subsequent siblings)
16 siblings, 1 reply; 36+ messages in thread
From: Richard Henderson @ 2020-05-19 2:53 UTC (permalink / raw)
To: qemu-devel; +Cc: peter.maydell, alex.bennee
Formalize the random set of numbers into an enum. Doing this
makes it easy to see that one of the responses in
recv_and_compare_register_info was inconsistent.
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
risu.h | 25 +++++++++++++++++--------
reginfo.c | 32 ++++++++++++++++----------------
risu.c | 18 +++++++++---------
3 files changed, 42 insertions(+), 33 deletions(-)
diff --git a/risu.h b/risu.h
index a7aa929..e6d07eb 100644
--- a/risu.h
+++ b/risu.h
@@ -57,6 +57,14 @@ typedef enum {
OP_COMPAREMEM = 4,
} RisuOp;
+/* Result of operation */
+typedef enum {
+ RES_OK = 0,
+ RES_END,
+ RES_MISMATCH,
+ RES_BAD_IO,
+} RisuResult;
+
/* The memory block should be this long */
#define MEMBLOCKLEN 8192
@@ -82,20 +90,21 @@ typedef struct {
*/
typedef int (*write_fn) (void *ptr, size_t bytes);
typedef int (*read_fn) (void *ptr, size_t bytes);
-typedef void (*respond_fn) (int response);
+typedef void (*respond_fn) (RisuResult response);
-/* Send the register information from the struct ucontext down the socket.
- * Return the response code from the master.
+/*
+ * Send the register information from the struct ucontext down the socket.
* NB: called from a signal handler.
*/
-int send_register_info(write_fn write_fn, void *uc);
+RisuResult 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.
+/*
+ * Read register info from the socket and compare it with that from the
+ * ucontext.
* NB: called from a signal handler.
*/
-int recv_and_compare_register_info(read_fn read_fn,
- respond_fn respond, void *uc);
+RisuResult 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/reginfo.c b/reginfo.c
index 2d67c93..b909a1f 100644
--- a/reginfo.c
+++ b/reginfo.c
@@ -21,7 +21,7 @@ uint8_t apprentice_memblock[MEMBLOCKLEN];
static int mem_used;
static int packet_mismatch;
-int send_register_info(write_fn write_fn, void *uc)
+RisuResult send_register_info(write_fn write_fn, void *uc)
{
struct reginfo ri;
trace_header_t header;
@@ -34,7 +34,7 @@ int send_register_info(write_fn write_fn, void *uc)
header.pc = get_pc(&ri);
header.risu_op = op;
if (write_fn(&header, sizeof(header)) != 0) {
- return -1;
+ return RES_BAD_IO;
}
switch (op) {
@@ -46,10 +46,10 @@ int send_register_info(write_fn write_fn, void *uc)
* (b) end of test (c) a non-risuop UNDEF
*/
if (write_fn(&ri, reginfo_size()) != 0) {
- return -1;
+ return RES_BAD_IO;
}
/* For OP_TEST_END, force return 1 to exit. */
- return op == OP_TESTEND;
+ return op == OP_TESTEND ? RES_END : RES_OK;
case OP_SETMEMBLOCK:
memblock = (void *)(uintptr_t)get_reginfo_paramreg(&ri);
break;
@@ -63,7 +63,7 @@ int send_register_info(write_fn write_fn, void *uc)
default:
abort();
}
- return 0;
+ return RES_OK;
}
/* Read register info from the socket and compare it with that from the
@@ -74,10 +74,10 @@ int send_register_info(write_fn write_fn, void *uc)
* 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)
+RisuResult recv_and_compare_register_info(read_fn read_fn,
+ respond_fn resp_fn, void *uc)
{
- int resp = 0;
+ RisuResult resp = RES_OK;
trace_header_t header;
RisuOp op;
@@ -85,18 +85,18 @@ int recv_and_compare_register_info(read_fn read_fn,
op = get_risuop(&master_ri);
if (read_fn(&header, sizeof(header)) != 0) {
- return -1;
+ return RES_BAD_IO;
}
if (header.risu_op != op) {
/* We are out of sync */
- resp = 2;
+ resp = RES_BAD_IO;
resp_fn(resp);
return resp;
}
/* send OK for the header */
- resp_fn(0);
+ resp_fn(RES_OK);
switch (op) {
case OP_COMPARE:
@@ -107,12 +107,12 @@ int recv_and_compare_register_info(read_fn read_fn,
*/
if (read_fn(&apprentice_ri, reginfo_size())) {
packet_mismatch = 1;
- resp = 2;
+ resp = RES_BAD_IO;
} else if (!reginfo_is_eq(&master_ri, &apprentice_ri)) {
/* register mismatch */
- resp = 2;
+ resp = RES_MISMATCH;
} else if (op == OP_TESTEND) {
- resp = 1;
+ resp = RES_END;
}
resp_fn(resp);
break;
@@ -127,10 +127,10 @@ int recv_and_compare_register_info(read_fn read_fn,
mem_used = 1;
if (read_fn(apprentice_memblock, MEMBLOCKLEN)) {
packet_mismatch = 1;
- resp = 2;
+ resp = RES_BAD_IO;
} else if (memcmp(memblock, apprentice_memblock, MEMBLOCKLEN) != 0) {
/* memory mismatch */
- resp = 2;
+ resp = RES_MISMATCH;
}
resp_fn(resp);
break;
diff --git a/risu.c b/risu.c
index 24ada8f..8796975 100644
--- a/risu.c
+++ b/risu.c
@@ -69,7 +69,7 @@ int write_trace(void *ptr, size_t bytes)
return (res == bytes) ? 0 : 1;
}
-void respond_sock(int r)
+void respond_sock(RisuResult r)
{
send_response_byte(comm_fd, r);
}
@@ -98,11 +98,11 @@ int read_trace(void *ptr, size_t bytes)
return (res == bytes) ? 0 : 1;
}
-void respond_trace(int r)
+void respond_trace(RisuResult r)
{
switch (r) {
- case 0: /* test ok */
- case 1: /* end of test */
+ case RES_OK:
+ case RES_END:
break;
default:
/* mismatch - if tracing we need to report, otherwise barf */
@@ -115,7 +115,7 @@ void respond_trace(int r)
static void master_sigill(int sig, siginfo_t *si, void *uc)
{
- int r;
+ RisuResult r;
signal_count++;
if (trace) {
@@ -125,7 +125,7 @@ static void master_sigill(int sig, siginfo_t *si, void *uc)
}
switch (r) {
- case 0:
+ case RES_OK:
/* match OK */
advance_pc(uc);
return;
@@ -137,7 +137,7 @@ static void master_sigill(int sig, siginfo_t *si, void *uc)
static void apprentice_sigill(int sig, siginfo_t *si, void *uc)
{
- int r;
+ RisuResult r;
signal_count++;
if (trace) {
@@ -147,11 +147,11 @@ static void apprentice_sigill(int sig, siginfo_t *si, void *uc)
}
switch (r) {
- case 0:
+ case RES_OK:
/* match OK */
advance_pc(uc);
return;
- case 1:
+ case RES_END:
/* end of test */
exit(EXIT_SUCCESS);
default:
--
2.20.1
^ permalink raw reply related [flat|nested] 36+ messages in thread* Re: [RISU v2 08/17] Add enum RisuResult
2020-05-19 2:53 ` [RISU v2 08/17] Add enum RisuResult Richard Henderson
@ 2020-05-19 19:06 ` Alex Bennée
0 siblings, 0 replies; 36+ messages in thread
From: Alex Bennée @ 2020-05-19 19:06 UTC (permalink / raw)
To: Richard Henderson; +Cc: peter.maydell, qemu-devel
Richard Henderson <richard.henderson@linaro.org> writes:
> Formalize the random set of numbers into an enum. Doing this
> makes it easy to see that one of the responses in
> recv_and_compare_register_info was inconsistent.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
--
Alex Bennée
^ permalink raw reply [flat|nested] 36+ messages in thread
* [RISU v2 09/17] Unify i/o functions and use RisuResult
2020-05-19 2:53 [RISU v2 00/17] risu cleanups and improvements Richard Henderson
` (7 preceding siblings ...)
2020-05-19 2:53 ` [RISU v2 08/17] Add enum RisuResult Richard Henderson
@ 2020-05-19 2:53 ` Richard Henderson
2020-05-20 15:50 ` Alex Bennée
2020-05-19 2:53 ` [RISU v2 10/17] Pass non-OK result back through siglongjmp Richard Henderson
` (7 subsequent siblings)
16 siblings, 1 reply; 36+ messages in thread
From: Richard Henderson @ 2020-05-19 2:53 UTC (permalink / raw)
To: qemu-devel; +Cc: peter.maydell, alex.bennee
Push the trace check down from the function calling the reginfo
function down into the i/o function. This means we don't have
to pass a function pointer.
Return a RisuResult from the i/o functions. This fixes a minor bug
in send_register_info (even before the conversion to RisuResult),
which returned the write_fn result directly.
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
risu.h | 25 ++++++++---------
comms.c | 8 +++---
reginfo.c | 60 ++++++++++++++++++++--------------------
risu.c | 82 ++++++++++++++++++++++---------------------------------
4 files changed, 80 insertions(+), 95 deletions(-)
diff --git a/risu.h b/risu.h
index e6d07eb..f44b781 100644
--- a/risu.h
+++ b/risu.h
@@ -34,13 +34,6 @@ void process_arch_opt(int opt, const char *arg);
#include REGINFO_HEADER(ARCH)
-/* Socket related routines */
-int master_connect(int port);
-int apprentice_connect(const char *hostname, int port);
-int send_data_pkt(int sock, void *pkt, int pktlen);
-int recv_data_pkt(int sock, void *pkt, int pktlen);
-void send_response_byte(int sock, int resp);
-
extern uintptr_t image_start_address;
extern void *memblock;
@@ -80,6 +73,13 @@ typedef struct {
uint32_t risu_op;
} trace_header_t;
+/* Socket related routines */
+int master_connect(int port);
+int apprentice_connect(const char *hostname, int port);
+RisuResult send_data_pkt(int sock, void *pkt, int pktlen);
+RisuResult recv_data_pkt(int sock, void *pkt, int pktlen);
+void send_response_byte(int sock, int resp);
+
/* Functions operating on reginfo */
/* Function prototypes for read/write helper functions.
@@ -88,23 +88,22 @@ typedef struct {
* recv_and_compare_register_info which can either be backed by the
* traditional network socket or a trace file.
*/
-typedef int (*write_fn) (void *ptr, size_t bytes);
-typedef int (*read_fn) (void *ptr, size_t bytes);
-typedef void (*respond_fn) (RisuResult response);
+RisuResult write_buffer(void *ptr, size_t bytes);
+RisuResult read_buffer(void *ptr, size_t bytes);
+void respond(RisuResult response);
/*
* Send the register information from the struct ucontext down the socket.
* NB: called from a signal handler.
*/
-RisuResult send_register_info(write_fn write_fn, void *uc);
+RisuResult send_register_info(void *uc);
/*
* Read register info from the socket and compare it with that from the
* ucontext.
* NB: called from a signal handler.
*/
-RisuResult recv_and_compare_register_info(read_fn read_fn,
- respond_fn respond, void *uc);
+RisuResult recv_and_compare_register_info(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/comms.c b/comms.c
index 861e845..21968da 100644
--- a/comms.c
+++ b/comms.c
@@ -168,7 +168,7 @@ ssize_t safe_writev(int fd, struct iovec *iov_in, int iovcnt)
* Note that both ends must agree on the length of the
* block of data.
*/
-int send_data_pkt(int sock, void *pkt, int pktlen)
+RisuResult send_data_pkt(int sock, void *pkt, int pktlen)
{
unsigned char resp;
/* First we send the packet length as a network-order 32 bit value.
@@ -196,7 +196,7 @@ int send_data_pkt(int sock, void *pkt, int pktlen)
return resp;
}
-int recv_data_pkt(int sock, void *pkt, int pktlen)
+RisuResult recv_data_pkt(int sock, void *pkt, int pktlen)
{
uint32_t net_pktlen;
recv_bytes(sock, &net_pktlen, sizeof(net_pktlen));
@@ -206,10 +206,10 @@ int recv_data_pkt(int sock, void *pkt, int pktlen)
* a response back.
*/
recv_and_discard_bytes(sock, net_pktlen);
- return 1;
+ return RES_BAD_IO;
}
recv_bytes(sock, pkt, pktlen);
- return 0;
+ return RES_OK;
}
void send_response_byte(int sock, int resp)
diff --git a/reginfo.c b/reginfo.c
index b909a1f..fee025e 100644
--- a/reginfo.c
+++ b/reginfo.c
@@ -21,10 +21,11 @@ uint8_t apprentice_memblock[MEMBLOCKLEN];
static int mem_used;
static int packet_mismatch;
-RisuResult send_register_info(write_fn write_fn, void *uc)
+RisuResult send_register_info(void *uc)
{
struct reginfo ri;
trace_header_t header;
+ RisuResult res;
RisuOp op;
reginfo_init(&ri, uc);
@@ -33,8 +34,9 @@ RisuResult send_register_info(write_fn write_fn, void *uc)
/* Write a header with PC/op to keep in sync */
header.pc = get_pc(&ri);
header.risu_op = op;
- if (write_fn(&header, sizeof(header)) != 0) {
- return RES_BAD_IO;
+ res = write_buffer(&header, sizeof(header));
+ if (res != RES_OK) {
+ return res;
}
switch (op) {
@@ -45,11 +47,12 @@ RisuResult send_register_info(write_fn write_fn, void *uc)
* Do a simple register compare on (a) explicit request
* (b) end of test (c) a non-risuop UNDEF
*/
- if (write_fn(&ri, reginfo_size()) != 0) {
- return RES_BAD_IO;
+ res = write_buffer(&ri, reginfo_size());
+ /* For OP_TEST_END, force exit. */
+ if (res == RES_OK && op == OP_TESTEND) {
+ res = RES_END;
}
- /* For OP_TEST_END, force return 1 to exit. */
- return op == OP_TESTEND ? RES_END : RES_OK;
+ break;
case OP_SETMEMBLOCK:
memblock = (void *)(uintptr_t)get_reginfo_paramreg(&ri);
break;
@@ -58,12 +61,11 @@ RisuResult send_register_info(write_fn write_fn, void *uc)
get_reginfo_paramreg(&ri) + (uintptr_t)memblock);
break;
case OP_COMPAREMEM:
- return write_fn(memblock, MEMBLOCKLEN);
- break;
+ return write_buffer(memblock, MEMBLOCKLEN);
default:
abort();
}
- return RES_OK;
+ return res;
}
/* Read register info from the socket and compare it with that from the
@@ -74,29 +76,29 @@ RisuResult send_register_info(write_fn write_fn, void *uc)
* that says whether it is register or memory data, so if the two
* sides get out of sync then we will fail obscurely.
*/
-RisuResult recv_and_compare_register_info(read_fn read_fn,
- respond_fn resp_fn, void *uc)
+RisuResult recv_and_compare_register_info(void *uc)
{
- RisuResult resp = RES_OK;
+ RisuResult res;
trace_header_t header;
RisuOp op;
reginfo_init(&master_ri, uc);
op = get_risuop(&master_ri);
- if (read_fn(&header, sizeof(header)) != 0) {
- return RES_BAD_IO;
+ res = read_buffer(&header, sizeof(header));
+ if (res != RES_OK) {
+ return res;
}
if (header.risu_op != op) {
/* We are out of sync */
- resp = RES_BAD_IO;
- resp_fn(resp);
- return resp;
+ res = RES_BAD_IO;
+ respond(res);
+ return res;
}
/* send OK for the header */
- resp_fn(RES_OK);
+ respond(RES_OK);
switch (op) {
case OP_COMPARE:
@@ -105,16 +107,16 @@ RisuResult recv_and_compare_register_info(read_fn read_fn,
/* Do a simple register compare on (a) explicit request
* (b) end of test (c) a non-risuop UNDEF
*/
- if (read_fn(&apprentice_ri, reginfo_size())) {
+ res = read_buffer(&apprentice_ri, reginfo_size());
+ if (res != RES_OK) {
packet_mismatch = 1;
- resp = RES_BAD_IO;
} else if (!reginfo_is_eq(&master_ri, &apprentice_ri)) {
/* register mismatch */
- resp = RES_MISMATCH;
+ res = RES_MISMATCH;
} else if (op == OP_TESTEND) {
- resp = RES_END;
+ res = RES_END;
}
- resp_fn(resp);
+ respond(res);
break;
case OP_SETMEMBLOCK:
memblock = (void *)(uintptr_t)get_reginfo_paramreg(&master_ri);
@@ -125,20 +127,20 @@ RisuResult recv_and_compare_register_info(read_fn read_fn,
break;
case OP_COMPAREMEM:
mem_used = 1;
- if (read_fn(apprentice_memblock, MEMBLOCKLEN)) {
+ res = read_buffer(apprentice_memblock, MEMBLOCKLEN);
+ if (res != RES_OK) {
packet_mismatch = 1;
- resp = RES_BAD_IO;
} else if (memcmp(memblock, apprentice_memblock, MEMBLOCKLEN) != 0) {
/* memory mismatch */
- resp = RES_MISMATCH;
+ res = RES_MISMATCH;
}
- resp_fn(resp);
+ respond(res);
break;
default:
abort();
}
- return resp;
+ return res;
}
/* Print a useful report on the status of the last comparison
diff --git a/risu.c b/risu.c
index 8796975..78c6b8f 100644
--- a/risu.c
+++ b/risu.c
@@ -46,44 +46,15 @@ static sigjmp_buf jmpbuf;
#define ARRAY_SIZE(x) (sizeof(x) / sizeof((x)[0]))
-/* Master functions */
+/* I/O functions */
-int read_sock(void *ptr, size_t bytes)
-{
- return recv_data_pkt(comm_fd, ptr, bytes);
-}
-
-int write_trace(void *ptr, size_t bytes)
+RisuResult read_buffer(void *ptr, size_t bytes)
{
size_t res;
-#ifdef HAVE_ZLIB
- if (comm_fd == STDOUT_FILENO) {
-#endif
- res = write(comm_fd, ptr, bytes);
-#ifdef HAVE_ZLIB
- } else {
- res = gzwrite(gz_trace_file, ptr, bytes);
+ if (!trace) {
+ return recv_data_pkt(comm_fd, ptr, bytes);
}
-#endif
- return (res == bytes) ? 0 : 1;
-}
-
-void respond_sock(RisuResult r)
-{
- send_response_byte(comm_fd, r);
-}
-
-/* Apprentice function */
-
-int write_sock(void *ptr, size_t bytes)
-{
- return send_data_pkt(comm_fd, ptr, bytes);
-}
-
-int read_trace(void *ptr, size_t bytes)
-{
- size_t res;
#ifdef HAVE_ZLIB
if (comm_fd == STDIN_FILENO) {
@@ -95,21 +66,34 @@ int read_trace(void *ptr, size_t bytes)
}
#endif
- return (res == bytes) ? 0 : 1;
+ return res == bytes ? RES_OK : RES_BAD_IO;
}
-void respond_trace(RisuResult r)
+RisuResult write_buffer(void *ptr, size_t bytes)
{
- switch (r) {
- case RES_OK:
- case RES_END:
- break;
- default:
- /* mismatch - if tracing we need to report, otherwise barf */
- if (!trace) {
- abort();
- }
- break;
+ size_t res;
+
+ if (!trace) {
+ return send_data_pkt(comm_fd, ptr, bytes);
+ }
+
+#ifdef HAVE_ZLIB
+ if (comm_fd == STDOUT_FILENO) {
+#endif
+ res = write(comm_fd, ptr, bytes);
+#ifdef HAVE_ZLIB
+ } else {
+ res = gzwrite(gz_trace_file, ptr, bytes);
+ }
+#endif
+
+ return res == bytes ? RES_OK : RES_BAD_IO;
+}
+
+void respond(RisuResult r)
+{
+ if (!trace) {
+ send_response_byte(comm_fd, r);
}
}
@@ -119,9 +103,9 @@ static void master_sigill(int sig, siginfo_t *si, void *uc)
signal_count++;
if (trace) {
- r = send_register_info(write_trace, uc);
+ r = send_register_info(uc);
} else {
- r = recv_and_compare_register_info(read_sock, respond_sock, uc);
+ r = recv_and_compare_register_info(uc);
}
switch (r) {
@@ -141,9 +125,9 @@ static void apprentice_sigill(int sig, siginfo_t *si, void *uc)
signal_count++;
if (trace) {
- r = recv_and_compare_register_info(read_trace, respond_trace, uc);
+ r = recv_and_compare_register_info(uc);
} else {
- r = send_register_info(write_sock, uc);
+ r = send_register_info(uc);
}
switch (r) {
--
2.20.1
^ permalink raw reply related [flat|nested] 36+ messages in thread* Re: [RISU v2 09/17] Unify i/o functions and use RisuResult
2020-05-19 2:53 ` [RISU v2 09/17] Unify i/o functions and use RisuResult Richard Henderson
@ 2020-05-20 15:50 ` Alex Bennée
0 siblings, 0 replies; 36+ messages in thread
From: Alex Bennée @ 2020-05-20 15:50 UTC (permalink / raw)
To: Richard Henderson; +Cc: peter.maydell, qemu-devel
Richard Henderson <richard.henderson@linaro.org> writes:
> Push the trace check down from the function calling the reginfo
> function down into the i/o function. This means we don't have
> to pass a function pointer.
>
> Return a RisuResult from the i/o functions. This fixes a minor bug
> in send_register_info (even before the conversion to RisuResult),
> which returned the write_fn result directly.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
--
Alex Bennée
^ permalink raw reply [flat|nested] 36+ messages in thread
* [RISU v2 10/17] Pass non-OK result back through siglongjmp
2020-05-19 2:53 [RISU v2 00/17] risu cleanups and improvements Richard Henderson
` (8 preceding siblings ...)
2020-05-19 2:53 ` [RISU v2 09/17] Unify i/o functions and use RisuResult Richard Henderson
@ 2020-05-19 2:53 ` Richard Henderson
2020-05-20 15:52 ` Alex Bennée
2020-05-19 2:53 ` [RISU v2 11/17] Always write for --master Richard Henderson
` (6 subsequent siblings)
16 siblings, 1 reply; 36+ messages in thread
From: Richard Henderson @ 2020-05-19 2:53 UTC (permalink / raw)
To: qemu-devel; +Cc: peter.maydell, alex.bennee
Rather than doing some work in the signal handler and
some work outside, move all of the non-resume work outside.
This works because we arranged for RES_OK to be 0, which
is the normal return from sigsetjmp.
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
risu.c | 50 ++++++++++++++++++++++++--------------------------
1 file changed, 24 insertions(+), 26 deletions(-)
diff --git a/risu.c b/risu.c
index 78c6b8f..d09ac0b 100644
--- a/risu.c
+++ b/risu.c
@@ -107,15 +107,10 @@ static void master_sigill(int sig, siginfo_t *si, void *uc)
} else {
r = recv_and_compare_register_info(uc);
}
-
- switch (r) {
- case RES_OK:
- /* match OK */
+ if (r == RES_OK) {
advance_pc(uc);
- return;
- default:
- /* mismatch, or end of test */
- siglongjmp(jmpbuf, 1);
+ } else {
+ siglongjmp(jmpbuf, r);
}
}
@@ -129,21 +124,10 @@ static void apprentice_sigill(int sig, siginfo_t *si, void *uc)
} else {
r = send_register_info(uc);
}
-
- switch (r) {
- case RES_OK:
- /* match OK */
+ if (r == RES_OK) {
advance_pc(uc);
- return;
- case RES_END:
- /* end of test */
- exit(EXIT_SUCCESS);
- default:
- /* mismatch */
- if (trace) {
- siglongjmp(jmpbuf, 1);
- }
- exit(EXIT_FAILURE);
+ } else {
+ siglongjmp(jmpbuf, r);
}
}
@@ -200,7 +184,9 @@ static void load_image(const char *imgfile)
static int master(void)
{
- if (sigsetjmp(jmpbuf, 1)) {
+ RisuResult res = sigsetjmp(jmpbuf, 1);
+
+ if (res != RES_OK) {
#ifdef HAVE_ZLIB
if (trace && comm_fd != STDOUT_FILENO) {
gzclose(gz_trace_file);
@@ -226,15 +212,27 @@ static int master(void)
static int apprentice(void)
{
- if (sigsetjmp(jmpbuf, 1)) {
+ RisuResult res = sigsetjmp(jmpbuf, 1);
+
+ if (res != RES_OK) {
#ifdef HAVE_ZLIB
if (trace && comm_fd != STDIN_FILENO) {
gzclose(gz_trace_file);
}
#endif
close(comm_fd);
- fprintf(stderr, "finished early after %zd checkpoints\n", signal_count);
- return report_match_status(true);
+
+ switch (res) {
+ case RES_END:
+ return EXIT_SUCCESS;
+ default:
+ if (!trace) {
+ return EXIT_FAILURE;
+ }
+ fprintf(stderr, "finished early after %zd checkpoints\n",
+ signal_count);
+ return report_match_status(true);
+ }
}
set_sigill_handler(&apprentice_sigill);
fprintf(stderr, "starting apprentice image at 0x%"PRIxPTR"\n",
--
2.20.1
^ permalink raw reply related [flat|nested] 36+ messages in thread* [RISU v2 11/17] Always write for --master
2020-05-19 2:53 [RISU v2 00/17] risu cleanups and improvements Richard Henderson
` (9 preceding siblings ...)
2020-05-19 2:53 ` [RISU v2 10/17] Pass non-OK result back through siglongjmp Richard Henderson
@ 2020-05-19 2:53 ` Richard Henderson
2020-05-20 16:12 ` Alex Bennée
2020-05-19 2:53 ` [RISU v2 12/17] Simplify syncing with master Richard Henderson
` (5 subsequent siblings)
16 siblings, 1 reply; 36+ messages in thread
From: Richard Henderson @ 2020-05-19 2:53 UTC (permalink / raw)
To: qemu-devel; +Cc: peter.maydell, alex.bennee
For trace, master of course must write to the file we create.
For sockets, we can report mismatches from either end. At present,
we are reporting mismatches from master. Reverse that so that we
report mismatches from the apprentice, just as we do for trace.
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
risu.h | 2 +-
reginfo.c | 38 ++++++++--------------
risu.c | 96 ++++++++++++++++++++++++++-----------------------------
3 files changed, 61 insertions(+), 75 deletions(-)
diff --git a/risu.h b/risu.h
index f44b781..2ded5c4 100644
--- a/risu.h
+++ b/risu.h
@@ -111,7 +111,7 @@ RisuResult recv_and_compare_register_info(void *uc);
* Should return 0 if it was a good match (ie end of test)
* and 1 for a mismatch.
*/
-int report_match_status(bool trace);
+int report_match_status(void);
/* Interface provided by CPU-specific code: */
diff --git a/reginfo.c b/reginfo.c
index fee025e..c37c5df 100644
--- a/reginfo.c
+++ b/reginfo.c
@@ -14,9 +14,8 @@
#include <stdlib.h>
#include "risu.h"
-struct reginfo master_ri, apprentice_ri;
-
-uint8_t apprentice_memblock[MEMBLOCKLEN];
+static struct reginfo master_ri, apprentice_ri;
+static uint8_t master_memblock[MEMBLOCKLEN];
static int mem_used;
static int packet_mismatch;
@@ -82,8 +81,8 @@ RisuResult recv_and_compare_register_info(void *uc)
trace_header_t header;
RisuOp op;
- reginfo_init(&master_ri, uc);
- op = get_risuop(&master_ri);
+ reginfo_init(&apprentice_ri, uc);
+ op = get_risuop(&apprentice_ri);
res = read_buffer(&header, sizeof(header));
if (res != RES_OK) {
@@ -107,7 +106,7 @@ RisuResult recv_and_compare_register_info(void *uc)
/* Do a simple register compare on (a) explicit request
* (b) end of test (c) a non-risuop UNDEF
*/
- res = read_buffer(&apprentice_ri, reginfo_size());
+ res = read_buffer(&master_ri, reginfo_size());
if (res != RES_OK) {
packet_mismatch = 1;
} else if (!reginfo_is_eq(&master_ri, &apprentice_ri)) {
@@ -119,18 +118,18 @@ RisuResult recv_and_compare_register_info(void *uc)
respond(res);
break;
case OP_SETMEMBLOCK:
- memblock = (void *)(uintptr_t)get_reginfo_paramreg(&master_ri);
+ memblock = (void *)(uintptr_t)get_reginfo_paramreg(&apprentice_ri);
break;
case OP_GETMEMBLOCK:
- set_ucontext_paramreg(uc, get_reginfo_paramreg(&master_ri) +
+ set_ucontext_paramreg(uc, get_reginfo_paramreg(&apprentice_ri) +
(uintptr_t)memblock);
break;
case OP_COMPAREMEM:
mem_used = 1;
- res = read_buffer(apprentice_memblock, MEMBLOCKLEN);
+ res = read_buffer(master_memblock, MEMBLOCKLEN);
if (res != RES_OK) {
packet_mismatch = 1;
- } else if (memcmp(memblock, apprentice_memblock, MEMBLOCKLEN) != 0) {
+ } else if (memcmp(memblock, master_memblock, MEMBLOCKLEN) != 0) {
/* memory mismatch */
res = RES_MISMATCH;
}
@@ -149,18 +148,13 @@ RisuResult recv_and_compare_register_info(void *uc)
* Should return 0 if it was a good match (ie end of test)
* and 1 for a mismatch.
*/
-int report_match_status(bool trace)
+int report_match_status(void)
{
int resp = 0;
fprintf(stderr, "match status...\n");
if (packet_mismatch) {
fprintf(stderr, "packet mismatch (probably disagreement "
"about UNDEF on load/store)\n");
- /* We don't have valid reginfo from the apprentice side
- * so stop now rather than printing anything about it.
- */
- fprintf(stderr, "%s reginfo:\n", trace ? "this" : "master");
- reginfo_dump(&master_ri, stderr);
return 1;
}
if (!reginfo_is_eq(&master_ri, &apprentice_ri)) {
@@ -168,7 +162,7 @@ int report_match_status(bool trace)
resp = 1;
}
if (mem_used
- && memcmp(memblock, &apprentice_memblock, MEMBLOCKLEN) != 0) {
+ && memcmp(memblock, &master_memblock, MEMBLOCKLEN) != 0) {
fprintf(stderr, "mismatch on memory!\n");
resp = 1;
}
@@ -177,15 +171,11 @@ int report_match_status(bool trace)
return 0;
}
- fprintf(stderr, "%s reginfo:\n", trace ? "this" : "master");
+ fprintf(stderr, "master reginfo:\n");
reginfo_dump(&master_ri, stderr);
- fprintf(stderr, "%s reginfo:\n", trace ? "trace" : "apprentice");
+ fprintf(stderr, "apprentice reginfo:\n");
reginfo_dump(&apprentice_ri, stderr);
- if (trace) {
- reginfo_dump_mismatch(&apprentice_ri, &master_ri, stderr);
- } else {
- reginfo_dump_mismatch(&master_ri, &apprentice_ri, stderr);
- }
+ reginfo_dump_mismatch(&master_ri, &apprentice_ri, stderr);
return resp;
}
diff --git a/risu.c b/risu.c
index d09ac0b..ea4b4d3 100644
--- a/risu.c
+++ b/risu.c
@@ -102,11 +102,7 @@ static void master_sigill(int sig, siginfo_t *si, void *uc)
RisuResult r;
signal_count++;
- if (trace) {
- r = send_register_info(uc);
- } else {
- r = recv_and_compare_register_info(uc);
- }
+ r = send_register_info(uc);
if (r == RES_OK) {
advance_pc(uc);
} else {
@@ -119,11 +115,7 @@ static void apprentice_sigill(int sig, siginfo_t *si, void *uc)
RisuResult r;
signal_count++;
- if (trace) {
- r = recv_and_compare_register_info(uc);
- } else {
- r = send_register_info(uc);
- }
+ r = recv_and_compare_register_info(uc);
if (r == RES_OK) {
advance_pc(uc);
} else {
@@ -186,61 +178,65 @@ static int master(void)
{
RisuResult res = sigsetjmp(jmpbuf, 1);
- if (res != RES_OK) {
+ switch (res) {
+ case RES_OK:
+ set_sigill_handler(&master_sigill);
+ fprintf(stderr, "starting master image at 0x%"PRIxPTR"\n",
+ image_start_address);
+ fprintf(stderr, "starting image\n");
+ image_start();
+ fprintf(stderr, "image returned unexpectedly\n");
+ return EXIT_FAILURE;
+
+ case RES_END:
#ifdef HAVE_ZLIB
if (trace && comm_fd != STDOUT_FILENO) {
gzclose(gz_trace_file);
}
#endif
close(comm_fd);
- if (trace) {
- fprintf(stderr, "trace complete after %zd checkpoints\n",
- signal_count);
- return EXIT_SUCCESS;
- } else {
- return report_match_status(false);
- }
+ return EXIT_SUCCESS;
+
+ case RES_BAD_IO:
+ fprintf(stderr, "i/o error after %zd checkpoints\n", signal_count);
+ return EXIT_FAILURE;
+
+ default:
+ fprintf(stderr, "unexpected result %d\n", res);
+ return EXIT_FAILURE;
}
- set_sigill_handler(&master_sigill);
- fprintf(stderr, "starting master image at 0x%"PRIxPTR"\n",
- image_start_address);
- fprintf(stderr, "starting image\n");
- image_start();
- fprintf(stderr, "image returned unexpectedly\n");
- return EXIT_FAILURE;
}
static int apprentice(void)
{
RisuResult res = sigsetjmp(jmpbuf, 1);
- if (res != RES_OK) {
-#ifdef HAVE_ZLIB
- if (trace && comm_fd != STDIN_FILENO) {
- gzclose(gz_trace_file);
- }
-#endif
- close(comm_fd);
+ switch (res) {
+ case RES_OK:
+ set_sigill_handler(&apprentice_sigill);
+ fprintf(stderr, "starting apprentice image at 0x%"PRIxPTR"\n",
+ image_start_address);
+ fprintf(stderr, "starting image\n");
+ image_start();
+ fprintf(stderr, "image returned unexpectedly\n");
+ return EXIT_FAILURE;
- switch (res) {
- case RES_END:
- return EXIT_SUCCESS;
- default:
- if (!trace) {
- return EXIT_FAILURE;
- }
- fprintf(stderr, "finished early after %zd checkpoints\n",
- signal_count);
- return report_match_status(true);
- }
+ case RES_END:
+ return EXIT_SUCCESS;
+
+ case RES_MISMATCH:
+ fprintf(stderr, "mismatch after %zd checkpoints\n", signal_count);
+ report_match_status();
+ return EXIT_FAILURE;
+
+ case RES_BAD_IO:
+ fprintf(stderr, "i/o error after %zd checkpoints\n", signal_count);
+ return EXIT_FAILURE;
+
+ default:
+ fprintf(stderr, "unexpected result %d\n", res);
+ return EXIT_FAILURE;
}
- set_sigill_handler(&apprentice_sigill);
- fprintf(stderr, "starting apprentice image at 0x%"PRIxPTR"\n",
- image_start_address);
- fprintf(stderr, "starting image\n");
- image_start();
- fprintf(stderr, "image returned unexpectedly\n");
- return EXIT_FAILURE;
}
static int ismaster;
--
2.20.1
^ permalink raw reply related [flat|nested] 36+ messages in thread* Re: [RISU v2 11/17] Always write for --master
2020-05-19 2:53 ` [RISU v2 11/17] Always write for --master Richard Henderson
@ 2020-05-20 16:12 ` Alex Bennée
0 siblings, 0 replies; 36+ messages in thread
From: Alex Bennée @ 2020-05-20 16:12 UTC (permalink / raw)
To: Richard Henderson; +Cc: peter.maydell, qemu-devel
Richard Henderson <richard.henderson@linaro.org> writes:
> For trace, master of course must write to the file we create.
>
> For sockets, we can report mismatches from either end. At present,
> we are reporting mismatches from master. Reverse that so that we
> report mismatches from the apprentice, just as we do for trace.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
--
Alex Bennée
^ permalink raw reply [flat|nested] 36+ messages in thread
* [RISU v2 12/17] Simplify syncing with master
2020-05-19 2:53 [RISU v2 00/17] risu cleanups and improvements Richard Henderson
` (10 preceding siblings ...)
2020-05-19 2:53 ` [RISU v2 11/17] Always write for --master Richard Henderson
@ 2020-05-19 2:53 ` Richard Henderson
2020-05-20 16:14 ` Alex Bennée
2020-05-19 2:53 ` [RISU v2 13/17] Split RES_MISMATCH for registers and memory Richard Henderson
` (4 subsequent siblings)
16 siblings, 1 reply; 36+ messages in thread
From: Richard Henderson @ 2020-05-19 2:53 UTC (permalink / raw)
To: qemu-devel; +Cc: peter.maydell, alex.bennee
Do not pass status like RES_BAD_IO from apprentice to master.
This means that when master reports i/o error that we know it
came from master; the apprentice will report its own i/o error.
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
reginfo.c | 11 +++++------
1 file changed, 5 insertions(+), 6 deletions(-)
diff --git a/reginfo.c b/reginfo.c
index c37c5df..31bc699 100644
--- a/reginfo.c
+++ b/reginfo.c
@@ -90,10 +90,9 @@ RisuResult recv_and_compare_register_info(void *uc)
}
if (header.risu_op != op) {
- /* We are out of sync */
- res = RES_BAD_IO;
- respond(res);
- return res;
+ /* We are out of sync. Tell master to exit. */
+ respond(RES_END);
+ return RES_BAD_IO;
}
/* send OK for the header */
@@ -115,7 +114,7 @@ RisuResult recv_and_compare_register_info(void *uc)
} else if (op == OP_TESTEND) {
res = RES_END;
}
- respond(res);
+ respond(res == RES_OK ? RES_OK : RES_END);
break;
case OP_SETMEMBLOCK:
memblock = (void *)(uintptr_t)get_reginfo_paramreg(&apprentice_ri);
@@ -133,7 +132,7 @@ RisuResult recv_and_compare_register_info(void *uc)
/* memory mismatch */
res = RES_MISMATCH;
}
- respond(res);
+ respond(res == RES_OK ? RES_OK : RES_END);
break;
default:
abort();
--
2.20.1
^ permalink raw reply related [flat|nested] 36+ messages in thread* Re: [RISU v2 12/17] Simplify syncing with master
2020-05-19 2:53 ` [RISU v2 12/17] Simplify syncing with master Richard Henderson
@ 2020-05-20 16:14 ` Alex Bennée
0 siblings, 0 replies; 36+ messages in thread
From: Alex Bennée @ 2020-05-20 16:14 UTC (permalink / raw)
To: Richard Henderson; +Cc: peter.maydell, qemu-devel
Richard Henderson <richard.henderson@linaro.org> writes:
> Do not pass status like RES_BAD_IO from apprentice to master.
> This means that when master reports i/o error that we know it
> came from master; the apprentice will report its own i/o error.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
--
Alex Bennée
^ permalink raw reply [flat|nested] 36+ messages in thread
* [RISU v2 13/17] Split RES_MISMATCH for registers and memory
2020-05-19 2:53 [RISU v2 00/17] risu cleanups and improvements Richard Henderson
` (11 preceding siblings ...)
2020-05-19 2:53 ` [RISU v2 12/17] Simplify syncing with master Richard Henderson
@ 2020-05-19 2:53 ` Richard Henderson
2020-05-20 16:25 ` Alex Bennée
2020-05-19 2:53 ` [RISU v2 14/17] Add magic and size to the trace header Richard Henderson
` (3 subsequent siblings)
16 siblings, 1 reply; 36+ messages in thread
From: Richard Henderson @ 2020-05-19 2:53 UTC (permalink / raw)
To: qemu-devel; +Cc: peter.maydell, alex.bennee
By remembering the specific comparison that failed, we do not
have to try again when it comes time to report. This makes
the mem_used flag redundant. Also, packet_mismatch is now
redundant with RES_BAD_IO.
This means that the only thing that report_match_status does
is to report on register status, so rename to report_mismatch_reg.
Also, we know there is a failure, so don't return a status from
the report.
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
risu.h | 13 ++++++-------
reginfo.c | 45 ++++++++-------------------------------------
risu.c | 10 +++++++---
3 files changed, 21 insertions(+), 47 deletions(-)
diff --git a/risu.h b/risu.h
index 2ded5c4..b234f93 100644
--- a/risu.h
+++ b/risu.h
@@ -54,7 +54,8 @@ typedef enum {
typedef enum {
RES_OK = 0,
RES_END,
- RES_MISMATCH,
+ RES_MISMATCH_REG,
+ RES_MISMATCH_MEM,
RES_BAD_IO,
} RisuResult;
@@ -105,13 +106,11 @@ RisuResult send_register_info(void *uc);
*/
RisuResult recv_and_compare_register_info(void *uc);
-/* Print a useful report on the status of the last comparison
- * done in recv_and_compare_register_info(). This is called on
- * exit, so need not restrict itself to signal-safe functions.
- * Should return 0 if it was a good match (ie end of test)
- * and 1 for a mismatch.
+/*
+ * Print a useful report on the status of the last reg comparison
+ * done in recv_and_compare_register_info().
*/
-int report_match_status(void);
+void report_mismatch_reg(void);
/* Interface provided by CPU-specific code: */
diff --git a/reginfo.c b/reginfo.c
index 31bc699..a007f16 100644
--- a/reginfo.c
+++ b/reginfo.c
@@ -17,9 +17,6 @@
static struct reginfo master_ri, apprentice_ri;
static uint8_t master_memblock[MEMBLOCKLEN];
-static int mem_used;
-static int packet_mismatch;
-
RisuResult send_register_info(void *uc)
{
struct reginfo ri;
@@ -107,10 +104,10 @@ RisuResult recv_and_compare_register_info(void *uc)
*/
res = read_buffer(&master_ri, reginfo_size());
if (res != RES_OK) {
- packet_mismatch = 1;
+ /* fail */
} else if (!reginfo_is_eq(&master_ri, &apprentice_ri)) {
/* register mismatch */
- res = RES_MISMATCH;
+ res = RES_MISMATCH_REG;
} else if (op == OP_TESTEND) {
res = RES_END;
}
@@ -124,13 +121,12 @@ RisuResult recv_and_compare_register_info(void *uc)
(uintptr_t)memblock);
break;
case OP_COMPAREMEM:
- mem_used = 1;
res = read_buffer(master_memblock, MEMBLOCKLEN);
if (res != RES_OK) {
- packet_mismatch = 1;
+ /* fail */
} else if (memcmp(memblock, master_memblock, MEMBLOCKLEN) != 0) {
/* memory mismatch */
- res = RES_MISMATCH;
+ res = RES_MISMATCH_MEM;
}
respond(res == RES_OK ? RES_OK : RES_END);
break;
@@ -141,40 +137,15 @@ RisuResult recv_and_compare_register_info(void *uc)
return res;
}
-/* Print a useful report on the status of the last comparison
- * done in recv_and_compare_register_info(). This is called on
- * exit, so need not restrict itself to signal-safe functions.
- * Should return 0 if it was a good match (ie end of test)
- * and 1 for a mismatch.
+/*
+ * Print a useful report on the status of the last reg comparison
+ * done in recv_and_compare_register_info().
*/
-int report_match_status(void)
+void report_mismatch_reg(void)
{
- int resp = 0;
- fprintf(stderr, "match status...\n");
- if (packet_mismatch) {
- fprintf(stderr, "packet mismatch (probably disagreement "
- "about UNDEF on load/store)\n");
- return 1;
- }
- if (!reginfo_is_eq(&master_ri, &apprentice_ri)) {
- fprintf(stderr, "mismatch on regs!\n");
- resp = 1;
- }
- if (mem_used
- && memcmp(memblock, &master_memblock, MEMBLOCKLEN) != 0) {
- fprintf(stderr, "mismatch on memory!\n");
- resp = 1;
- }
- if (!resp) {
- fprintf(stderr, "match!\n");
- return 0;
- }
-
fprintf(stderr, "master reginfo:\n");
reginfo_dump(&master_ri, stderr);
fprintf(stderr, "apprentice reginfo:\n");
reginfo_dump(&apprentice_ri, stderr);
-
reginfo_dump_mismatch(&master_ri, &apprentice_ri, stderr);
- return resp;
}
diff --git a/risu.c b/risu.c
index ea4b4d3..398faac 100644
--- a/risu.c
+++ b/risu.c
@@ -224,9 +224,13 @@ static int apprentice(void)
case RES_END:
return EXIT_SUCCESS;
- case RES_MISMATCH:
- fprintf(stderr, "mismatch after %zd checkpoints\n", signal_count);
- report_match_status();
+ case RES_MISMATCH_REG:
+ fprintf(stderr, "mismatch reg after %zd checkpoints\n", signal_count);
+ report_mismatch_reg();
+ return EXIT_FAILURE;
+
+ case RES_MISMATCH_MEM:
+ fprintf(stderr, "mismatch mem after %zd checkpoints\n", signal_count);
return EXIT_FAILURE;
case RES_BAD_IO:
--
2.20.1
^ permalink raw reply related [flat|nested] 36+ messages in thread* Re: [RISU v2 13/17] Split RES_MISMATCH for registers and memory
2020-05-19 2:53 ` [RISU v2 13/17] Split RES_MISMATCH for registers and memory Richard Henderson
@ 2020-05-20 16:25 ` Alex Bennée
0 siblings, 0 replies; 36+ messages in thread
From: Alex Bennée @ 2020-05-20 16:25 UTC (permalink / raw)
To: Richard Henderson; +Cc: peter.maydell, qemu-devel
Richard Henderson <richard.henderson@linaro.org> writes:
> By remembering the specific comparison that failed, we do not
> have to try again when it comes time to report. This makes
> the mem_used flag redundant. Also, packet_mismatch is now
> redundant with RES_BAD_IO.
>
> This means that the only thing that report_match_status does
> is to report on register status, so rename to report_mismatch_reg.
> Also, we know there is a failure, so don't return a status from
> the report.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
--
Alex Bennée
^ permalink raw reply [flat|nested] 36+ messages in thread
* [RISU v2 14/17] Add magic and size to the trace header
2020-05-19 2:53 [RISU v2 00/17] risu cleanups and improvements Richard Henderson
` (12 preceding siblings ...)
2020-05-19 2:53 ` [RISU v2 13/17] Split RES_MISMATCH for registers and memory Richard Henderson
@ 2020-05-19 2:53 ` Richard Henderson
2020-05-19 21:16 ` Richard Henderson
2020-05-19 2:53 ` [RISU v2 15/17] Compute reginfo_size based on the reginfo Richard Henderson
` (2 subsequent siblings)
16 siblings, 1 reply; 36+ messages in thread
From: Richard Henderson @ 2020-05-19 2:53 UTC (permalink / raw)
To: qemu-devel; +Cc: peter.maydell, alex.bennee
Sanity check that we're not getting out of sync with
the trace stream. This will be especially bad with
the change in size of the sve save data.
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
risu.h | 8 ++-
reginfo.c | 160 ++++++++++++++++++++++++++++++++++++++++++++----------
risu.c | 6 ++
3 files changed, 143 insertions(+), 31 deletions(-)
diff --git a/risu.h b/risu.h
index b234f93..eeb6775 100644
--- a/risu.h
+++ b/risu.h
@@ -56,6 +56,7 @@ typedef enum {
RES_END,
RES_MISMATCH_REG,
RES_MISMATCH_MEM,
+ RES_MISMATCH_HEAD,
RES_BAD_IO,
} RisuResult;
@@ -70,10 +71,14 @@ typedef enum {
struct reginfo;
typedef struct {
- uintptr_t pc;
+ uint32_t magic;
+ uint32_t size;
uint32_t risu_op;
+ uintptr_t pc;
} trace_header_t;
+#define RISU_MAGIC (('R' << 24) | ('i' << 16) | ('S' << 8) | 'u')
+
/* Socket related routines */
int master_connect(int port);
int apprentice_connect(const char *hostname, int port);
@@ -111,6 +116,7 @@ RisuResult recv_and_compare_register_info(void *uc);
* done in recv_and_compare_register_info().
*/
void report_mismatch_reg(void);
+void report_mismatch_header(void);
/* Interface provided by CPU-specific code: */
diff --git a/reginfo.c b/reginfo.c
index a007f16..f187d9c 100644
--- a/reginfo.c
+++ b/reginfo.c
@@ -16,6 +16,7 @@
static struct reginfo master_ri, apprentice_ri;
static uint8_t master_memblock[MEMBLOCKLEN];
+static trace_header_t master_header;
RisuResult send_register_info(void *uc)
{
@@ -23,32 +24,57 @@ RisuResult send_register_info(void *uc)
trace_header_t header;
RisuResult res;
RisuOp op;
+ void *extra;
reginfo_init(&ri, uc);
op = get_risuop(&ri);
/* Write a header with PC/op to keep in sync */
+ header.magic = RISU_MAGIC;
header.pc = get_pc(&ri);
header.risu_op = op;
+
+ switch (op) {
+ case OP_TESTEND:
+ case OP_COMPARE:
+ case OP_SIGILL:
+ header.size = reginfo_size();
+ extra = &ri;
+ break;
+
+ case OP_SETMEMBLOCK:
+ case OP_GETMEMBLOCK:
+ header.size = 0;
+ extra = NULL;
+ break;
+
+ case OP_COMPAREMEM:
+ header.size = MEMBLOCKLEN;
+ extra = memblock;
+ break;
+
+ default:
+ abort();
+ }
+
res = write_buffer(&header, sizeof(header));
if (res != RES_OK) {
return res;
}
+ if (extra) {
+ res = write_buffer(extra, header.size);
+ if (res != RES_OK) {
+ return res;
+ }
+ }
switch (op) {
case OP_COMPARE:
- case OP_TESTEND:
case OP_SIGILL:
- /*
- * Do a simple register compare on (a) explicit request
- * (b) end of test (c) a non-risuop UNDEF
- */
- res = write_buffer(&ri, reginfo_size());
- /* For OP_TEST_END, force exit. */
- if (res == RES_OK && op == OP_TESTEND) {
- res = RES_END;
- }
+ case OP_COMPAREMEM:
break;
+ case OP_TESTEND:
+ return RES_END;
case OP_SETMEMBLOCK:
memblock = (void *)(uintptr_t)get_reginfo_paramreg(&ri);
break;
@@ -56,12 +82,10 @@ RisuResult send_register_info(void *uc)
set_ucontext_paramreg(uc,
get_reginfo_paramreg(&ri) + (uintptr_t)memblock);
break;
- case OP_COMPAREMEM:
- return write_buffer(memblock, MEMBLOCKLEN);
default:
abort();
}
- return res;
+ return RES_OK;
}
/* Read register info from the socket and compare it with that from the
@@ -75,34 +99,48 @@ RisuResult send_register_info(void *uc)
RisuResult recv_and_compare_register_info(void *uc)
{
RisuResult res;
- trace_header_t header;
+ size_t extra_size;
RisuOp op;
reginfo_init(&apprentice_ri, uc);
op = get_risuop(&apprentice_ri);
- res = read_buffer(&header, sizeof(header));
- if (res != RES_OK) {
- return res;
+ switch (op) {
+ case OP_TESTEND:
+ case OP_COMPARE:
+ case OP_SIGILL:
+ extra_size = reginfo_size();
+ break;
+ case OP_SETMEMBLOCK:
+ case OP_GETMEMBLOCK:
+ extra_size = 0;
+ break;
+ case OP_COMPAREMEM:
+ extra_size = MEMBLOCKLEN;
+ break;
+ default:
+ abort();
}
- if (header.risu_op != op) {
- /* We are out of sync. Tell master to exit. */
- respond(RES_END);
- return RES_BAD_IO;
+ res = read_buffer(&master_header, sizeof(master_header));
+ if (res != RES_OK) {
+ goto fail_header;
+ }
+ if (master_header.magic != RISU_MAGIC ||
+ master_header.risu_op != op ||
+ master_header.size != extra_size) {
+ res = RES_MISMATCH_HEAD;
+ goto fail_header;
}
/* send OK for the header */
respond(RES_OK);
switch (op) {
- case OP_COMPARE:
case OP_TESTEND:
+ case OP_COMPARE:
case OP_SIGILL:
- /* Do a simple register compare on (a) explicit request
- * (b) end of test (c) a non-risuop UNDEF
- */
- res = read_buffer(&master_ri, reginfo_size());
+ res = read_buffer(&master_ri, extra_size);
if (res != RES_OK) {
/* fail */
} else if (!reginfo_is_eq(&master_ri, &apprentice_ri)) {
@@ -111,15 +149,17 @@ RisuResult recv_and_compare_register_info(void *uc)
} else if (op == OP_TESTEND) {
res = RES_END;
}
- respond(res == RES_OK ? RES_OK : RES_END);
break;
+
case OP_SETMEMBLOCK:
memblock = (void *)(uintptr_t)get_reginfo_paramreg(&apprentice_ri);
- break;
+ return RES_OK;
+
case OP_GETMEMBLOCK:
set_ucontext_paramreg(uc, get_reginfo_paramreg(&apprentice_ri) +
(uintptr_t)memblock);
- break;
+ return RES_OK;
+
case OP_COMPAREMEM:
res = read_buffer(master_memblock, MEMBLOCKLEN);
if (res != RES_OK) {
@@ -128,12 +168,14 @@ RisuResult recv_and_compare_register_info(void *uc)
/* memory mismatch */
res = RES_MISMATCH_MEM;
}
- respond(res == RES_OK ? RES_OK : RES_END);
break;
+
default:
abort();
}
+ fail_header:
+ respond(res == RES_OK ? RES_OK : RES_END);
return res;
}
@@ -149,3 +191,61 @@ void report_mismatch_reg(void)
reginfo_dump(&apprentice_ri, stderr);
reginfo_dump_mismatch(&master_ri, &apprentice_ri, stderr);
}
+
+void report_mismatch_header(void)
+{
+ fprintf(stderr, "header mismatch detail (master : apprentice):\n");
+ if (master_header.magic != RISU_MAGIC) {
+ fprintf(stderr, " magic: %08x vs %08x\n",
+ master_header.magic, RISU_MAGIC);
+ /* If the magic number is wrong, everything else is garbage too. */
+ return;
+ }
+
+ RisuOp a_op = get_risuop(&apprentice_ri);
+ RisuOp m_op = master_header.risu_op;
+ if (a_op != m_op) {
+ fprintf(stderr, " op : %d != %d\n", m_op, a_op);
+ /* If the opcode is mismatched, we can't compute size. */
+ } else {
+ const char *kind;
+ size_t m_sz = master_header.size;
+ size_t a_sz;
+
+ switch (a_op) {
+ case OP_TESTEND:
+ case OP_COMPARE:
+ case OP_SIGILL:
+ kind = "reginfo";
+ a_sz = reginfo_size();
+ break;
+ case OP_SETMEMBLOCK:
+ case OP_GETMEMBLOCK:
+ kind = "unexpected";
+ a_sz = 0;
+ break;
+ case OP_COMPAREMEM:
+ kind = "memblock";
+ a_sz = MEMBLOCKLEN;
+ break;
+ default:
+ abort();
+ }
+ if (a_sz != m_sz) {
+ fprintf(stderr, " size : %zd != %zd (%s)\n",
+ m_sz, a_sz, kind);
+ } else {
+ /* If magic, op, and size are the same, how did we get here? */
+ abort();
+ }
+ }
+
+ uint64_t a_pc = get_pc(&apprentice_ri);
+ uint64_t m_pc = master_header.pc;
+ if (a_pc != m_pc) {
+ fprintf(stderr, " pc : %016" PRIx64 " vs %016" PRIx64 "\n",
+ m_pc, a_pc);
+ } else {
+ fprintf(stderr, " pc : %016" PRIx64 "\n", a_pc);
+ }
+}
diff --git a/risu.c b/risu.c
index 398faac..95b4674 100644
--- a/risu.c
+++ b/risu.c
@@ -233,6 +233,12 @@ static int apprentice(void)
fprintf(stderr, "mismatch mem after %zd checkpoints\n", signal_count);
return EXIT_FAILURE;
+ case RES_MISMATCH_HEAD:
+ fprintf(stderr, "mismatch header after %zd checkpoints\n",
+ signal_count);
+ report_mismatch_header();
+ return EXIT_FAILURE;
+
case RES_BAD_IO:
fprintf(stderr, "i/o error after %zd checkpoints\n", signal_count);
return EXIT_FAILURE;
--
2.20.1
^ permalink raw reply related [flat|nested] 36+ messages in thread* Re: [RISU v2 14/17] Add magic and size to the trace header
2020-05-19 2:53 ` [RISU v2 14/17] Add magic and size to the trace header Richard Henderson
@ 2020-05-19 21:16 ` Richard Henderson
2020-05-20 16:59 ` Alex Bennée
0 siblings, 1 reply; 36+ messages in thread
From: Richard Henderson @ 2020-05-19 21:16 UTC (permalink / raw)
To: qemu-devel; +Cc: peter.maydell, alex.bennee
On 5/18/20 7:53 PM, Richard Henderson wrote:
> + if (master_header.magic != RISU_MAGIC ||
> + master_header.risu_op != op ||
> + master_header.size != extra_size) {
> + res = RES_MISMATCH_HEAD;
> + goto fail_header;
> }
Hmm. This isn't ideal.
Consider e.g. an insn being tested that should pass, so master steps past the
insn to the UDF and sends OP_COMPARE. But there's a bug in the emulator being
tested so the apprentice gets SIGILL on the insn and so op == OP_SIGILL.
So risu_op != op, but we only report the header difference.
Perhaps that's good enough to understand the this particular problem, without
the clutter of printing the rest of the reginfo frame -- at least if
report_mismatch_header is improved to print risu_op names instead of numbers.
Consider if master and apprentice are run with different --test-sve=<vq>
values. That will produce a mismatch in size.
Which could be a serious problem, if master_header.size > sizeof(master_ri) --
we can't even receive the data. In that case, what I'm doing here printing the
size mismatch is all that's possible.
But suppose master_header.size <= sizeof(master_ri), so we can receive the
data. So long as master_header.size == reginfo_size(&master_ri), then at least
the data is self-consistent, and we *can* print out the difference in
report_mismatch_reg(). Which in this case is going to be the difference in the
two ri->sve_vl values. That difference is likely to be easiest to understand
for the end user.
I should probably split out this receive logic from
recv_and_compare_register_info so that it can be reused by dump.
r~
^ permalink raw reply [flat|nested] 36+ messages in thread* Re: [RISU v2 14/17] Add magic and size to the trace header
2020-05-19 21:16 ` Richard Henderson
@ 2020-05-20 16:59 ` Alex Bennée
0 siblings, 0 replies; 36+ messages in thread
From: Alex Bennée @ 2020-05-20 16:59 UTC (permalink / raw)
To: Richard Henderson; +Cc: peter.maydell, qemu-devel
Richard Henderson <richard.henderson@linaro.org> writes:
> On 5/18/20 7:53 PM, Richard Henderson wrote:
>> + if (master_header.magic != RISU_MAGIC ||
>> + master_header.risu_op != op ||
>> + master_header.size != extra_size) {
>> + res = RES_MISMATCH_HEAD;
>> + goto fail_header;
>> }
>
> Hmm. This isn't ideal.
>
> Consider e.g. an insn being tested that should pass, so master steps past the
> insn to the UDF and sends OP_COMPARE. But there's a bug in the emulator being
> tested so the apprentice gets SIGILL on the insn and so op ==
> OP_SIGILL.
In my experience this is one of the major failure modes as usually it's
a decode error or a -cpu version error that leads you to go astray.
That said most useful additional piece of information to dump would be
the insn word of the failing PC. We even know if one is on a RISU_OP and
one isn't so we can be a bit more expressive.
> So risu_op != op, but we only report the header difference.
>
> Perhaps that's good enough to understand the this particular problem, without
> the clutter of printing the rest of the reginfo frame -- at least if
> report_mismatch_header is improved to print risu_op names instead of numbers.
>
> Consider if master and apprentice are run with different --test-sve=<vq>
> values. That will produce a mismatch in size.
>
> Which could be a serious problem, if master_header.size > sizeof(master_ri) --
> we can't even receive the data. In that case, what I'm doing here printing the
> size mismatch is all that's possible.
>
> But suppose master_header.size <= sizeof(master_ri), so we can receive the
> data. So long as master_header.size == reginfo_size(&master_ri), then at least
> the data is self-consistent, and we *can* print out the difference in
> report_mismatch_reg(). Which in this case is going to be the difference in the
> two ri->sve_vl values. That difference is likely to be easiest to understand
> for the end user.
>
> I should probably split out this receive logic from
> recv_and_compare_register_info so that it can be reused by dump.
>
>
> r~
--
Alex Bennée
^ permalink raw reply [flat|nested] 36+ messages in thread
* [RISU v2 15/17] Compute reginfo_size based on the reginfo
2020-05-19 2:53 [RISU v2 00/17] risu cleanups and improvements Richard Henderson
` (13 preceding siblings ...)
2020-05-19 2:53 ` [RISU v2 14/17] Add magic and size to the trace header Richard Henderson
@ 2020-05-19 2:53 ` Richard Henderson
2020-05-20 17:30 ` Alex Bennée
2020-05-19 2:53 ` [RISU v2 16/17] aarch64: Reorg sve reginfo to save space Richard Henderson
2020-05-19 2:53 ` [RISU v2 17/17] Add --dump option to inspect trace files Richard Henderson
16 siblings, 1 reply; 36+ messages in thread
From: Richard Henderson @ 2020-05-19 2:53 UTC (permalink / raw)
To: qemu-devel; +Cc: peter.maydell, alex.bennee
This will allow dumping of SVE frames without having
to know the SVE vector length beforehand.
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
risu.h | 2 +-
reginfo.c | 6 +++---
risu_reginfo_aarch64.c | 4 ++--
risu_reginfo_arm.c | 2 +-
risu_reginfo_i386.c | 2 +-
risu_reginfo_m68k.c | 2 +-
risu_reginfo_ppc64.c | 2 +-
7 files changed, 10 insertions(+), 10 deletions(-)
diff --git a/risu.h b/risu.h
index eeb6775..054cef7 100644
--- a/risu.h
+++ b/risu.h
@@ -155,6 +155,6 @@ int reginfo_dump(struct reginfo *ri, FILE * f);
int reginfo_dump_mismatch(struct reginfo *m, struct reginfo *a, FILE *f);
/* return size of reginfo */
-const int reginfo_size(void);
+int reginfo_size(struct reginfo *ri);
#endif /* RISU_H */
diff --git a/reginfo.c b/reginfo.c
index f187d9c..411c2a6 100644
--- a/reginfo.c
+++ b/reginfo.c
@@ -38,7 +38,7 @@ RisuResult send_register_info(void *uc)
case OP_TESTEND:
case OP_COMPARE:
case OP_SIGILL:
- header.size = reginfo_size();
+ header.size = reginfo_size(&ri);
extra = &ri;
break;
@@ -109,7 +109,7 @@ RisuResult recv_and_compare_register_info(void *uc)
case OP_TESTEND:
case OP_COMPARE:
case OP_SIGILL:
- extra_size = reginfo_size();
+ extra_size = reginfo_size(&master_ri);
break;
case OP_SETMEMBLOCK:
case OP_GETMEMBLOCK:
@@ -217,7 +217,7 @@ void report_mismatch_header(void)
case OP_COMPARE:
case OP_SIGILL:
kind = "reginfo";
- a_sz = reginfo_size();
+ a_sz = reginfo_size(&apprentice_ri);
break;
case OP_SETMEMBLOCK:
case OP_GETMEMBLOCK:
diff --git a/risu_reginfo_aarch64.c b/risu_reginfo_aarch64.c
index 028c690..7044648 100644
--- a/risu_reginfo_aarch64.c
+++ b/risu_reginfo_aarch64.c
@@ -69,7 +69,7 @@ void process_arch_opt(int opt, const char *arg)
#endif
}
-const int reginfo_size(void)
+int reginfo_size(struct reginfo *ri)
{
int size = offsetof(struct reginfo, simd.end);
#ifdef SVE_MAGIC
@@ -194,7 +194,7 @@ void reginfo_init(struct reginfo *ri, ucontext_t *uc)
/* reginfo_is_eq: compare the reginfo structs, returns nonzero if equal */
int reginfo_is_eq(struct reginfo *r1, struct reginfo *r2)
{
- return memcmp(r1, r2, reginfo_size()) == 0;
+ return memcmp(r1, r2, reginfo_size(r1)) == 0;
}
#ifdef SVE_MAGIC
diff --git a/risu_reginfo_arm.c b/risu_reginfo_arm.c
index 3662f12..3832e27 100644
--- a/risu_reginfo_arm.c
+++ b/risu_reginfo_arm.c
@@ -36,7 +36,7 @@ void process_arch_opt(int opt, const char *arg)
abort();
}
-const int reginfo_size(void)
+int reginfo_size(struct reginfo *ri)
{
return sizeof(struct reginfo);
}
diff --git a/risu_reginfo_i386.c b/risu_reginfo_i386.c
index 60fc239..902d33e 100644
--- a/risu_reginfo_i386.c
+++ b/risu_reginfo_i386.c
@@ -74,7 +74,7 @@ void process_arch_opt(int opt, const char *arg)
}
}
-const int reginfo_size(void)
+int reginfo_size(struct reginfo *ri)
{
return sizeof(struct reginfo);
}
diff --git a/risu_reginfo_m68k.c b/risu_reginfo_m68k.c
index 32b28c8..361f172 100644
--- a/risu_reginfo_m68k.c
+++ b/risu_reginfo_m68k.c
@@ -23,7 +23,7 @@ void process_arch_opt(int opt, const char *arg)
abort();
}
-const int reginfo_size(void)
+int reginfo_size(struct reginfo *ri)
{
return sizeof(struct reginfo);
}
diff --git a/risu_reginfo_ppc64.c b/risu_reginfo_ppc64.c
index 071c951..c86313c 100644
--- a/risu_reginfo_ppc64.c
+++ b/risu_reginfo_ppc64.c
@@ -32,7 +32,7 @@ void process_arch_opt(int opt, const char *arg)
abort();
}
-const int reginfo_size(void)
+int reginfo_size(struct reginfo *ri)
{
return sizeof(struct reginfo);
}
--
2.20.1
^ permalink raw reply related [flat|nested] 36+ messages in thread* Re: [RISU v2 15/17] Compute reginfo_size based on the reginfo
2020-05-19 2:53 ` [RISU v2 15/17] Compute reginfo_size based on the reginfo Richard Henderson
@ 2020-05-20 17:30 ` Alex Bennée
0 siblings, 0 replies; 36+ messages in thread
From: Alex Bennée @ 2020-05-20 17:30 UTC (permalink / raw)
To: Richard Henderson; +Cc: peter.maydell, qemu-devel
Richard Henderson <richard.henderson@linaro.org> writes:
> This will allow dumping of SVE frames without having
> to know the SVE vector length beforehand.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
> risu.h | 2 +-
> reginfo.c | 6 +++---
> risu_reginfo_aarch64.c | 4 ++--
> risu_reginfo_arm.c | 2 +-
> risu_reginfo_i386.c | 2 +-
> risu_reginfo_m68k.c | 2 +-
> risu_reginfo_ppc64.c | 2 +-
> 7 files changed, 10 insertions(+), 10 deletions(-)
>
> diff --git a/risu.h b/risu.h
> index eeb6775..054cef7 100644
> --- a/risu.h
> +++ b/risu.h
> @@ -155,6 +155,6 @@ int reginfo_dump(struct reginfo *ri, FILE * f);
> int reginfo_dump_mismatch(struct reginfo *m, struct reginfo *a, FILE *f);
>
> /* return size of reginfo */
> -const int reginfo_size(void);
> +int reginfo_size(struct reginfo *ri);
>
> #endif /* RISU_H */
> diff --git a/reginfo.c b/reginfo.c
> index f187d9c..411c2a6 100644
> --- a/reginfo.c
> +++ b/reginfo.c
> @@ -38,7 +38,7 @@ RisuResult send_register_info(void *uc)
> case OP_TESTEND:
> case OP_COMPARE:
> case OP_SIGILL:
> - header.size = reginfo_size();
> + header.size = reginfo_size(&ri);
> extra = &ri;
> break;
>
> @@ -109,7 +109,7 @@ RisuResult recv_and_compare_register_info(void *uc)
> case OP_TESTEND:
> case OP_COMPARE:
> case OP_SIGILL:
> - extra_size = reginfo_size();
> + extra_size = reginfo_size(&master_ri);
> break;
> case OP_SETMEMBLOCK:
> case OP_GETMEMBLOCK:
> @@ -217,7 +217,7 @@ void report_mismatch_header(void)
> case OP_COMPARE:
> case OP_SIGILL:
> kind = "reginfo";
> - a_sz = reginfo_size();
> + a_sz = reginfo_size(&apprentice_ri);
> break;
> case OP_SETMEMBLOCK:
> case OP_GETMEMBLOCK:
> diff --git a/risu_reginfo_aarch64.c b/risu_reginfo_aarch64.c
> index 028c690..7044648 100644
> --- a/risu_reginfo_aarch64.c
> +++ b/risu_reginfo_aarch64.c
> @@ -69,7 +69,7 @@ void process_arch_opt(int opt, const char *arg)
> #endif
> }
>
> -const int reginfo_size(void)
> +int reginfo_size(struct reginfo *ri)
> {
> int size = offsetof(struct reginfo, simd.end);
> #ifdef SVE_MAGIC
> @@ -194,7 +194,7 @@ void reginfo_init(struct reginfo *ri, ucontext_t *uc)
> /* reginfo_is_eq: compare the reginfo structs, returns nonzero if equal */
> int reginfo_is_eq(struct reginfo *r1, struct reginfo *r2)
> {
> - return memcmp(r1, r2, reginfo_size()) == 0;
> + return memcmp(r1, r2, reginfo_size(r1)) == 0;
> }
>
> #ifdef SVE_MAGIC
> diff --git a/risu_reginfo_arm.c b/risu_reginfo_arm.c
> index 3662f12..3832e27 100644
> --- a/risu_reginfo_arm.c
> +++ b/risu_reginfo_arm.c
> @@ -36,7 +36,7 @@ void process_arch_opt(int opt, const char *arg)
> abort();
> }
>
> -const int reginfo_size(void)
> +int reginfo_size(struct reginfo *ri)
> {
> return sizeof(struct reginfo);
I wonder if the fixed size architectures should return (sizeof *ri) to
reinforce the point? Anyway:
Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
--
Alex Bennée
^ permalink raw reply [flat|nested] 36+ messages in thread
* [RISU v2 16/17] aarch64: Reorg sve reginfo to save space
2020-05-19 2:53 [RISU v2 00/17] risu cleanups and improvements Richard Henderson
` (14 preceding siblings ...)
2020-05-19 2:53 ` [RISU v2 15/17] Compute reginfo_size based on the reginfo Richard Henderson
@ 2020-05-19 2:53 ` Richard Henderson
2020-05-20 18:43 ` Alex Bennée
2020-05-19 2:53 ` [RISU v2 17/17] Add --dump option to inspect trace files Richard Henderson
16 siblings, 1 reply; 36+ messages in thread
From: Richard Henderson @ 2020-05-19 2:53 UTC (permalink / raw)
To: qemu-devel; +Cc: peter.maydell, alex.bennee
Mirror the signal frame by storing all of the registers
as a lump. Use the signal macros to pull out the values.
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
risu_reginfo_aarch64.h | 16 +----
risu_reginfo_aarch64.c | 135 +++++++++++++++++++++--------------------
2 files changed, 73 insertions(+), 78 deletions(-)
diff --git a/risu_reginfo_aarch64.h b/risu_reginfo_aarch64.h
index c33b86f..01076b4 100644
--- a/risu_reginfo_aarch64.h
+++ b/risu_reginfo_aarch64.h
@@ -17,20 +17,8 @@
struct simd_reginfo {
__uint128_t vregs[32];
- char end[0];
};
-#ifdef SVE_MAGIC
-struct sve_reginfo {
- /* SVE */
- uint16_t vl; /* current VL */
- __uint128_t zregs[SVE_NUM_ZREGS][SVE_VQ_MAX];
- uint16_t pregs[SVE_NUM_PREGS][SVE_VQ_MAX];
- uint16_t ffr[SVE_VQ_MAX];
- char end[0];
-};
-#endif
-
/* The kernel headers set this based on future arch extensions.
The current arch maximum is 16. Save space below. */
#undef SVE_VQ_MAX
@@ -47,11 +35,13 @@ struct reginfo {
/* FP/SIMD */
uint32_t fpsr;
uint32_t fpcr;
+ uint32_t sve_vl;
union {
struct simd_reginfo simd;
#ifdef SVE_MAGIC
- struct sve_reginfo sve;
+ char sve[SVE_SIG_CONTEXT_SIZE(16) - SVE_SIG_REGS_OFFSET]
+ __attribute__((aligned(16)));
#endif
};
};
diff --git a/risu_reginfo_aarch64.c b/risu_reginfo_aarch64.c
index 7044648..a1020ac 100644
--- a/risu_reginfo_aarch64.c
+++ b/risu_reginfo_aarch64.c
@@ -71,15 +71,30 @@ void process_arch_opt(int opt, const char *arg)
int reginfo_size(struct reginfo *ri)
{
- int size = offsetof(struct reginfo, simd.end);
#ifdef SVE_MAGIC
- if (test_sve) {
- size = offsetof(struct reginfo, sve.end);
+ if (ri->sve_vl) {
+ int vq = sve_vq_from_vl(ri->sve_vl);
+ return (offsetof(struct reginfo, sve) +
+ SVE_SIG_CONTEXT_SIZE(vq) - SVE_SIG_REGS_OFFSET);
}
#endif
- return size;
+ return offsetof(struct reginfo, simd) + sizeof(ri->simd);
}
+#ifdef SVE_MAGIC
+static uint64_t *reginfo_zreg(struct reginfo *ri, int vq, int i)
+{
+ return (uint64_t *)(ri->sve + SVE_SIG_ZREG_OFFSET(vq, i) -
+ SVE_SIG_REGS_OFFSET);
+}
+
+static uint16_t *reginfo_preg(struct reginfo *ri, int vq, int i)
+{
+ return (uint16_t *)(ri->sve + SVE_SIG_PREG_OFFSET(vq, i) -
+ SVE_SIG_REGS_OFFSET);
+}
+#endif
+
/* reginfo_init: initialize with a ucontext */
void reginfo_init(struct reginfo *ri, ucontext_t *uc)
{
@@ -152,8 +167,6 @@ void reginfo_init(struct reginfo *ri, ucontext_t *uc)
return;
}
- ri->sve.vl = sve->vl;
-
if (sve->head.size < SVE_SIG_CONTEXT_SIZE(vq)) {
if (sve->head.size == sizeof(*sve)) {
/* SVE state is empty -- not an error. */
@@ -164,24 +177,9 @@ void reginfo_init(struct reginfo *ri, ucontext_t *uc)
return;
}
- /* Copy ZREG's one at a time */
- for (i = 0; i < SVE_NUM_ZREGS; i++) {
- memcpy(&ri->sve.zregs[i],
- (void *)sve + SVE_SIG_ZREG_OFFSET(vq, i),
- SVE_SIG_ZREG_SIZE(vq));
- }
-
- /* Copy PREG's one at a time */
- for (i = 0; i < SVE_NUM_PREGS; i++) {
- memcpy(&ri->sve.pregs[i],
- (void *)sve + SVE_SIG_PREG_OFFSET(vq, i),
- SVE_SIG_PREG_SIZE(vq));
- }
-
- /* Finally the FFR */
- memcpy(&ri->sve.ffr, (void *)sve + SVE_SIG_FFR_OFFSET(vq),
- SVE_SIG_FFR_SIZE(vq));
-
+ ri->sve_vl = sve->vl;
+ memcpy(ri->sve, (char *)sve + SVE_SIG_REGS_OFFSET,
+ SVE_SIG_CONTEXT_SIZE(vq) - SVE_SIG_REGS_OFFSET);
return;
}
#endif /* SVE_MAGIC */
@@ -225,18 +223,20 @@ static void sve_dump_preg_diff(FILE *f, int vq, const uint16_t *p1,
fprintf(f, "\n");
}
-static void sve_dump_zreg_diff(FILE *f, int vq, const __uint128_t *z1,
- const __uint128_t *z2)
+static void sve_dump_zreg_diff(FILE *f, int vq, const uint64_t *za,
+ const uint64_t *zb)
{
const char *pad = "";
int q;
for (q = 0; q < vq; ++q) {
- if (z1[q] != z2[q]) {
+ uint64_t za0 = za[2 * q], za1 = za[2 * q + 1];
+ uint64_t zb0 = zb[2 * q], zb1 = zb[2 * q + 1];
+
+ if (za0 != zb0 || za1 != zb1) {
fprintf(f, "%sq%-2d: %016" PRIx64 "%016" PRIx64
- " vs %016" PRIx64 "%016" PRIx64"\n", pad, q,
- (uint64_t)(z1[q] >> 64), (uint64_t)z1[q],
- (uint64_t)(z2[q] >> 64), (uint64_t)z2[q]);
+ " vs %016" PRIx64 "%016" PRIx64"\n",
+ pad, q, za1, za0, zb1, zb0);
pad = " ";
}
}
@@ -263,28 +263,30 @@ int reginfo_dump(struct reginfo *ri, FILE * f)
if (test_sve) {
int q, vq = test_sve;
- fprintf(f, " vl : %d\n", ri->sve.vl);
+ fprintf(f, " vl : %d\n", ri->sve_vl);
- for (i = 0; i < 32; i++) {
- fprintf(f, " Z%-2d q%-2d: %016" PRIx64 "%016" PRIx64 "\n", i, 0,
- (uint64_t)(ri->sve.zregs[i][0] >> 64),
- (uint64_t)ri->sve.zregs[i][0]);
+ for (i = 0; i < SVE_NUM_ZREGS; i++) {
+ uint64_t *z = reginfo_zreg(ri, vq, i);
+
+ fprintf(f, " Z%-2d q%-2d: %016" PRIx64 "%016" PRIx64 "\n",
+ i, 0, z[1], z[0]);
for (q = 1; q < vq; ++q) {
- fprintf(f, " q%-2d: %016" PRIx64 "%016" PRIx64 "\n", q,
- (uint64_t)(ri->sve.zregs[i][q] >> 64),
- (uint64_t)ri->sve.zregs[i][q]);
+ fprintf(f, " q%-2d: %016" PRIx64 "%016" PRIx64 "\n",
+ q, z[q * 2 + 1], z[q * 2]);
}
}
- for (i = 0; i < 16; i++) {
- fprintf(f, " P%-2d : ", i);
- sve_dump_preg(f, vq, &ri->sve.pregs[i][0]);
+ for (i = 0; i < SVE_NUM_PREGS + 1; i++) {
+ uint16_t *p = reginfo_preg(ri, vq, i);
+
+ if (i == SVE_NUM_PREGS) {
+ fprintf(f, " FFR : ");
+ } else {
+ fprintf(f, " P%-2d : ", i);
+ }
+ sve_dump_preg(f, vq, p);
fprintf(f, "\n");
}
- fprintf(f, " FFR : ");
- sve_dump_preg(f, vq, &ri->sve.ffr[0]);
- fprintf(f, "\n");
-
return !ferror(f);
}
#endif
@@ -338,31 +340,34 @@ int reginfo_dump_mismatch(struct reginfo *m, struct reginfo *a, FILE * f)
#ifdef SVE_MAGIC
if (test_sve) {
- int vq = sve_vq_from_vl(m->sve.vl);
+ int vq = sve_vq_from_vl(m->sve_vl);
- if (m->sve.vl != a->sve.vl) {
- fprintf(f, " vl : %d vs %d\n", m->sve.vl, a->sve.vl);
+ if (m->sve_vl != a->sve_vl) {
+ fprintf(f, " vl : %d vs %d\n", m->sve_vl, a->sve_vl);
}
for (i = 0; i < SVE_NUM_ZREGS; i++) {
- if (!sve_zreg_is_eq(vq, &m->sve.zregs[i], &a->sve.zregs[i])) {
- fprintf(f, " Z%-2d ", i);
- sve_dump_zreg_diff(f, vq, &m->sve.zregs[i][0],
- &a->sve.zregs[i][0]);
- }
- }
- for (i = 0; i < SVE_NUM_PREGS; i++) {
- if (!sve_preg_is_eq(vq, &m->sve.pregs[i], &a->sve.pregs[i])) {
- fprintf(f, " P%-2d : ", i);
- sve_dump_preg_diff(f, vq, &m->sve.pregs[i][0],
- &a->sve.pregs[i][0]);
- }
- }
- if (!sve_preg_is_eq(vq, &m->sve.ffr, &a->sve.ffr)) {
- fprintf(f, " FFR : ");
- sve_dump_preg_diff(f, vq, &m->sve.pregs[i][0], &a->sve.pregs[i][0]);
- }
+ uint64_t *zm = reginfo_zreg(m, vq, i);
+ uint64_t *za = reginfo_zreg(a, vq, i);
+ if (!sve_zreg_is_eq(vq, zm, za)) {
+ fprintf(f, " Z%-2d ", i);
+ sve_dump_zreg_diff(f, vq, zm, za);
+ }
+ }
+ for (i = 0; i < SVE_NUM_PREGS + 1; i++) {
+ uint16_t *pm = reginfo_preg(m, vq, i);
+ uint16_t *pa = reginfo_preg(a, vq, i);
+
+ if (!sve_preg_is_eq(vq, pm, pa)) {
+ if (i == SVE_NUM_PREGS) {
+ fprintf(f, " FFR : ");
+ } else {
+ fprintf(f, " P%-2d : ", i);
+ }
+ sve_dump_preg_diff(f, vq, pm, pa);
+ }
+ }
return !ferror(f);
}
#endif
--
2.20.1
^ permalink raw reply related [flat|nested] 36+ messages in thread* Re: [RISU v2 16/17] aarch64: Reorg sve reginfo to save space
2020-05-19 2:53 ` [RISU v2 16/17] aarch64: Reorg sve reginfo to save space Richard Henderson
@ 2020-05-20 18:43 ` Alex Bennée
0 siblings, 0 replies; 36+ messages in thread
From: Alex Bennée @ 2020-05-20 18:43 UTC (permalink / raw)
To: Richard Henderson; +Cc: peter.maydell, qemu-devel
Richard Henderson <richard.henderson@linaro.org> writes:
> Mirror the signal frame by storing all of the registers
> as a lump. Use the signal macros to pull out the values.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
I haven't been able to get my model to boot yet to test but I'm pretty
happy with the reported change in trace size.
--
Alex Bennée
^ permalink raw reply [flat|nested] 36+ messages in thread
* [RISU v2 17/17] Add --dump option to inspect trace files
2020-05-19 2:53 [RISU v2 00/17] risu cleanups and improvements Richard Henderson
` (15 preceding siblings ...)
2020-05-19 2:53 ` [RISU v2 16/17] aarch64: Reorg sve reginfo to save space Richard Henderson
@ 2020-05-19 2:53 ` Richard Henderson
2020-05-20 17:49 ` Alex Bennée
16 siblings, 1 reply; 36+ messages in thread
From: Richard Henderson @ 2020-05-19 2:53 UTC (permalink / raw)
To: qemu-devel; +Cc: peter.maydell, alex.bennee
Adjust some of the aarch64 code to look at the reginfo struct
instead of looking at test_sve, so that we do not need to pass
the --test-sve option in order to dump sve trace files.
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
risu.h | 1 +
risu.c | 108 ++++++++++++++++++++++++++++++++++++++++-
risu_reginfo_aarch64.c | 55 +++++++++++++--------
risu_reginfo_arm.c | 4 ++
risu_reginfo_i386.c | 4 ++
risu_reginfo_m68k.c | 4 ++
risu_reginfo_ppc64.c | 4 ++
7 files changed, 159 insertions(+), 21 deletions(-)
diff --git a/risu.h b/risu.h
index 054cef7..3bfe765 100644
--- a/risu.h
+++ b/risu.h
@@ -23,6 +23,7 @@
extern const struct option * const arch_long_opts;
extern const char * const arch_extra_help;
void process_arch_opt(int opt, const char *arg);
+void arch_init(void);
#define FIRST_ARCH_OPT 0x100
/* GCC computed include to pull in the correct risu_reginfo_*.h for
diff --git a/risu.c b/risu.c
index 95b4674..d7c7556 100644
--- a/risu.c
+++ b/risu.c
@@ -249,6 +249,93 @@ static int apprentice(void)
}
}
+static int dump_trace(void)
+{
+ trace_header_t header;
+ union {
+ struct reginfo ri;
+ unsigned char memblock[MEMBLOCKLEN];
+ } u;
+ const char *op_name;
+
+ while (1) {
+ if (read_buffer(&header, sizeof(header))) {
+ fprintf(stderr, "Trace header read failed\n");
+ return EXIT_FAILURE;
+ }
+
+ if (header.magic != RISU_MAGIC) {
+ fprintf(stderr, "Unexpected header magic (%#x)\n", header.magic);
+ return EXIT_FAILURE;
+ }
+
+ switch (header.risu_op) {
+ case OP_COMPARE:
+ op_name = "COMPARE";
+ break;
+ case OP_TESTEND:
+ op_name = "TESTEND";
+ break;
+ case OP_SETMEMBLOCK:
+ op_name = "SETMEMBLOCK";
+ break;
+ case OP_GETMEMBLOCK:
+ op_name = "GETMEMBLOCK";
+ break;
+ case OP_COMPAREMEM:
+ op_name = "COMPAREMEM";
+ break;
+ case OP_SIGILL:
+ op_name = "SIGILL";
+ break;
+ default:
+ op_name = "<unknown>";
+ break;
+ }
+
+ switch (header.risu_op) {
+ case OP_COMPARE:
+ case OP_TESTEND:
+ case OP_SIGILL:
+ if (header.size > sizeof(u.ri)) {
+ fprintf(stderr, "Unexpected trace size (%u)\n", header.size);
+ return EXIT_FAILURE;
+ }
+ if (read_buffer(&u.ri, header.size)) {
+ fprintf(stderr, "Reginfo read failed\n");
+ return EXIT_FAILURE;
+ }
+ if (header.size != reginfo_size(&u.ri)) {
+ fprintf(stderr, "Unexpected trace size (%u)\n", header.size);
+ return EXIT_FAILURE;
+ }
+ printf("%s: (pc %#lx)\n", op_name, (unsigned long)header.pc);
+ reginfo_dump(&u.ri, stdout);
+ putchar('\n');
+ if (header.risu_op == OP_TESTEND) {
+ return EXIT_SUCCESS;
+ }
+ break;
+
+ case OP_COMPAREMEM:
+ if (header.size != MEMBLOCKLEN) {
+ fprintf(stderr, "Unexpected trace size (%u)\n", header.size);
+ return EXIT_FAILURE;
+ }
+ if (read_buffer(&u.memblock, MEMBLOCKLEN)) {
+ fprintf(stderr, "Memblock read failed\n");
+ return EXIT_FAILURE;
+ }
+ /* TODO: Dump 8k of data? */
+ /* fall through */
+
+ default:
+ printf("%s\n", op_name);
+ break;
+ }
+ }
+}
+
static int ismaster;
static void usage(void)
@@ -261,6 +348,7 @@ static 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, " -d, --dump=FILE Dump " TRACE_TYPE " trace file\n");
fprintf(stderr, " -t, --trace=FILE Record/playback " TRACE_TYPE " trace file\n");
fprintf(stderr,
" -h, --host=HOST Specify master host machine (apprentice only)"
@@ -281,11 +369,12 @@ static struct option * setup_options(char **short_opts)
{"host", required_argument, 0, 'h'},
{"port", required_argument, 0, 'p'},
{"trace", required_argument, 0, 't'},
+ {"dump", required_argument, 0, 'd'},
{0, 0, 0, 0}
};
struct option *lopts = &default_longopts[0];
- *short_opts = "h:p:t:";
+ *short_opts = "d:h:p:t:";
if (arch_long_opts) {
const size_t osize = sizeof(struct option);
@@ -316,6 +405,7 @@ int main(int argc, char **argv)
char *trace_fn = NULL;
struct option *longopts;
char *shortopts;
+ bool dump = false;
longopts = setup_options(&shortopts);
@@ -330,6 +420,10 @@ int main(int argc, char **argv)
case 0:
/* flag set by getopt_long, do nothing */
break;
+ case 'd':
+ trace_fn = optarg;
+ trace = dump = true;
+ break;
case 't':
trace_fn = optarg;
trace = true;
@@ -351,6 +445,11 @@ int main(int argc, char **argv)
}
}
+ if (dump && ismaster) {
+ usage();
+ exit(1);
+ }
+
if (trace) {
if (strcmp(trace_fn, "-") == 0) {
comm_fd = ismaster ? STDOUT_FILENO : STDIN_FILENO;
@@ -366,6 +465,10 @@ int main(int argc, char **argv)
}
}
+ if (dump) {
+ return dump_trace();
+ }
+
imgfile = argv[optind];
if (!imgfile) {
fprintf(stderr, "Error: must specify image file name\n\n");
@@ -375,6 +478,9 @@ int main(int argc, char **argv)
load_image(imgfile);
+ /* Select requested SVE vector length. */
+ arch_init();
+
if (ismaster) {
if (!trace) {
fprintf(stderr, "master port %d\n", port);
diff --git a/risu_reginfo_aarch64.c b/risu_reginfo_aarch64.c
index a1020ac..fb8e11a 100644
--- a/risu_reginfo_aarch64.c
+++ b/risu_reginfo_aarch64.c
@@ -44,8 +44,6 @@ const char * const arch_extra_help
void process_arch_opt(int opt, const char *arg)
{
#ifdef SVE_MAGIC
- long want, got;
-
assert(opt == FIRST_ARCH_OPT);
test_sve = strtol(arg, 0, 10);
@@ -53,22 +51,37 @@ void process_arch_opt(int opt, const char *arg)
fprintf(stderr, "Invalid value for VQ (1-%d)\n", SVE_VQ_MAX);
exit(EXIT_FAILURE);
}
- want = sve_vl_from_vq(test_sve);
- got = prctl(PR_SVE_SET_VL, want);
- if (want != got) {
- if (got < 0) {
- perror("prctl PR_SVE_SET_VL");
- } else {
- fprintf(stderr, "Unsupported value for VQ (%d != %d)\n",
- test_sve, (int)sve_vq_from_vl(got));
- }
- exit(EXIT_FAILURE);
- }
#else
abort();
#endif
}
+void arch_init(void)
+{
+#ifdef SVE_MAGIC
+ long want, got1, got2;
+
+ if (test_sve == 0) {
+ return;
+ }
+
+ want = sve_vl_from_vq(test_sve);
+ asm(".arch_extension sve\n\trdvl %0, #1" : "=r"(got1));
+ if (want != got1) {
+ got2 = prctl(PR_SVE_SET_VL, want);
+ if (want != got2) {
+ if (got2 < 0) {
+ perror("prctl PR_SVE_SET_VL");
+ got2 = got1;
+ }
+ fprintf(stderr, "Unsupported value for VQ (%d != %d)\n",
+ test_sve, (int)sve_vq_from_vl(got1));
+ exit(EXIT_FAILURE);
+ }
+ }
+#endif
+}
+
int reginfo_size(struct reginfo *ri)
{
#ifdef SVE_MAGIC
@@ -170,6 +183,7 @@ void reginfo_init(struct reginfo *ri, ucontext_t *uc)
if (sve->head.size < SVE_SIG_CONTEXT_SIZE(vq)) {
if (sve->head.size == sizeof(*sve)) {
/* SVE state is empty -- not an error. */
+ goto do_simd;
} else {
fprintf(stderr, "risu_reginfo_aarch64: "
"failed to get complete SVE state\n");
@@ -182,6 +196,7 @@ void reginfo_init(struct reginfo *ri, ucontext_t *uc)
SVE_SIG_CONTEXT_SIZE(vq) - SVE_SIG_REGS_OFFSET);
return;
}
+ do_simd:
#endif /* SVE_MAGIC */
for (i = 0; i < 32; i++) {
@@ -260,8 +275,9 @@ int reginfo_dump(struct reginfo *ri, FILE * f)
fprintf(f, " fpcr : %08x\n", ri->fpcr);
#ifdef SVE_MAGIC
- if (test_sve) {
- int q, vq = test_sve;
+ if (ri->sve_vl) {
+ int vq = sve_vq_from_vl(ri->sve_vl);
+ int q;
fprintf(f, " vl : %d\n", ri->sve_vl);
@@ -339,13 +355,12 @@ int reginfo_dump_mismatch(struct reginfo *m, struct reginfo *a, FILE * f)
}
#ifdef SVE_MAGIC
- if (test_sve) {
+ if (m->sve_vl != a->sve_vl) {
+ fprintf(f, " vl : %d vs %d\n", m->sve_vl, a->sve_vl);
+ }
+ if (m->sve_vl) {
int vq = sve_vq_from_vl(m->sve_vl);
- if (m->sve_vl != a->sve_vl) {
- fprintf(f, " vl : %d vs %d\n", m->sve_vl, a->sve_vl);
- }
-
for (i = 0; i < SVE_NUM_ZREGS; i++) {
uint64_t *zm = reginfo_zreg(m, vq, i);
uint64_t *za = reginfo_zreg(a, vq, i);
diff --git a/risu_reginfo_arm.c b/risu_reginfo_arm.c
index 3832e27..2982435 100644
--- a/risu_reginfo_arm.c
+++ b/risu_reginfo_arm.c
@@ -36,6 +36,10 @@ void process_arch_opt(int opt, const char *arg)
abort();
}
+void arch_init(void)
+{
+}
+
int reginfo_size(struct reginfo *ri)
{
return sizeof(struct reginfo);
diff --git a/risu_reginfo_i386.c b/risu_reginfo_i386.c
index 902d33e..68f2323 100644
--- a/risu_reginfo_i386.c
+++ b/risu_reginfo_i386.c
@@ -74,6 +74,10 @@ void process_arch_opt(int opt, const char *arg)
}
}
+void arch_init(void)
+{
+}
+
int reginfo_size(struct reginfo *ri)
{
return sizeof(struct reginfo);
diff --git a/risu_reginfo_m68k.c b/risu_reginfo_m68k.c
index 361f172..499fdc4 100644
--- a/risu_reginfo_m68k.c
+++ b/risu_reginfo_m68k.c
@@ -23,6 +23,10 @@ void process_arch_opt(int opt, const char *arg)
abort();
}
+void arch_init(void)
+{
+}
+
int reginfo_size(struct reginfo *ri)
{
return sizeof(struct reginfo);
diff --git a/risu_reginfo_ppc64.c b/risu_reginfo_ppc64.c
index c86313c..3b04747 100644
--- a/risu_reginfo_ppc64.c
+++ b/risu_reginfo_ppc64.c
@@ -32,6 +32,10 @@ void process_arch_opt(int opt, const char *arg)
abort();
}
+void arch_init(void)
+{
+}
+
int reginfo_size(struct reginfo *ri)
{
return sizeof(struct reginfo);
--
2.20.1
^ permalink raw reply related [flat|nested] 36+ messages in thread* Re: [RISU v2 17/17] Add --dump option to inspect trace files
2020-05-19 2:53 ` [RISU v2 17/17] Add --dump option to inspect trace files Richard Henderson
@ 2020-05-20 17:49 ` Alex Bennée
0 siblings, 0 replies; 36+ messages in thread
From: Alex Bennée @ 2020-05-20 17:49 UTC (permalink / raw)
To: Richard Henderson; +Cc: peter.maydell, qemu-devel
Richard Henderson <richard.henderson@linaro.org> writes:
> Adjust some of the aarch64 code to look at the reginfo struct
> instead of looking at test_sve, so that we do not need to pass
> the --test-sve option in order to dump sve trace files.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
> risu.h | 1 +
> risu.c | 108 ++++++++++++++++++++++++++++++++++++++++-
> risu_reginfo_aarch64.c | 55 +++++++++++++--------
> risu_reginfo_arm.c | 4 ++
> risu_reginfo_i386.c | 4 ++
> risu_reginfo_m68k.c | 4 ++
> risu_reginfo_ppc64.c | 4 ++
> 7 files changed, 159 insertions(+), 21 deletions(-)
>
> diff --git a/risu.h b/risu.h
> index 054cef7..3bfe765 100644
> --- a/risu.h
> +++ b/risu.h
> @@ -23,6 +23,7 @@
> extern const struct option * const arch_long_opts;
> extern const char * const arch_extra_help;
> void process_arch_opt(int opt, const char *arg);
> +void arch_init(void);
> #define FIRST_ARCH_OPT 0x100
>
> /* GCC computed include to pull in the correct risu_reginfo_*.h for
> diff --git a/risu.c b/risu.c
> index 95b4674..d7c7556 100644
> --- a/risu.c
> +++ b/risu.c
> @@ -249,6 +249,93 @@ static int apprentice(void)
> }
> }
>
> +static int dump_trace(void)
> +{
> + trace_header_t header;
> + union {
> + struct reginfo ri;
> + unsigned char memblock[MEMBLOCKLEN];
> + } u;
If you kept a p(rev) copy you could also add an option to:
> + const char *op_name;
> +
<snip>
> + printf("%s: (pc %#lx)\n", op_name, (unsigned long)header.pc);
> + reginfo_dump(&u.ri, stdout);
optionally call reginfo_dump_mismatch(&u.ri, &p.ri, stdout) here so you
can see what is changing for each instruction. It looks a bit ugly
calling them mismatches though ;-)
Anyway:
Tested-by: Alex Bennée <alex.bennee@linaro.org>
Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
--
Alex Bennée
^ permalink raw reply [flat|nested] 36+ messages in thread