refactor: remove the E flag

This increases the space for DID
Checking for empty address range simply checks its length.
Shadow address ranges are neither device nor memory.
This is because they cannot be read/written with read_device/write_device.
They do not implement get_host_memory() either.

Author: diegonehab

src/Makefile

@@ -171,19 +171,19 @@ DEFS+=-DJSON_HAS_FILESYSTEM=0
 DEFS+=-DBOOST_ASIO_DISABLE_THREADS -DBOOST_ASIO_DISABLE_EPOLL -DBOOST_ASIO_DISABLE_EVENTFD
 
 ifeq ($(dump),yes)
-#DUMP_DEFS+=-DDUMP_ILLEGAL_INSN_EXCEPTIONS
-#DUMP_DEFS+=-DDUMP_EXCEPTIONS
-#DUMP_DEFS+=-DDUMP_INTERRUPTS
-#DUMP_DEFS+=-DDUMP_MMU_EXCEPTIONS
-#DUMP_DEFS+=-DDUMP_INVALID_CSR
-#DUMP_DEFS+=-DDUMP_REGS
-#DUMP_DEFS+=-DDUMP_INSN_HIST
-#DUMP_DEFS+=-DDUMP_STATS
+DUMP_DEFS+=-DDUMP_ILLEGAL_INSN_EXCEPTIONS
+DUMP_DEFS+=-DDUMP_EXCEPTIONS
+DUMP_DEFS+=-DDUMP_INTERRUPTS
+DUMP_DEFS+=-DDUMP_MMU_EXCEPTIONS
+DUMP_DEFS+=-DDUMP_INVALID_CSR
+DUMP_DEFS+=-DDUMP_REGS
+DUMP_DEFS+=-DDUMP_INSN_HIST
+DUMP_DEFS+=-DDUMP_STATS
 DUMP_DEFS+=-DDUMP_INSN
-#DUMP_DEFS+=-DDUMP_UARCH_INSN
-#DUMP_DEFS+=-DDUMP_SCOPED_NOTE
-#DUMP_DEFS+=-DDUMP_STATE_ACCESS
-#DUMP_DEFS+=-DDUMP_UARCH_STATE_ACCESS
+DUMP_DEFS+=-DDUMP_UARCH_INSN
+DUMP_DEFS+=-DDUMP_SCOPED_NOTE
+DUMP_DEFS+=-DDUMP_STATE_ACCESS
+DUMP_DEFS+=-DDUMP_UARCH_STATE_ACCESS
 endif
 DEFS += $(DUMP_DEFS)
 # Pass down UARCH_DEFS to sub-makefiles

src/address-range.h

