v1.7.1: bump version (unified release)

find_by_drive_id gated the product-id-blind four-field fallback behind an empty
product_id in 1.7.0, so a drive reporting a specific product_id (e.g. an LG BU40N
reporting "BD-RE BU40N") no longer matched its catalog entry (generic "BD-RE"),
silently skipping the LibreDrive firmware unlocker — the rip fell through to the
AACS host-cert path (LD: no, AACS: yes).

Run the four-field fallback (vendor/revision/vendor_specific/firmware_date)
unconditionally but ambiguity-guarded: bind a UNIQUE catalog match regardless of
the reported product_id (the real-drive case), and require the exact product_id
only when catalogued siblings genuinely share the four-field identity — preserving
the anti-mis-bind protection the 1.7.0 gate intended. Regression test added.

Bump to 1.7.1.

.github/workflows/ci.yml

@@ -17,10 +17,31 @@ on:
 # seven others down with it, which is why it gets the full OS matrix rather
 # than a token Linux run.
 #
-# Cargo.lock is gitignored (library convention), so `--locked` is deliberately
-# NOT used: with no committed lockfile it fails rather than resolves.
+# Cargo.lock is now committed (issue #15), so the resolved dependency graph is
+# recorded and the `deny` job audits it. `--locked` is still deliberately NOT
+# used, matching the binary crates' ci.yml: CI's sibling path-patch layout
+# resolves the freemkv crates as path packages, which `--locked` can't satisfy.
 
 jobs:
+  # ── Single source of truth for the MSRV ─────────────────────────────────────
+  #
+  # `rust-version` in Cargo.toml is the ONE place the MSRV is declared. This job
+  # reads it; every job that must build/test AT the MSRV consumes
+  # `needs.resolve-msrv.outputs.v` instead of a hardcoded pin, so the floor
+  # cannot drift out of sync across workflow files. The `test` job building
+  # green at this value IS the verification that the declared MSRV is real.
+  # clippy deliberately does NOT use it — it runs on `stable` (lint debt is
+  # caught against current Rust).
+  resolve-msrv:
+    runs-on: ubuntu-latest
+    outputs:
+      v: ${{ steps.msrv.outputs.v }}
+    steps:
+      - uses: actions/checkout@v7
+      - id: msrv
+        shell: bash
+        run: echo "v=$(grep -m1 '^rust-version' Cargo.toml | sed -E 's/.*\"([^\"]+)\".*/\1/')" >> "$GITHUB_OUTPUT"
+
   # CI owns the changelog: main may never carry a released version without a
   # CHANGELOG.md entry for it. The release orchestrator dates an existing
   # `## [X.Y.Z]` heading — it does NOT invent one — so a version bump that
@@ -48,24 +69,36 @@ jobs:
           echo "::warning::${msg} (allowed on ${GITHUB_REF_NAME:-unknown}; required before it can reach main)."
 
   lint:
+    needs: resolve-msrv
     runs-on: ubuntu-latest
     steps:
       - uses: actions/checkout@v7
-      - uses: dtolnay/rust-toolchain@1.90.0
+      # fmt runs on the pinned MSRV toolchain; clippy runs on stable. Linting on
+      # stable makes new-lint debt a BLOCKING gate the pinned version would
+      # otherwise hide. The MSRV pin is installed second so it stays the default
+      # toolchain (fmt uses it implicitly); clippy is invoked with `+stable`.
+      - uses: dtolnay/rust-toolchain@stable
         with:
-          components: clippy, rustfmt
+          components: clippy
+      - uses: dtolnay/rust-toolchain@master
+        with:
+          toolchain: ${{ needs.resolve-msrv.outputs.v }}
+          components: rustfmt
       - uses: Swatinem/rust-cache@v2
         with:
           shared-key: freemkv-unlock-ci
       - run: cargo fmt --check
       # --all-targets so the test code is linted too, not just the library.
-      - run: cargo clippy --all-targets -- -D warnings
+      - run: cargo +stable clippy --all-targets -- -D warnings
 
   test:
+    needs: resolve-msrv
     runs-on: ubuntu-latest
     steps:
       - uses: actions/checkout@v7
-      - uses: dtolnay/rust-toolchain@1.90.0
+      - uses: dtolnay/rust-toolchain@master
+        with:
+          toolchain: ${{ needs.resolve-msrv.outputs.v }}
       - uses: Swatinem/rust-cache@v2
         with:
           shared-key: freemkv-unlock-ci
@@ -79,26 +112,43 @@ jobs:
   # release gate). It mirrors the `test` job's environment — same pinned
   # toolchain, ubuntu-latest, no `if:` so it runs on every branch and PR just
   # as `test` does. `--locked` is intentionally omitted to match this crate's
-  # policy (Cargo.lock is gitignored; --locked would fail without it).
+  # policy (Cargo.lock is committed but --locked can't satisfy the path-patch layout).
   coverage:
+    needs: resolve-msrv
     runs-on: ubuntu-latest
     steps:
       - uses: actions/checkout@v7
-      - uses: dtolnay/rust-toolchain@1.90.0
+      - uses: dtolnay/rust-toolchain@master
         with:
+          toolchain: ${{ needs.resolve-msrv.outputs.v }}
           components: llvm-tools-preview
       - uses: Swatinem/rust-cache@v2
         with:
           shared-key: freemkv-unlock-coverage
       - uses: taiki-e/install-action@cargo-llvm-cov
       - run: cargo llvm-cov --workspace --ignore-run-fail --lcov --output-path lcov.info
-      - uses: codecov/codecov-action@v5
+      - uses: codecov/codecov-action@v7
         with:
           token: ${{ secrets.CODECOV_TOKEN }}
           files: lcov.info
           slug: freemkv/freemkv-unlock
           fail_ci_if_error: false
 
+  # cargo-deny gates the RESOLVED dependency graph: RUSTSEC advisories, a
+  # permissive licence allow-list, and source/ban hygiene. It mirrors the
+  # `test` job's checkout (single root checkout, no siblings) and runs on
+  # `stable` — advisory/licence debt is caught against current Rust, not the
+  # MSRV pin — so no `needs: resolve-msrv`.
+  deny:
+    runs-on: ubuntu-latest
+    steps:
+      - uses: actions/checkout@v7
+      - uses: dtolnay/rust-toolchain@stable
+      - uses: EmbarkStudios/cargo-deny-action@v2
+        with:
+          command: check advisories bans licenses sources
+          manifest-path: Cargo.toml
+
   # The crate ships on macOS and Windows through its consumers, so it has to
   # compile there. Build the tests (not just `cargo check`) — that forces
   # codegen and catches errors in test-only code, which `check` skips.
@@ -115,11 +165,14 @@ jobs:
     # than a slow one: release.sh's CI gate refuses while any run for the
     # commit is still in progress, so a never-scheduled job blocks releases
     # silently -- see the note on real-media in qa.yml.
+    needs: resolve-msrv
     if: github.ref_name == 'qa' || github.ref_name == 'main'
     runs-on: macos-latest
     steps:
       - uses: actions/checkout@v7
-      - uses: dtolnay/rust-toolchain@1.90.0
+      - uses: dtolnay/rust-toolchain@master
+        with:
+          toolchain: ${{ needs.resolve-msrv.outputs.v }}
       - uses: Swatinem/rust-cache@v2
         with:
           shared-key: freemkv-unlock-ci
@@ -138,11 +191,14 @@ jobs:
     # than a slow one: release.sh's CI gate refuses while any run for the
     # commit is still in progress, so a never-scheduled job blocks releases
     # silently -- see the note on real-media in qa.yml.
+    needs: resolve-msrv
     if: github.ref_name == 'qa' || github.ref_name == 'main'
     runs-on: windows-latest
     steps:
       - uses: actions/checkout@v7
-      - uses: dtolnay/rust-toolchain@1.90.0
+      - uses: dtolnay/rust-toolchain@master
+        with:
+          toolchain: ${{ needs.resolve-msrv.outputs.v }}
       - uses: Swatinem/rust-cache@v2
         with:
           shared-key: freemkv-unlock-ci

.github/workflows/comment-guard.yml

@@ -0,0 +1,17 @@
+name: comment-guard
+
+# Caps explanatory prose by COMMENT INTENT (not visibility):
+#   //! / /*! module docs  — unbounded (module-scoped, can't hide per-item text)
+#   ///  / /** */ item docs — 8 prose lines (fenced ``` examples excluded)
+#   //   / /* */  asides    — 3 lines (single-blank-separated runs count as one)
+# Overflow belongs in the module //! or docs/<topic>.md with a one-line // pointer.
+
+on: [push, pull_request]
+
+jobs:
+  comment-guard:
+    runs-on: ubuntu-latest
+    steps:
+      - uses: actions/checkout@v7
+      - name: Run comment-guard
+        run: python3 ci/comment-guard.py

.github/workflows/identity-guard.yml

@@ -0,0 +1,41 @@
+name: identity-guard
+# Prevention: no commit may carry an unrelated-business or non-canonical author/
+# committer email. dev@example.com (a dead corp address) once slipped onto the
+# release cascade + a batch of local commits; this fails the build if it recurs.
+on:
+  push:
+    branches: [dev, qa, main]
+  pull_request:
+jobs:
+  identity:
+    runs-on: ubuntu-latest
+    steps:
+      - uses: actions/checkout@v7
+        with:
+          fetch-depth: 0
+      - name: Reject banned author/committer emails
+        env:
+          BEFORE: ${{ github.event.before }}
+          AFTER: ${{ github.sha }}
+          EVENT: ${{ github.event_name }}
+          PR_BASE: ${{ github.event.pull_request.base.sha }}
+          PR_HEAD: ${{ github.event.pull_request.head.sha }}
+        run: |
+          set -euo pipefail
+          # Unrelated business domains + a personal address normalized to the
+          # GitHub noreply. Real external contributors are unaffected.
+          BANNED='@example-a\.invalid|@example-b\.invalid|owner@example\.invalid'
+          if [ "$EVENT" = "pull_request" ]; then
+            range="$PR_BASE..$PR_HEAD"
+          elif [ -n "${BEFORE:-}" ] && [ "$BEFORE" != "0000000000000000000000000000000000000000" ] && git cat-file -e "$BEFORE" 2>/dev/null; then
+            range="$BEFORE..$AFTER"
+          else
+            range="$AFTER~1..$AFTER"
+          fi
+          bad=$(git log --format='%an <%ae>%n%cn <%ce>' "$range" 2>/dev/null | sort -u | grep -iE "$BANNED" || true)
+          if [ -n "$bad" ]; then
+            echo "::error::commit(s) carry a banned author/committer identity:"; echo "$bad" | sed 's/^/  /'
+            echo "::error::Commit as your GitHub noreply (1085847+MattJackson@users.noreply.github.com). Fix: git config user.email <noreply>; git commit --amend --reset-author."
+            exit 1
+          fi
+          echo "identity-guard: clean"

.github/workflows/qa.yml

@@ -24,6 +24,19 @@ on:
   workflow_dispatch:
 
 jobs:
+  # Single source of truth for the MSRV — see ci.yml. Jobs that build/test AT
+  # the MSRV read `needs.resolve-msrv.outputs.v` from Cargo.toml's rust-version;
+  # clippy (cross-lint) runs on stable instead.
+  resolve-msrv:
+    runs-on: ubuntu-latest
+    outputs:
+      v: ${{ steps.msrv.outputs.v }}
+    steps:
+      - uses: actions/checkout@v7
+      - id: msrv
+        shell: bash
+        run: echo "v=$(grep -m1 '^rust-version' Cargo.toml | sed -E 's/.*\"([^\"]+)\".*/\1/')" >> "$GITHUB_OUTPUT"
+
   # ── Name the candidate ────────────────────────────────────────────────
   #
   # Every push to `qa` is a release candidate, so every push gets a tag:
@@ -65,6 +78,7 @@ jobs:
   # changes what the optimiser can prove. A test that only passes in debug is
   # a test that never guarded the binary anyone actually ships.
   release-tests:
+    needs: resolve-msrv
     strategy:
       fail-fast: false
       matrix:
@@ -74,7 +88,9 @@ jobs:
       - uses: actions/checkout@v7
         with:
           path: freemkv-unlock
-      - uses: dtolnay/rust-toolchain@1.90.0
+      - uses: dtolnay/rust-toolchain@master
+        with:
+          toolchain: ${{ needs.resolve-msrv.outputs.v }}
       - uses: Swatinem/rust-cache@v2
         with:
           workspaces: freemkv-unlock
@@ -90,7 +106,9 @@ jobs:
       - uses: actions/checkout@v7
         with:
           path: freemkv-unlock
-      - uses: dtolnay/rust-toolchain@1.90.0
+      # clippy runs on stable (matches ci.yml's lint job); build/test stay on
+      # the MSRV pin. See ci.yml for why the lint toolchain need not be the MSRV.
+      - uses: dtolnay/rust-toolchain@stable
         with:
           components: clippy
       - uses: Swatinem/rust-cache@v2
@@ -105,12 +123,15 @@ jobs:
   # of the SCSI transport and platform layers compile on no other runner, so
   # without this they are first built at release time.
   windows-build:
+    needs: resolve-msrv
     runs-on: windows-latest
     steps:
       - uses: actions/checkout@v7
         with:
           path: freemkv-unlock
-      - uses: dtolnay/rust-toolchain@1.90.0
+      - uses: dtolnay/rust-toolchain@master
+        with:
+          toolchain: ${{ needs.resolve-msrv.outputs.v }}
       - uses: Swatinem/rust-cache@v2
         with:
           workspaces: freemkv-unlock

.gitignore

@@ -1,3 +1,3 @@
 /target
-Cargo.lock
 .cargo/config.toml
+NEXT_SESSION.md

CHANGELOG.md

@@ -1,6 +1,42 @@
 # Changelog
 
-## [1.6.12] — UNRELEASED
+## [1.7.1] — 2026-09-14
+
+### Added
+
+- freemkv firmware: `full_unlock` now issues a trailing best-effort Raw Read `04 03` (data-clear / bus-off) after the VID read, so an unlocked drive is placed in bus-off mode and subsequent content `READ(10)`s return AACS-at-rest bytes without the in-transit AACS bus wrap. A no-op on firmware without the `04 03` lever and never discards an obtained unlock, so it is inert until the firmware side ships. Also exposed as `set_bus_off`.
+
+### Fixed
+
+- `ld/profile`: `find_by_drive_id` now binds a UNIQUE four-field identity match even when the drive reports a specific `product_id` the catalog stores generically (drive reports "BD-RE BU40N", catalog "BD-RE"). The 1.7.0 gating on an empty product id broke LibreDrive unlock on UHD-capable LG BU40N drives; this restores it.
+
+## [1.7.0] — 2026-09-02
+
+### Added
+
+- Preliminary, beta unlocking support for **Renesas** optical drives and for drives running **freemkv firmware**. Both are early and unvalidated; the established LibreDrive / AACS / CSS unlock routes are unchanged.
+
+### Fixed
+
+- `ld/profile`: `find_by_drive_id` no longer mis-binds a drive whose reported `product_id` is real but not catalogued to a sibling model's profile. The product-id-blind loose match is now gated on an empty product id, so a wrong profile — and its wrong LibreDrive microcode — can't be selected for a drive that does report a product id.
+
+### Changed
+
+- Version aligned to 1.7.0 for the unified release.
+
+## [1.6.14] — 2026-08-31
+
+### Changed
+
+- Version aligned to 1.6.14 for the unified release.
+
+## [1.6.13] — 2026-08-28
+
+### Changed
+
+- Version aligned to 1.6.13 for the unified release.
+
+## [1.6.12] — 2026-08-27
 
 ### Changed
 

Cargo.lock

@@ -0,0 +1,454 @@
+# This file is automatically @generated by Cargo.
+# It is not intended for manual editing.
+version = 4
+
+[[package]]
+name = "aes"
+version = "0.9.2"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "f8eb277bec05f56a0e0591f155a484cbd0f4f07ff2905051a48c72f004f7ed58"
+dependencies = [
+ "cipher",
+ "cpubits",
+ "cpufeatures 0.3.1",
+]
+
+[[package]]
+name = "autocfg"
+version = "1.5.1"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "f2032f911046de80f0a198e0901378627c33f59ea0ac00e363d481118bd70a53"
+
+[[package]]
+name = "base64"
+version = "0.23.1"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "ac07cdecf99051d9a5238b80f35af32cdeba5b336e55d957b318b50137e18da5"
+
+[[package]]
+name = "block-buffer"
+version = "0.10.4"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "3078c7629b62d3f0439517fa394996acacc5cbc91c5a20d8c658e77abd503a71"
+dependencies = [
+ "generic-array",
+]
+
+[[package]]
+name = "block-buffer"
+version = "0.12.1"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "d2f6c7dbe95a6ed67ad9f18e57daf93a2f034c524b99fd2b76d18fdfeb6660aa"
+dependencies = [
+ "hybrid-array",
+]
+
+[[package]]
+name = "cfg-if"
+version = "1.0.4"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "9330f8b2ff13f34540b44e946ef35111825727b38d33286ef986142615121801"
+
+[[package]]
+name = "chacha20"
+version = "0.10.2"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "65c35e4b699c7e15ccbe7ee35c005e4fc0a278d22238a2857e6ce2dadeda1b06"
+dependencies = [
+ "cfg-if",
+ "cpufeatures 0.3.1",
+ "rand_core",
+]
+
+[[package]]
+name = "cipher"
+version = "0.5.2"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "e8cf2a2c93cd704877c0858356ed03480ff301ee950b43f1cbe4573b088bfa6c"
+dependencies = [
+ "crypto-common 0.2.2",
+ "inout",
+]
+
+[[package]]
+name = "const-oid"
+version = "0.10.2"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "a6ef517f0926dd24a1582492c791b6a4818a4d94e789a334894aa15b0d12f55c"
+
+[[package]]
+name = "cpubits"
+version = "0.1.1"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "15b85f9c39137c3a891689859392b1bd49812121d0d61c9caf00d46ed5ce06ae"
+
+[[package]]
+name = "cpufeatures"
+version = "0.2.17"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "59ed5838eebb26a2bb2e58f6d5b5316989ae9d08bab10e0e6d103e656d1b0280"
+dependencies = [
+ "libc",
+]
+
+[[package]]
+name = "cpufeatures"
+version = "0.3.1"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "5ca28b0ae3115b884660db4118d803791fd6756b6e88f39c0f3f7859060d7566"
+dependencies = [
+ "libc",
+]
+
+[[package]]
+name = "crypto-common"
+version = "0.1.7"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "78c8292055d1c1df0cce5d180393dc8cce0abec0a7102adb6c7b1eef6016d60a"
+dependencies = [
+ "generic-array",
+ "typenum",
+]
+
+[[package]]
+name = "crypto-common"
+version = "0.2.2"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "ce6e4c961d6cd6c9a86db418387425e8bdeaf05b3c8bc1411e6dca4c252f1453"
+dependencies = [
+ "hybrid-array",
+]
+
+[[package]]
+name = "digest"
+version = "0.10.7"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "9ed9a281f7bc9b7576e61468ba615a66a5c8cfdff42420a70aa82701a3b1e292"
+dependencies = [
+ "block-buffer 0.10.4",
+ "crypto-common 0.1.7",
+]
+
+[[package]]
+name = "digest"
+version = "0.11.3"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "f1dd6dbb5841937940781866fa1281a1ff7bd3bf827091440879f9994983d5c2"
+dependencies = [
+ "block-buffer 0.12.1",
+ "const-oid",
+ "crypto-common 0.2.2",
+]
+
+[[package]]
+name = "freemkv-unlock"
+version = "1.7.1"
+dependencies = [
+ "aes",
+ "base64",
+ "num-bigint",
+ "num-traits",
+ "rand",
+ "serde",
+ "serde_json",
+ "sha1",
+ "sha2",
+ "tracing",
+]
+
+[[package]]
+name = "generic-array"
+version = "0.14.7"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "85649ca51fd72272d7821adaf274ad91c288277713d9c18820d8499a7ff69e9a"
+dependencies = [
+ "typenum",
+ "version_check",
+]
+
+[[package]]
+name = "getrandom"
+version = "0.4.3"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "300e883d756b2e4ec94e02791f39b04b522276138852cfc41d9fb7e904106099"
+dependencies = [
+ "cfg-if",
+ "libc",
+ "r-efi",
+ "rand_core",
+]
+
+[[package]]
+name = "hybrid-array"
+version = "0.4.14"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "707114b52a152fa7bdb290cd7cd5912d9467273b6d74e21b8d81aca1f8533f6b"
+dependencies = [
+ "typenum",
+]
+
+[[package]]
+name = "inout"
+version = "0.2.2"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "4250ce6452e92010fdf7268ccc5d14faa80bb12fc741938534c58f16804e03c7"
+dependencies = [
+ "hybrid-array",
+]
+
+[[package]]
+name = "itoa"
+version = "1.0.18"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "8f42a60cbdf9a97f5d2305f08a87dc4e09308d1276d28c869c684d7777685682"
+
+[[package]]
+name = "libc"
+version = "0.2.189"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "3eaf3ede3fee6db1a4c2ee091bf8a8b4dccdc6d17f656fb07896ee72867612f2"
+
+[[package]]
+name = "memchr"
+version = "2.8.3"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "cf8baf1c55e62ffcace7a9f06f4bd9cd3f0c4beb022d3b367256b91b87513d98"
+
+[[package]]
+name = "num-bigint"
+version = "0.5.1"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "93e7820bc0a80a0238e650327316f929ba18d5be054b647490a3a6a339f3e7c0"
+dependencies = [
+ "num-integer",
+ "num-traits",
+]
+
+[[package]]
+name = "num-integer"
+version = "0.1.47"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "7ce2d95d4b3734dc35aa2f45e1aa22cd416814592a4f9d9205e11affd5b8e10b"
+dependencies = [
+ "num-traits",
+]
+
+[[package]]
+name = "num-traits"
+version = "0.2.19"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "071dfc062690e90b734c0b2273ce72ad0ffa95f0c74596bc250dcfd960262841"
+dependencies = [
+ "autocfg",
+]
+
+[[package]]
+name = "once_cell"
+version = "1.21.4"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "9f7c3e4beb33f85d45ae3e3a1792185706c8e16d043238c593331cc7cd313b50"
+
+[[package]]
+name = "pin-project-lite"
+version = "0.2.17"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "a89322df9ebe1c1578d689c92318e070967d1042b512afbe49518723f4e6d5cd"
+
+[[package]]
+name = "proc-macro2"
+version = "1.0.107"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "985e7ec9bb745e6ce6535b544d84d6cd6f7ad8bd711c398938ae983b91a766d9"
+dependencies = [
+ "unicode-ident",
+]
+
+[[package]]
+name = "quote"
+version = "1.0.47"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "1fbf4db142a473a8d80c26bbf18454ed458bf8d26c8219c331daecfdbd079001"
+dependencies = [
+ "proc-macro2",
+]
+
+[[package]]
+name = "r-efi"
+version = "6.0.0"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "f8dcc9c7d52a811697d2151c701e0d08956f92b0e24136cf4cf27b57a6a0d9bf"
+
+[[package]]
+name = "rand"
+version = "0.10.2"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "c7f5fa3a058cd35567ef9bfa5e75732bee0f9e4c55fa90477bef2dfcdbc4be80"
+dependencies = [
+ "chacha20",
+ "getrandom",
+ "rand_core",
+]
+
+[[package]]
+name = "rand_core"
+version = "0.10.1"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "63b8176103e19a2643978565ca18b50549f6101881c443590420e4dc998a3c69"
+
+[[package]]
+name = "serde"
+version = "1.0.229"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "4148590afebada386688f18773da617792bf2ef03ffc1e4cbd2b1d45b023e0ba"
+dependencies = [
+ "serde_core",
+ "serde_derive",
+]
+
+[[package]]
+name = "serde_core"
+version = "1.0.229"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "67dca2c9c51e58a4791a4b1ed58308b39c64224d349a935ab5039aa360942a48"
+dependencies = [
+ "serde_derive",
+]
+
+[[package]]
+name = "serde_derive"
+version = "1.0.229"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "e7a5d71263a5a7d47b41f6b3f06ba276f10cc18b0931f1799f710578e2309348"
+dependencies = [
+ "proc-macro2",
+ "quote",
+ "syn 3.0.4",
+]
+
+[[package]]
+name = "serde_json"
+version = "1.0.151"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "c841b55ecdae098c80dcae9cf767f6f8a0c2cdb3416bbef72181df4d0fe73f14"
+dependencies = [
+ "itoa",
+ "memchr",
+ "serde",
+ "serde_core",
+ "zmij",
+]
+
+[[package]]
+name = "sha1"
+version = "0.10.7"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "a978451301f4db1d02937a4ab3ccce137717b81826e79b7d49ffe3244a13c3b8"
+dependencies = [
+ "cfg-if",
+ "cpufeatures 0.2.17",
+ "digest 0.10.7",
+]
+
+[[package]]
+name = "sha2"
+version = "0.11.0"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "446ba717509524cb3f22f17ecc096f10f4822d76ab5c0b9822c5f9c284e825f4"
+dependencies = [
+ "cfg-if",
+ "cpufeatures 0.3.1",
+ "digest 0.11.3",
+]
+
+[[package]]
+name = "syn"
+version = "2.0.119"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "872831b642d1a07999a962a351ed35b955ea2cfc8f3862091e2a240a84f17297"
+dependencies = [
+ "proc-macro2",
+ "quote",
+ "unicode-ident",
+]
+
+[[package]]
+name = "syn"
+version = "3.0.4"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "e6275cddf4610d1775e6d1fe9469b2e77d0f39fd98fb7450901b821e0c53649f"
+dependencies = [
+ "proc-macro2",
+ "quote",
+ "unicode-ident",
+]
+
+[[package]]
+name = "tracing"
+version = "0.1.44"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "63e71662fa4b2a2c3a26f570f037eb95bb1f85397f3cd8076caed2f026a6d100"
+dependencies = [
+ "pin-project-lite",
+ "tracing-attributes",
+ "tracing-core",
+]
+
+[[package]]
+name = "tracing-attributes"
+version = "0.1.31"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "7490cfa5ec963746568740651ac6781f701c9c5ea257c58e057f3ba8cf69e8da"
+dependencies = [
+ "proc-macro2",
+ "quote",
+ "syn 2.0.119",
+]
+
+[[package]]
+name = "tracing-core"
+version = "0.1.36"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "db97caf9d906fbde555dd62fa95ddba9eecfd14cb388e4f491a66d74cd5fb79a"
+dependencies = [
+ "once_cell",
+]
+
+[[package]]
+name = "typenum"
+version = "1.20.1"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "b6f5e870be6c3b371b77fe0ee0bafb859fa4964b4404c27de1d380043c4dda20"
+
+[[package]]
+name = "unicode-ident"
+version = "1.0.24"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "e6e4313cd5fcd3dad5cafa179702e2b244f760991f45397d14d4ebf38247da75"
+
+[[package]]
+name = "version_check"
+version = "0.9.5"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "0b928f33d975fc6ad9f86c8f283853ad26bdd5b10b7f1542aa2fa15e2289105a"
+
+[[package]]
+name = "zmij"
+version = "1.0.23"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "29666d0abbfad1e3dc4dcf6144730dd3a3ab225bbbdac83319345b1b44ccfc1b"
+
+[[patch.unused]]
+name = "freemkv-engine"
+version = "1.7.0"
+
+[[patch.unused]]
+name = "freemkv-i18n"
+version = "1.7.0"
+
+[[patch.unused]]
+name = "freemkv-keysources"
+version = "1.7.0"
+
+[[patch.unused]]
+name = "libfreemkv"
+version = "1.7.0"

Cargo.toml

@@ -1,8 +1,8 @@
 [package]
 name = "freemkv-unlock"
-version = "1.6.11"
+version = "1.7.1"
 edition = "2024"
-rust-version = "1.90"
+rust-version = "1.88"
 license = "MIT"
 description = "Unlock layer for the freemkv toolchain: the Unlocker contract + self-contained firmware/AACS/CSS unlocker modules. libfreemkv depends on this and dispatches via all_unlockers()."
 repository = "https://github.com/freemkv/freemkv-unlock"
@@ -26,3 +26,6 @@ sha1 = "0.10"
 sha2 = "0.11"
 aes = "0.9"
 rand = "0.10"
+
+[lints.clippy]
+uninlined_format_args = "allow"

NOTICE

@@ -0,0 +1,24 @@
+NOTICE — Third-Party Drive Microcode
+====================================
+
+freemkv-unlock's own source code is licensed under the MIT License (see LICENSE).
+The MIT License does NOT cover the drive-microcode payloads bundled in
+`src/ld/profiles.json`.
+
+Those payloads are volatile, RAM-only optical-drive microcode ("LibreDrive")
+authored and owned by the MakeMKV team (GuinpinSoft inc.). Each payload embeds
+its own copyright notice, e.g.:
+
+    "This is volatile RAM-only microcode for LibreDrive by MakeMKV -
+     http://www.makemkv.com/libredrive/
+     Copyright (c) MakeMKV team, all rights reserved."
+
+They are included solely to recognize supported drives and perform the
+non-persistent, RAM-only unlock in interoperation with MakeMKV's LibreDrive, on
+drives their owners already possess. They are third-party proprietary material —
+**Copyright (c) MakeMKV team, all rights reserved** — and are **NOT** licensed
+under MIT. No ownership is claimed or transferred.
+
+freemkv is an independent project, not affiliated with, endorsed by, or
+sponsored by MakeMKV / GuinpinSoft inc. With thanks to the MakeMKV team for
+LibreDrive. See https://www.makemkv.com/ and http://www.makemkv.com/libredrive/.

README.md

