Skip to content

Supervisor times PCI claims: the declaration in the device set, and storage and homes before the rows that need them - #1164

Merged
Japabu merged 5 commits into
mainfrom
supervised-claims
Nov 4, 2026
Merged

Japabu merged 5 commits into
mainfrom
supervised-claims

Conversation

@Japabu

@Japabu Japabu commented Nov 3, 2026 •

Copy link
Copy Markdown
Collaborator

Status: draft. On main 6dccc4956 (#934), head 12e913653; it answers reviews r1 and r2 and the re-review's blocker. Metal at 0ade582c4 judges green on both benches, acpi_server_death among them. bd2a67e01 and 12e913653 change only the restart-during-startup window and an assert, which no metal row reaches.

Problem

#934 made every PCI claim wait in the kernel, for up to 30 s, for the root bridges' declaration. The supervisor started the disk drivers right after the acpi holder, so on a boot whose first holder is a job, every claim waited out the deadline and the boot had no storage. Two tests were lost to that:

  • acpi_server_death (T14), whose log reaches the stick only through storage;
  • the guest arm reading a held function's configuration refused to the acpi holder.

The owner ruled (1–2 Nov 2026) that claims are the supervisor's to time: the kernel's claim-time wait goes, and the supervisor starts drivers off the device set once the declaration is in it.

What this changes

Kernel and ABI

  • toyos_abi::devset::Header carries the declaration's state: Pending, Held, or Withheld(Refused | Ended). It is bytes 1..3, refused by name when malformed (host-tested).
  • declare and withhold post the set's watch. The snapshot reads the generation before the state, so a change in between is posted again.
  • A PCI claim made while nothing is answered is refused at once: ClaimError::Undeclared, WouldBlock in userland.
  • Deleted with the wait: DECLARATION, DECLARATION_DEADLINE, answered()'s park, and aperture::Withheld::Unanswered.

Supervisor: startup ordered by what each row needs. toyos_supervisor::declaration::plan decides it, host-tested; main.rs follows it, and the sequence is written once, at storage_up. The order comes from manifest facts already stated, with no new flag.

  • SYSTEM: the plan covers services[1..], so SYSTEM, running since before the manifest, is in none of its lists.

  • Storage's state: Storage is Awaiting, Starting { drivers }, Up { drivers } or Never. Starting is storage_up's window. Inside it, a service that ends is started again, and so is a disk driver, since the homes the window makes go through it; no other drives row starts until the homes and every other row exist (Storage::may_start). So "no row starts before its HOME exists" holds for those restarts too, and the window cannot stall on its own disk. It is host-tested both ways, with a control for each. Every declared row's HOME is its state directory or the session's, on DATA, so a row starts only after its HOME exists. The exceptions are the rows the declaration comes through, and the log.

  • Nothing to wait for: where nothing can declare, every row starts at once, storage first, with no PCI driver, said once. "Nothing can declare" means no started acpi holder, and no started row that may mint the claim (device) to hand it to a row the manifest declares holding it. acpicase's acpiserver row now declares that claim. These are the virt images, undeclaredcase and diag.

  • First: logkeeper, the acpi claim's holder, and, where no holder is started, a row that may mint the claim and hand it on. A program's output waits in its ring until logkeeper reads it.

  • On the declaration held:

    1. the disk drivers;
    2. the block service and the file servers;
    3. the session's home and every state directory;
    4. every other row;
    5. the drives rows.

    The supervisor says how long after its first rows the declaration came.

  • On the declaration withheld, or none by a 30 s bound: said once by name. Storage and the PCI drivers are given up for the boot.

    • The unstarted storage rows' ports are closed, so a file request is refused rather than left waiting. That waiting is what stalled logkeeper on a withheld boot before this design closed them. SYSTEM is in no list, so it keeps its ports.
    • Every other row then starts with no storage.

Restored

  • acpi_server_death (T14 metal), with its judge.
    • Its job, acpi_release, now kills the server only once it has declared.
    • After 64 tries landing on the boot processor it claims from there and says so; the judge already requires the enable to have been asked off it.
    • The judge also reads the declaration and the supervisor's line.
    • acpicase gets its storage and console rows back.
  • The held-function guest arm, as acpi_mediated's held arm: acpi_holder_is_refused_a_held_functions_configuration.
    • acpicase, derived with two jobs: acpi_release's server declares, so storage and the virtio-serial driver start.
    • The probe then waits on its own device set until a process holds a virtio-serial function, bounded at 30 s, and expects HeldFunction by address and through ECAM.
    • Its issue is deleted.

Tests and checks

  • a_claim_waits_out_an_undeclared_machine becomes a_claim_before_the_declaration_is_refused_at_once. Its new job, pci_undeclared, reads Pending in the set and has each edu claim refused WouldBlock, and the supervisor says nothing can declare.
  • The build check is every_config_that_claims_a_pci_function_has_a_declarer: a started holder, or the server built in for a started device row to hand the claim.
  • Profile::HeadlessNoConsolePort stays for acpicase's probe rows, whose holders declare nothing.

Why the guest rows need QEMU

The device-set header and the kernel's refusal, the supervisor's ordering on a real boot, the held flag fed to the ACPI fence, and a probe holding the fixed hardware are a booted machine's. The decisions themselves are host-tested in toyos-abi, toyos-pci, toyos-supervisor and toyos-userbound. No bench boots without its ACPI server or can be made to withhold.

Checks

Evidence: ToyOSOrg/evidence pr-1164 at 31b14e6d5 for the latest commit, and at eaa5aef54, with rounds 1 and 2 beside it in supervised-claims-r1/ and -r2/.

check revision result
cargo test -p toyos-supervisor 12e913653 exit 0, 8 declaration tests (log)
controls: the plan with index 0 listed; disk drivers held out of the window; every row let into it f96bbd779, 12e913653 exit 101 each
cargo run -- --ci host 12e913653 exit 0, 109 green (log)
cargo run -- --build-only, x86-64 and aarch64 f96bbd779 exit 0, 0
11 x86 rows on the KVM guest host (qemu-remote.sh) bd2a67e01, which the latest commit changes only in the window's restart rule exit 0, 11 passed (log)
the same 11 plus virt_reboot (4 rows) and virt_first_entry, locally bd2a67e01 exit 0, 16 passed
round 1's broad sample, 18 rows over every image with a declarer 79473b788's WIP 18 passed
  • The 11 rows: acpi_press_is_one_sci, acpi_lock_given_back_on_one_cpu, acpi_mediated_access, acpi_supply_outlives_holder, acpi_holder_is_refused_a_held_functions_configuration, a_claim_before_the_declaration_is_refused_at_once, a_row_reaches_its_own_state_and_no_other, a_function_arrives_and_leaves, a_drives_row_holds_one_process_per_function, the_log_lands_on_the_boot_stick and session_lifecycle. Between them they cover the withheld, overdue, held, no-declarer and ordinary paths.
  • The local run was on a loaded host, before the owner's rule moved x86 guests to the KVM host.
  • Not run: hosted_clang_hello, which builds LLVM for hours.

Metal

boots T14 AMD
0ade582c4 (T14, AMD) acpi_server_death, acpi_tables_loaded, usbd_drives_the_type_c_controller: 3/3 amdvi_usbd_drives_its_own_entry: 1/1
79473b788 the same, 3/3 1/1
  • On acpicase at 0ade582c4: the job asked ACPI_ENABLE from cpu2, the supervisor read the declaration 79 ms after its first rows, ACPI_DISABLE was written at the server's death, and the log reached the stick.
  • The death issue is deleted as met.
  • No boot at bd2a67e01: it touches no path these rows read.

Risks and open points

  • A machine whose acpi holder never declares (issues/an-x86-machine-with-no-acpi-row-hands-over-no-pci-claim.md) now boots with no storage after 30 s, said once. Before, it had storage services and no disks.
  • Rows started before storage (logkeeper, the holder, and a device runner on a boot whose holder is a job) find their state directory once storage is up. Each is one the boot needs before storage can come.

🤖 Generated with Claude Code

…ge and homes after it

By the owner's ruling of 1–2 Nov 2026, the kernel no longer waits at claim
time for the root bridges' declaration; the supervisor starts what needs it
once the device set says it is held.

Kernel and ABI:
- `toyos_abi::devset::Header` carries the declaration's state (`Pending`,
  `Held`, `Withheld(Refused | Ended)`), bytes 1..3, refused by name when
  malformed. `declare` and `withhold` post the set's watch; the snapshot
  reads the generation before the state, so a change between is posted again.
