Hello Andreas Sandberg,

I'd like you to do a code review. Please visit

    https://gem5-review.googlesource.com/7922

to review the following change.


Change subject: arch-arm: IMPLEMENTATION DEFINED register
......................................................................

arch-arm: IMPLEMENTATION DEFINED register

A new pseudo register has been added to the Misc pool. It is the
implementation defined register. This kinds of registers are covered by
the architecture and must be treated differently than UNIMPLEMENTED
registers: their access can be trapped to EL2 (See HCR.TIDCP bit in the
arm arm).
Some previously undecoded registers in c9,c10,c11 have now this register
type.

Change-Id: Ibfc35982470b9dea0ecf39aaa6b1012a21852f53
Signed-off-by: Giacomo Travaglini <giacomo.travagl...@arm.com>
Reviewed-by: Andreas Sandberg <andreas.sandb...@arm.com>
---
M src/arch/arm/insts/pseudo.cc
M src/arch/arm/insts/pseudo.hh
M src/arch/arm/isa/formats/misc.isa
M src/arch/arm/miscregs.cc
M src/arch/arm/miscregs.hh
5 files changed, 88 insertions(+), 11 deletions(-)



diff --git a/src/arch/arm/insts/pseudo.cc b/src/arch/arm/insts/pseudo.cc
index 40e00ac..e2504d6 100644
--- a/src/arch/arm/insts/pseudo.cc
+++ b/src/arch/arm/insts/pseudo.cc
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2014,2016-2017 ARM Limited
+ * Copyright (c) 2014,2016-2018 ARM Limited
  * All rights reserved
  *
  * The license below extends only to copyright in the software and shall
@@ -181,10 +181,8 @@
                     fullMnemonic.size() ? fullMnemonic.c_str() : mnemonic);
 }

-
-
McrMrcMiscInst::McrMrcMiscInst(const char *_mnemonic, ExtMachInst _machInst,
-                             uint64_t _iss, MiscRegIndex _miscReg)
+                               uint64_t _iss, MiscRegIndex _miscReg)
     : ArmStaticInst(_mnemonic, _machInst, No_OpClass)
 {
     flags[IsNonSpeculative] = true;
@@ -217,3 +215,37 @@
 {
     return csprintf("%-10s (pipe flush)", mnemonic);
 }
+
+McrMrcImplDefined::McrMrcImplDefined(const char *_mnemonic,
+                                     ExtMachInst _machInst, uint64_t _iss,
+                                     MiscRegIndex _miscReg)
+    : McrMrcMiscInst(_mnemonic, _machInst, _iss, _miscReg)
+{}
+
+Fault
+McrMrcImplDefined::execute(ExecContext *xc, Trace::InstRecord *traceData) const
+{
+    uint32_t cpsr = xc->readMiscReg(MISCREG_CPSR);
+    uint32_t hcr = xc->readMiscReg(MISCREG_HCR);
+    uint32_t scr = xc->readMiscReg(MISCREG_SCR);
+    uint32_t hdcr = xc->readMiscReg(MISCREG_HDCR);
+    uint32_t hstr = xc->readMiscReg(MISCREG_HSTR);
+    uint32_t hcptr = xc->readMiscReg(MISCREG_HCPTR);
+
+    bool hypTrap  = mcrMrc15TrapToHyp(miscReg, hcr, cpsr, scr, hdcr, hstr,
+                                      hcptr, iss);
+    if (hypTrap) {
+        return std::make_shared<HypervisorTrap>(machInst, iss,
+                                                EC_TRAPPED_CP15_MCR_MRC);
+    } else {
+        return std::make_shared<UndefinedInstruction>(machInst, false,
+                                                      mnemonic);
+    }
+}
+
+std::string
+McrMrcImplDefined::generateDisassembly(Addr pc,
+                                       const SymbolTable *symtab) const
+{
+    return csprintf("%-10s (implementation defined)", mnemonic);
+}
diff --git a/src/arch/arm/insts/pseudo.hh b/src/arch/arm/insts/pseudo.hh
index fe28789..5fb7499 100644
--- a/src/arch/arm/insts/pseudo.hh
+++ b/src/arch/arm/insts/pseudo.hh
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2014,2016 ARM Limited
+ * Copyright (c) 2014,2016,2018 ARM Limited
  * All rights reserved
  *
  * The license below extends only to copyright in the software and shall
@@ -123,7 +123,7 @@
  */
 class McrMrcMiscInst : public ArmStaticInst
 {
-  private:
+  protected:
     uint64_t iss;
     MiscRegIndex miscReg;

@@ -138,4 +138,21 @@

 };

+/**
+ * This class is also used for IMPLEMENTATION DEFINED registers, whose mcr/mrc
+ * behaviour is trappable even for unimplemented registers.
+ */
+class McrMrcImplDefined : public McrMrcMiscInst
+{
+  public:
+    McrMrcImplDefined(const char *_mnemonic, ExtMachInst _machInst,
+                      uint64_t _iss, MiscRegIndex _miscReg);
+
+    Fault execute(ExecContext *xc, Trace::InstRecord *traceData) const;
+
+    std::string
+    generateDisassembly(Addr pc, const SymbolTable *symtab) const;
+
+};
+
 #endif
diff --git a/src/arch/arm/isa/formats/misc.isa b/src/arch/arm/isa/formats/misc.isa
index 4f834b8..a9acc21 100644
--- a/src/arch/arm/isa/formats/misc.isa
+++ b/src/arch/arm/isa/formats/misc.isa
@@ -1,6 +1,6 @@
 // -*- mode:c++ -*-

-// Copyright (c) 2010-2013,2016-2017 ARM Limited
+// Copyright (c) 2010-2013,2016-2018 ARM Limited
 // All rights reserved
 //
 // The license below extends only to copyright in the software and shall
@@ -219,6 +219,11 @@
                     machInst,
csprintf("miscreg crn:%d opc1:%d crm:%d opc2:%d %s unknown",
                     crn, opc1, crm, opc2, isRead ? "read" : "write"));
