* [PATCH v7 0/4] Introduce sdtrig ISA extension
@ 2024-03-14 18:59 Himanshu Chauhan
2024-03-14 18:59 ` [PATCH v7 1/4] target/riscv: Check for valid itimer pointer before free Himanshu Chauhan
` (3 more replies)
0 siblings, 4 replies; 11+ messages in thread
From: Himanshu Chauhan @ 2024-03-14 18:59 UTC (permalink / raw)
To: qemu-riscv, qemu-devel; +Cc: ajones
All the CPUs may or may not implement the debug triggers. Some CPUs
may implement only debug specification v0.13 and not sdtrig ISA
extension.
This patchset, adds sdtrig ISA as an extension which can be turned on or off by
sdtrig=<true/false> option. It is turned off by default.
When debug is true and sdtrig is false, the behaviour is as defined in debug
specification v0.13. If sdtrig is turned on, the behaviour is as defined
in the sdtrig ISA extension.
The "sdtrig" string is concatenated to ISA string when debug or sdtrig is enabled.
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
- setting/unsetting debug property shows warning and sets/unsets ext_sdtrig
- sdtrig is added to ISA string as RISC-V debug specification is frozen
Changes from v3:
- debug propery is not deprecated but it is superceded by sdtrig extension
- Mcontrol6 support is not published when only debug property is turned
on as debug spec v0.13 doesn't define mcontrol6 match triggers.
- Enabling sdtrig extension turns of debug property and a warning is printed.
This doesn't break debug specification implemenation since sdtrig is
backward compatible with debug specification.
- Disable debug property and enable sdtrig by default for Ventana's Veyron
CPUs.
Changes from v4:
- Enable debug flag if sdtrig was enabled but debug was disabled.
- Other cosmetic changes.
Changes from v5:
- Addressed comments from Andrew Jones
Changes from v6:
- Cosmetic changes. All patches were run through checkpatch.pl.
No errors/warning.
- Remove all debug || ext_sdtrig references. All decisions are based on
debug flag alone
- Added null check before itimers are deleted. Without this check a
crash is observed.
Himanshu Chauhan (4):
target/riscv: Check for valid itimer pointer before free
target/riscv: Enable mcontrol6 triggers only when sdtrig is selected
target/riscv: Expose sdtrig ISA extension
target/riscv: Enable sdtrig for Ventana's Veyron CPUs
target/riscv/cpu.c | 8 ++++++++
target/riscv/cpu_cfg.h | 1 +
target/riscv/debug.c | 35 +++++++++++++++++++++++++++++------
3 files changed, 38 insertions(+), 6 deletions(-)
--
2.34.1
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH v7 1/4] target/riscv: Check for valid itimer pointer before free
2024-03-14 18:59 [PATCH v7 0/4] Introduce sdtrig ISA extension Himanshu Chauhan
@ 2024-03-14 18:59 ` Himanshu Chauhan
2024-04-29 4:04 ` Alistair Francis
2024-03-14 18:59 ` [PATCH v7 2/4] target/riscv: Enable mcontrol6 triggers only when sdtrig is selected Himanshu Chauhan
` (2 subsequent siblings)
3 siblings, 1 reply; 11+ messages in thread
From: Himanshu Chauhan @ 2024-03-14 18:59 UTC (permalink / raw)
To: qemu-riscv, qemu-devel; +Cc: ajones
Check if each element of array of pointers for itimer contains a non-null
pointer before freeing.
Signed-off-by: Himanshu Chauhan <hchauhan@ventanamicro.com>
---
target/riscv/debug.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/target/riscv/debug.c b/target/riscv/debug.c
index e30d99cc2f..5f14b39b06 100644
--- a/target/riscv/debug.c
+++ b/target/riscv/debug.c
@@ -938,7 +938,10 @@ void riscv_trigger_reset_hold(CPURISCVState *env)
env->tdata3[i] = 0;
env->cpu_breakpoint[i] = NULL;
env->cpu_watchpoint[i] = NULL;
- timer_del(env->itrigger_timer[i]);
+ if (env->itrigger_timer[i]) {
+ timer_del(env->itrigger_timer[i]);
+ env->itrigger_timer[i] = NULL;
+ }
}
env->mcontext = 0;
--
2.34.1
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH v7 2/4] target/riscv: Enable mcontrol6 triggers only when sdtrig is selected
2024-03-14 18:59 [PATCH v7 0/4] Introduce sdtrig ISA extension Himanshu Chauhan
2024-03-14 18:59 ` [PATCH v7 1/4] target/riscv: Check for valid itimer pointer before free Himanshu Chauhan
@ 2024-03-14 18:59 ` Himanshu Chauhan
2024-03-14 19:29 ` Andrew Jones
2024-04-29 4:08 ` Alistair Francis
2024-03-14 18:59 ` [PATCH v7 3/4] target/riscv: Expose sdtrig ISA extension Himanshu Chauhan
2024-03-14 18:59 ` [PATCH v7 4/4] target/riscv: Enable sdtrig for Ventana's Veyron CPUs Himanshu Chauhan
3 siblings, 2 replies; 11+ messages in thread
From: Himanshu Chauhan @ 2024-03-14 18:59 UTC (permalink / raw)
To: qemu-riscv, qemu-devel; +Cc: ajones
The mcontrol6 triggers are not defined in debug specification v0.13
These triggers are defined in sdtrig ISA extension.
This patch:
* Adds ext_sdtrig capability which is used to select mcontrol6 triggers
* Keeps the debug property. All triggers that are defined in v0.13 are
exposed.
Signed-off-by: Himanshu Chauhan <hchauhan@ventanamicro.com>
---
target/riscv/cpu.c | 5 +++++
target/riscv/cpu_cfg.h | 1 +
target/riscv/debug.c | 30 +++++++++++++++++++++++++-----
3 files changed, 31 insertions(+), 5 deletions(-)
diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
index c160b9216b..ab631500ac 100644
--- a/target/riscv/cpu.c
+++ b/target/riscv/cpu.c
@@ -1008,6 +1008,11 @@ static void riscv_cpu_reset_hold(Object *obj)
set_default_nan_mode(1, &env->fp_status);
#ifndef CONFIG_USER_ONLY
+ if (!cpu->cfg.debug && cpu->cfg.ext_sdtrig) {
+ warn_report("Enabling 'debug' since 'sdtrig' is enabled.");
+ cpu->cfg.debug = true;
+ }
+
if (cpu->cfg.debug) {
riscv_trigger_reset_hold(env);
}
diff --git a/target/riscv/cpu_cfg.h b/target/riscv/cpu_cfg.h
index 2040b90da0..0c57e1acd4 100644
--- a/target/riscv/cpu_cfg.h
+++ b/target/riscv/cpu_cfg.h
@@ -114,6 +114,7 @@ struct RISCVCPUConfig {
bool ext_zvfbfwma;
bool ext_zvfh;
bool ext_zvfhmin;
+ bool ext_sdtrig;
bool ext_smaia;
bool ext_ssaia;
bool ext_sscofpmf;
diff --git a/target/riscv/debug.c b/target/riscv/debug.c
index 5f14b39b06..c40e727e12 100644
--- a/target/riscv/debug.c
+++ b/target/riscv/debug.c
@@ -100,13 +100,16 @@ static trigger_action_t get_trigger_action(CPURISCVState *env,
target_ulong tdata1 = env->tdata1[trigger_index];
int trigger_type = get_trigger_type(env, trigger_index);
trigger_action_t action = DBG_ACTION_NONE;
+ const RISCVCPUConfig *cfg = riscv_cpu_cfg(env);
switch (trigger_type) {
case TRIGGER_TYPE_AD_MATCH:
action = (tdata1 & TYPE2_ACTION) >> 12;
break;
case TRIGGER_TYPE_AD_MATCH6:
- action = (tdata1 & TYPE6_ACTION) >> 12;
+ if (cfg->ext_sdtrig) {
+ action = (tdata1 & TYPE6_ACTION) >> 12;
+ }
break;
case TRIGGER_TYPE_INST_CNT:
case TRIGGER_TYPE_INT:
@@ -727,7 +730,12 @@ void tdata_csr_write(CPURISCVState *env, int tdata_index, target_ulong val)
type2_reg_write(env, env->trigger_cur, tdata_index, val);
break;
case TRIGGER_TYPE_AD_MATCH6:
- type6_reg_write(env, env->trigger_cur, tdata_index, val);
+ if (riscv_cpu_cfg(env)->ext_sdtrig) {
+ type6_reg_write(env, env->trigger_cur, tdata_index, val);
+ } else {
+ qemu_log_mask(LOG_UNIMP, "trigger type: %d is not supported\n",
+ trigger_type);
+ }
break;
case TRIGGER_TYPE_INST_CNT:
itrigger_reg_write(env, env->trigger_cur, tdata_index, val);
@@ -750,9 +758,13 @@ void tdata_csr_write(CPURISCVState *env, int tdata_index, target_ulong val)
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_AD_MATCH6);
+ target_ulong ts = BIT(TRIGGER_TYPE_AD_MATCH);
+
+ if (riscv_cpu_cfg(env)->ext_sdtrig) {
+ ts |= BIT(TRIGGER_TYPE_AD_MATCH6);
+ }
+
+ return ts;
}
void riscv_cpu_debug_excp_handler(CPUState *cs)
@@ -803,6 +815,10 @@ bool riscv_cpu_debug_check_breakpoint(CPUState *cs)
}
break;
case TRIGGER_TYPE_AD_MATCH6:
+ if (!cpu->cfg.ext_sdtrig) {
+ break;
+ }
+
ctrl = env->tdata1[i];
pc = env->tdata2[i];
@@ -869,6 +885,10 @@ bool riscv_cpu_debug_check_watchpoint(CPUState *cs, CPUWatchpoint *wp)
}
break;
case TRIGGER_TYPE_AD_MATCH6:
+ if (!cpu->cfg.ext_sdtrig) {
+ break;
+ }
+
ctrl = env->tdata1[i];
addr = env->tdata2[i];
flags = 0;
--
2.34.1
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH v7 3/4] target/riscv: Expose sdtrig ISA extension
2024-03-14 18:59 [PATCH v7 0/4] Introduce sdtrig ISA extension Himanshu Chauhan
2024-03-14 18:59 ` [PATCH v7 1/4] target/riscv: Check for valid itimer pointer before free Himanshu Chauhan
2024-03-14 18:59 ` [PATCH v7 2/4] target/riscv: Enable mcontrol6 triggers only when sdtrig is selected Himanshu Chauhan
@ 2024-03-14 18:59 ` Himanshu Chauhan
2024-03-14 19:30 ` Andrew Jones
2024-04-29 5:41 ` Alistair Francis
2024-03-14 18:59 ` [PATCH v7 4/4] target/riscv: Enable sdtrig for Ventana's Veyron CPUs Himanshu Chauhan
3 siblings, 2 replies; 11+ messages in thread
From: Himanshu Chauhan @ 2024-03-14 18:59 UTC (permalink / raw)
To: qemu-riscv, qemu-devel; +Cc: ajones
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.
By default, the sdtrig extension is disabled and debug property enabled as usual.
Signed-off-by: Himanshu Chauhan <hchauhan@ventanamicro.com>
---
target/riscv/cpu.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
index ab631500ac..4231f36c1b 100644
--- a/target/riscv/cpu.c
+++ b/target/riscv/cpu.c
@@ -175,6 +175,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, ext_sdtrig),
ISA_EXT_DATA_ENTRY(smaia, PRIV_VERSION_1_12_0, ext_smaia),
ISA_EXT_DATA_ENTRY(smepmp, PRIV_VERSION_1_12_0, ext_smepmp),
ISA_EXT_DATA_ENTRY(smstateen, PRIV_VERSION_1_12_0, ext_smstateen),
@@ -1485,6 +1486,7 @@ const RISCVCPUMultiExtConfig riscv_cpu_extensions[] = {
MULTI_EXT_CFG_BOOL("zvfhmin", ext_zvfhmin, false),
MULTI_EXT_CFG_BOOL("sstc", ext_sstc, true),
+ MULTI_EXT_CFG_BOOL("sdtrig", ext_sdtrig, false),
MULTI_EXT_CFG_BOOL("smaia", ext_smaia, false),
MULTI_EXT_CFG_BOOL("smepmp", ext_smepmp, false),
MULTI_EXT_CFG_BOOL("smstateen", ext_smstateen, false),
--
2.34.1
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH v7 4/4] target/riscv: Enable sdtrig for Ventana's Veyron CPUs
2024-03-14 18:59 [PATCH v7 0/4] Introduce sdtrig ISA extension Himanshu Chauhan
` (2 preceding siblings ...)
2024-03-14 18:59 ` [PATCH v7 3/4] target/riscv: Expose sdtrig ISA extension Himanshu Chauhan
@ 2024-03-14 18:59 ` Himanshu Chauhan
2024-03-14 19:31 ` Andrew Jones
3 siblings, 1 reply; 11+ messages in thread
From: Himanshu Chauhan @ 2024-03-14 18:59 UTC (permalink / raw)
To: qemu-riscv, qemu-devel; +Cc: ajones
Ventana's Veyron CPUs support sdtrig ISA extension. By default, enable
the sdtrig extension and disable the debug property for these CPUs.
Signed-off-by: Himanshu Chauhan <hchauhan@ventanamicro.com>
---
target/riscv/cpu.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
index 4231f36c1b..c9dda73748 100644
--- a/target/riscv/cpu.c
+++ b/target/riscv/cpu.c
@@ -569,6 +569,7 @@ static void rv64_veyron_v1_cpu_init(Object *obj)
cpu->cfg.cbom_blocksize = 64;
cpu->cfg.cboz_blocksize = 64;
cpu->cfg.ext_zicboz = true;
+ cpu->cfg.ext_sdtrig = true;
cpu->cfg.ext_smaia = true;
cpu->cfg.ext_ssaia = true;
cpu->cfg.ext_sscofpmf = true;
--
2.34.1
^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH v7 2/4] target/riscv: Enable mcontrol6 triggers only when sdtrig is selected
2024-03-14 18:59 ` [PATCH v7 2/4] target/riscv: Enable mcontrol6 triggers only when sdtrig is selected Himanshu Chauhan
@ 2024-03-14 19:29 ` Andrew Jones
2024-04-29 4:08 ` Alistair Francis
1 sibling, 0 replies; 11+ messages in thread
From: Andrew Jones @ 2024-03-14 19:29 UTC (permalink / raw)
To: Himanshu Chauhan; +Cc: qemu-riscv, qemu-devel
On Fri, Mar 15, 2024 at 12:29:55AM +0530, Himanshu Chauhan wrote:
> The mcontrol6 triggers are not defined in debug specification v0.13
> These triggers are defined in sdtrig ISA extension.
>
> This patch:
> * Adds ext_sdtrig capability which is used to select mcontrol6 triggers
> * Keeps the debug property. All triggers that are defined in v0.13 are
> exposed.
>
> Signed-off-by: Himanshu Chauhan <hchauhan@ventanamicro.com>
> ---
> target/riscv/cpu.c | 5 +++++
> target/riscv/cpu_cfg.h | 1 +
> target/riscv/debug.c | 30 +++++++++++++++++++++++++-----
> 3 files changed, 31 insertions(+), 5 deletions(-)
>
> diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
> index c160b9216b..ab631500ac 100644
> --- a/target/riscv/cpu.c
> +++ b/target/riscv/cpu.c
> @@ -1008,6 +1008,11 @@ static void riscv_cpu_reset_hold(Object *obj)
> set_default_nan_mode(1, &env->fp_status);
>
> #ifndef CONFIG_USER_ONLY
> + if (!cpu->cfg.debug && cpu->cfg.ext_sdtrig) {
> + warn_report("Enabling 'debug' since 'sdtrig' is enabled.");
> + cpu->cfg.debug = true;
> + }
> +
> if (cpu->cfg.debug) {
> riscv_trigger_reset_hold(env);
> }
> diff --git a/target/riscv/cpu_cfg.h b/target/riscv/cpu_cfg.h
> index 2040b90da0..0c57e1acd4 100644
> --- a/target/riscv/cpu_cfg.h
> +++ b/target/riscv/cpu_cfg.h
> @@ -114,6 +114,7 @@ struct RISCVCPUConfig {
> bool ext_zvfbfwma;
> bool ext_zvfh;
> bool ext_zvfhmin;
> + bool ext_sdtrig;
> bool ext_smaia;
> bool ext_ssaia;
> bool ext_sscofpmf;
> diff --git a/target/riscv/debug.c b/target/riscv/debug.c
> index 5f14b39b06..c40e727e12 100644
> --- a/target/riscv/debug.c
> +++ b/target/riscv/debug.c
> @@ -100,13 +100,16 @@ static trigger_action_t get_trigger_action(CPURISCVState *env,
> target_ulong tdata1 = env->tdata1[trigger_index];
> int trigger_type = get_trigger_type(env, trigger_index);
> trigger_action_t action = DBG_ACTION_NONE;
> + const RISCVCPUConfig *cfg = riscv_cpu_cfg(env);
>
> switch (trigger_type) {
> case TRIGGER_TYPE_AD_MATCH:
> action = (tdata1 & TYPE2_ACTION) >> 12;
> break;
> case TRIGGER_TYPE_AD_MATCH6:
> - action = (tdata1 & TYPE6_ACTION) >> 12;
> + if (cfg->ext_sdtrig) {
> + action = (tdata1 & TYPE6_ACTION) >> 12;
> + }
> break;
> case TRIGGER_TYPE_INST_CNT:
> case TRIGGER_TYPE_INT:
> @@ -727,7 +730,12 @@ void tdata_csr_write(CPURISCVState *env, int tdata_index, target_ulong val)
> type2_reg_write(env, env->trigger_cur, tdata_index, val);
> break;
> case TRIGGER_TYPE_AD_MATCH6:
> - type6_reg_write(env, env->trigger_cur, tdata_index, val);
> + if (riscv_cpu_cfg(env)->ext_sdtrig) {
> + type6_reg_write(env, env->trigger_cur, tdata_index, val);
> + } else {
> + qemu_log_mask(LOG_UNIMP, "trigger type: %d is not supported\n",
> + trigger_type);
> + }
> break;
> case TRIGGER_TYPE_INST_CNT:
> itrigger_reg_write(env, env->trigger_cur, tdata_index, val);
> @@ -750,9 +758,13 @@ void tdata_csr_write(CPURISCVState *env, int tdata_index, target_ulong val)
>
> 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_AD_MATCH6);
> + target_ulong ts = BIT(TRIGGER_TYPE_AD_MATCH);
> +
> + if (riscv_cpu_cfg(env)->ext_sdtrig) {
> + ts |= BIT(TRIGGER_TYPE_AD_MATCH6);
> + }
> +
> + return ts;
> }
>
> void riscv_cpu_debug_excp_handler(CPUState *cs)
> @@ -803,6 +815,10 @@ bool riscv_cpu_debug_check_breakpoint(CPUState *cs)
> }
> break;
> case TRIGGER_TYPE_AD_MATCH6:
> + if (!cpu->cfg.ext_sdtrig) {
> + break;
> + }
> +
> ctrl = env->tdata1[i];
> pc = env->tdata2[i];
>
> @@ -869,6 +885,10 @@ bool riscv_cpu_debug_check_watchpoint(CPUState *cs, CPUWatchpoint *wp)
> }
> break;
> case TRIGGER_TYPE_AD_MATCH6:
> + if (!cpu->cfg.ext_sdtrig) {
> + break;
> + }
> +
> ctrl = env->tdata1[i];
> addr = env->tdata2[i];
> flags = 0;
> --
> 2.34.1
>
Reviewed-by: Andrew Jones <ajones@ventanamicro.com>
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v7 3/4] target/riscv: Expose sdtrig ISA extension
2024-03-14 18:59 ` [PATCH v7 3/4] target/riscv: Expose sdtrig ISA extension Himanshu Chauhan
@ 2024-03-14 19:30 ` Andrew Jones
2024-04-29 5:41 ` Alistair Francis
1 sibling, 0 replies; 11+ messages in thread
From: Andrew Jones @ 2024-03-14 19:30 UTC (permalink / raw)
To: Himanshu Chauhan; +Cc: qemu-riscv, qemu-devel
On Fri, Mar 15, 2024 at 12:29:56AM +0530, Himanshu Chauhan wrote:
> 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.
>
> By default, the sdtrig extension is disabled and debug property enabled as usual.
>
> Signed-off-by: Himanshu Chauhan <hchauhan@ventanamicro.com>
> ---
> target/riscv/cpu.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
> index ab631500ac..4231f36c1b 100644
> --- a/target/riscv/cpu.c
> +++ b/target/riscv/cpu.c
> @@ -175,6 +175,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, ext_sdtrig),
So we're sure this should be 1.12? Or do we need to introduce
PRIV_VERSION_1_13_0?
> ISA_EXT_DATA_ENTRY(smaia, PRIV_VERSION_1_12_0, ext_smaia),
> ISA_EXT_DATA_ENTRY(smepmp, PRIV_VERSION_1_12_0, ext_smepmp),
> ISA_EXT_DATA_ENTRY(smstateen, PRIV_VERSION_1_12_0, ext_smstateen),
> @@ -1485,6 +1486,7 @@ const RISCVCPUMultiExtConfig riscv_cpu_extensions[] = {
> MULTI_EXT_CFG_BOOL("zvfhmin", ext_zvfhmin, false),
> MULTI_EXT_CFG_BOOL("sstc", ext_sstc, true),
>
> + MULTI_EXT_CFG_BOOL("sdtrig", ext_sdtrig, false),
> MULTI_EXT_CFG_BOOL("smaia", ext_smaia, false),
> MULTI_EXT_CFG_BOOL("smepmp", ext_smepmp, false),
> MULTI_EXT_CFG_BOOL("smstateen", ext_smstateen, false),
> --
> 2.34.1
>
Thanks,
drew
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v7 4/4] target/riscv: Enable sdtrig for Ventana's Veyron CPUs
2024-03-14 18:59 ` [PATCH v7 4/4] target/riscv: Enable sdtrig for Ventana's Veyron CPUs Himanshu Chauhan
@ 2024-03-14 19:31 ` Andrew Jones
0 siblings, 0 replies; 11+ messages in thread
From: Andrew Jones @ 2024-03-14 19:31 UTC (permalink / raw)
To: Himanshu Chauhan; +Cc: qemu-riscv, qemu-devel
On Fri, Mar 15, 2024 at 12:29:57AM +0530, Himanshu Chauhan wrote:
> Ventana's Veyron CPUs support sdtrig ISA extension. By default, enable
> the sdtrig extension and disable the debug property for these CPUs.
You still have the 'and disable the debug property' here...
>
> Signed-off-by: Himanshu Chauhan <hchauhan@ventanamicro.com>
> ---
> target/riscv/cpu.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
> index 4231f36c1b..c9dda73748 100644
> --- a/target/riscv/cpu.c
> +++ b/target/riscv/cpu.c
> @@ -569,6 +569,7 @@ static void rv64_veyron_v1_cpu_init(Object *obj)
> cpu->cfg.cbom_blocksize = 64;
> cpu->cfg.cboz_blocksize = 64;
> cpu->cfg.ext_zicboz = true;
> + cpu->cfg.ext_sdtrig = true;
> cpu->cfg.ext_smaia = true;
> cpu->cfg.ext_ssaia = true;
> cpu->cfg.ext_sscofpmf = true;
> --
> 2.34.1
>
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v7 1/4] target/riscv: Check for valid itimer pointer before free
2024-03-14 18:59 ` [PATCH v7 1/4] target/riscv: Check for valid itimer pointer before free Himanshu Chauhan
@ 2024-04-29 4:04 ` Alistair Francis
0 siblings, 0 replies; 11+ messages in thread
From: Alistair Francis @ 2024-04-29 4:04 UTC (permalink / raw)
To: Himanshu Chauhan; +Cc: qemu-riscv, qemu-devel, ajones
On Fri, Mar 15, 2024 at 5:01 AM Himanshu Chauhan
<hchauhan@ventanamicro.com> wrote:
>
> Check if each element of array of pointers for itimer contains a non-null
> pointer before freeing.
>
> Signed-off-by: Himanshu Chauhan <hchauhan@ventanamicro.com>
Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
Alistair
> ---
> target/riscv/debug.c | 5 ++++-
> 1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/target/riscv/debug.c b/target/riscv/debug.c
> index e30d99cc2f..5f14b39b06 100644
> --- a/target/riscv/debug.c
> +++ b/target/riscv/debug.c
> @@ -938,7 +938,10 @@ void riscv_trigger_reset_hold(CPURISCVState *env)
> env->tdata3[i] = 0;
> env->cpu_breakpoint[i] = NULL;
> env->cpu_watchpoint[i] = NULL;
> - timer_del(env->itrigger_timer[i]);
> + if (env->itrigger_timer[i]) {
> + timer_del(env->itrigger_timer[i]);
> + env->itrigger_timer[i] = NULL;
> + }
> }
>
> env->mcontext = 0;
> --
> 2.34.1
>
>
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v7 2/4] target/riscv: Enable mcontrol6 triggers only when sdtrig is selected
2024-03-14 18:59 ` [PATCH v7 2/4] target/riscv: Enable mcontrol6 triggers only when sdtrig is selected Himanshu Chauhan
2024-03-14 19:29 ` Andrew Jones
@ 2024-04-29 4:08 ` Alistair Francis
1 sibling, 0 replies; 11+ messages in thread
From: Alistair Francis @ 2024-04-29 4:08 UTC (permalink / raw)
To: Himanshu Chauhan; +Cc: qemu-riscv, qemu-devel, ajones
On Fri, Mar 15, 2024 at 5:01 AM Himanshu Chauhan
<hchauhan@ventanamicro.com> wrote:
>
> The mcontrol6 triggers are not defined in debug specification v0.13
> These triggers are defined in sdtrig ISA extension.
>
> This patch:
> * Adds ext_sdtrig capability which is used to select mcontrol6 triggers
> * Keeps the debug property. All triggers that are defined in v0.13 are
> exposed.
Thanks for this!
>
> Signed-off-by: Himanshu Chauhan <hchauhan@ventanamicro.com>
> ---
> target/riscv/cpu.c | 5 +++++
> target/riscv/cpu_cfg.h | 1 +
> target/riscv/debug.c | 30 +++++++++++++++++++++++++-----
> 3 files changed, 31 insertions(+), 5 deletions(-)
>
> diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
> index c160b9216b..ab631500ac 100644
> --- a/target/riscv/cpu.c
> +++ b/target/riscv/cpu.c
> @@ -1008,6 +1008,11 @@ static void riscv_cpu_reset_hold(Object *obj)
> set_default_nan_mode(1, &env->fp_status);
>
> #ifndef CONFIG_USER_ONLY
> + if (!cpu->cfg.debug && cpu->cfg.ext_sdtrig) {
> + warn_report("Enabling 'debug' since 'sdtrig' is enabled.");
I don't think we need the warning. It isn't a problem for the user
Otherwise
Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
Alistair
> + cpu->cfg.debug = true;
> + }
> +
> if (cpu->cfg.debug) {
> riscv_trigger_reset_hold(env);
> }
> diff --git a/target/riscv/cpu_cfg.h b/target/riscv/cpu_cfg.h
> index 2040b90da0..0c57e1acd4 100644
> --- a/target/riscv/cpu_cfg.h
> +++ b/target/riscv/cpu_cfg.h
> @@ -114,6 +114,7 @@ struct RISCVCPUConfig {
> bool ext_zvfbfwma;
> bool ext_zvfh;
> bool ext_zvfhmin;
> + bool ext_sdtrig;
> bool ext_smaia;
> bool ext_ssaia;
> bool ext_sscofpmf;
> diff --git a/target/riscv/debug.c b/target/riscv/debug.c
> index 5f14b39b06..c40e727e12 100644
> --- a/target/riscv/debug.c
> +++ b/target/riscv/debug.c
> @@ -100,13 +100,16 @@ static trigger_action_t get_trigger_action(CPURISCVState *env,
> target_ulong tdata1 = env->tdata1[trigger_index];
> int trigger_type = get_trigger_type(env, trigger_index);
> trigger_action_t action = DBG_ACTION_NONE;
> + const RISCVCPUConfig *cfg = riscv_cpu_cfg(env);
>
> switch (trigger_type) {
> case TRIGGER_TYPE_AD_MATCH:
> action = (tdata1 & TYPE2_ACTION) >> 12;
> break;
> case TRIGGER_TYPE_AD_MATCH6:
> - action = (tdata1 & TYPE6_ACTION) >> 12;
> + if (cfg->ext_sdtrig) {
> + action = (tdata1 & TYPE6_ACTION) >> 12;
> + }
> break;
> case TRIGGER_TYPE_INST_CNT:
> case TRIGGER_TYPE_INT:
> @@ -727,7 +730,12 @@ void tdata_csr_write(CPURISCVState *env, int tdata_index, target_ulong val)
> type2_reg_write(env, env->trigger_cur, tdata_index, val);
> break;
> case TRIGGER_TYPE_AD_MATCH6:
> - type6_reg_write(env, env->trigger_cur, tdata_index, val);
> + if (riscv_cpu_cfg(env)->ext_sdtrig) {
> + type6_reg_write(env, env->trigger_cur, tdata_index, val);
> + } else {
> + qemu_log_mask(LOG_UNIMP, "trigger type: %d is not supported\n",
> + trigger_type);
> + }
> break;
> case TRIGGER_TYPE_INST_CNT:
> itrigger_reg_write(env, env->trigger_cur, tdata_index, val);
> @@ -750,9 +758,13 @@ void tdata_csr_write(CPURISCVState *env, int tdata_index, target_ulong val)
>
> 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_AD_MATCH6);
> + target_ulong ts = BIT(TRIGGER_TYPE_AD_MATCH);
> +
> + if (riscv_cpu_cfg(env)->ext_sdtrig) {
> + ts |= BIT(TRIGGER_TYPE_AD_MATCH6);
> + }
> +
> + return ts;
> }
>
> void riscv_cpu_debug_excp_handler(CPUState *cs)
> @@ -803,6 +815,10 @@ bool riscv_cpu_debug_check_breakpoint(CPUState *cs)
> }
> break;
> case TRIGGER_TYPE_AD_MATCH6:
> + if (!cpu->cfg.ext_sdtrig) {
> + break;
> + }
> +
> ctrl = env->tdata1[i];
> pc = env->tdata2[i];
>
> @@ -869,6 +885,10 @@ bool riscv_cpu_debug_check_watchpoint(CPUState *cs, CPUWatchpoint *wp)
> }
> break;
> case TRIGGER_TYPE_AD_MATCH6:
> + if (!cpu->cfg.ext_sdtrig) {
> + break;
> + }
> +
> ctrl = env->tdata1[i];
> addr = env->tdata2[i];
> flags = 0;
> --
> 2.34.1
>
>
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v7 3/4] target/riscv: Expose sdtrig ISA extension
2024-03-14 18:59 ` [PATCH v7 3/4] target/riscv: Expose sdtrig ISA extension Himanshu Chauhan
2024-03-14 19:30 ` Andrew Jones
@ 2024-04-29 5:41 ` Alistair Francis
1 sibling, 0 replies; 11+ messages in thread
From: Alistair Francis @ 2024-04-29 5:41 UTC (permalink / raw)
To: Himanshu Chauhan; +Cc: qemu-riscv, qemu-devel, ajones
On Fri, Mar 15, 2024 at 5:02 AM Himanshu Chauhan
<hchauhan@ventanamicro.com> wrote:
>
> 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.
>
> By default, the sdtrig extension is disabled and debug property enabled as usual.
>
> Signed-off-by: Himanshu Chauhan <hchauhan@ventanamicro.com>
Acked-by: Alistair Francis <alistair.francis@wdc.com>
Alistair
> ---
> target/riscv/cpu.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
> index ab631500ac..4231f36c1b 100644
> --- a/target/riscv/cpu.c
> +++ b/target/riscv/cpu.c
> @@ -175,6 +175,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, ext_sdtrig),
> ISA_EXT_DATA_ENTRY(smaia, PRIV_VERSION_1_12_0, ext_smaia),
> ISA_EXT_DATA_ENTRY(smepmp, PRIV_VERSION_1_12_0, ext_smepmp),
> ISA_EXT_DATA_ENTRY(smstateen, PRIV_VERSION_1_12_0, ext_smstateen),
> @@ -1485,6 +1486,7 @@ const RISCVCPUMultiExtConfig riscv_cpu_extensions[] = {
> MULTI_EXT_CFG_BOOL("zvfhmin", ext_zvfhmin, false),
> MULTI_EXT_CFG_BOOL("sstc", ext_sstc, true),
>
> + MULTI_EXT_CFG_BOOL("sdtrig", ext_sdtrig, false),
> MULTI_EXT_CFG_BOOL("smaia", ext_smaia, false),
> MULTI_EXT_CFG_BOOL("smepmp", ext_smepmp, false),
> MULTI_EXT_CFG_BOOL("smstateen", ext_smstateen, false),
> --
> 2.34.1
>
>
^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2024-04-29 5:42 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-03-14 18:59 [PATCH v7 0/4] Introduce sdtrig ISA extension Himanshu Chauhan
2024-03-14 18:59 ` [PATCH v7 1/4] target/riscv: Check for valid itimer pointer before free Himanshu Chauhan
2024-04-29 4:04 ` Alistair Francis
2024-03-14 18:59 ` [PATCH v7 2/4] target/riscv: Enable mcontrol6 triggers only when sdtrig is selected Himanshu Chauhan
2024-03-14 19:29 ` Andrew Jones
2024-04-29 4:08 ` Alistair Francis
2024-03-14 18:59 ` [PATCH v7 3/4] target/riscv: Expose sdtrig ISA extension Himanshu Chauhan
2024-03-14 19:30 ` Andrew Jones
2024-04-29 5:41 ` Alistair Francis
2024-03-14 18:59 ` [PATCH v7 4/4] target/riscv: Enable sdtrig for Ventana's Veyron CPUs Himanshu Chauhan
2024-03-14 19:31 ` Andrew Jones
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).