Skip to content

Commit

Permalink
refactor: simplify device handling
Browse files Browse the repository at this point in the history
Remove context from HTIF device. This is important because it was
the only device used in reproducible code that had a context.
This meant there was some gambiarra in the driver code to check if
the context was nullptr.

Remove read_device and write_device from i_state_access. The code
is now in interpret.cpp. This reduces the amount of code we need
when implementing a new state access class.

Simplify the mock pma implementations used by different state access
classes. I think we should merge them into a single one.
  • Loading branch information
diegonehab committed Jan 8, 2025
1 parent 44f8952 commit 8af3b46
Show file tree
Hide file tree
Showing 12 changed files with 178 additions and 222 deletions.
4 changes: 2 additions & 2 deletions src/htif-factory.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ static bool htif_peek(const pma_entry &pma, const machine & /*m*/, uint64_t page
return (page_offset % PMA_PAGE_SIZE) == 0 && page_offset < pma.get_length();
}

pma_entry make_htif_pma_entry(uint64_t start, uint64_t length, htif_runtime_config *context) {
pma_entry make_htif_pma_entry(uint64_t start, uint64_t length) {
const pma_entry::flags f{
true, // R
true, // W
Expand All @@ -41,7 +41,7 @@ pma_entry make_htif_pma_entry(uint64_t start, uint64_t length, htif_runtime_conf
false, // IW
PMA_ISTART_DID::HTIF // DID
};
return make_device_pma_entry("HTIF device", start, length, htif_peek, &htif_driver, context).set_flags(f);
return make_device_pma_entry("HTIF device", start, length, htif_peek, &htif_driver).set_flags(f);
}

} // namespace cartesi
2 changes: 1 addition & 1 deletion src/htif-factory.h
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@
namespace cartesi {

/// \brief Creates a PMA entry for the HTIF device
pma_entry make_htif_pma_entry(uint64_t start, uint64_t length, htif_runtime_config *context);
pma_entry make_htif_pma_entry(uint64_t start, uint64_t length);

} // namespace cartesi

Expand Down
21 changes: 6 additions & 15 deletions src/htif.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@
#include "htif.h"
#include "i-device-state-access.h"
#include "interpret.h"
#include "machine-runtime-config.h"
#include "os.h"
#include "pma-driver.h"

Expand Down Expand Up @@ -89,17 +88,12 @@ static execute_status htif_yield(i_device_state_access *a, uint64_t cmd, uint64_
return status;
}

static execute_status htif_console(htif_runtime_config *runtime_config, i_device_state_access *a, uint64_t cmd,
uint64_t data) {
static execute_status htif_console(i_device_state_access *a, uint64_t cmd, uint64_t data) {
// If console command is enabled, perform it and acknowledge
if (cmd < 64 && (((a->read_htif_iconsole() >> cmd) & 1) != 0)) {
if (cmd == HTIF_CONSOLE_CMD_PUTCHAR) {
const uint8_t ch = data & 0xff;
// In microarchitecture runtime_config will always be nullptr,
// therefore the HTIF runtime config is actually ignored.
if ((runtime_config == nullptr) || !runtime_config->no_console_putchar) {
os_putchar(ch);
}
os_putchar(ch);
a->write_htif_fromhost(HTIF_BUILD(HTIF_DEV_CONSOLE, cmd, 0, 0));
} else if (cmd == HTIF_CONSOLE_CMD_GETCHAR) {
// In blockchain, this command will never be enabled as there is no way to input the same character
Expand All @@ -115,8 +109,7 @@ static execute_status htif_console(htif_runtime_config *runtime_config, i_device
return execute_status::success;
}

static execute_status htif_write_tohost(htif_runtime_config *runtime_config, i_device_state_access *a,
uint64_t tohost) {
static execute_status htif_write_tohost(i_device_state_access *a, uint64_t tohost) {
// Decode tohost
const uint32_t device = HTIF_DEV_FIELD(tohost);
const uint32_t cmd = HTIF_CMD_FIELD(tohost);
Expand All @@ -128,7 +121,7 @@ static execute_status htif_write_tohost(htif_runtime_config *runtime_config, i_d
case HTIF_DEV_HALT:
return htif_halt(a, cmd, data);
case HTIF_DEV_CONSOLE:
return htif_console(runtime_config, a, cmd, data);
return htif_console(a, cmd, data);
case HTIF_DEV_YIELD:
return htif_yield(a, cmd, data);
//??D Unknown HTIF devices are silently ignored
Expand All @@ -138,10 +131,8 @@ static execute_status htif_write_tohost(htif_runtime_config *runtime_config, i_d
}

/// \brief HTIF device write callback. See ::pma_write.
static execute_status htif_write(void *context, i_device_state_access *a, uint64_t offset, uint64_t val,
static execute_status htif_write(void * /*context*/, i_device_state_access *a, uint64_t offset, uint64_t val,
int log2_size) {
// NOLINTNEXTLINE(cppcoreguidelines-pro-type-reinterpret-cast)
auto *runtime_config = reinterpret_cast<htif_runtime_config *>(context);
// Our HTIF only supports 64-bit writes
if (log2_size != 3) {
return execute_status::failure;
Expand All @@ -150,7 +141,7 @@ static execute_status htif_write(void *context, i_device_state_access *a, uint64
// Only these 64-bit aligned offsets are valid
switch (offset) {
case htif_tohost_rel_addr:
return htif_write_tohost(runtime_config, a, val);
return htif_write_tohost(a, val);
case htif_fromhost_rel_addr:
a->write_htif_fromhost(val);
return execute_status::success;
Expand Down
8 changes: 0 additions & 8 deletions src/i-state-access.h
Original file line number Diff line number Diff line change
Expand Up @@ -656,14 +656,6 @@ class i_state_access { // CRTP
return derived().do_get_host_memory(pma);
}

auto read_device(PMA_ENTRY_TYPE &pma, uint64_t mcycle, uint64_t offset, uint64_t *pval, int log2_size) {
return derived().do_read_device(pma, mcycle, offset, pval, log2_size);
}

auto write_device(PMA_ENTRY_TYPE &pma, uint64_t mcycle, uint64_t offset, uint64_t pval, int log2_size) {
return derived().do_write_device(pma, mcycle, offset, pval, log2_size);
}

/// \brief Try to translate a virtual address to a host pointer through the TLB.
/// \tparam ETYPE TLB entry type.
/// \tparam T Type of word that would be read with the pointer.
Expand Down
10 changes: 7 additions & 3 deletions src/interpret.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -849,8 +849,11 @@ static NO_INLINE std::pair<bool, uint64_t> read_virtual_memory_slow(STATE_ACCESS
if (likely(pma.get_istart_IO())) {
const uint64_t offset = paddr - pma.get_start();
uint64_t val{};
device_state_access da(a, mcycle);
// If we do not know how to read, we treat this as a PMA violation
if (likely(a.read_device(pma, mcycle, offset, &val, log2_size<U>::value))) {
const bool status = pma.get_device_noexcept().get_driver()->read(pma.get_device_noexcept().get_context(),
&da, offset, &val, log2_size<U>::value);
if (likely(status)) {
*pval = static_cast<T>(val);
// device logs its own state accesses
return {true, pc};
Expand Down Expand Up @@ -924,8 +927,9 @@ static NO_INLINE std::pair<execute_status, uint64_t> write_virtual_memory_slow(S
}
if (likely(pma.get_istart_IO())) {
const uint64_t offset = paddr - pma.get_start();
auto status =
a.write_device(pma, mcycle, offset, static_cast<U>(static_cast<T>(val64)), log2_size<U>::value);
device_state_access da(a, mcycle);
auto status = pma.get_device_noexcept().get_driver()->write(pma.get_device_noexcept().get_context(), &da,
offset, static_cast<U>(static_cast<T>(val64)), log2_size<U>::value);
// If we do not know how to write, we treat this as a PMA violation
if (likely(status != execute_status::failure)) {
return {status, pc};
Expand Down
7 changes: 3 additions & 4 deletions src/machine.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -392,7 +392,7 @@ machine::machine(const machine_config &c, const machine_runtime_config &r) : m_c
register_pma_entry(make_cmio_rx_buffer_pma_entry(m_c.cmio));

// Register HTIF device
register_pma_entry(make_htif_pma_entry(PMA_HTIF_START, PMA_HTIF_LENGTH, &m_r.htif));
register_pma_entry(make_htif_pma_entry(PMA_HTIF_START, PMA_HTIF_LENGTH));

// Copy HTIF state to from config to machine
write_reg(reg::htif_tohost, m_c.htif.tohost);
Expand Down Expand Up @@ -545,6 +545,7 @@ machine::machine(const machine_config &c, const machine_runtime_config &r) : m_c
}
os_open_tty();
}
os_silence_putchar(m_r.htif.no_console_putchar);

// Initialize memory range descriptions returned by get_memory_ranges method
for (const auto *pma : m_merkle_pmas) {
Expand Down Expand Up @@ -635,11 +636,9 @@ const machine_runtime_config &machine::get_runtime_config() const {

/// \brief Changes the machine runtime config.
void machine::set_runtime_config(const machine_runtime_config &r) {
if (r.htif.no_console_putchar != m_r.htif.no_console_putchar) {
throw std::runtime_error{"cannot change htif runtime configuration"};
}
m_r = r;
m_s.soft_yield = m_r.soft_yield;
os_silence_putchar(r.htif.no_console_putchar);
}

machine_config machine::get_serialization_config() const {
Expand Down
47 changes: 36 additions & 11 deletions src/os.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -120,6 +120,7 @@ using namespace std::string_literals;
struct tty_state {
bool initialized{false};
bool resize_pending{false};
bool silence_putchar{false};
std::array<char, TTY_BUF_SIZE> buf{}; // Characters in console input buffer
intptr_t buf_pos{};
intptr_t buf_len{};
Expand All @@ -135,12 +136,23 @@ struct tty_state {
};

/// Returns pointer to the global TTY state
static tty_state *get_state() {
static tty_state *get_tty_state() {
static tty_state data;
return &data;
}
#endif // HAVE_TTY

/// \brief putchar global state
struct putchar_state {
bool silence;
};

/// Returns pointer to the global TTY state
static putchar_state *get_putchar_state() {
static putchar_state data;
return &data;
}

#ifdef HAVE_TERMIOS
static int new_ttyfd(const char *path) {
int fd{};
Expand Down Expand Up @@ -176,7 +188,7 @@ static int get_ttyfd() {
#ifdef HAVE_SIGACTION
/// \brief Signal raised whenever TTY size changes
static void os_SIGWINCH_handler(int /*sig*/) {
auto *s = get_state();
auto *s = get_tty_state();
if (!s->initialized) {
return;
}
Expand Down Expand Up @@ -225,7 +237,7 @@ bool os_update_tty_size([[maybe_unused]] tty_state *s) {

void os_open_tty() {
#ifdef HAVE_TTY
auto *s = get_state();
auto *s = get_tty_state();
if (s->initialized) {
// Already initialized
return;
Expand Down Expand Up @@ -332,15 +344,15 @@ void os_open_tty() {
void os_close_tty() {
#ifdef HAVE_TTY
#ifdef HAVE_TERMIOS
auto *s = get_state();
auto *s = get_tty_state();
if (s->ttyfd >= 0) { // Restore old mode
tcsetattr(s->ttyfd, TCSANOW, &s->oldtty);
close(s->ttyfd);
s->ttyfd = 1;
}

#elif defined(_WIN32)
auto *s = get_state();
auto *s = get_tty_state();
if (s->hStdin) {
SetConsoleMode(s->hStdin, s->dwOldStdinMode);
s->hStdin = NULL;
Expand All @@ -351,7 +363,7 @@ void os_close_tty() {
}

void os_get_tty_size(uint16_t *pwidth, uint16_t *pheight) {
auto *s = get_state();
auto *s = get_tty_state();
if (!s->initialized) {
// fallback values
*pwidth = TTY_DEFAULT_COLS;
Expand All @@ -370,7 +382,7 @@ void os_get_tty_size(uint16_t *pwidth, uint16_t *pheight) {

void os_prepare_tty_select([[maybe_unused]] select_fd_sets *fds) {
#ifdef HAVE_TTY
auto *s = get_state();
auto *s = get_tty_state();
// Ignore if TTY is not initialized or stdin was closed
if (!s->initialized) {
return;
Expand All @@ -387,7 +399,7 @@ void os_prepare_tty_select([[maybe_unused]] select_fd_sets *fds) {
}

bool os_poll_selected_tty([[maybe_unused]] int select_ret, [[maybe_unused]] select_fd_sets *fds) {
auto *s = get_state();
auto *s = get_tty_state();
if (!s->initialized) { // We can't poll when TTY is not initialized
return false;
}
Expand Down Expand Up @@ -444,7 +456,7 @@ bool os_poll_selected_tty([[maybe_unused]] int select_ret, [[maybe_unused]] sele

bool os_poll_tty(uint64_t timeout_us) {
#ifdef _WIN32
auto *s = get_state();
auto *s = get_tty_state();
if (!s->initialized) { // We can't poll when TTY is not initialized
return false;
}
Expand All @@ -468,7 +480,7 @@ bool os_poll_tty(uint64_t timeout_us) {

int os_getchar() {
#ifdef HAVE_TTY
auto *s = get_state();
auto *s = get_tty_state();
if (!s->initialized) {
return -1;
}
Expand Down Expand Up @@ -501,9 +513,18 @@ static void fputc_with_line_buffering(uint8_t ch) {
}
}

void os_silence_putchar(bool yes) {
auto *ps = get_putchar_state();
ps->silence = yes;
}

void os_putchar(uint8_t ch) {
auto *ps = get_putchar_state();
if (ps->silence) {
return;
}
#ifdef HAVE_TTY
auto *s = get_state();
auto *s = get_tty_state();
if (!s->initialized) {
// Write through fputc(), so we can take advantage of buffering.
fputc_with_line_buffering(ch);
Expand All @@ -520,6 +541,10 @@ void os_putchar(uint8_t ch) {
}

void os_putchars(const uint8_t *data, size_t len) {
auto *ps = get_putchar_state();
if (ps->silence) {
return;
}
for (size_t i = 0; i < len; ++i) {
os_putchar(data[i]);
}
Expand Down
4 changes: 4 additions & 0 deletions src/os.h
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,10 @@ void os_putchar(uint8_t ch);
/// \param len Length of buffer.
void os_putchars(const uint8_t *data, size_t len);

/// \brief Silences putchar (and putchars) output
/// \param yes If true, putchar is silenced
void os_silence_putchar(bool yes);

/// \brief Creates a new directory
int os_mkdir(const char *path, int mode);

Expand Down
12 changes: 0 additions & 12 deletions src/record-step-state-access.h
Original file line number Diff line number Diff line change
Expand Up @@ -630,18 +630,6 @@ class record_step_state_access : public i_state_access<record_step_state_access,
return m_m.get_state().pmas[static_cast<int>(index)];
}

bool do_read_device(pma_entry &pma, uint64_t mcycle, uint64_t offset, uint64_t *pval, int log2_size) {
device_state_access da(*this, mcycle);
return pma.get_device_noexcept().get_driver()->read(pma.get_device_noexcept().get_context(), &da, offset, pval,
log2_size);
}

execute_status do_write_device(pma_entry &pma, uint64_t mcycle, uint64_t offset, uint64_t val, int log2_size) {
device_state_access da(*this, mcycle);
return pma.get_device_noexcept().get_driver()->write(pma.get_device_noexcept().get_context(), &da, offset, val,
log2_size);
}

template <TLB_entry_type ETYPE, typename T>
bool do_translate_vaddr_via_tlb(uint64_t vaddr, unsigned char **phptr) {
const uint64_t eidx = tlb_get_entry_index(vaddr);
Expand Down
Loading

0 comments on commit 8af3b46

Please sign in to comment.