base: Encapsulate the underlying fields in AddrRange

This patch makes the start and end address private in a move to
prevent direct manipulation and matching of ranges based on these
fields. This is done so that a transition to ranges with interleaving
support is possible.

As a result of hiding the start and end, a number of member functions
are needed to perform the comparisons and manipulations that
previously took place directly on the members. An accessor function is
provided for the start address, and a function is added to test if an
address is within a range. As a result of the latter the != and ==
operator is also removed in favour of the member function. A member
function that returns a string representation is also created to allow
debug printing.

In general, this patch does not add any functionality, but it does
take us closer to a situation where interleaving (and more cleverness)
can be added under the bonnet without exposing it to the user. More on
that in a later patch.

10 files changed, 104 insertions, 93 deletions

diff --git a/src/arch/arm/linux/system.cc b/src/arch/arm/linux/system.cc
index b06439406..74950cbaf 100644
--- a/src/arch/arm/linux/system.cc
+++ b/src/arch/arm/linux/system.cc
@@ -171,7 +171,7 @@ LinuxArmSystem::initState()
         }
         AtagMem am;
         am.memSize(atagRanges.begin()->size());
-        am.memStart(atagRanges.begin()->start);
+        am.memStart(atagRanges.begin()->start());
 
         AtagCmdline ad;
         ad.cmdline(params()->boot_osflags);
diff --git a/src/base/addr_range.hh b/src/base/addr_range.hh
index dcd53b77a..1e86aa859 100644
--- a/src/base/addr_range.hh
+++ b/src/base/addr_range.hh
@@ -45,32 +45,55 @@
 #ifndef __BASE_ADDR_RANGE_HH__
 #define __BASE_ADDR_RANGE_HH__
 
-#include <utility> // pair & make_pair
-
+#include "base/cprintf.hh"
 #include "base/types.hh"
 
 class AddrRange
 {
 
-  public:
+  private:
+
+    /// Private fields for the start and end of the range. In the
+    /// future, these will be extended with interleaving functionality
+    /// and hence should never be manipulated directly.
+    Addr _start;
+    Addr _end;
 
-    Addr start;
-    Addr end;
+  public:
 
     AddrRange()
-        : start(1), end(0)
+        : _start(1), _end(0)
     {}
 
     AddrRange(Addr _start, Addr _end)
-        : start(_start), end(_end)
+        : _start(_start), _end(_end)
     {}
 
-    AddrRange(const std::pair<Addr, Addr> &r)
-        : start(r.first), end(r.second)
-    {}
+    /**
+     * Get the size of the address range. For a case where
+     * interleaving is used this should probably cause a panic.
+     */
+    Addr size() const { return _end - _start + 1; }
+
+    /**
+     * Determine if the range is valid.
+     */
+    bool valid() const { return _start < _end; }
+
+    /**
+     * Get the start address of the range.
+     */
+    Addr start() const { return _start; }
 
-    Addr size() const { return end - start + 1; }
-    bool valid() const { return start < end; }
+    /**
+     * Get a string representation of the range. This could
+     * alternatively be implemented as a operator<<, but at the moment
+     * that seems like overkill.
+     */
+    std::string to_string() const
+    {
+        return csprintf("[%#llx : %#llx]", _start, _end);
+    }
 
     /**
      * Determine if another range intersects this one, i.e. if there
@@ -82,8 +105,7 @@ class AddrRange
      */
     bool intersects(const AddrRange& r) const
     {
-        return (start <= r.start && end >= r.start) ||
-            (start <= r.end && end >= r.end);
+        return _start <= r._end && _end >= r._start;
     }
 
     /**
@@ -96,60 +118,50 @@ class AddrRange
      */
     bool isSubset(const AddrRange& r) const
     {
-        return start >= r.start && end <= r.end;
+        return _start >= r._start && _end <= r._end;
+    }
+
+    /**
+     * Determine if the range contains an address.
+     *
+     * @param a Address to compare with
+     * @return true if the address is in the range
+     */
+    bool contains(const Addr& a) const
+    {
+        return a >= _start && a <= _end;
     }
-};
 
 /**
  * Keep the operators away from SWIG.
  */
 #ifndef SWIG
 
