From 72a645ec1e0459186ea0cd2f506cf22f9f5b1118 Mon Sep 17 00:00:00 2001 From: Thomas Harte Date: Mon, 25 Mar 2024 15:50:59 -0400 Subject: [PATCH] Fix trans; take further crack at MEMC permissions. --- InstructionSets/ARM/Executor.hpp | 12 ++++-- Machines/Acorn/Archimedes/Archimedes.cpp | 7 ++- .../Acorn/Archimedes/MemoryController.hpp | 43 ++++++++++--------- 3 files changed, 36 insertions(+), 26 deletions(-) diff --git a/InstructionSets/ARM/Executor.hpp b/InstructionSets/ARM/Executor.hpp index a5210c2f9..95d49866c 100644 --- a/InstructionSets/ARM/Executor.hpp +++ b/InstructionSets/ARM/Executor.hpp @@ -295,7 +295,10 @@ struct Executor { return; } - constexpr bool trans = !flags.pre_index() && flags.write_back_address(); + // "... post-indexed data transfers always write back the modified base. The only use of the [write-back address] + // bit in a post-indexed data transfer is in non-user mode code, where setting the W bit forces the /TRANS pin + // to go LOW for the transfer" + const bool trans = (registers_.mode() == Mode::User) || (!flags.pre_index() && flags.write_back_address()); if constexpr (flags.operation() == SingleDataTransferFlags::Operation::STR) { const uint32_t source = transfer.source() == 15 ? @@ -430,6 +433,7 @@ struct Executor { } bool address_error = false; + const bool trans = registers_.mode() == Mode::User; // Keep track of whether all accesses succeeded in order potentially to // throw a data abort later. @@ -447,7 +451,7 @@ struct Executor { // "If the abort occurs during a store multiple instruction, ARM takes little action until // the instruction completes, whereupon it enters the data abort trap. The memory manager is // responsible for preventing erroneous writes to the memory." - accesses_succeeded &= bus.template write(address, value, registers_.mode(), false); + accesses_succeeded &= bus.template write(address, value, registers_.mode(), trans); } } else { // When ARM detects a data abort during a load multiple instruction, it modifies the operation of @@ -458,7 +462,7 @@ struct Executor { // * The base register is restored, to its modified value if write-back was requested. if(accesses_succeeded) { const uint32_t replaced = value; - accesses_succeeded &= bus.template read(address, value, registers_.mode(), false); + accesses_succeeded &= bus.template read(address, value, registers_.mode(), trans); // Update the last-modified slot if the access succeeded; otherwise // undo the last modification if there was one, and undo the base @@ -483,7 +487,7 @@ struct Executor { } else { // Implicitly: do the access anyway, but don't store the value. I think. uint32_t throwaway; - bus.template read(address, throwaway, registers_.mode(), false); + bus.template read(address, throwaway, registers_.mode(), trans); } } diff --git a/Machines/Acorn/Archimedes/Archimedes.cpp b/Machines/Acorn/Archimedes/Archimedes.cpp index ec100425e..cc00e4f20 100644 --- a/Machines/Acorn/Archimedes/Archimedes.cpp +++ b/Machines/Acorn/Archimedes/Archimedes.cpp @@ -90,10 +90,15 @@ class ConcreteMachine: template void tick_cpu_video() { - tick_cpu(); if constexpr (!(offset % video_divider)) { tick_video(); } + +#ifndef NDEBUG + // Debug mode: run CPU a lot slower. + if constexpr (offset & 15) return; +#endif + tick_cpu(); } public: diff --git a/Machines/Acorn/Archimedes/MemoryController.hpp b/Machines/Acorn/Archimedes/MemoryController.hpp index 4e75198c6..b60fcbe19 100644 --- a/Machines/Acorn/Archimedes/MemoryController.hpp +++ b/Machines/Acorn/Archimedes/MemoryController.hpp @@ -60,7 +60,7 @@ struct MemoryController { } template - bool write(uint32_t address, IntT source, InstructionSet::ARM::Mode mode, bool) { + bool write(uint32_t address, IntT source, InstructionSet::ARM::Mode mode, bool trans) { // User mode may only _write_ to logically-mapped RAM (subject to further testing below). if(mode == InstructionSet::ARM::Mode::User && address >= 0x200'0000) { return false; @@ -107,7 +107,7 @@ struct MemoryController { } break; case Zone::LogicallyMappedRAM: { - const auto item = logical_ram(address, mode); + const auto item = logical_ram(address, trans); if(!item) { return false; } @@ -142,7 +142,7 @@ struct MemoryController { } template - bool read(uint32_t address, IntT &source, InstructionSet::ARM::Mode mode, bool) { + bool read(uint32_t address, IntT &source, InstructionSet::ARM::Mode mode, bool trans) { // User mode may only read logically-maped RAM and ROM. if(mode == InstructionSet::ARM::Mode::User && address >= 0x200'0000 && address < 0x380'0000) { return false; @@ -154,7 +154,7 @@ struct MemoryController { break; case Zone::LogicallyMappedRAM: { - const auto item = logical_ram(address, mode); + const auto item = logical_ram(address, trans); if(!item) { return false; } @@ -303,7 +303,7 @@ struct MemoryController { bool map_dirty_ = true; template - IntT *logical_ram(uint32_t address, InstructionSet::ARM::Mode mode) { + IntT *logical_ram(uint32_t address, bool trans) { // Possibly TODO: this recompute-if-dirty flag is supposed to ameliorate for an expensive // mapping process. It can be eliminated when the process is improved. if(map_dirty_) { @@ -341,24 +341,25 @@ struct MemoryController { } // TODO: eliminate switch here. - // Top of my head idea: is_read, is_user and is_os_mode make three bits, so + // Top of my head idea: is_read, trans and os_mode_ make three bits, so // keep a one-byte bitmap of permitted accesses rather than the raw protection // level? - switch(mapping_[page].protection_level) { - case 0b00: break; - case 0b01: - if(!is_read && mode == InstructionSet::ARM::Mode::User) { - return nullptr; - } - break; - default: - if(mode == InstructionSet::ARM::Mode::User) { - return nullptr; - } - if(!is_read && !os_mode_) { - return nullptr; - } - break; + if(trans) { + switch(mapping_[page].protection_level) { + case 0b00: break; + case 0b01: + // Reject: writes, in user mode. + if(!is_read && !os_mode_) { + return nullptr; + } + break; + default: + // Reject: writes, and user mode. + if(!is_read || !os_mode_) { + return nullptr; + } + break; + } } return reinterpret_cast(mapping_[page].target + address);