* [PATCH v11 00/18] plugins: Allow to read registers
@ 2023-10-12 13:05 Akihiko Odaki
2023-10-12 13:05 ` [PATCH v11 01/18] gdbstub: Add num_regs member to GDBFeature Akihiko Odaki
` (17 more replies)
0 siblings, 18 replies; 24+ messages in thread
From: Akihiko Odaki @ 2023-10-12 13:05 UTC (permalink / raw)
Cc: Alex Bennée, Mikhail Tyutin, Aleksandr Anenkov, qemu-devel,
Philippe Mathieu-Daudé, Akihiko Odaki
Based-on: <20231012054223.37870-1-akihiko.odaki@daynix.com>
("[PATCH 0/4] gdbstub and TCG plugin improvements")
I and other people in the University of Tokyo, where I research processor
design, found TCG plugins are very useful for processor design exploration.
The feature we find missing is the capability to read registers from
plugins. In this series, I propose to add such a capability by reusing
gdbstub code.
The reuse of gdbstub code ensures the long-term stability of the TCG plugin
interface for register access without incurring a burden to maintain yet
another interface for register access.
This process to add TCG plugin involves four major changes. The first one
is to add GDBFeature structure that represents a GDB feature, which usually
includes registers. GDBFeature can be generated from static XML files or
dynamically generated by architecture-specific code. In fact, this is a
refactoring independent of the feature this series adds, and potentially
it's benefitial even without the plugin feature. The plugin feature will
utilize this new structure to describe registers exposed to plugins.
The second one is to make gdb_read_register/gdb_write_register usable
outside of gdbstub context.
The third one is to actually make registers readable for plugins.
The execlog plugin will have new options to demonstrate the new feature.
I also have a plugin that uses this new feature to generate execution
traces for Sniper processor simulator, which is available at:
https://github.com/shioya-lab/sniper/tree/akihikodaki/bb
The below is a summary of patches:
Patch [01, 12] introduces num_regs member to GDBFeature.
Patch 13 adds members useful to identify registers to GDBFeature.
Patch 14 makes registers readable outside of gdbstub context.
Patch [15, 18] add the feature to read registers from plugins.
V10 -> V11:
Fixed QEMU_PLUGIN_CB_RW_REGS.
V9 -> V10:
Rebased to "[PATCH 0/4] gdbstub and TCG plugin improvements".
Dropped patch "plugins: Support C++".
Removed the unnecessary QEMU_PLUGIN_VERSION change.
V8 -> V9:
Rebased to "[PATCH 00/25] October maintainer omnibus pre-PR (tests,
gdbstub, plugins)".
Added patch "target/riscv: Move MISA limits to class".
Added patch "target/riscv: Remove misa_mxl validation".
Added patch "target/riscv: Validate misa_mxl_max only once".
Added patch "plugins: Use different helpers when reading".
Moved contrib/plugins/cc.cc to tests/plugin/cc.cc.
V7 -> V8:
Rebased to "[PATCH v3 00/12] gdbstub and TCG plugin improvements".
Clarified that initialization and exit hooks affect TCG state.
Simplified by adding the core feature to gdb_regs.
V6 -> V7:
Rebased to "[PATCH v2 00/11] gdbstub and TCG plugin improvements".
Replaced functions to get register identifiers.
V5 -> V6:
Rebased to "[PATCH 0/8] gdbstub and TCG plugin improvements".
V4 -> V5:
Corrected g_rw_lock_writer_lock() call. (Richard Henderson)
Replaced abort() with g_assert_not_reached(). (Richard Henderson)
Fixed CSR name leak in target/riscv. (Richard Henderson)
Removed gdb_has_xml variable.
V3 -> V4:
Added execlog changes I forgot to include in the last version.
V2 -> V3:
Added patch "hw/core/cpu: Return static value with gdb_arch_name()".
Added patch "gdbstub: Dynamically allocate target.xml buffer".
(Alex Bennée)
Added patch "gdbstub: Introduce GDBFeatureBuilder". (Alex Bennée)
Dropped Reviewed-by tags for "target/*: Use GDBFeature for dynamic XML".
Changed gdb_find_static_feature() to abort on failure. (Alex Bennée)
Changed the execlog plugin to log the register value only when changed.
(Alex Bennée)
Dropped 0x prefixes for register value logs for conciseness.
V1 -> V2:
Added SPDX-License-Identifier: GPL-2.0-or-later. (Philippe Mathieu-Daudé)
Split long lines. (Philippe Mathieu-Daudé)
Renamed gdb_features to gdb_static_features (Philippe Mathieu-Daudé)
Dropped RFC.
Akihiko Odaki (18):
gdbstub: Add num_regs member to GDBFeature
gdbstub: Introduce gdb_find_static_feature()
gdbstub: Introduce GDBFeatureBuilder
target/arm: Use GDBFeature for dynamic XML
target/ppc: Use GDBFeature for dynamic XML
target/riscv: Use GDBFeature for dynamic XML
gdbstub: Use GDBFeature for gdb_register_coprocessor
gdbstub: Use GDBFeature for GDBRegisterState
gdbstub: Change gdb_get_reg_cb and gdb_set_reg_cb
gdbstub: Simplify XML lookup
gdbstub: Infer number of core registers from XML
hw/core/cpu: Remove gdb_get_dynamic_xml member
gdbstub: Add members to identify registers to GDBFeature
gdbstub: Expose functions to read registers
cpu: Call plugin hooks only when ready
plugins: Use different helpers when reading registers
plugins: Allow to read registers
contrib/plugins: Allow to log registers
docs/devel/tcg-plugins.rst | 10 +-
accel/tcg/plugin-helpers.h | 3 +-
include/exec/gdbstub.h | 39 ++++++-
include/exec/plugin-gen.h | 4 +-
include/hw/core/cpu.h | 11 +-
include/qemu/plugin.h | 3 +
include/qemu/qemu-plugin.h | 50 +++++++-
plugins/plugin.h | 5 +-
target/arm/cpu.h | 27 ++---
target/arm/internals.h | 14 +--
target/hexagon/internal.h | 4 +-
target/microblaze/cpu.h | 4 +-
target/ppc/cpu-qom.h | 4 +-
target/ppc/cpu.h | 3 +-
target/riscv/cpu.h | 5 +-
target/s390x/cpu.h | 2 -
accel/tcg/plugin-gen.c | 41 +++++--
accel/tcg/translator.c | 2 +-
contrib/plugins/execlog.c | 120 ++++++++++++++-----
cpu-target.c | 11 --
gdbstub/gdbstub.c | 216 ++++++++++++++++++++++++++--------
hw/core/cpu-common.c | 15 ++-
plugins/api.c | 34 +++++-
plugins/core.c | 28 +++--
target/arm/cpu.c | 2 -
target/arm/cpu64.c | 1 -
target/arm/gdbstub.c | 221 +++++++++++++++++------------------
target/arm/gdbstub64.c | 117 +++++++++----------
target/avr/cpu.c | 1 -
target/hexagon/cpu.c | 4 +-
target/hexagon/gdbstub.c | 10 +-
target/i386/cpu.c | 2 -
target/loongarch/cpu.c | 2 -
target/loongarch/gdbstub.c | 13 ++-
target/m68k/cpu.c | 1 -
target/m68k/helper.c | 26 +++--
target/microblaze/cpu.c | 6 +-
target/microblaze/gdbstub.c | 9 +-
target/ppc/cpu_init.c | 5 +-
target/ppc/gdbstub.c | 108 ++++++++---------
target/riscv/cpu.c | 15 ---
target/riscv/gdbstub.c | 135 +++++++++++----------
target/rx/cpu.c | 1 -
target/s390x/cpu.c | 1 -
target/s390x/gdbstub.c | 105 ++++++++++-------
plugins/qemu-plugins.symbols | 3 +
scripts/feature_to_c.py | 58 ++++++++-
47 files changed, 943 insertions(+), 558 deletions(-)
--
2.42.0
^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH v11 01/18] gdbstub: Add num_regs member to GDBFeature
2023-10-12 13:05 [PATCH v11 00/18] plugins: Allow to read registers Akihiko Odaki
@ 2023-10-12 13:05 ` Akihiko Odaki
2023-10-12 13:05 ` [PATCH v11 02/18] gdbstub: Introduce gdb_find_static_feature() Akihiko Odaki
` (16 subsequent siblings)
17 siblings, 0 replies; 24+ messages in thread
From: Akihiko Odaki @ 2023-10-12 13:05 UTC (permalink / raw)
Cc: Alex Bennée, Mikhail Tyutin, Aleksandr Anenkov, qemu-devel,
Philippe Mathieu-Daudé, Akihiko Odaki, John Snow,
Cleber Rosa
Currently the number of registers exposed to GDB is written as magic
numbers in code. Derive the number of registers GDB actually see from
XML files to replace the magic numbers in code later.
Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
---
include/exec/gdbstub.h | 1 +
scripts/feature_to_c.py | 46 +++++++++++++++++++++++++++++++++++++++--
2 files changed, 45 insertions(+), 2 deletions(-)
diff --git a/include/exec/gdbstub.h b/include/exec/gdbstub.h
index 1a01c35f8e..a43aa34dad 100644
--- a/include/exec/gdbstub.h
+++ b/include/exec/gdbstub.h
@@ -13,6 +13,7 @@
typedef struct GDBFeature {
const char *xmlname;
const char *xml;
+ int num_regs;
} GDBFeature;
diff --git a/scripts/feature_to_c.py b/scripts/feature_to_c.py
index bcbcb83beb..e04d6b2df7 100755
--- a/scripts/feature_to_c.py
+++ b/scripts/feature_to_c.py
@@ -1,7 +1,7 @@
#!/usr/bin/env python3
# SPDX-License-Identifier: GPL-2.0-or-later
-import os, sys
+import os, sys, xml.etree.ElementTree
def writeliteral(indent, bytes):
sys.stdout.write(' ' * indent)
@@ -39,10 +39,52 @@ def writeliteral(indent, bytes):
with open(input, 'rb') as file:
read = file.read()
+ parser = xml.etree.ElementTree.XMLPullParser(['start', 'end'])
+ parser.feed(read)
+ events = parser.read_events()
+ event, element = next(events)
+ if event != 'start':
+ sys.stderr.write(f'unexpected event: {event}\n')
+ exit(1)
+ if element.tag != 'feature':
+ sys.stderr.write(f'unexpected start tag: {element.tag}\n')
+ exit(1)
+
+ regnum = 0
+ regnums = []
+ tags = ['feature']
+ for event, element in events:
+ if event == 'end':
+ if element.tag != tags[len(tags) - 1]:
+ sys.stderr.write(f'unexpected end tag: {element.tag}\n')
+ exit(1)
+
+ tags.pop()
+ if element.tag == 'feature':
+ break
+ elif event == 'start':
+ if len(tags) < 2 and element.tag == 'reg':
+ if 'regnum' in element.attrib:
+ regnum = int(element.attrib['regnum'])
+
+ regnums.append(regnum)
+ regnum += 1
+
+ tags.append(element.tag)
+ else:
+ raise Exception(f'unexpected event: {event}\n')
+
+ if len(tags):
+ sys.stderr.write('unterminated feature tag\n')
+ exit(1)
+
+ base_reg = min(regnums)
+ num_regs = max(regnums) - base_reg + 1 if len(regnums) else 0
+
sys.stdout.write(' {\n')
writeliteral(8, bytes(os.path.basename(input), 'utf-8'))
sys.stdout.write(',\n')
writeliteral(8, read)
- sys.stdout.write('\n },\n')
+ sys.stdout.write(f',\n {num_regs},\n }},\n')
sys.stdout.write(' { NULL }\n};\n')
--
2.42.0
^ permalink raw reply related [flat|nested] 24+ messages in thread
* [PATCH v11 02/18] gdbstub: Introduce gdb_find_static_feature()
2023-10-12 13:05 [PATCH v11 00/18] plugins: Allow to read registers Akihiko Odaki
2023-10-12 13:05 ` [PATCH v11 01/18] gdbstub: Add num_regs member to GDBFeature Akihiko Odaki
@ 2023-10-12 13:05 ` Akihiko Odaki
2023-10-12 13:05 ` [PATCH v11 03/18] gdbstub: Introduce GDBFeatureBuilder Akihiko Odaki
` (15 subsequent siblings)
17 siblings, 0 replies; 24+ messages in thread
From: Akihiko Odaki @ 2023-10-12 13:05 UTC (permalink / raw)
Cc: Alex Bennée, Mikhail Tyutin, Aleksandr Anenkov, qemu-devel,
Philippe Mathieu-Daudé, Akihiko Odaki, Richard Henderson
This function is useful to determine the number of registers exposed to
GDB from the XML name.
Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
---
include/exec/gdbstub.h | 2 ++
gdbstub/gdbstub.c | 13 +++++++++++++
2 files changed, 15 insertions(+)
diff --git a/include/exec/gdbstub.h b/include/exec/gdbstub.h
index a43aa34dad..071021415a 100644
--- a/include/exec/gdbstub.h
+++ b/include/exec/gdbstub.h
@@ -44,6 +44,8 @@ void gdb_register_coprocessor(CPUState *cpu,
*/
int gdbserver_start(const char *port_or_device);
+const GDBFeature *gdb_find_static_feature(const char *xmlname);
+
void gdb_set_stop_cpu(CPUState *cpu);
/* in gdbstub-xml.c, generated by scripts/feature_to_c.py */
diff --git a/gdbstub/gdbstub.c b/gdbstub/gdbstub.c
index b1532118d1..44c9167542 100644
--- a/gdbstub/gdbstub.c
+++ b/gdbstub/gdbstub.c
@@ -420,6 +420,19 @@ static const char *get_feature_xml(const char *p, const char **newp,
return NULL;
}
+const GDBFeature *gdb_find_static_feature(const char *xmlname)
+{
+ const GDBFeature *feature;
+
+ for (feature = gdb_static_features; feature->xmlname; feature++) {
+ if (!strcmp(feature->xmlname, xmlname)) {
+ return feature;
+ }
+ }
+
+ g_assert_not_reached();
+}
+
static int gdb_read_register(CPUState *cpu, GByteArray *buf, int reg)
{
CPUClass *cc = CPU_GET_CLASS(cpu);
--
2.42.0
^ permalink raw reply related [flat|nested] 24+ messages in thread
* [PATCH v11 03/18] gdbstub: Introduce GDBFeatureBuilder
2023-10-12 13:05 [PATCH v11 00/18] plugins: Allow to read registers Akihiko Odaki
2023-10-12 13:05 ` [PATCH v11 01/18] gdbstub: Add num_regs member to GDBFeature Akihiko Odaki
2023-10-12 13:05 ` [PATCH v11 02/18] gdbstub: Introduce gdb_find_static_feature() Akihiko Odaki
@ 2023-10-12 13:05 ` Akihiko Odaki
2023-10-12 13:05 ` [PATCH v11 04/18] target/arm: Use GDBFeature for dynamic XML Akihiko Odaki
` (14 subsequent siblings)
17 siblings, 0 replies; 24+ messages in thread
From: Akihiko Odaki @ 2023-10-12 13:05 UTC (permalink / raw)
Cc: Alex Bennée, Mikhail Tyutin, Aleksandr Anenkov, qemu-devel,
Philippe Mathieu-Daudé, Akihiko Odaki, Richard Henderson
GDBFeatureBuilder unifies the logic to generate dynamic GDBFeature.
Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
---
include/exec/gdbstub.h | 20 ++++++++++++++
gdbstub/gdbstub.c | 59 ++++++++++++++++++++++++++++++++++++++++++
2 files changed, 79 insertions(+)
diff --git a/include/exec/gdbstub.h b/include/exec/gdbstub.h
index 071021415a..cbd582260a 100644
--- a/include/exec/gdbstub.h
+++ b/include/exec/gdbstub.h
@@ -16,6 +16,11 @@ typedef struct GDBFeature {
int num_regs;
} GDBFeature;
+typedef struct GDBFeatureBuilder {
+ GDBFeature *feature;
+ GPtrArray *xml;
+} GDBFeatureBuilder;
+
/* Get or set a register. Returns the size of the register. */
typedef int (*gdb_get_reg_cb)(CPUArchState *env, GByteArray *buf, int reg);
@@ -44,6 +49,21 @@ void gdb_register_coprocessor(CPUState *cpu,
*/
int gdbserver_start(const char *port_or_device);
+void gdb_feature_builder_init(GDBFeatureBuilder *builder, GDBFeature *feature,
+ const char *name, const char *xmlname);
+
+void G_GNUC_PRINTF(2, 3)
+gdb_feature_builder_append_tag(const GDBFeatureBuilder *builder,
+ const char *format, ...);
+
+int gdb_feature_builder_append_reg(const GDBFeatureBuilder *builder,
+ const char *name,
+ int bitsize,
+ const char *type,
+ const char *group);
+
+void gdb_feature_builder_end(const GDBFeatureBuilder *builder);
+
const GDBFeature *gdb_find_static_feature(const char *xmlname);
void gdb_set_stop_cpu(CPUState *cpu);
diff --git a/gdbstub/gdbstub.c b/gdbstub/gdbstub.c
index 44c9167542..258dd54c90 100644
--- a/gdbstub/gdbstub.c
+++ b/gdbstub/gdbstub.c
@@ -420,6 +420,65 @@ static const char *get_feature_xml(const char *p, const char **newp,
return NULL;
}
+void gdb_feature_builder_init(GDBFeatureBuilder *builder, GDBFeature *feature,
+ const char *name, const char *xmlname)
+{
+ char *header = g_markup_printf_escaped(
+ "<?xml version=\"1.0\"?>"
+ "<!DOCTYPE target SYSTEM \"gdb-target.dtd\">"
+ "<feature name=\"%s\">",
+ name);
+
+ builder->feature = feature;
+ builder->xml = g_ptr_array_new();
+ g_ptr_array_add(builder->xml, header);
+ feature->xmlname = xmlname;
+ feature->num_regs = 0;
+}
+
+void gdb_feature_builder_append_tag(const GDBFeatureBuilder *builder,
+ const char *format, ...)
+{
+ va_list ap;
+ va_start(ap, format);
+ g_ptr_array_add(builder->xml, g_markup_vprintf_escaped(format, ap));
+ va_end(ap);
+}
+
+int gdb_feature_builder_append_reg(const GDBFeatureBuilder *builder,
+ const char *name,
+ int bitsize,
+ const char *type,
+ const char *group)
+{
+ if (group) {
+ gdb_feature_builder_append_tag(
+ builder,
+ "<reg name=\"%s\" bitsize=\"%d\" type=\"%s\" group=\"%s\"/>",
+ name, bitsize, type, group);
+ } else {
+ gdb_feature_builder_append_tag(
+ builder, "<reg name=\"%s\" bitsize=\"%d\" type=\"%s\"/>",
+ name, bitsize, type);
+ }
+
+ return builder->feature->num_regs++;
+}
+
+void gdb_feature_builder_end(const GDBFeatureBuilder *builder)
+{
+ g_ptr_array_add(builder->xml, (void *)"</feature>");
+ g_ptr_array_add(builder->xml, NULL);
+
+ builder->feature->xml = g_strjoinv(NULL, (void *)builder->xml->pdata);
+
+ for (guint i = 0; i < builder->xml->len - 2; i++) {
+ g_free(g_ptr_array_index(builder->xml, i));
+ }
+
+ g_ptr_array_free(builder->xml, TRUE);
+}
+
const GDBFeature *gdb_find_static_feature(const char *xmlname)
{
const GDBFeature *feature;
--
2.42.0
^ permalink raw reply related [flat|nested] 24+ messages in thread
* [PATCH v11 04/18] target/arm: Use GDBFeature for dynamic XML
2023-10-12 13:05 [PATCH v11 00/18] plugins: Allow to read registers Akihiko Odaki
` (2 preceding siblings ...)
2023-10-12 13:05 ` [PATCH v11 03/18] gdbstub: Introduce GDBFeatureBuilder Akihiko Odaki
@ 2023-10-12 13:05 ` Akihiko Odaki
2023-10-12 13:05 ` [PATCH v11 05/18] target/ppc: " Akihiko Odaki
` (13 subsequent siblings)
17 siblings, 0 replies; 24+ messages in thread
From: Akihiko Odaki @ 2023-10-12 13:05 UTC (permalink / raw)
Cc: Alex Bennée, Mikhail Tyutin, Aleksandr Anenkov, qemu-devel,
Philippe Mathieu-Daudé, Akihiko Odaki, Richard Henderson,
Peter Maydell, open list:ARM TCG CPUs
In preparation for a change to use GDBFeature as a parameter of
gdb_register_coprocessor(), convert the internal representation of
dynamic feature from plain XML to GDBFeature.
Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>
Acked-by: Richard Henderson <richard.henderson@linaro.org>
---
target/arm/cpu.h | 21 +++----
target/arm/internals.h | 2 +-
target/arm/gdbstub.c | 134 ++++++++++++++++++-----------------------
target/arm/gdbstub64.c | 90 ++++++++++++---------------
4 files changed, 109 insertions(+), 138 deletions(-)
diff --git a/target/arm/cpu.h b/target/arm/cpu.h
index a9edfb8353..3702ddaab8 100644
--- a/target/arm/cpu.h
+++ b/target/arm/cpu.h
@@ -25,6 +25,7 @@
#include "hw/registerfields.h"
#include "cpu-qom.h"
#include "exec/cpu-defs.h"
+#include "exec/gdbstub.h"
#include "qapi/qapi-types-common.h"
/* ARM processors have a weak memory model */
@@ -136,23 +137,21 @@ enum {
*/
/**
- * DynamicGDBXMLInfo:
- * @desc: Contains the XML descriptions.
- * @num: Number of the registers in this XML seen by GDB.
+ * DynamicGDBFeatureInfo:
+ * @desc: Contains the feature descriptions.
* @data: A union with data specific to the set of registers
* @cpregs_keys: Array that contains the corresponding Key of
* a given cpreg with the same order of the cpreg
* in the XML description.
*/
-typedef struct DynamicGDBXMLInfo {
- char *desc;
- int num;
+typedef struct DynamicGDBFeatureInfo {
+ GDBFeature desc;
union {
struct {
uint32_t *keys;
} cpregs;
} data;
-} DynamicGDBXMLInfo;
+} DynamicGDBFeatureInfo;
/* CPU state for each instance of a generic timer (in cp15 c14) */
typedef struct ARMGenericTimer {
@@ -880,10 +879,10 @@ struct ArchCPU {
uint64_t *cpreg_vmstate_values;
int32_t cpreg_vmstate_array_len;
- DynamicGDBXMLInfo dyn_sysreg_xml;
- DynamicGDBXMLInfo dyn_svereg_xml;
- DynamicGDBXMLInfo dyn_m_systemreg_xml;
- DynamicGDBXMLInfo dyn_m_secextreg_xml;
+ DynamicGDBFeatureInfo dyn_sysreg_feature;
+ DynamicGDBFeatureInfo dyn_svereg_feature;
+ DynamicGDBFeatureInfo dyn_m_systemreg_feature;
+ DynamicGDBFeatureInfo dyn_m_secextreg_feature;
/* Timers used by the generic (architected) timer */
QEMUTimer *gt_timer[NUM_GTIMERS];
diff --git a/target/arm/internals.h b/target/arm/internals.h
index 1dd9182a54..09350e96da 100644
--- a/target/arm/internals.h
+++ b/target/arm/internals.h
@@ -1440,7 +1440,7 @@ static inline uint64_t pmu_counter_mask(CPUARMState *env)
}
#ifdef TARGET_AARCH64
-int arm_gen_dynamic_svereg_xml(CPUState *cpu, int base_reg);
+GDBFeature *arm_gen_dynamic_svereg_feature(CPUState *cpu);
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 b7ace24bfc..fc0aa6f235 100644
--- a/target/arm/gdbstub.c
+++ b/target/arm/gdbstub.c
@@ -25,11 +25,10 @@
#include "internals.h"
#include "cpregs.h"
-typedef struct RegisterSysregXmlParam {
+typedef struct RegisterSysregFeatureParam {
CPUState *cs;
- GString *s;
- int n;
-} RegisterSysregXmlParam;
+ GDBFeatureBuilder builder;
+} RegisterSysregFeatureParam;
/* Old gdb always expect FPA registers. Newer (xml-aware) gdb only expect
whatever the target description contains. Due to a historical mishap
@@ -215,7 +214,7 @@ static int arm_gdb_get_sysreg(CPUARMState *env, GByteArray *buf, int reg)
const ARMCPRegInfo *ri;
uint32_t key;
- key = cpu->dyn_sysreg_xml.data.cpregs.keys[reg];
+ key = cpu->dyn_sysreg_feature.data.cpregs.keys[reg];
ri = get_arm_cp_reginfo(cpu->cp_regs, key);
if (ri) {
if (cpreg_field_is_64bit(ri)) {
@@ -232,34 +231,32 @@ static int arm_gdb_set_sysreg(CPUARMState *env, uint8_t *buf, int reg)
return 0;
}
-static void arm_gen_one_xml_sysreg_tag(GString *s, DynamicGDBXMLInfo *dyn_xml,
+static void arm_gen_one_feature_sysreg(GDBFeatureBuilder *builder,
+ DynamicGDBFeatureInfo *dyn_feature,
ARMCPRegInfo *ri, uint32_t ri_key,
- int bitsize, int regnum)
+ int bitsize)
{
- g_string_append_printf(s, "<reg name=\"%s\"", ri->name);
- g_string_append_printf(s, " bitsize=\"%d\"", bitsize);
- g_string_append_printf(s, " regnum=\"%d\"", regnum);
- g_string_append_printf(s, " group=\"cp_regs\"/>");
- dyn_xml->data.cpregs.keys[dyn_xml->num] = ri_key;
- dyn_xml->num++;
+ int num = gdb_feature_builder_append_reg(builder, ri->name, bitsize,
+ "int", "cp_regs");
+
+ dyn_feature->data.cpregs.keys[num] = ri_key;
}
-static void arm_register_sysreg_for_xml(gpointer key, gpointer value,
- gpointer p)
+static void arm_register_sysreg_for_feature(gpointer key, gpointer value,
+ gpointer p)
{
uint32_t ri_key = (uintptr_t)key;
ARMCPRegInfo *ri = value;
- RegisterSysregXmlParam *param = (RegisterSysregXmlParam *)p;
- GString *s = param->s;
+ RegisterSysregFeatureParam *param = p;
ARMCPU *cpu = ARM_CPU(param->cs);
CPUARMState *env = &cpu->env;
- DynamicGDBXMLInfo *dyn_xml = &cpu->dyn_sysreg_xml;
+ DynamicGDBFeatureInfo *dyn_feature = &cpu->dyn_sysreg_feature;
if (!(ri->type & (ARM_CP_NO_RAW | ARM_CP_NO_GDB))) {
if (arm_feature(env, ARM_FEATURE_AARCH64)) {
if (ri->state == ARM_CP_STATE_AA64) {
- arm_gen_one_xml_sysreg_tag(s , dyn_xml, ri, ri_key, 64,
- param->n++);
+ arm_gen_one_feature_sysreg(¶m->builder, dyn_feature,
+ ri, ri_key, 64);
}
} else {
if (ri->state == ARM_CP_STATE_AA32) {
@@ -268,32 +265,31 @@ static void arm_register_sysreg_for_xml(gpointer key, gpointer value,
return;
}
if (ri->type & ARM_CP_64BIT) {
- arm_gen_one_xml_sysreg_tag(s , dyn_xml, ri, ri_key, 64,
- param->n++);
+ arm_gen_one_feature_sysreg(¶m->builder, dyn_feature,
+ ri, ri_key, 64);
} else {
- arm_gen_one_xml_sysreg_tag(s , dyn_xml, ri, ri_key, 32,
- param->n++);
+ arm_gen_one_feature_sysreg(¶m->builder, dyn_feature,
+ ri, ri_key, 32);
}
}
}
}
}
-static int arm_gen_dynamic_sysreg_xml(CPUState *cs, int base_reg)
+static GDBFeature *arm_gen_dynamic_sysreg_feature(CPUState *cs)
{
ARMCPU *cpu = ARM_CPU(cs);
- GString *s = g_string_new(NULL);
- RegisterSysregXmlParam param = {cs, s, base_reg};
-
- cpu->dyn_sysreg_xml.num = 0;
- cpu->dyn_sysreg_xml.data.cpregs.keys = g_new(uint32_t, g_hash_table_size(cpu->cp_regs));
- 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.qemu.gdb.arm.sys.regs\">");
- g_hash_table_foreach(cpu->cp_regs, arm_register_sysreg_for_xml, ¶m);
- g_string_append_printf(s, "</feature>");
- cpu->dyn_sysreg_xml.desc = g_string_free(s, false);
- return cpu->dyn_sysreg_xml.num;
+ RegisterSysregFeatureParam param = {cs};
+ gsize num_regs = g_hash_table_size(cpu->cp_regs);
+
+ gdb_feature_builder_init(¶m.builder,
+ &cpu->dyn_sysreg_feature.desc,
+ "org.qemu.gdb.arm.sys.regs",
+ "system-registers.xml");
+ cpu->dyn_sysreg_feature.data.cpregs.keys = g_new(uint32_t, num_regs);
+ g_hash_table_foreach(cpu->cp_regs, arm_register_sysreg_for_feature, ¶m);
+ gdb_feature_builder_end(¶m.builder);
+ return &cpu->dyn_sysreg_feature.desc;
}
#ifdef CONFIG_TCG
@@ -385,31 +381,26 @@ static int arm_gdb_set_m_systemreg(CPUARMState *env, uint8_t *buf, int reg)
return 0; /* TODO */
}
-static int arm_gen_dynamic_m_systemreg_xml(CPUState *cs, int orig_base_reg)
+static GDBFeature *arm_gen_dynamic_m_systemreg_feature(CPUState *cs)
{
ARMCPU *cpu = ARM_CPU(cs);
CPUARMState *env = &cpu->env;
- GString *s = g_string_new(NULL);
- int base_reg = orig_base_reg;
+ GDBFeatureBuilder builder;
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");
+ gdb_feature_builder_init(&builder, &cpu->dyn_m_systemreg_feature.desc,
+ "org.gnu.gdb.arm.m-system", "arm-m-system.xml");
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++);
+ gdb_feature_builder_append_reg(&builder, m_sysreg_def[i].name, 32,
+ "int", NULL);
}
}
- 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;
+ gdb_feature_builder_end(&builder);
- return cpu->dyn_m_systemreg_xml.num;
+ return &cpu->dyn_m_systemreg_feature.desc;
}
#ifndef CONFIG_USER_ONLY
@@ -427,31 +418,26 @@ static int arm_gdb_set_m_secextreg(CPUARMState *env, uint8_t *buf, int reg)
return 0; /* TODO */
}
-static int arm_gen_dynamic_m_secextreg_xml(CPUState *cs, int orig_base_reg)
+static GDBFeature *arm_gen_dynamic_m_secextreg_feature(CPUState *cs)
{
ARMCPU *cpu = ARM_CPU(cs);
- GString *s = g_string_new(NULL);
- int base_reg = orig_base_reg;
+ GDBFeatureBuilder builder;
+ char *name;
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");
+ gdb_feature_builder_init(&builder, &cpu->dyn_m_secextreg_feature.desc,
+ "org.gnu.gdb.arm.secext", "arm-m-secext.xml");
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++);
+ name = g_strconcat(m_sysreg_def[i].name, "_ns", NULL);
+ gdb_feature_builder_append_reg(&builder, name, 32, "int", NULL);
+ name = g_strconcat(m_sysreg_def[i].name, "_s", NULL);
+ gdb_feature_builder_append_reg(&builder, name, 32, "int", NULL);
}
- 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;
+ gdb_feature_builder_end(&builder);
- return cpu->dyn_m_secextreg_xml.num;
+ return &cpu->dyn_m_secextreg_feature.desc;
}
#endif
#endif /* CONFIG_TCG */
@@ -461,14 +447,14 @@ const char *arm_gdb_get_dynamic_xml(CPUState *cs, const char *xmlname)
ARMCPU *cpu = ARM_CPU(cs);
if (strcmp(xmlname, "system-registers.xml") == 0) {
- return cpu->dyn_sysreg_xml.desc;
+ return cpu->dyn_sysreg_feature.desc.xml;
} else if (strcmp(xmlname, "sve-registers.xml") == 0) {
- return cpu->dyn_svereg_xml.desc;
+ return cpu->dyn_svereg_feature.desc.xml;
} else if (strcmp(xmlname, "arm-m-system.xml") == 0) {
- return cpu->dyn_m_systemreg_xml.desc;
+ return cpu->dyn_m_systemreg_feature.desc.xml;
#ifndef CONFIG_USER_ONLY
} else if (strcmp(xmlname, "arm-m-secext.xml") == 0) {
- return cpu->dyn_m_secextreg_xml.desc;
+ return cpu->dyn_m_secextreg_feature.desc.xml;
#endif
}
return NULL;
@@ -486,7 +472,7 @@ void arm_cpu_register_gdb_regs_for_features(ARMCPU *cpu)
*/
#ifdef TARGET_AARCH64
if (isar_feature_aa64_sve(&cpu->isar)) {
- int nreg = arm_gen_dynamic_svereg_xml(cs, cs->gdb_num_regs);
+ int nreg = arm_gen_dynamic_svereg_feature(cs)->num_regs;
gdb_register_coprocessor(cs, aarch64_gdb_get_sve_reg,
aarch64_gdb_set_sve_reg, nreg,
"sve-registers.xml", 0);
@@ -532,20 +518,20 @@ void arm_cpu_register_gdb_regs_for_features(ARMCPU *cpu)
1, "arm-m-profile-mve.xml", 0);
}
gdb_register_coprocessor(cs, arm_gdb_get_sysreg, arm_gdb_set_sysreg,
- arm_gen_dynamic_sysreg_xml(cs, cs->gdb_num_regs),
+ arm_gen_dynamic_sysreg_feature(cs)->num_regs,
"system-registers.xml", 0);
#ifdef CONFIG_TCG
if (arm_feature(env, ARM_FEATURE_M) && tcg_enabled()) {
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_gen_dynamic_m_systemreg_feature(cs)->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_gen_dynamic_m_secextreg_feature(cs)->num_regs,
"arm-m-secext.xml", 0);
}
#endif
diff --git a/target/arm/gdbstub64.c b/target/arm/gdbstub64.c
index d7b79a6589..632ac2a520 100644
--- a/target/arm/gdbstub64.c
+++ b/target/arm/gdbstub64.c
@@ -247,7 +247,7 @@ int aarch64_gdb_set_pauth_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(GDBFeatureBuilder *builder, int reg_width,
const char *name)
{
struct TypeSize {
@@ -282,10 +282,10 @@ static void output_vector_union_type(GString *s, int reg_width,
/* First define types and totals in a whole VL */
for (i = 0; i < ARRAY_SIZE(vec_lanes); i++) {
- g_string_append_printf(s,
- "<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);
+ gdb_feature_builder_append_tag(
+ builder, "<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);
}
/*
@@ -296,86 +296,72 @@ static void output_vector_union_type(GString *s, int reg_width,
for (i = 0; i < ARRAY_SIZE(suf); i++) {
int bits = 8 << i;
- g_string_append_printf(s, "<union id=\"%sn%c\">", name, suf[i]);
+ gdb_feature_builder_append_tag(builder, "<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=\"%s%c%c\"/>",
- vec_lanes[j].suffix, name,
- vec_lanes[j].sz, vec_lanes[j].suffix);
+ gdb_feature_builder_append_tag(
+ builder, "<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>");
+ gdb_feature_builder_append_tag(builder, "</union>");
}
/* And now the final union of unions */
- g_string_append_printf(s, "<union id=\"%s\">", name);
+ gdb_feature_builder_append_tag(builder, "<union id=\"%s\">", name);
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]);
+ gdb_feature_builder_append_tag(builder,
+ "<field name=\"%c\" type=\"%sn%c\"/>",
+ suf[i], name, suf[i]);
}
- g_string_append(s, "</union>");
+ gdb_feature_builder_append_tag(builder, "</union>");
}
-int arm_gen_dynamic_svereg_xml(CPUState *cs, int orig_base_reg)
+GDBFeature *arm_gen_dynamic_svereg_feature(CPUState *cs)
{
ARMCPU *cpu = ARM_CPU(cs);
- 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;
+ GDBFeatureBuilder builder;
+ char *name;
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\">");
+ gdb_feature_builder_init(&builder, &cpu->dyn_svereg_feature.desc,
+ "org.gnu.gdb.aarch64.sve", "sve-registers.xml");
/* Create the vector union type. */
- output_vector_union_type(s, reg_width, "svev");
+ output_vector_union_type(&builder, reg_width, "svev");
/* Create the predicate vector type. */
- g_string_append_printf(s,
- "<vector id=\"svep\" type=\"uint8\" count=\"%d\"/>",
- pred_width / 8);
+ gdb_feature_builder_append_tag(
+ &builder, "<vector id=\"svep\" type=\"uint8\" count=\"%d\"/>",
+ pred_width / 8);
/* 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++);
+ name = g_strdup_printf("z%d", i);
+ gdb_feature_builder_append_reg(&builder, name, reg_width, "svev", NULL);
}
/* 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++);
+ gdb_feature_builder_append_reg(&builder, "fpsr", 32, "int", "float");
+ gdb_feature_builder_append_reg(&builder, "fpcr", 32, "int", "float");
/* 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, pred_width, base_reg++);
+ name = g_strdup_printf("p%d", i);
+ gdb_feature_builder_append_reg(&builder, name, pred_width,
+ "svep", NULL);
}
- g_string_append_printf(s,
- "<reg name=\"ffr\" bitsize=\"%d\""
- " regnum=\"%d\" group=\"vector\""
- " type=\"svep\"/>",
- pred_width, base_reg++);
+ gdb_feature_builder_append_reg(&builder, "ffr", pred_width,
+ "svep", "vector");
/* Define the vector length pseudo-register. */
- g_string_append_printf(s,
- "<reg name=\"vg\" bitsize=\"64\""
- " regnum=\"%d\" type=\"int\"/>",
- base_reg++);
+ gdb_feature_builder_append_reg(&builder, "vg", 64, "int", NULL);
- g_string_append_printf(s, "</feature>");
+ gdb_feature_builder_end(&builder);
- info->desc = g_string_free(s, false);
- info->num = base_reg - orig_base_reg;
- return info->num;
+ return &cpu->dyn_svereg_feature.desc;
}
--
2.42.0
^ permalink raw reply related [flat|nested] 24+ messages in thread
* [PATCH v11 05/18] target/ppc: Use GDBFeature for dynamic XML
2023-10-12 13:05 [PATCH v11 00/18] plugins: Allow to read registers Akihiko Odaki
` (3 preceding siblings ...)
2023-10-12 13:05 ` [PATCH v11 04/18] target/arm: Use GDBFeature for dynamic XML Akihiko Odaki
@ 2023-10-12 13:05 ` Akihiko Odaki
2023-10-12 13:06 ` [PATCH v11 06/18] target/riscv: " Akihiko Odaki
` (12 subsequent siblings)
17 siblings, 0 replies; 24+ messages in thread
From: Akihiko Odaki @ 2023-10-12 13:05 UTC (permalink / raw)
Cc: Alex Bennée, Mikhail Tyutin, Aleksandr Anenkov, qemu-devel,
Philippe Mathieu-Daudé, Akihiko Odaki, Richard Henderson,
Nicholas Piggin, Daniel Henrique Barboza, Cédric Le Goater,
open list:PowerPC TCG CPUs
In preparation for a change to use GDBFeature as a parameter of
gdb_register_coprocessor(), convert the internal representation of
dynamic feature from plain XML to GDBFeature.
Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
---
target/ppc/cpu-qom.h | 4 ++--
target/ppc/cpu.h | 2 +-
target/ppc/cpu_init.c | 2 +-
target/ppc/gdbstub.c | 45 ++++++++++++++-----------------------------
4 files changed, 18 insertions(+), 35 deletions(-)
diff --git a/target/ppc/cpu-qom.h b/target/ppc/cpu-qom.h
index be33786bd8..8d5ebba5d3 100644
--- a/target/ppc/cpu-qom.h
+++ b/target/ppc/cpu-qom.h
@@ -20,6 +20,7 @@
#ifndef QEMU_PPC_CPU_QOM_H
#define QEMU_PPC_CPU_QOM_H
+#include "exec/gdbstub.h"
#include "hw/core/cpu.h"
#include "qom/object.h"
@@ -186,8 +187,7 @@ struct PowerPCCPUClass {
int bfd_mach;
uint32_t l1_dcache_size, l1_icache_size;
#ifndef CONFIG_USER_ONLY
- unsigned int gdb_num_sprs;
- const char *gdb_spr_xml;
+ GDBFeature gdb_spr;
#endif
const PPCHash64Options *hash64_opts;
struct ppc_radix_page_info *radix_page_info;
diff --git a/target/ppc/cpu.h b/target/ppc/cpu.h
index 30392ebeee..84cdc3055f 100644
--- a/target/ppc/cpu.h
+++ b/target/ppc/cpu.h
@@ -1384,7 +1384,7 @@ int ppc_cpu_gdb_write_register(CPUState *cpu, uint8_t *buf, int reg);
int ppc_cpu_gdb_write_register_apple(CPUState *cpu, uint8_t *buf, int reg);
#ifndef CONFIG_USER_ONLY
hwaddr ppc_cpu_get_phys_page_debug(CPUState *cpu, vaddr addr);
-void ppc_gdb_gen_spr_xml(PowerPCCPU *cpu);
+void ppc_gdb_gen_spr_feature(PowerPCCPU *cpu);
const char *ppc_gdb_get_dynamic_xml(CPUState *cs, const char *xml_name);
#endif
int ppc64_cpu_write_elf64_note(WriteCoreDumpFunction f, CPUState *cs,
diff --git a/target/ppc/cpu_init.c b/target/ppc/cpu_init.c
index 40fe14a6c2..af43e73af2 100644
--- a/target/ppc/cpu_init.c
+++ b/target/ppc/cpu_init.c
@@ -6683,7 +6683,7 @@ static void init_ppc_proc(PowerPCCPU *cpu)
(*pcc->init_proc)(env);
#if !defined(CONFIG_USER_ONLY)
- ppc_gdb_gen_spr_xml(cpu);
+ ppc_gdb_gen_spr_feature(cpu);
#endif
/* MSR bits & flags consistency checks */
diff --git a/target/ppc/gdbstub.c b/target/ppc/gdbstub.c
index ec5731e5d6..43de40ac36 100644
--- a/target/ppc/gdbstub.c
+++ b/target/ppc/gdbstub.c
@@ -300,15 +300,21 @@ int ppc_cpu_gdb_write_register_apple(CPUState *cs, uint8_t *mem_buf, int n)
}
#ifndef CONFIG_USER_ONLY
-void ppc_gdb_gen_spr_xml(PowerPCCPU *cpu)
+void ppc_gdb_gen_spr_feature(PowerPCCPU *cpu)
{
PowerPCCPUClass *pcc = POWERPC_CPU_GET_CLASS(cpu);
CPUPPCState *env = &cpu->env;
- GString *xml;
- char *spr_name;
+ GDBFeatureBuilder builder;
unsigned int num_regs = 0;
int i;
+ if (pcc->gdb_spr.xml) {
+ return;
+ }
+
+ gdb_feature_builder_init(&builder, &pcc->gdb_spr,
+ "org.qemu.power.spr", "power-spr.xml");
+
for (i = 0; i < ARRAY_SIZE(env->spr_cb); i++) {
ppc_spr_t *spr = &env->spr_cb[i];
@@ -326,35 +332,12 @@ void ppc_gdb_gen_spr_xml(PowerPCCPU *cpu)
*/
spr->gdb_id = num_regs;
num_regs++;
- }
-
- if (pcc->gdb_spr_xml) {
- return;
- }
-
- xml = g_string_new("<?xml version=\"1.0\"?>");
- g_string_append(xml, "<!DOCTYPE target SYSTEM \"gdb-target.dtd\">");
- g_string_append(xml, "<feature name=\"org.qemu.power.spr\">");
- for (i = 0; i < ARRAY_SIZE(env->spr_cb); i++) {
- ppc_spr_t *spr = &env->spr_cb[i];
-
- if (!spr->name) {
- continue;
- }
-
- spr_name = g_ascii_strdown(spr->name, -1);
- g_string_append_printf(xml, "<reg name=\"%s\"", spr_name);
- g_free(spr_name);
-
- g_string_append_printf(xml, " bitsize=\"%d\"", TARGET_LONG_BITS);
- g_string_append(xml, " group=\"spr\"/>");
+ gdb_feature_builder_append_reg(&builder, g_ascii_strdown(spr->name, -1),
+ TARGET_LONG_BITS, "int", "spr");
}
- g_string_append(xml, "</feature>");
-
- pcc->gdb_num_sprs = num_regs;
- pcc->gdb_spr_xml = g_string_free(xml, false);
+ gdb_feature_builder_end(&builder);
}
const char *ppc_gdb_get_dynamic_xml(CPUState *cs, const char *xml_name)
@@ -362,7 +345,7 @@ const char *ppc_gdb_get_dynamic_xml(CPUState *cs, const char *xml_name)
PowerPCCPUClass *pcc = POWERPC_CPU_GET_CLASS(cs);
if (strcmp(xml_name, "power-spr.xml") == 0) {
- return pcc->gdb_spr_xml;
+ return pcc->gdb_spr.xml;
}
return NULL;
}
@@ -600,6 +583,6 @@ void ppc_gdb_init(CPUState *cs, PowerPCCPUClass *pcc)
}
#ifndef CONFIG_USER_ONLY
gdb_register_coprocessor(cs, gdb_get_spr_reg, gdb_set_spr_reg,
- pcc->gdb_num_sprs, "power-spr.xml", 0);
+ pcc->gdb_spr.num_regs, "power-spr.xml", 0);
#endif
}
--
2.42.0
^ permalink raw reply related [flat|nested] 24+ messages in thread
* [PATCH v11 06/18] target/riscv: Use GDBFeature for dynamic XML
2023-10-12 13:05 [PATCH v11 00/18] plugins: Allow to read registers Akihiko Odaki
` (4 preceding siblings ...)
2023-10-12 13:05 ` [PATCH v11 05/18] target/ppc: " Akihiko Odaki
@ 2023-10-12 13:06 ` Akihiko Odaki
2023-10-12 13:06 ` [PATCH v11 07/18] gdbstub: Use GDBFeature for gdb_register_coprocessor Akihiko Odaki
` (11 subsequent siblings)
17 siblings, 0 replies; 24+ messages in thread
From: Akihiko Odaki @ 2023-10-12 13:06 UTC (permalink / raw)
Cc: Alex Bennée, Mikhail Tyutin, Aleksandr Anenkov, qemu-devel,
Philippe Mathieu-Daudé, Akihiko Odaki, Palmer Dabbelt,
Alistair Francis, Bin Meng, Weiwei Li, Daniel Henrique Barboza,
Liu Zhiwei, open list:RISC-V TCG CPUs
In preparation for a change to use GDBFeature as a parameter of
gdb_register_coprocessor(), convert the internal representation of
dynamic feature from plain XML to GDBFeature.
Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>
---
target/riscv/cpu.h | 5 +--
target/riscv/cpu.c | 4 +--
target/riscv/gdbstub.c | 77 ++++++++++++++++++------------------------
3 files changed, 38 insertions(+), 48 deletions(-)
diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
index 83f48f9e7e..42f9bd0de4 100644
--- a/target/riscv/cpu.h
+++ b/target/riscv/cpu.h
@@ -23,6 +23,7 @@
#include "hw/core/cpu.h"
#include "hw/registerfields.h"
#include "exec/cpu-defs.h"
+#include "exec/gdbstub.h"
#include "qemu/cpu-float.h"
#include "qom/object.h"
#include "qemu/int128.h"
@@ -390,8 +391,8 @@ struct ArchCPU {
CPURISCVState env;
- char *dyn_csr_xml;
- char *dyn_vreg_xml;
+ GDBFeature dyn_csr_feature;
+ GDBFeature dyn_vreg_feature;
/* Configuration Settings */
RISCVCPUConfig cfg;
diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
index 02c85bf1ac..9417a5d897 100644
--- a/target/riscv/cpu.c
+++ b/target/riscv/cpu.c
@@ -1989,9 +1989,9 @@ static const char *riscv_gdb_get_dynamic_xml(CPUState *cs, const char *xmlname)
RISCVCPU *cpu = RISCV_CPU(cs);
if (strcmp(xmlname, "riscv-csr.xml") == 0) {
- return cpu->dyn_csr_xml;
+ return cpu->dyn_csr_feature.xml;
} else if (strcmp(xmlname, "riscv-vector.xml") == 0) {
- return cpu->dyn_vreg_xml;
+ return cpu->dyn_vreg_feature.xml;
}
return NULL;
diff --git a/target/riscv/gdbstub.c b/target/riscv/gdbstub.c
index b9528cef5b..b7159f1db8 100644
--- a/target/riscv/gdbstub.c
+++ b/target/riscv/gdbstub.c
@@ -214,13 +214,14 @@ static int riscv_gdb_set_virtual(CPURISCVState *cs, uint8_t *mem_buf, int n)
return 0;
}
-static int riscv_gen_dynamic_csr_xml(CPUState *cs, int base_reg)
+static GDBFeature *riscv_gen_dynamic_csr_feature(CPUState *cs)
{
RISCVCPUClass *mcc = RISCV_CPU_GET_CLASS(cs);
RISCVCPU *cpu = RISCV_CPU(cs);
CPURISCVState *env = &cpu->env;
- GString *s = g_string_new(NULL);
+ GDBFeatureBuilder builder;
riscv_csr_predicate_fn predicate;
+ const char *name;
int bitsize = 16 << mcc->misa_mxl_max;
int i;
@@ -233,9 +234,8 @@ static int riscv_gen_dynamic_csr_xml(CPUState *cs, int base_reg)
bitsize = 64;
}
- g_string_printf(s, "<?xml version=\"1.0\"?>");
- g_string_append_printf(s, "<!DOCTYPE feature SYSTEM \"gdb-target.dtd\">");
- g_string_append_printf(s, "<feature name=\"org.gnu.gdb.riscv.csr\">");
+ gdb_feature_builder_init(&builder, &cpu->dyn_csr_feature,
+ "org.gnu.gdb.riscv.csr", "riscv-csr.xml");
for (i = 0; i < CSR_TABLE_SIZE; i++) {
if (env->priv_ver < csr_ops[i].min_priv_ver) {
@@ -243,72 +243,63 @@ static int riscv_gen_dynamic_csr_xml(CPUState *cs, int base_reg)
}
predicate = csr_ops[i].predicate;
if (predicate && (predicate(env, i) == RISCV_EXCP_NONE)) {
- if (csr_ops[i].name) {
- g_string_append_printf(s, "<reg name=\"%s\"", csr_ops[i].name);
- } else {
- g_string_append_printf(s, "<reg name=\"csr%03x\"", i);
+ g_autofree char *dynamic_name = NULL;
+ name = csr_ops[i].name;
+ if (!name) {
+ dynamic_name = g_strdup_printf("csr%03x", i);
+ name = dynamic_name;
}
- g_string_append_printf(s, " bitsize=\"%d\"", bitsize);
- g_string_append_printf(s, " regnum=\"%d\"/>", base_reg + i);
+
+ gdb_feature_builder_append_reg(&builder, name, bitsize,
+ "int", NULL);
}
}
- g_string_append_printf(s, "</feature>");
-
- cpu->dyn_csr_xml = g_string_free(s, false);
+ gdb_feature_builder_end(&builder);
#if !defined(CONFIG_USER_ONLY)
env->debugger = false;
#endif
- return CSR_TABLE_SIZE;
+ return &cpu->dyn_csr_feature;
}
-static int ricsv_gen_dynamic_vector_xml(CPUState *cs, int base_reg)
+static GDBFeature *ricsv_gen_dynamic_vector_feature(CPUState *cs)
{
RISCVCPU *cpu = RISCV_CPU(cs);
- GString *s = g_string_new(NULL);
- g_autoptr(GString) ts = g_string_new("");
+ GDBFeatureBuilder builder;
int reg_width = cpu->cfg.vlen;
- int num_regs = 0;
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.riscv.vector\">");
+ gdb_feature_builder_init(&builder, &cpu->dyn_vreg_feature,
+ "org.gnu.gdb.riscv.vector", "riscv-vector.xml");
/* 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, "%s", vec_lanes[i].id);
- g_string_append_printf(s,
- "<vector id=\"%s\" type=\"%s\" count=\"%d\"/>",
- ts->str, vec_lanes[i].gdb_type, count);
+ gdb_feature_builder_append_tag(
+ &builder, "<vector id=\"%s\" type=\"%s\" count=\"%d\"/>",
+ vec_lanes[i].id, vec_lanes[i].gdb_type, count);
}
/* Define unions */
- g_string_append_printf(s, "<union id=\"riscv_vector\">");
+ gdb_feature_builder_append_tag(&builder, "<union id=\"riscv_vector\">");
for (i = 0; i < ARRAY_SIZE(vec_lanes); i++) {
- g_string_append_printf(s, "<field name=\"%c\" type=\"%s\"/>",
- vec_lanes[i].suffix,
- vec_lanes[i].id);
+ gdb_feature_builder_append_tag(&builder,
+ "<field name=\"%c\" type=\"%s\"/>",
+ vec_lanes[i].suffix, vec_lanes[i].id);
}
- g_string_append(s, "</union>");
+ gdb_feature_builder_append_tag(&builder, "</union>");
/* Define vector registers */
for (i = 0; i < 32; i++) {
- g_string_append_printf(s,
- "<reg name=\"v%d\" bitsize=\"%d\""
- " regnum=\"%d\" group=\"vector\""
- " type=\"riscv_vector\"/>",
- i, reg_width, base_reg++);
- num_regs++;
+ gdb_feature_builder_append_reg(&builder, g_strdup_printf("v%d", i),
+ reg_width, "riscv_vector", "vector");
}
- g_string_append_printf(s, "</feature>");
+ gdb_feature_builder_end(&builder);
- cpu->dyn_vreg_xml = g_string_free(s, false);
- return num_regs;
+ return &cpu->dyn_vreg_feature;
}
void riscv_cpu_register_gdb_regs_for_features(CPUState *cs)
@@ -324,10 +315,9 @@ void riscv_cpu_register_gdb_regs_for_features(CPUState *cs)
32, "riscv-32bit-fpu.xml", 0);
}
if (env->misa_ext & RVV) {
- int base_reg = cs->gdb_num_regs;
gdb_register_coprocessor(cs, riscv_gdb_get_vector,
riscv_gdb_set_vector,
- ricsv_gen_dynamic_vector_xml(cs, base_reg),
+ ricsv_gen_dynamic_vector_feature(cs)->num_regs,
"riscv-vector.xml", 0);
}
switch (mcc->misa_mxl_max) {
@@ -347,9 +337,8 @@ void riscv_cpu_register_gdb_regs_for_features(CPUState *cs)
}
if (cpu->cfg.ext_icsr) {
- int base_reg = cs->gdb_num_regs;
gdb_register_coprocessor(cs, riscv_gdb_get_csr, riscv_gdb_set_csr,
- riscv_gen_dynamic_csr_xml(cs, base_reg),
+ riscv_gen_dynamic_csr_feature(cs)->num_regs,
"riscv-csr.xml", 0);
}
}
--
2.42.0
^ permalink raw reply related [flat|nested] 24+ messages in thread
* [PATCH v11 07/18] gdbstub: Use GDBFeature for gdb_register_coprocessor
2023-10-12 13:05 [PATCH v11 00/18] plugins: Allow to read registers Akihiko Odaki
` (5 preceding siblings ...)
2023-10-12 13:06 ` [PATCH v11 06/18] target/riscv: " Akihiko Odaki
@ 2023-10-12 13:06 ` Akihiko Odaki
2023-10-12 13:06 ` [PATCH v11 08/18] gdbstub: Use GDBFeature for GDBRegisterState Akihiko Odaki
` (10 subsequent siblings)
17 siblings, 0 replies; 24+ messages in thread
From: Akihiko Odaki @ 2023-10-12 13:06 UTC (permalink / raw)
Cc: Alex Bennée, Mikhail Tyutin, Aleksandr Anenkov, qemu-devel,
Philippe Mathieu-Daudé, Akihiko Odaki, Peter Maydell,
Brian Cain, Song Gao, Xiaojuan Yang, Laurent Vivier,
Edgar E. Iglesias, Nicholas Piggin, Daniel Henrique Barboza,
Cédric Le Goater, Palmer Dabbelt, Alistair Francis, Bin Meng,
Weiwei Li, Liu Zhiwei, Thomas Huth, Richard Henderson,
David Hildenbrand, Ilya Leoshkevich, open list:ARM TCG CPUs,
open list:PowerPC TCG CPUs, open list:RISC-V TCG CPUs,
open list:S390 general arch...
This is a tree-wide change to introduce GDBFeature parameter to
gdb_register_coprocessor(). The new parameter just replaces num_regs
and xml parameters for now. GDBFeature will be utilized to simplify XML
lookup in a following change.
Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>
Acked-by: Alex Bennée <alex.bennee@linaro.org>
---
include/exec/gdbstub.h | 2 +-
gdbstub/gdbstub.c | 13 +++++++------
target/arm/gdbstub.c | 34 ++++++++++++++++++----------------
target/hexagon/cpu.c | 3 +--
target/loongarch/gdbstub.c | 2 +-
target/m68k/helper.c | 6 +++---
target/microblaze/cpu.c | 5 +++--
target/ppc/gdbstub.c | 11 ++++++-----
target/riscv/gdbstub.c | 18 ++++++++++--------
target/s390x/gdbstub.c | 28 +++++++---------------------
10 files changed, 57 insertions(+), 65 deletions(-)
diff --git a/include/exec/gdbstub.h b/include/exec/gdbstub.h
index cbd582260a..6f643c744d 100644
--- a/include/exec/gdbstub.h
+++ b/include/exec/gdbstub.h
@@ -37,7 +37,7 @@ typedef int (*gdb_set_reg_cb)(CPUArchState *env, uint8_t *buf, int reg);
*/
void gdb_register_coprocessor(CPUState *cpu,
gdb_get_reg_cb get_reg, gdb_set_reg_cb set_reg,
- int num_regs, const char *xml, int g_pos);
+ const GDBFeature *feature, int g_pos);
/**
* gdbserver_start: start the gdb server
diff --git a/gdbstub/gdbstub.c b/gdbstub/gdbstub.c
index 258dd54c90..20586e0b6a 100644
--- a/gdbstub/gdbstub.c
+++ b/gdbstub/gdbstub.c
@@ -532,7 +532,7 @@ static int gdb_write_register(CPUState *cpu, uint8_t *mem_buf, int reg)
void gdb_register_coprocessor(CPUState *cpu,
gdb_get_reg_cb get_reg, gdb_set_reg_cb set_reg,
- int num_regs, const char *xml, int g_pos)
+ const GDBFeature *feature, int g_pos)
{
GDBRegisterState *s;
guint i;
@@ -541,7 +541,7 @@ void gdb_register_coprocessor(CPUState *cpu,
for (i = 0; i < cpu->gdb_regs->len; i++) {
/* Check for duplicates. */
s = &g_array_index(cpu->gdb_regs, GDBRegisterState, i);
- if (strcmp(s->xml, xml) == 0) {
+ if (strcmp(s->xml, feature->xmlname) == 0) {
return;
}
}
@@ -553,17 +553,18 @@ void gdb_register_coprocessor(CPUState *cpu,
g_array_set_size(cpu->gdb_regs, i + 1);
s = &g_array_index(cpu->gdb_regs, GDBRegisterState, i);
s->base_reg = cpu->gdb_num_regs;
- s->num_regs = num_regs;
+ s->num_regs = feature->num_regs;
s->get_reg = get_reg;
s->set_reg = set_reg;
- s->xml = xml;
+ s->xml = feature->xml;
/* Add to end of list. */
- cpu->gdb_num_regs += num_regs;
+ cpu->gdb_num_regs += feature->num_regs;
if (g_pos) {
if (g_pos != s->base_reg) {
error_report("Error: Bad gdb register numbering for '%s', "
- "expected %d got %d", xml, g_pos, s->base_reg);
+ "expected %d got %d", feature->xml,
+ g_pos, s->base_reg);
} else {
cpu->gdb_num_g_regs = cpu->gdb_num_regs;
}
diff --git a/target/arm/gdbstub.c b/target/arm/gdbstub.c
index fc0aa6f235..99040e0c4c 100644
--- a/target/arm/gdbstub.c
+++ b/target/arm/gdbstub.c
@@ -472,14 +472,14 @@ void arm_cpu_register_gdb_regs_for_features(ARMCPU *cpu)
*/
#ifdef TARGET_AARCH64
if (isar_feature_aa64_sve(&cpu->isar)) {
- int nreg = arm_gen_dynamic_svereg_feature(cs)->num_regs;
+ GDBFeature *feature = arm_gen_dynamic_svereg_feature(cs);
gdb_register_coprocessor(cs, aarch64_gdb_get_sve_reg,
- aarch64_gdb_set_sve_reg, nreg,
- "sve-registers.xml", 0);
+ aarch64_gdb_set_sve_reg, feature, 0);
} else {
gdb_register_coprocessor(cs, aarch64_gdb_get_fpu_reg,
aarch64_gdb_set_fpu_reg,
- 34, "aarch64-fpu.xml", 0);
+ gdb_find_static_feature("aarch64-fpu.xml"),
+ 0);
}
/*
* Note that we report pauth information via the feature name
@@ -490,19 +490,22 @@ void arm_cpu_register_gdb_regs_for_features(ARMCPU *cpu)
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);
+ gdb_find_static_feature("aarch64-pauth.xml"),
+ 0);
}
#endif
} else {
if (arm_feature(env, ARM_FEATURE_NEON)) {
gdb_register_coprocessor(cs, vfp_gdb_get_reg, vfp_gdb_set_reg,
- 49, "arm-neon.xml", 0);
+ gdb_find_static_feature("arm-neon.xml"),
+ 0);
} else if (cpu_isar_feature(aa32_simd_r32, cpu)) {
gdb_register_coprocessor(cs, vfp_gdb_get_reg, vfp_gdb_set_reg,
- 33, "arm-vfp3.xml", 0);
+ gdb_find_static_feature("arm-vfp3.xml"),
+ 0);
} else if (cpu_isar_feature(aa32_vfp_simd, cpu)) {
gdb_register_coprocessor(cs, vfp_gdb_get_reg, vfp_gdb_set_reg,
- 17, "arm-vfp.xml", 0);
+ gdb_find_static_feature("arm-vfp.xml"), 0);
}
if (!arm_feature(env, ARM_FEATURE_M)) {
/*
@@ -510,29 +513,28 @@ void arm_cpu_register_gdb_regs_for_features(ARMCPU *cpu)
* expose to gdb.
*/
gdb_register_coprocessor(cs, vfp_gdb_get_sysreg, vfp_gdb_set_sysreg,
- 2, "arm-vfp-sysregs.xml", 0);
+ gdb_find_static_feature("arm-vfp-sysregs.xml"),
+ 0);
}
}
if (cpu_isar_feature(aa32_mve, cpu) && tcg_enabled()) {
gdb_register_coprocessor(cs, mve_gdb_get_reg, mve_gdb_set_reg,
- 1, "arm-m-profile-mve.xml", 0);
+ gdb_find_static_feature("arm-m-profile-mve.xml"),
+ 0);
}
gdb_register_coprocessor(cs, arm_gdb_get_sysreg, arm_gdb_set_sysreg,
- arm_gen_dynamic_sysreg_feature(cs)->num_regs,
- "system-registers.xml", 0);
+ arm_gen_dynamic_sysreg_feature(cs), 0);
#ifdef CONFIG_TCG
if (arm_feature(env, ARM_FEATURE_M) && tcg_enabled()) {
gdb_register_coprocessor(cs,
arm_gdb_get_m_systemreg, arm_gdb_set_m_systemreg,
- arm_gen_dynamic_m_systemreg_feature(cs)->num_regs,
- "arm-m-system.xml", 0);
+ arm_gen_dynamic_m_systemreg_feature(cs), 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_feature(cs)->num_regs,
- "arm-m-secext.xml", 0);
+ arm_gen_dynamic_m_secextreg_feature(cs), 0);
}
#endif
}
diff --git a/target/hexagon/cpu.c b/target/hexagon/cpu.c
index 1adc11b713..60d52e1e9d 100644
--- a/target/hexagon/cpu.c
+++ b/target/hexagon/cpu.c
@@ -342,8 +342,7 @@ static void hexagon_cpu_realize(DeviceState *dev, Error **errp)
gdb_register_coprocessor(cs, hexagon_hvx_gdb_read_register,
hexagon_hvx_gdb_write_register,
- NUM_VREGS + NUM_QREGS,
- "hexagon-hvx.xml", 0);
+ gdb_find_static_feature("hexagon-hvx.xml"), 0);
qemu_init_vcpu(cs);
cpu_reset(cs);
diff --git a/target/loongarch/gdbstub.c b/target/loongarch/gdbstub.c
index 5fc2f19e96..843a869450 100644
--- a/target/loongarch/gdbstub.c
+++ b/target/loongarch/gdbstub.c
@@ -118,5 +118,5 @@ static int loongarch_gdb_set_fpu(CPULoongArchState *env,
void loongarch_cpu_register_gdb_regs_for_features(CPUState *cs)
{
gdb_register_coprocessor(cs, loongarch_gdb_get_fpu, loongarch_gdb_set_fpu,
- 41, "loongarch-fpu.xml", 0);
+ gdb_find_static_feature("loongarch-fpu.xml"), 0);
}
diff --git a/target/m68k/helper.c b/target/m68k/helper.c
index 0a1544cd68..675f2dcd5a 100644
--- a/target/m68k/helper.c
+++ b/target/m68k/helper.c
@@ -152,10 +152,10 @@ void m68k_cpu_init_gdb(M68kCPU *cpu)
if (m68k_feature(env, M68K_FEATURE_CF_FPU)) {
gdb_register_coprocessor(cs, cf_fpu_gdb_get_reg, cf_fpu_gdb_set_reg,
- 11, "cf-fp.xml", 18);
+ gdb_find_static_feature("cf-fp.xml"), 18);
} else if (m68k_feature(env, M68K_FEATURE_FPU)) {
- gdb_register_coprocessor(cs, m68k_fpu_gdb_get_reg,
- m68k_fpu_gdb_set_reg, 11, "m68k-fp.xml", 18);
+ gdb_register_coprocessor(cs, m68k_fpu_gdb_get_reg, m68k_fpu_gdb_set_reg,
+ gdb_find_static_feature("m68k-fp.xml"), 18);
}
/* TODO: Add [E]MAC registers. */
}
diff --git a/target/microblaze/cpu.c b/target/microblaze/cpu.c
index bbb3335cad..1998f69828 100644
--- a/target/microblaze/cpu.c
+++ b/target/microblaze/cpu.c
@@ -297,8 +297,9 @@ static void mb_cpu_initfn(Object *obj)
CPUMBState *env = &cpu->env;
gdb_register_coprocessor(CPU(cpu), mb_cpu_gdb_read_stack_protect,
- mb_cpu_gdb_write_stack_protect, 2,
- "microblaze-stack-protect.xml", 0);
+ mb_cpu_gdb_write_stack_protect,
+ gdb_find_static_feature("microblaze-stack-protect.xml"),
+ 0);
set_float_rounding_mode(float_round_nearest_even, &env->fp_status);
diff --git a/target/ppc/gdbstub.c b/target/ppc/gdbstub.c
index 43de40ac36..9f2aad7aa9 100644
--- a/target/ppc/gdbstub.c
+++ b/target/ppc/gdbstub.c
@@ -567,22 +567,23 @@ void ppc_gdb_init(CPUState *cs, PowerPCCPUClass *pcc)
{
if (pcc->insns_flags & PPC_FLOAT) {
gdb_register_coprocessor(cs, gdb_get_float_reg, gdb_set_float_reg,
- 33, "power-fpu.xml", 0);
+ gdb_find_static_feature("power-fpu.xml"), 0);
}
if (pcc->insns_flags & PPC_ALTIVEC) {
gdb_register_coprocessor(cs, gdb_get_avr_reg, gdb_set_avr_reg,
- 34, "power-altivec.xml", 0);
+ gdb_find_static_feature("power-altivec.xml"),
+ 0);
}
if (pcc->insns_flags & PPC_SPE) {
gdb_register_coprocessor(cs, gdb_get_spe_reg, gdb_set_spe_reg,
- 34, "power-spe.xml", 0);
+ gdb_find_static_feature("power-spe.xml"), 0);
}
if (pcc->insns_flags2 & PPC2_VSX) {
gdb_register_coprocessor(cs, gdb_get_vsx_reg, gdb_set_vsx_reg,
- 32, "power-vsx.xml", 0);
+ gdb_find_static_feature("power-vsx.xml"), 0);
}
#ifndef CONFIG_USER_ONLY
gdb_register_coprocessor(cs, gdb_get_spr_reg, gdb_set_spr_reg,
- pcc->gdb_spr.num_regs, "power-spr.xml", 0);
+ &pcc->gdb_spr, 0);
#endif
}
diff --git a/target/riscv/gdbstub.c b/target/riscv/gdbstub.c
index b7159f1db8..e33906635c 100644
--- a/target/riscv/gdbstub.c
+++ b/target/riscv/gdbstub.c
@@ -309,28 +309,31 @@ void riscv_cpu_register_gdb_regs_for_features(CPUState *cs)
CPURISCVState *env = &cpu->env;
if (env->misa_ext & RVD) {
gdb_register_coprocessor(cs, riscv_gdb_get_fpu, riscv_gdb_set_fpu,
- 32, "riscv-64bit-fpu.xml", 0);
+ gdb_find_static_feature("riscv-64bit-fpu.xml"),
+ 0);
} else if (env->misa_ext & RVF) {
gdb_register_coprocessor(cs, riscv_gdb_get_fpu, riscv_gdb_set_fpu,
- 32, "riscv-32bit-fpu.xml", 0);
+ gdb_find_static_feature("riscv-32bit-fpu.xml"),
+ 0);
}
if (env->misa_ext & RVV) {
gdb_register_coprocessor(cs, riscv_gdb_get_vector,
riscv_gdb_set_vector,
- ricsv_gen_dynamic_vector_feature(cs)->num_regs,
- "riscv-vector.xml", 0);
+ ricsv_gen_dynamic_vector_feature(cs), 0);
}
switch (mcc->misa_mxl_max) {
case MXL_RV32:
gdb_register_coprocessor(cs, riscv_gdb_get_virtual,
riscv_gdb_set_virtual,
- 1, "riscv-32bit-virtual.xml", 0);
+ gdb_find_static_feature("riscv-32bit-virtual.xml"),
+ 0);
break;
case MXL_RV64:
case MXL_RV128:
gdb_register_coprocessor(cs, riscv_gdb_get_virtual,
riscv_gdb_set_virtual,
- 1, "riscv-64bit-virtual.xml", 0);
+ gdb_find_static_feature("riscv-64bit-virtual.xml"),
+ 0);
break;
default:
g_assert_not_reached();
@@ -338,7 +341,6 @@ void riscv_cpu_register_gdb_regs_for_features(CPUState *cs)
if (cpu->cfg.ext_icsr) {
gdb_register_coprocessor(cs, riscv_gdb_get_csr, riscv_gdb_set_csr,
- riscv_gen_dynamic_csr_feature(cs)->num_regs,
- "riscv-csr.xml", 0);
+ riscv_gen_dynamic_csr_feature(cs), 0);
}
}
diff --git a/target/s390x/gdbstub.c b/target/s390x/gdbstub.c
index 6fbfd41bc8..02c388dc32 100644
--- a/target/s390x/gdbstub.c
+++ b/target/s390x/gdbstub.c
@@ -69,8 +69,6 @@ int s390_cpu_gdb_write_register(CPUState *cs, uint8_t *mem_buf, int n)
/* the values represent the positions in s390-acr.xml */
#define S390_A0_REGNUM 0
#define S390_A15_REGNUM 15
-/* total number of registers in s390-acr.xml */
-#define S390_NUM_AC_REGS 16
static int cpu_read_ac_reg(CPUS390XState *env, GByteArray *buf, int n)
{
@@ -98,8 +96,6 @@ static int cpu_write_ac_reg(CPUS390XState *env, uint8_t *mem_buf, int n)
#define S390_FPC_REGNUM 0
#define S390_F0_REGNUM 1
#define S390_F15_REGNUM 16
-/* total number of registers in s390-fpr.xml */
-#define S390_NUM_FP_REGS 17
static int cpu_read_fp_reg(CPUS390XState *env, GByteArray *buf, int n)
{
@@ -132,8 +128,6 @@ static int cpu_write_fp_reg(CPUS390XState *env, uint8_t *mem_buf, int n)
#define S390_V15L_REGNUM 15
#define S390_V16_REGNUM 16
#define S390_V31_REGNUM 31
-/* total number of registers in s390-vx.xml */
-#define S390_NUM_VREGS 32
static int cpu_read_vreg(CPUS390XState *env, GByteArray *buf, int n)
{
@@ -172,8 +166,6 @@ static int cpu_write_vreg(CPUS390XState *env, uint8_t *mem_buf, int n)
/* the values represent the positions in s390-cr.xml */
#define S390_C0_REGNUM 0
#define S390_C15_REGNUM 15
-/* total number of registers in s390-cr.xml */
-#define S390_NUM_C_REGS 16
#ifndef CONFIG_USER_ONLY
static int cpu_read_c_reg(CPUS390XState *env, GByteArray *buf, int n)
@@ -206,8 +198,6 @@ static int cpu_write_c_reg(CPUS390XState *env, uint8_t *mem_buf, int n)
#define S390_VIRT_CPUTM_REGNUM 1
#define S390_VIRT_BEA_REGNUM 2
#define S390_VIRT_PREFIX_REGNUM 3
-/* total number of registers in s390-virt.xml */
-#define S390_NUM_VIRT_REGS 4
static int cpu_read_virt_reg(CPUS390XState *env, GByteArray *mem_buf, int n)
{
@@ -254,8 +244,6 @@ static int cpu_write_virt_reg(CPUS390XState *env, uint8_t *mem_buf, int n)
#define S390_VIRT_KVM_PFT_REGNUM 1
#define S390_VIRT_KVM_PFS_REGNUM 2
#define S390_VIRT_KVM_PFC_REGNUM 3
-/* total number of registers in s390-virt-kvm.xml */
-#define S390_NUM_VIRT_KVM_REGS 4
static int cpu_read_virt_kvm_reg(CPUS390XState *env, GByteArray *mem_buf, int n)
{
@@ -303,8 +291,6 @@ static int cpu_write_virt_kvm_reg(CPUS390XState *env, uint8_t *mem_buf, int n)
#define S390_GS_GSD_REGNUM 1
#define S390_GS_GSSM_REGNUM 2
#define S390_GS_GSEPLA_REGNUM 3
-/* total number of registers in s390-gs.xml */
-#define S390_NUM_GS_REGS 4
static int cpu_read_gs_reg(CPUS390XState *env, GByteArray *buf, int n)
{
@@ -322,33 +308,33 @@ void s390_cpu_gdb_init(CPUState *cs)
{
gdb_register_coprocessor(cs, cpu_read_ac_reg,
cpu_write_ac_reg,
- S390_NUM_AC_REGS, "s390-acr.xml", 0);
+ gdb_find_static_feature("s390-acr.xml"), 0);
gdb_register_coprocessor(cs, cpu_read_fp_reg,
cpu_write_fp_reg,
- S390_NUM_FP_REGS, "s390-fpr.xml", 0);
+ gdb_find_static_feature("s390-fpr.xml"), 0);
gdb_register_coprocessor(cs, cpu_read_vreg,
cpu_write_vreg,
- S390_NUM_VREGS, "s390-vx.xml", 0);
+ gdb_find_static_feature("s390-vx.xml"), 0);
gdb_register_coprocessor(cs, cpu_read_gs_reg,
cpu_write_gs_reg,
- S390_NUM_GS_REGS, "s390-gs.xml", 0);
+ gdb_find_static_feature("s390-gs.xml"), 0);
#ifndef CONFIG_USER_ONLY
gdb_register_coprocessor(cs, cpu_read_c_reg,
cpu_write_c_reg,
- S390_NUM_C_REGS, "s390-cr.xml", 0);
+ gdb_find_static_feature("s390-cr.xml"), 0);
gdb_register_coprocessor(cs, cpu_read_virt_reg,
cpu_write_virt_reg,
- S390_NUM_VIRT_REGS, "s390-virt.xml", 0);
+ gdb_find_static_feature("s390-virt.xml"), 0);
if (kvm_enabled()) {
gdb_register_coprocessor(cs, cpu_read_virt_kvm_reg,
cpu_write_virt_kvm_reg,
- S390_NUM_VIRT_KVM_REGS, "s390-virt-kvm.xml",
+ gdb_find_static_feature("s390-virt-kvm.xml"),
0);
}
#endif
--
2.42.0
^ permalink raw reply related [flat|nested] 24+ messages in thread
* [PATCH v11 08/18] gdbstub: Use GDBFeature for GDBRegisterState
2023-10-12 13:05 [PATCH v11 00/18] plugins: Allow to read registers Akihiko Odaki
` (6 preceding siblings ...)
2023-10-12 13:06 ` [PATCH v11 07/18] gdbstub: Use GDBFeature for gdb_register_coprocessor Akihiko Odaki
@ 2023-10-12 13:06 ` Akihiko Odaki
2023-10-12 13:06 ` [PATCH v11 09/18] gdbstub: Change gdb_get_reg_cb and gdb_set_reg_cb Akihiko Odaki
` (9 subsequent siblings)
17 siblings, 0 replies; 24+ messages in thread
From: Akihiko Odaki @ 2023-10-12 13:06 UTC (permalink / raw)
Cc: Alex Bennée, Mikhail Tyutin, Aleksandr Anenkov, qemu-devel,
Philippe Mathieu-Daudé, Akihiko Odaki
Simplify GDBRegisterState by replacing num_regs and xml members with
one member that points to GDBFeature.
Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>
Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
---
gdbstub/gdbstub.c | 14 ++++++--------
1 file changed, 6 insertions(+), 8 deletions(-)
diff --git a/gdbstub/gdbstub.c b/gdbstub/gdbstub.c
index 20586e0b6a..9a4124afc4 100644
--- a/gdbstub/gdbstub.c
+++ b/gdbstub/gdbstub.c
@@ -47,10 +47,9 @@
typedef struct GDBRegisterState {
int base_reg;
- int num_regs;
gdb_get_reg_cb get_reg;
gdb_set_reg_cb set_reg;
- const char *xml;
+ const GDBFeature *feature;
} GDBRegisterState;
GDBState gdbserver_state;
@@ -390,7 +389,7 @@ static const char *get_feature_xml(const char *p, const char **newp,
g_ptr_array_add(
xml,
g_markup_printf_escaped("<xi:include href=\"%s\"/>",
- r->xml));
+ r->feature->xmlname));
}
g_ptr_array_add(xml, g_strdup("</target>"));
g_ptr_array_add(xml, NULL);
@@ -504,7 +503,7 @@ static int gdb_read_register(CPUState *cpu, GByteArray *buf, int reg)
for (guint i = 0; i < cpu->gdb_regs->len; i++) {
r = &g_array_index(cpu->gdb_regs, GDBRegisterState, i);
- if (r->base_reg <= reg && reg < r->base_reg + r->num_regs) {
+ if (r->base_reg <= reg && reg < r->base_reg + r->feature->num_regs) {
return r->get_reg(env, buf, reg - r->base_reg);
}
}
@@ -523,7 +522,7 @@ static int gdb_write_register(CPUState *cpu, uint8_t *mem_buf, int reg)
for (guint i = 0; i < cpu->gdb_regs->len; i++) {
r = &g_array_index(cpu->gdb_regs, GDBRegisterState, i);
- if (r->base_reg <= reg && reg < r->base_reg + r->num_regs) {
+ if (r->base_reg <= reg && reg < r->base_reg + r->feature->num_regs) {
return r->set_reg(env, mem_buf, reg - r->base_reg);
}
}
@@ -541,7 +540,7 @@ void gdb_register_coprocessor(CPUState *cpu,
for (i = 0; i < cpu->gdb_regs->len; i++) {
/* Check for duplicates. */
s = &g_array_index(cpu->gdb_regs, GDBRegisterState, i);
- if (strcmp(s->xml, feature->xmlname) == 0) {
+ if (s->feature == feature) {
return;
}
}
@@ -553,10 +552,9 @@ void gdb_register_coprocessor(CPUState *cpu,
g_array_set_size(cpu->gdb_regs, i + 1);
s = &g_array_index(cpu->gdb_regs, GDBRegisterState, i);
s->base_reg = cpu->gdb_num_regs;
- s->num_regs = feature->num_regs;
s->get_reg = get_reg;
s->set_reg = set_reg;
- s->xml = feature->xml;
+ s->feature = feature;
/* Add to end of list. */
cpu->gdb_num_regs += feature->num_regs;
--
2.42.0
^ permalink raw reply related [flat|nested] 24+ messages in thread
* [PATCH v11 09/18] gdbstub: Change gdb_get_reg_cb and gdb_set_reg_cb
2023-10-12 13:05 [PATCH v11 00/18] plugins: Allow to read registers Akihiko Odaki
` (7 preceding siblings ...)
2023-10-12 13:06 ` [PATCH v11 08/18] gdbstub: Use GDBFeature for GDBRegisterState Akihiko Odaki
@ 2023-10-12 13:06 ` Akihiko Odaki
2023-10-12 13:40 ` Philippe Mathieu-Daudé
2023-10-12 13:06 ` [PATCH v11 10/18] gdbstub: Simplify XML lookup Akihiko Odaki
` (8 subsequent siblings)
17 siblings, 1 reply; 24+ messages in thread
From: Akihiko Odaki @ 2023-10-12 13:06 UTC (permalink / raw)
Cc: Alex Bennée, Mikhail Tyutin, Aleksandr Anenkov, qemu-devel,
Philippe Mathieu-Daudé, Akihiko Odaki, Peter Maydell,
Brian Cain, Song Gao, Xiaojuan Yang, Laurent Vivier,
Edgar E. Iglesias, Nicholas Piggin, Daniel Henrique Barboza,
Cédric Le Goater, Palmer Dabbelt, Alistair Francis, Bin Meng,
Weiwei Li, Liu Zhiwei, Thomas Huth, Richard Henderson,
David Hildenbrand, Ilya Leoshkevich, open list:ARM TCG CPUs,
open list:PowerPC TCG CPUs, open list:RISC-V TCG CPUs,
open list:S390 general arch...
Align the parameters of gdb_get_reg_cb and gdb_set_reg_cb with the
gdb_read_register and gdb_write_register members of CPUClass to allow
to unify the logic to access registers of the core and coprocessors
in the future.
Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>
---
include/exec/gdbstub.h | 4 +-
target/arm/internals.h | 12 +++---
target/hexagon/internal.h | 4 +-
target/microblaze/cpu.h | 4 +-
gdbstub/gdbstub.c | 6 +--
target/arm/gdbstub.c | 51 ++++++++++++++++--------
target/arm/gdbstub64.c | 27 +++++++++----
target/hexagon/gdbstub.c | 10 ++++-
target/loongarch/gdbstub.c | 11 ++++--
target/m68k/helper.c | 20 ++++++++--
target/microblaze/gdbstub.c | 9 ++++-
target/ppc/gdbstub.c | 46 +++++++++++++++++-----
target/riscv/gdbstub.c | 46 ++++++++++++++++------
target/s390x/gdbstub.c | 77 ++++++++++++++++++++++++++++---------
14 files changed, 236 insertions(+), 91 deletions(-)
diff --git a/include/exec/gdbstub.h b/include/exec/gdbstub.h
index 6f643c744d..c34ecfc37a 100644
--- a/include/exec/gdbstub.h
+++ b/include/exec/gdbstub.h
@@ -23,8 +23,8 @@ typedef struct GDBFeatureBuilder {
/* Get or set a register. Returns the size of the register. */
-typedef int (*gdb_get_reg_cb)(CPUArchState *env, GByteArray *buf, int reg);
-typedef int (*gdb_set_reg_cb)(CPUArchState *env, uint8_t *buf, int reg);
+typedef int (*gdb_get_reg_cb)(CPUState *cpu, GByteArray *buf, int reg);
+typedef int (*gdb_set_reg_cb)(CPUState *cpu, uint8_t *buf, int reg);
/**
* gdb_register_coprocessor() - register a supplemental set of registers
diff --git a/target/arm/internals.h b/target/arm/internals.h
index 09350e96da..f7a48bad5f 100644
--- a/target/arm/internals.h
+++ b/target/arm/internals.h
@@ -1441,12 +1441,12 @@ static inline uint64_t pmu_counter_mask(CPUARMState *env)
#ifdef TARGET_AARCH64
GDBFeature *arm_gen_dynamic_svereg_feature(CPUState *cpu);
-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);
+int aarch64_gdb_get_sve_reg(CPUState *cs, GByteArray *buf, int reg);
+int aarch64_gdb_set_sve_reg(CPUState *cs, uint8_t *buf, int reg);
+int aarch64_gdb_get_fpu_reg(CPUState *cs, GByteArray *buf, int reg);
+int aarch64_gdb_set_fpu_reg(CPUState *cs, uint8_t *buf, int reg);
+int aarch64_gdb_get_pauth_reg(CPUState *cs, GByteArray *buf, int reg);
+int aarch64_gdb_set_pauth_reg(CPUState *cs, uint8_t *buf, int reg);
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/hexagon/internal.h b/target/hexagon/internal.h
index d732b6bb3c..beb08cb7e3 100644
--- a/target/hexagon/internal.h
+++ b/target/hexagon/internal.h
@@ -33,8 +33,8 @@
int hexagon_gdb_read_register(CPUState *cpu, GByteArray *buf, int reg);
int hexagon_gdb_write_register(CPUState *cpu, uint8_t *buf, int reg);
-int hexagon_hvx_gdb_read_register(CPUHexagonState *env, GByteArray *mem_buf, int n);
-int hexagon_hvx_gdb_write_register(CPUHexagonState *env, uint8_t *mem_buf, int n);
+int hexagon_hvx_gdb_read_register(CPUState *env, GByteArray *mem_buf, int n);
+int hexagon_hvx_gdb_write_register(CPUState *env, uint8_t *mem_buf, int n);
void hexagon_debug_vreg(CPUHexagonState *env, int regnum);
void hexagon_debug_qreg(CPUHexagonState *env, int regnum);
diff --git a/target/microblaze/cpu.h b/target/microblaze/cpu.h
index e43c49d4af..c14b3357b7 100644
--- a/target/microblaze/cpu.h
+++ b/target/microblaze/cpu.h
@@ -370,8 +370,8 @@ G_NORETURN void mb_cpu_do_unaligned_access(CPUState *cs, vaddr vaddr,
void mb_cpu_dump_state(CPUState *cpu, FILE *f, int flags);
int mb_cpu_gdb_read_register(CPUState *cpu, GByteArray *buf, int reg);
int mb_cpu_gdb_write_register(CPUState *cpu, uint8_t *buf, int reg);
-int mb_cpu_gdb_read_stack_protect(CPUArchState *cpu, GByteArray *buf, int reg);
-int mb_cpu_gdb_write_stack_protect(CPUArchState *cpu, uint8_t *buf, int reg);
+int mb_cpu_gdb_read_stack_protect(CPUState *cs, GByteArray *buf, int reg);
+int mb_cpu_gdb_write_stack_protect(CPUState *cs, uint8_t *buf, int reg);
static inline uint32_t mb_cpu_read_msr(const CPUMBState *env)
{
diff --git a/gdbstub/gdbstub.c b/gdbstub/gdbstub.c
index 9a4124afc4..63c049745a 100644
--- a/gdbstub/gdbstub.c
+++ b/gdbstub/gdbstub.c
@@ -494,7 +494,6 @@ const GDBFeature *gdb_find_static_feature(const char *xmlname)
static int gdb_read_register(CPUState *cpu, GByteArray *buf, int reg)
{
CPUClass *cc = CPU_GET_CLASS(cpu);
- CPUArchState *env = cpu_env(cpu);
GDBRegisterState *r;
if (reg < cc->gdb_num_core_regs) {
@@ -504,7 +503,7 @@ static int gdb_read_register(CPUState *cpu, GByteArray *buf, int reg)
for (guint i = 0; i < cpu->gdb_regs->len; i++) {
r = &g_array_index(cpu->gdb_regs, GDBRegisterState, i);
if (r->base_reg <= reg && reg < r->base_reg + r->feature->num_regs) {
- return r->get_reg(env, buf, reg - r->base_reg);
+ return r->get_reg(cpu, buf, reg - r->base_reg);
}
}
return 0;
@@ -513,7 +512,6 @@ static int gdb_read_register(CPUState *cpu, GByteArray *buf, int reg)
static int gdb_write_register(CPUState *cpu, uint8_t *mem_buf, int reg)
{
CPUClass *cc = CPU_GET_CLASS(cpu);
- CPUArchState *env = cpu_env(cpu);
GDBRegisterState *r;
if (reg < cc->gdb_num_core_regs) {
@@ -523,7 +521,7 @@ static int gdb_write_register(CPUState *cpu, uint8_t *mem_buf, int reg)
for (guint i = 0; i < cpu->gdb_regs->len; i++) {
r = &g_array_index(cpu->gdb_regs, GDBRegisterState, i);
if (r->base_reg <= reg && reg < r->base_reg + r->feature->num_regs) {
- return r->set_reg(env, mem_buf, reg - r->base_reg);
+ return r->set_reg(cpu, mem_buf, reg - r->base_reg);
}
}
return 0;
diff --git a/target/arm/gdbstub.c b/target/arm/gdbstub.c
index 99040e0c4c..aa2c6c1bde 100644
--- a/target/arm/gdbstub.c
+++ b/target/arm/gdbstub.c
@@ -104,9 +104,10 @@ int arm_cpu_gdb_write_register(CPUState *cs, uint8_t *mem_buf, int n)
return 0;
}
-static int vfp_gdb_get_reg(CPUARMState *env, GByteArray *buf, int reg)
+static int vfp_gdb_get_reg(CPUState *cs, GByteArray *buf, int reg)
{
- ARMCPU *cpu = env_archcpu(env);
+ ARMCPU *cpu = ARM_CPU(cs);
+ CPUARMState *env = &cpu->env;
int nregs = cpu_isar_feature(aa32_simd_r32, cpu) ? 32 : 16;
/* VFP data registers are always little-endian. */
@@ -128,9 +129,10 @@ static int vfp_gdb_get_reg(CPUARMState *env, GByteArray *buf, int reg)
return 0;
}
-static int vfp_gdb_set_reg(CPUARMState *env, uint8_t *buf, int reg)
+static int vfp_gdb_set_reg(CPUState *cs, uint8_t *buf, int reg)
{
- ARMCPU *cpu = env_archcpu(env);
+ ARMCPU *cpu = ARM_CPU(cs);
+ CPUARMState *env = &cpu->env;
int nregs = cpu_isar_feature(aa32_simd_r32, cpu) ? 32 : 16;
if (reg < nregs) {
@@ -154,8 +156,11 @@ static int vfp_gdb_set_reg(CPUARMState *env, uint8_t *buf, int reg)
return 0;
}
-static int vfp_gdb_get_sysreg(CPUARMState *env, GByteArray *buf, int reg)
+static int vfp_gdb_get_sysreg(CPUState *cs, GByteArray *buf, int reg)
{
+ ARMCPU *cpu = ARM_CPU(cs);
+ CPUARMState *env = &cpu->env;
+
switch (reg) {
case 0:
return gdb_get_reg32(buf, env->vfp.xregs[ARM_VFP_FPSID]);
@@ -165,8 +170,11 @@ static int vfp_gdb_get_sysreg(CPUARMState *env, GByteArray *buf, int reg)
return 0;
}
-static int vfp_gdb_set_sysreg(CPUARMState *env, uint8_t *buf, int reg)
+static int vfp_gdb_set_sysreg(CPUState *cs, uint8_t *buf, int reg)
{
+ ARMCPU *cpu = ARM_CPU(cs);
+ CPUARMState *env = &cpu->env;
+
switch (reg) {
case 0:
env->vfp.xregs[ARM_VFP_FPSID] = ldl_p(buf);
@@ -178,8 +186,11 @@ static int vfp_gdb_set_sysreg(CPUARMState *env, uint8_t *buf, int reg)
return 0;
}
-static int mve_gdb_get_reg(CPUARMState *env, GByteArray *buf, int reg)
+static int mve_gdb_get_reg(CPUState *cs, GByteArray *buf, int reg)
{
+ ARMCPU *cpu = ARM_CPU(cs);
+ CPUARMState *env = &cpu->env;
+
switch (reg) {
case 0:
return gdb_get_reg32(buf, env->v7m.vpr);
@@ -188,8 +199,11 @@ static int mve_gdb_get_reg(CPUARMState *env, GByteArray *buf, int reg)
}
}
-static int mve_gdb_set_reg(CPUARMState *env, uint8_t *buf, int reg)
+static int mve_gdb_set_reg(CPUState *cs, uint8_t *buf, int reg)
{
+ ARMCPU *cpu = ARM_CPU(cs);
+ CPUARMState *env = &cpu->env;
+
switch (reg) {
case 0:
env->v7m.vpr = ldl_p(buf);
@@ -208,9 +222,10 @@ static int mve_gdb_set_reg(CPUARMState *env, uint8_t *buf, int reg)
* We return the number of bytes copied
*/
-static int arm_gdb_get_sysreg(CPUARMState *env, GByteArray *buf, int reg)
+static int arm_gdb_get_sysreg(CPUState *cs, GByteArray *buf, int reg)
{
- ARMCPU *cpu = env_archcpu(env);
+ ARMCPU *cpu = ARM_CPU(cs);
+ CPUARMState *env = &cpu->env;
const ARMCPRegInfo *ri;
uint32_t key;
@@ -226,7 +241,7 @@ static int arm_gdb_get_sysreg(CPUARMState *env, GByteArray *buf, int reg)
return 0;
}
-static int arm_gdb_set_sysreg(CPUARMState *env, uint8_t *buf, int reg)
+static int arm_gdb_set_sysreg(CPUState *cs, uint8_t *buf, int reg)
{
return 0;
}
@@ -364,8 +379,11 @@ static int m_sysreg_get(CPUARMState *env, GByteArray *buf,
return gdb_get_reg32(buf, *ptr);
}
-static int arm_gdb_get_m_systemreg(CPUARMState *env, GByteArray *buf, int reg)
+static int arm_gdb_get_m_systemreg(CPUState *cs, GByteArray *buf, int reg)
{
+ ARMCPU *cpu = ARM_CPU(cs);
+ CPUARMState *env = &cpu->env;
+
/*
* Here, we emulate MRS instruction, where CONTROL has a mix of
* banked and non-banked bits.
@@ -376,7 +394,7 @@ static int arm_gdb_get_m_systemreg(CPUARMState *env, GByteArray *buf, int reg)
return m_sysreg_get(env, buf, reg, env->v7m.secure);
}
-static int arm_gdb_set_m_systemreg(CPUARMState *env, uint8_t *buf, int reg)
+static int arm_gdb_set_m_systemreg(CPUState *cs, uint8_t *buf, int reg)
{
return 0; /* TODO */
}
@@ -408,12 +426,15 @@ static GDBFeature *arm_gen_dynamic_m_systemreg_feature(CPUState *cs)
* 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)
+static int arm_gdb_get_m_secextreg(CPUState *cs, GByteArray *buf, int reg)
{
+ ARMCPU *cpu = ARM_CPU(cs);
+ CPUARMState *env = &cpu->env;
+
return m_sysreg_get(env, buf, reg >> 1, reg & 1);
}
-static int arm_gdb_set_m_secextreg(CPUARMState *env, uint8_t *buf, int reg)
+static int arm_gdb_set_m_secextreg(CPUState *cs, uint8_t *buf, int reg)
{
return 0; /* TODO */
}
diff --git a/target/arm/gdbstub64.c b/target/arm/gdbstub64.c
index 632ac2a520..b7978bdf02 100644
--- a/target/arm/gdbstub64.c
+++ b/target/arm/gdbstub64.c
@@ -72,8 +72,11 @@ int aarch64_cpu_gdb_write_register(CPUState *cs, uint8_t *mem_buf, int n)
return 0;
}
-int aarch64_gdb_get_fpu_reg(CPUARMState *env, GByteArray *buf, int reg)
+int aarch64_gdb_get_fpu_reg(CPUState *cs, GByteArray *buf, int reg)
{
+ ARMCPU *cpu = ARM_CPU(cs);
+ CPUARMState *env = &cpu->env;
+
switch (reg) {
case 0 ... 31:
{
@@ -92,8 +95,11 @@ 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_set_fpu_reg(CPUState *cs, uint8_t *buf, int reg)
{
+ ARMCPU *cpu = ARM_CPU(cs);
+ CPUARMState *env = &cpu->env;
+
switch (reg) {
case 0 ... 31:
/* 128 bit FP register */
@@ -116,9 +122,10 @@ int aarch64_gdb_set_fpu_reg(CPUARMState *env, uint8_t *buf, int reg)
}
}
-int aarch64_gdb_get_sve_reg(CPUARMState *env, GByteArray *buf, int reg)
+int aarch64_gdb_get_sve_reg(CPUState *cs, GByteArray *buf, int reg)
{
- ARMCPU *cpu = env_archcpu(env);
+ ARMCPU *cpu = ARM_CPU(cs);
+ CPUARMState *env = &cpu->env;
switch (reg) {
/* The first 32 registers are the zregs */
@@ -164,9 +171,10 @@ int aarch64_gdb_get_sve_reg(CPUARMState *env, GByteArray *buf, int reg)
return 0;
}
-int aarch64_gdb_set_sve_reg(CPUARMState *env, uint8_t *buf, int reg)
+int aarch64_gdb_set_sve_reg(CPUState *cs, uint8_t *buf, int reg)
{
- ARMCPU *cpu = env_archcpu(env);
+ ARMCPU *cpu = ARM_CPU(cs);
+ CPUARMState *env = &cpu->env;
/* The first 32 registers are the zregs */
switch (reg) {
@@ -210,8 +218,11 @@ 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)
+int aarch64_gdb_get_pauth_reg(CPUState *cs, GByteArray *buf, int reg)
{
+ ARMCPU *cpu = ARM_CPU(cs);
+ CPUARMState *env = &cpu->env;
+
switch (reg) {
case 0: /* pauth_dmask */
case 1: /* pauth_cmask */
@@ -241,7 +252,7 @@ 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)
+int aarch64_gdb_set_pauth_reg(CPUState *cs, uint8_t *buf, int reg)
{
/* All pseudo registers are read-only. */
return 0;
diff --git a/target/hexagon/gdbstub.c b/target/hexagon/gdbstub.c
index 54d37e006e..6007e6462b 100644
--- a/target/hexagon/gdbstub.c
+++ b/target/hexagon/gdbstub.c
@@ -81,8 +81,11 @@ static int gdb_get_qreg(CPUHexagonState *env, GByteArray *mem_buf, int n)
return total;
}
-int hexagon_hvx_gdb_read_register(CPUHexagonState *env, GByteArray *mem_buf, int n)
+int hexagon_hvx_gdb_read_register(CPUState *cs, GByteArray *mem_buf, int n)
{
+ HexagonCPU *cpu = HEXAGON_CPU(cs);
+ CPUHexagonState *env = &cpu->env;
+
if (n < NUM_VREGS) {
return gdb_get_vreg(env, mem_buf, n);
}
@@ -115,8 +118,11 @@ static int gdb_put_qreg(CPUHexagonState *env, uint8_t *mem_buf, int n)
return MAX_VEC_SIZE_BYTES / 8;
}
-int hexagon_hvx_gdb_write_register(CPUHexagonState *env, uint8_t *mem_buf, int n)
+int hexagon_hvx_gdb_write_register(CPUState *cs, uint8_t *mem_buf, int n)
{
+ HexagonCPU *cpu = HEXAGON_CPU(cs);
+ CPUHexagonState *env = &cpu->env;
+
if (n < NUM_VREGS) {
return gdb_put_vreg(env, mem_buf, n);
}
diff --git a/target/loongarch/gdbstub.c b/target/loongarch/gdbstub.c
index 843a869450..22c6889011 100644
--- a/target/loongarch/gdbstub.c
+++ b/target/loongarch/gdbstub.c
@@ -84,9 +84,11 @@ int loongarch_cpu_gdb_write_register(CPUState *cs, uint8_t *mem_buf, int n)
return length;
}
-static int loongarch_gdb_get_fpu(CPULoongArchState *env,
- GByteArray *mem_buf, int n)
+static int loongarch_gdb_get_fpu(CPUState *cs, GByteArray *mem_buf, int n)
{
+ LoongArchCPU *cpu = LOONGARCH_CPU(cs);
+ CPULoongArchState *env = &cpu->env;
+
if (0 <= n && n < 32) {
return gdb_get_reg64(mem_buf, env->fpr[n].vreg.D(0));
} else if (32 <= n && n < 40) {
@@ -97,9 +99,10 @@ static int loongarch_gdb_get_fpu(CPULoongArchState *env,
return 0;
}
-static int loongarch_gdb_set_fpu(CPULoongArchState *env,
- uint8_t *mem_buf, int n)
+static int loongarch_gdb_set_fpu(CPUState *cs, uint8_t *mem_buf, int n)
{
+ LoongArchCPU *cpu = LOONGARCH_CPU(cs);
+ CPULoongArchState *env = &cpu->env;
int length = 0;
if (0 <= n && n < 32) {
diff --git a/target/m68k/helper.c b/target/m68k/helper.c
index 675f2dcd5a..a5ee4d87e3 100644
--- a/target/m68k/helper.c
+++ b/target/m68k/helper.c
@@ -69,8 +69,11 @@ void m68k_cpu_list(void)
g_slist_free(list);
}
-static int cf_fpu_gdb_get_reg(CPUM68KState *env, GByteArray *mem_buf, int n)
+static int cf_fpu_gdb_get_reg(CPUState *cs, GByteArray *mem_buf, int n)
{
+ M68kCPU *cpu = M68K_CPU(cs);
+ CPUM68KState *env = &cpu->env;
+
if (n < 8) {
float_status s;
return gdb_get_reg64(mem_buf, floatx80_to_float64(env->fregs[n].d, &s));
@@ -86,8 +89,11 @@ static int cf_fpu_gdb_get_reg(CPUM68KState *env, GByteArray *mem_buf, int n)
return 0;
}
-static int cf_fpu_gdb_set_reg(CPUM68KState *env, uint8_t *mem_buf, int n)
+static int cf_fpu_gdb_set_reg(CPUState *cs, uint8_t *mem_buf, int n)
{
+ M68kCPU *cpu = M68K_CPU(cs);
+ CPUM68KState *env = &cpu->env;
+
if (n < 8) {
float_status s;
env->fregs[n].d = float64_to_floatx80(ldq_p(mem_buf), &s);
@@ -106,8 +112,11 @@ static int cf_fpu_gdb_set_reg(CPUM68KState *env, uint8_t *mem_buf, int n)
return 0;
}
-static int m68k_fpu_gdb_get_reg(CPUM68KState *env, GByteArray *mem_buf, int n)
+static int m68k_fpu_gdb_get_reg(CPUState *cs, GByteArray *mem_buf, int n)
{
+ M68kCPU *cpu = M68K_CPU(cs);
+ CPUM68KState *env = &cpu->env;
+
if (n < 8) {
int len = gdb_get_reg16(mem_buf, env->fregs[n].l.upper);
len += gdb_get_reg16(mem_buf, 0);
@@ -125,8 +134,11 @@ static int m68k_fpu_gdb_get_reg(CPUM68KState *env, GByteArray *mem_buf, int n)
return 0;
}
-static int m68k_fpu_gdb_set_reg(CPUM68KState *env, uint8_t *mem_buf, int n)
+static int m68k_fpu_gdb_set_reg(CPUState *cs, uint8_t *mem_buf, int n)
{
+ M68kCPU *cpu = M68K_CPU(cs);
+ CPUM68KState *env = &cpu->env;
+
if (n < 8) {
env->fregs[n].l.upper = lduw_be_p(mem_buf);
env->fregs[n].l.lower = ldq_be_p(mem_buf + 4);
diff --git a/target/microblaze/gdbstub.c b/target/microblaze/gdbstub.c
index 29ac6e9c0f..6ffc5ad075 100644
--- a/target/microblaze/gdbstub.c
+++ b/target/microblaze/gdbstub.c
@@ -94,8 +94,10 @@ int mb_cpu_gdb_read_register(CPUState *cs, GByteArray *mem_buf, int n)
return gdb_get_reg32(mem_buf, val);
}
-int mb_cpu_gdb_read_stack_protect(CPUMBState *env, GByteArray *mem_buf, int n)
+int mb_cpu_gdb_read_stack_protect(CPUState *cs, GByteArray *mem_buf, int n)
{
+ MicroBlazeCPU *cpu = MICROBLAZE_CPU(cs);
+ CPUMBState *env = &cpu->env;
uint32_t val;
switch (n) {
@@ -153,8 +155,11 @@ int mb_cpu_gdb_write_register(CPUState *cs, uint8_t *mem_buf, int n)
return 4;
}
-int mb_cpu_gdb_write_stack_protect(CPUMBState *env, uint8_t *mem_buf, int n)
+int mb_cpu_gdb_write_stack_protect(CPUState *cs, uint8_t *mem_buf, int n)
{
+ MicroBlazeCPU *cpu = MICROBLAZE_CPU(cs);
+ CPUMBState *env = &cpu->env;
+
switch (n) {
case GDB_SP_SHL:
env->slr = ldl_p(mem_buf);
diff --git a/target/ppc/gdbstub.c b/target/ppc/gdbstub.c
index 9f2aad7aa9..2392625441 100644
--- a/target/ppc/gdbstub.c
+++ b/target/ppc/gdbstub.c
@@ -366,8 +366,10 @@ static int gdb_find_spr_idx(CPUPPCState *env, int n)
return -1;
}
-static int gdb_get_spr_reg(CPUPPCState *env, GByteArray *buf, int n)
+static int gdb_get_spr_reg(CPUState *cs, GByteArray *buf, int n)
{
+ PowerPCCPU *cpu = POWERPC_CPU(cs);
+ CPUPPCState *env = &cpu->env;
int reg;
int len;
@@ -382,8 +384,10 @@ static int gdb_get_spr_reg(CPUPPCState *env, GByteArray *buf, int n)
return len;
}
-static int gdb_set_spr_reg(CPUPPCState *env, uint8_t *mem_buf, int n)
+static int gdb_set_spr_reg(CPUState *cs, uint8_t *mem_buf, int n)
{
+ PowerPCCPU *cpu = POWERPC_CPU(cs);
+ CPUPPCState *env = &cpu->env;
int reg;
int len;
@@ -400,8 +404,10 @@ static int gdb_set_spr_reg(CPUPPCState *env, uint8_t *mem_buf, int n)
}
#endif
-static int gdb_get_float_reg(CPUPPCState *env, GByteArray *buf, int n)
+static int gdb_get_float_reg(CPUState *cs, GByteArray *buf, int n)
{
+ PowerPCCPU *cpu = POWERPC_CPU(cs);
+ CPUPPCState *env = &cpu->env;
uint8_t *mem_buf;
if (n < 32) {
gdb_get_reg64(buf, *cpu_fpr_ptr(env, n));
@@ -418,8 +424,11 @@ static int gdb_get_float_reg(CPUPPCState *env, GByteArray *buf, int n)
return 0;
}
-static int gdb_set_float_reg(CPUPPCState *env, uint8_t *mem_buf, int n)
+static int gdb_set_float_reg(CPUState *cs, uint8_t *mem_buf, int n)
{
+ PowerPCCPU *cpu = POWERPC_CPU(cs);
+ CPUPPCState *env = &cpu->env;
+
if (n < 32) {
ppc_maybe_bswap_register(env, mem_buf, 8);
*cpu_fpr_ptr(env, n) = ldq_p(mem_buf);
@@ -433,8 +442,10 @@ static int gdb_set_float_reg(CPUPPCState *env, uint8_t *mem_buf, int n)
return 0;
}
-static int gdb_get_avr_reg(CPUPPCState *env, GByteArray *buf, int n)
+static int gdb_get_avr_reg(CPUState *cs, GByteArray *buf, int n)
{
+ PowerPCCPU *cpu = POWERPC_CPU(cs);
+ CPUPPCState *env = &cpu->env;
uint8_t *mem_buf;
if (n < 32) {
@@ -459,8 +470,11 @@ static int gdb_get_avr_reg(CPUPPCState *env, GByteArray *buf, int n)
return 0;
}
-static int gdb_set_avr_reg(CPUPPCState *env, uint8_t *mem_buf, int n)
+static int gdb_set_avr_reg(CPUState *cs, uint8_t *mem_buf, int n)
{
+ PowerPCCPU *cpu = POWERPC_CPU(cs);
+ CPUPPCState *env = &cpu->env;
+
if (n < 32) {
ppc_avr_t *avr = cpu_avr_ptr(env, n);
ppc_maybe_bswap_register(env, mem_buf, 16);
@@ -481,8 +495,11 @@ static int gdb_set_avr_reg(CPUPPCState *env, uint8_t *mem_buf, int n)
return 0;
}
-static int gdb_get_spe_reg(CPUPPCState *env, GByteArray *buf, int n)
+static int gdb_get_spe_reg(CPUState *cs, GByteArray *buf, int n)
{
+ PowerPCCPU *cpu = POWERPC_CPU(cs);
+ CPUPPCState *env = &cpu->env;
+
if (n < 32) {
#if defined(TARGET_PPC64)
gdb_get_reg32(buf, env->gpr[n] >> 32);
@@ -505,8 +522,11 @@ static int gdb_get_spe_reg(CPUPPCState *env, GByteArray *buf, int n)
return 0;
}
-static int gdb_set_spe_reg(CPUPPCState *env, uint8_t *mem_buf, int n)
+static int gdb_set_spe_reg(CPUState *cs, uint8_t *mem_buf, int n)
{
+ PowerPCCPU *cpu = POWERPC_CPU(cs);
+ CPUPPCState *env = &cpu->env;
+
if (n < 32) {
#if defined(TARGET_PPC64)
target_ulong lo = (uint32_t)env->gpr[n];
@@ -534,8 +554,11 @@ static int gdb_set_spe_reg(CPUPPCState *env, uint8_t *mem_buf, int n)
return 0;
}
-static int gdb_get_vsx_reg(CPUPPCState *env, GByteArray *buf, int n)
+static int gdb_get_vsx_reg(CPUState *cs, GByteArray *buf, int n)
{
+ PowerPCCPU *cpu = POWERPC_CPU(cs);
+ CPUPPCState *env = &cpu->env;
+
if (n < 32) {
gdb_get_reg64(buf, *cpu_vsrl_ptr(env, n));
ppc_maybe_bswap_register(env, gdb_get_reg_ptr(buf, 8), 8);
@@ -544,8 +567,11 @@ static int gdb_get_vsx_reg(CPUPPCState *env, GByteArray *buf, int n)
return 0;
}
-static int gdb_set_vsx_reg(CPUPPCState *env, uint8_t *mem_buf, int n)
+static int gdb_set_vsx_reg(CPUState *cs, uint8_t *mem_buf, int n)
{
+ PowerPCCPU *cpu = POWERPC_CPU(cs);
+ CPUPPCState *env = &cpu->env;
+
if (n < 32) {
ppc_maybe_bswap_register(env, mem_buf, 8);
*cpu_vsrl_ptr(env, n) = ldq_p(mem_buf);
diff --git a/target/riscv/gdbstub.c b/target/riscv/gdbstub.c
index e33906635c..aa0824e119 100644
--- a/target/riscv/gdbstub.c
+++ b/target/riscv/gdbstub.c
@@ -108,8 +108,11 @@ int riscv_cpu_gdb_write_register(CPUState *cs, uint8_t *mem_buf, int n)
return length;
}
-static int riscv_gdb_get_fpu(CPURISCVState *env, GByteArray *buf, int n)
+static int riscv_gdb_get_fpu(CPUState *cs, GByteArray *buf, int n)
{
+ RISCVCPU *cpu = RISCV_CPU(cs);
+ CPURISCVState *env = &cpu->env;
+
if (n < 32) {
if (env->misa_ext & RVD) {
return gdb_get_reg64(buf, env->fpr[n]);
@@ -121,8 +124,11 @@ static int riscv_gdb_get_fpu(CPURISCVState *env, GByteArray *buf, int n)
return 0;
}
-static int riscv_gdb_set_fpu(CPURISCVState *env, uint8_t *mem_buf, int n)
+static int riscv_gdb_set_fpu(CPUState *cs, uint8_t *mem_buf, int n)
{
+ RISCVCPU *cpu = RISCV_CPU(cs);
+ CPURISCVState *env = &cpu->env;
+
if (n < 32) {
env->fpr[n] = ldq_p(mem_buf); /* always 64-bit */
return sizeof(uint64_t);
@@ -130,8 +136,10 @@ static int riscv_gdb_set_fpu(CPURISCVState *env, uint8_t *mem_buf, int n)
return 0;
}
-static int riscv_gdb_get_vector(CPURISCVState *env, GByteArray *buf, int n)
+static int riscv_gdb_get_vector(CPUState *cs, GByteArray *buf, int n)
{
+ RISCVCPU *cpu = RISCV_CPU(cs);
+ CPURISCVState *env = &cpu->env;
uint16_t vlenb = riscv_cpu_cfg(env)->vlen >> 3;
if (n < 32) {
int i;
@@ -146,8 +154,10 @@ static int riscv_gdb_get_vector(CPURISCVState *env, GByteArray *buf, int n)
return 0;
}
-static int riscv_gdb_set_vector(CPURISCVState *env, uint8_t *mem_buf, int n)
+static int riscv_gdb_set_vector(CPUState *cs, uint8_t *mem_buf, int n)
{
+ RISCVCPU *cpu = RISCV_CPU(cs);
+ CPURISCVState *env = &cpu->env;
uint16_t vlenb = riscv_cpu_cfg(env)->vlen >> 3;
if (n < 32) {
int i;
@@ -160,8 +170,11 @@ static int riscv_gdb_set_vector(CPURISCVState *env, uint8_t *mem_buf, int n)
return 0;
}
-static int riscv_gdb_get_csr(CPURISCVState *env, GByteArray *buf, int n)
+static int riscv_gdb_get_csr(CPUState *cs, GByteArray *buf, int n)
{
+ RISCVCPU *cpu = RISCV_CPU(cs);
+ CPURISCVState *env = &cpu->env;
+
if (n < CSR_TABLE_SIZE) {
target_ulong val = 0;
int result;
@@ -174,8 +187,11 @@ static int riscv_gdb_get_csr(CPURISCVState *env, GByteArray *buf, int n)
return 0;
}
-static int riscv_gdb_set_csr(CPURISCVState *env, uint8_t *mem_buf, int n)
+static int riscv_gdb_set_csr(CPUState *cs, uint8_t *mem_buf, int n)
{
+ RISCVCPU *cpu = RISCV_CPU(cs);
+ CPURISCVState *env = &cpu->env;
+
if (n < CSR_TABLE_SIZE) {
target_ulong val = ldtul_p(mem_buf);
int result;
@@ -188,25 +204,31 @@ static int riscv_gdb_set_csr(CPURISCVState *env, uint8_t *mem_buf, int n)
return 0;
}
-static int riscv_gdb_get_virtual(CPURISCVState *cs, GByteArray *buf, int n)
+static int riscv_gdb_get_virtual(CPUState *cs, GByteArray *buf, int n)
{
if (n == 0) {
#ifdef CONFIG_USER_ONLY
return gdb_get_regl(buf, 0);
#else
- return gdb_get_regl(buf, cs->priv);
+ RISCVCPU *cpu = RISCV_CPU(cs);
+ CPURISCVState *env = &cpu->env;
+
+ return gdb_get_regl(buf, env->priv);
#endif
}
return 0;
}
-static int riscv_gdb_set_virtual(CPURISCVState *cs, uint8_t *mem_buf, int n)
+static int riscv_gdb_set_virtual(CPUState *cs, uint8_t *mem_buf, int n)
{
if (n == 0) {
#ifndef CONFIG_USER_ONLY
- cs->priv = ldtul_p(mem_buf) & 0x3;
- if (cs->priv == PRV_RESERVED) {
- cs->priv = PRV_S;
+ RISCVCPU *cpu = RISCV_CPU(cs);
+ CPURISCVState *env = &cpu->env;
+
+ env->priv = ldtul_p(mem_buf) & 0x3;
+ if (env->priv == PRV_RESERVED) {
+ env->priv = PRV_S;
}
#endif
return sizeof(target_ulong);
diff --git a/target/s390x/gdbstub.c b/target/s390x/gdbstub.c
index 02c388dc32..c1e7c59b82 100644
--- a/target/s390x/gdbstub.c
+++ b/target/s390x/gdbstub.c
@@ -70,8 +70,11 @@ int s390_cpu_gdb_write_register(CPUState *cs, uint8_t *mem_buf, int n)
#define S390_A0_REGNUM 0
#define S390_A15_REGNUM 15
-static int cpu_read_ac_reg(CPUS390XState *env, GByteArray *buf, int n)
+static int cpu_read_ac_reg(CPUState *cs, GByteArray *buf, int n)
{
+ S390CPU *cpu = S390_CPU(cs);
+ CPUS390XState *env = &cpu->env;
+
switch (n) {
case S390_A0_REGNUM ... S390_A15_REGNUM:
return gdb_get_reg32(buf, env->aregs[n]);
@@ -80,8 +83,11 @@ static int cpu_read_ac_reg(CPUS390XState *env, GByteArray *buf, int n)
}
}
-static int cpu_write_ac_reg(CPUS390XState *env, uint8_t *mem_buf, int n)
+static int cpu_write_ac_reg(CPUState *cs, uint8_t *mem_buf, int n)
{
+ S390CPU *cpu = S390_CPU(cs);
+ CPUS390XState *env = &cpu->env;
+
switch (n) {
case S390_A0_REGNUM ... S390_A15_REGNUM:
env->aregs[n] = ldl_p(mem_buf);
@@ -97,8 +103,11 @@ static int cpu_write_ac_reg(CPUS390XState *env, uint8_t *mem_buf, int n)
#define S390_F0_REGNUM 1
#define S390_F15_REGNUM 16
-static int cpu_read_fp_reg(CPUS390XState *env, GByteArray *buf, int n)
+static int cpu_read_fp_reg(CPUState *cs, GByteArray *buf, int n)
{
+ S390CPU *cpu = S390_CPU(cs);
+ CPUS390XState *env = &cpu->env;
+
switch (n) {
case S390_FPC_REGNUM:
return gdb_get_reg32(buf, env->fpc);
@@ -109,8 +118,11 @@ static int cpu_read_fp_reg(CPUS390XState *env, GByteArray *buf, int n)
}
}
-static int cpu_write_fp_reg(CPUS390XState *env, uint8_t *mem_buf, int n)
+static int cpu_write_fp_reg(CPUState *cs, uint8_t *mem_buf, int n)
{
+ S390CPU *cpu = S390_CPU(cs);
+ CPUS390XState *env = &cpu->env;
+
switch (n) {
case S390_FPC_REGNUM:
env->fpc = ldl_p(mem_buf);
@@ -129,8 +141,10 @@ static int cpu_write_fp_reg(CPUS390XState *env, uint8_t *mem_buf, int n)
#define S390_V16_REGNUM 16
#define S390_V31_REGNUM 31
-static int cpu_read_vreg(CPUS390XState *env, GByteArray *buf, int n)
+static int cpu_read_vreg(CPUState *cs, GByteArray *buf, int n)
{
+ S390CPU *cpu = S390_CPU(cs);
+ CPUS390XState *env = &cpu->env;
int ret;
switch (n) {
@@ -148,8 +162,11 @@ static int cpu_read_vreg(CPUS390XState *env, GByteArray *buf, int n)
return ret;
}
-static int cpu_write_vreg(CPUS390XState *env, uint8_t *mem_buf, int n)
+static int cpu_write_vreg(CPUState *cs, uint8_t *mem_buf, int n)
{
+ S390CPU *cpu = S390_CPU(cs);
+ CPUS390XState *env = &cpu->env;
+
switch (n) {
case S390_V0L_REGNUM ... S390_V15L_REGNUM:
env->vregs[n][1] = ldtul_p(mem_buf + 8);
@@ -168,8 +185,11 @@ static int cpu_write_vreg(CPUS390XState *env, uint8_t *mem_buf, int n)
#define S390_C15_REGNUM 15
#ifndef CONFIG_USER_ONLY
-static int cpu_read_c_reg(CPUS390XState *env, GByteArray *buf, int n)
+static int cpu_read_c_reg(CPUState *cs, GByteArray *buf, int n)
{
+ S390CPU *cpu = S390_CPU(cs);
+ CPUS390XState *env = &cpu->env;
+
switch (n) {
case S390_C0_REGNUM ... S390_C15_REGNUM:
return gdb_get_regl(buf, env->cregs[n]);
@@ -178,8 +198,11 @@ static int cpu_read_c_reg(CPUS390XState *env, GByteArray *buf, int n)
}
}
-static int cpu_write_c_reg(CPUS390XState *env, uint8_t *mem_buf, int n)
+static int cpu_write_c_reg(CPUState *cs, uint8_t *mem_buf, int n)
{
+ S390CPU *cpu = S390_CPU(cs);
+ CPUS390XState *env = &cpu->env;
+
switch (n) {
case S390_C0_REGNUM ... S390_C15_REGNUM:
env->cregs[n] = ldtul_p(mem_buf);
@@ -199,8 +222,11 @@ static int cpu_write_c_reg(CPUS390XState *env, uint8_t *mem_buf, int n)
#define S390_VIRT_BEA_REGNUM 2
#define S390_VIRT_PREFIX_REGNUM 3
-static int cpu_read_virt_reg(CPUS390XState *env, GByteArray *mem_buf, int n)
+static int cpu_read_virt_reg(CPUState *cs, GByteArray *mem_buf, int n)
{
+ S390CPU *cpu = S390_CPU(cs);
+ CPUS390XState *env = &cpu->env;
+
switch (n) {
case S390_VIRT_CKC_REGNUM:
return gdb_get_regl(mem_buf, env->ckc);
@@ -215,24 +241,27 @@ static int cpu_read_virt_reg(CPUS390XState *env, GByteArray *mem_buf, int n)
}
}
-static int cpu_write_virt_reg(CPUS390XState *env, uint8_t *mem_buf, int n)
+static int cpu_write_virt_reg(CPUState *cs, uint8_t *mem_buf, int n)
{
+ S390CPU *cpu = S390_CPU(cs);
+ CPUS390XState *env = &cpu->env;
+
switch (n) {
case S390_VIRT_CKC_REGNUM:
env->ckc = ldtul_p(mem_buf);
- cpu_synchronize_post_init(env_cpu(env));
+ cpu_synchronize_post_init(cs);
return 8;
case S390_VIRT_CPUTM_REGNUM:
env->cputm = ldtul_p(mem_buf);
- cpu_synchronize_post_init(env_cpu(env));
+ cpu_synchronize_post_init(cs);
return 8;
case S390_VIRT_BEA_REGNUM:
env->gbea = ldtul_p(mem_buf);
- cpu_synchronize_post_init(env_cpu(env));
+ cpu_synchronize_post_init(cs);
return 8;
case S390_VIRT_PREFIX_REGNUM:
env->psa = ldtul_p(mem_buf);
- cpu_synchronize_post_init(env_cpu(env));
+ cpu_synchronize_post_init(cs);
return 8;
default:
return 0;
@@ -245,8 +274,11 @@ static int cpu_write_virt_reg(CPUS390XState *env, uint8_t *mem_buf, int n)
#define S390_VIRT_KVM_PFS_REGNUM 2
#define S390_VIRT_KVM_PFC_REGNUM 3
-static int cpu_read_virt_kvm_reg(CPUS390XState *env, GByteArray *mem_buf, int n)
+static int cpu_read_virt_kvm_reg(CPUState *cs, GByteArray *mem_buf, int n)
{
+ S390CPU *cpu = S390_CPU(cs);
+ CPUS390XState *env = &cpu->env;
+
switch (n) {
case S390_VIRT_KVM_PP_REGNUM:
return gdb_get_regl(mem_buf, env->pp);
@@ -261,8 +293,11 @@ static int cpu_read_virt_kvm_reg(CPUS390XState *env, GByteArray *mem_buf, int n)
}
}
-static int cpu_write_virt_kvm_reg(CPUS390XState *env, uint8_t *mem_buf, int n)
+static int cpu_write_virt_kvm_reg(CPUState *cs, uint8_t *mem_buf, int n)
{
+ S390CPU *cpu = S390_CPU(cs);
+ CPUS390XState *env = &cpu->env;
+
switch (n) {
case S390_VIRT_KVM_PP_REGNUM:
env->pp = ldtul_p(mem_buf);
@@ -292,13 +327,19 @@ static int cpu_write_virt_kvm_reg(CPUS390XState *env, uint8_t *mem_buf, int n)
#define S390_GS_GSSM_REGNUM 2
#define S390_GS_GSEPLA_REGNUM 3
-static int cpu_read_gs_reg(CPUS390XState *env, GByteArray *buf, int n)
+static int cpu_read_gs_reg(CPUState *cs, GByteArray *buf, int n)
{
+ S390CPU *cpu = S390_CPU(cs);
+ CPUS390XState *env = &cpu->env;
+
return gdb_get_regl(buf, env->gscb[n]);
}
-static int cpu_write_gs_reg(CPUS390XState *env, uint8_t *mem_buf, int n)
+static int cpu_write_gs_reg(CPUState *cs, uint8_t *mem_buf, int n)
{
+ S390CPU *cpu = S390_CPU(cs);
+ CPUS390XState *env = &cpu->env;
+
env->gscb[n] = ldtul_p(mem_buf);
cpu_synchronize_post_init(env_cpu(env));
return 8;
--
2.42.0
^ permalink raw reply related [flat|nested] 24+ messages in thread
* [PATCH v11 10/18] gdbstub: Simplify XML lookup
2023-10-12 13:05 [PATCH v11 00/18] plugins: Allow to read registers Akihiko Odaki
` (8 preceding siblings ...)
2023-10-12 13:06 ` [PATCH v11 09/18] gdbstub: Change gdb_get_reg_cb and gdb_set_reg_cb Akihiko Odaki
@ 2023-10-12 13:06 ` Akihiko Odaki
2023-10-12 13:06 ` [PATCH v11 11/18] gdbstub: Infer number of core registers from XML Akihiko Odaki
` (7 subsequent siblings)
17 siblings, 0 replies; 24+ messages in thread
From: Akihiko Odaki @ 2023-10-12 13:06 UTC (permalink / raw)
Cc: Alex Bennée, Mikhail Tyutin, Aleksandr Anenkov, qemu-devel,
Philippe Mathieu-Daudé, Akihiko Odaki
Now we know all instances of GDBFeature that is used in CPU so we can
traverse them to find XML. This removes the need for a CPU-specific
lookup function for dynamic XMLs.
Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>
---
include/exec/gdbstub.h | 2 +
gdbstub/gdbstub.c | 85 +++++++++++++++++++++++-------------------
hw/core/cpu-common.c | 5 ++-
3 files changed, 52 insertions(+), 40 deletions(-)
diff --git a/include/exec/gdbstub.h b/include/exec/gdbstub.h
index c34ecfc37a..70e16be79e 100644
--- a/include/exec/gdbstub.h
+++ b/include/exec/gdbstub.h
@@ -26,6 +26,8 @@ typedef struct GDBFeatureBuilder {
typedef int (*gdb_get_reg_cb)(CPUState *cpu, GByteArray *buf, int reg);
typedef int (*gdb_set_reg_cb)(CPUState *cpu, uint8_t *buf, int reg);
+void gdb_init_cpu(CPUState *cpu);
+
/**
* gdb_register_coprocessor() - register a supplemental set of registers
* @cpu - the CPU associated with registers
diff --git a/gdbstub/gdbstub.c b/gdbstub/gdbstub.c
index 63c049745a..49fb23a68a 100644
--- a/gdbstub/gdbstub.c
+++ b/gdbstub/gdbstub.c
@@ -352,6 +352,7 @@ static const char *get_feature_xml(const char *p, const char **newp,
{
CPUState *cpu = gdb_get_first_cpu_in_process(process);
CPUClass *cc = CPU_GET_CLASS(cpu);
+ GDBRegisterState *r;
size_t len;
/*
@@ -365,7 +366,6 @@ static const char *get_feature_xml(const char *p, const char **newp,
/* Is it the main target xml? */
if (strncmp(p, "target.xml", len) == 0) {
if (!process->target_xml) {
- GDBRegisterState *r;
g_autoptr(GPtrArray) xml = g_ptr_array_new_with_free_func(g_free);
g_ptr_array_add(
@@ -380,10 +380,6 @@ static const char *get_feature_xml(const char *p, const char **newp,
g_markup_printf_escaped("<architecture>%s</architecture>",
cc->gdb_arch_name(cpu)));
}
- g_ptr_array_add(
- xml,
- g_markup_printf_escaped("<xi:include href=\"%s\"/>",
- cc->gdb_core_xml_file));
for (guint i = 0; i < cpu->gdb_regs->len; i++) {
r = &g_array_index(cpu->gdb_regs, GDBRegisterState, i);
g_ptr_array_add(
@@ -398,20 +394,11 @@ static const char *get_feature_xml(const char *p, const char **newp,
}
return process->target_xml;
}
- /* Is it dynamically generated by the target? */
- if (cc->gdb_get_dynamic_xml) {
- g_autofree char *xmlname = g_strndup(p, len);
- const char *xml = cc->gdb_get_dynamic_xml(cpu, xmlname);
- if (xml) {
- return xml;
- }
- }
- /* Is it one of the encoded gdb-xml/ files? */
- for (int i = 0; gdb_static_features[i].xmlname; i++) {
- const char *name = gdb_static_features[i].xmlname;
- if ((strncmp(name, p, len) == 0) &&
- strlen(name) == len) {
- return gdb_static_features[i].xml;
+ /* Is it one of the features? */
+ for (guint i = 0; i < cpu->gdb_regs->len; i++) {
+ r = &g_array_index(cpu->gdb_regs, GDBRegisterState, i);
+ if (strncmp(p, r->feature->xmlname, len) == 0) {
+ return r->feature->xml;
}
}
@@ -527,40 +514,62 @@ static int gdb_write_register(CPUState *cpu, uint8_t *mem_buf, int reg)
return 0;
}
+static void gdb_register_feature(CPUState *cpu, int base_reg,
+ gdb_get_reg_cb get_reg, gdb_set_reg_cb set_reg,
+ const GDBFeature *feature)
+{
+ guint i = cpu->gdb_regs->len;
+ GDBRegisterState *s;
+
+ g_array_set_size(cpu->gdb_regs, i + 1);
+ s = &g_array_index(cpu->gdb_regs, GDBRegisterState, i);
+ s->base_reg = base_reg;
+ s->get_reg = get_reg;
+ s->set_reg = set_reg;
+ s->feature = feature;
+}
+
+void gdb_init_cpu(CPUState *cpu)
+{
+ CPUClass *cc = CPU_GET_CLASS(cpu);
+ const GDBFeature *feature;
+
+ cpu->gdb_regs = g_array_new(false, false, sizeof(GDBRegisterState));
+
+ if (cc->gdb_core_xml_file) {
+ feature = gdb_find_static_feature(cc->gdb_core_xml_file);
+ gdb_register_feature(cpu, 0,
+ cc->gdb_read_register, cc->gdb_write_register,
+ feature);
+ }
+
+ cpu->gdb_num_regs = cpu->gdb_num_g_regs = cc->gdb_num_core_regs;
+}
+
void gdb_register_coprocessor(CPUState *cpu,
gdb_get_reg_cb get_reg, gdb_set_reg_cb set_reg,
const GDBFeature *feature, int g_pos)
{
GDBRegisterState *s;
guint i;
+ int base_reg = cpu->gdb_num_regs;
- if (cpu->gdb_regs) {
- for (i = 0; i < cpu->gdb_regs->len; i++) {
- /* Check for duplicates. */
- s = &g_array_index(cpu->gdb_regs, GDBRegisterState, i);
- if (s->feature == feature) {
- return;
- }
+ for (i = 0; i < cpu->gdb_regs->len; i++) {
+ /* Check for duplicates. */
+ s = &g_array_index(cpu->gdb_regs, GDBRegisterState, i);
+ if (s->feature == feature) {
+ return;
}
- } else {
- cpu->gdb_regs = g_array_new(false, false, sizeof(GDBRegisterState));
- i = 0;
}
- g_array_set_size(cpu->gdb_regs, i + 1);
- s = &g_array_index(cpu->gdb_regs, GDBRegisterState, i);
- s->base_reg = cpu->gdb_num_regs;
- s->get_reg = get_reg;
- s->set_reg = set_reg;
- s->feature = feature;
+ gdb_register_feature(cpu, base_reg, get_reg, set_reg, feature);
/* Add to end of list. */
cpu->gdb_num_regs += feature->num_regs;
if (g_pos) {
- if (g_pos != s->base_reg) {
+ if (g_pos != base_reg) {
error_report("Error: Bad gdb register numbering for '%s', "
- "expected %d got %d", feature->xml,
- g_pos, s->base_reg);
+ "expected %d got %d", feature->xml, g_pos, base_reg);
} else {
cpu->gdb_num_g_regs = cpu->gdb_num_regs;
}
diff --git a/hw/core/cpu-common.c b/hw/core/cpu-common.c
index bab8942c30..2a2a6eb3eb 100644
--- a/hw/core/cpu-common.c
+++ b/hw/core/cpu-common.c
@@ -27,6 +27,7 @@
#include "qemu/main-loop.h"
#include "exec/log.h"
#include "exec/cpu-common.h"
+#include "exec/gdbstub.h"
#include "qemu/error-report.h"
#include "qemu/qemu-print.h"
#include "sysemu/tcg.h"
@@ -223,11 +224,10 @@ static void cpu_common_unrealizefn(DeviceState *dev)
static void cpu_common_initfn(Object *obj)
{
CPUState *cpu = CPU(obj);
- CPUClass *cc = CPU_GET_CLASS(obj);
+ gdb_init_cpu(cpu);
cpu->cpu_index = UNASSIGNED_CPU_INDEX;
cpu->cluster_index = UNASSIGNED_CLUSTER_INDEX;
- cpu->gdb_num_regs = cpu->gdb_num_g_regs = cc->gdb_num_core_regs;
/* user-mode doesn't have configurable SMP topology */
/* the default value is changed by qemu_init_vcpu() for system-mode */
cpu->nr_cores = 1;
@@ -247,6 +247,7 @@ static void cpu_common_finalize(Object *obj)
{
CPUState *cpu = CPU(obj);
+ g_array_free(cpu->gdb_regs, TRUE);
qemu_lockcnt_destroy(&cpu->in_ioctl_lock);
qemu_mutex_destroy(&cpu->work_mutex);
}
--
2.42.0
^ permalink raw reply related [flat|nested] 24+ messages in thread
* [PATCH v11 11/18] gdbstub: Infer number of core registers from XML
2023-10-12 13:05 [PATCH v11 00/18] plugins: Allow to read registers Akihiko Odaki
` (9 preceding siblings ...)
2023-10-12 13:06 ` [PATCH v11 10/18] gdbstub: Simplify XML lookup Akihiko Odaki
@ 2023-10-12 13:06 ` Akihiko Odaki
2023-10-12 13:43 ` Philippe Mathieu-Daudé
2023-10-12 13:06 ` [PATCH v11 12/18] hw/core/cpu: Remove gdb_get_dynamic_xml member Akihiko Odaki
` (6 subsequent siblings)
17 siblings, 1 reply; 24+ messages in thread
From: Akihiko Odaki @ 2023-10-12 13:06 UTC (permalink / raw)
Cc: Alex Bennée, Mikhail Tyutin, Aleksandr Anenkov, qemu-devel,
Philippe Mathieu-Daudé, Akihiko Odaki, Eduardo Habkost,
Marcel Apfelbaum, Yanan Wang, Peter Maydell, Michael Rolnik,
Brian Cain, Song Gao, Xiaojuan Yang, Laurent Vivier,
Edgar E. Iglesias, Palmer Dabbelt, Alistair Francis, Bin Meng,
Weiwei Li, Daniel Henrique Barboza, Liu Zhiwei, Yoshinori Sato,
Thomas Huth, Richard Henderson, David Hildenbrand,
Ilya Leoshkevich, open list:ARM TCG CPUs,
open list:RISC-V TCG CPUs, open list:S390 general arch...
GDBFeature has the num_regs member so use it where applicable to
remove magic numbers.
Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>
---
include/hw/core/cpu.h | 3 ++-
target/s390x/cpu.h | 2 --
gdbstub/gdbstub.c | 5 ++++-
target/arm/cpu.c | 1 -
target/arm/cpu64.c | 1 -
target/avr/cpu.c | 1 -
target/hexagon/cpu.c | 1 -
target/i386/cpu.c | 2 --
target/loongarch/cpu.c | 2 --
target/m68k/cpu.c | 1 -
target/microblaze/cpu.c | 1 -
target/riscv/cpu.c | 1 -
target/rx/cpu.c | 1 -
target/s390x/cpu.c | 1 -
14 files changed, 6 insertions(+), 17 deletions(-)
diff --git a/include/hw/core/cpu.h b/include/hw/core/cpu.h
index 3968369554..11d4b5cd0c 100644
--- a/include/hw/core/cpu.h
+++ b/include/hw/core/cpu.h
@@ -127,7 +127,8 @@ struct SysemuCPUOps;
* @gdb_adjust_breakpoint: Callback for adjusting the address of a
* breakpoint. Used by AVR to handle a gdb mis-feature with
* its Harvard architecture split code and data.
- * @gdb_num_core_regs: Number of core registers accessible to GDB.
+ * @gdb_num_core_regs: Number of core registers accessible to GDB or 0 to infer
+ * from @gdb_core_xml_file.
* @gdb_core_xml_file: File name for core registers GDB XML description.
* @gdb_stop_before_watchpoint: Indicates whether GDB expects the CPU to stop
* before the insn which triggers a watchpoint rather than after it.
diff --git a/target/s390x/cpu.h b/target/s390x/cpu.h
index 7bea7075e1..83eafbe4b1 100644
--- a/target/s390x/cpu.h
+++ b/target/s390x/cpu.h
@@ -452,8 +452,6 @@ static inline void cpu_get_tb_cpu_state(CPUS390XState *env, vaddr *pc,
#define S390_R13_REGNUM 15
#define S390_R14_REGNUM 16
#define S390_R15_REGNUM 17
-/* Total Core Registers. */
-#define S390_NUM_CORE_REGS 18
static inline void setcc(S390CPU *cpu, uint64_t cc)
{
diff --git a/gdbstub/gdbstub.c b/gdbstub/gdbstub.c
index 49fb23a68a..c19f1785e4 100644
--- a/gdbstub/gdbstub.c
+++ b/gdbstub/gdbstub.c
@@ -541,9 +541,12 @@ void gdb_init_cpu(CPUState *cpu)
gdb_register_feature(cpu, 0,
cc->gdb_read_register, cc->gdb_write_register,
feature);
+ cpu->gdb_num_regs = cpu->gdb_num_g_regs = feature->num_regs;
}
- cpu->gdb_num_regs = cpu->gdb_num_g_regs = cc->gdb_num_core_regs;
+ if (cc->gdb_num_core_regs) {
+ cpu->gdb_num_regs = cpu->gdb_num_g_regs = cc->gdb_num_core_regs;
+ }
}
void gdb_register_coprocessor(CPUState *cpu,
diff --git a/target/arm/cpu.c b/target/arm/cpu.c
index 6c6c551573..6d9bf6a14e 100644
--- a/target/arm/cpu.c
+++ b/target/arm/cpu.c
@@ -2391,7 +2391,6 @@ static void arm_cpu_class_init(ObjectClass *oc, void *data)
#ifndef CONFIG_USER_ONLY
cc->sysemu_ops = &arm_sysemu_ops;
#endif
- cc->gdb_num_core_regs = 26;
cc->gdb_arch_name = arm_gdb_arch_name;
cc->gdb_get_dynamic_xml = arm_gdb_get_dynamic_xml;
cc->gdb_stop_before_watchpoint = true;
diff --git a/target/arm/cpu64.c b/target/arm/cpu64.c
index 1cb9d5b81a..5c7a4a0bf7 100644
--- a/target/arm/cpu64.c
+++ b/target/arm/cpu64.c
@@ -792,7 +792,6 @@ static void aarch64_cpu_class_init(ObjectClass *oc, void *data)
cc->gdb_read_register = aarch64_cpu_gdb_read_register;
cc->gdb_write_register = aarch64_cpu_gdb_write_register;
- cc->gdb_num_core_regs = 34;
cc->gdb_core_xml_file = "aarch64-core.xml";
cc->gdb_arch_name = aarch64_gdb_arch_name;
diff --git a/target/avr/cpu.c b/target/avr/cpu.c
index 14d8b9d1f0..01adfb5089 100644
--- a/target/avr/cpu.c
+++ b/target/avr/cpu.c
@@ -244,7 +244,6 @@ static void avr_cpu_class_init(ObjectClass *oc, void *data)
cc->gdb_read_register = avr_cpu_gdb_read_register;
cc->gdb_write_register = avr_cpu_gdb_write_register;
cc->gdb_adjust_breakpoint = avr_cpu_gdb_adjust_breakpoint;
- cc->gdb_num_core_regs = 35;
cc->gdb_core_xml_file = "avr-cpu.xml";
cc->tcg_ops = &avr_tcg_ops;
}
diff --git a/target/hexagon/cpu.c b/target/hexagon/cpu.c
index 60d52e1e9d..7c1426f70c 100644
--- a/target/hexagon/cpu.c
+++ b/target/hexagon/cpu.c
@@ -385,7 +385,6 @@ static void hexagon_cpu_class_init(ObjectClass *c, void *data)
cc->get_pc = hexagon_cpu_get_pc;
cc->gdb_read_register = hexagon_gdb_read_register;
cc->gdb_write_register = hexagon_gdb_write_register;
- cc->gdb_num_core_regs = TOTAL_PER_THREAD_REGS;
cc->gdb_stop_before_watchpoint = true;
cc->gdb_core_xml_file = "hexagon-core.xml";
cc->disas_set_info = hexagon_cpu_disas_set_info;
diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index 3aab05ddad..fa63ce9311 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -7968,10 +7968,8 @@ static void x86_cpu_common_class_init(ObjectClass *oc, void *data)
cc->gdb_arch_name = x86_gdb_arch_name;
#ifdef TARGET_X86_64
cc->gdb_core_xml_file = "i386-64bit.xml";
- cc->gdb_num_core_regs = 66;
#else
cc->gdb_core_xml_file = "i386-32bit.xml";
- cc->gdb_num_core_regs = 50;
#endif
cc->disas_set_info = x86_disas_set_info;
diff --git a/target/loongarch/cpu.c b/target/loongarch/cpu.c
index ef1bf89dac..1b25920895 100644
--- a/target/loongarch/cpu.c
+++ b/target/loongarch/cpu.c
@@ -775,7 +775,6 @@ static void loongarch32_cpu_class_init(ObjectClass *c, void *data)
{
CPUClass *cc = CPU_CLASS(c);
- cc->gdb_num_core_regs = 35;
cc->gdb_core_xml_file = "loongarch-base32.xml";
cc->gdb_arch_name = loongarch32_gdb_arch_name;
}
@@ -789,7 +788,6 @@ static void loongarch64_cpu_class_init(ObjectClass *c, void *data)
{
CPUClass *cc = CPU_CLASS(c);
- cc->gdb_num_core_regs = 35;
cc->gdb_core_xml_file = "loongarch-base64.xml";
cc->gdb_arch_name = loongarch64_gdb_arch_name;
}
diff --git a/target/m68k/cpu.c b/target/m68k/cpu.c
index 538d9473c2..5fdbe5602b 100644
--- a/target/m68k/cpu.c
+++ b/target/m68k/cpu.c
@@ -565,7 +565,6 @@ static void m68k_cpu_class_init(ObjectClass *c, void *data)
#endif
cc->disas_set_info = m68k_cpu_disas_set_info;
- cc->gdb_num_core_regs = 18;
cc->tcg_ops = &m68k_tcg_ops;
}
diff --git a/target/microblaze/cpu.c b/target/microblaze/cpu.c
index 1998f69828..9d3fbfe159 100644
--- a/target/microblaze/cpu.c
+++ b/target/microblaze/cpu.c
@@ -428,7 +428,6 @@ static void mb_cpu_class_init(ObjectClass *oc, void *data)
cc->sysemu_ops = &mb_sysemu_ops;
#endif
device_class_set_props(dc, mb_properties);
- cc->gdb_num_core_regs = 32 + 25;
cc->gdb_core_xml_file = "microblaze-core.xml";
cc->disas_set_info = mb_disas_set_info;
diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
index 9417a5d897..d6278d59d5 100644
--- a/target/riscv/cpu.c
+++ b/target/riscv/cpu.c
@@ -2167,7 +2167,6 @@ static void riscv_cpu_common_class_init(ObjectClass *c, void *data)
cc->get_pc = riscv_cpu_get_pc;
cc->gdb_read_register = riscv_cpu_gdb_read_register;
cc->gdb_write_register = riscv_cpu_gdb_write_register;
- cc->gdb_num_core_regs = 33;
cc->gdb_stop_before_watchpoint = true;
cc->disas_set_info = riscv_cpu_disas_set_info;
#ifndef CONFIG_USER_ONLY
diff --git a/target/rx/cpu.c b/target/rx/cpu.c
index 4d0d3a0c8c..7b9e46d1bc 100644
--- a/target/rx/cpu.c
+++ b/target/rx/cpu.c
@@ -235,7 +235,6 @@ static void rx_cpu_class_init(ObjectClass *klass, void *data)
cc->gdb_write_register = rx_cpu_gdb_write_register;
cc->disas_set_info = rx_cpu_disas_set_info;
- cc->gdb_num_core_regs = 26;
cc->gdb_core_xml_file = "rx-core.xml";
cc->tcg_ops = &rx_tcg_ops;
}
diff --git a/target/s390x/cpu.c b/target/s390x/cpu.c
index 6093ab0a12..f49ee0bb46 100644
--- a/target/s390x/cpu.c
+++ b/target/s390x/cpu.c
@@ -346,7 +346,6 @@ static void s390_cpu_class_init(ObjectClass *oc, void *data)
s390_cpu_class_init_sysemu(cc);
#endif
cc->disas_set_info = s390_cpu_disas_set_info;
- cc->gdb_num_core_regs = S390_NUM_CORE_REGS;
cc->gdb_core_xml_file = "s390x-core64.xml";
cc->gdb_arch_name = s390_gdb_arch_name;
--
2.42.0
^ permalink raw reply related [flat|nested] 24+ messages in thread
* [PATCH v11 12/18] hw/core/cpu: Remove gdb_get_dynamic_xml member
2023-10-12 13:05 [PATCH v11 00/18] plugins: Allow to read registers Akihiko Odaki
` (10 preceding siblings ...)
2023-10-12 13:06 ` [PATCH v11 11/18] gdbstub: Infer number of core registers from XML Akihiko Odaki
@ 2023-10-12 13:06 ` Akihiko Odaki
2023-10-12 13:45 ` Philippe Mathieu-Daudé
2023-10-12 13:06 ` [PATCH v11 13/18] gdbstub: Add members to identify registers to GDBFeature Akihiko Odaki
` (5 subsequent siblings)
17 siblings, 1 reply; 24+ messages in thread
From: Akihiko Odaki @ 2023-10-12 13:06 UTC (permalink / raw)
Cc: Alex Bennée, Mikhail Tyutin, Aleksandr Anenkov, qemu-devel,
Philippe Mathieu-Daudé, Akihiko Odaki, Eduardo Habkost,
Marcel Apfelbaum, Yanan Wang, Peter Maydell, Nicholas Piggin,
Daniel Henrique Barboza, Cédric Le Goater, Palmer Dabbelt,
Alistair Francis, Bin Meng, Weiwei Li, Liu Zhiwei,
open list:ARM TCG CPUs, open list:PowerPC TCG CPUs,
open list:RISC-V TCG CPUs
This function is no longer used.
Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>
---
include/hw/core/cpu.h | 4 ----
target/arm/cpu.h | 6 ------
target/ppc/cpu.h | 1 -
target/arm/cpu.c | 1 -
target/arm/gdbstub.c | 18 ------------------
target/ppc/cpu_init.c | 3 ---
target/ppc/gdbstub.c | 10 ----------
target/riscv/cpu.c | 14 --------------
8 files changed, 57 deletions(-)
diff --git a/include/hw/core/cpu.h b/include/hw/core/cpu.h
index 11d4b5cd0c..d2e70643f2 100644
--- a/include/hw/core/cpu.h
+++ b/include/hw/core/cpu.h
@@ -134,9 +134,6 @@ struct SysemuCPUOps;
* before the insn which triggers a watchpoint rather than after it.
* @gdb_arch_name: Optional callback that returns the architecture name known
* to GDB. The caller must free the returned string with g_free.
- * @gdb_get_dynamic_xml: Callback to return dynamically generated XML for the
- * gdb stub. Returns a pointer to the XML contents for the specified XML file
- * or NULL if the CPU doesn't have a dynamically generated content for it.
* @disas_set_info: Setup architecture specific components of disassembly info
* @adjust_watchpoint_address: Perform a target-specific adjustment to an
* address before attempting to match it against watchpoints.
@@ -167,7 +164,6 @@ struct CPUClass {
const char *gdb_core_xml_file;
const gchar * (*gdb_arch_name)(CPUState *cpu);
- const char * (*gdb_get_dynamic_xml)(CPUState *cpu, const char *xmlname);
void (*disas_set_info)(CPUState *cpu, disassemble_info *info);
diff --git a/target/arm/cpu.h b/target/arm/cpu.h
index 3702ddaab8..0e4f1d7d0c 100644
--- a/target/arm/cpu.h
+++ b/target/arm/cpu.h
@@ -1137,12 +1137,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);
-/* 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.
- */
-const char *arm_gdb_get_dynamic_xml(CPUState *cpu, const char *xmlname);
-
int arm_cpu_write_elf64_note(WriteCoreDumpFunction f, CPUState *cs,
int cpuid, DumpState *s);
int arm_cpu_write_elf32_note(WriteCoreDumpFunction f, CPUState *cs,
diff --git a/target/ppc/cpu.h b/target/ppc/cpu.h
index 84cdc3055f..1782b21edb 100644
--- a/target/ppc/cpu.h
+++ b/target/ppc/cpu.h
@@ -1385,7 +1385,6 @@ int ppc_cpu_gdb_write_register_apple(CPUState *cpu, uint8_t *buf, int reg);
#ifndef CONFIG_USER_ONLY
hwaddr ppc_cpu_get_phys_page_debug(CPUState *cpu, vaddr addr);
void ppc_gdb_gen_spr_feature(PowerPCCPU *cpu);
-const char *ppc_gdb_get_dynamic_xml(CPUState *cs, const char *xml_name);
#endif
int ppc64_cpu_write_elf64_note(WriteCoreDumpFunction f, CPUState *cs,
int cpuid, DumpState *s);
diff --git a/target/arm/cpu.c b/target/arm/cpu.c
index 6d9bf6a14e..e7fe11339b 100644
--- a/target/arm/cpu.c
+++ b/target/arm/cpu.c
@@ -2392,7 +2392,6 @@ static void arm_cpu_class_init(ObjectClass *oc, void *data)
cc->sysemu_ops = &arm_sysemu_ops;
#endif
cc->gdb_arch_name = arm_gdb_arch_name;
- cc->gdb_get_dynamic_xml = arm_gdb_get_dynamic_xml;
cc->gdb_stop_before_watchpoint = true;
cc->disas_set_info = arm_disas_set_info;
diff --git a/target/arm/gdbstub.c b/target/arm/gdbstub.c
index aa2c6c1bde..f9774b5547 100644
--- a/target/arm/gdbstub.c
+++ b/target/arm/gdbstub.c
@@ -463,24 +463,6 @@ static GDBFeature *arm_gen_dynamic_m_secextreg_feature(CPUState *cs)
#endif
#endif /* CONFIG_TCG */
-const char *arm_gdb_get_dynamic_xml(CPUState *cs, const char *xmlname)
-{
- ARMCPU *cpu = ARM_CPU(cs);
-
- if (strcmp(xmlname, "system-registers.xml") == 0) {
- return cpu->dyn_sysreg_feature.desc.xml;
- } else if (strcmp(xmlname, "sve-registers.xml") == 0) {
- return cpu->dyn_svereg_feature.desc.xml;
- } else if (strcmp(xmlname, "arm-m-system.xml") == 0) {
- return cpu->dyn_m_systemreg_feature.desc.xml;
-#ifndef CONFIG_USER_ONLY
- } else if (strcmp(xmlname, "arm-m-secext.xml") == 0) {
- return cpu->dyn_m_secextreg_feature.desc.xml;
-#endif
- }
- return NULL;
-}
-
void arm_cpu_register_gdb_regs_for_features(ARMCPU *cpu)
{
CPUState *cs = CPU(cpu);
diff --git a/target/ppc/cpu_init.c b/target/ppc/cpu_init.c
index af43e73af2..d8a992a45d 100644
--- a/target/ppc/cpu_init.c
+++ b/target/ppc/cpu_init.c
@@ -7384,9 +7384,6 @@ static void ppc_cpu_class_init(ObjectClass *oc, void *data)
#endif
cc->gdb_num_core_regs = 71;
-#ifndef CONFIG_USER_ONLY
- cc->gdb_get_dynamic_xml = ppc_gdb_get_dynamic_xml;
-#endif
#ifdef USE_APPLE_GDB
cc->gdb_read_register = ppc_cpu_gdb_read_register_apple;
cc->gdb_write_register = ppc_cpu_gdb_write_register_apple;
diff --git a/target/ppc/gdbstub.c b/target/ppc/gdbstub.c
index 2392625441..a20d5922dd 100644
--- a/target/ppc/gdbstub.c
+++ b/target/ppc/gdbstub.c
@@ -339,16 +339,6 @@ void ppc_gdb_gen_spr_feature(PowerPCCPU *cpu)
gdb_feature_builder_end(&builder);
}
-
-const char *ppc_gdb_get_dynamic_xml(CPUState *cs, const char *xml_name)
-{
- PowerPCCPUClass *pcc = POWERPC_CPU_GET_CLASS(cs);
-
- if (strcmp(xml_name, "power-spr.xml") == 0) {
- return pcc->gdb_spr.xml;
- }
- return NULL;
-}
#endif
#if !defined(CONFIG_USER_ONLY)
diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
index d6278d59d5..2f07fc193f 100644
--- a/target/riscv/cpu.c
+++ b/target/riscv/cpu.c
@@ -1984,19 +1984,6 @@ static const gchar *riscv_gdb_arch_name(CPUState *cs)
}
}
-static const char *riscv_gdb_get_dynamic_xml(CPUState *cs, const char *xmlname)
-{
- RISCVCPU *cpu = RISCV_CPU(cs);
-
- if (strcmp(xmlname, "riscv-csr.xml") == 0) {
- return cpu->dyn_csr_feature.xml;
- } else if (strcmp(xmlname, "riscv-vector.xml") == 0) {
- return cpu->dyn_vreg_feature.xml;
- }
-
- return NULL;
-}
-
#ifndef CONFIG_USER_ONLY
static int64_t riscv_get_arch_id(CPUState *cs)
{
@@ -2174,7 +2161,6 @@ static void riscv_cpu_common_class_init(ObjectClass *c, void *data)
cc->get_arch_id = riscv_get_arch_id;
#endif
cc->gdb_arch_name = riscv_gdb_arch_name;
- cc->gdb_get_dynamic_xml = riscv_gdb_get_dynamic_xml;
cc->tcg_ops = &riscv_tcg_ops;
object_class_property_add(c, "mvendorid", "uint32", cpu_get_mvendorid,
--
2.42.0
^ permalink raw reply related [flat|nested] 24+ messages in thread
* [PATCH v11 13/18] gdbstub: Add members to identify registers to GDBFeature
2023-10-12 13:05 [PATCH v11 00/18] plugins: Allow to read registers Akihiko Odaki
` (11 preceding siblings ...)
2023-10-12 13:06 ` [PATCH v11 12/18] hw/core/cpu: Remove gdb_get_dynamic_xml member Akihiko Odaki
@ 2023-10-12 13:06 ` Akihiko Odaki
2023-10-12 13:06 ` [PATCH v11 14/18] gdbstub: Expose functions to read registers Akihiko Odaki
` (4 subsequent siblings)
17 siblings, 0 replies; 24+ messages in thread
From: Akihiko Odaki @ 2023-10-12 13:06 UTC (permalink / raw)
Cc: Alex Bennée, Mikhail Tyutin, Aleksandr Anenkov, qemu-devel,
Philippe Mathieu-Daudé, Akihiko Odaki, John Snow,
Cleber Rosa, Palmer Dabbelt, Alistair Francis, Bin Meng,
Weiwei Li, Daniel Henrique Barboza, Liu Zhiwei,
open list:RISC-V TCG CPUs
These members will be used to help plugins to identify registers.
The added members in instances of GDBFeature dynamically generated by
CPUs will be filled in later changes.
Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>
---
include/exec/gdbstub.h | 3 +++
gdbstub/gdbstub.c | 10 ++++++++--
target/riscv/gdbstub.c | 4 +---
scripts/feature_to_c.py | 14 +++++++++++++-
4 files changed, 25 insertions(+), 6 deletions(-)
diff --git a/include/exec/gdbstub.h b/include/exec/gdbstub.h
index 70e16be79e..229fa382bc 100644
--- a/include/exec/gdbstub.h
+++ b/include/exec/gdbstub.h
@@ -13,12 +13,15 @@
typedef struct GDBFeature {
const char *xmlname;
const char *xml;
+ const char *name;
+ const char * const *regs;
int num_regs;
} GDBFeature;
typedef struct GDBFeatureBuilder {
GDBFeature *feature;
GPtrArray *xml;
+ GPtrArray *regs;
} GDBFeatureBuilder;
diff --git a/gdbstub/gdbstub.c b/gdbstub/gdbstub.c
index c19f1785e4..987af2298c 100644
--- a/gdbstub/gdbstub.c
+++ b/gdbstub/gdbstub.c
@@ -417,9 +417,10 @@ void gdb_feature_builder_init(GDBFeatureBuilder *builder, GDBFeature *feature,
builder->feature = feature;
builder->xml = g_ptr_array_new();
+ builder->regs = g_ptr_array_new();
g_ptr_array_add(builder->xml, header);
feature->xmlname = xmlname;
- feature->num_regs = 0;
+ feature->name = name;
}
void gdb_feature_builder_append_tag(const GDBFeatureBuilder *builder,
@@ -448,7 +449,9 @@ int gdb_feature_builder_append_reg(const GDBFeatureBuilder *builder,
name, bitsize, type);
}
- return builder->feature->num_regs++;
+ g_ptr_array_add(builder->regs, (void *)name);
+
+ return builder->regs->len - 1;
}
void gdb_feature_builder_end(const GDBFeatureBuilder *builder)
@@ -463,6 +466,9 @@ void gdb_feature_builder_end(const GDBFeatureBuilder *builder)
}
g_ptr_array_free(builder->xml, TRUE);
+
+ builder->feature->num_regs = builder->regs->len;
+ builder->feature->regs = (void *)g_ptr_array_free(builder->regs, FALSE);
}
const GDBFeature *gdb_find_static_feature(const char *xmlname)
diff --git a/target/riscv/gdbstub.c b/target/riscv/gdbstub.c
index aa0824e119..9082b0c6e2 100644
--- a/target/riscv/gdbstub.c
+++ b/target/riscv/gdbstub.c
@@ -265,11 +265,9 @@ static GDBFeature *riscv_gen_dynamic_csr_feature(CPUState *cs)
}
predicate = csr_ops[i].predicate;
if (predicate && (predicate(env, i) == RISCV_EXCP_NONE)) {
- g_autofree char *dynamic_name = NULL;
name = csr_ops[i].name;
if (!name) {
- dynamic_name = g_strdup_printf("csr%03x", i);
- name = dynamic_name;
+ name = g_strdup_printf("csr%03x", i);
}
gdb_feature_builder_append_reg(&builder, name, bitsize,
diff --git a/scripts/feature_to_c.py b/scripts/feature_to_c.py
index e04d6b2df7..807af0e685 100755
--- a/scripts/feature_to_c.py
+++ b/scripts/feature_to_c.py
@@ -50,7 +50,9 @@ def writeliteral(indent, bytes):
sys.stderr.write(f'unexpected start tag: {element.tag}\n')
exit(1)
+ feature_name = element.attrib['name']
regnum = 0
+ regnames = []
regnums = []
tags = ['feature']
for event, element in events:
@@ -67,6 +69,7 @@ def writeliteral(indent, bytes):
if 'regnum' in element.attrib:
regnum = int(element.attrib['regnum'])
+ regnames.append(element.attrib['name'])
regnums.append(regnum)
regnum += 1
@@ -85,6 +88,15 @@ def writeliteral(indent, bytes):
writeliteral(8, bytes(os.path.basename(input), 'utf-8'))
sys.stdout.write(',\n')
writeliteral(8, read)
- sys.stdout.write(f',\n {num_regs},\n }},\n')
+ sys.stdout.write(',\n')
+ writeliteral(8, bytes(feature_name, 'utf-8'))
+ sys.stdout.write(',\n (const char * const []) {\n')
+
+ for index, regname in enumerate(regnames):
+ sys.stdout.write(f' [{regnums[index] - base_reg}] =\n')
+ writeliteral(16, bytes(regname, 'utf-8'))
+ sys.stdout.write(',\n')
+
+ sys.stdout.write(f' }},\n {num_regs},\n }},\n')
sys.stdout.write(' { NULL }\n};\n')
--
2.42.0
^ permalink raw reply related [flat|nested] 24+ messages in thread
* [PATCH v11 14/18] gdbstub: Expose functions to read registers
2023-10-12 13:05 [PATCH v11 00/18] plugins: Allow to read registers Akihiko Odaki
` (12 preceding siblings ...)
2023-10-12 13:06 ` [PATCH v11 13/18] gdbstub: Add members to identify registers to GDBFeature Akihiko Odaki
@ 2023-10-12 13:06 ` Akihiko Odaki
2023-10-12 13:06 ` [PATCH v11 15/18] cpu: Call plugin hooks only when ready Akihiko Odaki
` (3 subsequent siblings)
17 siblings, 0 replies; 24+ messages in thread
From: Akihiko Odaki @ 2023-10-12 13:06 UTC (permalink / raw)
Cc: Alex Bennée, Mikhail Tyutin, Aleksandr Anenkov, qemu-devel,
Philippe Mathieu-Daudé, Akihiko Odaki
gdb_find_feature() and gdb_find_feature_register() find registers.
gdb_read_register() actually reads registers.
Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>
---
include/exec/gdbstub.h | 5 +++++
gdbstub/gdbstub.c | 31 ++++++++++++++++++++++++++++++-
2 files changed, 35 insertions(+), 1 deletion(-)
diff --git a/include/exec/gdbstub.h b/include/exec/gdbstub.h
index 229fa382bc..b85f700c5f 100644
--- a/include/exec/gdbstub.h
+++ b/include/exec/gdbstub.h
@@ -71,6 +71,11 @@ void gdb_feature_builder_end(const GDBFeatureBuilder *builder);
const GDBFeature *gdb_find_static_feature(const char *xmlname);
+int gdb_find_feature(CPUState *cpu, const char *name);
+int gdb_find_feature_register(CPUState *cpu, int feature, const char *name);
+
+int gdb_read_register(CPUState *cpu, GByteArray *buf, int reg);
+
void gdb_set_stop_cpu(CPUState *cpu);
/* in gdbstub-xml.c, generated by scripts/feature_to_c.py */
diff --git a/gdbstub/gdbstub.c b/gdbstub/gdbstub.c
index 987af2298c..a3fc65ec86 100644
--- a/gdbstub/gdbstub.c
+++ b/gdbstub/gdbstub.c
@@ -484,7 +484,36 @@ const GDBFeature *gdb_find_static_feature(const char *xmlname)
g_assert_not_reached();
}
-static int gdb_read_register(CPUState *cpu, GByteArray *buf, int reg)
+int gdb_find_feature(CPUState *cpu, const char *name)
+{
+ GDBRegisterState *r;
+
+ for (guint i = 0; i < cpu->gdb_regs->len; i++) {
+ r = &g_array_index(cpu->gdb_regs, GDBRegisterState, i);
+ if (!strcmp(name, r->feature->name)) {
+ return i;
+ }
+ }
+
+ return -1;
+}
+
+int gdb_find_feature_register(CPUState *cpu, int feature, const char *name)
+{
+ GDBRegisterState *r;
+
+ r = &g_array_index(cpu->gdb_regs, GDBRegisterState, feature);
+
+ for (int i = 0; i < r->feature->num_regs; i++) {
+ if (r->feature->regs[i] && !strcmp(name, r->feature->regs[i])) {
+ return r->base_reg + i;
+ }
+ }
+
+ return -1;
+}
+
+int gdb_read_register(CPUState *cpu, GByteArray *buf, int reg)
{
CPUClass *cc = CPU_GET_CLASS(cpu);
GDBRegisterState *r;
--
2.42.0
^ permalink raw reply related [flat|nested] 24+ messages in thread
* [PATCH v11 15/18] cpu: Call plugin hooks only when ready
2023-10-12 13:05 [PATCH v11 00/18] plugins: Allow to read registers Akihiko Odaki
` (13 preceding siblings ...)
2023-10-12 13:06 ` [PATCH v11 14/18] gdbstub: Expose functions to read registers Akihiko Odaki
@ 2023-10-12 13:06 ` Akihiko Odaki
2023-10-12 13:06 ` [PATCH v11 16/18] plugins: Use different helpers when reading registers Akihiko Odaki
` (2 subsequent siblings)
17 siblings, 0 replies; 24+ messages in thread
From: Akihiko Odaki @ 2023-10-12 13:06 UTC (permalink / raw)
Cc: Alex Bennée, Mikhail Tyutin, Aleksandr Anenkov, qemu-devel,
Philippe Mathieu-Daudé, Akihiko Odaki, Richard Henderson,
Paolo Bonzini
The initialization and exit hooks will not affect the state of vCPU
outside TCG context, but they may depend on the state of vCPU.
Therefore, it's better to call plugin hooks after the vCPU state is
fully initialized and before it gets uninitialized.
Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>
---
cpu-target.c | 11 -----------
hw/core/cpu-common.c | 10 ++++++++++
2 files changed, 10 insertions(+), 11 deletions(-)
diff --git a/cpu-target.c b/cpu-target.c
index 79363ae370..00cd7f4d69 100644
--- a/cpu-target.c
+++ b/cpu-target.c
@@ -42,7 +42,6 @@
#include "hw/core/accel-cpu.h"
#include "trace/trace-root.h"
#include "qemu/accel.h"
-#include "qemu/plugin.h"
uintptr_t qemu_host_page_size;
intptr_t qemu_host_page_mask;
@@ -143,11 +142,6 @@ void cpu_exec_realizefn(CPUState *cpu, Error **errp)
/* Wait until cpu initialization complete before exposing cpu. */
cpu_list_add(cpu);
- /* Plugin initialization must wait until cpu_index assigned. */
- if (tcg_enabled()) {
- qemu_plugin_vcpu_init_hook(cpu);
- }
-
#ifdef CONFIG_USER_ONLY
assert(qdev_get_vmsd(DEVICE(cpu)) == NULL ||
qdev_get_vmsd(DEVICE(cpu))->unmigratable);
@@ -174,11 +168,6 @@ void cpu_exec_unrealizefn(CPUState *cpu)
}
#endif
- /* Call the plugin hook before clearing cpu->cpu_index in cpu_list_remove */
- if (tcg_enabled()) {
- qemu_plugin_vcpu_exit_hook(cpu);
- }
-
cpu_list_remove(cpu);
/*
* Now that the vCPU has been removed from the RCU list, we can call
diff --git a/hw/core/cpu-common.c b/hw/core/cpu-common.c
index 2a2a6eb3eb..409397e2b5 100644
--- a/hw/core/cpu-common.c
+++ b/hw/core/cpu-common.c
@@ -210,6 +210,11 @@ static void cpu_common_realizefn(DeviceState *dev, Error **errp)
cpu_resume(cpu);
}
+ /* Plugin initialization must wait until the cpu is fully realized. */
+ if (tcg_enabled()) {
+ qemu_plugin_vcpu_init_hook(cpu);
+ }
+
/* NOTE: latest generic point where the cpu is fully realized */
}
@@ -217,6 +222,11 @@ static void cpu_common_unrealizefn(DeviceState *dev)
{
CPUState *cpu = CPU(dev);
+ /* Call the plugin hook before clearing the cpu is fully unrealized */
+ if (tcg_enabled()) {
+ qemu_plugin_vcpu_exit_hook(cpu);
+ }
+
/* NOTE: latest generic point before the cpu is fully unrealized */
cpu_exec_unrealizefn(cpu);
}
--
2.42.0
^ permalink raw reply related [flat|nested] 24+ messages in thread
* [PATCH v11 16/18] plugins: Use different helpers when reading registers
2023-10-12 13:05 [PATCH v11 00/18] plugins: Allow to read registers Akihiko Odaki
` (14 preceding siblings ...)
2023-10-12 13:06 ` [PATCH v11 15/18] cpu: Call plugin hooks only when ready Akihiko Odaki
@ 2023-10-12 13:06 ` Akihiko Odaki
2023-10-12 13:06 ` [PATCH v11 17/18] plugins: Allow to read registers Akihiko Odaki
2023-10-12 13:06 ` [PATCH v11 18/18] contrib/plugins: Allow to log registers Akihiko Odaki
17 siblings, 0 replies; 24+ messages in thread
From: Akihiko Odaki @ 2023-10-12 13:06 UTC (permalink / raw)
Cc: Alex Bennée, Mikhail Tyutin, Aleksandr Anenkov, qemu-devel,
Philippe Mathieu-Daudé, Akihiko Odaki, Richard Henderson,
Paolo Bonzini, Eduardo Habkost, Marcel Apfelbaum, Yanan Wang,
Alexandre Iooss, Mahmoud Mandour
This avoids optimizations incompatible when reading registers.
Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>
---
accel/tcg/plugin-helpers.h | 3 ++-
include/exec/plugin-gen.h | 4 ++--
include/hw/core/cpu.h | 4 ++--
include/qemu/plugin.h | 3 +++
plugins/plugin.h | 5 +++--
accel/tcg/plugin-gen.c | 41 ++++++++++++++++++++++++++++----------
accel/tcg/translator.c | 2 +-
plugins/api.c | 14 +++++++++++--
plugins/core.c | 28 ++++++++++++++++----------
9 files changed, 72 insertions(+), 32 deletions(-)
diff --git a/accel/tcg/plugin-helpers.h b/accel/tcg/plugin-helpers.h
index 8e685e0654..11796436f3 100644
--- a/accel/tcg/plugin-helpers.h
+++ b/accel/tcg/plugin-helpers.h
@@ -1,4 +1,5 @@
#ifdef CONFIG_PLUGIN
-DEF_HELPER_FLAGS_2(plugin_vcpu_udata_cb, TCG_CALL_NO_RWG | TCG_CALL_PLUGIN, void, i32, ptr)
+DEF_HELPER_FLAGS_2(plugin_vcpu_udata_cb_no_wg, TCG_CALL_NO_WG | TCG_CALL_PLUGIN, void, i32, ptr)
+DEF_HELPER_FLAGS_2(plugin_vcpu_udata_cb_no_rwg, TCG_CALL_NO_RWG | TCG_CALL_PLUGIN, void, i32, ptr)
DEF_HELPER_FLAGS_4(plugin_vcpu_mem_cb, TCG_CALL_NO_RWG | TCG_CALL_PLUGIN, void, i32, i32, i64, ptr)
#endif
diff --git a/include/exec/plugin-gen.h b/include/exec/plugin-gen.h
index c4552b5061..b964e1eb5c 100644
--- a/include/exec/plugin-gen.h
+++ b/include/exec/plugin-gen.h
@@ -22,7 +22,7 @@ bool plugin_gen_tb_start(CPUState *cpu, const struct DisasContextBase *db,
bool supress);
void plugin_gen_tb_end(CPUState *cpu, size_t num_insns);
void plugin_gen_insn_start(CPUState *cpu, const struct DisasContextBase *db);
-void plugin_gen_insn_end(void);
+void plugin_gen_insn_end(CPUState *cpu);
void plugin_gen_disable_mem_helpers(void);
void plugin_gen_empty_mem_callback(TCGv_i64 addr, uint32_t info);
@@ -39,7 +39,7 @@ static inline
void plugin_gen_insn_start(CPUState *cpu, const struct DisasContextBase *db)
{ }
-static inline void plugin_gen_insn_end(void)
+static inline void plugin_gen_insn_end(CPUState *cpu)
{ }
static inline void plugin_gen_tb_end(CPUState *cpu, size_t num_insns)
diff --git a/include/hw/core/cpu.h b/include/hw/core/cpu.h
index d2e70643f2..dbdca8b105 100644
--- a/include/hw/core/cpu.h
+++ b/include/hw/core/cpu.h
@@ -437,7 +437,7 @@ struct qemu_work_item;
* @trace_dstate_delayed: Delayed changes to trace_dstate (includes all changes
* to @trace_dstate).
* @trace_dstate: Dynamic tracing state of events for this vCPU (bitmask).
- * @plugin_mask: Plugin event bitmap. Modified only via async work.
+ * @plugin_flags: Plugin flags. Modified only via async work.
* @ignore_memory_transaction_failures: Cached copy of the MachineState
* flag of the same name: allows the board to suppress calling of the
* CPU do_transaction_failed hook function.
@@ -529,7 +529,7 @@ struct CPUState {
/* Use by accel-block: CPU is executing an ioctl() */
QemuLockCnt in_ioctl_lock;
- DECLARE_BITMAP(plugin_mask, QEMU_PLUGIN_EV_MAX);
+ unsigned long plugin_flags;
#ifdef CONFIG_PLUGIN
GArray *plugin_mem_cbs;
diff --git a/include/qemu/plugin.h b/include/qemu/plugin.h
index 7fdc3a4849..a534b9127b 100644
--- a/include/qemu/plugin.h
+++ b/include/qemu/plugin.h
@@ -16,6 +16,9 @@
#include "exec/memopidx.h"
#include "hw/core/cpu.h"
+#define QEMU_PLUGIN_FLAG_TB_CB_READ QEMU_PLUGIN_EV_MAX
+#define QEMU_PLUGIN_FLAG_INSN_CB_READ (QEMU_PLUGIN_EV_MAX + 1)
+
/*
* Option parsing/processing.
* Note that we can load an arbitrary number of plugins.
diff --git a/plugins/plugin.h b/plugins/plugin.h
index 5eb2fdbc85..ba0417194f 100644
--- a/plugins/plugin.h
+++ b/plugins/plugin.h
@@ -16,6 +16,7 @@
#include "qemu/qht.h"
#define QEMU_PLUGIN_MIN_VERSION 0
+#define QEMU_PLUGIN_FLAG_INSIN_CB_READ QEMU_PLUGIN_EV_MAX
/* global state */
struct qemu_plugin_state {
@@ -31,7 +32,7 @@ struct qemu_plugin_state {
* but with the HT we avoid adding a field to CPUState.
*/
GHashTable *cpu_ht;
- DECLARE_BITMAP(mask, QEMU_PLUGIN_EV_MAX);
+ unsigned long flags;
/*
* @lock protects the struct as well as ctx->uninstalling.
* The lock must be acquired by all API ops.
@@ -86,7 +87,7 @@ plugin_register_cb_udata(qemu_plugin_id_t id, enum qemu_plugin_event ev,
void
plugin_register_dyn_cb__udata(GArray **arr,
qemu_plugin_vcpu_udata_cb_t cb,
- enum qemu_plugin_cb_flags flags, void *udata);
+ unsigned int flags, void *udata);
void plugin_register_vcpu_mem_cb(GArray **arr,
diff --git a/accel/tcg/plugin-gen.c b/accel/tcg/plugin-gen.c
index 78b331b251..3bddd4d3c5 100644
--- a/accel/tcg/plugin-gen.c
+++ b/accel/tcg/plugin-gen.c
@@ -90,7 +90,10 @@ enum plugin_gen_cb {
* These helpers are stubs that get dynamically switched out for calls
* direct to the plugin if they are subscribed to.
*/
-void HELPER(plugin_vcpu_udata_cb)(uint32_t cpu_index, void *udata)
+void HELPER(plugin_vcpu_udata_cb_no_wg)(uint32_t cpu_index, void *udata)
+{ }
+
+void HELPER(plugin_vcpu_udata_cb_no_rwg)(uint32_t cpu_index, void *udata)
{ }
void HELPER(plugin_vcpu_mem_cb)(unsigned int vcpu_index,
@@ -98,7 +101,7 @@ void HELPER(plugin_vcpu_mem_cb)(unsigned int vcpu_index,
void *userdata)
{ }
-static void gen_empty_udata_cb(void)
+static void gen_empty_udata_cb(void (*gen_helper)(TCGv_i32, TCGv_ptr))
{
TCGv_i32 cpu_index = tcg_temp_ebb_new_i32();
TCGv_ptr udata = tcg_temp_ebb_new_ptr();
@@ -106,12 +109,22 @@ static void gen_empty_udata_cb(void)
tcg_gen_movi_ptr(udata, 0);
tcg_gen_ld_i32(cpu_index, tcg_env,
-offsetof(ArchCPU, env) + offsetof(CPUState, cpu_index));
- gen_helper_plugin_vcpu_udata_cb(cpu_index, udata);
+ gen_helper(cpu_index, udata);
tcg_temp_free_ptr(udata);
tcg_temp_free_i32(cpu_index);
}
+static void gen_empty_udata_cb_no_wg(void)
+{
+ gen_empty_udata_cb(gen_helper_plugin_vcpu_udata_cb_no_wg);
+}
+
+static void gen_empty_udata_cb_no_rwg(void)
+{
+ gen_empty_udata_cb(gen_helper_plugin_vcpu_udata_cb_no_rwg);
+}
+
/*
* For now we only support addi_i64.
* When we support more ops, we can generate one empty inline cb for each.
@@ -176,7 +189,7 @@ static void gen_wrapped(enum plugin_gen_from from,
tcg_gen_plugin_cb_end();
}
-static void plugin_gen_empty_callback(enum plugin_gen_from from)
+static void plugin_gen_empty_callback(CPUState *cpu, enum plugin_gen_from from)
{
switch (from) {
case PLUGIN_GEN_AFTER_INSN:
@@ -190,9 +203,15 @@ static void plugin_gen_empty_callback(enum plugin_gen_from from)
*/
gen_wrapped(from, PLUGIN_GEN_ENABLE_MEM_HELPER,
gen_empty_mem_helper);
- /* fall through */
+ gen_wrapped(from, PLUGIN_GEN_CB_UDATA,
+ cpu->plugin_flags & BIT(QEMU_PLUGIN_FLAG_INSN_CB_READ) ?
+ gen_empty_udata_cb_no_wg : gen_empty_udata_cb_no_rwg);
+ gen_wrapped(from, PLUGIN_GEN_CB_INLINE, gen_empty_inline_cb);
+ break;
case PLUGIN_GEN_FROM_TB:
- gen_wrapped(from, PLUGIN_GEN_CB_UDATA, gen_empty_udata_cb);
+ gen_wrapped(from, PLUGIN_GEN_CB_UDATA,
+ cpu->plugin_flags & BIT(QEMU_PLUGIN_FLAG_TB_CB_READ) ?
+ gen_empty_udata_cb_no_wg : gen_empty_udata_cb_no_rwg);
gen_wrapped(from, PLUGIN_GEN_CB_INLINE, gen_empty_inline_cb);
break;
default:
@@ -796,7 +815,7 @@ bool plugin_gen_tb_start(CPUState *cpu, const DisasContextBase *db,
{
bool ret = false;
- if (test_bit(QEMU_PLUGIN_EV_VCPU_TB_TRANS, cpu->plugin_mask)) {
+ if (cpu->plugin_flags & BIT(QEMU_PLUGIN_EV_VCPU_TB_TRANS)) {
struct qemu_plugin_tb *ptb = tcg_ctx->plugin_tb;
int i;
@@ -817,7 +836,7 @@ bool plugin_gen_tb_start(CPUState *cpu, const DisasContextBase *db,
ptb->mem_only = mem_only;
ptb->mem_helper = false;
- plugin_gen_empty_callback(PLUGIN_GEN_FROM_TB);
+ plugin_gen_empty_callback(cpu, PLUGIN_GEN_FROM_TB);
}
tcg_ctx->plugin_insn = NULL;
@@ -832,7 +851,7 @@ void plugin_gen_insn_start(CPUState *cpu, const DisasContextBase *db)
pinsn = qemu_plugin_tb_insn_get(ptb, db->pc_next);
tcg_ctx->plugin_insn = pinsn;
- plugin_gen_empty_callback(PLUGIN_GEN_FROM_INSN);
+ plugin_gen_empty_callback(cpu, PLUGIN_GEN_FROM_INSN);
/*
* Detect page crossing to get the new host address.
@@ -852,9 +871,9 @@ void plugin_gen_insn_start(CPUState *cpu, const DisasContextBase *db)
}
}
-void plugin_gen_insn_end(void)
+void plugin_gen_insn_end(CPUState *cpu)
{
- plugin_gen_empty_callback(PLUGIN_GEN_AFTER_INSN);
+ plugin_gen_empty_callback(cpu, PLUGIN_GEN_AFTER_INSN);
}
/*
diff --git a/accel/tcg/translator.c b/accel/tcg/translator.c
index 575b9812ad..bec58dd93f 100644
--- a/accel/tcg/translator.c
+++ b/accel/tcg/translator.c
@@ -189,7 +189,7 @@ void translator_loop(CPUState *cpu, TranslationBlock *tb, int *max_insns,
* to accurately track instrumented helpers that might access memory.
*/
if (plugin_enabled) {
- plugin_gen_insn_end();
+ plugin_gen_insn_end(cpu);
}
/* Stop translation if translate_insn so indicated. */
diff --git a/plugins/api.c b/plugins/api.c
index 5521b0ad36..326e37cb73 100644
--- a/plugins/api.c
+++ b/plugins/api.c
@@ -89,8 +89,13 @@ void qemu_plugin_register_vcpu_tb_exec_cb(struct qemu_plugin_tb *tb,
void *udata)
{
if (!tb->mem_only) {
+ bool read = flags == QEMU_PLUGIN_CB_R_REGS ||
+ flags == QEMU_PLUGIN_CB_RW_REGS;
+
plugin_register_dyn_cb__udata(&tb->cbs[PLUGIN_CB_REGULAR],
- cb, flags, udata);
+ cb,
+ read ? BIT(QEMU_PLUGIN_FLAG_TB_CB_READ) : 0,
+ udata);
}
}
@@ -109,8 +114,13 @@ void qemu_plugin_register_vcpu_insn_exec_cb(struct qemu_plugin_insn *insn,
void *udata)
{
if (!insn->mem_only) {
+ bool read = flags == QEMU_PLUGIN_CB_R_REGS ||
+ flags == QEMU_PLUGIN_CB_RW_REGS;
+
plugin_register_dyn_cb__udata(&insn->cbs[PLUGIN_CB_INSN][PLUGIN_CB_REGULAR],
- cb, flags, udata);
+ cb,
+ read ? BIT(QEMU_PLUGIN_FLAG_INSN_CB_READ) : 0,
+ udata);
}
}
diff --git a/plugins/core.c b/plugins/core.c
index fcd33a2bff..f461e84473 100644
--- a/plugins/core.c
+++ b/plugins/core.c
@@ -55,19 +55,19 @@ struct qemu_plugin_ctx *plugin_id_to_ctx_locked(qemu_plugin_id_t id)
static void plugin_cpu_update__async(CPUState *cpu, run_on_cpu_data data)
{
- bitmap_copy(cpu->plugin_mask, &data.host_ulong, QEMU_PLUGIN_EV_MAX);
+ cpu->plugin_flags = data.host_ulong;
tcg_flush_jmp_cache(cpu);
}
static void plugin_cpu_update__locked(gpointer k, gpointer v, gpointer udata)
{
CPUState *cpu = container_of(k, CPUState, cpu_index);
- run_on_cpu_data mask = RUN_ON_CPU_HOST_ULONG(*plugin.mask);
+ run_on_cpu_data flags = RUN_ON_CPU_HOST_ULONG(plugin.flags);
if (DEVICE(cpu)->realized) {
- async_run_on_cpu(cpu, plugin_cpu_update__async, mask);
+ async_run_on_cpu(cpu, plugin_cpu_update__async, flags);
} else {
- plugin_cpu_update__async(cpu, mask);
+ plugin_cpu_update__async(cpu, flags);
}
}
@@ -83,7 +83,7 @@ void plugin_unregister_cb__locked(struct qemu_plugin_ctx *ctx,
g_free(cb);
ctx->callbacks[ev] = NULL;
if (QLIST_EMPTY_RCU(&plugin.cb_lists[ev])) {
- clear_bit(ev, plugin.mask);
+ plugin.flags &= ~BIT(ev);
g_hash_table_foreach(plugin.cpu_ht, plugin_cpu_update__locked, NULL);
}
}
@@ -186,8 +186,8 @@ do_plugin_register_cb(qemu_plugin_id_t id, enum qemu_plugin_event ev,
cb->udata = udata;
ctx->callbacks[ev] = cb;
QLIST_INSERT_HEAD_RCU(&plugin.cb_lists[ev], cb, entry);
- if (!test_bit(ev, plugin.mask)) {
- set_bit(ev, plugin.mask);
+ if (!(plugin.flags & BIT(ev))) {
+ plugin.flags |= BIT(ev);
g_hash_table_foreach(plugin.cpu_ht, plugin_cpu_update__locked,
NULL);
}
@@ -296,15 +296,20 @@ void plugin_register_inline_op(GArray **arr,
void plugin_register_dyn_cb__udata(GArray **arr,
qemu_plugin_vcpu_udata_cb_t cb,
- enum qemu_plugin_cb_flags flags,
+ unsigned int flags,
void *udata)
{
struct qemu_plugin_dyn_cb *dyn_cb = plugin_get_dyn_cb(arr);
dyn_cb->userp = udata;
- /* Note flags are discarded as unused. */
dyn_cb->f.vcpu_udata = cb;
dyn_cb->type = PLUGIN_CB_REGULAR;
+
+ if (flags) {
+ QEMU_LOCK_GUARD(&plugin.lock);
+ plugin.flags |= flags;
+ g_hash_table_foreach(plugin.cpu_ht, plugin_cpu_update__locked, NULL);
+ }
}
void plugin_register_vcpu_mem_cb(GArray **arr,
@@ -357,7 +362,7 @@ qemu_plugin_vcpu_syscall(CPUState *cpu, int64_t num, uint64_t a1, uint64_t a2,
struct qemu_plugin_cb *cb, *next;
enum qemu_plugin_event ev = QEMU_PLUGIN_EV_VCPU_SYSCALL;
- if (!test_bit(ev, cpu->plugin_mask)) {
+ if (!(cpu->plugin_flags & BIT(ev))) {
return;
}
@@ -379,7 +384,7 @@ void qemu_plugin_vcpu_syscall_ret(CPUState *cpu, int64_t num, int64_t ret)
struct qemu_plugin_cb *cb, *next;
enum qemu_plugin_event ev = QEMU_PLUGIN_EV_VCPU_SYSCALL_RET;
- if (!test_bit(ev, cpu->plugin_mask)) {
+ if (!(cpu->plugin_flags & BIT(ev))) {
return;
}
@@ -428,6 +433,7 @@ void qemu_plugin_flush_cb(void)
{
qht_iter_remove(&plugin.dyn_cb_arr_ht, free_dyn_cb_arr, NULL);
qht_reset(&plugin.dyn_cb_arr_ht);
+ plugin.flags &= ~BIT(QEMU_PLUGIN_FLAG_INSIN_CB_READ);
plugin_cb__simple(QEMU_PLUGIN_EV_FLUSH);
}
--
2.42.0
^ permalink raw reply related [flat|nested] 24+ messages in thread
* [PATCH v11 17/18] plugins: Allow to read registers
2023-10-12 13:05 [PATCH v11 00/18] plugins: Allow to read registers Akihiko Odaki
` (15 preceding siblings ...)
2023-10-12 13:06 ` [PATCH v11 16/18] plugins: Use different helpers when reading registers Akihiko Odaki
@ 2023-10-12 13:06 ` Akihiko Odaki
2023-10-12 13:06 ` [PATCH v11 18/18] contrib/plugins: Allow to log registers Akihiko Odaki
17 siblings, 0 replies; 24+ messages in thread
From: Akihiko Odaki @ 2023-10-12 13:06 UTC (permalink / raw)
Cc: Alex Bennée, Mikhail Tyutin, Aleksandr Anenkov, qemu-devel,
Philippe Mathieu-Daudé, Akihiko Odaki, Alexandre Iooss,
Mahmoud Mandour
It is based on GDB protocol to ensure interface stability.
The timing of the vcpu init hook is also changed so that the hook will
get called after GDB features are initialized.
Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1706
Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>
---
include/qemu/qemu-plugin.h | 50 ++++++++++++++++++++++++++++++++++--
plugins/api.c | 20 +++++++++++++++
plugins/qemu-plugins.symbols | 3 +++
3 files changed, 71 insertions(+), 2 deletions(-)
diff --git a/include/qemu/qemu-plugin.h b/include/qemu/qemu-plugin.h
index 50a9957279..c0ce700258 100644
--- a/include/qemu/qemu-plugin.h
+++ b/include/qemu/qemu-plugin.h
@@ -11,6 +11,7 @@
#ifndef QEMU_QEMU_PLUGIN_H
#define QEMU_QEMU_PLUGIN_H
+#include <glib.h>
#include <inttypes.h>
#include <stdbool.h>
#include <stddef.h>
@@ -218,8 +219,8 @@ struct qemu_plugin_insn;
* @QEMU_PLUGIN_CB_R_REGS: callback reads the CPU's regs
* @QEMU_PLUGIN_CB_RW_REGS: callback reads and writes the CPU's regs
*
- * Note: currently unused, plugins cannot read or change system
- * register state.
+ * Note: currently QEMU_PLUGIN_CB_RW_REGS is unused, plugins cannot change
+ * system register state.
*/
enum qemu_plugin_cb_flags {
QEMU_PLUGIN_CB_NO_REGS,
@@ -664,4 +665,49 @@ uint64_t qemu_plugin_end_code(void);
*/
uint64_t qemu_plugin_entry_code(void);
+/**
+ * qemu_plugin_find_register_file() - find register file
+ *
+ * @vcpu_index: the index of the vcpu context
+ * @name: the name of the register file.
+ *
+ * Returns the identifier of the register file if it was found, and a negative
+ * value otherwise.
+ *
+ * The names of register files are identical with names of GDB's standard
+ * target features with some extensions. For details, see:
+ * https://sourceware.org/gdb/onlinedocs/gdb/Standard-Target-Features.html
+ */
+int qemu_plugin_find_register_file(unsigned int vcpu_index, const char *name);
+
+/**
+ * qemu_plugin_find_register() - find register
+ *
+ * @vcpu_index: the index of the vcpu context
+ * @file: the register file identifier determined with
+ * qemu_plugin_find_register_file().
+ * @name: the name of the register.
+ *
+ * The names of register are identical with names used in GDB's standard
+ * target features with some extensions. For details, see:
+ * https://sourceware.org/gdb/onlinedocs/gdb/Standard-Target-Features.html
+ */
+int qemu_plugin_find_register(unsigned int vcpu_index, int file,
+ const char *name);
+
+/**
+ * qemu_plugin_read_register() - read register
+ *
+ * @buf: the byte array to append the read register content to.
+ * @reg: the register identifier determined with
+ * qemu_plugin_find_register().
+ *
+ * This function is only available in a context that register read access is
+ * explicitly requested.
+ *
+ * Returns the size of the read register. The content of @buf is in target byte
+ * order.
+ */
+int qemu_plugin_read_register(GByteArray *buf, int reg);
+
#endif /* QEMU_QEMU_PLUGIN_H */
diff --git a/plugins/api.c b/plugins/api.c
index 326e37cb73..89144a2dec 100644
--- a/plugins/api.c
+++ b/plugins/api.c
@@ -35,10 +35,12 @@
*/
#include "qemu/osdep.h"
+#include "qemu/main-loop.h"
#include "qemu/plugin.h"
#include "qemu/log.h"
#include "tcg/tcg.h"
#include "exec/exec-all.h"
+#include "exec/gdbstub.h"
#include "exec/ram_addr.h"
#include "disas/disas.h"
#include "plugin.h"
@@ -437,3 +439,21 @@ uint64_t qemu_plugin_entry_code(void)
#endif
return entry;
}
+
+int qemu_plugin_find_register_file(unsigned int vcpu_index, const char *name)
+{
+ QEMU_IOTHREAD_LOCK_GUARD();
+ return gdb_find_feature(qemu_get_cpu(vcpu_index), name);
+}
+
+int qemu_plugin_find_register(unsigned int vcpu_index, int file,
+ const char *name)
+{
+ QEMU_IOTHREAD_LOCK_GUARD();
+ return gdb_find_feature_register(qemu_get_cpu(vcpu_index), file, name);
+}
+
+int qemu_plugin_read_register(GByteArray *buf, int reg)
+{
+ return gdb_read_register(current_cpu, buf, reg);
+}
diff --git a/plugins/qemu-plugins.symbols b/plugins/qemu-plugins.symbols
index 71f6c90549..977f1fcfcb 100644
--- a/plugins/qemu-plugins.symbols
+++ b/plugins/qemu-plugins.symbols
@@ -42,4 +42,7 @@
qemu_plugin_tb_vaddr;
qemu_plugin_uninstall;
qemu_plugin_vcpu_for_each;
+ qemu_plugin_find_register_file;
+ qemu_plugin_find_register;
+ qemu_plugin_read_register;
};
--
2.42.0
^ permalink raw reply related [flat|nested] 24+ messages in thread
* [PATCH v11 18/18] contrib/plugins: Allow to log registers
2023-10-12 13:05 [PATCH v11 00/18] plugins: Allow to read registers Akihiko Odaki
` (16 preceding siblings ...)
2023-10-12 13:06 ` [PATCH v11 17/18] plugins: Allow to read registers Akihiko Odaki
@ 2023-10-12 13:06 ` Akihiko Odaki
17 siblings, 0 replies; 24+ messages in thread
From: Akihiko Odaki @ 2023-10-12 13:06 UTC (permalink / raw)
Cc: Alex Bennée, Mikhail Tyutin, Aleksandr Anenkov, qemu-devel,
Philippe Mathieu-Daudé, Akihiko Odaki, Alexandre Iooss,
Mahmoud Mandour, Richard Henderson, Paolo Bonzini
This demonstrates how a register can be read from a plugin.
Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>
---
docs/devel/tcg-plugins.rst | 10 +++-
contrib/plugins/execlog.c | 120 +++++++++++++++++++++++++++----------
2 files changed, 97 insertions(+), 33 deletions(-)
diff --git a/docs/devel/tcg-plugins.rst b/docs/devel/tcg-plugins.rst
index 81dcd43a61..c9f8b27590 100644
--- a/docs/devel/tcg-plugins.rst
+++ b/docs/devel/tcg-plugins.rst
@@ -497,6 +497,15 @@ arguments if required::
$ qemu-system-arm $(QEMU_ARGS) \
-plugin ./contrib/plugins/libexeclog.so,ifilter=st1w,afilter=0x40001808 -d plugin
+This plugin can also dump a specified register. The specification of register
+follows `GDB standard target features <https://sourceware.org/gdb/onlinedocs/gdb/Standard-Target-Features.html>`__.
+
+Specify the name of the feature that contains the register and the name of the
+register with ``rfile`` and ``reg`` options, respectively::
+
+ $ qemu-system-arm $(QEMU_ARGS) \
+ -plugin ./contrib/plugins/libexeclog.so,rfile=org.gnu.gdb.arm.core,reg=sp -d plugin
+
- contrib/plugins/cache.c
Cache modelling plugin that measures the performance of a given L1 cache
@@ -583,4 +592,3 @@ The following API is generated from the inline documentation in
include the full kernel-doc annotations.
.. kernel-doc:: include/qemu/qemu-plugin.h
-
diff --git a/contrib/plugins/execlog.c b/contrib/plugins/execlog.c
index 82dc2f584e..f3e714c888 100644
--- a/contrib/plugins/execlog.c
+++ b/contrib/plugins/execlog.c
@@ -15,27 +15,43 @@
#include <qemu-plugin.h>
+typedef struct CPU {
+ /* Store last executed instruction on each vCPU as a GString */
+ GString *last_exec;
+ GByteArray *reg_history[2];
+
+ int reg;
+} CPU;
+
QEMU_PLUGIN_EXPORT int qemu_plugin_version = QEMU_PLUGIN_VERSION;
-/* Store last executed instruction on each vCPU as a GString */
-static GPtrArray *last_exec;
+static CPU *cpus;
+static int num_cpus;
static GRWLock expand_array_lock;
static GPtrArray *imatches;
static GArray *amatches;
+static char *rfile_name;
+static char *reg_name;
+
/*
- * Expand last_exec array.
+ * Expand cpu array.
*
* As we could have multiple threads trying to do this we need to
* serialise the expansion under a lock.
*/
-static void expand_last_exec(int cpu_index)
+static void expand_cpu(int cpu_index)
{
g_rw_lock_writer_lock(&expand_array_lock);
- while (cpu_index >= last_exec->len) {
- GString *s = g_string_new(NULL);
- g_ptr_array_add(last_exec, s);
+ if (cpu_index >= num_cpus) {
+ cpus = g_realloc_n(cpus, cpu_index + 1, sizeof(*cpus));
+ while (cpu_index >= num_cpus) {
+ cpus[num_cpus].last_exec = g_string_new(NULL);
+ cpus[num_cpus].reg_history[0] = g_byte_array_new();
+ cpus[num_cpus].reg_history[1] = g_byte_array_new();
+ num_cpus++;
+ }
}
g_rw_lock_writer_unlock(&expand_array_lock);
}
@@ -50,8 +66,8 @@ static void vcpu_mem(unsigned int cpu_index, qemu_plugin_meminfo_t info,
/* Find vCPU in array */
g_rw_lock_reader_lock(&expand_array_lock);
- g_assert(cpu_index < last_exec->len);
- s = g_ptr_array_index(last_exec, cpu_index);
+ g_assert(cpu_index < num_cpus);
+ s = cpus[cpu_index].last_exec;
g_rw_lock_reader_unlock(&expand_array_lock);
/* Indicate type of memory access */
@@ -77,28 +93,42 @@ static void vcpu_mem(unsigned int cpu_index, qemu_plugin_meminfo_t info,
*/
static void vcpu_insn_exec(unsigned int cpu_index, void *udata)
{
- GString *s;
+ int n;
+ int i;
- /* Find or create vCPU in array */
g_rw_lock_reader_lock(&expand_array_lock);
- if (cpu_index >= last_exec->len) {
- g_rw_lock_reader_unlock(&expand_array_lock);
- expand_last_exec(cpu_index);
- g_rw_lock_reader_lock(&expand_array_lock);
- }
- s = g_ptr_array_index(last_exec, cpu_index);
- g_rw_lock_reader_unlock(&expand_array_lock);
/* Print previous instruction in cache */
- if (s->len) {
- qemu_plugin_outs(s->str);
+ if (cpus[cpu_index].last_exec->len) {
+ if (cpus[cpu_index].reg >= 0) {
+ GByteArray *current = cpus[cpu_index].reg_history[0];
+ GByteArray *last = cpus[cpu_index].reg_history[1];
+
+ g_byte_array_set_size(current, 0);
+ n = qemu_plugin_read_register(current, cpus[cpu_index].reg);
+
+ if (n != last->len || memcmp(current->data, last->data, n)) {
+ g_string_append(cpus[cpu_index].last_exec, ", reg,");
+ for (i = 0; i < n; i++) {
+ g_string_append_printf(cpus[cpu_index].last_exec, " %02x",
+ current->data[i]);
+ }
+ }
+
+ cpus[cpu_index].reg_history[0] = last;
+ cpus[cpu_index].reg_history[1] = current;
+ }
+
+ qemu_plugin_outs(cpus[cpu_index].last_exec->str);
qemu_plugin_outs("\n");
}
/* Store new instruction in cache */
/* vcpu_mem will add memory access information to last_exec */
- g_string_printf(s, "%u, ", cpu_index);
- g_string_append(s, (char *)udata);
+ g_string_printf(cpus[cpu_index].last_exec, "%u, ", cpu_index);
+ g_string_append(cpus[cpu_index].last_exec, (char *)udata);
+
+ g_rw_lock_reader_unlock(&expand_array_lock);
}
/**
@@ -167,8 +197,10 @@ static void vcpu_tb_trans(qemu_plugin_id_t id, struct qemu_plugin_tb *tb)
QEMU_PLUGIN_MEM_RW, NULL);
/* Register callback on instruction */
- qemu_plugin_register_vcpu_insn_exec_cb(insn, vcpu_insn_exec,
- QEMU_PLUGIN_CB_NO_REGS, output);
+ qemu_plugin_register_vcpu_insn_exec_cb(
+ insn, vcpu_insn_exec,
+ rfile_name ? QEMU_PLUGIN_CB_R_REGS : QEMU_PLUGIN_CB_NO_REGS,
+ output);
/* reset skip */
skip = (imatches || amatches);
@@ -177,17 +209,33 @@ static void vcpu_tb_trans(qemu_plugin_id_t id, struct qemu_plugin_tb *tb)
}
}
+static void vcpu_init(qemu_plugin_id_t id, unsigned int vcpu_index)
+{
+ int reg = -1;
+
+ expand_cpu(vcpu_index);
+
+ if (rfile_name) {
+ int rfile = qemu_plugin_find_register_file(vcpu_index, rfile_name);
+ if (rfile >= 0) {
+ reg = qemu_plugin_find_register(vcpu_index, rfile, reg_name);
+ }
+ }
+
+ g_rw_lock_writer_lock(&expand_array_lock);
+ cpus[vcpu_index].reg = reg;
+ g_rw_lock_writer_unlock(&expand_array_lock);
+}
+
/**
* On plugin exit, print last instruction in cache
*/
static void plugin_exit(qemu_plugin_id_t id, void *p)
{
guint i;
- GString *s;
- for (i = 0; i < last_exec->len; i++) {
- s = g_ptr_array_index(last_exec, i);
- if (s->str) {
- qemu_plugin_outs(s->str);
+ for (i = 0; i < num_cpus; i++) {
+ if (cpus[i].last_exec->str) {
+ qemu_plugin_outs(cpus[i].last_exec->str);
qemu_plugin_outs("\n");
}
}
@@ -224,9 +272,7 @@ QEMU_PLUGIN_EXPORT int qemu_plugin_install(qemu_plugin_id_t id,
* we don't know the size before emulation.
*/
if (info->system_emulation) {
- last_exec = g_ptr_array_sized_new(info->system.max_vcpus);
- } else {
- last_exec = g_ptr_array_new();
+ cpus = g_new(CPU, info->system.max_vcpus);
}
for (int i = 0; i < argc; i++) {
@@ -236,13 +282,23 @@ QEMU_PLUGIN_EXPORT int qemu_plugin_install(qemu_plugin_id_t id,
parse_insn_match(tokens[1]);
} else if (g_strcmp0(tokens[0], "afilter") == 0) {
parse_vaddr_match(tokens[1]);
+ } else if (g_strcmp0(tokens[0], "rfile") == 0) {
+ rfile_name = g_strdup(tokens[1]);
+ } else if (g_strcmp0(tokens[0], "reg") == 0) {
+ reg_name = g_strdup(tokens[1]);
} else {
fprintf(stderr, "option parsing failed: %s\n", opt);
return -1;
}
}
+ if ((!rfile_name) != (!reg_name)) {
+ fputs("file and reg need to be set at the same time\n", stderr);
+ return -1;
+ }
+
/* Register translation block and exit callbacks */
+ qemu_plugin_register_vcpu_init_cb(id, vcpu_init);
qemu_plugin_register_vcpu_tb_trans_cb(id, vcpu_tb_trans);
qemu_plugin_register_atexit_cb(id, plugin_exit, NULL);
--
2.42.0
^ permalink raw reply related [flat|nested] 24+ messages in thread
* Re: [PATCH v11 09/18] gdbstub: Change gdb_get_reg_cb and gdb_set_reg_cb
2023-10-12 13:06 ` [PATCH v11 09/18] gdbstub: Change gdb_get_reg_cb and gdb_set_reg_cb Akihiko Odaki
@ 2023-10-12 13:40 ` Philippe Mathieu-Daudé
0 siblings, 0 replies; 24+ messages in thread
From: Philippe Mathieu-Daudé @ 2023-10-12 13:40 UTC (permalink / raw)
To: Akihiko Odaki
Cc: Alex Bennée, Mikhail Tyutin, Aleksandr Anenkov, qemu-devel,
Peter Maydell, Brian Cain, Song Gao, Xiaojuan Yang,
Laurent Vivier, Edgar E. Iglesias, Nicholas Piggin,
Daniel Henrique Barboza, Cédric Le Goater, Palmer Dabbelt,
Alistair Francis, Bin Meng, Weiwei Li, Liu Zhiwei, Thomas Huth,
Richard Henderson, David Hildenbrand, Ilya Leoshkevich,
open list:ARM TCG CPUs, open list:PowerPC TCG CPUs,
open list:RISC-V TCG CPUs, open list:S390 general arch...
On 12/10/23 15:06, Akihiko Odaki wrote:
> Align the parameters of gdb_get_reg_cb and gdb_set_reg_cb with the
> gdb_read_register and gdb_write_register members of CPUClass to allow
> to unify the logic to access registers of the core and coprocessors
> in the future.
>
> Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>
> ---
> include/exec/gdbstub.h | 4 +-
> target/arm/internals.h | 12 +++---
> target/hexagon/internal.h | 4 +-
> target/microblaze/cpu.h | 4 +-
> gdbstub/gdbstub.c | 6 +--
> target/arm/gdbstub.c | 51 ++++++++++++++++--------
> target/arm/gdbstub64.c | 27 +++++++++----
> target/hexagon/gdbstub.c | 10 ++++-
> target/loongarch/gdbstub.c | 11 ++++--
> target/m68k/helper.c | 20 ++++++++--
> target/microblaze/gdbstub.c | 9 ++++-
> target/ppc/gdbstub.c | 46 +++++++++++++++++-----
> target/riscv/gdbstub.c | 46 ++++++++++++++++------
> target/s390x/gdbstub.c | 77 ++++++++++++++++++++++++++++---------
> 14 files changed, 236 insertions(+), 91 deletions(-)
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v11 11/18] gdbstub: Infer number of core registers from XML
2023-10-12 13:06 ` [PATCH v11 11/18] gdbstub: Infer number of core registers from XML Akihiko Odaki
@ 2023-10-12 13:43 ` Philippe Mathieu-Daudé
2023-10-12 14:45 ` Akihiko Odaki
0 siblings, 1 reply; 24+ messages in thread
From: Philippe Mathieu-Daudé @ 2023-10-12 13:43 UTC (permalink / raw)
To: Akihiko Odaki
Cc: Alex Bennée, Mikhail Tyutin, Aleksandr Anenkov, qemu-devel,
Eduardo Habkost, Marcel Apfelbaum, Yanan Wang, Peter Maydell,
Michael Rolnik, Brian Cain, Song Gao, Xiaojuan Yang,
Laurent Vivier, Edgar E. Iglesias, Palmer Dabbelt,
Alistair Francis, Bin Meng, Weiwei Li, Daniel Henrique Barboza,
Liu Zhiwei, Yoshinori Sato, Thomas Huth, Richard Henderson,
David Hildenbrand, Ilya Leoshkevich, open list:ARM TCG CPUs,
open list:RISC-V TCG CPUs, open list:S390 general arch...
Hi Akihiko,
On 12/10/23 15:06, Akihiko Odaki wrote:
> GDBFeature has the num_regs member so use it where applicable to
> remove magic numbers.
>
> Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>
> ---
> include/hw/core/cpu.h | 3 ++-
> target/s390x/cpu.h | 2 --
> gdbstub/gdbstub.c | 5 ++++-
> target/arm/cpu.c | 1 -
> target/arm/cpu64.c | 1 -
> target/avr/cpu.c | 1 -
> target/hexagon/cpu.c | 1 -
> target/i386/cpu.c | 2 --
> target/loongarch/cpu.c | 2 --
> target/m68k/cpu.c | 1 -
> target/microblaze/cpu.c | 1 -
> target/riscv/cpu.c | 1 -
> target/rx/cpu.c | 1 -
> target/s390x/cpu.c | 1 -
> 14 files changed, 6 insertions(+), 17 deletions(-)
>
> diff --git a/include/hw/core/cpu.h b/include/hw/core/cpu.h
> index 3968369554..11d4b5cd0c 100644
> --- a/include/hw/core/cpu.h
> +++ b/include/hw/core/cpu.h
> @@ -127,7 +127,8 @@ struct SysemuCPUOps;
> * @gdb_adjust_breakpoint: Callback for adjusting the address of a
> * breakpoint. Used by AVR to handle a gdb mis-feature with
> * its Harvard architecture split code and data.
> - * @gdb_num_core_regs: Number of core registers accessible to GDB.
> + * @gdb_num_core_regs: Number of core registers accessible to GDB or 0 to infer
> + * from @gdb_core_xml_file.
> * @gdb_core_xml_file: File name for core registers GDB XML description.
> * @gdb_stop_before_watchpoint: Indicates whether GDB expects the CPU to stop
> * before the insn which triggers a watchpoint rather than after it.
> diff --git a/target/s390x/cpu.h b/target/s390x/cpu.h
> index 7bea7075e1..83eafbe4b1 100644
> --- a/target/s390x/cpu.h
> +++ b/target/s390x/cpu.h
> @@ -452,8 +452,6 @@ static inline void cpu_get_tb_cpu_state(CPUS390XState *env, vaddr *pc,
> #define S390_R13_REGNUM 15
> #define S390_R14_REGNUM 16
> #define S390_R15_REGNUM 17
> -/* Total Core Registers. */
> -#define S390_NUM_CORE_REGS 18
>
> static inline void setcc(S390CPU *cpu, uint64_t cc)
> {
> diff --git a/gdbstub/gdbstub.c b/gdbstub/gdbstub.c
> index 49fb23a68a..c19f1785e4 100644
> --- a/gdbstub/gdbstub.c
> +++ b/gdbstub/gdbstub.c
> @@ -541,9 +541,12 @@ void gdb_init_cpu(CPUState *cpu)
> gdb_register_feature(cpu, 0,
> cc->gdb_read_register, cc->gdb_write_register,
> feature);
> + cpu->gdb_num_regs = cpu->gdb_num_g_regs = feature->num_regs;
> }
>
> - cpu->gdb_num_regs = cpu->gdb_num_g_regs = cc->gdb_num_core_regs;
> + if (cc->gdb_num_core_regs) {
> + cpu->gdb_num_regs = cpu->gdb_num_g_regs = cc->gdb_num_core_regs;
Why not remove SysemuCPUOps::gdb_num_core_regs entirely?
> + }
> }
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v11 12/18] hw/core/cpu: Remove gdb_get_dynamic_xml member
2023-10-12 13:06 ` [PATCH v11 12/18] hw/core/cpu: Remove gdb_get_dynamic_xml member Akihiko Odaki
@ 2023-10-12 13:45 ` Philippe Mathieu-Daudé
0 siblings, 0 replies; 24+ messages in thread
From: Philippe Mathieu-Daudé @ 2023-10-12 13:45 UTC (permalink / raw)
To: Akihiko Odaki
Cc: Alex Bennée, Mikhail Tyutin, Aleksandr Anenkov, qemu-devel,
Eduardo Habkost, Marcel Apfelbaum, Yanan Wang, Peter Maydell,
Nicholas Piggin, Daniel Henrique Barboza, Cédric Le Goater,
Palmer Dabbelt, Alistair Francis, Bin Meng, Weiwei Li, Liu Zhiwei,
open list:ARM TCG CPUs, open list:PowerPC TCG CPUs,
open list:RISC-V TCG CPUs
On 12/10/23 15:06, Akihiko Odaki wrote:
> This function is no longer used.
>
> Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>
> ---
> include/hw/core/cpu.h | 4 ----
> target/arm/cpu.h | 6 ------
> target/ppc/cpu.h | 1 -
> target/arm/cpu.c | 1 -
> target/arm/gdbstub.c | 18 ------------------
> target/ppc/cpu_init.c | 3 ---
> target/ppc/gdbstub.c | 10 ----------
> target/riscv/cpu.c | 14 --------------
> 8 files changed, 57 deletions(-)
Shouldn't this patch follow "gdbstub: Simplify XML lookup",
before "gdbstub: Infer number of core registers from XML"?
Otherwise,
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v11 11/18] gdbstub: Infer number of core registers from XML
2023-10-12 13:43 ` Philippe Mathieu-Daudé
@ 2023-10-12 14:45 ` Akihiko Odaki
2023-10-12 15:53 ` Philippe Mathieu-Daudé
0 siblings, 1 reply; 24+ messages in thread
From: Akihiko Odaki @ 2023-10-12 14:45 UTC (permalink / raw)
To: Philippe Mathieu-Daudé
Cc: Alex Bennée, Mikhail Tyutin, Aleksandr Anenkov, qemu-devel,
Eduardo Habkost, Marcel Apfelbaum, Yanan Wang, Peter Maydell,
Michael Rolnik, Brian Cain, Song Gao, Xiaojuan Yang,
Laurent Vivier, Edgar E. Iglesias, Palmer Dabbelt,
Alistair Francis, Bin Meng, Weiwei Li, Daniel Henrique Barboza,
Liu Zhiwei, Yoshinori Sato, Thomas Huth, Richard Henderson,
David Hildenbrand, Ilya Leoshkevich, open list:ARM TCG CPUs,
open list:RISC-V TCG CPUs, open list:S390 general arch...
On 2023/10/12 22:43, Philippe Mathieu-Daudé wrote:
> Hi Akihiko,
>
> On 12/10/23 15:06, Akihiko Odaki wrote:
>> GDBFeature has the num_regs member so use it where applicable to
>> remove magic numbers.
>>
>> Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>
>> ---
>> include/hw/core/cpu.h | 3 ++-
>> target/s390x/cpu.h | 2 --
>> gdbstub/gdbstub.c | 5 ++++-
>> target/arm/cpu.c | 1 -
>> target/arm/cpu64.c | 1 -
>> target/avr/cpu.c | 1 -
>> target/hexagon/cpu.c | 1 -
>> target/i386/cpu.c | 2 --
>> target/loongarch/cpu.c | 2 --
>> target/m68k/cpu.c | 1 -
>> target/microblaze/cpu.c | 1 -
>> target/riscv/cpu.c | 1 -
>> target/rx/cpu.c | 1 -
>> target/s390x/cpu.c | 1 -
>> 14 files changed, 6 insertions(+), 17 deletions(-)
>>
>> diff --git a/include/hw/core/cpu.h b/include/hw/core/cpu.h
>> index 3968369554..11d4b5cd0c 100644
>> --- a/include/hw/core/cpu.h
>> +++ b/include/hw/core/cpu.h
>> @@ -127,7 +127,8 @@ struct SysemuCPUOps;
>> * @gdb_adjust_breakpoint: Callback for adjusting the address of a
>> * breakpoint. Used by AVR to handle a gdb mis-feature with
>> * its Harvard architecture split code and data.
>> - * @gdb_num_core_regs: Number of core registers accessible to GDB.
>> + * @gdb_num_core_regs: Number of core registers accessible to GDB or
>> 0 to infer
>> + * from @gdb_core_xml_file.
>> * @gdb_core_xml_file: File name for core registers GDB XML
>> description.
>> * @gdb_stop_before_watchpoint: Indicates whether GDB expects the
>> CPU to stop
>> * before the insn which triggers a watchpoint rather than
>> after it.
>> diff --git a/target/s390x/cpu.h b/target/s390x/cpu.h
>> index 7bea7075e1..83eafbe4b1 100644
>> --- a/target/s390x/cpu.h
>> +++ b/target/s390x/cpu.h
>> @@ -452,8 +452,6 @@ static inline void
>> cpu_get_tb_cpu_state(CPUS390XState *env, vaddr *pc,
>> #define S390_R13_REGNUM 15
>> #define S390_R14_REGNUM 16
>> #define S390_R15_REGNUM 17
>> -/* Total Core Registers. */
>> -#define S390_NUM_CORE_REGS 18
>> static inline void setcc(S390CPU *cpu, uint64_t cc)
>> {
>> diff --git a/gdbstub/gdbstub.c b/gdbstub/gdbstub.c
>> index 49fb23a68a..c19f1785e4 100644
>> --- a/gdbstub/gdbstub.c
>> +++ b/gdbstub/gdbstub.c
>> @@ -541,9 +541,12 @@ void gdb_init_cpu(CPUState *cpu)
>> gdb_register_feature(cpu, 0,
>> cc->gdb_read_register,
>> cc->gdb_write_register,
>> feature);
>> + cpu->gdb_num_regs = cpu->gdb_num_g_regs = feature->num_regs;
>> }
>> - cpu->gdb_num_regs = cpu->gdb_num_g_regs = cc->gdb_num_core_regs;
>> + if (cc->gdb_num_core_regs) {
>> + cpu->gdb_num_regs = cpu->gdb_num_g_regs = cc->gdb_num_core_regs;
>
> Why not remove SysemuCPUOps::gdb_num_core_regs entirely?
It is used by targets without XMLs and ppc, which overrides the number
from XML.
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v11 11/18] gdbstub: Infer number of core registers from XML
2023-10-12 14:45 ` Akihiko Odaki
@ 2023-10-12 15:53 ` Philippe Mathieu-Daudé
0 siblings, 0 replies; 24+ messages in thread
From: Philippe Mathieu-Daudé @ 2023-10-12 15:53 UTC (permalink / raw)
To: Akihiko Odaki
Cc: Alex Bennée, Mikhail Tyutin, Aleksandr Anenkov, qemu-devel,
Eduardo Habkost, Marcel Apfelbaum, Yanan Wang, Peter Maydell,
Michael Rolnik, Brian Cain, Song Gao, Xiaojuan Yang,
Laurent Vivier, Edgar E. Iglesias, Palmer Dabbelt,
Alistair Francis, Bin Meng, Weiwei Li, Daniel Henrique Barboza,
Liu Zhiwei, Yoshinori Sato, Thomas Huth, Richard Henderson,
David Hildenbrand, Ilya Leoshkevich, open list:ARM TCG CPUs,
open list:RISC-V TCG CPUs, open list:S390 general arch...
On 12/10/23 16:45, Akihiko Odaki wrote:
> On 2023/10/12 22:43, Philippe Mathieu-Daudé wrote:
>> Hi Akihiko,
>>
>> On 12/10/23 15:06, Akihiko Odaki wrote:
>>> GDBFeature has the num_regs member so use it where applicable to
>>> remove magic numbers.
>>>
>>> Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>
>>> ---
>>> include/hw/core/cpu.h | 3 ++-
>>> target/s390x/cpu.h | 2 --
>>> gdbstub/gdbstub.c | 5 ++++-
>>> target/arm/cpu.c | 1 -
>>> target/arm/cpu64.c | 1 -
>>> target/avr/cpu.c | 1 -
>>> target/hexagon/cpu.c | 1 -
>>> target/i386/cpu.c | 2 --
>>> target/loongarch/cpu.c | 2 --
>>> target/m68k/cpu.c | 1 -
>>> target/microblaze/cpu.c | 1 -
>>> target/riscv/cpu.c | 1 -
>>> target/rx/cpu.c | 1 -
>>> target/s390x/cpu.c | 1 -
>>> 14 files changed, 6 insertions(+), 17 deletions(-)
>>>
>>> diff --git a/include/hw/core/cpu.h b/include/hw/core/cpu.h
>>> index 3968369554..11d4b5cd0c 100644
>>> --- a/include/hw/core/cpu.h
>>> +++ b/include/hw/core/cpu.h
>>> @@ -127,7 +127,8 @@ struct SysemuCPUOps;
>>> * @gdb_adjust_breakpoint: Callback for adjusting the address of a
>>> * breakpoint. Used by AVR to handle a gdb mis-feature with
>>> * its Harvard architecture split code and data.
>>> - * @gdb_num_core_regs: Number of core registers accessible to GDB.
>>> + * @gdb_num_core_regs: Number of core registers accessible to GDB or
>>> 0 to infer
>>> + * from @gdb_core_xml_file.
>>> * @gdb_core_xml_file: File name for core registers GDB XML
>>> description.
>>> * @gdb_stop_before_watchpoint: Indicates whether GDB expects the
>>> CPU to stop
>>> * before the insn which triggers a watchpoint rather
>>> than after it.
>> Why not remove SysemuCPUOps::gdb_num_core_regs entirely?
>
> It is used by targets without XMLs and ppc, which overrides the number
> from XML.
Right.
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
^ permalink raw reply [flat|nested] 24+ messages in thread
end of thread, other threads:[~2023-10-12 15:54 UTC | newest]
Thread overview: 24+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-10-12 13:05 [PATCH v11 00/18] plugins: Allow to read registers Akihiko Odaki
2023-10-12 13:05 ` [PATCH v11 01/18] gdbstub: Add num_regs member to GDBFeature Akihiko Odaki
2023-10-12 13:05 ` [PATCH v11 02/18] gdbstub: Introduce gdb_find_static_feature() Akihiko Odaki
2023-10-12 13:05 ` [PATCH v11 03/18] gdbstub: Introduce GDBFeatureBuilder Akihiko Odaki
2023-10-12 13:05 ` [PATCH v11 04/18] target/arm: Use GDBFeature for dynamic XML Akihiko Odaki
2023-10-12 13:05 ` [PATCH v11 05/18] target/ppc: " Akihiko Odaki
2023-10-12 13:06 ` [PATCH v11 06/18] target/riscv: " Akihiko Odaki
2023-10-12 13:06 ` [PATCH v11 07/18] gdbstub: Use GDBFeature for gdb_register_coprocessor Akihiko Odaki
2023-10-12 13:06 ` [PATCH v11 08/18] gdbstub: Use GDBFeature for GDBRegisterState Akihiko Odaki
2023-10-12 13:06 ` [PATCH v11 09/18] gdbstub: Change gdb_get_reg_cb and gdb_set_reg_cb Akihiko Odaki
2023-10-12 13:40 ` Philippe Mathieu-Daudé
2023-10-12 13:06 ` [PATCH v11 10/18] gdbstub: Simplify XML lookup Akihiko Odaki
2023-10-12 13:06 ` [PATCH v11 11/18] gdbstub: Infer number of core registers from XML Akihiko Odaki
2023-10-12 13:43 ` Philippe Mathieu-Daudé
2023-10-12 14:45 ` Akihiko Odaki
2023-10-12 15:53 ` Philippe Mathieu-Daudé
2023-10-12 13:06 ` [PATCH v11 12/18] hw/core/cpu: Remove gdb_get_dynamic_xml member Akihiko Odaki
2023-10-12 13:45 ` Philippe Mathieu-Daudé
2023-10-12 13:06 ` [PATCH v11 13/18] gdbstub: Add members to identify registers to GDBFeature Akihiko Odaki
2023-10-12 13:06 ` [PATCH v11 14/18] gdbstub: Expose functions to read registers Akihiko Odaki
2023-10-12 13:06 ` [PATCH v11 15/18] cpu: Call plugin hooks only when ready Akihiko Odaki
2023-10-12 13:06 ` [PATCH v11 16/18] plugins: Use different helpers when reading registers Akihiko Odaki
2023-10-12 13:06 ` [PATCH v11 17/18] plugins: Allow to read registers Akihiko Odaki
2023-10-12 13:06 ` [PATCH v11 18/18] contrib/plugins: Allow to log registers Akihiko Odaki
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).