[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v2 8/9] gdbstub: Add support for MTE in user mode
|
From: |
Alex Bennée |
|
Subject: |
Re: [PATCH v2 8/9] gdbstub: Add support for MTE in user mode |
|
Date: |
Fri, 14 Jun 2024 11:51:32 +0100 |
Gustavo Romero <gustavo.romero@linaro.org> writes:
> 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 | 253 +++++++++++++++++++++++++
> target/arm/internals.h | 2 +
> 5 files changed, 268 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..1cbcd6fa98 100644
> --- a/target/arm/gdbstub.c
> +++ b/target/arm/gdbstub.c
> @@ -21,10 +21,13 @@
> #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"
> #include "cpregs.h"
> +#include "mte.h"
> +#include "tcg/mte_helper.h"
>
> typedef struct RegisterSysregFeatureParam {
> CPUState *cs;
> @@ -474,6 +477,246 @@ static GDBFeature
> *arm_gen_dynamic_m_secextreg_feature(CPUState *cs,
> #endif
> #endif /* CONFIG_TCG */
>
> +#ifdef TARGET_AARCH64
> +#ifdef CONFIG_USER_ONLY
> +static int aarch64_gdb_get_tag_ctl_reg(CPUState *cs, struct _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);
> +}
> +
> +static 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.
> + */
> + 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 */
> +#endif /* TARGET_AARCH64 */
> +
> +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);
> +
> +#ifdef CONFIG_USER_ONLY
> + /* MTE */
> + if (cpu_isar_feature(aa64_mte3, cpu)) {
> + g_string_append(qsupported_features, ";memory-tagging+");
> +
> + g_array_append_val(query_table, cmd_handler_table[qMemTags]);
> + g_array_append_val(query_table, cmd_handler_table[qIsAddressTagged]);
> +
> + g_array_append_val(set_table, cmd_handler_table[QMemTags]);
> + }
> +#endif
This fails:
FAILED: libqemu-arm-linux-user.fa.p/target_arm_gdbstub.c.o
cc -m64 -Ilibqemu-arm-linux-user.fa.p -I. -I../.. -Itarget/arm
-I../../target/arm -I../../common-user/host/x86_64
-I../../linux-user/include/host/x86_64 -I../../linux-user/include -Ilinux-user
-I../../linux-user -Ilinux-user/arm -I../../linux-user/arm -Iqapi -Itrace -Iui
-Iui/shader -I/usr/include/capstone -I/usr/include/glib-2.0
-I/usr/lib/x86_64-linux-gnu/glib-2.0/include -fdiagnostics-color=auto -Wall
-Winvalid-pch -Werror -std=gnu11 -O2 -g -fstack-protector-strong -Wempty-body
-Wendif-labels -Wexpansion-to-defined -Wformat-security -Wformat-y2k
-Wignored-qualifiers -Wimplicit-fallthrough=2 -Winit-self
-Wmissing-format-attribute -Wmissing-prototypes -Wnested-externs
-Wold-style-declaration -Wold-style-definition -Wredundant-decls -Wshadow=local
-Wstrict-prototypes -Wtype-limits -Wundef -Wvla -Wwrite-strings
-Wno-missing-include-dirs -Wno-psabi -Wno-shift-negative-value -isystem
/home/alex/lsrc/qemu.git/linux-headers -isystem linux-headers -iquote . -iquote
/home/alex/lsrc/qemu.git -iquote /home/alex/lsrc/qemu.git/include -iquote
/home/alex/lsrc/qemu.git/host/include/x86_64 -iquote
/home/alex/lsrc/qemu.git/host/include/generic -iquote
/home/alex/lsrc/qemu.git/tcg/i386 -pthread -mcx16 -mpopcnt -msse4.2
-D_GNU_SOURCE -D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE -fno-strict-aliasing
-fno-common -fwrapv -ftrivial-auto-var-init=zero -fzero-call-used-regs=used-gpr
-fPIE -isystem../../linux-headers -isystemlinux-headers -DCOMPILING_PER_TARGET
'-DCONFIG_TARGET="arm-linux-user-config-target.h"'
'-DCONFIG_DEVICES="arm-linux-user-config-devices.h"' -MD -MQ
libqemu-arm-linux-user.fa.p/target_arm_gdbstub.c.o -MF
libqemu-arm-linux-user.fa.p/target_arm_gdbstub.c.o.d -o
libqemu-arm-linux-user.fa.p/target_arm_gdbstub.c.o -c ../../target/arm/gdbstub.c
In file included from /usr/include/glib-2.0/glib.h:33,
from /home/alex/lsrc/qemu.git/include/glib-compat.h:32,
from /home/alex/lsrc/qemu.git/include/qemu/osdep.h:161,
from ../../target/arm/gdbstub.c:20:
../../target/arm/gdbstub.c: In function ‘arm_cpu_register_gdb_commands’:
../../target/arm/gdbstub.c:693:41: error: ‘cmd_handler_table’ undeclared
(first use in this function)
693 | g_array_append_val(query_table, cmd_handler_table[qMemTags]);
| ^~~~~~~~~~~~~~~~~
/usr/include/glib-2.0/glib/garray.h:66:61: note: in definition of macro
‘g_array_append_val’
66 | #define g_array_append_val(a,v) g_array_append_vals (a, &(v), 1)
| ^
../../target/arm/gdbstub.c:693:41: note: each undeclared identifier is
reported only once for each function it appears in
693 | g_array_append_val(query_table, cmd_handler_table[qMemTags]);
| ^~~~~~~~~~~~~~~~~
/usr/include/glib-2.0/glib/garray.h:66:61: note: in definition of macro
‘g_array_append_val’
66 | #define g_array_append_val(a,v) g_array_append_vals (a, &(v), 1)
| ^
../../target/arm/gdbstub.c:693:59: error: ‘qMemTags’ undeclared (first use in
this function)
693 | g_array_append_val(query_table, cmd_handler_table[qMemTags]);
| ^~~~~~~~
/usr/include/glib-2.0/glib/garray.h:66:61: note: in definition of macro
‘g_array_append_val’
66 | #define g_array_append_val(a,v) g_array_append_vals (a, &(v), 1)
| ^
../../target/arm/gdbstub.c:694:59: error: ‘qIsAddressTagged’ undeclared
(first use in this function)
694 | g_array_append_val(query_table,
cmd_handler_table[qIsAddressTagged]);
|
^~~~~~~~~~~~~~~~
/usr/include/glib-2.0/glib/garray.h:66:61: note: in definition of macro
‘g_array_append_val’
66 | #define g_array_append_val(a,v) g_array_append_vals (a, &(v), 1)
| ^
../../target/arm/gdbstub.c:696:57: error: ‘QMemTags’ undeclared (first use in
this function)
696 | g_array_append_val(set_table, cmd_handler_table[QMemTags]);
| ^~~~~~~~
/usr/include/glib-2.0/glib/garray.h:66:61: note: in definition of macro
‘g_array_append_val’
66 | #define g_array_append_val(a,v) g_array_append_vals (a, &(v), 1)
| ^
In file included from ../../target/arm/gdbstub.c:29:
../../target/arm/mte.h: At top level:
../../target/arm/mte.h:27:13: error: ‘set_mte_tcf0’ defined but not used
[-Werror=unused-function]
27 | static void set_mte_tcf0(CPUArchState *env, abi_long value)
| ^~~~~~~~~~~~
cc1: all warnings being treated as errors
As the command handler table has a different set of ifdef protections to
the fill code. However I think it might be easier to move all these bits
over to gdbstub64.c which is implicitly TARGET_AARCH64.
I would suggest keeping arm_cpu_register_gdb_commands but add something
like:
if (arm_feature(env, ARM_FEATURE_AARCH64)) {
#ifdef TARGET_AARCH64
aarch64_cpu_register_gdb_commands(...)
#endif
}
to setup the Aarch64 MTE specific bits.
> +
> + /* 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 +750,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/internals.h b/target/arm/internals.h
> index 11b5da2562..e27a9fa1e0 100644
> --- a/target/arm/internals.h
> +++ b/target/arm/internals.h
> @@ -358,6 +358,8 @@ 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 arm_restore_state_to_opc(CPUState *cs,
> const TranslationBlock *tb,
> const uint64_t *data);
--
Alex Bennée
Virtualisation Tech Lead @ Linaro
[PATCH v2 7/9] gdbstub: Make get cpu and hex conversion functions non-internal, Gustavo Romero, 2024/06/13
[PATCH v2 8/9] gdbstub: Add support for MTE in user mode, Gustavo Romero, 2024/06/13
- Re: [PATCH v2 8/9] gdbstub: Add support for MTE in user mode,
Alex Bennée <=
[PATCH v2 9/9] tests/tcg/aarch64: Add MTE gdbstub tests, Gustavo Romero, 2024/06/13
Re: [PATCH v2 0/9] Add MTE stubs for aarch64 user mode, Alex Bennée, 2024/06/14