[RFC PATCH v5 0/7] riscv: add initial sdext support
Hi, Per Daniel's review feedback, this v5 series is based on Alistair's riscv-to-apply.next branch [3]. It depends on patches 1-5 of Max Chou's "Add Zvfbfa extension support" v2 series (not included; apply them first) [2]. It is based on RISC-V Debug Specification 1.0 [1]. It introduces the sdext/sdtrig config bits, DCSR/DPC/DSCRATCH state, Debug Mode enter/leave helpers, DRET, EBREAK entry, single-step, and trigger action=debug mode. To reduce review load, this series focuses on the Sdext features first. The Debug Module (DM) and related flows will follow in a later series. Changes in v5: - Patch 2: removed redundant `cpu->cfg.ext_sdext = false;` in riscv_cpu_init() since it's already set to false in MULTI_EXT_CFG_BOOL. (Daniel) - Patch 7: changed LOG_UNIMP to LOG_GUEST_ERROR for invalid trigger action=debug mode when Sdext is not enabled. LOG_UNIMP is reserved for unimplemented features, while LOG_GUEST_ERROR is for invalid guest actions. (Daniel) Changes in v4: - Fixed linux-user build errors: moved debug_mode/dcsr/dpc/dscratch fields inside #ifndef CONFIG_USER_ONLY block in cpu.h, and wrapped all code using these fields with proper guards. (Daniel) - Updated dependency from single patch (patch 5 only) to patches 1-5 of Max Chou's "Add Zvfbfa extension support" v2 series to avoid compilation errors. Changes in v3: - Rebase onto Alistair's riscv-to-apply.next branch. - Depend on Max Chou's patch "target/riscv: Use the tb->cs_bqse as the extend tb flags." (not included; apply it first) [2]. - Patch 2: default sdext to false in riscv_cpu_extensions[]. Changes in v2: - Drop the RHCT expected AML update from this series. - Replace the split sdext/sdtrig config bits patch with deprecating the 'debug' CPU property. - Rebase and update patch subjects to target/riscv prefix. Changes in v1: - Debug Mode entry/exit updates DCSR/DPC and restores execution via DRET. - EBREAK honors DCSR ebreak bits and enters Debug Mode when enabled. - Single-step uses DCSR.STEP with a TB flag and a helper at TB exit. It references Max Chou's patch "target/riscv: Use the tb->cs_bqse as the extend tb flags." [2]. - Sdtrig supports action=debug mode for mcontrol/mcontrol6 and reports inst-count triggers in tinfo. Differences vs Debug Spec (known gaps): - No Debug Module (no DMI, dmcontrol/dmstatus, haltreq/resumereq). - No debug ROM, program buffer, abstract commands, or SBA. - Resume is modeled by leaving Debug Mode at cpu_exec_enter. - Step/exception ordering is simplified: if the stepped instruction traps, the normal exception is taken and Debug Mode is not forced. - Several DCSR fields are not fully modeled (stopcount/stoptime, etc). Roadmap (next stage, DM focus): 1) Add a DM core with DMI access and hart state tracking. 2) Implement halt/resume handshake and move Debug Mode transitions under DM control. 3) Add debug ROM, program buffer, and abstract commands for GPR/CSR and memory access. 4) Add SBA if required by tooling. 5) Tighten ordering rules for step/exception/trigger priorities. References: [1] https://github.com/riscv/riscv-debug-spec/releases/tag/1.0 [2] https://lore.kernel.org/qemu-devel/20260108132631.9429-1-max.chou@sifive.com... [3] https://github.com/alistair23/qemu/tree/riscv-to-apply.next Thanks, Chao Chao Liu (6): target/riscv: add sdext debug CSRs state target/riscv: add sdext Debug Mode helpers target/riscv: add dret instruction target/riscv: add sdext enter Debug Mode on ebreak target/riscv: add sdext single-step support target/riscv: add sdtrig trigger action=debug mode Daniel Henrique Barboza (1): target/riscv: deprecate 'debug' CPU property docs/about/deprecated.rst | 7 + include/exec/translation-block.h | 4 +- target/riscv/cpu.c | 59 +++++++- target/riscv/cpu.h | 9 ++ target/riscv/cpu_bits.h | 33 +++++ target/riscv/cpu_cfg_fields.h.inc | 3 +- target/riscv/cpu_helper.c | 90 ++++++++++++ target/riscv/csr.c | 128 +++++++++++++++++- target/riscv/debug.c | 58 +++++++- target/riscv/helper.h | 3 + target/riscv/insn32.decode | 1 + .../riscv/insn_trans/trans_privileged.c.inc | 24 ++++ target/riscv/machine.c | 44 ++++-- target/riscv/op_helper.c | 70 ++++++++++ target/riscv/tcg/tcg-cpu.c | 21 ++- target/riscv/translate.c | 16 ++- 16 files changed, 545 insertions(+), 25 deletions(-) -- 2.53.0
From: Daniel Henrique Barboza <daniel.barboza@oss.qualcomm.com> 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/cover.1768622881.git.chao.liu.zevorn@gmai... Signed-off-by: Daniel Henrique Barboza <daniel.barboza@oss.qualcomm.com> --- 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
On Tue, Feb 03, 2026 at 08:56:00PM +0800, Chao Liu wrote:
From: Daniel Henrique Barboza <daniel.barboza@oss.qualcomm.com>
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/cover.1768622881.git.chao.liu.zevorn@gmai...
Signed-off-by: Daniel Henrique Barboza <daniel.barboza@oss.qualcomm.com>
Reviewed-by: Chao Liu <chao.liu.zevorn@gmail.com> 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
RISC-V Debug Specification: https://github.com/riscv/riscv-debug-spec/releases/tag/1.0 Add architectural state for Sdext Debug Mode: debug_mode, dcsr, dpc and dscratch0/1. Wire up CSR access for dcsr/dpc/dscratch and gate them to Debug Mode (or host debugger access). The Sdext is not fully implemented, so it is disabled by default. Signed-off-by: Chao Liu <chao.liu.zevorn@gmail.com> Reviewed-by: Daniel Henrique Barboza <daniel.barboza@oss.qualcomm.com> --- target/riscv/cpu.c | 8 ++ target/riscv/cpu.h | 4 + target/riscv/cpu_bits.h | 33 ++++++++ target/riscv/cpu_cfg_fields.h.inc | 1 + target/riscv/csr.c | 126 ++++++++++++++++++++++++++++++ target/riscv/machine.c | 20 +++++ 6 files changed, 192 insertions(+) diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c index 9fa4e09f17..89a320074c 100644 --- a/target/riscv/cpu.c +++ b/target/riscv/cpu.c @@ -210,6 +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(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), @@ -782,6 +783,12 @@ static void riscv_cpu_reset_hold(Object *obj, ResetType type) env->vill = true; #ifndef CONFIG_USER_ONLY + env->debug_mode = false; + env->dcsr = DCSR_DEBUGVER(4); + env->dpc = 0; + env->dscratch[0] = 0; + env->dscratch[1] = 0; + if (cpu->cfg.ext_sdtrig) { riscv_trigger_reset_hold(env); } @@ -1246,6 +1253,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("sdext", ext_sdext, 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), diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h index 4c0676ed53..2a265faae5 100644 --- a/target/riscv/cpu.h +++ b/target/riscv/cpu.h @@ -476,6 +476,10 @@ struct CPUArchState { /* True if in debugger mode. */ bool debugger; + bool debug_mode; + target_ulong dcsr; + target_ulong dpc; + target_ulong dscratch[2]; uint64_t mstateen[SMSTATEEN_MAX_COUNT]; uint64_t hstateen[SMSTATEEN_MAX_COUNT]; diff --git a/target/riscv/cpu_bits.h b/target/riscv/cpu_bits.h index b62dd82fe7..bb59f7ff56 100644 --- a/target/riscv/cpu_bits.h +++ b/target/riscv/cpu_bits.h @@ -467,6 +467,39 @@ #define CSR_DCSR 0x7b0 #define CSR_DPC 0x7b1 #define CSR_DSCRATCH 0x7b2 +#define CSR_DSCRATCH1 0x7b3 + +/* DCSR fields */ +#define DCSR_XDEBUGVER_SHIFT 28 +#define DCSR_XDEBUGVER_MASK (0xfu << DCSR_XDEBUGVER_SHIFT) +#define DCSR_DEBUGVER(val) ((target_ulong)(val) << DCSR_XDEBUGVER_SHIFT) +#define DCSR_EXTCAUSE_SHIFT 24 +#define DCSR_EXTCAUSE_MASK (0x7u << DCSR_EXTCAUSE_SHIFT) +#define DCSR_CETRIG BIT(19) +#define DCSR_PELP BIT(18) +#define DCSR_EBREAKVS BIT(17) +#define DCSR_EBREAKVU BIT(16) +#define DCSR_EBREAKM BIT(15) +#define DCSR_EBREAKS BIT(13) +#define DCSR_EBREAKU BIT(12) +#define DCSR_STEPIE BIT(11) +#define DCSR_STOPCOUNT BIT(10) +#define DCSR_STOPTIME BIT(9) +#define DCSR_CAUSE_SHIFT 6 +#define DCSR_CAUSE_MASK (0x7u << DCSR_CAUSE_SHIFT) +#define DCSR_V BIT(5) +#define DCSR_MPRVEN BIT(4) +#define DCSR_NMIP BIT(3) +#define DCSR_STEP BIT(2) +#define DCSR_PRV_MASK 0x3u + +#define DCSR_CAUSE_EBREAK 1 +#define DCSR_CAUSE_TRIGGER 2 +#define DCSR_CAUSE_HALTREQ 3 +#define DCSR_CAUSE_STEP 4 +#define DCSR_CAUSE_RESET 5 +#define DCSR_CAUSE_GROUP 6 +#define DCSR_CAUSE_OTHER 7 /* Performance Counters */ #define CSR_MHPMCOUNTER3 0xb03 diff --git a/target/riscv/cpu_cfg_fields.h.inc b/target/riscv/cpu_cfg_fields.h.inc index 1e9162281f..e0d70fe8c7 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_sdext) BOOL_FIELD(ext_sdtrig) BOOL_FIELD(ext_smstateen) BOOL_FIELD(ext_sstc) diff --git a/target/riscv/csr.c b/target/riscv/csr.c index 870fad87ac..3e38c943e0 100644 --- a/target/riscv/csr.c +++ b/target/riscv/csr.c @@ -3136,6 +3136,126 @@ static RISCVException write_mtval(CPURISCVState *env, int csrno, return RISCV_EXCP_NONE; } +#if !defined(CONFIG_USER_ONLY) +static RISCVException sdext(CPURISCVState *env, int csrno) +{ + if (!riscv_cpu_cfg(env)->ext_sdext) { + return RISCV_EXCP_ILLEGAL_INST; + } + + if (!env->debug_mode && !env->debugger) { + return RISCV_EXCP_ILLEGAL_INST; + } + + return RISCV_EXCP_NONE; +} + +static target_ulong dcsr_visible_mask(CPURISCVState *env) +{ + target_ulong mask = (target_ulong)-1; + RISCVCPU *cpu = env_archcpu(env); + + if (!riscv_has_ext(env, RVH)) { + mask &= ~(DCSR_EBREAKVS | DCSR_EBREAKVU | DCSR_V); + } + if (!riscv_has_ext(env, RVS)) { + mask &= ~DCSR_EBREAKS; + } + if (!riscv_has_ext(env, RVU)) { + mask &= ~DCSR_EBREAKU; + } + if (!cpu->cfg.ext_zicfilp) { + mask &= ~DCSR_PELP; + } + if (!cpu->cfg.ext_smdbltrp) { + mask &= ~DCSR_CETRIG; + } + + return mask; +} + +static RISCVException read_dcsr(CPURISCVState *env, int csrno, + target_ulong *val) +{ + *val = env->dcsr & dcsr_visible_mask(env); + return RISCV_EXCP_NONE; +} + +static target_ulong dcsr_writable_mask(CPURISCVState *env) +{ + target_ulong mask = DCSR_EBREAKM | DCSR_EBREAKS | DCSR_EBREAKU | + DCSR_STEPIE | DCSR_STOPCOUNT | DCSR_STOPTIME | + DCSR_STEP | DCSR_PRV_MASK; + RISCVCPU *cpu = env_archcpu(env); + + mask |= DCSR_MPRVEN; + + if (riscv_has_ext(env, RVH)) { + mask |= DCSR_EBREAKVS | DCSR_EBREAKVU | DCSR_V; + } + if (riscv_has_ext(env, RVS)) { + mask |= DCSR_EBREAKS; + } + if (riscv_has_ext(env, RVU)) { + mask |= DCSR_EBREAKU; + } + if (cpu->cfg.ext_zicfilp) { + mask |= DCSR_PELP; + } + if (cpu->cfg.ext_smdbltrp) { + mask |= DCSR_CETRIG; + } + + return mask; +} + +static RISCVException write_dcsr(CPURISCVState *env, int csrno, + target_ulong val, uintptr_t ra) +{ + target_ulong mask = dcsr_writable_mask(env); + target_ulong new_val = env->dcsr; + + new_val &= ~mask; + new_val |= val & mask; + new_val &= ~DCSR_XDEBUGVER_MASK; + new_val |= DCSR_DEBUGVER(4); + env->dcsr = new_val; + return RISCV_EXCP_NONE; +} + +static RISCVException read_dpc(CPURISCVState *env, int csrno, + target_ulong *val) +{ + *val = env->dpc & get_xepc_mask(env); + return RISCV_EXCP_NONE; +} + +static RISCVException write_dpc(CPURISCVState *env, int csrno, + target_ulong val, uintptr_t ra) +{ + env->dpc = val & get_xepc_mask(env); + return RISCV_EXCP_NONE; +} + +static RISCVException read_dscratch(CPURISCVState *env, int csrno, + target_ulong *val) +{ + int index = (csrno == CSR_DSCRATCH1) ? 1 : 0; + + *val = env->dscratch[index]; + return RISCV_EXCP_NONE; +} + +static RISCVException write_dscratch(CPURISCVState *env, int csrno, + target_ulong val, uintptr_t ra) +{ + int index = (csrno == CSR_DSCRATCH1) ? 1 : 0; + + env->dscratch[index] = val; + return RISCV_EXCP_NONE; +} +#endif /* !CONFIG_USER_ONLY */ + /* Execution environment configuration setup */ static RISCVException read_menvcfg(CPURISCVState *env, int csrno, target_ulong *val) @@ -6297,6 +6417,12 @@ riscv_csr_operations csr_ops[CSR_TABLE_SIZE] = { [CSR_TDATA3] = { "tdata3", debug, read_tdata, write_tdata }, [CSR_TINFO] = { "tinfo", debug, read_tinfo, write_ignore }, [CSR_MCONTEXT] = { "mcontext", debug, read_mcontext, write_mcontext }, +#if !defined(CONFIG_USER_ONLY) + [CSR_DCSR] = { "dcsr", sdext, read_dcsr, write_dcsr }, + [CSR_DPC] = { "dpc", sdext, read_dpc, write_dpc }, + [CSR_DSCRATCH] = { "dscratch0", sdext, read_dscratch, write_dscratch }, + [CSR_DSCRATCH1] = { "dscratch1", sdext, read_dscratch, write_dscratch }, +#endif [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 62c51c8033..52264cf047 100644 --- a/target/riscv/machine.c +++ b/target/riscv/machine.c @@ -248,6 +248,25 @@ static const VMStateDescription vmstate_sdtrig = { VMSTATE_UINTTL_ARRAY(env.tdata1, RISCVCPU, RV_MAX_TRIGGERS), VMSTATE_UINTTL_ARRAY(env.tdata2, RISCVCPU, RV_MAX_TRIGGERS), VMSTATE_UINTTL_ARRAY(env.tdata3, RISCVCPU, RV_MAX_TRIGGERS), + VMSTATE_BOOL_V(env.debug_mode, RISCVCPU, 3), + VMSTATE_UINTTL_V(env.dcsr, RISCVCPU, 3), + VMSTATE_UINTTL_V(env.dpc, RISCVCPU, 3), + VMSTATE_UINTTL_ARRAY_V(env.dscratch, RISCVCPU, 2, 3), + VMSTATE_END_OF_LIST() + } +}; + +static const VMStateDescription vmstate_sdext = { + .name = "cpu/sdext", + .version_id = 1, + .minimum_version_id = 1, + .needed = sdtrig_needed, + .post_load = sdtrig_post_load, + .fields = (const VMStateField[]) { + VMSTATE_BOOL_V(env.debug_mode, RISCVCPU, 3), + VMSTATE_UINTTL_V(env.dcsr, RISCVCPU, 3), + VMSTATE_UINTTL_V(env.dpc, RISCVCPU, 3), + VMSTATE_UINTTL_ARRAY_V(env.dscratch, RISCVCPU, 2, 3), VMSTATE_END_OF_LIST() } }; @@ -499,6 +518,7 @@ const VMStateDescription vmstate_riscv_cpu = { &vmstate_ctr, &vmstate_sstc, &vmstate_sdtrig, + &vmstate_sdext, NULL } }; -- 2.53.0
RISC-V Debug Specification: https://github.com/riscv/riscv-debug-spec/releases/tag/1.0 Add helpers to enter/leave Debug Mode and to update dpc/dcsr. Model resume without a Debug Module by leaving Debug Mode at cpu_exec_enter and continuing from dpc. Signed-off-by: Chao Liu <chao.liu.zevorn@gmail.com> Reviewed-by: Daniel Henrique Barboza <daniel.barboza@oss.qualcomm.com> --- target/riscv/cpu.h | 3 ++ target/riscv/cpu_helper.c | 84 ++++++++++++++++++++++++++++++++++++++ target/riscv/debug.c | 5 +++ target/riscv/tcg/tcg-cpu.c | 14 +++++++ 4 files changed, 106 insertions(+) diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h index 2a265faae5..62732957a4 100644 --- a/target/riscv/cpu.h +++ b/target/riscv/cpu.h @@ -624,6 +624,9 @@ bool riscv_cpu_tlb_fill(CPUState *cs, vaddr address, int size, char *riscv_isa_string(RISCVCPU *cpu); int riscv_cpu_max_xlen(RISCVCPUClass *mcc); bool riscv_cpu_option_set(const char *optname); +void riscv_cpu_enter_debug_mode(CPURISCVState *env, target_ulong pc, + uint32_t cause); +void riscv_cpu_leave_debug_mode(CPURISCVState *env); #ifndef CONFIG_USER_ONLY void riscv_cpu_do_interrupt(CPUState *cpu); diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c index dd6c861a90..0e266ff3a9 100644 --- a/target/riscv/cpu_helper.c +++ b/target/riscv/cpu_helper.c @@ -136,6 +136,90 @@ bool riscv_env_smode_dbltrp_enabled(CPURISCVState *env, bool virt) #endif } +#ifndef CONFIG_USER_ONLY +static bool riscv_sdext_enabled(CPURISCVState *env) +{ + return riscv_cpu_cfg(env)->ext_sdext; +} +#endif + +void riscv_cpu_enter_debug_mode(CPURISCVState *env, target_ulong pc, + uint32_t cause) +{ +#ifndef CONFIG_USER_ONLY + if (!riscv_sdext_enabled(env)) { + return; + } + + env->debug_mode = true; + env->dpc = pc & get_xepc_mask(env); + env->dcsr &= ~(DCSR_CAUSE_MASK | DCSR_PRV_MASK | DCSR_V); + env->dcsr |= ((target_ulong)(cause & 0x7)) << DCSR_CAUSE_SHIFT; + env->dcsr |= env->priv & DCSR_PRV_MASK; + if (env->virt_enabled && riscv_has_ext(env, RVH)) { + env->dcsr |= DCSR_V; + } + + if (env_archcpu(env)->cfg.ext_zicfilp) { + if (env->elp) { + env->dcsr |= DCSR_PELP; + } else { + env->dcsr &= ~DCSR_PELP; + } + env->elp = false; + } +#endif +} + +void riscv_cpu_leave_debug_mode(CPURISCVState *env) +{ +#ifndef CONFIG_USER_ONLY + if (!riscv_sdext_enabled(env)) { + return; + } + + target_ulong new_priv = env->dcsr & DCSR_PRV_MASK; + bool new_virt = riscv_has_ext(env, RVH) && (env->dcsr & DCSR_V); + + if (new_priv > PRV_M) { + new_priv = PRV_M; + } + if (new_priv == PRV_M) { + new_virt = false; + } + + if (new_priv == PRV_S && !riscv_has_ext(env, RVS)) { + new_priv = PRV_M; + new_virt = false; + } else if (new_priv == PRV_U && !riscv_has_ext(env, RVU)) { + new_priv = riscv_has_ext(env, RVS) ? PRV_S : PRV_M; + new_virt = false; + } + + env->debug_mode = false; + riscv_cpu_set_mode(env, new_priv, new_virt); + + if (env_archcpu(env)->cfg.ext_zicfilp) { + env->elp = cpu_get_fcfien(env) && (env->dcsr & DCSR_PELP); + env->dcsr &= ~DCSR_PELP; + } + + if (new_priv != PRV_M) { + env->mstatus = set_field(env->mstatus, MSTATUS_MPRV, 0); + } + + if (env_archcpu(env)->cfg.ext_smdbltrp && new_priv != PRV_M) { + env->mstatus = set_field(env->mstatus, MSTATUS_MDT, 0); + } + if (env_archcpu(env)->cfg.ext_ssdbltrp && (new_priv == PRV_U || new_virt)) { + env->mstatus = set_field(env->mstatus, MSTATUS_SDT, 0); + if (new_virt && new_priv == PRV_U) { + env->vsstatus = set_field(env->vsstatus, MSTATUS_SDT, 0); + } + } +#endif +} + RISCVPmPmm riscv_pm_get_pmm(CPURISCVState *env) { #ifndef CONFIG_USER_ONLY diff --git a/target/riscv/debug.c b/target/riscv/debug.c index 5664466749..5877a60c50 100644 --- a/target/riscv/debug.c +++ b/target/riscv/debug.c @@ -927,6 +927,11 @@ void riscv_cpu_debug_excp_handler(CPUState *cs) RISCVCPU *cpu = RISCV_CPU(cs); CPURISCVState *env = &cpu->env; + /* Triggers must not match or fire while in Debug Mode. */ + if (env->debug_mode) { + return; + } + if (cs->watchpoint_hit) { if (cs->watchpoint_hit->flags & BP_CPU) { do_trigger_action(env, DBG_ACTION_BP); diff --git a/target/riscv/tcg/tcg-cpu.c b/target/riscv/tcg/tcg-cpu.c index d9fbb5bf58..f80e3413f8 100644 --- a/target/riscv/tcg/tcg-cpu.c +++ b/target/riscv/tcg/tcg-cpu.c @@ -266,6 +266,19 @@ static vaddr riscv_pointer_wrap(CPUState *cs, int mmu_idx, } return extract64(result, 0, 64 - pm_len); } + +static void riscv_cpu_exec_enter(CPUState *cs) +{ + RISCVCPU *cpu = RISCV_CPU(cs); + CPURISCVState *env = &cpu->env; + + if (!cpu->cfg.ext_sdext || !env->debug_mode) { + return; + } + target_ulong pc = env->dpc; + riscv_cpu_leave_debug_mode(env); + env->pc = pc; +} #endif const TCGCPUOps riscv_tcg_ops = { @@ -282,6 +295,7 @@ const TCGCPUOps riscv_tcg_ops = { #ifndef CONFIG_USER_ONLY .tlb_fill = riscv_cpu_tlb_fill, .pointer_wrap = riscv_pointer_wrap, + .cpu_exec_enter = riscv_cpu_exec_enter, .cpu_exec_interrupt = riscv_cpu_exec_interrupt, .cpu_exec_halt = riscv_cpu_has_work, .cpu_exec_reset = cpu_reset, -- 2.53.0
RISC-V Debug Specification: https://github.com/riscv/riscv-debug-spec/releases/tag/1.0 Add DRET decode/translate and a helper to leave Debug Mode and return to dpc. Executing DRET outside Debug Mode raises illegal instruction. Signed-off-by: Chao Liu <chao.liu.zevorn@gmail.com> Reviewed-by: Daniel Henrique Barboza <daniel.barboza@oss.qualcomm.com> --- target/riscv/helper.h | 1 + target/riscv/insn32.decode | 1 + target/riscv/insn_trans/trans_privileged.c.inc | 18 ++++++++++++++++++ target/riscv/op_helper.c | 16 ++++++++++++++++ 4 files changed, 36 insertions(+) diff --git a/target/riscv/helper.h b/target/riscv/helper.h index b785456ee0..6140b6340d 100644 --- a/target/riscv/helper.h +++ b/target/riscv/helper.h @@ -131,6 +131,7 @@ DEF_HELPER_6(csrrw_i128, tl, env, int, tl, tl, tl, tl) #ifndef CONFIG_USER_ONLY DEF_HELPER_1(sret, tl, env) DEF_HELPER_1(mret, tl, env) +DEF_HELPER_1(dret, tl, env) DEF_HELPER_1(mnret, tl, env) DEF_HELPER_1(ctr_clear, void, env) DEF_HELPER_1(wfi, void, env) diff --git a/target/riscv/insn32.decode b/target/riscv/insn32.decode index 6e35c4b1e6..4db842d5d9 100644 --- a/target/riscv/insn32.decode +++ b/target/riscv/insn32.decode @@ -118,6 +118,7 @@ sctrclr 000100000100 00000 000 00000 1110011 uret 0000000 00010 00000 000 00000 1110011 sret 0001000 00010 00000 000 00000 1110011 mret 0011000 00010 00000 000 00000 1110011 +dret 0111101 10010 00000 000 00000 1110011 wfi 0001000 00101 00000 000 00000 1110011 sfence_vma 0001001 ..... ..... 000 00000 1110011 @sfence_vma diff --git a/target/riscv/insn_trans/trans_privileged.c.inc b/target/riscv/insn_trans/trans_privileged.c.inc index 8a62b4cfcd..f8641b1977 100644 --- a/target/riscv/insn_trans/trans_privileged.c.inc +++ b/target/riscv/insn_trans/trans_privileged.c.inc @@ -125,6 +125,24 @@ static bool trans_mret(DisasContext *ctx, arg_mret *a) #endif } +static bool trans_dret(DisasContext *ctx, arg_dret *a) +{ +#ifndef CONFIG_USER_ONLY + if (!ctx->cfg_ptr->ext_sdext) { + return false; + } + decode_save_opc(ctx, 0); + translator_io_start(&ctx->base); + gen_update_pc(ctx, 0); + gen_helper_dret(cpu_pc, tcg_env); + exit_tb(ctx); /* no chaining */ + ctx->base.is_jmp = DISAS_NORETURN; + return true; +#else + return false; +#endif +} + static bool trans_mnret(DisasContext *ctx, arg_mnret *a) { #ifndef CONFIG_USER_ONLY diff --git a/target/riscv/op_helper.c b/target/riscv/op_helper.c index 6ccc127c30..99736bbebb 100644 --- a/target/riscv/op_helper.c +++ b/target/riscv/op_helper.c @@ -454,6 +454,22 @@ target_ulong helper_mret(CPURISCVState *env) return retpc; } +target_ulong helper_dret(CPURISCVState *env) +{ + uintptr_t ra = GETPC(); +#ifdef CONFIG_USER_ONLY + riscv_raise_exception(env, RISCV_EXCP_ILLEGAL_INST, ra); + return 0; +#else + if (!riscv_cpu_cfg(env)->ext_sdext || !env->debug_mode) { + riscv_raise_exception(env, RISCV_EXCP_ILLEGAL_INST, ra); + } + target_ulong retpc = env->dpc & get_xepc_mask(env); + riscv_cpu_leave_debug_mode(env); + return retpc; +#endif +} + target_ulong helper_mnret(CPURISCVState *env) { target_ulong retpc = env->mnepc; -- 2.53.0
RISC-V Debug Specification: https://github.com/riscv/riscv-debug-spec/releases/tag/1.0 Route EBREAK via helper_sdext_ebreak. If Sdext is enabled and the matching dcsr.ebreak* bit is set, enter Debug Mode with cause=ebreak and stop with EXCP_DEBUG. Otherwise keep the normal breakpoint trap. Signed-off-by: Chao Liu <chao.liu.zevorn@gmail.com> Reviewed-by: Daniel Henrique Barboza <daniel.barboza@oss.qualcomm.com> --- target/riscv/helper.h | 1 + .../riscv/insn_trans/trans_privileged.c.inc | 6 ++++ target/riscv/op_helper.c | 34 +++++++++++++++++++ 3 files changed, 41 insertions(+) diff --git a/target/riscv/helper.h b/target/riscv/helper.h index 6140b6340d..acff73051b 100644 --- a/target/riscv/helper.h +++ b/target/riscv/helper.h @@ -141,6 +141,7 @@ DEF_HELPER_1(tlb_flush_all, void, env) DEF_HELPER_4(ctr_add_entry, void, env, tl, tl, tl) /* Native Debug */ DEF_HELPER_1(itrigger_match, void, env) +DEF_HELPER_2(sdext_ebreak, void, env, tl) #endif /* Hypervisor functions */ diff --git a/target/riscv/insn_trans/trans_privileged.c.inc b/target/riscv/insn_trans/trans_privileged.c.inc index f8641b1977..84f0c77513 100644 --- a/target/riscv/insn_trans/trans_privileged.c.inc +++ b/target/riscv/insn_trans/trans_privileged.c.inc @@ -68,9 +68,15 @@ static bool trans_ebreak(DisasContext *ctx, arg_ebreak *a) if (pre == 0x01f01013 && ebreak == 0x00100073 && post == 0x40705013) { generate_exception(ctx, RISCV_EXCP_SEMIHOST); } else { +#ifndef CONFIG_USER_ONLY + gen_update_pc(ctx, 0); + gen_helper_sdext_ebreak(tcg_env, tcg_constant_tl(ebreak_addr)); + ctx->base.is_jmp = DISAS_NORETURN; +#else tcg_gen_st_tl(tcg_constant_tl(ebreak_addr), tcg_env, offsetof(CPURISCVState, badaddr)); generate_exception(ctx, RISCV_EXCP_BREAKPOINT); +#endif } return true; } diff --git a/target/riscv/op_helper.c b/target/riscv/op_helper.c index 99736bbebb..b6417b4b0b 100644 --- a/target/riscv/op_helper.c +++ b/target/riscv/op_helper.c @@ -470,6 +470,40 @@ target_ulong helper_dret(CPURISCVState *env) #endif } +void helper_sdext_ebreak(CPURISCVState *env, target_ulong pc) +{ + CPUState *cs = env_cpu(env); + bool enter_debug = false; + + if (riscv_cpu_cfg(env)->ext_sdext && !env->debug_mode) { + if (env->virt_enabled) { + if (env->priv == PRV_S) { + enter_debug = env->dcsr & DCSR_EBREAKVS; + } else if (env->priv == PRV_U) { + enter_debug = env->dcsr & DCSR_EBREAKVU; + } + } else { + if (env->priv == PRV_M) { + enter_debug = env->dcsr & DCSR_EBREAKM; + } else if (env->priv == PRV_S) { + enter_debug = env->dcsr & DCSR_EBREAKS; + } else if (env->priv == PRV_U) { + enter_debug = env->dcsr & DCSR_EBREAKU; + } + } + } + + env->badaddr = pc; + + if (enter_debug) { + riscv_cpu_enter_debug_mode(env, pc, DCSR_CAUSE_EBREAK); + cs->exception_index = EXCP_DEBUG; + cpu_loop_exit_restore(cs, GETPC()); + } + + riscv_raise_exception(env, RISCV_EXCP_BREAKPOINT, GETPC()); +} + target_ulong helper_mnret(CPURISCVState *env) { target_ulong retpc = env->mnepc; -- 2.53.0
RISC-V Debug Specification: https://github.com/riscv/riscv-debug-spec/releases/tag/1.0 Use a TB flag when dcsr.step is set (and we are not in Debug Mode). When the flag is on, build 1-insn TBs and do not chain to the next TB. Add a TB-exit helper that enters Debug Mode with cause=step and sets dpc to the next pc, then stops with EXCP_DEBUG. If dcsr.stepie is 0, do not take interrupts while stepping. Treat WFI as a nop so the hart does not sleep during a step. PS: This patch references Max Chou's handling of ext_tb_flags. https://lore.kernel.org/qemu-devel/20260108132631.9429-6-max.chou@sifive.com... Signed-off-by: Chao Liu <chao.liu.zevorn@gmail.com> Reviewed-by: Daniel Henrique Barboza <daniel.barboza@oss.qualcomm.com> --- include/exec/translation-block.h | 4 ++-- target/riscv/cpu.h | 2 ++ target/riscv/cpu_helper.c | 6 ++++++ target/riscv/helper.h | 1 + target/riscv/op_helper.c | 20 ++++++++++++++++++++ target/riscv/tcg/tcg-cpu.c | 5 +++++ target/riscv/translate.c | 16 ++++++++++++++-- 7 files changed, 50 insertions(+), 4 deletions(-) diff --git a/include/exec/translation-block.h b/include/exec/translation-block.h index 40cc699031..ee15608c89 100644 --- a/include/exec/translation-block.h +++ b/include/exec/translation-block.h @@ -64,8 +64,8 @@ struct TranslationBlock { * x86: the original user, the Code Segment virtual base, * arm: an extension of tb->flags, * s390x: instruction data for EXECUTE, - * sparc: the next pc of the instruction queue (for delay slots). - * riscv: an extension of tb->flags, + * sparc: the next pc of the instruction queue (for delay slots), + * riscv: an extension of tb->flags. */ uint64_t cs_base; diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h index 62732957a4..0d6b70c9f0 100644 --- a/target/riscv/cpu.h +++ b/target/riscv/cpu.h @@ -712,6 +712,8 @@ FIELD(TB_FLAGS, PM_SIGNEXTEND, 31, 1) FIELD(EXT_TB_FLAGS, MISA_EXT, 0, 32) FIELD(EXT_TB_FLAGS, ALTFMT, 32, 1) +/* sdext single-step needs a TB flag to build 1-insn TBs */ +FIELD(EXT_TB_FLAGS, SDEXT_STEP, 33, 1) #ifdef TARGET_RISCV32 #define riscv_cpu_mxl(env) ((void)(env), MXL_RV32) diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c index 0e266ff3a9..a0874f4e23 100644 --- a/target/riscv/cpu_helper.c +++ b/target/riscv/cpu_helper.c @@ -635,6 +635,12 @@ bool riscv_cpu_exec_interrupt(CPUState *cs, int interrupt_request) if (interrupt_request & mask) { RISCVCPU *cpu = RISCV_CPU(cs); CPURISCVState *env = &cpu->env; + + if (cpu->cfg.ext_sdext && !env->debug_mode && + (env->dcsr & DCSR_STEP) && !(env->dcsr & DCSR_STEPIE)) { + return false; + } + int interruptno = riscv_cpu_local_irq_pending(env); if (interruptno >= 0) { cs->exception_index = RISCV_EXCP_INT_FLAG | interruptno; diff --git a/target/riscv/helper.h b/target/riscv/helper.h index acff73051b..0b709c2b99 100644 --- a/target/riscv/helper.h +++ b/target/riscv/helper.h @@ -141,6 +141,7 @@ DEF_HELPER_1(tlb_flush_all, void, env) DEF_HELPER_4(ctr_add_entry, void, env, tl, tl, tl) /* Native Debug */ DEF_HELPER_1(itrigger_match, void, env) +DEF_HELPER_1(sdext_step, void, env) DEF_HELPER_2(sdext_ebreak, void, env, tl) #endif diff --git a/target/riscv/op_helper.c b/target/riscv/op_helper.c index b6417b4b0b..e7878d7aa4 100644 --- a/target/riscv/op_helper.c +++ b/target/riscv/op_helper.c @@ -470,6 +470,22 @@ target_ulong helper_dret(CPURISCVState *env) #endif } +void helper_sdext_step(CPURISCVState *env) +{ +#ifndef CONFIG_USER_ONLY + CPUState *cs = env_cpu(env); + + if (!riscv_cpu_cfg(env)->ext_sdext || env->debug_mode || + !(env->dcsr & DCSR_STEP)) { + return; + } + + riscv_cpu_enter_debug_mode(env, env->pc, DCSR_CAUSE_STEP); + cs->exception_index = EXCP_DEBUG; + cpu_loop_exit_restore(cs, GETPC()); +#endif +} + void helper_sdext_ebreak(CPURISCVState *env, target_ulong pc) { CPUState *cs = env_cpu(env); @@ -602,6 +618,10 @@ void helper_wfi(CPURISCVState *env) (prv_u || (prv_s && get_field(env->hstatus, HSTATUS_VTW)))) { riscv_raise_exception(env, RISCV_EXCP_VIRT_INSTRUCTION_FAULT, GETPC()); } else { + if (riscv_cpu_cfg(env)->ext_sdext && !env->debug_mode && + (env->dcsr & DCSR_STEP)) { + return; + } cs->halted = 1; cs->exception_index = EXCP_HLT; cpu_loop_exit(cs); diff --git a/target/riscv/tcg/tcg-cpu.c b/target/riscv/tcg/tcg-cpu.c index f80e3413f8..53d862080c 100644 --- a/target/riscv/tcg/tcg-cpu.c +++ b/target/riscv/tcg/tcg-cpu.c @@ -193,6 +193,11 @@ static TCGTBCPUState riscv_get_tb_cpu_state(CPUState *cs) flags = FIELD_DP32(flags, TB_FLAGS, PM_SIGNEXTEND, pm_signext); ext_flags = FIELD_DP64(ext_flags, EXT_TB_FLAGS, MISA_EXT, env->misa_ext); +#ifndef CONFIG_USER_ONLY + if (cpu->cfg.ext_sdext && !env->debug_mode && (env->dcsr & DCSR_STEP)) { + ext_flags = FIELD_DP64(ext_flags, EXT_TB_FLAGS, SDEXT_STEP, 1); + } +#endif return (TCGTBCPUState){ .pc = env->xl == MXL_RV32 ? env->pc & UINT32_MAX : env->pc, diff --git a/target/riscv/translate.c b/target/riscv/translate.c index f687c75fe4..c222b4bb06 100644 --- a/target/riscv/translate.c +++ b/target/riscv/translate.c @@ -110,6 +110,8 @@ typedef struct DisasContext { bool ztso; /* Use icount trigger for native debug */ bool itrigger; + /* Enter Debug Mode after next instruction (sdext single-step). */ + bool sdext_step; /* FRM is known to contain a valid value. */ bool frm_valid; bool insn_start_updated; @@ -284,6 +286,9 @@ static void lookup_and_goto_ptr(DisasContext *ctx) if (ctx->itrigger) { gen_helper_itrigger_match(tcg_env); } + if (ctx->sdext_step) { + gen_helper_sdext_step(tcg_env); + } #endif tcg_gen_lookup_and_goto_ptr(); } @@ -294,6 +299,9 @@ static void exit_tb(DisasContext *ctx) if (ctx->itrigger) { gen_helper_itrigger_match(tcg_env); } + if (ctx->sdext_step) { + gen_helper_sdext_step(tcg_env); + } #endif tcg_gen_exit_tb(NULL, 0); } @@ -307,7 +315,8 @@ static void gen_goto_tb(DisasContext *ctx, unsigned tb_slot_idx, * Under itrigger, instruction executes one by one like singlestep, * direct block chain benefits will be small. */ - if (translator_use_goto_tb(&ctx->base, dest) && !ctx->itrigger) { + if (translator_use_goto_tb(&ctx->base, dest) && + !ctx->itrigger && !ctx->sdext_step) { /* * For pcrel, the pc must always be up-to-date on entry to * the linked TB, so that it can use simple additions for all @@ -1302,6 +1311,7 @@ static void riscv_tr_init_disas_context(DisasContextBase *dcbase, CPUState *cs) RISCVCPUClass *mcc = RISCV_CPU_GET_CLASS(cs); RISCVCPU *cpu = RISCV_CPU(cs); uint32_t tb_flags = ctx->base.tb->flags; + uint64_t ext_tb_flags = ctx->base.tb->cs_base; ctx->pc_save = ctx->base.pc_first; ctx->priv = FIELD_EX32(tb_flags, TB_FLAGS, PRIV); @@ -1338,6 +1348,7 @@ static void riscv_tr_init_disas_context(DisasContextBase *dcbase, CPUState *cs) ctx->bcfi_enabled = FIELD_EX32(tb_flags, TB_FLAGS, BCFI_ENABLED); ctx->fcfi_lp_expected = FIELD_EX32(tb_flags, TB_FLAGS, FCFI_LP_EXPECTED); ctx->fcfi_enabled = FIELD_EX32(tb_flags, TB_FLAGS, FCFI_ENABLED); + ctx->sdext_step = FIELD_EX64(ext_tb_flags, EXT_TB_FLAGS, SDEXT_STEP); ctx->zero = tcg_constant_tl(0); ctx->virt_inst_excp = false; ctx->decoders = cpu->decoders; @@ -1388,7 +1399,8 @@ static void riscv_tr_translate_insn(DisasContextBase *dcbase, CPUState *cpu) /* Only the first insn within a TB is allowed to cross a page boundary. */ if (ctx->base.is_jmp == DISAS_NEXT) { - if (ctx->itrigger || !translator_is_same_page(&ctx->base, ctx->base.pc_next)) { + if (ctx->itrigger || ctx->sdext_step || + !translator_is_same_page(&ctx->base, ctx->base.pc_next)) { ctx->base.is_jmp = DISAS_TOO_MANY; } else { unsigned page_ofs = ctx->base.pc_next & ~TARGET_PAGE_MASK; -- 2.53.0
RISC-V Debug Specification: https://github.com/riscv/riscv-debug-spec/releases/tag/1.0 Allow mcontrol/mcontrol6 action=1 when Sdext is enabled. When such a trigger hits, enter Debug Mode with cause=trigger and stop with EXCP_DEBUG. Also report inst-count triggers in tinfo and read their action field. Signed-off-by: Chao Liu <chao.liu.zevorn@gmail.com> Reviewed-by: Daniel Henrique Barboza <daniel.barboza@oss.qualcomm.com> --- target/riscv/debug.c | 53 ++++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 51 insertions(+), 2 deletions(-) diff --git a/target/riscv/debug.c b/target/riscv/debug.c index 5877a60c50..6c69c2f796 100644 --- a/target/riscv/debug.c +++ b/target/riscv/debug.c @@ -110,6 +110,8 @@ static trigger_action_t get_trigger_action(CPURISCVState *env, action = (tdata1 & TYPE6_ACTION) >> 12; break; case TRIGGER_TYPE_INST_CNT: + action = tdata1 & ITRIGGER_ACTION; + break; case TRIGGER_TYPE_INT: case TRIGGER_TYPE_EXCP: case TRIGGER_TYPE_EXT_SRC: @@ -280,6 +282,7 @@ static target_ulong textra_validate(CPURISCVState *env, target_ulong tdata3) static void do_trigger_action(CPURISCVState *env, target_ulong trigger_index) { + CPUState *cs = env_cpu(env); trigger_action_t action = get_trigger_action(env, trigger_index); switch (action) { @@ -289,6 +292,21 @@ static void do_trigger_action(CPURISCVState *env, target_ulong trigger_index) riscv_raise_exception(env, RISCV_EXCP_BREAKPOINT, 0); break; case DBG_ACTION_DBG_MODE: + if (!env_archcpu(env)->cfg.ext_sdext) { + qemu_log_mask(LOG_GUEST_ERROR, + "trigger action=debug mode requires Sdext\n"); + riscv_raise_exception(env, RISCV_EXCP_BREAKPOINT, 0); + } + riscv_cpu_enter_debug_mode(env, env->pc, DCSR_CAUSE_TRIGGER); + /* + * If this came from the Trigger Module's CPU breakpoint/watchpoint, + * we're already returning via EXCP_DEBUG. Otherwise, stop now. + */ + if (cs->exception_index != EXCP_DEBUG) { + cs->exception_index = EXCP_DEBUG; + cpu_loop_exit_restore(cs, GETPC()); + } + break; case DBG_ACTION_TRACE0: case DBG_ACTION_TRACE1: case DBG_ACTION_TRACE2: @@ -441,6 +459,7 @@ static target_ulong type2_mcontrol_validate(CPURISCVState *env, { target_ulong val; uint32_t size; + uint32_t action; /* validate the generic part first */ val = tdata1_validate(env, ctrl, TRIGGER_TYPE_AD_MATCH); @@ -448,11 +467,25 @@ static target_ulong type2_mcontrol_validate(CPURISCVState *env, /* validate unimplemented (always zero) bits */ warn_always_zero_bit(ctrl, TYPE2_MATCH, "match"); warn_always_zero_bit(ctrl, TYPE2_CHAIN, "chain"); - warn_always_zero_bit(ctrl, TYPE2_ACTION, "action"); warn_always_zero_bit(ctrl, TYPE2_TIMING, "timing"); warn_always_zero_bit(ctrl, TYPE2_SELECT, "select"); warn_always_zero_bit(ctrl, TYPE2_HIT, "hit"); + action = (ctrl & TYPE2_ACTION) >> 12; + if (action == DBG_ACTION_BP) { + val |= ctrl & TYPE2_ACTION; + } else if (action == DBG_ACTION_DBG_MODE) { + if (env_archcpu(env)->cfg.ext_sdext) { + val |= ctrl & TYPE2_ACTION; + } else { + qemu_log_mask(LOG_GUEST_ERROR, + "trigger action=debug mode requires Sdext\n"); + } + } else { + qemu_log_mask(LOG_UNIMP, "trigger action: %u is not supported\n", + action); + } + /* validate size encoding */ size = type2_breakpoint_size(env, ctrl); if (access_size[size] == -1) { @@ -569,6 +602,7 @@ static target_ulong type6_mcontrol6_validate(CPURISCVState *env, { target_ulong val; uint32_t size; + uint32_t action; /* validate the generic part first */ val = tdata1_validate(env, ctrl, TRIGGER_TYPE_AD_MATCH6); @@ -576,11 +610,25 @@ static target_ulong type6_mcontrol6_validate(CPURISCVState *env, /* validate unimplemented (always zero) bits */ warn_always_zero_bit(ctrl, TYPE6_MATCH, "match"); warn_always_zero_bit(ctrl, TYPE6_CHAIN, "chain"); - warn_always_zero_bit(ctrl, TYPE6_ACTION, "action"); warn_always_zero_bit(ctrl, TYPE6_TIMING, "timing"); warn_always_zero_bit(ctrl, TYPE6_SELECT, "select"); warn_always_zero_bit(ctrl, TYPE6_HIT, "hit"); + action = (ctrl & TYPE6_ACTION) >> 12; + if (action == DBG_ACTION_BP) { + val |= ctrl & TYPE6_ACTION; + } else if (action == DBG_ACTION_DBG_MODE) { + if (env_archcpu(env)->cfg.ext_sdext) { + val |= ctrl & TYPE6_ACTION; + } else { + qemu_log_mask(LOG_GUEST_ERROR, + "trigger action=debug mode requires Sdext\n"); + } + } else { + qemu_log_mask(LOG_UNIMP, "trigger action: %u is not supported\n", + action); + } + /* validate size encoding */ size = extract32(ctrl, 16, 4); if (access_size[size] == -1) { @@ -919,6 +967,7 @@ target_ulong tinfo_csr_read(CPURISCVState *env) { /* assume all triggers support the same types of triggers */ return BIT(TRIGGER_TYPE_AD_MATCH) | + BIT(TRIGGER_TYPE_INST_CNT) | BIT(TRIGGER_TYPE_AD_MATCH6); } -- 2.53.0
On 2/3/2026 9:55 AM, Chao Liu wrote:
Hi,
Per Daniel's review feedback, this v5 series is based on Alistair's riscv-to-apply.next branch [3]. It depends on patches 1-5 of Max Chou's "Add Zvfbfa extension support" v2 series (not included; apply them first) [2].
It is based on RISC-V Debug Specification 1.0 [1].
It introduces the sdext/sdtrig config bits, DCSR/DPC/DSCRATCH state, Debug Mode enter/leave helpers, DRET, EBREAK entry, single-step, and trigger action=debug mode.
To reduce review load, this series focuses on the Sdext features first. The Debug Module (DM) and related flows will follow in a later series.
We still need a reviewed-by in patch 1 (that I can't give, after all it's my patch hehe). Aside from that the series LGTM. It's not sensible to think that we're going to push upstream an error/bug free extension. The code will need more mileage with more people using it and we'll fix what is needed along the way. We have a long time until the release anyway. My vote is to queue this up (we'll need Max's series queued first). When this lands upstream we'll go back to the Server SoC reference board and get that upstreamed too. Thanks, Daniel
Changes in v5: - Patch 2: removed redundant `cpu->cfg.ext_sdext = false;` in riscv_cpu_init() since it's already set to false in MULTI_EXT_CFG_BOOL. (Daniel) - Patch 7: changed LOG_UNIMP to LOG_GUEST_ERROR for invalid trigger action=debug mode when Sdext is not enabled. LOG_UNIMP is reserved for unimplemented features, while LOG_GUEST_ERROR is for invalid guest actions. (Daniel)
Changes in v4: - Fixed linux-user build errors: moved debug_mode/dcsr/dpc/dscratch fields inside #ifndef CONFIG_USER_ONLY block in cpu.h, and wrapped all code using these fields with proper guards. (Daniel) - Updated dependency from single patch (patch 5 only) to patches 1-5 of Max Chou's "Add Zvfbfa extension support" v2 series to avoid compilation errors.
Changes in v3: - Rebase onto Alistair's riscv-to-apply.next branch. - Depend on Max Chou's patch "target/riscv: Use the tb->cs_bqse as the extend tb flags." (not included; apply it first) [2]. - Patch 2: default sdext to false in riscv_cpu_extensions[].
Changes in v2: - Drop the RHCT expected AML update from this series. - Replace the split sdext/sdtrig config bits patch with deprecating the 'debug' CPU property. - Rebase and update patch subjects to target/riscv prefix.
Changes in v1: - Debug Mode entry/exit updates DCSR/DPC and restores execution via DRET. - EBREAK honors DCSR ebreak bits and enters Debug Mode when enabled. - Single-step uses DCSR.STEP with a TB flag and a helper at TB exit. It references Max Chou's patch "target/riscv: Use the tb->cs_bqse as the extend tb flags." [2]. - Sdtrig supports action=debug mode for mcontrol/mcontrol6 and reports inst-count triggers in tinfo.
Differences vs Debug Spec (known gaps): - No Debug Module (no DMI, dmcontrol/dmstatus, haltreq/resumereq). - No debug ROM, program buffer, abstract commands, or SBA. - Resume is modeled by leaving Debug Mode at cpu_exec_enter. - Step/exception ordering is simplified: if the stepped instruction traps, the normal exception is taken and Debug Mode is not forced. - Several DCSR fields are not fully modeled (stopcount/stoptime, etc).
Roadmap (next stage, DM focus): 1) Add a DM core with DMI access and hart state tracking. 2) Implement halt/resume handshake and move Debug Mode transitions under DM control. 3) Add debug ROM, program buffer, and abstract commands for GPR/CSR and memory access. 4) Add SBA if required by tooling. 5) Tighten ordering rules for step/exception/trigger priorities.
References: [1] https://github.com/riscv/riscv-debug-spec/releases/tag/1.0 [2] https://lore.kernel.org/qemu-devel/20260108132631.9429-1-max.chou@sifive.com... [3] https://github.com/alistair23/qemu/tree/riscv-to-apply.next
Thanks, Chao
Chao Liu (6): target/riscv: add sdext debug CSRs state target/riscv: add sdext Debug Mode helpers target/riscv: add dret instruction target/riscv: add sdext enter Debug Mode on ebreak target/riscv: add sdext single-step support target/riscv: add sdtrig trigger action=debug mode
Daniel Henrique Barboza (1): target/riscv: deprecate 'debug' CPU property
docs/about/deprecated.rst | 7 + include/exec/translation-block.h | 4 +- target/riscv/cpu.c | 59 +++++++- target/riscv/cpu.h | 9 ++ target/riscv/cpu_bits.h | 33 +++++ target/riscv/cpu_cfg_fields.h.inc | 3 +- target/riscv/cpu_helper.c | 90 ++++++++++++ target/riscv/csr.c | 128 +++++++++++++++++- target/riscv/debug.c | 58 +++++++- target/riscv/helper.h | 3 + target/riscv/insn32.decode | 1 + .../riscv/insn_trans/trans_privileged.c.inc | 24 ++++ target/riscv/machine.c | 44 ++++-- target/riscv/op_helper.c | 70 ++++++++++ target/riscv/tcg/tcg-cpu.c | 21 ++- target/riscv/translate.c | 16 ++- 16 files changed, 545 insertions(+), 25 deletions(-)
-- 2.53.0
On Mon, Feb 09, 2026 at 04:21:09PM -0300, Daniel Henrique Barboza wrote:
On 2/3/2026 9:55 AM, Chao Liu wrote:
Hi,
Per Daniel's review feedback, this v5 series is based on Alistair's riscv-to-apply.next branch [3]. It depends on patches 1-5 of Max Chou's "Add Zvfbfa extension support" v2 series (not included; apply them first) [2].
It is based on RISC-V Debug Specification 1.0 [1].
It introduces the sdext/sdtrig config bits, DCSR/DPC/DSCRATCH state, Debug Mode enter/leave helpers, DRET, EBREAK entry, single-step, and trigger action=debug mode.
To reduce review load, this series focuses on the Sdext features first. The Debug Module (DM) and related flows will follow in a later series.
We still need a reviewed-by in patch 1 (that I can't give, after all it's my patch hehe).
Aside from that the series LGTM. It's not sensible to think that we're going to push upstream an error/bug free extension. The code will need more mileage with more people using it and we'll fix what is needed along the way. We have a long time until the release anyway.
My vote is to queue this up (we'll need Max's series queued first). When this lands upstream we'll go back to the Server SoC reference board and get that upstreamed too.
I agree with your points. I’ve also added a Reviewed-by sign-off to patch1. Thanks, Chao
Thanks, Daniel
Changes in v5: - Patch 2: removed redundant `cpu->cfg.ext_sdext = false;` in riscv_cpu_init() since it's already set to false in MULTI_EXT_CFG_BOOL. (Daniel) - Patch 7: changed LOG_UNIMP to LOG_GUEST_ERROR for invalid trigger action=debug mode when Sdext is not enabled. LOG_UNIMP is reserved for unimplemented features, while LOG_GUEST_ERROR is for invalid guest actions. (Daniel)
Changes in v4: - Fixed linux-user build errors: moved debug_mode/dcsr/dpc/dscratch fields inside #ifndef CONFIG_USER_ONLY block in cpu.h, and wrapped all code using these fields with proper guards. (Daniel) - Updated dependency from single patch (patch 5 only) to patches 1-5 of Max Chou's "Add Zvfbfa extension support" v2 series to avoid compilation errors.
Changes in v3: - Rebase onto Alistair's riscv-to-apply.next branch. - Depend on Max Chou's patch "target/riscv: Use the tb->cs_bqse as the extend tb flags." (not included; apply it first) [2]. - Patch 2: default sdext to false in riscv_cpu_extensions[].
Changes in v2: - Drop the RHCT expected AML update from this series. - Replace the split sdext/sdtrig config bits patch with deprecating the 'debug' CPU property. - Rebase and update patch subjects to target/riscv prefix.
Changes in v1: - Debug Mode entry/exit updates DCSR/DPC and restores execution via DRET. - EBREAK honors DCSR ebreak bits and enters Debug Mode when enabled. - Single-step uses DCSR.STEP with a TB flag and a helper at TB exit. It references Max Chou's patch "target/riscv: Use the tb->cs_bqse as the extend tb flags." [2]. - Sdtrig supports action=debug mode for mcontrol/mcontrol6 and reports inst-count triggers in tinfo.
Differences vs Debug Spec (known gaps): - No Debug Module (no DMI, dmcontrol/dmstatus, haltreq/resumereq). - No debug ROM, program buffer, abstract commands, or SBA. - Resume is modeled by leaving Debug Mode at cpu_exec_enter. - Step/exception ordering is simplified: if the stepped instruction traps, the normal exception is taken and Debug Mode is not forced. - Several DCSR fields are not fully modeled (stopcount/stoptime, etc).
Roadmap (next stage, DM focus): 1) Add a DM core with DMI access and hart state tracking. 2) Implement halt/resume handshake and move Debug Mode transitions under DM control. 3) Add debug ROM, program buffer, and abstract commands for GPR/CSR and memory access. 4) Add SBA if required by tooling. 5) Tighten ordering rules for step/exception/trigger priorities.
References: [1] https://github.com/riscv/riscv-debug-spec/releases/tag/1.0 [2] https://lore.kernel.org/qemu-devel/20260108132631.9429-1-max.chou@sifive.com... [3] https://github.com/alistair23/qemu/tree/riscv-to-apply.next
Thanks, Chao
Chao Liu (6): target/riscv: add sdext debug CSRs state target/riscv: add sdext Debug Mode helpers target/riscv: add dret instruction target/riscv: add sdext enter Debug Mode on ebreak target/riscv: add sdext single-step support target/riscv: add sdtrig trigger action=debug mode
Daniel Henrique Barboza (1): target/riscv: deprecate 'debug' CPU property
docs/about/deprecated.rst | 7 + include/exec/translation-block.h | 4 +- target/riscv/cpu.c | 59 +++++++- target/riscv/cpu.h | 9 ++ target/riscv/cpu_bits.h | 33 +++++ target/riscv/cpu_cfg_fields.h.inc | 3 +- target/riscv/cpu_helper.c | 90 ++++++++++++ target/riscv/csr.c | 128 +++++++++++++++++- target/riscv/debug.c | 58 +++++++- target/riscv/helper.h | 3 + target/riscv/insn32.decode | 1 + .../riscv/insn_trans/trans_privileged.c.inc | 24 ++++ target/riscv/machine.c | 44 ++++-- target/riscv/op_helper.c | 70 ++++++++++ target/riscv/tcg/tcg-cpu.c | 21 ++- target/riscv/translate.c | 16 ++- 16 files changed, 545 insertions(+), 25 deletions(-)
-- 2.53.0
participants (2)
-
Chao Liu -
Daniel Henrique Barboza