Skip to content

Commit

Permalink
refactor: combine calloc/calloc+copy/mmap memory ranges constructors
Browse files Browse the repository at this point in the history
  • Loading branch information
@edubart
edubart committed Mar 10, 2025
1 parent d95e21c commit 6a24b73
Show file tree
Hide file tree
Showing 9 changed files with 385 additions and 288 deletions.
1 change: 1 addition & 0 deletions src/Makefile
View file
Original file line number Diff line number Diff line change
Expand Up @@ -358,6 +358,7 @@ LIBCARTESI_OBJS:= \
machine.o \
memory-address-range.o \
os.o \
os-mmap.o \
plic-address-range.o \
pristine-merkle-tree.o \
replay-step-state-access-interop.o \
Expand Down
55 changes: 16 additions & 39 deletions src/machine.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -94,26 +94,6 @@ using namespace std::string_literals;

static const auto throw_invalid_argument = [](const char *err) { throw std::invalid_argument{err}; };

/// \brief Creates a memory address range.
/// \param d Description of address range for use in error messages.
/// \param start Target physical address where range starts.
/// \param length Length of range, in bytes.
/// \param f Flags for address range.
/// \param image_filename Path to backing file.
/// \param shared If true, changes to memory range reflect in backing file.
/// \returns New address range with flags already set.
/// \details If \p image_filename is non-empty, return a memory-mapped range, otherwise use calloc.
static inline auto make_memory_address_range(const std::string &d, uint64_t start, uint64_t length, pmas_flags flags,
const std::string &image_filename, bool shared) {
if (image_filename.empty() && shared) {
throw std::invalid_argument{"shared address range requires non-empty image filename when initializing " + d};
}
if (image_filename.empty() || length > static_cast<uint64_t>(os_get_file_length(image_filename.c_str()))) {
return make_callocd_memory_address_range(d, start, length, flags, image_filename);
}
return make_mmapd_memory_address_range(d, start, length, flags, image_filename, shared);
}

void machine::check_address_range(const address_range &ar, register_where where) {
if (!where.interpret && !where.merkle) {
throw std::runtime_error{"address range "s + ar.get_description() + " must be registered somwhere"s};
Expand Down Expand Up @@ -157,8 +137,8 @@ void machine::replace_memory_range(const memory_range_config &config) {
}
// Replace range, preserving original flags.
// This will automatically start with all pages dirty.
existing = make_moved_unique(make_memory_address_range(existing->get_description(), existing->get_start(),
existing->get_length(), existing->get_flags(), config.image_filename, config.shared));
existing = make_moved_unique(memory_address_range{existing->get_description(), existing->get_start(),
existing->get_length(), existing->get_flags(), config.image_filename, config.shared});
return;
}
}
Expand Down Expand Up @@ -195,14 +175,13 @@ void machine::init_uarch(const uarch_config &c) {
constexpr auto ram_description = "uarch RAM";
if (c.ram.image_filename.empty()) {
m_us.ram = &register_address_range(
make_callocd_memory_address_range(ram_description, AR_UARCH_RAM_START, UARCH_RAM_LENGTH, uram_flags),
memory_address_range{ram_description, AR_UARCH_RAM_START, UARCH_RAM_LENGTH, uram_flags},
register_where{.merkle = true, .interpret = false});
memcpy(m_us.ram->get_host_memory(), uarch_pristine_ram, uarch_pristine_ram_len);
} else {
m_us.ram =
&register_address_range(make_memory_address_range(ram_description, AR_UARCH_RAM_START, UARCH_RAM_LENGTH,
uram_flags, c.ram.image_filename, false /* not shared */),
register_where{.merkle = true, .interpret = false});
m_us.ram = &register_address_range(memory_address_range{ram_description, AR_UARCH_RAM_START, UARCH_RAM_LENGTH,
uram_flags, c.ram.image_filename, false /* not shared */},
register_where{.merkle = true, .interpret = false});
}
}

