diff options
author | Andrew Waterman <andrew@sifive.com> | 2023-01-10 15:44:22 -0800 |
---|---|---|
committer | Andrew Waterman <andrew@sifive.com> | 2023-01-10 15:44:22 -0800 |
commit | 3b2e8d53b39b8baba8d2e4c31408d92776b32e86 (patch) | |
tree | 54f1ed0177fdfc2284ceb9e3d24b7806af36ba78 | |
parent | 32742924154a41e457f5b64e745485a3ebcd0daf (diff) | |
parent | ca1a5fd8f0280cca41ed54cb98c21514a882dacd (diff) | |
download | riscv-isa-sim-3b2e8d53b39b8baba8d2e4c31408d92776b32e86.zip riscv-isa-sim-3b2e8d53b39b8baba8d2e4c31408d92776b32e86.tar.gz riscv-isa-sim-3b2e8d53b39b8baba8d2e4c31408d92776b32e86.tar.bz2 |
Merge branch 'aap-sc-aap-sc/mem_cfg_corner_cases'
-rw-r--r-- | ci-tests/testlib.c | 2 | ||||
-rw-r--r-- | riscv/cfg.cc | 6 | ||||
-rw-r--r-- | riscv/cfg.h | 13 | ||||
-rw-r--r-- | spike_main/spike.cc | 48 |
4 files changed, 53 insertions, 16 deletions
diff --git a/ci-tests/testlib.c b/ci-tests/testlib.c index 9aa6797..d06277f 100644 --- a/ci-tests/testlib.c +++ b/ci-tests/testlib.c @@ -7,7 +7,7 @@ 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))); + mems.push_back(std::make_pair(cfg.get_base(), new mem_t(cfg.get_size()))); } return mems; } diff --git a/riscv/cfg.cc b/riscv/cfg.cc index ef1d579..457aa92 100644 --- a/riscv/cfg.cc +++ b/riscv/cfg.cc @@ -17,7 +17,11 @@ bool mem_cfg_t::check_if_supported(reg_t base, reg_t size) // 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. + // + // Note: (base + size == 0) part of the assertion is to handle cases like + // { base = 0xffff_ffff_ffff_f000, size: 0x1000 } return (size % PGSIZE == 0) && (base % PGSIZE == 0) && - (base + size > base); + (size > 0) && + ((base + size > base) || (base + size == 0)); } diff --git a/riscv/cfg.h b/riscv/cfg.h index e9eeeda..1fb358f 100644 --- a/riscv/cfg.h +++ b/riscv/cfg.h @@ -41,6 +41,19 @@ public: mem_cfg_t(reg_t base, reg_t size); + reg_t get_base() const { + return base; + } + + reg_t get_size() const { + return size; + } + + reg_t get_inclusive_end() const { + return base + size - 1; + } + +private: reg_t base; reg_t size; }; diff --git a/spike_main/spike.cc b/spike_main/spike.cc index 2f66033..8386a49 100644 --- a/spike_main/spike.cc +++ b/spike_main/spike.cc @@ -120,18 +120,27 @@ static void read_file_bytes(const char *filename,size_t fileoff, 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()); } static bool check_mem_overlap(const mem_cfg_t& L, const mem_cfg_t& R) { - const reg_t L_end = L.base + L.size - 1; - const reg_t R_end = R.base + R.size - 1; + return std::max(L.get_base(), R.get_base()) <= std::min(L.get_inclusive_end(), R.get_inclusive_end()); +} + +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; - return std::max(L.base, R.base) <= std::min(L_end, R_end); + auto start = std::min(L.get_base(), R.get_base()); + auto end = std::max(L.get_inclusive_end(), R.get_inclusive_end()); + + return (start == 0ull) && (end == std::numeric_limits<uint64_t>::max()); } static mem_cfg_t merge_mem_regions(const mem_cfg_t& L, const mem_cfg_t& R) @@ -139,9 +148,9 @@ static mem_cfg_t merge_mem_regions(const mem_cfg_t& L, const mem_cfg_t& R) // one can merge only intersecting regions assert(check_mem_overlap(L, R)); - const reg_t merged_base = std::min(L.base, R.base); - const reg_t merged_end_incl = std::max(L.base + L.size - 1, R.base + R.size - 1); - const reg_t merged_size = merged_end_incl - merged_base + 1; + const auto merged_base = std::min(L.get_base(), R.get_base()); + const auto merged_end_incl = std::max(L.get_inclusive_end(), R.get_inclusive_end()); + const auto merged_size = merged_end_incl - merged_base + 1; return mem_cfg_t(merged_base, merged_size); } @@ -165,6 +174,18 @@ merge_overlapping_memory_regions(std::vector<mem_cfg_t> mems) merged_mem.push_back(mem_int); continue; } + // there is a weird corner case preventing two memory regions from being + // merged: if the resulting size of a region is 2^64 bytes - currently, + // such regions are not representable by mem_cfg_t class (because the + // actual size field is effectively a 64 bit value) + // so we create two smaller memory regions that total for 2^64 bytes as + // a workaround + 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); } @@ -216,15 +237,14 @@ static std::vector<mem_cfg_t> parse_mem_layout(const char* arg) const unsigned long long max_allowed_pa = (1ull << MAX_PADDR_BITS) - 1ull; assert(max_allowed_pa <= std::numeric_limits<reg_t>::max()); mem_cfg_t mem_region(base, size); - auto last_pa_region = mem_region.base + mem_region.size - 1; - if (last_pa_region > max_allowed_pa) { - int bits_required = 64 - clz(last_pa_region); + if (mem_region.get_inclusive_end() > max_allowed_pa) { + int bits_required = 64 - clz(mem_region.get_inclusive_end()); fprintf(stderr, "Unsupported memory region " "{base = 0x%" PRIX64 ", size = 0x%" PRIX64 "} specified," " which requires %d bits of physical address\n" " The largest accessible physical address " "is 0x%llX (defined by MAX_PADDR_BITS constant, which is %d)\n", - mem_region.base, mem_region.size, bits_required, + mem_region.get_base(), mem_region.get_size(), bits_required, max_allowed_pa, MAX_PADDR_BITS); exit(EXIT_FAILURE); } @@ -249,7 +269,7 @@ 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))); + mems.push_back(std::make_pair(cfg.get_base(), new mem_t(cfg.get_size()))); } return mems; } |