Enhanced Apple IIe problem writing to disks

[ryandesign]

I am experiencing a problem with Clock Signal 23.10.29 where it writes to Apple II disks incorrectly when emulating an enhanced Apple IIe. Steps to reproduce:

1. Use AppleCommander 1.9.0 to create a new blank DOS disk image:
   1. Launch the AppleCommander GUI.
   2. Click Create…
   3. Leave "DOS 3.3" selected and click Next >
   4. Enter a name for the disk and click Next >
   5. Leave "DOS ordered" selected and click Finish.
   6. Click Save.
   7. Choose where to save the disk and click Save.
2. Open the macOS version of Clock Signal and create a new Enhanced IIe.
3. Press Command-Shift-O and select the January 1983 Apple DOS 3.3 system master disk.
4. After it boots, press Command-Shift-O and select the blank disk created in step 1 above.
5. Type CATALOG and press Return and verify the catalog shows an empty disk.
6. Attempt to save the contents of the hires screen to disk with BSAVE HIRES,A$2000,L$2000. I/O ERROR occurs.
7. Try CATALOG again. I/O ERROR occurs.

The problem does not occur if in step 2 above I create an unenhanced Apple IIe instead. I cannot reproduce the problem in Open Emulator or Virtual ][ when emulating an enhanced IIe.

Bad enhanced IIe ROM file? Difference in the timing of 6502 and 65C02 instructions? Something else?

[ryandesign]

The use of a blank disk is not required. You see the same problem if you skip steps 1 and 4, saving the file directly to the system master disk. This will corrupt the system master disk so use a copy.

[ryandesign]

I see the same problem booting with Beagle Bros. ProntoDOS 1984-08-22 or ProDOS 2.4.2. (The ProDOS disk ships completely full. Emulating an unenhanced IIe, after starting BASIC.SYSTEM, I can remove a file (e.g. UNLOCK BLOCKWARDEN followed by DELETE BLOCKWARDEN) to make room and then BSAVE the hires page, but emulating an enhanced IIe, just deleting BLOCKWARDEN already causes the disk corruption and I/O error.)

Are you able to reproduce the issue or is it just me?

Do you have any ideas about how to debug this? Is there any way to log what the Disk II card and/or drive are doing and might that help pinpoint the problem, or do you already suspect a particular area of the code?

[TomHarte]

I feel like I have half a memory that upon diagnosis the issue was to do with different 65c02 behaviour on page crossings. But at the time I couldn't explain why the existing RWTS should work on a 65c02, making me confident that I wasn't at the real diagnosis. I then falsely believed the issue to have been resolved.

I'll try to stick my nose back in soon.

[ryandesign]

If I modify the code:

CLK/Machines/Apple/AppleII/AppleII.cpp

Line 81 in ed31cfd

(model == Analyser::Static::AppleII::Target::Model::EnhancedIIe) ? CPU::MOS6502::Personality::PSynertek65C02 : CPU::MOS6502::Personality::P6502,

and give all Apple II models a PSynertek65C02 then I see the problem with an unenhanced IIe as well. And similarly, if I change the enhanced Apple IIe to have a P6502 then I don't see this problem with the enhanced IIe.

However, I have a different reproduction recipe for what may be the same problem and it occurs on all Apple II models (with their original/correct processors): #1218.

[ryandesign]

Adding some logging to show the bytes that get loaded into the Disk II data latch:

--- a/Components/DiskII/DiskII.cpp
+++ b/Components/DiskII/DiskII.cpp
@@ -108,7 +108,7 @@ void DiskII::run_for(const Cycles cycles) {
 					return;
 				}
 			break;
-			case 0xb:	shift_register_ = data_input_;								break;	// load data register from data bus
+			case 0xb:	shift_register_ = data_input_;	fprintf(stderr, "%02x ", data_input_);			break;	// load data register from data bus
 		}
 
 		// Currently writing?

The datastream with a 6502 writing an empty file with SAVE HI looks plausible:

ff ff ff ff ff d5 aa ad 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 de aa eb ff ff ff ff ff ff d5 aa ad 9b 9b 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 ed ed 96 96 96 96 96 b7 b7 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 de aa eb ff ff ff ff ff ff d5 aa ad 96 96 96 96 96 96 96 96 96 96 96 96 9b 9a 97 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 9d a6 9b 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 96 de aa eb ff 

With a 65C02, it's much less plausible:

1d 1d 1d 1d 1d 1d 1d 1d bd bd bd bd bd bd bd bd bd bd bd bd bd bd bd bd bd bd bd bd bd bd bd bd bd bd bd bd bd bd bd bd bd bd bd bd bd bd bd bd bd bd bd bd bd bd bd bd bd bd bd bd bd bd bd bd bd bd bd bd bd bd bd bd bd bd bd bd bd bd bd bd bd bd bd bd bd bd bd bd bd bd bd bd bd bd bd bd bd bd bd bd bd bd bd bd bd bd bd bd bd bd bd bd bd bd bd bd bd bd bd bd bd bd bd bd bd bd bd bd bd bd bd bd bd bd bd bd bd bd bd bd bd bd bd bd bd bd bd bd bd bd bd bd bd bd bd bd bd bd bd bd bd bd bd bd bd bd bd bd bd bd bd bd bd bd bd bd bd bd bd bd bd bd bd bd bd bd bd bd bd bd bd bd bd bd bd bd bd bd bd bd bd bd bd bd bd bd bd bd bd bd bd bd bd bd bd bd bd bd bd bd bd bd bd bd bd bd bd bd bd bd bd bd bd bd bd bd bd bd bd bd bd bd bd bd bd bd bd bd bd bd bd bd bd bd bd bd bd bd bd bd bd bd bd bd bd bd bd bd bd bd bd bd bd bd bd bd bd bd bd bd bd bd bd bd bd bd bd bd bd bd bd bd bd bd bd bd bd bd bd bd bd bd bd bd bd bd bd bd bd bd bd bd bd bd bd bd bd bd bd bd bd bd bd bd bd bd bd bd bd bd bd bd bd bd bd bd bd bd bd bd bd bd bd bd bd bd bd bd bd bd 1d 1d 1d 1d 1d 

