From edef0732acf5a57619a76b1be9adc3b2bb12cb80 Mon Sep 17 00:00:00 2001 From: Thomas Harte Date: Mon, 4 Dec 2023 12:19:21 -0500 Subject: [PATCH 1/2] Avoid potential infinite read loops. --- Machines/PCCompatible/PCCompatible.cpp | 56 ++++++++++++-------------- 1 file changed, 25 insertions(+), 31 deletions(-) diff --git a/Machines/PCCompatible/PCCompatible.cpp b/Machines/PCCompatible/PCCompatible.cpp index d525086c2..02720448f 100644 --- a/Machines/PCCompatible/PCCompatible.cpp +++ b/Machines/PCCompatible/PCCompatible.cpp @@ -114,16 +114,15 @@ class FloppyController { decoder_.geometry().head, decoder_.geometry().cylinder, decoder_.geometry().sector); -// log = true; status_.begin(decoder_); // Search for a matching sector. auto target = decoder_.geometry(); -// bool found_sector = false; - bool complete = false; while(!complete) { + bool found_sector = false; + for(auto &pair: drives_[decoder_.target().drive].sectors(decoder_.target().head)) { if( (pair.second.address.track == target.cylinder) && @@ -131,19 +130,15 @@ class FloppyController { (pair.second.address.side == target.head) && (pair.second.size == target.size) ) { -// found_sector = true; -// bool wrote_in_full = true; + found_sector = true; + bool wrote_in_full = true; - printf("Writing data beginning: "); for(int c = 0; c < 128 << target.size; c++) { - if(c < 8) printf("%02x ", pair.second.samples[0].data()[c]); - const auto access_result = dma_.write(2, pair.second.samples[0].data()[c]); switch(access_result) { default: break; case AccessResult::NotAccepted: - printf("FDC: DMA not permitted\n"); -// wrote_in_full = false; + wrote_in_full = false; break; case AccessResult::AcceptedWithEOP: complete = true; @@ -154,18 +149,23 @@ class FloppyController { } } - ++target.sector; // TODO: multitrack? - printf("\n"); + if(!wrote_in_full) { + status_.set(Intel::i8272::Status1::OverRun); + status_.set(Intel::i8272::Status0::AbnormalTermination); + break; + } -// if(wrote_in_full) { -// } else { -// printf("FDC: didn't write in full\n"); -// // TODO: Overrun, presumably? -// } + ++target.sector; // TODO: multitrack? break; } } + + if(!found_sector) { + status_.set(Intel::i8272::Status1::EndOfCylinder); + status_.set(Intel::i8272::Status0::AbnormalTermination); + break; + } } results_.serialise( @@ -175,18 +175,6 @@ class FloppyController { decoder_.geometry().sector, decoder_.geometry().size); -// if(!found_sector) { -// printf("FDC: sector not found\n"); -// // TODO: there's more than this, I think. -// status_.set(Intel::i8272::Status0::AbnormalTermination); -// results_.serialise( -// status_, -// decoder_.geometry().cylinder, -// decoder_.geometry().head, -// decoder_.geometry().sector, -// decoder_.geometry().size); -// } - // TODO: what if head has changed? drives_[decoder_.target().drive].status = decoder_.drive_head(); drives_[decoder_.target().drive].raised_interrupt = true; @@ -281,6 +269,12 @@ class FloppyController { } void set_disk(std::shared_ptr disk, int drive) { + if(drives_[drive].disk) { + // TODO: drive should only transition to unready if it was ready in the first place. + drives_[drive].status = uint8_t(Intel::i8272::Status0::BecameNotReady); + drives_[drive].raised_interrupt = true; + pic_.apply_edge<6>(true); + } drives_[drive].disk = disk; } @@ -833,8 +827,8 @@ class IO { case 0x03b1: case 0x03b3: case 0x03b5: case 0x03b7: if constexpr (std::is_same_v) { - mda_.write<1>(value); - mda_.write<0>(value >> 8); + mda_.write<1>(uint8_t(value)); + mda_.write<0>(uint8_t(value >> 8)); } else { mda_.write<1>(value); } From bf8a4b7efe901d4f8e792927dbaee9413929c3f7 Mon Sep 17 00:00:00 2001 From: Thomas Harte Date: Mon, 4 Dec 2023 12:28:29 -0500 Subject: [PATCH 2/2] Ensure sector cache is cleared upon disk change. --- Machines/PCCompatible/PCCompatible.cpp | 29 +++++++++++++++++++------- 1 file changed, 21 insertions(+), 8 deletions(-) diff --git a/Machines/PCCompatible/PCCompatible.cpp b/Machines/PCCompatible/PCCompatible.cpp index 02720448f..905dd7997 100644 --- a/Machines/PCCompatible/PCCompatible.cpp +++ b/Machines/PCCompatible/PCCompatible.cpp @@ -269,13 +269,13 @@ class FloppyController { } void set_disk(std::shared_ptr disk, int drive) { - if(drives_[drive].disk) { - // TODO: drive should only transition to unready if it was ready in the first place. - drives_[drive].status = uint8_t(Intel::i8272::Status0::BecameNotReady); - drives_[drive].raised_interrupt = true; - pic_.apply_edge<6>(true); - } - drives_[drive].disk = disk; +// if(drives_[drive].has_disk()) { +// // TODO: drive should only transition to unready if it was ready in the first place. +// drives_[drive].status = uint8_t(Intel::i8272::Status0::BecameNotReady); +// drives_[drive].raised_interrupt = true; +// pic_.apply_edge<6>(true); +// } + drives_[drive].set_disk(disk); } private: @@ -317,7 +317,14 @@ class FloppyController { bool motor = false; bool exists = true; - std::shared_ptr disk; + bool has_disk() const { + return bool(disk); + } + + void set_disk(std::shared_ptr image) { + disk = image; + cached.clear(); + } Storage::Encodings::MFM::SectorMap §ors(bool side) { if(cached.track == track && cached.side == side) { @@ -356,7 +363,13 @@ class FloppyController { uint8_t track = 0xff; bool side; Storage::Encodings::MFM::SectorMap sectors; + + void clear() { + track = 0xff; + sectors.clear(); + } } cached; + std::shared_ptr disk; } drives_[4];