diff --git a/issues/a-class-claims-call-in-flight-acts-after-the-claims-release.md b/issues/a-class-claims-call-in-flight-acts-after-the-claims-release.md new file mode 100644 index 00000000000..62367bf3cee --- /dev/null +++ b/issues/a-class-claims-call-in-flight-acts-after-the-claims-release.md @@ -0,0 +1,53 @@ +--- +status: open +kind: defect +opened: 2026-10-08 +--- + +# A class claim's call in flight acts after the claim's release + +A claim on a class this kernel still drives — the framebuffer, HDA audio, +virtio-sound — is a per-class flag (`device::TAKEN`), and a call on one checks +the handle and then acts with nothing held across the two: + +- `SYS_DEVICE_REG_READ` and `SYS_DEVICE_REG_WRITE` resolve the claim to + `RegTarget::Hda` or `RegTarget::VirtioSound` under the process-data lock + (`sys_device_reg`, `kernel/src/syscall/device.rs`) and reach + `drivers::hda::reg_write` or `drivers::virtio_sound::reg_write` after it; +- `SYS_GPU_PRESENT`, `SYS_GPU_SET_CURSOR`, `SYS_GPU_MOVE_CURSOR` and + `SYS_GPU_SET_RESOLUTION` ask `holds_claim` and then call `crate::gpu` + (`kernel/src/syscall/dispatch.rs`). + +The flag clears when the claim's last handle goes (`Claim`'s drop, +`kernel/src/device.rs`) and the next claim sets it again. So a call whose +claim a sibling thread closed after the check, and whose class another process +claimed before the act, writes a register of the audio controller, or presents +on the display, that the second process now holds. + +A claim on a PCI function, an ISA function or the ACPI fixed hardware does not +have this: its calls borrow what names the device from the claim under the +lock its release takes it with (`DeviceClaim::pci`, `DeviceClaim::isa`). That +borrow is no fix to copy here as it stands: `SYS_GPU_SET_RESOLUTION` allocates, +and the claim's lock is a spinlock. + +**A poll has the same shape on these classes.** The watch a poll on an audio +claim or the mouse's registers on is the class's one `static` +(`drivers::AUDIO_WATCH`, `mouse::WATCH`, by `ops::read_watch`), and +`inbox::arm` registers on it with nothing held. A sibling thread's close +between the resolve and the registration lets the poll land after the close +answered the watch's polls (`ops::close`, and for audio `Claim`'s drop in +`kernel/src/device.rs` too), and the class's next holder's first interrupt +fires it into the old process's ring: one bit of another process's device +activity. A claim on a PCI +function, an ISA function or the ACPI fixed hardware registers with what the +claim lends (`DeviceClaim::add_poll`) and does not have it. + +**Read from the code, not run.** A guest cannot order a close and a second +claim between the two steps of one syscall. + +**Exit condition**: no call on a released class claim reaches the device, and +no poll registered on one stays on its watch, once the class's next claim is +minted; or the class is gone from the kernel, as +`issues/every-driver-is-still-in-the-kernel.md` retires each of them. + +**Owner**: none. That track is open and nobody holds it. diff --git a/issues/a-handler-tells-a-slot-it-read-before-the-slot-changed-hands.md b/issues/a-handler-tells-a-slot-it-read-before-the-slot-changed-hands.md new file mode 100644 index 00000000000..75b4c9a9e90 --- /dev/null +++ b/issues/a-handler-tells-a-slot-it-read-before-the-slot-changed-hands.md @@ -0,0 +1,35 @@ +--- +status: open +kind: finding +opened: 2026-10-08 +--- + +# A handler tells a slot it read before the slot changed hands + +Two interrupt handlers name a `pcidev` slot by a number and write that slot's +record a moment later, with nothing excluding a release in between: + +- the remapping fault handler reads the faulting stream's slot + (`owner_of`, `kernel/src/arch/x86_64/vtd/fault.rs`) and then calls + `pcidev::note_fault(slot)`; +- a claimed function's vector handler is its slot (`idt/user_dev.rs`) and + calls `pcidev::isr(slot)`. + +`pcidev::tear_down` clears the stream's slot and takes the remapping entry +away before the slot is free, so a handler that *starts* after a release names +nothing. One already past its read on another CPU still holds the number: if +the slot is released and bound to a second function before its write, that +function's claim is told a fault — every call on it refuses `Io` from then +on — or counts one interrupt its device did not raise. + +The interval is two adjacent statements of a handler running with interrupts +off, and a second function's bring-up in between is config-space writes and, +after a reset, a wait of at least the reset's settle time; no authority moves +either way. Nothing in the tree can hold a handler there, so this is read and +not run. + +**Exit condition**: the slot's record says which binding it is for and a +handler's write is refused by a record that has moved on, or this is folded +into `kernel/src/pcidev/mod.rs`'s header as the bound it is. + +**Owner**: none. No track covers a slot's record. diff --git a/issues/a-pci-call-in-flight-names-its-function-by-a-slot-the-next-claim-reuses.md b/issues/a-pci-call-in-flight-names-its-function-by-a-slot-the-next-claim-reuses.md deleted file mode 100644 index 5e8445994aa..00000000000 --- a/issues/a-pci-call-in-flight-names-its-function-by-a-slot-the-next-claim-reuses.md +++ /dev/null @@ -1,38 +0,0 @@ ---- -status: open -kind: defect -opened: 2026-10-08 ---- - -# A PCI call in flight names its function by a slot the next claim reuses - -Every syscall on a PCI claim — the BAR, the DMA grants and mappings, the -config read — resolves its handle to a `pcidev` slot number -(`pci_slot` and the register target, `kernel/src/syscall/device.rs`) and lets -go of the claim before it acts. What -it then acts on is whatever `BOUND[slot]` holds when `with_bound` -(`kernel/src/pcidev/mod.rs`) takes that slot's lock, which checks that the -slot is bound and not faulted, and not whose binding it is. The two steps are -under different locks with nothing held across them. - -A slot is not a name for one claim: the last handle's close releases it, a -function that was reset leaves it `Free`, and `slot::reserve` -(`kernel/pci/src/slot.rs`) gives the first free slot to the next claim, -whichever function and process that is. A call that resolved its slot before -its own claim was released therefore runs against the next holder's function, -and answers its caller with that function's object. A claim is authority over -one function for one holder; here it reaches another's. - -**Read from the code, not run.** A call made after the close is refused at -the handle, so only a call already past `pci_slot` is exposed, and a thread -in a syscall holding no lock can be preempted there. A guest cannot order a -release and a second claim inside that interval; the test owes the kernel a -hold at that point, which is an actuator this tree does not have. - -**Exit condition**: a call on a claim acts on that claim's binding or is -refused — its identity is held from the handle to the act, or the binding is -checked against it — and a test that holds a call between the two steps, -across the claim's release and the slot's next claim, sees it refused with -the second holder's function untouched. - -**Owner**: whoever holds `issues/every-driver-is-still-in-the-kernel.md`. diff --git a/issues/a-poll-registered-racing-its-own-close-outlives-the-release.md b/issues/a-poll-registered-racing-its-own-close-outlives-the-release.md deleted file mode 100644 index 7cbc6e084ea..00000000000 --- a/issues/a-poll-registered-racing-its-own-close-outlives-the-release.md +++ /dev/null @@ -1,28 +0,0 @@ ---- -status: open -kind: defect -opened: 2026-09-26 ---- - -# A poll registered racing its own process's close outlives the release - -`kernel/src/inbox/mod.rs`'s `process_watch` resolves the handle, clones the -object out, and only then `add_poll`s onto the object's watch with no lock -held. A PCI function's watch is a per-slot `static` -(`kernel/src/pcidev/mod.rs`'s `WATCHES`), and the release's `tear_down` answers -every poll on it with `cancel_polls`. A sibling thread of the same process that -closes the claim between the resolve and the `add_poll` lets the poll land on -the slot's watch *after* that cancel: the slot's next holder's first interrupt -then fires it into the old process's ring — one bit of another process's device -activity, across the isolation boundary. - -It predates the watch: main's `inbox::add_watcher` pushed onto -`pcidev`'s per-slot watcher list in the same order. - -## Exit condition - -A poll cannot reach a watch whose object has ended — the registration rechecks -the object's life after its `add_poll` and answers itself as gone — with a -guest test that closes a claim from one thread while another submits -`OP_WATCH` on it, and a second process claiming the slot afterwards sees no -completion arrive in the first one's ring. diff --git a/issues/a-reset-stops-xhci-and-leaves-every-claimed-pci-function-armed.md b/issues/a-reset-stops-xhci-and-leaves-every-claimed-pci-function-armed.md index df7628e42e6..49d0d2dc9bd 100644 --- a/issues/a-reset-stops-xhci-and-leaves-every-claimed-pci-function-armed.md +++ b/issues/a-reset-stops-xhci-and-leaves-every-claimed-pci-function-armed.md @@ -33,9 +33,9 @@ caches, waits for the log to be durable, seals `DONE` and calls `pcidev::release` (`kernel/src/pcidev/mod.rs:782`) calls `tear_down` (`:796`), which disables bus mastering (`:797`), masks the function's MSI-X entry (`:798`), empties its IOMMU domain (`:799`) and only then resets the function -(`:809`). The one caller of `pcidev::release` in the kernel -(`rg -n 'pcidev::release' kernel/src`, one hit) is `kernel/src/device.rs:75`, -inside `Claim`'s `Drop` — a dying process's handle table. +(`:809`). `release` is private to `pcidev` and its one caller is +`pcidev::Binding`'s `Drop`; a binding is held by a `device::Claim` and dropped +with it — a dying process's handle table. **So on every one of the five paths, a function whose holder is alive is left as its holder last wrote it when the register is written.** No process is diff --git a/issues/lan-talk-is-unread-on-the-t14-since-the-claims-binding-landed.md b/issues/lan-talk-is-unread-on-the-t14-since-the-claims-binding-landed.md new file mode 100644 index 00000000000..f87fcca74ed --- /dev/null +++ b/issues/lan-talk-is-unread-on-the-t14-since-the-claims-binding-landed.md @@ -0,0 +1,28 @@ +--- +status: open +kind: tooling +opened: 2026-10-08 +--- + +# `lan_talk` has not been read on the T14 since a device call began to borrow its claim's binding + +#763 makes every `pcidev` call act on a `&Binding` its claim lends, and a poll +register through the claim. The I219 under traffic is the one holder that +drives those calls on real hardware at rate: its interrupt records +(`take_record`, `has_irq`), its polls and its DMA grants. The `lan_talk` metal +row is what reads it, and it was staged from #763's head and not read: the +bench's wired path between the T14 and the development machine is down. + +The row cannot tell #763 from `main` while the path is down: staged from +`main` at `6f87cdb9c` and booted the same day, it is red the same way, `the +stream never opened` (`src/metaltalk.rs`), which is the host failing to reach +the machine and not the machine's answer. + +What stands in for it meanwhile: under QEMU, `iommu_virtio_platform` and the +suite's network tests drive the same `pcidev` functions through virtio-net's +claim; on the T14, `claim_reuses_its_remapping_entry`, `isa_` and `acpi_`. None +is the I219. + +Owner: the orchestrator, which holds the T14 and the bench. + +**Exit**: `lan_talk` green from the T14 on a head that contains #763. diff --git a/issues/the-power-offs-global-lock-line-is-logged-after-the-last-console-drain.md b/issues/the-power-offs-global-lock-line-is-logged-after-the-last-console-drain.md new file mode 100644 index 00000000000..e23d06a9a25 --- /dev/null +++ b/issues/the-power-offs-global-lock-line-is-logged-after-the-last-console-drain.md @@ -0,0 +1,57 @@ +--- +status: open +kind: defect +opened: 2026-10-08 +--- + +# The power-off logs the Global Lock's give-back after the console's last drain, and `acpi_mediated_access` waits for that line + +`acpi_mediated_access` (`tests/toyos.rs`) ends its wait on the kernel's line +`acpi: the Global Lock given back for a holder that left it taken (the machine +is stopping)`. Twice it never arrived, and the harness ended the guest after +`GUEST_QUIET` (15 s) of silence: + +``` +FAIL acpi_mediated_access: STALLED: waiting for the probe's power-off to give the lock back — it went quiet +``` + +| where | head | instrument | load | result | +|---|---|---|---|---| +| `main`'s nightly, run 37758546165, `tcg / suite` | `b6bcb9691` | TCG on a hosted 4-core x86-64 runner, 31 tests one wide | the suite alone | STALL after 18 s | +| the development machine, a whole-suite run of #763 | `252d19247` | TCG, x86-64 guest on an AArch64 host, 14 cores, 12 wide | load average 45 to 67: five agents compiling beside the suite | STALL after 35 s | +| `main`'s merge queue, six runs, #749 to #746 (37753990562, 37755528529, 37757410477, 37758981663, 37759626406, 37760845496), `guest / suite` | `6f87cdb9c` to `1084ddc9a` | KVM on hosted 4-core runners, 31 tests one wide | the suite alone | PASS, 3 s each | +| the development machine, `acpi_` by name | `252d19247` | TCG, 2 tests | run alone | PASS, 4 s | + +The nightly's head has #749, which added the test, and nothing of #763: the +red is `main`'s. KVM on a hosted runner is not TCG on a loaded host, so the +six green runs say only that the test passes where the guest is fast. + +## What was read + +`power::shutdown` (`kernel/src/power.rs`) calls `serial::flush_final()` under +the comment "Last chance: nothing drains the log ring after this point", and +then `arch::power::off`. `off` (`kernel/src/arch/x86_64/power.rs`) calls +`acpi_mode::settle`, whose `give_back` logs the line the test waits for, then +`acpi_mode::quiet`, then writes `SLP_TYP` and `SLP_EN`. So the line is +committed after the stop's last drain. Past boot the console's one writer is +`klogd` (`kernel/src/log/console.rs`, `Drain::Thread`), woken at the commit: the +line reaches the host only if `klogd`, on the other CPU, puts it on the wire +before this CPU has made one port read, `quiet`'s writes and the two writes +that end the machine. Nothing waits for it. + +That is a reading, and no run confirms it: neither red kept what the guest +said after boot. The test passes `await_guest`'s error up without its capture, +and the `uart-*.log` the nightly kept (artifact `serial-suite`, `uart-6.log`, +the `i8042-withheld` boot) ends where every boot's does, at the hand-over to +the virtio console. It fits both reds: the nightly's 18 s is the 3 s the test +takes under KVM and the 15 s of `GUEST_QUIET`. A probe whose arm failed would +have ended the wait by `===TEST_END`, and a kernel panic by its own line. + +The branch that saw it locally moves nothing here: `settle`, `give_back`, +`off`, `shutdown` and the console are `main`'s bytes in #763. + +Owner: the `acpi` claim's author (#749). + +**Exit**: `acpi_mediated_access` green in the nightly's `tcg / suite`, with the +give-back's line on the wire before `SLP_EN` by construction and not by +`klogd` winning; and a stall of this test carrying what the guest said. diff --git a/kernel/src/arch/aarch64/acpi_mode.rs b/kernel/src/arch/aarch64/acpi_mode.rs index eca660cf4f1..174479f6ec8 100644 --- a/kernel/src/arch/aarch64/acpi_mode.rs +++ b/kernel/src/arch/aarch64/acpi_mode.rs @@ -5,25 +5,25 @@ use toyos_abi::acpi::AcpiInfo; use crate::device::ClaimError; -pub fn claim() -> Result<(usize, AcpiInfo), ClaimError> { +pub fn claim() -> Result<(crate::isa::Row, AcpiInfo), ClaimError> { Err(ClaimError::Absent) } /// Never called: nothing is released where nothing can be claimed. -pub fn release(_row: usize) { +pub fn release() { unreachable!("AArch64 has no ACPI row") } /// Never called, as [`release`]: the three are reached only with a claim. -pub fn access(_row: usize, _request: &mut toyos_abi::acpi::Access) -> Result<(), toyos_abi::syscall::SyscallError> { +pub fn access(_row: &crate::isa::Row, _request: &mut toyos_abi::acpi::Access) -> Result<(), toyos_abi::syscall::SyscallError> { unreachable!("AArch64 has no ACPI row") } -pub fn lock_take() -> Result { +pub fn lock_take(_row: &crate::isa::Row) -> Result { unreachable!("AArch64 has no ACPI row") } -pub fn lock_release() -> Result<(), toyos_abi::syscall::SyscallError> { +pub fn lock_release(_row: &crate::isa::Row) -> Result<(), toyos_abi::syscall::SyscallError> { unreachable!("AArch64 has no ACPI row") } diff --git a/kernel/src/arch/x86_64/acpi_mode.rs b/kernel/src/arch/x86_64/acpi_mode.rs index 7735c082585..cb4efc59726 100644 --- a/kernel/src/arch/x86_64/acpi_mode.rs +++ b/kernel/src/arch/x86_64/acpi_mode.rs @@ -137,26 +137,27 @@ struct Facs { word: LockWordAt, } -/// What this kernel does for the claim's holder, and for whom. +/// What this kernel does for the claim's holder. struct Holder { - /// A claim exists: nothing is done for a holder that is gone. - claimed: bool, /// The holder took the Global Lock and has not given it back. locked: bool, } /// Held across everything this kernel does for the claim's holder, from the /// decision to the last instruction of the act: each mediated access, and -/// each change of the lock word. -static HOLDER: Lock = Lock::new(Holder { claimed: false, locked: false }); +/// each change of the lock word. Taken with the claim's own lock held +/// (`object::Held`), and `pcidev`'s machine record and then `paging`'s record +/// of windows under it; nothing holding one of those two takes this or a +/// claim's. +static HOLDER: Lock = Lock::new(Holder { locked: false }); /// The right to act for the claim's holder, held across the act; none once -/// the claim is gone or the stop has begun. A claim that exists is the one -/// the caller's process bound: `isa::claim_row` mints no next one while that -/// process has a thread left, one inside this call included. -fn acting() -> Result, SyscallError> { +/// the stop has begun. The claim is there for the whole of the act: its row +/// is lent under the lock its release takes the row with, and [`release`] +/// runs only once that has it. +fn acting(_claimed: &isa::Row) -> Result, SyscallError> { let holder = HOLDER.lock(); - if !holder.claimed || crate::quiesce::begun() { + if crate::quiesce::begun() { return Err(SyscallError::Gone); } Ok(holder) @@ -312,19 +313,12 @@ fn embedded_controller(rsdp_addr: u64, gpe0: Block) -> Result { /// The row, claimed, with the machine in ACPI mode; or the refusal that /// leaves it in legacy mode, said by name. -pub fn claim() -> Result<(usize, AcpiInfo), ClaimError> { +pub fn claim() -> Result<(isa::Row, AcpiInfo), ClaimError> { let hardware = hardware().ok_or(ClaimError::Absent)?; + // Released by the refusal's return, which drops it. let row = isa::claim_row(ROW)?; - match enter(hardware) { - Ok(()) => { - HOLDER.lock().claimed = true; - Ok((row, info(hardware))) - } - Err(refused) => { - isa::release(row); - Err(refused) - } - } + enter(hardware)?; + Ok((row, info(hardware))) } fn info(hardware: &Hardware) -> AcpiInfo { @@ -403,19 +397,15 @@ fn enter(hardware: &Hardware) -> Result<(), ClaimError> { } /// The claim's last handle went: the machine back in legacy mode where the -/// mint took it out of it, and then the row released, so no claimant finds -/// `SCI_EN` set by a holder whose disable is still to come. -pub fn release(row: usize) { +/// mint took it out of it. Before the row's own release, which its caller's +/// drop of the row is, so no claimant finds `SCI_EN` set by a holder whose +/// disable is still to come. +pub fn release() { let hardware = hardware().expect("a claimed row has its hardware"); - { - let mut holder = HOLDER.lock(); - holder.claimed = false; - give_back(hardware, &mut holder, "its claim is gone"); - } + give_back(hardware, &mut HOLDER.lock(), "its claim is gone"); if ENABLED.load(Ordering::Relaxed) { leave(hardware); } - isa::release(row); } /// `ACPI_DISABLE`, written where the mint wrote `ACPI_ENABLE`, and `SCI_EN` @@ -508,9 +498,9 @@ fn lock_word(word: LockWordAt) -> &'static AtomicU32 { /// `Ok(false)` where the firmware owns it, with the pending bit left set for /// the firmware's release to answer with `GBL_STS`, and `NotSupported` on a /// machine whose lock this kernel cannot take. -pub fn lock_take() -> Result { +pub fn lock_take(row: &isa::Row) -> Result { let hardware = hardware().expect("a claimed row has its hardware"); - let mut holder = acting()?; + let mut holder = acting(row)?; if holder.locked { return Err(SyscallError::AlreadyExists); } @@ -534,9 +524,9 @@ pub fn lock_take() -> Result { } /// Give the Global Lock back for the claim's holder. -pub fn lock_release() -> Result<(), SyscallError> { +pub fn lock_release(row: &isa::Row) -> Result<(), SyscallError> { let hardware = hardware().expect("a claimed row has its hardware"); - let mut holder = acting()?; + let mut holder = acting(row)?; if !holder.locked { return Err(SyscallError::InvalidArgument); } @@ -712,9 +702,9 @@ fn memory(hardware: &Hardware, acting: &Holder, request: &mut Access, width: Wid } /// One port access, decided and made. -fn port(row: usize, _acting: &Holder, address: u64, width: Width, write: Option) -> Result { +fn port(_acting: &Holder, address: u64, width: Width, write: Option) -> Result { let port = u16::try_from(address).map_err(|_| Refused::PortSpan)?; - let passed = firmware::port(|port| pio::standing(port, row), port, width, write.is_some())?; + let passed = firmware::port(|port| pio::standing(port, ROW), port, width, write.is_some())?; match write { None => Ok(read_port(&passed)), Some(value) => { @@ -724,12 +714,12 @@ fn port(row: usize, _acting: &Holder, address: u64, width: Width, write: Option< } } -/// Make the access `request` names for the holder of `row`'s claim, or -/// refuse it by name: a read's value, the refusal and the memory type are -/// written back into it. `Err` is a request that names no space, width or -/// direction, a value wider than its width, or a reserved byte that is not -/// zero; and `Gone` once the claim is gone or the stop has begun. -pub fn access(row: usize, request: &mut Access) -> Result<(), SyscallError> { +/// Make the access `request` names for the holder of the claim that lends +/// `row`, or refuse it by name: a read's value, the refusal and the memory +/// type are written back into it. `Err` is a request that names no space, +/// width or direction, a value wider than its width, or a reserved byte that +/// is not zero; and `Gone` once the stop has begun. +pub fn access(row: &isa::Row, request: &mut Access) -> Result<(), SyscallError> { let hardware = hardware().expect("a claimed row has its hardware"); let (Some(space), Some(width)) = (Space::from_raw(request.space), Width::from_raw(request.width)) else { return Err(SyscallError::InvalidArgument); @@ -742,11 +732,11 @@ pub fn access(row: usize, request: &mut Access) -> Result<(), SyscallError> { if request.reserved != [0; 3] { return Err(SyscallError::InvalidArgument); } - let acting = acting()?; + let acting = acting(row)?; request.memory_type = toyos_abi::acpi::UNLISTED; let made = match space { Space::SystemMemory => memory(hardware, &acting, request, width, write), - Space::SystemIo => port(row, &acting, request.address, width, write), + Space::SystemIo => port(&acting, request.address, width, write), Space::PciConfig => { // `toyos_abi::acpi::pci_address`: nothing above the segment group. let at = request.address; diff --git a/kernel/src/device.rs b/kernel/src/device.rs index 8da06cb1816..c955c974510 100644 --- a/kernel/src/device.rs +++ b/kernel/src/device.rs @@ -44,12 +44,16 @@ pub struct Claim { /// so what it gives back is its `pcidev` slot; a partition's exclusivity is its /// view's own hold on the blocks (`block::Partition::of`); an ISA function's and /// the ACPI fixed hardware's is its `isa` row's. +/// +/// The slot and the row are held as what their modules' calls take, and the +/// drop of each is its release: a call borrows one from the claim's `Held`, so +/// none is made on a slot or a row its claim has let go. enum Claimed { Class(DeviceType), - PciFunction(usize), + PciFunction(crate::pcidev::Binding), Partition(crate::block::Partition), - Isa(usize), - Acpi(usize), + Isa(crate::isa::Row), + Acpi(crate::isa::Row), } impl Claim { @@ -62,12 +66,36 @@ impl Claim { Ok(Self { what: Claimed::Class(class) }) } - /// The guard for a `pcidev` slot the caller has already reserved. - /// - /// It exists from the moment the slot is taken, so a bring-up that refuses - /// half way through frees the slot on the way out rather than stranding it. - pub(crate) fn pci(slot: usize) -> Self { - Self { what: Claimed::PciFunction(slot) } + /// The claim on a function `pcidev` has bound. + pub(crate) fn pci(binding: crate::pcidev::Binding) -> Self { + Self { what: Claimed::PciFunction(binding) } + } + + /// What `pcidev`'s calls on this claim are made with, or `None` for a + /// claim on anything but a PCI function. + pub(crate) fn binding(&self) -> Option<&crate::pcidev::Binding> { + match &self.what { + Claimed::PciFunction(binding) => Some(binding), + Claimed::Class(_) | Claimed::Partition(_) | Claimed::Isa(_) | Claimed::Acpi(_) => None, + } + } + + /// What `isa`'s calls on this claim are made with, or `None` for a claim + /// on anything but an ISA function or the ACPI fixed hardware. + pub(crate) fn row(&self) -> Option<&crate::isa::Row> { + match &self.what { + Claimed::Isa(row) | Claimed::Acpi(row) => Some(row), + Claimed::Class(_) | Claimed::PciFunction(_) | Claimed::Partition(_) => None, + } + } + + /// Register a poll on the watch of the function or row this claim holds. + pub(crate) fn add_poll(&self, entry: crate::inbox::PollEntry) { + match &self.what { + Claimed::PciFunction(binding) => crate::pcidev::add_poll(binding, entry), + Claimed::Isa(row) | Claimed::Acpi(row) => crate::isa::add_poll(row, entry), + Claimed::Class(_) | Claimed::Partition(_) => unreachable!("a poll registered through a claim on no function or row"), + } } /// The view a partition claim transfers through, or `None` for any other. @@ -81,8 +109,8 @@ impl Claim { impl Drop for Claim { fn drop(&mut self) { - match self.what { - Claimed::Class(class) => { + match &self.what { + &Claimed::Class(class) => { // Before the flag goes: a poll the next holder registers is not this claim's to answer. match class { DeviceType::HdaAudio | DeviceType::VirtioSound => { @@ -98,14 +126,16 @@ impl Drop for Claim { } *taken(class).lock() = false; } - // Bus mastering off, then the domain, then the pages: `release` - // owns that order, and this is where a dying process reaches it. - Claimed::PciFunction(slot) => crate::pcidev::release(slot), + // Bus mastering off, then the domain, then the pages: the + // binding's drop owns that order, and this is where a dying + // process reaches it. + Claimed::PciFunction(_) => {} // The view drops with this, and its hold with the last clone of it. Claimed::Partition(_) => {} - Claimed::Isa(row) => crate::isa::release(row), - // Back to the mode the firmware handed over, too. - Claimed::Acpi(row) => crate::arch::acpi_mode::release(row), + // The row's drop is its release. + Claimed::Isa(_) => {} + // Back to the mode the firmware handed over, and then the row's drop. + Claimed::Acpi(_) => crate::arch::acpi_mode::release(), } } } @@ -182,8 +212,8 @@ pub fn try_claim(class: DeviceType, selector: [u64; 2]) -> Result { let found = crate::gpt::claimable(toyos_abi::part::PartGuid::from_wire(selector))?; @@ -200,13 +230,13 @@ pub fn try_claim(class: DeviceType, selector: [u64; 2]) -> Result { // The row's own guard, taken inside, and the switch to ACPI mode. let (row, info) = crate::arch::acpi_mode::claim()?; let claim = Claim { what: Claimed::Acpi(row) }; - Ok(DeviceClaim::new(class, DeviceInfo::Acpi(info, row), claim)) + Ok(DeviceClaim::new(class, DeviceInfo::Acpi(info), claim)) } DeviceType::HdaAudio => { let (info, pcm) = crate::drivers::hda::info().ok_or(ClaimError::Absent)?; diff --git a/kernel/src/inventory.rs b/kernel/src/inventory.rs index 40fbc451daa..8a907e325b4 100644 --- a/kernel/src/inventory.rs +++ b/kernel/src/inventory.rs @@ -68,10 +68,9 @@ fn claims() -> Vec { let data = data.lock(); for (_, entry) in data.handles.iter() { let KObjectRef::Device(claim) = entry.object() else { continue }; - let on = if let Some(slot) = claim.pci_slot() { - // A slot nobody holds any more is a claim whose release is - // under way; it names nothing. - let Some(at) = crate::pcidev::held_at(slot) else { continue }; + let on = if claim.class() == toyos_abi::syscall::DeviceType::PciFunction { + // A function the last handle has let go names nothing. + let Some(at) = claim.pci(crate::pcidev::held_at) else { continue }; Claimed::Pci(at) } else if claim.class() == toyos_abi::syscall::DeviceType::Partition { // A partition the last handle has let go names nothing. diff --git a/kernel/src/isa.rs b/kernel/src/isa.rs index b190377c306..146b37dbc37 100644 --- a/kernel/src/isa.rs +++ b/kernel/src/isa.rs @@ -15,7 +15,8 @@ //! threads opens them and every other switch closes them //! (`arch::pio::switch_to`). A handle moved on after that answers nothing but //! refusals, and the row is claimable again only once both the claim and the -//! process are gone. A binding reaches the process's other threads at their +//! process are gone. **That holds for every row**: nothing here tells one from +//! another. A binding reaches the process's other threads at their //! next switch, and its end needs no switch at all: teardown runs once every //! thread has left, and no thread that has left returns to Ring 3. //! @@ -26,6 +27,12 @@ //! its handler too, and stays masked until the holder has served what raised //! it and acknowledged the claim ([`ack`]); it is masked from the claim until //! the holder's first acknowledgement. +//! +//! **A call on a claim is made with the claim's [`Row`] and never with a row +//! number**, lent under the lock the claim's release takes it with +//! (`object::Held`): the call runs wholly before the release or finds no row, +//! so none acknowledges, reads or registers a poll on a row its claim has +//! given up, and the release answers every poll registered before it. use alloc::boxed::Box; use alloc::vec::Vec; @@ -37,6 +44,7 @@ use toyos_userbound::Ports; use crate::arch::pio; use crate::device::ClaimError; +use crate::inbox::PollEntry; use crate::pcidev::record::Interrupt; use crate::process::Pid; use crate::sync::Lock; @@ -105,8 +113,32 @@ fn lines(row: usize) -> &'static [pio::Line] { if at.is_null() { &[] } else { unsafe { &*at } } } +/// One claim's hold on a row, from its mint to the release its drop is. +/// +/// Neither `Copy` nor `Clone` and made only by [`claim_row`], so a reference +/// to it is a row whose claim has not been released. +pub struct Row(usize); + +/// The claim's last handle went: its lines masked, its record emptied and its +/// polls answered. The ports stay with the process that bound them until that +/// process ends. +impl Drop for Row { + fn drop(&mut self) { + let row = self.0; + for &line in lines(row) { + pio::set_masked(line, true); + } + IRQ[row].clear(); + // The claim is gone, so a poll on it is answered rather than left for the + // next holder's interrupts; the row stays minted until that is made, so no + // next claim's poll is among the ones answered. + WATCHES[row].cancel_polls(); + *MINTED[row].lock() = false; + } +} + /// The row an `isa:` selector names whole. -pub fn claim(set: IsaId) -> Result { +pub fn claim(set: IsaId) -> Result { let row = (0..MAX_ROWS) .find(|&row| { function(row).is_some_and(|f| { @@ -122,7 +154,7 @@ pub fn claim(set: IsaId) -> Result { /// Mint the claim on `row`: its ports checked against every port this kernel /// declared, its edge lines routed and unmasked, its level lines routed and /// left masked for the holder's first acknowledgement. -pub fn claim_row(row: usize) -> Result { +pub fn claim_row(row: usize) -> Result { let function = function(row).ok_or(ClaimError::Absent)?; if let Some((run, holder)) = function.runs.iter().find_map(|&run| pio::holder(run).map(|h| (run, h))) { log!("isa: {}'s ports {:#x}+{} are {holder}'s", function.name, run.first(), run.count()); @@ -155,28 +187,13 @@ pub fn claim_row(row: usize) -> Result { } } *minted = true; - Ok(row) -} - -/// The claim's last handle went: its lines masked, its record emptied and its -/// polls answered. The ports stay with the process that bound them until that -/// process ends. -pub fn release(row: usize) { - for &line in lines(row) { - pio::set_masked(line, true); - } - IRQ[row].clear(); - // The claim is gone, so a poll on it is answered rather than left for the - // next holder's interrupts; the row stays minted until that is made, so no - // next claim's poll is among the ones answered. - WATCHES[row].cancel_polls(); - *MINTED[row].lock() = false; + Ok(Row(row)) } /// The holder served what raised the row's level lines: unmask them. A row /// with no level line has nothing to acknowledge. -pub fn ack(row: usize) -> Result<(), ()> { - let mut level = lines(row).iter().copied().filter(|&line| pio::level(line)).peekable(); +pub fn ack(row: &Row) -> Result<(), ()> { + let mut level = lines(row.0).iter().copied().filter(|&line| pio::level(line)).peekable(); if level.peek().is_none() { return Err(()); } @@ -189,7 +206,8 @@ pub fn ack(row: usize) -> Result<(), ()> { /// Open the row's ports to `pid` for the rest of its life. Called once per /// claim, by the claim's first read, and never twice for a row: [`claim_row`] /// mints none while a process holds its ports. -pub fn bind(row: usize, pid: Pid) { +pub fn bind(row: &Row, pid: Pid) { + let row = row.0; let name = function(row).expect("a claimed row was filled").name; match BOUND[row].compare_exchange(NOBODY, pid.raw(), Ordering::AcqRel, Ordering::Acquire) { Ok(_) => log!("isa: {name}'s ports are pid {pid}'s"), @@ -200,11 +218,17 @@ pub fn bind(row: usize, pid: Pid) { pio::switch_to(Some(pid)); } -/// Whether `pid` holds the row's ports. +/// Whether `pid` holds the row's ports: the context switch's question, of +/// every row. pub fn bound_to(row: usize, pid: Pid) -> bool { BOUND[row].load(Ordering::Acquire) == pid.raw() } +/// Whether `pid` holds the ports of the row this claim is on. +pub fn held_by(row: &Row, pid: Pid) -> bool { + bound_to(row.0, pid) +} + /// Called from the process teardown, once every thread has left. pub fn process_ends(pid: Pid) { for (row, bound) in BOUND.iter().enumerate() { @@ -215,7 +239,8 @@ pub fn process_ends(pid: Pid) { } /// The interrupts since the last read, or `None` for none. -pub fn take_record(row: usize) -> Option { +pub fn take_record(row: &Row) -> Option { + let row = row.0; let taken = IRQ[row].take(); if taken.is_some() && IRQ[row].take_unannounced() { log!("isa: {} took its first interrupt", function(row).expect("a claimed row was filled").name); @@ -223,8 +248,8 @@ pub fn take_record(row: usize) -> Option { taken.map(|count| DeviceIrqRecord { count }) } -pub fn has_irq(row: usize) -> bool { - IRQ[row].armed() +pub fn has_irq(row: &Row) -> bool { + IRQ[row.0].armed() } /// Records one interrupt, masks the row's level lines and posts the claim's @@ -240,8 +265,9 @@ pub fn isr(row: usize) { WATCHES[row].post_in_place(); } -pub fn watch(row: usize) -> &'static IrqWatch { - &WATCHES[row] +/// Register a poll on the watch of the row a claim holds. +pub(crate) fn add_poll(row: &Row, entry: PollEntry) { + WATCHES[row.0].add_poll(entry); } /// The function a filled row raises `wire` with too, if one does: a second diff --git a/kernel/src/object/device.rs b/kernel/src/object/device.rs index 54145354633..b2423518eaf 100644 --- a/kernel/src/object/device.rs +++ b/kernel/src/object/device.rs @@ -20,20 +20,18 @@ pub enum DeviceInfo { // Keyboard and mouse answer with events, not a description. Events, Framebuffer(FramebufferInfo, FramebufferBuffers), - /// The function, and the `pcidev` slot every call on this claim reaches it - /// through. No buffer handle beside it: a PCI claimant asks for its own - /// memory, so this mint installs nothing. - PciFunction(toyos_abi::pci::PciFunctionInfo, u8), + /// The function. No buffer handle beside it: a PCI claimant asks for its + /// own memory, so this mint installs nothing. + PciFunction(toyos_abi::pci::PciFunctionInfo), Hda(toyos_abi::hda::HdaInfo, Arc), VirtioSound(toyos_abi::virtio_sound::VirtioSoundInfo, Arc), /// Which partition, how long, and both its GUIDs; the view it moves blocks /// through is the claim's own (`device::Claim::partition`). Partition(toyos_abi::part::PartitionInfo), - /// The ports and lines granted, as the selector named them, and the `isa` - /// row they are. - Isa(toyos_abi::syscall::IsaId, usize), - /// The fixed hardware's blocks, and the `isa` row they are. - Acpi(toyos_abi::acpi::AcpiInfo, usize), + /// The ports and lines granted, as the selector named them. + Isa(toyos_abi::syscall::IsaId), + /// The fixed hardware's blocks. + Acpi(toyos_abi::acpi::AcpiInfo), } /// The two scanout buffers and the cursor plane. @@ -75,10 +73,10 @@ impl DeviceInfo { } // Nothing to install: every address in it is a size, and the memory // is what `SYS_DEVICE_DMA_ALLOC` answers later. - Self::PciFunction(info, _) => bytes(info).into(), + Self::PciFunction(info) => bytes(info).into(), Self::Partition(info) => bytes(info).into(), - Self::Isa(set, _) => set.wire().iter().flat_map(|word| word.to_ne_bytes()).collect(), - Self::Acpi(info, _) => bytes(info).into(), + Self::Isa(set) => set.wire().iter().flat_map(|word| word.to_ne_bytes()).collect(), + Self::Acpi(info) => bytes(info).into(), Self::Hda(info, pcm) => { let mut info = *info; info.pcm = install_buffers(table, &[pcm])?[0]; @@ -97,12 +95,6 @@ impl DeviceInfo { pub struct DeviceClaim { pub(super) core: ObjectCore, class: DeviceType, - /// The `pcidev` slot for a claim on one, read without the `described` lock: - /// a poll's readiness check and a `close` are both places that must not - /// take it. - pci_slot: Option, - /// The `isa` row for a claim on one, read without that lock for the same reasons. - isa_row: Option, // No Rights::DUP: at most one handle exists, so info_read needs no per-handle state. info_read: AtomicBool, described: crate::sync::Lock, @@ -118,19 +110,9 @@ struct Described { impl DeviceClaim { pub fn new(class: DeviceType, info: DeviceInfo, claim: Claim) -> Arc { - let pci_slot = match &info { - DeviceInfo::PciFunction(_, slot) => Some(*slot), - _ => None, - }; - let isa_row = match &info { - DeviceInfo::Isa(_, row) | DeviceInfo::Acpi(_, row) => Some(*row), - _ => None, - }; Arc::new(Self { core: Self::new_core(), class, - pci_slot, - isa_row, info_read: AtomicBool::new(false), described: crate::sync::Lock::new(Described { info, bytes: None }), reference: Held::new(claim), @@ -141,18 +123,34 @@ impl DeviceClaim { self.class } - /// Which `pcidev` slot this claim drives, for a claim on a PCI function. + /// `f` over the function this claim drives, with what `pcidev`'s calls + /// take: `None` for a claim on anything else, and once the last handle has + /// let the function go. /// - /// Every call the substrate answers goes through this: the handle is the - /// authority and the slot is what it names. - pub fn pci_slot(&self) -> Option { - self.pci_slot.map(usize::from) + /// Every call the substrate answers goes through this, under the lock the + /// release takes the binding with: `f` has returned before the function is + /// let go, or never runs. So `f` does not wait. + pub fn pci(&self, f: impl FnOnce(&crate::pcidev::Binding) -> R) -> Option { + self.reference.with(|claim| claim.binding().map(f)).flatten() } - /// Which `isa` row this claim holds, for a claim on an ISA function or the - /// ACPI fixed hardware. - pub fn isa_row(&self) -> Option { - self.isa_row + /// [`pci`](Self::pci), for the `isa` row of a claim on an ISA function or + /// the ACPI fixed hardware. + pub fn isa(&self, f: impl FnOnce(&crate::isa::Row) -> R) -> Option { + self.reference.with(|claim| claim.row().map(f)).flatten() + } + + /// Register `entry` on the watch of the function or row this claim holds, + /// with what the claim lends, so before its release: the release answers + /// every poll registered before it, and one that finds the claim released + /// is ended here, by its registrant. No poll is left for the interrupts of + /// whoever holds the slot or the row next. + pub(crate) fn add_poll(&self, entry: crate::inbox::PollEntry) { + let mut entry = Some(entry); + self.reference.with(|claim| claim.add_poll(entry.take().expect("lent once"))); + if let Some(entry) = entry { + kernel::sched::watch::Ring::fire(&entry, kernel::sched::watch::Fire::Gone); + } } /// The view a partition claim transfers through: `None` for a claim on diff --git a/kernel/src/object/ops.rs b/kernel/src/object/ops.rs index 3ed962a6869..5ebb983baf6 100644 --- a/kernel/src/object/ops.rs +++ b/kernel/src/object/ops.rs @@ -243,8 +243,11 @@ pub fn pipe_write(object: &KObjectRef) -> Option<(PipeId, WaitClass)> { pub enum WatchRef { Static(&'static Watch), Shared(Arc), - /// A device's, which its interrupt handler posts. + /// A device class's, which its interrupt handler posts. Irq(&'static IrqWatch), + /// The watch of the function or row a claim holds: named by the claim, + /// and reached only with what the claim lends. + Claim(Arc), } impl WatchRef { @@ -253,6 +256,7 @@ impl WatchRef { Self::Static(watch) => watch.add_poll(entry), Self::Shared(watch) => watch.add_poll(entry), Self::Irq(watch) => watch.add_poll(entry), + Self::Claim(claim) => claim.add_poll(entry), } } @@ -261,6 +265,8 @@ impl WatchRef { Self::Static(watch) => watch.cancel_polls(), Self::Shared(watch) => watch.cancel_polls(), Self::Irq(watch) => watch.cancel_polls(), + // Its release answers them, and its close ends none (`close_ends_polls`). + Self::Claim(_) => unreachable!("a claim's polls are cancelled by its release alone"), } } } @@ -277,12 +283,9 @@ pub fn read_watch(object: &KObjectRef) -> Option { KObjectRef::Device(d) => match d.class() { device_registry::DeviceType::Keyboard => Some(WatchRef::Static(&keyboard::WATCH)), device_registry::DeviceType::Mouse => Some(WatchRef::Static(&mouse::WATCH)), - device_registry::DeviceType::PciFunction => { - d.pci_slot().map(|slot| WatchRef::Irq(crate::pcidev::watch(slot))) - } - device_registry::DeviceType::Isa | device_registry::DeviceType::Acpi => { - d.isa_row().map(|row| WatchRef::Irq(crate::isa::watch(row))) - } + device_registry::DeviceType::PciFunction + | device_registry::DeviceType::Isa + | device_registry::DeviceType::Acpi => Some(WatchRef::Claim(d.clone())), device_registry::DeviceType::HdaAudio | device_registry::DeviceType::VirtioSound => { Some(WatchRef::Irq(&crate::drivers::AUDIO_WATCH)) } @@ -317,18 +320,20 @@ pub fn write_watch(object: &KObjectRef) -> Option { /// and the keyboard, which the machine ends on its own and which other handles /// share: a console closing is not every console's keyboard going away. `false` /// for a process, which only its own end ends: closing one handle to it ends no -/// other's watch. +/// other's watch. `false` for a claim on a function or a row: its one handle's +/// close is its release, which answers every poll on the watch before the slot +/// or the row can be held again (`pcidev`'s and `isa::Row`'s release). fn close_ends_polls(object: &KObjectRef) -> bool { match object { KObjectRef::SysCap(_) => false, KObjectRef::Console(_) => false, KObjectRef::Process(_) => false, KObjectRef::Device(d) => match d.class() { - device_registry::DeviceType::Keyboard => false, - device_registry::DeviceType::Mouse + device_registry::DeviceType::Keyboard | device_registry::DeviceType::PciFunction | device_registry::DeviceType::Isa - | device_registry::DeviceType::Acpi + | device_registry::DeviceType::Acpi => false, + device_registry::DeviceType::Mouse | device_registry::DeviceType::HdaAudio | device_registry::DeviceType::VirtioSound | device_registry::DeviceType::Framebuffer @@ -427,10 +432,10 @@ pub fn read_device( if buf.len() < toyos_abi::pci::DeviceIrqRecord::SIZE { return Some(SyscallError::InvalidArgument.to_u64()); } - let slot = claim.pci_slot().expect("a PCI claim knows its slot"); - let record = match crate::pcidev::take_record(slot) { - Ok(record) => record?, - Err(refused) => return Some(refused.to_u64()), + let record = match claim.pci(crate::pcidev::take_record) { + Some(Ok(record)) => record?, + Some(Err(refused)) => return Some(refused.to_u64()), + None => return Some(SyscallError::Gone.to_u64()), }; buf.write_at(0, record_bytes(&record)); Some(toyos_abi::pci::DeviceIrqRecord::SIZE as u64) @@ -438,19 +443,29 @@ pub fn read_device( // The PCI shape, but the description is what binds the ports to the // reader, and nothing after it answers any other process. device_registry::DeviceType::Isa | device_registry::DeviceType::Acpi => { - let row = claim.isa_row().expect("an ISA or ACPI claim knows its row"); let pid = crate::process::current_process(); if !claim.info_read() { - crate::isa::bind(row, pid); + if claim.isa(|row| crate::isa::bind(row, pid)).is_none() { + return Some(SyscallError::Gone.to_u64()); + } return Some(claim.describe(table, buf)); } - if !crate::isa::bound_to(row, pid) { - return Some(SyscallError::PermissionDenied.to_u64()); - } - if buf.len() < toyos_abi::pci::DeviceIrqRecord::SIZE { - return Some(SyscallError::InvalidArgument.to_u64()); - } - let record = crate::isa::take_record(row)?; + let room = buf.len() >= toyos_abi::pci::DeviceIrqRecord::SIZE; + // The holder's check and the take are one borrow of the row. + let taken = claim.isa(|row| { + if !crate::isa::held_by(row, pid) { + Err(SyscallError::PermissionDenied) + } else if !room { + Err(SyscallError::InvalidArgument) + } else { + Ok(crate::isa::take_record(row)) + } + }); + let record = match taken { + Some(Ok(record)) => record?, + Some(Err(refused)) => return Some(refused.to_u64()), + None => return Some(SyscallError::Gone.to_u64()), + }; buf.write_at(0, record_bytes(&record)); Some(toyos_abi::pci::DeviceIrqRecord::SIZE as u64) } @@ -794,11 +809,13 @@ pub fn has_data(object: &KObjectRef) -> bool { KObjectRef::Device(d) => match d.class() { device_registry::DeviceType::Keyboard => keyboard::has_data(), device_registry::DeviceType::Mouse => mouse::has_data(), + // A claim its last handle has let go has nothing to read: a poll + // on it is ended where it registers (`DeviceClaim::add_poll`). device_registry::DeviceType::PciFunction => { - !d.info_read() || d.pci_slot().is_some_and(crate::pcidev::has_irq) + !d.info_read() || d.pci(crate::pcidev::has_irq).unwrap_or(false) } device_registry::DeviceType::Isa | device_registry::DeviceType::Acpi => { - !d.info_read() || d.isa_row().is_some_and(crate::isa::has_irq) + !d.info_read() || d.isa(crate::isa::has_irq).unwrap_or(false) } device_registry::DeviceType::Framebuffer => true, device_registry::DeviceType::Partition => true, @@ -876,20 +893,28 @@ fn write_device(claim: &DeviceClaim, buf: &UserBytes) -> u64 { | device_registry::DeviceType::PciFunction | device_registry::DeviceType::Partition => return SyscallError::PermissionDenied.to_u64(), } - let row = claim.isa_row().expect("an ISA or ACPI claim knows its row"); - if !claim.info_read() || !crate::isa::bound_to(row, crate::process::current_process()) { + if !claim.info_read() { return SyscallError::PermissionDenied.to_u64(); } let mut word = [0u8; 4]; - if buf.len() != word.len() { - return SyscallError::InvalidArgument.to_u64(); - } - buf.read_at(0, &mut word); - if u32::from_ne_bytes(word) != toyos_abi::acpi::ACK { - return SyscallError::InvalidArgument.to_u64(); - } - match crate::isa::ack(row) { - Ok(()) => word.len() as u64, - Err(()) => SyscallError::InvalidArgument.to_u64(), + let acknowledges = buf.len() == word.len() && { + buf.read_at(0, &mut word); + u32::from_ne_bytes(word) == toyos_abi::acpi::ACK + }; + let pid = crate::process::current_process(); + // The holder's check and the unmask are one borrow of the row. + let acked = claim.isa(|row| { + if !crate::isa::held_by(row, pid) { + Err(SyscallError::PermissionDenied) + } else if !acknowledges { + Err(SyscallError::InvalidArgument) + } else { + crate::isa::ack(row).map_err(|()| SyscallError::InvalidArgument) + } + }); + match acked { + Some(Ok(())) => word.len() as u64, + Some(Err(refused)) => refused.to_u64(), + None => SyscallError::Gone.to_u64(), } } diff --git a/kernel/src/pcidev/mod.rs b/kernel/src/pcidev/mod.rs index 3a0bdffbf78..0b5487fa7e2 100644 --- a/kernel/src/pcidev/mod.rs +++ b/kernel/src/pcidev/mod.rs @@ -94,6 +94,22 @@ //! grants are still mapped; the next claim of the slot waits the reset out and //! writes back what the reset cleared and [`bring_up`] does not write itself //! ([`Kept`]) before it reads a register of the function. +//! +//! **A call names its function by its claim's [`Binding`] and never by a slot +//! number.** A slot is the next claim's once its holder's release is over, so +//! a number carried from a claim to a call would be another holder's function +//! by the time it was used. The binding is lent by the claim under the lock +//! its release takes it with (`object::Held`): a call runs wholly before the +//! release or finds no binding. Outside a claim's own mint and release, only +//! the two handlers name a slot, by the vector and the requester the hardware +//! gave them. +//! +//! **A poll is on a slot's watch only while its claim holds the slot.** It is +//! registered with the binding ([`add_poll`]), so before the release, whose +//! own answer to every poll on the watch comes before the slot can be +//! reserved again; a registration that finds the claim released is ended by +//! its registrant (`DeviceClaim::add_poll`). So a slot's next holder's +//! interrupt fires no poll of the last one's, in whatever ring. /// No `crate::` reference, so `kernel-loom` compiles it and models the /// interleaving no guest test lands on. @@ -120,6 +136,7 @@ use crate::mm::policy::{CachePolicy, MmioPolicy}; use crate::mm::{align_2m, DirectMap, Mmio, PAGE_2M}; use crate::object::shm::{Region, SharedMemObject}; use crate::sync::Lock; +use crate::inbox::PollEntry; use crate::watch::IrqWatch; /// How many functions this machine can hand out at once. @@ -365,6 +382,23 @@ static MACHINE: Lock = Lock::new(Machine { /// takes it once the teardown is over. static SLOTS: Lock<[Slot; MAX_FUNCTIONS]> = Lock::new([Slot::Free; MAX_FUNCTIONS]); +/// One claim's hold on a slot, from its reservation to the release its drop +/// is: what every call on the claim is made with. +/// +/// Neither `Copy` nor `Clone` and made only by [`claim`], so there is one per +/// reservation, and a reference to it is a slot no other claim holds. +pub struct Binding { + slot: usize, + /// The requester the slot is held for. + who: u16, +} + +impl Drop for Binding { + fn drop(&mut self) { + release(self.slot); + } +} + /// Take a slot for `who`, or say why not. /// /// The scan and the take are one critical section: two claims arriving together @@ -417,10 +451,9 @@ pub fn segment() -> u16 { MACHINE.lock().segment } -/// The function a claim's slot holds, or `None` for a slot nobody holds. -pub fn held_at(slot: usize) -> Option { - let Some(Slot::Held(who)) = SLOTS.lock().get(slot).copied() else { return None }; - Some(addr_of(MACHINE.lock().segment, who)) +/// The function a claim holds. +pub fn held_at(binding: &Binding) -> toyos_abi::inventory::PciAddr { + addr_of(MACHINE.lock().segment, binding.who) } pub(crate) fn requester(pci: &PciDevice) -> u16 { @@ -681,7 +714,7 @@ impl core::fmt::Display for Refusal { /// Claim `id`, bring the function up to the point a driver takes over, and /// answer what that driver needs to know. -pub fn claim(id: PciId) -> Result<(PciFunctionInfo, u8, Claim), ClaimError> { +pub fn claim(id: PciId) -> Result<(PciFunctionInfo, Claim), ClaimError> { let (pci, driven) = { let machine = MACHINE.lock(); let mut found: Option = None; @@ -717,9 +750,10 @@ pub fn claim(id: PciId) -> Result<(PciFunctionInfo, u8, Claim), ClaimError> { // `Owned` before `Exhausted`: a second claim on a function somebody holds // is a different fact from a machine with no slot left. - let slot = reserve(requester(&pci))?; - // Dropped by every refusal below, and `release` is what gives the slot back. - let claim = Claim::pci(slot); + let who = requester(&pci); + let slot = reserve(who)?; + // Dropped by every refusal below, which is what gives the slot back. + let binding = Binding { slot, who }; match bring_up(pci, id, slot) { Ok(mut bound) => { @@ -744,7 +778,7 @@ pub fn claim(id: PciId) -> Result<(PciFunctionInfo, u8, Claim), ClaimError> { id.device, VECTORS[slot], ); - Ok((info, slot as u8, claim)) + Ok((info, Claim::pci(binding))) } Err(why) => { log!( @@ -1403,7 +1437,7 @@ fn alone_in_its_page(claimed: &PciDevice, index: u8, at: u64, span: u64) { /// The order is what makes a dying driver safe. A page freed while the function /// could still reach it is a device writing into memory the allocator has /// already handed to somebody else. -pub fn release(slot: usize) { +fn release(slot: usize) { // Two statements, because edition 2021 keeps an `if let`'s scrutinee // temporaries alive to the end of its block: `BOUND[slot]`'s guard would be // held across the unmaps, the reset and the watch's answer. @@ -1492,17 +1526,17 @@ fn tear_down(slot: usize, mut bound: Bound) { ); } -/// What every call a claim answers checks first. +/// What every call a claim answers checks first, over what that claim bound. fn with_bound( - slot: usize, + binding: &Binding, f: impl FnOnce(&mut Bound) -> Result, ) -> Result { - if IRQ[slot].faulted() { + if IRQ[binding.slot].faulted() { return Err(SyscallError::Io); } - let mut guard = BOUND[slot].lock(); - let bound = guard.as_mut().ok_or(SyscallError::NotFound)?; - f(bound) + let mut guard = BOUND[binding.slot].lock(); + // Bound before `claim` hands the binding out, and unbound only by its drop. + f(guard.as_mut().expect("pcidev: a claim's binding has its function bound")) } /// One memory BAR as an object to map, a fresh one for every ask. @@ -1510,8 +1544,8 @@ fn with_bound( /// **The claim keeps where the window is and never an object over it**: an /// object ends with its last handle, which its holder closes whenever it /// likes, and the claim outlives that. -pub fn bar_object(slot: usize, index: u64) -> Result, SyscallError> { - with_bound(slot, |bound| { +pub fn bar_object(binding: &Binding, index: u64) -> Result, SyscallError> { + with_bound(binding, |bound| { let index = usize::try_from(index).map_err(|_| SyscallError::InvalidArgument)?; if index >= BARS || bound.bar_bytes[index] == 0 { return Err(SyscallError::InvalidArgument); @@ -1529,10 +1563,11 @@ pub fn bar_object(slot: usize, index: u64) -> Result, Sysca /// Memory this function may reach, and nothing else may. pub fn dma_alloc( - slot: usize, + binding: &Binding, bytes: u64, ) -> Result<(Arc, u64, u64), SyscallError> { - with_bound(slot, |bound| { + let slot = binding.slot; + with_bound(binding, |bound| { if bytes == 0 || bytes > MAX_GRANT_BYTES { return Err(SyscallError::InvalidArgument); } @@ -1581,8 +1616,9 @@ pub fn dma_alloc( /// Found by its memory and not by position: a [`dma_map`] on another thread of /// the same holder can push between the grant and this, and the address the /// caller was *told* is not always the address the grant is at. -pub fn dma_undo(slot: usize, memory: &Arc) { - let _ = with_bound(slot, |bound| { +pub fn dma_undo(binding: &Binding, memory: &Arc) { + let slot = binding.slot; + let _ = with_bound(binding, |bound| { let Some(index) = bound.grants.iter().position(|grant| Arc::ptr_eq(&grant.memory, memory)) else { return Ok(()); @@ -1612,8 +1648,9 @@ pub fn dma_undo(slot: usize, memory: &Arc) { /// Counted against [`MAX_GRANT_TOTAL`] with the claim's own grants, so a holder /// mapping every region its clients send is bounded by the same number; and a /// window with no run of the region's length left free is the same refusal. -pub fn dma_map(slot: usize, region: &Arc) -> Result<(u64, u64), SyscallError> { - with_bound(slot, |bound| { +pub fn dma_map(binding: &Binding, region: &Arc) -> Result<(u64, u64), SyscallError> { + let slot = binding.slot; + with_bound(binding, |bound| { let (phys, span) = region.ram().ok_or(SyscallError::InvalidArgument)?; // One region, one address: a second mapping of it would be two grants // naming one set of pages, and taking either back would leave the @@ -1644,8 +1681,13 @@ pub fn dma_map(slot: usize, region: &Arc) -> Result<(u64, u64), /// there is refused at the unit and recorded against this claim — and only /// after that is the region's `Arc` let go, so its pages can never be freed /// under a translation that still names them. -pub fn dma_unmap(slot: usize, at: u64) -> Result<(), SyscallError> { - let grant = with_bound(slot, |bound| { +/// +/// **Answers the region, for the caller to let go with nothing held**: the +/// last `Arc` of one can run its zero-handle teardown, which takes other +/// objects' locks, and the caller holds its claim's. +pub fn dma_unmap(binding: &Binding, at: u64) -> Result, SyscallError> { + let slot = binding.slot; + with_bound(binding, |bound| { let index = bound .grants .iter() @@ -1655,12 +1697,8 @@ pub fn dma_unmap(slot: usize, at: u64) -> Result<(), SyscallError> { if let Err(why) = bound.space.unmap(grant.at, grant.bytes) { panic!("pcidev: slot {slot} could not take {:#x} back: {why}", grant.at); } - Ok(grant) - })?; - // Outside the slot's lock: the last `Arc` of a region can run its - // zero-handle teardown, which takes other objects' locks. - drop(grant); - Ok(()) + Ok(grant.memory) + }) } /// The window a claim's configuration reads are checked against, for the one @@ -1679,8 +1717,8 @@ pub fn config_window(offset: u64, width: RegWidth) -> Result Result { - with_bound(slot, |bound| { +pub fn config_read(binding: &Binding, at: Register, width: RegWidth) -> Result { + with_bound(binding, |bound| { Ok(match width { RegWidth::U8 => bound.pci.read_config_u8(at.offset()) as u32, RegWidth::U16 => bound.pci.read_config_u16(at.offset()) as u32, @@ -1692,7 +1730,8 @@ pub fn config_read(slot: usize, at: Register, width: RegWidth) -> Result Result, SyscallError> { +pub fn take_record(binding: &Binding) -> Result, SyscallError> { + let slot = binding.slot; if IRQ[slot].faulted() { return Err(SyscallError::Io); } @@ -1705,8 +1744,8 @@ pub fn take_record(slot: usize) -> Result, SyscallError> /// Whether a read of the claim answers at once: a message is waiting, or the /// refusal is. -pub fn has_irq(slot: usize) -> bool { - IRQ[slot].armed() || IRQ[slot].faulted() +pub fn has_irq(binding: &Binding) -> bool { + IRQ[binding.slot].armed() || IRQ[binding.slot].faulted() } /// Records one message and posts the claim's watch. Called from the vector's @@ -1728,7 +1767,7 @@ pub fn note_fault(slot: usize) { crate::preempt::set_need_resched(); } -/// The watch of the function a claim holds at `slot`. -pub fn watch(slot: usize) -> &'static IrqWatch { - &WATCHES[slot] +/// Register a poll on the watch of the function a claim holds. +pub(crate) fn add_poll(binding: &Binding, entry: PollEntry) { + WATCHES[binding.slot].add_poll(entry); } diff --git a/kernel/src/syscall/device.rs b/kernel/src/syscall/device.rs index 231a0b1c2fd..f6084b35e97 100644 --- a/kernel/src/syscall/device.rs +++ b/kernel/src/syscall/device.rs @@ -3,6 +3,9 @@ //! [`holds_claim`] checks a claim's class against what the syscall requires; //! device register access and protocol stay in the device's own code. +use alloc::sync::Arc; + +use crate::object::device::DeviceClaim; use crate::object::{ops, KObjectRef}; use crate::user_ptr::SyscallContext; use crate::UserAddr; @@ -39,7 +42,8 @@ pub(super) fn holds_claim( enum RegTarget { Hda, VirtioSound, - /// A claimed PCI function's own config space, **read-only**. + /// A claimed PCI function's own config space, **read-only**, through the + /// claim itself: what names the function is lent by it for each access. /// /// There is no writing counterpart and that is the point: a driver cannot /// find its own registers without its capability chain — virtio's four @@ -48,7 +52,7 @@ enum RegTarget { /// mastering, the BARs and the MSI-X control word are all writes, and /// refusing them is what stops a holder aiming its device at memory or its /// interrupt at a vector. - PciConfig(usize), + PciConfig(Arc), } /// Reads or writes one register of the claimed device; the device owns its allow-list. @@ -57,14 +61,12 @@ pub(super) fn sys_device_reg(handle: RawHandle, offset: u64, width: u64, value: return SyscallError::InvalidArgument.to_u64(); }; let target = process::with_process_data(|data| { - data.handles - .get::(handle, Rights::NONE) - .map(|claim| match claim.class() { - device::DeviceType::HdaAudio => Some(RegTarget::Hda), - device::DeviceType::VirtioSound => Some(RegTarget::VirtioSound), - device::DeviceType::PciFunction => claim.pci_slot().map(RegTarget::PciConfig), - _ => None, - }) + data.handles.get::(handle, Rights::NONE).map(|claim| match claim.class() { + device::DeviceType::HdaAudio => Some(RegTarget::Hda), + device::DeviceType::VirtioSound => Some(RegTarget::VirtioSound), + device::DeviceType::PciFunction => Some(RegTarget::PciConfig(claim)), + _ => None, + }) }); // `refuse` must run with nothing held; the guard above has already been dropped. let target = match target { @@ -84,8 +86,10 @@ pub(super) fn sys_device_reg(handle: RawHandle, offset: u64, width: u64, value: // witness `pcidev::config_window` answers is the only thing // the register read takes, so an unchecked number cannot // reach the register file. - RegTarget::PciConfig(slot) => match crate::pcidev::config_window(offset, width) { - Ok(at) => crate::pcidev::config_read(slot, at, width), + RegTarget::PciConfig(claim) => match crate::pcidev::config_window(offset, width) { + Ok(at) => on_function(&claim, |function| { + crate::pcidev::config_read(function, at, width) + }), Err(_) => Err(SyscallError::InvalidArgument), }, }; @@ -119,30 +123,44 @@ pub(super) fn sys_device_reg(handle: RawHandle, offset: u64, width: u64, value: /// `PermissionDenied`, as its ports answer nothing there. pub(super) fn sys_acpi(ctx: &SyscallContext, handle: RawHandle, op: u64, at: u64) -> u64 { use toyos_abi::acpi::op as ops; - let held = process::with_process_data(|data| { - data.handles - .get::(handle, Rights::WRITE) - .map(|claim| (claim.class(), claim.isa_row())) - }); - let row = match held { - Ok((device::DeviceType::Acpi, Some(row))) => row, - Ok((class, _)) => { - return crate::object::HandleError::WrongType { held: class.class_name(), wanted: "an acpi claim" }.refuse() + let claim = match process::with_process_data(|data| data.handles.get::(handle, Rights::WRITE)) { + Ok(claim) if claim.class() == device::DeviceType::Acpi => claim, + Ok(claim) => { + let held = claim.class().class_name(); + // `refuse` does not return: the claim is let go first. + drop(claim); + return crate::object::HandleError::WrongType { held, wanted: "an acpi claim" }.refuse(); } Err(e) => return e.refuse(), }; - if !crate::isa::bound_to(row, process::current_process()) { - return SyscallError::PermissionDenied.to_u64(); - } - let done = match op { - ops::ACCESS => ctx.copy_in::(UserAddr::new(at)).and_then(|mut request| { - crate::arch::acpi_mode::access(row, &mut request)?; - ctx.copy_out(UserAddr::new(at), &request).map(|()| 0) - }), - ops::LOCK_TAKE => crate::arch::acpi_mode::lock_take().map(|taken| if taken { ops::TAKEN } else { ops::PENDING }), - ops::LOCK_RELEASE => crate::arch::acpi_mode::lock_release().map(|()| 0), - _ => Err(SyscallError::InvalidArgument), + // Copied in before the row is borrowed and out after it is given back: + // no user copy is made under the claim's lock. + let mut request = match op { + ops::ACCESS => match ctx.copy_in::(UserAddr::new(at)) { + Ok(request) => Some(request), + Err(e) => return e.to_u64(), + }, + _ => None, }; + let pid = process::current_process(); + // The holder's check and the act are one borrow of the row, as a read's are. + let done = claim.isa(|row| { + if !crate::isa::held_by(row, pid) { + return Err(SyscallError::PermissionDenied); + } + match (op, request.as_mut()) { + (ops::ACCESS, Some(request)) => crate::arch::acpi_mode::access(row, request).map(|()| 0), + (ops::LOCK_TAKE, _) => { + crate::arch::acpi_mode::lock_take(row).map(|taken| if taken { ops::TAKEN } else { ops::PENDING }) + } + (ops::LOCK_RELEASE, _) => crate::arch::acpi_mode::lock_release(row).map(|()| 0), + _ => Err(SyscallError::InvalidArgument), + } + }); + let done = done.unwrap_or(Err(SyscallError::Gone)).and_then(|word| match &request { + Some(request) => ctx.copy_out(UserAddr::new(at), request).map(|()| word), + None => Ok(word), + }); done.unwrap_or_else(|e| e.to_u64()) } @@ -190,34 +208,48 @@ pub(super) fn sys_device_claim(syscap: RawHandle, class: u64, selector: [u64; 2] }) } -/// The `pcidev` slot a claim handle names, with the right every substrate call +/// The PCI function claim a handle names, with the right every substrate call /// demands. /// /// [`Rights::WRITE`] for all three: each one gives the holder control of the /// device — a register window, memory the device can reach, its interrupt — and /// a claim carries no `DUP`, so no narrower handle to one can exist. -fn pci_slot(handle: RawHandle) -> Result { - let slot = process::with_process_data(|data| { - data.handles - .get::(handle, Rights::WRITE) - .map(|claim| claim.pci_slot()) +/// +/// **The claim, and nothing read out of it**: a call reaches its function only +/// through [`on_function`], so what it acts on is this claim's or nothing. +fn pci_claim(handle: RawHandle) -> Result, crate::object::Refusal> { + let claim = process::with_process_data(|data| { + data.handles.get::(handle, Rights::WRITE) })?; // A claim of another class is the wrong kind of thing here, not a missing // device: it names a device this call has no meaning for. - slot.ok_or(crate::object::HandleError::WrongType { - held: device::DeviceType::PciFunction.class_name(), - wanted: "a PCI function claim", + match claim.class() { + device::DeviceType::PciFunction => Ok(claim), + other => Err(crate::object::HandleError::WrongType { + held: other.class_name(), + wanted: "a PCI function claim", + } + .into()), } - .into()) +} + +/// One `pcidev` call on the function `claim` drives, or [`SyscallError::Gone`] +/// for a claim whose last handle went under the call: a sibling thread closed +/// it, and whatever holds its slot now is not this caller's. +fn on_function( + claim: &DeviceClaim, + call: impl FnOnce(&crate::pcidev::Binding) -> Result, +) -> Result { + claim.pci(call).unwrap_or(Err(SyscallError::Gone)) } /// One memory BAR of a claimed function, as an object to map. pub(super) fn sys_device_bar_map(handle: RawHandle, index: u64) -> u64 { - let slot = match pci_slot(handle) { - Ok(slot) => slot, + let claim = match pci_claim(handle) { + Ok(claim) => claim, Err(e) => return e.refuse(), }; - let object = match crate::pcidev::bar_object(slot, index) { + let object = match on_function(&claim, |function| crate::pcidev::bar_object(function, index)) { Ok(object) => object, Err(e) => return e.to_u64(), }; @@ -233,8 +265,8 @@ pub(super) fn sys_device_dma_alloc( bytes: u64, out: UserAddr, ) -> u64 { - let slot = match pci_slot(handle) { - Ok(slot) => slot, + let claim = match pci_claim(handle) { + Ok(claim) => claim, Err(e) => return e.refuse(), }; // The output window is taken before the allocation, the same order @@ -244,7 +276,8 @@ pub(super) fn sys_device_dma_alloc( let Some(mut window) = ctx.user_bytes_mut(out, len) else { return SyscallError::BadAddress.to_u64(); }; - let (memory, device_addr, granted) = match crate::pcidev::dma_alloc(slot, bytes) { + let granted = on_function(&claim, |function| crate::pcidev::dma_alloc(function, bytes)); + let (memory, device_addr, granted) = match granted { Ok(grant) => grant, Err(e) => return e.to_u64(), }; @@ -256,8 +289,9 @@ pub(super) fn sys_device_dma_alloc( // The grant goes back rather than staying mapped with nothing naming // it: a caller left holding neither the handle nor the quota would be // refused every later grant with no way out but dying. + // A claim released since took the grant back itself. Err(e) => { - crate::pcidev::dma_undo(slot, &memory); + claim.pci(|function| crate::pcidev::dma_undo(function, &memory)); return e.to_u64(); } }; @@ -286,21 +320,26 @@ pub(super) fn sys_device_dma_map( region: RawHandle, out: UserAddr, ) -> u64 { - let slot = match pci_slot(handle) { - Ok(slot) => slot, + let claim = match pci_claim(handle) { + Ok(claim) => claim, Err(e) => return e.refuse(), }; let memory = match process::with_process_data(|data| { data.handles.get::(region, Rights::MAP) }) { Ok(memory) => memory, - Err(e) => return e.refuse(), + Err(e) => { + // `refuse` does not return: the claim is let go first. + drop(claim); + return e.refuse(); + } }; let len = core::mem::size_of::() as u64; let Some(mut window) = ctx.user_bytes_mut(out, len) else { return SyscallError::BadAddress.to_u64(); }; - let (device_addr, bytes) = match crate::pcidev::dma_map(slot, &memory) { + let mapped = on_function(&claim, |function| crate::pcidev::dma_map(function, &memory)); + let (device_addr, bytes) = match mapped { Ok(mapped) => mapped, Err(e) => return e.to_u64(), }; @@ -320,12 +359,13 @@ pub(super) fn sys_device_dma_map( /// Take back what [`sys_device_dma_map`] put at `device_addr`. pub(super) fn sys_device_dma_unmap(handle: RawHandle, device_addr: u64) -> u64 { - let slot = match pci_slot(handle) { - Ok(slot) => slot, + let claim = match pci_claim(handle) { + Ok(claim) => claim, Err(e) => return e.refuse(), }; - match crate::pcidev::dma_unmap(slot, device_addr) { - Ok(()) => 0, + // The region is let go here, with the claim's lock given back. + match on_function(&claim, |function| crate::pcidev::dma_unmap(function, device_addr)) { + Ok(_region) => 0, Err(e) => e.to_u64(), } }