[PATCH v5 3/4] target/riscv: Apply modularized matching conditions for watchpoint

2024-06-03 Thread Alvin Chang via
We have implemented trigger_common_match(), which checks if the enabled
privilege levels of the trigger match CPU's current privilege level.
Remove the related code in riscv_cpu_debug_check_watchpoint() and invoke
trigger_common_match() to check the privilege levels of the type 2 and
type 6 triggers for the watchpoints.

This commit also changes the behavior of looping the triggers. In
previous implementation, if we have a type 2 trigger and
env->virt_enabled is true, we directly return false to stop the loop.
Now we keep looping all the triggers until we find a matched trigger.

Only load/store bits and loaded/stored address should be further checked
in riscv_cpu_debug_check_watchpoint().

Signed-off-by: Alvin Chang 
Acked-by: Alistair Francis 
---
 target/riscv/debug.c | 26 ++
 1 file changed, 6 insertions(+), 20 deletions(-)

diff --git a/target/riscv/debug.c b/target/riscv/debug.c
index 11125f333b..c290d6002e 100644
--- a/target/riscv/debug.c
+++ b/target/riscv/debug.c
@@ -901,13 +901,12 @@ bool riscv_cpu_debug_check_watchpoint(CPUState *cs, 
CPUWatchpoint *wp)
 for (i = 0; i < RV_MAX_TRIGGERS; i++) {
 trigger_type = get_trigger_type(env, i);
 
+if (!trigger_common_match(env, trigger_type, i)) {
+continue;
+}
+
 switch (trigger_type) {
 case TRIGGER_TYPE_AD_MATCH:
-/* type 2 trigger cannot be fired in VU/VS mode */
-if (env->virt_enabled) {
-return false;
-}
-
 ctrl = env->tdata1[i];
 addr = env->tdata2[i];
 flags = 0;
@@ -920,10 +919,7 @@ bool riscv_cpu_debug_check_watchpoint(CPUState *cs, 
CPUWatchpoint *wp)
 }
 
 if ((wp->flags & flags) && (wp->vaddr == addr)) {
-/* check U/S/M bit against current privilege level */
-if ((ctrl >> 3) & BIT(env->priv)) {
-return true;
-}
+return true;
 }
 break;
 case TRIGGER_TYPE_AD_MATCH6:
@@ -939,17 +935,7 @@ bool riscv_cpu_debug_check_watchpoint(CPUState *cs, 
CPUWatchpoint *wp)
 }
 
 if ((wp->flags & flags) && (wp->vaddr == addr)) {
-if (env->virt_enabled) {
-/* check VU/VS bit against current privilege level */
-if ((ctrl >> 23) & BIT(env->priv)) {
-return true;
-}
-} else {
-/* check U/S/M bit against current privilege level */
-if ((ctrl >> 3) & BIT(env->priv)) {
-return true;
-}
-}
+return true;
 }
 break;
 default:
-- 
2.34.1




[PATCH v5 4/4] target/riscv: Apply modularized matching conditions for icount trigger

2024-06-03 Thread Alvin Chang via
We have implemented trigger_common_match(), which checks if the enabled
privilege levels of the trigger match CPU's current privilege level. We
can invoke trigger_common_match() to check the privilege levels of the
type 3 triggers.

Signed-off-by: Alvin Chang 
Acked-by: Alistair Francis 
---
 target/riscv/debug.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/target/riscv/debug.c b/target/riscv/debug.c
index c290d6002e..0b5099ff9a 100644
--- a/target/riscv/debug.c
+++ b/target/riscv/debug.c
@@ -624,7 +624,7 @@ void helper_itrigger_match(CPURISCVState *env)
 if (get_trigger_type(env, i) != TRIGGER_TYPE_INST_CNT) {
 continue;
 }
-if (check_itrigger_priv(env, i)) {
+if (!trigger_common_match(env, TRIGGER_TYPE_INST_CNT, i)) {
 continue;
 }
 count = itrigger_get_count(env, i);
-- 
2.34.1




[PATCH v5 1/4] target/riscv: Add functions for common matching conditions of trigger

2024-06-03 Thread Alvin Chang via
According to RISC-V Debug specification version 0.13 [1] (also applied
to version 1.0 [2] but it has not been ratified yet), there are several
common matching conditions before firing a trigger, including the
enabled privilege levels of the trigger.

This commit adds trigger_common_match() to prepare the common matching
conditions for the type 2/3/6 triggers. For now, we just implement
trigger_priv_match() to check if the enabled privilege levels of the
trigger match CPU's current privilege level.

[1]: https://github.com/riscv/riscv-debug-spec/releases/tag/task_group_vote
[2]: https://github.com/riscv/riscv-debug-spec/releases/tag/1.0.0-rc1-asciidoc

Signed-off-by: Alvin Chang 
Reviewed-by: Alistair Francis 
---
 target/riscv/debug.c | 70 
 1 file changed, 70 insertions(+)

diff --git a/target/riscv/debug.c b/target/riscv/debug.c
index b110370ea6..05e001d041 100644
--- a/target/riscv/debug.c
+++ b/target/riscv/debug.c
@@ -241,6 +241,76 @@ static void do_trigger_action(CPURISCVState *env, 
target_ulong trigger_index)
 }
 }
 
+/*
+ * Check the privilege level of specific trigger matches CPU's current 
privilege
+ * level.
+ */
+static bool trigger_priv_match(CPURISCVState *env, trigger_type_t type,
+   int trigger_index)
+{
+target_ulong ctrl = env->tdata1[trigger_index];
+
+switch (type) {
+case TRIGGER_TYPE_AD_MATCH:
+/* type 2 trigger cannot be fired in VU/VS mode */
+if (env->virt_enabled) {
+return false;
+}
+/* check U/S/M bit against current privilege level */
+if ((ctrl >> 3) & BIT(env->priv)) {
+return true;
+}
+break;
+case TRIGGER_TYPE_AD_MATCH6:
+if (env->virt_enabled) {
+/* check VU/VS bit against current privilege level */
+if ((ctrl >> 23) & BIT(env->priv)) {
+return true;
+}
+} else {
+/* check U/S/M bit against current privilege level */
+if ((ctrl >> 3) & BIT(env->priv)) {
+return true;
+}
+}
+break;
+case TRIGGER_TYPE_INST_CNT:
+if (env->virt_enabled) {
+/* check VU/VS bit against current privilege level */
+if ((ctrl >> 25) & BIT(env->priv)) {
+return true;
+}
+} else {
+/* check U/S/M bit against current privilege level */
+if ((ctrl >> 6) & BIT(env->priv)) {
+return true;
+}
+}
+break;
+case TRIGGER_TYPE_INT:
+case TRIGGER_TYPE_EXCP:
+case TRIGGER_TYPE_EXT_SRC:
+qemu_log_mask(LOG_UNIMP, "trigger type: %d is not supported\n", type);
+break;
+case TRIGGER_TYPE_NO_EXIST:
+case TRIGGER_TYPE_UNAVAIL:
+qemu_log_mask(LOG_GUEST_ERROR, "trigger type: %d does not exist\n",
+  type);
+break;
+default:
+g_assert_not_reached();
+}
+
+return false;
+}
+
+/* Common matching conditions for all types of the triggers. */
+static bool trigger_common_match(CPURISCVState *env, trigger_type_t type,
+ int trigger_index)
+{
+return trigger_priv_match(env, type, trigger_index);
+}
+
 /* type 2 trigger */
 
 static uint32_t type2_breakpoint_size(CPURISCVState *env, target_ulong ctrl)
-- 
2.34.1




[PATCH v5 2/4] target/riscv: Apply modularized matching conditions for breakpoint

2024-06-03 Thread Alvin Chang via
We have implemented trigger_common_match(), which checks if the enabled
privilege levels of the trigger match CPU's current privilege level.
Remove the related code in riscv_cpu_debug_check_breakpoint() and invoke
trigger_common_match() to check the privilege levels of the type 2 and
type 6 triggers for the breakpoints.

This commit also changes the behavior of looping the triggers. In
previous implementation, if we have a type 2 trigger and
env->virt_enabled is true, we directly return false to stop the loop.
Now we keep looping all the triggers until we find a matched trigger.

Only the execution bit and the executed PC should be futher checked in
riscv_cpu_debug_check_breakpoint().

Signed-off-by: Alvin Chang 
Reviewed-by: Alistair Francis 
---
 target/riscv/debug.c | 31 ---
 1 file changed, 8 insertions(+), 23 deletions(-)

