workaround to support access to large 64-bit physical addresses

Current spike implementation erroneously assumes that a maximum physical
address is limited to (1 << MAX_PADDR_BITS).

There are few issues with this assumption:
1. if satp.MODE == Bare, then virtual addresses are equal to physical
   and there are no limitations and no additional requirements on the
   number of address bits and address values one may use.
2. the actual limit can only be derived from the current privilege
   mode and CSR configuration and thus can be determined only in runtime.

This patch implements a simple workaround by making queries to the
platform configuration as a last-ditch effort before memory access
abort.

To figure out if the current configuration supports large 64-bit
addresses only ordinary memory regions are taken into account - bus
devices (abstract devices) are excluded. Proper support for such
cases may require significant refactoring.

riscv/cfg.h

@@ -29,21 +29,36 @@ class cfg_arg_t {
 };
 
 // Configuration that describes a memory region
-class mem_cfg_t
-{
-public:
-  mem_cfg_t(reg_t base, reg_t size)
-    : base(base), size(size)
-  {
-    // The truth of these assertions should be ensured by whatever is creating
-    // the regions in the first place, but we have them here to make sure that
-    // we can't end up describing memory regions that don't make sense. They
-    // ask that the page size is a multiple of the minimum page size, that the
-    // page is aligned to the minimum page size, that the page is non-empty and
-    // that the top address is still representable in a reg_t.
-    assert((size % PGSIZE == 0) &&
-           (base % PGSIZE == 0) &&
-           (base + size > base));
+// Note: the current implementation does not support the size of 2^64
+struct mem_cfg_t {
+  bool is_valid_region() const {
+    // For a memory region to be valid it should have:
+    // * size is a multiple of the minimum page size
+    // * base is aligned to the minimum page size
+    // * memory region should be non-empty
+    // * the last accessible address should still be representable in a reg_t
+
+    // [(base + size) > base] - means that we don't have 64-bit overflow
+    // [(base + size) == 0] - allows for overflow, but only if we exactly
+    // 1 bit short -  this is to allow regions like:
+    //  { base = 0xffff_ffff_ffff_f000, size: 0x1000 }
+    return (size % PGSIZE == 0) && (base % PGSIZE == 0) &&
+      ((base + size > base) || (base + size == 0));
+  }
+
+  reg_t get_base() const {
+    assert(is_valid_region());
+    return base;
+  }
+
+  reg_t get_size() const {
+    assert(is_valid_region());
+    return size;
+  }
+
+  reg_t get_inclusive_end() const {
+    assert(is_valid_region());
+    return base + size - 1;
   }
 
   reg_t base;

riscv/sim.cc

@@ -267,9 +267,43 @@ void sim_t::set_procs_debug(bool value)
     procs[i]->set_debug(value);
 }
 
-static bool paddr_ok(reg_t addr)
+static bool is_ascending(const std::pair<reg_t, mem_t*> &L,
+                         const std::pair<reg_t, mem_t*> &R)
 {
-  return (addr >> MAX_PADDR_BITS) == 0;
+  return L.first < R.first;
+}
+
+bool sim_t::paddr_ok(reg_t addr) const
+{
+  // TODO: this implementation erroneously assumes that a maximum physical
+  // address is limited to (1 << MAX_PADDR_BITS).
+  // There are a few issues with this assumption:
+  // 1. if satp.MODE == Bare, then virtual addresses are equal to physical
+  //    and there are no limitations on the number of bits one may use.
+  // 2. the actual limit can only be derived from the current privilege
+  //    mode and CSR configuration and thus can be determined only in runtime.
+  //
+  // To do such a check properly the translation routine must return some
+  // auxiliary attributes, like translation mode, memory attributes, etc.
+  // These could be later consumed by routines like paddr_ok.
+  //
+  // In the meantime, to allow access to large physical addresses and to
+  // minimize the scope of changes we implement a small add-hoc solution to
+  // lookup platform configuration and figure out if we have any memory
+  // above (1 << MAX_PADDR_BITS)
+  if ((addr >> MAX_PADDR_BITS) == 0)
+    return true;
+
+  if (mems.empty())
+    return false;
+
+  assert(std::is_sorted(mems.begin(), mems.end(), is_ascending));
+  // last-ditch effort to detect if the platform has physical memory
+  // at the addresses greater than (1 << MAX_PADDR_BITS)
+  reg_t base_addr = mems.back().first;
+  reg_t size = mems.back().second->size();
+  reg_t last_available_pa = base_addr + size - 1;
+  return (last_available_pa >= (1ull << MAX_PADDR_BITS));
 }
 
 bool sim_t::mmio_load(reg_t addr, size_t len, uint8_t* bytes)

riscv/sim.h

@@ -68,6 +68,8 @@ class sim_t : public htif_t, public simif_t
   // Callback for processors to let the simulation know they were reset.
   void proc_reset(unsigned id);
 
+  bool paddr_ok(reg_t addr) const;
+
 private:
   isa_parser_t isa;
   const cfg_t * const cfg;

spike_main/spike.cc

@@ -8,6 +8,7 @@
 #include "extension.h"
 #include <dlfcn.h>
 #include <fesvr/option_parser.h>
+#include <limits>
 #include <stdio.h>
 #include <stdlib.h>
 #include <vector>
@@ -109,39 +110,86 @@ static void read_file_bytes(const char *filename,size_t fileoff,
   mem->store(memoff, read_sz, (uint8_t*)&read_buf[0]);
 }
 
-bool sort_mem_region(const mem_cfg_t &a, const mem_cfg_t &b)
+static bool sort_mem_region(const mem_cfg_t &a, const mem_cfg_t &b)
 {
-  if (a.base == b.base)
-    return (a.size < b.size);
+  if (a.get_base() == b.get_base())
+    return (a.get_size() < b.get_size());
   else
-    return (a.base < b.base);
+    return (a.get_base() < b.get_base());
 }
 
-void merge_overlapping_memory_regions(std::vector<mem_cfg_t> &mems)
+static bool check_mem_overlap(const mem_cfg_t& L, const mem_cfg_t& R)
 {
-  // check the user specified memory regions and merge the overlapping or
-  // eliminate the containing parts
-  assert(!mems.empty());
+  assert(L.is_valid_region());
+  assert(R.is_valid_region());
+
+  reg_t L_start = L.get_base();
+  reg_t L_end   = L.get_inclusive_end();
+
+  reg_t R_start = R.get_base();
+  reg_t R_end   = R.get_inclusive_end();
+
+  return std::max(L_start, R_start) <= std::min(L_end, R_end);
+}
+
+// there is a weird corner-case preventing two memory regions to be merged -
+// this can happen when the resulting size of a region is 2^64.
+// such regions are not representable with mem_cfg_t class
+static bool check_if_merge_covers_64bit_space(const mem_cfg_t& L,
+                                              const mem_cfg_t& R)
+{
+  if (!check_mem_overlap(L, R))
+    return false;
+
+  reg_t start = std::min(L.get_base(), R.get_base());
+  reg_t end = std::max(L.get_inclusive_end(), R.get_inclusive_end());
+
+  // if [base == 0] and [inclusive_end == (2 ^ 64 - 1)] - we can't merge such
+  // regions, since the mem_cfg_t does not support the resulting size
+  return (start == 0ull) && (end == std::numeric_limits<uint64_t>::max());
+}
+
+// one can merge only intersecting regions
+static mem_cfg_t merge_mem_regions(const mem_cfg_t& L, const mem_cfg_t& R)
+{
+  assert(check_mem_overlap(L, R));
+
+  reg_t merged_base = std::min(L.get_base(), R.get_base());
+  reg_t merged_end_incl = std::max(L.get_inclusive_end(), R.get_inclusive_end());
+  reg_t merged_size = merged_end_incl - merged_base + 1;
+
+  mem_cfg_t result{merged_base, merged_size};
+  assert(result.is_valid_region());
+  return result;
+}
+
+static void merge_overlapping_memory_regions(std::vector<mem_cfg_t> &mems)
+{
+  if (mems.empty())
+    return;
 
   std::sort(mems.begin(), mems.end(), sort_mem_region);
-  for (auto it = mems.begin() + 1; it != mems.end(); ) {
-    reg_t start = prev(it)->base;
-    reg_t end = prev(it)->base + prev(it)->size;
-    reg_t start2 = it->base;
-    reg_t end2 = it->base + it->size;
-
-    //contains -> remove
-    if (start2 >= start && end2 <= end) {
-      it = mems.erase(it);
-    //partial overlapped -> extend
-    } else if (start2 >= start && start2 < end) {
-      prev(it)->size = std::max(end, end2) - start;
-      it = mems.erase(it);
-    // no overlapping -> keep it
-    } else {
-      it++;
+
+  std::vector<mem_cfg_t> merged_mem;
+  merged_mem.push_back(mems.front());
+
+  for (auto mem_it = std::next(mems.begin()); mem_it != mems.end(); ++mem_it) {
+    const auto& mem_int = *mem_it;
+    if (!check_mem_overlap(merged_mem.back(), mem_int)) {
+      merged_mem.push_back(mem_int);
+      continue;
+    }
+    if (check_if_merge_covers_64bit_space(merged_mem.back(), mem_int)) {
+      merged_mem.clear();
+      merged_mem.push_back(mem_cfg_t{0ull, 0ull - PGSIZE});
+      merged_mem.push_back(mem_cfg_t{0ull - PGSIZE, PGSIZE});
+      break;
     }
+    merged_mem.back() = merge_mem_regions(merged_mem.back(), mem_int);
   }
+  assert(std::all_of(merged_mem.begin(), merged_mem.end(),
+                     [](const auto& Itm) { return Itm.is_valid_region(); }));
+  mems.swap(merged_mem);
 }
 
 static std::vector<mem_cfg_t> parse_mem_layout(const char* arg)
@@ -155,7 +203,7 @@ static std::vector<mem_cfg_t> parse_mem_layout(const char* arg)
     reg_t size = reg_t(mb) << 20;
     if (size != (size_t)size)
       throw std::runtime_error("Size would overflow size_t");
-    res.push_back(mem_cfg_t(reg_t(DRAM_BASE), size));
+    res.push_back(mem_cfg_t{reg_t(DRAM_BASE), size});
     return res;
   }
 
@@ -173,16 +221,21 @@ static std::vector<mem_cfg_t> parse_mem_layout(const char* arg)
     if (size % PGSIZE != 0)
       size += PGSIZE - size % PGSIZE;
 
-    if (base + size < base)
-      help();
-
     if (size != size0) {
       fprintf(stderr, "Warning: the memory at  [0x%llX, 0x%llX] has been realigned\n"
                       "to the %ld KiB page size: [0x%llX, 0x%llX]\n",
               base0, base0 + size0 - 1, long(PGSIZE / 1024), base, base + size - 1);
     }
 
-    res.push_back(mem_cfg_t(reg_t(base), reg_t(size)));
+    mem_cfg_t mem_region{base, size};
+    if (!mem_region.is_valid_region()) {
+      fprintf(stderr, "unsupported memory region [0x%lluX, 0x%lluX] specified\n",
+                      (unsigned long long)mem_region.get_base(),
+                      (unsigned long long)mem_region.get_inclusive_end());
+      exit(EXIT_FAILURE);
+    }
+
+    res.push_back(mem_region);
     if (!*p)
       break;
     if (*p != ',')
@@ -199,9 +252,11 @@ static std::vector<std::pair<reg_t, mem_t*>> make_mems(const std::vector<mem_cfg
 {
   std::vector<std::pair<reg_t, mem_t*>> mems;
   mems.reserve(layout.size());
-  for (const auto &cfg : layout) {
-    mems.push_back(std::make_pair(cfg.base, new mem_t(cfg.size)));
-  }
+  std::transform(layout.begin(), layout.end(), std::back_inserter(mems),
+                 [](const auto& cfg) {
+                   // TODO: we should use std::unique_ptr here
+                   return std::make_pair(cfg.get_base(), new mem_t(cfg.get_size()));
+                 });
   return mems;
 }