AltairZ80: choose DSK boot sector from image size

Based on open-simh PR 512 by Peter Schorn, record the attached DSK image
size and use it to select the standard boot ROM start sector. Mini-Disk
images keep the Mini-Disk boot ROM, regular Altair disk images start at
sector 0, and other standard DSK images keep the existing sector 8
behavior.

This version adds integration coverage for the boot ROM bytes, documents
the behavior in the AltairZ80 manual, and notes the Python dependency for
the full integration test suite.

Author: pmetzger

BUILDING.md

@@ -72,6 +72,9 @@ For testing, you will need:
 
 - `cmocka` 1.1.5 or newer
   Provides the host-side C unit-test framework used under `tests/unit/`.
+- Python 3
+  Runs integration-test drivers that need more than a simple simulator
+  command script.
 
 For networking, you will need the libraries for the backends you enable:
 
@@ -97,6 +100,9 @@ Notes:
   instead of `bison.exe`. The CMake build accepts either executable name.
 - `cmocka` is for host-side unit tests rather than the historical
   simulator binaries themselves.
+- Python 3 is required for the full integration test suite. CMake can
+  configure without Python, but Python-backed tests are skipped when no
+  interpreter is found.
 - `SDL2_ttf` is the usual missing piece when a video-enabled build does
   not configure successfully.
 - `libedit` is optional, but without it SCP falls back to plainer
@@ -352,6 +358,7 @@ pkg-config --modversion libpng
 pkg-config --modversion zlib
 pkg-config --modversion libedit
 pkg-config --modversion slirp
