From 6d3e8e79681d17c771bc03a9bc84059b7de9cce9 Mon Sep 17 00:00:00 2001 From: Kelly Fox Date: Tue, 27 Oct 2026 15:31:18 -0500 Subject: [PATCH 1/5] firmware: a GeoRAM or REU save holds the computer while it writes, so the file is one instant The Alt-button save wrote up to 512K to SD while the computer kept writing the same banks through the IO handlers, so a save could mix before and after, such as a RAM disk whose directory and BAM disagree. WriteRAMBanksFile, shared by both saves, now stops the computer with the safe asynchronous freeze the Pause button uses, writes, and releases it. If the computer cannot be stopped, nothing is saved and the last save is kept. --- Source/Teensy/MinimalBoot/Common/DMAControl.c | 7 + .../MinimalBoot/Common/IO_Handlers/RAMBanks.c | 20 ++- Source/Teensy/tests/ram-banks.test.js | 52 ++++++ Source/Teensy/tests/ram_banks_harness.cpp | 152 ++++++++++++++++++ 4 files changed, 229 insertions(+), 2 deletions(-) create mode 100644 Source/Teensy/tests/ram-banks.test.js create mode 100644 Source/Teensy/tests/ram_banks_harness.cpp diff --git a/Source/Teensy/MinimalBoot/Common/DMAControl.c b/Source/Teensy/MinimalBoot/Common/DMAControl.c index 173e5d24..a82f02ca 100644 --- a/Source/Teensy/MinimalBoot/Common/DMAControl.c +++ b/Source/Teensy/MinimalBoot/Common/DMAControl.c @@ -260,6 +260,13 @@ FLASHMEM bool CloseDMA() return WaitForDMAState(DMA_S_DisableReady, DMA_HANDSHAKE_CEILING_mS); } +// Stops the computer on a safe cycle, with nothing to transfer, until CloseDMA. +FLASHMEM bool HoldC64() +{ + DMA_State = DMA_S_StartAsynch; + return WaitForDMAState(DMA_S_TransferReady, DMA_HANDSHAKE_CEILING_mS); +} + // A slice on a live C64 starts within DMA_TIMEOUT_CYCLES (~5 mS), so this holds a job to about // what its slice cap allows even on a bus that makes every handshake run to its own ceiling. #define DMA_SPANS_CEILING_mS (C64SlicesMax * 5 + 1000) diff --git a/Source/Teensy/MinimalBoot/Common/IO_Handlers/RAMBanks.c b/Source/Teensy/MinimalBoot/Common/IO_Handlers/RAMBanks.c index 2b0abd83..d6ebae6b 100644 --- a/Source/Teensy/MinimalBoot/Common/IO_Handlers/RAMBanks.c +++ b/Source/Teensy/MinimalBoot/Common/IO_Handlers/RAMBanks.c @@ -29,6 +29,8 @@ extern bool SDFullInit(); extern bool USBFileSystemWait(); extern FS *FSfromSourceID(RegMenuTypes SourceID); extern void FreeDriveDirMenu(); +extern bool HoldC64(); +extern bool CloseDMA(); extern uint8_t RAM_Image[]; extern uint8_t *ptrRAM_ImageEnd; // points to end of CRT-used RAM1 buffer @@ -128,9 +130,8 @@ FLASHMEM void LoadRAMBanks(uint8_t *Bank[], uint8_t NumBanks, const char *Source ReadRAMBanksFile(LoadFile, Bank, NumBanks); } -FLASHMEM static bool WriteRAMBanksFile(FS *sourceFS, const char *FileName, uint8_t *Bank[], uint8_t NumBanks) +FLASHMEM static bool WriteHeldRAMBanksFile(FS *sourceFS, const char *FileName, uint8_t *Bank[], uint8_t NumBanks) { - Serial.printf("Saving: %s\n", FileName); sourceFS->remove(FileName); // FILE_WRITE appends to an existing file File SaveFile = sourceFS->open(FileName, FILE_WRITE); if (!SaveFile) @@ -153,6 +154,21 @@ FLASHMEM static bool WriteRAMBanksFile(FS *sourceFS, const char *FileName, uint8 return true; } +// The computer writes the banks through the IO handlers while it runs, so a save it is not +// stopped for mixes the RAM before and after: a RAM disk whose directory and BAM disagree. +FLASHMEM static bool WriteRAMBanksFile(FS *sourceFS, const char *FileName, uint8_t *Bank[], uint8_t NumBanks) +{ + Serial.printf("Saving: %s\n", FileName); + if (!HoldC64()) + { + Serial.println("Unable to pause the computer!"); + return false; + } + bool Saved = WriteHeldRAMBanksFile(sourceFS, FileName, Bank, NumBanks); + CloseDMA(); + return Saved; +} + FLASHMEM static void FlashSavedLED() { uint32_t LastmS = millis(); diff --git a/Source/Teensy/tests/ram-banks.test.js b/Source/Teensy/tests/ram-banks.test.js new file mode 100644 index 00000000..6ad53023 --- /dev/null +++ b/Source/Teensy/tests/ram-banks.test.js @@ -0,0 +1,52 @@ +// SPDX-License-Identifier: MIT +// Saving GeoRAM or REU banks to SD while the computer keeps writing them. +import test from 'node:test'; +import assert from 'node:assert/strict'; +import fs from 'node:fs'; +import os from 'node:os'; +import path from 'node:path'; +import { spawnSync } from 'node:child_process'; + +import { commandExists } from '../../../tools/lib/toolchain.mjs'; + +const HARNESS_SOURCE = path.join(import.meta.dirname, 'ram_banks_harness.cpp'); + +const NOT_CI = ['', '0', 'false']; +const inCi = !NOT_CI.includes(process.env.CI ?? ''); +const skip = !commandExists('c++') && !inCi && 'c++ is not on PATH'; + +const BANKS_BYTES = 4 * 0x2000; +const PRELOAD = 'georam.ram 3'; + +let harnessPath; +function harness() { + if (harnessPath) return harnessPath; + const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'ram-banks-')); + harnessPath = path.join(dir, 'harness'); + const built = spawnSync('c++', ['-std=gnu++17', '-Wall', '-Wextra', '-Werror', '-Wno-deprecated-declarations', '-o', harnessPath, HARNESS_SOURCE], { encoding: 'utf8' }); + assert.equal(built.status, 0, built.stderr); + return harnessPath; +} + +function save(...args) { + const ran = spawnSync(harness(), args, { encoding: 'utf8' }); + assert.equal(ran.status, 0, ran.stderr); + return ran.stdout.split('\n').filter(Boolean); +} + +test('the REU save is the banks as they were when it began, and the computer runs again', { skip }, () => { + assert.deepEqual(save('numbered'), ['consistent', PRELOAD, `reu000.reu ${BANKS_BYTES}`]); +}); + +test('the GeoRAM save is the banks as they were when it began, and the computer runs again', { skip }, () => { + assert.deepEqual(save('replacing'), ['consistent', 'georam.bak 3', `georam.ram ${BANKS_BYTES}`]); +}); + +test('a computer that cannot be stopped is not saved, and the last save is kept', { skip }, () => { + assert.deepEqual(save('replacing', 'holdfails'), [PRELOAD]); + assert.deepEqual(save('numbered', 'holdfails'), [PRELOAD]); +}); + +test('a write the card refuses still lets the computer run again', { skip }, () => { + assert.deepEqual(save('replacing', 'writefails'), [PRELOAD, 'georam.tmp 8192']); +}); diff --git a/Source/Teensy/tests/ram_banks_harness.cpp b/Source/Teensy/tests/ram_banks_harness.cpp new file mode 100644 index 00000000..76f5331c --- /dev/null +++ b/Source/Teensy/tests/ram_banks_harness.cpp @@ -0,0 +1,152 @@ +// SPDX-License-Identifier: MIT +// ram_banks_harness: one save of RAM expansion banks to an in-memory SD card, with a computer +// that writes every bank between file writes unless its bus is held, as it does through the IO +// handlers while GeoRAM or the REU is installed. Arguments: +// numbered|replacing SaveRAMBanks (the REU's save) or SaveRAMBanksReplacing (GeoRAM's) +// [holdfails] HoldC64 cannot stop the computer +// [writefails] the card refuses the second bank's write +// Prints "consistent" when the saved file is the banks as they were when the save began, else +// "torn"; "held" if the computer is still stopped afterwards; then each file left on the card. +#include +#include +#include +#include +#include +#include +#include + +#define FLASHMEM +#define MaxPathLength 256 +#define RAM_ImageSize 0 +#define FILE_READ 0 +#define FILE_WRITE 1 +#define SetLEDOn +#define SetLEDOff + +static struct +{ + template void printf(const char *Format, Args... args) { std::fprintf(stderr, Format, args...); } + void println(const char *Line) { std::fprintf(stderr, "%s\n", Line); } +} Serial; + +static uint32_t Now; +static uint32_t millis() { return Now++; } + +enum RegMenuTypes { rmtSD, rmtUSBDrive, rmtTeensy }; +static uint8_t NumCrtChips; + +static constexpr int NumBanks = 4; +static uint8_t *Bank[NumBanks]; +static bool Held, HoldFails, WriteFails; +static int WritesSeen; + +static void ComputerRuns() +{ + if (Held) return; + for (int Num = 0; Num < NumBanks; Num++) Bank[Num][0]++; +} + +struct File +{ + std::vector *Data = nullptr; + size_t Pos = 0; + explicit operator bool() const { return Data != nullptr; } + size_t write(const uint8_t *Buf, size_t Len) + { + ComputerRuns(); + if (WriteFails && ++WritesSeen == 2) return 0; + Data->insert(Data->end(), Buf, Buf + Len); + return Len; + } + int read(uint8_t *Buf, size_t Len) + { + size_t Got = std::min(Len, Data->size() - Pos); + std::memcpy(Buf, Data->data() + Pos, Got); + Pos += Got; + return (int)Got; + } + int available() { return (int)(Data->size() - Pos); } + uint32_t size() { return (uint32_t)Data->size(); } + void close() { Data = nullptr; } +}; + +static struct FS +{ + std::map> Files; + File open(const char *Name, int Mode) + { + File Opened; + if (Mode == FILE_WRITE) Opened.Data = &Files[Name]; + else if (Files.count(Name)) Opened.Data = &Files[Name]; + return Opened; + } + bool exists(const char *Name) { return Files.count(Name) != 0; } + bool remove(const char *Name) { return Files.erase(Name) != 0; } + bool rename(const char *From, const char *To) + { + if (!exists(From) || exists(To)) return false; + Files[To] = Files[From]; + Files.erase(From); + return true; + } +} Card; + +static RegMenuTypes RegMenuTypeFromFileName(char **ptrptrFileName) +{ + *ptrptrFileName += strlen("SD:"); + return rmtSD; +} +static bool SDFullInit() { return true; } +static bool USBFileSystemWait() { return true; } +static FS *FSfromSourceID(RegMenuTypes) { return &Card; } +static void FreeDriveDirMenu() {} +uint8_t RAM_Image[1]; +uint8_t *ptrRAM_ImageEnd = RAM_Image; + +bool HoldC64() +{ + if (HoldFails) return false; + Held = true; + return true; +} + +bool CloseDMA() +{ + Held = false; + return true; +} + +// RAMBanks.c casts pointers to uint32_t, as wide as a pointer on the Teensy and not on the host +#define uint32_t uintptr_t +#include "../MinimalBoot/Common/IO_Handlers/RAMBanks.c" +#undef uint32_t + +int main(int argc, char **argv) +{ + if (argc < 2) return 2; + const bool Replacing = !strcmp(argv[1], "replacing"); + for (int Arg = 2; Arg < argc; Arg++) + { + HoldFails |= !strcmp(argv[Arg], "holdfails"); + WriteFails |= !strcmp(argv[Arg], "writefails"); + } + + std::vector AtStart; + for (int Num = 0; Num < NumBanks; Num++) + { + Bank[Num] = (uint8_t *)malloc(RAM_Bank_Size); + memset(Bank[Num], Num + 1, RAM_Bank_Size); + AtStart.insert(AtStart.end(), Bank[Num], Bank[Num] + RAM_Bank_Size); + } + Card.Files["georam.ram"] = std::vector(3, 0xee); + + if (Replacing) SaveRAMBanksReplacing(Bank, NumBanks, "SD:georam.ram"); + else SaveRAMBanks(Bank, NumBanks, "SD:reu.reu"); + + const char *Saved = Replacing ? "georam.ram" : "reu000.reu"; + if (Card.exists(Saved) && Card.Files[Saved].size() == AtStart.size()) + std::puts(Card.Files[Saved] == AtStart ? "consistent" : "torn"); + if (Held) std::puts("held"); + for (auto &Entry : Card.Files) std::printf("%s %zu\n", Entry.first.c_str(), Entry.second.size()); + return 0; +} From 4848e2a74148ca65128f910859a298f234162493 Mon Sep 17 00:00:00 2001 From: Kelly Fox Date: Tue, 27 Oct 2026 15:35:03 -0500 Subject: [PATCH 2/5] fix: review findings for 6d3e8e7 --- Source/Teensy/MinimalBoot/Common/IO_Handlers/RAMBanks.c | 9 +++++++++ Source/Teensy/tests/ram-banks.test.js | 5 +++++ Source/Teensy/tests/ram_banks_harness.cpp | 5 +++++ 3 files changed, 19 insertions(+) diff --git a/Source/Teensy/MinimalBoot/Common/IO_Handlers/RAMBanks.c b/Source/Teensy/MinimalBoot/Common/IO_Handlers/RAMBanks.c index d6ebae6b..72dff82e 100644 --- a/Source/Teensy/MinimalBoot/Common/IO_Handlers/RAMBanks.c +++ b/Source/Teensy/MinimalBoot/Common/IO_Handlers/RAMBanks.c @@ -29,8 +29,15 @@ extern bool SDFullInit(); extern bool USBFileSystemWait(); extern FS *FSfromSourceID(RegMenuTypes SourceID); extern void FreeDriveDirMenu(); +#ifdef Fab04_FullDMACapable extern bool HoldC64(); extern bool CloseDMA(); +#else +// No DMA to stop the computer with, so it is never saved, rather than saved torn. +static bool HoldC64() { return false; } +static bool CloseDMA() { return true; } +#endif +extern bool isFrozen; extern uint8_t RAM_Image[]; extern uint8_t *ptrRAM_ImageEnd; // points to end of CRT-used RAM1 buffer @@ -159,6 +166,8 @@ FLASHMEM static bool WriteHeldRAMBanksFile(FS *sourceFS, const char *FileName, u FLASHMEM static bool WriteRAMBanksFile(FS *sourceFS, const char *FileName, uint8_t *Bank[], uint8_t NumBanks) { Serial.printf("Saving: %s\n", FileName); + // Paused by the host, the computer is already held, and releasing it would resume it. + if (isFrozen) return WriteHeldRAMBanksFile(sourceFS, FileName, Bank, NumBanks); if (!HoldC64()) { Serial.println("Unable to pause the computer!"); diff --git a/Source/Teensy/tests/ram-banks.test.js b/Source/Teensy/tests/ram-banks.test.js index 6ad53023..232102ad 100644 --- a/Source/Teensy/tests/ram-banks.test.js +++ b/Source/Teensy/tests/ram-banks.test.js @@ -50,3 +50,8 @@ test('a computer that cannot be stopped is not saved, and the last save is kept' test('a write the card refuses still lets the computer run again', { skip }, () => { assert.deepEqual(save('replacing', 'writefails'), [PRELOAD, 'georam.tmp 8192']); }); + +test('a computer the host paused is saved and stays paused', { skip }, () => { + assert.deepEqual(save('replacing', 'paused'), ['consistent', 'held', 'georam.bak 3', `georam.ram ${BANKS_BYTES}`]); + assert.deepEqual(save('numbered', 'paused'), ['consistent', 'held', PRELOAD, `reu000.reu ${BANKS_BYTES}`]); +}); diff --git a/Source/Teensy/tests/ram_banks_harness.cpp b/Source/Teensy/tests/ram_banks_harness.cpp index 76f5331c..51192f76 100644 --- a/Source/Teensy/tests/ram_banks_harness.cpp +++ b/Source/Teensy/tests/ram_banks_harness.cpp @@ -5,6 +5,7 @@ // numbered|replacing SaveRAMBanks (the REU's save) or SaveRAMBanksReplacing (GeoRAM's) // [holdfails] HoldC64 cannot stop the computer // [writefails] the card refuses the second bank's write +// [paused] the host has paused the computer before the save // Prints "consistent" when the saved file is the banks as they were when the save began, else // "torn"; "held" if the computer is still stopped afterwards; then each file left on the card. #include @@ -16,6 +17,7 @@ #include #define FLASHMEM +#define Fab04_FullDMACapable #define MaxPathLength 256 #define RAM_ImageSize 0 #define FILE_READ 0 @@ -39,6 +41,7 @@ static constexpr int NumBanks = 4; static uint8_t *Bank[NumBanks]; static bool Held, HoldFails, WriteFails; static int WritesSeen; +bool isFrozen; static void ComputerRuns() { @@ -129,6 +132,7 @@ int main(int argc, char **argv) { HoldFails |= !strcmp(argv[Arg], "holdfails"); WriteFails |= !strcmp(argv[Arg], "writefails"); + isFrozen |= !strcmp(argv[Arg], "paused"); } std::vector AtStart; @@ -139,6 +143,7 @@ int main(int argc, char **argv) AtStart.insert(AtStart.end(), Bank[Num], Bank[Num] + RAM_Bank_Size); } Card.Files["georam.ram"] = std::vector(3, 0xee); + Held = isFrozen; if (Replacing) SaveRAMBanksReplacing(Bank, NumBanks, "SD:georam.ram"); else SaveRAMBanks(Bank, NumBanks, "SD:reu.reu"); From b1a3413f16d1a4fd532451c6f2ec32afa45dbb07 Mon Sep 17 00:00:00 2001 From: Kelly Fox Date: Tue, 27 Oct 2026 20:32:31 -0500 Subject: [PATCH 3/5] tests: the RAM-bank harness's LED macros are statements, so GCC's -Wextra accepts the else they end --- Source/Teensy/tests/ram_banks_harness.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Source/Teensy/tests/ram_banks_harness.cpp b/Source/Teensy/tests/ram_banks_harness.cpp index 51192f76..a32f50eb 100644 --- a/Source/Teensy/tests/ram_banks_harness.cpp +++ b/Source/Teensy/tests/ram_banks_harness.cpp @@ -22,8 +22,8 @@ #define RAM_ImageSize 0 #define FILE_READ 0 #define FILE_WRITE 1 -#define SetLEDOn -#define SetLEDOff +#define SetLEDOn do {} while (0) +#define SetLEDOff do {} while (0) static struct { From 604331d5d8a17d4b6eb6118f0166f3fca2344ac4 Mon Sep 17 00:00:00 2001 From: Kelly Fox Date: Thu, 29 Oct 2026 13:30:32 -0500 Subject: [PATCH 4/5] fix: review findings for b1a3413 --- Source/Teensy/MinimalBoot/Common/IO_Handlers/RAMBanks.c | 2 +- Source/Teensy/tests/ram_banks_harness.cpp | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/Source/Teensy/MinimalBoot/Common/IO_Handlers/RAMBanks.c b/Source/Teensy/MinimalBoot/Common/IO_Handlers/RAMBanks.c index 72dff82e..c53f30be 100644 --- a/Source/Teensy/MinimalBoot/Common/IO_Handlers/RAMBanks.c +++ b/Source/Teensy/MinimalBoot/Common/IO_Handlers/RAMBanks.c @@ -170,7 +170,7 @@ FLASHMEM static bool WriteRAMBanksFile(FS *sourceFS, const char *FileName, uint8 if (isFrozen) return WriteHeldRAMBanksFile(sourceFS, FileName, Bank, NumBanks); if (!HoldC64()) { - Serial.println("Unable to pause the computer!"); + Serial.println(PSTR("Unable to pause the computer!")); return false; } bool Saved = WriteHeldRAMBanksFile(sourceFS, FileName, Bank, NumBanks); diff --git a/Source/Teensy/tests/ram_banks_harness.cpp b/Source/Teensy/tests/ram_banks_harness.cpp index a32f50eb..f2f82c9f 100644 --- a/Source/Teensy/tests/ram_banks_harness.cpp +++ b/Source/Teensy/tests/ram_banks_harness.cpp @@ -17,6 +17,7 @@ #include #define FLASHMEM +#define PSTR(s) s #define Fab04_FullDMACapable #define MaxPathLength 256 #define RAM_ImageSize 0 From 518a2cfd1cdd49a0d5cbc61c1aa48545afdb350f Mon Sep 17 00:00:00 2001 From: Kelly Fox Date: Fri, 30 Oct 2026 13:15:36 -0500 Subject: [PATCH 5/5] tests: the RAM-bank harness builds and its output parses on Windows The harness brings its own strlcpy and strlcat, which MinGW-w64 and glibc before 2.38 lack, and the test splits the harness's output on \r?\n, since a MinGW build writes CRLF. Both as Travis suggested in review of #77. --- Source/Teensy/tests/ram-banks.test.js | 3 ++- Source/Teensy/tests/ram_banks_harness.cpp | 19 +++++++++++++++++++ 2 files changed, 21 insertions(+), 1 deletion(-) diff --git a/Source/Teensy/tests/ram-banks.test.js b/Source/Teensy/tests/ram-banks.test.js index 232102ad..b3a77900 100644 --- a/Source/Teensy/tests/ram-banks.test.js +++ b/Source/Teensy/tests/ram-banks.test.js @@ -31,7 +31,8 @@ function harness() { function save(...args) { const ran = spawnSync(harness(), args, { encoding: 'utf8' }); assert.equal(ran.status, 0, ran.stderr); - return ran.stdout.split('\n').filter(Boolean); + // A MinGW build of the harness writes its lines as \r\n: stdout is in text mode there. + return ran.stdout.split(/\r?\n/).filter(Boolean); } test('the REU save is the banks as they were when it began, and the computer runs again', { skip }, () => { diff --git a/Source/Teensy/tests/ram_banks_harness.cpp b/Source/Teensy/tests/ram_banks_harness.cpp index f2f82c9f..d32e144e 100644 --- a/Source/Teensy/tests/ram_banks_harness.cpp +++ b/Source/Teensy/tests/ram_banks_harness.cpp @@ -120,6 +120,25 @@ bool CloseDMA() return true; } +// RAMBanks.c uses strlcpy and strlcat, which MinGW and glibc before 2.38 do not have. These +// stand in for them on every host, so the build does not depend on the C library it finds; the +// #undefs cover a C library that makes them fortify macros. +static size_t harness_strlcpy(char *Dest, const char *Src, size_t Size) +{ + size_t Len = strlen(Src); + if (Size) { size_t Copy = Len < Size - 1 ? Len : Size - 1; memcpy(Dest, Src, Copy); Dest[Copy] = 0; } + return Len; +} +static size_t harness_strlcat(char *Dest, const char *Src, size_t Size) +{ + size_t Len = strnlen(Dest, Size); + return Len == Size ? Size + strlen(Src) : Len + harness_strlcpy(Dest + Len, Src, Size - Len); +} +#undef strlcpy +#undef strlcat +#define strlcpy harness_strlcpy +#define strlcat harness_strlcat + // RAMBanks.c casts pointers to uint32_t, as wide as a pointer on the Teensy and not on the host #define uint32_t uintptr_t #include "../MinimalBoot/Common/IO_Handlers/RAMBanks.c"