diff --git a/build/ci.rs b/build/ci.rs index 206b42276c8..32a8297bd99 100644 --- a/build/ci.rs +++ b/build/ci.rs @@ -606,6 +606,9 @@ pub(crate) const CONTROLS: &[Control] = &[ red(KERNEL_LOOM, "device-irq-lossy", Some("device_irq"), &[ Fails("every_message_is_counted_once"), ]), + red(KERNEL_LOOM, "device-irq-gate-intent", Some("device_irq_gate"), &[ + Fails("hardware_ends_as_the_record_owes_after_two_messages"), + ]), Control { krate: KERNEL_LOOM, feature: "no-preempt-guard", diff --git a/issues/a-claimed-function-can-storm-its-cpu.md b/issues/a-claimed-function-can-storm-its-cpu.md index 91a7e744bfd..e153145f22f 100644 --- a/issues/a-claimed-function-can-storm-its-cpu.md +++ b/issues/a-claimed-function-can-storm-its-cpu.md @@ -7,28 +7,31 @@ opened: 2026-10-04 # A claimed function can storm its CPU -A function a process drives raises its claim's interrupt on cpu0 for -every message it sends — on x86-64 the claim's vector -(`kernel/src/arch/x86_64/idt/user_dev.rs`), on AArch64 the function's LPI through -the ITS (`kernel/src/arch/aarch64/irqchip/its.rs`) — and `pcidev::isr` -(`kernel/src/pcidev/mod.rs`) takes each one; nothing bounds how often. Its -holder programs the device through the BARs it maps, so a hostile or broken -holder can make the device send messages as fast as the bus carries -them, and each one is an interrupt cpu0 takes before anything else it runs. -The kernel's own xHCI measured what that does to a CPU for a kernel -driver's source, ten timer interrupts in thirty-four seconds on T14 run 20, -before that driver left the kernel; a claim is the same source with its -holder outside the kernel, which every xHCI controller now is. +A claim's message is masked from the second message that lands before its +holder reads the record until that read (`kernel/src/pcidev/record.rs`), so +what lands between two reads stops at what arrives before the mask takes. +That bounds it per read and not per unit of time, and three things remain. -The lever is the claim's own translation: on x86-64 its remapping entry, which -made not present stops the function's messages at the unit; on AArch64 its -LPI's configuration byte, which cleared and followed by an `INV` disables -the LPI at the redistributor. Masking through the device would need the -holder's cooperation. What rate is a storm is not decided, and a number -chosen without a measurement behind it is the silent decision this file -exists to refuse. +**A holder paces cpu0.** A holder that loops on forcing a cause through its +own registers and reading its record, on any CPU, still raises one cpu0 +interrupt per iteration, charged to whatever cpu0 interrupted and not to the +holder. One that forces two per read also costs cpu0 an invalidation +completion wait with interrupts off at the mask (VT-d's interrupt-entry-cache +invalidation, AMD-Vi's completion wait, or the ITS's `SYNC`); the read's own +wait runs in the holder's call, and reaches cpu0 only as a spin on the unit's +queue behind it. -Owner: the IOMMU track. Exit: a claimed function's messages past a bound the -kernel derives and states are stopped at its entry and recorded against its -claim, and a T14 row with a holder that provokes them shows cpu0's timer and -other sources still served. +**A refused message is recorded nowhere.** While masked, a message is target +aborted (AMD-Vi) or blocked with fault processing disabled (VT-d), and +neither the unit nor the claim's record counts it. On AArch64 a disabled LPI +stays pending and folds into one. + +**No T14 row shows cpu0's other sources served under a holder that +provokes its function.** `i219_msi`'s storm phase shows a forced storm on +an unread claim stopped at the entry, and not the timer. + +Owner: the IOMMU track. Exit: cpu0's interrupts from a claim are bounded in +time, and the work they cost is charged to their holder, by a bound the +kernel derives and states; a message refused past it is counted against +the claim; and a T14 row with a holder that loops force-and-read shows +cpu0's timer and other sources still served. diff --git a/issues/a-read-into-a-claims-bar-is-not-refused-on-aarch64.md b/issues/a-read-into-a-claims-bar-is-not-refused-on-aarch64.md new file mode 100644 index 00000000000..ed489f705aa --- /dev/null +++ b/issues/a-read-into-a-claims-bar-is-not-refused-on-aarch64.md @@ -0,0 +1,27 @@ +--- +status: open +kind: defect +opened: 2026-11-06 +--- + +# A read into a claim's BAR is not refused on AArch64 + +`virt_claim_lpi`'s `claim_bar_fault` job (`tests/guest/rust-tests/src/bin/claim_bar_fault.rs`) +reads a pipe into a slice that points at its claim's mapped BAR registers and +expects `Err(BadAddress)`. On main at 0b2dec674 the read answered `Ok(16)`, +and the job ended at line 100: "a read into the BAR was not refused +BadAddress", left `Ok(16)`, right `Err(BadAddress)`. + +Seen on the guest runner's aarch64 row, both at main's own commit (run +42124293522, branch `storm-main-check`) and at #1193's rebased head (run +42120673220); the same row passed at #1193's heads before the rebase, on +#1188's base (runs 42035960983, 42077063953). Between the two bases main +took, among others, #1150, which routes every user copy through the process's +own mapping. Which change made the read succeed, and whether the kernel or +the test's expectation is now wrong, is not established. + +Evidence: https://github.com/ToyOSOrg/evidence/tree/05c9ce10c698b6618a8ac9c5f38f41fe1e2451a0/pr-1193/r4/logs + +Owner: the memory track (S0-a). Exit: a read whose destination is a claim's +BAR is refused as the kernel's copy contract states, or the contract and the +test say otherwise, and `virt_claim_lpi` passes on main. diff --git a/issues/the-intel-nics-room-and-wake-are-unread-on-hardware.md b/issues/the-intel-nics-room-and-wake-are-unread-on-hardware.md index 86660fb6fc1..72b44f94130 100644 --- a/issues/the-intel-nics-room-and-wake-are-unread-on-hardware.md +++ b/issues/the-intel-nics-room-and-wake-are-unread-on-hardware.md @@ -35,6 +35,18 @@ write-back policy this path's. What either part does with its ring across a link change is not in this exit: `issues/no-machine-has-read-an-intel-nic-across-a-link-change.md`. +**Sighting, 2026-11-05: `i219_msi`'s burst does not fill the T14's ring +either.** Its traffic phase queues `4 * TX_RING` frames back to back and +then requires `tx_full` and a taken wake, and on the bench's link the wire +sends them faster than the driver queues them: "the ring full 0 time(s), 0 +wake(s) asked and 0 taken, 0 stranded; the MAC counted 65 sent", both on +#1188's T14 run, where the job failed first on its receive check +(https://github.com/ToyOSOrg/evidence/blob/72d7fa82557c6680fb33008665e717ddc83cb849/pr-1188/r3/t14-kernel-excerpt.log, +line 43), and on #1193's B arm, where it is the job's failure +(https://github.com/ToyOSOrg/evidence/tree/3bfbd7de150de5686df1edc749efc33b07722354/pr-1193/r1). +That phase's `tx_full` requirement is this exit's premise and the burst does +not establish it. + ## Owner The T14's outbound rows of `issues/toyos-has-its-own-network-stack.md`: the diff --git a/issues/the-sysroot-pin-test-refused-a-move-it-expects-to-make.md b/issues/the-sysroot-pin-test-refused-a-move-it-expects-to-make.md new file mode 100644 index 00000000000..59fd0de51a7 --- /dev/null +++ b/issues/the-sysroot-pin-test-refused-a-move-it-expects-to-make.md @@ -0,0 +1,26 @@ +--- +status: open +kind: tooling +opened: 2026-11-05 +--- + +# The sysroot pin test refused a move it expects to make, once + +`sysroot::tests::the_primary_s_fork_checkout_is_moved_to_its_pin` +(`build/sysroot.rs`) redded once in a `cargo run -- --ci host` run at +ce5d8469d, on a host other builds were loading, and passed run alone at the +same revision. + +What the log shows: after three moves of the test's own fork checkout +(0a69 to 3a47, back, and to 3a47 again), the fourth, the test's `pin(&c1)` +before its deliberate edit, refused with "is at 3a47… with uncommitted work, +and this tree pins the fork at 0a69…", naming one entry, +`1 .M N... 100644 100644 100644 82a3da18… 82a3da18… library/std/src/lib.rs`. +The test had written nothing to that file before the refusal. Why `git +status` reported it modified is not established. + +Evidence: #1193's host CI log at ce5d8469d and the test alone passing there: +https://github.com/ToyOSOrg/evidence/tree/38c6ab9b31d7e7eff1b3ffc9be1180c3130f29c7/pr-1193/r3/logs + +Owner: the build track. Exit: the cause found and fixed, or the test shown to +hold under the load it redded in. diff --git a/kernel/Cargo.toml b/kernel/Cargo.toml index 20fd3581617..caa3327f8da 100644 --- a/kernel/Cargo.toml +++ b/kernel/Cargo.toml @@ -62,6 +62,10 @@ shard-publish-relaxed = [] # `device_irq` reds. They *are* the record's design, so turning them off reverts # the whole of what the model claims. device-irq-lossy = [] +# `src/pcidev/record.rs`'s gate follows each side's intent instead of the +# record, so a mask landing after the read that opened it strands the claim, and +# `device_irq_gate` reds. +device-irq-gate-intent = [] # The same arrangement for the CPU roster's publication edge: `src/smp_roster.rs`'s # `commit` publishes the count with a `Relaxed` store instead of the release the # slot store needs, so a reader can see a count over an unfilled slot, and diff --git a/kernel/amdvi/src/irte.rs b/kernel/amdvi/src/irte.rs index 575ed966f2e..62c6a5b3722 100644 --- a/kernel/amdvi/src/irte.rs +++ b/kernel/amdvi/src/irte.rs @@ -5,6 +5,9 @@ //! entry is the message's own vector. const REMAP_EN: u64 = 1 << 0; +/// `SupIOPF`: no `IO_PAGE_FAULT` is logged for a message through the entry, +/// whatever `RemapEn` says (Table 22). +const SUP_IOPF: u64 = 1 << 1; const DESTINATION_SHIFT: u32 = 8; /// `Vector`, bits 71:64: bits 7:0 of the second doubleword. const VECTOR_SHIFT: u32 = 0; @@ -30,6 +33,13 @@ pub const fn entry(vector: u8, destination: Destination) -> [u64; 2] { pub const ABSENT: [u64; 2] = [0, 0]; +/// `entry` refusing its messages silently: `RemapEn` clear, so each is target +/// aborted (Table 22, §2.1.3.1), and `SupIOPF` set, so none is logged. The +/// rest of the entry is kept for the [`entry`] it was to deliver again. +pub const fn masked(entry: [u64; 2]) -> [u64; 2] { + [entry[0] & !REMAP_EN | SUP_IOPF, entry[1]] +} + /// Whether an entry read back delivers anything. pub const fn present(words: [u64; 2]) -> bool { words[0] & REMAP_EN != 0 diff --git a/kernel/amdvi/tests/encodings.rs b/kernel/amdvi/tests/encodings.rs index a0ad5895673..fdf6ad3f16b 100644 --- a/kernel/amdvi/tests/encodings.rs +++ b/kernel/amdvi/tests/encodings.rs @@ -244,6 +244,17 @@ fn a_remapping_entry_delivers_its_vector_to_its_destination() { assert!(!irte::present(irte::ABSENT)); } +/// Table 22: a masked entry has `RemapEn` [0] clear and `SupIOPF` [1] set, +/// and every other bit of the entry it masks. +#[test] +fn a_masked_entry_aborts_silently_and_keeps_its_fields() { + let delivering = irte::entry(0xFB, Destination::Extended(0x1234_5678)); + let masked = irte::masked(delivering); + assert_eq!(masked, [delivering[0] & !1 | 1 << 1, delivering[1]]); + assert!(!irte::present(masked)); + assert_eq!((irte::vector(masked), irte::destination(masked)), (0xFB, 0x1234_5678)); +} + /// Figure 48: `INVALIDATE_INTERRUPT_TABLE`, code 05h, the id at [15:0]. #[test] fn invalidating_an_interrupt_table_names_its_requester() { diff --git a/kernel/loom/Cargo.toml b/kernel/loom/Cargo.toml index bf01b3134fd..f9d8c0a3446 100644 --- a/kernel/loom/Cargo.toml +++ b/kernel/loom/Cargo.toml @@ -142,6 +142,17 @@ shard-publish-relaxed = [] # Never on by default and never reachable from a kernel build, which declares # the same name only so `cfg` checking knows it. device-irq-lossy = [] +# The negative control for a claimed function's interrupt gate. The ISR masks +# and the read opens by their own intent, not by what the record owes, so a +# mask that lands after the read opened the gate strands the claim, and +# `device_irq_gate.rs` must red: +# +# cargo test --manifest-path kernel/loom/Cargo.toml --features device-irq-gate-intent \ +# --test device_irq_gate +# +# Never on by default and never reachable from a kernel build, which declares +# the same name only so `cfg` checking knows it. +device-irq-gate-intent = [] # The negative control for a program's log ring, `sdk/toyos/src/log/ring.rs`. It # makes the store that publishes a record — the shared ring's `published` word # and a lane's `head` — `Relaxed`, so a reader can find the word with the body diff --git a/kernel/loom/tests/device_irq.rs b/kernel/loom/tests/device_irq.rs index b098aebdb6a..24a6c044819 100644 --- a/kernel/loom/tests/device_irq.rs +++ b/kernel/loom/tests/device_irq.rs @@ -117,12 +117,12 @@ fn an_idle_record_answers_nothing() { loom::model(|| { let irq = Interrupt::new(); assert!(irq.take().is_none(), "an untouched record answered a reader"); - assert!(!irq.armed(), "an untouched record read ready"); + assert!(!irq.pending(), "an untouched record read ready"); irq.took(7); - assert!(irq.armed(), "a record with a message in it read empty"); + assert!(irq.pending(), "a record with a message in it read empty"); let record = irq.take().expect("a record with a message in it answered nothing"); assert_eq!((record.count, record.first_nanos, record.last_nanos), (1, 7, 7)); assert!(irq.take().is_none(), "the same message was answered twice"); - assert!(!irq.armed(), "a drained record still reads ready"); + assert!(!irq.pending(), "a drained record still reads ready"); }); } diff --git a/kernel/loom/tests/device_irq_gate.rs b/kernel/loom/tests/device_irq_gate.rs new file mode 100644 index 00000000000..20bc51d4274 --- /dev/null +++ b/kernel/loom/tests/device_irq_gate.rs @@ -0,0 +1,64 @@ +//! Loom: a claimed function's interrupt ends masked exactly while its record +//! owes a mask, from the second unread message, however the handler and a +//! read interleave (`pcidev/record.rs`); the `device-irq-gate-intent` control +//! must red the two-message model. + +use kernel_loom::device_irq::{Gate, Interrupt}; +use loom::sync::{Arc, Mutex}; + +/// The claim's interrupt as hardware has it, behind the gate's lock. +struct Line(Mutex); + +impl Gate for Line { + fn hold(&self, masked: impl FnOnce() -> bool) { + let mut hardware = self.0.lock().unwrap(); + *hardware = masked(); + } +} + +/// Messages against one read that may run at any point among them, and a +/// last read once they have all landed: after each, hardware is masked +/// exactly while the record owes it. +fn model(messages: u64) { + loom::model(move || { + let irq = Arc::new(Interrupt::new()); + let line = Arc::new(Line(Mutex::new(false))); + + let isr = { + let (irq, line) = (irq.clone(), line.clone()); + loom::thread::spawn(move || { + for at in 1..=messages { + irq.took_masking(at, &*line); + } + }) + }; + + let taken = irq.take_opening(&*line).map_or(0, |record| u64::from(record.count)); + isr.join().unwrap(); + let masked = *line.0.lock().unwrap(); + assert_eq!( + masked, + irq.owes_mask(), + "the read took {taken} of {messages} message(s) and hardware is {} where the record owes it {}", + if masked { "masked" } else { "open" }, + if irq.owes_mask() { "masked" } else { "open" }, + ); + + let left = irq.take_opening(&*line).map_or(0, |record| u64::from(record.count)); + assert_eq!(taken + left, messages, "the reads took {taken} and {left} of {messages}"); + assert!(!*line.0.lock().unwrap(), "every message was read and the claim is still masked"); + }); +} + +/// One message per read never masks. +#[test] +fn one_message_per_read_never_masks() { + model(1); +} + +/// A second message before a read masks, and the read that takes them opens +/// it, wherever the read lands among them. +#[test] +fn hardware_ends_as_the_record_owes_after_two_messages() { + model(2); +} diff --git a/kernel/src/arch/aarch64/irqchip/its.rs b/kernel/src/arch/aarch64/irqchip/its.rs index db18f02443c..fef70016c0e 100644 --- a/kernel/src/arch/aarch64/irqchip/its.rs +++ b/kernel/src/arch/aarch64/irqchip/its.rs @@ -28,6 +28,11 @@ //! does not consume within [`TIMEOUT_PER_SECOND`]'s bound, is a kernel //! defect, and panics. //! +//! **A claim's LPI is masked by its configuration byte's enable** (Table 5-1), +//! reread at the redistributor on an `INV` (§5.3.6): a disabled LPI's message +//! stays pending and is taken once it is enabled again. Every LPI is enabled +//! whenever no claim has masked it. +//! //! **A claim's DeviceID is the one the SMMUv3's own routes carry** //! (`super::super::smmu::its_device`): a function the unit would not put on a //! domain is refused before its claim reaches the unit. @@ -44,7 +49,7 @@ use crate::log; use crate::mm::pmm::{self, PhysPage}; use crate::mm::policy::MmioPolicy; use crate::mm::{DirectMap, Mmio, PAGE_2M}; -use crate::sync::Lock; +use crate::sync::{Lock, Masked}; /// A command is consumed within a second, and a register settles in one: /// `settles` takes the bound as a rate. @@ -89,19 +94,29 @@ const ITT_ALIGN: u64 = 256; static ITS: Lock> = Lock::new(None); -/// The ITS this kernel drives, armed. -struct Live { +/// The armed ITS's command queue: a leaf, taken with interrupts masked and +/// holding nothing else, because a claim's interrupt handler masks its LPI +/// through it ([`set_masked`]) while any other context may hold it. +static QUEUE: Masked> = Masked::new(None); + +struct Queue { regs: Mmio, - /// The MADT's id for it, which the IORT names it by. - id: u32, - /// `GITS_TRANSLATER`'s physical address: every claim's message address. - doorbell: u32, queue: Mmio, commands: CommandQueue, /// `GITS_CWRITER` as last written. cwriter: u64, /// The boot CPU's redistributor, as this ITS names it. target: Target, +} + +/// The ITS this kernel drives, armed. +struct Live { + /// The MADT's id for it, which the IORT names it by. + id: u32, + /// `GITS_TRANSLATER`'s physical address: every claim's message address. + doorbell: u32, + /// The redistributor's LPI configuration table. + configuration: Mmio, collection: Collection, events: EventBits, space: lpi::Space, @@ -127,9 +142,17 @@ impl Live { self.itts.push((device, table)); Some(table) } +} + +/// `commands`, then a `SYNC` to the boot CPU's redistributor, consumed: +/// every one has taken effect when this returns (§5.3.15). +fn issue(commands: &[Command]) { + let closed = crate::arch::IrqGuard::close(); + let mut held = QUEUE.lock(&closed); + held.as_mut().expect("ITS: a command issued with no ITS armed").issue(commands); +} - /// `commands`, then a `SYNC` to the boot CPU's redistributor, consumed: - /// every one has taken effect when this returns (§5.3.15). +impl Queue { fn issue(&mut self, commands: &[Command]) { let (regs, queue) = (self.regs, self.commands); for command in commands.iter().copied().chain([Command::Sync(self.target)]) { @@ -346,14 +369,15 @@ fn bring_up(id: u32, base: u64, frame: u64) -> Result Result) -> Result impl core::fmt::Display { alloc::format!("LPI {}", self.lpi.intid()) } + + pub fn line(&self) -> Line { + self.line + } +} + +/// A claim's LPI, which [`set_masked`] masks; named only while its +/// [`Claimed`] is held. +#[derive(Clone, Copy)] +pub struct Line { + /// The LPI's configuration byte. + configuration: Mmio, + device: u32, + event: toyos_its::Event, +} + +/// `line`'s LPI disabled, or enabled again, at the redistributor before this +/// returns. Takes only the command queue. +pub fn set_masked(line: Line, masked: bool) { + line.configuration.write_u8(0, lpi::configuration(PRIORITY, !masked)); + issue(&[Command::Reconfigure { device: line.device, event: line.event }]); } /// Map `source`'s DeviceID to its translation table, its one event to `pcidev` @@ -486,24 +529,28 @@ pub fn claim(function: usize, source: StreamId) -> Result { let table = live.table_of(device).ok_or(Refused::NoMemory)?; let event = live.events.event(EVENT).expect("event 0 is in every table"); let (collection, events) = (live.collection, live.events); - live.issue(&[ + issue(&[ Command::MapDevice { device, table, events }, Command::MapEvent { device, event, lpi, collection }, ]); live.held.push(device); log!("ITS: {source}, DeviceID {device:#x}, event {EVENT} is LPI {}", lpi.intid()); - Ok(Claimed { msi: Msi { address: live.doorbell, data: EVENT }, device, lpi }) + let configuration = live.configuration.subregion(lpi.configuration_index() as u64, 1); + Ok(Claimed { msi: Msi { address: live.doorbell, data: EVENT }, line: Line { configuration, device, event }, lpi }) } /// A claim's DeviceID unmapped, and anything it left pending dropped, before /// this returns. +/// +/// Its LPI is enabled again first, where its claim left it masked, so the +/// next claim on the function finds it as every unclaimed LPI is. pub fn release(claimed: &Claimed, source: StreamId) { - let device = claimed.device; + let Line { device, event, configuration } = claimed.line; let mut held = ITS.lock(); let live = held.as_mut().unwrap_or_else(|| panic!("ITS: {source} released with no ITS armed")); let at = live.held.iter().position(|held| *held == device); live.held.swap_remove(at.unwrap_or_else(|| panic!("ITS: DeviceID {device:#x} released unmapped"))); - let event = live.events.event(EVENT).expect("event 0 is in every table"); - live.issue(&[Command::Discard { device, event }, Command::UnmapDevice { device }]); + configuration.write_u8(0, lpi::configuration(PRIORITY, true)); + issue(&[Command::Reconfigure { device, event }, Command::Discard { device, event }, Command::UnmapDevice { device }]); log!("ITS: {source}, DeviceID {device:#x}, unmapped from LPI {}", claimed.lpi.intid()); } diff --git a/kernel/src/arch/x86_64/amdvi/domain.rs b/kernel/src/arch/x86_64/amdvi/domain.rs index 42d8d2b5f21..49a9792e06b 100644 --- a/kernel/src/arch/x86_64/amdvi/domain.rs +++ b/kernel/src/arch/x86_64/amdvi/domain.rs @@ -11,7 +11,8 @@ //! A requester an IVMD names stays on the identity domain: firmware declares //! memory it must keep reaching, which a domain of its own would not map. //! -//! `DOMAINS` is taken before `UNITS` or `TABLES`, and those two are never held together. +//! `DOMAINS` is taken before `UNITS` or `TABLES`, and those two are never held together; +//! a unit's slot in `UNITS` is a leaf. use alloc::vec::Vec; diff --git a/kernel/src/arch/x86_64/amdvi/interrupt.rs b/kernel/src/arch/x86_64/amdvi/interrupt.rs index 46657eabb61..9e60fb756df 100644 --- a/kernel/src/arch/x86_64/amdvi/interrupt.rs +++ b/kernel/src/arch/x86_64/amdvi/interrupt.rs @@ -13,6 +13,11 @@ //! `GuestMode` 0 (Table 21, Figure 17); its destination is 32 bits wide under //! `XTEn` (§2.2.5.3, Figure 19), which an x2APIC machine needs. //! +//! **A claim's entry is masked with `RemapEn` clear and `SupIOPF` set** +//! ([`irte::masked`]): its messages are target aborted, which the host bridge +//! completes with no error reported (§2.1.3.1), and none is logged. Each +//! change is published with `INVALIDATE_INTERRUPT_TABLE` (§2.4.5). +//! //! This module's lock is taken before `DOMAINS`, `UNITS` and `TABLES`, never //! after any of them. @@ -123,21 +128,44 @@ fn report(source: StreamId, vector: u8, read: [u64; 2]) { ); } +/// A claim's entry, held until [`release`]: whose table it is in, where, +/// and what it holds while it delivers. +#[derive(Clone, Copy)] +pub struct Line { + requester: u16, + vector: u8, + slot: crate::mm::Mmio, + delivers: [u64; 2], +} + /// `source`'s entry at `vector`, delivering it to this kernel's CPU. -fn arm(source: StreamId, vector: u8) { +fn arm(source: StreamId, vector: u8) -> Line { let mut remap = REMAP.lock(); let destination = match remap.extended { Some(true) => Destination::Extended(MSG_DEST), _ => Destination::Narrow(MSG_DEST_NARROW), }; - let read = remap.write(source.requester(), vector, irte::entry(vector, destination)); + let requester = source.requester(); + let delivers = remap.write(requester, vector, irte::entry(vector, destination)); + let table = remap.tables[&requester]; drop(remap); - report(source, vector, read); + report(source, vector, delivers); + Line { requester, vector, slot: memory(table.get() + u64::from(vector) * 16, 16), delivers } +} + +/// A claim's entry masked or delivering again, gone from every unit's cache +/// before this returns. One 64-bit write: `RemapEn` and `SupIOPF` are both +/// in the low half. Takes only the units' buffers, so a claim's interrupt +/// handler may call it. +pub(in crate::arch::x86_64) fn set_masked(line: Line, masked: bool) { + let entry = if masked { irte::masked(line.delivers) } else { line.delivers }; + line.slot.write_u64(0, entry[0]); + submit(&[Command::InvalidateInterrupts(line.requester)]); } /// A claim's entry at `vector` for `source`, whose entry from now on passes /// nothing firmware declared for it and gives it no `SysMgt`. -pub(in crate::arch::x86_64) fn claim(vector: u8, source: StreamId) -> Result { +pub(in crate::arch::x86_64) fn claim(vector: u8, source: StreamId) -> Result<(Msi, Line), Refused> { // A requester the platform speaks as is refused before here // (`iommu::isolates`), and one a kernel driver holds by `pcidev::claim`. // A table already written is a re-claim's, whose setting is `CLAIMED`; @@ -147,15 +175,14 @@ pub(in crate::arch::x86_64) fn claim(vector: u8, source: StreamId) -> Result = Lock::new(Tables::new()); -/// Every translating unit, and the single owner of each one's command buffer: -/// a buffer has one tail, so a second holder would write behind its back. -static UNITS: Lock> = Lock::new(Vec::new()); +/// Each translating unit's command buffer, [`PUBLISHED`] of them: a buffer +/// has one tail, so its slot is the one owner. A leaf, taken with interrupts +/// masked and holding nothing else, because a claim's interrupt handler +/// masks its entry through it ([`interrupt::set_masked`]) while any other +/// context may hold it. +static UNITS: [Masked>; MAX_UNITS] = [const { Masked::new(None) }; MAX_UNITS]; +static PUBLISHED: AtomicUsize = AtomicUsize::new(0); + +/// Each translating unit's SMI filter, until [`settle`] writes it. +static SMI: Lock> = Lock::new(Vec::new()); + +struct Unfiltered { + index: usize, + regs: Mmio, + /// [`Ready::smi`]. + filter: Option>, +} /// `bytes` at `phys`, through the direct map. fn memory(phys: u64, bytes: u64) -> Mmio { @@ -476,8 +490,6 @@ struct Live { tail: usize, done: Address<12>, waited: u64, - /// [`Ready::smi`], until [`settle`] writes it. - smi: Option>, } impl Live { @@ -518,8 +530,9 @@ impl Live { /// `commands` on every translating unit, finished on each before this returns. fn submit(commands: &[Command]) { - for unit in UNITS.lock().iter_mut() { - unit.submit(commands); + for slot in &UNITS[..PUBLISHED.load(Ordering::Acquire)] { + let closed = crate::arch::IrqGuard::close(); + slot.lock(&closed).as_mut().expect("amdvi: a published unit has its buffer").submit(commands); } } @@ -568,7 +581,7 @@ fn program(unit: Ready, devices: &[PciDevice], remap: Option) { || regs.read_u32(reg::STATUS) & running == running, || format!("iommu: amdvi unit{index} never ran its command buffer and event log: STATUS={:#010x}", regs.read_u32(reg::STATUS)), ); - let mut live = Live { index, regs, commands: memory(commands.get(), PAGE_SIZE), tail: 0, done, waited: 0, smi }; + let mut live = Live { index, regs, commands: memory(commands.get(), PAGE_SIZE), tail: 0, done, waited: 0 }; // §2.4.8 needs `INVALIDATE_IOMMU_ALL` alone. QEMU 11.1.1 moves a function // off pass-through only on an `INVALIDATE_DEVTAB_ENTRY` that names it // (`issues/qemus-invalidate-iommu-all-leaves-every-function-untranslated.md`), @@ -585,34 +598,40 @@ fn program(unit: Ready, devices: &[PciDevice], remap: Option) { events.get(), devices.len(), ); - UNITS.lock().push(live); + let mut settling = SMI.lock(); + let at = PUBLISHED.load(Ordering::Relaxed); + { + let closed = crate::arch::IrqGuard::close(); + *UNITS[at].lock(&closed) = Some(live); + } + PUBLISHED.store(at + 1, Ordering::Release); + settling.push(Unfiltered { index, regs, filter: smi }); } /// Every unit's SMI filter on, once every function this kernel drives is out /// of firmware's hands. A refused SMI is logged as an `IO_PAGE_FAULT` /// (§2.1.5.1) for [`fault`]. pub fn settle() { - let mut units = UNITS.lock(); + let mut units = SMI.lock(); let mut everywhere = !units.is_empty(); - for unit in units.iter_mut() { - let Some(filter) = unit.smi.take() else { + for Unfiltered { index, regs, filter } in units.iter_mut() { + let Some(filter) = filter.take() else { everywhere = false; continue; }; for (i, register) in (0u64..).zip(&filter) { - if !SmiFilter(unit.regs.read_u64(reg::SMI_FILTER + 8 * i)).locked() { - unit.regs.write_u64(reg::SMI_FILTER + 8 * i, register.0); + if !SmiFilter(regs.read_u64(reg::SMI_FILTER + 8 * i)).locked() { + regs.write_u64(reg::SMI_FILTER + 8 * i, register.0); } } - unit.regs.write_u64(reg::CONTROL, unit.regs.read_u64(reg::CONTROL) | control::SMIF_EN | control::SMIF_LOG_EN); + regs.write_u64(reg::CONTROL, regs.read_u64(reg::CONTROL) | control::SMIF_EN | control::SMIF_LOG_EN); let passes: Vec = (0..filter.len() as u64) - .filter_map(|i| SmiFilter(unit.regs.read_u64(reg::SMI_FILTER + 8 * i)).passes()) + .filter_map(|i| SmiFilter(regs.read_u64(reg::SMI_FILTER + 8 * i)).passes()) .map(|id| format!("{}", requester(id))) .collect(); log!( - "iommu: amdvi unit{} SMI filter on control={:#018x}, passing {}", - unit.index, - unit.regs.read_u64(reg::CONTROL), + "iommu: amdvi unit{index} SMI filter on control={:#018x}, passing {}", + regs.read_u64(reg::CONTROL), passes.join(" ") ); } diff --git a/kernel/src/arch/x86_64/iommu_unit.rs b/kernel/src/arch/x86_64/iommu_unit.rs index 4a5143161aa..73efa480410 100644 --- a/kernel/src/arch/x86_64/iommu_unit.rs +++ b/kernel/src/arch/x86_64/iommu_unit.rs @@ -282,8 +282,7 @@ pub mod interrupt { pub struct Claimed { pub msi: Msi, vector: ClaimVector, - /// VT-d's entry index; AMD-Vi's entry is the vector's own. - entry: Option, + line: Line, } impl Claimed { @@ -291,6 +290,27 @@ pub mod interrupt { pub fn interrupt(&self) -> impl core::fmt::Display { self.vector } + + pub fn line(&self) -> Line { + self.line + } + } + + /// A claim's remapping entry, which [`set_masked`] masks; named only + /// while its [`Claimed`] is held. + #[derive(Clone, Copy)] + pub enum Line { + Vtd(vtd::interrupt::Line), + AmdVi(amdvi::interrupt::Line), + } + + /// `line`'s messages refused silently at the unit, or delivered again, + /// before this returns. Takes only the units' invalidation queues. + pub fn set_masked(line: Line, masked: bool) { + match line { + Line::Vtd(line) => vtd::interrupt::set_masked(line, masked), + Line::AmdVi(line) => amdvi::interrupt::set_masked(line, masked), + } } /// Whether a claimed function's every message reaches only its own @@ -321,13 +341,13 @@ pub mod interrupt { pub fn claim(function: usize, source: StreamId) -> Result { let vector = user_dev::take(function).ok_or(Refused::NoVector)?; let made = match backend() { - Backend::AmdVi => amdvi::interrupt::claim(vector.vector(), source).map(|msi| (msi, None)), + Backend::AmdVi => amdvi::interrupt::claim(vector.vector(), source).map(|(msi, line)| (msi, Line::AmdVi(line))), Backend::Vtd => vtd::interrupt::claim(vector.vector(), source) - .map(|entry| (vtd::interrupt::message(entry), Some(entry))), + .map(|line| (vtd::interrupt::message(line.index()), Line::Vtd(line))), Backend::None => unreachable!("iommu: a claim's message made where nothing remaps"), }; match made { - Ok((msi, entry)) => Ok(Claimed { msi, vector, entry }), + Ok((msi, line)) => Ok(Claimed { msi, vector, line }), Err(why) => { user_dev::give_back(vector); Err(why) @@ -337,9 +357,9 @@ pub mod interrupt { /// A claim's entry gone from every unit, and then its vector free. pub fn release(claimed: &Claimed, source: StreamId) { - match claimed.entry { - Some(entry) => vtd::interrupt::release(entry, source), - None => amdvi::interrupt::release(claimed.vector.vector(), source), + match claimed.line { + Line::Vtd(line) => vtd::interrupt::release(line, source), + Line::AmdVi(line) => amdvi::interrupt::release(line, source), } user_dev::give_back(claimed.vector); } diff --git a/kernel/src/arch/x86_64/vtd/domain.rs b/kernel/src/arch/x86_64/vtd/domain.rs index bb7af2bec6d..d4a0e71b19a 100644 --- a/kernel/src/arch/x86_64/vtd/domain.rs +++ b/kernel/src/arch/x86_64/vtd/domain.rs @@ -13,7 +13,8 @@ //! 6.1: with caching mode reported, software invalidates after *every* change, //! a mapping becoming present included. //! -//! Lock order here is `DOMAINS`, `REMAP`, `UNITS`, `TABLES`, never the reverse. +//! Lock order here is `DOMAINS`, `REMAP`, `UNITS`, `TABLES`, never the reverse, +//! and a unit's invalidation queue last of all. use crate::log; use alloc::vec::Vec; diff --git a/kernel/src/arch/x86_64/vtd/interrupt.rs b/kernel/src/arch/x86_64/vtd/interrupt.rs index b0706752daf..a4a2903e592 100644 --- a/kernel/src/arch/x86_64/vtd/interrupt.rs +++ b/kernel/src/arch/x86_64/vtd/interrupt.rs @@ -29,6 +29,12 @@ //! the boot, one per arming, and by a claim until its release, when it goes //! not present and is free again. //! +//! **A claim's entry is masked not present with `FPD` set** (§9.9 Figure 9-9, +//! `FPD` bit 1, evaluated whatever `P` says): a message through it is blocked +//! (§5.1.4), and its fault, `22h`, is a qualified one (§5.1.4.1 Table 13), +//! recorded only where `FPD` is clear. Each change is followed by the entry's +//! interrupt-entry-cache invalidation (§6.4, §6.5.2.7). +//! //! This module's lock is taken before `UNITS` and before `TABLES`, never after //! either. @@ -57,6 +63,7 @@ const ENTRIES: usize = 256; const EXTENDED_INTERRUPT_MODE: u64 = 1 << 11; const PRESENT: u64 = 1; +const FAULT_PROCESSING_DISABLE: u64 = 1 << 1; const TRIGGER_LEVEL: u64 = 1 << 4; const VECTOR_SHIFT: u64 = 16; const DESTINATION_SHIFT: u64 = 32; @@ -212,15 +219,43 @@ pub(in crate::arch::x86_64) fn message(index: usize) -> Msi { } } +/// A claim's entry, held until [`release`]: where it is and what its low +/// half holds while it delivers. +#[derive(Clone, Copy)] +pub struct Line { + table: Table, + index: usize, + delivers: u64, +} + +impl Line { + pub(in crate::arch::x86_64) fn index(self) -> usize { + self.index + } +} + /// An entry delivering a claim's `vector`, written for `source` and published -/// to every unit before this returns; its index, held until [`release`]. -pub(in crate::arch::x86_64) fn claim(vector: u8, source: StreamId) -> Result { - allocate(source, vector, MSG_DEST, false) +/// to every unit before this returns. +pub(in crate::arch::x86_64) fn claim(vector: u8, source: StreamId) -> Result { + let index = allocate(source, vector, MSG_DEST, false)?; + let table = REMAP.lock().table.expect("iommu: a claim's entry was written with a table armed"); + Ok(Line { table, index, delivers: table.read_pair(index).0 }) +} + +/// A claim's entry masked or delivering again, gone from every remapping +/// unit's cache before this returns. One 64-bit write: `P` and `FPD` are both +/// in the low half, so the unit reads the old entry or the new one. Takes +/// only the units' queues, so a claim's interrupt handler may call it. +pub(in crate::arch::x86_64) fn set_masked(line: Line, masked: bool) { + let low = if masked { line.delivers & !PRESENT | FAULT_PROCESSING_DISABLE } else { line.delivers }; + line.table.write_low(line.index, low); + super::remapping_queues(|queue| queue.invalidate_interrupt(line.index)); } /// A claim's entry not present again and gone from every unit's cache, and /// then free. -pub(in crate::arch::x86_64) fn release(entry: usize, source: StreamId) { +pub(in crate::arch::x86_64) fn release(line: Line, source: StreamId) { + let entry = line.index; let lo = { let remap = REMAP.lock(); let Some(table) = remap.table else { diff --git a/kernel/src/arch/x86_64/vtd/mod.rs b/kernel/src/arch/x86_64/vtd/mod.rs index 6f013910a3a..0daa6e73510 100644 --- a/kernel/src/arch/x86_64/vtd/mod.rs +++ b/kernel/src/arch/x86_64/vtd/mod.rs @@ -26,8 +26,9 @@ use crate::drivers::acpi::TableError; use crate::drivers::pci::PciDevice; use crate::iommu::{AddressWidth, StreamId}; use crate::mm::Mmio; -use crate::sync::Lock; +use crate::sync::{Lock, Masked}; use crate::time::{Duration, Tripwire}; +use core::sync::atomic::{AtomicUsize, Ordering}; use super::iommu_unit::{yn, MAX_UNITS}; use dmar::{Dmar, Malformed, Scope, Scopes, Structure}; @@ -88,19 +89,47 @@ const COMMAND_TIMEOUT: Tripwire = Tripwire::absurd( /// Remapping tables outlive `init`: the unit keeps reading them while it is on. static TABLES: Lock = Lock::new(Tables::new()); -/// Every translating unit, and the single owner of each one's invalidation -/// queue: a queue has one tail, so a second holder would submit behind its back. +/// Every translating unit. static UNITS: Lock> = Lock::new(Vec::new()); -/// A unit past its hand-over: its window, its queue, and the root table its -/// context entries live in. -pub(super) struct Live { - regs: Mmio, +/// Each translating unit's invalidation queue, [`PUBLISHED`] of them, in the +/// order [`UNITS`] holds them: a queue has one tail, so its slot is the one +/// owner. A leaf, taken with interrupts masked and holding nothing else, +/// because a claim's interrupt handler masks its entry through it +/// ([`interrupt::set_masked`]) while any other context may hold it. +static QUEUES: [Masked>; MAX_UNITS] = [const { Masked::new(None) }; MAX_UNITS]; +static PUBLISHED: AtomicUsize = AtomicUsize::new(0); + +struct Published { queue: Queue, - root: Table, /// A descriptor type a unit does not implement stalls its queue, so the /// interrupt-entry-cache type goes only to a unit that remaps. remaps: bool, +} + +/// `f` on the queue of the unit at `index` in [`QUEUES`]. +fn with_queue(index: usize, f: impl FnOnce(&mut Published) -> R) -> R { + let closed = crate::arch::IrqGuard::close(); + let mut held = QUEUES[index].lock(&closed); + f(held.as_mut().expect("vtd: a published unit has its queue")) +} + +/// `f` on every remapping unit's queue. +fn remapping_queues(mut f: impl FnMut(&mut Queue)) { + for index in 0..PUBLISHED.load(Ordering::Acquire) { + with_queue(index, |unit| { + if unit.remaps { + f(&mut unit.queue); + } + }); + } +} + +/// A unit past its hand-over: its queue's place in [`QUEUES`], and the root +/// table its context entries live in. +pub(super) struct Live { + queue: usize, + root: Table, /// [`Unit::pci_all`]. pci_all: bool, /// [`Unit::hierarchies`]. @@ -109,22 +138,20 @@ pub(super) struct Live { impl Live { pub(super) fn invalidate_domain(&mut self, domain: u16) { - self.queue.invalidate_domain(self.regs, domain); + with_queue(self.queue, |unit| unit.queue.invalidate_domain(domain)); } /// Moves `stream` onto `domain` in this unit's tables, and out of /// whatever this unit still holds for it under its old one. pub(super) fn attach(&mut self, stream: StreamId, id: u16, space: table::Space) { let displaced = table::bind(&mut TABLES.lock(), self.root, stream, id, space); - self.queue.invalidate_context(self.regs, displaced); + with_queue(self.queue, |unit| unit.queue.invalidate_context(displaced)); } } /// Every remapping unit's interrupt-entry cache, gone. pub(super) fn invalidate_interrupt_entries() { - for unit in UNITS.lock().iter_mut().filter(|u| u.remaps) { - unit.queue.invalidate_interrupts(unit.regs); - } + remapping_queues(Queue::invalidate_interrupts); } /// Every region firmware reserved and the interrupt range are what no domain's @@ -464,12 +491,12 @@ impl Unit { self.command(QUEUED_INVALIDATION_ENABLE, true, "queued invalidation"); self.regs.write_u64(RTADDR_REG, root.phys()); self.command(SET_ROOT_TABLE_POINTER, true, "the root table pointer"); - queue.invalidate_all(self.regs); + queue.invalidate_all(); self.command(TRANSLATION_ENABLE, true, "translation"); if self.caps.interrupt_remapping() { self.regs.write_u64(interrupt::IRTA_REG, remap.phys()); self.command(interrupt::SET_TABLE_POINTER, true, "the interrupt remap table pointer"); - queue.invalidate_interrupts(self.regs); + queue.invalidate_interrupts(); self.command(interrupt::INTERRUPT_REMAPPING_ENABLE, true, "interrupt remapping"); let status = self.regs.read_u32(GSTS_REG) & PERSISTENT; self.regs.write_u32(GCMD_REG, status | interrupt::COMPATIBILITY_FORMAT); @@ -676,7 +703,7 @@ fn translate(unit: Unit, plan: Plan, root: Table, via: Invalidation) -> Translat unit.regs.write_u64(RTADDR_REG, root.phys()); unit.command(SET_ROOT_TABLE_POINTER, true, "the root table pointer"); match queue.as_mut() { - Some(queue) => queue.invalidate_all(unit.regs), + Some(queue) => queue.invalidate_all(), None => unit.invalidate_through_registers(), } unit.command(TRANSLATION_ENABLE, true, "translation"); @@ -701,12 +728,21 @@ fn finish(translating: Translating, devices: &[PciDevice], remap: Option) if let Some(irta) = irta { unit.regs.write_u64(interrupt::IRTA_REG, irta); unit.command(interrupt::SET_TABLE_POINTER, true, "the interrupt remap table pointer"); - queue.invalidate_interrupts(unit.regs); + queue.invalidate_interrupts(); unit.command(interrupt::INTERRUPT_REMAPPING_ENABLE, true, "interrupt remapping"); } // After `IRE`, so nothing is invalidated on a unit that is not yet reading // the table, and after `TE`, so nothing reaches a unit that is not on. - UNITS.lock().push(Live { regs: unit.regs, queue, root, remaps: remap.is_some(), pci_all: unit.pci_all, hierarchies: unit.hierarchies.clone() }); + { + let mut units = UNITS.lock(); + let at = PUBLISHED.load(Ordering::Relaxed); + { + let closed = crate::arch::IrqGuard::close(); + *QUEUES[at].lock(&closed) = Some(Published { queue, remaps: remap.is_some() }); + } + PUBLISHED.store(at + 1, Ordering::Release); + units.push(Live { queue: at, root, pci_all: unit.pci_all, hierarchies: unit.hierarchies.clone() }); + } let gsts = unit.regs.read_u32(GSTS_REG); log!( diff --git a/kernel/src/arch/x86_64/vtd/queue.rs b/kernel/src/arch/x86_64/vtd/queue.rs index 22230e98582..11b857fffea 100644 --- a/kernel/src/arch/x86_64/vtd/queue.rs +++ b/kernel/src/arch/x86_64/vtd/queue.rs @@ -18,6 +18,10 @@ const CONTEXT_CACHE: u64 = 0x1; const IOTLB: u64 = 0x2; /// Type 4h, Section 6.5.2.7 — granularity inverted against the two above: clear is global. const INTERRUPT_ENTRY_CACHE: u64 = 0x4; +/// `G`, bit 4 of an interrupt-entry-cache descriptor: set, one index (§6.5.2.7). +const INDEX_SELECTIVE: u64 = 1 << 4; +/// `IIDX`, bits 47:32; `IM` 31:27 left 0 names that index alone. +const INDEX_SHIFT: u64 = 32; const WAIT: u64 = 0x5; /// Granularity field, bits 5:4: `01` is global — every domain, every entry. @@ -48,6 +52,7 @@ const ACK_TIMEOUT: Tripwire = Tripwire::absurd( ); pub struct Queue { + regs: Mmio, descriptors: Table, /// Own page: the ring uses its full 4 KiB, so an inline status word would collide with a descriptor slot. status: Table, @@ -57,7 +62,7 @@ pub struct Queue { impl Queue { /// The caller enables `GCMD.QIE` only after this returns, since a queue the unit has not been pointed at is a queue it never reads. pub fn new(tables: &mut Tables, regs: Mmio) -> Self { - let queue = Self { descriptors: tables.alloc(), status: tables.alloc(), tail: 0 }; + let queue = Self { regs, descriptors: tables.alloc(), status: tables.alloc(), tail: 0 }; // Descriptor width 0, queue size 0: the register is the base address alone. regs.write_u64(IQA_REG, queue.descriptors.phys()); regs.write_u64(IQT_REG, 0); @@ -66,58 +71,43 @@ impl Queue { /// Every cached translation and context entry, gone, acknowledged by the /// unit before this returns. - pub fn invalidate_all(&mut self, regs: Mmio) { - self.submit( - regs, - &[ - (CONTEXT_CACHE | GLOBAL, 0), - (IOTLB | GLOBAL | DRAIN_WRITES | DRAIN_READS, 0), - ], - ); + pub fn invalidate_all(&mut self) { + self.submit(&[(CONTEXT_CACHE | GLOBAL, 0), (IOTLB | GLOBAL | DRAIN_WRITES | DRAIN_READS, 0)]); } /// Every translation cached for one domain, gone. Owed after any change to /// its second-level tables, a mapping becoming present included, because /// `CAP.CM` says the unit may have cached the absence. - pub fn invalidate_domain(&mut self, regs: Mmio, domain: u16) { - self.submit( - regs, - &[( - IOTLB | DOMAIN | DRAIN_WRITES | DRAIN_READS - | ((domain as u64) << DOMAIN_SHIFT), - 0, - )], - ); + pub fn invalidate_domain(&mut self, domain: u16) { + self.submit(&[(IOTLB | DOMAIN | DRAIN_WRITES | DRAIN_READS | ((domain as u64) << DOMAIN_SHIFT), 0)]); } /// A replaced context entry, then everything cached under the domain it /// named, in Section 6.5.2.1's order: the unit looks a translation up by /// requester and keeps it under that id (Section 6.2.1). - pub fn invalidate_context(&mut self, regs: Mmio, displaced: Displaced) { + pub fn invalidate_context(&mut self, displaced: Displaced) { let domain = (displaced.domain() as u64) << DOMAIN_SHIFT; - self.submit( - regs, - &[ - ( - CONTEXT_CACHE - | DEVICE - | domain - | ((displaced.requester() as u64) << SOURCE_SHIFT), - 0, - ), - (IOTLB | DOMAIN | DRAIN_WRITES | DRAIN_READS | domain, 0), - ], - ); + self.submit(&[ + (CONTEXT_CACHE | DEVICE | domain | ((displaced.requester() as u64) << SOURCE_SHIFT), 0), + (IOTLB | DOMAIN | DRAIN_WRITES | DRAIN_READS | domain, 0), + ]); } /// Every cached interrupt remapping entry, gone. Owed after `SIRTP` where /// `CAP.ESIRTPS` is clear (Section 6.7); sent only to a unit that remaps, /// since a type it does not implement stalls its queue. - pub fn invalidate_interrupts(&mut self, regs: Mmio) { - self.submit(regs, &[(INTERRUPT_ENTRY_CACHE, 0)]); + pub fn invalidate_interrupts(&mut self) { + self.submit(&[(INTERRUPT_ENTRY_CACHE, 0)]); + } + + /// Interrupt remapping entry `index`, gone from the cache, and every + /// message the unit already remapped through it delivered (§6.5.2.7). + pub fn invalidate_interrupt(&mut self, index: usize) { + self.submit(&[(INTERRUPT_ENTRY_CACHE | INDEX_SELECTIVE | (index as u64) << INDEX_SHIFT, 0)]); } - fn submit(&mut self, regs: Mmio, descriptors: &[(u64, u64)]) { + fn submit(&mut self, descriptors: &[(u64, u64)]) { + let regs = self.regs; assert!( descriptors.len() + 1 < QUEUE_ENTRIES, "iommu: {} descriptors do not fit a {QUEUE_ENTRIES}-entry queue", diff --git a/kernel/src/arch/x86_64/vtd/table.rs b/kernel/src/arch/x86_64/vtd/table.rs index a16fd3f530b..e5bc5c26e9e 100644 --- a/kernel/src/arch/x86_64/vtd/table.rs +++ b/kernel/src/arch/x86_64/vtd/table.rs @@ -112,6 +112,11 @@ impl Table { self.write(index * 2, lo); } + /// Rewrites only the low half of the 16-byte entry at `index`. + pub fn write_low(self, index: usize, lo: u64) { + self.write(index * 2, lo); + } + /// Zeroes a 16-byte entry low half first: the low half holds `P`, and an /// entry whose high half went first would for a moment be present over a /// field it no longer means. diff --git a/kernel/src/iommu/mod.rs b/kernel/src/iommu/mod.rs index e9fb3037b2b..d272a6e6649 100644 --- a/kernel/src/iommu/mod.rs +++ b/kernel/src/iommu/mod.rs @@ -306,6 +306,11 @@ impl Remapped { self.claimed.interrupt() } + /// What masks the message, for as long as this is held. + pub fn line(&self) -> MessageLine { + MessageLine(self.claimed.line()) + } + fn stream(&self) -> StreamId { StreamId::pci(self.function.bus, self.function.dev, self.function.func) } @@ -317,6 +322,21 @@ impl Drop for Remapped { } } +/// What masks a claimed function's message at the unit that translates it: +/// its remapping entry, or its LPI. Valid while its [`Remapped`] is held. +#[derive(Clone, Copy)] +pub struct MessageLine(messages::Line); + +impl MessageLine { + /// The function's messages refused, or delivered again, before this + /// returns: a refused one on x86-64 is dropped at the unit and recorded + /// nowhere, and on AArch64 left pending until it is delivered. Takes only + /// the units' invalidation queues, so an interrupt handler may call it. + pub fn set_masked(self, masked: bool) { + messages::set_masked(self.0, masked); + } +} + /// A message for `pcidev` function `number`, raising an interrupt taken for /// it and translated for `function` alone. pub fn claim_msi( diff --git a/kernel/src/isa.rs b/kernel/src/isa.rs index b8b65c38f48..aa25d9bc3b9 100644 --- a/kernel/src/isa.rs +++ b/kernel/src/isa.rs @@ -250,7 +250,7 @@ pub fn take_record(row: &Row) -> Option { } pub fn has_irq(row: &Row) -> bool { - IRQ[row.0].armed() + IRQ[row.0].pending() } /// Records one interrupt, masks the row's level lines and posts the claim's diff --git a/kernel/src/pcidev/mod.rs b/kernel/src/pcidev/mod.rs index 88a8e727cd3..ee0962f69ac 100644 --- a/kernel/src/pcidev/mod.rs +++ b/kernel/src/pcidev/mod.rs @@ -136,12 +136,12 @@ use toyos_pci::{af, aperture, bar, express, hotplug, pages, pm}; use crate::device::{Claim, ClaimError}; use crate::drivers::pci::{arm_claimed_msi, arm_claimed_msix, mask_claimed_msix, NoCapability, NoEntry, PciDevice}; -use crate::iommu::{IommuError, OwnSpace, Refused, Remapped}; +use crate::iommu::{IommuError, MessageLine, OwnSpace, Refused, Remapped}; use kernel::claimspace::{Span, Window}; use crate::mm::policy::CachePolicy; use crate::mm::align_2m; use crate::object::shm::SharedMemObject; -use crate::sync::Lock; +use crate::sync::{Lock, Masked}; use crate::inbox::PollEntry; use crate::watch::IrqWatch; @@ -189,6 +189,8 @@ struct Function { /// its fault is never this kernel's ([`note_fault`]). claimed: AtomicBool, irq: Interrupt, + /// What masks its claim's message while [`Self::irq`] holds a count. + gate: Gate, /// What its holder's poll waits on: two processes each driving a function /// must not learn when the other's device is busy. watch: IrqWatch, @@ -249,6 +251,7 @@ fn new_record(pci: PciDevice, arrival: Arrival) -> Function { held: AtomicBool::new(false), claimed: AtomicBool::new(false), irq: Interrupt::new(), + gate: Gate(Masked::new(None)), watch: IrqWatch::new(), space: Lock::new(None), bound: Lock::new(None), @@ -1044,8 +1047,10 @@ pub fn claim(arrival: Arrival, firmware: FirmwareWindows) -> Result<(PciFunction bound.armed.message().interrupt(), ); let function = binding.function(); + let line = bound.armed.message().line(); *function.bound.lock() = Some(bound); function.irq.clear(); + function.gate.arm(line); changed(); Ok((info, Claim::pci(binding))) } @@ -1522,6 +1527,7 @@ fn release(number: usize, arrival: Arrival) { /// that maps nothing and a message that is not present. fn tear_down_departed(function: &Function, bound: Bound) { let Bound { space, armed, grants, id, .. } = bound; + function.gate.disarm(); drop(armed); for grant in grants.iter() { if let Err(why) = space.unmap(grant.at, grant.bytes) { @@ -1586,6 +1592,7 @@ fn tear_down(function: &Function, mut bound: Bound) { // Once the function no longer speaks, and before it can be claimed again: // its translation goes, and its interrupt is free. bound.pci.drain_messages(); + function.gate.disarm(); drop(message); // **The reset before the domain gives anything back.** With mastering off // the function starts nothing new; its grants stay mapped until it is @@ -1970,9 +1977,10 @@ pub fn config_read(binding: &Binding, at: Register, width: RegWidth) -> Result Result, SyscallError> { let function = binding.function(); if !binding.here() { @@ -1981,7 +1989,7 @@ pub fn take_record(binding: &Binding) -> Result, Syscall if function.irq.faulted() { return Err(SyscallError::Io); } - let taken = function.irq.take(); + let taken = function.irq.take_opening(&function.gate); if taken.is_some() && function.irq.take_unannounced() { let bound = function.bound.lock(); let bound = bound.as_ref().expect("pcidev: a claim's binding has its function bound"); @@ -1994,13 +2002,16 @@ pub fn take_record(binding: &Binding) -> Result, Syscall /// refusal is. pub fn has_irq(binding: &Binding) -> bool { let function = binding.function(); - !binding.here() || function.irq.armed() || function.irq.faulted() + !binding.here() || function.irq.pending() || function.irq.faulted() } -/// Records one message and posts the claim's watch. Called from the +/// Records one message, masks the claim's message until its holder reads +/// the record where this is the second unread one, and posts the claim's +/// watch: a function's interrupts between two reads stop at what lands +/// before the mask, whatever its holder makes it send. Called from the /// interrupt's handler for function `number`, so it allocates nothing; -/// `record.rs` owns the counting, and `kernel-loom` models it against a -/// concurrent reader. +/// `record.rs` owns the counting and the mask's rule, and `kernel-loom` +/// models both against a concurrent reader. /// /// **The handler holds the preempt count raised across it**: the post lets go /// of a count of its own, and at depth zero that would run a scheduler pass @@ -2011,10 +2022,49 @@ pub fn isr(number: usize) { if let Some(slot) = slot::of(function) { return slot.isr(); } - function.irq.took(crate::clock::nanos_since_boot()); + function.irq.took_masking(crate::clock::nanos_since_boot(), &function.gate); function.watch.post_in_place(); } +/// A claim's message line and whether it is masked, while the claim holds +/// it. A leaf but for the units' invalidation queues, taken with interrupts +/// masked: the handler takes it too. +struct Gate(Masked>); + +struct Gated { + line: MessageLine, + masked: bool, +} + +impl Gate { + /// A claim's line, which its arming left delivering. + fn arm(&self, line: MessageLine) { + let closed = crate::arch::IrqGuard::close(); + let previous = self.0.lock(&closed).replace(Gated { line, masked: false }); + assert!(previous.is_none(), "pcidev: a claim's line was armed over another's"); + } + + /// Before the claim's message is given back, so nothing masks an entry + /// another claim may be given. + fn disarm(&self) { + let closed = crate::arch::IrqGuard::close(); + self.0.lock(&closed).take(); + } +} + +impl record::Gate for Gate { + fn hold(&self, masked: impl FnOnce() -> bool) { + let closed = crate::arch::IrqGuard::close(); + let mut held = self.0.lock(&closed); + let Some(gated) = held.as_mut() else { return }; + let masked = masked(); + if masked != gated.masked { + gated.line.set_masked(masked); + gated.masked = masked; + } + } +} + /// The unit refused requester `who` an access: `true` where a claim holds it, /// a claim held it, or it has left the device set, so that no driver of this /// kernel's is to blame, and its bus mastering is then cleared. diff --git a/kernel/src/pcidev/record.rs b/kernel/src/pcidev/record.rs index 47694d97228..88e76f7145f 100644 --- a/kernel/src/pcidev/record.rs +++ b/kernel/src/pcidev/record.rs @@ -27,6 +27,15 @@ //! the first can take read `n + 1` and let read `n + 2`'s first message into //! the slot the first is loading; that costs the racing holder its own time, //! never a count. +//! +//! **A claim's interrupt is masked from the second message before a read** +//! ([`Gate`], [`MASKED_FROM`]): a driver that reads between messages never +//! pays for a mask, and one that falls behind is held to what lands before +//! the mask takes. Each side changes the count first and then, under the +//! gate's lock, puts hardware in the state the record then owes, read from +//! the word and not from its own change: the side that takes the lock last +//! sees both changes, so a mask that lands after the read that opened the +//! gate cannot leave a claim masked with nothing to read. #[cfg(not(feature = "loom"))] use core::sync::atomic::{AtomicBool, AtomicU64, Ordering}; @@ -34,7 +43,7 @@ use core::sync::atomic::{AtomicBool, AtomicU64, Ordering}; #[cfg(feature = "loom")] use loom::sync::atomic::{AtomicBool, AtomicU64, Ordering}; -use toyos_abi::pci::DeviceIrqRecord; +use toyos_abi::pci::{DeviceIrqRecord, MASKED_FROM}; /// The words that say only what they hold: the latch, the fault and the /// newest stamp, whose reader takes it after the exchange that orders it. @@ -177,10 +186,41 @@ impl Interrupt { /// Whether a message is waiting, for a readiness check that consumes /// nothing. - pub fn armed(&self) -> bool { + pub fn pending(&self) -> bool { self.word.load(ORDER) & COUNT != 0 } + /// [`Self::took`], then the claim's interrupt masked behind `gate` where + /// this message found one waiting unread. + pub fn took_masking(&self, now: u64, gate: &impl Gate) { + self.took(now); + #[cfg(feature = "device-irq-gate-intent")] + { + let seen = self.owes_mask(); + gate.hold(|| seen); + } + #[cfg(not(feature = "device-irq-gate-intent"))] + gate.hold(|| self.owes_mask()); + } + + /// [`Self::take`], then the claim's interrupt opened behind `gate` where + /// the read took something. + pub fn take_opening(&self, gate: &impl Gate) -> Option { + let taken = self.take(); + if taken.is_some() { + #[cfg(feature = "device-irq-gate-intent")] + gate.hold(|| false); + #[cfg(not(feature = "device-irq-gate-intent"))] + gate.hold(|| self.owes_mask()); + } + taken + } + + /// Whether the record owes its claim's interrupt masked. + pub fn owes_mask(&self) -> bool { + self.word.load(ORDER) & COUNT >= u64::from(MASKED_FROM) + } + /// Whether this is the first message this claim has taken. Answers `true` /// once per claim and `false` ever after, so a caller may log on it. /// @@ -208,3 +248,11 @@ impl Interrupt { self.unannounced.store(true, ORDER); } } + +/// What masks a claim's interrupt in hardware, behind a lock both sides of +/// its record take. +pub trait Gate { + /// Under the lock, put hardware in the state `masked` answers, where it + /// is not already in it. + fn hold(&self, masked: impl FnOnce() -> bool); +} diff --git a/sdk/abi/src/pci.rs b/sdk/abi/src/pci.rs index a1629f7d5a1..f0fa833099c 100644 --- a/sdk/abi/src/pci.rs +++ b/sdk/abi/src/pci.rs @@ -197,12 +197,18 @@ pub struct DmaMapping { const _: () = assert!(core::mem::size_of::() == 8 + 8); -/// The interrupts that landed since the last read of a claim. +/// The interrupts the kernel took for a claim since its last read. /// /// One record and not a queue: the kernel accumulates, so a driver that slept /// through several is told about all of them at once and there is no ring for a -/// slow reader to overflow. The count says how many messages arrived, never -/// what any of them meant: that is in the device's own rings. +/// slow reader to overflow. The count is of interrupts the kernel took, never +/// of messages the device sent or what any of them meant: the interrupt +/// controller folds messages that arrive while one is pending, and what they +/// meant is in the device's own rings. +/// +/// **A claimed PCI function's interrupt is masked from the second interrupt +/// taken before a read until that read**: a message sent meanwhile is refused +/// at the unit on x86-64, and left pending until the read on AArch64. /// /// **The two times are when messages landed, which a driver reading its rings /// later cannot recover**: a driver clocked by its device — soundserver's DLL — @@ -214,14 +220,18 @@ pub struct DeviceIrqRecord { /// Never 0 in a record that was answered; an empty count is `WouldBlock`. pub count: u32, pub _pad: u32, - /// When the first message `count` holds landed. + /// When the first interrupt `count` holds was taken. pub first_nanos: u64, - /// When the newest landed: never older than the newest message `count` - /// holds, and newer only by one that lands as the read is made, which the - /// next read counts. + /// When the newest was taken: never older than the newest interrupt + /// `count` holds, and newer only by one taken as the read is made, which + /// the next read counts. pub last_nanos: u64, } +/// The count from which a claimed PCI function's interrupt is masked until +/// the read that takes it. +pub const MASKED_FROM: u32 = 2; + impl DeviceIrqRecord { pub const SIZE: usize = core::mem::size_of::(); } diff --git a/sdk/toyos/src/device.rs b/sdk/toyos/src/device.rs index 56b02ded5cb..b637a3056b1 100644 --- a/sdk/toyos/src/device.rs +++ b/sdk/toyos/src/device.rs @@ -192,6 +192,12 @@ impl PciDev { } /// The interrupts since the last read, or `Err(WouldBlock)` for none. + /// + /// **Take the record before servicing the device.** From a second + /// interrupt before a read to that read, the function's interrupt is + /// masked, and what the device signals meanwhile raises nothing: a driver + /// that serviced first and read after could miss an event that arrived in + /// between. pub fn irq(&self) -> Result { irq_record(&self.0) } diff --git a/system/soundserver/virtio-sound/src/lib.rs b/system/soundserver/virtio-sound/src/lib.rs index b1249293175..c182ffc04ae 100644 --- a/system/soundserver/virtio-sound/src/lib.rs +++ b/system/soundserver/virtio-sound/src/lib.rs @@ -240,8 +240,8 @@ impl Queues { pub struct Played { /// The periods the device gave back, one bit each. pub mask: u32, - /// How many of them, in the order they came back, the record's newest - /// notification is the time of; the rest are a later record's. + /// 1 where this read anchors the clock at the record's newest time, else + /// 0: these periods are then clocked by the next read that anchors. pub clocked: u32, } @@ -260,8 +260,6 @@ pub struct Sound { bell: D, queues: Queues, running: bool, - /// Periods a read counted before their notification landed. - unnotified: u32, } impl Sound { @@ -272,7 +270,7 @@ impl Sound { if streams == 0 { return Err(Refusal::NoStream); } - let mut sound = Self { mem, bell, queues, running: false, unnotified: 0 }; + let mut sound = Self { mem, bell, queues, running: false }; for buffer in 0..EVENT_BUFS { sound.post_event(buffer); } @@ -333,29 +331,24 @@ impl Sound { Ok(()) } - /// The periods the device has given back since a record of `notified` - /// notifications was taken; nothing without a record. + /// The periods the device has given back since the last read, for a + /// record of `taken` interrupts; nothing without a record. /// - /// The device writes a used element before the notification that says - /// so, and with `avail.flags` zero and no `VIRTIO_F_EVENT_IDX` it owes one - /// for every element (§2.7.7), so a ring read after a record was taken - /// holds every period that record counts. With no record the ring is not - /// read: a period already in it has a notification still to land, which - /// makes the record readable again, and it is read then. + /// The periods are the used ring's and never the count's: the interrupt + /// controller folds messages and the claim's mask drops them, so a count + /// says only that the device spoke (`toyos_abi::pci`). With no record the + /// ring is not read: a period already in it has a notification still to + /// land, which makes the record readable again, and it is read then. /// - /// **A period read before its notification landed is not clocked by this - /// record**: notifications answer periods in the order they came back, the - /// ones an earlier read counted first, and [`Played::clocked`] counts the - /// periods of this mask the record's notifications reach. - pub fn played(&mut self, notified: Option) -> Result, Refusal> { - let Some(notified) = notified else { return Ok(None) }; + /// **The record's newest time anchors the clock where one interrupt was + /// taken and one period read**, and any other read anchors nothing, its + /// periods carrying into the next that does. A period read ahead of its + /// notification can make the next such read anchor early, by at most that + /// read's wake latency. + pub fn played(&mut self, taken: Option) -> Result, Refusal> { + let Some(count) = taken else { return Ok(None) }; let mask = self.completed()?; - let periods = mask.count_ones(); - let fresh = notified.saturating_sub(self.unnotified); - let clocked = fresh.min(periods); - // No more can be owed than the ring holds periods. - self.unnotified = - (self.unnotified.saturating_sub(notified) + periods - clocked).min(PERIODS as u32); + let clocked = u32::from(count == 1 && mask.count_ones() == 1); Ok((mask != 0).then_some(Played { mask, clocked })) } diff --git a/system/soundserver/virtio-sound/src/tests.rs b/system/soundserver/virtio-sound/src/tests.rs index 1d3a26ad92e..a5134347a4d 100644 --- a/system/soundserver/virtio-sound/src/tests.rs +++ b/system/soundserver/virtio-sound/src/tests.rs @@ -586,9 +586,10 @@ impl Claim { } /// No period is stranded: every one comes back exactly once, and none while -/// the claim is left unreadable. **And none is stamped earlier than its own -/// notification**: a period a record clocks is one whose notification that -/// record, or an earlier one, carried. +/// the claim is left unreadable. **And within two periods none is stamped +/// earlier than its own notification**: a period a record clocks is one whose +/// notification that record, or an earlier one, carried. A third period can +/// be (`Sound::played`). /// /// Two periods, each a used element and then its notification (§2.7.7), /// against two wakes of a driver that takes the record and then reads the @@ -664,3 +665,25 @@ fn no_period_is_stranded_between_its_used_element_and_its_notification() { } assert_eq!(orders, 70); } + +/// A record of one interrupt over two periods anchors nothing, and the +/// steady reads after it are each clocked again. +#[test] +fn a_record_folded_over_two_periods_does_not_stop_the_clock() { + let model = Model::new(); + let mut sound = opened(&model); + for period in 0..PERIODS { + sound.submit(period).expect("a period goes out"); + } + model.play(2); + let folded = sound.played(Some(1)).expect("whole periods").expect("two periods came back"); + assert_eq!(folded.mask.count_ones(), 2); + let mut clocked = vec![folded.clocked]; + for _ in 0..3 { + model.play(1); + let played = sound.played(Some(1)).expect("a whole period").expect("one period came back"); + assert_eq!(played.mask.count_ones(), 1); + clocked.push(played.clocked); + } + assert_eq!(clocked, [0, 1, 1, 1], "how many periods the folded read and each steady read clocked"); +} diff --git a/tests/guest/rust-tests/src/bin/i219_msi.rs b/tests/guest/rust-tests/src/bin/i219_msi.rs index 78382fa9033..7e9a102995a 100644 --- a/tests/guest/rust-tests/src/bin/i219_msi.rs +++ b/tests/guest/rust-tests/src/bin/i219_msi.rs @@ -1,8 +1,8 @@ //! The T14's I219, judged on what `toyos-i219` rests on where its interrupts //! are concerned, on one claim: a capability list with MSI and no MSI-X, //! causes forced through `ICS` counted on the claim's vector unmasked and -//! masked, an asserted read of `ICR` under auto-mask, and then the driver -//! itself under traffic. Every reading is printed; the exit is whether each +//! masked, a storm of them with the record left unread, an asserted read of +//! `ICR` under auto-mask, and then the driver itself under traffic. Every reading is printed; the exit is whether each //! held. //! //! The PCH datasheet (631120 §8.1) and the 82574's (§10.2.4) are what each @@ -36,6 +36,16 @@ const RXSEQ: u32 = 1 << 3; const FORCES: u32 = 4; /// How long a forced cause is given to arrive, and a masked one to stay away. const WINDOW_NANOS: u64 = toyos_i219::DELIVERY_DEADLINE_NANOS; +/// Causes forced back to back with the claim's record left unread: a holder +/// making its function speak as fast as its own register writes go. +const STORM: u32 = 10_000; +/// The most of them that may land: the first never masks the claim's entry +/// and the second does, draining what the unit already remapped before its +/// end of interrupt, which a local APIC holds as one more request for the +/// vector beside the one in service (Intel SDM Vol. 3A, 325384-093, §13.8.4, +/// "Interrupt Acceptance for Fixed Interrupts"). Unmasked, every one lands: +/// the `ICR` read between forces outlasts the handler. +const STORM_BOUND: u32 = 3; /// How long the link is given to come up after the driver's reset. const LINK_NANOS: u64 = 15_000_000_000; /// How long frames are listened for after the DHCP discover. @@ -140,6 +150,24 @@ fn force(regs: &R, claim: &Claim, forced: u32, unmask: u32) -> (u3 (count, after, icr) } +/// `STORM` causes forced with `ITR` off and the record unread, each read off +/// `ICR` as asserted, then the messages that landed: the claim's interrupt +/// is masked from the second until this read. +fn storm(regs: &R, claim: &Claim) -> (u32, u32) { + regs.write32(regs::IMC, u32::MAX); + regs.write32(regs::ITR, 0); + let _ = regs.read32(regs::ICR); + let _ = claim.taken(); + regs.write32(regs::IMS, RXSEQ); + let mut asserted = 0; + for _ in 0..STORM { + regs.write32(regs::ICS, RXSEQ); + asserted += u32::from(regs.read32(regs::ICR) & (cause::INT_ASSERTED | RXSEQ) == cause::INT_ASSERTED | RXSEQ); + } + regs.write32(regs::IMC, u32::MAX); + (asserted, messages(claim, WINDOW_NANOS)) +} + /// One read of `ICR` with an interrupt asserted under auto-mask, as every /// pass of the driver makes it: the read, `IMS` after it, and a second read. /// §10.2.4.1's case 2 says the read clears `ICR` and writes `IAM` into `IMC`. @@ -290,6 +318,20 @@ fn main() { fail("a forced cause was not exactly one message unmasked and none masked"); } + let (asserted, landed) = storm(®s, &claim); + say(&format!("storm: {asserted} of {STORM} forced causes asserted, {landed} message(s) landed unread")); + if asserted != STORM { + fail("the storm did not assert every cause it forced"); + } + if landed == 0 || landed > STORM_BOUND { + fail(&format!("a storm landed {landed} message(s) on an unread claim, outside 1..={STORM_BOUND}")); + } + let (count, _, _) = force(®s, &claim, RXSEQ, RXSEQ); + say(&format!("after the storm's read, RXSEQ forced unmasked: {count} message(s)")); + if count != 1 { + fail("the read that took the storm's record did not open the claim's interrupt again"); + } + let (first, mask, second) = auto_masked_read(®s, &claim); say(&format!( "IAME on: the asserted ICR read {first:#010x}, IMS after it {mask:#010x}, the second read {second:#010x}" diff --git a/tests/toyos.rs b/tests/toyos.rs index 7df43da25b1..3711286314d 100644 --- a/tests/toyos.rs +++ b/tests/toyos.rs @@ -1546,8 +1546,10 @@ const METAL: &[(&str, metal::Metal)] = &[ ( // The I219's interrupts on the function itself, which QEMU's e1000e // is not: no MSI-X capability, causes forced through `ICS` counted on - // the claim's MSI vector unmasked and masked, `ICR`'s asserted read - // under auto-mask, and the driver under traffic on the bench's wire. + // the claim's MSI vector unmasked and masked, a storm of them stopped + // at the claim's entry until its record is read, `ICR`'s asserted + // read under auto-mask, and the driver under traffic on the bench's + // wire. // It shares the boot with `claim_reuses_its_remapping_entry`, whose // judge reads only its own job's records. "i219_messages_on_its_msi_vector",