+python3 --version
 bison --version
 ```
 

docs/simulators/AltairZ80/altairz80.md

@@ -947,6 +947,11 @@ for disk 0 to the original number of 77. The command `D TRACKS[0-7]
 77` changes the highest track number for all disks to 77. The Mini-Disk
 support was added by Mike Douglas in May 2014.
 
+When booting from `DSK`, AltairZ80 chooses the boot ROM behavior from
+the attached image size. Mini-Disk-sized images use the Mini-Disk boot
+ROM. Standard Altair disk images boot from track 0, sector 0. Other
+image sizes use the standard boot ROM starting at track 0, sector 8.
+
 The DSK device can be configured with
 
 | command | action |

simulators/AltairZ80/CMakeLists.txt

@@ -93,3 +93,25 @@ add_simulator(altairz80
     LABEL AltairZ80
     PKG_FAMILY altairz80_family
     TEST altairz80)
+
+set(altairz80_boot_rom_choice_output_dir
+    "${CMAKE_CURRENT_BINARY_DIR}/tests/altairz80-boot-rom-choice")
+
+find_package(Python3 COMPONENTS Interpreter)
+if (Python3_Interpreter_FOUND)
+    add_test(
+        NAME zimh-altairz80-boot-rom-choice
+        COMMAND
+            ${Python3_EXECUTABLE}
+            "${CMAKE_CURRENT_SOURCE_DIR}/tests/boot_rom_choice_test.py"
+            $<TARGET_FILE:altairz80>
+            --work-dir
+            "${altairz80_boot_rom_choice_output_dir}")
+
+    set_tests_properties(
+        zimh-altairz80-boot-rom-choice
+        PROPERTIES
+            LABELS "zimh;integration;zimh-AltairZ80"
+            ENVIRONMENT
+            "SIMH_TEST_OUTPUT_DIR=${altairz80_boot_rom_choice_output_dir}")
+endif ()

simulators/AltairZ80/altairz80_defs.h

@@ -52,6 +52,9 @@
 #define LDA_INSTRUCTION         0x3e                        /* op-code for LD A,<8-bit value> instruction   */
 #define UNIT_NO_OFFSET_1        0x37                        /* LD A,<unitno>                                */
 #define UNIT_NO_OFFSET_2        0xb4                        /* LD a,80h | <unitno>                          */
+/* Standard disk boot ROM patch points. */
+#define LDB_INSTRUCTION         0x06
+#define START_SECTOR_OFFSET     0x57
 
 #define CPU_INDEX_8080          4                           /* index of default PC register */
 

simulators/AltairZ80/altairz80_dsk.c

@@ -157,6 +157,9 @@
 #define MINI_DISK_SIZE      (MINI_DISK_TRACKS * MINI_DISK_SECT * DSK_SECTSIZE)
 #define MINI_DISK_DELTA     4096                    /* threshold for detecting mini disks       */
 
+#define ALTAIR_DISK_SIZE    337664                  /* size of regular Altair disks             */
+#define ALTAIR_DISK_DELTA   256                     /* threshold for detecting Altair disks     */
+
 int32 dsk10(const int32 port, const int32 io, const int32 data);
 int32 dsk11(const int32 port, const int32 io, const int32 data);
 int32 dsk12(const int32 port, const int32 io, const int32 data);
@@ -187,6 +190,8 @@ static int32 sectors_per_track [NUM_OF_DSK] = { DSK_SECT, DSK_SECT, DSK_SECT, DS
                                                 DSK_SECT, DSK_SECT, DSK_SECT, DSK_SECT,
                                                 DSK_SECT, DSK_SECT, DSK_SECT, DSK_SECT,
                                                 DSK_SECT, DSK_SECT, DSK_SECT, DSK_SECT };
+static int32 current_image_size [NUM_OF_DSK] = {
+    0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0};
 static int32 tracks         [NUM_OF_DSK]    = { MAX_TRACKS, MAX_TRACKS, MAX_TRACKS, MAX_TRACKS,
                                                 MAX_TRACKS, MAX_TRACKS, MAX_TRACKS, MAX_TRACKS,
                                                 MAX_TRACKS, MAX_TRACKS, MAX_TRACKS, MAX_TRACKS,
@@ -310,6 +315,8 @@ static REG dsk_reg[] = {
                "Number of tracks register array"), REG_CIRC                                 },
     { BRDATAD (SECTPERTRACK, sectors_per_track, 10, 32,  NUM_OF_DSK,
                "Number of sectors per track register array"), REG_CIRC                      },
+    { BRDATAD (IMAGESIZE, current_image_size,    10, 32,  NUM_OF_DSK,
+               "Size of disk image array"), REG_CIRC + REG_RO                               },
     { DRDATAD (IN9COUNT,     in9_count,         4,
                "Count of IN(9) register"),  REG_RO                                          },
     { DRDATAD (IN9MESSAGE,   in9_message,       4,
@@ -396,6 +403,23 @@ static t_stat dsk_reset(DEVICE *dptr) {
     sim_map_resource(0x0A, 1, RESOURCE_TYPE_IO, &dsk12, "dsk12", dptr->flags & DEV_DIS);
     return SCPE_OK;
 }
+
+static t_bool dsk_image_size_is_near(
+    const int32 image_size, const int32 expected_size, const int32 delta)
+{
+    return (expected_size - delta < image_size) &&
+           (image_size < expected_size + delta);
+}
+
+static int32 dsk_boot_start_sector(const int32 image_size)
+{
+    if (dsk_image_size_is_near(
+            image_size, ALTAIR_DISK_SIZE, ALTAIR_DISK_DELTA))
+        return 0;
+
+    return 8;
+}
+
 /* dsk_attach - determine type of drive attached based on disk image size */
 
 static t_stat dsk_attach(UNIT *uptr, const char *cptr) {
@@ -413,8 +437,10 @@ static t_stat dsk_attach(UNIT *uptr, const char *cptr) {
      tracks to 16, otherwise, 32 sectors per track. */
 
     imageSize = sim_fsize(uptr -> fileref);
-    sectors_per_track[thisUnitIndex] = (((MINI_DISK_SIZE - MINI_DISK_DELTA < imageSize) &&
-                                         (imageSize < MINI_DISK_SIZE + MINI_DISK_DELTA)) ?
+    current_image_size[thisUnitIndex] = imageSize;
+    sectors_per_track[thisUnitIndex] = (dsk_image_size_is_near(
+                                            imageSize, MINI_DISK_SIZE,
+                                            MINI_DISK_DELTA) ?
                                         MINI_DISK_SECT : DSK_SECT);
     return SCPE_OK;
 }
@@ -426,7 +452,8 @@ void install_ALTAIRbootROM(void) {
 }
 
 /*  The boot routine modifies the boot ROM in such a way that subsequently
-    the specified disk is used for boot purposes.
+    the specified disk is used for boot purposes.  For the standard boot ROM,
+    the starting sector is chosen from the attached image size.
 */
 static t_stat dsk_boot(int32 unitno, DEVICE *dptr) {
     if (cpu_unit.flags & (UNIT_CPU_ALTAIRROM | UNIT_CPU_BANKED)) {
@@ -435,17 +462,25 @@ static t_stat dsk_boot(int32 unitno, DEVICE *dptr) {
                                                    ALTAIR_ROM_LOW, TRUE) == SCPE_OK);
             ASSURE(result);
         } else {
-        /* check whether we are really modifying an LD A,<> instruction */
+            if (bootrom_dsk[START_SECTOR_OFFSET - 1] == LDB_INSTRUCTION)
+                bootrom_dsk[START_SECTOR_OFFSET] =
+                    dsk_boot_start_sector(current_image_size[unitno]);
+            else {
+                sim_printf("Incorrect boot ROM first sector offset detected.\n");
+                return SCPE_IERR;
+            }
+
+            /* check whether we are really modifying an LD A,<> instruction */
             if ((bootrom_dsk[UNIT_NO_OFFSET_1 - 1] == LDA_INSTRUCTION) &&
                 (bootrom_dsk[UNIT_NO_OFFSET_2 - 1] == LDA_INSTRUCTION)) {
-            bootrom_dsk[UNIT_NO_OFFSET_1] = unitno & 0xff;             /* LD A,<unitno>        */
-            bootrom_dsk[UNIT_NO_OFFSET_2] = 0x80 | (unitno & 0xff);    /* LD a,80h | <unitno>  */
-        } else { /* Attempt to modify non LD A,<> instructions is refused. */
-                sim_printf("Incorrect boot ROM offsets detected.\n");
-            return SCPE_IERR;
+                bootrom_dsk[UNIT_NO_OFFSET_1] = unitno & 0xff;          /* LD A,<unitno>        */
+                bootrom_dsk[UNIT_NO_OFFSET_2] = 0x80 | (unitno & 0xff); /* LD a,80h | <unitno>  */
+            } else { /* Attempt to modify non LD A,<> instructions is refused. */
+                sim_printf("Incorrect boot ROM unit number offset detected.\n");
+                return SCPE_IERR;
+            }
+            install_ALTAIRbootROM();                                    /* install modified ROM */
         }
-        install_ALTAIRbootROM();                                         /* install modified ROM */
-    }
     }
     *((int32 *) sim_PC->loc) = ALTAIR_ROM_LOW;
     return SCPE_OK;

simulators/AltairZ80/tests/boot_rom_choice_test.py

@@ -0,0 +1,90 @@
+#!/usr/bin/env python3
+
+import argparse
+import re
+import subprocess
+from pathlib import Path
+
+
+def write_sized_file(path: Path, size: int) -> None:
+    chunk = b"x" * 32768
+    remaining = size
+
+    with path.open("wb") as image:
+        while remaining >= len(chunk):
+            image.write(chunk)
+            remaining -= len(chunk)
+        if remaining:
+            image.write(chunk[:remaining])
+
+
+def run_boot_rom_case(
+    simulator: Path,
+    work_dir: Path,
+    name: str,
+    image_size: int,
+    expected_bytes: tuple[str, str, str],
+) -> None:
+    image = work_dir / f"{name}.dsk"
+    script = work_dir / f"{name}.ini"
+
+    write_sized_file(image, image_size)
+    script.write_text(
+        "\n".join(
+            [
+                "set cpu 8080",
+                "set cpu altairrom",
+                "break 0xff00",
+                f'attach dsk0 "{image}"',
+                "boot dsk0",
+                "examine 0xff56-0xff58",
+                "exit 0",
+                "",
+            ]
+        ),
+        encoding="utf-8",
+    )
+
+    result = subprocess.run(
+        [str(simulator), str(script), "-v"],
+        check=False,
+        text=True,
+        capture_output=True,
+        timeout=10,
+    )
+    if result.returncode != 0:
+        raise RuntimeError(
+            f"{name}: simulator exited with {result.returncode}\n"
+            f"stdout:\n{result.stdout}\n"
+            f"stderr:\n{result.stderr}"
+        )
+
+    for address, expected_byte in zip(("FF56", "FF57", "FF58"), expected_bytes):
+        pattern = rf"{address}:\s+{re.escape(expected_byte)}\b"
+        if re.search(pattern, result.stdout) is None:
+            raise AssertionError(
+                f"{name}: expected {address} to be {expected_byte}\n"
+                f"stdout:\n{result.stdout}"
+            )
+
+
+def main() -> None:
+    parser = argparse.ArgumentParser()
+    parser.add_argument("simulator", type=Path)
+    parser.add_argument("--work-dir", type=Path, required=True)
+    args = parser.parse_args()
+
+    args.work_dir.mkdir(parents=True, exist_ok=True)
+    run_boot_rom_case(
+        args.simulator, args.work_dir, "mini-disk", 76720, ("00", "3E", "10")
+    )
+    run_boot_rom_case(
+        args.simulator, args.work_dir, "regular-altair", 337664, ("06", "00", "C5")
+    )
+    run_boot_rom_case(
+        args.simulator, args.work_dir, "nonstandard", 1000, ("06", "08", "C5")
+    )
+
+
+if __name__ == "__main__":
+    main()