From fde58a43659d6674b263ac6ec65d6570cfedadb0 Mon Sep 17 00:00:00 2001
From: Jan Vrany <jan.vrany@labware.com>
Date: Tue, 8 Aug 2023 14:36:23 +0100
Subject: [PATCH 1/3] arch-power: Fix reporting traps to GDB

Due to inverted logic in POWER fault handlers, unimplemented opcode and
trap faults did not report trap to GDB (if connected). This commit fixes
the problem.

While at it, I opted to use `if (! ...) { panic(...) }` rather than
`panic_if(...)`. I find it easier to understand in this case.

Change-Id: I6cd5dfd5f6546b8541d685e877afef21540d6824
---
 src/arch/power/faults.cc | 20 ++++++++++++--------
 1 file changed, 12 insertions(+), 8 deletions(-)

diff --git a/src/arch/power/faults.cc b/src/arch/power/faults.cc
index 0d8f2ddd68..77fc8cba96 100644
--- a/src/arch/power/faults.cc
+++ b/src/arch/power/faults.cc
@@ -42,24 +42,28 @@ namespace PowerISA
 void
 UnimplementedOpcodeFault::invoke(ThreadContext *tc, const StaticInstPtr &inst)
 {
-    panic_if(tc->getSystemPtr()->trapToGdb(GDBSignal::ILL, tc->contextId()),
-             "Unimplemented opcode encountered at virtual address %#x\n",
-             tc->pcState().instAddr());
+    if (! tc->getSystemPtr()->trapToGdb(GDBSignal::ILL, tc->contextId()) ) {
+        panic("Unimplemented opcode encountered at virtual address %#x\n",
+              tc->pcState().instAddr());
+    }
 }
 
 void
 AlignmentFault::invoke(ThreadContext *tc, const StaticInstPtr &inst)
 {
-    panic_if(!tc->getSystemPtr()->trapToGdb(GDBSignal::BUS, tc->contextId()),
-             "Alignment fault when accessing virtual address %#x\n", vaddr);
+    if (! tc->getSystemPtr()->trapToGdb(GDBSignal::BUS, tc->contextId()) ) {
+        panic("Alignment fault when accessing virtual address %#x\n",
+              vaddr);
+    }
 }
 
 void
 TrapFault::invoke(ThreadContext *tc, const StaticInstPtr &inst)
 {
-    panic_if(tc->getSystemPtr()->trapToGdb(GDBSignal::TRAP, tc->contextId()),
-             "Trap encountered at virtual address %#x\n",
-             tc->pcState().instAddr());
+    if (! tc->getSystemPtr()->trapToGdb(GDBSignal::TRAP, tc->contextId()) ) {
+        panic("Trap encountered at virtual address %#x\n",
+              tc->pcState().instAddr());
+    }
 }
 
 } // namespace PowerISA

From 546b3eac7df292afc387067216b49a485e357923 Mon Sep 17 00:00:00 2001
From: Jan Vrany <jan.vrany@labware.com>
Date: Tue, 8 Aug 2023 15:07:06 +0100
Subject: [PATCH 2/3] arch-riscv: Do not advance PC when handling faults in SE
 mode

On RISC-V when trap occurs the contents of PC register contains the
address of instruction that caused that trap (as opposed to the address
of instruction following it in instruction stream). Therefore this commit
does not advance the PC before reporting trap in SE mode.

Change-Id: I83f3766cff276312cefcf1b4ac6e78a6569846b9
---
 src/arch/riscv/faults.cc | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/src/arch/riscv/faults.cc b/src/arch/riscv/faults.cc
index a929902e8b..9bea0668e8 100644
--- a/src/arch/riscv/faults.cc
+++ b/src/arch/riscv/faults.cc
@@ -164,8 +164,6 @@ RiscvFault::invoke(ThreadContext *tc, const StaticInstPtr &inst)
         pc_state.set(addr);
         tc->pcState(pc_state);
     } else {
-        inst->advancePC(pc_state);
-        tc->pcState(pc_state);
         invokeSE(tc, inst);
     }
 }
@@ -234,6 +232,12 @@ BreakpointFault::invokeSE(ThreadContext *tc, const StaticInstPtr &inst)
 void
 SyscallFault::invokeSE(ThreadContext *tc, const StaticInstPtr &inst)
 {
+    /* Advance the PC to next instruction so - once (simulated) syscall
+       is executed - execution continues. */
+    auto pc_state = tc->pcState().as<PCState>();
+    inst->advancePC(pc_state);
+    tc->pcState(pc_state);
+
     tc->getSystemPtr()->workload->syscall(tc);
 }
 

From 3564348eecfcf73e2105ea4ca954dd53e9fe89d9 Mon Sep 17 00:00:00 2001
From: Jan Vrany <jan.vrany@labware.com>
Date: Tue, 8 Aug 2023 15:19:30 +0100
Subject: [PATCH 3/3] arch-riscv: Report traps to GDB in SE mode

This commit add code to report illegal instruction and breakpoint traps
to GDB (if connected). This merely follows what POWER does.
---
 src/arch/riscv/faults.cc | 12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/src/arch/riscv/faults.cc b/src/arch/riscv/faults.cc
index 9bea0668e8..89bb838f88 100644
--- a/src/arch/riscv/faults.cc
+++ b/src/arch/riscv/faults.cc
@@ -205,9 +205,11 @@ UnknownInstFault::invokeSE(ThreadContext *tc, const StaticInstPtr &inst)
 void
 IllegalInstFault::invokeSE(ThreadContext *tc, const StaticInstPtr &inst)
 {
-    auto *rsi = static_cast<RiscvStaticInst *>(inst.get());
-    panic("Illegal instruction 0x%08x at pc %s: %s", rsi->machInst,
-        tc->pcState(), reason.c_str());
+    if (! tc->getSystemPtr()->trapToGdb(GDBSignal::ILL, tc->contextId()) ) {
+        auto *rsi = static_cast<RiscvStaticInst *>(inst.get());
+        panic("Illegal instruction 0x%08x at pc %s: %s", rsi->machInst,
+            tc->pcState(), reason.c_str());
+    }
 }
 
 void
@@ -226,7 +228,9 @@ IllegalFrmFault::invokeSE(ThreadContext *tc, const StaticInstPtr &inst)
 void
 BreakpointFault::invokeSE(ThreadContext *tc, const StaticInstPtr &inst)
 {
-    schedRelBreak(0);
+    if (! tc->getSystemPtr()->trapToGdb(GDBSignal::TRAP, tc->contextId()) ) {
+        schedRelBreak(0);
+    }
 }
 
 void