From e07b3da98362f3daf7da383e3a7d497b7b0ca03d Mon Sep 17 00:00:00 2001 From: Thomas Harte Date: Fri, 7 Mar 2025 14:01:59 -0500 Subject: [PATCH 01/16] Add commentary; start fleshing out AT keyboard controller. --- Machines/PCCompatible/KeyboardController.hpp | 55 +++++++++++++++++++- Machines/PCCompatible/RTC.hpp | 3 ++ 2 files changed, 57 insertions(+), 1 deletion(-) diff --git a/Machines/PCCompatible/KeyboardController.hpp b/Machines/PCCompatible/KeyboardController.hpp index 5e599f6bb..476a8b6c0 100644 --- a/Machines/PCCompatible/KeyboardController.hpp +++ b/Machines/PCCompatible/KeyboardController.hpp @@ -13,9 +13,16 @@ namespace PCCompatible { +/*! + Provides an implementation of either an XT- or AT-style keyboard controller, + as determined by the model template parameter. +*/ template class KeyboardController; +/*! + Models the XT keyboard controller. +*/ template class KeyboardController> { public: @@ -99,6 +106,9 @@ private: int reset_delay_ = 0; }; +/*! + Models the AT keyboard controller. +*/ template class KeyboardController> { public: @@ -111,12 +121,16 @@ public: } template - void write([[maybe_unused]] const uint16_t port, [[maybe_unused]] const IntT value) { + void write(const uint16_t port,const IntT value) { switch(port) { default: log_.error().append("Unimplemented AT keyboard write: %04x to %04x", value, port); break; + case 0x0060: + parameter_ = value; + break; + case 0x0061: // TODO: // b7: 1 = reset IRQ 0 @@ -124,6 +138,20 @@ public: // b2: enable parity check speaker_.set_control(value & 0x01, value & 0x02); break; + + case 0x0064: + switch(value) { + default: + log_.error().append("Unimplemented AT keyboard command %04x", value); + break; + + case 0xaa: // Self-test; 0x55 => no issues found. + is_tested_ = true; + parameter_ = 0x55; + has_input_ = true; + break; + } + break; } } @@ -134,10 +162,31 @@ public: log_.error().append("Unimplemented AT keyboard read from %04x", port); break; + case 0x0060: + return parameter_; + case 0x0061: + // In a real machine bit 4 toggles as a function of memory refresh; it is often + // used by BIOSes to check that refresh is happening, with no greater inspection + // than that it is toggling. So toggle on read. refresh_toggle_ ^= 0x10; + log_.info().append("AT keyboard: %02x from %04x", refresh_toggle_, port); return refresh_toggle_; + + case 0x0064: + // Status: + // b7 = 1 => parity error on transmission; + // b6 = 1 => receive timeout; + // b5 = 1 => transmit timeout; + // b4 = 1 => keyboard active; + // b3 = 1 = data at 0060 is command, 0 = data; + // b2 = 1 = selftest OK; 0 = just powered up or reset; + // b1 = 1 => input buffer has data; + // b0 = 1 => output data is full. + return + (is_tested_ ? 0x4 : 0x0) | + (has_input_ ? 0x2 : 0x0); } return IntT(~0); } @@ -148,6 +197,10 @@ private: PICs &pics_; Speaker &speaker_; uint8_t refresh_toggle_ = 0; + + bool is_tested_ = false; + bool has_input_ = false; + uint8_t parameter_; }; } diff --git a/Machines/PCCompatible/RTC.hpp b/Machines/PCCompatible/RTC.hpp index 65634afe6..bfce48433 100644 --- a/Machines/PCCompatible/RTC.hpp +++ b/Machines/PCCompatible/RTC.hpp @@ -12,6 +12,9 @@ namespace PCCompatible { +/*! + Implements enough of the MC146818 to satisfy those BIOSes I've tested. +*/ class RTC { public: template From d7b46ee03c31d8bd3d6939b07746c1b478dad942 Mon Sep 17 00:00:00 2001 From: Thomas Harte Date: Fri, 7 Mar 2025 14:23:50 -0500 Subject: [PATCH 02/16] Adopt compact form. --- Machines/PCCompatible/PCCompatible.cpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/Machines/PCCompatible/PCCompatible.cpp b/Machines/PCCompatible/PCCompatible.cpp index 152b1a4f9..a61701ff7 100644 --- a/Machines/PCCompatible/PCCompatible.cpp +++ b/Machines/PCCompatible/PCCompatible.cpp @@ -942,10 +942,10 @@ std::unique_ptr Machine::PCCompatible( const ROMMachine::ROMFetcher &rom_fetcher ) { const Target *const pc_target = dynamic_cast(target); - + using VideoAdaptor = Target::VideoAdaptor; switch(pc_target->adaptor) { - case Target::VideoAdaptor::MDA: return machine(*pc_target, rom_fetcher); - case Target::VideoAdaptor::CGA: return machine(*pc_target, rom_fetcher); + case VideoAdaptor::MDA: return machine(*pc_target, rom_fetcher); + case VideoAdaptor::CGA: return machine(*pc_target, rom_fetcher); default: return nullptr; } } From 8caa1a9664136f14b66e40ca189bd74b92938465 Mon Sep 17 00:00:00 2001 From: Thomas Harte Date: Fri, 7 Mar 2025 14:24:08 -0500 Subject: [PATCH 03/16] Experiment with dialogue. --- Machines/PCCompatible/KeyboardController.hpp | 67 +++++++++++++++----- 1 file changed, 51 insertions(+), 16 deletions(-) diff --git a/Machines/PCCompatible/KeyboardController.hpp b/Machines/PCCompatible/KeyboardController.hpp index 476a8b6c0..9a3676755 100644 --- a/Machines/PCCompatible/KeyboardController.hpp +++ b/Machines/PCCompatible/KeyboardController.hpp @@ -121,13 +121,14 @@ public: } template - void write(const uint16_t port,const IntT value) { + void write(const uint16_t port, const IntT value) { switch(port) { default: log_.error().append("Unimplemented AT keyboard write: %04x to %04x", value, port); break; case 0x0060: + log_.error().append("Keyboard parameter set to: ", value); parameter_ = value; break; @@ -140,17 +141,9 @@ public: break; case 0x0064: - switch(value) { - default: - log_.error().append("Unimplemented AT keyboard command %04x", value); - break; - - case 0xaa: // Self-test; 0x55 => no issues found. - is_tested_ = true; - parameter_ = 0x55; - has_input_ = true; - break; - } + has_output_ = true; + is_command_ = true; + parameter_ = value; break; } } @@ -163,6 +156,8 @@ public: break; case 0x0060: + log_.error().append("Read keyboard parameter of %02x", parameter_); + has_input_ = false; return parameter_; case 0x0061: @@ -174,7 +169,7 @@ public: log_.info().append("AT keyboard: %02x from %04x", refresh_toggle_, port); return refresh_toggle_; - case 0x0064: + case 0x0064: { // Status: // b7 = 1 => parity error on transmission; // b6 = 1 => receive timeout; @@ -184,9 +179,15 @@ public: // b2 = 1 = selftest OK; 0 = just powered up or reset; // b1 = 1 => input buffer has data; // b0 = 1 => output data is full. - return - (is_tested_ ? 0x4 : 0x0) | - (has_input_ ? 0x2 : 0x0); + const uint8_t status = + (is_command_ ? 0x08 : 0x00) | + (is_tested_ ? 0x04 : 0x00) | + (has_input_ ? 0x02 : 0x00) | + (has_output_ ? 0x01 : 0x00); + log_.error().append("Reading status: %02x", status); + perform_command(); + return status; + } } return IntT(~0); } @@ -194,13 +195,47 @@ public: private: Log::Logger log_; + void perform_command() { + if(!is_command_) { + return; + } + + has_output_ = false; + is_command_ = false; + const auto set_input = [&](const uint8_t input) { + parameter_ = input; + has_input_ = true; + is_command_ = false; + }; + + auto info = log_.info(); + info.append("AT keyboard command %04x", command_); + switch(command_) { + default: + info.append("; unimplemented"); + break; + + case 0xaa: // Self-test; 0x55 => no issues found. + log_.error().append("Keyboard self-test"); + is_tested_ = true; + set_input(0x55); + break; + + case 0xd1: // Set output byte. b0 = the A20 gate. + break; + } + } + PICs &pics_; Speaker &speaker_; uint8_t refresh_toggle_ = 0; bool is_tested_ = false; bool has_input_ = false; + bool has_output_ = false; + bool is_command_ = false; uint8_t parameter_; + uint8_t command_; }; } From 93078abe879e4a6a1985d39ef67371564636c2af Mon Sep 17 00:00:00 2001 From: Thomas Harte Date: Fri, 7 Mar 2025 23:25:22 -0500 Subject: [PATCH 04/16] Buffer lines prior to output. --- Outputs/Log.hpp | 53 ++++++++++++++++++++++++++++++++----------------- 1 file changed, 35 insertions(+), 18 deletions(-) diff --git a/Outputs/Log.hpp b/Outputs/Log.hpp index dfe4ecd16..c1e289183 100644 --- a/Outputs/Log.hpp +++ b/Outputs/Log.hpp @@ -10,6 +10,7 @@ #include #include +#include namespace Log { // TODO: if adopting C++20, std::format would be a better model to apply below. @@ -88,7 +89,7 @@ constexpr bool is_enabled(const Source source) { } } -constexpr const char *prefix(Source source) { +constexpr const char *prefix(const Source source) { switch(source) { default: return nullptr; @@ -142,36 +143,52 @@ class Logger { public: static constexpr bool enabled = is_enabled(source); - struct LogLine { - public: - LogLine(FILE *const stream) : stream_(stream) { - if constexpr (!enabled) return; + template + struct LogLine; + template <> + struct LogLine { + public: + explicit LogLine(FILE *const stream) noexcept : stream_(stream) { const auto source_prefix = prefix(source); - if(source_prefix) { - fprintf(stream_, "[%s] ", source_prefix); - } + if(!source_prefix) return; + + output_.resize(strlen(source_prefix) + 4); + std::snprintf(output_.data(), output_.size(), "[%s] ", source_prefix); + output_.pop_back(); } ~LogLine() { - if constexpr (!enabled) return; - fprintf(stream_, "\n"); + fprintf(stream_, "%s\n", output_.c_str()); } - void append(const char *const format, ...) { - if constexpr (!enabled) return; - va_list args; - va_start(args, format); - vfprintf(stream_, format, args); - va_end(args); + template + void append(const char (&format)[size], Args... args) { +#pragma GCC diagnostic push +#pragma GCC diagnostic ignored "-Wformat" + const auto append_size = std::snprintf(nullptr, 0, format, args...); + const auto end = output_.size(); + output_.resize(output_.size() + size_t(append_size) + 1); + std::snprintf(output_.data() + end, size_t(append_size) + 1, format, args...); + output_.pop_back(); +#pragma GCC diagnostic pop } private: + std::string output_; FILE *stream_; }; - LogLine info() { return LogLine(stdout); } - LogLine error() { return LogLine(stderr); } + template <> + struct LogLine { + explicit LogLine(FILE *) noexcept {} + + template + void append(const char (&)[size], Args...) {} + }; + + LogLine info() { return LogLine(stdout); } + LogLine error() { return LogLine(stderr); } }; } From cee2484108c58d2a197c3e40cf78ced43b032238 Mon Sep 17 00:00:00 2001 From: Thomas Harte Date: Fri, 7 Mar 2025 23:32:07 -0500 Subject: [PATCH 05/16] Flip input/output, perform commands instantly. --- Machines/PCCompatible/KeyboardController.hpp | 67 ++++++++------------ 1 file changed, 26 insertions(+), 41 deletions(-) diff --git a/Machines/PCCompatible/KeyboardController.hpp b/Machines/PCCompatible/KeyboardController.hpp index 9a3676755..4ea1a8eaf 100644 --- a/Machines/PCCompatible/KeyboardController.hpp +++ b/Machines/PCCompatible/KeyboardController.hpp @@ -141,9 +141,28 @@ public: break; case 0x0064: - has_output_ = true; is_command_ = true; - parameter_ = value; + const auto set_input = [&](const uint8_t input) { + parameter_ = input; + has_input_ = true; + is_command_ = false; + }; + + auto info = log_.info(); + info.append("AT keyboard command %04x", value); + switch(value) { + default: + info.append("; unimplemented"); + break; + + case 0xaa: // Self-test; 0x55 => no issues found. + log_.error().append("Keyboard self-test"); + set_input(0x55); + break; + + case 0xd1: // Set output byte. b0 = the A20 gate. + break; + } break; } } @@ -177,15 +196,13 @@ public: // b4 = 1 => keyboard active; // b3 = 1 = data at 0060 is command, 0 = data; // b2 = 1 = selftest OK; 0 = just powered up or reset; - // b1 = 1 => input buffer has data; - // b0 = 1 => output data is full. + // b1 = 1 => 'input' buffer full (i.e. don't write 0x60 or 0x64 now — this is input to the controller); + // b0 = 1 => 'output' data is full (i.e. reading from 0x60 now makes sense — output is to PC). const uint8_t status = (is_command_ ? 0x08 : 0x00) | - (is_tested_ ? 0x04 : 0x00) | - (has_input_ ? 0x02 : 0x00) | - (has_output_ ? 0x01 : 0x00); + (has_output_ ? 0x02 : 0x00) | + (has_input_ ? 0x01 : 0x00); log_.error().append("Reading status: %02x", status); - perform_command(); return status; } } @@ -195,47 +212,15 @@ public: private: Log::Logger log_; - void perform_command() { - if(!is_command_) { - return; - } - - has_output_ = false; - is_command_ = false; - const auto set_input = [&](const uint8_t input) { - parameter_ = input; - has_input_ = true; - is_command_ = false; - }; - - auto info = log_.info(); - info.append("AT keyboard command %04x", command_); - switch(command_) { - default: - info.append("; unimplemented"); - break; - - case 0xaa: // Self-test; 0x55 => no issues found. - log_.error().append("Keyboard self-test"); - is_tested_ = true; - set_input(0x55); - break; - - case 0xd1: // Set output byte. b0 = the A20 gate. - break; - } - } - PICs &pics_; Speaker &speaker_; uint8_t refresh_toggle_ = 0; - bool is_tested_ = false; bool has_input_ = false; bool has_output_ = false; + bool is_command_ = false; uint8_t parameter_; - uint8_t command_; }; } From bff10c171407b59636aa16a92fee4c97e7d4955e Mon Sep 17 00:00:00 2001 From: Thomas Harte Date: Fri, 7 Mar 2025 23:34:12 -0500 Subject: [PATCH 06/16] Resolve newfound log ambiguity. --- Components/8530/z8530.cpp | 36 ++++++++++++++++++------------------ 1 file changed, 18 insertions(+), 18 deletions(-) diff --git a/Components/8530/z8530.cpp b/Components/8530/z8530.cpp index a5d376533..4f4697c18 100644 --- a/Components/8530/z8530.cpp +++ b/Components/8530/z8530.cpp @@ -14,7 +14,7 @@ using namespace Zilog::SCC; namespace { -Log::Logger log; +Log::Logger logger; } @@ -54,7 +54,7 @@ std::uint8_t z8530::read(const int address) { case 2: // Handled non-symmetrically between channels. if(address & 1) { - log.error().append("Unimplemented: register 2 status bits"); + logger.error().append("Unimplemented: register 2 status bits"); } else { result = interrupt_vector_; @@ -111,11 +111,11 @@ void z8530::write(const int address, const std::uint8_t value) { case 2: // Interrupt vector register; used only by Channel B. // So there's only one of these. interrupt_vector_ = value; - log.info().append("Interrupt vector set to %d", value); + logger.info().append("Interrupt vector set to %d", value); break; case 9: // Master interrupt and reset register; there is also only one of these. - log.info().append("Master interrupt and reset register: %02x", value); + logger.info().append("Master interrupt and reset register: %02x", value); master_interrupt_control_ = value; break; } @@ -152,7 +152,7 @@ uint8_t z8530::Channel::read(const bool data, const uint8_t pointer) { if(data) { return data_; } else { - log.info().append("Control read from register %d", pointer); + logger.info().append("Control read from register %d", pointer); // Otherwise, this is a control read... switch(pointer) { default: @@ -237,10 +237,10 @@ void z8530::Channel::write(const bool data, const uint8_t pointer, const uint8_t data_ = value; return; } else { - log.info().append("Control write: %02x to register %d", value, pointer); + logger.info().append("Control write: %02x to register %d", value, pointer); switch(pointer) { default: - log.info().append("Unrecognised control write: %02x to register %d", value, pointer); + logger.info().append("Unrecognised control write: %02x to register %d", value, pointer); break; case 0x0: // Write register 0 — CRC reset and other functions. @@ -248,13 +248,13 @@ void z8530::Channel::write(const bool data, const uint8_t pointer, const uint8_t switch(value >> 6) { default: /* Do nothing. */ break; case 1: - log.error().append("TODO: reset Rx CRC checker."); + logger.error().append("TODO: reset Rx CRC checker."); break; case 2: - log.error().append("TODO: reset Tx CRC checker."); + logger.error().append("TODO: reset Tx CRC checker."); break; case 3: - log.error().append("TODO: reset Tx underrun/EOM latch."); + logger.error().append("TODO: reset Tx underrun/EOM latch."); break; } @@ -262,24 +262,24 @@ void z8530::Channel::write(const bool data, const uint8_t pointer, const uint8_t switch((value >> 3)&7) { default: /* Do nothing. */ break; case 2: -// log.info().append("reset ext/status interrupts."); +// logger.info().append("reset ext/status interrupts."); external_status_interrupt_ = false; external_interrupt_status_ = 0; break; case 3: - log.error().append("TODO: send abort (SDLC)."); + logger.error().append("TODO: send abort (SDLC)."); break; case 4: - log.error().append("TODO: enable interrupt on next Rx character."); + logger.error().append("TODO: enable interrupt on next Rx character."); break; case 5: - log.error().append("TODO: reset Tx interrupt pending."); + logger.error().append("TODO: reset Tx interrupt pending."); break; case 6: - log.error().append("TODO: reset error."); + logger.error().append("TODO: reset error."); break; case 7: - log.error().append("TODO: reset highest IUS."); + logger.error().append("TODO: reset highest IUS."); break; } break; @@ -304,7 +304,7 @@ void z8530::Channel::write(const bool data, const uint8_t pointer, const uint8_t b1 = 1 => transmit buffer empty interrupt is enabled; 0 => it isn't. b0 = 1 => external interrupt is enabled; 0 => it isn't. */ - log.info().append("Interrupt mask: %02x", value); + logger.info().append("Interrupt mask: %02x", value); break; case 0x2: // Write register 2 - interrupt vector. @@ -319,7 +319,7 @@ void z8530::Channel::write(const bool data, const uint8_t pointer, const uint8_t case 2: receive_bit_count = 6; break; case 3: receive_bit_count = 8; break; } - log.info().append("Receive bit count: %d", receive_bit_count); + logger.info().append("Receive bit count: %d", receive_bit_count); /* b7,b6: From 4e882e7d4d9ffe4bfdb6aeac6cda2106400919de Mon Sep 17 00:00:00 2001 From: Thomas Harte Date: Fri, 7 Mar 2025 23:36:47 -0500 Subject: [PATCH 07/16] Accept `uint8_t`s only. --- Machines/PCCompatible/KeyboardController.hpp | 3 +-- Machines/PCCompatible/PCCompatible.cpp | 2 +- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/Machines/PCCompatible/KeyboardController.hpp b/Machines/PCCompatible/KeyboardController.hpp index 4ea1a8eaf..86447cc83 100644 --- a/Machines/PCCompatible/KeyboardController.hpp +++ b/Machines/PCCompatible/KeyboardController.hpp @@ -120,8 +120,7 @@ public: void post([[maybe_unused]] const uint8_t value) { } - template - void write(const uint16_t port, const IntT value) { + void write(const uint16_t port, const uint8_t value) { switch(port) { default: log_.error().append("Unimplemented AT keyboard write: %04x to %04x", value, port); diff --git a/Machines/PCCompatible/PCCompatible.cpp b/Machines/PCCompatible/PCCompatible.cpp index a61701ff7..9397b42df 100644 --- a/Machines/PCCompatible/PCCompatible.cpp +++ b/Machines/PCCompatible/PCCompatible.cpp @@ -292,7 +292,7 @@ public: if constexpr (is_xt(model)) { ppi_.write(port, uint8_t(value)); } else { - keyboard_.write(port, value); + keyboard_.write(port, uint8_t(value)); } break; From 96326411bf6d41008f69d2a5a91e631327f65353 Mon Sep 17 00:00:00 2001 From: Thomas Harte Date: Fri, 7 Mar 2025 23:41:56 -0500 Subject: [PATCH 08/16] Move explicit specialization to namespace scope. --- Outputs/Log.hpp | 93 ++++++++++++++++++++++++------------------------- 1 file changed, 46 insertions(+), 47 deletions(-) diff --git a/Outputs/Log.hpp b/Outputs/Log.hpp index c1e289183..f7595206f 100644 --- a/Outputs/Log.hpp +++ b/Outputs/Log.hpp @@ -138,57 +138,56 @@ constexpr const char *prefix(const Source source) { } } +template +struct LogLine; + +template +struct LogLine { +public: + explicit LogLine(FILE *const stream) noexcept : stream_(stream) { + const auto source_prefix = prefix(source); + if(!source_prefix) return; + + output_.resize(strlen(source_prefix) + 4); + std::snprintf(output_.data(), output_.size(), "[%s] ", source_prefix); + output_.pop_back(); + } + + ~LogLine() { + fprintf(stream_, "%s\n", output_.c_str()); + } + + template + void append(const char (&format)[size], Args... args) { +#pragma GCC diagnostic push +#pragma GCC diagnostic ignored "-Wformat" + const auto append_size = std::snprintf(nullptr, 0, format, args...); + const auto end = output_.size(); + output_.resize(output_.size() + size_t(append_size) + 1); + std::snprintf(output_.data() + end, size_t(append_size) + 1, format, args...); + output_.pop_back(); +#pragma GCC diagnostic pop + } + +private: + std::string output_; + FILE *stream_; +}; + +template +struct LogLine { + explicit LogLine(FILE *) noexcept {} + + template + void append(const char (&)[size], Args...) {} +}; + template class Logger { public: static constexpr bool enabled = is_enabled(source); - - template - struct LogLine; - - template <> - struct LogLine { - public: - explicit LogLine(FILE *const stream) noexcept : stream_(stream) { - const auto source_prefix = prefix(source); - if(!source_prefix) return; - - output_.resize(strlen(source_prefix) + 4); - std::snprintf(output_.data(), output_.size(), "[%s] ", source_prefix); - output_.pop_back(); - } - - ~LogLine() { - fprintf(stream_, "%s\n", output_.c_str()); - } - - template - void append(const char (&format)[size], Args... args) { -#pragma GCC diagnostic push -#pragma GCC diagnostic ignored "-Wformat" - const auto append_size = std::snprintf(nullptr, 0, format, args...); - const auto end = output_.size(); - output_.resize(output_.size() + size_t(append_size) + 1); - std::snprintf(output_.data() + end, size_t(append_size) + 1, format, args...); - output_.pop_back(); -#pragma GCC diagnostic pop - } - - private: - std::string output_; - FILE *stream_; - }; - - template <> - struct LogLine { - explicit LogLine(FILE *) noexcept {} - - template - void append(const char (&)[size], Args...) {} - }; - - LogLine info() { return LogLine(stdout); } - LogLine error() { return LogLine(stderr); } + LogLine info() { return LogLine(stdout); } + LogLine error() { return LogLine(stderr); } }; } From 7a88d31fd931eed34c8d449eb2c407489ff67a59 Mon Sep 17 00:00:00 2001 From: Thomas Harte Date: Fri, 7 Mar 2025 23:47:34 -0500 Subject: [PATCH 09/16] Add include for strlen. --- Outputs/Log.hpp | 1 + 1 file changed, 1 insertion(+) diff --git a/Outputs/Log.hpp b/Outputs/Log.hpp index f7595206f..17a4a3377 100644 --- a/Outputs/Log.hpp +++ b/Outputs/Log.hpp @@ -10,6 +10,7 @@ #include #include +#include #include namespace Log { From 98f20c57c17873fca1aa04d2d2205d50b7bba1de Mon Sep 17 00:00:00 2001 From: Thomas Harte Date: Sat, 8 Mar 2025 00:47:56 -0500 Subject: [PATCH 10/16] Attempt to support reset-by-8042, resulting in boot loop. --- Machines/PCCompatible/CPUControl.hpp | 45 +++++++++++++++++++ Machines/PCCompatible/KeyboardController.hpp | 45 ++++++++++++++----- Machines/PCCompatible/PCCompatible.cpp | 7 ++- Machines/PCCompatible/Registers.hpp | 1 + .../Clock Signal.xcodeproj/project.pbxproj | 2 + 5 files changed, 88 insertions(+), 12 deletions(-) create mode 100644 Machines/PCCompatible/CPUControl.hpp diff --git a/Machines/PCCompatible/CPUControl.hpp b/Machines/PCCompatible/CPUControl.hpp new file mode 100644 index 000000000..b700cafeb --- /dev/null +++ b/Machines/PCCompatible/CPUControl.hpp @@ -0,0 +1,45 @@ +// +// CPUControl.hpp +// Clock Signal +// +// Created by Thomas Harte on 08/03/2025. +// Copyright © 2025 Thomas Harte. All rights reserved. +// + +#pragma once + +#include "Memory.hpp" +#include "ProcessorByModel.hpp" +#include "Registers.hpp" +#include "Segments.hpp" + +#include "Analyser/Static/PCCompatible/Target.hpp" + +namespace PCCompatible { + +template +class CPUControl { +public: + CPUControl( + Registers ®isters, + Segments &segments, + Memory &memory + ) : registers_(registers), segments_(segments), memory_(memory) {} + + void reset() { + registers_.reset(); + segments_.reset(); + } + + void set_a20_enabled(bool) { + // Assumed: this'll be something to set on Memory. + } + +private: + Registers ®isters_; + Segments &segments_; + Memory &memory_; +}; + + +} diff --git a/Machines/PCCompatible/KeyboardController.hpp b/Machines/PCCompatible/KeyboardController.hpp index 86447cc83..1d5b382a5 100644 --- a/Machines/PCCompatible/KeyboardController.hpp +++ b/Machines/PCCompatible/KeyboardController.hpp @@ -8,6 +8,7 @@ #pragma once +#include "CPUControl.hpp" #include "PIC.hpp" #include "Speaker.hpp" @@ -92,6 +93,8 @@ public: pics_.pic[0].template apply_edge<1>(true); } + void set_cpu_control(CPUControl *) {} + private: enum class Mode { NormalOperation = 0b01, @@ -117,7 +120,11 @@ public: void run_for([[maybe_unused]] const Cycles cycles) { } - void post([[maybe_unused]] const uint8_t value) { + void post(const uint8_t value) { + parameter_ = value; + has_input_ = true; + is_command_ = false; + pics_.pic[0].template apply_edge<1>(true); } void write(const uint16_t port, const uint8_t value) { @@ -141,11 +148,6 @@ public: case 0x0064: is_command_ = true; - const auto set_input = [&](const uint8_t input) { - parameter_ = input; - has_input_ = true; - is_command_ = false; - }; auto info = log_.info(); info.append("AT keyboard command %04x", value); @@ -156,10 +158,23 @@ public: case 0xaa: // Self-test; 0x55 => no issues found. log_.error().append("Keyboard self-test"); - set_input(0x55); + post(0x55); break; - case 0xd1: // Set output byte. b0 = the A20 gate. + case 0xd1: // Set output byte. b1 = the A20 gate. + log_.error().append("Should set A20 gate: %d", value & 0x02); + cpu_control_->set_a20_enabled(value & 0x02); + break; + + case 0xf0: case 0xf1: case 0xf2: case 0xf3: + case 0xf4: case 0xf5: case 0xf6: case 0xf7: + case 0xf8: case 0xf9: case 0xfa: case 0xfb: + case 0xfc: case 0xfd: case 0xfe: case 0xff: + log_.error().append("Should reset: %x", value & 0x0f); + + if(!(value & 1)) { + cpu_control_->reset(); + } break; } break; @@ -167,7 +182,7 @@ public: } template - IntT read([[maybe_unused]] const uint16_t port) { + IntT read(const uint16_t port) { switch(port) { default: log_.error().append("Unimplemented AT keyboard read from %04x", port); @@ -175,7 +190,10 @@ public: case 0x0060: log_.error().append("Read keyboard parameter of %02x", parameter_); - has_input_ = false; + + // TODO: disabled in response to BIOS expectations but possibly a false + // positive because commands are responded to instantly? +// has_input_ = false; return parameter_; case 0x0061: @@ -198,6 +216,7 @@ public: // b1 = 1 => 'input' buffer full (i.e. don't write 0x60 or 0x64 now — this is input to the controller); // b0 = 1 => 'output' data is full (i.e. reading from 0x60 now makes sense — output is to PC). const uint8_t status = + 0x10 | (is_command_ ? 0x08 : 0x00) | (has_output_ ? 0x02 : 0x00) | (has_input_ ? 0x01 : 0x00); @@ -208,11 +227,17 @@ public: return IntT(~0); } + void set_cpu_control(CPUControl *const control) { + cpu_control_ = control; + } + private: Log::Logger log_; PICs &pics_; Speaker &speaker_; + CPUControl *cpu_control_ = nullptr; + uint8_t refresh_toggle_ = 0; bool has_input_ = false; diff --git a/Machines/PCCompatible/PCCompatible.cpp b/Machines/PCCompatible/PCCompatible.cpp index 9397b42df..d4a1e05fe 100644 --- a/Machines/PCCompatible/PCCompatible.cpp +++ b/Machines/PCCompatible/PCCompatible.cpp @@ -9,6 +9,7 @@ #include "PCCompatible.hpp" #include "CGA.hpp" +#include "CPUControl.hpp" #include "DMA.hpp" #include "FloppyController.hpp" #include "KeyboardController.hpp" @@ -884,14 +885,15 @@ private: segments(registers), memory(registers, segments), flow_controller(registers, segments), + cpu_control(registers, segments, memory), io(pit, dma, ppi, pics, card, fdc, keyboard, rtc) { + keyboard.set_cpu_control(&cpu_control); reset(); } void reset() { - registers.reset(); - segments.reset(); + cpu_control.reset(); } InstructionSet::x86::Flags flags; @@ -899,6 +901,7 @@ private: Segments segments; Memory memory; FlowController flow_controller; + CPUControl cpu_control; IO io; static constexpr auto model = processor_model(pc_model); } context_; diff --git a/Machines/PCCompatible/Registers.hpp b/Machines/PCCompatible/Registers.hpp index 53da2d7d2..49e64a518 100644 --- a/Machines/PCCompatible/Registers.hpp +++ b/Machines/PCCompatible/Registers.hpp @@ -9,6 +9,7 @@ #pragma once #include "InstructionSets/x86/Model.hpp" +#include "Numeric/RegisterSizes.hpp" namespace PCCompatible { diff --git a/OSBindings/Mac/Clock Signal.xcodeproj/project.pbxproj b/OSBindings/Mac/Clock Signal.xcodeproj/project.pbxproj index 39e4e854f..e67997d3e 100644 --- a/OSBindings/Mac/Clock Signal.xcodeproj/project.pbxproj +++ b/OSBindings/Mac/Clock Signal.xcodeproj/project.pbxproj @@ -1457,6 +1457,7 @@ 4B1FBE222D7B739700BAC888 /* FloppyController.hpp */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.cpp.h; path = FloppyController.hpp; sourceTree = ""; }; 4B1FBE232D7B73C800BAC888 /* Speaker.hpp */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.cpp.h; path = Speaker.hpp; sourceTree = ""; }; 4B1FBE242D7B753000BAC888 /* KeyboardController.hpp */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.cpp.h; path = KeyboardController.hpp; sourceTree = ""; }; + 4B1FBE252D7C0B2200BAC888 /* CPUControl.hpp */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.cpp.h; path = CPUControl.hpp; sourceTree = ""; }; 4B2005402B804AA300420C5C /* OperationMapper.hpp */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.cpp.h; path = OperationMapper.hpp; sourceTree = ""; }; 4B2005422B804D6400420C5C /* ARMDecoderTests.mm */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.objcpp; path = ARMDecoderTests.mm; sourceTree = ""; }; 4B2005462B8BD7A500420C5C /* Registers.hpp */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.cpp.h; path = Registers.hpp; sourceTree = ""; }; @@ -2560,6 +2561,7 @@ children = ( 425739372B051EA800B7D1E4 /* PCCompatible.cpp */, 429B13632B20234B006BB4CB /* CGA.hpp */, + 4B1FBE252D7C0B2200BAC888 /* CPUControl.hpp */, 4267A9C92B0D4F17008A59BB /* DMA.hpp */, 4B1FBE222D7B739700BAC888 /* FloppyController.hpp */, 4B1FBE242D7B753000BAC888 /* KeyboardController.hpp */, From e927feb2d6295d2c250dc3288512236025df5499 Mon Sep 17 00:00:00 2001 From: Thomas Harte Date: Sat, 8 Mar 2025 21:59:40 -0500 Subject: [PATCH 11/16] Reintroduce is-tested flag. --- Machines/PCCompatible/KeyboardController.hpp | 3 +++ 1 file changed, 3 insertions(+) diff --git a/Machines/PCCompatible/KeyboardController.hpp b/Machines/PCCompatible/KeyboardController.hpp index 1d5b382a5..cc092296e 100644 --- a/Machines/PCCompatible/KeyboardController.hpp +++ b/Machines/PCCompatible/KeyboardController.hpp @@ -158,6 +158,7 @@ public: case 0xaa: // Self-test; 0x55 => no issues found. log_.error().append("Keyboard self-test"); + is_tested_ = true; post(0x55); break; @@ -218,6 +219,7 @@ public: const uint8_t status = 0x10 | (is_command_ ? 0x08 : 0x00) | + (is_tested_ ? 0x04 : 0x00) | (has_output_ ? 0x02 : 0x00) | (has_input_ ? 0x01 : 0x00); log_.error().append("Reading status: %02x", status); @@ -242,6 +244,7 @@ private: bool has_input_ = false; bool has_output_ = false; + bool is_tested_ = false; bool is_command_ = false; uint8_t parameter_; From 4f35d5dabd19676a86181ca852fe1e1cfe077b74 Mon Sep 17 00:00:00 2001 From: Thomas Harte Date: Sun, 9 Mar 2025 23:09:58 -0400 Subject: [PATCH 12/16] Log A20 line. --- Machines/PCCompatible/CPUControl.hpp | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/Machines/PCCompatible/CPUControl.hpp b/Machines/PCCompatible/CPUControl.hpp index b700cafeb..8f9e46951 100644 --- a/Machines/PCCompatible/CPUControl.hpp +++ b/Machines/PCCompatible/CPUControl.hpp @@ -14,6 +14,7 @@ #include "Segments.hpp" #include "Analyser/Static/PCCompatible/Target.hpp" +#include "Outputs/Log.hpp" namespace PCCompatible { @@ -31,14 +32,17 @@ public: segments_.reset(); } - void set_a20_enabled(bool) { + void set_a20_enabled(const bool enabled) { // Assumed: this'll be something to set on Memory. + log_.info().append("A20 line is now: ", enabled); } private: Registers ®isters_; Segments &segments_; Memory &memory_; + + Log::Logger log_; }; From d77d8df1acf9ac5f7188c784c62645ce8098b150 Mon Sep 17 00:00:00 2001 From: Thomas Harte Date: Tue, 11 Mar 2025 22:35:13 -0400 Subject: [PATCH 13/16] Add closer-to-real keyboard command loop. --- Machines/PCCompatible/KeyboardController.hpp | 150 ++++++++++++------- 1 file changed, 98 insertions(+), 52 deletions(-) diff --git a/Machines/PCCompatible/KeyboardController.hpp b/Machines/PCCompatible/KeyboardController.hpp index cc092296e..6e3f05d81 100644 --- a/Machines/PCCompatible/KeyboardController.hpp +++ b/Machines/PCCompatible/KeyboardController.hpp @@ -117,13 +117,19 @@ class KeyboardController> { public: KeyboardController(PICs &pics, Speaker &speaker) : pics_(pics), speaker_(speaker) {} - void run_for([[maybe_unused]] const Cycles cycles) { + void run_for(const Cycles cycles) { + if(!write_delay_) return; + write_delay_ -= cycles.as(); + if(write_delay_ <= 0) { + write_delay_ = 0; + perform_command(); + } } void post(const uint8_t value) { - parameter_ = value; + input_ = value; has_input_ = true; - is_command_ = false; + command_phase_ = CommandPhase::WaitingForCommand; pics_.pic[0].template apply_edge<1>(true); } @@ -134,8 +140,9 @@ public: break; case 0x0060: - log_.error().append("Keyboard parameter set to: ", value); - parameter_ = value; +// log_.error().append("Keyboard parameter set to %02x", value); + has_output_ = true; + write_delay_ = 10; break; case 0x0061: @@ -147,37 +154,11 @@ public: break; case 0x0064: - is_command_ = true; - - auto info = log_.info(); - info.append("AT keyboard command %04x", value); - switch(value) { - default: - info.append("; unimplemented"); - break; - - case 0xaa: // Self-test; 0x55 => no issues found. - log_.error().append("Keyboard self-test"); - is_tested_ = true; - post(0x55); - break; - - case 0xd1: // Set output byte. b1 = the A20 gate. - log_.error().append("Should set A20 gate: %d", value & 0x02); - cpu_control_->set_a20_enabled(value & 0x02); - break; - - case 0xf0: case 0xf1: case 0xf2: case 0xf3: - case 0xf4: case 0xf5: case 0xf6: case 0xf7: - case 0xf8: case 0xf9: case 0xfa: case 0xfb: - case 0xfc: case 0xfd: case 0xfe: case 0xff: - log_.error().append("Should reset: %x", value & 0x0f); - - if(!(value & 1)) { - cpu_control_->reset(); - } - break; - } +// log_.info().append("AT keyboard command %04x", value); + command_ = value; + has_command_ = true; + command_phase_ = CommandPhase::WaitingForData; + write_delay_ = 10; break; } } @@ -186,24 +167,23 @@ public: IntT read(const uint16_t port) { switch(port) { default: - log_.error().append("Unimplemented AT keyboard read from %04x", port); +// log_.error().append("Unimplemented AT keyboard read from %04x", port); break; case 0x0060: - log_.error().append("Read keyboard parameter of %02x", parameter_); - - // TODO: disabled in response to BIOS expectations but possibly a false - // positive because commands are responded to instantly? -// has_input_ = false; - return parameter_; + log_.error().append("Read from keyboard controller of %02x", input_); + has_input_ = false; + return input_; case 0x0061: // In a real machine bit 4 toggles as a function of memory refresh; it is often // used by BIOSes to check that refresh is happening, with no greater inspection // than that it is toggling. So toggle on read. + // + // TODO: is this really from the keyboard controller? refresh_toggle_ ^= 0x10; - log_.info().append("AT keyboard: %02x from %04x", refresh_toggle_, port); +// log_.info().append("AT keyboard: %02x from %04x", refresh_toggle_, port); return refresh_toggle_; case 0x0064: { @@ -218,11 +198,11 @@ public: // b0 = 1 => 'output' data is full (i.e. reading from 0x60 now makes sense — output is to PC). const uint8_t status = 0x10 | - (is_command_ ? 0x08 : 0x00) | - (is_tested_ ? 0x04 : 0x00) | - (has_output_ ? 0x02 : 0x00) | - (has_input_ ? 0x01 : 0x00); - log_.error().append("Reading status: %02x", status); + (command_phase_ == CommandPhase::WaitingForData ? 0x08 : 0x00) | + (is_tested_ ? 0x04 : 0x00) | + (has_output_ ? 0x02 : 0x00) | + (has_input_ ? 0x01 : 0x00); +// log_.error().append("Reading status: %02x", status); return status; } } @@ -234,6 +214,64 @@ public: } private: + void perform_command() { + // Wait for a command. + if(!has_command_) return; + + // Wait for a parameter if one is needed. + if( + (command_ >= 0x60 && command_ < 0x80) || + (command_ == 0xc1) || (command_ == 0xc2) || + (command_ >= 0xd1 && command_ < 0xd5) + ) { + if(!has_output_) { + return; + } + } + + + { + auto info = log_.info(); + info.append("Keyboard command: %02x", command_); + if(has_output_) { + info.append(" / %02x", output_); + } + } + + // Consume command and parameter, and execute. + has_command_ = has_output_ = false; + + switch(command_) { + default: + log_.info().append("Keyboard command unimplemented", command_); + break; + + case 0xaa: // Self-test; 0x55 => no issues found. + log_.error().append("Keyboard self-test"); + is_tested_ = true; + post(0x55); + break; + + case 0xd1: // Set output byte. b1 = the A20 gate. + log_.error().append("Should set A20 gate: %d", output_ & 0x02); + cpu_control_->set_a20_enabled(output_ & 0x02); + break; + + case 0xf0: case 0xf1: case 0xf2: case 0xf3: + case 0xf4: case 0xf5: case 0xf6: case 0xf7: + case 0xf8: case 0xf9: case 0xfa: case 0xfb: + case 0xfc: case 0xfd: case 0xfe: case 0xff: + log_.error().append("Should reset: %x", command_ & 0x0f); + + if(!(command_ & 1)) { + cpu_control_->reset(); + } + break; + } + + command_phase_ = CommandPhase::WaitingForCommand; + } + Log::Logger log_; PICs &pics_; @@ -243,11 +281,19 @@ private: uint8_t refresh_toggle_ = 0; bool has_input_ = false; + uint8_t input_; bool has_output_ = false; - bool is_tested_ = false; + uint8_t output_; + bool has_command_ = false; + uint8_t command_; - bool is_command_ = false; - uint8_t parameter_; + int write_delay_ = 0; + + bool is_tested_ = false; + enum class CommandPhase { + WaitingForCommand, + WaitingForData, + } command_phase_ = CommandPhase::WaitingForCommand; }; } From e76ca304e6af969c1063c3c2ac98a72d93aca6e9 Mon Sep 17 00:00:00 2001 From: Thomas Harte Date: Tue, 11 Mar 2025 22:54:54 -0400 Subject: [PATCH 14/16] Attempt to support 60 and c0. --- Machines/PCCompatible/KeyboardController.hpp | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/Machines/PCCompatible/KeyboardController.hpp b/Machines/PCCompatible/KeyboardController.hpp index 6e3f05d81..226648db5 100644 --- a/Machines/PCCompatible/KeyboardController.hpp +++ b/Machines/PCCompatible/KeyboardController.hpp @@ -141,6 +141,7 @@ public: case 0x0060: // log_.error().append("Keyboard parameter set to %02x", value); + output_ = value; has_output_ = true; write_delay_ = 10; break; @@ -202,7 +203,7 @@ public: (is_tested_ ? 0x04 : 0x00) | (has_output_ ? 0x02 : 0x00) | (has_input_ ? 0x01 : 0x00); -// log_.error().append("Reading status: %02x", status); + log_.error().append("Reading status: %02x", status); return status; } } @@ -248,7 +249,6 @@ private: case 0xaa: // Self-test; 0x55 => no issues found. log_.error().append("Keyboard self-test"); - is_tested_ = true; post(0x55); break; @@ -257,6 +257,14 @@ private: cpu_control_->set_a20_enabled(output_ & 0x02); break; + case 0x60: + is_tested_ = output_ & 0x4; + break; + + case 0xc0: // Read input port. + post(0xd0); + break; + case 0xf0: case 0xf1: case 0xf2: case 0xf3: case 0xf4: case 0xf5: case 0xf6: case 0xf7: case 0xf8: case 0xf9: case 0xfa: case 0xfb: From 592cea6a2755131fd8db37007227911e03aa9d94 Mon Sep 17 00:00:00 2001 From: Thomas Harte Date: Wed, 12 Mar 2025 12:31:29 -0400 Subject: [PATCH 15/16] Tweak timing to once again pass BIOS controller test. --- Machines/PCCompatible/KeyboardController.hpp | 35 ++++++++++++++------ Machines/PCCompatible/PCCompatible.cpp | 2 +- 2 files changed, 26 insertions(+), 11 deletions(-) diff --git a/Machines/PCCompatible/KeyboardController.hpp b/Machines/PCCompatible/KeyboardController.hpp index 226648db5..c776750f7 100644 --- a/Machines/PCCompatible/KeyboardController.hpp +++ b/Machines/PCCompatible/KeyboardController.hpp @@ -159,7 +159,11 @@ public: command_ = value; has_command_ = true; command_phase_ = CommandPhase::WaitingForData; - write_delay_ = 10; + + write_delay_ = performance_delay(command_); + if(!write_delay_) { + perform_command(); + } break; } } @@ -215,22 +219,33 @@ public: } private: + static constexpr bool requires_parameter(const uint8_t command) { + return + (command >= 0x60 && command < 0x80) || + (command == 0xc1) || (command == 0xc2) || + (command >= 0xd1 && command < 0xd5); + } + + static constexpr int performance_delay(const uint8_t command) { + if(requires_parameter(command)) { + return 3; + } + + switch(command) { + case 0xaa: return 5; + default: return 0; + } + } + void perform_command() { // Wait for a command. if(!has_command_) return; // Wait for a parameter if one is needed. - if( - (command_ >= 0x60 && command_ < 0x80) || - (command_ == 0xc1) || (command_ == 0xc2) || - (command_ >= 0xd1 && command_ < 0xd5) - ) { - if(!has_output_) { - return; - } + if(requires_parameter(command_) && !has_output_) { + return; } - { auto info = log_.info(); info.append("Keyboard command: %02x", command_); diff --git a/Machines/PCCompatible/PCCompatible.cpp b/Machines/PCCompatible/PCCompatible.cpp index d4a1e05fe..9b7cc6594 100644 --- a/Machines/PCCompatible/PCCompatible.cpp +++ b/Machines/PCCompatible/PCCompatible.cpp @@ -920,7 +920,7 @@ private: using namespace PCCompatible; namespace { -static constexpr bool ForceAT = false; +static constexpr bool ForceAT = true; template std::unique_ptr machine(const Target &target, const ROMMachine::ROMFetcher &rom_fetcher) { From 13b32b269c0293cd5504232b7ef4ff707d4c9edb Mon Sep 17 00:00:00 2001 From: Thomas Harte Date: Wed, 12 Mar 2025 12:47:04 -0400 Subject: [PATCH 16/16] Force the AT in debug mode only. --- Machines/PCCompatible/PCCompatible.cpp | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/Machines/PCCompatible/PCCompatible.cpp b/Machines/PCCompatible/PCCompatible.cpp index 9b7cc6594..375089782 100644 --- a/Machines/PCCompatible/PCCompatible.cpp +++ b/Machines/PCCompatible/PCCompatible.cpp @@ -920,7 +920,11 @@ private: using namespace PCCompatible; namespace { +#ifndef NDEBUG static constexpr bool ForceAT = true; +#else +static constexpr bool ForceAT = false; +#endif template std::unique_ptr machine(const Target &target, const ROMMachine::ROMFetcher &rom_fetcher) {