Skip to content

The filesystem protocol speaks its own refusals, and pci-claim its own - #948

Merged
Japabu merged 6 commits into
mainfrom
wt/toyos-fserr
Oct 20, 2026
Merged

Japabu merged 6 commits into
mainfrom
wt/toyos-fserr

Conversation

@Japabu

@Japabu Japabu commented Oct 15, 2026 •

Copy link
Copy Markdown
Collaborator

Problem

The file protocol's replies crossed as kernel ABI words: Reply.status was a SyscallError, and the fileserver's Volume interface, both volumes, the absent volume and the request bounds all spoke SyscallError (255 uses across system/fileserver/src/{absent,bounds,data,fat,main,volume}.rs, 42 in sdk/toyos/src/fs.rs). Distinct refusals collapsed into kernel words (not empty, not a directory and is a directory were all InvalidArgument; no space and an entry past its bound were both ResourceExhausted), and an unknown status became Unknown. toyos-pci-claim handed its callers the kernel's word too.

Resulting behavior

  • toyos::fs::Error sits beside the wire types in sdk/toyos/src/fs.rs. It has 31 named variants: 28 a server sends, each with its own wire code, and three that are the client end's own (Gone, NotHeld, Malformed) and have none. from_code knows only the server's refusals, so any other status, including 29 to 31, reads as Malformed, and Reply::refused of a client word panics. A frame type or byte count no server sends is Malformed too. There is no catch-all and no kernel code inside it.
  • Kernel failures are translated once at each end. On the client, Error::kernel covers the calls the protocol itself makes: window create, connect, the hello's loan, the frame transport. On the server, it covers a stream's pipe. Those calls only answer Gone and ResourceExhausted for handles the caller made, so any other word panics as a broken invariant. A name the namespace lacks becomes NotHeld.
  • The fileserver speaks Error throughout. Each format crate's error is translated once (data::refusal, fat::word), now one to one where the protocol has the word (NotADirectory, IsADirectory, NotEmpty, NoSpace, FileTooLarge, LimitExceeded, InvalidName). The server's own refusals are named too: ReadOnly, NotOpenForWriting, Root, Escapes, CrossesDirectories, Loop, IntoItself, TooManyOpen/Connections/Streams, UnknownFile, InvalidPath, InvalidRequest, Stale.
  • diskserver's client names a missing block service NotHeld, so the fileserver no longer matches on Kernel(SyscallError::NotFound). A block service that ended under a DATA read is now Io to the file client, no longer Gone, which std had read as "the file server is gone".
  • std (sdk/std/sys/fs.rs) maps each served refusal to its io::ErrorKind in one exhaustive table and no longer routes served errors through to_io_error. Kinds that change: ReadOnly stays PermissionDenied; NoSpace is now StorageFull and the limits are QuotaExceeded, where both used to be OutOfMemory; NotEmpty is DirectoryNotEmpty; Stale and Gone are StaleNetworkFileHandle; a served readdir that keeps growing is Interrupted (was OutOfMemory), as the kernel route says. A served seek past the largest offset stays InvalidInput, as the kernel route answers it.
  • The supervisor's power-off sync and the app_view guest case read the new type directly. Dir::sync returns Error, since a link is no answer to a sync, and Dir::symlink returns Error, since it names no path to resolve.
  • toyos-pci-claim: KernelRefused.why is now Why (Gone, Exhausted, NotOffered, Faulted), built in one place. Faulted is the kernel's Io, which every claim call answers once the IOMMU has refused the function an access (pcidev::with_bound), so a faulting device ends its bring-up by name rather than panicking its driver, and the virtio capability walk refuses with WalkRefusal<Why>. toyos_pci_claim::describe is the one read of a claim's description, so netstack's I219 path no longer builds the kernel refusal itself. toyos_pci_claim::lend maps a region a peer lent (PciDev::dma_map); its refusals about the region itself (not ordinary memory, already lent, sent without MAP) are Why::Unlendable, since they are the peer's words and not the claim's.

Remaining SyscallError uses (measured at the head)

crate before after why each remains
system/fileserver 255 5 main.rs: the import and a stream pipe's read_nonblock (WouldBlock / Gone), a syscall made and judged on the spot. bounds.rs tests: a Lent::Kernel fixture, which is shm's own type.
sdk/toyos/src/fs.rs 42 5 in code, 14 in tests the import, Error::kernel's parameter and two arms, and namespace.open's NotFound read as NotHeld. The tests list each kernel word.
lib/pci-claim 8 10 in code, 17 in tests the import, Why::of's parameter and four arms, Why::of_lent's parameter and two arms, and KernelRefused::on's parameter, all at the syscalls.
system/hda (new on main) 2 2 the import and the interrupt record's WouldBlock, a syscall judged on the spot as every other driver's irq() is.