Expand Down Expand Up @@ -291,8 +270,7 @@ void machine::init_ram_ar(const ram_config &ram) {
if (ram.length == 0) {
throw std::invalid_argument("RAM length cannot be zero");
}
register_address_range(
make_callocd_memory_address_range("RAM"s, AR_RAM_START, ram.length, ram_flags, ram.image_filename),
register_address_range(memory_address_range{"RAM"s, AR_RAM_START, ram.length, ram_flags, ram.image_filename},
register_where{.merkle = true, .interpret = true});
}

Expand Down Expand Up @@ -333,7 +311,7 @@ void machine::init_flash_drive_ars(flash_drive_configs &flash_drive) {
f.length = length;
}
register_address_range(
make_memory_address_range(flash_description, f.start, f.length, flash_flags, f.image_filename, f.shared),
memory_address_range{flash_description, f.start, f.length, flash_flags, f.image_filename, f.shared},
register_where{.merkle = true, .interpret = true});
i++;
}
Expand Down Expand Up @@ -441,11 +419,11 @@ void machine::init_cmio_ars(const cmio_config &c) {
.IW = true,
.DID = PMA_ISTART_DID::cmio_rx_buffer,
};
register_address_range(make_memory_address_range("CMIO tx buffer memory range"s, AR_CMIO_TX_BUFFER_START,
AR_CMIO_TX_BUFFER_LENGTH, tx_flags, c.tx_buffer.image_filename, c.tx_buffer.shared),
register_address_range(memory_address_range{"CMIO tx buffer memory range"s, AR_CMIO_TX_BUFFER_START,
AR_CMIO_TX_BUFFER_LENGTH, tx_flags, c.tx_buffer.image_filename, c.tx_buffer.shared},
register_where{.merkle = true, .interpret = true});
register_address_range(make_memory_address_range("CMIO rx buffer memory range"s, AR_CMIO_RX_BUFFER_START,
AR_CMIO_RX_BUFFER_LENGTH, rx_flags, c.rx_buffer.image_filename, c.rx_buffer.shared),
register_address_range(memory_address_range{"CMIO rx buffer memory range"s, AR_CMIO_RX_BUFFER_START,
AR_CMIO_RX_BUFFER_LENGTH, rx_flags, c.rx_buffer.image_filename, c.rx_buffer.shared},
register_where{.merkle = true, .interpret = true});
}

Expand Down Expand Up @@ -499,7 +477,7 @@ void machine::init_pmas_contents(memory_address_range &pmas) const {

void machine::init_tlb_contents(const std::string &image_filename) {
if (!image_filename.empty()) {
auto shadow_tlb_ptr = make_unique_mmap<shadow_tlb_state>(image_filename.c_str(), 1, false /* not shared */);
auto shadow_tlb_ptr = make_unique_mmap<shadow_tlb_state>(1, os_mmap_flags{}, image_filename);
auto &shadow_tlb = *shadow_tlb_ptr;
for (auto set_index : {TLB_CODE, TLB_READ, TLB_WRITE}) {
for (uint64_t slot_index = 0; slot_index < TLB_SET_SIZE; ++slot_index) {
Expand Down Expand Up @@ -532,8 +510,7 @@ static inline auto make_dtb_address_range(const std::string &image_filename) {
.IW = true,
.DID = PMA_ISTART_DID::memory,
};
return make_memory_address_range("DTB"s, AR_DTB_START, AR_DTB_LENGTH, dtb_flags, image_filename,
false /* not shared */);
return memory_address_range{"DTB"s, AR_DTB_START, AR_DTB_LENGTH, dtb_flags, image_filename, false /* not shared */};
}

void machine::init_dtb_contents(const machine_config &c, address_range &dtb) {
Expand All @@ -553,7 +530,7 @@ static inline auto make_pmas_address_range() {
.IW = false,
.DID = PMA_ISTART_DID::memory,
};
return make_callocd_memory_address_range("PMAs", AR_PMAS_START, AR_PMAS_LENGTH, m_pmas_flags);
return memory_address_range{"PMAs", AR_PMAS_START, AR_PMAS_LENGTH, m_pmas_flags};
}

// ??D It is best to leave the std::move() on r because it may one day be necessary!
Expand Down Expand Up @@ -2502,7 +2479,7 @@ interpreter_break_reason machine::log_step(uint64_t mcycle_count, const std::str
interpreter_break_reason machine::verify_step(const hash_type &root_hash_before, const std::string &filename,
uint64_t mcycle_count, const hash_type &root_hash_after) {
auto data_length = os_get_file_length(filename.c_str(), "step log file");
auto data = make_unique_mmap<unsigned char>(filename.c_str(), data_length, false /* not shared */);
auto data = make_unique_mmap<unsigned char>(data_length, os_mmap_flags{}, filename);
replay_step_state_access::context context;
replay_step_state_access a(context, data.get(), data_length, root_hash_before);
uint64_t mcycle_end{};
Expand Down
47 changes: 2 additions & 45 deletions src/memory-address-range.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,9 +28,9 @@ class base_error : public std::invalid_argument {
};

memory_address_range::memory_address_range(const std::string &description, uint64_t start, uint64_t length,
const pmas_flags &flags, const std::string &image_filename, const mmapd &m) try :
const pmas_flags &flags, const std::string &image_filename, bool shared) try :
address_range(description.c_str(), start, length, flags, [](const char *err) { throw base_error{err}; }),
m_ptr{make_unique_mmap<unsigned char>(image_filename.c_str(), length, m.shared)},
m_ptr{make_unique_mmap<unsigned char>(length, os_mmap_flags{.shared = shared}, image_filename)},
m_host_memory{std::get<mmapd_ptr>(m_ptr).get()} {
if (!is_memory()) {
throw std::invalid_argument{"memory range must be flagged memory when initializing "s + description};
Expand All @@ -44,47 +44,4 @@ memory_address_range::memory_address_range(const std::string &description, uint6
throw std::invalid_argument{"unknown exception when initializing "s + description};
}

memory_address_range::memory_address_range(const std::string &description, uint64_t start, uint64_t length,
const pmas_flags &flags, const std::string &image_filename, const callocd & /*c*/) try :
address_range(description.c_str(), start, length, flags, [](const char *err) { throw base_error{err}; }),
m_ptr{make_unique_calloc<unsigned char>(length)},
m_host_memory{std::get<callocd_ptr>(m_ptr).get()} {
if (!is_memory()) {
throw std::invalid_argument{"memory range must be flagged memory when initializing "s + description};
}
m_dirty_page_map.resize((length / (8 * AR_PAGE_SIZE)) + 1, 0xff);
// Try to load image file, if any
if (!image_filename.empty()) {
auto fp = make_unique_fopen(image_filename.c_str(), "rb", std::nothrow_t{});
if (!fp) {
throw std::system_error{errno, std::generic_category(), "error opening image file '"s + image_filename};
}
// Get file size
if (fseek(fp.get(), 0, SEEK_END) != 0) {
throw std::system_error{errno, std::generic_category(),
"error obtaining length of image file '"s + image_filename};
}
const auto file_length = static_cast<uint64_t>(ftello(fp.get()));
if (fseek(fp.get(), 0, SEEK_SET) != 0) {
throw std::system_error{errno, std::generic_category(),
"error obtaining length of image file '"s + image_filename};
}
// Check against PMA range size
if (file_length > length) {
throw std::runtime_error{"image file '"s + image_filename + "' is too large for range"s};
}
// Read to host memory
const auto read_length = static_cast<uint64_t>(fread(m_host_memory, 1, file_length, fp.get()));
if (read_length != file_length) {
throw std::runtime_error{"error reading from image file '"s + image_filename};
}
}
} catch (base_error &b) {
throw; // already contains the description
} catch (std::exception &e) {
throw std::invalid_argument{e.what() + " when initializing "s + description};
} catch (...) {
throw std::invalid_argument{"unknown exception when initializing "s + description};
}

} // namespace cartesi
32 changes: 2 additions & 30 deletions src/memory-address-range.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
#include <variant>

#include "address-range.h"
#include "os-mmap.h"
#include "unique-c-ptr.h"

namespace cartesi {
Expand Down Expand Up @@ -48,33 +49,14 @@ class memory_address_range : public address_range {
public:
using ptr_type = std::unique_ptr<memory_address_range>;

/// \brief Mmap'd range data (shared or not).
struct mmapd {
bool shared;
};

/// \brief Constructor for mmap'd ranges.
/// \param description Description of address range for use in error messages
/// \param start Start of address range
/// \param length Length of address range
/// \param flags Range flags
/// \param image_filename Path to backing file.
/// \param m Mmap'd range data (shared or not).
memory_address_range(const std::string &description, uint64_t start, uint64_t length, const pmas_flags &flags,
const std::string &image_filename, const mmapd &m);

/// \brief Calloc'd range data (just a tag).
struct callocd {};

/// \brief Constructor for calloc'd ranges.
/// \param description Description of address range for use in error messages
/// \param start Start of address range
/// \param length Length of address range
/// \param flags Range flags
/// \param image_filename Path to backing file.
/// \param c Calloc'd range data (just a tag).
memory_address_range(const std::string &description, uint64_t start, uint64_t length, const pmas_flags &flags,
const std::string &image_filename, const callocd & /*c*/);
const std::string &image_filename = "", bool shared = false);

memory_address_range(const memory_address_range &) = delete;
memory_address_range &operator=(const memory_address_range &) = delete;
Expand Down Expand Up @@ -128,16 +110,6 @@ class memory_address_range : public address_range {
}
};

static inline auto make_callocd_memory_address_range(const std::string &description, uint64_t start, uint64_t length,
pmas_flags flags, const std::string &image_filename = {}) {
return memory_address_range{description, start, length, flags, image_filename, memory_address_range::callocd{}};
}

static inline auto make_mmapd_memory_address_range(const std::string &description, uint64_t start, uint64_t length,
pmas_flags flags, const std::string &image_filename, bool shared) {
return memory_address_range{description, start, length, flags, image_filename, memory_address_range::mmapd{shared}};
}

} // namespace cartesi

#endif
Loading

0 comments on commit 6a24b73

Please sign in to comment.