- A PCI claim made while nothing is answered is refused at once,
  `ClaimError::Undeclared`, `WouldBlock` to userland. Deleted with the wait:
  `DECLARATION`, `DECLARATION_DEADLINE`, `answered()`'s park and
  `aperture::Withheld::Unanswered`.

Supervisor (`toyos_supervisor::declaration`, host-tested):
- Where nothing can declare (no started `acpi` holder, and no started row
  that may mint the claim with the ACPI server built in), nothing waits:
  every row starts at once, storage first, no PCI driver, said once.
- Else first: `logkeeper`, the `acpi` claim's holder, and where none is
  started a row that may mint the claim and hand it on (`device`), as a test
  boot's runner does. A program's output waits in its ring for logkeeper.
- On the declaration held: the disk drivers, the block service, the file
  servers, then the session's home and every state directory, then every
  other row, so no row starts before its `HOME` exists; then every `drives`
  row. It says how long after its first rows the declaration came.
- On the declaration withheld, or none by a 30 s bound: said once by name,
  storage and the PCI drivers are given up for the boot, the unstarted
  storage rows' ports are closed so a file request is refused rather than
  left waiting, and every other row starts with no storage.

Restored by it:
- `acpi_server_death` (T14) with its judge and its job, `acpi_release`, which
  now kills the server only once it has declared: on that boot the job makes
  the first `acpi` claim, and the log reaches the stick through the storage
  the declaration starts. acpicase gets its storage and console rows back.
