* [RISU PATCH v4 01/29] Use bool for tracing variables
2022-07-08 15:46 [RISU PATCH v4 00/29] risu cleanups and improvements Richard Henderson
@ 2022-07-08 15:46 ` Richard Henderson
2022-07-08 15:46 ` [RISU PATCH v4 02/29] Unify master_fd and apprentice_fd to comm_fd Richard Henderson
` (28 subsequent siblings)
29 siblings, 0 replies; 47+ messages in thread
From: Richard Henderson @ 2022-07-08 15:46 UTC (permalink / raw)
To: qemu-devel; +Cc: qemu-arm, Alex Bennée, Peter Maydell
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.34.1
^ permalink raw reply related [flat|nested] 47+ messages in thread* [RISU PATCH v4 02/29] Unify master_fd and apprentice_fd to comm_fd
2022-07-08 15:46 [RISU PATCH v4 00/29] risu cleanups and improvements Richard Henderson
2022-07-08 15:46 ` [RISU PATCH v4 01/29] Use bool for tracing variables Richard Henderson
@ 2022-07-08 15:46 ` Richard Henderson
2022-07-08 15:46 ` [RISU PATCH v4 03/29] Hoist trace file and socket opening Richard Henderson
` (27 subsequent siblings)
29 siblings, 0 replies; 47+ messages in thread
From: Richard Henderson @ 2022-07-08 15:46 UTC (permalink / raw)
To: qemu-devel; +Cc: qemu-arm, Alex Bennée, Peter Maydell
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.34.1
^ permalink raw reply related [flat|nested] 47+ messages in thread* [RISU PATCH v4 03/29] Hoist trace file and socket opening
2022-07-08 15:46 [RISU PATCH v4 00/29] risu cleanups and improvements Richard Henderson
2022-07-08 15:46 ` [RISU PATCH v4 01/29] Use bool for tracing variables Richard Henderson
2022-07-08 15:46 ` [RISU PATCH v4 02/29] Unify master_fd and apprentice_fd to comm_fd Richard Henderson
@ 2022-07-08 15:46 ` Richard Henderson
2022-07-08 15:46 ` [RISU PATCH v4 04/29] Adjust tracefile open for write Richard Henderson
` (26 subsequent siblings)
29 siblings, 0 replies; 47+ messages in thread
From: Richard Henderson @ 2022-07-08 15:46 UTC (permalink / raw)
To: qemu-devel; +Cc: qemu-arm, Alex Bennée, Peter Maydell
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.34.1
^ permalink raw reply related [flat|nested] 47+ messages in thread* [RISU PATCH v4 04/29] Adjust tracefile open for write
2022-07-08 15:46 [RISU PATCH v4 00/29] risu cleanups and improvements Richard Henderson
` (2 preceding siblings ...)
2022-07-08 15:46 ` [RISU PATCH v4 03/29] Hoist trace file and socket opening Richard Henderson
@ 2022-07-08 15:46 ` Richard Henderson
2022-07-08 15:46 ` [RISU PATCH v4 05/29] Use EXIT_FAILURE, EXIT_SUCCESS Richard Henderson
` (25 subsequent siblings)
29 siblings, 0 replies; 47+ messages in thread
From: Richard Henderson @ 2022-07-08 15:46 UTC (permalink / raw)
To: qemu-devel; +Cc: qemu-arm, Alex Bennée, Peter Maydell
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.34.1
^ permalink raw reply related [flat|nested] 47+ messages in thread* [RISU PATCH v4 05/29] Use EXIT_FAILURE, EXIT_SUCCESS
2022-07-08 15:46 [RISU PATCH v4 00/29] risu cleanups and improvements Richard Henderson
` (3 preceding siblings ...)
2022-07-08 15:46 ` [RISU PATCH v4 04/29] Adjust tracefile open for write Richard Henderson
@ 2022-07-08 15:46 ` Richard Henderson
2022-07-08 15:46 ` [RISU PATCH v4 06/29] Make some risu.c symbols static Richard Henderson
` (24 subsequent siblings)
29 siblings, 0 replies; 47+ messages in thread
From: Richard Henderson @ 2022-07-08 15:46 UTC (permalink / raw)
To: qemu-devel; +Cc: qemu-arm, Alex Bennée, Peter Maydell
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.34.1
^ permalink raw reply related [flat|nested] 47+ messages in thread* [RISU PATCH v4 06/29] Make some risu.c symbols static
2022-07-08 15:46 [RISU PATCH v4 00/29] risu cleanups and improvements Richard Henderson
` (4 preceding siblings ...)
2022-07-08 15:46 ` [RISU PATCH v4 05/29] Use EXIT_FAILURE, EXIT_SUCCESS Richard Henderson
@ 2022-07-08 15:46 ` Richard Henderson
2022-07-08 15:46 ` [RISU PATCH v4 07/29] Add enum RisuOp Richard Henderson
` (23 subsequent siblings)
29 siblings, 0 replies; 47+ messages in thread
From: Richard Henderson @ 2022-07-08 15:46 UTC (permalink / raw)
To: qemu-devel; +Cc: qemu-arm, Alex Bennée
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.34.1
^ permalink raw reply related [flat|nested] 47+ messages in thread* [RISU PATCH v4 07/29] Add enum RisuOp
2022-07-08 15:46 [RISU PATCH v4 00/29] risu cleanups and improvements Richard Henderson
` (5 preceding siblings ...)
2022-07-08 15:46 ` [RISU PATCH v4 06/29] Make some risu.c symbols static Richard Henderson
@ 2022-07-08 15:46 ` Richard Henderson
2022-07-08 15:46 ` [RISU PATCH v4 08/29] Add enum RisuResult Richard Henderson
` (22 subsequent siblings)
29 siblings, 0 replies; 47+ messages in thread
From: Richard Henderson @ 2022-07-08 15:46 UTC (permalink / raw)
To: qemu-devel; +Cc: qemu-arm, Alex Bennée
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.34.1
^ permalink raw reply related [flat|nested] 47+ messages in thread* [RISU PATCH v4 08/29] Add enum RisuResult
2022-07-08 15:46 [RISU PATCH v4 00/29] risu cleanups and improvements Richard Henderson
` (6 preceding siblings ...)
2022-07-08 15:46 ` [RISU PATCH v4 07/29] Add enum RisuOp Richard Henderson
@ 2022-07-08 15:46 ` Richard Henderson
2022-07-08 15:46 ` [RISU PATCH v4 09/29] Unify i/o functions and use RisuResult Richard Henderson
` (21 subsequent siblings)
29 siblings, 0 replies; 47+ messages in thread
From: Richard Henderson @ 2022-07-08 15:46 UTC (permalink / raw)
To: qemu-devel; +Cc: qemu-arm, Alex Bennée
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.34.1
^ permalink raw reply related [flat|nested] 47+ messages in thread* [RISU PATCH v4 09/29] Unify i/o functions and use RisuResult
2022-07-08 15:46 [RISU PATCH v4 00/29] risu cleanups and improvements Richard Henderson
` (7 preceding siblings ...)
2022-07-08 15:46 ` [RISU PATCH v4 08/29] Add enum RisuResult Richard Henderson
@ 2022-07-08 15:46 ` Richard Henderson
2022-07-08 15:46 ` [RISU PATCH v4 10/29] Pass non-OK result back through siglongjmp Richard Henderson
` (20 subsequent siblings)
29 siblings, 0 replies; 47+ messages in thread
From: Richard Henderson @ 2022-07-08 15:46 UTC (permalink / raw)
To: qemu-devel; +Cc: qemu-arm, Alex Bennée
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.34.1
^ permalink raw reply related [flat|nested] 47+ messages in thread* [RISU PATCH v4 10/29] Pass non-OK result back through siglongjmp
2022-07-08 15:46 [RISU PATCH v4 00/29] risu cleanups and improvements Richard Henderson
` (8 preceding siblings ...)
2022-07-08 15:46 ` [RISU PATCH v4 09/29] Unify i/o functions and use RisuResult Richard Henderson
@ 2022-07-08 15:46 ` Richard Henderson
2022-07-08 15:46 ` [RISU PATCH v4 11/29] Always write for --master Richard Henderson
` (19 subsequent siblings)
29 siblings, 0 replies; 47+ messages in thread
From: Richard Henderson @ 2022-07-08 15:46 UTC (permalink / raw)
To: qemu-devel; +Cc: qemu-arm, Alex Bennée
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.34.1
^ permalink raw reply related [flat|nested] 47+ messages in thread* [RISU PATCH v4 11/29] Always write for --master
2022-07-08 15:46 [RISU PATCH v4 00/29] risu cleanups and improvements Richard Henderson
` (9 preceding siblings ...)
2022-07-08 15:46 ` [RISU PATCH v4 10/29] Pass non-OK result back through siglongjmp Richard Henderson
@ 2022-07-08 15:46 ` Richard Henderson
2022-07-08 15:46 ` [RISU PATCH v4 12/29] Simplify syncing with master Richard Henderson
` (18 subsequent siblings)
29 siblings, 0 replies; 47+ messages in thread
From: Richard Henderson @ 2022-07-08 15:46 UTC (permalink / raw)
To: qemu-devel; +Cc: qemu-arm, Alex Bennée
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.34.1
^ permalink raw reply related [flat|nested] 47+ messages in thread* [RISU PATCH v4 12/29] Simplify syncing with master
2022-07-08 15:46 [RISU PATCH v4 00/29] risu cleanups and improvements Richard Henderson
` (10 preceding siblings ...)
2022-07-08 15:46 ` [RISU PATCH v4 11/29] Always write for --master Richard Henderson
@ 2022-07-08 15:46 ` Richard Henderson
2022-07-08 15:46 ` [RISU PATCH v4 13/29] Split RES_MISMATCH for registers and memory Richard Henderson
` (17 subsequent siblings)
29 siblings, 0 replies; 47+ messages in thread
From: Richard Henderson @ 2022-07-08 15:46 UTC (permalink / raw)
To: qemu-devel; +Cc: qemu-arm, Alex Bennée
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.34.1
^ permalink raw reply related [flat|nested] 47+ messages in thread* [RISU PATCH v4 13/29] Split RES_MISMATCH for registers and memory
2022-07-08 15:46 [RISU PATCH v4 00/29] risu cleanups and improvements Richard Henderson
` (11 preceding siblings ...)
2022-07-08 15:46 ` [RISU PATCH v4 12/29] Simplify syncing with master Richard Henderson
@ 2022-07-08 15:46 ` Richard Henderson
2022-07-08 15:46 ` [RISU PATCH v4 14/29] Merge reginfo.c into risu.c Richard Henderson
` (16 subsequent siblings)
29 siblings, 0 replies; 47+ messages in thread
From: Richard Henderson @ 2022-07-08 15:46 UTC (permalink / raw)
To: qemu-devel; +Cc: qemu-arm, Alex Bennée
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.34.1
^ permalink raw reply related [flat|nested] 47+ messages in thread* [RISU PATCH v4 14/29] Merge reginfo.c into risu.c
2022-07-08 15:46 [RISU PATCH v4 00/29] risu cleanups and improvements Richard Henderson
` (12 preceding siblings ...)
2022-07-08 15:46 ` [RISU PATCH v4 13/29] Split RES_MISMATCH for registers and memory Richard Henderson
@ 2022-07-08 15:46 ` Richard Henderson
2022-07-18 10:46 ` Peter Maydell
2022-07-08 15:46 ` [RISU PATCH v4 15/29] Rearrange reginfo and memblock buffers Richard Henderson
` (15 subsequent siblings)
29 siblings, 1 reply; 47+ messages in thread
From: Richard Henderson @ 2022-07-08 15:46 UTC (permalink / raw)
To: qemu-devel; +Cc: qemu-arm
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.34.1
^ permalink raw reply related [flat|nested] 47+ messages in thread* [RISU PATCH v4 15/29] Rearrange reginfo and memblock buffers
2022-07-08 15:46 [RISU PATCH v4 00/29] risu cleanups and improvements Richard Henderson
` (13 preceding siblings ...)
2022-07-08 15:46 ` [RISU PATCH v4 14/29] Merge reginfo.c into risu.c Richard Henderson
@ 2022-07-08 15:46 ` Richard Henderson
2022-07-18 10:46 ` Peter Maydell
2022-07-08 15:46 ` [RISU PATCH v4 16/29] Split out recv_register_info Richard Henderson
` (14 subsequent siblings)
29 siblings, 1 reply; 47+ messages in thread
From: Richard Henderson @ 2022-07-08 15:46 UTC (permalink / raw)
To: qemu-devel; +Cc: qemu-arm
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.34.1
^ permalink raw reply related [flat|nested] 47+ messages in thread* Re: [RISU PATCH v4 15/29] Rearrange reginfo and memblock buffers
2022-07-08 15:46 ` [RISU PATCH v4 15/29] Rearrange reginfo and memblock buffers Richard Henderson
@ 2022-07-18 10:46 ` Peter Maydell
0 siblings, 0 replies; 47+ messages in thread
From: Peter Maydell @ 2022-07-18 10:46 UTC (permalink / raw)
To: Richard Henderson; +Cc: qemu-devel, qemu-arm
On Fri, 8 Jul 2022 at 17:06, Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> 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>
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
thanks
-- PMM
^ permalink raw reply [flat|nested] 47+ messages in thread
* [RISU PATCH v4 16/29] Split out recv_register_info
2022-07-08 15:46 [RISU PATCH v4 00/29] risu cleanups and improvements Richard Henderson
` (14 preceding siblings ...)
2022-07-08 15:46 ` [RISU PATCH v4 15/29] Rearrange reginfo and memblock buffers Richard Henderson
@ 2022-07-08 15:46 ` Richard Henderson
2022-07-18 10:48 ` Peter Maydell
2022-07-08 15:46 ` [RISU PATCH v4 17/29] Add magic and size to the trace header Richard Henderson
` (13 subsequent siblings)
29 siblings, 1 reply; 47+ messages in thread
From: Richard Henderson @ 2022-07-08 15:46 UTC (permalink / raw)
To: qemu-devel; +Cc: qemu-arm
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.34.1
^ permalink raw reply related [flat|nested] 47+ messages in thread* [RISU PATCH v4 17/29] Add magic and size to the trace header
2022-07-08 15:46 [RISU PATCH v4 00/29] risu cleanups and improvements Richard Henderson
` (15 preceding siblings ...)
2022-07-08 15:46 ` [RISU PATCH v4 16/29] Split out recv_register_info Richard Henderson
@ 2022-07-08 15:46 ` Richard Henderson
2022-07-18 10:51 ` Peter Maydell
2022-07-08 15:46 ` [RISU PATCH v4 18/29] Compute reginfo_size based on the reginfo Richard Henderson
` (12 subsequent siblings)
29 siblings, 1 reply; 47+ messages in thread
From: Richard Henderson @ 2022-07-08 15:46 UTC (permalink / raw)
To: qemu-devel; +Cc: qemu-arm
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.34.1
^ permalink raw reply related [flat|nested] 47+ messages in thread* [RISU PATCH v4 18/29] Compute reginfo_size based on the reginfo
2022-07-08 15:46 [RISU PATCH v4 00/29] risu cleanups and improvements Richard Henderson
` (16 preceding siblings ...)
2022-07-08 15:46 ` [RISU PATCH v4 17/29] Add magic and size to the trace header Richard Henderson
@ 2022-07-08 15:46 ` Richard Henderson
2022-07-18 11:37 ` Peter Maydell
2022-07-08 15:46 ` [RISU PATCH v4 19/29] aarch64: Assume system support for SVE Richard Henderson
` (11 subsequent siblings)
29 siblings, 1 reply; 47+ messages in thread
From: Richard Henderson @ 2022-07-08 15:46 UTC (permalink / raw)
To: qemu-devel; +Cc: qemu-arm
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.34.1
^ permalink raw reply related [flat|nested] 47+ messages in thread* [RISU PATCH v4 19/29] aarch64: Assume system support for SVE
2022-07-08 15:46 [RISU PATCH v4 00/29] risu cleanups and improvements Richard Henderson
` (17 preceding siblings ...)
2022-07-08 15:46 ` [RISU PATCH v4 18/29] Compute reginfo_size based on the reginfo Richard Henderson
@ 2022-07-08 15:46 ` Richard Henderson
2022-07-18 11:38 ` Peter Maydell
2022-07-08 15:46 ` [RISU PATCH v4 20/29] aarch64: Reorg sve reginfo to save space Richard Henderson
` (10 subsequent siblings)
29 siblings, 1 reply; 47+ messages in thread
From: Richard Henderson @ 2022-07-08 15:46 UTC (permalink / raw)
To: qemu-devel; +Cc: qemu-arm
SVE support is no longer new, assume it's present.
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
risu_reginfo_aarch64.h | 4 ----
risu_reginfo_aarch64.c | 24 ------------------------
2 files changed, 28 deletions(-)
diff --git a/risu_reginfo_aarch64.h b/risu_reginfo_aarch64.h
index c33b86f..efbca56 100644
--- a/risu_reginfo_aarch64.h
+++ b/risu_reginfo_aarch64.h
@@ -20,7 +20,6 @@ struct simd_reginfo {
char end[0];
};
-#ifdef SVE_MAGIC
struct sve_reginfo {
/* SVE */
uint16_t vl; /* current VL */
@@ -29,7 +28,6 @@ struct sve_reginfo {
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. */
@@ -50,9 +48,7 @@ struct reginfo {
union {
struct simd_reginfo simd;
-#ifdef SVE_MAGIC
struct sve_reginfo sve;
-#endif
};
};
diff --git a/risu_reginfo_aarch64.c b/risu_reginfo_aarch64.c
index 7044648..16a57ba 100644
--- a/risu_reginfo_aarch64.c
+++ b/risu_reginfo_aarch64.c
@@ -24,11 +24,6 @@
#include "risu.h"
#include "risu_reginfo_aarch64.h"
-#ifndef SVE_MAGIC
-const struct option * const arch_long_opts;
-const char * const arch_extra_help;
-#else
-
/* Should we test SVE register state */
static int test_sve;
static const struct option extra_opts[] = {
@@ -39,11 +34,9 @@ static const struct option extra_opts[] = {
const struct option * const arch_long_opts = &extra_opts[0];
const char * const arch_extra_help
= " --test-sve=<vq> Compare SVE registers with VQ\n";
-#endif
void process_arch_opt(int opt, const char *arg)
{
-#ifdef SVE_MAGIC
long want, got;
assert(opt == FIRST_ARCH_OPT);
@@ -64,19 +57,14 @@ void process_arch_opt(int opt, const char *arg)
}
exit(EXIT_FAILURE);
}
-#else
- abort();
-#endif
}
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);
}
-#endif
return size;
}
@@ -86,9 +74,7 @@ void reginfo_init(struct reginfo *ri, ucontext_t *uc)
int i;
struct _aarch64_ctx *ctx, *extra = NULL;
struct fpsimd_context *fp = NULL;
-#ifdef SVE_MAGIC
struct sve_context *sve = NULL;
-#endif
/* necessary to be able to compare with memcmp later */
memset(ri, 0, sizeof(*ri));
@@ -110,14 +96,12 @@ void reginfo_init(struct reginfo *ri, ucontext_t *uc)
case FPSIMD_MAGIC:
fp = (void *)ctx;
break;
-#ifdef SVE_MAGIC
case SVE_MAGIC:
sve = (void *)ctx;
break;
case EXTRA_MAGIC:
extra = (void *)((struct extra_context *)(ctx))->datap;
break;
-#endif
case 0:
/* End of list. */
ctx = extra;
@@ -137,7 +121,6 @@ void reginfo_init(struct reginfo *ri, ucontext_t *uc)
ri->fpsr = fp->fpsr;
ri->fpcr = fp->fpcr;
-#ifdef SVE_MAGIC
if (test_sve) {
int vq = test_sve;
@@ -184,7 +167,6 @@ void reginfo_init(struct reginfo *ri, ucontext_t *uc)
return;
}
-#endif /* SVE_MAGIC */
for (i = 0; i < 32; i++) {
ri->simd.vregs[i] = fp->vregs[i];
@@ -197,7 +179,6 @@ int reginfo_is_eq(struct reginfo *r1, struct reginfo *r2)
return memcmp(r1, r2, reginfo_size(r1)) == 0;
}
-#ifdef SVE_MAGIC
static int sve_zreg_is_eq(int vq, const void *z1, const void *z2)
{
return memcmp(z1, z2, vq * 16) == 0;
@@ -241,7 +222,6 @@ static void sve_dump_zreg_diff(FILE *f, int vq, const __uint128_t *z1,
}
}
}
-#endif
/* reginfo_dump: print state to a stream, returns nonzero on success */
int reginfo_dump(struct reginfo *ri, FILE * f)
@@ -259,7 +239,6 @@ int reginfo_dump(struct reginfo *ri, FILE * f)
fprintf(f, " fpsr : %08x\n", ri->fpsr);
fprintf(f, " fpcr : %08x\n", ri->fpcr);
-#ifdef SVE_MAGIC
if (test_sve) {
int q, vq = test_sve;
@@ -287,7 +266,6 @@ int reginfo_dump(struct reginfo *ri, FILE * f)
return !ferror(f);
}
-#endif
for (i = 0; i < 32; i++) {
fprintf(f, " V%-2d : %016" PRIx64 "%016" PRIx64 "\n", i,
@@ -336,7 +314,6 @@ int reginfo_dump_mismatch(struct reginfo *m, struct reginfo *a, FILE * f)
fprintf(f, " fpcr : %08x vs %08x\n", m->fpcr, a->fpcr);
}
-#ifdef SVE_MAGIC
if (test_sve) {
int vq = sve_vq_from_vl(m->sve.vl);
@@ -365,7 +342,6 @@ int reginfo_dump_mismatch(struct reginfo *m, struct reginfo *a, FILE * f)
return !ferror(f);
}
-#endif
for (i = 0; i < 32; i++) {
if (m->simd.vregs[i] != a->simd.vregs[i]) {
--
2.34.1
^ permalink raw reply related [flat|nested] 47+ messages in thread* [RISU PATCH v4 20/29] aarch64: Reorg sve reginfo to save space
2022-07-08 15:46 [RISU PATCH v4 00/29] risu cleanups and improvements Richard Henderson
` (18 preceding siblings ...)
2022-07-08 15:46 ` [RISU PATCH v4 19/29] aarch64: Assume system support for SVE Richard Henderson
@ 2022-07-08 15:46 ` Richard Henderson
2022-07-18 11:40 ` Peter Maydell
2022-07-08 15:46 ` [RISU PATCH v4 21/29] aarch64: Use arch_init to configure sve Richard Henderson
` (9 subsequent siblings)
29 siblings, 1 reply; 47+ messages in thread
From: Richard Henderson @ 2022-07-08 15:46 UTC (permalink / raw)
To: qemu-devel; +Cc: qemu-arm
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 | 45 ++++++-----
risu_reginfo_aarch64.c | 171 ++++++++++++++++++++---------------------
2 files changed, 108 insertions(+), 108 deletions(-)
diff --git a/risu_reginfo_aarch64.h b/risu_reginfo_aarch64.h
index efbca56..536c12b 100644
--- a/risu_reginfo_aarch64.h
+++ b/risu_reginfo_aarch64.h
@@ -13,27 +13,17 @@
#ifndef RISU_REGINFO_AARCH64_H
#define RISU_REGINFO_AARCH64_H
-#include <signal.h> /* for SVE_MAGIC */
-
-struct simd_reginfo {
- __uint128_t vregs[32];
- char end[0];
-};
-
-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];
-};
+#include <signal.h>
/* The kernel headers set this based on future arch extensions.
The current arch maximum is 16. Save space below. */
#undef SVE_VQ_MAX
#define SVE_VQ_MAX 16
+#define ROUND_UP(SIZE, POW2) (((SIZE) + (POW2) - 1) & -(POW2))
+#define RISU_SVE_REGS_SIZE(VQ) ROUND_UP(SVE_SIG_REGS_SIZE(VQ), 16)
+#define RISU_SIMD_REGS_SIZE (32 * 16)
+
struct reginfo {
uint64_t fault_address;
uint64_t regs[31];
@@ -45,11 +35,28 @@ struct reginfo {
/* FP/SIMD */
uint32_t fpsr;
uint32_t fpcr;
+ uint16_t sve_vl;
+ uint16_t reserved;
- union {
- struct simd_reginfo simd;
- struct sve_reginfo sve;
- };
+ char extra[RISU_SVE_REGS_SIZE(SVE_VQ_MAX)]
+ __attribute__((aligned(16)));
};
+static inline uint64_t *reginfo_vreg(struct reginfo *ri, int i)
+{
+ return (uint64_t *)&ri->extra[i * 16];
+}
+
+static inline uint64_t *reginfo_zreg(struct reginfo *ri, int vq, int i)
+{
+ return (uint64_t *)&ri->extra[SVE_SIG_ZREG_OFFSET(vq, i) -
+ SVE_SIG_REGS_OFFSET];
+}
+
+static inline uint16_t *reginfo_preg(struct reginfo *ri, int vq, int i)
+{
+ return (uint16_t *)&ri->extra[SVE_SIG_PREG_OFFSET(vq, i) -
+ SVE_SIG_REGS_OFFSET];
+}
+
#endif /* RISU_REGINFO_AARCH64_H */
diff --git a/risu_reginfo_aarch64.c b/risu_reginfo_aarch64.c
index 16a57ba..81a77ba 100644
--- a/risu_reginfo_aarch64.c
+++ b/risu_reginfo_aarch64.c
@@ -61,9 +61,13 @@ void process_arch_opt(int opt, const char *arg)
int reginfo_size(struct reginfo *ri)
{
- int size = offsetof(struct reginfo, simd.end);
- if (test_sve) {
- size = offsetof(struct reginfo, sve.end);
+ int size = offsetof(struct reginfo, extra);
+
+ if (ri->sve_vl) {
+ int vq = sve_vq_from_vl(ri->sve_vl);
+ size += RISU_SVE_REGS_SIZE(vq);
+ } else {
+ size += RISU_SIMD_REGS_SIZE;
}
return size;
}
@@ -128,6 +132,7 @@ void reginfo_init(struct reginfo *ri, ucontext_t *uc)
fprintf(stderr, "risu_reginfo_aarch64: failed to get SVE state\n");
return;
}
+
if (sve->vl != sve_vl_from_vq(vq)) {
fprintf(stderr, "risu_reginfo_aarch64: "
"unexpected SVE state: %d != %d\n",
@@ -135,42 +140,22 @@ 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. */
- } else {
- fprintf(stderr, "risu_reginfo_aarch64: "
- "failed to get complete SVE state\n");
- }
+ if (sve->head.size <= SVE_SIG_CONTEXT_SIZE(0)) {
+ /* Only AdvSIMD state is present. */
+ } else if (sve->head.size < SVE_SIG_CONTEXT_SIZE(vq)) {
+ fprintf(stderr, "risu_reginfo_aarch64: "
+ "failed to get complete SVE state\n");
+ return;
+ } else {
+ ri->sve_vl = sve->vl;
+ memcpy(reginfo_zreg(ri, vq, 0),
+ (char *)sve + SVE_SIG_REGS_OFFSET,
+ SVE_SIG_REGS_SIZE(vq));
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));
-
- return;
}
- for (i = 0; i < 32; i++) {
- ri->simd.vregs[i] = fp->vregs[i];
- }
+ memcpy(reginfo_vreg(ri, 0), fp->vregs, RISU_SIMD_REGS_SIZE);
}
/* reginfo_is_eq: compare the reginfo structs, returns nonzero if equal */
@@ -206,18 +191,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 = " ";
}
}
@@ -239,38 +226,41 @@ int reginfo_dump(struct reginfo *ri, FILE * f)
fprintf(f, " fpsr : %08x\n", ri->fpsr);
fprintf(f, " fpcr : %08x\n", ri->fpcr);
- 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);
+ 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);
}
for (i = 0; i < 32; i++) {
- fprintf(f, " V%-2d : %016" PRIx64 "%016" PRIx64 "\n", i,
- (uint64_t) (ri->simd.vregs[i] >> 64),
- (uint64_t) (ri->simd.vregs[i]));
+ uint64_t *v = reginfo_vreg(ri, i);
+ fprintf(f, " V%-2d : %016" PRIx64 "%016" PRIx64 "\n",
+ i, v[1], v[0]);
}
return !ferror(f);
@@ -314,44 +304,47 @@ int reginfo_dump_mismatch(struct reginfo *m, struct reginfo *a, FILE * f)
fprintf(f, " fpcr : %08x vs %08x\n", m->fpcr, a->fpcr);
}
- if (test_sve) {
- 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);
- }
+ if (m->sve_vl) {
+ int vq = sve_vq_from_vl(m->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);
}
for (i = 0; i < 32; i++) {
- if (m->simd.vregs[i] != a->simd.vregs[i]) {
+ uint64_t *mv = reginfo_vreg(m, i);
+ uint64_t *av = reginfo_vreg(a, i);
+
+ if (mv[0] != av[0] || mv[1] != av[1]) {
fprintf(f, " V%-2d : "
"%016" PRIx64 "%016" PRIx64 " vs "
- "%016" PRIx64 "%016" PRIx64 "\n", i,
- (uint64_t) (m->simd.vregs[i] >> 64),
- (uint64_t) m->simd.vregs[i],
- (uint64_t) (a->simd.vregs[i] >> 64),
- (uint64_t) a->simd.vregs[i]);
+ "%016" PRIx64 "%016" PRIx64 "\n",
+ i, mv[1], mv[0], av[1], av[0]);
}
}
--
2.34.1
^ permalink raw reply related [flat|nested] 47+ messages in thread* Re: [RISU PATCH v4 20/29] aarch64: Reorg sve reginfo to save space
2022-07-08 15:46 ` [RISU PATCH v4 20/29] aarch64: Reorg sve reginfo to save space Richard Henderson
@ 2022-07-18 11:40 ` Peter Maydell
0 siblings, 0 replies; 47+ messages in thread
From: Peter Maydell @ 2022-07-18 11:40 UTC (permalink / raw)
To: Richard Henderson; +Cc: qemu-devel, qemu-arm
On Fri, 8 Jul 2022 at 17:38, Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> 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 | 45 ++++++-----
> risu_reginfo_aarch64.c | 171 ++++++++++++++++++++---------------------
> 2 files changed, 108 insertions(+), 108 deletions(-)
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
thanks
-- PMM
^ permalink raw reply [flat|nested] 47+ messages in thread
* [RISU PATCH v4 21/29] aarch64: Use arch_init to configure sve
2022-07-08 15:46 [RISU PATCH v4 00/29] risu cleanups and improvements Richard Henderson
` (19 preceding siblings ...)
2022-07-08 15:46 ` [RISU PATCH v4 20/29] aarch64: Reorg sve reginfo to save space Richard Henderson
@ 2022-07-08 15:46 ` Richard Henderson
2022-07-18 11:44 ` Peter Maydell
2022-07-08 15:46 ` [RISU PATCH v4 22/29] ppc64: Use uint64_t to represent double Richard Henderson
` (8 subsequent siblings)
29 siblings, 1 reply; 47+ messages in thread
From: Richard Henderson @ 2022-07-08 15:46 UTC (permalink / raw)
To: qemu-devel; +Cc: qemu-arm
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.
Diagnose EINVAL as either cpu or kernel does not support SVE.
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
risu.h | 1 +
risu.c | 3 +++
risu_reginfo_aarch64.c | 31 ++++++++++++++++++++-----------
risu_reginfo_arm.c | 4 ++++
risu_reginfo_i386.c | 4 ++++
risu_reginfo_m68k.c | 4 ++++
risu_reginfo_ppc64.c | 4 ++++
7 files changed, 40 insertions(+), 11 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 81a77ba..be47980 100644
--- a/risu_reginfo_aarch64.c
+++ b/risu_reginfo_aarch64.c
@@ -19,6 +19,7 @@
#include <stdbool.h>
#include <inttypes.h>
#include <assert.h>
+#include <errno.h>
#include <sys/prctl.h>
#include "risu.h"
@@ -37,8 +38,6 @@ const char * const arch_extra_help
void process_arch_opt(int opt, const char *arg)
{
- long want, got;
-
assert(opt == FIRST_ARCH_OPT);
test_sve = strtol(arg, 0, 10);
@@ -46,16 +45,26 @@ 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));
+}
+
+void arch_init(void)
+{
+ long want, got;
+
+ if (test_sve) {
+ want = sve_vl_from_vq(test_sve);
+ got = prctl(PR_SVE_SET_VL, want);
+ if (want != got) {
+ if (got >= 0) {
+ fprintf(stderr, "Unsupported VQ for SVE (%d != %d)\n",
+ test_sve, (int)sve_vq_from_vl(got));
+ } else if (errno == EINVAL) {
+ fprintf(stderr, "System does not support SVE\n");
+ } else {
+ perror("prctl PR_SVE_SET_VL");
+ }
+ exit(EXIT_FAILURE);
}
- exit(EXIT_FAILURE);
}
}
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.34.1
^ permalink raw reply related [flat|nested] 47+ messages in thread* Re: [RISU PATCH v4 21/29] aarch64: Use arch_init to configure sve
2022-07-08 15:46 ` [RISU PATCH v4 21/29] aarch64: Use arch_init to configure sve Richard Henderson
@ 2022-07-18 11:44 ` Peter Maydell
0 siblings, 0 replies; 47+ messages in thread
From: Peter Maydell @ 2022-07-18 11:44 UTC (permalink / raw)
To: Richard Henderson; +Cc: qemu-devel, qemu-arm
On Fri, 8 Jul 2022 at 17:41, Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> 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.
>
> Diagnose EINVAL as either cpu or kernel does not support SVE.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
thanks
-- PMM
^ permalink raw reply [flat|nested] 47+ messages in thread
* [RISU PATCH v4 22/29] ppc64: Use uint64_t to represent double
2022-07-08 15:46 [RISU PATCH v4 00/29] risu cleanups and improvements Richard Henderson
` (20 preceding siblings ...)
2022-07-08 15:46 ` [RISU PATCH v4 21/29] aarch64: Use arch_init to configure sve Richard Henderson
@ 2022-07-08 15:46 ` Richard Henderson
2022-07-12 11:17 ` Matheus K. Ferst
2022-07-08 15:46 ` [RISU PATCH v4 23/29] Standardize reginfo_dump_mismatch printing Richard Henderson
` (7 subsequent siblings)
29 siblings, 1 reply; 47+ messages in thread
From: Richard Henderson @ 2022-07-08 15:46 UTC (permalink / raw)
To: qemu-devel; +Cc: qemu-arm
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.34.1
^ permalink raw reply related [flat|nested] 47+ messages in thread* [RISU PATCH v4 23/29] Standardize reginfo_dump_mismatch printing
2022-07-08 15:46 [RISU PATCH v4 00/29] risu cleanups and improvements Richard Henderson
` (21 preceding siblings ...)
2022-07-08 15:46 ` [RISU PATCH v4 22/29] ppc64: Use uint64_t to represent double Richard Henderson
@ 2022-07-08 15:46 ` Richard Henderson
2022-07-18 11:54 ` Peter Maydell
2022-07-08 15:46 ` [RISU PATCH v4 24/29] Add --fulldump and --diffdup options Richard Henderson
` (6 subsequent siblings)
29 siblings, 1 reply; 47+ messages in thread
From: Richard Henderson @ 2022-07-08 15:46 UTC (permalink / raw)
To: qemu-devel; +Cc: qemu-arm
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 be47980..e0f80c0 100644
--- a/risu_reginfo_aarch64.c
+++ b/risu_reginfo_aarch64.c
@@ -275,15 +275,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",
@@ -342,7 +342,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;
}
for (i = 0; i < 32; i++) {
@@ -356,6 +356,4 @@ int reginfo_dump_mismatch(struct reginfo *m, struct reginfo *a, FILE * f)
i, mv[1], mv[0], av[1], av[0]);
}
}
-
- 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.34.1
^ permalink raw reply related [flat|nested] 47+ messages in thread* Re: [RISU PATCH v4 23/29] Standardize reginfo_dump_mismatch printing
2022-07-08 15:46 ` [RISU PATCH v4 23/29] Standardize reginfo_dump_mismatch printing Richard Henderson
@ 2022-07-18 11:54 ` Peter Maydell
0 siblings, 0 replies; 47+ messages in thread
From: Peter Maydell @ 2022-07-18 11:54 UTC (permalink / raw)
To: Richard Henderson; +Cc: qemu-devel, qemu-arm
On Fri, 8 Jul 2022 at 17:39, Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> 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>
> 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]);
> }
This used to skip r1 and r13, and now it does not.
> -
> - 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]);
> }
> }
thanks
-- PMM
^ permalink raw reply [flat|nested] 47+ messages in thread
* [RISU PATCH v4 24/29] Add --fulldump and --diffdup options
2022-07-08 15:46 [RISU PATCH v4 00/29] risu cleanups and improvements Richard Henderson
` (22 preceding siblings ...)
2022-07-08 15:46 ` [RISU PATCH v4 23/29] Standardize reginfo_dump_mismatch printing Richard Henderson
@ 2022-07-08 15:46 ` Richard Henderson
2022-07-18 12:06 ` Peter Maydell
2022-07-08 15:46 ` [RISU PATCH v4 25/29] Remove return value from reginfo_dump Richard Henderson
` (5 subsequent siblings)
29 siblings, 1 reply; 47+ messages in thread
From: Richard Henderson @ 2022-07-08 15:46 UTC (permalink / raw)
To: qemu-devel; +Cc: qemu-arm
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.34.1
^ permalink raw reply related [flat|nested] 47+ messages in thread* [RISU PATCH v4 25/29] Remove return value from reginfo_dump
2022-07-08 15:46 [RISU PATCH v4 00/29] risu cleanups and improvements Richard Henderson
` (23 preceding siblings ...)
2022-07-08 15:46 ` [RISU PATCH v4 24/29] Add --fulldump and --diffdup options Richard Henderson
@ 2022-07-08 15:46 ` Richard Henderson
2022-07-18 11:55 ` Peter Maydell
2022-07-08 15:46 ` [RISU PATCH v4 26/29] ppc64: Clean up reginfo handling Richard Henderson
` (4 subsequent siblings)
29 siblings, 1 reply; 47+ messages in thread
From: Richard Henderson @ 2022-07-08 15:46 UTC (permalink / raw)
To: qemu-devel; +Cc: qemu-arm
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 e0f80c0..b86864d 100644
--- a/risu_reginfo_aarch64.c
+++ b/risu_reginfo_aarch64.c
@@ -219,8 +219,8 @@ static void sve_dump_zreg_diff(FILE *f, int vq, const uint64_t *za,
}
}
-/* 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);
@@ -263,7 +263,7 @@ int reginfo_dump(struct reginfo *ri, FILE * f)
sve_dump_preg(f, vq, p);
fprintf(f, "\n");
}
- return !ferror(f);
+ return;
}
for (i = 0; i < 32; i++) {
@@ -271,8 +271,6 @@ int reginfo_dump(struct reginfo *ri, FILE * f)
fprintf(f, " V%-2d : %016" PRIx64 "%016" PRIx64 "\n",
i, v[1], v[0]);
}
-
- 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.34.1
^ permalink raw reply related [flat|nested] 47+ messages in thread* [RISU PATCH v4 26/29] ppc64: Clean up reginfo handling
2022-07-08 15:46 [RISU PATCH v4 00/29] risu cleanups and improvements Richard Henderson
` (24 preceding siblings ...)
2022-07-08 15:46 ` [RISU PATCH v4 25/29] Remove return value from reginfo_dump Richard Henderson
@ 2022-07-08 15:46 ` Richard Henderson
2022-07-12 11:17 ` Matheus K. Ferst
2022-07-08 15:46 ` [RISU PATCH v4 27/29] aarch64: Tidy reginfo dumping ahead of ZA state Richard Henderson
` (3 subsequent siblings)
29 siblings, 1 reply; 47+ messages in thread
From: Richard Henderson @ 2022-07-08 15:46 UTC (permalink / raw)
To: qemu-devel; +Cc: qemu-arm
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..eeb0460 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)) == 0;
}
/* 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.34.1
^ permalink raw reply related [flat|nested] 47+ messages in thread* Re: [RISU PATCH v4 26/29] ppc64: Clean up reginfo handling
2022-07-08 15:46 ` [RISU PATCH v4 26/29] ppc64: Clean up reginfo handling Richard Henderson
@ 2022-07-12 11:17 ` Matheus K. Ferst
0 siblings, 0 replies; 47+ messages in thread
From: Matheus K. Ferst @ 2022-07-12 11:17 UTC (permalink / raw)
To: Richard Henderson, qemu-devel; +Cc: qemu-arm
On 08/07/2022 12:46, Richard Henderson wrote:
> 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..eeb0460 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",
> };
>
NIP value depends on image_start address, and LNK will be the return
address, so I guess we shouldn't compare them. Some MSR bits are defined
from start under linux-user (e.g. FE0 and FE1), but I think we can work
around that.
CTR will likely be image_start since calling a function pointer is
usually implemented with mtctr/bctr, but risugen doesn't initialize it,
so the comparison will fail at the first checkpoint. VRSAVE is also not
initialized, so I guess we need something like
> write_mov_ri(0, rand(0xffffffff));
> insn32(0x7c0903a6); # mtctr r0
>
> write_mov_ri(0, rand(0xffffffff));
> insn32(0x7c0043a6); # mtvrsave r0
in write_random_regdata
> 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)) == 0;
> }
>
> /* 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.34.1
>
>
Thanks,
Matheus K. Ferst
Instituto de Pesquisas ELDORADO <http://www.eldorado.org.br/>
Analista de Software
Aviso Legal - Disclaimer <https://www.eldorado.org.br/disclaimer.html>
^ permalink raw reply [flat|nested] 47+ messages in thread
* [RISU PATCH v4 27/29] aarch64: Tidy reginfo dumping ahead of ZA state
2022-07-08 15:46 [RISU PATCH v4 00/29] risu cleanups and improvements Richard Henderson
` (25 preceding siblings ...)
2022-07-08 15:46 ` [RISU PATCH v4 26/29] ppc64: Clean up reginfo handling Richard Henderson
@ 2022-07-08 15:46 ` Richard Henderson
2022-07-18 12:08 ` Peter Maydell
2022-07-08 15:46 ` [RISU PATCH v4 28/29] aarch64: Add support for ZA storage Richard Henderson
` (2 subsequent siblings)
29 siblings, 1 reply; 47+ messages in thread
From: Richard Henderson @ 2022-07-08 15:46 UTC (permalink / raw)
To: qemu-devel; +Cc: qemu-arm
A misalignment for sve_vl, plus add a bit more space
on the left for the ZA[n] field name.
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
risu_reginfo_aarch64.c | 29 ++++++++++++++++++-----------
1 file changed, 18 insertions(+), 11 deletions(-)
diff --git a/risu_reginfo_aarch64.c b/risu_reginfo_aarch64.c
index b86864d..9ea0acf 100644
--- a/risu_reginfo_aarch64.c
+++ b/risu_reginfo_aarch64.c
@@ -183,6 +183,18 @@ static int sve_preg_is_eq(int vq, const void *p1, const void *p2)
return memcmp(p1, p2, vq * 2) == 0;
}
+static void sve_dump_zreg(FILE *f, int vq, const uint64_t *z)
+{
+ const char *pad = "";
+ int q;
+
+ for (q = 0; q < vq; q++) {
+ fprintf(f, "%s[%-2d] %016" PRIx64 "%016" PRIx64 "\n",
+ pad, q, z[2 * q + 1], z[2 * q]);
+ pad = " "; /* 11 spaces */
+ }
+}
+
static void sve_dump_preg(FILE *f, int vq, const uint16_t *p)
{
int q;
@@ -211,10 +223,10 @@ static void sve_dump_zreg_diff(FILE *f, int vq, const uint64_t *za,
uint64_t zb0 = zb[2 * q], zb1 = zb[2 * q + 1];
if (za0 != zb0 || za1 != zb1) {
- fprintf(f, "%sq%-2d: %016" PRIx64 "%016" PRIx64
+ fprintf(f, "%s[%-2d]: %016" PRIx64 "%016" PRIx64
" vs %016" PRIx64 "%016" PRIx64"\n",
pad, q, za1, za0, zb1, zb0);
- pad = " ";
+ pad = " "; /* 11 spaces */
}
}
}
@@ -237,19 +249,14 @@ void reginfo_dump(struct reginfo *ri, FILE * f)
if (ri->sve_vl) {
int vq = sve_vq_from_vl(ri->sve_vl);
- int q;
fprintf(f, " vl : %d\n", ri->sve_vl);
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, z[q * 2 + 1], z[q * 2]);
- }
+ fprintf(f, " Z%-2d : ", i);
+ sve_dump_zreg(f, vq, z);
}
for (i = 0; i < SVE_NUM_PREGS + 1; i++) {
@@ -312,7 +319,7 @@ void reginfo_dump_mismatch(struct reginfo *m, struct reginfo *a, FILE * f)
}
if (m->sve_vl != a->sve_vl) {
- fprintf(f, " vl : %d vs %d\n", m->sve_vl, a->sve_vl);
+ fprintf(f, " vl : %d vs %d\n", m->sve_vl, a->sve_vl);
}
if (m->sve_vl) {
@@ -323,7 +330,7 @@ void reginfo_dump_mismatch(struct reginfo *m, struct reginfo *a, FILE * f)
uint64_t *za = reginfo_zreg(a, vq, i);
if (!sve_zreg_is_eq(vq, zm, za)) {
- fprintf(f, " Z%-2d ", i);
+ fprintf(f, " Z%-2d : ", i);
sve_dump_zreg_diff(f, vq, zm, za);
}
}
--
2.34.1
^ permalink raw reply related [flat|nested] 47+ messages in thread* [RISU PATCH v4 28/29] aarch64: Add support for ZA storage
2022-07-08 15:46 [RISU PATCH v4 00/29] risu cleanups and improvements Richard Henderson
` (26 preceding siblings ...)
2022-07-08 15:46 ` [RISU PATCH v4 27/29] aarch64: Tidy reginfo dumping ahead of ZA state Richard Henderson
@ 2022-07-08 15:46 ` Richard Henderson
2022-07-18 12:09 ` Peter Maydell
2022-07-08 15:47 ` [RISU PATCH v4 29/29] aarch64: Trivial SME test Richard Henderson
2022-07-18 12:19 ` [RISU PATCH v4 00/29] risu cleanups and improvements Peter Maydell
29 siblings, 1 reply; 47+ messages in thread
From: Richard Henderson @ 2022-07-08 15:46 UTC (permalink / raw)
To: qemu-devel; +Cc: qemu-arm
Require NVL == SVL on startup, to make it easier to manage reginfo.
Most of the time PSTATE.SM would be active with PSTATE.ZA anyway,
for any non-trivial SME testing.
Extend saved storage only when PSTATE.ZA is active.
Use a carefully reserved uint16_t for saving SVCR.
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
risu_reginfo_aarch64.h | 52 ++++++++++++-
risu_reginfo_aarch64.c | 161 ++++++++++++++++++++++++++++++++++++-----
2 files changed, 191 insertions(+), 22 deletions(-)
diff --git a/risu_reginfo_aarch64.h b/risu_reginfo_aarch64.h
index 536c12b..097b7ad 100644
--- a/risu_reginfo_aarch64.h
+++ b/risu_reginfo_aarch64.h
@@ -21,6 +21,43 @@
#define SVE_VQ_MAX 16
#define ROUND_UP(SIZE, POW2) (((SIZE) + (POW2) - 1) & -(POW2))
+
+#ifdef ZA_MAGIC
+/* System headers have all Streaming SVE definitions. */
+typedef struct sve_context risu_sve_context;
+typedef struct za_context risu_za_context;
+#else
+#define ZA_MAGIC 0x54366345
+#define SVE_SIG_FLAG_SM 1
+
+/* System headers missing flags field. */
+typedef struct {
+ struct _aarch64_ctx head;
+ uint16_t vl;
+ uint16_t flags;
+ uint16_t reserved[2];
+} risu_sve_context;
+
+typedef struct {
+ struct _aarch64_ctx head;
+ uint16_t vl;
+ uint16_t reserved[3];
+} risu_za_context;
+
+#define ZA_SIG_REGS_OFFSET \
+ ROUND_UP(sizeof(risu_za_context), SVE_VQ_BYTES)
+
+#define ZA_SIG_REGS_SIZE(vq) \
+ ((vq) * (vq) * SVE_VQ_BYTES * SVE_VQ_BYTES)
+
+#define ZA_SIG_ZAV_OFFSET(vq, n) \
+ (ZA_SIG_REGS_OFFSET + (SVE_SIG_ZREG_SIZE(vq) * n))
+
+#define ZA_SIG_CONTEXT_SIZE(vq) \
+ (ZA_SIG_REGS_OFFSET + ZA_SIG_REGS_SIZE(vq))
+
+#endif /* ZA_MAGIC */
+
#define RISU_SVE_REGS_SIZE(VQ) ROUND_UP(SVE_SIG_REGS_SIZE(VQ), 16)
#define RISU_SIMD_REGS_SIZE (32 * 16)
@@ -36,12 +73,16 @@ struct reginfo {
uint32_t fpsr;
uint32_t fpcr;
uint16_t sve_vl;
- uint16_t reserved;
+ uint16_t svcr;
- char extra[RISU_SVE_REGS_SIZE(SVE_VQ_MAX)]
+ char extra[RISU_SVE_REGS_SIZE(SVE_VQ_MAX) +
+ ZA_SIG_REGS_SIZE(SVE_VQ_MAX)]
__attribute__((aligned(16)));
};
+#define SVCR_SM 1
+#define SVCR_ZA 2
+
static inline uint64_t *reginfo_vreg(struct reginfo *ri, int i)
{
return (uint64_t *)&ri->extra[i * 16];
@@ -59,4 +100,11 @@ static inline uint16_t *reginfo_preg(struct reginfo *ri, int vq, int i)
SVE_SIG_REGS_OFFSET];
}
+static inline uint64_t *reginfo_zav(struct reginfo *ri, int vq, int i)
+{
+ return (uint64_t *)&ri->extra[RISU_SVE_REGS_SIZE(vq) +
+ ZA_SIG_ZAV_OFFSET(vq, i) -
+ ZA_SIG_REGS_OFFSET];
+}
+
#endif /* RISU_REGINFO_AARCH64_H */
diff --git a/risu_reginfo_aarch64.c b/risu_reginfo_aarch64.c
index 9ea0acf..af620f3 100644
--- a/risu_reginfo_aarch64.c
+++ b/risu_reginfo_aarch64.c
@@ -25,25 +25,44 @@
#include "risu.h"
#include "risu_reginfo_aarch64.h"
+#ifndef PR_SME_SET_VL
+#define PR_SME_SET_VL 63
+#endif
+
/* Should we test SVE register state */
static int test_sve;
+static int test_za;
static const struct option extra_opts[] = {
{"test-sve", required_argument, NULL, FIRST_ARCH_OPT },
+ {"test-za", required_argument, NULL, FIRST_ARCH_OPT + 1 },
{0, 0, 0, 0}
};
const struct option * const arch_long_opts = &extra_opts[0];
const char * const arch_extra_help
- = " --test-sve=<vq> Compare SVE registers with VQ\n";
+ = " --test-sve=<vq> Compare SVE registers with VQ\n"
+ " --test-za=<vq> Compare ZA storage with VQ\n";
void process_arch_opt(int opt, const char *arg)
{
- assert(opt == FIRST_ARCH_OPT);
- test_sve = strtol(arg, 0, 10);
-
- if (test_sve <= 0 || test_sve > SVE_VQ_MAX) {
- fprintf(stderr, "Invalid value for VQ (1-%d)\n", SVE_VQ_MAX);
- exit(EXIT_FAILURE);
+ switch (opt) {
+ case FIRST_ARCH_OPT:
+ test_sve = strtol(arg, 0, 10);
+ if (test_sve <= 0 || test_sve > SVE_VQ_MAX) {
+ fprintf(stderr, "Invalid value for SVE VQ (1-%d)\n", SVE_VQ_MAX);
+ exit(EXIT_FAILURE);
+ }
+ break;
+ case FIRST_ARCH_OPT + 1:
+ test_za = strtol(arg, 0, 10);
+ if (test_za <= 0 || test_za > SVE_VQ_MAX
+ || (test_za & (test_za - 1))) {
+ fprintf(stderr, "Invalid value for ZA VQ (1-%d)\n", SVE_VQ_MAX);
+ exit(EXIT_FAILURE);
+ }
+ break;
+ default:
+ abort();
}
}
@@ -51,6 +70,31 @@ void arch_init(void)
{
long want, got;
+ if (test_za) {
+ /*
+ * For now, reginfo requires NVL == SVL.
+ * There doesn't seem to be much advantage to differing.
+ */
+ if (test_sve && test_sve != test_za) {
+ fprintf(stderr, "Mismatched values for SVE and ZA VQ\n");
+ exit(EXIT_FAILURE);
+ }
+
+ want = sve_vl_from_vq(test_za);
+ got = prctl(PR_SME_SET_VL, want);
+ if (want != got) {
+ if (got >= 0) {
+ fprintf(stderr, "Unsupported VQ for ZA (%d != %d)\n",
+ test_za, (int)sve_vq_from_vl(got));
+ } else if (errno == EINVAL) {
+ fprintf(stderr, "System does not support SME\n");
+ } else {
+ perror("prctl PR_SME_SET_VL");
+ }
+ exit(EXIT_FAILURE);
+ }
+ }
+
if (test_sve) {
want = sve_vl_from_vq(test_sve);
got = prctl(PR_SVE_SET_VL, want);
@@ -75,6 +119,9 @@ int reginfo_size(struct reginfo *ri)
if (ri->sve_vl) {
int vq = sve_vq_from_vl(ri->sve_vl);
size += RISU_SVE_REGS_SIZE(vq);
+ if (ri->svcr & SVCR_ZA) {
+ size += ZA_SIG_REGS_SIZE(vq);
+ }
} else {
size += RISU_SIMD_REGS_SIZE;
}
@@ -84,10 +131,11 @@ int reginfo_size(struct reginfo *ri)
/* reginfo_init: initialize with a ucontext */
void reginfo_init(struct reginfo *ri, ucontext_t *uc)
{
- int i;
+ int i, vq;
struct _aarch64_ctx *ctx, *extra = NULL;
struct fpsimd_context *fp = NULL;
- struct sve_context *sve = NULL;
+ risu_sve_context *sve = NULL;
+ risu_za_context *za = NULL;
/* necessary to be able to compare with memcmp later */
memset(ri, 0, sizeof(*ri));
@@ -112,6 +160,9 @@ void reginfo_init(struct reginfo *ri, ucontext_t *uc)
case SVE_MAGIC:
sve = (void *)ctx;
break;
+ case ZA_MAGIC:
+ za = (void *)ctx;
+ break;
case EXTRA_MAGIC:
extra = (void *)((struct extra_context *)(ctx))->datap;
break;
@@ -134,21 +185,55 @@ void reginfo_init(struct reginfo *ri, ucontext_t *uc)
ri->fpsr = fp->fpsr;
ri->fpcr = fp->fpcr;
- if (test_sve) {
- int vq = test_sve;
+ /*
+ * Note that arch_init required NVL==SVL, so test_sve and test_za
+ * are equal when non-zero. We will verify this matches below.
+ */
+ vq = test_sve | test_za;
+ ri->sve_vl = sve_vl_from_vq(vq);
- if (sve == NULL) {
- fprintf(stderr, "risu_reginfo_aarch64: failed to get SVE state\n");
+ if (test_za) {
+ if (za == NULL) {
+ /* ZA_MAGIC is supposed to be present, even if empty. */
+ fprintf(stderr, "risu_reginfo_aarch64: missing ZA state\n");
return;
}
+ assert(za->head.size >= ZA_SIG_CONTEXT_SIZE(0));
- if (sve->vl != sve_vl_from_vq(vq)) {
+ if (za->vl != ri->sve_vl) {
fprintf(stderr, "risu_reginfo_aarch64: "
- "unexpected SVE state: %d != %d\n",
- sve->vl, sve_vl_from_vq(vq));
+ "unexpected ZA VQ: %d != %d\n",
+ za->vl, ri->sve_vl);
+ return;
+ }
+ if (za->head.size == ZA_SIG_CONTEXT_SIZE(0)) {
+ /* ZA storage is disabled. */
+ } else if (za->head.size < ZA_SIG_CONTEXT_SIZE(vq)) {
+ fprintf(stderr, "risu_reginfo_aarch64: "
+ "failed to get complete ZA state\n");
+ return;
+ } else {
+ ri->svcr |= SVCR_ZA;
+ memcpy(reginfo_zav(ri, vq, 0), (char *)za + ZA_SIG_REGS_OFFSET,
+ ZA_SIG_CONTEXT_SIZE(vq) - ZA_SIG_REGS_OFFSET);
+ }
+ }
+
+ if (test_sve) {
+ if (sve == NULL) {
+ /* SVE_MAGIC is supposed to be present, even if empty. */
+ fprintf(stderr, "risu_reginfo_aarch64: missing SVE state\n");
return;
}
+ if (sve->vl != ri->sve_vl) {
+ fprintf(stderr, "risu_reginfo_aarch64: "
+ "unexpected SVE VQ: %d != %d\n",
+ sve->vl, ri->sve_vl);
+ return;
+ }
+
+ ri->svcr |= sve->flags & SVE_SIG_FLAG_SM;
if (sve->head.size <= SVE_SIG_CONTEXT_SIZE(0)) {
/* Only AdvSIMD state is present. */
} else if (sve->head.size < SVE_SIG_CONTEXT_SIZE(vq)) {
@@ -156,7 +241,6 @@ void reginfo_init(struct reginfo *ri, ucontext_t *uc)
"failed to get complete SVE state\n");
return;
} else {
- ri->sve_vl = sve->vl;
memcpy(reginfo_zreg(ri, vq, 0),
(char *)sve + SVE_SIG_REGS_OFFSET,
SVE_SIG_REGS_SIZE(vq));
@@ -164,7 +248,18 @@ void reginfo_init(struct reginfo *ri, ucontext_t *uc)
}
}
- memcpy(reginfo_vreg(ri, 0), fp->vregs, RISU_SIMD_REGS_SIZE);
+ /*
+ * Be prepared for ZA state present but SVE state absent (VQ != 0).
+ * In which case, copy AdvSIMD vregs into the low portion of zregs;
+ * pregs remain all zero.
+ */
+ if (vq == 0) {
+ memcpy(reginfo_vreg(ri, 0), fp->vregs, RISU_SIMD_REGS_SIZE);
+ } else {
+ for (i = 0; i < 32; ++i) {
+ memcpy(reginfo_zreg(ri, vq, i), &fp->vregs[i], 16);
+ }
+ }
}
/* reginfo_is_eq: compare the reginfo structs, returns nonzero if equal */
@@ -248,9 +343,11 @@ void reginfo_dump(struct reginfo *ri, FILE * f)
fprintf(f, " fpcr : %08x\n", ri->fpcr);
if (ri->sve_vl) {
- int vq = sve_vq_from_vl(ri->sve_vl);
+ int vl = ri->sve_vl;
+ int vq = sve_vq_from_vl(vl);
- fprintf(f, " vl : %d\n", ri->sve_vl);
+ fprintf(f, " vl : %d\n", vl);
+ fprintf(f, " svcr : %d\n", ri->svcr);
for (i = 0; i < SVE_NUM_ZREGS; i++) {
uint64_t *z = reginfo_zreg(ri, vq, i);
@@ -270,6 +367,14 @@ void reginfo_dump(struct reginfo *ri, FILE * f)
sve_dump_preg(f, vq, p);
fprintf(f, "\n");
}
+
+ if (ri->svcr & SVCR_ZA) {
+ for (i = 0; i < vl; ++i) {
+ uint64_t *z = reginfo_zav(ri, vq, i);
+ fprintf(f, " ZA[%-3d]: ", i);
+ sve_dump_zreg(f, vq, z);
+ }
+ }
return;
}
@@ -322,6 +427,10 @@ void reginfo_dump_mismatch(struct reginfo *m, struct reginfo *a, FILE * f)
fprintf(f, " vl : %d vs %d\n", m->sve_vl, a->sve_vl);
}
+ if (m->svcr != a->svcr) {
+ fprintf(f, " svcr : %d vs %d\n", m->svcr, a->svcr);
+ }
+
if (m->sve_vl) {
int vq = sve_vq_from_vl(m->sve_vl);
@@ -347,6 +456,18 @@ void reginfo_dump_mismatch(struct reginfo *m, struct reginfo *a, FILE * f)
sve_dump_preg_diff(f, vq, pm, pa);
}
}
+
+ if (m->svcr & a->svcr & SVCR_ZA) {
+ for (i = 0; i < vq * 16; i++) {
+ uint64_t *zm = reginfo_zav(m, vq, i);
+ uint64_t *za = reginfo_zav(a, vq, i);
+
+ if (!sve_zreg_is_eq(vq, zm, za)) {
+ fprintf(f, " ZA[%-3d]: ", i);
+ sve_dump_zreg_diff(f, vq, zm, za);
+ }
+ }
+ }
return;
}
--
2.34.1
^ permalink raw reply related [flat|nested] 47+ messages in thread* Re: [RISU PATCH v4 28/29] aarch64: Add support for ZA storage
2022-07-08 15:46 ` [RISU PATCH v4 28/29] aarch64: Add support for ZA storage Richard Henderson
@ 2022-07-18 12:09 ` Peter Maydell
0 siblings, 0 replies; 47+ messages in thread
From: Peter Maydell @ 2022-07-18 12:09 UTC (permalink / raw)
To: Richard Henderson; +Cc: qemu-devel, qemu-arm
On Fri, 8 Jul 2022 at 18:07, Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> Require NVL == SVL on startup, to make it easier to manage reginfo.
> Most of the time PSTATE.SM would be active with PSTATE.ZA anyway,
> for any non-trivial SME testing.
>
> Extend saved storage only when PSTATE.ZA is active.
> Use a carefully reserved uint16_t for saving SVCR.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
thanks
-- PMM
^ permalink raw reply [flat|nested] 47+ messages in thread
* [RISU PATCH v4 29/29] aarch64: Trivial SME test
2022-07-08 15:46 [RISU PATCH v4 00/29] risu cleanups and improvements Richard Henderson
` (27 preceding siblings ...)
2022-07-08 15:46 ` [RISU PATCH v4 28/29] aarch64: Add support for ZA storage Richard Henderson
@ 2022-07-08 15:47 ` Richard Henderson
2022-07-18 12:11 ` Peter Maydell
2022-07-18 12:19 ` [RISU PATCH v4 00/29] risu cleanups and improvements Peter Maydell
29 siblings, 1 reply; 47+ messages in thread
From: Richard Henderson @ 2022-07-08 15:47 UTC (permalink / raw)
To: qemu-devel; +Cc: qemu-arm
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
test_sme_aarch64.s | 55 ++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 55 insertions(+)
create mode 100644 test_sme_aarch64.s
diff --git a/test_sme_aarch64.s b/test_sme_aarch64.s
new file mode 100644
index 0000000..a5ef909
--- /dev/null
+++ b/test_sme_aarch64.s
@@ -0,0 +1,55 @@
+ .arch_extension sme
+
+ mov w0, #0
+ mov w1, #0
+ mov w2, #0
+ mov w3, #0
+ mov w4, #0
+ mov w5, #0
+ mov w6, #0
+ mov w7, #0
+ mov w8, #0
+ mov w9, #0
+ mov w10, #0
+ mov w11, #0
+ mov w12, #0
+ mov w13, #0
+ mov w14, #0
+ mov w15, #0
+ mov w16, #0
+ mov w17, #0
+ mov w18, #0
+ mov w19, #0
+ mov w20, #0
+ mov w21, #0
+ mov w22, #0
+ mov w23, #0
+ mov w24, #0
+ mov w25, #0
+ mov w26, #0
+ mov w27, #0
+ mov w28, #0
+ mov w29, #0
+ mov w30, #0
+
+ smstart
+
+ ptrue p0.b
+ rdsvl x12, #1
+
+0: subs w12, w12, #1
+ lsl w13, w12, #4
+ index z0.b, w13, #1
+ mova za0h.b[w12, #0], p0/m, z0.b
+ b.ne 0b
+
+ .inst 0x00005af0 /* compare */
+
+ rdsvl x12, #1
+0: subs w12, w12, #1
+ lsl w13, w12, #4
+ index z0.b, w13, #1
+ mova za0v.b[w12, #0], p0/m, z0.b
+ b.ne 0b
+
+ .inst 0x00005af1 /* exit */
--
2.34.1
^ permalink raw reply related [flat|nested] 47+ messages in thread* Re: [RISU PATCH v4 29/29] aarch64: Trivial SME test
2022-07-08 15:47 ` [RISU PATCH v4 29/29] aarch64: Trivial SME test Richard Henderson
@ 2022-07-18 12:11 ` Peter Maydell
0 siblings, 0 replies; 47+ messages in thread
From: Peter Maydell @ 2022-07-18 12:11 UTC (permalink / raw)
To: Richard Henderson; +Cc: qemu-devel, qemu-arm
On Fri, 8 Jul 2022 at 17:59, Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
> test_sme_aarch64.s | 55 ++++++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 55 insertions(+)
> create mode 100644 test_sme_aarch64.s
>
> diff --git a/test_sme_aarch64.s b/test_sme_aarch64.s
> new file mode 100644
> index 0000000..a5ef909
> --- /dev/null
> +++ b/test_sme_aarch64.s
> @@ -0,0 +1,55 @@
> + .arch_extension sme
Can we have the usual copyright-and-license comment at the
top of the file, please?
thanks
-- PMM
^ permalink raw reply [flat|nested] 47+ messages in thread
* Re: [RISU PATCH v4 00/29] risu cleanups and improvements
2022-07-08 15:46 [RISU PATCH v4 00/29] risu cleanups and improvements Richard Henderson
` (28 preceding siblings ...)
2022-07-08 15:47 ` [RISU PATCH v4 29/29] aarch64: Trivial SME test Richard Henderson
@ 2022-07-18 12:19 ` Peter Maydell
29 siblings, 0 replies; 47+ messages in thread
From: Peter Maydell @ 2022-07-18 12:19 UTC (permalink / raw)
To: Richard Henderson; +Cc: qemu-devel, qemu-arm
On Fri, 8 Jul 2022 at 16:49, Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> If you can imagine, v3 was back in 2020:
> https://lore.kernel.org/qemu-devel/20200522023440.26261-1-richard.henderson@linaro.org/
>
> I've refreshed that, not that risu has changed much in that time,
> and then also added support for SME to aarch64, i.e. SVCR and ZA
> storage are now present in the reginfo, and compared.
>
> I include a small test case, which allows one to see that ZA
> storage is being handled properly. When run with
>
> ./risu --test-sve=1 --test-za=1 --master -t test_sme_aarch64.{out,bin}
> ./risu --fulldump -t test_sme_aarch64.out
>
> one can see the 16x16 bytes filled with row major then
> column major indexes.
I've applied patches 1-22 to risu upstream git, and left
review comments/reviewed-by tags on the rest.
thanks
-- PMM
^ permalink raw reply [flat|nested] 47+ messages in thread