From 4f6fdbf8bfc38c7d8f3a6281d676d73b9b8a097f Mon Sep 17 00:00:00 2001
From: Yu-Cheng Chang <rogerycchang@google.com>
Date: Sun, 26 May 2024 11:18:42 +0800
Subject: [PATCH] arch-riscv: Fix c.jalr and c.jr instruction (#1163)

The bit 0 of register should be 0 for jump address. Wrong handling the
jump address may cause infinite run or segment fault.

gem5 issue: https://github.com/gem5/gem5/issues/981
---
 src/arch/riscv/isa/decoder.isa            |  8 ++---
 src/arch/riscv/isa/formats/compressed.isa | 41 +++++++++++++++++++++++
 src/arch/riscv/isa/formats/standard.isa   |  8 ++---
 3 files changed, 47 insertions(+), 10 deletions(-)

diff --git a/src/arch/riscv/isa/decoder.isa b/src/arch/riscv/isa/decoder.isa
index 52dc11209b..0eb10d89b2 100644
--- a/src/arch/riscv/isa/decoder.isa
+++ b/src/arch/riscv/isa/decoder.isa
@@ -448,12 +448,12 @@ decode QUADRANT default Unknown::unknown() {
         }
         0x4: decode CFUNCT1 {
             0x0: decode RC2 {
-                0x0: Jump::c_jr({{
+                0x0: CJump::c_jr({{
                     if (RC1 == 0) {
                         return std::make_shared<IllegalInstFault>(
                                 "source reg x0", machInst);
                     }
-                    NPC = rvZext(Rc1);
+                    NPC = rvZext(Rc1 & (~0x1));
                 }}, IsIndirectControl, IsUncondControl);
                 default: CROp::c_mv({{
                     // RC1 == 0 is HINT
@@ -468,9 +468,9 @@ decode QUADRANT default Unknown::unknown() {
                         return std::make_shared<BreakpointFault>(
                             xc->pcState());
                     }}, IsSerializeAfter, IsNonSpeculative, No_OpClass);
-                    default: Jump::c_jalr({{
+                    default: CJump::c_jalr({{
                         ra = rvSext(NPC);
-                        NPC = rvZext(Rc1);
+                        NPC = rvZext(Rc1 & (~0x1));
                     }}, IsIndirectControl, IsUncondControl, IsCall);
                 }
                 default: CompressedROp::c_add({{
diff --git a/src/arch/riscv/isa/formats/compressed.isa b/src/arch/riscv/isa/formats/compressed.isa
index 7a9fd634c8..607415b424 100644
--- a/src/arch/riscv/isa/formats/compressed.isa
+++ b/src/arch/riscv/isa/formats/compressed.isa
@@ -157,6 +157,36 @@ def template CBasicExecute {{
     }
 }};
 
+def template CJumpExecute {{
+    Fault
+    %(class_name)s::execute(
+        ExecContext *xc, trace::InstRecord *traceData) const
+    {
+        %(op_decl)s;
+        %(op_rd)s;
+        %(code)s;
+        %(op_wb)s;
+        return NoFault;
+    }
+
+    std::unique_ptr<PCStateBase>
+    %(class_name)s::branchTarget(ThreadContext *tc) const
+    {
+        PCStateBase *pc_ptr = tc->pcState().clone();
+        pc_ptr->as<PCState>().set(tc->getReg(srcRegIdx(0)) & ~0x1);
+        return std::unique_ptr<PCStateBase>{pc_ptr};
+    }
+
+    std::string
+    %(class_name)s::generateDisassembly(
+            Addr pc, const loader::SymbolTable *symtab) const
+    {
+        std::stringstream ss;
+        ss << mnemonic << ' ' << registerName(srcRegIdx(0));
+        return ss.str();
+    }
+}};
+
 def format CompressedROp(code, *opt_flags) {{
     regs = ['destRegIdx(0)','srcRegIdx(1)']
     iop = InstObjParams(name, Name, 'RegOp',
@@ -166,3 +196,14 @@ def format CompressedROp(code, *opt_flags) {{
     decode_block = BasicDecode.subst(iop)
     exec_output = CBasicExecute.subst(iop)
 }};
+
+def format CJump(code, *opt_flags) {{
+    regs = ['srcRegIdx(0)']
+    iop = InstObjParams(name, Name, 'ImmOp<int64_t>',
+        {'code': code, 'imm_code': 'imm = 0;',
+         'regs': ','.join(regs)}, opt_flags)
+    header_output = JumpDeclare.subst(iop)
+    decoder_output = JumpConstructor.subst(iop)
+    decode_block = BasicDecode.subst(iop)
+    exec_output = CJumpExecute.subst(iop)
+}};
diff --git a/src/arch/riscv/isa/formats/standard.isa b/src/arch/riscv/isa/formats/standard.isa
index d2fe233c55..b201a90fef 100644
--- a/src/arch/riscv/isa/formats/standard.isa
+++ b/src/arch/riscv/isa/formats/standard.isa
@@ -315,12 +315,8 @@ def template JumpExecute {{
             Addr pc, const loader::SymbolTable *symtab) const
     {
         std::stringstream ss;
-        ss << mnemonic << ' ';
-        if (QUADRANT == 0x3)
-            ss << registerName(destRegIdx(0)) << ", "
-               << imm << "(" << registerName(srcRegIdx(0)) << ")";
-        else
-            ss << registerName(srcRegIdx(0));
+        ss << mnemonic << ' ' << registerName(destRegIdx(0)) << ", "
+           << imm << "(" << registerName(srcRegIdx(0)) << ")";
         return ss.str();
     }
 }};