QEMU-Arm Archive on lore.kernel.org
 help / color / mirror / Atom feed
From: "Alex Bennée" <alex.bennee@linaro.org>
To: "Marc-André Lureau" <marcandre.lureau@redhat.com>
Cc: qemu-devel@nongnu.org,
	"Dr. David Alan Gilbert" <dave@treblig.org>,
	"Philippe Mathieu-Daudé" <philmd@oss.qualcomm.com>,
	"Zhao Liu" <zhao1.liu@intel.com>,
	"Peter Maydell" <peter.maydell@linaro.org>,
	"Chinmay Rath" <rathc@linux.ibm.com>,
	"Nicholas Piggin" <npiggin@gmail.com>,
	"Glenn Miles" <milesg@linux.ibm.com>,
	"Harsh Prateek Bora" <harshpb@linux.ibm.com>,
	"Palmer Dabbelt" <palmer@dabbelt.com>,
	"Alistair Francis" <alistair.francis@wdc.com>,
	"Weiwei Li" <liwei1518@gmail.com>,
	"Daniel Henrique Barboza" <daniel.barboza@oss.qualcomm.com>,
	"Liu Zhiwei" <zhiwei_liu@linux.alibaba.com>,
	"Chao Liu" <chao.liu@processmission.com>,
	"Max Filippov" <jcmvbkbc@gmail.com>,
	qemu-arm@nongnu.org, qemu-ppc@nongnu.org, qemu-riscv@nongnu.org,
	"Paolo Bonzini" <pbonzini@redhat.com>,
	"Laurent Vivier" <laurent@vivier.eu>,
	"Mark Cave-Ayland" <mark.cave-ayland@ilande.co.uk>,
	"Artyom Tarasenko" <atar4qemu@gmail.com>,
	"Helge Deller" <deller@gmx.de>,
	"Pierrick Bouvier" <pierrick.bouvier@oss.qualcomm.com>,
	"Warner Losh" <imp@bsdimp.com>, "Kyle Evans" <kevans@freebsd.org>
