Giacomo Travaglini has uploaded this change for review. ( https://gem5-review.googlesource.com/c/public/gem5/+/35240 )

Change subject: arch-arm: Rewrite the TLB flushing interface
......................................................................

arch-arm: Rewrite the TLB flushing interface

We are now using an overloaded flush method which has
different TLBI ops as arguments.

This is simplifying the interface and it is allowing us to
encode some state in the TLBIOp which will then be passed
to the TLB. This is a step towards making the TLB a stateless
cache of translations

Change-Id: Ic4fbae72dc3cfe756047148b1cf5f144298c8b08
Signed-off-by: Giacomo Travaglini <[email protected]>
---
M src/arch/arm/tlb.cc
M src/arch/arm/tlb.hh
M src/arch/arm/tlbi_op.cc
M src/arch/arm/tlbi_op.hh
4 files changed, 104 insertions(+), 110 deletions(-)



diff --git a/src/arch/arm/tlb.cc b/src/arch/arm/tlb.cc
index 8cb7638..3ab0e6e 100644
--- a/src/arch/arm/tlb.cc
+++ b/src/arch/arm/tlb.cc
@@ -52,6 +52,7 @@
 #include "arch/arm/stage2_mmu.hh"
 #include "arch/arm/system.hh"
 #include "arch/arm/table_walker.hh"
+#include "arch/arm/tlbi_op.hh"
 #include "arch/arm/utility.hh"
 #include "base/inifile.hh"
 #include "base/str.hh"
@@ -268,19 +269,18 @@
 }

 void