@@ -36,4 +36,13 @@ for u in freemkv_unlock::all_unlockers() {
 To remove an unlocker, delete its module directory and its one line in
 `all_unlockers()` — nothing else changes.
 
-License: MIT.
+## License
+
+freemkv-unlock's own source is MIT — see [LICENSE](LICENSE).
+
+`src/ld/profiles.json` bundles volatile, RAM-only **LibreDrive** microcode
+authored and owned by the **MakeMKV team** (GuinpinSoft inc.) — *Copyright (c)
+MakeMKV team, all rights reserved.* That microcode is third-party proprietary
+material, **not** covered by the MIT license; no ownership is claimed. See
+[`NOTICE`](NOTICE). With thanks to the MakeMKV team for
+[LibreDrive](http://www.makemkv.com/libredrive/).

ci/comment-guard.py

@@ -0,0 +1,252 @@
+#!/usr/bin/env python3
+"""comment-guard: cap explanatory prose by COMMENT INTENT, not visibility.
+
+Three tiers, keyed on the syntax the author reached for:
+
+  * `//!` / `/*!` — a module (or crate) design doc. UNBOUNDED. There is one per
+    file, at the top, documenting the module as a whole, so it can't be used to
+    smuggle per-item rationale the way item docs can. Long correctness / design
+    essays (e.g. a memory-ordering proof) legitimately live here.
+  * `///` / `/** */` — item documentation. Up to DOC_CAP *prose* lines. Fenced
+    ``` code blocks do NOT count, so real examples aren't punished. This holds
+    whether or not the item is `pub`: a `pub(crate)` fn deserves a real doc
+    paragraph just like a `pub` one. Capping `///` here (rather than exempting
+    it, as the old guard did — issue #17) is what stops rationale being hidden
+    behind a doc sigil, while still leaving room for a genuine paragraph.
+  * `//` / `/* */` — a quick internal aside. Up to INLINE_CAP lines. This is the
+    shape that sprawls into walls, so it's kept tight: stanzas separated by a
+    single blank line count CUMULATIVELY, so one essay can't be split into runs.
+
+Overflow belongs in the module `//!`, or in docs/<topic>.md with a one-line `//`
+pointer — both show up in review.
+
+Scans src/ and tests/ under the given root (default cwd). `--selftest` runs
+built-in cases. Exit non-zero if any block exceeds its cap.
+"""
+import re
+import sys
+from pathlib import Path
+
+INLINE_CAP = 3
+DOC_CAP = 8
+
+_RAW_OPEN = re.compile(r'r(#*)"')  # raw string opener: r"…", r#"…"#, br##"…"## …
+
+
+def _strip_line_comment(s):
+    """Return (kind, body) for a whole-line comment, else (None, None).
+
+    kind is 'doc' for `///`/`//!`, 'plain' for other `//`. A trailing comment on
+    a code line is not a whole-line comment and is ignored (single-line, cheap).
+    """
+    if s.startswith("//!"):
+        return "doc", s[3:]
+    if s.startswith("///") and not s.startswith("////"):
+        return "doc", s[3:]
+    if s.startswith("//"):
+        return "plain", s[2:]
+    return None, None
+
+
+def _fence_toggle(body):
+    """True if this doc body line opens/closes a ``` fenced code block."""
+    return body.strip().startswith("```")
+
+
+def _classify(lines):
+    """Yield per-line dicts: comment kind ('doc'/'plain'/None), doc-ness of
+    block comments, and code info. Block comments are tracked across lines.
+    """
+    info = []
+    in_block = False
+    block_is_doc = False
+    raw_hashes = None  # None = not in a raw string; int N = open, needs '"' + N*'#'
+    for raw in lines:
+        s = raw.lstrip()
+        rec = {"raw": raw, "s": s, "comment": None, "is_doc": False, "code": False}
+        # Inside a raw string literal (e.g. an embedded HTML/CSS/JS asset): the
+        # bytes are string data, not Rust comments, so the guard is blind to them
+        # until the string closes. Prevents `/* */` or `//` lines in an embedded
+        # page from being counted as comment blocks.
+        if raw_hashes is not None:
+            if ('"' + "#" * raw_hashes) in raw:
+                raw_hashes = None
+            info.append(rec)
+            continue
+        if in_block:
+            rec["comment"] = "doc" if block_is_doc else "plain"
+            rec["is_doc"] = block_is_doc
+            if "*/" in raw:
+                in_block = False
+            info.append(rec)
+            continue
+        # A block comment only starts a comment-line if it's the first token.
+        if s.startswith("/*"):
+            rec["is_doc"] = s.startswith("/**") or s.startswith("/*!")
+            rec["comment"] = "doc" if rec["is_doc"] else "plain"
+            if "*/" not in s[2:]:
+                in_block = True
+                block_is_doc = rec["is_doc"]
+            info.append(rec)
+            continue
+        kind, _ = _strip_line_comment(s)
+        if kind is not None:
+            rec["comment"] = kind
+            rec["is_doc"] = kind == "doc"
+            info.append(rec)
+            continue
+        # Code (or blank). Blank = empty after strip.
+        rec["code"] = s != ""
+        # A code line may OPEN a raw string that spans following lines. Detected
+        # only on code (comments were handled above), so `// see r"x"` is safe.
+        m = _RAW_OPEN.search(raw)
+        if m and ('"' + "#" * len(m.group(1))) not in raw[m.end():]:
+            raw_hashes = len(m.group(1))
+        info.append(rec)
+    return info
+
+
+def violations(path):
+    lines = path.read_text(encoding="utf-8", errors="replace").splitlines()
+    info = _classify(lines)
+    n = len(info)
+    out = []
+
+    i = 0
+    while i < n:
+        rec = info[i]
+        if rec["comment"] is None:
+            i += 1
+            continue
+
+        start = i
+        if rec["is_doc"]:
+            # Doc run: contiguous doc lines. `//!` / `/*!` module docs are
+            # UNBOUNDED; any other doc run caps PROSE (fenced ``` excluded) at
+            # DOC_CAP, regardless of the documented item's visibility.
+            j = i
+            prose = 0
+            in_fence = False
+            module_doc = False
+            while j < n and info[j]["comment"] == "doc":
+                s = info[j]["s"]
+                if s.startswith("//!") or s.startswith("/*!"):
+                    module_doc = True
+                body = (
+                    s[3:] if (s.startswith("///") or s.startswith("//!")) else s
+                )
+                if _fence_toggle(body):
+                    in_fence = not in_fence
+                elif not in_fence and body.strip() != "":
+                    prose += 1
+                j += 1
+            if not module_doc and prose > DOC_CAP:
+                out.append((start + 1, prose, DOC_CAP, "doc prose"))
+            i = j
+            continue
+
+        # Internal (plain) run, with single-blank bridging (cumulative).
+        j = i
+        count = 0
+        while j < n:
+            r = info[j]
+            if r["comment"] == "plain":
+                count += 1
+                j += 1
+                continue
+            if r["comment"] == "doc":
+                break
+            if r["comment"] is None and not r["code"]:
+                # blank: bridge only a SINGLE blank, else stop
+                if j + 1 < n and info[j + 1]["comment"] == "plain":
+                    j += 1
+                    continue
+                break
+            break  # code
+        if count > INLINE_CAP:
+            out.append((start + 1, count, INLINE_CAP, "inline comment"))
+        i = j
+    return out
+
+
+def main(argv):
+    if "--selftest" in argv:
+        return _selftest()
+    root = Path(argv[1]) if len(argv) > 1 else Path.cwd()
+    files = sorted(p for d in ("src", "tests") for p in (root / d).rglob("*.rs"))
+    total = 0
+    for f in files:
+        for start, length, cap, kind in violations(f):
+            rel = f.relative_to(root)
+            print(f"{rel}:{start}: {kind} block is {length} lines (max {cap})")
+            total += 1
+    if total:
+        print(f"\ncomment-guard: {total} comment block(s) exceed their cap.")
+        print(
+            f"Inline // asides cap at {INLINE_CAP}; /// item-doc prose caps at "
+            f"{DOC_CAP} (fenced examples excluded); //! module docs are unbounded. "
+            "Trim, move the rationale into the module //!, or docs/<topic>.md "
+            "with a // pointer."
+        )
+        return 1
+    return 0
+
+
+def _selftest():
+    import tempfile
+
+    cases = [
+        # (rust source, expected number of violations)
+        # short /// doc -> ok
+        ("/// short doc\npub fn a() {}\n", 0),
+        # /// on a PRIVATE item now gets the doc cap (8), so 4 lines is fine
+        ("/// l1\n/// l2\n/// l3\n/// l4\nfn priv_a() {}\n", 0),
+        # ...but /// is NOT unlimited: 9 prose lines on a private item fails
+        ("".join(f"/// l{k}\n" for k in range(9)) + "fn priv_b() {}\n", 1),
+        # 8 prose /// on a pub item -> ok (boundary)
+        ("".join(f"/// l{k}\n" for k in range(8)) + "pub fn c() {}\n", 0),
+        # 9 prose /// on a pub item -> violation
+        ("".join(f"/// l{k}\n" for k in range(9)) + "pub fn d() {}\n", 1),
+        # /// with a long fenced example -> example excluded, ok
+        (
+            "/// summary\n/// # Examples\n/// ```\n"
+            + "".join("/// code\n" for _ in range(12))
+            + "/// ```\npub fn e() {}\n",
+            0,
+        ),
+        # pub(crate) gets the SAME doc cap as pub (no pub/private split anymore)
+        ("".join(f"/// l{k}\n" for k in range(6)) + "pub(crate) fn f() {}\n", 0),
+        # bare inline 4 lines -> violation
+        ("// a\n// b\n// c\n// d\nlet x = 1;\n", 1),
+        # blank-split dodge: 3 + blank + 3 inline -> cumulative -> violation
+        ("// a\n// b\n// c\n\n// d\n// e\n// f\nfn z() {}\n", 1),
+        # plain /* */ block, 5 lines -> inline cap -> violation
+        ("/* one\n two\n three\n four\n five */\nfn q() {}\n", 1),
+        # comment-like lines INSIDE a raw string are string data, not comments
+        (
+            'const H: &str = r##"<style>\n'
+            + "".join("/* css note */\n" for _ in range(8))
+            + '</style>"##;\nfn h() {}\n',
+            0,
+        ),
+        # //! module doc may run long -> EXEMPT (no cap)
+        ("".join(f"//! l{k}\n" for k in range(40)) + "\npub fn m() {}\n", 0),
+        # /*! block module doc may run long -> EXEMPT
+        ("/*! l0\n" + "".join(f" l{k}\n" for k in range(30)) + "*/\nfn n() {}\n", 0),
+    ]
+    ok = True
+    with tempfile.TemporaryDirectory() as d:
+        for idx, (src, want) in enumerate(cases):
+            p = Path(d) / f"case{idx}.rs"
+            p.write_text(src)
+            got = len(violations(p))
+            flag = "ok" if got == want else "FAIL"
+            if got != want:
+                ok = False
+            print(f"  selftest {idx}: want {want} got {got} [{flag}]")
+    print("selftest PASSED" if ok else "selftest FAILED")
+    return 0 if ok else 1
+
+
+if __name__ == "__main__":
+    sys.exit(main(sys.argv))

deny.toml

@@ -0,0 +1,42 @@
+# cargo-deny — gates the RESOLVED dependency graph (issue #15).
+# Runs in CI against the committed Cargo.lock: RUSTSEC advisories, a permissive
+# licence allow-list, and source/ban hygiene. Update the lock (cargo update)
+# to clear an advisory rather than adding an `ignore` entry without cause.
+
+[advisories]
+yanked = "deny"
+# ignore = ["RUSTSEC-0000-0000"]  # only with a written justification
+
+[licenses]
+# The whole tree is permissive. r-efi's LGPL arm is satisfied by Apache-2.0
+# (its expression is "Apache-2.0 OR LGPL-2.1-or-later"), so no copyleft is
+# allowed here. Add a licence only when a real dependency introduces it.
+allow = [
+    "MIT",
+    "Apache-2.0",
+    "Apache-2.0 WITH LLVM-exception",
+    "0BSD",
+    "Unicode-3.0",
+    "Unlicense",
+    "Zlib",
+    "BSD-2-Clause",
+    "BSD-3-Clause",
+    "ISC",
+    "CDLA-Permissive-2.0",
+]
+confidence-threshold = 0.9
+
+[bans]
+multiple-versions = "warn"   # transitive dupes (digest 0.10/0.11 etc.) are noise, not a gate
+wildcards = "deny"
+# The freemkv siblings resolve as path deps under CI's path-patch layout, which
+# reads as a wildcard version req. That is internal, not a supply-chain wildcard.
+allow-wildcard-paths = true
+
+[sources]
+unknown-registry = "deny"
+unknown-git = "deny"
+# The freemkv siblings are first-party git-tag deps; allow git sources from
+# our own org only (CI resolves them as git, not the local path override).
+[sources.allow-org]
+github = ["freemkv"]

docs/aacs-handshake.md

@@ -0,0 +1,184 @@
+# AACS bus authentication handshake
+
+## Flow
+
+1. Invalidate AGIDs → allocate fresh AGID
+2. Send host certificate + nonce
+3. Receive drive certificate + nonce
+4. Receive drive key point + signature, verify
+5. Sign host key point, send
+6. ECDH: host_priv × drive_key_point → bus key (low 128 bits of x)
+7. Read VID or Read Data Keys (encrypted with bus key)
+
+## Why `scsi_read`/`scsi_write` check status at the seam
+
+The contract this crate is built on (`ScsiTransport::execute`) returns `Ok`
+on a SCSI sense and `Err` ONLY on a transport-layer fault. These two helpers
+used to propagate the `?` and nothing else — so a CHECK CONDITION (`Ok`,
+non-zero `status`) handed the caller back its own ZERO-FILLED buffer and
+every REPORT/SEND KEY step consumed zeros as valid drive data: a drive
+certificate of zeros, a key point of zeros, a Volume ID of zeros, all
+reported as success. It has not fired in production only because the sole
+transport implementor violates the contract in the opposite direction (it
+returns `Err` for any non-zero status). Plugging in a CONFORMING transport
+would have turned the whole handshake into a zero-buffer success, so the
+status and the transferred length are checked here, at the seam.
+
+## Why `scsi_write` shares this reasoning
+
+Same contract reasoning as `scsi_read`: a drive that REFUSES the host
+certificate answers with `Ok` + CHECK CONDITION, and treating that as a
+successful send carried the handshake on against a drive that never
+accepted it.
+
+## `ec_mul` design notes
+
+**Constant-time tradeoff:** this branches on `scalar.bit(0)` and clones
+BigUints per iteration, so its timing is data-dependent on the secret scalar
+(the long-term host private key in `ecdsa_sign`, the ephemeral key in ECDH).
+This is deliberate: the handshake runs once per disc against a local optical
+drive, so throughput and the narrow local-timing surface do not justify
+pulling in a vetted constant-time backend. Revisit if this ever signs in a
+remote/shared context.
+
+**Cofactor:** both AACS curves used here have cofactor 1, so a point that
+lies on the curve is automatically in the prime-order subgroup — no
+small-subgroup defense / `n·P == O` check is required for the inputs this is
+called with.
+
+## `verify_cert_p256` — provisional AACS 2.0 cert layout
+
+Verifies an AACS 2.0 drive certificate (type 0x11) against an AACS 2.0 LA
+key. PROVISIONAL LAYOUT — the byte offsets below are NOT yet confirmed
+against a genuine AACS 2.0 cert, and no such cert exists on this machine to
+pin them (an exhaustive hunt found only 2.0 *content* certs, a different
+format; a real host/drive AKE cert needs a live USB/SCSI capture). What IS
+confirmed: first, the P-256/SHA-256 ECDSA primitive and curve constants are
+correct (`ecdsa_verify_p256_verifies_a_genuine_aacs2_content_cert` verifies a
+real 2.0 content cert under the published CC key); second, the 132-byte
+4-byte-header framing MATCHES the step-6 drive-key message (x[4..36] /
+y[36..68] / r[68..100] / s[100..132]), the natural 20→32-byte scaling of the
+established AACS 1.0 84-byte key-message framing.
+
+What is NOT confirmed: that a *certificate* shares that framing. No known
+implementation verifies 2.0 certs against a confirmed layout (the published
+spec draft leaves that format ambiguous), and the AACS 2.0 draft architecture
+lists the 2.0 host cert (type 0x12) with a 6-byte Host ID + a NEW 4-byte
+paired-Device-Key-Set field before the 64-byte public key — implying a
+~140–144-byte cert with the pubkey near offset 12–16 and a signed range of
+~76–80 bytes, NOT 68. The drive cert (type 0x11) has no paired-DKS field
+(~140 bytes, pubkey ~[12..76], signed ~first 76). So the offsets HERE
+(pubkey [4..68], signed [..68]) are a best-effort self-consistent
+placeholder, chosen to match the confirmed key message; they will almost
+certainly move when a real cert lands (see the `#[ignore]` real-cert slot).
+This matters for USEFULNESS, not SAFETY: a wrong signed range makes
+verification FAIL, and since the caller treats a verify failure as FATAL,
+the path fails CLOSED (rejects) rather than trusting anything — never the
+reverse.
+
+The `>= 132` guard keeps the `cert[100..132]` slice safe while letting a
+full-length cert reach verification (the old `>= 138` guard rejected every
+132-byte input unconditionally).
+
+The LA anchor is a PARAMETER, not the compiled-in constant, so a unit test
+can drive the whole chain under a test LA keypair (sign a synthetic drive
+cert, prove it is accepted / a bad-sig cert is rejected) without the real
+anchor's private half, which no one has. Production always threads
+`AACS2_LA_PUB_X/Y` from `aacs2_authenticate_p256`.
+
+## Why an unknown drive-cert type must be rejected
+
+This used to be an `if` / `else if` with NO `else`, so an unknown type
+(e.g. 0x02) fell straight through: `is_aacs20` stayed false, so the step-6
+key-signature check still ran — against `cert_pub_key(drive_cert)`, the
+drive's OWN key lifted out of a cert that was never checked against the LA.
+A rogue USB bridge could set byte 0 to 0x02, present its own keypair, sign
+`host_nonce || drive_key_point`, and the handshake would return Ok with an
+attacker-chosen bus key. The chain of trust must not appear to have run
+when it did not.
+
+## AACS 2.0 chain-of-trust gate (`aacs2_authenticate_p256_with_agid`)
+
+The SAME shape the AACS 1.0 sibling (`aacs_authenticate_with_agid`)
+enforces, now MANDATORY because this path is live. Two distinct rejections,
+both BEFORE any drive key is trusted:
+
+**(a) Cert TYPE gate.** An AACS 2.0 drive cert is type 0x11. Anything else
+is unexpected on the native P-256 AKE and is REJECTED here, never allowed to
+fall through. The earlier `if type==0x11 && !verify` form left NO branch for
+a non-0x11 type: a rogue bridge could send a type-0x10 (or any) cert, skip
+verification entirely, and still reach the step-6 key check against
+`cert_pub_key_p256(drive_cert)` — its OWN key, lifted from a cert the LA
+never signed — winning an attacker-chosen bus key. The chain of trust must
+not merely LOOK like it ran.
+
+**(b) Cert VERIFY is FATAL.** A type-0x11 cert whose P-256/SHA-256 signature
+does not verify under the real 2.0 LA anchor (`AACS2_LA_PUB`, on-curve — see
+`la_anchor_keys_are_on_curve`) is REJECTED, not logged-and-continued.
+`ecdsa_verify_p256` range/curve-checks every point (`point_on_curve`), so a
+forged or off-curve cert key cannot slip past. The previous "non-fatal for
+backward compat" behaviour was the exact hole the SECURITY note demanded be
+closed before wiring.
+
+## `run_cert_handshake` — cert-agnostic AKE selection
+
+The AACS 1.0 host cert is always present and is tried FIRST: an AACS 2.0
+drive accepts a 1.0 host cert for backward compatibility, which is enough to
+learn the Volume ID. If the drive REFUSES the 1.0 cert (a plain cert-level
+rejection — NOT a dead or wedged bus) and THIS host cert also carries AACS
+2.0 (P-256) credentials, fall back to the native P-256 AKE.
+
+This does NOT branch on "do we possess 2.0 certs?" as a feature switch: it
+consumes whatever the supplied `HostCert` carries, exactly as the 1.0 path
+consumes `hc.certificate`. A host cert without v2 credentials simply has
+nothing more to present and keeps its 1.0 error; a host cert WITH them
+drives the P-256 AKE the moment the drive asks for it. When a real 2.0 host
+cert is supplied at runtime, the P-256 handshake runs — no code change, no
+possession gate.
+
+A transport fault on the 1.0 attempt short-circuits the fallback:
+re-running the P-256 AKE against a bus that is gone re-issues AGID
+invalidation + SEND/REPORT KEY for nothing and buries the real (replug)
+cause. The `Err` arm below still classifies it as `Transport`.
+
+## AACS 2.0 (P-256) host-certificate handshake — live-path test plumbing
+
+No genuine 2.0 host cert exists for us to sign with (a side agent is hunting
+one; see the `#[ignore]` slot at the end). So the tests below prove the
+PLUMBING under a self-generated test LA keypair: a 132-byte drive cert
+signed by the test LA is threaded through the SAME code the production
+anchor uses, only the anchor differs (a parameter, not a recompile). The
+layout mirrored is the code's self-consistent 4-byte-header framing —
+header(4)+pub_x(32)+pub_y(32)+sig_r(32)+sig_s(32)=132, LA signs SHA-256 over
+cert[..68] — identical to the step-6 drive-key message framing and the
+156-byte SEND KEY host-cert payload (4+20 nonce+132 cert).
+
+## `ecdsa_verify_p256_verifies_a_genuine_aacs2_content_cert` — why it matters
+
+SANITY: `ecdsa_verify_p256` + the P-256 curve constants + SHA-256 are correct
+against GENUINE AACS 2.0 material. No real 2.0 host/drive AKE cert exists
+anywhere on this machine (an exhaustive hunt found none — it needs a live
+USB/SCSI AKE capture), but a real AACS 2.0 CONTENT certificate does
+(`research/aacs/iso-aacs/CivilWar/Content000.cer`, type 0x10, 232 bytes). It
+is signed by the AACS 2.0 *Content Cert* key (P-256/SHA-256, 64-byte
+signature over the first `signed_len` bytes) — a DIFFERENT key for a
+DIFFERENT purpose than the AKE LA anchor, but it exercises the EXACT same
+`ecdsa_verify_p256` primitive `verify_cert_p256` relies on; only the pubkey
+and the offsets differ. Proving it verifies here pins that the P-256
+ECDSA/SHA-256 math and the `point_on_curve` gate are right, so when a
+genuine 2.0 host/drive cert (and its correct signed range) lands, only the
+offsets — not the crypto — remain to be fixed.
+
+The CC pubkey is the published AACS 2.0 Content Certificate public key
+(P-256). Corrupting any `P256_*` constant, or removing SHA-256, makes this
+real-vector verification go red.
+
+## Cert support
+
+- AACS 1.0: custom 160-bit curve, SHA-1, 20-byte keys
+- AACS 2.0: an AACS 2.0 drive accepts an AACS 1.0 host cert for backward
+  compatibility (that yields the Volume ID), and the native P-256/SHA-256
+  AACS 2.0 AKE is the fallback when the drive refuses the 1.0 cert AND the
+  supplied host cert carries AACS 2.0 (P-256) credentials. Both paths are
+  LIVE and share the same chain-of-trust hardening; see
+  `run_cert_handshake` for the cert-agnostic dispatch.

docs/drive-profile-visibility.md

@@ -0,0 +1,16 @@
+# `DriveProfile` field visibility and `allow(dead_code)`
+
+Only `identity` and `signature` are public — they are what the catalog is
+public FOR ("is this drive supported?", and the emulator's impersonation).
+Everything below them is unlock MECHANISM: the MediaTek firmware image and
+the per-drive vendor CDB templates. `Cargo.toml` says this crate is never
+published because it carries drive firmware; re-exporting `DriveProfile`
+with a `pub firmware: Vec<u8>` handed that firmware, and every vendor CDB,
+to any downstream crate that called `ld::profile()`. They are `pub(crate)`
+so the mechanism stays inside the unlocker.
+
+`allow(dead_code)`: this is the on-disk catalog schema, and several
+captured CDB templates have no in-crate consumer yet. They were `pub`
+before, so the compiler saw the (external) use; making them `pub(crate)`
+is what exposes them as unread. Deleting them would silently drop fields
+from the format the profile-extraction pipeline emits.

docs/module-visibility.md

@@ -0,0 +1,29 @@
+# Module visibility rationale (`src/lib.rs`)
+
+Why each unlocker module is `pub mod` or `mod`, and what's actually public
+about it. The consumer never reaches an unlocker impl directly — only through
+[`all_unlockers`] — so "public" here means "exposes something other than the
+unlocker impl itself."
+
+## `ld`
+
+Public ONLY for its drive-profile catalog (`ld::profiles` / the `Profiles`
+object) and, under the `emulation` feature, the unlock-handshake wire format
+the bdemu test-emulator needs. The unlocker impl itself (`Mt1959Unlocker`) is
+`pub(crate)` — clients still reach unlockers only through `all_unlockers`.
+`aacs` and `css` carry no such public catalog, so they stay fully private.
+
+## `renesas`
+
+Public for its `is_renesas` drive-probe, which reports a dead bus as
+`Err(UnlockError::Transport)` rather than "not a Renesas drive" — a
+distinction the consumer needs before it can trust a negative probe result.
+The unlocker impl (`Renesas`) is `pub(crate)` — reached only through
+`all_unlockers`.
+
+## `freemkv`
+
+Carries no public catalog (the firmware self-identifies rather than matching
+a bundled profile) and no emulation wire format yet, so it stays fully
+private — the unlocker impl (`FreemkvUnlocker`) is reached only through
+`all_unlockers`.

docs/scsi-mock-fixture.md

@@ -0,0 +1,9 @@
+# `scsi::mock` — crate-wide mock transport
+
+The crate-wide mock transport. Before this existed, EVERY mock in the crate
+returned `Ok` with status 0 and a full `bytes_transferred` — so no test could
+ever exercise the three outcomes the contract actually distinguishes, and a
+whole class of transport-fault misclassification bugs sat uncaught (a dead bus
+read as "this unlocker doesn't apply", a zero-filled buffer read as valid
+drive data). This fixture can express all of them, and is the red-before-green
+vehicle for the transport-contract fixes across ld / aacs / css / renesas.

docs/unlocked-debug-redaction.md

@@ -0,0 +1,10 @@
+# Why `Unlocked`'s `Debug` impl is hand-written
+
+`Unlocked` is the public carrier that leaves this crate, and both `bus_key`
+(the AACS bus key) and `vid` (the Volume ID that feeds VUK derivation) are key
+material that must never reach a log or a test-failure message in plaintext.
+The `AacsAuth` / `CertHandshake` siblings already redact; a derived `Debug`
+here would print the very bytes they hide.
+
+Presence is still observable (`Some`/`None`) so a log can say WHETHER a key
+was obtained without revealing WHAT it is.

src/aacs/mod.rs

@@ -12,7 +12,7 @@ use aes::Aes128;
 use aes::cipher::{Array, BlockCipherDecrypt, KeyInit};
 
 use crate::scsi::ScsiTransport;
-use crate::{DiscKind, UnlockCtx, UnlockError, Unlocked, Unlocker};
+use crate::{DiscKind, HostCert, UnlockCtx, UnlockError, Unlocked, Unlocker};
 
 /// AES-128-ECB decrypt a single 16-byte block — used to decrypt the bus key /
 /// read_data_key the drive returns after the handshake.
@@ -25,53 +25,63 @@ pub(crate) fn aes_ecb_decrypt(key: &[u8; 16], data: &[u8; 16]) -> [u8; 16] {
     out
 }
 
-/// The AACS host-certificate unlocker. Matches a Blu-ray/UHD disc
-/// (`DiscKind::Aacs`) and runs the cert handshake against the host certs the
-/// consumer collected (via [`UnlockCtx::host_certs`]), learning the Volume ID
-/// and — on AACS 2.0 — the bus key.
-pub struct AacsCert;
+/// The AACS host-certificate unlocker — the fallback for a drive with NO vendor
+/// unlock (can't be raw-read-enabled), so the bus is removed by a real host-cert
+/// AKE + bus key rather than a CDB. The host certs are injected at CONSTRUCTION
+/// (the one place certs enter the system). Owns its AKE crypto — no reach into
+/// libfreemkv.
+pub struct AacsUnlocker {
+    host_certs: Vec<HostCert>,
+}
 
-impl AacsCert {
-    pub fn new() -> Self {
-        AacsCert
+impl AacsUnlocker {
+    pub fn new(host_certs: Vec<HostCert>) -> Self {
+        AacsUnlocker { host_certs }
     }
 }
 
-impl Default for AacsCert {
-    fn default() -> Self {
-        Self::new()
-    }
-}
-
-impl Unlocker for AacsCert {
+impl Unlocker for AacsUnlocker {
     fn name(&self) -> &'static str {
         "AACS"
     }
 
-    /// AACS removes BUS encryption via the host-cert handshake; it provides no
-    /// drive features. Self-guards on the disc kind (the consumer iterates every
-    /// unlocker's `unlock_bus`, so a non-AACS disc must decline here).
-    fn unlock_bus(
+    /// Remove AACS bus encryption via the host-cert handshake. Like every other
+    /// unlocker this DOES unlock the drive — just with a cert + AKE instead of a
+    /// vendor CDB. `Some` on a successful handshake (VID + bus key learned);
+    /// `None` on a non-AACS disc, no usable cert, or a rejected handshake (fall
+    /// through); `Err(Transport)` on a dead bus.
+    fn unlock(
         &self,
         scsi: &mut dyn ScsiTransport,
         ctx: &UnlockCtx,
-    ) -> std::result::Result<Unlocked, UnlockError> {
-        if ctx.kind != DiscKind::Aacs {
-            return Err(UnlockError::NotApplicable);
+    ) -> std::result::Result<Option<Unlocked>, UnlockError> {
+        if ctx.kind != DiscKind::Aacs || self.host_certs.is_empty() {
+            // Wrong disc kind, or no host cert to authenticate with — the loop
+            // falls through to a VID-less / keysource path.
+            return Ok(None);
         }
-        if ctx.host_certs.is_empty() {
-            // No host cert to authenticate with — the consumer falls back to a
-            // VID-less / keysource path.
-            return Err(UnlockError::NoUsableHostCert);
-        }
-        let h = handshake::run_cert_handshake(scsi, ctx.host_certs)?;
-        Ok(Unlocked {
-            vid: Some(h.volume_id),
-            // Host-cert AKE path: bus removal depends on the bus key, not a
-            // firmware unlock.
-            bus_key: h.read_data_key,
-            drive_unlocked: false,
-        })
+        crate::fallthrough(
+            handshake::run_cert_handshake(scsi, &self.host_certs).map(|h| {
+                // A UHD disc whose bus-key fetch the drive refused (non-transport)
+                // returns Ok with read_data_key: None + read_data_key_err: Some —
+                // otherwise indistinguishable from an AACS-1.0 disc. Surface it.
+                if h.read_data_key.is_none()
+                    && let Some(code) = h.read_data_key_err
+                {
+                    tracing::warn!(
+                        target: "freemkv::disc",
+                        phase = "read_data_key_dropped",
+                        error_code = code,
+                        "AACS auth + VID succeeded but the drive served no read_data_key (bus key); \
+                         unlock reports bus_key: None"
+                    );
+                }
+                Unlocked {
+                    vid: Some(h.volume_id),
+                    bus_key: h.read_data_key,
+                }
+            }),
+        )
     }
 }
 
@@ -115,11 +125,11 @@ mod tests {
         crate::DriveId::default()
     }
 
-    /// `unlock_bus` self-guards on the disc kind: on a non-AACS disc it declines
-    /// (`NotApplicable`) WITHOUT touching the transport, so iterating it on a
-    /// CSS/unknown disc is safe.
+    /// Self-guards on the disc kind: on a non-AACS disc `unlock()` declines
+    /// (`Ok(false)`) WITHOUT touching the transport, even WITH a cert present —
+    /// so the reason is the kind, not a missing cert.
     #[test]
-    fn unlock_bus_declines_non_aacs_kinds() {
+    fn declines_non_aacs_kinds() {
         struct DeadTransport;
         impl ScsiTransport for DeadTransport {
             fn execute(
@@ -135,51 +145,41 @@ mod tests {
         let id = id();
         let mut t = DeadTransport;
         for k in [DiscKind::Unknown, DiscKind::Unencrypted, DiscKind::Css] {
-            let r = AacsCert::new().unlock_bus(&mut t, &UnlockCtx::new(&id, k, &[]));
-            assert_eq!(r.unwrap_err(), UnlockError::NotApplicable, "declines {k:?}");
+            assert!(
+                AacsUnlocker::new(vec![host_cert()])
+                    .unlock(&mut t, &UnlockCtx::new(&id, k))
+                    .expect("declines")
+                    .is_none(),
+                "declines {k:?}"
+            );
         }
     }
 
-    /// `Default` must delegate to `new()` (there is only one way to build an
-    /// `AacsCert`, so this pins the two constructors never drift apart).
+    // Full success path through `Unlocker::unlock`: a self-consistent AACS 1.0
+    // emulator proves the entry point learns `vid`/`bus_key` — and that the cert
+    // route reports `unlock() == true` (it DOES unlock the drive, via the cert).
     #[test]
-    #[allow(clippy::default_constructed_unit_structs)]
-    fn default_matches_new() {
-        let _ = AacsCert::default();
-        let _ = AacsCert::new();
-    }
-
-    /// THE full success path through `Unlocker::unlock_bus` itself: a
-    /// self-consistent AACS 1.0 drive emulator carries auth + VID + data-key
-    /// reads all the way to `Ok(Unlocked { .. })`, so the crate's public entry
-    /// point — not just `run_cert_handshake` underneath it — is proven to wire
-    /// `volume_id` and `read_data_key` into `Unlocked` correctly.
-    #[test]
-    fn unlock_bus_succeeds_end_to_end() {
+    fn unlock_succeeds_end_to_end() {
         let mut t = handshake::tests::DriveEmu::new();
         t.serve_data_keys = true;
         let id = id();
-        let certs = [host_cert()];
-        let ctx = UnlockCtx::new(&id, DiscKind::Aacs, &certs);
-        let unlocked = AacsCert::new()
-            .unlock_bus(&mut t, &ctx)
-            .expect("auth + VID + data-key reads all succeed");
-        assert_eq!(unlocked.vid, Some([0x5Au8; 16]));
-        assert!(unlocked.bus_key.is_some());
-        assert!(
-            !unlocked.drive_unlocked,
-            "cert path never sets drive_unlocked"
-        );
+        let ctx = UnlockCtx::new(&id, DiscKind::Aacs);
+        let out = AacsUnlocker::new(vec![host_cert()])
+            .unlock(&mut t, &ctx)
+            .expect("auth + VID + data-key reads all succeed")
+            .expect("the cert route unlocks the drive");
+        assert_eq!(out.vid, Some([0x5Au8; 16]));
+        assert!(out.bus_key.is_some());
     }
 
     fn host_cert() -> crate::HostCert {
         handshake::tests::dummy_cert()
     }
 
-    /// With no host certs there is nothing to authenticate with → NoUsableHostCert,
+    /// With no host certs there is nothing to authenticate with → `Ok(false)`,
     /// and the transport is never touched.
     #[test]
-    fn no_host_certs_is_no_usable_host_cert() {
+    fn no_host_certs_declines() {
         struct DeadTransport;
         impl ScsiTransport for DeadTransport {
             fn execute(
@@ -194,7 +194,11 @@ mod tests {
         }
         let id = id();
         let mut t = DeadTransport;
-        let r = AacsCert::new().unlock_bus(&mut t, &UnlockCtx::new(&id, DiscKind::Aacs, &[]));
-        assert_eq!(r.unwrap_err(), UnlockError::NoUsableHostCert);
+        assert!(
+            AacsUnlocker::new(vec![])
+                .unlock(&mut t, &UnlockCtx::new(&id, DiscKind::Aacs))
+                .expect("no certs declines")
+                .is_none()
+        );
     }
 }

src/css/error.rs

@@ -104,10 +104,9 @@ mod tests {
         );
     }
 
-    /// `is_transport_failure` is false for `CssAuthFailed`, false for a
-    /// `Scsi` error carrying a sense (a drive rejection, not a dead bus) even
-    /// at the transport-failure status, and true only for the genuine
-    /// transport fault (status + no sense).
+    // False for `CssAuthFailed` and for a `Scsi` error with a sense (a
+    // drive rejection, not a dead bus) even at transport-failure status;
+    // true only for the genuine transport fault (status + no sense).
     #[test]
     fn is_transport_failure_false_paths() {
         assert!(!Error::CssAuthFailed.is_transport_failure());

src/css/mod.rs

@@ -12,18 +12,9 @@ mod error;
 use crate::css::error::{Error, Result, step_err};
 use crate::scsi::{DataDirection, ScsiTransport};
 
-/// Issue ONE CSS bus-auth CDB, honouring the transport contract.
-///
-/// The single seam every bus-auth step now goes through. It closes two holes
-/// that each step open-coded before:
-///   * `Err` was collapsed to `CssAuthFailed`, so a dead bus looked like "this
-///     unlocker doesn't apply" and the consumer kept probing it ([`step_err`]);
-///   * `Ok` was never inspected, but per the contract a CHECK CONDITION arrives
-///     as `Ok` with a non-zero `status` — so a refused command handed the step
-///     back its own zero-filled buffer to parse as drive data.
-///
-/// `min_bytes` is the number of bytes the step must actually receive (0 for a
-/// write / no-data command, whose transferred count no transport promises).
+// Issue ONE CSS bus-auth CDB, honouring the transport contract: treats a
+// non-zero status as CssAuthFailed and checks bytes_transferred >= min_bytes
+// rather than trusting an unfilled buffer. See docs/css-mod.md#css_scsi.
 fn css_scsi(
     scsi: &mut dyn ScsiTransport,
     cdb: &[u8],
@@ -209,52 +200,47 @@ impl Default for DvdUnlocker {
 
 impl crate::Unlocker for DvdUnlocker {
     fn name(&self) -> &'static str {
-        // User-facing unlocker label. This unlocker's job is the DVD
-        // read-enablement bus-auth (it clears the drive's scrambled-read
-        // barrier and learns no key) — a property of the DVD medium, NOT of
-        // whether the content happens to be CSS-scrambled. Reporting it as
-        // "DVD" is honest: on any DVD the bus-auth ran; the CSS descramble
-        // itself is keyless and handled downstream, so it is not a separate
-        // "did an unlocker run" signal.
+        // "DVD" names the medium this bus-auth unlocks, not the CSS scheme:
+        // the bus-auth runs on any DVD regardless of whether the content is
+        // actually scrambled. See docs/css-mod.md#dvdunlocker-name.
         "DVD"
     }
 
-    /// CSS removes the scrambled-sector barrier (a bus-level concern); it
-    /// provides no drive features. Self-guards against the hardware (below), so
-    /// it declines cleanly when the consumer iterates it on a non-DVD.
-    fn unlock_bus(
+    /// CSS removes the scrambled-sector barrier via bus-auth. Self-guards
+    /// against the hardware (below), so it declines cleanly (`Ok(None)`) when the
+    /// consumer iterates it on a non-DVD. `Some` once the bus-auth runs;
+    /// `Err(Transport)` on a dead bus. Yields no VID and no bus key (descrambling
+    /// is keyless — the returned `Unlocked` is empty).
+    fn unlock(
         &self,
         scsi: &mut dyn ScsiTransport,
         _ctx: &crate::UnlockCtx,
-    ) -> std::result::Result<crate::Unlocked, crate::UnlockError> {
-        // Self-guard against the hardware — do NOT trust the caller-declared
-        // DiscKind alone. If the drive does not report a DVD profile, refuse
-        // (NotApplicable) WITHOUT issuing any CSS CDB, so a mis-routed
-        // Blu-ray/UHD is never sent CSS bus-auth.
+    ) -> std::result::Result<Option<crate::Unlocked>, crate::UnlockError> {
+        // Self-guard against the hardware, not just the caller-declared
+        // DiscKind: decline without issuing any CSS CDB when the drive doesn't
+        // report a DVD profile.
         if !mounted_disc_is_dvd(scsi)? {
             tracing::debug!(
                 target: "freemkv::css",
                 phase = "dvd_unlocker_not_dvd",
-                "DvdUnlocker invoked on a non-DVD profile; refusing (NotApplicable)"
+                "DvdUnlocker invoked on a non-DVD profile; declining"
             );
-            return Err(crate::UnlockError::NotApplicable);
+            return Ok(None);
         }
-        // The bus-auth handshake is what unlocks scrambled-sector reads; the lba
-        // is not consumed by the unlock primitive (the disc-key REPORT KEY is
-        // best-effort). CSS yields neither a Volume ID nor an AACS bus key.
-        unlock_css_reads(scsi, 0)?;
-        Ok(crate::Unlocked::default())
+        // Bus-auth unlocks scrambled-sector reads (the lba isn't consumed here;
+        // the disc-key REPORT KEY is best-effort). A refused handshake falls
+        // through (`fallthrough` → `Ok(None)`); only a dead bus is an `Err`.
+        crate::fallthrough(
+            unlock_css_reads(scsi, 0)
+                .map(|()| crate::Unlocked::default())
+                .map_err(crate::UnlockError::from),
+        )
     }
 }
 
-/// Transport-level "is the mounted disc a DVD?" probe (GET CONFIGURATION
-/// current-profile, DVD family `0x0010..=0x001F`). Lets the DvdUnlocker
-/// self-verify against the drive instead of trusting the caller's DiscKind.
-///
-/// `Err(Transport)` on a dead bus. This is the FIRST command the DVD unlocker
-/// issues, and its match had no `Err` arm at all: a transport fault fell into
-/// `_ => false`, which the caller turned into `NotApplicable` — a dead bus read
-/// as "not a DVD", with nothing logged anywhere on the path.
+// Probes GET CONFIGURATION current-profile (DVD family 0x0010..=0x001F) so
+// DvdUnlocker can self-verify against the drive rather than trust the
+// caller's DiscKind. See docs/css-mod.md#mounted_disc_is_dvd.
 fn mounted_disc_is_dvd(
     scsi: &mut dyn ScsiTransport,
 ) -> std::result::Result<bool, crate::UnlockError> {
@@ -305,19 +291,16 @@ fn mounted_disc_is_dvd(
 
 fn unlock_css_reads_inner(scsi: &mut dyn ScsiTransport, _lba: u32) -> Result<()> {
     tracing::debug!(target: "freemkv::css", "css unlock: begin");
-    // The bus-auth challenge-response sets the drive's Authentication Success
-    // Flag (ASF=1), which is what opens scrambled-sector reads. This is the
-    // ONLY step required to unlock reads; a failure here is fatal — we
-    // genuinely cannot read scrambled sectors.
+    // The bus-auth challenge-response sets ASF=1, which is what opens
+    // scrambled-sector reads. It's the only required step, so a failure
+    // here is fatal.
     let agid = establish_authenticated_session(scsi).inspect_err(|e| {
         tracing::warn!(target: "freemkv::css", error_code = e.code(), "css unlock: bus authentication failed");
     })?;
     tracing::debug!(target: "freemkv::css", agid, "css unlock: bus authentication ok");
-    // Disc-key REPORT KEY: issued BEST-EFFORT for any firmware that ties part
-    // of its read-unlock to it. The bytes are unused (the descramble key is
-    // recovered keylessly) and a failure is NON-FATAL — the gate is already
-    // open from bus-auth. This replaces the title-key REPORT KEY, whose hard
-    // failure used to abort the whole unlock (the 7014 bug on USB bridges).
+    // Disc-key REPORT KEY: best-effort only, for firmware that ties part of
+    // its read-unlock to it; bytes are unused and failure is non-fatal.
+    // See docs/css-mod.md#disc-key-report-key (7014 bug history).
     if let Err(e) = read_disc_key(scsi, agid) {
         tracing::debug!(target: "freemkv::css", error_code = e.code(), "css unlock: disc-key REPORT KEY skipped (non-fatal)");
     }
@@ -327,14 +310,8 @@ fn unlock_css_reads_inner(scsi: &mut dyn ScsiTransport, _lba: u32) -> Result<()>
 
 // ── Step 1: Bus Authentication ────────────────────────────────────────────
 
-/// Run the CSS bus-authentication challenge-response (invalidate AGIDs →
-/// allocate AGID → host challenge → brute-force the variant → drive challenge →
-/// send host key). Completing the handshake sets the drive's Authentication
-/// Success Flag (ASF=1) — which is the ENTIRE purpose: it unlocks
-/// scrambled-sector reads. Returns the negotiated AGID (the caller needs it for
-/// the best-effort disc-key REPORT KEY). The CSS bus key is intentionally NOT
-/// derived: descrambling is keyless — the key is recovered directly from the
-/// data — so the bus key has no consumer.
+// Runs the CSS bus-auth handshake to set ASF=1 (invalidate AGIDs, allocate,
+// challenge/response). Returns the AGID; no bus key is derived. See docs/css-mod.md.
 fn establish_authenticated_session(scsi: &mut dyn ScsiTransport) -> Result<u8> {
     // Invalidate all AGIDs via REPORT KEY format 0x3F. This is also what makes
     // an abandoned AGID self-heal — which is not a reason to abandon one.
@@ -453,22 +430,15 @@ fn authenticate_with_agid(scsi: &mut dyn ScsiTransport, agid: u8) -> Result<()>
     )?;
 
     // The authenticated session (ASF=1) is now established — scrambled-sector
-    // reads are unlocked, which is the only thing we needed. The CSS bus key
-    // would be CryptKey(2, variant, key1 || key2), but it has no consumer
-    // (descrambling is keyless — the key is recovered directly from the
-    // data), so it is not derived.
+    // reads are unlocked. The CSS bus key would be CryptKey(2, variant,
+    // key1 || key2), but has no consumer (descrambling is keyless).
     Ok(())
 }
 
 // ── Step 2: Disc Key ──────────────────────────────────────────────────────
 
-/// Issue READ DVD STRUCTURE format 0x02 (the Disc Key block — opcode 0xAD, NOT
-/// the REPORT KEY 0xA4 disc-key block; format 0x01 is Copyright Information)
-/// purely for the bus-auth unlock side
-/// effect. The returned block contents are not used — the descramble title key
-/// is recovered keylessly elsewhere, so the genuine disc-key REPORT KEY is
-/// intentionally skipped. (If a drive is ever found where bus-auth alone does
-/// not open scrambled reads, a real REPORT KEY format 0x02 belongs here.)
+// Issues READ DVD STRUCTURE format 0x02 (opcode 0xAD) purely for its
+// bus-auth side effect; returned bytes are unused. See docs/css-mod.md#read_disc_key.
 fn read_disc_key(scsi: &mut dyn ScsiTransport, agid: u8) -> Result<()> {
     // READ DVD STRUCTURE, format 0x02 (disc key), 2048+4 bytes
     let alloc_len: u16 = 2048 + 4;
@@ -493,10 +463,8 @@ fn read_disc_key(scsi: &mut dyn ScsiTransport, agid: u8) -> Result<()> {
 
 fn crypt_key(key_type: usize, variant: u8, challenge: &[u8; 10]) -> [u8; 5] {
     // key_type indexes PERM_CHALLENGE ([_;3]); variant indexes
-    // VARIANTS/PERM_VARIANT ([_;32]). All internal callers pass key_type in
-    // 0..3 and variant in 0..32; the asserts document the contract for the
-    // pub(crate) test entry point test_crypt_key and turn a would-be
-    // out-of-bounds panic into an explicit precondition violation.
+    // VARIANTS/PERM_VARIANT ([_;32]). Asserts turn an out-of-bounds index
+    // into an explicit precondition violation. See docs/css-mod.md.
     debug_assert!(key_type < 3, "crypt_key: key_type out of range");
     debug_assert!((variant as usize) < 32, "crypt_key: variant out of range");
     let perm = &PERM_CHALLENGE[key_type];
@@ -650,14 +618,9 @@ fn send_key_cdb(agid: u8, format: u8, param_len: u16) -> [u8; 12] {
 mod tests {
     use super::*;
 
-    /// SECURITY REGRESSION GUARD: no instrumentation in libfreemkv may emit
-    /// raw key material. Scan every source file for a `tracing` field that
-    /// binds a forbidden key name to a value-producing expression (`= expr`
-    /// or `%expr` / `?expr`). The only allowed forms are a string literal
-    /// (e.g. `disc_key = "<redacted>"`) or a `_fp` fingerprint field.
-    ///
-    /// This is a source-scan test (not a runtime capture) so it stays cheap
-    /// and catches re-introductions at compile/CI time.
+    // SECURITY REGRESSION GUARD: scans source files for a `tracing` field
+    // binding a forbidden key name to a value expression (only a string
+    // literal or `_fp` field is allowed). See docs/css-mod.md#key-guard.
     #[test]
     fn no_key_bytes_in_instrumentation() {
         use std::path::Path;
@@ -709,10 +672,9 @@ mod tests {
                         continue;
                     }
                     for &name in forbidden {
-                        // A fingerprint field (`<name>_fp = ...`) is allowed.
-                        // Match `<name>` followed by optional fingerprint
-                        // suffix then `=` and a value that is NOT a string
-                        // literal redaction marker.
+                        // A fingerprint field (`<name>_fp = ...`) is allowed;
+                        // match `<name>` then `=` with a value that is not a
+                        // string-literal redaction marker.
                         if let Some(idx) = line.find(name) {
                             let after = &line[idx + name.len()..];
                             let after = after.trim_start();
@@ -743,10 +705,9 @@ mod tests {
             }
         }
 
-        // Scan this crate's `src` plus the sibling workspace crates so the
-        // key-material logging guard covers every crate that can reach the
-        // CSS/AACS internals, not just libfreemkv. Missing sibling dirs (e.g.
-        // when building the crate standalone) are simply skipped.
+        // Scan this crate's `src` plus sibling workspace crates so the
+        // guard covers everything that can reach CSS/AACS internals.
+        // Missing sibling dirs (standalone builds) are simply skipped.
         let manifest = Path::new(env!("CARGO_MANIFEST_DIR"));
         let workspace = manifest.parent().unwrap_or(manifest);
         let mut violations = Vec::new();
@@ -796,15 +757,8 @@ mod tests {
 
     // ── CSS constant-table integrity ───────────────────────────────────────
 
-    /// Each PERM_CHALLENGE row is a permutation of indices 0..10 (it reorders
-    /// the 10 challenge bytes). A non-permutation would drop/duplicate
-    /// challenge bytes, weakening or corrupting the bus key derivation.
-    ///
-    /// Grounding: crypt_key does `scratch[i] = challenge[perm[i]]` for i in
-    /// 0..10 — perm must be a bijection on 0..10 to use every challenge byte
-    /// exactly once.
-    /// Mutation: change PERM_CHALLENGE[0] entry `9` to `8` (duplicate) -> the
-    /// "covers 0..10" assert fires.
+    // Each PERM_CHALLENGE row must be a permutation of indices 0..10; a
+    // non-permutation would drop/duplicate bytes. See docs/css-mod.md#perm-challenge-rows.
     #[test]
     fn perm_challenge_rows_are_permutations() {
         for (row, perm) in PERM_CHALLENGE.iter().enumerate() {
@@ -821,15 +775,9 @@ mod tests {
         }
     }
 
-    /// Each PERM_VARIANT row maps the 32 variants to 32 distinct 5-bit values
-    /// (it is a permutation of 0..32). key_type 1 uses PERM_VARIANT[0],
-    /// key_type 2 uses PERM_VARIANT[1] to pick the css_variant; a collision
-    /// would make two variants indistinguishable.
-    ///
-    /// Grounding: `css_variant = PERM_VARIANT[k][variant]` then indexes
-    /// VARIANTS[css_variant] (0..32).
-    /// Mutation: set PERM_VARIANT[0][1] = PERM_VARIANT[0][0] -> duplicate
-    /// assert fires; also any value >= 32 would later index VARIANTS OOB.
+    // Each PERM_VARIANT row must map the 32 variants to 32 distinct 5-bit
+    // values; a collision would make two variants indistinguishable.
+    // See docs/css-mod.md#perm-variant-rows.
     #[test]
     fn perm_variant_rows_are_permutations_of_0_31() {
         for (row, perm) in PERM_VARIANT.iter().enumerate() {
@@ -849,16 +797,8 @@ mod tests {
 
     // ── crypt_key behaviour ────────────────────────────────────────────────
 
-    /// crypt_key result depends on every challenge byte. The challenge is
-    /// permuted into `scratch` and folded through the LFSR seeding and the 6
-    /// XOR rounds. Flipping any single challenge byte must change the output.
-    ///
-    /// Grounding: scratch[i]=challenge[perm[i]] for all 10 i, and scratch
-    /// seeds both LFSRs (bytes 5..10 via tmp1) and the round terms (bytes
-    /// 0..5).
-    /// Mutation: in `scratch[i] = challenge[perm[i]]` replace with
-    /// `challenge[i]` for a perm that drops a byte — or hardcode one scratch
-    /// entry — and some challenge byte stops mattering; this fails.
+    // crypt_key's result must depend on every challenge byte: flipping any
+    // single byte must change the output. See docs/css-mod.md#crypt-key-byte-dependence.
     #[test]
     fn crypt_key_depends_on_every_challenge_byte() {
         let base: [u8; 10] = [0, 1, 2, 3, 4, 5, 6, 7, 8, 9];
@@ -874,17 +814,9 @@ mod tests {
         }
     }
 
-    /// crypt_key(0, v, ..) must produce a DISTINCT result for each of the 32
-    /// variants on a fixed challenge. bus_auth brute-forces the variant by
-    /// matching crypt_key(0, v, host_challenge) == key1; if two variants
-    /// collided, the wrong variant could be selected and the whole auth
-    /// derail.
-    ///
-    /// Grounding: variant selects css_variant -> VARIANTS[css_variant] -> cse,
-    /// which feeds every round; distinct variants give distinct cse-driven
-    /// keys in practice.
-    /// Mutation: make `cse` ignore the variant (e.g. `let cse = 0`) -> all 32
-    /// outputs collapse to one value; the distinctness assert fires.
+    // crypt_key(0, v, ..) must be distinct for each of the 32 variants:
+    // bus-auth brute-forces the variant by matching against key1, so a
+    // collision could select the wrong one. See docs/css-mod.md.
     #[test]
     fn crypt_key_type0_distinct_per_variant() {
         let challenge: [u8; 10] = [0, 1, 2, 3, 4, 5, 6, 7, 8, 9];
@@ -899,16 +831,9 @@ mod tests {
         }
     }
 
-    /// crypt_key enforces its documented precondition `key_type < 3` via
-    /// debug_assert (active in test builds). A key_type of 3 would index
-    /// PERM_CHALLENGE (len 3) out of bounds; the assert turns that into an
-    /// explicit precondition panic.
-    ///
-    /// Grounding: `debug_assert!(key_type < 3, ...)`; PERM_CHALLENGE has 3
-    /// rows (indices 0,1,2).
-    /// Mutation: delete the debug_assert AND the match-arm guard — but the
-    /// match `_ =>` arm would then index PERM_CHALLENGE[3] OOB and panic
-    /// differently; with the assert in place this test pins the contract.
+    // crypt_key's `key_type < 3` debug_assert must fire for key_type 3
+    // (which would otherwise index PERM_CHALLENGE out of bounds).
+    // See docs/css-mod.md#crypt-key-preconditions.
     #[test]
     #[should_panic]
     fn crypt_key_rejects_out_of_range_key_type() {
@@ -916,12 +841,9 @@ mod tests {
         let _ = crypt_key(3, 0, &challenge);
     }
 
-    /// crypt_key enforces `variant < 32` via debug_assert. A variant of 32
-    /// would index VARIANTS / PERM_VARIANT (len 32) out of bounds.
-    ///
-    /// Grounding: `debug_assert!((variant as usize) < 32, ...)`.
-    /// Mutation: removing the assert makes this index VARIANTS[32] (still a
-    /// panic, but unguarded); the assert documents/enforces the contract.
+    // crypt_key's `variant < 32` debug_assert must fire for variant 32
+    // (which would otherwise index VARIANTS/PERM_VARIANT out of bounds).
+    // See docs/css-mod.md#crypt-key-preconditions.
     #[test]
     #[should_panic]
     fn crypt_key_rejects_out_of_range_variant() {
@@ -931,17 +853,8 @@ mod tests {
 
     // ── SCSI CDB builders (MMC REPORT KEY / SEND KEY layout) ───────────────
 
-    /// report_key_cdb encodes a 12-byte MMC REPORT KEY (opcode 0xA4) CDB:
-    ///   byte 0  = operation code 0xA4
-    ///   bytes 8-9 = allocation length, big-endian
-    ///   byte 10 = (AGID << 6) | (key_format & 0x3F)
-    /// All other bytes are zero.
-    ///
-    /// Grounding: MMC REPORT KEY CDB; the AGID is the top 2 bits of byte 10,
-    /// key format the low 6 bits.
-    /// Mutation: change `(alloc_len >> 8)` to `alloc_len` for byte 8 (lose the
-    /// big-endian split) -> byte 8/9 assert fails. Change `agid << 6` to
-    /// `agid << 5` -> the AGID-position assert fails.
+    // report_key_cdb encodes a 12-byte MMC REPORT KEY (opcode 0xA4) CDB:
+    // byte 0=0xA4, bytes 8-9=big-endian len, byte 10=(AGID<<6)|(format&0x3F).
     #[test]
     fn report_key_cdb_matches_mmc_layout() {
         let cdb = report_key_cdb(0b10, 0x04, 0x010C); // AGID=2, format=0x04, len=268
@@ -962,26 +875,18 @@ mod tests {
         assert_eq!(cdb.len(), 12, "REPORT KEY is a 12-byte CDB");
     }
 
-    /// The key format field is masked to 6 bits: a format with high bits set
-    /// must not corrupt the AGID. report_key_cdb(0, 0xFF, _) -> byte 10 low 6
-    /// bits = 0x3F, AGID = 0.
-    ///
-    /// Grounding: `(agid << 6) | (format & 0x3F)`.
-    /// Mutation: drop the `& 0x3F` mask -> 0xFF would overwrite the AGID bits;
-    /// byte 10 would be 0xFF not 0x3F, this fails.
+    // The key-format field is masked to 6 bits so a format with high bits
+    // set (e.g. 0xFF) cannot corrupt the AGID bits of byte 10.
+    // See docs/css-mod.md#cdb-builders.
     #[test]
     fn report_key_cdb_masks_format_to_6_bits() {
         let cdb = report_key_cdb(0, 0xFF, 8);
         assert_eq!(cdb[10], 0x3F, "format masked to 6 bits, AGID stays 0");
     }
 
-    /// send_key_cdb encodes a 12-byte MMC SEND KEY (opcode 0xA3) CDB with the
-    /// parameter-list length at bytes 8-9 (big-endian) and AGID/format at byte
-    /// 10.
-    ///
-    /// Grounding: MMC SEND KEY CDB layout.
-    /// Mutation: change opcode to SCSI_REPORT_KEY -> opcode assert fails;
-    /// swap bytes 8/9 -> length assert fails.
+    // send_key_cdb encodes a 12-byte MMC SEND KEY (opcode 0xA3) CDB with the
+    // parameter-list length at bytes 8-9 (big-endian) and AGID/format at
+    // byte 10. See docs/css-mod.md#cdb-builders.
     #[test]
     fn send_key_cdb_matches_mmc_layout() {
         let cdb = send_key_cdb(0b11, 0x03, 0x000C); // AGID=3, format=3, param_len=12
@@ -996,16 +901,9 @@ mod tests {
         assert_eq!(cdb.len(), 12);
     }
 
-    /// Allocation length larger than 255 must split across bytes 8 (high) and
-    /// 9 (low) — a 16-bit big-endian field. report_key_cdb with alloc_len
-    /// 0x0804 (2052, the disc-key block size used in read_disc_key) -> byte 8
-    /// = 0x08, byte 9 = 0x04.
-    ///
-    /// Grounding: read_disc_key uses `alloc_len = 2048 + 4 = 2052 = 0x0804`
-    /// and writes `cdb[8] = (alloc_len >> 8); cdb[9] = alloc_len`.
-    /// Mutation: write only byte 9 (`cdb[9] = alloc_len as u8`) without byte 8
-    /// -> the drive sees a 4-byte transfer, truncating the disc-key block;
-    /// this asserts the high byte is present.
+    // Allocation length > 255 must split across bytes 8 (high) and 9 (low)
+    // as a 16-bit big-endian field, e.g. 0x0804 (the disc-key block size).
+    // See docs/css-mod.md#cdb-builders.
     #[test]
     fn report_key_cdb_alloc_len_is_16bit_big_endian() {
         let cdb = report_key_cdb(0, 0x00, 0x0804);
@@ -1013,10 +911,9 @@ mod tests {
         assert_eq!(cdb[9], 0x04, "low byte of 2052-byte transfer");
     }
 
-    /// The unlocker's user-facing name is "DVD" (the medium it read-unlocks),
-    /// not "CSS" (the scheme). Apps render the unlocker report from this name,
-    /// so it is a stable contract — the bus-auth ran on any DVD, encrypted or
-    /// not, and the report must say so rather than conflate it with a CSS crack.
+    // The unlocker's user-facing name is "DVD" (the medium), not "CSS" (the
+    // scheme); apps render the unlocker report from this name, so it is a
+    // stable contract.
     #[test]
     fn dvd_unlocker_is_named_dvd() {
         use crate::Unlocker;
@@ -1033,37 +930,13 @@ mod tests {
     }
 
     /// DvdUnlocker provides bus removal only — it never provides drive features.
-    #[test]
-    fn dvd_unlocker_provides_no_features() {
-        use crate::scsi::{DataDirection, ScsiResult};
-        use crate::{DiscKind, DriveId, UnlockCtx, UnlockError, Unlocker};
-        struct DeadTransport;
-        impl ScsiTransport for DeadTransport {
-            fn execute(
-                &mut self,
-                _cdb: &[u8],
-                _dir: DataDirection,
-                _data: &mut [u8],
-                _timeout_ms: u32,
-            ) -> crate::scsi::Result<ScsiResult> {
-                panic!("unlock_features must not touch the transport");
-            }
-        }
-        let id = DriveId::default();
-        let mut t = DeadTransport;
-        let r =
-            DvdUnlocker::new().unlock_features(&mut t, &UnlockCtx::new(&id, DiscKind::Css, &[]));
-        assert_eq!(r.unwrap_err(), UnlockError::NotApplicable);
-    }
-
-    /// Defense in depth: even when the caller declares `DiscKind::Css`, the
-    /// DvdUnlocker self-verifies against the drive's GET CONFIGURATION profile.
-    /// A drive reporting a Blu-ray profile → `NotApplicable`, and NOT a single
-    /// CSS CDB is issued (no bus-auth fired at a BD).
+    // Defense in depth: even when the caller declares `DiscKind::Css`,
+    // DvdUnlocker self-verifies against GET CONFIGURATION; a BD profile
+    // must yield NotApplicable with no CSS CDB issued.
     #[test]
     fn dvd_unlocker_self_guards_against_non_dvd() {
         use crate::scsi::{DataDirection, ScsiResult};
-        use crate::{DiscKind, DriveId, UnlockCtx, UnlockError, Unlocker};
+        use crate::{DiscKind, DriveId, UnlockCtx, Unlocker};
 
         /// Reports a BD-ROM profile (0x0040) to GET CONFIGURATION and counts any
         /// other CDB (i.e. CSS bus-auth activity).
@@ -1104,10 +977,9 @@ mod tests {
         };
 
         let mut t = BdTransport { non_config_cdbs: 0 };
-        let r = DvdUnlocker::new().unlock_bus(&mut t, &UnlockCtx::new(&id, DiscKind::Css, &[]));
-        assert_eq!(
-            r.unwrap_err(),
-            UnlockError::NotApplicable,
+        let r = DvdUnlocker::new().unlock(&mut t, &UnlockCtx::new(&id, DiscKind::Css));
+        assert!(
+            r.expect("BD profile declines, not a hard error").is_none(),
             "a BD-profile drive must be refused"
         );
         assert_eq!(
@@ -1120,16 +992,14 @@ mod tests {
     use crate::scsi::mock::{MockTransport, Reply};
     use crate::{DiscKind, DriveId, UnlockCtx, UnlockError, Unlocker};
 
-    /// THE defect-7 test. `mounted_disc_is_dvd` is the FIRST command the DVD
-    /// unlocker issues and its match had no `Err` arm: a transport fault fell
-    /// into `_ => false`, so a dead bus was reported as "not a DVD"
-    /// (`NotApplicable`) and the consumer kept probing it — with nothing logged
-    /// anywhere on the path. Catches restoring that catch-all arm.
+    // Defect-7 regression: a transport fault on the first probe command
+    // must abort as Transport, not fall through to NotApplicable (which
+    // let the consumer keep probing a dead bus). See docs/css-mod.md.
     #[test]
     fn transport_fault_probing_for_a_dvd_aborts() {
         let id = DriveId::default();
         let mut t = MockTransport::always(Reply::TransportFault);
-        let r = DvdUnlocker::new().unlock_bus(&mut t, &UnlockCtx::new(&id, DiscKind::Css, &[]));
+        let r = DvdUnlocker::new().unlock(&mut t, &UnlockCtx::new(&id, DiscKind::Css));
         assert_eq!(r.unwrap_err(), UnlockError::Transport);
         assert_eq!(
             t.calls(),
@@ -1145,15 +1015,13 @@ mod tests {
     fn check_condition_probing_for_a_dvd_declines() {
         let id = DriveId::default();
         let mut t = MockTransport::always(Reply::illegal_request());
-        let r = DvdUnlocker::new().unlock_bus(&mut t, &UnlockCtx::new(&id, DiscKind::Css, &[]));
-        assert_eq!(r.unwrap_err(), UnlockError::NotApplicable);
+        let r = DvdUnlocker::new().unlock(&mut t, &UnlockCtx::new(&id, DiscKind::Css));
+        assert!(r.expect("inconclusive probe declines").is_none());
     }
 
-    /// A DVD is mounted, then the bus dies during bus-auth. THE defect-2 test:
-    /// every CSS step used to do `.map_err(|_| CssAuthFailed)`, discarding the
-    /// `ScsiError` and making the `Transport` arm of the error conversion
-    /// unreachable from any real call site — so a dead bus arrived as
-    /// `NotApplicable` and the consumer went on probing it.
+    // Defect-2 regression: a DVD is mounted, then the bus dies mid
+    // bus-auth — must abort as Transport, not collapse to CssAuthFailed /
+    // NotApplicable. See docs/css-mod.md.
     #[test]
     fn transport_fault_during_bus_auth_aborts() {
         let id = DriveId::default();
@@ -1162,7 +1030,7 @@ mod tests {
         config[6] = 0x00;
         config[7] = 0x10;
         let mut t = MockTransport::scripted(vec![Reply::good(config)], Reply::TransportFault);
-        let r = DvdUnlocker::new().unlock_bus(&mut t, &UnlockCtx::new(&id, DiscKind::Css, &[]));
+        let r = DvdUnlocker::new().unlock(&mut t, &UnlockCtx::new(&id, DiscKind::Css));
         assert_eq!(r.unwrap_err(), UnlockError::Transport);
     }
 
@@ -1174,14 +1042,13 @@ mod tests {
         let mut config = vec![0u8; 8];
         config[7] = 0x10;
         let mut t = MockTransport::scripted(vec![Reply::good(config)], Reply::illegal_request());
-        let r = DvdUnlocker::new().unlock_bus(&mut t, &UnlockCtx::new(&id, DiscKind::Css, &[]));
-        assert_eq!(r.unwrap_err(), UnlockError::NotApplicable);
+        let r = DvdUnlocker::new().unlock(&mut t, &UnlockCtx::new(&id, DiscKind::Css));
+        assert!(r.expect("a refused bus-auth declines").is_none());
     }
 
-    /// A conforming transport answering the AGID allocation with a CHECK
-    /// CONDITION must NOT let the handshake carry on off the caller's own
-    /// zero-filled buffer (agid 0, a challenge answered by zeros). Catches
-    /// dropping the `status` check in `css_scsi`.
+    // A CHECK CONDITION on AGID allocation must not let the handshake carry
+    // on off the caller's own zero-filled buffer; catches a dropped
+    // `status` check in `css_scsi`.
     #[test]
     fn agid_allocation_check_condition_fails_the_auth() {
         // 4 AGID invalidations are best-effort; the 5th command is the alloc.
@@ -1198,10 +1065,9 @@ mod tests {
         assert!(matches!(e, Error::CssAuthFailed));
     }
 
-    /// Defect 18: an AGID allocated and then lost to a failed challenge must be
-    /// RELEASED (REPORT KEY format 0x3F), not abandoned. A drive has four; the
-    /// leak self-heals because the next session invalidates all of them, but
-    /// leaving a drive short between attempts is a state we should not create.
+    // Defect 18: an AGID lost to a failed challenge must be RELEASED
+    // (REPORT KEY format 0x3F), not abandoned, even though a drive's four
+    // AGIDs self-heal on the next session's invalidation pass.
     #[test]
     fn a_failed_handshake_releases_the_agid_it_allocated() {
         let mut t = MockTransport::scripted(
@@ -1281,10 +1147,9 @@ mod tests {
         );
     }
 
-    /// Host challenge + Key1 both succeed, but the drive challenge REPORT KEY
-    /// (step 3) is refused. Distinct from `key1_report_failure_fails_the_auth`
-    /// (step 2) and `key2_send_failure_fails_the_auth` (step 4) below — each
-    /// pins a different `css_scsi` call site's `?`.
+    // Host challenge + Key1 succeed but the drive-challenge REPORT KEY
+    // (step 3) is refused; distinct from the step-2 and step-4 failure
+    // tests, each pinning a different `css_scsi` call site's `?`.
     #[test]
     fn drive_challenge_report_failure_fails_the_auth() {
         struct FailAtDriveChallenge(FakeDvdDrive);
@@ -1368,13 +1233,8 @@ mod tests {
 
     // ── Full happy-path bus-auth ────────────────────────────────────────────
 
-    /// A fake drive that plays its half of the CSS bus-auth handshake for
-    /// real: it captures the host's genuinely-random per-session challenge
-    /// and answers Key1 with the actual `crypt_key(0, variant, ..)` for a
-    /// fixed `variant`, so the host's brute-force loop matches it honestly
-    /// (not via a canned constant the loop happens to hit). Also answers
-    /// GET CONFIGURATION as a DVD and READ DVD STRUCTURE (disc key) as a
-    /// success, so the full `unlock_bus` path can be driven end to end.
+    // A fake drive that plays its half of the CSS handshake for real:
+    // answers Key1 honestly, and DVD/disc-key probes as success.
     struct FakeDvdDrive {
         variant: u8,
         host_challenge: [u8; 10],
@@ -1445,12 +1305,9 @@ mod tests {
         }
     }
 
-    /// End-to-end happy path for the bus-auth handshake: AGID allocation,
-    /// host challenge, a Key1 that genuinely matches the brute-forced
-    /// variant, the drive-challenge/Key2 exchange, and the disc-key REPORT
-    /// KEY all succeed. Exercises `establish_authenticated_session`'s `Ok`
-    /// return and `read_disc_key`'s success path, neither of which the
-    /// failure-path tests above reach.
+    // End-to-end happy path for the bus-auth handshake (AGID, challenge,
+    // Key1/Key2, disc-key REPORT KEY), exercising the `Ok` returns that
+    // the failure-path tests above never reach.
     #[test]
     fn full_bus_auth_and_disc_key_succeed() {
         let mut t = FakeDvdDrive {
@@ -1463,10 +1320,9 @@ mod tests {
         read_disc_key(&mut t, agid).expect("disc-key REPORT KEY succeeds");
     }
 
-    /// The same happy path driven through the public `unlock_bus` entry
-    /// point (DVD self-guard, full bus-auth, best-effort disc key), proving
-    /// `DvdUnlocker::unlock_bus` returns `Ok(Unlocked::default())` on success
-    /// — the path every failure-injection test above deliberately avoids.
+    // The same happy path through the public `unlock` entry point, proving it
+    // returns `Some(Unlocked::default())` on success — the path every
+    // failure-injection test above deliberately avoids.
     #[test]
     fn dvd_unlocker_succeeds_end_to_end() {
         let id = DriveId::default();
@@ -1474,14 +1330,12 @@ mod tests {
             variant: 3,
             host_challenge: [0u8; 10],
         };
-        let r = DvdUnlocker::new().unlock_bus(&mut t, &UnlockCtx::new(&id, DiscKind::Css, &[]));
-        let unlocked = r.expect("full unlock succeeds");
+        let r = DvdUnlocker::new().unlock(&mut t, &UnlockCtx::new(&id, DiscKind::Css));
+        let unlocked = r
+            .expect("full unlock succeeds")
+            .expect("DVD bus-auth unlocks the drive");
         assert!(unlocked.vid.is_none(), "CSS yields no Volume ID");
         assert!(unlocked.bus_key.is_none(), "CSS yields no AACS bus key");
-        assert!(
-            !unlocked.drive_unlocked,
-            "CSS is a bus-auth unlock, not a firmware drive-unlock"
-        );
     }
 
     /// `unlock_css_reads` (the crate-level public entry point, distinct from
@@ -1497,10 +1351,9 @@ mod tests {
         unlock_css_reads(&mut t, 0).expect("css bus-auth + best-effort disc key succeed");
     }
 
-    /// A drive whose bus-auth succeeds but refuses the best-effort disc-key
-    /// REPORT KEY (CHECK CONDITION). `unlock_css_reads_inner` must swallow
-    /// that failure — the read barrier is already open from bus-auth, so the
-    /// whole unlock must still report `Ok`, not propagate the disc-key error.
+    // A drive whose bus-auth succeeds but refuses the best-effort disc-key
+    // REPORT KEY; `unlock_css_reads_inner` must swallow that failure since
+    // the read barrier is already open from bus-auth.
     struct DiscKeyRefusingDrive(FakeDvdDrive);
 
     impl ScsiTransport for DiscKeyRefusingDrive {
@@ -1532,10 +1385,9 @@ mod tests {
             .expect("bus-auth succeeded; a refused best-effort disc-key read must not fail it");
     }
 
-    /// `crypt_key`'s `key_type == 2` arm (`PERM_VARIANT[1]`) has no
-    /// production caller (only key_type 0, the variant brute-force, and 1,
-    /// the drive-challenge Key2, are ever invoked from `authenticate_with_agid`)
-    /// but is a pure, directly-testable function of its documented contract.
+    // `crypt_key`'s key_type==2 arm (`PERM_VARIANT[1]`) has no production
+    // caller (only 0 and 1 are invoked from `authenticate_with_agid`) but
+    // is a pure, directly-testable function.
     #[test]
     fn crypt_key_type2_is_deterministic_and_distinct_from_type1() {
         let challenge: [u8; 10] = [0, 1, 2, 3, 4, 5, 6, 7, 8, 9];

src/freemkv/mod.rs

@@ -0,0 +1,784 @@
+//! freemkv — the self-identifying custom-firmware unlocker.
+//!
+//! Detects a freemkv-firmware drive by issuing the vendor Identity command
+//! (sub-function `0x01` of the `0x3C` READ BUFFER "knock" ABI) and checking
+//! that the response starts `b"freemkv"` — no bundled profile database is
+//! needed, unlike [`crate::ld`], because the firmware self-identifies.
+//!
+//! The full knock CDB layout, sub-function table, fixed-length reply rules,
+//! and toggle-polarity details are documented in `docs/freemkv-abi.md`.
+
+use crate::scsi::{DataDirection, ScsiTransport};
+use crate::{UnlockCtx, UnlockError, Unlocked, Unlocker};
+
+/// READ BUFFER (0x3C) opcode every freemkv vendor command hijacks.
+const KNOCK_OPCODE: u8 = 0x3C;
+/// Knock-mode byte (CDB byte 1) that distinguishes this ABI from an ordinary
+/// READ BUFFER mode.
+const KNOCK_MODE: u8 = 0x0E;
+/// The `C0 DE` knock (CDB bytes 2-3) that marks a freemkv vendor command.
+const KNOCK_MAGIC: [u8; 2] = [0xC0, 0xDE];
+
+/// State byte that selects OEM behaviour (clears the feature's RAM flag). The
+/// OFF pole of the Region toggle.
+const STATE_OFF: u8 = 0x00;
+/// State byte that enables the patched behaviour (sets the feature's RAM flag).
+/// The ON pole of the Region toggle.
+const STATE_ON: u8 = 0x01;
+
+/// Speed sub-function (`0x02`) cap value selecting OEM read-speed behaviour.
+#[allow(dead_code)]
+const SPEED_CAP_OEM: u8 = 0x00;
+/// Speed sub-function (`0x02`) cap value selecting the uncapped / maximum read
+/// speed (full riplock lift). `0x01`..=`0xFE` select an intermediate ceiling.
+const SPEED_CAP_MAX: u8 = 0xFF;
+
+/// Raw Read (`0x04`) mode: OEM cert enforcement.
+#[allow(dead_code)]
+const RAW_READ_OFF: u8 = 0x00;
+/// Raw Read (`0x04`) mode: "cert is valid" — the drive reports host auth as
+/// already succeeded, so a bare `0xAD` fmt `0x80` yields the VID with NO cert
+/// and NO AKE. This is the mode this unlocker sets.
+const RAW_READ_CERT_VALID: u8 = 0x01;
+/// Raw Read (`0x04`) mode: "accept any host cert, revoked or not" — the host
+/// still runs the real AKE. Driven by the AACS cert unlocker, not here.
+#[allow(dead_code)]
+const RAW_READ_ACCEPT_ANY: u8 = 0x02;
+/// Raw Read (`0x04`) mode: "data clear" — remove AACS in-transit BUS encryption
+/// so content `READ(10)` returns the AACS-at-rest bytes unwrapped (the host then
+/// applies title keys). Unlike `01`/`02` (VID paths, followed by a bare `0xAD`),
+/// `03` is followed by ordinary `READ(10)`s. Issued as a trailing best-effort
+/// step of [`FreemkvUnlocker::full_unlock`] (after the VID read, since it
+/// repurposes `flag[0x04]`) so an unlocked drive ends up in bus-off for its
+/// content reads. Inert on firmware without the `04 03` lever.
+const RAW_READ_DATA_CLEAR: u8 = 0x03;
+
+/// Vendor sub-functions (CDB byte 4). These numeric values ARE the wire
+/// protocol and match `freemkv-firmware`'s `abi.rs::SubFn`.
+mod subfn {
+    /// Identity — detection (read; ignores state).
+    pub(super) const IDENTITY: u8 = 0x01;
+    /// Speed / riplock (state byte IS the cap value: `00` = OEM, `01`..=`FF`
+    /// ceiling, `FF` = max).
+    pub(super) const SPEED: u8 = 0x02;
+    /// Region-free (toggle; `01` = RPC region-free).
+    pub(super) const REGION: u8 = 0x03;
+    /// Raw Read (transport unlock; state selects the cert-gate mode).
+    pub(super) const RAW_READ: u8 = 0x04;
+    /// DumpAll diagnostic RAM read (address big-endian in `cdb[5..9]`).
+    pub(super) const DUMP_ALL: u8 = 0x09;
+}
+
+/// The ASCII magic that leads the Identity (subfn 01) reply — the `RESP_MAGIC`
+/// of the canonical `abi.rs`. This is the ENTIRE freemkv-detection mechanism
+/// (no bundled profile database — the firmware self-identifies).
+const IDENTITY_MARKER: &[u8] = b"freemkv";
+
+// Fixed response length for EVERY freemkv knock command; the drive commits this
+// many bytes on data-in regardless of subfn. A short/zero allocation desyncs the
+// transfer (ABORTED COMMAND, then a wedged FIFO) — see docs/freemkv-abi.md.
+const KNOCK_RESP_LEN: usize = 64;
+
+/// Bytes returned by one DumpAll (subfn 0x09) diagnostic read — the firmware
+/// always commits a fixed 64-byte window.
+const MEMREAD_LEN: usize = 64;
+
+/// Build the 10-byte knock CDB for a freemkv vendor sub-function:
+/// `3C 0E C0 DE <subfn> <state> <len_hi> <len_mid> <len_lo> 00`.
+fn build_cdb(subfn: u8, state: u8, alloc_len: u32) -> [u8; 10] {
+    let len = alloc_len.to_be_bytes();
+    [
+        KNOCK_OPCODE,
+        KNOCK_MODE,
+        KNOCK_MAGIC[0],
+        KNOCK_MAGIC[1],
+        subfn,
+        state,
+        len[1],
+        len[2],
+        len[3],
+        0x00,
+    ]
+}
+
+// Build the DumpAll (subfn 0x09) CDB: addr packed big-endian into cdb[5..9] (the
+// allocation-length field is reused to carry the address for the fixed 64-byte
+// window, matching freemkv-firmware's abi.rs::build_memread_cdb) — bypasses build_cdb.
+fn build_memread_cdb(addr: u32) -> [u8; 10] {
+    let a = addr.to_be_bytes();
+    [
+        KNOCK_OPCODE,
+        KNOCK_MODE,
+        KNOCK_MAGIC[0],
+        KNOCK_MAGIC[1],
+        subfn::DUMP_ALL,
+        a[0],
+        a[1],
+        a[2],
+        a[3],
+        0x00,
+    ]
+}
+
+/// Whether a transport error is a genuine dead bus (a senseless
+/// transport-failure status) rather than a drive rejection surfaced through a
+/// non-conforming transport (`Err` carrying a sense).
+fn is_dead_bus(e: &crate::scsi::ScsiError) -> bool {
+    e.status == crate::scsi::SCSI_STATUS_TRANSPORT_FAILURE && e.sense.is_none()
+}
+
+// The freemkv custom-firmware unlocker. Detection: subfn-01 Identity, checking
+// the response starts "freemkv" — no bundled profile catalog needed, unlike
+// crate::ld::LdUnlocker. Stateless: `unlock()` returns what it learned.
+#[derive(Default)]
+pub struct FreemkvUnlocker;
+
+impl FreemkvUnlocker {
+    pub fn new() -> Self {
+        FreemkvUnlocker
+    }
+
+    // Issue the subfn-01 Identity command: Ok(true) if response starts "freemkv",
+    // Ok(false) if rejected/mismatched (not this firmware). A dead bus is
+    // Err(Transport) — this is the FIRST command, so a transport fault must abort.
+    fn identify(&self, scsi: &mut dyn ScsiTransport) -> std::result::Result<bool, UnlockError> {
+        // Identity MUST request the fixed KNOCK_RESP_LEN (64) allocation like
+        // every other knock: the firmware commits 64 bytes on every subfn, so a
+        // short 32-byte read desyncs the data-in phase and wedges the VID read.
+        let cdb = build_cdb(subfn::IDENTITY, STATE_OFF, KNOCK_RESP_LEN as u32);
+        let mut buf = [0u8; KNOCK_RESP_LEN];
+        match scsi.execute(&cdb, DataDirection::FromDevice, &mut buf, 5_000) {
+            Ok(r) => {
+                let matched = r.status == 0
+                    && r.bytes_transferred >= IDENTITY_MARKER.len()
+                    && buf[..IDENTITY_MARKER.len()] == *IDENTITY_MARKER;
+                if !matched {
+                    tracing::debug!(
+                        target: "freemkv::disc",
+                        phase = "freemkv_identity_no_match",
+                        "Identity probe did not report freemkv firmware"
+                    );
+                }
+                Ok(matched)
+            }
+            Err(e) => {
+                if is_dead_bus(&e) {
+                    tracing::warn!(
+                        target: "freemkv::disc",
+                        phase = "freemkv_identity_transport_fault",
+                        "transport fault on the freemkv Identity probe; aborting"
+                    );
+                    return Err(UnlockError::Transport);
+                }
+                tracing::debug!(
+                    target: "freemkv::disc",
+                    phase = "freemkv_identity_rejected",
+                    status = e.status,
+                    "freemkv Identity probe rejected by the drive; not this firmware"
+                );
+                Ok(false)
+            }
+        }
+    }
+
+    // Issue a payload-less sub-function carrying an explicit state byte (toggle
+    // pole, Speed cap, or Raw Read mode). Ok(()) on GOOD status; NotApplicable
+    // if rejected; Transport only on a dead bus.
+    fn send_state(
+        &self,
+        scsi: &mut dyn ScsiTransport,
+        subfn: u8,
+        state: u8,
+    ) -> std::result::Result<(), UnlockError> {
+        // Every knock command returns a fixed KNOCK_RESP_LEN data-in payload; a
+        // toggle reads and DISCARDS it (only the GOOD status matters), but it MUST
+        // set up the data-in phase or the drive aborts + wedges (see KNOCK_RESP_LEN).
+        let cdb = build_cdb(subfn, state, KNOCK_RESP_LEN as u32);
+        let mut buf = [0u8; KNOCK_RESP_LEN];
+        match scsi.execute(&cdb, DataDirection::FromDevice, &mut buf, 5_000) {
+            Ok(r) if r.status == 0 => Ok(()),
+            Ok(r) => {
+                tracing::debug!(
+                    target: "freemkv::disc",
+                    phase = "freemkv_subfn_rejected",
+                    subfn,
+                    state,
+                    status = r.status,
+                    "freemkv sub-function rejected by the drive"
+                );
+                Err(UnlockError::NotApplicable)
+            }
+            Err(e) => {
+                if is_dead_bus(&e) {
+                    tracing::warn!(
+                        target: "freemkv::disc",
+                        phase = "freemkv_subfn_transport_fault",
+                        subfn,
+                        "transport fault on a freemkv sub-function; aborting"
+                    );
+                    return Err(UnlockError::Transport);
+                }
+                tracing::debug!(
+                    target: "freemkv::disc",
+                    phase = "freemkv_subfn_rejected_as_err",
+                    subfn,
+                    status = e.status,
+                    "freemkv sub-function rejected (via Err)"
+                );
+                Err(UnlockError::NotApplicable)
+            }
+        }
+    }
+
+    /// Speed / riplock (subfn 0x02). The state byte IS the cap: `0x00` restores
+    /// OEM behaviour, `0xFF` lifts riplock to full speed, `0x01`..=`0xFE` select
+    /// an intermediate ceiling.
+    fn set_speed(
+        &self,
+        scsi: &mut dyn ScsiTransport,
+        cap: u8,
+    ) -> std::result::Result<(), UnlockError> {
+        self.send_state(scsi, subfn::SPEED, cap)
+    }
+
+    /// Region toggle (subfn 0x03). `on` = DVD RPC region-free.
+    fn set_region_free(
+        &self,
+        scsi: &mut dyn ScsiTransport,
+        on: bool,
+    ) -> std::result::Result<(), UnlockError> {
+        let state = if on { STATE_ON } else { STATE_OFF };
+        self.send_state(scsi, subfn::REGION, state)
+    }
+
+    // Raw Read (subfn 0x04) in RAW_READ_CERT_VALID mode: tells the drive host auth
+    // already succeeded so a bare 0xAD fmt 0x80 returns the VID with no cert/AKE.
+    // Load-bearing: if rejected, this firmware can't do the one-command unlock.
+    fn set_raw_read(&self, scsi: &mut dyn ScsiTransport) -> std::result::Result<(), UnlockError> {
+        self.send_state(scsi, subfn::RAW_READ, RAW_READ_CERT_VALID)
+    }
+
+    // Raw Read (subfn 0x04) mode 0x03 "data clear": remove AACS in-transit bus
+    // encryption so content READ(10) returns at-rest bytes unwrapped. Best-effort
+    // trailing step of full_unlock; inert without the 04 03 firmware lever.
+    fn set_bus_off(&self, scsi: &mut dyn ScsiTransport) -> std::result::Result<(), UnlockError> {
+        self.send_state(scsi, subfn::RAW_READ, RAW_READ_DATA_CLEAR)
+    }
+
+    /// DumpAll diagnostic RAM read (subfn 0x09): return the 64-byte window at
+    /// `addr`. A host-side diagnostic path only — not used by the unlock flow.
+    #[allow(dead_code)]
+    fn dump_ram(
+        &self,
+        scsi: &mut dyn ScsiTransport,
+        addr: u32,
+    ) -> std::result::Result<[u8; MEMREAD_LEN], UnlockError> {
+        let cdb = build_memread_cdb(addr);
+        let mut buf = [0u8; MEMREAD_LEN];
+        match scsi.execute(&cdb, DataDirection::FromDevice, &mut buf, 5_000) {
+            Ok(r) if r.status == 0 && r.bytes_transferred >= MEMREAD_LEN => Ok(buf),
+            Ok(_) => Err(UnlockError::NotApplicable),
+            Err(e) => {
+                if is_dead_bus(&e) {
+                    Err(UnlockError::Transport)
+                } else {
+                    Err(UnlockError::NotApplicable)
+                }
+            }
+        }
+    }
+
+    // Full freemkv unlock: 01 Identity (hard gate) → 03 Region → 02 Speed (best-
+    // effort) → 04 01 Raw Read → bare 0xAD VID (load-bearing) → 04 03 bus-off
+    // (trailing, best-effort). See docs/freemkv-abi.md for failure-mode semantics.
+    fn full_unlock(
+        &self,
+        scsi: &mut dyn ScsiTransport,
+    ) -> std::result::Result<Unlocked, UnlockError> {
+        // 01 — Identity: must be a freemkv drive.
+        if !self.identify(scsi)? {
+            return Err(UnlockError::NotApplicable);
+        }
+        // Best-effort feature toggle: only a dead bus aborts.
+        let best_effort = |r: std::result::Result<(), UnlockError>,
+                           what: &'static str|
+         -> std::result::Result<(), UnlockError> {
+            match r {
+                Ok(()) => Ok(()),
+                Err(UnlockError::Transport) => Err(UnlockError::Transport),
+                Err(_) => {
+                    tracing::debug!(
+                        target: "freemkv::disc",
+                        phase = "freemkv_feature_unavailable",
+                        feature = what,
+                        "feature unavailable; continuing"
+                    );
+                    Ok(())
+                }
+            }
+        };
+        // 03 — Region-free (feature).
+        best_effort(self.set_region_free(scsi, true), "region")?;
+        // 02 — Speed / riplock lift to full (feature).
+        best_effort(self.set_speed(scsi, SPEED_CAP_MAX), "speed")?;
+        // 04 01 — Raw Read "cert valid" (LOAD-BEARING). No fallback: a firmware
+        // that rejects it can't do the one-command unlock.
+        match self.set_raw_read(scsi) {
+            Ok(()) => {}
+            Err(UnlockError::Transport) => return Err(UnlockError::Transport),
+            Err(_) => {
+                tracing::debug!(
+                    target: "freemkv::disc",
+                    phase = "freemkv_raw_read_rejected",
+                    "Raw Read (04 01) rejected — cannot unlock this drive"
+                );
+                return Err(UnlockError::VidUnavailable);
+            }
+        }
+        // Bare 0xAD VID read — the shared BEST-EFFORT reader (identical to the
+        // LD/Renesas routes). Raw Read already unlocked the drive, so a VID miss
+        // must not discard it: only a dead bus propagates (`?`), else `None`.
+        let vid = crate::vid::read_aacs_vid(scsi)?;
+        // 04 03 bus-off (trailing, repurposes flag[0x04] after the VID read):
+        // remove in-transit bus encryption. FULLY best-effort — inert without the
+        // lever, and a failure here never discards an already-obtained unlock.
+        if let Err(e) = self.set_bus_off(scsi) {
+            tracing::debug!(
+                target: "freemkv::disc",
+                phase = "freemkv_bus_off_unavailable",
+                ?e,
+                "bus-off (04 03) not applied; continuing"
+            );
+        }
+        tracing::debug!(
+            target: "freemkv::disc",
+            phase = "freemkv_unlocked",
+            has_vid = vid.is_some(),
+            "freemkv drive unlocked (Raw Read on)"
+        );
+        Ok(Unlocked { vid, bus_key: None })
+    }
+}
+
+impl Unlocker for FreemkvUnlocker {
+    fn name(&self) -> &'static str {
+        "freemkv"
+    }
+
+    /// Recognise the drive by its Identity knock, lift riplock/region
+    /// (best-effort), turn on Raw Read (the actual unlock), and read the Volume
+    /// ID with a bare `0xAD` (best-effort). `Some` when Raw Read succeeded — the
+    /// drive is unlocked whether or not the VID read did; `None` if it isn't a
+    /// freemkv drive; `Err(Transport)` on a dead bus. `ctx` is unused: this
+    /// unlocker self-identifies rather than matching on drive identity.
+    fn unlock(
+        &self,
+        scsi: &mut dyn ScsiTransport,
+        _ctx: &UnlockCtx,
+    ) -> std::result::Result<Option<Unlocked>, UnlockError> {
+        crate::fallthrough(self.full_unlock(scsi))
+    }
+}
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+    use crate::DiscKind;
+    use crate::scsi::mock::{MockTransport, Reply};
+    use crate::scsi::{DataDirection, Result, ScsiResult, ScsiTransport};
+
+    fn ctx(id: &crate::DriveId) -> UnlockCtx<'_> {
+        UnlockCtx::new(id, DiscKind::Unknown)
+    }
+
+    /// Serves a fixed payload with a GOOD status (used for the Identity probe).
+    struct FakeTransport {
+        payload: Vec<u8>,
+    }
+    impl ScsiTransport for FakeTransport {
+        fn execute(
+            &mut self,
+            _cdb: &[u8],
+            _dir: DataDirection,
+            data: &mut [u8],
+            _timeout_ms: u32,
+        ) -> Result<ScsiResult> {
+            let n = self.payload.len().min(data.len());
+            data[..n].copy_from_slice(&self.payload[..n]);
+            Ok(ScsiResult {
+                status: 0,
+                bytes_transferred: n,
+                sense: [0u8; 32],
+            })
+        }
+    }
+
+    fn freemkv_identity_payload() -> Vec<u8> {
+        let mut p = vec![0u8; KNOCK_RESP_LEN];
+        let s = b"freemkv 0.6.4";
+        p[..s.len()].copy_from_slice(s);
+        p
+    }
+
+    /// A well-formed format-0x80 VID structure: 4-byte header, 16-byte VID at
+    /// offset 4, 16-byte MAC (zeroed on the bare path — it isn't checked). 36 =
+    /// the fixed `0xAD` fmt-0x80 response length (see `crate::vid`).
+    fn vid_ds_response(vid: [u8; 16]) -> Vec<u8> {
+        let mut p = vec![0u8; 36];
+        p[4..20].copy_from_slice(&vid);
+        p
+    }
+
+    // ── CDB shape ────────────────────────────────────────────────────────
+
+    /// The 10-byte knock CDB layout is a pinned wire-format contract.
+    #[test]
+    fn build_cdb_encodes_the_knock_shape() {
+        let cdb = build_cdb(subfn::IDENTITY, STATE_OFF, KNOCK_RESP_LEN as u32);
+        assert_eq!(cdb.len(), 10);
+        assert_eq!(cdb[0], 0x3C);
+        assert_eq!(cdb[1], 0x0E);
+        assert_eq!(cdb[2], 0xC0);
+        assert_eq!(cdb[3], 0xDE);
+        assert_eq!(cdb[4], subfn::IDENTITY);
+        assert_eq!(cdb[5], 0x00);
+        assert_eq!([cdb[6], cdb[7], cdb[8]], [0x00, 0x00, 0x40]); // 64 (KNOCK_RESP_LEN), 24-bit BE
+        assert_eq!(cdb[9], 0x00);
+    }
+
+    /// The 24-bit allocation length is big-endian across bytes 6..8.
+    #[test]
+    fn build_cdb_encodes_alloc_len_24bit_big_endian() {
+        let cdb = build_cdb(subfn::RAW_READ, RAW_READ_CERT_VALID, 0x01_2345);
+        assert_eq!([cdb[6], cdb[7], cdb[8]], [0x01, 0x23, 0x45]);
+        assert_eq!(cdb[4], subfn::RAW_READ);
+        assert_eq!(cdb[5], 0x01);
+    }
+
+    /// The sub-function numbers ARE the wire protocol and match the firmware
+    /// `abi.rs::SubFn`: 01 Identity, 02 Speed, 03 Region, 04 Raw Read, 09 DumpAll.
+    #[test]
+    fn subfn_values_match_firmware_abi() {
+        assert_eq!(subfn::IDENTITY, 0x01);
+        assert_eq!(subfn::SPEED, 0x02);
+        assert_eq!(subfn::REGION, 0x03);
+        assert_eq!(subfn::RAW_READ, 0x04);
+        assert_eq!(subfn::DUMP_ALL, 0x09);
+    }
+
+    /// The Raw Read mode selectors match the firmware: 00 OEM, 01 cert-valid
+    /// (this unlocker), 02 accept-any-cert (the AKE path), 03 data-clear (bus-off).
+    #[test]
+    fn raw_read_modes_are_pinned() {
+        assert_eq!(RAW_READ_OFF, 0x00);
+        assert_eq!(RAW_READ_CERT_VALID, 0x01);
+        assert_eq!(RAW_READ_ACCEPT_ANY, 0x02);
+        assert_eq!(RAW_READ_DATA_CLEAR, 0x03);
+    }
+
+    /// Exact CDB bytes for each vendor sub-function that build_cdb serves.
+    #[test]
+    fn build_cdb_exact_bytes_per_subfn() {
+        assert_eq!(
+            build_cdb(subfn::IDENTITY, STATE_OFF, KNOCK_RESP_LEN as u32),
+            [0x3C, 0x0E, 0xC0, 0xDE, 0x01, 0x00, 0x00, 0x00, 0x40, 0x00]
+        );
+        // Toggles carry the fixed KNOCK_RESP_LEN (64 = 0x40) data-in allocation in
+        // bytes 6..8 — the host must read the response or the drive wedges.
+        assert_eq!(
+            build_cdb(subfn::SPEED, SPEED_CAP_MAX, KNOCK_RESP_LEN as u32),
+            [0x3C, 0x0E, 0xC0, 0xDE, 0x02, 0xFF, 0x00, 0x00, 0x40, 0x00]
+        );
+        assert_eq!(
+            build_cdb(subfn::REGION, STATE_ON, KNOCK_RESP_LEN as u32),
+            [0x3C, 0x0E, 0xC0, 0xDE, 0x03, 0x01, 0x00, 0x00, 0x40, 0x00]
+        );
+        assert_eq!(
+            build_cdb(subfn::RAW_READ, RAW_READ_CERT_VALID, KNOCK_RESP_LEN as u32),
+            [0x3C, 0x0E, 0xC0, 0xDE, 0x04, 0x01, 0x00, 0x00, 0x40, 0x00]
+        );
+        assert_eq!(
+            build_cdb(subfn::RAW_READ, RAW_READ_DATA_CLEAR, KNOCK_RESP_LEN as u32),
+            [0x3C, 0x0E, 0xC0, 0xDE, 0x04, 0x03, 0x00, 0x00, 0x40, 0x00]
+        );
+    }
+
+    // ── Detection ────────────────────────────────────────────────────────
+
+    #[test]
+    fn identify_true_on_freemkv_marker_and_issues_identity_cdb() {
+        let mut t = MockTransport::always(Reply::good(freemkv_identity_payload()));
+        assert!(FreemkvUnlocker::new().identify(&mut t).expect("no fault"));
+        assert_eq!(
+            t.cdbs[0],
+            build_cdb(subfn::IDENTITY, STATE_OFF, KNOCK_RESP_LEN as u32)
+        );
+    }
+
+    #[test]
+    fn identify_false_on_non_matching_payload() {
+        let mut t = FakeTransport {
+            payload: vec![0u8; KNOCK_RESP_LEN],
+        };
+        assert!(!FreemkvUnlocker::new().identify(&mut t).expect("no fault"));
+    }
+
+    #[test]
+    fn identify_false_on_short_response() {
+        let mut t = MockTransport::always(Reply::short(freemkv_identity_payload(), 3));
+        assert!(!FreemkvUnlocker::new().identify(&mut t).expect("no fault"));
+    }
+
+    #[test]
+    fn identify_false_when_command_rejected() {
+        let mut t = MockTransport::always(Reply::illegal_request());
+        assert!(!FreemkvUnlocker::new().identify(&mut t).expect("no fault"));
+    }
+
+    #[test]
+    fn identify_false_when_command_rejected_as_err() {
+        let mut t = MockTransport::always(Reply::illegal_request_as_err());
+        assert!(!FreemkvUnlocker::new().identify(&mut t).expect("no fault"));
+    }
+
+    #[test]
+    fn identify_transport_fault_aborts() {
+        let mut t = MockTransport::always(Reply::TransportFault);
+        assert_eq!(
+            FreemkvUnlocker::new().identify(&mut t).unwrap_err(),
+            UnlockError::Transport
+        );
+    }
+
+    // ── Feature toggles ──────────────────────────────────────────────────
+
+    #[test]
+    fn set_speed_max_issues_the_cap_ff_cdb() {
+        let mut t = MockTransport::always(Reply::good(vec![]));
+        FreemkvUnlocker::new()
+            .set_speed(&mut t, SPEED_CAP_MAX)
+            .expect("ok");
+        assert_eq!(
+            t.cdbs[0],
+            [0x3C, 0x0E, 0xC0, 0xDE, 0x02, 0xFF, 0x00, 0x00, 0x40, 0x00]
+        );
+    }
+
+    #[test]
+    fn set_speed_intermediate_cap_is_placed_in_state_byte() {
+        let mut t = MockTransport::always(Reply::good(vec![]));
+        FreemkvUnlocker::new().set_speed(&mut t, 0x42).expect("ok");
+        assert_eq!(
+            t.cdbs[0],
+            [0x3C, 0x0E, 0xC0, 0xDE, 0x02, 0x42, 0x00, 0x00, 0x40, 0x00]
+        );
+    }
+
+    #[test]
+    fn set_region_free_on_issues_the_state_01_cdb() {
+        let mut t = MockTransport::always(Reply::good(vec![]));
+        FreemkvUnlocker::new()
+            .set_region_free(&mut t, true)
+            .expect("ok");
+        assert_eq!(
+            t.cdbs[0],
+            [0x3C, 0x0E, 0xC0, 0xDE, 0x03, 0x01, 0x00, 0x00, 0x40, 0x00]
+        );
+    }
+
+    #[test]
+    fn set_raw_read_issues_the_04_01_cdb() {
+        let mut t = MockTransport::always(Reply::good(vec![]));
+        FreemkvUnlocker::new().set_raw_read(&mut t).expect("ok");
+        assert_eq!(
+            t.cdbs[0],
+            [0x3C, 0x0E, 0xC0, 0xDE, 0x04, 0x01, 0x00, 0x00, 0x40, 0x00]
+        );
+    }
+
+    #[test]
+    fn set_bus_off_issues_the_04_03_cdb() {
+        let mut t = MockTransport::always(Reply::good(vec![]));
+        FreemkvUnlocker::new().set_bus_off(&mut t).expect("ok");
+        assert_eq!(
+            t.cdbs[0],
+            [0x3C, 0x0E, 0xC0, 0xDE, 0x04, 0x03, 0x00, 0x00, 0x40, 0x00]
+        );
+    }
+
+    #[test]
+    fn toggle_drive_rejection_is_not_applicable() {
+        let mut t = MockTransport::always(Reply::illegal_request());
+        let err = FreemkvUnlocker::new().set_raw_read(&mut t).unwrap_err();
+        assert_eq!(err, UnlockError::NotApplicable);
+    }
+
+    #[test]
+    fn toggle_transport_fault_propagates() {
+        let mut t = MockTransport::always(Reply::TransportFault);
+        let err = FreemkvUnlocker::new()
+            .set_speed(&mut t, SPEED_CAP_MAX)
+            .unwrap_err();
+        assert_eq!(err, UnlockError::Transport);
+    }
+
+    // ── full_unlock / unlock ─────────────────────────────────────────────
+    // The bare 0xAD VID read itself is tested in crate::vid; here we cover the
+    // full unlock sequence end-to-end, including the best-effort VID handling.
+
+    /// The full unlock runs `01→03→02→04 01→AD→04 03` in order: identity, region,
+    /// speed, raw-read, bare VID read, then the trailing bus-off knock.
+    #[test]
+    fn full_unlock_issues_identity_region_speed_rawread_then_bare_vid() {
+        let vid = [0x7Cu8; 16];
+        let mut t = MockTransport::scripted(
+            vec![
+                Reply::good(freemkv_identity_payload()), // 01 Identity
+                Reply::good(vec![]),                     // 03 Region
+                Reply::good(vec![]),                     // 02 Speed
+                Reply::good(vec![]),                     // 04 01 Raw Read
+                Reply::good(vid_ds_response(vid)),       // 0xAD bare VID
+                Reply::good(vec![]),                     // 04 03 bus-off (trailing)
+            ],
+            Reply::TransportFault,
+        );
+        let id = crate::DriveId::default();
+        let out = FreemkvUnlocker::new()
+            .unlock(&mut t, &ctx(&id))
+            .expect("no fault")
+            .expect("Raw Read on ⇒ unlocked");
+        assert_eq!(out.vid, Some(vid));
+        // Four vendor knocks, the standard 0xAD read, then the bus-off knock.
+        assert_eq!(t.cdbs.len(), 6);
+        let vendor_subfns: Vec<u8> = t.cdbs[..4].iter().map(|c| c[4]).collect();
+        assert_eq!(vendor_subfns, vec![0x01, 0x03, 0x02, 0x04]);
+        assert_eq!(t.cdbs[3][5], RAW_READ_CERT_VALID); // Raw Read state 01
+        assert_eq!(t.cdbs[4][0], crate::scsi::SCSI_READ_DISC_STRUCTURE);
+        // Trailing bus-off: Raw Read sub-function, state 03 (data-clear).
+        assert_eq!(t.cdbs[5][4], subfn::RAW_READ);
+        assert_eq!(t.cdbs[5][5], RAW_READ_DATA_CLEAR);
+    }
+
+    /// A missing region/speed feature does not fail the unlock (best-effort).
+    #[test]
+    fn full_unlock_tolerates_missing_region_and_speed() {
+        let vid = [0x3Cu8; 16];
+        let mut t = MockTransport::scripted(
+            vec![
+                Reply::good(freemkv_identity_payload()), // identity
+                Reply::illegal_request(),                // region unsupported
+                Reply::illegal_request(),                // speed unsupported
+                Reply::good(vec![]),                     // raw read
+                Reply::good(vid_ds_response(vid)),       // bare VID
+            ],
+            Reply::TransportFault,
+        );
+        let id = crate::DriveId::default();
+        let out = FreemkvUnlocker::new()
+            .unlock(&mut t, &ctx(&id))
+            .expect("no fault")
+            .expect("unlocked");
+        assert_eq!(out.vid, Some(vid));
+    }
+
+    /// Raw Read is LOAD-BEARING: if the drive rejects it, this isn't an
+    /// unlockable freemkv drive, so `unlock()` declines (`Ok(false)`) and falls
+    /// through — never a hard error.
+    #[test]
+    fn declines_when_raw_read_rejected() {
+        let mut t = MockTransport::scripted(
+            vec![
+                Reply::good(freemkv_identity_payload()), // identity
+                Reply::good(vec![]),                     // region
+                Reply::good(vec![]),                     // speed
+                Reply::illegal_request(),                // raw read REJECTED
+            ],
+            Reply::TransportFault,
+        );
+        let id = crate::DriveId::default();
+        assert!(
+            FreemkvUnlocker::new()
+                .unlock(&mut t, &ctx(&id))
+                .expect("no fault")
+                .is_none()
+        );
+    }
+
+    /// Best-effort VID: Raw Read succeeded (drive unlocked), but the bare VID
+    /// read was rejected — `unlock()` still returns `true`, with `vid() == None`.
+    /// This is the fix: the old load-bearing VID aborted the whole unlock here,
+    /// dropping a genuinely-unlocked drive to the cert path.
+    #[test]
+    fn unlocks_without_vid_when_bare_read_fails() {
+        let mut t = MockTransport::scripted(
+            vec![
+                Reply::good(freemkv_identity_payload()), // identity
+                Reply::good(vec![]),                     // region
+                Reply::good(vec![]),                     // speed
+                Reply::good(vec![]),                     // raw read
+                Reply::illegal_request(),                // bare VID: no medium
+            ],
+            Reply::TransportFault,
+        );
+        let id = crate::DriveId::default();
+        let out = FreemkvUnlocker::new()
+            .unlock(&mut t, &ctx(&id))
+            .expect("no fault")
+            .expect("unlocked despite no VID");
+        assert_eq!(out.vid, None);
+    }
+
+    #[test]
+    fn declines_non_freemkv_drive() {
+        let mut t = MockTransport::always(Reply::illegal_request());
+        let id = crate::DriveId::default();
+        assert!(
+            FreemkvUnlocker::new()
+                .unlock(&mut t, &ctx(&id))
+                .expect("no fault")
+                .is_none()
+        );
+    }
+
+    #[test]
+    fn transport_fault_propagates() {
+        let mut t = MockTransport::always(Reply::TransportFault);
+        let id = crate::DriveId::default();
+        assert_eq!(
+            FreemkvUnlocker::new()
+                .unlock(&mut t, &ctx(&id))
+                .unwrap_err(),
+            UnlockError::Transport
+        );
+    }
+
+    // ── DumpAll (subfn 0x09) ─────────────────────────────────────────────
+
+    #[test]
+    fn dump_ram_builds_memread_cdb_and_returns_the_window() {
+        let mut payload = vec![0xABu8; MEMREAD_LEN];
+        payload[0] = 0xEE;
+        let mut t = MockTransport::always(Reply::good(payload));
+        let got = FreemkvUnlocker::new()
+            .dump_ram(&mut t, 0x01F8_1234)
+            .expect("dump ok");
+        assert_eq!(got[0], 0xEE);
+        assert_eq!(
+            t.cdbs[0],
+            [0x3C, 0x0E, 0xC0, 0xDE, 0x09, 0x01, 0xF8, 0x12, 0x34, 0x00]
+        );
+    }
+
+    #[test]
+    fn build_memread_cdb_packs_address_big_endian_at_5_to_9() {
+        let cdb = build_memread_cdb(0xDEAD_BEEF);
+        assert_eq!(cdb[4], subfn::DUMP_ALL);
+        assert_eq!([cdb[5], cdb[6], cdb[7], cdb[8]], [0xDE, 0xAD, 0xBE, 0xEF]);
+        assert_eq!(cdb[9], 0x00);
+    }
+
+    #[test]
+    fn name_is_freemkv() {
+        assert_eq!(FreemkvUnlocker::new().name(), "freemkv");
+    }
+}

src/ld/NOTICE

@@ -0,0 +1,15 @@
+THIRD-PARTY NOTICE for profiles.json
+====================================
+
+`profiles.json` in this directory contains volatile, RAM-only optical-drive
+microcode ("LibreDrive") authored and owned by the MakeMKV team (GuinpinSoft
+inc.). Each payload embeds its own notice:
+
+    "This is volatile RAM-only microcode for LibreDrive by MakeMKV -
+     http://www.makemkv.com/libredrive/
+     Copyright (c) MakeMKV team, all rights reserved."
+
+This microcode is third-party proprietary material — Copyright (c) MakeMKV team,
+all rights reserved — and is NOT covered by this repository's MIT LICENSE. No
+ownership is claimed. See the repository-root NOTICE. Thanks to the MakeMKV team
+for LibreDrive: https://www.makemkv.com/ , http://www.makemkv.com/libredrive/ .

src/ld/README.md

@@ -19,3 +19,13 @@ This module performs only the **non-persistent** unlock — the access state it
 sets up lives in drive RAM and is gone on power cycle. The one-time, permanent
 drive preparation is the drive owner's own manual step and is **never**
 automated here.
+
+## Third-party microcode (`profiles.json`)
+
+`profiles.json` bundles volatile, RAM-only optical-drive microcode
+("LibreDrive") authored and owned by the **MakeMKV team** (GuinpinSoft inc.).
+Each payload carries its own notice — *"Copyright (c) MakeMKV team, all rights
+reserved."* This microcode is **third-party proprietary material and is NOT
+covered by this repository's MIT license**; no ownership is claimed. See the
+[`NOTICE`](./NOTICE) in this directory and the repository-root `NOTICE`. With
+thanks to the MakeMKV team for [LibreDrive](http://www.makemkv.com/libredrive/).

src/ld/cdb.rs

@@ -1,7 +1,7 @@
 //! Public unlock-CDB seam.
 //!
 //! These items expose the *minimum* an external consumer (e.g. the bdemu
-//! drive emulator) needs to recognise and answer the Mt1959Unlocker unlock
+//! drive emulator) needs to recognise and answer the LdUnlocker unlock
 //! READ_BUFFER handshake, without that consumer open-coding the
 //! handshake internals. The concrete CDB shapes and the verification
 //! marker are unlock-handshake details and must live ONLY in this crate.
@@ -15,7 +15,7 @@ pub const UNLOCK_MARKER: &[u8] = b"MMkv";
 
 /// Returns `true` when a READ_BUFFER (`0x3C`) CDB with the given mode
 /// (`cdb[1] & 0x1F`) and buffer id (`cdb[2]`) is an unlock-handshake
-/// read — i.e. one of the Mt1959Unlocker unlock variants.
+/// read — i.e. one of the LdUnlocker unlock variants.
 ///
 /// This is the single source of truth for the unlock READ_BUFFER CDB
 /// shapes; consumers must call it rather than hardcoding the mode /

src/ld/mod.rs

@@ -6,13 +6,9 @@
 //! encryption AT THE DRIVE (the unlocked drive serves clear content) and
 //! reporting the OEM Volume ID.
 
-// `cdb` carries ONLY the unlock-handshake wire format that the bdemu emulator
-// needs (the real unlocker drives its CDBs from per-drive profile templates, not
-// these constants). Compile it only when the `emulation` feature exposes it, so
-// it never dead-codes in a normal build.
-// Compiled under `cfg(test)` as well as the feature: its tests pin the unlock
-// wire format, and gating the whole module on a non-default feature meant CI
-// (which builds with default features) never ran them.
+// `cdb` is only the bdemu emulator's wire format (real unlocking uses profile
+// templates), gated behind `emulation`; also compiled under `cfg(test)` so its
+// wire-format tests run in default-feature CI. See docs/ld-mod.md — cdb gating.
 #[cfg(any(feature = "emulation", test))]
 mod cdb;
 mod error;
@@ -24,12 +20,8 @@ use crate::scsi::{DataDirection, ScsiTransport};
 use crate::{DriveId, UnlockCtx, UnlockError, Unlocked, Unlocker};
 
 // ── Public profile catalog ──────────────────────────────────────────────────
-//
-// The catalog of drives the MT1959 unlocker recognizes is the one piece of
-// ld worth exposing publicly: it answers "is this drive supported?" without
-// unlocking, and the bdemu test-emulator reads it to impersonate a supported
-// drive. The unlock *mechanism* (firmware blobs, upload sequence, CDB wire
-// format) stays private — only the catalog and its match result are public.
+// Only the catalog is public (supported-drive lookup, used by bdemu); the
+// unlock mechanism stays private. See docs/ld-mod.md — public catalog design.
 
 pub use profile::{DriveProfile as Profile, Identity, Platform, ProfileMatch, Profiles};
 
@@ -54,17 +46,15 @@ pub fn profile(drive_id: &DriveId) -> Option<ProfileMatch> {
 #[cfg(feature = "emulation")]
 pub use cdb::{UNLOCK_MARKER, is_unlock_read_buffer};
 
-/// The MT1959 unlocker. `pub(crate)` — clients reach it only through
-/// [`crate::all_unlockers`], never by name (the locked-design contract).
-///
-/// Matches a drive against the bundled profile database and, on a hit,
-/// runs the MediaTek MT1959 firmware-unlock (and disc-speed calibration)
-/// handshake over the raw SCSI transport.
-pub(crate) struct Mt1959Unlocker;
+// The MT1959 unlocker. Matches a drive against the bundled profile database and
+// runs the firmware-unlock + disc-speed-calibration handshake over raw SCSI.
+// Stateless: `unlock()` returns what it learned.
+#[derive(Default)]
+pub struct LdUnlocker;
 
-impl Mt1959Unlocker {
-    pub(crate) fn new() -> Self {
-        Mt1959Unlocker
+impl LdUnlocker {
+    pub fn new() -> Self {
+        LdUnlocker
     }
 }
 
@@ -72,21 +62,13 @@ impl Mt1959Unlocker {
 /// drive-info "is this drive supported?" display), or `None`. A pure profile
 /// lookup — does NOT touch the drive or unlock anything.
 pub(crate) fn firmware_name(id: &DriveId) -> Option<&'static str> {
-    profile::find_bundled(id).map(|_| "MT1959")
+    profile::find_bundled(id).map(|_| "LD")
 }
 
-impl Mt1959Unlocker {
-    /// Read the OEM Volume ID via the matched profile's vendor CDB.
-    ///
-    /// Takes the profile the caller ALREADY matched rather than re-running
-    /// `find_bundled`: the catalog is 206 entries and drive-prep used to scan it
-    /// twice per drive (once here, once in `firmware_unlock`) for the same
-    /// answer.
-    ///
-    /// `Ok(Some(vid))` on a well-formed 36-byte response (signature `00 22 00`,
-    /// VID at `[4..20]`); `Ok(None)` when there is no OEM-VID CDB or the response
-    /// is short / bad-signature / drive-rejected (the drive is still unlocked,
-    /// just no VID); `Err` only on a transport fault.
+impl LdUnlocker {
+    // Read the OEM Volume ID via the matched profile's vendor CDB (profile passed
+    // in to avoid a redundant 206-entry catalog scan). Ok(Some) on a well-formed
+    // response; Ok(None) if unreadable; Err only on a transport fault.
     fn read_oem_vid(
         &self,
         scsi: &mut dyn ScsiTransport,
@@ -101,10 +83,9 @@ impl Mt1959Unlocker {
 
         let mut buf = vec![0u8; RESPONSE_LEN];
         let result = scsi.execute(&cdb, DataDirection::FromDevice, &mut buf, 5_000)?;
-        // Per the transport contract a drive sense arrives as `Ok` with a
-        // non-zero `status`, NOT as `Err`. Without this check a CHECK CONDITION
-        // would be read as a successful 36-byte response and the caller's own
-        // zero fill parsed as a Volume ID.
+        // Per the transport contract a drive sense arrives as `Ok` with non-zero
+        // `status`, not `Err` — without this check a CHECK CONDITION reads as a
+        // successful response and the zero-filled buffer parses as a bogus VID.
         if result.status != 0 {
             tracing::warn!(
                 target: "freemkv::disc",
@@ -138,16 +119,10 @@ impl Mt1959Unlocker {
     }
 }
 
-impl Mt1959Unlocker {
-    /// The MediaTek firmware unlock. Because the MT1959 unlocker removes AACS bus
-    /// encryption AT THE DRIVE (the unlocked drive serves CLEAR content), this ONE
-    /// operation satisfies BOTH the drive-features and the bus-removal capability
-    /// — so `unlock_features` and `unlock_bus` both delegate here. The result
-    /// carries `drive_unlocked: true` (no bus key needed) and the OEM Volume ID.
-    ///
-    /// A no-firmware-route drive (Renesas / no profile) returns `NotApplicable`; a
-    /// transport fault propagates as `Transport`; any other firmware failure also
-    /// falls through as `NotApplicable`.
+impl LdUnlocker {
+    // The MediaTek firmware unlock. Since it removes AACS at the drive (clear
+    // content), this one op satisfies both features and bus-removal, so both
+    // trait methods delegate here. See docs/ld-mod.md — firmware_unlock contract.
     fn firmware_unlock(
         &self,
         scsi: &mut dyn ScsiTransport,
@@ -167,14 +142,9 @@ impl Mt1959Unlocker {
         // A transport fault → UnlockError::Transport; any other firmware failure
         // → NotApplicable (via From<error::Error>).
         mt.init(scsi)?;
-        // `init` only proves the handshake COMPLETED — `do_unlock` returns Ok on
-        // a response that carried the per-drive signature but not both firmware
-        // markers. Only `is_unlocked()` means the drive actually reached the
-        // extended-access state and serves clear content. Reporting
-        // `drive_unlocked: true` off `init` alone told the consumer the bus was
-        // clear on a half-unlocked drive, which suppressed the cert-auth
-        // fallback and shipped ciphertext at rc=0. A partial unlock must fall
-        // through to the next unlocker.
+        // `init` only proves the handshake completed, not that the drive reached
+        // extended-access state. Reporting unlocked off `init` alone shipped
+        // ciphertext at rc=0. See docs/ld-mod.md — half-unlock fallback.
         if !mt.is_unlocked() {
             tracing::warn!(
                 target: "freemkv::disc",
@@ -183,19 +153,13 @@ impl Mt1959Unlocker {
             );
             return Err(UnlockError::NotApplicable);
         }
-        // Prime the per-region speed table. Best-effort — it must not fail the
-        // unlock — but NOT silent: speed calibration can fail completely, and an
-        // unlogged `let _ =` made a fully-failed calibration indistinguishable
-        // from a successful one in the rip log.
+        // Prime the per-region speed table. Best-effort (must not fail the unlock)
+        // but not silent — an unlogged `let _ =` made a failed calibration
+        // indistinguishable from success in the rip log.
         if let Err(e) = mt.probe_disc(scsi) {
-            // A transport fault here is a DEAD BUS, not a speed-calibration miss.
-            // The rest of this path (`read_oem_vid`) is a no-op for the 140/206
-            // profiles that carry no `read_vid_cdb`, so it never touches the bus
-            // again — meaning this swallowed fault was the ONLY dead-bus signal,
-            // and warn-and-continue turned it into `Ok(Unlocked{drive_unlocked:
-            // true})`: a dead bus rendered as a fully-unlocked drive (the flagship
-            // failure-that-looks-like-success). A genuine calibration miss (drive
-            // sense / short reply) still continues on the default speed table.
+            // A transport fault here is a dead bus, not a calibration miss — most
+            // profiles never touch the bus again, so this was the only dead-bus
+            // signal. See docs/ld-mod.md — probe_disc dead-bus classification.
             if e.is_transport_failure() {
                 tracing::warn!(
                     target: "freemkv::disc",
@@ -212,38 +176,26 @@ impl Mt1959Unlocker {
             );
         }
         let vid = self.read_oem_vid(scsi, &m.profile)?;
-        Ok(Unlocked {
-            vid,
-            bus_key: None,
-            drive_unlocked: true,
-        })
+        Ok(Unlocked { vid, bus_key: None })
     }
 }
 
-impl Unlocker for Mt1959Unlocker {
+impl Unlocker for LdUnlocker {
     fn name(&self) -> &'static str {
-        "MT1959"
+        "LD"
     }
 
-    /// Mt1959Unlocker provides drive features (riplock/speed, OEM VID) — and, because
-    /// its firmware unlock serves clear content, bus removal comes free with it.
-    fn unlock_features(
+    /// Match the drive against the bundled profile database and run the firmware
+    /// unlock — one op that removes bus encryption at the drive (clear content)
+    /// and reads the OEM Volume ID (best-effort). `Some` when the firmware
+    /// handshake reached the extended-access state; `None` for an unknown drive
+    /// or an incomplete handshake; `Err(Transport)` on a dead bus.
+    fn unlock(
         &self,
         scsi: &mut dyn ScsiTransport,
         ctx: &UnlockCtx,
-    ) -> std::result::Result<Unlocked, UnlockError> {
-        self.firmware_unlock(scsi, ctx)
-    }
-
-    /// Same firmware code as [`unlock_features`]: Mt1959Unlocker removes the bus at
-    /// the drive. In practice the consumer skips this because drive-prep already
-    /// set `drive_unlocked`; it's here for completeness / a bus-first call order.
-    fn unlock_bus(
-        &self,
-        scsi: &mut dyn ScsiTransport,
-        ctx: &UnlockCtx,
-    ) -> std::result::Result<Unlocked, UnlockError> {
-        self.firmware_unlock(scsi, ctx)
+    ) -> std::result::Result<Option<Unlocked>, UnlockError> {
+        crate::fallthrough(self.firmware_unlock(scsi, ctx))
     }
 }
 
@@ -256,7 +208,7 @@ mod tests {
     /// Unlock context for a fake drive id (kind/host-certs irrelevant to the
     /// firmware unlocker — it keys off the drive identity).
     fn ctx(id: &DriveId) -> UnlockCtx<'_> {
-        UnlockCtx::new(id, DiscKind::Unknown, &[])
+        UnlockCtx::new(id, DiscKind::Unknown)
     }
 
     /// A fake transport that fills the response buffer from a fixed payload and
@@ -322,7 +274,7 @@ mod tests {
             payload,
             bytes_transferred: 36,
         };
-        let got = Mt1959Unlocker::new()
+        let got = LdUnlocker::new()
             .read_oem_vid(&mut t, &known_vid_profile())
             .expect("parse ok");
         assert_eq!(got, Some(vid), "VID parsed from [4..20]");
@@ -335,7 +287,7 @@ mod tests {
             payload: vec![0u8; 36],
             bytes_transferred: 20,
         };
-        let got = Mt1959Unlocker::new()
+        let got = LdUnlocker::new()
             .read_oem_vid(&mut t, &known_vid_profile())
             .expect("short response is Ok(None)");
         assert_eq!(got, None);
@@ -350,7 +302,7 @@ mod tests {
             payload,
             bytes_transferred: 36,
         };
-        let got = Mt1959Unlocker::new()
+        let got = LdUnlocker::new()
             .read_oem_vid(&mut t, &known_vid_profile())
             .expect("bad header is Ok(None)");
         assert_eq!(got, None);
@@ -363,7 +315,7 @@ mod tests {
     fn read_oem_vid_check_condition_is_none_not_a_vid() {
         use crate::scsi::mock::{MockTransport, Reply};
         let mut t = MockTransport::always(Reply::illegal_request());
-        let got = Mt1959Unlocker::new()
+        let got = LdUnlocker::new()
             .read_oem_vid(&mut t, &known_vid_profile())
             .expect("a drive sense is not a transport fault");
         assert_eq!(got, None, "a CHECK CONDITION must never yield a VID");
@@ -375,7 +327,7 @@ mod tests {
     fn read_oem_vid_transport_fault_propagates() {
         use crate::scsi::mock::{MockTransport, Reply};
         let mut t = MockTransport::always(Reply::TransportFault);
-        let err = Mt1959Unlocker::new()
+        let err = LdUnlocker::new()
             .read_oem_vid(&mut t, &known_vid_profile())
             .expect_err("a dead bus must not be Ok(None)");
         assert!(err.is_transport_failure());
@@ -391,7 +343,7 @@ mod tests {
             payload: vec![0u8; 36],
             bytes_transferred: 36,
         };
-        let got = Mt1959Unlocker::new()
+        let got = LdUnlocker::new()
             .read_oem_vid(&mut t, &p)
             .expect("no CDB is Ok(None)");
         assert_eq!(got, None);
@@ -414,21 +366,18 @@ mod tests {
             payload: vec![0u8; 36],
             bytes_transferred: 36,
         };
-        let err = Mt1959Unlocker::new()
-            .unlock_features(
+        let unlocked = LdUnlocker::new()
+            .unlock(
                 &mut t,
                 &ctx(&make_drive_id("FAKE-VND", "9.99", "XX12345", "")),
             )
-            .expect_err("no profile → NotApplicable");
-        assert_eq!(err, UnlockError::NotApplicable);
+            .expect("no profile → declines, not a hard error");
+        assert!(unlocked.is_none());
     }
 
-    /// THE defect-1 test. The drive answers every unlock READ_BUFFER with a
-    /// response carrying the per-drive signature and the primary firmware marker
-    /// but NOT the secondary one — `do_unlock` returns `Ok` and `init()`
-    /// succeeds, yet the drive is NOT in the extended-access state. Reporting
-    /// `drive_unlocked: true` here suppresses the cert-auth fallback. Catches
-    /// removing the `is_unlocked()` gate in `firmware_unlock`.
+    // THE defect-1 test: response carries the signature + primary marker but not
+    // the secondary one, so init() succeeds but the drive isn't in extended-access
+    // state. Catches removing the `is_unlocked()` gate in `firmware_unlock`.
     #[test]
     fn half_unlocked_drive_falls_through_instead_of_claiming_unlocked() {
         use crate::scsi::mock::{MockTransport, Reply};
@@ -445,17 +394,20 @@ mod tests {
         resp[12..16].copy_from_slice(&[0x4D, 0x4D, 0x6B, 0x76]);
 
         let mut t = MockTransport::always(Reply::good(resp));
-        let err = Mt1959Unlocker::new()
-            .unlock_features(&mut t, &ctx(&id))
-            .expect_err("a half-unlocked drive must fall through to cert-auth");
-        assert_eq!(err, UnlockError::NotApplicable);
+        let unlocked = LdUnlocker::new()
+            .unlock(&mut t, &ctx(&id))
+            .expect("a half-unlock declines, not a hard error");
+        assert!(
+            unlocked.is_none(),
+            "a half-unlocked drive must fall through to cert-auth"
+        );
     }
 
-    /// The whole-unlock happy path still reports `drive_unlocked` when BOTH
-    /// firmware markers are present — the `is_unlocked()` gate must not have
-    /// made a genuinely unlocked drive fall through.
+    /// The whole-unlock happy path still reports unlocked when BOTH firmware
+    /// markers are present — the `is_unlocked()` gate must not have made a
+    /// genuinely unlocked drive fall through.
     #[test]
-    fn fully_unlocked_drive_reports_drive_unlocked() {
+    fn fully_unlocked_drive_reports_unlocked() {
         use crate::scsi::mock::{MockTransport, Reply};
         let id = known_vid_drive_id();
         let sig = profile::find_bundled(&id)
@@ -469,22 +421,18 @@ mod tests {
         resp[16..20].copy_from_slice(&[0x4C, 0x62, 0x44, 0x72]);
 
         let mut t = MockTransport::always(Reply::good(resp));
-        let u = Mt1959Unlocker::new()
-            .unlock_features(&mut t, &ctx(&id))
-            .expect("both markers → unlocked");
-        assert!(u.drive_unlocked);
+        assert!(
+            LdUnlocker::new()
+                .unlock(&mut t, &ctx(&id))
+                .expect("no fault")
+                .is_some(),
+            "both markers → unlocked"
+        );
     }
 
-    /// THE probe-disc dead-bus test. The drive unlocks fully (both firmware
-    /// markers), then the bus DIES during disc-speed calibration. `probe_disc`
-    /// used to be warn-and-continued regardless of the fault, and — because the
-    /// 140/206 profiles without a `read_vid_cdb` never touch the bus again — a
-    /// dead bus was reported as `Ok(Unlocked{drive_unlocked:true})`: a dead bus
-    /// rendered as a successful unlock (the flagship failure-that-looks-like-
-    /// success). `firmware_unlock` must now abort with `Transport`.
-    /// MUTATION: reverting the `if e.is_transport_failure()` return in
-    /// `firmware_unlock` (warn-and-continue), OR flattening the probe loops'
-    /// transport classification, makes this go red.
+    // THE probe-disc dead-bus test: bus dies during speed calibration after a
+    // full unlock; must abort with Transport, not report a successful unlock.
+    // See docs/ld-mod.md — probe_disc dead-bus test / mutation notes.
     #[test]
     fn transport_fault_during_probe_disc_is_transport_not_a_successful_unlock() {
         let id = known_vid_drive_id();
@@ -528,17 +476,15 @@ mod tests {
         resp[16..20].copy_from_slice(&[0x4C, 0x62, 0x44, 0x72]); // secondary marker
         let mut t = ProbeFaultsDrive { resp };
 
-        let err = Mt1959Unlocker::new()
-            .unlock_features(&mut t, &ctx(&id))
+        let err = LdUnlocker::new()
+            .unlock(&mut t, &ctx(&id))
             .expect_err("a dead bus during probe must abort, not report success");
         assert_eq!(err, UnlockError::Transport);
     }
 
-    /// A drive-sense (not a dead bus) rejecting the speed probe is a genuine
-    /// calibration miss, not a transport fault — `firmware_unlock` must
-    /// warn-and-continue on the drive's default speed table, still reporting a
-    /// full unlock. The non-transport sibling of
-    /// `transport_fault_during_probe_disc_is_transport_not_a_successful_unlock`.
+    // A drive-sense (not a dead bus) rejecting the speed probe is a genuine
+    // calibration miss: warn-and-continue on the default speed table, still
+    // reporting a full unlock. Non-transport sibling of the probe-fault test.
     #[test]
     fn drive_sense_during_probe_disc_still_reports_a_successful_unlock() {
         let id = known_vid_drive_id();
@@ -581,34 +527,12 @@ mod tests {
             }
         }
         let mut t = ProbeSenseDrive { resp };
-        let u = Mt1959Unlocker::new()
-            .unlock_features(&mut t, &ctx(&id))
-            .expect("a calibration miss must not fail the whole unlock");
-        assert!(u.drive_unlocked);
-    }
-
-    /// `unlock_bus` is the same firmware code as `unlock_features` (bus removal
-    /// comes free with the drive unlock) — pins that the trait method actually
-    /// delegates rather than being an accidental no-op stub.
-    #[test]
-    fn unlock_bus_delegates_to_the_same_firmware_unlock() {
-        use crate::scsi::mock::{MockTransport, Reply};
-        let id = known_vid_drive_id();
-        let sig = profile::find_bundled(&id)
-            .expect("profile")
-            .profile
-            .signature;
-
-        let mut resp = vec![0u8; 64];
-        resp[0..4].copy_from_slice(&sig);
-        resp[12..16].copy_from_slice(&[0x4D, 0x4D, 0x6B, 0x76]);
-        resp[16..20].copy_from_slice(&[0x4C, 0x62, 0x44, 0x72]);
-
-        let mut t = MockTransport::always(Reply::good(resp));
-        let u = Mt1959Unlocker::new()
-            .unlock_bus(&mut t, &ctx(&id))
-            .expect("unlock_bus runs the same firmware handshake");
-        assert!(u.drive_unlocked);
+        assert!(
+            LdUnlocker::new()
+                .unlock(&mut t, &ctx(&id))
+                .expect("a calibration miss must not fail the whole unlock")
+                .is_some()
+        );
     }
 
     /// Catches classifying a dead bus as "not this unlocker's drive": the very
@@ -618,8 +542,8 @@ mod tests {
     fn transport_fault_during_unlock_is_transport_not_not_applicable() {
         use crate::scsi::mock::{MockTransport, Reply};
         let mut t = MockTransport::always(Reply::TransportFault);
-        let err = Mt1959Unlocker::new()
-            .unlock_features(&mut t, &ctx(&known_vid_drive_id()))
+        let err = LdUnlocker::new()
+            .unlock(&mut t, &ctx(&known_vid_drive_id()))
             .expect_err("dead bus");
         assert_eq!(err, UnlockError::Transport);
     }

src/ld/platform/mod.rs

@@ -1,4 +1,4 @@
-//! Platform-specific drive unlock + disc probing (Mt1959Unlocker internals).
+//! Platform-specific drive unlock + disc probing (LdUnlocker internals).
 
 pub mod mt1959;
 
@@ -18,7 +18,7 @@ pub(crate) trait PlatformDriver: Send {
     fn is_ready(&self) -> bool;
 
     /// True if the drive is currently in the extended-access state. THE gate
-    /// `Mt1959Unlocker::firmware_unlock` reports `drive_unlocked` from — `init()`
+    /// `LdUnlocker::firmware_unlock` reports `drive_unlocked` from — `init()`
     /// returning `Ok` only means the handshake completed, not that the drive
     /// serves clear content.
     fn is_unlocked(&self) -> bool {
@@ -31,10 +31,8 @@ mod tests {
     use super::*;
     use crate::scsi::mock::{MockTransport, Reply};
 
-    /// A minimal driver that takes every `PlatformDriver` default (`is_unlocked`)
-    /// so the shim's dispatch and the default's `false` are both exercised —
-    /// every real driver (`Mt1959`) overrides `is_unlocked`, so without this the
-    /// default body is dead in coverage even though it is reachable code.
+    // Minimal driver taking the `PlatformDriver` default `is_unlocked`, so
+    // the default's `false` path is exercised (real drivers override it).
     struct StubDriver;
 
     impl PlatformDriver for StubDriver {

src/ld/platform/mt1959/mod.rs

@@ -117,21 +117,17 @@ impl Mt1959 {
         expected: usize,
     ) -> Result<usize> {
         // The READ_BUFFER CDB transfer-length is a single byte; an
-        // `expected` above 255 cannot be expressed and would silently
-        // truncate. All in-crate callers pass small fixed sizes (4); guard
-        // the invariant rather than emit a malformed CDB.
+        // `expected` above 255 would silently truncate. Guard the
+        // invariant rather than emit a malformed CDB.
         debug_assert!(
             expected <= u8::MAX as usize,
             "read_buffer_probe expected exceeds 1-byte CDB length field"
         );
         let cdb = self.read_buffer_sub(sub_cmd, address, expected as u8);
         let result = scsi.execute(&cdb, DataDirection::FromDevice, buf, 5_000)?;
-        // A drive sense arrives as `Ok` with a non-zero status (the transport
-        // contract), and a merely SHORT response is the drive answering badly —
-        // neither is a dead bus. Fabricating a transport-failure status here
-        // turned a short probe reply into a hard abort of a rip that would
-        // otherwise have succeeded; a real transport fault still propagates
-        // through the `?` above with its own status.
+        // A drive sense arrives as `Ok` with a non-zero status; a merely
+        // SHORT response is the drive answering badly — neither is a dead
+        // bus. See docs/mt1959.md — short-probe-not-transport-failure.
         if result.status != 0 || result.bytes_transferred != expected {
             return Err(Error::Scsi {
                 opcode: SCSI_READ_BUFFER,
@@ -183,13 +179,9 @@ impl Mt1959 {
             });
         }
 
-        // `response` is a fixed 64-byte buffer, so `response.len()` is
-        // always >= every offset below — the meaningful bound is how many
-        // bytes the drive actually delivered. Every marker check MUST be
-        // reachable: when the checks were each guarded by their own `n >= ..`
-        // the drive delivering ZERO bytes skipped all three and `do_unlock`
-        // returned `Ok` on a buffer the drive never wrote. Require enough bytes
-        // to actually validate, up front.
+        // `response` is a fixed 64-byte buffer; the meaningful bound is how
+        // many bytes the drive actually delivered. Require enough bytes to
+        // validate before checking markers, up front. See docs/mt1959.md — min-unlock-response-length.
         let n = result.bytes_transferred.min(response.len());
         if n < MIN_UNLOCK_RESPONSE {
             tracing::debug!(
@@ -212,15 +204,9 @@ impl Mt1959 {
             return Err(Error::UnlockFailed);
         }
 
-        // Extended-access state is active when BOTH the per-drive
-        // signature matched AND the response carries the secondary
-        // marker at offset 16 (repeated through bytes 16..64) AND the
-        // primary mode marker at [12..16] is present. The active-mode
-        // marker at [12..16] is the primary gate; the [16..20] marker
-        // is the redundant confirmation the firmware writes through
-        // the rest of the response. Requiring both before we tell the
-        // upper layer "OEM path is live" keeps any partial / corrupted
-        // response from steering us off the cert-auth fallback.
+        // Extended-access state requires the signature match AND both the
+        // primary marker at [12..16] AND the secondary marker at [16..20].
+        // See docs/mt1959.md — unlock-marker-gating.
         self.unlocked =
             response[FIRMWARE_MODE_OFFSET..FIRMWARE_MODE_OFFSET + 4] == FIRMWARE_MODE_SIG;
 
@@ -295,11 +281,8 @@ impl Mt1959 {
                     );
                     return Err(Error::UnlockFailed);
                 }
-                // A transport fault means the bus is gone. Re-uploading firmware
-                // into a dead bus three times accomplishes nothing and the
-                // generic UnlockFailed it used to end on is classified as
-                // "unlocker not applicable", so the consumer kept probing a dead
-                // drive instead of aborting. Surface the fault unchanged.
+                // A dead bus must abort here, not be retried three times as
+                // firmware reloads. See docs/mt1959.md — run-init-transport-fault.
                 Err(e) if e.is_transport_failure() => {
                     tracing::warn!(
                         target: "freemkv::disc",
@@ -365,10 +348,8 @@ impl Mt1959 {
             self.do_unlock(scsi)?;
         }
 
-        // Detect disc type from capacity to select probe mode.
-        // BD:  3C 01 44 12 01 00 00 00 04 00  (init_addr = 0x0100)
-        // UHD: 3C 01 44 12 02 00 00 00 04 00  (init_addr = 0x0200)
-        // Empirically verified via SCSI capture: BD and UHD use different init addresses.
+        // Detect disc type from capacity to select probe mode (BD vs UHD
+        // init address). See docs/mt1959.md — probe-init-address-detection.
         let cap_cdb = [
             SCSI_READ_CAPACITY,
             0x00,
@@ -382,16 +363,12 @@ impl Mt1959 {
             0x00,
         ];
         let mut cap_buf = [0u8; READ_CAPACITY_RESPONSE_SIZE];
-        // A transport fault here is a dead bus: propagate it (the caller
-        // classifies it as `Transport` and aborts) instead of silently
-        // defaulting the disc type. A drive sense (`Ok`, non-zero status) or a
-        // short reply just means "capacity unknown" -> assume BD.
+        // A transport fault here is a dead bus: propagate it. A drive
+        // sense or short reply just means "capacity unknown" -> assume BD.
         let cap = scsi.execute(&cap_cdb, DataDirection::FromDevice, &mut cap_buf, 5_000)?;
         let disc_sectors = if cap.status == 0 && cap.bytes_transferred >= 4 {
-            // last_lba + 1 = sector count. A 0xFFFFFFFF last-LBA is the
-            // READ CAPACITY(10) "capacity exceeds 32 bits" sentinel; saturate
-            // rather than wrap to 0 (which would misclassify a huge disc as
-            // BD). A saturated count stays above the UHD threshold -> UHD.
+            // last_lba + 1 = sector count; saturate on the 32-bit-overflow
+            // sentinel rather than wrap to 0. See docs/mt1959.md — capacity-saturation.
             u32::from_be_bytes([cap_buf[0], cap_buf[1], cap_buf[2], cap_buf[3]]).saturating_add(1)
         } else {
             0
@@ -434,14 +411,9 @@ impl Mt1959 {
                     );
                 })
             {
-                // A dead bus MUST keep its transport classification: this loop is
-                // reached from `probe_disc`, whose caller aborts the rip only when
-                // it sees `Transport`. Flattening every failure to `UnlockFailed`
-                // (which classifies as "not applicable") let a bus that died mid
-                // speed-probe fall through to `Ok(Unlocked{drive_unlocked:true})`.
-                // A short / rejected probe reply is still the drive's OWN failure —
-                // keep it as `UnlockFailed` so it doesn't hard-abort a rip that
-                // could continue on the default speed table.
+                // A dead bus MUST keep its transport classification; a
+                // short/rejected reply is the drive's own failure and stays
+                // `UnlockFailed`. See docs/mt1959.md — probe-failure-classification.
                 if e.is_transport_failure() {
                     return Err(e);
                 }
@@ -450,10 +422,8 @@ impl Mt1959 {
             addr = addr.wrapping_add(PROBE_STEP);
         }
 
-        // Pass 2: continue past the coarse range. It used to restart at 0 with
-        // the SAME PROBE_STEP, re-issuing all 88 pass-1 addresses verbatim — 88
-        // redundant SCSI round-trips per disc, which dominates drive prep at
-        // real bus latency and teaches the drive's speed table nothing new.
+        // Pass 2: continue past the coarse range (do not restart at 0).
+        // See docs/mt1959.md — probe-pass-overlap-fix.
         let mut addr: u32 = PROBE_COARSE_END as u32;
         while addr < PROBE_FINE_END {
             let mut resp = [0u8; PROBE_RESPONSE_SIZE as usize];
@@ -464,12 +434,9 @@ impl Mt1959 {
                 &mut resp,
                 PROBE_RESPONSE_SIZE as usize,
             ) {
-                // Fine probing is best-effort — a short / rejected reply just ends
-                // the sweep early. But a bus that DIED here must not be swallowed:
-                // a bare `break` returned `Ok(())` from `run_probe`, so a dead bus
-                // mid-pass-2 vanished into `Ok(Unlocked{drive_unlocked:true})` with
-                // no log at all (the flagship failure-that-looks-like-success). A
-                // transport fault keeps its classification and aborts.
+                // Fine probing is best-effort — a short/rejected reply just ends
+                // the sweep early, but a transport fault must keep its
+                // classification and abort. See docs/mt1959.md — probe-pass2-transport-fault.
                 if e.is_transport_failure() {
                     tracing::warn!(
                         target: "freemkv::disc",
@@ -619,10 +586,43 @@ mod tests {
         assert_eq!(len, 2208, "MODE SELECT length = firmware.len(), not 2496");
     }
 
-    /// variant_a had no direct test at all (variant_b did). Pins its upload
-    /// sequence: WRITE_BUFFER carrying the firmware at its real per-drive
-    /// length in the CDB's 24-bit length field, then the 0x45 verify READ_BUFFER
-    /// whose result used to be discarded outright.
+    // Byte-for-byte pin of variant_b's Step-2 metadata-read (READ_BUFFER offset
+    // 0x3000 at cdb[4]=0x30, length 0x10 at cdb[8]) and Step-3 write-extra CDBs.
+    #[test]
+    fn variant_b_meta_read_and_write_extra_cdbs_are_exact_bytes() {
+        let mut profile = fixture_profile([0x11, 0x22, 0x33, 0x44]);
+        profile.firmware = vec![0u8; 2208];
+        let mut mt = Mt1959::new(profile, true);
+        let mut t = RecordingTransport { cdbs: Vec::new() };
+        let _ = variant_b::load_firmware(&mut mt, &mut t);
+
+        // Step 2: READ_BUFFER mode 6, offset 0x3000, length 0x10.
+        let meta = t
+            .cdbs
+            .iter()
+            .find(|c| c.first() == Some(&SCSI_READ_BUFFER) && c.get(4) == Some(&0x30))
+            .expect("Step-2 metadata READ_BUFFER issued");
+        assert_eq!(
+            meta.as_slice(),
+            &[0x3C, 0x06, 0x00, 0x00, 0x30, 0x00, 0x00, 0x00, 0x10, 0x00]
+        );
+
+        // Step 3: WRITE_BUFFER of the 0x10-byte extra block. Distinguish from
+        // the Step-1 firmware upload (WRITE_BUFFER too) by its length byte.
+        let extra = t
+            .cdbs
+            .iter()
+            .find(|c| c.first() == Some(&SCSI_WRITE_BUFFER) && c.get(8) == Some(&0x10))
+            .expect("Step-3 write-extra WRITE_BUFFER issued");
+        assert_eq!(
+            extra.as_slice(),
+            &[0x3B, 0x06, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x10, 0x00]
+        );
+    }
+
+    // Pins variant_a's upload sequence: WRITE_BUFFER at the real per-drive
+    // length in the CDB's 24-bit length field, then the 0x45 verify
+    // READ_BUFFER whose result used to be discarded outright.
     #[test]
     fn variant_a_uploads_at_the_real_length_and_issues_the_verify_read() {
         let mut profile = fixture_profile([0x11, 0x22, 0x33, 0x44]);
@@ -648,6 +648,40 @@ mod tests {
         );
     }
 
+    // Byte-for-byte pin of variant_a's two firmware-upload CDBs (not just the
+    // length field): the WRITE_BUFFER (mode cdb[1]=0x06, reserved, 24-bit len,
+    // control byte 9) and the whole 0x45 verify READ_BUFFER CDB.
+    #[test]
+    fn variant_a_write_buffer_and_verify_cdbs_are_exact_bytes() {
+        let mut profile = fixture_profile([0x11, 0x22, 0x33, 0x44]);
+        profile.firmware = vec![0u8; 2208]; // 0x0008A0
+        let mut mt = Mt1959::new(profile, false);
+        let mut t = RecordingTransport { cdbs: Vec::new() };
+        let _ = variant_a::load_firmware(&mut mt, &mut t);
+
+        let wb = t
+            .cdbs
+            .iter()
+            .find(|c| c.first() == Some(&SCSI_WRITE_BUFFER))
+            .expect("WRITE_BUFFER issued");
+        // 3B [mode 06] [rsvd 00 00 00 00] [len 24-bit BE = 0008A0] [control 00]
+        assert_eq!(
+            wb.as_slice(),
+            &[0x3B, 0x06, 0x00, 0x00, 0x00, 0x00, 0x00, 0x08, 0xA0, 0x00]
+        );
+
+        let verify = t
+            .cdbs
+            .iter()
+            .find(|c| c.first() == Some(&SCSI_READ_BUFFER) && c.get(2) == Some(&0x45))
+            .expect("0x45 verify READ_BUFFER issued");
+        // 3C [MODE_A 01] [buffer_id 45] [rsvd ×5] [alloc len 04] [control 00]
+        assert_eq!(
+            verify.as_slice(),
+            &[0x3C, 0x01, 0x45, 0x00, 0x00, 0x00, 0x00, 0x00, 0x04, 0x00]
+        );
+    }
+
     /// An EMPTY firmware blob cannot be uploaded — catches a profile whose
     /// firmware failed to decode being pushed at the drive as a zero-length
     /// WRITE_BUFFER.
@@ -660,12 +694,9 @@ mod tests {
         assert!(t.cdbs.is_empty(), "no CDB may reach the drive");
     }
 
-    /// THE defect-15 test. A drive that returns ZERO bytes used to skip the
-    /// signature check, the firmware-active check AND the mode check — every
-    /// gate was individually guarded by its own `n >= ..` — so `do_unlock`
-    /// returned `Ok(vec![0u8; 64])` and (with the old `drive_unlocked: true`)
-    /// reported a fully unlocked drive. Catches restoring those per-check
-    /// guards.
+    // THE defect-15 test: a zero-byte response used to skip every marker
+    // check (each individually `n >= ..` guarded) and report a fully
+    // unlocked drive. See docs/mt1959.md — defect-15.
     #[test]
     fn do_unlock_rejects_a_zero_length_response() {
         let sig = [0x99, 0x9E, 0xC3, 0x75];
@@ -706,10 +737,9 @@ mod tests {
         assert!(!e.is_transport_failure(), "a sense is not a dead bus");
     }
 
-    /// THE defect-16 test (the inverse misclassification). A merely SHORT probe
-    /// response used to be fabricated into a transport-failure status, which
-    /// hard-aborts a rip that would otherwise have succeeded. Catches
-    /// reintroducing the fabricated 0xFF.
+    // THE defect-16 test: a merely short probe response used to be
+    // fabricated into a transport-failure status, hard-aborting a rip
+    // that would otherwise have succeeded.
     #[test]
     fn short_probe_response_is_not_a_transport_failure() {
         let mut t = crate::scsi::mock::MockTransport::always(crate::scsi::mock::Reply::short(
@@ -744,11 +774,9 @@ mod tests {
         assert!(e.is_transport_failure());
     }
 
-    /// THE defect-4 test. A dead bus must abort `run_init` at once. It used to
-    /// match `Err(_)` and re-upload firmware into the dead bus on all three
-    /// attempts, then return the generic `UnlockFailed`, which classifies as
-    /// `NotApplicable` — so the consumer kept probing. Catches removing the
-    /// `is_transport_failure` arm.
+    // THE defect-4 test: a dead bus must abort `run_init` at once, not
+    // re-upload firmware on all three attempts and return the generic
+    // `UnlockFailed`. See docs/mt1959.md — defect-4.
     #[test]
     fn run_init_aborts_on_a_transport_fault_without_reloading_firmware() {
         let mut t =
@@ -823,11 +851,9 @@ mod tests {
         }
     }
 
-    /// A drive SENSE (not a dead bus) during the coarse speed probe is the
-    /// drive's OWN failure, not a transport fault — `run_probe` must report
-    /// `UnlockFailed`, not `Transport`. The sibling of
-    /// `transport_fault_during_pass1_probe_stays_a_transport_failure`, which
-    /// only exercises the `is_transport_failure()` arm.
+    // A drive SENSE (not a dead bus) during the coarse probe is the
+    // drive's own failure — `run_probe` must report `UnlockFailed`, not
+    // `Transport`. Sibling of the pass-1 transport-fault test below.
     #[test]
     fn drive_sense_during_pass1_probe_is_unlock_failed_not_transport() {
         struct SenseAtFirstProbe;
@@ -866,12 +892,9 @@ mod tests {
         assert!(matches!(e, Error::UnlockFailed));
     }
 
-    /// THE probe pass-1 dead-bus test. A bus that dies during the coarse speed
-    /// probe must keep its transport classification out of `run_probe` so the
-    /// caller (`firmware_unlock`) aborts with `Transport` rather than reporting a
-    /// fully-unlocked drive.
-    /// MUTATION: flattening the pass-1 error to `Error::UnlockFailed` makes the
-    /// `is_transport_failure` assert go red (UnlockFailed maps to NotApplicable).
+    // THE probe pass-1 dead-bus test: a bus that dies during the coarse
+    // probe must keep its transport classification so the caller aborts
+    // with `Transport`. See docs/mt1959.md — pass1-dead-bus-mutation.
     #[test]
     fn transport_fault_during_pass1_probe_stays_a_transport_failure() {
         let mut t = ProbeFaultsTransport { fault_at: 0 };
@@ -884,12 +907,9 @@ mod tests {
         assert_eq!(crate::UnlockError::from(e), crate::UnlockError::Transport);
     }
 
-    /// THE probe pass-2 dead-bus test. Pass 1 completes; the bus dies in the fine
-    /// probe. The loop used to `break` on ANY error and return `Ok(())`, so a
-    /// dead bus mid-pass-2 vanished into `Ok`. It must now propagate a transport
-    /// fault (a short/rejected fine probe still ends the sweep benignly).
-    /// MUTATION: restoring the bare `break` makes `expect_err` panic (run_probe
-    /// returns Ok).
+    // THE probe pass-2 dead-bus test: the loop used to `break` on ANY
+    // error and return `Ok(())`, swallowing a dead bus mid-pass-2. It
+    // must now propagate. See docs/mt1959.md — pass2-dead-bus-mutation.
     #[test]
     fn transport_fault_during_pass2_probe_stays_a_transport_failure() {
         let mut t = ProbeFaultsTransport {
@@ -904,10 +924,8 @@ mod tests {
         assert_eq!(crate::UnlockError::from(e), crate::UnlockError::Transport);
     }
 
-    /// `validate`'s transport-abort branch had no test. A dead bus must propagate
-    /// as a transport fault on the FIRST attempt, not be retried five times.
-    /// MUTATION: turning the `Err(e) => return Err(Error::from(e))` arm into a
-    /// `continue` makes this exhaust to `UnlockFailed` and the assert go red.
+    // `validate`'s transport-abort branch: a dead bus must propagate on
+    // the first attempt, not be retried five times.
     #[test]
     fn validate_propagates_a_transport_fault_without_retrying() {
         let mut t =
@@ -932,13 +950,9 @@ mod tests {
         assert_eq!(t.calls(), 5, "retries the full five attempts");
     }
 
-    /// variant_a's firmware-verify-read transport-abort branch (~variant_a.rs:97)
-    /// had no test. WRITE_BUFFER succeeds, the 0x45 verify read faults: a dead bus
-    /// must abort THERE, not be swallowed into the unlock retries.
-    /// MUTATION: dropping the `if err.is_transport_failure() { return Err }` lets
-    /// execution fall through to `do_unlock` (scripted to answer, so it fails on
-    /// the signature as `UnlockFailed`), and the `is_transport_failure` assert
-    /// goes red — a genuine red-before-green, not green-both-ways.
+    // variant_a's firmware-verify-read transport-abort branch: WRITE_BUFFER
+    // succeeds, the 0x45 verify read faults — a dead bus must abort there,
+    // not be swallowed into the unlock retries. See docs/mt1959.md — variant-a-verify-read-transport-abort.
     #[test]
     fn variant_a_verify_read_transport_fault_aborts() {
         let mut profile = fixture_profile([0; 4]);
@@ -974,14 +988,9 @@ mod tests {
         assert!(t.cdbs.is_empty(), "no CDB may reach the drive");
     }
 
-    /// THE full success path: WRITE_BUFFER upload, a fully-good verify read
-    /// (exercising the `Ok(r) if status==0 && bytes_transferred==len` arm no
-    /// other variant_a test reaches), then BOTH `do_unlock` calls succeed —
-    /// the first fatally-gated, the second best-effort. Every other
-    /// variant_a test above deliberately stops short (empty firmware, a
-    /// dead bus, or `do_unlock` failing against `RecordingTransport`'s empty
-    /// response) — this is the only one that reaches `load_firmware`'s final
-    /// `Ok(())`.
+    // THE full success path: WRITE_BUFFER upload, a fully-good verify
+    // read, then both `do_unlock` calls succeed. The only variant_a test
+    // that reaches `load_firmware`'s final `Ok(())`.
     #[test]
     fn variant_a_load_firmware_succeeds_end_to_end() {
         let sig = [0x11, 0x22, 0x33, 0x44];
@@ -1002,12 +1011,9 @@ mod tests {
         assert!(mt.init_complete, "do_unlock #1 must have set init_complete");
     }
 
-    /// variant_b had no transport-abort test. A firmware-upload STEP that hits a
-    /// dead bus (here the metadata read) must abort via `trace_step`, not be
-    /// swallowed and carried into the unlock retries.
-    /// MUTATION: dropping `trace_step`'s `if err.is_transport_failure()` return
-    /// lets execution reach `do_unlock` (scripted good → signature fails as
-    /// `UnlockFailed`), so the `is_transport_failure` assert goes red.
+    // variant_b's firmware-upload step (here the metadata read) hitting a
+    // dead bus must abort via `trace_step`, not be swallowed into the
+    // unlock retries. See docs/mt1959.md — variant-b-upload-step-transport-abort.
     #[test]
     fn variant_b_upload_step_transport_fault_aborts() {
         let mut profile = fixture_profile([0; 4]);
@@ -1056,12 +1062,9 @@ mod tests {
         }
     }
 
-    /// Build a synthetic 64-byte unlock response.
-    ///
-    /// `mode_marker`: bytes [12..16]. Pass `FIRMWARE_ACTIVE_SIG` for the
-    /// active-mode primary marker.
-    /// `id_marker`:   bytes [16..20] (and repeated through [20..64] in
-    /// real responses; only [16..20] is checked).
+    // Build a synthetic 64-byte unlock response. `mode_marker` fills
+    // bytes [12..16]; `id_marker` fills bytes [16..20] (repeated through
+    // [20..64] in real responses; only [16..20] is checked).
     fn build_response(signature: [u8; 4], mode_marker: [u8; 4], id_marker: [u8; 4]) -> Vec<u8> {
         let mut r = vec![0u8; 64];
         r[0..4].copy_from_slice(&signature);
@@ -1144,11 +1147,9 @@ mod tests {
         assert_eq!(t.calls(), 1, "must not retry a signature mismatch");
     }
 
-    /// `run_init`'s retry loop: `do_unlock` fails generically (too short to
-    /// validate — not a signature mismatch, not a transport fault), the
-    /// firmware reload it falls back to also fails generically (an empty
-    /// firmware blob) rather than fatally, so the loop must `continue` and
-    /// try again — three times — before exhausting to the last error.
+    // `run_init`'s retry loop: a generic `do_unlock` failure plus a
+    // generic firmware-reload failure must `continue` and retry three
+    // times before exhausting to the last error.
     #[test]
     fn run_init_exhausts_after_three_generic_failures() {
         let mut t = crate::scsi::mock::MockTransport::always(crate::scsi::mock::Reply::short(
@@ -1168,11 +1169,9 @@ mod tests {
         );
     }
 
-    /// Same shape as `run_init_exhausts_after_three_generic_failures`, but for
-    /// a MODE_B (`is_variant_b = true`) drive — pins that `run_init`'s mode
-    /// dispatch (`self.mode == MODE_A`) actually reaches `variant_b::load_firmware`
-    /// and not just its `variant_a` sibling, which every other `run_init` test
-    /// above exercises exclusively.
+    // Same shape as the mode-A generic-failure test, but for a MODE_B
+    // drive — pins that `run_init`'s dispatch actually reaches
+    // `variant_b::load_firmware`, not just its variant_a sibling.
     #[test]
     fn run_init_exhausts_after_three_generic_failures_mode_b() {
         let mut t = crate::scsi::mock::MockTransport::always(crate::scsi::mock::Reply::short(
@@ -1190,10 +1189,9 @@ mod tests {
         );
     }
 
-    /// Same generic `do_unlock` failure, but this time the firmware-reload
-    /// fallback itself hits a dead bus. That must abort `run_init`
-    /// immediately (transport classification preserved), not be folded into
-    /// `last_err` and retried.
+    // Same generic `do_unlock` failure, but the firmware-reload fallback
+    // itself hits a dead bus — must abort `run_init` immediately, not be
+    // folded into `last_err` and retried.
     #[test]
     fn run_init_firmware_reload_transport_fault_aborts() {
         let mut profile = fixture_profile([0; 4]);
@@ -1232,10 +1230,9 @@ mod tests {
         assert!(mt.init_complete, "run_probe must do_unlock itself first");
     }
 
-    /// A READ CAPACITY drive sense (status != 0) means "capacity unknown" ->
-    /// `disc_sectors` falls back to 0, which is still below the UHD
-    /// threshold, so probing continues with the BD init address rather than
-    /// aborting.
+    // A READ CAPACITY drive sense means "capacity unknown" -> falls back
+    // to 0 sectors, which is below the UHD threshold, so probing
+    // continues with the BD init address rather than aborting.
     #[test]
     fn run_probe_capacity_check_condition_falls_back_to_zero_sectors() {
         struct CapacitySenseTransport;
@@ -1327,10 +1324,9 @@ mod tests {
         );
     }
 
-    /// The fine (pass-2) probe sweep ends early on a merely-rejected probe
-    /// reply (not a transport fault) — it must `break` out of the sweep and
-    /// let `run_probe` still return `Ok`, rather than propagating the drive's
-    /// refusal as a hard failure.
+    // The fine (pass-2) probe sweep ends early on a merely-rejected
+    // probe reply (not a transport fault) — it must `break` and let
+    // `run_probe` still return `Ok`.
     #[test]
     fn run_probe_pass2_ends_early_on_a_rejected_probe_without_failing() {
         struct Pass2RejectsTransport {

src/ld/platform/mt1959/variant_a.rs

@@ -21,9 +21,8 @@ pub(super) fn load_firmware(mt: &mut Mt1959, scsi: &mut dyn ScsiTransport) -> Re
     }
 
     // Upload firmware via WRITE_BUFFER. The CDB's length is a 24-bit field;
-    // if the blob exceeds that, the encoded length would silently disagree
-    // with the bytes actually sent (`data`). Reject rather than upload a
-    // length-mismatched command.
+    // reject blobs that would exceed it rather than send a length-mismatched
+    // command.
     let len = firmware.len();
     if len > WRITE_BUFFER_MAX_LEN {
         return Err(crate::ld::error::Error::UnlockFailed);
@@ -43,12 +42,9 @@ pub(super) fn load_firmware(mt: &mut Mt1959, scsi: &mut dyn ScsiTransport) -> Re
     let mut data = firmware.clone();
     scsi.execute(&cdb, DataDirection::ToDevice, &mut data, 30_000)?;
 
-    // Verify the firmware loaded (non-fatal — different buffer_id 0x45). There
-    // is no documented expected payload to compare against, so the outcome is
-    // TRACED rather than asserted: discarding it with `let _ =` meant a corrupt
-    // upload proceeded straight into the do_unlock retries with no diagnostic
-    // whatsoever, which is exactly the silent-failure shape this crate exists to
-    // avoid.
+    // Verify the firmware loaded (non-fatal — different buffer_id 0x45). No
+    // documented expected payload exists, so the outcome is traced rather
+    // than asserted, instead of silently discarded before the unlock retries.
     let verify_cdb = [
         super::SCSI_READ_BUFFER,
         super::MODE_A,
@@ -100,10 +96,9 @@ pub(super) fn load_firmware(mt: &mut Mt1959, scsi: &mut dyn ScsiTransport) -> Re
         }
     }
 
-    // Double unlock after firmware upload. The first establishes the
-    // unlock and is fatal on failure; the second is a confirmation pass and
-    // is best-effort (matching variant B), so a benign hiccup on the
-    // redundant call doesn't fail an already-successful unlock.
+    // Double unlock after firmware upload: first is fatal on failure, second
+    // is a best-effort confirmation pass (matching variant B) so a benign
+    // hiccup there doesn't fail an already-successful unlock.
     mt.do_unlock(scsi)?;
     let _ = mt.do_unlock(scsi);
     Ok(())

src/ld/platform/mt1959/variant_b.rs

@@ -13,11 +13,9 @@ const FIRMWARE_EXTRA: [u8; 16] = [0; 16];
 /// that drive — real profiles must supply their own (see `DriveProfile`).
 const VENDOR_VERIFY: [u8; 10] = [0xF1, 0x01, 0x02, 0x00, 0x0D, 0x30, 0x01, 0xF3, 0xAD, 0x23];
 
-/// Trace the outcome of a best-effort firmware-upload step and swallow
-/// everything EXCEPT a transport fault. These steps are advisory (the unlock
-/// retries below are the real gate) but their results were previously thrown
-/// away entirely, so a firmware upload could fail end to end and leave nothing
-/// in the log to say so — and a dead bus kept the sequence running.
+// See docs/variant-b-firmware-upload.md — trace_step rationale.
+// Swallows everything except a transport fault; these steps are advisory,
+// the unlock retries below are the real gate.
 fn trace_step(phase: &'static str, r: crate::scsi::Result<crate::scsi::ScsiResult>) -> Result<()> {
     match r {
         Ok(res) if res.status == 0 => {
@@ -60,12 +58,9 @@ pub(super) fn load_firmware(mt: &mut Mt1959, scsi: &mut dyn ScsiTransport) -> Re
         return Err(crate::ld::error::Error::UnlockFailed);
     }
 
-    // Step 1: Upload the firmware via MODE SELECT. The profile's `firmware` is
-    // the exact per-drive image — extracted at the drive's own load-CDB length
-    // (2192..2528 bytes; the old fixed 0x9C0 truncated some drives and over-read
-    // others into blob strings). Upload all of it. MODE SELECT(10)'s
-    // parameter-list length is 16-bit, so reject only a blob that can't be
-    // expressed in the CDB.
+    // Step 1: Upload the firmware via MODE SELECT (see docs/variant-b-firmware-upload.md).
+    // MODE SELECT(10)'s parameter-list length is 16-bit, so reject only a
+    // blob that can't be expressed in the CDB.
     let write_len = firmware.len();
     if write_len > u16::MAX as usize {
         return Err(crate::ld::error::Error::UnlockFailed);
@@ -128,13 +123,9 @@ pub(super) fn load_firmware(mt: &mut Mt1959, scsi: &mut dyn ScsiTransport) -> Re
         scsi.execute(&write_extra_cdb, DataDirection::ToDevice, &mut data2, 5_000),
     )?;
 
-    // Step 4: Vendor verify (0xF1 — B-only, not standard SCSI). PER-DRIVE: take
-    // it from the profile (39 distinct values across the 140 B drives). The
-    // const is only a legacy fallback — it carries one drive's token.
-    // The result used to be discarded with `let _ =` despite the comment calling
-    // this a verify step: a corrupt upload proceeded straight to the do_unlock
-    // retries with no diagnostic. There is no documented expected payload, so
-    // the outcome is TRACED; only a dead bus aborts.
+    // Step 4: Vendor verify (0xF1 — B-only, not standard SCSI), per-drive.
+    // See docs/variant-b-firmware-upload.md for why the outcome is traced
+    // rather than discarded.
     let verify_cdb = mt.profile.fw_verify_cdb.unwrap_or(VENDOR_VERIFY);
     let mut dummy = [0u8; 0];
     trace_step(
@@ -142,11 +133,9 @@ pub(super) fn load_firmware(mt: &mut Mt1959, scsi: &mut dyn ScsiTransport) -> Re
         scsi.execute(&verify_cdb, DataDirection::None, &mut dummy, 5_000),
     )?;
 
-    // Step 5: Unlock retries (up to 5, then a final fatal attempt). On a
-    // successful unlock we issue one confirmation pass; its result is
-    // intentionally best-effort — the first call already established the
-    // unlock state, so a hiccup on the redundant confirmation must not fail
-    // an otherwise-good unlock.
+    // Step 5: Unlock retries (up to 5, then a final fatal attempt). The
+    // confirmation pass after success is intentionally best-effort — see
+    // docs/variant-b-firmware-upload.md.
     for _attempt in 0..5 {
         match mt.do_unlock(scsi) {
             Ok(_) => {

src/ld/profile.rs

@@ -3,7 +3,7 @@
 use crate::ld::error::{Error, Result};
 use serde::Deserialize;
 
-/// The Mt1959Unlocker profile catalog — the set of optical drives the firmware
+/// The LdUnlocker profile catalog — the set of optical drives the firmware
 /// unlocker recognizes, keyed by chipset + variant. Loaded from the bundled
 /// JSON; the public entry point is [`crate::ld::profiles`].
 #[derive(Debug, Deserialize)]
@@ -42,20 +42,10 @@ pub struct Identity {
 
 /// Per-drive profile.
 ///
-/// VISIBILITY: only `identity` and `signature` are public — they are what the
-/// catalog is public FOR ("is this drive supported?", and the emulator's
-/// impersonation). Everything below them is unlock MECHANISM: the MediaTek
-/// firmware image and the per-drive vendor CDB templates. `Cargo.toml` says this
-/// crate is never published because it carries drive firmware; re-exporting
-/// `DriveProfile` with a `pub firmware: Vec<u8>` handed that firmware, and every
-/// vendor CDB, to any downstream crate that called `ld::profile()`. They are
-/// `pub(crate)` so the mechanism stays inside the unlocker.
-///
-/// `allow(dead_code)`: this is the on-disk catalog schema, and several captured
-/// CDB templates have no in-crate consumer yet. They were `pub` before, so the
-/// compiler saw the (external) use; making them `pub(crate)` is what exposes
-/// them as unread. Deleting them would silently drop fields from the format the
-/// profile-extraction pipeline emits.
+/// Only `identity` and `signature` are public; everything else (firmware
+/// image, per-drive vendor CDB templates) is `pub(crate)` unlock mechanism
+/// that must stay inside this unpublished crate.
+/// See docs/drive-profile-visibility.md — field-visibility and allow(dead_code) rationale.
 #[allow(dead_code)]
 #[derive(Clone, Deserialize)]
 pub struct DriveProfile {
@@ -72,11 +62,8 @@ pub struct DriveProfile {
     pub(crate) firmware: Vec<u8>,
 
     // ── OEM-extended-access CDB templates ──────────────────────────────
-    //
-    // All optional — older profile blobs that pre-date the CDB capture
-    // pipeline simply omit these fields and decode as `None`. Encoded
-    // in the JSON as lowercase hex strings without separators
-    // (e.g. `"3c014410e29100002400"` for a 10-byte CDB).
+    // All optional (`None` if pre-capture-pipeline). Hex strings, no
+    // separators, e.g. `"3c014410e29100002400"`.
     #[serde(default)]
     pub(crate) unlock_init_value: u8,
     #[serde(default)]
@@ -113,14 +100,9 @@ pub struct DriveProfile {
     pub(crate) speed_calc_table: Option<Vec<u8>>,
 }
 
-// Hand-written, REDACTING Debug. `DriveProfile` is `pub` and reachable through
-// the public `Profiles` catalog (`ld::bundled` / `ld::profiles`), whose own
-// derived `Debug` would recurse into this one. `Cargo.toml` says this crate is
-// NEVER published because it carries drive firmware; a derived `Debug` would
-// render that firmware image AND every captured per-drive vendor CDB (the unlock
-// MECHANISM) straight into any log. Show only the PUBLIC identity + signature
-// (what the catalog is public FOR); collapse the firmware to a byte length and
-// omit the vendor CDB templates entirely so no mechanism bytes reach a log.
+// Hand-written, REDACTING Debug: a derived one would recurse through the
+// public `Profiles` catalog and print raw firmware + vendor CDB bytes into
+// logs. Show only identity + signature; collapse firmware to a length.
 impl std::fmt::Debug for DriveProfile {
     fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
         f.debug_struct("DriveProfile")
@@ -167,23 +149,16 @@ pub struct ProfileMatch {
     pub platform: Platform,
 }
 
-// ── Parsing ────────────────────────────────────────────────────────────
-
-/// Decode an even-length ASCII hex string into bytes.
-///
-/// Operates on raw bytes rather than `&str` char-boundary slices: a
-/// non-ASCII input (e.g. a hand-edited profile with a multi-byte char)
-/// could otherwise have `&s[i..i+2]` land inside a UTF-8 char boundary and
-/// panic. Hex is ASCII, so any non-ASCII or non-hex byte simply fails to
-/// decode. The error is a stable, language-neutral token (`"hex"`), not a
-/// translatable English message.
+// ── Parsing: decode hex on raw bytes (not `&str` slices) so non-ASCII
+// can't land `&s[i..i+2]` inside a UTF-8 char boundary and panic — it
+// just fails to decode with `"hex"`.
 fn decode_hex(s: &str) -> std::result::Result<Vec<u8>, &'static str> {
     let bytes = s.as_bytes();
     if !bytes.len().is_multiple_of(2) {
         return Err("hex");
     }
     let mut out = Vec::with_capacity(bytes.len() / 2);
-    for pair in bytes.chunks_exact(2) {
+    for pair in bytes.as_chunks::<2>().0.iter() {
         let hi = (pair[0] as char).to_digit(16).ok_or("hex")?;
         let lo = (pair[1] as char).to_digit(16).ok_or("hex")?;
         out.push((hi * 16 + lo) as u8);
@@ -223,9 +198,7 @@ where
 }
 
 // ── Fixed-length hex deserializers for CDB templates ────────────────────
-//
-// Profile JSON encodes CDBs as lowercase hex strings without separators.
-// An empty string / null / missing field decodes as `None`.
+// Lowercase hex strings, no separators; empty/null/missing decodes as `None`.
 
 fn parse_hex_bytes(s: &str) -> std::result::Result<Vec<u8>, &'static str> {
     decode_hex(s)
@@ -306,18 +279,14 @@ pub fn bundled() -> Option<&'static Profiles> {
     CACHE
         .get_or_init(|| match load_from_str(BUNDLED_PROFILES) {
             Ok(p) => Some(p),
-            // The `None` is cached PERMANENTLY (correctly — the blob is fixed at
-            // compile time), which means a bad shipped profiles.json makes
-            // Mt1959Unlocker report "drive not supported" for every drive for the
-            // life of the process. Unlogged, that is indistinguishable from a
-            // genuinely uncataloged drive; log it once, loudly, at the one place
-            // the parse happens.
+            // `None` caches permanently; unlogged that's indistinguishable
+            // from a genuinely uncataloged drive, so log once, loudly, here.
             Err(e) => {
                 tracing::error!(
                     target: "freemkv::disc",
                     phase = "bundled_profiles_parse_failed",
                     error_code = e.code(),
-                    "bundled Mt1959Unlocker profile catalog failed to parse; no drive can match"
+                    "bundled LdUnlocker profile catalog failed to parse; no drive can match"
                 );
                 None
             }
@@ -338,15 +307,14 @@ fn load_from_str(data: &str) -> Result<Profiles> {
     serde_json::from_str(data).map_err(|_| Error::ProfileParse)
 }
 
-/// Find a profile matching a drive's INQUIRY fields.
+/// Find a profile matching a drive's INQUIRY fields, whitespace-trimmed.
 ///
-/// Per platform section (MT1959-A, then MT1959-B, then Renesas), tried in
-/// order of decreasing specificity: full match including `product_id` and
-/// `firmware_date` (skipped when the drive reports no product), then the same
-/// without `product_id`. Both require `firmware_date`; every profile carries a
-/// unique identity under these fields, so a match is exact — no looser
-/// vendor/revision fallback that could pick the wrong same-model variant. All
-/// comparisons are whitespace-trimmed. Returns the first section that matches.
+/// Per platform section (MT1959-A, -B, Renesas): (1) exact match including
+/// `product_id` when the drive reports one; (2) product-id-blind fallback on
+/// the four-field identity (vendor/revision/vendor_specific/firmware_date).
+/// Catalogs store a GENERIC product_id (e.g. "BD-RE") vs a drive's specific one
+/// ("BD-RE BU40N"), so pass 1 misses; pass 2 binds a UNIQUE four-field match —
+/// a shared four-tuple binds only when the drive reported no product_id.
 pub fn find_by_drive_id(profiles: &Profiles, drive_id: &crate::DriveId) -> Option<ProfileMatch> {
     let v = drive_id.vendor_id.trim();
     let prod = drive_id.product_id.trim();
@@ -374,16 +342,38 @@ pub fn find_by_drive_id(profiles: &Profiles, drive_id: &crate::DriveId) -> Optio
             });
         }
 
-        if let Some(p) = list.iter().find(|p| {
-            p.identity.vendor_id.trim() == v
-                && p.identity.product_revision.trim() == r
-                && p.identity.vendor_specific.trim() == vs
-                && p.identity.firmware_date.trim() == date
-        }) {
-            return Some(ProfileMatch {
-                profile: p.clone(),
-                platform,
-            });
+        // Product-id-blind fallback: catalogs store a GENERIC product_id while
+        // drives report a specific one, so the exact pass misses. Match the
+        // four-field identity instead; guard mis-binding on shared tuples below.
+        let four_field: Vec<&DriveProfile> = list
+            .iter()
+            .filter(|p| {
+                p.identity.vendor_id.trim() == v
+                    && p.identity.product_revision.trim() == r
+                    && p.identity.vendor_specific.trim() == vs
+                    && p.identity.firmware_date.trim() == date
+            })
+            .collect();
+        match four_field.as_slice() {
+            // Exactly one four-field match — unambiguous; bind regardless of
+            // the reported product_id (the common real-drive case, and the
+            // pre-1.7.0 behaviour a UHD-capable LG BU40N relied on).
+            [only] => {
+                return Some(ProfileMatch {
+                    profile: (*only).clone(),
+                    platform,
+                });
+            }
+            // Shared four-tuple: bind only when the drive reported NO
+            // product_id. A non-empty one that missed the exact pass is an
+            // uncataloged variant and must not mis-bind to a sibling.
+            [first, ..] if prod.is_empty() => {
+                return Some(ProfileMatch {
+                    profile: (*first).clone(),
+                    platform,
+                });
+            }
+            _ => {}
         }
     }
 
@@ -441,6 +431,47 @@ mod tests {
         let id0 = make_drive_id("TIE", "1.00", "XX00000", "200001010000");
         let m0 = find_by_drive_id(&profiles, &id0).unwrap();
         assert_eq!(m0.profile.signature, [0xaa, 0xaa, 0xaa, 0xaa]);
+
+        // A REPORTED product_id that matches no cataloged entry must NOT fall
+        // through to the four-field pass and bind to a sibling (MODEL-A/B);
+        // an uncataloged variant is no match, not a wrong match.
+        let mut idc = make_drive_id("TIE", "1.00", "XX00000", "200001010000");
+        idc.product_id = "MODEL-C".to_string();
+        assert!(
+            find_by_drive_id(&profiles, &idc).is_none(),
+            "an uncataloged product_id must not mis-bind to a same-vendor sibling",
+        );
+    }
+
+    /// REGRESSION (1.7.0): a real drive reports a SPECIFIC product_id (e.g.
+    /// "BD-RE BU40N") while the catalog stores a GENERIC one ("BD-RE"), so the
+    /// exact pass misses. When the four-field identity is UNIQUE it must still
+    /// bind — the pre-1.7.0 behaviour a UHD LG BU40N relies on for LibreDrive
+    /// unlock. 1.7.0 gated the four-field pass behind an empty product_id, so it
+    /// silently stopped matching and the firmware unlocker was skipped entirely.
+    #[test]
+    fn find_by_drive_id_specific_product_id_binds_unique_four_field() {
+        use serde_json::json;
+        let profiles: Profiles = serde_json::from_str(
+            &json!({
+                "mt1959_a": [
+                    {"identity": {"vendor_id":"HL-DT-ST","product_id":"BD-RE",
+                        "product_revision":"1.03","vendor_specific":"NM00000",
+                        "firmware_date":"211810241934"},
+                        "signature":"12345678","firmware":""}
+                ]
+            })
+            .to_string(),
+        )
+        .unwrap();
+
+        // Drive reports the specific marketing product_id; catalog has "BD-RE".
+        let mut id = make_drive_id("HL-DT-ST", "1.03", "NM00000", "211810241934");
+        id.product_id = "BD-RE BU40N".to_string();
+        let m = find_by_drive_id(&profiles, &id)
+            .expect("unique four-field identity must match despite the product_id mismatch");
+        assert_eq!(m.profile.signature, [0x12, 0x34, 0x56, 0x78]);
+        assert_eq!(m.platform, Platform::Mt1959A);
     }
 
     #[test]
@@ -543,10 +574,8 @@ mod tests {
         assert_eq!(parse_hex4("aabbccdd").unwrap(), [0xaa, 0xbb, 0xcc, 0xdd]);
     }
 
-    /// Platform::name() returns stable, non-empty, language-neutral identifiers.
-    /// These strings are logged and keyed on in caller code; changing them is a
-    /// breaking change.
-    /// Mutation: swapping Mt1959A and Mt1959B names silently misroutes firmware upload.
+    // Platform::name() strings are logged/keyed by callers; changing them is
+    // a breaking change. Mutation: swapping A/B names misroutes firmware upload.
     #[test]
     fn platform_name_is_stable() {
         // The exact strings are part of the public stable API (logged/keyed).
@@ -555,11 +584,9 @@ mod tests {
         assert_eq!(Platform::Renesas.name(), "Renesas");
     }
 
-    /// find_by_drive_id: exact match (including firmware_date) wins over loose match.
-    /// Spec: two-pass — first an exact match including firmware_date, then looser.
-    /// Build two synthetic Profiles entries that differ only by firmware_date,
-    /// and verify the correct one is selected.
-    /// Mutation: doing only the loose pass would return the first entry regardless of date.
+    // find_by_drive_id: two-pass — exact match (incl. firmware_date) wins
+    // over loose. Mutation: skipping the exact pass returns the first entry
+    // regardless of date.
     #[test]
     fn find_by_drive_id_exact_date_wins_over_loose() {
         use serde_json::json;
@@ -651,10 +678,8 @@ mod tests {
         assert!(matches!(result, Err(Error::ProfileParse)));
     }
 
-    /// Bundled profiles is non-empty (mt1959_a has at least one entry).
-    /// This pins the embedded JSON: if profiles.json is accidentally emptied
-    /// or truncated, this test goes red.
-    /// Mutation: clearing profiles.json would make this fail.
+    // Pins the embedded JSON: if profiles.json is emptied/truncated, this
+    // goes red.
     #[test]
     fn bundled_profiles_has_entries() {
         let profiles = load_bundled().unwrap();
@@ -664,10 +689,8 @@ mod tests {
         );
     }
 
-    /// deserialization of a profile with missing optional CDB fields
-    /// produces None for those fields (not a parse error).
-    /// Spec: all CDB template fields are `#[serde(default)]` — they are optional.
-    /// Mutation: making a CDB field required breaks backward-compat with old blobs.
+    // All CDB fields are `#[serde(default)]`. Mutation: making one required
+    // breaks backward-compat with old blobs.
     #[test]
     fn profile_optional_cdb_fields_default_to_none() {
         use serde_json::json;
@@ -715,12 +738,8 @@ mod tests {
         );
     }
 
-    /// `DriveProfile`'s `Debug` must NOT render the firmware image or the
-    /// per-drive vendor CDB templates — this crate is unpublished precisely
-    /// because it carries drive firmware, and the profile is reachable through
-    /// the public `Profiles` catalog.
-    /// MUTATION: restoring `#[derive(Debug)]` prints the raw firmware bytes
-    /// (e.g. the 0xEE marker below) and the CDB bytes, so this goes red.
+    // `Debug` must not render firmware/CDB bytes. Mutation: restoring
+    // `#[derive(Debug)]` prints them (e.g. the 0xEE marker below), goes red.
     #[test]
     fn drive_profile_debug_redacts_firmware_and_cdbs() {
         use serde_json::json;
@@ -749,10 +768,8 @@ mod tests {
         assert!(s.contains("VIS"), "identity must remain visible: {s}");
     }
 
-    /// deserialize_hex4 of an empty string must produce [0;4] without error.
-    /// This matches `deserialize_hex4`'s explicit early-return for empty strings.
-    /// Mutation: treating empty string as an error prevents profiles where signature
-    ///           was not captured from loading.
+    // deserialize_hex4("") must produce [0;4]. Mutation: treating empty as
+    // an error blocks profiles with no captured signature from loading.
     #[test]
     fn profile_empty_signature_deserialises_as_zeroes() {
         use serde_json::json;

src/lib.rs

@@ -1,33 +1,44 @@
 //! freemkv-unlock — the unlock layer for the freemkv toolchain.
 //!
 //! An **unlocker removes a drive-level bus-encryption barrier** so the drive
-//! serves readable (de-bus'd / de-scrambled) sectors. Content-key decryption is
-//! a separate layer — the consumer's (libfreemkv's) job.
+//! serves readable (de-bus'd / de-scrambled) sectors. Content-key decryption
+//! is a separate layer — the consumer's (libfreemkv's) job.
 //!
-//! This crate defines the [`Unlocker`] contract + the SCSI transport contract,
-//! and holds the self-contained unlocker modules (firmware / AACS cert / CSS).
-//! libfreemkv depends on this crate and dispatches via [`all_unlockers`]; it
-//! never names an individual unlocker. To remove an unlocker, delete its module
-//! dir and its one line in [`all_unlockers`] — nothing else changes.
+//! This crate defines the [`Unlocker`] contract + the SCSI transport
+//! contract, and holds the self-contained unlocker modules (firmware / AACS
+//! cert / CSS). The consumer (libfreemkv) assembles its own dispatch list from
+//! the exported unlocker types and drives them all through the same trait.
 
 pub mod scsi;
 
 mod aacs;
 mod css;
-// `ld` is public ONLY for its drive-profile catalog (`ld::profiles` / the
-// `Profiles` object) and, under the `emulation` feature, the unlock-handshake
-// wire format the bdemu test-emulator needs. The unlocker impl itself
-// (`Mt1959Unlocker`) is `pub(crate)` — clients still reach unlockers only through
-// [`all_unlockers`]. `aacs` and `css` carry no such public catalog, so they
-// stay fully private.
+// Shared best-effort AACS Volume ID read, used by every route that opens the
+// drive to raw reads (freemkv / MT1959 / Renesas). See `vid`.
+mod vid;
+// `ld` is public only for its drive-profile catalog + (under `emulation`) the
+// handshake wire format bdemu needs; the unlocker impl stays `pub(crate)`.
+// See docs/module-visibility.md — module visibility rationale.
 pub mod ld;
-// `renesis` is public for its `is_renesas` drive-probe (which reports a dead
-// bus as `Err(UnlockError::Transport)` rather than "not a Renesas drive"); the unlocker impl
-// (`Renesis`) is `pub(crate)` — reached only through [`all_unlockers`].
-pub mod renesis;
+// `renesas` is public for its `is_renesas` drive-probe (dead bus vs. "not a
+// Renesas drive"); the unlocker impl stays `pub(crate)`.
+// See docs/module-visibility.md — module visibility rationale.
+pub mod renesas;
+// `freemkv` self-identifies rather than matching a bundled profile, so it
+// stays fully private. See docs/module-visibility.md — module visibility rationale.
+mod freemkv;
 
 use scsi::ScsiTransport;
 
+// The five unlockers, exposed as concrete types so the consumer assembles its
+// own dispatch list and injects each one's deps at construction (certs → AACS)
+// — no central factory to thread another unlocker's config through.
+pub use aacs::AacsUnlocker;
+pub use css::DvdUnlocker;
+pub use freemkv::FreemkvUnlocker;
+pub use ld::LdUnlocker;
+pub use renesas::Renesas;
+
 /// Drive identity an unlocker matches against — four raw INQUIRY-derived fields,
 /// filled by the consumer (this crate parses no INQUIRY itself).
 #[derive(Debug, Clone, Default)]
@@ -50,7 +61,7 @@ pub enum DiscKind {
 
 /// A host certificate for the AACS cert handshake (raw; the consumer collects
 /// these from its key sources and passes them in).
-#[derive(Debug, Clone)]
+#[derive(Clone)]
 pub struct HostCert {
     /// AACS 1.0 host private key (20 bytes).
     pub private_key: [u8; 20],
@@ -62,47 +73,55 @@ pub struct HostCert {
     pub certificate_v2: Option<Vec<u8>>,
 }
 
-/// Context handed to an unlocker: drive identity, disc kind, and (for the cert
-/// route) the host certs the consumer collected.
-pub struct UnlockCtx<'a> {
-    pub drive_id: &'a DriveId,
-    pub kind: DiscKind,
-    pub host_certs: &'a [HostCert],
-}
-
-impl<'a> UnlockCtx<'a> {
-    pub fn new(drive_id: &'a DriveId, kind: DiscKind, host_certs: &'a [HostCert]) -> Self {
-        Self {
-            drive_id,
-            kind,
-            host_certs,
-        }
+// Manual Debug: `private_key`/`private_key_v2` are the raw host private keys —
+// they must never reach a log in plaintext; presence (Some/None) of the v2 key
+// stays observable. Mirrors the redacting Debug on AacsAuth/Unlocked.
+impl std::fmt::Debug for HostCert {
+    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+        f.debug_struct("HostCert")
+            .field("private_key", &"[redacted]")
+            .field("certificate", &self.certificate)
+            .field("private_key_v2", &self.private_key_v2.map(|_| "[redacted]"))
+            .field("certificate_v2", &self.certificate_v2)
+            .finish()
     }
 }
 
-/// What removing bus encryption yielded. `drive_unlocked` means the drive now
-/// serves clear content (firmware route) — equivalent, for the gate, to a cert
-/// `bus_key`.
+/// Per-attempt context the consumer hands to EVERY unlocker, uniformly: the
+/// drive identity and the mounted disc's kind. These are the shared facts the
+/// dispatch loop knows; anything an individual unlocker needs beyond them (the
+/// AACS cert route's host certs) is injected into THAT unlocker at construction,
+/// so the loop and the other unlockers never see it. See docs/module-visibility.md.
+pub struct UnlockCtx<'a> {
+    pub drive_id: &'a DriveId,
+    pub kind: DiscKind,
+}
+
+impl<'a> UnlockCtx<'a> {
+    pub fn new(drive_id: &'a DriveId, kind: DiscKind) -> Self {
+        Self { drive_id, kind }
+    }
+}
+
+/// What a successful unlock captured: the Volume ID (best-effort — may be `None`
+/// even on success) and, for the cert route, the bus key the read path applies
+/// to de-bus content. Returned inside the `Some` of [`Unlocker::unlock`]; there
+/// is no `drive_unlocked` flag — "unlocked" is simply `unlock()` returning
+/// `Some`, uniformly for every route.
 #[derive(Clone, Default)]
 pub struct Unlocked {
     pub vid: Option<[u8; 16]>,
     pub bus_key: Option<[u8; 16]>,
-    pub drive_unlocked: bool,
 }
 
-// Hand-written, REDACTING Debug — `Unlocked` is the public carrier that leaves
-// this crate, and both `bus_key` (the AACS bus key) and `vid` (the Volume ID
-// that feeds VUK derivation) are key material that must never reach a log or a
-// test-failure message in plaintext. The `AacsAuth` / `CertHandshake` siblings
-// already redact; a derived `Debug` here would print the very bytes they hide.
-// Presence is still observable (Some/None) so a log can say WHETHER a key was
-// obtained without revealing WHAT it is.
+// Hand-written, REDACTING Debug: `bus_key`/`vid` are key material that must
+// never reach a log in plaintext; presence (Some/None) stays observable.
+// See docs/unlocked-debug-redaction.md — full rationale.
 impl std::fmt::Debug for Unlocked {
     fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
         f.debug_struct("Unlocked")
             .field("vid", &self.vid.map(|_| "[redacted]"))
             .field("bus_key", &self.bus_key.map(|_| "[redacted]"))
-            .field("drive_unlocked", &self.drive_unlocked)
             .finish()
     }
 }
@@ -125,47 +144,46 @@ pub enum UnlockError {
 }
 
 /// An unlocker removes a drive-level bus-encryption barrier. Implementors are
-/// the self-contained modules in this crate; the consumer only ever sees the
-/// trait, via [`all_unlockers`]. (Each module owns its own conversion from its
-/// internal error to [`UnlockError`].)
+/// the self-contained modules in this crate; the consumer drives them all
+/// through this trait. (Each module owns its own conversion from its internal
+/// error to [`UnlockError`].)
 ///
 /// NOTE: drive tuning (e.g. SET CD SPEED to lift riplock) is deliberately NOT
 /// here — that is the consumer's concern, not bus removal.
 pub trait Unlocker: Send + Sync {
-    /// Short, stable identifier for this unlocker (e.g. "MT1959", "AACS",
-    /// "DVD", "Renesas"). The ONE place a name lives — apps render the unlocker
-    /// report from [`all_unlockers`], never hardcoding names, so adding/removing
+    /// Short, stable identifier for this unlocker (e.g. "freemkv", "LD",
+    /// "AACS", "DVD", "Renesas"). The ONE place a name lives — apps render the
+    /// unlocker report from `name()`, never hardcoding names, so adding/removing
     /// an unlocker updates every report with no app change.
     fn name(&self) -> &'static str;
 
-    /// Unlock DRIVE FEATURES — riplock/speed, OEM Volume ID, OEM extended-access
-    /// reads. The consumer runs this at drive-prep, trying each unlocker until
-    /// one handles the drive. `Ok(_)` = this unlocker handled it (Mt1959Unlocker also
-    /// removes bus encryption at the drive, so its result carries
-    /// `drive_unlocked: true`); `Err(NotApplicable)` = not this unlocker's drive;
-    /// `Err(Transport)` = dead bus (the consumer aborts). Default: not provided.
-    fn unlock_features(
+    /// Attempt to unlock the drive, whatever the mechanism (vendor CDB, profile
+    /// firmware handshake, AACS cert AKE, CSS auth):
+    /// `Ok(Some(unlocked))` = this one unlocked it (read `vid`/`bus_key`, stop);
+    /// `Ok(None)` = not this unlocker's drive (try the next); `Err(Transport)` =
+    /// dead bus (abort). Only a dead bus is `Err` — a missing VID, rejected
+    /// handshake, missing cert, or wrong disc kind all fall through as `Ok(None)`
+    /// (the VID inside `Unlocked` is best-effort). `&self`: stateless, returns
+    /// what it learned rather than stashing it.
+    fn unlock(
         &self,
         scsi: &mut dyn ScsiTransport,
         ctx: &UnlockCtx,
-    ) -> std::result::Result<Unlocked, UnlockError> {
-        let _ = (scsi, ctx);
-        Err(UnlockError::NotApplicable)
-    }
+    ) -> std::result::Result<Option<Unlocked>, UnlockError>;
+}
 
-    /// Remove BUS ENCRYPTION for the mounted disc (AACS host-cert handshake, or
-    /// CSS scrambled-sector auth). The consumer runs this only when the bus isn't
-    /// already clear, trying each unlocker until one handles it. Same `Ok` /
-    /// `Err(NotApplicable)` / `Err(Transport)` contract as
-    /// [`Unlocker::unlock_features`].
-    /// Default: not provided.
-    fn unlock_bus(
-        &self,
-        scsi: &mut dyn ScsiTransport,
-        ctx: &UnlockCtx,
-    ) -> std::result::Result<Unlocked, UnlockError> {
-        let _ = (scsi, ctx);
-        Err(UnlockError::NotApplicable)
+/// The shared "only a dead bus is fatal" rule every unlocker uses to turn its
+/// mechanism's `Result<Unlocked>` into the trait's outcome: success → `Some`, a
+/// dead bus → `Err(Transport)`, any other failure → `None` (fall through to the
+/// next unlocker). Written once here so the five unlockers stay identical in
+/// error handling and differ only in their CDBs and checks.
+pub(crate) fn fallthrough(
+    r: std::result::Result<Unlocked, UnlockError>,
+) -> std::result::Result<Option<Unlocked>, UnlockError> {
+    match r {
+        Ok(u) => Ok(Some(u)),
+        Err(UnlockError::Transport) => Err(UnlockError::Transport),
+        Err(_) => Ok(None),
     }
 }
 
@@ -178,70 +196,46 @@ pub fn unlocker_name(drive_id: &DriveId) -> Option<&'static str> {
     ld::firmware_name(drive_id)
 }
 
-/// Every unlocker, in dispatch order (firmware → cert → css). This is the ONLY
-/// place an unlocker is named. Remove one = delete its line here + its module
-/// dir; the consumer never changes.
-pub fn all_unlockers() -> Vec<Box<dyn Unlocker>> {
-    vec![
-        Box::new(ld::Mt1959Unlocker::new()),
-        Box::new(renesis::Renesis::new()),
-        Box::new(aacs::AacsCert::new()),
-        Box::new(css::DvdUnlocker::new()),
-    ]
-}
-
 #[cfg(test)]
 mod tests {
     use super::*;
 
-    /// The dispatch order is a documented contract (`firmware → cert → css`) and
-    /// had NO test: `all_unlockers` is the one place an unlocker is named, and
-    /// the consumer stops at the first that doesn't decline, so reordering this
-    /// list silently changes which unlocker claims a drive. Catches a reorder,
-    /// a rename, and an accidental addition/removal.
-    #[test]
-    fn all_unlockers_dispatch_order_is_firmware_then_cert_then_css() {
-        let names: Vec<&'static str> = all_unlockers().iter().map(|u| u.name()).collect();
-        assert_eq!(names, vec!["MT1959", "Renesas", "AACS", "DVD"]);
+    // The canonical dispatch order the consumer assembles (freemkv → firmware →
+    // Renesas → cert → css). It lives in the consumer now (no factory), but the
+    // types are ours, so pin their names here so a rename is caught crate-local.
+    fn canonical_unlockers() -> Vec<Box<dyn Unlocker>> {
+        vec![
+            Box::new(FreemkvUnlocker::new()),
+            Box::new(LdUnlocker::new()),
+            Box::new(Renesas::new()),
+            Box::new(AacsUnlocker::new(Vec::new())),
+            Box::new(DvdUnlocker::new()),
+        ]
     }
 
-    /// Every unlocker declines by default rather than claiming a capability it
-    /// does not provide — and declines WITHOUT touching the transport, so the
-    /// consumer can iterate the whole registry on any drive.
     #[test]
-    fn unlockers_decline_capabilities_they_do_not_provide() {
-        struct DeadTransport;
-        impl scsi::ScsiTransport for DeadTransport {
-            fn execute(
-                &mut self,
-                _cdb: &[u8],
-                _dir: scsi::DataDirection,
-                _data: &mut [u8],
-                _timeout_ms: u32,
-            ) -> scsi::Result<scsi::ScsiResult> {
-                panic!("a declining unlocker must not issue a CDB");
-            }
-        }
-        let id = DriveId::default(); // matches no bundled profile
-        let ctx = UnlockCtx::new(&id, DiscKind::Unencrypted, &[]);
-        let mut t = DeadTransport;
-        for u in all_unlockers() {
-            // AACS/DVD provide no drive features; Mt1959Unlocker/Renesas are the
-            // feature unlockers but decline an unknown drive identity.
+    fn unlocker_names_are_stable() {
+        let names: Vec<&'static str> = canonical_unlockers().iter().map(|u| u.name()).collect();
+        assert_eq!(names, vec!["freemkv", "LD", "Renesas", "AACS", "DVD"]);
+    }
+
+    /// The uniform contract every unlocker obeys, whatever its mechanism: on a
+    /// DEAD BUS, `unlock()` either declined before touching it (`Ok(false)`) or
+    /// engaged it and reported the fault (`Err(Transport)`) — but NEVER claims a
+    /// dead bus as unlocked, and NEVER surfaces a non-`Transport` hard error.
+    /// This is the guardrail that was missing: it holds the next new unlocker to
+    /// the same discipline the loop relies on.
+    #[test]
+    fn no_unlocker_claims_a_dead_bus_or_hard_errors() {
+        let id = DriveId::default();
+        let ctx = UnlockCtx::new(&id, DiscKind::Aacs);
+        for u in canonical_unlockers() {
             let name = u.name();
-            if name == "AACS" || name == "DVD" {
-                assert_eq!(
-                    u.unlock_features(&mut t, &ctx).unwrap_err(),
-                    UnlockError::NotApplicable,
-                    "{name} must not claim drive features"
-                );
-            }
-            if name == "Renesas" {
-                assert_eq!(
-                    u.unlock_bus(&mut t, &ctx).unwrap_err(),
-                    UnlockError::NotApplicable,
-                    "{name} leaves bus removal to the cert unlocker"
-                );
+            let mut t = scsi::mock::MockTransport::always(scsi::mock::Reply::TransportFault);
+            match u.unlock(&mut t, &ctx) {
+                Ok(None) => {}                    // declined before touching — fine
+                Err(UnlockError::Transport) => {} // engaged, dead bus — fine
+                other => panic!("{name} violated the dead-bus contract: {other:?}"),
             }
         }
     }
@@ -253,16 +247,14 @@ mod tests {
         assert_eq!(unlocker_name(&DriveId::default()), None);
     }
 
-    /// `Unlocked` is the public carrier that leaves the crate, and its `bus_key`
-    /// and `vid` are key material. Its `Debug` must NOT print those bytes.
-    /// MUTATION: restoring `#[derive(Debug)]` prints the raw byte arrays, so the
-    /// distinctive marker bytes appear in the output and this goes red.
+    // `bus_key`/`vid` are key material; `Debug` must NOT print those bytes.
+    // MUTATION: restoring `#[derive(Debug)]` prints the raw byte arrays, so
+    // the marker bytes appear in the output and this test goes red.
     #[test]
     fn unlocked_debug_redacts_key_material() {
         let u = Unlocked {
             vid: Some([0xAB; 16]),
             bus_key: Some([0xCD; 16]),
-            drive_unlocked: true,
         };
         let s = format!("{u:?}");
         // A derived Debug renders `[171, 171, ...]` (0xAB) / `[205, ...]` (0xCD).
@@ -271,6 +263,30 @@ mod tests {
         assert!(s.contains("[redacted]"), "must mark redaction: {s}");
         // Presence (Some/None) stays observable.
         assert!(s.contains("Some"), "must still show a key WAS present: {s}");
-        assert!(s.contains("drive_unlocked: true"));
+    }
+
+    // `private_key`/`private_key_v2` are the raw host private keys; `Debug` must
+    // NOT print those bytes. Restoring `#[derive(Debug)]` renders the byte
+    // arrays (e.g. 0xAB → 171 / 0xCD → 205), turning this test red.
+    #[test]
+    fn host_cert_debug_redacts_private_keys() {
+        let c = HostCert {
+            private_key: [0xAB; 20],
+            certificate: vec![0x01, 0x02],
+            private_key_v2: Some([0xCD; 32]),
+            certificate_v2: None,
+        };
+        let s = format!("{c:?}");
+        assert!(!s.contains("171"), "private_key bytes must not appear: {s}");
+        assert!(
+            !s.contains("205"),
+            "private_key_v2 bytes must not appear: {s}"
+        );
+        assert!(s.contains("[redacted]"), "must mark redaction: {s}");
+        // Presence of the v2 key stays observable.
+        assert!(
+            s.contains("Some"),
+            "must still show the v2 key WAS present: {s}"
+        );
     }
 }

src/renesas/mod.rs

@@ -0,0 +1,443 @@
+//! renesas — Renesas-platform detection (Pioneer + HL-DT-ST Renesas drives).
+//!
+//! Optical drives split into two controller families: MediaTek (handled by
+//! [`crate::ld`]) and Renesas. This module identifies the Renesas side via a
+//! single vendor identity probe (see [`is_renesas`]) and reports the match so a
+//! Renesas drive is named honestly. It does not modify drive state; AACS bus
+//! decryption is handled by the host cert.
+
+use crate::scsi::{DataDirection, ScsiTransport};
+use crate::{UnlockCtx, UnlockError, Unlocked, Unlocker};
+
+/// READ_BUFFER mode 0x02, buffer 0xF1 — the Renesas vendor identity buffer.
+const RB_F1_CDB: [u8; 10] = [0x3C, 0x02, 0xF1, 0x00, 0x00, 0x00, 0x00, 0x00, 0x30, 0x00];
+const RB_F1_LEN: usize = 48;
+/// The ASCII interface marker a Renesas controller returns at `[16..19]`.
+const RENESAS_MARKER: &[u8] = b"SAT";
+const RENESAS_MARKER_OFFSET: usize = 16;
+
+/// `Ok(true)` if `scsi` is a Renesas-platform drive (Pioneer or HL-DT-ST
+/// Renesas).
+///
+/// Issues the vendor READ_BUFFER 0x02/0xF1 probe: a Renesas controller serves
+/// a 48-byte identity block whose bytes `[16..19]` are the ASCII `SAT`
+/// interface tag. A rejection (CHECK CONDITION or `Err` with a sense) is
+/// `Ok(false)`: not a Renesas drive.
+///
+/// `Err(Transport)` on a dead bus. See docs/renesas-mod.md for why a
+/// transport fault must not be folded into `Ok(false)`.
+pub fn is_renesas(scsi: &mut dyn ScsiTransport) -> std::result::Result<bool, UnlockError> {
+    let mut buf = [0u8; RB_F1_LEN];
+    match scsi.execute(&RB_F1_CDB, DataDirection::FromDevice, &mut buf, 5_000) {
+        Ok(r) => {
+            let end = RENESAS_MARKER_OFFSET + RENESAS_MARKER.len();
+            Ok(r.status == 0
+                && r.bytes_transferred >= end
+                && &buf[RENESAS_MARKER_OFFSET..end] == RENESAS_MARKER)
+        }
+        // Only a senseless transport-failure status is a dead bus; anything
+        // else the transport reports as `Err` is the drive refusing.
+        Err(e) => {
+            if e.status == crate::scsi::SCSI_STATUS_TRANSPORT_FAILURE && e.sense.is_none() {
+                tracing::warn!(
+                    target: "freemkv::disc",
+                    phase = "renesas_probe_transport_fault",
+                    "transport fault on the Renesas identity probe; aborting"
+                );
+                return Err(UnlockError::Transport);
+            }
+            tracing::debug!(
+                target: "freemkv::disc",
+                phase = "renesas_probe_rejected",
+                status = e.status,
+                "Renesas identity probe rejected by the drive; not a Renesas platform"
+            );
+            Ok(false)
+        }
+    }
+}
+
+/// The Renesas/Pioneer-platform unlocker. Its vendor "open" CDB puts the drive
+/// in the same raw-read / extended-access state the firmware routes reach (this
+/// is what MakeMKV's LibreDrive does on stock Pioneer firmware), so the Volume
+/// ID is then read with the SAME standard `0xAD` reader as freemkv/MT1959 — no
+/// host cert, no AKE. Stateless: `unlock()` returns what it learned.
+#[derive(Default)]
+pub struct Renesas;
+
+impl Renesas {
+    pub fn new() -> Self {
+        Renesas
+    }
+
+    // MakeMKV's vendor "open" sequence: primary read (A), and on refusal a
+    // knock + second read (B). See docs/renesas-mod.md for the full sequence
+    // and why we run A→knock→B rather than bailing after A.
+    fn vendor_open(scsi: &mut dyn ScsiTransport) -> std::result::Result<bool, UnlockError> {
+        const RB_B0_04_CDB: [u8; 10] = [0x3C, 0x02, 0xB0, 0x00, 0x00, 0x04, 0x00, 0x00, 0xA4, 0x00];
+        const KNOCK_A5AAAA_CDB: [u8; 10] =
+            [0x3B, 0x02, 0x41, 0xA5, 0xAA, 0xAA, 0x00, 0x00, 0x00, 0x00];
+        const RB_B0_500000_CDB: [u8; 10] =
+            [0x3C, 0x02, 0xB0, 0x50, 0x00, 0x00, 0x00, 0x00, 0xA4, 0x00];
+
+        // A: primary open read.
+        if read_is_good(scsi, &RB_B0_04_CDB)? {
+            return Ok(true);
+        }
+        // B: MakeMKV's fallback — knock, then the second-window read. The knock is
+        // fire-and-forget (payload-less); its own status is not the signal.
+        match scsi.execute(&KNOCK_A5AAAA_CDB, DataDirection::None, &mut [], 5_000) {
+            Ok(_) => {}
+            Err(e)
+                if e.status == crate::scsi::SCSI_STATUS_TRANSPORT_FAILURE && e.sense.is_none() =>
+            {
+                return Err(UnlockError::Transport);
+            }
+            Err(_) => {} // a drive that refuses the knock still gets the B read tried
+        }
+        read_is_good(scsi, &RB_B0_500000_CDB)
+    }
+}
+
+/// Issue a 164-byte vendor READ_BUFFER; `Ok(true)` on GOOD status, `Ok(false)`
+/// on CHECK CONDITION (drive refused), `Err(Transport)` on a senseless
+/// transport failure (dead bus).
+fn read_is_good(
+    scsi: &mut dyn ScsiTransport,
+    cdb: &[u8; 10],
+) -> std::result::Result<bool, UnlockError> {
+    let mut buf = [0u8; 164];
+    match scsi.execute(cdb, DataDirection::FromDevice, &mut buf, 5_000) {
+        Ok(r) => Ok(r.status == 0),
+        Err(e) => {
+            if e.status == crate::scsi::SCSI_STATUS_TRANSPORT_FAILURE && e.sense.is_none() {
+                return Err(UnlockError::Transport);
+            }
+            Ok(false)
+        }
+    }
+}
+
+impl Unlocker for Renesas {
+    fn name(&self) -> &'static str {
+        "Renesas"
+    }
+
+    /// Gate on the `0xF1` SAT identity, then the vendor open read `0xB0@0x04`
+    /// (LOAD-BEARING — it's the unlock), then read the Volume ID with the shared
+    /// best-effort `0xAD` reader. `Some` once the open succeeds — the drive
+    /// serves clear content whether or not the bonus VID read did; `None` if it
+    /// isn't a Renesas drive or the open is refused; `Err(Transport)` on a dead
+    /// bus. See docs/renesas-mod.md for the MakeMKV-parity rationale.
+    fn unlock(
+        &self,
+        scsi: &mut dyn ScsiTransport,
+        _ctx: &UnlockCtx,
+    ) -> std::result::Result<Option<Unlocked>, UnlockError> {
+        if !is_renesas(scsi)? {
+            return Ok(None);
+        }
+        if !Self::vendor_open(scsi)? {
+            tracing::debug!(
+                target: "freemkv::disc",
+                phase = "renesas_open_rejected",
+                "Renesas drive recognized but RB 0xB0@0x04 refused; deferring to next unlocker"
+            );
+            return Ok(None);
+        }
+        // The open enabled raw reads (extended-access) — read the VID exactly as
+        // the firmware routes do. Best-effort: a VID miss must not discard the
+        // unlock (only a dead bus propagates via `?`).
+        let vid = crate::vid::read_aacs_vid(scsi)?;
+        tracing::debug!(
+            target: "freemkv::disc",
+            phase = "renesas_opened",
+            has_vid = vid.is_some(),
+            "Renesas drive opened (RB 0xB0@0x04 GOOD, raw reads enabled)"
+        );
+        Ok(Some(Unlocked { vid, bus_key: None }))
+    }
+}
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+    use crate::DiscKind;
+    use crate::scsi::{DataDirection, Result, ScsiError, ScsiResult, ScsiTransport};
+
+    /// Serves a fixed READ_BUFFER payload (Renesas-like) with a Good status.
+    struct RenesasTransport {
+        payload: Vec<u8>,
+    }
+    impl ScsiTransport for RenesasTransport {
+        fn execute(
+            &mut self,
+            _cdb: &[u8],
+            _dir: DataDirection,
+            data: &mut [u8],
+            _timeout_ms: u32,
+        ) -> Result<ScsiResult> {
+            let n = self.payload.len().min(data.len());
+            data[..n].copy_from_slice(&self.payload[..n]);
+            Ok(ScsiResult {
+                status: 0,
+                bytes_transferred: n,
+                sense: [0u8; 32],
+            })
+        }
+    }
+
+    // Rejects like a MediaTek drive: ILLEGAL REQUEST with a sense, which is
+    // what distinguishes this from a dead bus. See docs/renesas-mod.md.
+    struct RejectingTransport;
+    impl ScsiTransport for RejectingTransport {
+        fn execute(
+            &mut self,
+            _cdb: &[u8],
+            _dir: DataDirection,
+            _data: &mut [u8],
+            _timeout_ms: u32,
+        ) -> Result<ScsiResult> {
+            let mut sense = [0u8; 32];
+            sense[2] = 0x05; // ILLEGAL REQUEST
+            sense[12] = 0x20; // invalid command operation code
+            Err(ScsiError {
+                status: crate::scsi::SCSI_STATUS_CHECK_CONDITION,
+                sense: Some(sense),
+            })
+        }
+    }
+
+    fn renesas_payload() -> Vec<u8> {
+        // 48-byte RB 0xF1 block with "SAT" at [16..19] (the real S13JX shape).
+        let mut p = vec![0x20u8; 48];
+        p[16..19].copy_from_slice(b"SAT");
+        p
+    }
+
+    #[test]
+    fn is_renesas_true_on_sat_marker() {
+        let mut t = RenesasTransport {
+            payload: renesas_payload(),
+        };
+        assert!(is_renesas(&mut t).expect("no transport fault"));
+    }
+
+    #[test]
+    fn is_renesas_false_when_command_rejected() {
+        let mut t = RejectingTransport;
+        assert!(!is_renesas(&mut t).expect("a drive rejection is not a bus fault"));
+    }
+
+    #[test]
+    fn is_renesas_false_on_missing_marker() {
+        // Good status but no "SAT" at [16..19] (e.g. a stray buffer).
+        let mut t = RenesasTransport {
+            payload: vec![0u8; 48],
+        };
+        assert!(!is_renesas(&mut t).expect("no transport fault"));
+    }
+
+    #[test]
+    fn is_renesas_false_on_short_response() {
+        // Fewer than 19 bytes returned — can't carry the marker.
+        let mut t = RenesasTransport {
+            payload: vec![0x20u8; 8],
+        };
+        assert!(!is_renesas(&mut t).expect("no transport fault"));
+    }
+
+    #[test]
+    fn opened_renesas_reports_unlocked_and_reads_vid() {
+        let mut t = RenesasTransport {
+            payload: renesas_payload(),
+        };
+        let id = crate::DriveId::default();
+        let ctx = UnlockCtx::new(&id, DiscKind::Unknown);
+        // Recognized + vendor-open GOOD → unlocked; the open enables raw reads,
+        // so the standard 0xAD VID read returns a (non-zero) VID best-effort.
+        let out = Renesas::new()
+            .unlock(&mut t, &ctx)
+            .expect("no fault")
+            .expect("renesas → unlocked");
+        assert!(out.vid.is_some(), "opened drive yields a VID via 0xAD");
+        assert_eq!(out.bus_key, None, "raw-read route needs no bus key");
+    }
+
+    /// A drive REJECTION (ILLEGAL REQUEST, with a sense) is "not a Renesas
+    /// drive" → `Ok(false)`, fall through to the next unlocker.
+    #[test]
+    fn declines_non_renesas() {
+        let mut t = RejectingTransport;
+        let id = crate::DriveId::default();
+        let ctx = UnlockCtx::new(&id, DiscKind::Unknown);
+        assert!(
+            Renesas::new()
+                .unlock(&mut t, &ctx)
+                .expect("non-renesas declines")
+                .is_none()
+        );
+    }
+
+    // Same rejection via a CONFORMING transport (`Ok` + CHECK CONDITION);
+    // must reach the same answer. See docs/renesas-mod.md.
+    #[test]
+    fn check_condition_is_not_a_renesas_drive() {
+        use crate::scsi::mock::{MockTransport, Reply};
+        let mut t = MockTransport::always(Reply::illegal_request());
+        assert!(!is_renesas(&mut t).expect("a drive sense is not a bus fault"));
+    }
+
+    /// THE defect-8 test: a dead bus on the FIRST command the unlocker issues
+    /// must abort the consumer, not be reported as "not a Renesas drive".
+    /// Catches restoring the `Err(_) => false` arm.
+    #[test]
+    fn transport_fault_aborts_instead_of_declining() {
+        use crate::scsi::mock::{MockTransport, Reply};
+        let mut t = MockTransport::always(Reply::TransportFault);
+        assert_eq!(is_renesas(&mut t).unwrap_err(), UnlockError::Transport);
+
+        let mut t = MockTransport::always(Reply::TransportFault);
+        let id = crate::DriveId::default();
+        let ctx = UnlockCtx::new(&id, DiscKind::Unknown);
+        assert_eq!(
+            Renesas::new().unlock(&mut t, &ctx).unwrap_err(),
+            UnlockError::Transport
+        );
+    }
+
+    // A recognized Renesas drive whose A read (RB 0xB0@0x04) is REFUSED but
+    // whose B read (RB 0xB0@0x50) SUCCEEDS after the knock must unlock — and the
+    // knock CDB has to be issued BETWEEN A and B. See docs/renesas-mod.md.
+    #[test]
+    fn knock_runs_between_a_and_b_and_b_opens() {
+        // The exact knock CDB the code issues (pinned wire format).
+        const KNOCK_A5AAAA_CDB: [u8; 10] =
+            [0x3B, 0x02, 0x41, 0xA5, 0xAA, 0xAA, 0x00, 0x00, 0x00, 0x00];
+
+        struct AFailsBSucceeds {
+            cdbs: Vec<Vec<u8>>,
+        }
+        impl ScsiTransport for AFailsBSucceeds {
+            fn execute(
+                &mut self,
+                cdb: &[u8],
+                _dir: DataDirection,
+                data: &mut [u8],
+                _timeout_ms: u32,
+            ) -> Result<ScsiResult> {
+                self.cdbs.push(cdb.to_vec());
+                // Gate 0xF1 → SAT identity.
+                if cdb.get(2) == Some(&0xF1) {
+                    let p = renesas_payload();
+                    let n = p.len().min(data.len());
+                    data[..n].copy_from_slice(&p[..n]);
+                    return Ok(ScsiResult {
+                        status: 0,
+                        bytes_transferred: n,
+                        sense: [0u8; 32],
+                    });
+                }
+                // A read (RB 0xB0@0x04) → refuse with a drive sense.
+                if cdb.get(2) == Some(&0xB0) && cdb.get(3) == Some(&0x00) {
+                    let mut sense = [0u8; 32];
+                    sense[2] = 0x05; // ILLEGAL REQUEST
+                    sense[12] = 0x20;
+                    return Err(ScsiError {
+                        status: crate::scsi::SCSI_STATUS_CHECK_CONDITION,
+                        sense: Some(sense),
+                    });
+                }
+                // B read (RB 0xB0@0x50), the knock (0x3B/0x41), and the 0xAD VID
+                // read all return GOOD so the open + VID read succeed.
+                let n = data.len().min(36);
+                if !data.is_empty() {
+                    // Non-zero VID so read_aacs_vid yields Some.
+                    for b in data.iter_mut().take(n) {
+                        *b = 0x5A;
+                    }
+                }
+                Ok(ScsiResult {
+                    status: 0,
+                    bytes_transferred: data.len(),
+                    sense: [0u8; 32],
+                })
+            }
+        }
+
+        let mut t = AFailsBSucceeds { cdbs: Vec::new() };
+        let id = crate::DriveId::default();
+        let ctx = UnlockCtx::new(&id, DiscKind::Unknown);
+        let out = Renesas::new()
+            .unlock(&mut t, &ctx)
+            .expect("no fault")
+            .expect("B read opens the drive → unlocked");
+        assert_eq!(out.bus_key, None);
+
+        // The knock must appear, between the A read (0xB0@0x04) and the B read
+        // (0xB0@0x50).
+        let a = t
+            .cdbs
+            .iter()
+            .position(|c| c.get(2) == Some(&0xB0) && c.get(3) == Some(&0x00))
+            .expect("A read issued");
+        let knock = t
+            .cdbs
+            .iter()
+            .position(|c| c.as_slice() == KNOCK_A5AAAA_CDB)
+            .expect("knock CDB issued");
+        let b = t
+            .cdbs
+            .iter()
+            .position(|c| c.get(2) == Some(&0xB0) && c.get(3) == Some(&0x50))
+            .expect("B read issued");
+        assert!(a < knock && knock < b, "knock must run between A and B");
+    }
+
+    // A recognized Renesas drive that REFUSES the vendor open read must
+    // defer to the next unlocker, not claim the drive. See docs/renesas-mod.md.
+    #[test]
+    fn open_rejection_defers_to_next_unlocker() {
+        struct GateOkOpenRefused;
+        impl ScsiTransport for GateOkOpenRefused {
+            fn execute(
+                &mut self,
+                cdb: &[u8],
+                _dir: DataDirection,
+                data: &mut [u8],
+                _timeout_ms: u32,
+            ) -> Result<ScsiResult> {
+                if cdb.get(2) == Some(&0xF1) {
+                    // Serve the SAT identity so is_renesas() matches.
+                    let p = renesas_payload();
+                    let n = p.len().min(data.len());
+                    data[..n].copy_from_slice(&p[..n]);
+                    Ok(ScsiResult {
+                        status: 0,
+                        bytes_transferred: n,
+                        sense: [0u8; 32],
+                    })
+                } else {
+                    // RB 0xB0@0x04 (the open read) → CHECK CONDITION.
+                    let mut sense = [0u8; 32];
+                    sense[2] = 0x05; // ILLEGAL REQUEST
+                    sense[12] = 0x20; // invalid command operation code
+                    Err(ScsiError {
+                        status: crate::scsi::SCSI_STATUS_CHECK_CONDITION,
+                        sense: Some(sense),
+                    })
+                }
+            }
+        }
+        let mut t = GateOkOpenRefused;
+        let id = crate::DriveId::default();
+        let ctx = UnlockCtx::new(&id, DiscKind::Unknown);
+        assert!(
+            Renesas::new()
+                .unlock(&mut t, &ctx)
+                .expect("open refused → declines, not a hard error")
+                .is_none(),
+            "a refused vendor open must defer to the next unlocker"
+        );
+    }
+}

src/renesis.rs

@@ -1,265 +0,0 @@
-//! renesis — Renesas-platform detection (Pioneer + HL-DT-ST Renesas drives).
-//!
-//! Optical drives split into two controller families: MediaTek (handled by
-//! [`crate::ld`]) and Renesas. This module identifies the Renesas side via a
-//! single vendor identity probe (see [`is_renesas`]) and reports the match so a
-//! Renesas drive is named honestly. It does not modify drive state; AACS bus
-//! decryption is handled by the host cert.
-
-use crate::scsi::{DataDirection, ScsiTransport};
-use crate::{UnlockCtx, UnlockError, Unlocked, Unlocker};
-
-/// READ_BUFFER mode 0x02, buffer 0xF1 — the Renesas vendor identity buffer.
-const RB_F1_CDB: [u8; 10] = [0x3C, 0x02, 0xF1, 0x00, 0x00, 0x00, 0x00, 0x00, 0x30, 0x00];
-const RB_F1_LEN: usize = 48;
-/// The ASCII interface marker a Renesas controller returns at `[16..19]`.
-const RENESAS_MARKER: &[u8] = b"SAT";
-const RENESAS_MARKER_OFFSET: usize = 16;
-
-/// `Ok(true)` if `scsi` is a Renesas-platform drive (Pioneer or HL-DT-ST
-/// Renesas).
-///
-/// Issues the vendor READ_BUFFER 0x02/0xF1 probe: a Renesas controller serves a
-/// 48-byte identity block whose bytes `[16..19]` are the ASCII `SAT` interface
-/// tag. A MediaTek drive rejects it — as a CHECK CONDITION (`Ok` with a
-/// non-zero status) or, through a non-conforming transport, as an `Err`
-/// carrying a sense — which is `Ok(false)`: not a Renesas drive. This is the
-/// definitive Renesas-vs-MediaTek split.
-///
-/// A TRANSPORT fault is `Err(Transport)`. It used to be folded into the same
-/// `false` as a drive rejection, and because this probe is the first SCSI
-/// command the unlocker issues, a dead bus read as "not a Renesas drive" — the
-/// consumer then walked the remaining unlockers against a bus that was gone.
-pub fn is_renesas(scsi: &mut dyn ScsiTransport) -> std::result::Result<bool, UnlockError> {
-    let mut buf = [0u8; RB_F1_LEN];
-    match scsi.execute(&RB_F1_CDB, DataDirection::FromDevice, &mut buf, 5_000) {
-        Ok(r) => {
-            let end = RENESAS_MARKER_OFFSET + RENESAS_MARKER.len();
-            Ok(r.status == 0
-                && r.bytes_transferred >= end
-                && &buf[RENESAS_MARKER_OFFSET..end] == RENESAS_MARKER)
-        }
-        // Only a senseless transport-failure status is a dead bus; anything
-        // else the transport reports as `Err` is the drive refusing.
-        Err(e) => {
-            if e.status == crate::scsi::SCSI_STATUS_TRANSPORT_FAILURE && e.sense.is_none() {
-                tracing::warn!(
-                    target: "freemkv::disc",
-                    phase = "renesas_probe_transport_fault",
-                    "transport fault on the Renesas identity probe; aborting"
-                );
-                return Err(UnlockError::Transport);
-            }
-            tracing::debug!(
-                target: "freemkv::disc",
-                phase = "renesas_probe_rejected",
-                status = e.status,
-                "Renesas identity probe rejected by the drive; not a Renesas platform"
-            );
-            Ok(false)
-        }
-    }
-}
-
-/// The Renesas-platform unlocker. `pub(crate)` — reached only through
-/// [`crate::all_unlockers`].
-pub(crate) struct Renesis;
-
-impl Renesis {
-    pub(crate) fn new() -> Self {
-        Renesis
-    }
-}
-
-impl Unlocker for Renesis {
-    fn name(&self) -> &'static str {
-        "Renesas"
-    }
-
-    /// Report whether the drive is a Renesas platform. On a match, returns `Ok`
-    /// with `drive_unlocked: false` — the drive is recognized but its state is
-    /// not modified here (AACS bus decryption is handled by the host cert). A
-    /// non-Renesas drive → `NotApplicable`.
-    fn unlock_features(
-        &self,
-        scsi: &mut dyn ScsiTransport,
-        _ctx: &UnlockCtx,
-    ) -> std::result::Result<Unlocked, UnlockError> {
-        if !is_renesas(scsi)? {
-            return Err(UnlockError::NotApplicable);
-        }
-        tracing::debug!(
-            target: "freemkv::disc",
-            phase = "renesas_recognized",
-            "Renesas drive recognized; bus handled by cert"
-        );
-        Ok(Unlocked {
-            vid: None,
-            bus_key: None,
-            drive_unlocked: false,
-        })
-    }
-}
-
-#[cfg(test)]
-mod tests {
-    use super::*;
-    use crate::DiscKind;
-    use crate::scsi::{DataDirection, Result, ScsiError, ScsiResult, ScsiTransport};
-
-    /// Serves a fixed READ_BUFFER payload (Renesas-like) with a Good status.
-    struct RenesasTransport {
-        payload: Vec<u8>,
-    }
-    impl ScsiTransport for RenesasTransport {
-        fn execute(
-            &mut self,
-            _cdb: &[u8],
-            _dir: DataDirection,
-            data: &mut [u8],
-            _timeout_ms: u32,
-        ) -> Result<ScsiResult> {
-            let n = self.payload.len().min(data.len());
-            data[..n].copy_from_slice(&self.payload[..n]);
-            Ok(ScsiResult {
-                status: 0,
-                bytes_transferred: n,
-                sense: [0u8; 32],
-            })
-        }
-    }
-
-    /// Rejects the command the way a MediaTek drive does: ILLEGAL REQUEST. The
-    /// sense is what distinguishes this from a dead bus — the old fixture sent
-    /// `sense: None`, which is the wire shape of a TRANSPORT fault, and the test
-    /// built on it asserted `NotApplicable`, pinning exactly the
-    /// misclassification this file had.
-    struct RejectingTransport;
-    impl ScsiTransport for RejectingTransport {
-        fn execute(
-            &mut self,
-            _cdb: &[u8],
-            _dir: DataDirection,
-            _data: &mut [u8],
-            _timeout_ms: u32,
-        ) -> Result<ScsiResult> {
-            let mut sense = [0u8; 32];
-            sense[2] = 0x05; // ILLEGAL REQUEST
-            sense[12] = 0x20; // invalid command operation code
-            Err(ScsiError {
-                status: crate::scsi::SCSI_STATUS_CHECK_CONDITION,
-                sense: Some(sense),
-            })
-        }
-    }
-
-    fn renesas_payload() -> Vec<u8> {
-        // 48-byte RB 0xF1 block with "SAT" at [16..19] (the real S13JX shape).
-        let mut p = vec![0x20u8; 48];
-        p[16..19].copy_from_slice(b"SAT");
-        p
-    }
-
-    #[test]
-    fn is_renesas_true_on_sat_marker() {
-        let mut t = RenesasTransport {
-            payload: renesas_payload(),
-        };
-        assert!(is_renesas(&mut t).expect("no transport fault"));
-    }
-
-    #[test]
-    fn is_renesas_false_when_command_rejected() {
-        let mut t = RejectingTransport;
-        assert!(!is_renesas(&mut t).expect("a drive rejection is not a bus fault"));
-    }
-
-    #[test]
-    fn is_renesas_false_on_missing_marker() {
-        // Good status but no "SAT" at [16..19] (e.g. a stray buffer).
-        let mut t = RenesasTransport {
-            payload: vec![0u8; 48],
-        };
-        assert!(!is_renesas(&mut t).expect("no transport fault"));
-    }
-
-    #[test]
-    fn is_renesas_false_on_short_response() {
-        // Fewer than 19 bytes returned — can't carry the marker.
-        let mut t = RenesasTransport {
-            payload: vec![0x20u8; 8],
-        };
-        assert!(!is_renesas(&mut t).expect("no transport fault"));
-    }
-
-    #[test]
-    fn features_report_match_without_bus_removal_on_renesas() {
-        let mut t = RenesasTransport {
-            payload: renesas_payload(),
-        };
-        let id = crate::DriveId::default();
-        let ctx = UnlockCtx::new(&id, DiscKind::Unknown, &[]);
-        // Recognized → Ok, but a feature-only unlock: bus NOT removed, no VID.
-        let u = Renesis::new()
-            .unlock_features(&mut t, &ctx)
-            .expect("renesas → Ok");
-        assert!(
-            !u.drive_unlocked,
-            "renesis does not remove the bus (cert does)"
-        );
-        assert_eq!(u.vid, None);
-        assert_eq!(u.bus_key, None);
-    }
-
-    /// A drive REJECTION (ILLEGAL REQUEST, with a sense) is "not a Renesas
-    /// drive" → fall through to the next unlocker.
-    #[test]
-    fn features_not_applicable_on_non_renesas() {
-        let mut t = RejectingTransport;
-        let id = crate::DriveId::default();
-        let ctx = UnlockCtx::new(&id, DiscKind::Unknown, &[]);
-        let err = Renesis::new().unlock_features(&mut t, &ctx).unwrap_err();
-        assert_eq!(err, UnlockError::NotApplicable);
-    }
-
-    /// The same rejection delivered the way a CONFORMING transport delivers it —
-    /// `Ok` with a CHECK CONDITION status — must reach the same answer. Catches
-    /// dropping the `r.status == 0` check, which would read the probe buffer's
-    /// zero fill as an identity block.
-    #[test]
-    fn check_condition_is_not_a_renesas_drive() {
-        use crate::scsi::mock::{MockTransport, Reply};
-        let mut t = MockTransport::always(Reply::illegal_request());
-        assert!(!is_renesas(&mut t).expect("a drive sense is not a bus fault"));
-    }
-
-    /// THE defect-8 test: a dead bus on the FIRST command the unlocker issues
-    /// must abort the consumer, not be reported as "not a Renesas drive".
-    /// Catches restoring the `Err(_) => false` arm.
-    #[test]
-    fn transport_fault_aborts_instead_of_declining() {
-        use crate::scsi::mock::{MockTransport, Reply};
-        let mut t = MockTransport::always(Reply::TransportFault);
-        assert_eq!(is_renesas(&mut t).unwrap_err(), UnlockError::Transport);
-
-        let mut t = MockTransport::always(Reply::TransportFault);
-        let id = crate::DriveId::default();
-        let ctx = UnlockCtx::new(&id, DiscKind::Unknown, &[]);
-        assert_eq!(
-            Renesis::new().unlock_features(&mut t, &ctx).unwrap_err(),
-            UnlockError::Transport
-        );
-    }
-
-    #[test]
-    fn does_not_provide_bus_removal() {
-        // Renesas leaves bus encryption to the cert: unlock_bus is the default.
-        let mut t = RenesasTransport {
-            payload: renesas_payload(),
-        };
-        let id = crate::DriveId::default();
-        let ctx = UnlockCtx::new(&id, DiscKind::Aacs, &[]);
-        let err = Renesis::new().unlock_bus(&mut t, &ctx).unwrap_err();
-        assert_eq!(err, UnlockError::NotApplicable);
-    }
-}

src/scsi.rs

@@ -100,14 +100,8 @@ pub(crate) fn build_set_cd_speed(read_speed: u16) -> [u8; 12] {
 }
 
 // ── Test fixture ────────────────────────────────────────────────────────────
-//
-// The crate-wide mock transport. Before this existed, EVERY mock in the crate
-// returned `Ok` with status 0 and a full `bytes_transferred` — so no test could
-// ever exercise the three outcomes the contract actually distinguishes, and a
-// whole class of transport-fault misclassification bugs sat uncaught (a dead bus
-// read as "this unlocker doesn't apply", a zero-filled buffer read as valid
-// drive data). This fixture can express all of them, and is the red-before-green
-// vehicle for the transport-contract fixes across ld / aacs / css / renesis.
+// Crate-wide mock transport, able to express all three transport outcomes.
+// See docs/scsi-mock-fixture.md — why this fixture exists
 #[cfg(test)]
 #[allow(dead_code)] // a fixture: each helper is used by a subset of the modules
 pub(crate) mod mock {
@@ -322,10 +316,9 @@ mod tests {
         assert_eq!(build_set_cd_speed(0xFFFF)[2..4], [0xFF, 0xFF]);
     }
 
-    /// The fixture itself must honour the contract it exists to test: a drive
-    /// sense is `Ok` with a non-zero status, a bus fault is `Err`. If this ever
-    /// drifts, every transport-contract test built on it silently stops testing
-    /// anything.
+    // The fixture must honour the contract it tests: a drive sense is `Ok`
+    // with non-zero status, a bus fault is `Err`. If this drifts, every
+    // transport-contract test built on it silently stops testing anything.
     #[test]
     fn mock_transport_expresses_the_three_contract_outcomes() {
         let mut t = MockTransport::scripted(

src/vid.rs

@@ -0,0 +1,193 @@
+//! Shared AACS Volume ID read — the standard `READ DISC STRUCTURE` (`0xAD`,
+//! format `0x80`) that returns the VID on a drive whose host-auth / bus has
+//! already been opened by a firmware or vendor CDB unlock (freemkv Raw Read,
+//! MT1959, or a Pioneer/Renesas vendor open). Because those routes put the
+//! drive in the same raw-read state, the VID read is identical across them — so
+//! it lives here once.
+//!
+//! BEST-EFFORT: only a dead bus is an `Err(Transport)`. A CHECK CONDITION, a
+//! short response, or an all-zero VID all yield `Ok(None)` — a VID miss must
+//! never discard an unlock that already removed the bus (a key source can still
+//! supply the key). See docs/freemkv-abi.md for the fixed-length reply rules.
+
+use crate::UnlockError;
+use crate::scsi::{DataDirection, ScsiTransport};
+
+/// The 16-byte AACS Volume ID.
+const VID_LEN: usize = 16;
+/// `READ DISC STRUCTURE` format for the Volume ID.
+const DISC_STRUCT_FMT_VID: u8 = 0x80;
+/// Response length: a 4-byte header, the 16-byte VID, and a 16-byte MAC. On the
+/// bare-read path (no bus key) the MAC can't be verified and is ignored.
+const VID_STRUCT_LEN: u16 = 36;
+
+/// The standard `0xAD` fmt-`0x80` (Blu-ray, AGID 0) Volume ID CDB — NOT a vendor
+/// knock; valid once the drive's bus/host-auth is open. Mirrors libfreemkv's
+/// `read_volume_id`.
+pub(crate) fn build_vid_cdb() -> [u8; 12] {
+    let mut cdb = [0u8; 12];
+    cdb[0] = crate::scsi::SCSI_READ_DISC_STRUCTURE;
+    cdb[1] = 0x01; // Blu-ray
+    cdb[7] = DISC_STRUCT_FMT_VID;
+    cdb[8] = (VID_STRUCT_LEN >> 8) as u8;
+    cdb[9] = (VID_STRUCT_LEN & 0xFF) as u8;
+    // cdb[10] = agid << 6; AGID 0 on the bare path (no AKE) → 0.
+    cdb
+}
+
+/// A senseless transport-failure status is a genuine dead bus, not a drive
+/// rejection surfaced through a non-conforming transport (`Err` with a sense).
+fn is_dead_bus(e: &crate::scsi::ScsiError) -> bool {
+    e.status == crate::scsi::SCSI_STATUS_TRANSPORT_FAILURE && e.sense.is_none()
+}
+
+/// Read the AACS VID with the bare `0xAD` fmt `0x80` (valid only after the drive
+/// is unlocked). `Ok(Some(vid))` on a well-formed non-zero VID; `Ok(None)` for
+/// any "no VID" outcome (rejected / short / all-zero); `Err(Transport)` only on
+/// a dead bus.
+pub(crate) fn read_aacs_vid(
+    scsi: &mut dyn ScsiTransport,
+) -> std::result::Result<Option<[u8; 16]>, UnlockError> {
+    let cdb = build_vid_cdb();
+    let mut buf = [0u8; VID_STRUCT_LEN as usize];
+    let result = match scsi.execute(&cdb, DataDirection::FromDevice, &mut buf, 5_000) {
+        Ok(r) => r,
+        Err(e) => {
+            if is_dead_bus(&e) {
+                return Err(UnlockError::Transport);
+            }
+            tracing::debug!(
+                target: "freemkv::disc",
+                phase = "vid_rejected_as_err",
+                status = e.status,
+                "bare VID read rejected (via Err); no Volume ID"
+            );
+            return Ok(None);
+        }
+    };
+    // A drive sense arrives as Ok with a non-zero status; without this check a
+    // CHECK CONDITION's zero-filled buffer would parse as a VID.
+    if result.status != 0 {
+        tracing::debug!(
+            target: "freemkv::disc",
+            phase = "vid_check_condition",
+            status = result.status,
+            "bare VID read returned a drive sense (no medium / drive not open)"
+        );
+        return Ok(None);
+    }
+    // Need the 4-byte header + the 16-byte VID.
+    if result.bytes_transferred < 4 + VID_LEN {
+        tracing::debug!(
+            target: "freemkv::disc",
+            phase = "vid_short_response",
+            bytes_transferred = result.bytes_transferred,
+            "bare VID response too short"
+        );
+        return Ok(None);
+    }
+    let mut vid = [0u8; VID_LEN];
+    vid.copy_from_slice(&buf[4..4 + VID_LEN]);
+    // An all-zero VID is what a permissive stub or an unfilled response leaves
+    // behind — reject it rather than pass a bogus key downstream.
+    if vid.iter().all(|&b| b == 0) {
+        tracing::debug!(
+            target: "freemkv::disc",
+            phase = "vid_all_zero",
+            "bare VID read returned an all-zero Volume ID"
+        );
+        return Ok(None);
+    }
+    tracing::debug!(target: "freemkv::disc", phase = "vid_ok", "Volume ID retrieved via bare 0xAD read");
+    Ok(Some(vid))
+}
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+    use crate::scsi::mock::{MockTransport, Reply};
+
+    /// A well-formed format-0x80 VID structure: 4-byte header, 16-byte VID at
+    /// offset 4, 16-byte MAC (zeroed on the bare path — it isn't checked).
+    fn vid_ds_response(vid: [u8; 16]) -> Vec<u8> {
+        let mut p = vec![0u8; VID_STRUCT_LEN as usize];
+        p[4..20].copy_from_slice(&vid);
+        p
+    }
+
+    /// The bare VID read is the STANDARD 0xAD READ DISC STRUCTURE (format 0x80,
+    /// Blu-ray, AGID 0, len 36) — NOT a vendor knock.
+    #[test]
+    fn build_vid_cdb_is_standard_0xad_fmt_80() {
+        let cdb = build_vid_cdb();
+        assert_eq!(cdb[0], crate::scsi::SCSI_READ_DISC_STRUCTURE);
+        assert_eq!(cdb[1], 0x01); // Blu-ray
+        assert_eq!(cdb[7], 0x80); // AACS Volume ID
+        assert_eq!([cdb[8], cdb[9]], [0x00, 0x24]); // 36, BE16
+        assert_eq!(cdb[10], 0x00); // AGID 0 (no AKE)
+    }
+
+    /// Parses the VID from response[4..20] and issues exactly one 0xAD read.
+    #[test]
+    fn reads_and_parses_offset_4() {
+        let vid = [0x5Au8; 16];
+        let mut t = MockTransport::always(Reply::good(vid_ds_response(vid)));
+        let got = read_aacs_vid(&mut t).expect("no fault");
+        assert_eq!(got, Some(vid));
+        assert_eq!(t.cdbs.len(), 1, "exactly one command");
+        assert_eq!(t.cdbs[0][0], crate::scsi::SCSI_READ_DISC_STRUCTURE);
+        assert_eq!(t.cdbs[0][7], 0x80);
+    }
+
+    /// The MAC region (bytes 20..36) is ignored on the bare path.
+    #[test]
+    fn ignores_the_unverifiable_mac() {
+        let vid: [u8; 16] = [
+            0x00, 0x11, 0x22, 0x33, 0x44, 0x55, 0x66, 0x77, 0x88, 0x99, 0xAA, 0xBB, 0xCC, 0xDD,
+            0xEE, 0xFF,
+        ];
+        let mut t = MockTransport::always(Reply::good(vid_ds_response(vid)));
+        assert_eq!(read_aacs_vid(&mut t).expect("no fault"), Some(vid));
+    }
+
+    /// A response too short to carry header + VID → best-effort `Ok(None)`.
+    #[test]
+    fn short_response_is_none() {
+        let mut t = MockTransport::always(Reply::short(vid_ds_response([0x11; 16]), 19));
+        assert_eq!(read_aacs_vid(&mut t).expect("no fault"), None);
+    }
+
+    /// A CHECK CONDITION (no medium / drive not open) → `Ok(None)`, never a VID
+    /// parsed from the zero fill.
+    #[test]
+    fn check_condition_is_none() {
+        let mut t = MockTransport::always(Reply::illegal_request());
+        assert_eq!(read_aacs_vid(&mut t).expect("no fault"), None);
+    }
+
+    /// An all-zero VID (permissive stub / unfilled response) → `Ok(None)`.
+    #[test]
+    fn all_zero_is_none() {
+        let mut t = MockTransport::always(Reply::good(vid_ds_response([0u8; 16])));
+        assert_eq!(read_aacs_vid(&mut t).expect("no fault"), None);
+    }
+
+    /// A rejection surfaced as `Err` WITH a sense (a non-conforming transport)
+    /// is a drive refusal, not a dead bus → `Ok(None)` VID-miss, never
+    /// `Err(Transport)`.
+    #[test]
+    fn err_with_sense_is_a_vid_miss_not_transport() {
+        let mut t = MockTransport::always(Reply::illegal_request_as_err());
+        assert_eq!(
+            read_aacs_vid(&mut t).expect("a sense is not a dead bus"),
+            None
+        );
+    }
+
+    /// Only a dead bus is an error.
+    #[test]
+    fn transport_fault_propagates() {
+        let mut t = MockTransport::always(Reply::TransportFault);
+        assert_eq!(read_aacs_vid(&mut t).unwrap_err(), UnlockError::Transport);
+    }
+}