Subject: Re: [PATCH 02/10] gdbstub: fix next register base after register gaps
Date: Fri, 11 Sep 2026 16:29:00 +0100	[thread overview]
Message-ID: <87fqzf958j.fsf@draig.linaro.org> (raw)
In-Reply-To: <20260911-query-reg-v1-2-0c04f0c02cf3@redhat.com> ("Marc-André Lureau"'s message of "Fri, 11 Sep 2026 16:27:25 +0400")

Marc-André Lureau <marcandre.lureau@redhat.com> writes:

> Commit b3e88abb200c ("gdbstub: Consider GDBFeature::base_reg in
> gdb_register_coprocessor()") identified that register numbers may
> have gaps. We also need to adjust the next base_reg correctly
> to avoid overlapping registers. Rename the field to be more
> explicit about its usage.
>
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>

Acked-by: Alex Bennée <alex.bennee@linaro.org>

> ---
>  gdbstub/gdbstub.c      | 13 ++++++-------
>  include/hw/core/cpu.h  |  2 +-
>  target/arm/gdbstub.c   |  8 +++++---
>  target/arm/gdbstub64.c | 10 ++++++----
>  target/ppc/gdbstub.c   |  2 +-
>  target/riscv/gdbstub.c |  9 ++++++---
>  target/xtensa/cpu.c    |  2 +-
>  7 files changed, 26 insertions(+), 20 deletions(-)
>
> diff --git a/gdbstub/gdbstub.c b/gdbstub/gdbstub.c
> index 0a328b0dd488..0ce871d74929 100644
> --- a/gdbstub/gdbstub.c
> +++ b/gdbstub/gdbstub.c
> @@ -605,13 +605,13 @@ 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_next_base_reg = cpu->gdb_num_g_regs = feature->num_regs;
>      } else {
> -        cpu->gdb_num_regs = cpu->gdb_num_g_regs = cc->gdb_num_core_regs;
> +        cpu->gdb_next_base_reg = cpu->gdb_num_g_regs = cc->gdb_num_core_regs;
>      }
>  
>      trace_gdbxml_init_cpu(object_get_typename(OBJECT(cpu)), cpu->cpu_index,
> -                          cpu->gdb_num_regs, cpu->gdb_num_g_regs,
> +                          cpu->gdb_next_base_reg, cpu->gdb_num_g_regs,
>                            cc->gdb_num_core_regs);
>  }
>  
> @@ -621,7 +621,7 @@ void gdb_register_coprocessor(CPUState *cpu,
>  {
>      GDBRegisterState *s;
>      guint i;
> -    int base_reg = cpu->gdb_num_regs;
> +    int base_reg = cpu->gdb_next_base_reg;
>  
>      for (i = 0; i < cpu->gdb_regs->len; i++) {
>          /* Check for duplicates.  */
> @@ -639,7 +639,7 @@ void gdb_register_coprocessor(CPUState *cpu,
>      gdb_register_feature(cpu, base_reg, get_reg, set_reg, feature);
>  
>      /* Add to end of list.  */
> -    cpu->gdb_num_regs += feature->num_regs;
> +    cpu->gdb_next_base_reg = base_reg + feature->num_regs;
>  }
>  
>  void gdb_unregister_coprocessor_all(CPUState *cpu)
> @@ -651,7 +651,7 @@ void gdb_unregister_coprocessor_all(CPUState *cpu)
>      g_array_free(cpu->gdb_regs, true);
>  
>      cpu->gdb_regs = NULL;
> -    cpu->gdb_num_regs = 0;
> +    cpu->gdb_next_base_reg = 0;
>      cpu->gdb_num_g_regs = 0;
>  }
>  
> @@ -2522,4 +2522,3 @@ void gdb_create_default_process(GDBState *s)
>      process->attached = false;
>      process->target_xml = NULL;
>  }
> -
> diff --git a/include/hw/core/cpu.h b/include/hw/core/cpu.h
> index 328e30045ce1..02212f555c2e 100644
> --- a/include/hw/core/cpu.h
> +++ b/include/hw/core/cpu.h
> @@ -521,7 +521,7 @@ struct CPUState {
>      struct CPUJumpCache *tb_jmp_cache;
>  
>      GArray *gdb_regs;
> -    int gdb_num_regs;
> +    int gdb_next_base_reg;
>      int gdb_num_g_regs;
>      QTAILQ_ENTRY(CPUState) node;
>  
> diff --git a/target/arm/gdbstub.c b/target/arm/gdbstub.c
> index d6e29c4cf467..e5cbe4d951ef 100644
> --- a/target/arm/gdbstub.c
> +++ b/target/arm/gdbstub.c
> @@ -555,19 +555,21 @@ void arm_cpu_register_gdb_regs_for_features(ARMCPU *cpu)
>          gdb_register_coprocessor(cs, mve_gdb_get_reg, mve_gdb_set_reg,
>                                   gdb_find_static_feature("arm-m-profile-mve.xml"));
>      }
> +    GDBFeature *feature =
> +        arm_gen_dynamic_sysreg_feature(cs, cs->gdb_next_base_reg);
>      gdb_register_coprocessor(cs, arm_gdb_get_sysreg, arm_gdb_set_sysreg,
> -                             arm_gen_dynamic_sysreg_feature(cs, cs->gdb_num_regs));
> +                             feature);
>  
>  #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, cs->gdb_num_regs));
> +            arm_gen_dynamic_m_systemreg_feature(cs, cs->gdb_next_base_reg));
>  #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, cs->gdb_num_regs));
> +                arm_gen_dynamic_m_secextreg_feature(cs, cs->gdb_next_base_reg));
>          }
>  #endif
>      }
> diff --git a/target/arm/gdbstub64.c b/target/arm/gdbstub64.c
> index 0c3e5b30bd6a..cf5b95b54641 100644
> --- a/target/arm/gdbstub64.c
> +++ b/target/arm/gdbstub64.c
> @@ -885,7 +885,8 @@ void aarch64_cpu_register_gdb_regs_for_features(ARMCPU *cpu)
>      CPUState *cs = CPU(cpu);
>      if (isar_feature_aa64_sve(&cpu->isar) ||
>          isar_feature_aa64_sme(&cpu->isar)) {
> -        GDBFeature *feature = arm_gen_dynamic_svereg_feature(cs, cs->gdb_num_regs);
> +        GDBFeature *feature =
> +            arm_gen_dynamic_svereg_feature(cs, cs->gdb_next_base_reg);
>          gdb_register_coprocessor(cs, aarch64_gdb_get_sve_reg,
>                                   aarch64_gdb_set_sve_reg, feature);
>      } else {
> @@ -896,7 +897,7 @@ void aarch64_cpu_register_gdb_regs_for_features(ARMCPU *cpu)
>  
>      if (isar_feature_aa64_sme(&cpu->isar)) {
>          GDBFeature *sme_feature =
> -            arm_gen_dynamic_smereg_feature(cs, cs->gdb_num_regs);
> +            arm_gen_dynamic_smereg_feature(cs, cs->gdb_next_base_reg);
>          gdb_register_coprocessor(cs, aarch64_gdb_get_sme_reg,
>                                   aarch64_gdb_set_sme_reg, sme_feature);
>          if (isar_feature_aa64_sme2(&cpu->isar)) {
> @@ -927,7 +928,8 @@ void aarch64_cpu_register_gdb_regs_for_features(ARMCPU *cpu)
>  #endif
>  
>      /* All AArch64 CPUs have at least TPIDR */
> +    GDBFeature *tls_feature =
> +        arm_gen_dynamic_tls_feature(cs, cs->gdb_next_base_reg);
>      gdb_register_coprocessor(cs, aarch64_gdb_get_tls_reg,
> -                             aarch64_gdb_set_tls_reg,
> -                             arm_gen_dynamic_tls_feature(cs, cs->gdb_num_regs));
> +                             aarch64_gdb_set_tls_reg, tls_feature);
>  }
> diff --git a/target/ppc/gdbstub.c b/target/ppc/gdbstub.c
> index 4d622c5cad59..ec25c541c685 100644
> --- a/target/ppc/gdbstub.c
> +++ b/target/ppc/gdbstub.c
> @@ -206,7 +206,7 @@ static void gdb_gen_spr_feature(CPUState *cs)
>  
>      gdb_feature_builder_init(&builder, &pcc->gdb_spr,
>                               "org.qemu.power.spr", "power-spr.xml",
> -                             cs->gdb_num_regs);
> +                             cs->gdb_next_base_reg);
>  
>      for (i = 0; i < ARRAY_SIZE(env->spr_cb); i++) {
>          ppc_spr_t *spr = &env->spr_cb[i];
> diff --git a/target/riscv/gdbstub.c b/target/riscv/gdbstub.c
> index 9abbf5bcdf0e..f279f4ab2461 100644
> --- a/target/riscv/gdbstub.c
> +++ b/target/riscv/gdbstub.c
> @@ -348,9 +348,10 @@ void riscv_cpu_register_gdb_regs_for_features(CPUState *cs)
>                                   gdb_find_static_feature("riscv-32bit-fpu.xml"));
>      }
>      if (cpu->cfg.ext_zve32x) {
> +        GDBFeature *feature =
> +            ricsv_gen_dynamic_vector_feature(cs, cs->gdb_next_base_reg);
>          gdb_register_coprocessor(cs, riscv_gdb_get_vector,
> -                                 riscv_gdb_set_vector,
> -                                 ricsv_gen_dynamic_vector_feature(cs, cs->gdb_num_regs));
> +                                 riscv_gdb_set_vector, feature);
>      }
>  
>  #ifdef CONFIG_TCG
> @@ -373,8 +374,10 @@ void riscv_cpu_register_gdb_regs_for_features(CPUState *cs)
>      }
>  
>      if (cpu->cfg.ext_zicsr) {
> +        GDBFeature *feature =
> +            riscv_gen_dynamic_csr_feature(cs, cs->gdb_next_base_reg);
>          gdb_register_coprocessor(cs, riscv_gdb_get_csr, riscv_gdb_set_csr,
> -                                 riscv_gen_dynamic_csr_feature(cs, cs->gdb_num_regs));
> +                                 feature);
>      }
>  #endif
>  }
> diff --git a/target/xtensa/cpu.c b/target/xtensa/cpu.c
> index 7c25b9ab707a..48863a1859d4 100644
> --- a/target/xtensa/cpu.c
> +++ b/target/xtensa/cpu.c
> @@ -263,7 +263,7 @@ static void xtensa_cpu_realizefn(DeviceState *dev, Error **errp)
>          return;
>      }
>  
> -    cs->gdb_num_regs = xcc->config->gdb_regmap.num_regs;
> +    cs->gdb_next_base_reg = xcc->config->gdb_regmap.num_regs;
>  
>      qemu_init_vcpu(cs);

