From 5739862dbbd616ee5d98194ede74663061742828 Mon Sep 17 00:00:00 2001 From: Thomas Harte Date: Fri, 3 Nov 2023 15:36:30 -0400 Subject: [PATCH 01/12] Add specific entryway for preauthorised writes. --- .../Implementation/PerformImplementation.hpp | 10 +++- InstructionSets/x86/Perform.hpp | 1 - OSBindings/Mac/Clock SignalTests/8088Tests.mm | 56 ++++++++++++++++--- 3 files changed, 55 insertions(+), 12 deletions(-) diff --git a/InstructionSets/x86/Implementation/PerformImplementation.hpp b/InstructionSets/x86/Implementation/PerformImplementation.hpp index 0971685c4..3bf4b7615 100644 --- a/InstructionSets/x86/Implementation/PerformImplementation.hpp +++ b/InstructionSets/x86/Implementation/PerformImplementation.hpp @@ -194,9 +194,13 @@ namespace Primitive { template void push(IntT &value, ContextT &context) { context.registers.sp_ -= sizeof(IntT); - context.memory.template access( - Source::SS, - context.registers.sp_) = value; + if constexpr (preauthorised) { + context.memory.template preauthorised_write(Source::SS, context.registers.sp_, value); + } else { + context.memory.template access( + Source::SS, + context.registers.sp_) = value; + } context.memory.template write_back(); } diff --git a/InstructionSets/x86/Perform.hpp b/InstructionSets/x86/Perform.hpp index 2b61a100b..cbeacab3b 100644 --- a/InstructionSets/x86/Perform.hpp +++ b/InstructionSets/x86/Perform.hpp @@ -33,7 +33,6 @@ enum class AccessType { /// all necessary stack space is available ahead of pushing anything, though each individual push will then result in /// a further `Preauthorised` access. PreauthorisedRead, - PreauthorisedWrite, }; template < diff --git a/OSBindings/Mac/Clock SignalTests/8088Tests.mm b/OSBindings/Mac/Clock SignalTests/8088Tests.mm index 2e61e651e..831223d1c 100644 --- a/OSBindings/Mac/Clock SignalTests/8088Tests.mm +++ b/OSBindings/Mac/Clock SignalTests/8088Tests.mm @@ -106,7 +106,6 @@ struct Memory { // Writes: return a reference. template struct ReturnType { using type = IntT &; }; template struct ReturnType { using type = IntT &; }; - template struct ReturnType { using type = IntT &; }; // Constructor. Memory(Registers ®isters) : registers_(registers) { @@ -155,22 +154,24 @@ struct Memory { } } + // // Access call-ins. + // // Accesses an address based on segment:offset. template - typename ReturnType::type &access(InstructionSet::x86::Source segment, uint32_t address) { + 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(address == 0xffff) { - write_back_address_[0] = (segment_base(segment) + address) & 0xf'ffff; + 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, address, Tag::Accessed); + 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) { @@ -197,6 +198,41 @@ struct Memory { } } + // + // Direct write. + // + template + void preauthorised_write(InstructionSet::x86::Source segment, uint16_t offset, IntT value) { + if(!test_preauthorisation(address(segment, offset))) { + printf("Non-preauthorised access\n"); + } + + // Bytes can be written without further ado. + if constexpr (std::is_same_v) { + memory[address(segment, offset) & 0xf'ffff] = value; + return; + } + + // Words that straddle the segment end must be split in two. + if(offset == 0xffff) { + memory[address(segment, offset) & 0xf'ffff] = value & 0xff; + memory[address(segment, 0x0000) & 0xf'ffff] = value >> 8; + return; + } + + const uint32_t target = address(segment, offset) & 0xf'ffff; + + // Words that straddle the end of physical RAM must also be split in two. + if(target == 0xf'ffff) { + memory[0xf'ffff] = value & 0xff; + memory[0x0'0000] = value >> 8; + return; + } + + // It's safe just to write then. + *reinterpret_cast(&memory[target]) = value; + } + private: enum class Tag { Seeded, @@ -236,12 +272,16 @@ struct Memory { return physical_address << 4; } + uint32_t address(InstructionSet::x86::Source segment, uint16_t offset) { + return (segment_base(segment) + offset) & 0xf'ffff; + } + // 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 address, Tag tag) { - const uint32_t physical_address = (segment_base(segment) + address) & 0xf'ffff; + typename ReturnType::type &access(InstructionSet::x86::Source segment, uint16_t offset, Tag tag) { + const uint32_t physical_address = address(segment, offset); return access(physical_address, tag); } @@ -249,7 +289,7 @@ struct Memory { // to a selector, they're just at an absolute location. template typename ReturnType::type &access(uint32_t address, Tag tag) { - if constexpr (type == AccessType::PreauthorisedRead || type == AccessType::PreauthorisedWrite) { + if constexpr (type == AccessType::PreauthorisedRead) { if(!test_preauthorisation(address)) { printf("Non preauthorised access\n"); } From f96c33102a18a66d10cfa927b5341e3d4f67219f Mon Sep 17 00:00:00 2001 From: Thomas Harte Date: Sat, 4 Nov 2023 22:22:50 -0400 Subject: [PATCH 02/12] Add documentation. --- .../Implementation/PerformImplementation.hpp | 43 +++++++++++++------ OSBindings/Mac/Clock SignalTests/8088Tests.mm | 2 + 2 files changed, 33 insertions(+), 12 deletions(-) diff --git a/InstructionSets/x86/Implementation/PerformImplementation.hpp b/InstructionSets/x86/Implementation/PerformImplementation.hpp index 3bf4b7615..ba9d89448 100644 --- a/InstructionSets/x86/Implementation/PerformImplementation.hpp +++ b/InstructionSets/x86/Implementation/PerformImplementation.hpp @@ -28,13 +28,14 @@ IntT *resolve( IntT *immediate = nullptr ); +/// Calculates the absolute address for @c pointer given the registers and memory provided in @c context and taking any +/// referenced offset from @c instruction. template uint32_t address( InstructionT &instruction, DataPointer pointer, ContextT &context ) { - // TODO: non-word indexes and bases. if constexpr (source == Source::DirectAddress) { return instruction.offset(); } @@ -53,6 +54,9 @@ uint32_t address( return address + *resolve(instruction, pointer.base(), pointer, context); } +/// @returns a pointer to the contents of the register identified by the combination of @c IntT and @c Source if any; +/// @c nullptr otherwise. @c access is currently unused but is intended to provide the hook upon which updates to +/// segment registers can be tracked for protected modes. template IntT *register_(ContextT &context) { static constexpr bool supports_dword = is_32bit(ContextT::model); @@ -103,10 +107,21 @@ IntT *register_(ContextT &context) { else if constexpr (std::is_same_v) { return &context.registers.bh(); } else { return nullptr; } + // Segment registers are always 16-bit. + case Source::ES: if constexpr (std::is_same_v) return &context.registers.es(); else return nullptr; + case Source::CS: if constexpr (std::is_same_v) return &context.registers.cs(); else return nullptr; + case Source::SS: if constexpr (std::is_same_v) return &context.registers.ss(); else return nullptr; + case Source::DS: if constexpr (std::is_same_v) return &context.registers.ds(); else return nullptr; + + // 16-bit models don't have FS and GS. + case Source::FS: if constexpr (is_32bit(ContextT::model) && std::is_same_v) return &context.registers.fs(); else return nullptr; + case Source::GS: if constexpr (is_32bit(ContextT::model) && std::is_same_v) return &context.registers.gs(); else return nullptr; + default: return nullptr; } } +///Obtains the address described by @c pointer from @c instruction given the registers and memory as described by @c context. template uint32_t address( InstructionT &instruction, @@ -131,6 +146,13 @@ uint32_t address( } } +/// Obtain a pointer to the value desribed by @c source, which is one of those named by @c pointer, using @c instruction and @c context +/// for offsets, registers and memory contents. +/// +/// If @c source is Source::None then @c none is returned. +/// +/// If @c source is Source::Immediate then the appropriate portion of @c instrucion's operand +/// is copied to @c *immediate and @c immediate is returned. template IntT *resolve( InstructionT &instruction, @@ -146,6 +168,7 @@ IntT *resolve( // * otherwise return the appropriate value. uint32_t target_address; switch(source) { + // Defer all register accesses to the register-specific lookup. case Source::eAX: return register_(context); case Source::eCX: return register_(context); case Source::eDX: return register_(context); @@ -154,23 +177,19 @@ IntT *resolve( case Source::eBPorCH: return register_(context); case Source::eSIorDH: return register_(context); case Source::eDIorBH: return register_(context); + case Source::ES: return register_(context); + case Source::CS: return register_(context); + case Source::SS: return register_(context); + case Source::DS: return register_(context); + case Source::FS: return register_(context); + case Source::GS: return register_(context); - // Segment registers are always 16-bit. - case Source::ES: if constexpr (std::is_same_v) return &context.registers.es(); else return nullptr; - case Source::CS: if constexpr (std::is_same_v) return &context.registers.cs(); else return nullptr; - case Source::SS: if constexpr (std::is_same_v) return &context.registers.ss(); else return nullptr; - case Source::DS: if constexpr (std::is_same_v) return &context.registers.ds(); else return nullptr; - - // 16-bit models don't have FS and GS. - case Source::FS: if constexpr (is_32bit(ContextT::model) && std::is_same_v) return &context.registers.fs(); else return nullptr; - case Source::GS: if constexpr (is_32bit(ContextT::model) && std::is_same_v) return &context.registers.gs(); else return nullptr; + case Source::None: return none; case Source::Immediate: *immediate = instruction.operand(); return immediate; - case Source::None: return none; - case Source::Indirect: target_address = address(instruction, pointer, context); break; diff --git a/OSBindings/Mac/Clock SignalTests/8088Tests.mm b/OSBindings/Mac/Clock SignalTests/8088Tests.mm index 831223d1c..90d4dfbaa 100644 --- a/OSBindings/Mac/Clock SignalTests/8088Tests.mm +++ b/OSBindings/Mac/Clock SignalTests/8088Tests.mm @@ -126,7 +126,9 @@ struct Memory { tags[address] = Tag::AccessExpected; } + // // Preauthorisation call-ins. + // void preauthorise_stack_write(uint32_t length) { uint16_t sp = registers_.sp_; while(length--) { From 009915f4de1e6e6952233fa961575eae04c1cce9 Mon Sep 17 00:00:00 2001 From: Thomas Harte Date: Sun, 5 Nov 2023 21:42:22 -0500 Subject: [PATCH 03/12] Start promotion of ReturnType. --- InstructionSets/x86/AccessType.hpp | 56 +++++ .../Implementation/PerformImplementation.hpp | 190 +--------------- .../x86/Implementation/Resolver.hpp | 207 ++++++++++++++++++ InstructionSets/x86/Perform.hpp | 20 -- .../Clock Signal.xcodeproj/project.pbxproj | 6 +- OSBindings/Mac/Clock SignalTests/8088Tests.mm | 15 +- 6 files changed, 273 insertions(+), 221 deletions(-) create mode 100644 InstructionSets/x86/AccessType.hpp create mode 100644 InstructionSets/x86/Implementation/Resolver.hpp diff --git a/InstructionSets/x86/AccessType.hpp b/InstructionSets/x86/AccessType.hpp new file mode 100644 index 000000000..544bb2e02 --- /dev/null +++ b/InstructionSets/x86/AccessType.hpp @@ -0,0 +1,56 @@ +// +// AccessType.hpp +// Clock Signal +// +// Created by Thomas Harte on 05/11/2023. +// Copyright © 2023 Thomas Harte. All rights reserved. +// + +#ifndef AccessType_h +#define AccessType_h + +namespace InstructionSet::x86 { + +/// Explains the type of access that `perform` intends to perform; is provided as a template parameter to whatever +/// the caller supplies as `MemoryT` and `RegistersT` when obtaining a reference to whatever the processor +/// intends to reference. +/// +/// `perform` guarantees to validate all accesses before modifying any state, giving the caller opportunity to generate +/// any exceptions that might be applicable. +enum class AccessType { + /// The requested value will be read from. + Read, + /// The requested value will be written to. + Write, + /// The requested value will be read from and then written to. + ReadModifyWrite, + /// The requested value has already been authorised for whatever form of access is now intended, so there's no + /// need further to inspect. This is done e.g. by operations that will push multiple values to the stack to verify that + /// all necessary stack space is available ahead of pushing anything, though each individual push will then result in + /// a further `Preauthorised` access. + PreauthorisedRead, +}; + +template struct ReturnType; + +// Reads: return a value directly. +template struct ReturnType { using type = IntT; }; +template struct ReturnType { using type = IntT; }; + +// Writes: return a custom type that can be written but not read. +template +class Writeable { + public: + Writeable(IntT &target) : target_(target) {} + void operator=(IntT value) { target_ = value; } + private: + IntT &target_; +}; +template struct ReturnType { using type = Writeable; }; + +// Read-modify-writes: return a reference. +template struct ReturnType { using type = IntT &; }; + +} + +#endif /* AccessType_h */ diff --git a/InstructionSets/x86/Implementation/PerformImplementation.hpp b/InstructionSets/x86/Implementation/PerformImplementation.hpp index ba9d89448..d932357a3 100644 --- a/InstructionSets/x86/Implementation/PerformImplementation.hpp +++ b/InstructionSets/x86/Implementation/PerformImplementation.hpp @@ -13,199 +13,13 @@ #include "../../../Numeric/Carry.hpp" #include "../../../Numeric/RegisterSizes.hpp" #include "../Interrupts.hpp" +#include "../AccessType.hpp" +#include "Resolver.hpp" #include namespace InstructionSet::x86 { -template -IntT *resolve( - InstructionT &instruction, - Source source, - DataPointer pointer, - ContextT &context, - IntT *none = nullptr, - IntT *immediate = nullptr -); - -/// Calculates the absolute address for @c pointer given the registers and memory provided in @c context and taking any -/// referenced offset from @c instruction. -template -uint32_t address( - InstructionT &instruction, - DataPointer pointer, - ContextT &context -) { - if constexpr (source == Source::DirectAddress) { - return instruction.offset(); - } - - uint32_t address; - uint16_t zero = 0; - address = *resolve(instruction, pointer.index(), pointer, context, &zero); - if constexpr (is_32bit(ContextT::model)) { - address <<= pointer.scale(); - } - address += instruction.offset(); - - if constexpr (source == Source::IndirectNoBase) { - return address; - } - return address + *resolve(instruction, pointer.base(), pointer, context); -} - -/// @returns a pointer to the contents of the register identified by the combination of @c IntT and @c Source if any; -/// @c nullptr otherwise. @c access is currently unused but is intended to provide the hook upon which updates to -/// segment registers can be tracked for protected modes. -template -IntT *register_(ContextT &context) { - static constexpr bool supports_dword = is_32bit(ContextT::model); - - switch(source) { - case Source::eAX: - // Slightly contorted if chain here and below: - // - // (i) does the `constexpr` version of a `switch`; and - // (i) ensures .eax() etc aren't called on @c registers for 16-bit processors, so they need not implement 32-bit storage. - if constexpr (supports_dword && std::is_same_v) { return &context.registers.eax(); } - else if constexpr (std::is_same_v) { return &context.registers.ax(); } - else if constexpr (std::is_same_v) { return &context.registers.al(); } - else { return nullptr; } - case Source::eCX: - if constexpr (supports_dword && std::is_same_v) { return &context.registers.ecx(); } - else if constexpr (std::is_same_v) { return &context.registers.cx(); } - else if constexpr (std::is_same_v) { return &context.registers.cl(); } - else { return nullptr; } - case Source::eDX: - if constexpr (supports_dword && std::is_same_v) { return &context.registers.edx(); } - else if constexpr (std::is_same_v) { return &context.registers.dx(); } - else if constexpr (std::is_same_v) { return &context.registers.dl(); } - else if constexpr (std::is_same_v) { return nullptr; } - case Source::eBX: - if constexpr (supports_dword && std::is_same_v) { return &context.registers.ebx(); } - else if constexpr (std::is_same_v) { return &context.registers.bx(); } - else if constexpr (std::is_same_v) { return &context.registers.bl(); } - else if constexpr (std::is_same_v) { return nullptr; } - case Source::eSPorAH: - if constexpr (supports_dword && std::is_same_v) { return &context.registers.esp(); } - else if constexpr (std::is_same_v) { return &context.registers.sp(); } - else if constexpr (std::is_same_v) { return &context.registers.ah(); } - else { return nullptr; } - case Source::eBPorCH: - if constexpr (supports_dword && std::is_same_v) { return &context.registers.ebp(); } - else if constexpr (std::is_same_v) { return &context.registers.bp(); } - else if constexpr (std::is_same_v) { return &context.registers.ch(); } - else { return nullptr; } - case Source::eSIorDH: - if constexpr (supports_dword && std::is_same_v) { return &context.registers.esi(); } - else if constexpr (std::is_same_v) { return &context.registers.si(); } - else if constexpr (std::is_same_v) { return &context.registers.dh(); } - else { return nullptr; } - case Source::eDIorBH: - if constexpr (supports_dword && std::is_same_v) { return &context.registers.edi(); } - else if constexpr (std::is_same_v) { return &context.registers.di(); } - else if constexpr (std::is_same_v) { return &context.registers.bh(); } - else { return nullptr; } - - // Segment registers are always 16-bit. - case Source::ES: if constexpr (std::is_same_v) return &context.registers.es(); else return nullptr; - case Source::CS: if constexpr (std::is_same_v) return &context.registers.cs(); else return nullptr; - case Source::SS: if constexpr (std::is_same_v) return &context.registers.ss(); else return nullptr; - case Source::DS: if constexpr (std::is_same_v) return &context.registers.ds(); else return nullptr; - - // 16-bit models don't have FS and GS. - case Source::FS: if constexpr (is_32bit(ContextT::model) && std::is_same_v) return &context.registers.fs(); else return nullptr; - case Source::GS: if constexpr (is_32bit(ContextT::model) && std::is_same_v) return &context.registers.gs(); else return nullptr; - - default: return nullptr; - } -} - -///Obtains the address described by @c pointer from @c instruction given the registers and memory as described by @c context. -template -uint32_t address( - InstructionT &instruction, - DataPointer pointer, - ContextT &context -) { - // TODO: at least on the 8086 this isn't how register 'addresses' are resolved; instead whatever was the last computed address - // remains in the address register and is returned. Find out what other x86s do and make a decision. - switch(pointer.source()) { - default: return 0; - case Source::eAX: return *register_(context); - case Source::eCX: return *register_(context); - case Source::eDX: return *register_(context); - case Source::eBX: return *register_(context); - case Source::eSPorAH: return *register_(context); - case Source::eBPorCH: return *register_(context); - case Source::eSIorDH: return *register_(context); - case Source::eDIorBH: return *register_(context); - case Source::Indirect: return address(instruction, pointer, context); - case Source::IndirectNoBase: return address(instruction, pointer, context); - case Source::DirectAddress: return address(instruction, pointer, context); - } -} - -/// Obtain a pointer to the value desribed by @c source, which is one of those named by @c pointer, using @c instruction and @c context -/// for offsets, registers and memory contents. -/// -/// If @c source is Source::None then @c none is returned. -/// -/// If @c source is Source::Immediate then the appropriate portion of @c instrucion's operand -/// is copied to @c *immediate and @c immediate is returned. -template -IntT *resolve( - InstructionT &instruction, - Source source, - DataPointer pointer, - ContextT &context, - IntT *none, - IntT *immediate -) { - // Rules: - // - // * if this is a memory access, set target_address and break; - // * otherwise return the appropriate value. - uint32_t target_address; - switch(source) { - // Defer all register accesses to the register-specific lookup. - case Source::eAX: return register_(context); - case Source::eCX: return register_(context); - case Source::eDX: return register_(context); - case Source::eBX: return register_(context); - case Source::eSPorAH: return register_(context); - case Source::eBPorCH: return register_(context); - case Source::eSIorDH: return register_(context); - case Source::eDIorBH: return register_(context); - case Source::ES: return register_(context); - case Source::CS: return register_(context); - case Source::SS: return register_(context); - case Source::DS: return register_(context); - case Source::FS: return register_(context); - case Source::GS: return register_(context); - - case Source::None: return none; - - case Source::Immediate: - *immediate = instruction.operand(); - return immediate; - - case Source::Indirect: - target_address = address(instruction, pointer, context); - break; - case Source::IndirectNoBase: - target_address = address(instruction, pointer, context); - break; - case Source::DirectAddress: - target_address = address(instruction, pointer, context); - break; - } - - // If execution has reached here then a memory fetch is required. - // Do it and exit. - return &context.memory.template access(instruction.data_segment(), target_address); -}; - namespace Primitive { // The below takes a reference in order properly to handle PUSH SP, diff --git a/InstructionSets/x86/Implementation/Resolver.hpp b/InstructionSets/x86/Implementation/Resolver.hpp new file mode 100644 index 000000000..300191119 --- /dev/null +++ b/InstructionSets/x86/Implementation/Resolver.hpp @@ -0,0 +1,207 @@ +// +// Resolver.hpp +// Clock Signal +// +// Created by Thomas Harte on 05/11/2023. +// Copyright © 2023 Thomas Harte. All rights reserved. +// + +#ifndef Resolver_h +#define Resolver_h + +#include "../AccessType.hpp" + +namespace InstructionSet::x86 { + +/// Obtain a pointer to the value desribed by @c source, which is one of those named by @c pointer, using @c instruction and @c context +/// for offsets, registers and memory contents. +/// +/// If @c source is Source::None then @c none is returned. +/// +/// If @c source is Source::Immediate then the appropriate portion of @c instrucion's operand +/// is copied to @c *immediate and @c immediate is returned. +template +IntT *resolve( + InstructionT &instruction, + Source source, + DataPointer pointer, + ContextT &context, + IntT *none = nullptr, + IntT *immediate = nullptr +); + +/// Calculates the absolute address for @c pointer given the registers and memory provided in @c context and taking any +/// referenced offset from @c instruction. +template +uint32_t address( + InstructionT &instruction, + DataPointer pointer, + ContextT &context +) { + if constexpr (source == Source::DirectAddress) { + return instruction.offset(); + } + + uint32_t address; + uint16_t zero = 0; + address = *resolve(instruction, pointer.index(), pointer, context, &zero); + if constexpr (is_32bit(ContextT::model)) { + address <<= pointer.scale(); + } + address += instruction.offset(); + + if constexpr (source == Source::IndirectNoBase) { + return address; + } + return address + *resolve(instruction, pointer.base(), pointer, context); +} + +/// @returns a pointer to the contents of the register identified by the combination of @c IntT and @c Source if any; +/// @c nullptr otherwise. @c access is currently unused but is intended to provide the hook upon which updates to +/// segment registers can be tracked for protected modes. +template +IntT *register_(ContextT &context) { + static constexpr bool supports_dword = is_32bit(ContextT::model); + + switch(source) { + case Source::eAX: + // Slightly contorted if chain here and below: + // + // (i) does the `constexpr` version of a `switch`; and + // (i) ensures .eax() etc aren't called on @c registers for 16-bit processors, so they need not implement 32-bit storage. + if constexpr (supports_dword && std::is_same_v) { return &context.registers.eax(); } + else if constexpr (std::is_same_v) { return &context.registers.ax(); } + else if constexpr (std::is_same_v) { return &context.registers.al(); } + else { return nullptr; } + case Source::eCX: + if constexpr (supports_dword && std::is_same_v) { return &context.registers.ecx(); } + else if constexpr (std::is_same_v) { return &context.registers.cx(); } + else if constexpr (std::is_same_v) { return &context.registers.cl(); } + else { return nullptr; } + case Source::eDX: + if constexpr (supports_dword && std::is_same_v) { return &context.registers.edx(); } + else if constexpr (std::is_same_v) { return &context.registers.dx(); } + else if constexpr (std::is_same_v) { return &context.registers.dl(); } + else if constexpr (std::is_same_v) { return nullptr; } + case Source::eBX: + if constexpr (supports_dword && std::is_same_v) { return &context.registers.ebx(); } + else if constexpr (std::is_same_v) { return &context.registers.bx(); } + else if constexpr (std::is_same_v) { return &context.registers.bl(); } + else if constexpr (std::is_same_v) { return nullptr; } + case Source::eSPorAH: + if constexpr (supports_dword && std::is_same_v) { return &context.registers.esp(); } + else if constexpr (std::is_same_v) { return &context.registers.sp(); } + else if constexpr (std::is_same_v) { return &context.registers.ah(); } + else { return nullptr; } + case Source::eBPorCH: + if constexpr (supports_dword && std::is_same_v) { return &context.registers.ebp(); } + else if constexpr (std::is_same_v) { return &context.registers.bp(); } + else if constexpr (std::is_same_v) { return &context.registers.ch(); } + else { return nullptr; } + case Source::eSIorDH: + if constexpr (supports_dword && std::is_same_v) { return &context.registers.esi(); } + else if constexpr (std::is_same_v) { return &context.registers.si(); } + else if constexpr (std::is_same_v) { return &context.registers.dh(); } + else { return nullptr; } + case Source::eDIorBH: + if constexpr (supports_dword && std::is_same_v) { return &context.registers.edi(); } + else if constexpr (std::is_same_v) { return &context.registers.di(); } + else if constexpr (std::is_same_v) { return &context.registers.bh(); } + else { return nullptr; } + + // Segment registers are always 16-bit. + case Source::ES: if constexpr (std::is_same_v) return &context.registers.es(); else return nullptr; + case Source::CS: if constexpr (std::is_same_v) return &context.registers.cs(); else return nullptr; + case Source::SS: if constexpr (std::is_same_v) return &context.registers.ss(); else return nullptr; + case Source::DS: if constexpr (std::is_same_v) return &context.registers.ds(); else return nullptr; + + // 16-bit models don't have FS and GS. + case Source::FS: if constexpr (is_32bit(ContextT::model) && std::is_same_v) return &context.registers.fs(); else return nullptr; + case Source::GS: if constexpr (is_32bit(ContextT::model) && std::is_same_v) return &context.registers.gs(); else return nullptr; + + default: return nullptr; + } +} + +///Obtains the address described by @c pointer from @c instruction given the registers and memory as described by @c context. +template +uint32_t address( + InstructionT &instruction, + DataPointer pointer, + ContextT &context +) { + // TODO: at least on the 8086 this isn't how register 'addresses' are resolved; instead whatever was the last computed address + // remains in the address register and is returned. Find out what other x86s do and make a decision. + switch(pointer.source()) { + default: return 0; + case Source::eAX: return *register_(context); + case Source::eCX: return *register_(context); + case Source::eDX: return *register_(context); + case Source::eBX: return *register_(context); + case Source::eSPorAH: return *register_(context); + case Source::eBPorCH: return *register_(context); + case Source::eSIorDH: return *register_(context); + case Source::eDIorBH: return *register_(context); + case Source::Indirect: return address(instruction, pointer, context); + case Source::IndirectNoBase: return address(instruction, pointer, context); + case Source::DirectAddress: return address(instruction, pointer, context); + } +} + +// See forward declaration, above, for details. +template +IntT *resolve( + InstructionT &instruction, + Source source, + DataPointer pointer, + ContextT &context, + IntT *none, + IntT *immediate +) { + // Rules: + // + // * if this is a memory access, set target_address and break; + // * otherwise return the appropriate value. + uint32_t target_address; + switch(source) { + // Defer all register accesses to the register-specific lookup. + case Source::eAX: return register_(context); + case Source::eCX: return register_(context); + case Source::eDX: return register_(context); + case Source::eBX: return register_(context); + case Source::eSPorAH: return register_(context); + case Source::eBPorCH: return register_(context); + case Source::eSIorDH: return register_(context); + case Source::eDIorBH: return register_(context); + case Source::ES: return register_(context); + case Source::CS: return register_(context); + case Source::SS: return register_(context); + case Source::DS: return register_(context); + case Source::FS: return register_(context); + case Source::GS: return register_(context); + + case Source::None: return none; + + case Source::Immediate: + *immediate = instruction.operand(); + return immediate; + + case Source::Indirect: + target_address = address(instruction, pointer, context); + break; + case Source::IndirectNoBase: + target_address = address(instruction, pointer, context); + break; + case Source::DirectAddress: + target_address = address(instruction, pointer, context); + break; + } + + // If execution has reached here then a memory fetch is required. + // Do it and exit. + return &context.memory.template access(instruction.data_segment(), target_address); +} + +} + +#endif /* Resolver_h */ diff --git a/InstructionSets/x86/Perform.hpp b/InstructionSets/x86/Perform.hpp index cbeacab3b..918c289c4 100644 --- a/InstructionSets/x86/Perform.hpp +++ b/InstructionSets/x86/Perform.hpp @@ -15,26 +15,6 @@ namespace InstructionSet::x86 { -/// Explains the type of access that `perform` intends to perform; is provided as a template parameter to whatever -/// the caller supplies as `MemoryT` and `RegistersT` when obtaining a reference to whatever the processor -/// intends to reference. -/// -/// `perform` guarantees to validate all accesses before modifying any state, giving the caller opportunity to generate -/// any exceptions that might be applicable. -enum class AccessType { - /// The requested value will be read from. - Read, - /// The requested value will be written to. - Write, - /// The requested value will be read from and then written to. - ReadModifyWrite, - /// The requested value has already been authorised for whatever form of access is now intended, so there's no - /// need further to inspect. This is done e.g. by operations that will push multiple values to the stack to verify that - /// all necessary stack space is available ahead of pushing anything, though each individual push will then result in - /// a further `Preauthorised` access. - PreauthorisedRead, -}; - template < Model model_, typename FlowControllerT, diff --git a/OSBindings/Mac/Clock Signal.xcodeproj/project.pbxproj b/OSBindings/Mac/Clock Signal.xcodeproj/project.pbxproj index 5f01c2ea3..647a359ef 100644 --- a/OSBindings/Mac/Clock Signal.xcodeproj/project.pbxproj +++ b/OSBindings/Mac/Clock Signal.xcodeproj/project.pbxproj @@ -1150,6 +1150,8 @@ 42A5E8412ABBE16F00A0DD5D /* nop_test.bin */ = {isa = PBXFileReference; lastKnownFileType = archive.macbinary; path = nop_test.bin; sourceTree = ""; }; 42A5E8422ABBE16F00A0DD5D /* lax_test.bin */ = {isa = PBXFileReference; lastKnownFileType = archive.macbinary; path = lax_test.bin; sourceTree = ""; }; 42A5E8432ABBE16F00A0DD5D /* branch_backwards_test.bin */ = {isa = PBXFileReference; lastKnownFileType = archive.macbinary; path = branch_backwards_test.bin; sourceTree = ""; }; + 42AA41232AF888370016751C /* AccessType.hpp */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.cpp.h; path = AccessType.hpp; sourceTree = ""; }; + 42AA41242AF8893F0016751C /* Resolver.hpp */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.cpp.h; path = Resolver.hpp; sourceTree = ""; }; 42AD552E2A0C4D5000ACE410 /* 68000.hpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.h; path = 68000.hpp; sourceTree = ""; }; 42AD55302A0C4D5000ACE410 /* 68000Storage.hpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.h; path = 68000Storage.hpp; sourceTree = ""; }; 42AD55312A0C4D5000ACE410 /* 68000Implementation.hpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.h; path = 68000Implementation.hpp; sourceTree = ""; }; @@ -2328,6 +2330,7 @@ isa = PBXGroup; children = ( 42437B382ACF2798006DFED1 /* PerformImplementation.hpp */, + 42AA41242AF8893F0016751C /* Resolver.hpp */, ); path = Implementation; sourceTree = ""; @@ -4997,12 +5000,13 @@ children = ( 4BEDA3B925B25563000C2DBD /* Decoder.cpp */, 4B69DEB52AB79E4F0055B217 /* Instruction.cpp */, + 42AA41232AF888370016751C /* AccessType.hpp */, 4BEDA3B825B25563000C2DBD /* Decoder.hpp */, + 42437B342ACF02A9006DFED1 /* Flags.hpp */, 4BEDA3DB25B2588F000C2DBD /* Instruction.hpp */, 42437B392AD07465006DFED1 /* Interrupts.hpp */, 4BE3C69527CBC540000EAD28 /* Model.hpp */, 42437B352ACF0AA2006DFED1 /* Perform.hpp */, - 42437B342ACF02A9006DFED1 /* Flags.hpp */, 42437B372ACF2798006DFED1 /* Implementation */, ); path = x86; diff --git a/OSBindings/Mac/Clock SignalTests/8088Tests.mm b/OSBindings/Mac/Clock SignalTests/8088Tests.mm index 90d4dfbaa..2e19f35f8 100644 --- a/OSBindings/Mac/Clock SignalTests/8088Tests.mm +++ b/OSBindings/Mac/Clock SignalTests/8088Tests.mm @@ -19,6 +19,7 @@ #include "NSData+dataWithContentsOfGZippedFile.h" +#include "../../../InstructionSets/x86/AccessType.hpp" #include "../../../InstructionSets/x86/Decoder.hpp" #include "../../../InstructionSets/x86/Perform.hpp" #include "../../../InstructionSets/x86/Flags.hpp" @@ -97,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); @@ -162,7 +153,7 @@ struct Memory { // Accesses an address based on segment:offset. template - typename ReturnType::type &access(InstructionSet::x86::Source segment, uint16_t offset) { + typename InstructionSet::x86::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. @@ -185,7 +176,7 @@ struct Memory { // Accesses an address based on physical location. template - typename ReturnType::type &access(uint32_t address) { + typename InstructionSet::x86::ReturnType::type &access(uint32_t address) { return access(address, Tag::Accessed); } From 797c9fe129ac31f317607caf3dd1154f06852a19 Mon Sep 17 00:00:00 2001 From: Thomas Harte Date: Sun, 5 Nov 2023 21:47:52 -0500 Subject: [PATCH 04/12] Temporarily avoid use of Writeable. --- InstructionSets/x86/AccessType.hpp | 3 ++- OSBindings/Mac/Clock SignalTests/8088Tests.mm | 4 ++-- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/InstructionSets/x86/AccessType.hpp b/InstructionSets/x86/AccessType.hpp index 544bb2e02..12eb03846 100644 --- a/InstructionSets/x86/AccessType.hpp +++ b/InstructionSets/x86/AccessType.hpp @@ -46,7 +46,8 @@ class Writeable { private: IntT &target_; }; -template struct ReturnType { using type = Writeable; }; +//template struct ReturnType { using type = Writeable; }; +template struct ReturnType { using type = IntT &; }; // Read-modify-writes: return a reference. template struct ReturnType { using type = IntT &; }; diff --git a/OSBindings/Mac/Clock SignalTests/8088Tests.mm b/OSBindings/Mac/Clock SignalTests/8088Tests.mm index 2e19f35f8..d39e79df3 100644 --- a/OSBindings/Mac/Clock SignalTests/8088Tests.mm +++ b/OSBindings/Mac/Clock SignalTests/8088Tests.mm @@ -273,7 +273,7 @@ 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::ReturnType::type &access(InstructionSet::x86::Source segment, uint16_t offset, Tag tag) { const uint32_t physical_address = address(segment, offset); return access(physical_address, tag); } @@ -281,7 +281,7 @@ struct Memory { // 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::ReturnType::type &access(uint32_t address, Tag tag) { if constexpr (type == AccessType::PreauthorisedRead) { if(!test_preauthorisation(address)) { printf("Non preauthorised access\n"); From 2af774601f5eddf59bd09928cffb96480536b612 Mon Sep 17 00:00:00 2001 From: Thomas Harte Date: Mon, 6 Nov 2023 16:04:31 -0500 Subject: [PATCH 05/12] Temporarily disentangle `Memory` and access internals; start to be overt in PerformImplementation. --- InstructionSets/x86/AccessType.hpp | 17 +++++--- .../Implementation/PerformImplementation.hpp | 24 ++++++----- .../x86/Implementation/Resolver.hpp | 42 +++++++++---------- OSBindings/Mac/Clock SignalTests/8088Tests.mm | 17 ++++++-- 4 files changed, 59 insertions(+), 41 deletions(-) diff --git a/InstructionSets/x86/AccessType.hpp b/InstructionSets/x86/AccessType.hpp index 12eb03846..6f90345a4 100644 --- a/InstructionSets/x86/AccessType.hpp +++ b/InstructionSets/x86/AccessType.hpp @@ -31,11 +31,11 @@ enum class AccessType { PreauthorisedRead, }; -template struct ReturnType; +template struct Accessor; // Reads: return a value directly. -template struct ReturnType { using type = IntT; }; -template struct ReturnType { using type = IntT; }; +template struct Accessor { using type = IntT; }; +template struct Accessor { using type = IntT; }; // Writes: return a custom type that can be written but not read. template @@ -46,11 +46,16 @@ class Writeable { private: IntT &target_; }; -//template struct ReturnType { using type = Writeable; }; -template struct ReturnType { using type = IntT &; }; +//template struct Accessor { using type = Writeable; }; +template struct Accessor { using type = IntT &; }; // Read-modify-writes: return a reference. -template struct ReturnType { using type = IntT &; }; +template struct Accessor { using type = IntT &; }; + +// Shorthands; assumed that preauthorised reads have the same return type as reads. +template using read_t = typename Accessor::type; +template using write_t = typename Accessor::type; +template using modify_t = typename Accessor::type; } diff --git a/InstructionSets/x86/Implementation/PerformImplementation.hpp b/InstructionSets/x86/Implementation/PerformImplementation.hpp index d932357a3..3d854b86a 100644 --- a/InstructionSets/x86/Implementation/PerformImplementation.hpp +++ b/InstructionSets/x86/Implementation/PerformImplementation.hpp @@ -279,8 +279,12 @@ void add(IntT &destination, IntT source, ContextT &context) { destination = result; } -template -void sub(IntT &destination, IntT source, ContextT &context) { +template +void sub( + typename Accessor::type destination, + read_t source, + ContextT &context +) { /* DEST ← DEST - (SRC [+ CF]); */ @@ -298,7 +302,7 @@ void sub(IntT &destination, IntT source, ContextT &context) { context.flags.template set_from(result); - if constexpr (write_back) { + if constexpr (destination_type == AccessType::Write) { destination = result; } } @@ -1374,8 +1378,8 @@ template < // // (though GCC offers C++20 syntax as an extension, and Clang seems to follow along, so maybe I'm overthinking) IntT immediate; - const auto source_r = [&]() -> IntT& { - return *resolve( + const auto source_r = [&]() -> IntT { + return resolve( instruction, instruction.source().source(), instruction.source(), @@ -1384,7 +1388,7 @@ template < &immediate); }; const auto source_rmw = [&]() -> IntT& { - return *resolve( + return resolve( instruction, instruction.source().source(), instruction.source(), @@ -1392,8 +1396,8 @@ template < nullptr, &immediate); }; - const auto destination_r = [&]() -> IntT& { - return *resolve( + const auto destination_r = [&]() -> IntT { + return resolve( instruction, instruction.destination().source(), instruction.destination(), @@ -1402,7 +1406,7 @@ template < &immediate); }; const auto destination_w = [&]() -> IntT& { - return *resolve( + return resolve( instruction, instruction.destination().source(), instruction.destination(), @@ -1411,7 +1415,7 @@ template < &immediate); }; const auto destination_rmw = [&]() -> IntT& { - return *resolve( + return resolve( instruction, instruction.destination().source(), instruction.destination(), diff --git a/InstructionSets/x86/Implementation/Resolver.hpp b/InstructionSets/x86/Implementation/Resolver.hpp index 300191119..9f9c9d9e1 100644 --- a/InstructionSets/x86/Implementation/Resolver.hpp +++ b/InstructionSets/x86/Implementation/Resolver.hpp @@ -21,7 +21,7 @@ namespace InstructionSet::x86 { /// If @c source is Source::Immediate then the appropriate portion of @c instrucion's operand /// is copied to @c *immediate and @c immediate is returned. template -IntT *resolve( +typename Accessor::type resolve( InstructionT &instruction, Source source, DataPointer pointer, @@ -44,7 +44,7 @@ uint32_t address( uint32_t address; uint16_t zero = 0; - address = *resolve(instruction, pointer.index(), pointer, context, &zero); + address = resolve(instruction, pointer.index(), pointer, context, &zero); if constexpr (is_32bit(ContextT::model)) { address <<= pointer.scale(); } @@ -53,7 +53,7 @@ uint32_t address( if constexpr (source == Source::IndirectNoBase) { return address; } - return address + *resolve(instruction, pointer.base(), pointer, context); + return address + resolve(instruction, pointer.base(), pointer, context); } /// @returns a pointer to the contents of the register identified by the combination of @c IntT and @c Source if any; @@ -150,7 +150,7 @@ uint32_t address( // See forward declaration, above, for details. template -IntT *resolve( +typename Accessor::type resolve( InstructionT &instruction, Source source, DataPointer pointer, @@ -165,26 +165,26 @@ IntT *resolve( uint32_t target_address; switch(source) { // Defer all register accesses to the register-specific lookup. - case Source::eAX: return register_(context); - case Source::eCX: return register_(context); - case Source::eDX: return register_(context); - case Source::eBX: return register_(context); - case Source::eSPorAH: return register_(context); - case Source::eBPorCH: return register_(context); - case Source::eSIorDH: return register_(context); - case Source::eDIorBH: return register_(context); - case Source::ES: return register_(context); - case Source::CS: return register_(context); - case Source::SS: return register_(context); - case Source::DS: return register_(context); - case Source::FS: return register_(context); - case Source::GS: return register_(context); + case Source::eAX: return *register_(context); + case Source::eCX: return *register_(context); + case Source::eDX: return *register_(context); + case Source::eBX: return *register_(context); + case Source::eSPorAH: return *register_(context); + case Source::eBPorCH: return *register_(context); + case Source::eSIorDH: return *register_(context); + case Source::eDIorBH: return *register_(context); + case Source::ES: return *register_(context); + case Source::CS: return *register_(context); + case Source::SS: return *register_(context); + case Source::DS: return *register_(context); + case Source::FS: return *register_(context); + case Source::GS: return *register_(context); - case Source::None: return none; + case Source::None: return *none; case Source::Immediate: *immediate = instruction.operand(); - return immediate; + return *immediate; case Source::Indirect: target_address = address(instruction, pointer, context); @@ -199,7 +199,7 @@ IntT *resolve( // If execution has reached here then a memory fetch is required. // Do it and exit. - return &context.memory.template access(instruction.data_segment(), target_address); + return context.memory.template access(instruction.data_segment(), target_address); } } diff --git a/OSBindings/Mac/Clock SignalTests/8088Tests.mm b/OSBindings/Mac/Clock SignalTests/8088Tests.mm index d39e79df3..275fc2852 100644 --- a/OSBindings/Mac/Clock SignalTests/8088Tests.mm +++ b/OSBindings/Mac/Clock SignalTests/8088Tests.mm @@ -98,6 +98,15 @@ 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 &; }; + // Constructor. Memory(Registers ®isters) : registers_(registers) { memory.resize(1024*1024); @@ -153,7 +162,7 @@ struct Memory { // Accesses an address based on segment:offset. template - typename InstructionSet::x86::ReturnType::type &access(InstructionSet::x86::Source segment, uint16_t offset) { + 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. @@ -176,7 +185,7 @@ struct Memory { // Accesses an address based on physical location. template - typename InstructionSet::x86::ReturnType::type &access(uint32_t address) { + typename ReturnType::type &access(uint32_t address) { return access(address, Tag::Accessed); } @@ -273,7 +282,7 @@ 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 InstructionSet::x86::ReturnType::type &access(InstructionSet::x86::Source segment, uint16_t offset, Tag tag) { + typename ReturnType::type &access(InstructionSet::x86::Source segment, uint16_t offset, Tag tag) { const uint32_t physical_address = address(segment, offset); return access(physical_address, tag); } @@ -281,7 +290,7 @@ struct Memory { // 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 InstructionSet::x86::ReturnType::type &access(uint32_t address, Tag tag) { + typename ReturnType::type &access(uint32_t address, Tag tag) { if constexpr (type == AccessType::PreauthorisedRead) { if(!test_preauthorisation(address)) { printf("Non preauthorised access\n"); From 2bed2c2c5cd1cd0fe17d3672558f41bfc0d95f0e Mon Sep 17 00:00:00 2001 From: Thomas Harte Date: Tue, 7 Nov 2023 09:14:42 -0500 Subject: [PATCH 06/12] Further simplify syntax. --- InstructionSets/x86/AccessType.hpp | 1 + InstructionSets/x86/Implementation/PerformImplementation.hpp | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/InstructionSets/x86/AccessType.hpp b/InstructionSets/x86/AccessType.hpp index 6f90345a4..b666c59ea 100644 --- a/InstructionSets/x86/AccessType.hpp +++ b/InstructionSets/x86/AccessType.hpp @@ -56,6 +56,7 @@ template struct Accessor { us template using read_t = typename Accessor::type; template using write_t = typename Accessor::type; template using modify_t = typename Accessor::type; +template using access_t = typename Accessor::type; } diff --git a/InstructionSets/x86/Implementation/PerformImplementation.hpp b/InstructionSets/x86/Implementation/PerformImplementation.hpp index 3d854b86a..c3a3626f6 100644 --- a/InstructionSets/x86/Implementation/PerformImplementation.hpp +++ b/InstructionSets/x86/Implementation/PerformImplementation.hpp @@ -281,7 +281,7 @@ void add(IntT &destination, IntT source, ContextT &context) { template void sub( - typename Accessor::type destination, + access_t destination, read_t source, ContextT &context ) { From 0262875088f123fd1fa197430b2f028a20dc0473 Mon Sep 17 00:00:00 2001 From: Thomas Harte Date: Tue, 7 Nov 2023 09:58:42 -0500 Subject: [PATCH 07/12] Claw back to building. --- .../Implementation/PerformImplementation.hpp | 355 +++++++++++++----- OSBindings/Mac/Clock SignalTests/8088Tests.mm | 1 + 2 files changed, 262 insertions(+), 94 deletions(-) diff --git a/InstructionSets/x86/Implementation/PerformImplementation.hpp b/InstructionSets/x86/Implementation/PerformImplementation.hpp index c3a3626f6..495363122 100644 --- a/InstructionSets/x86/Implementation/PerformImplementation.hpp +++ b/InstructionSets/x86/Implementation/PerformImplementation.hpp @@ -258,7 +258,11 @@ void das(uint8_t &al, ContextT &context) { } template -void add(IntT &destination, IntT source, ContextT &context) { +void add( + modify_t destination, + read_t source, + ContextT &context +) { /* DEST ← DEST + SRC [+ CF]; */ @@ -308,7 +312,11 @@ void sub( } template -void test(IntT &destination, IntT source, ContextT &context) { +void test( + read_t destination, + read_t source, + ContextT &context +) { /* TEMP ← SRC1 AND SRC2; SF ← MSB(TEMP); @@ -332,7 +340,10 @@ void test(IntT &destination, IntT source, ContextT &context) { } template -void xchg(IntT &destination, IntT &source) { +void xchg( + modify_t destination, + modify_t source +) { /* TEMP ← DEST DEST ← SRC @@ -342,7 +353,12 @@ void xchg(IntT &destination, IntT &source) { } template -void mul(IntT &destination_high, IntT &destination_low, IntT source, ContextT &context) { +void mul( + modify_t destination_high, + modify_t destination_low, + read_t source, + ContextT &context +) { /* IF byte operation THEN @@ -364,7 +380,12 @@ void mul(IntT &destination_high, IntT &destination_low, IntT source, ContextT &c } template -void imul(IntT &destination_high, IntT &destination_low, IntT source, ContextT &context) { +void imul( + modify_t destination_high, + modify_t destination_low, + read_t source, + ContextT &context +) { /* (as modified by https://www.felixcloutier.com/x86/daa ...) @@ -399,7 +420,12 @@ void imul(IntT &destination_high, IntT &destination_low, IntT source, ContextT & } template -void div(IntT &destination_high, IntT &destination_low, IntT source, ContextT &context) { +void div( + modify_t destination_high, + modify_t destination_low, + read_t source, + ContextT &context +) { /* IF SRC = 0 THEN #DE; (* divide error *) @@ -455,7 +481,12 @@ void div(IntT &destination_high, IntT &destination_low, IntT source, ContextT &c } template -void idiv(IntT &destination_high, IntT &destination_low, IntT source, ContextT &context) { +void idiv( + modify_t destination_high, + modify_t destination_low, + read_t source, + ContextT &context +) { /* IF SRC = 0 THEN #DE; (* divide error *) @@ -512,7 +543,10 @@ void idiv(IntT &destination_high, IntT &destination_low, IntT source, ContextT & } template -void inc(IntT &destination, ContextT &context) { +void inc( + modify_t destination, + ContextT &context +) { /* DEST ← DEST + 1; */ @@ -528,7 +562,11 @@ void inc(IntT &destination, ContextT &context) { } template -void jump(bool condition, IntT displacement, ContextT &context) { +void jump( + bool condition, + IntT displacement, + ContextT &context +) { /* IF condition THEN @@ -547,7 +585,11 @@ void jump(bool condition, IntT displacement, ContextT &context) { } template -void loop(IntT &counter, OffsetT displacement, ContextT &context) { +void loop( + modify_t counter, + OffsetT displacement, + ContextT &context +) { --counter; if(counter) { context.flow_controller.jump(context.registers.ip() + displacement); @@ -555,7 +597,11 @@ void loop(IntT &counter, OffsetT displacement, ContextT &context) { } template -void loope(IntT &counter, OffsetT displacement, ContextT &context) { +void loope( + modify_t counter, + OffsetT displacement, + ContextT &context +) { --counter; if(counter && context.flags.template flag()) { context.flow_controller.jump(context.registers.ip() + displacement); @@ -563,7 +609,11 @@ void loope(IntT &counter, OffsetT displacement, ContextT &context) { } template -void loopne(IntT &counter, OffsetT displacement, ContextT &context) { +void loopne( + modify_t counter, + OffsetT displacement, + ContextT &context +) { --counter; if(counter && !context.flags.template flag()) { context.flow_controller.jump(context.registers.ip() + displacement); @@ -571,7 +621,10 @@ void loopne(IntT &counter, OffsetT displacement, ContextT &context) { } template -void dec(IntT &destination, ContextT &context) { +void dec( + modify_t destination, + ContextT &context +) { /* DEST ← DEST - 1; */ @@ -588,7 +641,11 @@ void dec(IntT &destination, ContextT &context) { } template -void and_(IntT &destination, IntT source, ContextT &context) { +void and_( + modify_t destination, + read_t source, + ContextT &context +) { /* DEST ← DEST AND SRC; */ @@ -603,7 +660,11 @@ void and_(IntT &destination, IntT source, ContextT &context) { } template -void or_(IntT &destination, IntT source, ContextT &context) { +void or_( + modify_t destination, + read_t source, + ContextT &context +) { /* DEST ← DEST OR SRC; */ @@ -618,7 +679,11 @@ void or_(IntT &destination, IntT source, ContextT &context) { } template -void xor_(IntT &destination, IntT source, ContextT &context) { +void xor_( + modify_t destination, + read_t source, + ContextT &context +) { /* DEST ← DEST XOR SRC; */ @@ -633,7 +698,10 @@ void xor_(IntT &destination, IntT source, ContextT &context) { } template -void neg(IntT &destination, ContextT &context) { +void neg( + modify_t destination, + ContextT &context +) { /* IF DEST = 0 THEN CF ← 0 @@ -655,35 +723,49 @@ void neg(IntT &destination, ContextT &context) { } template -void not_(IntT &destination) { +void not_( + modify_t destination +) { /* DEST ← NOT DEST; */ /* Flags affected: none. */ - destination = ~destination; + destination = ~destination; } template -void call_relative(IntT offset, ContextT &context) { +void call_relative( + IntT offset, + ContextT &context +) { push(context.registers.ip(), context); context.flow_controller.jump(context.registers.ip() + offset); } template -void call_absolute(IntT target, ContextT &context) { +void call_absolute( + read_t target, + ContextT &context +) { push(context.registers.ip(), context); context.flow_controller.jump(target); } template -void jump_absolute(IntT target, ContextT &context) { +void jump_absolute( + read_t target, + ContextT &context +) { context.flow_controller.jump(target); } template -void call_far(InstructionT &instruction, ContextT &context) { +void call_far( + InstructionT &instruction, + ContextT &context +) { // TODO: eliminate 16-bit assumption below. const Source source_segment = instruction.data_segment(); context.memory.preauthorise_stack_write(sizeof(uint16_t) * 2); @@ -722,7 +804,10 @@ void call_far(InstructionT &instruction, ContextT &context) { } template -void jump_far(InstructionT &instruction, ContextT &context) { +void jump_far( + InstructionT &instruction, + ContextT &context +) { // TODO: eliminate 16-bit assumption below. uint16_t source_address = 0; const auto pointer = instruction.destination(); @@ -751,7 +836,9 @@ void jump_far(InstructionT &instruction, ContextT &context) { } template -void iret(ContextT &context) { +void iret( + ContextT &context +) { // TODO: all modes other than 16-bit real mode. context.memory.preauthorise_stack_read(sizeof(uint16_t) * 3); const auto ip = pop(context); @@ -761,14 +848,20 @@ void iret(ContextT &context) { } template -void ret_near(InstructionT instruction, ContextT &context) { +void ret_near( + InstructionT instruction, + ContextT &context +) { const auto ip = pop(context); context.registers.sp() += instruction.operand(); context.flow_controller.jump(ip); } template -void ret_far(InstructionT instruction, ContextT &context) { +void ret_far( + InstructionT instruction, + ContextT &context +) { context.memory.preauthorise_stack_read(sizeof(uint16_t) * 2); const auto ip = pop(context); const auto cs = pop(context); @@ -778,8 +871,8 @@ void ret_far(InstructionT instruction, ContextT &context) { template void ld( - InstructionT &instruction, - uint16_t &destination, + const InstructionT &instruction, + write_t destination, ContextT &context ) { const auto pointer = instruction.source(); @@ -798,7 +891,7 @@ void ld( template void lea( const InstructionT &instruction, - IntT &destination, + write_t destination, ContextT &context ) { // TODO: address size. @@ -819,19 +912,27 @@ void xlat( } template -void mov(IntT &destination, IntT source) { +void mov( + write_t destination, + read_t source +) { destination = source; } template -void into(ContextT &context) { +void into( + ContextT &context +) { if(context.flags.template flag()) { interrupt(Interrupt::OnOverflow, context); } } template -void sahf(uint8_t &ah, ContextT &context) { +void sahf( + uint8_t &ah, + ContextT &context +) { /* EFLAGS(SF:ZF:0:AF:0:PF:1:CF) ← AH; */ @@ -843,7 +944,10 @@ void sahf(uint8_t &ah, ContextT &context) { } template -void lahf(uint8_t &ah, ContextT &context) { +void lahf( + uint8_t &ah, + ContextT &context +) { /* AH ← EFLAGS(SF:ZF:0:AF:0:PF:1:CF); */ @@ -857,7 +961,9 @@ void lahf(uint8_t &ah, ContextT &context) { } template -void cbw(IntT &ax) { +void cbw( + IntT &ax +) { constexpr IntT test_bit = 1 << (sizeof(IntT) * 4 - 1); constexpr IntT low_half = (1 << (sizeof(IntT) * 4)) - 1; @@ -869,7 +975,10 @@ void cbw(IntT &ax) { } template -void cwd(IntT &dx, IntT ax) { +void cwd( + IntT &dx, + IntT ax +) { dx = ax & Numeric::top_bit() ? IntT(~0) : IntT(0); } @@ -892,19 +1001,29 @@ void cmc(ContextT &context) { } template -void salc(uint8_t &al, ContextT &context) { +void salc( + uint8_t &al, + ContextT &context +) { al = context.flags.template flag() ? 0xff : 0x00; } template -void setmo(IntT &destination, ContextT &context) { +void setmo( + write_t destination, + ContextT &context +) { destination = ~0; context.flags.template set_from(0); context.flags.template set_from(destination); } template -void rcl(IntT &destination, uint8_t count, ContextT &context) { +void rcl( + modify_t destination, + uint8_t count, + ContextT &context +) { /* (* RCL and RCR instructions *) SIZE ← OperandSize @@ -960,7 +1079,11 @@ void rcl(IntT &destination, uint8_t count, ContextT &context) { } template -void rcr(IntT &destination, uint8_t count, ContextT &context) { +void rcr( + modify_t destination, + uint8_t count, + ContextT &context +) { /* (* RCR instruction operation *) IF COUNT = 1 @@ -1002,7 +1125,11 @@ void rcr(IntT &destination, uint8_t count, ContextT &context) { } template -void rol(IntT &destination, uint8_t count, ContextT &context) { +void rol( + modify_t destination, + uint8_t count, + ContextT &context +) { /* (* ROL and ROR instructions *) SIZE ← OperandSize @@ -1049,7 +1176,11 @@ void rol(IntT &destination, uint8_t count, ContextT &context) { } template -void ror(IntT &destination, uint8_t count, ContextT &context) { +void ror( + modify_t destination, + uint8_t count, + ContextT &context +) { /* (* ROL and ROR instructions *) SIZE ← OperandSize @@ -1152,7 +1283,11 @@ void ror(IntT &destination, uint8_t count, ContextT &context) { For a non-zero count, the AF flag is undefined. */ template -void sal(IntT &destination, uint8_t count, ContextT &context) { +void sal( + modify_t destination, + uint8_t count, + ContextT &context +) { switch(count) { case 0: return; case Numeric::bit_size(): @@ -1179,7 +1314,11 @@ void sal(IntT &destination, uint8_t count, ContextT &context) { } template -void sar(IntT &destination, uint8_t count, ContextT &context) { +void sar( + modify_t destination, + uint8_t count, + ContextT &context +) { if(!count) { return; } @@ -1198,7 +1337,11 @@ void sar(IntT &destination, uint8_t count, ContextT &context) { } template -void shr(IntT &destination, uint8_t count, ContextT &context) { +void shr( + modify_t destination, + uint8_t count, + ContextT &context +) { if(!count) { return; } @@ -1219,23 +1362,32 @@ void shr(IntT &destination, uint8_t count, ContextT &context) { } template -void popf(ContextT &context) { +void popf( + ContextT &context +) { context.flags.set(pop(context)); } template -void pushf(ContextT &context) { +void pushf( + ContextT &context +) { uint16_t value = context.flags.get(); push(value, context); } template -bool repetition_over(const AddressT &eCX) { +bool repetition_over( + const AddressT &eCX +) { return repetition != Repetition::None && !eCX; } template -void repeat(AddressT &eCX, ContextT &context) { +void repeat( + AddressT &eCX, + ContextT &context +) { if( repetition == Repetition::None || // No repetition => stop. !(--eCX) // [e]cx is zero after being decremented => stop. @@ -1252,7 +1404,13 @@ void repeat(AddressT &eCX, ContextT &context) { } template -void cmps(const InstructionT &instruction, AddressT &eCX, AddressT &eSI, AddressT &eDI, ContextT &context) { +void cmps( + const InstructionT &instruction, + AddressT &eCX, + AddressT &eSI, + AddressT &eDI, + ContextT &context +) { if(repetition_over(eCX)) { return; } @@ -1262,7 +1420,7 @@ void cmps(const InstructionT &instruction, AddressT &eCX, AddressT &eSI, Address eSI += context.flags.template direction() * sizeof(IntT); eDI += context.flags.template direction() * sizeof(IntT); - Primitive::sub(lhs, rhs, context); + Primitive::sub(lhs, rhs, context); repeat(eCX, context); } @@ -1276,7 +1434,7 @@ void scas(AddressT &eCX, AddressT &eDI, IntT &eAX, ContextT &context) { const IntT rhs = context.memory.template access(Source::ES, eDI); eDI += context.flags.template direction() * sizeof(IntT); - Primitive::sub(eAX, rhs, context); + Primitive::sub(eAX, rhs, context); repeat(eCX, context); } @@ -1513,39 +1671,45 @@ template < case Operation::HLT: context.flow_controller.halt(); return; case Operation::WAIT: context.flow_controller.wait(); return; - case Operation::ADC: Primitive::add(destination_rmw(), source_r(), context); break; - case Operation::ADD: Primitive::add(destination_rmw(), source_r(), context); break; - case Operation::SBB: Primitive::sub(destination_rmw(), source_r(), context); break; - case Operation::SUB: Primitive::sub(destination_rmw(), source_r(), context); break; - case Operation::CMP: Primitive::sub(destination_r(), source_r(), context); return; - case Operation::TEST: Primitive::test(destination_r(), source_r(), context); return; + case Operation::ADC: Primitive::add(destination_rmw(), source_r(), context); break; + case Operation::ADD: Primitive::add(destination_rmw(), source_r(), context); break; + case Operation::SBB: + Primitive::sub(destination_rmw(), source_r(), context); + break; + case Operation::SUB: + Primitive::sub(destination_rmw(), source_r(), context); + break; + case Operation::CMP: + Primitive::sub(destination_r(), source_r(), context); + return; + case Operation::TEST: Primitive::test(destination_r(), source_r(), context); return; - case Operation::MUL: Primitive::mul(pair_high(), pair_low(), source_r(), context); return; - case Operation::IMUL_1: Primitive::imul(pair_high(), pair_low(), source_r(), context); return; - case Operation::DIV: Primitive::div(pair_high(), pair_low(), source_r(), context); return; - case Operation::IDIV: Primitive::idiv(pair_high(), pair_low(), source_r(), context); return; + case Operation::MUL: Primitive::mul(pair_high(), pair_low(), source_r(), context); return; + case Operation::IMUL_1: Primitive::imul(pair_high(), pair_low(), source_r(), context); return; + case Operation::DIV: Primitive::div(pair_high(), pair_low(), source_r(), context); return; + case Operation::IDIV: Primitive::idiv(pair_high(), pair_low(), source_r(), context); return; - case Operation::INC: Primitive::inc(destination_rmw(), context); break; - case Operation::DEC: Primitive::dec(destination_rmw(), context); break; + case Operation::INC: Primitive::inc(destination_rmw(), context); break; + case Operation::DEC: Primitive::dec(destination_rmw(), context); break; - case Operation::AND: Primitive::and_(destination_rmw(), source_r(), context); break; - case Operation::OR: Primitive::or_(destination_rmw(), source_r(), context); break; - case Operation::XOR: Primitive::xor_(destination_rmw(), source_r(), context); break; - case Operation::NEG: Primitive::neg(source_rmw(), context); break; // TODO: should be a destination. - case Operation::NOT: Primitive::not_(source_rmw()); break; // TODO: should be a destination. + case Operation::AND: Primitive::and_(destination_rmw(), source_r(), context); break; + case Operation::OR: Primitive::or_(destination_rmw(), source_r(), context); break; + case Operation::XOR: Primitive::xor_(destination_rmw(), source_r(), context); break; + case Operation::NEG: Primitive::neg(source_rmw(), context); break; // TODO: should be a destination. + case Operation::NOT: Primitive::not_(source_rmw()); break; // TODO: should be a destination. - case Operation::CALLrel: Primitive::call_relative(instruction.displacement(), context); return; - case Operation::CALLabs: Primitive::call_absolute(destination_r(), context); return; - case Operation::CALLfar: Primitive::call_far(instruction, context); return; + case Operation::CALLrel: Primitive::call_relative(instruction.displacement(), context); return; + case Operation::CALLabs: Primitive::call_absolute(destination_r(), context); return; + case Operation::CALLfar: Primitive::call_far(instruction, context); return; - case Operation::JMPrel: jcc(true); return; - case Operation::JMPabs: Primitive::jump_absolute(destination_r(), context); return; - case Operation::JMPfar: Primitive::jump_far(instruction, context); return; + case Operation::JMPrel: jcc(true); return; + case Operation::JMPabs: Primitive::jump_absolute(destination_r(), context); return; + case Operation::JMPfar: Primitive::jump_far(instruction, context); return; - case Operation::JCXZ: jcc(!eCX()); return; - case Operation::LOOP: Primitive::loop(eCX(), instruction.offset(), context); return; - case Operation::LOOPE: Primitive::loope(eCX(), instruction.offset(), context); return; - case Operation::LOOPNE: Primitive::loopne(eCX(), instruction.offset(), context); return; + case Operation::JCXZ: jcc(!eCX()); return; + case Operation::LOOP: Primitive::loop(eCX(), instruction.offset(), context); return; + case Operation::LOOPE: Primitive::loope(eCX(), instruction.offset(), context); return; + case Operation::LOOPNE: Primitive::loopne(eCX(), instruction.offset(), context); return; case Operation::IRET: Primitive::iret(context); return; case Operation::RETnear: Primitive::ret_near(instruction, context); return; @@ -1560,33 +1724,33 @@ template < case Operation::LDS: if constexpr (data_size == DataSize::Word) Primitive::ld(instruction, destination_w(), context); return; case Operation::LES: if constexpr (data_size == DataSize::Word) Primitive::ld(instruction, destination_w(), context); return; - case Operation::LEA: Primitive::lea(instruction, destination_w(), context); return; - case Operation::MOV: Primitive::mov(destination_w(), source_r()); break; + case Operation::LEA: Primitive::lea(instruction, destination_w(), context); return; + case Operation::MOV: Primitive::mov(destination_w(), source_r()); break; case Operation::JO: jcc(context.flags.template condition()); return; case Operation::JNO: jcc(!context.flags.template condition()); return; case Operation::JB: jcc(context.flags.template condition()); return; - case Operation::JNB: jcc(!context.flags.template condition()); return; + case Operation::JNB: jcc(!context.flags.template condition()); return; case Operation::JZ: jcc(context.flags.template condition()); return; case Operation::JNZ: jcc(!context.flags.template condition()); return; case Operation::JBE: jcc(context.flags.template condition()); return; case Operation::JNBE: jcc(!context.flags.template condition()); return; case Operation::JS: jcc(context.flags.template condition()); return; case Operation::JNS: jcc(!context.flags.template condition()); return; - case Operation::JP: jcc(!context.flags.template condition()); return; + case Operation::JP: jcc(!context.flags.template condition()); return; case Operation::JNP: jcc(context.flags.template condition()); return; case Operation::JL: jcc(context.flags.template condition()); return; case Operation::JNL: jcc(!context.flags.template condition()); return; case Operation::JLE: jcc(context.flags.template condition()); return; case Operation::JNLE: jcc(!context.flags.template condition()); return; - case Operation::RCL: Primitive::rcl(destination_rmw(), shift_count(), context); break; - case Operation::RCR: Primitive::rcr(destination_rmw(), shift_count(), context); break; - case Operation::ROL: Primitive::rol(destination_rmw(), shift_count(), context); break; - case Operation::ROR: Primitive::ror(destination_rmw(), shift_count(), context); break; - case Operation::SAL: Primitive::sal(destination_rmw(), shift_count(), context); break; - case Operation::SAR: Primitive::sar(destination_rmw(), shift_count(), context); break; - case Operation::SHR: Primitive::shr(destination_rmw(), shift_count(), context); break; + case Operation::RCL: Primitive::rcl(destination_rmw(), shift_count(), context); break; + case Operation::RCR: Primitive::rcr(destination_rmw(), shift_count(), context); break; + case Operation::ROL: Primitive::rol(destination_rmw(), shift_count(), context); break; + case Operation::ROR: Primitive::ror(destination_rmw(), shift_count(), context); break; + case Operation::SAL: Primitive::sal(destination_rmw(), shift_count(), context); break; + case Operation::SAR: Primitive::sar(destination_rmw(), shift_count(), context); break; + case Operation::SHR: Primitive::shr(destination_rmw(), shift_count(), context); break; case Operation::CLC: Primitive::clc(context); return; case Operation::CLD: Primitive::cld(context); return; @@ -1596,12 +1760,12 @@ template < case Operation::STI: Primitive::sti(context); return; case Operation::CMC: Primitive::cmc(context); return; - case Operation::XCHG: Primitive::xchg(destination_rmw(), source_rmw()); break; + case Operation::XCHG: Primitive::xchg(destination_rmw(), source_rmw()); break; case Operation::SALC: Primitive::salc(context.registers.al(), context); return; case Operation::SETMO: if constexpr (ContextT::model == Model::i8086) { - Primitive::setmo(destination_w(), context); + Primitive::setmo(destination_w(), context); break; } else { // TODO. @@ -1612,7 +1776,7 @@ template < // Test CL out here to avoid taking a reference to memory if // no write is going to occur. if(context.registers.cl()) { - Primitive::setmo(destination_w(), context); + Primitive::setmo(destination_w(), context); } break; } else { @@ -1626,7 +1790,10 @@ template < case Operation::XLAT: Primitive::xlat(instruction, context); return; case Operation::POP: destination_w() = Primitive::pop(context); break; - case Operation::PUSH: Primitive::push(source_r(), context); break; + case Operation::PUSH: + Primitive::push(source_rmw(), context); // PUSH SP modifies SP before pushing it; + // hence PUSH is sometimes read-modify-write. + break; case Operation::POPF: Primitive::popf(context); break; case Operation::PUSHF: Primitive::pushf(context); break; diff --git a/OSBindings/Mac/Clock SignalTests/8088Tests.mm b/OSBindings/Mac/Clock SignalTests/8088Tests.mm index 275fc2852..a28c37670 100644 --- a/OSBindings/Mac/Clock SignalTests/8088Tests.mm +++ b/OSBindings/Mac/Clock SignalTests/8088Tests.mm @@ -106,6 +106,7 @@ struct Memory { // Writes: return a reference. template struct ReturnType { using type = IntT &; }; + template struct ReturnType { using type = IntT &; }; // Constructor. Memory(Registers ®isters) : registers_(registers) { From e56e49a318002eaaeadff1bceab27d9ef9b4485f Mon Sep 17 00:00:00 2001 From: Thomas Harte Date: Tue, 7 Nov 2023 10:09:04 -0500 Subject: [PATCH 08/12] Fix SUB/SBB writes. --- InstructionSets/x86/AccessType.hpp | 4 ++++ .../x86/Implementation/PerformImplementation.hpp | 14 +++++++------- 2 files changed, 11 insertions(+), 7 deletions(-) diff --git a/InstructionSets/x86/AccessType.hpp b/InstructionSets/x86/AccessType.hpp index b666c59ea..bba27e948 100644 --- a/InstructionSets/x86/AccessType.hpp +++ b/InstructionSets/x86/AccessType.hpp @@ -31,6 +31,10 @@ enum class AccessType { PreauthorisedRead, }; +constexpr bool is_writeable(AccessType type) { + return type == AccessType::ReadModifyWrite || type == AccessType::Write; +} + template struct Accessor; // Reads: return a value directly. diff --git a/InstructionSets/x86/Implementation/PerformImplementation.hpp b/InstructionSets/x86/Implementation/PerformImplementation.hpp index 495363122..713ca3141 100644 --- a/InstructionSets/x86/Implementation/PerformImplementation.hpp +++ b/InstructionSets/x86/Implementation/PerformImplementation.hpp @@ -306,7 +306,7 @@ void sub( context.flags.template set_from(result); - if constexpr (destination_type == AccessType::Write) { + if constexpr (is_writeable(destination_type)) { destination = result; } } @@ -1536,7 +1536,7 @@ template < // // (though GCC offers C++20 syntax as an extension, and Clang seems to follow along, so maybe I'm overthinking) IntT immediate; - const auto source_r = [&]() -> IntT { + const auto source_r = [&]() -> read_t { return resolve( instruction, instruction.source().source(), @@ -1545,7 +1545,7 @@ template < nullptr, &immediate); }; - const auto source_rmw = [&]() -> IntT& { + const auto source_rmw = [&]() -> modify_t { return resolve( instruction, instruction.source().source(), @@ -1554,7 +1554,7 @@ template < nullptr, &immediate); }; - const auto destination_r = [&]() -> IntT { + const auto destination_r = [&]() -> read_t { return resolve( instruction, instruction.destination().source(), @@ -1563,7 +1563,7 @@ template < nullptr, &immediate); }; - const auto destination_w = [&]() -> IntT& { + const auto destination_w = [&]() -> write_t { return resolve( instruction, instruction.destination().source(), @@ -1572,7 +1572,7 @@ template < nullptr, &immediate); }; - const auto destination_rmw = [&]() -> IntT& { + const auto destination_rmw = [&]() -> modify_t { return resolve( instruction, instruction.destination().source(), @@ -1762,7 +1762,7 @@ template < case Operation::XCHG: Primitive::xchg(destination_rmw(), source_rmw()); break; - case Operation::SALC: Primitive::salc(context.registers.al(), context); return; + case Operation::SALC: Primitive::salc(context.registers.al(), context); return; case Operation::SETMO: if constexpr (ContextT::model == Model::i8086) { Primitive::setmo(destination_w(), context); From 91b7d5587169905c2b52cb19dae1bb343a890c9d Mon Sep 17 00:00:00 2001 From: Thomas Harte Date: Tue, 7 Nov 2023 10:13:18 -0500 Subject: [PATCH 09/12] Get strict about writeables. --- InstructionSets/x86/AccessType.hpp | 5 ++--- InstructionSets/x86/Implementation/PerformImplementation.hpp | 4 ++-- InstructionSets/x86/Implementation/Resolver.hpp | 4 ++-- 3 files changed, 6 insertions(+), 7 deletions(-) diff --git a/InstructionSets/x86/AccessType.hpp b/InstructionSets/x86/AccessType.hpp index bba27e948..14ad92891 100644 --- a/InstructionSets/x86/AccessType.hpp +++ b/InstructionSets/x86/AccessType.hpp @@ -46,12 +46,11 @@ template class Writeable { public: Writeable(IntT &target) : target_(target) {} - void operator=(IntT value) { target_ = value; } + IntT operator=(IntT value) { return target_ = value; } private: IntT &target_; }; -//template struct Accessor { using type = Writeable; }; -template struct Accessor { using type = IntT &; }; +template struct Accessor { using type = Writeable; }; // Read-modify-writes: return a reference. template struct Accessor { using type = IntT &; }; diff --git a/InstructionSets/x86/Implementation/PerformImplementation.hpp b/InstructionSets/x86/Implementation/PerformImplementation.hpp index 713ca3141..e9d2900da 100644 --- a/InstructionSets/x86/Implementation/PerformImplementation.hpp +++ b/InstructionSets/x86/Implementation/PerformImplementation.hpp @@ -1013,9 +1013,9 @@ void setmo( write_t destination, ContextT &context ) { - destination = ~0; + const auto result = destination = ~0; context.flags.template set_from(0); - context.flags.template set_from(destination); + context.flags.template set_from(result); } template diff --git a/InstructionSets/x86/Implementation/Resolver.hpp b/InstructionSets/x86/Implementation/Resolver.hpp index 9f9c9d9e1..0f84eb825 100644 --- a/InstructionSets/x86/Implementation/Resolver.hpp +++ b/InstructionSets/x86/Implementation/Resolver.hpp @@ -44,7 +44,7 @@ uint32_t address( uint32_t address; uint16_t zero = 0; - address = resolve(instruction, pointer.index(), pointer, context, &zero); + address = resolve(instruction, pointer.index(), pointer, context, &zero); if constexpr (is_32bit(ContextT::model)) { address <<= pointer.scale(); } @@ -53,7 +53,7 @@ uint32_t address( if constexpr (source == Source::IndirectNoBase) { return address; } - return address + resolve(instruction, pointer.base(), pointer, context); + return address + resolve(instruction, pointer.base(), pointer, context); } /// @returns a pointer to the contents of the register identified by the combination of @c IntT and @c Source if any; From 413e7b7de1e56130f1427bd68607d32dba3bf2ac Mon Sep 17 00:00:00 2001 From: Thomas Harte Date: Tue, 7 Nov 2023 14:03:20 -0500 Subject: [PATCH 10/12] 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. From f608153c1a92e40c1ef8a68fc1a245218a07fd4e Mon Sep 17 00:00:00 2001 From: Thomas Harte Date: Tue, 7 Nov 2023 14:38:23 -0500 Subject: [PATCH 11/12] Don't bother prepropulating for writes. --- OSBindings/Mac/Clock SignalTests/8088Tests.mm | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/OSBindings/Mac/Clock SignalTests/8088Tests.mm b/OSBindings/Mac/Clock SignalTests/8088Tests.mm index f698860a3..2c96678b7 100644 --- a/OSBindings/Mac/Clock SignalTests/8088Tests.mm +++ b/OSBindings/Mac/Clock SignalTests/8088Tests.mm @@ -302,7 +302,12 @@ struct Memory { 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); + + // Prepopulate only if this is a modify. + if constexpr (type == AccessType::ReadModifyWrite) { + 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); From b927cf4159c80f4078a74e5b843cff8f68526ea8 Mon Sep 17 00:00:00 2001 From: Thomas Harte Date: Tue, 7 Nov 2023 22:08:44 -0500 Subject: [PATCH 12/12] Resolve new decoding errors. --- InstructionSets/x86/Decoder.cpp | 12 ++++++------ InstructionSets/x86/Instruction.cpp | 7 ++++++- 2 files changed, 12 insertions(+), 7 deletions(-) diff --git a/InstructionSets/x86/Decoder.cpp b/InstructionSets/x86/Decoder.cpp index bc7bdb341..32d676b1d 100644 --- a/InstructionSets/x86/Decoder.cpp +++ b/InstructionSets/x86/Decoder.cpp @@ -163,19 +163,19 @@ std::pair::InstructionT> Decoder::decode(con PartialBlock(0x20, AND); break; case 0x26: segment_override_ = Source::ES; break; - case 0x27: Complete(DAA, eAX, eAX, DataSize::Byte); break; + case 0x27: Complete(DAA, None, None, DataSize::Byte); break; PartialBlock(0x28, SUB); break; case 0x2e: segment_override_ = Source::CS; break; - case 0x2f: Complete(DAS, eAX, eAX, DataSize::Byte); break; + case 0x2f: Complete(DAS, None, None, DataSize::Byte); break; PartialBlock(0x30, XOR); break; case 0x36: segment_override_ = Source::SS; break; - case 0x37: Complete(AAA, eAX, eAX, DataSize::Word); break; + case 0x37: Complete(AAA, None, None, DataSize::Word); break; PartialBlock(0x38, CMP); break; case 0x3e: segment_override_ = Source::DS; break; - case 0x3f: Complete(AAS, eAX, eAX, DataSize::Word); break; + case 0x3f: Complete(AAS, None, None, DataSize::Word); break; #undef PartialBlock @@ -361,8 +361,8 @@ std::pair::InstructionT> Decoder::decode(con case 0x96: Complete(XCHG, eAX, eSI, data_size_); break; case 0x97: Complete(XCHG, eAX, eDI, data_size_); break; - case 0x98: Complete(CBW, eAX, AH, data_size_); break; - case 0x99: Complete(CWD, eAX, eDX, data_size_); break; + case 0x98: Complete(CBW, None, None, data_size_); break; + case 0x99: Complete(CWD, None, None, data_size_); break; case 0x9a: Far(CALLfar); break; case 0x9b: Complete(WAIT, None, None, DataSize::Byte); break; case 0x9c: Complete(PUSHF, None, None, data_size_); break; diff --git a/InstructionSets/x86/Instruction.cpp b/InstructionSets/x86/Instruction.cpp index 40d519eb6..d66a43d3c 100644 --- a/InstructionSets/x86/Instruction.cpp +++ b/InstructionSets/x86/Instruction.cpp @@ -506,7 +506,12 @@ std::string InstructionSet::x86::to_string( default: { const int operands = max_displayed_operands(instruction.second.operation()); const bool displacement = has_displacement(instruction.second.operation()); - const bool print_first = operands > 1 && instruction.second.destination().source() != Source::None; + const bool print_first = + instruction.second.destination().source() != Source::None && + ( + operands > 1 || + (operands > 0 && instruction.second.source().source() == Source::None) + ); if(print_first) { operation += to_string(instruction.second.destination(), instruction.second, offset_length, immediate_length); }