- The held-function guest arm, as `acpi_mediated`'s `held` arm on a boot
  whose first job's server declared:
  `acpi_holder_is_refused_a_held_functions_configuration`.

The undeclared row is now `a_claim_before_the_declaration_is_refused_at_once`:
a job's claim of each `edu` refused `WouldBlock` while the set reads
`Pending`, and the supervisor's bound said. The build check is
`every_config_that_claims_a_pci_function_has_a_declarer`: a started holder,
or the server built in for a started `device` row to hand the claim.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@Japabu

Japabu commented Nov 4, 2026

Copy link
Copy Markdown
Collaborator Author

Review r1 of #1164 at 79473b788

Architecture

This matches the owner's ruling. The kernel no longer waits: answered(), DECLARATION, DECLARATION_DEADLINE and Withheld::Unanswered are gone, and a claim made before an answer returns ClaimError::Undeclared, which userland sees as WouldBlock. The declaration state lives in the device-set header (toyos_abi::devset, the right boundary). Its encoding is validated and refused by name, and host tests cover every value and five malformed byte pairs. The ordering is correct. changed() bumps the generation and posts SET_WATCH, and snapshot() reads the generation before the declaration. Since the handle's readiness compares generations, a declaration that lands between the two reads is seen again on the next read. The supervisor waits on TOKEN_DEVICES, and the 30 s BOUND only feeds the poll timeout (storage.deadline()), so it is a failure timeout and not a fixed delay. Storage::read checks Held before the bound. A declaration that is read at or after the deadline, while the boot is still Awaiting, still starts storage, and a declaration after Overdue starts nothing. Both behaviours are deterministic. The order comes from facts the manifest already states (devices, syscap, roles, drives, serves), with no new flag. A holder that dies before declaring gives Withheld(Ended), which is said once and closes the ports. A holder that dies after declaring leaves Held in place.

The new library (toyos_supervisor::declaration) holds the predicates and the state machine. The order itself (disk drivers, then block and file servers, then homes, then the remaining rows, then drives) lives in main.rs and has no host test. Inspection says that order is right on the paths below.

Required

  1. system/supervisor/src/main.rs:1138, :1147, :1186: unstarted includes SYSTEM, a row that is already started. In both boot() partitions, index 0 (SYSTEM) goes into self.unstarted. start_without_storage then tells SYSTEM apart from unstarted rows by child.is_none(), and start_service by child.is_some(). But SYSTEM is restart = true. If it ended and is starved, waiting for memory (child == None), when the declaration is withheld or overdue, its ports are closed and /system is lost for the boot. If that happens on the Held path, storage_up → start_service(0) spawns it outside the memory wait, and memory_returned later spawns it again. This is the same SYSTEM-ports bug the evidence's sc-guests-5 found, patched by testing a process's state rather than fixed in the representation. Fix: build unstarted from 1..len (SYSTEM is never unstarted) and drop the child.is_none() filter.

  2. sdk/abi/src/acpi.rs:41: the kernel/program boundary still documents the deleted wait. It says "Until a declaration is held the kernel hands over no PCI function, and a claim made meanwhile waits for it". This is now false: such a claim is refused WouldBlock, and the device-set header says when to try again. Rewrite the sentence. The device_claim doc (sdk/abi/src/syscall.rs:1867) could name WouldBlock as well; that part is optional.

  3. system/supervisor/lifecycle/src/declaration.rs:58-67 and main.rs:811, :1501: Storage does not describe storage, and a second flag makes up for it. On a boot with no declarer, boot() sets Storage::Never ("Never started") and then starts storage. up() is documented as "Whether the drivers may be started". storage_started exists only because the enum does not say whether storage runs. Inspection shows storage_started equals "storage rows started", which is "Up, or the no-declarer path": the rows started between Up and the flag are storage rows and disk drivers, and make_state_dir filters both out. Give the state one accurate representation, for example Up { drivers: bool } (or name the enum for what it tracks), and delete storage_started.

  4. Comments: one is false and several repeat each other.

    • declaration.rs:7 says logkeeper "keeps no home". It is service = true, so its HOME is /state/logkeeper, made after it starts. It simply uses no HOME (neither logkeeper nor acpiserver reads one, which I checked). Say what is true or delete it.
    • The module header (:1-22) describes the startup sequence that main.rs implements and documents again at boot(), storage_up and start_without_storage. The same sequence is now written four times. Keep the header to what the module decides (comes, starts_first, Storage/BOUND) and keep the sequence at storage_up.
    • main.rs:743-746, is_storage: "Started once the root bridges' memory is declared" is false on the no-declarer path, which starts storage at once.
  5. Evidence: the reuse claim is false for two rows. The PR says the later SYSTEM-ports change "touch[es] only boots with no declarer and withheld boots, whose rows were rerun". acpi_press_is_one_sci takes the overdue path: sc-guests-5 shows "not declared 30 s into the boot" and then storage ports closed, including fileserver system. acpi_lock_given_back_on_one_cpu takes the withheld path. Both passed only before SYSTEM's ports were kept, and sc-guests-6/-7 do not rerun them. The cheapest check: rerun those two rows at head (about 2 min), or correct the claim.

  6. Metal before landing. Every image's start order changed, and on the T14 and the AMD bench storage itself now waits on the supervisor's Held read. acpi_tables_on_metal now also requires the supervisor's "declared" line. The staged T14 (acpicase acpi_server_death; testcases acpi_tables_loaded, usbd_drives_the_type_c_controller) and AMD (amdvi_usbd_drives_its_own_entry) runs are the only evidence that a metal boot still reaches its stick through this path. The PR's own scope says they are owed. They are not optional.