libc. sdk/libc does not speak the file protocol: C programs name no file a file server holds (issues/c-programs-name-no-file-a-file-server-holds.md). Its errcode::of maps kernel syscalls it makes itself, which is its boundary, so nothing changes there. When libc gains served paths, I recommend its errno for toyos::fs::Error live in sdk/libc beside errcode, since POSIX compatibility belongs to libc, and not in the protocol crate. That is a recommendation, not something built here.

Checks and evidence

All evidence is in ToyOSOrg/evidence pr-948 at 07b25c6ff6e2 (README).

  • Host tests for the translations: fs::tests::every_refusal_round_trips_through_its_own_code, a_reply_outside_the_protocol_is_malformed (unknown status, the kernel's old wire words, wrong frame types, a link answering a fid call, an over-long link or count), every_kernel_refusal_the_calls_can_see_is_one_named_error, and pci-claim's every_kernel_refusal_a_claim_call_can_see_is_one_named_reason. The fileserver's 58 unit and module tests are green with their expectations moved to the named words.
  • cargo run -- --ci host: exit 0, 86 steps.
  • cargo run -- --build-only: x86_64 exit 0, aarch64 exit 0, x86_64 after merging main exit 0.
  • Negative controls, each a checked patch applied, run and reverted (patches and logs): removing ResourceExhausted from Error::kernel, removing InvalidArgument from Why::of, mapping FAT's Io to NotFound, and reading an unknown status as Io. Each went red (exit 101) in the test named above.
  • QEMU x86_64 at the head (log): exit 0. No QEMU row runs the file-server guest binaries; they ride the T14's shared testcases boot. The rows that reach changed code are nvme_disk_boots (diskserver's NotHeld list and DATA served from it; the boot must be clean) and machine_shutdown_short_stop (the supervisor's Dir::sync before the stop; its judge checks only that nothing panics, not that the sync answered).
  • Metal: the T14 ran the staged testcases file members and app_view on PROCTREECASE at 394dc5e7d: 10/10 passed (orchestrator's run). The review round's changes do not reach those paths on a healthy boot. Why::Faulted is reached only after an IOMMU fault, the client words' codes are never sent, and a stream's pipe refused only by a full handle table. The merge with main brings ipc: a typed payload is exactly one T, and FrameRx refuses a frame past its MAX #942's stricter frame decoding onto every file call; this PR changes no frame layout, and the QEMU boot at the head runs the fileserver through it. So that run stands for what this PR changes.

Review round (evidence)

  • Why::Faulted maps the kernel's Io, which pcidev::with_bound answers every claim call once the IOMMU has faulted the function. I re-checked every word pci-claim's calls can see against the kernel: Gone, ResourceExhausted, InvalidArgument, Io; a PCI description is infallible once claimed. Control neg5 removes that arm and goes red.
  • The client's own words (Gone, NotHeld, Malformed) have no wire code; a status of 29 to 31 is Malformed. Control neg6 reads 29 as Gone again and goes red. The uniqueness assertion is gone: the discriminants are the codes, so the compiler enforces it.
  • A stream pipe refused by the client's own full handle table answers OutOfResources and drops the connection, since the kernel leaves the pipe queued for the next answer. A wrong handle count is Malformed.
  • A served seek past the largest offset is InvalidInput, as the kernel route answers it.
  • Merging main (ipc: a typed payload is exactly one T, and FrameRx refuses a frame past its MAX #942 and the soundserver HDA move) needed one fix: main's new HDA path called KernelRefused::on, which this branch made private. It now calls toyos_pci_claim::describe.
  • cargo run -- --ci host at the head (log): exit 1, 1 of 86 steps red, the build system's llvm::tests::the_caller_s_environment_reaches_neither_the_build_nor_the_key. That is a host defect independent of this branch, which changes nothing under build/. The host's selected developer directory is Xcode.app, whose /usr/bin/git shim honors the test's ambient SDKROOT=/no/such.sdk and fails (SDKROOT=/no/such.sdk git --version reproduces it outside the suite). The same test passed in this PR's first host run. The orchestrator should record it under issues/.
  • Builds at the head: x86_64 and aarch64, exit 0.

Merge of main 4225780 (evidence)

  • Audio leaves soundserver for an hda driver per controller; the matcher takes any prog-if #953 moved HDA into system/hda. The modify/delete conflict on system/soundserver/src/hda.rs resolves to main's deletion; this branch's one hunk there is carried to the new driver, which reads its description through toyos_pci_claim::describe and ends by name on a refusal rather than expect, and maps soundserver's ring through toyos_pci_claim::lend. Control neg7 drops PermissionDenied from Why::of_lent and goes red (101).
  • cargo run -- --ci host: exit 0, 87 steps. --build-only: x86_64 and aarch64 exit 0.
  • QEMU x86_64 nvme_disk_boots, machine_shutdown_short_stop, hda_plays_to_the_controller_that_reaches_an_output: exit 0, 3 passed. The hda row's playing controller maps its ring through lend.
  • The T14 run of the file members stands as before: the merge changes no file-protocol path.

Risks

  • Error::kernel and Why::of panic on a kernel word their calls are not documented to get. I checked the kernel paths (connect_through, HandleQueue::push, sys_shm_create, dup, sys_handle_recv, sys_device_reg, pcidev::with_bound, bar_object, dma_alloc). A kernel change that adds a word will fail these clients loudly; it will not be misreported.
  • The status codes changed, so a fileserver and a client from different builds disagree. Both ship in one signed image; a swap replaces a fileserver with the same image's build.

🤖 Generated with Claude Code

Japabu and others added 2 commits October 15, 2026 12:18
`toyos::fs::Error` is the file protocol's word: each refusal a client can
act on is one named variant with its own wire code, and a reply's status
is read against that table alone, so a status, a frame type or a byte
count no server sends is `Malformed` rather than another answer. The
fileserver's `Volume` interface, the DATA and FAT volumes, the absent
volume and the request bounds all answer it; the format crates' errors
are translated once each (`data::refusal`, `fat::word`), now one-to-one
where the protocol has the word (not a directory, not empty, no space,
too large) instead of folded into kernel words.

A kernel refusal of a call the protocol itself makes (the window, the
connection, the hello's loan, a stream's pipe) is translated once by
`Error::kernel`: `Gone` and `ResourceExhausted` are the only words those
calls get for handles the caller made, and any other fails loudly. A
directory name this process was not given is `NotHeld`; diskserver's
client likewise names a missing block service `NotHeld` instead of
handing the fileserver a kernel `NotFound` to match on.

std's served-file path maps each refusal to its `io::ErrorKind` in one
table and no longer passes kernel words through `to_io_error`. The
supervisor's power-off sync and the app_view guest case read the new
type directly.

`toyos-pci-claim`'s `KernelRefused` carries `Why` (Gone, Exhausted,
NotOffered) instead of a `SyscallError`, and `describe` is its one place
a claim's description is read, so netstack's I219 path no longer builds
the kernel refusal itself.

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

Japabu commented Oct 15, 2026

Copy link
Copy Markdown
Collaborator Author

Review of #948 at 394dc5e7d

Architecture

The ownership is right and the PR does what root CLAUDE.md asks: the file protocol owns its refusals beside its wire types, and the kernel's words are translated where each syscall is made. I checked every remaining SyscallError use. sdk/toyos/src/fs.rs has 5 in code: the import, Error::kernel's parameter and two arms, and reconnect's NotFound→NotHeld. lib/pci-claim has 6: the import, Why::of's three arms, and KernelRefused::on. The fileserver has 5: the import and drain's read_nonblock in main.rs, and the Lent::Kernel fixture in the bounds.rs tests. No Volume signature and no wire field carries a kernel word. Merging main into the branch was clean: none of the 16 files it brought overlaps this branch. Production grows by about 200 lines, most of it the 31-variant table with its docs and std's exhaustive ErrorKind table. Tests grow by about 90 lines. The fileserver's 58 tests were edited in place.

On the client, I traced every call Error::kernel covers through the kernel:

  • SharedMemory::create(2 MiB): sys_shm_create/map_into can only answer ResourceExhausted for this constant size.
  • namespace.open: NotFound, Gone, ResourceExhausted. The name is ≤ MAX_SERVICE_NAME, well under MAX_USER_STR.
  • dup and handle_send: TableFull→ResourceExhausted; queue closed→Gone; queue full→ResourceExhausted.
  • Frame read/write: BrokenPipe→Gone, NoMemory→ResourceExhausted, cancelled()→Gone.
  • TooLarge really is unreachable: every payload is fixed and far below MAX_FRAME_LEN.
  • On the fileserver, sys_pipe answers only ResourceExhausted.

So the claim holds for toyos::fs. It does not hold for pci-claim (finding 1).

Findings

BLOCKER 1: lib/pci-claim/src/lib.rs:184-190 (and the test at :225). Why::of panics on Io, which every claim call answers once the IOMMU has faulted the function.

  • kernel/src/pcidev/mod.rs:1727-1733 with_bound returns SyscallError::Io when function.irq.faulted().
  • config_read, bar_object and dma_alloc all go through it.
  • note_fault (:1972), called from the unit's fault handler, sets that flag. Its own contract says "every call the claim answers refuses from here on".

Failure scenario: an external or hot-plug device is untrusted and its DMA sits behind the IOMMU, and such a device can issue requests whatever its bus-master enable says. Suppose it faults between the claim and the driver's bring-up calls. The next ClaimConfig::read8/read32 (virtio.rs:57,61), Bar::map or Grant::alloc answers Io. The driver (netstack, soundserver, vdagent or compositor) then panics with "which it never answers it". Before this PR the same refusal was reported as KernelRefused { why: Io }.

That is a userland crash caused by external input, and the kernel explicitly reports the failure, so this is not an invariant violation. The PR's Risks paragraph says sys_device_reg, sys_device_bar_map and sys_device_dma_alloc were checked, but with_bound's first line was missed. The unit test then enshrines the bug by listing Io among the "never" words, and neg2 cannot catch it.

Fix: add a named reason, e.g. Why::Faulted ("the IOMMU refused the function an access; the claim answers nothing more"), mapped from Io. Take Io off the test's never-list.

BLOCKER 2: sdk/toyos/src/fs.rs:195-200,210-212,277. The client-side words Gone, NotHeld and Malformed have wire codes and decode from a server's status.

  • The PR body and the test doc at :887 say a status "no server of this protocol sends is refused by name, never read as another answer".
  • No server sends 29, 30 or 31, yet from_code reads them as Gone, NotHeld and Malformed.

Concrete consequence: a file server that answers its hello with status 30 makes Dir::connect return Err(Error::NotHeld). Std's capability() (sdk/std/sys/fs.rs:426-427) then caches the prefix as absent for the life of the process, so route sends that subtree to a shallower capability or to the kernel VFS. The old code had the same hole through NotFound. This PR closed that path and reopened it through NotHeld.

Fix: keep the client's own words out of the wire table. For example, generate ALL and the codes only for the server's refusals, and have from_code answer None for anything else, so status 29–31 becomes Malformed. Add those three codes to a_reply_outside_the_protocol_is_malformed. That is the negative control that would have caught this.

BLOCKER 3 (PR description, for the orchestrator): the QEMU coverage claim for lent_device_memory_refused is false.

  • tests/guest/rust-tests/src/bin/lent_device_memory_refused.rs uses only the toyos::fs::WINDOW_BYTES constant, SharedMemory::lent and PciDev::describe directly.
  • It reaches no hello, no fileserver and no pci-claim code. The body's "(a lent window refused at the hello)" describes code this test does not run.
  • The other two rows do reach changed code. nvme_disk_boots covers diskserver's NotHeld and data_partitions. machine_shutdown_short_stop covers the supervisor's Dir::sync before the stop. Even so, must_be_clean would not notice a "would not sync" line, so that row checks only that nothing panics.
  • Correct the body. No further run is needed for this finding. Once blockers 1 and 2 are fixed, host tests cover them and --ci host is the check to rerun.

SUGGESTION 4: sdk/toyos/src/fs.rs:609 and sdk/toyos/src/ipc.rs:258. A stream's pipe that the client's own full handle table refuses reads as Malformed.

  • recv_handles_exact turns ResourceExhausted from sys_handle_recv into None, and stream maps None to Malformed ("the file server answered outside its protocol"). That blames the server for a local limit OutOfResources exists to name.
  • The kernel also leaves the refused batch queued, so the next STREAM answer would take the previous pipe.
  • Both behaviours predate this PR. Either distinguish the two failures (an Err from recv_handles → Error::kernel, a wrong count → Malformed) or record the queued-batch desync under issues/.

SUGGESTION 5: sdk/toyos/src/fs.rs:881. The code-uniqueness assertion is checked by the compiler.
Explicit discriminants on a #[repr(u8)] enum cannot repeat (E0081), so the assertion can go. The kernel-word tests (:912, and pci-claim's) restate their match. neg1 and neg2 only show that deleting an arm fails a test listing the arms. The real claim, which words each call can see, is established by reading the kernel, and blocker 1 shows where that reading went wrong. Keep the tests short and accurate. Don't count them as evidence for that claim.

SUGGESTION 6: sdk/std/sys/fs.rs:691-693. A served seek past MAX_FILE_BYTES is now FileTooLarge, while the kernel route still answers InvalidInput for the same seek. Also, the served readdir that keeps growing changed from OutOfMemory to Interrupted (:808), which matches the kernel route now but is missing from the body's list of changed kinds.

The questions asked

  • (3) Decoder. An unknown status, an unknown frame type, a LINK answering a fid call, a count past what was asked (counted), a short or malformed listing (for_each_entry) and a missing stream handle are all refused as Malformed, and none of them panics. The one exception is blocker 2. for_each_entry's index arithmetic cannot overflow, since a listing is at most the window. On the server, the expect("a request comes after the window") in path/put holds: answer drops a client that asks before its hello (main.rs:565).

  • (4) std table. The match is exhaustive, with no wildcard. These changes are right:

    • NotEmpty→DirectoryNotEmpty
    • NoSpace→StorageFull
    • NameTooLong/InvalidName→InvalidFilename
    • Loop→FilesystemLoop
    • CrossesDirectories→CrossesDevices, as std's own rename refusal already says
    • FileTooLarge, IsADirectory, NotADirectory

    The limits mapping to QuotaExceeded is defensible ("some other kind of quota"). No served error used to produce Other except Io, which still does, and nothing in the tree branches on Other, InvalidInput or OutOfMemory from a file call. sftp branches only on NotFound/PermissionDenied, and both are unchanged.

  • (5) diskserver. NotHeld is correct, and its only consumer is data_partitions. Mapping DiskError::Gone to Io is also correct: the block service ending under a DATA read is a device that did not do it, from where a file client stands. Before, std told the user the file server was gone.

  • (6) pci-claim. describe is now the one read of the description for every pci-claim consumer: i219 directly, and virtio's negotiate for the other four. KernelRefused::on becoming private is what makes that hold. Apart from blocker 1, Why is right.

  • (7) Tests. I checked the moved fileserver expectations against POSIX meanings: require_parent_dir→NotADirectory, rmdir non-empty→NotEmpty, rename onto a directory→IsADirectory, a dead node→Stale. All are right. The four controls are real, each red with exit 101 at b50d5cb3e. As blocker 1 shows, neg2 points the wrong way.

  • (8) libc. I agree. The errno for toyos::fs::Error belongs in sdk/libc beside errcode once C programs reach served paths, since POSIX compatibility is libc's. It does not belong in the protocol crate.

  • (9) Evidence. ci-host.log is exit 0 with 86 steps, all green, at b50d5cb3e. The FAILED lines in it are the designed loom controls, each of which "reached a verdict". The new tests pass at ci-host.log:10391,11486,11491,11501,11574. A host run at b50d5cb3e is valid for the merged head because the merge touched no overlapping file, and an x86_64 build at the merged head exists. The T14 rows are pending and the orchestrator owns them.

  • ipc: a typed payload is exactly one T, and FrameRx refuses a frame past its MAX #942. Nothing here becomes redundant. ipc: a typed payload is exactly one T, and FrameRx refuses a frame past its MAX #942's only fileserver hunk is the drop message at main.rs:480, which this branch doesn't touch, so a textual conflict is unlikely. After ipc: a typed payload is exactly one T, and FrameRx refuses a frame past its MAX #942, a Reply frame of the wrong length becomes IpcError::Malformed, and transport already maps that to Error::Malformed and drops the connection. counted judges values inside the payload, not frame lengths, so it is still needed.

SEND BACK

Japabu and others added 3 commits October 15, 2026 13:10
…r cross the wire

`toyos-pci-claim`'s `Why::of` panicked on `Io`, which the kernel answers
every claim call once the IOMMU has refused the function an access
(`pcidev::with_bound`): an untrusted device faulting during bring-up would
have ended its driver. It is `Why::Faulted` now. The calls' other words,
checked against the kernel again, are `Gone`, `ResourceExhausted` and
`InvalidArgument`; a PCI description is infallible once claimed.

`toyos::fs::Error`'s client-only words (`Gone`, `NotHeld`, `Malformed`)
have no wire code: `code` answers `None` for them, `from_code` knows only
the server's refusals, so a status of 29 to 31 or any other no server
sends is `Malformed`, and `Reply::refused` of a client word fails loudly.
The discriminants are the codes, so the compiler holds each to its own and
the uniqueness test goes.

A stream whose pipe the client's own full handle table refused answers
`OutOfResources`, not `Malformed`, and drops the connection, since the
kernel leaves the refused pipe queued for the next stream's answer. A
served seek past the largest offset is `InvalidInput`, as the kernel's
route answers it.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Main's HDA path built the description's refusal with
`KernelRefused::on`, which this branch made private so that
`toyos_pci_claim::describe` is the one place a claim's description is
read and translated.

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

Japabu commented Oct 20, 2026

Copy link
Copy Markdown
Collaborator Author

Re-review of #948 at a597bb04a

This round checks the three round-1 blockers, the suggestions the implementer took and the merge fix. Since 394dc5e7d the branch adds the review commit 33bc10b58, which touches lib/pci-claim, sdk/std/sys/fs.rs and sdk/toyos/src/fs.rs, plus a merge of main at 0e6716fc0 and a597bb04a, which changes one line in soundserver.

Architecture

Nothing changed shape. The round only finishes the translation at the two boundaries: the kernel's words become Why and Error, and the protocol's words become a status. Production code stays small. Why gains one variant and one arm. The error macro splits into wire/local arms, and the client's own words leave the code table.

Blockers from round 1

1. Why::Faulted: resolved. I re-traced every kernel call pci-claim makes:

  • describe, a device read, answers the minted description.
  • config_read goes through sys_device_reg_read.
  • map_bar is sys_device_bar_map plus shm_map.
  • dma_alloc is sys_device_dma_alloc plus shm_map.

The words they can answer:

  • Io: pcidev::with_bound (kernel/src/pcidev/mod.rs:1733) answers it once note_fault has run. It is now Faulted (lib/pci-claim/src/lib.rs:192).
  • Gone: on_function (kernel/src/syscall/device.rs:178) and map_into.
  • ResourceExhausted: the grant quota, AddressesExhausted, SharedMemObject::create, a full handle table on install, and alloc_and_map.
  • InvalidArgument: a bad BAR index or grant size, and config_window.

The rest cannot reach a correct holder:

  • NotSupported (a claim of another class) and PermissionDenied (the rights of a handle the kernel minted) cannot happen to a holder that has the claim as the supervisor minted it.
  • BadAddress points at the SDK's own stack.
  • BadHandle, Stale and WrongType end the caller through handle_fault. They are never returned.

None of these can be driven by device or peer input, so no reachable panic remains. The test's never-list no longer includes Io. neg5 (948/r2/neg5.*, at 33bc10b58) removes the arm and fails the test with exit 101 at lib.rs:192.

2. Client-only words off the wire: resolved.

  • code() answers None for Gone, NotHeld and Malformed.
  • from_code matches only the 28 server codes, with _ => None.
  • result() turns None into Malformed (sdk/toyos/src/fs.rs:296-301).
  • a_reply_outside_the_protocol_is_malformed asserts the old 29–31, the new discriminants 0x80–0x82, u64::MAX and a kernel word.
  • neg6 (948/r2/neg6.*) reads 29 as Gone again and fails with exit 101 at fs.rs:944. That is the control I asked for.

The panic in Reply::refused (fs.rs:290-293) is a server-side invariant on the server's own code, and no client input can reach it.

  • Reply::refused has no caller outside system/fileserver.
  • The fileserver's Error values come only from:
    • its named server words;
    • data::refusal, where DiskError::Gone becomes Io (data.rs:154);
    • fat::word;
    • bounds;
    • resolve;
    • Error::kernel around pipe_pair (main.rs:704).
  • sys_pipe answers only ResourceExhausted (kernel/src/syscall/ipc.rs:50), so that path yields OutOfResources.
  • The hello's refusal is TooManyConnections.
  • No fileserver code names Gone, NotHeld or Malformed (I grepped). A disconnected client only makes try_send fail, which drops the client.
  • No client request is turned into a client-only word.

3. PR body: resolved. The QEMU row now states plainly that no row runs the file-server guest binaries, what nvme_disk_boots and machine_shutdown_short_stop do reach, and that the judge does not check the sync's answer.

Suggestions the implementer took

  • Stream pipe (fs.rs:628-647). recv_handles is used directly now.

    • Ok(1) is the pipe.
    • Any other count closes what arrived, drops the connection and answers Malformed.
    • Err drops the connection and goes through Error::kernel.

    sys_handle_recv (kernel/src/syscall/ipc.rs:397-428) answers a peer's input only with these:

    • ResourceExhausted, from the client's own table. It becomes OutOfResources.
    • 0 for nothing queued or a closed queue. It becomes Malformed.
    • InvalidArgument, when the width is above the cap. A peer cannot cause it: dispatch caps a send at MAX_TRANSFER_HANDLES (dispatch.rs:459), and the cap here is MAX_TRANSFER_HANDLES.

    So no panic is reachable from the server. Dropping the connection removes the desync that would otherwise leave the pipe queued for the next answer.

  • Served seek (sdk/std/sys/fs.rs:690-694). A seek past either end is InvalidRequest, which maps to InvalidInput (:378), the same kind as the kernel route.

  • readdir kind change. It is now in the body's list.

Merge fix

system/soundserver/src/hda.rs:199 now calls toyos_pci_claim::describe(&dev)?, and From<KernelRefused> for Refusal carries it. KernelRefused::on has no caller outside the crate. No consumer matches on Why exhaustively, so Faulted affects nothing outside it. toyos_pci_claim::describe is still the only place a claim's description is read.

Evidence

Findings

No blockers.

SUGGESTION A — PR body, Metal paragraph. The reuse justification covers only the review round. It should also name the merge: #942's decode is now on every file call, and the branch's frames are unchanged, so the T14 run stands. This belongs to the orchestrator.

SUGGESTION B — sdk/toyos/src/fs.rs:121-122. "and past them, the client's words', so the compiler holds every one to its own" does not parse well. Something like "The discriminants are the wire codes; the client's words sit past them" is enough, since E0081 needs no explanation.

SUGGESTION C — sdk/std/sys/fs.rs:378 with :694. A seek past either end is refused locally, but the user still sees "the file server takes no such request" when no server was asked. Either give std's local seek its own const_error! with the same InvalidInput kind, or leave it.

LAND

#953 moved the HDA driver out of soundserver into system/hda. The
modify/delete conflict on system/soundserver/src/hda.rs resolves to the
deletion: this branch's one hunk there (the description read through
toyos_pci_claim::describe) is carried to the new driver.

The new driver is a pci-claim caller main added, so it takes this
branch's rule rather than a second path: its description is
toyos_pci_claim::describe, ended by name instead of an expect, and the
ring soundserver lends is mapped through toyos_pci_claim::lend, since
KernelRefused::on is private here. lend names the region's own refusals
Why::Unlendable (not ordinary memory, already lent, or sent without
MAP): those are the peer's words about its region, not the claim's, so
Why::of's panic on an unexpected word would misreport them.

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

Japabu commented Oct 20, 2026

Copy link
Copy Markdown
Collaborator Author

Merged origin/main 4225780 (#953, #954) at 7b77570.

Resolved. The only conflict was modify/delete on system/soundserver/src/hda.rs: main deleted it (#953 moved HDA to system/hda), and this branch had changed one hunk there (the description read through toyos_pci_claim::describe). I took main's deletion and moved that hunk into the new driver. Everything else merged cleanly.

#948's rule applied to main's new pci-claim caller (system/hda/src/main.rs).

  • dev.describe().expect(..) became toyos_pci_claim::describe. A refusal now ends the driver with a named message instead of panicking.
  • The ring soundserver lends was mapped with KernelRefused::on, which is private on this branch, so the merge would not have compiled. It now goes through a new toyos_pci_claim::lend. The kernel's dma_map refuses a region a peer sent with InvalidArgument (not ordinary memory, or already lent) or PermissionDenied (sent without MAP). Those describe the peer's region, not the claim, so Why::of_lent names both Why::Unlendable and passes every other word to Why::of. Why::of would have panicked on PermissionDenied.
  • The driver's irq() WouldBlock match stays as it is, the same as every other driver's on this branch.

Checks at 7b77570 (evidence):

The PR body has been updated to include this.

@Japabu

Japabu commented Oct 20, 2026

Copy link
Copy Markdown
Collaborator Author

Review: merge resolution at 7b77570 (main 4225780, #953 and #954)

Scope: only what the merge resolution added, from git show --remerge-diff 7b77570b9. The earlier LAND at a597bb0 still holds and I have not reopened anything it settled.

Architectural assessment

The resolution is correct and minimal. system/soundserver/src/hda.rs had a modify/delete conflict, and it resolves to main's deletion. The branch's one hunk in that file moved into the new caller, system/hda/src/main.rs:165-171. #953 added system/hda as a pci-claim caller, and it now follows #948's rule rather than introducing a second path. Without that change, the KernelRefused::on call that main added at system/hda/src/main.rs:260 would not compile, because on is private on this branch.

toyos_pci_claim::lend belongs in pci-claim. dma_map is a claim call, and pci-claim is the one place that turns a claim call's SyscallError into Why. soundserver, which owns the region, cannot own the kernel's refusal of the driver's own map call. Translating in hda instead would bring back the per-caller SyscallError handling that #948 removes.

A separate reason is justified. For dma_map, InvalidArgument means "not kernel-owned RAM" or "this claim already maps it" (kernel/src/pcidev/mod.rs:1856-1862). Why::NotOffered ("the function has no such thing to give") would misreport both. PermissionDenied comes from a region handle without MAP (kernel/src/syscall/device.rs:260-262). Why::of treats that word as a broken invariant and panics on it, but here it is an ordinary refusal of the peer's input.

Can a reachable kernel word still panic Why::of? No. I traced sys_device_dma_map and the dispatch arm:

  • PermissionDenied: the claim handle without WRITE, or the region handle without MAP.
  • ResourceExhausted: lend_room.
  • InvalidArgument: not RAM, or already mapped.
  • Gone: on_function.
  • Io: with_bound on a faulted function.
  • BadHandle, Stale and WrongType on either handle never return a word. They go through handle_fault, which ends the caller.
  • BadAddress is possible only for the output pointer. The SDK passes a stack local there, so a BadAddress is a genuine invariant violation and panicking on it is right.

Size: production +31 lines, test +11 lines (including the new test's doc comment and blank line), in lib/pci-claim. hda changes +7/-1. The 365 deleted lines are main's deletion.

Findings

No blockers.

SUGGESTION lib/pci-claim/src/lib.rs:212 — PermissionDenied is ambiguous in lend. It also covers a claim handle without WRITE, which is a supervisor invariant that Why::of reports loudly for every other claim call. Under lend it would be reported as Unlendable ("came without the right to map it"), which blames the peer. That is not a crash and the supervisor always mints WRITE, so the gap is small. Optional fixes: one sentence on Unlendable, or accept it as is.

Note, outside this merge, for the orchestrator. In #953's code on main, sys_device_dma_map looks up the peer-sent region with get, not get_sent. If soundserver sends a handle that is not shared memory, the kernel ends the hda process with a handle fault instead of returning InvalidArgument. It is a kernel-side containment question that came with #953, the peer is a system service, and #948 neither caused nor worsened it. It may deserve an issue record. It does not block this PR.

Tests and evidence

  • a_lent_region_the_function_cannot_reach_is_unlendable (lib/pci-claim/src/lib.rs:255-264) is a hermetic host test that mirrors the settled Why::of test. Its claim is that the two peer words become Unlendable and every other word goes to Why::of, including its loud failure.
  • Control neg7 (evidence pr-948 @ fbb82cf19, 948/r3/neg7.patch/.log) removes PermissionDenied from of_lent. The test then fails at the exact panic the merge comment describes, with exit 101 at 7b77570. That is the right defect to expose.
  • cargo run -- --ci host passed all 87 steps, exit 0. --build-only exited 0 on both x86_64 and aarch64. hda_plays_to_the_controller_that_reaches_an_output and two boot tests passed on QEMU x86_64. All of these ran at 7b77570, the merge head under review, so no reused evidence needs checking.
  • The QEMU HDA test is real evidence that lend maps soundserver's ring on the success path. No guest test reaches the refusal arms. The host test plus inspection of the kernel words covers them, so nothing further is needed.

The merge commit message and PR comment 6300158110 match the diff and the kernel code. The note that hda's irq() WouldBlock match "stays as it is" agrees with the other drivers on this branch.

LAND

@Japabu
Japabu marked this pull request as ready for review October 20, 2026 20:03
@Japabu
Japabu added this pull request to the merge queue Oct 20, 2026
Merged via the queue into main with commit c7ba1fa Oct 20, 2026
8 checks passed
@Japabu
Japabu deleted the wt/toyos-fserr branch October 20, 2026 20:43
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