diff --git a/target/riscv/debug.c b/target/riscv/debug.c
index 05e001d041..11125f333b 100644
--- a/target/riscv/debug.c
+++ b/target/riscv/debug.c
@@ -855,22 +855,18 @@ bool riscv_cpu_debug_check_breakpoint(CPUState *cs)
 for (i = 0; i < RV_MAX_TRIGGERS; i++) {
 trigger_type = get_trigger_type(env, i);
 
+if (!trigger_common_match(env, trigger_type, i)) {
+continue;
+}
+
 switch (trigger_type) {
 case TRIGGER_TYPE_AD_MATCH:
-/* type 2 trigger cannot be fired in VU/VS mode */
-if (env->virt_enabled) {
-return false;
-}
-
 ctrl = env->tdata1[i];
 pc = env->tdata2[i];
 
 if ((ctrl & TYPE2_EXEC) && (bp->pc == pc)) {
-/* check U/S/M bit against current privilege level */
-if ((ctrl >> 3) & BIT(env->priv)) {
-env->badaddr = pc;
-return true;
-}
+env->badaddr = pc;
+return true;
 }
 break;
 case TRIGGER_TYPE_AD_MATCH6:
@@ -878,19 +874,8 @@ bool riscv_cpu_debug_check_breakpoint(CPUState *cs)
 pc = env->tdata2[i];
 
 if ((ctrl & TYPE6_EXEC) && (bp->pc == pc)) {
-if (env->virt_enabled) {
-/* check VU/VS bit against current privilege level */
-if ((ctrl >> 23) & BIT(env->priv)) {
-env->badaddr = pc;
-return true;
-}
-} else {
-/* check U/S/M bit against current privilege level */
-if ((ctrl >> 3) & BIT(env->priv)) {
-env->badaddr = pc;
-return true;
-}
-}
+env->badaddr = pc;
+return true;
 }
 break;
 default:
-- 
2.34.1




[PATCH v5 0/4] RISC-V: Modularize common match conditions for trigger

2024-06-03 Thread Alvin Chang via
According to RISC-V Debug specification ratified version 0.13 [1]
(also applied to version 1.0 [2] but it has not been ratified yet), the
enabled privilege levels of the trigger is common match conditions for
all the types of the trigger.

This series modularize the code for checking the privilege levels of
type 2/3/6 triggers by implementing functions trigger_common_match()
and trigger_priv_match().

Additional match conditions, such as CSR tcontrol and textra, can be
further implemented into trigger_common_match() in the future.

[1]: https://github.com/riscv/riscv-debug-spec/releases/tag/task_group_vote
[2]: https://github.com/riscv/riscv-debug-spec/releases/tag/1.0.0-rc1-asciidoc

Changes from v4:
- Rebasing on riscv-to-apply.next 

Changes from v3:
- Change this series to target Debug Spec. version 0.13

Changes from v2:
- Explicitly mention the targeting version of RISC-V Debug Spec.

Changes from v1:
- Fix typo
- Add commit description for changing behavior of looping the triggers
  when we check type 2 triggers.

Alvin Chang (4):
  target/riscv: Add functions for common matching conditions of trigger
  target/riscv: Apply modularized matching conditions for breakpoint
  target/riscv: Apply modularized matching conditions for watchpoint
  target/riscv: Apply modularized matching conditions for icount trigger

 target/riscv/debug.c | 129 ---
 1 file changed, 85 insertions(+), 44 deletions(-)

-- 
2.34.1




[PATCH v4 1/4] target/riscv: Add functions for common matching conditions of trigger

2024-02-26 Thread Alvin Chang via
According to RISC-V Debug specification version 0.13 [1] (also applied
to version 1.0 [2] but it has not been ratified yet), there are several
common matching conditions before firing a trigger, including the
enabled privilege levels of the trigger.

This commit adds trigger_common_match() to prepare the common matching
conditions for the type 2/3/6 triggers. For now, we just implement
trigger_priv_match() to check if the enabled privilege levels of the
trigger match CPU's current privilege level.

[1]: https://github.com/riscv/riscv-debug-spec/releases/tag/task_group_vote
[2]: https://github.com/riscv/riscv-debug-spec/releases/tag/1.0.0-rc1-asciidoc

Signed-off-by: Alvin Chang 
---
 target/riscv/debug.c | 70 
 1 file changed, 70 insertions(+)

diff --git a/target/riscv/debug.c b/target/riscv/debug.c
index e30d99cc2f..3891236b82 100644
--- a/target/riscv/debug.c
+++ b/target/riscv/debug.c
@@ -241,6 +241,76 @@ static void do_trigger_action(CPURISCVState *env, 
target_ulong trigger_index)
 }
 }
 
+/*
+ * Check the privilege level of specific trigger matches CPU's current 
privilege
+ * level.
+ */
+static bool trigger_priv_match(CPURISCVState *env, trigger_type_t type,
+   int trigger_index)
+{
+target_ulong ctrl = env->tdata1[trigger_index];
+
+switch (type) {
+case TRIGGER_TYPE_AD_MATCH:
+/* type 2 trigger cannot be fired in VU/VS mode */
+if (env->virt_enabled) {
+return false;
+}
+/* check U/S/M bit against current privilege level */
+if ((ctrl >> 3) & BIT(env->priv)) {
+return true;
+}
+break;
+case TRIGGER_TYPE_AD_MATCH6:
+if (env->virt_enabled) {
+/* check VU/VS bit against current privilege level */
+if ((ctrl >> 23) & BIT(env->priv)) {
+return true;
+}
+} else {
+/* check U/S/M bit against current privilege level */
+if ((ctrl >> 3) & BIT(env->priv)) {
+return true;
+}
+}
+break;
+case TRIGGER_TYPE_INST_CNT:
+if (env->virt_enabled) {
+/* check VU/VS bit against current privilege level */
+if ((ctrl >> 25) & BIT(env->priv)) {
+return true;
+}
+} else {
+/* check U/S/M bit against current privilege level */
+if ((ctrl >> 6) & BIT(env->priv)) {
+return true;
+}
+}
+break;
+case TRIGGER_TYPE_INT:
+case TRIGGER_TYPE_EXCP:
+case TRIGGER_TYPE_EXT_SRC:
+qemu_log_mask(LOG_UNIMP, "trigger type: %d is not supported\n", type);
+break;
+case TRIGGER_TYPE_NO_EXIST:
+case TRIGGER_TYPE_UNAVAIL:
+qemu_log_mask(LOG_GUEST_ERROR, "trigger type: %d does not exist\n",
+  type);
+break;
+default:
+g_assert_not_reached();
+}
+
+return false;
+}
+
+/* Common matching conditions for all types of the triggers. */
+static bool trigger_common_match(CPURISCVState *env, trigger_type_t type,
+ int trigger_index)
+{
+return trigger_priv_match(env, type, trigger_index);
+}
+
 /* type 2 trigger */
 
 static uint32_t type2_breakpoint_size(CPURISCVState *env, target_ulong ctrl)
-- 
2.34.1




[PATCH v4 3/4] target/riscv: Apply modularized matching conditions for watchpoint

2024-02-26 Thread Alvin Chang via
We have implemented trigger_common_match(), which checks if the enabled
privilege levels of the trigger match CPU's current privilege level.
Remove the related code in riscv_cpu_debug_check_watchpoint() and invoke
trigger_common_match() to check the privilege levels of the type 2 and
type 6 triggers for the watchpoints.

This commit also changes the behavior of looping the triggers. In
previous implementation, if we have a type 2 trigger and
env->virt_enabled is true, we directly return false to stop the loop.
Now we keep looping all the triggers until we find a matched trigger.

Only load/store bits and loaded/stored address should be further checked
in riscv_cpu_debug_check_watchpoint().

Signed-off-by: Alvin Chang 
---
 target/riscv/debug.c | 26 ++
 1 file changed, 6 insertions(+), 20 deletions(-)

diff --git a/target/riscv/debug.c b/target/riscv/debug.c
index b7b0fa8945..9f9f332019 100644
--- a/target/riscv/debug.c
+++ b/target/riscv/debug.c
@@ -899,13 +899,12 @@ bool riscv_cpu_debug_check_watchpoint(CPUState *cs, 
CPUWatchpoint *wp)
 for (i = 0; i < RV_MAX_TRIGGERS; i++) {
 trigger_type = get_trigger_type(env, i);
 
+if (!trigger_common_match(env, trigger_type, i)) {
+continue;
+}
+
 switch (trigger_type) {
 case TRIGGER_TYPE_AD_MATCH:
-/* type 2 trigger cannot be fired in VU/VS mode */
-if (env->virt_enabled) {
-return false;
-}
-
 ctrl = env->tdata1[i];
 addr = env->tdata2[i];
 flags = 0;
@@ -918,10 +917,7 @@ bool riscv_cpu_debug_check_watchpoint(CPUState *cs, 
CPUWatchpoint *wp)
 }
 
 if ((wp->flags & flags) && (wp->vaddr == addr)) {
-/* check U/S/M bit against current privilege level */
-if ((ctrl >> 3) & BIT(env->priv)) {
-return true;
-}
+return true;
 }
 break;
 case TRIGGER_TYPE_AD_MATCH6:
@@ -937,17 +933,7 @@ bool riscv_cpu_debug_check_watchpoint(CPUState *cs, 
CPUWatchpoint *wp)
 }
 
 if ((wp->flags & flags) && (wp->vaddr == addr)) {
-if (env->virt_enabled) {
-/* check VU/VS bit against current privilege level */
-if ((ctrl >> 23) & BIT(env->priv)) {
-return true;
-}
-} else {
-/* check U/S/M bit against current privilege level */
-if ((ctrl >> 3) & BIT(env->priv)) {
-return true;
-}
-}
+return true;
 }
 break;
 default:
-- 
2.34.1




[PATCH v4 0/4] RISC-V: Modularize common match conditions for trigger

2024-02-26 Thread Alvin Chang via
According to RISC-V Debug specification ratified version 0.13 [1]
(also applied to version 1.0 [2] but it has not been ratified yet), the
enabled privilege levels of the trigger is common match conditions for
all the types of the trigger.

This series modularize the code for checking the privilege levels of
type 2/3/6 triggers by implementing functions trigger_common_match()
and trigger_priv_match().

Additional match conditions, such as CSR tcontrol and textra, can be
further implemented into trigger_common_match() in the future.

[1]: https://github.com/riscv/riscv-debug-spec/releases/tag/task_group_vote
[2]: https://github.com/riscv/riscv-debug-spec/releases/tag/1.0.0-rc1-asciidoc

Changes from v3:
- Change this series to target Debug Spec. version 0.13

Changes from v2:
- Explicitly mention the targeting version of RISC-V Debug Spec.

Changes from v1:
- Fix typo
- Add commit description for changing behavior of looping the triggers
  when we check type 2 triggers.

Alvin Chang (4):
  target/riscv: Add functions for common matching conditions of trigger
  target/riscv: Apply modularized matching conditions for breakpoint
  target/riscv: Apply modularized matching conditions for watchpoint
  target/riscv: Apply modularized matching conditions for icount trigger

 target/riscv/debug.c | 124 +--
 1 file changed, 83 insertions(+), 41 deletions(-)

-- 
2.34.1




[PATCH v4 4/4] target/riscv: Apply modularized matching conditions for icount trigger

