diff --git a/Cargo.lock b/Cargo.lock index 4df80a99005..bfd200834ab 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3343,6 +3343,7 @@ dependencies = [ "toyos-net-tcp", "toyos-net-udp", "toyos-net-wire", + "toyos-pci-claim", "toyos-tco", "toyos-virtio", ] @@ -5158,6 +5159,9 @@ dependencies = [ "toyos-hda", "toyos-inspect", "toyos-mixer", + "toyos-pci-claim", + "toyos-virtio", + "toyos-virtio-sound", ] [[package]] @@ -5911,6 +5915,16 @@ dependencies = [ "toyos-abi", ] +[[package]] +name = "toyos-pci-claim" +version = "0.1.0" +dependencies = [ + "toyos", + "toyos-abi", + "toyos-device-memory", + "toyos-virtio", +] + [[package]] name = "toyos-phys" version = "0.1.0" @@ -6032,6 +6046,14 @@ dependencies = [ "toyos-untrusted", ] +[[package]] +name = "toyos-virtio-sound" +version = "0.1.0" +dependencies = [ + "toyos-device-memory", + "toyos-virtio", +] + [[package]] name = "toyos-wallclock" version = "0.1.0" diff --git a/Cargo.toml b/Cargo.toml index f0da1b65531..7331f685d52 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -108,6 +108,7 @@ members = [ "userland/snake", "userland/soundserver", "userland/soundserver/mixer", + "userland/soundserver/virtio-sound", "userland/sprite", "userland/sshserver", "userland/supervisor", @@ -117,6 +118,7 @@ members = [ "userland/test-runner", "userland/toybox", "userland/toyos-font", + "userland/toyos-pci-claim", "userland/toyos-window", "userland/trace", "userland/update", diff --git a/issues/a-class-claims-call-in-flight-acts-after-the-claims-release.md b/issues/a-class-claims-call-in-flight-acts-after-the-claims-release.md index 62367bf3cee..00e946a2c00 100644 --- a/issues/a-class-claims-call-in-flight-acts-after-the-claims-release.md +++ b/issues/a-class-claims-call-in-flight-acts-after-the-claims-release.md @@ -6,14 +6,14 @@ opened: 2026-10-08 # A class claim's call in flight acts after the claim's release -A claim on a class this kernel still drives — the framebuffer, HDA audio, -virtio-sound — is a per-class flag (`device::TAKEN`), and a call on one checks +A claim on a class this kernel still drives — the framebuffer, HDA audio — is +a per-class flag (`device::TAKEN`), and a call on one checks the handle and then acts with nothing held across the two: - `SYS_DEVICE_REG_READ` and `SYS_DEVICE_REG_WRITE` resolve the claim to - `RegTarget::Hda` or `RegTarget::VirtioSound` under the process-data lock - (`sys_device_reg`, `kernel/src/syscall/device.rs`) and reach - `drivers::hda::reg_write` or `drivers::virtio_sound::reg_write` after it; + `RegTarget::Hda` under the process-data lock (`sys_device_reg`, + `kernel/src/syscall/device.rs`) and reach `drivers::hda::reg_write` after + it; - `SYS_GPU_PRESENT`, `SYS_GPU_SET_CURSOR`, `SYS_GPU_MOVE_CURSOR` and `SYS_GPU_SET_RESOLUTION` ask `holds_claim` and then call `crate::gpu` (`kernel/src/syscall/dispatch.rs`). diff --git a/issues/diskservers-nvme-bring-up-open-codes-what-toyos-pci-claim-holds.md b/issues/diskservers-nvme-bring-up-open-codes-what-toyos-pci-claim-holds.md new file mode 100644 index 00000000000..f85a5450d3d --- /dev/null +++ b/issues/diskservers-nvme-bring-up-open-codes-what-toyos-pci-claim-holds.md @@ -0,0 +1,18 @@ +--- +status: open +kind: defect +opened: 2026-10-10 +--- + +# diskserver's NVMe bring-up open-codes what `toyos-pci-claim` holds + +`Controller::open` (`userland/diskserver/src/nvme.rs`) maps BAR 0 with +`PciDev::map_bar` and an `unsafe Window::new` over it, allocates its grant with +`PciDev::dma_alloc` and another `unsafe Window::new`, and names each kernel +refusal as `Refusal::Kernel(call, e)`. That is `Bar::map`, `Grant::alloc` and +`KernelRefused` of `userland/toyos-pci-claim/src/lib.rs`, which netstack's two +drivers and soundserver's virtio-sound driver use: two more `unsafe` sites +whose argument is the one the crate already makes. + +Exit: diskserver reaches its BAR and grant through `toyos-pci-claim`, or the +reason it cannot is at the crate's module header. diff --git a/issues/every-driver-is-still-in-the-kernel.md b/issues/every-driver-is-still-in-the-kernel.md index 0e39f881f5b..9151fb598ff 100644 --- a/issues/every-driver-is-still-in-the-kernel.md +++ b/issues/every-driver-is-still-in-the-kernel.md @@ -29,23 +29,32 @@ is not built. What is left of the staged work: -1. **Audio and virtio-gpu, re-scoped.** `drivers/hda.rs` and - `drivers/virtio_sound.rs` bring their device up and gate soundd's register - access; `drivers/virtio_gpu.rs` is the only `Gpu` whose `SYS_GPU_*` calls do - anything, since GOP's are all no-ops. Each leaves when its userland holder - claims the function as `pci`, as netd does, retiring the `hda-audio` and - `virtio-sound` classes, their arms of `SYS_DEVICE_REG_READ`/`WRITE`, and - `SYS_GPU_*`, which is an ABI change. GOP stays: it is memory the loader - hands over, and the panic console paints it. - A virtio holder stands on `toyos-virtio`, whose first client is netstack's - NIC, and the second one owes that crate two things. The walk of the - capability list moves into it from `userland/netstack/src/virtio_net.rs`, - over a configuration read the caller passes in, which closes - `issues/the-virtio-capability-walk-reads-a-refused-configuration-read-as-zeros.md`. - And before a client ends its device on `UsedRefusal::Written` for a chain +1. **HDA and virtio-gpu, re-scoped.** `drivers/hda.rs` brings its device up + and gates soundserver's register access; `drivers/virtio_gpu.rs` is the only + `Gpu` whose `SYS_GPU_*` calls do anything, since GOP's are all no-ops. Each + leaves when its userland holder claims the function as `pci`, as netstack + and soundserver's virtio-sound driver do, retiring the `hda-audio` class, + its arms of `SYS_DEVICE_REG_READ`/`WRITE`, and `SYS_GPU_*`, which is an ABI + change. GOP stays: it is memory the loader hands over, and the panic console + paints it. HDA leaving feeds soundserver's DLL from a claim's record on + every machine, so that diff lands + `issues/soundservers-virtio-clock-can-take-a-period-one-notification-early.md`'s + exit. + A virtio holder stands on `toyos-virtio`, which walks the capability list + too. Before a client ends its device on `UsedRefusal::Written` for a chain the device only reads, whose bound is 0, what QEMU's device reports as `len` on such a queue is measured: the NIC's transmit queue is the only one read so far. + `iommu_virtio_platform`'s decline control, `declining_is_not_free` + (`tests/common/iommu.rs`), declines through the kernel's + `virtio-no-access-platform` actuator, which reaches only a virtio function + the kernel drives; with the NIC and virtio-sound in processes that is the + virtio-gpu `Profile::HeadlessVirtioGpu` adds for it, and virtio-gpu leaving + leaves the control nothing to decline. Exit for the control, in the diff + that moves virtio-gpu out: the decline is made where the features are + negotiated, `toyos-virtio`'s `Offer::accept`, for a function a process + drives, and the control reds when that function keeps `FEATURES_OK`; + `Profile::HeadlessVirtioGpu` goes in the same diff. 2. Done: **BAR sizing and re-assignment onto 2 MiB boundaries** is `pcidev::place_bar`, with the overlap refusal kept as the assertion that it worked rather than as the mechanism. diff --git a/issues/soundservers-virtio-clock-can-take-a-period-one-notification-early.md b/issues/soundservers-virtio-clock-can-take-a-period-one-notification-early.md new file mode 100644 index 00000000000..4179c9f0e0c --- /dev/null +++ b/issues/soundservers-virtio-clock-can-take-a-period-one-notification-early.md @@ -0,0 +1,33 @@ +--- +status: open +kind: defect +opened: 2026-10-10 +--- + +# soundserver's virtio-sound clock can take a period one notification early + +`Sound::played` (`userland/soundserver/virtio-sound/src/lib.rs`) reads the +transmit queue's used ring only with the claim's interrupt record in hand, and +`Virtio::completion` (`userland/soundserver/src/virtio.rs`) hands the mix loop +that record's `last_nanos`. A record says when its notifications landed and +nothing about which period each was for. So when the device has written a +newer period's used element and its notification has not landed by the ring +read, that period is counted in the mask and stamped with the older +notification's time, and `dll.update(last_nanos, n)` in +`userland/soundserver/src/mix.rs` takes an update whose newest period is +clocked at least one period early: an error shaped like `n − 1` periods on +that update. Its notification then lands into the next record, which comes +back with no period. + +How often the window opens is unmeasured, and no metal machine runs +virtio-sound. Nothing bounds the error but that rarity. + +Owned by stage 1 of `issues/every-driver-is-still-in-the-kernel.md`: the +diff that moves HDA onto a `pci` claim feeds soundserver's DLL from a claim's +record (`kernel/src/pcidev/record.rs`) on every machine, and lands this exit +with it. Exit: the +record ties a time to the used elements it answers for — or the driver holds +back from a DLL update every period past those its record's notifications +answer for — and `toyos-virtio-sound`'s interleaving test, +`no_period_is_stranded_between_its_used_element_and_its_notification`, asserts +that no period comes back stamped earlier than its own notification. diff --git a/issues/the-virtio-capability-walk-reads-a-refused-configuration-read-as-zeros.md b/issues/the-virtio-capability-walk-reads-a-refused-configuration-read-as-zeros.md deleted file mode 100644 index a8cf43c6eaa..00000000000 --- a/issues/the-virtio-capability-walk-reads-a-refused-configuration-read-as-zeros.md +++ /dev/null @@ -1,35 +0,0 @@ ---- -status: open -kind: defect -opened: 2026-10-09 ---- - -# The virtio capability walk reads a refused configuration read as zeros - -`vendor_caps` (`userland/netstack/src/virtio_net.rs`) walks a function's -capability list through its claim, and two things in it believe more than they -were told: - -- A field of a vendor capability is read as - `dev.config_read(next + at, width).unwrap_or(0)`, so a read the kernel - refused becomes a `cfg_type`, `bar`, `offset` or `length` of 0. A capability - made of zeros is then refused or ignored by `toyos_virtio::pci::Layout::of` - under some other name (`MissingCap`, `TooShort`), and the kernel's word for - what happened is gone. -- The capabilities pointer and every `next` link are used as read. The *PCI - Local Bus Specification*, revision 3.0, section 6.7, reserves the bottom two - bits of each and has software mask them before using the value as an offset - (cited from memory: the document was not at hand when this was filed). - An unmasked pointer with either bit set is an offset no capability is at, - and the 32-bit reads at `next + 8` and on are then misaligned, which the - claim refuses and the first item turns into zeros. - -Neither reaches memory: the claim bounds every read to the function's own -configuration space (`config_space_is_bounded`). What they cost is a device -refused for the wrong reason. - -Owned by the second client of `toyos-virtio`, which moves the walk into the -crate (stage 1 of `issues/every-driver-is-still-in-the-kernel.md`). Exit: the -walk is the crate's, over a configuration read its caller passes in; a refused -read is a refusal by name and a link is masked before it is an offset; and a -host test is red against each of the two as they stand today. diff --git a/issues/three-kernel-byte-views-still-rest-on-a-hand-layout-claim.md b/issues/three-kernel-byte-views-still-rest-on-a-hand-layout-claim.md index 0c4f3f609fa..f531b782ebb 100644 --- a/issues/three-kernel-byte-views-still-rest-on-a-hand-layout-claim.md +++ b/issues/three-kernel-byte-views-still-rest-on-a-hand-layout-claim.md @@ -16,7 +16,7 @@ user memory through an `unsafe` `from_raw_parts` of the kernel's own whose |---|---|---| | `DmaGrant` | `grant_bytes`, `kernel/src/syscall/device.rs` | `size_of == 4 + 4 + 8 + 8`, a sum written by hand | | `DmaMapping` | inline in `sys_device_dma_map`, the same file | `size_of == 8 + 8`, a sum written by hand | -| `DeviceIrqRecord` | `record_bytes`, `kernel/src/object/ops.rs` | `SIZE == 4`, a total written by hand; the struct is one `u32` | +| `DeviceIrqRecord` | `record_bytes`, `kernel/src/object/ops.rs` | `SIZE == 4 + 4 + 8 + 8`, a sum written by hand | Read, nothing run: none of the three has padding today. Each assertion compares the struct's size with a number a person wrote, and nothing ties diff --git a/issues/toyos-runs-on-arm64.md b/issues/toyos-runs-on-arm64.md index 405332a519e..ddb43379c73 100644 --- a/issues/toyos-runs-on-arm64.md +++ b/issues/toyos-runs-on-arm64.md @@ -328,8 +328,9 @@ Each stage names its exit; "measured" means a number from a run. stage's SMMUv3 first. Stubbed on AArch64, each owned by the small-kernel track, which moves the driver out of the kernel: - `arch::msi_message` refuses, so the kernel's xHCI (`virt`'s boot stick), - HDA, virtio-sound, virtio-console and virtio-gpu drivers each - refuse their function by name. + HDA, virtio-console and virtio-gpu drivers each refuse their function by + name, and a process's claim on one (netstack's, soundserver's + virtio-sound) is refused with them. - `drivers::gop` refuses a scanout that is not whole 2 MiB pages of its own, which a `ramfb` scanout carved out of RAM need not be. diff --git a/issues/whether-a-region-may-be-lent-is-a-runtime-option.md b/issues/whether-a-region-may-be-lent-is-a-runtime-option.md index 37cd278d586..a0e3bc40a8e 100644 --- a/issues/whether-a-region-may-be-lent-is-a-runtime-option.md +++ b/issues/whether-a-region-may-be-lent-is-a-runtime-option.md @@ -11,10 +11,14 @@ may be lent a region by reading `Region::pages` — `Some` for pages the kernel allocated for the region, `None` for an aperture, firmware's framebuffer, or a pool a kernel driver owns — together with its cache policy. A new region kind that fills the field the wrong way is lendable, or refused, with no compile -error; the only check is `blockd_lends_within_its_bound`'s refusal of -virtio-sound's pool. And the field's name says nothing about lending: the -virtio-gpu scanout and cursor carry `Some` and are lendable today, which is the -same authority their holder already has but was decided by nobody. +error. The one test written for it, `blockd_lends_within_its_bound`, refused +virtio-sound's pool, which left the kernel with its driver; the test is out of +the guest suite and staged as a T14 row in +`issues/the-guest-suite-runs-only-what-no-cheaper-tier-reaches.md`'s stage J, +where the kernel driver's pool it can refuse is HDA's. And the field's name +says nothing about lending: the virtio-gpu scanout and cursor carry `Some` and +are lendable today, which is the same authority their holder already has but +was decided by nobody. **Exit condition.** A region's kind is a type — owned pages, a kernel driver's pool, an aperture — and `dma_map` takes only the owned-pages kind, so a region diff --git a/kernel/loom/tests/device_irq.rs b/kernel/loom/tests/device_irq.rs index e2c42d979bf..b098aebdb6a 100644 --- a/kernel/loom/tests/device_irq.rs +++ b/kernel/loom/tests/device_irq.rs @@ -2,15 +2,13 @@ //! //! The kernel programs one MSI-X vector per claimed function and accumulates //! what arrives into a record its holder reads through a syscall. So there are -//! two parties on two CPUs and one word between them: an ISR that bumps a -//! count, and a reader that takes it. +//! two parties on two CPUs: an ISR that counts a message and stamps when it +//! landed, and a reader that takes the count and its times. //! -//! **The invariant is that every message is counted exactly once.** Nothing -//! here orders anything else — the word is the whole of what it says, so the -//! orderings are `Relaxed` and the property is an interleaving. What makes it -//! hold is that the taking side is a read-modify-write: a reader that loaded a +//! **The invariant is that every message is counted exactly once, and a +//! read's times are its own messages'.** What makes the first hold is that +//! both sides change the count by a read-modify-write: a reader that loaded a //! count and then cleared it drops every message the ISR recorded in between. -//! //! That is the record's whole design, so the negative control is it turned off //! — a cargo feature rather than a comment: //! @@ -19,8 +17,8 @@ //! --test device_irq //! ``` //! -//! makes the `swap` a load and a store and the ISR's `fetch_add` a load, an -//! add and a store, and [`every_message_is_counted_once`] must red. +//! makes every compare-exchange a load and a store, and +//! [`every_message_is_counted_once`] must red. use kernel_loom::device_irq::Interrupt; use loom::sync::Arc; @@ -29,8 +27,8 @@ use loom::sync::Arc; /// /// Two messages against one reader that may run at any point between them: what /// the reader took plus what is left is exactly what arrived. A `store` where -/// the count has a `fetch_add` fails this, and so does a reader that loads and -/// then clears instead of swapping. +/// the count has a compare-exchange fails this, and so does a reader that +/// loads and then clears instead of exchanging. #[test] fn every_message_is_counted_once() { loom::model(|| { @@ -39,14 +37,14 @@ fn every_message_is_counted_once() { let isr = { let irq = irq.clone(); loom::thread::spawn(move || { - irq.took(); - irq.took(); + irq.took(1); + irq.took(2); }) }; - let taken = irq.take().unwrap_or(0); + let taken = irq.take().map_or(0, |record| record.count); isr.join().unwrap(); - let left = irq.take().unwrap_or(0); + let left = irq.take().map_or(0, |record| record.count); assert_eq!( taken + left, @@ -57,6 +55,58 @@ fn every_message_is_counted_once() { }); } +/// A read's first time is the landing of the first message it counts, and its +/// newest time no older than the newest it counts. +/// +/// Three messages, landing at 1, 2 and 3, against a reader that takes once at +/// any point among them and then takes what is left: whatever the split, each +/// read's `first_nanos` is its own first message's, and a message that lands +/// while the reader is between its exchange and its loads stamps the next +/// read and never this one. A single slot for both reads' first times fails +/// this, and so does a stamp made after the exchange that counts it. +#[test] +fn a_reads_times_are_its_own_messages() { + loom::model(|| { + let irq = Arc::new(Interrupt::new()); + + let isr = { + let irq = irq.clone(); + loom::thread::spawn(move || { + for at in 1..=3 { + irq.took(at); + } + }) + }; + + let taken = irq.take(); + isr.join().unwrap(); + let left = irq.take(); + + let counted = taken.map_or(0, |record| record.count); + if let Some(record) = taken { + assert_eq!(record.first_nanos, 1, "the first read counted from message 1 and was told {}", record.first_nanos); + assert!( + record.last_nanos >= u64::from(counted), + "a read that counted {counted} message(s) was told the newest landed at {}", + record.last_nanos + ); + } + match left { + None => assert_eq!(counted, 3, "the first read counted {counted} and nothing was left"), + Some(left) => { + assert_eq!( + left.first_nanos, + u64::from(counted) + 1, + "the second read counted from message {} and was told {}", + counted + 1, + left.first_nanos + ); + assert_eq!(left.last_nanos, 3, "the newest message landed at 3 and the last read was told {}", left.last_nanos); + } + } + }); +} + /// A reader that finds nothing answers nothing, and leaves nothing behind. /// /// Single-threaded and deliberately so: this is the answer on the path a @@ -68,9 +118,10 @@ fn an_idle_record_answers_nothing() { let irq = Interrupt::new(); assert!(irq.take().is_none(), "an untouched record answered a reader"); assert!(!irq.armed(), "an untouched record read ready"); - irq.took(); + irq.took(7); assert!(irq.armed(), "a record with a message in it read empty"); - assert_eq!(irq.take(), Some(1)); + 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"); }); diff --git a/kernel/src/arch/aarch64/irqchip.rs b/kernel/src/arch/aarch64/irqchip.rs index 08efbb0def0..a76feb9b094 100644 --- a/kernel/src/arch/aarch64/irqchip.rs +++ b/kernel/src/arch/aarch64/irqchip.rs @@ -45,7 +45,6 @@ pub(super) enum Intid { /// What `irq-storm` floods this CPU with. Storm, Hda, - VirtioSound, } pub(super) const SGI_KICK: u32 = Intid::Kick as u32; diff --git a/kernel/src/arch/aarch64/trap.rs b/kernel/src/arch/aarch64/trap.rs index 41881d73319..d324cfa9756 100644 --- a/kernel/src/arch/aarch64/trap.rs +++ b/kernel/src/arch/aarch64/trap.rs @@ -498,7 +498,6 @@ pub fn install() { } pub const HDA_VECTOR: u8 = irqchip::Intid::Hda as u8; -pub const VIRTIO_SOUND_VECTOR: u8 = irqchip::Intid::VirtioSound as u8; /// The crash report for a panic, from the frame pointer the panic handler /// stood on: the backtrace, which CPU is on which stack, and what the diff --git a/kernel/src/arch/x86_64/idt/mod.rs b/kernel/src/arch/x86_64/idt/mod.rs index 4194826b6ab..cc9e2fe1b8d 100644 --- a/kernel/src/arch/x86_64/idt/mod.rs +++ b/kernel/src/arch/x86_64/idt/mod.rs @@ -11,7 +11,6 @@ mod timer; mod tlb; pub(crate) mod unclaimed; mod user_dev; -mod virtio_sound; mod xhci; use core::arch::naked_asm; @@ -46,9 +45,6 @@ pub const DMA_FAULT_VECTOR: u8 = Vector::DmaFault as u8; /// The vector the HDA controller's message-signalled interrupt carries. pub const HDA_VECTOR: u8 = Vector::Hda as u8; -/// The vector the virtio-sound device's MSI-X entry carries. -pub const VIRTIO_SOUND_VECTOR: u8 = Vector::VirtioSound as u8; - const PF_PRESENT: u64 = 1 << 0; const PF_WRITE: u64 = 1 << 1; const PF_INSTRUCTION_FETCH: u64 = 1 << 4; @@ -260,7 +256,6 @@ idt_vectors! { ring0 Nmi = 0x02, nmi::nmi_entry, ist 2; ring3 Timer = 0x20, timer::timer_entry; ring3 Xhci = 0x21, xhci::xhci_entry; - ring3 VirtioSound = 0x23, virtio_sound::virtio_sound_entry; ring3 I8042 = 0x24, i8042::i8042_entry; ring3 DmaFault = 0x25, dma_fault::dma_fault_entry; ring3 Hda = 0x26, hda::hda_entry; diff --git a/kernel/src/arch/x86_64/idt/virtio_sound.rs b/kernel/src/arch/x86_64/idt/virtio_sound.rs deleted file mode 100644 index 6e3744d624a..00000000000 --- a/kernel/src/arch/x86_64/idt/virtio_sound.rs +++ /dev/null @@ -1,13 +0,0 @@ -use super::device_irq::device_irq_entry; - -// Heap-free, and takes only interrupts-off locks: may interrupt a CPU holding the controller lock, which disables preemption but not interrupts. -extern "sysv64" fn virtio_sound_handler() { - crate::arch::percpu::irq_took!(Sound); - crate::drivers::virtio_sound::isr_complete(); - crate::arch::apic::eoi(); -} - -device_irq_entry! { - /// Virtio-sound MSI-X entry (see `device_irq_entry` for the asm contract). - pub(super) fn virtio_sound_entry => virtio_sound_handler -} diff --git a/kernel/src/device.rs b/kernel/src/device.rs index c955c974510..ae655b5c95d 100644 --- a/kernel/src/device.rs +++ b/kernel/src/device.rs @@ -113,9 +113,7 @@ impl Drop for Claim { &Claimed::Class(class) => { // Before the flag goes: a poll the next holder registers is not this claim's to answer. match class { - DeviceType::HdaAudio | DeviceType::VirtioSound => { - crate::drivers::AUDIO_WATCH.cancel_polls() - } + DeviceType::HdaAudio => crate::drivers::AUDIO_WATCH.cancel_polls(), DeviceType::Keyboard | DeviceType::Mouse | DeviceType::Framebuffer @@ -243,11 +241,6 @@ pub fn try_claim(class: DeviceType, selector: [u64; 2]) -> Result { - let (info, dma) = crate::drivers::virtio_sound::info().ok_or(ClaimError::Absent)?; - let claim = Claim::acquire(class)?; - Ok(DeviceClaim::new(class, DeviceInfo::VirtioSound(info, shm(dma)), claim)) - } } } diff --git a/kernel/src/drivers/mod.rs b/kernel/src/drivers/mod.rs index a4f8cf9f890..d089972afcf 100644 --- a/kernel/src/drivers/mod.rs +++ b/kernel/src/drivers/mod.rs @@ -13,7 +13,6 @@ pub mod usb_storage; pub mod virtio; pub mod virtio_console; pub mod virtio_gpu; -pub mod virtio_sound; pub mod gop; pub mod hda; pub mod panic_console; diff --git a/kernel/src/drivers/virtio.rs b/kernel/src/drivers/virtio.rs index 9f40bfb0055..131929f8c84 100644 --- a/kernel/src/drivers/virtio.rs +++ b/kernel/src/drivers/virtio.rs @@ -36,20 +36,15 @@ pub const COMMON_DEVICE_FEATURE_SELECT: u64 = 0x00; pub const COMMON_DEVICE_FEATURE: u64 = 0x04; pub const COMMON_DRIVER_FEATURE_SELECT: u64 = 0x08; pub const COMMON_DRIVER_FEATURE: u64 = 0x0C; -const COMMON_MSIX_CONFIG: u64 = 0x10; pub const COMMON_DEVICE_STATUS: u64 = 0x14; const COMMON_QUEUE_SELECT: u64 = 0x16; pub const COMMON_QUEUE_SIZE: u64 = 0x18; -const COMMON_QUEUE_MSIX: u64 = 0x1A; pub const COMMON_QUEUE_ENABLE: u64 = 0x1C; pub const COMMON_QUEUE_NOTIFY_OFF: u64 = 0x1E; pub const COMMON_QUEUE_DESC: u64 = 0x20; pub const COMMON_QUEUE_DRIVER: u64 = 0x28; pub const COMMON_QUEUE_DEVICE: u64 = 0x30; -/// Sentinel a virtio device reads back for a vector it could not allocate (virtio 1.2 §4.1.5.1.2). -const NO_VECTOR: u16 = 0xFFFF; - const VIRTQ_DESC_F_NEXT: u16 = 1; const VIRTQ_DESC_F_WRITE: u16 = 2; @@ -92,7 +87,6 @@ struct VirtioPciConfig { notify_off_multiplier: u32, #[allow(dead_code)] // parsed from spec, used for interrupt-based operation isr: Mmio, - device: Mmio, } /// Why a virtio PCI capability names no config window, or the window it names. @@ -262,13 +256,14 @@ impl VirtioPciConfig { } } - Ok(Self { + let config = Self { common: common.ok_or(MissingCap::Common)?, notify: notify.ok_or(MissingCap::Notify)?, notify_off_multiplier, isr: isr.ok_or(MissingCap::Isr)?, - device: device.ok_or(MissingCap::Device)?, - }) + }; + device.ok_or(MissingCap::Device)?; + Ok(config) } } @@ -295,20 +290,6 @@ impl<'pool> VirtqueueRegions<'pool> { used: buf.subview(used_off, used_size), } } - - /// Compute regions from three separate DMA pages. - pub fn from_separate( - desc: Dma<'pool>, - avail: Dma<'pool>, - used: Dma<'pool>, - queue_size: u16, - ) -> Self { - Self { - desc: desc.subview(0, queue_size as usize * DESC_BYTES), - avail: avail.subview(0, avail_bytes(queue_size)), - used: used.subview(0, used_bytes(queue_size)), - } - } } /// Proof a descriptor slot is available for submission; `id()` is always below the queue's size. @@ -320,47 +301,6 @@ impl DescSlot { } -/// Interrupt-context, lock-free consumer of a virtqueue's used ring; an ISR can drain while another CPU submits under a lock. -/// Lock-free because it reads only device-written memory and its own `last_used_idx`, never shared driver state. -pub struct UsedRingConsumer<'pool> { - used: Dma<'pool>, - size: u16, - last_used_idx: u16, - refused: u32, -} - -impl UsedRingConsumer<'_> { - /// Non-blocking poll: the head descriptor id of a completed chain, or `None` if nothing new. - /// Never logs: the only caller is an ISR and the log backend's lock is one it cannot wait on. - pub fn poll(&mut self) -> Option { - loop { - let used_idx: u16 = self.used.read(USED_IDX_OFF); - if used_idx == self.last_used_idx { - return None; - } - // The device writes the element before it bumps the idx (virtio 1.2 - // §2.7.8), so the element is read after the idx that counts it. - barrier::dma_rmb(); - let slot = self.last_used_idx % self.size; - let id: Untrusted = - Untrusted::new(self.used.read(USED_RING_OFF + slot as usize * USED_ELEM_SIZE)); - self.last_used_idx = self.last_used_idx.wrapping_add(1); - // A refused head is skipped, not returned: `None` here means the ring is empty. - let Ok(head) = id.index(self.size as usize) else { - self.refused = self.refused.saturating_add(1); - continue; - }; - // Exact: `index` proved `head < self.size`, a `u16`. - return Some(head as u16); - } - } - - /// How many used-ring elements this consumer has refused, for the life of the boot. - pub fn refused(&self) -> u32 { - self.refused - } -} - /// A VirtIO split virtqueue. pub struct Virtqueue<'pool> { desc: Dma<'pool>, @@ -369,7 +309,6 @@ pub struct Virtqueue<'pool> { size: u16, last_used_idx: u16, notify_offset: u16, - used_split: bool, /// Bytes each chain's descriptor was given; the one bound a device-reported `len` is compared against. 0 means no chain. chain_bytes: alloc::vec::Vec, } @@ -407,23 +346,10 @@ impl<'pool> Virtqueue<'pool> { size: queue_size, last_used_idx: 0, notify_offset: 0, - used_split: false, chain_bytes: alloc::vec![0u32; queue_size as usize], } } - /// Hand the used ring to a dedicated consumer; afterwards `poll_used`/`has_used` panic here. - pub fn split_used_consumer(&mut self) -> UsedRingConsumer<'pool> { - assert!(!self.used_split, "virtqueue: used ring already split"); - self.used_split = true; - UsedRingConsumer { - used: self.used, - size: self.size, - last_used_idx: self.last_used_idx, - refused: 0, - } - } - /// What the device is programmed with for each ring. pub fn descs_addr(&self) -> u64 { self.desc.device_addr() } pub fn avail_addr(&self) -> u64 { self.avail.device_addr() } @@ -434,30 +360,6 @@ impl<'pool> Virtqueue<'pool> { self.notify_offset as u64 * multiplier as u64 } - /// Write one descriptor chain without publishing it. - /// Addressed by index, not [`DescSlot`]: the in-flight proof a slot carries belongs to the publisher, not here. - pub fn write_chain(&mut self, first_desc: u16, bufs: &[(u64, u32, BufDir)]) { - assert!( - (first_desc as usize + bufs.len()) <= self.size as usize, - "virtqueue: chain at {first_desc} of {} descriptors runs past a queue of {}", - bufs.len(), - self.size - ); - self.chain_bytes[first_desc as usize] = chain_bytes(bufs); - for (i, (addr, len, dir)) in bufs.iter().enumerate() { - let desc_idx = first_desc + i as u16; - let mut flags: u16 = match dir { - BufDir::Readable => 0, - BufDir::Writable => VIRTQ_DESC_F_WRITE, - }; - if i != bufs.len() - 1 { - flags |= VIRTQ_DESC_F_NEXT; - } - let desc = VirtqDesc { addr: *addr, len: *len, flags, next: desc_idx + 1 }; - self.desc.write(desc_idx as usize * core::mem::size_of::(), desc); - } - } - /// Where the used element at ring position `i` sits. fn used_elem_at(&self, i: u16) -> usize { USED_RING_OFF + i as usize * USED_ELEM_SIZE @@ -541,7 +443,6 @@ impl<'pool> Virtqueue<'pool> { /// Check if the device has completed any request. pub fn has_used(&self) -> bool { - assert!(!self.used_split, "virtqueue: used ring split off"); let used_idx: u16 = self.used.read(USED_IDX_OFF); used_idx != self.last_used_idx } @@ -550,13 +451,13 @@ impl<'pool> Virtqueue<'pool> { /// A refused element is skipped, never returned, so one forged element cannot hide the ones behind it. /// Never logs: the caller may hold `serial::BackendGuard`, the lock the log backend itself takes. pub fn poll_used(&mut self) -> Option<(DescSlot, u32)> { - assert!(!self.used_split, "virtqueue: used ring split off"); loop { let used_idx: u16 = self.used.read(USED_IDX_OFF); if used_idx == self.last_used_idx { return None; } - // The element after the idx that counts it, as in `UsedRingConsumer::poll`. + // The device writes the element before it bumps the idx (virtio 1.2 + // §2.7.8), so the element is read after the idx that counts it. barrier::dma_rmb(); let slot = self.last_used_idx % self.size; let id = self.used_ring_id(slot); @@ -571,8 +472,7 @@ impl<'pool> Virtqueue<'pool> { /// What a used-ring element must satisfy: a head inside this queue's table, at a published /// chain, written no further than that chain. Refused rather than clamped: there is nothing - /// here to recover from a forged completion, and userland maps virtio-sound's control and - /// event queues writable, so neither device-written field is trustworthy unchecked. + /// here to recover from a forged completion. fn parse_used(&self, id: Untrusted, len: Untrusted) -> Option<(DescSlot, u32)> { // `chain_bytes` is exactly `size` long: the descriptor table's own bound, not a constant beside it. let head = id.index(self.chain_bytes.len()).ok()?; @@ -809,22 +709,6 @@ pub fn cap_selftest() { log!("virtio: pci cap selftest {passed}/{CASES}"); } -/// Which of a device's two interrupt sources it declined to bind — not a driver or kernel bug. -#[derive(Debug, Clone, Copy)] -pub enum NoVector { - Config, - Queue(u16), -} - -impl core::fmt::Display for NoVector { - fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result { - match self { - Self::Config => write!(f, "its configuration-change interrupt"), - Self::Queue(queue) => write!(f, "queue {queue}'s interrupt"), - } - } -} - /// A fully initialized VirtIO device. pub struct VirtioDevice { config: VirtioPciConfig, @@ -916,22 +800,6 @@ impl VirtioDevice { common.write_u16(COMMON_QUEUE_ENABLE, 1); } - /// Point the device's config-change and `queue`'s used-ring interrupt at `pci::MSIX_ENTRY`. - /// Kept separate from `PciDevice::enable_msix`: both calls are needed, neither implies the other. - pub fn bind_msix(&self, queue: u16) -> Result<(), NoVector> { - let common = self.config.common; - common.write_u16(COMMON_MSIX_CONFIG, super::pci::MSIX_ENTRY); - if common.read_u16(COMMON_MSIX_CONFIG) == NO_VECTOR { - return Err(NoVector::Config); - } - common.write_u16(COMMON_QUEUE_SELECT, queue); - common.write_u16(COMMON_QUEUE_MSIX, super::pci::MSIX_ENTRY); - if common.read_u16(COMMON_QUEUE_MSIX) == NO_VECTOR { - return Err(NoVector::Queue(queue)); - } - Ok(()) - } - /// Set DRIVER_OK — device is now live. pub fn activate(&self) { let status = STATUS_ACKNOWLEDGE as u32 @@ -948,8 +816,4 @@ impl VirtioDevice { pub fn notify_off_multiplier(&self) -> u32 { self.config.notify_off_multiplier } - - pub fn device_config(&self) -> Mmio { - self.config.device - } } diff --git a/kernel/src/drivers/virtio_sound.rs b/kernel/src/drivers/virtio_sound.rs deleted file mode 100644 index e39ff6c5b76..00000000000 --- a/kernel/src/drivers/virtio_sound.rs +++ /dev/null @@ -1,454 +0,0 @@ -//! virtio-sound: bring-up, the virtqueues, and the register allow-list. -//! -//! Every DMA address lives in the descriptor tables, built once at bind from -//! kernel-allocated offsets; after bind the driver's whole vocabulary is an -//! avail-ring index and a doorbell write. Stream selection, format and timing -//! are soundserver's, not this driver's. -//! -//! Structure layouts and command codes follow VirtIO 1.2 §5.14. - -use core::cell::UnsafeCell; -use core::sync::atomic::{AtomicBool, AtomicU32, AtomicU64, Ordering}; - -use toyos_abi::audio::AudioCompletionRecord; -use toyos_abi::syscall::{RegWidth, SyscallError}; -use toyos_abi::virtio_sound as abi; - -use super::pci::{PciDevice, MSIX_ENTRY}; -use super::virtio::{BufDir, UsedRingConsumer, VirtioDevice, Virtqueue, VirtqueueRegions, - VIRTIO_F_VERSION_1}; -use super::DmaPool; -use crate::log; -use crate::mm::policy::CachePolicy; -use crate::mm::{Dma, Mmio}; -use crate::object::shm::Region; -use crate::sync::Lock; - -const VIRTIO_VENDOR: u16 = 0x1AF4; -const VIRTIO_SND_DEVICE: u16 = 0x1059; // 0x1040 + device_id 25 - -/// Size of the TX transfer header, which QEMU subtracts from the chain's -/// readable length to get the PCM byte count; the kernel never reads its contents. -const XFER_HEADER_BYTES: u32 = 4; -/// The per-period status the device writes back: status and latency, two `le32`. -const STATUS_BYTES: u32 = 8; -/// One event: a code and its data. -const EVENT_BYTES: u32 = 8; - -/// Kernel-only DMA page: three descriptor tables plus the TX used ring the -/// handler alone consumes. -const OFF_CTRL_DESC: usize = 0x0000; -const OFF_EVENT_DESC: usize = 0x0400; -const OFF_TX_DESC: usize = 0x0800; -const OFF_TX_USED: usize = 0x0C00; -const KERNEL_DMA_BYTES: usize = 0x1000; - -const _: () = { - use super::virtio::DESC_BYTES; - assert!(abi::CONTROL_QUEUE_SIZE as usize * DESC_BYTES <= OFF_EVENT_DESC - OFF_CTRL_DESC); - assert!(abi::EVENT_QUEUE_SIZE as usize * DESC_BYTES <= OFF_TX_DESC - OFF_EVENT_DESC); - assert!(abi::TX_QUEUE_SIZE as usize * DESC_BYTES <= OFF_TX_USED - OFF_TX_DESC); - assert!(abi::used_bytes(abi::TX_QUEUE_SIZE) <= KERNEL_DMA_BYTES - OFF_TX_USED); -}; - -/// Cap on logged refusals, past which a misbehaving driver can't spend unbounded log. -const MAX_NAMED_REFUSALS: usize = 16; - - -/// Written once before the vector arms and read without a lock afterwards — -/// not lock-guarded because the handler may interrupt a CPU holding [`CONTROLLER`]. -struct TxIsr { - consumer: UnsafeCell>>, - /// Stray used-ring entries (head names no chain); a userland bug, so counted - /// rather than logged from the ISR. - stray: AtomicU32, - named_stray: AtomicBool, -} - -// SAFETY: `consumer` is write-once before the vector arms and read only by the -// handler after; every other field is atomic. -unsafe impl Sync for TxIsr {} - -static TX_ISR: TxIsr = TxIsr { - consumer: UnsafeCell::new(None), - stray: AtomicU32::new(0), - named_stray: AtomicBool::new(false), -}; - -/// Drain the TX used ring; a head naming no chain is untrusted input, counted not asserted. -fn drain_tx() -> u32 { - // SAFETY: sole accessor after init — see `TxIsr`. - let consumer = unsafe { &mut *TX_ISR.consumer.get() }; - // A configuration-change interrupt shares this vector and may arrive before init installs the consumer. - let Some(consumer) = consumer.as_mut() else { return 0 }; - let mut mask = 0u32; - let refused_before = consumer.refused(); - while let Some(head) = consumer.poll() { - let idx = head as usize / abi::TX_CHAIN as usize; - if idx >= abi::PERIODS || head % abi::TX_CHAIN != 0 { - TX_ISR.stray.fetch_add(1, Ordering::Relaxed); - continue; - } - mask |= 1 << idx; - } - // Folds in the consumer's own refusals; a head past the queue never reaches the loop above. - let refused = consumer.refused() - refused_before; - if refused != 0 { - TX_ISR.stray.fetch_add(refused, Ordering::Relaxed); - } - mask -} - -/// Rust half of the MSI-X handler, called from the IDT entry. -pub fn isr_complete() { - // Timestamp first — this is the hardware-completion time the DLL feeds on. - let timestamp = crate::clock::nanos_since_boot(); - let mask = drain_tx(); - if mask == 0 { - return; - } - isr_push_completion(mask, timestamp); - super::AUDIO_WATCH.post_in_place(); - crate::preempt::set_need_resched(); -} - - -const RECORD_RING_CAP: u32 = 16; - -/// SPSC: producer is the MSI-X handler (single CPU, IF=0); consumer holds [`CONTROLLER`]. -/// One record per interrupt, never accumulated — a folded mask would misreport lateness to soundserver's DLL. -struct RecordRing { - slots: [UnsafeCell; RECORD_RING_CAP as usize], - head: AtomicU32, - tail: AtomicU32, -} - -const _: () = assert!( - RECORD_RING_CAP as usize >= abi::PERIODS, - "record ring must hold one record per period" -); - -// SAFETY: slot access is arbitrated by the head/tail protocol above. -unsafe impl Sync for RecordRing {} - -static RECORDS: RecordRing = RecordRing { - slots: [const { - UnsafeCell::new(AudioCompletionRecord { mask: 0, _pad: 0, timestamp_nanos: 0 }) - }; RECORD_RING_CAP as usize], - head: AtomicU32::new(0), - tail: AtomicU32::new(0), -}; - -/// Overflow sink for a full ring: mask is OR'd (idempotent) and timestamp is -/// newest-wins, so a driver that stops reading costs itself only timestamp granularity. -struct Spill { - mask: AtomicU32, - timestamp: AtomicU64, -} - -static SPILL: Spill = Spill { mask: AtomicU32::new(0), timestamp: AtomicU64::new(0) }; - -fn isr_push_completion(mask: u32, timestamp_nanos: u64) { - let ring = &RECORDS; - let head = ring.head.load(Ordering::Relaxed); // sole writer of head - let tail = ring.tail.load(Ordering::Acquire); - if head.wrapping_sub(tail) >= RECORD_RING_CAP { - SPILL.timestamp.store(timestamp_nanos, Ordering::Relaxed); - SPILL.mask.fetch_or(mask, Ordering::Release); - return; - } - let slot = (head % RECORD_RING_CAP) as usize; - // SAFETY: slot is outside [tail, head) — not visible to the consumer. - unsafe { - *ring.slots[slot].get() = AudioCompletionRecord { mask, _pad: 0, timestamp_nanos }; - } - // Release publishes the record before the consumer can observe the new head. - ring.head.store(head.wrapping_add(1), Ordering::Release); -} - -/// Pop the oldest pending record; called under [`CONTROLLER`], so the tail store -/// needs no CAS. Spill returns last because it is always newer than anything still queued. -fn pop_completion() -> Option { - let ring = &RECORDS; - let tail = ring.tail.load(Ordering::Relaxed); // sole writer of tail - // Acquire pairs with the producer's Release store of head. - let head = ring.head.load(Ordering::Acquire); - if head == tail { - let mask = SPILL.mask.swap(0, Ordering::AcqRel); - if mask == 0 { - return None; - } - return Some(AudioCompletionRecord { - mask, - _pad: 0, - timestamp_nanos: SPILL.timestamp.load(Ordering::Relaxed), - }); - } - // SAFETY: `tail != head` puts this slot in `[tail, head)`, unwritten by the - // producer until wrap; the Acquire load of `head` above pairs with its Release store. - let rec = unsafe { *ring.slots[(tail % RECORD_RING_CAP) as usize].get() }; - ring.tail.store(tail.wrapping_add(1), Ordering::Release); - Some(rec) -} - -/// True if completion records are pending; lock-free. -pub fn has_pending() -> bool { - RECORDS.head.load(Ordering::Acquire) != RECORDS.tail.load(Ordering::Acquire) - || SPILL.mask.load(Ordering::Acquire) != 0 -} - -/// Copy up to `buf.len() / 16` pending records into `buf`, oldest first, and -/// name a stray completion the first time one has been counted. -pub fn drain_completed(buf: &mut crate::user_ptr::UserBytesMut) -> usize { - let stray = TX_ISR.stray.load(Ordering::Relaxed); - if stray != 0 && !TX_ISR.named_stray.swap(true, Ordering::Relaxed) { - log!( - "virtio-sound: the device completed a chain this driver never built ({stray} so far) \ - — a used-ring head past the queue, or one that heads no chain" - ); - } - let max = buf.len() / AudioCompletionRecord::SIZE; - let _guard = CONTROLLER.lock(); - let mut written = 0; - for _ in 0..max { - let Some(rec) = pop_completion() else { break }; - // Field-wise serialization — never expose struct padding. - let mut record = [0u8; AudioCompletionRecord::SIZE]; - record[0..4].copy_from_slice(&rec.mask.to_le_bytes()); - record[8..16].copy_from_slice(&rec.timestamp_nanos.to_le_bytes()); - buf.write_at(written, &record); - written += AudioCompletionRecord::SIZE; - } - written -} - - -/// Notify region only; virtqueues and DMA pools are leaked at bring-up because -/// `TX_ISR` holds a used-ring consumer into one of them for the life of the boot. -struct Bound { - notify: Mmio, -} - -static CONTROLLER: Lock> = Lock::new(None); -static INFO: Lock> = Lock::new(None); -static REFUSALS: AtomicU32 = AtomicU32::new(0); - -pub fn info() -> Option<(abi::VirtioSoundInfo, Region)> { - INFO.lock().clone() -} - - -/// Allow-list for the three doorbells; a doorbell value is a queue index, not an -/// address, so it can reach nothing the already-selected offset did not. -fn write_permit(info: &abi::VirtioSoundInfo, offset: u64, width: RegWidth) -> bool { - let Ok(offset) = u32::try_from(offset) else { return false }; - width == RegWidth::U16 - && [info.notify_control, info.notify_event, info.notify_tx].contains(&offset) -} - -fn refuse(what: &str, offset: u64, width: RegWidth) -> SyscallError { - if REFUSALS.fetch_add(1, Ordering::Relaxed) < MAX_NAMED_REFUSALS as u32 { - log!("virtio-sound: refused a {width:?} {what} of {offset:#x} — not on the allow-list"); - } - SyscallError::PermissionDenied -} - -/// No register is readable; every read is a refusal — answers arrive via memory, not MMIO. -pub fn reg_read(offset: u64, width: RegWidth) -> Result { - Err(refuse("read", offset, width)) -} - -pub fn reg_write(offset: u64, width: RegWidth, value: u32) -> Result<(), SyscallError> { - let (info, _) = info().ok_or(SyscallError::NotFound)?; - if !write_permit(&info, offset, width) { - return Err(refuse("write", offset, width)); - } - if value > width.max_value() { - return Err(SyscallError::InvalidArgument); - } - let guard = CONTROLLER.lock(); - let controller = guard.as_ref().ok_or(SyscallError::NotFound)?; - controller.notify.write_u16(offset, value as u16); - Ok(()) -} - - -/// Bring up virtio-sound, or leave it unclaimed and log why — audio is optional, -/// so a refusal beats a panic over a peripheral. -pub fn init(devices: &[PciDevice]) { - let Some(pci) = devices.iter().find(|d| d.is_id(VIRTIO_VENDOR, VIRTIO_SND_DEVICE)) else { - return; - }; - log!("virtio-sound: found at PCI {:02x}:{:02x}.{}", pci.bus, pci.dev, pci.func); - - let device = match VirtioDevice::init(pci, VIRTIO_F_VERSION_1) { - Ok(device) => device, - Err(why) => { - log!("virtio-sound: NOT INITIALISED — PCI {:02x}:{:02x}.{} {why}", - pci.bus, pci.dev, pci.func); - return; - } - }; - - let cfg = device.device_config(); - let (jacks, streams, chmaps) = (cfg.read_u32(0), cfg.read_u32(4), cfg.read_u32(8)); - log!("virtio-sound: {jacks} jacks, {streams} streams, {chmaps} chmaps"); - if streams == 0 { - log!("virtio-sound: NOT INITIALISED — the device offers no PCM stream to play into"); - return; - } - - // Placed after the stream check so a streamless device allocates nothing; see [`Bound`]. - let space = crate::iommu::DeviceSpace::create(); - let kernel_mem = DmaPool::alloc_in(KERNEL_DMA_BYTES, space).leak(); - let shared = DmaPool::alloc_in(abi::SHARED_BYTES, space).leak(); - // Before the device is told any address, and after both pools' mappings. - space.attach(pci.bus, pci.dev, pci.func); - // Exclusive: just allocated, not yet told to the device or mapped to userland. - shared.zero(); - - let mut controlq = queue( - kernel_mem.subview(OFF_CTRL_DESC, OFF_EVENT_DESC - OFF_CTRL_DESC), - shared.subview(abi::OFF_CTRL_AVAIL, abi::avail_bytes(abi::CONTROL_QUEUE_SIZE)), - shared.subview(abi::OFF_CTRL_USED, abi::used_bytes(abi::CONTROL_QUEUE_SIZE)), - abi::CONTROL_QUEUE_SIZE, - ); - let mut eventq = queue( - kernel_mem.subview(OFF_EVENT_DESC, OFF_TX_DESC - OFF_EVENT_DESC), - shared.subview(abi::OFF_EVENT_AVAIL, abi::avail_bytes(abi::EVENT_QUEUE_SIZE)), - shared.subview(abi::OFF_EVENT_USED, abi::used_bytes(abi::EVENT_QUEUE_SIZE)), - abi::EVENT_QUEUE_SIZE, - ); - // TX used ring lives in kernel memory only — userland must never fabricate a completion by rewriting it. - let mut txq = queue( - kernel_mem.subview(OFF_TX_DESC, OFF_TX_USED - OFF_TX_DESC), - shared.subview(abi::OFF_TX_AVAIL, abi::avail_bytes(abi::TX_QUEUE_SIZE)), - kernel_mem.subview(OFF_TX_USED, abi::used_bytes(abi::TX_QUEUE_SIZE)), - abi::TX_QUEUE_SIZE, - ); - - build_chains(&mut controlq, &mut eventq, &mut txq, shared.device_addr()); - - // Installed before the vector can fire, so no interrupt observes a half-written Option. - // SAFETY: MSI-X is not enabled yet. - unsafe { *TX_ISR.consumer.get() = Some(txq.split_used_consumer()) }; - - device.setup_queue(abi::CONTROL_QUEUE, &mut controlq); - device.setup_queue(abi::EVENT_QUEUE, &mut eventq); - device.setup_queue(abi::TX_QUEUE, &mut txq); - if !arm_interrupt(pci, &device) { - return; - } - device.enable_queue(abi::CONTROL_QUEUE); - device.enable_queue(abi::EVENT_QUEUE); - device.enable_queue(abi::TX_QUEUE); - device.activate(); - - // DmaPool allocations are whole 2 MiB pages; ABI offsets are relative to that page. - let dma_region = Region { - phys: crate::DirectMap::from_phys(shared.host_phys()), - size: crate::mm::PAGE_2M, - cache: CachePolicy::Normal, - pages: None, - }; - let multiplier = device.notify_off_multiplier(); - let info = abi::VirtioSoundInfo { - dma: toyos_abi::HANDLE_INVALID, - notify_control: controlq.notify_bytes(multiplier) as u32, - notify_event: eventq.notify_bytes(multiplier) as u32, - notify_tx: txq.notify_bytes(multiplier) as u32, - jacks, - streams, - chmaps, - }; - - *CONTROLLER.lock() = Some(Bound { notify: device.notify_mmio() }); - *INFO.lock() = Some((info, dma_region)); - - log!( - "virtio-sound: bound, {} periods of {} bytes, doorbells at {:#x}/{:#x}/{:#x}", - abi::PERIODS, - abi::PERIOD_BYTES, - info.notify_control, - info.notify_event, - info.notify_tx - ); -} - -fn queue<'pool>( - desc: Dma<'pool>, - avail: Dma<'pool>, - used: Dma<'pool>, - size: u16, -) -> Virtqueue<'pool> { - Virtqueue::from_regions(&VirtqueueRegions::from_separate(desc, avail, used, size), size) -} - -/// Builds every chain once; after this no descriptor is ever written again — the -/// driver's whole vocabulary becomes an avail-ring index and a doorbell write. -fn build_chains( - controlq: &mut Virtqueue<'_>, - eventq: &mut Virtqueue<'_>, - txq: &mut Virtqueue<'_>, - base: u64, -) { - let at = |offset: usize| base + offset as u64; - - // One chain serves every command: the device reads the header first and - // takes only what that command defines. - controlq.write_chain( - 0, - &[ - (at(abi::OFF_CTRL_REQ), abi::CTRL_BUF_BYTES as u32, BufDir::Readable), - (at(abi::OFF_CTRL_RESP), abi::CTRL_BUF_BYTES as u32, BufDir::Writable), - ], - ); - - // One descriptor per buffer: buffer index equals descriptor index. - for i in 0..abi::EVENT_BUFS { - eventq.write_chain( - i as u16, - &[(at(abi::OFF_EVENT_BUFS + i * abi::EVENT_BUF_STRIDE), EVENT_BYTES, BufDir::Writable)], - ); - } - - for i in 0..abi::PERIODS { - txq.write_chain( - abi::tx_chain_head(i), - &[ - (at(abi::OFF_TX_XFER + i * abi::XFER_STRIDE), XFER_HEADER_BYTES, BufDir::Readable), - (at(abi::OFF_PCM + i * abi::PERIOD_BYTES), abi::PERIOD_BYTES as u32, BufDir::Readable), - (at(abi::OFF_TX_STATUS + i * abi::STATUS_STRIDE), STATUS_BYTES, BufDir::Writable), - ], - ); - } -} - -/// Arm the TX completion interrupt, or refuse and log why; never panics. The -/// handler is the TX used ring's only consumer, so an unarmed device leaves -/// every period in flight forever. -fn arm_interrupt(pci: &PciDevice, device: &VirtioDevice) -> bool { - let vector = crate::arch::trap::VIRTIO_SOUND_VECTOR; - if pci.enable_msix(vector).is_none() { - 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", - pci.bus, - pci.dev, - pci.func - ); - return false; - } - if let Err(refused) = device.bind_msix(abi::TX_QUEUE) { - log!( - "virtio-sound: NOT INITIALISED at PCI {:02x}:{:02x}.{} — the device refused a vector \ - for {refused}", - pci.bus, - pci.dev, - pci.func - ); - return false; - } - log!("virtio-sound: MSI-X vector {vector:#x} on table entry {MSIX_ENTRY}"); - true -} diff --git a/kernel/src/irq_census.rs b/kernel/src/irq_census.rs index 6be952f5f95..ec55b603ab9 100644 --- a/kernel/src/irq_census.rs +++ b/kernel/src/irq_census.rs @@ -26,8 +26,6 @@ pub enum Source { /// claim an interrupt belonged to is the claim's own record, and the census /// is about this machine's interrupt routing. UserDev, - /// Vector 0x23, virtio-sound MSI-X. - Sound, /// Vector 0x24, the i8042's I/O APIC pin — both PS/2 lines. I8042, /// Vector 0x25, the remapping unit's fault event. @@ -47,11 +45,11 @@ pub enum Source { } impl Source { - pub const COUNT: usize = 12; + pub const COUNT: usize = 11; /// Order `tests/toyos.rs`'s `irq_census_conservation` parses back; must match variant order. pub const NAMES: [&'static str; Self::COUNT] = [ - "timer", "kick", "xhci", "userdev", "sound", "i8042", "dmafault", "hda", "tlb", "nmi", + "timer", "kick", "xhci", "userdev", "i8042", "dmafault", "hda", "tlb", "nmi", "spurious", "unclaimed", ]; } diff --git a/kernel/src/isa.rs b/kernel/src/isa.rs index 146b37dbc37..e270962c1d3 100644 --- a/kernel/src/isa.rs +++ b/kernel/src/isa.rs @@ -245,7 +245,7 @@ pub fn take_record(row: &Row) -> Option { if taken.is_some() && IRQ[row].take_unannounced() { log!("isa: {} took its first interrupt", function(row).expect("a claimed row was filled").name); } - taken.map(|count| DeviceIrqRecord { count }) + taken } pub fn has_irq(row: &Row) -> bool { @@ -256,7 +256,7 @@ pub fn has_irq(row: &Row) -> bool { /// watch. Called from the row's handler, so it takes no lock but the watch's /// own and the I/O APIC's masked one, and allocates nothing. pub fn isr(row: usize) { - IRQ[row].took(); + IRQ[row].took(crate::clock::nanos_since_boot()); for &line in lines(row) { if pio::level(line) { pio::set_masked(line, true); diff --git a/kernel/src/main.rs b/kernel/src/main.rs index cbf0cf818a5..5aef1ee13a4 100644 --- a/kernel/src/main.rs +++ b/kernel/src/main.rs @@ -111,7 +111,7 @@ mod late_panic { use crate::mm::policy::MmioPolicy; use alloc::boxed::Box; use arch::{cpu, percpu}; -use drivers::{acpi, gop, pci, serial, virtio_console, virtio_gpu, virtio_sound, xhci}; +use drivers::{acpi, gop, pci, serial, virtio_console, virtio_gpu, xhci}; use toyos_abi::boot::{KernelArgs, MemoryMapEntry}; use toyos_rootimage::handoff::{held, Descriptor}; @@ -525,7 +525,6 @@ pub(crate) unsafe extern "C" fn kernel_main(loader_args: &mut KernelArgs) -> ! { virtio_console::init(&pci_devices); - virtio_sound::init(&pci_devices); drivers::hda::init(&pci_devices); if let Some((gpu_driver, gpu_info)) = virtio_gpu::init(&pci_devices) { diff --git a/kernel/src/object/device.rs b/kernel/src/object/device.rs index b2423518eaf..d6b1a3279bc 100644 --- a/kernel/src/object/device.rs +++ b/kernel/src/object/device.rs @@ -24,7 +24,6 @@ pub enum DeviceInfo { /// own memory, so this mint installs nothing. PciFunction(toyos_abi::pci::PciFunctionInfo), Hda(toyos_abi::hda::HdaInfo, Arc), - VirtioSound(toyos_abi::virtio_sound::VirtioSoundInfo, Arc), /// Which partition, how long, and both its GUIDs; the view it moves blocks /// through is the claim's own (`device::Claim::partition`). Partition(toyos_abi::part::PartitionInfo), @@ -82,11 +81,6 @@ impl DeviceInfo { info.pcm = install_buffers(table, &[pcm])?[0]; bytes(&info).into() } - Self::VirtioSound(info, dma) => { - let mut info = *info; - info.dma = install_buffers(table, &[dma])?[0]; - bytes(&info).into() - } }) } } diff --git a/kernel/src/object/ops.rs b/kernel/src/object/ops.rs index f04c3222fc4..d5e3ba8577e 100644 --- a/kernel/src/object/ops.rs +++ b/kernel/src/object/ops.rs @@ -287,9 +287,7 @@ pub fn read_watch(object: &KObjectRef) -> Option { device_registry::DeviceType::PciFunction | device_registry::DeviceType::Isa | device_registry::DeviceType::Acpi => Some(WatchRef::Claim(d.clone())), - device_registry::DeviceType::HdaAudio | device_registry::DeviceType::VirtioSound => { - Some(WatchRef::Irq(&crate::drivers::AUDIO_WATCH)) - } + device_registry::DeviceType::HdaAudio => Some(WatchRef::Irq(&crate::drivers::AUDIO_WATCH)), device_registry::DeviceType::Framebuffer => None, // A partition answers its description and has nothing to wait for. device_registry::DeviceType::Partition => None, @@ -362,7 +360,6 @@ fn close_ends_polls(object: &KObjectRef) -> bool { | device_registry::DeviceType::Acpi => false, device_registry::DeviceType::Mouse | device_registry::DeviceType::HdaAudio - | device_registry::DeviceType::VirtioSound | device_registry::DeviceType::Framebuffer | device_registry::DeviceType::Partition => true, }, @@ -506,17 +503,6 @@ pub fn read_device( let n = crate::drivers::hda::drain_completed(buf); if n == 0 { None } else { Some(n as u64) } } - device_registry::DeviceType::VirtioSound => { - if !claim.info_read() { - return Some(claim.describe(table, buf)); - } - if buf.len() < toyos_abi::audio::AudioCompletionRecord::SIZE { - return Some(SyscallError::InvalidArgument.to_u64()); - } - // Completion records, oldest first; empty answers `None` so a blocking read parks. - let n = crate::drivers::virtio_sound::drain_completed(buf); - if n == 0 { None } else { Some(n as u64) } - } } } @@ -692,8 +678,7 @@ pub fn fstat(object: &KObjectRef) -> Stat { device_registry::DeviceType::PciFunction | device_registry::DeviceType::Isa | device_registry::DeviceType::Acpi => FileType::Unknown, - device_registry::DeviceType::HdaAudio - | device_registry::DeviceType::VirtioSound => FileType::Unknown, + device_registry::DeviceType::HdaAudio => FileType::Unknown, device_registry::DeviceType::Partition => FileType::Unknown, }), } @@ -770,7 +755,6 @@ fn partition_fsync(claim: &DeviceClaim) -> u64 { | device_registry::DeviceType::Mouse | device_registry::DeviceType::Framebuffer | device_registry::DeviceType::HdaAudio - | device_registry::DeviceType::VirtioSound | device_registry::DeviceType::PciFunction | device_registry::DeviceType::Isa | device_registry::DeviceType::Acpi => { @@ -849,9 +833,6 @@ pub fn has_data(object: &KObjectRef) -> bool { device_registry::DeviceType::HdaAudio => { !d.info_read() || crate::drivers::hda::has_pending() } - device_registry::DeviceType::VirtioSound => { - !d.info_read() || crate::drivers::virtio_sound::has_pending() - } }, KObjectRef::PipeWrite(_) | KObjectRef::Inbox(_) | KObjectRef::SysCap(_) | KObjectRef::Connector(_) | KObjectRef::Namespace(_) @@ -916,7 +897,6 @@ fn write_device(claim: &DeviceClaim, buf: &UserBytes) -> u64 { | device_registry::DeviceType::Mouse | device_registry::DeviceType::Framebuffer | device_registry::DeviceType::HdaAudio - | device_registry::DeviceType::VirtioSound | device_registry::DeviceType::PciFunction | device_registry::DeviceType::Partition => return SyscallError::PermissionDenied.to_u64(), } diff --git a/kernel/src/pcidev/mod.rs b/kernel/src/pcidev/mod.rs index 0b5487fa7e2..3a73f50847b 100644 --- a/kernel/src/pcidev/mod.rs +++ b/kernel/src/pcidev/mod.rs @@ -1739,7 +1739,7 @@ pub fn take_record(binding: &Binding) -> Result, Syscall if taken.is_some() && IRQ[slot].take_unannounced() { log!("pcidev: slot {slot} took its first message on vector {:#x}", VECTORS[slot]); } - Ok(taken.map(|count| DeviceIrqRecord { count })) + Ok(taken) } /// Whether a read of the claim answers at once: a message is waiting, or the @@ -1752,7 +1752,7 @@ pub fn has_irq(binding: &Binding) -> bool { /// handler, so it allocates nothing; `record.rs` owns the counting, and /// `kernel-loom` models it against a concurrent reader. pub fn isr(slot: usize) { - IRQ[slot].took(); + IRQ[slot].took(crate::clock::nanos_since_boot()); WATCHES[slot].post_in_place(); } diff --git a/kernel/src/pcidev/record.rs b/kernel/src/pcidev/record.rs index 72ff55f33b7..8ba27c7b569 100644 --- a/kernel/src/pcidev/record.rs +++ b/kernel/src/pcidev/record.rs @@ -6,57 +6,77 @@ //! not an ordering, and no guest test in this suite lands on it. //! //! **Two parties race**: the ISR, on whichever CPU the unit routed the message -//! to, and the holder reading its record through a syscall on any CPU. +//! to, and the holder reading its record through a syscall on any CPU. One +//! handler at a time: a slot has one vector, delivered to one CPU. //! //! **The invariant is that every message is counted exactly once.** The count //! is a read-modify-write on both sides and never a load followed by a store: //! a reader that loaded a count and then cleared it drops every message the ISR //! recorded in between, and a driver that misses one waits for a device that -//! has already spoken. No ordering carries anything across these words — each -//! is the whole of what it says — so the orderings here are `Relaxed` and the -//! model is about the interleaving; the holder reads them after the wake the -//! claim's watch post owes it, which orders them. +//! has already spoken. +//! +//! **And a read's times are its own messages'.** The count shares its word +//! with the number of reads that have taken one, so which read a message falls +//! to and whether it is that read's first are one compare-exchange. The first +//! message of read `n` stamps the slot of `n`'s parity, before the exchange +//! that counts it publishes the stamp (`Release`, taken by the read's +//! `Acquire`), and read `n + 1`'s first message stamps the other slot: one +//! landing while a reader is between its exchange and its load never +//! overwrites the time that reader is about to load. A second reader racing +//! 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. #[cfg(not(feature = "loom"))] -use core::sync::atomic::{AtomicBool, AtomicU32, Ordering}; +use core::sync::atomic::{AtomicBool, AtomicU64, Ordering}; #[cfg(feature = "loom")] -use loom::sync::atomic::{AtomicBool, AtomicU32, Ordering}; +use loom::sync::atomic::{AtomicBool, AtomicU64, Ordering}; + +use toyos_abi::pci::DeviceIrqRecord; -/// Every word here is the whole of what it says and orders nothing else, so -/// the only property is the interleaving — which `device-irq-lossy` is the -/// control for and `kernel-loom` is the model of. +/// 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. const ORDER: Ordering = Ordering::Relaxed; -/// The negative control: the read-modify-writes become a load and a store, -/// which is the whole of what this record's design is. Never on in a kernel -/// build. +/// The count is the word's low half and the reads that took one its high half. +const COUNT: u64 = 0xFFFF_FFFF; +const READ: u64 = 1 << 32; + +/// The negative control: each compare-exchange on the word becomes a load and +/// a store, which is the whole of what this record's design is. Never on in a +/// kernel build. #[cfg(feature = "device-irq-lossy")] -macro_rules! take_word { - ($word:expr, $empty:expr) => {{ - let held = $word.load(ORDER); - $word.store($empty, ORDER); - held +macro_rules! exchange { + ($word:expr, $held:expr, $new:expr, $success:expr) => {{ + let seen = $word.load(ORDER); + if seen == $held { + $word.store($new, ORDER); + Ok(seen) + } else { + Err(seen) + } }}; } #[cfg(not(feature = "device-irq-lossy"))] -macro_rules! take_word { - ($word:expr, $empty:expr) => { - $word.swap($empty, ORDER) +macro_rules! exchange { + ($word:expr, $held:expr, $new:expr, $success:expr) => { + $word.compare_exchange_weak($held, $new, $success, ORDER) }; } #[cfg(feature = "device-irq-lossy")] -macro_rules! bump { - ($word:expr) => {{ +macro_rules! take_word { + ($word:expr, $empty:expr) => {{ let held = $word.load(ORDER); - $word.store(held.wrapping_add(1), ORDER); + $word.store($empty, ORDER); + held }}; } #[cfg(not(feature = "device-irq-lossy"))] -macro_rules! bump { - ($word:expr) => { - $word.fetch_add(1, ORDER) +macro_rules! take_word { + ($word:expr, $empty:expr) => { + $word.swap($empty, ORDER) }; } @@ -64,8 +84,12 @@ macro_rules! bump { /// /// Atomics only: the handler allocates nothing. pub struct Interrupt { - /// Messages since the holder's last read. - count: AtomicU32, + /// Messages since the holder's last read, under the reads that took one. + word: AtomicU64, + /// When each read's first message landed, by the read's parity. + first: [AtomicU64; 2], + /// When the newest message landed. + last: AtomicU64, /// The unit refused this function an access. Every call the claim answers /// refuses from here on: its bus mastering is gone, so a driver that kept /// going would be driving nothing. @@ -84,7 +108,9 @@ impl Interrupt { #[cfg(not(feature = "loom"))] pub const fn new() -> Self { Self { - count: AtomicU32::new(0), + word: AtomicU64::new(0), + first: [AtomicU64::new(0), AtomicU64::new(0)], + last: AtomicU64::new(0), faulted: AtomicBool::new(false), unannounced: AtomicBool::new(true), } @@ -95,36 +121,63 @@ impl Interrupt { #[cfg(feature = "loom")] pub fn new() -> Self { Self { - count: AtomicU32::new(0), + word: AtomicU64::new(0), + first: [AtomicU64::new(0), AtomicU64::new(0)], + last: AtomicU64::new(0), faulted: AtomicBool::new(false), unannounced: AtomicBool::new(true), } } - /// Record one message. Called from the vector's ISR, so it takes no lock - /// and allocates nothing. + /// Record one message, which landed at `now`. Called from the vector's + /// ISR, so it takes no lock and allocates nothing. /// - /// `fetch_add` and not a store: the reader may take the count between any - /// two of these, and what it took plus what is left has to be what arrived. - pub fn took(&self) { - bump!(self.count); + /// A compare-exchange and not a store: the reader may take the count + /// between any two of these, and what it took plus what is left has to be + /// what arrived. + pub fn took(&self, now: u64) { + self.last.store(now, ORDER); + let mut word = self.word.load(ORDER); + loop { + if word & COUNT == 0 { + self.first[(word / READ) as usize & 1].store(now, ORDER); + } + match exchange!(self.word, word, word.wrapping_add(1), Ordering::Release) { + Ok(_) => return, + Err(seen) => word = seen, + } + } } - /// The messages since the last read, or `None` for none. + /// The messages since the last read and when they landed, or `None` for + /// none. /// - /// `swap` and not a load followed by a store: a message the ISR records - /// between the two would be cleared without ever having been counted. - pub fn take(&self) -> Option { - match take_word!(self.count, 0) { - 0 => None, - count => Some(count), + /// A compare-exchange and not a load followed by a store: a message the + /// ISR records between the two would be cleared without ever having been + /// counted. + pub fn take(&self) -> Option { + let mut word = self.word.load(ORDER); + loop { + if word & COUNT == 0 { + return None; + } + match exchange!(self.word, word, (word / READ).wrapping_add(1) * READ, Ordering::Acquire) { + Ok(_) => break, + Err(seen) => word = seen, + } } + Some(DeviceIrqRecord { + count: (word & COUNT) as u32, + _pad: 0, + first_nanos: self.first[(word / READ) as usize & 1].load(ORDER), + last_nanos: self.last.load(ORDER), + }) } /// Whether a message is waiting, for a readiness check that consumes /// nothing. pub fn armed(&self) -> bool { - self.count.load(ORDER) != 0 + self.word.load(ORDER) & COUNT != 0 } /// Whether this is the first message this slot has taken. Answers `true` @@ -149,7 +202,7 @@ impl Interrupt { /// Back to the state a fresh slot is in, for a claim being minted or given /// up. The holder is not running at either point. pub fn clear(&self) { - self.count.store(0, ORDER); + self.word.store(0, ORDER); self.faulted.store(false, ORDER); self.unannounced.store(true, ORDER); } diff --git a/kernel/src/syscall/device.rs b/kernel/src/syscall/device.rs index 05d4b7dfe82..64aebc035d2 100644 --- a/kernel/src/syscall/device.rs +++ b/kernel/src/syscall/device.rs @@ -41,7 +41,6 @@ pub(super) fn holds_claim( /// The register-access stub a device claim's class selects. enum RegTarget { Hda, - VirtioSound, /// A claimed PCI function's own config space, **read-only**, through the /// claim itself: what names the function is lent by it for each access. /// @@ -63,7 +62,6 @@ pub(super) fn sys_device_reg(handle: RawHandle, offset: u64, width: u64, value: let target = process::with_process_data(|data| { data.handles.get::(handle, Rights::NONE).map(|claim| match claim.class() { device::DeviceType::HdaAudio => Some(RegTarget::Hda), - device::DeviceType::VirtioSound => Some(RegTarget::VirtioSound), device::DeviceType::PciFunction => Some(RegTarget::PciConfig(claim)), _ => None, }) @@ -81,7 +79,6 @@ pub(super) fn sys_device_reg(handle: RawHandle, offset: u64, width: u64, value: None => { let read = match target { RegTarget::Hda => crate::drivers::hda::reg_read(offset, width), - RegTarget::VirtioSound => crate::drivers::virtio_sound::reg_read(offset, width), // The one place a caller's offset becomes an access: the // witness `pcidev::config_window` answers is the only thing // the register read takes, so an unchecked number cannot @@ -102,9 +99,6 @@ pub(super) fn sys_device_reg(handle: RawHandle, offset: u64, width: u64, value: Ok(value) => { let written = match target { RegTarget::Hda => crate::drivers::hda::reg_write(offset, width, value), - RegTarget::VirtioSound => { - crate::drivers::virtio_sound::reg_write(offset, width, value) - } // Config space has no write path from userland at all. RegTarget::PciConfig(_) => Err(SyscallError::NotSupported), }; @@ -180,7 +174,6 @@ pub(super) fn sys_device_claim(syscap: RawHandle, class: u64, selector: [u64; 2] | device::DeviceType::Mouse | device::DeviceType::Framebuffer | device::DeviceType::HdaAudio - | device::DeviceType::VirtioSound | device::DeviceType::Acpi => 0, device::DeviceType::PciFunction => 1, device::DeviceType::Partition | device::DeviceType::Isa => 2, @@ -470,7 +463,6 @@ pub(super) fn sys_partition_transfer( | device::DeviceType::Mouse | device::DeviceType::Framebuffer | device::DeviceType::HdaAudio - | device::DeviceType::VirtioSound | device::DeviceType::PciFunction | device::DeviceType::Isa | device::DeviceType::Acpi => { diff --git a/kernel/src/syscall/io.rs b/kernel/src/syscall/io.rs index 89a4a03d3cd..ced9afa8f3d 100644 --- a/kernel/src/syscall/io.rs +++ b/kernel/src/syscall/io.rs @@ -31,7 +31,6 @@ enum WriteBlock { /// What `sys_read` parks on when the handle has nothing to give. enum ReadBlock { Pipe(alloc::sync::Arc, pipe::PipeId, WaitClass), - VirtioSound, Hda, /// A claimed keyboard, woken by its own IRQ. Keyboard(Deadline), @@ -90,12 +89,11 @@ pub(super) fn sys_write(h: RawHandle, buf: &UserBytes) -> u64 { } } -/// Only these four device classes block; the rest answer `NotFound` on an +/// Only these device classes block; the rest answer `NotFound` on an /// empty blocking read. fn read_block_device(claim: &crate::object::device::DeviceClaim) -> ReadBlock { match claim.class() { device::DeviceType::Keyboard => ReadBlock::Keyboard(Deadline::never()), - device::DeviceType::VirtioSound if claim.info_read() => ReadBlock::VirtioSound, device::DeviceType::HdaAudio if claim.info_read() => ReadBlock::Hda, _ => ReadBlock::Refused(SyscallError::NotFound.to_u64()), } @@ -168,21 +166,6 @@ pub(super) fn sys_read(h: RawHandle, buf: &mut UserBytesMut) -> u64 { return cancelled(); } } - Err(ReadBlock::VirtioSound) => { - let parkable = crate::scheduler::Parkable::at_entry(); - if watch::wait_until( - &parkable, - &crate::drivers::AUDIO_WATCH, - 0, - WaitClass::Io, - Deadline::never(), - crate::drivers::virtio_sound::has_pending, - ) - .is_err() - { - return cancelled(); - } - } Err(ReadBlock::Hda) => { let parkable = crate::scheduler::Parkable::at_entry(); if watch::wait_until( diff --git a/src/clippy.rs b/src/clippy.rs index 7580c0f8147..fcabdb691e1 100644 --- a/src/clippy.rs +++ b/src/clippy.rs @@ -62,8 +62,10 @@ fn control_features() -> Vec { /// `undocumented_unsafe_blocks` is adopted per area as each area's /// justifications land. `toyos-xhci` has a shape of its own because the /// workspace run builds it only with `toyos-xhci-sim`'s `flaws`, never as the -/// kernel does. `kernel-loom` has one because `victim-retires-mid-probe`'s -/// test arm excludes `no-preempt-guard`, which `$CONTROLS` turns on beside it. +/// kernel does. `toyos-pci-claim` has one because `$GUESTS` excludes it and +/// only netstack's host tests build its `host-grant`. `kernel-loom` has one +/// because `victim-retires-mid-probe`'s test arm excludes `no-preempt-guard`, +/// which `$CONTROLS` turns on beside it. /// The kernel's library is linted on the host apart from them: its tests as /// `--ci host` runs them, and again with `$KERNEL_CONTROLS`. const SHAPES: &[Shape] = &[ @@ -151,6 +153,23 @@ const SHAPES: &[Shape] = &[ before: &["-p", "toyos-xhci", "--all-targets"], after: &["$ADOPTED", "-D", "warnings"], }, + // `--no-deps` because its dependency `toyos` is a guest crate no shape + // lints. Its own target directory because `--no-deps` checks without clippy + // the host crates the workspace shapes lint, which `toyos-abi`'s shape + // below says costs every run. + Shape { + before: &[ + "-p", + "toyos-pci-claim", + "--all-targets", + "--features", + "host-grant", + "--no-deps", + "--target-dir", + "target/clippy-pci-claim", + ], + after: &["$ADOPTED", "-D", "warnings"], + }, Shape { before: &[ "-p", diff --git a/src/sourcegate.rs b/src/sourcegate.rs index aa98e730fe6..764dd5ecd16 100644 --- a/src/sourcegate.rs +++ b/src/sourcegate.rs @@ -140,7 +140,6 @@ const AUTO_TRAIT_IMPLS: &[(&str, usize)] = &[ ("kernel/src/drivers/hda.rs", 1), ("kernel/src/drivers/panic_console/mod.rs", 3), ("kernel/src/drivers/virtio_console.rs", 1), - ("kernel/src/drivers/virtio_sound.rs", 2), ("kernel/src/arch/x86_64/hw.rs", 1), ("kernel/src/mm/mmio.rs", 2), ("kernel/src/mm/region.rs", 2), diff --git a/system.toml b/system.toml index fdc2ef3c6c2..ba018d9f89c 100644 --- a/system.toml +++ b/system.toml @@ -74,7 +74,7 @@ login = true [programs.soundserver] service = true serves = ["soundserver"] -devices = ["hda-audio", "virtio-sound"] +devices = ["hda-audio", "pci:1af4:1059"] syscap = ["rt"] # netstack holds the NIC's PCI function and drives it: the virtqueues, the register diff --git a/tests/checks.rs b/tests/checks.rs index f48dc76f464..9905dc25759 100644 --- a/tests/checks.rs +++ b/tests/checks.rs @@ -488,7 +488,7 @@ mod checks { let xhci = if cpu == 0 { 40 } else { 0 }; format!( "[ {at} cpu{on} kernel] irq: cpu{cpu} timer=100 kick={kick} xhci={xhci} userdev=0 \ - sound=0 i8042=0 dmafault=0 hda=0 tlb={tlb} nmi=0 spurious=0 unclaimed=0\n" + i8042=0 dmafault=0 hda=0 tlb={tlb} nmi=0 spurious=0 unclaimed=0\n" ) }; let issued = |at: &str, on: u32, total: u64| { @@ -991,7 +991,7 @@ mod checks { assert!(judge(&[&readback("jobcase", &done(stopped), jobcase)]).is_err()); // What the seal itself writes of the machine, above the ring's tail. - let census = "| irq: cpu0 timer=9 kick=2 xhci=1729 userdev=0 sound=0 i8042=0 dmafault=0 hda=0 tlb=0 \ + let census = "| irq: cpu0 timer=9 kick=2 xhci=1729 userdev=0 i8042=0 dmafault=0 hda=0 tlb=0 \ nmi=0 spurious=0 unclaimed=0\n\ | tlb: shootdowns=4 wait=12us max=3us dlopen=0 pcid=0 mmio=0 unmap=4 pipe=0 staged=0 \ bench=0\n\ diff --git a/tests/common/iommu.rs b/tests/common/iommu.rs index a82220cb98b..07915f7fa4f 100644 --- a/tests/common/iommu.rs +++ b/tests/common/iommu.rs @@ -1,6 +1,7 @@ //! `iommu_virtio_platform`: whether each virtio function QEMU creates behind //! its emulated VT-d unit, and none created without one, negotiated -//! `VIRTIO_F_ACCESS_PLATFORM`. +//! `VIRTIO_F_ACCESS_PLATFORM`, and that a process's claim on one is refused +//! where no unit is. use std::collections::{BTreeMap, BTreeSet}; use std::path::Path; @@ -11,6 +12,9 @@ use super::serial::Serial; /// The function `tests/netcase`'s netstack claims, as its `devices` row spells it. const NETSTACK_CLAIMS: &str = "1af4:1041"; +/// soundserver's feature line, the kernel's shape under soundserver's name. +const SOUNDSERVER_NEGOTIATED: &str = "soundserver: VirtIO: PCI "; + /// fileserver's word for DATA's directories served from memory. const IN_MEMORY: &str = "are in memory and will not survive a reboot"; @@ -69,13 +73,20 @@ pub fn iommu_virtio_platform(test_config: &Path) -> Result<(), String> { // program's line either. let mut qemu = QemuInstance::boot_with_options(&netcase(), &[], &[], options); let said: Vec = if behind_unit { - vec![CLAIM_BOUNDED.to_string(), NETSTACK_NEGOTIATED.to_string(), bar_moved(), msix_armed()] + vec![ + NETSTACK_NEGOTIATED.to_string(), + SOUNDSERVER_NEGOTIATED.to_string(), + bar_moved(), + msix_armed(), + ] } else { vec![ DISKSERVER_REFUSED.to_string(), FILESERVER_WITHOUT_DATA.to_string(), not_handed_over(CLAIMED_AT, NOT_REMAPPED), not_handed_over(NVME_AT, NOT_REMAPPED), + not_handed_over(SOUND_AT, NOT_REMAPPED), + super::audio::NULL_SINK.to_string(), ] }; let mut text = qemu.boot_log().to_string(); @@ -99,13 +110,6 @@ pub fn iommu_virtio_platform(test_config: &Path) -> Result<(), String> { // unit has three negotiators, one of them across the boundary, and the // arm without one has two and a refusal. let expected = if behind_unit { - // And the claim netstack was given is bounded to its own function's - // configuration space, which is what makes its capability walk — - // an index by numbers the *device* wrote — safe to run at all. - // netstack asks the kernel for a read past the end, one straddling it - // and one misaligned, and refuses to drive a claim that answers any - // of them. - log.must_say(CLAIM_BOUNDED)?; // The two things a hand-over spends, on the same function and the // same machine the arm below requires to be unspent. Without this // pair those `must_not_say`s would pass against a kernel that had @@ -115,7 +119,8 @@ pub fn iommu_virtio_platform(test_config: &Path) -> Result<(), String> { created.len() } else { no_unit_is_no_claim(&log)?; - created.len() - 1 + // The NIC's and the sound function's claims, refused. + created.len() - 2 }; let mut negotiated = Vec::new(); @@ -154,26 +159,29 @@ pub fn iommu_virtio_platform(test_config: &Path) -> Result<(), String> { } let sound = class_function(&log, "0401") .ok_or_else(|| format!("{name}: this machine enumerated no audio function"))?; - if !negotiated.iter().any(|(who, _)| *who == sound) { + if sound != SOUND_AT { + return Err(format!("{name}: the audio function is {sound}, where {SOUND_AT} is owed")); + } + if behind_unit != negotiated.iter().any(|(who, _)| *who == sound) { return Err(format!( - "{name}: the audio function {sound} negotiated nothing, so whether it is behind \ - the unit was never asked: {negotiated:?}" + "{name}: the audio function {sound} negotiated features = {}, where a process's \ + claim on it is owed = {behind_unit}: {negotiated:?}", + !behind_unit )); } eprintln!( - " [iommu] {name}: {} virtio function(s) behind a unit = {behind_unit}, the audio \ - function {sound} among them{}", + " [iommu] {name}: {} virtio function(s) negotiated, behind a unit = {behind_unit}; {}", negotiated.len(), - if behind_unit { "" } else { "; the NIC's claim refused for want of a unit" } + if behind_unit { + format!("the audio function {sound} among them") + } else { + format!("the NIC's and the audio function {sound}'s claims refused for want of a unit") + } ); } declining_is_not_free(test_config) } -/// netstack's, once its claim answers nothing outside its own function. -const CLAIM_BOUNDED: &str = - "netstack: this claim answers 4096 bytes of configuration space and refuses every access \ - outside them"; /// netstack's feature line, the kernel's shape under netstack's name. const NETSTACK_NEGOTIATED: &str = "netstack: VirtIO: PCI "; const DISKSERVER_REFUSED: &str = @@ -185,6 +193,10 @@ const FILESERVER_WITHOUT_DATA: &str = /// the one its diskserver row claims. const NVME_AT: &str = "00:02.0"; +/// The slot QEMU's `-device` order puts the virtio-sound function on, the one +/// `tests/netcase`'s soundserver row claims. +const SOUND_AT: &str = "00:04.0"; + /// Why a machine with no unit hands no function over, in the kernel's words. const NOT_REMAPPED: &str = "its interrupts would not be remapped on this machine"; @@ -205,10 +217,14 @@ fn not_handed_over(at: &str, why: &str) -> String { /// mint, and netstack exits rather than driving anything — and the machine /// finishes booting, which is the half a refusal that panicked would fail. The /// NVMe controller is refused the same, and DATA with it by name: a disk that -/// is there and cannot be used is never answered with memory. +/// is there and cannot be used is never answered with memory. So is the +/// virtio-sound function, and soundserver takes its null sink. fn no_unit_is_no_claim(log: &Serial) -> Result<(), String> { // netstack's own exit is the third saying, and is not read here. - refused_claim(log, NETSTACK_CLAIMS, NOT_REMAPPED, &[NVME_AT])?; + refused_claim(log, NETSTACK_CLAIMS, NOT_REMAPPED, &[NVME_AT, SOUND_AT])?; + log.must_say(¬_handed_over(SOUND_AT, NOT_REMAPPED))?; + log.must_say("supervisor: soundserver: pci:1af4:1059 is on this machine and could not be handed over")?; + log.must_say(super::audio::NULL_SINK)?; log.must_say(¬_handed_over(NVME_AT, NOT_REMAPPED))?; log.must_say("supervisor: diskserver: pci:1b36:0010 is on this machine and could not be handed over")?; log.must_say(DISKSERVER_REFUSED)?; @@ -226,14 +242,16 @@ fn no_unit_is_no_claim(log: &Serial) -> Result<(), String> { /// `virtio_validate_features` returns `-EFAULT` and `virtio_set_status` returns /// before it stores the status (`hw/virtio/virtio.c:2270-2276` and `:2292-2299` /// at v11.1.1), so `FEATURES_OK` never sticks. The actuator withholds the bit -/// from every virtio device but the console, and each of them is refused for it. +/// from every virtio device the kernel drives but the console — on this +/// machine the virtio-gpu it adds for that, since every other function is a +/// process's claim — and each of them is refused for it. fn declining_is_not_free(test_config: &Path) -> Result<(), String> { let qemu = QemuInstance::boot_with_options( test_config, &[], &[], BootOptions { - profile: Profile::Headless, + profile: Profile::HeadlessVirtioGpu, kernel_params: &["virtio-no-access-platform"], ..Default::default() }, diff --git a/tests/common/irqcensus.rs b/tests/common/irqcensus.rs index 7dc7f901412..5501db22957 100644 --- a/tests/common/irqcensus.rs +++ b/tests/common/irqcensus.rs @@ -15,8 +15,8 @@ use std::collections::BTreeMap; /// and [`Census::parse`] refuses a line whose fields are not exactly these, so /// a source added on one side and not the other is a red rather than a silently /// dropped column. -pub const SOURCES: [&str; 12] = [ - "timer", "kick", "xhci", "userdev", "sound", "i8042", "dmafault", "hda", "tlb", "nmi", "spurious", +pub const SOURCES: [&str; 11] = [ + "timer", "kick", "xhci", "userdev", "i8042", "dmafault", "hda", "tlb", "nmi", "spurious", "unclaimed", ]; @@ -25,7 +25,7 @@ pub const SOURCES: [&str; 12] = [ /// `MSG_ADDR` names physical destination 0 and the one I/O APIC pin this kernel /// routes goes to the BSP, so today every one of these is cpu0's alone. The day /// that stops being true is the day the track's change lands. -pub const DEVICE_SOURCES: [&str; 6] = ["xhci", "userdev", "sound", "i8042", "dmafault", "hda"]; +pub const DEVICE_SOURCES: [&str; 5] = ["xhci", "userdev", "i8042", "dmafault", "hda"]; /// One CPU's counters out of one `irq:` line. #[derive(Clone, Debug, PartialEq, Eq)] diff --git a/tests/common/qemu.rs b/tests/common/qemu.rs index 448a000089d..0e204f43488 100644 --- a/tests/common/qemu.rs +++ b/tests/common/qemu.rs @@ -694,6 +694,10 @@ pub enum Profile { /// disk is the only storage it has, and the only device its firmware can /// boot. HeadlessNoUsb, + /// [`Profile::Headless`] with a virtio-gpu behind the unit: a virtio + /// function the kernel still drives itself beside the console, which is + /// what `virtio-no-access-platform` withholds the bit from. + HeadlessVirtioGpu, /// [`Profile::Headless`] with a second controller, QEMU's `qemu-xhci` /// on MSI and with no MSI-X, carrying a stick and a keyboard: the /// controller `xhci-leave=1b36:000d` leaves to `usbd`, armed on MSI as @@ -749,6 +753,7 @@ impl Profile { | Self::HeadlessNoIommu | Self::HeadlessE1000e | Self::HeadlessNoUsb + | Self::HeadlessVirtioGpu | Self::HeadlessUsbSpare | Self::Metal => Arch::X86_64, } @@ -886,6 +891,8 @@ struct Shape { /// RNDR for firmware or the kernel to draw from; a q35's firmware answers /// the protocol from RDRAND without one. rng: bool, + /// A virtio-gpu, which the kernel drives. + virtio_gpu: bool, } /// Whether `virt` has its SMMUv3. With it come two of QEMU's `iommu-testdev`, @@ -954,6 +961,7 @@ impl Profile { iommu: None, smmu: Smmu::Absent, rng: true, + virtio_gpu: false, }, Self::Headless => Shape { vga: "none", @@ -967,6 +975,7 @@ impl Profile { iommu: Some(IOMMU_DEFAULT), smmu: Smmu::Absent, rng: false, + virtio_gpu: false, }, Self::Metal => Shape { vga: "std", @@ -984,10 +993,12 @@ impl Profile { iommu: Some(IOMMU_DEFAULT), smmu: Smmu::Absent, rng: false, + virtio_gpu: false, }, Self::HeadlessNoIommu => Shape { iommu: None, ..Self::Headless.shape() }, Self::HeadlessE1000e => Shape { nic: Nic::E1000e, ..Self::Headless.shape() }, Self::HeadlessNoUsb => Shape { xhci: &[], usb: &[], ..Self::Headless.shape() }, + Self::HeadlessVirtioGpu => Shape { virtio_gpu: true, ..Self::Headless.shape() }, Self::HeadlessUsbSpare => Shape { xhci: &[XHCI_DEFAULT, XHCI_SPARE], usb: &[ @@ -2270,6 +2281,9 @@ fn qemu_command( if shape.rng { qemu.arg("-device").arg("virtio-rng-pci"); } + if shape.virtio_gpu { + qemu.arg("-device").arg(format!("virtio-gpu-pci{platform}")); + } if shape.smmu == Smmu::WithTestdev { // Two, the first enumerated below the second: the kernel's selftest // routes nothing for the first, whose entry the table holds. diff --git a/tests/logstallcase/system.toml b/tests/logstallcase/system.toml index 133b98cf3d5..6f790e6e673 100644 --- a/tests/logstallcase/system.toml +++ b/tests/logstallcase/system.toml @@ -18,7 +18,7 @@ syscap = ["logread"] [programs.soundserver] service = true serves = ["soundserver"] -devices = ["hda-audio", "virtio-sound"] +devices = ["hda-audio", "pci:1af4:1059"] syscap = ["rt"] [programs.test-runner] diff --git a/tests/metalcase/system.toml b/tests/metalcase/system.toml index 49bbffaaa84..6bbfc4c6056 100644 --- a/tests/metalcase/system.toml +++ b/tests/metalcase/system.toml @@ -31,7 +31,7 @@ devices = ["framebuffer", "keyboard", "mouse"] [programs.soundserver] service = true serves = ["soundserver"] -devices = ["hda-audio", "virtio-sound"] +devices = ["hda-audio", "pci:1af4:1059"] syscap = ["rt"] # netstack holds the NIC's PCI function and drives it: the virtqueues, the register diff --git a/tests/netcase/system.toml b/tests/netcase/system.toml index e3bbd82f857..e956a131d72 100644 --- a/tests/netcase/system.toml +++ b/tests/netcase/system.toml @@ -1,5 +1,6 @@ # The one boot that runs netstack with a virtio NIC in front of it, with diskserver's -# NVMe controller beside it: `iommu_virtio_platform`'s machine. +# NVMe controller and soundserver's virtio-sound beside it: `iommu_virtio_platform`'s +# machine, where every virtio function is a process's claim or a kernel driver's. # # netstack's `main` opens the NIC before anything else and returns on `NotFound`, # so a config with no NIC never reaches a line of the daemon proper. @@ -8,7 +9,7 @@ # announce itself. [boot] -start = ["logkeeper", "diskserver", "fileserver", "netstack", "test-runner"] +start = ["logkeeper", "diskserver", "fileserver", "netstack", "soundserver", "test-runner"] # `every_boot_config_runs_logkeeper` is what refuses a boot config without it: the # kernel keeps the record ring and writes no file, so such a boot's `/log` is @@ -26,6 +27,12 @@ service = true serves = ["netstack"] devices = ["pci:1af4:1041", "pci:8086:10d3"] +[programs.soundserver] +service = true +serves = ["soundserver"] +devices = ["pci:1af4:1059"] +syscap = ["rt"] + # `netstack` because `netstack_socket_churn` connects through it and reads its # counts. [programs.test-runner] diff --git a/tests/testcases/system.toml b/tests/testcases/system.toml index 6eb7929d323..fe24cfb7756 100644 --- a/tests/testcases/system.toml +++ b/tests/testcases/system.toml @@ -26,7 +26,7 @@ receives = ["power"] [programs.soundserver] service = true serves = ["soundserver"] -devices = ["hda-audio", "virtio-sound"] +devices = ["hda-audio", "pci:1af4:1059"] syscap = ["rt"] # The test estate's authority. diff --git a/tests/toyos-rust-tests/src/bin/isa_lines.rs b/tests/toyos-rust-tests/src/bin/isa_lines.rs index f35033a7084..7eac42c5106 100644 --- a/tests/toyos-rust-tests/src/bin/isa_lines.rs +++ b/tests/toyos-rust-tests/src/bin/isa_lines.rs @@ -153,7 +153,7 @@ fn record(claim: &Device) -> Option { match syscall::read_nonblock(claim.as_handle(), &mut record) { Ok(n) => { assert_eq!(n, DeviceIrqRecord::SIZE, "isa: a record of {n} bytes"); - Some(u32::from_ne_bytes(record)) + Some(u32::from_ne_bytes([record[0], record[1], record[2], record[3]])) } Err(SyscallError::WouldBlock) => None, Err(other) => panic!("isa: a bound claim's read answered {other:?}"), diff --git a/tests/toyos-rust-tests/src/bin/virtio_sound_counts.rs b/tests/toyos-rust-tests/src/bin/virtio_sound_counts.rs new file mode 100644 index 00000000000..d0072d7d52e --- /dev/null +++ b/tests/toyos-rust-tests/src/bin/virtio_sound_counts.rs @@ -0,0 +1,134 @@ +//! One stream through soundserver's own virtio-sound driver, judged by what +//! soundserver counted and in what state it left the device — never by how long +//! anything took. +//! +//! soundserver suspends only once every period it submitted has come back from +//! the device, so a stream that ends with soundserver suspended is one whose +//! every submitted period completed; and the periods it submitted cover at +//! least the periods this client filled. With no client its wait has no +//! timeout, so the suspend is reached only through the claim's interrupts: a +//! transmit queue with no vector leaves it `running`. + +use std::sync::mpsc; +use std::time::{Duration, Instant}; + +use cpal::traits::{DeviceTrait, HostTrait, StreamTrait}; +use toyos_inspect::{Value, SOUND}; + +/// Periods of tone the client fills: a few laps of the eight-period pipeline. +const PERIODS: u64 = 64; + +/// A hang ceiling on each wait here, none of which is more than a second of +/// audio away. +const WITHIN: Duration = Duration::from_secs(30); + +const FREQ_HZ: f64 = 440.0; +const AMPLITUDE: f64 = 16000.0; + +/// What soundserver's `inspect` answer says of its device and stream. +struct Sound { + device: String, + state: String, + clients: u64, + submitted: u64, + period_frames: u64, + rate: u64, +} + +fn sound() -> Sound { + let answer = inspect::ask(SOUND).unwrap_or_else(|why| panic!("soundserver's inspect answer: {why}")); + let text = |path: &str| match answer.get(path) { + Some(Value::Text(text)) => text.clone(), + other => panic!("soundserver's snapshot has no text at {path}: {other:?}"), + }; + let number = |path: &str| match answer.get(path) { + Some(&Value::U64(n)) => n, + other => panic!("soundserver's snapshot has no number at {path}: {other:?}"), + }; + Sound { + device: text("sound.device"), + state: text("sound.stream.state"), + clients: number("sound.stream.clients"), + submitted: number("sound.periods.submitted"), + period_frames: number("sound.period_frames"), + rate: number("sound.rate_hz"), + } +} + +fn main() { + let before = sound(); + assert_eq!(before.device, "virtio-sound", "soundserver drives no virtio-sound device"); + assert_eq!( + (before.state.as_str(), before.clients), + ("suspended", 0), + "soundserver had a stream before this client's: no count here is this client's alone" + ); + + let filled = play(PERIODS * before.period_frames); + + // soundserver says nothing to a client that left; its answer is where the + // suspend is read, and the device playing its tail out wakes it once a + // period. + let deadline = Instant::now() + WITHIN; + let after = loop { + let now = sound(); + if now.state == "suspended" && now.clients == 0 { + break now; + } + assert!( + Instant::now() < deadline, + "soundserver's stream still reads `{}` with {} client(s) {WITHIN:?} after its only client \ + closed: a submitted period never came back", + now.state, + now.clients + ); + std::thread::sleep(Duration::from_nanos(1_000_000_000 * now.period_frames / now.rate)); + }; + + let submitted = after.submitted - before.submitted; + let covered = filled / before.period_frames; + assert!( + submitted >= covered, + "soundserver submitted {submitted} period(s) for a client that filled {covered}" + ); + println!( + "virtio_sound_counts: {submitted} period(s) submitted for {covered} filled, every one \ + completed before the stream stopped" + ); +} + +/// Play a tone until `frames` have been filled, and close the stream: the +/// frames filled. +fn play(frames: u64) -> u64 { + let host = cpal::default_host(); + let device = host.default_output_device().expect("no audio output device"); + let config = device.default_output_config().expect("no audio config"); + let sample_rate = config.sample_rate() as f64; + let channels = config.channels() as usize; + let (done, filled) = mpsc::channel(); + let mut n: u64 = 0; + let stream = device + .build_output_stream( + config.into(), + move |data: &mut [i16], _: &cpal::OutputCallbackInfo| { + for frame in data.chunks_exact_mut(channels) { + let phase = 2.0 * std::f64::consts::PI * FREQ_HZ * n as f64 / sample_rate; + frame.fill((AMPLITUDE * phase.sin()) as i16); + n += 1; + } + if n >= frames { + // The receiver is gone once one has been taken. + let _ = done.send(n); + } + }, + |err| panic!("the stream reported an error: {err}"), + None, + ) + .expect("failed to build audio stream"); + stream.play().expect("failed to play"); + let filled = filled + .recv_timeout(WITHIN) + .unwrap_or_else(|why| panic!("{frames} frames were not filled within {WITHIN:?}: {why}")); + drop(stream); + filled +} diff --git a/tests/toyos.rs b/tests/toyos.rs index 5dc34455148..d3f573ce03a 100644 --- a/tests/toyos.rs +++ b/tests/toyos.rs @@ -186,6 +186,10 @@ const RUST_SKIP: &[&str] = &[ // It claims QEMU's virtio NIC, which the T14 has none of: `bar_map_again` // runs it. "bar_map_again", + // It plays through soundserver's virtio-sound driver, which the T14 has no + // device for, and its counts are judged beside soundserver's lines: + // `virtio_sound_counts` runs it. + "virtio_sound_counts", // It claims the NVMe controller a boot off that disk starts no server // for, and reads the inventory: `block_grants_reach_their_partitions` // runs it on tests/blockgrantcase. @@ -375,6 +379,10 @@ const MACHINE_TESTS: &[&str] = &[ // function is its I219, and a kernel that dies on this takes the bench's // machine down with it. "bar_map_again", + // A stream through soundserver's own virtio-sound driver, counted and + // ordered: the device exists only on a QEMU machine — the T14's sound is + // HDA — and its audio goes nowhere, since no guest test plays any. + "virtio_sound_counts", // `console/system.toml`'s image, which runs no job and hands no machine // back: a metal boot of it ends with a hand on the power button. "console_image_boots", @@ -3696,6 +3704,7 @@ fn run_machine_test(name: &str, test_config: &Path) -> Result<(), String> { "machine_shutdown_wire_kept" => power::machine_shutdown_wire_held(test_config, true), "machine_shutdown_wire_at_the_seal" => power::machine_shutdown_wire_at_the_seal(test_config), "bar_map_again" => bar_map_again(test_config), + "virtio_sound_counts" => virtio_sound_counts(test_config), "console_image_boots" => console_image_boots(), "nvme_disk_keeps_log_and_home" => nvme_disk_keeps_log_and_home(test_config), "block_grants_reach_their_partitions" => block_grants_reach_their_partitions(), @@ -4173,6 +4182,69 @@ fn bar_map_again(test_config: &Path) -> Result<(), String> { Ok(()) } +/// soundserver brings the virtio-sound function up itself, behind the unit, and +/// plays one stream through it to the end: the job's counts (`inspect`'s), and +/// soundserver's lines in their order — the stream started once after the +/// client connected, and stopped once, before soundserver suspended, with no +/// refusal on the way. +fn virtio_sound_counts(test_config: &Path) -> Result<(), String> { + const JOB: &str = "virtio_sound_counts"; + /// soundserver's bring-up, which it says before any client is accepted. + const BROUGHT_UP: [&str; 3] = [ + "virtio-sound: configured stream 0: 44100Hz 2ch s16le", + "soundserver: ready, 8 buffers, 44100Hz 2ch, 512 bytes/period, 128 frames/period", + "soundserver: suspended", + ]; + /// The stream's lines, in the order they are owed, each once. + const IN_ORDER: [&str; 6] = [ + " connected (id=", + "soundserver: resumed", + "virtio-sound: stream 0 started", + " removed (", + "virtio-sound: stream 0 stopped", + "soundserver: suspended", + ]; + let bin = qemu::build_toyos_bin(qemu::SUITE_ARCH, &compile::repo_root().join("tests/toyos-rust-tests"), JOB); + let mut qemu = + QemuInstance::boot_with_options(test_config, &[], &[(JOB.to_string(), bin)], BootOptions::default()); + let mut console = qemu.boot_log().to_string(); + await_guest(&mut qemu, &mut console, "soundserver's bring-up", |c| BROUGHT_UP.iter().all(|l| c.contains(l))) + .map_err(|e| format!("{e}\n{console}"))?; + let boot = serial::Serial::named("the boot", console); + boot.must_be_clean()?; + boot.must_say("soundserver: VirtIO: PCI ")?; + boot.must_say("access_platform=y")?; + boot.must_not_say(audio::NULL_SINK)?; + + let result = qemu.run_test(&format!("test_rs_{JOB}"), Duration::from_secs(120)); + if let Some(why) = &result.error { + return Err(format!("{why}\nthe job said:\n{}", result.stdout)); + } + if result.exit_code != Some(0) { + return Err(format!("the job ended {:?}:\n{}", result.exit_code, result.stdout)); + } + let said = &result.stdout; + let Some(counted) = said.lines().find(|l| l.contains("virtio_sound_counts: ")) else { + return Err(format!("the job never said what it counted:\n{said}")); + }; + if said.contains("cannot be driven on") { + return Err(format!("soundserver refused its device mid-stream:\n{said}")); + } + let mut from = 0; + for line in IN_ORDER { + let times = said.matches(line).count(); + let Some(at) = said[from..].find(line) else { + return Err(format!("`{line}` is not after the line before it in the window:\n{said}")); + }; + if times != 1 { + return Err(format!("`{line}` said {times} times in one stream's window:\n{said}")); + } + from += at + line.len(); + } + eprintln!(" [sound] {}", counted.trim()); + Ok(()) +} + /// Every device delivery is still cpu0's, and no CPU took a shootdown IPI the /// issuer did not count. /// diff --git a/toyos-abi/src/audio.rs b/toyos-abi/src/audio.rs index 2123cf52cf5..8e8098c9613 100644 --- a/toyos-abi/src/audio.rs +++ b/toyos-abi/src/audio.rs @@ -4,15 +4,13 @@ use core::sync::atomic::AtomicU32; /// One batch of DMA buffer completions, recorded at interrupt time. /// -/// Reads on a sound device's handle (after the initial info read) return an +/// Reads on the HDA claim's handle (after the initial info read) return an /// array of these: the kernel writes as many pending records as fit in the caller's /// buffer and returns the byte count. `mask` bit N set means period N finished /// playing, and `timestamp_nanos` is `nanos_since_boot` captured in the /// interrupt handler — the clock source for soundserver's DLL, and the reason the /// mask is derived there rather than by the driver at wake time. Records are /// returned oldest-first. -/// -/// Both stubs produce it, so the two backends differ in nothing a mixer sees. #[repr(C)] #[derive(Clone, Copy)] pub struct AudioCompletionRecord { diff --git a/toyos-abi/src/hda.rs b/toyos-abi/src/hda.rs index d71960cc5b5..bfc516ea9dd 100644 --- a/toyos-abi/src/hda.rs +++ b/toyos-abi/src/hda.rs @@ -7,13 +7,8 @@ //! against an allow-list and refused by name. Nothing here names a physical //! address. //! -//! [`RegWidth`](crate::syscall::RegWidth) is those calls' and not this device's, -//! since virtio-sound's stub reaches its notification registers the same way. -//! //! Completions come back as [`AudioCompletionRecord`](crate::audio::AudioCompletionRecord), -//! the same record the virtio-sound stub produces, because the mask is derived -//! from a position read in the interrupt handler and the two backends then -//! differ in nothing a mixer can see. +//! whose mask is derived from a position read in the interrupt handler. //! //! [`syscall::device_reg_read`]: crate::syscall::device_reg_read //! [`syscall::device_reg_write`]: crate::syscall::device_reg_write diff --git a/toyos-abi/src/inventory.rs b/toyos-abi/src/inventory.rs index ec93d304d1b..d819aa2410c 100644 --- a/toyos-abi/src/inventory.rs +++ b/toyos-abi/src/inventory.rs @@ -533,7 +533,7 @@ mod tests { state: PartState::Kernel, }), Record::Claim(Claim { on: Claimed::Pci(at), holder }), - Record::Claim(Claim { on: Claimed::Class(DeviceType::VirtioSound), holder }), + Record::Claim(Claim { on: Claimed::Class(DeviceType::HdaAudio), holder }), Record::Claim(Claim { on: Claimed::Partition { device: 3, unique_guid: [0x11; 16] }, holder, diff --git a/toyos-abi/src/lib.rs b/toyos-abi/src/lib.rs index 469b1c72882..6aa383ad4c9 100644 --- a/toyos-abi/src/lib.rs +++ b/toyos-abi/src/lib.rs @@ -32,7 +32,6 @@ pub mod ring; pub mod syscall; pub mod trace; pub mod usersafe; -pub mod virtio_sound; pub use handle::{RawHandle, Rights, HANDLE_INVALID}; pub use usersafe::UserSafe; diff --git a/toyos-abi/src/pci.rs b/toyos-abi/src/pci.rs index 3323e5a0e5c..2e22bb2965c 100644 --- a/toyos-abi/src/pci.rs +++ b/toyos-abi/src/pci.rs @@ -86,18 +86,28 @@ const _: () = assert!(core::mem::size_of::() == 8 + 8); /// 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 — and it is the whole record, because what a message -/// meant is in the device's own rings and a driver that wanted a time has -/// its clock page (`crate::clock`). +/// what any of them meant: that is in the device's own rings. +/// +/// **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 — +/// is clocked by these and not by when it woke. Both are nanoseconds since +/// boot on the clock page's clock (`crate::clock`). #[repr(C)] #[derive(Clone, Copy)] 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. + 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. + pub last_nanos: u64, } impl DeviceIrqRecord { pub const SIZE: usize = core::mem::size_of::(); } -const _: () = assert!(DeviceIrqRecord::SIZE == 4); +const _: () = assert!(DeviceIrqRecord::SIZE == 4 + 4 + 8 + 8); diff --git a/toyos-abi/src/syscall.rs b/toyos-abi/src/syscall.rs index 4d7f398da6a..c09675841eb 100644 --- a/toyos-abi/src/syscall.rs +++ b/toyos-abi/src/syscall.rs @@ -1284,11 +1284,6 @@ device_classes! { /// An Intel HDA controller the kernel has brought up but drives no policy /// on. HdaAudio = 5 => "hda-audio", - /// A virtio-sound device, on the same terms: the kernel negotiated its - /// features, built its virtqueues and owns their descriptors, and every - /// decision above that — the stream, the rate, the format, when a period is - /// published — belongs to whoever holds this. - VirtioSound = 6 => "virtio-sound", /// One PCI function, driven by whoever holds the claim: the kernel binds no /// driver to it, keeps config space, puts it in an address space of its own /// before it lets it master the bus, and programs its interrupt vector. diff --git a/toyos-abi/src/virtio_sound.rs b/toyos-abi/src/virtio_sound.rs deleted file mode 100644 index c5dbdcc83a5..00000000000 --- a/toyos-abi/src/virtio_sound.rs +++ /dev/null @@ -1,149 +0,0 @@ -//! What the kernel's virtio-sound stub hands its driver. -//! -//! The line through this device is the one HDA's is: **the kernel writes every -//! address.** A split virtqueue names memory in exactly one place, its -//! descriptor table, and the three tables here live in a page no process maps. -//! What a driver gets is the region those descriptors point into, the two ring -//! indices that select one of them, and one register write to say it has. -//! -//! So the layout below is the whole interface. It is a set of constants rather -//! than fields of [`VirtioSoundInfo`] because both halves have to agree on it -//! for the descriptors to mean anything, and a number the kernel reports is a -//! number a driver could believe instead of the one the descriptors were built -//! from. - -/// The pipeline, in periods and bytes. -pub const PERIODS: usize = 8; -pub const PERIOD_BYTES: usize = 512; - -/// Virtqueue indices, fixed by the virtio sound device specification. -pub const CONTROL_QUEUE: u16 = 0; -pub const EVENT_QUEUE: u16 = 1; -pub const TX_QUEUE: u16 = 2; - -/// Descriptors per queue. Powers of two, as virtio 1.2 §2.7 requires. -/// -/// The TX queue holds one three-descriptor chain per period and nothing else, -/// so its slots are exhausted by [`PERIODS`] and never by a driver's pace. -pub const TX_QUEUE_SIZE: u16 = 32; -pub const CONTROL_QUEUE_SIZE: u16 = 16; -pub const EVENT_QUEUE_SIZE: u16 = 16; - -/// Descriptors in one TX chain: the transfer header, the PCM period, and the -/// status the device writes back. -pub const TX_CHAIN: u16 = 3; - -/// The first descriptor of the chain carrying period `idx`. -/// -/// A function and not a search: the chains are built once, at bind, and the -/// driver publishes one by index. It never writes a descriptor, so there is -/// nothing here for it to get wrong beyond naming the wrong period. -pub const fn tx_chain_head(idx: usize) -> u16 { - idx as u16 * TX_CHAIN -} - -/// How many buffers the event queue keeps posted. -pub const EVENT_BUFS: usize = 8; -/// Stride between them. The event structure is eight bytes; the rest is so a -/// buffer index and a descriptor index are the same number. -pub const EVENT_BUF_STRIDE: usize = 16; - -/// Stride between transfer headers, for the same reason. -pub const XFER_STRIDE: usize = 16; -/// Stride between the per-period status structures the device writes. -pub const STATUS_STRIDE: usize = 8; - -/// How much of a control request or response the descriptors describe. -/// -/// One pair of buffers serves every command, so the length is fixed at bind and -/// has to cover the longest of them. The device reads the header first and takes -/// only what that command defines, so a buffer longer than the message is not a -/// message with garbage after it. -pub const CTRL_BUF_BYTES: usize = 128; - -/// An avail ring's bytes: flags, index, and one descriptor index per slot. -pub const fn avail_bytes(size: u16) -> usize { - 4 + size as usize * 2 -} -/// A used ring's: flags, index, and one `(id, len)` pair per slot. -pub const fn used_bytes(size: u16) -> usize { - 4 + size as usize * 8 -} - -// Byte offsets into the shared region. Every ring here is one the driver -// publishes to or reads from; the TX used ring is absent because the interrupt -// handler is its only consumer, and the descriptor tables are absent because -// they are the addresses. -pub const OFF_PCM: usize = 0x0000; -pub const OFF_TX_AVAIL: usize = 0x1000; -pub const OFF_TX_XFER: usize = 0x1080; -pub const OFF_TX_STATUS: usize = 0x1100; -pub const OFF_CTRL_AVAIL: usize = 0x1200; -pub const OFF_CTRL_USED: usize = 0x1240; -pub const OFF_CTRL_REQ: usize = 0x1300; -pub const OFF_CTRL_RESP: usize = 0x1400; -pub const OFF_EVENT_AVAIL: usize = 0x1500; -pub const OFF_EVENT_USED: usize = 0x1540; -pub const OFF_EVENT_BUFS: usize = 0x1600; -pub const SHARED_BYTES: usize = 0x2000; - -/// Nothing overlaps and everything fits. -/// -/// The device writes four of these regions and the driver writes three, at -/// addresses the kernel computed once — so an overlap is not a bug a test would -/// find, it is one period of PCM landing on a used ring. -const _: () = { - let regions = [ - (OFF_PCM, PERIODS * PERIOD_BYTES), - (OFF_TX_AVAIL, avail_bytes(TX_QUEUE_SIZE)), - (OFF_TX_XFER, PERIODS * XFER_STRIDE), - (OFF_TX_STATUS, PERIODS * STATUS_STRIDE), - (OFF_CTRL_AVAIL, avail_bytes(CONTROL_QUEUE_SIZE)), - (OFF_CTRL_USED, used_bytes(CONTROL_QUEUE_SIZE)), - (OFF_CTRL_REQ, CTRL_BUF_BYTES), - (OFF_CTRL_RESP, CTRL_BUF_BYTES), - (OFF_EVENT_AVAIL, avail_bytes(EVENT_QUEUE_SIZE)), - (OFF_EVENT_USED, used_bytes(EVENT_QUEUE_SIZE)), - (OFF_EVENT_BUFS, EVENT_BUFS * EVENT_BUF_STRIDE), - ]; - let mut i = 0; - while i < regions.len() { - let (start, len) = regions[i]; - assert!(start + len <= SHARED_BYTES); - let mut j = i + 1; - while j < regions.len() { - let (other, other_len) = regions[j]; - assert!(start + len <= other || other + other_len <= start); - j += 1; - } - i += 1; - } - assert!(PERIODS * TX_CHAIN as usize <= TX_QUEUE_SIZE as usize); - assert!(EVENT_BUFS <= EVENT_QUEUE_SIZE as usize); -}; - -crate::user_safe! { - /// The device the kernel brought up, as the driver needs to see it. - /// - /// No physical address and no register window: a shared-memory token, the three - /// offsets inside the notification region that are the driver's whole write - /// surface, and what the device said about itself in its configuration space. - #[derive(Clone, Copy)] - pub struct VirtioSoundInfo { - /// The shared region, mapped writable, laid out by the constants above. - pub dma: crate::RawHandle, - /// Byte offsets into the notification region — the offset space - /// [`device_reg_write`] names for this device, and the only one it has. - /// - /// [`device_reg_write`]: crate::syscall::device_reg_write - pub notify_control: u32, - pub notify_event: u32, - pub notify_tx: u32, - /// The device's configuration space, read once by the kernel, which decided - /// nothing with it. Which stream to open and at what rate is the driver's. - pub jacks: u32, - pub streams: u32, - pub chmaps: u32, - } -} - diff --git a/toyos-inspect/src/dev.rs b/toyos-inspect/src/dev.rs index 3f917e4b085..f16b335221c 100644 --- a/toyos-inspect/src/dev.rs +++ b/toyos-inspect/src/dev.rs @@ -261,7 +261,7 @@ mod tests { Record::Claim(Claim { on: Claimed::Pci(NIC), holder: netstack }), // The same claim through a second handle in the same table. Record::Claim(Claim { on: Claimed::Pci(NIC), holder: netstack }), - Record::Claim(Claim { on: Claimed::Class(DeviceType::VirtioSound), holder: holder(7, "soundserver") }), + Record::Claim(Claim { on: Claimed::Class(DeviceType::HdaAudio), holder: holder(7, "soundserver") }), Record::Claim(Claim { on: Claimed::Partition { device: 1, unique_guid: [0xcd; 16] }, holder: holder(12, "test-runner"), @@ -304,7 +304,7 @@ mod tests { text("dev.disk.0.part1.unique").as_deref(), Some("cdcdcdcd-cdcd-cdcd-cdcd-cdcdcdcdcdcd") ); - assert_eq!(text("dev.class.virtio-sound.holder.7").as_deref(), Some("soundserver")); + assert_eq!(text("dev.class.hda-audio.holder.7").as_deref(), Some("soundserver")); } #[test] diff --git a/toyos-manifest/src/lib.rs b/toyos-manifest/src/lib.rs index e8b5c0c45ad..6d1745736ba 100644 --- a/toyos-manifest/src/lib.rs +++ b/toyos-manifest/src/lib.rs @@ -553,7 +553,7 @@ mod tests { name: "soundserver".into(), path: "/system/bin/soundserver".into(), serves: vec!["soundserver".into()], - devices: vec!["hda-audio".into(), "virtio-sound".into()], + devices: vec!["hda-audio".into(), "pci:1af4:1059".into()], syscap: vec!["rt".into()], service: true, ..Program::default() diff --git a/toyos-virtio/src/lib.rs b/toyos-virtio/src/lib.rs index ff89774760a..49c91676456 100644 --- a/toyos-virtio/src/lib.rs +++ b/toyos-virtio/src/lib.rs @@ -4,8 +4,11 @@ //! Every `§` in this crate is a section of *Virtual I/O Device (VIRTIO) //! Version 1.2*, OASIS Committee Specification 01. [`pci`] is §4.1 with the //! initialisation §3.1.1 orders and the negotiation §2.2 bounds; [`queue`] is -//! §2.7. A device type — its feature bits, its configuration fields, what its -//! buffers carry — is its driver's and is not here. +//! §2.7. [`pci::vendor_caps`] walks the capability list through a +//! [`pci::ConfigSpace`] the driver's claim answers, under §6.7 of the *PCI +//! Local Bus Specification* 3.0. A device type — its feature bits, its +//! configuration fields, what its buffers carry — is its driver's and is not +//! here. //! //! # The boundary //! @@ -49,8 +52,6 @@ //! §2.7.10.1 permits whatever `used.flags` says — reading it would buy one //! skipped register write for one more device-written word believed. //! - **No packed ring, no indirect descriptors, no legacy interface.** -//! - **No walk of configuration space**: [`pci::Layout::of`] takes the vendor -//! capabilities a driver read through its claim. #![no_std] #![forbid(unsafe_code)] diff --git a/toyos-virtio/src/pci.rs b/toyos-virtio/src/pci.rs index 6c08b11757e..642ec8e0e55 100644 --- a/toyos-virtio/src/pci.rs +++ b/toyos-virtio/src/pci.rs @@ -61,6 +61,10 @@ pub mod status { pub const FAILED: u8 = 128; } +/// §4.1.5.1.2: the vector field value that maps a source to no MSI-X entry, +/// so it raises no interrupt at all. +pub const NO_VECTOR: u16 = 0xFFFF; + /// §6: "This indicates compliance with this specification". pub const VIRTIO_F_VERSION_1: u64 = 1 << 32; /// §6: the device reaches memory through addresses the platform translates, @@ -81,6 +85,96 @@ pub struct VendorCap { pub notify_off_multiplier: u32, } +/// A function's configuration space, as a driver reads it through its claim. +/// +/// Each read is one access of the width its name says, at an offset the walk +/// has already aligned for it, and answers the claim's refusal unchanged. +pub trait ConfigSpace { + type Refused: Copy + core::fmt::Debug + PartialEq + Eq; + fn read8(&self, at: u16) -> Result; + fn read32(&self, at: u16) -> Result; +} + +/// *PCI Local Bus Specification* 3.0 §6.2.3: `Status`, whose bit 4 says the +/// capabilities pointer is valid; §6.7: the pointer, the vendor-specific +/// capability's id, and the first offset past the predefined header, below +/// which no capability is. +const STATUS: u16 = 0x06; +const STATUS_CAPABILITIES: u8 = 1 << 4; +const CAPABILITIES_PTR: u16 = 0x34; +const CAP_ID_VENDOR: u8 = 0x09; +const FIRST_CAP: u8 = 0x40; +/// Every dword-aligned place a capability can be in the 256-byte header: a +/// walk that has taken more links than this has come back round. +const CAP_PLACES: usize = (0x100 - FIRST_CAP as usize) / 4; + +/// Why a capability list was not walked to its end. +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +pub enum WalkRefusal { + /// The claim refused a read the walk had to make, in its own word. + Read { at: u16, why: E }, + /// A link points into the predefined header (§6.7). + IntoHeader { at: u8 }, + /// The list came back round on itself. + Looped, +} + +impl core::fmt::Display for WalkRefusal { + fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result { + match self { + Self::Read { at, why } => { + write!(f, "its configuration space refused a read at {at:#x}: {why:?}") + } + Self::IntoHeader { at } => { + write!(f, "its capability list links to {at:#x}, inside the PCI header") + } + Self::Looped => write!(f, "its capability list never ends"), + } + } +} + +/// The vendor-specific capabilities a function publishes (§4.1.4), in its +/// list's order: what [`Layout::of`] chooses among. +/// +/// Every link is the device's: each is masked of the two bits §6.7 reserves +/// before it is an offset, a link into the header is refused, and so is a +/// list longer than the header has places for. A read the claim refuses ends +/// the walk under that refusal's own word. +pub fn vendor_caps(config: &C) -> Result, WalkRefusal> { + let read8 = |at: u16| config.read8(at).map_err(|why| WalkRefusal::Read { at, why }); + let read32 = |at: u16| config.read32(at).map_err(|why| WalkRefusal::Read { at, why }); + let mut found = Vec::new(); + if read8(STATUS)? & STATUS_CAPABILITIES == 0 { + return Ok(found); + } + let mut next = read8(CAPABILITIES_PTR)? & !3; + let mut taken = 0; + while next != 0 { + if next < FIRST_CAP { + return Err(WalkRefusal::IntoHeader { at: next }); + } + if taken == CAP_PLACES { + return Err(WalkRefusal::Looped); + } + taken += 1; + let at = next as u16; + if read8(at)? == CAP_ID_VENDOR { + let cfg_type = read8(at + 3)?; + found.push(VendorCap { + cfg_type, + bar: read8(at + 4)?, + offset: read32(at + 8)?, + length: read32(at + 12)?, + // §4.1.4.4: present after the notification structure's + // capability and no other's. + notify_off_multiplier: if cfg_type == CFG_NOTIFY { read32(at + 16)? } else { 0 }, + }); + } + next = read8(at + 1)? & !3; + } + Ok(found) +} + /// Which of a device's interrupt sources. #[derive(Clone, Copy, Debug, PartialEq, Eq)] pub enum Source { @@ -278,10 +372,10 @@ impl Offer { /// and nothing more is written to it. pub fn acknowledge(regs: R, layout: &Layout) -> Result { let window = regs.bytes(); - // §4.1.4.3.1 and §4.1.4.4.1: the alignment of each offset. + // §4.1.4.3.1, §4.1.4.4.1 and §4.1.4.6.1: the alignment of each offset. let common = Region::of(&layout.common, "COMMON_CFG", window, common::BYTES, 4)?; let notify = Region::of(&layout.notify, "NOTIFY_CFG", window, 0, 2)?; - let device = Region::of(&layout.device, "DEVICE_CFG", window, 0, 1)?; + let device = Region::of(&layout.device, "DEVICE_CFG", window, 0, 4)?; let mut wires = Wires { regs, common: common.at, @@ -425,6 +519,17 @@ impl Setup { Ok((self, byte)) } + /// The 32-bit field `at` bytes into the device-specific structure, read + /// 32 bits wide as §4.1.3.1 has a 32-bit field read. + pub fn device_read32(mut self, at: usize) -> Result<(Self, u32), Refusal> { + let Region { at: base, bytes } = self.wires.device; + if at.checked_add(4).is_none_or(|end| end > bytes) || !at.is_multiple_of(4) { + return Err(self.wires.refuse(Refusal::PastDeviceConfig { at, bytes })); + } + let word = self.wires.regs.read32(base + at); + Ok((self, word)) + } + /// §3.1.1 step 8: the device is live. pub fn driver_ok(mut self) -> Live { self.wires.set_status(status::DRIVER_OK); diff --git a/toyos-virtio/src/stub.rs b/toyos-virtio/src/stub.rs index a7c0e2f36c7..1a96a506701 100644 --- a/toyos-virtio/src/stub.rs +++ b/toyos-virtio/src/stub.rs @@ -364,9 +364,13 @@ impl State { let value = write.expect("stub: a read of the notification structure"); self.notify(at - NOTIFY_AT as usize, bytes, value); } else if device.contains(&at) { - assert!(write.is_none() && bytes == 1, "stub: the device structure is bytes, read"); - self.trace.push(Event::DeviceConfig { at: at - DEVICE_AT as usize }); - return self.device_config[at - DEVICE_AT as usize] as u32; + assert!(write.is_none(), "stub: the device structure is read and never written"); + let at = at - DEVICE_AT as usize; + assert!(at + bytes <= self.device_config.len(), "stub: a read past the device structure"); + self.trace.push(Event::DeviceConfig { at }); + let mut word = [0u8; 4]; + word[..bytes].copy_from_slice(&self.device_config[at..at + bytes]); + return u32::from_le_bytes(word); } else { panic!("stub: an access at {at:#x}, which is in none of the device's structures"); } diff --git a/toyos-virtio/src/tests.rs b/toyos-virtio/src/tests.rs index 4a426bee9c6..5fc3de419a0 100644 --- a/toyos-virtio/src/tests.rs +++ b/toyos-virtio/src/tests.rs @@ -7,12 +7,12 @@ use std::vec::Vec; use toyos_untrusted::Refused; use crate::pci::{ - status, Layout, Live, Offer, Refusal, Setup, Source, VendorCap, VIRTIO_F_ACCESS_PLATFORM, - VIRTIO_F_VERSION_1, + status, Layout, Live, Offer, Refusal, Setup, Source, VendorCap, WalkRefusal, + VIRTIO_F_ACCESS_PLATFORM, VIRTIO_F_VERSION_1, }; use crate::queue::{desc_bytes, Buffer, Parts, Used, UsedRefusal, Virtqueue}; use crate::stub::{ - Event, Machine, Ring, BAR_BYTES, COMMON_AT, DEVICE_BASE, FEATURE_OFFERED, FEATURE_WITHHELD, + Event, Machine, Ring, BAR_BYTES, COMMON_AT, DEVICE_AT, DEVICE_BASE, FEATURE_OFFERED, FEATURE_WITHHELD, GRANT_BYTES, NOTIFY_AT, NOTIFY_OFF_MULTIPLIER, QUEUES, }; @@ -139,6 +139,9 @@ fn a_structure_its_bar_does_not_hold_is_refused_and_never_reached() { // notification structure's. assert_eq!(refused(1, COMMON_AT + 2, 0x38), Some(Refusal::Misaligned("COMMON_CFG"))); assert_eq!(refused(2, NOTIFY_AT + 1, 0x100), Some(Refusal::Misaligned("NOTIFY_CFG"))); + // §4.1.4.6.1: "The offset for the device-specific configuration MUST be + // 4-byte aligned", which is what a 32-bit field read there rests on. + assert_eq!(refused(4, DEVICE_AT + 2, 6), Some(Refusal::Misaligned("DEVICE_CFG"))); // A structure that ends on the BAR's last byte is inside it. assert_eq!(refused(2, bar - 0x1000, 0x1000), None); } @@ -434,6 +437,163 @@ fn a_field_past_the_device_structure_is_refused_and_not_read() { } } +/// §4.1.3.1: "32-bit wide and aligned accesses for 32-bit … wide fields". A +/// field that is not a whole aligned dword of the structure is refused and not +/// read. +#[test] +fn a_32_bit_field_is_read_32_bits_wide_and_only_inside_the_structure() { + let machine = Machine::new(); + let device = setup(&machine); + machine.forget_trace(); + let (_, word) = device.device_read32(0).unwrap_or_else(|why| panic!("dword 0 is inside: {why}")); + assert_eq!(word, 0x1200_5452); + assert_eq!(machine.trace(), [Event::DeviceConfig { at: 0 }]); + // Past the six bytes, straddling their end, off a dword, and wrapping. + for at in [8, 4, 2, usize::MAX - 1] { + let machine = Machine::new(); + let device = setup(&machine); + machine.forget_trace(); + assert_eq!(device.device_read32(at).err(), Some(Refusal::PastDeviceConfig { at, bytes: 6 })); + assert_eq!(machine.trace(), [Event::Status(11 | status::FAILED)]); + } +} + +// --- PCI 3.0 §6.7: the capability list --- + +/// A function's configuration space: 256 bytes, and the offsets a claim +/// refuses. +struct Config { + bytes: [u8; 256], + refused: Vec, +} + +impl Config { + /// Capabilities `at`, each linked to the next and the last to none, the + /// pointer at the first, and `Status` saying there is a list. + fn listing(at: &[u8]) -> Self { + let mut config = Self { bytes: [0; 256], refused: Vec::new() }; + config.bytes[0x06] = 1 << 4; + config.bytes[0x34] = at.first().copied().unwrap_or(0); + for (nth, &cap) in at.iter().enumerate() { + config.bytes[cap as usize + 1] = at.get(nth + 1).copied().unwrap_or(0); + } + config + } + + /// A vendor capability (§4.1.4) at `at`. + fn vendor(&mut self, at: u8, cfg_type: u8, bar: u8, offset: u32, length: u32, mult: u32) { + let at = at as usize; + self.bytes[at] = 0x09; + self.bytes[at + 3] = cfg_type; + self.bytes[at + 4] = bar; + self.bytes[at + 8..at + 12].copy_from_slice(&offset.to_le_bytes()); + self.bytes[at + 12..at + 16].copy_from_slice(&length.to_le_bytes()); + self.bytes[at + 16..at + 20].copy_from_slice(&mult.to_le_bytes()); + } +} + +/// What a claim answers for a read it refused. +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +struct ClaimRefused; + +impl crate::pci::ConfigSpace for Config { + type Refused = ClaimRefused; + + fn read8(&self, at: u16) -> Result { + if self.refused.contains(&at) { + return Err(ClaimRefused); + } + Ok(self.bytes[at as usize]) + } + + fn read32(&self, at: u16) -> Result { + assert_eq!(at % 4, 0, "a 32-bit read at {at:#x}, off its alignment"); + if self.refused.contains(&at) { + return Err(ClaimRefused); + } + let at = at as usize; + Ok(u32::from_le_bytes(self.bytes[at..at + 4].try_into().unwrap())) + } +} + +/// The vendor capabilities in the list's order, any other capability passed +/// over, and the multiplier read where §4.1.4.4 puts one and nowhere else. +#[test] +fn the_walk_answers_every_vendor_capability_in_the_lists_order() { + let mut config = Config::listing(&[0x98, 0x40, 0x70, 0x84]); + config.vendor(0x98, 1, 4, 0x0000, 0x1000, 0); + config.bytes[0x40] = 0x11; // MSI-X: not a vendor's. + config.vendor(0x70, 2, 4, 0x3000, 0x1000, 4); + config.vendor(0x84, 4, 4, 0x2000, 0x1000, 0xDEAD); + let caps = crate::pci::vendor_caps(&config).expect("a list that ends"); + assert_eq!( + caps, + [ + VendorCap { cfg_type: 1, bar: 4, offset: 0, length: 0x1000, notify_off_multiplier: 0 }, + VendorCap { cfg_type: 2, bar: 4, offset: 0x3000, length: 0x1000, notify_off_multiplier: 4 }, + VendorCap { cfg_type: 4, bar: 4, offset: 0x2000, length: 0x1000, notify_off_multiplier: 0 }, + ] + ); +} + +/// §6.2.3: with `Status` bit 4 clear the pointer means nothing, and nothing +/// behind it is read. +#[test] +fn a_function_without_a_capability_list_has_no_vendor_capability() { + let mut config = Config::listing(&[0x40]); + config.vendor(0x40, 1, 4, 0, 0x1000, 0); + config.bytes[0x06] = 0; + config.refused = (0x08..0x100).collect(); + assert_eq!(crate::pci::vendor_caps(&config), Ok(Vec::new())); +} + +/// A read the claim refused is that refusal, at that offset, and never a +/// field of zeros another check refuses under a name of its own. +#[test] +fn a_refused_read_ends_the_walk_by_the_claims_own_word() { + for at in [0x06, 0x34, 0x40, 0x41, 0x43, 0x44, 0x48, 0x4C, 0x50] { + let mut config = Config::listing(&[0x40]); + config.vendor(0x40, 2, 4, 0x3000, 0x1000, 4); + config.refused = Vec::from([at]); + assert_eq!( + crate::pci::vendor_caps(&config), + Err(WalkRefusal::Read { at, why: ClaimRefused }), + "a refusal at {at:#x}" + ); + } +} + +/// §6.7: "the bottom two bits are Reserved and must be set to 00b. Software +/// must mask these bits off before using this register as a pointer", of the +/// pointer and of every link. +#[test] +fn every_link_is_masked_of_its_two_reserved_bits_before_it_is_an_offset() { + let mut config = Config::listing(&[0x40, 0x60]); + config.bytes[0x34] = 0x43; + config.bytes[0x41] = 0x62; + config.vendor(0x40, 1, 4, 0, 0x1000, 0); + config.vendor(0x60, 4, 4, 0x2000, 0x1000, 0); + let caps = crate::pci::vendor_caps(&config).expect("a list that ends"); + assert_eq!(caps.iter().map(|cap| cap.cfg_type).collect::>(), [1, 4]); +} + +/// A link into the header and a list that comes back round are each refused +/// by name, and neither is walked for ever. +#[test] +fn a_link_into_the_header_or_round_the_list_is_refused() { + let mut config = Config::listing(&[0x40]); + config.bytes[0x41] = 0x3C; + assert_eq!(crate::pci::vendor_caps(&config), Err(WalkRefusal::IntoHeader { at: 0x3C })); + + let mut config = Config::listing(&[0x40, 0x50]); + config.bytes[0x51] = 0x40; + assert_eq!(crate::pci::vendor_caps(&config), Err(WalkRefusal::Looped)); + + // The longest list a header holds is no loop: a capability on every dword. + let every: Vec = (0x40..=0xFCu8).step_by(4).collect(); + assert_eq!(crate::pci::vendor_caps(&Config::listing(&every)), Ok(Vec::new())); +} + // --- §2.7: the queue --- /// §4.1.5.1.3 step 4 has the three parts zeroed, and §2.7.10.1 has the driver diff --git a/toyos/src/device.rs b/toyos/src/device.rs index 7c69e4f03be..c34783084a7 100644 --- a/toyos/src/device.rs +++ b/toyos/src/device.rs @@ -21,7 +21,6 @@ macro_rules! device_info { device_info!( toyos_abi::FramebufferInfo, toyos_abi::pci::PciFunctionInfo, - toyos_abi::virtio_sound::VirtioSoundInfo, toyos_abi::hda::HdaInfo, toyos_abi::part::PartitionInfo, toyos_abi::acpi::AcpiInfo, @@ -179,9 +178,9 @@ impl PciDev { /// A claim's interrupts since its last read, or `Err(WouldBlock)` for none. fn irq_record(dev: &Device) -> Result { - let mut record = toyos_abi::pci::DeviceIrqRecord { count: 0 }; + let mut record = toyos_abi::pci::DeviceIrqRecord { count: 0, _pad: 0, first_nanos: 0, last_nanos: 0 }; // SAFETY: the slice covers exactly the record being filled, and every - // bit pattern of its one integer field is a valid one. + // bit pattern of each of its integer fields is a valid one. let buf = unsafe { core::slice::from_raw_parts_mut(&mut record as *mut _ as *mut u8, toyos_abi::pci::DeviceIrqRecord::SIZE) }; @@ -277,51 +276,6 @@ impl AsHandle for PartitionDev { fn as_handle(&self) -> RawHandle { self.0.as_handle() } } -/// A virtio-sound device the kernel brought up and drives no policy on. -/// -/// What the claimant gets is the region the descriptors point into, mapped -/// writable, an interrupt it may wait on, and one register call per queue -/// doorbell. It gets no descriptor table and no physical address, so there is -/// nothing here that can point the device at memory. -pub struct VirtioSoundDev(pub(crate) Device); - -impl VirtioSoundDev { - pub fn info(&self) -> Result { - read_info(&self.0) - } - - /// Drain all pending completion records in one nonblocking read. - /// - /// Returns the number of records written to `records`. The kernel ring - /// holds at most 16 records, so a 16-entry buffer always drains fully. - /// Empty ring surfaces as `Err(WouldBlock)`. - pub fn read_completions( - &self, - records: &mut [toyos_abi::audio::AudioCompletionRecord], - ) -> Result { - const REC_SIZE: usize = toyos_abi::audio::AudioCompletionRecord::SIZE; - let buf = unsafe { - core::slice::from_raw_parts_mut( - records.as_mut_ptr() as *mut u8, - records.len() * REC_SIZE, - ) - }; - let n = syscall::read_nonblock(self.0.0.0, buf)?; - assert_eq!(n % REC_SIZE, 0, "partial audio completion record ({n} bytes)"); - Ok(n / REC_SIZE) - } - - /// Ring one queue's doorbell. `offset` is one of the three the info struct - /// reports and nothing else is on the kernel's allow-list. - pub fn notify(&self, offset: u32, queue: u16) -> Result<(), SyscallError> { - syscall::device_reg_write(self.0.as_handle(), offset, syscall::RegWidth::U16, queue as u32) - } -} - -impl AsHandle for VirtioSoundDev { - fn as_handle(&self) -> RawHandle { self.0.as_handle() } -} - /// An Intel HDA controller the kernel brought up and drives no policy on. /// /// What the claimant gets is a PCM ring it may write, an interrupt it may wait diff --git a/toyos/src/endow.rs b/toyos/src/endow.rs index d4d9b97df79..4927ba31de1 100644 --- a/toyos/src/endow.rs +++ b/toyos/src/endow.rs @@ -77,7 +77,6 @@ from_handle! { crate::AcpiDev => |h| crate::AcpiDev(Device(h)), crate::PartitionDev => |h| crate::PartitionDev(Device(h)), crate::HdaDev => |h| crate::HdaDev(Device(h)), - crate::VirtioSoundDev => |h| crate::VirtioSoundDev(Device(h)), } /// This process's endowment table, parsed once. @@ -224,9 +223,7 @@ pub fn provided(labels: &mut [Option<(&'static str, Connector)>]) -> usize { /// /// `None` is a machine that had no such device when the supervisor asked, or a program /// the manifest gives none — the honest answer, and the one soundserver degrades -/// on. It replaces a two-syscall probe: "did I get an HDA or a virtio-sound?" -/// is now "which claims are in my endowment table?", which is the same question -/// with the answer already in hand. +/// on. pub fn device(class: DeviceType) -> Option { with_prefixed(DEV_PREFIX, class.class_name(), |label| Endowments::get().take::(label)) } diff --git a/toyos/src/lib.rs b/toyos/src/lib.rs index 5b7e3e30012..89d6799526d 100644 --- a/toyos/src/lib.rs +++ b/toyos/src/lib.rs @@ -35,7 +35,7 @@ pub mod volatile; pub use ipc::Connection; pub use device::{ - AcpiDev, DmaRegion, FramebufferDev, HdaDev, Keyboard, Mouse, PartitionDev, PciDev, VirtioSoundDev, + AcpiDev, DmaRegion, FramebufferDev, HdaDev, Keyboard, Mouse, PartitionDev, PciDev, }; pub use toyos_abi::RawHandle; diff --git a/userland/netstack/Cargo.toml b/userland/netstack/Cargo.toml index 498ad8ba171..db88966465c 100644 --- a/userland/netstack/Cargo.toml +++ b/userland/netstack/Cargo.toml @@ -10,6 +10,7 @@ toyos = { path = "../../toyos" } toyos-device-memory = { path = "../../toyos-device-memory" } toyos-i219 = { path = "../../toyos-i219" } toyos-virtio = { path = "../../toyos-virtio" } +toyos-pci-claim = { path = "../toyos-pci-claim" } toyos-tco = { path = "../../toyos-tco" } toyos-mdns = { path = "mdns" } toyos-dns = { path = "../../toyos-dns" } @@ -22,5 +23,8 @@ toyos-net-tcp = { path = "../../toyos-net-shard/tcp" } toyos-net-udp = { path = "../../toyos-net-udp" } toyos-net-wire = { path = "../../toyos-net-wire" } +[dev-dependencies] +toyos-pci-claim = { path = "../toyos-pci-claim", features = ["host-grant"] } + [package.metadata.toyos.host] exempt.owns = "the NIC ToyOS claims for it" diff --git a/userland/netstack/src/device.rs b/userland/netstack/src/device.rs deleted file mode 100644 index 74fe0aa6092..00000000000 --- a/userland/netstack/src/device.rs +++ /dev/null @@ -1,163 +0,0 @@ -//! What both NIC drivers need from the substrate: `toyos-device-memory`'s -//! boundary over the SDK's `toyos::volatile::Window`, the kernel's word for a -//! call a bring-up cannot go on without, and the latch a diagnostic is printed -//! on. -//! -//! **[`Bar`] and [`Grant`] are the one implementation of the boundary both -//! driver crates are written against**, an instruction deep: every offset -//! that reaches them a driver crate has bounded, and the two barriers are here -//! and nowhere else in this program. Neither owns its mapping: a driver's -//! holder keeps the `SharedMemory` and the `DmaRegion` for as long as it keeps -//! the driver. - -use std::cell::Cell; -use std::sync::atomic::{fence, Ordering}; - -use toyos::volatile::Window; -use toyos_abi::syscall::SyscallError; -use toyos_device_memory::{DmaBuffers, Registers}; - -/// A mapped BAR, as a driver reaches its device's registers. -#[derive(Clone, Copy)] -pub struct Bar(Window); - -impl Bar { - pub fn over(window: Window) -> Self { - Self(window) - } -} - -impl Registers for Bar { - fn bytes(&self) -> usize { - self.0.bytes() - } - - fn read8(&self, at: usize) -> u8 { - self.0.read(at) - } - - fn read16(&self, at: usize) -> u16 { - self.0.read(at) - } - - fn read32(&self, at: usize) -> u32 { - self.0.read(at) - } - - fn write8(&self, at: usize, value: u8) { - self.0.write(at, value); - } - - fn write16(&self, at: usize, value: u16) { - self.0.write(at, value); - } - - fn write32(&self, at: usize, value: u32) { - self.0.write(at, value); - } -} - -/// A DMA grant, as a driver reaches its rings and descriptors. -#[derive(Clone, Copy)] -pub struct Grant { - window: Window, - /// Where the device reaches the grant's first byte. Not a physical address: - /// what the unit translates for this function and for nothing else. - device_base: u64, -} - -impl Grant { - pub fn over(window: Window, device_base: u64) -> Self { - Self { window, device_base } - } - - /// The same bytes, for the holder's own view of them: the frames, which no - /// driver crate reaches. - pub fn window(&self) -> Window { - self.window - } -} - -impl DmaBuffers for Grant { - fn bytes(&self) -> usize { - self.window.bytes() - } - - fn device_addr(&self, at: usize) -> u64 { - self.device_base + at as u64 - } - - fn read16(&self, at: usize) -> u16 { - self.window.read(at) - } - - fn read32(&self, at: usize) -> u32 { - self.window.read(at) - } - - fn read64(&self, at: usize) -> u64 { - self.window.read(at) - } - - fn write16(&self, at: usize, value: u16) { - self.window.write(at, value); - } - - fn write32(&self, at: usize, value: u32) { - self.window.write(at, value); - } - - fn write64(&self, at: usize, value: u64) { - self.window.write(at, value); - } - - fn publish(&self) { - fence(Ordering::Release); - } - - fn observe(&self) { - fence(Ordering::Acquire); - } -} - -/// The kernel refused a call a bring-up cannot go on without, and the word is -/// the kernel's own. -#[derive(Clone, Copy, PartialEq, Eq, Debug)] -pub struct KernelRefused { - pub call: &'static str, - pub why: SyscallError, -} - -impl KernelRefused { - pub fn on(call: &'static str) -> impl Fn(SyscallError) -> Self { - move |why| Self { call, why } - } -} - -impl std::fmt::Display for KernelRefused { - fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - write!(f, "the kernel refused {}: {:?}", self.call, self.why) - } -} - -/// What a diagnostic last said, so it says it again only on a change. -/// -/// **On change, not per element**: a device flooding a ring with descriptors a -/// driver will not act on costs one line, not one per descriptor, which is the -/// difference between a diagnostic and a way to drown the console from the -/// other side of the boundary. -pub struct Latch(Cell); - -impl Default for Latch { - fn default() -> Self { - Self(Cell::new(T::default())) - } -} - -impl Latch { - /// The previous value if `now` is not it, and `None` if nothing has moved. - pub fn moved(&self, now: T) -> Option { - let was = self.0.replace(now); - (was != now).then_some(was) - } -} diff --git a/userland/netstack/src/i219.rs b/userland/netstack/src/i219.rs index f1ebc450c62..a71fbc5a289 100644 --- a/userland/netstack/src/i219.rs +++ b/userland/netstack/src/i219.rs @@ -1,6 +1,6 @@ //! The I219, as netstack drives it: the clock and the claim the driver's logic -//! is written against beside `device.rs`'s register window and grant, and -//! nothing else. +//! is written against beside `toyos-pci-claim`'s register window and grant, +//! and nothing else. //! //! What the kernel keeps is the *claim*: config space, the interrupt vector it //! programmed into this function, and the address space the function @@ -15,7 +15,7 @@ //! descriptor, which is what lets the driver's logic live in a crate that //! forbids `unsafe` at all. -use std::cell::RefCell; +use std::cell::{Cell, RefCell}; use std::rc::Rc; use toyos::shm::SharedMemory; @@ -24,8 +24,7 @@ use toyos::{DmaRegion, PciDev}; use toyos_abi::syscall::SyscallError; use toyos_device_memory::Registers; use toyos_i219::{Clock, Interrupts}; - -use crate::device::{Bar, Grant, KernelRefused, Latch}; +use toyos_pci_claim::{Bar, Grant, KernelRefused}; /// The machine's monotonic clock: one syscall, which the kernel serves from an /// anchor plus the timestamp counter — and the thread's own sleep, which is how @@ -113,21 +112,14 @@ fn registers(dev: &PciDev) -> Result<(Bar, SharedMemory), Opening> { .find(|(_, bytes)| **bytes >= toyos_i219::regs::REGISTER_BYTES as u64) .map(|(index, bytes)| (index as u32, *bytes)) .ok_or(Opening::NoWindow)?; - let mapped = dev - .map_bar(bar, bytes) - .map_err(KernelRefused::on("the BAR")) - .map_err(Opening::Kernel)?; + let (window, mapped) = Bar::map(dev, bar, bytes).map_err(Opening::Kernel)?; crate::say!( "netstack: I219: PCI {:02x}:{:02x}.{} registers in BAR {bar} ({bytes:#x} bytes)", info.bus, info.dev, info.func, ); - // SAFETY: `map_bar` answered `bytes` bytes of live mapping, and `mapped` - // goes to the caller with the window, which keeps both for as long as it - // keeps either. - let window = unsafe { Window::new(mapped.as_ptr(), bytes as usize) }; - Ok((Bar::over(window), mapped)) + Ok((window, mapped)) } /// Everything the claim is asked for before the part is reached, in the order @@ -145,17 +137,8 @@ fn granted(dev: &PciDev) -> Result<(Bar, Grant, SharedMemory, DmaRegion), Openin bar.read32(toyos_i219::regs::TCTL) ); toyos_i219::quiesce(&bar); - let region = dev - .dma_alloc(toyos_i219::GRANT_BYTES) - .map_err(KernelRefused::on("a DMA grant")) - .map_err(Opening::Kernel)?; - let device_base = region.device_addr; - // SAFETY: `dma_alloc` answered `GRANT_BYTES` bytes of live mapping, and - // `region` goes to the caller with the grant, which keeps both for as - // long as it keeps either. - let window = - unsafe { Window::new(region.memory.as_ptr(), toyos_i219::GRANT_BYTES as usize) }; - Ok((bar, Grant::over(window, device_base), mapped, region)) + let (grant, region) = Grant::alloc(dev, toyos_i219::GRANT_BYTES).map_err(Opening::Kernel)?; + Ok((bar, grant, mapped, region)) } /// What a bring-up says about itself: a sentence each for what it asked the @@ -356,3 +339,25 @@ impl Nic { } } } + +/// What a diagnostic last said, so it says it again only on a change. +/// +/// **On change, not per element**: a device flooding a ring with descriptors a +/// driver will not act on costs one line, not one per descriptor, which is the +/// difference between a diagnostic and a way to drown the console from the +/// other side of the boundary. +struct Latch(Cell); + +impl Default for Latch { + fn default() -> Self { + Self(Cell::new(T::default())) + } +} + +impl Latch { + /// The previous value if `now` is not it, and `None` if nothing has moved. + fn moved(&self, now: T) -> Option { + let was = self.0.replace(now); + (was != now).then_some(was) + } +} diff --git a/userland/netstack/src/main.rs b/userland/netstack/src/main.rs index 568d89f0fae..014e6615b87 100644 --- a/userland/netstack/src/main.rs +++ b/userland/netstack/src/main.rs @@ -44,7 +44,6 @@ use toyos_net_wire::Instant; mod card; mod client; -mod device; mod i219; mod pipes; mod serve; diff --git a/userland/netstack/src/virtio_net.rs b/userland/netstack/src/virtio_net.rs index 752063ca830..b729671f927 100644 --- a/userland/netstack/src/virtio_net.rs +++ b/userland/netstack/src/virtio_net.rs @@ -9,7 +9,7 @@ //! //! **The transport and the rings are `toyos-virtio`'s**, where every word the //! device writes is bounded and every refusal is host-tested, over -//! `device.rs`'s register window and grant. What is this file's is the +//! `toyos-pci-claim`'s register window and grant. What is this file's is the //! network device (§5.1): which buffer goes with which head, the header in //! front of every frame, and what becomes of a refusal. //! @@ -24,30 +24,16 @@ use std::cell::RefCell; use toyos::shm::SharedMemory; -use toyos::volatile::Window; use toyos::{DmaRegion, PciDev}; -use toyos_abi::syscall::{RegWidth, SyscallError}; +use toyos_abi::syscall::SyscallError; use toyos_device_memory::DmaBuffers; -use toyos_virtio::pci::{Layout, Live, Offer, VendorCap}; +use toyos_pci_claim::virtio::{negotiate, Negotiated, Refusal}; +use toyos_pci_claim::{Bar, Grant}; +use toyos_virtio::pci::Live; use toyos_virtio::queue::{avail_bytes, desc_bytes, Buffer, Parts, Published, Used, Virtqueue}; -use crate::device::{Bar, Grant, KernelRefused}; - -/// PCI's own vendor-specific capability id; virtio's config structures are all -/// published under it (§4.1.4). -const CAP_ID_VENDOR: u8 = 0x09; - -/// Where the capability list starts, and how far a walk may follow it. The -/// pointer is the *device's*, so a malformed or cyclic chain ends the walk -/// rather than running for ever. -const CAPABILITIES_PTR: u32 = 0x34; -const MAX_CAPABILITIES: usize = 48; - /// §5.1.3: the device has a MAC address of its own to read. const VIRTIO_NET_F_MAC: u64 = 1 << 5; -/// §6, for the feature line: the bit `toyos-virtio` accepts wherever it is -/// offered, and a gate reads back. -const VIRTIO_F_ACCESS_PLATFORM: u64 = toyos_virtio::pci::VIRTIO_F_ACCESS_PLATFORM; /// The one MSI-X table entry the kernel programs. const MSIX_ENTRY: u16 = 0; @@ -91,38 +77,6 @@ const _: () = { assert!(NET_HDR_SIZE < RX_BUF_SIZE && NET_HDR_SIZE < TX_BUF_SIZE); }; -/// Why the device was not brought up. Each keeps its own word: a machine with -/// no such device and one that refused a feature set ask different things of a -/// caller. -#[derive(Clone, Copy, PartialEq, Eq, Debug)] -pub enum Refusal { - /// What the device published or answered, as the transport refused it. - Device(toyos_virtio::pci::Refusal), - Kernel(KernelRefused), - /// The claim answered a configuration read it had to refuse. - Unbounded(&'static str, u32), -} - -impl From for Refusal { - fn from(why: toyos_virtio::pci::Refusal) -> Self { - Self::Device(why) - } -} - -impl std::fmt::Display for Refusal { - fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - match self { - Self::Device(why) => write!(f, "{why}"), - Self::Kernel(why) => write!(f, "{why}"), - Self::Unbounded(what, at) => write!( - f, - "the claim answered a {what} at {at:#x}, so it is not a claim on one \ - function's own configuration space" - ), - } - } -} - /// The next chain the device has finished with on `rings`, or `None`. /// /// **Every way the used ring is not believed ends the device's use**: a head @@ -135,36 +89,6 @@ fn finished(rings: &mut Virtqueue) -> Option { .unwrap_or_else(|why| panic!("netstack: this NIC cannot be driven on — {why}")) } -/// The bound the capability walk rests on, asked once before the walk. -/// -/// **The walk below indexes configuration space by numbers the *device* wrote** -/// — the capability pointer and every `next` link in the chain — and it is safe -/// only because a claim answers its own function's 4 KiB and nothing else. That -/// is the kernel's contract, so this is where the driver that depends on it -/// checks it: a read past the end and one not aligned for its own width are -/// both refused, and the first byte is not. An aligned read that straddles the -/// end cannot be written — 4096 is a multiple of every width — and one whose -/// offset wraps cannot be expressed, `PciDev::config_read` taking a `u32`; both -/// are answered where the arithmetic lives, in `toyos-dma`'s host tests. -fn config_space_is_bounded(dev: &PciDev) -> Result<(), Refusal> { - const CONFIG_BYTES: u32 = 4096; - for (what, at, width) in [ - ("read past its configuration space", CONFIG_BYTES, RegWidth::U8), - ("misaligned read", 1, RegWidth::U16), - ] { - if dev.config_read(at, width).is_ok() { - return Err(Refusal::Unbounded(what, at)); - } - } - // And the bound is a bound rather than a wall: the vendor id is still there. - dev.config_read(0, RegWidth::U16).map_err(KernelRefused::on("its vendor id")).map_err(Refusal::Kernel)?; - crate::say!( - "netstack: this claim answers {CONFIG_BYTES} bytes of configuration space and refuses \ - every access outside them" - ); - Ok(()) -} - /// The virtio-net function, brought up and driving. pub struct VirtioNet { dev: PciDev, @@ -184,52 +108,11 @@ impl VirtioNet { /// feature negotiation, both queues and their vectors in the order /// `toyos-virtio`'s types fix, which is virtio 1.2 §3.1.1's. pub fn open(dev: PciDev) -> Result { - let info = dev.describe().map_err(KernelRefused::on("the claim's description")).map_err(Refusal::Kernel)?; - - config_space_is_bounded(&dev)?; - let layout = Layout::of(&vendor_caps(&dev))?; - // The kernel hands out a BAR at a time, and reports 0 bytes for one it - // keeps back. - let bar = layout.bar(); - let bar_bytes = *info - .bar_bytes - .get(bar as usize) - .filter(|bytes| **bytes > 0) - .ok_or(toyos_virtio::pci::Refusal::MissingCap("a BAR this claim may map"))?; - let mapped = dev - .map_bar(bar as u32, bar_bytes) - .map_err(KernelRefused::on("the register window")) - .map_err(Refusal::Kernel)?; - // SAFETY: the mapping is `bar_bytes` long and lives as long as - // `mapped`, which this struct holds for its own life. - let window = unsafe { Window::new(mapped.as_ptr(), bar_bytes as usize) }; - - let offer = Offer::acknowledge(Bar::over(window), &layout)?; - let offered = offer.features(); - let setup = offer.accept(VIRTIO_NET_F_MAC)?; - let features = setup.features(); - // The line the kernel's virtio drivers print, in the same shape: - // `iommu_virtio_platform` reads it back for every virtio function the - // machine creates. - crate::say!( - "netstack: VirtIO: PCI {:02x}:{:02x}.{} features device={offered:#x} \ - negotiated={features:#x} access_platform={}", - info.bus, - info.dev, - info.func, - if features & VIRTIO_F_ACCESS_PLATFORM != 0 { 'y' } else { 'n' }, - ); + let Negotiated { setup, mapping, features } = negotiate(&dev, VIRTIO_NET_F_MAC)?; + crate::say!("netstack: {features}"); - let region = dev - .dma_alloc(GRANT_BYTES) - .map_err(KernelRefused::on("a DMA grant")) - .map_err(Refusal::Kernel)?; - // SAFETY: the grant covers at least `GRANT_BYTES` — the kernel rounds - // the request up to whole pages, never down — and lives as long as - // `region`, which this struct holds. - let window = unsafe { Window::new(region.memory.as_ptr(), GRANT_BYTES as usize) }; - window.zero(); - let grant = Grant::over(window, region.device_addr); + let (grant, region) = Grant::alloc(&dev, GRANT_BYTES)?; + grant.window().zero(); let rx = Virtqueue::new(grant, RX_QUEUE, RX_QUEUE_SIZE, RX_PARTS); let tx = TxQueue::new(grant); @@ -248,7 +131,7 @@ impl VirtioNet { let nic = Self { dev, device: setup.driver_ok(), - _bar: mapped, + _bar: mapping, _region: region, grant, rx: RefCell::new(rx), @@ -417,37 +300,6 @@ impl TxQueue { } } -/// The vendor capabilities a function published, in its list's order, walked -/// once. -fn vendor_caps(dev: &PciDev) -> Vec { - let mut found = Vec::new(); - let mut seen = 0usize; - let Ok(first) = dev.config_read(CAPABILITIES_PTR, RegWidth::U8) else { - return found; - }; - let mut next = first; - // The pointer is the device's: a chain that does not terminate, or one - // pointing outside the header, ends the walk rather than running off - // the window or for ever. - while next >= 0x40 && next < 0x100 && seen < MAX_CAPABILITIES { - seen += 1; - let Ok(id) = dev.config_read(next, RegWidth::U8) else { break }; - if id as u8 == CAP_ID_VENDOR { - let read = |at: u32, width| dev.config_read(next + at, width).unwrap_or(0); - found.push(VendorCap { - cfg_type: read(3, RegWidth::U8) as u8, - bar: read(4, RegWidth::U8) as u8, - offset: read(8, RegWidth::U32), - length: read(12, RegWidth::U32), - notify_off_multiplier: read(16, RegWidth::U32), - }); - } - let Ok(link) = dev.config_read(next + 1, RegWidth::U8) else { break }; - next = link; - } - found -} - /// The transmit queue's room, over a plain allocation for the grant. What a /// used-ring element must satisfy is `toyos-virtio`'s, and tested there. #[cfg(test)] @@ -460,11 +312,7 @@ mod tests { /// A transmit queue over a plain allocation the size of the grant. fn queue() -> TxQueue { - let backing = vec![0u64; GRANT_BYTES as usize / 8].leak(); - // SAFETY: `leak` gives the allocation the `'static` lifetime the - // window needs, and nothing but this queue and the test reaches it. - let window = unsafe { Window::new(backing.as_mut_ptr().cast(), GRANT_BYTES as usize) }; - TxQueue::new(Grant::over(window, DEVICE_BASE)) + TxQueue::new(Grant::leaked(GRANT_BYTES as usize, DEVICE_BASE)) } /// The head in available-ring entry `nth` (§2.7.6). diff --git a/userland/soundserver/Cargo.toml b/userland/soundserver/Cargo.toml index 13fc21e9b8c..4926eac28ca 100644 --- a/userland/soundserver/Cargo.toml +++ b/userland/soundserver/Cargo.toml @@ -9,6 +9,9 @@ toyos-abi = { path = "../../toyos-abi" } toyos = { path = "../../toyos" } toyos-hda = { path = "../../toyos-hda" } toyos-mixer = { path = "mixer" } +toyos-virtio-sound = { path = "virtio-sound" } +toyos-virtio = { path = "../../toyos-virtio" } +toyos-pci-claim = { path = "../toyos-pci-claim" } toyos-inspect = { path = "../../toyos-inspect" } rubato = "0.16" diff --git a/userland/soundserver/src/backend.rs b/userland/soundserver/src/backend.rs index fe2a4f0ecc5..dab37a918e8 100644 --- a/userland/soundserver/src/backend.rs +++ b/userland/soundserver/src/backend.rs @@ -7,7 +7,6 @@ //! refilled* and nothing else in the loop can be written without knowing which. use toyos::AsHandle; -use toyos_abi::audio::AudioCompletionRecord; use toyos_abi::syscall; use toyos_abi::RawHandle; @@ -37,6 +36,17 @@ pub(crate) enum Pipeline { Ring, } +/// Periods the device has given back, and when it said so. +#[derive(Clone, Copy)] +pub(crate) struct Completion { + pub(crate) mask: u32, + /// When the device's first word about these periods landed: where a + /// wake's lateness is split between the device and soundserver. + pub(crate) first_nanos: u64, + /// When its newest landed, which the DLL takes as the last period's. + pub(crate) last_nanos: u64, +} + /// The device half of the mix loop. /// /// Two implementations and no more: a framework before there are three is an @@ -55,8 +65,8 @@ pub(crate) trait Backend { /// mapped once at claim. fn buffer(&self, idx: usize) -> *mut u8; - /// Completion records, oldest first, into `out`. `0` is nothing pending. - fn completions(&mut self, out: &mut [AudioCompletionRecord]) -> usize; + /// The periods given back since the last call, or `None` for none. + fn completion(&mut self) -> Option; /// Period `idx` has played and is soundserver's again. /// @@ -95,12 +105,8 @@ impl Backend for VirtioBackend { self.virtio.buffer(idx) } - fn completions(&mut self, out: &mut [AudioCompletionRecord]) -> usize { - // Where the kernel used to service the event queue inside the same - // syscall: the device's own view of an underrun, which this process's - // counters cannot see. - self.virtio.poll_events(); - self.virtio.completions(out) + fn completion(&mut self) -> Option { + self.virtio.completion() } fn released(&mut self, _idx: usize) {} @@ -133,13 +139,16 @@ impl Backend for HdaBackend { self.buffers[idx] } - fn completions(&mut self, out: &mut [AudioCompletionRecord]) -> usize { + /// The kernel's record has one time, the newest interrupt's, and it is + /// both of the completion's. + fn completion(&mut self) -> Option { match self.hda.dev().completions() { - Ok(record) => { - out[0] = record; - 1 - } - Err(syscall::SyscallError::WouldBlock) => 0, + Ok(record) => Some(Completion { + mask: record.mask, + first_nanos: record.timestamp_nanos, + last_nanos: record.timestamp_nanos, + }), + Err(syscall::SyscallError::WouldBlock) => None, Err(e) => panic!("soundserver: hda completions failed: {e:?}"), } } diff --git a/userland/soundserver/src/main.rs b/userland/soundserver/src/main.rs index d73ebebdbc3..e47bca86fb6 100644 --- a/userland/soundserver/src/main.rs +++ b/userland/soundserver/src/main.rs @@ -22,7 +22,7 @@ use toyos::endow; use toyos::port::Acceptor; use toyos::shm::SharedMemory; -use toyos::{HdaDev, VirtioSoundDev}; +use toyos::{HdaDev, PciDev}; use toyos_abi::syscall::{self, DeviceType}; use toyos_mixer::{period_frames, ramp_frames}; @@ -83,7 +83,7 @@ fn main() { // // The order is virtio first, and it is not a preference between two cards: // no machine in this project has both. The T14 has only the second. - if let Some(dev) = endow::device::(DeviceType::VirtioSound) { + if let Some(dev) = endow::pci_function::(virtio::PCI_ID) { match virtio::Virtio::claim(dev) { Ok((virtio, rate, channels)) => return run_virtio(acceptor, virtio, rate, channels), Err(why) => { @@ -109,10 +109,10 @@ fn run_virtio(acceptor: Acceptor, virtio: virtio::Virtio, rate: u32, channels: u acceptor, "virtio-sound", &mut VirtioBackend { virtio }, - toyos_abi::virtio_sound::PERIODS, + toyos_virtio_sound::PERIODS, rate, channels as u16, - toyos_abi::virtio_sound::PERIOD_BYTES, + toyos_virtio_sound::PERIOD_BYTES, ); } diff --git a/userland/soundserver/src/mix.rs b/userland/soundserver/src/mix.rs index ea8000eccdc..4f2b18362cb 100644 --- a/userland/soundserver/src/mix.rs +++ b/userland/soundserver/src/mix.rs @@ -14,7 +14,6 @@ use toyos::endow::Endowments; use toyos::poller::{Poller, READABLE}; use toyos::syscap::SysCap; -use toyos_abi::audio::AudioCompletionRecord; use toyos_abi::syscall; use toyos_abi::RawHandle; use toyos_hda::stream; @@ -238,7 +237,6 @@ pub(crate) fn mix_thread( let mut convert_buf = vec![0.0f32; max_client_frames * 2]; let mut dither_rng = Xorshift32::new(toyos_abi::clock::nanos_since_boot() as u32); let mut dll = Dll::new(period_nanos as f64); - let mut records = [AudioCompletionRecord { mask: 0, _pad: 0, timestamp_nanos: 0 }; 16]; const TOKEN_AUDIO: u64 = u64::MAX - 1; const TOKEN_CMD: u64 = u64::MAX - 2; @@ -315,54 +313,50 @@ pub(crate) fn mix_thread( idle_wakes = 0; } - let n_records = backend.completions(&mut records); - if n_records > 0 { - // Read before the record loop, because it is what a *pickup* is - // measured to: the instant soundserver first held the record, not the - // instant it finished acting on a batch of them. + let completed = backend.completion(); + if let Some(rec) = completed { + // Read before the record is acted on, because it is what a + // *pickup* is measured to: the instant soundserver first held the + // record, not the instant it finished acting on it. let seen_at = toyos_abi::clock::nanos_since_boot(); - let mut wake_completions = 0u32; - for rec in &records[..n_records] { - let n = rec.mask.count_ones(); - assert!(n > 0, "soundserver: completion record with empty mask"); - assert_eq!(free_mask & rec.mask, 0, "soundserver: repeated completion for free buffer"); - // Where the engine is, taken from what it reported rather than - // predicted: a mask a driver reads late is the OR of every - // `completed` since it last looked, so it can name a whole lap - // — which places the engine nowhere — and a cursor soundserver - // stepped itself would have to be right about how many laps - // that was. Re-deriving it per record cannot drift. - if pipeline == Pipeline::Ring { - match stream::decode(rec.mask, num_buffers) { - Some(stream::Completed::Run { first, count }) => { - ring_cursor = (first + count) % num_buffers; - } - // Every period played and the mask says no more than - // that. The cursor stays where it was: the fill order - // from here is a guess either way, and a lap of silence - // has already gone out — it counts as the drain it - // is, and the next record re-anchors. - Some(stream::Completed::Lapped) => {} - None => panic!( - "soundserver: the engine completed {:#x}, which is no walk of a \ - {num_buffers}-period ring", - rec.mask - ), + let n = rec.mask.count_ones(); + assert!(n > 0, "soundserver: completion record with empty mask"); + assert_eq!(free_mask & rec.mask, 0, "soundserver: repeated completion for free buffer"); + // Where the engine is, taken from what it reported rather than + // predicted: a mask a driver reads late is the OR of every + // `completed` since it last looked, so it can name a whole lap + // — which places the engine nowhere — and a cursor soundserver + // stepped itself would have to be right about how many laps + // that was. Re-deriving it per record cannot drift. + if pipeline == Pipeline::Ring { + match stream::decode(rec.mask, num_buffers) { + Some(stream::Completed::Run { first, count }) => { + ring_cursor = (first + count) % num_buffers; } + // Every period played and the mask says no more than + // that. The cursor stays where it was: the fill order + // from here is a guess either way, and a lap of silence + // has already gone out — it counts as the drain it + // is, and the next record re-anchors. + Some(stream::Completed::Lapped) => {} + None => panic!( + "soundserver: the engine completed {:#x}, which is no walk of a \ + {num_buffers}-period ring", + rec.mask + ), } - unplayed = unplayed.saturating_sub(n as usize); - free_mask |= rec.mask; - // Zero-on-complete, before anything can decide to leave - // this buffer unfilled: the engine returns to it in - // `num_buffers` periods whatever soundserver does. - for idx in 0..num_buffers { - if rec.mask & (1 << idx) != 0 { - backend.released(idx); - } + } + unplayed = unplayed.saturating_sub(n as usize); + free_mask |= rec.mask; + // Zero-on-complete, before anything can decide to leave + // this buffer unfilled: the engine returns to it in + // `num_buffers` periods whatever soundserver does. + for idx in 0..num_buffers { + if rec.mask & (1 << idx) != 0 { + backend.released(idx); } - wake_completions += n; - dll.update(rec.timestamp_nanos as f64, n); } + dll.update(rec.last_nanos as f64, n); // Measured against the prediction this wait was *armed* on, not // against whatever the DLL holds when the wait returns. They differ // on a window's first wake, armed while soundserver was still idle and @@ -371,9 +365,10 @@ pub(crate) fn mix_thread( // whenever soundserver armed a timer the distance from that prediction // is the sample, however large. // - // **And it is recorded in two halves**, split at the oldest - // record's ISR timestamp: everything before it is the device - // failing to complete when it was due, everything after it is + // **And it is recorded in two halves**, split at when the + // device's first message about these periods landed: everything + // before it is the device failing to complete when it was due, + // everything after it is // soundserver failing to run once it had. `WorstWake` is where that // distinction is argued; here it costs one subtraction, because // both instants were already in hand. @@ -384,18 +379,18 @@ pub(crate) fn mix_thread( // landed *before* it was due was not late by any amount, and a // soundserver that is nonetheless late here slept through it, so all // of the overshoot is the pickup. - let irq_at = records[0].timestamp_nanos.max(t_est); + let irq_at = rec.first_nanos.max(t_est); stats.wake( seen_at.saturating_sub(t_est), irq_at.saturating_sub(t_est), seen_at.saturating_sub(irq_at), - wake_completions, + n, period_nanos, ); } if !streams.is_empty() { - stats.completions += wake_completions; - stats.max_batch = stats.max_batch.max(wake_completions); + stats.completions += n; + stats.max_batch = stats.max_batch.max(n); } } else if armed_on.is_some() { // Armed on a grid point, woken, and the device had produced @@ -601,6 +596,7 @@ pub(crate) fn mix_thread( } if wake_left_idle(was_streaming, started_at_wake, !streams.is_empty(), cmd_ready) { + let n_records = usize::from(completed.is_some()); idle_wakes = idle_wakes.saturating_add(1); if idle_wakes < IDLE_WAKES_SAID { say!("soundserver: idle wake {idle_wakes} ({n_records} records)"); diff --git a/userland/soundserver/src/virtio.rs b/userland/soundserver/src/virtio.rs index 21348a03513..1bd40d6def2 100644 --- a/userland/soundserver/src/virtio.rs +++ b/userland/soundserver/src/virtio.rs @@ -1,523 +1,177 @@ -//! soundserver as the driver of a virtio-sound device. +//! soundserver as the driver of a virtio-sound function it holds as a PCI +//! claim. //! -//! The kernel negotiated the device's features, built its virtqueues and owns -//! their descriptors, and answers one register write per queue doorbell. -//! Everything that is a *decision* is here — which stream, at what rate and -//! format, when a period goes out and when the stream runs — and every one of -//! them is a message this process writes into a buffer of its own and publishes -//! by index. +//! What the kernel keeps is the claim: config space, the vector it programmed +//! into the function's MSI-X table, and the address space the function +//! translates through. **The transport and the queues are `toyos-virtio`'s, +//! and the sound device's messages and queues `toyos-virtio-sound`'s**, where +//! every word the device writes back is bounded and every refusal is +//! host-tested; the register window, the grant and the bring-up up to +//! `FEATURES_OK` are `toyos-pci-claim`'s. This file is what is left: the +//! sound device's vectors, the times its periods came back at, and what +//! becomes of a refusal. //! -//! There is no descriptor here and no physical address. The chains were built -//! once, at bind, out of offsets into the region below; what this file writes -//! into an avail ring is which of them to run. +//! **Only the transmit queue raises an interrupt.** The control queue is polled +//! for the one answer it owes, and the event queue is read when a period comes +//! back, so the claim is readable exactly when a period has played — and the +//! kernel's record of that message says when it landed, which is the time the +//! DLL is clocked by. //! -//! Structure layouts and command codes are VirtIO 1.2 §5.14. - -use core::ptr::{read_volatile, write_volatile}; -use core::sync::atomic::{fence, Ordering}; +//! **A refusal after bring-up ends soundserver** by its own name, as netstack's +//! does: no conforming device writes one. use toyos::shm::SharedMemory; -use toyos::VirtioSoundDev; -use toyos_abi::audio::AudioCompletionRecord; +use toyos::{DmaRegion, PciDev}; use toyos_abi::syscall::SyscallError; -use toyos_abi::virtio_sound as abi; - -const R_PCM_INFO: u32 = 0x0100; -const R_PCM_SET_PARAMS: u32 = 0x0101; -const R_PCM_PREPARE: u32 = 0x0102; -const R_PCM_START: u32 = 0x0104; -const R_PCM_STOP: u32 = 0x0105; - -const EVT_JACK_CONNECTED: u32 = 0x1000; -const EVT_JACK_DISCONNECTED: u32 = 0x1001; -const EVT_PCM_PERIOD_ELAPSED: u32 = 0x1100; -const EVT_PCM_XRUN: u32 = 0x1101; - -const S_OK: u32 = 0x8000; - -const FMT_S16: u8 = 5; -const RATE_44100: u8 = 6; -const RATE_48000: u8 = 7; - -/// The one stream this driver opens. A device with several is a decision this -/// file has not been asked to make, and taking the first of them is the blind -/// choice forbidden one layer down — but virtio-sound numbers its streams and reports -/// only how many, so there is nothing here to choose *by*. -const STREAM_ID: u32 = 0; - -/// The rates this driver can encode, best first. 44100 leads because it is what -/// the mixer, the resampler and the gate's recorded counters are sized against; -/// 48000 is the one every other device offers. -const SUPPORTED_RATES: [(u32, u8); 2] = [(44100, RATE_44100), (48000, RATE_48000)]; - -/// How many times a control command's completion is polled before the device is -/// called dead. -/// -/// **A count and not a deadline, and the difference is the whole of it.** A wall -/// clock keeps running while this guest is not: under host load a vCPU can lose -/// tens of milliseconds without executing an instruction, so a duration here -/// would measure the host's scheduler and report a healthy device as a stopped -/// one — at a suspend boundary, in the audio path. A count advances only when -/// this driver actually looked. -/// -/// Policy, not physics: the specification has no number. What it is set against -/// is that a device answering at all answers in one round trip, so anything this -/// side of enormous separates "slow" from "gone". -const CTRL_POLLS: u32 = 100_000; -/// Spins between two looks at the used ring. -const SPINS_PER_POLL: u32 = 256; - -#[repr(C)] -#[derive(Clone, Copy)] -struct Hdr { - code: u32, -} - -#[repr(C)] -#[derive(Clone, Copy)] -struct QueryInfo { - hdr: Hdr, - start_id: u32, - count: u32, - size: u32, -} +use toyos_pci_claim::virtio::{negotiate, Negotiated}; +use toyos_pci_claim::{Bar, Grant, KernelRefused}; +use toyos_virtio::pci::{Live, NO_VECTOR}; +use toyos_virtio_sound::{Queues, Sound, PERIOD_BYTES, STREAM_ID}; -#[repr(C)] -#[derive(Clone, Copy)] -struct PcmInfo { - /// The info header the device leads every entry with: its HDA function node. - hdr: u32, - features: u32, - formats: u64, - rates: u64, - direction: u8, - channels_min: u8, - channels_max: u8, - _padding: [u8; 5], -} +use crate::backend::Completion; -#[repr(C)] -#[derive(Clone, Copy)] -struct PcmSetParams { - hdr: Hdr, - stream_id: u32, - buffer_bytes: u32, - period_bytes: u32, - features: u32, - channels: u8, - format: u8, - rate: u8, - _padding: u8, -} +/// virtio's vendor id and the sound device's (§4.1.2.1: `0x1040` + 25), as +/// the manifest row spells the claim. +pub const PCI_ID: toyos_abi::syscall::PciId = toyos_abi::syscall::PciId { vendor: 0x1af4, device: 0x1059 }; -#[repr(C)] -#[derive(Clone, Copy)] -struct PcmHdr { - hdr: Hdr, - stream_id: u32, -} +/// The one MSI-X table entry the kernel programs. +const MSIX_ENTRY: u16 = 0; -#[repr(C)] -#[derive(Clone, Copy)] -struct PcmXfer { - stream_id: u32, -} +/// §5.14.4: `streams`, the second `le32` of the configuration. +const CONFIG_STREAMS: usize = 4; -#[repr(C)] -#[derive(Clone, Copy)] -struct Event { - code: u32, - data: u32, -} - -/// The control buffers the kernel described have to hold every message this file -/// sends or reads back, and the descriptor lengths are fixed at bind — so this -/// is the one place the two halves could disagree about a size. -const _: () = { - assert!(core::mem::size_of::() <= abi::CTRL_BUF_BYTES); - assert!(core::mem::size_of::() <= abi::CTRL_BUF_BYTES); - assert!(core::mem::size_of::() <= abi::CTRL_BUF_BYTES); - assert!( - core::mem::size_of::() + core::mem::size_of::() <= abi::CTRL_BUF_BYTES - ); -}; - -// Avail: flags(u16) idx(u16) ring[size](u16). Used: flags(u16) idx(u16) -// ring[size](id:u32 len:u32). -const AVAIL_IDX: usize = 2; -const AVAIL_RING: usize = 4; -const USED_IDX: usize = 2; -const USED_RING: usize = 4; -const USED_ELEM: usize = 8; - -/// The driver's half of a virtqueue: an index to publish, and a doorbell to ring -/// after publishing it. -/// -/// No descriptor table, which is the whole point — the chains are the kernel's -/// and this names one by its head. -struct Avail { - ring: *mut u8, - size: u16, - doorbell: u32, - queue: u16, +/// Why this machine's virtio-sound function cannot carry audio. Each is a line +/// soundserver prints before it falls back to the null sink. +pub enum Refusal { + Claim(toyos_pci_claim::virtio::Refusal), + Sound(toyos_virtio_sound::Refusal), } -impl Avail { - /// Make `head`'s chain available to the device and ring the doorbell. - fn publish(&self, dev: &VirtioSoundDev, head: u16) { - unsafe { - let idx_ptr = self.ring.add(AVAIL_IDX) as *mut u16; - let idx = read_volatile(idx_ptr); - write_volatile( - self.ring.add(AVAIL_RING + (idx % self.size) as usize * 2) as *mut u16, - head, - ); - // The chain has to be in the ring before the index covers it. - fence(Ordering::Release); - write_volatile(idx_ptr, idx.wrapping_add(1)); - fence(Ordering::Release); +impl core::fmt::Display for Refusal { + fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result { + match self { + Self::Claim(why) => write!(f, "{why}"), + Self::Sound(why) => write!(f, "{why}"), } - dev.notify(self.doorbell, self.queue).unwrap_or_else(|e| { - panic!("soundserver: virtio-sound refused queue {}'s doorbell: {e}", self.queue) - }); } } -/// A used ring this process consumes. -/// -/// The TX queue has no such thing here and that is the design: its consumer is -/// the interrupt handler, which timestamps the completion, and a ring this -/// process could rewrite would be a mask the kernel derived from userland. -struct Used { - ring: *mut u8, - size: u16, - last: u16, -} - -impl Used { - fn poll(&mut self) -> Option { - unsafe { - let used_idx = read_volatile(self.ring.add(USED_IDX) as *const u16); - if used_idx == self.last { - return None; - } - // The device wrote the element before bumping its index. - fence(Ordering::Acquire); - let slot = (self.last % self.size) as usize; - let id = read_volatile(self.ring.add(USED_RING + slot * USED_ELEM) as *const u32); - self.last = self.last.wrapping_add(1); - Some(id as u16) - } +impl From for Refusal { + fn from(why: toyos_pci_claim::virtio::Refusal) -> Self { + Self::Claim(why) } } -/// Why this machine's virtio-sound device cannot carry audio. Each is a line -/// soundserver prints before falling back to the null sink — "no sound" without which -/// of these it was is a report nobody can act on. -pub enum Refusal { - /// The kernel's answers stopped making sense, which is a bug here or there - /// and never a property of the machine. - Kernel(SyscallError), - /// The device did not answer a control command inside its deadline. - Silent(&'static str), - /// It answered, and said no. - Rejected(&'static str, u32), - /// It offers nothing this driver implements. Already named on its own line, - /// with the bitmap it was read from. - NoFormat, +impl From for Refusal { + fn from(why: toyos_virtio::pci::Refusal) -> Self { + Self::Claim(why.into()) + } } -impl core::fmt::Display for Refusal { - fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result { - match self { - Self::Kernel(e) => write!(f, "the kernel refused a call this driver has to make ({e})"), - Self::Silent(what) => write!(f, "the device never answered {what}"), - Self::Rejected(what, status) => write!(f, "the device refused {what} ({status:#x})"), - Self::NoFormat => write!(f, "the device offers no format or rate this driver mixes"), - } +impl From for Refusal { + fn from(why: KernelRefused) -> Self { + Self::Claim(why.into()) } } pub struct Virtio { - dev: VirtioSoundDev, - /// The region every ring and buffer below points into. Held so the mapping - /// outlives the pointers taken out of it. - _shm: SharedMemory, - base: *mut u8, - control: Avail, - control_done: Used, - events: Avail, - events_done: Used, - tx: Avail, - running: bool, + dev: PciDev, + sound: Sound>, + grant: Grant, + /// Held for their mappings' lives: the register window points into the + /// first and the grant into the second. + _bar: SharedMemory, + _region: DmaRegion, } impl Virtio { - /// Ask what the device's stream can do, and configure it. - /// - /// The claim is the argument: `/system/bin/supervisor` minted it and endowed it, so - /// "does this machine have a virtio-sound?" was already answered before - /// soundserver's first instruction. - pub fn claim(dev: VirtioSoundDev) -> Result<(Self, u32, u8), Refusal> { - let info = dev.info().map_err(Refusal::Kernel)?; - let shm = SharedMemory::adopt(info.dma, 2 * 1024 * 1024) - .map_err(Refusal::Kernel)?; - let base = shm.as_ptr(); - - let avail = |offset: usize, size: u16, doorbell: u32, queue: u16| Avail { - ring: unsafe { base.add(offset) }, - size, - doorbell, - queue, - }; - let used = |offset: usize, size: u16| Used { - ring: unsafe { base.add(offset) }, - size, - last: 0, - }; - let mut virtio = Virtio { - dev, - _shm: shm, - base, - control: avail( - abi::OFF_CTRL_AVAIL, - abi::CONTROL_QUEUE_SIZE, - info.notify_control, - abi::CONTROL_QUEUE, - ), - control_done: used(abi::OFF_CTRL_USED, abi::CONTROL_QUEUE_SIZE), - events: avail( - abi::OFF_EVENT_AVAIL, - abi::EVENT_QUEUE_SIZE, - info.notify_event, - abi::EVENT_QUEUE, - ), - events_done: used(abi::OFF_EVENT_USED, abi::EVENT_QUEUE_SIZE), - tx: avail(abi::OFF_TX_AVAIL, abi::TX_QUEUE_SIZE, info.notify_tx, abi::TX_QUEUE), - running: false, - }; - - for i in 0..abi::EVENT_BUFS { - virtio.events.publish(&virtio.dev, i as u16); - } - - let pcm = virtio.pcm_info()?; + /// Bring the function up in the order `toyos-virtio`'s types fix, which is + /// §3.1.1's, and stream 0 in §5.14.5's: the rate and channel count chosen. + pub fn claim(dev: PciDev) -> Result<(Self, u32, u8), Refusal> { + // §5.14.3: the sound device defines no feature bit. + let Negotiated { setup, mapping, features } = negotiate(&dev, 0)?; + say!("soundserver: {features}"); + + let (grant, region) = Grant::alloc(&dev, toyos_virtio_sound::GRANT_BYTES as u64)?; + grant.window().zero(); + + let queues = Queues::new(grant); + let (setup, streams) = setup + .config_vector(NO_VECTOR)? + .enable(&queues.control, NO_VECTOR)? + .enable(&queues.events, NO_VECTOR)? + .enable(&queues.tx, MSIX_ENTRY)? + .device_read32(CONFIG_STREAMS)?; + say!("virtio-sound: {streams} stream(s)"); + let (sound, stream) = + Sound::open(queues, grant, setup.driver_ok(), streams).map_err(Refusal::Sound)?; + let pcm = stream.info; say!( "virtio-sound: stream {STREAM_ID}: dir={} ch={}-{} fmts={:#x} rates={:#x}", pcm.direction, pcm.channels_min, pcm.channels_max, pcm.formats, pcm.rates ); - let (rate, channels) = choose_params(&pcm).ok_or(Refusal::NoFormat)?; - virtio.configure(rate, channels)?; - Ok((virtio, rate, channels)) + say!( + "virtio-sound: configured stream {STREAM_ID}: {}Hz {}ch s16le", + stream.rate, stream.channels + ); + let virtio = Self { dev, sound, grant, _bar: mapping, _region: region }; + Ok((virtio, stream.rate, stream.channels)) } - pub fn dev(&self) -> &VirtioSoundDev { + pub fn dev(&self) -> &PciDev { &self.dev } pub fn buffer(&self, idx: usize) -> *mut u8 { - unsafe { self.base.add(abi::OFF_PCM + idx * abi::PERIOD_BYTES) } + self.grant.window().sub(Sound::>::period(idx), PERIOD_BYTES).as_ptr() } - pub fn completions(&self, out: &mut [AudioCompletionRecord]) -> usize { - match self.dev.read_completions(out) { - Ok(n) => n, - Err(SyscallError::WouldBlock) => 0, - Err(e) => panic!("soundserver: read_completions failed: {e:?}"), - } - } - - /// Put period `idx` on the wire. + /// The periods played since the last call, and when the device said so. /// - /// The chain is already built and its PCM descriptor already names this - /// buffer, so what a period costs is a store of the stream id, a store into - /// the avail ring, and one doorbell — the same one syscall the deleted - /// `SYS_AUDIO_SUBMIT` cost. + /// The claim's record is taken before the ring is read, and the ring is + /// read only with one in hand ([`Sound::played`]). + pub fn completion(&mut self) -> Option { + let record = match self.dev.irq() { + Ok(record) => Some(record), + Err(SyscallError::WouldBlock) => None, + Err(why) => panic!("soundserver: the virtio-sound claim's interrupt record: {why:?}"), + }; + // The device's own view of an underrun, which soundserver's counters + // cannot see. + self.sound + .events(|event| match event.name() { + Some(name) => { + say!("virtio-sound: device event {:#x} ({name}) data={}", event.code, event.data) + } + None => say!("virtio-sound: device event {:#x} data={}", event.code, event.data), + }) + .unwrap_or_else(|why| panic!("soundserver: virtio-sound cannot be driven on — {why}")); + let (mask, record) = self + .sound + .played(record) + .unwrap_or_else(|why| panic!("soundserver: virtio-sound cannot be driven on — {why}"))?; + Some(Completion { mask, first_nanos: record.first_nanos, last_nanos: record.last_nanos }) + } + + /// Put period `idx` on the wire: its PCM descriptor is a whole period. pub fn submit(&mut self, idx: usize, bytes: usize) { - assert_eq!( - bytes, - abi::PERIOD_BYTES, - "soundserver: the TX chain's PCM descriptor is a whole period and the kernel built it" - ); - self.start(); - unsafe { - write_volatile( - self.base.add(abi::OFF_TX_XFER + idx * abi::XFER_STRIDE) as *mut PcmXfer, - PcmXfer { stream_id: STREAM_ID }, - ); + assert_eq!(bytes, PERIOD_BYTES, "soundserver: a virtio-sound period is {PERIOD_BYTES} bytes"); + let starting = !self.sound.running(); + self.sound + .submit(idx) + .unwrap_or_else(|why| panic!("soundserver: virtio-sound could not start its stream: {why}")); + if starting { + say!("virtio-sound: stream {STREAM_ID} started"); } - self.tx.publish(&self.dev, abi::tx_chain_head(idx)); - } - - fn start(&mut self) { - if self.running { - return; - } - self.simple_ctrl(R_PCM_START, "START") - .unwrap_or_else(|e| panic!("soundserver: virtio-sound could not start its stream: {e}")); - self.running = true; - say!("virtio-sound: stream {STREAM_ID} started"); } pub fn stop(&mut self) { - if !self.running { + if !self.sound.running() { return; } - self.simple_ctrl(R_PCM_STOP, "STOP") - .unwrap_or_else(|e| panic!("soundserver: virtio-sound could not stop its stream: {e}")); - self.running = false; + self.sound + .stop() + .unwrap_or_else(|why| panic!("soundserver: virtio-sound could not stop its stream: {why}")); say!("virtio-sound: stream {STREAM_ID} stopped"); } - - /// Report what the device says went wrong, and repost the buffer it said it - /// in. The device's own view of an underrun, which soundserver's counters cannot - /// see: they measure what this process failed to submit. - pub fn poll_events(&mut self) { - while let Some(id) = self.events_done.poll() { - let idx = id as usize; - if idx >= abi::EVENT_BUFS { - say!("soundserver: virtio-sound returned event buffer {idx}, which it never had"); - continue; - } - let event = unsafe { - read_volatile( - self.base.add(abi::OFF_EVENT_BUFS + idx * abi::EVENT_BUF_STRIDE) - as *const Event, - ) - }; - let name = match event.code { - EVT_JACK_CONNECTED => " (jack connected)", - EVT_JACK_DISCONNECTED => " (jack disconnected)", - EVT_PCM_PERIOD_ELAPSED => " (period elapsed)", - EVT_PCM_XRUN => " (PCM XRUN)", - _ => "", - }; - say!( - "virtio-sound: device event {:#x}{name} data={}", - event.code, event.data - ); - self.events.publish(&self.dev, id); - } - } - - fn pcm_info(&mut self) -> Result { - let query = QueryInfo { - hdr: Hdr { code: R_PCM_INFO }, - start_id: STREAM_ID, - count: 1, - size: core::mem::size_of::() as u32, - }; - self.ctrl(&query, "PCM_INFO")?; - Ok(unsafe { - core::ptr::read_unaligned( - self.base.add(abi::OFF_CTRL_RESP + core::mem::size_of::()) as *const PcmInfo, - ) - }) - } - - fn configure(&mut self, rate: u32, channels: u8) -> Result<(), Refusal> { - // `expect`, not a fallback: `choose_params` has already checked this rate - // against the device's own bitmap, so an unencodable one here means the - // two disagree — a driver bug, not a device we cannot drive. - let code = rate_code(rate).expect("soundserver: a rate chosen from the device's own bitmap"); - let params = PcmSetParams { - hdr: Hdr { code: R_PCM_SET_PARAMS }, - stream_id: STREAM_ID, - buffer_bytes: (abi::PERIOD_BYTES * abi::PERIODS) as u32, - period_bytes: abi::PERIOD_BYTES as u32, - features: 0, - channels, - format: FMT_S16, - rate: code, - _padding: 0, - }; - self.ctrl(¶ms, "SET_PARAMS")?; - self.simple_ctrl(R_PCM_PREPARE, "PREPARE")?; - say!("virtio-sound: configured stream {STREAM_ID}: {rate}Hz {channels}ch s16le"); - Ok(()) - } - - fn simple_ctrl(&mut self, code: u32, what: &'static str) -> Result<(), Refusal> { - self.ctrl(&PcmHdr { hdr: Hdr { code }, stream_id: STREAM_ID }, what) - } - - /// One control command: copy the request into the buffer the kernel's chain - /// describes, publish that chain, wait for it back, and read the status. - fn ctrl(&mut self, req: &T, what: &'static str) -> Result<(), Refusal> { - unsafe { - core::ptr::copy_nonoverlapping( - req as *const T as *const u8, - self.base.add(abi::OFF_CTRL_REQ), - core::mem::size_of::(), - ); - } - self.control.publish(&self.dev, 0); - - let mut answered = false; - for _ in 0..CTRL_POLLS { - if self.control_done.poll().is_some() { - answered = true; - break; - } - for _ in 0..SPINS_PER_POLL { - core::hint::spin_loop(); - } - } - if !answered { - return Err(Refusal::Silent(what)); - } - - let status = unsafe { read_volatile(self.base.add(abi::OFF_CTRL_RESP) as *const u32) }; - if status != S_OK { - return Err(Refusal::Rejected(what, status)); - } - Ok(()) - } -} - -fn rate_code(hz: u32) -> Option { - SUPPORTED_RATES.iter().find(|(rate, _)| *rate == hz).map(|(_, code)| *code) -} - -/// Pick a rate and channel count the device actually advertises. -/// -/// `None` means it offers nothing this driver implements — audio is optional, so -/// soundserver presents the null sink rather than dying over a peripheral, but the -/// log has to name the missing capability or the next person is decoding a -/// bitmap by hand on a laptop with no serial. -fn choose_params(info: &PcmInfo) -> Option<(u32, u8)> { - if info.formats & (1 << FMT_S16) == 0 { - say!( - "virtio-sound: no usable format — device offers {:#x}, driver needs S16 (bit {FMT_S16})", - info.formats - ); - return None; - } - - let Some(&(rate, _)) = SUPPORTED_RATES.iter().find(|(_, code)| info.rates & (1 << code) != 0) - else { - say!( - "virtio-sound: no usable rate — device offers {:#x}, driver needs 44100 (bit \ - {RATE_44100}) or 48000 (bit {RATE_48000})", - info.rates - ); - return None; - }; - - // Stereo if the device takes it; soundserver converts either way, so the only - // unusable case is a device whose minimum is more channels than we mix. - if info.channels_min > 2 { - say!( - "virtio-sound: no usable channel count — device needs at least {}, driver mixes at \ - most 2", - info.channels_min - ); - return None; - } - let channels = if info.channels_max >= 2 { 2 } else { info.channels_max }; - if channels == 0 { - say!("virtio-sound: device advertises a maximum of zero channels"); - return None; - } - Some((rate, channels)) } diff --git a/userland/soundserver/virtio-sound/Cargo.toml b/userland/soundserver/virtio-sound/Cargo.toml new file mode 100644 index 00000000000..bb8bed344da --- /dev/null +++ b/userland/soundserver/virtio-sound/Cargo.toml @@ -0,0 +1,24 @@ +# A member of the host workspace (root `Cargo.toml`), like `toyos-mixer` beside +# it: pure, and its tests run on the host against a model of the device that +# answers on the doorbell, with the specification's tables as the oracle. + +[package] +name = "toyos-virtio-sound" +description = "soundserver's virtio-sound driver: the sound device's messages (VirtIO 1.2 §5.14) and its three queues over one grant, pure." +version = "0.1.0" +edition = "2021" +license = "MIT OR Apache-2.0" +publish = false + +[lib] +doctest = false + +[dependencies] +# The transport's `Live`, which a doorbell is rung through, and the split +# virtqueue every word the device writes back is bounded by. +toyos-virtio = { path = "../../../toyos-virtio" } +# `DmaBuffers`, the grant the three queues and every buffer are laid out in. +toyos-device-memory = { path = "../../../toyos-device-memory" } + +[lints.rust] +warnings = "deny" diff --git a/userland/soundserver/virtio-sound/src/lib.rs b/userland/soundserver/virtio-sound/src/lib.rs new file mode 100644 index 00000000000..e4a5dec9a6e --- /dev/null +++ b/userland/soundserver/virtio-sound/src/lib.rs @@ -0,0 +1,450 @@ +//! soundserver's virtio-sound driver: one output stream through the device's +//! three queues, every decision about them, and none of the instructions that +//! carry them out. +//! +//! Every `§` here is a section of *Virtual I/O Device (VIRTIO) Version 1.2*, +//! OASIS Committee Specification 01. [`wire`] is §5.14.6's messages; this +//! module is the queues they travel on, laid out in one grant, over +//! `toyos-virtio`'s split virtqueue and the [`Doorbell`] its live transport +//! rings. +//! +//! # The device is not trusted +//! +//! Every word it writes back — a used element, a status, a response, an +//! event — is input from outside the driver. What `toyos-virtio` bounds (a +//! head, a length no more than a chain may be written) it bounds; what is the +//! sound device's is bounded here, and one that fails is a [`Refusal`] by +//! name. §2.7.4 has a driver believe nothing past the `len` a used element +//! gives, so a status, a response or an event shorter than its structure is a +//! refusal and is not read. +//! +//! **A refusal is the end of the device's use**, as `toyos-virtio`'s are: one +//! at bring-up leaves soundserver on its null sink, and one after it ends +//! soundserver, since no conforming device writes it and every period after it +//! is one the mixer would count as played on the device's word. +//! +//! # What is not here +//! +//! - **One stream, stream 0.** The device numbers its streams and reports +//! only how many, so there is nothing here to choose a second one by. +//! - **No feature of §5.14.6.6.2 is selected**, no channel map or jack is +//! asked about, and the receive queue is never configured. + +#![no_std] +#![forbid(unsafe_code)] + +extern crate alloc; +#[cfg(test)] +extern crate std; + +pub mod wire; + +#[cfg(test)] +mod tests; + +use toyos_device_memory::{DmaBuffers, Registers}; +use toyos_virtio::pci::Live; +use toyos_virtio::queue::{Buffer, Parts, Published, Used, UsedRefusal, Virtqueue}; + +use wire::{Event, Failed, Params, PcmInfo, Request}; + +/// The pipeline, in periods and bytes: 128 frames of 16-bit stereo each. +pub const PERIODS: usize = 8; +pub const PERIOD_BYTES: usize = 512; + +/// The one stream this driver opens. +pub const STREAM_ID: u32 = 0; + +/// Descriptors per queue, powers of two (§2.7). The transmit queue holds one +/// three-descriptor chain per period, its transfer header, its PCM and its +/// status; the control queue one request and its response; the event queue +/// one buffer per descriptor. +pub const TX_QUEUE_SIZE: u16 = 32; +pub const CONTROL_QUEUE_SIZE: u16 = 2; +pub const EVENT_QUEUE_SIZE: u16 = 8; +const TX_CHAIN: u16 = 3; +const EVENT_BUFS: u16 = EVENT_QUEUE_SIZE; + +/// The longest response this driver asks for: a header and one stream's +/// information (§5.14.6.2 has the driver provide exactly that). +const RESPONSE_BYTES: u32 = wire::HDR_BYTES + wire::PCM_INFO_BYTES; +const REQUEST_BYTES: u32 = wire::SET_PARAMS_BYTES; + +/// The grant's layout: the periods, then each queue on a page of its own with +/// the buffers its chains name. +const PCM: usize = 0x0000; +const TX_PARTS: Parts = Parts::contiguous(0x1000, TX_QUEUE_SIZE); +const TX_XFER: usize = 0x1800; +const TX_STATUS: usize = 0x1C00; +const CONTROL_PARTS: Parts = Parts::contiguous(0x2000, CONTROL_QUEUE_SIZE); +const REQUEST: usize = 0x2800; +const RESPONSE: usize = 0x2C00; +const EVENT_PARTS: Parts = Parts::contiguous(0x3000, EVENT_QUEUE_SIZE); +const EVENTS: usize = 0x3800; +pub const GRANT_BYTES: usize = 0x4000; + +const _: () = { + assert!(PCM + PERIODS * PERIOD_BYTES <= TX_PARTS.desc); + assert!(TX_PARTS.end(TX_QUEUE_SIZE) <= TX_XFER); + assert!(TX_XFER + PERIODS * wire::XFER_BYTES as usize <= TX_STATUS); + assert!(TX_STATUS + PERIODS * wire::STATUS_BYTES as usize <= CONTROL_PARTS.desc); + assert!(CONTROL_PARTS.end(CONTROL_QUEUE_SIZE) <= REQUEST); + assert!(REQUEST + REQUEST_BYTES as usize <= RESPONSE); + assert!(RESPONSE + RESPONSE_BYTES as usize <= EVENT_PARTS.desc); + assert!(EVENT_PARTS.end(EVENT_QUEUE_SIZE) <= EVENTS); + assert!(EVENTS + EVENT_BUFS as usize * wire::EVENT_BYTES as usize <= GRANT_BYTES); + assert!(PERIODS * TX_CHAIN as usize <= TX_QUEUE_SIZE as usize); + assert!(PERIODS <= u32::BITS as usize); + // §5.14.6.6.3.2: `buffer_bytes % period_bytes == 0`, and every period + // whole 16-bit stereo frames. + assert!(PERIOD_BYTES.is_multiple_of(4)); +}; + +/// The rates this driver can encode, best first. 44100 leads because it is +/// what the mixer, the resampler and the recorded counters are sized against; +/// 48000 is the one every other device offers. +const RATES: [(u32, u8); 2] = [(44100, wire::RATE_44100), (48000, wire::RATE_48000)]; + +/// How many times a control command's completion is polled before the device +/// is called silent, and the spins between two looks. +/// +/// **A count and not a deadline**: a wall clock keeps running while this guest +/// is not, so a duration here would measure the host's scheduler and call a +/// healthy device gone. Policy, not physics — the specification has no number, +/// and a device answering at all answers in one round trip. +const CONTROL_POLLS: u32 = 100_000; +const SPINS_PER_POLL: u32 = 256; + +/// Where a chain made available is told to the device: the live transport. +pub trait Doorbell { + fn ring(&self, published: Published); +} + +impl Doorbell for Live { + fn ring(&self, published: Published) { + self.notify(published); + } +} + +/// Why the device is not driven. Each is something it answered, or did not. +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +pub enum Refusal { + /// Its configuration says it has no PCM stream (§5.14.4). + NoStream, + /// A used ring `toyos-virtio` did not believe. + Used(u16, UsedRefusal), + /// It never answered a control request. + Silent(Request), + /// It answered fewer bytes than the response's structure. + ShortAnswer { request: Request, written: u32, wanted: u32 }, + /// It answered a status §5.14.6 does not define. + UnknownStatus(Request, u32), + /// It answered, and said no. + Rejected(Request, Failed), + /// Stream 0 does not carry output (§5.14.6.6.2). + NotOutput { direction: u8 }, + /// It offers no format, rate or channel count this driver writes. + NoFormat { formats: u64 }, + NoRate { rates: u64 }, + NoChannels { min: u8, max: u8 }, + /// A period's status is shorter than its structure. + ShortStatus { period: usize, written: u32 }, + /// A period's status is not `S_OK` (§5.14.6.8), or no status at all. + PeriodFailed { period: usize, status: Result }, + /// An event is shorter than its structure. + ShortEvent { written: u32 }, +} + +impl core::fmt::Display for Refusal { + fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result { + match self { + Self::NoStream => write!(f, "its configuration offers no PCM stream"), + Self::Used(queue, why) => write!(f, "its queue {queue}'s used ring: {why}"), + Self::Silent(request) => write!(f, "it never answered {}", request.name()), + Self::ShortAnswer { request, written, wanted } => write!( + f, + "it answered {} with {written} byte(s), where the response is {wanted}", + request.name() + ), + Self::UnknownStatus(request, word) => { + write!(f, "it answered {} with the status {word:#x}, which is none", request.name()) + } + Self::Rejected(request, failed) => { + write!(f, "it refused {} ({})", request.name(), failed.name()) + } + Self::NotOutput { direction } => { + write!(f, "its stream {STREAM_ID} has direction {direction}, not output") + } + Self::NoFormat { formats } => write!( + f, + "it offers the formats {formats:#x}, without S16 (bit {})", + wire::FMT_S16 + ), + Self::NoRate { rates } => write!( + f, + "it offers the rates {rates:#x}, without 44100 (bit {}) or 48000 (bit {})", + wire::RATE_44100, + wire::RATE_48000 + ), + Self::NoChannels { min, max } => { + write!(f, "it takes {min} to {max} channel(s), and this driver writes 1 or 2") + } + Self::ShortStatus { period, written } => write!( + f, + "it gave period {period} back with {written} byte(s) of its {}-byte status", + wire::STATUS_BYTES + ), + Self::PeriodFailed { period, status: Ok(failed) } => { + write!(f, "it failed period {period} ({})", failed.name()) + } + Self::PeriodFailed { period, status: Err(word) } => { + write!(f, "it gave period {period} back with the status {word:#x}, which is none") + } + Self::ShortEvent { written } => write!( + f, + "it wrote an event of {written} byte(s), where one is {}", + wire::EVENT_BYTES + ), + } + } +} + +/// The three queues, laid out in the grant before the transport is given +/// them: [`toyos_virtio::pci::Setup::enable`] takes each, and [`Sound::open`] +/// takes them back once the device is live. +pub struct Queues { + pub control: Virtqueue, + pub events: Virtqueue, + pub tx: Virtqueue, +} + +impl Queues { + /// # Panics + /// If the grant is shorter than [`GRANT_BYTES`]: the caller's own mistake. + pub fn new(mem: M) -> Self { + Self { + control: Virtqueue::new(mem.clone(), wire::CONTROL_QUEUE, CONTROL_QUEUE_SIZE, CONTROL_PARTS), + events: Virtqueue::new(mem.clone(), wire::EVENT_QUEUE, EVENT_QUEUE_SIZE, EVENT_PARTS), + tx: Virtqueue::new(mem, wire::TX_QUEUE, TX_QUEUE_SIZE, TX_PARTS), + } + } +} + +/// What the device said stream 0 is, and what this driver chose of it. +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +pub struct Stream { + pub info: PcmInfo, + pub rate: u32, + pub channels: u8, +} + +/// The device, live and configured: stream 0 prepared, every event buffer +/// posted. +pub struct Sound { + mem: M, + bell: D, + queues: Queues, + running: bool, +} + +impl Sound { + /// §5.14.5 on a live device whose configuration counts `streams`: post + /// the event buffers, ask what stream 0 is, choose a rate and a channel + /// count it offers, and set and prepare it (§5.14.6.6.1). + pub fn open(queues: Queues, mem: M, bell: D, streams: u32) -> Result<(Self, Stream), Refusal> { + if streams == 0 { + return Err(Refusal::NoStream); + } + let mut sound = Self { mem, bell, queues, running: false }; + for buffer in 0..EVENT_BUFS { + sound.post_event(buffer); + } + let info = sound.pcm_info()?; + let (rate, code, channels) = choose(&info)?; + let params = Params { + buffer_bytes: (PERIODS * PERIOD_BYTES) as u32, + period_bytes: PERIOD_BYTES as u32, + channels, + format: wire::FMT_S16, + rate: code, + }; + sound.control(Request::SetParams, &wire::set_params(STREAM_ID, params), wire::HDR_BYTES)?; + sound.simple(Request::Prepare, wire::R_PCM_PREPARE)?; + Ok((sound, Stream { info, rate, channels })) + } + + /// Where period `idx`'s samples go in the grant. + pub fn period(idx: usize) -> usize { + assert!(idx < PERIODS, "virtio-sound: there is no period {idx}"); + PCM + idx * PERIOD_BYTES + } + + pub fn running(&self) -> bool { + self.running + } + + /// Put period `idx` on the wire, starting the stream first if it is + /// stopped (§5.14.6.6.1). + /// + /// # Panics + /// If period `idx` is still the device's: the caller's own mistake. + pub fn submit(&mut self, idx: usize) -> Result<(), Refusal> { + let pcm = Self::period(idx); + if !self.running { + self.simple(Request::Start, wire::R_PCM_START)?; + self.running = true; + } + let xfer = TX_XFER + idx * wire::XFER_BYTES as usize; + let status = TX_STATUS + idx * wire::STATUS_BYTES as usize; + self.mem.write32(xfer, STREAM_ID); + let chain = [ + Buffer::readable(self.mem.device_addr(xfer), wire::XFER_BYTES), + Buffer::readable(self.mem.device_addr(pcm), PERIOD_BYTES as u32), + Buffer::writable(self.mem.device_addr(status), wire::STATUS_BYTES), + ]; + let published = self.queues.tx.publish(idx as u16 * TX_CHAIN, &chain); + self.bell.ring(published); + Ok(()) + } + + /// Stop the stream; nothing when it is stopped. + pub fn stop(&mut self) -> Result<(), Refusal> { + if self.running { + self.simple(Request::Stop, wire::R_PCM_STOP)?; + self.running = false; + } + Ok(()) + } + + /// The periods the device has given back that `taken` answers for, as a + /// mask beside it; nothing without a record. + /// + /// `taken` is the claim's interrupt record, taken before this call. 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. + pub fn played(&mut self, taken: Option) -> Result, Refusal> { + let Some(record) = taken else { return Ok(None) }; + let mask = self.completed()?; + Ok((mask != 0).then_some((mask, record))) + } + + /// Every period the device has given back since the last call, as a mask. + fn completed(&mut self) -> Result { + let mut mask = 0; + while let Some(Used { head, written }) = used(&mut self.queues.tx)? { + // A head `toyos-virtio` answered is one a chain was published at, + // and this driver publishes one only at a period's. + let period = (head / TX_CHAIN) as usize; + if written < wire::STATUS_BYTES { + return Err(Refusal::ShortStatus { period, written }); + } + let word = self.mem.read32(TX_STATUS + period * wire::STATUS_BYTES as usize); + match wire::status(word) { + Ok(Ok(())) => mask |= 1 << period, + Ok(Err(failed)) => return Err(Refusal::PeriodFailed { period, status: Ok(failed) }), + Err(word) => return Err(Refusal::PeriodFailed { period, status: Err(word) }), + } + } + Ok(mask) + } + + /// Every event the device has written since the last call, oldest first, + /// each buffer posted again once it is read. + pub fn events(&mut self, mut said: impl FnMut(Event)) -> Result<(), Refusal> { + while let Some(Used { head, written }) = used(&mut self.queues.events)? { + if written < wire::EVENT_BYTES { + return Err(Refusal::ShortEvent { written }); + } + let at = EVENTS + head as usize * wire::EVENT_BYTES as usize; + said(Event { code: self.mem.read32(at), data: self.mem.read32(at + 4) }); + self.post_event(head); + } + Ok(()) + } + + fn post_event(&mut self, buffer: u16) { + let at = EVENTS + buffer as usize * wire::EVENT_BYTES as usize; + let chain = [Buffer::writable(self.mem.device_addr(at), wire::EVENT_BYTES)]; + let published = self.queues.events.publish(buffer, &chain); + self.bell.ring(published); + } + + fn pcm_info(&mut self) -> Result { + let request = Request::PcmInfo; + self.control(request, &wire::pcm_info_query(STREAM_ID, 1), RESPONSE_BYTES)?; + let at = RESPONSE + wire::HDR_BYTES as usize; + Ok(PcmInfo::decode(core::array::from_fn(|word| self.mem.read32(at + 4 * word)))) + } + + fn simple(&mut self, request: Request, code: u32) -> Result<(), Refusal> { + self.control(request, &wire::pcm_hdr(code, STREAM_ID), wire::HDR_BYTES) + } + + /// One control round trip: the request's words into the request buffer, a + /// chain of it and a `response`-byte response, published and rung, and + /// the answer's length and status held to §5.14.6. + fn control(&mut self, request: Request, words: &[u32], response: u32) -> Result<(), Refusal> { + for (nth, word) in words.iter().enumerate() { + self.mem.write32(REQUEST + 4 * nth, *word); + } + let chain = [ + Buffer::readable(self.mem.device_addr(REQUEST), 4 * words.len() as u32), + Buffer::writable(self.mem.device_addr(RESPONSE), response), + ]; + let published = self.queues.control.publish(0, &chain); + self.bell.ring(published); + + let mut answer = None; + for _ in 0..CONTROL_POLLS { + answer = used(&mut self.queues.control)?; + if answer.is_some() { + break; + } + for _ in 0..SPINS_PER_POLL { + core::hint::spin_loop(); + } + } + let Used { written, .. } = answer.ok_or(Refusal::Silent(request))?; + if written < response { + return Err(Refusal::ShortAnswer { request, written, wanted: response }); + } + match wire::status(self.mem.read32(RESPONSE)) { + Ok(Ok(())) => Ok(()), + Ok(Err(failed)) => Err(Refusal::Rejected(request, failed)), + Err(word) => Err(Refusal::UnknownStatus(request, word)), + } + } +} + +/// The next chain `queue`'s device has finished with, or the used ring's +/// refusal under the queue's own index. +fn used(queue: &mut Virtqueue) -> Result, Refusal> { + let index = queue.index(); + queue.poll_used().map_err(|why| Refusal::Used(index, why)) +} + +/// A rate and a channel count stream 0 offers that this driver writes, and +/// the rate's code. +fn choose(info: &PcmInfo) -> Result<(u32, u8, u8), Refusal> { + if info.direction != wire::D_OUTPUT { + return Err(Refusal::NotOutput { direction: info.direction }); + } + if info.formats & (1 << wire::FMT_S16) == 0 { + return Err(Refusal::NoFormat { formats: info.formats }); + } + let (rate, code) = *RATES + .iter() + .find(|(_, code)| info.rates & (1 << code) != 0) + .ok_or(Refusal::NoRate { rates: info.rates })?; + // Stereo where the device takes it; the mixer converts either way. + let (min, max) = (info.channels_min, info.channels_max); + let channels = max.min(2); + if channels == 0 || min > channels { + return Err(Refusal::NoChannels { min, max }); + } + Ok((rate, code, channels)) +} diff --git a/userland/soundserver/virtio-sound/src/tests.rs b/userland/soundserver/virtio-sound/src/tests.rs new file mode 100644 index 00000000000..6a2987812fb --- /dev/null +++ b/userland/soundserver/virtio-sound/src/tests.rs @@ -0,0 +1,648 @@ +//! The driver against a model of the device that answers on the doorbell, +//! with §5.14's tables as the oracle for every byte it is sent and every +//! answer a device can lie in. + +use alloc::boxed::Box; +use alloc::rc::Rc; +use alloc::vec; +use alloc::vec::Vec; +use core::cell::RefCell; + +use toyos_device_memory::DmaBuffers; +use toyos_virtio::queue::{Published, UsedRefusal}; + +use crate::wire::{self, Event, Failed, PcmInfo, Request}; +use crate::{ + Doorbell, Queues, Refusal, Sound, Stream, CONTROL_PARTS, EVENTS, EVENT_PARTS, PERIODS, PERIOD_BYTES, + TX_PARTS, +}; + +/// Where the device reaches the grant's first byte. +const BASE: u64 = 0x4000_0000; + +/// The grant, as both the driver and the model reach it. +#[derive(Clone)] +struct Mem(Rc>>); + +impl Mem { + fn new() -> Self { + // Anything but zero, so a part the driver forgets to clear shows. + Self(Rc::new(RefCell::new(vec![0xA5; crate::GRANT_BYTES]))) + } + + fn get(&self, at: usize, bytes: usize) -> u64 { + let mem = self.0.borrow(); + let mut word = [0u8; 8]; + word[..bytes].copy_from_slice(&mem[at..at + bytes]); + u64::from_le_bytes(word) + } + + fn set(&self, at: usize, bytes: usize, value: u64) { + self.0.borrow_mut()[at..at + bytes].copy_from_slice(&value.to_le_bytes()[..bytes]); + } + + fn bytes_at(&self, addr: u64, len: u32) -> Vec { + let at = (addr - BASE) as usize; + self.0.borrow()[at..at + len as usize].to_vec() + } +} + +impl DmaBuffers for Mem { + fn bytes(&self) -> usize { + crate::GRANT_BYTES + } + fn device_addr(&self, at: usize) -> u64 { + BASE + at as u64 + } + fn read16(&self, at: usize) -> u16 { + self.get(at, 2) as u16 + } + fn read32(&self, at: usize) -> u32 { + self.get(at, 4) as u32 + } + fn read64(&self, at: usize) -> u64 { + self.get(at, 8) + } + fn write16(&self, at: usize, value: u16) { + self.set(at, 2, value as u64) + } + fn write32(&self, at: usize, value: u32) { + self.set(at, 4, value as u64) + } + fn write64(&self, at: usize, value: u64) { + self.set(at, 8, value) + } + fn publish(&self) {} + fn observe(&self) {} +} + +/// One element of a chain the device took: §2.7.5's descriptor. +#[derive(Clone, Debug, PartialEq, Eq)] +struct Desc { + addr: u64, + len: u32, + writable: bool, +} + +/// What the model answers a control request with. +struct Answer { + status: u32, + payload: Vec, + /// The `len` it reports, where it is not the bytes it wrote. + len: Option, + silent: bool, +} + +fn ok() -> Answer { + Answer { status: wire::S_OK, payload: Vec::new(), len: None, silent: false } +} + +/// QEMU's stream at v11.1.1: S16 at 44100 and 48000 among others, output, +/// one or two channels. +fn qemu_stream() -> PcmInfo { + PcmInfo { + hda_fn_nid: 0, + features: 0, + formats: 1 << wire::FMT_S16 | 1 << 3, + rates: 1 << wire::RATE_44100 | 1 << wire::RATE_48000 | 1 << 1, + direction: wire::D_OUTPUT, + channels_min: 1, + channels_max: 2, + } +} + +/// `info` as the eight words of §5.14.6.6.2's structure. +fn encode(info: PcmInfo) -> Vec { + let mut bytes = [0u8; 32]; + bytes[0..4].copy_from_slice(&info.hda_fn_nid.to_le_bytes()); + bytes[4..8].copy_from_slice(&info.features.to_le_bytes()); + bytes[8..16].copy_from_slice(&info.formats.to_le_bytes()); + bytes[16..24].copy_from_slice(&info.rates.to_le_bytes()); + bytes[24] = info.direction; + bytes[25] = info.channels_min; + bytes[26] = info.channels_max; + words(&bytes) +} + +fn words(bytes: &[u8]) -> Vec { + bytes.chunks(4).map(|w| u32::from_le_bytes(w.try_into().unwrap())).collect() +} + +/// How the model answers each control request it reads. +type Answering = Box Answer>; + +struct State { + /// Per queue: the available entries taken so far, and the used ones + /// written. + taken: [u16; 3], + used: [u16; 3], + /// Every control request, as the device read it. + requests: Vec>, + /// Every chain the device took, by queue, in order. + chains: [Vec<(u16, Vec)>; 3], + /// Chains the device holds: the transmit queue's until played, the + /// event queue's until an event. + held: [Vec<(u16, Vec)>; 3], + answer: Answering, +} + +#[derive(Clone)] +struct Model { + mem: Mem, + state: Rc>, +} + +impl Model { + fn new() -> Self { + Self::answering(|request| match request[0] { + wire::R_PCM_INFO => Answer { payload: encode(qemu_stream()), ..ok() }, + _ => ok(), + }) + } + + fn answering(answer: impl FnMut(&[u32]) -> Answer + 'static) -> Self { + Self { + mem: Mem::new(), + state: Rc::new(RefCell::new(State { + taken: [0; 3], + used: [0; 3], + requests: Vec::new(), + chains: Default::default(), + held: Default::default(), + answer: Box::new(answer), + })), + } + } + + fn parts(queue: u16) -> (crate::Parts, u16) { + match queue { + wire::CONTROL_QUEUE => (CONTROL_PARTS, crate::CONTROL_QUEUE_SIZE), + wire::EVENT_QUEUE => (EVENT_PARTS, crate::EVENT_QUEUE_SIZE), + wire::TX_QUEUE => (TX_PARTS, crate::TX_QUEUE_SIZE), + other => panic!("model: queue {other} was never configured"), + } + } + + /// The chain at `head`, by following its descriptors (§2.7.5). + fn chain(&self, queue: u16, head: u16) -> Vec { + let (parts, size) = Self::parts(queue); + let mut descs = Vec::new(); + let mut at = head; + loop { + assert!(at < size && descs.len() < size as usize, "model: a chain off the table"); + let d = parts.desc + at as usize * 16; + let flags = self.mem.get(d + 12, 2); + descs.push(Desc { + addr: self.mem.get(d, 8), + len: self.mem.get(d + 8, 4) as u32, + writable: flags & 2 != 0, + }); + if flags & 1 == 0 { + return descs; + } + at = self.mem.get(d + 14, 2) as u16; + } + } + + /// A used element on `queue`: `head`, `len` bytes written (§2.7.8). + fn give_back(&self, queue: u16, head: u16, len: u32) { + let (parts, size) = Self::parts(queue); + let mut state = self.state.borrow_mut(); + let used = &mut state.used[queue as usize]; + let element = parts.used + 4 + (*used % size) as usize * 8; + self.mem.set(element, 4, head as u64); + self.mem.set(element + 4, 4, len as u64); + *used = used.wrapping_add(1); + self.mem.set(parts.used + 2, 2, *used as u64); + } + + /// Play the `count` oldest periods: each status `status`, `len` reported. + fn play_with(&self, count: usize, status: u32, len: u32) { + for _ in 0..count { + let (head, chain) = self.state.borrow_mut().held[wire::TX_QUEUE as usize].remove(0); + self.mem.set((chain[2].addr - BASE) as usize, 4, status as u64); + self.give_back(wire::TX_QUEUE, head, len); + } + } + + fn play(&self, count: usize) { + self.play_with(count, wire::S_OK, wire::STATUS_BYTES); + } + + /// Write `event` into the oldest event buffer, `len` reported. + fn event(&self, event: Event, len: u32) { + let (head, chain) = self.state.borrow_mut().held[wire::EVENT_QUEUE as usize].remove(0); + let at = (chain[0].addr - BASE) as usize; + self.mem.set(at, 4, event.code as u64); + self.mem.set(at + 4, 4, event.data as u64); + self.give_back(wire::EVENT_QUEUE, head, len); + } + + /// The control requests' codes, in the order the device read them. + fn codes(&self) -> Vec { + self.state.borrow().requests.iter().map(|r| words(&r[..4])[0]).collect() + } + + fn chains(&self, queue: u16) -> Vec<(u16, Vec)> { + self.state.borrow().chains[queue as usize].clone() + } +} + +impl Doorbell for Model { + fn ring(&self, published: Published) { + let queue = published.queue(); + let (parts, size) = Self::parts(queue); + loop { + let available = self.mem.get(parts.avail + 2, 2) as u16; + let taken = self.state.borrow().taken[queue as usize]; + if taken == available { + return; + } + let head = self.mem.get(parts.avail + 4 + (taken % size) as usize * 2, 2) as u16; + self.state.borrow_mut().taken[queue as usize] = taken.wrapping_add(1); + let chain = self.chain(queue, head); + self.state.borrow_mut().chains[queue as usize].push((head, chain.clone())); + if queue != wire::CONTROL_QUEUE { + self.state.borrow_mut().held[queue as usize].push((head, chain)); + continue; + } + let request: Vec = chain + .iter() + .filter(|d| !d.writable) + .flat_map(|d| self.mem.bytes_at(d.addr, d.len)) + .collect(); + self.state.borrow_mut().requests.push(request.clone()); + let answer = (self.state.borrow_mut().answer)(&words(&request)); + if answer.silent { + continue; + } + let response = chain.iter().find(|d| d.writable).expect("a response buffer"); + let mut out = vec![answer.status]; + out.extend(&answer.payload); + let wrote = (4 * out.len() as u32).min(response.len); + for (nth, word) in out.iter().enumerate().take(wrote as usize / 4) { + self.mem.set((response.addr - BASE) as usize + 4 * nth, 4, *word as u64); + } + self.give_back(queue, head, answer.len.unwrap_or(wrote)); + } + } +} + +fn open(model: &Model, streams: u32) -> Result<(Sound, Stream), Refusal> { + Sound::open(Queues::new(model.mem.clone()), model.mem.clone(), model.clone(), streams) +} + +fn opened(model: &Model) -> Sound { + open(model, 1).unwrap_or_else(|why| panic!("the model's device opens: {why}")).0 +} + +// --- §5.14.6: the messages, byte for byte --- + +/// §5.14.6.1 and §5.14.6.6.2: `virtio_snd_query_info` is `hdr`, `start_id`, +/// `count` and `size`, each `le32`; §5.14.6.6.3: `virtio_snd_pcm_set_params` +/// is `hdr`, `stream_id`, `buffer_bytes`, `period_bytes`, `features`, then +/// `channels`, `format`, `rate` and a padding byte; §5.14.6.6: a PREPARE is +/// `hdr` and `stream_id`. The bring-up is §5.14.5's order. +#[test] +fn the_bring_up_sends_the_specifications_bytes_in_its_order() { + let model = Model::new(); + let (_, stream) = open(&model, 1).expect("the model's device opens"); + assert_eq!(stream.rate, 44100); + assert_eq!(stream.channels, 2); + assert_eq!(stream.info, qemu_stream()); + + let requests = model.state.borrow().requests.clone(); + assert_eq!( + requests, + [ + vec![0x00, 0x01, 0, 0, 0, 0, 0, 0, 1, 0, 0, 0, 32, 0, 0, 0], + vec![ + 0x01, 0x01, 0, 0, 0, 0, 0, 0, 0x00, 0x10, 0, 0, 0x00, 0x02, 0, 0, 0, 0, 0, 0, 2, + 5, 6, 0 + ], + vec![0x02, 0x01, 0, 0, 0, 0, 0, 0], + ] + ); + // §5.14.6.2: a response buffer of `sizeof(hdr) + count * size`; a bare + // status for the others. + let responses: Vec = model + .chains(wire::CONTROL_QUEUE) + .iter() + .map(|(_, chain)| { + assert_eq!(chain.len(), 2); + assert!(!chain[0].writable && chain[1].writable); + chain[1].len + }) + .collect(); + assert_eq!(responses, [36, 4, 4]); + // §5.14.5.1: the event queue full of device-writable buffers of at least + // `virtio_snd_event`'s eight bytes, and none device-readable. + let events = model.chains(wire::EVENT_QUEUE); + assert_eq!(events.len(), crate::EVENT_QUEUE_SIZE as usize); + for (head, chain) in &events { + assert_eq!(chain, &[Desc { addr: BASE + (EVENTS + *head as usize * 8) as u64, len: 8, writable: true }]); + } + assert!(model.chains(wire::TX_QUEUE).is_empty(), "a period before the stream is started"); +} + +/// §5.14.6.6.2's `virtio_snd_pcm_info`: `hdr` at 0, `features` at 4, +/// `formats` at 8 and `rates` at 16 as `le64`, then `direction`, +/// `channels_min` and `channels_max` at 24, 25 and 26. +#[test] +fn stream_information_is_decoded_at_the_specifications_offsets() { + let mut bytes = [0u8; 32]; + bytes[0..4].copy_from_slice(&0x11u32.to_le_bytes()); + bytes[4..8].copy_from_slice(&0x2222_2222u32.to_le_bytes()); + bytes[8..16].copy_from_slice(&0x0807_0605_0403_0201u64.to_le_bytes()); + bytes[16..24].copy_from_slice(&0x1817_1615_1413_1211u64.to_le_bytes()); + bytes[24..27].copy_from_slice(&[1, 3, 7]); + bytes[27..32].copy_from_slice(&[0xFF; 5]); + let info = PcmInfo::decode(words(&bytes).try_into().unwrap()); + assert_eq!( + info, + PcmInfo { + hda_fn_nid: 0x11, + features: 0x2222_2222, + formats: 0x0807_0605_0403_0201, + rates: 0x1817_1615_1413_1211, + direction: 1, + channels_min: 3, + channels_max: 7, + } + ); +} + +/// §5.14.6: the four status codes and the four event types. +#[test] +fn every_code_is_the_specifications_number() { + assert_eq!(wire::status(0x8000), Ok(Ok(()))); + assert_eq!(wire::status(0x8001), Ok(Err(Failed::BadMsg))); + assert_eq!(wire::status(0x8002), Ok(Err(Failed::NotSupp))); + assert_eq!(wire::status(0x8003), Ok(Err(Failed::IoErr))); + for none in [0, 0x7FFF, 0x8004, u32::MAX] { + assert_eq!(wire::status(none), Err(none)); + } + let named = |code| Event { code, data: 0 }.name(); + assert_eq!(named(0x1000), Some("jack connected")); + assert_eq!(named(0x1001), Some("jack disconnected")); + assert_eq!(named(0x1100), Some("period elapsed")); + assert_eq!(named(0x1101), Some("PCM XRUN")); + assert_eq!(named(0x1102), None); + // §5.14.6.6.2's enumerations, by the bit each is. + assert_eq!((wire::FMT_S16, wire::RATE_44100, wire::RATE_48000), (5, 6, 7)); +} + +// --- What the driver chooses --- + +/// A rate and a channel count the stream offers: 44100 over 48000, stereo +/// over mono, and each refusal names the bitmap or range it read. +#[test] +fn the_stream_is_set_to_what_it_offers_or_refused_by_what_it_lacks() { + let with = |change: fn(&mut PcmInfo)| { + let mut info = qemu_stream(); + change(&mut info); + let model = Model::answering(move |request| match request[0] { + wire::R_PCM_INFO => Answer { payload: encode(info), ..ok() }, + _ => ok(), + }); + let set = open(&model, 1).map(|(_, stream)| (stream.rate, stream.channels)); + (set, model) + }; + assert_eq!(with(|i| i.rates = 1 << wire::RATE_48000).0, Ok((48000, 2))); + assert_eq!(with(|i| i.channels_max = 1).0, Ok((44100, 1))); + assert_eq!(with(|i| i.channels_max = 8).0, Ok((44100, 2))); + let refused = [ + (with(|i| i.direction = 1), Refusal::NotOutput { direction: 1 }), + (with(|i| i.formats = 1 << 3), Refusal::NoFormat { formats: 1 << 3 }), + (with(|i| i.rates = 1 << 10), Refusal::NoRate { rates: 1 << 10 }), + (with(|i| i.channels_min = 3), Refusal::NoChannels { min: 3, max: 2 }), + (with(|i| i.channels_max = 0), Refusal::NoChannels { min: 1, max: 0 }), + ]; + for ((set, model), refusal) in refused { + assert_eq!(set, Err(refusal)); + // Nothing past the question: a stream it cannot carry is never set. + assert_eq!(model.codes(), [wire::R_PCM_INFO]); + } +} + +// --- A device's answers, believed only as far as §5.14.6 goes --- + +/// The negative control: each malformed answer is refused by its own name, +/// and the bring-up goes no further than the request it answered. +#[test] +fn a_malformed_control_answer_is_refused_by_name() { + let answering = |answer: fn(&[u32]) -> Option| { + let model = Model::answering(move |request| { + answer(request).unwrap_or_else(|| match request[0] { + wire::R_PCM_INFO => Answer { payload: encode(qemu_stream()), ..ok() }, + _ => ok(), + }) + }); + (open(&model, 1).err(), model.codes().len()) + }; + // A status §5.14.6 does not define. + assert_eq!( + answering(|r| (r[0] == wire::R_PCM_PREPARE).then(|| Answer { status: 0x1234, ..ok() })), + (Some(Refusal::UnknownStatus(Request::Prepare, 0x1234)), 3) + ); + // An `S_OK` with no stream information behind it. + assert_eq!( + answering(|r| (r[0] == wire::R_PCM_INFO).then(ok)), + (Some(Refusal::ShortAnswer { request: Request::PcmInfo, written: 4, wanted: 36 }), 1) + ); + // A `len` that covers less than the status word. + assert_eq!( + answering(|r| (r[0] == wire::R_PCM_PREPARE).then(|| Answer { len: Some(3), ..ok() })), + (Some(Refusal::ShortAnswer { request: Request::Prepare, written: 3, wanted: 4 }), 3) + ); + // A `len` past the response buffer is `toyos-virtio`'s to refuse. + assert!(matches!( + answering(|r| (r[0] == wire::R_PCM_SET_PARAMS).then(|| Answer { len: Some(5), ..ok() })), + (Some(Refusal::Used(wire::CONTROL_QUEUE, UsedRefusal::Written { head: 0, .. })), 2) + )); + // A well-formed no. + assert_eq!( + answering(|r| (r[0] == wire::R_PCM_SET_PARAMS).then(|| Answer { status: wire::S_NOT_SUPP, ..ok() })), + (Some(Refusal::Rejected(Request::SetParams, Failed::NotSupp)), 2) + ); +} + +/// A device that never answers is called silent after a bounded count of +/// looks, and a device with no stream is never asked anything. +#[test] +fn a_silent_device_and_a_streamless_one_are_refused() { + let model = Model::answering(|_| Answer { silent: true, ..ok() }); + assert_eq!(open(&model, 1).err(), Some(Refusal::Silent(Request::PcmInfo))); + + let model = Model::new(); + assert_eq!(open(&model, 0).err(), Some(Refusal::NoStream)); + assert!(model.codes().is_empty() && model.chains(wire::EVENT_QUEUE).is_empty()); +} + +// --- §5.14.6.8: the periods --- + +/// The stream is started once before its first period, every period is +/// §5.14.6.8's three parts — the stream id, the PCM, a status for the device +/// — and a completion is the set of periods given back with `S_OK`. +#[test] +fn periods_go_out_after_one_start_and_come_back_as_a_mask() { + let model = Model::new(); + let mut sound = opened(&model); + for idx in 0..PERIODS { + sound.submit(idx).expect("a period goes out"); + } + assert_eq!(model.codes(), [wire::R_PCM_INFO, wire::R_PCM_SET_PARAMS, wire::R_PCM_PREPARE, wire::R_PCM_START]); + let chains = model.chains(wire::TX_QUEUE); + assert_eq!(chains.len(), PERIODS); + for (idx, (head, chain)) in chains.iter().enumerate() { + assert_eq!(*head as usize, idx * 3); + assert_eq!(chain.len(), 3); + assert_eq!((chain[0].len, chain[0].writable), (wire::XFER_BYTES, false)); + assert_eq!(model.mem.bytes_at(chain[0].addr, 4), [0, 0, 0, 0], "stream 0"); + assert_eq!(chain[1], Desc { addr: BASE + (idx * PERIOD_BYTES) as u64, len: 512, writable: false }); + assert_eq!((chain[2].len, chain[2].writable), (wire::STATUS_BYTES, true)); + } + assert_eq!(sound.completed(), Ok(0)); + model.play(3); + assert_eq!(sound.completed(), Ok(0b111)); + model.play(5); + assert_eq!(sound.completed(), Ok(0b1111_1000)); + + // A drained stream stops once, and starts again on its next period. + sound.stop().expect("STOP"); + sound.stop().expect("nothing"); + sound.submit(4).expect("a period goes out"); + assert_eq!( + model.codes()[3..], + [wire::R_PCM_START, wire::R_PCM_STOP, wire::R_PCM_START] + ); +} + +/// A period the device gives back with a short status, a failure, or a +/// status §5.14.6.8 does not define is refused by that name, as is a used +/// element for no period in flight. +#[test] +fn a_period_given_back_wrongly_is_refused_by_name() { + let given_back = |status: u32, len: u32| { + let model = Model::new(); + let mut sound = opened(&model); + sound.submit(0).expect("a period goes out"); + sound.submit(1).expect("a period goes out"); + model.play(1); + model.play_with(1, status, len); + sound.completed() + }; + assert_eq!(given_back(wire::S_OK, 8), Ok(0b11)); + assert_eq!(given_back(wire::S_OK, 4), Err(Refusal::ShortStatus { period: 1, written: 4 })); + assert_eq!( + given_back(wire::S_IO_ERR, 8), + Err(Refusal::PeriodFailed { period: 1, status: Ok(Failed::IoErr) }) + ); + assert_eq!(given_back(7, 8), Err(Refusal::PeriodFailed { period: 1, status: Err(7) })); + + let model = Model::new(); + let mut sound = opened(&model); + sound.submit(0).expect("a period goes out"); + model.give_back(wire::TX_QUEUE, 3, 8); + assert_eq!( + sound.completed(), + Err(Refusal::Used(wire::TX_QUEUE, UsedRefusal::NoChain { head: 3 })) + ); +} + +/// An event is read, handed up and its buffer posted again; one shorter than +/// `virtio_snd_event` is refused and not read. +#[test] +fn an_event_is_handed_up_and_its_buffer_posted_again() { + let model = Model::new(); + let mut sound = opened(&model); + model.event(Event { code: wire::EVT_PCM_XRUN, data: 0 }, 8); + let mut said = Vec::new(); + sound.events(|event| said.push(event)).expect("a whole event"); + assert_eq!(said, [Event { code: wire::EVT_PCM_XRUN, data: 0 }]); + assert_eq!(model.chains(wire::EVENT_QUEUE).len(), crate::EVENT_QUEUE_SIZE as usize + 1); + + model.event(Event { code: wire::EVT_JACK_CONNECTED, data: 1 }, 4); + let mut said = Vec::new(); + assert_eq!(sound.events(|event| said.push(event)), Err(Refusal::ShortEvent { written: 4 })); + assert!(said.is_empty()); +} + +/// The claim's interrupt record, as [`Sound::played`] is handed it: when the +/// oldest and newest notification since the last take landed. +#[derive(Default)] +struct Claim(Option<(u64, u64)>); + +impl Claim { + fn land(&mut self, at: u64) { + let first = self.0.map_or(at, |(first, _)| first); + self.0 = Some((first, at)); + } + + fn take(&mut self) -> Option<(u64, u64)> { + self.0.take() + } +} + +/// No period is stranded: every one comes back exactly once, and none while +/// the claim is left unreadable. +/// +/// 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 +/// ring, in every interleaving; then the driver wakes for as long as the +/// claim reads ready, which is all a waiting soundserver does. A ring read +/// with no record that took what it found would leave a period nobody's +/// notification answers for, and the claim would never wake the driver for +/// it again. +#[test] +fn no_period_is_stranded_between_its_used_element_and_its_notification() { + #[derive(Clone, Copy)] + enum Step { + Used, + Notified, + Take, + Read, + } + const DEVICE: [Step; 4] = [Step::Used, Step::Notified, Step::Used, Step::Notified]; + const DRIVER: [Step; 4] = [Step::Take, Step::Read, Step::Take, Step::Read]; + + let mut orders = 0; + // Bit `n` of `pick` set: the `n`th step of the merge is the device's. + for pick in 0u32..1 << 8 { + if pick.count_ones() != 4 { + continue; + } + orders += 1; + let model = Model::new(); + let mut sound = opened(&model); + sound.submit(0).expect("a period goes out"); + sound.submit(1).expect("a period goes out"); + let mut claim = Claim::default(); + let (mut device, mut driver, mut landed) = (DEVICE.iter(), DRIVER.iter(), 0); + let mut taken = None; + let mut back = Vec::new(); + for n in 0..8 { + let step = if pick & 1 << n != 0 { device.next() } else { driver.next() }; + match step.copied().expect("four steps each") { + Step::Used => model.play(1), + Step::Notified => { + landed += 1; + claim.land(landed); + } + Step::Take => taken = claim.take(), + Step::Read => back.extend(sound.played(taken.take()).expect("a whole period")), + } + } + while let Some(record) = claim.take() { + back.extend(sound.played(Some(record)).expect("a whole period")); + } + let masks: Vec = back.iter().map(|(mask, _)| *mask).collect(); + let mut seen = 0; + for mask in &masks { + assert_eq!(seen & mask, 0, "order {pick:#010b}: a period came back twice in {masks:?}"); + seen |= mask; + } + assert_eq!(seen, 0b11, "order {pick:#010b}: the periods that came back are {masks:?}"); + } + assert_eq!(orders, 70); +} diff --git a/userland/soundserver/virtio-sound/src/wire.rs b/userland/soundserver/virtio-sound/src/wire.rs new file mode 100644 index 00000000000..82be867fb45 --- /dev/null +++ b/userland/soundserver/virtio-sound/src/wire.rs @@ -0,0 +1,187 @@ +//! The sound device's messages (§5.14.6): their codes, and each structure as +//! the little-endian words it is on the wire. +//! +//! **Every structure here is a whole number of `le32` words**, the `u8` +//! fields of one packed into a word in the order the structure lists them, +//! so a message is written into the grant and read out of it as words and no +//! field is ever a byte access of its own. + +/// §5.14.2: the queues, by index. The fourth, `rxq`, carries input and is +/// never configured. +pub const CONTROL_QUEUE: u16 = 0; +pub const EVENT_QUEUE: u16 = 1; +pub const TX_QUEUE: u16 = 2; + +/// §5.14.6: the request types this driver sends. +pub const R_PCM_INFO: u32 = 0x0100; +pub const R_PCM_SET_PARAMS: u32 = 0x0101; +pub const R_PCM_PREPARE: u32 = 0x0102; +pub const R_PCM_START: u32 = 0x0104; +pub const R_PCM_STOP: u32 = 0x0105; + +/// §5.14.6: the event types a device sends. +pub const EVT_JACK_CONNECTED: u32 = 0x1000; +pub const EVT_JACK_DISCONNECTED: u32 = 0x1001; +pub const EVT_PCM_PERIOD_ELAPSED: u32 = 0x1100; +pub const EVT_PCM_XRUN: u32 = 0x1101; + +/// §5.14.6: the status codes, and the only four a device answers. +pub const S_OK: u32 = 0x8000; +pub const S_BAD_MSG: u32 = 0x8001; +pub const S_NOT_SUPP: u32 = 0x8002; +pub const S_IO_ERR: u32 = 0x8003; + +/// §5.14.6: `VIRTIO_SND_D_OUTPUT`. +pub const D_OUTPUT: u8 = 0; + +/// §5.14.6.6.2: the one sample format this driver writes, and the two rates +/// it can encode, as bit numbers of `formats` and `rates`. +pub const FMT_S16: u8 = 5; +pub const RATE_44100: u8 = 6; +pub const RATE_48000: u8 = 7; + +/// Bytes of each structure. +pub const HDR_BYTES: u32 = 4; +pub const QUERY_INFO_BYTES: u32 = 16; +pub const PCM_HDR_BYTES: u32 = 8; +pub const PCM_INFO_BYTES: u32 = 32; +pub const SET_PARAMS_BYTES: u32 = 24; +pub const XFER_BYTES: u32 = 4; +pub const STATUS_BYTES: u32 = 8; +pub const EVENT_BYTES: u32 = 8; + +/// A control request this driver sends, by what it is called. +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +pub enum Request { + PcmInfo, + SetParams, + Prepare, + Start, + Stop, +} + +impl Request { + pub fn name(self) -> &'static str { + match self { + Self::PcmInfo => "PCM_INFO", + Self::SetParams => "PCM_SET_PARAMS", + Self::Prepare => "PCM_PREPARE", + Self::Start => "PCM_START", + Self::Stop => "PCM_STOP", + } + } +} + +/// A status a device answered that was not `S_OK`, by its name. +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +pub enum Failed { + BadMsg, + NotSupp, + IoErr, +} + +impl Failed { + pub fn name(self) -> &'static str { + match self { + Self::BadMsg => "VIRTIO_SND_S_BAD_MSG", + Self::NotSupp => "VIRTIO_SND_S_NOT_SUPP", + Self::IoErr => "VIRTIO_SND_S_IO_ERR", + } + } +} + +/// A status word: `Ok(Ok(()))` for `S_OK`, `Ok(Err(_))` for one of the three +/// failures, and `Err(word)` for anything §5.14.6 does not define. +pub fn status(word: u32) -> Result, u32> { + match word { + S_OK => Ok(Ok(())), + S_BAD_MSG => Ok(Err(Failed::BadMsg)), + S_NOT_SUPP => Ok(Err(Failed::NotSupp)), + S_IO_ERR => Ok(Err(Failed::IoErr)), + other => Err(other), + } +} + +/// §5.14.6.1: `virtio_snd_query_info` asking for `count` PCM streams from +/// `start_id`, each answered as one [`PCM_INFO_BYTES`] structure. +pub fn pcm_info_query(start_id: u32, count: u32) -> [u32; 4] { + [R_PCM_INFO, start_id, count, PCM_INFO_BYTES] +} + +/// §5.14.6.6: `virtio_snd_pcm_hdr`, which is the whole of a PREPARE, START or +/// STOP. +pub fn pcm_hdr(code: u32, stream_id: u32) -> [u32; 2] { + [code, stream_id] +} + +/// §5.14.6.6.3: `virtio_snd_pcm_set_params`, no feature selected and the +/// padding byte 0 (§5.14.6.6.3.2). +pub fn set_params(stream_id: u32, params: Params) -> [u32; 6] { + let Params { buffer_bytes, period_bytes, channels, format, rate } = params; + [ + R_PCM_SET_PARAMS, + stream_id, + buffer_bytes, + period_bytes, + 0, + u32::from_le_bytes([channels, format, rate, 0]), + ] +} + +/// What a SET_PARAMS selects. +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +pub struct Params { + pub buffer_bytes: u32, + pub period_bytes: u32, + pub channels: u8, + pub format: u8, + pub rate: u8, +} + +/// §5.14.6.6.2: one stream's `virtio_snd_pcm_info`, every field the device's. +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +pub struct PcmInfo { + pub hda_fn_nid: u32, + pub features: u32, + pub formats: u64, + pub rates: u64, + pub direction: u8, + pub channels_min: u8, + pub channels_max: u8, +} + +impl PcmInfo { + /// The structure's eight words, as the device wrote them. + pub fn decode(words: [u32; 8]) -> Self { + let [direction, channels_min, channels_max, _] = words[6].to_le_bytes(); + Self { + hda_fn_nid: words[0], + features: words[1], + formats: words[2] as u64 | (words[3] as u64) << 32, + rates: words[4] as u64 | (words[5] as u64) << 32, + direction, + channels_min, + channels_max, + } + } +} + +/// §5.14.6: `virtio_snd_event`. +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +pub struct Event { + pub code: u32, + pub data: u32, +} + +impl Event { + /// The event's name, or `None` for a type §5.14.6 does not define. + pub fn name(self) -> Option<&'static str> { + match self.code { + EVT_JACK_CONNECTED => Some("jack connected"), + EVT_JACK_DISCONNECTED => Some("jack disconnected"), + EVT_PCM_PERIOD_ELAPSED => Some("period elapsed"), + EVT_PCM_XRUN => Some("PCM XRUN"), + _ => None, + } + } +} diff --git a/userland/supervisor/src/main.rs b/userland/supervisor/src/main.rs index 55658eb162f..3349ee8e944 100644 --- a/userland/supervisor/src/main.rs +++ b/userland/supervisor/src/main.rs @@ -2135,9 +2135,7 @@ fn start<'a>( let request = DeviceRequest::parse(name) .unwrap_or_else(|| panic!("supervisor: `{name}` is not a device this ABI has")); // A device this machine does not have is not endowed, and the supervisor says - // which: "did I get an HDA or a virtio-sound?" becomes "which claims - // are in my endowment table?", which is the same question with the - // answer already in hand. + // which. // // The label is the manifest's own spelling, which is exactly what the // claimant looks the claim up by: one string, written once. diff --git a/userland/toyos-pci-claim/Cargo.toml b/userland/toyos-pci-claim/Cargo.toml new file mode 100644 index 00000000000..fa4dbdf34df --- /dev/null +++ b/userland/toyos-pci-claim/Cargo.toml @@ -0,0 +1,21 @@ +[package] +name = "toyos-pci-claim" +description = "A PCI claim as a userland driver reaches its function: toyos-device-memory's register window and DMA grant over the SDK's Window, and a virtio function's capability walk, mapping and feature negotiation through toyos-virtio." +version = "0.1.0" +edition = "2021" +license = "MIT OR Apache-2.0" +publish = false + +[dependencies] +toyos = { path = "../../toyos" } +toyos-abi = { path = "../../toyos-abi" } +toyos-device-memory = { path = "../../toyos-device-memory" } +toyos-virtio = { path = "../../toyos-virtio" } + +[features] +# `Grant::leaked`, for a driver crate's host tests: a dev-dependency's, never +# enabled by a build that ships. +host-grant = [] + +[lib] +doctest = false diff --git a/userland/toyos-pci-claim/src/lib.rs b/userland/toyos-pci-claim/src/lib.rs new file mode 100644 index 00000000000..eb8eeb69a16 --- /dev/null +++ b/userland/toyos-pci-claim/src/lib.rs @@ -0,0 +1,171 @@ +//! A PCI claim, as a userland driver reaches the function behind it. +//! +//! What the kernel keeps is the claim: config space, the vector it programmed +//! into the function's MSI-X table, and the address space the function +//! translates through. **This crate is the one implementation of +//! `toyos-device-memory`'s boundary over the SDK's [`Window`]** — [`Bar`] and +//! [`Grant`], an instruction deep, every offset that reaches them bounded by +//! the driver crate above — and, for a virtio function, the bring-up every +//! virtio driver makes before its device type begins ([`virtio`]). +//! +//! Neither [`Bar`] nor [`Grant`] owns its mapping: whoever holds the driver +//! keeps the `SharedMemory` and the `DmaRegion` for as long as it keeps the +//! driver, and the constructors here hand both back together. + +use std::sync::atomic::{fence, Ordering}; + +use toyos::shm::SharedMemory; +use toyos::volatile::Window; +use toyos::{DmaRegion, PciDev}; +use toyos_abi::syscall::SyscallError; +use toyos_device_memory::{DmaBuffers, Registers}; + +pub mod virtio; + +/// A mapped BAR, as a driver reaches its device's registers. +#[derive(Clone, Copy)] +pub struct Bar(Window); + +impl Bar { + /// BAR `index` of the claim, `bytes` long, and the mapping the window + /// points into. + pub fn map(dev: &PciDev, index: u32, bytes: u64) -> Result<(Self, SharedMemory), KernelRefused> { + let mapped = dev.map_bar(index, bytes).map_err(KernelRefused::on("the register window"))?; + // SAFETY: `map_bar` answered `bytes` bytes of live mapping, and + // `mapped` goes to the caller with the window, which keeps both for as + // long as it keeps either. + let window = unsafe { Window::new(mapped.as_ptr(), bytes as usize) }; + Ok((Self(window), mapped)) + } +} + +impl Registers for Bar { + fn bytes(&self) -> usize { + self.0.bytes() + } + + fn read8(&self, at: usize) -> u8 { + self.0.read(at) + } + + fn read16(&self, at: usize) -> u16 { + self.0.read(at) + } + + fn read32(&self, at: usize) -> u32 { + self.0.read(at) + } + + fn write8(&self, at: usize, value: u8) { + self.0.write(at, value); + } + + fn write16(&self, at: usize, value: u16) { + self.0.write(at, value); + } + + fn write32(&self, at: usize, value: u32) { + self.0.write(at, value); + } +} + +/// A DMA grant, as a driver reaches its rings and descriptors. +#[derive(Clone, Copy)] +pub struct Grant { + window: Window, + /// Where the device reaches the grant's first byte. Not a physical address: + /// what the unit translates for this function and for nothing else. + device_base: u64, +} + +impl Grant { + /// A fresh grant of at least `bytes` for the claim's function, and the + /// region the window points into. + pub fn alloc(dev: &PciDev, bytes: u64) -> Result<(Self, DmaRegion), KernelRefused> { + let region = dev.dma_alloc(bytes).map_err(KernelRefused::on("a DMA grant"))?; + // SAFETY: the kernel rounds the request up to whole pages, never down, + // and `region` goes to the caller with the grant, which keeps both for + // as long as it keeps either. + let window = unsafe { Window::new(region.memory.as_ptr(), bytes as usize) }; + Ok((Self { window, device_base: region.device_addr }, region)) + } + + /// A grant over a leaked host allocation of `bytes`, zeroed, with the + /// address a device would be told: a driver crate's host test's. + #[cfg(feature = "host-grant")] + pub fn leaked(bytes: usize, device_base: u64) -> Self { + let backing = vec![0u64; bytes.div_ceil(8)].leak(); + // SAFETY: `leak` gives the allocation the `'static` lifetime the window + // needs, and it is at least `bytes` long. + let window = unsafe { Window::new(backing.as_mut_ptr().cast(), bytes) }; + Self { window, device_base } + } + + /// The same bytes, for the holder's own view of them: what no driver crate + /// reaches — frames, PCM. + pub fn window(&self) -> Window { + self.window + } +} + +impl DmaBuffers for Grant { + fn bytes(&self) -> usize { + self.window.bytes() + } + + fn device_addr(&self, at: usize) -> u64 { + self.device_base + at as u64 + } + + fn read16(&self, at: usize) -> u16 { + self.window.read(at) + } + + fn read32(&self, at: usize) -> u32 { + self.window.read(at) + } + + fn read64(&self, at: usize) -> u64 { + self.window.read(at) + } + + fn write16(&self, at: usize, value: u16) { + self.window.write(at, value); + } + + fn write32(&self, at: usize, value: u32) { + self.window.write(at, value); + } + + fn write64(&self, at: usize, value: u64) { + self.window.write(at, value); + } + + fn publish(&self) { + fence(Ordering::Release); + } + + fn observe(&self) { + fence(Ordering::Acquire); + } +} + +/// The kernel refused a call a bring-up cannot go on without, and the word is +/// the kernel's own. +#[derive(Clone, Copy, PartialEq, Eq, Debug)] +pub struct KernelRefused { + pub call: &'static str, + pub why: SyscallError, +} + +impl KernelRefused { + pub fn on(call: &'static str) -> impl Fn(SyscallError) -> Self { + move |why| Self { call, why } + } +} + +impl std::fmt::Display for KernelRefused { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + write!(f, "the kernel refused {}: {:?}", self.call, self.why) + } +} diff --git a/userland/toyos-pci-claim/src/virtio.rs b/userland/toyos-pci-claim/src/virtio.rs new file mode 100644 index 00000000000..2877da56f96 --- /dev/null +++ b/userland/toyos-pci-claim/src/virtio.rs @@ -0,0 +1,117 @@ +//! A virtio function's claim, brought to the features its driver and device +//! agreed: the capability walk, the register window, and §3.1.1 up to +//! `FEATURES_OK`, in the order `toyos-virtio`'s types fix. +//! +//! What is the device type's own — its queues, their vectors, its +//! configuration — begins at the [`Setup`] this answers. + +use toyos::shm::SharedMemory; +use toyos::PciDev; +use toyos_abi::syscall::{RegWidth, SyscallError}; +use toyos_virtio::pci::{vendor_caps, ConfigSpace, Layout, Offer, Setup, WalkRefusal, VIRTIO_F_ACCESS_PLATFORM}; + +use crate::{Bar, KernelRefused}; + +/// Why the function was not brought up. Each keeps its own word: a machine +/// whose kernel refused a call and a device that refused a feature set ask +/// different things of a caller. +#[derive(Clone, Copy, PartialEq, Eq, Debug)] +pub enum Refusal { + Kernel(KernelRefused), + /// The capability list, as the walk refused it. + Walk(WalkRefusal), + /// What the device published or answered, as the transport refused it. + Device(toyos_virtio::pci::Refusal), +} + +impl From for Refusal { + fn from(why: toyos_virtio::pci::Refusal) -> Self { + Self::Device(why) + } +} + +impl From for Refusal { + fn from(why: KernelRefused) -> Self { + Self::Kernel(why) + } +} + +impl std::fmt::Display for Refusal { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + match self { + Self::Kernel(why) => write!(f, "{why}"), + Self::Walk(why) => write!(f, "{why}"), + Self::Device(why) => write!(f, "{why}"), + } + } +} + +/// The claim's configuration space, as the capability walk reads it: each +/// read one `config_read` of its width, and its refusal the kernel's word. +struct ClaimConfig<'a>(&'a PciDev); + +impl ConfigSpace for ClaimConfig<'_> { + type Refused = SyscallError; + + fn read8(&self, at: u16) -> Result { + self.0.config_read(at as u32, RegWidth::U8).map(|byte| byte as u8) + } + + fn read32(&self, at: u16) -> Result { + self.0.config_read(at as u32, RegWidth::U32) + } +} + +/// A function past `FEATURES_OK`. +pub struct Negotiated { + pub setup: Setup, + /// Held for the register window's life: `setup`'s window points into it. + pub mapping: SharedMemory, + /// The driver's feature line, for it to say under its own name. + pub features: Features, +} + +/// What was offered and agreed, in the shape `iommu_virtio_platform` reads +/// back for every virtio function a machine creates. +#[derive(Clone, Copy)] +pub struct Features { + at: (u8, u8, u8), + offered: u64, + negotiated: u64, +} + +impl std::fmt::Display for Features { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + let (bus, dev, func) = self.at; + write!( + f, + "VirtIO: PCI {bus:02x}:{dev:02x}.{func} features device={:#x} negotiated={:#x} \ + access_platform={}", + self.offered, + self.negotiated, + if self.negotiated & VIRTIO_F_ACCESS_PLATFORM != 0 { 'y' } else { 'n' }, + ) + } +} + +/// Walk the claim's capabilities, map the BAR they name, and negotiate +/// `wanted` beside what `toyos-virtio` accepts wherever it is offered. +/// +/// Takes the claim's one description (`PciDev::describe`). +pub fn negotiate(dev: &PciDev, wanted: u64) -> Result { + let info = dev.describe().map_err(KernelRefused::on("the claim's description"))?; + let layout = Layout::of(&vendor_caps(&ClaimConfig(dev)).map_err(Refusal::Walk)?)?; + // The kernel reports 0 bytes for a BAR it keeps back. + let bar = layout.bar(); + let bar_bytes = *info + .bar_bytes + .get(bar as usize) + .filter(|bytes| **bytes > 0) + .ok_or(toyos_virtio::pci::Refusal::MissingCap("a BAR this claim may map"))?; + let (window, mapping) = Bar::map(dev, bar as u32, bar_bytes)?; + let offer = Offer::acknowledge(window, &layout)?; + let offered = offer.features(); + let setup = offer.accept(wanted)?; + let features = Features { at: (info.bus, info.dev, info.func), offered, negotiated: setup.features() }; + Ok(Negotiated { setup, mapping, features }) +}