* [PATCH v3 1/2] target/riscv: Mark debug property as deprecated
2024-02-29 13:37 [PATCH v3 0/2] Export debug triggers as an extension Himanshu Chauhan
@ 2024-02-29 13:37 ` Himanshu Chauhan
2024-02-29 13:37 ` [PATCH v3 2/2] target/riscv: Export sdtrig in ISA string Himanshu Chauhan
2024-02-29 15:12 ` [PATCH v3 0/2] Export debug triggers as an extension Andrew Jones
2 siblings, 0 replies; 6+ messages in thread
From: Himanshu Chauhan @ 2024-02-29 13:37 UTC (permalink / raw)
To: qemu-riscv, qemu-devel; +Cc: Himanshu Chauhan
The debug property implements the ratified debug specification v0.13.
This specification is superseded by (now frozen) RISC-V debug specification v1.0
It defines sdtrig ISA extension which is forward and backward comptible with
the debug specification v0.13.
This patch deprecates the debug property and replaces with ext_sdtrig.
A deprecation warning is displayed if debug property is used.
Signed-off-by: Himanshu Chauhan <hchauhan@ventanamicro.com>
---
target/riscv/cpu.c | 37 ++++++++++++++++++++++++++++++++++---
target/riscv/cpu_cfg.h | 2 +-
target/riscv/cpu_helper.c | 2 +-
target/riscv/csr.c | 2 +-
target/riscv/machine.c | 2 +-
5 files changed, 38 insertions(+), 7 deletions(-)
diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
index 5ff0192c52..5d5d8f0375 100644
--- a/target/riscv/cpu.c
+++ b/target/riscv/cpu.c
@@ -996,7 +996,7 @@ static void riscv_cpu_reset_hold(Object *obj)
set_default_nan_mode(1, &env->fp_status);
#ifndef CONFIG_USER_ONLY
- if (cpu->cfg.debug) {
+ if (cpu->cfg.ext_sdtrig) {
riscv_trigger_reset_hold(env);
}
@@ -1156,7 +1156,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
@@ -1718,6 +1718,37 @@ static const PropertyInfo prop_mmu = {
.set = prop_mmu_set,
};
+static void prop_debug_set(Object *obj, Visitor *v, const char *name,
+ void *opaque, Error **errp)
+{
+ RISCVCPU *cpu = RISCV_CPU(obj);
+ bool value;
+
+ warn_report("\"debug\" property is being deprecated.");
+
+ visit_type_bool(v, name, &value, errp);
+
+ if (cpu->cfg.ext_sdtrig != value && !riscv_cpu_is_dynamic(obj)) {
+ return;
+ }
+
+ cpu->cfg.ext_sdtrig = value;
+}
+
+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);
+}
+
+static const PropertyInfo prop_debug = {
+ .name = "debug",
+ .get = prop_debug_get,
+ .set = prop_debug_set,
+};
+
static void prop_pmp_set(Object *obj, Visitor *v, const char *name,
void *opaque, Error **errp)
{
@@ -2229,7 +2260,7 @@ RISCVCPUProfile *riscv_profiles[] = {
};
static Property riscv_cpu_properties[] = {
- DEFINE_PROP_BOOL("debug", RISCVCPU, cfg.debug, true),
+ {.name = "debug", .info = &prop_debug}, /* Deprecated */
{.name = "pmu-mask", .info = &prop_pmu_mask},
{.name = "pmu-num", .info = &prop_pmu_num}, /* Deprecated */
diff --git a/target/riscv/cpu_cfg.h b/target/riscv/cpu_cfg.h
index 833bf58217..9fb4ca577f 100644
--- a/target/riscv/cpu_cfg.h
+++ b/target/riscv/cpu_cfg.h
@@ -113,6 +113,7 @@ struct RISCVCPUConfig {
bool ext_zvfbfwma;
bool ext_zvfh;
bool ext_zvfhmin;
+ bool ext_sdtrig;
bool ext_smaia;
bool ext_ssaia;
bool ext_sscofpmf;
@@ -148,7 +149,6 @@ struct RISCVCPUConfig {
uint16_t cboz_blocksize;
bool mmu;
bool pmp;
- bool debug;
bool misa_w;
bool short_isa_string;
diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
index d462d95ee1..b2ad97d601 100644
--- a/target/riscv/cpu_helper.c
+++ b/target/riscv/cpu_helper.c
@@ -129,7 +129,7 @@ void cpu_get_tb_cpu_state(CPURISCVState *env, vaddr *pc,
? 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
diff --git a/target/riscv/csr.c b/target/riscv/csr.c
index d4e8ac13b9..e60599d74e 100644
--- a/target/riscv/csr.c
+++ b/target/riscv/csr.c
@@ -546,7 +546,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 81cf22894e..1b775342d2 100644
--- a/target/riscv/machine.c
+++ b/target/riscv/machine.c
@@ -230,7 +230,7 @@ static bool debug_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)
--
2.34.1
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH v3 2/2] target/riscv: Export sdtrig in ISA string
2024-02-29 13:37 [PATCH v3 0/2] Export debug triggers as an extension Himanshu Chauhan
2024-02-29 13:37 ` [PATCH v3 1/2] target/riscv: Mark debug property as deprecated Himanshu Chauhan
@ 2024-02-29 13:37 ` Himanshu Chauhan
2024-02-29 15:12 ` [PATCH v3 0/2] Export debug triggers as an extension Andrew Jones
2 siblings, 0 replies; 6+ messages in thread
From: Himanshu Chauhan @ 2024-02-29 13:37 UTC (permalink / raw)
To: qemu-riscv, qemu-devel; +Cc: Himanshu Chauhan
This patch adds "sdtrig" in the ISA string when sdtrig extension is enabled.
The sdtrig extension may or may not be implemented in a system. Therefore, the
-cpu rv64,sdtrig=<true/false>
option can be used to dynamically turn sdtrig extension on or off.
Signed-off-by: Himanshu Chauhan <hchauhan@ventanamicro.com>
---
target/riscv/cpu.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
index 5d5d8f0375..6f98c0195c 100644
--- a/target/riscv/cpu.c
+++ b/target/riscv/cpu.c
@@ -1461,6 +1461,7 @@ const RISCVCPUMultiExtConfig riscv_cpu_extensions[] = {
MULTI_EXT_CFG_BOOL("zve64d", ext_zve64d, false),
MULTI_EXT_CFG_BOOL("sstc", ext_sstc, true),
+ MULTI_EXT_CFG_BOOL("sdtrig", ext_sdtrig, true),
MULTI_EXT_CFG_BOOL("smepmp", ext_smepmp, false),
MULTI_EXT_CFG_BOOL("smstateen", ext_smstateen, false),
MULTI_EXT_CFG_BOOL("svadu", ext_svadu, true),
@@ -1724,7 +1725,7 @@ static void prop_debug_set(Object *obj, Visitor *v, const char *name,
RISCVCPU *cpu = RISCV_CPU(obj);
bool value;
- warn_report("\"debug\" property is being deprecated.");
+ warn_report("\"debug\" property is deprecated; use \"sdtrig\"");
visit_type_bool(v, name, &value, errp);
--
2.34.1
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH v3 0/2] Export debug triggers as an extension
2024-02-29 13:37 [PATCH v3 0/2] Export debug triggers as an extension Himanshu Chauhan
2024-02-29 13:37 ` [PATCH v3 1/2] target/riscv: Mark debug property as deprecated Himanshu Chauhan
2024-02-29 13:37 ` [PATCH v3 2/2] target/riscv: Export sdtrig in ISA string Himanshu Chauhan
@ 2024-02-29 15:12 ` Andrew Jones
2024-02-29 16:19 ` Anup Patel
2 siblings, 1 reply; 6+ messages in thread
From: Andrew Jones @ 2024-02-29 15:12 UTC (permalink / raw)
To: Himanshu Chauhan; +Cc: qemu-riscv, qemu-devel
On Thu, Feb 29, 2024 at 07:07:43PM +0530, Himanshu Chauhan wrote:
> All the CPUs may or may not implement the debug triggers (sdtrig)
> extension. The presence of it should be dynamically detectable.
> This patch exports the debug triggers as an extension which
> can be turned on or off by sdtrig=<true/false> option. It is
> turned on by default.
>
> "sdtrig" is concatenated to ISA string when it is enabled.
> Like so:
> rv64imafdch_zicbom_*_sdtrig_*_sstc_svadu
>
> Changes from v1:
> - Replaced the debug property with ext_sdtrig
> - Marked it experimenatal by naming it x-sdtrig
> - x-sdtrig is added to ISA string
> - Reversed the patch order
>
> Changes from v2:
> - Mark debug property as deprecated and replace internally with sdtrig extension
I'm getting lost in our discussions, but I thought we needed both in case
a machine only implements debug 0.13, since sdtrig is at least 'more than'
debug, even if backwards compatible (which I also wasn't sure was the
case). If, OTOH, QEMU's debug implementation exactly implements sdtrig's
specification, then I'm in favor of deprecating the 'debug' extension.
Thanks,
drew
> - setting/unsetting debug property shows warning and sets/unsets ext_sdtrig
> - sdtrig is added to ISA string as RISC-V debug specification is frozen
>
> Himanshu Chauhan (2):
> target/riscv: Mark debug property as deprecated
> target/riscv: Export sdtrig in ISA string
>
> target/riscv/cpu.c | 38 +++++++++++++++++++++++++++++++++++---
> target/riscv/cpu_cfg.h | 2 +-
> target/riscv/cpu_helper.c | 2 +-
> target/riscv/csr.c | 2 +-
> target/riscv/machine.c | 2 +-
> 5 files changed, 39 insertions(+), 7 deletions(-)
>
> --
> 2.34.1
>
>
^ permalink raw reply [flat|nested] 6+ messages in thread