* [PATCH v5 1/9] gdbstub: Clean up process_string_cmd
2024-06-27 4:13 [PATCH v5 0/9] Add MTE stubs for aarch64 user mode Gustavo Romero
@ 2024-06-27 4:13 ` Gustavo Romero
2024-06-27 4:13 ` [PATCH v5 2/9] gdbstub: Move GdbCmdParseEntry into a new header file Gustavo Romero
` (7 subsequent siblings)
8 siblings, 0 replies; 21+ messages in thread
From: Gustavo Romero @ 2024-06-27 4:13 UTC (permalink / raw)
To: qemu-devel, philmd, alex.bennee, richard.henderson
Cc: peter.maydell, gustavo.romero
Change 'process_string_cmd' to return true on success and false on
failure, instead of 0 and -1.
Signed-off-by: Gustavo Romero <gustavo.romero@linaro.org>
Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
---
gdbstub/gdbstub.c | 40 ++++++++++++++++++++--------------------
1 file changed, 20 insertions(+), 20 deletions(-)
diff --git a/gdbstub/gdbstub.c b/gdbstub/gdbstub.c
index b3574997ea..37314b92e5 100644
--- a/gdbstub/gdbstub.c
+++ b/gdbstub/gdbstub.c
@@ -962,14 +962,14 @@ static inline int startswith(const char *string, const char *pattern)
return !strncmp(string, pattern, strlen(pattern));
}
-static int process_string_cmd(const char *data,
- const GdbCmdParseEntry *cmds, int num_cmds)
+static bool process_string_cmd(const char *data,
+ const GdbCmdParseEntry *cmds, int num_cmds)
{
int i;
g_autoptr(GArray) params = g_array_new(false, true, sizeof(GdbCmdVariant));
if (!cmds) {
- return -1;
+ return false;
}
for (i = 0; i < num_cmds; i++) {
@@ -984,16 +984,16 @@ static int process_string_cmd(const char *data,
if (cmd->schema) {
if (cmd_parse_params(&data[strlen(cmd->cmd)],
cmd->schema, params)) {
- return -1;
+ return false;
}
}
gdbserver_state.allow_stop_reply = cmd->allow_stop_reply;
cmd->handler(params, NULL);
- return 0;
+ return true;
}
- return -1;
+ return false;
}
static void run_cmd_parser(const char *data, const GdbCmdParseEntry *cmd)
@@ -1007,7 +1007,7 @@ static void run_cmd_parser(const char *data, const GdbCmdParseEntry *cmd)
/* In case there was an error during the command parsing we must
* send a NULL packet to indicate the command is not supported */
- if (process_string_cmd(data, cmd, 1)) {
+ if (!process_string_cmd(data, cmd, 1)) {
gdb_put_packet("");
}
}
@@ -1523,9 +1523,9 @@ static void handle_v_commands(GArray *params, void *user_ctx)
return;
}
- if (process_string_cmd(get_param(params, 0)->data,
- gdb_v_commands_table,
- ARRAY_SIZE(gdb_v_commands_table))) {
+ if (!process_string_cmd(get_param(params, 0)->data,
+ gdb_v_commands_table,
+ ARRAY_SIZE(gdb_v_commands_table))) {
gdb_put_packet("");
}
}
@@ -1889,15 +1889,15 @@ static void handle_gen_query(GArray *params, void *user_ctx)
return;
}
- if (!process_string_cmd(get_param(params, 0)->data,
- gdb_gen_query_set_common_table,
- ARRAY_SIZE(gdb_gen_query_set_common_table))) {
+ if (process_string_cmd(get_param(params, 0)->data,
+ gdb_gen_query_set_common_table,
+ ARRAY_SIZE(gdb_gen_query_set_common_table))) {
return;
}
- if (process_string_cmd(get_param(params, 0)->data,
- gdb_gen_query_table,
- ARRAY_SIZE(gdb_gen_query_table))) {
+ if (!process_string_cmd(get_param(params, 0)->data,
+ gdb_gen_query_table,
+ ARRAY_SIZE(gdb_gen_query_table))) {
gdb_put_packet("");
}
}
@@ -1908,13 +1908,13 @@ static void handle_gen_set(GArray *params, void *user_ctx)
return;
}
- if (!process_string_cmd(get_param(params, 0)->data,
- gdb_gen_query_set_common_table,
- ARRAY_SIZE(gdb_gen_query_set_common_table))) {
+ if (process_string_cmd(get_param(params, 0)->data,
+ gdb_gen_query_set_common_table,
+ ARRAY_SIZE(gdb_gen_query_set_common_table))) {
return;
}
- if (process_string_cmd(get_param(params, 0)->data,
+ if (!process_string_cmd(get_param(params, 0)->data,
gdb_gen_set_table,
ARRAY_SIZE(gdb_gen_set_table))) {
gdb_put_packet("");
--
2.34.1
^ permalink raw reply related [flat|nested] 21+ messages in thread* [PATCH v5 2/9] gdbstub: Move GdbCmdParseEntry into a new header file
2024-06-27 4:13 [PATCH v5 0/9] Add MTE stubs for aarch64 user mode Gustavo Romero
2024-06-27 4:13 ` [PATCH v5 1/9] gdbstub: Clean up process_string_cmd Gustavo Romero
@ 2024-06-27 4:13 ` Gustavo Romero
2024-06-27 6:02 ` Philippe Mathieu-Daudé
2024-06-27 4:13 ` [PATCH v5 3/9] gdbstub: Add support for target-specific stubs Gustavo Romero
` (6 subsequent siblings)
8 siblings, 1 reply; 21+ messages in thread
From: Gustavo Romero @ 2024-06-27 4:13 UTC (permalink / raw)
To: qemu-devel, philmd, alex.bennee, richard.henderson
Cc: peter.maydell, gustavo.romero
Move GdbCmdParseEntry and its associated types into a separate header
file to allow the use of GdbCmdParseEntry and other gdbstub command
functions outside of gdbstub.c.
Since GdbCmdParseEntry and get_param are now public, kdoc
GdbCmdParseEntry and rename get_param to gdb_get_cmd_param.
This commit also makes gdb_put_packet public since is used in gdbstub
command handling.
Signed-off-by: Gustavo Romero <gustavo.romero@linaro.org>
Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
---
gdbstub/gdbstub.c | 134 ++++++++++++++-----------------------
gdbstub/internals.h | 22 ------
gdbstub/syscalls.c | 7 +-
gdbstub/system.c | 7 +-
gdbstub/user-target.c | 25 +++----
gdbstub/user.c | 7 +-
include/gdbstub/commands.h | 74 ++++++++++++++++++++
7 files changed, 148 insertions(+), 128 deletions(-)
create mode 100644 include/gdbstub/commands.h
diff --git a/gdbstub/gdbstub.c b/gdbstub/gdbstub.c
index 37314b92e5..9ff2f4177d 100644
--- a/gdbstub/gdbstub.c
+++ b/gdbstub/gdbstub.c
@@ -30,6 +30,7 @@
#include "qemu/error-report.h"
#include "trace.h"
#include "exec/gdbstub.h"
+#include "gdbstub/commands.h"
#include "gdbstub/syscalls.h"
#ifdef CONFIG_USER_ONLY
#include "accel/tcg/vcpu-state.h"
@@ -920,43 +921,6 @@ static int cmd_parse_params(const char *data, const char *schema,
return 0;
}
-typedef void (*GdbCmdHandler)(GArray *params, void *user_ctx);
-
-/*
- * cmd_startswith -> cmd is compared using startswith
- *
- * allow_stop_reply -> true iff the gdbstub can respond to this command with a
- * "stop reply" packet. The list of commands that accept such response is
- * defined at the GDB Remote Serial Protocol documentation. see:
- * https://sourceware.org/gdb/onlinedocs/gdb/Stop-Reply-Packets.html#Stop-Reply-Packets.
- *
- * schema definitions:
- * Each schema parameter entry consists of 2 chars,
- * the first char represents the parameter type handling
- * the second char represents the delimiter for the next parameter
- *
- * Currently supported schema types:
- * 'l' -> unsigned long (stored in .val_ul)
- * 'L' -> unsigned long long (stored in .val_ull)
- * 's' -> string (stored in .data)
- * 'o' -> single char (stored in .opcode)
- * 't' -> thread id (stored in .thread_id)
- * '?' -> skip according to delimiter
- *
- * Currently supported delimiters:
- * '?' -> Stop at any delimiter (",;:=\0")
- * '0' -> Stop at "\0"
- * '.' -> Skip 1 char unless reached "\0"
- * Any other value is treated as the delimiter value itself
- */
-typedef struct GdbCmdParseEntry {
- GdbCmdHandler handler;
- const char *cmd;
- bool cmd_startswith;
- const char *schema;
- bool allow_stop_reply;
-} GdbCmdParseEntry;
-
static inline int startswith(const char *string, const char *pattern)
{
return !strncmp(string, pattern, strlen(pattern));
@@ -1023,7 +987,7 @@ static void handle_detach(GArray *params, void *user_ctx)
return;
}
- pid = get_param(params, 0)->val_ul;
+ pid = gdb_get_cmd_param(params, 0)->val_ul;
}
#ifdef CONFIG_USER_ONLY
@@ -1061,13 +1025,13 @@ static void handle_thread_alive(GArray *params, void *user_ctx)
return;
}
- if (get_param(params, 0)->thread_id.kind == GDB_READ_THREAD_ERR) {
+ if (gdb_get_cmd_param(params, 0)->thread_id.kind == GDB_READ_THREAD_ERR) {
gdb_put_packet("E22");
return;
}
- cpu = gdb_get_cpu(get_param(params, 0)->thread_id.pid,
- get_param(params, 0)->thread_id.tid);
+ cpu = gdb_get_cpu(gdb_get_cmd_param(params, 0)->thread_id.pid,
+ gdb_get_cmd_param(params, 0)->thread_id.tid);
if (!cpu) {
gdb_put_packet("E22");
return;
@@ -1079,7 +1043,7 @@ static void handle_thread_alive(GArray *params, void *user_ctx)
static void handle_continue(GArray *params, void *user_ctx)
{
if (params->len) {
- gdb_set_cpu_pc(get_param(params, 0)->val_ull);
+ gdb_set_cpu_pc(gdb_get_cmd_param(params, 0)->val_ull);
}
gdbserver_state.signal = 0;
@@ -1095,7 +1059,7 @@ static void handle_cont_with_sig(GArray *params, void *user_ctx)
* omit the addr parameter
*/
if (params->len) {
- signal = get_param(params, 0)->val_ul;
+ signal = gdb_get_cmd_param(params, 0)->val_ul;
}
gdbserver_state.signal = gdb_signal_to_target(signal);
@@ -1115,18 +1079,18 @@ static void handle_set_thread(GArray *params, void *user_ctx)
return;
}
- if (get_param(params, 1)->thread_id.kind == GDB_READ_THREAD_ERR) {
+ if (gdb_get_cmd_param(params, 1)->thread_id.kind == GDB_READ_THREAD_ERR) {
gdb_put_packet("E22");
return;
}
- if (get_param(params, 1)->thread_id.kind != GDB_ONE_THREAD) {
+ if (gdb_get_cmd_param(params, 1)->thread_id.kind != GDB_ONE_THREAD) {
gdb_put_packet("OK");
return;
}
- pid = get_param(params, 1)->thread_id.pid;
- tid = get_param(params, 1)->thread_id.tid;
+ pid = gdb_get_cmd_param(params, 1)->thread_id.pid;
+ tid = gdb_get_cmd_param(params, 1)->thread_id.tid;
#ifdef CONFIG_USER_ONLY
if (gdb_handle_set_thread_user(pid, tid)) {
return;
@@ -1142,7 +1106,7 @@ static void handle_set_thread(GArray *params, void *user_ctx)
* Note: This command is deprecated and modern gdb's will be using the
* vCont command instead.
*/
- switch (get_param(params, 0)->opcode) {
+ switch (gdb_get_cmd_param(params, 0)->opcode) {
case 'c':
gdbserver_state.c_cpu = cpu;
gdb_put_packet("OK");
@@ -1167,9 +1131,9 @@ static void handle_insert_bp(GArray *params, void *user_ctx)
}
res = gdb_breakpoint_insert(gdbserver_state.c_cpu,
- get_param(params, 0)->val_ul,
- get_param(params, 1)->val_ull,
- get_param(params, 2)->val_ull);
+ gdb_get_cmd_param(params, 0)->val_ul,
+ gdb_get_cmd_param(params, 1)->val_ull,
+ gdb_get_cmd_param(params, 2)->val_ull);
if (res >= 0) {
gdb_put_packet("OK");
return;
@@ -1191,9 +1155,9 @@ static void handle_remove_bp(GArray *params, void *user_ctx)
}
res = gdb_breakpoint_remove(gdbserver_state.c_cpu,
- get_param(params, 0)->val_ul,
- get_param(params, 1)->val_ull,
- get_param(params, 2)->val_ull);
+ gdb_get_cmd_param(params, 0)->val_ul,
+ gdb_get_cmd_param(params, 1)->val_ull,
+ gdb_get_cmd_param(params, 2)->val_ull);
if (res >= 0) {
gdb_put_packet("OK");
return;
@@ -1225,10 +1189,10 @@ static void handle_set_reg(GArray *params, void *user_ctx)
return;
}
- reg_size = strlen(get_param(params, 1)->data) / 2;
- gdb_hextomem(gdbserver_state.mem_buf, get_param(params, 1)->data, reg_size);
+ reg_size = strlen(gdb_get_cmd_param(params, 1)->data) / 2;
+ gdb_hextomem(gdbserver_state.mem_buf, gdb_get_cmd_param(params, 1)->data, reg_size);
gdb_write_register(gdbserver_state.g_cpu, gdbserver_state.mem_buf->data,
- get_param(params, 0)->val_ull);
+ gdb_get_cmd_param(params, 0)->val_ull);
gdb_put_packet("OK");
}
@@ -1243,7 +1207,7 @@ static void handle_get_reg(GArray *params, void *user_ctx)
reg_size = gdb_read_register(gdbserver_state.g_cpu,
gdbserver_state.mem_buf,
- get_param(params, 0)->val_ull);
+ gdb_get_cmd_param(params, 0)->val_ull);
if (!reg_size) {
gdb_put_packet("E14");
return;
@@ -1264,16 +1228,16 @@ static void handle_write_mem(GArray *params, void *user_ctx)
}
/* gdb_hextomem() reads 2*len bytes */
- if (get_param(params, 1)->val_ull >
- strlen(get_param(params, 2)->data) / 2) {
+ if (gdb_get_cmd_param(params, 1)->val_ull >
+ strlen(gdb_get_cmd_param(params, 2)->data) / 2) {
gdb_put_packet("E22");
return;
}
- gdb_hextomem(gdbserver_state.mem_buf, get_param(params, 2)->data,
- get_param(params, 1)->val_ull);
+ gdb_hextomem(gdbserver_state.mem_buf, gdb_get_cmd_param(params, 2)->data,
+ gdb_get_cmd_param(params, 1)->val_ull);
if (gdb_target_memory_rw_debug(gdbserver_state.g_cpu,
- get_param(params, 0)->val_ull,
+ gdb_get_cmd_param(params, 0)->val_ull,
gdbserver_state.mem_buf->data,
gdbserver_state.mem_buf->len, true)) {
gdb_put_packet("E14");
@@ -1291,16 +1255,16 @@ static void handle_read_mem(GArray *params, void *user_ctx)
}
/* gdb_memtohex() doubles the required space */
- if (get_param(params, 1)->val_ull > MAX_PACKET_LENGTH / 2) {
+ if (gdb_get_cmd_param(params, 1)->val_ull > MAX_PACKET_LENGTH / 2) {
gdb_put_packet("E22");
return;
}
g_byte_array_set_size(gdbserver_state.mem_buf,
- get_param(params, 1)->val_ull);
+ gdb_get_cmd_param(params, 1)->val_ull);
if (gdb_target_memory_rw_debug(gdbserver_state.g_cpu,
- get_param(params, 0)->val_ull,
+ gdb_get_cmd_param(params, 0)->val_ull,
gdbserver_state.mem_buf->data,
gdbserver_state.mem_buf->len, false)) {
gdb_put_packet("E14");
@@ -1324,8 +1288,8 @@ static void handle_write_all_regs(GArray *params, void *user_ctx)
}
cpu_synchronize_state(gdbserver_state.g_cpu);
- len = strlen(get_param(params, 0)->data) / 2;
- gdb_hextomem(gdbserver_state.mem_buf, get_param(params, 0)->data, len);
+ len = strlen(gdb_get_cmd_param(params, 0)->data) / 2;
+ gdb_hextomem(gdbserver_state.mem_buf, gdb_get_cmd_param(params, 0)->data, len);
registers = gdbserver_state.mem_buf->data;
for (reg_id = 0;
reg_id < gdbserver_state.g_cpu->gdb_num_g_regs && len > 0;
@@ -1360,7 +1324,7 @@ static void handle_read_all_regs(GArray *params, void *user_ctx)
static void handle_step(GArray *params, void *user_ctx)
{
if (params->len) {
- gdb_set_cpu_pc(get_param(params, 0)->val_ull);
+ gdb_set_cpu_pc(gdb_get_cmd_param(params, 0)->val_ull);
}
cpu_single_step(gdbserver_state.c_cpu, gdbserver_state.sstep_flags);
@@ -1373,7 +1337,7 @@ static void handle_backward(GArray *params, void *user_ctx)
gdb_put_packet("E22");
}
if (params->len == 1) {
- switch (get_param(params, 0)->opcode) {
+ switch (gdb_get_cmd_param(params, 0)->opcode) {
case 's':
if (replay_reverse_step()) {
gdb_continue();
@@ -1408,7 +1372,7 @@ static void handle_v_cont(GArray *params, void *user_ctx)
return;
}
- res = gdb_handle_vcont(get_param(params, 0)->data);
+ res = gdb_handle_vcont(gdb_get_cmd_param(params, 0)->data);
if ((res == -EINVAL) || (res == -ERANGE)) {
gdb_put_packet("E22");
} else if (res) {
@@ -1426,7 +1390,7 @@ static void handle_v_attach(GArray *params, void *user_ctx)
goto cleanup;
}
- process = gdb_get_process(get_param(params, 0)->val_ul);
+ process = gdb_get_process(gdb_get_cmd_param(params, 0)->val_ul);
if (!process) {
goto cleanup;
}
@@ -1523,7 +1487,7 @@ static void handle_v_commands(GArray *params, void *user_ctx)
return;
}
- if (!process_string_cmd(get_param(params, 0)->data,
+ if (!process_string_cmd(gdb_get_cmd_param(params, 0)->data,
gdb_v_commands_table,
ARRAY_SIZE(gdb_v_commands_table))) {
gdb_put_packet("");
@@ -1555,7 +1519,7 @@ static void handle_set_qemu_sstep(GArray *params, void *user_ctx)
return;
}
- new_sstep_flags = get_param(params, 0)->val_ul;
+ new_sstep_flags = gdb_get_cmd_param(params, 0)->val_ul;
if (new_sstep_flags & ~gdbserver_state.supported_sstep_flags) {
gdb_put_packet("E22");
@@ -1615,13 +1579,13 @@ static void handle_query_thread_extra(GArray *params, void *user_ctx)
CPUState *cpu;
if (!params->len ||
- get_param(params, 0)->thread_id.kind == GDB_READ_THREAD_ERR) {
+ gdb_get_cmd_param(params, 0)->thread_id.kind == GDB_READ_THREAD_ERR) {
gdb_put_packet("E22");
return;
}
- cpu = gdb_get_cpu(get_param(params, 0)->thread_id.pid,
- get_param(params, 0)->thread_id.tid);
+ cpu = gdb_get_cpu(gdb_get_cmd_param(params, 0)->thread_id.pid,
+ gdb_get_cmd_param(params, 0)->thread_id.tid);
if (!cpu) {
return;
}
@@ -1673,7 +1637,7 @@ static void handle_query_supported(GArray *params, void *user_ctx)
#endif
if (params->len) {
- const char *gdb_supported = get_param(params, 0)->data;
+ const char *gdb_supported = gdb_get_cmd_param(params, 0)->data;
if (strstr(gdb_supported, "multiprocess+")) {
gdbserver_state.multiprocess = true;
@@ -1707,15 +1671,15 @@ static void handle_query_xfer_features(GArray *params, void *user_ctx)
return;
}
- p = get_param(params, 0)->data;
+ p = gdb_get_cmd_param(params, 0)->data;
xml = get_feature_xml(p, &p, process);
if (!xml) {
gdb_put_packet("E00");
return;
}
- addr = get_param(params, 1)->val_ul;
- len = get_param(params, 2)->val_ul;
+ addr = gdb_get_cmd_param(params, 1)->val_ul;
+ len = gdb_get_cmd_param(params, 2)->val_ul;
total_len = strlen(xml);
if (addr > total_len) {
gdb_put_packet("E00");
@@ -1889,13 +1853,13 @@ static void handle_gen_query(GArray *params, void *user_ctx)
return;
}
- if (process_string_cmd(get_param(params, 0)->data,
+ if (process_string_cmd(gdb_get_cmd_param(params, 0)->data,
gdb_gen_query_set_common_table,
ARRAY_SIZE(gdb_gen_query_set_common_table))) {
return;
}
- if (!process_string_cmd(get_param(params, 0)->data,
+ if (!process_string_cmd(gdb_get_cmd_param(params, 0)->data,
gdb_gen_query_table,
ARRAY_SIZE(gdb_gen_query_table))) {
gdb_put_packet("");
@@ -1908,13 +1872,13 @@ static void handle_gen_set(GArray *params, void *user_ctx)
return;
}
- if (process_string_cmd(get_param(params, 0)->data,
+ if (process_string_cmd(gdb_get_cmd_param(params, 0)->data,
gdb_gen_query_set_common_table,
ARRAY_SIZE(gdb_gen_query_set_common_table))) {
return;
}
- if (!process_string_cmd(get_param(params, 0)->data,
+ if (!process_string_cmd(gdb_get_cmd_param(params, 0)->data,
gdb_gen_set_table,
ARRAY_SIZE(gdb_gen_set_table))) {
gdb_put_packet("");
diff --git a/gdbstub/internals.h b/gdbstub/internals.h
index 32f9f63297..34121dc61a 100644
--- a/gdbstub/internals.h
+++ b/gdbstub/internals.h
@@ -106,7 +106,6 @@ static inline int tohex(int v)
*/
void gdb_put_strbuf(void);
-int gdb_put_packet(const char *buf);
int gdb_put_packet_binary(const char *buf, int len, bool dump);
void gdb_hextomem(GByteArray *mem, const char *buf, int len);
void gdb_memtohex(GString *buf, const uint8_t *mem, int len);
@@ -166,27 +165,6 @@ void gdb_put_buffer(const uint8_t *buf, int len);
*/
void gdb_init_gdbserver_state(void);
-typedef enum GDBThreadIdKind {
- GDB_ONE_THREAD = 0,
- GDB_ALL_THREADS, /* One process, all threads */
- GDB_ALL_PROCESSES,
- GDB_READ_THREAD_ERR
-} GDBThreadIdKind;
-
-typedef union GdbCmdVariant {
- const char *data;
- uint8_t opcode;
- unsigned long val_ul;
- unsigned long long val_ull;
- struct {
- GDBThreadIdKind kind;
- uint32_t pid;
- uint32_t tid;
- } thread_id;
-} GdbCmdVariant;
-
-#define get_param(p, i) (&g_array_index(p, GdbCmdVariant, i))
-
void gdb_handle_query_rcmd(GArray *params, void *ctx); /* system */
void gdb_handle_query_offsets(GArray *params, void *user_ctx); /* user */
void gdb_handle_query_xfer_auxv(GArray *params, void *user_ctx); /*user */
diff --git a/gdbstub/syscalls.c b/gdbstub/syscalls.c
index 02e3a8f74c..4e1295b782 100644
--- a/gdbstub/syscalls.c
+++ b/gdbstub/syscalls.c
@@ -16,6 +16,7 @@
#include "sysemu/runstate.h"
#include "gdbstub/user.h"
#include "gdbstub/syscalls.h"
+#include "gdbstub/commands.h"
#include "trace.h"
#include "internals.h"
@@ -154,9 +155,9 @@ void gdb_handle_file_io(GArray *params, void *user_ctx)
uint64_t ret;
int err;
- ret = get_param(params, 0)->val_ull;
+ ret = gdb_get_cmd_param(params, 0)->val_ull;
if (params->len >= 2) {
- err = get_param(params, 1)->val_ull;
+ err = gdb_get_cmd_param(params, 1)->val_ull;
} else {
err = 0;
}
@@ -196,7 +197,7 @@ void gdb_handle_file_io(GArray *params, void *user_ctx)
gdbserver_syscall_state.current_syscall_cb = NULL;
}
- if (params->len >= 3 && get_param(params, 2)->opcode == (uint8_t)'C') {
+ if (params->len >= 3 && gdb_get_cmd_param(params, 2)->opcode == (uint8_t)'C') {
gdb_put_packet("T02");
return;
}
diff --git a/gdbstub/system.c b/gdbstub/system.c
index d235403855..1ad87fe7fd 100644
--- a/gdbstub/system.c
+++ b/gdbstub/system.c
@@ -16,6 +16,7 @@
#include "qemu/cutils.h"
#include "exec/gdbstub.h"
#include "gdbstub/syscalls.h"
+#include "gdbstub/commands.h"
#include "exec/hwaddr.h"
#include "exec/tb-flush.h"
#include "sysemu/cpus.h"
@@ -501,7 +502,7 @@ void gdb_handle_set_qemu_phy_mem_mode(GArray *params, void *ctx)
return;
}
- if (!get_param(params, 0)->val_ul) {
+ if (!gdb_get_cmd_param(params, 0)->val_ul) {
phy_memory_mode = 0;
} else {
phy_memory_mode = 1;
@@ -519,7 +520,7 @@ void gdb_handle_query_rcmd(GArray *params, void *ctx)
return;
}
- len = strlen(get_param(params, 0)->data);
+ len = strlen(gdb_get_cmd_param(params, 0)->data);
if (len % 2) {
gdb_put_packet("E01");
return;
@@ -527,7 +528,7 @@ void gdb_handle_query_rcmd(GArray *params, void *ctx)
g_assert(gdbserver_state.mem_buf->len == 0);
len = len / 2;
- gdb_hextomem(gdbserver_state.mem_buf, get_param(params, 0)->data, len);
+ gdb_hextomem(gdbserver_state.mem_buf, gdb_get_cmd_param(params, 0)->data, len);
g_byte_array_append(gdbserver_state.mem_buf, &zero, 1);
qemu_chr_be_write(gdbserver_system_state.mon_chr,
gdbserver_state.mem_buf->data,
diff --git a/gdbstub/user-target.c b/gdbstub/user-target.c
index a9c6c64512..b5e01fd8b0 100644
--- a/gdbstub/user-target.c
+++ b/gdbstub/user-target.c
@@ -9,6 +9,7 @@
#include "qemu/osdep.h"
#include "exec/gdbstub.h"
+#include "gdbstub/commands.h"
#include "qemu.h"
#include "internals.h"
#ifdef CONFIG_LINUX
@@ -250,8 +251,8 @@ void gdb_handle_query_xfer_auxv(GArray *params, void *user_ctx)
return;
}
- offset = get_param(params, 0)->val_ul;
- len = get_param(params, 1)->val_ul;
+ offset = gdb_get_cmd_param(params, 0)->val_ul;
+ len = gdb_get_cmd_param(params, 1)->val_ul;
ts = get_task_state(gdbserver_state.c_cpu);
saved_auxv = ts->info->saved_auxv;
auxv_len = ts->info->auxv_len;
@@ -288,7 +289,7 @@ void gdb_handle_query_xfer_auxv(GArray *params, void *user_ctx)
static const char *get_filename_param(GArray *params, int i)
{
- const char *hex_filename = get_param(params, i)->data;
+ const char *hex_filename = gdb_get_cmd_param(params, i)->data;
gdb_hextomem(gdbserver_state.mem_buf, hex_filename,
strlen(hex_filename) / 2);
g_byte_array_append(gdbserver_state.mem_buf, (const guint8 *)"", 1);
@@ -306,8 +307,8 @@ static void hostio_reply_with_data(const void *buf, size_t n)
void gdb_handle_v_file_open(GArray *params, void *user_ctx)
{
const char *filename = get_filename_param(params, 0);
- uint64_t flags = get_param(params, 1)->val_ull;
- uint64_t mode = get_param(params, 2)->val_ull;
+ uint64_t flags = gdb_get_cmd_param(params, 1)->val_ull;
+ uint64_t mode = gdb_get_cmd_param(params, 2)->val_ull;
#ifdef CONFIG_LINUX
int fd = do_guest_openat(cpu_env(gdbserver_state.g_cpu), 0, filename,
@@ -325,7 +326,7 @@ void gdb_handle_v_file_open(GArray *params, void *user_ctx)
void gdb_handle_v_file_close(GArray *params, void *user_ctx)
{
- int fd = get_param(params, 0)->val_ul;
+ int fd = gdb_get_cmd_param(params, 0)->val_ul;
if (close(fd) == -1) {
g_string_printf(gdbserver_state.str_buf, "F-1,%d", errno);
@@ -338,9 +339,9 @@ void gdb_handle_v_file_close(GArray *params, void *user_ctx)
void gdb_handle_v_file_pread(GArray *params, void *user_ctx)
{
- int fd = get_param(params, 0)->val_ul;
- size_t count = get_param(params, 1)->val_ull;
- off_t offset = get_param(params, 2)->val_ull;
+ int fd = gdb_get_cmd_param(params, 0)->val_ul;
+ size_t count = gdb_get_cmd_param(params, 1)->val_ull;
+ off_t offset = gdb_get_cmd_param(params, 2)->val_ull;
size_t bufsiz = MIN(count, BUFSIZ);
g_autofree char *buf = g_try_malloc(bufsiz);
@@ -383,9 +384,9 @@ void gdb_handle_v_file_readlink(GArray *params, void *user_ctx)
void gdb_handle_query_xfer_exec_file(GArray *params, void *user_ctx)
{
- uint32_t pid = get_param(params, 0)->val_ul;
- uint32_t offset = get_param(params, 1)->val_ul;
- uint32_t length = get_param(params, 2)->val_ul;
+ uint32_t pid = gdb_get_cmd_param(params, 0)->val_ul;
+ uint32_t offset = gdb_get_cmd_param(params, 1)->val_ul;
+ uint32_t length = gdb_get_cmd_param(params, 2)->val_ul;
GDBProcess *process = gdb_get_process(pid);
if (!process) {
diff --git a/gdbstub/user.c b/gdbstub/user.c
index e34b58b407..b36033bc7a 100644
--- a/gdbstub/user.c
+++ b/gdbstub/user.c
@@ -16,6 +16,7 @@
#include "exec/hwaddr.h"
#include "exec/tb-flush.h"
#include "exec/gdbstub.h"
+#include "gdbstub/commands.h"
#include "gdbstub/syscalls.h"
#include "gdbstub/user.h"
#include "gdbstub/enums.h"
@@ -793,7 +794,7 @@ void gdb_syscall_return(CPUState *cs, int num)
void gdb_handle_set_catch_syscalls(GArray *params, void *user_ctx)
{
- const char *param = get_param(params, 0)->data;
+ const char *param = gdb_get_cmd_param(params, 0)->data;
GDBSyscallsMask catch_syscalls_mask;
bool catch_all_syscalls;
unsigned int num;
@@ -858,8 +859,8 @@ void gdb_handle_query_xfer_siginfo(GArray *params, void *user_ctx)
unsigned long offset, len;
uint8_t *siginfo_offset;
- offset = get_param(params, 0)->val_ul;
- len = get_param(params, 1)->val_ul;
+ offset = gdb_get_cmd_param(params, 0)->val_ul;
+ len = gdb_get_cmd_param(params, 1)->val_ul;
if (offset + len > gdbserver_user_state.siginfo_len) {
/* Invalid offset and/or requested length. */
diff --git a/include/gdbstub/commands.h b/include/gdbstub/commands.h
new file mode 100644
index 0000000000..dd45c38472
--- /dev/null
+++ b/include/gdbstub/commands.h
@@ -0,0 +1,74 @@
+#ifndef GDBSTUB_COMMANDS_H
+#define GDBSTUB
+
+typedef void (*GdbCmdHandler)(GArray *params, void *user_ctx);
+
+typedef enum GDBThreadIdKind {
+ GDB_ONE_THREAD = 0,
+ GDB_ALL_THREADS, /* One process, all threads */
+ GDB_ALL_PROCESSES,
+ GDB_READ_THREAD_ERR
+} GDBThreadIdKind;
+
+typedef union GdbCmdVariant {
+ const char *data;
+ uint8_t opcode;
+ unsigned long val_ul;
+ unsigned long long val_ull;
+ struct {
+ GDBThreadIdKind kind;
+ uint32_t pid;
+ uint32_t tid;
+ } thread_id;
+} GdbCmdVariant;
+
+#define gdb_get_cmd_param(p, i) (&g_array_index(p, GdbCmdVariant, i))
+
+/**
+ * typedef GdbCmdParseEntry - gdb command parser
+ *
+ * This structure keeps the information necessary to match a gdb command,
+ * parse it (extract its parameters), and select the correct handler for it.
+ *
+ * @cmd: The command to be matched
+ * @cmd_startswith: If true, @cmd is compared using startswith
+ * @schema: Each schema for the command parameter entry consists of 2 chars,
+ * the first char represents the parameter type handling the second char
+ * represents the delimiter for the next parameter.
+ *
+ * Currently supported schema types:
+ * 'l' -> unsigned long (stored in .val_ul)
+ * 'L' -> unsigned long long (stored in .val_ull)
+ * 's' -> string (stored in .data)
+ * 'o' -> single char (stored in .opcode)
+ * 't' -> thread id (stored in .thread_id)
+ * '?' -> skip according to delimiter
+ *
+ * Currently supported delimiters:
+ * '?' -> Stop at any delimiter (",;:=\0")
+ * '0' -> Stop at "\0"
+ * '.' -> Skip 1 char unless reached "\0"
+ * Any other value is treated as the delimiter value itself
+ *
+ * @allow_stop_reply: True iff the gdbstub can respond to this command with a
+ * "stop reply" packet. The list of commands that accept such response is
+ * defined at the GDB Remote Serial Protocol documentation. See:
+ * https://sourceware.org/gdb/onlinedocs/gdb/Stop-Reply-Packets.html#Stop-Reply-Packets.
+ */
+typedef struct GdbCmdParseEntry {
+ GdbCmdHandler handler;
+ const char *cmd;
+ bool cmd_startswith;
+ const char *schema;
+ bool allow_stop_reply;
+} GdbCmdParseEntry;
+
+#define get_cmd_parsers(p) (&g_array_index(p, GdbCmdParseEntry, 0))
+
+/**
+ * gdb_put_packet() - put string into gdb server's buffer so it is sent
+ * to the client
+ */
+int gdb_put_packet(const char *buf);
+
+#endif /* GDBSTUB_COMMANDS_H */
--
2.34.1
^ permalink raw reply related [flat|nested] 21+ messages in thread* Re: [PATCH v5 2/9] gdbstub: Move GdbCmdParseEntry into a new header file
2024-06-27 4:13 ` [PATCH v5 2/9] gdbstub: Move GdbCmdParseEntry into a new header file Gustavo Romero
@ 2024-06-27 6:02 ` Philippe Mathieu-Daudé
2024-06-28 5:10 ` Gustavo Romero
0 siblings, 1 reply; 21+ messages in thread
From: Philippe Mathieu-Daudé @ 2024-06-27 6:02 UTC (permalink / raw)
To: Gustavo Romero, qemu-devel, alex.bennee, richard.henderson; +Cc: peter.maydell
On 27/6/24 06:13, Gustavo Romero wrote:
> Move GdbCmdParseEntry and its associated types into a separate header
> file to allow the use of GdbCmdParseEntry and other gdbstub command
> functions outside of gdbstub.c.
>
> Since GdbCmdParseEntry and get_param are now public, kdoc
> GdbCmdParseEntry and rename get_param to gdb_get_cmd_param.
>
> This commit also makes gdb_put_packet public since is used in gdbstub
> command handling.
>
> Signed-off-by: Gustavo Romero <gustavo.romero@linaro.org>
> Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
> ---
> gdbstub/gdbstub.c | 134 ++++++++++++++-----------------------
> gdbstub/internals.h | 22 ------
> gdbstub/syscalls.c | 7 +-
> gdbstub/system.c | 7 +-
> gdbstub/user-target.c | 25 +++----
> gdbstub/user.c | 7 +-
> include/gdbstub/commands.h | 74 ++++++++++++++++++++
> 7 files changed, 148 insertions(+), 128 deletions(-)
> create mode 100644 include/gdbstub/commands.h
> +#define get_cmd_parsers(p) (&g_array_index(p, GdbCmdParseEntry, 0))
Dead code AFAICT.
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v5 2/9] gdbstub: Move GdbCmdParseEntry into a new header file
2024-06-27 6:02 ` Philippe Mathieu-Daudé
@ 2024-06-28 5:10 ` Gustavo Romero
0 siblings, 0 replies; 21+ messages in thread
From: Gustavo Romero @ 2024-06-28 5:10 UTC (permalink / raw)
To: Philippe Mathieu-Daudé, qemu-devel, alex.bennee,
richard.henderson
Cc: peter.maydell
Hi Phil,
On 6/27/24 3:02 AM, Philippe Mathieu-Daudé wrote:
> On 27/6/24 06:13, Gustavo Romero wrote:
>> Move GdbCmdParseEntry and its associated types into a separate header
>> file to allow the use of GdbCmdParseEntry and other gdbstub command
>> functions outside of gdbstub.c.
>>
>> Since GdbCmdParseEntry and get_param are now public, kdoc
>> GdbCmdParseEntry and rename get_param to gdb_get_cmd_param.
>>
>> This commit also makes gdb_put_packet public since is used in gdbstub
>> command handling.
>>
>> Signed-off-by: Gustavo Romero <gustavo.romero@linaro.org>
>> Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
>> ---
>> gdbstub/gdbstub.c | 134 ++++++++++++++-----------------------
>> gdbstub/internals.h | 22 ------
>> gdbstub/syscalls.c | 7 +-
>> gdbstub/system.c | 7 +-
>> gdbstub/user-target.c | 25 +++----
>> gdbstub/user.c | 7 +-
>> include/gdbstub/commands.h | 74 ++++++++++++++++++++
>> 7 files changed, 148 insertions(+), 128 deletions(-)
>> create mode 100644 include/gdbstub/commands.h
>
>
>> +#define get_cmd_parsers(p) (&g_array_index(p, GdbCmdParseEntry, 0))
>
> Dead code AFAICT.
Yes, that's a leftover. Removed in v6. Thanks.
Cheers,
Gustavo
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH v5 3/9] gdbstub: Add support for target-specific stubs
2024-06-27 4:13 [PATCH v5 0/9] Add MTE stubs for aarch64 user mode Gustavo Romero
2024-06-27 4:13 ` [PATCH v5 1/9] gdbstub: Clean up process_string_cmd Gustavo Romero
2024-06-27 4:13 ` [PATCH v5 2/9] gdbstub: Move GdbCmdParseEntry into a new header file Gustavo Romero
@ 2024-06-27 4:13 ` Gustavo Romero
2024-06-27 4:13 ` [PATCH v5 4/9] target/arm: Fix exception case in allocation_tag_mem_probe Gustavo Romero
` (5 subsequent siblings)
8 siblings, 0 replies; 21+ messages in thread
From: Gustavo Romero @ 2024-06-27 4:13 UTC (permalink / raw)
To: qemu-devel, philmd, alex.bennee, richard.henderson
Cc: peter.maydell, gustavo.romero
Currently, it's not possible to have stubs specific to a given target,
even though there are GDB features which are target-specific, like, for
instance, memory tagging.
This commit introduces gdb_extend_qsupported_features,
gdb_extend_query_table, and gdb_extend_set_table functions as interfaces
to extend the qSupported string, the query handler table, and the set
handler table, allowing target-specific stub implementations.
Signed-off-by: Gustavo Romero <gustavo.romero@linaro.org>
Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
---
gdbstub/gdbstub.c | 102 ++++++++++++++++++++++++++++++++++---
include/gdbstub/commands.h | 22 ++++++++
2 files changed, 118 insertions(+), 6 deletions(-)
diff --git a/gdbstub/gdbstub.c b/gdbstub/gdbstub.c
index 9ff2f4177d..b1ca253f97 100644
--- a/gdbstub/gdbstub.c
+++ b/gdbstub/gdbstub.c
@@ -1609,6 +1609,20 @@ static void handle_query_thread_extra(GArray *params, void *user_ctx)
gdb_put_strbuf();
}
+static char *extended_qsupported_features;
+void gdb_extend_qsupported_features(char *qsupported_features)
+{
+ /*
+ * We don't support different sets of CPU gdb features on different CPUs yet
+ * so assert the feature strings are the same on all CPUs, or is set only
+ * once (1 CPU).
+ */
+ g_assert(extended_qsupported_features == NULL ||
+ g_strcmp0(extended_qsupported_features, qsupported_features) == 0);
+
+ extended_qsupported_features = qsupported_features;
+}
+
static void handle_query_supported(GArray *params, void *user_ctx)
{
CPUClass *cc;
@@ -1648,6 +1662,11 @@ static void handle_query_supported(GArray *params, void *user_ctx)
}
g_string_append(gdbserver_state.str_buf, ";vContSupported+;multiprocess+");
+
+ if (extended_qsupported_features) {
+ g_string_append(gdbserver_state.str_buf, extended_qsupported_features);
+ }
+
gdb_put_strbuf();
}
@@ -1729,6 +1748,41 @@ static const GdbCmdParseEntry gdb_gen_query_set_common_table[] = {
},
};
+/* Compares if a set of command parsers is equal to another set of parsers. */
+static bool cmp_cmds(GdbCmdParseEntry *c, GdbCmdParseEntry *d, int size)
+{
+ for (int i = 0; i < size; i++) {
+ if (!(c[i].handler == d[i].handler &&
+ g_strcmp0(c[i].cmd, d[i].cmd) == 0 &&
+ c[i].cmd_startswith == d[i].cmd_startswith &&
+ g_strcmp0(c[i].schema, d[i].schema) == 0)) {
+
+ /* Sets are different. */
+ return false;
+ }
+ }
+
+ /* Sets are equal, i.e. contain the same command parsers. */
+ return true;
+}
+
+static GdbCmdParseEntry *extended_query_table;
+static int extended_query_table_size;
+void gdb_extend_query_table(GdbCmdParseEntry *table, int size)
+{
+ /*
+ * We don't support different sets of CPU gdb features on different CPUs yet
+ * so assert query table is the same on all CPUs, or is set only once
+ * (1 CPU).
+ */
+ g_assert(extended_query_table == NULL ||
+ (extended_query_table_size == size &&
+ cmp_cmds(extended_query_table, table, size)));
+
+ extended_query_table = table;
+ extended_query_table_size = size;
+}
+
static const GdbCmdParseEntry gdb_gen_query_table[] = {
{
.handler = handle_query_curr_tid,
@@ -1821,6 +1875,22 @@ static const GdbCmdParseEntry gdb_gen_query_table[] = {
#endif
};
+static GdbCmdParseEntry *extended_set_table;
+static int extended_set_table_size;
+void gdb_extend_set_table(GdbCmdParseEntry *table, int size)
+{
+ /*
+ * We don't support different sets of CPU gdb features on different CPUs yet
+ * so assert set table is the same on all CPUs, or is set only once (1 CPU).
+ */
+ g_assert(extended_set_table == NULL ||
+ (extended_set_table_size == size &&
+ cmp_cmds(extended_set_table, table, size)));
+
+ extended_set_table = table;
+ extended_set_table_size = size;
+}
+
static const GdbCmdParseEntry gdb_gen_set_table[] = {
/* Order is important if has same prefix */
{
@@ -1859,11 +1929,21 @@ static void handle_gen_query(GArray *params, void *user_ctx)
return;
}
- if (!process_string_cmd(gdb_get_cmd_param(params, 0)->data,
- gdb_gen_query_table,
- ARRAY_SIZE(gdb_gen_query_table))) {
- gdb_put_packet("");
+ if (process_string_cmd(gdb_get_cmd_param(params, 0)->data,
+ gdb_gen_query_table,
+ ARRAY_SIZE(gdb_gen_query_table))) {
+ return;
+ }
+
+ if (extended_query_table &&
+ process_string_cmd(gdb_get_cmd_param(params, 0)->data,
+ extended_query_table,
+ extended_query_table_size)) {
+ return;
}
+
+ /* Can't handle query, return Empty response. */
+ gdb_put_packet("");
}
static void handle_gen_set(GArray *params, void *user_ctx)
@@ -1878,11 +1958,21 @@ static void handle_gen_set(GArray *params, void *user_ctx)
return;
}
- if (!process_string_cmd(gdb_get_cmd_param(params, 0)->data,
+ if (process_string_cmd(gdb_get_cmd_param(params, 0)->data,
gdb_gen_set_table,
ARRAY_SIZE(gdb_gen_set_table))) {
- gdb_put_packet("");
+ return;
}
+
+ if (extended_set_table &&
+ process_string_cmd(gdb_get_cmd_param(params, 0)->data,
+ extended_set_table,
+ extended_set_table_size)) {
+ return;
+ }
+
+ /* Can't handle set, return Empty response. */
+ gdb_put_packet("");
}
static void handle_target_halt(GArray *params, void *user_ctx)
diff --git a/include/gdbstub/commands.h b/include/gdbstub/commands.h
index dd45c38472..2204c3ddbe 100644
--- a/include/gdbstub/commands.h
+++ b/include/gdbstub/commands.h
@@ -71,4 +71,26 @@ typedef struct GdbCmdParseEntry {
*/
int gdb_put_packet(const char *buf);
+/**
+ * gdb_extend_query_table() - Extend query table.
+ * @table: The table with the additional query packet handlers.
+ * @size: The number of handlers to be added.
+ */
+void gdb_extend_query_table(GdbCmdParseEntry *table, int size);
+
+/**
+ * gdb_extend_set_table() - Extend set table.
+ * @table: The table with the additional set packet handlers.
+ * @size: The number of handlers to be added.
+ */
+void gdb_extend_set_table(GdbCmdParseEntry *table, int size);
+
+/**
+ * gdb_extend_qsupported_features() - Extend the qSupported features string.
+ * @qsupported_features: The additional qSupported feature(s) string. The string
+ * should start with a semicolon and, if there are more than one feature, the
+ * features should be separate by a semiocolon.
+ */
+void gdb_extend_qsupported_features(char *qsupported_features);
+
#endif /* GDBSTUB_COMMANDS_H */
--
2.34.1
^ permalink raw reply related [flat|nested] 21+ messages in thread* [PATCH v5 4/9] target/arm: Fix exception case in allocation_tag_mem_probe
2024-06-27 4:13 [PATCH v5 0/9] Add MTE stubs for aarch64 user mode Gustavo Romero
` (2 preceding siblings ...)
2024-06-27 4:13 ` [PATCH v5 3/9] gdbstub: Add support for target-specific stubs Gustavo Romero
@ 2024-06-27 4:13 ` Gustavo Romero
2024-06-27 4:13 ` [PATCH v5 5/9] target/arm: Make some MTE helpers widely available Gustavo Romero
` (4 subsequent siblings)
8 siblings, 0 replies; 21+ messages in thread
From: Gustavo Romero @ 2024-06-27 4:13 UTC (permalink / raw)
To: qemu-devel, philmd, alex.bennee, richard.henderson
Cc: peter.maydell, gustavo.romero
If page in 'ptr_access' is inaccessible and probe is 'true'
allocation_tag_mem_probe should not throw an exception, but currently it
does, so fix it.
Signed-off-by: Gustavo Romero <gustavo.romero@linaro.org>
Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
---
target/arm/tcg/mte_helper.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/target/arm/tcg/mte_helper.c b/target/arm/tcg/mte_helper.c
index 037ac6dd60..a50d576294 100644
--- a/target/arm/tcg/mte_helper.c
+++ b/target/arm/tcg/mte_helper.c
@@ -96,6 +96,9 @@ static uint8_t *allocation_tag_mem_probe(CPUARMState *env, int ptr_mmu_idx,
assert(!(probe && ra));
if (!(flags & (ptr_access == MMU_DATA_STORE ? PAGE_WRITE_ORG : PAGE_READ))) {
+ if (probe) {
+ return NULL;
+ }
cpu_loop_exit_sigsegv(env_cpu(env), ptr, ptr_access,
!(flags & PAGE_VALID), ra);
}
--
2.34.1
^ permalink raw reply related [flat|nested] 21+ messages in thread* [PATCH v5 5/9] target/arm: Make some MTE helpers widely available
2024-06-27 4:13 [PATCH v5 0/9] Add MTE stubs for aarch64 user mode Gustavo Romero
` (3 preceding siblings ...)
2024-06-27 4:13 ` [PATCH v5 4/9] target/arm: Fix exception case in allocation_tag_mem_probe Gustavo Romero
@ 2024-06-27 4:13 ` Gustavo Romero
2024-06-27 6:02 ` Philippe Mathieu-Daudé
2024-06-27 4:13 ` [PATCH v5 6/9] target/arm: Factor out code for setting MTE TCF0 field Gustavo Romero
` (3 subsequent siblings)
8 siblings, 1 reply; 21+ messages in thread
From: Gustavo Romero @ 2024-06-27 4:13 UTC (permalink / raw)
To: qemu-devel, philmd, alex.bennee, richard.henderson
Cc: peter.maydell, gustavo.romero
Make the MTE helpers allocation_tag_mem_probe, load_tag1, and store_tag1
available to other subsystems.
Signed-off-by: Gustavo Romero <gustavo.romero@linaro.org>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
---
target/arm/tcg/mte_helper.c | 45 ++++---------------------
target/arm/tcg/mte_helper.h | 66 +++++++++++++++++++++++++++++++++++++
2 files changed, 73 insertions(+), 38 deletions(-)
create mode 100644 target/arm/tcg/mte_helper.h
diff --git a/target/arm/tcg/mte_helper.c b/target/arm/tcg/mte_helper.c
index a50d576294..9d2ba287ee 100644
--- a/target/arm/tcg/mte_helper.c
+++ b/target/arm/tcg/mte_helper.c
@@ -29,6 +29,7 @@
#include "hw/core/tcg-cpu-ops.h"
#include "qapi/error.h"
#include "qemu/guest-random.h"
+#include "mte_helper.h"
static int choose_nonexcluded_tag(int tag, int offset, uint16_t exclude)
@@ -50,42 +51,10 @@ static int choose_nonexcluded_tag(int tag, int offset, uint16_t exclude)
return tag;
}
-/**
- * allocation_tag_mem_probe:
- * @env: the cpu environment
- * @ptr_mmu_idx: the addressing regime to use for the virtual address
- * @ptr: the virtual address for which to look up tag memory
- * @ptr_access: the access to use for the virtual address
- * @ptr_size: the number of bytes in the normal memory access
- * @tag_access: the access to use for the tag memory
- * @probe: true to merely probe, never taking an exception
- * @ra: the return address for exception handling
- *
- * Our tag memory is formatted as a sequence of little-endian nibbles.
- * That is, the byte at (addr >> (LOG2_TAG_GRANULE + 1)) contains two
- * tags, with the tag at [3:0] for the lower addr and the tag at [7:4]
- * for the higher addr.
- *
- * Here, resolve the physical address from the virtual address, and return
- * a pointer to the corresponding tag byte.
- *
- * If there is no tag storage corresponding to @ptr, return NULL.
- *
- * If the page is inaccessible for @ptr_access, or has a watchpoint, there are
- * three options:
- * (1) probe = true, ra = 0 : pure probe -- we return NULL if the page is not
- * accessible, and do not take watchpoint traps. The calling code must
- * handle those cases in the right priority compared to MTE traps.
- * (2) probe = false, ra = 0 : probe, no fault expected -- the caller guarantees
- * that the page is going to be accessible. We will take watchpoint traps.
- * (3) probe = false, ra != 0 : non-probe -- we will take both memory access
- * traps and watchpoint traps.
- * (probe = true, ra != 0 is invalid and will assert.)
- */
-static uint8_t *allocation_tag_mem_probe(CPUARMState *env, int ptr_mmu_idx,
- uint64_t ptr, MMUAccessType ptr_access,
- int ptr_size, MMUAccessType tag_access,
- bool probe, uintptr_t ra)
+uint8_t *allocation_tag_mem_probe(CPUARMState *env, int ptr_mmu_idx,
+ uint64_t ptr, MMUAccessType ptr_access,
+ int ptr_size, MMUAccessType tag_access,
+ bool probe, uintptr_t ra)
{
#ifdef CONFIG_USER_ONLY
uint64_t clean_ptr = useronly_clean_ptr(ptr);
@@ -287,7 +256,7 @@ uint64_t HELPER(addsubg)(CPUARMState *env, uint64_t ptr,
return address_with_allocation_tag(ptr + offset, rtag);
}
-static int load_tag1(uint64_t ptr, uint8_t *mem)
+int load_tag1(uint64_t ptr, uint8_t *mem)
{
int ofs = extract32(ptr, LOG2_TAG_GRANULE, 1) * 4;
return extract32(*mem, ofs, 4);
@@ -321,7 +290,7 @@ static void check_tag_aligned(CPUARMState *env, uint64_t ptr, uintptr_t ra)
}
/* For use in a non-parallel context, store to the given nibble. */
-static void store_tag1(uint64_t ptr, uint8_t *mem, int tag)
+void store_tag1(uint64_t ptr, uint8_t *mem, int tag)
{
int ofs = extract32(ptr, LOG2_TAG_GRANULE, 1) * 4;
*mem = deposit32(*mem, ofs, 4, tag);
diff --git a/target/arm/tcg/mte_helper.h b/target/arm/tcg/mte_helper.h
new file mode 100644
index 0000000000..1f471fb69b
--- /dev/null
+++ b/target/arm/tcg/mte_helper.h
@@ -0,0 +1,66 @@
+/*
+ * ARM MemTag operation helpers.
+ *
+ * This code is licensed under the GNU GPL v2 or later.
+ *
+ * SPDX-License-Identifier: LGPL-2.1-or-later
+ */
+
+#ifndef TARGET_ARM_MTE_H
+#define TARGET_ARM_MTE_H
+
+#include "exec/mmu-access-type.h"
+
+/**
+ * allocation_tag_mem_probe:
+ * @env: the cpu environment
+ * @ptr_mmu_idx: the addressing regime to use for the virtual address
+ * @ptr: the virtual address for which to look up tag memory
+ * @ptr_access: the access to use for the virtual address
+ * @ptr_size: the number of bytes in the normal memory access
+ * @tag_access: the access to use for the tag memory
+ * @probe: true to merely probe, never taking an exception
+ * @ra: the return address for exception handling
+ *
+ * Our tag memory is formatted as a sequence of little-endian nibbles.
+ * That is, the byte at (addr >> (LOG2_TAG_GRANULE + 1)) contains two
+ * tags, with the tag at [3:0] for the lower addr and the tag at [7:4]
+ * for the higher addr.
+ *
+ * Here, resolve the physical address from the virtual address, and return
+ * a pointer to the corresponding tag byte.
+ *
+ * If there is no tag storage corresponding to @ptr, return NULL.
+ *
+ * If the page is inaccessible for @ptr_access, or has a watchpoint, there are
+ * three options:
+ * (1) probe = true, ra = 0 : pure probe -- we return NULL if the page is not
+ * accessible, and do not take watchpoint traps. The calling code must
+ * handle those cases in the right priority compared to MTE traps.
+ * (2) probe = false, ra = 0 : probe, no fault expected -- the caller guarantees
+ * that the page is going to be accessible. We will take watchpoint traps.
+ * (3) probe = false, ra != 0 : non-probe -- we will take both memory access
+ * traps and watchpoint traps.
+ * (probe = true, ra != 0 is invalid and will assert.)
+ */
+uint8_t *allocation_tag_mem_probe(CPUARMState *env, int ptr_mmu_idx,
+ uint64_t ptr, MMUAccessType ptr_access,
+ int ptr_size, MMUAccessType tag_access,
+ bool probe, uintptr_t ra);
+
+/**
+ * load_tag1 - Load 1 tag (nibble) from byte
+ * @ptr: The tagged address
+ * @mem: The tag address (packed, 2 tags in byte)
+ */
+int load_tag1(uint64_t ptr, uint8_t *mem);
+
+/**
+ * store_tag1 - Store 1 tag (nibble) into byte
+ * @ptr: The tagged address
+ * @mem: The tag address (packed, 2 tags in byte)
+ * @tag: The tag to be stored in the nibble
+ */
+void store_tag1(uint64_t ptr, uint8_t *mem, int tag);
+
+#endif /* TARGET_ARM_MTE_H */
--
2.34.1
^ permalink raw reply related [flat|nested] 21+ messages in thread* Re: [PATCH v5 5/9] target/arm: Make some MTE helpers widely available
2024-06-27 4:13 ` [PATCH v5 5/9] target/arm: Make some MTE helpers widely available Gustavo Romero
@ 2024-06-27 6:02 ` Philippe Mathieu-Daudé
0 siblings, 0 replies; 21+ messages in thread
From: Philippe Mathieu-Daudé @ 2024-06-27 6:02 UTC (permalink / raw)
To: Gustavo Romero, qemu-devel, alex.bennee, richard.henderson; +Cc: peter.maydell
On 27/6/24 06:13, Gustavo Romero wrote:
> Make the MTE helpers allocation_tag_mem_probe, load_tag1, and store_tag1
> available to other subsystems.
>
> Signed-off-by: Gustavo Romero <gustavo.romero@linaro.org>
> Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
> ---
> target/arm/tcg/mte_helper.c | 45 ++++---------------------
> target/arm/tcg/mte_helper.h | 66 +++++++++++++++++++++++++++++++++++++
> 2 files changed, 73 insertions(+), 38 deletions(-)
> create mode 100644 target/arm/tcg/mte_helper.h
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH v5 6/9] target/arm: Factor out code for setting MTE TCF0 field
2024-06-27 4:13 [PATCH v5 0/9] Add MTE stubs for aarch64 user mode Gustavo Romero
` (4 preceding siblings ...)
2024-06-27 4:13 ` [PATCH v5 5/9] target/arm: Make some MTE helpers widely available Gustavo Romero
@ 2024-06-27 4:13 ` Gustavo Romero
2024-06-27 6:05 ` Philippe Mathieu-Daudé
2024-06-27 4:13 ` [PATCH v5 7/9] gdbstub: Make get cpu and hex conversion functions non-internal Gustavo Romero
` (2 subsequent siblings)
8 siblings, 1 reply; 21+ messages in thread
From: Gustavo Romero @ 2024-06-27 4:13 UTC (permalink / raw)
To: qemu-devel, philmd, alex.bennee, richard.henderson
Cc: peter.maydell, gustavo.romero
Factor out the code used for setting the MTE TCF0 field from the prctl
code into a convenient function. Other subsystems, like gdbstub, need to
set this field as well, so keep it as a separate function to avoid
duplication and ensure consistency in how this field is set across the
board.
Signed-off-by: Gustavo Romero <gustavo.romero@linaro.org>
---
linux-user/aarch64/meson.build | 2 ++
linux-user/aarch64/mte_user_helper.c | 34 ++++++++++++++++++++++++++++
linux-user/aarch64/mte_user_helper.h | 25 ++++++++++++++++++++
linux-user/aarch64/target_prctl.h | 22 ++----------------
4 files changed, 63 insertions(+), 20 deletions(-)
create mode 100644 linux-user/aarch64/mte_user_helper.c
create mode 100644 linux-user/aarch64/mte_user_helper.h
diff --git a/linux-user/aarch64/meson.build b/linux-user/aarch64/meson.build
index 248c578d15..f75bb3cd75 100644
--- a/linux-user/aarch64/meson.build
+++ b/linux-user/aarch64/meson.build
@@ -9,3 +9,5 @@ vdso_le_inc = gen_vdso.process('vdso-le.so',
extra_args: ['-r', '__kernel_rt_sigreturn'])
linux_user_ss.add(when: 'TARGET_AARCH64', if_true: [vdso_be_inc, vdso_le_inc])
+
+linux_user_ss.add(when: 'TARGET_AARCH64', if_true: [files('mte_user_helper.c')])
diff --git a/linux-user/aarch64/mte_user_helper.c b/linux-user/aarch64/mte_user_helper.c
new file mode 100644
index 0000000000..8be6deaf03
--- /dev/null
+++ b/linux-user/aarch64/mte_user_helper.c
@@ -0,0 +1,34 @@
+/*
+ * ARM MemTag convenience functions.
+ *
+ * This code is licensed under the GNU GPL v2 or later.
+ *
+ * SPDX-License-Identifier: LGPL-2.1-or-later
+ */
+
+#include <sys/prctl.h>
+#include "mte_user_helper.h"
+
+void arm_set_mte_tcf0(CPUArchState *env, abi_long value)
+{
+ /*
+ * Write PR_MTE_TCF to SCTLR_EL1[TCF0].
+ *
+ * The kernel has a per-cpu configuration for the sysadmin,
+ * /sys/devices/system/cpu/cpu<N>/mte_tcf_preferred,
+ * which qemu does not implement.
+ *
+ * Because there is no performance difference between the modes, and
+ * because SYNC is most useful for debugging MTE errors, choose SYNC
+ * as the preferred mode. With this preference, and the way the API
+ * uses only two bits, there is no way for the program to select
+ * ASYMM mode.
+ */
+ unsigned tcf = 0;
+ if (value & PR_MTE_TCF_SYNC) {
+ tcf = 1;
+ } else if (value & PR_MTE_TCF_ASYNC) {
+ tcf = 2;
+ }
+ env->cp15.sctlr_el[1] = deposit64(env->cp15.sctlr_el[1], 38, 2, tcf);
+}
diff --git a/linux-user/aarch64/mte_user_helper.h b/linux-user/aarch64/mte_user_helper.h
new file mode 100644
index 0000000000..ee3f6b190a
--- /dev/null
+++ b/linux-user/aarch64/mte_user_helper.h
@@ -0,0 +1,25 @@
+/*
+ * ARM MemTag convenience functions.
+ *
+ * This code is licensed under the GNU GPL v2 or later.
+ *
+ * SPDX-License-Identifier: LGPL-2.1-or-later
+ */
+
+#ifndef AARCH64_MTE_USER_HELPER_H
+#define AARCH64_MTE USER_HELPER_H
+
+#include "qemu/osdep.h"
+#include "qemu.h"
+
+/**
+ * arm_set_mte_tcf0 - Set TCF0 field in SCTLR_EL1 register
+ * @env: The CPU environment
+ * @value: The value to be set for the Tag Check Fault in EL0 field.
+ *
+ * Only SYNC and ASYNC modes can be selected. If ASYMM mode is given, the SYNC
+ * mode is selected instead. So, there is no way to set the ASYMM mode.
+ */
+void arm_set_mte_tcf0(CPUArchState *env, abi_long value);
+
+#endif /* AARCH64_MTE_USER_HELPER_H */
diff --git a/linux-user/aarch64/target_prctl.h b/linux-user/aarch64/target_prctl.h
index aa8e203c15..ed75b9e4b5 100644
--- a/linux-user/aarch64/target_prctl.h
+++ b/linux-user/aarch64/target_prctl.h
@@ -7,6 +7,7 @@
#define AARCH64_TARGET_PRCTL_H
#include "target/arm/cpu-features.h"
+#include "mte_user_helper.h"
static abi_long do_prctl_sve_get_vl(CPUArchState *env)
{
@@ -173,26 +174,7 @@ static abi_long do_prctl_set_tagged_addr_ctrl(CPUArchState *env, abi_long arg2)
env->tagged_addr_enable = arg2 & PR_TAGGED_ADDR_ENABLE;
if (cpu_isar_feature(aa64_mte, cpu)) {
- /*
- * Write PR_MTE_TCF to SCTLR_EL1[TCF0].
- *
- * The kernel has a per-cpu configuration for the sysadmin,
- * /sys/devices/system/cpu/cpu<N>/mte_tcf_preferred,
- * which qemu does not implement.
- *
- * Because there is no performance difference between the modes, and
- * because SYNC is most useful for debugging MTE errors, choose SYNC
- * as the preferred mode. With this preference, and the way the API
- * uses only two bits, there is no way for the program to select
- * ASYMM mode.
- */
- unsigned tcf = 0;
- if (arg2 & PR_MTE_TCF_SYNC) {
- tcf = 1;
- } else if (arg2 & PR_MTE_TCF_ASYNC) {
- tcf = 2;
- }
- env->cp15.sctlr_el[1] = deposit64(env->cp15.sctlr_el[1], 38, 2, tcf);
+ arm_set_mte_tcf0(env, arg2);
/*
* Write PR_MTE_TAG to GCR_EL1[Exclude].
--
2.34.1
^ permalink raw reply related [flat|nested] 21+ messages in thread* Re: [PATCH v5 6/9] target/arm: Factor out code for setting MTE TCF0 field
2024-06-27 4:13 ` [PATCH v5 6/9] target/arm: Factor out code for setting MTE TCF0 field Gustavo Romero
@ 2024-06-27 6:05 ` Philippe Mathieu-Daudé
2024-06-28 5:20 ` Gustavo Romero
0 siblings, 1 reply; 21+ messages in thread
From: Philippe Mathieu-Daudé @ 2024-06-27 6:05 UTC (permalink / raw)
To: Gustavo Romero, qemu-devel, alex.bennee, richard.henderson; +Cc: peter.maydell
On 27/6/24 06:13, Gustavo Romero wrote:
> Factor out the code used for setting the MTE TCF0 field from the prctl
> code into a convenient function. Other subsystems, like gdbstub, need to
> set this field as well, so keep it as a separate function to avoid
> duplication and ensure consistency in how this field is set across the
> board.
>
> Signed-off-by: Gustavo Romero <gustavo.romero@linaro.org>
> ---
> linux-user/aarch64/meson.build | 2 ++
> linux-user/aarch64/mte_user_helper.c | 34 ++++++++++++++++++++++++++++
> linux-user/aarch64/mte_user_helper.h | 25 ++++++++++++++++++++
> linux-user/aarch64/target_prctl.h | 22 ++----------------
> 4 files changed, 63 insertions(+), 20 deletions(-)
> create mode 100644 linux-user/aarch64/mte_user_helper.c
> create mode 100644 linux-user/aarch64/mte_user_helper.h
> diff --git a/linux-user/aarch64/mte_user_helper.h b/linux-user/aarch64/mte_user_helper.h
> new file mode 100644
> index 0000000000..ee3f6b190a
> --- /dev/null
> +++ b/linux-user/aarch64/mte_user_helper.h
> @@ -0,0 +1,25 @@
> +/*
> + * ARM MemTag convenience functions.
> + *
> + * This code is licensed under the GNU GPL v2 or later.
> + *
> + * SPDX-License-Identifier: LGPL-2.1-or-later
> + */
> +
> +#ifndef AARCH64_MTE_USER_HELPER_H
> +#define AARCH64_MTE USER_HELPER_H
> +
> +#include "qemu/osdep.h"
https://www.qemu.org/docs/master/devel/style.html#include-directives
Do not include “qemu/osdep.h” from header files since the .c file
will have already included it.
> +#include "qemu.h"
"qemu.h" shouldn't be required neither.
Conditional to removing both lines:
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v5 6/9] target/arm: Factor out code for setting MTE TCF0 field
2024-06-27 6:05 ` Philippe Mathieu-Daudé
@ 2024-06-28 5:20 ` Gustavo Romero
2024-06-28 9:22 ` Philippe Mathieu-Daudé
0 siblings, 1 reply; 21+ messages in thread
From: Gustavo Romero @ 2024-06-28 5:20 UTC (permalink / raw)
To: Philippe Mathieu-Daudé, qemu-devel, alex.bennee,
richard.henderson
Cc: peter.maydell
Hi Phil,
On 6/27/24 3:05 AM, Philippe Mathieu-Daudé wrote:
> On 27/6/24 06:13, Gustavo Romero wrote:
>> Factor out the code used for setting the MTE TCF0 field from the prctl
>> code into a convenient function. Other subsystems, like gdbstub, need to
>> set this field as well, so keep it as a separate function to avoid
>> duplication and ensure consistency in how this field is set across the
>> board.
>>
>> Signed-off-by: Gustavo Romero <gustavo.romero@linaro.org>
>> ---
>> linux-user/aarch64/meson.build | 2 ++
>> linux-user/aarch64/mte_user_helper.c | 34 ++++++++++++++++++++++++++++
>> linux-user/aarch64/mte_user_helper.h | 25 ++++++++++++++++++++
>> linux-user/aarch64/target_prctl.h | 22 ++----------------
>> 4 files changed, 63 insertions(+), 20 deletions(-)
>> create mode 100644 linux-user/aarch64/mte_user_helper.c
>> create mode 100644 linux-user/aarch64/mte_user_helper.h
>
>
>> diff --git a/linux-user/aarch64/mte_user_helper.h b/linux-user/aarch64/mte_user_helper.h
>> new file mode 100644
>> index 0000000000..ee3f6b190a
>> --- /dev/null
>> +++ b/linux-user/aarch64/mte_user_helper.h
>> @@ -0,0 +1,25 @@
>> +/*
>> + * ARM MemTag convenience functions.
>> + *
>> + * This code is licensed under the GNU GPL v2 or later.
>> + *
>> + * SPDX-License-Identifier: LGPL-2.1-or-later
>> + */
>> +
>> +#ifndef AARCH64_MTE_USER_HELPER_H
>> +#define AARCH64_MTE USER_HELPER_H
>> +
>> +#include "qemu/osdep.h"
>
> https://www.qemu.org/docs/master/devel/style.html#include-directives
>
> Do not include “qemu/osdep.h” from header files since the .c file
> will have already included it.
>
>> +#include "qemu.h"
>
> "qemu.h" shouldn't be required neither.
If I remove qemu/osdep.h CPUArchState can't resolved. If I remove qemu.h
then abi_long can't be resolved. I'm in a tight corner here.
Cheers,
Gustavo
> Conditional to removing both lines:
>
> Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v5 6/9] target/arm: Factor out code for setting MTE TCF0 field
2024-06-28 5:20 ` Gustavo Romero
@ 2024-06-28 9:22 ` Philippe Mathieu-Daudé
0 siblings, 0 replies; 21+ messages in thread
From: Philippe Mathieu-Daudé @ 2024-06-28 9:22 UTC (permalink / raw)
To: Gustavo Romero, qemu-devel, alex.bennee, richard.henderson; +Cc: peter.maydell
On 28/6/24 07:20, Gustavo Romero wrote:
> Hi Phil,
>
> On 6/27/24 3:05 AM, Philippe Mathieu-Daudé wrote:
>> On 27/6/24 06:13, Gustavo Romero wrote:
>>> Factor out the code used for setting the MTE TCF0 field from the prctl
>>> code into a convenient function. Other subsystems, like gdbstub, need to
>>> set this field as well, so keep it as a separate function to avoid
>>> duplication and ensure consistency in how this field is set across the
>>> board.
>>>
>>> Signed-off-by: Gustavo Romero <gustavo.romero@linaro.org>
>>> ---
>>> linux-user/aarch64/meson.build | 2 ++
>>> linux-user/aarch64/mte_user_helper.c | 34 ++++++++++++++++++++++++++++
>>> linux-user/aarch64/mte_user_helper.h | 25 ++++++++++++++++++++
>>> linux-user/aarch64/target_prctl.h | 22 ++----------------
>>> 4 files changed, 63 insertions(+), 20 deletions(-)
>>> create mode 100644 linux-user/aarch64/mte_user_helper.c
>>> create mode 100644 linux-user/aarch64/mte_user_helper.h
>>
>>
>>> diff --git a/linux-user/aarch64/mte_user_helper.h
>>> b/linux-user/aarch64/mte_user_helper.h
>>> new file mode 100644
>>> index 0000000000..ee3f6b190a
>>> --- /dev/null
>>> +++ b/linux-user/aarch64/mte_user_helper.h
>>> @@ -0,0 +1,25 @@
>>> +/*
>>> + * ARM MemTag convenience functions.
>>> + *
>>> + * This code is licensed under the GNU GPL v2 or later.
>>> + *
>>> + * SPDX-License-Identifier: LGPL-2.1-or-later
>>> + */
>>> +
>>> +#ifndef AARCH64_MTE_USER_HELPER_H
>>> +#define AARCH64_MTE USER_HELPER_H
>>> +
>>> +#include "qemu/osdep.h"
>>
>> https://www.qemu.org/docs/master/devel/style.html#include-directives
>>
>> Do not include “qemu/osdep.h” from header files since the .c file
>> will have already included it.
>>
>>> +#include "qemu.h"
>>
>> "qemu.h" shouldn't be required neither.
>
> If I remove qemu/osdep.h CPUArchState can't resolved. If I remove qemu.h
> then abi_long can't be resolved. I'm in a tight corner here.
Does it work with "exec/cpu-all.h"?
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH v5 7/9] gdbstub: Make get cpu and hex conversion functions non-internal
2024-06-27 4:13 [PATCH v5 0/9] Add MTE stubs for aarch64 user mode Gustavo Romero
` (5 preceding siblings ...)
2024-06-27 4:13 ` [PATCH v5 6/9] target/arm: Factor out code for setting MTE TCF0 field Gustavo Romero
@ 2024-06-27 4:13 ` Gustavo Romero
2024-06-27 6:10 ` Philippe Mathieu-Daudé
2024-06-27 4:13 ` [PATCH v5 8/9] gdbstub: Add support for MTE in user mode Gustavo Romero
2024-06-27 4:13 ` [PATCH v5 9/9] tests/tcg/aarch64: Add MTE gdbstub tests Gustavo Romero
8 siblings, 1 reply; 21+ messages in thread
From: Gustavo Romero @ 2024-06-27 4:13 UTC (permalink / raw)
To: qemu-devel, philmd, alex.bennee, richard.henderson
Cc: peter.maydell, gustavo.romero
Make the gdb_first_attached_cpu and gdb_hextomem non-internal so they
are not confined to use only in gdbstub.c.
Signed-off-by: Gustavo Romero <gustavo.romero@linaro.org>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
---
gdbstub/internals.h | 2 --
include/exec/gdbstub.h | 5 +++++
include/gdbstub/commands.h | 6 ++++++
3 files changed, 11 insertions(+), 2 deletions(-)
diff --git a/gdbstub/internals.h b/gdbstub/internals.h
index 34121dc61a..81875abf5f 100644
--- a/gdbstub/internals.h
+++ b/gdbstub/internals.h
@@ -107,7 +107,6 @@ static inline int tohex(int v)
void gdb_put_strbuf(void);
int gdb_put_packet_binary(const char *buf, int len, bool dump);
-void gdb_hextomem(GByteArray *mem, const char *buf, int len);
void gdb_memtohex(GString *buf, const uint8_t *mem, int len);
void gdb_memtox(GString *buf, const char *mem, int len);
void gdb_read_byte(uint8_t ch);
@@ -130,7 +129,6 @@ bool gdb_got_immediate_ack(void);
/* utility helpers */
GDBProcess *gdb_get_process(uint32_t pid);
CPUState *gdb_get_first_cpu_in_process(GDBProcess *process);
-CPUState *gdb_first_attached_cpu(void);
void gdb_append_thread_id(CPUState *cpu, GString *buf);
int gdb_get_cpu_index(CPUState *cpu);
unsigned int gdb_get_max_cpus(void); /* both */
diff --git a/include/exec/gdbstub.h b/include/exec/gdbstub.h
index 1bd2c4ec2a..77e5ec9a5b 100644
--- a/include/exec/gdbstub.h
+++ b/include/exec/gdbstub.h
@@ -135,4 +135,9 @@ void gdb_set_stop_cpu(CPUState *cpu);
/* in gdbstub-xml.c, generated by scripts/feature_to_c.py */
extern const GDBFeature gdb_static_features[];
+/**
+ * Return the first attached CPU
+ */
+CPUState *gdb_first_attached_cpu(void);
+
#endif /* GDBSTUB_H */
diff --git a/include/gdbstub/commands.h b/include/gdbstub/commands.h
index 2204c3ddbe..914b6d7313 100644
--- a/include/gdbstub/commands.h
+++ b/include/gdbstub/commands.h
@@ -93,4 +93,10 @@ void gdb_extend_set_table(GdbCmdParseEntry *table, int size);
*/
void gdb_extend_qsupported_features(char *qsupported_features);
+/**
+ * Convert a hex string to bytes. Conversion is done per byte, so 2 hex digits
+ * are converted to 1 byte. Invalid hex digits are treated as 0 digits.
+ */
+void gdb_hextomem(GByteArray *mem, const char *buf, int len);
+
#endif /* GDBSTUB_COMMANDS_H */
--
2.34.1
^ permalink raw reply related [flat|nested] 21+ messages in thread* Re: [PATCH v5 7/9] gdbstub: Make get cpu and hex conversion functions non-internal
2024-06-27 4:13 ` [PATCH v5 7/9] gdbstub: Make get cpu and hex conversion functions non-internal Gustavo Romero
@ 2024-06-27 6:10 ` Philippe Mathieu-Daudé
2024-06-27 6:19 ` Philippe Mathieu-Daudé
2024-06-27 11:05 ` Alex Bennée
0 siblings, 2 replies; 21+ messages in thread
From: Philippe Mathieu-Daudé @ 2024-06-27 6:10 UTC (permalink / raw)
To: Gustavo Romero, qemu-devel, alex.bennee, richard.henderson; +Cc: peter.maydell
On 27/6/24 06:13, Gustavo Romero wrote:
> Make the gdb_first_attached_cpu and gdb_hextomem non-internal so they
> are not confined to use only in gdbstub.c.
>
> Signed-off-by: Gustavo Romero <gustavo.romero@linaro.org>
> Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
> ---
> gdbstub/internals.h | 2 --
> include/exec/gdbstub.h | 5 +++++
> include/gdbstub/commands.h | 6 ++++++
> 3 files changed, 11 insertions(+), 2 deletions(-)
> diff --git a/include/exec/gdbstub.h b/include/exec/gdbstub.h
> index 1bd2c4ec2a..77e5ec9a5b 100644
> --- a/include/exec/gdbstub.h
> +++ b/include/exec/gdbstub.h
> @@ -135,4 +135,9 @@ void gdb_set_stop_cpu(CPUState *cpu);
> /* in gdbstub-xml.c, generated by scripts/feature_to_c.py */
> extern const GDBFeature gdb_static_features[];
>
> +/**
> + * Return the first attached CPU
> + */
> +CPUState *gdb_first_attached_cpu(void);
Alex, it seems dubious to expose the API like that.
IMHO GdbCmdHandler should take a GDBRegisterState argument,
then this would become:
CPUState *gdb_first_attached_cpu(GDBRegisterState *s);
Regards,
Phil.
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v5 7/9] gdbstub: Make get cpu and hex conversion functions non-internal
2024-06-27 6:10 ` Philippe Mathieu-Daudé
@ 2024-06-27 6:19 ` Philippe Mathieu-Daudé
2024-06-27 11:05 ` Alex Bennée
1 sibling, 0 replies; 21+ messages in thread
From: Philippe Mathieu-Daudé @ 2024-06-27 6:19 UTC (permalink / raw)
To: Gustavo Romero, qemu-devel, alex.bennee, richard.henderson; +Cc: peter.maydell
On 27/6/24 08:10, Philippe Mathieu-Daudé wrote:
> On 27/6/24 06:13, Gustavo Romero wrote:
>> Make the gdb_first_attached_cpu and gdb_hextomem non-internal so they
>> are not confined to use only in gdbstub.c.
>>
>> Signed-off-by: Gustavo Romero <gustavo.romero@linaro.org>
>> Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
>> ---
>> gdbstub/internals.h | 2 --
>> include/exec/gdbstub.h | 5 +++++
>> include/gdbstub/commands.h | 6 ++++++
>> 3 files changed, 11 insertions(+), 2 deletions(-)
>
>
>> diff --git a/include/exec/gdbstub.h b/include/exec/gdbstub.h
>> index 1bd2c4ec2a..77e5ec9a5b 100644
>> --- a/include/exec/gdbstub.h
>> +++ b/include/exec/gdbstub.h
>> @@ -135,4 +135,9 @@ void gdb_set_stop_cpu(CPUState *cpu);
>> /* in gdbstub-xml.c, generated by scripts/feature_to_c.py */
>> extern const GDBFeature gdb_static_features[];
>> +/**
>> + * Return the first attached CPU
>> + */
>> +CPUState *gdb_first_attached_cpu(void);
>
> Alex, it seems dubious to expose the API like that.
>
> IMHO GdbCmdHandler should take a GDBRegisterState argument,
> then this would become:
>
> CPUState *gdb_first_attached_cpu(GDBRegisterState *s);
(With GDBRegisterState typedef being forward-declared).
That said, this can be done as another series on top...
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v5 7/9] gdbstub: Make get cpu and hex conversion functions non-internal
2024-06-27 6:10 ` Philippe Mathieu-Daudé
2024-06-27 6:19 ` Philippe Mathieu-Daudé
@ 2024-06-27 11:05 ` Alex Bennée
2024-06-27 12:26 ` Philippe Mathieu-Daudé
1 sibling, 1 reply; 21+ messages in thread
From: Alex Bennée @ 2024-06-27 11:05 UTC (permalink / raw)
To: Philippe Mathieu-Daudé
Cc: Gustavo Romero, qemu-devel, richard.henderson, peter.maydell
Philippe Mathieu-Daudé <philmd@linaro.org> writes:
> On 27/6/24 06:13, Gustavo Romero wrote:
>> Make the gdb_first_attached_cpu and gdb_hextomem non-internal so they
>> are not confined to use only in gdbstub.c.
>> Signed-off-by: Gustavo Romero <gustavo.romero@linaro.org>
>> Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
>> ---
>> gdbstub/internals.h | 2 --
>> include/exec/gdbstub.h | 5 +++++
>> include/gdbstub/commands.h | 6 ++++++
>> 3 files changed, 11 insertions(+), 2 deletions(-)
>
>
>> diff --git a/include/exec/gdbstub.h b/include/exec/gdbstub.h
>> index 1bd2c4ec2a..77e5ec9a5b 100644
>> --- a/include/exec/gdbstub.h
>> +++ b/include/exec/gdbstub.h
>> @@ -135,4 +135,9 @@ void gdb_set_stop_cpu(CPUState *cpu);
>> /* in gdbstub-xml.c, generated by scripts/feature_to_c.py */
>> extern const GDBFeature gdb_static_features[];
>> +/**
>> + * Return the first attached CPU
>> + */
>> +CPUState *gdb_first_attached_cpu(void);
>
> Alex, it seems dubious to expose the API like that.
>
> IMHO GdbCmdHandler should take a GDBRegisterState argument,
> then this would become:
>
> CPUState *gdb_first_attached_cpu(GDBRegisterState *s);
Maybe instead of exposing this we can use user_ctx for something? If we
look at handle_set_reg/handle_get_reg we can see that passes down
gdbserver_state.g_cpu down to the eventual helpers. We could define
something like:
--8<---------------cut here---------------start------------->8---
fixups to avoid get_first_cpu()
5 files changed, 25 insertions(+), 18 deletions(-)
gdbstub/internals.h | 1 +
include/exec/gdbstub.h | 5 -----
include/gdbstub/commands.h | 3 +++
gdbstub/gdbstub.c | 7 ++++++-
target/arm/gdbstub64.c | 27 +++++++++++++++------------
modified gdbstub/internals.h
@@ -129,6 +129,7 @@ bool gdb_got_immediate_ack(void);
/* utility helpers */
GDBProcess *gdb_get_process(uint32_t pid);
CPUState *gdb_get_first_cpu_in_process(GDBProcess *process);
+CPUState *gdb_first_attached_cpu(void);
void gdb_append_thread_id(CPUState *cpu, GString *buf);
int gdb_get_cpu_index(CPUState *cpu);
unsigned int gdb_get_max_cpus(void); /* both */
modified include/exec/gdbstub.h
@@ -135,9 +135,4 @@ void gdb_set_stop_cpu(CPUState *cpu);
/* in gdbstub-xml.c, generated by scripts/feature_to_c.py */
extern const GDBFeature gdb_static_features[];
-/**
- * Return the first attached CPU
- */
-CPUState *gdb_first_attached_cpu(void);
-
#endif /* GDBSTUB_H */
modified include/gdbstub/commands.h
@@ -54,6 +54,8 @@ typedef union GdbCmdVariant {
* "stop reply" packet. The list of commands that accept such response is
* defined at the GDB Remote Serial Protocol documentation. See:
* https://sourceware.org/gdb/onlinedocs/gdb/Stop-Reply-Packets.html#Stop-Reply-Packets.
+ *
+ * @need_cpu_context: pass current CPU to command via user_ctx.
*/
typedef struct GdbCmdParseEntry {
GdbCmdHandler handler;
@@ -61,6 +63,7 @@ typedef struct GdbCmdParseEntry {
bool cmd_startswith;
const char *schema;
bool allow_stop_reply;
+ bool need_cpu_context;
} GdbCmdParseEntry;
#define get_cmd_parsers(p) (&g_array_index(p, GdbCmdParseEntry, 0))
modified gdbstub/gdbstub.c
@@ -938,6 +938,7 @@ static bool process_string_cmd(const char *data,
for (i = 0; i < num_cmds; i++) {
const GdbCmdParseEntry *cmd = &cmds[i];
+ void *user_ctx = NULL;
g_assert(cmd->handler && cmd->cmd);
if ((cmd->cmd_startswith && !startswith(data, cmd->cmd)) ||
@@ -952,8 +953,12 @@ static bool process_string_cmd(const char *data,
}
}
+ if (cmd->need_cpu_context) {
+ user_ctx = (void *) gdbserver_state.g_cpu;
+ }
+
gdbserver_state.allow_stop_reply = cmd->allow_stop_reply;
- cmd->handler(params, NULL);
+ cmd->handler(params, user_ctx);
return true;
}
modified target/arm/gdbstub64.c
@@ -427,9 +427,9 @@ int aarch64_gdb_set_tag_ctl_reg(CPUState *cs, uint8_t *buf, int reg)
return 1;
}
-static void handle_q_memtag(GArray *params, G_GNUC_UNUSED void *user_ctx)
+static void handle_q_memtag(GArray *params, void *user_ctx)
{
- ARMCPU *cpu = ARM_CPU(gdb_first_attached_cpu());
+ ARMCPU *cpu = ARM_CPU(user_ctx);
CPUARMState *env = &cpu->env;
uint64_t addr = gdb_get_cmd_param(params, 0)->val_ull;
@@ -470,9 +470,9 @@ static void handle_q_memtag(GArray *params, G_GNUC_UNUSED void *user_ctx)
gdb_put_packet(str_buf->str);
}
-static void handle_q_isaddresstagged(GArray *params, G_GNUC_UNUSED void *user_ctx)
+static void handle_q_isaddresstagged(GArray *params, void *user_ctx)
{
- ARMCPU *cpu = ARM_CPU(gdb_first_attached_cpu());
+ ARMCPU *cpu = ARM_CPU(user_ctx);
CPUARMState *env = &cpu->env;
uint64_t addr = gdb_get_cmd_param(params, 0)->val_ull;
@@ -487,9 +487,9 @@ static void handle_q_isaddresstagged(GArray *params, G_GNUC_UNUSED void *user_ct
gdb_put_packet(reply);
}
-static void handle_Q_memtag(GArray *params, G_GNUC_UNUSED void *user_ctx)
+static void handle_Q_memtag(GArray *params, void *user_ctx)
{
- ARMCPU *cpu = ARM_CPU(gdb_first_attached_cpu());
+ ARMCPU *cpu = ARM_CPU(user_ctx);
CPUARMState *env = &cpu->env;
uint64_t start_addr = gdb_get_cmd_param(params, 0)->val_ull;
@@ -567,21 +567,24 @@ enum Command {
static GdbCmdParseEntry cmd_handler_table[NUM_CMDS] = {
[qMemTags] = {
.handler = handle_q_memtag,
- .cmd_startswith = 1,
+ .cmd_startswith = true,
.cmd = "MemTags:",
- .schema = "L,l:l0"
+ .schema = "L,l:l0",
+ .need_cpu_context = true,
},
[qIsAddressTagged] = {
.handler = handle_q_isaddresstagged,
- .cmd_startswith = 1,
+ .cmd_startswith = true,
.cmd = "IsAddressTagged:",
- .schema = "L0"
+ .schema = "L0",
+ .need_cpu_context = true,
},
[QMemTags] = {
.handler = handle_Q_memtag,
- .cmd_startswith = 1,
+ .cmd_startswith = true,
.cmd = "MemTags:",
- .schema = "L,l:l:s0"
+ .schema = "L,l:l:s0",
+ .need_cpu_context = true,
},
};
#endif /* CONFIG_USER_ONLY */
--8<---------------cut here---------------end--------------->8---
>
> Regards,
>
> Phil.
--
Alex Bennée
Virtualisation Tech Lead @ Linaro
^ permalink raw reply [flat|nested] 21+ messages in thread* Re: [PATCH v5 7/9] gdbstub: Make get cpu and hex conversion functions non-internal
2024-06-27 11:05 ` Alex Bennée
@ 2024-06-27 12:26 ` Philippe Mathieu-Daudé
2024-06-28 5:12 ` Gustavo Romero
0 siblings, 1 reply; 21+ messages in thread
From: Philippe Mathieu-Daudé @ 2024-06-27 12:26 UTC (permalink / raw)
To: Alex Bennée
Cc: Gustavo Romero, qemu-devel, richard.henderson, peter.maydell
On 27/6/24 13:05, Alex Bennée wrote:
> Philippe Mathieu-Daudé <philmd@linaro.org> writes:
>
>> On 27/6/24 06:13, Gustavo Romero wrote:
>>> Make the gdb_first_attached_cpu and gdb_hextomem non-internal so they
>>> are not confined to use only in gdbstub.c.
>>> Signed-off-by: Gustavo Romero <gustavo.romero@linaro.org>
>>> Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
>>> ---
>>> gdbstub/internals.h | 2 --
>>> include/exec/gdbstub.h | 5 +++++
>>> include/gdbstub/commands.h | 6 ++++++
>>> 3 files changed, 11 insertions(+), 2 deletions(-)
>>
>>
>>> diff --git a/include/exec/gdbstub.h b/include/exec/gdbstub.h
>>> index 1bd2c4ec2a..77e5ec9a5b 100644
>>> --- a/include/exec/gdbstub.h
>>> +++ b/include/exec/gdbstub.h
>>> @@ -135,4 +135,9 @@ void gdb_set_stop_cpu(CPUState *cpu);
>>> /* in gdbstub-xml.c, generated by scripts/feature_to_c.py */
>>> extern const GDBFeature gdb_static_features[];
>>> +/**
>>> + * Return the first attached CPU
>>> + */
>>> +CPUState *gdb_first_attached_cpu(void);
>>
>> Alex, it seems dubious to expose the API like that.
>>
>> IMHO GdbCmdHandler should take a GDBRegisterState argument,
>> then this would become:
>>
>> CPUState *gdb_first_attached_cpu(GDBRegisterState *s);
>
> Maybe instead of exposing this we can use user_ctx for something? If we
> look at handle_set_reg/handle_get_reg we can see that passes down
> gdbserver_state.g_cpu down to the eventual helpers. We could define
> something like:
>
> --8<---------------cut here---------------start------------->8---
> fixups to avoid get_first_cpu()
>
> 5 files changed, 25 insertions(+), 18 deletions(-)
> gdbstub/internals.h | 1 +
> include/exec/gdbstub.h | 5 -----
> include/gdbstub/commands.h | 3 +++
> gdbstub/gdbstub.c | 7 ++++++-
> target/arm/gdbstub64.c | 27 +++++++++++++++------------
> @@ -54,6 +54,8 @@ typedef union GdbCmdVariant {
> * "stop reply" packet. The list of commands that accept such response is
> * defined at the GDB Remote Serial Protocol documentation. See:
> * https://sourceware.org/gdb/onlinedocs/gdb/Stop-Reply-Packets.html#Stop-Reply-Packets.
> + *
> + * @need_cpu_context: pass current CPU to command via user_ctx.
> */
> typedef struct GdbCmdParseEntry {
> GdbCmdHandler handler;
> @@ -61,6 +63,7 @@ typedef struct GdbCmdParseEntry {
> bool cmd_startswith;
> const char *schema;
> bool allow_stop_reply;
> + bool need_cpu_context;
> } GdbCmdParseEntry;
>
> #define get_cmd_parsers(p) (&g_array_index(p, GdbCmdParseEntry, 0))
> modified gdbstub/gdbstub.c
> @@ -938,6 +938,7 @@ static bool process_string_cmd(const char *data,
>
> for (i = 0; i < num_cmds; i++) {
> const GdbCmdParseEntry *cmd = &cmds[i];
> + void *user_ctx = NULL;
> g_assert(cmd->handler && cmd->cmd);
>
> if ((cmd->cmd_startswith && !startswith(data, cmd->cmd)) ||
> @@ -952,8 +953,12 @@ static bool process_string_cmd(const char *data,
> }
> }
>
> + if (cmd->need_cpu_context) {
> + user_ctx = (void *) gdbserver_state.g_cpu;
LGTM.
> + }
> +
> gdbserver_state.allow_stop_reply = cmd->allow_stop_reply;
> - cmd->handler(params, NULL);
> + cmd->handler(params, user_ctx);
> return true;
> }
^ permalink raw reply [flat|nested] 21+ messages in thread* Re: [PATCH v5 7/9] gdbstub: Make get cpu and hex conversion functions non-internal
2024-06-27 12:26 ` Philippe Mathieu-Daudé
@ 2024-06-28 5:12 ` Gustavo Romero
0 siblings, 0 replies; 21+ messages in thread
From: Gustavo Romero @ 2024-06-28 5:12 UTC (permalink / raw)
To: Philippe Mathieu-Daudé, Alex Bennée
Cc: qemu-devel, richard.henderson, peter.maydell
Hi Phil, Alex,
On 6/27/24 9:26 AM, Philippe Mathieu-Daudé wrote:
> On 27/6/24 13:05, Alex Bennée wrote:
>> Philippe Mathieu-Daudé <philmd@linaro.org> writes:
>>
>>> On 27/6/24 06:13, Gustavo Romero wrote:
>>>> Make the gdb_first_attached_cpu and gdb_hextomem non-internal so they
>>>> are not confined to use only in gdbstub.c.
>>>> Signed-off-by: Gustavo Romero <gustavo.romero@linaro.org>
>>>> Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
>>>> ---
>>>> gdbstub/internals.h | 2 --
>>>> include/exec/gdbstub.h | 5 +++++
>>>> include/gdbstub/commands.h | 6 ++++++
>>>> 3 files changed, 11 insertions(+), 2 deletions(-)
>>>
>>>
>>>> diff --git a/include/exec/gdbstub.h b/include/exec/gdbstub.h
>>>> index 1bd2c4ec2a..77e5ec9a5b 100644
>>>> --- a/include/exec/gdbstub.h
>>>> +++ b/include/exec/gdbstub.h
>>>> @@ -135,4 +135,9 @@ void gdb_set_stop_cpu(CPUState *cpu);
>>>> /* in gdbstub-xml.c, generated by scripts/feature_to_c.py */
>>>> extern const GDBFeature gdb_static_features[];
>>>> +/**
>>>> + * Return the first attached CPU
>>>> + */
>>>> +CPUState *gdb_first_attached_cpu(void);
>>>
>>> Alex, it seems dubious to expose the API like that.
>>>
>>> IMHO GdbCmdHandler should take a GDBRegisterState argument,
>>> then this would become:
>>>
>>> CPUState *gdb_first_attached_cpu(GDBRegisterState *s);
>>
>> Maybe instead of exposing this we can use user_ctx for something? If we
>> look at handle_set_reg/handle_get_reg we can see that passes down
>> gdbserver_state.g_cpu down to the eventual helpers. We could define
>> something like:
>>
>> --8<---------------cut here---------------start------------->8---
>> fixups to avoid get_first_cpu()
>>
>> 5 files changed, 25 insertions(+), 18 deletions(-)
>> gdbstub/internals.h | 1 +
>> include/exec/gdbstub.h | 5 -----
>> include/gdbstub/commands.h | 3 +++
>> gdbstub/gdbstub.c | 7 ++++++-
>> target/arm/gdbstub64.c | 27 +++++++++++++++------------
>
>
>> @@ -54,6 +54,8 @@ typedef union GdbCmdVariant {
>> * "stop reply" packet. The list of commands that accept such response is
>> * defined at the GDB Remote Serial Protocol documentation. See:
>> * https://sourceware.org/gdb/onlinedocs/gdb/Stop-Reply-Packets.html#Stop-Reply-Packets.
>> + *
>> + * @need_cpu_context: pass current CPU to command via user_ctx.
>> */
>> typedef struct GdbCmdParseEntry {
>> GdbCmdHandler handler;
>> @@ -61,6 +63,7 @@ typedef struct GdbCmdParseEntry {
>> bool cmd_startswith;
>> const char *schema;
>> bool allow_stop_reply;
>> + bool need_cpu_context;
>> } GdbCmdParseEntry;
>> #define get_cmd_parsers(p) (&g_array_index(p, GdbCmdParseEntry, 0))
>> modified gdbstub/gdbstub.c
>> @@ -938,6 +938,7 @@ static bool process_string_cmd(const char *data,
>> for (i = 0; i < num_cmds; i++) {
>> const GdbCmdParseEntry *cmd = &cmds[i];
>> + void *user_ctx = NULL;
>> g_assert(cmd->handler && cmd->cmd);
>> if ((cmd->cmd_startswith && !startswith(data, cmd->cmd)) ||
>> @@ -952,8 +953,12 @@ static bool process_string_cmd(const char *data,
>> }
>> }
>> + if (cmd->need_cpu_context) {
>> + user_ctx = (void *) gdbserver_state.g_cpu;
>
> LGTM.
Thanks for the suggestion. I added it to v6.
Cheers,
Gustavo
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH v5 8/9] gdbstub: Add support for MTE in user mode
2024-06-27 4:13 [PATCH v5 0/9] Add MTE stubs for aarch64 user mode Gustavo Romero
` (6 preceding siblings ...)
2024-06-27 4:13 ` [PATCH v5 7/9] gdbstub: Make get cpu and hex conversion functions non-internal Gustavo Romero
@ 2024-06-27 4:13 ` Gustavo Romero
2024-06-27 4:13 ` [PATCH v5 9/9] tests/tcg/aarch64: Add MTE gdbstub tests Gustavo Romero
8 siblings, 0 replies; 21+ messages in thread
From: Gustavo Romero @ 2024-06-27 4:13 UTC (permalink / raw)
To: qemu-devel, philmd, alex.bennee, richard.henderson
Cc: peter.maydell, gustavo.romero
This commit implements the stubs to handle the qIsAddressTagged,
qMemTag, and QMemTag GDB packets, allowing all GDB 'memory-tag'
subcommands to work with QEMU gdbstub on aarch64 user mode. It also
implements the get/set functions for the special GDB MTE register
'tag_ctl', used to control the MTE fault type at runtime.
Signed-off-by: Gustavo Romero <gustavo.romero@linaro.org>
---
configs/targets/aarch64-linux-user.mak | 2 +-
gdb-xml/aarch64-mte.xml | 11 ++
target/arm/cpu.c | 1 +
target/arm/gdbstub.c | 46 ++++++
target/arm/gdbstub64.c | 220 +++++++++++++++++++++++++
target/arm/internals.h | 6 +
6 files changed, 285 insertions(+), 1 deletion(-)
create mode 100644 gdb-xml/aarch64-mte.xml
diff --git a/configs/targets/aarch64-linux-user.mak b/configs/targets/aarch64-linux-user.mak
index ba8bc5fe3f..8f0ed21d76 100644
--- a/configs/targets/aarch64-linux-user.mak
+++ b/configs/targets/aarch64-linux-user.mak
@@ -1,6 +1,6 @@
TARGET_ARCH=aarch64
TARGET_BASE_ARCH=arm
-TARGET_XML_FILES= gdb-xml/aarch64-core.xml gdb-xml/aarch64-fpu.xml gdb-xml/aarch64-pauth.xml
+TARGET_XML_FILES= gdb-xml/aarch64-core.xml gdb-xml/aarch64-fpu.xml gdb-xml/aarch64-pauth.xml gdb-xml/aarch64-mte.xml
TARGET_HAS_BFLT=y
CONFIG_SEMIHOSTING=y
CONFIG_ARM_COMPATIBLE_SEMIHOSTING=y
diff --git a/gdb-xml/aarch64-mte.xml b/gdb-xml/aarch64-mte.xml
new file mode 100644
index 0000000000..4b70b4f17a
--- /dev/null
+++ b/gdb-xml/aarch64-mte.xml
@@ -0,0 +1,11 @@
+<?xml version="1.0"?>
+<!-- Copyright (C) 2021-2023 Free Software Foundation, Inc.
+
+ Copying and distribution of this file, with or without modification,
+ are permitted in any medium without royalty provided the copyright
+ notice and this notice are preserved. -->
+
+<!DOCTYPE feature SYSTEM "gdb-target.dtd">
+<feature name="org.gnu.gdb.aarch64.mte">
+ <reg name="tag_ctl" bitsize="64" type="uint64" group="system" save-restore="no"/>
+</feature>
diff --git a/target/arm/cpu.c b/target/arm/cpu.c
index 35fa281f1b..14d4eca127 100644
--- a/target/arm/cpu.c
+++ b/target/arm/cpu.c
@@ -2518,6 +2518,7 @@ static void arm_cpu_realizefn(DeviceState *dev, Error **errp)
register_cp_regs_for_features(cpu);
arm_cpu_register_gdb_regs_for_features(cpu);
+ arm_cpu_register_gdb_commands(cpu);
init_cpreg_list(cpu);
diff --git a/target/arm/gdbstub.c b/target/arm/gdbstub.c
index a3bb73cfa7..c3a9b5eb1e 100644
--- a/target/arm/gdbstub.c
+++ b/target/arm/gdbstub.c
@@ -21,6 +21,7 @@
#include "cpu.h"
#include "exec/gdbstub.h"
#include "gdbstub/helpers.h"
+#include "gdbstub/commands.h"
#include "sysemu/tcg.h"
#include "internals.h"
#include "cpu-features.h"
@@ -474,6 +475,41 @@ static GDBFeature *arm_gen_dynamic_m_secextreg_feature(CPUState *cs,
#endif
#endif /* CONFIG_TCG */
+void arm_cpu_register_gdb_commands(ARMCPU *cpu)
+{
+ GArray *query_table =
+ g_array_new(FALSE, FALSE, sizeof(GdbCmdParseEntry));
+ GArray *set_table =
+ g_array_new(FALSE, FALSE, sizeof(GdbCmdParseEntry));
+ GString *qsupported_features = g_string_new(NULL);
+
+ if (arm_feature(&cpu->env, ARM_FEATURE_AARCH64)) {
+ #ifdef TARGET_AARCH64
+ aarch64_cpu_register_gdb_commands(cpu, qsupported_features, query_table,
+ set_table);
+ #endif
+ }
+
+ /* Set arch-specific handlers for 'q' commands. */
+ if (query_table->len) {
+ gdb_extend_query_table(&g_array_index(query_table,
+ GdbCmdParseEntry, 0),
+ query_table->len);
+ }
+
+ /* Set arch-specific handlers for 'Q' commands. */
+ if (set_table->len) {
+ gdb_extend_set_table(&g_array_index(set_table,
+ GdbCmdParseEntry, 0),
+ set_table->len);
+ }
+
+ /* Set arch-specific qSupported feature. */
+ if (qsupported_features->len) {
+ gdb_extend_qsupported_features(qsupported_features->str);
+ }
+}
+
void arm_cpu_register_gdb_regs_for_features(ARMCPU *cpu)
{
CPUState *cs = CPU(cpu);
@@ -507,6 +543,16 @@ void arm_cpu_register_gdb_regs_for_features(ARMCPU *cpu)
gdb_find_static_feature("aarch64-pauth.xml"),
0);
}
+
+#ifdef CONFIG_USER_ONLY
+ /* Memory Tagging Extension (MTE) 'tag_ctl' pseudo-register. */
+ if (cpu_isar_feature(aa64_mte, cpu)) {
+ gdb_register_coprocessor(cs, aarch64_gdb_get_tag_ctl_reg,
+ aarch64_gdb_set_tag_ctl_reg,
+ gdb_find_static_feature("aarch64-mte.xml"),
+ 0);
+ }
+#endif
#endif
} else {
if (arm_feature(env, ARM_FEATURE_NEON)) {
diff --git a/target/arm/gdbstub64.c b/target/arm/gdbstub64.c
index caa31ff3fa..8c63398f98 100644
--- a/target/arm/gdbstub64.c
+++ b/target/arm/gdbstub64.c
@@ -21,6 +21,12 @@
#include "cpu.h"
#include "internals.h"
#include "gdbstub/helpers.h"
+#include "gdbstub/commands.h"
+#include "tcg/mte_helper.h"
+#if defined(CONFIG_USER_ONLY) && defined(CONFIG_LINUX)
+#include <sys/prctl.h>
+#include "mte_user_helper.h"
+#endif
int aarch64_cpu_gdb_read_register(CPUState *cs, GByteArray *mem_buf, int n)
{
@@ -381,3 +387,217 @@ GDBFeature *arm_gen_dynamic_svereg_feature(CPUState *cs, int base_reg)
return &cpu->dyn_svereg_feature.desc;
}
+
+#ifdef CONFIG_USER_ONLY
+int aarch64_gdb_get_tag_ctl_reg(CPUState *cs, GByteArray *buf, int reg)
+{
+ ARMCPU *cpu = ARM_CPU(cs);
+ CPUARMState *env = &cpu->env;
+ uint64_t tcf0;
+
+ assert(reg == 0);
+
+ tcf0 = extract64(env->cp15.sctlr_el[1], 38, 2);
+
+ return gdb_get_reg64(buf, tcf0);
+}
+
+int aarch64_gdb_set_tag_ctl_reg(CPUState *cs, uint8_t *buf, int reg)
+{
+ ARMCPU *cpu = ARM_CPU(cs);
+ CPUARMState *env = &cpu->env;
+
+ uint8_t tcf;
+
+ assert(reg == 0);
+
+ tcf = *buf << PR_MTE_TCF_SHIFT;
+
+ if (!tcf) {
+ return 0;
+ }
+
+ /*
+ * 'tag_ctl' register is actually a "pseudo-register" provided by GDB to
+ * expose options regarding the type of MTE fault that can be controlled at
+ * runtime.
+ */
+ arm_set_mte_tcf0(env, tcf);
+
+ return 1;
+}
+
+static void handle_q_memtag(GArray *params, G_GNUC_UNUSED void *user_ctx)
+{
+ ARMCPU *cpu = ARM_CPU(gdb_first_attached_cpu());
+ CPUARMState *env = &cpu->env;
+
+ uint64_t addr = gdb_get_cmd_param(params, 0)->val_ull;
+ uint64_t len = gdb_get_cmd_param(params, 1)->val_ul;
+ int type = gdb_get_cmd_param(params, 2)->val_ul;
+
+ uint8_t *tags;
+ uint8_t addr_tag;
+
+ g_autoptr(GString) str_buf = g_string_new(NULL);
+
+ /*
+ * GDB does not query multiple tags for a memory range on remote targets, so
+ * that's not supported either by gdbstub.
+ */
+ if (len != 1) {
+ gdb_put_packet("E02");
+ }
+
+ /* GDB never queries a tag different from an allocation tag (type 1). */
+ if (type != 1) {
+ gdb_put_packet("E03");
+ }
+
+ /* Note that tags are packed here (2 tags packed in one byte). */
+ tags = allocation_tag_mem_probe(env, 0, addr, MMU_DATA_LOAD, 8 /* 64-bit */,
+ MMU_DATA_LOAD, true, 0);
+ if (!tags) {
+ /* Address is not in a tagged region. */
+ gdb_put_packet("E04");
+ return;
+ }
+
+ /* Unpack tag from byte. */
+ addr_tag = load_tag1(addr, tags);
+ g_string_printf(str_buf, "m%.2x", addr_tag);
+
+ gdb_put_packet(str_buf->str);
+}
+
+static void handle_q_isaddresstagged(GArray *params, G_GNUC_UNUSED void *user_ctx)
+{
+ ARMCPU *cpu = ARM_CPU(gdb_first_attached_cpu());
+ CPUARMState *env = &cpu->env;
+
+ uint64_t addr = gdb_get_cmd_param(params, 0)->val_ull;
+
+ uint8_t *tags;
+ const char *reply;
+
+ tags = allocation_tag_mem_probe(env, 0, addr, MMU_DATA_LOAD, 8 /* 64-bit */,
+ MMU_DATA_LOAD, true, 0);
+ reply = tags ? "01" : "00";
+
+ gdb_put_packet(reply);
+}
+
+static void handle_Q_memtag(GArray *params, G_GNUC_UNUSED void *user_ctx)
+{
+ ARMCPU *cpu = ARM_CPU(gdb_first_attached_cpu());
+ CPUARMState *env = &cpu->env;
+
+ uint64_t start_addr = gdb_get_cmd_param(params, 0)->val_ull;
+ uint64_t len = gdb_get_cmd_param(params, 1)->val_ul;
+ int type = gdb_get_cmd_param(params, 2)->val_ul;
+ char const *new_tags_str = gdb_get_cmd_param(params, 3)->data;
+
+ uint64_t end_addr;
+
+ int num_new_tags;
+ uint8_t *tags;
+
+ g_autoptr(GByteArray) new_tags = g_byte_array_new();
+
+ /*
+ * Only the allocation tag (i.e. type 1) can be set at the stub side.
+ */
+ if (type != 1) {
+ gdb_put_packet("E02");
+ return;
+ }
+
+ end_addr = start_addr + (len - 1); /* 'len' is always >= 1 */
+ /* Check if request's memory range does not cross page boundaries. */
+ if ((start_addr ^ end_addr) & TARGET_PAGE_MASK) {
+ gdb_put_packet("E03");
+ return;
+ }
+
+ /*
+ * Get all tags in the page starting from the tag of the start address.
+ * Note that there are two tags packed into a single byte here.
+ */
+ tags = allocation_tag_mem_probe(env, 0, start_addr, MMU_DATA_STORE,
+ 8 /* 64-bit */, MMU_DATA_STORE, true, 0);
+ if (!tags) {
+ /* Address is not in a tagged region. */
+ gdb_put_packet("E04");
+ return;
+ }
+
+ /* Convert tags provided by GDB, 2 hex digits per tag. */
+ num_new_tags = strlen(new_tags_str) / 2;
+ gdb_hextomem(new_tags, new_tags_str, num_new_tags);
+
+ uint64_t address = start_addr;
+ int new_tag_index = 0;
+ while (address <= end_addr) {
+ uint8_t new_tag;
+ int packed_index;
+
+ /*
+ * Find packed tag index from unpacked tag index. There are two tags
+ * in one packed index (one tag per nibble).
+ */
+ packed_index = new_tag_index / 2;
+
+ new_tag = new_tags->data[new_tag_index % num_new_tags];
+ store_tag1(address, tags + packed_index, new_tag);
+
+ address += TAG_GRANULE;
+ new_tag_index++;
+ }
+
+ gdb_put_packet("OK");
+}
+
+enum Command {
+ qMemTags,
+ qIsAddressTagged,
+ QMemTags,
+ NUM_CMDS
+};
+
+static GdbCmdParseEntry cmd_handler_table[NUM_CMDS] = {
+ [qMemTags] = {
+ .handler = handle_q_memtag,
+ .cmd_startswith = 1,
+ .cmd = "MemTags:",
+ .schema = "L,l:l0"
+ },
+ [qIsAddressTagged] = {
+ .handler = handle_q_isaddresstagged,
+ .cmd_startswith = 1,
+ .cmd = "IsAddressTagged:",
+ .schema = "L0"
+ },
+ [QMemTags] = {
+ .handler = handle_Q_memtag,
+ .cmd_startswith = 1,
+ .cmd = "MemTags:",
+ .schema = "L,l:l:s0"
+ },
+};
+#endif /* CONFIG_USER_ONLY */
+
+void aarch64_cpu_register_gdb_commands(ARMCPU *cpu, GString *qsupported,
+ GArray *qtable, GArray *stable)
+{
+#ifdef CONFIG_USER_ONLY
+ /* MTE */
+ if (cpu_isar_feature(aa64_mte, cpu)) {
+ g_string_append(qsupported, ";memory-tagging+");
+
+ g_array_append_val(qtable, cmd_handler_table[qMemTags]);
+ g_array_append_val(qtable, cmd_handler_table[qIsAddressTagged]);
+
+ g_array_append_val(stable, cmd_handler_table[QMemTags]);
+ }
+#endif
+}
diff --git a/target/arm/internals.h b/target/arm/internals.h
index 11b5da2562..e1aa1a63b9 100644
--- a/target/arm/internals.h
+++ b/target/arm/internals.h
@@ -358,6 +358,10 @@ void init_cpreg_list(ARMCPU *cpu);
void arm_cpu_register_gdb_regs_for_features(ARMCPU *cpu);
void arm_translate_init(void);
+void arm_cpu_register_gdb_commands(ARMCPU *cpu);
+void aarch64_cpu_register_gdb_commands(ARMCPU *cpu, GString *, GArray *,
+ GArray *);
+
void arm_restore_state_to_opc(CPUState *cs,
const TranslationBlock *tb,
const uint64_t *data);
@@ -1640,6 +1644,8 @@ int aarch64_gdb_get_fpu_reg(CPUState *cs, GByteArray *buf, int reg);
int aarch64_gdb_set_fpu_reg(CPUState *cs, uint8_t *buf, int reg);
int aarch64_gdb_get_pauth_reg(CPUState *cs, GByteArray *buf, int reg);
int aarch64_gdb_set_pauth_reg(CPUState *cs, uint8_t *buf, int reg);
+int aarch64_gdb_get_tag_ctl_reg(CPUState *cs, GByteArray *buf, int reg);
+int aarch64_gdb_set_tag_ctl_reg(CPUState *cs, uint8_t *buf, int reg);
void arm_cpu_sve_finalize(ARMCPU *cpu, Error **errp);
void arm_cpu_sme_finalize(ARMCPU *cpu, Error **errp);
void arm_cpu_pauth_finalize(ARMCPU *cpu, Error **errp);
--
2.34.1
^ permalink raw reply related [flat|nested] 21+ messages in thread* [PATCH v5 9/9] tests/tcg/aarch64: Add MTE gdbstub tests
2024-06-27 4:13 [PATCH v5 0/9] Add MTE stubs for aarch64 user mode Gustavo Romero
` (7 preceding siblings ...)
2024-06-27 4:13 ` [PATCH v5 8/9] gdbstub: Add support for MTE in user mode Gustavo Romero
@ 2024-06-27 4:13 ` Gustavo Romero
8 siblings, 0 replies; 21+ messages in thread
From: Gustavo Romero @ 2024-06-27 4:13 UTC (permalink / raw)
To: qemu-devel, philmd, alex.bennee, richard.henderson
Cc: peter.maydell, gustavo.romero
Add tests to exercise the MTE stubs. The tests will only run if a
version of GDB that supports MTE is available in the test environment.
Signed-off-by: Gustavo Romero <gustavo.romero@linaro.org>
---
configure | 4 ++
tests/tcg/aarch64/Makefile.target | 14 +++-
tests/tcg/aarch64/gdbstub/test-mte.py | 86 +++++++++++++++++++++++
tests/tcg/aarch64/mte-8.c | 98 +++++++++++++++++++++++++++
4 files changed, 201 insertions(+), 1 deletion(-)
create mode 100644 tests/tcg/aarch64/gdbstub/test-mte.py
create mode 100644 tests/tcg/aarch64/mte-8.c
diff --git a/configure b/configure
index 5ad1674ca5..10f7e1259a 100755
--- a/configure
+++ b/configure
@@ -1673,6 +1673,10 @@ for target in $target_list; do
echo "GDB=$gdb_bin" >> $config_target_mak
fi
+ if test "${arch}" = "aarch64" && version_ge ${gdb_version##* } 15.0; then
+ echo "GDB_HAS_MTE=y" >> $config_target_mak
+ fi
+
echo "run-tcg-tests-$target: $qemu\$(EXESUF)" >> Makefile.prereqs
tcg_tests_targets="$tcg_tests_targets $target"
fi
diff --git a/tests/tcg/aarch64/Makefile.target b/tests/tcg/aarch64/Makefile.target
index 70d728ae9a..f306e3d257 100644
--- a/tests/tcg/aarch64/Makefile.target
+++ b/tests/tcg/aarch64/Makefile.target
@@ -62,7 +62,7 @@ AARCH64_TESTS += bti-2
# MTE Tests
ifneq ($(CROSS_CC_HAS_ARMV8_MTE),)
-AARCH64_TESTS += mte-1 mte-2 mte-3 mte-4 mte-5 mte-6 mte-7
+AARCH64_TESTS += mte-1 mte-2 mte-3 mte-4 mte-5 mte-6 mte-7 mte-8
mte-%: CFLAGS += -march=armv8.5-a+memtag
endif
@@ -128,6 +128,18 @@ run-gdbstub-sve-ioctls: sve-ioctls
basic gdbstub SVE ZLEN support)
EXTRA_RUNS += run-gdbstub-sysregs run-gdbstub-sve-ioctls
+
+ifeq ($(GDB_HAS_MTE),y)
+run-gdbstub-mte: mte-8
+ $(call run-test, $@, $(GDB_SCRIPT) \
+ --gdb $(GDB) \
+ --qemu $(QEMU) --qargs "$(QEMU_OPTS)" \
+ --bin $< --test $(AARCH64_SRC)/gdbstub/test-mte.py, \
+ gdbstub MTE support)
+
+EXTRA_RUNS += run-gdbstub-mte
+endif
+
endif
endif
diff --git a/tests/tcg/aarch64/gdbstub/test-mte.py b/tests/tcg/aarch64/gdbstub/test-mte.py
new file mode 100644
index 0000000000..2db0663c1a
--- /dev/null
+++ b/tests/tcg/aarch64/gdbstub/test-mte.py
@@ -0,0 +1,86 @@
+from __future__ import print_function
+#
+# Test GDB memory-tag commands that exercise the stubs for the qIsAddressTagged,
+# qMemTag, and QMemTag packets. Logical tag-only commands rely on local
+# operations, hence don't exercise any stub.
+#
+# The test consists in breaking just after a atag() call (which sets the
+# allocation tag -- see mte-8.c for details) and setting/getting tags in
+# different memory locations and ranges starting at the address of the array
+# 'a'.
+#
+# This is launched via tests/guest-debug/run-test.py
+#
+
+
+import gdb
+import re
+from test_gdbstub import main, report
+
+
+PATTERN_0 = "Memory tags for address 0x[0-9a-f]+ match \(0x[0-9a-f]+\)."
+PATTERN_1 = ".*(0x[0-9a-f]+)"
+
+
+def run_test():
+ gdb.execute("break 95", False, True)
+ gdb.execute("continue", False, True)
+ try:
+ # Test if we can check correctly that the allocation tag for
+ # array 'a' matches the logical tag after atag() is called.
+ co = gdb.execute("memory-tag check a", False, True)
+ tags_match = re.findall(PATTERN_0, co, re.MULTILINE)
+ if tags_match:
+ report(True, f"{tags_match[0]}")
+ else:
+ report(False, "Logical and allocation tags don't match!")
+
+ # Test allocation tag 'set and print' commands. Commands on logical
+ # tags rely on local operation and so don't exercise any stub.
+
+ # Set the allocation tag for the first granule (16 bytes) of
+ # address starting at 'a' address to a known value, i.e. 0x04.
+ gdb.execute("memory-tag set-allocation-tag a 1 04", False, True)
+
+ # Then set the allocation tag for the second granule to a known
+ # value, i.e. 0x06. This tests that contiguous tag granules are
+ # set correct and don't run over each other.
+ gdb.execute("memory-tag set-allocation-tag a+16 1 06", False, True)
+
+ # Read the known values back and check if they remain the same.
+
+ co = gdb.execute("memory-tag print-allocation-tag a", False, True)
+ first_tag = re.match(PATTERN_1, co)[1]
+
+ co = gdb.execute("memory-tag print-allocation-tag a+16", False, True)
+ second_tag = re.match(PATTERN_1, co)[1]
+
+ if first_tag == "0x4" and second_tag == "0x6":
+ report(True, "Allocation tags are correctly set/printed.")
+ else:
+ report(False, "Can't set/print allocation tags!")
+
+ # Now test fill pattern by setting a whole page with a pattern.
+ gdb.execute("memory-tag set-allocation-tag a 4096 0a0b", False, True)
+
+ # And read back the tags of the last two granules in page so
+ # we also test if the pattern is set correctly up to the end of
+ # the page.
+ co = gdb.execute("memory-tag print-allocation-tag a+4096-32", False, True)
+ tag = re.match(PATTERN_1, co)[1]
+
+ co = gdb.execute("memory-tag print-allocation-tag a+4096-16", False, True)
+ last_tag = re.match(PATTERN_1, co)[1]
+
+ if tag == "0xa" and last_tag == "0xb":
+ report(True, "Fill pattern is ok.")
+ else:
+ report(False, "Fill pattern failed!")
+
+ except gdb.error:
+ # This usually happens because a GDB version that does not
+ # support memory tagging was used to run the test.
+ report(False, "'memory-tag' command failed!")
+
+
+main(run_test, expected_arch="aarch64")
diff --git a/tests/tcg/aarch64/mte-8.c b/tests/tcg/aarch64/mte-8.c
new file mode 100644
index 0000000000..9fffd7b737
--- /dev/null
+++ b/tests/tcg/aarch64/mte-8.c
@@ -0,0 +1,98 @@
+/*
+ * To be compiled with -march=armv8.5-a+memtag
+ *
+ * This test is adapted from a Linux test. Please see:
+ *
+ * https://www.kernel.org/doc/html/next/arch/arm64/memory-tagging-extension.html#example-of-correct-usage
+ */
+#include <errno.h>
+#include <stdint.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <unistd.h>
+#include <sys/auxv.h>
+#include <sys/mman.h>
+#include <sys/prctl.h>
+#include <string.h>
+/*
+ * From arch/arm64/include/uapi/asm/hwcap.h
+ */
+#define HWCAP2_MTE (1 << 18)
+
+/*
+ * From arch/arm64/include/uapi/asm/mman.h
+ */
+#define PROT_MTE 0x20
+
+/*
+ * Insert a random logical tag into the given pointer.
+ */
+#define insert_random_tag(ptr) ({ \
+ uint64_t __val; \
+ asm("irg %0, %1" : "=r" (__val) : "r" (ptr)); \
+ __val; \
+})
+
+/*
+ * Set the allocation tag on the destination address.
+ */
+#define set_tag(tagged_addr) do { \
+ asm volatile("stg %0, [%0]" : : "r" (tagged_addr) : "memory"); \
+} while (0)
+
+
+int main(int argc, char *argv[])
+{
+ unsigned char *a;
+ unsigned long page_sz = sysconf(_SC_PAGESIZE);
+ unsigned long hwcap2 = getauxval(AT_HWCAP2);
+
+ /* check if MTE is present */
+ if (!(hwcap2 & HWCAP2_MTE))
+ return EXIT_FAILURE;
+
+ /*
+ * Enable the tagged address ABI, synchronous or asynchronous MTE
+ * tag check faults (based on per-CPU preference) and allow all
+ * non-zero tags in the randomly generated set.
+ */
+ if (prctl(PR_SET_TAGGED_ADDR_CTRL,
+ PR_TAGGED_ADDR_ENABLE | PR_MTE_TCF_SYNC | PR_MTE_TCF_ASYNC |
+ (0xfffe << PR_MTE_TAG_SHIFT),
+ 0, 0, 0)) {
+ perror("prctl() failed");
+ return EXIT_FAILURE;
+ }
+
+ a = mmap(0, page_sz, PROT_READ | PROT_WRITE,
+ MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);
+ if (a == MAP_FAILED) {
+ perror("mmap() failed");
+ return EXIT_FAILURE;
+ }
+
+ printf("a[] address is %p\n", a);
+
+ /*
+ * Enable MTE on the above anonymous mmap. The flag could be passed
+ * directly to mmap() and skip this step.
+ */
+ if (mprotect(a, page_sz, PROT_READ | PROT_WRITE | PROT_MTE)) {
+ perror("mprotect() failed");
+ return EXIT_FAILURE;
+ }
+
+ /* access with the default tag (0) */
+ a[0] = 1;
+ a[1] = 2;
+
+ printf("a[0] = %hhu a[1] = %hhu\n", a[0], a[1]);
+
+ /* set the logical and allocation tags */
+ a = (unsigned char *)insert_random_tag(a);
+ set_tag(a);
+
+ printf("%p\n", a);
+
+ return 0;
+}
--
2.34.1
^ permalink raw reply related [flat|nested] 21+ messages in thread