+          case MISCREG_IMPDEF_UNIMPL:
+            return new McrMrcImplDefined(
+                isRead ? "mrc implementation defined" :
+                         "mcr implementation defined",
+                machInst, iss, MISCREG_IMPDEF_UNIMPL);
           case MISCREG_CP15ISB:
             return new Isb(machInst, iss);
           case MISCREG_CP15DSB:
diff --git a/src/arch/arm/miscregs.cc b/src/arch/arm/miscregs.cc
index 445ba18..7838ffe 100644
--- a/src/arch/arm/miscregs.cc
+++ b/src/arch/arm/miscregs.cc
@@ -502,6 +502,19 @@
         }
         break;
       case 9:
+        // Every cop register with CRn = 9 and CRm in
+        // {0-2}, {5-8} is implementation defined regardless
+        // of opc1 and opc2.
+        switch (crm) {
+          case 0:
+          case 1:
+          case 2:
+          case 5:
+          case 6:
+          case 7:
+          case 8:
+            return MISCREG_IMPDEF_UNIMPL;
+        }
         if (opc1 == 0) {
             switch (crm) {
               case 12:
@@ -565,7 +578,9 @@
       case 10:
         if (opc1 == 0) {
// crm 0, 1, 4, and 8, with op2 0 - 7, reserved for TLB lockdown
-            if (crm == 2) { // TEX Remap Registers
+            if (crm < 2) {
+                return MISCREG_IMPDEF_UNIMPL;
+            } else if (crm == 2) { // TEX Remap Registers
                 if (opc2 == 0) {
                     // Selector is TTBCR.EAE
                     return MISCREG_PRRR_MAIR0;
@@ -609,6 +624,8 @@
               case 8:
               case 15:
                 // Reserved for DMA operations for TCM access
+                return MISCREG_IMPDEF_UNIMPL;
+              default:
                 break;
             }
         }
@@ -689,7 +706,7 @@
         break;
       case 15:
         // Implementation defined
-        return MISCREG_CP15_UNIMPL;
+        return MISCREG_IMPDEF_UNIMPL;
     }
     // Unrecognized register
     return MISCREG_CP15_UNIMPL;
diff --git a/src/arch/arm/miscregs.hh b/src/arch/arm/miscregs.hh
index 12f6f3f..a4a95a3 100644
--- a/src/arch/arm/miscregs.hh
+++ b/src/arch/arm/miscregs.hh
@@ -677,7 +677,7 @@

         // NUM_PHYS_MISCREGS specifies the number of actual physical
         // registers, not considering the following pseudo-registers
-        // (dummy registers), like UNKNOWN, CP15_UNIMPL...
+ // (dummy registers), like UNKNOWN, CP15_UNIMPL, MISCREG_IMPDEF_UNIMPL.
         // Checkpointing should use this physical index when
         // saving/restoring register values.
         NUM_PHYS_MISCREGS = 606,        // 606
@@ -690,6 +690,11 @@
         MISCREG_A64_UNIMPL,
         MISCREG_UNKNOWN,

+        // Implementation defined register: this represent
+        // a pool of unimplemented registers whose access can throw
+        // either UNDEFINED or hypervisor trap exception.
+        MISCREG_IMPDEF_UNIMPL,
+
         // Total number of Misc Registers: Physical + Dummy
         NUM_MISCREGS
     };
@@ -1380,7 +1385,8 @@
         "cp14_unimpl",
         "cp15_unimpl",
         "a64_unimpl",
-        "unknown"
+        "unknown",
+        "impl_defined"
     };

static_assert(sizeof(miscRegName) / sizeof(*miscRegName) == NUM_MISCREGS,

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

Gerrit-Project: public/gem5
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: Ibfc35982470b9dea0ecf39aaa6b1012a21852f53
Gerrit-Change-Number: 7922
Gerrit-PatchSet: 1
Gerrit-Owner: Giacomo Travaglini <giacomo.travagl...@arm.com>
Gerrit-Reviewer: Andreas Sandberg <andreas.sandb...@arm.com>
_______________________________________________
gem5-dev mailing list
gem5-dev@gem5.org
http://m5sim.org/mailman/listinfo/gem5-dev

Reply via email to