diff --git a/issues/hardware/toyos-i219-refuses-a-part-outside-msi-x-mode-at-ivar.md b/issues/hardware/toyos-i219-refuses-a-part-outside-msi-x-mode-at-ivar.md new file mode 100644 index 0000000000..6fa0988d5b --- /dev/null +++ b/issues/hardware/toyos-i219-refuses-a-part-outside-msi-x-mode-at-ivar.md @@ -0,0 +1,29 @@ +--- +status: open +kind: defect +opened: 2026-09-08 +--- + +# `toyos-i219` refuses a part outside MSI-X mode at `IVAR`, and the T14's I219 is one + +`toyos-i219`'s `open` writes §10.2.4.9's `IVAR` and reads it back, refusing a +part that does not take the write: + +``` +nic.regs.write(regs::IVAR, ivar::ALL_ON_VECTOR_ZERO); +nic.accepted(regs::IVAR, ivar::ALL_ON_VECTOR_ZERO)?; +``` + +§10.2.4.9 defines that register only "in MSI-X mode" and says nothing about what +a part outside it answers, so the read-back is a guess refused rather than a +guess driven on. + +The T14's `00:1f.6` is outside that mode: Linux's own reading of the same +function is `IR-PCI-MSI-0000:00:1f.6 … enp0s31f6` in `/proc/interrupts` and +`mode=msi` at `/sys/bus/pci/devices/0000:00:1f.6/msi_irqs/162`. So the driver may +refuse the part the moment a claim on it is granted, and what `IVAR` answers on +an MSI part has never been read: no hand-over of `00:1f.6` has reached the +driver yet, so nothing has run the write. + +Owned by the stage-2 I219 worker: the first `nic.accepted(regs::IVAR, …)` on the +bench either passes or names the register that has to be driven differently. diff --git a/issues/kernel/a-claimed-function-must-have-msi-x-and-the-i219-may-not.md b/issues/kernel/a-claimed-function-must-have-msi-x-and-the-i219-may-not.md deleted file mode 100644 index d23afcf223..0000000000 --- a/issues/kernel/a-claimed-function-must-have-msi-x-and-the-i219-may-not.md +++ /dev/null @@ -1,43 +0,0 @@ ---- -status: open -kind: finding -opened: 2026-09-07 ---- - -# A claimed function must publish MSI-X, and the I219 may not - -`kernel/src/pcidev/mod.rs`'s `bring_up` arms exactly one interrupt mechanism: - -``` -let entry = pci.enable_msix(VECTORS[slot]).ok_or(Refusal::NoMsix)?; -``` - -A function that publishes no MSI-X capability is refused by name, and `Bound` -holds an `Mmio` pointing at that function's one table entry so `tear_down` can -mask it. Every device this project has handed to a process so far is a virtio -function, and every one of those has MSI-X, so the refusal has never been -reached other than by `virtio_net_no_msix`'s deliberate `vectors=0`. - -**The kernel can already arm the other mechanism and nothing calls it.** -`PciDevice::enable_msi` exists in `kernel/src/drivers/pci.rs`, `toyos-pci`'s -`msi` module decodes the capability, and `iommu::remap_msi` is on that path -too. What is missing is `pcidev` choosing between them and a `Bound` that can -hold either — MSI has no per-entry table, so the masking `tear_down` does has -no counterpart and the capability's optional per-vector mask bit is what -stands in for it. - -Why it matters now: the ThinkPad T14's onboard NIC is an Intel I219 at -`00:1f.6`, and the e1000e family's PCH parts (I217/I218/I219) are documented as -MSI parts — Linux's `e1000e` sets `FLAG_HAS_MSIX` for the 82574 and 82583 and -for nothing else. **This has not been read off the laptop**, and it is the -first thing to check there: if `lspci -vv` on `00:1f.6` shows a `MSI-X` -capability the refusal never fires and nothing here is owed; if it shows only -`MSI`, netd's claim on `pci:8086:15fc` is refused `NoMsix`, netd exits, and -stage 2's metal half cannot run until this is built. - -`toyos-i219` writes §10.2.4.9's `IVAR` and reads it back, and refuses a part -that does not take the write by name — §10.2.4.9 defines the register only "in -MSI-X mode" and says nothing about what a part outside it answers. So an I219 -that is an MSI part is refused twice over: by `pcidev::bring_up` before the -driver runs, and by the driver if the claim is ever granted. Both refusals name -what to read off the laptop. diff --git a/issues/kernel/nothing-asserts-that-a-claim-answers-no-configuration-write.md b/issues/kernel/nothing-asserts-that-a-claim-answers-no-configuration-write.md new file mode 100644 index 0000000000..1bcb3c4c86 --- /dev/null +++ b/issues/kernel/nothing-asserts-that-a-claim-answers-no-configuration-write.md @@ -0,0 +1,24 @@ +--- +status: open +kind: tooling +opened: 2026-09-08 +--- + +# Nothing asserts that a claim answers no configuration write + +`SYS_DEVICE_REG_WRITE` on a `RegTarget::PciConfig` target is refused +`NotSupported` in `kernel/src/arch/syscall/device.rs`, and no test in any tier +reads that refusal. It is what a handed-over MSI function's safety rests on: its +message address and data are words of configuration space rather than a table in +a BAR, so nothing is withheld from the holder and the whole of the boundary is +that the write path does not exist. A one-field mutation there — the arm +answering `Ok` — hands the holder the ability to aim the device's write at any +address the LAPIC decodes, and every arm in every tier stays green. + +The SDK's `PciDev` offers `config_read` and no write, so a driver cannot express +the call without reaching past it into `toyos_abi::syscall`. + +Owned by whoever next adds a boot config with a test binary holding a claimable +function. Exit condition: a guest arm in which the holder calls +`SYS_DEVICE_REG_WRITE` on its own claim and the kernel refuses it, red against a +kernel whose `PciConfig` write arm answers `Ok`. diff --git a/issues/kernel/nothing-reaches-the-msi-arm-of-a-claimed-function.md b/issues/kernel/nothing-reaches-the-msi-arm-of-a-claimed-function.md new file mode 100644 index 0000000000..c5a779bff0 --- /dev/null +++ b/issues/kernel/nothing-reaches-the-msi-arm-of-a-claimed-function.md @@ -0,0 +1,34 @@ +--- +status: open +kind: tooling +opened: 2026-09-08 +--- + +# Nothing reaches the MSI arm of a claimed function + +`pcidev::bring_up` arms a claimed function on MSI where it publishes no MSI-X, +and no test in any tier arms one. `virtio_net_no_msix` calls +`PciDevice::enable_msi` from `bring_up` and reads false back; nothing reaches a +true, and so nothing reaches: + +- `PciDevice::disable_msi` from either hand-back site (`bring_up`'s `place_bars` + failure and `tear_down`), or `Armed::Msi`'s teardown, which turns the + capability off where there is no table entry to mask; +- `Refusal::MsixUnusable` and `Unarmed::Blocked`, owed only by a function that + publishes MSI-X this kernel cannot arm and by a unit that refuses the message; +- `Refusal::CapsTruncated` and `NoCapability::Truncated`, owed by a function + whose capability list ends at a link the spec forbids. The walk's half of that + decision is host-tested in `toyos-pci/src/caps.rs`; the kernel's refusal arm + is reached by nothing. + +The two pre-existing MSI armings in this kernel — xHCI's and HDA's +`arm_interrupt` — never disarm, so MSI teardown is exercised nowhere in the tree +at all. + +Owned by the network track's stage-2 I219 worker. Exit condition: the first +`userdev` interrupt counted against a claim on `00:1f.6` on the bench, which +needs the 32-bit BAR window before it, plus netd exiting from that claim, which +runs `tear_down`'s MSI arm. A guest exit is the alternative and costs more: an +actuator that hides a function's MSI-X capability from the claim path, a boot +config whose own test binary holds a claimable function, and the tier row and CI +price of the boot that carries them. diff --git a/kernel/src/drivers/hda.rs b/kernel/src/drivers/hda.rs index d56e635b23..1b22527d0d 100644 --- a/kernel/src/drivers/hda.rs +++ b/kernel/src/drivers/hda.rs @@ -622,7 +622,7 @@ fn probe(pci: &PciDevice) -> Option<(Mmio, u16, u16)> { /// Put the function in D0 if firmware left it lower; D3hot reads all ones, indistinguishable from /// an absent controller. fn power_up(pci: &PciDevice) { - let Some(cap) = pci.capabilities().find(|c| c.id() == CAP_POWER_MANAGEMENT) else { + let Ok(cap) = pci.capability(CAP_POWER_MANAGEMENT) else { return; }; let pmcsr = cap.read_u16(PM_CONTROL_STATUS); @@ -663,7 +663,7 @@ fn reset_stream(stream: Mmio) -> bool { /// panic, over a peripheral. fn arm_interrupt(pci: &PciDevice) -> bool { let vector = crate::arch::idt::HDA_VECTOR; - if pci.enable_msix(vector).is_some() || pci.enable_msi(vector) { + if pci.enable_msix(vector).is_ok() || pci.enable_msi(vector) { return true; } log!( diff --git a/kernel/src/drivers/pci.rs b/kernel/src/drivers/pci.rs index 407c5ab1d8..58fe18809d 100644 --- a/kernel/src/drivers/pci.rs +++ b/kernel/src/drivers/pci.rs @@ -30,6 +30,26 @@ const MSG_ADDR: u32 = 0xFEE0_0000; // The same CPU, named as a destination rather than encoded in an address, for the unit to put in an entry. const MSG_DEST: u32 = 0; +/// Why a walk of a function's capability list answered no capability. +pub enum NoCapability { + /// The walk reached the list's terminator and nothing on it carried the id. + Absent, + /// The walk ended at a link the spec forbids, so whether the function + /// publishes the capability past it was never read. + Truncated, +} + +/// Why [`PciDevice::enable_msix`] armed nothing. +pub enum Unarmed { + /// No MSI-X capability came off the walk, which is a table this function + /// does not have only where the walk reached the list's terminator. + NoTable(NoCapability), + /// It publishes one whose table this kernel could not reach. + Unusable, + /// The unit refuses this function's message, and MSI would carry the same one. + Blocked, +} + pub struct Capability<'a> { device: &'a PciDevice, offset: u64, @@ -224,17 +244,16 @@ impl PciDevice { /// /// Answers the entry's own window, which stays this kernel's: masking is a /// write to it, and a claimant that could reach it could aim the device's - /// message at any address the LAPIC decodes. `None` is MSI-X that could not - /// be armed. - pub fn enable_msix(&self, vector: u8) -> Option { - let cap = self.capabilities().find(|c| c.id() == msix::CAP_ID)?; + /// message at any address the LAPIC decodes. + pub fn enable_msix(&self, vector: u8) -> Result { + let cap = self.capability(msix::CAP_ID).map_err(Unarmed::NoTable)?; let control = cap.read_u16(msix::MESSAGE_CONTROL); let table = match msix::Msix::decode(control, cap.read_u32(msix::TABLE)) { Ok(table) => table, Err(why) => { log!("PCI {:02x}:{:02x}.{}: MSI-X not armed, {}", self.bus, self.dev, self.func, why); - return None; + return Err(Unarmed::Unusable); } }; // Decoded, not assumed memory: a device may name a BAR that is an I/O BAR. @@ -243,7 +262,7 @@ impl PciDevice { Err(why) => { log!("PCI {:02x}:{:02x}.{}: MSI-X not armed, its table names BAR {} and {}", self.bus, self.dev, self.func, table.bir(), why); - return None; + return Err(Unarmed::Unusable); } }; let address = match table.table_address(base) { @@ -251,11 +270,11 @@ impl PciDevice { Err(why) => { log!("PCI {:02x}:{:02x}.{}: MSI-X not armed, {}", self.bus, self.dev, self.func, why); - return None; + return Err(Unarmed::Unusable); } }; - let (message, data) = self.message(vector)?; + let (message, data) = self.message(vector).ok_or(Unarmed::Blocked)?; let entry = address + MSIX_ENTRY as u64 * msix::ENTRY_BYTES; let table = crate::mm::paging::map_mmio(entry, 0x1000, MmioPolicy::Uncacheable); @@ -271,14 +290,14 @@ impl PciDevice { table.read_u32(msix::ENTRY_ADDRESS_LO), table.read_u32(msix::ENTRY_DATA), ); - Some(table) + Ok(table) } /// Put MSI-X back off, for a hand-over that armed a vector and was then /// refused: a function left enabled at a vector nobody holds delivers into /// a slot with no reader. pub fn disable_msix(&self) { - let Some(cap) = self.capabilities().find(|c| c.id() == msix::CAP_ID) else { return }; + let Ok(cap) = self.capability(msix::CAP_ID) else { return }; let control = cap.read_u16(msix::MESSAGE_CONTROL); cap.write_u16(msix::MESSAGE_CONTROL, msix::Msix::disabled(control)); } @@ -310,7 +329,7 @@ impl PciDevice { /// Point this function's single MSI message at `vector` and enable it. pub fn enable_msi(&self, vector: u8) -> bool { - let Some(cap) = self.capabilities().find(|c| c.id() == msi::CAP_ID) else { + let Ok(cap) = self.capability(msi::CAP_ID) else { return false; }; @@ -337,6 +356,28 @@ impl PciDevice { true } + /// Put MSI back off: the counterpart of [`Self::disable_msix`]. + /// + /// The per-vector mask an arming cleared stays clear: a function whose Mask + /// bit this set would owe a message on the set-to-clear transition a later + /// arming makes of it, with its Pending bit set (PCIe §7.7.1.7). + pub fn disable_msi(&self) { + let Ok(cap) = self.capability(msi::CAP_ID) else { return }; + let control = cap.read_u16(msi::MESSAGE_CONTROL); + cap.write_u16(msi::MESSAGE_CONTROL, msi::Msi::disabled(control)); + } + + /// Every reader that asks a function what it has asks here, so no reader + /// can mistake a list that ended early for one that named nothing. + pub fn capability(&self, id: u8) -> Result, NoCapability> { + let mut walk = self.capabilities(); + match walk.find(|cap| cap.id() == id) { + Some(cap) => Ok(cap), + None if walk.walk.truncated() => Err(NoCapability::Truncated), + None => Err(NoCapability::Absent), + } + } + pub fn capabilities(&self) -> CapabilityIter<'_> { let first = self.mmio.read_u8(CAPABILITIES_PTR); CapabilityIter { device: self, walk: caps::CapWalk::new(), next: first } diff --git a/kernel/src/drivers/virtio_sound.rs b/kernel/src/drivers/virtio_sound.rs index 955ddd4b4a..8f1cf4096a 100644 --- a/kernel/src/drivers/virtio_sound.rs +++ b/kernel/src/drivers/virtio_sound.rs @@ -483,7 +483,7 @@ fn build_chains( /// every period in flight forever. fn arm_interrupt(pci: &PciDevice, device: &VirtioDevice) -> bool { let vector = crate::arch::idt::VIRTIO_SOUND_VECTOR; - if pci.enable_msix(vector).is_none() { + if pci.enable_msix(vector).is_err() { log!( "virtio-sound: NOT INITIALISED at PCI {:02x}:{:02x}.{} — its MSI-X could not be \ armed and this driver has no other way to be told a period completed", diff --git a/kernel/src/drivers/xhci/wait/boot.rs b/kernel/src/drivers/xhci/wait/boot.rs index 4bd865f989..418a44eef4 100644 --- a/kernel/src/drivers/xhci/wait/boot.rs +++ b/kernel/src/drivers/xhci/wait/boot.rs @@ -98,7 +98,7 @@ fn await_connect_settle(controllers: &[XhciController]) { // `None` must stay a refusal, never a degradation: there is no polled mode, and // every event-ring read depends on `irq_ring`, which only the ISR sets. fn arm_interrupt(pci_dev: &PciDevice) -> Option<&'static str> { - if pci_dev.enable_msix(XHCI_VECTOR).is_some() { + if pci_dev.enable_msix(XHCI_VECTOR).is_ok() { return Some("MSI-X"); } pci_dev.enable_msi(XHCI_VECTOR).then_some("MSI") diff --git a/kernel/src/pcidev/mod.rs b/kernel/src/pcidev/mod.rs index ea8d9f5813..55f6ab93d4 100644 --- a/kernel/src/pcidev/mod.rs +++ b/kernel/src/pcidev/mod.rs @@ -3,11 +3,12 @@ //! The line through the device is **who can name an address**. This module //! keeps config space — there is no write path to it from userland — puts the //! function in an address space of its own at the unit *before* it enables bus -//! mastering, programs the interrupt vector into its MSI-X table, and hands out -//! every device address a descriptor may carry. Nothing the holder writes into -//! a descriptor can make the device touch memory the kernel did not grant it: -//! the domain maps the grants and nothing else, and an address outside them is -//! refused at the unit and recorded against that claim. +//! mastering, programs the interrupt vector into whichever of the function's two +//! message mechanisms it has, and hands out every device address a descriptor +//! may carry. Nothing the holder writes into a descriptor can make the device +//! touch memory the kernel did not grant it: the domain maps the grants and +//! nothing else, and an address outside them is refused at the unit and +//! recorded against that claim. //! //! **A window is 2 MiB because that is the only page this kernel maps.** A BAR //! a process may see is re-assigned onto a 2 MiB boundary above everything @@ -16,7 +17,13 @@ //! //! **The BAR holding the MSI-X table or PBA is never mapped**: a holder that //! could rewrite the table could point the device's message at any address the -//! LAPIC decodes. +//! LAPIC decodes. **So a function is armed on MSI only where a walk that +//! reached its capability list's terminator found no MSI-X**: its message is +//! then a word of config space, which has no write path from userland, and it +//! has no table in a BAR for [`msix_bar`] to keep back. A list that ends at a +//! link the spec forbids says nothing about what it publishes past that link, +//! so it is refused by name rather than armed on the mechanism the walk +//! happened to reach. //! //! **A function with no address space of its own is not handed over**, because //! every grant would answer with a physical address and a descriptor holding @@ -31,16 +38,6 @@ //! advertises one (PCIe §6.6.2), which no device in reach does — so the order //! above is the mechanism and the reset is the belt. //! -//! **What is read back, and what is not.** `Owned` is -//! `pci_function_is_exclusive`, `NoMsix` is `virtio_net_no_msix`, -//! `Untranslated` is `iommu_virtio_platform`'s no-unit arm, the domain is -//! `userdev_dma_fault`, and `SYS_DEVICE_REG_READ`'s bound is netd's own -//! `config_space_is_bounded`. `Ambiguous`, `KernelDriven`, `Exhausted`, every -//! window refusal, and every bound `SYS_DEVICE_BAR_MAP` and -//! `SYS_DEVICE_DMA_ALLOC` check are refused here and read by nothing: a -//! registration of them waits on a boot config whose own test binary holds a -//! claimable function, and netd holds this machine's only one. -//! //! Nothing here is specific to what a function *is*. /// No `crate::` reference, so `kernel-loom` compiles it and models the @@ -58,7 +55,7 @@ use toyos_dma::Register; use toyos_pci::{bar, express, msix}; use crate::device::{Claim, ClaimError}; -use crate::drivers::pci::PciDevice; +use crate::drivers::pci::{NoCapability, PciDevice, Unarmed}; use crate::inbox::InboxId; use crate::iommu::{DeviceSpace, IommuError}; use crate::mm::paging::{CachePolicy, MmioPolicy}; @@ -121,12 +118,19 @@ struct Grant { bytes: u64, } +/// How a claimed function was made to speak. Both deliver [`VECTORS`]`[slot]` +/// into the same [`Interrupt`] and the claim answers the same handle either way. +enum Armed { + /// This function's one MSI-X table entry, mapped for the kernel alone. + Msix(Mmio), + Msi, +} + /// What a live slot drives. The ISR never reads this. struct Bound { pci: PciDevice, space: DeviceSpace, - /// This function's one MSI-X table entry, mapped for the kernel alone. - entry: Mmio, + armed: Armed, id: PciId, /// Where each mappable BAR was put, and how much of it the function /// advertises; 0 bytes is a slot with no BAR this claim may map. @@ -329,7 +333,9 @@ fn window(assigned: u64, ceiling: u64) -> (u64, u64) { /// place. #[derive(Clone, Copy, PartialEq, Eq, Debug)] enum Refusal { - NoMsix, + NoInterrupt, + MsixUnusable, + CapsTruncated, Untranslated(IommuError), NoWindow, BarUnsizable(u8), @@ -341,10 +347,20 @@ enum Refusal { impl core::fmt::Display for Refusal { fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result { match self { - Self::NoMsix => write!( + Self::NoInterrupt => write!( + f, + "neither its MSI-X nor its MSI could be armed, and a claim with no interrupt \ + is a driver that would never be told anything" + ), + Self::MsixUnusable => write!( f, - "its MSI-X could not be armed, and a claim with no interrupt is a driver \ - that would never be told anything" + "it publishes MSI-X and this kernel could not arm it, and MSI is not a fallback \ + for a function that has a table" + ), + Self::CapsTruncated => write!( + f, + "its capability list ends at a link the PCI spec forbids, so whether it holds \ + an MSI-X table in a BAR was never read, and MSI is not armed on a guess" ), Self::Untranslated(why) => write!( f, @@ -475,11 +491,18 @@ fn bring_up(pci: PciDevice, id: PciId, slot: usize) -> Result { // Decode must be on for a BAR to answer, and off across each move. pci.enable_memory_space(); - // Then the interrupt, still before a window is cut: a function whose MSI-X - // cannot be armed is one no holder could ever be told anything about, and - // `virtio_net_no_msix` reads that refusal off the console *and* the absence - // of any BAR line after it. - let entry = pci.enable_msix(VECTORS[slot]).ok_or(Refusal::NoMsix)?; + // Then the interrupt, still before a window is cut: a function neither + // mechanism can be armed on is one no holder could ever be told anything + // about. + let armed = match pci.enable_msix(VECTORS[slot]) { + Ok(entry) => Armed::Msix(entry), + Err(Unarmed::Unusable) => return Err(Refusal::MsixUnusable), + Err(Unarmed::Blocked) => return Err(Refusal::NoInterrupt), + Err(Unarmed::NoTable(NoCapability::Truncated)) => return Err(Refusal::CapsTruncated), + Err(Unarmed::NoTable(NoCapability::Absent)) => { + pci.enable_msi(VECTORS[slot]).then_some(Armed::Msi).ok_or(Refusal::NoInterrupt)? + } + }; // From here a refusal has to undo: a vector is armed, and the arms below // move the function's BARs. @@ -489,7 +512,7 @@ fn bring_up(pci: PciDevice, id: PciId, slot: usize) -> Result { Ok(Bound { pci, space, - entry, + armed, id, bar_at, bar_bytes, @@ -499,7 +522,10 @@ fn bring_up(pci: PciDevice, id: PciId, slot: usize) -> Result { }) } Err(why) => { - pci.disable_msix(); + match armed { + Armed::Msix(_) => pci.disable_msix(), + Armed::Msi => pci.disable_msi(), + } Err(why) } } @@ -555,7 +581,7 @@ fn slot_space(slot: usize) -> Result { /// has in reach does, so nothing rests on this: what makes a re-claim safe is /// that bus mastering starts on the first grant and not at hand-over. fn reset(pci: &PciDevice) -> Option { - let cap = pci.capabilities().find(|c| c.id() == express::CAP_ID)?; + let cap = pci.capability(express::CAP_ID).ok()?; if !express::resets(cap.read_u32(express::DEVICE_CAPABILITIES)) { return None; } @@ -587,7 +613,7 @@ fn settle_after_reset(pci: &PciDevice) { /// the two live in one BAR on every device in reach, and a device that split /// them costs the second BAR too rather than publishing one of them. fn msix_bar(pci: &PciDevice) -> Option { - let cap = pci.capabilities().find(|c| c.id() == msix::CAP_ID)?; + let cap = pci.capability(msix::CAP_ID).ok()?; let control = cap.read_u16(msix::MESSAGE_CONTROL); let table = msix::Msix::decode(control, cap.read_u32(msix::TABLE)).ok()?; Some(table.bir()) @@ -732,7 +758,10 @@ pub fn release(slot: usize) { fn tear_down(slot: usize, bound: Bound) { bound.pci.disable_bus_master(); - bound.entry.write_u32(msix::ENTRY_VECTOR_CONTROL, msix::ENTRY_MASKED); + match &bound.armed { + Armed::Msix(entry) => entry.write_u32(msix::ENTRY_VECTOR_CONTROL, msix::ENTRY_MASKED), + Armed::Msi => bound.pci.disable_msi(), + } crate::iommu::note_user_owned(bound.pci.bus, bound.pci.dev, bound.pci.func, None); for grant in bound.grants.iter() { if let Err(why) = bound.space.unmap(grant.at, grant.bytes) { diff --git a/tests/common/faults.rs b/tests/common/faults.rs index c1338bad02..c4b1e5a3ec 100644 --- a/tests/common/faults.rs +++ b/tests/common/faults.rs @@ -212,12 +212,6 @@ pub fn idle_stack_guard( /// A NIC that cannot raise an interrupt must cost the machine networking and /// nothing else. /// -/// The MSI-X setup was written out three times and the copies answered this -/// question three different ways: the xHCI driver fell back to MSI, and both -/// virtio drivers called `panic!`. So the one device on the bus with no way to -/// deliver a packet took down a kernel whose disk, console, audio and USB were -/// all working — class M1 again, on the mechanism M1's own fix went through. -/// /// The other two virtio functions keep their vectors, which is what makes the /// verdict mean anything: the console that carries the refusal and the audio /// device beside it are on the same bus, driven by the same code, and neither @@ -278,14 +272,8 @@ pub fn virtio_net_no_msix() -> Result<(), String> { // Refused by name, at a named function, and not by claiming a mode it does // not have: the xHCI driver's `polled mode` line is the defect this whole // family exists to keep out of the tree. - // - // **The refusal moved with the driver.** It used to be the kernel's own - // virtio-net `init` giving up; it is now the *claim* being refused, before - // any driver exists — a function whose interrupt cannot be armed is one - // whose holder would never be told anything, and handing it over anyway - // would be handing out a device that looks alive and never speaks. log.must_say("pcidev: PCI 00:03.0 NOT HANDED OVER")?; - log.must_say("its MSI-X could not be armed")?; + log.must_say("neither its MSI-X nor its MSI could be armed")?; log.must_not_say("[1af4:1041] handed over")?; // And the refusal is the *whole* of it: no BAR moved for a function nobody // can be given one. diff --git a/tests/common/iommu.rs b/tests/common/iommu.rs index c6768648b9..67970638b4 100644 --- a/tests/common/iommu.rs +++ b/tests/common/iommu.rs @@ -806,12 +806,7 @@ fn no_unit_is_no_claim(log: &Serial) -> Result<(), String> { Ok(()) } -/// The two lines a hand-over spends, on the function `netcase` claims. -/// -/// Named once because both arms of `iommu_virtio_platform` read them, in -/// opposite directions: the arm with a unit requires them and the arm without -/// one requires their absence. An absence nothing ever produces would pass -/// against a kernel that had stopped writing the line at all. +/// The two lines a hand-over spends: one arm requires them, the other their absence. const BAR_MOVED: &str = "pcidev: PCI 00:03.0 BAR"; const MSIX_ARMED: &str = "PCI 00:03.0: msix address="; diff --git a/toyos-pci/src/caps.rs b/toyos-pci/src/caps.rs index 3ab605b555..1d69d7be03 100644 --- a/toyos-pci/src/caps.rs +++ b/toyos-pci/src/caps.rs @@ -11,26 +11,39 @@ pub const FIRST_CAP: u8 = 0x40; #[derive(Debug, Default)] pub struct CapWalk { seen: [u64; 4], + truncated: bool, } impl CapWalk { pub const fn new() -> Self { - Self { seen: [0; 4] } + Self { seen: [0; 4], truncated: false } } /// The next capability's offset, or `None` to end the walk: the terminator /// (0), a pointer the spec forbids, or one already visited (a cycle). pub fn step(&mut self, raw: u8) -> Option { - if raw == 0 || raw < FIRST_CAP || raw & 0x3 != 0 { + if raw == 0 { + return None; + } + if raw < FIRST_CAP || raw & 0x3 != 0 { + self.truncated = true; return None; } let (word, bit) = ((raw >> 6) as usize, 1u64 << (raw & 0x3F)); if self.seen[word] & bit != 0 { + self.truncated = true; return None; } self.seen[word] |= bit; Some(raw) } + + /// Whether the walk ended at a link the spec forbids rather than at the + /// terminator: nothing past that link was read, so what the function + /// publishes past it is unknown and never absent. + pub const fn truncated(&self) -> bool { + self.truncated + } } #[cfg(test)] @@ -39,14 +52,18 @@ mod tests { #[test] fn the_terminator_ends_the_walk() { - assert_eq!(CapWalk::new().step(0), None); + let mut w = CapWalk::new(); + assert_eq!(w.step(0), None); + assert!(!w.truncated()); } /// PCI spec §6.7: a capability pointer is dword-aligned. #[test] fn a_pointer_that_is_not_dword_aligned_is_refused() { for raw in [0x41u8, 0x42, 0x43, 0x4F, 0xFD, 0xFE, 0xFF] { - assert_eq!(CapWalk::new().step(raw), None, "{raw:#x}"); + let mut w = CapWalk::new(); + assert_eq!(w.step(raw), None, "{raw:#x}"); + assert!(w.truncated(), "{raw:#x}"); } } @@ -54,7 +71,9 @@ mod tests { #[test] fn a_pointer_below_the_standard_header_is_refused() { for raw in [0x04u8, 0x20, 0x3C] { - assert_eq!(CapWalk::new().step(raw), None, "{raw:#x}"); + let mut w = CapWalk::new(); + assert_eq!(w.step(raw), None, "{raw:#x}"); + assert!(w.truncated(), "{raw:#x}"); } assert_eq!(CapWalk::new().step(FIRST_CAP), Some(FIRST_CAP)); } @@ -66,6 +85,7 @@ mod tests { assert_eq!(w.step(0x50), Some(0x50)); assert_eq!(w.step(0xF8), Some(0xF8)); assert_eq!(w.step(0), None); + assert!(!w.truncated()); } /// A visited set, not an "increasing" test: a list may be laid out out of @@ -75,11 +95,13 @@ mod tests { let mut w = CapWalk::new(); assert_eq!(w.step(0x40), Some(0x40)); assert_eq!(w.step(0x40), None); + assert!(w.truncated()); let mut w = CapWalk::new(); assert_eq!(w.step(0x60), Some(0x60)); assert_eq!(w.step(0x50), Some(0x50)); assert_eq!(w.step(0x60), None); + assert!(w.truncated()); } #[test] diff --git a/toyos-pci/src/msi.rs b/toyos-pci/src/msi.rs index db05f0ea90..0f9d3b3faa 100644 --- a/toyos-pci/src/msi.rs +++ b/toyos-pci/src/msi.rs @@ -79,6 +79,12 @@ impl Msi { pub fn enabled(message_control: u16) -> u16 { (message_control & !MULTI_MESSAGE_ENABLE) | ENABLE } + + /// Message Control with the function delivering nothing, and everything it + /// said about itself left alone. + pub fn disabled(message_control: u16) -> u16 { + message_control & !ENABLE + } } #[cfg(test)] @@ -135,4 +141,10 @@ mod tests { let ctrl = ADDRESS_64 | PER_VECTOR_MASK | (5 << 1); assert_eq!(Msi::enabled(ctrl), ctrl | ENABLE); } + + #[test] + fn disabling_clears_the_enable_bit_and_nothing_else() { + let ctrl = ADDRESS_64 | PER_VECTOR_MASK | MULTI_MESSAGE_ENABLE | (5 << 1); + assert_eq!(Msi::disabled(ctrl | ENABLE), ctrl); + } } diff --git a/userland/netd/src/i219.rs b/userland/netd/src/i219.rs index 5f960b98c4..93cc901eb7 100644 --- a/userland/netd/src/i219.rs +++ b/userland/netd/src/i219.rs @@ -150,9 +150,6 @@ impl Nic { .describe() .map_err(KernelRefused::on("the claim's description")) .map_err(Opening::Kernel)?; - // The register file is in BAR 0 on every part of this family; the - // lowest BAR the claim will map is taken rather than assumed, because - // the kernel reports 0 bytes for one it keeps — the MSI-X table's. let (bar, bytes) = info .bar_bytes .iter()