Hi Peter,

On 17/3/25 15:28, Peter Maydell wrote:
Currently we provide an AArch64 gdbstub for CPUs which are
TYPE_AARCH64_CPU, and an AArch32 gdbstub for those which are only
TYPE_ARM_CPU.  This mostly does the right thing, except in the
corner case of KVM with -cpu host,aarch64=off.  That produces a CPU
which is TYPE_AARCH64_CPU but which has ARM_FEATURE_AARCH64 removed
and which to the guest is in AArch32 mode.

Now we have moved all the handling of AArch64-vs-AArch32 gdbstub
behaviour into TYPE_ARM_CPU we can change the condition we use for
whether to select the AArch64 gdbstub to look at ARM_FEATURE_AARCH64.
This will mean that we now correctly provide an AArch32 gdbstub for
aarch64=off CPUs.

Signed-off-by: Peter Maydell <peter.mayd...@linaro.org>
---
  target/arm/internals.h | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/target/arm/internals.h b/target/arm/internals.h
index a14c269fa5a..a18d87fa28b 100644
--- a/target/arm/internals.h
+++ b/target/arm/internals.h
@@ -1694,7 +1694,7 @@ void aarch64_add_sme_properties(Object *obj);
  /* Return true if the gdbstub is presenting an AArch64 CPU */
  static inline bool arm_gdbstub_is_aarch64(ARMCPU *cpu)
  {
-    return object_dynamic_cast(OBJECT(cpu), TYPE_AARCH64_CPU);
+    return arm_feature(&cpu->env, ARM_FEATURE_AARCH64);

Unfortunately this doesn't work well: while a Aarch64 CPU is of type
TYPE_AARCH64_CPU right after being instantiated (not yet QOM realized),
the features are only finalized during arm_cpu_instance_init():

static void arm_cpu_instance_init(Object *obj)
{
    ARMCPUClass *acc = ARM_CPU_GET_CLASS(obj);

    acc->info->initfn(obj);
    arm_cpu_post_init(obj);
}

void arm_cpu_post_init(Object *obj)
{
    ARMCPU *cpu = ARM_CPU(obj);

    /*
     * Some features imply others. Figure this out now, because we
     * are going to look at the feature bits in deciding which
     * properties to add.
     */
    arm_cpu_propagate_feature_implications(cpu);
    ...
}

The GDB feature checks are done earlier:

  object_init_with_type
   -> cpu_common_initfn
       -> gdb_init_cpu
          -> gdb_get_core_xml_file
             -> arm_gdb_get_core_xml_file
                -> arm_gdbstub_is_aarch64

At this point the feature set is empty, triggering the
assertion in gdb_find_static_feature():

$ ./build/qemu-aarch64 build/tests/tcg/aarch64-linux-user/semihosting
**
ERROR:../../gdbstub/gdbstub.c:494:gdb_find_static_feature: code should not be reached Bail out! ERROR:../../gdbstub/gdbstub.c:494:gdb_find_static_feature: code should not be reached
Aborted (core dumped)

I suppose gdb_init_cpu() needs more splitting work. For now I'll drop
this patches 5+ from my queue.

Regards,

Phil.

Reply via email to