On 1/17/2026 1:27 AM, Chao Liu wrote:
Add cfg.ext_sdext and cfg.ext_sdtrig and expose them as ISA
extensions. Keep the legacy 'debug' CPU property as a global kill
switch and force-disable both when it is off.
I would rather put the 'debug' flag in deprecation. It's a flag that at
this moment means 'enable sdtrig' and I'd rather get rid of it than
making it enable sdext too.
But deprecating 'debug' is out of scope for this work and we can handle
it later. Matter of fact I might drop patches deprecating it today.
About enabling sdext by default, the patch is breaking bios-table-test
because we're adding an extra string in riscv,isa ...
Trigger CSRs (tselect/tdata*/tinfo/mcontext) and trigger setup now
depend on ext_sdtrig instead of cfg.debug.
Signed-off-by: Chao Liu <[email protected]>
---
target/riscv/cpu.c | 18 +++++++++++++++---
target/riscv/cpu_cfg_fields.h.inc | 2 ++
target/riscv/csr.c | 16 ++++++++--------
target/riscv/machine.c | 4 ++--
target/riscv/tcg/tcg-cpu.c | 11 +----------
5 files changed, 28 insertions(+), 23 deletions(-)
diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
index 73d4280d7c..bc0b385cc1 100644
--- a/target/riscv/cpu.c
+++ b/target/riscv/cpu.c
@@ -189,7 +189,8 @@ 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(sdext, PRIV_VERSION_1_12_0, ext_sdext),
+ 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),
@@ -783,7 +784,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);
}
@@ -922,6 +923,15 @@ void riscv_cpu_finalize_features(RISCVCPU *cpu, Error **errp)
return;
}
}
+
+ /*
+ * Keep the legacy 'debug' CPU property as a global kill switch.
+ * If it is off, force-disable Sdext/Sdtrig regardless of ISA strings.
+ */
+ if (!cpu->cfg.debug) {
+ cpu->cfg.ext_sdext = false;
+ cpu->cfg.ext_sdtrig = false;
+ }
}
static void riscv_cpu_realize(DeviceState *dev, Error **errp)
@@ -946,7 +956,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
@@ -1112,6 +1122,8 @@ static void riscv_cpu_init(Object *obj)
*/
RISCV_CPU(obj)->cfg.ext_zicntr = !mcc->def->bare;
RISCV_CPU(obj)->cfg.ext_zihpm = !mcc->def->bare;
+ RISCV_CPU(obj)->cfg.ext_sdext = true;
+ RISCV_CPU(obj)->cfg.ext_sdtrig = true;
^ here. Every time we change the defaults we have to dance around
bios-tables-test due to riscv,isa changes.
If we really want sdext to be enabled by default we should (1) only
enable it when the feature is already fully implemented and (2) changing
bios-tables-test according to avoid leaving broken tests.
The remaining of the patch has the right idea, like here:
/* Default values for non-bool cpu properties */
cpu->cfg.pmu_mask = MAKE_64BIT_MASK(3, 16);
diff --git a/target/riscv/cpu_cfg_fields.h.inc
b/target/riscv/cpu_cfg_fields.h.inc
index a154ecdc79..9701319195 100644
--- a/target/riscv/cpu_cfg_fields.h.inc
+++ b/target/riscv/cpu_cfg_fields.h.inc
@@ -44,6 +44,8 @@ BOOL_FIELD(ext_zihpm)
BOOL_FIELD(ext_zimop)
BOOL_FIELD(ext_zcmop)
BOOL_FIELD(ext_ztso)
+BOOL_FIELD(ext_sdext)
+BOOL_FIELD(ext_sdtrig)
BOOL_FIELD(ext_smstateen)
BOOL_FIELD(ext_sstc)
BOOL_FIELD(ext_smcdeleg)
diff --git a/target/riscv/csr.c b/target/riscv/csr.c
index 5c91658c3d..4f071b1db2 100644
--- a/target/riscv/csr.c
+++ b/target/riscv/csr.c
@@ -775,9 +775,9 @@ static RISCVException have_mseccfg(CPURISCVState *env, int
csrno)
return RISCV_EXCP_ILLEGAL_INST;
}
-static RISCVException debug(CPURISCVState *env, int csrno)
+static RISCVException sdtrig(CPURISCVState *env, int csrno)
{
- if (riscv_cpu_cfg(env)->debug) {
+ if (riscv_cpu_cfg(env)->ext_sdtrig) {
return RISCV_EXCP_NONE;
}
debug == sdtrig is what we want and we should use 'sdtrig' everywhere
'debug' is appearing, but there's a particular way to go about it. We
need to add getters/setters for 'debug' that would enable/disable
sdtrig. We should avoid adding this kind of handling in init() or
finalize().
I can do that in a deprecation patch. You can use your patches to handle
sdext only.
Thanks,
Daniel
@@ -6308,12 +6308,12 @@ riscv_csr_operations csr_ops[CSR_TABLE_SIZE] = {
.min_priv_ver = PRIV_VERSION_1_12_0 },
/* Debug CSRs */
- [CSR_TSELECT] = { "tselect", debug, read_tselect, write_tselect },
- [CSR_TDATA1] = { "tdata1", debug, read_tdata, write_tdata },
- [CSR_TDATA2] = { "tdata2", debug, read_tdata, write_tdata },
- [CSR_TDATA3] = { "tdata3", debug, read_tdata, write_tdata },
- [CSR_TINFO] = { "tinfo", debug, read_tinfo, write_ignore },
- [CSR_MCONTEXT] = { "mcontext", debug, read_mcontext, write_mcontext },
+ [CSR_TSELECT] = { "tselect", sdtrig, read_tselect, write_tselect },
+ [CSR_TDATA1] = { "tdata1", sdtrig, read_tdata, write_tdata },
+ [CSR_TDATA2] = { "tdata2", sdtrig, read_tdata, write_tdata },
+ [CSR_TDATA3] = { "tdata3", sdtrig, read_tdata, write_tdata },
+ [CSR_TINFO] = { "tinfo", sdtrig, read_tinfo, write_ignore },
+ [CSR_MCONTEXT] = { "mcontext", sdtrig, read_mcontext, write_mcontext },
[CSR_MCTRCTL] = { "mctrctl", ctr_mmode, NULL, NULL, rmw_xctrctl },
[CSR_SCTRCTL] = { "sctrctl", ctr_smode, NULL, NULL, rmw_xctrctl
},
diff --git a/target/riscv/machine.c b/target/riscv/machine.c
index 18d790af0d..d6a0b8e357 100644
--- a/target/riscv/machine.c
+++ b/target/riscv/machine.c
@@ -222,7 +222,7 @@ static bool debug_needed(void *opaque)
{
RISCVCPU *cpu = opaque;
- return cpu->cfg.debug;
+ return cpu->cfg.ext_sdext || cpu->cfg.ext_sdtrig;
}
static int debug_post_load(void *opaque, int version_id)
@@ -230,7 +230,7 @@ static int debug_post_load(void *opaque, int version_id)
RISCVCPU *cpu = opaque;
CPURISCVState *env = &cpu->env;
- if (icount_enabled()) {
+ if (cpu->cfg.ext_sdtrig && icount_enabled()) {
env->itrigger_enabled = riscv_itrigger_enabled(env);
}
diff --git a/target/riscv/tcg/tcg-cpu.c b/target/riscv/tcg/tcg-cpu.c
index d3968251fa..b5a26cf662 100644
--- a/target/riscv/tcg/tcg-cpu.c
+++ b/target/riscv/tcg/tcg-cpu.c
@@ -177,7 +177,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
@@ -469,15 +469,6 @@ static void riscv_cpu_disable_priv_spec_isa_exts(RISCVCPU
*cpu)
continue;
}
- /*
- * cpu.debug = true is marked as 'sdtrig', priv spec 1.12.
- * Skip this warning since existing CPUs with older priv
- * spec and debug = true will be impacted.
- */
- if (!strcmp(edata->name, "sdtrig")) {
- continue;
- }
-
isa_ext_update_enabled(cpu, edata->ext_enable_offset, false);
/*