-/**
- * @param range1 is a range.
- * @param range2 is a range.
- * @return if range1 is less than range2 and does not overlap range1.
- */
-inline bool
-operator<(const AddrRange& range1, const AddrRange& range2)
-{
-    return range1.start < range2.start;
-}
-
-/**
- * @param addr address in the range
- * @param range range compared against.
- * @return indicates that the address is not within the range.
- */
-inline bool
-operator!=(const Addr& addr, const AddrRange& range)
-{
-    return addr < range.start || addr > range.end;
-}
+    /**
+     * Less-than operator used to turn an STL map into a binary search
+     * tree of non-overlapping address ranges.
+     *
+     * @param r Range to compare with
+     * @return true if the start address is less than that of the other range
+     */
+    bool operator<(const AddrRange& r) const
+    {
+        return _start < r._start;
+    }
 
-/**
- * @param range range compared against.
- * @param pos position compared to the range.
- * @return indicates that position pos is within the range.
- */
-inline bool
-operator==(const AddrRange& range, const Addr& addr)
-{
-    return addr >= range.start && addr <= range.end;
-}
+#endif // SWIG
+};
 
 inline AddrRange
 RangeEx(Addr start, Addr end)
-{ return std::make_pair(start, end - 1); }
+{ return AddrRange(start, end - 1); }
 
 inline AddrRange
 RangeIn(Addr start, Addr end)
-{ return std::make_pair(start, end); }
+{ return AddrRange(start, end); }
 
 inline AddrRange
 RangeSize(Addr start, Addr size)
-{ return std::make_pair(start, start + size - 1); }
-
-#endif // SWIG
+{ return AddrRange(start, start + size - 1); }
 
 #endif // __BASE_ADDR_RANGE_HH__