2024-02-26 Thread Alvin Chang via
We have implemented trigger_common_match(), which checks if the enabled
privilege levels of the trigger match CPU's current privilege level. We
can invoke trigger_common_match() to check the privilege levels of the
type 3 triggers.

Signed-off-by: Alvin Chang 
---
 target/riscv/debug.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/target/riscv/debug.c b/target/riscv/debug.c
index 9f9f332019..eb45e2c147 100644
--- a/target/riscv/debug.c
+++ b/target/riscv/debug.c
@@ -624,7 +624,7 @@ void helper_itrigger_match(CPURISCVState *env)
 if (get_trigger_type(env, i) != TRIGGER_TYPE_INST_CNT) {
 continue;
 }
-if (check_itrigger_priv(env, i)) {
+if (!trigger_common_match(env, TRIGGER_TYPE_INST_CNT, i)) {
 continue;
 }
 count = itrigger_get_count(env, i);
-- 
2.34.1




[PATCH v4 2/4] target/riscv: Apply modularized matching conditions for breakpoint

2024-02-26 Thread Alvin Chang via
We have implemented trigger_common_match(), which checks if the enabled
privilege levels of the trigger match CPU's current privilege level.
Remove the related code in riscv_cpu_debug_check_breakpoint() and invoke
trigger_common_match() to check the privilege levels of the type 2 and
type 6 triggers for the breakpoints.

This commit also changes the behavior of looping the triggers. In
previous implementation, if we have a type 2 trigger and
env->virt_enabled is true, we directly return false to stop the loop.
Now we keep looping all the triggers until we find a matched trigger.

Only the execution bit and the executed PC should be futher checked in
riscv_cpu_debug_check_breakpoint().

Signed-off-by: Alvin Chang 
Reviewed-by: Alistair Francis 
---
 target/riscv/debug.c | 26 ++
 1 file changed, 6 insertions(+), 20 deletions(-)

diff --git a/target/riscv/debug.c b/target/riscv/debug.c
index 3891236b82..b7b0fa8945 100644
--- a/target/riscv/debug.c
+++ b/target/riscv/debug.c
@@ -855,21 +855,17 @@ bool riscv_cpu_debug_check_breakpoint(CPUState *cs)
 for (i = 0; i < RV_MAX_TRIGGERS; i++) {
 trigger_type = get_trigger_type(env, i);
 
+if (!trigger_common_match(env, trigger_type, i)) {
+continue;
+}
+
 switch (trigger_type) {
 case TRIGGER_TYPE_AD_MATCH:
-/* type 2 trigger cannot be fired in VU/VS mode */
-if (env->virt_enabled) {
-return false;
-}
-
 ctrl = env->tdata1[i];
 pc = env->tdata2[i];
 
 if ((ctrl & TYPE2_EXEC) && (bp->pc == pc)) {
-/* check U/S/M bit against current privilege level */
-if ((ctrl >> 3) & BIT(env->priv)) {
-return true;
-}
+return true;
 }
 break;
 case TRIGGER_TYPE_AD_MATCH6:
@@ -877,17 +873,7 @@ bool riscv_cpu_debug_check_breakpoint(CPUState *cs)
 pc = env->tdata2[i];
 
 if ((ctrl & TYPE6_EXEC) && (bp->pc == pc)) {
-if (env->virt_enabled) {
-/* check VU/VS bit against current privilege level */
-if ((ctrl >> 23) & BIT(env->priv)) {
-return true;
-}
-} else {
-/* check U/S/M bit against current privilege level */
-if ((ctrl >> 3) & BIT(env->priv)) {
-return true;
-}
-}
+return true;
 }
 break;
 default:
-- 
2.34.1




[PATCH v3 0/4] RISC-V: Modularize common match conditions for trigger

2024-02-25 Thread Alvin Chang via
According to latest RISC-V Debug specification version 1.0 [1], the
enabled privilege levels of the trigger is common match conditions for
all the types of the trigger.

This series modularize the code for checking the privilege levels of
type 2/3/6 triggers by implementing functions trigger_common_match()
and trigger_priv_match().

Additional match conditions, such as CSR tcontrol and textra, can be
further implemented into trigger_common_match() in the future.

[1]: https://github.com/riscv/riscv-debug-spec/releases/tag/1.0.0-rc1-asciidoc

Changes from v2:
- Explicitly mention the targeting version of RISC-V Debug Spec.

Changes from v1:
- Fix typo
- Add commit description for changing behavior of looping the triggers
  when we check type 2 triggers.

Alvin Chang (4):
  target/riscv: Add functions for common matching conditions of trigger
  target/riscv: Apply modularized matching conditions for breakpoint
  target/riscv: Apply modularized matching conditions for watchpoint
  target/riscv: Apply modularized matching conditions for icount trigger

 target/riscv/debug.c | 124 +--
 1 file changed, 83 insertions(+), 41 deletions(-)

-- 
2.34.1




[PATCH v3 2/4] target/riscv: Apply modularized matching conditions for breakpoint

2024-02-25 Thread Alvin Chang via
We have implemented trigger_common_match(), which checks if the enabled
privilege levels of the trigger match CPU's current privilege level.
Remove the related code in riscv_cpu_debug_check_breakpoint() and invoke
trigger_common_match() to check the privilege levels of the type 2 and
type 6 triggers for the breakpoints.

This commit also changes the behavior of looping the triggers. In
previous implementation, if we have a type 2 trigger and
env->virt_enabled is true, we directly return false to stop the loop.
Now we keep looping all the triggers until we find a matched trigger.

Only the execution bit and the executed PC should be futher checked in
riscv_cpu_debug_check_breakpoint().

Signed-off-by: Alvin Chang 
---
 target/riscv/debug.c | 26 ++
 1 file changed, 6 insertions(+), 20 deletions(-)

diff --git a/target/riscv/debug.c b/target/riscv/debug.c
index 3891236b82..b7b0fa8945 100644
--- a/target/riscv/debug.c
+++ b/target/riscv/debug.c
@@ -855,21 +855,17 @@ bool riscv_cpu_debug_check_breakpoint(CPUState *cs)
 for (i = 0; i < RV_MAX_TRIGGERS; i++) {
 trigger_type = get_trigger_type(env, i);
 
+if (!trigger_common_match(env, trigger_type, i)) {
+continue;
+}
+
 switch (trigger_type) {
 case TRIGGER_TYPE_AD_MATCH:
-/* type 2 trigger cannot be fired in VU/VS mode */
-if (env->virt_enabled) {
-return false;
-}
-
 ctrl = env->tdata1[i];
 pc = env->tdata2[i];
 
 if ((ctrl & TYPE2_EXEC) && (bp->pc == pc)) {
-/* check U/S/M bit against current privilege level */
-if ((ctrl >> 3) & BIT(env->priv)) {
-return true;
-}
+return true;
 }
 break;
 case TRIGGER_TYPE_AD_MATCH6:
@@ -877,17 +873,7 @@ bool riscv_cpu_debug_check_breakpoint(CPUState *cs)
 pc = env->tdata2[i];
 
 if ((ctrl & TYPE6_EXEC) && (bp->pc == pc)) {
-if (env->virt_enabled) {
-/* check VU/VS bit against current privilege level */
-if ((ctrl >> 23) & BIT(env->priv)) {
-return true;
-}
-} else {
-/* check U/S/M bit against current privilege level */
-if ((ctrl >> 3) & BIT(env->priv)) {
-return true;
-}
-}
+return true;
 }
 break;
 default:
-- 
2.34.1




[PATCH v3 4/4] target/riscv: Apply modularized matching conditions for icount trigger

2024-02-25 Thread Alvin Chang via
We have implemented trigger_common_match(), which checks if the enabled
privilege levels of the trigger match CPU's current privilege level. We
can invoke trigger_common_match() to check the privilege levels of the
type 3 triggers.

Signed-off-by: Alvin Chang 
---
 target/riscv/debug.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/target/riscv/debug.c b/target/riscv/debug.c
index 9f9f332019..eb45e2c147 100644
--- a/target/riscv/debug.c
+++ b/target/riscv/debug.c
@@ -624,7 +624,7 @@ void helper_itrigger_match(CPURISCVState *env)
 if (get_trigger_type(env, i) != TRIGGER_TYPE_INST_CNT) {
 continue;
 }
-if (check_itrigger_priv(env, i)) {
+if (!trigger_common_match(env, TRIGGER_TYPE_INST_CNT, i)) {
 continue;
 }
 count = itrigger_get_count(env, i);
-- 
2.34.1




[PATCH v3 3/4] target/riscv: Apply modularized matching conditions for watchpoint

2024-02-25 Thread Alvin Chang via
We have implemented trigger_common_match(), which checks if the enabled
privilege levels of the trigger match CPU's current privilege level.
Remove the related code in riscv_cpu_debug_check_watchpoint() and invoke
trigger_common_match() to check the privilege levels of the type 2 and
type 6 triggers for the watchpoints.

This commit also changes the behavior of looping the triggers. In
previous implementation, if we have a type 2 trigger and
env->virt_enabled is true, we directly return false to stop the loop.
Now we keep looping all the triggers until we find a matched trigger.

Only load/store bits and loaded/stored address should be further checked
in riscv_cpu_debug_check_watchpoint().

Signed-off-by: Alvin Chang 
---
 target/riscv/debug.c | 26 ++
 1 file changed, 6 insertions(+), 20 deletions(-)

diff --git a/target/riscv/debug.c b/target/riscv/debug.c
index b7b0fa8945..9f9f332019 100644
--- a/target/riscv/debug.c
+++ b/target/riscv/debug.c
@@ -899,13 +899,12 @@ bool riscv_cpu_debug_check_watchpoint(CPUState *cs, 
CPUWatchpoint *wp)
 for (i = 0; i < RV_MAX_TRIGGERS; i++) {
 trigger_type = get_trigger_type(env, i);
 
+if (!trigger_common_match(env, trigger_type, i)) {
+continue;
+}
+
 switch (trigger_type) {
 case TRIGGER_TYPE_AD_MATCH:
-/* type 2 trigger cannot be fired in VU/VS mode */
-if (env->virt_enabled) {
-return false;
-}
-
 ctrl = env->tdata1[i];
 addr = env->tdata2[i];
 flags = 0;
@@ -918,10 +917,7 @@ bool riscv_cpu_debug_check_watchpoint(CPUState *cs, 
CPUWatchpoint *wp)
 }
 
 if ((wp->flags & flags) && (wp->vaddr == addr)) {
-/* check U/S/M bit against current privilege level */
-if ((ctrl >> 3) & BIT(env->priv)) {
-return true;
-}
+return true;
 }
 break;
 case TRIGGER_TYPE_AD_MATCH6:
@@ -937,17 +933,7 @@ bool riscv_cpu_debug_check_watchpoint(CPUState *cs, 
CPUWatchpoint *wp)
 }
 
 if ((wp->flags & flags) && (wp->vaddr == addr)) {
-if (env->virt_enabled) {
-/* check VU/VS bit against current privilege level */
-if ((ctrl >> 23) & BIT(env->priv)) {
-return true;
-}
-} else {
-/* check U/S/M bit against current privilege level */
-if ((ctrl >> 3) & BIT(env->priv)) {
-return true;
-}
-}
+return true;
 }
 break;
 default:
-- 
2.34.1




[PATCH v3 1/4] target/riscv: Add functions for common matching conditions of trigger

2024-02-25 Thread Alvin Chang via
According to RISC-V Debug specification version 1.0 [1], there are
several common matching conditions before firing a trigger, including
the enabled privilege levels of the trigger.

This commit adds trigger_common_match() to prepare the common matching
conditions for the type 2/3/6 triggers. For now, we just implement
trigger_priv_match() to check if the enabled privilege levels of the
trigger match CPU's current privilege level.

[1]: https://github.com/riscv/riscv-debug-spec/releases/tag/1.0.0-rc1-asciidoc

Signed-off-by: Alvin Chang 
---
 target/riscv/debug.c | 70 
 1 file changed, 70 insertions(+)

diff --git a/target/riscv/debug.c b/target/riscv/debug.c
index e30d99cc2f..3891236b82 100644
--- a/target/riscv/debug.c
+++ b/target/riscv/debug.c
@@ -241,6 +241,76 @@ static void do_trigger_action(CPURISCVState *env, 
target_ulong trigger_index)
 }
 }
 
+/*
+ * Check the privilege level of specific trigger matches CPU's current 
privilege
+ * level.
+ */
+static bool trigger_priv_match(CPURISCVState *env, trigger_type_t type,
+   int trigger_index)
+{
+target_ulong ctrl = env->tdata1[trigger_index];
+
+switch (type) {
+case TRIGGER_TYPE_AD_MATCH:
+/* type 2 trigger cannot be fired in VU/VS mode */
+if (env->virt_enabled) {
+return false;
+}
+/* check U/S/M bit against current privilege level */
+if ((ctrl >> 3) & BIT(env->priv)) {
+return true;
+}
+break;
+case TRIGGER_TYPE_AD_MATCH6:
+if (env->virt_enabled) {
+/* check VU/VS bit against current privilege level */
+if ((ctrl >> 23) & BIT(env->priv)) {
+return true;
+}
+} else {
+/* check U/S/M bit against current privilege level */
+if ((ctrl >> 3) & BIT(env->priv)) {
+return true;
+}
+}
+break;
+case TRIGGER_TYPE_INST_CNT:
+if (env->virt_enabled) {
+/* check VU/VS bit against current privilege level */
+if ((ctrl >> 25) & BIT(env->priv)) {
+return true;
+}
+} else {
+/* check U/S/M bit against current privilege level */
+if ((ctrl >> 6) & BIT(env->priv)) {
+return true;
+}
+}
+break;
+case TRIGGER_TYPE_INT:
+case TRIGGER_TYPE_EXCP:
+case TRIGGER_TYPE_EXT_SRC:
+qemu_log_mask(LOG_UNIMP, "trigger type: %d is not supported\n", type);
+break;
+case TRIGGER_TYPE_NO_EXIST:
+case TRIGGER_TYPE_UNAVAIL:
+qemu_log_mask(LOG_GUEST_ERROR, "trigger type: %d does not exist\n",
+  type);
+break;
+default:
+g_assert_not_reached();
+}
+
+return false;
+}
+
+/* Common matching conditions for all types of the triggers. */
+static bool trigger_common_match(CPURISCVState *env, trigger_type_t type,
+ int trigger_index)
+{
+return trigger_priv_match(env, type, trigger_index);
+}
+
 /* type 2 trigger */
 
 static uint32_t type2_breakpoint_size(CPURISCVState *env, target_ulong ctrl)
-- 
2.34.1




[PATCH v2 1/4] target/riscv: Add functions for common matching conditions of trigger

2024-02-22 Thread Alvin Chang via
According to RISC-V Debug specification, there are several common
matching conditions before firing a trigger, including the enabled
privilege levels of the trigger.

This commit adds trigger_common_match() to prepare the common matching
conditions for the type 2/3/6 triggers. For now, we just implement
trigger_priv_match() to check if the enabled privilege levels of the
trigger match CPU's current privilege level.

Signed-off-by: Alvin Chang 
---
 target/riscv/debug.c | 70 
 1 file changed, 70 insertions(+)

diff --git a/target/riscv/debug.c b/target/riscv/debug.c
index e30d99cc2f..3891236b82 100644
--- a/target/riscv/debug.c
+++ b/target/riscv/debug.c
@@ -241,6 +241,76 @@ static void do_trigger_action(CPURISCVState *env, 
target_ulong trigger_index)
 }
 }
 
+/*
+ * Check the privilege level of specific trigger matches CPU's current 
privilege
+ * level.
+ */
+static bool trigger_priv_match(CPURISCVState *env, trigger_type_t type,
+   int trigger_index)
+{
+target_ulong ctrl = env->tdata1[trigger_index];
+
+switch (type) {
+case TRIGGER_TYPE_AD_MATCH:
+/* type 2 trigger cannot be fired in VU/VS mode */
+if (env->virt_enabled) {
+return false;
+}
+/* check U/S/M bit against current privilege level */
+if ((ctrl >> 3) & BIT(env->priv)) {
+return true;
+}
+break;
+case TRIGGER_TYPE_AD_MATCH6:
+if (env->virt_enabled) {
+/* check VU/VS bit against current privilege level */
+if ((ctrl >> 23) & BIT(env->priv)) {
+return true;
+}
+} else {
+/* check U/S/M bit against current privilege level */
+if ((ctrl >> 3) & BIT(env->priv)) {
+return true;
+}
+}
+break;
+case TRIGGER_TYPE_INST_CNT:
+if (env->virt_enabled) {
+/* check VU/VS bit against current privilege level */
+if ((ctrl >> 25) & BIT(env->priv)) {
+return true;
+}
+} else {
+/* check U/S/M bit against current privilege level */
+if ((ctrl >> 6) & BIT(env->priv)) {
+return true;
+}
+}
+break;
+case TRIGGER_TYPE_INT:
+case TRIGGER_TYPE_EXCP:
+case TRIGGER_TYPE_EXT_SRC:
+qemu_log_mask(LOG_UNIMP, "trigger type: %d is not supported\n", type);
+break;
+case TRIGGER_TYPE_NO_EXIST:
+case TRIGGER_TYPE_UNAVAIL:
+qemu_log_mask(LOG_GUEST_ERROR, "trigger type: %d does not exist\n",
+  type);
+break;
+default:
+g_assert_not_reached();
+}
+
+return false;
+}
+
+/* Common matching conditions for all types of the triggers. */
+static bool trigger_common_match(CPURISCVState *env, trigger_type_t type,
+ int trigger_index)
+{
+return trigger_priv_match(env, type, trigger_index);
+}
+
 /* type 2 trigger */
 
 static uint32_t type2_breakpoint_size(CPURISCVState *env, target_ulong ctrl)
-- 
2.34.1




[PATCH v2 0/4] RISC-V: Modularize common match conditions for trigger

2024-02-22 Thread Alvin Chang via
According to RISC-V Debug specification, the enabled privilege levels of
the trigger is common match conditions for all the types of the trigger.
This series modularize the code for checking the privilege levels of
type 2/3/6 triggers by implementing functions trigger_common_match()
and trigger_priv_match().

Additional match conditions, such as CSR tcontrol and textra, can be
further implemented into trigger_common_match() in the future.

Changes from v1:
- Fix typo
- Add commit description for changing behavior of looping the triggers
  when we check type 2 triggers.

Alvin Chang (4):
  target/riscv: Add functions for common matching conditions of trigger
  target/riscv: Apply modularized matching conditions for breakpoint
  target/riscv: Apply modularized matching conditions for watchpoint
  target/riscv: Apply modularized matching conditions for icount trigger

 target/riscv/debug.c | 124 +--
 1 file changed, 83 insertions(+), 41 deletions(-)

-- 
2.34.1




[PATCH v2 4/4] target/riscv: Apply modularized matching conditions for icount trigger

2024-02-22 Thread Alvin Chang via
We have implemented trigger_common_match(), which checks if the enabled
privilege levels of the trigger match CPU's current privilege level. We
can invoke trigger_common_match() to check the privilege levels of the
type 3 triggers.

Signed-off-by: Alvin Chang 
---
 target/riscv/debug.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/target/riscv/debug.c b/target/riscv/debug.c
index 9f9f332019..eb45e2c147 100644
--- a/target/riscv/debug.c
+++ b/target/riscv/debug.c
@@ -624,7 +624,7 @@ void helper_itrigger_match(CPURISCVState *env)
 if (get_trigger_type(env, i) != TRIGGER_TYPE_INST_CNT) {
 continue;
 }
-if (check_itrigger_priv(env, i)) {
+if (!trigger_common_match(env, TRIGGER_TYPE_INST_CNT, i)) {
 continue;
 }
 count = itrigger_get_count(env, i);
-- 
2.34.1




[PATCH v2 3/4] target/riscv: Apply modularized matching conditions for watchpoint

2024-02-22 Thread Alvin Chang via
We have implemented trigger_common_match(), which checks if the enabled
privilege levels of the trigger match CPU's current privilege level.
Remove the related code in riscv_cpu_debug_check_watchpoint() and invoke
trigger_common_match() to check the privilege levels of the type 2 and
type 6 triggers for the watchpoints.

This commit also changes the behavior of looping the triggers. In
previous implementation, if we have a type 2 trigger and
env->virt_enabled is true, we directly return false to stop the loop.
Now we keep looping all the triggers until we find a matched trigger.

Only load/store bits and loaded/stored address should be further checked
in riscv_cpu_debug_check_watchpoint().

Signed-off-by: Alvin Chang 
---
 target/riscv/debug.c | 26 ++
 1 file changed, 6 insertions(+), 20 deletions(-)

diff --git a/target/riscv/debug.c b/target/riscv/debug.c
index b7b0fa8945..9f9f332019 100644
--- a/target/riscv/debug.c
+++ b/target/riscv/debug.c
@@ -899,13 +899,12 @@ bool riscv_cpu_debug_check_watchpoint(CPUState *cs, 
CPUWatchpoint *wp)
 for (i = 0; i < RV_MAX_TRIGGERS; i++) {
 trigger_type = get_trigger_type(env, i);
 
+if (!trigger_common_match(env, trigger_type, i)) {
+continue;
+}
+
 switch (trigger_type) {
 case TRIGGER_TYPE_AD_MATCH:
-/* type 2 trigger cannot be fired in VU/VS mode */
-if (env->virt_enabled) {
-return false;
-}
-
 ctrl = env->tdata1[i];
 addr = env->tdata2[i];
 flags = 0;
@@ -918,10 +917,7 @@ bool riscv_cpu_debug_check_watchpoint(CPUState *cs, 
CPUWatchpoint *wp)
 }
 
 if ((wp->flags & flags) && (wp->vaddr == addr)) {
-/* check U/S/M bit against current privilege level */
-if ((ctrl >> 3) & BIT(env->priv)) {
-return true;
-}
+return true;
 }
 break;
 case TRIGGER_TYPE_AD_MATCH6:
@@ -937,17 +933,7 @@ bool riscv_cpu_debug_check_watchpoint(CPUState *cs, 
CPUWatchpoint *wp)
 }
 
 if ((wp->flags & flags) && (wp->vaddr == addr)) {
-if (env->virt_enabled) {
-/* check VU/VS bit against current privilege level */
-if ((ctrl >> 23) & BIT(env->priv)) {
-return true;
-}
-} else {
-/* check U/S/M bit against current privilege level */
-if ((ctrl >> 3) & BIT(env->priv)) {
-return true;
-}
-}
+return true;
 }
 break;
 default:
-- 
2.34.1




[PATCH v2 2/4] target/riscv: Apply modularized matching conditions for breakpoint

2024-02-22 Thread Alvin Chang via
We have implemented trigger_common_match(), which checks if the enabled
privilege levels of the trigger match CPU's current privilege level.
Remove the related code in riscv_cpu_debug_check_breakpoint() and invoke
trigger_common_match() to check the privilege levels of the type 2 and
type 6 triggers for the breakpoints.

This commit also changes the behavior of looping the triggers. In
previous implementation, if we have a type 2 trigger and
env->virt_enabled is true, we directly return false to stop the loop.
Now we keep looping all the triggers until we find a matched trigger.

Only the execution bit and the executed PC should be futher checked in
riscv_cpu_debug_check_breakpoint().

Signed-off-by: Alvin Chang 
---
 target/riscv/debug.c | 26 ++
 1 file changed, 6 insertions(+), 20 deletions(-)

diff --git a/target/riscv/debug.c b/target/riscv/debug.c
index 3891236b82..b7b0fa8945 100644
--- a/target/riscv/debug.c
+++ b/target/riscv/debug.c
@@ -855,21 +855,17 @@ bool riscv_cpu_debug_check_breakpoint(CPUState *cs)
 for (i = 0; i < RV_MAX_TRIGGERS; i++) {
 trigger_type = get_trigger_type(env, i);
 
+if (!trigger_common_match(env, trigger_type, i)) {
+continue;
+}
+
 switch (trigger_type) {
 case TRIGGER_TYPE_AD_MATCH:
-/* type 2 trigger cannot be fired in VU/VS mode */
-if (env->virt_enabled) {
-return false;
-}
-
 ctrl = env->tdata1[i];
 pc = env->tdata2[i];
 
 if ((ctrl & TYPE2_EXEC) && (bp->pc == pc)) {
-/* check U/S/M bit against current privilege level */
-if ((ctrl >> 3) & BIT(env->priv)) {
-return true;
-}
+return true;
 }
 break;
 case TRIGGER_TYPE_AD_MATCH6:
@@ -877,17 +873,7 @@ bool riscv_cpu_debug_check_breakpoint(CPUState *cs)
 pc = env->tdata2[i];
 
 if ((ctrl & TYPE6_EXEC) && (bp->pc == pc)) {
-if (env->virt_enabled) {
-/* check VU/VS bit against current privilege level */
-if ((ctrl >> 23) & BIT(env->priv)) {
-return true;
-}
-} else {
-/* check U/S/M bit against current privilege level */
-if ((ctrl >> 3) & BIT(env->priv)) {
-return true;
-}
-}
+return true;
 }
 break;
 default:
-- 
2.34.1




[PATCH 1/4] target/riscv: Add functions for common matching conditions of trigger

2024-02-18 Thread Alvin Chang via
According to RISC-V Debug specification, there are several common
matching conditions before firing a trigger, including the enabled
privilege levels of the trigger.

This commit adds trigger_common_match() to prepare the common matching
conditions for the type 2/3/6 triggers. For now, we just implement
trigger_priv_match() to check if the enabled privilege levels of the
trigger match CPU's current privilege level.

Signed-off-by: Alvin Chang 
---
 target/riscv/debug.c | 70 
 1 file changed, 70 insertions(+)

diff --git a/target/riscv/debug.c b/target/riscv/debug.c
index e30d99cc2f..7932233073 100644
--- a/target/riscv/debug.c
+++ b/target/riscv/debug.c
@@ -241,6 +241,76 @@ static void do_trigger_action(CPURISCVState *env, 
target_ulong trigger_index)
 }
 }
 
+/*
+ * Check the privilege level of specific trigger matches CPU's current 
privilege
+ * level.
+ */
+static bool trigger_priv_match(CPURISCVState *env, trigger_type_t type,
+   int trigger_index)
+{
+target_ulong ctrl = env->tdata1[trigger_index];
+
+switch (type) {
+case TRIGGER_TYPE_AD_MATCH:
+/* type 2 trigger cannot be fired in VU/VS mode */
+if (env->virt_enabled) {
+return false;
+}
+/* check U/S/M bit against current privilege level */
+if ((ctrl >> 3) & BIT(env->priv)) {
+return true;
+}
+break;
+case TRIGGER_TYPE_AD_MATCH6:
+if (env->virt_enabled) {
+/* check VU/VS bit against current privilege level */
+if ((ctrl >> 23) & BIT(env->priv)) {
+return true;
+}
+} else {
+/* check U/S/M bit against current privilege level */
+if ((ctrl >> 3) & BIT(env->priv)) {
+return true;
+}
+}
+break;
+case TRIGGER_TYPE_INST_CNT:
+if (env->virt_enabled) {
+/* check VU/VS bit against current privilege level */
+if ((ctrl >> 25) & BIT(env->priv)) {
+return true;
+}
+} else {
+/* check U/S/M bit against current privilege level */
+if ((ctrl >> 6) & BIT(env->priv)) {
+return true;
+}
+}
+break;
+case TRIGGER_TYPE_INT:
+case TRIGGER_TYPE_EXCP:
+case TRIGGER_TYPE_EXT_SRC:
+qemu_log_mask(LOG_UNIMP, "trigger type: %d is not supported\n", type);
+break;
+case TRIGGER_TYPE_NO_EXIST:
+case TRIGGER_TYPE_UNAVAIL:
+qemu_log_mask(LOG_GUEST_ERROR, "trigger type: %d does not exit\n",
+  type);
+break;
+default:
+g_assert_not_reached();
+}
+
+return false;
+}
+
+/* Common matching conditions for all types of the triggers. */
+static bool trigger_common_match(CPURISCVState *env, trigger_type_t type,
+ int trigger_index)
+{
+return trigger_priv_match(env, type, trigger_index);
+}
+
 /* type 2 trigger */
 
 static uint32_t type2_breakpoint_size(CPURISCVState *env, target_ulong ctrl)
-- 
2.34.1




[PATCH 0/4] RISC-V: Modularize common match conditions for trigger

2024-02-18 Thread Alvin Chang via
According to RISC-V Debug specification, the enabled privilege levels of
the trigger is common match conditions for all the types of the trigger.
This series modularize the code for checking the privilege levels of
type 2/3/6 triggers by implementing functions trigger_common_match()
and trigger_priv_match().

Additional match conditions, such as CSR tcontrol and textra, can be
further implemented into trigger_common_match() in the future.

Alvin Chang (4):
  target/riscv: Add functions for common matching conditions of trigger
  target/riscv: Apply modularized matching conditions for breakpoint
  target/riscv: Apply modularized matching conditions for watchpoint
  target/riscv: Apply modularized matching conditions for icount trigger

 target/riscv/debug.c | 124 +--
 1 file changed, 83 insertions(+), 41 deletions(-)

-- 
2.34.1




[PATCH 3/4] target/riscv: Apply modularized matching conditions for watchpoint

2024-02-18 Thread Alvin Chang via
We have implemented trigger_common_match(), which checks if the enabled
privilege levels of the trigger match CPU's current privilege level.
Remove the related code in riscv_cpu_debug_check_watchpoint() and invoke
trigger_common_match() to check the privilege levels of the type 2 and
type 6 triggers for the watchpoints.

Only load/store bits and loaded/stored address should be further checked
in riscv_cpu_debug_check_watchpoint().

Signed-off-by: Alvin Chang 
---
 target/riscv/debug.c | 26 ++
 1 file changed, 6 insertions(+), 20 deletions(-)

diff --git a/target/riscv/debug.c b/target/riscv/debug.c
index b971ed5d7a..67ba19c966 100644
--- a/target/riscv/debug.c
+++ b/target/riscv/debug.c
@@ -899,13 +899,12 @@ bool riscv_cpu_debug_check_watchpoint(CPUState *cs, 
CPUWatchpoint *wp)
 for (i = 0; i < RV_MAX_TRIGGERS; i++) {
 trigger_type = get_trigger_type(env, i);
 
+if (!trigger_common_match(env, trigger_type, i)) {
+continue;
+}
+
 switch (trigger_type) {
 case TRIGGER_TYPE_AD_MATCH:
-/* type 2 trigger cannot be fired in VU/VS mode */
-if (env->virt_enabled) {
-return false;
-}
-
 ctrl = env->tdata1[i];
 addr = env->tdata2[i];
 flags = 0;
@@ -918,10 +917,7 @@ bool riscv_cpu_debug_check_watchpoint(CPUState *cs, 
CPUWatchpoint *wp)
 }
 
 if ((wp->flags & flags) && (wp->vaddr == addr)) {
-/* check U/S/M bit against current privilege level */
-if ((ctrl >> 3) & BIT(env->priv)) {
-return true;
-}
+return true;
 }
 break;
 case TRIGGER_TYPE_AD_MATCH6:
@@ -937,17 +933,7 @@ bool riscv_cpu_debug_check_watchpoint(CPUState *cs, 
CPUWatchpoint *wp)
 }
 
 if ((wp->flags & flags) && (wp->vaddr == addr)) {
-if (env->virt_enabled) {
-/* check VU/VS bit against current privilege level */
-if ((ctrl >> 23) & BIT(env->priv)) {
-return true;
-}
-} else {
-/* check U/S/M bit against current privilege level */
-if ((ctrl >> 3) & BIT(env->priv)) {
-return true;
-}
-}
+return true;
 }
 break;
 default:
-- 
2.34.1




[PATCH 2/4] target/riscv: Apply modularized matching conditions for breakpoint

2024-02-18 Thread Alvin Chang via
We have implemented trigger_common_match(), which checks if the enabled
privilege levels of the trigger match CPU's current privilege level.
Remove the related code in riscv_cpu_debug_check_breakpoint() and invoke
trigger_common_match() to check the privilege levels of the type 2 and
type 6 triggers for the breakpoints.

Only the execution bit and the executed PC should be futher checked in
riscv_cpu_debug_check_breakpoint().

Signed-off-by: Alvin Chang 
---
 target/riscv/debug.c | 26 ++
 1 file changed, 6 insertions(+), 20 deletions(-)

diff --git a/target/riscv/debug.c b/target/riscv/debug.c
index 7932233073..b971ed5d7a 100644
--- a/target/riscv/debug.c
+++ b/target/riscv/debug.c
@@ -855,21 +855,17 @@ bool riscv_cpu_debug_check_breakpoint(CPUState *cs)
 for (i = 0; i < RV_MAX_TRIGGERS; i++) {
 trigger_type = get_trigger_type(env, i);
 
+if (!trigger_common_match(env, trigger_type, i)) {
+continue;
+}
+
 switch (trigger_type) {
 case TRIGGER_TYPE_AD_MATCH:
-/* type 2 trigger cannot be fired in VU/VS mode */
-if (env->virt_enabled) {
-return false;
-}
-
 ctrl = env->tdata1[i];
 pc = env->tdata2[i];
 
 if ((ctrl & TYPE2_EXEC) && (bp->pc == pc)) {
-/* check U/S/M bit against current privilege level */
-if ((ctrl >> 3) & BIT(env->priv)) {
-return true;
-}
+return true;
 }
 break;
 case TRIGGER_TYPE_AD_MATCH6:
@@ -877,17 +873,7 @@ bool riscv_cpu_debug_check_breakpoint(CPUState *cs)
 pc = env->tdata2[i];
 
 if ((ctrl & TYPE6_EXEC) && (bp->pc == pc)) {
-if (env->virt_enabled) {
-/* check VU/VS bit against current privilege level */
-if ((ctrl >> 23) & BIT(env->priv)) {
-return true;
-}
-} else {
-/* check U/S/M bit against current privilege level */
-if ((ctrl >> 3) & BIT(env->priv)) {
-return true;
-}
-}
+return true;
 }
 break;
 default:
-- 
2.34.1




[PATCH 4/4] target/riscv: Apply modularized matching conditions for icount trigger

2024-02-18 Thread Alvin Chang via
We have implemented trigger_common_match(), which checks if the enabled
privilege levels of the trigger match CPU's current privilege level. We
can invoke trigger_common_match() to check the privilege levels of the
type 3 triggers.

Signed-off-by: Alvin Chang 
---
 target/riscv/debug.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/target/riscv/debug.c b/target/riscv/debug.c
index 67ba19c966..de996a393c 100644
--- a/target/riscv/debug.c
+++ b/target/riscv/debug.c
@@ -624,7 +624,7 @@ void helper_itrigger_match(CPURISCVState *env)
 if (get_trigger_type(env, i) != TRIGGER_TYPE_INST_CNT) {
 continue;
 }
-if (check_itrigger_priv(env, i)) {
+if (!trigger_common_match(env, TRIGGER_TYPE_INST_CNT, i)) {
 continue;
 }
 count = itrigger_get_count(env, i);
-- 
2.34.1




[PATCH 4/4] target/riscv: Set the value of CSR tcontrol when mret is executed

2024-02-15 Thread Alvin Chang via
The RISC-V debug specification defines the following operation for CSR
tcontrol when "mret" is executed:
- tcontrol.MTE is set to the value of tcontrol.MPTE

This commit implements the above operation into helper_mret().

Note that from tech-debug mailing list:
https://lists.riscv.org/g/tech-debug/topic/102702615#1461
The debug specification does not mention the operation to tcontrol.MPTE
when "mret" is executed. Therefore, we just keep its current value.

Signed-off-by: Alvin Chang 
---
 target/riscv/op_helper.c | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/target/riscv/op_helper.c b/target/riscv/op_helper.c
index f414aaebdb..12822b3afa 100644
--- a/target/riscv/op_helper.c
+++ b/target/riscv/op_helper.c
@@ -347,6 +347,12 @@ target_ulong helper_mret(CPURISCVState *env)
 mstatus = set_field(mstatus, MSTATUS_MPRV, 0);
 }
 env->mstatus = mstatus;
+
+uint64_t tcontrol = env->tcontrol;
+tcontrol = set_field(tcontrol, TCONTROL_MTE,
+ get_field(tcontrol, TCONTROL_MPTE));
+env->tcontrol = tcontrol;
+
 riscv_cpu_set_mode(env, prev_priv);
 
 if (riscv_has_ext(env, RVH)) {
-- 
2.34.1




[PATCH 1/4] target/riscv: Add CSR tcontrol of debug trigger module

2024-02-15 Thread Alvin Chang via
The RISC-V debug specification defines an optional CSR "tcontrol" within
the trigger module. This commit adds its read/write operations and
related bit-field definitions.

Signed-off-by: Alvin Chang 
---
 target/riscv/cpu.h  |  1 +
 target/riscv/cpu_bits.h |  3 +++
 target/riscv/csr.c  | 15 +++
 3 files changed, 19 insertions(+)

diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
index f52dce78ba..f9ae3e3025 100644
--- a/target/riscv/cpu.h
+++ b/target/riscv/cpu.h
@@ -364,6 +364,7 @@ struct CPUArchState {
 target_ulong tdata1[RV_MAX_TRIGGERS];
 target_ulong tdata2[RV_MAX_TRIGGERS];
 target_ulong tdata3[RV_MAX_TRIGGERS];
+target_ulong tcontrol;
 target_ulong mcontext;
 struct CPUBreakpoint *cpu_breakpoint[RV_MAX_TRIGGERS];
 struct CPUWatchpoint *cpu_watchpoint[RV_MAX_TRIGGERS];
diff --git a/target/riscv/cpu_bits.h b/target/riscv/cpu_bits.h
index fc2068ee4d..3b3a7a0fa4 100644
--- a/target/riscv/cpu_bits.h
+++ b/target/riscv/cpu_bits.h
@@ -353,6 +353,7 @@
 #define CSR_TDATA2  0x7a2
 #define CSR_TDATA3  0x7a3
 #define CSR_TINFO   0x7a4
+#define CSR_TCONTROL0x7a5
 #define CSR_MCONTEXT0x7a8
 
 /* Debug Mode Registers */
@@ -900,6 +901,8 @@ typedef enum RISCVException {
 #define JVT_BASE   (~0x3F)
 
 /* Debug Sdtrig CSR masks */
+#define TCONTROL_MTE   BIT(3)
+#define TCONTROL_MPTE  BIT(7)
 #define MCONTEXT32 0x003F
 #define MCONTEXT64 0x1FFFULL
 #define MCONTEXT32_HCONTEXT0x007F
diff --git a/target/riscv/csr.c b/target/riscv/csr.c
index d4e8ac13b9..816c530481 100644
--- a/target/riscv/csr.c
+++ b/target/riscv/csr.c
@@ -3937,6 +3937,20 @@ static RISCVException read_tinfo(CPURISCVState *env, int 
csrno,
 return RISCV_EXCP_NONE;
 }
 
+static RISCVException read_tcontrol(CPURISCVState *env, int csrno,
+target_ulong *val)
+{
+*val = env->tcontrol;
+return RISCV_EXCP_NONE;
+}
+
+static RISCVException write_tcontrol(CPURISCVState *env, int csrno,
+ target_ulong val)
+{
+env->tcontrol = val & (TCONTROL_MPTE | TCONTROL_MTE);
+return RISCV_EXCP_NONE;
+}
+
 static RISCVException read_mcontext(CPURISCVState *env, int csrno,
 target_ulong *val)
 {
@@ -4861,6 +4875,7 @@ riscv_csr_operations csr_ops[CSR_TABLE_SIZE] = {
 [CSR_TDATA2]=  { "tdata2",   debug, read_tdata,write_tdata},
 [CSR_TDATA3]=  { "tdata3",   debug, read_tdata,write_tdata},
 [CSR_TINFO] =  { "tinfo",debug, read_tinfo,write_ignore   },
+[CSR_TCONTROL]  =  { "tcontrol", debug, read_tcontrol, write_tcontrol },
 [CSR_MCONTEXT]  =  { "mcontext", debug, read_mcontext, write_mcontext },
 
 /* User Pointer Masking */
-- 
2.34.1




[PATCH 3/4] target/riscv: Set the value of CSR tcontrol when trapping to M-mode

2024-02-15 Thread Alvin Chang via
>From the RISC-V debug specification, it defines the following operations
for CSR tcontrol when any trap into M-mode is taken:
1. tcontrol.MPTE is set to the value of tcontrol.MTE
2. tcontrol.MTE is set to 0

This commit implements the above operations into
riscv_cpu_do_interrupt().

Signed-off-by: Alvin Chang 
---
 target/riscv/cpu_helper.c | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
index d462d95ee1..037ae21062 100644
--- a/target/riscv/cpu_helper.c
+++ b/target/riscv/cpu_helper.c
@@ -1806,6 +1806,12 @@ void riscv_cpu_do_interrupt(CPUState *cs)
 riscv_cpu_set_virt_enabled(env, 0);
 }
 
+/* Trapping to M-mode. Set tcontrol CSR in debug Sdtrig extension. */
+s = env->tcontrol;
+s = set_field(s, TCONTROL_MPTE, get_field(s, TCONTROL_MTE));
+s = set_field(s, TCONTROL_MTE, 0);
+env->tcontrol = s;
+
 s = env->mstatus;
 s = set_field(s, MSTATUS_MPIE, get_field(s, MSTATUS_MIE));
 s = set_field(s, MSTATUS_MPP, env->priv);
-- 
2.34.1




[PATCH 0/4] RISC-V: Implement CSR tcontrol in debug spec

2024-02-15 Thread Alvin Chang via
The RISC-V Debug specification defines CSR "tcontrol" in the trigger
module:
  https://github.com/riscv/riscv-debug-spec

This series implements it and the related operations.

Alvin Chang (4):
  target/riscv: Add CSR tcontrol of debug trigger module
  target/riscv: Reset CSR tcontrol when the trigger module resets
  target/riscv: Set the value of CSR tcontrol when trapping to M-mode
  target/riscv: Set the value of CSR tcontrol when mret is executed

 target/riscv/cpu.h|  1 +
 target/riscv/cpu_bits.h   |  3 +++
 target/riscv/cpu_helper.c |  6 ++
 target/riscv/csr.c| 15 +++
 target/riscv/debug.c  |  1 +
 target/riscv/op_helper.c  |  6 ++
 6 files changed, 32 insertions(+)

-- 
2.34.1




[PATCH 2/4] target/riscv: Reset CSR tcontrol when the trigger module resets

2024-02-15 Thread Alvin Chang via
When the trigger module resets, reset the value of CSR tcontrol as zero.

Signed-off-by: Alvin Chang 
---
 target/riscv/debug.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/target/riscv/debug.c b/target/riscv/debug.c
index e30d99cc2f..e3832a643e 100644
--- a/target/riscv/debug.c
+++ b/target/riscv/debug.c
@@ -941,5 +941,6 @@ void riscv_trigger_reset_hold(CPURISCVState *env)
 timer_del(env->itrigger_timer[i]);
 }
 
+env->tcontrol = 0;
 env->mcontext = 0;
 }
-- 
2.34.1




[PATCH v2] target/riscv: Implement optional CSR mcontext of debug Sdtrig extension

2023-12-19 Thread Alvin Chang via
The debug Sdtrig extension defines an CSR "mcontext". This commit
implements its predicate and read/write operations into CSR table.
Its value is reset as 0 when the trigger module is reset.

Signed-off-by: Alvin Chang 
---
Changes from v1: Remove dedicated cfg, always implement mcontext.

 target/riscv/cpu.h  |  1 +
 target/riscv/cpu_bits.h |  7 +++
 target/riscv/csr.c  | 36 +++-
 target/riscv/debug.c|  2 ++
 4 files changed, 41 insertions(+), 5 deletions(-)

diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
index d74b361..e117641 100644
--- a/target/riscv/cpu.h
+++ b/target/riscv/cpu.h
@@ -345,6 +345,7 @@ struct CPUArchState {
 target_ulong tdata1[RV_MAX_TRIGGERS];
 target_ulong tdata2[RV_MAX_TRIGGERS];
 target_ulong tdata3[RV_MAX_TRIGGERS];
+target_ulong mcontext;
 struct CPUBreakpoint *cpu_breakpoint[RV_MAX_TRIGGERS];
 struct CPUWatchpoint *cpu_watchpoint[RV_MAX_TRIGGERS];
 QEMUTimer *itrigger_timer[RV_MAX_TRIGGERS];
diff --git a/target/riscv/cpu_bits.h b/target/riscv/cpu_bits.h
index ebd7917..3296648 100644
--- a/target/riscv/cpu_bits.h
+++ b/target/riscv/cpu_bits.h
@@ -361,6 +361,7 @@
 #define CSR_TDATA2  0x7a2
 #define CSR_TDATA3  0x7a3
 #define CSR_TINFO   0x7a4
+#define CSR_MCONTEXT0x7a8
 
 /* Debug Mode Registers */
 #define CSR_DCSR0x7b0
@@ -905,4 +906,10 @@ typedef enum RISCVException {
 /* JVT CSR bits */
 #define JVT_MODE   0x3F
 #define JVT_BASE   (~0x3F)
+
+/* Debug Sdtrig CSR masks */
+#define MCONTEXT32 0x003F
+#define MCONTEXT64 0x1FFFULL
+#define MCONTEXT32_HCONTEXT0x007F
+#define MCONTEXT64_HCONTEXT0x3FFFULL
 #endif
diff --git a/target/riscv/csr.c b/target/riscv/csr.c
index fde7ce1..ff1e128 100644
--- a/target/riscv/csr.c
+++ b/target/riscv/csr.c
@@ -3900,6 +3900,31 @@ static RISCVException read_tinfo(CPURISCVState *env, int 
csrno,
 return RISCV_EXCP_NONE;
 }
 
+static RISCVException read_mcontext(CPURISCVState *env, int csrno,
+target_ulong *val)
+{
+*val = env->mcontext;
+return RISCV_EXCP_NONE;
+}
+
+static RISCVException write_mcontext(CPURISCVState *env, int csrno,
+ target_ulong val)
+{
+bool rv32 = riscv_cpu_mxl(env) == MXL_RV32 ? true : false;
+int32_t mask;
+
+if (riscv_has_ext(env, RVH)) {
+/* Spec suggest 7-bit for RV32 and 14-bit for RV64 w/ H extension */
+mask = rv32 ? MCONTEXT32_HCONTEXT : MCONTEXT64_HCONTEXT;
+} else {
+/* Spec suggest 6-bit for RV32 and 13-bit for RV64 w/o H extension */
+mask = rv32 ? MCONTEXT32 : MCONTEXT64;
+}
+
+env->mcontext = val & mask;
+return RISCV_EXCP_NONE;
+}
+
 /*
  * Functions to access Pointer Masking feature registers
  * We have to check if current priv lvl could modify
@@ -4794,11 +4819,12 @@ riscv_csr_operations csr_ops[CSR_TABLE_SIZE] = {
 [CSR_PMPADDR15] =  { "pmpaddr15", pmp, read_pmpaddr, write_pmpaddr },
 
 /* Debug CSRs */
-[CSR_TSELECT]   =  { "tselect", debug, read_tselect, write_tselect },
-[CSR_TDATA1]=  { "tdata1",  debug, read_tdata,   write_tdata   },
-[CSR_TDATA2]=  { "tdata2",  debug, read_tdata,   write_tdata   },
-[CSR_TDATA3]=  { "tdata3",  debug, read_tdata,   write_tdata   },
-[CSR_TINFO] =  { "tinfo",   debug, read_tinfo,   write_ignore  },
+[CSR_TSELECT]   =  { "tselect",  debug, read_tselect,  write_tselect  },
+[CSR_TDATA1]=  { "tdata1",   debug, read_tdata,write_tdata},
+[CSR_TDATA2]=  { "tdata2",   debug, read_tdata,write_tdata},
+[CSR_TDATA3]=  { "tdata3",   debug, read_tdata,write_tdata},
+[CSR_TINFO] =  { "tinfo",debug, read_tinfo,write_ignore   },
+[CSR_MCONTEXT]  =  { "mcontext", debug, read_mcontext, write_mcontext },
 
 /* User Pointer Masking */
 [CSR_UMTE]={ "umte",pointer_masking, read_umte,  write_umte },
diff --git a/target/riscv/debug.c b/target/riscv/debug.c
index 4945d1a..e30d99c 100644
--- a/target/riscv/debug.c
+++ b/target/riscv/debug.c
@@ -940,4 +940,6 @@ void riscv_trigger_reset_hold(CPURISCVState *env)
 env->cpu_watchpoint[i] = NULL;
 timer_del(env->itrigger_timer[i]);
 }
+
+env->mcontext = 0;
 }
-- 
2.34.1




[PATCH] target/riscv: Implement optional CSR mcontext of debug Sdtrig extension

2023-12-16 Thread Alvin Chang via
The debug Sdtrig extension defines an optional CSR "mcontext". Since it
is optional, this commit adds new CPU configuration
"ext_sdtrig_mcontext" and uses property "sdtrig_mcontext" to control
whether it is implemented or not. Its predicate and read/write
operations are also implemented into CSR table. Its value is reset as 0
when the trigger module is reset.

Signed-off-by: Alvin Chang 
---
 target/riscv/cpu.c  |  4 
 target/riscv/cpu.h  |  1 +
 target/riscv/cpu_bits.h |  7 +++
 target/riscv/cpu_cfg.h  |  1 +
 target/riscv/csr.c  | 36 
 target/riscv/debug.c|  2 ++
 6 files changed, 51 insertions(+)

diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
index 83c7c0c..dff757f 100644
--- a/target/riscv/cpu.c
+++ b/target/riscv/cpu.c
@@ -1479,6 +1479,10 @@ Property riscv_cpu_options[] = {
 DEFINE_PROP_UINT16("cbom_blocksize", RISCVCPU, cfg.cbom_blocksize, 64),
 DEFINE_PROP_UINT16("cboz_blocksize", RISCVCPU, cfg.cboz_blocksize, 64),
 
+/* Optional CSR of debug Sdtrig extension */
+DEFINE_PROP_BOOL("sdtrig_mcontext", RISCVCPU, cfg.ext_sdtrig_mcontext,
+ false),
+
 DEFINE_PROP_END_OF_LIST(),
 };
 
diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
index d74b361..e117641 100644
--- a/target/riscv/cpu.h
+++ b/target/riscv/cpu.h
@@ -345,6 +345,7 @@ struct CPUArchState {
 target_ulong tdata1[RV_MAX_TRIGGERS];
 target_ulong tdata2[RV_MAX_TRIGGERS];
 target_ulong tdata3[RV_MAX_TRIGGERS];
+target_ulong mcontext;
 struct CPUBreakpoint *cpu_breakpoint[RV_MAX_TRIGGERS];
 struct CPUWatchpoint *cpu_watchpoint[RV_MAX_TRIGGERS];
 QEMUTimer *itrigger_timer[RV_MAX_TRIGGERS];
diff --git a/target/riscv/cpu_bits.h b/target/riscv/cpu_bits.h
index ebd7917..3296648 100644
--- a/target/riscv/cpu_bits.h
+++ b/target/riscv/cpu_bits.h
@@ -361,6 +361,7 @@
 #define CSR_TDATA2  0x7a2
 #define CSR_TDATA3  0x7a3
 #define CSR_TINFO   0x7a4
+#define CSR_MCONTEXT0x7a8
 
 /* Debug Mode Registers */
 #define CSR_DCSR0x7b0
@@ -905,4 +906,10 @@ typedef enum RISCVException {
 /* JVT CSR bits */
 #define JVT_MODE   0x3F
 #define JVT_BASE   (~0x3F)
+
+/* Debug Sdtrig CSR masks */
+#define MCONTEXT32 0x003F
+#define MCONTEXT64 0x1FFFULL
+#define MCONTEXT32_HCONTEXT0x007F
+#define MCONTEXT64_HCONTEXT0x3FFFULL
 #endif
diff --git a/target/riscv/cpu_cfg.h b/target/riscv/cpu_cfg.h
index f4605fb..4f1cb04 100644
--- a/target/riscv/cpu_cfg.h
+++ b/target/riscv/cpu_cfg.h
@@ -147,6 +147,7 @@ struct RISCVCPUConfig {
 bool pmp;
 bool debug;
 bool misa_w;
+bool ext_sdtrig_mcontext;
 
 bool short_isa_string;
 
diff --git a/target/riscv/csr.c b/target/riscv/csr.c
index fde7ce1..0b68787 100644
--- a/target/riscv/csr.c
+++ b/target/riscv/csr.c
@@ -549,6 +549,15 @@ static RISCVException debug(CPURISCVState *env, int csrno)
 
 return RISCV_EXCP_ILLEGAL_INST;
 }
+
+static RISCVException sdtrig_mcontext(CPURISCVState *env, int csrno)
+{
+if (riscv_cpu_cfg(env)->debug && riscv_cpu_cfg(env)->ext_sdtrig_mcontext) {
+return RISCV_EXCP_NONE;
+}
+
+return RISCV_EXCP_ILLEGAL_INST;
+}
 #endif
 
 static RISCVException seed(CPURISCVState *env, int csrno)
@@ -3900,6 +3909,31 @@ static RISCVException read_tinfo(CPURISCVState *env, int 
csrno,
 return RISCV_EXCP_NONE;
 }
 
+static RISCVException read_mcontext(CPURISCVState *env, int csrno,
+target_ulong *val)
+{
+*val = env->mcontext;
+return RISCV_EXCP_NONE;
+}
+
+static RISCVException write_mcontext(CPURISCVState *env, int csrno,
+ target_ulong val)
+{
+bool rv32 = riscv_cpu_mxl(env) == MXL_RV32 ? true : false;
+int32_t mask;
+
+if (riscv_has_ext(env, RVH)) {
+/* Spec suggest 7-bit for RV32 and 14-bit for RV64 w/ H extension */
+mask = rv32 ? MCONTEXT32_HCONTEXT : MCONTEXT64_HCONTEXT;
+} else {
+/* Spec suggest 6-bit for RV32 and 13-bit for RV64 w/o H extension */
+mask = rv32 ? MCONTEXT32 : MCONTEXT64;
+}
+
+env->mcontext = val & mask;
+return RISCV_EXCP_NONE;
+}
+
 /*
  * Functions to access Pointer Masking feature registers
  * We have to check if current priv lvl could modify
@@ -4799,6 +4833,8 @@ riscv_csr_operations csr_ops[CSR_TABLE_SIZE] = {
 [CSR_TDATA2]=  { "tdata2",  debug, read_tdata,   write_tdata   },
 [CSR_TDATA3]=  { "tdata3",  debug, read_tdata,   write_tdata   },
 [CSR_TINFO] =  { "tinfo",   debug, read_tinfo,   write_ignore  },
+[CSR_MCONTEXT]  =  { "mcontext", sdtrig_mcontext,read_mcontext,
+ write_mcontext},
 
 /* User Pointer Masking */
 [CSR_UMTE]={ "umte",pointer_masking, read_umte,  

[PATCH v5] target/riscv: update checks on writing pmpcfg for Smepmp to version 1.0

2023-11-13 Thread Alvin Chang via
Current checks on writing pmpcfg for Smepmp follows Smepmp version
0.9.1. However, Smepmp specification has already been ratified, and
there are some differences between version 0.9.1 and 1.0. In this
commit we update the checks of writing pmpcfg to follow Smepmp version
1.0.

When mseccfg.MML is set, the constraints to modify PMP rules are:
1. Locked rules cannot be removed or modified until a PMP reset, unless
   mseccfg.RLB is set.
2. From Smepmp specification version 1.0, chapter 2 section 4b:
   Adding a rule with executable privileges that either is M-mode-only
   or a locked Shared-Region is not possible and such pmpcfg writes are
   ignored, leaving pmpcfg unchanged.

The commit transfers the value of pmpcfg into the index of the Smepmp
truth table, and checks the rules by aforementioned specification
changes.

Signed-off-by: Alvin Chang 
---
Changes from v4: Rebase on master.

Changes from v3: Modify "epmp_operation" to "smepmp_operation".

Changes from v2: Adopt switch case ranges and numerical order.

Changes from v1: Convert ePMP over to Smepmp.

 target/riscv/pmp.c | 40 
 1 file changed, 32 insertions(+), 8 deletions(-)

diff --git a/target/riscv/pmp.c b/target/riscv/pmp.c
index 162e88a90a..4069514069 100644
--- a/target/riscv/pmp.c
+++ b/target/riscv/pmp.c
@@ -102,16 +102,40 @@ static bool pmp_write_cfg(CPURISCVState *env, uint32_t 
pmp_index, uint8_t val)
 locked = false;
 }
 
-/* mseccfg.MML is set */
-if (MSECCFG_MML_ISSET(env)) {
-/* not adding execute bit */
-if ((val & PMP_LOCK) != 0 && (val & PMP_EXEC) != PMP_EXEC) {
+/*
+ * mseccfg.MML is set. Locked rules cannot be removed or modified
+ * until a PMP reset. Besides, from Smepmp specification version 
1.0
+ * , chapter 2 section 4b says:
+ * Adding a rule with executable privileges that either is
+ * M-mode-only or a locked Shared-Region is not possible and such
+ * pmpcfg writes are ignored, leaving pmpcfg unchanged.
+ */
+if (MSECCFG_MML_ISSET(env) && !pmp_is_locked(env, pmp_index)) {
+/*
+ * Convert the PMP permissions to match the truth table in the
+ * Smepmp spec.
+ */
+const uint8_t smepmp_operation =
+((val & PMP_LOCK) >> 4) | ((val & PMP_READ) << 2) |
+(val & PMP_WRITE) | ((val & PMP_EXEC) >> 2);
+
+switch (smepmp_operation) {
+case 0 ... 8:
 locked = false;
-}
-/* shared region and not adding X bit */
-if ((val & PMP_LOCK) != PMP_LOCK &&
-(val & 0x7) != (PMP_WRITE | PMP_EXEC)) {
+break;
+case 9 ... 11:
+break;
+case 12:
+locked = false;
+break;
+case 13:
+break;
+case 14:
+case 15:
 locked = false;
+break;
+default:
+g_assert_not_reached();
 }
 }
 } else {
-- 
2.34.1