diff --git a/riscv/cfg.h b/riscv/cfg.h index 6369bd84b3..aebaf572d3 100644 --- a/riscv/cfg.h +++ b/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; diff --git a/riscv/sim.cc b/riscv/sim.cc index 0ef13b8bfb..741e685cdd 100644 --- a/riscv/sim.cc +++ b/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 &L, + const std::pair &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) diff --git a/riscv/sim.h b/riscv/sim.h index c355f3167c..8ff0777320 100644 --- a/riscv/sim.h +++ b/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; diff --git a/spike_main/spike.cc b/spike_main/spike.cc index 70b4313d59..6d089fb25c 100644 --- a/spike_main/spike.cc +++ b/spike_main/spike.cc @@ -8,6 +8,7 @@ #include "extension.h" #include #include +#include #include #include #include @@ -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 &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::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 &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 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 parse_mem_layout(const char* arg) @@ -155,7 +203,7 @@ static std::vector 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 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> make_mems(const std::vector> 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; }