Optional

  • main.rs:1131: comes() is given system.program("acpiserver").is_some(), so whether a declarer exists is inferred from a program name. The build test does the same. Acceptable for test images today; a shared constant would at least tie the two together.
  • ended() → drive() re-enters during storage_up's files() waits. This predates the PR: main's boot() had the same shape. If a storage server ends and restarts there, start_drivers(|_| true) starts non-disk drives rows before the homes and remaining rows, and their state directories fail with "worker still waits". The new "no row before its home" claim does not hold in that failure case. Skipping start_drivers until storage_up has finished would close it.
  • declaration::HELD_SAID is used only by main.rs, while the harness repeats the literal three times (tests/toyos.rs). Either have the harness use the constant or inline it.
  • Add a host-test case for Held read at or after since + BOUND while still Awaiting (expect Start). That pins the deadline race described above.
  • kernel/src/pcidev/mod.rs:825: declaration() clones the whole declaration's window Vec on every device-set read just to pick a variant. Match on the borrowed answer under the lock instead.
  • acpi_release's module header still says it claims "from a thread off the boot processor". After 64 tries it no longer does. The T14 judge still means something: acpi_death_on_metal refuses asked from cpu0, so the fallback turns a job panic into a named judge failure on metal and only relaxes the QEMU held row, which does not care which CPU asks.
  • Commit message: "and the supervisor's bound said" for the undeclared row is wrong. That boot takes the no-declarer path, and the test asserts "nothing on this boot can declare". The PR body has it right.
  • issues/the-acpi-servers-death-has-no-readback-on-the-t14.md is now status: assigned. issues/README.md requires the body to say who holds it (this PR or the metal run).

Checked and fine

  • Closing the unstarted storage ports cannot hang. Dropping an acceptor answers its queued and future connects with Gone, and the evidence shows logkeeper unstalled on withheld boots. Apart from SYSTEM (item 1), no running server is closed, because a storage row in unstarted is by construction never started.
  • On Held, every row except logkeeper, the holder and a device runner on a boot whose holder is a job starts after make_session_home, its own state directory and load_grants. Non-disk drives rows start only after storage_up, apart from the re-entrance noted above. The exceptions use no HOME.
  • The held row, read in sc-held-1.log: declared at 9.675, disk drivers before diskserver before the file servers, and virtio-console after them. The probe found the function Claimed while the supervisor held the mint, before the driver was spawned. The fence keys on the claim, so the claim still holds. Its assertion is a specific HeldFunction refusal, which a broken fence cannot produce.
  • The deleted issue's slug is not referenced anywhere in the tree.
  • The build check mirrors comes.

SEND BACK

Japabu and others added 2 commits November 4, 2026 06:50
…of it, storage one state

- `toyos_supervisor::declaration::plan` decides which rows start first,
  which are storage and which follow, over `services[1..]`: SYSTEM, started
  before the manifest was read, is in no list, so a withheld or overdue boot
  never closes its ports and the held path never starts it again. The
  `child.is_none()` filter goes. Host-tested, with a control that lets the
  plan include index 0 (it fails).
- The row predicates (`declares`, `is_block`, `is_disk_driver`,
  `is_storage`) move there beside it. Whether a declarer exists is a manifest
  fact, a row holding the `acpi` claim started or not, rather than the name
  `acpiserver`; acpicase's acpiserver row declares the claim its job hands
  it, and the build check reads the same fact.
- `Storage` is `Awaiting`, `Up { drivers }` or `Never`: a boot nothing can
  declare on is `Up { drivers: false }`. `storage_started` goes.
- The startup sequence is written once, at `storage_up`; the module header
  says what the module decides, and logkeeper is said to use no `HOME`
  rather than to keep none.
- `acpi.rs`'s module doc and `device_claim`'s say a PCI claim before the
  declaration is `WouldBlock`.
- `declaration()` matches the answer under the lock instead of cloning its
  windows; the supervisor's line is one constant in the harness; a held
  declaration read at the bound is host-tested; `acpi_release`'s header
  says it claims off the boot processor where one of its tries runs there;
  the death issue names #1164 as its holder.

