Repository navigation
kernel, supervisor: a claim's firmware window, granted at its mint by the signed image's row, and every PCI claim's mappings ending with it - #1137
Conversation
Review r1 of #1137 at 4be4b0fArchitectureThe authority path is right. A grant needs a Four things need fixing:
Net: +680/−3. About 140 of those lines are tests: userbound 60, opregion 35, manifest 15, judge 27. Required
Optional
OpRegion facts against rev 0.5These match the specification as cited:
Tests and evidence
SEND BACK |
4be4b0f to
990eb56
Compare
|
Review r1, addressed at Required
Optional
Host CI: exit 0, 108 steps green, at |
Review r2 of #1137 at 990eb56ArchitectureThe r1 shape is in place:
I found no way for userland to panic the kernel on these paths. An in-flight Two things block landing. First, the pushed commit is not the rebase the PR describes. Second, the revocation on move and on release has two races that defeat the property the PR claims. The diff against Required
Optional
r1 items
Tests and evidence
SEND BACK |
…nd no aperture is handed past the revoke it should meet - **A move takes what the claim handed out under the lock that moves it.** IPC's send and a spawn's commit take the claim's `handed` list under the sender's process lock, before the claim is visible to its receiver, and revoke it once that lock is let go. A send that is refused gives it back. So a receiver that takes the claim and maps a BAR before the sender's revoke runs keeps its mapping (r2 required 2). - **An aperture is installed only while the caller still holds the claim.** `bar_map` and `firmware_window` install the new handle under the caller's process lock, and only if its table holds this claim at `handle`. A release or a move takes the claim out under that lock and revokes after it, so every aperture either met that revoke or is refused `Gone` (required 3). - `pcidev::revoke_handed(number)` is the by-number form #934's departure calls from its refresh loop. `Handed::revoke` documents its terms: thread context only, never the hot-plug interrupt, never with `MACHINE` held. The false clause about a departure is gone (required 4). - The BAR issue states what `bar_map_again` now reads and what it still does not (required 5). - `bar_map_again` sends its claim to itself over a connection. A BAR handle kept past the move maps `Gone`, and the new holder's fresh BAR maps and reads the device. The judge requires both lines (optional 5). - The window fence is decided from `Machine::identities` in `pcidev::claim`, before the function is touched, so a refused window no longer resets it (optional 1). An unknown window bit is `InvalidArgument` in `sys_device_claim` (optional 2). - `Bound::firmware` is `FirmwareWindows`, and `FirmwareWindow::ALL` is the one list of windows (optional 8). The ABI doc says what the kernel checks (optional 4), and the AArch64 stub keeps only the direct map's reason (optional 3). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
990eb56 to
2831e6e
Compare
|
Review r2, addressed at Required
Optional, taken
🤖 Generated with Claude Code |
…nd no aperture is handed past the revoke it should meet - **A move takes what the claim handed out under the lock that moves it.** IPC's send and a spawn's commit take the claim's `handed` list under the sender's process lock, before the claim is visible to its receiver, and revoke it once that lock is let go. A send that is refused gives it back. So a receiver that takes the claim and maps a BAR before the sender's revoke runs keeps its mapping (r2 required 2). - **An aperture is installed only while the caller still holds the claim.** `bar_map` and `firmware_window` install the new handle under the caller's process lock, and only if its table holds this claim at `handle`. A release or a move takes the claim out under that lock and revokes after it, so every aperture either met that revoke or is refused `Gone` (required 3). - `pcidev::revoke_handed(number)` is the by-number form #934's departure calls from its refresh loop. `Handed::revoke` documents its terms: thread context only, never the hot-plug interrupt, never with `MACHINE` held. The false clause about a departure is gone (required 4). - The BAR issue states what `bar_map_again` now reads and what it still does not (required 5). - `bar_map_again` sends its claim to itself over a connection. A BAR handle kept past the move maps `Gone`, and the new holder's fresh BAR maps and reads the device. The judge requires both lines (optional 5). - The window fence is decided from `Machine::identities` in `pcidev::claim`, before the function is touched, so a refused window no longer resets it (optional 1). An unknown window bit is `InvalidArgument` in `sys_device_claim` (optional 2). - `Bound::firmware` is `FirmwareWindows`, and `FirmwareWindow::ALL` is the one list of windows (optional 8). The ABI doc says what the kernel checks (optional 4), and the AArch64 stub keeps only the direct map's reason (optional 3). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
2831e6e to
adc35d8
Compare
…se's display row is granted its window, and the PSR restart point is the durable brief Rebased onto main c81fc83. #1062 took syscall 160 and #1116 took 161-163, so SYS_DEVICE_FIRMWARE_WINDOW is 164, the first commit's message notwithstanding. The profile has 166 bins, and the guard names 164. #1149 runs tgl-display in benchcase "as images/system.toml runs it", so its row carries the same `firmware = ["igd-opregion"]` grant. The display track's PSR restart line now links the clean-room brief at its durable evidence commit. Only facts.md there is an implementer's input. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Round 3, at
Ready for the focused review of r2's fixes and this round. 🤖 Generated with Claude Code |
Review r3 of #1137 at adc35d8ArchitectureThe shape r2 accepted is unchanged. The r2 fixes put the move's revoke decision under the lock that does the move, and they gate every aperture install on the claim still being held. That is the smallest design that closes both races. It adds no new state: one r2 required items
Optional r2 items taken
I found no userland path that panics the kernel. ASLS arithmetic is u64 from a u32. The window is page-aligned by construction, so Required
Optional
Tests and evidence
SEND BACK (one comment-only fix, required 1) |
… the signed image's row, and every PCI claim's mappings ending with it
The display driver needs the IGD OpRegion live. Its VBT holds the panel's
PSR and backlight settings, and its mailboxes are firmware's channel to the
driver. It is firmware memory outside the claim's BARs, which root
CLAUDE.md allows only as a privilege the signed image's system.toml grants
to a program it ships.
- **Manifest.** `firmware = ["igd-opregion"]` on a `drives` row of the
image's manifest. The render refuses an unknown window and one on a row
with no `drives`. Packages have no spelling for it.
- **Mint.** The supervisor mints the function's claim with the windows its
row names, carried in the mint's second selector word
(`DeviceRequest::Function(arrival, FirmwareWindows)`). A window exists
only from the mint, by construction: there is no call that adds one
later. A window the function has none of refuses the mint, so a row
naming one fails its start by name. The IGD OpRegion goes only to an
Intel function of class 03h, a bug fence on the row.
- **Window.** SYS_DEVICE_FIRMWARE_WINDOW (164) answers a granted claim's
window, by IGD OpRegion specification rev 0.5:
- It reads ASLS (config FCh, §5.1.2).
- `arch::opregion::window` decides the 8 KB at it in one pass with
`toyos_userbound::window` (§3.1.2's size; §2.3's firmware-reserved
NVS). Every page must be reserved or ACPI NVS in firmware's map,
inside the direct map, none memory the kernel hands out, and none a
BAR or driven window. Every 2 MiB leaf must be cacheable in the
direct map.
- It reads the header there and holds it to "IntelGraphicsMem" and
8 KB (§3.1.1-3.1.2).
- A refusal comes back named (`WindowRefused`). The aperture is
cacheable, and its handle carries MAP and WRITE alone.
- AArch64 has no such machine, and its direct map no such reach.
- **Every mapping made through a claim ends with it.** The claim records
each aperture it hands out, BARs and firmware windows alike.
`pcidev::revoke_handed`, one function, takes each back from every space
that maps it, through the revoke an aperture's last handle already does.
It runs at the claim's last handle (its release, its holder's death) and
when the claim moves, by IPC or a spawn's endowment. So no process keeps
a mapping of a function its claim no longer gives it.
`issues/a-bars-mapping-outlives-the-claim-it-was-asked-on.md` keeps its
remaining case, a function's departure, open.
- **tgl-display** maps the window, validates a copy with
`opregion::OpRegion::parse`, says OVER and MBOX raw, and lets the mapping
go.
- MBOX needs only bit 3, the VBT's. The bits the 2016 revision calls
reserved may name a later revision's mailboxes.
- The VBT is handed out raw: Intel has published no layout for it.
- `bar_map_again` keeps a BAR's handle past its claim's end and requires
its map to be refused, `Gone`.
- metalcase runs tgl-display with the grant, and the T14 row
`igd_opregion_window` requires the window read and valid.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…nd no aperture is handed past the revoke it should meet - **A move takes what the claim handed out under the lock that moves it.** IPC's send and a spawn's commit take the claim's `handed` list under the sender's process lock, before the claim is visible to its receiver, and revoke it once that lock is let go. A send that is refused gives it back. So a receiver that takes the claim and maps a BAR before the sender's revoke runs keeps its mapping (r2 required 2). - **An aperture is installed only while the caller still holds the claim.** `bar_map` and `firmware_window` install the new handle under the caller's process lock, and only if its table holds this claim at `handle`. A release or a move takes the claim out under that lock and revokes after it, so every aperture either met that revoke or is refused `Gone` (required 3). - `pcidev::revoke_handed(number)` is the by-number form #934's departure calls from its refresh loop. `Handed::revoke` documents its terms: thread context only, never the hot-plug interrupt, never with `MACHINE` held. The false clause about a departure is gone (required 4). - The BAR issue states what `bar_map_again` now reads and what it still does not (required 5). - `bar_map_again` sends its claim to itself over a connection. A BAR handle kept past the move maps `Gone`, and the new holder's fresh BAR maps and reads the device. The judge requires both lines (optional 5). - The window fence is decided from `Machine::identities` in `pcidev::claim`, before the function is touched, so a refused window no longer resets it (optional 1). An unknown window bit is `InvalidArgument` in `sys_device_claim` (optional 2). - `Bound::firmware` is `FirmwareWindows`, and `FirmwareWindow::ALL` is the one list of windows (optional 8). The ABI doc says what the kernel checks (optional 4), and the AArch64 stub keeps only the direct map's reason (optional 3). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…se's display row is granted its window, and the PSR restart point is the durable brief Rebased onto main c81fc83. #1062 took syscall 160 and #1116 took 161-163, so SYS_DEVICE_FIRMWARE_WINDOW is 164. The profile has 166 bins, and the guard names 164. #1149 runs tgl-display in benchcase "as images/system.toml runs it", so its row carries the same `firmware = ["igd-opregion"]` grant. The display track's PSR restart line now links the clean-room brief at its durable evidence commit. Only facts.md there is an implementer's input. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…, and a BAR handle's life is in the ABI - `pcidev::revoke_handed` says only what it is: by number, for a caller with no binding, in thread context and never under `MACHINE`. - `device_bar_map` says a BAR's object lives no longer than the claim's hold: an end or move takes its mappings back, and a kept handle maps `Gone`. - `sys_device_claim` names the window bits its selector carries and refuses. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
adc35d8 to
577298f
Compare
|
Review r3, addressed at
🤖 Generated with Claude Code |
…d out Resolutions: - sdk/abi/src/syscall.rs: main's SYS_DEVICE_FIRMWARE_WINDOW keeps 164; SYS_DEVICE_SET is 165 and SYS_DEVICE_SET_READ 166, with 168 profile bins. - issues/a-bars-mapping-outlives-the-claim-it-was-asked-on.md: main's, which names this PR's departure as what is left; the next commit wires it. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
#1137 revokes what a claim handed out at its last handle and when it moves. A function that leaves from behind a hot-plug slot while its claim is still held kept its BAR mappings until that claim was released, and the next arrival behind the port may be assigned the same addresses. `slot::refresh`'s per-record loop, which consumes each departure, now takes the departed claim's `Handed` there, under `REFRESHING`, where it can only be the departed claim's: an arrival on the record is listed in a later refresh, after §6.6.1's wait. It is revoked once `REFRESHING` is let go, on the reader's thread with no `MACHINE` held, because the revoke's shootdown waits on other CPUs and `REFRESHING` disables preemption. That is `revoke_handed`'s take and revoke split around the lock; never the slot's interrupt. `pciholder` maps its BAR 0 before the pull and keeps the handle; once the function has left, mapping that handle again answers `Gone`, which `a_function_arrives_and_leaves` now requires (`kept=Some(Gone)`). The BAR issue's departure item and the track's gap are closed. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Draft. On
mainatc81fc83eb(#1132, #1134, #1138, #1062, #1116, #1148 and #1149 included), withopregion()wired into tgl-display over stages 2 and 3.Problem
The display driver needs the IGD OpRegion: its VBT holds the panel's PSR and backlight settings, and its mailboxes are firmware's live channel to the driver. It is firmware memory outside the iGPU's BARs. Root
CLAUDE.mdallows that only as a privilege the signed image'ssystem.tomlgrants to a program it ships.Separately, a BAR's mapping outlived the claim it was asked on (
issues/a-bars-mapping-outlives-the-claim-it-was-asked-on.md). The window would have had the same defect.What this PR does
firmware = ["igd-opregion"]on adrivesrow. The render refuses an unknown window, and one on a row withoutdrives. Installed packages have no spelling for it.drivesrow's claim with the windows the row names, carried in the mint's second selector word (DeviceRequest::Function(arrival, FirmwareWindows)).pcidev::claimdecides it from the enumerated identity before the function is touched. A window the function lacks refuses the mint, so the row fails its start by name, and the function is not reset. An unknown window bit isInvalidArgument.DEVICEright. The supervisor sets a window only from the signed image's own row.SYS_DEVICE_FIRMWARE_WINDOW(164).arch::opregion::windowdecides the 8 KB at it in one pass withtoyos_userbound::window. Every page must be reserved or ACPI NVS in firmware's map (§2.3), inside the direct map, and none memory the kernel hands out, a BAR or a driven window. Every 2 MiB leaf must be cacheable in the direct map.WindowRefused).NotFirmware): the direct map there does not reach firmware's reserved memory. An Intel display card in a slot could ask.handedlist under the sender's process lock, before the receiver can see the claim, and revokes it once that lock is let go. A refused send puts it back. So a receiver that maps a BAR before the sender's revoke runs keeps its mapping.handle, under that same lock. A release or move takes the claim out under it and revokes after. So every aperture either met that revoke or is refusedGone, including one a sibling thread asks for across the release or move.pcidev::revoke_handed(number)revokes by function number, for a caller with no binding, such as PCI root bridges declared by acpiserver, device lifecycle and native hot-plug #934's departure from its refresh loop. It must run in thread context and never withMACHINEheld, because the shootdown waits on other CPUs.bound, which is let go before any revoke. Each revoke then takes the aperture'sHeld, then the spaces that map it. A move takesboundinside the sender's process lock and the claim'sHeld, and revokes only after letting all of them go.metalcaseandbenchcaserun tgl-display with the grant, asimages/system.tomldoes. On the T14, metalcase's rowigd_opregion_windowrequires the window read and valid.bar_map_again(QEMU) keeps a BAR handle past its claim's move, by sending the claim to itself over a connection, and past its end. It requires both maps refusedGone, and requires the new holder's fresh BAR to map and read the device.Decisions
An observation: could acpiserver's
acpi_accessbecome a window?Not as one mechanism for both, on present facts. acpiserver's accesses are decided one at a time because the AML chooses each address at evaluation:
A mapped window can apply none of these after the mapping. A narrower simplification is possible: a window over every reserved and ACPI NVS range, for the AML's ordinary memory accesses, with registers, configuration and ports still mediated. That would remove a syscall per NVS access, but it splits one policy across two mechanisms, and the gain is unmeasured. So I am not proposing it now. It is worth measuring if acpiserver's access rate ever matters.
Checks
At
adc35d850(the head577298faddiffers only in doc comments and commit messages). Evidence: https://github.com/ToyOSOrg/evidence/blob/b9d05ee6d839cb88868ca8e4aed541008371ef96/r6/README.mdcargo run -- --ci host: exit 0, 109 steps green. An earlier run at this head died of a full shared disk (ENOSPC); the rerun followed once space was freed.bar_map_again(QEMU, x86_64): PASS, with the move and end lines.2831e6e52: one metalcase boot, rc=0, judged by hand from the readback (r4).igd_opregion_windowanddisplay_output_presentsboth PASS.IGD OpRegion, 8192 bytes at offset 0x18 of its pages; OVER 02010000, MBOX 0000001d, VBT mailbox 6144 bytes.bar_map_againfails at the move assertion (r3);Risks
metalcaseon the T14 runs tgl-display, so its other rows boot with stage 1's watermarks.🤖 Generated with Claude Code