* [PATCH v3 01/25] Use bool for tracing variables
2020-05-22 2:34 [PATCH v3 00/25] risu cleanups and improvements Richard Henderson
@ 2020-05-22 2:34 ` Richard Henderson
2020-05-22 2:34 ` [PATCH v3 02/25] Unify master_fd and apprentice_fd to comm_fd Richard Henderson
` (26 subsequent siblings)
27 siblings, 0 replies; 49+ messages in thread
From: Richard Henderson @ 2020-05-22 2:34 UTC (permalink / raw)
To: qemu-devel; +Cc: Peter Maydell, alex.bennee
Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
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] 49+ messages in thread* [PATCH v3 02/25] Unify master_fd and apprentice_fd to comm_fd
2020-05-22 2:34 [PATCH v3 00/25] risu cleanups and improvements Richard Henderson
2020-05-22 2:34 ` [PATCH v3 01/25] Use bool for tracing variables Richard Henderson
@ 2020-05-22 2:34 ` Richard Henderson
2020-05-22 2:34 ` [PATCH v3 03/25] Hoist trace file and socket opening Richard Henderson
` (25 subsequent siblings)
27 siblings, 0 replies; 49+ messages in thread
From: Richard Henderson @ 2020-05-22 2:34 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: Alex Bennée <alex.bennee@linaro.org>
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] 49+ messages in thread* [PATCH v3 03/25] Hoist trace file and socket opening
2020-05-22 2:34 [PATCH v3 00/25] risu cleanups and improvements Richard Henderson
2020-05-22 2:34 ` [PATCH v3 01/25] Use bool for tracing variables Richard Henderson
2020-05-22 2:34 ` [PATCH v3 02/25] Unify master_fd and apprentice_fd to comm_fd Richard Henderson
@ 2020-05-22 2:34 ` Richard Henderson
2020-05-22 2:34 ` [PATCH v3 04/25] Adjust tracefile open for write Richard Henderson
` (24 subsequent siblings)
27 siblings, 0 replies; 49+ messages in thread
From: Richard Henderson @ 2020-05-22 2:34 UTC (permalink / raw)
To: qemu-devel; +Cc: Peter Maydell, alex.bennee
We will want to share this code with --dump.
Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
v3: Hoist socket connecting as well as trace file opening.
---
risu.c | 49 +++++++++++++++++++++++--------------------------
1 file changed, 23 insertions(+), 26 deletions(-)
diff --git a/risu.c b/risu.c
index 059348f..2f6a677 100644
--- a/risu.c
+++ b/risu.c
@@ -363,6 +363,29 @@ 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
+ }
+ } else {
+ if (ismaster) {
+ fprintf(stderr, "master port %d\n", port);
+ comm_fd = master_connect(port);
+ } else {
+ fprintf(stderr, "apprentice host %s port %d\n", hostname, port);
+ comm_fd = apprentice_connect(hostname, port);
+ }
+ }
+
imgfile = argv[optind];
if (!imgfile) {
fprintf(stderr, "Error: must specify image file name\n\n");
@@ -373,34 +396,8 @@ 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 {
- 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 {
- fprintf(stderr, "apprentice host %s port %d\n", hostname, port);
- comm_fd = apprentice_connect(hostname, port);
- }
return apprentice();
}
}
--
2.20.1
^ permalink raw reply related [flat|nested] 49+ messages in thread* [PATCH v3 04/25] Adjust tracefile open for write
2020-05-22 2:34 [PATCH v3 00/25] risu cleanups and improvements Richard Henderson
` (2 preceding siblings ...)
2020-05-22 2:34 ` [PATCH v3 03/25] Hoist trace file and socket opening Richard Henderson
@ 2020-05-22 2:34 ` Richard Henderson
2020-05-22 2:34 ` [PATCH v3 05/25] Use EXIT_FAILURE, EXIT_SUCCESS Richard Henderson
` (23 subsequent siblings)
27 siblings, 0 replies; 49+ messages in thread
From: Richard Henderson @ 2020-05-22 2:34 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: Alex Bennée <alex.bennee@linaro.org>
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 2f6a677..819b786 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] 49+ messages in thread* [PATCH v3 05/25] Use EXIT_FAILURE, EXIT_SUCCESS
2020-05-22 2:34 [PATCH v3 00/25] risu cleanups and improvements Richard Henderson
` (3 preceding siblings ...)
2020-05-22 2:34 ` [PATCH v3 04/25] Adjust tracefile open for write Richard Henderson
@ 2020-05-22 2:34 ` Richard Henderson
2020-05-22 2:34 ` [PATCH v3 06/25] Make some risu.c symbols static Richard Henderson
` (22 subsequent siblings)
27 siblings, 0 replies; 49+ messages in thread
From: Richard Henderson @ 2020-05-22 2:34 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: Alex Bennée <alex.bennee@linaro.org>
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 819b786..26dc116 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);
@@ -390,7 +390,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] 49+ messages in thread* [PATCH v3 06/25] Make some risu.c symbols static
2020-05-22 2:34 [PATCH v3 00/25] risu cleanups and improvements Richard Henderson
` (4 preceding siblings ...)
2020-05-22 2:34 ` [PATCH v3 05/25] Use EXIT_FAILURE, EXIT_SUCCESS Richard Henderson
@ 2020-05-22 2:34 ` Richard Henderson
2020-05-22 2:34 ` [PATCH v3 07/25] Add enum RisuOp Richard Henderson
` (21 subsequent siblings)
27 siblings, 0 replies; 49+ messages in thread
From: Richard Henderson @ 2020-05-22 2:34 UTC (permalink / raw)
To: qemu-devel; +Cc: alex.bennee
These are unused in other translation units.
Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
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 26dc116..ab17c71 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] 49+ messages in thread* [PATCH v3 07/25] Add enum RisuOp
2020-05-22 2:34 [PATCH v3 00/25] risu cleanups and improvements Richard Henderson
` (5 preceding siblings ...)
2020-05-22 2:34 ` [PATCH v3 06/25] Make some risu.c symbols static Richard Henderson
@ 2020-05-22 2:34 ` Richard Henderson
2020-05-22 2:34 ` [PATCH v3 08/25] Add enum RisuResult Richard Henderson
` (20 subsequent siblings)
27 siblings, 0 replies; 49+ messages in thread
From: Richard Henderson @ 2020-05-22 2:34 UTC (permalink / raw)
To: qemu-devel; +Cc: alex.bennee
Formalize the set of defines, plus -1, into an enum.
Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
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] 49+ messages in thread* [PATCH v3 08/25] Add enum RisuResult
2020-05-22 2:34 [PATCH v3 00/25] risu cleanups and improvements Richard Henderson
` (6 preceding siblings ...)
2020-05-22 2:34 ` [PATCH v3 07/25] Add enum RisuOp Richard Henderson
@ 2020-05-22 2:34 ` Richard Henderson
2020-05-22 2:34 ` [PATCH v3 09/25] Unify i/o functions and use RisuResult Richard Henderson
` (19 subsequent siblings)
27 siblings, 0 replies; 49+ messages in thread
From: Richard Henderson @ 2020-05-22 2:34 UTC (permalink / raw)
To: qemu-devel; +Cc: 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.
Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
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 ab17c71..7b14f72 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] 49+ messages in thread* [PATCH v3 09/25] Unify i/o functions and use RisuResult
2020-05-22 2:34 [PATCH v3 00/25] risu cleanups and improvements Richard Henderson
` (7 preceding siblings ...)
2020-05-22 2:34 ` [PATCH v3 08/25] Add enum RisuResult Richard Henderson
@ 2020-05-22 2:34 ` Richard Henderson
2020-05-22 2:34 ` [PATCH v3 10/25] Pass non-OK result back through siglongjmp Richard Henderson
` (18 subsequent siblings)
27 siblings, 0 replies; 49+ messages in thread
From: Richard Henderson @ 2020-05-22 2:34 UTC (permalink / raw)
To: qemu-devel; +Cc: 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.
Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
risu.h | 32 +++++++++-------------
comms.c | 8 +++---
reginfo.c | 60 ++++++++++++++++++++--------------------
risu.c | 82 ++++++++++++++++++++++---------------------------------
4 files changed, 81 insertions(+), 101 deletions(-)
diff --git a/risu.h b/risu.h
index e6d07eb..c83b803 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,31 +73,32 @@ 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.
- *
- * We pass the helper function to send_register_info and
- * 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);
+/* Function prototypes for read/write helper functions. */
+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 7b14f72..1917311 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] 49+ messages in thread* [PATCH v3 10/25] Pass non-OK result back through siglongjmp
2020-05-22 2:34 [PATCH v3 00/25] risu cleanups and improvements Richard Henderson
` (8 preceding siblings ...)
2020-05-22 2:34 ` [PATCH v3 09/25] Unify i/o functions and use RisuResult Richard Henderson
@ 2020-05-22 2:34 ` Richard Henderson
2020-05-22 2:34 ` [PATCH v3 11/25] Always write for --master Richard Henderson
` (17 subsequent siblings)
27 siblings, 0 replies; 49+ messages in thread
From: Richard Henderson @ 2020-05-22 2:34 UTC (permalink / raw)
To: qemu-devel; +Cc: 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.
Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
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 1917311..f238117 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] 49+ messages in thread* [PATCH v3 11/25] Always write for --master
2020-05-22 2:34 [PATCH v3 00/25] risu cleanups and improvements Richard Henderson
` (9 preceding siblings ...)
2020-05-22 2:34 ` [PATCH v3 10/25] Pass non-OK result back through siglongjmp Richard Henderson
@ 2020-05-22 2:34 ` Richard Henderson
2020-05-22 2:34 ` [PATCH v3 12/25] Simplify syncing with master Richard Henderson
` (16 subsequent siblings)
27 siblings, 0 replies; 49+ messages in thread
From: Richard Henderson @ 2020-05-22 2:34 UTC (permalink / raw)
To: qemu-devel; +Cc: 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.
Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
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 c83b803..f383b64 100644
--- a/risu.h
+++ b/risu.h
@@ -106,7 +106,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 f238117..199f697 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] 49+ messages in thread* [PATCH v3 12/25] Simplify syncing with master
2020-05-22 2:34 [PATCH v3 00/25] risu cleanups and improvements Richard Henderson
` (10 preceding siblings ...)
2020-05-22 2:34 ` [PATCH v3 11/25] Always write for --master Richard Henderson
@ 2020-05-22 2:34 ` Richard Henderson
2020-05-22 2:34 ` [PATCH v3 13/25] Split RES_MISMATCH for registers and memory Richard Henderson
` (15 subsequent siblings)
27 siblings, 0 replies; 49+ messages in thread
From: Richard Henderson @ 2020-05-22 2:34 UTC (permalink / raw)
To: qemu-devel; +Cc: 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.
Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
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] 49+ messages in thread* [PATCH v3 13/25] Split RES_MISMATCH for registers and memory
2020-05-22 2:34 [PATCH v3 00/25] risu cleanups and improvements Richard Henderson
` (11 preceding siblings ...)
2020-05-22 2:34 ` [PATCH v3 12/25] Simplify syncing with master Richard Henderson
@ 2020-05-22 2:34 ` Richard Henderson
2020-05-22 2:34 ` [PATCH v3 14/25] Merge reginfo.c into risu.c Richard Henderson
` (14 subsequent siblings)
27 siblings, 0 replies; 49+ messages in thread
From: Richard Henderson @ 2020-05-22 2:34 UTC (permalink / raw)
To: qemu-devel; +Cc: 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.
Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
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 f383b64..77d6128 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;
@@ -100,13 +101,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 199f697..d6c2deb 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] 49+ messages in thread* [PATCH v3 14/25] Merge reginfo.c into risu.c
2020-05-22 2:34 [PATCH v3 00/25] risu cleanups and improvements Richard Henderson
` (12 preceding siblings ...)
2020-05-22 2:34 ` [PATCH v3 13/25] Split RES_MISMATCH for registers and memory Richard Henderson
@ 2020-05-22 2:34 ` Richard Henderson
2020-06-23 8:33 ` Alex Bennée
2020-05-22 2:34 ` [PATCH v3 15/25] Rearrange reginfo and memblock buffers Richard Henderson
` (13 subsequent siblings)
27 siblings, 1 reply; 49+ messages in thread
From: Richard Henderson @ 2020-05-22 2:34 UTC (permalink / raw)
To: qemu-devel; +Cc: alex.bennee
The distinction between the two is artificial. Following
patches will rearrange the functions involved to make it
easier for dumping of the trace file.
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
Makefile | 2 +-
risu.h | 28 +---------
reginfo.c | 151 ------------------------------------------------------
risu.c | 129 ++++++++++++++++++++++++++++++++++++++++++++--
4 files changed, 126 insertions(+), 184 deletions(-)
delete mode 100644 reginfo.c
diff --git a/Makefile b/Makefile
index 6ab014a..ad7f879 100644
--- a/Makefile
+++ b/Makefile
@@ -20,7 +20,7 @@ CFLAGS ?= -g
ALL_CFLAGS = -Wall -D_GNU_SOURCE -DARCH=$(ARCH) -U$(ARCH) $(BUILD_INC) $(CFLAGS) $(EXTRA_CFLAGS)
PROG=risu
-SRCS=risu.c comms.c reginfo.c risu_$(ARCH).c risu_reginfo_$(ARCH).c
+SRCS=risu.c comms.c risu_$(ARCH).c risu_reginfo_$(ARCH).c
HDRS=risu.h risu_reginfo_$(ARCH).h
BINS=test_$(ARCH).bin
diff --git a/risu.h b/risu.h
index 77d6128..dd9fda5 100644
--- a/risu.h
+++ b/risu.h
@@ -35,7 +35,6 @@ void process_arch_opt(int opt, const char *arg);
#include REGINFO_HEADER(ARCH)
extern uintptr_t image_start_address;
-extern void *memblock;
/* Ops code under test can request from risu: */
typedef enum {
@@ -83,34 +82,9 @@ void send_response_byte(int sock, int resp);
/* Functions operating on reginfo */
-/* Function prototypes for read/write helper functions. */
-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(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(void *uc);
-
-/*
- * Print a useful report on the status of the last reg comparison
- * done in recv_and_compare_register_info().
- */
-void report_mismatch_reg(void);
-
/* Interface provided by CPU-specific code: */
-/* Move the PC past this faulting insn by adjusting ucontext
- */
+/* Move the PC past this faulting insn by adjusting ucontext. */
void advance_pc(void *uc);
/* Set the parameter register in a ucontext_t to the specified value.
diff --git a/reginfo.c b/reginfo.c
deleted file mode 100644
index a007f16..0000000
--- a/reginfo.c
+++ /dev/null
@@ -1,151 +0,0 @@
-/******************************************************************************
- * Copyright (c) 2017 Linaro Limited
- * All rights reserved. This program and the accompanying materials
- * are made available under the terms of the Eclipse Public License v1.0
- * which accompanies this distribution, and is available at
- * http://www.eclipse.org/legal/epl-v10.html
- *
- * Contributors:
- * Peter Maydell (Linaro) - initial implementation
- *****************************************************************************/
-
-#include <stdio.h>
-#include <string.h>
-#include <stdlib.h>
-#include "risu.h"
-
-static struct reginfo master_ri, apprentice_ri;
-static uint8_t master_memblock[MEMBLOCKLEN];
-
-RisuResult send_register_info(void *uc)
-{
- struct reginfo ri;
- trace_header_t header;
- RisuResult res;
- RisuOp op;
-
- reginfo_init(&ri, uc);
- op = get_risuop(&ri);
-
- /* Write a header with PC/op to keep in sync */
- header.pc = get_pc(&ri);
- header.risu_op = op;
- res = write_buffer(&header, sizeof(header));
- 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;
- }
- break;
- case OP_SETMEMBLOCK:
- memblock = (void *)(uintptr_t)get_reginfo_paramreg(&ri);
- break;
- case OP_GETMEMBLOCK:
- set_ucontext_paramreg(uc,
- get_reginfo_paramreg(&ri) + (uintptr_t)memblock);
- break;
- case OP_COMPAREMEM:
- return write_buffer(memblock, MEMBLOCKLEN);
- default:
- abort();
- }
- return res;
-}
-
-/* Read register info from the socket and compare it with that from the
- * ucontext. Return 0 for match, 1 for end-of-test, 2 for mismatch.
- * NB: called from a signal handler.
- *
- * We don't have any kind of identifying info in the incoming data
- * 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(void *uc)
-{
- RisuResult res;
- trace_header_t header;
- RisuOp op;
-
- reginfo_init(&apprentice_ri, uc);
- op = get_risuop(&apprentice_ri);
-
- res = read_buffer(&header, sizeof(header));
- if (res != RES_OK) {
- return res;
- }
-
- if (header.risu_op != op) {
- /* We are out of sync. Tell master to exit. */
- respond(RES_END);
- return RES_BAD_IO;
- }
-
- /* send OK for the header */
- respond(RES_OK);
-
- 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 = read_buffer(&master_ri, reginfo_size());
- if (res != RES_OK) {
- /* fail */
- } else if (!reginfo_is_eq(&master_ri, &apprentice_ri)) {
- /* register mismatch */
- res = RES_MISMATCH_REG;
- } 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;
- case OP_GETMEMBLOCK:
- set_ucontext_paramreg(uc, get_reginfo_paramreg(&apprentice_ri) +
- (uintptr_t)memblock);
- break;
- case OP_COMPAREMEM:
- res = read_buffer(master_memblock, MEMBLOCKLEN);
- if (res != RES_OK) {
- /* fail */
- } else if (memcmp(memblock, master_memblock, MEMBLOCKLEN) != 0) {
- /* memory mismatch */
- res = RES_MISMATCH_MEM;
- }
- respond(res == RES_OK ? RES_OK : RES_END);
- break;
- default:
- abort();
- }
-
- return res;
-}
-
-/*
- * Print a useful report on the status of the last reg comparison
- * done in recv_and_compare_register_info().
- */
-void report_mismatch_reg(void)
-{
- 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);
-}
diff --git a/risu.c b/risu.c
index d6c2deb..a0e20d5 100644
--- a/risu.c
+++ b/risu.c
@@ -28,7 +28,10 @@
#include "config.h"
#include "risu.h"
-void *memblock;
+static void *memblock;
+static struct reginfo master_ri, apprentice_ri;
+static uint8_t master_memblock[MEMBLOCKLEN];
+
static int comm_fd;
static bool trace;
@@ -48,7 +51,7 @@ static sigjmp_buf jmpbuf;
/* I/O functions */
-RisuResult read_buffer(void *ptr, size_t bytes)
+static RisuResult read_buffer(void *ptr, size_t bytes)
{
size_t res;
@@ -69,7 +72,7 @@ RisuResult read_buffer(void *ptr, size_t bytes)
return res == bytes ? RES_OK : RES_BAD_IO;
}
-RisuResult write_buffer(void *ptr, size_t bytes)
+static RisuResult write_buffer(void *ptr, size_t bytes)
{
size_t res;
@@ -90,13 +93,60 @@ RisuResult write_buffer(void *ptr, size_t bytes)
return res == bytes ? RES_OK : RES_BAD_IO;
}
-void respond(RisuResult r)
+static void respond(RisuResult r)
{
if (!trace) {
send_response_byte(comm_fd, r);
}
}
+static RisuResult send_register_info(void *uc)
+{
+ struct reginfo ri;
+ trace_header_t header;
+ RisuResult res;
+ RisuOp op;
+
+ reginfo_init(&ri, uc);
+ op = get_risuop(&ri);
+
+ /* Write a header with PC/op to keep in sync */
+ header.pc = get_pc(&ri);
+ header.risu_op = op;
+ res = write_buffer(&header, sizeof(header));
+ 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;
+ }
+ break;
+ case OP_SETMEMBLOCK:
+ memblock = (void *)(uintptr_t)get_reginfo_paramreg(&ri);
+ break;
+ case OP_GETMEMBLOCK:
+ set_ucontext_paramreg(uc,
+ get_reginfo_paramreg(&ri) + (uintptr_t)memblock);
+ break;
+ case OP_COMPAREMEM:
+ return write_buffer(memblock, MEMBLOCKLEN);
+ default:
+ abort();
+ }
+ return res;
+}
+
static void master_sigill(int sig, siginfo_t *si, void *uc)
{
RisuResult r;
@@ -110,6 +160,71 @@ static void master_sigill(int sig, siginfo_t *si, void *uc)
}
}
+static RisuResult recv_and_compare_register_info(void *uc)
+{
+ RisuResult res;
+ trace_header_t header;
+ RisuOp op;
+
+ reginfo_init(&apprentice_ri, uc);
+ op = get_risuop(&apprentice_ri);
+
+ res = read_buffer(&header, sizeof(header));
+ if (res != RES_OK) {
+ return res;
+ }
+
+ if (header.risu_op != op) {
+ /* We are out of sync. Tell master to exit. */
+ respond(RES_END);
+ return RES_BAD_IO;
+ }
+
+ /* send OK for the header */
+ respond(RES_OK);
+
+ 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 = read_buffer(&master_ri, reginfo_size());
+ if (res != RES_OK) {
+ /* fail */
+ } else if (!reginfo_is_eq(&master_ri, &apprentice_ri)) {
+ /* register mismatch */
+ res = RES_MISMATCH_REG;
+ } 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;
+ case OP_GETMEMBLOCK:
+ set_ucontext_paramreg(uc, get_reginfo_paramreg(&apprentice_ri) +
+ (uintptr_t)memblock);
+ break;
+ case OP_COMPAREMEM:
+ res = read_buffer(master_memblock, MEMBLOCKLEN);
+ if (res != RES_OK) {
+ /* fail */
+ } else if (memcmp(memblock, master_memblock, MEMBLOCKLEN) != 0) {
+ /* memory mismatch */
+ res = RES_MISMATCH_MEM;
+ }
+ respond(res == RES_OK ? RES_OK : RES_END);
+ break;
+ default:
+ abort();
+ }
+
+ return res;
+}
+
static void apprentice_sigill(int sig, siginfo_t *si, void *uc)
{
RisuResult r;
@@ -226,7 +341,11 @@ static int apprentice(void)
case RES_MISMATCH_REG:
fprintf(stderr, "mismatch reg after %zd checkpoints\n", signal_count);
- report_mismatch_reg();
+ 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 EXIT_FAILURE;
case RES_MISMATCH_MEM:
--
2.20.1
^ permalink raw reply related [flat|nested] 49+ messages in thread* [PATCH v3 15/25] Rearrange reginfo and memblock buffers
2020-05-22 2:34 [PATCH v3 00/25] risu cleanups and improvements Richard Henderson
` (13 preceding siblings ...)
2020-05-22 2:34 ` [PATCH v3 14/25] Merge reginfo.c into risu.c Richard Henderson
@ 2020-05-22 2:34 ` Richard Henderson
2020-06-23 8:47 ` Alex Bennée
2020-05-22 2:34 ` [PATCH v3 16/25] Split out recv_register_info Richard Henderson
` (12 subsequent siblings)
27 siblings, 1 reply; 49+ messages in thread
From: Richard Henderson @ 2020-05-22 2:34 UTC (permalink / raw)
To: qemu-devel; +Cc: alex.bennee
For send_register_info from master_sigill, do not keep a
reginfo buffer on the stack. At the moment, this struct
is quite large for aarch64.
Put the two reginfo buffers into an array, for the benefit
of future dumping. For recv_and_compare_register_info,
index this array with constants, so it's a simple rename.
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
risu.c | 58 ++++++++++++++++++++++++++++++++--------------------------
1 file changed, 32 insertions(+), 26 deletions(-)
diff --git a/risu.c b/risu.c
index a0e20d5..b91ad38 100644
--- a/risu.c
+++ b/risu.c
@@ -28,10 +28,16 @@
#include "config.h"
#include "risu.h"
-static void *memblock;
-static struct reginfo master_ri, apprentice_ri;
-static uint8_t master_memblock[MEMBLOCKLEN];
+enum {
+ MASTER = 0, APPRENTICE = 1
+};
+static struct reginfo ri[2];
+static uint8_t other_memblock[MEMBLOCKLEN];
+static trace_header_t header;
+
+/* Memblock pointer into the execution image. */
+static void *memblock;
static int comm_fd;
static bool trace;
@@ -102,16 +108,15 @@ static void respond(RisuResult r)
static RisuResult send_register_info(void *uc)
{
- struct reginfo ri;
- trace_header_t header;
+ uint64_t paramreg;
RisuResult res;
RisuOp op;
- reginfo_init(&ri, uc);
- op = get_risuop(&ri);
+ reginfo_init(&ri[MASTER], uc);
+ op = get_risuop(&ri[MASTER]);
/* Write a header with PC/op to keep in sync */
- header.pc = get_pc(&ri);
+ header.pc = get_pc(&ri[MASTER]);
header.risu_op = op;
res = write_buffer(&header, sizeof(header));
if (res != RES_OK) {
@@ -126,18 +131,19 @@ static RisuResult send_register_info(void *uc)
* Do a simple register compare on (a) explicit request
* (b) end of test (c) a non-risuop UNDEF
*/
- res = write_buffer(&ri, reginfo_size());
+ res = write_buffer(&ri[MASTER], reginfo_size());
/* For OP_TEST_END, force exit. */
if (res == RES_OK && op == OP_TESTEND) {
res = RES_END;
}
break;
case OP_SETMEMBLOCK:
- memblock = (void *)(uintptr_t)get_reginfo_paramreg(&ri);
+ paramreg = get_reginfo_paramreg(&ri[MASTER]);
+ memblock = (void *)(uintptr_t)paramreg;
break;
case OP_GETMEMBLOCK:
- set_ucontext_paramreg(uc,
- get_reginfo_paramreg(&ri) + (uintptr_t)memblock);
+ paramreg = get_reginfo_paramreg(&ri[MASTER]);
+ set_ucontext_paramreg(uc, paramreg + (uintptr_t)memblock);
break;
case OP_COMPAREMEM:
return write_buffer(memblock, MEMBLOCKLEN);
@@ -162,12 +168,12 @@ static void master_sigill(int sig, siginfo_t *si, void *uc)
static RisuResult recv_and_compare_register_info(void *uc)
{
+ uint64_t paramreg;
RisuResult res;
- trace_header_t header;
RisuOp op;
- reginfo_init(&apprentice_ri, uc);
- op = get_risuop(&apprentice_ri);
+ reginfo_init(&ri[APPRENTICE], uc);
+ op = get_risuop(&ri[APPRENTICE]);
res = read_buffer(&header, sizeof(header));
if (res != RES_OK) {
@@ -190,10 +196,10 @@ static 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(&master_ri, reginfo_size());
+ res = read_buffer(&ri[MASTER], reginfo_size());
if (res != RES_OK) {
/* fail */
- } else if (!reginfo_is_eq(&master_ri, &apprentice_ri)) {
+ } else if (!reginfo_is_eq(&ri[MASTER], &ri[APPRENTICE])) {
/* register mismatch */
res = RES_MISMATCH_REG;
} else if (op == OP_TESTEND) {
@@ -202,17 +208,18 @@ static RisuResult recv_and_compare_register_info(void *uc)
respond(res == RES_OK ? RES_OK : RES_END);
break;
case OP_SETMEMBLOCK:
- memblock = (void *)(uintptr_t)get_reginfo_paramreg(&apprentice_ri);
+ paramreg = get_reginfo_paramreg(&ri[APPRENTICE]);
+ memblock = (void *)(uintptr_t)paramreg;
break;
case OP_GETMEMBLOCK:
- set_ucontext_paramreg(uc, get_reginfo_paramreg(&apprentice_ri) +
- (uintptr_t)memblock);
+ paramreg = get_reginfo_paramreg(&ri[APPRENTICE]);
+ set_ucontext_paramreg(uc, paramreg + (uintptr_t)memblock);
break;
case OP_COMPAREMEM:
- res = read_buffer(master_memblock, MEMBLOCKLEN);
+ res = read_buffer(other_memblock, MEMBLOCKLEN);
if (res != RES_OK) {
/* fail */
- } else if (memcmp(memblock, master_memblock, MEMBLOCKLEN) != 0) {
+ } else if (memcmp(memblock, other_memblock, MEMBLOCKLEN) != 0) {
/* memory mismatch */
res = RES_MISMATCH_MEM;
}
@@ -221,7 +228,6 @@ static RisuResult recv_and_compare_register_info(void *uc)
default:
abort();
}
-
return res;
}
@@ -342,10 +348,10 @@ static int apprentice(void)
case RES_MISMATCH_REG:
fprintf(stderr, "mismatch reg after %zd checkpoints\n", signal_count);
fprintf(stderr, "master reginfo:\n");
- reginfo_dump(&master_ri, stderr);
+ reginfo_dump(&ri[MASTER], stderr);
fprintf(stderr, "apprentice reginfo:\n");
- reginfo_dump(&apprentice_ri, stderr);
- reginfo_dump_mismatch(&master_ri, &apprentice_ri, stderr);
+ reginfo_dump(&ri[APPRENTICE], stderr);
+ reginfo_dump_mismatch(&ri[MASTER], &ri[APPRENTICE], stderr);
return EXIT_FAILURE;
case RES_MISMATCH_MEM:
--
2.20.1
^ permalink raw reply related [flat|nested] 49+ messages in thread* Re: [PATCH v3 15/25] Rearrange reginfo and memblock buffers
2020-05-22 2:34 ` [PATCH v3 15/25] Rearrange reginfo and memblock buffers Richard Henderson
@ 2020-06-23 8:47 ` Alex Bennée
0 siblings, 0 replies; 49+ messages in thread
From: Alex Bennée @ 2020-06-23 8:47 UTC (permalink / raw)
To: Richard Henderson; +Cc: qemu-devel
Richard Henderson <richard.henderson@linaro.org> writes:
> For send_register_info from master_sigill, do not keep a
> reginfo buffer on the stack. At the moment, this struct
> is quite large for aarch64.
>
> Put the two reginfo buffers into an array, for the benefit
> of future dumping. For recv_and_compare_register_info,
> index this array with constants, so it's a simple rename.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
<snip>
> @@ -221,7 +228,6 @@ static RisuResult recv_and_compare_register_info(void *uc)
> default:
> abort();
> }
> -
> return res;
> }
nit: stray whitespace change
Otherwise:
Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
--
Alex Bennée
^ permalink raw reply [flat|nested] 49+ messages in thread
* [PATCH v3 16/25] Split out recv_register_info
2020-05-22 2:34 [PATCH v3 00/25] risu cleanups and improvements Richard Henderson
` (14 preceding siblings ...)
2020-05-22 2:34 ` [PATCH v3 15/25] Rearrange reginfo and memblock buffers Richard Henderson
@ 2020-05-22 2:34 ` Richard Henderson
2020-06-23 10:38 ` Alex Bennée
2020-05-22 2:34 ` [PATCH v3 17/25] Add magic and size to the trace header Richard Henderson
` (11 subsequent siblings)
27 siblings, 1 reply; 49+ messages in thread
From: Richard Henderson @ 2020-05-22 2:34 UTC (permalink / raw)
To: qemu-devel; +Cc: alex.bennee
We will want to share this code when dumping.
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
risu.c | 50 ++++++++++++++++++++++++++++++++++----------------
1 file changed, 34 insertions(+), 16 deletions(-)
diff --git a/risu.c b/risu.c
index b91ad38..80bc3b1 100644
--- a/risu.c
+++ b/risu.c
@@ -166,6 +166,34 @@ static void master_sigill(int sig, siginfo_t *si, void *uc)
}
}
+static RisuResult recv_register_info(struct reginfo *ri)
+{
+ RisuResult res;
+
+ res = read_buffer(&header, sizeof(header));
+ if (res != RES_OK) {
+ return res;
+ }
+
+ /* send OK for the header */
+ respond(RES_OK);
+
+ switch (header.risu_op) {
+ case OP_COMPARE:
+ case OP_TESTEND:
+ case OP_SIGILL:
+ return read_buffer(ri, reginfo_size());
+ case OP_COMPAREMEM:
+ return read_buffer(other_memblock, MEMBLOCKLEN);
+ case OP_SETMEMBLOCK:
+ case OP_GETMEMBLOCK:
+ return RES_OK;
+ default:
+ /* TODO: Create a better error message. */
+ return RES_BAD_IO;
+ }
+}
+
static RisuResult recv_and_compare_register_info(void *uc)
{
uint64_t paramreg;
@@ -173,33 +201,26 @@ static RisuResult recv_and_compare_register_info(void *uc)
RisuOp op;
reginfo_init(&ri[APPRENTICE], uc);
- op = get_risuop(&ri[APPRENTICE]);
- res = read_buffer(&header, sizeof(header));
+ res = recv_register_info(&ri[MASTER]);
if (res != RES_OK) {
+ /* I/O error. Tell master to exit. */
+ respond(RES_END);
return res;
}
+ op = get_risuop(&ri[APPRENTICE]);
if (header.risu_op != op) {
/* We are out of sync. Tell master to exit. */
respond(RES_END);
return RES_BAD_IO;
}
- /* send OK for the header */
- respond(RES_OK);
-
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 = read_buffer(&ri[MASTER], reginfo_size());
- if (res != RES_OK) {
- /* fail */
- } else if (!reginfo_is_eq(&ri[MASTER], &ri[APPRENTICE])) {
+ if (!reginfo_is_eq(&ri[MASTER], &ri[APPRENTICE])) {
/* register mismatch */
res = RES_MISMATCH_REG;
} else if (op == OP_TESTEND) {
@@ -216,10 +237,7 @@ static RisuResult recv_and_compare_register_info(void *uc)
set_ucontext_paramreg(uc, paramreg + (uintptr_t)memblock);
break;
case OP_COMPAREMEM:
- res = read_buffer(other_memblock, MEMBLOCKLEN);
- if (res != RES_OK) {
- /* fail */
- } else if (memcmp(memblock, other_memblock, MEMBLOCKLEN) != 0) {
+ if (memcmp(memblock, other_memblock, MEMBLOCKLEN) != 0) {
/* memory mismatch */
res = RES_MISMATCH_MEM;
}
--
2.20.1
^ permalink raw reply related [flat|nested] 49+ messages in thread* [PATCH v3 17/25] Add magic and size to the trace header
2020-05-22 2:34 [PATCH v3 00/25] risu cleanups and improvements Richard Henderson
` (15 preceding siblings ...)
2020-05-22 2:34 ` [PATCH v3 16/25] Split out recv_register_info Richard Henderson
@ 2020-05-22 2:34 ` Richard Henderson
2020-06-23 14:52 ` Alex Bennée
2020-05-22 2:34 ` [PATCH v3 18/25] Compute reginfo_size based on the reginfo Richard Henderson
` (10 subsequent siblings)
27 siblings, 1 reply; 49+ messages in thread
From: Richard Henderson @ 2020-05-22 2:34 UTC (permalink / raw)
To: qemu-devel; +Cc: 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 | 10 +++-
risu.c | 162 ++++++++++++++++++++++++++++++++++++++++++++-------------
2 files changed, 136 insertions(+), 36 deletions(-)
diff --git a/risu.h b/risu.h
index dd9fda5..bfcf0af 100644
--- a/risu.h
+++ b/risu.h
@@ -55,7 +55,11 @@ typedef enum {
RES_END,
RES_MISMATCH_REG,
RES_MISMATCH_MEM,
+ RES_MISMATCH_OP,
RES_BAD_IO,
+ RES_BAD_MAGIC,
+ RES_BAD_SIZE,
+ RES_BAD_OP,
} RisuResult;
/* The memory block should be this long */
@@ -69,10 +73,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);
diff --git a/risu.c b/risu.c
index 80bc3b1..a248db1 100644
--- a/risu.c
+++ b/risu.c
@@ -111,32 +111,54 @@ static RisuResult send_register_info(void *uc)
uint64_t paramreg;
RisuResult res;
RisuOp op;
+ void *extra;
reginfo_init(&ri[MASTER], uc);
op = get_risuop(&ri[MASTER]);
/* Write a header with PC/op to keep in sync */
+ header.magic = RISU_MAGIC;
header.pc = get_pc(&ri[MASTER]);
header.risu_op = op;
+
+ switch (op) {
+ case OP_TESTEND:
+ case OP_COMPARE:
+ case OP_SIGILL:
+ header.size = reginfo_size();
+ extra = &ri[MASTER];
+ break;
+ case OP_COMPAREMEM:
+ header.size = MEMBLOCKLEN;
+ extra = memblock;
+ break;
+ case OP_SETMEMBLOCK:
+ case OP_GETMEMBLOCK:
+ header.size = 0;
+ extra = NULL;
+ 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[MASTER], 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:
paramreg = get_reginfo_paramreg(&ri[MASTER]);
memblock = (void *)(uintptr_t)paramreg;
@@ -145,12 +167,10 @@ static RisuResult send_register_info(void *uc)
paramreg = get_reginfo_paramreg(&ri[MASTER]);
set_ucontext_paramreg(uc, paramreg + (uintptr_t)memblock);
break;
- case OP_COMPAREMEM:
- return write_buffer(memblock, MEMBLOCKLEN);
default:
abort();
}
- return res;
+ return RES_OK;
}
static void master_sigill(int sig, siginfo_t *si, void *uc)
@@ -175,22 +195,35 @@ static RisuResult recv_register_info(struct reginfo *ri)
return res;
}
- /* send OK for the header */
- respond(RES_OK);
+ if (header.magic != RISU_MAGIC) {
+ /* If the magic number is wrong, we can't trust the rest. */
+ return RES_BAD_MAGIC;
+ }
switch (header.risu_op) {
case OP_COMPARE:
case OP_TESTEND:
case OP_SIGILL:
- return read_buffer(ri, reginfo_size());
+ /* If we can't store the data, report invalid size. */
+ if (header.size > sizeof(*ri)) {
+ return RES_BAD_SIZE;
+ }
+ respond(RES_OK);
+ return read_buffer(ri, header.size);
+
case OP_COMPAREMEM:
+ if (header.size != MEMBLOCKLEN) {
+ return RES_BAD_SIZE;
+ }
+ respond(RES_OK);
return read_buffer(other_memblock, MEMBLOCKLEN);
+
case OP_SETMEMBLOCK:
case OP_GETMEMBLOCK:
- return RES_OK;
+ return header.size == 0 ? RES_OK : RES_BAD_SIZE;
+
default:
- /* TODO: Create a better error message. */
- return RES_BAD_IO;
+ return RES_BAD_OP;
}
}
@@ -204,48 +237,71 @@ static RisuResult recv_and_compare_register_info(void *uc)
res = recv_register_info(&ri[MASTER]);
if (res != RES_OK) {
- /* I/O error. Tell master to exit. */
- respond(RES_END);
- return res;
+ goto done;
}
op = get_risuop(&ri[APPRENTICE]);
- if (header.risu_op != op) {
- /* We are out of sync. Tell master to exit. */
- respond(RES_END);
- return RES_BAD_IO;
- }
switch (op) {
case OP_COMPARE:
case OP_TESTEND:
case OP_SIGILL:
- if (!reginfo_is_eq(&ri[MASTER], &ri[APPRENTICE])) {
+ /*
+ * If we have nothing to compare against, report an op mismatch.
+ * Otherwise allow the compare to continue, and assume that
+ * something in the reginfo will be different.
+ */
+ if (header.risu_op != OP_COMPARE &&
+ header.risu_op != OP_TESTEND &&
+ header.risu_op != OP_SIGILL) {
+ res = RES_MISMATCH_OP;
+ } else if (!reginfo_is_eq(&ri[MASTER], &ri[APPRENTICE])) {
/* register mismatch */
res = RES_MISMATCH_REG;
+ } else if (op != header.risu_op) {
+ /* The reginfo matched. We should have matched op. */
+ res = RES_MISMATCH_OP;
} else if (op == OP_TESTEND) {
res = RES_END;
}
- respond(res == RES_OK ? RES_OK : RES_END);
break;
+
case OP_SETMEMBLOCK:
+ if (op != header.risu_op) {
+ res = RES_MISMATCH_OP;
+ break;
+ }
paramreg = get_reginfo_paramreg(&ri[APPRENTICE]);
memblock = (void *)(uintptr_t)paramreg;
break;
+
case OP_GETMEMBLOCK:
+ if (op != header.risu_op) {
+ res = RES_MISMATCH_OP;
+ break;
+ }
paramreg = get_reginfo_paramreg(&ri[APPRENTICE]);
set_ucontext_paramreg(uc, paramreg + (uintptr_t)memblock);
break;
+
case OP_COMPAREMEM:
+ if (op != header.risu_op) {
+ res = RES_MISMATCH_OP;
+ break;
+ }
if (memcmp(memblock, other_memblock, MEMBLOCKLEN) != 0) {
/* memory mismatch */
res = RES_MISMATCH_MEM;
}
- respond(res == RES_OK ? RES_OK : RES_END);
break;
+
default:
abort();
}
+
+ done:
+ /* On error, tell master to exit. */
+ respond(res == RES_OK ? RES_OK : RES_END);
return res;
}
@@ -346,6 +402,25 @@ static int master(void)
}
}
+static const char *op_name(RisuOp op)
+{
+ switch (op) {
+ case OP_SIGILL:
+ return "SIGILL";
+ case OP_COMPARE:
+ return "COMPARE";
+ case OP_TESTEND:
+ return "TESTEND";
+ case OP_SETMEMBLOCK:
+ return "SETMEMBLOCK";
+ case OP_GETMEMBLOCK:
+ return "GETMEMBLOCK";
+ case OP_COMPAREMEM:
+ return "COMPAREMEM";
+ }
+ abort();
+}
+
static int apprentice(void)
{
RisuResult res = sigsetjmp(jmpbuf, 1);
@@ -364,7 +439,7 @@ static int apprentice(void)
return EXIT_SUCCESS;
case RES_MISMATCH_REG:
- fprintf(stderr, "mismatch reg after %zd checkpoints\n", signal_count);
+ fprintf(stderr, "Mismatch reg after %zd checkpoints\n", signal_count);
fprintf(stderr, "master reginfo:\n");
reginfo_dump(&ri[MASTER], stderr);
fprintf(stderr, "apprentice reginfo:\n");
@@ -373,15 +448,32 @@ static int apprentice(void)
return EXIT_FAILURE;
case RES_MISMATCH_MEM:
- fprintf(stderr, "mismatch mem after %zd checkpoints\n", signal_count);
+ fprintf(stderr, "Mismatch mem after %zd checkpoints\n", signal_count);
+ return EXIT_FAILURE;
+
+ case RES_MISMATCH_OP:
+ /* Out of sync, but both opcodes are known valid. */
+ fprintf(stderr, "Mismatch header after %zd checkpoints\n"
+ "mismatch detail (master : apprentice):\n"
+ " opcode: %s vs %s\n",
+ signal_count, op_name(header.risu_op),
+ op_name(get_risuop(&ri[APPRENTICE])));
return EXIT_FAILURE;
case RES_BAD_IO:
- fprintf(stderr, "i/o error after %zd checkpoints\n", signal_count);
+ fprintf(stderr, "I/O error\n");
+ return EXIT_FAILURE;
+ case RES_BAD_MAGIC:
+ fprintf(stderr, "Unexpected magic number: %#08x\n", header.magic);
+ return EXIT_FAILURE;
+ case RES_BAD_SIZE:
+ fprintf(stderr, "Unexpected payload size: %u\n", header.size);
+ return EXIT_FAILURE;
+ case RES_BAD_OP:
+ fprintf(stderr, "Unexpected opcode: %d\n", header.risu_op);
return EXIT_FAILURE;
-
default:
- fprintf(stderr, "unexpected result %d\n", res);
+ fprintf(stderr, "Unexpected result %d\n", res);
return EXIT_FAILURE;
}
}
--
2.20.1
^ permalink raw reply related [flat|nested] 49+ messages in thread* Re: [PATCH v3 17/25] Add magic and size to the trace header
2020-05-22 2:34 ` [PATCH v3 17/25] Add magic and size to the trace header Richard Henderson
@ 2020-06-23 14:52 ` Alex Bennée
0 siblings, 0 replies; 49+ messages in thread
From: Alex Bennée @ 2020-06-23 14:52 UTC (permalink / raw)
To: Richard Henderson; +Cc: qemu-devel
Richard Henderson <richard.henderson@linaro.org> writes:
> 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 | 10 +++-
> risu.c | 162 ++++++++++++++++++++++++++++++++++++++++++++-------------
> 2 files changed, 136 insertions(+), 36 deletions(-)
>
> diff --git a/risu.h b/risu.h
> index dd9fda5..bfcf0af 100644
> --- a/risu.h
> +++ b/risu.h
> @@ -55,7 +55,11 @@ typedef enum {
> RES_END,
> RES_MISMATCH_REG,
> RES_MISMATCH_MEM,
> + RES_MISMATCH_OP,
> RES_BAD_IO,
> + RES_BAD_MAGIC,
> + RES_BAD_SIZE,
> + RES_BAD_OP,
> } RisuResult;
>
> /* The memory block should be this long */
> @@ -69,10 +73,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')
> +
I guess a fixed constant magic value should compress well.
Anyway:
Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
--
Alex Bennée
^ permalink raw reply [flat|nested] 49+ messages in thread
* [PATCH v3 18/25] Compute reginfo_size based on the reginfo
2020-05-22 2:34 [PATCH v3 00/25] risu cleanups and improvements Richard Henderson
` (16 preceding siblings ...)
2020-05-22 2:34 ` [PATCH v3 17/25] Add magic and size to the trace header Richard Henderson
@ 2020-05-22 2:34 ` Richard Henderson
2020-06-23 14:54 ` Alex Bennée
2020-05-22 2:34 ` [PATCH v3 19/25] aarch64: Reorg sve reginfo to save space Richard Henderson
` (9 subsequent siblings)
27 siblings, 1 reply; 49+ messages in thread
From: Richard Henderson @ 2020-05-22 2:34 UTC (permalink / raw)
To: qemu-devel; +Cc: 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 +-
risu.c | 9 +++++++--
risu_reginfo_aarch64.c | 4 ++--
risu_reginfo_arm.c | 4 ++--
risu_reginfo_i386.c | 4 ++--
risu_reginfo_m68k.c | 4 ++--
risu_reginfo_ppc64.c | 4 ++--
7 files changed, 18 insertions(+), 13 deletions(-)
diff --git a/risu.h b/risu.h
index bfcf0af..3cad3d5 100644
--- a/risu.h
+++ b/risu.h
@@ -126,6 +126,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/risu.c b/risu.c
index a248db1..a70b778 100644
--- a/risu.c
+++ b/risu.c
@@ -125,7 +125,7 @@ static RisuResult send_register_info(void *uc)
case OP_TESTEND:
case OP_COMPARE:
case OP_SIGILL:
- header.size = reginfo_size();
+ header.size = reginfo_size(&ri[MASTER]);
extra = &ri[MASTER];
break;
case OP_COMPAREMEM:
@@ -209,7 +209,12 @@ static RisuResult recv_register_info(struct reginfo *ri)
return RES_BAD_SIZE;
}
respond(RES_OK);
- return read_buffer(ri, header.size);
+ res = read_buffer(ri, header.size);
+ if (res == RES_OK && header.size != reginfo_size(ri)) {
+ /* The payload size is not self-consistent with the data. */
+ return RES_BAD_SIZE;
+ }
+ return res;
case OP_COMPAREMEM:
if (header.size != MEMBLOCKLEN) {
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..47c52e8 100644
--- a/risu_reginfo_arm.c
+++ b/risu_reginfo_arm.c
@@ -36,9 +36,9 @@ 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);
+ return sizeof(*ri);
}
static void reginfo_init_vfp(struct reginfo *ri, ucontext_t *uc)
diff --git a/risu_reginfo_i386.c b/risu_reginfo_i386.c
index 60fc239..50505ab 100644
--- a/risu_reginfo_i386.c
+++ b/risu_reginfo_i386.c
@@ -74,9 +74,9 @@ void process_arch_opt(int opt, const char *arg)
}
}
-const int reginfo_size(void)
+int reginfo_size(struct reginfo *ri)
{
- return sizeof(struct reginfo);
+ return sizeof(*ri);
}
static void *xsave_feature_buf(struct _xstate *xs, int feature)
diff --git a/risu_reginfo_m68k.c b/risu_reginfo_m68k.c
index 32b28c8..4eb30cd 100644
--- a/risu_reginfo_m68k.c
+++ b/risu_reginfo_m68k.c
@@ -23,9 +23,9 @@ 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);
+ return sizeof(*ri);
}
/* reginfo_init: initialize with a ucontext */
diff --git a/risu_reginfo_ppc64.c b/risu_reginfo_ppc64.c
index 071c951..39e8f1c 100644
--- a/risu_reginfo_ppc64.c
+++ b/risu_reginfo_ppc64.c
@@ -32,9 +32,9 @@ 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);
+ return sizeof(*ri);
}
/* reginfo_init: initialize with a ucontext */
--
2.20.1
^ permalink raw reply related [flat|nested] 49+ messages in thread* [PATCH v3 19/25] aarch64: Reorg sve reginfo to save space
2020-05-22 2:34 [PATCH v3 00/25] risu cleanups and improvements Richard Henderson
` (17 preceding siblings ...)
2020-05-22 2:34 ` [PATCH v3 18/25] Compute reginfo_size based on the reginfo Richard Henderson
@ 2020-05-22 2:34 ` Richard Henderson
2020-06-23 16:32 ` Alex Bennée
2020-05-22 2:34 ` [PATCH v3 20/25] aarch64: Use arch_init to configure sve Richard Henderson
` (8 subsequent siblings)
27 siblings, 1 reply; 49+ messages in thread
From: Richard Henderson @ 2020-05-22 2:34 UTC (permalink / raw)
To: qemu-devel; +Cc: 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] 49+ messages in thread* [PATCH v3 20/25] aarch64: Use arch_init to configure sve
2020-05-22 2:34 [PATCH v3 00/25] risu cleanups and improvements Richard Henderson
` (18 preceding siblings ...)
2020-05-22 2:34 ` [PATCH v3 19/25] aarch64: Reorg sve reginfo to save space Richard Henderson
@ 2020-05-22 2:34 ` Richard Henderson
2020-06-23 16:54 ` Alex Bennée
2020-05-22 2:34 ` [PATCH v3 21/25] ppc64: Use uint64_t to represent double Richard Henderson
` (7 subsequent siblings)
27 siblings, 1 reply; 49+ messages in thread
From: Richard Henderson @ 2020-05-22 2:34 UTC (permalink / raw)
To: qemu-devel; +Cc: 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 | 3 +++
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, 55 insertions(+), 20 deletions(-)
diff --git a/risu.h b/risu.h
index 3cad3d5..bdb70c1 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 a70b778..1c096a8 100644
--- a/risu.c
+++ b/risu.c
@@ -617,6 +617,9 @@ int main(int argc, char **argv)
load_image(imgfile);
+ /* E.g. select requested SVE vector length. */
+ arch_init();
+
if (ismaster) {
return master();
} else {
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 47c52e8..202120b 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(*ri);
diff --git a/risu_reginfo_i386.c b/risu_reginfo_i386.c
index 50505ab..e9730be 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(*ri);
diff --git a/risu_reginfo_m68k.c b/risu_reginfo_m68k.c
index 4eb30cd..4c25e77 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(*ri);
diff --git a/risu_reginfo_ppc64.c b/risu_reginfo_ppc64.c
index 39e8f1c..c80e387 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(*ri);
--
2.20.1
^ permalink raw reply related [flat|nested] 49+ messages in thread* [PATCH v3 21/25] ppc64: Use uint64_t to represent double
2020-05-22 2:34 [PATCH v3 00/25] risu cleanups and improvements Richard Henderson
` (19 preceding siblings ...)
2020-05-22 2:34 ` [PATCH v3 20/25] aarch64: Use arch_init to configure sve Richard Henderson
@ 2020-05-22 2:34 ` Richard Henderson
2020-06-23 16:58 ` Alex Bennée
2020-05-22 2:34 ` [PATCH v3 22/25] Standardize reginfo_dump_mismatch printing Richard Henderson
` (6 subsequent siblings)
27 siblings, 1 reply; 49+ messages in thread
From: Richard Henderson @ 2020-05-22 2:34 UTC (permalink / raw)
To: qemu-devel; +Cc: alex.bennee
We want to do exact bitwise comparisons of the data,
not be held hostage to IEEE comparisons and NaNs.
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
risu_reginfo_ppc64.h | 3 ++-
risu_reginfo_ppc64.c | 29 +++++++++--------------------
2 files changed, 11 insertions(+), 21 deletions(-)
diff --git a/risu_reginfo_ppc64.h b/risu_reginfo_ppc64.h
index 7f2c962..4b1d8bd 100644
--- a/risu_reginfo_ppc64.h
+++ b/risu_reginfo_ppc64.h
@@ -20,7 +20,8 @@ struct reginfo {
uint64_t nip;
uint64_t prev_addr;
gregset_t gregs;
- fpregset_t fpregs;
+ uint64_t fpregs[32];
+ uint64_t fpscr;
vrregset_t vrregs;
};
diff --git a/risu_reginfo_ppc64.c b/risu_reginfo_ppc64.c
index c80e387..9899b36 100644
--- a/risu_reginfo_ppc64.c
+++ b/risu_reginfo_ppc64.c
@@ -45,6 +45,7 @@ int reginfo_size(struct reginfo *ri)
void reginfo_init(struct reginfo *ri, ucontext_t *uc)
{
int i;
+
memset(ri, 0, sizeof(*ri));
ri->faulting_insn = *((uint32_t *) uc->uc_mcontext.regs->nip);
@@ -54,16 +55,11 @@ void reginfo_init(struct reginfo *ri, ucontext_t *uc)
ri->gregs[i] = uc->uc_mcontext.gp_regs[i];
}
- for (i = 0; i < NFPREG; i++) {
- ri->fpregs[i] = uc->uc_mcontext.fp_regs[i];
- }
+ memcpy(ri->fpregs, uc->uc_mcontext.fp_regs, 32 * sizeof(double));
+ ri->fpscr = uc->uc_mcontext.fp_regs[32];
- for (i = 0; i < 32; i++) {
- ri->vrregs.vrregs[i][0] = uc->uc_mcontext.v_regs->vrregs[i][0];
- ri->vrregs.vrregs[i][1] = uc->uc_mcontext.v_regs->vrregs[i][1];
- ri->vrregs.vrregs[i][2] = uc->uc_mcontext.v_regs->vrregs[i][2];
- ri->vrregs.vrregs[i][3] = uc->uc_mcontext.v_regs->vrregs[i][3];
- }
+ memcpy(ri->vrregs.vrregs, uc->uc_mcontext.v_regs->vrregs,
+ sizeof(ri->vrregs.vrregs[0]) * 32);
ri->vrregs.vscr = uc->uc_mcontext.v_regs->vscr;
ri->vrregs.vrsave = uc->uc_mcontext.v_regs->vrsave;
}
@@ -91,10 +87,6 @@ int reginfo_is_eq(struct reginfo *m, struct reginfo *a)
}
for (i = 0; i < 32; i++) {
- if (isnan(m->fpregs[i]) && isnan(a->fpregs[i])) {
- continue;
- }
-
if (m->fpregs[i] != a->fpregs[i]) {
return 0;
}
@@ -141,10 +133,10 @@ int reginfo_dump(struct reginfo *ri, FILE * f)
fprintf(f, "\tdscr : %16lx\n\n", ri->gregs[44]);
for (i = 0; i < 16; i++) {
- fprintf(f, "\tf%2d: %.4f\tf%2d: %.4f\n", i, ri->fpregs[i],
+ fprintf(f, "\tf%2d: %016lx\tf%2d: %016lx\n", i, ri->fpregs[i],
i + 16, ri->fpregs[i + 16]);
}
- fprintf(f, "\tfpscr: %f\n\n", ri->fpregs[32]);
+ fprintf(f, "\tfpscr: %016lx\n\n", ri->fpscr);
for (i = 0; i < 32; i++) {
fprintf(f, "vr%02d: %8x, %8x, %8x, %8x\n", i,
@@ -181,13 +173,10 @@ int reginfo_dump_mismatch(struct reginfo *m, struct reginfo *a, FILE *f)
}
for (i = 0; i < 32; i++) {
- if (isnan(m->fpregs[i]) && isnan(a->fpregs[i])) {
- continue;
- }
-
if (m->fpregs[i] != a->fpregs[i]) {
fprintf(f, "Mismatch: Register f%d\n", i);
- fprintf(f, "m: [%f] != a: [%f]\n", m->fpregs[i], a->fpregs[i]);
+ fprintf(f, "m: [%016lx] != a: [%016lx]\n",
+ m->fpregs[i], a->fpregs[i]);
}
}
--
2.20.1
^ permalink raw reply related [flat|nested] 49+ messages in thread* [PATCH v3 22/25] Standardize reginfo_dump_mismatch printing
2020-05-22 2:34 [PATCH v3 00/25] risu cleanups and improvements Richard Henderson
` (20 preceding siblings ...)
2020-05-22 2:34 ` [PATCH v3 21/25] ppc64: Use uint64_t to represent double Richard Henderson
@ 2020-05-22 2:34 ` Richard Henderson
2020-06-23 17:03 ` Alex Bennée
2020-05-22 2:34 ` [PATCH v3 23/25] Add --fulldump and --diffdup options Richard Henderson
` (5 subsequent siblings)
27 siblings, 1 reply; 49+ messages in thread
From: Richard Henderson @ 2020-05-22 2:34 UTC (permalink / raw)
To: qemu-devel; +Cc: alex.bennee
Hoist the "master vs apprentice" label to apprentice(), since
we will want different labels for dumping. Remove all of the
"mismatch" text from reginfo_dump_mismatch -- just print "vs".
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
risu.h | 4 ++--
risu.c | 1 +
risu_reginfo_aarch64.c | 12 +++++-------
risu_reginfo_arm.c | 18 ++++++++---------
risu_reginfo_i386.c | 6 +-----
risu_reginfo_m68k.c | 23 +++++++---------------
risu_reginfo_ppc64.c | 44 ++++++++++++++++++------------------------
7 files changed, 44 insertions(+), 64 deletions(-)
diff --git a/risu.h b/risu.h
index bdb70c1..99f0d8e 100644
--- a/risu.h
+++ b/risu.h
@@ -123,8 +123,8 @@ int reginfo_is_eq(struct reginfo *r1, struct reginfo *r2);
/* print reginfo state to a stream, returns 1 on success, 0 on failure */
int reginfo_dump(struct reginfo *ri, FILE * f);
-/* reginfo_dump_mismatch: print mismatch details to a stream, ret nonzero=ok */
-int reginfo_dump_mismatch(struct reginfo *m, struct reginfo *a, FILE *f);
+/* reginfo_dump_mismatch: print mismatch details to a stream */
+void reginfo_dump_mismatch(struct reginfo *m, struct reginfo *a, FILE *f);
/* return size of reginfo */
int reginfo_size(struct reginfo *ri);
diff --git a/risu.c b/risu.c
index 1c096a8..f613fa9 100644
--- a/risu.c
+++ b/risu.c
@@ -449,6 +449,7 @@ static int apprentice(void)
reginfo_dump(&ri[MASTER], stderr);
fprintf(stderr, "apprentice reginfo:\n");
reginfo_dump(&ri[APPRENTICE], stderr);
+ fprintf(stderr, "mismatch detail (master : apprentice):\n");
reginfo_dump_mismatch(&ri[MASTER], &ri[APPRENTICE], stderr);
return EXIT_FAILURE;
diff --git a/risu_reginfo_aarch64.c b/risu_reginfo_aarch64.c
index fb8e11a..b443745 100644
--- a/risu_reginfo_aarch64.c
+++ b/risu_reginfo_aarch64.c
@@ -316,15 +316,15 @@ int reginfo_dump(struct reginfo *ri, FILE * f)
return !ferror(f);
}
-/* reginfo_dump_mismatch: print mismatch details to a stream, ret nonzero=ok */
-int reginfo_dump_mismatch(struct reginfo *m, struct reginfo *a, FILE * f)
+void reginfo_dump_mismatch(struct reginfo *m, struct reginfo *a, FILE * f)
{
int i;
- fprintf(f, "mismatch detail (master : apprentice):\n");
+
if (m->faulting_insn != a->faulting_insn) {
- fprintf(f, " faulting insn mismatch %08x vs %08x\n",
+ fprintf(f, " faulting insn: %08x vs %08x\n",
m->faulting_insn, a->faulting_insn);
}
+
for (i = 0; i < 31; i++) {
if (m->regs[i] != a->regs[i]) {
fprintf(f, " X%-2d : %016" PRIx64 " vs %016" PRIx64 "\n",
@@ -383,7 +383,7 @@ int reginfo_dump_mismatch(struct reginfo *m, struct reginfo *a, FILE * f)
sve_dump_preg_diff(f, vq, pm, pa);
}
}
- return !ferror(f);
+ return;
}
#endif
@@ -398,6 +398,4 @@ int reginfo_dump_mismatch(struct reginfo *m, struct reginfo *a, FILE * f)
(uint64_t) a->simd.vregs[i]);
}
}
-
- return !ferror(f);
}
diff --git a/risu_reginfo_arm.c b/risu_reginfo_arm.c
index 202120b..ba1035e 100644
--- a/risu_reginfo_arm.c
+++ b/risu_reginfo_arm.c
@@ -183,32 +183,33 @@ int reginfo_dump(struct reginfo *ri, FILE *f)
return !ferror(f);
}
-int reginfo_dump_mismatch(struct reginfo *m, struct reginfo *a, FILE *f)
+void reginfo_dump_mismatch(struct reginfo *m, struct reginfo *a, FILE *f)
{
int i;
- fprintf(f, "mismatch detail (master : apprentice):\n");
if (m->faulting_insn_size != a->faulting_insn_size) {
- fprintf(f, " faulting insn size mismatch %d vs %d\n",
+ fprintf(f, " faulting insn size: %d vs %d\n",
m->faulting_insn_size, a->faulting_insn_size);
} else if (m->faulting_insn != a->faulting_insn) {
if (m->faulting_insn_size == 2) {
- fprintf(f, " faulting insn mismatch %04x vs %04x\n",
+ fprintf(f, " faulting insn: %04x vs %04x\n",
m->faulting_insn, a->faulting_insn);
} else {
- fprintf(f, " faulting insn mismatch %08x vs %08x\n",
+ fprintf(f, " faulting insn: %08x vs %08x\n",
m->faulting_insn, a->faulting_insn);
}
}
+
for (i = 0; i < 16; i++) {
if (m->gpreg[i] != a->gpreg[i]) {
- fprintf(f, " r%d: %08x vs %08x\n", i, m->gpreg[i],
- a->gpreg[i]);
+ fprintf(f, " r%d: %08x vs %08x\n", i, m->gpreg[i], a->gpreg[i]);
}
}
+
if (m->cpsr != a->cpsr) {
fprintf(f, " cpsr: %08x vs %08x\n", m->cpsr, a->cpsr);
}
+
for (i = 0; i < 32; i++) {
if (m->fpregs[i] != a->fpregs[i]) {
fprintf(f, " d%d: %016llx vs %016llx\n", i,
@@ -216,9 +217,8 @@ int reginfo_dump_mismatch(struct reginfo *m, struct reginfo *a, FILE *f)
(unsigned long long) a->fpregs[i]);
}
}
+
if (m->fpscr != a->fpscr) {
fprintf(f, " fpscr: %08x vs %08x\n", m->fpscr, a->fpscr);
}
-
- return !ferror(f);
}
diff --git a/risu_reginfo_i386.c b/risu_reginfo_i386.c
index e9730be..57e4c00 100644
--- a/risu_reginfo_i386.c
+++ b/risu_reginfo_i386.c
@@ -349,14 +349,12 @@ int reginfo_dump(struct reginfo *ri, FILE *f)
return !ferror(f);
}
-int reginfo_dump_mismatch(struct reginfo *m, struct reginfo *a, FILE *f)
+void reginfo_dump_mismatch(struct reginfo *m, struct reginfo *a, FILE *f)
{
int i, j, n, w;
uint64_t features;
char r;
- fprintf(f, "Mismatch (master v apprentice):\n");
-
for (i = 0; i < NGREG; i++) {
if (m->gregs[i] != a->gregs[i]) {
assert(regname[i]);
@@ -399,6 +397,4 @@ int reginfo_dump_mismatch(struct reginfo *m, struct reginfo *a, FILE *f)
i, m->kregs[i], a->kregs[i]);
}
}
-
- return !ferror(f);
}
diff --git a/risu_reginfo_m68k.c b/risu_reginfo_m68k.c
index 4c25e77..29edce9 100644
--- a/risu_reginfo_m68k.c
+++ b/risu_reginfo_m68k.c
@@ -118,13 +118,12 @@ int reginfo_dump(struct reginfo *ri, FILE *f)
return !ferror(f);
}
-int reginfo_dump_mismatch(struct reginfo *m, struct reginfo *a, FILE * f)
+void reginfo_dump_mismatch(struct reginfo *m, struct reginfo *a, FILE *f)
{
int i;
if (m->gregs[R_PS] != a->gregs[R_PS]) {
- fprintf(f, "Mismatch: Register PS\n");
- fprintf(f, "master: [%x] - apprentice: [%x]\n",
+ fprintf(f, " PS: %08x vs %08x\n",
m->gregs[R_PS], a->gregs[R_PS]);
}
@@ -133,22 +132,18 @@ int reginfo_dump_mismatch(struct reginfo *m, struct reginfo *a, FILE * f)
continue;
}
if (m->gregs[i] != a->gregs[i]) {
- fprintf(f, "Mismatch: Register %c%d\n", i < 8 ? 'D' : 'A',
- i % 8);
- fprintf(f, "master: [%x] - apprentice: [%x]\n", m->gregs[i],
- a->gregs[i]);
+ fprintf(f, " %c%d: %08x vs %08x\n",
+ i < 8 ? 'D' : 'A', i % 8, m->gregs[i], a->gregs[i]);
}
}
if (m->fpregs.f_pcr != a->fpregs.f_pcr) {
- fprintf(f, "Mismatch: Register FPCR\n");
- fprintf(f, "m: [%04x] != a: [%04x]\n",
+ fprintf(f, " FPCR: %04x vs %04x\n",
m->fpregs.f_pcr, a->fpregs.f_pcr);
}
if (m->fpregs.f_psr != a->fpregs.f_psr) {
- fprintf(f, "Mismatch: Register FPSR\n");
- fprintf(f, "m: [%08x] != a: [%08x]\n",
+ fprintf(f, " FPSR: %04x vs %04x\n",
m->fpregs.f_psr, a->fpregs.f_psr);
}
@@ -156,14 +151,10 @@ int reginfo_dump_mismatch(struct reginfo *m, struct reginfo *a, FILE * f)
if (m->fpregs.f_fpregs[i][0] != a->fpregs.f_fpregs[i][0] ||
m->fpregs.f_fpregs[i][1] != a->fpregs.f_fpregs[i][1] ||
m->fpregs.f_fpregs[i][2] != a->fpregs.f_fpregs[i][2]) {
- fprintf(f, "Mismatch: Register FP%d\n", i);
- fprintf(f, "m: [%08x %08x %08x] != a: [%08x %08x %08x]\n",
+ fprintf(f, " FP%d: %08x%08x%08x vs %08x%08x%08x\n", i,
m->fpregs.f_fpregs[i][0], m->fpregs.f_fpregs[i][1],
m->fpregs.f_fpregs[i][2], a->fpregs.f_fpregs[i][0],
a->fpregs.f_fpregs[i][1], a->fpregs.f_fpregs[i][2]);
}
}
-
-
- return !ferror(f);
}
diff --git a/risu_reginfo_ppc64.c b/risu_reginfo_ppc64.c
index 9899b36..e96dc48 100644
--- a/risu_reginfo_ppc64.c
+++ b/risu_reginfo_ppc64.c
@@ -27,6 +27,15 @@
const struct option * const arch_long_opts;
const char * const arch_extra_help;
+static const char * const greg_names[NGREG] = {
+ "r0", "r1", "r2", "r3", "r4", "r5", "r6", "r7",
+ "r8", "r9", "r10", "r11", "r12", "r13", "r14", "r15",
+ "r16", "r17", "r18", "r19", "r20", "r21", "r22", "r23",
+ "r24", "r25", "r26", "r27", "r28", "r29", "r30", "r31",
+ [XER] = "xer",
+ [CCR] = "ccr",
+};
+
void process_arch_opt(int opt, const char *arg)
{
abort();
@@ -147,35 +156,21 @@ int reginfo_dump(struct reginfo *ri, FILE * f)
return !ferror(f);
}
-int reginfo_dump_mismatch(struct reginfo *m, struct reginfo *a, FILE *f)
+void reginfo_dump_mismatch(struct reginfo *m, struct reginfo *a, FILE *f)
{
int i;
- for (i = 0; i < 32; i++) {
- if (i == 1 || i == 13) {
- continue;
+
+ for (i = 0; i < NGREG; i++) {
+ if (greg_names[i] != NULL && m->gregs[i] != a->gregs[i]) {
+ fprintf(f, "%6s: %016lx vs %016lx\n",
+ greg_names[i], m->gregs[i], a->gregs[i]);
}
-
- if (m->gregs[i] != a->gregs[i]) {
- fprintf(f, "Mismatch: Register r%d\n", i);
- fprintf(f, "master: [%lx] - apprentice: [%lx]\n",
- m->gregs[i], a->gregs[i]);
- }
- }
-
- if (m->gregs[XER] != a->gregs[XER]) {
- fprintf(f, "Mismatch: XER\n");
- fprintf(f, "m: [%lx] != a: [%lx]\n", m->gregs[XER], a->gregs[XER]);
- }
-
- if (m->gregs[CCR] != a->gregs[CCR]) {
- fprintf(f, "Mismatch: Cond. Register\n");
- fprintf(f, "m: [%lx] != a: [%lx]\n", m->gregs[CCR], a->gregs[CCR]);
}
for (i = 0; i < 32; i++) {
if (m->fpregs[i] != a->fpregs[i]) {
- fprintf(f, "Mismatch: Register f%d\n", i);
- fprintf(f, "m: [%016lx] != a: [%016lx]\n",
+ fprintf(f, "%*s%d: %016lx vs %016lx\n",
+ 6 - (i < 10 ? 1 : 2), "f", i,
m->fpregs[i], a->fpregs[i]);
}
}
@@ -186,13 +181,12 @@ int reginfo_dump_mismatch(struct reginfo *m, struct reginfo *a, FILE *f)
m->vrregs.vrregs[i][2] != a->vrregs.vrregs[i][2] ||
m->vrregs.vrregs[i][3] != a->vrregs.vrregs[i][3]) {
- fprintf(f, "Mismatch: Register vr%d\n", i);
- fprintf(f, "m: [%x, %x, %x, %x] != a: [%x, %x, %x, %x]\n",
+ fprintf(f, "%*s%d: %08x%08x%08x%08x vs %08x%08x%08x%08x\n",
+ 6 - (i < 10 ? 1 : 2), "vr", i,
m->vrregs.vrregs[i][0], m->vrregs.vrregs[i][1],
m->vrregs.vrregs[i][2], m->vrregs.vrregs[i][3],
a->vrregs.vrregs[i][0], a->vrregs.vrregs[i][1],
a->vrregs.vrregs[i][2], a->vrregs.vrregs[i][3]);
}
}
- return !ferror(f);
}
--
2.20.1
^ permalink raw reply related [flat|nested] 49+ messages in thread* [PATCH v3 23/25] Add --fulldump and --diffdup options
2020-05-22 2:34 [PATCH v3 00/25] risu cleanups and improvements Richard Henderson
` (21 preceding siblings ...)
2020-05-22 2:34 ` [PATCH v3 22/25] Standardize reginfo_dump_mismatch printing Richard Henderson
@ 2020-05-22 2:34 ` Richard Henderson
2020-05-22 2:34 ` [PATCH v3 24/25] Remove return value from reginfo_dump Richard Henderson
` (4 subsequent siblings)
27 siblings, 0 replies; 49+ messages in thread
From: Richard Henderson @ 2020-05-22 2:34 UTC (permalink / raw)
To: qemu-devel; +Cc: alex.bennee
These allow the inspection of the trace files.
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
risu.c | 117 +++++++++++++++++++++++++++++++++++++++++++++++++--------
1 file changed, 102 insertions(+), 15 deletions(-)
diff --git a/risu.c b/risu.c
index f613fa9..8d907d9 100644
--- a/risu.c
+++ b/risu.c
@@ -484,23 +484,101 @@ static int apprentice(void)
}
}
-static int ismaster;
+static int dump_trace(bool isfull)
+{
+ RisuResult res;
+ int tick = 0;
+
+ while (1) {
+ struct reginfo *this_ri;
+
+ this_ri = &ri[tick & 1];
+ res = recv_register_info(this_ri);
+
+ switch (res) {
+ case RES_OK:
+ switch (header.risu_op) {
+ case OP_COMPARE:
+ case OP_TESTEND:
+ case OP_SIGILL:
+ printf("%s: (pc %#lx)\n", op_name(header.risu_op),
+ (unsigned long)header.pc);
+
+ if (isfull || tick == 0) {
+ reginfo_dump(this_ri, stdout);
+ } else {
+ struct reginfo *prev_ri = &ri[(tick - 1) & 1];
+
+ if (reginfo_is_eq(prev_ri, this_ri)) {
+ /*
+ * ??? There should never be no change -- at minimum
+ * PC should have advanced. But for completeness...
+ */
+ printf("change detail: none\n");
+ } else {
+ printf("change detail (prev : next):\n");
+ reginfo_dump_mismatch(prev_ri, this_ri, stdout);
+ }
+ }
+ putchar('\n');
+ if (header.risu_op == OP_TESTEND) {
+ return EXIT_SUCCESS;
+ }
+ tick++;
+ break;
+
+ case OP_COMPAREMEM:
+ /* TODO: Dump 8k of data? */
+ /* fall through */
+ default:
+ printf("%s\n", op_name(header.risu_op));
+ break;
+ }
+ break;
+
+ case RES_BAD_IO:
+ fprintf(stderr, "I/O error\n");
+ return EXIT_FAILURE;
+ case RES_BAD_MAGIC:
+ fprintf(stderr, "Unexpected magic number: %#08x\n", header.magic);
+ return EXIT_FAILURE;
+ case RES_BAD_SIZE:
+ fprintf(stderr, "Unexpected payload size: %u\n", header.size);
+ return EXIT_FAILURE;
+ case RES_BAD_OP:
+ fprintf(stderr, "Unexpected opcode: %d\n", header.risu_op);
+ return EXIT_FAILURE;
+ default:
+ fprintf(stderr, "Unexpected recv result %d\n", res);
+ return EXIT_FAILURE;
+ }
+ }
+}
+
+enum {
+ DO_APPRENTICE,
+ DO_MASTER,
+ DO_FULLDUMP,
+ DO_DIFFDUMP,
+};
+
+static int operation = DO_APPRENTICE;
static void usage(void)
{
fprintf(stderr,
- "Usage: risu [--master] [--host <ip>] [--port <port>] <image file>"
- "\n\n");
- fprintf(stderr,
- "Run through the pattern file verifying each instruction\n");
- fprintf(stderr, "between master and apprentice risu processes.\n\n");
- fprintf(stderr, "Options:\n");
- fprintf(stderr, " --master Be the master (server)\n");
- fprintf(stderr, " -t, --trace=FILE Record/playback " TRACE_TYPE " trace file\n");
- fprintf(stderr,
- " -h, --host=HOST Specify master host machine (apprentice only)"
- "\n");
- fprintf(stderr,
+ "Usage: risu [--master|--fulldump|--diffdump]\n"
+ " [--host <ip>] [--port <port>] <image file>\n"
+ "\n"
+ "Run through the pattern file verifying each instruction\n"
+ "between master and apprentice risu processes.\n"
+ "\n"
+ "Options:\n"
+ " --master Be the master (server)\n"
+ " --fulldump Dump each record\n"
+ " --diffdump Dump difference between each record\n"
+ " -t, --trace=FILE Record/playback " TRACE_TYPE " trace file\n"
+ " -h, --host=HOST Specify master host machine\n"
" -p, --port=PORT Specify the port to connect to/listen on "
"(default 9191)\n");
if (arch_extra_help) {
@@ -512,7 +590,9 @@ static struct option * setup_options(char **short_opts)
{
static struct option default_longopts[] = {
{"help", no_argument, 0, '?'},
- {"master", no_argument, &ismaster, 1},
+ {"master", no_argument, &operation, DO_MASTER},
+ {"fulldump", no_argument, &operation, DO_FULLDUMP},
+ {"diffdump", no_argument, &operation, DO_DIFFDUMP},
{"host", required_argument, 0, 'h'},
{"port", required_argument, 0, 'p'},
{"trace", required_argument, 0, 't'},
@@ -520,7 +600,7 @@ static struct option * setup_options(char **short_opts)
};
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);
@@ -551,6 +631,7 @@ int main(int argc, char **argv)
char *trace_fn = NULL;
struct option *longopts;
char *shortopts;
+ bool ismaster;
longopts = setup_options(&shortopts);
@@ -586,6 +667,8 @@ int main(int argc, char **argv)
}
}
+ ismaster = operation == DO_MASTER;
+
if (trace) {
if (strcmp(trace_fn, "-") == 0) {
comm_fd = ismaster ? STDOUT_FILENO : STDIN_FILENO;
@@ -609,6 +692,10 @@ int main(int argc, char **argv)
}
}
+ if (operation == DO_FULLDUMP || operation == DO_DIFFDUMP) {
+ return dump_trace(operation == DO_FULLDUMP);
+ }
+
imgfile = argv[optind];
if (!imgfile) {
fprintf(stderr, "Error: must specify image file name\n\n");
--
2.20.1
^ permalink raw reply related [flat|nested] 49+ messages in thread* [PATCH v3 24/25] Remove return value from reginfo_dump
2020-05-22 2:34 [PATCH v3 00/25] risu cleanups and improvements Richard Henderson
` (22 preceding siblings ...)
2020-05-22 2:34 ` [PATCH v3 23/25] Add --fulldump and --diffdup options Richard Henderson
@ 2020-05-22 2:34 ` Richard Henderson
2020-06-23 17:38 ` Alex Bennée
2020-05-22 2:34 ` [PATCH v3 25/25] ppc64: Clean up reginfo handling Richard Henderson
` (3 subsequent siblings)
27 siblings, 1 reply; 49+ messages in thread
From: Richard Henderson @ 2020-05-22 2:34 UTC (permalink / raw)
To: qemu-devel; +Cc: alex.bennee
No uses actually checked the error indication. Even if we wanted
to check ferror on the stream, we should do that generically rather
than per arch.
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
risu.h | 4 ++--
risu_reginfo_aarch64.c | 8 +++-----
risu_reginfo_arm.c | 6 ++----
risu_reginfo_i386.c | 6 ++----
risu_reginfo_m68k.c | 6 ++----
risu_reginfo_ppc64.c | 6 ++----
6 files changed, 13 insertions(+), 23 deletions(-)
diff --git a/risu.h b/risu.h
index 99f0d8e..6eceb9f 100644
--- a/risu.h
+++ b/risu.h
@@ -120,8 +120,8 @@ void reginfo_init(struct reginfo *ri, ucontext_t *uc);
/* return 1 if structs are equal, 0 otherwise. */
int reginfo_is_eq(struct reginfo *r1, struct reginfo *r2);
-/* print reginfo state to a stream, returns 1 on success, 0 on failure */
-int reginfo_dump(struct reginfo *ri, FILE * f);
+/* print reginfo state to a stream */
+void reginfo_dump(struct reginfo *ri, FILE *f);
/* reginfo_dump_mismatch: print mismatch details to a stream */
void reginfo_dump_mismatch(struct reginfo *m, struct reginfo *a, FILE *f);
diff --git a/risu_reginfo_aarch64.c b/risu_reginfo_aarch64.c
index b443745..64e0e8b 100644
--- a/risu_reginfo_aarch64.c
+++ b/risu_reginfo_aarch64.c
@@ -258,8 +258,8 @@ static void sve_dump_zreg_diff(FILE *f, int vq, const uint64_t *za,
}
#endif
-/* reginfo_dump: print state to a stream, returns nonzero on success */
-int reginfo_dump(struct reginfo *ri, FILE * f)
+/* reginfo_dump: print state to a stream */
+void reginfo_dump(struct reginfo *ri, FILE * f)
{
int i;
fprintf(f, " faulting insn %08x\n", ri->faulting_insn);
@@ -303,7 +303,7 @@ int reginfo_dump(struct reginfo *ri, FILE * f)
sve_dump_preg(f, vq, p);
fprintf(f, "\n");
}
- return !ferror(f);
+ return;
}
#endif
@@ -312,8 +312,6 @@ int reginfo_dump(struct reginfo *ri, FILE * f)
(uint64_t) (ri->simd.vregs[i] >> 64),
(uint64_t) (ri->simd.vregs[i]));
}
-
- return !ferror(f);
}
void reginfo_dump_mismatch(struct reginfo *m, struct reginfo *a, FILE * f)
diff --git a/risu_reginfo_arm.c b/risu_reginfo_arm.c
index ba1035e..09813c4 100644
--- a/risu_reginfo_arm.c
+++ b/risu_reginfo_arm.c
@@ -161,8 +161,8 @@ int reginfo_is_eq(struct reginfo *r1, struct reginfo *r2)
return memcmp(r1, r2, sizeof(*r1)) == 0; /* ok since we memset 0 */
}
-/* reginfo_dump: print the state to a stream, returns nonzero on success */
-int reginfo_dump(struct reginfo *ri, FILE *f)
+/* reginfo_dump: print the state to a stream */
+void reginfo_dump(struct reginfo *ri, FILE *f)
{
int i;
if (ri->faulting_insn_size == 2) {
@@ -179,8 +179,6 @@ int reginfo_dump(struct reginfo *ri, FILE *f)
i, (unsigned long long) ri->fpregs[i]);
}
fprintf(f, " fpscr: %08x\n", ri->fpscr);
-
- return !ferror(f);
}
void reginfo_dump_mismatch(struct reginfo *m, struct reginfo *a, FILE *f)
diff --git a/risu_reginfo_i386.c b/risu_reginfo_i386.c
index 57e4c00..37506fa 100644
--- a/risu_reginfo_i386.c
+++ b/risu_reginfo_i386.c
@@ -310,8 +310,8 @@ static char get_vecletter(uint64_t features)
}
}
-/* reginfo_dump: print state to a stream, returns nonzero on success */
-int reginfo_dump(struct reginfo *ri, FILE *f)
+/* reginfo_dump: print state to a stream */
+void reginfo_dump(struct reginfo *ri, FILE *f)
{
uint64_t features;
int i, j, n, w;
@@ -345,8 +345,6 @@ int reginfo_dump(struct reginfo *ri, FILE *f)
fprintf(f, " k%-5d: %016" PRIx64 "\n", i, ri->kregs[i]);
}
}
-
- return !ferror(f);
}
void reginfo_dump_mismatch(struct reginfo *m, struct reginfo *a, FILE *f)
diff --git a/risu_reginfo_m68k.c b/risu_reginfo_m68k.c
index 29edce9..38d7dd3 100644
--- a/risu_reginfo_m68k.c
+++ b/risu_reginfo_m68k.c
@@ -92,8 +92,8 @@ int reginfo_is_eq(struct reginfo *m, struct reginfo *a)
return 1;
}
-/* reginfo_dump: print state to a stream, returns nonzero on success */
-int reginfo_dump(struct reginfo *ri, FILE *f)
+/* reginfo_dump: print state to a stream */
+void reginfo_dump(struct reginfo *ri, FILE *f)
{
int i;
fprintf(f, " pc \e[1;101;37m0x%08x\e[0m\n", ri->pc);
@@ -114,8 +114,6 @@ int reginfo_dump(struct reginfo *ri, FILE *f)
}
fprintf(f, "\n");
-
- return !ferror(f);
}
void reginfo_dump_mismatch(struct reginfo *m, struct reginfo *a, FILE *f)
diff --git a/risu_reginfo_ppc64.c b/risu_reginfo_ppc64.c
index e96dc48..134a152 100644
--- a/risu_reginfo_ppc64.c
+++ b/risu_reginfo_ppc64.c
@@ -112,8 +112,8 @@ int reginfo_is_eq(struct reginfo *m, struct reginfo *a)
return 1;
}
-/* reginfo_dump: print state to a stream, returns nonzero on success */
-int reginfo_dump(struct reginfo *ri, FILE * f)
+/* reginfo_dump: print state to a stream */
+void reginfo_dump(struct reginfo *ri, FILE * f)
{
int i;
@@ -152,8 +152,6 @@ int reginfo_dump(struct reginfo *ri, FILE * f)
ri->vrregs.vrregs[i][0], ri->vrregs.vrregs[i][1],
ri->vrregs.vrregs[i][2], ri->vrregs.vrregs[i][3]);
}
-
- return !ferror(f);
}
void reginfo_dump_mismatch(struct reginfo *m, struct reginfo *a, FILE *f)
--
2.20.1
^ permalink raw reply related [flat|nested] 49+ messages in thread* [PATCH v3 25/25] ppc64: Clean up reginfo handling
2020-05-22 2:34 [PATCH v3 00/25] risu cleanups and improvements Richard Henderson
` (23 preceding siblings ...)
2020-05-22 2:34 ` [PATCH v3 24/25] Remove return value from reginfo_dump Richard Henderson
@ 2020-05-22 2:34 ` Richard Henderson
2020-05-22 4:10 ` Richard Henderson
2020-06-23 17:45 ` Alex Bennée
2020-06-22 19:17 ` [PATCH v3 00/25] risu cleanups and improvements Peter Maydell
` (2 subsequent siblings)
27 siblings, 2 replies; 49+ messages in thread
From: Richard Henderson @ 2020-05-22 2:34 UTC (permalink / raw)
To: qemu-devel; +Cc: alex.bennee
Several of the gp_reg[] elements are not relevant -- e.g. orig r3,
which is related to system calls. Omit those from the original
reginfo_init(), so that any differences are automatically hidden.
Do not only compare bit 4 of CCR -- this register is 32 bits wide
with 8 cr subfields. We should compare all of them.
Tidy reginfo_dump() output. Especially, do not dump the non-
relevant fields.
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
risu_reginfo_ppc64.c | 114 +++++++++++++++++--------------------------
1 file changed, 44 insertions(+), 70 deletions(-)
diff --git a/risu_reginfo_ppc64.c b/risu_reginfo_ppc64.c
index 134a152..62d7ff2 100644
--- a/risu_reginfo_ppc64.c
+++ b/risu_reginfo_ppc64.c
@@ -21,19 +21,30 @@
#include "risu.h"
#include "risu_reginfo_ppc64.h"
-#define XER 37
-#define CCR 38
+/* Names for indexes within gregset_t, ignoring those irrelevant here */
+enum {
+ NIP = 32,
+ MSR = 33,
+ CTR = 35,
+ LNK = 36,
+ XER = 37,
+ CCR = 38,
+};
const struct option * const arch_long_opts;
const char * const arch_extra_help;
static const char * const greg_names[NGREG] = {
- "r0", "r1", "r2", "r3", "r4", "r5", "r6", "r7",
- "r8", "r9", "r10", "r11", "r12", "r13", "r14", "r15",
- "r16", "r17", "r18", "r19", "r20", "r21", "r22", "r23",
- "r24", "r25", "r26", "r27", "r28", "r29", "r30", "r31",
- [XER] = "xer",
- [CCR] = "ccr",
+ "r0", "r1", "r2", "r3", "r4", "r5", "r6", "r7",
+ "r8", "r9", "r10", "r11", "r12", "r13", "r14", "r15",
+ "r16", "r17", "r18", "r19", "r20", "r21", "r22", "r23",
+ "r24", "r25", "r26", "r27", "r28", "r29", "r30", "r31",
+ [NIP] = "nip",
+ [MSR] = "msr",
+ [CTR] = "ctr",
+ [LNK] = "lnk",
+ [XER] = "xer",
+ [CCR] = "ccr",
};
void process_arch_opt(int opt, const char *arg)
@@ -61,8 +72,13 @@ void reginfo_init(struct reginfo *ri, ucontext_t *uc)
ri->nip = uc->uc_mcontext.regs->nip - image_start_address;
for (i = 0; i < NGREG; i++) {
- ri->gregs[i] = uc->uc_mcontext.gp_regs[i];
+ /* Do not copy gp_reg entries not relevant to the context. */
+ if (greg_names[i]) {
+ ri->gregs[i] = uc->uc_mcontext.gp_regs[i];
+ }
}
+ ri->gregs[1] = 0xdeadbeef; /* sp */
+ ri->gregs[13] = 0xdeadbeef; /* gp */
memcpy(ri->fpregs, uc->uc_mcontext.fp_regs, 32 * sizeof(double));
ri->fpscr = uc->uc_mcontext.fp_regs[32];
@@ -76,79 +92,37 @@ 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 *m, struct reginfo *a)
{
- int i;
- for (i = 0; i < 32; i++) {
- if (i == 1 || i == 13) {
- continue;
- }
-
- if (m->gregs[i] != a->gregs[i]) {
- return 0;
- }
- }
-
- if (m->gregs[XER] != a->gregs[XER]) {
- return 0;
- }
-
- if ((m->gregs[CCR] & 0x10) != (a->gregs[CCR] & 0x10)) {
- return 0;
- }
-
- for (i = 0; i < 32; i++) {
- if (m->fpregs[i] != a->fpregs[i]) {
- return 0;
- }
- }
-
- for (i = 0; i < 32; i++) {
- if (m->vrregs.vrregs[i][0] != a->vrregs.vrregs[i][0] ||
- m->vrregs.vrregs[i][1] != a->vrregs.vrregs[i][1] ||
- m->vrregs.vrregs[i][2] != a->vrregs.vrregs[i][2] ||
- m->vrregs.vrregs[i][3] != a->vrregs.vrregs[i][3]) {
- return 0;
- }
- }
- return 1;
+ return memcmp(m, a, sizeof(*m));
}
/* reginfo_dump: print state to a stream */
void reginfo_dump(struct reginfo *ri, FILE * f)
{
- int i;
+ const char *sep;
+ int i, j;
- fprintf(f, " faulting insn 0x%x\n", ri->faulting_insn);
- fprintf(f, " prev insn 0x%x\n", ri->prev_insn);
- fprintf(f, " prev addr 0x%" PRIx64 "\n\n", ri->nip);
+ fprintf(f, "%6s: %08x\n", "insn", ri->faulting_insn);
+ fprintf(f, "%6s: %016lx\n", "pc", ri->nip);
- for (i = 0; i < 16; i++) {
- fprintf(f, "\tr%2d: %16lx\tr%2d: %16lx\n", i, ri->gregs[i],
- i + 16, ri->gregs[i + 16]);
+ sep = "";
+ for (i = j = 0; i < NGREG; i++) {
+ if (greg_names[i] != NULL) {
+ fprintf(f, "%s%6s: %016lx", sep, greg_names[i], ri->gregs[i]);
+ sep = (++j & 1 ? " " : "\n");
+ }
}
- fprintf(f, "\n");
- fprintf(f, "\tnip : %16lx\n", ri->gregs[32]);
- fprintf(f, "\tmsr : %16lx\n", ri->gregs[33]);
- fprintf(f, "\torig r3: %16lx\n", ri->gregs[34]);
- fprintf(f, "\tctr : %16lx\n", ri->gregs[35]);
- fprintf(f, "\tlnk : %16lx\n", ri->gregs[36]);
- fprintf(f, "\txer : %16lx\n", ri->gregs[37]);
- fprintf(f, "\tccr : %16lx\n", ri->gregs[38]);
- fprintf(f, "\tmq : %16lx\n", ri->gregs[39]);
- fprintf(f, "\ttrap : %16lx\n", ri->gregs[40]);
- fprintf(f, "\tdar : %16lx\n", ri->gregs[41]);
- fprintf(f, "\tdsisr : %16lx\n", ri->gregs[42]);
- fprintf(f, "\tresult : %16lx\n", ri->gregs[43]);
- fprintf(f, "\tdscr : %16lx\n\n", ri->gregs[44]);
-
- for (i = 0; i < 16; i++) {
- fprintf(f, "\tf%2d: %016lx\tf%2d: %016lx\n", i, ri->fpregs[i],
- i + 16, ri->fpregs[i + 16]);
+ sep = "\n";
+ for (i = j = 0; i < 32; i++) {
+ fprintf(f, "%s%*s%d: %016lx",
+ sep, 6 - (i < 10 ? 1 : 2), "f", i, ri->fpregs[i]);
+ sep = (++j & 1 ? " " : "\n");
}
- fprintf(f, "\tfpscr: %016lx\n\n", ri->fpscr);
+ fprintf(f, "\n%6s: %016lx\n", "fpscr", ri->fpscr);
for (i = 0; i < 32; i++) {
- fprintf(f, "vr%02d: %8x, %8x, %8x, %8x\n", i,
+ fprintf(f, "%*s%d: %08x %08x %08x %08x\n",
+ 6 - (i < 10 ? 1 : 2), "vr", i,
ri->vrregs.vrregs[i][0], ri->vrregs.vrregs[i][1],
ri->vrregs.vrregs[i][2], ri->vrregs.vrregs[i][3]);
}
--
2.20.1
^ permalink raw reply related [flat|nested] 49+ messages in thread* Re: [PATCH v3 25/25] ppc64: Clean up reginfo handling
2020-05-22 2:34 ` [PATCH v3 25/25] ppc64: Clean up reginfo handling Richard Henderson
@ 2020-05-22 4:10 ` Richard Henderson
2020-06-23 17:45 ` Alex Bennée
1 sibling, 0 replies; 49+ messages in thread
From: Richard Henderson @ 2020-05-22 4:10 UTC (permalink / raw)
To: qemu-devel; +Cc: alex.bennee
On 5/21/20 7:34 PM, Richard Henderson wrote:
> + return memcmp(m, a, sizeof(*m));
Ho hum. memcmp(...) == 0. Fixed locally.
r~
^ permalink raw reply [flat|nested] 49+ messages in thread
* Re: [PATCH v3 25/25] ppc64: Clean up reginfo handling
2020-05-22 2:34 ` [PATCH v3 25/25] ppc64: Clean up reginfo handling Richard Henderson
2020-05-22 4:10 ` Richard Henderson
@ 2020-06-23 17:45 ` Alex Bennée
2020-06-23 18:06 ` Richard Henderson
1 sibling, 1 reply; 49+ messages in thread
From: Alex Bennée @ 2020-06-23 17:45 UTC (permalink / raw)
To: Richard Henderson; +Cc: qemu-devel
Richard Henderson <richard.henderson@linaro.org> writes:
> Several of the gp_reg[] elements are not relevant -- e.g. orig r3,
> which is related to system calls. Omit those from the original
> reginfo_init(), so that any differences are automatically hidden.
>
> Do not only compare bit 4 of CCR -- this register is 32 bits wide
> with 8 cr subfields. We should compare all of them.
>
> Tidy reginfo_dump() output. Especially, do not dump the non-
> relevant fields.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
I guess this means any traces that exist will need to be redone?
Anyway with your hotfix:
Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
--
Alex Bennée
^ permalink raw reply [flat|nested] 49+ messages in thread
* Re: [PATCH v3 25/25] ppc64: Clean up reginfo handling
2020-06-23 17:45 ` Alex Bennée
@ 2020-06-23 18:06 ` Richard Henderson
0 siblings, 0 replies; 49+ messages in thread
From: Richard Henderson @ 2020-06-23 18:06 UTC (permalink / raw)
To: Alex Bennée; +Cc: qemu-devel
On 6/23/20 10:45 AM, Alex Bennée wrote:
>
> Richard Henderson <richard.henderson@linaro.org> writes:
>
>> Several of the gp_reg[] elements are not relevant -- e.g. orig r3,
>> which is related to system calls. Omit those from the original
>> reginfo_init(), so that any differences are automatically hidden.
>>
>> Do not only compare bit 4 of CCR -- this register is 32 bits wide
>> with 8 cr subfields. We should compare all of them.
>>
>> Tidy reginfo_dump() output. Especially, do not dump the non-
>> relevant fields.
>>
>> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
>
> I guess this means any traces that exist will need to be redone?
Well, that's true anyway because of the format change.
r~
^ permalink raw reply [flat|nested] 49+ messages in thread
* Re: [PATCH v3 00/25] risu cleanups and improvements
2020-05-22 2:34 [PATCH v3 00/25] risu cleanups and improvements Richard Henderson
` (24 preceding siblings ...)
2020-05-22 2:34 ` [PATCH v3 25/25] ppc64: Clean up reginfo handling Richard Henderson
@ 2020-06-22 19:17 ` Peter Maydell
2020-06-23 8:23 ` Alex Bennée
2020-06-23 9:00 ` Alex Bennée
2020-06-23 14:44 ` [PATCH] risu: don't do a full register compare for OP_SIGILL Alex Bennée
27 siblings, 1 reply; 49+ messages in thread
From: Peter Maydell @ 2020-06-22 19:17 UTC (permalink / raw)
To: Richard Henderson; +Cc: Alex Bennée, QEMU Developers
Just a note that I'm assuming this is still on Alex's plate to review.
Ping me if it gets reviews and is ready to apply.
thanks
-- PMM
On Fri, 22 May 2020 at 03:35, Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> Version 3 changes the --dump option to --fulldump and --diffdump,
> after an off-hand suggestion by Alex.
>
> These are now mode options, similar to --master. Which means that
> dumping is an orthogonal apprentice type, which means that we can
> dump from a socket. I'm not sure that will be useful as such, but
> I think it makes main be a bit cleaner.
>
> If using old trace files with the new risu, you get
>
> Unexpected magic number: 0x000078
>
> If for somehow you use different risu for master and apprentice on
> sockets, the apprentice will hang waiting for data that the master
> will never write. This is less than helpful, but should be trivial
> to avoid.
>
> While cleaning up the interface for reginfo_dump_mismatch(), I
> noticed some bugs on the ppc64 side.
>
> The patches without reviews are:
>
> 0014-Merge-reginfo.c-into-risu.c.patch
> 0015-Rearrange-reginfo-and-memblock-buffers.patch
> 0016-Split-out-recv_register_info.patch
> 0017-Add-magic-and-size-to-the-trace-header.patch
> 0018-Compute-reginfo_size-based-on-the-reginfo.patch
> 0019-aarch64-Reorg-sve-reginfo-to-save-space.patch
> 0020-aarch64-Use-arch_init-to-configure-sve.patch
> 0021-ppc64-Use-uint64_t-to-represent-double.patch
> 0022-Standardize-reginfo_dump_mismatch-printing.patch
> 0023-Add-fulldump-and-diffdup-options.patch
> 0024-Remove-return-value-from-reginfo_dump.patch
> 0025-ppc64-Clean-up-reginfo-handling.patch
>
> most of which are new, and those that aren't new have had
> significant modifications.
>
>
> r~
>
>
> Richard Henderson (25):
> Use bool for tracing variables
> Unify master_fd and apprentice_fd to comm_fd
> Hoist trace file and socket opening
> Adjust tracefile open for write
> Use EXIT_FAILURE, EXIT_SUCCESS
> Make some risu.c symbols static
> Add enum RisuOp
> Add enum RisuResult
> Unify i/o functions and use RisuResult
> Pass non-OK result back through siglongjmp
> Always write for --master
> Simplify syncing with master
> Split RES_MISMATCH for registers and memory
> Merge reginfo.c into risu.c
> Rearrange reginfo and memblock buffers
> Split out recv_register_info
> Add magic and size to the trace header
> Compute reginfo_size based on the reginfo
> aarch64: Reorg sve reginfo to save space
> aarch64: Use arch_init to configure sve
> ppc64: Use uint64_t to represent double
> Standardize reginfo_dump_mismatch printing
> Add --fulldump and --diffdup options
> Remove return value from reginfo_dump
> ppc64: Clean up reginfo handling
>
> Makefile | 2 +-
> risu.h | 103 +++----
> risu_reginfo_aarch64.h | 16 +-
> risu_reginfo_ppc64.h | 3 +-
> comms.c | 34 +--
> reginfo.c | 183 -----------
> risu.c | 676 ++++++++++++++++++++++++++++++-----------
> risu_aarch64.c | 6 +-
> risu_arm.c | 6 +-
> risu_i386.c | 4 +-
> risu_m68k.c | 4 +-
> risu_ppc64.c | 4 +-
> risu_reginfo_aarch64.c | 212 +++++++------
> risu_reginfo_arm.c | 32 +-
> risu_reginfo_i386.c | 22 +-
> risu_reginfo_m68k.c | 37 +--
> risu_reginfo_ppc64.c | 183 +++++------
> 17 files changed, 803 insertions(+), 724 deletions(-)
> delete mode 100644 reginfo.c
>
> --
> 2.20.1
^ permalink raw reply [flat|nested] 49+ messages in thread* Re: [PATCH v3 00/25] risu cleanups and improvements
2020-05-22 2:34 [PATCH v3 00/25] risu cleanups and improvements Richard Henderson
` (25 preceding siblings ...)
2020-06-22 19:17 ` [PATCH v3 00/25] risu cleanups and improvements Peter Maydell
@ 2020-06-23 9:00 ` Alex Bennée
2020-06-23 13:30 ` Alex Bennée
2020-06-23 20:26 ` Richard Henderson
2020-06-23 14:44 ` [PATCH] risu: don't do a full register compare for OP_SIGILL Alex Bennée
27 siblings, 2 replies; 49+ messages in thread
From: Alex Bennée @ 2020-06-23 9:00 UTC (permalink / raw)
To: Richard Henderson; +Cc: qemu-devel
Richard Henderson <richard.henderson@linaro.org> writes:
> Version 3 changes the --dump option to --fulldump and --diffdump,
> after an off-hand suggestion by Alex.
>
> These are now mode options, similar to --master. Which means that
> dumping is an orthogonal apprentice type, which means that we can
> dump from a socket. I'm not sure that will be useful as such, but
> I think it makes main be a bit cleaner.
Hmm recording traces I ran into a difference, need to track down if its
a master or apprentice bug (both are native):
./builds/arm64/risu aarch64-all-v8dot0/insn_LDAPR__INC.risu.bin -t aarch64-all-v8dot0/insn_LDAPR__INC.risu.bin.trace
fails with:
loading test image aarch64-all-v8dot0/insn_LDAPR__INC.risu.bin...
starting apprentice image at 0xffff8548c000
starting image
Mismatch reg after 4 checkpoints
master reginfo:
faulting insn 38bfc1f4
.
.
.
mismatch detail (master : apprentice):
X15 : 0000ffff9eba41dc vs 0000ffff8548c1dc
>
> If using old trace files with the new risu, you get
>
> Unexpected magic number: 0x000078
>
> If for somehow you use different risu for master and apprentice on
> sockets, the apprentice will hang waiting for data that the master
> will never write. This is less than helpful, but should be trivial
> to avoid.
>
> While cleaning up the interface for reginfo_dump_mismatch(), I
> noticed some bugs on the ppc64 side.
>
> The patches without reviews are:
>
> 0014-Merge-reginfo.c-into-risu.c.patch
> 0015-Rearrange-reginfo-and-memblock-buffers.patch
> 0016-Split-out-recv_register_info.patch
> 0017-Add-magic-and-size-to-the-trace-header.patch
> 0018-Compute-reginfo_size-based-on-the-reginfo.patch
> 0019-aarch64-Reorg-sve-reginfo-to-save-space.patch
> 0020-aarch64-Use-arch_init-to-configure-sve.patch
> 0021-ppc64-Use-uint64_t-to-represent-double.patch
> 0022-Standardize-reginfo_dump_mismatch-printing.patch
> 0023-Add-fulldump-and-diffdup-options.patch
> 0024-Remove-return-value-from-reginfo_dump.patch
> 0025-ppc64-Clean-up-reginfo-handling.patch
>
> most of which are new, and those that aren't new have had
> significant modifications.
>
>
> r~
>
>
> Richard Henderson (25):
> Use bool for tracing variables
> Unify master_fd and apprentice_fd to comm_fd
> Hoist trace file and socket opening
> Adjust tracefile open for write
> Use EXIT_FAILURE, EXIT_SUCCESS
> Make some risu.c symbols static
> Add enum RisuOp
> Add enum RisuResult
> Unify i/o functions and use RisuResult
> Pass non-OK result back through siglongjmp
> Always write for --master
> Simplify syncing with master
> Split RES_MISMATCH for registers and memory
> Merge reginfo.c into risu.c
> Rearrange reginfo and memblock buffers
> Split out recv_register_info
> Add magic and size to the trace header
> Compute reginfo_size based on the reginfo
> aarch64: Reorg sve reginfo to save space
> aarch64: Use arch_init to configure sve
> ppc64: Use uint64_t to represent double
> Standardize reginfo_dump_mismatch printing
> Add --fulldump and --diffdup options
> Remove return value from reginfo_dump
> ppc64: Clean up reginfo handling
>
> Makefile | 2 +-
> risu.h | 103 +++----
> risu_reginfo_aarch64.h | 16 +-
> risu_reginfo_ppc64.h | 3 +-
> comms.c | 34 +--
> reginfo.c | 183 -----------
> risu.c | 676 ++++++++++++++++++++++++++++++-----------
> risu_aarch64.c | 6 +-
> risu_arm.c | 6 +-
> risu_i386.c | 4 +-
> risu_m68k.c | 4 +-
> risu_ppc64.c | 4 +-
> risu_reginfo_aarch64.c | 212 +++++++------
> risu_reginfo_arm.c | 32 +-
> risu_reginfo_i386.c | 22 +-
> risu_reginfo_m68k.c | 37 +--
> risu_reginfo_ppc64.c | 183 +++++------
> 17 files changed, 803 insertions(+), 724 deletions(-)
> delete mode 100644 reginfo.c
--
Alex Bennée
^ permalink raw reply [flat|nested] 49+ messages in thread* Re: [PATCH v3 00/25] risu cleanups and improvements
2020-06-23 9:00 ` Alex Bennée
@ 2020-06-23 13:30 ` Alex Bennée
2020-06-23 20:26 ` Richard Henderson
1 sibling, 0 replies; 49+ messages in thread
From: Alex Bennée @ 2020-06-23 13:30 UTC (permalink / raw)
To: Richard Henderson; +Cc: qemu-devel
Alex Bennée <alex.bennee@linaro.org> writes:
> Richard Henderson <richard.henderson@linaro.org> writes:
>
>> Version 3 changes the --dump option to --fulldump and --diffdump,
>> after an off-hand suggestion by Alex.
>>
>> These are now mode options, similar to --master. Which means that
>> dumping is an orthogonal apprentice type, which means that we can
>> dump from a socket. I'm not sure that will be useful as such, but
>> I think it makes main be a bit cleaner.
>
> Hmm recording traces I ran into a difference, need to track down if its
> a master or apprentice bug (both are native):
>
> ./builds/arm64/risu aarch64-all-v8dot0/insn_LDAPR__INC.risu.bin -t aarch64-all-v8dot0/insn_LDAPR__INC.risu.bin.trace
>
> fails with:
>
> loading test image aarch64-all-v8dot0/insn_LDAPR__INC.risu.bin...
> starting apprentice image at 0xffff8548c000
> starting image
> Mismatch reg after 4 checkpoints
> master reginfo:
> faulting insn 38bfc1f4
> .
> .
> .
> mismatch detail (master : apprentice):
> X15 : 0000ffff9eba41dc vs 0000ffff8548c1dc
This is sort of bogus due to LDAPR being a v8.3 instruction so we fault
and x15 is a pointer into the memblock which isn't stable. If it had
executed we would have nixed the absolution address to and offset before
the OP_COMPARE.
I think for OP_SIGILL the only thing that can be truly valid would be to
compare PCs so we can check the apprentice also faults on the same SIGILL?
>
>>
>> If using old trace files with the new risu, you get
>>
>> Unexpected magic number: 0x000078
>>
>> If for somehow you use different risu for master and apprentice on
>> sockets, the apprentice will hang waiting for data that the master
>> will never write. This is less than helpful, but should be trivial
>> to avoid.
>>
>> While cleaning up the interface for reginfo_dump_mismatch(), I
>> noticed some bugs on the ppc64 side.
>>
>> The patches without reviews are:
>>
>> 0014-Merge-reginfo.c-into-risu.c.patch
>> 0015-Rearrange-reginfo-and-memblock-buffers.patch
>> 0016-Split-out-recv_register_info.patch
>> 0017-Add-magic-and-size-to-the-trace-header.patch
>> 0018-Compute-reginfo_size-based-on-the-reginfo.patch
>> 0019-aarch64-Reorg-sve-reginfo-to-save-space.patch
>> 0020-aarch64-Use-arch_init-to-configure-sve.patch
>> 0021-ppc64-Use-uint64_t-to-represent-double.patch
>> 0022-Standardize-reginfo_dump_mismatch-printing.patch
>> 0023-Add-fulldump-and-diffdup-options.patch
>> 0024-Remove-return-value-from-reginfo_dump.patch
>> 0025-ppc64-Clean-up-reginfo-handling.patch
>>
>> most of which are new, and those that aren't new have had
>> significant modifications.
>>
>>
>> r~
>>
>>
>> Richard Henderson (25):
>> Use bool for tracing variables
>> Unify master_fd and apprentice_fd to comm_fd
>> Hoist trace file and socket opening
>> Adjust tracefile open for write
>> Use EXIT_FAILURE, EXIT_SUCCESS
>> Make some risu.c symbols static
>> Add enum RisuOp
>> Add enum RisuResult
>> Unify i/o functions and use RisuResult
>> Pass non-OK result back through siglongjmp
>> Always write for --master
>> Simplify syncing with master
>> Split RES_MISMATCH for registers and memory
>> Merge reginfo.c into risu.c
>> Rearrange reginfo and memblock buffers
>> Split out recv_register_info
>> Add magic and size to the trace header
>> Compute reginfo_size based on the reginfo
>> aarch64: Reorg sve reginfo to save space
>> aarch64: Use arch_init to configure sve
>> ppc64: Use uint64_t to represent double
>> Standardize reginfo_dump_mismatch printing
>> Add --fulldump and --diffdup options
>> Remove return value from reginfo_dump
>> ppc64: Clean up reginfo handling
>>
>> Makefile | 2 +-
>> risu.h | 103 +++----
>> risu_reginfo_aarch64.h | 16 +-
>> risu_reginfo_ppc64.h | 3 +-
>> comms.c | 34 +--
>> reginfo.c | 183 -----------
>> risu.c | 676 ++++++++++++++++++++++++++++++-----------
>> risu_aarch64.c | 6 +-
>> risu_arm.c | 6 +-
>> risu_i386.c | 4 +-
>> risu_m68k.c | 4 +-
>> risu_ppc64.c | 4 +-
>> risu_reginfo_aarch64.c | 212 +++++++------
>> risu_reginfo_arm.c | 32 +-
>> risu_reginfo_i386.c | 22 +-
>> risu_reginfo_m68k.c | 37 +--
>> risu_reginfo_ppc64.c | 183 +++++------
>> 17 files changed, 803 insertions(+), 724 deletions(-)
>> delete mode 100644 reginfo.c
--
Alex Bennée
^ permalink raw reply [flat|nested] 49+ messages in thread
* Re: [PATCH v3 00/25] risu cleanups and improvements
2020-06-23 9:00 ` Alex Bennée
2020-06-23 13:30 ` Alex Bennée
@ 2020-06-23 20:26 ` Richard Henderson
1 sibling, 0 replies; 49+ messages in thread
From: Richard Henderson @ 2020-06-23 20:26 UTC (permalink / raw)
To: Alex Bennée; +Cc: qemu-devel
On 6/23/20 2:00 AM, Alex Bennée wrote:
>
> Richard Henderson <richard.henderson@linaro.org> writes:
>
>> Version 3 changes the --dump option to --fulldump and --diffdump,
>> after an off-hand suggestion by Alex.
>>
>> These are now mode options, similar to --master. Which means that
>> dumping is an orthogonal apprentice type, which means that we can
>> dump from a socket. I'm not sure that will be useful as such, but
>> I think it makes main be a bit cleaner.
>
> Hmm recording traces I ran into a difference, need to track down if its
> a master or apprentice bug (both are native):
>
> ./builds/arm64/risu aarch64-all-v8dot0/insn_LDAPR__INC.risu.bin -t aarch64-all-v8dot0/insn_LDAPR__INC.risu.bin.trace
>
> fails with:
>
> loading test image aarch64-all-v8dot0/insn_LDAPR__INC.risu.bin...
> starting apprentice image at 0xffff8548c000
> starting image
> Mismatch reg after 4 checkpoints
> master reginfo:
> faulting insn 38bfc1f4
> .
> .
> .
> mismatch detail (master : apprentice):
> X15 : 0000ffff9eba41dc vs 0000ffff8548c1dc
Did this work before? I can't see how.
Your v8dot0 string is suspicious, because LDAPR is a v8.3 instruction. Are you
sure you're testing what you think you are?
r~
^ permalink raw reply [flat|nested] 49+ messages in thread
* [PATCH] risu: don't do a full register compare for OP_SIGILL
2020-05-22 2:34 [PATCH v3 00/25] risu cleanups and improvements Richard Henderson
` (26 preceding siblings ...)
2020-06-23 9:00 ` Alex Bennée
@ 2020-06-23 14:44 ` Alex Bennée
2020-06-23 15:23 ` Richard Henderson
2020-06-23 15:54 ` Peter Maydell
27 siblings, 2 replies; 49+ messages in thread
From: Alex Bennée @ 2020-06-23 14:44 UTC (permalink / raw)
To: qemu-devel; +Cc: Alex Bennée, richard.henderson
OP_SIGILL means we have an unexpected invalid operation. If this is a
load or store the register state may be un-rectified pointing at the
memblock so would be invalid. In this case just compare the PC and
make sure the other end also faulted at the same place.
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
---
risu.c | 14 ++++++++++----
1 file changed, 10 insertions(+), 4 deletions(-)
diff --git a/risu.c b/risu.c
index 8d907d9..6d6dcf9 100644
--- a/risu.c
+++ b/risu.c
@@ -124,7 +124,6 @@ static RisuResult send_register_info(void *uc)
switch (op) {
case OP_TESTEND:
case OP_COMPARE:
- case OP_SIGILL:
header.size = reginfo_size(&ri[MASTER]);
extra = &ri[MASTER];
break;
@@ -132,6 +131,7 @@ static RisuResult send_register_info(void *uc)
header.size = MEMBLOCKLEN;
extra = memblock;
break;
+ case OP_SIGILL:
case OP_SETMEMBLOCK:
case OP_GETMEMBLOCK:
header.size = 0;
@@ -203,7 +203,6 @@ static RisuResult recv_register_info(struct reginfo *ri)
switch (header.risu_op) {
case OP_COMPARE:
case OP_TESTEND:
- case OP_SIGILL:
/* If we can't store the data, report invalid size. */
if (header.size > sizeof(*ri)) {
return RES_BAD_SIZE;
@@ -223,6 +222,7 @@ static RisuResult recv_register_info(struct reginfo *ri)
respond(RES_OK);
return read_buffer(other_memblock, MEMBLOCKLEN);
+ case OP_SIGILL:
case OP_SETMEMBLOCK:
case OP_GETMEMBLOCK:
return header.size == 0 ? RES_OK : RES_BAD_SIZE;
@@ -250,7 +250,6 @@ static RisuResult recv_and_compare_register_info(void *uc)
switch (op) {
case OP_COMPARE:
case OP_TESTEND:
- case OP_SIGILL:
/*
* If we have nothing to compare against, report an op mismatch.
* Otherwise allow the compare to continue, and assume that
@@ -270,7 +269,14 @@ static RisuResult recv_and_compare_register_info(void *uc)
res = RES_END;
}
break;
-
+ case OP_SIGILL:
+ /* We can only check the op and PC */
+ if (header.risu_op != OP_SIGILL) {
+ res = RES_MISMATCH_OP;
+ } else if (header.pc != get_pc(&ri[APPRENTICE])) {
+ res = RES_MISMATCH_REG;
+ }
+ break;
case OP_SETMEMBLOCK:
if (op != header.risu_op) {
res = RES_MISMATCH_OP;
--
2.20.1
^ permalink raw reply related [flat|nested] 49+ messages in thread* Re: [PATCH] risu: don't do a full register compare for OP_SIGILL
2020-06-23 14:44 ` [PATCH] risu: don't do a full register compare for OP_SIGILL Alex Bennée
@ 2020-06-23 15:23 ` Richard Henderson
2020-06-23 15:54 ` Peter Maydell
1 sibling, 0 replies; 49+ messages in thread
From: Richard Henderson @ 2020-06-23 15:23 UTC (permalink / raw)
To: Alex Bennée, qemu-devel
On 6/23/20 7:44 AM, Alex Bennée wrote:
> + } else if (header.pc != get_pc(&ri[APPRENTICE])) {
> + res = RES_MISMATCH_REG;
You need a new MISMATCH code, because this one implies that you have a reginfo
struct to compare.
But thanks, I'll incorporate this.
r~
^ permalink raw reply [flat|nested] 49+ messages in thread* Re: [PATCH] risu: don't do a full register compare for OP_SIGILL
2020-06-23 14:44 ` [PATCH] risu: don't do a full register compare for OP_SIGILL Alex Bennée
2020-06-23 15:23 ` Richard Henderson
@ 2020-06-23 15:54 ` Peter Maydell
2020-06-23 16:17 ` Alex Bennée
1 sibling, 1 reply; 49+ messages in thread
From: Peter Maydell @ 2020-06-23 15:54 UTC (permalink / raw)
To: Alex Bennée; +Cc: Richard Henderson, QEMU Developers
On Tue, 23 Jun 2020 at 16:07, Alex Bennée <alex.bennee@linaro.org> wrote:
>
> OP_SIGILL means we have an unexpected invalid operation. If this is a
> load or store the register state may be un-rectified pointing at the
> memblock so would be invalid. In this case just compare the PC and
> make sure the other end also faulted at the same place.
In case of mismatch of the PC do we still print the full register dump?
thanks
-- PMM
^ permalink raw reply [flat|nested] 49+ messages in thread
* Re: [PATCH] risu: don't do a full register compare for OP_SIGILL
2020-06-23 15:54 ` Peter Maydell
@ 2020-06-23 16:17 ` Alex Bennée
2020-06-23 19:55 ` Richard Henderson
0 siblings, 1 reply; 49+ messages in thread
From: Alex Bennée @ 2020-06-23 16:17 UTC (permalink / raw)
To: Peter Maydell; +Cc: Richard Henderson, QEMU Developers
Peter Maydell <peter.maydell@linaro.org> writes:
> On Tue, 23 Jun 2020 at 16:07, Alex Bennée <alex.bennee@linaro.org> wrote:
>>
>> OP_SIGILL means we have an unexpected invalid operation. If this is a
>> load or store the register state may be un-rectified pointing at the
>> memblock so would be invalid. In this case just compare the PC and
>> make sure the other end also faulted at the same place.
>
> In case of mismatch of the PC do we still print the full register
> dump?
As rth points out with a new mismatch code we could do whatever we
wanted on a fail although the "master" will not have sent a register
dump so we can only dump the local register state.
>
> thanks
> -- PMM
--
Alex Bennée
^ permalink raw reply [flat|nested] 49+ messages in thread
* Re: [PATCH] risu: don't do a full register compare for OP_SIGILL
2020-06-23 16:17 ` Alex Bennée
@ 2020-06-23 19:55 ` Richard Henderson
0 siblings, 0 replies; 49+ messages in thread
From: Richard Henderson @ 2020-06-23 19:55 UTC (permalink / raw)
To: Alex Bennée, Peter Maydell; +Cc: QEMU Developers
On 6/23/20 9:17 AM, Alex Bennée wrote:
>
> Peter Maydell <peter.maydell@linaro.org> writes:
>
>> On Tue, 23 Jun 2020 at 16:07, Alex Bennée <alex.bennee@linaro.org> wrote:
>>>
>>> OP_SIGILL means we have an unexpected invalid operation. If this is a
>>> load or store the register state may be un-rectified pointing at the
>>> memblock so would be invalid. In this case just compare the PC and
>>> make sure the other end also faulted at the same place.
>>
>> In case of mismatch of the PC do we still print the full register
>> dump?
No. If we want that, we should do something else, like remember that the
memory pointer is in use and zap it out before reporting the register set.
But, generally, if we see SIGILL, then we have not actually executed anything,
so the register state doesn't matter too much. What probably does want
reporting in this case is the insn opcode.
r~
^ permalink raw reply [flat|nested] 49+ messages in thread