From 413e7b7de1e56130f1427bd68607d32dba3bf2ac Mon Sep 17 00:00:00 2001 From: Thomas Harte Date: Tue, 7 Nov 2023 14:03:20 -0500 Subject: [PATCH] Switch `Memory` to using accessors. --- OSBindings/Mac/Clock SignalTests/8088Tests.mm | 88 ++++++++----------- 1 file changed, 38 insertions(+), 50 deletions(-) diff --git a/OSBindings/Mac/Clock SignalTests/8088Tests.mm b/OSBindings/Mac/Clock SignalTests/8088Tests.mm index a28c37670..f698860a3 100644 --- a/OSBindings/Mac/Clock SignalTests/8088Tests.mm +++ b/OSBindings/Mac/Clock SignalTests/8088Tests.mm @@ -98,16 +98,6 @@ struct Memory { public: using AccessType = InstructionSet::x86::AccessType; - template struct ReturnType; - - // Reads: return a value directly. - template struct ReturnType { using type = IntT; }; - template struct ReturnType { using type = IntT; }; - - // Writes: return a reference. - template struct ReturnType { using type = IntT &; }; - template struct ReturnType { using type = IntT &; }; - // Constructor. Memory(Registers ®isters) : registers_(registers) { memory.resize(1024*1024); @@ -163,30 +153,13 @@ struct Memory { // Accesses an address based on segment:offset. template - typename ReturnType::type &access(InstructionSet::x86::Source segment, uint16_t offset) { - if constexpr (std::is_same_v) { - // If this is a 16-bit access that runs past the end of the segment, it'll wrap back - // to the start. So the 16-bit value will need to be a local cache. - if(offset == 0xffff) { - write_back_address_[0] = address(segment, offset); - write_back_address_[1] = (write_back_address_[0] - 65535) & 0xf'ffff; - write_back_value_ = memory[write_back_address_[0]] | (memory[write_back_address_[1]] << 8); - return write_back_value_; - } - } - auto &value = access(segment, offset, Tag::Accessed); - - // If the CPU has indicated a write, it should be safe to fuzz the value now. - if(type == AccessType::Write) { - value = IntT(~0); - } - - return value; + typename InstructionSet::x86::Accessor::type access(InstructionSet::x86::Source segment, uint16_t offset) { + return access(segment, offset, Tag::Accessed); } // Accesses an address based on physical location. template - typename ReturnType::type &access(uint32_t address) { + typename InstructionSet::x86::Accessor::type access(uint32_t address) { return access(address, Tag::Accessed); } @@ -283,42 +256,57 @@ struct Memory { // Entry point used by the flow controller so that it can mark up locations at which the flags were written, // so that defined-flag-only masks can be applied while verifying RAM contents. template - typename ReturnType::type &access(InstructionSet::x86::Source segment, uint16_t offset, Tag tag) { + typename InstructionSet::x86::Accessor::type access(InstructionSet::x86::Source segment, uint16_t offset, Tag tag) { const uint32_t physical_address = address(segment, offset); + + if constexpr (std::is_same_v) { + // If this is a 16-bit access that runs past the end of the segment, it'll wrap back + // to the start. So the 16-bit value will need to be a local cache. + if(offset == 0xffff) { + return split_word(physical_address, address(segment, 0), tag); + } + } + return access(physical_address, tag); } // An additional entry point for the flow controller; on the original 8086 interrupt vectors aren't relative // to a selector, they're just at an absolute location. template - typename ReturnType::type &access(uint32_t address, Tag tag) { + typename InstructionSet::x86::Accessor::type access(uint32_t address, Tag tag) { if constexpr (type == AccessType::PreauthorisedRead) { if(!test_preauthorisation(address)) { printf("Non preauthorised access\n"); } } - // Check for address wraparound - if(address > 0x10'0000 - sizeof(IntT)) { - if constexpr (std::is_same_v) { - address &= 0xf'ffff; - } else { - address &= 0xf'ffff; - if(address == 0xf'ffff) { - // This is a 16-bit access comprising the final byte in memory and the first. - write_back_address_[0] = address; - write_back_address_[1] = 0; - write_back_value_ = memory[write_back_address_[0]] | (memory[write_back_address_[1]] << 8); - return write_back_value_; - } - } + for(size_t c = 0; c < sizeof(IntT); c++) { + tags[(address + c) & 0xf'ffff] = tag; } - if(tags.find(address) == tags.end()) { - printf("Access to unexpected RAM address\n"); + // Dispense with the single-byte case trivially. + if constexpr (std::is_same_v) { + return memory[address]; + } else if(address != 0xf'ffff) { + return *reinterpret_cast(&memory[address]); + } else { + return split_word(address, 0, tag); + } + } + + template + typename InstructionSet::x86::Accessor::type + split_word(uint32_t low_address, uint32_t high_address, Tag tag) { + if constexpr (is_writeable(type)) { + write_back_address_[0] = low_address; + write_back_address_[1] = high_address; + tags[low_address] = tag; + tags[high_address] = tag; + write_back_value_ = memory[write_back_address_[0]] | (memory[write_back_address_[1]] << 8); + return write_back_value_; + } else { + return memory[low_address] | (memory[high_address] << 8); } - tags[address] = tag; - return *reinterpret_cast(&memory[address]); } static constexpr uint32_t NoWriteBack = 0; // A low byte address of 0 can't require write-back.