-TLB::flushAllSecurity(bool secure_lookup, ExceptionLevel target_el,
-                      bool ignore_el, bool in_host)
+TLB::flush(const TLBIALL& tlbi_op)
 {
     DPRINTF(TLB, "Flushing all TLB entries (%s lookup)\n",
-            (secure_lookup ? "secure" : "non-secure"));
+            (tlbi_op.secureLookup ? "secure" : "non-secure"));
     int x = 0;
     TlbEntry *te;
     while (x < size) {
         te = &table[x];
-        const bool el_match = ignore_el ?
-            true : te->checkELMatch(target_el, in_host);
-        if (te->valid && secure_lookup == !te->nstid &&
-            (te->vmid == vmid || secure_lookup) && el_match) {
+        const bool el_match = te->checkELMatch(
+            tlbi_op.targetEL, tlbi_op.inHost);
+        if (te->valid && tlbi_op.secureLookup == !te->nstid &&
+            (te->vmid == vmid || tlbi_op.secureLookup) && el_match) {

             DPRINTF(TLB, " -  %s\n", te->print());
             te->valid = false;
@@ -294,14 +294,14 @@
// If there's a second stage TLB (and we're not it) then flush it as well
     // if we're currently in hyp mode
     if (!isStage2 && isHyp) {
-        stage2Tlb->flushAllSecurity(secure_lookup, EL1, true, false);
+        stage2Tlb->flush(tlbi_op.makeStage2());
     }
 }

 void
-TLB::flushAllNs(ExceptionLevel target_el, bool ignore_el)
+TLB::flush(const TLBIALLN &tlbi_op)
 {
-    bool hyp = target_el == EL2;
+    bool hyp = tlbi_op.targetEL == EL2;

     DPRINTF(TLB, "Flushing all NS TLB entries (%s lookup)\n",
             (hyp ? "hyp" : "non-hyp"));
@@ -309,8 +309,7 @@
     TlbEntry *te;
     while (x < size) {
         te = &table[x];
-        const bool el_match = ignore_el ?
-            true : te->checkELMatch(target_el, false);
+        const bool el_match = te->checkELMatch(tlbi_op.targetEL, false);

         if (te->valid && te->nstid && te->isHyp == hyp && el_match) {

@@ -325,36 +324,36 @@

// If there's a second stage TLB (and we're not it) then flush it as well
     if (!isStage2 && !hyp) {
-        stage2Tlb->flushAllNs(EL1, true);
+        stage2Tlb->flush(tlbi_op.makeStage2());
     }
 }

 void
-TLB::flushMvaAsid(Addr mva, uint64_t asn, bool secure_lookup,
-                  ExceptionLevel target_el, bool in_host)
+TLB::flush(const TLBIMVA &tlbi_op)
 {
     DPRINTF(TLB, "Flushing TLB entries with mva: %#x, asid: %#x "
-            "(%s lookup)\n", mva, asn, (secure_lookup ?
-            "secure" : "non-secure"));
-    _flushMva(mva, asn, secure_lookup, false, target_el, in_host);
+            "(%s lookup)\n", tlbi_op.addr, tlbi_op.asid,
+            (tlbi_op.secureLookup ? "secure" : "non-secure"));
+    _flushMva(tlbi_op.addr, tlbi_op.asid, tlbi_op.secureLookup, false,
+        tlbi_op.targetEL, tlbi_op.inHost);
     stats.flushTlbMvaAsid++;
 }

 void
-TLB::flushAsid(uint64_t asn, bool secure_lookup, ExceptionLevel target_el,
-               bool in_host)
+TLB::flush(const TLBIASID &tlbi_op)
 {
-    DPRINTF(TLB, "Flushing TLB entries with asid: %#x (%s lookup)\n", asn,
-            (secure_lookup ? "secure" : "non-secure"));
+    DPRINTF(TLB, "Flushing TLB entries with asid: %#x (%s lookup)\n",
+ tlbi_op.asid, (tlbi_op.secureLookup ? "secure" : "non-secure"));

     int x = 0 ;
     TlbEntry *te;

     while (x < size) {
         te = &table[x];
-        if (te->valid && te->asid == asn && secure_lookup == !te->nstid &&
-            (te->vmid == vmid || secure_lookup) &&
-            te->checkELMatch(target_el, in_host)) {
+        if (te->valid && te->asid == tlbi_op.asid &&
+            tlbi_op.secureLookup == !te->nstid &&
+            (te->vmid == vmid || tlbi_op.secureLookup) &&
+            te->checkELMatch(tlbi_op.targetEL, tlbi_op.inHost)) {

             te->valid = false;
             DPRINTF(TLB, " -  %s\n", te->print());
@@ -366,12 +365,13 @@
 }

 void
-TLB::flushMva(Addr mva, bool secure_lookup, ExceptionLevel target_el,
-              bool in_host) {
+TLB::flush(const TLBIMVAA &tlbi_op) {

-    DPRINTF(TLB, "Flushing TLB entries with mva: %#x (%s lookup)\n", mva,
-            (secure_lookup ? "secure" : "non-secure"));
-    _flushMva(mva, 0xbeef, secure_lookup, true, target_el, in_host);
+    DPRINTF(TLB, "Flushing TLB entries with mva: %#x (%s lookup)\n",
+            tlbi_op.addr,
+            (tlbi_op.secureLookup ? "secure" : "non-secure"));
+    _flushMva(tlbi_op.addr, 0xbeef, tlbi_op.secureLookup, true,
+        tlbi_op.targetEL, tlbi_op.inHost);
     stats.flushTlbMva++;
 }

@@ -399,10 +399,11 @@
 }

 void
-TLB::flushIpaVmid(Addr ipa, bool secure_lookup, ExceptionLevel target_el)
+TLB::flush(const TLBIIPA &tlbi_op)
 {
     assert(!isStage2);
- stage2Tlb->_flushMva(ipa, 0xbeef, secure_lookup, true, target_el, false);
+    stage2Tlb->_flushMva(tlbi_op.addr, 0xbeef, tlbi_op.secureLookup,
+        true, tlbi_op.targetEL, false);
 }

 void
diff --git a/src/arch/arm/tlb.hh b/src/arch/arm/tlb.hh
index ed621ad..753b8b5 100644
--- a/src/arch/arm/tlb.hh
+++ b/src/arch/arm/tlb.hh
@@ -61,6 +61,13 @@
 class Stage2MMU;
 class TLB;

+class TLBIALL;
+class TLBIALLN;
+class TLBIMVA;
+class TLBIASID;
+class TLBIMVAA;
+class TLBIIPA;
+
 class TlbTestInterface
 {
   public:
@@ -253,46 +260,31 @@


     /** Reset the entire TLB
-     * @param secure_lookup if the operation affects the secure world
      */
-    void flushAllSecurity(bool secure_lookup, ExceptionLevel target_el,
-                          bool ignore_el = false, bool in_host = false);
+    void flush(const TLBIALL& tlbi_op);

/** Remove all entries in the non secure world, depending on whether they
      *  were allocated in hyp mode or not
      */
-    void flushAllNs(ExceptionLevel target_el, bool ignore_el = false);
-
+    void flush(const TLBIALLN &tlbi_op);

     /** Remove any entries that match both a va and asn
-     * @param mva virtual address to flush
-     * @param asn contextid/asn to flush on match
-     * @param secure_lookup if the operation affects the secure world
      */
-    void flushMvaAsid(Addr mva, uint64_t asn, bool secure_lookup,
-                      ExceptionLevel target_el, bool in_host = false);
+    void flush(const TLBIMVA &tlbi_op);

     /** Remove any entries that match the asn
-     * @param asn contextid/asn to flush on match
-     * @param secure_lookup if the operation affects the secure world
      */
-    void flushAsid(uint64_t asn, bool secure_lookup,
-                   ExceptionLevel target_el, bool in_host = false);
+    void flush(const TLBIASID &tlbi_op);

     /** Remove all entries that match the va regardless of asn
-     * @param mva address to flush from cache
-     * @param secure_lookup if the operation affects the secure world
      */
-    void flushMva(Addr mva, bool secure_lookup, ExceptionLevel target_el,
-                  bool in_host = false);
+    void flush(const TLBIMVAA &tlbi_op);

     /**
      * Invalidate all entries in the stage 2 TLB that match the given ipa
      * and the current VMID
-     * @param ipa the address to invalidate
-     * @param secure_lookup if the operation affects the secure world
      */
- void flushIpaVmid(Addr ipa, bool secure_lookup, ExceptionLevel target_el);
+    void flush(const TLBIIPA &tlbi_op);

     Fault trickBoxCheck(const RequestPtr &req, Mode mode,
                         TlbEntry::DomainType domain);
diff --git a/src/arch/arm/tlbi_op.cc b/src/arch/arm/tlbi_op.cc
index f3b9bd1..f1ba30b 100644
--- a/src/arch/arm/tlbi_op.cc
+++ b/src/arch/arm/tlbi_op.cc
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2018-2019 ARM Limited
+ * Copyright (c) 2018-2020 ARM Limited
  * All rights reserved
  *
  * The license below extends only to copyright in the software and shall
@@ -46,68 +46,66 @@
 TLBIALL::operator()(ThreadContext* tc)
 {
     HCR hcr = tc->readMiscReg(MISCREG_HCR_EL2);
-    bool in_host = (hcr.tge == 1 && hcr.e2h == 1);
-    getITBPtr(tc)->flushAllSecurity(secureLookup, targetEL, in_host);
-    getDTBPtr(tc)->flushAllSecurity(secureLookup, targetEL, in_host);
+    inHost = (hcr.tge == 1 && hcr.e2h == 1);
+    getITBPtr(tc)->flush(*this);
+    getDTBPtr(tc)->flush(*this);

     // If CheckerCPU is connected, need to notify it of a flush
     CheckerCPU *checker = tc->getCheckerCpuPtr();
     if (checker) {
-        getITBPtr(checker)->flushAllSecurity(secureLookup,
-                                               targetEL, in_host);
-        getDTBPtr(checker)->flushAllSecurity(secureLookup,
-                                               targetEL, in_host);
+        getITBPtr(checker)->flush(*this);
+        getDTBPtr(checker)->flush(*this);
     }
 }

 void
 ITLBIALL::operator()(ThreadContext* tc)
 {
-    getITBPtr(tc)->flushAllSecurity(secureLookup, targetEL);
+    getITBPtr(tc)->flush(*this);
 }

 void
 DTLBIALL::operator()(ThreadContext* tc)
 {
-    getDTBPtr(tc)->flushAllSecurity(secureLookup, targetEL);
+    getDTBPtr(tc)->flush(*this);
 }

 void
 TLBIASID::operator()(ThreadContext* tc)
 {
     HCR hcr = tc->readMiscReg(MISCREG_HCR_EL2);
-    bool in_host = (hcr.tge == 1 && hcr.e2h == 1);
-    getITBPtr(tc)->flushAsid(asid, secureLookup, targetEL, in_host);
-    getDTBPtr(tc)->flushAsid(asid, secureLookup, targetEL, in_host);
+    inHost = (hcr.tge == 1 && hcr.e2h == 1);
+    getITBPtr(tc)->flush(*this);
+    getDTBPtr(tc)->flush(*this);
     CheckerCPU *checker = tc->getCheckerCpuPtr();
     if (checker) {
- getITBPtr(checker)->flushAsid(asid, secureLookup, targetEL, in_host); - getDTBPtr(checker)->flushAsid(asid, secureLookup, targetEL, in_host);
+        getITBPtr(checker)->flush(*this);
+        getDTBPtr(checker)->flush(*this);
     }
 }

 void
 ITLBIASID::operator()(ThreadContext* tc)
 {
-    getITBPtr(tc)->flushAsid(asid, secureLookup, targetEL);
+    getITBPtr(tc)->flush(*this);
 }

 void
 DTLBIASID::operator()(ThreadContext* tc)
 {
-    getDTBPtr(tc)->flushAsid(asid, secureLookup, targetEL);
+    getDTBPtr(tc)->flush(*this);
 }

 void
 TLBIALLN::operator()(ThreadContext* tc)
 {
-    getITBPtr(tc)->flushAllNs(targetEL);
-    getDTBPtr(tc)->flushAllNs(targetEL);
+    getITBPtr(tc)->flush(*this);
+    getDTBPtr(tc)->flush(*this);

     CheckerCPU *checker = tc->getCheckerCpuPtr();
     if (checker) {
-        getITBPtr(checker)->flushAllNs(targetEL);
-        getDTBPtr(checker)->flushAllNs(targetEL);
+        getITBPtr(checker)->flush(*this);
+        getDTBPtr(checker)->flush(*this);
     }
 }

@@ -115,14 +113,14 @@
 TLBIMVAA::operator()(ThreadContext* tc)
 {
     HCR hcr = tc->readMiscReg(MISCREG_HCR_EL2);
-    bool in_host = (hcr.tge == 1 && hcr.e2h == 1);
-    getITBPtr(tc)->flushMva(addr, secureLookup, targetEL, in_host);
-    getDTBPtr(tc)->flushMva(addr, secureLookup, targetEL, in_host);
+    inHost = (hcr.tge == 1 && hcr.e2h == 1);
+    getITBPtr(tc)->flush(*this);
+    getDTBPtr(tc)->flush(*this);

     CheckerCPU *checker = tc->getCheckerCpuPtr();
     if (checker) {
- getITBPtr(checker)->flushMva(addr, secureLookup, targetEL, in_host); - getDTBPtr(checker)->flushMva(addr, secureLookup, targetEL, in_host);
+        getITBPtr(checker)->flush(*this);
+        getDTBPtr(checker)->flush(*this);
     }
 }

@@ -130,49 +128,39 @@
 TLBIMVA::operator()(ThreadContext* tc)
 {
     HCR hcr = tc->readMiscReg(MISCREG_HCR_EL2);
-    bool in_host = (hcr.tge == 1 && hcr.e2h == 1);
-    getITBPtr(tc)->flushMvaAsid(addr, asid,
-                                  secureLookup, targetEL, in_host);
-    getDTBPtr(tc)->flushMvaAsid(addr, asid,
-                                  secureLookup, targetEL, in_host);
+    inHost = (hcr.tge == 1 && hcr.e2h == 1);
+    getITBPtr(tc)->flush(*this);
+    getDTBPtr(tc)->flush(*this);

     CheckerCPU *checker = tc->getCheckerCpuPtr();
     if (checker) {
-        getITBPtr(checker)->flushMvaAsid(
-            addr, asid, secureLookup, targetEL, in_host);
-        getDTBPtr(checker)->flushMvaAsid(
-            addr, asid, secureLookup, targetEL, in_host);
+        getITBPtr(checker)->flush(*this);
+        getDTBPtr(checker)->flush(*this);
     }
 }

 void
 ITLBIMVA::operator()(ThreadContext* tc)
 {
-    getITBPtr(tc)->flushMvaAsid(
-        addr, asid, secureLookup, targetEL);
+    getITBPtr(tc)->flush(*this);
 }

 void
 DTLBIMVA::operator()(ThreadContext* tc)
 {
-    getDTBPtr(tc)->flushMvaAsid(
-        addr, asid, secureLookup, targetEL);
+    getDTBPtr(tc)->flush(*this);
 }

 void
 TLBIIPA::operator()(ThreadContext* tc)
 {
-    getITBPtr(tc)->flushIpaVmid(addr,
-        secureLookup, targetEL);
-    getDTBPtr(tc)->flushIpaVmid(addr,
-        secureLookup, targetEL);
+    getITBPtr(tc)->flush(*this);
+    getDTBPtr(tc)->flush(*this);

     CheckerCPU *checker = tc->getCheckerCpuPtr();
     if (checker) {
-        getITBPtr(checker)->flushIpaVmid(addr,
-            secureLookup, targetEL);
-        getDTBPtr(checker)->flushIpaVmid(addr,
-            secureLookup, targetEL);
+        getITBPtr(checker)->flush(*this);
+        getDTBPtr(checker)->flush(*this);
     }
 }

diff --git a/src/arch/arm/tlbi_op.hh b/src/arch/arm/tlbi_op.hh
index 4c41068..5371da0 100644
--- a/src/arch/arm/tlbi_op.hh
+++ b/src/arch/arm/tlbi_op.hh
@@ -72,7 +72,6 @@
             (*this)(oc);
     }

-  protected:
     bool secureLookup;
     ExceptionLevel targetEL;
 };
@@ -82,10 +81,18 @@
 {
   public:
     TLBIALL(ExceptionLevel _targetEL, bool _secure)
-      : TLBIOp(_targetEL, _secure)
+      : TLBIOp(_targetEL, _secure), inHost(false)
     {}

     void operator()(ThreadContext* tc) override;
+
+    TLBIALL
+    makeStage2() const
+    {
+        return TLBIALL(EL1, secureLookup);
+    }
+
+    bool inHost;
 };

 /** Instruction TLB Invalidate All */
@@ -119,13 +126,13 @@
 {
   public:
     TLBIASID(ExceptionLevel _targetEL, bool _secure, uint16_t _asid)
-      : TLBIOp(_targetEL, _secure), asid(_asid)
+      : TLBIOp(_targetEL, _secure), asid(_asid), inHost(false)
     {}

     void operator()(ThreadContext* tc) override;

-  protected:
     uint16_t asid;
+    bool inHost;
 };

 /** Instruction TLB Invalidate by ASID match */
@@ -142,7 +149,7 @@
 };

 /** Data TLB Invalidate by ASID match */
-class DTLBIASID : public TLBIOp
+class DTLBIASID : public TLBIASID
 {
   public:
     DTLBIASID(ExceptionLevel _targetEL, bool _secure, uint16_t _asid)
@@ -163,6 +170,12 @@
     {}

     void operator()(ThreadContext* tc) override;
+
+    TLBIALLN
+    makeStage2() const
+    {
+        return TLBIALLN(EL1);
+    }
 };

 /** TLB Invalidate by VA, All ASID */
@@ -171,13 +184,13 @@
   public:
     TLBIMVAA(ExceptionLevel _targetEL, bool _secure,
              Addr _addr)
-      : TLBIOp(_targetEL, _secure), addr(_addr)
+      : TLBIOp(_targetEL, _secure), addr(_addr), inHost(false)
     {}

     void operator()(ThreadContext* tc) override;

-  protected:
     Addr addr;
+    bool inHost;
 };

 /** TLB Invalidate by VA */
@@ -186,14 +199,15 @@
   public:
     TLBIMVA(ExceptionLevel _targetEL, bool _secure,
             Addr _addr, uint16_t _asid)
-      : TLBIOp(_targetEL, _secure), addr(_addr), asid(_asid)
+      : TLBIOp(_targetEL, _secure), addr(_addr), asid(_asid),
+        inHost(false)
     {}

     void operator()(ThreadContext* tc) override;

-  protected:
     Addr addr;
     uint16_t asid;
+    bool inHost;
 };

 /** Instruction TLB Invalidate by VA */
@@ -234,7 +248,6 @@

     void operator()(ThreadContext* tc) override;

-  protected:
     Addr addr;
 };


--
To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/35240
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: Ic4fbae72dc3cfe756047148b1cf5f144298c8b08
Gerrit-Change-Number: 35240
Gerrit-PatchSet: 1
Gerrit-Owner: Giacomo Travaglini <[email protected]>
Gerrit-MessageType: newchange
_______________________________________________
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