Jui-min Lee has submitted this change. ( https://gem5-review.googlesource.com/c/public/gem5/+/52363 )

Change subject: arch-riscv: Add NMI support
......................................................................

arch-riscv: Add NMI support

NMI is platform dependent according to riscv spec, so we intentionally
propose a very minimal design that doesn't give user any new accessible
register to interact with the NMI mechanism.

In current design, the NMI reset vector is fixed to 0x0 and always set
mcause to zero. mret from NMI handler is still possible, but it's up to
the user to detect whether a M-mode trap handler is interrupted and to
recover from it (if at all possible).

1. Add new fault type to represent NMI fault
2. Add non-standard registers to save the status of NMI
   a. nmivec[63:2] = NMI reset vector address
   b. nmie[0:0] = is NMI enabled = not in NMI handler
   c. nmip[0:0] = is NMI pending
3. Add new function in RiscvInterrupts to raise/clear NMI

Bug: 200169094
Test: None
Change-Id: Ia81e1c9589bc02f0690d094fff5f13412846acbe
Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/52363
Reviewed-by: Yu-hsin Wang <[email protected]>
Reviewed-by: Jason Lowe-Power <[email protected]>
Maintainer: Jason Lowe-Power <[email protected]>
Tested-by: kokoro <[email protected]>
---
M src/arch/riscv/isa.cc
M src/arch/riscv/regs/misc.hh
M src/arch/riscv/isa/decoder.isa
M src/arch/riscv/interrupts.hh
M src/arch/riscv/faults.cc
M src/arch/riscv/faults.hh
6 files changed, 111 insertions(+), 12 deletions(-)

Approvals:
  Jason Lowe-Power: Looks good to me, approved; Looks good to me, approved
  Yu-hsin Wang: Looks good to me, but someone else must approve
  kokoro: Regressions pass




diff --git a/src/arch/riscv/faults.cc b/src/arch/riscv/faults.cc
index 36e2ce9..01f6827 100644
--- a/src/arch/riscv/faults.cc
+++ b/src/arch/riscv/faults.cc
@@ -67,8 +67,17 @@
         PrivilegeMode prv = PRV_M;
         STATUS status = tc->readMiscReg(MISCREG_STATUS);

+ // According to riscv-privileged-v1.11, if a NMI occurs at the middle + // of a M-mode trap handler, the state (epc/cause) will be overwritten + // and is not necessary recoverable. There's nothing we can do here so
+        // we'll just warn our user that the CPU state might be broken.
+        warn_if(isNonMaskableInterrupt() && pp == PRV_M && status.mie == 0,
+                "NMI overwriting M-mode trap handler state");
+
         // Set fault handler privilege mode
-        if (isInterrupt()) {
+        if (isNonMaskableInterrupt()) {
+            prv = PRV_M;
+        } else if (isInterrupt()) {
             if (pp != PRV_M &&
                 bits(tc->readMiscReg(MISCREG_MIDELEG), _code) != 0) {
                 prv = PRV_S;
@@ -113,7 +122,7 @@
           case PRV_M:
             cause = MISCREG_MCAUSE;
             epc = MISCREG_MEPC;
-            tvec = MISCREG_MTVEC;
+ tvec = isNonMaskableInterrupt() ? MISCREG_NMIVEC : MISCREG_MTVEC;
             tval = MISCREG_MTVAL;

             status.mpp = pp;
@@ -136,6 +145,12 @@
         tc->setMiscReg(tval, trap_value());
         tc->setMiscReg(MISCREG_PRV, prv);
         tc->setMiscReg(MISCREG_STATUS, status);
+        // Temporarily mask NMI while we're in NMI handler. Otherweise, the
+        // checkNonMaskableInterrupt will always return true and we'll be
+        // stucked in an infinite loop.
+        if (isNonMaskableInterrupt()) {
+            tc->setMiscReg(MISCREG_NMIE, 0);
+        }

         // Set PC to fault handler address
         Addr addr = mbits(tc->readMiscReg(tvec), 63, 2);
diff --git a/src/arch/riscv/faults.hh b/src/arch/riscv/faults.hh
index a8df3f5..1ea8f70 100644
--- a/src/arch/riscv/faults.hh
+++ b/src/arch/riscv/faults.hh
@@ -96,19 +96,30 @@
     NumInterruptTypes
 };

+enum class FaultType
+{
+    INTERRUPT,
+    NON_MASKABLE_INTERRUPT,
+    OTHERS,
+};
+
 class RiscvFault : public FaultBase
 {
   protected:
     const FaultName _name;
-    const bool _interrupt;
+    const FaultType _fault_type;
     ExceptionCode _code;

-    RiscvFault(FaultName n, bool i, ExceptionCode c)
-        : _name(n), _interrupt(i), _code(c)
+    RiscvFault(FaultName n, FaultType ft, ExceptionCode c)
+        : _name(n), _fault_type(ft), _code(c)
     {}

     FaultName name() const override { return _name; }
-    bool isInterrupt() const { return _interrupt; }
+ bool isInterrupt() const { return _fault_type == FaultType::INTERRUPT; }
+    bool isNonMaskableInterrupt() const
+    {
+        return _fault_type == FaultType::NON_MASKABLE_INTERRUPT;
+    }
     ExceptionCode exception() const { return _code; }
     virtual RegVal trap_value() const { return 0; }

@@ -132,10 +143,22 @@
 class InterruptFault : public RiscvFault
 {
   public:
-    InterruptFault(ExceptionCode c) : RiscvFault("interrupt", true, c) {}
+    InterruptFault(ExceptionCode c)
+        : RiscvFault("interrupt", FaultType::INTERRUPT, c)
+    {}
InterruptFault(int c) : InterruptFault(static_cast<ExceptionCode>(c)) {}
 };

+class NonMaskableInterruptFault : public RiscvFault
+{
+  public:
+    NonMaskableInterruptFault()
+        : RiscvFault("non_maskable_interrupt",
+                     FaultType::NON_MASKABLE_INTERRUPT,
+                     static_cast<ExceptionCode>(0))
+    {}
+};
+
 class InstFault : public RiscvFault
 {
   protected:
@@ -143,7 +166,7 @@

   public:
     InstFault(FaultName n, const ExtMachInst inst)
-        : RiscvFault(n, false, INST_ILLEGAL), _inst(inst)
+        : RiscvFault(n, FaultType::OTHERS, INST_ILLEGAL), _inst(inst)
     {}

     RegVal trap_value() const override { return _inst; }
@@ -208,7 +231,7 @@

   public:
     AddressFault(const Addr addr, ExceptionCode code)
-        : RiscvFault("Address", false, code), _addr(addr)
+        : RiscvFault("Address", FaultType::OTHERS, code), _addr(addr)
     {}

     RegVal trap_value() const override { return _addr; }
@@ -221,7 +244,7 @@

   public:
     BreakpointFault(const PCState &pc)
-        : RiscvFault("Breakpoint", false, BREAKPOINT), pcState(pc)
+ : RiscvFault("Breakpoint", FaultType::OTHERS, BREAKPOINT), pcState(pc)
     {}

     RegVal trap_value() const override { return pcState.pc(); }
@@ -232,7 +255,7 @@
 {
   public:
     SyscallFault(PrivilegeMode prv)
-        : RiscvFault("System call", false, ECALL_USER)
+        : RiscvFault("System call", FaultType::OTHERS, ECALL_USER)
     {
         switch (prv) {
           case PRV_U:
diff --git a/src/arch/riscv/interrupts.hh b/src/arch/riscv/interrupts.hh
index dcd88b1..88b7b2e 100644
--- a/src/arch/riscv/interrupts.hh
+++ b/src/arch/riscv/interrupts.hh
@@ -105,16 +105,24 @@
         return std::bitset<NumInterruptTypes>(mask);
     }

+    bool
+    checkNonMaskableInterrupt() const
+    {
+ return tc->readMiscReg(MISCREG_NMIP) & tc->readMiscReg(MISCREG_NMIE);
+    }
+
     bool checkInterrupt(int num) const { return ip[num] && ie[num]; }
     bool checkInterrupts() const
     {
-        return (ip & ie & globalMask()).any();
+ return checkNonMaskableInterrupt() || (ip & ie & globalMask()).any();
     }

     Fault
     getInterrupt()
     {
         assert(checkInterrupts());
+        if (checkNonMaskableInterrupt())
+            return std::make_shared<NonMaskableInterruptFault>();
         std::bitset<NumInterruptTypes> mask = globalMask();
         const std::vector<int> interrupt_order {
             INT_EXT_MACHINE, INT_TIMER_MACHINE, INT_SOFTWARE_MACHINE,
@@ -143,11 +151,15 @@
         ip[int_num] = false;
     }

+    void postNMI() { tc->setMiscReg(MISCREG_NMIP, 1); }
+    void clearNMI() { tc->setMiscReg(MISCREG_NMIP, 0); }
+
     void
     clearAll()
     {
         DPRINTF(Interrupt, "All interrupts cleared\n");
         ip = 0;
+        clearNMI();
     }

     uint64_t readIP() const { return (uint64_t)ip.to_ulong(); }
diff --git a/src/arch/riscv/isa.cc b/src/arch/riscv/isa.cc
index ac2725f..e26b6fa 100644
--- a/src/arch/riscv/isa.cc
+++ b/src/arch/riscv/isa.cc
@@ -185,6 +185,10 @@
     [MISCREG_UTVAL]         = "UTVAL",
     [MISCREG_FFLAGS]        = "FFLAGS",
     [MISCREG_FRM]           = "FRM",
+
+    [MISCREG_NMIVEC]        = "NMIVEC",
+    [MISCREG_NMIE]          = "NMIE",
+    [MISCREG_NMIP]          = "NMIP",
 }};

 ISA::ISA(const Params &p) : BaseISA(p)
@@ -232,6 +236,8 @@
     // don't set it to zero; software may try to determine the supported
     // triggers, starting at zero. simply set a different value here.
     miscRegFile[MISCREG_TSELECT] = 1;
+    // NMI is always enabled.
+    miscRegFile[MISCREG_NMIE] = 1;
 }

 bool
diff --git a/src/arch/riscv/isa/decoder.isa b/src/arch/riscv/isa/decoder.isa
index f30ecdf..3cbba5a 100644
--- a/src/arch/riscv/isa/decoder.isa
+++ b/src/arch/riscv/isa/decoder.isa
@@ -1420,6 +1420,7 @@
                         } else {
STATUS status = xc->readMiscReg(MISCREG_STATUS);
                             xc->setMiscReg(MISCREG_PRV, status.mpp);
+                            xc->setMiscReg(MISCREG_NMIE, 1);
                             status.mie = status.mpie;
                             status.mpie = 1;
                             status.mpp = PRV_U;
diff --git a/src/arch/riscv/regs/misc.hh b/src/arch/riscv/regs/misc.hh
index 5bfcbe1..4eaedfe 100644
--- a/src/arch/riscv/regs/misc.hh
+++ b/src/arch/riscv/regs/misc.hh
@@ -187,6 +187,16 @@
     MISCREG_FFLAGS,
     MISCREG_FRM,

+    // These registers are not in the standard, hence does not exist in the
+ // CSRData map. These are mainly used to provide a minimal implementation
+    // for non-maskable-interrupt in our simple cpu.
+    // non-maskable-interrupt-vector-base-address: NMI version of xTVEC
+    MISCREG_NMIVEC,
+    // non-maskable-interrupt-enable: NMI version of xIE
+    MISCREG_NMIE,
+    // non-maskable-interrupt-pending: NMI version of xIP
+    MISCREG_NMIP,
+
     NUM_MISCREGS
 };


--
To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/52363
To unsubscribe, or for help writing mail filters, visit https://gem5-review.googlesource.com/settings

Gerrit-Project: public/gem5
Gerrit-Branch: develop
Gerrit-Change-Id: Ia81e1c9589bc02f0690d094fff5f13412846acbe
Gerrit-Change-Number: 52363
Gerrit-PatchSet: 4
Gerrit-Owner: Jui-min Lee <[email protected]>
Gerrit-Reviewer: Ayaz Akram <[email protected]>
Gerrit-Reviewer: Gabe Black <[email protected]>
Gerrit-Reviewer: Jason Lowe-Power <[email protected]>
Gerrit-Reviewer: Jui-min Lee <[email protected]>
Gerrit-Reviewer: Yu-hsin Wang <[email protected]>
Gerrit-Reviewer: kokoro <[email protected]>
Gerrit-CC: Bobby R. Bruce <[email protected]>
Gerrit-CC: Earl Ou <[email protected]>
Gerrit-MessageType: merged
_______________________________________________
gem5-dev mailing list -- [email protected]
To unsubscribe send an email to [email protected]
%(web_page_url)slistinfo%(cgiext)s/%(_internal_name)s

Reply via email to