Correction to `79473b788`'s message: the undeclared row asserts the
supervisor's word that nothing on that boot can declare, not its bound.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Judged off the T14's boot of acpicase at #1164's 79473b7: the job's claim
asked `ACPI_ENABLE` from cpu2, the server declared the root bridges' memory
before it was killed, the supervisor read the declaration 79 ms after its
first rows and started storage, `ACPI_DISABLE` was written at the server's
death, and the log reached the stick. The records that pointed at it say the
row runs again.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@Japabu

Japabu commented Nov 4, 2026

Copy link
Copy Markdown
Collaborator Author

Review r1, answered at f96bbd779 (code) and 0ade582c4 (issues). Evidence: pr-1164 af9a1d0ae, supervised-claims-r2/.

Required

  1. Fixed in the representation. toyos_supervisor::declaration::plan builds first, storage and rest over services[1..], so SYSTEM is in none of them.

    • The held path never starts SYSTEM again, and a withheld or overdue boot never closes its ports. The child.is_none() filter is gone.
    • system_is_in_no_list holds this for every boot shape. A control that lets the plan include index 0 fails it (exit 101).
    • The row predicates moved there with it.
  2. Fixed. acpi.rs's module doc now says a claim before the declaration is refused WouldBlock, and the device set's header says when it is held. device_claim names it too.

  3. Fixed. Storage is Awaiting, Up { drivers } or Never. A boot nothing can declare on is Up { drivers: false }, and storage_started is gone.

  4. Fixed. The sequence is written once, at storage_up.

    • The module header says what the module decides.
    • logkeeper "uses no HOME".
    • is_storage no longer claims a declaration precedes it.
  5. Rerun at the head. acpi_press_is_one_sci and acpi_lock_given_back_on_one_cpu pass, among 15 rows covering the withheld, overdue, held, no-declarer and ordinary paths.

  6. Metal. Your boots at 79473b788 judge green:

    • T14: acpi_server_death, acpi_tables_loaded and usbd_drives_the_type_c_controller.
    • AMD: amdvi_usbd_drives_its_own_entry.

    On acpicase the declaration came 79 ms after the first rows, ACPI_DISABLE was written at the server's death, and the log is on the stick. The death issue is deleted as met. Both benches are restaged at 0ade582c4 into a new directory.

Optional, taken

  • The possible declarer is a manifest fact: a row holding the acpi claim, started or not. acpicase's acpiserver row declares it, and the build check reads the same fact.
  • The supervisor's line: inlined in the supervisor, with one constant in the harness.
  • At the bound: a declaration held at or after the bound is host-tested to start storage.
  • declaration(): it borrows under the lock rather than cloning the windows.
  • acpi_release's header: corrected.
  • The commit message's undeclared-row wording: corrected in f96bbd779's message.
  • The issue's holder: named, then the issue deleted once its exit was met.

Optional, not taken

  • The ended() → drive() re-entrance during storage_up: it predates this PR, and closing it needs a state of its own.

🤖 Generated with Claude Code

@Japabu

Japabu commented Nov 4, 2026

Copy link
Copy Markdown
Collaborator Author

Review r2 of #1164 at 0ade582c4 (code at f96bbd779)

Architecture

Round 1's main finding is fixed in the representation, as asked. toyos_supervisor::declaration::plan partitions services[1..] into first, storage and rest. main.rs only consumes those lists: boot starts first, storage_up takes storage and then rest, and start_without_storage takes storage (closing its ports) and then rest. Each list is taken exactly once, and which path takes it is decided by Storage's one-way transitions. So no row is started twice, and no started row has its ports closed. Storage is now Awaiting / Up { drivers } / Never, and storage_started is gone. The no-declarer boot is Up { drivers: false } from boot(). Whether a declarer exists is now a manifest fact (a row holding acpi, started or not) and no longer a program name. The build check reads the same fact: holder || handed in build.rs:3239-3245 matches held || (holder && mints) in plan. All 24 configs that declare acpiserver give it devices = ["acpi"], and acpicase is the only one where it is not started, so declarer detection changes on no image. Nothing outside the supervisor's start path reads an unstarted row's devices, so the added devices = ["acpi"] has no side effect.

The kernel's declaration() now matches the borrowed answer under MACHINE's lock and clones nothing.

