[PATCH] target/arm/cpu64: Use 32-bit GDBstub when running in 32-bit KVM mode

[PATCH] target/arm/cpu64: Use 32-bit GDBstub when running in 32-bit KVM mode

[Ard Biesheuvel]

When running under KVM, we may decide to run the CPU in 32-bit mode, by
setting the 'aarch64=off' CPU option. In this case, we need to switch to
the 32-bit version of the GDB stub too, so that GDB has the correct view
of the CPU state. Without this, GDB debugging does not work at all, and
errors out upon connecting to the target with a mysterious 'g' packet
length error.

Cc: Richard Henderson <richard.henderson@linaro.org>
Cc: Peter Maydell <peter.maydell@linaro.org>
Cc: Alex Bennee <alex.bennee@linaro.org>
Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
---
 target/arm/cpu64.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/target/arm/cpu64.c b/target/arm/cpu64.c
index 15245a60a8c7..3dede9e2ec31 100644
--- a/target/arm/cpu64.c
+++ b/target/arm/cpu64.c
@@ -903,9 +903,15 @@ static bool aarch64_cpu_get_aarch64(Object *obj, Error **errp)
     return arm_feature(&cpu->env, ARM_FEATURE_AARCH64);
 }
 
+static gchar *arm_gdb_arch_name(CPUState *cs)
+{
+    return g_strdup("arm");
+}
+
 static void aarch64_cpu_set_aarch64(Object *obj, bool value, Error **errp)
 {
     ARMCPU *cpu = ARM_CPU(obj);
+    CPUClass *cc = CPU_GET_CLASS(obj);
 
     /* At this time, this property is only allowed if KVM is enabled.  This
      * restriction allows us to avoid fixing up functionality that assumes a
@@ -919,6 +925,12 @@ static void aarch64_cpu_set_aarch64(Object *obj, bool value, Error **errp)
             return;
         }
         unset_feature(&cpu->env, ARM_FEATURE_AARCH64);
+
+        cc->gdb_read_register = arm_cpu_gdb_read_register;
+        cc->gdb_write_register = arm_cpu_gdb_write_register;
+        cc->gdb_num_core_regs = 26;
+        cc->gdb_core_xml_file = "arm-core.xml";
+        cc->gdb_arch_name = arm_gdb_arch_name;
     } else {
         set_feature(&cpu->env, ARM_FEATURE_AARCH64);
     }
-- 
2.30.2

Re: [PATCH] target/arm/cpu64: Use 32-bit GDBstub when running in 32-bit KVM mode

[Richard Henderson]

On 1/7/22 7:54 AM, Ard Biesheuvel wrote:
> diff --git a/target/arm/cpu64.c b/target/arm/cpu64.c
> index 15245a60a8c7..3dede9e2ec31 100644
> --- a/target/arm/cpu64.c
> +++ b/target/arm/cpu64.c
> @@ -903,9 +903,15 @@ static bool aarch64_cpu_get_aarch64(Object *obj, Error **errp)
>       return arm_feature(&cpu->env, ARM_FEATURE_AARCH64);
>   }
>   
> +static gchar *arm_gdb_arch_name(CPUState *cs)
> +{
> +    return g_strdup("arm");
> +}
> +
>   static void aarch64_cpu_set_aarch64(Object *obj, bool value, Error **errp)
>   {
>       ARMCPU *cpu = ARM_CPU(obj);
> +    CPUClass *cc = CPU_GET_CLASS(obj);
>   
>       /* At this time, this property is only allowed if KVM is enabled.  This
>        * restriction allows us to avoid fixing up functionality that assumes a
> @@ -919,6 +925,12 @@ static void aarch64_cpu_set_aarch64(Object *obj, bool value, Error **errp)
>               return;
>           }
>           unset_feature(&cpu->env, ARM_FEATURE_AARCH64);
> +
> +        cc->gdb_read_register = arm_cpu_gdb_read_register;
> +        cc->gdb_write_register = arm_cpu_gdb_write_register;
> +        cc->gdb_num_core_regs = 26;
> +        cc->gdb_core_xml_file = "arm-core.xml";
> +        cc->gdb_arch_name = arm_gdb_arch_name;

It would be nice to split out a function in cpu.c for this,
perhaps arm_cpu_class_gdb_init().

Otherwise this looks like the correct approach.


r~