From 3651887977281a8e8601c834bd6b61c46e1eb865 Mon Sep 17 00:00:00 2001 From: japabu Date: Tue, 29 Sep 2026 00:28:47 +0200 Subject: [PATCH 01/10] The panic console takes no input, and an isa claim grants a process exact ports The first stage of taking the i8042 out of the kernel (owner ruling: drivers are userland, and a dead kernel takes no input, with no emergency way). The whole move does not fit one reviewable change: the driver is 1,496 lines, a dozen guest tests measure its internals through kernel actuators, and the harness paces every typed line on its drain trace. This stage lands what the server will stand on and deletes nothing half way; ps2d and the driver's deletion are stage 7.2 of the small-kernel track. The panic console takes no input. The pager cycles on its own and is never steered, the reboot bound has no retire and fires whoever is at the machine, and poll_byte, the one read of port 0x60 outside the driver's ISR, is gone from both architectures. screen_pager_keys is deleted with the keys it pressed; panic_key_holds becomes panic_ignores_keys, the same guest with a key pressed inside the bound that must now reset anyway. screen_paged_scrollback stays: it presses nothing and judges the automatic cycle, which survives. The panic-path issue about reading the key off an unconfigured i8042 closes with the key. The isa claim, DeviceType::Isa ("isa:0060,0064:1,12"), names an ISA function by exactly its ports and lines, and the kernel grants only a whole row of its architecture's GRANTABLE table: on x86-64 the i8042, on AArch64 nothing. The first read of the claim binds the ports to the reading process and answers the set back; each CPU's TSS now carries an I/O permission bitmap for ports below 0x100 and its refusing end byte (Intel SDM Vol. 1 19.5.2), and every context switch opens a row's ports for the process holding it and closes them for every other. IOPL stays 0. The ports stay with that process until its teardown, after every thread has left, so no revocation ever races a running thread; a handle moved on after the bind answers PermissionDenied. The lines are routed once per boot to vector 0x2c, unmasked while a claim exists, and counted into the same record a claimed PCI function's are. A claim on a function the kernel drives is refused by name, which is every machine whose i8042 the kernel armed. A Ring 3 #GP at an in or out now names the port and the direction in the kill record. No syscall was added. Tests: toyos-abi's parser and wire round trip; isa_grant in three roles on three machines (refused where the kernel drives; granted, and refused one port over and to every other process, on a machine with no i8042; a real controller the kernel gave up on driven by the claim to a keystroke's record and byte). The holder can reset the machine through 0x64's 0xFE, which the bitmap cannot filter: filed as a question for the owner. Co-Authored-By: Claude Opus 5.5 --- README.md | 5 +- ...rch-module-in-userland-and-guest-probes.md | 2 +- ...2s-holder-holds-the-machines-reset-line.md | 25 ++ .../every-driver-is-still-in-the-kernel.md | 2 + ...-small-interrupts-post-and-threads-wait.md | 17 +- ...nd-may-leave-nothing-to-end-the-machine.md | 8 +- ...reads-its-key-off-an-unconfigured-i8042.md | 32 -- .../src/arch/aarch64/keyboard_controller.rs | 7 +- kernel/src/arch/aarch64/pio.rs | 26 +- kernel/src/arch/x86_64/hw.rs | 1 + kernel/src/arch/x86_64/i8042/mod.rs | 19 +- kernel/src/arch/x86_64/idt/exceptions.rs | 20 +- kernel/src/arch/x86_64/idt/isa.rs | 18 + kernel/src/arch/x86_64/idt/mod.rs | 7 + kernel/src/arch/x86_64/percpu.rs | 36 +- kernel/src/arch/x86_64/pio.rs | 136 ++++++++ kernel/src/device.rs | 14 +- kernel/src/drivers/panic_console/mod.rs | 100 +----- kernel/src/drivers/pci.rs | 2 +- kernel/src/isa.rs | 180 ++++++++++ kernel/src/main.rs | 1 + kernel/src/object/device.rs | 16 + kernel/src/object/ops.rs | 33 +- kernel/src/panic.rs | 6 +- kernel/src/panic_reboot.rs | 30 +- kernel/src/pcidev/mod.rs | 2 +- kernel/src/process.rs | 2 + kernel/src/sched/driver.rs | 7 +- kernel/src/syscall/device.rs | 5 +- src/sourcegate.rs | 1 + tests/common/power.rs | 108 ++---- tests/common/qemu.rs | 5 +- tests/test-durations | 2 - tests/toyos-rust-tests/src/bin/isa_grant.rs | 271 +++++++++++++++ tests/toyos.rs | 312 +++++++----------- toyos-abi/src/syscall.rs | 225 ++++++++++++- toyos-ps2/tests/decode.rs | 6 +- toyos/src/syscap.rs | 12 + userland/init/src/main.rs | 1 + 39 files changed, 1225 insertions(+), 477 deletions(-) create mode 100644 issues/isolation/the-i8042s-holder-holds-the-machines-reset-line.md delete mode 100644 issues/panic-path/a-storage-phase-panic-reads-its-key-off-an-unconfigured-i8042.md create mode 100644 kernel/src/arch/x86_64/idt/isa.rs create mode 100644 kernel/src/isa.rs create mode 100644 tests/toyos-rust-tests/src/bin/isa_grant.rs diff --git a/README.md b/README.md index 04b3eadddf0..c44de52f1b4 100644 --- a/README.md +++ b/README.md @@ -194,9 +194,8 @@ attempt. ![ToyOS booting on a ThinkPad T14](first-boot.jpg) The screenshot is the laptop's own panel. The machine has no serial port, so -the kernel renders its log and its panics to the framebuffer, and pages them -with PageUp and PageDown polled straight off the keyboard controller after -every CPU has halted. It is reporting a real bug: the page cache sized an index +the kernel renders its log and its panics to the framebuffer. It is reporting a +real bug: the page cache sized an index from the disk's block count, which fit in QEMU's test image and wanted 238 MB on a 244 GB drive. diff --git a/issues/build/assembly-outside-an-arch-module-in-userland-and-guest-probes.md b/issues/build/assembly-outside-an-arch-module-in-userland-and-guest-probes.md index 6035bb6f8a7..f25057351bb 100644 --- a/issues/build/assembly-outside-an-arch-module-in-userland-and-guest-probes.md +++ b/issues/build/assembly-outside-an-arch-module-in-userland-and-guest-probes.md @@ -18,7 +18,7 @@ pointing here: `_mm_sfence` after writing a write-combining framebuffer. Userland has no portable way to say "drain my stores to the scanout"; the SDK (`toyos/src`) owes one, and it is also only changed under an ABI brief. -- Twenty guest probes in `tests/toyos-rust-tests/src/bin/`, whose subject +- The guest probes in `tests/toyos-rust-tests/src/bin/`, whose subject is an x86 instruction (`rdgsbase`, `fxsave64`, `int1`, x87 control words) or the raw `syscall` gate with arguments no SDK call will pass. They run in the x86-64 suite, which is the only suite until the harness gains its arch diff --git a/issues/isolation/the-i8042s-holder-holds-the-machines-reset-line.md b/issues/isolation/the-i8042s-holder-holds-the-machines-reset-line.md new file mode 100644 index 00000000000..115b0efe97b --- /dev/null +++ b/issues/isolation/the-i8042s-holder-holds-the-machines-reset-line.md @@ -0,0 +1,25 @@ +--- +status: owner +kind: question +opened: 2026-09-28 +--- + +# The i8042's holder holds the machine's reset line + +An `isa` claim on the i8042 (`kernel/src/arch/x86_64/pio.rs`'s `GRANTABLE`) +opens ports 0x60 and 0x64 to the process that binds it, through the TSS I/O +permission bitmap, and the bitmap grants a port or refuses it: it cannot see +the value written. The controller's command port takes `0xFE`, which pulses the +CPU's reset line, and `0xD1`, which writes its output port, where the same +line lives (IBM PC AT Technical Reference, the 8042's commands). So whichever +process drives the keyboard can reset the machine whenever it likes: no memory +is exposed and the kernel does not crash, but a userland bug in that one +program ends every other one without a word in the log. + +Neither way out is free: filtering the command byte means the kernel decoding +the controller's protocol, a syscall or a trapped instruction per access in +place of the bitmap; keeping 0x64 in the kernel means the keyboard driver is +not wholly userland, which the owner ruled it must be. + +**Exit**: the owner rules that the keyboard's holder may hold the reset line, +or names which of the two costs to pay. diff --git a/issues/kernel/every-driver-is-still-in-the-kernel.md b/issues/kernel/every-driver-is-still-in-the-kernel.md index 2bd6b1ef0c3..390af5dca39 100644 --- a/issues/kernel/every-driver-is-still-in-the-kernel.md +++ b/issues/kernel/every-driver-is-still-in-the-kernel.md @@ -36,6 +36,8 @@ What is left of the staged work: 3. Done: **the capability itself** is `DeviceType::PciFunction` plus `SYS_DEVICE_BAR_MAP` and `SYS_DEVICE_DMA_ALLOC`, with config space readable and unwritable and the interrupt delivered as a record on the claim. +4. **The i8042 (PS/2)**: staged as stage 7 of + `issues/kernel/the-kernel-is-small-interrupts-post-and-threads-wait.md`. Two constraints that were not obvious before the code was read: diff --git a/issues/kernel/the-kernel-is-small-interrupts-post-and-threads-wait.md b/issues/kernel/the-kernel-is-small-interrupts-post-and-threads-wait.md index 614cc269375..331e3e49cd6 100644 --- a/issues/kernel/the-kernel-is-small-interrupts-post-and-threads-wait.md +++ b/issues/kernel/the-kernel-is-small-interrupts-post-and-threads-wait.md @@ -128,7 +128,7 @@ times: keyboard claim and mass storage over `toyos-blockring`, and the kernel USB bridge is deleted. **What must work with no userland stays off USB**: the panic console pages its report by itself and needs no - keyboard, and steers only from the i8042 it polls; the kernel's one + keyboard; the kernel's one hotkey, Ctrl+Alt+D (`kernel/src/keyboard.rs`, the blocked-task dump), is recognised on the i8042's transitions and no longer on a USB keyboard's, which from here reach the kernel only as usbd's keyboard @@ -145,6 +145,21 @@ times: to their device's `Watch`, and the device's thread does the work. The per-CPU IRQ relay, the driver list in the scheduler pass and the idle special cases are deleted. +7. **The i8042 leaves the kernel.** Owner ruling, 2026-09-28: drivers are + userland, and a dead kernel takes no input, with no emergency way. + 1. **The panic console takes no input, and an `isa` claim grants a process + exact ports through the TSS I/O permission bitmap and its ISA lines as + records** (`kernel/src/isa.rs`). **Done** (PR_NUMBER). + 2. **ps2d**, the server over that claim, feeding the kernel's keyboard and + mouse streams so Ctrl+Alt+D and the merge with USB HID stay where they + are; the kernel's driver, its vector, its actuators and the + `keyboard_controller` seam deleted. Constraints: the harness paces typed + input on the kernel's `i8042: drain bytes=` trace (`shell_type_once`, + every `i8042-trace` boot), every boot config that types needs the server, + and a keyboard claim is refused while no source exists, which init's + order of endowment then decides. **Exit**: every keyboard and mouse guest + test green with no i8042 code in the kernel, and typing resumes after + ps2d is killed and restarted. ## Standing diff --git a/issues/panic-path/a-fatal-event-stands-down-both-bounds-and-may-leave-nothing-to-end-the-machine.md b/issues/panic-path/a-fatal-event-stands-down-both-bounds-and-may-leave-nothing-to-end-the-machine.md index dc6c2cff938..1d1c7c8c1ae 100644 --- a/issues/panic-path/a-fatal-event-stands-down-both-bounds-and-may-leave-nothing-to-end-the-machine.md +++ b/issues/panic-path/a-fatal-event-stands-down-both-bounds-and-may-leave-nothing-to-end-the-machine.md @@ -23,9 +23,7 @@ with no fallback by design (`kernel/src/drivers/acpi.rs:277-283`). Where the table named none, `arm` returns `Bound::Held` (`kernel/src/panic_reboot.rs:134-145`) and `hold_the_panel` loops `while bound.is_armed()` (`kernel/src/drivers/panic_console/mod.rs:647-650`), -which never ends. A keypress also retires the bound, which is right for a -machine with somebody in front of it and is exactly wrong for one running -unattended. +which never ends. **On the T14 that last bound is the one thing already known not to work.** `issues/hardware/an-armed-tco-has-never-reset-the-t14.md` records that no claim @@ -61,5 +59,5 @@ prints it. **Exit condition**: a boot that takes a fatal event on the T14 with nobody in front of it either ends itself and leaves a record naming the fault, or the run -shows which of `Bound::Held` and the retired-by-a-keypress path held it — read -off the machine, not argued from the tree. +shows that `Bound::Held` held it — read off the machine, not argued from the +tree. diff --git a/issues/panic-path/a-storage-phase-panic-reads-its-key-off-an-unconfigured-i8042.md b/issues/panic-path/a-storage-phase-panic-reads-its-key-off-an-unconfigured-i8042.md deleted file mode 100644 index 3c9c27ed479..00000000000 --- a/issues/panic-path/a-storage-phase-panic-reads-its-key-off-an-unconfigured-i8042.md +++ /dev/null @@ -1,32 +0,0 @@ ---- -status: open -kind: defect -opened: 2026-09-27 ---- - -# A panic in the storage phase reads its key off an i8042 the kernel has not configured - -The panic panel's reset bound is retired by a key press -(`kernel/src/drivers/panic_console/mod.rs`, `read_key`), which polls port -`0x60` through `keyboard_controller::poll_byte`. `i8042::init` -(`kernel/src/arch/x86_64/i8042/mod.rs`) is what turns translation on, stops -and restarts scanning and enables the port-1 clock. It now runs first in the -device phase (`arch::boot::platform_devices` in `kernel/src/main.rs`), after -storage, so its lines stay on a panel that shows the log's tail. - -So a panic anywhere in the storage phase — NVMe or xHCI init, -`rootfs::hold_source`, the DATA and FAT mounts — meets the controller as the -firmware left it. Whether a key press then reaches `read_key` as a set-1 make -code depends on the firmware: with translation off or scanning off it does -not, and the panel resets at its bound however many keys are pressed. That -ordering is the one the tree had before storage moved behind init's spawn, so -it has shipped before; it is still a weakness. No boot exercises a panic in -the storage phase with the firmware's controller state left unconfigured. - -## Exit condition - -A key press retires the panel's bound for a panic anywhere after the i8042 -probe could have run — the controller configured before the first phase that -can panic on a device, without moving its lines off the panel's tail — and a -boot that panics in the storage phase on the metal-sim shape shows the press -retiring it. diff --git a/kernel/src/arch/aarch64/keyboard_controller.rs b/kernel/src/arch/aarch64/keyboard_controller.rs index ef6462509b8..cebd8f04265 100644 --- a/kernel/src/arch/aarch64/keyboard_controller.rs +++ b/kernel/src/arch/aarch64/keyboard_controller.rs @@ -1,10 +1,5 @@ //! The platform's own keyboard controller. An Arm machine has none: its -//! keyboards are USB, and the panic panel's key poll reads nothing here. - -/// No byte ever waits. -pub fn poll_byte() -> Option<(u8, bool)> { - None -} +//! keyboards are USB. /// Nothing to decide about a controller that is not there. pub fn verdict_due() -> bool { diff --git a/kernel/src/arch/aarch64/pio.rs b/kernel/src/arch/aarch64/pio.rs index b2886833d47..d70a15b620f 100644 --- a/kernel/src/arch/aarch64/pio.rs +++ b/kernel/src/arch/aarch64/pio.rs @@ -1,9 +1,33 @@ -//! The I/O port space: AArch64 has none. +//! The I/O port space: AArch64 has none, and so no ISA function to grant. + +use core::convert::Infallible; + +use crate::process::Pid; /// Whether this architecture has an I/O port space at all. Firmware tables /// that name a port are only honoured where it does. pub const EXISTS: bool = false; +/// Nothing: every `isa` claim is refused as naming no function. +pub const GRANTABLE: &[crate::isa::Grantable] = &[]; + +/// No line is routed where no row exists. +pub type Line = Infallible; + +/// Never called: [`GRANTABLE`] has no row to route. +pub fn route(_row: usize, _irq: u8) -> Result { + unreachable!("AArch64 has no ISA bus") +} + +pub fn set_masked(line: Line, _masked: bool) { + match line {} +} + +/// Never called: nothing is bound where nothing can be claimed. +pub fn switch_to(_pid: Option) { + unreachable!("AArch64 has no I/O permission bitmap") +} + /// Never called: every caller checks [`EXISTS`] first. pub unsafe fn outb(_port: u16, _value: u8) { unreachable!("AArch64 has no I/O port space") diff --git a/kernel/src/arch/x86_64/hw.rs b/kernel/src/arch/x86_64/hw.rs index 78a458f6694..a45c730c06d 100644 --- a/kernel/src/arch/x86_64/hw.rs +++ b/kernel/src/arch/x86_64/hw.rs @@ -433,6 +433,7 @@ impl Hw for KernelHw { crate::preempt::set_count(incoming.preempt); percpu::set_current_tid(incoming.id.map(|id| id.1)); percpu::set_current_pid(incoming.id.map(|id| id.0)); + super::pio::switch_to(incoming.id.map(|id| id.0)); match incoming.id { Some(_) => { // Here, not in the pass: this is the one place a task (not idle) becomes what a diff --git a/kernel/src/arch/x86_64/i8042/mod.rs b/kernel/src/arch/x86_64/i8042/mod.rs index 44abd8fc377..1683d0c1f88 100644 --- a/kernel/src/arch/x86_64/i8042/mod.rs +++ b/kernel/src/arch/x86_64/i8042/mod.rs @@ -103,6 +103,11 @@ static LAST_IRQ_NS: AtomicU64 = AtomicU64::new(0); /// only CPU an `irq_ring` record for this source can exist on. static IRQ_CPU: AtomicU32 = AtomicU32::new(u32::MAX); +/// Whether this driver holds the controller, which refuses an `isa` claim on it. +pub fn drives() -> bool { + ACTIVE.load(Ordering::Relaxed) +} + fn is_irq_cpu() -> bool { IRQ_CPU.load(Ordering::Relaxed) == crate::arch::percpu::cpu_id() } @@ -815,8 +820,8 @@ fn trace_drain(bytes: usize, keys: usize, motion: usize, woke_kb: bool, woke_ms: } // Each read below is done as its section's sole reader: init before the -// vector is armed, the aux re-enable on `IRQ_CPU` under `IrqGuard::close`, -// and the panic pager with every CPU halted — so no ISR ever races them. +// vector is armed, and the aux re-enable on `IRQ_CPU` under `IrqGuard::close` +// — so no ISR ever races them. fn deadline(millis: u64) -> u64 { crate::clock::nanos_since_boot() + millis * 1_000_000 @@ -1379,16 +1384,6 @@ pub fn init(rsdp_addr: u64) { } } -/// One byte from the controller if it has one; never waits. Only legal once -/// every CPU is halted — port 0x60's sole reader is otherwise the ISR. -pub fn poll_byte() -> Option<(u8, bool)> { - let status = inb(STATUS); - if status & OBF == 0 { - return None; - } - Some((inb(DATA), status & AUXB != 0)) -} - /// The handler's drain loop, without the EOI. Runs with interrupts off on /// `IRQ_CPU`, keeping `push_isr`'s producer single. Publishes the same /// record the ISR does — a silent push here would manufacture a lost edge. diff --git a/kernel/src/arch/x86_64/idt/exceptions.rs b/kernel/src/arch/x86_64/idt/exceptions.rs index dce2ffc825a..2285b90be3a 100644 --- a/kernel/src/arch/x86_64/idt/exceptions.rs +++ b/kernel/src/arch/x86_64/idt/exceptions.rs @@ -90,6 +90,17 @@ fn safe_read_u64(addr: u64, user_pml4: *const u64) -> Option { } } +/// The `in` or `out` at `rip`, read through the page tables as the rest of the +/// report reads user memory; `None` for any other instruction or an unreadable one. +fn port_access_at(rip: u64, rdx: u64, pml4: *const u64) -> Option { + let at = rip & !7; + let lo = safe_read_u64(at, pml4)?; + let hi = safe_read_u64(at + 8, pml4)?; + // Shifted rather than indexed: nothing on this path may panic. + let code = ((u128::from(lo) | u128::from(hi) << 64) >> (8 * (rip & 7))) as u32; + super::super::pio::port_access(code.to_le_bytes(), rdx as u16) +} + pub(crate) struct ExceptionContext<'a> { frame: &'a TrapFrame, cr2: u64, @@ -180,7 +191,14 @@ fn crash_report_exception(ctx: &ExceptionContext) { log!("SIGFPE tid={}: {}", tid, name) } Vector::GeneralProtection | Vector::StackSegment | Vector::AlignmentCheck => { - log!("SIGBUS tid={}: {} (error_code={:#x})", tid, name, ctx.frame.error_code) + log!("SIGBUS tid={}: {} (error_code={:#x})", tid, name, ctx.frame.error_code); + // At CPL 3 an `in` or `out` faults only on a port the bitmap refuses. + if let Some(access) = (ctx.vector() == Vector::GeneralProtection) + .then(|| port_access_at(ctx.frame.rip, ctx.frame.rdx, pml4)) + .flatten() + { + log!(" {access}, which this process holds no grant for"); + } } _ => log!("FATAL tid={}: {}", tid, name), } diff --git a/kernel/src/arch/x86_64/idt/isa.rs b/kernel/src/arch/x86_64/idt/isa.rs new file mode 100644 index 00000000000..f2980a231a5 --- /dev/null +++ b/kernel/src/arch/x86_64/idt/isa.rs @@ -0,0 +1,18 @@ +//! The vector a claimed ISA function's lines deliver on: a count into the +//! row's record and a pass owed, as a claimed PCI function's message is. + +use super::device_irq::device_irq_entry; +use crate::irq_ring::IrqSource; + +extern "sysv64" fn isa0_handler() { + crate::arch::percpu::irq_took!(UserDev); + crate::isa::isr(0); + crate::irq_ring::isr_publish(IrqSource::UserDev, crate::clock::nanos_since_boot()); + crate::preempt::set_need_resched(); + crate::arch::apic::eoi(); +} + +device_irq_entry! { + /// `pio::GRANTABLE`'s row 0. + pub(super) fn isa0_entry => isa0_handler +} diff --git a/kernel/src/arch/x86_64/idt/mod.rs b/kernel/src/arch/x86_64/idt/mod.rs index 3c7d56d78a2..6ca5d7b1229 100644 --- a/kernel/src/arch/x86_64/idt/mod.rs +++ b/kernel/src/arch/x86_64/idt/mod.rs @@ -3,6 +3,7 @@ mod device_irq; mod dma_fault; mod hda; mod i8042; +mod isa; #[cfg(feature = "boot-actuators")] mod log_nest; mod nmi; @@ -32,6 +33,11 @@ const PIC2_DATA: u16 = 0xA1; /// The vector both PS/2 lines are routed to. pub const I8042_VECTOR: u8 = Vector::I8042 as u8; +/// The vector each `pio::GRANTABLE` row's lines are routed to, by row: the +/// vector is how the kernel knows whose record an interrupt belongs to. +pub const ISA_VECTORS: [u8; 1] = [Vector::Isa0 as u8]; +const _: () = assert!(ISA_VECTORS.len() == super::pio::GRANTABLE.len(), "a row with no vector"); + /// The vector an IOMMU writes into its own `FEDATA`. pub const DMA_FAULT_VECTOR: u8 = Vector::DmaFault as u8; @@ -268,6 +274,7 @@ idt_vectors! { ring3 UserDev1 = 0x29, user_dev::user_dev1_entry; ring3 UserDev2 = 0x2A, user_dev::user_dev2_entry; ring3 UserDev3 = 0x2B, user_dev::user_dev3_entry; + ring3 Isa0 = 0x2C, isa::isa0_entry; // Ring 0 because it never returns: `cli; hlt` forever. ring0 HaltAll = 0xFD, stub_halt_all; ring3 TlbFlush = 0xFE, tlb::tlb_flush_entry; diff --git a/kernel/src/arch/x86_64/percpu.rs b/kernel/src/arch/x86_64/percpu.rs index f70b189345e..b4e653dac17 100644 --- a/kernel/src/arch/x86_64/percpu.rs +++ b/kernel/src/arch/x86_64/percpu.rs @@ -22,7 +22,11 @@ pub const STAR_SYSRET_BASE: u16 = USER_DS - 8; const _: () = assert!(STAR_SYSRET_BASE + 8 == USER_DS); const _: () = assert!(STAR_SYSRET_BASE + 16 == USER_CS); -/// 64-bit TSS (104 bytes). +/// Ports the I/O permission bitmap names; every port from here on is past the +/// TSS limit, which refuses it without a bit (Intel SDM Vol. 1 §19.5.2). +pub const IO_PORTS: usize = 0x100; + +/// The 104-byte 64-bit TSS, then the I/O permission bitmap. #[repr(C, packed)] pub struct Tss { reserved0: u32, @@ -34,6 +38,12 @@ pub struct Tss { reserved2: u64, reserved3: u16, iopb_offset: u16, + /// A set bit refuses its port to Ring 3; `pio::switch_to` clears one only + /// while the process holding it runs. + io_bitmap: [u8; IO_PORTS / 8], + /// All ones: the processor reads two bytes for every check, and the byte + /// past the last one the bitmap names must refuse. + io_bitmap_end: u8, } impl Tss { @@ -47,11 +57,19 @@ impl Tss { ist: [0; 7], reserved2: 0, reserved3: 0, - iopb_offset: size_of::() as u16, + iopb_offset: offset_of!(Tss, io_bitmap) as u16, + io_bitmap: [0xFF; IO_PORTS / 8], + io_bitmap_end: 0xFF, } } } +const _: () = assert!(offset_of!(Tss, io_bitmap) == 104, "the bitmap follows the architectural TSS"); +const _: () = assert!( + offset_of!(Tss, io_bitmap_end) + 1 == size_of::(), + "the TSS limit ends at the refusing byte" +); + /// Per-CPU fault state machine for the escalation policy on nested faults. #[repr(u8)] #[derive(Clone, Copy, PartialEq, Eq, Debug)] @@ -661,6 +679,20 @@ pub unsafe fn set_kernel_stack(rsp: u64) { core::ptr::write_unaligned(&raw mut (*percpu).tss.rsp0, rsp); } +/// Open or close `port` to Ring 3 on this CPU. Called with interrupts off, so +/// the CPU whose bitmap this reaches cannot change under the write. +pub fn set_port_open(port: u16, open: bool) { + let (byte, bit) = (port as usize / 8, 1u8 << (port % 8)); + assert!(byte < IO_PORTS / 8, "port {port:#x} is past the I/O permission bitmap"); + let percpu = gs::read_u64::() as *mut PerCpu; + // SAFETY: this CPU's own `PerCpu`, read from `gs:[0]`; `byte` is inside the + // array (asserted), and a `u8` has no alignment a packed struct could break. + unsafe { + let at = &raw mut (*percpu).tss.io_bitmap[byte]; + *at = if open { *at & !bit } else { *at | bit }; + } +} + /// The two words [`set_kernel_stack`] writes: `kernel_rsp` (syscall entry) and `tss.rsp0` (Ring 3 interrupt entry); read only by an instrument. /// # Safety: must be called from the CPU whose GS base points to the relevant PerCpu. #[cfg(feature = "stack-witness")] diff --git a/kernel/src/arch/x86_64/pio.rs b/kernel/src/arch/x86_64/pio.rs index 5a29539bbb2..ccb21c86561 100644 --- a/kernel/src/arch/x86_64/pio.rs +++ b/kernel/src/arch/x86_64/pio.rs @@ -1,7 +1,143 @@ //! The I/O port space: x86-64 has one, reached by `in` and `out`. +//! +//! **A process reaches a port only through its CPU's I/O permission bitmap, +//! with IOPL left at 0** (Intel SDM Vol. 1 §19.5.2, AMD APM Vol. 2 §12.2.4): +//! each CPU's TSS carries one bit per port below [`IO_PORTS`], set unless the +//! process running there holds a [`GRANTABLE`] row naming that port, and +//! every port at or past it is past the TSS limit, which the processor refuses +//! by itself. + +use alloc::format; +use alloc::string::String; + +use super::ioapic::{self, Gsi}; +pub use super::percpu::IO_PORTS; +use crate::isa::Grantable; +use crate::process::Pid; /// Whether this architecture has an I/O port space at all. Firmware tables /// that name a port are only honoured where it does. pub const EXISTS: bool = true; pub use super::cpu::{outb, outw}; + +/// The functions a process may be handed: the i8042's data and command ports, +/// its keyboard line and its aux line. +pub const GRANTABLE: &[Grantable] = &[Grantable { + name: "the i8042", + ports: &[0x60, 0x64], + irqs: &[1, 12], + kernel_drives: super::i8042::drives, +}]; + +const _: () = { + let mut row = 0; + while row < GRANTABLE.len() { + let mut i = 0; + while i < GRANTABLE[row].ports.len() { + assert!((GRANTABLE[row].ports[i] as usize) < IO_PORTS, "a port past the bitmap"); + i += 1; + } + row += 1; + } +}; + +/// A routed line: the I/O APIC input it arrives on. +pub type Line = Gsi; + +/// Point ISA line `irq` at the row's vector on the CPU every device interrupt +/// targets, masked. +pub fn route(row: usize, irq: u8) -> Result { + let line = ioapic::gsi_for_isa_irq(irq).ok_or_else(|| String::from("no I/O APIC"))?; + ioapic::route( + line.gsi, + super::idt::ISA_VECTORS[row], + crate::drivers::pci::MSG_DEST, + line.trigger, + line.polarity, + ) + .map_err(|why| format!("{why:?}"))?; + Ok(line.gsi) +} + +pub fn set_masked(line: Line, masked: bool) { + ioapic::set_masked(line, masked).expect("a line `route` placed has a unit"); +} + +/// Open every row's ports if `pid` holds that row and close them otherwise, on +/// this CPU: at every switch, with `pid` the incoming task's process. +pub fn switch_to(pid: Option) { + for (row, grantable) in GRANTABLE.iter().enumerate() { + let open = pid.is_some_and(|pid| crate::isa::bound_to(row, pid)); + for &port in grantable.ports { + super::percpu::set_port_open(port, open); + } + } +} + +/// An `in` or `out` as decoded from the bytes at a faulting instruction. +#[derive(Clone, Copy)] +pub struct PortAccess { + pub out: bool, + pub port: u16, + /// How many ports from `port` on the access spans, every one of which the + /// bitmap must open. + pub bytes: u8, +} + +impl core::fmt::Display for PortAccess { + fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result { + let (verb, way) = if self.out { ("out", "to") } else { ("in", "from") }; + write!(f, "{verb} of {} byte(s) {way} port {:#06x}", self.bytes, self.port) + } +} + +/// The port access `code` begins with, or `None` for any other instruction. +/// What a Ring 3 #GP names, since its error code (0) says nothing; `dx` is +/// what the forms that take their port from DX read. +/// +/// Up to two prefixes of those an `in`/`out` can carry: operand size (which +/// halves a 4-byte access), a REP for the string forms, and REX. +pub const fn port_access(code: [u8; 4], dx: u16) -> Option { + const fn prefix(byte: u8) -> bool { + matches!(byte, 0x66 | 0xF2 | 0xF3 | 0x40..=0x4F) + } + // Destructured rather than indexed: the crash report calls this, and + // nothing on that path may panic. + let (half, op, imm) = match code { + [a, b, op, imm] if prefix(a) && prefix(b) => (a == 0x66 || b == 0x66, op, imm), + [a, op, imm, _] if prefix(a) => (a == 0x66, op, imm), + [op, imm, _, _] => (false, op, imm), + }; + let wide = if half { 2 } else { 4 }; + let (out, port, bytes) = match op { + 0xE4 => (false, imm as u16, 1), + 0xE5 => (false, imm as u16, wide), + 0xE6 => (true, imm as u16, 1), + 0xE7 => (true, imm as u16, wide), + 0xEC | 0x6C => (false, dx, 1), + 0xED | 0x6D => (false, dx, wide), + 0xEE | 0x6E => (true, dx, 1), + 0xEF | 0x6F => (true, dx, wide), + _ => return None, + }; + Some(PortAccess { out, port, bytes }) +} + +const _: () = { + const fn is(access: Option, out: bool, port: u16, bytes: u8) -> bool { + match access { + Some(a) => a.out == out && a.port == port && a.bytes == bytes, + None => false, + } + } + // `in al, 0x61`, `out 0x64, al`, `in eax, dx`, `in ax, dx` and `rep outsb`. + assert!(is(port_access([0xE4, 0x61, 0, 0], 0), false, 0x61, 1)); + assert!(is(port_access([0xE6, 0x64, 0, 0], 0), true, 0x64, 1)); + assert!(is(port_access([0xED, 0, 0, 0], 0x60), false, 0x60, 4)); + assert!(is(port_access([0x66, 0xED, 0, 0], 0x60), false, 0x60, 2)); + assert!(is(port_access([0xF3, 0x6E, 0, 0], 0x3F8), true, 0x3F8, 1)); + // `mov eax, 0x61` (B8) and `hlt` are no port access. + assert!(port_access([0xB8, 0x61, 0, 0], 0).is_none()); + assert!(port_access([0xF4, 0, 0, 0], 0).is_none()); +}; diff --git a/kernel/src/device.rs b/kernel/src/device.rs index 8fe19f0fea8..dec40422edf 100644 --- a/kernel/src/device.rs +++ b/kernel/src/device.rs @@ -42,11 +42,13 @@ pub struct Claim { /// What one claim holds. A class is at most one device on this machine and a /// per-class flag says whether it is taken; a PCI function is one of several, /// 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`). +/// view's own hold on the blocks (`block::Partition::of`); an ISA function's is +/// its `isa` row's. enum Claimed { Class(DeviceType), PciFunction(usize), Partition(crate::block::Partition), + Isa(usize), } impl Claim { @@ -71,7 +73,7 @@ impl Claim { pub(crate) fn partition(&self) -> Option<&crate::block::Partition> { match &self.what { Claimed::Partition(view) => Some(view), - Claimed::Class(_) | Claimed::PciFunction(_) => None, + Claimed::Class(_) | Claimed::PciFunction(_) | Claimed::Isa(_) => None, } } } @@ -85,6 +87,7 @@ impl Drop for Claim { Claimed::PciFunction(slot) => crate::pcidev::release(slot), // The view drops with this, and its hold with the last clone of it. Claimed::Partition(_) => {} + Claimed::Isa(row) => crate::isa::release(row), } } } @@ -174,6 +177,13 @@ pub fn try_claim(class: DeviceType, selector: [u64; 2]) -> Result { + let set = toyos_abi::syscall::IsaId::from_wire(selector).ok_or(ClaimError::Absent)?; + // The row's own guard, taken inside as a PCI slot's is. + let row = crate::isa::claim(set)?; + let claim = Claim { what: Claimed::Isa(row) }; + Ok(DeviceClaim::new(class, DeviceInfo::Isa(set, row), claim)) + } DeviceType::HdaAudio => { let (info, pcm) = crate::drivers::hda::info().ok_or(ClaimError::Absent)?; let claim = Claim::acquire(class)?; diff --git a/kernel/src/drivers/panic_console/mod.rs b/kernel/src/drivers/panic_console/mod.rs index bd46003575e..3b240b6181c 100644 --- a/kernel/src/drivers/panic_console/mod.rs +++ b/kernel/src/drivers/panic_console/mod.rs @@ -8,7 +8,7 @@ //! //! The two holds this module ends a panic in — [`page_forever`] and //! [`hold_the_panel`] — are also where `crate::panic_reboot`'s bound is -//! watched, because the keyboard poll that retires it is here. +//! watched. Neither reads input: a dead kernel takes none. mod access; mod latch; @@ -18,7 +18,6 @@ use core::cell::UnsafeCell; use core::sync::atomic::{AtomicBool, AtomicU32, AtomicU64, AtomicUsize, Ordering}; use toyos_abi::boot::{KernelArgs, MemoryMapEntry}; -use toyos_ps2::{KeyDecoder, KeyOutcome}; use crate::log; use crate::panic_reboot::Bound; @@ -669,7 +668,7 @@ pub fn seal_wedge(said: core::fmt::Arguments) { /// after `panic_flush`, on the CPU whose [`render`] claimed [`FATAL`]; the /// handler's other two exits reach [`hold_the_panel`] the same way, and every /// one of the three is the last call its CPU makes. -pub fn page_forever(mut bound: Bound) -> ! { +pub fn page_forever(bound: Bound) -> ! { if !crate::clock::calibrated() { hold_the_panel(bound); } @@ -680,98 +679,33 @@ pub fn page_forever(mut bound: Bound) -> ! { if pages < 2 { hold_the_panel(bound); } - // `None` is the screenful [`render`] already painted, not a numbered page, so the first key reaches either end. + // `None` is the screenful [`render`] already painted, not a numbered page. let mut shown: Option = None; - let mut keys = KeyDecoder::new(); - // Once steered, the cycle is his: there is no way back to automatic paging. - let mut steered = false; // Spins rather than `hlt`: nothing would wake it, and re-arming the LAPIC timer would dispatch the scheduler mid-panic. loop { - let step = hold((!steered).then_some(PAGE_HOLD.nanos()), &mut keys, &mut bound); - steered |= step.is_some(); - let next = match (shown, step.unwrap_or(PageKey::Down)) { - (None, PageKey::Down) => 0, - (None, PageKey::Up) => pages - 1, - (Some(page), PageKey::Down) => (page + 1) % pages, - (Some(page), PageKey::Up) => (page + pages - 1) % pages, - }; + let until = crate::clock::nanos_since_boot().saturating_add(PAGE_HOLD.nanos()); + while crate::clock::nanos_since_boot() < until { + bound.check(); + core::hint::spin_loop(); + } + let next = shown.map_or(0, |page| (page + 1) % pages); paint(Fill::Fatal, text, Page::Nth(next), Watch::No, || false); shown = Some(next); } } -/// Keep the panel as it is until `bound` resets the machine, or for good once a -/// key has retired it. The panic path's terminal hold wherever there is no -/// second page to cycle — and the whole of it on a machine with no panel at all. -/// -/// One poller: every caller is the CPU that claimed [`FATAL`], because two CPUs -/// reading port 0x60 would each see half of every scancode. -pub fn hold_the_panel(mut bound: Bound) -> ! { - let mut keys = KeyDecoder::new(); - while bound.is_armed() { - read_key(&mut keys, &mut bound); - bound.check(); - core::hint::spin_loop(); - } - // Nothing left to wait for, so this CPU costs the machine no power. - crate::arch::cpu::halt() -} - -/// One byte off the controller, folded into `keys`; a key **press** retires -/// `bound`, since pressing one is how the person reading the panel says he is -/// there. `None` is a poll that found nothing, which is not the decoder's -/// `Pending`. -/// -/// The pointer shares the port; its packet bytes look like scancodes to -/// anything that does not skip them. [`i8042::poll_byte`] is an `inb` — no -/// lock, no MMIO. -/// -/// [`i8042::poll_byte`]: crate::arch::keyboard_controller::poll_byte -fn read_key(keys: &mut KeyDecoder, bound: &mut Bound) -> Option { - let (byte, false) = crate::arch::keyboard_controller::poll_byte()? else { - return None; - }; - let outcome = keys.feed(byte); - // A make code and nothing else. A break code is the release of a key - // pressed before this panel existed, and a controller's own byte — an ACK, - // a self-test result — is the hardware answering itself; neither is a - // person saying he is here to read it. - if matches!(outcome, KeyOutcome::Key { pressed: true, .. }) { - bound.retire(); +/// Keep the panel as it is until `bound` resets the machine. The panic path's +/// terminal hold wherever there is no second page to cycle — and the whole of +/// it on a machine with no panel at all. +pub fn hold_the_panel(bound: Bound) -> ! { + if !bound.is_armed() { + // Nothing to wait for, so this CPU costs the machine no power. + crate::arch::cpu::halt() } - Some(outcome) -} - -/// Which way the next paint moves. -#[derive(Clone, Copy)] -enum PageKey { - Up, - Down, -} - -/// HID usages: what the wire decoder emits, not scancodes. -const HID_PAGE_UP: u8 = 0x4B; -const HID_PAGE_DOWN: u8 = 0x4E; - -/// Wait for a page key, giving up after `nanos`; `None` means the deadline -/// expired. `bound` is retired by any key and resets the machine at its own -/// expiry, so an unattended panel pages until it is over and no longer. -fn hold(nanos: Option, keys: &mut KeyDecoder, bound: &mut Bound) -> Option { - let target = nanos.map(|n| crate::clock::nanos_since_boot().saturating_add(n)); - while target.is_none_or(|t| crate::clock::nanos_since_boot() < t) { - match read_key(keys, bound) { - Some(KeyOutcome::Key { usage: HID_PAGE_UP, pressed: true }) => { - return Some(PageKey::Up); - } - Some(KeyOutcome::Key { usage: HID_PAGE_DOWN, pressed: true }) => { - return Some(PageKey::Down); - } - _ => {} - } + loop { bound.check(); core::hint::spin_loop(); } - None } /// Repaint at a boot phase boundary, so a machine that wedges later still shows how far it got. diff --git a/kernel/src/drivers/pci.rs b/kernel/src/drivers/pci.rs index 5e2737de802..334547a685c 100644 --- a/kernel/src/drivers/pci.rs +++ b/kernel/src/drivers/pci.rs @@ -29,7 +29,7 @@ pub const MSIX_ENTRY: u16 = 0; // Every device interrupt in this kernel targets cpu0, named as a destination for // the message and for the unit to put in an entry. -const MSG_DEST: u32 = 0; +pub(crate) const MSG_DEST: u32 = 0; /// No requester id: a bus/device/function is sixteen bits, so this is none of them. pub(crate) const NO_FUNCTION: u32 = u32::MAX; diff --git a/kernel/src/isa.rs b/kernel/src/isa.rs new file mode 100644 index 00000000000..2024cf5fd6a --- /dev/null +++ b/kernel/src/isa.rs @@ -0,0 +1,180 @@ +//! A legacy ISA function driven by a process: the I/O ports it decodes, +//! opened in the CPU's I/O permission bitmap, and the lines it raises, +//! answered as interrupt records on the claim. +//! +//! **What can be claimed is the architecture's [`GRANTABLE`] table, matched +//! exactly.** A port no row names is never opened, a set that is not a whole +//! row is refused rather than trimmed to one, and a row this kernel drives +//! itself is refused by name. +//! +//! **The ports belong to a process, not to the handle.** The first read of the +//! claim binds them to the process that reads it ([`bind`]); from then until +//! that process's teardown ([`process_ends`]) every switch onto one of its +//! 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 +//! 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. +//! +//! **A line is routed once per boot and masked while no claim holds it**, since +//! an interrupt-remapping entry is never given back. Its ISR counts into the +//! record a claimed PCI function's does, read back the same way. + +use alloc::vec::Vec; +use core::sync::atomic::{AtomicU32, Ordering}; + +use toyos_abi::pci::DeviceIrqRecord; +use toyos_abi::syscall::IsaId; + +use crate::arch::pio::{self, GRANTABLE}; +use crate::device::ClaimError; +use crate::pcidev::record::Interrupt; +use crate::process::Pid; +use crate::sync::Lock; +use crate::watch::Watch; + +/// One function a process may be handed whole. +pub struct Grantable { + /// What the log calls it. + pub name: &'static str, + /// Ascending, as [`IsaId`] spells them. + pub ports: &'static [u16], + pub irqs: &'static [u8], + /// Whether this kernel drives the function itself, which no claim shares. + pub kernel_drives: fn() -> bool, +} + +/// How many rows any architecture's table has; a static array per row below. +const MAX_ROWS: usize = 1; +const _: () = assert!(GRANTABLE.len() <= MAX_ROWS, "every grantable row needs its state"); + +/// No process holds the row's ports. +const NOBODY: u32 = Pid::MAX.0; + +struct Row { + /// A claim on the row exists. + minted: bool, + /// The row's lines, routed by its first claim and kept; `Err` is a line + /// this machine could not route, which refuses every claim after it too. + lines: Option, ()>>, +} + +static ROWS: [Lock; MAX_ROWS] = + [const { Lock::new(Row { minted: false, lines: None }) }; MAX_ROWS]; + +/// The pid whose threads the row's ports are open for, or [`NOBODY`]; read by +/// every context switch, which takes no lock. +static BOUND: [AtomicU32; MAX_ROWS] = [const { AtomicU32::new(NOBODY) }; MAX_ROWS]; + +static IRQ: [Interrupt; MAX_ROWS] = [const { Interrupt::new() }; MAX_ROWS]; + +/// What a claim's poll waits on, one per row. +static WATCHES: [Watch; MAX_ROWS] = [const { Watch::new() }; MAX_ROWS]; + +/// Mint the claim on the row `set` names, lines routed and unmasked. +pub fn claim(set: IsaId) -> Result { + let row = GRANTABLE + .iter() + .position(|g| { + set.ports().eq(g.ports.iter().copied()) && set.irqs().eq(g.irqs.iter().copied()) + }) + .ok_or(ClaimError::Absent)?; + let grantable = &GRANTABLE[row]; + if (grantable.kernel_drives)() { + return Err(ClaimError::KernelDriven); + } + let mut state = ROWS[row].lock(); + if state.minted || BOUND[row].load(Ordering::Acquire) != NOBODY { + return Err(ClaimError::Owned); + } + let lines = state.lines.get_or_insert_with(|| { + grantable + .irqs + .iter() + .map(|&irq| { + pio::route(row, irq).map_err(|why| { + log!("isa: {} line {irq} not routable: {why}", grantable.name); + }) + }) + .collect() + }); + let Ok(lines) = lines else { return Err(ClaimError::Unusable) }; + IRQ[row].clear(); + for &line in lines.iter() { + pio::set_masked(line, false); + } + state.minted = true; + Ok(row) +} + +/// The claim's last handle went: its lines masked, its record emptied. The +/// ports stay with the process that bound them until that process ends. +pub fn release(row: usize) { + let mut state = ROWS[row].lock(); + if let Some(Ok(lines)) = &state.lines { + for &line in lines { + pio::set_masked(line, true); + } + } + IRQ[row].clear(); + state.minted = false; +} + +/// 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`] +/// mints none while a process holds its ports. +pub fn bind(row: usize, pid: Pid) { + match BOUND[row].compare_exchange(NOBODY, pid.raw(), Ordering::AcqRel, Ordering::Acquire) { + Ok(_) => log!("isa: {}'s ports are pid {pid}'s", GRANTABLE[row].name), + Err(held) => assert!(held == pid.raw(), "isa: row {row} is pid {held}'s, and pid {pid} bound it"), + } + // This thread is already running, so no switch opens them for it. + let _irq = crate::arch::IrqGuard::close(); + pio::switch_to(Some(pid)); +} + +/// Whether `pid` holds the row's ports. +pub fn bound_to(row: usize, pid: Pid) -> bool { + BOUND[row].load(Ordering::Acquire) == pid.raw() +} + +/// Called from the process teardown, once every thread has left. +pub fn process_ends(pid: Pid) { + for (row, bound) in BOUND.iter().enumerate().take(GRANTABLE.len()) { + if bound.compare_exchange(pid.raw(), NOBODY, Ordering::AcqRel, Ordering::Relaxed).is_ok() { + log!("isa: {}'s ports went back with pid {pid}", GRANTABLE[row].name); + } + } +} + +/// The interrupts since the last read, or `None` for none. +pub fn take_record(row: usize) -> Option { + IRQ[row].take().map(|count| DeviceIrqRecord { count }) +} + +pub fn has_irq(row: usize) -> bool { + IRQ[row].armed() +} + +/// Called from the row's ISR: no lock, no allocation. +pub fn isr(row: usize) { + IRQ[row].took(); +} + +/// Turn every interrupt taken since the last pass into a wake. +pub fn drain_pending() { + for (row, irq) in IRQ.iter().enumerate().take(GRANTABLE.len()) { + if !irq.take_pending() { + continue; + } + if irq.take_unannounced() { + log!("isa: {} took its first interrupt", GRANTABLE[row].name); + } + WATCHES[row].post(); + } +} + +pub fn watch(row: usize) -> &'static Watch { + &WATCHES[row] +} diff --git a/kernel/src/main.rs b/kernel/src/main.rs index ef64ca60a95..59dc0ff8926 100644 --- a/kernel/src/main.rs +++ b/kernel/src/main.rs @@ -89,6 +89,7 @@ mod pipe; mod device; mod pcidev; +mod isa; mod gpu; mod user_ptr; mod vma; diff --git a/kernel/src/object/device.rs b/kernel/src/object/device.rs index 2694474e484..fa65bbb2ad0 100644 --- a/kernel/src/object/device.rs +++ b/kernel/src/object/device.rs @@ -28,6 +28,9 @@ pub enum DeviceInfo { /// 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 two scanout buffers and the cursor plane. @@ -71,6 +74,7 @@ impl DeviceInfo { // is what `SYS_DEVICE_DMA_ALLOC` answers later. Self::PciFunction(info, _) => info.as_bytes().into(), Self::Partition(info) => info.as_bytes().into(), + Self::Isa(set, _) => set.wire().iter().flat_map(|word| word.to_ne_bytes()).collect(), Self::Hda(info, pcm) => { let mut info = *info; info.pcm = install_buffers(table, &[pcm])?[0]; @@ -93,6 +97,8 @@ pub struct DeviceClaim { /// 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, @@ -112,10 +118,15 @@ impl DeviceClaim { DeviceInfo::PciFunction(_, slot) => Some(*slot), _ => None, }; + let isa_row = match &info { + DeviceInfo::Isa(_, 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), @@ -134,6 +145,11 @@ impl DeviceClaim { self.pci_slot.map(usize::from) } + /// Which `isa` row this claim holds, for a claim on an ISA function. + pub fn isa_row(&self) -> Option { + self.isa_row + } + /// The view a partition claim transfers through: `None` for a claim on /// anything else, and once the last handle has let the partition go. /// diff --git a/kernel/src/object/ops.rs b/kernel/src/object/ops.rs index 3808154fb29..c6608579cb9 100644 --- a/kernel/src/object/ops.rs +++ b/kernel/src/object/ops.rs @@ -257,6 +257,9 @@ pub fn read_watch(object: &KObjectRef) -> Option { device_registry::DeviceType::PciFunction => { d.pci_slot().map(|slot| WatchRef::Static(crate::pcidev::watch(slot))) } + device_registry::DeviceType::Isa => { + d.isa_row().map(|row| WatchRef::Static(crate::isa::watch(row))) + } device_registry::DeviceType::HdaAudio | device_registry::DeviceType::VirtioSound => { Some(WatchRef::Static(&crate::drivers::AUDIO_WATCH)) } @@ -301,6 +304,7 @@ fn close_ends_polls(object: &KObjectRef) -> bool { } device_registry::DeviceType::Mouse | device_registry::DeviceType::PciFunction + | device_registry::DeviceType::Isa | device_registry::DeviceType::HdaAudio | device_registry::DeviceType::VirtioSound | device_registry::DeviceType::Framebuffer @@ -407,6 +411,25 @@ pub fn read_device( buf.write_at(0, record_bytes(&record)); Some(toyos_abi::pci::DeviceIrqRecord::SIZE as u64) } + // 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 => { + let row = claim.isa_row().expect("an ISA claim knows its row"); + let pid = crate::process::current_process(); + if !claim.info_read() { + crate::isa::bind(row, pid); + 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)?; + buf.write_at(0, record_bytes(&record)); + Some(toyos_abi::pci::DeviceIrqRecord::SIZE as u64) + } device_registry::DeviceType::HdaAudio => { if !claim.info_read() { return Some(claim.describe(table, buf)); @@ -598,7 +621,9 @@ pub fn fstat(object: &KObjectRef) -> Stat { device_registry::DeviceType::Keyboard => FileType::Keyboard, device_registry::DeviceType::Mouse => FileType::Mouse, device_registry::DeviceType::Framebuffer => FileType::Framebuffer, - device_registry::DeviceType::PciFunction => FileType::Unknown, + device_registry::DeviceType::PciFunction | device_registry::DeviceType::Isa => { + FileType::Unknown + } device_registry::DeviceType::HdaAudio | device_registry::DeviceType::VirtioSound => FileType::Unknown, device_registry::DeviceType::Partition => FileType::Unknown, @@ -767,7 +792,8 @@ fn partition_fsync(claim: &DeviceClaim) -> u64 { | device_registry::DeviceType::Framebuffer | device_registry::DeviceType::HdaAudio | device_registry::DeviceType::VirtioSound - | device_registry::DeviceType::PciFunction => { + | device_registry::DeviceType::PciFunction + | device_registry::DeviceType::Isa => { return SyscallError::PermissionDenied.to_u64(); } } @@ -836,6 +862,9 @@ pub fn has_data(object: &KObjectRef) -> bool { device_registry::DeviceType::PciFunction => { !d.info_read() || d.pci_slot().is_some_and(crate::pcidev::has_irq) } + device_registry::DeviceType::Isa => { + !d.info_read() || d.isa_row().is_some_and(crate::isa::has_irq) + } device_registry::DeviceType::Framebuffer => true, device_registry::DeviceType::Partition => true, device_registry::DeviceType::HdaAudio => { diff --git a/kernel/src/panic.rs b/kernel/src/panic.rs index 55b778d63d6..e022712919a 100644 --- a/kernel/src/panic.rs +++ b/kernel/src/panic.rs @@ -338,8 +338,8 @@ pub fn last_words( } /// Halt all CPUs: stop the others, flush pending log output, then hold this -/// machine's panel until a key retires the reboot bound or the bound returns it -/// to firmware. Every fatal path's one funnel. +/// machine's panel until the reboot bound returns it to firmware. Every fatal +/// path's one funnel. // panic_flush bypasses the log-ring and serial locks — once the others are stopped a wedged holder never releases them, so taking them normally could deadlock. pub fn halt_all_cpus() -> ! { // Before the halt and the panel: from here this machine holds a report for @@ -371,7 +371,7 @@ pub fn halt_all_cpus() -> ! { // Must follow the flush — it's the deepest stack this path reaches. crate::arch::trap::report_fault_stack(); // page_forever runs strictly after the flush: it is an unbounded loop and may only run once the serial report is out. - // Only the CPU that painted watches the bound; the rest halt below, since two CPUs polling one keyboard would split every key. + // Only the CPU that painted watches the bound; the rest halt below. if painted { crate::drivers::panic_console::page_forever(bound); } diff --git a/kernel/src/panic_reboot.rs b/kernel/src/panic_reboot.rs index b36141ae16f..465234b7677 100644 --- a/kernel/src/panic_reboot.rs +++ b/kernel/src/panic_reboot.rs @@ -1,8 +1,6 @@ //! What a panicked kernel does with the machine once its report is on the //! panel: it holds the panel for [`toyos_tco::PANIC_BOUND_MS`] and then returns -//! the machine to firmware. A key press retires the bound for good — a key is -//! how a person at the machine says the panel is being read — and nobody -//! pressing one inside the bound means nobody is there to read it. +//! the machine to firmware. //! //! **The bound is carried in counter ticks, not nanoseconds** (`cpu::counter`: //! the TSC, the generic timer's count). A panic may land before `clock::init`, @@ -28,12 +26,11 @@ const PANIC_BOUND: Budget = Budget::of( ); /// `tco-fast`'s counterpart for this bound: a judge cannot spend the shipped -/// minute per boot, and its control cannot press a key inside a bound shorter -/// than the round trip that presses it. +/// minute per boot. #[cfg(feature = "boot-actuators")] const FAST_BOUND: Budget = Budget::of( Duration::from_secs(5), - "a guest reaches the reset inside one test, and a control still beats it to the keyboard", + "a guest reaches the reset inside one test", ); /// Whether a reboot is armed on this panic, and when. @@ -41,17 +38,12 @@ const FAST_BOUND: Budget = Budget::of( pub enum Bound { /// Reset the machine at this `cpu::counter` reading. At(u64), - /// Hold the panel: somebody is reading it, or nothing here could time a - /// wait, or this machine has no reset register to write. + /// Hold the panel: nothing here could time a wait, or this machine has no + /// reset register to write. Held, } impl Bound { - /// A key arrived: the machine is that person's from here on. - pub fn retire(&mut self) { - *self = Self::Held; - } - /// Reset the machine if the bound has passed; every wait on the panic path /// calls this, and it is the only place that decides the reset has come due. pub fn check(self) { @@ -121,12 +113,9 @@ pub fn arm(on_the_record: bool) -> Bound { match (deadline(budget), acpi::can_reboot()) { (Some((cycles, source)), true) => { if on_the_record { - alert!( - "{ARMED} in {secs} s unless a key is pressed, timed by {}", - source.named() - ); + alert!("{ARMED} in {secs} s, timed by {}", source.named()); } else { - serial::panic_raw(b"panic: rebooting unless a key is pressed\n"); + serial::panic_raw(b"panic: rebooting\n"); } Bound::At(cycles) } @@ -159,10 +148,7 @@ pub fn arm(on_the_record: bool) -> Bound { /// Return the machine to firmware. The second of this path's two lines, and it /// goes out raw: the log has already been flushed and drained by here. pub fn reboot_now() -> ! { - serial::panic_raw( - b"\npanic: no key inside the bound, so nobody is here: returning this machine to \ - firmware\n", - ); + serial::panic_raw(b"\npanic: the bound is spent: returning this machine to firmware\n"); // Not `acpi::reboot`: its flush waits on the console wire, and a CPU this // panic stopped may be holding it. acpi::reset_now() diff --git a/kernel/src/pcidev/mod.rs b/kernel/src/pcidev/mod.rs index 0fc518dadb6..fe96bf4c1bb 100644 --- a/kernel/src/pcidev/mod.rs +++ b/kernel/src/pcidev/mod.rs @@ -94,7 +94,7 @@ /// No `crate::` reference, so `kernel-loom` compiles it and models the /// interleaving no guest test lands on. -mod record; +pub(crate) mod record; use alloc::string::String; use alloc::sync::Arc; diff --git a/kernel/src/process.rs b/kernel/src/process.rs index f8680f7be27..b7cc88fc644 100644 --- a/kernel/src/process.rs +++ b/kernel/src/process.rs @@ -962,6 +962,8 @@ fn teardown_resources( crate::arch::trap::log_unclaimed(); ops::close_all(&mut data.handles); + // Every thread has left, and none returns to Ring 3 to use them. + crate::isa::process_ends(pid); data.elf.elf_alloc.take(); data.elf.loaded_libs.clear(); data.mmap_regions.clear(); diff --git a/kernel/src/sched/driver.rs b/kernel/src/sched/driver.rs index 43233c12614..48fec021f43 100644 --- a/kernel/src/sched/driver.rs +++ b/kernel/src/sched/driver.rs @@ -673,10 +673,11 @@ fn drain_irqs(entered: super::dump::Entered) { crate::drivers::panic_console::hold_report(); if crate::irq_ring::take(crate::irq_ring::IrqSource::UserDev).is_some() { - // Which claim it was is the per-slot flag `pcidev` keeps; the record - // here says only that a pass is owed, so one function's interrupt does - // not wake every user driver in the machine. + // Which claim it was is the per-slot flag `pcidev` and `isa` keep; the + // record here says only that a pass is owed, so one function's + // interrupt does not wake every user driver in the machine. crate::pcidev::drain_pending(); + crate::isa::drain_pending(); } if crate::irq_ring::take(crate::irq_ring::IrqSource::Audio).is_some() { // Both backends share one watch, so a second would need the parking side diff --git a/kernel/src/syscall/device.rs b/kernel/src/syscall/device.rs index 8472b219b3d..6b6b5fabfce 100644 --- a/kernel/src/syscall/device.rs +++ b/kernel/src/syscall/device.rs @@ -131,7 +131,7 @@ pub(super) fn sys_device_claim(syscap: RawHandle, class: u64, selector: [u64; 2] | device::DeviceType::HdaAudio | device::DeviceType::VirtioSound => 0, device::DeviceType::PciFunction => 1, - device::DeviceType::Partition => 2, + device::DeviceType::Partition | device::DeviceType::Isa => 2, }; if selector[read..].iter().any(|&word| word != 0) { return SyscallError::InvalidArgument.to_u64(); @@ -397,7 +397,8 @@ pub(super) fn sys_partition_transfer( | device::DeviceType::Framebuffer | device::DeviceType::HdaAudio | device::DeviceType::VirtioSound - | device::DeviceType::PciFunction => { + | device::DeviceType::PciFunction + | device::DeviceType::Isa => { drop(claim); return crate::object::HandleError::WrongType { held: class.class_name(), diff --git a/src/sourcegate.rs b/src/sourcegate.rs index e3e02dd9bcf..44e68948102 100644 --- a/src/sourcegate.rs +++ b/src/sourcegate.rs @@ -1477,6 +1477,7 @@ const ARCH_RULES: &[PlaceRule] = &[ ("tests/toyos-rust-tests/src/bin/fpu_isolation.rs", USERLAND_ASM), ("tests/toyos-rust-tests/src/bin/gsbase_probe.rs", USERLAND_ASM), ("tests/toyos-rust-tests/src/bin/inventory_bounds.rs", USERLAND_ASM), + ("tests/toyos-rust-tests/src/bin/isa_grant.rs", USERLAND_ASM), ("tests/toyos-rust-tests/src/bin/log_hold.rs", USERLAND_ASM), ("tests/toyos-rust-tests/src/bin/mmap_prot.rs", USERLAND_ASM), ("tests/toyos-rust-tests/src/bin/nmi_window_spin.rs", USERLAND_ASM), diff --git a/tests/common/power.rs b/tests/common/power.rs index 34ccec04762..fe80c4bc858 100644 --- a/tests/common/power.rs +++ b/tests/common/power.rs @@ -780,9 +780,8 @@ const FED_FOR: Duration = Duration::from_secs(20); /// /// A kernel constant does not cross into the harness, so it is written here and /// then *read back*: [`panic_armed`] is the whole arm line including this -/// number, and both tests demand it before they judge anything. A bound that -/// moved in the kernel and not here reds on that line rather than on a stop -/// reason nobody could attribute. +/// number. A bound that moved in the kernel and not here reds on that line +/// rather than on a stop reason nobody could attribute. const PANIC_FAST_SECS: u64 = 5; /// The panic path's arm line, which is also this boot's ready marker: the guest @@ -791,13 +790,10 @@ const PANIC_FAST_SECS: u64 = 5; const PANIC_ARMED_HEAD: &str = "panic: rebooting in"; fn panic_armed() -> String { - format!("{PANIC_ARMED_HEAD} {PANIC_FAST_SECS} s unless a key is pressed, timed by ") + format!("{PANIC_ARMED_HEAD} {PANIC_FAST_SECS} s, timed by ") } -/// A guest whose kernel panicked and armed the bound. `Profile::Metal` for the -/// same reason `screen_pager_keys` needs it: QEMU routes injected keys to one -/// handler per device class, and this is the only GOP profile with an i8042 and -/// no `usb-kbd` to send them to instead. +/// A guest whose kernel panicked and armed the bound. fn panicked() -> BootOptions { BootOptions { profile: qemu::Profile::Metal, @@ -811,11 +807,9 @@ fn panicked() -> BootOptions { /// What a panicked guest that never stopped means where the bound should have /// ended it. const PANICKED_AND_STAYED_UP: &str = - "QEMU never reported stopping: nobody pressed a key and the panicked guest held its panel \ - anyway"; + "QEMU never reported stopping: the panicked guest held its panel past the bound"; -/// A panicked kernel nobody is at returns the machine to firmware itself. -/// [`panic_key_holds`] is the same guest with a key pressed inside the bound. +/// A panicked kernel returns the machine to firmware itself. /// /// The verdict is QEMU's stop reason arriving inside the bound the arm line /// names, which is what the budget below is: a reset that had to wait longer @@ -987,97 +981,33 @@ const RESET_ALLOWANCE: Duration = Duration::from_secs(20); /// The panic path's second line, written raw because the log is already drained /// by then (`kernel/src/panic_reboot.rs`'s `reboot_now`). -const PANIC_REBOOTING: &str = "panic: no key inside the bound, so nobody is here"; +const PANIC_REBOOTING: &str = "panic: the bound is spent"; -/// The control on [`panic_reboots`]: the same guest with one key pressed inside -/// the bound holds its panel and is still there several bounds later. -/// -/// The key is `a`, not a page key: what retires the bound is that somebody is at -/// the machine, and `screen_pager_keys` is where the pager's own two keys are judged. -pub fn panic_key_holds( +/// The control on [`panic_reboots`], and the owner's ruling that a dead kernel +/// takes no input: the same guest with a key pressed inside the bound returns +/// itself to firmware all the same. +pub fn panic_ignores_keys( test_config: &Path, c_bins: &[(String, Vec)], rust_bins: &[(String, Vec)], ) -> Result<(), String> { let mut qemu = QemuInstance::boot_with_options(test_config, c_bins, rust_bins, panicked()); let boot = serial::Serial::boot(&qemu); - // Demanded before the key, so a control that never armed a bound cannot - // pass by holding a panel nothing was counting down. + // Demanded before the key, so a guest that never armed a bound cannot pass + // on a reset something else caused. boot.must_say(&panic_armed())?; + // One monitor per `-qmp` socket: the key goes out before the watch connects. let socket = qemu.qmp_socket().to_path_buf(); qemu::qmp_send_keys(&socket, &[("a", true), ("a", false)]); - - let mut stop = qemu::QmpShutdown::open(qemu.qmp_socket(), qemu.budget(PANEL_HELD_FOR)); - if let Some(seen) = stop.reason() { - let tail = qemu.drain_serial(WAIT); - return Err(format!( - "a key was pressed inside the bound and QEMU stopped this guest anyway, for \ - {seen:?}\n{tail}" - )); - } - - // One monitor per `-qmp` socket, so the shutdown watch is given up before - // the screendump connects. - drop(stop); - // QEMU not exiting is not the claim in this test's name; the panel still - // carrying the report is. The fill and not a line of it, because a key that - // is not a page key leaves the pager unsteered and which page is up when the - // dump is taken is nobody's to say. - let fill = qemu.screendump().fill(); - if fill != FILL_FATAL { - return Err(format!( - "the guest is still up {PANEL_HELD_FOR:?} after the key, but its panel fills \ - {fill:?} and not the fatal {FILL_FATAL:?}: whatever it holds is not the report" - )); - } - - eprintln!(" [power] a key retired the bound and the report held the panel {PANEL_HELD_FOR:?}"); - drop(qemu); - release_alone_does_not_retire(test_config, c_bins, rust_bins) -} - -/// The negative arm on [`panic_key_holds`]: a byte that is not a key **press** -/// leaves the bound armed, and the machine still resets itself. -/// -/// A bare break code is what this injects, because it is the one byte of that -/// class QMP can deliver — a controller's own ACK reaches the port through no -/// monitor command. The claim is the same either way: `read_key` retires on a -/// make code and on nothing else, so a panel nobody pressed a key at is a panel -/// nobody is reading, whatever else the controller said. -fn release_alone_does_not_retire( - test_config: &Path, - c_bins: &[(String, Vec)], - rust_bins: &[(String, Vec)], -) -> Result<(), String> { - let mut qemu = QemuInstance::boot_with_options(test_config, c_bins, rust_bins, panicked()); - let boot = serial::Serial::boot(&qemu); - boot.must_say(&panic_armed())?; - - let socket = qemu.qmp_socket().to_path_buf(); - qemu::qmp_send_keys(&socket, &[("a", false)]); - - let budget = qemu.budget(Duration::from_secs(PANIC_FAST_SECS) + RESET_ALLOWANCE); - let mut stop = qemu::QmpShutdown::open(qemu.qmp_socket(), budget); - let reason = stop.reason(); - let tail = qemu.drain_serial(WAIT); - returned_to_firmware( - reason, - "a key release retired a bound only a key press may retire, and the guest held its panel", - &tail, - )?; - - eprintln!(" [power] a release alone left the bound armed and the guest reset itself"); + let watch = watch_the_bound(&qemu); + let never = "QEMU never reported stopping: a key pressed inside the bound held the panel of a \ + kernel that takes no input"; + let (budget, _) = resets_inside_the_bound(&mut qemu, watch, never)?; + eprintln!(" [power] a key pressed inside the bound, and the guest reset itself inside {budget:?}"); Ok(()) } -/// What `panic_console`'s `Fill::Fatal` paints behind a report — the one thing -/// on that panel which does not depend on the page the pager has up. -const FILL_FATAL: [u8; 3] = [0x60, 0x00, 0x00]; - -/// Several bounds, so the control is not a race the guest won once. -const PANEL_HELD_FOR: Duration = Duration::from_secs(PANIC_FAST_SECS * 4); - /// A line of the first boot's own report, which has to come back out of DRAM on /// the boot after it: the panic's message, so what is recovered is the crash /// and not merely a page that checksummed. diff --git a/tests/common/qemu.rs b/tests/common/qemu.rs index f8a6d81435a..c0dd4f32f7d 100644 --- a/tests/common/qemu.rs +++ b/tests/common/qemu.rs @@ -480,10 +480,7 @@ impl Liveness { /// /// A test that ran out of time has not found the guest doing the wrong thing; /// it has found nothing at all, and the two readings send an agent to opposite -/// places. `screen_pager_keys` reporting `0 page moves over 30 keystrokes` -/// after 0.3 s was bisected as a kernel regression twice in one day by two -/// agents, and the fact it was hiding is that the whole run had collapsed -/// before the guest could answer once. +/// places. /// /// Still red. A guest that stopped answering may have stopped for a reason this /// tree owns, and a status that is not a failure is a status nobody reads. What diff --git a/tests/test-durations b/tests/test-durations index c32248eab77..51d9de35ba7 100644 --- a/tests/test-durations +++ b/tests/test-durations @@ -299,7 +299,6 @@ nvme_wide_sector 3500 operation_nesting 5075 page_cache_partition_offset 6984 panic_before_peripherals_reboots 3227 -panic_key_holds 42033 panic_reboots 9768 pci_capability_walk 5775 pci_claim_caps_truncated 2583 @@ -343,7 +342,6 @@ screen_late_panic 3926 screen_loader_lines 3991 screen_log_absent 1823 screen_paged_scrollback 7384 -screen_pager_keys 16152 screen_panic_muted 4416 shipped_config_boots 3024 shm_release_reclaims 29 diff --git a/tests/toyos-rust-tests/src/bin/isa_grant.rs b/tests/toyos-rust-tests/src/bin/isa_grant.rs new file mode 100644 index 00000000000..4a57679b29c --- /dev/null +++ b/tests/toyos-rust-tests/src/bin/isa_grant.rs @@ -0,0 +1,271 @@ +//! The `isa` claim: the i8042's two ports opened in the I/O permission bitmap +//! for the one process that bound them, and its lines answered as records. +//! +//! Roles, one per machine the harness boots: +//! +//! - `driven`: the kernel drives the i8042 here, and the claim is refused. +//! - `grant`: a machine with no i8042 (`i8042=off`), where the ports float and +//! nothing the kernel drives stands in the way. Every port access that must +//! be refused is made in a child of its own, which prints a marker, makes the +//! one access and must never print again; the kernel's record of each kill +//! is the harness's to read. +//! - `device`: the real controller the kernel gave up on at boot, driven from +//! here until a keystroke arrives as a record and a byte. +//! +//! The children: `unbound` holds the claim and never read it, `bound` read it +//! and steps one port past what it was granted, `unclaimed` holds nothing, and +//! `moved` was handed a claim its parent had already bound. + +use std::os::toyos::process::CommandExt; +use std::process::{Command, Stdio}; +use std::time::{Duration, Instant}; + +use toyos::endow::Endowments; +use toyos::poller::{Poller, READABLE}; +use toyos::syscap::SysCap; +use toyos::{AsHandle, Device}; +use toyos_abi::pci::DeviceIrqRecord; +use toyos_abi::syscall::{self, IsaId, SyscallError, SYSCAP_LABEL}; + +const SELF_PATH: &str = "/system/bin/test_rs_isa_grant"; +const CLAIM_LABEL: &str = "isa-claim"; + +/// The i8042's data and command ports, keyboard and aux lines. +const I8042: &str = "0060,0064:1,12"; +const DATA: u16 = 0x60; +const STATUS: u16 = 0x64; +/// The port just past the data port, which no row names. +const PAST: u16 = 0x61; + +const OBF: u8 = 1 << 0; +const IBF: u8 = 1 << 1; + +/// A controller answers a command in microseconds; this is the liveness +/// ceiling on one that does not, and it panics by name. +const CONTROLLER: Duration = Duration::from_secs(1); +/// The host types once it reads the ready line; a record that has not come in +/// this long is not coming. +const KEYSTROKE: Duration = Duration::from_secs(20); + +/// Set 1's make code for `a`: the controller translates the keyboard's set 2. +const A_MAKE: u8 = 0x1E; + +fn main() { + match std::env::args().nth(1).as_deref() { + Some("driven") => driven(&syscap()), + Some("grant") => grant(&syscap()), + Some("device") => device(&syscap()), + Some("unbound") => unbound(), + Some("bound") => bound(), + Some("unclaimed") => unclaimed(), + Some("moved") => moved(), + other => panic!("isa_grant: unknown role {other:?}"), + } +} + +fn syscap() -> SysCap { + Endowments::get() + .take(SYSCAP_LABEL) + .expect("the test estate is endowed a device-minting capability") +} + +fn set(text: &str) -> IsaId { + IsaId::parse(text).unwrap_or_else(|| panic!("{text:?} is no ISA set")) +} + +fn claim(cap: &SysCap, text: &str) -> Result { + cap.claim_isa(set(text)) +} + +fn inb(port: u16) -> u8 { + let value: u8; + // SAFETY: an `in` has no memory effect; a port this process holds no grant + // for faults it, which is what the refusing roles exist to show. + unsafe { + core::arch::asm!("in al, dx", in("dx") port, out("al") value, options(nomem, nostack)); + } + value +} + +fn outb(port: u16, value: u8) { + // SAFETY: as `inb`; the ports written are the i8042's, granted to this process. + unsafe { + core::arch::asm!("out dx, al", in("dx") port, in("al") value, options(nomem, nostack)); + } +} + +fn driven(cap: &SysCap) { + match claim(cap, I8042) { + Err(SyscallError::PermissionDenied) => {} + other => panic!("isa: the kernel drives the i8042 and a claim on it answered {:?}", other.map(|_| ())), + } + println!("isa: the claim on a controller the kernel drives was refused PermissionDenied"); +} + +/// Read the claim's description, which binds its ports to this process, and +/// check it names what was claimed. +fn bind(claim: &Device) { + let mut words = [0u8; 16]; + let n = claim.read(&mut words).expect("isa: the claim's first read is its description"); + assert_eq!(n, words.len(), "isa: a description of {n} bytes"); + let wire = [ + u64::from_ne_bytes(words[..8].try_into().expect("eight bytes")), + u64::from_ne_bytes(words[8..].try_into().expect("eight bytes")), + ]; + assert_eq!(IsaId::from_wire(wire), Some(set(I8042)), "isa: the description names another set"); +} + +/// Spawn `role` holding `claim`, or nothing; answers its stdout and whether it +/// exited cleanly. +fn child(role: &str, claim: Option) -> (String, bool) { + let mut command = Command::new(SELF_PATH); + command.arg(role).stdout(Stdio::piped()); + if let Some(claim) = claim { + command.endow(CLAIM_LABEL, claim.into_raw().0); + } + let out = command.output().expect("isa: spawn a child role"); + (String::from_utf8_lossy(&out.stdout).into_owned(), out.status.success()) +} + +/// A child that must have printed `marker` and then died at the access after it. +fn killed_after(role: &str, (said, clean): (String, bool), marker: &str) { + assert!(said.contains(marker), "isa: {role} never reached its access: {said:?}"); + assert!(!clean && !said.contains("survived"), "isa: {role} survived its access: {said:?}"); + println!("isa: {role} was ended at {marker:?}"); +} + +fn grant(cap: &SysCap) { + // Nothing but a whole row is a grant: a subset, a missing line and a + // neighbouring port are each no function this machine can hand out. + for part in ["0060:1", "0060,0064:1", "0061,0064:1,12", "0064:12"] { + match claim(cap, part) { + Err(SyscallError::NotFound) => {} + other => panic!("isa: {part:?} answered {:?}, want NotFound", other.map(|_| ())), + } + } + println!("isa: a subset, a missing line and a neighbouring port were each refused NotFound"); + + let held = claim(cap, I8042).expect("isa: the i8042 row is free on a machine with none"); + match claim(cap, I8042) { + Err(SyscallError::AlreadyExists) => {} + other => panic!("isa: a second claim answered {:?}, want AlreadyExists", other.map(|_| ())), + } + killed_after("unbound", child("unbound", Some(held)), "unbound: in from 0x64"); + + // The claim died with its unbound holder, so the row is free again. + let held = claim(cap, I8042).expect("isa: the row came back from a holder that never bound it"); + killed_after("bound", child("bound", Some(held)), "bound: in from 0x61"); + killed_after("unclaimed", child("unclaimed", None), "unclaimed: in from 0x60"); + + // The ports went back with the process that bound them. + let held = claim(cap, I8042).expect("isa: the row came back from a holder that bound it"); + bind(&held); + let status = inb(STATUS); + println!("isa: bound here, status reads {status:#04x}"); + killed_after("moved", child("moved", Some(held)), "moved: in from 0x64"); + // The claim is gone with the child, and the ports are still this process's. + match claim(cap, I8042) { + Err(SyscallError::AlreadyExists) => {} + other => panic!( + "isa: a claim while this process holds the bound ports answered {:?}", + other.map(|_| ()) + ), + } + let _ = inb(STATUS); + println!("isa: a moved claim carries nothing, and the ports stay with the process that bound them"); + println!("===ISA_GRANT_OK==="); +} + +fn taken() -> Device { + Endowments::get().take(CLAIM_LABEL).expect("isa: this role is endowed the claim") +} + +fn unbound() { + let _claim = taken(); + println!("unbound: in from 0x64"); + let _ = inb(STATUS); + println!("unbound: survived"); +} + +fn bound() { + let claim = taken(); + bind(&claim); + // A machine with no i8042 floats its ports: all ones. + println!("bound: 0x64 read {:#04x}", inb(STATUS)); + println!("bound: in from 0x61"); + let _ = inb(PAST); + println!("bound: survived"); +} + +fn unclaimed() { + println!("unclaimed: in from 0x60"); + let _ = inb(DATA); + println!("unclaimed: survived"); +} + +fn moved() { + let claim = taken(); + let mut buf = [0u8; 16]; + match claim.read(&mut buf) { + Err(SyscallError::PermissionDenied) => println!("moved: its read was refused"), + other => panic!("moved: a claim bound to another process answered {other:?}"), + } + println!("moved: in from 0x64"); + let _ = inb(STATUS); + println!("moved: survived"); +} + +/// Spin on the status register until `ready`, or panic naming `what`. +fn wait_status(what: &str, ready: impl Fn(u8) -> bool) { + let by = Instant::now() + CONTROLLER; + while !ready(inb(STATUS)) { + assert!(Instant::now() < by, "isa device: the controller never {what} in {CONTROLLER:?}"); + std::hint::spin_loop(); + } +} + +fn command(byte: u8) { + wait_status("took a command", |s| s & IBF == 0); + outb(STATUS, byte); +} + +fn device(cap: &SysCap) { + let claim = claim(cap, I8042).expect("isa device: the kernel gave the controller up at boot"); + bind(&claim); + // Whatever the kernel's aborted probe left behind. + for _ in 0..32 { + if inb(STATUS) & OBF == 0 { + break; + } + let _ = inb(DATA); + } + assert_eq!(inb(STATUS) & OBF, 0, "isa device: the output buffer never drained"); + // Read the configuration byte, then write it back with the keyboard's line + // on and its clock running (i8042: 0x20 reads it, 0x60 writes it). + command(0x20); + wait_status("answered its configuration", |s| s & OBF != 0); + let config = inb(DATA); + command(0x60); + wait_status("took the configuration", |s| s & IBF == 0); + outb(DATA, (config | 0x01) & !0x10); + command(0xAE); + println!("isa device: config {config:#04x}, keyboard line on"); + println!("===ISA_DEVICE_READY==="); + + let poller = Poller::new(1); + poller.watch(&claim, READABLE, 0); + let mut woke = false; + poller.wait(1, KEYSTROKE.as_nanos() as u64, |_| woke = true); + assert!(woke, "isa device: no record in {KEYSTROKE:?} of the keystroke"); + let mut record = [0u8; DeviceIrqRecord::SIZE]; + let n = syscall::read_nonblock(claim.as_handle(), &mut record) + .expect("isa device: a readable claim reads a record"); + assert_eq!(n, DeviceIrqRecord::SIZE, "isa device: a record of {n} bytes"); + let count = u32::from_ne_bytes(record); + assert!(count >= 1, "isa device: a record counting {count}"); + wait_status("had the byte behind its interrupt", |s| s & OBF != 0); + let byte = inb(DATA); + assert_eq!(byte, A_MAKE, "isa device: the interrupt carried {byte:#04x}, not `a`'s make"); + println!("isa device: {count} interrupt(s), scancode {byte:#04x}"); + println!("===ISA_DEVICE_OK==="); +} diff --git a/tests/toyos.rs b/tests/toyos.rs index ff90ef5aae8..9baf96bd040 100644 --- a/tests/toyos.rs +++ b/tests/toyos.rs @@ -247,6 +247,8 @@ const RUST_SKIP: &[&str] = &[ // Meaningful only on `MetalNoUsb`, where no input source exists; on every // other machine both claims succeed. `input_claim_absent` runs it. "input_absent", + // One role per machine shape, each named by the `isa_` test that boots it. + "isa_grant", // Needs a display whose mode can change, which is `Profile::VirtioGpu` // alone; the shared boot has no display at all. `gpu_set_resolution` runs // it there, and `iommu_gpu_scanout_swap` the second. @@ -549,7 +551,6 @@ const SCREEN_TESTS: &[(&str, Sched, Tier)] = &[ // `screen_blocked_dump` has one but paints through `paint_report` rather // than through `halt_all_cpus`. ("screen_fatal_halt_composited", Sched::Parallel, Tier::Nightly), - ("screen_pager_keys", Sched::Serial, Tier::Nightly), // AArch64 guests on QEMU `virt`: local, because no CI runner boots one yet. ("virt_early_panic", Sched::Parallel, Tier::Local), ("virt_early_fault", Sched::Parallel, Tier::Local), @@ -652,6 +653,14 @@ const MACHINE_TESTS: &[(&str, Sched, Tier)] = &[ ("input_merge", Sched::Parallel, Tier::Weekly), ("metal_sim_input", Sched::Parallel, Tier::Weekly), ("input_claim_absent", Sched::Parallel, Tier::Weekly), + // The `isa` claim, a boot each: refused where the kernel drives the i8042, + // its ports granted and refused by the I/O permission bitmap where there is + // none, and a real controller the kernel gave up on driven to a keystroke. + // Every verdict is a guest's line or the kernel's record of a kill; the one + // wait is on the guest's own ready line. + ("isa_claim_refused_where_the_kernel_drives", Sched::Parallel, Tier::Fast), + ("isa_ports_are_the_binders_alone", Sched::Parallel, Tier::Fast), + ("isa_lines_reach_their_holder", Sched::Parallel, Tier::Nightly), // One boot; every verdict is a PPM header field or a console line, and no // clock is in any of them. ("gpu_set_resolution", Sched::Parallel, Tier::Fast), @@ -985,8 +994,8 @@ const MACHINE_TESTS: &[(&str, Sched, Tier)] = &[ // The same verdict from inside `percpu::init_bsp`: the earliest point a // panic is reportable, and the window the owner's T14 stops in. ("panic_before_peripherals_reboots", Sched::Parallel, Tier::Nightly), - // Serial like `watchdog_fed`: its verdict is that nothing happened for a span of host clock. - ("panic_key_holds", Sched::Serial, Tier::Weekly), + // Its control: a key pressed inside the bound, and the same stop reason. + ("panic_ignores_keys", Sched::Parallel, Tier::Weekly), // The boot chain's three answers. The two chain names each watch a guest // take its own reset and read the pass after it, so both are anchored to // the bound the first boot counts down. @@ -1518,6 +1527,9 @@ const CARRIES: &[(&str, &[&str])] = &[ ("launcher_refusals", &["test_rs_launcher_refusals"]), ("spawn_cwd", &["test_rs_spawn_cwd"]), ("input_claim_absent", &["test_rs_input_absent"]), + ("isa_claim_refused_where_the_kernel_drives", &["test_rs_isa_grant"]), + ("isa_ports_are_the_binders_alone", &["test_rs_isa_grant"]), + ("isa_lines_reach_their_holder", &["test_rs_isa_grant"]), ("gpu_set_resolution", &["test_rs_gpu_set_resolution"]), ("iommu_gpu_scanout_swap", &["test_rs_gpu_scanout_swap"]), ("userdev_dma_fault", &["test_rs_log_origin"]), @@ -5779,7 +5791,7 @@ fn run_screen_test( // The bound is derived: a panel promising a minute while the kernel // counts something else is the failure this line exists to catch. let armed = format!( - "panic: rebooting in {} s unless a key is pressed", + "panic: rebooting in {} s, timed by ", toyos_tco::PANIC_BOUND_MS / 1_000 ); for want in ["PANIC:", "test-late-panic: on-screen console check", &armed] { @@ -6094,191 +6106,6 @@ fn run_screen_test( } Ok(()) } - "screen_pager_keys" => { - // The halted pager takes PageDown off the i8042 with every - // CPU stopped, and this is the only place that claim can be made: - // the decode is `toyos-ps2`'s and host-tested, but that a keystroke - // reaches a machine which has stopped scheduling is a fact about - // the controller and the poll, not about the table. - // - // `Profile::Metal` because QEMU routes injected keys to one handler - // per device class: every profile with a `usb-kbd` sends them there - // instead, and this is the only GOP machine without one. - let mut qemu = QemuInstance::boot_with_options( - test_config, - c_bins, - rust_bins, - BootOptions { - profile: qemu::Profile::Metal, - qmp: true, - kernel_params: &["test-late-panic"], - ready_marker: "PANIC:", - ..Default::default() - }, - ); - let socket = qemu.qmp_socket().to_path_buf(); - - // The footer only exists once the report overflows the screen, so - // waiting for one is waiting for the pager to be the thing on - // screen. `page_forever` returns without looping below two pages. - // Retried, because a dump taken while the pager is repainting - // catches a half-written bottom row and no footer at all. - let footer = |q: &mut QemuInstance| { - for _ in 0..4 { - let text = q.screendump().text(); - if let Some(f) = text.lines().rev().find(|l| l.starts_with("[page ")) { - return Some(f.to_string()); - } - thread::sleep(Duration::from_millis(50)); - } - None - }; - let deadline = Instant::now() + qemu.budget(Duration::from_secs(30)); - let mut last = loop { - if let Some(f) = footer(&mut qemu) { - break f; - } - if Instant::now() >= deadline { - return Err(format!( - "{STALLED} no `[page n/m]` footer ever appeared; nothing was paging" - )); - } - }; - - // How long the unattended deadline actually takes to move the page, - // measured before a key is pressed because the first key retires it - // for good. This is what stops the last phase passing vacuously: a - // guest too slow to have paged in its window would prove nothing by - // not paging, and this is the window measured on *this* guest. - let timing_from = Instant::now(); - let unattended_move = loop { - let Some(now) = footer(&mut qemu) else { - return Err(format!( - "{STALLED} the footer vanished while timing the unattended deadline" - )); - }; - if now != last { - last = now; - break timing_from.elapsed(); - } - if Instant::now() >= deadline { - return Err(format!( - "{STALLED} the pager did not advance on its own in {:.1}s against a 3s \ - deadline — nothing here can say whether a keystroke stops it", - timing_from.elapsed().as_secs_f64() - )); - } - }; - - // **One keystroke, then its page, then the next keystroke.** The - // verdict is that every one of them moved the page, and there is no - // clock of the host's in it: a guest that is slow costs this run - // wall clock and never a move. - // - // It used to inject all thirty at the host's own speed and compare - // the moves it saw against what a 3 s deadline could have produced - // in the elapsed time — `moved >= elapsed/3 + 1` times three. That - // arithmetic asks a guest which has not been given time to repaint - // once for three moves, so on a host that got through the thirty in - // 0.3 s it demanded 3.3 of them and reported `0 page moves over 30 - // keystrokes in 0.3s`: the symptom, where the fact was that nothing - // had run. Two agents bisected that as a kernel regression on one - // day. Unpaced it was wrong about the wire as well — thirty - // press/release pairs is sixty scancodes into QEMU's 16-byte - // `PS2_QUEUE_SIZE` (`hw/input/ps2.c`), so the keys a full-panel - // repaint had no room for were never delivered at all. - // - // What makes "every key moved it" the whole claim, with no rate - // beside it, is the phase below: after the first keystroke the - // deadline is retired for good, so it contributes no move to this - // loop, and if it were still running the steered page would not hold. - const SAMPLES: usize = 30; - let started = Instant::now(); - for key in 1..=SAMPLES { - qemu::qmp_send_keys(&socket, &[("pgdn", true), ("pgdn", false)]); - let by = Instant::now() + qemu.budget(Duration::from_secs(20)); - loop { - let Some(now) = footer(&mut qemu) else { - return Err(format!( - "{STALLED} the footer vanished after {} of {SAMPLES} keystrokes", - key - 1 - )); - }; - if now != last { - last = now; - break; - } - if Instant::now() >= by { - return Err(format!( - "keystroke {key} of {SAMPLES} left the pager on {last:?}: a PageDown \ - reached a halted machine and no page came of it" - )); - } - } - } - let elapsed = started.elapsed(); - - // Nothing is in flight — the loop above did not send a key until the - // page the one before it moved was on the screen — so this asks only - // that the panel is not mid-repaint before the watch starts. - const SETTLED: Duration = Duration::from_secs(1); - let settle_by = Instant::now() + qemu.budget(Duration::from_secs(20)); - let mut held = last; - let mut stable_since = Instant::now(); - loop { - let Some(now) = footer(&mut qemu) else { - return Err(format!( - "{STALLED} the footer vanished while the last page settled" - )); - }; - if now != held { - held = now; - stable_since = Instant::now(); - } else if stable_since.elapsed() >= SETTLED { - break; - } - if Instant::now() >= settle_by { - return Err(format!( - "the pager never held one page for {}s after the last keystroke, so \ - something is still moving it", - SETTLED.as_secs() - )); - } - } - - // And now the owner's complaint, which is the other half: a page he - // steered to must stay up. The window is twice what the unattended - // deadline was measured to need above, so a pager still running it - // moves at least twice inside this and a slow guest cannot pass by - // being slow. - let quiet = unattended_move * 2 + Duration::from_secs(1); - let watching_from = Instant::now(); - while watching_from.elapsed() < quiet { - let Some(now) = footer(&mut qemu) else { - return Err("the footer vanished while watching a steered page".into()); - }; - if now != held { - return Err(format!( - "the page moved from {held:?} to {now:?} on its own {:.1}s into a {:.1}s \ - watch after the last keystroke — the deadline is still running under a \ - reader who has taken the wheel, which is what it must not do", - watching_from.elapsed().as_secs_f64(), - quiet.as_secs_f64() - )); - } - } - print_screen( - name, - &format!( - "every one of {SAMPLES} keystrokes moved the page, in {:.1}s; unattended it \ - moved once in {:.1}s, and after a keystroke it held {held} for {:.1}s", - elapsed.as_secs_f64(), - unattended_move.as_secs_f64(), - quiet.as_secs_f64(), - ), - ); - Ok(()) - } "screen_fatal_halt" => { // The steady-state fatal path: userland is up, the display is // idle, and SYS_DEBUG action 3 runs halt_all_cpus for real. @@ -11045,6 +10872,21 @@ fn metal_sim_client_death(boot: &mut Boot) -> Result<(), String> { Ok(()) } +/// A `test_rs_isa_grant` role that ran to its end: no ceiling, exit 0, and the +/// line it prints last. +fn isa_verdict(result: &TestResult, last: &str) -> Result<(), String> { + if let Some(err) = &result.error { + return Err(format!("{err}\n{}", result.stdout)); + } + if result.exit_code != Some(0) || !result.stdout.contains(last) { + return Err(format!( + "isa_grant exited {:?} without {last:?}:\n{}\n{}", + result.exit_code, result.stdout, result.serial + )); + } + Ok(()) +} + /// Run one machine-shape test. Like `run_screen_test`, each of these owns its /// QEMU — the machine shape *is* the test — except for the runs of adjacent /// names that share one through `held` (see [`group_boot`]). @@ -11114,7 +10956,7 @@ fn run_machine_test( "panic_before_peripherals_reboots" => { power::panic_before_peripherals_reboots(test_config, c_bins, rust_bins) } - "panic_key_holds" => power::panic_key_holds(test_config, c_bins, rust_bins), + "panic_ignores_keys" => power::panic_ignores_keys(test_config, c_bins, rust_bins), "blackbox_panic_chain" => power::blackbox_panic_chain(test_config, c_bins, rust_bins), "panic_outlives_the_deadline" => { power::panic_outlives_the_deadline(test_config, c_bins, rust_bins) @@ -15736,6 +15578,96 @@ fn run_machine_test( eprintln!(" [input] no input source exists and both claims refused NotFound"); Ok(()) } + "isa_claim_refused_where_the_kernel_drives" => { + let mut qemu = QemuInstance::boot_with_options( + test_config, + c_bins, + rust_bins, + BootOptions { profile: qemu::Profile::Metal, ..Default::default() }, + ); + // The premise: without it the refusal below could be anything's. + let Some(armed) = qemu.boot_log().lines().find(|l| l.contains("scanning on")) else { + return Err(format!("the kernel never drove the i8042:\n{}", qemu.boot_log())); + }; + let armed = armed.to_string(); + let result = qemu.run_test("test_rs_isa_grant driven", Duration::from_secs(30)); + isa_verdict(&result, "refused PermissionDenied")?; + eprintln!(" [isa] {}", armed.trim()); + eprintln!(" [isa] the claim on the controller the kernel drives was refused"); + Ok(()) + } + "isa_ports_are_the_binders_alone" => { + // No i8042 at all, so the kernel drives nothing the claim names and + // the ports float: the bitmap is the only thing between a process + // and them. + let options = BootOptions { + profile: qemu::Profile::MetalNoUsb, + i8042: false, + ..Default::default() + }; + let mut qemu = + QemuInstance::boot_with_options(test_config, c_bins, rust_bins, options); + if !qemu.boot_log().contains("i8042: absent") { + return Err(format!("the i8042 is not absent:\n{}", qemu.boot_log())); + } + let result = qemu.run_test("test_rs_isa_grant grant", Duration::from_secs(60)); + isa_verdict(&result, "===ISA_GRANT_OK===")?; + // Each refused access is the kernel's to name: the port, and whose + // grant it is not. The unbound holder and the moved claim both die + // at 0x64, the bound child one port past what it holds, and the + // process holding nothing at the data port. + for (port, times) in [("0x0064", 2), ("0x0061", 1), ("0x0060", 1)] { + let named = format!( + "in of 1 byte(s) from port {port}, which this process holds no grant for" + ); + let seen = result.serial.matches(named.as_str()).count(); + if seen != times { + return Err(format!( + "the kernel named {named:?} {seen} time(s), want {times}:\n{}", + result.serial + )); + } + } + for want in + ["isa: the i8042's ports are pid", "isa: the i8042's ports went back with pid"] + { + if !result.serial.contains(want) { + return Err(format!("the kernel never said {want:?}:\n{}", result.serial)); + } + } + eprintln!(" [isa] {}", result.stdout.trim().replace('\n', "\n [isa] ")); + Ok(()) + } + "isa_lines_reach_their_holder" => { + // `i8042-budget-expired` has the kernel give the controller up + // before it arms a line, so a real i8042 is here for a claim. + let mut qemu = QemuInstance::boot_with_options( + test_config, + c_bins, + rust_bins, + BootOptions { + profile: qemu::Profile::Metal, + qmp: true, + kernel_params: &["i8042-budget-expired"], + ..Default::default() + }, + ); + if !qemu.boot_log().contains("init budget spent before the self-test stage") { + return Err(format!("the kernel did not give the i8042 up:\n{}", qemu.boot_log())); + } + let result = qemu.run_test_hooked( + "test_rs_isa_grant device", + Duration::from_secs(60), + "===ISA_DEVICE_READY===", + |socket| qemu::qmp_send_keys(socket, &[("a", true), ("a", false)]), + ); + isa_verdict(&result, "===ISA_DEVICE_OK===")?; + if !result.serial.contains("isa: the i8042 took its first interrupt") { + return Err(format!("the line's first interrupt was never said:\n{}", result.serial)); + } + eprintln!(" [isa] {}", result.stdout.trim().replace('\n', "\n [isa] ")); + Ok(()) + } "gpu_set_resolution" => { /// Mirrored in `tests/toyos-rust-tests/src/bin/gpu_set_resolution.rs`. const WANT: (usize, usize) = (800, 600); diff --git a/toyos-abi/src/syscall.rs b/toyos-abi/src/syscall.rs index f471ce741fe..465065d0d9e 100644 --- a/toyos-abi/src/syscall.rs +++ b/toyos-abi/src/syscall.rs @@ -1264,6 +1264,14 @@ device_classes! { /// Like `pci`, a class whose name is not the whole of the entry — /// `part:` — and [`DeviceRequest`] is the one parser. Partition = 8 => "part", + /// One legacy ISA function, driven by whoever holds the claim: the process + /// that first reads the claim may `in` and `out` the function's ports for + /// the rest of its life and no other process may, and the function's lines + /// are answered as interrupt records on the claim ([`IsaId`]). + /// + /// Like `pci`, a class whose name is not the whole of the entry — + /// `isa:,…:,…` — and [`DeviceRequest`] is the one parser. + Isa = 9 => "isa", } /// A PCI function named by what identifies the *card*, not the slot firmware @@ -1320,9 +1328,134 @@ fn hex16(text: &str) -> Option { Some(value) } +/// A legacy ISA function named by everything it decodes and raises: +/// `isa:0060,0064:1,12` is the i8042's two ports and its keyboard and aux +/// lines. +/// +/// **The name is the grant**, so it is exact: the kernel opens these ports and +/// routes these lines, and a set that is not one function it can hand out +/// whole is refused rather than trimmed to one it can. One spelling per set — +/// ports as four lowercase hex digits, lines in decimal with no leading zero, +/// each list strictly ascending — because every reader compares the string. +#[derive(Clone, Copy, PartialEq, Eq, Debug)] +pub struct IsaId { + /// Ascending and nonzero, then zeros: port 0 is the DMA controller's and + /// never a function a process drives, which is what lets 0 mean no port. + ports: [u16; IsaId::MAX_PORTS], + /// One bit per ISA line. + irqs: u16, +} + +impl IsaId { + pub const MAX_PORTS: usize = 4; + /// Bounded so the longest name fits [`DeviceRequest::MAX_NAME`]. + pub const MAX_IRQS: usize = 4; + /// The ISA bus has sixteen lines. + const LINES: u8 = 16; + + /// `None` for a set with no port or no line, a port 0, a line past 15, a + /// list longer than its bound, or a list not strictly ascending. + pub fn new(ports: &[u16], irqs: &[u8]) -> Option { + if ports.is_empty() || ports.len() > Self::MAX_PORTS || ports[0] == 0 || !ascending(ports) { + return None; + } + if irqs.is_empty() || irqs.len() > Self::MAX_IRQS || !ascending(irqs) { + return None; + } + let mut set = Self { ports: [0; Self::MAX_PORTS], irqs: 0 }; + set.ports[..ports.len()].copy_from_slice(ports); + for &irq in irqs { + if irq >= Self::LINES { + return None; + } + set.irqs |= 1 << irq; + } + Some(set) + } + + pub fn ports(&self) -> impl Iterator + '_ { + self.ports.iter().copied().take_while(|&port| port != 0) + } + + pub fn irqs(&self) -> impl Iterator + '_ { + (0..Self::LINES).filter(|line| self.irqs & (1 << line) != 0) + } + + /// `",…:,…"`, the tail of an `isa:` entry. + pub fn parse(text: &str) -> Option { + let (ports, irqs) = text.split_once(':')?; + let mut port_list = [0u16; Self::MAX_PORTS]; + let mut n_ports = 0; + for port in ports.split(',') { + *port_list.get_mut(n_ports)? = hex16(port)?; + n_ports += 1; + } + let mut irq_list = [0u8; Self::MAX_IRQS]; + let mut n_irqs = 0; + for irq in irqs.split(',') { + *irq_list.get_mut(n_irqs)? = decimal_line(irq)?; + n_irqs += 1; + } + Self::new(&port_list[..n_ports], &irq_list[..n_irqs]) + } + + /// The two selector words [`device_claim`] carries: the ports in 16-bit + /// lanes from the low end, and the lines as a mask. + pub fn wire(self) -> [u64; 2] { + let ports = self + .ports + .iter() + .enumerate() + .fold(0u64, |word, (lane, &port)| word | (port as u64) << (16 * lane)); + [ports, self.irqs as u64] + } + + /// The selector words decoded, refusing whatever [`Self::new`] refuses. + pub fn from_wire([ports, irqs]: [u64; 2]) -> Option { + let lanes: [u16; Self::MAX_PORTS] = + core::array::from_fn(|lane| (ports >> (16 * lane)) as u16); + let named = lanes.iter().take_while(|&&port| port != 0).count(); + if lanes[named..].iter().any(|&port| port != 0) || irqs > u16::MAX as u64 { + return None; + } + let mut lines = [0u8; Self::LINES as usize]; + let mut n = 0; + for line in 0..Self::LINES { + if irqs & (1 << line) != 0 { + lines[n] = line; + n += 1; + } + } + if n > Self::MAX_IRQS { + return None; + } + Self::new(&lanes[..named], &lines[..n]) + } +} + +fn ascending(list: &[T]) -> bool { + list.windows(2).all(|pair| pair[0] < pair[1]) +} + +/// One ISA line in decimal, `0` to `15`, with no sign and no leading zero. +fn decimal_line(text: &str) -> Option { + let bytes = text.as_bytes(); + if bytes.is_empty() || bytes.len() > 2 || (bytes.len() == 2 && bytes[0] == b'0') { + return None; + } + let mut value = 0u8; + for &byte in bytes { + if !byte.is_ascii_digit() { + return None; + } + value = value * 10 + (byte - b'0'); + } + (value < IsaId::LINES).then_some(value) +} + /// What one `devices` entry asks for: a class, and for -/// [`DeviceType::PciFunction`] which function, and for a partition which -/// partition. +/// [`DeviceType::PciFunction`] which function, for a partition which +/// partition, and for an ISA function which ports and lines. /// /// One parser, because four places read the same spelling — the build system's /// gate, `/system/bin/init`'s mint, the kernel's claim and the claimant's own @@ -1333,6 +1466,7 @@ pub enum DeviceRequest { Class(DeviceType), Pci(PciId), Partition(crate::part::PartGuid), + Isa(IsaId), } impl DeviceRequest { @@ -1342,6 +1476,9 @@ impl DeviceRequest { /// The spelling that carries a unique partition GUID. pub const PART_PREFIX: &'static str = "part:"; + /// The spelling that carries an ISA function's ports and lines. + pub const ISA_PREFIX: &'static str = "isa:"; + /// The longest a `devices` entry, and so a `dev:` label's tail, can be: /// `part:` and a GUID, rounded up to a multiple of eight. pub const MAX_NAME: usize = 48; @@ -1353,11 +1490,14 @@ impl DeviceRequest { if let Some(guid) = name.strip_prefix(Self::PART_PREFIX) { return crate::part::PartGuid::parse(guid).map(Self::Partition); } + if let Some(set) = name.strip_prefix(Self::ISA_PREFIX) { + return IsaId::parse(set).map(Self::Isa); + } let class = DeviceType::from_class_name(name)?; match class { - // A bare `pci` or `part` names no device, and would otherwise - // leave the selector at zero. - DeviceType::PciFunction | DeviceType::Partition => None, + // A bare `pci`, `part` or `isa` names no device, and would + // otherwise leave the selector at zero. + DeviceType::PciFunction | DeviceType::Partition | DeviceType::Isa => None, _ => Some(Self::Class(class)), } } @@ -1367,6 +1507,7 @@ impl DeviceRequest { Self::Class(class) => class, Self::Pci(_) => DeviceType::PciFunction, Self::Partition(_) => DeviceType::Partition, + Self::Isa(_) => DeviceType::Isa, } } @@ -1377,6 +1518,7 @@ impl DeviceRequest { Self::Class(_) => [0, 0], Self::Pci(id) => [id.wire(), 0], Self::Partition(guid) => guid.wire(), + Self::Isa(set) => set.wire(), } } @@ -1400,6 +1542,35 @@ impl DeviceRequest { return core::str::from_utf8(&buf[..at + GUID_TEXT_LEN]) .expect("a prefix and a GUID's text are ASCII"); } + Self::Isa(set) => { + let mut at = Self::ISA_PREFIX.len(); + buf[..at].copy_from_slice(Self::ISA_PREFIX.as_bytes()); + for (i, port) in set.ports().enumerate() { + if i > 0 { + buf[at] = b','; + at += 1; + } + for shift in [12, 8, 4, 0] { + buf[at] = HEX[((port >> shift) & 0xF) as usize]; + at += 1; + } + } + buf[at] = b':'; + at += 1; + for (i, line) in set.irqs().enumerate() { + if i > 0 { + buf[at] = b','; + at += 1; + } + if line >= 10 { + buf[at] = b'1'; + at += 1; + } + buf[at] = b'0' + line % 10; + at += 1; + } + return core::str::from_utf8(&buf[..at]).expect("hex, digits and separators are ASCII"); + } Self::Pci(id) => id, }; let prefix = Self::PCI_PREFIX.as_bytes(); @@ -2370,6 +2541,21 @@ mod tests { "part:c12a7328-f81f-11d2-ba4b-00a0c93ec93b", // one spelling: uppercase // A partition is named by its unique GUID and nothing else. "part-type:C12A7328-F81F-11D2-BA4B-00A0C93EC93B", + "isa", // a bare ISA class names no ports + "isa:", + "isa:0060,0064", // a function with no line + "isa:0060,0064:", + "isa::1", + "isa:60,64:1,12", // one spelling: four hex digits + "isa:0064,0060:1,12", // and ascending + "isa:0060,0060:1", + "isa:0060:12,1", + "isa:0060:01", + "isa:0060:16", // the bus has sixteen lines + "isa:0000:1", // port 0 means no port + "isa:0060,0061,0062,0063,0064:1", // more ports than a selector word holds + "isa:0060:1,2,3,4,5", + "isa:0060:+1", "", ] { assert_eq!(DeviceRequest::parse(bad), None, "{bad:?} parsed"); @@ -2389,6 +2575,10 @@ mod tests { "mouse", // The longest entry there is, which is what `MAX_NAME` is for. "part:0FC63DAF-8483-4772-8E79-3D69D8477DE4", + "isa:0060,0064:1,12", + "isa:0001:0", + // The longest an ISA entry can be. + "isa:fff0,fff1,fff2,fffe:12,13,14,15", ] { let request = DeviceRequest::parse(name).expect("a name this table has"); let mut buf = [0u8; DeviceRequest::MAX_NAME]; @@ -2408,6 +2598,31 @@ mod tests { assert_eq!(request.map(DeviceRequest::selector), Some(guid.wire())); } + /// An `isa:` entry is its ports in the first selector word and its lines in + /// the second, and the kernel decodes nothing a config could not have + /// written: a port lane after a gap, or a line past the bus, is no set. + #[test] + fn an_isa_entry_survives_the_wire_and_nothing_else_decodes() { + let request = DeviceRequest::parse("isa:0060,0064:1,12").expect("the i8042's set"); + let DeviceRequest::Isa(set) = request else { panic!("{request:?} is not an ISA set") }; + assert_eq!(request.class(), DeviceType::Isa); + assert!(set.ports().eq([0x60, 0x64])); + assert!(set.irqs().eq([1, 12])); + assert_eq!(request.selector(), [0x0064_0060, (1 << 1) | (1 << 12)]); + assert_eq!(IsaId::from_wire(set.wire()), Some(set)); + for bad in [ + [0, 1 << 1], // no port + [0x0060, 0], // no line + [0x0060_0000, 1 << 1], // a port after a gap + [0x0060_0064, 1 << 1], // not ascending + [0x0060, 1 << 16], // past the bus + [0x0060, 0b1_1111], // more lines than a name holds + [0x0060_0060, 1 << 1], // one port twice + ] { + assert_eq!(IsaId::from_wire(bad), None, "{bad:x?} decoded"); + } + } + /// The selector is one word on the wire and the kernel decodes it back; /// a vendor lost to a shift would claim a different card. #[test] diff --git a/toyos-ps2/tests/decode.rs b/toyos-ps2/tests/decode.rs index 979750c76ab..aef9b850401 100644 --- a/toyos-ps2/tests/decode.rs +++ b/toyos-ps2/tests/decode.rs @@ -87,16 +87,14 @@ fn make_and_break_and_the_e0_prefix() { assert_eq!(press(&mut d, &[0x4B]), [(0x5C, true)]); } -/// The panic console's pager reads these two straight off a halted controller -/// with its own decoder, and its whole input vocabulary is what this asserts. #[test] -fn the_pager_keys_decode_to_their_hid_usages() { +fn page_up_and_down_decode_to_their_hid_usages() { let mut d = KeyDecoder::new(); assert_eq!(press(&mut d, &[0xE0, 0x49]), [(0x4B, true)], "PageUp make"); assert_eq!(press(&mut d, &[0xE0, 0xC9]), [(0x4B, false)], "PageUp break"); assert_eq!(press(&mut d, &[0xE0, 0x51]), [(0x4E, true)], "PageDown make"); assert_eq!(press(&mut d, &[0xE0, 0xD1]), [(0x4E, false)], "PageDown break"); - // Unprefixed they are the keypad's 9 and 3, which a pager must not answer. + // Unprefixed they are the keypad's 9 and 3. assert_eq!(press(&mut d, &[0x49]), [(0x61, true)]); assert_eq!(press(&mut d, &[0x51]), [(0x5B, true)]); } diff --git a/toyos/src/syscap.rs b/toyos/src/syscap.rs index 2bc9aec4b45..954c8f232ca 100644 --- a/toyos/src/syscap.rs +++ b/toyos/src/syscap.rs @@ -53,6 +53,18 @@ impl SysCap { self.mint(DeviceRequest::Partition(guid)) } + /// Mint the claim for one legacy ISA function, by exactly its ports and + /// lines. Apart from [`Self::claim`] for the reason [`Self::claim_pci`] is. + /// + /// `NotFound` is a set that is not one function this machine can hand out + /// whole, and `PermissionDenied` one the kernel drives itself. + pub fn claim_isa( + &self, + set: toyos_abi::syscall::IsaId, + ) -> Result { + self.mint(DeviceRequest::Isa(set)) + } + fn mint(&self, request: DeviceRequest) -> Result { let raw = syscall::device_claim(self.0.raw(), request)?; // SAFETY: the kernel installed this handle in this process's table for diff --git a/userland/init/src/main.rs b/userland/init/src/main.rs index f8e66db7df3..da9b99f5d18 100644 --- a/userland/init/src/main.rs +++ b/userland/init/src/main.rs @@ -1479,6 +1479,7 @@ fn start<'a>( DeviceRequest::Class(class) => syscap.claim::(class), DeviceRequest::Pci(id) => syscap.claim_pci::(id), DeviceRequest::Partition(name) => syscap.claim_partition::(name), + DeviceRequest::Isa(set) => syscap.claim_isa::(set), }; let asked = Instant::now(); let mut held_still = 0u32; From e72cf7f4cf4b406f5c11b14b5655b1ef5f43e38d Mon Sep 17 00:00:00 2001 From: japabu Date: Tue, 29 Sep 2026 00:31:03 +0200 Subject: [PATCH 02/10] The track names #592 and its line count; the refusal boot runs nightly The kernel-drives refusal is one boot for one syscall word, so it runs with the nightly tier; the binding test, the security boundary itself, stays in the fast one. Co-Authored-By: Claude Opus 5.5 --- .../the-kernel-is-small-interrupts-post-and-threads-wait.md | 4 +++- tests/toyos-rust-tests/src/bin/isa_grant.rs | 5 ++++- tests/toyos.rs | 2 +- 3 files changed, 8 insertions(+), 3 deletions(-) diff --git a/issues/kernel/the-kernel-is-small-interrupts-post-and-threads-wait.md b/issues/kernel/the-kernel-is-small-interrupts-post-and-threads-wait.md index 331e3e49cd6..3a71f7822f0 100644 --- a/issues/kernel/the-kernel-is-small-interrupts-post-and-threads-wait.md +++ b/issues/kernel/the-kernel-is-small-interrupts-post-and-threads-wait.md @@ -149,7 +149,9 @@ times: userland, and a dead kernel takes no input, with no emergency way. 1. **The panic console takes no input, and an `isa` claim grants a process exact ports through the TSS I/O permission bitmap and its ISA lines as - records** (`kernel/src/isa.rs`). **Done** (PR_NUMBER). + records** (`kernel/src/isa.rs`). **Done** (#592): `kernel/src` grew from + 64984 to 65370 lines, the mechanism landing before the driver it + replaces is deleted. 2. **ps2d**, the server over that claim, feeding the kernel's keyboard and mouse streams so Ctrl+Alt+D and the merge with USB HID stay where they are; the kernel's driver, its vector, its actuators and the diff --git a/tests/toyos-rust-tests/src/bin/isa_grant.rs b/tests/toyos-rust-tests/src/bin/isa_grant.rs index 4a57679b29c..60299c5b1e6 100644 --- a/tests/toyos-rust-tests/src/bin/isa_grant.rs +++ b/tests/toyos-rust-tests/src/bin/isa_grant.rs @@ -97,7 +97,10 @@ fn outb(port: u16, value: u8) { fn driven(cap: &SysCap) { match claim(cap, I8042) { Err(SyscallError::PermissionDenied) => {} - other => panic!("isa: the kernel drives the i8042 and a claim on it answered {:?}", other.map(|_| ())), + other => panic!( + "isa: the kernel drives the i8042 and a claim on it answered {:?}", + other.map(|_| ()) + ), } println!("isa: the claim on a controller the kernel drives was refused PermissionDenied"); } diff --git a/tests/toyos.rs b/tests/toyos.rs index 9baf96bd040..97ba8cbfc39 100644 --- a/tests/toyos.rs +++ b/tests/toyos.rs @@ -658,7 +658,7 @@ const MACHINE_TESTS: &[(&str, Sched, Tier)] = &[ // none, and a real controller the kernel gave up on driven to a keystroke. // Every verdict is a guest's line or the kernel's record of a kill; the one // wait is on the guest's own ready line. - ("isa_claim_refused_where_the_kernel_drives", Sched::Parallel, Tier::Fast), + ("isa_claim_refused_where_the_kernel_drives", Sched::Parallel, Tier::Nightly), ("isa_ports_are_the_binders_alone", Sched::Parallel, Tier::Fast), ("isa_lines_reach_their_holder", Sched::Parallel, Tier::Nightly), // One boot; every verdict is a PPM header field or a console line, and no From 7c13193f1902c3b16ff5f26424ae0af1ab886747 Mon Sep 17 00:00:00 2001 From: japabu Date: Tue, 29 Sep 2026 03:58:43 +0200 Subject: [PATCH 03/10] The crash report reads user memory through a split window's 4 KiB leaf isa_ports_are_the_binders_alone was red: every refused `in` was killed as a bare #GP and none named its port. The decode was reached and its bytes were wrong. `safe_read_u64` walked a user address to the PDE and read it as a 2 MiB leaf always, but an image whose first 2 MiB holds text beside rodata and data (`isa_grant`: r--, r-x and rw- LOADs under 0x9f000) is mapped by `map_window` as a page table of 4 KiB leaves. The PDE then names that table, and the report read the table's 2 MiB-aligned neighbourhood instead of the `ec` at the faulting rip. The stack, one uniform window, read correctly, which is why the backtrace never showed it. The walk now goes one level down when the PDE's PS bit is clear, as `paging::debug_page_walk` does. Co-Authored-By: Claude Opus 5.5 --- kernel/src/arch/x86_64/idt/exceptions.rs | 19 ++++++++++++++----- 1 file changed, 14 insertions(+), 5 deletions(-) diff --git a/kernel/src/arch/x86_64/idt/exceptions.rs b/kernel/src/arch/x86_64/idt/exceptions.rs index 2285b90be3a..10862789aab 100644 --- a/kernel/src/arch/x86_64/idt/exceptions.rs +++ b/kernel/src/arch/x86_64/idt/exceptions.rs @@ -76,11 +76,20 @@ fn safe_read_u64(addr: u64, user_pml4: *const u64) -> Option { // SAFETY: the walk's argument, one level down again. let pde = unsafe { *pd.add(pd_idx) }; if pde & 1 == 0 { return None; } - let page_phys = pde & 0x000F_FFFF_FFE0_0000; - let offset = addr & (mm::PAGE_2M - 1); - // SAFETY: the walk's argument — a direct-map read of a byte inside the - // present 2 MiB leaf the three entries above resolved. - Some(unsafe { *crate::DirectMap::from_phys(page_phys + offset).as_ptr::() }) + let leaf = if pde & (1 << 7) != 0 { + (pde & 0x000F_FFFF_FFE0_0000) + (addr & (mm::PAGE_2M - 1)) + } else { + // A window of mixed rights — an image's text beside its data — is + // split into 4 KiB leaves one level down. + let pt = crate::DirectMap::from_phys(pde & 0x000F_FFFF_FFFF_F000).as_ptr::(); + // SAFETY: the walk's argument, one level down again. + let pte = unsafe { *pt.add(((addr >> 12) & 0x1FF) as usize) }; + if pte & 1 == 0 { return None; } + (pte & 0x000F_FFFF_FFFF_F000) + (addr & 0xFFF) + }; + // SAFETY: the walk's argument — a direct-map read of 8 aligned bytes + // inside the present leaf the entries above resolved. + Some(unsafe { *crate::DirectMap::from_phys(leaf).as_ptr::() }) } else if mm::is_kernel_addr(addr) { // SAFETY: `addr` is 8-aligned (checked at the top) and a kernel address // (checked in this arm), so it is inside the direct map. From f9a2f27a3bbc4db748339e30bdeb183ef125ec54 Mon Sep 17 00:00:00 2001 From: japabu Date: Tue, 29 Sep 2026 08:35:15 +0200 Subject: [PATCH 04/10] PR #592 round 3: the crash report reads only user memory, the panel stops paging, and the isa claim's orderings are tested The review of 7c13193f sent the branch back; each finding in turn. The crash report's hand walk of the user page tables is gone. It copied AddressSpace::walk's split-window arm, had drifted once already, and lacked walk's user-address refusal, so a process that faulted with rsp or rbp at the direct map had 64 bytes of physical memory printed into its kill record and the log. paging::translate_in_current_tables is the one lock-free translation of the current CR3, every leaf size; present_in_current_tables is its is_some(). read_user_u64 refuses a non-user address before it translates, and the report names the refusal for rsp, for rbp, and for a user #PF's page walk at a kernel address, which read the kernel's tables. The guest test crash_report_reads_no_kernel_memory kills one child with rsp and rbp at the direct map and another reading there, and judges the console and /log's kernel records: every refusal named, no stack word and no walk. port_access_at reads the second word only when the instruction runs into it, and a multi-byte access that faults on a port past its grant names that port: isa_grant's new `wide` child does `in ax, dx` at 0x60 and dies at 0x61. The i8042's quarantine masks lines 1 and 12 before it stores ACTIVE false (Release, and drives() reads Acquire), so a claim that finds the driver gone finds the lines masked. The isa-claim-straddles-quarantine actuator holds the quarantine between its two steps, in two scheduler passes, until a claim begun after the first has been answered, and isa_claim_straddles_the_quarantine drives a flood into the quarantine with a claim loop running and then a keystroke through the claim. The panel no longer pages (owner ruling: no scrolling on a dead kernel, no emergency path). page_forever, PAGE_HOLD and Page::Nth are deleted, and with them the Page argument, footer_cells' page number and check_no_stale_cells; halt_all_cpus holds with hold_the_panel. screen_paged_scrollback goes with its row in tests/test-durations. screen_fatal_behind_a_painter proved its CPU watched the bound by a second page; it now proves it by the reset, under panic-reboot-fast. isa_lines_reach_their_holder presses a key after the device role's claim is gone, with the controller read empty first, and requires the line's first interrupt announced once: an unmasked line would announce it again on the record release() cleared. isa_ports_close_on_one_cpu runs the grant role on one CPU, so every child runs where the one before ran. switch_to skips a row whose first bit already says what the incoming process holds. panicked() says again why it is Profile::Metal. The reset-line issue is a defect owned by the power broker track, which cites it. The count of kernel/src lines is gone from the stage entry, and the issue clauses about the pager are deleted. Co-Authored-By: Claude Opus 5.5 --- ...tal-session-runs-a-pre-flash-gate-first.md | 4 +- ...2s-holder-holds-the-machines-reset-line.md | 13 +- ...oker-authority-with-a-human-in-the-loop.md | 3 + ...-small-interrupts-post-and-threads-wait.md | 7 +- .../no-console-between-boot-and-terminal.md | 3 +- kernel/src/actuator.rs | 5 + kernel/src/arch/x86_64/i8042/mod.rs | 34 +- kernel/src/arch/x86_64/idt/exceptions.rs | 146 ++++---- kernel/src/arch/x86_64/paging.rs | 22 +- kernel/src/arch/x86_64/percpu.rs | 10 + kernel/src/arch/x86_64/pio.rs | 14 + kernel/src/drivers/panic_console/mod.rs | 92 ++--- kernel/src/isa.rs | 61 +++ kernel/src/panic.rs | 4 +- tests/common/faults.rs | 69 ++++ tests/common/power.rs | 6 +- tests/test-durations | 1 - .../src/bin/fault_gate_child.rs | 30 ++ tests/toyos-rust-tests/src/bin/isa_grant.rs | 74 +++- tests/toyos.rs | 349 +++++++----------- 20 files changed, 555 insertions(+), 392 deletions(-) diff --git a/issues/hardware/a-metal-session-runs-a-pre-flash-gate-first.md b/issues/hardware/a-metal-session-runs-a-pre-flash-gate-first.md index 015c0407e01..4b51e666487 100644 --- a/issues/hardware/a-metal-session-runs-a-pre-flash-gate-first.md +++ b/issues/hardware/a-metal-session-runs-a-pre-flash-gate-first.md @@ -39,9 +39,7 @@ anything touching the boot path. these to be read-verified only: TCG reports every CPU feature present, so the missing-bit path cannot run. 4. **The on-screen console. If this fails, do not flash.** Every screen test. - Confirm the muted profile actually removes the UART, and that the paging test - is driven by a timer rather than a keypress — input may be dead on the - machine. *False pass:* the late-panic gate passes with the capture routine's + Confirm the muted profile actually removes the UART. *False pass:* the late-panic gate passes with the capture routine's body replaced by a bare return, so these cover rendering, not capture. 5. **Input, which is the milestone and which a verdict once omitted entirely.** The 2026-08-01 verdict recorded GO over six sections and a seventeen-row diff --git a/issues/isolation/the-i8042s-holder-holds-the-machines-reset-line.md b/issues/isolation/the-i8042s-holder-holds-the-machines-reset-line.md index 115b0efe97b..e5743697288 100644 --- a/issues/isolation/the-i8042s-holder-holds-the-machines-reset-line.md +++ b/issues/isolation/the-i8042s-holder-holds-the-machines-reset-line.md @@ -1,6 +1,6 @@ --- -status: owner -kind: question +status: open +kind: defect opened: 2026-09-28 --- @@ -21,5 +21,10 @@ the controller's protocol, a syscall or a trapped instruction per access in place of the bitmap; keeping 0x64 in the kernel means the keyboard driver is not wholly userland, which the owner ruled it must be. -**Exit**: the owner rules that the keyboard's holder may hold the reset line, -or names which of the two costs to pay. +A recorded weakness of the power broker's track +(`issues/isolation/the-power-broker-authority-with-a-human-in-the-loop.md`), +which owns it (owner ruling). + +**Exit**: resetting the machine is the power broker's decision alone: the +keyboard's holder reaches the controller's reset line only through the broker, +or not at all. diff --git a/issues/isolation/the-power-broker-authority-with-a-human-in-the-loop.md b/issues/isolation/the-power-broker-authority-with-a-human-in-the-loop.md index c8d0702143c..e7947487d0a 100644 --- a/issues/isolation/the-power-broker-authority-with-a-human-in-the-loop.md +++ b/issues/isolation/the-power-broker-authority-with-a-human-in-the-loop.md @@ -31,3 +31,6 @@ so the native shape is smaller: Unstaffed until the owner opens it; sequenced naturally with the userland/ product era. What must not happen meanwhile is the accident this track exists to prevent: `POWER` spreading to more manifest rows. + +Its recorded weakness: the i8042's holder resets the machine without it +(`issues/isolation/the-i8042s-holder-holds-the-machines-reset-line.md`). diff --git a/issues/kernel/the-kernel-is-small-interrupts-post-and-threads-wait.md b/issues/kernel/the-kernel-is-small-interrupts-post-and-threads-wait.md index 3a71f7822f0..5a95172ab78 100644 --- a/issues/kernel/the-kernel-is-small-interrupts-post-and-threads-wait.md +++ b/issues/kernel/the-kernel-is-small-interrupts-post-and-threads-wait.md @@ -127,8 +127,7 @@ times: 10. **usbd**, stage 5's second half: the whole xHCI moves, HID to the keyboard claim and mass storage over `toyos-blockring`, and the kernel USB bridge is deleted. **What must work with no userland stays off - USB**: the panic console pages its report by itself and needs no - keyboard; the kernel's one + USB**: the kernel's one hotkey, Ctrl+Alt+D (`kernel/src/keyboard.rs`, the blocked-task dump), is recognised on the i8042's transitions and no longer on a USB keyboard's, which from here reach the kernel only as usbd's keyboard @@ -149,9 +148,7 @@ times: userland, and a dead kernel takes no input, with no emergency way. 1. **The panic console takes no input, and an `isa` claim grants a process exact ports through the TSS I/O permission bitmap and its ISA lines as - records** (`kernel/src/isa.rs`). **Done** (#592): `kernel/src` grew from - 64984 to 65370 lines, the mechanism landing before the driver it - replaces is deleted. + records** (`kernel/src/isa.rs`). **Done** (#592). 2. **ps2d**, the server over that claim, feeding the kernel's keyboard and mouse streams so Ctrl+Alt+D and the merge with USB HID stay where they are; the kernel's driver, its vector, its actuators and the diff --git a/issues/panic-path/no-console-between-boot-and-terminal.md b/issues/panic-path/no-console-between-boot-and-terminal.md index 370a5b8da86..0f0bd96844e 100644 --- a/issues/panic-path/no-console-between-boot-and-terminal.md +++ b/issues/panic-path/no-console-between-boot-and-terminal.md @@ -36,8 +36,7 @@ it by saying where the log will be, and cannot put a line in it. (`kernel/src/drivers/panic_console/mod.rs:639`), and a compositor claiming the framebuffer sets it. So on `bootable.img` the last kernel screenful ever painted is the one at `Boot: complete`, the desktop overwrites it a few tens of -milliseconds later, and no key pauses it: `page_forever` is reached only from -`halt_all_cpus`, so a *successful* boot never pages. +milliseconds later, and no key pauses it. **The durable answer landed and is not this.** "A log sink that survives userland" is `/system/bin/logd`: the kernel keeps the record ring and the console and diff --git a/kernel/src/actuator.rs b/kernel/src/actuator.rs index cd2f2cd8bae..48bbf4863c3 100644 --- a/kernel/src/actuator.rs +++ b/kernel/src/actuator.rs @@ -89,6 +89,11 @@ actuators! { /// Cap the i8042 ISR at 4 bytes and answer empty until the mute verdict is out; `service` then polls the rest, so the verdict beats the sequence on every boot instead of on a loaded shard's luck. i8042_split_burst = "i8042-split-burst"; + /// Hold the i8042's quarantine between its two steps, in two scheduler + /// passes, until an `isa` claim begun after the first has been answered: + /// the claim lands between them on every boot. + isa_claim_straddles_quarantine = "isa-claim-straddles-quarantine"; + /// Script the input core directly at end of boot. test_input_merge = "test-input-merge"; diff --git a/kernel/src/arch/x86_64/i8042/mod.rs b/kernel/src/arch/x86_64/i8042/mod.rs index 1683d0c1f88..bbde0646b4e 100644 --- a/kernel/src/arch/x86_64/i8042/mod.rs +++ b/kernel/src/arch/x86_64/i8042/mod.rs @@ -105,7 +105,7 @@ static IRQ_CPU: AtomicU32 = AtomicU32::new(u32::MAX); /// Whether this driver holds the controller, which refuses an `isa` claim on it. pub fn drives() -> bool { - ACTIVE.load(Ordering::Relaxed) + ACTIVE.load(Ordering::Acquire) } fn is_irq_cpu() -> bool { @@ -595,6 +595,13 @@ pub fn service() { quarantine(); return; } + #[cfg(feature = "boot-actuators")] + if crate::actuator::isa_claim_straddles_quarantine() { + if let Some(masked) = crate::isa::straddle::resume() { + let_go(masked); + return; + } + } if !ACTIVE.load(Ordering::Relaxed) { return; } @@ -778,15 +785,14 @@ fn drain() -> Drained { /// A controller producing bytes faster than the ISR's bound can drain them. /// One masked line and a dead keyboard, never a spinning CPU. +/// +/// **Masked before the driver lets go**: `isa::claim` refuses only while +/// [`ACTIVE`] holds, so a claim that lands once it is clear routes and unmasks +/// lines nothing here touches again. fn quarantine() { QUARANTINE.store(false, Ordering::Relaxed); - ACTIVE.store(false, Ordering::Relaxed); // The pin is about to be masked, so no health verdict follows this line. HEALTH.store(HEALTH_DONE, Ordering::Relaxed); - // Force-released: nothing else can lift a held key or pointer button - // once the line is masked. - crate::keyboard::release_all(); - crate::mouse::release_buttons(crate::mouse::PointerSource::PS2); // The count, not the intent: the log line is only true if the mask took. let mut masked = 0; for line in [KEYBOARD_GSI.load(Ordering::Relaxed), AUX_GSI.load(Ordering::Relaxed)] { @@ -794,6 +800,22 @@ fn quarantine() { masked += 1; } } + #[cfg(feature = "boot-actuators")] + if crate::actuator::isa_claim_straddles_quarantine() { + crate::isa::straddle::hold(masked); + return; + } + let_go(masked); +} + +/// The quarantine's second step, once `masked` of the lines are down. +fn let_go(masked: u32) { + // `Release`: a claim that reads the driver gone finds the lines masked. + ACTIVE.store(false, Ordering::Release); + // Force-released: nothing else can lift a held key or pointer button + // once the line is masked. + crate::keyboard::release_all(); + crate::mouse::release_buttons(crate::mouse::PointerSource::PS2); log!( "i8042: quarantined — output buffer never emptied, masked={} (kbd={} aux={} lost={})", masked, diff --git a/kernel/src/arch/x86_64/idt/exceptions.rs b/kernel/src/arch/x86_64/idt/exceptions.rs index 10862789aab..a5f95088012 100644 --- a/kernel/src/arch/x86_64/idt/exceptions.rs +++ b/kernel/src/arch/x86_64/idt/exceptions.rs @@ -10,12 +10,19 @@ use super::{Vector, TrapFrame, PF_PRESENT, PF_WRITE, PF_INSTRUCTION_FETCH}; /// Walk RBP chain for user backtrace through page tables. Takes no pid: this /// always backtraces the process running on this CPU. -fn user_backtrace(start_rbp: u64, pml4: *const u64, max_frames: usize) { +fn user_backtrace(start_rbp: u64, max_frames: usize) { let mut rbp = start_rbp; for _ in 0..max_frames { - if rbp == 0 || !rbp.is_multiple_of(8) { break; } - let Some(saved_rbp) = safe_read_u64(rbp, pml4) else { break }; - let Some(return_addr) = safe_read_u64(rbp + 8, pml4) else { break }; + if rbp == 0 { break; } + let saved_rbp = match read_user_u64(rbp) { + Ok(word) => word, + Err(Unread::Refused) => { + log!(" rbp {:#x} refused: no user address", rbp); + break; + } + Err(Unread::Absent) => break, + }; + let Ok(return_addr) = read_user_u64(rbp + 8) else { break }; if return_addr == 0 { break; } process::resolve_user_symbol_return(return_addr).log_bare(return_addr); rbp = saved_rbp; @@ -46,67 +53,41 @@ fn safe_read_kernel(addr: u64) -> Option { Some(unsafe { core::ptr::read_volatile(addr as *const u64) }) } -/// Reads a u64; for a user address, walks page tables by hand to avoid -/// demand-paging faults inside an exception handler. -fn safe_read_u64(addr: u64, user_pml4: *const u64) -> Option { - if !addr.is_multiple_of(8) || addr == 0 { - return None; +/// Why the crash report read no word at an address. +#[derive(Clone, Copy)] +enum Unread { + /// Not a user address: a crash report is never how a process reads the kernel. + Refused, + /// Misaligned, or nothing mapped there. + Absent, +} + +/// A word of the faulting process's memory, read through the tables this CPU +/// runs under: the crash path may neither take the address space's lock nor +/// demand-page. +fn read_user_u64(addr: u64) -> Result { + if !toyos_userbound::is_user_addr(addr) { + return Err(Unread::Refused); } - if !user_pml4.is_null() { - let pml4_idx = ((addr >> 39) & 0x1FF) as usize; - let pdpt_idx = ((addr >> 30) & 0x1FF) as usize; - let pd_idx = ((addr >> 21) & 0x1FF) as usize; - // SAFETY: each read is guarded by the present bit of the entry before - // it; `user_pml4` is a direct-map pointer to the live PML4 from `CR3`; - // every index is masked to nine bits, so `add` stays inside the - // 512-entry table, and each next-level pointer and the final - // `page_phys + offset` stay inside the direct map by the same walk. - // - // Hand-rolled instead of `mm::paging`: a faulted CPU may not take the - // address space lock and may not demand-page. - // - // Not `read_volatile` either: see `kernel_backtrace`. - let pml4e = unsafe { *user_pml4.add(pml4_idx) }; - if pml4e & 1 == 0 { return None; } - let pdpt = crate::DirectMap::from_phys(pml4e & 0x000F_FFFF_FFFF_F000).as_ptr::(); - // SAFETY: the walk's argument, one level down. - let pdpte = unsafe { *pdpt.add(pdpt_idx) }; - if pdpte & 1 == 0 { return None; } - let pd = crate::DirectMap::from_phys(pdpte & 0x000F_FFFF_FFFF_F000).as_ptr::(); - // SAFETY: the walk's argument, one level down again. - let pde = unsafe { *pd.add(pd_idx) }; - if pde & 1 == 0 { return None; } - let leaf = if pde & (1 << 7) != 0 { - (pde & 0x000F_FFFF_FFE0_0000) + (addr & (mm::PAGE_2M - 1)) - } else { - // A window of mixed rights — an image's text beside its data — is - // split into 4 KiB leaves one level down. - let pt = crate::DirectMap::from_phys(pde & 0x000F_FFFF_FFFF_F000).as_ptr::(); - // SAFETY: the walk's argument, one level down again. - let pte = unsafe { *pt.add(((addr >> 12) & 0x1FF) as usize) }; - if pte & 1 == 0 { return None; } - (pte & 0x000F_FFFF_FFFF_F000) + (addr & 0xFFF) - }; - // SAFETY: the walk's argument — a direct-map read of 8 aligned bytes - // inside the present leaf the entries above resolved. - Some(unsafe { *crate::DirectMap::from_phys(leaf).as_ptr::() }) - } else if mm::is_kernel_addr(addr) { - // SAFETY: `addr` is 8-aligned (checked at the top) and a kernel address - // (checked in this arm), so it is inside the direct map. - Some(unsafe { *(addr as *const u64) }) - } else { - None + if !addr.is_multiple_of(8) { + return Err(Unread::Absent); } + let at = mm::paging::translate_in_current_tables(addr).ok_or(Unread::Absent)?; + // SAFETY: a direct-map address of 8 aligned bytes inside the present leaf + // the current tables resolved. Not `read_volatile`: see `kernel_backtrace`. + Ok(unsafe { *at.as_ptr::() }) } /// The `in` or `out` at `rip`, read through the page tables as the rest of the /// report reads user memory; `None` for any other instruction or an unreadable one. -fn port_access_at(rip: u64, rdx: u64, pml4: *const u64) -> Option { - let at = rip & !7; - let lo = safe_read_u64(at, pml4)?; - let hi = safe_read_u64(at + 8, pml4)?; +fn port_access_at(rip: u64, rdx: u64) -> Option { + let (at, shift) = (rip & !7, rip & 7); + let lo = read_user_u64(at).ok()?; + // The next word only where the four bytes run into it: an `in` that ends + // just before an unmapped page is still named. + let hi = if shift > 4 { read_user_u64(at + 8).ok()? } else { 0 }; // Shifted rather than indexed: nothing on this path may panic. - let code = ((u128::from(lo) | u128::from(hi) << 64) >> (8 * (rip & 7))) as u32; + let code = ((u128::from(lo) | u128::from(hi) << 64) >> (8 * shift)) as u32; super::super::pio::port_access(code.to_le_bytes(), rdx as u16) } @@ -177,7 +158,6 @@ fn crash_report_exception(ctx: &ExceptionContext) { let ring3 = ctx.ring().is_user(); let tid = percpu::current_tid().unwrap_or(crate::process::Tid(0)); let pid = percpu::current_pid(); - let pml4 = if ring3 { crate::DirectMap::from_phys(crate::mm::paging::Cr3::current().phys()).as_ptr::() } else { core::ptr::null() }; let (pf_action, pf_cause) = if ctx.vector() == Vector::PageFault { let action = if ctx.frame.error_code & PF_INSTRUCTION_FETCH != 0 { "execute" } @@ -203,10 +183,17 @@ fn crash_report_exception(ctx: &ExceptionContext) { log!("SIGBUS tid={}: {} (error_code={:#x})", tid, name, ctx.frame.error_code); // At CPL 3 an `in` or `out` faults only on a port the bitmap refuses. if let Some(access) = (ctx.vector() == Vector::GeneralProtection) - .then(|| port_access_at(ctx.frame.rip, ctx.frame.rdx, pml4)) + .then(|| port_access_at(ctx.frame.rip, ctx.frame.rdx)) .flatten() { - log!(" {access}, which this process holds no grant for"); + match super::super::pio::refused_port(access) { + port if port == access.port => { + log!(" {access}, which this process holds no grant for") + } + port => log!( + " {access}, reaching port {port:#06x}, which this process holds no grant for" + ), + } } } _ => log!("FATAL tid={}: {}", tid, name), @@ -230,7 +217,12 @@ fn crash_report_exception(ctx: &ExceptionContext) { } if ctx.vector() == Vector::PageFault { - crate::mm::paging::debug_page_walk(ctx.cr2); + // A user fault walks its own half: the kernel's tables are no process's to read. + if ring3 && !toyos_userbound::is_user_addr(ctx.cr2) { + log!(" Page walk for {:#x} refused: no user address", ctx.cr2); + } else { + crate::mm::paging::debug_page_walk(ctx.cr2); + } } log!(" Registers:"); @@ -259,7 +251,7 @@ fn crash_report_exception(ctx: &ExceptionContext) { log!(" Backtrace:"); if ring3 { if pid.is_some() { - user_backtrace(ctx.frame.rbp, pml4, 32); + user_backtrace(ctx.frame.rbp, 32); } } else { kernel_backtrace(ctx.frame.rbp, 32); @@ -273,17 +265,23 @@ fn crash_report_exception(ctx: &ExceptionContext) { percpu::syscall_num(), user_rip, percpu::user_rsp()); log!(" User backtrace:"); process::resolve_user_symbol(user_rip).log_bare(user_rip); - let pml4 = crate::DirectMap::from_phys(crate::mm::paging::Cr3::current().phys()).as_ptr::(); - user_backtrace(percpu::syscall_rbp(), pml4, 20); + user_backtrace(percpu::syscall_rbp(), 20); } } - if safe_read_u64(ctx.frame.rsp, pml4).is_some() { - log!(" Stack (from RSP):"); - for i in 0..8u64 { - let addr = ctx.frame.rsp + i * 8; - let Some(val) = safe_read_u64(addr, pml4) else { break }; - log!(" [{:#x}] = {:#018x}", addr, val); + // A user fault's stack is the process's own, and a stack pointer it aimed + // at the kernel reads nothing. + let read = |addr: u64| if ring3 { read_user_u64(addr) } else { safe_read_kernel(addr).ok_or(Unread::Absent) }; + match read(ctx.frame.rsp) { + Err(Unread::Refused) => log!(" Stack (from RSP): {:#x} refused: no user address", ctx.frame.rsp), + Err(Unread::Absent) => {} + Ok(_) => { + log!(" Stack (from RSP):"); + for i in 0..8u64 { + let addr = ctx.frame.rsp.wrapping_add(i * 8); + let Ok(val) = read(addr) else { break }; + log!(" [{:#x}] = {:#018x}", addr, val); + } } } @@ -339,8 +337,7 @@ fn crash_report_panic(info: &core::panic::PanicInfo, rbp: u64) { percpu::syscall_num(), user_rip, percpu::user_rsp()); log!(" User backtrace:"); process::resolve_user_symbol(user_rip).log_bare(user_rip); - let pml4 = crate::DirectMap::from_phys(crate::mm::paging::Cr3::current().phys()).as_ptr::(); - user_backtrace(percpu::syscall_rbp(), pml4, 20); + user_backtrace(percpu::syscall_rbp(), 20); } } } @@ -403,11 +400,10 @@ pub(super) fn double_fault_handler(frame: &TrapFrame) -> ! { log!(" User context (pid={:?} tid={:?}):", pid, tid); log!(" rip={:#018x} rsp={:#018x} rbp={:#018x}", maybe_rip, maybe_rsp, user_rbp); - let pml4 = crate::DirectMap::from_phys(crate::mm::paging::Cr3::current().phys()).as_ptr::(); log!(" User backtrace:"); if pid.is_some() { process::resolve_user_symbol(maybe_rip).log_bare(maybe_rip); - user_backtrace(user_rbp, pml4, 20); + user_backtrace(user_rbp, 20); } else { log!(" {:#x}", maybe_rip); } diff --git a/kernel/src/arch/x86_64/paging.rs b/kernel/src/arch/x86_64/paging.rs index 43991575b4a..bea6be7d5d8 100644 --- a/kernel/src/arch/x86_64/paging.rs +++ b/kernel/src/arch/x86_64/paging.rs @@ -993,23 +993,37 @@ fn has(entry: u64, flag: u64) -> u8 { /// space); lock-free and silent, for the panic path to prove a mapping /// before writing through it. pub fn present_in_current_tables(addr: u64) -> bool { + translate_in_current_tables(addr).is_some() +} + +/// Where `addr` lands in the tables this CPU runs under now, whatever the +/// leaf's size; lock-free, for a crash path that may take no lock. Rights are +/// not asked: a caller reading on a process's behalf refuses a non-user +/// address itself. +pub fn translate_in_current_tables(addr: u64) -> Option { // SAFETY: `Cr3::current().phys()` names the table this CPU runs under // right now, so it can't be freed meanwhile. No lock: each entry read is // one aligned `u64`, atomic at the hardware level, so no read is torn. let mut table = unsafe { PageTablePage::from_phys(Cr3::current().phys()) }; for level in 0..3 { - let entry = table[((addr >> (39 - level * 9)) & 0x1FF) as usize]; + let shift = 39 - level * 9; + let entry = table[((addr >> shift) & 0x1FF) as usize]; if entry & PAGE_PRESENT == 0 { - return false; + return None; } if level > 0 && entry & PAGE_SIZE_BIT != 0 { - return true; + let span = 1u64 << shift; + return Some(crate::mm::DirectMap::from_phys( + (entry & ADDR_MASK & !(span - 1)) | (addr & (span - 1)), + )); } // SAFETY: PRESENT just checked; same argument as above the loop — a // present entry under the current CR3 names a live table. table = unsafe { PageTablePage::from_phys(entry & ADDR_MASK) }; } - table[((addr >> 12) & 0x1FF) as usize] & PAGE_PRESENT != 0 + let pte = table[((addr >> 12) & 0x1FF) as usize]; + (pte & PAGE_PRESENT != 0) + .then(|| crate::mm::DirectMap::from_phys((pte & ADDR_MASK) | (addr & 0xFFF))) } /// Give every 2 MiB leaf covering `[phys, phys + size)` the write-combining diff --git a/kernel/src/arch/x86_64/percpu.rs b/kernel/src/arch/x86_64/percpu.rs index b4e653dac17..7a85c2aaa9b 100644 --- a/kernel/src/arch/x86_64/percpu.rs +++ b/kernel/src/arch/x86_64/percpu.rs @@ -693,6 +693,16 @@ pub fn set_port_open(port: u16, open: bool) { } } +/// Whether this CPU's bitmap opens `port` to Ring 3; every port past it is +/// refused by the TSS limit. Panic-free: the crash report asks it. +pub fn port_open(port: u16) -> bool { + let (byte, bit) = (port as usize / 8, 1u8 << (port % 8)); + let percpu = gs::read_u64::() as *const PerCpu; + // SAFETY: this CPU's own `PerCpu`, read from `gs:[0]`; `get` keeps the read + // inside the array, and a `u8` has no alignment a packed struct could break. + unsafe { (*percpu).tss.io_bitmap.get(byte).is_some_and(|&b| b & bit == 0) } +} + /// The two words [`set_kernel_stack`] writes: `kernel_rsp` (syscall entry) and `tss.rsp0` (Ring 3 interrupt entry); read only by an instrument. /// # Safety: must be called from the CPU whose GS base points to the relevant PerCpu. #[cfg(feature = "stack-witness")] diff --git a/kernel/src/arch/x86_64/pio.rs b/kernel/src/arch/x86_64/pio.rs index ccb21c86561..e555b4f0334 100644 --- a/kernel/src/arch/x86_64/pio.rs +++ b/kernel/src/arch/x86_64/pio.rs @@ -69,12 +69,26 @@ pub fn set_masked(line: Line, masked: bool) { pub fn switch_to(pid: Option) { for (row, grantable) in GRANTABLE.iter().enumerate() { let open = pid.is_some_and(|pid| crate::isa::bound_to(row, pid)); + // A row's ports open and close together, so its first bit is its state. + let Some(&first) = grantable.ports.first() else { continue }; + if super::percpu::port_open(first) == open { + continue; + } for &port in grantable.ports { super::percpu::set_port_open(port, open); } } } +/// The first port of `access` this CPU refuses Ring 3, which is the port its +/// #GP faulted on; the process that faulted is still this CPU's. +pub fn refused_port(access: PortAccess) -> u16 { + (0..u16::from(access.bytes)) + .map(|i| access.port.wrapping_add(i)) + .find(|&port| !super::percpu::port_open(port)) + .unwrap_or(access.port) +} + /// An `in` or `out` as decoded from the bytes at a faulting instruction. #[derive(Clone, Copy)] pub struct PortAccess { diff --git a/kernel/src/drivers/panic_console/mod.rs b/kernel/src/drivers/panic_console/mod.rs index 3b240b6181c..c1c707df19b 100644 --- a/kernel/src/drivers/panic_console/mod.rs +++ b/kernel/src/drivers/panic_console/mod.rs @@ -6,9 +6,9 @@ //! [`render`] paints it inside `halt_all_cpus`, before `panic_flush`. virtio-gpu //! is unsupported: its scanout needs the unbounded-poll wedge this module avoids. //! -//! The two holds this module ends a panic in — [`page_forever`] and -//! [`hold_the_panel`] — are also where `crate::panic_reboot`'s bound is -//! watched. Neither reads input: a dead kernel takes none. +//! The hold this module ends a panic in, [`hold_the_panel`], is also where +//! `crate::panic_reboot`'s bound is watched. It reads no input and scrolls +//! nothing: a dead kernel takes none, and its report shows what fits, once. mod access; mod latch; @@ -43,12 +43,6 @@ const _: () = assert!(SNAPSHOT_CAP >= MAX_ROWS * MAX_COLS); /// One bit per byte `text` can hold — worst case is a message of nothing but newlines, one line per byte. const ALERT_WORDS: usize = SNAPSHOT_CAP.div_ceil(64); -/// How long each page stays up; a [`Cadence`], not a deadline — nothing expires. -const PAGE_HOLD: Cadence = Cadence::every( - Duration::from_secs(3), - "a five-page report cycles inside a 15-second video", -); - /// How long Ctrl+Alt+D's report keeps the panel. const REPORT_HOLD: Budget = Budget::of( Duration::from_secs(15), @@ -579,8 +573,7 @@ fn live_tail() -> View<'static> { // already partly consumed. /// What a fatal path paints: the captured report, or the live ring for a -/// path that reached `halt_all_cpus` without the panic handler. Idempotent -/// for the captured case, which [`page_forever`] walks repeatedly. +/// path that reached `halt_all_cpus` without the panic handler. fn fatal_text() -> View<'static> { if CAPTURE_ACCESS.read() { // SAFETY: `read` changed the state to `READING` before access; every writer refuses that state. @@ -630,8 +623,8 @@ fn fatal_claimed() -> bool { } /// Paint the newest page of the captured report, fatal paths only; returns -/// whether this call claimed the screen, entitling [`page_forever`] — which it -/// does whatever any painter that is not a fatal path holds. +/// whether this call claimed the screen, entitling [`hold_the_panel`] — which +/// it does whatever any painter that is not a fatal path holds. pub fn render() -> bool { if !seize() { return false; @@ -641,7 +634,7 @@ pub fn render() -> bool { // Before the paint, from the same view the panel gets: a fault inside the // painter then costs the screen and not the copy the next boot reads. crate::blackbox::record_panic(text.text); - paint(Fill::Fatal, text, Page::Last, Watch::No, || false); + paint(Fill::Fatal, text, Watch::No, || false); true } @@ -663,45 +656,15 @@ pub fn seal_wedge(said: core::fmt::Arguments) { crate::blackbox::record_wedge(format_args!("{said}{Census}\n"), live_tail().text); } -/// Cycle the report across the screen until the machine is switched off, or -/// until `bound` returns it to firmware. Reached only from `halt_all_cpus`, -/// after `panic_flush`, on the CPU whose [`render`] claimed [`FATAL`]; the -/// handler's other two exits reach [`hold_the_panel`] the same way, and every -/// one of the three is the last call its CPU makes. -pub fn page_forever(bound: Bound) -> ! { - if !crate::clock::calibrated() { - hold_the_panel(bound); - } - let text = fatal_text(); - let Some(fb) = snapshot() else { hold_the_panel(bound) }; - let Some((cols, grid_rows)) = geometry(&fb) else { hold_the_panel(bound) }; - let (_, pages, _) = pagination(text.text, cols, grid_rows); - if pages < 2 { - hold_the_panel(bound); - } - // `None` is the screenful [`render`] already painted, not a numbered page. - let mut shown: Option = None; - // Spins rather than `hlt`: nothing would wake it, and re-arming the LAPIC timer would dispatch the scheduler mid-panic. - loop { - let until = crate::clock::nanos_since_boot().saturating_add(PAGE_HOLD.nanos()); - while crate::clock::nanos_since_boot() < until { - bound.check(); - core::hint::spin_loop(); - } - let next = shown.map_or(0, |page| (page + 1) % pages); - paint(Fill::Fatal, text, Page::Nth(next), Watch::No, || false); - shown = Some(next); - } -} - -/// Keep the panel as it is until `bound` resets the machine. The panic path's -/// terminal hold wherever there is no second page to cycle — and the whole of -/// it on a machine with no panel at all. +/// Keep the panel as it is until `bound` resets the machine: the panic path's +/// terminal hold, and the last call its CPU makes. pub fn hold_the_panel(bound: Bound) -> ! { if !bound.is_armed() { // Nothing to wait for, so this CPU costs the machine no power. crate::arch::cpu::halt() } + // Spins rather than `hlt`: nothing would wake it, and re-arming the LAPIC + // timer would dispatch the scheduler mid-panic. loop { bound.check(); core::hint::spin_loop(); @@ -743,7 +706,7 @@ fn repaint() { if SCREEN_OWNED_BY_USERLAND.load(Ordering::Relaxed) || !take_for_boot() { return; } - paint(Fill::Boot, live_tail(), Page::Last, Watch::No, fatal_claimed); + paint(Fill::Boot, live_tail(), Watch::No, fatal_claimed); PAINTING.store(false, Ordering::SeqCst); } @@ -779,7 +742,7 @@ fn paint_held_report() { #[cfg(feature = "boot-actuators")] stall::inside_the_latch(); forget_the_glass(); - paint(Fill::Boot, report_text(), Page::Last, Watch::Yes, fatal_claimed); + paint(Fill::Boot, report_text(), Watch::Yes, fatal_claimed); PAINTING.store(false, Ordering::SeqCst); } @@ -819,14 +782,6 @@ pub fn hold_report() { paint_held_report(); } -/// Which slice of the text a paint shows. -#[derive(Clone, Copy)] -enum Page { - /// The newest screenful: a page-aligned last page would leave the bottom blank when rows divide badly. - Last, - Nth(usize), -} - /// Carries "halted" vs "still booting" at zero cost, and proves the console ran this boot. #[derive(Clone, Copy)] enum Fill { @@ -1072,7 +1027,7 @@ fn spent(began: u64, pixels: u64) { /// `stop` is asked before every row and every scanline of a fill: a paint it /// answers yes to leaves at once, and the grid is forgotten, not believed. -fn paint(fill: Fill, view: View, page: Page, watch: Watch, stop: impl Fn() -> bool) { +fn paint(fill: Fill, view: View, watch: Watch, stop: impl Fn() -> bool) { let Some(fb) = snapshot() else { return }; if !mapped(&fb) { return; @@ -1082,14 +1037,9 @@ fn paint(fill: Fill, view: View, page: Page, watch: Watch, stop: impl Fn() -> bo let mut pixels = 0u64; let text = view.text; let (total, pages, per) = pagination(text, cols, grid_rows); - // `Last` is the newest `per` rows, not page `pages - 1`, so both an even - // and uneven division fill the screen; the footer's page number therefore - // can't be derived from `first` and `shown` carries it separately. - let newest = total.saturating_sub(per); - let (first, shown) = match page { - Page::Last => (newest, pages), - Page::Nth(n) => (n.saturating_mul(per).min(newest), (n + 1).min(pages)), - }; + // The newest `per` rows, not page `pages - 1`: a page-aligned last page + // would leave the bottom blank when rows divide badly. + let first = total.saturating_sub(per); let ground = match fill { Fill::Fatal => rgb(&fb, 0x60, 0x00, 0x00), @@ -1143,7 +1093,7 @@ fn paint(fill: Fill, view: View, page: Page, watch: Watch, stop: impl Fn() -> bo } } if pages > 1 && r == grid_rows - 1 { - footer_cells(shown, pages, want); + footer_cells(pages, want); } let glassed = glass.row(r, cols); @@ -1208,15 +1158,15 @@ fn flush_stores() { crate::arch::barrier::scanout_flush(); } -/// `[page 2/4]` into the bottom row's cells; not decoration — the pager advances on a timer with no key to press. -fn footer_cells(page: usize, pages: usize, row: &mut [Cell]) { +/// `[page 4/4]` into the bottom row's cells: the screen is the last page of more. +fn footer_cells(pages: usize, row: &mut [Cell]) { let mut buf = [0u8; 24]; let mut n = 0; for &b in b"[page " { buf[n] = b; n += 1; } - n += write_num(&mut buf[n..], page); + n += write_num(&mut buf[n..], pages); buf[n] = b'/'; n += 1; n += write_num(&mut buf[n..], pages); diff --git a/kernel/src/isa.rs b/kernel/src/isa.rs index 2024cf5fd6a..971ff89a27f 100644 --- a/kernel/src/isa.rs +++ b/kernel/src/isa.rs @@ -74,6 +74,17 @@ static WATCHES: [Watch; MAX_ROWS] = [const { Watch::new() }; MAX_ROWS]; /// Mint the claim on the row `set` names, lines routed and unmasked. pub fn claim(set: IsaId) -> Result { + #[cfg(feature = "boot-actuators")] + if crate::actuator::isa_claim_straddles_quarantine() { + let begun = straddle::begin(); + let answer = mint(set); + straddle::answered(begun); + return answer; + } + mint(set) +} + +fn mint(set: IsaId) -> Result { let row = GRANTABLE .iter() .position(|g| { @@ -178,3 +189,53 @@ pub fn drain_pending() { pub fn watch(row: usize) -> &'static Watch { &WATCHES[row] } + +/// The `isa-claim-straddles-quarantine` actuator: a claim answered between the +/// two steps of the i8042's quarantine, which holds after its first until one +/// begun after it has been. +#[cfg(feature = "boot-actuators")] +pub mod straddle { + use core::sync::atomic::{AtomicBool, AtomicU32, AtomicU64, Ordering}; + + /// Claims begun this boot. + static BEGUN: AtomicU64 = AtomicU64::new(0); + /// [`BEGUN`] when the quarantine's first step ran; [`NONE`] when it holds none. + static HELD_FROM: AtomicU64 = AtomicU64::new(NONE); + const NONE: u64 = u64::MAX; + /// The lines the first step masked, for the second's log. + static MASKED: AtomicU32 = AtomicU32::new(0); + /// A claim begun after the first step has been answered. + static STRADDLED: AtomicBool = AtomicBool::new(false); + + pub(super) fn begin() -> u64 { + BEGUN.fetch_add(1, Ordering::SeqCst) + } + + /// A claim that began before the first step read the controller as driven + /// either way, so only a later one decides anything. + pub(super) fn answered(begun: u64) { + let from = HELD_FROM.load(Ordering::SeqCst); + if from != NONE && begun >= from { + STRADDLED.store(true, Ordering::SeqCst); + } + } + + /// The quarantine's first step ran and masked `masked` lines. + pub fn hold(masked: u32) { + MASKED.store(masked, Ordering::SeqCst); + STRADDLED.store(false, Ordering::SeqCst); + HELD_FROM.store(BEGUN.load(Ordering::SeqCst), Ordering::SeqCst); + log!("isa: the i8042's quarantine holds after its first step for a claim"); + } + + /// The first step's masked count, once a claim begun after it has been + /// answered; the second step is the caller's. + pub fn resume() -> Option { + if !STRADDLED.swap(false, Ordering::SeqCst) { + return None; + } + HELD_FROM.store(NONE, Ordering::SeqCst); + log!("isa: a claim was answered between the i8042's quarantine steps"); + Some(MASKED.load(Ordering::SeqCst)) + } +} diff --git a/kernel/src/panic.rs b/kernel/src/panic.rs index e022712919a..63ef445ab72 100644 --- a/kernel/src/panic.rs +++ b/kernel/src/panic.rs @@ -370,10 +370,10 @@ pub fn halt_all_cpus() -> ! { unsafe { serial::panic_flush(); } // Must follow the flush — it's the deepest stack this path reaches. crate::arch::trap::report_fault_stack(); - // page_forever runs strictly after the flush: it is an unbounded loop and may only run once the serial report is out. + // hold_the_panel runs strictly after the flush: it is an unbounded loop and may only run once the serial report is out. // Only the CPU that painted watches the bound; the rest halt below. if painted { - crate::drivers::panic_console::page_forever(bound); + crate::drivers::panic_console::hold_the_panel(bound); } cpu::halt(); } diff --git a/tests/common/faults.rs b/tests/common/faults.rs index 21bdb039902..31b7e50b1a4 100644 --- a/tests/common/faults.rs +++ b/tests/common/faults.rs @@ -16,8 +16,11 @@ use std::io::Write; use std::path::Path; use std::time::Duration; +use toyos_build::bootlog; + use super::qemu::{self, BootOptions, QemuInstance}; use super::serial::Serial; +use super::{compile, logstream}; /// The line `ist1_report` writes to the UART. const MARKER: &str = "[ist1] used "; @@ -1298,3 +1301,69 @@ fn unstamped(line: &str) -> &str { let line = line.trim(); line.strip_prefix("[kernel ").and_then(|rest| rest.split_once("] ")).map_or(line, |(_, said)| said) } + +/// Where `test_rs_fault_gate_child`'s kernel arms aim: the direct map's first +/// words, which no process may name. +const KERNEL_RSP: &str = "0xffff800000000000"; +const KERNEL_RBP: &str = "0xffff800000000010"; +const KERNEL_READ: &str = "0xffff800000000008"; + +/// **A crash report reads a faulting process's memory only at user +/// addresses.** One child dies with its stack and frame pointers aimed at the +/// kernel's direct map, another reading there; the report names each refusal, +/// and neither the console nor `/log` carries a word from behind them or the +/// kernel's page walk for the read. +pub fn crash_report_reads_no_kernel_memory( + c_bins: &[(String, Vec)], + rust_bins: &[(String, Vec)], +) -> Result<(), String> { + const CONFIG: &str = "tests/testcases"; + let staged = logstream::stage(CONFIG, "crash-report-kernel-reads", c_bins, rust_bins)?; + let options = BootOptions { + boot_image: Some(qemu::Staged::Written(staged.image.clone())), + ..Default::default() + }; + let mut guest = QemuInstance::boot_with_options( + &compile::repo_root().join(CONFIG), + c_bins, + rust_bins, + options, + ); + let mut console = guest.boot_log().to_string(); + let mut said = String::new(); + for (kind, header) in [("kernel_stack", "SIGILL tid="), ("kernel_page", "SEGFAULT tid=")] { + let ran = guest.run_test(&format!("test_rs_fault_gate_child {kind}"), Duration::from_secs(30)); + if ran.exit_code == Some(0) || ran.stdout.contains("survived") { + return Err(format!("{kind} survived its fault\n{}", ran.stdout)); + } + // The premise: the kernel reported this child's fault at all. + if !ran.serial.contains(header) { + return Err(format!("{kind}: no {header:?} report\n{}", ran.serial)); + } + said.push_str(&ran.before); + said.push_str(&ran.serial); + } + let log = logstream::shut_down(guest, &mut console, &staged)?.concat(); + let _ = std::fs::remove_file(&staged.image); + let kernel = bootlog::kernel_records(&log); + for (what, text) in [("the console", said.as_str()), ("/log's kernel records", kernel.as_str())] { + for refused in [ + format!("Stack (from RSP): {KERNEL_RSP} refused: no user address"), + format!("rbp {KERNEL_RBP} refused: no user address"), + format!("Page walk for {KERNEL_READ} refused: no user address"), + ] { + if !text.contains(&refused) { + return Err(format!("{what} carries no {refused:?}\n{text}")); + } + } + // A stack word the report read is `[address] = value`, and the walk's + // header is `Page walk for address [PML4=…`. + let walk = format!("Page walk for {KERNEL_READ} ["); + let leaked = |l: &&str| (l.contains("[0xffff8") && l.contains("] = ")) || l.contains(&walk); + if let Some(line) = text.lines().find(leaked) { + return Err(format!("{what} carries kernel memory a crash report read: {line:?}")); + } + } + eprintln!(" [crash] both reports refused the direct map, on the console and in /log"); + Ok(()) +} diff --git a/tests/common/power.rs b/tests/common/power.rs index 5f59de4fe6d..b633d741491 100644 --- a/tests/common/power.rs +++ b/tests/common/power.rs @@ -739,7 +739,11 @@ fn panic_armed() -> String { format!("{PANIC_ARMED_HEAD} {PANIC_FAST_SECS} s, timed by ") } -/// A guest whose kernel panicked and armed the bound. +/// A guest whose kernel panicked and armed the bound. `Profile::Metal`: QEMU +/// routes an injected key to one handler per device class, and this is the only +/// GOP profile with an i8042 and no `usb-kbd` to send it to instead, so +/// `panic_ignores_keys`' key reaches the controller a kernel that read input +/// would read. fn panicked() -> BootOptions { BootOptions { profile: qemu::Profile::Metal, diff --git a/tests/test-durations b/tests/test-durations index 90d093b237f..2696184147f 100644 --- a/tests/test-durations +++ b/tests/test-durations @@ -327,7 +327,6 @@ screen_i8042_health 4495 screen_late_panic 3926 screen_loader_lines 3991 screen_log_absent 1823 -screen_paged_scrollback 7384 screen_panic_muted 4416 shipped_config_boots 3024 shm_release_reclaims 29 diff --git a/tests/toyos-rust-tests/src/bin/fault_gate_child.rs b/tests/toyos-rust-tests/src/bin/fault_gate_child.rs index c1c9c10cb6e..46330cc4eae 100644 --- a/tests/toyos-rust-tests/src/bin/fault_gate_child.rs +++ b/tests/toyos-rust-tests/src/bin/fault_gate_child.rs @@ -19,6 +19,8 @@ fn main() { "xm" => simd_exception(), "ac" => alignment_check(), "pf" => read_null(), + "kernel_stack" => kernel_stack(), + "kernel_page" => kernel_page(), other => panic!("unknown fault kind {other}"), } println!("survived {kind}"); @@ -177,3 +179,31 @@ fn alignment_check() { } println!(" RFLAGS.AC readback after a misaligned load: {}", (flags >> 18) & 1); } + +/// The kernel's direct map starts here (`kernel/src/mm/mod.rs`'s +/// `PHYS_OFFSET`): the first byte of physical memory, and no process's. +const DIRECT_MAP: u64 = 0xFFFF_8000_0000_0000; + +/// #UD (6) with the stack and frame pointers aimed at the direct map, so a +/// crash report that followed either would print physical memory. +#[inline(never)] +fn kernel_stack() { + // SAFETY: none — the fault is the point, and it ends this process. + unsafe { + core::arch::asm!( + "mov rsp, {s}", + "mov rbp, {f}", + "ud2", + s = in(reg) DIRECT_MAP, + f = in(reg) DIRECT_MAP + 0x10, + options(noreturn), + ); + } +} + +/// #PF (14) at a direct-map address, whose page walk is the kernel's tables. +#[inline(never)] +fn kernel_page() { + // SAFETY: none — the fault is the point, and it ends this process. + unsafe { core::ptr::read_volatile((DIRECT_MAP + 8) as *const u64) }; +} diff --git a/tests/toyos-rust-tests/src/bin/isa_grant.rs b/tests/toyos-rust-tests/src/bin/isa_grant.rs index 60299c5b1e6..6980315188a 100644 --- a/tests/toyos-rust-tests/src/bin/isa_grant.rs +++ b/tests/toyos-rust-tests/src/bin/isa_grant.rs @@ -11,10 +11,15 @@ //! is the harness's to read. //! - `device`: the real controller the kernel gave up on at boot, driven from //! here until a keystroke arrives as a record and a byte. +//! - `straddled`: `device`, on a controller the kernel drove until its +//! quarantine, the claim asked for until the kernel lets it go. +//! - `released`: nothing, run after `device` has ended, so the key the host +//! presses in between has had its interrupt. //! //! The children: `unbound` holds the claim and never read it, `bound` read it -//! and steps one port past what it was granted, `unclaimed` holds nothing, and -//! `moved` was handed a claim its parent had already bound. +//! and steps one port past what it was granted, `wide` read it and makes a +//! two-byte access at its data port, `unclaimed` holds nothing, and `moved` was +//! handed a claim its parent had already bound. use std::os::toyos::process::CommandExt; use std::process::{Command, Stdio}; @@ -47,16 +52,23 @@ const CONTROLLER: Duration = Duration::from_secs(1); /// this long is not coming. const KEYSTROKE: Duration = Duration::from_secs(20); -/// Set 1's make code for `a`: the controller translates the keyboard's set 2. +/// Set 1's make and break codes for `a`: the controller translates the +/// keyboard's set 2. const A_MAKE: u8 = 0x1E; +const A_BREAK: u8 = 0x9E; fn main() { match std::env::args().nth(1).as_deref() { Some("driven") => driven(&syscap()), Some("grant") => grant(&syscap()), - Some("device") => device(&syscap()), + Some("device") => { + device(claim(&syscap(), I8042).expect("isa device: the kernel gave the controller up at boot")) + } + Some("straddled") => device(claim_once_let_go(&syscap())), + Some("released") => println!("isa released: the claim before this one is gone"), Some("unbound") => unbound(), Some("bound") => bound(), + Some("wide") => wide(), Some("unclaimed") => unclaimed(), Some("moved") => moved(), other => panic!("isa_grant: unknown role {other:?}"), @@ -87,6 +99,15 @@ fn inb(port: u16) -> u8 { value } +fn inw(port: u16) -> u16 { + let value: u16; + // SAFETY: as `inb`; the access spans `port` and the port after it. + unsafe { + core::arch::asm!("in ax, dx", in("dx") port, out("ax") value, options(nomem, nostack)); + } + value +} + fn outb(port: u16, value: u8) { // SAFETY: as `inb`; the ports written are the i8042's, granted to this process. unsafe { @@ -158,6 +179,8 @@ fn grant(cap: &SysCap) { // The claim died with its unbound holder, so the row is free again. let held = claim(cap, I8042).expect("isa: the row came back from a holder that never bound it"); killed_after("bound", child("bound", Some(held)), "bound: in from 0x61"); + let held = claim(cap, I8042).expect("isa: the row came back from the bound child"); + killed_after("wide", child("wide", Some(held)), "wide: in of two bytes from 0x60"); killed_after("unclaimed", child("unclaimed", None), "unclaimed: in from 0x60"); // The ports went back with the process that bound them. @@ -200,6 +223,14 @@ fn bound() { println!("bound: survived"); } +fn wide() { + let claim = taken(); + bind(&claim); + println!("wide: in of two bytes from 0x60"); + let _ = inw(DATA); + println!("wide: survived"); +} + fn unclaimed() { println!("unclaimed: in from 0x60"); let _ = inb(DATA); @@ -232,10 +263,11 @@ fn command(byte: u8) { outb(STATUS, byte); } -fn device(cap: &SysCap) { - let claim = claim(cap, I8042).expect("isa device: the kernel gave the controller up at boot"); +/// Drive the controller through `claim` until a keystroke arrives as a record +/// and a byte. +fn device(claim: Device) { bind(&claim); - // Whatever the kernel's aborted probe left behind. + // Whatever the kernel left behind. for _ in 0..32 { if inb(STATUS) & OBF == 0 { break; @@ -269,6 +301,34 @@ fn device(cap: &SysCap) { wait_status("had the byte behind its interrupt", |s| s & OBF != 0); let byte = inb(DATA); assert_eq!(byte, A_MAKE, "isa device: the interrupt carried {byte:#04x}, not `a`'s make"); + // The key's release too, so the output buffer is empty when this claim + // goes and the host's next key would raise the line. + wait_status("had the key's release", |s| s & OBF != 0); + let released = inb(DATA); + assert_eq!(released, A_BREAK, "isa device: the release carried {released:#04x}, not `a`'s break"); println!("isa device: {count} interrupt(s), scancode {byte:#04x}"); println!("===ISA_DEVICE_OK==="); } + +/// The claim on a controller the kernel drives, asked for until the kernel lets +/// it go: at least one refusal first, or the kernel never drove it here. +fn claim_once_let_go(cap: &SysCap) -> Device { + println!("===ISA_CLAIMING==="); + let by = Instant::now() + KEYSTROKE; + let mut refused = 0u32; + let claim = loop { + match claim(cap, I8042) { + Ok(claim) => break claim, + Err(SyscallError::PermissionDenied) => refused += 1, + Err(other) => panic!("isa straddled: a claim answered {other:?}"), + } + assert!( + Instant::now() < by, + "isa straddled: the kernel never let the controller go in {KEYSTROKE:?}" + ); + std::thread::yield_now(); + }; + assert!(refused > 0, "isa straddled: the first claim was granted, so the kernel never drove the controller"); + println!("isa straddled: {refused} claim(s) refused before the kernel let the controller go"); + claim +} diff --git a/tests/toyos.rs b/tests/toyos.rs index 00dd5a95f5e..2b17b88a6be 100644 --- a/tests/toyos.rs +++ b/tests/toyos.rs @@ -508,7 +508,6 @@ const SCREEN_TESTS: &[(&str, Sched, Tier)] = &[ // compositor is holding it. The verdict is the report on the panel. ("screen_blocked_dump", Sched::Parallel, Tier::Nightly), ("screen_late_panic", Sched::Parallel, Tier::Fast), - ("screen_paged_scrollback", Sched::Parallel, Tier::Weekly), ("screen_panic_muted", Sched::Parallel, Tier::Weekly), ("screen_console_panic", Sched::Parallel, Tier::Fast), ("screen_fatal_halt", Sched::Parallel, Tier::Fast), @@ -612,11 +611,12 @@ const MACHINE_TESTS: &[(&str, Sched, Tier)] = &[ // The `isa` claim, a boot each: refused where the kernel drives the i8042, // its ports granted and refused by the I/O permission bitmap where there is // none, and a real controller the kernel gave up on driven to a keystroke. - // Every verdict is a guest's line or the kernel's record of a kill; the one - // wait is on the guest's own ready line. + // Every verdict is a guest's line or the kernel's record. ("isa_claim_refused_where_the_kernel_drives", Sched::Parallel, Tier::Nightly), ("isa_ports_are_the_binders_alone", Sched::Parallel, Tier::Fast), + ("isa_ports_close_on_one_cpu", Sched::Parallel, Tier::Fast), ("isa_lines_reach_their_holder", Sched::Parallel, Tier::Nightly), + ("isa_claim_straddles_the_quarantine", Sched::Parallel, Tier::Nightly), // One boot; every verdict is a PPM header field or a console line, and no // clock is in any of them. ("gpu_set_resolution", Sched::Parallel, Tier::Fast), @@ -964,6 +964,8 @@ const MACHINE_TESTS: &[(&str, Sched, Tier)] = &[ // The exception entry's own seal, off the page's bytes on an ordinary boot. ("blackbox_fault_sealed", Sched::Parallel, Tier::Nightly), ("double_fault_stack", Sched::Parallel, Tier::Nightly), + // A boot of its own: its verdict reads `/log` off the volume after it. + ("crash_report_reads_no_kernel_memory", Sched::Parallel, Tier::Fast), // One boot of its own, ten seconds of Ring 3 spinning, and every verdict is // a count the kernel printed or a line it printed: how many NMIs landed at // CPL 0 with a user `rsp`, against how many landed in Ring 3, both off the @@ -1400,7 +1402,9 @@ const CARRIES: &[(&str, &[&str])] = &[ ("input_claim_absent", &["test_rs_input_absent"]), ("isa_claim_refused_where_the_kernel_drives", &["test_rs_isa_grant"]), ("isa_ports_are_the_binders_alone", &["test_rs_isa_grant"]), + ("isa_ports_close_on_one_cpu", &["test_rs_isa_grant"]), ("isa_lines_reach_their_holder", &["test_rs_isa_grant"]), + ("isa_claim_straddles_the_quarantine", &["test_rs_isa_grant"]), ("gpu_set_resolution", &["test_rs_gpu_set_resolution"]), ("iommu_gpu_scanout_swap", &["test_rs_gpu_scanout_swap"]), ("userdev_dma_fault", &["test_rs_log_origin"]), @@ -1451,6 +1455,7 @@ const CARRIES: &[(&str, &[&str])] = &[ ("writeback_durability", &["test_rs_writeback_durability"]), ("kernel_log_file", &["test_rs_writeback_durability"]), ("double_fault_stack", &["test_rs_test_panic_child"]), + ("crash_report_reads_no_kernel_memory", &["test_rs_fault_gate_child"]), ("idle_stack_guard", &["test_rs_test_panic_child"]), ("syscall_panic_halts", &["test_rs_test_panic_child"]), ("syscall_fault_halts", &["test_rs_test_panic_child"]), @@ -3398,7 +3403,7 @@ fn print_screen(name: &str, text: &str) { /// /// The three summary strings are the answer; the absence of a `[page n/m]` /// footer is what makes one photograph the *whole* answer, because Ctrl+Alt+D -/// paints once and never enters the pager — a report that needed two pages +/// paints once — a report that needed two pages /// would leave the verdict on one nobody can reach. And the fill is the report /// having taken the panel rather than sitting on a client's screen, which is /// the half `boot_checkpoint` deliberately will not do. @@ -3524,38 +3529,6 @@ fn check_wrap(dump: &screen::Ppm) -> Result<(), String> { Ok(()) } -/// Every row on the panel is text the log actually carries. -/// -/// **The check the panel's grid owes**: `panic_console` writes only the cells -/// whose character or colour moved, so a cell it fails to write is one the -/// previous paint left standing, and past the end of a line that replaced a -/// longer one that is a string no line of the log contains. -fn check_no_stale_cells(dump: &screen::Ppm, console: &str) -> Result<(), String> { - let said: String = console - .replace("[kernel ", "[") - .bytes() - .map(|byte| match byte { - b'\n' => '\n', - b'\t' => ' ', - 0x20..=0x7E => byte as char, - _ => '.', - }) - .collect(); - for row in dump.rows() { - let row = row.trim_end(); - if row.is_empty() || row.starts_with("[page ") || said.contains(row) { - continue; - } - return Err(format!( - "the panel row {row:?} is in no line of the log, so a cell the paint that put \ - this screen up did not write is still standing from the one before \ - it\ndecoded screen:\n{}", - dump.text() - )); - } - Ok(()) -} - /// Run one screen test. `Err` carries the decoded screen, because a failure /// here is almost always "the text is not what I expected" and the decoded /// grid is the only readable form of that. @@ -5034,10 +5007,7 @@ fn run_screen_test( ); // Here the marker reaches serial *before* the paint — the drain is // what emits it — so unlike the halt paths this one has to look - // more than once. And once the report outgrows one screen the - // pager cycles it, so the window in which any given page is up is - // `PAGE_HOLD_NS`, not forever: the timeout has to cover a whole - // cycle rather than just the paint. + // more than once. let dump = qemu.screendump_until("PANIC:", Duration::from_secs(30)); let text = dump.text(); print_screen(name, &text); @@ -5073,116 +5043,6 @@ fn run_screen_test( } Ok(()) } - "screen_paged_scrollback" => { - // The screen is smaller than the report, and on the target laptop - // there is no key to press for the rest of it. So the claim under - // test is not "the console renders" — `screen_late_panic` has that - // — but "a line the report page cannot hold reaches the screen - // anyway, with no input". Same feature and image as - // `screen_late_panic`, so it costs a boot and no rebuild. - let mut qemu = QemuInstance::boot_with_options( - test_config, - c_bins, - rust_bins, - BootOptions { - profile: qemu::Profile::Gop, - qmp: true, - kernel_params: &["test-late-panic"], - ready_marker: "PANIC:", - ..Default::default() - }, - ); - - // The first kernel line of the boot, and the one a photograph of - // the final screen has never been able to show. - const HEAD: &str = "panic console: armed"; - const TAIL: &str = "PANIC:"; - - let mut pages: Vec = Vec::new(); - let mut report: Option = None; - let mut head_seen = false; - // **The only incremental paints a guest makes**: the report's own - // paint follows a fill, and every page the pager puts up after it - // is written against the grid the one before left — which is the - // paint `check_no_stale_cells` exists for. The footers of the - // settled captures it judged, and two of them, because the first is - // the page the fill painted. - const JUDGED_PAGES: usize = 2; - let mut judged: Vec = Vec::new(); - let mut before: Option = None; - // A liveness ceiling on a machine that is halted and paging, so - // there is no console to read progress off and this is the case - // `qemu::budget` exists for. - let deadline = Instant::now() + qemu.budget(Duration::from_secs(40)); - while Instant::now() < deadline - && !(head_seen && report.is_some() && judged.len() >= JUDGED_PAGES) - { - let dump = qemu.screendump(); - let text = dump.text(); - let Some(footer) = text.lines().rev().find(|l| l.starts_with("[page ")) else { - // Before the panic the screen still carries a boot - // checkpoint; only a paginated screen has a footer. - before = None; - thread::sleep(Duration::from_millis(200)); - continue; - }; - if !pages.contains(&footer.to_string()) { - pages.push(footer.to_string()); - } - if text.contains(TAIL) { - report = Some(text.clone()); - } - head_seen |= text.contains(HEAD); - // **A screendump is not a shutter**: one taken across a paint - // carries the rows already written above the rows the paint - // replaced, and a row half of each is in no line of any log. Two - // identical captures are a paint that finished. - if before.as_deref() == Some(text.as_str()) { - check_no_stale_cells(&dump, &qemu.console_stream().since(0))?; - if !judged.contains(&footer.to_string()) { - judged.push(footer.to_string()); - } - } - before = Some(text.clone()); - thread::sleep(Duration::from_millis(200)); - } - - let seen = pages.join(" "); - print_screen(name, &format!("footers seen: {seen}")); - let Some(report) = report else { - return Err(format!( - "{STALLED} {TAIL:?} never reached the screen; footers seen: {seen}" - )); - }; - // The premise. If one screen holds both ends there is nothing to - // page and the rest of this test would pass vacuously — which is - // the shape the metal-track review kept finding. - if report.contains(HEAD) { - return Err(format!( - "one screen holds both {HEAD:?} and {TAIL:?}; nothing to page\n{report}" - )); - } - if !head_seen { - return Err(format!( - "{HEAD:?} never reached the screen — the pager did not advance past the \ - report. footers seen: {seen}\nreport page:\n{report}" - )); - } - if pages.len() < 2 { - return Err(format!( - "only one page footer ever appeared ({seen}); the pager is not cycling" - )); - } - if judged.len() < JUDGED_PAGES { - return Err(format!( - "only {} settled page(s) were judged for stale cells ({}), so no paint made \ - against the grid the one before it left was ever read", - judged.len(), - judged.join(" ") - )); - } - Ok(()) - } "screen_fatal_halt" => { // The steady-state fatal path: userland is up, the display is // idle, and SYS_DEBUG action 3 runs halt_all_cpus for real. @@ -5210,8 +5070,6 @@ fn run_screen_test( ) { return Err(format!("{FATAL_HALT_NONCE:?} never reached the console")); } - // Polled, not sampled once: the report is longer than a screen - // here, so the nonce is on one page of a cycling set. let dump = qemu.screendump_until(FATAL_HALT_NONCE, Duration::from_secs(30)); let text = dump.text(); print_screen(name, &text); @@ -5253,7 +5111,7 @@ fn run_screen_test( // go fatal once it holds the latch, so the fatal path meets a // holder beneath itself; the report must take the screen // regardless, and its CPU must go on to watch the reset bound, - // which is what the paging proves. + // which is what the reset proves. const HELD: &str = "panel: a painter holding the panel went fatal"; let mut qemu = QemuInstance::boot_with_options( test_config, @@ -5262,7 +5120,7 @@ fn run_screen_test( BootOptions { profile: qemu::Profile::Gop, qmp: true, - kernel_params: &["panel-painter-stalls"], + kernel_params: &["panel-painter-stalls", "panic-reboot-fast"], ..Default::default() }, ); @@ -5287,17 +5145,8 @@ fn run_screen_test( dump.fill() )); } - // The pager runs only on the CPU that claimed the panel, and it is - // the loop that watches the reset bound: a second page is its proof. - let paged = qemu.screendump_while(Duration::from_secs(20), Duration::from_millis(200), |d| { - d.rows().iter().any(|r| r.contains("[page ")) && d.text() != text - }); - if paged.text() == text { - return Err(format!( - "the report never paged, so no CPU is watching the reset bound\ndecoded \ - screen:\n{text}" - )); - } + let mut tail = String::new(); + qemu::await_reset(&mut qemu, &mut tail, "the CPU that took the panel to reset the machine", &[])?; Ok(()) } "screen_fatal_halt_composited" => { @@ -5348,8 +5197,7 @@ fn run_screen_test( ); } - // The probe fires 5 s after the claim; the poll is for that plus - // the pager cycling pages. + // The probe fires 5 s after the claim; the poll is for that. const MARKER: &str = "metal-panic-probe"; let dump = qemu.screendump_while( Duration::from_secs(40), @@ -9553,6 +9401,58 @@ fn isa_verdict(result: &TestResult, last: &str) -> Result<(), String> { Ok(()) } +/// `test_rs_isa_grant grant` on `smp` CPUs, and every access it makes that the +/// kernel must refuse, named by the kernel. +fn isa_ports( + test_config: &Path, + c_bins: &[(String, Vec)], + rust_bins: &[(String, Vec)], + smp: u32, +) -> Result<(), String> { + // No i8042 at all, so the kernel drives nothing the claim names and the + // ports float: the bitmap is the only thing between a process and them. + let options = BootOptions { + profile: qemu::Profile::MetalNoUsb, + i8042: false, + smp, + ..Default::default() + }; + let mut qemu = QemuInstance::boot_with_options(test_config, c_bins, rust_bins, options); + if !qemu.boot_log().contains("i8042: absent") { + return Err(format!("the i8042 is not absent:\n{}", qemu.boot_log())); + } + let result = qemu.run_test("test_rs_isa_grant grant", Duration::from_secs(60)); + isa_verdict(&result, "===ISA_GRANT_OK===")?; + // Each refused access is the kernel's to name: the port, and whose grant it + // is not. The unbound holder and the moved claim both die at 0x64, the + // bound child one port past what it holds, the wide one on the port its + // access spans past the grant, and the process holding nothing at the data + // port. + const NAMED: [(&str, usize); 4] = [ + ("in of 1 byte(s) from port 0x0064", 2), + ("in of 1 byte(s) from port 0x0061", 1), + ("in of 1 byte(s) from port 0x0060", 1), + ("in of 2 byte(s) from port 0x0060, reaching port 0x0061", 1), + ]; + for (access, times) in NAMED { + let named = format!("{access}, which this process holds no grant for"); + let seen = result.serial.matches(named.as_str()).count(); + if seen != times { + return Err(format!( + "the kernel named {named:?} {seen} time(s), want {times}:\n{}", + result.serial + )); + } + } + for want in ["isa: the i8042's ports are pid", "isa: the i8042's ports went back with pid"] { + if !result.serial.contains(want) { + return Err(format!("the kernel never said {want:?}:\n{}", result.serial)); + } + } + eprintln!(" [isa] {}", result.stdout.trim().replace('\n', "\n [isa] ")); + Ok(()) +} + /// Run one machine-shape test. Like `run_screen_test`, each of these owns its /// QEMU — the machine shape *is* the test — except for the runs of adjacent /// names that share one through `held` (see [`group_boot`]). @@ -10101,6 +10001,9 @@ fn run_machine_test( common::blockd::blockd_lends_within_its_bound(test_config, c_bins, rust_bins) } "double_fault_stack" => faults::double_fault_stack(test_config, c_bins, rust_bins), + "crash_report_reads_no_kernel_memory" => { + faults::crash_report_reads_no_kernel_memory(c_bins, rust_bins) + } "syscall_window_nmi" => faults::syscall_window_nmi(test_config, c_bins, rust_bins), "syscall_window_nmi_controls" => { faults::syscall_window_nmi_controls(test_config, c_bins, rust_bins) @@ -13792,48 +13695,10 @@ fn run_machine_test( eprintln!(" [isa] the claim on the controller the kernel drives was refused"); Ok(()) } - "isa_ports_are_the_binders_alone" => { - // No i8042 at all, so the kernel drives nothing the claim names and - // the ports float: the bitmap is the only thing between a process - // and them. - let options = BootOptions { - profile: qemu::Profile::MetalNoUsb, - i8042: false, - ..Default::default() - }; - let mut qemu = - QemuInstance::boot_with_options(test_config, c_bins, rust_bins, options); - if !qemu.boot_log().contains("i8042: absent") { - return Err(format!("the i8042 is not absent:\n{}", qemu.boot_log())); - } - let result = qemu.run_test("test_rs_isa_grant grant", Duration::from_secs(60)); - isa_verdict(&result, "===ISA_GRANT_OK===")?; - // Each refused access is the kernel's to name: the port, and whose - // grant it is not. The unbound holder and the moved claim both die - // at 0x64, the bound child one port past what it holds, and the - // process holding nothing at the data port. - for (port, times) in [("0x0064", 2), ("0x0061", 1), ("0x0060", 1)] { - let named = format!( - "in of 1 byte(s) from port {port}, which this process holds no grant for" - ); - let seen = result.serial.matches(named.as_str()).count(); - if seen != times { - return Err(format!( - "the kernel named {named:?} {seen} time(s), want {times}:\n{}", - result.serial - )); - } - } - for want in - ["isa: the i8042's ports are pid", "isa: the i8042's ports went back with pid"] - { - if !result.serial.contains(want) { - return Err(format!("the kernel never said {want:?}:\n{}", result.serial)); - } - } - eprintln!(" [isa] {}", result.stdout.trim().replace('\n', "\n [isa] ")); - Ok(()) - } + "isa_ports_are_the_binders_alone" => isa_ports(test_config, c_bins, rust_bins, 2), + // One CPU: every child runs where the one before it ran, so a switch + // that fails to close the row leaves the next child its ports. + "isa_ports_close_on_one_cpu" => isa_ports(test_config, c_bins, rust_bins, 1), "isa_lines_reach_their_holder" => { // `i8042-budget-expired` has the kernel give the controller up // before it arms a line, so a real i8042 is here for a claim. @@ -13858,8 +13723,70 @@ fn run_machine_test( |socket| qemu::qmp_send_keys(socket, &[("a", true), ("a", false)]), ); isa_verdict(&result, "===ISA_DEVICE_OK===")?; - if !result.serial.contains("isa: the i8042 took its first interrupt") { - return Err(format!("the line's first interrupt was never said:\n{}", result.serial)); + // The claim went with `device`, which read the controller empty + // first, so a key now raises line 1 again: masked, it reaches no + // one, and unmasked it is a second first interrupt on a record the + // release cleared. `released` runs after the key has been sent. + let socket = qemu.qmp_socket().to_path_buf(); + qemu::qmp_send_keys(&socket, &[("a", true), ("a", false)]); + let after = qemu.run_test("test_rs_isa_grant released", Duration::from_secs(30)); + isa_verdict(&after, "isa released:")?; + const FIRST: &str = "isa: the i8042 took its first interrupt"; + let said = format!("{}{}{}", result.serial, after.before, after.serial); + match said.matches(FIRST).count() { + 1 => {} + seen => { + return Err(format!( + "the kernel said {FIRST:?} {seen} time(s), want once: a key after the \ + claim went reached its line\n{said}" + )) + } + } + eprintln!(" [isa] {}", result.stdout.trim().replace('\n', "\n [isa] ")); + eprintln!(" [isa] a key after the claim went raised nothing"); + Ok(()) + } + "isa_claim_straddles_the_quarantine" => { + // The kernel drives the i8042 until a flood quarantines it, and + // `isa-claim-straddles-quarantine` holds the quarantine between its + // two steps until a claim has been answered: refused while the + // driver holds the controller, and granted after, with lines the + // quarantine no longer touches. + let mut qemu = QemuInstance::boot_with_options( + test_config, + c_bins, + rust_bins, + BootOptions { + profile: qemu::Profile::Metal, + qmp: true, + kernel_params: &["i8042-fault", "isa-claim-straddles-quarantine"], + ..Default::default() + }, + ); + if !qemu.boot_log().contains("i8042: fault injection armed") { + return Err(format!("the fault was never armed:\n{}", qemu.boot_log())); + } + // The first key floods the driver into its quarantine, the second + // is the holder's keystroke. + let result = qemu.run_test_paced( + "test_rs_isa_grant straddled", + Duration::from_secs(90), + |socket, line| { + if line.contains("===ISA_CLAIMING===") || line.contains("===ISA_DEVICE_READY===") { + let socket = socket.expect("this boot has a QMP socket"); + qemu::qmp_send_keys(socket, &[("a", true), ("a", false)]); + } + }, + ); + isa_verdict(&result, "===ISA_DEVICE_OK===")?; + for want in [ + "isa: the i8042's quarantine holds after its first step for a claim", + "isa: a claim was answered between the i8042's quarantine steps", + "i8042: quarantined", + ] { + if !result.serial.contains(want) { + return Err(format!("the kernel never said {want:?}:\n{}", result.serial)); + } } eprintln!(" [isa] {}", result.stdout.trim().replace('\n', "\n [isa] ")); Ok(()) From 418df1f897c2d5c0b190a995482c46570b16eb12 Mon Sep 17 00:00:00 2001 From: japabu Date: Tue, 29 Sep 2026 09:50:16 +0200 Subject: [PATCH 05/10] issues: the future i8042 server is ps2server, not ps2d Services get no d suffix; the naming rule caught this stage before the server exists. Co-Authored-By: Claude Opus 5.5 --- ...he-kernel-is-small-interrupts-post-and-threads-wait.md | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/issues/kernel/the-kernel-is-small-interrupts-post-and-threads-wait.md b/issues/kernel/the-kernel-is-small-interrupts-post-and-threads-wait.md index 5a95172ab78..c06ff59b5d6 100644 --- a/issues/kernel/the-kernel-is-small-interrupts-post-and-threads-wait.md +++ b/issues/kernel/the-kernel-is-small-interrupts-post-and-threads-wait.md @@ -149,16 +149,16 @@ times: 1. **The panic console takes no input, and an `isa` claim grants a process exact ports through the TSS I/O permission bitmap and its ISA lines as records** (`kernel/src/isa.rs`). **Done** (#592). - 2. **ps2d**, the server over that claim, feeding the kernel's keyboard and - mouse streams so Ctrl+Alt+D and the merge with USB HID stay where they - are; the kernel's driver, its vector, its actuators and the + 2. **ps2server**, the server over that claim, feeding the kernel's keyboard + and mouse streams so Ctrl+Alt+D and the merge with USB HID stay where + they are; the kernel's driver, its vector, its actuators and the `keyboard_controller` seam deleted. Constraints: the harness paces typed input on the kernel's `i8042: drain bytes=` trace (`shell_type_once`, every `i8042-trace` boot), every boot config that types needs the server, and a keyboard claim is refused while no source exists, which init's order of endowment then decides. **Exit**: every keyboard and mouse guest test green with no i8042 code in the kernel, and typing resumes after - ps2d is killed and restarted. + ps2server is killed and restarted. ## Standing From 744b782de316b771c788e8224cafb0500d5211a6 Mon Sep 17 00:00:00 2001 From: japabu Date: Tue, 29 Sep 2026 10:56:39 +0200 Subject: [PATCH 06/10] PR #592 round 4: the quarantine's flood closes to one CPU, the after-release census waits on the controller, and a refusal never guesses a port MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `service()` swaps `QUARANTINE` instead of loading it, so two CPUs racing the top of a scheduler pass cannot both run `quarantine()` — the store the old code made inside `quarantine()` is gone with it, since the swap already claimed the flood. `isa_lines_reach_their_holder`'s after-release check now waits, bounded, on the i8042's output buffer through the QMP monitor before judging the interrupt census, instead of racing the harness's own scheduling against QEMU's delivery of the key it just sent. `pio::refused_port` answers `None` rather than the access's own port when no port in the span is closed, so a kill record never blames a port the bitmap did not actually refuse. The stale pager-reentry comment in the panic path is deleted with the pager itself. No deterministic guest test forces the quarantine race: the window it closed was a few instructions between one atomic load and a later, unsynchronized store, narrower than a scheduler-pass-granularity actuator can straddle without wiring a permanent rendezvous into every pass's hottest path. Co-Authored-By: Claude Opus 5.5 --- kernel/src/arch/x86_64/i8042/mod.rs | 11 ++++++-- kernel/src/arch/x86_64/idt/exceptions.rs | 7 +++-- kernel/src/arch/x86_64/pio.rs | 7 ++--- kernel/src/main.rs | 1 - tests/toyos.rs | 34 +++++++++++++++++++++++- 5 files changed, 51 insertions(+), 9 deletions(-) diff --git a/kernel/src/arch/x86_64/i8042/mod.rs b/kernel/src/arch/x86_64/i8042/mod.rs index bbde0646b4e..ca04fd4e653 100644 --- a/kernel/src/arch/x86_64/i8042/mod.rs +++ b/kernel/src/arch/x86_64/i8042/mod.rs @@ -591,7 +591,15 @@ pub fn service() { // Unconditional and first: an undrained `irq_ring` record keeps // `any_pending_self` true, spinning a CPU that never halts. let recorded = crate::irq_ring::take(IrqSource::I8042).is_some(); - if QUARANTINE.load(Ordering::Relaxed) { + // `swap`, not `load`: two CPUs racing `service()` must not both see the + // flood and both run `quarantine()`. `Relaxed` only has to make the swap + // itself exclusive; `quarantine()`'s own writes (`ACTIVE`'s `Release`, + // the I/O APIC's mask) carry whatever ordering they separately need. No + // guest test forces this: the window this closes was a few instructions + // between one atomic load and a later, unsynchronized store, narrower + // than anything a scheduler-pass-granularity actuator can straddle + // without adding a permanent rendezvous to every pass's hottest path. + if QUARANTINE.swap(false, Ordering::Relaxed) { quarantine(); return; } @@ -790,7 +798,6 @@ fn drain() -> Drained { /// [`ACTIVE`] holds, so a claim that lands once it is clear routes and unmasks /// lines nothing here touches again. fn quarantine() { - QUARANTINE.store(false, Ordering::Relaxed); // The pin is about to be masked, so no health verdict follows this line. HEALTH.store(HEALTH_DONE, Ordering::Relaxed); // The count, not the intent: the log line is only true if the mask took. diff --git a/kernel/src/arch/x86_64/idt/exceptions.rs b/kernel/src/arch/x86_64/idt/exceptions.rs index a5f95088012..6ec73802bc5 100644 --- a/kernel/src/arch/x86_64/idt/exceptions.rs +++ b/kernel/src/arch/x86_64/idt/exceptions.rs @@ -187,12 +187,15 @@ fn crash_report_exception(ctx: &ExceptionContext) { .flatten() { match super::super::pio::refused_port(access) { - port if port == access.port => { + Some(port) if port == access.port => { log!(" {access}, which this process holds no grant for") } - port => log!( + Some(port) => log!( " {access}, reaching port {port:#06x}, which this process holds no grant for" ), + // The decode named a span the bitmap opens whole: the + // fault is real, but blaming a port in it would be a guess. + None => log!(" {access}, which faulted at a port this decode cannot name"), } } } diff --git a/kernel/src/arch/x86_64/pio.rs b/kernel/src/arch/x86_64/pio.rs index e555b4f0334..bf030a40d08 100644 --- a/kernel/src/arch/x86_64/pio.rs +++ b/kernel/src/arch/x86_64/pio.rs @@ -81,12 +81,13 @@ pub fn switch_to(pid: Option) { } /// The first port of `access` this CPU refuses Ring 3, which is the port its -/// #GP faulted on; the process that faulted is still this CPU's. -pub fn refused_port(access: PortAccess) -> u16 { +/// #GP faulted on; the process that faulted is still this CPU's. `None` if +/// every port in the span is open: the decode named a port the bitmap does +/// not actually refuse, and the kill record must not guess one. +pub fn refused_port(access: PortAccess) -> Option { (0..u16::from(access.bytes)) .map(|i| access.port.wrapping_add(i)) .find(|&port| !super::percpu::port_open(port)) - .unwrap_or(access.port) } /// An `in` or `out` as decoded from the bytes at a faulting instruction. diff --git a/kernel/src/main.rs b/kernel/src/main.rs index 65b42ff73fb..f3ae2f51a84 100644 --- a/kernel/src/main.rs +++ b/kernel/src/main.rs @@ -137,7 +137,6 @@ fn panic(info: &core::panic::PanicInfo) -> ! { let bound = panic_reboot::arm(false); // No capture(): the outer panic's snapshot is the one worth showing. // render() is safe by construction here: a fault inside the renderer itself would find the fatal panel already claimed, and return without touching a pixel. - // Only the CPU that took the panel watches the bound; a reentry inside the pager finds it held and halts. if drivers::panic_console::render() { drivers::panic_console::hold_the_panel(bound); } diff --git a/tests/toyos.rs b/tests/toyos.rs index 2b17b88a6be..bae595915f0 100644 --- a/tests/toyos.rs +++ b/tests/toyos.rs @@ -9386,6 +9386,34 @@ fn metal_sim_client_death(boot: &mut Boot) -> Result<(), String> { Ok(()) } +/// The i8042 status register (port 0x64), read through the monitor rather +/// than a guest `in`: nothing here holds the claim once its process is gone, +/// and the byte sits in the controller whether or not its line is masked. +fn isa_status_byte(socket: &Path) -> Result { + let read = qemu::QmpMonitor::open(socket).human("i/1xb 0x64"); + read.split_whitespace() + .find_map(|tok| tok.strip_prefix("0x")) + .and_then(|hex| u8::from_str_radix(hex, 16).ok()) + .ok_or_else(|| format!("the monitor's `i/1xb 0x64` did not answer a byte: {read:?}")) +} + +/// Wait, bounded, for the i8042's output buffer to hold a byte: the key an +/// injected event carries reaches the controller on QEMU's own clock, not +/// the harness's, and the census below is only true once it has. +fn wait_for_obf(socket: &Path, ceiling: Duration) -> Result<(), String> { + const OBF: u8 = 1 << 0; + let deadline = Instant::now() + ceiling; + loop { + if isa_status_byte(socket)? & OBF != 0 { + return Ok(()); + } + if Instant::now() >= deadline { + return Err(format!("the i8042's output buffer never held a byte in {ceiling:?}")); + } + thread::sleep(Duration::from_millis(2)); + } +} + /// A `test_rs_isa_grant` role that ran to its end: no ceiling, exit 0, and the /// line it prints last. fn isa_verdict(result: &TestResult, last: &str) -> Result<(), String> { @@ -13726,9 +13754,13 @@ fn run_machine_test( // The claim went with `device`, which read the controller empty // first, so a key now raises line 1 again: masked, it reaches no // one, and unmasked it is a second first interrupt on a record the - // release cleared. `released` runs after the key has been sent. + // release cleared. `released` runs after the byte has reached the + // controller — OBF sets whether or not the line stayed masked, so + // waiting on it orders the census against QEMU's delivery instead + // of the host's scheduling of this harness. let socket = qemu.qmp_socket().to_path_buf(); qemu::qmp_send_keys(&socket, &[("a", true), ("a", false)]); + wait_for_obf(&socket, Duration::from_secs(5))?; let after = qemu.run_test("test_rs_isa_grant released", Duration::from_secs(30)); isa_verdict(&after, "isa released:")?; const FIRST: &str = "isa: the i8042 took its first interrupt"; From 494bfb00c1b922ab1017b9667c29df7fe51db518 Mon Sep 17 00:00:00 2001 From: japabu Date: Tue, 29 Sep 2026 13:09:22 +0200 Subject: [PATCH 07/10] PR #592 round 5: the i8042's flood is taken at most once per boot Round 4's `swap` on `QUARANTINE` stopped two CPUs from both taking one flood, but the ISR is the flag's other writer: an ISR still running on IRQ_CPU after the swap, or the edge the LAPIC already held in IRR when the mask landed (a flood re-edges on every 0x60 read), set it again. `service()` checks the flag before its `!ACTIVE` return, so the next pass ran `quarantine()` a second time and masked GSI 1 and 12 under a claim granted after the first `let_go`. The flag is now `flood::Flood`, a three-state word whose field only its own module can touch: the ISR raises it only from idle (`compare_exchange(IDLE, RAISED)`), `service()` moves it RAISED -> TAKEN and nothing moves it back, and `quarantine()` takes the `Taken` witness that only that move mints, so the quarantine runs at most once per boot by construction. `isa-claim-straddles-quarantine` now raises the flood once more after a granted claim, through `keyboard_controller::raise_flood`, standing in for an ISR in flight at the mask; `isa_claim_straddles_the_quarantine` must red on the fix's revert (`m11`) with no record of the keystroke. The AArch64 controller gets the empty `raise_flood` its `isa` needs to compile under `boot-actuators`. Review NOTEs: `wait_for_obf` holds one QMP monitor for its polls, and `isa_lines_reach_their_holder` asserts OBF clear before the key so only that key can satisfy the wait. The kill line for a #GP at a port the bitmap opens says the decode cannot attribute it to a port. Filed: the last i8042 ISR can read a byte the new holder owns, the switch's bitmap cost is unmeasured, and the bitmap has no silicon reading. The test-coverage narration above the take is deleted. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01U6SVYFkdvV2t38KzNrESxs --- ...ermission-bitmap-has-no-silicon-reading.md | 18 ++++++ ...isr-can-read-a-byte-its-new-holder-owns.md | 26 ++++++++ ...the-isa-port-switchs-cost-is-unmeasured.md | 18 ++++++ kernel/src/actuator.rs | 3 +- .../src/arch/aarch64/keyboard_controller.rs | 4 ++ kernel/src/arch/x86_64/i8042/mod.rs | 63 +++++++++++++++---- kernel/src/arch/x86_64/idt/exceptions.rs | 6 +- kernel/src/isa.rs | 5 +- tests/toyos.rs | 23 +++++-- 9 files changed, 143 insertions(+), 23 deletions(-) create mode 100644 issues/isolation/the-io-permission-bitmap-has-no-silicon-reading.md create mode 100644 issues/kernel/the-i8042s-last-isr-can-read-a-byte-its-new-holder-owns.md create mode 100644 issues/kernel/the-isa-port-switchs-cost-is-unmeasured.md diff --git a/issues/isolation/the-io-permission-bitmap-has-no-silicon-reading.md b/issues/isolation/the-io-permission-bitmap-has-no-silicon-reading.md new file mode 100644 index 00000000000..81483e2b9d6 --- /dev/null +++ b/issues/isolation/the-io-permission-bitmap-has-no-silicon-reading.md @@ -0,0 +1,18 @@ +--- +status: open +kind: tooling +opened: 2026-09-29 +--- + +# The I/O permission bitmap has no silicon reading + +What an `isa` claim opens, and every port it leaves closed, rests on the TSS I/O +permission bitmap and its limit (`kernel/src/arch/x86_64/pio.rs`, +`kernel/src/arch/x86_64/percpu.rs`). The guest tests that read it +(`isa_ports_are_the_binders_alone`, `isa_ports_close_on_one_cpu`) run locally +under TCG, whose `check_io` is QEMU's own implementation of the check; the +processor's reading comes only from KVM, which only `nightly.yml` runs, and from +metal, which has not run them. + +**Exit**: both tests green on a KVM run of the branch that carries the bitmap, +or on the T14. diff --git a/issues/kernel/the-i8042s-last-isr-can-read-a-byte-its-new-holder-owns.md b/issues/kernel/the-i8042s-last-isr-can-read-a-byte-its-new-holder-owns.md new file mode 100644 index 00000000000..929e69aa610 --- /dev/null +++ b/issues/kernel/the-i8042s-last-isr-can-read-a-byte-its-new-holder-owns.md @@ -0,0 +1,26 @@ +--- +status: open +kind: defect +opened: 2026-09-29 +--- + +# The i8042's last ISR can read a byte its new holder owns + +`i8042::quarantine` (`kernel/src/arch/x86_64/i8042/mod.rs`) masks GSI 1 and 12 +and then lets the controller go, and an `isa` claim may be granted from that +moment. The mask stops new edges at the I/O APIC, not an edge the local APIC +already holds in IRR for the driver's vector, nor a `handler` already running +on `IRQ_CPU`: a flood re-edges on every `0x60` read, so one such edge is the +usual case. That handler reads port 0x60 up to `ISR_BURST` times while OBF is +set, so a byte that arrives for the claim's holder before it runs goes into the +kernel's ring, which nothing drains any more, and the holder never sees it. +Under `i8042-fault` it reads sixteen bytes whatever OBF says. + +The flood itself can no longer run the quarantine twice (the flood is taken at +most once per boot); this is the handler's port read, which nothing orders +against the grant. No test drives a byte into that window. + +**Exit**: no read of port 0x60 by the kernel's handler can follow a granted +`isa` claim on the i8042 — the grant waits on the driver's vector being idle on +`IRQ_CPU`, or the handler refuses the port once the driver has let go — with a +guest test that stages the late handler and must see the holder's byte. diff --git a/issues/kernel/the-isa-port-switchs-cost-is-unmeasured.md b/issues/kernel/the-isa-port-switchs-cost-is-unmeasured.md new file mode 100644 index 00000000000..bc1b340d6ad --- /dev/null +++ b/issues/kernel/the-isa-port-switchs-cost-is-unmeasured.md @@ -0,0 +1,18 @@ +--- +status: open +kind: tooling +opened: 2026-09-29 +--- + +# The `isa` port switch's cost on the scheduler hot path is unmeasured + +`arch::pio::switch_to` (`kernel/src/arch/x86_64/pio.rs`) runs on every context +switch, from `KernelHw::switch`: per `GRANTABLE` row one `BOUND` load, one +bitmap byte read, and on a change one bitmap write per port. On x86-64 that is +one row on every switch of every CPU, whether or not any process holds a claim. +Nothing has measured what it adds to a switch, and a timing verdict comes only +from metal. + +**Exit**: the T14's switch cost with and without the call, from one metal run, +recorded in the commit that closes this; or the switch skips the rows entirely +while no row is bound, with the skip's cost measured the same way. diff --git a/kernel/src/actuator.rs b/kernel/src/actuator.rs index 48bbf4863c3..c3689ee0a5d 100644 --- a/kernel/src/actuator.rs +++ b/kernel/src/actuator.rs @@ -91,7 +91,8 @@ actuators! { /// Hold the i8042's quarantine between its two steps, in two scheduler /// passes, until an `isa` claim begun after the first has been answered: - /// the claim lands between them on every boot. + /// the claim lands between them on every boot. A granted claim then raises + /// the flood again, as an ISR still in flight at the mask would. isa_claim_straddles_quarantine = "isa-claim-straddles-quarantine"; /// Script the input core directly at end of boot. diff --git a/kernel/src/arch/aarch64/keyboard_controller.rs b/kernel/src/arch/aarch64/keyboard_controller.rs index cebd8f04265..5909303a309 100644 --- a/kernel/src/arch/aarch64/keyboard_controller.rs +++ b/kernel/src/arch/aarch64/keyboard_controller.rs @@ -11,3 +11,7 @@ pub fn service() {} /// Nothing to report. pub fn report_line() {} + +/// Nothing floods: no `isa` claim is ever granted here. +#[cfg(feature = "boot-actuators")] +pub fn raise_flood() {} diff --git a/kernel/src/arch/x86_64/i8042/mod.rs b/kernel/src/arch/x86_64/i8042/mod.rs index ca04fd4e653..b3cc0a385f5 100644 --- a/kernel/src/arch/x86_64/i8042/mod.rs +++ b/kernel/src/arch/x86_64/i8042/mod.rs @@ -65,7 +65,7 @@ const ISA_IRQ_AUX: u8 = 12; const ISR_BURST: usize = 16; static ACTIVE: AtomicBool = AtomicBool::new(false); -static QUARANTINE: AtomicBool = AtomicBool::new(false); +static QUARANTINE: flood::Flood = flood::Flood::new(); static KBD_EVENTS: AtomicU32 = AtomicU32::new(0); static AUX_EVENTS: AtomicU32 = AtomicU32::new(0); static LOST_EDGES: AtomicU32 = AtomicU32::new(0); @@ -108,6 +108,51 @@ pub fn drives() -> bool { ACTIVE.load(Ordering::Acquire) } +/// The flood that quarantines the driver: raised by [`handler`], taken by +/// [`service`], and taken at most once per boot, since nothing gives the +/// controller back once the quarantine has let it go. +mod flood { + use core::sync::atomic::{AtomicU8, Ordering}; + + const IDLE: u8 = 0; + const RAISED: u8 = 1; + const TAKEN: u8 = 2; + + pub struct Flood(AtomicU8); + + /// The boot's one quarantine; only [`Flood::take`] makes one. + pub struct Taken(()); + + impl Flood { + pub const fn new() -> Self { + Self(AtomicU8::new(IDLE)) + } + + /// Only from idle: an ISR still in flight when the quarantine masks + /// the lines raises nothing under a claim granted after it. + pub fn raise(&self) { + // Refused when already raised or taken, which is the point. + let _ = self.0.compare_exchange(IDLE, RAISED, Ordering::Relaxed, Ordering::Relaxed); + } + + /// The quarantine, to the one caller that moves the flood from raised + /// to taken. `Relaxed`: exclusivity is the read-modify-write's own. + pub fn take(&self) -> Option { + self.0 + .compare_exchange(RAISED, TAKEN, Ordering::Relaxed, Ordering::Relaxed) + .ok() + .map(|_| Taken(())) + } + } +} + +/// Under `isa-claim-straddles-quarantine`, the stand-in for an ISR still in +/// flight at the quarantine's mask: the flood raised as [`handler`] raises it. +#[cfg(feature = "boot-actuators")] +pub fn raise_flood() { + QUARANTINE.raise(); +} + fn is_irq_cpu() -> bool { IRQ_CPU.load(Ordering::Relaxed) == crate::arch::percpu::cpu_id() } @@ -517,7 +562,7 @@ pub extern "sysv64" fn handler() { } if n == ISR_BURST && buffer_full(inb(STATUS)) { // It cannot mask the line itself — that needs the I/O APIC lock. - QUARANTINE.store(true, Ordering::Relaxed); + QUARANTINE.raise(); } // Only the first interrupt can be the arming edge (IRR delivers it before // any later assertion), and it settles the debt either way. @@ -591,16 +636,8 @@ pub fn service() { // Unconditional and first: an undrained `irq_ring` record keeps // `any_pending_self` true, spinning a CPU that never halts. let recorded = crate::irq_ring::take(IrqSource::I8042).is_some(); - // `swap`, not `load`: two CPUs racing `service()` must not both see the - // flood and both run `quarantine()`. `Relaxed` only has to make the swap - // itself exclusive; `quarantine()`'s own writes (`ACTIVE`'s `Release`, - // the I/O APIC's mask) carry whatever ordering they separately need. No - // guest test forces this: the window this closes was a few instructions - // between one atomic load and a later, unsynchronized store, narrower - // than anything a scheduler-pass-granularity actuator can straddle - // without adding a permanent rendezvous to every pass's hottest path. - if QUARANTINE.swap(false, Ordering::Relaxed) { - quarantine(); + if let Some(taken) = QUARANTINE.take() { + quarantine(taken); return; } #[cfg(feature = "boot-actuators")] @@ -797,7 +834,7 @@ fn drain() -> Drained { /// **Masked before the driver lets go**: `isa::claim` refuses only while /// [`ACTIVE`] holds, so a claim that lands once it is clear routes and unmasks /// lines nothing here touches again. -fn quarantine() { +fn quarantine(_: flood::Taken) { // The pin is about to be masked, so no health verdict follows this line. HEALTH.store(HEALTH_DONE, Ordering::Relaxed); // The count, not the intent: the log line is only true if the mask took. diff --git a/kernel/src/arch/x86_64/idt/exceptions.rs b/kernel/src/arch/x86_64/idt/exceptions.rs index 6ec73802bc5..67573e7c254 100644 --- a/kernel/src/arch/x86_64/idt/exceptions.rs +++ b/kernel/src/arch/x86_64/idt/exceptions.rs @@ -193,9 +193,9 @@ fn crash_report_exception(ctx: &ExceptionContext) { Some(port) => log!( " {access}, reaching port {port:#06x}, which this process holds no grant for" ), - // The decode named a span the bitmap opens whole: the - // fault is real, but blaming a port in it would be a guess. - None => log!(" {access}, which faulted at a port this decode cannot name"), + // The bitmap opens the span whole, so the #GP is not + // the port's: a string form's non-canonical `rsi`/`rdi`, say. + None => log!(" {access}, a #GP this decode cannot attribute to a port"), } } } diff --git a/kernel/src/isa.rs b/kernel/src/isa.rs index 971ff89a27f..96f427856ae 100644 --- a/kernel/src/isa.rs +++ b/kernel/src/isa.rs @@ -79,6 +79,9 @@ pub fn claim(set: IsaId) -> Result { let begun = straddle::begin(); let answer = mint(set); straddle::answered(begun); + if answer.is_ok() { + crate::arch::keyboard_controller::raise_flood(); + } return answer; } mint(set) @@ -192,7 +195,7 @@ pub fn watch(row: usize) -> &'static Watch { /// The `isa-claim-straddles-quarantine` actuator: a claim answered between the /// two steps of the i8042's quarantine, which holds after its first until one -/// begun after it has been. +/// begun after it has been; a granted one then raises the flood again. #[cfg(feature = "boot-actuators")] pub mod straddle { use core::sync::atomic::{AtomicBool, AtomicU32, AtomicU64, Ordering}; diff --git a/tests/toyos.rs b/tests/toyos.rs index bae595915f0..3ea7a0516f7 100644 --- a/tests/toyos.rs +++ b/tests/toyos.rs @@ -9389,22 +9389,25 @@ fn metal_sim_client_death(boot: &mut Boot) -> Result<(), String> { /// The i8042 status register (port 0x64), read through the monitor rather /// than a guest `in`: nothing here holds the claim once its process is gone, /// and the byte sits in the controller whether or not its line is masked. -fn isa_status_byte(socket: &Path) -> Result { - let read = qemu::QmpMonitor::open(socket).human("i/1xb 0x64"); +fn isa_status_byte(monitor: &mut qemu::QmpMonitor) -> Result { + let read = monitor.human("i/1xb 0x64"); read.split_whitespace() .find_map(|tok| tok.strip_prefix("0x")) .and_then(|hex| u8::from_str_radix(hex, 16).ok()) .ok_or_else(|| format!("the monitor's `i/1xb 0x64` did not answer a byte: {read:?}")) } +/// The i8042 status register's output-buffer-full bit. +const I8042_OBF: u8 = 1 << 0; + /// Wait, bounded, for the i8042's output buffer to hold a byte: the key an /// injected event carries reaches the controller on QEMU's own clock, not /// the harness's, and the census below is only true once it has. fn wait_for_obf(socket: &Path, ceiling: Duration) -> Result<(), String> { - const OBF: u8 = 1 << 0; + let mut monitor = qemu::QmpMonitor::open(socket); let deadline = Instant::now() + ceiling; loop { - if isa_status_byte(socket)? & OBF != 0 { + if isa_status_byte(&mut monitor)? & I8042_OBF != 0 { return Ok(()); } if Instant::now() >= deadline { @@ -13759,6 +13762,15 @@ fn run_machine_test( // waiting on it orders the census against QEMU's delivery instead // of the host's scheduling of this harness. let socket = qemu.qmp_socket().to_path_buf(); + // Empty first, so only this key can satisfy the wait. The monitor + // closes before the keys go: the socket serves one connection. + let status = isa_status_byte(&mut qemu::QmpMonitor::open(&socket))?; + if status & I8042_OBF != 0 { + return Err(format!( + "the i8042 held a byte before the key (status {status:#04x}): the wait \ + below could not tell the key's from it" + )); + } qemu::qmp_send_keys(&socket, &[("a", true), ("a", false)]); wait_for_obf(&socket, Duration::from_secs(5))?; let after = qemu.run_test("test_rs_isa_grant released", Duration::from_secs(30)); @@ -13783,7 +13795,8 @@ fn run_machine_test( // `isa-claim-straddles-quarantine` holds the quarantine between its // two steps until a claim has been answered: refused while the // driver holds the controller, and granted after, with lines the - // quarantine no longer touches. + // quarantine no longer touches even when the grant raises the + // flood again, as an ISR in flight at the mask would. let mut qemu = QemuInstance::boot_with_options( test_config, c_bins, From 0d537d75b4eddf506f269a55a25716ddb1bf1b36 Mon Sep 17 00:00:00 2001 From: japabu Date: Tue, 29 Sep 2026 15:10:48 +0200 Subject: [PATCH 08/10] PR #592 round 6: the i8042's quarantine is taken and let go on IRQ_CPU alone Round 6's BLOCKER: `service()` took the flood on whichever CPU passed first, and under `isa-claim-straddles-quarantine` resumed it there too (the m11 log shows `let_go` on cpu1 while IRQ_CPU is cpu0). IRQ_CPU could read ACTIVE true while another CPU sat between the mask and `let_go`, then spend up to 30 ms in `aux_reenable` with interrupts closed: commands to 0x64/0x60, up to 16 reads of 0x60, and on its third failure a mask of GSI 12, all after a claim on a third CPU had been granted and had unmasked the lines. The flood is now taken, and the straddle's second step resumed, only under a `Pinned`: a witness that closes interrupts itself and asserts it is on IRQ_CPU, owns its `IrqGuard` (so it is `!Send`), and is what `Flood::take` and `let_go` require. `service()` mints one only when `is_irq_cpu()`. So every kernel access to the controller either precedes the let-go in IRQ_CPU's program order or does not happen: - `aux_reenable` and the split rescue run only on IRQ_CPU, after a Relaxed read of ACTIVE that IRQ_CPU itself cleared in `let_go`. - `handler` runs only on IRQ_CPU (the vector is routed to it) and now reads no port once the flood is taken. A handler already executing when the flood is raised cannot overlap the take, which runs on the same CPU with interrupts closed, so it finished before the let-go; a handler whose edge is held in IRR is delivered after the take's guard drops and reads TAKEN. That covers both late handlers the issue counted as one and the `aux_reenable` path it missed, so issues/kernel/the-i8042s-last-isr-can-read-a-byte-its-new-holder-owns.md is deleted; nothing else cited it. Because the second step now runs only on IRQ_CPU, the claim that straddles the quarantine kicks IRQ_CPU (`wake_irq_cpu`): a halted CPU has stopped its timer and would otherwise reach no pass to resume on. The straddle actuator's `resume` is a one-shot: one word moves WAITING -> STRADDLED -> RESUMED and nothing moves it back, where a refused claim answered between the old swap and the store of NONE re-armed it and ran the second step twice. `isa_claim_straddles_the_quarantine` asserts each of the hold, resume and quarantine lines appears exactly once, before the device verdict, so m11 reds on the count. Drops the flood's `raise` comment, which restated its doc. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01U6SVYFkdvV2t38KzNrESxs --- ...isr-can-read-a-byte-its-new-holder-owns.md | 26 ------ .../src/arch/aarch64/keyboard_controller.rs | 4 + kernel/src/arch/x86_64/i8042/mod.rs | 83 +++++++++++++++---- kernel/src/isa.rs | 29 ++++--- tests/toyos.rs | 8 +- 5 files changed, 93 insertions(+), 57 deletions(-) delete mode 100644 issues/kernel/the-i8042s-last-isr-can-read-a-byte-its-new-holder-owns.md diff --git a/issues/kernel/the-i8042s-last-isr-can-read-a-byte-its-new-holder-owns.md b/issues/kernel/the-i8042s-last-isr-can-read-a-byte-its-new-holder-owns.md deleted file mode 100644 index 929e69aa610..00000000000 --- a/issues/kernel/the-i8042s-last-isr-can-read-a-byte-its-new-holder-owns.md +++ /dev/null @@ -1,26 +0,0 @@ ---- -status: open -kind: defect -opened: 2026-09-29 ---- - -# The i8042's last ISR can read a byte its new holder owns - -`i8042::quarantine` (`kernel/src/arch/x86_64/i8042/mod.rs`) masks GSI 1 and 12 -and then lets the controller go, and an `isa` claim may be granted from that -moment. The mask stops new edges at the I/O APIC, not an edge the local APIC -already holds in IRR for the driver's vector, nor a `handler` already running -on `IRQ_CPU`: a flood re-edges on every `0x60` read, so one such edge is the -usual case. That handler reads port 0x60 up to `ISR_BURST` times while OBF is -set, so a byte that arrives for the claim's holder before it runs goes into the -kernel's ring, which nothing drains any more, and the holder never sees it. -Under `i8042-fault` it reads sixteen bytes whatever OBF says. - -The flood itself can no longer run the quarantine twice (the flood is taken at -most once per boot); this is the handler's port read, which nothing orders -against the grant. No test drives a byte into that window. - -**Exit**: no read of port 0x60 by the kernel's handler can follow a granted -`isa` claim on the i8042 — the grant waits on the driver's vector being idle on -`IRQ_CPU`, or the handler refuses the port once the driver has let go — with a -guest test that stages the late handler and must see the holder's byte. diff --git a/kernel/src/arch/aarch64/keyboard_controller.rs b/kernel/src/arch/aarch64/keyboard_controller.rs index 5909303a309..371434223c4 100644 --- a/kernel/src/arch/aarch64/keyboard_controller.rs +++ b/kernel/src/arch/aarch64/keyboard_controller.rs @@ -15,3 +15,7 @@ pub fn report_line() {} /// Nothing floods: no `isa` claim is ever granted here. #[cfg(feature = "boot-actuators")] pub fn raise_flood() {} + +/// Nothing to wake: no quarantine runs here. +#[cfg(feature = "boot-actuators")] +pub fn wake_irq_cpu() {} diff --git a/kernel/src/arch/x86_64/i8042/mod.rs b/kernel/src/arch/x86_64/i8042/mod.rs index b3cc0a385f5..af589fdb204 100644 --- a/kernel/src/arch/x86_64/i8042/mod.rs +++ b/kernel/src/arch/x86_64/i8042/mod.rs @@ -114,6 +114,8 @@ pub fn drives() -> bool { mod flood { use core::sync::atomic::{AtomicU8, Ordering}; + use super::Pinned; + const IDLE: u8 = 0; const RAISED: u8 = 1; const TAKEN: u8 = 2; @@ -131,18 +133,42 @@ mod flood { /// Only from idle: an ISR still in flight when the quarantine masks /// the lines raises nothing under a claim granted after it. pub fn raise(&self) { - // Refused when already raised or taken, which is the point. let _ = self.0.compare_exchange(IDLE, RAISED, Ordering::Relaxed, Ordering::Relaxed); } /// The quarantine, to the one caller that moves the flood from raised /// to taken. `Relaxed`: exclusivity is the read-modify-write's own. - pub fn take(&self) -> Option { + pub fn take(&self, _: &Pinned) -> Option { self.0 .compare_exchange(RAISED, TAKEN, Ordering::Relaxed, Ordering::Relaxed) .ok() .map(|_| Taken(())) } + + /// Exact on `IRQ_CPU`, the one CPU that takes it. + pub fn taken(&self) -> bool { + self.0.load(Ordering::Relaxed) == TAKEN + } + } +} + +/// Interrupts closed on [`IRQ_CPU`]: the vector's CPU, and the only one that +/// drives the controller by polled I/O. The quarantine runs under one, so every +/// port access the kernel makes precedes its let-go in one CPU's program order. +struct Pinned { + _irq: crate::arch::IrqGuard, +} + +impl Pinned { + fn close() -> Self { + let irq = crate::arch::IrqGuard::close(); + assert!( + is_irq_cpu(), + "i8042: cpu {} would quarantine the controller cpu {} drives", + crate::arch::percpu::cpu_id(), + IRQ_CPU.load(Ordering::Relaxed) + ); + Self { _irq: irq } } } @@ -153,6 +179,16 @@ pub fn raise_flood() { QUARANTINE.raise(); } +/// Under `isa-claim-straddles-quarantine`, a pass owed on [`IRQ_CPU`], where +/// alone the quarantine's second step runs; a halted CPU has stopped its timer. +#[cfg(feature = "boot-actuators")] +pub fn wake_irq_cpu() { + let cpu = IRQ_CPU.load(Ordering::Relaxed); + if cpu != crate::arch::percpu::cpu_id() { + crate::arch::irqchip::kick_cpu(cpu); + } +} + fn is_irq_cpu() -> bool { IRQ_CPU.load(Ordering::Relaxed) == crate::arch::percpu::cpu_id() } @@ -539,6 +575,12 @@ fn buffer_full(status: u8) -> bool { /// anything to it. pub extern "sysv64" fn handler() { crate::arch::percpu::irq_took!(I8042); + // A late edge, executed or held in IRR, on a controller the quarantine + // may have handed on: the take ran here with interrupts closed. + if QUARANTINE.taken() { + crate::arch::apic::eoi(); + return; + } let timestamp = crate::clock::nanos_since_boot(); // No compare-exchange: this handler cannot nest, so there's no second writer. let first = FIRST_IRQ_NS.load(Ordering::Relaxed) == 0; @@ -636,17 +678,10 @@ pub fn service() { // Unconditional and first: an undrained `irq_ring` record keeps // `any_pending_self` true, spinning a CPU that never halts. let recorded = crate::irq_ring::take(IrqSource::I8042).is_some(); - if let Some(taken) = QUARANTINE.take() { - quarantine(taken); + if is_irq_cpu() && quarantine_step(&Pinned::close()) { return; } - #[cfg(feature = "boot-actuators")] - if crate::actuator::isa_claim_straddles_quarantine() { - if let Some(masked) = crate::isa::straddle::resume() { - let_go(masked); - return; - } - } + // `IRQ_CPU` cleared it itself, so none of its polled I/O below follows the let-go. if !ACTIVE.load(Ordering::Relaxed) { return; } @@ -828,13 +863,31 @@ fn drain() -> Drained { out } +/// The quarantine's step that is due, if any; `true` when one ran. +fn quarantine_step(pinned: &Pinned) -> bool { + if let Some(taken) = QUARANTINE.take(pinned) { + quarantine(taken, pinned); + return true; + } + #[cfg(feature = "boot-actuators")] + if crate::actuator::isa_claim_straddles_quarantine() { + if let Some(masked) = crate::isa::straddle::resume() { + let_go(masked, pinned); + return true; + } + } + false +} + /// A controller producing bytes faster than the ISR's bound can drain them. /// One masked line and a dead keyboard, never a spinning CPU. /// /// **Masked before the driver lets go**: `isa::claim` refuses only while /// [`ACTIVE`] holds, so a claim that lands once it is clear routes and unmasks -/// lines nothing here touches again. -fn quarantine(_: flood::Taken) { +/// lines nothing here touches again. **And let go last on [`IRQ_CPU`]**: the +/// handler reads no port once the flood is taken, and the aux re-enable reads +/// [`ACTIVE`] there, so no kernel access to the controller follows a grant. +fn quarantine(_: flood::Taken, pinned: &Pinned) { // The pin is about to be masked, so no health verdict follows this line. HEALTH.store(HEALTH_DONE, Ordering::Relaxed); // The count, not the intent: the log line is only true if the mask took. @@ -849,11 +902,11 @@ fn quarantine(_: flood::Taken) { crate::isa::straddle::hold(masked); return; } - let_go(masked); + let_go(masked, pinned); } /// The quarantine's second step, once `masked` of the lines are down. -fn let_go(masked: u32) { +fn let_go(masked: u32, _: &Pinned) { // `Release`: a claim that reads the driver gone finds the lines masked. ACTIVE.store(false, Ordering::Release); // Force-released: nothing else can lift a held key or pointer button diff --git a/kernel/src/isa.rs b/kernel/src/isa.rs index 96f427856ae..50aa2234747 100644 --- a/kernel/src/isa.rs +++ b/kernel/src/isa.rs @@ -198,17 +198,21 @@ pub fn watch(row: usize) -> &'static Watch { /// begun after it has been; a granted one then raises the flood again. #[cfg(feature = "boot-actuators")] pub mod straddle { - use core::sync::atomic::{AtomicBool, AtomicU32, AtomicU64, Ordering}; + use core::sync::atomic::{AtomicU32, AtomicU64, AtomicU8, Ordering}; /// Claims begun this boot. static BEGUN: AtomicU64 = AtomicU64::new(0); - /// [`BEGUN`] when the quarantine's first step ran; [`NONE`] when it holds none. + /// [`BEGUN`] when the quarantine's first step ran; [`NONE`] before it. static HELD_FROM: AtomicU64 = AtomicU64::new(NONE); const NONE: u64 = u64::MAX; /// The lines the first step masked, for the second's log. static MASKED: AtomicU32 = AtomicU32::new(0); - /// A claim begun after the first step has been answered. - static STRADDLED: AtomicBool = AtomicBool::new(false); + /// `WAITING` → `STRADDLED` → `RESUMED`, and nothing moves it back: the + /// second step runs once. + static STEP: AtomicU8 = AtomicU8::new(WAITING); + const WAITING: u8 = 0; + const STRADDLED: u8 = 1; + const RESUMED: u8 = 2; pub(super) fn begin() -> u64 { BEGUN.fetch_add(1, Ordering::SeqCst) @@ -218,26 +222,25 @@ pub mod straddle { /// either way, so only a later one decides anything. pub(super) fn answered(begun: u64) { let from = HELD_FROM.load(Ordering::SeqCst); - if from != NONE && begun >= from { - STRADDLED.store(true, Ordering::SeqCst); + if from != NONE + && begun >= from + && STEP.compare_exchange(WAITING, STRADDLED, Ordering::SeqCst, Ordering::SeqCst).is_ok() + { + crate::arch::keyboard_controller::wake_irq_cpu(); } } /// The quarantine's first step ran and masked `masked` lines. pub fn hold(masked: u32) { MASKED.store(masked, Ordering::SeqCst); - STRADDLED.store(false, Ordering::SeqCst); HELD_FROM.store(BEGUN.load(Ordering::SeqCst), Ordering::SeqCst); log!("isa: the i8042's quarantine holds after its first step for a claim"); } - /// The first step's masked count, once a claim begun after it has been - /// answered; the second step is the caller's. + /// The first step's masked count, once, after a claim begun after it has + /// been answered; the second step is the caller's. pub fn resume() -> Option { - if !STRADDLED.swap(false, Ordering::SeqCst) { - return None; - } - HELD_FROM.store(NONE, Ordering::SeqCst); + STEP.compare_exchange(STRADDLED, RESUMED, Ordering::SeqCst, Ordering::SeqCst).ok()?; log!("isa: a claim was answered between the i8042's quarantine steps"); Some(MASKED.load(Ordering::SeqCst)) } diff --git a/tests/toyos.rs b/tests/toyos.rs index 1d6429b33d7..807322ec569 100644 --- a/tests/toyos.rs +++ b/tests/toyos.rs @@ -13850,16 +13850,18 @@ fn run_machine_test( } }, ); - isa_verdict(&result, "===ISA_DEVICE_OK===")?; + // Each once: the flood is taken, and the quarantine lets go, once per boot. for want in [ "isa: the i8042's quarantine holds after its first step for a claim", "isa: a claim was answered between the i8042's quarantine steps", "i8042: quarantined", ] { - if !result.serial.contains(want) { - return Err(format!("the kernel never said {want:?}:\n{}", result.serial)); + let said = result.serial.matches(want).count(); + if said != 1 { + return Err(format!("the kernel said {want:?} {said} times:\n{}", result.serial)); } } + isa_verdict(&result, "===ISA_DEVICE_OK===")?; eprintln!(" [isa] {}", result.stdout.trim().replace('\n', "\n [isa] ")); Ok(()) } From cdc0ec19358f8b907107632609a02e982b6feb94 Mon Sep 17 00:00:00 2001 From: japabu Date: Tue, 29 Sep 2026 15:53:23 +0200 Subject: [PATCH 09/10] PR #592 round 7: a late i8042 edge is staged under the holder's byte Under `isa-claim-straddles-quarantine`, the holder's own interrupt (`isa0_handler`) sends the i8042 vector to itself on `IRQ_CPU`, so the kernel's handler runs at that interrupt's return with the holder's byte still in OBF: the edge a flood leaves in IRR, delivered after the grant. The handler's early return once the flood is taken is what keeps it off port 0x60, and `isa_claim_straddles_the_quarantine` is now the test that reds without it. The handler comment's "executed or" is gone: a handler already executing never reaches that check; only an edge held in IRR does. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01U6SVYFkdvV2t38KzNrESxs --- kernel/src/arch/x86_64/i8042/mod.rs | 10 +++++++++- kernel/src/arch/x86_64/idt/isa.rs | 4 ++++ 2 files changed, 13 insertions(+), 1 deletion(-) diff --git a/kernel/src/arch/x86_64/i8042/mod.rs b/kernel/src/arch/x86_64/i8042/mod.rs index af589fdb204..ab3822198f9 100644 --- a/kernel/src/arch/x86_64/i8042/mod.rs +++ b/kernel/src/arch/x86_64/i8042/mod.rs @@ -189,6 +189,14 @@ pub fn wake_irq_cpu() { } } +/// Under `isa-claim-straddles-quarantine`, the edge a flood leaves in IRR, +/// delivered with the holder's byte in OBF. +#[cfg(feature = "boot-actuators")] +pub fn stage_late_edge() { + assert!(is_irq_cpu(), "i8042: the holder's line is not on IRQ_CPU"); + crate::arch::apic::send_self(I8042_VECTOR); +} + fn is_irq_cpu() -> bool { IRQ_CPU.load(Ordering::Relaxed) == crate::arch::percpu::cpu_id() } @@ -575,7 +583,7 @@ fn buffer_full(status: u8) -> bool { /// anything to it. pub extern "sysv64" fn handler() { crate::arch::percpu::irq_took!(I8042); - // A late edge, executed or held in IRR, on a controller the quarantine + // A late edge, held in IRR, on a controller the quarantine // may have handed on: the take ran here with interrupts closed. if QUARANTINE.taken() { crate::arch::apic::eoi(); diff --git a/kernel/src/arch/x86_64/idt/isa.rs b/kernel/src/arch/x86_64/idt/isa.rs index f2980a231a5..407fce41c5d 100644 --- a/kernel/src/arch/x86_64/idt/isa.rs +++ b/kernel/src/arch/x86_64/idt/isa.rs @@ -7,6 +7,10 @@ use crate::irq_ring::IrqSource; extern "sysv64" fn isa0_handler() { crate::arch::percpu::irq_took!(UserDev); crate::isa::isr(0); + #[cfg(feature = "boot-actuators")] + if crate::actuator::isa_claim_straddles_quarantine() { + crate::arch::i8042::stage_late_edge(); + } crate::irq_ring::isr_publish(IrqSource::UserDev, crate::clock::nanos_since_boot()); crate::preempt::set_need_resched(); crate::arch::apic::eoi(); From 34ef28c8d76a6a3ece79497097bab6411acc318c Mon Sep 17 00:00:00 2001 From: japabu Date: Tue, 29 Sep 2026 16:07:07 +0200 Subject: [PATCH 10/10] i8042: the quarantine's ordering claims name the accesses that drive the controller `report_line`, under the `heartbeat` actuator, reads the status port on any CPU that reads `ACTIVE` true, which can follow the let-go; it stays, since `kernel_heartbeat` needs its line beside every beat. `Pinned` and `quarantine` now claim only what holds: every access that drives the controller precedes the let-go, and a status read may follow it. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01U6SVYFkdvV2t38KzNrESxs --- kernel/src/arch/x86_64/i8042/mod.rs | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/kernel/src/arch/x86_64/i8042/mod.rs b/kernel/src/arch/x86_64/i8042/mod.rs index ab3822198f9..2e49a2cd3d2 100644 --- a/kernel/src/arch/x86_64/i8042/mod.rs +++ b/kernel/src/arch/x86_64/i8042/mod.rs @@ -154,7 +154,8 @@ mod flood { /// Interrupts closed on [`IRQ_CPU`]: the vector's CPU, and the only one that /// drives the controller by polled I/O. The quarantine runs under one, so every -/// port access the kernel makes precedes its let-go in one CPU's program order. +/// port access that drives the controller precedes its let-go in one CPU's +/// program order; a status read may follow it. struct Pinned { _irq: crate::arch::IrqGuard, } @@ -894,7 +895,8 @@ fn quarantine_step(pinned: &Pinned) -> bool { /// [`ACTIVE`] holds, so a claim that lands once it is clear routes and unmasks /// lines nothing here touches again. **And let go last on [`IRQ_CPU`]**: the /// handler reads no port once the flood is taken, and the aux re-enable reads -/// [`ACTIVE`] there, so no kernel access to the controller follows a grant. +/// [`ACTIVE`] there, so no kernel access that drives the controller follows a +/// grant. fn quarantine(_: flood::Taken, pinned: &Pinned) { // The pin is about to be masked, so no health verdict follows this line. HEALTH.store(HEALTH_DONE, Ordering::Relaxed);