[ryandesign]

I feel like I have half a memory that upon diagnosis the issue was to do with different 65c02 behaviour on page crossings.

I agree... Working my way through the places in the code that vary based on is_65c02(), I found this one change that makes disk writing work on the 65C02 as it does on the 6502:

--- a/Processors/6502/Implementation/6502Implementation.hpp
+++ b/Processors/6502/Implementation/6502Implementation.hpp
@@ -530,7 +530,7 @@ template <Personality personality, typename T, bool uses_ready_line> void Proces
 // MARK: - Addressing Mode Work
 
 #define page_crossing_stall_read()	\
-	if(is_65c02(personality)) {	\
+	if(false) {	\
 		throwaway_read(pc_.full - 1);	\
 	} else {	\
 		throwaway_read(address_.full);	\

Some possibly interesting information:

https://groups.google.com/g/comp.sys.apple2/c/RuTGaRxu5Iw/m/z_1zklRuDAwJ

65SC02, compared to NMOS 6502, generally tries to replace dummy I/O with
something safe. One notable exception is keeping RWTS happy (STA abs,X),
but there are also few surprises here and there.

STA abs,X has been fixed for the PX (page-crossing) case by adding a dummy
read of the program counter, so the change was rW -> W. In the non-PX case
it still reads the destination address, so there is no change: RW -> RW.

** The decision by the 65C02 designers to keep the STA abs,X non-PX
behaviour must have been because of the disk controller soft switches in the
Apple II. No other explanation makes sense. **

https://comp.sys.apple2.programmer.narkive.com/79Uzs2i6/phantom-reads-on-6502-65c02-and-65816

Another aspect: "Everybody" talks about STA abs,x compatibility with
Woz' RWTS. But what about STA abs,y (presumably not used in RWTS to
access I/O)? In all cases identical to STA abs,x?

1. STA $C080,X with X=$60 will read from $C0E0 before writing to $C0E0 on
   6502, 65c02, 65816.
2. STA $BFFF,X wth X=$60 will read from $BF5F on a 6502 and 65816, but will
   not on a 65c02.
3. STA ($10),y with $10=$c080 and Y=$60 will read $c0e0 before writing
   $c0e0 on 6502/65c02/65816.

What's confusing is there are some models of 65c02 which may have a fix for
some of this behavior. Those 65c02's were generally not used in Apples (since
they mess up RWTS). I suppose it's possible they were used in the Apple IIc
or IIc+ since those models moved to the IWM which could be more forgiving
of these bus accesses (and had no slots, so didn't require strict Disk II
compatibility).

The original 65816 was a buggy mess which also fixed these behaviors as
well--and the description of this obsolete 65816 is given in the Cortland
documentation someone else posted--so then it appears WDC went and put back
all of the 6502 quirks, and that's the 65816 that got used in the Apple IIgs.

Does your code observe these rules?

[TomHarte]

The linked pull request attempts to alter STA abs, x behaviour on the 65c02 to match my new understanding, assuming I read the linked material properly. I'll try to give it a proper testing.

[ryandesign]

Thanks! I'll check it out too.

Upon rereading, I see I quoted the question "what about STA abs,y" but not the answer. I think the answer was that STA abs,y should read then write just as STA abs,x does. And STA (zp),y should as well. Code for test programs was given to check this behavior, but I don't have a real 65C02 on hand so at best I could test what other emulators have implemented.

[ryandesign]

This change does seem to work to fix RWTS. Great!

In the threads I mentioned, they point out where in Sather's Understanding the Apple IIe you can find tables documenting exactly what's on the address and data bus for each cycle of each instruction in the 6502 (page 4-23, table 4.1) and 65C02 (page 4-27, table 4.3), with the latter using bold print to indicate where it differs from the former.

Your PR only changes STA abs,x so that takes care of line 22 of tables 4.1 and 4.3 ("STA $5772,X (NO PX)").

At the bottom of page 4-24 it says:

Y-indexed instructions are represented by X-indexed examples when Y-indexed execution is identical to X-indexed execution.

and table 4.2 on that page confirms, on the STA line, in the ABS X and ABS Y columns, that the implementation for both is the same. So STA abs,y should be changed the same way. Table 4.4 on page 4-28 shows STZ abs,x should be changed the same way.

From line 23 of tables 4.1 and 4.3 ("ASL $5772,X (NO PX)") it looks like ASL abs,x should also be changed the same way. And from the tables 4.2 and 4.4 (page 4-28), LSL, ROL, ROR, DEC, and INC with abs,x should also be changed.

Looks like I was wrong about STA (zp),y: according to line 28 of the tables ("STA ($70),Y (NO PX)"), that opcode was indeed fixed on the 65C02 so your code is already correct for that case.

[cybernesto]

I just faced this issue as well. I hope it can be fixed in a release soon.