On Thu, May 28, 2026 at 4:19 AM Daniel Henrique Barboza <[email protected]> wrote: > > Starting on commit f31ba686a9 ("target/riscv/cpu.c: add 'sdtrig' in > riscv,isa') the 'debug' flag has been used as an alias for 'sdtrig'. > > We're going to add more debug trigger extensions, e.g. 'sdext' [1]. And > all of a sudden the existence of this flag is now weird. Do we keep it > as a 'sdtrig' only or do we add 'sdext'? > > The solution proposed here is to deprecate it. The flag was introduced a > long time ago as a way to encapsulate support for all debug related > CSRs. Today we have specific debug trigger extensions and there's no > more use for a generic 'debug' flag. Users should be encouraged to > enable/disable extensions directly instead of using "made-up" flags that > exists only in a QEMU context. > > The following changes are made: > > - 'ext_sdtrig' flag was added in cpu->cfg. 'debug' flag was removed from > cpu->cfg; > - All occurrences of cpu->cfg.debug were replaced to 'ext_sdtrig'; > - Two explicit getters and setters for the 'debug' property were added. > The property will simply get/set ext_sdtrig; > - vmstate_debug was renamed to vmstate_sdtrig. We're aware that this > will impact migration between QEMU 10.2 to newer versions, but we're > still in a point where the migration break cost isn't big enough to > justify adding migration compatibility scaffolding. > > Finally, deprecated.rst was updated to deprecate 'debug' and encourage > users to use 'ext_sdtrig' instead. > > [1] > https://lore.kernel.org/qemu-devel/[email protected]/ > > Signed-off-by: Daniel Henrique Barboza <[email protected]> > Reviewed-by: Chao Liu <[email protected]> > Tested-by: Tao Tang <[email protected]> > --- > docs/about/deprecated.rst | 6 ++++ > target/riscv/cpu.c | 50 ++++++++++++++++++++++++++++--- > target/riscv/cpu_cfg_fields.h.inc | 2 +- > target/riscv/csr.c | 2 +- > target/riscv/machine.c | 24 +++++++-------- > target/riscv/tcg/tcg-cpu.c | 2 +- > 6 files changed, 67 insertions(+), 19 deletions(-) > > diff --git a/docs/about/deprecated.rst b/docs/about/deprecated.rst > index 97750f5edc..7014d706c1 100644 > --- a/docs/about/deprecated.rst > +++ b/docs/about/deprecated.rst > @@ -417,6 +417,12 @@ ABI is long-obsolete. We are therefore deprecating both > OABI support > and NWFPE emulation, and they will be removed in a future QEMU > release. > > +``debug=true|false`` on RISC-V CPUs (since 11.1) > +^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ > + > +This option, since QEMU 10.1, has been a simple alias to the ``sdtrig`` > +extension. Users are advised to enable/disable ``sdtrig`` directly instead > +of using ``debug``.
I think this should be explicit that we are deprecating support for the ratified debug extension [1], not just the flag. Anyone using the ratified spec will need to move to the new ratified (but not backwards compatible) spec. 1: https://github.com/riscv/riscv-debug-spec/releases/tag/task_group_vote Alistair > > Backwards compatibility > ----------------------- > diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c > index 862834b480..54e8f78a4d 100644 > --- a/target/riscv/cpu.c > +++ b/target/riscv/cpu.c > @@ -226,7 +226,7 @@ const RISCVIsaExtData isa_edata_arr[] = { > ISA_EXT_DATA_ENTRY(zvkt, PRIV_VERSION_1_12_0, ext_zvkt), > ISA_EXT_DATA_ENTRY(zhinx, PRIV_VERSION_1_12_0, ext_zhinx), > ISA_EXT_DATA_ENTRY(zhinxmin, PRIV_VERSION_1_12_0, ext_zhinxmin), > - ISA_EXT_DATA_ENTRY(sdtrig, PRIV_VERSION_1_12_0, debug), > + ISA_EXT_DATA_ENTRY(sdtrig, PRIV_VERSION_1_12_0, ext_sdtrig), > ISA_INTERNAL_EXT_DATA_ENTRY(shcounterenw, PRIV_VERSION_1_12_0, > has_priv_1_12), > ISA_INTERNAL_EXT_DATA_ENTRY(sha, PRIV_VERSION_1_12_0, ext_sha), > @@ -807,7 +807,7 @@ static void riscv_cpu_reset_hold(Object *obj, ResetType > type) > env->vill = true; > > #ifndef CONFIG_USER_ONLY > - if (cpu->cfg.debug) { > + if (cpu->cfg.ext_sdtrig) { > riscv_trigger_reset_hold(env); > } > > @@ -971,7 +971,7 @@ static void riscv_cpu_realize(DeviceState *dev, Error > **errp) > riscv_cpu_register_gdb_regs_for_features(cs); > > #ifndef CONFIG_USER_ONLY > - if (cpu->cfg.debug) { > + if (cpu->cfg.ext_sdtrig) { > riscv_trigger_realize(&cpu->env); > } > #endif > @@ -1150,6 +1150,14 @@ static void riscv_cpu_init(Object *obj) > cpu->env.vext_ver = VEXT_VERSION_1_00_0; > cpu->cfg.max_satp_mode = -1; > > + /* > + * 'debug' started being deprecated in 11.0, been just a proxy > + * to set ext_sdtrig ever since. It has been enabled by default > + * for a long time though, so we're stuck with setting set 'strig' > + * by default too. At least for now ... > + */ > + cpu->cfg.ext_sdtrig = true; > + > if (mcc->def->profile) { > mcc->def->profile->enabled = true; > } > @@ -2514,8 +2522,42 @@ RISCVCPUImpliedExtsRule > *riscv_multi_ext_implied_rules[] = { > NULL > }; > > +/* > + * DEPRECATED_11.0: just a proxy for ext_sdtrig. > + */ > +static void prop_debug_get(Object *obj, Visitor *v, const char *name, > + void *opaque, Error **errp) > +{ > + bool value = RISCV_CPU(obj)->cfg.ext_sdtrig; > + > + visit_type_bool(v, name, &value, errp); > +} > + > +/* > + * DEPRECATED_11.0: just a proxy for ext_sdtrig. > + */ > +static void prop_debug_set(Object *obj, Visitor *v, const char *name, > + void *opaque, Error **errp) > +{ > + RISCVCPU *cpu = RISCV_CPU(obj); > + bool value; > + > + visit_type_bool(v, name, &value, errp); > + cpu->cfg.ext_sdtrig = value; > +} > + > +/* > + * DEPRECATED_11.0: just a proxy for ext_sdtrig. > + */ > +static const PropertyInfo prop_debug = { > + .type = "bool", > + .description = "DEPRECATED: use 'sdtrig' instead.", > + .get = prop_debug_get, > + .set = prop_debug_set, > +}; > + > static const Property riscv_cpu_properties[] = { > - DEFINE_PROP_BOOL("debug", RISCVCPU, cfg.debug, true), > + {.name = "debug", .info = &prop_debug}, > > {.name = "pmu-mask", .info = &prop_pmu_mask}, > {.name = "pmu-num", .info = &prop_pmu_num}, /* Deprecated */ > diff --git a/target/riscv/cpu_cfg_fields.h.inc > b/target/riscv/cpu_cfg_fields.h.inc > index 734fa079f2..44235bfaa1 100644 > --- a/target/riscv/cpu_cfg_fields.h.inc > +++ b/target/riscv/cpu_cfg_fields.h.inc > @@ -46,6 +46,7 @@ BOOL_FIELD(ext_zilsd) > BOOL_FIELD(ext_zimop) > BOOL_FIELD(ext_zcmop) > BOOL_FIELD(ext_ztso) > +BOOL_FIELD(ext_sdtrig) > BOOL_FIELD(ext_smstateen) > BOOL_FIELD(ext_sstc) > BOOL_FIELD(ext_smcdeleg) > @@ -159,7 +160,6 @@ BOOL_FIELD(ext_xlrbr) > > BOOL_FIELD(mmu) > BOOL_FIELD(pmp) > -BOOL_FIELD(debug) > BOOL_FIELD(misa_w) > > BOOL_FIELD(short_isa_string) > diff --git a/target/riscv/csr.c b/target/riscv/csr.c > index 5514e0f455..17992715ff 100644 > --- a/target/riscv/csr.c > +++ b/target/riscv/csr.c > @@ -796,7 +796,7 @@ static RISCVException have_mseccfg(CPURISCVState *env, > int csrno) > > static RISCVException debug(CPURISCVState *env, int csrno) > { > - if (riscv_cpu_cfg(env)->debug) { > + if (riscv_cpu_cfg(env)->ext_sdtrig) { > return RISCV_EXCP_NONE; > } > > diff --git a/target/riscv/machine.c b/target/riscv/machine.c > index 6776e7bf5a..4ca5c9c8d7 100644 > --- a/target/riscv/machine.c > +++ b/target/riscv/machine.c > @@ -218,14 +218,14 @@ static const VMStateDescription vmstate_kvmtimer = { > }; > #endif > > -static bool debug_needed(void *opaque) > +static bool sdtrig_needed(void *opaque) > { > RISCVCPU *cpu = opaque; > > - return cpu->cfg.debug; > + return cpu->cfg.ext_sdtrig; > } > > -static int debug_post_load(void *opaque, int version_id) > +static int sdtrig_post_load(void *opaque, int version_id) > { > RISCVCPU *cpu = opaque; > CPURISCVState *env = &cpu->env; > @@ -237,12 +237,12 @@ static int debug_post_load(void *opaque, int version_id) > return 0; > } > > -static const VMStateDescription vmstate_debug = { > - .name = "cpu/debug", > - .version_id = 2, > - .minimum_version_id = 2, > - .needed = debug_needed, > - .post_load = debug_post_load, > +static const VMStateDescription vmstate_sdtrig = { > + .name = "cpu/sdtrig", > + .version_id = 1, > + .minimum_version_id = 1, > + .needed = sdtrig_needed, > + .post_load = sdtrig_post_load, > .fields = (const VMStateField[]) { > VMSTATE_UINTTL(env.trigger_cur, RISCVCPU), > VMSTATE_UINTTL_ARRAY(env.tdata1, RISCVCPU, RV_MAX_TRIGGERS), > @@ -444,8 +444,8 @@ static const VMStateDescription vmstate_mseccfg = { > > const VMStateDescription vmstate_riscv_cpu = { > .name = "cpu", > - .version_id = 11, > - .minimum_version_id = 11, > + .version_id = 12, > + .minimum_version_id = 12, > .post_load = riscv_cpu_post_load, > .fields = (const VMStateField[]) { > VMSTATE_UINTTL_ARRAY(env.gpr, RISCVCPU, 32), > @@ -511,7 +511,6 @@ const VMStateDescription vmstate_riscv_cpu = { > &vmstate_kvmtimer, > #endif > &vmstate_envcfg, > - &vmstate_debug, > &vmstate_smstateen, > &vmstate_jvt, > &vmstate_elp, > @@ -519,6 +518,7 @@ const VMStateDescription vmstate_riscv_cpu = { > &vmstate_ctr, > &vmstate_sstc, > &vmstate_mseccfg, > + &vmstate_sdtrig, > NULL > } > }; > diff --git a/target/riscv/tcg/tcg-cpu.c b/target/riscv/tcg/tcg-cpu.c > index 3e22e7ed53..a6dd864264 100644 > --- a/target/riscv/tcg/tcg-cpu.c > +++ b/target/riscv/tcg/tcg-cpu.c > @@ -180,7 +180,7 @@ static TCGTBCPUState riscv_get_tb_cpu_state(CPUState *cs) > ? EXT_STATUS_DIRTY : EXT_STATUS_DISABLED; > } > > - if (cpu->cfg.debug && !icount_enabled()) { > + if (cpu->cfg.ext_sdtrig && !icount_enabled()) { > flags = FIELD_DP32(flags, TB_FLAGS, ITRIGGER, env->itrigger_enabled); > } > #endif > -- > 2.43.0 > >