Round 1 items

  1. Fixed. plan iterates enumerate().skip(1), and services[0] is system_service (main.rs:644). system_is_in_no_list covers holder and no-holder boots with both holder values, and also checks that each other row appears exactly once. The control (mutation-system-listed.{patch,log}) drops the skip(1) and fails with storage: [0, 3], exit 101. One leftover is under Required below.
  2. Fixed. acpi.rs says a claim before the declaration is refused WouldBlock and points to devset::Declaration. device_claim names it too.
  3. Fixed. Storage::started() gates make_state_dir, and drivers() gates both start_drivers calls.
  4. Fixed. The module header now states only what the module decides. The sequence is written once, at storage_up. "Uses no HOME" is true. The false is_storage comment is gone.
  5. Verified. sc-r2-guests.log was built at f96bbd779 ("build f96bbd7… clean") and has 15 passed, exit 0. acpi_press_is_one_sci takes the overdue path ("not declared 30 s into the boot") and acpi_lock_given_back_on_one_cpu the withheld one. Both, and the other withheld boots, close diskserver and fileserver data/log/boot but not fileserver system. The no-declarer and held paths appear in the same log.
  6. Verified as reported.
    • T14 (metal-sc-judge-t14.log, exit 0): acpi_tables_loaded, acpi_server_death and usbd_drives_the_type_c_controller pass. On acpicase: ACPI_ENABLE … asked from cpu2, the declaration read 79 ms in, ACPI_DISABLE asked from cpu3, and the FACS check is in the judge.
    • AMD (metal-sc-judge-amd.log, exit 0): amdvi_usbd_drives_its_own_entry passes.
    • Those boots are of 79473b788. Reusing them for f96bbd779 holds by inspection. On a held boot, the only behavioural differences are:
      • SYSTEM no longer reaches start_service, where it returned early before anyway, unless starved.
      • On acpicase, declarer detection reaches the same result from a different fact.
      • Storage's representation.
    • None of these changes the held path's order. The restage at the head is confirmation, not a gap.

Guarantees rechecked after the restructure

  • No row starts before its home exists.
    • Held path: first, then disk drivers, storage, the session home, the state directories of running rows, grants, then rest (each through start_service → make_state_dir), and then the other drives rows from drive().
    • No-declarer path: first is empty, so logkeeper is in rest and starts after the homes.
    • Withheld or overdue: make_state_dir is a no-op because nothing is made.
    • Exception: the re-entrance below.
  • Withheld-path port closing. Only plan.storage is closed. That list can be taken only while Awaiting, and nothing in it was ever started. first contains no storage row: logkeeper, the holder and the minter all precede is_storage in the match and none of them is storage.

Required

  1. system/supervisor/src/main.rs:1557-1576 (files() → ended() → drive()): the declined re-entrance is not recorded in issues/.

    • No file under issues/ mentions it. I searched for storage_up, re-entrance, worker still waits and ended().
    • The trace stands. During storage_up's make_session_home, make_state_dir or load_grants, files() calls ended() when any service ends. ended() then calls self.drive() (main.rs:2187). By that point Storage is already Up { drivers: true }, so start_drivers(|_| true) starts the non-disk drives rows before the homes and rest. Their state directories are then refused with "the supervisor's file worker still waits".
    • It predates the PR, and declining a fix here is reasonable. But the PR now claims "a row starts only after its HOME exists", and CLAUDE.md's "No silent debt" requires the exception to be recorded with evidence, owner and exit.
    • Cheapest resolution: file it, with the trace above, the supervisor as owner, and as exit that no drives row starts before storage_up returns.
  2. system/supervisor/src/main.rs:1158-1162: the child.is_some() guard in start_service and its comment "SYSTEM was started before the manifest was read." are now dead and misleading.

    • With index 0 in no list, no caller passes a row with a child: boot's first, and storage_up's and start_without_storage's storage/rest.
    • The comment says SYSTEM reaches this function, which is now false.
    • If a future change did send a started row here, the guard would skip it silently. That is the "patched by testing a process's state" pattern round 1 asked to remove.
    • Delete the guard and its comment. If an invariant check is wanted, use assert!(service.child.is_none()).

Optional

  • system/supervisor/lifecycle/src/declaration.rs:90, Storage::Up's doc ("on a declaration held, drivers and the PCI drivers start beside it; on a boot nothing can declare on, not") is hard to parse. Suggested wording: "Started: drivers where a declaration is held, so the PCI drivers may claim; not where nothing can declare."
  • main.rs:653: the placeholder Plan { comes: true, … } and Storage::new(true, …) in main() are overwritten at once by boot(). Plan::comes is never read after boot(), because Storage carries it. You could have plan return comes beside the lists, or derive Default for Plan. This is minor.
  • tests/guest/rust-tests/src/bin/acpi_release.rs:2-4: "from a thread off the boot processor where one of its tries runs there" is awkward, and it does not say what happens otherwise. Suggested wording: "from a thread off the boot processor, or after 64 tries that all landed on it, from the boot processor, which it says".

Evidence

  • The host results, the control and both image builds are at f96bbd779, with exit codes recorded.
  • The 15 guest rows are at f96bbd779.
  • The metal runs are at 79473b788. Their reuse is justified above.
  • 0ade582c4 changes only issue records. The deleted issue's exit (enable asked off cpu0, ACPI_DISABLE with SCI_EN clear, FACS outside AddressRangeMemory, declaration before the death) is what acpi_death_on_metal judges, and the T14 log passes it.

SEND BACK

… a row is started once by construction

