* [PATCH v2 01/14] target/arm: Normalize aarch64 gdbstub get/set function names
2023-02-21 2:19 [PATCH v2 00/14] target/arm: gdbstub cleanups and additions Richard Henderson
@ 2023-02-21 2:19 ` Richard Henderson
2023-02-21 7:13 ` Philippe Mathieu-Daudé
2023-02-21 2:19 ` [PATCH v2 02/14] target/arm: Unexport arm_gen_dynamic_sysreg_xml Richard Henderson
` (12 subsequent siblings)
13 siblings, 1 reply; 28+ messages in thread
From: Richard Henderson @ 2023-02-21 2:19 UTC (permalink / raw)
To: qemu-devel; +Cc: qemu-arm, Fabiano Rosas
Make the form of the function names between fp and sve the same:
- arm_gdb_*_svereg -> aarch64_gdb_*_sve_reg.
- aarch64_fpu_gdb_*_reg -> aarch64_gdb_*_fpu_reg.
Reviewed-by: Fabiano Rosas <farosas@suse.de>
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
target/arm/internals.h | 8 ++++----
target/arm/gdbstub.c | 9 +++++----
target/arm/gdbstub64.c | 8 ++++----
3 files changed, 13 insertions(+), 12 deletions(-)
diff --git a/target/arm/internals.h b/target/arm/internals.h
index 759b70c646..121ecd420b 100644
--- a/target/arm/internals.h
+++ b/target/arm/internals.h
@@ -1326,10 +1326,10 @@ static inline uint64_t pmu_counter_mask(CPUARMState *env)
}
#ifdef TARGET_AARCH64
-int arm_gdb_get_svereg(CPUARMState *env, GByteArray *buf, int reg);
-int arm_gdb_set_svereg(CPUARMState *env, uint8_t *buf, int reg);
-int aarch64_fpu_gdb_get_reg(CPUARMState *env, GByteArray *buf, int reg);
-int aarch64_fpu_gdb_set_reg(CPUARMState *env, uint8_t *buf, int reg);
+int aarch64_gdb_get_sve_reg(CPUARMState *env, GByteArray *buf, int reg);
+int aarch64_gdb_set_sve_reg(CPUARMState *env, uint8_t *buf, int reg);
+int aarch64_gdb_get_fpu_reg(CPUARMState *env, GByteArray *buf, int reg);
+int aarch64_gdb_set_fpu_reg(CPUARMState *env, 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);
diff --git a/target/arm/gdbstub.c b/target/arm/gdbstub.c
index 2f806512d0..cf1c01e3cf 100644
--- a/target/arm/gdbstub.c
+++ b/target/arm/gdbstub.c
@@ -466,12 +466,13 @@ void arm_cpu_register_gdb_regs_for_features(ARMCPU *cpu)
*/
#ifdef TARGET_AARCH64
if (isar_feature_aa64_sve(&cpu->isar)) {
- gdb_register_coprocessor(cs, arm_gdb_get_svereg, arm_gdb_set_svereg,
- arm_gen_dynamic_svereg_xml(cs, cs->gdb_num_regs),
+ int nreg = arm_gen_dynamic_svereg_xml(cs, cs->gdb_num_regs);
+ gdb_register_coprocessor(cs, aarch64_gdb_get_sve_reg,
+ aarch64_gdb_set_sve_reg, nreg,
"sve-registers.xml", 0);
} else {
- gdb_register_coprocessor(cs, aarch64_fpu_gdb_get_reg,
- aarch64_fpu_gdb_set_reg,
+ gdb_register_coprocessor(cs, aarch64_gdb_get_fpu_reg,
+ aarch64_gdb_set_fpu_reg,
34, "aarch64-fpu.xml", 0);
}
#endif
diff --git a/target/arm/gdbstub64.c b/target/arm/gdbstub64.c
index 07a6746944..c598cb0375 100644
--- a/target/arm/gdbstub64.c
+++ b/target/arm/gdbstub64.c
@@ -72,7 +72,7 @@ int aarch64_cpu_gdb_write_register(CPUState *cs, uint8_t *mem_buf, int n)
return 0;
}
-int aarch64_fpu_gdb_get_reg(CPUARMState *env, GByteArray *buf, int reg)
+int aarch64_gdb_get_fpu_reg(CPUARMState *env, GByteArray *buf, int reg)
{
switch (reg) {
case 0 ... 31:
@@ -92,7 +92,7 @@ int aarch64_fpu_gdb_get_reg(CPUARMState *env, GByteArray *buf, int reg)
}
}
-int aarch64_fpu_gdb_set_reg(CPUARMState *env, uint8_t *buf, int reg)
+int aarch64_gdb_set_fpu_reg(CPUARMState *env, uint8_t *buf, int reg)
{
switch (reg) {
case 0 ... 31:
@@ -116,7 +116,7 @@ int aarch64_fpu_gdb_set_reg(CPUARMState *env, uint8_t *buf, int reg)
}
}
-int arm_gdb_get_svereg(CPUARMState *env, GByteArray *buf, int reg)
+int aarch64_gdb_get_sve_reg(CPUARMState *env, GByteArray *buf, int reg)
{
ARMCPU *cpu = env_archcpu(env);
@@ -164,7 +164,7 @@ int arm_gdb_get_svereg(CPUARMState *env, GByteArray *buf, int reg)
return 0;
}
-int arm_gdb_set_svereg(CPUARMState *env, uint8_t *buf, int reg)
+int aarch64_gdb_set_sve_reg(CPUARMState *env, uint8_t *buf, int reg)
{
ARMCPU *cpu = env_archcpu(env);
--
2.34.1
^ permalink raw reply related [flat|nested] 28+ messages in thread* Re: [PATCH v2 01/14] target/arm: Normalize aarch64 gdbstub get/set function names
2023-02-21 2:19 ` [PATCH v2 01/14] target/arm: Normalize aarch64 gdbstub get/set function names Richard Henderson
@ 2023-02-21 7:13 ` Philippe Mathieu-Daudé
0 siblings, 0 replies; 28+ messages in thread
From: Philippe Mathieu-Daudé @ 2023-02-21 7:13 UTC (permalink / raw)
To: Richard Henderson, qemu-devel; +Cc: qemu-arm, Fabiano Rosas
On 21/2/23 03:19, Richard Henderson wrote:
> Make the form of the function names between fp and sve the same:
> - arm_gdb_*_svereg -> aarch64_gdb_*_sve_reg.
> - aarch64_fpu_gdb_*_reg -> aarch64_gdb_*_fpu_reg.
>
> Reviewed-by: Fabiano Rosas <farosas@suse.de>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
> target/arm/internals.h | 8 ++++----
> target/arm/gdbstub.c | 9 +++++----
> target/arm/gdbstub64.c | 8 ++++----
> 3 files changed, 13 insertions(+), 12 deletions(-)
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
^ permalink raw reply [flat|nested] 28+ messages in thread
* [PATCH v2 02/14] target/arm: Unexport arm_gen_dynamic_sysreg_xml
2023-02-21 2:19 [PATCH v2 00/14] target/arm: gdbstub cleanups and additions Richard Henderson
2023-02-21 2:19 ` [PATCH v2 01/14] target/arm: Normalize aarch64 gdbstub get/set function names Richard Henderson
@ 2023-02-21 2:19 ` Richard Henderson
2023-02-21 7:14 ` Philippe Mathieu-Daudé
2023-02-21 2:19 ` [PATCH v2 03/14] target/arm: Move arm_gen_dynamic_svereg_xml to gdbstub64.c Richard Henderson
` (11 subsequent siblings)
13 siblings, 1 reply; 28+ messages in thread
From: Richard Henderson @ 2023-02-21 2:19 UTC (permalink / raw)
To: qemu-devel; +Cc: qemu-arm, Fabiano Rosas
This function is not used outside gdbstub.c.
Reviewed-by: Fabiano Rosas <farosas@suse.de>
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
target/arm/cpu.h | 1 -
target/arm/gdbstub.c | 2 +-
2 files changed, 1 insertion(+), 2 deletions(-)
diff --git a/target/arm/cpu.h b/target/arm/cpu.h
index 12b1082537..32ca6c9a0d 100644
--- a/target/arm/cpu.h
+++ b/target/arm/cpu.h
@@ -1116,7 +1116,6 @@ int arm_cpu_gdb_write_register(CPUState *cpu, uint8_t *buf, int reg);
* Helpers to dynamically generates XML descriptions of the sysregs
* and SVE registers. Returns the number of registers in each set.
*/
-int arm_gen_dynamic_sysreg_xml(CPUState *cpu, int base_reg);
int arm_gen_dynamic_svereg_xml(CPUState *cpu, int base_reg);
/* Returns the dynamically generated XML for the gdb stub.
diff --git a/target/arm/gdbstub.c b/target/arm/gdbstub.c
index cf1c01e3cf..52581e9784 100644
--- a/target/arm/gdbstub.c
+++ b/target/arm/gdbstub.c
@@ -305,7 +305,7 @@ static void arm_register_sysreg_for_xml(gpointer key, gpointer value,
}
}
-int arm_gen_dynamic_sysreg_xml(CPUState *cs, int base_reg)
+static int arm_gen_dynamic_sysreg_xml(CPUState *cs, int base_reg)
{
ARMCPU *cpu = ARM_CPU(cs);
GString *s = g_string_new(NULL);
--
2.34.1
^ permalink raw reply related [flat|nested] 28+ messages in thread* [PATCH v2 03/14] target/arm: Move arm_gen_dynamic_svereg_xml to gdbstub64.c
2023-02-21 2:19 [PATCH v2 00/14] target/arm: gdbstub cleanups and additions Richard Henderson
2023-02-21 2:19 ` [PATCH v2 01/14] target/arm: Normalize aarch64 gdbstub get/set function names Richard Henderson
2023-02-21 2:19 ` [PATCH v2 02/14] target/arm: Unexport arm_gen_dynamic_sysreg_xml Richard Henderson
@ 2023-02-21 2:19 ` Richard Henderson
2023-02-21 7:14 ` Philippe Mathieu-Daudé
2023-02-21 2:19 ` [PATCH v2 04/14] target/arm: Split out output_vector_union_type Richard Henderson
` (10 subsequent siblings)
13 siblings, 1 reply; 28+ messages in thread
From: Richard Henderson @ 2023-02-21 2:19 UTC (permalink / raw)
To: qemu-devel; +Cc: qemu-arm, Fabiano Rosas
The function is only used for aarch64, so move it to the
file that has the other aarch64 gdbstub stuff. Move the
declaration to internals.h.
Reviewed-by: Fabiano Rosas <farosas@suse.de>
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
target/arm/cpu.h | 6 ---
target/arm/internals.h | 1 +
target/arm/gdbstub.c | 120 -----------------------------------------
target/arm/gdbstub64.c | 118 ++++++++++++++++++++++++++++++++++++++++
4 files changed, 119 insertions(+), 126 deletions(-)
diff --git a/target/arm/cpu.h b/target/arm/cpu.h
index 32ca6c9a0d..059fe62eaa 100644
--- a/target/arm/cpu.h
+++ b/target/arm/cpu.h
@@ -1112,12 +1112,6 @@ hwaddr arm_cpu_get_phys_page_attrs_debug(CPUState *cpu, vaddr addr,
int arm_cpu_gdb_read_register(CPUState *cpu, GByteArray *buf, int reg);
int arm_cpu_gdb_write_register(CPUState *cpu, uint8_t *buf, int reg);
-/*
- * Helpers to dynamically generates XML descriptions of the sysregs
- * and SVE registers. Returns the number of registers in each set.
- */
-int arm_gen_dynamic_svereg_xml(CPUState *cpu, int base_reg);
-
/* Returns the dynamically generated XML for the gdb stub.
* Returns a pointer to the XML contents for the specified XML file or NULL
* if the XML name doesn't match the predefined one.
diff --git a/target/arm/internals.h b/target/arm/internals.h
index 121ecd420b..32b8562cbf 100644
--- a/target/arm/internals.h
+++ b/target/arm/internals.h
@@ -1326,6 +1326,7 @@ static inline uint64_t pmu_counter_mask(CPUARMState *env)
}
#ifdef TARGET_AARCH64
+int arm_gen_dynamic_svereg_xml(CPUState *cpu, int base_reg);
int aarch64_gdb_get_sve_reg(CPUARMState *env, GByteArray *buf, int reg);
int aarch64_gdb_set_sve_reg(CPUARMState *env, uint8_t *buf, int reg);
int aarch64_gdb_get_fpu_reg(CPUARMState *env, GByteArray *buf, int reg);
diff --git a/target/arm/gdbstub.c b/target/arm/gdbstub.c
index 52581e9784..bf8aff7824 100644
--- a/target/arm/gdbstub.c
+++ b/target/arm/gdbstub.c
@@ -322,126 +322,6 @@ static int arm_gen_dynamic_sysreg_xml(CPUState *cs, int base_reg)
return cpu->dyn_sysreg_xml.num;
}
-struct TypeSize {
- const char *gdb_type;
- int size;
- const char sz, suffix;
-};
-
-static const struct TypeSize vec_lanes[] = {
- /* quads */
- { "uint128", 128, 'q', 'u' },
- { "int128", 128, 'q', 's' },
- /* 64 bit */
- { "ieee_double", 64, 'd', 'f' },
- { "uint64", 64, 'd', 'u' },
- { "int64", 64, 'd', 's' },
- /* 32 bit */
- { "ieee_single", 32, 's', 'f' },
- { "uint32", 32, 's', 'u' },
- { "int32", 32, 's', 's' },
- /* 16 bit */
- { "ieee_half", 16, 'h', 'f' },
- { "uint16", 16, 'h', 'u' },
- { "int16", 16, 'h', 's' },
- /* bytes */
- { "uint8", 8, 'b', 'u' },
- { "int8", 8, 'b', 's' },
-};
-
-
-int arm_gen_dynamic_svereg_xml(CPUState *cs, int base_reg)
-{
- ARMCPU *cpu = ARM_CPU(cs);
- GString *s = g_string_new(NULL);
- DynamicGDBXMLInfo *info = &cpu->dyn_svereg_xml;
- g_autoptr(GString) ts = g_string_new("");
- int i, j, bits, reg_width = (cpu->sve_max_vq * 128);
- info->num = 0;
- g_string_printf(s, "<?xml version=\"1.0\"?>");
- g_string_append_printf(s, "<!DOCTYPE target SYSTEM \"gdb-target.dtd\">");
- g_string_append_printf(s, "<feature name=\"org.gnu.gdb.aarch64.sve\">");
-
- /* First define types and totals in a whole VL */
- for (i = 0; i < ARRAY_SIZE(vec_lanes); i++) {
- int count = reg_width / vec_lanes[i].size;
- g_string_printf(ts, "svev%c%c", vec_lanes[i].sz, vec_lanes[i].suffix);
- g_string_append_printf(s,
- "<vector id=\"%s\" type=\"%s\" count=\"%d\"/>",
- ts->str, vec_lanes[i].gdb_type, count);
- }
- /*
- * Now define a union for each size group containing unsigned and
- * signed and potentially float versions of each size from 128 to
- * 8 bits.
- */
- for (bits = 128, i = 0; bits >= 8; bits /= 2, i++) {
- const char suf[] = { 'q', 'd', 's', 'h', 'b' };
- g_string_append_printf(s, "<union id=\"svevn%c\">", suf[i]);
- for (j = 0; j < ARRAY_SIZE(vec_lanes); j++) {
- if (vec_lanes[j].size == bits) {
- g_string_append_printf(s, "<field name=\"%c\" type=\"svev%c%c\"/>",
- vec_lanes[j].suffix,
- vec_lanes[j].sz, vec_lanes[j].suffix);
- }
- }
- g_string_append(s, "</union>");
- }
- /* And now the final union of unions */
- g_string_append(s, "<union id=\"svev\">");
- for (bits = 128, i = 0; bits >= 8; bits /= 2, i++) {
- const char suf[] = { 'q', 'd', 's', 'h', 'b' };
- g_string_append_printf(s, "<field name=\"%c\" type=\"svevn%c\"/>",
- suf[i], suf[i]);
- }
- g_string_append(s, "</union>");
-
- /* Finally the sve prefix type */
- g_string_append_printf(s,
- "<vector id=\"svep\" type=\"uint8\" count=\"%d\"/>",
- reg_width / 8);
-
- /* Then define each register in parts for each vq */
- for (i = 0; i < 32; i++) {
- g_string_append_printf(s,
- "<reg name=\"z%d\" bitsize=\"%d\""
- " regnum=\"%d\" type=\"svev\"/>",
- i, reg_width, base_reg++);
- info->num++;
- }
- /* fpscr & status registers */
- g_string_append_printf(s, "<reg name=\"fpsr\" bitsize=\"32\""
- " regnum=\"%d\" group=\"float\""
- " type=\"int\"/>", base_reg++);
- g_string_append_printf(s, "<reg name=\"fpcr\" bitsize=\"32\""
- " regnum=\"%d\" group=\"float\""
- " type=\"int\"/>", base_reg++);
- info->num += 2;
-
- for (i = 0; i < 16; i++) {
- g_string_append_printf(s,
- "<reg name=\"p%d\" bitsize=\"%d\""
- " regnum=\"%d\" type=\"svep\"/>",
- i, cpu->sve_max_vq * 16, base_reg++);
- info->num++;
- }
- g_string_append_printf(s,
- "<reg name=\"ffr\" bitsize=\"%d\""
- " regnum=\"%d\" group=\"vector\""
- " type=\"svep\"/>",
- cpu->sve_max_vq * 16, base_reg++);
- g_string_append_printf(s,
- "<reg name=\"vg\" bitsize=\"64\""
- " regnum=\"%d\" type=\"int\"/>",
- base_reg++);
- info->num += 2;
- g_string_append_printf(s, "</feature>");
- cpu->dyn_svereg_xml.desc = g_string_free(s, false);
-
- return cpu->dyn_svereg_xml.num;
-}
-
-
const char *arm_gdb_get_dynamic_xml(CPUState *cs, const char *xmlname)
{
ARMCPU *cpu = ARM_CPU(cs);
diff --git a/target/arm/gdbstub64.c b/target/arm/gdbstub64.c
index c598cb0375..59fb5465d5 100644
--- a/target/arm/gdbstub64.c
+++ b/target/arm/gdbstub64.c
@@ -209,3 +209,121 @@ int aarch64_gdb_set_sve_reg(CPUARMState *env, uint8_t *buf, int reg)
return 0;
}
+
+struct TypeSize {
+ const char *gdb_type;
+ short size;
+ char sz, suffix;
+};
+
+static const struct TypeSize vec_lanes[] = {
+ /* quads */
+ { "uint128", 128, 'q', 'u' },
+ { "int128", 128, 'q', 's' },
+ /* 64 bit */
+ { "ieee_double", 64, 'd', 'f' },
+ { "uint64", 64, 'd', 'u' },
+ { "int64", 64, 'd', 's' },
+ /* 32 bit */
+ { "ieee_single", 32, 's', 'f' },
+ { "uint32", 32, 's', 'u' },
+ { "int32", 32, 's', 's' },
+ /* 16 bit */
+ { "ieee_half", 16, 'h', 'f' },
+ { "uint16", 16, 'h', 'u' },
+ { "int16", 16, 'h', 's' },
+ /* bytes */
+ { "uint8", 8, 'b', 'u' },
+ { "int8", 8, 'b', 's' },
+};
+
+int arm_gen_dynamic_svereg_xml(CPUState *cs, int base_reg)
+{
+ ARMCPU *cpu = ARM_CPU(cs);
+ GString *s = g_string_new(NULL);
+ DynamicGDBXMLInfo *info = &cpu->dyn_svereg_xml;
+ g_autoptr(GString) ts = g_string_new("");
+ int i, j, bits, reg_width = (cpu->sve_max_vq * 128);
+ info->num = 0;
+ g_string_printf(s, "<?xml version=\"1.0\"?>");
+ g_string_append_printf(s, "<!DOCTYPE target SYSTEM \"gdb-target.dtd\">");
+ g_string_append_printf(s, "<feature name=\"org.gnu.gdb.aarch64.sve\">");
+
+ /* First define types and totals in a whole VL */
+ for (i = 0; i < ARRAY_SIZE(vec_lanes); i++) {
+ int count = reg_width / vec_lanes[i].size;
+ g_string_printf(ts, "svev%c%c", vec_lanes[i].sz, vec_lanes[i].suffix);
+ g_string_append_printf(s,
+ "<vector id=\"%s\" type=\"%s\" count=\"%d\"/>",
+ ts->str, vec_lanes[i].gdb_type, count);
+ }
+ /*
+ * Now define a union for each size group containing unsigned and
+ * signed and potentially float versions of each size from 128 to
+ * 8 bits.
+ */
+ for (bits = 128, i = 0; bits >= 8; bits /= 2, i++) {
+ const char suf[] = { 'q', 'd', 's', 'h', 'b' };
+ g_string_append_printf(s, "<union id=\"svevn%c\">", suf[i]);
+ for (j = 0; j < ARRAY_SIZE(vec_lanes); j++) {
+ if (vec_lanes[j].size == bits) {
+ g_string_append_printf(s, "<field name=\"%c\" type=\"svev%c%c\"/>",
+ vec_lanes[j].suffix,
+ vec_lanes[j].sz, vec_lanes[j].suffix);
+ }
+ }
+ g_string_append(s, "</union>");
+ }
+ /* And now the final union of unions */
+ g_string_append(s, "<union id=\"svev\">");
+ for (bits = 128, i = 0; bits >= 8; bits /= 2, i++) {
+ const char suf[] = { 'q', 'd', 's', 'h', 'b' };
+ g_string_append_printf(s, "<field name=\"%c\" type=\"svevn%c\"/>",
+ suf[i], suf[i]);
+ }
+ g_string_append(s, "</union>");
+
+ /* Finally the sve prefix type */
+ g_string_append_printf(s,
+ "<vector id=\"svep\" type=\"uint8\" count=\"%d\"/>",
+ reg_width / 8);
+
+ /* Then define each register in parts for each vq */
+ for (i = 0; i < 32; i++) {
+ g_string_append_printf(s,
+ "<reg name=\"z%d\" bitsize=\"%d\""
+ " regnum=\"%d\" type=\"svev\"/>",
+ i, reg_width, base_reg++);
+ info->num++;
+ }
+ /* fpscr & status registers */
+ g_string_append_printf(s, "<reg name=\"fpsr\" bitsize=\"32\""
+ " regnum=\"%d\" group=\"float\""
+ " type=\"int\"/>", base_reg++);
+ g_string_append_printf(s, "<reg name=\"fpcr\" bitsize=\"32\""
+ " regnum=\"%d\" group=\"float\""
+ " type=\"int\"/>", base_reg++);
+ info->num += 2;
+
+ for (i = 0; i < 16; i++) {
+ g_string_append_printf(s,
+ "<reg name=\"p%d\" bitsize=\"%d\""
+ " regnum=\"%d\" type=\"svep\"/>",
+ i, cpu->sve_max_vq * 16, base_reg++);
+ info->num++;
+ }
+ g_string_append_printf(s,
+ "<reg name=\"ffr\" bitsize=\"%d\""
+ " regnum=\"%d\" group=\"vector\""
+ " type=\"svep\"/>",
+ cpu->sve_max_vq * 16, base_reg++);
+ g_string_append_printf(s,
+ "<reg name=\"vg\" bitsize=\"64\""
+ " regnum=\"%d\" type=\"int\"/>",
+ base_reg++);
+ info->num += 2;
+ g_string_append_printf(s, "</feature>");
+ info->desc = g_string_free(s, false);
+
+ return info->num;
+}
--
2.34.1
^ permalink raw reply related [flat|nested] 28+ messages in thread* Re: [PATCH v2 03/14] target/arm: Move arm_gen_dynamic_svereg_xml to gdbstub64.c
2023-02-21 2:19 ` [PATCH v2 03/14] target/arm: Move arm_gen_dynamic_svereg_xml to gdbstub64.c Richard Henderson
@ 2023-02-21 7:14 ` Philippe Mathieu-Daudé
0 siblings, 0 replies; 28+ messages in thread
From: Philippe Mathieu-Daudé @ 2023-02-21 7:14 UTC (permalink / raw)
To: Richard Henderson, qemu-devel; +Cc: qemu-arm, Fabiano Rosas
On 21/2/23 03:19, Richard Henderson wrote:
> The function is only used for aarch64, so move it to the
> file that has the other aarch64 gdbstub stuff. Move the
> declaration to internals.h.
>
> Reviewed-by: Fabiano Rosas <farosas@suse.de>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
> target/arm/cpu.h | 6 ---
> target/arm/internals.h | 1 +
> target/arm/gdbstub.c | 120 -----------------------------------------
> target/arm/gdbstub64.c | 118 ++++++++++++++++++++++++++++++++++++++++
> 4 files changed, 119 insertions(+), 126 deletions(-)
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
^ permalink raw reply [flat|nested] 28+ messages in thread
* [PATCH v2 04/14] target/arm: Split out output_vector_union_type
2023-02-21 2:19 [PATCH v2 00/14] target/arm: gdbstub cleanups and additions Richard Henderson
` (2 preceding siblings ...)
2023-02-21 2:19 ` [PATCH v2 03/14] target/arm: Move arm_gen_dynamic_svereg_xml to gdbstub64.c Richard Henderson
@ 2023-02-21 2:19 ` Richard Henderson
2023-02-21 2:19 ` [PATCH v2 05/14] target/arm: Simplify register counting in arm_gen_dynamic_svereg_xml Richard Henderson
` (9 subsequent siblings)
13 siblings, 0 replies; 28+ messages in thread
From: Richard Henderson @ 2023-02-21 2:19 UTC (permalink / raw)
To: qemu-devel; +Cc: qemu-arm, Fabiano Rosas, Peter Maydell
Create a subroutine for creating the union of unions
of the various type sizes that a vector may contain.
Reviewed-by: Fabiano Rosas <farosas@suse.de>
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
target/arm/gdbstub64.c | 83 +++++++++++++++++++++++-------------------
1 file changed, 45 insertions(+), 38 deletions(-)
diff --git a/target/arm/gdbstub64.c b/target/arm/gdbstub64.c
index 59fb5465d5..811833d8de 100644
--- a/target/arm/gdbstub64.c
+++ b/target/arm/gdbstub64.c
@@ -210,44 +210,39 @@ int aarch64_gdb_set_sve_reg(CPUARMState *env, uint8_t *buf, int reg)
return 0;
}
-struct TypeSize {
- const char *gdb_type;
- short size;
- char sz, suffix;
-};
-
-static const struct TypeSize vec_lanes[] = {
- /* quads */
- { "uint128", 128, 'q', 'u' },
- { "int128", 128, 'q', 's' },
- /* 64 bit */
- { "ieee_double", 64, 'd', 'f' },
- { "uint64", 64, 'd', 'u' },
- { "int64", 64, 'd', 's' },
- /* 32 bit */
- { "ieee_single", 32, 's', 'f' },
- { "uint32", 32, 's', 'u' },
- { "int32", 32, 's', 's' },
- /* 16 bit */
- { "ieee_half", 16, 'h', 'f' },
- { "uint16", 16, 'h', 'u' },
- { "int16", 16, 'h', 's' },
- /* bytes */
- { "uint8", 8, 'b', 'u' },
- { "int8", 8, 'b', 's' },
-};
-
-int arm_gen_dynamic_svereg_xml(CPUState *cs, int base_reg)
+static void output_vector_union_type(GString *s, int reg_width)
{
- ARMCPU *cpu = ARM_CPU(cs);
- GString *s = g_string_new(NULL);
- DynamicGDBXMLInfo *info = &cpu->dyn_svereg_xml;
+ struct TypeSize {
+ const char *gdb_type;
+ short size;
+ char sz, suffix;
+ };
+
+ static const struct TypeSize vec_lanes[] = {
+ /* quads */
+ { "uint128", 128, 'q', 'u' },
+ { "int128", 128, 'q', 's' },
+ /* 64 bit */
+ { "ieee_double", 64, 'd', 'f' },
+ { "uint64", 64, 'd', 'u' },
+ { "int64", 64, 'd', 's' },
+ /* 32 bit */
+ { "ieee_single", 32, 's', 'f' },
+ { "uint32", 32, 's', 'u' },
+ { "int32", 32, 's', 's' },
+ /* 16 bit */
+ { "ieee_half", 16, 'h', 'f' },
+ { "uint16", 16, 'h', 'u' },
+ { "int16", 16, 'h', 's' },
+ /* bytes */
+ { "uint8", 8, 'b', 'u' },
+ { "int8", 8, 'b', 's' },
+ };
+
+ static const char suf[] = { 'q', 'd', 's', 'h', 'b' };
+
g_autoptr(GString) ts = g_string_new("");
- int i, j, bits, reg_width = (cpu->sve_max_vq * 128);
- info->num = 0;
- g_string_printf(s, "<?xml version=\"1.0\"?>");
- g_string_append_printf(s, "<!DOCTYPE target SYSTEM \"gdb-target.dtd\">");
- g_string_append_printf(s, "<feature name=\"org.gnu.gdb.aarch64.sve\">");
+ int i, j, bits;
/* First define types and totals in a whole VL */
for (i = 0; i < ARRAY_SIZE(vec_lanes); i++) {
@@ -263,7 +258,6 @@ int arm_gen_dynamic_svereg_xml(CPUState *cs, int base_reg)
* 8 bits.
*/
for (bits = 128, i = 0; bits >= 8; bits /= 2, i++) {
- const char suf[] = { 'q', 'd', 's', 'h', 'b' };
g_string_append_printf(s, "<union id=\"svevn%c\">", suf[i]);
for (j = 0; j < ARRAY_SIZE(vec_lanes); j++) {
if (vec_lanes[j].size == bits) {
@@ -277,11 +271,24 @@ int arm_gen_dynamic_svereg_xml(CPUState *cs, int base_reg)
/* And now the final union of unions */
g_string_append(s, "<union id=\"svev\">");
for (bits = 128, i = 0; bits >= 8; bits /= 2, i++) {
- const char suf[] = { 'q', 'd', 's', 'h', 'b' };
g_string_append_printf(s, "<field name=\"%c\" type=\"svevn%c\"/>",
suf[i], suf[i]);
}
g_string_append(s, "</union>");
+}
+
+int arm_gen_dynamic_svereg_xml(CPUState *cs, int base_reg)
+{
+ ARMCPU *cpu = ARM_CPU(cs);
+ GString *s = g_string_new(NULL);
+ DynamicGDBXMLInfo *info = &cpu->dyn_svereg_xml;
+ int i, reg_width = (cpu->sve_max_vq * 128);
+ info->num = 0;
+ g_string_printf(s, "<?xml version=\"1.0\"?>");
+ g_string_append_printf(s, "<!DOCTYPE target SYSTEM \"gdb-target.dtd\">");
+ g_string_append_printf(s, "<feature name=\"org.gnu.gdb.aarch64.sve\">");
+
+ output_vector_union_type(s, reg_width);
/* Finally the sve prefix type */
g_string_append_printf(s,
--
2.34.1
^ permalink raw reply related [flat|nested] 28+ messages in thread* [PATCH v2 05/14] target/arm: Simplify register counting in arm_gen_dynamic_svereg_xml
2023-02-21 2:19 [PATCH v2 00/14] target/arm: gdbstub cleanups and additions Richard Henderson
` (3 preceding siblings ...)
2023-02-21 2:19 ` [PATCH v2 04/14] target/arm: Split out output_vector_union_type Richard Henderson
@ 2023-02-21 2:19 ` Richard Henderson
2023-02-21 7:16 ` Philippe Mathieu-Daudé
2023-02-21 2:19 ` [PATCH v2 06/14] target/arm: Hoist pred_width " Richard Henderson
` (8 subsequent siblings)
13 siblings, 1 reply; 28+ messages in thread
From: Richard Henderson @ 2023-02-21 2:19 UTC (permalink / raw)
To: qemu-devel; +Cc: qemu-arm
Rather than increment base_reg and num, compute num
from the change to base_reg at the end. Clean up some
nearby comments.
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
target/arm/gdbstub64.c | 27 ++++++++++++++++-----------
1 file changed, 16 insertions(+), 11 deletions(-)
diff --git a/target/arm/gdbstub64.c b/target/arm/gdbstub64.c
index 811833d8de..070ba20d99 100644
--- a/target/arm/gdbstub64.c
+++ b/target/arm/gdbstub64.c
@@ -277,32 +277,35 @@ static void output_vector_union_type(GString *s, int reg_width)
g_string_append(s, "</union>");
}
-int arm_gen_dynamic_svereg_xml(CPUState *cs, int base_reg)
+int arm_gen_dynamic_svereg_xml(CPUState *cs, int orig_base_reg)
{
ARMCPU *cpu = ARM_CPU(cs);
GString *s = g_string_new(NULL);
DynamicGDBXMLInfo *info = &cpu->dyn_svereg_xml;
- int i, reg_width = (cpu->sve_max_vq * 128);
- info->num = 0;
+ int reg_width = cpu->sve_max_vq * 128;
+ int base_reg = orig_base_reg;
+ int i;
+
g_string_printf(s, "<?xml version=\"1.0\"?>");
g_string_append_printf(s, "<!DOCTYPE target SYSTEM \"gdb-target.dtd\">");
g_string_append_printf(s, "<feature name=\"org.gnu.gdb.aarch64.sve\">");
+ /* Create the vector union type. */
output_vector_union_type(s, reg_width);
- /* Finally the sve prefix type */
+ /* Create the predicate vector type. */
g_string_append_printf(s,
"<vector id=\"svep\" type=\"uint8\" count=\"%d\"/>",
reg_width / 8);
- /* Then define each register in parts for each vq */
+ /* Define the vector registers. */
for (i = 0; i < 32; i++) {
g_string_append_printf(s,
"<reg name=\"z%d\" bitsize=\"%d\""
" regnum=\"%d\" type=\"svev\"/>",
i, reg_width, base_reg++);
- info->num++;
}
+
/* fpscr & status registers */
g_string_append_printf(s, "<reg name=\"fpsr\" bitsize=\"32\""
" regnum=\"%d\" group=\"float\""
@@ -310,27 +313,29 @@ int arm_gen_dynamic_svereg_xml(CPUState *cs, int base_reg)
g_string_append_printf(s, "<reg name=\"fpcr\" bitsize=\"32\""
" regnum=\"%d\" group=\"float\""
" type=\"int\"/>", base_reg++);
- info->num += 2;
+ /* Define the predicate registers. */
for (i = 0; i < 16; i++) {
g_string_append_printf(s,
"<reg name=\"p%d\" bitsize=\"%d\""
" regnum=\"%d\" type=\"svep\"/>",
i, cpu->sve_max_vq * 16, base_reg++);
- info->num++;
}
g_string_append_printf(s,
"<reg name=\"ffr\" bitsize=\"%d\""
" regnum=\"%d\" group=\"vector\""
" type=\"svep\"/>",
cpu->sve_max_vq * 16, base_reg++);
+
+ /* Define the vector length pseudo-register. */
g_string_append_printf(s,
"<reg name=\"vg\" bitsize=\"64\""
" regnum=\"%d\" type=\"int\"/>",
base_reg++);
- info->num += 2;
- g_string_append_printf(s, "</feature>");
- info->desc = g_string_free(s, false);
+ g_string_append_printf(s, "</feature>");
+
+ info->desc = g_string_free(s, false);
+ info->num = base_reg - orig_base_reg;
return info->num;
}
--
2.34.1
^ permalink raw reply related [flat|nested] 28+ messages in thread* [PATCH v2 06/14] target/arm: Hoist pred_width in arm_gen_dynamic_svereg_xml
2023-02-21 2:19 [PATCH v2 00/14] target/arm: gdbstub cleanups and additions Richard Henderson
` (4 preceding siblings ...)
2023-02-21 2:19 ` [PATCH v2 05/14] target/arm: Simplify register counting in arm_gen_dynamic_svereg_xml Richard Henderson
@ 2023-02-21 2:19 ` Richard Henderson
2023-02-21 7:16 ` Philippe Mathieu-Daudé
2023-02-21 2:19 ` [PATCH v2 07/14] target/arm: Fix svep width " Richard Henderson
` (7 subsequent siblings)
13 siblings, 1 reply; 28+ messages in thread
From: Richard Henderson @ 2023-02-21 2:19 UTC (permalink / raw)
To: qemu-devel; +Cc: qemu-arm, Fabiano Rosas
Reviewed-by: Fabiano Rosas <farosas@suse.de>
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
target/arm/gdbstub64.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/target/arm/gdbstub64.c b/target/arm/gdbstub64.c
index 070ba20d99..895e19f084 100644
--- a/target/arm/gdbstub64.c
+++ b/target/arm/gdbstub64.c
@@ -283,6 +283,7 @@ int arm_gen_dynamic_svereg_xml(CPUState *cs, int orig_base_reg)
GString *s = g_string_new(NULL);
DynamicGDBXMLInfo *info = &cpu->dyn_svereg_xml;
int reg_width = cpu->sve_max_vq * 128;
+ int pred_width = cpu->sve_max_vq * 16;
int base_reg = orig_base_reg;
int i;
@@ -319,13 +320,13 @@ int arm_gen_dynamic_svereg_xml(CPUState *cs, int orig_base_reg)
g_string_append_printf(s,
"<reg name=\"p%d\" bitsize=\"%d\""
" regnum=\"%d\" type=\"svep\"/>",
- i, cpu->sve_max_vq * 16, base_reg++);
+ i, pred_width, base_reg++);
}
g_string_append_printf(s,
"<reg name=\"ffr\" bitsize=\"%d\""
" regnum=\"%d\" group=\"vector\""
" type=\"svep\"/>",
- cpu->sve_max_vq * 16, base_reg++);
+ pred_width, base_reg++);
/* Define the vector length pseudo-register. */
g_string_append_printf(s,
--
2.34.1
^ permalink raw reply related [flat|nested] 28+ messages in thread* [PATCH v2 07/14] target/arm: Fix svep width in arm_gen_dynamic_svereg_xml
2023-02-21 2:19 [PATCH v2 00/14] target/arm: gdbstub cleanups and additions Richard Henderson
` (5 preceding siblings ...)
2023-02-21 2:19 ` [PATCH v2 06/14] target/arm: Hoist pred_width " Richard Henderson
@ 2023-02-21 2:19 ` Richard Henderson
2023-02-21 2:19 ` [PATCH v2 08/14] target/arm: Add name argument to output_vector_union_type Richard Henderson
` (6 subsequent siblings)
13 siblings, 0 replies; 28+ messages in thread
From: Richard Henderson @ 2023-02-21 2:19 UTC (permalink / raw)
To: qemu-devel; +Cc: qemu-arm, Peter Maydell
Define svep based on the size of the predicates,
not the primary vector registers.
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
target/arm/gdbstub64.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/target/arm/gdbstub64.c b/target/arm/gdbstub64.c
index 895e19f084..d0e1305f6f 100644
--- a/target/arm/gdbstub64.c
+++ b/target/arm/gdbstub64.c
@@ -297,7 +297,7 @@ int arm_gen_dynamic_svereg_xml(CPUState *cs, int orig_base_reg)
/* Create the predicate vector type. */
g_string_append_printf(s,
"<vector id=\"svep\" type=\"uint8\" count=\"%d\"/>",
- reg_width / 8);
+ pred_width / 8);
/* Define the vector registers. */
for (i = 0; i < 32; i++) {
--
2.34.1
^ permalink raw reply related [flat|nested] 28+ messages in thread* [PATCH v2 08/14] target/arm: Add name argument to output_vector_union_type
2023-02-21 2:19 [PATCH v2 00/14] target/arm: gdbstub cleanups and additions Richard Henderson
` (6 preceding siblings ...)
2023-02-21 2:19 ` [PATCH v2 07/14] target/arm: Fix svep width " Richard Henderson
@ 2023-02-21 2:19 ` Richard Henderson
2023-02-21 7:18 ` Philippe Mathieu-Daudé
2023-02-21 2:19 ` [PATCH v2 09/14] target/arm: Simplify iteration over bit widths Richard Henderson
` (5 subsequent siblings)
13 siblings, 1 reply; 28+ messages in thread
From: Richard Henderson @ 2023-02-21 2:19 UTC (permalink / raw)
To: qemu-devel; +Cc: qemu-arm, Peter Maydell
This will make the function usable between SVE and SME.
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
target/arm/gdbstub64.c | 28 ++++++++++++++--------------
1 file changed, 14 insertions(+), 14 deletions(-)
diff --git a/target/arm/gdbstub64.c b/target/arm/gdbstub64.c
index d0e1305f6f..36166bf81e 100644
--- a/target/arm/gdbstub64.c
+++ b/target/arm/gdbstub64.c
@@ -210,7 +210,8 @@ int aarch64_gdb_set_sve_reg(CPUARMState *env, uint8_t *buf, int reg)
return 0;
}
-static void output_vector_union_type(GString *s, int reg_width)
+static void output_vector_union_type(GString *s, int reg_width,
+ const char *name)
{
struct TypeSize {
const char *gdb_type;
@@ -240,39 +241,38 @@ static void output_vector_union_type(GString *s, int reg_width)
};
static const char suf[] = { 'q', 'd', 's', 'h', 'b' };
-
- g_autoptr(GString) ts = g_string_new("");
int i, j, bits;
/* First define types and totals in a whole VL */
for (i = 0; i < ARRAY_SIZE(vec_lanes); i++) {
- int count = reg_width / vec_lanes[i].size;
- g_string_printf(ts, "svev%c%c", vec_lanes[i].sz, vec_lanes[i].suffix);
g_string_append_printf(s,
- "<vector id=\"%s\" type=\"%s\" count=\"%d\"/>",
- ts->str, vec_lanes[i].gdb_type, count);
+ "<vector id=\"%s%c%c\" type=\"%s\" count=\"%d\"/>",
+ name, vec_lanes[i].sz, vec_lanes[i].suffix,
+ vec_lanes[i].gdb_type, reg_width / vec_lanes[i].size);
}
+
/*
* Now define a union for each size group containing unsigned and
* signed and potentially float versions of each size from 128 to
* 8 bits.
*/
for (bits = 128, i = 0; bits >= 8; bits /= 2, i++) {
- g_string_append_printf(s, "<union id=\"svevn%c\">", suf[i]);
+ g_string_append_printf(s, "<union id=\"%sn%c\">", name, suf[i]);
for (j = 0; j < ARRAY_SIZE(vec_lanes); j++) {
if (vec_lanes[j].size == bits) {
- g_string_append_printf(s, "<field name=\"%c\" type=\"svev%c%c\"/>",
- vec_lanes[j].suffix,
+ g_string_append_printf(s, "<field name=\"%c\" type=\"%s%c%c\"/>",
+ vec_lanes[j].suffix, name,
vec_lanes[j].sz, vec_lanes[j].suffix);
}
}
g_string_append(s, "</union>");
}
+
/* And now the final union of unions */
- g_string_append(s, "<union id=\"svev\">");
+ g_string_append_printf(s, "<union id=\"%s\">", name);
for (bits = 128, i = 0; bits >= 8; bits /= 2, i++) {
- g_string_append_printf(s, "<field name=\"%c\" type=\"svevn%c\"/>",
- suf[i], suf[i]);
+ g_string_append_printf(s, "<field name=\"%c\" type=\"%sn%c\"/>",
+ suf[i], name, suf[i]);
}
g_string_append(s, "</union>");
}
@@ -292,7 +292,7 @@ int arm_gen_dynamic_svereg_xml(CPUState *cs, int orig_base_reg)
g_string_append_printf(s, "<feature name=\"org.gnu.gdb.aarch64.sve\">");
/* Create the vector union type. */
- output_vector_union_type(s, reg_width);
+ output_vector_union_type(s, reg_width, "svev");
/* Create the predicate vector type. */
g_string_append_printf(s,
--
2.34.1
^ permalink raw reply related [flat|nested] 28+ messages in thread* [PATCH v2 09/14] target/arm: Simplify iteration over bit widths
2023-02-21 2:19 [PATCH v2 00/14] target/arm: gdbstub cleanups and additions Richard Henderson
` (7 preceding siblings ...)
2023-02-21 2:19 ` [PATCH v2 08/14] target/arm: Add name argument to output_vector_union_type Richard Henderson
@ 2023-02-21 2:19 ` Richard Henderson
2023-02-21 2:19 ` [PATCH v2 10/14] target/arm: Create pauth_ptr_mask Richard Henderson
` (4 subsequent siblings)
13 siblings, 0 replies; 28+ messages in thread
From: Richard Henderson @ 2023-02-21 2:19 UTC (permalink / raw)
To: qemu-devel; +Cc: qemu-arm, Peter Maydell
Order suf[] by the log8 of the width.
Use ARRAY_SIZE instead of hard-coding 128.
This changes the order of the union definitions,
but retains the order of the union-of-union members.
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
target/arm/gdbstub64.c | 10 ++++++----
1 file changed, 6 insertions(+), 4 deletions(-)
diff --git a/target/arm/gdbstub64.c b/target/arm/gdbstub64.c
index 36166bf81e..3d9e9e97c8 100644
--- a/target/arm/gdbstub64.c
+++ b/target/arm/gdbstub64.c
@@ -240,8 +240,8 @@ static void output_vector_union_type(GString *s, int reg_width,
{ "int8", 8, 'b', 's' },
};
- static const char suf[] = { 'q', 'd', 's', 'h', 'b' };
- int i, j, bits;
+ static const char suf[] = { 'b', 'h', 's', 'd', 'q' };
+ int i, j;
/* First define types and totals in a whole VL */
for (i = 0; i < ARRAY_SIZE(vec_lanes); i++) {
@@ -256,7 +256,9 @@ static void output_vector_union_type(GString *s, int reg_width,
* signed and potentially float versions of each size from 128 to
* 8 bits.
*/
- for (bits = 128, i = 0; bits >= 8; bits /= 2, i++) {
+ for (i = 0; i < ARRAY_SIZE(suf); i++) {
+ int bits = 8 << i;
+
g_string_append_printf(s, "<union id=\"%sn%c\">", name, suf[i]);
for (j = 0; j < ARRAY_SIZE(vec_lanes); j++) {
if (vec_lanes[j].size == bits) {
@@ -270,7 +272,7 @@ static void output_vector_union_type(GString *s, int reg_width,
/* And now the final union of unions */
g_string_append_printf(s, "<union id=\"%s\">", name);
- for (bits = 128, i = 0; bits >= 8; bits /= 2, i++) {
+ for (i = ARRAY_SIZE(suf) - 1; i >= 0; i--) {
g_string_append_printf(s, "<field name=\"%c\" type=\"%sn%c\"/>",
suf[i], name, suf[i]);
}
--
2.34.1
^ permalink raw reply related [flat|nested] 28+ messages in thread* [PATCH v2 10/14] target/arm: Create pauth_ptr_mask
2023-02-21 2:19 [PATCH v2 00/14] target/arm: gdbstub cleanups and additions Richard Henderson
` (8 preceding siblings ...)
2023-02-21 2:19 ` [PATCH v2 09/14] target/arm: Simplify iteration over bit widths Richard Henderson
@ 2023-02-21 2:19 ` Richard Henderson
2023-02-21 2:19 ` [PATCH v2 11/14] target/arm: Implement gdbstub pauth extension Richard Henderson
` (3 subsequent siblings)
13 siblings, 0 replies; 28+ messages in thread
From: Richard Henderson @ 2023-02-21 2:19 UTC (permalink / raw)
To: qemu-devel; +Cc: qemu-arm, Peter Maydell
Keep the logic for pauth within pauth_helper.c, and expose
a helper function for use with the gdbstub pac extension.
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
target/arm/internals.h | 10 ++++++++++
target/arm/pauth_helper.c | 26 ++++++++++++++++++++++----
2 files changed, 32 insertions(+), 4 deletions(-)
diff --git a/target/arm/internals.h b/target/arm/internals.h
index 32b8562cbf..370655061e 100644
--- a/target/arm/internals.h
+++ b/target/arm/internals.h
@@ -1350,6 +1350,16 @@ int exception_target_el(CPUARMState *env);
bool arm_singlestep_active(CPUARMState *env);
bool arm_generate_debug_exceptions(CPUARMState *env);
+/**
+ * pauth_ptr_mask:
+ * @env: cpu context
+ * @ptr: selects between TTBR0 and TTBR1
+ * @data: selects between TBI and TBID
+ *
+ * Return a mask of the bits of @ptr that contain the authentication code.
+ */
+uint64_t pauth_ptr_mask(CPUARMState *env, uint64_t ptr, bool data);
+
/* Add the cpreg definitions for debug related system registers */
void define_debug_regs(ARMCPU *cpu);
diff --git a/target/arm/pauth_helper.c b/target/arm/pauth_helper.c
index d0483bf051..20f347332d 100644
--- a/target/arm/pauth_helper.c
+++ b/target/arm/pauth_helper.c
@@ -339,14 +339,32 @@ static uint64_t pauth_addpac(CPUARMState *env, uint64_t ptr, uint64_t modifier,
return pac | ext | ptr;
}
-static uint64_t pauth_original_ptr(uint64_t ptr, ARMVAParameters param)
+static uint64_t pauth_ptr_mask_internal(ARMVAParameters param)
{
- /* Note that bit 55 is used whether or not the regime has 2 ranges. */
- uint64_t extfield = sextract64(ptr, 55, 1);
int bot_pac_bit = 64 - param.tsz;
int top_pac_bit = 64 - 8 * param.tbi;
- return deposit64(ptr, bot_pac_bit, top_pac_bit - bot_pac_bit, extfield);
+ return MAKE_64BIT_MASK(bot_pac_bit, top_pac_bit - bot_pac_bit);
+}
+
+static uint64_t pauth_original_ptr(uint64_t ptr, ARMVAParameters param)
+{
+ uint64_t mask = pauth_ptr_mask_internal(param);
+
+ /* Note that bit 55 is used whether or not the regime has 2 ranges. */
+ if (extract64(ptr, 55, 1)) {
+ return ptr | mask;
+ } else {
+ return ptr & ~mask;
+ }
+}
+
+uint64_t pauth_ptr_mask(CPUARMState *env, uint64_t ptr, bool data)
+{
+ ARMMMUIdx mmu_idx = arm_stage1_mmu_idx(env);
+ ARMVAParameters param = aa64_va_parameters(env, ptr, mmu_idx, data);
+
+ return pauth_ptr_mask_internal(param);
}
static uint64_t pauth_auth(CPUARMState *env, uint64_t ptr, uint64_t modifier,
--
2.34.1
^ permalink raw reply related [flat|nested] 28+ messages in thread* [PATCH v2 11/14] target/arm: Implement gdbstub pauth extension
2023-02-21 2:19 [PATCH v2 00/14] target/arm: gdbstub cleanups and additions Richard Henderson
` (9 preceding siblings ...)
2023-02-21 2:19 ` [PATCH v2 10/14] target/arm: Create pauth_ptr_mask Richard Henderson
@ 2023-02-21 2:19 ` Richard Henderson
2023-02-21 9:14 ` Luis Machado
2023-02-21 17:10 ` Peter Maydell
2023-02-21 2:19 ` [PATCH v2 12/14] target/arm: Export arm_v7m_mrs_control Richard Henderson
` (2 subsequent siblings)
13 siblings, 2 replies; 28+ messages in thread
From: Richard Henderson @ 2023-02-21 2:19 UTC (permalink / raw)
To: qemu-devel; +Cc: qemu-arm, Luis Machado, Thiago Jung Bauermann
The extension is primarily defined by the Linux kernel NT_ARM_PAC_MASK
ptrace register set.
The original gdb feature consists of two masks, data and code, which are
used to mask out the authentication code within a pointer. Following
discussion with Luis Machado, add two more masks in order to support
pointers within the high half of the address space (i.e. TTBR1 vs TTBR0).
Cc: Luis Machado <luis.machado@arm.com>
Cc: Thiago Jung Bauermann <thiago.bauermann@linaro.org>
Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1105
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
configs/targets/aarch64-linux-user.mak | 2 +-
configs/targets/aarch64-softmmu.mak | 2 +-
configs/targets/aarch64_be-linux-user.mak | 2 +-
target/arm/internals.h | 2 ++
target/arm/gdbstub.c | 5 ++++
target/arm/gdbstub64.c | 34 +++++++++++++++++++++++
gdb-xml/aarch64-pauth.xml | 15 ++++++++++
7 files changed, 59 insertions(+), 3 deletions(-)
create mode 100644 gdb-xml/aarch64-pauth.xml
diff --git a/configs/targets/aarch64-linux-user.mak b/configs/targets/aarch64-linux-user.mak
index db552f1839..ba8bc5fe3f 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
+TARGET_XML_FILES= gdb-xml/aarch64-core.xml gdb-xml/aarch64-fpu.xml gdb-xml/aarch64-pauth.xml
TARGET_HAS_BFLT=y
CONFIG_SEMIHOSTING=y
CONFIG_ARM_COMPATIBLE_SEMIHOSTING=y
diff --git a/configs/targets/aarch64-softmmu.mak b/configs/targets/aarch64-softmmu.mak
index d489e6da83..b4338e9568 100644
--- a/configs/targets/aarch64-softmmu.mak
+++ b/configs/targets/aarch64-softmmu.mak
@@ -1,5 +1,5 @@
TARGET_ARCH=aarch64
TARGET_BASE_ARCH=arm
TARGET_SUPPORTS_MTTCG=y
-TARGET_XML_FILES= gdb-xml/aarch64-core.xml gdb-xml/aarch64-fpu.xml gdb-xml/arm-core.xml gdb-xml/arm-vfp.xml gdb-xml/arm-vfp3.xml gdb-xml/arm-vfp-sysregs.xml gdb-xml/arm-neon.xml gdb-xml/arm-m-profile.xml gdb-xml/arm-m-profile-mve.xml
+TARGET_XML_FILES= gdb-xml/aarch64-core.xml gdb-xml/aarch64-fpu.xml gdb-xml/arm-core.xml gdb-xml/arm-vfp.xml gdb-xml/arm-vfp3.xml gdb-xml/arm-vfp-sysregs.xml gdb-xml/arm-neon.xml gdb-xml/arm-m-profile.xml gdb-xml/arm-m-profile-mve.xml gdb-xml/aarch64-pauth.xml
TARGET_NEED_FDT=y
diff --git a/configs/targets/aarch64_be-linux-user.mak b/configs/targets/aarch64_be-linux-user.mak
index dc78044fb1..acb5620cdb 100644
--- a/configs/targets/aarch64_be-linux-user.mak
+++ b/configs/targets/aarch64_be-linux-user.mak
@@ -1,7 +1,7 @@
TARGET_ARCH=aarch64
TARGET_BASE_ARCH=arm
TARGET_BIG_ENDIAN=y
-TARGET_XML_FILES= gdb-xml/aarch64-core.xml gdb-xml/aarch64-fpu.xml
+TARGET_XML_FILES= gdb-xml/aarch64-core.xml gdb-xml/aarch64-fpu.xml gdb-xml/aarch64-pauth.xml
TARGET_HAS_BFLT=y
CONFIG_SEMIHOSTING=y
CONFIG_ARM_COMPATIBLE_SEMIHOSTING=y
diff --git a/target/arm/internals.h b/target/arm/internals.h
index 370655061e..fb88b16579 100644
--- a/target/arm/internals.h
+++ b/target/arm/internals.h
@@ -1331,6 +1331,8 @@ int aarch64_gdb_get_sve_reg(CPUARMState *env, GByteArray *buf, int reg);
int aarch64_gdb_set_sve_reg(CPUARMState *env, uint8_t *buf, int reg);
int aarch64_gdb_get_fpu_reg(CPUARMState *env, GByteArray *buf, int reg);
int aarch64_gdb_set_fpu_reg(CPUARMState *env, uint8_t *buf, int reg);
+int aarch64_gdb_get_pauth_reg(CPUARMState *env, GByteArray *buf, int reg);
+int aarch64_gdb_set_pauth_reg(CPUARMState *env, 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);
diff --git a/target/arm/gdbstub.c b/target/arm/gdbstub.c
index bf8aff7824..062c8d447a 100644
--- a/target/arm/gdbstub.c
+++ b/target/arm/gdbstub.c
@@ -355,6 +355,11 @@ void arm_cpu_register_gdb_regs_for_features(ARMCPU *cpu)
aarch64_gdb_set_fpu_reg,
34, "aarch64-fpu.xml", 0);
}
+ if (isar_feature_aa64_pauth(&cpu->isar)) {
+ gdb_register_coprocessor(cs, aarch64_gdb_get_pauth_reg,
+ aarch64_gdb_set_pauth_reg,
+ 4, "aarch64-pauth.xml", 0);
+ }
#endif
} else {
if (arm_feature(env, ARM_FEATURE_NEON)) {
diff --git a/target/arm/gdbstub64.c b/target/arm/gdbstub64.c
index 3d9e9e97c8..3bee892fb7 100644
--- a/target/arm/gdbstub64.c
+++ b/target/arm/gdbstub64.c
@@ -210,6 +210,40 @@ int aarch64_gdb_set_sve_reg(CPUARMState *env, uint8_t *buf, int reg)
return 0;
}
+int aarch64_gdb_get_pauth_reg(CPUARMState *env, GByteArray *buf, int reg)
+{
+ switch (reg) {
+ case 0: /* pauth_dmask */
+ case 1: /* pauth_cmask */
+ case 2: /* pauth_dmask_high */
+ case 3: /* pauth_cmask_high */
+ /*
+ * Note that older versions of this feature only contained
+ * pauth_{d,c}mask, for use with Linux user processes, and
+ * thus exclusively in the low half of the address space.
+ *
+ * To support system mode, and to debug kernels, two new regs
+ * were added to cover the high half of the address space.
+ * For the purpose of pauth_ptr_mask, we can use any well-formed
+ * address within the address space half -- here, 0 and -1.
+ */
+ {
+ bool is_data = !(reg & 1);
+ bool is_high = reg & 2;
+ uint64_t mask = pauth_ptr_mask(env, -is_high, is_data);
+ return gdb_get_reg64(buf, mask);
+ }
+ default:
+ return 0;
+ }
+}
+
+int aarch64_gdb_set_pauth_reg(CPUARMState *env, uint8_t *buf, int reg)
+{
+ /* All pseudo registers are read-only. */
+ return 0;
+}
+
static void output_vector_union_type(GString *s, int reg_width,
const char *name)
{
diff --git a/gdb-xml/aarch64-pauth.xml b/gdb-xml/aarch64-pauth.xml
new file mode 100644
index 0000000000..24af5f903c
--- /dev/null
+++ b/gdb-xml/aarch64-pauth.xml
@@ -0,0 +1,15 @@
+<?xml version="1.0"?>
+<!-- Copyright (C) 2018-2022 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.pauth">
+ <reg name="pauth_dmask" bitsize="64"/>
+ <reg name="pauth_cmask" bitsize="64"/>
+ <reg name="pauth_dmask_high" bitsize="64"/>
+ <reg name="pauth_cmask_high" bitsize="64"/>
+</feature>
+
--
2.34.1
^ permalink raw reply related [flat|nested] 28+ messages in thread* Re: [PATCH v2 11/14] target/arm: Implement gdbstub pauth extension
2023-02-21 2:19 ` [PATCH v2 11/14] target/arm: Implement gdbstub pauth extension Richard Henderson
@ 2023-02-21 9:14 ` Luis Machado
2023-02-21 17:10 ` Peter Maydell
1 sibling, 0 replies; 28+ messages in thread
From: Luis Machado @ 2023-02-21 9:14 UTC (permalink / raw)
To: Richard Henderson, qemu-devel; +Cc: qemu-arm, Thiago Jung Bauermann
Hi,
On 2/21/23 02:19, Richard Henderson wrote:
> The extension is primarily defined by the Linux kernel NT_ARM_PAC_MASK
> ptrace register set.
>
> The original gdb feature consists of two masks, data and code, which are
> used to mask out the authentication code within a pointer. Following
> discussion with Luis Machado, add two more masks in order to support
> pointers within the high half of the address space (i.e. TTBR1 vs TTBR0).
>
> Cc: Luis Machado <luis.machado@arm.com>
> Cc: Thiago Jung Bauermann <thiago.bauermann@linaro.org>
> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1105
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
> configs/targets/aarch64-linux-user.mak | 2 +-
> configs/targets/aarch64-softmmu.mak | 2 +-
> configs/targets/aarch64_be-linux-user.mak | 2 +-
> target/arm/internals.h | 2 ++
> target/arm/gdbstub.c | 5 ++++
> target/arm/gdbstub64.c | 34 +++++++++++++++++++++++
> gdb-xml/aarch64-pauth.xml | 15 ++++++++++
> 7 files changed, 59 insertions(+), 3 deletions(-)
> create mode 100644 gdb-xml/aarch64-pauth.xml
>
> diff --git a/configs/targets/aarch64-linux-user.mak b/configs/targets/aarch64-linux-user.mak
> index db552f1839..ba8bc5fe3f 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
> +TARGET_XML_FILES= gdb-xml/aarch64-core.xml gdb-xml/aarch64-fpu.xml gdb-xml/aarch64-pauth.xml
> TARGET_HAS_BFLT=y
> CONFIG_SEMIHOSTING=y
> CONFIG_ARM_COMPATIBLE_SEMIHOSTING=y
> diff --git a/configs/targets/aarch64-softmmu.mak b/configs/targets/aarch64-softmmu.mak
> index d489e6da83..b4338e9568 100644
> --- a/configs/targets/aarch64-softmmu.mak
> +++ b/configs/targets/aarch64-softmmu.mak
> @@ -1,5 +1,5 @@
> TARGET_ARCH=aarch64
> TARGET_BASE_ARCH=arm
> TARGET_SUPPORTS_MTTCG=y
> -TARGET_XML_FILES= gdb-xml/aarch64-core.xml gdb-xml/aarch64-fpu.xml gdb-xml/arm-core.xml gdb-xml/arm-vfp.xml gdb-xml/arm-vfp3.xml gdb-xml/arm-vfp-sysregs.xml gdb-xml/arm-neon.xml gdb-xml/arm-m-profile.xml gdb-xml/arm-m-profile-mve.xml
> +TARGET_XML_FILES= gdb-xml/aarch64-core.xml gdb-xml/aarch64-fpu.xml gdb-xml/arm-core.xml gdb-xml/arm-vfp.xml gdb-xml/arm-vfp3.xml gdb-xml/arm-vfp-sysregs.xml gdb-xml/arm-neon.xml gdb-xml/arm-m-profile.xml gdb-xml/arm-m-profile-mve.xml gdb-xml/aarch64-pauth.xml
> TARGET_NEED_FDT=y
> diff --git a/configs/targets/aarch64_be-linux-user.mak b/configs/targets/aarch64_be-linux-user.mak
> index dc78044fb1..acb5620cdb 100644
> --- a/configs/targets/aarch64_be-linux-user.mak
> +++ b/configs/targets/aarch64_be-linux-user.mak
> @@ -1,7 +1,7 @@
> TARGET_ARCH=aarch64
> TARGET_BASE_ARCH=arm
> TARGET_BIG_ENDIAN=y
> -TARGET_XML_FILES= gdb-xml/aarch64-core.xml gdb-xml/aarch64-fpu.xml
> +TARGET_XML_FILES= gdb-xml/aarch64-core.xml gdb-xml/aarch64-fpu.xml gdb-xml/aarch64-pauth.xml
> TARGET_HAS_BFLT=y
> CONFIG_SEMIHOSTING=y
> CONFIG_ARM_COMPATIBLE_SEMIHOSTING=y
> diff --git a/target/arm/internals.h b/target/arm/internals.h
> index 370655061e..fb88b16579 100644
> --- a/target/arm/internals.h
> +++ b/target/arm/internals.h
> @@ -1331,6 +1331,8 @@ int aarch64_gdb_get_sve_reg(CPUARMState *env, GByteArray *buf, int reg);
> int aarch64_gdb_set_sve_reg(CPUARMState *env, uint8_t *buf, int reg);
> int aarch64_gdb_get_fpu_reg(CPUARMState *env, GByteArray *buf, int reg);
> int aarch64_gdb_set_fpu_reg(CPUARMState *env, uint8_t *buf, int reg);
> +int aarch64_gdb_get_pauth_reg(CPUARMState *env, GByteArray *buf, int reg);
> +int aarch64_gdb_set_pauth_reg(CPUARMState *env, 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);
> diff --git a/target/arm/gdbstub.c b/target/arm/gdbstub.c
> index bf8aff7824..062c8d447a 100644
> --- a/target/arm/gdbstub.c
> +++ b/target/arm/gdbstub.c
> @@ -355,6 +355,11 @@ void arm_cpu_register_gdb_regs_for_features(ARMCPU *cpu)
> aarch64_gdb_set_fpu_reg,
> 34, "aarch64-fpu.xml", 0);
> }
> + if (isar_feature_aa64_pauth(&cpu->isar)) {
> + gdb_register_coprocessor(cs, aarch64_gdb_get_pauth_reg,
> + aarch64_gdb_set_pauth_reg,
> + 4, "aarch64-pauth.xml", 0);
> + }
> #endif
> } else {
> if (arm_feature(env, ARM_FEATURE_NEON)) {
> diff --git a/target/arm/gdbstub64.c b/target/arm/gdbstub64.c
> index 3d9e9e97c8..3bee892fb7 100644
> --- a/target/arm/gdbstub64.c
> +++ b/target/arm/gdbstub64.c
> @@ -210,6 +210,40 @@ int aarch64_gdb_set_sve_reg(CPUARMState *env, uint8_t *buf, int reg)
> return 0;
> }
>
> +int aarch64_gdb_get_pauth_reg(CPUARMState *env, GByteArray *buf, int reg)
> +{
> + switch (reg) {
> + case 0: /* pauth_dmask */
> + case 1: /* pauth_cmask */
> + case 2: /* pauth_dmask_high */
> + case 3: /* pauth_cmask_high */
> + /*
> + * Note that older versions of this feature only contained
> + * pauth_{d,c}mask, for use with Linux user processes, and
> + * thus exclusively in the low half of the address space.
> + *
> + * To support system mode, and to debug kernels, two new regs
> + * were added to cover the high half of the address space.
> + * For the purpose of pauth_ptr_mask, we can use any well-formed
> + * address within the address space half -- here, 0 and -1.
> + */
> + {
> + bool is_data = !(reg & 1);
> + bool is_high = reg & 2;
> + uint64_t mask = pauth_ptr_mask(env, -is_high, is_data);
> + return gdb_get_reg64(buf, mask);
> + }
> + default:
> + return 0;
> + }
> +}
> +
> +int aarch64_gdb_set_pauth_reg(CPUARMState *env, uint8_t *buf, int reg)
> +{
> + /* All pseudo registers are read-only. */
> + return 0;
> +}
> +
> static void output_vector_union_type(GString *s, int reg_width,
> const char *name)
> {
> diff --git a/gdb-xml/aarch64-pauth.xml b/gdb-xml/aarch64-pauth.xml
> new file mode 100644
> index 0000000000..24af5f903c
> --- /dev/null
> +++ b/gdb-xml/aarch64-pauth.xml
> @@ -0,0 +1,15 @@
> +<?xml version="1.0"?>
> +<!-- Copyright (C) 2018-2022 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.pauth">
> + <reg name="pauth_dmask" bitsize="64"/>
> + <reg name="pauth_cmask" bitsize="64"/>
> + <reg name="pauth_dmask_high" bitsize="64"/>
> + <reg name="pauth_cmask_high" bitsize="64"/>
> +</feature>
> +
FTR, I've pushed the gdb-side changes: https://sourceware.org/git/?p=binutils-gdb.git;a=commit;h=6d0020873deb2f2c4e0965dc2ebf227bc1db3140
IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.
^ permalink raw reply [flat|nested] 28+ messages in thread* Re: [PATCH v2 11/14] target/arm: Implement gdbstub pauth extension
2023-02-21 2:19 ` [PATCH v2 11/14] target/arm: Implement gdbstub pauth extension Richard Henderson
2023-02-21 9:14 ` Luis Machado
@ 2023-02-21 17:10 ` Peter Maydell
1 sibling, 0 replies; 28+ messages in thread
From: Peter Maydell @ 2023-02-21 17:10 UTC (permalink / raw)
To: Richard Henderson
Cc: qemu-devel, qemu-arm, Luis Machado, Thiago Jung Bauermann
On Tue, 21 Feb 2023 at 02:21, Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> The extension is primarily defined by the Linux kernel NT_ARM_PAC_MASK
> ptrace register set.
>
> The original gdb feature consists of two masks, data and code, which are
> used to mask out the authentication code within a pointer. Following
> discussion with Luis Machado, add two more masks in order to support
> pointers within the high half of the address space (i.e. TTBR1 vs TTBR0).
>
> Cc: Luis Machado <luis.machado@arm.com>
> Cc: Thiago Jung Bauermann <thiago.bauermann@linaro.org>
> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1105
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
thanks
-- PMM
^ permalink raw reply [flat|nested] 28+ messages in thread
* [PATCH v2 12/14] target/arm: Export arm_v7m_mrs_control
2023-02-21 2:19 [PATCH v2 00/14] target/arm: gdbstub cleanups and additions Richard Henderson
` (10 preceding siblings ...)
2023-02-21 2:19 ` [PATCH v2 11/14] target/arm: Implement gdbstub pauth extension Richard Henderson
@ 2023-02-21 2:19 ` Richard Henderson
2023-02-21 7:23 ` Philippe Mathieu-Daudé
2023-02-21 2:19 ` [PATCH v2 13/14] target/arm: Export arm_v7m_get_sp_ptr Richard Henderson
2023-02-21 2:19 ` [PATCH v2 14/14] target/arm: Implement gdbstub m-profile systemreg and secext Richard Henderson
13 siblings, 1 reply; 28+ messages in thread
From: Richard Henderson @ 2023-02-21 2:19 UTC (permalink / raw)
To: qemu-devel; +Cc: qemu-arm, David Reiss, Peter Maydell
From: David Reiss <dreiss@meta.com>
Allow the function to be used outside of m_helper.c.
Rename with an "arm_" prefix.
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
Signed-off-by: David Reiss <dreiss@meta.com>
[rth: Split out of a larger patch]
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
target/arm/internals.h | 3 +++
target/arm/m_helper.c | 6 +++---
2 files changed, 6 insertions(+), 3 deletions(-)
diff --git a/target/arm/internals.h b/target/arm/internals.h
index fb88b16579..89052b1c94 100644
--- a/target/arm/internals.h
+++ b/target/arm/internals.h
@@ -1339,6 +1339,9 @@ void arm_cpu_pauth_finalize(ARMCPU *cpu, Error **errp);
void arm_cpu_lpa2_finalize(ARMCPU *cpu, Error **errp);
#endif
+/* Read the CONTROL register as the MRS instruction would. */
+uint32_t arm_v7m_mrs_control(CPUARMState *env, uint32_t secure);
+
#ifdef CONFIG_USER_ONLY
static inline void define_cortex_a72_a57_a53_cp_reginfo(ARMCPU *cpu) { }
#else
diff --git a/target/arm/m_helper.c b/target/arm/m_helper.c
index f94e87e728..03be79e7bf 100644
--- a/target/arm/m_helper.c
+++ b/target/arm/m_helper.c
@@ -56,7 +56,7 @@ static uint32_t v7m_mrs_xpsr(CPUARMState *env, uint32_t reg, unsigned el)
return xpsr_read(env) & mask;
}
-static uint32_t v7m_mrs_control(CPUARMState *env, uint32_t secure)
+uint32_t arm_v7m_mrs_control(CPUARMState *env, uint32_t secure)
{
uint32_t value = env->v7m.control[secure];
@@ -93,7 +93,7 @@ uint32_t HELPER(v7m_mrs)(CPUARMState *env, uint32_t reg)
case 0 ... 7: /* xPSR sub-fields */
return v7m_mrs_xpsr(env, reg, 0);
case 20: /* CONTROL */
- return v7m_mrs_control(env, 0);
+ return arm_v7m_mrs_control(env, 0);
default:
/* Unprivileged reads others as zero. */
return 0;
@@ -2465,7 +2465,7 @@ uint32_t HELPER(v7m_mrs)(CPUARMState *env, uint32_t reg)
case 0 ... 7: /* xPSR sub-fields */
return v7m_mrs_xpsr(env, reg, el);
case 20: /* CONTROL */
- return v7m_mrs_control(env, env->v7m.secure);
+ return arm_v7m_mrs_control(env, env->v7m.secure);
case 0x94: /* CONTROL_NS */
/*
* We have to handle this here because unprivileged Secure code
--
2.34.1
^ permalink raw reply related [flat|nested] 28+ messages in thread* Re: [PATCH v2 12/14] target/arm: Export arm_v7m_mrs_control
2023-02-21 2:19 ` [PATCH v2 12/14] target/arm: Export arm_v7m_mrs_control Richard Henderson
@ 2023-02-21 7:23 ` Philippe Mathieu-Daudé
0 siblings, 0 replies; 28+ messages in thread
From: Philippe Mathieu-Daudé @ 2023-02-21 7:23 UTC (permalink / raw)
To: Richard Henderson, qemu-devel; +Cc: qemu-arm, David Reiss, Peter Maydell
On 21/2/23 03:19, Richard Henderson wrote:
> From: David Reiss <dreiss@meta.com>
>
> Allow the function to be used outside of m_helper.c.
> Rename with an "arm_" prefix.
>
> Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
> Signed-off-by: David Reiss <dreiss@meta.com>
> [rth: Split out of a larger patch]
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
> target/arm/internals.h | 3 +++
> target/arm/m_helper.c | 6 +++---
> 2 files changed, 6 insertions(+), 3 deletions(-)
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
^ permalink raw reply [flat|nested] 28+ messages in thread
* [PATCH v2 13/14] target/arm: Export arm_v7m_get_sp_ptr
2023-02-21 2:19 [PATCH v2 00/14] target/arm: gdbstub cleanups and additions Richard Henderson
` (11 preceding siblings ...)
2023-02-21 2:19 ` [PATCH v2 12/14] target/arm: Export arm_v7m_mrs_control Richard Henderson
@ 2023-02-21 2:19 ` Richard Henderson
2023-02-21 7:24 ` Philippe Mathieu-Daudé
2023-02-21 2:19 ` [PATCH v2 14/14] target/arm: Implement gdbstub m-profile systemreg and secext Richard Henderson
13 siblings, 1 reply; 28+ messages in thread
From: Richard Henderson @ 2023-02-21 2:19 UTC (permalink / raw)
To: qemu-devel; +Cc: qemu-arm, David Reiss, Peter Maydell
From: David Reiss <dreiss@meta.com>
Allow the function to be used outside of m_helper.c.
Move to be outside of ifndef CONFIG_USER_ONLY block.
Rename from get_v7m_sp_ptr.
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
Signed-off-by: David Reiss <dreiss@meta.com>
[rth: Split out of a larger patch]
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
target/arm/internals.h | 10 +++++
target/arm/m_helper.c | 84 +++++++++++++++++++++---------------------
2 files changed, 51 insertions(+), 43 deletions(-)
diff --git a/target/arm/internals.h b/target/arm/internals.h
index 89052b1c94..523822ac87 100644
--- a/target/arm/internals.h
+++ b/target/arm/internals.h
@@ -1342,6 +1342,16 @@ void arm_cpu_lpa2_finalize(ARMCPU *cpu, Error **errp);
/* Read the CONTROL register as the MRS instruction would. */
uint32_t arm_v7m_mrs_control(CPUARMState *env, uint32_t secure);
+/*
+ * Return a pointer to the location where we currently store the
+ * stack pointer for the requested security state and thread mode.
+ * This pointer will become invalid if the CPU state is updated
+ * such that the stack pointers are switched around (eg changing
+ * the SPSEL control bit).
+ */
+uint32_t *arm_v7m_get_sp_ptr(CPUARMState *env, bool secure,
+ bool threadmode, bool spsel);
+
#ifdef CONFIG_USER_ONLY
static inline void define_cortex_a72_a57_a53_cp_reginfo(ARMCPU *cpu) { }
#else
diff --git a/target/arm/m_helper.c b/target/arm/m_helper.c
index 03be79e7bf..081fc3f5f7 100644
--- a/target/arm/m_helper.c
+++ b/target/arm/m_helper.c
@@ -650,42 +650,6 @@ void HELPER(v7m_blxns)(CPUARMState *env, uint32_t dest)
arm_rebuild_hflags(env);
}
-static uint32_t *get_v7m_sp_ptr(CPUARMState *env, bool secure, bool threadmode,
- bool spsel)
-{
- /*
- * Return a pointer to the location where we currently store the
- * stack pointer for the requested security state and thread mode.
- * This pointer will become invalid if the CPU state is updated
- * such that the stack pointers are switched around (eg changing
- * the SPSEL control bit).
- * Compare the v8M ARM ARM pseudocode LookUpSP_with_security_mode().
- * Unlike that pseudocode, we require the caller to pass us in the
- * SPSEL control bit value; this is because we also use this
- * function in handling of pushing of the callee-saves registers
- * part of the v8M stack frame (pseudocode PushCalleeStack()),
- * and in the tailchain codepath the SPSEL bit comes from the exception
- * return magic LR value from the previous exception. The pseudocode
- * opencodes the stack-selection in PushCalleeStack(), but we prefer
- * to make this utility function generic enough to do the job.
- */
- bool want_psp = threadmode && spsel;
-
- if (secure == env->v7m.secure) {
- if (want_psp == v7m_using_psp(env)) {
- return &env->regs[13];
- } else {
- return &env->v7m.other_sp;
- }
- } else {
- if (want_psp) {
- return &env->v7m.other_ss_psp;
- } else {
- return &env->v7m.other_ss_msp;
- }
- }
-}
-
static bool arm_v7m_load_vector(ARMCPU *cpu, int exc, bool targets_secure,
uint32_t *pvec)
{
@@ -810,8 +774,8 @@ static bool v7m_push_callee_stack(ARMCPU *cpu, uint32_t lr, bool dotailchain,
!mode;
mmu_idx = arm_v7m_mmu_idx_for_secstate_and_priv(env, M_REG_S, priv);
- frame_sp_p = get_v7m_sp_ptr(env, M_REG_S, mode,
- lr & R_V7M_EXCRET_SPSEL_MASK);
+ frame_sp_p = arm_v7m_get_sp_ptr(env, M_REG_S, mode,
+ lr & R_V7M_EXCRET_SPSEL_MASK);
want_psp = mode && (lr & R_V7M_EXCRET_SPSEL_MASK);
if (want_psp) {
limit = env->v7m.psplim[M_REG_S];
@@ -1656,10 +1620,8 @@ static void do_v7m_exception_exit(ARMCPU *cpu)
* use 'frame_sp_p' after we do something that makes it invalid.
*/
bool spsel = env->v7m.control[return_to_secure] & R_V7M_CONTROL_SPSEL_MASK;
- uint32_t *frame_sp_p = get_v7m_sp_ptr(env,
- return_to_secure,
- !return_to_handler,
- spsel);
+ uint32_t *frame_sp_p = arm_v7m_get_sp_ptr(env, return_to_secure,
+ !return_to_handler, spsel);
uint32_t frameptr = *frame_sp_p;
bool pop_ok = true;
ARMMMUIdx mmu_idx;
@@ -1965,7 +1927,7 @@ static bool do_v7m_function_return(ARMCPU *cpu)
threadmode = !arm_v7m_is_handler_mode(env);
spsel = env->v7m.control[M_REG_S] & R_V7M_CONTROL_SPSEL_MASK;
- frame_sp_p = get_v7m_sp_ptr(env, true, threadmode, spsel);
+ frame_sp_p = arm_v7m_get_sp_ptr(env, true, threadmode, spsel);
frameptr = *frame_sp_p;
/*
@@ -2900,3 +2862,39 @@ uint32_t HELPER(v7m_tt)(CPUARMState *env, uint32_t addr, uint32_t op)
}
#endif /* !CONFIG_USER_ONLY */
+
+uint32_t *arm_v7m_get_sp_ptr(CPUARMState *env, bool secure, bool threadmode,
+ bool spsel)
+{
+ /*
+ * Return a pointer to the location where we currently store the
+ * stack pointer for the requested security state and thread mode.
+ * This pointer will become invalid if the CPU state is updated
+ * such that the stack pointers are switched around (eg changing
+ * the SPSEL control bit).
+ * Compare the v8M ARM ARM pseudocode LookUpSP_with_security_mode().
+ * Unlike that pseudocode, we require the caller to pass us in the
+ * SPSEL control bit value; this is because we also use this
+ * function in handling of pushing of the callee-saves registers
+ * part of the v8M stack frame (pseudocode PushCalleeStack()),
+ * and in the tailchain codepath the SPSEL bit comes from the exception
+ * return magic LR value from the previous exception. The pseudocode
+ * opencodes the stack-selection in PushCalleeStack(), but we prefer
+ * to make this utility function generic enough to do the job.
+ */
+ bool want_psp = threadmode && spsel;
+
+ if (secure == env->v7m.secure) {
+ if (want_psp == v7m_using_psp(env)) {
+ return &env->regs[13];
+ } else {
+ return &env->v7m.other_sp;
+ }
+ } else {
+ if (want_psp) {
+ return &env->v7m.other_ss_psp;
+ } else {
+ return &env->v7m.other_ss_msp;
+ }
+ }
+}
--
2.34.1
^ permalink raw reply related [flat|nested] 28+ messages in thread* Re: [PATCH v2 13/14] target/arm: Export arm_v7m_get_sp_ptr
2023-02-21 2:19 ` [PATCH v2 13/14] target/arm: Export arm_v7m_get_sp_ptr Richard Henderson
@ 2023-02-21 7:24 ` Philippe Mathieu-Daudé
0 siblings, 0 replies; 28+ messages in thread
From: Philippe Mathieu-Daudé @ 2023-02-21 7:24 UTC (permalink / raw)
To: Richard Henderson, qemu-devel; +Cc: qemu-arm, David Reiss, Peter Maydell
On 21/2/23 03:19, Richard Henderson wrote:
> From: David Reiss <dreiss@meta.com>
>
> Allow the function to be used outside of m_helper.c.
> Move to be outside of ifndef CONFIG_USER_ONLY block.
> Rename from get_v7m_sp_ptr.
>
> Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
> Signed-off-by: David Reiss <dreiss@meta.com>
> [rth: Split out of a larger patch]
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
> target/arm/internals.h | 10 +++++
> target/arm/m_helper.c | 84 +++++++++++++++++++++---------------------
> 2 files changed, 51 insertions(+), 43 deletions(-)
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
^ permalink raw reply [flat|nested] 28+ messages in thread
* [PATCH v2 14/14] target/arm: Implement gdbstub m-profile systemreg and secext
2023-02-21 2:19 [PATCH v2 00/14] target/arm: gdbstub cleanups and additions Richard Henderson
` (12 preceding siblings ...)
2023-02-21 2:19 ` [PATCH v2 13/14] target/arm: Export arm_v7m_get_sp_ptr Richard Henderson
@ 2023-02-21 2:19 ` Richard Henderson
2023-02-21 7:32 ` Philippe Mathieu-Daudé
2023-02-21 17:25 ` Peter Maydell
13 siblings, 2 replies; 28+ messages in thread
From: Richard Henderson @ 2023-02-21 2:19 UTC (permalink / raw)
To: qemu-devel; +Cc: qemu-arm, David Reiss
The upstream gdb xml only implements {MSP,PSP}{,_NS,S}, but
go ahead and implement the other system registers as well.
Since there is significant overlap between the two, implement
them with common code. The only exception is the systemreg
view of CONTROL, which merges the banked bits as per MRS.
Signed-off-by: David Reiss <dreiss@meta.com>
[rth: Substatial rewrite using enumerator and shared code.]
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
target/arm/cpu.h | 2 +
target/arm/gdbstub.c | 194 +++++++++++++++++++++++++++++++++++++++++++
2 files changed, 196 insertions(+)
diff --git a/target/arm/cpu.h b/target/arm/cpu.h
index 059fe62eaa..6e97a256fb 100644
--- a/target/arm/cpu.h
+++ b/target/arm/cpu.h
@@ -869,6 +869,8 @@ struct ArchCPU {
DynamicGDBXMLInfo dyn_sysreg_xml;
DynamicGDBXMLInfo dyn_svereg_xml;
+ DynamicGDBXMLInfo dyn_m_systemreg_xml;
+ DynamicGDBXMLInfo dyn_m_secextreg_xml;
/* Timers used by the generic (architected) timer */
QEMUTimer *gt_timer[NUM_GTIMERS];
diff --git a/target/arm/gdbstub.c b/target/arm/gdbstub.c
index 062c8d447a..fef53e4ef5 100644
--- a/target/arm/gdbstub.c
+++ b/target/arm/gdbstub.c
@@ -322,6 +322,180 @@ static int arm_gen_dynamic_sysreg_xml(CPUState *cs, int base_reg)
return cpu->dyn_sysreg_xml.num;
}
+typedef enum {
+ M_SYSREG_MSP,
+ M_SYSREG_PSP,
+ M_SYSREG_PRIMASK,
+ M_SYSREG_CONTROL,
+ M_SYSREG_BASEPRI,
+ M_SYSREG_FAULTMASK,
+ M_SYSREG_MSPLIM,
+ M_SYSREG_PSPLIM,
+} MProfileSysreg;
+
+static const struct {
+ const char *name;
+ int feature;
+} m_sysreg_def[] = {
+ [M_SYSREG_MSP] = { "msp", ARM_FEATURE_M },
+ [M_SYSREG_PSP] = { "psp", ARM_FEATURE_M },
+ [M_SYSREG_PRIMASK] = { "primask", ARM_FEATURE_M },
+ [M_SYSREG_CONTROL] = { "control", ARM_FEATURE_M },
+ [M_SYSREG_BASEPRI] = { "basepri", ARM_FEATURE_M_MAIN },
+ [M_SYSREG_FAULTMASK] = { "faultmask", ARM_FEATURE_M_MAIN },
+ [M_SYSREG_MSPLIM] = { "msplim", ARM_FEATURE_V8 },
+ [M_SYSREG_PSPLIM] = { "psplim", ARM_FEATURE_V8 },
+};
+
+static uint32_t *m_sysreg_ptr(CPUARMState *env, MProfileSysreg reg, bool sec)
+{
+ uint32_t *ptr;
+
+ switch (reg) {
+ case M_SYSREG_MSP:
+ ptr = arm_v7m_get_sp_ptr(env, sec, false, true);
+ break;
+ case M_SYSREG_PSP:
+ ptr = arm_v7m_get_sp_ptr(env, sec, true, true);
+ break;
+ case M_SYSREG_MSPLIM:
+ ptr = &env->v7m.msplim[sec];
+ break;
+ case M_SYSREG_PSPLIM:
+ ptr = &env->v7m.psplim[sec];
+ break;
+ case M_SYSREG_PRIMASK:
+ ptr = &env->v7m.primask[sec];
+ break;
+ case M_SYSREG_BASEPRI:
+ ptr = &env->v7m.basepri[sec];
+ break;
+ case M_SYSREG_FAULTMASK:
+ ptr = &env->v7m.faultmask[sec];
+ break;
+ case M_SYSREG_CONTROL:
+ ptr = &env->v7m.control[sec];
+ break;
+ default:
+ return NULL;
+ }
+ return arm_feature(env, m_sysreg_def[reg].feature) ? ptr : NULL;
+}
+
+static int m_sysreg_get(CPUARMState *env, GByteArray *buf,
+ MProfileSysreg reg, bool secure)
+{
+ uint32_t *ptr = m_sysreg_ptr(env, reg, secure);
+
+ if (ptr == NULL) {
+ return 0;
+ }
+ return gdb_get_reg32(buf, *ptr);
+}
+
+static int m_sysreg_set(CPUARMState *env, uint8_t *buf,
+ MProfileSysreg reg, bool secure)
+{
+ uint32_t *ptr = m_sysreg_ptr(env, reg, secure);
+
+ if (ptr == NULL) {
+ return 0;
+ }
+ *ptr = ldl_p(buf);
+ return 4;
+}
+
+static int arm_gdb_get_m_systemreg(CPUARMState *env, GByteArray *buf, int reg)
+{
+ /*
+ * Here, we emulate MRS instruction, where CONTROL has a mix of
+ * banked and non-banked bits.
+ */
+ if (reg == M_SYSREG_CONTROL) {
+ return gdb_get_reg32(buf, arm_v7m_mrs_control(env, env->v7m.secure));
+ }
+ return m_sysreg_get(env, buf, reg, env->v7m.secure);
+}
+
+static int arm_gdb_set_m_systemreg(CPUARMState *env, uint8_t *buf, int reg)
+{
+ /* Control is a mix of banked and non-banked bits -- disallow write. */
+ if (reg == M_SYSREG_CONTROL) {
+ return 0;
+ }
+ return m_sysreg_set(env, buf, reg, env->v7m.secure);
+}
+
+static int arm_gen_dynamic_m_systemreg_xml(CPUState *cs, int orig_base_reg)
+{
+ ARMCPU *cpu = ARM_CPU(cs);
+ CPUARMState *env = &cpu->env;
+ GString *s = g_string_new(NULL);
+ int base_reg = orig_base_reg;
+ int i;
+
+ g_string_printf(s, "<?xml version=\"1.0\"?>");
+ g_string_append_printf(s, "<!DOCTYPE target SYSTEM \"gdb-target.dtd\">");
+ g_string_append_printf(s, "<feature name=\"org.gnu.gdb.arm.m-system\">\n");
+
+ for (i = 0; i < ARRAY_SIZE(m_sysreg_def); i++) {
+ if (arm_feature(env, m_sysreg_def[i].feature)) {
+ g_string_append_printf(s,
+ "<reg name=\"%s\" bitsize=\"32\" regnum=\"%d\"/>\n",
+ m_sysreg_def[i].name, base_reg++);
+ }
+ }
+
+ g_string_append_printf(s, "</feature>");
+ cpu->dyn_m_systemreg_xml.desc = g_string_free(s, false);
+ cpu->dyn_m_systemreg_xml.num = base_reg - orig_base_reg;
+
+ return cpu->dyn_m_systemreg_xml.num;
+}
+
+#ifndef CONFIG_USER_ONLY
+/*
+ * For user-only, we see the non-secure registers via m_systemreg above.
+ * For secext, encode the non-secure view as even and secure view as odd.
+ */
+static int arm_gdb_get_m_secextreg(CPUARMState *env, GByteArray *buf, int reg)
+{
+ return m_sysreg_get(env, buf, reg >> 1, reg & 1);
+}
+
+static int arm_gdb_set_m_secextreg(CPUARMState *env, uint8_t *buf, int reg)
+{
+ return m_sysreg_set(env, buf, reg >> 1, reg & 1);
+}
+
+static int arm_gen_dynamic_m_secextreg_xml(CPUState *cs, int orig_base_reg)
+{
+ ARMCPU *cpu = ARM_CPU(cs);
+ GString *s = g_string_new(NULL);
+ int base_reg = orig_base_reg;
+ int i;
+
+ g_string_printf(s, "<?xml version=\"1.0\"?>");
+ g_string_append_printf(s, "<!DOCTYPE target SYSTEM \"gdb-target.dtd\">");
+ g_string_append_printf(s, "<feature name=\"org.gnu.gdb.arm.secext\">\n");
+
+ for (i = 0; i < ARRAY_SIZE(m_sysreg_def); i++) {
+ g_string_append_printf(s,
+ "<reg name=\"%s_ns\" bitsize=\"32\" regnum=\"%d\"/>\n",
+ m_sysreg_def[i].name, base_reg++);
+ g_string_append_printf(s,
+ "<reg name=\"%s_s\" bitsize=\"32\" regnum=\"%d\"/>\n",
+ m_sysreg_def[i].name, base_reg++);
+ }
+
+ g_string_append_printf(s, "</feature>");
+ cpu->dyn_m_secextreg_xml.desc = g_string_free(s, false);
+ cpu->dyn_m_secextreg_xml.num = base_reg - orig_base_reg;
+
+ return cpu->dyn_m_secextreg_xml.num;
+}
+#endif
+
const char *arm_gdb_get_dynamic_xml(CPUState *cs, const char *xmlname)
{
ARMCPU *cpu = ARM_CPU(cs);
@@ -330,6 +504,12 @@ const char *arm_gdb_get_dynamic_xml(CPUState *cs, const char *xmlname)
return cpu->dyn_sysreg_xml.desc;
} else if (strcmp(xmlname, "sve-registers.xml") == 0) {
return cpu->dyn_svereg_xml.desc;
+ } else if (strcmp(xmlname, "arm-m-system.xml") == 0) {
+ return cpu->dyn_m_systemreg_xml.desc;
+#ifndef CONFIG_USER_ONLY
+ } else if (strcmp(xmlname, "arm-m-secext.xml") == 0) {
+ return cpu->dyn_m_secextreg_xml.desc;
+#endif
}
return NULL;
}
@@ -389,4 +569,18 @@ void arm_cpu_register_gdb_regs_for_features(ARMCPU *cpu)
arm_gen_dynamic_sysreg_xml(cs, cs->gdb_num_regs),
"system-registers.xml", 0);
+ if (arm_feature(env, ARM_FEATURE_M)) {
+ gdb_register_coprocessor(cs,
+ arm_gdb_get_m_systemreg, arm_gdb_set_m_systemreg,
+ arm_gen_dynamic_m_systemreg_xml(cs, cs->gdb_num_regs),
+ "arm-m-system.xml", 0);
+#ifndef CONFIG_USER_ONLY
+ if (arm_feature(env, ARM_FEATURE_M_SECURITY)) {
+ gdb_register_coprocessor(cs,
+ arm_gdb_get_m_secextreg, arm_gdb_set_m_secextreg,
+ arm_gen_dynamic_m_secextreg_xml(cs, cs->gdb_num_regs),
+ "arm-m-secext.xml", 0);
+ }
+#endif
+ }
}
--
2.34.1
^ permalink raw reply related [flat|nested] 28+ messages in thread* Re: [PATCH v2 14/14] target/arm: Implement gdbstub m-profile systemreg and secext
2023-02-21 2:19 ` [PATCH v2 14/14] target/arm: Implement gdbstub m-profile systemreg and secext Richard Henderson
@ 2023-02-21 7:32 ` Philippe Mathieu-Daudé
2023-02-21 17:25 ` Peter Maydell
1 sibling, 0 replies; 28+ messages in thread
From: Philippe Mathieu-Daudé @ 2023-02-21 7:32 UTC (permalink / raw)
To: Richard Henderson, qemu-devel; +Cc: qemu-arm, David Reiss
On 21/2/23 03:19, Richard Henderson wrote:
> The upstream gdb xml only implements {MSP,PSP}{,_NS,S}, but
> go ahead and implement the other system registers as well.
>
> Since there is significant overlap between the two, implement
> them with common code. The only exception is the systemreg
> view of CONTROL, which merges the banked bits as per MRS.
>
> Signed-off-by: David Reiss <dreiss@meta.com>
> [rth: Substatial rewrite using enumerator and shared code.]
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
> target/arm/cpu.h | 2 +
> target/arm/gdbstub.c | 194 +++++++++++++++++++++++++++++++++++++++++++
> 2 files changed, 196 insertions(+)
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
^ permalink raw reply [flat|nested] 28+ messages in thread* Re: [PATCH v2 14/14] target/arm: Implement gdbstub m-profile systemreg and secext
2023-02-21 2:19 ` [PATCH v2 14/14] target/arm: Implement gdbstub m-profile systemreg and secext Richard Henderson
2023-02-21 7:32 ` Philippe Mathieu-Daudé
@ 2023-02-21 17:25 ` Peter Maydell
2023-02-21 17:33 ` Richard Henderson
1 sibling, 1 reply; 28+ messages in thread
From: Peter Maydell @ 2023-02-21 17:25 UTC (permalink / raw)
To: Richard Henderson; +Cc: qemu-devel, qemu-arm, David Reiss
On Tue, 21 Feb 2023 at 02:21, Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> The upstream gdb xml only implements {MSP,PSP}{,_NS,S}, but
> go ahead and implement the other system registers as well.
>
> Since there is significant overlap between the two, implement
> them with common code. The only exception is the systemreg
> view of CONTROL, which merges the banked bits as per MRS.
>
> Signed-off-by: David Reiss <dreiss@meta.com>
> [rth: Substatial rewrite using enumerator and shared code.]
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
> target/arm/cpu.h | 2 +
> target/arm/gdbstub.c | 194 +++++++++++++++++++++++++++++++++++++++++++
> 2 files changed, 196 insertions(+)
>
> diff --git a/target/arm/cpu.h b/target/arm/cpu.h
> index 059fe62eaa..6e97a256fb 100644
> --- a/target/arm/cpu.h
> +++ b/target/arm/cpu.h
> @@ -869,6 +869,8 @@ struct ArchCPU {
>
> DynamicGDBXMLInfo dyn_sysreg_xml;
> DynamicGDBXMLInfo dyn_svereg_xml;
> + DynamicGDBXMLInfo dyn_m_systemreg_xml;
> + DynamicGDBXMLInfo dyn_m_secextreg_xml;
>
> /* Timers used by the generic (architected) timer */
> QEMUTimer *gt_timer[NUM_GTIMERS];
> diff --git a/target/arm/gdbstub.c b/target/arm/gdbstub.c
> index 062c8d447a..fef53e4ef5 100644
> --- a/target/arm/gdbstub.c
> +++ b/target/arm/gdbstub.c
> @@ -322,6 +322,180 @@ static int arm_gen_dynamic_sysreg_xml(CPUState *cs, int base_reg)
> return cpu->dyn_sysreg_xml.num;
> }
>
> +typedef enum {
> + M_SYSREG_MSP,
> + M_SYSREG_PSP,
> + M_SYSREG_PRIMASK,
> + M_SYSREG_CONTROL,
> + M_SYSREG_BASEPRI,
> + M_SYSREG_FAULTMASK,
> + M_SYSREG_MSPLIM,
> + M_SYSREG_PSPLIM,
> +} MProfileSysreg;
> +
> +static const struct {
> + const char *name;
> + int feature;
> +} m_sysreg_def[] = {
> + [M_SYSREG_MSP] = { "msp", ARM_FEATURE_M },
> + [M_SYSREG_PSP] = { "psp", ARM_FEATURE_M },
> + [M_SYSREG_PRIMASK] = { "primask", ARM_FEATURE_M },
> + [M_SYSREG_CONTROL] = { "control", ARM_FEATURE_M },
> + [M_SYSREG_BASEPRI] = { "basepri", ARM_FEATURE_M_MAIN },
> + [M_SYSREG_FAULTMASK] = { "faultmask", ARM_FEATURE_M_MAIN },
> + [M_SYSREG_MSPLIM] = { "msplim", ARM_FEATURE_V8 },
> + [M_SYSREG_PSPLIM] = { "psplim", ARM_FEATURE_V8 },
> +};
> +
> +static uint32_t *m_sysreg_ptr(CPUARMState *env, MProfileSysreg reg, bool sec)
> +{
> + uint32_t *ptr;
> +
> + switch (reg) {
> + case M_SYSREG_MSP:
> + ptr = arm_v7m_get_sp_ptr(env, sec, false, true);
> + break;
> + case M_SYSREG_PSP:
> + ptr = arm_v7m_get_sp_ptr(env, sec, true, true);
> + break;
> + case M_SYSREG_MSPLIM:
> + ptr = &env->v7m.msplim[sec];
> + break;
> + case M_SYSREG_PSPLIM:
> + ptr = &env->v7m.psplim[sec];
> + break;
> + case M_SYSREG_PRIMASK:
> + ptr = &env->v7m.primask[sec];
> + break;
> + case M_SYSREG_BASEPRI:
> + ptr = &env->v7m.basepri[sec];
> + break;
> + case M_SYSREG_FAULTMASK:
> + ptr = &env->v7m.faultmask[sec];
> + break;
> + case M_SYSREG_CONTROL:
> + ptr = &env->v7m.control[sec];
> + break;
> + default:
> + return NULL;
> + }
> + return arm_feature(env, m_sysreg_def[reg].feature) ? ptr : NULL;
> +}
> +
> +static int m_sysreg_get(CPUARMState *env, GByteArray *buf,
> + MProfileSysreg reg, bool secure)
> +{
> + uint32_t *ptr = m_sysreg_ptr(env, reg, secure);
> +
> + if (ptr == NULL) {
> + return 0;
> + }
> + return gdb_get_reg32(buf, *ptr);
> +}
> +
> +static int m_sysreg_set(CPUARMState *env, uint8_t *buf,
> + MProfileSysreg reg, bool secure)
> +{
> + uint32_t *ptr = m_sysreg_ptr(env, reg, secure);
> +
> + if (ptr == NULL) {
> + return 0;
> + }
> + *ptr = ldl_p(buf);
> + return 4;
> +}
> +
> +static int arm_gdb_get_m_systemreg(CPUARMState *env, GByteArray *buf, int reg)
> +{
> + /*
> + * Here, we emulate MRS instruction, where CONTROL has a mix of
> + * banked and non-banked bits.
> + */
> + if (reg == M_SYSREG_CONTROL) {
> + return gdb_get_reg32(buf, arm_v7m_mrs_control(env, env->v7m.secure));
> + }
> + return m_sysreg_get(env, buf, reg, env->v7m.secure);
> +}
> +
> +static int arm_gdb_set_m_systemreg(CPUARMState *env, uint8_t *buf, int reg)
> +{
> + /* Control is a mix of banked and non-banked bits -- disallow write. */
> + if (reg == M_SYSREG_CONTROL) {
> + return 0;
> + }
You also want to enforce the RES0 bits on registers like
PSPLIM, MSPLIM, FAULTMASK, PSP, MSP, if you're going to
implement writes. Effectively you really end up wanting to
get helper_v7m_msr to do the work for you.
> + return m_sysreg_set(env, buf, reg, env->v7m.secure);
> +}
-- PMM
^ permalink raw reply [flat|nested] 28+ messages in thread* Re: [PATCH v2 14/14] target/arm: Implement gdbstub m-profile systemreg and secext
2023-02-21 17:25 ` Peter Maydell
@ 2023-02-21 17:33 ` Richard Henderson
0 siblings, 0 replies; 28+ messages in thread
From: Richard Henderson @ 2023-02-21 17:33 UTC (permalink / raw)
To: Peter Maydell; +Cc: qemu-devel, qemu-arm, David Reiss
On 2/21/23 07:25, Peter Maydell wrote:
> You also want to enforce the RES0 bits on registers like
> PSPLIM, MSPLIM, FAULTMASK, PSP, MSP, if you're going to
> implement writes. Effectively you really end up wanting to
> get helper_v7m_msr to do the work for you.
Ho hum. I should have known it was more complicated than all that.
I should probably just drop the write portion of the patch again for now.
r~
^ permalink raw reply [flat|nested] 28+ messages in thread