diff --git a/src/base/addr_range_map.hh b/src/base/addr_range_map.hh
index c35befdce..1992c5e48 100644
--- a/src/base/addr_range_map.hh
+++ b/src/base/addr_range_map.hh
@@ -73,7 +73,7 @@ class AddrRangeMap
         i = tree.upper_bound(r);
 
         if (i == tree.begin()) {
-            if (i->first.start <= r.end && i->first.end >= r.start)
+            if (i->first.intersects(r))
                 return i;
             else
                 // Nothing could match, so return end()
@@ -82,7 +82,7 @@ class AddrRangeMap
 
         --i;
 
-        if (i->first.start <= r.end && i->first.end >= r.start)
+        if (i->first.intersects(r))
             return i;
 
         return tree.end();
@@ -96,7 +96,7 @@ class AddrRangeMap
         i = tree.upper_bound(r);
 
         if (i == tree.begin()) {
-            if (i->first.start <= r.end && i->first.end >= r.start)
+            if (i->first.intersects(r))
                 return i;
             else
                 // Nothing could match, so return end()
@@ -105,7 +105,7 @@ class AddrRangeMap
 
         --i;
 
-        if (i->first.start <= r.end && i->first.end >= r.start)
+        if (i->first.intersects(r))
             return i;
 
         return tree.end();
diff --git a/src/kern/tru64/tru64_events.cc b/src/kern/tru64/tru64_events.cc
index fd4c20bdd..943d961fb 100644
--- a/src/kern/tru64/tru64_events.cc
+++ b/src/kern/tru64/tru64_events.cc
@@ -65,7 +65,7 @@ BadAddrEvent::process(ThreadContext *tc)
     // get the address ranges of the connected slave port
     AddrRangeList resp = dataPort.getAddrRanges();
     for (iter = resp.begin(); iter != resp.end(); iter++) {
-        if (*iter == (K0Seg2Phys(a0) & PAddrImplMask))
+        if (iter->contains(K0Seg2Phys(a0) & PAddrImplMask))
             found = true;
     }
 
diff --git a/src/mem/abstract_mem.cc b/src/mem/abstract_mem.cc
index 086985f8d..fb8b7d81b 100644
--- a/src/mem/abstract_mem.cc
+++ b/src/mem/abstract_mem.cc
@@ -303,8 +303,8 @@ AbstractMemory::checkLockedAddrList(PacketPtr pkt)
 void
 AbstractMemory::access(PacketPtr pkt)
 {
-    assert(pkt->getAddr() >= range.start &&
-           (pkt->getAddr() + pkt->getSize() - 1) <= range.end);
+    assert(AddrRange(pkt->getAddr(),
+                     pkt->getAddr() + pkt->getSize() - 1).isSubset(range));
 
     if (pkt->memInhibitAsserted()) {
         DPRINTF(MemoryAccess, "mem inhibited on 0x%x: not responding\n",
@@ -312,7 +312,7 @@ AbstractMemory::access(PacketPtr pkt)
         return;
     }
 
-    uint8_t *hostAddr = pmemAddr + pkt->getAddr() - range.start;
+    uint8_t *hostAddr = pmemAddr + pkt->getAddr() - range.start();
 
     if (pkt->cmd == MemCmd::SwapReq) {
         TheISA::IntReg overwrite_val;
@@ -384,10 +384,10 @@ AbstractMemory::access(PacketPtr pkt)
 void
 AbstractMemory::functionalAccess(PacketPtr pkt)
 {
-    assert(pkt->getAddr() >= range.start &&
-           (pkt->getAddr() + pkt->getSize() - 1) <= range.end);
+    assert(AddrRange(pkt->getAddr(),
+                     pkt->getAddr() + pkt->getSize() - 1).isSubset(range));
 
-    uint8_t *hostAddr = pmemAddr + pkt->getAddr() - range.start;
+    uint8_t *hostAddr = pmemAddr + pkt->getAddr() - range.start();
 
     if (pkt->isRead()) {
         if (pmemAddr)
diff --git a/src/mem/abstract_mem.hh b/src/mem/abstract_mem.hh
index d6d33c8ee..c1ecbdba4 100644
--- a/src/mem/abstract_mem.hh
+++ b/src/mem/abstract_mem.hh
@@ -266,7 +266,7 @@ class AbstractMemory : public MemObject
      *
      * @return the start address of the memory
      */
-    Addr start() const { return range.start; }
+    Addr start() const { return range.start(); }
 
     /**
      *  Should this memory be passed to the kernel and part of the OS
diff --git a/src/mem/addr_mapper.cc b/src/mem/addr_mapper.cc
index b754cb7e6..660848c82 100644
--- a/src/mem/addr_mapper.cc
+++ b/src/mem/addr_mapper.cc
@@ -253,9 +253,9 @@ Addr
 RangeAddrMapper::remapAddr(Addr addr) const
 {
     for (int i = 0; i < originalRanges.size(); ++i) {
-        if (originalRanges[i] == addr) {
-            Addr offset = addr - originalRanges[i].start;
-            return offset + remappedRanges[i].start;
+        if (originalRanges[i].contains(addr)) {
+            Addr offset = addr - originalRanges[i].start();
+            return offset + remappedRanges[i].start();
         }
     }
 
@@ -277,11 +277,12 @@ RangeAddrMapper::getAddrRanges() const
                       " ranges but are not a subset.\n");
             if (range.isSubset(originalRanges[j])) {
                 // range is a subset
-                Addr offset = range.start - originalRanges[j].start;
-                range.start -= offset;
-                range.end -= offset;
+                Addr offset = range.start() - originalRanges[j].start();
+                Addr start = range.start() - offset;
+                ranges.push_back(AddrRange(start, start + range.size() - 1));
+            } else {
+                ranges.push_back(range);
             }
-            ranges.push_back(range);
         }
     }
 
diff --git a/src/mem/bus.cc b/src/mem/bus.cc
index ddbdcb136..e8a7084b0 100644
--- a/src/mem/bus.cc
+++ b/src/mem/bus.cc
@@ -357,7 +357,7 @@ BaseBus::findPort(Addr addr)
 
     // Check if this matches the default range
     if (useDefaultRange) {
-        if (defaultRange == addr) {
+        if (defaultRange.contains(addr)) {
             DPRINTF(BusAddrRanges, "  found addr %#llx on default\n",
                     addr);
             return defaultPortID;
@@ -430,8 +430,8 @@ BaseBus::recvRangeChange(PortID master_port_id)
         AddrRangeList ranges = masterPorts[master_port_id]->getAddrRanges();
 
         for (AddrRangeConstIter r = ranges.begin(); r != ranges.end(); ++r) {
-            DPRINTF(BusAddrRanges, "Adding range %#llx : %#llx for id %d\n",
-                    r->start, r->end, master_port_id);
+            DPRINTF(BusAddrRanges, "Adding range %s for id %d\n",
+                    r->to_string(), master_port_id);
             if (portMap.insert(*r, master_port_id) == portMap.end()) {
                 PortID conflict_id = portMap.find(*r)->second;
                 fatal("%s has two ports with same range:\n\t%s\n\t%s\n",
@@ -466,9 +466,9 @@ BaseBus::recvRangeChange(PortID master_port_id)
                         // overlapping the default range
                         if (r->intersects(defaultRange) &&
                             !r->isSubset(defaultRange))
-                            fatal("Range %#llx : %#llx intersects the " \
+                            fatal("Range %s intersects the " \
                                   "default range of %s but is not a " \
-                                  "subset\n", r->start, r->end, name());
+                                  "subset\n", r->to_string(), name());
                     }
                 }
             }
@@ -497,18 +497,16 @@ BaseBus::getAddrRanges() const
     // start out with the default range
     AddrRangeList ranges;
     ranges.push_back(defaultRange);
-    DPRINTF(BusAddrRanges, "  -- %#llx : %#llx DEFAULT\n",
-            defaultRange.start, defaultRange.end);
+    DPRINTF(BusAddrRanges, "  -- %s DEFAULT\n", defaultRange.to_string());
 
     // add any range that is not a subset of the default range
     for (PortMapConstIter p = portMap.begin(); p != portMap.end(); ++p) {
         if (useDefaultRange && p->first.isSubset(defaultRange)) {
-            DPRINTF(BusAddrRanges, "  -- %#llx : %#llx is a SUBSET\n",
-                    p->first.start, p->first.end);
+            DPRINTF(BusAddrRanges, "  -- %s is a SUBSET\n",
+                    p->first.to_string());
         } else {
             ranges.push_back(p->first);
-            DPRINTF(BusAddrRanges, "  -- %#llx : %#llx\n",
-                    p->first.start, p->first.end);
+            DPRINTF(BusAddrRanges, "  -- %s\n", p->first.to_string());
         }
     }
 
diff --git a/src/mem/bus.hh b/src/mem/bus.hh
index 19ffa020c..59dabbfe4 100644
--- a/src/mem/bus.hh
+++ b/src/mem/bus.hh
@@ -264,13 +264,13 @@ class BaseBus : public MemObject
     // Checks the cache and returns the id of the port that has the requested
     // address within its range
     inline PortID checkPortCache(Addr addr) const {
-        if (portCache[0].valid && portCache[0].range == addr) {
+        if (portCache[0].valid && portCache[0].range.contains(addr)) {
             return portCache[0].id;
         }
-        if (portCache[1].valid && portCache[1].range == addr) {
+        if (portCache[1].valid && portCache[1].range.contains(addr)) {
             return portCache[1].id;
         }
-        if (portCache[2].valid && portCache[2].range == addr) {
+        if (portCache[2].valid && portCache[2].range.contains(addr)) {
             return portCache[2].id;
         }
 
diff --git a/src/mem/physical.cc b/src/mem/physical.cc
index b6f4c7a95..ef1f7159e 100644
--- a/src/mem/physical.cc
+++ b/src/mem/physical.cc
@@ -120,8 +120,8 @@ PhysicalMemory::createBackingStore(AddrRange range,
                                    const vector<AbstractMemory*>& _memories)
 {
     // perform the actual mmap
-    DPRINTF(BusAddrRanges, "Creating backing store for range %x:%x\n",
-            range.start, range.end);
+    DPRINTF(BusAddrRanges, "Creating backing store for range %s\n",
+            range.to_string());
     int map_flags = MAP_ANON | MAP_PRIVATE;
     uint8_t* pmem = (uint8_t*) mmap(NULL, range.size(),
                                     PROT_READ | PROT_WRITE,
@@ -129,8 +129,8 @@ PhysicalMemory::createBackingStore(AddrRange range,
 
     if (pmem == (uint8_t*) MAP_FAILED) {
         perror("mmap");
-        fatal("Could not mmap %d bytes for range %x:%x!\n", range.size(),
-              range.start, range.end);
+        fatal("Could not mmap %d bytes for range %s!\n", range.size(),
+              range.to_string());
     }
 
     // remember this backing store so we can checkpoint it and unmap
@@ -157,8 +157,8 @@ PhysicalMemory::createBackingStore(AddrRange range,
 
     if (init_to_zero != 0) {
         if (init_to_zero != _memories.size())
-            fatal("Some, but not all memories in range %x:%x are set zero\n",
-                  range.start, range.end);
+            fatal("Some, but not all memories in range %s are set zero\n",
+                  range.to_string());
 
         memset(pmem, 0, range.size());
     }
@@ -176,7 +176,7 @@ bool
 PhysicalMemory::isMemAddr(Addr addr) const
 {
     // see if the address is within the last matched range
-    if (addr != rangeCache) {
+    if (!rangeCache.contains(addr)) {
         // lookup in the interval tree
         AddrRangeMap<AbstractMemory*>::const_iterator r = addrMap.find(addr);
         if (r == addrMap.end()) {