- `storage_up` waits on the supervisor's file worker for the homes, the
  state directories and the grants, and a service that ended meanwhile went
  through `ended()` to `drive()`. `Storage` was already `Up`, so that reading
  started the other `drives` rows before their homes existed. `Storage` now
  has the window as its own state, `Starting { drivers }`: a service that
  ends there is started again, but `drives` rows wait for `finish()`, which
  `storage_up` calls once every other row is started. Storage's own disk
  drivers start the window on `declared()`. Host-tested with a service ending
  inside the window; a control that lets `drivers()` answer inside it fails.
- `start_service` asserts the row was never started instead of skipping one
  silently: since round 1's plan, no caller passes one.
- `Plan` derives `Default` for the supervisor's construction;
  `Storage::Up`'s doc and `acpi_release`'s header say what they mean.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@Japabu

Japabu commented Nov 4, 2026

Copy link
Copy Markdown
Collaborator Author

Review r2, answered at bd2a67e01. Evidence: pr-1164 eaa5aef54, supervised-claims-r3/.

Required

  1. Fixed in this PR, not recorded. Storage gains Starting { drivers }, storage_up's window.
    • A service that ends inside the window is started again, but drive() starts no drives row until finish(). storage_up calls finish() once the homes, the state directories, the grants and every other row are done.
    • Storage's own disk drivers start the window on declared().
    • no_drives_row_starts_inside_storages_window models a service ending inside the window. A control that lets drivers() answer true inside it fails (exit 101).
    • "No row starts before its HOME exists" now holds for those restarts too.
  2. Fixed. start_service asserts the row was never started; the dead guard and its comment are gone.

Optional, all taken

  • Storage::Up's doc, reworded.
  • Plan derives Default for the supervisor's construction.
  • acpi_release's header, in your wording.

Checks

  • --ci host: 109 green at bd2a67e01.
  • The 11 x86 ordering rows pass on the KVM guest host by the owner's new rule. The same 11 plus the virt rows also passed locally beforehand.

Metal

  • At 0ade582c4: T14 3/3, acpi_server_death among them, and AMD 1/1.
  • I agree no new boot is needed. bd2a67e01 touches only the restart-during-startup window and an assert, and no metal row reaches either.

🤖 Generated with Claude Code

@Japabu

Japabu commented Nov 4, 2026

Copy link
Copy Markdown
Collaborator Author

Review r3 of #1164 at bd2a67e (focused on r2's required items)

Architecture. A Starting state for storage_up's window is the right representation. The window becomes a fact of Storage and stops being something each caller has to infer from Up. The plan is still a partition, and each list is taken once. The lifecycle stays in the supervisor library. The fix overshoots in one place: it holds back every drives start inside the window, including the disk drivers that the window itself depends on.

Item 1: re-entrance of storage_up

Verified:

  • storage_up runs once. Storage::read moves Awaiting to Starting only once. new(false) goes straight to Starting, and boot calls storage_up directly on that path. A drive() inside the window reads Starting as Step::Nothing, so neither storage_up nor start_without_storage can be entered again.
  • The rest rows start only after the session home, the state directories and the grants. Other drives rows start only after finish(): drive() gates them on drivers(), which is Up { drivers: true } alone, and the Step::Start arm runs start_drivers(|_| true) immediately after storage_up returns. No owed function is lost. Drives::ended marks it Owed, and that drive() call starts it.
  • A supervised service that ends inside the window still restarts through ended() and start_again. I found no new deadlock among supervised services.
  • The control (drivers() answering declared()) fails with exit 101 on the window assertion, as reported. It exercises the library predicate. The main.rs wiring (finish() at the end of storage_up, declared() at its head) was checked by inspection, which is enough here.

BLOCKER

  • system/supervisor/src/main.rs:1218 — a disk driver that ends inside the window now stays down until the window closes. Before this commit, ended() → drive() saw Up { drivers: true } and restarted it at once. Now drivers() is false in Starting, so the restart waits for finish(). The window's own calls depend on that driver. make_session_home, make_state_dir and load_grants go through the file server to diskserver. diskserver waits for a driver that ended after settling, up to DRIVER_BOUND (30 s, lib/blockring/src/wire.rs:72). The supervisor's files() gives up at FILES_BOUND (30 s). Once the worker owes a call, each later files() call fails at once. So in this case the boot stalls 30 s, and every home, state directory and grant of the boot is lost: the rows then start without the homes the change exists to guarantee. This is the "lost restart" r2 asked about. It is a new regression on a plausible path: a disk driver failing on its first I/O. Disk drivers are storage rows. storage_up itself starts them before any home, so restarting one inside the window breaks no ordering rule. Cheapest fix: in drive(), start the disk drivers while storage is Starting with a declaration held (else if self.storage.declared() { self.start_drivers(is_disk_driver); }, or a Storage predicate saying the same). Extend the host test so that a disk driver may be started inside the window while the other drives rows may not.

Item 2: the assert in start_service

