From ae50b69312b3ef308d0256148c7dd9415b59ffe1 Mon Sep 17 00:00:00 2001
From: Evgeniy Naydanov <evgeniy.naydanov@syntacore.com>
Date: Fri, 6 Sep 2024 12:12:26 +0300
Subject: [PATCH] [debug] Reserve triggers propperly in HwbpManual

https://github.com/riscv-collab/riscv-openocd/pull/1111 introduces a
change in OpenOCD behavior: a manual trigger should be manually removed
to step/resume from it.
This was not concidered in previous stop-gap solutions.

This commit on the other hand:
1. Determins if `reserve trigger` is supported by the target.
2. Changes `HwbpManual` test's behavior depending on that.
3. Cleans up some minor mistakes in `HwbpManual`
---
 debug/gdbserver.py | 84 +++++++++++++++++++++++++++++++++++-----------
 1 file changed, 64 insertions(+), 20 deletions(-)

diff --git a/debug/gdbserver.py b/debug/gdbserver.py
index 1280878d3..5c7efa181 100755
--- a/debug/gdbserver.py
+++ b/debug/gdbserver.py
@@ -672,10 +672,56 @@ def set_field(reg, mask, val):
 class HwbpManual(DebugTest):
     """Make sure OpenOCD behaves "normal" when the user sets a trigger by
     writing the trigger registers themselves directly."""
+
+    reserve_trigger_supported = None
+
     def early_applicable(self):
         return self.target.support_manual_hwbp and \
             self.hart.instruction_hardware_breakpoint_count >= 1
 
+    def check_reserve_trigger_support(self):
+        not_supp_msg = "RESERVE_TRIGGER_NOT_SUPPORTED"
+        self.reserve_trigger_supported = not_supp_msg not in self.gdb.command(
+                    "monitor if [catch {riscv reserve_trigger 0 on} e] {echo " +
+                    not_supp_msg + "}").splitlines()
+
+    def reserve_trigger(self, t, on):
+        if self.reserve_trigger_supported is None:
+            self.check_reserve_trigger_support()
+        if self.reserve_trigger_supported:
+            on_str = "on" if on else "off"
+            self.gdb.command(
+                    f"monitor riscv reserve_trigger {t} {on_str}")
+
+    def set_manual_trigger(self, tdata1, tdata2):
+        for tselect in itertools.count(0):
+            self.gdb.p(f"$tselect={tselect}")
+            if self.gdb.p("$tselect") != tselect:
+                raise TestNotApplicable
+
+            self.reserve_trigger(tselect, True)
+
+            # Need to disable the trigger before writing tdata2
+            self.gdb.p("$tdata1=0")
+            # Need to write a valid value to tdata2 before writing tdata1
+            self.gdb.p(f"$tdata2=0x{tdata2:x}")
+            self.gdb.p(f"$tdata1=0x{tdata1:x}")
+
+            tdata2_rb = self.gdb.p("$tdata2")
+            tdata1_rb = self.gdb.p("$tdata1")
+            if tdata1_rb == tdata1 and tdata2_rb == tdata2:
+                return tselect
+
+            type_rb = tdata1_rb & MCONTROL_TYPE(self.hart.xlen)
+            type_none = set_field(0, MCONTROL_TYPE(self.hart.xlen),
+                                  MCONTROL_TYPE_NONE)
+            if type_rb == type_none:
+                raise TestNotApplicable
+
+            self.gdb.p("$tdata1=0")
+            self.reserve_trigger(tselect, False)
+        assert False
+
     def test(self):
         if not self.hart.honors_tdata1_hmode:
             # Run to main before setting the breakpoint, because startup code
@@ -684,6 +730,9 @@ def test(self):
             self.gdb.c()
 
         self.gdb.command("delete")
+
+        self.check_reserve_trigger_support()
+
         #self.gdb.hbreak("rot13")
         tdata1 = MCONTROL_DMODE(self.hart.xlen)
         tdata1 = set_field(tdata1, MCONTROL_TYPE(self.hart.xlen),
@@ -692,26 +741,9 @@ def test(self):
         tdata1 = set_field(tdata1, MCONTROL_MATCH, MCONTROL_MATCH_EQUAL)
         tdata1 |= MCONTROL_M | MCONTROL_S | MCONTROL_U | MCONTROL_EXECUTE
 
-        tselect = 0
-        while True:
-            self.gdb.p(f"$tselect={tselect}")
-            value = self.gdb.p("$tselect")
-            if value != tselect:
-                raise TestNotApplicable
-            # Need to disable the trigger before writing tdata2
-            self.gdb.p("$tdata1=0")
-            # Need to write a valid value to tdata2 before writing tdata1
-            self.gdb.p("$tdata2=&rot13")
-            self.gdb.p(f"$tdata1=0x{tdata1:x}")
-            value = self.gdb.p("$tdata1")
-            if value == tdata1:
-                break
-            if value & MCONTROL_TYPE(self.hart.xlen) == MCONTROL_TYPE_NONE:
-                raise TestNotApplicable
-            self.gdb.p("$tdata1=0")
-            tselect += 1
+        tdata2 = self.gdb.p("&rot13")
 
-        self.gdb.command(f"monitor riscv reserve_trigger {tselect} on")
+        tselect = self.set_manual_trigger(tdata1, tdata2)
 
         # The breakpoint should be hit exactly 2 times.
         for _ in range(2):
@@ -728,14 +760,26 @@ def test(self):
         self.gdb.c()
         before = self.gdb.p("$pc")
         assertEqual(before, self.gdb.p("&crc32a"))
+
         self.gdb.stepi()
         after = self.gdb.p("$pc")
-        assertNotEqual(before, after)
+        if self.reserve_trigger_supported:
+            # OpenOCD should not disable reserved triggers by itself.
+            assertEqual(before, after)
+        else:
+            assertNotEqual(before, after)
 
         # Remove the manual HW breakpoint.
         assertEqual(tselect, self.gdb.p("$tselect"))
         self.gdb.p("$tdata1=0")
 
+        if self.reserve_trigger_supported:
+            self.gdb.stepi()
+            after = self.gdb.p("$pc")
+            assertNotEqual(before, after)
+
+        self.reserve_trigger_supported = None
+
         self.gdb.b("_exit")
         self.exit()