Code review 2026-09-03, branch 2: machines F-20 onward

.github/workflows/clang-format.yml

@@ -31,3 +31,16 @@ jobs:
         run: |
           find src -name '*.c' -o -name '*.h' \
             | xargs clang-format-18 --dry-run --Werror
+
+      # STYLE_GUIDE.md "File Structure": every source file begins with the
+      # two-line copyright notice.  The tree is clean; this keeps it that way
+      # (the 2026-09-03 review found four files missing it, and F-26 a fifth).
+      - name: Check SPDX headers (src/)
+        run: |
+          missing=$(find src \( -name '*.c' -o -name '*.h' \) \
+            -exec sh -c 'head -1 "$1" | grep -q "^// SPDX-License-Identifier:" || echo "$1"' _ {} \;)
+          if [ -n "$missing" ]; then
+            echo "Files whose first line is not an SPDX identifier:"
+            echo "$missing"
+            exit 1
+          fi

src/core/cpu/ppc/ppc_fpu.c

@@ -230,7 +230,7 @@ void ppc_do_mcrfs(ppc_t *p, uint32_t iw) {
 // are never writable and re-derive.  mtfsb0/mtfsb1 (folios 10-131/132):
 // bits 1 and 2 cannot be explicitly written.
 void ppc_do_mtfsf(ppc_t *p, uint32_t iw) {
-    uint32_t m = ppc_crm_mask((iw >> 17) & 0xFFu) & ~PPC_FPSCR_UNWRITABLE;
+    uint32_t m = ppc_crm_mask((iw >> 17) & 0xFFu) & ~ppc_fpscr_nowrite(p);
     p->fpscr = ppc_fpscr_derive(((uint32_t)p->fpr[PPC_RB(iw)] & m) | (p->fpscr & ~m));
     if (PPC_RC(iw))
         ppc_set_cr_field(p, 1, p->fpscr >> 28);
@@ -239,7 +239,7 @@ void ppc_do_mtfsf(ppc_t *p, uint32_t iw) {
 
 void ppc_do_mtfsfi(ppc_t *p, uint32_t iw) {
     uint32_t sh = 28 - 4 * PPC_CRFD(iw);
-    uint32_t m = (0xFu << sh) & ~PPC_FPSCR_UNWRITABLE;
+    uint32_t m = (0xFu << sh) & ~ppc_fpscr_nowrite(p);
     p->fpscr = ppc_fpscr_derive(((((iw >> 12) & 0xFu) << sh) & m) | (p->fpscr & ~m));
     if (PPC_RC(iw))
         ppc_set_cr_field(p, 1, p->fpscr >> 28);
@@ -253,10 +253,15 @@ void ppc_do_mtfsfi(ppc_t *p, uint32_t iw) {
 // registers altered" line names only FPSCR[crbD], but that list also omits
 // the derived VX, so it reads as a summary rather than an exhaustive action
 // list — unlike §5.4.7.4.1, which IS one and does override the same table
-// for FR/FI on disabled overflow.  powerpc-test's model agrees.)
+// for FR/FI on disabled overflow.)
+//
+// The transition rule is what makes this conditional: ppc_fpscr_raise sets
+// FX only when the bit was previously 0.  And on the 601, VXSOFT and VXSQRT
+// are not implemented at all (ppc_fpscr_nowrite), so mtfsb1 of either is a
+// no-op there and sets nothing — FX included.
 void ppc_do_mtfsb(ppc_t *p, uint32_t iw, bool set) {
     uint32_t bit = 0x80000000u >> PPC_RT(iw);
-    if (!(bit & PPC_FPSCR_UNWRITABLE)) {
+    if (!(bit & ppc_fpscr_nowrite(p))) {
         if (!set)
             p->fpscr &= ~bit;
         else if (bit & PPC_FPSCR_EXCEPTIONS)

src/core/cpu/ppc/ppc_internal.h

@@ -16,6 +16,7 @@
 
 #include "machine_profile.h" // CPU_MODEL_PPC601 / CPU_MODEL_PPC604
 #include "memory.h"
+#include "ppc_softfp.h" // FPSCR bit masks (leaf header: stdint only)
 
 #include <assert.h>
 #include <stdbool.h>
@@ -200,6 +201,27 @@ static inline uint32_t ppc_msr_mask(const ppc_t *p) {
     return ppc_is_604(p) ? PPC_MSR_MASK_604 : PPC_MSR_MASK;
 }
 
+// FPSCR bits no "move to FPSCR" instruction may write on the active model.
+// Always FEX and VX (derived summaries — MPCFPE32B Table 2-1: "cannot alter
+// explicitly"), plus VXSOFT and VXSQRT on the 601, which 601UM Table 2-1
+// marks "Not implemented in the 601" (bits 21 and 22).
+//
+// Not implemented means the bits do not exist, not merely that hardware
+// never raises them: VXSOFT can ONLY ever be set by software — MPCFPE32B
+// bit 21, "can be altered only by the mcrfs, mtfsfi, mtfsf, mtfsb0, or
+// mtfsb1 instructions" — so if the 601 held the storage the bit would be
+// fully functional and there would be nothing to call unimplemented.  The
+// same row style marks VXSQRT, whose purpose is likewise to let software
+// simulate the fsqrt/frsqrte the 601 does not have (601UM Table 5-17).
+//
+// Consequence for mtfsb1: writing an unimplemented bit is a no-op, so it
+// causes no 0->1 transition and therefore does NOT set FX.  The 604 keeps
+// both bits — the 604UM has no FPSCR table of its own and defers to the
+// architecture, where bits 21 and 22 are ordinary sticky bits.
+static inline uint32_t ppc_fpscr_nowrite(const ppc_t *p) {
+    return PPC_FPSCR_UNWRITABLE | (ppc_is_604(p) ? 0u : (PPC_FPSCR_VXSOFT | PPC_FPSCR_VXSQRT));
+}
+
 // MSR bits an exception entry preserves: ME and EP on both models, plus PM
 // on the 604 (unlisted in the 604UM per-exception MSR rows — unaltered).
 static inline uint32_t ppc_msr_exception_keep(const ppc_t *p) {

src/core/cpu/ppc/ppc_softfp.h

@@ -44,8 +44,8 @@
 #define PPC_FPSCR_C      0x00010000u // bit 15: result class descriptor
 #define PPC_FPSCR_FPCC   0x0000F000u // bits 16-19: FL/FG/FE/FU
 #define PPC_FPSCR_FPRF   0x0001F000u // bits 15-19: C + FPCC
-#define PPC_FPSCR_VXSOFT 0x00000400u // bit 21: software-request invalid (601: storage only)
-#define PPC_FPSCR_VXSQRT 0x00000200u // bit 22: invalid sqrt (601: storage only)
+#define PPC_FPSCR_VXSOFT 0x00000400u // bit 21: software-request invalid (not on the 601)
+#define PPC_FPSCR_VXSQRT 0x00000200u // bit 22: invalid sqrt (not on the 601)
 #define PPC_FPSCR_VXCVI  0x00000100u // bit 23: invalid integer convert
 #define PPC_FPSCR_VE     0x00000080u // bit 24: invalid-op exception enable
 #define PPC_FPSCR_OE     0x00000040u // bit 25: overflow exception enable

src/core/machine_profile.h

@@ -198,7 +198,8 @@ typedef struct media_slot {
 
 // Machine lifecycle + host-input vtable.  The behavior half of a machine
 // (proposal §4.4): hw_profile_t is pure descriptor DATA and points at one of
-// these.  system.c / nubus.c dispatch through it; every hook is NULL-safe.
+// these.  system.c / nubus.c / pci.c dispatch through it; every hook is
+// NULL-safe.
 // (memory_layout_init and checkpoint_restore are deliberately absent — they
 // were never dispatched: each init runs its own layout directly and restore
 // is folded into init.)
@@ -214,15 +215,20 @@ typedef struct machine_substrate {
     void (*teardown)(struct config *cfg);
     void (*checkpoint_save)(struct config *cfg, checkpoint_t *cp);
 
-    void (*update_ipl)(struct config *cfg, int source, bool active); // NuBus IRQ routing
     void (*trigger_vbl)(struct config *cfg);
 
     // Drive NuBus slot `slot` ($9..$E) /NMRQ active/inactive.  `umbrella_edge`
     // is true when this transition flips the "any slot asserted" aggregate.
-    // Every NuBus machine implements it (GLUE → VIA2 port-A bit + CA1 on the
-    // umbrella edge; MDU/OSS → the chipset's own IRQ controller via update_ipl);
-    // keeps nubus.c machine-agnostic — no cfg->via2 poke (proposal §4.4).  NULL
-    // on non-NuBus machines (Plus / Lisa), which never reach it.
+    // Every NuBus machine implements it, and each converts the slot number to
+    // its own controller's numbering ITSELF: GLUE → VIA2 port-A bit + CA1 on
+    // the umbrella edge; MCU → VIA2 PA1-5 + /SLOTIRQ; MDU → the RBV's slot
+    // register; OSS → OSS source bits; AV → PSC SInt bits 3-5; PDM → BART.
+    // There is deliberately no shared "convert slot to an IRQ source mask"
+    // helper: slot numbering matches a machine's interrupt-source numbering
+    // only by coincidence, and the one that existed put a IIci's slot $C on
+    // its NMI source (mdu.c).  Keeps nubus.c machine-agnostic — no cfg->via2
+    // poke (proposal §4.4).  NULL on non-NuBus machines (Plus / Lisa), which
+    // never reach it.
     void (*nubus_slot_irq)(struct config *cfg, int slot, bool active, bool umbrella_edge);
 
     // Drive PCI slot `slot`'s strapped INTA-D line active/inactive.  The
@@ -347,15 +353,29 @@ typedef struct hw_profile {
     const struct builtin_video_desc *builtin_video;
 
     // Behavior: the lifecycle + host-input vtable for this machine.  Machines
-    // of the same chipset family SHARE one substrate (glue_substrate /
-    // mdu_substrate; iifx is bespoke).
+    // of the same chipset family SHARE one substrate (glue_substrate for
+    // SE/30-IIcx-IIx, mdu_substrate for IIci-IIsi, and so on).  A family with
+    // one machine still gets its own -- the IIfx and both PowerPC families --
+    // which is a statement about how many machines share the board, not about
+    // how much code the family writes for itself.
+    //
+    // "Bespoke substrate" is not "bespoke machine": every 68k family, the IIfx
+    // included, builds through mac030_build_core + mac030_build_lowspeed,
+    // checkpoints through mac030_checkpoint_save_core, and tears down through
+    // machine_teardown_config_devices.  What a family keeps for itself is what
+    // its hardware actually does differently -- for the IIfx, the OSS
+    // interrupt controller, the FMC ROM-invert POST window, the SCSI DMA
+    // engine, and a ROM overlay that doubles as a trip-wire.
     const machine_substrate_t *substrate;
 
     // Per-machine board descriptor — chipset-family data the shared substrate
     // interprets (proposal §4.2.2/§4.4).  Typed by convention: the family
     // substrate casts it to its concrete type (mac030_glue_board_t for
-    // GLUE/MDU).  NULL for families whose substrate needs no board (Plus,
-    // Lisa, and the bespoke IIfx, which carry their data directly).
+    // GLUE/MDU).  NULL where a substrate serves exactly one machine and can
+    // therefore reach its data directly (Plus, Lisa, IIfx).  The IIfx does
+    // define a mac030_board_desc_t of its own -- it simply has no second
+    // machine to vary against, so routing it through here would add a cast
+    // without adding sharing.
     const void *board;
 } hw_profile_t;
 

src/core/peripherals/mouse.c

@@ -12,8 +12,6 @@
 #include "system.h"
 #include "value.h"
 
-#include <string.h>
-
 #include <assert.h>
 #include <stddef.h>
 #include <string.h>

src/core/peripherals/nubus/nubus.c

@@ -546,8 +546,10 @@ void nubus_reset(nubus_bus_t *bus) {
 
 // Drive a slot's /NMRQ line through the machine substrate (proposal §4.4): the
 // bus owns the slot-IRQ aggregate mask and the umbrella transition, the chipset
-// owns HOW the line reaches the CPU (GLUE → VIA2; MDU/OSS → its own IRQ
-// controller).  nubus.c stays machine-agnostic — no cfg->via2 here.
+// owns HOW the line reaches the CPU (GLUE/MCU → VIA2; MDU → the RBV; OSS →
+// the OSS; AV → the PSC; PDM → BART), including converting the slot number
+// into whatever its controller numbers sources by.  nubus.c stays
+// machine-agnostic — no cfg->via2 here.
 static void nubus_route_slot_irq(config_t *cfg, int slot, bool active, bool umbrella_edge) {
     if (cfg && cfg->machine && cfg->machine->substrate->nubus_slot_irq)
         cfg->machine->substrate->nubus_slot_irq(cfg, slot, active, umbrella_edge);

src/core/peripherals/swim3.c

@@ -26,7 +26,6 @@
 // on this emulator (`debug.log swim3 5`), not from its source.
 
 #include "swim3.h"
-#include "floppy.h"
 
 #include "floppy.h"
 #include "log.h"

src/core/peripherals/swim3_xfer.c

@@ -36,7 +36,6 @@
 // nibblisation, the 3-byte checksum); and the byte streams the ROM's own
 // .Sony driver puts on the DMA channel, observed on this emulator.
 
-#include "floppy.h"
 #include "swim3.h"
 
 #include "floppy.h"

src/core/system.c

@@ -816,9 +816,20 @@ void system_destroy(config_t *config) {
     // pinned a fresh emulator that still references the rom object.
     root_uninstall_if(config);
 
-    // Tear down NuBus before peripherals so cards (which hold device
-    // pointers via cfg->via2 etc.) free cleanly first.  No-op when nubus
-    // is NULL (Plus today; future 68000-family machines).
+    // Tear down the expansion buses before the peripherals, so cards --
+    // which hold pointers to devices the substrate owns -- free cleanly
+    // first.  NuBus cards hold cfg->via2 and friends; the Network
+    // Servers' two 53C825As borrow cfg->scsi and machine.scsi2, so a card
+    // outliving its bus is a use-after-free either way.  Both are no-ops
+    // when the machine has no such bus.
+    //
+    // The order BETWEEN the two is not load-bearing: no profile declares
+    // both nubus_slots and pci_slots, so a machine has at most one of
+    // these.  Do not read a dependency into it.
+    if (config->pci) {
+        pci_root_delete(config->pci);
+        config->pci = NULL;
+    }
     if (config->nubus) {
         nubus_delete(config->nubus);
         config->nubus = NULL;

src/machines/av/av.c

@@ -13,6 +13,8 @@
 #include "av.h"
 #include "appletalk.h"
 
+#include "machine_teardown.h" // the shared config_t-owned delete chain
+
 #include "civic.h"
 #include "cuda.h"
 #include "dsp.h"
@@ -272,6 +274,31 @@ static void av_via1_irq(void *context, bool active) {
     av_update_ipl((config_t *)context, AV_IRQ_VIA1, active);
 }
 
+// substrate.nubus_slot_irq — the PSC aggregates NuBus slot interrupts itself,
+// so the bus drives one SInt source per slot and the PSC raises the VIA2
+// window's CA1 bit while any of them is asserted (psc.c psc_update_slot_bit).
+// The umbrella edge is therefore the chip's business, not ours, exactly as on
+// the MDU's RBV.
+//
+// Slots C/D/E map to SInt bits 3/4/5 (psc.h; the guest's PSCVIA2SlotInt reads
+// PSCVIA2SInt under mask ~$78 -- slots C/D/E plus on-board VBL on bit 6 --
+// and inverts, the register reading active-LOW).  So bit = slot - $9, and
+// av_psc_slot_source owns the inversion.
+//
+// No AV board declares a slot table yet (.slots = NULL on both, "declared but
+// unpopulated"), so nothing reaches this today.  It exists so the first AV
+// declaration-ROM card does not have to discover that its /NMRQ went nowhere.
+static void av_nubus_slot_irq(config_t *cfg, int slot, bool active, bool umbrella_edge) {
+    (void)umbrella_edge; // the PSC aggregates internally
+    av_state_t *st = (av_state_t *)cfg->machine_context;
+    if (!st || !st->psc)
+        return;
+    int bit = slot - 0x9;
+    if (bit < 3 || bit > 5) // only C/D/E exist on this family
+        return;
+    av_psc_slot_source(st->psc, bit, active);
+}
+
 // ============================================================
 // SCSI: the Curio's 53C96 at island $18000 ($10 register stride)
 // ============================================================
@@ -549,7 +576,7 @@ static void av_memory_layout(config_t *cfg) {
     // I/O island: the serialized window at $50F00000 plus its non-serialized
     // alias at $50F40000, folded by the $3FFFF mirror mask.
     mac030_io_fill_interface(&st->io_interface);
-    memory_map_add(cfg->mem_map, 0x50F00000u, 0x00080000u, "AV I/O", &st->io_interface, &st->io);
+    memory_map_add(cfg->mem_map, 0x50F00000u, 0x00080000u, "I/O", &st->io_interface, &st->io);
 
     // CPU-ID register page at $5FFFF000 (the register itself is $5FFFFFFC).
     st->cpuid_interface.read_uint8 = av_cpuid_read8;
@@ -747,14 +774,7 @@ static int av_init(config_t *cfg, checkpoint_t *cp) {
     if (cp)
         system_read_checkpoint_data(cp, &cfg->irq, sizeof(cfg->irq));
 
-    cfg->rtc = rtc_init(cfg->scheduler, cp, true);
-    cfg->scc = scc_init(NULL, cfg->scheduler, av_scc_irq, cfg, cp);
-    scc_set_clocks(cfg->scc, 7833600, 3686400);
-
-    // AppleTalk rides the SCC's LocalTalk channel, so it is built as soon as
-    // the SCC exists — and, because the checkpoint stream is positional, in
-    // the same relative place the save writes it (right after scc_checkpoint).
-    appletalk_init(cfg->scheduler, cfg->scc, cp);
+    mac030_build_lowspeed(cfg, cp, av_scc_irq);
 
     uint8_t via_ff = via_freq_factor_for_clock(cfg->machine->freq);
     cfg->via1 =
@@ -851,41 +871,9 @@ static void av_teardown(config_t *cfg) {
             st->bus_mmu = NULL;
         }
     }
-    if (cfg->scsi) {
-        scsi_delete(cfg->scsi);
-        cfg->scsi = NULL;
-    }
-    if (cfg->via1) {
-        via_delete(cfg->via1);
-        cfg->via1 = NULL;
-    }
-    // The AppleTalk stack is a client of the SCC's LocalTalk channel, so it
-    // goes first — it holds the scc pointer it was given at init.
-    appletalk_delete();
-    if (cfg->scc) {
-        scc_delete(cfg->scc);
-        cfg->scc = NULL;
-    }
-    if (cfg->rtc) {
-        rtc_delete(cfg->rtc);
-        cfg->rtc = NULL;
-    }
-    if (cfg->scheduler) {
-        scheduler_delete(cfg->scheduler);
-        cfg->scheduler = NULL;
-    }
-    if (cfg->cpu) {
-        cpu_delete(cfg->cpu);
-        cfg->cpu = NULL;
-    }
-    if (cfg->mem_map) {
-        memory_map_delete(cfg->mem_map);
-        cfg->mem_map = NULL;
-    }
-    if (cfg->debugger) {
-        debug_cleanup(cfg->debugger);
-        cfg->debugger = NULL;
-    }
+    // The config_t-owned devices, in the family-shared canonical order
+    // (machine_teardown.h).  Was a byte-identical copy in five families.
+    machine_teardown_config_devices(cfg);
     if (st) {
         free(st);
         cfg->machine_context = NULL;
@@ -894,14 +882,7 @@ static void av_teardown(config_t *cfg) {
 
 static void av_checkpoint_save(config_t *cfg, checkpoint_t *cp) {
     av_state_t *st = av_st(cfg);
-    memory_map_checkpoint(cfg->mem_map, cp);
-    cpu_checkpoint(cfg->cpu, cp); // includes the 040 MMU register file
-    scheduler_checkpoint(cfg->scheduler, cp);
-    system_write_checkpoint_data(cp, &cfg->irq, sizeof(cfg->irq));
-    rtc_checkpoint(cfg->rtc, cp);
-    scc_checkpoint(cfg->scc, cp);
-    appletalk_checkpoint(cp);
-    via_checkpoint(cfg->via1, cp);
+    mac030_checkpoint_save_core(cfg, cp);
     // Device order mirrors the checkpoint READS in av_build_devices — the
     // stream is sequential, so save and restore must walk it identically.
     av_psc_checkpoint(st->psc, cp);
@@ -953,7 +934,7 @@ const machine_substrate_t av_substrate = {
     .reset = av_reset,
     .teardown = av_teardown,
     .checkpoint_save = av_checkpoint_save,
-    .update_ipl = av_update_ipl, // VIA1→1, VIA2→2, L3-L6→3-6, NMI→7
+    .nubus_slot_irq = av_nubus_slot_irq, // slots C/D/E → PSC SInt bits 3-5
     .trigger_vbl = av_trigger_vbl,
     .fd_insert = mac_fd_insert,
     .fd_present = mac_fd_present,