GeoRAM and REU saves hold the computer while they write

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)

Source/Teensy/MinimalBoot/Common/IO_Handlers/RAMBanks.c

@@ -29,6 +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
 
@@ -128,9 +137,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 +161,23 @@ 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);
+   // 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(PSTR("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();

Source/Teensy/tests/ram-banks.test.js

@@ -0,0 +1,58 @@
+// 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);
+  // 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 }, () => {
+  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']);
+});
+
+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}`]);
+});

Source/Teensy/tests/ram_banks_harness.cpp

@@ -0,0 +1,177 @@
+// 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
+//   [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 <cstdint>
+#include <cstdio>
+#include <cstdlib>
+#include <cstring>
+#include <map>
+#include <string>
+#include <vector>
+
+#define FLASHMEM
+#define PSTR(s) s
+#define Fab04_FullDMACapable
+#define MaxPathLength 256
+#define RAM_ImageSize 0
+#define FILE_READ 0
+#define FILE_WRITE 1
+#define SetLEDOn do {} while (0)
+#define SetLEDOff do {} while (0)
+
+static struct
+{
+   template <typename... Args> 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;
+bool isFrozen;
+
+static void ComputerRuns()
+{
+   if (Held) return;
+   for (int Num = 0; Num < NumBanks; Num++) Bank[Num][0]++;
+}
+
+struct File
+{
+   std::vector<uint8_t> *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<std::string, std::vector<uint8_t>> 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 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"
+#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");
+      isFrozen |= !strcmp(argv[Arg], "paused");
+   }
+
+   std::vector<uint8_t> 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<uint8_t>(3, 0xee);
+   Held = isFrozen;
+
+   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;
+}