[PATCH v2 8/9] gdbstub: Add support for MTE in user mode

Gustavo Romero posted 9 patches 5 months, 2 weeks ago
There is a newer version of this series
[PATCH v2 8/9] gdbstub: Add support for MTE in user mode
Posted by Gustavo Romero 5 months, 2 weeks ago
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
+
+    /* 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);
-- 
2.34.1
Re: [PATCH v2 8/9] gdbstub: Add support for MTE in user mode
Posted by Alex Bennée 5 months, 1 week ago
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
Re: [PATCH v2 8/9] gdbstub: Add support for MTE in user mode
Posted by Gustavo Romero 5 months, 1 week ago
Hi Alex,

On 6/14/24 7:51 AM, Alex Bennée wrote:
> 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.

You got that error building for arm-linux-user, which I really tested
many times. I don't understand how that sneaked in. Anyways, let me
move it to gdbstub64.c.

Got a question tho, the qsupported string and the table pointers need
to be passed to aarch64_cpu_register_gdb_commands, besides 'cpu', so they
can be expanded in gdbstub64.c and later, if necessary, continue to
expanded in gdbstub.c, so it would be:

     aarch64_cpu_register_gdb_commands(cpu, qsupported_features, query_table, set_table)

Is that your suggestion?


Cheers,
Gustavo