-- 
Alex Bennée
Virtualisation Tech Lead @ Linaro


  reply	other threads:[~2026-09-11 15:30 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-09-11 12:27 [PATCH 00/10] monitor/hmp: replace MonitorDef with GDB registers Marc-André Lureau
2026-09-11 12:27 ` [PATCH 01/10] hmp: don't crash on invalid register Marc-André Lureau
2026-09-11 12:27 ` [PATCH 02/10] gdbstub: fix next register base after register gaps Marc-André Lureau
2026-09-11 15:29   ` Alex Bennée [this message]
2026-09-11 12:27 ` [PATCH 03/10] gdbstub: set feature->base_reg for dynamic features Marc-André Lureau
2026-09-11 12:27 ` [PATCH 04/10] target/i386: expose segment limits via GDB Marc-André Lureau
2026-09-11 12:27 ` [PATCH 05/10] monitor/hmp: add CPUClass::get_pc fallback for HMP $pc Marc-André Lureau
2026-09-11 12:27 ` [PATCH 06/10] target/m68k: expose system registers via GDB Marc-André Lureau
2026-09-11 12:27 ` [PATCH 07/10] target/sparc64: " Marc-André Lureau
2026-09-11 12:27 ` [PATCH 08/10] target/riscv: bypass smstateen check in debugger mode Marc-André Lureau
2026-09-11 12:27 ` [PATCH 09/10] target/riscv: expose register aliases via GDB Marc-André Lureau
2026-09-11 12:27 ` [PATCH 10/10] monitor/hmp: remove legacy MonitorDef infrastructure Marc-André Lureau

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=87fqzf958j.fsf@draig.linaro.org \
    --to=alex.bennee@linaro.org \
    --cc=alistair.francis@wdc.com \
    --cc=atar4qemu@gmail.com \
    --cc=chao.liu@processmission.com \
    --cc=daniel.barboza@oss.qualcomm.com \
    --cc=dave@treblig.org \
    --cc=deller@gmx.de \
    --cc=harshpb@linux.ibm.com \
    --cc=imp@bsdimp.com \
    --cc=jcmvbkbc@gmail.com \
    --cc=kevans@freebsd.org \
    --cc=laurent@vivier.eu \
    --cc=liwei1518@gmail.com \
    --cc=marcandre.lureau@redhat.com \
    --cc=mark.cave-ayland@ilande.co.uk \
    --cc=milesg@linux.ibm.com \
    --cc=npiggin@gmail.com \
    --cc=palmer@dabbelt.com \
    --cc=pbonzini@redhat.com \
    --cc=peter.maydell@linaro.org \
    --cc=philmd@oss.qualcomm.com \
    --cc=pierrick.bouvier@oss.qualcomm.com \
    --cc=qemu-arm@nongnu.org \
    --cc=qemu-devel@nongnu.org \
    --cc=qemu-ppc@nongnu.org \
    --cc=qemu-riscv@nongnu.org \
    --cc=rathc@linux.ibm.com \
    --cc=zhao1.liu@intel.com \
    --cc=zhiwei_liu@linux.alibaba.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox