ROM 920379 uses zero page $b0 in GO64 mode, which is used by other programs

[snoopy-f64]

Test Environment (required)
Current Xemu

Describe the bug
In Forum64 the user @godot, the author of the BASIC Extension TSB for the C64 found an error in the C64 mode of MEGA65. It only occurs in ROM versions 920380 and newer.

GoDot describes here:

https://www.forum64.de/index.php?thread/128470-mega65-detection-in-c64-mode/&postID=2045506#post2045506

that it could be an issue with zeropage address $b0 in C64 mode.

To Reproduce
Steps to reproduce the behavior:

1. Switch to C64 mode
2. Load TSB and then use DIR

Expected behavior
DIR should show the directory:

With ROM 920380 and newer the DIR command of TSB only shows the title of the directory:

[dansanderson]

So 920379 works?

There are no changes to the C64 part of the ROM from 920379 to 920380, so there should not be any changes to how the C64 ROM uses the KERNAL variable space (and we'd never change KERNAL variable use in the C64 ROM intentionally). MEGA65 mode and C64 mode do not share KERNAL variables.

I'm less sure about how the two modes share DOS code, though. This change might be suspect, but it went in with 920379 (and was the sole change for that release), not 920380: https://github.com/MEGA65/mega65-rom/commit/49c7a845ec3ab81a04774f36759da0476d74737d

[snoopy-f64]

I have just tested it:

The last ROM version which works fine with the DIR command of TSB in C64 mode is 920378.

From 920379 it doesn't work anymore.

[dansanderson]

Yeah, it's the CBDOS change in 920379 for sure. It relocates some CBDOS variables to zero page $b0 in GO64 mode. This is an unfortunate choice. While that region may be unused by the KERNAL as far as there not being a tape device present, it's common for C64 programs to repurpose that space precisely because it's typically unused. This is the only substantial change we've made to the C64 KERNAL piece of the MEGA65 ROM (other than the banner change, which should be harmless).

Prior to the change, the DOS routines used $dff2-$dffb for similar purposes. I don't think it's as easy as going back to doing that, but we could look into it. This does share DOS code with MEGA65 mode, and this change was part of a larger refactoring effort.

[ki-bo]

So this is not a change to the C64 kernel but rather to the DOS, which was not present on a real C64 (being part of the disk drive there). Can we move those variables into the DOS allocated block at $10000 instead? This is not something programmer expect to be available.

[dansanderson]

I finally got around to reviewing this code in detail. One important clarification: the new C64 ZP dependency here is $b0-$b3, which is storing the cbdos pointer to $1.006c-$1.007f, which is where the CBDOS comm area actually lives in the revision. This is used exclusively to access the CBDOS comm area via 32-bit indirect addressing from specific KERNAL routines. It's not the comm area location that's the problem, it's only this pointer, and only for the C64 KERNAL.

Background: The CBDOS comm area is 20 bytes of variable space used to pass values between the KERNAL and CBDOS. CBDOS is replicating the behavior of what would otherwise be running on a separate 6502 device, namely the disk drive. The KERNAL uses MAPLO=$(1)001 during CBDOS code, so it sees the comm area as if it were in the ZP in bank 0, but it's actually in bank 1. The MAPLO setting is managed by a MAP fence in the KERNAL: in kernel64.src, this is dos_in and dos_out. These routines, plus dos_setstatus, access the CBDOS comm area outside the MAP fence using the cbdos pointer. The pointer must be located in the KERNAL ZP to use 32-bit indirect addressing. (cbdos itself cannot live in the CBDOS comm area because it is a pointer to the CBDOS comm area.)

As far as I can tell, only dos_in, dos_out, and dos_setstatus access the CBDOS comm area via the cbdos pointer. dos_in currently sets cbdos first thing (cf. set_cbdos_pointer), clobbering whatever is there. We could wrap the uses of cbdos with code that stashes its current values of $b0-$b3 on the stack, sets the pointer, then restores the original values when it's done. To be safe, both stash and restore should happen in all three of dos_in, dos_out, and dos_setstatus.

The MEGA65 KERNAL has similar routines and code. I don't see a need to change the C65 KERNAL, because we still have a private API contract over the MEGA65 KERNAL ZP and the space for cbdos is already allocated. Only the C64 KERNAL needs to protect $b0-$b3.

This'll be straightforward to test, I just wanted to write out my conclusion in prose first to see if it sounds right. 😅

[dansanderson]

(OK easier said than done, considering part of what's happening with the comm area in the first place is restoring the KERNAL stack pointer and caller return address... 😬)

[dansanderson]

I'm considering just stashing $b0-$b3 to $100-$103 while it's in use for CBDOS. Technically that would break the stack if it were full, but so would pushing these bytes onto the stack properly.

Someone talk me out of it if it's a bad idea. It's difficult to know all of the ways C64 software fills all the little nooks unused by the C64 KERNAL, and we're really just repairing one edge case by potentially interfering with what would hopefully be a more rare edge case, e.g. maybe someone already uses $100-$103 for sneaky temporary storage.

I don't yet understand how the previous version of this used the "I/O area" of the C64 memory map north of $DE00. Maybe I could get away with borrowing a few bytes from there, if $100-$103 is inappropriate. I believe the CBDOS comm region used to live there, and was moved to make space for 80x50 mode color RAM in C65 mode. This workaround would be exclusive to the C64 KERNAL, so maybe it can still use that space.

[ki-bo]

I wouldn't be surprised if C64 software is doing the same trick reusing the lower part of the stack for other data, also assuming it won't conflict there with regular stack operations.

Maybe we should review again how it was done when all was still working. Still did not get that so far.

[dansanderson]

Previously the CBDOS comm area was located at $dff2-$dfff. In the C64 memory map, this is in a region referred to as "I/O area #2". In the C65 memory map, this memory is accessed with CRAM2K enabled such that it lived in color RAM. I assume one motivation for relocating it to bank 1 was 80x50 mode, which needs to use that color RAM as color RAM.

I should be able to do this copy/restore of $b0-$b3 to $dffc-$dfff in the C64 version. This at least would not be a regression of using those bytes for CBDOS purposes, however OK that ever was.

[dansanderson]

OK, I think I have a fix. At least it resolves the issue with TSB in GO64 mode as described above. :)

[dansanderson]

I'm resolving this as fixed.

Testers, remember that the latest ROM master has a dependency on the dev core that is not yet emulated in Xemu "stable." The Xemu "next" branch does have partial support for the new keyboard scanner and I am able to test the latest ROM and reproduce the TSB fix there. (The new keyboard emulation still needs work but it's functional enough to complete this test.)