From 05f21fa25009c61f000bc8f5672e776ecc06405f Mon Sep 17 00:00:00 2001 From: Andrew Waterman Date: Thu, 2 Mar 2023 14:32:54 -0800 Subject: [PATCH 1/3] Improve plic_context_t initialization style --- riscv/devices.h | 17 ++++++++--------- 1 file changed, 8 insertions(+), 9 deletions(-) diff --git a/riscv/devices.h b/riscv/devices.h index 2a9c0059..82ce0fe6 100644 --- a/riscv/devices.h +++ b/riscv/devices.h @@ -82,18 +82,17 @@ class clint_t : public abstract_device_t { struct plic_context_t { plic_context_t(processor_t* proc, bool mmode) - : proc(proc), mmode(mmode), priority_threshold(0), enable{}, pending{}, - pending_priority{}, claimed{} + : proc(proc), mmode(mmode) {} - processor_t *proc; - bool mmode; + processor_t *proc; + bool mmode; - uint8_t priority_threshold; - uint32_t enable[PLIC_MAX_DEVICES/32]; - uint32_t pending[PLIC_MAX_DEVICES/32]; - uint8_t pending_priority[PLIC_MAX_DEVICES]; - uint32_t claimed[PLIC_MAX_DEVICES/32]; + uint8_t priority_threshold {}; + uint32_t enable[PLIC_MAX_DEVICES/32] {}; + uint32_t pending[PLIC_MAX_DEVICES/32] {}; + uint8_t pending_priority[PLIC_MAX_DEVICES] {}; + uint32_t claimed[PLIC_MAX_DEVICES/32] {}; }; class plic_t : public abstract_device_t, public abstract_interrupt_controller_t { From 360e55535da0d3a0f8123bbe9ca4b2947950e8ea Mon Sep 17 00:00:00 2001 From: Scott Johnson Date: Fri, 3 Mar 2023 06:06:28 -0800 Subject: [PATCH 2/3] Fix misaligned accesses to clint's msip regs Misaligned MMIO is unspecified but this is simple enough. --- riscv/clint.cc | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/riscv/clint.cc b/riscv/clint.cc index cf9e923f..33122e7f 100644 --- a/riscv/clint.cc +++ b/riscv/clint.cc @@ -71,12 +71,14 @@ bool clint_t::store(reg_t addr, size_t len, const uint8_t* bytes) return store(addr, 4, bytes) && store(addr + 4, 4, bytes + 4); } - msip_t msip = 0; - write_little_endian_reg(&msip, addr, len, bytes); + if (addr % sizeof(msip_t) == 0) { // ignore in-between bytes + msip_t msip = 0; + write_little_endian_reg(&msip, addr, len, bytes); - const auto hart_id = (addr - MSIP_BASE) / sizeof(msip_t); - if (sim->get_harts().count(hart_id)) - sim->get_harts().at(hart_id)->state.mip->backdoor_write_with_mask(MIP_MSIP, msip & 1 ? MIP_MSIP : 0); + const auto hart_id = (addr - MSIP_BASE) / sizeof(msip_t); + if (sim->get_harts().count(hart_id)) + sim->get_harts().at(hart_id)->state.mip->backdoor_write_with_mask(MIP_MSIP, msip & 1 ? MIP_MSIP : 0); + } } else if (addr >= MTIMECMP_BASE && addr < MTIME_BASE) { const auto hart_id = (addr - MTIMECMP_BASE) / sizeof(mtimecmp_t); if (sim->get_harts().count(hart_id)) From 1951f80361e7cb82a8baa975183d036e8a944bd2 Mon Sep 17 00:00:00 2001 From: Scott Johnson Date: Fri, 3 Mar 2023 05:22:52 -0800 Subject: [PATCH 3/3] Don't issue misaligned or non-power-of-2 MMIO accesses @aswaterman explains: Rather than requiring each MMIO device to support arbitrary sizes and alignments, decompose MMIO misaligned loads and stores in such a way as to guarantee their constituent parts are always aligned. (Specifically, they now always decompose to a sequence of one-byte accesses.) This is not a semantic change for main-memory accesses, but it is a semantic change for I/O devices. It makes them more realistic, in that most bus standards don't support non-power-of-2-sized accesses. --- riscv/mmu.cc | 31 ++++++++++++++++++++++++------- riscv/mmu.h | 1 + 2 files changed, 25 insertions(+), 7 deletions(-) diff --git a/riscv/mmu.cc b/riscv/mmu.cc index fc80dfa1..8ce7a3a7 100644 --- a/riscv/mmu.cc +++ b/riscv/mmu.cc @@ -147,18 +147,35 @@ bool mmu_t::mmio_fetch(reg_t paddr, size_t len, uint8_t* bytes) bool mmu_t::mmio_load(reg_t paddr, size_t len, uint8_t* bytes) { - if (!mmio_ok(paddr, LOAD)) - return false; - - return sim->mmio_load(paddr, len, bytes); + return mmio(paddr, len, bytes, LOAD); } bool mmu_t::mmio_store(reg_t paddr, size_t len, const uint8_t* bytes) { - if (!mmio_ok(paddr, STORE)) - return false; + return mmio(paddr, len, const_cast(bytes), STORE); +} + +bool mmu_t::mmio(reg_t paddr, size_t len, uint8_t* bytes, access_type type) +{ + bool power_of_2 = (len & (len - 1)) == 0; + bool naturally_aligned = (paddr & (len - 1)) == 0; + + if (power_of_2 && naturally_aligned) { + if (!mmio_ok(paddr, type)) + return false; + + if (type == STORE) + return sim->mmio_store(paddr, len, bytes); + else + return sim->mmio_load(paddr, len, bytes); + } - return sim->mmio_store(paddr, len, bytes); + for (size_t i = 0; i < len; i++) { + if (!mmio(paddr + i, 1, bytes + i, type)) + return false; + } + + return true; } void mmu_t::check_triggers(triggers::operation_t operation, reg_t address, std::optional data) diff --git a/riscv/mmu.h b/riscv/mmu.h index 6cf696bb..56789e02 100644 --- a/riscv/mmu.h +++ b/riscv/mmu.h @@ -339,6 +339,7 @@ private: bool mmio_fetch(reg_t paddr, size_t len, uint8_t* bytes); bool mmio_load(reg_t paddr, size_t len, uint8_t* bytes); bool mmio_store(reg_t paddr, size_t len, const uint8_t* bytes); + bool mmio(reg_t paddr, size_t len, uint8_t* bytes, access_type type); bool mmio_ok(reg_t paddr, access_type type); void check_triggers(triggers::operation_t operation, reg_t address, std::optional data = std::nullopt); reg_t translate(reg_t addr, reg_t len, access_type type, uint32_t xlate_flags);