On Tue, Feb 03, 2026 at 08:56:00PM +0800, Chao Liu wrote: > From: Daniel Henrique Barboza <[email protected]> > > 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]> Thanks, Chao > --- > docs/about/deprecated.rst | 7 +++++ > target/riscv/cpu.c | 51 ++++++++++++++++++++++++++++--- > 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, 69 insertions(+), 19 deletions(-) > > diff --git a/docs/about/deprecated.rst b/docs/about/deprecated.rst > index 7abb3dab59..44a6e53044 100644 > --- a/docs/about/deprecated.rst > +++ b/docs/about/deprecated.rst > @@ -507,6 +507,13 @@ It was implemented as a no-op instruction in TCG up to > QEMU 9.0, but > only with ``-cpu max`` (which does not guarantee migration compatibility > across versions). > > +``debug=true|false`` on RISC-V CPUs (since 11.0) > +^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ > + > +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``. > + > Backwards compatibility > ----------------------- > > diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c > index 01cb62bde4..9fa4e09f17 100644 > --- a/target/riscv/cpu.c > +++ b/target/riscv/cpu.c > @@ -210,7 +210,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_EXT_DATA_ENTRY(shcounterenw, PRIV_VERSION_1_12_0, has_priv_1_12), > ISA_EXT_DATA_ENTRY(sha, PRIV_VERSION_1_12_0, ext_sha), > ISA_EXT_DATA_ENTRY(shgatpa, PRIV_VERSION_1_12_0, has_priv_1_12), > @@ -782,7 +782,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); > } > > @@ -945,7 +945,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 > @@ -1124,6 +1124,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; > } > @@ -1238,6 +1246,7 @@ const RISCVCPUMultiExtConfig riscv_cpu_extensions[] = { > MULTI_EXT_CFG_BOOL("smcdeleg", ext_smcdeleg, false), > MULTI_EXT_CFG_BOOL("sscsrind", ext_sscsrind, false), > MULTI_EXT_CFG_BOOL("ssccfg", ext_ssccfg, false), > + MULTI_EXT_CFG_BOOL("sdtrig", ext_sdtrig, true), > MULTI_EXT_CFG_BOOL("smctr", ext_smctr, false), > MULTI_EXT_CFG_BOOL("ssctr", ext_ssctr, false), > MULTI_EXT_CFG_BOOL("zifencei", ext_zifencei, true), > @@ -2649,8 +2658,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 3696f02ee0..1e9162281f 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) > @@ -157,7 +158,6 @@ BOOL_FIELD(ext_xmipslsp) > > 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 05c7ec8352..870fad87ac 100644 > --- a/target/riscv/csr.c > +++ b/target/riscv/csr.c > @@ -777,7 +777,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 09c032a879..62c51c8033 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), > @@ -425,8 +425,8 @@ static const VMStateDescription vmstate_sstc = { > > 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), > @@ -492,13 +492,13 @@ const VMStateDescription vmstate_riscv_cpu = { > &vmstate_kvmtimer, > #endif > &vmstate_envcfg, > - &vmstate_debug, > &vmstate_smstateen, > &vmstate_jvt, > &vmstate_elp, > &vmstate_ssp, > &vmstate_ctr, > &vmstate_sstc, > + &vmstate_sdtrig, > NULL > } > }; > diff --git a/target/riscv/tcg/tcg-cpu.c b/target/riscv/tcg/tcg-cpu.c > index 378b298886..d9fbb5bf58 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.53.0 >
