-
Notifications
You must be signed in to change notification settings - Fork 2.2k
wozfdc.cpp: improve timing of data register reads #14404
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
When one of the even-numbered c08x locations is read, the FDC returns the value
of its data register. However, its LSS (logic state sequencer) runs fast enough
that between when the CPU puts the address on the bus and when it reads the
result, it manages to complete one cycle. This is explained in Understanding the
Apple II, page 9-22.
This patch emulates this behavior. Its effect can be seen with the INIT command
in DOS 3.2 and earlier. Without the patch, the command fails with an I/O error;
with the patch, it succeeds. The reason is that it checks if the disk is
write-protected after formatting every track, so it executes the following
instructions while the FDC is in write mode:
lda $c08d,x
lda $c08e,x
bmi error
The way it's supposed to work is:
1. The second LDA instruction switches the FDC to "check write protect" mode.
2. The LSS runs for 1 cycle, which loads the write-protect status into the data
register.
3. The data register is copied into A, which puts the write-protect status into
the N flag.
4. The BMI instruction tests the status.
Without step 2, the N flag is loaded with whatever was in the high bit of the
data register, which seems to be 1 more often than not, so DOS thinks the disk
is write-protected, and aborts.
| address |= 0x10; | ||
|
|
||
| uint64_t cycles_limit = time_to_cycles(machine().time()) + extra_cycles; | ||
| assert(cycles <= cycles_limit); // make sure we aren't going back in time |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This assert is firing in 0.282, if you use a PAL config:
appulatord_282 apple2eefr -flop1 <anydisk.po>
It fires right around the 1 second mark into execution, going backwards one cycle after a generic_tick callback, into a device read.
In a non-assert build, the PAL config will corrupt disks immediately when writing.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Aw, dangit. I assume this doesn't happen without the lss_sync(1) call?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The assert still fires with lss_sync(1) removed.
I also went back to 0.279: the write corruption happens there too, and adding (only) the assert, it fires there too.
So, this PR isn't introducing a regression, it is exposing an existing bug.
(A pretty bad bug, since the PAL configs destroy disks.)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tracking disk corruption in #14474
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I assume you mean #14474.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Whoops, copy-paste-o, thanks.
When one of the even-numbered c08x locations is read, the FDC returns the value of its data register. However, its LSS (logic state sequencer) runs fast enough that between when the CPU puts the address on the bus and when it reads the result, it manages to complete one cycle. This is explained in Understanding the Apple II, page 9-22.
This patch emulates this behavior. Its effect can be seen with the INIT command in DOS 3.2 and earlier. Without the patch, the command fails with an I/O error; with the patch, it succeeds. The reason is that it checks if the disk is write-protected after formatting every track, so it executes the following instructions while the FDC is in write mode:
The way it's supposed to work is:
Without step 2, the N flag is loaded with whatever was in the high bit of the data register, which seems to be 1 more often than not, so DOS thinks the disk is write-protected, and aborts.