It holds on every caller. start_service is called only from boot (plan.first), storage_up (plan.storage, plan.rest) and start_without_storage (plan.rest). plan puts each row from index 1 onward in exactly one list (host-tested by system_is_in_no_list), and each list is mem::taken once. storage_up and start_without_storage are mutually exclusive, and each runs once. A row gets a child before its start_service only through start_again/start_from, cut_over or memory_returned. Each of those needs a child it had before: accept_swap refuses a service that is not running, and starved is set only after a restart. Restarts never go through start_service. A row whose start was refused keeps child == None and is never started again. Removing the guard is right.

The metal argument

It holds. On the normal path the behaviour is identical: declared() equals the old drivers() at the head of storage_up, and started() includes Starting. The only difference is a drive() reached through ended() inside the window. The T14 judge log shows both declarations (76 ms and 79 ms) with no "ended" or "started again" line. The AMD log likewise shows none. acpi_release's kill of acpiserver ends a job child of the runner, not a supervised row, so it does not reach ended(). The blocker's fix only touches that same window and needs no new boot either. The host test and inspection are enough.

Comments: Storage::Starting, declared(), step 5 of storage_up and acpi_release's header are accurate and short enough.

SEND BACK

…tarts again

`bd2a67e01` held every `drives` row back while storage and the homes are
started, disk drivers among them. The window's own steps, the session's
home, the state directories and the grants, go through the block service to
a disk driver, so one that ended there was not restarted until `finish()`:
the boot waited its 30 s and lost its homes. `Storage::may_start` now
decides each row: every one once storage is up on a declaration, and inside
the window a disk driver alone. `drive()` starts what it allows.

`only_a_disk_driver_starts_inside_storages_window` holds both directions,
and on a boot nothing can declare on, neither. Two controls fail it: disk
drivers held back in the window, and every row let in.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@Japabu

Japabu commented Nov 4, 2026

Copy link
Copy Markdown
Collaborator Author

Re-review blocker, answered at 12e913653. Evidence: pr-1164 31b14e6d5, supervised-claims-r4/.

Change: Storage::may_start(program) decides each drives row, and drive() starts what it allows.

  • Once storage is up on a declaration, every row may start.
  • Inside storage's window, only a disk driver may, so one that ends there is started again and the window's home, state directories and grants keep their disk.
  • Other drives rows still wait for finish().

Test: only_a_disk_driver_starts_inside_storages_window checks both directions, and that neither starts on a boot nothing can declare on. Each direction has a control, both exit 101:

  • disk drivers held back inside the window;
  • every row let in.

Checks: --ci host, 109 green. No metal and no guest rerun, as briefed.

🤖 Generated with Claude Code

@Japabu
Japabu marked this pull request as ready for review November 4, 2026 09:00
@Japabu
Japabu added this pull request to the merge queue Nov 4, 2026
Merged via the queue into main with commit 7835695 Nov 4, 2026
10 checks passed
@Japabu
Japabu deleted the supervised-claims branch November 4, 2026 09:21
@Japabu

Japabu commented Nov 4, 2026

Copy link
Copy Markdown
Collaborator Author

Orchestrator verification of the r3 blocker fix at 12e913653, recorded after the merge because it was missing before it.

  • Scope: the r3 review found one blocker. A disk driver that ended inside Storage::Starting was not restarted until finish(). Everything else in r3 was verified: the assert, the single storage_up entry, and the no-new-boot argument.
  • The fix: 12e913653 adds Storage::may_start(program) = drivers() || (declared() && is_disk_driver(program)). drive() now starts drives rows through start_drivers(|row| storage.may_start(row)). That is the shape the reviewer proposed: inside the window a disk driver may restart, and every other drives row waits for finish().
  • The test: only_a_disk_driver_starts_inside_storages_window covers before the declaration, inside the window, after finish(), and a boot where nothing declares. Each direction has a negative control that fails with the test's own message (exit 101), per the implementer's evidence (pr-1164 31b14e6d5).
  • Diff: 2 files, +35/−16, only the window predicate, its caller and the test. No metal row reaches the window, so r3's no-new-boot reasoning still applies. Host CI is green at 12e913653 (109 steps).

Verdict: blocker resolved. I verified it directly rather than through a fourth reviewer pass.

Japabu added a commit that referenced this pull request Nov 4, 2026
…before the declaration

Main's start plan (#1164) put every row that neither keeps the log, holds
the acpi claim nor may mint it after the declaration, storage and the
homes, and the UART's driver among them. The kernel lets go of the UART at
its hand-over, before userland runs, so a machine whose only console is its
UART said nothing from userland's start until storage was up, and nothing at
all where storage hung. The driver needs neither the declaration nor a
home.

The plan starts first the rows that carry the log, by what they hold: the
log's origins (logkeeper) and the console's input (the UART's driver), in
place of logkeeper's name. A host test pins the UART's driver in `first`;
the plan's old rule reds it.

The refused-stop comment and record now say only the lines the UART had
already taken repeat, and the record's exit paragraph is wrapped.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant