plugin updates (register access) for 9.0 (pre-PR?)

[PATCH v3 16/21] gdbstub: expose api to find registers

Posted by Alex Bennée 1 year, 11 months ago

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>

---
v3
  - rm unused api functions left over
---
 include/exec/gdbstub.h | 28 ++++++++++++++++++++++++++++
 gdbstub/gdbstub.c      | 27 ++++++++++++++++++++++++++-
 2 files changed, 54 insertions(+), 1 deletion(-)

diff --git a/include/exec/gdbstub.h b/include/exec/gdbstub.h
index da9ddfe54c5..eb14b91139b 100644
--- a/include/exec/gdbstub.h
+++ b/include/exec/gdbstub.h
@@ -111,6 +111,34 @@ void gdb_feature_builder_end(const GDBFeatureBuilder *builder);
  */
 const GDBFeature *gdb_find_static_feature(const char *xmlname);
 
+/**
+ * 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..14f2f32e63f 100644
--- a/gdbstub/gdbstub.c
+++ b/gdbstub/gdbstub.c
@@ -490,7 +490,32 @@ const GDBFeature *gdb_find_static_feature(const char *xmlname)
     g_assert_not_reached();
 }
 
-static int gdb_read_register(CPUState *cpu, GByteArray *buf, int reg)
+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;
-- 
2.39.2

Re: [PATCH v3 16/21] gdbstub: expose api to find registers

Posted by Akihiko Odaki 1 year, 10 months ago

On 2024/01/22 23:56, 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>
> 
> ---
> v3
>    - rm unused api functions left over
> ---
>   include/exec/gdbstub.h | 28 ++++++++++++++++++++++++++++
>   gdbstub/gdbstub.c      | 27 ++++++++++++++++++++++++++-
>   2 files changed, 54 insertions(+), 1 deletion(-)
> 
> diff --git a/include/exec/gdbstub.h b/include/exec/gdbstub.h
> index da9ddfe54c5..eb14b91139b 100644
> --- a/include/exec/gdbstub.h
> +++ b/include/exec/gdbstub.h
> @@ -111,6 +111,34 @@ void gdb_feature_builder_end(const GDBFeatureBuilder *builder);
>    */
>   const GDBFeature *gdb_find_static_feature(const char *xmlname);
>   
> +/**
> + * 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 {

nit: Add struct name; docs/devel/style.rst says struct has a CamelCase 
name *and* corresponding typedef, though this rule is apparently not 
strictly enforced.

Re: [PATCH v3 16/21] gdbstub: expose api to find registers

Posted by Alex Bennée 1 year, 10 months ago

Akihiko Odaki <akihiko.odaki@daynix.com> writes:

> On 2024/01/22 23:56, 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>
>> ---
>> v3
>>    - rm unused api functions left over
>> ---
>>   include/exec/gdbstub.h | 28 ++++++++++++++++++++++++++++
>>   gdbstub/gdbstub.c      | 27 ++++++++++++++++++++++++++-
>>   2 files changed, 54 insertions(+), 1 deletion(-)
>> diff --git a/include/exec/gdbstub.h b/include/exec/gdbstub.h
>> index da9ddfe54c5..eb14b91139b 100644
>> --- a/include/exec/gdbstub.h
>> +++ b/include/exec/gdbstub.h
>> @@ -111,6 +111,34 @@ void gdb_feature_builder_end(const GDBFeatureBuilder *builder);
>>    */
>>   const GDBFeature *gdb_find_static_feature(const char *xmlname);
>>   +/**
>> + * 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 {
>
> nit: Add struct name; docs/devel/style.rst says struct has a CamelCase
> name *and* corresponding typedef, though this rule is apparently not
> strictly enforced.

I think the wording is a little ambiguous here, especially with the
reference to typedefs.h where the anonymous structure typedefs are held.
In this case we don't need the structname because there is no internal
reference to itself.

-- 
Alex Bennée
Virtualisation Tech Lead @ Linaro

Re: [PATCH v3 16/21] gdbstub: expose api to find registers

Posted by Akihiko Odaki 1 year, 10 months ago

On 2024/02/03 20:44, Alex Bennée wrote:
> Akihiko Odaki <akihiko.odaki@daynix.com> writes:
> 
>> On 2024/01/22 23:56, 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>
>>> ---
>>> v3
>>>     - rm unused api functions left over
>>> ---
>>>    include/exec/gdbstub.h | 28 ++++++++++++++++++++++++++++
>>>    gdbstub/gdbstub.c      | 27 ++++++++++++++++++++++++++-
>>>    2 files changed, 54 insertions(+), 1 deletion(-)
>>> diff --git a/include/exec/gdbstub.h b/include/exec/gdbstub.h
>>> index da9ddfe54c5..eb14b91139b 100644
>>> --- a/include/exec/gdbstub.h
>>> +++ b/include/exec/gdbstub.h
>>> @@ -111,6 +111,34 @@ void gdb_feature_builder_end(const GDBFeatureBuilder *builder);
>>>     */
>>>    const GDBFeature *gdb_find_static_feature(const char *xmlname);
>>>    +/**
>>> + * 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 {
>>
>> nit: Add struct name; docs/devel/style.rst says struct has a CamelCase
>> name *and* corresponding typedef, though this rule is apparently not
>> strictly enforced.
> 
> I think the wording is a little ambiguous here, especially with the
> reference to typedefs.h where the anonymous structure typedefs are held.
> In this case we don't need the structname because there is no internal
> reference to itself.
> 

The majority of structure typedefs have tag names though; grep "typedef 
struct".

That said, I'm fine even without a tag name. As I mentioned earlier, 
this convention in QEMU is not consistent, and I have no other reason to 
have a tag name.