| Message ID | 20240116104809.250076-17-alex.bennee@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | [PULL,01/22] hw/riscv: Use misa_mxl instead of misa_mxl_max | expand |
On 2024/01/16 19:48, Alex Bennée wrote: > Expose an internal API to QEMU to return all the registers for a vCPU. > The list containing the details required to called gdb_read_register(). > > Based-on: <20231025093128.33116-15-akihiko.odaki@daynix.com> > Cc: Akihiko Odaki <akihiko.odaki@daynix.com> > Message-Id: <20240103173349.398526-38-alex.bennee@linaro.org> > Signed-off-by: Alex Bennée <alex.bennee@linaro.org> I'm not for to pull this, "[PULL 17/22] plugins: add an API to read registers", and "[PULL 19/22] contrib/plugins: extend execlog to track register changes" in the current state. I have only commented the API aspect of these patches, but looking at them now, I see something wrong with their implementations. I'll add comments to respective patches. > > diff --git a/include/exec/gdbstub.h b/include/exec/gdbstub.h > index da9ddfe54c5..7bddea8259e 100644 > --- a/include/exec/gdbstub.h > +++ b/include/exec/gdbstub.h > @@ -111,6 +111,53 @@ void gdb_feature_builder_end(const GDBFeatureBuilder *builder); > */ > const GDBFeature *gdb_find_static_feature(const char *xmlname); > > +/** > + * gdb_find_feature() - Find a feature associated with a CPU. > + * @cpu: The CPU associated with the feature. > + * @name: The feature's name. > + * > + * Return: The feature's number. > + */ > +int gdb_find_feature(CPUState *cpu, const char *name); This function is not used. > + > +/** > + * gdb_find_feature_register() - Find a register associated with a CPU. > + * @cpu: The CPU associated with the register. > + * @feature: The feature's number returned by gdb_find_feature(). > + * @name: The register's name. > + * > + * Return: The register's number. > + */ > +int gdb_find_feature_register(CPUState *cpu, int feature, const char *name); This function is also no longer needed. Regards, Akihiko Odaki
Akihiko Odaki <akihiko.odaki@daynix.com> writes: > On 2024/01/16 19:48, Alex Bennée wrote: >> Expose an internal API to QEMU to return all the registers for a vCPU. >> The list containing the details required to called gdb_read_register(). >> Based-on: <20231025093128.33116-15-akihiko.odaki@daynix.com> >> Cc: Akihiko Odaki <akihiko.odaki@daynix.com> >> Message-Id: <20240103173349.398526-38-alex.bennee@linaro.org> >> Signed-off-by: Alex Bennée <alex.bennee@linaro.org> > > I'm not for to pull this, "[PULL 17/22] plugins: add an API to read > registers", and "[PULL 19/22] contrib/plugins: extend execlog to track > register changes" in the current state. I have only commented the API > aspect of these patches, but looking at them now, I see something > wrong with their implementations. I'll add comments to respective > patches. OK - the patches where on the list for 14 days so I was going to merge as is and fix up any issues after the fact. But if there is review incoming I can re-spin without the final series. There is other plugin activity going on I'd like to get merged this cycle so I don't want to hold stuff up. > >> diff --git a/include/exec/gdbstub.h b/include/exec/gdbstub.h >> index da9ddfe54c5..7bddea8259e 100644 >> --- a/include/exec/gdbstub.h >> +++ b/include/exec/gdbstub.h >> @@ -111,6 +111,53 @@ void gdb_feature_builder_end(const GDBFeatureBuilder *builder); >> */ >> const GDBFeature *gdb_find_static_feature(const char *xmlname); >> +/** >> + * gdb_find_feature() - Find a feature associated with a CPU. >> + * @cpu: The CPU associated with the feature. >> + * @name: The feature's name. >> + * >> + * Return: The feature's number. >> + */ >> +int gdb_find_feature(CPUState *cpu, const char *name); > > This function is not used. > >> + >> +/** >> + * gdb_find_feature_register() - Find a register associated with a CPU. >> + * @cpu: The CPU associated with the register. >> + * @feature: The feature's number returned by gdb_find_feature(). >> + * @name: The register's name. >> + * >> + * Return: The register's number. >> + */ >> +int gdb_find_feature_register(CPUState *cpu, int feature, const char *name); > > This function is also no longer needed. Ahh looks like a merge failure when I re-based.
diff --git a/include/exec/gdbstub.h b/include/exec/gdbstub.h index da9ddfe54c5..7bddea8259e 100644 --- a/include/exec/gdbstub.h +++ b/include/exec/gdbstub.h @@ -111,6 +111,53 @@ void gdb_feature_builder_end(const GDBFeatureBuilder *builder); */ const GDBFeature *gdb_find_static_feature(const char *xmlname); +/** + * gdb_find_feature() - Find a feature associated with a CPU. + * @cpu: The CPU associated with the feature. + * @name: The feature's name. + * + * Return: The feature's number. + */ +int gdb_find_feature(CPUState *cpu, const char *name); + +/** + * gdb_find_feature_register() - Find a register associated with a CPU. + * @cpu: The CPU associated with the register. + * @feature: The feature's number returned by gdb_find_feature(). + * @name: The register's name. + * + * Return: The register's number. + */ +int gdb_find_feature_register(CPUState *cpu, int feature, const char *name); + +/** + * gdb_read_register() - Read a register associated with a CPU. + * @cpu: The CPU associated with the register. + * @buf: The buffer that the read register will be appended to. + * @reg: The register's number returned by gdb_find_feature_register(). + * + * Return: The number of read bytes. + */ +int gdb_read_register(CPUState *cpu, GByteArray *buf, int reg); + +/** + * typedef GDBRegDesc - a register description from gdbstub + */ +typedef struct { + int gdb_reg; + const char *name; + const char *feature_name; +} GDBRegDesc; + +/** + * gdb_get_register_list() - Return list of all registers for CPU + * @cpu: The CPU being searched + * + * Returns a GArray of GDBRegDesc, caller frees array but not the + * const strings. + */ +GArray *gdb_get_register_list(CPUState *cpu); + 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 420ab2a3766..b0230138246 100644 --- a/gdbstub/gdbstub.c +++ b/gdbstub/gdbstub.c @@ -490,7 +490,61 @@ 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; +} + +GArray *gdb_get_register_list(CPUState *cpu) +{ + GArray *results = g_array_new(true, true, sizeof(GDBRegDesc)); + + /* registers are only available once the CPU is initialised */ + if (!cpu->gdb_regs) { + return results; + } + + for (int f = 0; f < cpu->gdb_regs->len; f++) { + GDBRegisterState *r = &g_array_index(cpu->gdb_regs, GDBRegisterState, f); + for (int i = 0; i < r->feature->num_regs; i++) { + const char *name = r->feature->regs[i]; + GDBRegDesc desc = { + r->base_reg + i, + name, + r->feature->name + }; + g_array_append_val(results, desc); + } + } + + return results; +} + +int gdb_read_register(CPUState *cpu, GByteArray *buf, int reg) { CPUClass *cc = CPU_GET_CLASS(cpu); GDBRegisterState *r;
Expose an internal API to QEMU to return all the registers for a vCPU. The list containing the details required to called gdb_read_register(). Based-on: <20231025093128.33116-15-akihiko.odaki@daynix.com> Cc: Akihiko Odaki <akihiko.odaki@daynix.com> Message-Id: <20240103173349.398526-38-alex.bennee@linaro.org> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>