@@ -56,7 +56,6 @@ class address_range {
         m_length{0},
         m_length_bit_ceil{0},
         m_flags{} {
-        m_flags.E = true;
         for (unsigned i = 0; i < std::min<unsigned>(N, m_description.size() - 1); ++i) {
             m_description[i] = description[i];
         }
@@ -70,7 +69,8 @@ class address_range {
     constexpr virtual ~address_range() {}; // = default; // doesn't work due to bug in gcc
 
     template <typename ABRT, size_t N, typename... ARGS>
-    [[noreturn]] static void ABRTF(ABRT abrt, const char (&fmt)[N], ARGS... args) {
+    [[noreturn]]
+    static void ABRTF(ABRT abrt, const char (&fmt)[N], ARGS... args) {
         char buf[256]{};
         std::ignore = snprintf(buf, std::size(buf), fmt, args...);
         abrt(buf);
@@ -98,36 +98,29 @@ class address_range {
             m_description[i] = description[i];
         }
         // All address ranges must be page-aligned
-        if ((m_length & ~AR_ISTART_START_MASK) != 0) {
-            ABRTF(abrt, "length must be multiple of page size when initializing %s", m_description);
+        if ((m_length & ~PMA_ISTART_START_MASK) != 0) {
+            ABRTF(abrt, "length must be multiple of page size when initializing %s", description);
         }
-        if ((m_start & ~AR_ISTART_START_MASK) != 0) {
-            ABRTF(abrt, "start of %s (0x%" PRIx64 ") must be aligned to page boundary of %d bytes", m_description,
-                start, AR_PAGE_SIZE);
+        if ((m_start & ~PMA_ISTART_START_MASK) != 0) {
+            ABRTF(abrt, "start of %s (0x%" PRIx64 ") must be aligned to page boundary of %" PRId64 " bytes",
+                description, start, AR_PAGE_SIZE);
         }
         // It must be possible to round length up to the next power of two
         if (m_length_bit_ceil == 0) {
-            ABRTF(abrt, "range too long when initializing %s", m_description);
+            ABRTF(abrt, "address range too long when initializing %s", description);
         }
         // Empty range must really be empty
         if (m_length == 0) {
             if (m_start != 0) {
-                ABRTF(abrt, "range with length 0 must start at 0 when initializing %s", m_description);
-            }
-            if (!m_flags.E) {
-                ABRTF(abrt, "range with length 0 must be flagged empty when initializing %s", m_description);
+                ABRTF(abrt, "range with length 0 must start at 0 when initializing %s", description);
             }
             if (m_flags.M) {
-                ABRTF(abrt, "memory range cannot be empty when initializing %s", m_description);
+                ABRTF(abrt, "memory address range cannot have length 0 when initializing %s", description);
             }
             if (m_flags.IO) {
-                ABRTF(abrt, "device range cannot be empty when initializing %s", m_description);
+                ABRTF(abrt, "device address range cannot have length 0 when initializing %s", description);
             }
         }
-        // Non-empty range must either be memory or device
-        if (static_cast<int>(m_flags.M) + static_cast<int>(m_flags.IO) + static_cast<int>(m_flags.E) != 1) {
-            ABRTF(abrt, "range must be one of empty, memory, or device when initializing %s", m_description);
-        }
     }
 
     /// \brief Checks if a range of addresses is entirely contained within this range
@@ -178,36 +171,39 @@ class address_range {
 
     /// \brief Test if address range is occupied by memory
     /// \returns True if and only if range is occupied by memory
+    /// \details In this case, get_host_memory() is guaranteed not to return nullptr.
     bool is_memory() const noexcept {
         return m_flags.M;
     }
 
     /// \brief Test if address range is occupied by a device
     /// \returns True if and only if range is occupied by a device
+    /// \details In this case, read_device() and write_device() are operational.
     bool is_device() const noexcept {
         return m_flags.IO;
     }
 
     /// \brief Test if address range is empty
     /// \returns True if and only if range is empty
+    /// \details Empty ranges should be used only for sentinels.
     bool is_empty() const noexcept {
-        return m_flags.E;
+        return m_length == 0;
     }
 
     /// \brief Tests if range is readable
-    /// \returns True if and only if range is readable
+    /// \returns True if and only if range is readable from within the machine.
     bool is_readable() const noexcept {
         return m_flags.R;
     }
 
     /// \brief Tests if range is writeable
-    /// \returns True if and only if range is writeable
+    /// \returns True if and only if range is writeable from within the machine.
     bool is_writeable() const noexcept {
         return m_flags.W;
     }
 
     /// \brief Tests if range is executable
-    /// \returns True if and only if range is executable
+    /// \returns True if and only if range is executable from within the machine.
     bool is_executable() const noexcept {
         return m_flags.X;
     }

src/clint-address-range.h

@@ -19,6 +19,7 @@
 
 #include <cstdint>
 
+#include "address-range-constants.h"
 #include "pristine-address-range.h"
 
 /// \file
@@ -31,7 +32,6 @@ class clint_address_range final : public pristine_address_range {
     static constexpr pmas_flags m_clint_flags{
         .M = false,
         .IO = true,
-        .E = false,
         .R = true,
         .W = true,
         .X = false,
@@ -42,8 +42,8 @@ class clint_address_range final : public pristine_address_range {
 
 public:
     template <typename ABRT>
-    clint_address_range(uint64_t start, uint64_t length, ABRT abrt) :
-        pristine_address_range("CLINT device", start, length, m_clint_flags, abrt) {
+    explicit clint_address_range(ABRT abrt) :
+        pristine_address_range("CLINT device", AR_CLINT_START, AR_CLINT_LENGTH, m_clint_flags, abrt) {
         ;
     }
 
@@ -61,8 +61,8 @@ class clint_address_range final : public pristine_address_range {
 };
 
 template <typename ABRT>
-static inline clint_address_range make_clint_address_range(uint64_t start, uint64_t length, ABRT abrt) {
-    return clint_address_range{start, length, abrt};
+static inline clint_address_range make_clint_address_range(ABRT abrt) {
+    return clint_address_range{abrt};
 }
 
 } // namespace cartesi

src/find-pmas-entry.h

@@ -40,7 +40,7 @@ address_range &find_pma(const STATE_ACCESS a, uint64_t paddr, uint64_t &index) {
         // The pmas array always contain a sentinel.
         // It is an entry with zero length.
         // If we hit it, return it
-        if (unlikely(ar.get_length() == 0)) {
+        if (unlikely(ar.is_empty())) {
             return ar;
         }
         if (ar.contains_absolute(paddr, sizeof(T))) {

src/htif-address-range.h

@@ -19,6 +19,7 @@
 
 #include <cstdint>
 
+#include "address-range-constants.h"
 #include "i-device-state-access.h"
 #include "pmas-constants.h"
 #include "pristine-address-range.h"
@@ -33,7 +34,6 @@ class htif_address_range final : public pristine_address_range {
     static constexpr pmas_flags m_htif_flags{
         .M = false,
         .IO = true,
-        .E = false,
         .R = true,
         .W = true,
         .X = false,
@@ -44,8 +44,8 @@ class htif_address_range final : public pristine_address_range {
 
 public:
     template <typename ABRT>
-    htif_address_range(uint64_t start, uint64_t length, ABRT abrt) :
-        pristine_address_range("HTIF device", start, length, m_htif_flags, abrt) {
+    explicit htif_address_range(ABRT abrt) :
+        pristine_address_range("HTIF device", AR_HTIF_START, AR_HTIF_LENGTH, m_htif_flags, abrt) {
         ;
     }
 
@@ -63,8 +63,8 @@ class htif_address_range final : public pristine_address_range {
 };
 
 template <typename ABRT>
-static inline htif_address_range make_htif_address_range(uint64_t start, uint64_t length, ABRT abrt) {
-    return htif_address_range{start, length, abrt};
+static inline htif_address_range make_htif_address_range(ABRT abrt) {
+    return htif_address_range{abrt};
 }
 
 } // namespace cartesi